Am 21.12.24 um 06:59 schrieb Kurt Borja:
Devices with hdmi_mux, amplifier or deepslp quirks create a sysfs group
for each available feature. To accomplish this, helper create/remove
functions were called on module init, but they had the following
problems:
- Create helpers called remove helpers on failure, which in turn tried
to remove the sysfs group that failed to be created
- If group creation failed mid way, previous successfully created groups
were not cleaned up
- Module exit only removed hdmi_mux group
To improve this, drop all helpers and let the platform driver manage these
sysfs groups, while controlling visibility with their respective quirks.
Reviewed-by: Armin Wolf <W_Armin@xxxxxx>
Reviewed-by: Mario Limonciello <mario.limonciello@xxxxxxx>
Signed-off-by: Kurt Borja <kuurtb@xxxxxxxxx>
---
drivers/platform/x86/dell/alienware-wmi.c | 114 ++++++++--------------
1 file changed, 38 insertions(+), 76 deletions(-)
diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
index 273ce9b10765..e010c94555e8 100644
--- a/drivers/platform/x86/dell/alienware-wmi.c
+++ b/drivers/platform/x86/dell/alienware-wmi.c
@@ -417,12 +417,6 @@ static struct platform_zone *zone_data;
static struct platform_profile_handler pp_handler;
static enum wmax_thermal_mode supported_thermal_profiles[PLATFORM_PROFILE_LAST];
-static struct platform_driver platform_driver = {
- .driver = {
- .name = "alienware-wmi",
- }
-};
-
static struct attribute_group zone_attribute_group = {
.name = "rgb_zones",
};
@@ -804,6 +798,12 @@ static DEVICE_ATTR(cable, S_IRUGO, show_hdmi_cable, NULL);
static DEVICE_ATTR(source, S_IRUGO | S_IWUSR, show_hdmi_source,
toggle_hdmi_source);
+static bool hdmi_group_visible(struct kobject *kobj)
+{
+ return quirks->hdmi_mux;
+}
+DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(hdmi);
+
static struct attribute *hdmi_attrs[] = {
&dev_attr_cable.attr,
&dev_attr_source.attr,
@@ -812,25 +812,10 @@ static struct attribute *hdmi_attrs[] = {
static const struct attribute_group hdmi_attribute_group = {
.name = "hdmi",
+ .is_visible = SYSFS_GROUP_VISIBLE(hdmi),
.attrs = hdmi_attrs,
};
-static void remove_hdmi(struct platform_device *dev)
-{
- if (quirks->hdmi_mux > 0)
- sysfs_remove_group(&dev->dev.kobj, &hdmi_attribute_group);
-}
-
-static int create_hdmi(struct platform_device *dev)
-{
- int ret;
-
- ret = sysfs_create_group(&dev->dev.kobj, &hdmi_attribute_group);
- if (ret)
- remove_hdmi(dev);
- return ret;
-}
-
/*
* Alienware GFX amplifier support
* - Currently supports reading cable status
@@ -859,6 +844,12 @@ static ssize_t show_amplifier_status(struct device *dev,
static DEVICE_ATTR(status, S_IRUGO, show_amplifier_status, NULL);
+static bool amplifier_group_visible(struct kobject *kobj)
+{
+ return quirks->amplifier;
+}
+DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(amplifier);
+
static struct attribute *amplifier_attrs[] = {
&dev_attr_status.attr,
NULL,
@@ -866,25 +857,10 @@ static struct attribute *amplifier_attrs[] = {
static const struct attribute_group amplifier_attribute_group = {
.name = "amplifier",
+ .is_visible = SYSFS_GROUP_VISIBLE(amplifier),
.attrs = amplifier_attrs,
};
-static void remove_amplifier(struct platform_device *dev)
-{
- if (quirks->amplifier > 0)
- sysfs_remove_group(&dev->dev.kobj, &lifier_attribute_group);
-}
-
-static int create_amplifier(struct platform_device *dev)
-{
- int ret;
-
- ret = sysfs_create_group(&dev->dev.kobj, &lifier_attribute_group);
- if (ret)
- remove_amplifier(dev);
- return ret;
-}
-
/*
* Deep Sleep Control support
* - Modifies BIOS setting for deep sleep control allowing extra wakeup events
@@ -937,6 +913,12 @@ static ssize_t toggle_deepsleep(struct device *dev,
static DEVICE_ATTR(deepsleep, S_IRUGO | S_IWUSR, show_deepsleep_status, toggle_deepsleep);
+static bool deepsleep_group_visible(struct kobject *kobj)
+{
+ return quirks->deepslp;
+}
+DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(deepsleep);
+
static struct attribute *deepsleep_attrs[] = {
&dev_attr_deepsleep.attr,
NULL,
@@ -944,25 +926,10 @@ static struct attribute *deepsleep_attrs[] = {
static const struct attribute_group deepsleep_attribute_group = {
.name = "deepsleep",
+ .is_visible = SYSFS_GROUP_VISIBLE(deepsleep),
.attrs = deepsleep_attrs,
};
-static void remove_deepsleep(struct platform_device *dev)
-{
- if (quirks->deepslp > 0)
- sysfs_remove_group(&dev->dev.kobj, &deepsleep_attribute_group);
-}
-
-static int create_deepsleep(struct platform_device *dev)
-{
- int ret;
-
- ret = sysfs_create_group(&dev->dev.kobj, &deepsleep_attribute_group);
- if (ret)
- remove_deepsleep(dev);
- return ret;
-}
-
/*
* Thermal Profile control
* - Provides thermal profile control through the Platform Profile API
@@ -1172,6 +1139,23 @@ static void remove_thermal_profile(void)
platform_profile_remove(&pp_handler);
}
+/*
+ * Platform Driver
+ */
+static const struct attribute_group *alienfx_groups[] = {
+ &hdmi_attribute_group,
+ &lifier_attribute_group,
+ &deepsleep_attribute_group,
+ NULL
+};
+
+static struct platform_driver platform_driver = {
+ .driver = {
+ .name = "alienware-wmi",
+ .dev_groups = alienfx_groups,
+ },
+};
+
static int __init alienware_wmi_init(void)
{
int ret;
@@ -1211,24 +1195,6 @@ static int __init alienware_wmi_init(void)
if (ret)
goto fail_platform_device2;
- if (quirks->hdmi_mux > 0) {
- ret = create_hdmi(platform_device);
- if (ret)
- goto fail_prep_hdmi;
- }
-
- if (quirks->amplifier > 0) {
- ret = create_amplifier(platform_device);
- if (ret)
- goto fail_prep_amplifier;
- }
-
- if (quirks->deepslp > 0) {
- ret = create_deepsleep(platform_device);
- if (ret)
- goto fail_prep_deepsleep;
- }
-
if (quirks->thermal) {
ret = create_thermal_profile(platform_device);
if (ret)
@@ -1245,9 +1211,6 @@ static int __init alienware_wmi_init(void)
alienware_zone_exit(platform_device);
remove_thermal_profile();
fail_prep_thermal_profile:
-fail_prep_deepsleep:
-fail_prep_amplifier:
-fail_prep_hdmi:
platform_device_del(platform_device);
fail_platform_device2:
platform_device_put(platform_device);
@@ -1262,7 +1225,6 @@ module_init(alienware_wmi_init);
static void __exit alienware_wmi_exit(void)
{
alienware_zone_exit(platform_device);
- remove_hdmi(platform_device);
remove_thermal_profile();
platform_device_unregister(platform_device);
platform_driver_unregister(&platform_driver);
