GPIO static allocation warning with v6.2-rcX

GPIO static allocation warning with v6.2-rcX

[Marco Felsch]

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

Regards,
  Marco

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

[Bartosz Golaszewski]

On Fri, Jan 20, 2023 at 11:46 AM Marco Felsch <m.felsch@pengutronix.de> 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.

Bart

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

[Bartosz Golaszewski]

On Mon, Jan 23, 2023 at 3:55 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Fri, Jan 20, 2023 at 11:46 AM Marco Felsch <m.felsch@pengutronix.de> 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.
>

Oh and the drivers could use some updating. I'll take a look at them
one-by-one to see if there's anything other than sysfs that could
potentially break if we switch them to using dynamic allocation.

Bart

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

[Sascha Hauer]

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@pengutronix.de> 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. 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 |

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

[Bartosz Golaszewski]

On Wed, Jan 25, 2023 at 10:35 AM Sascha Hauer <sha@pengutronix.de> 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@pengutronix.de> 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.
>

History knows interfaces that were long deprecated, triggered warnings
and were eventually removed (sysctl?). We have not yet broken
user-space. IMO a warning is in order if an interface was deprecated.

> 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.
>

We're also doing it step-by-step.

> 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. 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

On that note: as libgpiod v2.0 is almost ready for release, I started
working on the idea I've long had in mind - DBus interface for GPIOs.
I've started writing the code over Christmas and some very limited
functionality is available on my github[1]. For now it only allows to
read chip properties but my goal is to have a complete DBus API for
libgpiod v2.1. Is this something you could work with? It addresses
your main concern (having a central authority and persistent state of
GPIOs) while using the character device behind the scenes.

Bart

[1] https://github.com/brgl/libgpiod-private/tree/topic/dbus

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

[Kent Gibson]

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@pengutronix.de> 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 |

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

[Linus Walleij]

On Wed, Jan 25, 2023 at 10:35 AM Sascha Hauer <sha@pengutronix.de> wrote:

> 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 see it more as permanent deprecation and nudging than removal as of now.

For some reason people perceive all nudging as militant and as a
my-way-or-the-highway style of communication but it's not really intended.
OK maybe I am softer now than in the past, one grow humbler with
old age.

It will become more interesting once we removed all in-kernel users of
the global numberspace, which is well underway. At that point we can
move the management of the global numberspace into the sysfs code
and distros etc that don't want to use it can compile it out completely.

But the idea was never to slam people to not use something they use and like,
it was to offer something new because they want it and like it more.

So using the character device with libgpiod users can:

- Get and set multiple lines at the same time
- Do biasing (if supported by HW) pull up/down
- Do drive strength (if supported by HW)
- Properly listen to IRQ-driven events on lines with real-time timestamps
  to ensure strict event order
- Use HTE timestamps (new feature!)
- libgpiod for the above with C, C++, Python and Rust bindings

By offering those new tasty features only for the character device and not
for the sysfs ABI, at least the advanced users are expected to switch over
to using the character device.

It's not like we're unaware about the upsides of the sysfs ABI. Easy to use
to do quick hacks. Always there. I have been trying to think of e.g. a
debugfs ABI that would be even easier to use for even quicker hacks, but
using only local per-gpiochip offsets, see the bottom of the TODO file:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/TODO

Yours,
Linus Walleij

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

[Linus Walleij]

On Wed, Jan 25, 2023 at 10:35 AM Sascha Hauer <sha@pengutronix.de> wrote:

> 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.

It is ~500 and used to be thousands.
An update on that specifically.

Mainline:
$ git grep 'linux\/gpio\.h' | wc -l
505

linux-next:
$ git grep 'linux\/gpio\.h' | wc -l
362

This is because Arnd delete a big slew of unmaintained boardfiles which were
the big offenders.

Now certainly we can fix 362 files soon enough, especially since we have not
just me, but Dmitry T and Andy S and Arnd working on it in parallel.

Yours,
Linus Walleij

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

[Sascha Hauer]

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@pengutronix.de> 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.

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 |

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

[Kent Gibson]

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@pengutronix.de> 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.

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

[Sascha Hauer]

On Wed, Jan 25, 2023 at 02:56:12PM +0100, Bartosz Golaszewski wrote:
> On Wed, Jan 25, 2023 at 10:35 AM Sascha Hauer <sha@pengutronix.de> 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@pengutronix.de> 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.
> >
> 
> History knows interfaces that were long deprecated, triggered warnings
> and were eventually removed (sysctl?). We have not yet broken
> user-space. IMO a warning is in order if an interface was deprecated.
> 
> > 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.
> >
> 
> We're also doing it step-by-step.

My suggestion is to keep the sysfs API, not to remove it. Since the
Kernel now issues a warning when a GPIO driver is trying to provide
fixed GPIO numbers and that motivates people to get rid of the warning.

And I just found out this is already happening:

https://lore.kernel.org/lkml/Y8WF2gXbrGnDLQ+S@atomide.com/T/

I can understand that you are trying to push people to use the new API,
but please do this by making the new API superior to the old API, not by
actively breaking the old API. And adding a warning like this *is* actively
breaking the old API for users.

> 
> > 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. 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
> 
> On that note: as libgpiod v2.0 is almost ready for release, I started
> working on the idea I've long had in mind - DBus interface for GPIOs.
> I've started writing the code over Christmas and some very limited
> functionality is available on my github[1]. For now it only allows to
> read chip properties but my goal is to have a complete DBus API for
> libgpiod v2.1. Is this something you could work with? It addresses
> your main concern (having a central authority and persistent state of
> GPIOs) while using the character device behind the scenes.

I do not understand why we should have such a complexity just to
maintain the status of a GPIO when the Kernel could simply do this.

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 |

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

[Sascha Hauer]

On Thu, Jan 26, 2023 at 10:35:22AM +0100, Linus Walleij wrote:
> On Wed, Jan 25, 2023 at 10:35 AM Sascha Hauer <sha@pengutronix.de> wrote:
> 
> > 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 see it more as permanent deprecation and nudging than removal as of now.

I am fine with permanent nudging. The sysfs API is deprecated and it's
ok to remind people that there's a new API they should use instead, but
as said, please don't actively break this API. Well to be correct,
adding this warning is not breaking the API by itself, instead it
motivates other people to do so, so you're getting there even without
making your own hands dirty ;)

> 
> For some reason people perceive all nudging as militant and as a
> my-way-or-the-highway style of communication but it's not really intended.
> OK maybe I am softer now than in the past, one grow humbler with
> old age.
> 
> It will become more interesting once we removed all in-kernel users of
> the global numberspace, which is well underway. At that point we can
> move the management of the global numberspace into the sysfs code
> and distros etc that don't want to use it can compile it out completely.

Perfectly fine with me. As I said: Push the sysfs API into a corner
where it hurts nobody let it stay there.

> 
> But the idea was never to slam people to not use something they use and like,
> it was to offer something new because they want it and like it more.
> 
> So using the character device with libgpiod users can:
> 
> - Get and set multiple lines at the same time
> - Do biasing (if supported by HW) pull up/down
> - Do drive strength (if supported by HW)
> - Properly listen to IRQ-driven events on lines with real-time timestamps
>   to ensure strict event order
> - Use HTE timestamps (new feature!)
> - libgpiod for the above with C, C++, Python and Rust bindings
> 
> By offering those new tasty features only for the character device and not
> for the sysfs ABI, at least the advanced users are expected to switch over
> to using the character device.

Ack, perfectly fine with me. Providing new features only for the new API
is a good way of gently pushing people towards the new API.

What's missing is a way to let a GPIO stay in the current state when I
release a GPIO chip. Unlike the new features you listed above this is a
feature that the sysfs API offers and that's important for us.

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 |

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

[Robert Schwebel]

On Thu, Jan 26, 2023 at 11:49:27AM +0100, Sascha Hauer wrote:
> What's missing is a way to let a GPIO stay in the current state when I
> release a GPIO chip. Unlike the new features you listed above this is a
> feature that the sysfs API offers and that's important for us.

An example where it is used is labgrid: our test automation controller
(LXA-TAC) doesn't run any software for controlling power of the device-
under-test; to switch on a DuT, labgrid does

  ssh tac echo 1 > /sys/some/path/to/gpio

While this could also be done with a daemon offering a dbus api, this
would be significantly more complex. In a critical environment, one
needs to make sure that the daemon process never fails, otherwhise the
power of the DuT would maybe be in a random state. Then of course one
can add a watchdog, but with the current sysfs interface it's really
simple. Of course that would also work if the new interface would offer
a "keep this line as it is" feature, but adding a dbus daemon just for
keeping the state of a pin sounds overcomplex when the kernel could also
provide that functionality.

Another example that came up on friday when we talked about this is a
motor for an airplane: It doesn't have only one "safe state" it could
fall back to if something fails (i.e. daemon disappears). The safe state
on power-on (with uninitialized external hardware) might be different
from the one on the ground (motor-off) or while being in the air
(motor-on). Of course one would probably not build an airplane without
further safety mechanics, but we have several less-desasterous-but-
still-very-expensive-in-the-case-of-failure use cases in the field, like
multi hundret kilowatt motors in agricultural or heavy construction
machine equipment being switched on/off by a GPIO that cause significant
loss of material / work on failure.

I hope those examples help a bit to understand the issues. As Sascha
said: when the new interface provides the same features sysfs offers
today, without adding tons of new complexity, increasing the pressure on
people to move there is perfectly fine. 

rsc
-- 
Pengutronix e.K.                           | Dipl.-Ing. Robert Schwebel  |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

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

[Linus Walleij]

On Sun, Jan 29, 2023 at 7:33 PM Robert Schwebel
<r.schwebel@pengutronix.de> wrote:

> While this could also be done with a daemon offering a dbus api, this
> would be significantly more complex. In a critical environment, one
> needs to make sure that the daemon process never fails, otherwhise the
> power of the DuT would maybe be in a random state. Then of course one
> can add a watchdog, but with the current sysfs interface it's really
> simple. Of course that would also work if the new interface would offer
> a "keep this line as it is" feature, but adding a dbus daemon just for
> keeping the state of a pin sounds overcomplex when the kernel could also
> provide that functionality.

One issue we face as developers is scaleability. Things that
seem straight forward on a single board computer in a lab get
really complex in a big system with man GPIO chips.

One of the big dangers of the sysfs ABI is that it is dependent on
probe order which the kernel sadly does not really guarantee.

It's a bit like /dev/sda /dev/sdb after you boot up a system with
two USB drives, certainly you know which one is sda and sdb
if you plug them in by hand, but if you just boot the system with
both in, what is the kernel expected to do, and what is expected
to happen? What happens in practice is first come first serve...

This means a script can work for years, and then some random
day a PCI device is sleepy and comes after some other device,
and the numbers of all GPIOs are shuffled.

However, if you feel safe about that, for example if there is only
one GPIO chip on the entire system so there will only ever be
gpiochip0, what do you think about my debugfs proposal?

The below is cut and paste from the drivers/gpio/TODO file:

-------------------------------
Debugfs in place of sysfs

The old sysfs code that enables simple uses of GPIOs from the
command line is still popular despite the existence of the proper
character device. The reason is that it is simple to use on
root filesystems where you only have a minimal set of tools such
as "cat", "echo" etc.

The old sysfs still need to be strongly deprecated and removed
as it relies on the global GPIO numberspace that assume a strict
order of global GPIO numbers that do not change between boots
and is independent of probe order.

To solve this and provide an ABI that people can use for hacks
and development, implement a debugfs interface to manipulate
GPIO lines that can do everything that sysfs can do today: one
directory per gpiochip and one file entry per line:

/sys/kernel/debug/gpiochip/gpiochip0
/sys/kernel/debug/gpiochip/gpiochip0/gpio0
/sys/kernel/debug/gpiochip/gpiochip0/gpio1
/sys/kernel/debug/gpiochip/gpiochip0/gpio2
/sys/kernel/debug/gpiochip/gpiochip0/gpio3
...
/sys/kernel/debug/gpiochip/gpiochip1
/sys/kernel/debug/gpiochip/gpiochip1/gpio0
/sys/kernel/debug/gpiochip/gpiochip1/gpio1
...

