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.
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.
> >
> >
>
