Hi Heikki On 11/01/2021 14:10, Heikki Krogerus wrote: > This helper will register a software node and then assign > it to device at the same time. The function will also make > sure that the device can't have more than one software node. > > Tested-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Signed-off-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> > --- I like this change. One comment below, but for what it's worth: Reviewed-by: Daniel Scally <djrscally@xxxxxxxxx> > +/** > + * device_remove_software_node - Remove device's software node > + * @dev: The device with the software node. > + * > + * This function will unregister the software node of @dev. > + */ > +void device_remove_software_node(struct device *dev) > +{ > + struct swnode *swnode; > + > + swnode = dev_to_swnode(dev); > + if (!swnode) > + return; > + > + kobject_put(&swnode->kobj); > +} > +EXPORT_SYMBOL_GPL(device_remove_software_node); I wonder if this also ought to set dev_fwnode(dev)->secondary back to ERR_PTR(-ENODEV)? > + > int software_node_notify(struct device *dev, unsigned long action) > { > - struct fwnode_handle *fwnode = dev_fwnode(dev); > struct swnode *swnode; > int ret; > > - if (!fwnode) > - return 0; > - > - if (!is_software_node(fwnode)) > - fwnode = fwnode->secondary; > - if (!is_software_node(fwnode)) > + swnode = dev_to_swnode(dev); > + if (!swnode) > return 0; > > - swnode = to_swnode(fwnode); > - > switch (action) { > case KOBJ_ADD: > ret = sysfs_create_link(&dev->kobj, &swnode->kobj, > diff --git a/include/linux/property.h b/include/linux/property.h > index 0a9001fe7aeab..b0e413dc59271 100644 > --- a/include/linux/property.h > +++ b/include/linux/property.h > @@ -488,4 +488,7 @@ fwnode_create_software_node(const struct property_entry *properties, > const struct fwnode_handle *parent); > void fwnode_remove_software_node(struct fwnode_handle *fwnode); > > +int device_add_software_node(struct device *dev, const struct software_node *swnode); > +void device_remove_software_node(struct device *dev); > + > #endif /* _LINUX_PROPERTY_H_ */ >