The exact files and design of the debugfs interface can be
discussed but the idea is to provide a low-level access point
for debugging and hacking and to expose all lines without the
need of any exporting. Also provide ample ammunition to shoot
oneself in the foot, because this is debugfs after all.
-------------------------------

Yours,
Linus Walleij

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

[Marco Felsch]

Hi Linus,

On 23-01-30, Linus Walleij wrote:
> On Sun, Jan 29, 2023 at 7:33 PM Robert Schwebel
> <r.schwebel@pengutronix.de> wrote:
> 
> > While this could also be done with a daemon offering a dbus api, this
> > would be significantly more complex. In a critical environment, one
> > needs to make sure that the daemon process never fails, otherwhise the
> > power of the DuT would maybe be in a random state. Then of course one
> > can add a watchdog, but with the current sysfs interface it's really
> > simple. Of course that would also work if the new interface would offer
> > a "keep this line as it is" feature, but adding a dbus daemon just for
> > keeping the state of a pin sounds overcomplex when the kernel could also
> > provide that functionality.
> 
> One issue we face as developers is scaleability. Things that
> seem straight forward on a single board computer in a lab get
> really complex in a big system with man GPIO chips.
> 
> One of the big dangers of the sysfs ABI is that it is dependent on
> probe order which the kernel sadly does not really guarantee.

Does it? At least the drive I listed (e.g. the imx gpio driver) uses
aliases to make it reliable.

Regards,
  Marco

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

[Linus Walleij]

On Mon, Jan 30, 2023 at 12:02 PM Marco Felsch <m.felsch@pengutronix.de> wrote:
> On 23-01-30, Linus Walleij wrote:
> > On Sun, Jan 29, 2023 at 7:33 PM Robert Schwebel
> > <r.schwebel@pengutronix.de> wrote:
> >
> > > While this could also be done with a daemon offering a dbus api, this
> > > would be significantly more complex. In a critical environment, one
> > > needs to make sure that the daemon process never fails, otherwhise the
> > > power of the DuT would maybe be in a random state. Then of course one
> > > can add a watchdog, but with the current sysfs interface it's really
> > > simple. Of course that would also work if the new interface would offer
> > > a "keep this line as it is" feature, but adding a dbus daemon just for
> > > keeping the state of a pin sounds overcomplex when the kernel could also
> > > provide that functionality.
> >
> > One issue we face as developers is scaleability. Things that
> > seem straight forward on a single board computer in a lab get
> > really complex in a big system with man GPIO chips.
> >
> > One of the big dangers of the sysfs ABI is that it is dependent on
> > probe order which the kernel sadly does not really guarantee.
>
> Does it? At least the drive I listed (e.g. the imx gpio driver) uses
> aliases to make it reliable.

I'm not sure that is the intended use of the aliases in device tree.
(Rob can maybe answer this.)

Besides, the problem exist also in ACPI (think every x86
laptop) which does not have anything like the alias mechanism
AFAIK. If it does, Andy will tell us.

Yours,
Linus Walleij

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

[Rob Herring]

On Mon, Jan 30, 2023 at 9:02 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Mon, Jan 30, 2023 at 12:02 PM Marco Felsch <m.felsch@pengutronix.de> wrote:
> > On 23-01-30, Linus Walleij wrote:
> > > On Sun, Jan 29, 2023 at 7:33 PM Robert Schwebel
> > > <r.schwebel@pengutronix.de> wrote:
> > >
> > > > While this could also be done with a daemon offering a dbus api, this
> > > > would be significantly more complex. In a critical environment, one
> > > > needs to make sure that the daemon process never fails, otherwhise the
> > > > power of the DuT would maybe be in a random state. Then of course one
> > > > can add a watchdog, but with the current sysfs interface it's really
> > > > simple. Of course that would also work if the new interface would offer
> > > > a "keep this line as it is" feature, but adding a dbus daemon just for
> > > > keeping the state of a pin sounds overcomplex when the kernel could also
> > > > provide that functionality.
> > >
> > > One issue we face as developers is scaleability. Things that
> > > seem straight forward on a single board computer in a lab get
> > > really complex in a big system with man GPIO chips.
> > >
> > > One of the big dangers of the sysfs ABI is that it is dependent on
> > > probe order which the kernel sadly does not really guarantee.
> >
> > Does it? At least the drive I listed (e.g. the imx gpio driver) uses
> > aliases to make it reliable.
>
> I'm not sure that is the intended use of the aliases in device tree.
> (Rob can maybe answer this.)

The kernel community's position on stable device numbers for userspace
is quite clear.

That is how aliases get used by the kernel though. IMO, they should be
limited to a documented set of alias names, and GPIO is not one of
them. We replaced the whole GPIO sysfs API for this reason (among
others).

IIRC, i.MX is one of the worst abusers with aliases for everything,
most of which should be removed.

Rob

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

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1644 bytes --]

On Mon, Jan 30, 2023 at 11:19:11AM +0100, Linus Walleij wrote:
> On Sun, Jan 29, 2023 at 7:33 PM Robert Schwebel
> <r.schwebel@pengutronix.de> wrote:
> 
> > While this could also be done with a daemon offering a dbus api, this
> > would be significantly more complex. In a critical environment, one
> > needs to make sure that the daemon process never fails, otherwhise the
> > power of the DuT would maybe be in a random state. Then of course one
> > can add a watchdog, but with the current sysfs interface it's really
> > simple. Of course that would also work if the new interface would offer
> > a "keep this line as it is" feature, but adding a dbus daemon just for
> > keeping the state of a pin sounds overcomplex when the kernel could also
> > provide that functionality.
> 
> One issue we face as developers is scaleability. Things that
> seem straight forward on a single board computer in a lab get
> really complex in a big system with man GPIO chips.

This is the point where the discussion took a wrong turn.

Yes, there is awareness that the sysfs API has a downside (here: on some
platforms the number allocation is not stable).

But the problem here is that the alternative (i.e. using the newer
devctrl API) also has a downside that makes it unsuitable (or overly
complex) to use for some workflows.

Just an idea: Wouldn't a nice solution be to make it possible to opt-out
of the reset to the safe state after use?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Bartosz Golaszewski]

On Mon, Jan 30, 2023 at 5:48 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> On Mon, Jan 30, 2023 at 11:19:11AM +0100, Linus Walleij wrote:
> > On Sun, Jan 29, 2023 at 7:33 PM Robert Schwebel
> > <r.schwebel@pengutronix.de> wrote:
> >
> > > While this could also be done with a daemon offering a dbus api, this
> > > would be significantly more complex. In a critical environment, one
> > > needs to make sure that the daemon process never fails, otherwhise the
> > > power of the DuT would maybe be in a random state. Then of course one
> > > can add a watchdog, but with the current sysfs interface it's really
> > > simple. Of course that would also work if the new interface would offer
> > > a "keep this line as it is" feature, but adding a dbus daemon just for
> > > keeping the state of a pin sounds overcomplex when the kernel could also
> > > provide that functionality.
> >
> > One issue we face as developers is scaleability. Things that
> > seem straight forward on a single board computer in a lab get
> > really complex in a big system with man GPIO chips.
>
> This is the point where the discussion took a wrong turn.
>
> Yes, there is awareness that the sysfs API has a downside (here: on some
> platforms the number allocation is not stable).
>
> But the problem here is that the alternative (i.e. using the newer
> devctrl API) also has a downside that makes it unsuitable (or overly
> complex) to use for some workflows.
>
> Just an idea: Wouldn't a nice solution be to make it possible to opt-out
> of the reset to the safe state after use?
>

This is a misconception though. There IS NO safe-state. This is
entirely driver-dependent. When you release a line, its state is no
longer defined by design because it SHOULD be driven by the user of
the line for AS LONG as they keep it.

Sysfs interface is in fact just an in-kernel user and in that regard
is no different from a user-space dbus daemon or interactive gpioset
except for existing in kernel-space. It's no more immune to code bugs
either so I find the argument about the daemon crashing invalid. Your
program setting sysfs entries may crash leaving the line inaccessible
to another valid user. Sysfs code may break - especially as it's no
longer maintained as actively as before. When you unexport a line in
sysfs - its state is no more defined than when releasing it using the
character device.

The so-called safe-state is something that apparently comes back every
3 years (according to Rob) for both user- and kernel-space users but
so far nobody has figured out a good way of implementing it. One of
these days...

Bart

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

[Andy Shevchenko]

On Mon, Jan 30, 2023 at 04:01:49PM +0100, Linus Walleij wrote:
> On Mon, Jan 30, 2023 at 12:02 PM Marco Felsch <m.felsch@pengutronix.de> wrote:
> > On 23-01-30, Linus Walleij wrote:
> > > On Sun, Jan 29, 2023 at 7:33 PM Robert Schwebel
> > > <r.schwebel@pengutronix.de> wrote:
> > >
> > > > While this could also be done with a daemon offering a dbus api, this
> > > > would be significantly more complex. In a critical environment, one
> > > > needs to make sure that the daemon process never fails, otherwhise the
> > > > power of the DuT would maybe be in a random state. Then of course one
> > > > can add a watchdog, but with the current sysfs interface it's really
> > > > simple. Of course that would also work if the new interface would offer
> > > > a "keep this line as it is" feature, but adding a dbus daemon just for
> > > > keeping the state of a pin sounds overcomplex when the kernel could also
> > > > provide that functionality.

Kernel provides it with a cost of a lot of downsides. That's why we moved to
character device approach which is already complex enough. And yeah, the best
what we can do now to support persistent states is to run a daemon.

This also solves the potential security issue (wrong process to access GPIO)
with legacy interface.

No doubt we need to kill sysfs GPIO ABI.

> > > One issue we face as developers is scaleability. Things that
> > > seem straight forward on a single board computer in a lab get
> > > really complex in a big system with man GPIO chips.
> > >
> > > One of the big dangers of the sysfs ABI is that it is dependent on
> > > probe order which the kernel sadly does not really guarantee.
> >
> > Does it? At least the drive I listed (e.g. the imx gpio driver) uses
> > aliases to make it reliable.
> 
> I'm not sure that is the intended use of the aliases in device tree.
> (Rob can maybe answer this.)
> 
> Besides, the problem exist also in ACPI (think every x86
> laptop) which does not have anything like the alias mechanism
> AFAIK. If it does, Andy will tell us.

We never rely on the pure numbers on ACPI based platforms. Yeah, we have couple
of drivers contradict this, but because of wrong numbers in the ACPI tables
(BIOS bugs) which should have not happened to begin with.

And yes, we don't have aliases because we don't need them. You can always
access GPIO pins based on the naming of the controller + relative number of
line on it. I do not understand why you chose aliases approach instead of
making your scripts more robust based on the other information available.

-- 
With Best Regards,
Andy Shevchenko

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

[Linus Walleij]

On Mon, Jan 30, 2023 at 5:48 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Mon, Jan 30, 2023 at 11:19:11AM +0100, Linus Walleij wrote:
> > On Sun, Jan 29, 2023 at 7:33 PM Robert Schwebel
> > <r.schwebel@pengutronix.de> wrote:
> >
> > > While this could also be done with a daemon offering a dbus api, this
> > > would be significantly more complex. In a critical environment, one
> > > needs to make sure that the daemon process never fails, otherwhise the
> > > power of the DuT would maybe be in a random state. Then of course one
> > > can add a watchdog, but with the current sysfs interface it's really
> > > simple. Of course that would also work if the new interface would offer
> > > a "keep this line as it is" feature, but adding a dbus daemon just for
> > > keeping the state of a pin sounds overcomplex when the kernel could also
> > > provide that functionality.
> >
> > One issue we face as developers is scaleability. Things that
> > seem straight forward on a single board computer in a lab get
> > really complex in a big system with man GPIO chips.
>
> This is the point where the discussion took a wrong turn.
>
> Yes, there is awareness that the sysfs API has a downside (here: on some
> platforms the number allocation is not stable).
>
> But the problem here is that the alternative (i.e. using the newer
> devctrl API) also has a downside that makes it unsuitable (or overly
> complex) to use for some workflows.

That should make it possible to use my suggested debugfs facility
that provide the same, but does not use the global numberspace.

