[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241120164314.7177-2-kuurtb@gmail.com>
Date: Wed, 20 Nov 2024 13:43:15 -0300
From: Kurt Borja <kuurtb@...il.com>
To: kuurtb@...il.com
Cc: Dell.Client.Kernel@...l.com,
hdegoede@...hat.com,
ilpo.jarvinen@...ux.intel.com,
linux-kernel@...r.kernel.org,
mario.limonciello@....com,
platform-driver-x86@...r.kernel.org,
w_armin@....de
Subject: [PATCH v2 2/4] alienware-wmi: Improves sysfs groups creation
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.
Signed-off-by: Kurt Borja <kuurtb@...il.com>
---
v2:
- Drop the helpers approach in favor of letting the driver manage the
wmax sysfs groups
---
Again I think not cleaning up created sysfs group is not actually a bug
so this is only an improvement.
---
drivers/platform/x86/dell/alienware-wmi.c | 112 +++++++---------------
1 file changed, 36 insertions(+), 76 deletions(-)
diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
index 6760c7627f62..ecab14d90b27 100644
--- a/drivers/platform/x86/dell/alienware-wmi.c
+++ b/drivers/platform/x86/dell/alienware-wmi.c
@@ -410,17 +410,12 @@ struct wmax_u32_args {
u8 arg3;
};
+
static struct platform_device *platform_device;
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",
};
@@ -781,6 +776,12 @@ static ssize_t toggle_hdmi_source(struct device *dev,
return count;
}
+static bool hdmi_group_visible(struct kobject *kobj)
+{
+ return quirks->hdmi_mux;
+}
+DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(hdmi);
+
static DEVICE_ATTR(cable, S_IRUGO, show_hdmi_cable, NULL);
static DEVICE_ATTR(source, S_IRUGO | S_IWUSR, show_hdmi_source,
toggle_hdmi_source);
@@ -793,25 +794,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
@@ -838,6 +824,12 @@ static ssize_t show_amplifier_status(struct device *dev,
return sysfs_emit(buf, "unconnected connected [unknown]\n");
}
+static bool amplifier_group_visible(struct kobject *kobj)
+{
+ return quirks->amplifier;
+}
+DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(amplifier);
+
static DEVICE_ATTR(status, S_IRUGO, show_amplifier_status, NULL);
static struct attribute *amplifier_attrs[] = {
@@ -847,25 +839,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
@@ -916,6 +893,12 @@ static ssize_t toggle_deepsleep(struct device *dev,
return count;
}
+static bool deepsleep_group_visible(struct kobject *kobj)
+{
+ return quirks->deepslp;
+}
+DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(deepsleep);
+
static DEVICE_ATTR(deepsleep, S_IRUGO | S_IWUSR, show_deepsleep_status, toggle_deepsleep);
static struct attribute *deepsleep_attrs[] = {
@@ -925,25 +908,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
@@ -1151,6 +1119,20 @@ static void remove_thermal_profile(void)
platform_profile_remove();
}
+const struct attribute_group *wmax_groups[] = {
+ &hdmi_attribute_group,
+ &lifier_attribute_group,
+ &deepsleep_attribute_group,
+ NULL
+};
+
+static struct platform_driver platform_driver = {
+ .driver = {
+ .name = "alienware-wmi",
+ .dev_groups = wmax_groups,
+ }
+};
+
static int __init alienware_wmi_init(void)
{
int ret;
@@ -1190,24 +1172,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();
if (ret)
@@ -1223,9 +1187,6 @@ static int __init alienware_wmi_init(void)
fail_prep_zones:
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);
@@ -1240,7 +1201,6 @@ module_init(alienware_wmi_init);
static void __exit alienware_wmi_exit(void)
{
if (platform_device) {
- remove_hdmi(platform_device);
remove_thermal_profile();
platform_device_unregister(platform_device);
platform_driver_unregister(&platform_driver);
--
2.47.0
Powered by blists - more mailing lists