From 23908d8f248f0aa912c7f05e276722e5caf4dc02 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 4 Nov 2025 18:44:56 +0100 Subject: [PATCH 1/3] test: gpio: include in build, and fixup bitrot Commit ebaa3d053e5 ("test: fix CONFIG_ACPIGEN dependencies"), which got into v2022.10-rc1, accidentally left out a $ before (CONFIG_DM_GPIO), with the effect that test/dm/gpio.c has not been built for three years. Unsurprisingly, the code in there has bit-rotted. - There's a missing ; causing plain build fail. That code was added in 9bf87e256c2 ("test: dm: update test for open-drain/open-source emulation in gpio-uclass"), which was part of v2020.07-rc3, i.e. long before the commit causing gpio.c to not be built at all. It did build at that time, but also, the missing semicolon wasn't found when fa847bb409d ("test: Wrap assert macros in ({ ... }) and fix missing semicolons") happened in 2023. - Commit 592b6f394ae ("led: add function naming option from linux") bumped sandbox,gpio-count for bank gpio_a in test.dts to 25, but didn't update the expected global gpio numbers accordingly. - The "lookup by label" test likely worked when it was added, but then I inadvertently broke it when I noticed that dm_gpio_lookup_label() seemed to be broken in commit 10e66449d7e ("gpio-uclass: fix gpio lookup by label") - which landed in v2023.01-rc1, so after gpio.c was no longer being built. The "label" (which is a u-boot concept) that a "hogged gpio" gets is .gpio-hog, which is why it used to work with the strncmp() but doesn't with strcmp(). We can either revert 10e66449d7e or append the ".gpio-hog" suffix as done below. I don't really have a dog in that race; when I did 10e66449d7e, it was because I thought the "lookup by label" was actually about the standardized gpio-line-names property, but then I learnt it was not, so is not at all useful to me. - The leak check now fails. Test: gpio_leak: gpio.c test/dm/core.c:112, dm_leak_check_end(): uts->start.uordblks == end.uordblks: Expected 0x2a95b0 (2790832), got 0x2a9650 (2790992) test/dm/gpio.c:328, dm_test_gpio_leak(): 0 == dm_leak_check_end(uts): Expected 0x0 (0), got 0x1 (1) Test: gpio_leak: gpio.c (flat tree) test/dm/core.c:112, dm_leak_check_end(): uts->start.uordblks == end.uordblks: Expected 0x2a9650 (2790992), got 0x2a9700 (2791168) test/dm/gpio.c:328, dm_test_gpio_leak(): 0 == dm_leak_check_end(uts): Expected 0x0 (0), got 0x1 (1) And it fails with the same differences (160/176) even if I remove the three lines that actually exercise any of the gpio code, i.e. make the whole function amount to ut_assertok(dm_leak_check_end(uts)); Test: gpio_leak: gpio.c test/dm/core.c:112, dm_leak_check_end(): uts->start.uordblks == end.uordblks: Expected 0x2a95b0 (2790832), got 0x2a9650 (2790992) test/dm/gpio.c:325, dm_test_gpio_leak(): 0 == dm_leak_check_end(uts): Expected 0x0 (0), got 0x1 (1) Test: gpio_leak: gpio.c (flat tree) test/dm/core.c:112, dm_leak_check_end(): uts->start.uordblks == end.uordblks: Expected 0x2a9650 (2790992), got 0x2a9700 (2791168) test/dm/gpio.c:325, dm_test_gpio_leak(): 0 == dm_leak_check_end(uts): Expected 0x0 (0), got 0x1 (1) So I suspect that the leak is somewhere in the test framework setup/teardown code - dm_leack_check_end() isn't really used anywhere else except in a dm/core test. Bisecting to figure out where that was introduced is somewhat of a hassle because of the other bitrot, and because of the SWIG failure that makes it very hard to build older U-Boots. So since it's better to have most of the gpio tests actually working instead of leaving all of gpio.c as dead code, #if 0 that part out and leave it as an archeological exercise. Signed-off-by: Rasmus Villemoes Reviewed-by: Heiko Schocher --- test/dm/Makefile | 2 +- test/dm/gpio.c | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/test/dm/Makefile b/test/dm/Makefile index 2db0e3b8dfd..a261b3fb4b7 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -25,7 +25,7 @@ ifeq ($(CONFIG_ACPIGEN),y) obj-y += acpi.o obj-y += acpigen.o obj-y += acpi_dp.o -obj-(CONFIG_DM_GPIO) += gpio.o +obj-$(CONFIG_DM_GPIO) += gpio.o obj-y += irq.o endif obj-$(CONFIG_ADC) += adc.o diff --git a/test/dm/gpio.c b/test/dm/gpio.c index b45946c143c..34a5d1a974e 100644 --- a/test/dm/gpio.c +++ b/test/dm/gpio.c @@ -29,14 +29,14 @@ static int dm_test_gpio(struct unit_test_state *uts) /* * We expect to get 4 banks. One is anonymous (just numbered) and - * comes from plat. The other are named a (20 gpios), + * comes from plat. The other are named a (25 gpios), * b (10 gpios) and c (10 gpios) and come from the device tree. See * test/dm/test.dts. */ ut_assertok(gpio_lookup_name("b4", &dev, &offset, &gpio)); ut_asserteq_str(dev->name, "extra-gpios"); ut_asserteq(4, offset); - ut_asserteq(CONFIG_SANDBOX_GPIO_COUNT + 20 + 4, gpio); + ut_asserteq(CONFIG_SANDBOX_GPIO_COUNT + 25 + 4, gpio); name = gpio_get_bank_info(dev, &offset_count); ut_asserteq_str("b", name); @@ -110,7 +110,7 @@ static int dm_test_gpio(struct unit_test_state *uts) name = gpio_get_bank_info(dev, &offset_count); ut_asserteq_str("a", name); - ut_asserteq(20, offset_count); + ut_asserteq(25, offset_count); /* add gpio hog tests */ ut_assertok(gpio_hog_lookup_name("hog_input_active_low", &desc)); @@ -135,7 +135,7 @@ static int dm_test_gpio(struct unit_test_state *uts) ut_asserteq(0, dm_gpio_get_value(desc)); /* Check if lookup for labels work */ - ut_assertok(gpio_lookup_name("hog_input_active_low", &dev, &offset, + ut_assertok(gpio_lookup_name("hog_input_active_low.gpio-hog", &dev, &offset, &gpio)); ut_asserteq_str(dev->name, "base-gpios"); ut_asserteq(10, offset); @@ -161,7 +161,7 @@ static int dm_test_gpio_opendrain_opensource(struct unit_test_state *uts) ut_asserteq_str("pinmux-gpios", gpio_c->name); ut_asserteq(8, gpio_request_list_by_name(dev, "test3-gpios", desc_list, - ARRAY_SIZE(desc_list), 0)) + ARRAY_SIZE(desc_list), 0)); ut_asserteq(true, !!device_active(gpio_c)); ut_asserteq_ptr(gpio_c, desc_list[0].dev); @@ -309,6 +309,8 @@ static int dm_test_gpio_copy(struct unit_test_state *uts) DM_TEST(dm_test_gpio_copy, UTF_SCAN_PDATA | UTF_SCAN_FDT); /* Test that we don't leak memory with GPIOs */ +/* Disabled for now as there seems to be a leak in the test framework itself. */ +#if 0 static int dm_test_gpio_leak(struct unit_test_state *uts) { ut_assertok(dm_test_gpio(uts)); @@ -319,6 +321,7 @@ static int dm_test_gpio_leak(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_gpio_leak, UTF_SCAN_PDATA | UTF_SCAN_FDT); +#endif /* Test that we can find GPIOs using phandles */ static int dm_test_gpio_phandles(struct unit_test_state *uts) From c92c3768b61aa84dd6adc214308d69814a8c0af0 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 4 Nov 2025 18:44:57 +0100 Subject: [PATCH 2/3] gpio: search gpio-line-names property in dm_gpio_lookup_name 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 dev_read_stringlist_search() depends on both OF_CONTROL and OF_LIBFDT (which matters for the SPL case), we need some .config conditional. However, it only adds about ~50 bytes of code to U-Boot proper, and dm_gpio_lookup_name() most often ends up being GC'ed for SPL, thus adds no overhead there, so for now make it a hidden symbol which is merely a convenient shorthand for CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(OF_LIBFDT). Signed-off-by: Rasmus Villemoes Reviewed-by: Heiko Schocher --- drivers/gpio/Kconfig | 16 ++++++++++++++++ drivers/gpio/gpio-uclass.c | 11 ++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index db077e472a8..a47c127d98c 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -97,6 +97,22 @@ config SPL_DM_GPIO_LOOKUP_LABEL different gpios on different hardware versions for the same functionality in board code. +config DM_GPIO_LOOKUP_LINE_NAME + def_bool y + depends on OF_CONTROL && OF_LIBFDT + help + This option enables specifying a GPIO by referring to its + name as defined by the "gpio-line-names" property in the + gpio controller's devicetree node. + +config SPL_DM_GPIO_LOOKUP_LINE_NAME + def_bool y + depends on SPL_OF_CONTROL && SPL_OF_LIBFDT + help + This option enables specifying a GPIO by referring to its + name as defined by the "gpio-line-names" property in the + gpio controller's devicetree node. + config ADI_GPIO bool "ADI GPIO driver" depends on DM_GPIO && ARCH_SC5XX diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 3d9f8b32b8d..6f52fdf4ef4 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,15 @@ 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. */ + if (CONFIG_IS_ENABLED(DM_GPIO_LOOKUP_LINE_NAME)) { + ret = dev_read_stringlist_search(dev, "gpio-line-names", name); + if (ret >= 0) { + offset = ret; + break; + } + } } if (!dev) From e5e4b60c55e448e6a9a6bf6f7e9cfd01fe3103e1 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 4 Nov 2025 18:44:58 +0100 Subject: [PATCH 3/3] test: gpio: add test for gpio-line-names lookup Signed-off-by: Rasmus Villemoes Reviewed-by: Heiko Schocher --- arch/sandbox/dts/test.dts | 2 ++ test/dm/gpio.c | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index a2c739a2044..13d8e498b03 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -823,6 +823,7 @@ #gpio-cells = <1>; gpio-bank-name = "a"; sandbox,gpio-count = <25>; + gpio-line-names = "", "eth1-reset", "rtc-irq"; hog_input_active_low { gpio-hog; input; @@ -851,6 +852,7 @@ #gpio-cells = <5>; gpio-bank-name = "b"; sandbox,gpio-count = <10>; + gpio-line-names = "factory-reset"; }; gpio_c: pinmux-gpios { diff --git a/test/dm/gpio.c b/test/dm/gpio.c index 34a5d1a974e..0fb05b5ca06 100644 --- a/test/dm/gpio.c +++ b/test/dm/gpio.c @@ -143,6 +143,17 @@ static int dm_test_gpio(struct unit_test_state *uts) ut_assert(gpio_lookup_name("hog_not_exist", &dev, &offset, &gpio)); + /* Check if lookup for gpio-line-names work */ + ut_assertok(gpio_lookup_name("factory-reset", &dev, &offset, &gpio)); + ut_asserteq_str(dev->name, "extra-gpios"); + ut_asserteq(0, offset); + ut_asserteq(CONFIG_SANDBOX_GPIO_COUNT + 25 + 0, gpio); + + ut_assertok(gpio_lookup_name("rtc-irq", &dev, &offset, &gpio)); + ut_asserteq_str(dev->name, "base-gpios"); + ut_asserteq(2, offset); + ut_asserteq(CONFIG_SANDBOX_GPIO_COUNT + 2, gpio); + return 0; } DM_TEST(dm_test_gpio, UTF_SCAN_PDATA | UTF_SCAN_FDT);