Linux-Rockchip Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Diederik de Haas" <diederik@cknow-tech.com>
To: "FUKAUMI Naoki" <naoki@radxa.com>, <heiko@sntech.de>
Cc: <robh@kernel.org>, <krzk+dt@kernel.org>, <conor+dt@kernel.org>,
	<jbx6244@gmail.com>, <dsimic@manjaro.org>, <pgwipeout@gmail.com>,
	<jonas@kwiboo.se>, <ziyao@disroot.org>, <amadeus@jmu.edu.cn>,
	<nicolas.frattaroli@collabora.com>, <pbrobinson@gmail.com>,
	<wens@kernel.org>, <detlev.casanova@collabora.com>,
	<stephen@radxa.com>, <sebastian.reichel@collabora.com>,
	<liujianfeng1994@gmail.com>, <andy.yan@rock-chips.com>,
	<damon.ding@rock-chips.com>, <kylepzak@projectinitiative.io>,
	<devicetree@vger.kernel.org>,
	<linux-rockchip@lists.infradead.org>
Subject: Re: [PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards
Date: Tue, 11 Nov 2025 14:07:25 +0100	[thread overview]
Message-ID: <DE5W0143QGG2.3C7SW89IJKZ4P@cknow-tech.com> (raw)
In-Reply-To: <20251111054112.55505-1-naoki@radxa.com>

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

  parent reply	other threads:[~2025-11-11 13:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DE5W0143QGG2.3C7SW89IJKZ4P@cknow-tech.com \
    --to=diederik@cknow-tech.com \
    --cc=amadeus@jmu.edu.cn \
    --cc=andy.yan@rock-chips.com \
    --cc=conor+dt@kernel.org \
    --cc=damon.ding@rock-chips.com \
    --cc=detlev.casanova@collabora.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dsimic@manjaro.org \
    --cc=heiko@sntech.de \
    --cc=jbx6244@gmail.com \
    --cc=jonas@kwiboo.se \
    --cc=krzk+dt@kernel.org \
    --cc=kylepzak@projectinitiative.io \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=liujianfeng1994@gmail.com \
    --cc=naoki@radxa.com \
    --cc=nicolas.frattaroli@collabora.com \
    --cc=pbrobinson@gmail.com \
    --cc=pgwipeout@gmail.com \
    --cc=robh@kernel.org \
    --cc=sebastian.reichel@collabora.com \
    --cc=stephen@radxa.com \
    --cc=wens@kernel.org \
    --cc=ziyao@disroot.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox