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
