[PATCH] iio: adc: xilinx-ams: Use device_for_each_child_node_scoped()

[Michal Simek]

On 4/28/24 18:17, Jonathan Cameron wrote:
> On Sat, 27 Apr 2024 06:13:01 -0300
> Marcelo Schmitt <marcelo.schmitt1 at gmail.com> wrote:
> 
>> Hi Pedro, Roberto,
>>
>> Patch looks overall good except for the _scoped() function name and arguments,
>> must have been miss-typed or miss-copied somehow.
>> Comment inline.
>>
>> Regards,
>> Marcelo
>>
>> On 04/24, Pedro Mariano wrote:
>>> Using device_for_each_child_node_scoped instead of
>>> device_for_each_child_node automatically releases the handle on early exit
>>> which reduces the chance of bugs that cause resource leaks.
>>>
>>> Co-developed-by: Roberto Bolgheroni <robertobolgheroni at usp.br>
>>> Signed-off-by: Roberto Bolgheroni <robertobolgheroni at usp.br>
>>> Signed-off-by: Pedro Mariano <pedro.mariano at usp.br>
>>> ---
>>>   drivers/iio/adc/xilinx-ams.c | 7 ++-----
>>>   1 file changed, 2 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
>>> index f0b71a122..7f5571d9d 100644
>>> --- a/drivers/iio/adc/xilinx-ams.c
>>> +++ b/drivers/iio/adc/xilinx-ams.c
>>> @@ -1261,7 +1261,6 @@ static int ams_parse_firmware(struct iio_dev *indio_dev)
>>>   	struct ams *ams = iio_priv(indio_dev);
>>>   	struct iio_chan_spec *ams_channels, *dev_channels;
>>>   	struct device *dev = indio_dev->dev.parent;
>>> -	struct fwnode_handle *child = NULL;
>>>   	struct fwnode_handle *fwnode = dev_fwnode(dev);
>>>   	size_t ams_size;
>>>   	int ret, ch_cnt = 0, i, rising_off, falling_off;
>>> @@ -1283,13 +1282,11 @@ static int ams_parse_firmware(struct iio_dev *indio_dev)
>>>   		num_channels += ret;
>>>   	}
>>>   
>>> -	fwnode_for_each_child_node(fwnode, child) {
>>> +	fwnode_for_each_child_node_scoped(fwnode, child) {
>> should be
>> 	device_for_each_child_node_scoped(dev, child) {
> 
> Yes, we didn't bother with a fwnode specific version of this macro because they
> aren't nearly as common.  I'm not sure why this driver didn't always use the
> device form or why it needs the fwnode_device_is_available()
> I suspect this dates back to some confusion on why there were _available variants.
> 
> Chances are this driver only cares about DT and in that case the callback used is

That's correct. We are using this driver on DT system.

Thanks,
Michal