sysfs: Introduce macros for attribute groups with visibility control

[PATCH 6/7] platform/x86/dell: alienware-wmi: update sysfs visibility macros

Posted by David E. Box 7 months, 3 weeks ago

Replace deprecated visibility macros and align group naming with new API.

In alienware-wmi-base.c, use NAMED_ATTRIBUTE_GROUP_COMBO_VISIBLE(rgb_zones)
to define the rgb_zones attribute group concisely. To preserve the existing
userspace ABI, rename zone_attr_visible and rgb_zones_attr_visible to
zone_group_visible and rgb_zones_group_visible, respectively, to reflect the
'rgb_zones' group.

In alienware-wmi-wmax.c, replace DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE() with
the renamed DEFINE_SYSFS_GROUP_VISIBILITY() macro for the hdmi, amplifier,
and deepsleep attributes to match the updated API.

While here, add missing sysfs.h include and sort headers alphabetically. No
functional changes are intended.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
 .../platform/x86/dell/alienware-wmi-base.c    | 23 ++++++++-----------
 .../platform/x86/dell/alienware-wmi-wmax.c    |  7 +++---
 2 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/platform/x86/dell/alienware-wmi-base.c b/drivers/platform/x86/dell/alienware-wmi-base.c
index 64562b92314f..ee41892e562c 100644
--- a/drivers/platform/x86/dell/alienware-wmi-base.c
+++ b/drivers/platform/x86/dell/alienware-wmi-base.c
@@ -10,10 +10,11 @@
 
 #include <linux/acpi.h>
 #include <linux/cleanup.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
 #include <linux/dmi.h>
 #include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/sysfs.h>
 #include "alienware-wmi.h"
 
 MODULE_AUTHOR("Mario Limonciello <mario.limonciello@outlook.com>");
