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

[Dan Williams <dan.j.williams@intel.com>]

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>