Re: [PATCH v7 02/10] property: Add functions to iterate named child

[Marcelo Schmitt]

Hello,

LGTM, few minor comments inline.
With those sorted
Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@xxxxxxxxx>

On 03/13, Matti Vaittinen wrote:
> There are a few use-cases where child nodes with a specific name need to
> be parsed. Code like:
> 
> fwnode_for_each_child_node()
> 	if (fwnode_name_eq())
> 		...
> 
> can be found from a various drivers/subsystems. Adding a macro for this
> can simplify things a bit.
> 
> In a few cases the data from the found nodes is later 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 helpers for iterating and counting device's sub-nodes with certain
> name instead of open-coding this in every user.
> 
> Suggested-by: Jonathan Cameron <jic23@xxxxxxxxxx>
> Signed-off-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
> ---
...
> +/**
> + * fwnode_get_named_child_node_count - 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. Potential
> + * 'number' -ending after the 'at sign' for scanned names is ignored.
> + * E.g.::
> + *   device_get_named_child_node_count(dev, "channel");

The function being documented is fwnode_get_named_child_node_count().
Shouldn't the example be
   fwnode_get_named_child_node_count(fwnode, "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.
> + */
...
> +#define device_for_each_named_child_node(dev, child, name)		\
> +		device_for_each_child_node(dev, child)			\
> +			if (!fwnode_name_eq(child, name)) { } else

nitpicking: add only one tab for each indentation level.
	device_for_each_child_node(dev, child)			\
		if (!fwnode_name_eq(child, name)) { } else

> +
>  #define device_for_each_child_node_scoped(dev, child)			\
>  	for (struct fwnode_handle *child __free(fwnode_handle) =	\
>  		device_get_next_child_node(dev, NULL);			\
>  	     child; child = device_get_next_child_node(dev, child))
>  
> +#define device_for_each_named_child_node_scoped(dev, child, name)	\
> +		device_for_each_child_node_scoped(dev, child)		\
> +			if (!fwnode_name_eq(child, name)) { } else
> +

nitpicking: instead of two tabs, only one tab before device_for_..._scoped().
	device_for_each_child_node_scoped(dev, child)		\
		if (!fwnode_name_eq(child, name)) { } else

Regards,
Marcelo