People who don't care about the complexity involved with the character
device certainly will not care about the downsides of using debugfs in
production either. (Such as interfering with any chardev users...)

> Just an idea: Wouldn't a nice solution be to make it possible to opt-out
> of the reset to the safe state after use?

As Bartosz says there is no reset to any safe state, whatever that
would be. The state is kept in hardware just like with sysfs.

You can even set up the state from sysfs and then read it back
from the character device or vice versa, after freeing each
hogs resources.

It's really just the .get_direction() and .get() callbacks on the
gpio_chip that read the state, .set_direction_input|output()
and .set() sets it, like they always did.

Consider the example too tools/gpio/gpio-hammer.c
that I wrote. It reads the initial line values of the GPIO lines
(from hardware) and then start to invert them. It doesn't start
from any specific state?

Yours,
Linus Walleij

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

[Alexander Stein]

Hi Rob,

Am Montag, 30. Januar 2023, 16:45:57 CET schrieb Rob Herring:
> On Mon, Jan 30, 2023 at 9:02 AM Linus Walleij <linus.walleij@linaro.org> 
wrote:
> > On Mon, Jan 30, 2023 at 12:02 PM Marco Felsch <m.felsch@pengutronix.de> 
wrote:
> > > On 23-01-30, Linus Walleij wrote:
> > > > On Sun, Jan 29, 2023 at 7:33 PM Robert Schwebel
> > > > 
> > > > <r.schwebel@pengutronix.de> wrote:
> > > > > While this could also be done with a daemon offering a dbus api,
> > > > > this
> > > > > would be significantly more complex. In a critical environment, one
> > > > > needs to make sure that the daemon process never fails, otherwhise
> > > > > the
> > > > > power of the DuT would maybe be in a random state. Then of course
> > > > > one
> > > > > can add a watchdog, but with the current sysfs interface it's really
> > > > > simple. Of course that would also work if the new interface would
> > > > > offer
> > > > > a "keep this line as it is" feature, but adding a dbus daemon just
> > > > > for
> > > > > keeping the state of a pin sounds overcomplex when the kernel could
> > > > > also
> > > > > provide that functionality.
> > > > 
> > > > One issue we face as developers is scaleability. Things that
> > > > seem straight forward on a single board computer in a lab get
> > > > really complex in a big system with man GPIO chips.
> > > > 
> > > > One of the big dangers of the sysfs ABI is that it is dependent on
> > > > probe order which the kernel sadly does not really guarantee.
> > > 
> > > Does it? At least the drive I listed (e.g. the imx gpio driver) uses
> > > aliases to make it reliable.
> > 
> > I'm not sure that is the intended use of the aliases in device tree.
> > (Rob can maybe answer this.)
> 
> The kernel community's position on stable device numbers for userspace
> is quite clear.

Sorry for this stupid question: What position is it? Is it documented 
somewhere?

> That is how aliases get used by the kernel though. IMO, they should be
> limited to a documented set of alias names, and GPIO is not one of
> them. We replaced the whole GPIO sysfs API for this reason (among
> others).

So in which case, or under which circumstances, aliases are acceptable?

Best regards,
Alexander

> IIRC, i.MX is one of the worst abusers with aliases for everything,
> most of which should be removed.
> 
> Rob

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

[Kent Gibson]

I didn't respond to this earlier, as I considered it best to let the
thread die and not inflame the issue.

Adding Andy as he raised the issue again in another thread.

On Sun, Jan 29, 2023 at 07:33:39PM +0100, Robert Schwebel wrote:
> On Thu, Jan 26, 2023 at 11:49:27AM +0100, Sascha Hauer wrote:
> > What's missing is a way to let a GPIO stay in the current state when I
> > release a GPIO chip. Unlike the new features you listed above this is a
> > feature that the sysfs API offers and that's important for us.
> 

I assume you mean release a line...

The new (cdev) interface provides access control over lines so you can
guarantee that you, and only you, have control over the line.
This is not possible with sysfs.

