* [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards
@ 2025-11-11 5:41 FUKAUMI Naoki
2025-11-11 6:04 ` Dragan Simic
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: FUKAUMI Naoki @ 2025-11-11 5:41 UTC (permalink / raw)
To: heiko
Cc: robh, krzk+dt, conor+dt, jbx6244, dsimic, pgwipeout, didi.debian,
jonas, ziyao, amadeus, nicolas.frattaroli, pbrobinson, wens,
detlev.casanova, stephen, sebastian.reichel, liujianfeng1994,
andy.yan, damon.ding, kylepzak, devicetree, linux-rockchip,
FUKAUMI Naoki
Radxa's boards turn all LEDs on at boot(loader), but some boards don't
have `default-state` property in Linux kernel tree but have it in
U-Boot tree instead[1].
This patch adds `default-state = "on"` for (almost) all LEDs (with a
few exceptions which should be "off" such as RGB LEDs on E25 and LAN/
WAN LEDs on E20C/E52C).
Also, remove following redundant properties:
linux,default-trigger = "default-on"; // use default-state = "on"
default-state = "off"; // default is "off"
[1]
https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3328-rock-pi-e-base-u-boot.dtsi#L10-12
https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3399-rock-4c-plus-u-boot.dtsi#L11-17
https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi#L11-13
https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3566-radxa-cm3-io-u-boot.dtsi#L10-12
https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3566-rock-3c-u-boot.dtsi#L14-16
https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3568-radxa-e25-u-boot.dtsi#L7-24
https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi#L11-13
https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi#L11-13
https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi#L10-12
Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
---
Changes in v2:
- Add more URLs for reference
- Reword commit message
---
arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts | 1 -
arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts | 1 +
arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts | 3 ++-
arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi | 1 +
arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts | 2 --
arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts | 1 -
arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts | 1 +
arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts | 1 +
arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi | 1 +
arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 1 +
arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts | 3 ++-
arch/arm64/boot/dts/rockchip/rk3582-radxa-e52c.dts | 2 --
arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts | 3 ++-
arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi | 1 +
arch/arm64/boot/dts/rockchip/rk3588-rock-5t.dts | 1 +
arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 3 ++-
16 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts b/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts
index 7a32972bc2496..c1e3098b9a7bc 100644
--- a/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts
@@ -35,7 +35,6 @@ green-led {
function = LED_FUNCTION_POWER;
gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;
label = "rockpis:green:power";
- linux,default-trigger = "default-on";
};
blue-led {
diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
index a4bdd87d0729f..d3d6f34b66fb0 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
@@ -59,6 +59,7 @@ leds {
led-0 {
color = <LED_COLOR_ID_BLUE>;
+ default-state = "on";
gpios = <&gpio3 RK_PA5 GPIO_ACTIVE_LOW>;
linux,default-trigger = "heartbeat";
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts b/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
index 962b8b231c960..a83ffbef22a7b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
@@ -39,14 +39,15 @@ leds {
led-0 {
function = LED_FUNCTION_POWER;
color = <LED_COLOR_ID_GREEN>;
+ default-state = "on";
gpios = <&gpio3 RK_PD4 GPIO_ACTIVE_LOW>;
- linux,default-trigger = "default-on";
};
/* USER_LED2 */
led-1 {
function = LED_FUNCTION_STATUS;
color = <LED_COLOR_ID_BLUE>;
+ default-state = "on";
gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_HIGH>;
linux,default-trigger = "heartbeat";
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
index 046dbe3290178..ef434c23fe85c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
@@ -35,6 +35,7 @@ leds {
led-0 {
function = LED_FUNCTION_STATUS;
color = <LED_COLOR_ID_BLUE>;
+ default-state = "on";
gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_HIGH>;
linux,default-trigger = "heartbeat";
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
index b324527561558..79d316a1d8495 100644
--- a/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
@@ -62,7 +62,6 @@ leds {
led-lan {
color = <LED_COLOR_ID_GREEN>;
- default-state = "off";
function = LED_FUNCTION_LAN;
gpios = <&gpio4 RK_PB5 GPIO_ACTIVE_HIGH>;
linux,default-trigger = "netdev";
@@ -78,7 +77,6 @@ led-sys {
led-wan {
color = <LED_COLOR_ID_GREEN>;
- default-state = "off";
function = LED_FUNCTION_WAN;
gpios = <&gpio4 RK_PC0 GPIO_ACTIVE_HIGH>;
linux,default-trigger = "netdev";
diff --git a/arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts b/arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts
index c03ae1dd34560..0b696d49b71fa 100644
--- a/arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts
@@ -45,7 +45,6 @@ led-1 {
default-state = "on";
function = LED_FUNCTION_STATUS;
gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_LOW>;
- linux,default-trigger = "default-on";
};
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts b/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts
index b5b253f04cdf5..9e7212b70e3f1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts
@@ -46,6 +46,7 @@ leds {
led-1 {
gpios = <&gpio4 RK_PA4 GPIO_ACTIVE_LOW>;
color = <LED_COLOR_ID_GREEN>;
+ default-state = "on";
function = LED_FUNCTION_ACTIVITY;
linux,default-trigger = "heartbeat";
pinctrl-names = "default";
diff --git a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
index 6224d72813e59..3ec108bcf89a1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
@@ -47,6 +47,7 @@ led-0 {
gpios = <&gpio0 RK_PA0 GPIO_ACTIVE_HIGH>;
function = LED_FUNCTION_HEARTBEAT;
color = <LED_COLOR_ID_BLUE>;
+ default-state = "on";
linux,default-trigger = "heartbeat";
pinctrl-names = "default";
pinctrl-0 = <&user_led2>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi b/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi
index 729e38b9f620e..140582f8e1034 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi
@@ -23,6 +23,7 @@ led_user: led-0 {
gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;
function = LED_FUNCTION_HEARTBEAT;
color = <LED_COLOR_ID_GREEN>;
+ default-state = "on";
linux,default-trigger = "heartbeat";
pinctrl-names = "default";
pinctrl-0 = <&led_user_en>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
index 44cfdfeed6681..e6c18df0fa582 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
@@ -47,6 +47,7 @@ led_user: led-0 {
gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
function = LED_FUNCTION_HEARTBEAT;
color = <LED_COLOR_ID_BLUE>;
+ default-state = "on";
linux,default-trigger = "heartbeat";
pinctrl-names = "default";
pinctrl-0 = <&led_user_en>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
index 9bc33422ced50..99d3a8be8f18c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
@@ -52,13 +52,14 @@ leds: leds {
power-led {
color = <LED_COLOR_ID_GREEN>;
+ default-state = "on";
function = LED_FUNCTION_STATUS;
gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
- linux,default-trigger = "default-on";
};
user-led {
color = <LED_COLOR_ID_BLUE>;
+ default-state = "on";
function = LED_FUNCTION_HEARTBEAT;
gpios = <&gpio0 RK_PC4 GPIO_ACTIVE_LOW>;
linux,default-trigger = "heartbeat";
diff --git a/arch/arm64/boot/dts/rockchip/rk3582-radxa-e52c.dts b/arch/arm64/boot/dts/rockchip/rk3582-radxa-e52c.dts
index 854c118418eb8..f737769d4a007 100644
--- a/arch/arm64/boot/dts/rockchip/rk3582-radxa-e52c.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3582-radxa-e52c.dts
@@ -71,7 +71,6 @@ leds-1 {
led-1 {
color = <LED_COLOR_ID_GREEN>;
- default-state = "off";
function = LED_FUNCTION_LAN;
linux,default-trigger = "netdev";
pwms = <&pwm14 0 1000000 PWM_POLARITY_INVERTED>;
@@ -80,7 +79,6 @@ led-1 {
led-2 {
color = <LED_COLOR_ID_GREEN>;
- default-state = "off";
function = LED_FUNCTION_WAN;
linux,default-trigger = "netdev";
pwms = <&pwm11 0 1000000 PWM_POLARITY_INVERTED>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
index bc8140883de47..86477346c3f5a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
@@ -88,11 +88,12 @@ gpio-leds {
pinctrl-0 = <&led_pins>;
power-led1 {
+ default-state = "on";
gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
- linux,default-trigger = "default-on";
};
hdd-led2 {
+ default-state = "on";
gpios = <&gpio0 RK_PC0 GPIO_ACTIVE_HIGH>;
linux,default-trigger = "disk-activity";
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi
index e5c474e4d02a6..8c4a4270f9f93 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi
@@ -30,6 +30,7 @@ leds {
led_rgb_b {
function = LED_FUNCTION_STATUS;
color = <LED_COLOR_ID_BLUE>;
+ default-state = "on";
gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
linux,default-trigger = "heartbeat";
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5t.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5t.dts
index 0dd90c744380b..87e9d4b86dad4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5t.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5t.dts
@@ -33,6 +33,7 @@ leds {
led_rgb_b {
function = LED_FUNCTION_STATUS;
color = <LED_COLOR_ID_BLUE>;
+ default-state = "on";
gpios = <&gpio0 RK_PA0 GPIO_ACTIVE_HIGH>;
linux,default-trigger = "heartbeat";
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
index 19a08f7794e67..46c81e796b100 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
@@ -54,6 +54,7 @@ leds {
io-led {
color = <LED_COLOR_ID_BLUE>;
+ default-state = "on";
function = LED_FUNCTION_STATUS;
gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_HIGH>;
linux,default-trigger = "heartbeat";
@@ -61,9 +62,9 @@ io-led {
power-led {
color = <LED_COLOR_ID_GREEN>;
+ default-state = "on";
function = LED_FUNCTION_POWER;
gpios = <&gpio3 RK_PC4 GPIO_ACTIVE_HIGH>;
- linux,default-trigger = "default-on";
};
};
--
2.43.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards
2025-11-11 5:41 [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards FUKAUMI Naoki
@ 2025-11-11 6:04 ` Dragan Simic
2025-11-11 13:07 ` Diederik de Haas
2025-11-12 15:04 ` FUKAUMI Naoki
2 siblings, 0 replies; 13+ messages in thread
From: Dragan Simic @ 2025-11-11 6:04 UTC (permalink / raw)
To: FUKAUMI Naoki
Cc: heiko, robh, krzk+dt, conor+dt, jbx6244, pgwipeout, didi.debian,
jonas, ziyao, amadeus, nicolas.frattaroli, pbrobinson, wens,
detlev.casanova, stephen, sebastian.reichel, liujianfeng1994,
andy.yan, damon.ding, kylepzak, devicetree, linux-rockchip
Hello Naoki,
Thanks for the v2. Please, see a couple of remarks below.
On Tuesday, November 11, 2025 06:41 CET, FUKAUMI Naoki <naoki@radxa.com> wrote:
> Radxa's boards turn all LEDs on at boot(loader), but some boards don't
s/Radxa's/Radxa/ -- because those are boards made by Radxa, not some
boards belonging to Radxa; if it were specifically about the board
designs, the possessive form would be fine, for example
> have `default-state` property in Linux kernel tree but have it in
> U-Boot tree instead[1].
>
> This patch adds `default-state = "on"` for (almost) all LEDs (with a
> few exceptions which should be "off" such as RGB LEDs on E25 and LAN/
> WAN LEDs on E20C/E52C).
>
> Also, remove following redundant properties:
> linux,default-trigger = "default-on"; // use default-state = "on"
> default-state = "off"; // default is "off"
>
> [1]
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3328-rock-pi-e-base-u-boot.dtsi#L10-12
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3399-rock-4c-plus-u-boot.dtsi#L11-17
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi#L11-13
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3566-radxa-cm3-io-u-boot.dtsi#L10-12
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3566-rock-3c-u-boot.dtsi#L14-16
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3568-radxa-e25-u-boot.dtsi#L7-24
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi#L11-13
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi#L11-13
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi#L10-12
Hmm, this particularly complex reference presents us with a somewhat
interesting literary challenge. :) There should be only one URL per
reference, so one way to resolve this would be to reword the opening
paragraph like this:
Radxa boards are intended to turn all their LEDs on at boot time,
as part of the intended visual feedback to the end users, but some
boards don't have the associated "default-state" LED properties
defined in the Linux kernel, yet they have them defined in U-Boot.
This includes ROCK Pi E, [1] ROCK 4C+, [2] ROCK Pi 4, [3] CM3, [4]
ROCK 3C, [5] E25, [6] ROCK 3A, [7] ROCK 5B, [8] and ROCK 5A. [9]
[1] https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3328-rock-pi-e-base-u-boot.dtsi#L10-12
[2] https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3399-rock-4c-plus-u-boot.dtsi#L11-17
[3] https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi#L11-13
[4] https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3566-radxa-cm3-io-u-boot.dtsi#L10-12
[5] https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3566-rock-3c-u-boot.dtsi#L14-16
[6] https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3568-radxa-e25-u-boot.dtsi#L7-24
[7] https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi#L11-13
[8] https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi#L11-13
[9] https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi#L10-12
Of course, it would also make sense to order the list of boards and
the associated references alphabetically, which would make the list
more readable and is left as an exercise. :)
> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
> ---
> Changes in v2:
> - Add more URLs for reference
> - Reword commit message
> ---
> arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts | 1 -
> arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts | 1 +
> arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts | 3 ++-
> arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi | 1 +
> arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts | 2 --
> arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts | 1 -
> arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts | 1 +
> arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts | 1 +
> arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi | 1 +
> arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 1 +
> arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts | 3 ++-
> arch/arm64/boot/dts/rockchip/rk3582-radxa-e52c.dts | 2 --
> arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts | 3 ++-
> arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi | 1 +
> arch/arm64/boot/dts/rockchip/rk3588-rock-5t.dts | 1 +
> arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 3 ++-
> 16 files changed, 16 insertions(+), 10 deletions(-)
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards
2025-11-11 5:41 [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards FUKAUMI Naoki
2025-11-11 6:04 ` Dragan Simic
@ 2025-11-11 13:07 ` Diederik de Haas
2025-11-11 14:46 ` Dragan Simic
2025-11-12 15:04 ` FUKAUMI Naoki
2 siblings, 1 reply; 13+ messages in thread
From: Diederik de Haas @ 2025-11-11 13:07 UTC (permalink / raw)
To: FUKAUMI Naoki, heiko
Cc: robh, krzk+dt, conor+dt, jbx6244, dsimic, pgwipeout, jonas, ziyao,
amadeus, nicolas.frattaroli, pbrobinson, wens, detlev.casanova,
stephen, sebastian.reichel, liujianfeng1994, andy.yan, damon.ding,
kylepzak, devicetree, linux-rockchip
On Tue Nov 11, 2025 at 6:41 AM CET, FUKAUMI Naoki wrote:
> Radxa's boards turn all LEDs on at boot(loader), but some boards don't
> have `default-state` property in Linux kernel tree but have it in
> U-Boot tree instead[1].
>
> This patch adds `default-state = "on"` for (almost) all LEDs (with a
> few exceptions which should be "off" such as RGB LEDs on E25 and LAN/
> WAN LEDs on E20C/E52C).
I'm missing the *why* these changes would be an improvement.
Personally, for both 'heartbeat' and 'netdev' triggers, I want them to
be off by default and once it gets a 'heartbeat' or a 'netdev' trigger,
THEN I want the LED to be on/blinking.
> Also, remove following redundant properties:
> linux,default-trigger = "default-on"; // use default-state = "on"
> default-state = "off"; // default is "off"
>
> [1]
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3328-rock-pi-e-base-u-boot.dtsi#L10-12
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3399-rock-4c-plus-u-boot.dtsi#L11-17
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi#L11-13
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3566-radxa-cm3-io-u-boot.dtsi#L10-12
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3566-rock-3c-u-boot.dtsi#L14-16
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3568-radxa-e25-u-boot.dtsi#L7-24
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi#L11-13
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi#L11-13
That the *bootloader* turns them on, is fine by me as it signals the
board has received power and the bootloader has started.
I don't think that that automatically means that Linux must do the same.
Not a Radxa board, but the PineTab2's keyboard LEDs are turned on. I
find that useful wrt the bootloader, but (actually) annoying that it is
on by default with Linux. When I want/need backlight on the keyboard,
I'll turn it on myself. Now I need to turn it off in 90% of cases.
My 0.02
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi#L10-12
>
> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
> ---
> Changes in v2:
> - Add more URLs for reference
> - Reword commit message
> ---
> arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts | 1 -
> arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts | 1 +
> arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts | 3 ++-
> arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi | 1 +
> arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts | 2 --
> arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts | 1 -
> arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts | 1 +
> arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts | 1 +
> arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi | 1 +
> arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 1 +
> arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts | 3 ++-
> arch/arm64/boot/dts/rockchip/rk3582-radxa-e52c.dts | 2 --
> arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts | 3 ++-
> arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi | 1 +
> arch/arm64/boot/dts/rockchip/rk3588-rock-5t.dts | 1 +
> arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 3 ++-
> 16 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts b/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts
> index 7a32972bc2496..c1e3098b9a7bc 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts
> @@ -35,7 +35,6 @@ green-led {
> function = LED_FUNCTION_POWER;
> gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;
> label = "rockpis:green:power";
> - linux,default-trigger = "default-on";
> };
>
> blue-led {
> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
> index a4bdd87d0729f..d3d6f34b66fb0 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
> @@ -59,6 +59,7 @@ leds {
>
> led-0 {
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> gpios = <&gpio3 RK_PA5 GPIO_ACTIVE_LOW>;
> linux,default-trigger = "heartbeat";
> };
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts b/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
> index 962b8b231c960..a83ffbef22a7b 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
> @@ -39,14 +39,15 @@ leds {
> led-0 {
> function = LED_FUNCTION_POWER;
> color = <LED_COLOR_ID_GREEN>;
> + default-state = "on";
> gpios = <&gpio3 RK_PD4 GPIO_ACTIVE_LOW>;
> - linux,default-trigger = "default-on";
> };
>
> /* USER_LED2 */
> led-1 {
> function = LED_FUNCTION_STATUS;
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_HIGH>;
> linux,default-trigger = "heartbeat";
> };
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> index 046dbe3290178..ef434c23fe85c 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> @@ -35,6 +35,7 @@ leds {
> led-0 {
> function = LED_FUNCTION_STATUS;
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_HIGH>;
> linux,default-trigger = "heartbeat";
> };
> diff --git a/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
> index b324527561558..79d316a1d8495 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
> @@ -62,7 +62,6 @@ leds {
>
> led-lan {
> color = <LED_COLOR_ID_GREEN>;
> - default-state = "off";
> function = LED_FUNCTION_LAN;
> gpios = <&gpio4 RK_PB5 GPIO_ACTIVE_HIGH>;
> linux,default-trigger = "netdev";
> @@ -78,7 +77,6 @@ led-sys {
>
> led-wan {
> color = <LED_COLOR_ID_GREEN>;
> - default-state = "off";
> function = LED_FUNCTION_WAN;
> gpios = <&gpio4 RK_PC0 GPIO_ACTIVE_HIGH>;
> linux,default-trigger = "netdev";
> diff --git a/arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts b/arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts
> index c03ae1dd34560..0b696d49b71fa 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts
> @@ -45,7 +45,6 @@ led-1 {
> default-state = "on";
> function = LED_FUNCTION_STATUS;
> gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_LOW>;
> - linux,default-trigger = "default-on";
> };
> };
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts b/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts
> index b5b253f04cdf5..9e7212b70e3f1 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts
> @@ -46,6 +46,7 @@ leds {
> led-1 {
> gpios = <&gpio4 RK_PA4 GPIO_ACTIVE_LOW>;
> color = <LED_COLOR_ID_GREEN>;
> + default-state = "on";
> function = LED_FUNCTION_ACTIVITY;
> linux,default-trigger = "heartbeat";
> pinctrl-names = "default";
> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
> index 6224d72813e59..3ec108bcf89a1 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
> @@ -47,6 +47,7 @@ led-0 {
> gpios = <&gpio0 RK_PA0 GPIO_ACTIVE_HIGH>;
> function = LED_FUNCTION_HEARTBEAT;
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> linux,default-trigger = "heartbeat";
> pinctrl-names = "default";
> pinctrl-0 = <&user_led2>;
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi b/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi
> index 729e38b9f620e..140582f8e1034 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi
> @@ -23,6 +23,7 @@ led_user: led-0 {
> gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;
> function = LED_FUNCTION_HEARTBEAT;
> color = <LED_COLOR_ID_GREEN>;
> + default-state = "on";
> linux,default-trigger = "heartbeat";
> pinctrl-names = "default";
> pinctrl-0 = <&led_user_en>;
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> index 44cfdfeed6681..e6c18df0fa582 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -47,6 +47,7 @@ led_user: led-0 {
> gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
> function = LED_FUNCTION_HEARTBEAT;
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> linux,default-trigger = "heartbeat";
> pinctrl-names = "default";
> pinctrl-0 = <&led_user_en>;
> diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> index 9bc33422ced50..99d3a8be8f18c 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> @@ -52,13 +52,14 @@ leds: leds {
>
> power-led {
> color = <LED_COLOR_ID_GREEN>;
> + default-state = "on";
> function = LED_FUNCTION_STATUS;
> gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
> - linux,default-trigger = "default-on";
> };
>
> user-led {
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> function = LED_FUNCTION_HEARTBEAT;
> gpios = <&gpio0 RK_PC4 GPIO_ACTIVE_LOW>;
> linux,default-trigger = "heartbeat";
> diff --git a/arch/arm64/boot/dts/rockchip/rk3582-radxa-e52c.dts b/arch/arm64/boot/dts/rockchip/rk3582-radxa-e52c.dts
> index 854c118418eb8..f737769d4a007 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3582-radxa-e52c.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3582-radxa-e52c.dts
> @@ -71,7 +71,6 @@ leds-1 {
>
> led-1 {
> color = <LED_COLOR_ID_GREEN>;
> - default-state = "off";
> function = LED_FUNCTION_LAN;
> linux,default-trigger = "netdev";
> pwms = <&pwm14 0 1000000 PWM_POLARITY_INVERTED>;
> @@ -80,7 +79,6 @@ led-1 {
>
> led-2 {
> color = <LED_COLOR_ID_GREEN>;
> - default-state = "off";
> function = LED_FUNCTION_WAN;
> linux,default-trigger = "netdev";
> pwms = <&pwm11 0 1000000 PWM_POLARITY_INVERTED>;
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
> index bc8140883de47..86477346c3f5a 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
> @@ -88,11 +88,12 @@ gpio-leds {
> pinctrl-0 = <&led_pins>;
>
> power-led1 {
> + default-state = "on";
> gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
> - linux,default-trigger = "default-on";
> };
>
> hdd-led2 {
> + default-state = "on";
> gpios = <&gpio0 RK_PC0 GPIO_ACTIVE_HIGH>;
> linux,default-trigger = "disk-activity";
> };
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi
> index e5c474e4d02a6..8c4a4270f9f93 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi
> @@ -30,6 +30,7 @@ leds {
> led_rgb_b {
> function = LED_FUNCTION_STATUS;
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
> linux,default-trigger = "heartbeat";
> };
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5t.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5t.dts
> index 0dd90c744380b..87e9d4b86dad4 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5t.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5t.dts
> @@ -33,6 +33,7 @@ leds {
> led_rgb_b {
> function = LED_FUNCTION_STATUS;
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> gpios = <&gpio0 RK_PA0 GPIO_ACTIVE_HIGH>;
> linux,default-trigger = "heartbeat";
> };
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> index 19a08f7794e67..46c81e796b100 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> @@ -54,6 +54,7 @@ leds {
>
> io-led {
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> function = LED_FUNCTION_STATUS;
> gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_HIGH>;
> linux,default-trigger = "heartbeat";
> @@ -61,9 +62,9 @@ io-led {
>
> power-led {
> color = <LED_COLOR_ID_GREEN>;
> + default-state = "on";
> function = LED_FUNCTION_POWER;
> gpios = <&gpio3 RK_PC4 GPIO_ACTIVE_HIGH>;
> - linux,default-trigger = "default-on";
> };
> };
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards
2025-11-11 13:07 ` Diederik de Haas
@ 2025-11-11 14:46 ` Dragan Simic
2025-11-11 15:32 ` FUKAUMI Naoki
0 siblings, 1 reply; 13+ messages in thread
From: Dragan Simic @ 2025-11-11 14:46 UTC (permalink / raw)
To: Diederik de Haas
Cc: FUKAUMI Naoki, heiko, robh, krzk+dt, conor+dt, jbx6244, pgwipeout,
jonas, ziyao, amadeus, nicolas.frattaroli, pbrobinson, wens,
detlev.casanova, stephen, sebastian.reichel, liujianfeng1994,
andy.yan, damon.ding, kylepzak, devicetree, linux-rockchip
Hello Diederik,
On Tuesday, November 11, 2025 14:07 CET, "Diederik de Haas" <diederik@cknow-tech.com> wrote:
> On Tue Nov 11, 2025 at 6:41 AM CET, FUKAUMI Naoki wrote:
> > Radxa's boards turn all LEDs on at boot(loader), but some boards don't
> > have `default-state` property in Linux kernel tree but have it in
> > U-Boot tree instead[1].
> >
> > This patch adds `default-state = "on"` for (almost) all LEDs (with a
> > few exceptions which should be "off" such as RGB LEDs on E25 and LAN/
> > WAN LEDs on E20C/E52C).
>
> I'm missing the *why* these changes would be an improvement.
>
> Personally, for both 'heartbeat' and 'netdev' triggers, I want them to
> be off by default and once it gets a 'heartbeat' or a 'netdev' trigger,
> THEN I want the LED to be on/blinking.
That's a good question for Naoki. My own preference would also
be to have the device's power LED turned on by U-Boot as quickly
as possible after supplying power to the board or turning it on
by pressing the power button. I'm actually not a big fan of
having all the LEDs shining for a couple of seconds or so, which
may actually look like some error condition to me.
Having all that in mind, I may suggest that just the U-Boot's
behavior is changed to turn the power LEDs on only.
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards
2025-11-11 14:46 ` Dragan Simic
@ 2025-11-11 15:32 ` FUKAUMI Naoki
2025-11-11 16:14 ` Dragan Simic
0 siblings, 1 reply; 13+ messages in thread
From: FUKAUMI Naoki @ 2025-11-11 15:32 UTC (permalink / raw)
To: Dragan Simic, Diederik de Haas
Cc: heiko, robh, krzk+dt, conor+dt, jbx6244, pgwipeout, jonas, ziyao,
amadeus, nicolas.frattaroli, pbrobinson, wens, detlev.casanova,
stephen, sebastian.reichel, liujianfeng1994, andy.yan, damon.ding,
kylepzak, devicetree, linux-rockchip
Hi Diederik, Dragan,
On 11/11/25 23:46, Dragan Simic wrote:
> Hello Diederik,
>
> On Tuesday, November 11, 2025 14:07 CET, "Diederik de Haas" <diederik@cknow-tech.com> wrote:
>> On Tue Nov 11, 2025 at 6:41 AM CET, FUKAUMI Naoki wrote:
>>> Radxa's boards turn all LEDs on at boot(loader), but some boards don't
>>> have `default-state` property in Linux kernel tree but have it in
>>> U-Boot tree instead[1].
>>>
>>> This patch adds `default-state = "on"` for (almost) all LEDs (with a
>>> few exceptions which should be "off" such as RGB LEDs on E25 and LAN/
>>> WAN LEDs on E20C/E52C).
>>
>> I'm missing the *why* these changes would be an improvement.
>>
>> Personally, for both 'heartbeat' and 'netdev' triggers, I want them to
>> be off by default and once it gets a 'heartbeat' or a 'netdev' trigger,
>> THEN I want the LED to be on/blinking.
>
> That's a good question for Naoki. My own preference would also
> be to have the device's power LED turned on by U-Boot as quickly
> as possible after supplying power to the board or turning it on
> by pressing the power button. I'm actually not a big fan of
> having all the LEDs shining for a couple of seconds or so, which
> may actually look like some error condition to me.
>
> Having all that in mind, I may suggest that just the U-Boot's
> behavior is changed to turn the power LEDs on only.
I can't quite explain it, but...
- 1st (Power) LED
The 1st (power) LED turns on automatically/immediately without software
intervention. (On some boards, this LED cannot be controlled by software
at all.)
In DTS, this should be described using `default-state = "on"`. The use
of the Linux-specific property `linux,default-trigger = "default-on"` is
unsuitable for non-Linux environments.
- 2nd (Heartbeat) LED
The 2nd (heartbeat) LED can be controlled by software. It should be lit
up as quickly as possible to indicate that the very first software
(e.g., the bootloader) is running.
On Linux, usually this is used as `linux,default-trigger = "heartbeat"`.
It indicates that kernel is running (regardless of the `default-state`
setting), and its behavior can be modified in user space.
Best regards,
--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards
2025-11-11 15:32 ` FUKAUMI Naoki
@ 2025-11-11 16:14 ` Dragan Simic
2025-11-11 18:32 ` Quentin Schulz
0 siblings, 1 reply; 13+ messages in thread
From: Dragan Simic @ 2025-11-11 16:14 UTC (permalink / raw)
To: FUKAUMI Naoki
Cc: Diederik de Haas, heiko, robh, krzk+dt, conor+dt, jbx6244,
pgwipeout, jonas, ziyao, amadeus, nicolas.frattaroli, pbrobinson,
wens, detlev.casanova, stephen, sebastian.reichel,
liujianfeng1994, andy.yan, damon.ding, kylepzak, devicetree,
linux-rockchip, Quentin Schulz
Hello all,
(+ Quentin)
On Tuesday, November 11, 2025 16:32 CET, FUKAUMI Naoki <naoki@radxa.com> wrote:
> On 11/11/25 23:46, Dragan Simic wrote:
> > On Tuesday, November 11, 2025 14:07 CET, "Diederik de Haas" <diederik@cknow-tech.com> wrote:
> >> On Tue Nov 11, 2025 at 6:41 AM CET, FUKAUMI Naoki wrote:
> >>> Radxa's boards turn all LEDs on at boot(loader), but some boards don't
> >>> have `default-state` property in Linux kernel tree but have it in
> >>> U-Boot tree instead[1].
> >>>
> >>> This patch adds `default-state = "on"` for (almost) all LEDs (with a
> >>> few exceptions which should be "off" such as RGB LEDs on E25 and LAN/
> >>> WAN LEDs on E20C/E52C).
> >>
> >> I'm missing the *why* these changes would be an improvement.
> >>
> >> Personally, for both 'heartbeat' and 'netdev' triggers, I want them to
> >> be off by default and once it gets a 'heartbeat' or a 'netdev' trigger,
> >> THEN I want the LED to be on/blinking.
> >
> > That's a good question for Naoki. My own preference would also
> > be to have the device's power LED turned on by U-Boot as quickly
> > as possible after supplying power to the board or turning it on
> > by pressing the power button. I'm actually not a big fan of
> > having all the LEDs shining for a couple of seconds or so, which
> > may actually look like some error condition to me.
> >
> > Having all that in mind, I may suggest that just the U-Boot's
> > behavior is changed to turn the power LEDs on only.
>
> I can't quite explain it, but...
>
> - 1st (Power) LED
>
> The 1st (power) LED turns on automatically/immediately without software
> intervention. (On some boards, this LED cannot be controlled by software
> at all.)
>
> In DTS, this should be described using `default-state = "on"`. The use
> of the Linux-specific property `linux,default-trigger = "default-on"` is
> unsuitable for non-Linux environments.
>
> - 2nd (Heartbeat) LED
>
> The 2nd (heartbeat) LED can be controlled by software. It should be lit
> up as quickly as possible to indicate that the very first software
> (e.g., the bootloader) is running.
>
> On Linux, usually this is used as `linux,default-trigger = "heartbeat"`.
> It indicates that kernel is running (regardless of the `default-state`
> setting), and its behavior can be modified in user space.
As discussed already in the #linux-rockchip IRC channel, [1] perhaps
the best option would be to have the power LEDs turned on as quickly
upon powering on the board as possible, and to have U-Boot pulsate
the heartbeat LEDs using the LED_BOOT feature. In such a scenario,
no other LEDs would be turned on early, and the LED-related DT parts
specific to U-Boot would be migrated to the kernel DTs.
[1] https://libera.catirclogs.org/linux-rockchip/2025-11-11#38997824;
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards
2025-11-11 16:14 ` Dragan Simic
@ 2025-11-11 18:32 ` Quentin Schulz
2025-11-11 23:42 ` FUKAUMI Naoki
0 siblings, 1 reply; 13+ messages in thread
From: Quentin Schulz @ 2025-11-11 18:32 UTC (permalink / raw)
To: Dragan Simic, FUKAUMI Naoki
Cc: Diederik de Haas, heiko, robh, krzk+dt, conor+dt, jbx6244,
pgwipeout, jonas, ziyao, amadeus, nicolas.frattaroli, pbrobinson,
wens, detlev.casanova, stephen, sebastian.reichel,
liujianfeng1994, andy.yan, damon.ding, kylepzak, devicetree,
linux-rockchip
Hi all,
On 11/11/25 5:14 PM, Dragan Simic wrote:
> Hello all,
>
> (+ Quentin)
>
> On Tuesday, November 11, 2025 16:32 CET, FUKAUMI Naoki <naoki@radxa.com> wrote:
>> On 11/11/25 23:46, Dragan Simic wrote:
>>> On Tuesday, November 11, 2025 14:07 CET, "Diederik de Haas" <diederik@cknow-tech.com> wrote:
>>>> On Tue Nov 11, 2025 at 6:41 AM CET, FUKAUMI Naoki wrote:
>>>>> Radxa's boards turn all LEDs on at boot(loader), but some boards don't
>>>>> have `default-state` property in Linux kernel tree but have it in
>>>>> U-Boot tree instead[1].
>>>>>
>>>>> This patch adds `default-state = "on"` for (almost) all LEDs (with a
>>>>> few exceptions which should be "off" such as RGB LEDs on E25 and LAN/
>>>>> WAN LEDs on E20C/E52C).
>>>>
>>>> I'm missing the *why* these changes would be an improvement.
>>>>
>>>> Personally, for both 'heartbeat' and 'netdev' triggers, I want them to
>>>> be off by default and once it gets a 'heartbeat' or a 'netdev' trigger,
>>>> THEN I want the LED to be on/blinking.
>>>
>>> That's a good question for Naoki. My own preference would also
>>> be to have the device's power LED turned on by U-Boot as quickly
>>> as possible after supplying power to the board or turning it on
>>> by pressing the power button. I'm actually not a big fan of
>>> having all the LEDs shining for a couple of seconds or so, which
>>> may actually look like some error condition to me.
>>>
>>> Having all that in mind, I may suggest that just the U-Boot's
>>> behavior is changed to turn the power LEDs on only.
>>
>> I can't quite explain it, but...
>>
>> - 1st (Power) LED
>>
>> The 1st (power) LED turns on automatically/immediately without software
>> intervention. (On some boards, this LED cannot be controlled by software
>> at all.)
>>
>> In DTS, this should be described using `default-state = "on"`. The use
>> of the Linux-specific property `linux,default-trigger = "default-on"` is
>> unsuitable for non-Linux environments.
>>
>> - 2nd (Heartbeat) LED
>>
>> The 2nd (heartbeat) LED can be controlled by software. It should be lit
>> up as quickly as possible to indicate that the very first software
>> (e.g., the bootloader) is running.
>>
>> On Linux, usually this is used as `linux,default-trigger = "heartbeat"`.
>> It indicates that kernel is running (regardless of the `default-state`
>> setting), and its behavior can be modified in user space.
>
> As discussed already in the #linux-rockchip IRC channel, [1] perhaps
> the best option would be to have the power LEDs turned on as quickly
> upon powering on the board as possible, and to have U-Boot pulsate
> the heartbeat LEDs using the LED_BOOT feature. In such a scenario,
> no other LEDs would be turned on early, and the LED-related DT parts
> specific to U-Boot would be migrated to the kernel DTs.
>
> [1] https://libera.catirclogs.org/linux-rockchip/2025-11-11#38997824;
>
The LED_BOOT feature (guarded by the Kconfig symbol of the same name) in
U-Boot only applies if /options/u-boot/boot-led property is set.
If the driver for the LED (typically a GPIO LED controller I guess, so
LED_GPIO symbol) is compiled in, then, as far as I could tell, the Boot
LED will be turned on right before entering the main loop of U-Boot.
If LED_BLINK (if HW blinking is supported) or LED_SW_BLINK is enabled,
the Boot LED will be blinking some time after relocation but still
turned on soon after (if it reaches that part of the code). This means
it'll be on before the kernel starts.
I'm not sure there's a way to hook something *after* we've entered
U-Boot main-loop (read "call led_boot_blink() from some board file"),
aside from calling `led <led-name> blink <period>` from U-Boot CLI.
I'm a bit bummed by this behavior, I would have preferred the ability to
have the Boot LED blink until the kernel starts. I could then have a
different period for U-Boot (50% duty cycle at 250ms period by default)
and for the kernel. Of course, if it's SW blinking, once exiting U-Boot
it won't blink anymore until the kernel takes over, but that's also a
nice information to have. Anyway, I'm not sure this is actually possible
with the LED_BOOT feature though one should be able to do this by
specifying the label of an LED node to fetch from DT and then calling
led_set_period(dev, period_ms); followed by led_set_state(dev,
LEDST_BLINK); in a board file, but this is also not so nice as it then
also requires some C board-specific code in U-Boot.
In U-Boot, only LEDs which have a "default-state" properties will be
auto-configured, otherwise one needs to control them manually (e.g. via
the `led` CLI command).
If one wants to detect via an LED the current boot stage (U-Boot
reached, kernel started), then we need to NOT use LED_BOOT feature and
have U-Boot set the "boot" LED the opposite state than the default HW
state, i.e. if the LED is on without any running SW (power applied to
the device, empty boot media), then U-Boot should set it to off. Then
the kernel simply needs to start the heartbeat mode whenever ready. If
the default HW state is off, then U-Boot should set it on. I haven't
looked into the kernel side of things, but there could be a window
during which default-state property is applied before the heartbeat is
actually started.
The logic exposed in the previous paragraph should provide visual cues
on the current boot stage.
Note that LEDs with linux,default-trigger = "pattern" (with
default-state property) will be blinking once auto-configured in U-Boot
as well according to my reading of the led-uclass.c.
Hope this helps.
Cheers,
Quentin
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards
2025-11-11 18:32 ` Quentin Schulz
@ 2025-11-11 23:42 ` FUKAUMI Naoki
2025-11-12 9:40 ` Diederik de Haas
0 siblings, 1 reply; 13+ messages in thread
From: FUKAUMI Naoki @ 2025-11-11 23:42 UTC (permalink / raw)
To: Quentin Schulz, Dragan Simic
Cc: Diederik de Haas, heiko, robh, krzk+dt, conor+dt, jbx6244,
pgwipeout, jonas, ziyao, amadeus, nicolas.frattaroli, pbrobinson,
wens, detlev.casanova, stephen, sebastian.reichel,
liujianfeng1994, andy.yan, damon.ding, kylepzak, devicetree,
linux-rockchip
Hi Quentin,
On 11/12/25 03:32, Quentin Schulz wrote:
> Hi all,
>
> On 11/11/25 5:14 PM, Dragan Simic wrote:
>> Hello all,
>>
>> (+ Quentin)
>>
>> On Tuesday, November 11, 2025 16:32 CET, FUKAUMI Naoki
>> <naoki@radxa.com> wrote:
>>> On 11/11/25 23:46, Dragan Simic wrote:
>>>> On Tuesday, November 11, 2025 14:07 CET, "Diederik de Haas"
>>>> <diederik@cknow-tech.com> wrote:
>>>>> On Tue Nov 11, 2025 at 6:41 AM CET, FUKAUMI Naoki wrote:
>>>>>> Radxa's boards turn all LEDs on at boot(loader), but some boards
>>>>>> don't
>>>>>> have `default-state` property in Linux kernel tree but have it in
>>>>>> U-Boot tree instead[1].
>>>>>>
>>>>>> This patch adds `default-state = "on"` for (almost) all LEDs (with a
>>>>>> few exceptions which should be "off" such as RGB LEDs on E25 and LAN/
>>>>>> WAN LEDs on E20C/E52C).
>>>>>
>>>>> I'm missing the *why* these changes would be an improvement.
>>>>>
>>>>> Personally, for both 'heartbeat' and 'netdev' triggers, I want them to
>>>>> be off by default and once it gets a 'heartbeat' or a 'netdev'
>>>>> trigger,
>>>>> THEN I want the LED to be on/blinking.
>>>>
>>>> That's a good question for Naoki. My own preference would also
>>>> be to have the device's power LED turned on by U-Boot as quickly
>>>> as possible after supplying power to the board or turning it on
>>>> by pressing the power button. I'm actually not a big fan of
>>>> having all the LEDs shining for a couple of seconds or so, which
>>>> may actually look like some error condition to me.
>>>>
>>>> Having all that in mind, I may suggest that just the U-Boot's
>>>> behavior is changed to turn the power LEDs on only.
>>>
>>> I can't quite explain it, but...
>>>
>>> - 1st (Power) LED
>>>
>>> The 1st (power) LED turns on automatically/immediately without software
>>> intervention. (On some boards, this LED cannot be controlled by software
>>> at all.)
>>>
>>> In DTS, this should be described using `default-state = "on"`. The use
>>> of the Linux-specific property `linux,default-trigger = "default-on"` is
>>> unsuitable for non-Linux environments.
>>>
>>> - 2nd (Heartbeat) LED
>>>
>>> The 2nd (heartbeat) LED can be controlled by software. It should be lit
>>> up as quickly as possible to indicate that the very first software
>>> (e.g., the bootloader) is running.
>>>
>>> On Linux, usually this is used as `linux,default-trigger = "heartbeat"`.
>>> It indicates that kernel is running (regardless of the `default-state`
>>> setting), and its behavior can be modified in user space.
>>
>> As discussed already in the #linux-rockchip IRC channel, [1] perhaps
>> the best option would be to have the power LEDs turned on as quickly
>> upon powering on the board as possible, and to have U-Boot pulsate
>> the heartbeat LEDs using the LED_BOOT feature. In such a scenario,
>> no other LEDs would be turned on early, and the LED-related DT parts
>> specific to U-Boot would be migrated to the kernel DTs.
>>
>> [1] https://libera.catirclogs.org/linux-rockchip/2025-11-11#38997824;
>>
>
> The LED_BOOT feature (guarded by the Kconfig symbol of the same name) in
> U-Boot only applies if /options/u-boot/boot-led property is set.
For the default state of the heartbeat LED, I'm thinking of using
LED_BOOT (/options/u-boot/boot-led), but I'm concerned that this is
U-Boot-specific.
> If the driver for the LED (typically a GPIO LED controller I guess, so
> LED_GPIO symbol) is compiled in, then, as far as I could tell, the Boot
> LED will be turned on right before entering the main loop of U-Boot.
>
> If LED_BLINK (if HW blinking is supported) or LED_SW_BLINK is enabled,
> the Boot LED will be blinking some time after relocation but still
> turned on soon after (if it reaches that part of the code). This means
> it'll be on before the kernel starts.
>
> I'm not sure there's a way to hook something *after* we've entered U-
> Boot main-loop (read "call led_boot_blink() from some board file"),
> aside from calling `led <led-name> blink <period>` from U-Boot CLI.
>
> I'm a bit bummed by this behavior, I would have preferred the ability to
> have the Boot LED blink until the kernel starts. I could then have a
> different period for U-Boot (50% duty cycle at 250ms period by default)
> and for the kernel. Of course, if it's SW blinking, once exiting U-Boot
> it won't blink anymore until the kernel takes over, but that's also a
> nice information to have. Anyway, I'm not sure this is actually possible
> with the LED_BOOT feature though one should be able to do this by
> specifying the label of an LED node to fetch from DT and then calling
> led_set_period(dev, period_ms); followed by led_set_state(dev,
> LEDST_BLINK); in a board file, but this is also not so nice as it then
> also requires some C board-specific code in U-Boot.
>
> In U-Boot, only LEDs which have a "default-state" properties will be
> auto-configured, otherwise one needs to control them manually (e.g. via
> the `led` CLI command).
As you know, default "default-state" is "off".
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/leds/common.yaml?h=v6.17#n74
As far as I understand, there should not be any workarounds for specific
implementations.
https://lore.kernel.org/linux-rockchip/3389401.44csPzL39Z@phil/
So removing `default-state = "off"` is acceptable, right?
Best regards,
--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.
> If one wants to detect via an LED the current boot stage (U-Boot
> reached, kernel started), then we need to NOT use LED_BOOT feature and
> have U-Boot set the "boot" LED the opposite state than the default HW
> state, i.e. if the LED is on without any running SW (power applied to
> the device, empty boot media), then U-Boot should set it to off. Then
> the kernel simply needs to start the heartbeat mode whenever ready. If
> the default HW state is off, then U-Boot should set it on. I haven't
> looked into the kernel side of things, but there could be a window
> during which default-state property is applied before the heartbeat is
> actually started.
>
> The logic exposed in the previous paragraph should provide visual cues
> on the current boot stage.
>
> Note that LEDs with linux,default-trigger = "pattern" (with default-
> state property) will be blinking once auto-configured in U-Boot as well
> according to my reading of the led-uclass.c.
>
> Hope this helps.
>
> Cheers,
> Quentin
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards
2025-11-11 23:42 ` FUKAUMI Naoki
@ 2025-11-12 9:40 ` Diederik de Haas
2025-11-12 10:34 ` Quentin Schulz
0 siblings, 1 reply; 13+ messages in thread
From: Diederik de Haas @ 2025-11-12 9:40 UTC (permalink / raw)
To: FUKAUMI Naoki, Quentin Schulz, Dragan Simic
Cc: Diederik de Haas, heiko, robh, krzk+dt, conor+dt, jbx6244,
pgwipeout, jonas, ziyao, amadeus, nicolas.frattaroli, pbrobinson,
wens, detlev.casanova, stephen, sebastian.reichel,
liujianfeng1994, andy.yan, damon.ding, kylepzak, devicetree,
linux-rockchip
Hi Naoki,
On Wed Nov 12, 2025 at 12:42 AM CET, FUKAUMI Naoki wrote:
> On 11/12/25 03:32, Quentin Schulz wrote:
>> On 11/11/25 5:14 PM, Dragan Simic wrote:
>>> On Tuesday, November 11, 2025 16:32 CET, FUKAUMI Naoki
>>> <naoki@radxa.com> wrote:
>>>> On 11/11/25 23:46, Dragan Simic wrote:
>>>>> On Tuesday, November 11, 2025 14:07 CET, "Diederik de Haas"
>>>>> <diederik@cknow-tech.com> wrote:
>>>>>> On Tue Nov 11, 2025 at 6:41 AM CET, FUKAUMI Naoki wrote:
>>>>>>> Radxa's boards turn all LEDs on at boot(loader), but some boards
>>>>>>> don't have `default-state` property in Linux kernel tree but
>>>>>>> have it in U-Boot tree instead[1].
>>>>>>>
>>>>>>> This patch adds `default-state = "on"` for (almost) all LEDs (with a
>>>>>>> few exceptions which should be "off" such as RGB LEDs on E25 and LAN/
>>>>>>> WAN LEDs on E20C/E52C).
>>>>>>
>>>>>> I'm missing the *why* these changes would be an improvement.
>>>>>>
>>>>>> Personally, for both 'heartbeat' and 'netdev' triggers, I want them to
>>>>>> be off by default and once it gets a 'heartbeat' or a 'netdev'
>>>>>> trigger, THEN I want the LED to be on/blinking.
>>>>>
>>>>> That's a good question for Naoki. My own preference would also
>>>>> be to have the device's power LED turned on by U-Boot as quickly
>>>>> as possible after supplying power to the board or turning it on
>>>>> by pressing the power button. I'm actually not a big fan of
>>>>> having all the LEDs shining for a couple of seconds or so, which
>>>>> may actually look like some error condition to me.
>>>>>
>>>>> Having all that in mind, I may suggest that just the U-Boot's
>>>>> behavior is changed to turn the power LEDs on only.
>>>>
>>>> I can't quite explain it, but...
>>>>
>>>> - 1st (Power) LED
>>>>
>>>> The 1st (power) LED turns on automatically/immediately without software
>>>> intervention. (On some boards, this LED cannot be controlled by software
>>>> at all.)
>>>>
>>>> In DTS, this should be described using `default-state = "on"`. The use
>>>> of the Linux-specific property `linux,default-trigger = "default-on"` is
>>>> unsuitable for non-Linux environments.
>>>>
>>>> - 2nd (Heartbeat) LED
>>>>
>>>> The 2nd (heartbeat) LED can be controlled by software. It should be lit
>>>> up as quickly as possible to indicate that the very first software
>>>> (e.g., the bootloader) is running.
>>>>
>>>> On Linux, usually this is used as `linux,default-trigger = "heartbeat"`.
>>>> It indicates that kernel is running (regardless of the `default-state`
>>>> setting), and its behavior can be modified in user space.
>>>
>>> As discussed already in the #linux-rockchip IRC channel, [1] perhaps
>>> the best option would be to have the power LEDs turned on as quickly
>>> upon powering on the board as possible, and to have U-Boot pulsate
>>> the heartbeat LEDs using the LED_BOOT feature. In such a scenario,
>>> no other LEDs would be turned on early, and the LED-related DT parts
>>> specific to U-Boot would be migrated to the kernel DTs.
>>>
>>> [1] https://libera.catirclogs.org/linux-rockchip/2025-11-11#38997824;
>>
>> The LED_BOOT feature (guarded by the Kconfig symbol of the same name) in
>> U-Boot only applies if /options/u-boot/boot-led property is set.
>
> For the default state of the heartbeat LED, I'm thinking of using
> LED_BOOT (/options/u-boot/boot-led), but I'm concerned that this is
> U-Boot-specific.
If U-Boot wants to use the heartbeat LED to signal the *bootloader* is
running, I guess that's fine. And if you want to make it solid or
blinking, that seems best discussed on the U-Boot ML.
I still consider the bootloader and the kernel stages separate.
And I haven't seen an argument why I should change *my* opinion on the
heartbeat and netdev triggers (default-state) wrt the kernel.
I don't think that what U-Boot does or doesn't do, should determine what
the Linux kernel does or doesn't do.
I have no plans to use another bootloader then U-Boot, but it's possible
that people do, so what the Linux kernel does should be independent from
what the/a specific bootloader does.
And as I said before, *I* want LEDs with netdev and heartbeat triggers,
to be off (at the start, which is indeed the default value).
I use the heartbeat trigger to:
1) See the kernel has started (and has gotten to the point the heartbeat
'infrastructure' has been set up
2) Wait for the blinking to slow down as that (generally) means it's
pretty much done with the boot process and the SSH server should
probably be running then, so I can login
3) When the heartbeat LED is solid, that means the system has crashed
(f.e. due to overheating ...)
And also, if you're going to change/override other people's choices, a
motivation as to why would be 'nice'.
>> <more discussion about LED functionality in U-Boot ...>
>
> As you know, default "default-state" is "off".
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/leds/common.yaml?h=v6.17#n74
>
> As far as I understand, there should not be any workarounds for specific
> implementations.
> https://lore.kernel.org/linux-rockchip/3389401.44csPzL39Z@phil/
>
> So removing `default-state = "off"` is acceptable, right?
I don't see/understand the connection with 'workarounds for specific
implementations' with removing ``default-state = "off"``.
IMO it's perfectly fine to remove ``default-state = "off"``, although
having it explicitly may be useful, especially if the commit that set
that property specified *why* it should be "off".
Relatedly, when a node does not have the 'default-state' property, I
would _assume_ the author wanted/intended it to be "off". Ideally it
would be described in the commit message, but that is optional.
But if that is changed, then it should be motivated *why*.
Cheers,
Diederik
>
> Best regards,
>
> --
> FUKAUMI Naoki
> Radxa Computer (Shenzhen) Co., Ltd.
>
>> If one wants to detect via an LED the current boot stage (U-Boot
>> reached, kernel started), then we need to NOT use LED_BOOT feature and
>> have U-Boot set the "boot" LED the opposite state than the default HW
>> state, i.e. if the LED is on without any running SW (power applied to
>> the device, empty boot media), then U-Boot should set it to off. Then
>> the kernel simply needs to start the heartbeat mode whenever ready. If
>> the default HW state is off, then U-Boot should set it on. I haven't
>> looked into the kernel side of things, but there could be a window
>> during which default-state property is applied before the heartbeat is
>> actually started.
>>
>> The logic exposed in the previous paragraph should provide visual cues
>> on the current boot stage.
>>
>> Note that LEDs with linux,default-trigger = "pattern" (with default-
>> state property) will be blinking once auto-configured in U-Boot as well
>> according to my reading of the led-uclass.c.
>>
>> Hope this helps.
>>
>> Cheers,
>> Quentin
>>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards
2025-11-12 9:40 ` Diederik de Haas
@ 2025-11-12 10:34 ` Quentin Schulz
2025-11-12 11:36 ` FUKAUMI Naoki
0 siblings, 1 reply; 13+ messages in thread
From: Quentin Schulz @ 2025-11-12 10:34 UTC (permalink / raw)
To: Diederik de Haas, FUKAUMI Naoki, Dragan Simic
Cc: heiko, robh, krzk+dt, conor+dt, jbx6244, pgwipeout, jonas, ziyao,
amadeus, nicolas.frattaroli, pbrobinson, wens, detlev.casanova,
stephen, sebastian.reichel, liujianfeng1994, andy.yan, damon.ding,
kylepzak, devicetree, linux-rockchip
Hi all,
On 11/12/25 10:40 AM, Diederik de Haas wrote:
> [You don't often get email from diederik@cknow-tech.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Hi Naoki,
>
> On Wed Nov 12, 2025 at 12:42 AM CET, FUKAUMI Naoki wrote:
>> On 11/12/25 03:32, Quentin Schulz wrote:
>>> On 11/11/25 5:14 PM, Dragan Simic wrote:
>>>> On Tuesday, November 11, 2025 16:32 CET, FUKAUMI Naoki
>>>> <naoki@radxa.com> wrote:
>>>>> On 11/11/25 23:46, Dragan Simic wrote:
>>>>>> On Tuesday, November 11, 2025 14:07 CET, "Diederik de Haas"
>>>>>> <diederik@cknow-tech.com> wrote:
>>>>>>> On Tue Nov 11, 2025 at 6:41 AM CET, FUKAUMI Naoki wrote:
>>>>>>>> Radxa's boards turn all LEDs on at boot(loader), but some boards
>>>>>>>> don't have `default-state` property in Linux kernel tree but
>>>>>>>> have it in U-Boot tree instead[1].
>>>>>>>>
>>>>>>>> This patch adds `default-state = "on"` for (almost) all LEDs (with a
>>>>>>>> few exceptions which should be "off" such as RGB LEDs on E25 and LAN/
>>>>>>>> WAN LEDs on E20C/E52C).
>>>>>>>
>>>>>>> I'm missing the *why* these changes would be an improvement.
>>>>>>>
>>>>>>> Personally, for both 'heartbeat' and 'netdev' triggers, I want them to
>>>>>>> be off by default and once it gets a 'heartbeat' or a 'netdev'
>>>>>>> trigger, THEN I want the LED to be on/blinking.
>>>>>>
>>>>>> That's a good question for Naoki. My own preference would also
>>>>>> be to have the device's power LED turned on by U-Boot as quickly
>>>>>> as possible after supplying power to the board or turning it on
>>>>>> by pressing the power button. I'm actually not a big fan of
>>>>>> having all the LEDs shining for a couple of seconds or so, which
>>>>>> may actually look like some error condition to me.
>>>>>>
>>>>>> Having all that in mind, I may suggest that just the U-Boot's
>>>>>> behavior is changed to turn the power LEDs on only.
>>>>>
>>>>> I can't quite explain it, but...
>>>>>
>>>>> - 1st (Power) LED
>>>>>
>>>>> The 1st (power) LED turns on automatically/immediately without software
>>>>> intervention. (On some boards, this LED cannot be controlled by software
>>>>> at all.)
>>>>>
>>>>> In DTS, this should be described using `default-state = "on"`. The use
>>>>> of the Linux-specific property `linux,default-trigger = "default-on"` is
>>>>> unsuitable for non-Linux environments.
>>>>>
I think the wording in the binding can be understood two ways.
The binding says the following about the default-state property:
"""
The initial state of the LED. If the LED is already on or off and the
default-state property is set the to same value, then no glitch
should be
produced where the LED momentarily turns off (or on). The "keep"
setting
will keep the LED at whatever its current state is, without
producing a
glitch.
"""
I think the issue here is around the meaning of "initial state". I
believe Naoki is probably thinking about the **HW** initial state of the
LED, which is whatever is the state of the LED without SW control. I
think Diederik is thinking about this being the state of the LED right
when the SW takes over and configures the LED before the trigger is setup.
In the first interpretation, there's no need for an "improvement" for
the patches as they would just fix correctness of the DT wrt HW state at
boot.
In the second interpretation, a change of this value must be justified
as people will simply disagree forever and we could end up with people
reverting other people's patches after each release. If it's just a
matter of taste, I believe the typical answer is keeping the status quo.
We should find a way to make this binding not up to interpretation.
Additionally, if the LED cannot be controlled on some boards, I don't
think it should be part of the DT.
>>>>> - 2nd (Heartbeat) LED
>>>>>
>>>>> The 2nd (heartbeat) LED can be controlled by software. It should be lit
>>>>> up as quickly as possible to indicate that the very first software
>>>>> (e.g., the bootloader) is running.
>>>>>
My understanding is Naoki wants to use default-state = on, for the
bootloader to turn it on as soon as it takes over control of the LEDs.
>>>>> On Linux, usually this is used as `linux,default-trigger = "heartbeat"`.
>>>>> It indicates that kernel is running (regardless of the `default-state`
>>>>> setting), and its behavior can be modified in user space.
>>>>
>>>> As discussed already in the #linux-rockchip IRC channel, [1] perhaps
>>>> the best option would be to have the power LEDs turned on as quickly
>>>> upon powering on the board as possible, and to have U-Boot pulsate
>>>> the heartbeat LEDs using the LED_BOOT feature. In such a scenario,
>>>> no other LEDs would be turned on early, and the LED-related DT parts
>>>> specific to U-Boot would be migrated to the kernel DTs.
>>>>
>>>> [1] https://libera.catirclogs.org/linux-rockchip/2025-11-11#38997824;
>>>
>>> The LED_BOOT feature (guarded by the Kconfig symbol of the same name) in
>>> U-Boot only applies if /options/u-boot/boot-led property is set.
>>
>> For the default state of the heartbeat LED, I'm thinking of using
>> LED_BOOT (/options/u-boot/boot-led), but I'm concerned that this is
>> U-Boot-specific.
>
> If U-Boot wants to use the heartbeat LED to signal the *bootloader* is
> running, I guess that's fine. And if you want to make it solid or
> blinking, that seems best discussed on the U-Boot ML.
>
The solution may still involve configuring the Device Tree, and we're
trying to have U-Boot-specific changes to the Device Tree in U-Boot
source tree to a minimum.
> I still consider the bootloader and the kernel stages separate.
They do however share most of their Device Tree (for Rockchip at least)
and the least (ideally no) changes we can have in U-Boot the better.
> And I haven't seen an argument why I should change *my* opinion on the
> heartbeat and netdev triggers (default-state) wrt the kernel.
>
Device Tree is not kernel specific as you said already.
> I don't think that what U-Boot does or doesn't do, should determine what
> the Linux kernel does or doesn't do.
It shouldn't, but (most of) the Device Tree is shared, so you cannot
just dismiss U-Boot behavior when talking about Linux behavior based on
Device Tree interpretation. We may have a need for a bootloader-specific
property. We have a Linux-specific one after all
(linux,default-trigger). Though... that does seem to be on the edge of
what the DT is made for (description of the HW, not logic/policy).
> I have no plans to use another bootloader then U-Boot, but it's possible
> that people do, so what the Linux kernel does should be independent from
> what the/a specific bootloader does.
>
Barebox also uses upstream DT as far as I know and supports some Radxa
products (Rock 5B/5T/..., CM3, Rock (RK3188), Rock 3A from the
arch/arm/boards/radxa-* directories). Zephyr has support for RK3568,
RK3588, and other SoCs, and uses upstream DT as well.
Again, we're talking about modifications of the Device Tree here, so
typically I would expect all consumers of that DT to be interpreting the
properties the same way, except if you have OS-specific properties/nodes
(think u-boot,config-compatible nodes, linux, prefixed properties,
bootph- properties, ...).
> And as I said before, *I* want LEDs with netdev and heartbeat triggers,
> to be off (at the start, which is indeed the default value).
> I use the heartbeat trigger to:
> 1) See the kernel has started (and has gotten to the point the heartbeat
> 'infrastructure' has been set up
> 2) Wait for the blinking to slow down as that (generally) means it's
> pretty much done with the boot process and the SSH server should
> probably be running then, so I can login
> 3) When the heartbeat LED is solid, that means the system has crashed
> (f.e. due to overheating ...)
>
If the *HW* default state of the LED is off and the default-state
property is off, then you won't be able to tell apart a completely
bricked board and one that is stuck somewhere between U-Boot proper and
the Linux kernel taking over that LED.
> And also, if you're going to change/override other people's choices, a
> motivation as to why would be 'nice'.
>
>>> <more discussion about LED functionality in U-Boot ...>
>>
>> As you know, default "default-state" is "off".
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/leds/common.yaml?h=v6.17#n74
>>
>> As far as I understand, there should not be any workarounds for specific
>> implementations.
>> https://lore.kernel.org/linux-rockchip/3389401.44csPzL39Z@phil/
>>
>> So removing `default-state = "off"` is acceptable, right?
>
> I don't see/understand the connection with 'workarounds for specific
> implementations' with removing ``default-state = "off"``.
>
> IMO it's perfectly fine to remove ``default-state = "off"``, although
> having it explicitly may be useful, especially if the commit that set
> that property specified *why* it should be "off".
>
The status property defaults to okay, and we do not want them to be
listed explicitly. Not sure if there's consensus on applying this to all
properties which have defaults, across all subsystems.
> Relatedly, when a node does not have the 'default-state' property, I
> would _assume_ the author wanted/intended it to be "off". Ideally it
> would be described in the commit message, but that is optional.
The lack of a property doesn't necessarily mean it was forgotten, agreed.
> But if that is changed, then it should be motivated *why*.
>
Agreed.
Cheers,
Quentin
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards
2025-11-12 10:34 ` Quentin Schulz
@ 2025-11-12 11:36 ` FUKAUMI Naoki
2025-11-12 13:21 ` Diederik de Haas
0 siblings, 1 reply; 13+ messages in thread
From: FUKAUMI Naoki @ 2025-11-12 11:36 UTC (permalink / raw)
To: Quentin Schulz, Diederik de Haas, Dragan Simic
Cc: heiko, robh, krzk+dt, conor+dt, jbx6244, pgwipeout, jonas, ziyao,
amadeus, nicolas.frattaroli, pbrobinson, wens, detlev.casanova,
stephen, sebastian.reichel, liujianfeng1994, andy.yan, damon.ding,
kylepzak, devicetree, linux-rockchip
Hi all,
My goal is to minimize the DTS fragments
(u-boot/arch/arm/dts/*-u-boot.dtsi) in U-Boot, consolidate them
upstream, and improve clarity/visibility.
On 11/12/25 19:34, Quentin Schulz wrote:
> Hi all,
>
> On 11/12/25 10:40 AM, Diederik de Haas wrote:
>> [You don't often get email from diederik@cknow-tech.com. Learn why
>> this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> Hi Naoki,
>>
>> On Wed Nov 12, 2025 at 12:42 AM CET, FUKAUMI Naoki wrote:
>>> On 11/12/25 03:32, Quentin Schulz wrote:
>>>> On 11/11/25 5:14 PM, Dragan Simic wrote:
>>>>> On Tuesday, November 11, 2025 16:32 CET, FUKAUMI Naoki
>>>>> <naoki@radxa.com> wrote:
>>>>>> On 11/11/25 23:46, Dragan Simic wrote:
>>>>>>> On Tuesday, November 11, 2025 14:07 CET, "Diederik de Haas"
>>>>>>> <diederik@cknow-tech.com> wrote:
>>>>>>>> On Tue Nov 11, 2025 at 6:41 AM CET, FUKAUMI Naoki wrote:
>>>>>>>>> Radxa's boards turn all LEDs on at boot(loader), but some boards
>>>>>>>>> don't have `default-state` property in Linux kernel tree but
>>>>>>>>> have it in U-Boot tree instead[1].
>>>>>>>>>
>>>>>>>>> This patch adds `default-state = "on"` for (almost) all LEDs
>>>>>>>>> (with a
>>>>>>>>> few exceptions which should be "off" such as RGB LEDs on E25
>>>>>>>>> and LAN/
>>>>>>>>> WAN LEDs on E20C/E52C).
>>>>>>>>
>>>>>>>> I'm missing the *why* these changes would be an improvement.
>>>>>>>>
>>>>>>>> Personally, for both 'heartbeat' and 'netdev' triggers, I want
>>>>>>>> them to
>>>>>>>> be off by default and once it gets a 'heartbeat' or a 'netdev'
>>>>>>>> trigger, THEN I want the LED to be on/blinking.
>>>>>>>
>>>>>>> That's a good question for Naoki. My own preference would also
>>>>>>> be to have the device's power LED turned on by U-Boot as quickly
>>>>>>> as possible after supplying power to the board or turning it on
>>>>>>> by pressing the power button. I'm actually not a big fan of
>>>>>>> having all the LEDs shining for a couple of seconds or so, which
>>>>>>> may actually look like some error condition to me.
>>>>>>>
>>>>>>> Having all that in mind, I may suggest that just the U-Boot's
>>>>>>> behavior is changed to turn the power LEDs on only.
>>>>>>
>>>>>> I can't quite explain it, but...
>>>>>>
>>>>>> - 1st (Power) LED
>>>>>>
>>>>>> The 1st (power) LED turns on automatically/immediately without
>>>>>> software
>>>>>> intervention. (On some boards, this LED cannot be controlled by
>>>>>> software
>>>>>> at all.)
I'm not saying the DTS has anything about LEDs that can't be controlled
by software, nor am I trying to add such a thing to the DTS.
I'm just pointing out that the power LED is always on right after
power-up. This makes it useless for determining if the software is running.
>>>>>> In DTS, this should be described using `default-state = "on"`. The
>>>>>> use
>>>>>> of the Linux-specific property `linux,default-trigger = "default-
>>>>>> on"` is
>>>>>> unsuitable for non-Linux environments.
>>>>>>
>
> I think the wording in the binding can be understood two ways.
>
> The binding says the following about the default-state property:
>
> """
> The initial state of the LED. If the LED is already on or off and
> the
> default-state property is set the to same value, then no glitch
> should be
> produced where the LED momentarily turns off (or on). The "keep"
> setting
> will keep the LED at whatever its current state is, without
> producing a
> glitch.
> """
>
> I think the issue here is around the meaning of "initial state". I
> believe Naoki is probably thinking about the **HW** initial state of the
> LED, which is whatever is the state of the LED without SW control. I
> think Diederik is thinking about this being the state of the LED right
> when the SW takes over and configures the LED before the trigger is setup.
>
> In the first interpretation, there's no need for an "improvement" for
> the patches as they would just fix correctness of the DT wrt HW state at
> boot.
>
> In the second interpretation, a change of this value must be justified
> as people will simply disagree forever and we could end up with people
> reverting other people's patches after each release. If it's just a
> matter of taste, I believe the typical answer is keeping the status quo.
>
> We should find a way to make this binding not up to interpretation.
>
> Additionally, if the LED cannot be controlled on some boards, I don't
> think it should be part of the DT.
>
>>>>>> - 2nd (Heartbeat) LED
>>>>>>
>>>>>> The 2nd (heartbeat) LED can be controlled by software. It should
>>>>>> be lit
>>>>>> up as quickly as possible to indicate that the very first software
>>>>>> (e.g., the bootloader) is running.
>>>>>>
>
> My understanding is Naoki wants to use default-state = on, for the
> bootloader to turn it on as soon as it takes over control of the LEDs.
>
>>>>>> On Linux, usually this is used as `linux,default-trigger =
>>>>>> "heartbeat"`.
>>>>>> It indicates that kernel is running (regardless of the `default-
>>>>>> state`
>>>>>> setting), and its behavior can be modified in user space.
>>>>>
>>>>> As discussed already in the #linux-rockchip IRC channel, [1] perhaps
>>>>> the best option would be to have the power LEDs turned on as quickly
>>>>> upon powering on the board as possible, and to have U-Boot pulsate
>>>>> the heartbeat LEDs using the LED_BOOT feature. In such a scenario,
>>>>> no other LEDs would be turned on early, and the LED-related DT parts
>>>>> specific to U-Boot would be migrated to the kernel DTs.
>>>>>
>>>>> [1] https://libera.catirclogs.org/linux-rockchip/2025-11-11#38997824;
>>>>
>>>> The LED_BOOT feature (guarded by the Kconfig symbol of the same
>>>> name) in
>>>> U-Boot only applies if /options/u-boot/boot-led property is set.
>>>
>>> For the default state of the heartbeat LED, I'm thinking of using
>>> LED_BOOT (/options/u-boot/boot-led), but I'm concerned that this is
>>> U-Boot-specific.
>>
>> If U-Boot wants to use the heartbeat LED to signal the *bootloader* is
>> running, I guess that's fine. And if you want to make it solid or
>> blinking, that seems best discussed on the U-Boot ML.
>>
>
> The solution may still involve configuring the Device Tree, and we're
> trying to have U-Boot-specific changes to the Device Tree in U-Boot
> source tree to a minimum.
>
>> I still consider the bootloader and the kernel stages separate.
>
> They do however share most of their Device Tree (for Rockchip at least)
> and the least (ideally no) changes we can have in U-Boot the better.
>
>> And I haven't seen an argument why I should change *my* opinion on the
>> heartbeat and netdev triggers (default-state) wrt the kernel.
>>
>
> Device Tree is not kernel specific as you said already.
>
>> I don't think that what U-Boot does or doesn't do, should determine what
>> the Linux kernel does or doesn't do.
>
> It shouldn't, but (most of) the Device Tree is shared, so you cannot
> just dismiss U-Boot behavior when talking about Linux behavior based on
> Device Tree interpretation. We may have a need for a bootloader-specific
> property. We have a Linux-specific one after all (linux,default-
> trigger). Though... that does seem to be on the edge of what the DT is
> made for (description of the HW, not logic/policy).
>
>> I have no plans to use another bootloader then U-Boot, but it's possible
>> that people do, so what the Linux kernel does should be independent from
>> what the/a specific bootloader does.
Each software should be independent, but hardware (state) cannot be
independent.
> Barebox also uses upstream DT as far as I know and supports some Radxa
> products (Rock 5B/5T/..., CM3, Rock (RK3188), Rock 3A from the arch/arm/
> boards/radxa-* directories). Zephyr has support for RK3568, RK3588, and
> other SoCs, and uses upstream DT as well.
>
> Again, we're talking about modifications of the Device Tree here, so
> typically I would expect all consumers of that DT to be interpreting the
> properties the same way, except if you have OS-specific properties/nodes
> (think u-boot,config-compatible nodes, linux, prefixed properties,
> bootph- properties, ...).
>
>> And as I said before, *I* want LEDs with netdev and heartbeat triggers,
>> to be off (at the start, which is indeed the default value).
If you are using U-Boot, heartbeat LED is already on by U-Boot,
e.g.
https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi#L10-12
But it's not visible in DTS in Linux,
e.g.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts?h=v6.17#n55
I think this situation should be fixed.
Best regards,
--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.
>> I use the heartbeat trigger to:
>> 1) See the kernel has started (and has gotten to the point the heartbeat
>> 'infrastructure' has been set up
>> 2) Wait for the blinking to slow down as that (generally) means it's
>> pretty much done with the boot process and the SSH server should
>> probably be running then, so I can login
>> 3) When the heartbeat LED is solid, that means the system has crashed
>> (f.e. due to overheating ...)
>>
>
> If the *HW* default state of the LED is off and the default-state
> property is off, then you won't be able to tell apart a completely
> bricked board and one that is stuck somewhere between U-Boot proper and
> the Linux kernel taking over that LED.
>
>> And also, if you're going to change/override other people's choices, a
>> motivation as to why would be 'nice'.
>>
>>>> <more discussion about LED functionality in U-Boot ...>
>>>
>>> As you know, default "default-state" is "off".
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/
>>> linux.git/tree/Documentation/devicetree/bindings/leds/common.yaml?
>>> h=v6.17#n74
>>>
>>> As far as I understand, there should not be any workarounds for specific
>>> implementations.
>>> https://lore.kernel.org/linux-rockchip/3389401.44csPzL39Z@phil/
>>>
>>> So removing `default-state = "off"` is acceptable, right?
>>
>> I don't see/understand the connection with 'workarounds for specific
>> implementations' with removing ``default-state = "off"``.
>>
>> IMO it's perfectly fine to remove ``default-state = "off"``, although
>> having it explicitly may be useful, especially if the commit that set
>> that property specified *why* it should be "off".
>>
>
> The status property defaults to okay, and we do not want them to be
> listed explicitly. Not sure if there's consensus on applying this to all
> properties which have defaults, across all subsystems.
>
>> Relatedly, when a node does not have the 'default-state' property, I
>> would _assume_ the author wanted/intended it to be "off". Ideally it
>> would be described in the commit message, but that is optional.
>
> The lack of a property doesn't necessarily mean it was forgotten, agreed.
>
>> But if that is changed, then it should be motivated *why*.
>>
>
> Agreed.
>
> Cheers,
> Quentin
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards
2025-11-12 11:36 ` FUKAUMI Naoki
@ 2025-11-12 13:21 ` Diederik de Haas
0 siblings, 0 replies; 13+ messages in thread
From: Diederik de Haas @ 2025-11-12 13:21 UTC (permalink / raw)
To: FUKAUMI Naoki, Quentin Schulz, Diederik de Haas, Dragan Simic
Cc: heiko, robh, krzk+dt, conor+dt, jbx6244, pgwipeout, jonas, ziyao,
amadeus, nicolas.frattaroli, pbrobinson, wens, detlev.casanova,
stephen, sebastian.reichel, liujianfeng1994, andy.yan, damon.ding,
kylepzak, devicetree, linux-rockchip
Hi,
On Wed Nov 12, 2025 at 12:36 PM CET, FUKAUMI Naoki wrote:
> My goal is to minimize the DTS fragments
> (u-boot/arch/arm/dts/*-u-boot.dtsi) in U-Boot, consolidate them
> upstream, and improve clarity/visibility.
Long story short: I was wrong.
It was mostly Quentin's argument/reminder that the DT is about
describing the hardware.
I think Quentin's point about 'initial state' was right on point,
certainly from my perspective and where my logic error came from.
Cheers,
Diederik
> On 11/12/25 19:34, Quentin Schulz wrote:
>> On 11/12/25 10:40 AM, Diederik de Haas wrote:
>>> [You don't often get email from diederik@cknow-tech.com. Learn why
>>> this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>
>>> On Wed Nov 12, 2025 at 12:42 AM CET, FUKAUMI Naoki wrote:
>>>> On 11/12/25 03:32, Quentin Schulz wrote:
>>>>> On 11/11/25 5:14 PM, Dragan Simic wrote:
>>>>>> On Tuesday, November 11, 2025 16:32 CET, FUKAUMI Naoki
>>>>>> <naoki@radxa.com> wrote:
>>>>>>> On 11/11/25 23:46, Dragan Simic wrote:
>>>>>>>> On Tuesday, November 11, 2025 14:07 CET, "Diederik de Haas"
>>>>>>>> <diederik@cknow-tech.com> wrote:
>>>>>>>>> On Tue Nov 11, 2025 at 6:41 AM CET, FUKAUMI Naoki wrote:
>>>>>>>>>> Radxa's boards turn all LEDs on at boot(loader), but some boards
>>>>>>>>>> don't have `default-state` property in Linux kernel tree but
>>>>>>>>>> have it in U-Boot tree instead[1].
>>>>>>>>>>
>>>>>>>>>> This patch adds `default-state = "on"` for (almost) all LEDs
>>>>>>>>>> (with a
>>>>>>>>>> few exceptions which should be "off" such as RGB LEDs on E25
>>>>>>>>>> and LAN/
>>>>>>>>>> WAN LEDs on E20C/E52C).
>>>>>>>>>
>>>>>>>>> I'm missing the *why* these changes would be an improvement.
>>>>>>>>>
>>>>>>>>> Personally, for both 'heartbeat' and 'netdev' triggers, I want
>>>>>>>>> them to
>>>>>>>>> be off by default and once it gets a 'heartbeat' or a 'netdev'
>>>>>>>>> trigger, THEN I want the LED to be on/blinking.
>>>>>>>>
>>>>>>>> That's a good question for Naoki. My own preference would also
>>>>>>>> be to have the device's power LED turned on by U-Boot as quickly
>>>>>>>> as possible after supplying power to the board or turning it on
>>>>>>>> by pressing the power button. I'm actually not a big fan of
>>>>>>>> having all the LEDs shining for a couple of seconds or so, which
>>>>>>>> may actually look like some error condition to me.
>>>>>>>>
>>>>>>>> Having all that in mind, I may suggest that just the U-Boot's
>>>>>>>> behavior is changed to turn the power LEDs on only.
>>>>>>>
>>>>>>> I can't quite explain it, but...
>>>>>>>
>>>>>>> - 1st (Power) LED
>>>>>>>
>>>>>>> The 1st (power) LED turns on automatically/immediately without
>>>>>>> software
>>>>>>> intervention. (On some boards, this LED cannot be controlled by
>>>>>>> software
>>>>>>> at all.)
>
> I'm not saying the DTS has anything about LEDs that can't be controlled
> by software, nor am I trying to add such a thing to the DTS.
>
> I'm just pointing out that the power LED is always on right after
> power-up. This makes it useless for determining if the software is running.
>
>>>>>>> In DTS, this should be described using `default-state = "on"`. The
>>>>>>> use
>>>>>>> of the Linux-specific property `linux,default-trigger = "default-
>>>>>>> on"` is
>>>>>>> unsuitable for non-Linux environments.
>>>>>>>
>>
>> I think the wording in the binding can be understood two ways.
>>
>> The binding says the following about the default-state property:
>>
>> """
>> The initial state of the LED. If the LED is already on or off and
>> the
>> default-state property is set the to same value, then no glitch
>> should be
>> produced where the LED momentarily turns off (or on). The "keep"
>> setting
>> will keep the LED at whatever its current state is, without
>> producing a
>> glitch.
>> """
>>
>> I think the issue here is around the meaning of "initial state". I
>> believe Naoki is probably thinking about the **HW** initial state of the
>> LED, which is whatever is the state of the LED without SW control. I
>> think Diederik is thinking about this being the state of the LED right
>> when the SW takes over and configures the LED before the trigger is setup.
>>
>> In the first interpretation, there's no need for an "improvement" for
>> the patches as they would just fix correctness of the DT wrt HW state at
>> boot.
>>
>> In the second interpretation, a change of this value must be justified
>> as people will simply disagree forever and we could end up with people
>> reverting other people's patches after each release. If it's just a
>> matter of taste, I believe the typical answer is keeping the status quo.
>>
>> We should find a way to make this binding not up to interpretation.
>>
>> Additionally, if the LED cannot be controlled on some boards, I don't
>> think it should be part of the DT.
>>
>>>>>>> - 2nd (Heartbeat) LED
>>>>>>>
>>>>>>> The 2nd (heartbeat) LED can be controlled by software. It should
>>>>>>> be lit
>>>>>>> up as quickly as possible to indicate that the very first software
>>>>>>> (e.g., the bootloader) is running.
>>>>>>>
>>
>> My understanding is Naoki wants to use default-state = on, for the
>> bootloader to turn it on as soon as it takes over control of the LEDs.
>>
>>>>>>> On Linux, usually this is used as `linux,default-trigger =
>>>>>>> "heartbeat"`.
>>>>>>> It indicates that kernel is running (regardless of the `default-
>>>>>>> state`
>>>>>>> setting), and its behavior can be modified in user space.
>>>>>>
>>>>>> As discussed already in the #linux-rockchip IRC channel, [1] perhaps
>>>>>> the best option would be to have the power LEDs turned on as quickly
>>>>>> upon powering on the board as possible, and to have U-Boot pulsate
>>>>>> the heartbeat LEDs using the LED_BOOT feature. In such a scenario,
>>>>>> no other LEDs would be turned on early, and the LED-related DT parts
>>>>>> specific to U-Boot would be migrated to the kernel DTs.
>>>>>>
>>>>>> [1] https://libera.catirclogs.org/linux-rockchip/2025-11-11#38997824;
>>>>>
>>>>> The LED_BOOT feature (guarded by the Kconfig symbol of the same
>>>>> name) in
>>>>> U-Boot only applies if /options/u-boot/boot-led property is set.
>>>>
>>>> For the default state of the heartbeat LED, I'm thinking of using
>>>> LED_BOOT (/options/u-boot/boot-led), but I'm concerned that this is
>>>> U-Boot-specific.
>>>
>>> If U-Boot wants to use the heartbeat LED to signal the *bootloader* is
>>> running, I guess that's fine. And if you want to make it solid or
>>> blinking, that seems best discussed on the U-Boot ML.
>>>
>>
>> The solution may still involve configuring the Device Tree, and we're
>> trying to have U-Boot-specific changes to the Device Tree in U-Boot
>> source tree to a minimum.
>>
>>> I still consider the bootloader and the kernel stages separate.
>>
>> They do however share most of their Device Tree (for Rockchip at least)
>> and the least (ideally no) changes we can have in U-Boot the better.
>>
>>> And I haven't seen an argument why I should change *my* opinion on the
>>> heartbeat and netdev triggers (default-state) wrt the kernel.
>>>
>>
>> Device Tree is not kernel specific as you said already.
>>
>>> I don't think that what U-Boot does or doesn't do, should determine what
>>> the Linux kernel does or doesn't do.
>>
>> It shouldn't, but (most of) the Device Tree is shared, so you cannot
>> just dismiss U-Boot behavior when talking about Linux behavior based on
>> Device Tree interpretation. We may have a need for a bootloader-specific
>> property. We have a Linux-specific one after all (linux,default-
>> trigger). Though... that does seem to be on the edge of what the DT is
>> made for (description of the HW, not logic/policy).
>>
>>> I have no plans to use another bootloader then U-Boot, but it's possible
>>> that people do, so what the Linux kernel does should be independent from
>>> what the/a specific bootloader does.
>
> Each software should be independent, but hardware (state) cannot be
> independent.
>
>> Barebox also uses upstream DT as far as I know and supports some Radxa
>> products (Rock 5B/5T/..., CM3, Rock (RK3188), Rock 3A from the arch/arm/
>> boards/radxa-* directories). Zephyr has support for RK3568, RK3588, and
>> other SoCs, and uses upstream DT as well.
>>
>> Again, we're talking about modifications of the Device Tree here, so
>> typically I would expect all consumers of that DT to be interpreting the
>> properties the same way, except if you have OS-specific properties/nodes
>> (think u-boot,config-compatible nodes, linux, prefixed properties,
>> bootph- properties, ...).
>>
>>> And as I said before, *I* want LEDs with netdev and heartbeat triggers,
>>> to be off (at the start, which is indeed the default value).
>
> If you are using U-Boot, heartbeat LED is already on by U-Boot,
> e.g.
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi#L10-12
>
> But it's not visible in DTS in Linux,
> e.g.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts?h=v6.17#n55
>
> I think this situation should be fixed.
>
> Best regards,
>
> --
> FUKAUMI Naoki
> Radxa Computer (Shenzhen) Co., Ltd.
>
>>> I use the heartbeat trigger to:
>>> 1) See the kernel has started (and has gotten to the point the heartbeat
>>> 'infrastructure' has been set up
>>> 2) Wait for the blinking to slow down as that (generally) means it's
>>> pretty much done with the boot process and the SSH server should
>>> probably be running then, so I can login
>>> 3) When the heartbeat LED is solid, that means the system has crashed
>>> (f.e. due to overheating ...)
>>>
>>
>> If the *HW* default state of the LED is off and the default-state
>> property is off, then you won't be able to tell apart a completely
>> bricked board and one that is stuck somewhere between U-Boot proper and
>> the Linux kernel taking over that LED.
>>
>>> And also, if you're going to change/override other people's choices, a
>>> motivation as to why would be 'nice'.
>>>
>>>>> <more discussion about LED functionality in U-Boot ...>
>>>>
>>>> As you know, default "default-state" is "off".
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/
>>>> linux.git/tree/Documentation/devicetree/bindings/leds/common.yaml?
>>>> h=v6.17#n74
>>>>
>>>> As far as I understand, there should not be any workarounds for specific
>>>> implementations.
>>>> https://lore.kernel.org/linux-rockchip/3389401.44csPzL39Z@phil/
>>>>
>>>> So removing `default-state = "off"` is acceptable, right?
>>>
>>> I don't see/understand the connection with 'workarounds for specific
>>> implementations' with removing ``default-state = "off"``.
>>>
>>> IMO it's perfectly fine to remove ``default-state = "off"``, although
>>> having it explicitly may be useful, especially if the commit that set
>>> that property specified *why* it should be "off".
>>>
>>
>> The status property defaults to okay, and we do not want them to be
>> listed explicitly. Not sure if there's consensus on applying this to all
>> properties which have defaults, across all subsystems.
>>
>>> Relatedly, when a node does not have the 'default-state' property, I
>>> would _assume_ the author wanted/intended it to be "off". Ideally it
>>> would be described in the commit message, but that is optional.
>>
>> The lack of a property doesn't necessarily mean it was forgotten, agreed.
>>
>>> But if that is changed, then it should be motivated *why*.
>>>
>>
>> Agreed.
>>
>> Cheers,
>> Quentin
>>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards
2025-11-11 5:41 [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards FUKAUMI Naoki
2025-11-11 6:04 ` Dragan Simic
2025-11-11 13:07 ` Diederik de Haas
@ 2025-11-12 15:04 ` FUKAUMI Naoki
2 siblings, 0 replies; 13+ messages in thread
From: FUKAUMI Naoki @ 2025-11-12 15:04 UTC (permalink / raw)
To: heiko
Cc: robh, krzk+dt, conor+dt, jbx6244, dsimic, pgwipeout, didi.debian,
jonas, ziyao, amadeus, nicolas.frattaroli, pbrobinson, wens,
detlev.casanova, stephen, sebastian.reichel, liujianfeng1994,
andy.yan, damon.ding, kylepzak, devicetree, linux-rockchip
Hi all,
How about this?
----
Subject: Consolidates and clarifies LED device tree descriptions on
Radxa boards
Currently, on Radxa boards, the power LED is turned on immediately after
power-up, independent of software control. The heartbeat LED and other
available LEDs are subsequently turned on by the initial software, such
as U-Boot, to indicate software is running.
However, the device tree description for this behavior is inconsistent
and fragmented, with definitions split between the main Linux DTS files
and separate U-Boot files (u-boot/arch/arm/dts/*-u-boot.dtsi).
This patch series addresses this inconsistency and fragmentation by
consolidating the description within the upstream device tree, thereby
improving overall clarity.
Patch 1 addresses the inconsistency for the power LED by using
default-state = "on" instead of linux,default-trigger = "default-on".
Patch 2 addresses the fragmentation for the heartbeat LED by
consolidating its default-state = "on" definition into the main Linux DTS.
Patch 3 addresses inconsistency by removing redundant default-state =
"off" definitions, as this is already the default configuration.
(Alternatively, the opposite could be done: explicitly adding
default-state = "off" for other LEDs.)
----
Best regards,
--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.
On 11/11/25 14:41, FUKAUMI Naoki wrote:
> Radxa's boards turn all LEDs on at boot(loader), but some boards don't
> have `default-state` property in Linux kernel tree but have it in
> U-Boot tree instead[1].
>
> This patch adds `default-state = "on"` for (almost) all LEDs (with a
> few exceptions which should be "off" such as RGB LEDs on E25 and LAN/
> WAN LEDs on E20C/E52C).
>
> Also, remove following redundant properties:
> linux,default-trigger = "default-on"; // use default-state = "on"
> default-state = "off"; // default is "off"
>
> [1]
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3328-rock-pi-e-base-u-boot.dtsi#L10-12
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3399-rock-4c-plus-u-boot.dtsi#L11-17
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi#L11-13
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3566-radxa-cm3-io-u-boot.dtsi#L10-12
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3566-rock-3c-u-boot.dtsi#L14-16
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3568-radxa-e25-u-boot.dtsi#L7-24
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi#L11-13
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi#L11-13
> https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi#L10-12
>
> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
> ---
> Changes in v2:
> - Add more URLs for reference
> - Reword commit message
> ---
> arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts | 1 -
> arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts | 1 +
> arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts | 3 ++-
> arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi | 1 +
> arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts | 2 --
> arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts | 1 -
> arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts | 1 +
> arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts | 1 +
> arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi | 1 +
> arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 1 +
> arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts | 3 ++-
> arch/arm64/boot/dts/rockchip/rk3582-radxa-e52c.dts | 2 --
> arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts | 3 ++-
> arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi | 1 +
> arch/arm64/boot/dts/rockchip/rk3588-rock-5t.dts | 1 +
> arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 3 ++-
> 16 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts b/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts
> index 7a32972bc2496..c1e3098b9a7bc 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts
> @@ -35,7 +35,6 @@ green-led {
> function = LED_FUNCTION_POWER;
> gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;
> label = "rockpis:green:power";
> - linux,default-trigger = "default-on";
> };
>
> blue-led {
> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
> index a4bdd87d0729f..d3d6f34b66fb0 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
> @@ -59,6 +59,7 @@ leds {
>
> led-0 {
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> gpios = <&gpio3 RK_PA5 GPIO_ACTIVE_LOW>;
> linux,default-trigger = "heartbeat";
> };
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts b/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
> index 962b8b231c960..a83ffbef22a7b 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
> @@ -39,14 +39,15 @@ leds {
> led-0 {
> function = LED_FUNCTION_POWER;
> color = <LED_COLOR_ID_GREEN>;
> + default-state = "on";
> gpios = <&gpio3 RK_PD4 GPIO_ACTIVE_LOW>;
> - linux,default-trigger = "default-on";
> };
>
> /* USER_LED2 */
> led-1 {
> function = LED_FUNCTION_STATUS;
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_HIGH>;
> linux,default-trigger = "heartbeat";
> };
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> index 046dbe3290178..ef434c23fe85c 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> @@ -35,6 +35,7 @@ leds {
> led-0 {
> function = LED_FUNCTION_STATUS;
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_HIGH>;
> linux,default-trigger = "heartbeat";
> };
> diff --git a/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
> index b324527561558..79d316a1d8495 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
> @@ -62,7 +62,6 @@ leds {
>
> led-lan {
> color = <LED_COLOR_ID_GREEN>;
> - default-state = "off";
> function = LED_FUNCTION_LAN;
> gpios = <&gpio4 RK_PB5 GPIO_ACTIVE_HIGH>;
> linux,default-trigger = "netdev";
> @@ -78,7 +77,6 @@ led-sys {
>
> led-wan {
> color = <LED_COLOR_ID_GREEN>;
> - default-state = "off";
> function = LED_FUNCTION_WAN;
> gpios = <&gpio4 RK_PC0 GPIO_ACTIVE_HIGH>;
> linux,default-trigger = "netdev";
> diff --git a/arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts b/arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts
> index c03ae1dd34560..0b696d49b71fa 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts
> @@ -45,7 +45,6 @@ led-1 {
> default-state = "on";
> function = LED_FUNCTION_STATUS;
> gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_LOW>;
> - linux,default-trigger = "default-on";
> };
> };
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts b/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts
> index b5b253f04cdf5..9e7212b70e3f1 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts
> @@ -46,6 +46,7 @@ leds {
> led-1 {
> gpios = <&gpio4 RK_PA4 GPIO_ACTIVE_LOW>;
> color = <LED_COLOR_ID_GREEN>;
> + default-state = "on";
> function = LED_FUNCTION_ACTIVITY;
> linux,default-trigger = "heartbeat";
> pinctrl-names = "default";
> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
> index 6224d72813e59..3ec108bcf89a1 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
> @@ -47,6 +47,7 @@ led-0 {
> gpios = <&gpio0 RK_PA0 GPIO_ACTIVE_HIGH>;
> function = LED_FUNCTION_HEARTBEAT;
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> linux,default-trigger = "heartbeat";
> pinctrl-names = "default";
> pinctrl-0 = <&user_led2>;
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi b/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi
> index 729e38b9f620e..140582f8e1034 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi
> @@ -23,6 +23,7 @@ led_user: led-0 {
> gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;
> function = LED_FUNCTION_HEARTBEAT;
> color = <LED_COLOR_ID_GREEN>;
> + default-state = "on";
> linux,default-trigger = "heartbeat";
> pinctrl-names = "default";
> pinctrl-0 = <&led_user_en>;
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> index 44cfdfeed6681..e6c18df0fa582 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -47,6 +47,7 @@ led_user: led-0 {
> gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
> function = LED_FUNCTION_HEARTBEAT;
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> linux,default-trigger = "heartbeat";
> pinctrl-names = "default";
> pinctrl-0 = <&led_user_en>;
> diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> index 9bc33422ced50..99d3a8be8f18c 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> @@ -52,13 +52,14 @@ leds: leds {
>
> power-led {
> color = <LED_COLOR_ID_GREEN>;
> + default-state = "on";
> function = LED_FUNCTION_STATUS;
> gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
> - linux,default-trigger = "default-on";
> };
>
> user-led {
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> function = LED_FUNCTION_HEARTBEAT;
> gpios = <&gpio0 RK_PC4 GPIO_ACTIVE_LOW>;
> linux,default-trigger = "heartbeat";
> diff --git a/arch/arm64/boot/dts/rockchip/rk3582-radxa-e52c.dts b/arch/arm64/boot/dts/rockchip/rk3582-radxa-e52c.dts
> index 854c118418eb8..f737769d4a007 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3582-radxa-e52c.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3582-radxa-e52c.dts
> @@ -71,7 +71,6 @@ leds-1 {
>
> led-1 {
> color = <LED_COLOR_ID_GREEN>;
> - default-state = "off";
> function = LED_FUNCTION_LAN;
> linux,default-trigger = "netdev";
> pwms = <&pwm14 0 1000000 PWM_POLARITY_INVERTED>;
> @@ -80,7 +79,6 @@ led-1 {
>
> led-2 {
> color = <LED_COLOR_ID_GREEN>;
> - default-state = "off";
> function = LED_FUNCTION_WAN;
> linux,default-trigger = "netdev";
> pwms = <&pwm11 0 1000000 PWM_POLARITY_INVERTED>;
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
> index bc8140883de47..86477346c3f5a 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
> @@ -88,11 +88,12 @@ gpio-leds {
> pinctrl-0 = <&led_pins>;
>
> power-led1 {
> + default-state = "on";
> gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
> - linux,default-trigger = "default-on";
> };
>
> hdd-led2 {
> + default-state = "on";
> gpios = <&gpio0 RK_PC0 GPIO_ACTIVE_HIGH>;
> linux,default-trigger = "disk-activity";
> };
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi
> index e5c474e4d02a6..8c4a4270f9f93 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi
> @@ -30,6 +30,7 @@ leds {
> led_rgb_b {
> function = LED_FUNCTION_STATUS;
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
> linux,default-trigger = "heartbeat";
> };
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5t.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5t.dts
> index 0dd90c744380b..87e9d4b86dad4 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5t.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5t.dts
> @@ -33,6 +33,7 @@ leds {
> led_rgb_b {
> function = LED_FUNCTION_STATUS;
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> gpios = <&gpio0 RK_PA0 GPIO_ACTIVE_HIGH>;
> linux,default-trigger = "heartbeat";
> };
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> index 19a08f7794e67..46c81e796b100 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> @@ -54,6 +54,7 @@ leds {
>
> io-led {
> color = <LED_COLOR_ID_BLUE>;
> + default-state = "on";
> function = LED_FUNCTION_STATUS;
> gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_HIGH>;
> linux,default-trigger = "heartbeat";
> @@ -61,9 +62,9 @@ io-led {
>
> power-led {
> color = <LED_COLOR_ID_GREEN>;
> + default-state = "on";
> function = LED_FUNCTION_POWER;
> gpios = <&gpio3 RK_PC4 GPIO_ACTIVE_HIGH>;
> - linux,default-trigger = "default-on";
> };
> };
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-11-12 15:05 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-11 5:41 [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards FUKAUMI Naoki
2025-11-11 6:04 ` Dragan Simic
2025-11-11 13:07 ` Diederik de Haas
2025-11-11 14:46 ` Dragan Simic
2025-11-11 15:32 ` FUKAUMI Naoki
2025-11-11 16:14 ` Dragan Simic
2025-11-11 18:32 ` Quentin Schulz
2025-11-11 23:42 ` FUKAUMI Naoki
2025-11-12 9:40 ` Diederik de Haas
2025-11-12 10:34 ` Quentin Schulz
2025-11-12 11:36 ` FUKAUMI Naoki
2025-11-12 13:21 ` Diederik de Haas
2025-11-12 15:04 ` FUKAUMI Naoki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox