* setting gpio-line-names in dts for sama5d2 SoC
@ 2021-03-16 13:07 Alexander Dahl
[not found] ` <CAHp75Vf05NN0dXUrMSOXBRuYRnQRHO_92itZ3ndOyX1oERWt=g@mail.gmail.com>
0 siblings, 1 reply; 7+ messages in thread
From: Alexander Dahl @ 2021-03-16 13:07 UTC (permalink / raw)
To: linux-gpio
Hei hei,
for a Microchip SAMA5D2 SoC based device I try to set gpio-line-names in .dts file like this:
257 &pioA {
258 gpio-line-names = "", /* PA0 */
259 "", /* PA1 */
260 "", /* PA2 */
261 "FOO", /* PA3 */
262 "BAR", /* PA4 */
263 "", /* PA5 */
… and so on for all 4 * 32 GPIO Pins. However when calling `gpioinfo` in userspace, I always get this:
$ gpioinfo
gpiochip0 - 128 lines:
line 0: "PA0" unused input active-high
line 1: "PA1" unused input active-high
line 2: "PA2" unused input active-high
line 3: "PA3" unused input active-high
line 4: "PA4" unused input active-high
line 5: "PA5" unused input active-high
… and so on. Those "PA0" line names are set by the pinctrl driver in drivers/pinctrl/pinctrl-at91-pio4.c before calling gpiochip_add_data() and from reading the code in drivers/gpio/gpiolib.c I suspect devprop_gpiochip_set_names() is never called then, so those names in .dts are simply ignored.
Those default names from the at91 pio4 pinctrl driver are certainly correct speaking of pin names, but from a userspace point of view it would be better if I could override those from dts, so an application using libgpiod could look for a name like e.g. "VALVE7_EN" without caring to which pin that's actually connected. Can I override those with a currently present kernel? Or is it a problem in either the at91 pio4 driver or the gpiolib core? Or is that no real usecase and should I do it differently?
Greets
Alex
^ permalink raw reply [flat|nested] 7+ messages in thread[parent not found: <CAHp75Vf05NN0dXUrMSOXBRuYRnQRHO_92itZ3ndOyX1oERWt=g@mail.gmail.com>]
* Re: setting gpio-line-names in dts for sama5d2 SoC [not found] ` <CAHp75Vf05NN0dXUrMSOXBRuYRnQRHO_92itZ3ndOyX1oERWt=g@mail.gmail.com> @ 2021-03-17 12:33 ` Andy Shevchenko 2021-03-18 9:50 ` Alexander Dahl 0 siblings, 1 reply; 7+ messages in thread From: Andy Shevchenko @ 2021-03-17 12:33 UTC (permalink / raw) To: Alexander Dahl; +Cc: linux-gpio, Bartosz Golaszewski, Linus Walleij +Cc: Bart, Linus On Wed, Mar 17, 2021 at 1:22 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > > > On Tuesday, March 16, 2021, Alexander Dahl <ada@thorsis.com> wrote: >> >> Hei hei, >> >> for a Microchip SAMA5D2 SoC based device I try to set gpio-line-names in .dts file like this: >> >> 257 &pioA { >> 258 gpio-line-names = "", /* PA0 */ >> 259 "", /* PA1 */ >> 260 "", /* PA2 */ >> 261 "FOO", /* PA3 */ >> 262 "BAR", /* PA4 */ >> 263 "", /* PA5 */ >> >> … and so on for all 4 * 32 GPIO Pins. However when calling `gpioinfo` in userspace, I always get this: >> >> $ gpioinfo >> gpiochip0 - 128 lines: >> line 0: "PA0" unused input active-high >> line 1: "PA1" unused input active-high >> line 2: "PA2" unused input active-high >> line 3: "PA3" unused input active-high >> line 4: "PA4" unused input active-high >> line 5: "PA5" unused input active-high >> >> … and so on. Those "PA0" line names are set by the pinctrl driver in drivers/pinctrl/pinctrl-at91-pio4.c before calling gpiochip_add_data() and from reading the code in drivers/gpio/gpiolib.c I suspect devprop_gpiochip_set_names() is never called then, so those names in .dts are simply ignored. >> >> Those default names from the at91 pio4 pinctrl driver are certainly correct speaking of pin names, but from a userspace point of view it would be better if I could override those from dts, so an application using libgpiod could look for a name like e.g. "VALVE7_EN" without caring to which pin that's actually connected. Can I override those with a currently present kernel? Or is it a problem in either the at91 pio4 driver or the gpiolib core? Or is that no real usecase and should I do it differently? > > > I would like to know the consequences if allow this, but to me it sounds like a bug in the gpiolib.c. > >> >> Greets >> Alex > > > > -- > With Best Regards, > Andy Shevchenko > > -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: setting gpio-line-names in dts for sama5d2 SoC 2021-03-17 12:33 ` Andy Shevchenko @ 2021-03-18 9:50 ` Alexander Dahl 2021-03-18 12:40 ` Andy Shevchenko 0 siblings, 1 reply; 7+ messages in thread From: Alexander Dahl @ 2021-03-18 9:50 UTC (permalink / raw) To: Andy Shevchenko; +Cc: linux-gpio, Bartosz Golaszewski, Linus Walleij Hei hei, meanwhile I tried to proof my hypothesis. See below. > Andy Shevchenko <andy.shevchenko@gmail.com> hat am 17.03.2021 13:33 geschrieben: > > > +Cc: Bart, Linus > > On Wed, Mar 17, 2021 at 1:22 AM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > > > > > > > On Tuesday, March 16, 2021, Alexander Dahl <ada@thorsis.com> wrote: > >> > >> Hei hei, > >> > >> for a Microchip SAMA5D2 SoC based device I try to set gpio-line-names in .dts file like this: > >> > >> 257 &pioA { > >> 258 gpio-line-names = "", /* PA0 */ > >> 259 "", /* PA1 */ > >> 260 "", /* PA2 */ > >> 261 "FOO", /* PA3 */ > >> 262 "BAR", /* PA4 */ > >> 263 "", /* PA5 */ > >> > >> … and so on for all 4 * 32 GPIO Pins. However when calling `gpioinfo` in userspace, I always get this: > >> > >> $ gpioinfo > >> gpiochip0 - 128 lines: > >> line 0: "PA0" unused input active-high > >> line 1: "PA1" unused input active-high > >> line 2: "PA2" unused input active-high > >> line 3: "PA3" unused input active-high > >> line 4: "PA4" unused input active-high > >> line 5: "PA5" unused input active-high > >> > >> … and so on. Those "PA0" line names are set by the pinctrl driver in drivers/pinctrl/pinctrl-at91-pio4.c before calling gpiochip_add_data() and from reading the code in drivers/gpio/gpiolib.c I suspect devprop_gpiochip_set_names() is never called then, so those names in .dts are simply ignored. > >> > >> Those default names from the at91 pio4 pinctrl driver are certainly correct speaking of pin names, but from a userspace point of view it would be better if I could override those from dts, so an application using libgpiod could look for a name like e.g. "VALVE7_EN" without caring to which pin that's actually connected. Can I override those with a currently present kernel? I don't think I can override with a recent kernel. >>> Or is it a problem in either the at91 pio4 driver or the gpiolib core? Or is that no real usecase and should I do it differently? If I disable the name assignment in the pinctrl-at91-pio4 driver, I get the line names from the dts. I used the following diff: diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c b/drivers/pinctrl/pinctrl-at91-pio4.c index 578b387100d9..f994a2468cc3 100644 --- a/drivers/pinctrl/pinctrl-at91-pio4.c +++ b/drivers/pinctrl/pinctrl-at91-pio4.c @@ -1095,7 +1095,9 @@ static int atmel_pinctrl_probe(struct platform_device *pdev) atmel_pioctrl->gpio_chip->ngpio = atmel_pioctrl->npins; atmel_pioctrl->gpio_chip->label = dev_name(dev); atmel_pioctrl->gpio_chip->parent = dev; +#if 0 atmel_pioctrl->gpio_chip->names = atmel_pioctrl->group_names; +#endif atmel_pioctrl->pm_wakeup_sources = devm_kcalloc(dev, atmel_pioctrl->nbanks, Now gpioinfo shows what I set in dts, more or less similar to this: gpiochip0 - 128 lines: line 0: unnamed unused input active-high line 1: unnamed unused input active-high line 2: unnamed unused input active-high line 3: "FOO" unused input active-high line 4: "BAR" unused input active-high line 5: unnamed unused input active-high > > > > > > I would like to know the consequences if allow this, but to me it sounds like a bug in the gpiolib.c. At least the documentation recommends to not use pin names: https://www.kernel.org/doc/Documentation/devicetree/bindings/gpio/gpio.txt If anyone has an idea how to fix this, let me know. I can try to make a patch then. Currently however, I'm not familiar enough with the gpio subsystem to just start that. Greets Alex ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: setting gpio-line-names in dts for sama5d2 SoC 2021-03-18 9:50 ` Alexander Dahl @ 2021-03-18 12:40 ` Andy Shevchenko 2021-03-18 13:56 ` Alexander Dahl 0 siblings, 1 reply; 7+ messages in thread From: Andy Shevchenko @ 2021-03-18 12:40 UTC (permalink / raw) To: Alexander Dahl, Kent Gibson Cc: linux-gpio, Bartosz Golaszewski, Linus Walleij +Cc: Kent (sorry, forgot to include you earlier) On Thu, Mar 18, 2021 at 11:50 AM Alexander Dahl <ada@thorsis.com> wrote: > > Andy Shevchenko <andy.shevchenko@gmail.com> hat am 17.03.2021 13:33 geschrieben: > > On Wed, Mar 17, 2021 at 1:22 AM Andy Shevchenko > > <andy.shevchenko@gmail.com> wrote: > > > On Tuesday, March 16, 2021, Alexander Dahl <ada@thorsis.com> wrote: ... > > >> for a Microchip SAMA5D2 SoC based device I try to set gpio-line-names in .dts file like this: > > >> > > >> 257 &pioA { > > >> 258 gpio-line-names = "", /* PA0 */ > > >> 259 "", /* PA1 */ > > >> 260 "", /* PA2 */ > > >> 261 "FOO", /* PA3 */ > > >> 262 "BAR", /* PA4 */ > > >> 263 "", /* PA5 */ > > >> > > >> … and so on for all 4 * 32 GPIO Pins. However when calling `gpioinfo` in userspace, I always get this: > > >> > > >> $ gpioinfo > > >> gpiochip0 - 128 lines: > > >> line 0: "PA0" unused input active-high > > >> line 1: "PA1" unused input active-high > > >> line 2: "PA2" unused input active-high > > >> line 3: "PA3" unused input active-high > > >> line 4: "PA4" unused input active-high > > >> line 5: "PA5" unused input active-high > > >> > > >> … and so on. Those "PA0" line names are set by the pinctrl driver in drivers/pinctrl/pinctrl-at91-pio4.c before calling gpiochip_add_data() and from reading the code in drivers/gpio/gpiolib.c I suspect devprop_gpiochip_set_names() is never called then, so those names in .dts are simply ignored. > > >> > > >> Those default names from the at91 pio4 pinctrl driver are certainly correct speaking of pin names, but from a userspace point of view it would be better if I could override those from dts, so an application using libgpiod could look for a name like e.g. "VALVE7_EN" without caring to which pin that's actually connected. Can I override those with a currently present kernel? > > I don't think I can override with a recent kernel. > > >>> Or is it a problem in either the at91 pio4 driver or the gpiolib core? Or is that no real usecase and should I do it differently? > > If I disable the name assignment in the pinctrl-at91-pio4 driver, I get the line names from the dts. I used the following diff: ... > Now gpioinfo shows what I set in dts, more or less similar to this: > > gpiochip0 - 128 lines: > line 0: unnamed unused input active-high > line 1: unnamed unused input active-high > line 2: unnamed unused input active-high > line 3: "FOO" unused input active-high > line 4: "BAR" unused input active-high > line 5: unnamed unused input active-high > > > > I would like to know the consequences if allow this, but to me it sounds like a bug in the gpiolib.c. > > At least the documentation recommends to not use pin names: https://www.kernel.org/doc/Documentation/devicetree/bindings/gpio/gpio.txt > > If anyone has an idea how to fix this, let me know. I can try to make a patch then. Currently however, I'm not familiar enough with the gpio subsystem to just start that. You may look into the commit 7cba1a4d5e16 ("gpiolib: generalize devprop_gpiochip_set_names() for device properties") which unifies the logic, but at the same time removed one comment: - /* If the chip defines names itself, these take precedence */ So, logic was that for a long time. And unfortunately I don't see how we may change this without breakage. This backs us to the discussion if the name of the line is ABI or not. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: setting gpio-line-names in dts for sama5d2 SoC 2021-03-18 12:40 ` Andy Shevchenko @ 2021-03-18 13:56 ` Alexander Dahl 2021-03-18 14:59 ` Andy Shevchenko 0 siblings, 1 reply; 7+ messages in thread From: Alexander Dahl @ 2021-03-18 13:56 UTC (permalink / raw) To: Andy Shevchenko, Kent Gibson Cc: linux-gpio, Bartosz Golaszewski, Linus Walleij Hello Andy, > Andy Shevchenko <andy.shevchenko@gmail.com> hat am 18.03.2021 13:40 geschrieben: > You may look into the commit > 7cba1a4d5e16 ("gpiolib: generalize devprop_gpiochip_set_names() for > device properties") > which unifies the logic, but at the same time removed one comment: > > - /* If the chip defines names itself, these take precedence */ Well, that could be added again. And if that precedence stays as is, it should be documented in the gpio devicetree bindings to not cause further confusion? > So, logic was that for a long time. And unfortunately I don't see how > we may change this without breakage. This backs us to the discussion > if the name of the line is ABI or not. I want to point out one point for consideration. There's usually a difference between the SoC and the boards using that SoC. So those pins are all the same from the chips point of view and the default names are set in a driver related to the chip (family). However the line names set in device tree are probably related to a board. Obviously different boards may have different line names, even when using the same SoC. In other words: if I design a new board, there's no interface I would break, at least not from the userspace point of view just looking at the line names, or is there? Thanks for looking into this. Greets Alex ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: setting gpio-line-names in dts for sama5d2 SoC 2021-03-18 13:56 ` Alexander Dahl @ 2021-03-18 14:59 ` Andy Shevchenko 2021-03-20 11:20 ` Linus Walleij 0 siblings, 1 reply; 7+ messages in thread From: Andy Shevchenko @ 2021-03-18 14:59 UTC (permalink / raw) To: Alexander Dahl, Rob Herring Cc: Kent Gibson, linux-gpio, Bartosz Golaszewski, Linus Walleij On Thu, Mar 18, 2021 at 3:56 PM Alexander Dahl <ada@thorsis.com> wrote: > > Andy Shevchenko <andy.shevchenko@gmail.com> hat am 18.03.2021 13:40 geschrieben: > > You may look into the commit > > 7cba1a4d5e16 ("gpiolib: generalize devprop_gpiochip_set_names() for > > device properties") > > which unifies the logic, but at the same time removed one comment: > > > > - /* If the chip defines names itself, these take precedence */ > > Well, that could be added again. And if that precedence stays as is, it should be documented in the gpio devicetree bindings to not cause further confusion? > > > So, logic was that for a long time. And unfortunately I don't see how > > we may change this without breakage. This backs us to the discussion > > if the name of the line is ABI or not. > > I want to point out one point for consideration. There's usually a difference between the SoC and the boards using that SoC. So those pins are all the same from the chips point of view and the default names are set in a driver related to the chip (family). However the line names set in device tree are probably related to a board. Obviously different boards may have different line names, even when using the same SoC. > > In other words: if I design a new board, there's no interface I would break, at least not from the userspace point of view just looking at the line names, or is there? I understand that names are platform specific and I'm on your side as an IoT specialist, but on the Linux side we may not blindly do changes like this due to ABI concerns. You see the problem here: you can't fix all (legacy) DTSs in the world which do not have GPIO line names listed, and for those users we effectively break their tools and scripts which relies on the hard coded naming scheme. So, we may change if and only if the names of the lines are not an ABI! What we can do as a workaround is to reverse the partsing order, and if somebody complains, it will be their DTS issues, whoever not the best solution either. +Cc: Rob to hear his opinion. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: setting gpio-line-names in dts for sama5d2 SoC 2021-03-18 14:59 ` Andy Shevchenko @ 2021-03-20 11:20 ` Linus Walleij 0 siblings, 0 replies; 7+ messages in thread From: Linus Walleij @ 2021-03-20 11:20 UTC (permalink / raw) To: Andy Shevchenko Cc: Alexander Dahl, Rob Herring, Kent Gibson, linux-gpio, Bartosz Golaszewski On Thu, Mar 18, 2021 at 4:00 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Thu, Mar 18, 2021 at 3:56 PM Alexander Dahl <ada@thorsis.com> wrote: > > In other words: if I design a new board, there's no interface I would break, at least not from the userspace point of view just looking at the line names, or is there? > > I understand that names are platform specific and I'm on your side as > an IoT specialist, but on the Linux side we may not blindly do changes > like this due to ABI concerns. You see the problem here: you can't fix > all (legacy) DTSs in the world which do not have GPIO line names > listed, and for those users we effectively break their tools and > scripts which relies on the hard coded naming scheme. > > So, we may change if and only if the names of the lines are not an ABI! > > What we can do as a workaround is to reverse the partsing order, and > if somebody complains, it will be their DTS issues, whoever not the > best solution either. The line names are not really supposed to be used as ABI since they are not necessarily unique. This problem was pointed out by Johan a few times. The real ABI is the HW line number (local offset) when used with the GPIO character device. This is logical since the gpio-line-names attribute is optional. It must be possible to get a handle to a GPIO line without a name. And using the offset number should be the preferred way for userspace: use a number on a gpiochipN with the character device. (The topology of the gpiochip per se can be determined from sysfs.) However this has not stopped userspace from using it as ABI, and these names are used by the GPIO aggregator. It's hard to do it right without resetting history and requireing that all names in the device tree be unique as well, at the root of the problem, but when introduced, these names were there for the file /sys/kernel/debug/gpio, not for exposing to userspace. However the character device does give the name away, as a piece of information, it is not supposed to be used to look up lines but merely as information, but well people use it for looking up lines by name anyway so what can we do. I don't think it's a big deal to change these names. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-03-20 11:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-16 13:07 setting gpio-line-names in dts for sama5d2 SoC Alexander Dahl
[not found] ` <CAHp75Vf05NN0dXUrMSOXBRuYRnQRHO_92itZ3ndOyX1oERWt=g@mail.gmail.com>
2021-03-17 12:33 ` Andy Shevchenko
2021-03-18 9:50 ` Alexander Dahl
2021-03-18 12:40 ` Andy Shevchenko
2021-03-18 13:56 ` Alexander Dahl
2021-03-18 14:59 ` Andy Shevchenko
2021-03-20 11:20 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).