[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241119043534.25683-1-kuurtb@gmail.com>
Date: Tue, 19 Nov 2024 01:35:34 -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 5/5] 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 in favor of two helpers that make use
of sysfs_create_groups/sysfs_remove_groups to cleanly create/remove
groups at module init/exit.
Signed-off-by: Kurt Borja <kuurtb@...il.com>
---
I have a question. Do the created sysfs groups get removed when their
kobj reference count goes to 0? I ask because I want to know if this is
a bug fix.
---
drivers/platform/x86/dell/alienware-wmi.c | 105 ++++++++--------------
1 file changed, 36 insertions(+), 69 deletions(-)
diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
index 44f1f7b57d0a..e9ed2089cba0 100644
--- a/drivers/platform/x86/dell/alienware-wmi.c
+++ b/drivers/platform/x86/dell/alienware-wmi.c
@@ -410,8 +410,10 @@ struct wmax_u32_args {
u8 arg3;
};
+
static struct platform_device *platform_device;
static struct platform_zone *zone_data;
+const struct attribute_group *wmax_groups[4];
static struct platform_profile_handler pp_handler;
static enum wmax_thermal_mode supported_thermal_profiles[PLATFORM_PROFILE_LAST];
@@ -810,22 +812,6 @@ static const struct attribute_group hdmi_attribute_group = {
.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
@@ -864,22 +850,6 @@ static const struct attribute_group amplifier_attribute_group = {
.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
@@ -942,22 +912,6 @@ static const struct attribute_group deepsleep_attribute_group = {
.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
@@ -1165,6 +1119,34 @@ static void remove_thermal_profile(void)
platform_profile_remove();
}
+static int __init create_wmax_groups(struct platform_device *pdev)
+{
+ int no_groups = 0;
+
+ if (quirks->hdmi_mux) {
+ wmax_groups[no_groups] = &hdmi_attribute_group;
+ no_groups++;
+ }
+
+ if (quirks->amplifier) {
+ wmax_groups[no_groups] = &lifier_attribute_group;
+ no_groups++;
+ }
+
+ if (quirks->deepslp) {
+ wmax_groups[no_groups] = &deepsleep_attribute_group;
+ no_groups++;
+ }
+
+ return no_groups > 0 ? device_add_groups(&pdev->dev, wmax_groups) : 0;
+}
+
+static void __exit remove_wmax_groups(struct platform_device *pdev)
+{
+ if (!wmax_groups[0])
+ device_remove_groups(&pdev->dev, wmax_groups);
+}
+
static int __init alienware_wmi_init(void)
{
int ret;
@@ -1203,23 +1185,9 @@ static int __init alienware_wmi_init(void)
goto fail_platform_device1;
}
- 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;
- }
+ ret = create_wmax_groups(platform_device);
+ if (ret)
+ goto fail_prep_groups;
if (quirks->thermal) {
ret = create_thermal_profile();
@@ -1236,9 +1204,8 @@ 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:
+ remove_wmax_groups(platform_device);
+fail_prep_groups:
platform_device_unregister(platform_device);
fail_platform_device1:
platform_driver_unregister(&platform_driver);
@@ -1251,7 +1218,7 @@ module_init(alienware_wmi_init);
static void __exit alienware_wmi_exit(void)
{
alienware_zone_exit(platform_device);
- remove_hdmi(platform_device);
+ remove_wmax_groups(platform_device);
remove_thermal_profile();
platform_device_unregister(platform_device);
platform_driver_unregister(&platform_driver);
--
2.47.0
Powered by blists - more mailing lists