| Message ID | 20210327222012.54103-3-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/8] software node: Free resources explicitly when swnode_register() fails | expand |
Hi Andy,
I love your patch! Yet something to improve:
[auto build test ERROR on driver-core/driver-core-testing]
[also build test ERROR on linuxtv-media/master linux/master linus/master v5.12-rc4 next-20210326]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Andy-Shevchenko/software-node-Free-resources-explicitly-when-swnode_register-fails/20210328-062322
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git ecdc996baf291b903342cc704f4086a88c361967
config: microblaze-randconfig-r016-20210328 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/1d48129700f39fc70d26e5faee27e6fd7d8d5234
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andy-Shevchenko/software-node-Free-resources-explicitly-when-swnode_register-fails/20210328-062322
git checkout 1d48129700f39fc70d26e5faee27e6fd7d8d5234
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=microblaze
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from include/linux/kobject.h:20,
from include/linux/energy_model.h:7,
from include/linux/device.h:16,
from drivers/base/swnode.c:9:
drivers/base/swnode.c: In function 'swnode_register_properties':
>> include/linux/sysfs.h:55:8: error: 'struct kobj_attribute' has no member named 'key'
55 | (attr)->key = &__key; \
| ^~
drivers/base/swnode.c:835:3: note: in expansion of macro 'sysfs_attr_init'
835 | sysfs_attr_init(&attrs[n]);
| ^~~~~~~~~~~~~~~
vim +55 include/linux/sysfs.h
^1da177e4c3f41 Linus Torvalds 2005-04-16 39
35960258ed388c Eric W. Biederman 2010-02-12 40 /**
35960258ed388c Eric W. Biederman 2010-02-12 41 * sysfs_attr_init - initialize a dynamically allocated sysfs attribute
35960258ed388c Eric W. Biederman 2010-02-12 42 * @attr: struct attribute to initialize
35960258ed388c Eric W. Biederman 2010-02-12 43 *
35960258ed388c Eric W. Biederman 2010-02-12 44 * Initialize a dynamically allocated struct attribute so we can
35960258ed388c Eric W. Biederman 2010-02-12 45 * make lockdep happy. This is a new requirement for attributes
35960258ed388c Eric W. Biederman 2010-02-12 46 * and initially this is only needed when lockdep is enabled.
35960258ed388c Eric W. Biederman 2010-02-12 47 * Lockdep gives a nice error when your attribute is added to
35960258ed388c Eric W. Biederman 2010-02-12 48 * sysfs if you don't have this.
35960258ed388c Eric W. Biederman 2010-02-12 49 */
6992f5334995af Eric W. Biederman 2010-02-11 50 #ifdef CONFIG_DEBUG_LOCK_ALLOC
6992f5334995af Eric W. Biederman 2010-02-11 51 #define sysfs_attr_init(attr) \
6992f5334995af Eric W. Biederman 2010-02-11 52 do { \
6992f5334995af Eric W. Biederman 2010-02-11 53 static struct lock_class_key __key; \
6992f5334995af Eric W. Biederman 2010-02-11 54 \
6992f5334995af Eric W. Biederman 2010-02-11 @55 (attr)->key = &__key; \
6992f5334995af Eric W. Biederman 2010-02-11 56 } while (0)
6992f5334995af Eric W. Biederman 2010-02-11 57 #else
6992f5334995af Eric W. Biederman 2010-02-11 58 #define sysfs_attr_init(attr) do {} while (0)
6992f5334995af Eric W. Biederman 2010-02-11 59 #endif
6992f5334995af Eric W. Biederman 2010-02-11 60
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
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
On Sun, Mar 28, 2021 at 9:47 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > 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. ... > > + 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 :( It is following: it's a "one value" for property in question, As we may read in [1]: "...so it is socially acceptable to express an array of values of the same type." And here is exactly the case: *values of the same type*. > Also, there is no Documentation/ABI/ entries for your new sysfs files, > so that means we couldn't take this patcheset anyway :( True, I'll fix this, thanks! [1]: https://www.kernel.org/doc/html/latest/filesystems/sysfs.html -- With Best Regards, Andy Shevchenko
On Sun, Mar 28, 2021 at 03:56:26PM +0300, Andy Shevchenko wrote: > On Sun, Mar 28, 2021 at 9:47 AM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > 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. > > ... > > > > + 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 :( > > It is following: it's a "one value" for property in question, > > As we may read in [1]: "...so it is socially acceptable to express an > array of values of the same type." > > And here is exactly the case: *values of the same type*. So what is it going to look like exactly? And what tool is going to be there to parse this mess? Who is going to to use it? Arrays of data are not "one value". thanks, greg k-h
On Sun, Mar 28, 2021 at 03:02:24PM +0200, Greg Kroah-Hartman wrote: > On Sun, Mar 28, 2021 at 03:56:26PM +0300, Andy Shevchenko wrote: > > On Sun, Mar 28, 2021 at 9:47 AM Greg Kroah-Hartman > > <gregkh@linuxfoundation.org> wrote: > > > > > > 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. > > > > ... > > > > > > + 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 :( > > > > It is following: it's a "one value" for property in question, > > > > As we may read in [1]: "...so it is socially acceptable to express an > > array of values of the same type." > > > > And here is exactly the case: *values of the same type*. > > So what is it going to look like exactly? Basically we have two approaches (already done in the kernel!) use space or comma for a separator. So: - for boolean it will be an empty string (and it's one value always) - for integers it will be, for example, '0,1,2' (w/o single quotes) for property array with values 0, 1, and 2 - for plain integers or arrays out of 1 element it will be plain integer - for strings it will be, for example, '"str1","str2"' (w/o single quotes) for array of string { "str1", "str2" } - for single string or array out of 1 element, it will be '"str"' (w/o single quotes) This should be a part of documentation. > And what tool is going to be > there to parse this mess? Who is going to to use it? I guess something like hwinfo (needs a patch). The idea behind that this is following what ACPI and DT provides to the users via /sys/firmware/ (however, in binary format). I can re-do to provide a binary, and it will effectively make software nodes in align with the rest. -- With Best Regards, Andy Shevchenko
On Mon, Mar 29, 2021 at 04:01:18PM +0300, Andy Shevchenko wrote: > On Sun, Mar 28, 2021 at 03:02:24PM +0200, Greg Kroah-Hartman wrote: > > On Sun, Mar 28, 2021 at 03:56:26PM +0300, Andy Shevchenko wrote: > > > On Sun, Mar 28, 2021 at 9:47 AM Greg Kroah-Hartman > > > <gregkh@linuxfoundation.org> wrote: > > > > > > > > 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. > > > > > > ... > > > > > > > > + 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 :( > > > > > > It is following: it's a "one value" for property in question, > > > > > > As we may read in [1]: "...so it is socially acceptable to express an > > > array of values of the same type." > > > > > > And here is exactly the case: *values of the same type*. > > > > So what is it going to look like exactly? > > Basically we have two approaches (already done in the kernel!) use space or > comma for a separator. So: > - for boolean it will be an empty string (and it's one value always) > - for integers it will be, for example, '0,1,2' (w/o single quotes) > for property array with values 0, 1, and 2 > - for plain integers or arrays out of 1 element it will be plain integer > - for strings it will be, for example, '"str1","str2"' (w/o single quotes) > for array of string { "str1", "str2" } > - for single string or array out of 1 element, it will be '"str"' (w/o single > quotes) > > This should be a part of documentation. And I will complain then too, these "lists of values" are not for sysfs, sorry. > > And what tool is going to be > > there to parse this mess? Who is going to to use it? > > I guess something like hwinfo (needs a patch). If nothing needs this, then why are you adding these? > The idea behind that this is following what ACPI and DT provides to the users > via /sys/firmware/ (however, in binary format). I can re-do to provide a > binary, and it will effectively make software nodes in align with the rest. binary files in sysfs are only to be used as a "pass through" from hardware to userspace. That does not seem relevant here. sorry, please keep this out of sysfs for now. greg k-h
On Mon, Mar 29, 2021 at 4:46 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Mon, Mar 29, 2021 at 04:01:18PM +0300, Andy Shevchenko wrote: > > On Sun, Mar 28, 2021 at 03:02:24PM +0200, Greg Kroah-Hartman wrote: > > > On Sun, Mar 28, 2021 at 03:56:26PM +0300, Andy Shevchenko wrote: > > > > On Sun, Mar 28, 2021 at 9:47 AM Greg Kroah-Hartman > > > > <gregkh@linuxfoundation.org> wrote: > > > > > 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. ... > > > > > > + 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 :( > > > > > > > > It is following: it's a "one value" for property in question, > > > > > > > > As we may read in [1]: "...so it is socially acceptable to express an > > > > array of values of the same type." > > > > > > > > And here is exactly the case: *values of the same type*. > > > > > > So what is it going to look like exactly? > > > > Basically we have two approaches (already done in the kernel!) use space or > > comma for a separator. So: > > - for boolean it will be an empty string (and it's one value always) > > - for integers it will be, for example, '0,1,2' (w/o single quotes) > > for property array with values 0, 1, and 2 > > - for plain integers or arrays out of 1 element it will be plain integer > > - for strings it will be, for example, '"str1","str2"' (w/o single quotes) > > for array of string { "str1", "str2" } > > - for single string or array out of 1 element, it will be '"str"' (w/o single > > quotes) > > > > This should be a part of documentation. > > And I will complain then too, these "lists of values" are not for sysfs, > sorry. How can you recommend showing an array(s) of values of the same type? > > > And what tool is going to be > > > there to parse this mess? Who is going to to use it? > > > > I guess something like hwinfo (needs a patch). > > If nothing needs this, then why are you adding these? OK, I will prepare tools first, but I have no idea for the format. So, how can I be sure that tools will accept the patch if there is no kernel interface available? Seems like a stale case. > > The idea behind that this is following what ACPI and DT provides to the users > > via /sys/firmware/ (however, in binary format). I can re-do to provide a > > binary, and it will effectively make software nodes in align with the rest. > > binary files in sysfs are only to be used as a "pass through" from > hardware to userspace. That does not seem relevant here. This makes sense. > sorry, please keep this out of sysfs for now. OK. For now I will resubmit the rest with addressed comments. Thanks! -- With Best Regards, Andy Shevchenko
On Mon, Mar 29, 2021 at 05:51:01PM +0300, Andy Shevchenko wrote: > On Mon, Mar 29, 2021 at 4:46 PM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > On Mon, Mar 29, 2021 at 04:01:18PM +0300, Andy Shevchenko wrote: > > > On Sun, Mar 28, 2021 at 03:02:24PM +0200, Greg Kroah-Hartman wrote: > > > > On Sun, Mar 28, 2021 at 03:56:26PM +0300, Andy Shevchenko wrote: > > > > > On Sun, Mar 28, 2021 at 9:47 AM Greg Kroah-Hartman > > > > > <gregkh@linuxfoundation.org> wrote: > > > > > > 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. > > ... > > > > > > > > + 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 :( > > > > > > > > > > It is following: it's a "one value" for property in question, > > > > > > > > > > As we may read in [1]: "...so it is socially acceptable to express an > > > > > array of values of the same type." > > > > > > > > > > And here is exactly the case: *values of the same type*. > > > > > > > > So what is it going to look like exactly? > > > > > > Basically we have two approaches (already done in the kernel!) use space or > > > comma for a separator. So: > > > - for boolean it will be an empty string (and it's one value always) > > > - for integers it will be, for example, '0,1,2' (w/o single quotes) > > > for property array with values 0, 1, and 2 > > > - for plain integers or arrays out of 1 element it will be plain integer > > > - for strings it will be, for example, '"str1","str2"' (w/o single quotes) > > > for array of string { "str1", "str2" } > > > - for single string or array out of 1 element, it will be '"str"' (w/o single > > > quotes) > > > > > > This should be a part of documentation. > > > > And I will complain then too, these "lists of values" are not for sysfs, > > sorry. > > How can you recommend showing an array(s) of values of the same type? I do not think that is something that should be shown in sysfs if at all possible. > > > > And what tool is going to be > > > > there to parse this mess? Who is going to to use it? > > > > > > I guess something like hwinfo (needs a patch). > > > > If nothing needs this, then why are you adding these? > > OK, I will prepare tools first, but I have no idea for the format. So, > how can I be sure that tools will accept the patch if there is no > kernel interface available? Seems like a stale case. Develop both together. Do not think that you will get the kernel format right without actually knowing what userspace needs/wants. That is crazy. thanks, greg k-h
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]); + break; + case DEV_PROP_U16: + for (i = 0; i < prop->length / sizeof(u16); i++) + len += sysfs_emit_at(buf, len, "%u,", ((u16 *)pointer)[i]); + break; + case DEV_PROP_U32: + for (i = 0; i < prop->length / sizeof(u32); i++) + len += sysfs_emit_at(buf, len, "%u,", ((u32 *)pointer)[i]); + break; + case DEV_PROP_U64: + for (i = 0; i < prop->length / sizeof(u64); i++) + len += sysfs_emit_at(buf, len, "%llu,", ((u64 *)pointer)[i]); + break; + case DEV_PROP_STRING: + for (i = 0; i < prop->length / sizeof(const char **); i++) + len += sysfs_emit_at(buf, len, "\"%s\",", ((const char **)pointer)[i]); + break; + default: + return -EINVAL; + } + + sysfs_emit_at(buf, len ? len - 1 : 0, "\n"); + return len; +} + +static int swnode_register_properties(struct swnode *swnode) +{ + const struct property_entry *props = swnode->node->properties; + struct attribute **group_attrs; + struct kobj_attribute *attrs; + int n; + int ret; + + n = property_entries_count(props); + if (n == 0) + return 0; + + group_attrs = kcalloc(n + 1, sizeof(*group_attrs), GFP_KERNEL); + if (!group_attrs) + return -ENOMEM; + + attrs = kcalloc(n, sizeof(*attrs), GFP_KERNEL); + if (!attrs) { + kfree(group_attrs); + return -ENOMEM; + } + + while (n--) { + sysfs_attr_init(&attrs[n]); + attrs[n].attr.mode = 0444; + attrs[n].attr.name = props[n].name; + attrs[n].show = swnode_property_show; + group_attrs[n] = &attrs[n].attr; + } + + swnode->property_group.name = "properties"; + swnode->property_group.attrs = group_attrs; + + ret = sysfs_create_group(&swnode->kobj, &swnode->property_group); + if (ret) { + kfree(attrs); + kfree(group_attrs); + return ret; + } + + swnode->property_attrs = attrs; + swnode->properties = true; + return 0; +} + +static void swnode_unregister_properties(struct swnode *swnode) +{ + /* + * Nodes under sysfs are removed by __kobject_del(), + * we don't need to call sysfs_remove_group() explicitly. + */ + kfree(swnode->property_group.attrs); + kfree(swnode->property_attrs); +} + static void software_node_release(struct kobject *kobj) { struct swnode *swnode = kobj_to_swnode(kobj); @@ -757,6 +875,9 @@ static void software_node_release(struct kobject *kobj) ida_simple_remove(&swnode_root_ids, swnode->id); } + if (swnode->properties) + swnode_unregister_properties(swnode); + if (swnode->allocated) software_node_free(swnode->node); @@ -810,6 +931,12 @@ swnode_register(const struct software_node *node, struct swnode *parent, return ERR_PTR(ret); } + ret = swnode_register_properties(swnode); + if (ret) { + kobject_put(&swnode->kobj); + return ERR_PTR(ret); + } + /* * Assign the flag only in the successful case, so * the above kobject_put() won't mess up with properties.
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(-)