[PATCH v4 0/2] led: add function naming option from linux

in linux we have the option to create the name of a led optionally through the following properties:
- function - color - function-enumerator
This series adds support for parsing this properties if there is no label property.
patch 1 is new in this series since v4, which deletes the file doc/device-tree-bindings/leds/common.txt as Tom suggested.
Azure build: https://dev.azure.com/hs0298/hs/_build/results?buildId=168&view=results
shows an error: https://dev.azure.com/hs0298/hs/_build/results?buildId=168&view=logs&...
for sandbox64_clang ... sandbox64 target works! The error has nothing ToDo with this patchseries, as it is in dm_test_cmd_hash_md5, which tests content from source files, this series do not touch.
Changes in v4: - new in version 4 - remove ./doc/device-tree-bindings/leds/common.txt in seperate patch as this is now already in ./dts/upstream/Bindings/leds/common.txt as Tom commented. measured size impact of patch "led: add function naming option from linux": $ ./u-boot-size-test.sh capricorn_cxg3 --branch led-mainline [...] aarch64: (for 1/1 boards) all +886.0 rodata +230.0 text +656.0 capricorn_cxg3 : all +886 rodata +230 text +656 u-boot: add: 1/0, grow: 1/0 bytes: 416/0 (416) function old new delta led_post_bind 220 516 +296 led_colors - 120 +120 $ ./u-boot-size-test.sh aristainetos2c --branch led-mainline [...] 01: Merge patch series "add the support of sha256_hmac and sha256_hkdf" 40: led: add function naming option from linux arm: (for 2/2 boards) all +557.0 bss +20.0 rodata +161.0 text +376.0 aristainetos2c : all +557 bss +20 rodata +161 text +376 u-boot: add: 1/0, grow: 1/0 bytes: 264/0 (264) function old new delta led_post_bind 128 332 +204 led_colors - 60 +60 aristainetos2ccslb: all +557 bss +20 rodata +161 text +376 u-boot: add: 1/0, grow: 1/0 bytes: 264/0 (264) function old new delta led_post_bind 128 332 +204 led_colors - 60 +60 - rebase to current HEAD d8a7100d658. ("Subtree merge tag 'v6.13-dts' of dts repo [1] into dts/upstream")
Changes in v3: - work in comment from Tom: remove new file include/dt-bindings/leds/common.h in this patch as it is already in dts/upstream/include/dt-bindings/leds/common.h and so the checkpatch warnings from this file are gone! Thanks! Good catch! remove file include/linux/uapi/linux/uleds.h and as we only need one define from it, add this define in include/led.h
Changes in v2: - remove unused name variable in function led_get_label() - rebased patch to current HEAD 82d262ae162 ("Merge patch series "MediaTek MT7629 OF_UPSTREAM migration (v2)"")
Heiko Schocher (2): doc: remove redundant leds bindings led: add function naming option from linux
arch/sandbox/dts/test.dts | 37 ++++++++++++- doc/device-tree-bindings/leds/common.txt | 23 -------- drivers/led/led-uclass.c | 70 ++++++++++++++++++++++-- include/led.h | 6 ++ test/dm/led.c | 22 +++++++- test/dm/ofnode.c | 2 +- 6 files changed, 130 insertions(+), 30 deletions(-) delete mode 100644 doc/device-tree-bindings/leds/common.txt

remove file doc/device-tree-bindings/leds/common.txt as we have this now already in dts/upstream/include/dt-bindings/leds/common.h which is imported from linux.
Signed-off-by: Heiko Schocher hs@denx.de
---
Changes in v4: - new in version 4
doc/device-tree-bindings/leds/common.txt | 23 ----------------------- 1 file changed, 23 deletions(-) delete mode 100644 doc/device-tree-bindings/leds/common.txt
diff --git a/doc/device-tree-bindings/leds/common.txt b/doc/device-tree-bindings/leds/common.txt deleted file mode 100644 index 2d88816dd55..00000000000 --- a/doc/device-tree-bindings/leds/common.txt +++ /dev/null @@ -1,23 +0,0 @@ -Common leds properties. - -Optional properties for child nodes: -- label : The label for this LED. If omitted, the label is - taken from the node name (excluding the unit address). - -- linux,default-trigger : This parameter, if present, is a - string defining the trigger assigned to the LED. Current triggers are: - "backlight" - LED will act as a back-light, controlled by the framebuffer - system - "default-on" - LED will turn on (but for leds-gpio see "default-state" - property in Documentation/devicetree/bindings/gpio/led.txt) - "heartbeat" - LED "double" flashes at a load average based rate - "ide-disk" - LED indicates disk activity - "timer" - LED flashes at a fixed, configurable rate - -Examples: - -system-status { - label = "Status"; - linux,default-trigger = "heartbeat"; - ... -};