With cdev, and internally with gpiolib, the state is only guaranteed while
the line has an owner, and when you release line it becomes unowned -
ownership effectively reverts to the GPIO chip driver.
Its state does not become random, it becomes undefined from the
perspective of the former owner.  Neither cdev nor gpiolib change the
line state when the line is released - any changes at that point are a
function of the GPIO chip driver.

(With sysfs, it becomes the owner of exported lines.)

> An example where it is used is labgrid: our test automation controller
> (LXA-TAC) doesn't run any software for controlling power of the device-
> under-test; to switch on a DuT, labgrid does
> 
>   ssh tac echo 1 > /sys/some/path/to/gpio
> 

As can anything else that can access /sys/some/path/to/gpio.
So it is possible for multiple test benches to mess with the DuT power at
the same time.  Using cdev would make that impossible.

cdev providing access control does mean that it is more awkward to use
than sysfs, but to provide a "set and forget" feature akin to sysfs would
undermine that access control and the associated benefits.

> While this could also be done with a daemon offering a dbus api, this
> would be significantly more complex. In a critical environment, one
> needs to make sure that the daemon process never fails, otherwhise the
> power of the DuT would maybe be in a random state. Then of course one
> can add a watchdog, but with the current sysfs interface it's really
> simple. Of course that would also work if the new interface would offer
> a "keep this line as it is" feature, but adding a dbus daemon just for
> keeping the state of a pin sounds overcomplex when the kernel could also
> provide that functionality.
> 

So it is non-trivial for your test framework to run a process on a
remote host for the duration of a test?
That is a problem.
Seems like something that would be generally useful, and having it run
gpioset for the duration of a test *should* be pretty trivial.

Unlike sysfs, when you request a line with cdev nothing else can come
along and mess with your power switch while your test is using it.
Which seems like something you might want in a critical environment.

Wrt complexity, I would contend that it is actually more work to add
this functionality to the kernel.  The state of GPIO lines when not held
by gpiolib is dependent on the GPIO chip driver, so solving the problem
for all cases would involve extending the GPIO chip driver API and
updating *all* GPIO chip drivers to keep the state of the pin as
requested - even when the line is freed.
Even if this were to happen, it would take significant time and effort,
so it would be more practical to try to find a solution that uses cdev
as it stands.

> Another example that came up on friday when we talked about this is a
> motor for an airplane: It doesn't have only one "safe state" it could
> fall back to if something fails (i.e. daemon disappears). The safe state
> on power-on (with uninitialized external hardware) might be different
> from the one on the ground (motor-off) or while being in the air
> (motor-on). Of course one would probably not build an airplane without
> further safety mechanics, but we have several less-desasterous-but-
> still-very-expensive-in-the-case-of-failure use cases in the field, like
> multi hundret kilowatt motors in agricultural or heavy construction
> machine equipment being switched on/off by a GPIO that cause significant
> loss of material / work on failure.
> 

Agreed - such applications should have external safety mechanics, so
the software safe state is irrelevant.

> I hope those examples help a bit to understand the issues. As Sascha
> said: when the new interface provides the same features sysfs offers
> today, without adding tons of new complexity, increasing the pressure on
> people to move there is perfectly fine. 
> 

Can you elaborate on "adding tons of new complexity"?
You mean that you can't drive GPIOs just by echoing into files?

I don't see any GPIO developers "pressuring" anyone to switch.
The standard process to remove old interfaces is to mark them as
deprecated and provide time for users to migrate to alternatives.
This is exactly what has been done.
The sysfs interface has been deprecated for a very long time (since
2015??).
And the deprecation suggested it was scheduled for removal in 2020.
Exactly how much warning is required for you not to feel "pressured"?

We well understand that there are trade-offs involved in the switch from
sysfs to cdev, and have endeavoured to accomodate sysfs users.
The problem as I see it is sysfs users who continue to insist on feature
parity while not appreciating the associated costs.

Cheers,
Kent.

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

[Andy Shevchenko]

On Thu, Mar 02, 2023 at 10:19:03AM +0800, Kent Gibson wrote:
> I didn't respond to this earlier, as I considered it best to let the
> thread die and not inflame the issue.
> 
> Adding Andy as he raised the issue again in another thread.

Thank you, Kent, it is helpful.

-- 
With Best Regards,
Andy Shevchenko