Re: [PATCH v6 1/2] iio: adc: mcp3564: Use device_for_each_child_node_scoped()

[<Marius.Cristea@microchip.com>]

Hi Jonathan,



On Sun, 2024-04-28 at 18:40 +0100, Jonathan Cameron wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Switching to the _scoped() version removes the need for manual
> calling of fwnode_handle_put() in the paths where the code
> exits the loop early. In this case that's all in error paths.
> 
> Cc: Marius Cristea <marius.cristea@microchip.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/adc/mcp3564.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/adc/mcp3564.c b/drivers/iio/adc/mcp3564.c
> index 311b613b6057..e2ae13f1e842 100644
> --- a/drivers/iio/adc/mcp3564.c
> +++ b/drivers/iio/adc/mcp3564.c
> @@ -998,7 +998,6 @@ static int mcp3564_parse_fw_children(struct
> iio_dev *indio_dev)
>         struct mcp3564_state *adc = iio_priv(indio_dev);
>         struct device *dev = &adc->spi->dev;
>         struct iio_chan_spec *channels;
> -       struct fwnode_handle *child;
>         struct iio_chan_spec chanspec = mcp3564_channel_template;
>         struct iio_chan_spec temp_chanspec =
> mcp3564_temp_channel_template;
>         struct iio_chan_spec burnout_chanspec =
> mcp3564_burnout_channel_template;
> @@ -1025,7 +1024,7 @@ static int mcp3564_parse_fw_children(struct
> iio_dev *indio_dev)
>         if (!channels)
>                 return dev_err_probe(dev, -ENOMEM, "Can't allocate
> memory\n");
> 
> -       device_for_each_child_node(dev, child) {
> +       device_for_each_child_node_scoped(dev, child) {
>                 node_name = fwnode_get_name(child);
> 
>                 if (fwnode_property_present(child, "diff-channels"))
> {
> @@ -1033,26 +1032,25 @@ static int mcp3564_parse_fw_children(struct
> iio_dev *indio_dev)
>                                                              "diff-
> channels",
>                                                              inputs,
>                                                             
> ARRAY_SIZE(inputs));
> +                       if (ret)
> +                               return ret;
> +
>                         chanspec.differential = 1;
>                 } else {
>                         ret = fwnode_property_read_u32(child, "reg",
> &inputs[0]);
> +                       if (ret)
> +                               return ret;
> 
>                         chanspec.differential = 0;
>                         inputs[1] = MCP3564_AGND;
>                 }
> -               if (ret) {
> -                       fwnode_handle_put(child);
> -                       return ret;
> -               }
> 
>                 if (inputs[0] > MCP3564_INTERNAL_VCM ||
> -                   inputs[1] > MCP3564_INTERNAL_VCM) {
> -                       fwnode_handle_put(child);
> +                   inputs[1] > MCP3564_INTERNAL_VCM)
>                         return dev_err_probe(&indio_dev->dev, -
> EINVAL,
>                                              "Channel index > %d, for
> %s\n",
>                                              MCP3564_INTERNAL_VCM +
> 1,
>                                              node_name);
> -               }
> 
>                 chanspec.address = (inputs[0] << 4) | inputs[1];
>                 chanspec.channel = inputs[0];
> --
> 2.44.0
> 


  The changes looks OK,

Reviewed-by: Marius Cristea <marius.cristea@microchip.com>


Thanks,
Marius