Hi, On Mon, Sep 17, 2018 at 11:16:00AM -0700, Dmitry Torokhov wrote: > Several drivers rely on having notion of sub-nodes when describing > hardware, let's allow static board-defined properties also have it. > > The board files will then attach properties to devices in the following > fashion: > > device_add_properties(&board_platform_device.dev, > main_device_props); > device_add_child_properties(&board_platform_device.dev, > dev_fwnode(&board_platform_device.dev), > child1_device_props); > device_add_child_properties(&board_platform_device.dev, > dev_fwnode(&board_platform_device.dev), > child2_device_props); > ... > platform_device_register(&board_platform_device.dev); > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> > --- > drivers/base/pset_property.c | 110 +++++++++++++++++++++++++++++++---- > include/linux/property.h | 4 ++ > 2 files changed, 102 insertions(+), 12 deletions(-) > > diff --git a/drivers/base/pset_property.c b/drivers/base/pset_property.c > index 08ecc13080ae..63f2377aefe8 100644 > --- a/drivers/base/pset_property.c > +++ b/drivers/base/pset_property.c > @@ -18,6 +18,11 @@ struct property_set { > struct device *dev; > struct fwnode_handle fwnode; > const struct property_entry *properties; > + > + struct property_set *parent; > + /* Entry in parent->children list */ > + struct list_head child_node; > + struct list_head children; Add const char *name; and you can implement also pset_get_named_child_node(). > }; > > static const struct fwnode_operations pset_fwnode_ops; > @@ -283,10 +288,47 @@ pset_fwnode_property_read_string_array(const struct fwnode_handle *fwnode, > val, nval); > } > > +struct fwnode_handle *pset_fwnode_get_parent(const struct fwnode_handle *fwnode) > +{ > + struct property_set *pset = to_pset_node(fwnode); > + > + return pset ? &pset->parent->fwnode : NULL; > +} > + > +struct fwnode_handle * > +pset_fwnode_get_next_subnode(const struct fwnode_handle *fwnode, > + struct fwnode_handle *child) > +{ > + const struct property_set *pset = to_pset_node(fwnode); > + struct property_set *first_child; > + struct property_set *next; > + > + if (!pset) > + return NULL; > + > + if (list_empty(&pset->children)) > + return NULL; > + > + first_child = list_first_entry(&pset->children, struct property_set, > + child_node); > + > + if (child) { > + next = list_next_entry(to_pset_node(child), child_node); > + if (next == first_child) > + return NULL; > + } else { > + next = first_child; > + } > + > + return &next->fwnode; > +} > + > static const struct fwnode_operations pset_fwnode_ops = { > .property_present = pset_fwnode_property_present, > .property_read_int_array = pset_fwnode_read_int_array, > .property_read_string_array = pset_fwnode_property_read_string_array, > + .get_parent = pset_fwnode_get_parent, > + .get_next_child_node = pset_fwnode_get_next_subnode, > }; > > static void property_entry_free_data(const struct property_entry *p) > @@ -439,24 +481,31 @@ EXPORT_SYMBOL_GPL(property_entries_free); > */ > static void pset_free_set(struct property_set *pset) > { > + struct property_set *child, *next; > + > if (!pset) > return; > > + list_for_each_entry_safe(child, next, &pset->children, child_node) { > + list_del(&child->child_node); > + pset_free_set(child); > + } > + > property_entries_free(pset->properties); > kfree(pset); > } > > /** > - * pset_copy_set - copies property set > - * @pset: Property set to copy > + * pset_create_set - creates property set. > + * @src: property entries for the set. > * > - * This function takes a deep copy of the given property set and returns > - * pointer to the copy. Call device_free_property_set() to free resources > - * allocated in this function. > + * This function takes a deep copy of the given property entries and creates > + * property set. Call pset_free_set() to free resources allocated in this > + * function. > * > * Return: Pointer to the new property set or error pointer. > */ > -static struct property_set *pset_copy_set(const struct property_set *pset) > +static struct property_set *pset_create_set(const struct property_entry *src) > { > struct property_entry *properties; > struct property_set *p; > @@ -465,7 +514,11 @@ static struct property_set *pset_copy_set(const struct property_set *pset) > if (!p) > return ERR_PTR(-ENOMEM); > > - properties = property_entries_dup(pset->properties); > + INIT_LIST_HEAD(&p->child_node); > + INIT_LIST_HEAD(&p->children); > + p->fwnode.ops = &pset_fwnode_ops; > + > + properties = property_entries_dup(src); > if (IS_ERR(properties)) { > kfree(p); > return ERR_CAST(properties); > @@ -521,20 +574,53 @@ EXPORT_SYMBOL_GPL(device_remove_properties); > int device_add_properties(struct device *dev, > const struct property_entry *properties) > { > - struct property_set *p, pset; > + struct property_set *p; > > if (!properties) > return -EINVAL; > > - pset.properties = properties; > - > - p = pset_copy_set(&pset); > + p = pset_create_set(properties); > if (IS_ERR(p)) > return PTR_ERR(p); > > - p->fwnode.ops = &pset_fwnode_ops; > set_secondary_fwnode(dev, &p->fwnode); > p->dev = dev; > return 0; > } > EXPORT_SYMBOL_GPL(device_add_properties); > + > +/** > + * device_add_child_properties - Add a collection of properties to a device object. > + * @dev: Device to add properties to. @parent: missing > + * @properties: Collection of properties to add. > + * > + * Associate a collection of device properties represented by @properties as a > + * child of given @parent firmware node. The function takes a copy of > + * @properties. > + */ > +struct fwnode_handle * > +device_add_child_properties(struct device *dev, device_add_child_properties(struct device *dev, const char *child_name, > + struct fwnode_handle *parent, > + const struct property_entry *properties) > +{ > + struct property_set *p; > + struct property_set *parent_pset; > + > + if (!properties) > + return ERR_PTR(-EINVAL); > + > + parent_pset = to_pset_node(parent); > + if (!parent_pset) > + return ERR_PTR(-EINVAL); > + > + p = pset_create_set(properties); > + if (IS_ERR(p)) > + return ERR_CAST(p); > + > + p->dev = dev; p->name = kstrdup_const(child_name, GFP_KERNEL); You'll need to kfree_const(pset->name) in pset_free_set() of course. > + p->parent = parent_pset; > + list_add_tail(&p->child_node, &parent_pset->children); > + > + return &p->fwnode; > +} > +EXPORT_SYMBOL_GPL(device_add_child_properties); Br, -- heikki