On Tue, Jan 28, 2025 at 02:52:45PM +0100, Heiko Schocher wrote:
remove file doc/device-tree-bindings/leds/common.txt as we have this now already in dts/upstream/include/dt-bindings/leds/common.h which is imported from linux.
Signed-off-by: Heiko Schocher hs@denx.de
Reviewed-by: Tom Rini trini@konsulko.com

in linux we have the option to create the name of a led optionally through the following properties:
- function - color - function-enumerator
This patch adds support for parsing this properties if there is no label property.
The led name is created in led_post_bind() and we need some storage place for it. Currently this patch prevents to use malloc() instead it stores the name in new member :
char name[LED_MAX_NAME_SIZE];
of struct led_uc_plat. While at it append led tests for the new feature.
Signed-off-by: Heiko Schocher hs@denx.de ---
Changes in v4: - remove ./doc/device-tree-bindings/leds/common.txt in seperate patch as this is now already in ./dts/upstream/Bindings/leds/common.txt as Tom commented. measured size impact of patch "led: add function naming option from linux": $ ./u-boot-size-test.sh capricorn_cxg3 --branch led-mainline [...] aarch64: (for 1/1 boards) all +886.0 rodata +230.0 text +656.0 capricorn_cxg3 : all +886 rodata +230 text +656 u-boot: add: 1/0, grow: 1/0 bytes: 416/0 (416) function old new delta led_post_bind 220 516 +296 led_colors - 120 +120 $ ./u-boot-size-test.sh aristainetos2c --branch led-mainline [...] 01: Merge patch series "add the support of sha256_hmac and sha256_hkdf" 40: led: add function naming option from linux arm: (for 2/2 boards) all +557.0 bss +20.0 rodata +161.0 text +376.0 aristainetos2c : all +557 bss +20 rodata +161 text +376 u-boot: add: 1/0, grow: 1/0 bytes: 264/0 (264) function old new delta led_post_bind 128 332 +204 led_colors - 60 +60 aristainetos2ccslb: all +557 bss +20 rodata +161 text +376 u-boot: add: 1/0, grow: 1/0 bytes: 264/0 (264) function old new delta led_post_bind 128 332 +204 led_colors - 60 +60 - rebase to current HEAD d8a7100d658. ("Subtree merge tag 'v6.13-dts' of dts repo [1] into dts/upstream")
Changes in v3: - work in comment from Tom: remove new file include/dt-bindings/leds/common.h in this patch as it is already in dts/upstream/include/dt-bindings/leds/common.h and so the checkpatch warnings from this file are gone! Thanks! Good catch! remove file include/linux/uapi/linux/uleds.h and as we only need one define from it, add this define in include/led.h
Changes in v2: - remove unused name variable in function led_get_label() - rebased patch to current HEAD 82d262ae162 ("Merge patch series "MediaTek MT7629 OF_UPSTREAM migration (v2)"")
arch/sandbox/dts/test.dts | 37 ++++++++++++++++++++- drivers/led/led-uclass.c | 70 ++++++++++++++++++++++++++++++++++++--- include/led.h | 6 ++++ test/dm/led.c | 22 +++++++++++- test/dm/ofnode.c | 2 +- 5 files changed, 130 insertions(+), 7 deletions(-)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index ae52b375ccb..b8f3012873e 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -13,6 +13,7 @@ #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/gpio/sandbox-gpio.h> #include <dt-bindings/input/input.h> +#include <dt-bindings/leds/common.h> #include <dt-bindings/pinctrl/sandbox-pinmux.h> #include <dt-bindings/mux/mux.h>
@@ -820,7 +821,7 @@ gpio-controller; #gpio-cells = <1>; gpio-bank-name = "a"; - sandbox,gpio-count = <20>; + sandbox,gpio-count = <25>; hog_input_active_low { gpio-hog; input; @@ -1010,6 +1011,40 @@ /* label intentionally omitted */ default-state = "off"; }; + + led-20 { + gpios = <&gpio_a 20 0>; + /* label intentionally omitted */ + function = LED_FUNCTION_STATUS; + color = <LED_COLOR_ID_RED>; + function-enumerator = <20>; + }; + + led-21 { + gpios = <&gpio_a 21 0>; + /* label intentionally omitted */ + function = LED_FUNCTION_STATUS; + color = <LED_COLOR_ID_GREEN>; + }; + + led-22 { + gpios = <&gpio_a 22 0>; + /* label intentionally omitted */ + function = LED_FUNCTION_STATUS; + }; + + led-23 { + gpios = <&gpio_a 23 0>; + /* label intentionally omitted */ + color = <LED_COLOR_ID_GREEN>; + }; + + led-24 { + gpios = <&gpio_a 24 0>; + label = "sandbox:function"; + function = LED_FUNCTION_STATUS; + color = <LED_COLOR_ID_GREEN>; + }; };
wdt-gpio-toggle { diff --git a/drivers/led/led-uclass.c b/drivers/led/led-uclass.c index 760750568c0..27ef890ed0a 100644 --- a/drivers/led/led-uclass.c +++ b/drivers/led/led-uclass.c @@ -13,6 +13,25 @@ #include <dm/lists.h> #include <dm/root.h> #include <dm/uclass-internal.h> +#include <dt-bindings/leds/common.h> + +static const char * const led_colors[LED_COLOR_ID_MAX] = { + [LED_COLOR_ID_WHITE] = "white", + [LED_COLOR_ID_RED] = "red", + [LED_COLOR_ID_GREEN] = "green", + [LED_COLOR_ID_BLUE] = "blue", + [LED_COLOR_ID_AMBER] = "amber", + [LED_COLOR_ID_VIOLET] = "violet", + [LED_COLOR_ID_YELLOW] = "yellow", + [LED_COLOR_ID_IR] = "ir", + [LED_COLOR_ID_MULTI] = "multicolor", + [LED_COLOR_ID_RGB] = "rgb", + [LED_COLOR_ID_PURPLE] = "purple", + [LED_COLOR_ID_ORANGE] = "orange", + [LED_COLOR_ID_PINK] = "pink", + [LED_COLOR_ID_CYAN] = "cyan", + [LED_COLOR_ID_LIME] = "lime", +};
int led_bind_generic(struct udevice *parent, const char *driver_name) { @@ -232,11 +251,54 @@ int led_activity_blink(void) #endif #endif
-static const char *led_get_label(ofnode node) +static const char *led_get_function_name(struct udevice *dev) +{ + struct led_uc_plat *uc_plat; + const char *func; + u32 color; + u32 enumerator; + int ret; + int cp; + + if (!dev) + return NULL; + + uc_plat = dev_get_uclass_plat(dev); + if (!uc_plat) + return NULL; + + if (uc_plat->label) + return uc_plat->label; + + /* Now try to detect function label name */ + func = dev_read_string(dev, "function"); + cp = dev_read_u32(dev, "color", &color); + if (cp == 0 || func) { + ret = dev_read_u32(dev, "function-enumerator", &enumerator); + if (!ret) { + snprintf(uc_plat->name, LED_MAX_NAME_SIZE, + "%s:%s-%d", + cp ? "" : led_colors[color], + func ? func : "", enumerator); + } else { + snprintf(uc_plat->name, LED_MAX_NAME_SIZE, + "%s:%s", + cp ? "" : led_colors[color], + func ? func : ""); + } + uc_plat->label = uc_plat->name; + } + + return uc_plat->label; +} + +static const char *led_get_label(struct udevice *dev, ofnode node) { const char *label;
label = ofnode_read_string(node, "label"); + if (!label) + label = led_get_function_name(dev); if (!label && !ofnode_read_string(node, "compatible")) label = ofnode_get_name(node);
@@ -249,7 +311,7 @@ static int led_post_bind(struct udevice *dev) const char *default_state;
if (!uc_plat->label) - uc_plat->label = led_get_label(dev_ofnode(dev)); + uc_plat->label = led_get_label(dev, dev_ofnode(dev));
uc_plat->default_state = LEDST_COUNT;
@@ -314,14 +376,14 @@ static int led_init(struct uclass *uc) #ifdef CONFIG_LED_BOOT ret = ofnode_options_get_by_phandle("boot-led", &led_node); if (!ret) - priv->boot_led_label = led_get_label(led_node); + priv->boot_led_label = led_get_label(NULL, led_node); priv->boot_led_period = ofnode_options_read_int("boot-led-period-ms", 250); #endif
#ifdef CONFIG_LED_ACTIVITY ret = ofnode_options_get_by_phandle("activity-led", &led_node); if (!ret) - priv->activity_led_label = led_get_label(led_node); + priv->activity_led_label = led_get_label(NULL, led_node); priv->activity_led_period = ofnode_options_read_int("activity-led-period-ms", 250); #endif diff --git a/include/led.h b/include/led.h index 64247cd3a70..a67db7af38d 100644 --- a/include/led.h +++ b/include/led.h @@ -53,6 +53,11 @@
struct udevice;
+/* + * value imported from linux:include/linux/uapi/linux/uleds.h + */ +#define LED_MAX_NAME_SIZE 64 + enum led_state_t { LEDST_OFF = 0, LEDST_ON = 1, @@ -86,6 +91,7 @@ struct led_sw_blink { struct led_uc_plat { const char *label; enum led_state_t default_state; + char name[LED_MAX_NAME_SIZE]; #ifdef CONFIG_LED_SW_BLINK struct led_sw_blink *sw_blink; #endif diff --git a/test/dm/led.c b/test/dm/led.c index e5b86326c3a..36652c2833a 100644 --- a/test/dm/led.c +++ b/test/dm/led.c @@ -20,7 +20,12 @@ static int dm_test_led_base(struct unit_test_state *uts) ut_assertok(uclass_get_device(UCLASS_LED, 1, &dev)); ut_assertok(uclass_get_device(UCLASS_LED, 2, &dev)); ut_assertok(uclass_get_device(UCLASS_LED, 3, &dev)); - ut_asserteq(-ENODEV, uclass_get_device(UCLASS_LED, 4, &dev)); + ut_assertok(uclass_get_device(UCLASS_LED, 4, &dev)); + ut_assertok(uclass_get_device(UCLASS_LED, 5, &dev)); + ut_assertok(uclass_get_device(UCLASS_LED, 6, &dev)); + ut_assertok(uclass_get_device(UCLASS_LED, 7, &dev)); + ut_assertok(uclass_get_device(UCLASS_LED, 8, &dev)); + ut_asserteq(-ENODEV, uclass_get_device(UCLASS_LED, 9, &dev));
return 0; } @@ -110,6 +115,21 @@ static int dm_test_led_label(struct unit_test_state *uts)
ut_asserteq(-ENODEV, led_get_by_label("sandbox:blue", &dev));
+ /* Test if function, color and function-enumerator naming works */ + ut_assertok(led_get_by_label("red:status-20", &dev)); + + /* Test if function, color naming works */ + ut_assertok(led_get_by_label("green:status", &dev)); + + /* Test if function, without color naming works */ + ut_assertok(led_get_by_label(":status", &dev)); + + /* Test if color without function naming works */ + ut_assertok(led_get_by_label("green:", &dev)); + + /* Test if function, color naming is ignored if label is found */ + ut_assertok(led_get_by_label("sandbox:function", &dev)); + return 0; } DM_TEST(dm_test_led_label, UTF_SCAN_PDATA | UTF_SCAN_FDT); diff --git a/test/dm/ofnode.c b/test/dm/ofnode.c index 4a23a3ca38c..cc8b444ff9a 100644 --- a/test/dm/ofnode.c +++ b/test/dm/ofnode.c @@ -1599,7 +1599,7 @@ static int dm_test_ofnode_delete(struct unit_test_state *uts) ut_assert(!ofnode_valid(node)); ut_assert(!ofnode_valid(ofnode_path("/leds/default_on")));
- ut_asserteq(2, ofnode_get_child_count(ofnode_path("/leds"))); + ut_asserteq(7, ofnode_get_child_count(ofnode_path("/leds")));
return 0; }

On Tue, Jan 28, 2025 at 02:52:46PM +0100, Heiko Schocher wrote:
in linux we have the option to create the name of a led optionally through the following properties:
- function
- color
- function-enumerator
This patch adds support for parsing this properties if there is no label property.
The led name is created in led_post_bind() and we need some storage place for it. Currently this patch prevents to use malloc() instead it stores the name in new member :
char name[LED_MAX_NAME_SIZE];
of struct led_uc_plat. While at it append led tests for the new feature.
Signed-off-by: Heiko Schocher hs@denx.de
Reviewed-by: Tom Rini trini@konsulko.com
participants (2)
-
Heiko Schocher
-
Tom Rini