Rafael, looking for your input on this.
Just converted it again, having forgotten this patch set was outstanding until
I saw the same line of code and it all came back..
Thanks,
Jonathan
On Sun, 3 Oct 2021 16:45:41 +0100
Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> On Sun, 15 Aug 2021 17:09:51 +0100
> Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> > On Tue, 27 Jul 2021 19:20:13 +0100
> > Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> wrote:
> >
> > > On Tue, 27 Jul 2021 17:16:07 +0300
> > > Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
> > >
> > > > On Tue, Jul 27, 2021 at 4:52 PM Jonathan Cameron
> > > > <Jonathan.Cameron@xxxxxxxxxx> wrote:
> > > > > On Sun, 25 Jul 2021 23:33:12 +0300
> > > > > Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
> > > > > > On Sun, Jul 25, 2021 at 8:22 PM Jonathan Cameron <jic23@xxxxxxxxxx> 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,
> >
> > Rafael, if you have a chance to give input on the questions below it would
> > be much appreciated.
>
> Rafael, if you have a chance to look at this it would be great.
> >
> > Thanks,
> >
> > Jonathan
> >
> > >
> > > 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.
> > > >
> > > >
> > >
> >
>
