| Message ID | 20240820-device_child_node_access-v3-1-1ee09bdedb9e@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | use device_for_each_child_node_scoped to access device child nodes | expand |
On Tue, 20 Aug 2024 21:02:26 +0200 Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote: > Use `device_for_each_child_node_scoped()` in `ams_parse_firmware()` > to explicitly state device child node access, and simplify the child > node handling as it is not required outside the loop. > > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> Applied, but I would ideally still like one of the xilinx folk others familiar with this driver to take a look. It'll be a few days before this ends up in next anyway as I need to rebase after Greg (hopefully) takes the pull request from last week. It would be lovely to get rid of the direct fwnode usage in here but I'm not 100% sure if there is a path that will land on a disabled fwnode. Thanks, Jonathan > --- > drivers/iio/adc/xilinx-ams.c | 15 +++++---------- > 1 file changed, 5 insertions(+), 10 deletions(-) > > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c > index f051358d6b50..ebc583b07e0c 100644 > --- a/drivers/iio/adc/xilinx-ams.c > +++ b/drivers/iio/adc/xilinx-ams.c > @@ -1275,7 +1275,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; > @@ -1297,16 +1296,12 @@ static int ams_parse_firmware(struct iio_dev *indio_dev) > num_channels += ret; > } > > - fwnode_for_each_child_node(fwnode, child) { > - if (fwnode_device_is_available(child)) { > - ret = ams_init_module(indio_dev, child, ams_channels + num_channels); > - if (ret < 0) { > - fwnode_handle_put(child); > - return ret; > - } > + device_for_each_child_node_scoped(dev, child) { > + ret = ams_init_module(indio_dev, child, ams_channels + num_channels); > + if (ret < 0) > + return ret; > > - num_channels += ret; > - } > + num_channels += ret; > } > > for (i = 0; i < num_channels; i++) { >
Hi Jonathan, On 8/26/24 12:52, Jonathan Cameron wrote: > On Tue, 20 Aug 2024 21:02:26 +0200 > Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote: > >> Use `device_for_each_child_node_scoped()` in `ams_parse_firmware()` >> to explicitly state device child node access, and simplify the child >> node handling as it is not required outside the loop. >> >> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> > Applied, but I would ideally still like one of the xilinx folk > others familiar with this driver to take a look. It'll be a > few days before this ends up in next anyway as I need to rebase > after Greg (hopefully) takes the pull request from last week. I just get back from vacation. > > It would be lovely to get rid of the direct fwnode usage > in here but I'm not 100% sure if there is a path that will land > on a disabled fwnode. Conall: Please take a look at this and test it. Thanks, Michal
diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c index f051358d6b50..ebc583b07e0c 100644 --- a/drivers/iio/adc/xilinx-ams.c +++ b/drivers/iio/adc/xilinx-ams.c @@ -1275,7 +1275,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; @@ -1297,16 +1296,12 @@ static int ams_parse_firmware(struct iio_dev *indio_dev) num_channels += ret; } - fwnode_for_each_child_node(fwnode, child) { - if (fwnode_device_is_available(child)) { - ret = ams_init_module(indio_dev, child, ams_channels + num_channels); - if (ret < 0) { - fwnode_handle_put(child); - return ret; - } + device_for_each_child_node_scoped(dev, child) { + ret = ams_init_module(indio_dev, child, ams_channels + num_channels); + if (ret < 0) + return ret; - num_channels += ret; - } + num_channels += ret; } for (i = 0; i < num_channels; i++) {
Use `device_for_each_child_node_scoped()` in `ams_parse_firmware()` to explicitly state device child node access, and simplify the child node handling as it is not required outside the loop. Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> --- drivers/iio/adc/xilinx-ams.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)