On Wed, Nov 16, 2022 at 04:09:44PM +0200, Andy Shevchenko wrote: > On Wed, Nov 16, 2022 at 02:47:07PM +0100, Thierry Reding wrote: > > On Wed, Nov 16, 2022 at 11:18:59AM +0200, Andy Shevchenko wrote: ... > > > + /* If the calling driver did not initialize firmware node, do it here */ > > > if (gc->fwnode) > > > fwnode = gc->fwnode; > > > else if (gc->parent) > > > fwnode = dev_fwnode(gc->parent); > > > + gc->fwnode = fwnode; > > > > I'm not sure we want to set this one. We recently discussed this in > > another thread and my reading is that gc->fwnode is supposed to be used > > only to explicitly override which fwnode to use if the default isn't > > appropriate. Right now the standard way to access the device's fwnode > > seems to be dev_fwnode(&gdev->dev), with only very few exceptions, so > > it'd be great if we could settle on that, rather than introduce a second > > field that contains the same value and use them interchangeably. > > > > One way we could enforce this is by setting gc->fwnode to NULL here > > instead of fwnode. That should cause a crash anywhere it's used after > > this, so we should be able to easily weed out any abuses. > > > > Of course if people prefer to use gc->fwnode instead, then we may want > > to remove all uses of gdev->dev.fwnode. There's simply no point in > > keeping the same value in two different place, it's just going to lead > > to a big mess. > > I prefer that we explicitly use GPIO device firmware node. > Independently on this message I came up with another patch > (I'm just about to sent it right away) which I think it > the best to have in current case. > > Ideally I would like to see const struct gpio_chip *gc to be a parameter > to the GPIO chip add(). But it may happen in distant future. I have updated this patch locally to use dev_of_node() instead of to_of_node(chip->fwnode), and also relying on the patch I just sent. Nevertheless, for of_gpiochip_add()/of_gpiochip_remove() and of_mm_gpiochip_add_data() I still left use of fwnode, because it feels the right thing to do: we are taking reference on the input data in such cases. -- With Best Regards, Andy Shevchenko
