public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [PATCH] gpio: search gpio-line-names property in dm_gpio_lookup_name
@ 2025-10-07 10:26 Rasmus Villemoes
  2025-10-15 21:08 ` Tom Rini
  0 siblings, 1 reply; 3+ messages in thread
From: Rasmus Villemoes @ 2025-10-07 10:26 UTC (permalink / raw)
  To: u-boot; +Cc: Tom Rini, Rasmus Villemoes

In scripts as well as interactively, it's much nicer to be able to
refer to GPIOs via their names defined in the device tree property
"gpio-line-names", instead of the rather opaque names derived from the
bank name with a _xx suffix. E.g.

  gpio read factory_reset FACTORY_RESET
  if test $factory_reset = 1 ; then ...

versus

  gpio read factory_reset gpio@481ac000_16
  if test $factory_reset = 1 ; then ...

This is also consistent with the move on the linux/userspace side towards
using line names instead of legacy chip+offset or the even more legacy
global gpio numbering in sysfs.

As this seems to only add about ~50 bytes of code to U-Boot proper,
and dm_gpio_lookup_name() most often ends up being GC'ed for SPL, so
adds no overhead there, adding yet another config knob (or two, if it
also needed an SPL variant) for this does not seem warranted.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 drivers/gpio/gpio-uclass.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index 3d9f8b32b8d..6b357a2671b 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -164,7 +164,7 @@ int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc)
 	for (uclass_first_device(UCLASS_GPIO, &dev);
 	     dev;
 	     uclass_next_device(&dev)) {
-		int len;
+		int len, ret;
 
 		uc_priv = dev_get_uclass_priv(dev);
 		if (numeric != -1) {
@@ -188,6 +188,13 @@ int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc)
 		 */
 		if (!dm_gpio_lookup_label(name, uc_priv, &offset))
 			break;
+
+		/* Also search the "gpio-line-names" property in DT for a match. */
+		ret = dev_read_stringlist_search(dev, "gpio-line-names", name);
+		if (ret >= 0) {
+			offset = ret;
+			break;
+		}
 	}
 
 	if (!dev)
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] gpio: search gpio-line-names property in dm_gpio_lookup_name
  2025-10-07 10:26 [PATCH] gpio: search gpio-line-names property in dm_gpio_lookup_name Rasmus Villemoes
@ 2025-10-15 21:08 ` Tom Rini
  2025-10-16  9:17   ` Rasmus Villemoes
  0 siblings, 1 reply; 3+ messages in thread
From: Tom Rini @ 2025-10-15 21:08 UTC (permalink / raw)
  To: Rasmus Villemoes; +Cc: u-boot

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

On Tue, Oct 07, 2025 at 12:26:37PM +0200, Rasmus Villemoes wrote:

> In scripts as well as interactively, it's much nicer to be able to
> refer to GPIOs via their names defined in the device tree property
> "gpio-line-names", instead of the rather opaque names derived from the
> bank name with a _xx suffix. E.g.
> 
>   gpio read factory_reset FACTORY_RESET
>   if test $factory_reset = 1 ; then ...
> 
> versus
> 
>   gpio read factory_reset gpio@481ac000_16
>   if test $factory_reset = 1 ; then ...
> 
> This is also consistent with the move on the linux/userspace side towards
> using line names instead of legacy chip+offset or the even more legacy
> global gpio numbering in sysfs.
> 
> As this seems to only add about ~50 bytes of code to U-Boot proper,
> and dm_gpio_lookup_name() most often ends up being GC'ed for SPL, so
> adds no overhead there, adding yet another config knob (or two, if it
> also needed an SPL variant) for this does not seem warranted.
> 
> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>

On a few platforms we now get a failure to build:
       arm:  +   work_92105
+(work_92105) arm-linux-gnueabi-ld: drivers/gpio/gpio-uclass.o: in function `dev_read_stringlist_search':
+(work_92105) include/dm/read.h:1078:(.text.dm_gpio_lookup_name+0x8c): undefined reference to `ofnode_stringlist_search'
+(work_92105) make[1]: *** [Makefile:2029: u-boot] Error 1
+(work_92105) make: *** [Makefile:198: sub-make] Error 2

-- 
Tom

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] gpio: search gpio-line-names property in dm_gpio_lookup_name
  2025-10-15 21:08 ` Tom Rini
@ 2025-10-16  9:17   ` Rasmus Villemoes
  0 siblings, 0 replies; 3+ messages in thread
From: Rasmus Villemoes @ 2025-10-16  9:17 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot

On Wed, Oct 15 2025, Tom Rini <trini@konsulko.com> wrote:

> On Tue, Oct 07, 2025 at 12:26:37PM +0200, Rasmus Villemoes wrote:
>
>> In scripts as well as interactively, it's much nicer to be able to
>> refer to GPIOs via their names defined in the device tree property
>> "gpio-line-names", instead of the rather opaque names derived from the
>> bank name with a _xx suffix. E.g.
>> 
>>   gpio read factory_reset FACTORY_RESET
>>   if test $factory_reset = 1 ; then ...
>> 
>> versus
>> 
>>   gpio read factory_reset gpio@481ac000_16
>>   if test $factory_reset = 1 ; then ...
>> 
>> This is also consistent with the move on the linux/userspace side towards
>> using line names instead of legacy chip+offset or the even more legacy
>> global gpio numbering in sysfs.
>> 
>> As this seems to only add about ~50 bytes of code to U-Boot proper,
>> and dm_gpio_lookup_name() most often ends up being GC'ed for SPL, so
>> adds no overhead there, adding yet another config knob (or two, if it
>> also needed an SPL variant) for this does not seem warranted.
>> 
>> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
>
> On a few platforms we now get a failure to build:
>        arm:  +   work_92105
> +(work_92105) arm-linux-gnueabi-ld: drivers/gpio/gpio-uclass.o: in function `dev_read_stringlist_search':
> +(work_92105) include/dm/read.h:1078:(.text.dm_gpio_lookup_name+0x8c): undefined reference to `ofnode_stringlist_search'
> +(work_92105) make[1]: *** [Makefile:2029: u-boot] Error 1
> +(work_92105) make: *** [Makefile:198: sub-make] Error 2

Hm, interesting. The same call is already being done in the same .c
file, and AFAICT not under any different #ifdef conditions.

But I suppose it's possible that gpio_request_by_line_name() always end
up being GC'ed on those platforms, but dm_gpio_lookup_name() (probably
via gpio_lookup_name) does not.

ofnode_stringlist_search is available whenever CONFIG_OF_CONTROL, which
probably explains why you say "a few". And I guess it kind of makes
sense to at least make this conditional on there _being_ a dt node
associated to the udevice. I'll send a v2 with the block wrapped in if
(CONFIG_IS_ENABLED(OF_CONTROL)).

Rasmus

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-10-16  9:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-07 10:26 [PATCH] gpio: search gpio-line-names property in dm_gpio_lookup_name Rasmus Villemoes
2025-10-15 21:08 ` Tom Rini
2025-10-16  9:17   ` Rasmus Villemoes

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox