[PATCH v2 0/4] alienware-wmi: Improvements

[PATCH v2 0/4] alienware-wmi: Improvements

[Kurt Borja]

Hi!

I want to migrate this driver to the new WMI interface. These are some
improvements I figured I should send beforehand.

I made everything on top of pdx86/for-next because I think they are not
fixes, just improvements. Check individual patches for more details.

Regards,
Kurt

---
v2:
 - Dropped patch 4/5 bacause it's empty after changes
 - Changed patch order:
    1 -> 3
    2 -> 4
    3 -> 1
    5 -> 2
---
Kurt Borja (4):
  alienware-wmi: Migrate to device managed resources
  alienware-wmi: Improves sysfs groups creation
  alienware-wmi: Simplify platform device creation
  alienware-wmi: Remove unnecessary check at module exit

 drivers/platform/x86/dell/alienware-wmi.c | 186 ++++++++--------------
 1 file changed, 62 insertions(+), 124 deletions(-)

-- 
2.47.0

[PATCH v2 1/4] alienware-wmi: Migrate to device managed resources

[Kurt Borja]

These resources are tied to the platform device lifetime thus make them
make them device managed. Also propagate devm_led_classdev_register() in
case of failure.

This indirectly improves module exit error handling, as potentially not
registered led class or sysfs groups are unregistered.

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
v2:
 - led_classdev_register() is now device managed
 - sysfs_create_group() is now device managed
 - Removed alienware_zone_exit() because it's empty now
---

It seems even if the led class is not registered, led_classdev_unregister
fails safely. Same for the sysfs group. If I'm wrong and this is
actually a fix please point it out.

---
 drivers/platform/x86/dell/alienware-wmi.c | 51 ++++++++---------------
 1 file changed, 17 insertions(+), 34 deletions(-)

diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
index 77465ed9b449..6760c7627f62 100644
--- a/drivers/platform/x86/dell/alienware-wmi.c
+++ b/drivers/platform/x86/dell/alienware-wmi.c
@@ -411,8 +411,6 @@ struct wmax_u32_args {
 };
 
 static struct platform_device *platform_device;
-static struct device_attribute *zone_dev_attrs;
-static struct attribute **zone_attrs;
 static struct platform_zone *zone_data;
 static struct platform_profile_handler pp_handler;
 static enum wmax_thermal_mode supported_thermal_profiles[PLATFORM_PROFILE_LAST];
@@ -624,10 +622,13 @@ static ssize_t store_control_state(struct device *dev,
 static DEVICE_ATTR(lighting_control_state, 0644, show_control_state,
 		   store_control_state);
 
-static int alienware_zone_init(struct platform_device *dev)
+static int alienware_zone_init(struct platform_device *pdev)
 {
 	u8 zone;
 	char *name;
+	struct device_attribute *zone_dev_attrs;
+	struct attribute **zone_attrs;
+	int ret;
 
 	if (interface == WMAX) {
 		lighting_control_state = WMAX_RUNNING;
@@ -644,28 +645,25 @@ static int alienware_zone_init(struct platform_device *dev)
 	 *        the lighting control + null terminated
 	 *      - zone_data num_zones is for the distinct zones
 	 */
-	zone_dev_attrs =
-	    kcalloc(quirks->num_zones + 1, sizeof(struct device_attribute),
-		    GFP_KERNEL);
+	zone_dev_attrs = devm_kcalloc(&pdev->dev, quirks->num_zones + 1,
+				      sizeof(struct device_attribute), GFP_KERNEL);
 	if (!zone_dev_attrs)
 		return -ENOMEM;
 
-	zone_attrs =
-	    kcalloc(quirks->num_zones + 2, sizeof(struct attribute *),
-		    GFP_KERNEL);
+	zone_attrs = devm_kcalloc(&pdev->dev, quirks->num_zones + 2,
+				  sizeof(struct attribute *), GFP_KERNEL);
 	if (!zone_attrs)
 		return -ENOMEM;
 
-	zone_data =
-	    kcalloc(quirks->num_zones, sizeof(struct platform_zone),
-		    GFP_KERNEL);
+	zone_data = devm_kcalloc(&pdev->dev, quirks->num_zones,
+				 sizeof(struct platform_zone), GFP_KERNEL);
 	if (!zone_data)
 		return -ENOMEM;
 
 	for (zone = 0; zone < quirks->num_zones; zone++) {
-		name = kasprintf(GFP_KERNEL, "zone%02hhX", zone);
-		if (name == NULL)
-			return 1;
+		name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "zone%02hhX", zone);
+		if (!name)
+			return -ENOMEM;
 		sysfs_attr_init(&zone_dev_attrs[zone].attr);
 		zone_dev_attrs[zone].attr.name = name;
 		zone_dev_attrs[zone].attr.mode = 0644;
@@ -678,24 +676,11 @@ static int alienware_zone_init(struct platform_device *dev)
 	zone_attrs[quirks->num_zones] = &dev_attr_lighting_control_state.attr;
 	zone_attribute_group.attrs = zone_attrs;
 
-	led_classdev_register(&dev->dev, &global_led);
-
-	return sysfs_create_group(&dev->dev.kobj, &zone_attribute_group);
-}
-
-static void alienware_zone_exit(struct platform_device *dev)
-{
-	u8 zone;
+	ret = devm_led_classdev_register(&pdev->dev, &global_led);
+	if (ret < 0)
+		return ret;
 
-	sysfs_remove_group(&dev->dev.kobj, &zone_attribute_group);
-	led_classdev_unregister(&global_led);
-	if (zone_dev_attrs) {
-		for (zone = 0; zone < quirks->num_zones; zone++)
-			kfree(zone_dev_attrs[zone].attr.name);
-	}
-	kfree(zone_dev_attrs);
-	kfree(zone_data);
-	kfree(zone_attrs);
+	return devm_device_add_group(&pdev->dev, &zone_attribute_group);
 }
 
 static acpi_status alienware_wmax_command(void *in_args, size_t in_size,
@@ -1236,7 +1221,6 @@ static int __init alienware_wmi_init(void)
 	return 0;
 
 fail_prep_zones:
-	alienware_zone_exit(platform_device);
 	remove_thermal_profile();
 fail_prep_thermal_profile:
 fail_prep_deepsleep:
@@ -1256,7 +1240,6 @@ module_init(alienware_wmi_init);
 static void __exit alienware_wmi_exit(void)
 {
 	if (platform_device) {
-		alienware_zone_exit(platform_device);
 		remove_hdmi(platform_device);
 		remove_thermal_profile();
 		platform_device_unregister(platform_device);
-- 
2.47.0

[PATCH v2 2/4] alienware-wmi: Improves sysfs groups creation

[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.

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, &amplifier_attribute_group);
-}
-
-static int create_amplifier(struct platform_device *dev)
-{
-	int ret;
-
-	ret = sysfs_create_group(&dev->dev.kobj, &amplifier_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,
+	&amplifier_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

[PATCH v2 3/4] alienware-wmi: Simplify platform device creation

[Kurt Borja]

Simplify platform device creation by using
platform_device_register_simple().

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
v2:
 - Unchanged
---
 drivers/platform/x86/dell/alienware-wmi.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
index ecab14d90b27..512f6b22585c 100644
--- a/drivers/platform/x86/dell/alienware-wmi.c
+++ b/drivers/platform/x86/dell/alienware-wmi.c
@@ -1163,14 +1163,13 @@ static int __init alienware_wmi_init(void)
 	ret = platform_driver_register(&platform_driver);
 	if (ret)
 		goto fail_platform_driver;
-	platform_device = platform_device_alloc("alienware-wmi", PLATFORM_DEVID_NONE);
-	if (!platform_device) {
-		ret = -ENOMEM;
+
+	platform_device = platform_device_register_simple("alienware-wmi",
+							  PLATFORM_DEVID_NONE, NULL, 0);
+	if (IS_ERR(platform_device)) {
+		ret = PTR_ERR(platform_device);
 		goto fail_platform_device1;
 	}
-	ret = platform_device_add(platform_device);
-	if (ret)
-		goto fail_platform_device2;
 
 	if (quirks->thermal) {
 		ret = create_thermal_profile();
@@ -1187,9 +1186,7 @@ static int __init alienware_wmi_init(void)
 fail_prep_zones:
 	remove_thermal_profile();
 fail_prep_thermal_profile:
-	platform_device_del(platform_device);
-fail_platform_device2:
-	platform_device_put(platform_device);
+	platform_device_unregister(platform_device);
 fail_platform_device1:
 	platform_driver_unregister(&platform_driver);
 fail_platform_driver:
-- 
2.47.0

[PATCH v2 4/4] alienware-wmi: Remove unnecessary check at module exit

[Kurt Borja]

Module initialization fails if platform device fails to register so it's
always not NULL at exit.

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
v2:
 - Unchanged
---
 drivers/platform/x86/dell/alienware-wmi.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
index 512f6b22585c..5e0acaf35952 100644
--- a/drivers/platform/x86/dell/alienware-wmi.c
+++ b/drivers/platform/x86/dell/alienware-wmi.c
@@ -1197,11 +1197,9 @@ module_init(alienware_wmi_init);
 
 static void __exit alienware_wmi_exit(void)
 {
-	if (platform_device) {
-		remove_thermal_profile();
-		platform_device_unregister(platform_device);
-		platform_driver_unregister(&platform_driver);
-	}
+	remove_thermal_profile();
+	platform_device_unregister(platform_device);
+	platform_driver_unregister(&platform_driver);
 }
 
 module_exit(alienware_wmi_exit);
-- 
2.47.0

Re: [PATCH v2 0/4] alienware-wmi: Improvements

[Mario Limonciello]

On 11/20/2024 10:38, Kurt Borja wrote:
> Hi!
> 
> I want to migrate this driver to the new WMI interface. These are some
> improvements I figured I should send beforehand.
> 
> I made everything on top of pdx86/for-next because I think they are not
> fixes, just improvements. Check individual patches for more details.
> 
> Regards,
> Kurt
> 
> ---
> v2:
>   - Dropped patch 4/5 bacause it's empty after changes
>   - Changed patch order:
>      1 -> 3
>      2 -> 4
>      3 -> 1
>      5 -> 2
> ---
> Kurt Borja (4):
>    alienware-wmi: Migrate to device managed resources
>    alienware-wmi: Improves sysfs groups creation
>    alienware-wmi: Simplify platform device creation
>    alienware-wmi: Remove unnecessary check at module exit
> 
>   drivers/platform/x86/dell/alienware-wmi.c | 186 ++++++++--------------
>   1 file changed, 62 insertions(+), 124 deletions(-)
> 

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

Re: [PATCH v2 2/4] alienware-wmi: Improves sysfs groups creation

[Kurt Borja]

On Wed, Nov 20, 2024 at 01:43:15PM -0300, Kurt Borja wrote:
> 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;
>  };
>  
> +

Just realized the extra line here.

>  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, &amplifier_attribute_group);
> -}
> -
> -static int create_amplifier(struct platform_device *dev)
> -{
> -	int ret;
> -
> -	ret = sysfs_create_group(&dev->dev.kobj, &amplifier_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,
> +	&amplifier_attribute_group,
> +	&deepsleep_attribute_group,
> +	NULL
> +};

And this is not static.

I'll fix it.

> [...]