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

Greg Kroah-Hartman
2021-03-28T06:46:22Z

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@linux.intel.com>
> ---
> 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