Re: [PATCH v1 3/8] software node: Show properties and their values in sysfs

[Greg Kroah-Hartman]

On Sun, Mar 28, 2021 at 12:20:07AM +0200, Andy Shevchenko wrote:
> It's very convenient to see what properties and their values
> are currently being assigned in the registered software nodes.
> 
> Show properties and their values in sysfs.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
>  drivers/base/swnode.c | 137 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 132 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 19aa44bc2628..d7fe1a887d2d 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -10,6 +10,7 @@
>  #include <linux/kernel.h>
>  #include <linux/property.h>
>  #include <linux/slab.h>
> +#include <linux/sysfs.h>
>  
>  struct swnode {
>  	int id;
> @@ -17,6 +18,10 @@ struct swnode {
>  	struct fwnode_handle fwnode;
>  	const struct software_node *node;
>  
> +	/* properties in sysfs */
> +	struct kobj_attribute *property_attrs;
> +	struct attribute_group property_group;
> +
>  	/* hierarchy */
>  	struct ida child_ids;
>  	struct list_head entry;
> @@ -25,6 +30,7 @@ struct swnode {
>  
>  	unsigned int allocated:1;
>  	unsigned int managed:1;
> +	unsigned int properties:1;
>  };
>  
>  static DEFINE_IDA(swnode_root_ids);
> @@ -299,6 +305,18 @@ static int property_entry_copy_data(struct property_entry *dst,
>  	return 0;
>  }
>  
> +static int property_entries_count(const struct property_entry *properties)
> +{
> +	int n = 0;
> +
> +	if (properties) {
> +		while (properties[n].name)
> +			n++;
> +	}
> +
> +	return n;
> +}
> +
>  /**
>   * property_entries_dup - duplicate array of properties
>   * @properties: array of properties to copy
> @@ -310,15 +328,13 @@ struct property_entry *
>  property_entries_dup(const struct property_entry *properties)
>  {
>  	struct property_entry *p;
> -	int i, n = 0;
> +	int i, n;
>  	int ret;
>  
> -	if (!properties)
> +	n = property_entries_count(properties);
> +	if (n == 0)
>  		return NULL;
>  
> -	while (properties[n].name)
> -		n++;
> -
>  	p = kcalloc(n + 1, sizeof(*p), GFP_KERNEL);
>  	if (!p)
>  		return ERR_PTR(-ENOMEM);
> @@ -746,6 +762,108 @@ static void software_node_free(const struct software_node *node)
>  	kfree(node);
>  }
>  
> +static ssize_t
> +swnode_property_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> +{
> +	struct swnode *swnode = kobj_to_swnode(kobj);
> +	const struct property_entry *prop;
> +	const void *pointer;
> +	ssize_t len = 0;
> +	int i;
> +
> +	prop = property_entry_get(swnode->node->properties, attr->attr.name);
> +	if (!prop)
> +		return -EINVAL;
> +
> +	/* We can't fail here, because it means boolean property */
> +	pointer = property_get_pointer(prop);
> +	if (!pointer)
> +		return sysfs_emit(buf, "\n");
> +
> +	switch (prop->type) {
> +	case DEV_PROP_U8:
> +		for (i = 0; i < prop->length / sizeof(u8); i++)
> +			len += sysfs_emit_at(buf, len, "%u,", ((u8 *)pointer)[i]);

No, sysfs is "one value per file", and that is not what you are showing
here at all :(

Also, there is no Documentation/ABI/ entries for your new sysfs files,
so that means we couldn't take this patcheset anyway :(

greg k-h