| Message ID | 5e35f44db2b4ed43f75c4c53fd0576df9ad24ab2.1740993491.git.mazziesaccount@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Support ROHM BD79124 ADC | expand |
Hi, > +/** > + * fwnode_get_child_node_count_named - number of child nodes with given name > + * @fwnode: Node which child nodes are counted. > + * @name: String to match child node name against. > + * > + * Scan child nodes and count all the nodes with a specific name. Return the > + * number of found nodes. Potential '@number' -ending for scanned names is > + * ignored. Eg, > + * device_get_child_node_count(dev, "channel"); > + * would match all the nodes: > + * channel { }, channel@0 {}, channel@0xabba {}... > + * > + * Return: the number of child nodes with a matching name for a given device. > + */ > +unsigned int fwnode_get_child_node_count_named(const struct fwnode_handle *fwnode, > + const char *name) > +{ > + struct fwnode_handle *child; > + unsigned int count = 0; > + > + fwnode_for_each_child_node(fwnode, child) > + if (fwnode_name_eq(child, name)) > + count++; > + > + return count; > +} > +EXPORT_SYMBOL_GPL(fwnode_get_child_node_count_named); > + > +/** > + * device_get_child_node_count_named - number of child nodes with given name > + * @dev: Device to count the child nodes for. > + * @name: String to match child node name against. > + * > + * Scan device's child nodes and find all the nodes with a specific name and > + * return the number of found nodes. Potential '@number' -ending for scanned > + * names is ignored. Eg, > + * device_get_child_node_count(dev, "channel"); > + * would match all the nodes: > + * channel { }, channel@0 {}, channel@0xabba {}... > + * > + * Return: the number of child nodes with a matching name for a given device. > + */ > +unsigned int device_get_child_node_count_named(const struct device *dev, > + const char *name) > +{ > + const struct fwnode_handle *fwnode = dev_fwnode(dev); > + > + if (!fwnode) > + return -EINVAL; > + > + if (IS_ERR(fwnode)) > + return PTR_ERR(fwnode); > + > + return fwnode_get_child_node_count_named(fwnode, name); > +} > +EXPORT_SYMBOL_GPL(device_get_child_node_count_named); Sorry if I missed something in the v4 thread, but why not do all the checks in fwnode_get_child_node_count_named(), and make this an inline function? static inline unsigned int device_get_child_node_count_named(const struct device *dev, const char *name) { return fwnode_get_child_node_count_named(dev_fwnode(fwnode), name); } thanks,
On Mon, Mar 03, 2025 at 01:31:45PM +0200, Matti Vaittinen wrote: > There are some use-cases where child nodes with a specific name need to > be parsed. In a few cases the data from the found nodes is added to an > array which is allocated based on the number of found nodes. One example > of such use is the IIO subsystem's ADC channel nodes, where the relevant > nodes are named as channel[@N]. > > Add a helpers for counting device's sub-nodes with certain name instead > of open-coding this in every user. ... > +unsigned int fwnode_get_child_node_count_named(const struct fwnode_handle *fwnode, > + const char *name) > +{ > + struct fwnode_handle *child; > + unsigned int count = 0; > + fwnode_for_each_child_node(fwnode, child) > + if (fwnode_name_eq(child, name)) I would expect this to be a separate macro fwnode_for_each_named_child_node() (and its device variant) that gives us more consistent approach. > + count++; And the above looks like missing {}, which won't be needed with the other suggestion in place. > + return count; > +} > + if (!fwnode) > + return -EINVAL; > + > + if (IS_ERR(fwnode)) > + return PTR_ERR(fwnode); I expect that this will return 0 or number of nodes. Why do we need an error code? If it's really required, it should be in the fwnode API above. Also do we care about secondary fwnodes? > + return fwnode_get_child_node_count_named(fwnode, name); > +} ... > +unsigned int fwnode_get_child_node_count_named(const struct fwnode_handle *fwnode, > + const char *name); To me the following name sounds better: fwnode_get_named_child_node_count(). > +unsigned int device_get_child_node_count_named(const struct device *dev, > + const char *name); In the similar way.
On Mon, Mar 03, 2025 at 01:50:13PM +0200, Heikki Krogerus wrote: ... > > +unsigned int device_get_child_node_count_named(const struct device *dev, > > + const char *name) > > +{ > > + const struct fwnode_handle *fwnode = dev_fwnode(dev); > > + > > + if (!fwnode) > > + return -EINVAL; > > + > > + if (IS_ERR(fwnode)) > > + return PTR_ERR(fwnode); > > + > > + return fwnode_get_child_node_count_named(fwnode, name); > > +} > > +EXPORT_SYMBOL_GPL(device_get_child_node_count_named); > > Sorry if I missed something in the v4 thread, but why not do all the > checks in fwnode_get_child_node_count_named(), and make this an inline > function? +1, or drop the checks and make it return 0 depending on the follow up use cases. > static inline unsigned int > device_get_child_node_count_named(const struct device *dev, const char *name) > { > return fwnode_get_child_node_count_named(dev_fwnode(fwnode), name); > }
diff --git a/drivers/base/property.c b/drivers/base/property.c index c1392743df9c..3faf02b99cff 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -945,6 +945,63 @@ unsigned int device_get_child_node_count(const struct device *dev) } EXPORT_SYMBOL_GPL(device_get_child_node_count); +/** + * fwnode_get_child_node_count_named - number of child nodes with given name + * @fwnode: Node which child nodes are counted. + * @name: String to match child node name against. + * + * Scan child nodes and count all the nodes with a specific name. Return the + * number of found nodes. Potential '@number' -ending for scanned names is + * ignored. Eg, + * device_get_child_node_count(dev, "channel"); + * would match all the nodes: + * channel { }, channel@0 {}, channel@0xabba {}... + * + * Return: the number of child nodes with a matching name for a given device. + */ +unsigned int fwnode_get_child_node_count_named(const struct fwnode_handle *fwnode, + const char *name) +{ + struct fwnode_handle *child; + unsigned int count = 0; + + fwnode_for_each_child_node(fwnode, child) + if (fwnode_name_eq(child, name)) + count++; + + return count; +} +EXPORT_SYMBOL_GPL(fwnode_get_child_node_count_named); + +/** + * device_get_child_node_count_named - number of child nodes with given name + * @dev: Device to count the child nodes for. + * @name: String to match child node name against. + * + * Scan device's child nodes and find all the nodes with a specific name and + * return the number of found nodes. Potential '@number' -ending for scanned + * names is ignored. Eg, + * device_get_child_node_count(dev, "channel"); + * would match all the nodes: + * channel { }, channel@0 {}, channel@0xabba {}... + * + * Return: the number of child nodes with a matching name for a given device. + */ +unsigned int device_get_child_node_count_named(const struct device *dev, + const char *name) +{ + const struct fwnode_handle *fwnode = dev_fwnode(dev); + + if (!fwnode) + return -EINVAL; + + if (IS_ERR(fwnode)) + return PTR_ERR(fwnode); + + return fwnode_get_child_node_count_named(fwnode, name); +} +EXPORT_SYMBOL_GPL(device_get_child_node_count_named); + bool device_dma_supported(const struct device *dev) { return fwnode_call_bool_op(dev_fwnode(dev), device_dma_supported); diff --git a/include/linux/property.h b/include/linux/property.h index e214ecd241eb..269ab539515b 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -209,6 +209,10 @@ int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index); int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name); unsigned int device_get_child_node_count(const struct device *dev); +unsigned int fwnode_get_child_node_count_named(const struct fwnode_handle *fwnode, + const char *name); +unsigned int device_get_child_node_count_named(const struct device *dev, + const char *name); static inline int device_property_read_u8(const struct device *dev, const char *propname, u8 *val)
There are some use-cases where child nodes with a specific name need to be parsed. In a few cases the data from the found nodes is added to an array which is allocated based on the number of found nodes. One example of such use is the IIO subsystem's ADC channel nodes, where the relevant nodes are named as channel[@N]. Add a helpers for counting device's sub-nodes with certain name instead of open-coding this in every user. Suggested-by: Jonathan Cameron <jic23@kernel.org> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> --- Revision history: v4 => v5: - Use given name instead of string 'channel' when counting the nodes - Add also fwnode_get_child_node_count_named() as suggested by Rob. v3 => v4: - New patch as suggested by Jonathan, see discussion in: https://lore.kernel.org/lkml/20250223161338.5c896280@jic23-huawei/ --- drivers/base/property.c | 57 ++++++++++++++++++++++++++++++++++++++++ include/linux/property.h | 4 +++ 2 files changed, 61 insertions(+)