[PATCH 0/7] sysfs: Introduce macros for attribute groups with visibility control

[PATCH 0/7] sysfs: Introduce macros for attribute groups with visibility control

[David E. Box]

The ATTRIBUTE_GROUP() helper does not support adding an .is_visible
function for visibility control. With the introduction of
SYSFS_GROUP_VISIBLE, DEFINE_SYSFS_GROUP_VISIBLE, and related macros,
attribute group definitions can now fully encapsulate visibility logic
while eliminating boilerplate code.

The following new macros are introduced:

        NAMED_ATTRIBUTE_GROUP_VISIBLE()
        NAMED_ATTRIBUTE_GROUPS_VISIBLE()
        NAMED_ATTRIBUTE_GROUP_COMBO_VISIBLE()
        NAMED_ATTRIBUTE_GROUPS_COMBO_VISIBLE()

This isn’t just a cleanup effort — I plan to use these macros in new driver
code I'm working on, and wanted to avoid having to open-code these common
visibility patterns yet again. Documenting and generalizing them now will
help avoid duplication and make future code easier to read and maintain.

These macros integrate visibility logic directly into attribute group
definitions, improving readability and maintainability. The
DEFINE[_SIMPLE_]ATTRIBUTE_GROUP_VISIBLE() macros current have four users.
Two out of them could be modified. The usbtouchscreen driver uses the @name
field which isn't supported by ATTRIBUTE_GROUPS(). But for the ones that
could be modified the diffstat was significant:

 drivers/pci/doe.c                              |  2 +-
 drivers/platform/x86/dell/alienware-wmi-base.c | 23 +++++++++--------------
 drivers/platform/x86/dell/alienware-wmi-wmax.c |  7 ++++---
 drivers/soundwire/sysfs_slave.c                | 32 +++++++++++++-------------------
 4 files changed, 27 insertions(+), 37 deletions(-)

David E. Box (7):
  sysfs: Rename attribute group visibility macros
  sysfs: Introduce macros to simplify creation of visible attribute
    groups
  docs: sysfs.rst: document additional attribute group macros
  pci: doe: Replace sysfs visibility macro
  soundwire: sysfs: Use ATTRIBUTE_GROUP_VISIBLE()
  platform/x86/dell: alienware-wmi: update sysfs visibility macros
  sysfs: Remove transitional attribute group alias macros

 Documentation/filesystems/sysfs.rst           | 244 ++++++++++++++++++
 drivers/pci/doe.c                             |   2 +-
 .../platform/x86/dell/alienware-wmi-base.c    |  23 +-
 .../platform/x86/dell/alienware-wmi-wmax.c    |   7 +-
 drivers/soundwire/sysfs_slave.c               |  32 +--
 include/linux/sysfs.h                         |  46 +++-
 6 files changed, 306 insertions(+), 48 deletions(-)


base-commit: 9c32cda43eb78f78c73aee4aa344b777714e259b
-- 
2.43.0

[PATCH 1/7] sysfs: Rename attribute group visibility macros

[David E. Box]

Update the naming of several macros to provide clearer semantics for
controlling group and attribute visibility per Dan Williams' suggestion.
Also, add transitional aliases mapping the old macro names to the new ones
so that driver code remains functional before changes are again made in a
future macro encapsulation patch. This approach ensures that when the
encapsulation work is applied, drivers will only need to be updated once
without breaking compatibility.

Suggested-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
 include/linux/sysfs.h | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 18f7e1fd093c..00dc88776f21 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -116,7 +116,7 @@ struct attribute_group {
 #define SYSFS_GROUP_INVISIBLE	020000
 
 /*
- * DEFINE_SYSFS_GROUP_VISIBLE(name):
+ * DEFINE_SYSFS_GROUP_COMBO_VISIBILITY(name):
  *	A helper macro to pair with the assignment of ".is_visible =
  *	SYSFS_GROUP_VISIBLE(name)", that arranges for the directory
  *	associated with a named attribute_group to optionally be hidden.
@@ -142,7 +142,7 @@ struct attribute_group {
  *       return true;
  * }
  *
- * DEFINE_SYSFS_GROUP_VISIBLE(example);
+ * DEFINE_SYSFS_GROUP_COMBO_VISIBILITY(example);
  *
  * static struct attribute_group example_group = {
  *       .name = "example",
@@ -153,9 +153,9 @@ struct attribute_group {
  * Note that it expects <name>_attr_visible and <name>_group_visible to
  * be defined. For cases where individual attributes do not need
  * separate visibility consideration, only entire group visibility at
- * once, see DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE().
+ * once, see DEFINE_SYSFS_GROUP_VISIBILITY().
  */
-#define DEFINE_SYSFS_GROUP_VISIBLE(name)                             \
+#define DEFINE_SYSFS_GROUP_COMBO_VISIBILITY(name)                          \
 	static inline umode_t sysfs_group_visible_##name(            \
 		struct kobject *kobj, struct attribute *attr, int n) \
 	{                                                            \
@@ -165,9 +165,9 @@ struct attribute_group {
 	}
 
 /*
- * DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(name):
+ * DEFINE_SYSFS_GROUP_VISIBILITY(name):
  *	A helper macro to pair with SYSFS_GROUP_VISIBLE() that like
- *	DEFINE_SYSFS_GROUP_VISIBLE() controls group visibility, but does
+ *	DEFINE_SYSFS_GROUP_COMBO_VISIBILITY() controls group visibility, but does
  *	not require the implementation of a per-attribute visibility
  *	callback.
  * Ex.
@@ -179,7 +179,7 @@ struct attribute_group {
  *       return true;
  * }
  *
- * DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(example);
+ * DEFINE_SYSFS_GROUP_VISIBILITY(example);
  *
  * static struct attribute_group example_group = {
  *       .name = "example",
@@ -187,7 +187,7 @@ struct attribute_group {
  *       .attrs = &example_attrs,
  * };
  */
-#define DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(name)                   \
+#define DEFINE_SYSFS_GROUP_VISIBILITY(name)                 \
 	static inline umode_t sysfs_group_visible_##name(         \
 		struct kobject *kobj, struct attribute *a, int n) \
 	{                                                         \
@@ -197,12 +197,12 @@ struct attribute_group {
 	}
 
 /*
- * Same as DEFINE_SYSFS_GROUP_VISIBLE, but for groups with only binary
+ * Same as DEFINE_SYSFS_GROUP_COMBO_VISIBILITY, but for groups with only binary
  * attributes. If an attribute_group defines both text and binary
  * attributes, the group visibility is determined by the function
  * specified to is_visible() not is_bin_visible()
  */
-#define DEFINE_SYSFS_BIN_GROUP_VISIBLE(name)                                   \
+#define DEFINE_SYSFS_BIN_GROUP_COMBO_VISIBILITY(name)                                \
 	static inline umode_t sysfs_group_visible_##name(                      \
 		struct kobject *kobj, const struct bin_attribute *attr, int n) \
 	{                                                                      \
@@ -211,7 +211,7 @@ struct attribute_group {
 		return name##_attr_visible(kobj, attr, n);                     \
 	}
 
-#define DEFINE_SIMPLE_SYSFS_BIN_GROUP_VISIBLE(name)                         \
+#define DEFINE_SYSFS_BIN_GROUP_VISIBILITY(name)                       \
 	static inline umode_t sysfs_group_visible_##name(                   \
 		struct kobject *kobj, const struct bin_attribute *a, int n) \
 	{                                                                   \
@@ -220,6 +220,12 @@ struct attribute_group {
 		return a->mode;                                             \
 	}
 
+/* Transitional aliases: so legacy code using old names continues to work */
+#define DEFINE_SYSFS_GROUP_VISIBLE(name) DEFINE_SYSFS_GROUP_COMBO_VISIBILITY(name)
+#define DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(name) DEFINE_SYSFS_GROUP_VISIBILITY(name)
+#define DEFINE_SYSFS_BIN_GROUP_VISIBLE(name) DEFINE_SYSFS_BIN_GROUP_COMBO_VISIBILITY(name)
+#define DEFINE_SIMPLE_SYSFS_BIN_GROUP_VISIBLE(name) DEFINE_SYSFS_BIN_GROUP_VISIBILITY(name)
+
 #define SYSFS_GROUP_VISIBLE(fn) sysfs_group_visible_##fn
 
 /*
-- 
2.43.0

Re: [PATCH 1/7] sysfs: Rename attribute group visibility macros

[Dan Williams]

David E. Box wrote:
> Update the naming of several macros to provide clearer semantics for
> controlling group and attribute visibility per Dan Williams' suggestion.
> Also, add transitional aliases mapping the old macro names to the new ones
> so that driver code remains functional before changes are again made in a
> future macro encapsulation patch. This approach ensures that when the
> encapsulation work is applied, drivers will only need to be updated once
> without breaking compatibility.
> 
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
>  include/linux/sysfs.h | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index 18f7e1fd093c..00dc88776f21 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -116,7 +116,7 @@ struct attribute_group {
>  #define SYSFS_GROUP_INVISIBLE	020000
>  
>  /*
> - * DEFINE_SYSFS_GROUP_VISIBLE(name):
> + * DEFINE_SYSFS_GROUP_COMBO_VISIBILITY(name):

This patch looks good, I just find myself wishing what "combo" means was
mentioned somewhere to clarify the distinction with the non-combo
flavor.

Something like:

@@ -123,6 +123,10 @@ struct attribute_group {
  *     This allows for static declaration of attribute_groups, and the
  *     simplification of attribute visibility lifetime that implies,
  *     without polluting sysfs with empty attribute directories.
+ *
+ *     "COMBO" implies that both the individual attribute
+ *     @name_attr_visible() and group @name_group_visible() helpers
+ *     must be defined.
  * Ex.
  *
  * static umode_t example_attr_visible(struct kobject *kobj,

>  /*
> - * DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(name):
> + * DEFINE_SYSFS_GROUP_VISIBILITY(name):
>   *	A helper macro to pair with SYSFS_GROUP_VISIBLE() that like
> - *	DEFINE_SYSFS_GROUP_VISIBLE() controls group visibility, but does
> + *	DEFINE_SYSFS_GROUP_COMBO_VISIBILITY() controls group visibility, but does
>   *	not require the implementation of a per-attribute visibility
>   *	callback.

@@ -166,10 +170,10 @@ struct attribute_group {
 
 /*
  * DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(name):
- *     A helper macro to pair with SYSFS_GROUP_VISIBLE() that like
- *     DEFINE_SYSFS_GROUP_VISIBLE() controls group visibility, but does
- *     not require the implementation of a per-attribute visibility
- *     callback.
+ *     A helper macro to pair with SYSFS_GROUP_VISIBLE(). Unlike
+ *     DEFINE_SYSFS_GROUP_COMBO_VISIBILITY() only a single
+ *     @name_group_visible() helper needs to be defined.
+ *
  * Ex.
  *
  * static bool example_group_visible(struct kobject *kobj)

...and then that hopefully makes it clearer what the requirements are
because truth to be told even I forgot what "combo" meant.

With those clarifications you can add:

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

[PATCH 2/7] sysfs: Introduce macros to simplify creation of visible attribute groups

[David E. Box]

Introduce macros to reduce boilerplate in attribute group definitions.
Combine DEFINE_SYSFS_ATTRIBUTE_GROUP_[COMBO]_VISIBILITY() with attribute
definitions in order to simplify group declarations involving visibility
logic.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/sysfs.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 00dc88776f21..0804bffd6013 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -303,6 +303,30 @@ static const struct attribute_group _name##_group = {		\
 };								\
 __ATTRIBUTE_GROUPS(_name)
 
+#define NAMED_ATTRIBUTE_GROUP_VISIBLE(_name)		\
+DEFINE_SYSFS_GROUP_VISIBILITY(_name);			\
+static const struct attribute_group _name##_group = {	\
+	.name = __stringify(_name),			\
+	.attrs = _name##_attrs,				\
+	.is_visible = SYSFS_GROUP_VISIBLE(_name),	\
+}
+
+#define NAMED_ATTRIBUTE_GROUPS_VISIBLE(_name)	\
+NAMED_ATTRIBUTE_GROUP_VISIBLE(_name);		\
+__ATTRIBUTE_GROUPS(_name)
+
+#define NAMED_ATTRIBUTE_GROUP_COMBO_VISIBLE(_name)	\
+DEFINE_SYSFS_GROUP_COMBO_VISIBILITY(_name);		\
+static const struct attribute_group _name##_group = {	\
+	.name = __stringify(_name),			\
+	.attrs = _name##_attrs,				\
+	.is_visible = SYSFS_GROUP_VISIBLE(_name),	\
+}
+
+#define NAMED_ATTRIBUTE_GROUPS_COMBO_VISIBLE(_name)	\
+NAMED_ATTRIBUTE_GROUP_COMBO_VISIBLE(_name);		\
+__ATTRIBUTE_GROUPS(_name)
+
 struct file;
 struct vm_area_struct;
 struct address_space;
-- 
2.43.0

Re: [PATCH 2/7] sysfs: Introduce macros to simplify creation of visible attribute groups

[Dan Williams]

David E. Box wrote:
> Introduce macros to reduce boilerplate in attribute group definitions.
> Combine DEFINE_SYSFS_ATTRIBUTE_GROUP_[COMBO]_VISIBILITY() with attribute
> definitions in order to simplify group declarations involving visibility
> logic.
> 
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  include/linux/sysfs.h | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index 00dc88776f21..0804bffd6013 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -303,6 +303,30 @@ static const struct attribute_group _name##_group = {		\
>  };								\
>  __ATTRIBUTE_GROUPS(_name)
>  
> +#define NAMED_ATTRIBUTE_GROUP_VISIBLE(_name)		\
> +DEFINE_SYSFS_GROUP_VISIBILITY(_name);			\
> +static const struct attribute_group _name##_group = {	\
> +	.name = __stringify(_name),			\
> +	.attrs = _name##_attrs,				\
> +	.is_visible = SYSFS_GROUP_VISIBLE(_name),	\
> +}
> +
> +#define NAMED_ATTRIBUTE_GROUPS_VISIBLE(_name)	\
> +NAMED_ATTRIBUTE_GROUP_VISIBLE(_name);		\
> +__ATTRIBUTE_GROUPS(_name)
> +
> +#define NAMED_ATTRIBUTE_GROUP_COMBO_VISIBLE(_name)	\
> +DEFINE_SYSFS_GROUP_COMBO_VISIBILITY(_name);		\
> +static const struct attribute_group _name##_group = {	\
> +	.name = __stringify(_name),			\
> +	.attrs = _name##_attrs,				\
> +	.is_visible = SYSFS_GROUP_VISIBLE(_name),	\
> +}
> +
> +#define NAMED_ATTRIBUTE_GROUPS_COMBO_VISIBLE(_name)	\
> +NAMED_ATTRIBUTE_GROUP_COMBO_VISIBLE(_name);		\
> +__ATTRIBUTE_GROUPS(_name)
> +

Looks good to me, I like that this makes clear that it is setting {
.name } in the resulting group.

I would not mind a comment like "See DEFINE_SYSFS_GROUP_COMBO_VISIBILITY
for method definition requirements", but only add that as a follow-on
patch if someone else acks that idea.

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

[PATCH 3/7] docs: sysfs.rst: document additional attribute group macros

[David E. Box]

Add documentation to Documentation/filesystems/sysfs.rst for several sysfs
helper macros, including recently introduced and previously undocumented
helpers.

Document the following macros:

	__ATTR_IGNORE_LOCKDEP
	DEFINE_SYSFS_GROUP_VISIBILITY
	DEFINE_SYSFS_BIN_GROUP_VISIBILITY
	DEFINE_SYSFS_BIN_GROUP_COMBO_VISIBILITY
	ATTRIBUTE_GROUPS
	BIN_ATTRIBUTE_GROUPS
	NAMED_ATTRIBUTE_GROUP_VISIBLE
	NAMED_ATTRIBUTE_GROUPS_VISIBLE
	NAMED_ATTRIBUTE_GROUP_COMBO_VISIBLE
	NAMED_ATTRIBUTE_GROUPS_COMBO_VISIBLE

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
 Documentation/filesystems/sysfs.rst | 244 ++++++++++++++++++++++++++++
 1 file changed, 244 insertions(+)

diff --git a/Documentation/filesystems/sysfs.rst b/Documentation/filesystems/sysfs.rst
index c32993bc83c7..16bcc3e7c80c 100644
--- a/Documentation/filesystems/sysfs.rst
+++ b/Documentation/filesystems/sysfs.rst
@@ -147,6 +147,250 @@ __ATTR_RW(name):
 __ATTR_NULL:
 	         which sets the name to NULL and is used as end of list
                  indicator (see: kernel/workqueue.c)
+__ATTR_IGNORE_LOCKDEP(name, mode, show, store):
+                 like __ATTR() but disables lockdep checks; used in cases
+                 where lockdep may emit false positives
+
+Additional Attribute Helpers
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+ATTRIBUTE_GROUPS(name):
+       Convenience macro to create an array of attribute group pointers.
+
+Example::
+
+    static struct attribute *foo_attrs[] = {
+            &attr1.attr,
+            &attr2.attr,
+            NULL
+    };
+    ATTRIBUTE_GROUPS(foo);
+
+BIN_ATTRIBUTE_GROUPS(name):
+        Same as ATTRIBUTE_GROUPS(), but for bin_attribute_group structures.
+
+Example::
+
+    static struct bin_attribute *foo_attrs[] = {
+            &bin_attr1.attr,
+            &bin_attr2.attr,
+            NULL
+    };
+    BIN_ATTRIBUTE_GROUPS(bin_foo);
+
+DEFINE_SYSFS_GROUP_COMBO_VISIBILITY(name):
+        A helper macro to pair with the assignment of
+
+                ".is_visible = SYSFS_GROUP_VISIBLE(name)",
+
+        that arranges for the directory associated with a named attribute_group
+        to optionally be hidden.  This allows for static declaration of
+        attribute_groups, and the simplification of attribute visibility
+        lifetime that implies, without polluting sysfs with empty attribute
+        directories.
+
+Example::
+
+    static umode_t example_attr_visible(struct kobject *kobj,
+                                        struct attribute *attr, int n)
+    {
+            if (example_attr_condition)
+                    return 0;
+            if (ro_attr_condition)
+                    return 0444;
+            return a->mode;
+    }
+
+    static bool example_group_visible(struct kobject *kobj)
+    {
+            if (example_group_condition)
+                    return false;
+            return true;
+    }
+
+    DEFINE_SYSFS_GROUP_COMBO_VISIBILITY(example);
+
+    static struct attribute_group example_group = {
+            .name = "example",
+            .is_visible = SYSFS_GROUP_VISIBLE(example),
+            .attrs = &example_attrs,
+    };
+
+Note that it expects <name>_attr_visible and <name>_group_visible to
+be defined. For cases where individual attributes do not need
+separate visibility consideration, only entire group visibility at
+once, see DEFINE_SYSFS_GROUP_VISIBILITY().
+
+DEFINE_SYSFS_GROUP_VISIBILITY(name):
+        A helper macro to pair with SYSFS_GROUP_VISIBLE() that, like
+        DEFINE_SYSFS_GROUP_COMBO_VISIBILITY(), controls group visibility, but
+        does not require the implementation of a per-attribute visibility
+        callback.
+
+Example::
+
+    static bool example_group_visible(struct kobject *kobj)
+    {
+            if (example_group_condition)
+                    return false;
+            return true;
+    }
+
+    DEFINE_SYSFS_GROUP_VISIBILITY(example);
+
+    static struct attribute_group example_group = {
+            .name = "example",
+            .is_visible = SYSFS_GROUP_VISIBLE(example),
+            .attrs = &example_attrs,
+    };
+
+DEFINE_SYSFS_BIN_GROUP_COMBO_VISIBILITY(name):
+DEFINE_SYSFS_BIN_GROUP_VISIBILITY(name):
+        Same as DEFINE_SYSFS_GROUP_VISIBILITY(), but for groups with only binary
+        attributes. If an attribute_group defines both text and binary
+        attributes, the group visibility is determined by the function
+        specified to is_visible() not is_bin_visible().
+
+Named Attribute Group Macros (with visibility)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+These macros define struct attribute_group objects with a static name and
+visibility function(s). They are useful for creating named directories in sysfs
+where individual attributes can be conditionally exposed.
+
+NAMED_ATTRIBUTE_GROUP_VISIBLE(name):
+        Defines an attribute group with a fixed directory name (matching name)
+        with a group visibility function. Expects an attribute array
+        <name>_attrs. The macro automatically defines the visibility function.
+
+Example::
+
+    static ssize_t foo_show(struct device *dev, struct device_attribute *attr,
+                            char *buf)
+    {
+            ...
+    }
+
+    static ssize_t foo_store(struct device *dev, struct device_attribute *attr,
+                             const char *buf, size_t count)
+    {
+            ...
+    }
+    static DEVICE_ATTR_RW(foo);
+
+    static bool bar_group_visible(struct kobject *kobj)
+    {
+            if (bar_group_condition)
+                    return false;
+            return true;
+    }
+
+    static struct attribute *bar_attrs[] = {
+            &dev_attr_foo.attr,
+            NULL
+    };
+    NAMED_ATTRIBUTE_GROUP_VISIBLE(bar);
+
+Creates::
+
+    static const struct attribute_group bar_group = {
+            .name = "bar",
+            .attrs = bar_attrs,
+            .is_visible = SYSFS_GROUP_VISIBLE(bar),
+    };
+
+    /*
+     * Where SYSFS_GROUP_VISIBLE(bar) is a function created by
+     * DEFINE_SYSFS_GROUP_VISIBILITY(bar) that calls bar_group_visible().
+     */
+
+NAMED_ATTRIBUTE_GROUPS_VISIBLE(name):
+        Like NAMED_ATTRIBUTE_GROUP_VISIBLE(), defines the visible attribute
+        group but also creates the group list <name>_groups[].
+
+Example::
+
+    ...
+
+    static struct attribute *bar_attrs[] = {
+            &attr1.attr,
+            &attr2.attr,
+            NULL
+    };
+    NAMED_ATTRIBUTE_GROUPS_VISIBLE(bar);
+
+Creates::
+
+    static const struct attribute_group bar_group = {
+            .name = "bar",
+            .attrs = bar_attrs,
+            .is_visible = SYSFS_GROUP_VISIBLE(bar),
+    };
+
+    static const struct attribute_group *bar_groups[] = {
+            &bar_group,
+            NULL
+    };
+
+NAMED_ATTRIBUTE_GROUP_COMBO_VISIBLE(name):
+        Same as NAMED_ATTRIBUTE_GROUP_VISIBLE(), but uses the "combo" visibility
+        variant to support both group and per-attribute visibility control.
+        Automatically generates the combo visibility boilerplate.
+
+Example::
+
+    static ssize_t foo_show(struct device *dev, struct device_attribute *attr,
+                            char *buf)
+    {
+            ...
+    }
+
+    static ssize_t foo_store(struct device *dev, struct device_attribute *attr,
+                             const char *buf, size_t count)
+    {
+            ...
+    }
+    static DEVICE_ATTR_RW(foo);
+
+    static umode_t foo_attr_visible(struct kobject *kobj,
+                                    struct attribute *attr, int n)
+    {
+            if (example_attr_condition)
+                    return 0;
+            return attr->mode;
+    }
+
+    static bool foo_group_visible(struct kobject *kobj)
+    {
+            if (foo_group_condition)
+                    return false;
+            return true;
+    }
+
+    static struct attribute *foo_attrs[] = {
+            &dev_attr_foo.attr,
+            NULL
+    };
+    NAMED_ATTRIBUTE_GROUP_COMBO_VISIBLE(foo);
+
+Creates::
+
+    static const struct attribute_group foo_group = {
+            .name = "foo",
+            .attrs = foo_attrs,
+            .is_visible = SYSFS_GROUP_VISIBLE(foo),
+    };
+
+    /*
+     * Where SYSFS_GROUP_VISIBLE(foo) is a function created by
+     * DEFINE_SYSFS_GROUP_COMBO_VISIBILITY(foo) that calls foo_group_visible()
+     * and foo_attr_visible().
+     */
+
+NAMED_ATTRIBUTE_GROUPS_COMBO_VISIBLE(name):
+        Like NAMED_ATTRIBUTE_GROUP_COMBO_VISIBLE() defines the attribute group,
+        supporting both group and per-attribute visibility control, but also
+        creates the group list <name>_groups[].
 
 Subsystem-Specific Callbacks
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-- 
2.43.0

Re: [PATCH 3/7] docs: sysfs.rst: document additional attribute group macros

[Dan Williams]

David E. Box wrote:
> Add documentation to Documentation/filesystems/sysfs.rst for several sysfs
> helper macros, including recently introduced and previously undocumented
> helpers.
> 
> Document the following macros:
> 
> 	__ATTR_IGNORE_LOCKDEP
> 	DEFINE_SYSFS_GROUP_VISIBILITY
> 	DEFINE_SYSFS_BIN_GROUP_VISIBILITY
> 	DEFINE_SYSFS_BIN_GROUP_COMBO_VISIBILITY
> 	ATTRIBUTE_GROUPS
> 	BIN_ATTRIBUTE_GROUPS
> 	NAMED_ATTRIBUTE_GROUP_VISIBLE
> 	NAMED_ATTRIBUTE_GROUPS_VISIBLE
> 	NAMED_ATTRIBUTE_GROUP_COMBO_VISIBLE
> 	NAMED_ATTRIBUTE_GROUPS_COMBO_VISIBLE
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
>  Documentation/filesystems/sysfs.rst | 244 ++++++++++++++++++++++++++++
>  1 file changed, 244 insertions(+)

Looks clean and useful, thanks for taking the time to improve the
documentation.

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

[PATCH 4/7] pci: doe: Replace sysfs visibility macro

[David E. Box]

Replace deprecated DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE() call with the new
DEFINE_SYSFS_GROUP_VISIBILITY() helper for the pci_doe_features_sysfs group
in drivers/pci/doe.c.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
 drivers/pci/doe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
index aae9a8a00406..18b355506dc1 100644
--- a/drivers/pci/doe.c
+++ b/drivers/pci/doe.c
@@ -119,7 +119,7 @@ static bool pci_doe_features_sysfs_group_visible(struct kobject *kobj)
 
 	return !xa_empty(&pdev->doe_mbs);
 }
-DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(pci_doe_features_sysfs)
+DEFINE_SYSFS_GROUP_VISIBILITY(pci_doe_features_sysfs)
 
 const struct attribute_group pci_doe_sysfs_group = {
 	.name	    = "doe_features",
-- 
2.43.0

Re: [PATCH 4/7] pci: doe: Replace sysfs visibility macro

[Dan Williams]

David E. Box wrote:
> Replace deprecated DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE() call with the new
> DEFINE_SYSFS_GROUP_VISIBILITY() helper for the pci_doe_features_sysfs group
> in drivers/pci/doe.c.
> 
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
>  drivers/pci/doe.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> index aae9a8a00406..18b355506dc1 100644
> --- a/drivers/pci/doe.c
> +++ b/drivers/pci/doe.c
> @@ -119,7 +119,7 @@ static bool pci_doe_features_sysfs_group_visible(struct kobject *kobj)
>  
>  	return !xa_empty(&pdev->doe_mbs);
>  }
> -DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(pci_doe_features_sysfs)
> +DEFINE_SYSFS_GROUP_VISIBILITY(pci_doe_features_sysfs)

Looks correct to me.

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

Re: [PATCH 4/7] pci: doe: Replace sysfs visibility macro

[Ilpo Järvinen]

On Wed, 23 Apr 2025, David E. Box wrote:

> Replace deprecated DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE() call with the new
> DEFINE_SYSFS_GROUP_VISIBILITY() helper for the pci_doe_features_sysfs group
> in drivers/pci/doe.c.
> 
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
>  drivers/pci/doe.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> index aae9a8a00406..18b355506dc1 100644
> --- a/drivers/pci/doe.c
> +++ b/drivers/pci/doe.c
> @@ -119,7 +119,7 @@ static bool pci_doe_features_sysfs_group_visible(struct kobject *kobj)
>  
>  	return !xa_empty(&pdev->doe_mbs);
>  }
> -DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(pci_doe_features_sysfs)
> +DEFINE_SYSFS_GROUP_VISIBILITY(pci_doe_features_sysfs)

Hi David,

Is it intentional to not have semicolon at the end?

>  const struct attribute_group pci_doe_sysfs_group = {
>  	.name	    = "doe_features",
> 

-- 
 i.

Re: [PATCH 4/7] pci: doe: Replace sysfs visibility macro

[David E. Box]

On Fri, 2025-04-25 at 13:57 +0300, Ilpo Järvinen wrote:
> On Wed, 23 Apr 2025, David E. Box wrote:
> 
> > Replace deprecated DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE() call with the new
> > DEFINE_SYSFS_GROUP_VISIBILITY() helper for the pci_doe_features_sysfs group
> > in drivers/pci/doe.c.
> > 
> > Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> > ---
> >  drivers/pci/doe.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> > index aae9a8a00406..18b355506dc1 100644
> > --- a/drivers/pci/doe.c
> > +++ b/drivers/pci/doe.c
> > @@ -119,7 +119,7 @@ static bool pci_doe_features_sysfs_group_visible(struct
> > kobject *kobj)
> >  
> >  	return !xa_empty(&pdev->doe_mbs);
> >  }
> > -DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(pci_doe_features_sysfs)
> > +DEFINE_SYSFS_GROUP_VISIBILITY(pci_doe_features_sysfs)
> 
> Hi David,
> 
> Is it intentional to not have semicolon at the end?

