| Message ID | 20200608134300.76091-3-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | mfd: Make use of software nodes | expand |
On Mon, Jun 8, 2020 at 3:43 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > For backward compatibility with some legacy devices, introduce > a new property snps,gpio-base to read GPIO base. Don't advertise > to discourage users from utilizing it. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> I usually have a very negative gut reaction to any attempts to hardcode the heavily deprecated use of the gpio_base for the global GPIO numberspace. The reason is clear from drivers/gpio/TODO I think. I need a really good explanation why this is needed, the only reason people have been pushing this in the past is "oh we are using the sysfs and we don't wanna change the GPIO numbers in our scripts" which I really want to push back on now that we have the chardev and the libgpiod utils. If this is needed for something driver internal, it should stay in a driver-local variable. Yours, Linus Walleij
On Wed, Jun 10, 2020 at 01:26:59PM +0200, Linus Walleij wrote: > On Mon, Jun 8, 2020 at 3:43 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > For backward compatibility with some legacy devices, introduce > > a new property snps,gpio-base to read GPIO base. Don't advertise > > to discourage users from utilizing it. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > I usually have a very negative gut reaction to any attempts to hardcode > the heavily deprecated use of the gpio_base for the global GPIO > numberspace. The reason is clear from drivers/gpio/TODO I think. > > I need a really good explanation why this is needed, the only reason > people have been pushing this in the past is "oh we are using the > sysfs and we don't wanna change the GPIO numbers in our scripts" > which I really want to push back on now that we have the chardev and > the libgpiod utils. I understand the point, that's why this property is not being advertised. The problem here is that on existing (legacy) platform we have to deal with an absolute numbers of GPIOs and those numbers can work if we keep the base > If this is needed for something driver internal, it should stay in a > driver-local > variable. It's a cross drivers variable. The purpose of this series is to unify interface in the GPIO driver how to deal with device properties (so, be agnostic from resource provider at the end). Everything so far so good, but then we need a mean how not to break the platform. Because dropping the base in this case will immediately make a regression. We can re-use 'gpio-base' as gpio-mockup is using it silently. So, in such case we won't add any deviation to the property namespace. Or we can rename above to linux,gpio-base and re-use it here with this name to explicitly show that this is not firmware property.
diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c index a7ca72086511..e3d7589434eb 100644 --- a/drivers/gpio/gpio-dwapb.c +++ b/drivers/gpio/gpio-dwapb.c @@ -599,7 +599,8 @@ static struct dwapb_platform_data *dwapb_gpio_get_pdata(struct device *dev) pp->ngpio = 32; } - pp->gpio_base = -1; + if (fwnode_property_read_u32(fwnode, "snps,gpio-base", &pp->gpio_base)) + pp->gpio_base = -1; /* * Only port A can provide interrupts in all configurations of
For backward compatibility with some legacy devices, introduce a new property snps,gpio-base to read GPIO base. Don't advertise to discourage users from utilizing it. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/gpio/gpio-dwapb.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)