Re: [PATCH 2/6] hwmon: (ltc2992) use device_for_each_child_node_scoped() to access child nodes

[Jonathan Cameron]

On Sat, 06 Jul 2024 17:23:34 +0200
Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> wrote:

> The iterated nodes are direct children of the device node, and the
> `device_for_each_child_node()` macro accounts for child node
> availability.
> 
> `fwnode_for_each_available_child_node()` is meant to access the child
> nodes of an fwnode, and therefore not direct child nodes of the device
> node.
> 
> In this case, the child nodes are not required outside the loop, and
> the scoped version of the macro can be used to remove the repetitive
> `goto put` pattern.
> 
> Use `device_for_each_child_node_scoped_scoped()` to indicate device's
> direct child nodes.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx>
One thing that is possibly a follow up suggestion but could maybe
be done in this patch.  Either way.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>

> ---
>  drivers/hwmon/ltc2992.c | 19 +++++--------------
>  1 file changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/hwmon/ltc2992.c b/drivers/hwmon/ltc2992.c
> index d4a93223cd3b..541fa09dc6e7 100644
> --- a/drivers/hwmon/ltc2992.c
> +++ b/drivers/hwmon/ltc2992.c
> @@ -854,33 +854,24 @@ static const struct regmap_config ltc2992_regmap_config = {
>  
>  static int ltc2992_parse_dt(struct ltc2992_state *st)
>  {
> -	struct fwnode_handle *fwnode;
> -	struct fwnode_handle *child;

I'd take the opportunity for
	struct device *dev = &st->client->dev;

Given there are several users and this modifies one of them.

>  	u32 addr;
>  	u32 val;
>  	int ret;
>  
> -	fwnode = dev_fwnode(&st->client->dev);
> -
> -	fwnode_for_each_available_child_node(fwnode, child) {
> +	device_for_each_child_node_scoped(&st->client->dev, child) {
>  		ret = fwnode_property_read_u32(child, "reg", &addr);
> -		if (ret < 0) {
> -			fwnode_handle_put(child);
> +		if (ret < 0)
>  			return ret;
> -		}
>  
> -		if (addr > 1) {
> -			fwnode_handle_put(child);
> +		if (addr > 1)
>  			return -EINVAL;
> -		}
>  
>  		ret = fwnode_property_read_u32(child, "shunt-resistor-micro-ohms", &val);
>  		if (!ret) {
> -			if (!val) {
> -				fwnode_handle_put(child);
> +			if (!val)
>  				return dev_err_probe(&st->client->dev, -EINVAL,
>  						     "shunt resistor value cannot be zero\n");
> -			}
> +
>  			st->r_sense_uohm[addr] = val;
>  		}
>  	}
>