Hi Ilpo,

I was just doing a straight name swap and didn't not notice the lack of a
semicolon. Of course, since DEFINE_SYSFS_GROUP_VISIBILITY() expands to a
function definition, a trailing semicolon isn't necessary.

I suspect the issue is with the other instances where it was added, which makes
the usage inconsistent. What would you suggest?

David

Re: [PATCH 4/7] pci: doe: Replace sysfs visibility macro

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1851 bytes --]

On Fri, 25 Apr 2025, David E. Box wrote:

> On Fri, 2025-04-25 at 13:57 +0300, Ilpo Järvinen wrote:
> > On Wed, 23 Apr 2025, David E. Box wrote:
> > 
> > > Replace deprecated DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE() call with the new
> > > DEFINE_SYSFS_GROUP_VISIBILITY() helper for the pci_doe_features_sysfs group
> > > in drivers/pci/doe.c.
> > > 
> > > Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> > > ---
> > >  drivers/pci/doe.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> > > index aae9a8a00406..18b355506dc1 100644
> > > --- a/drivers/pci/doe.c
> > > +++ b/drivers/pci/doe.c
> > > @@ -119,7 +119,7 @@ static bool pci_doe_features_sysfs_group_visible(struct
> > > kobject *kobj)
> > >  
> > >  	return !xa_empty(&pdev->doe_mbs);
> > >  }
> > > -DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(pci_doe_features_sysfs)
> > > +DEFINE_SYSFS_GROUP_VISIBILITY(pci_doe_features_sysfs)
> > 
> > Hi David,
> > 
> > Is it intentional to not have semicolon at the end?
> 
> Hi Ilpo,
> 
> I was just doing a straight name swap and didn't not notice the lack of a
> semicolon. Of course, since DEFINE_SYSFS_GROUP_VISIBILITY() expands to a
> function definition, a trailing semicolon isn't necessary.
> 
> I suspect the issue is with the other instances where it was added, which makes
> the usage inconsistent. What would you suggest?

