Skip to contentSkip to conversation

arm64: dts: rockchip: use "pwm-leds" for multicolor PWM LEDs on Radxa E25

PATCH In review FUKAUMI Naoki 3 participants 5 messages Aug 16, 2024 – Oct 30, 2024

Subsystems (2)
Device tree bindings Arm Rockchip
lore
Share & tools
lore

FUKAUMI Naoki

author
#0 lore raw

PATCH 1/1 arm64: dts: rockchip: use "pwm-leds" for multicolor PWM LEDs on Radxa E25

to make multicolor PWM LEDs behavior more consistent with vendor kernel[1], use "pwm-leds" for it on Radxa E25.

[1] https://github.com/radxa/kernel/blob/linux-5.10-gen-rkr4.1/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts#L100-L121

Fixes 2bf2f4d9f673 ("arm64: dts: rockchip: Add Radxa CM3I E25")
Signed-off-by FUKAUMI Naoki <[email protected]>
---
 .../boot/dts/rockchip/rk3568-radxa-e25.dts    | 36 ++++++++++---------
 1 file changed, 20 insertions(+), 16 deletions(-)
arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts +20 -16
index 72ad74c38a2b..0b527f67bdbd 100644
@@ -12,26 +12,30 @@ aliases {
 	};
 
 	pwm-leds {
-		compatible = "pwm-leds-multicolor";
+		compatible = "pwm-leds";
 
-		multi-led {
-			color = <LED_COLOR_ID_RGB>;
+		led-red {
+			color = <LED_COLOR_ID_RED>;
+			default-state = "on";
+			function = LED_FUNCTION_STATUS;
 			max-brightness = <255>;
+			pwms = <&pwm1 0 1000000 0>;
+		};
 
-			led-red {
-				color = <LED_COLOR_ID_RED>;
-				pwms = <&pwm1 0 1000000 0>;
-			};
-
-			led-green {
-				color = <LED_COLOR_ID_GREEN>;
-				pwms = <&pwm2 0 1000000 0>;
-			};
+		led-green {
+			color = <LED_COLOR_ID_GREEN>;
+			default-state = "on";
+			function = LED_FUNCTION_STATUS;
+			max-brightness = <255>;
+			pwms = <&pwm2 0 1000000 0>;
+		};
 
-			led-blue {
-				color = <LED_COLOR_ID_BLUE>;
-				pwms = <&pwm12 0 1000000 0>;
-			};
+		led-blue {
+			color = <LED_COLOR_ID_BLUE>;
+			default-state = "on";
+			function = LED_FUNCTION_STATUS;
+			max-brightness = <255>;
+			pwms = <&pwm12 0 1000000 0>;
 		};
 	};
Signature · 1 line
2.43.0
maintainer
#1 lore raw

Am Freitag, 16. August 2024, 13:04:02 CEST schrieb FUKAUMI Naoki:

Quoted 2 lines

to make multicolor PWM LEDs behavior more consistent with vendor kernel[1], use "pwm-leds" for it on Radxa E25.

sorry, but that is definitly not a valid reason.

A devicetree does describe actual hardware and is not a space for configuration choices. So that whole notion to "match a kernel" is not correct.

Looking at
https://wiki.radxa.com/Rock3/CM/CM3I/E25/getting_started
the specification table clearly designates the board's LED as "RGB LED" - so one LED and multicolor .

Heiko

Quoted 60 lines

[1] https://github.com/radxa/kernel/blob/linux-5.10-gen-rkr4.1/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts#L100-L121

Fixes 2bf2f4d9f673 ("arm64: dts: rockchip: Add Radxa CM3I E25")
Signed-off-by FUKAUMI Naoki <[email protected]>
---
 .../boot/dts/rockchip/rk3568-radxa-e25.dts    | 36 ++++++++++---------
 1 file changed, 20 insertions(+), 16 deletions(-)
arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts +20 -16
index 72ad74c38a2b..0b527f67bdbd 100644
@@ -12,26 +12,30 @@ aliases {
 	};
 
 	pwm-leds {
-		compatible = "pwm-leds-multicolor";
+		compatible = "pwm-leds";
 
-		multi-led {
-			color = <LED_COLOR_ID_RGB>;
+		led-red {
+			color = <LED_COLOR_ID_RED>;
+			default-state = "on";
+			function = LED_FUNCTION_STATUS;
 			max-brightness = <255>;
+			pwms = <&pwm1 0 1000000 0>;
+		};
 
-			led-red {
-				color = <LED_COLOR_ID_RED>;
-				pwms = <&pwm1 0 1000000 0>;
-			};
-
-			led-green {
-				color = <LED_COLOR_ID_GREEN>;
-				pwms = <&pwm2 0 1000000 0>;
-			};
+		led-green {
+			color = <LED_COLOR_ID_GREEN>;
+			default-state = "on";
+			function = LED_FUNCTION_STATUS;
+			max-brightness = <255>;
+			pwms = <&pwm2 0 1000000 0>;
+		};
 
-			led-blue {
-				color = <LED_COLOR_ID_BLUE>;
-				pwms = <&pwm12 0 1000000 0>;
-			};
+		led-blue {
+			color = <LED_COLOR_ID_BLUE>;
+			default-state = "on";
+			function = LED_FUNCTION_STATUS;
+			max-brightness = <255>;
+			pwms = <&pwm12 0 1000000 0>;
 		};
 	};
author
#2 lore raw

Hi,

Quoted 5 lines · Heiko Stübner

Am Freitag, 16. August 2024, 13:04:02 CEST schrieb FUKAUMI Naoki:

Quoted 2 lines

to make multicolor PWM LEDs behavior more consistent with vendor kernel[1], use "pwm-leds" for it on Radxa E25.

sorry, but that is definitly not a valid reason.

I see. I'll not change it.

Quoted 8 lines

A devicetree does describe actual hardware and is not a space for configuration choices. So that whole notion to "match a kernel" is not correct.

Looking at
https://wiki.radxa.com/Rock3/CM/CM3I/E25/getting_started
the specification table clearly designates the board's LED as "RGB LED" - so one LED and multicolor .

I understand following behavior is not possible on mainline.

https://github.com/radxa-pkg/rsetup/blob/main/config/00-rgb0-rainbow.conf

this is not what we(Radxa) want, but we need to follow the rule.

(btw wiki.radxa.com is outdated, it's not used anymore)

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

Quoted 49 lines

Heiko

Quoted 46 lines

[1] https://github.com/radxa/kernel/blob/linux-5.10-gen-rkr4.1/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts#L100-L121

Fixes 2bf2f4d9f673 ("arm64: dts: rockchip: Add Radxa CM3I E25")
Signed-off-by FUKAUMI Naoki <[email protected]>
---
  .../boot/dts/rockchip/rk3568-radxa-e25.dts    | 36 ++++++++++---------
  1 file changed, 20 insertions(+), 16 deletions(-)
arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts +14 -12
index 72ad74c38a2b..0b527f67bdbd 100644
@@ -12,26 +12,30 @@ aliases {
  	};
 
  	pwm-leds {
-		compatible = "pwm-leds-multicolor";
+		compatible = "pwm-leds";
 
-		multi-led {
-			color = <LED_COLOR_ID_RGB>;
+		led-red {
+			color = <LED_COLOR_ID_RED>;
+			default-state = "on";
+			function = LED_FUNCTION_STATUS;
  			max-brightness = <255>;
+			pwms = <&pwm1 0 1000000 0>;
+		};
 
-			led-red {
-				color = <LED_COLOR_ID_RED>;
-				pwms = <&pwm1 0 1000000 0>;
-			};
-
-			led-green {
-				color = <LED_COLOR_ID_GREEN>;
-				pwms = <&pwm2 0 1000000 0>;
-			};
+		led-green {
+			color = <LED_COLOR_ID_GREEN>;
+			default-state = "on";
+			function = LED_FUNCTION_STATUS;
+			max-brightness = <255>;
+			pwms = <&pwm2 0 1000000 0>;
+		};
Quoted 19 lines
Quoted 4 lines
arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts +0 -3
-			led-blue {
-				color = <LED_COLOR_ID_BLUE>;
-				pwms = <&pwm12 0 1000000 0>;

arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts aliases { · line 41

Quoted 10 lines
arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts +6 -1
-			};
+		led-blue {
+			color = <LED_COLOR_ID_BLUE>;
+			default-state = "on";
+			function = LED_FUNCTION_STATUS;
+			max-brightness = <255>;
+			pwms = <&pwm12 0 1000000 0>;
  		};
  	};

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip

#3 lore raw
Quoted 20 lines · FUKAUMI Naoki

Hi,

Quoted 5 lines · Heiko Stübner

Am Freitag, 16. August 2024, 13:04:02 CEST schrieb FUKAUMI Naoki:

Quoted 2 lines

to make multicolor PWM LEDs behavior more consistent with vendor kernel[1], use "pwm-leds" for it on Radxa E25.

sorry, but that is definitly not a valid reason.

I see. I'll not change it.

Quoted 8 lines

A devicetree does describe actual hardware and is not a space for configuration choices. So that whole notion to "match a kernel" is not correct.

Looking at
https://wiki.radxa.com/Rock3/CM/CM3I/E25/getting_started
the specification table clearly designates the board's LED as "RGB LED" - so one LED and multicolor .

Quoted 7 lines

I understand following behavior is not possible on mainline.

https://github.com/radxa-pkg/rsetup/blob/main/config/00-rgb0-rainbow.conf

this is not what we(Radxa) want, but we need to follow the rule.

(btw wiki.radxa.com is outdated, it's not used anymore)

Your Radxa vendor kernel and whatever you have *MUST* align to the upstream, not the other way. The upstream is the correct source of doing things, not the other way.

Best regards,
Krzysztof

author
#4 lore raw

Hi,

Quoted 25 lines · Krzysztof Kozlowski
Quoted 24 lines · FUKAUMI Naoki

Hi,

Quoted 5 lines · Heiko Stübner

Am Freitag, 16. August 2024, 13:04:02 CEST schrieb FUKAUMI Naoki:

Quoted 2 lines

to make multicolor PWM LEDs behavior more consistent with vendor kernel[1], use "pwm-leds" for it on Radxa E25.

sorry, but that is definitly not a valid reason.

I see. I'll not change it.

Quoted 8 lines

A devicetree does describe actual hardware and is not a space for configuration choices. So that whole notion to "match a kernel" is not correct.

Looking at
https://wiki.radxa.com/Rock3/CM/CM3I/E25/getting_started
the specification table clearly designates the board's LED as "RGB LED" - so one LED and multicolor .

I understand following behavior is not possible on mainline.

https://github.com/radxa-pkg/rsetup/blob/main/config/00-rgb0-rainbow.conf

Quoted 7 lines
Quoted 3 lines

this is not what we(Radxa) want, but we need to follow the rule.

(btw wiki.radxa.com is outdated, it's not used anymore)

Your Radxa vendor kernel and whatever you have *MUST* align to the upstream, not the other way. The upstream is the correct source of doing things, not the other way.

could you tell me how to make rainbow color effect with "pwm-leds-multicolor" driver?

as far as I know, "pattern" trigger can change (only) brightness, not color.

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

Quoted 2 lines

Best regards,
Krzysztof

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip