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: > > All new drivers should use fwnode and / or parent to provide the > > necessary information to the GPIO library. ... > > + /* 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. -- With Best Regards, Andy Shevchenko