Hi,

When I saw that lack of semicolon, my first assumption was there's 
something special here that _requires_ leaving the semicolon out, which 
turned out untrue after an unnecessary roundtrip to read the macro. So IMO 
it would be better to have the semicolon there to tell the reader there's 
nothing of special interest here.

Also, you used semicolon in the example. :-)

-- 
 i.

[PATCH 5/7] soundwire: sysfs: Use ATTRIBUTE_GROUP_VISIBLE()

[David E. Box]

Replace the manual definition of the dp0 attribute group with the newly
introduced ATTRIBUTE_GROUP_VISIBLE() macro, simplifying the code and
improving maintainability.

Consolidate the definition of dp0_attrs and move the attribute array above
the macro so that they are visibly tied together.  While here, also remove
the unneeded trailing comma after NULL at the end of all attribute arrays.
No functional changes are intended.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/soundwire/sysfs_slave.c | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/drivers/soundwire/sysfs_slave.c b/drivers/soundwire/sysfs_slave.c
index c5c22d1708ec..400f2a17f140 100644
--- a/drivers/soundwire/sysfs_slave.c
+++ b/drivers/soundwire/sysfs_slave.c
@@ -103,7 +103,7 @@ static DEVICE_ATTR_RO(modalias);
 
 static struct attribute *slave_attrs[] = {
 	&dev_attr_modalias.attr,
-	NULL,
+	NULL
 };
 
 static const struct attribute_group slave_attr_group = {
@@ -126,7 +126,7 @@ static struct attribute *slave_dev_attrs[] = {
 	&dev_attr_master_count.attr,
 	&dev_attr_source_ports.attr,
 	&dev_attr_sink_ports.attr,
-	NULL,
+	NULL
 };
 
 static const struct attribute_group sdw_slave_dev_attr_group = {
@@ -170,16 +170,6 @@ static ssize_t words_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(words);
 
-static struct attribute *dp0_attrs[] = {
-	&dev_attr_max_word.attr,
-	&dev_attr_min_word.attr,
-	&dev_attr_words.attr,
-	&dev_attr_BRA_flow_controlled.attr,
-	&dev_attr_simple_ch_prep_sm.attr,
-	&dev_attr_imp_def_interrupts.attr,
-	NULL,
-};
-
 static umode_t dp0_attr_visible(struct kobject *kobj, struct attribute *attr,
 			      int n)
 {
@@ -198,19 +188,23 @@ static bool dp0_group_visible(struct kobject *kobj)
 		return true;
 	return false;
 }
-DEFINE_SYSFS_GROUP_VISIBLE(dp0);
 
-static const struct attribute_group dp0_group = {
-	.attrs = dp0_attrs,
-	.is_visible = SYSFS_GROUP_VISIBLE(dp0),
-	.name = "dp0",
+static struct attribute *dp0_attrs[] = {
+	&dev_attr_max_word.attr,
+	&dev_attr_min_word.attr,
+	&dev_attr_words.attr,
+	&dev_attr_BRA_flow_controlled.attr,
+	&dev_attr_simple_ch_prep_sm.attr,
+	&dev_attr_imp_def_interrupts.attr,
+	NULL
 };
+NAMED_ATTRIBUTE_GROUP_COMBO_VISIBLE(dp0);
 
 const struct attribute_group *sdw_attr_groups[] = {
 	&slave_attr_group,
 	&sdw_slave_dev_attr_group,
 	&dp0_group,
-	NULL,
+	NULL
 };
 
 /*
@@ -249,7 +243,7 @@ static DEVICE_ATTR_RO(device_number);
 static struct attribute *slave_status_attrs[] = {
 	&dev_attr_status.attr,
 	&dev_attr_device_number.attr,
-	NULL,
+	NULL
 };
 
 /*
-- 
2.43.0

Re: [PATCH 5/7] soundwire: sysfs: Use ATTRIBUTE_GROUP_VISIBLE()

[Dan Williams]

David E. Box wrote:
> Replace the manual definition of the dp0 attribute group with the newly
> introduced ATTRIBUTE_GROUP_VISIBLE() macro, simplifying the code and
> improving maintainability.
> 
> Consolidate the definition of dp0_attrs and move the attribute array above
> the macro so that they are visibly tied together.  While here, also remove
> the unneeded trailing comma after NULL at the end of all attribute arrays.

I am generally not a fan of "while we're at it" clauses, but this looks
ok.

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

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

[David E. Box]

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

[Kurt Borja]

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,

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

[Dan Williams]

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>

[PATCH 7/7] sysfs: Remove transitional attribute group alias macros

[David E. Box]

Remove transitional macros used to alias named and visible sysfs attribute
group definitions. These were temporarily introduced to ease migration but
are now redundant due to the adoption of the new encapsulated
NAMED_ATTRIBUTE_* macros across all relevant drivers.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
 include/linux/sysfs.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 0804bffd6013..877fd1976668 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -220,12 +220,6 @@ struct attribute_group {
 		return a->mode;                                             \
 	}
 
-/* Transitional aliases: so legacy code using old names continues to work */
-#define DEFINE_SYSFS_GROUP_VISIBLE(name) DEFINE_SYSFS_GROUP_COMBO_VISIBILITY(name)
-#define DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(name) DEFINE_SYSFS_GROUP_VISIBILITY(name)
-#define DEFINE_SYSFS_BIN_GROUP_VISIBLE(name) DEFINE_SYSFS_BIN_GROUP_COMBO_VISIBILITY(name)
-#define DEFINE_SIMPLE_SYSFS_BIN_GROUP_VISIBLE(name) DEFINE_SYSFS_BIN_GROUP_VISIBILITY(name)
-
 #define SYSFS_GROUP_VISIBLE(fn) sysfs_group_visible_##fn
 
 /*
-- 
2.43.0

Re: [PATCH 7/7] sysfs: Remove transitional attribute group alias macros

[Dan Williams]

David E. Box wrote:
> Remove transitional macros used to alias named and visible sysfs attribute
> group definitions. These were temporarily introduced to ease migration but
> are now redundant due to the adoption of the new encapsulated
> NAMED_ATTRIBUTE_* macros across all relevant drivers.
> 
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>

Appreciate this split just in case there is some lag in the acceptance
of the individual conversion patches.

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