On Sat, 5 Feb 2022 20:58:12 +0200
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> On Sat, Feb 05, 2022 at 08:50:31PM +0200, Andy Shevchenko wrote:
> > On Sat, Feb 05, 2022 at 06:44:59PM +0000, Jonathan Cameron wrote:
> > > On Sat, 5 Feb 2022 19:29:39 +0200
> > > Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:
> > >
> > > A few comments from me, mostly because I couldn't resist jumping in ;)
> > > Note this is only some of the things Andy raised....
> >
> > ...
> >
> > > > > +static int ltc2688_channel_config(struct ltc2688_state *st)
> > > > > +{
> > > > > + struct device *dev = &st->spi->dev;
> > > > > + struct device_node *child;
> > > > > + u32 reg, clk_input, val, tmp[2];
> > > > > + int ret, span;
> > > > > +
> > > > > + for_each_available_child_of_node(dev->of_node, child) {
> > > >
> > > > device_for_each_child_node()
> > >
> > > This is the old issue with missing
> > > device_for_each_available_child_node()
> > > though can just add a check on whether it's available inside the loop.
> >
> > Didn't we discuss this with Rob and he told that device_for_each_child_node()
> > is already for available only?
>
> https://lore.kernel.org/lkml/20211205190101.26de4a57@jic23-huawei/T/#u
>
> So, the fwnode has a correct implementation, and we may use it here.
>
I wasn't totally sure of the conclusion of that discussion.
a) Fine to just use device_for_each_child_node() for this case and not worry about it.
b) Worth adding device_for_each_available_child_node() with the same implementation
c) (possibly workaround / avoid the issue) Use device_for_each_child_node() but also
check validity (hopefully compiler would remove the check) in order to act
as documentation.
I'm fine with any of the above.
J
