Re: GPIO static allocation warning with v6.2-rcX

[Kent Gibson <warthog618@xxxxxxxxx>]

On Thu, Jan 26, 2023 at 11:14:58AM +0100, Sascha Hauer wrote:
> On Thu, Jan 26, 2023 at 09:57:18AM +0800, Kent Gibson wrote:
> > On Wed, Jan 25, 2023 at 10:35:48AM +0100, Sascha Hauer wrote:
> > > On Mon, Jan 23, 2023 at 03:55:18PM +0100, Bartosz Golaszewski wrote:
> > > > On Fri, Jan 20, 2023 at 11:46 AM Marco Felsch <m.felsch@xxxxxxxxxxxxxx> wrote:
> > > > >
> > > > > Hi all,
> > > > >
> > > > > I stumbled over the following warning while testing the new v6.2-rc4 on
> > > > > a imx8mm-evk:
> > > > >
> > > > > [    1.507131] gpio gpiochip0: Static allocation of GPIO base is deprecated, use dynamic allocation.
> > > > > [    1.517786] gpio gpiochip1: Static allocation of GPIO base is deprecated, use dynamic allocation.
> > > > > [    1.528273] gpio gpiochip2: Static allocation of GPIO base is deprecated, use dynamic allocation.
> > > > > [    1.538739] gpio gpiochip3: Static allocation of GPIO base is deprecated, use dynamic allocation.
> > > > > [    1.549195] gpio gpiochip4: Static allocation of GPIO base is deprecated, use dynamic allocation.
> > > > >
> > > > > The warning was introduced by commit [1] but at least the following
> > > > > drivers are parsing the alias for a gpiochip to use it as base:
> > > > >  - drivers/gpio/gpio-mxs.c
> > > > >  - drivers/gpio/gpio-mxc.c
> > > > >  - drivers/gpio/gpio-clps711x.c
> > > > >  - drivers/gpio/gpio-mvebu.c
> > > > >  - drivers/gpio/gpio-rockchip.c
> > > > >  - drivers/gpio/gpio-vf610.c
> > > > >  - drivers/gpio/gpio-zynq.c
> > > > >
> > > > > According commit [2] it seems valid and correct to me to use the alias
> > > > > and the user-space may rely on this.
> > > > >
> > > > > Now my question is how we can get rid of the warning without breaking
> > > > > the user-space?
> > > > >
> > > > > [1] 502df79b86056 gpiolib: Warn on drivers still using static gpiobase allocation
> > > > > [2] 7e6086d9e54a1 gpio/mxc: specify gpio base for device tree probe
> > > > >
> > > > 
> > > > The warning is there to remind you that static GPIO base numbers have
> > > > been long deprecated and only user-space programs using sysfs will
> > > > break if you remove it, everyone else - including user-space programs
> > > > using libgpiod or scripts using gpio-tools that are part of the
> > > > project - will be fine.
> > > > 
> > > > Any chance you can port your user-space programs to libgpiod?
> > > > 
> > > > The warning doesn't break compatibility so I'm not eager to remove it.
> > > 
> > > Well it's a warning and sooner or later somebody will come along and
> > > removes this warning by removing the GPIO controller bases from the dts
> > > files which in turn will then break things at least for us, but I
> > > suspect for many other people as well.
> > > 
> > > You are trying to remove the GPIO sysfs API for many years now without
> > > success so far, and I doubt that you will succeed in future because the
> > > Kernel comes with the promise that userspace won't be broke.
> > > 
> > > I can understand that you want to get rid of the global GPIO number
> > > space. Currently you can't, because there are still hundreds of
> > > in-Kernel users of the legacy API. When all these are fixed and the GPIO
> > > sysfs API is the only remaining user of the global GPIO number space
> > > then we could move the numbering to gpiolib-sysfs.c and no longer bother
> > > the core with it. At this point the sysfs API would be a GPIO consumer
> > > just like every other consumer and we could leave it there until only
> > > the oldest of us remember what it's good for.
> > > 
> > > Instead of trying to remove the sysfs API I really think it would be a
> > > better strategy to push it into a corner where it can stay without
> > > being a maintenance burden.
> > > 
> > > Regarding the usage of libgpiod for our projects: I think one of the
> > > major shortcomings is that the character interface doesn't allow to
> > > just set a GPIO to a value and leave it in that state without having
> > > to keep the process alive. While you may argument that it's cleaner
> > > to go to a "safe state" (or "idle state") when the process finishes
> > > that's simply not the way how many projects out there work.
> > 
> > You can argue that, but that is not what cdev and the gpiolib subsystem 
> > do.
> > 
> > When a line is released cdev and the gpiolib subsystem do not explicitly
> > change anything, so the line may well remain in the state it was set.
> > The state becomes "undefined" from the user perspective, as the line is
> > now accessible to other processes and as the kernel MAY reset it.
> > The latter is the case where the line being released is the last
> > requested line on a gpiochip, in which case the gpiolib subsystem 
> > will release the chip and the chip MAY get reset back to defaults
> > (depends on the gpiochip).
> > 
> > Given that, you can get sysfs-like behaviour as long as you hold at least
> > one line on a GPIO chip, and that could be a line hogged from DT or an
> > other internal kernel user.
> 
> Having to hold one line to get a well defined behaviour of another line
> is a kludge or a workaround, not a solution.
> 

Strictly speaking it isn't even a well defined behaviour, so my bad for
even suggesting it.

Cheers,
Kent.