@@ -326,8 +327,8 @@ static ssize_t lighting_control_state_store(struct device *dev,
 
 static DEVICE_ATTR_RW(lighting_control_state);
 
-static umode_t zone_attr_visible(struct kobject *kobj,
-				 struct attribute *attr, int n)
+static umode_t rgb_zones_attr_visible(struct kobject *kobj,
+				      struct attribute *attr, int n)
 {
 	if (n < alienfx->num_zones + 1)
 		return attr->mode;
@@ -335,13 +336,12 @@ static umode_t zone_attr_visible(struct kobject *kobj,
 	return 0;
 }
 
-static bool zone_group_visible(struct kobject *kobj)
+static bool rgb_zones_group_visible(struct kobject *kobj)
 {
 	return alienfx->num_zones > 0;
 }
-DEFINE_SYSFS_GROUP_VISIBLE(zone);
 
-static struct attribute *zone_attrs[] = {
+static struct attribute *rgb_zones_attrs[] = {
 	&dev_attr_lighting_control_state.attr,
 	&dev_attr_zone00.attr,
 	&dev_attr_zone01.attr,
@@ -349,12 +349,7 @@ static struct attribute *zone_attrs[] = {
 	&dev_attr_zone03.attr,
 	NULL
 };
-
-static struct attribute_group zone_attribute_group = {
-	.name = "rgb_zones",
-	.is_visible = SYSFS_GROUP_VISIBLE(zone),
-	.attrs = zone_attrs,
-};
+NAMED_ATTRIBUTE_GROUP_COMBO_VISIBLE(rgb_zones);
 
 /*
  * LED Brightness (Global)
@@ -410,7 +405,7 @@ static int alienfx_probe(struct platform_device *pdev)
 }
 
 static const struct attribute_group *alienfx_groups[] = {
-	&zone_attribute_group,
+	&rgb_zones_group,
 	WMAX_DEV_GROUPS
 	NULL
 };
diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
index 0c3be03385f8..559415849bcc 100644
--- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
+++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
@@ -13,6 +13,7 @@
 #include <linux/dmi.h>
 #include <linux/moduleparam.h>
 #include <linux/platform_profile.h>
+#include <linux/sysfs.h>
 #include <linux/wmi.h>
 #include "alienware-wmi.h"
 
@@ -356,7 +357,7 @@ static bool hdmi_group_visible(struct kobject *kobj)
 {
 	return alienware_interface == WMAX && alienfx->hdmi_mux;
 }
-DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(hdmi);
+DEFINE_SYSFS_GROUP_VISIBILITY(hdmi);
 
 static struct attribute *hdmi_attrs[] = {
 	&dev_attr_cable.attr,
@@ -404,7 +405,7 @@ static bool amplifier_group_visible(struct kobject *kobj)
 {
 	return alienware_interface == WMAX && alienfx->amplifier;
 }
-DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(amplifier);
+DEFINE_SYSFS_GROUP_VISIBILITY(amplifier);
 
 static struct attribute *amplifier_attrs[] = {
 	&dev_attr_status.attr,
@@ -475,7 +476,7 @@ static bool deepsleep_group_visible(struct kobject *kobj)
 {
 	return alienware_interface == WMAX && alienfx->deepslp;
 }
-DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(deepsleep);
+DEFINE_SYSFS_GROUP_VISIBILITY(deepsleep);
 
 static struct attribute *deepsleep_attrs[] = {
 	&dev_attr_deepsleep.attr,
-- 
2.43.0

Re: [PATCH 6/7] platform/x86/dell: alienware-wmi: update sysfs visibility macros

Posted by Dan Williams 7 months, 3 weeks ago

David E. Box wrote:
> Replace deprecated visibility macros and align group naming with new API.
> 
> In alienware-wmi-base.c, use NAMED_ATTRIBUTE_GROUP_COMBO_VISIBLE(rgb_zones)
> to define the rgb_zones attribute group concisely. To preserve the existing
> userspace ABI, rename zone_attr_visible and rgb_zones_attr_visible to
> zone_group_visible and rgb_zones_group_visible, respectively, to reflect the
> 'rgb_zones' group.
> 
> In alienware-wmi-wmax.c, replace DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE() with
> the renamed DEFINE_SYSFS_GROUP_VISIBILITY() macro for the hdmi, amplifier,
> and deepsleep attributes to match the updated API.
> 
> While here, add missing sysfs.h include and sort headers alphabetically. No
> functional changes are intended.
> 
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
>  .../platform/x86/dell/alienware-wmi-base.c    | 23 ++++++++-----------
>  .../platform/x86/dell/alienware-wmi-wmax.c    |  7 +++---
>  2 files changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/alienware-wmi-base.c b/drivers/platform/x86/dell/alienware-wmi-base.c
> index 64562b92314f..ee41892e562c 100644
> --- a/drivers/platform/x86/dell/alienware-wmi-base.c
> +++ b/drivers/platform/x86/dell/alienware-wmi-base.c
> @@ -10,10 +10,11 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/cleanup.h>
> -#include <linux/module.h>
> -#include <linux/platform_device.h>
>  #include <linux/dmi.h>
>  #include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/sysfs.h>
>  #include "alienware-wmi.h"
>  
>  MODULE_AUTHOR("Mario Limonciello <mario.limonciello@outlook.com>");
> @@ -326,8 +327,8 @@ static ssize_t lighting_control_state_store(struct device *dev,
>  
>  static DEVICE_ATTR_RW(lighting_control_state);
>  
> -static umode_t zone_attr_visible(struct kobject *kobj,
> -				 struct attribute *attr, int n)
> +static umode_t rgb_zones_attr_visible(struct kobject *kobj,
> +				      struct attribute *attr, int n)
>  {
>  	if (n < alienfx->num_zones + 1)
>  		return attr->mode;
> @@ -335,13 +336,12 @@ static umode_t zone_attr_visible(struct kobject *kobj,
>  	return 0;
>  }
>  
> -static bool zone_group_visible(struct kobject *kobj)
> +static bool rgb_zones_group_visible(struct kobject *kobj)
>  {
>  	return alienfx->num_zones > 0;
>  }
> -DEFINE_SYSFS_GROUP_VISIBLE(zone);
>  
> -static struct attribute *zone_attrs[] = {
> +static struct attribute *rgb_zones_attrs[] = {

Yes, this rename is necessary to fit the scheme.

Looks good,

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Re: [PATCH 6/7] platform/x86/dell: alienware-wmi: update sysfs visibility macros

Posted by Kurt Borja 7 months, 3 weeks ago

On Wed Apr 23, 2025 at 2:50 PM -03, David E. Box wrote:
> Replace deprecated visibility macros and align group naming with new API.
>
> In alienware-wmi-base.c, use NAMED_ATTRIBUTE_GROUP_COMBO_VISIBLE(rgb_zones)
> to define the rgb_zones attribute group concisely. To preserve the existing
> userspace ABI, rename zone_attr_visible and rgb_zones_attr_visible to
> zone_group_visible and rgb_zones_group_visible, respectively, to reflect the
> 'rgb_zones' group.
>
> In alienware-wmi-wmax.c, replace DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE() with
> the renamed DEFINE_SYSFS_GROUP_VISIBILITY() macro for the hdmi, amplifier,
> and deepsleep attributes to match the updated API.
>
> While here, add missing sysfs.h include and sort headers alphabetically. No
> functional changes are intended.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>

Thanks! I like these new macros.

Reviewed-by: Kurt Borja <kuurtb@gmail.com>

Small comment bellow.

> ---
>  .../platform/x86/dell/alienware-wmi-base.c    | 23 ++++++++-----------
>  .../platform/x86/dell/alienware-wmi-wmax.c    |  7 +++---
>  2 files changed, 13 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/platform/x86/dell/alienware-wmi-base.c b/drivers/platform/x86/dell/alienware-wmi-base.c
> index 64562b92314f..ee41892e562c 100644
> --- a/drivers/platform/x86/dell/alienware-wmi-base.c
> +++ b/drivers/platform/x86/dell/alienware-wmi-base.c
> @@ -10,10 +10,11 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/cleanup.h>
> -#include <linux/module.h>
> -#include <linux/platform_device.h>
>  #include <linux/dmi.h>
>  #include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/sysfs.h>
>  #include "alienware-wmi.h"
>  
>  MODULE_AUTHOR("Mario Limonciello <mario.limonciello@outlook.com>");
> @@ -326,8 +327,8 @@ static ssize_t lighting_control_state_store(struct device *dev,
>  
>  static DEVICE_ATTR_RW(lighting_control_state);
>  
> -static umode_t zone_attr_visible(struct kobject *kobj,
> -				 struct attribute *attr, int n)
> +static umode_t rgb_zones_attr_visible(struct kobject *kobj,
> +				      struct attribute *attr, int n)
>  {
>  	if (n < alienfx->num_zones + 1)
>  		return attr->mode;
> @@ -335,13 +336,12 @@ static umode_t zone_attr_visible(struct kobject *kobj,
>  	return 0;
>  }
>  
> -static bool zone_group_visible(struct kobject *kobj)
> +static bool rgb_zones_group_visible(struct kobject *kobj)
>  {
>  	return alienfx->num_zones > 0;
>  }
> -DEFINE_SYSFS_GROUP_VISIBLE(zone);
>  
> -static struct attribute *zone_attrs[] = {
> +static struct attribute *rgb_zones_attrs[] = {
>  	&dev_attr_lighting_control_state.attr,
>  	&dev_attr_zone00.attr,
>  	&dev_attr_zone01.attr,
> @@ -349,12 +349,7 @@ static struct attribute *zone_attrs[] = {
>  	&dev_attr_zone03.attr,
>  	NULL
>  };
> -
> -static struct attribute_group zone_attribute_group = {
> -	.name = "rgb_zones",
> -	.is_visible = SYSFS_GROUP_VISIBLE(zone),
> -	.attrs = zone_attrs,
> -};
> +NAMED_ATTRIBUTE_GROUP_COMBO_VISIBLE(rgb_zones);
>  
>  /*
>   * LED Brightness (Global)
> @@ -410,7 +405,7 @@ static int alienfx_probe(struct platform_device *pdev)
>  }
>  
>  static const struct attribute_group *alienfx_groups[] = {
> -	&zone_attribute_group,
> +	&rgb_zones_group,
>  	WMAX_DEV_GROUPS
>  	NULL
>  };
> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> index 0c3be03385f8..559415849bcc 100644
> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> @@ -13,6 +13,7 @@
>  #include <linux/dmi.h>
>  #include <linux/moduleparam.h>
>  #include <linux/platform_profile.h>
> +#include <linux/sysfs.h>

JFYI, this line conflicts with linux-next.

-- 
 ~ Kurt

>  #include <linux/wmi.h>
>  #include "alienware-wmi.h"
>  
> @@ -356,7 +357,7 @@ static bool hdmi_group_visible(struct kobject *kobj)
>  {
>  	return alienware_interface == WMAX && alienfx->hdmi_mux;
>  }
> -DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(hdmi);
> +DEFINE_SYSFS_GROUP_VISIBILITY(hdmi);
>  
>  static struct attribute *hdmi_attrs[] = {
>  	&dev_attr_cable.attr,
> @@ -404,7 +405,7 @@ static bool amplifier_group_visible(struct kobject *kobj)
>  {
>  	return alienware_interface == WMAX && alienfx->amplifier;
>  }
> -DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(amplifier);
> +DEFINE_SYSFS_GROUP_VISIBILITY(amplifier);
>  
>  static struct attribute *amplifier_attrs[] = {
>  	&dev_attr_status.attr,
> @@ -475,7 +476,7 @@ static bool deepsleep_group_visible(struct kobject *kobj)
>  {
>  	return alienware_interface == WMAX && alienfx->deepslp;
>  }
> -DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(deepsleep);
> +DEFINE_SYSFS_GROUP_VISIBILITY(deepsleep);
>  
>  static struct attribute *deepsleep_attrs[] = {
>  	&dev_attr_deepsleep.attr,

[PATCH 1/7] sysfs: Rename attribute group visibility macros
[PATCH 2/7] sysfs: Introduce macros to simplify creation of visible attribute groups
[PATCH 3/7] docs: sysfs.rst: document additional attribute group macros
[PATCH 4/7] pci: doe: Replace sysfs visibility macro
[PATCH 5/7] soundwire: sysfs: Use ATTRIBUTE_GROUP_VISIBLE()
[PATCH 6/7] platform/x86/dell: alienware-wmi: update sysfs visibility macros
[PATCH 7/7] sysfs: Remove transitional attribute group alias macros