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

[Kent Gibson <warthog618@xxxxxxxxx>]

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.

e.g. I have a RPi4 which has the STATUS_LED_G_CLK line already held so I
can drive the GPIO5 line, which is on the same chip, just like I could
with sysfs:

$ gpioinfo STATUS_LED_G_CLK GPIO5
gpiochip0 5	"GPIO5"         	input
gpiochip0 42	"STATUS_LED_G_CLK"	output consumer="led0"

# set GPIO5 high
$ gpioset -t0 GPIO5=1

# GPIO5 has been released but is still an output and active:
$ gpioinfo GPIO5
gpiochip0 5	"GPIO5"         	output
$ gpioget --as-is GPIO5
"GPIO5"=active

# set GPIO5 low
$ gpioset -t0 GPIO5=0
$ gpioget --as-is GPIO5
"GPIO5"=inactive

# set GPIO5 high
$ gpioset -t0 GPIO5=1
$ gpioget --as-is GPIO5
"GPIO5"=active
...

That is using the tools from libgpiod v2, btw, but just for the
gpioget "--as-is" option to prevent gpioget switching the line to an
input. The same mechanic works with libgpiod v1.

Would that work for your cases?

Cheers,
Kent.

> Virtually
> everyone has scripts poking GPIO states into sysfs and currently you
> can't do this with the character device API. If you want to get rid
> of the sysfs API then you should work on making the character device
> API more attractive for users and I think this is one point could use
> some improvement.
> 
> Sascha
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |