Re: [PATCH 2/2] iio:adc:ad7124: Convert to fwnode handling of child node parsing.

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Tue, 27 Jul 2021 17:16:07 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, Jul 27, 2021 at 4:52 PM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> > On Sun, 25 Jul 2021 23:33:12 +0300
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:  
> > > On Sun, Jul 25, 2021 at 8:22 PM Jonathan Cameron <jic23@kernel.org> wrote:  
> 
> ...
> 
> > > > -       for_each_available_child_of_node(np, child) {
> > > > +       device_for_each_child_node(dev, child) {  
> > >
> > > Isn't this
> > >   fwnode_for_each_available_child_node()
> > > better to use?  
> >
> > Given we would be extracting the fwnode just to call this
> > loop, I'd say no, device version makes more sense..
> >  
> > >
> > > ...
> > >
> > > So the gaps I see are
> > >   device_get_available_child_node_count()
> > > and
> > >   device_for_each_available_child_node()  
> >
> > Do we then fix the fact that
> > device_for_each_child_node() will call the _available() form
> > for device tree?  That seems inconsistent currently and
> > I was assuming that was deliberate...  
> 
> I'm not sure I got your point. Mine (see below) is to add the APIs
> that you want to use as a direct replacement of the corresponding OF
> counterparts.
+CC Rafael,

The oddity is that device_for_each_child_node() is a direct replacement
of the for_each_available_child_of_node() other than the obvious
use of device rather than the of node.

https://elixir.bootlin.com/linux/v5.14-rc3/source/drivers/of/property.c#L939

static struct fwnode_handle *
of_fwnode_get_next_child_node(const struct fwnode_handle *fwnode,
			      struct fwnode_handle *child)
{
	return of_fwnode_handle(of_get_next_available_child(to_of_node(fwnode),
							    to_of_node(child)));
}

So the question becomes whether there is any desire at all to have a
version of the device_for_each_child_node() that does not check
if it is available or not.

Looks like it goes all the way back.  Rafael, any comment on why the available
for is used here and whether it makes sense to introduce separate
versions for looping over children that cover the _available_ and everything
cases?

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/base/property.c?id=8a0662d9ed2968e1186208336a8e1fab3fdfea63

I'm kind of assuming this was deliberate as we don't want to encourage
accessing disabled firmware nodes.

Jonathan

> 
> > > Both of them I think are easy to add and avoid possible breakage.  
> 
>