From: Kurt Borja <kuurtb@gmail.com>
To: kuurtb@gmail.com
Cc: Dell.Client.Kernel@dell.com, hdegoede@redhat.com,
ilpo.jarvinen@linux.intel.com, linux-kernel@vger.kernel.org,
mario.limonciello@amd.com, platform-driver-x86@vger.kernel.org,
w_armin@gmx.de
Subject: [PATCH v2 2/4] alienware-wmi: Improves sysfs groups creation
Date: Wed, 20 Nov 2024 13:43:15 -0300 [thread overview]
Message-ID: <20241120164314.7177-2-kuurtb@gmail.com> (raw)
In-Reply-To: <20241120163834.6446-3-kuurtb@gmail.com>
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@gmail.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
next prev parent reply other threads:[~2024-11-20 16:43 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-20 16:38 [PATCH v2 0/4] alienware-wmi: Improvements Kurt Borja
2024-11-20 16:41 ` [PATCH v2 1/4] alienware-wmi: Migrate to device managed resources Kurt Borja
2024-11-20 16:43 ` Kurt Borja [this message]
2024-11-21 15:37 ` [PATCH v2 2/4] alienware-wmi: Improves sysfs groups creation Kurt Borja
2024-11-20 16:43 ` [PATCH v2 3/4] alienware-wmi: Simplify platform device creation Kurt Borja
2024-11-20 16:44 ` [PATCH v2 4/4] alienware-wmi: Remove unnecessary check at module exit Kurt Borja
2024-11-20 18:08 ` [PATCH v2 0/4] alienware-wmi: Improvements Mario Limonciello
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241120164314.7177-2-kuurtb@gmail.com \
--to=kuurtb@gmail.com \
--cc=Dell.Client.Kernel@dell.com \
--cc=hdegoede@redhat.com \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mario.limonciello@amd.com \
--cc=platform-driver-x86@vger.kernel.org \
--cc=w_armin@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.