[PATCH 00/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa boards

[PATCH 00/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa boards

[FUKAUMI Naoki]

the standard definition of LEDs for Radxa boards are:

  color = <LED_COLOR_ID_xxx>; [1]
  default-state = "on"; [2]
  function = LED_FUNCTION_xxx [1] [3]
  gpios = <&gpioX RK_Pxx GPIO_ACTIVE_xxx>;
  linux,default-trigger = "xxx"; [3]
  pinctrl-names = "default"; [4]
  pinctrl-0 = <&xxx>; [4]

[1] use "color:function" instead of `label = "xxx:yyy:zzz"`
[2] all LEDs should be turned on at U-Boot
[3] if trigger is "heartbeat", FUNCTION should be HEARTBEAT
[4] if it's SOM and carrier board, put pinctrl properties here
    if it's SBC, put them under `compatible = "gpio-leds"`

FUKAUMI Naoki (10):
  arm64: dts: rockchip: standardize the definition of LEDs for Radxa
    ROCK Pi S
  arm64: dts: rockchip: standardize the definition of LEDs for Radxa
    ROCK Pi E
  arm64: dts: rockchip: standardize the definition of LEDs for Radxa
    ROCK 4C+
  arm64: dts: rockchip: standardize the definition of LEDs for Radxa
    ROCK Pi 4
  arm64: dts: rockchip: standardize the definition of LEDs for Radxa CM3
    IO
  arm64: dts: rockchip: standardize the definition of LEDs for Radxa
    ROCK 3C
  arm64: dts: rockchip: standardize the definition of LEDs for Radxa E25
  arm64: dts: rockchip: standardize the definition of LEDs for Radxa
    ROCK 3A
  arm64: dts: rockchip: standardize the definition of LEDs for Radxa
    ROCK 5B
  arm64: dts: rockchip: standardize the definition of LEDs for Radxa
    ROCK 5A

 arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts    | 4 +---
 arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dtsi   | 4 +++-
 arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts | 6 ++++--
 arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi   | 3 ++-
 arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts | 5 +++--
 arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3.dtsi   | 6 +++---
 arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts      | 9 +++++----
 arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi  | 5 +++--
 arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts    | 7 ++++---
 arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts      | 9 +++++----
 arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts      | 3 ++-
 arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts     | 3 ++-
 12 files changed, 37 insertions(+), 27 deletions(-)

-- 
2.43.0

[PATCH 01/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa ROCK Pi S

[FUKAUMI Naoki]

- remove label
- cosmetic change

Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
---
 arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts | 4 +---
 1 file changed, 1 insertion(+), 3 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 62d18ca769a1..504054f9723e 100644
--- a/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts
@@ -27,14 +27,13 @@ chosen {
 	leds {
 		compatible = "gpio-leds";
 		pinctrl-names = "default";
-		pinctrl-0 = <&green_led>, <&heartbeat_led>;
+		pinctrl-0 = <&green_led &heartbeat_led>;
 
 		green-led {
 			color = <LED_COLOR_ID_GREEN>;
 			default-state = "on";
 			function = LED_FUNCTION_POWER;
 			gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;
-			label = "rockpis:green:power";
 			linux,default-trigger = "default-on";
 		};
 
@@ -43,7 +42,6 @@ blue-led {
 			default-state = "on";
 			function = LED_FUNCTION_HEARTBEAT;
 			gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_HIGH>;
-			label = "rockpis:blue:user";
 			linux,default-trigger = "heartbeat";
 		};
 	};
-- 
2.43.0

[PATCH 02/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa ROCK Pi E

[FUKAUMI Naoki]

- sort properties
- add default-state and function

Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
---
 arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dtsi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dtsi b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dtsi
index bb01143dc91a..4e77701f2e01 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dtsi
@@ -49,11 +49,13 @@ gmac_clkin: external-gmac-clock {
 
 	leds {
 		compatible = "gpio-leds";
-		pinctrl-0 = <&led_pin>;
 		pinctrl-names = "default";
+		pinctrl-0 = <&led_pin>;
 
 		led-0 {
 			color = <LED_COLOR_ID_BLUE>;
+			default-state = "on";
+			function = LED_FUNCTION_HEARTBEAT;
 			gpios = <&gpio3 RK_PA5 GPIO_ACTIVE_LOW>;
 			linux,default-trigger = "heartbeat";
 		};
-- 
2.43.0

[PATCH 03/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa ROCK 4C+

[FUKAUMI Naoki]

- sort properties
- add default-state
- change function for led-1 from STATUS to HEARTBEAT

Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
---
 arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

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 d4b4dced3e39..7454bfb76a7d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
@@ -37,16 +37,18 @@ leds {
 
 		/* USER_LED1 */
 		led-0 {
-			function = LED_FUNCTION_POWER;
 			color = <LED_COLOR_ID_GREEN>;
+			default-state = "on";
+			function = LED_FUNCTION_POWER;
 			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";
+			function = LED_FUNCTION_HEARTBEAT;
 			gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_HIGH>;
 			linux,default-trigger = "heartbeat";
 		};
-- 
2.43.0

[PATCH 04/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa ROCK Pi 4

[FUKAUMI Naoki]

- sort properties
- add default-state
- change function from STATUS to HEARTBEAT

Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
---
 arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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 9666504cd1c1..bdca1a135f06 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
@@ -33,8 +33,9 @@ leds {
 
 		/* USER_LED2 */
 		led-0 {
-			function = LED_FUNCTION_STATUS;
 			color = <LED_COLOR_ID_BLUE>;
+			default-state = "on";
+			function = LED_FUNCTION_HEARTBEAT;
 			gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_HIGH>;
 			linux,default-trigger = "heartbeat";
 		};
-- 
2.43.0

[PATCH 05/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa CM3 IO

[FUKAUMI Naoki]

- sort properties
- add default-state for led-1
- change function for led-0 from STATUS to HEARTBEAT
- change trigger for led-0 from "timer" to "heartbeat"
- change trigger for led-1 from "heartbeat" to "timer"

Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
---
 arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts | 5 +++--
 arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3.dtsi   | 6 +++---
 2 files changed, 6 insertions(+), 5 deletions(-)

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 3ae24e39450a..f9033d9d17f8 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts
@@ -44,10 +44,11 @@ leds {
 		compatible = "gpio-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";
+			gpios = <&gpio4 RK_PA4 GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "timer";
 			pinctrl-names = "default";
 			pinctrl-0 = <&pi_nled_activity>;
 		};
diff --git a/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3.dtsi
index 45de2630bb50..81783d748707 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3.dtsi
@@ -18,11 +18,11 @@ leds {
 		compatible = "gpio-leds";
 
 		led-0 {
-			gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;
 			color = <LED_COLOR_ID_GREEN>;
-			function = LED_FUNCTION_STATUS;
-			linux,default-trigger = "timer";
 			default-state = "on";
+			function = LED_FUNCTION_HEARTBEAT;
+			gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;
+			linux,default-trigger = "heartbeat";
 			pinctrl-names = "default";
 			pinctrl-0 = <&user_led2>;
 		};
-- 
2.43.0

[PATCH 06/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa ROCK 3C

[FUKAUMI Naoki]

- move pinctrl properties
- sort properties
- add default-state

Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
---
 arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
index f2cc086e5001..f65ac2e7943b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
@@ -42,14 +42,15 @@ hdmi_con_in: endpoint {
 
 	leds {
 		compatible = "gpio-leds";
+		pinctrl-names = "default";
+		pinctrl-0 = <&user_led2>;
 
 		led-0 {
-			gpios = <&gpio0 RK_PA0 GPIO_ACTIVE_HIGH>;
-			function = LED_FUNCTION_HEARTBEAT;
 			color = <LED_COLOR_ID_BLUE>;
+			default-state = "on";
+			function = LED_FUNCTION_HEARTBEAT;
+			gpios = <&gpio0 RK_PA0 GPIO_ACTIVE_HIGH>;
 			linux,default-trigger = "heartbeat";
-			pinctrl-names = "default";
-			pinctrl-0 = <&user_led2>;
 		};
 	};
 
-- 
2.43.0

[PATCH 07/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa E25

[FUKAUMI Naoki]

- sort properties
- add default-state
- add function for multi-led
- remove redundant parameters from pwms

Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
---
 arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi | 5 +++--
 arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts   | 7 ++++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi b/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi
index 45b03dcbbad4..b287d9cca6f9 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi
@@ -20,9 +20,10 @@ gpio-leds {
 		compatible = "gpio-leds";
 
 		led_user: led-0 {
-			gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;
-			function = LED_FUNCTION_HEARTBEAT;
 			color = <LED_COLOR_ID_GREEN>;
+			default-state = "on";
+			function = LED_FUNCTION_HEARTBEAT;
+			gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;
 			linux,default-trigger = "heartbeat";
 			pinctrl-names = "default";
 			pinctrl-0 = <&led_user_en>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts b/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts
index 72ad74c38a2b..4b5203c0037a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts
@@ -16,21 +16,22 @@ pwm-leds {
 
 		multi-led {
 			color = <LED_COLOR_ID_RGB>;
+			function = LED_FUNCTION_ACTIVITY;
 			max-brightness = <255>;
 
 			led-red {
 				color = <LED_COLOR_ID_RED>;
-				pwms = <&pwm1 0 1000000 0>;
+				pwms = <&pwm1 0 1000000>;
 			};
 
 			led-green {
 				color = <LED_COLOR_ID_GREEN>;
-				pwms = <&pwm2 0 1000000 0>;
+				pwms = <&pwm2 0 1000000>;
 			};
 
 			led-blue {
 				color = <LED_COLOR_ID_BLUE>;
-				pwms = <&pwm12 0 1000000 0>;
+				pwms = <&pwm12 0 1000000>;
 			};
 		};
 	};
-- 
2.43.0

[PATCH 08/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa ROCK 3A

[FUKAUMI Naoki]

- move pinctrl properties
- sort properties
- add default-state

Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
---
 arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
index 59f1403b4fa5..4fd6096f16ea 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
@@ -42,14 +42,15 @@ gmac1_clkin: external-gmac1-clock {
 
 	leds {
 		compatible = "gpio-leds";
+		pinctrl-names = "default";
+		pinctrl-0 = <&led_user_en>;
 
 		led_user: led-0 {
-			gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
-			function = LED_FUNCTION_HEARTBEAT;
 			color = <LED_COLOR_ID_BLUE>;
+			default-state = "on";
+			function = LED_FUNCTION_HEARTBEAT;
+			gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
 			linux,default-trigger = "heartbeat";
-			pinctrl-names = "default";
-			pinctrl-0 = <&led_user_en>;
 		};
 	};
 
-- 
2.43.0

[PATCH 09/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa ROCK 5B

[FUKAUMI Naoki]

- sort properties
- add default-state
- change function from STATUS to HEARTBEAT

Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
---
 arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
index 966bbc582d89..62c6ed179e5d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
@@ -43,8 +43,9 @@ leds {
 		pinctrl-0 = <&led_rgb_b>;
 
 		led_rgb_b {
-			function = LED_FUNCTION_STATUS;
 			color = <LED_COLOR_ID_BLUE>;
+			default-state = "on";
+			function = LED_FUNCTION_HEARTBEAT;
 			gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
 			linux,default-trigger = "heartbeat";
 		};
-- 
2.43.0

[PATCH 10/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa ROCK 5A

[FUKAUMI Naoki]

- add default-state
- change function from STATUS to HEARTBEAT

Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
---
 arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
index 03ed48246d36..17364582297d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
@@ -42,7 +42,8 @@ leds {
 
 		io-led {
 			color = <LED_COLOR_ID_BLUE>;
-			function = LED_FUNCTION_STATUS;
+			default-state = "on";
+			function = LED_FUNCTION_HEARTBEAT;
 			gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_HIGH>;
 			linux,default-trigger = "heartbeat";
 		};
-- 
2.43.0

Re: [PATCH 01/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa ROCK Pi S

[Krzysztof Kozlowski]

On 08/08/2024 11:37, FUKAUMI Naoki wrote:
> - remove label

Why? You affect user-space.

> - cosmetic change

What? Why? Commit subject also does not help here at all.

Best regards,
Krzysztof

Re: [PATCH 02/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa ROCK Pi E

[Krzysztof Kozlowski]

On 08/08/2024 11:38, FUKAUMI Naoki wrote:
> - sort properties

You mean un-sort?!?!

> - add default-state and function

Commit subject is totally wrong. Sorry, write specific subjects
describing what you are doing. There is no standardization happening
here, whatever that means.

> 
> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dtsi | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dtsi b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dtsi
> index bb01143dc91a..4e77701f2e01 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dtsi
> @@ -49,11 +49,13 @@ gmac_clkin: external-gmac-clock {
>  
>  	leds {
>  		compatible = "gpio-leds";
> -		pinctrl-0 = <&led_pin>;
>  		pinctrl-names = "default";
> +		pinctrl-0 = <&led_pin>;

Nope, that's wrong order.

>  

Best regards,
Krzysztof

Re: [PATCH 03/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa ROCK 4C+

[Krzysztof Kozlowski]

On 08/08/2024 11:38, FUKAUMI Naoki wrote:
> - sort properties

Please, stop. This makes no sense.

> - add default-state
> - change function for led-1 from STATUS to HEARTBEAT

Don't mix changes.



Best regards,
Krzysztof

Re: [PATCH 07/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa E25

[Krzysztof Kozlowski]

On 08/08/2024 11:38, FUKAUMI Naoki wrote:
> - sort properties
> - add default-state
> - add function for multi-led
> - remove redundant parameters from pwms

This does not match commit subject at all. Anyway, one commit per
logical change. Trivial cleanups are not per board, but per entire
subsystem. Changes with impact can be per board.

The way you split your work makes review, bisecting and backporting
unnecessary difficult. It's not even logical. There is no logical change
like "I will do 10 things for board X".

Best regards,
Krzysztof

Re: [PATCH 10/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa ROCK 5A

[Krzysztof Kozlowski]

On 08/08/2024 11:38, FUKAUMI Naoki wrote:
> - add default-state
> - change function from STATUS to HEARTBEAT

Why? Explain in ALL YOUR COMMITS why you are doing this.

What you are doing is visible from the diff.


Best regards,
Krzysztof

Re: [PATCH 08/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa ROCK 3A

[Krzysztof Kozlowski]

On 08/08/2024 11:38, FUKAUMI Naoki wrote:
> - move pinctrl properties
> - sort properties
> - add default-state
> 
> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> index 59f1403b4fa5..4fd6096f16ea 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -42,14 +42,15 @@ gmac1_clkin: external-gmac1-clock {
>  
>  	leds {
>  		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&led_user_en>;

Weird order. names never precede the property.

Best regards,
Krzysztof

Re: [PATCH 00/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa boards

[Rob Herring (Arm)]

On Thu, 08 Aug 2024 18:37:58 +0900, FUKAUMI Naoki wrote:
> the standard definition of LEDs for Radxa boards are:
> 
>   color = <LED_COLOR_ID_xxx>; [1]
>   default-state = "on"; [2]
>   function = LED_FUNCTION_xxx [1] [3]
>   gpios = <&gpioX RK_Pxx GPIO_ACTIVE_xxx>;
>   linux,default-trigger = "xxx"; [3]
>   pinctrl-names = "default"; [4]
>   pinctrl-0 = <&xxx>; [4]
> 
> [1] use "color:function" instead of `label = "xxx:yyy:zzz"`
> [2] all LEDs should be turned on at U-Boot
> [3] if trigger is "heartbeat", FUNCTION should be HEARTBEAT
> [4] if it's SOM and carrier board, put pinctrl properties here
>     if it's SBC, put them under `compatible = "gpio-leds"`
> 
> FUKAUMI Naoki (10):
>   arm64: dts: rockchip: standardize the definition of LEDs for Radxa
>     ROCK Pi S
>   arm64: dts: rockchip: standardize the definition of LEDs for Radxa
>     ROCK Pi E
>   arm64: dts: rockchip: standardize the definition of LEDs for Radxa
>     ROCK 4C+
>   arm64: dts: rockchip: standardize the definition of LEDs for Radxa
>     ROCK Pi 4
>   arm64: dts: rockchip: standardize the definition of LEDs for Radxa CM3
>     IO
>   arm64: dts: rockchip: standardize the definition of LEDs for Radxa
>     ROCK 3C
>   arm64: dts: rockchip: standardize the definition of LEDs for Radxa E25
>   arm64: dts: rockchip: standardize the definition of LEDs for Radxa
>     ROCK 3A
>   arm64: dts: rockchip: standardize the definition of LEDs for Radxa
>     ROCK 5B
>   arm64: dts: rockchip: standardize the definition of LEDs for Radxa
>     ROCK 5A
> 
>  arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts    | 4 +---
>  arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dtsi   | 4 +++-
>  arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts | 6 ++++--
>  arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi   | 3 ++-
>  arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3-io.dts | 5 +++--
>  arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3.dtsi   | 6 +++---
>  arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts      | 9 +++++----
>  arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi  | 5 +++--
>  arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts    | 7 ++++---
>  arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts      | 9 +++++----
>  arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts      | 3 ++-
>  arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts     | 3 ++-
>  12 files changed, 37 insertions(+), 27 deletions(-)
> 
> --
> 2.43.0
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y rockchip/rk3308-rock-pi-s.dtb rockchip/rk3399-rock-4c-plus.dtb rockchip/rk3566-radxa-cm3-io.dtb rockchip/rk3566-rock-3c.dtb rockchip/rk3568-radxa-e25.dtb rockchip/rk3568-rock-3a.dtb rockchip/rk3588-rock-5b.dtb rockchip/rk3588s-rock-5a.dtb' for 20240808093808.1740-1-naoki@radxa.com:

arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts:24.5-30: Warning (pwms_property): /pwm-leds/multi-led/led-red:pwms: property size (12) too small for cell size 3
arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts:29.5-30: Warning (pwms_property): /pwm-leds/multi-led/led-green:pwms: property size (12) too small for cell size 3
arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts:34.5-31: Warning (pwms_property): /pwm-leds/multi-led/led-blue:pwms: property size (12) too small for cell size 3

Re: [PATCH 07/10] arm64: dts: rockchip: standardize the definition of LEDs for Radxa E25

[Krzysztof Kozlowski]

On 08/08/2024 11:38, FUKAUMI Naoki wrote:
> - sort properties
> - add default-state
> - add function for multi-led
> - remove redundant parameters from pwms

Why redundant? I don't think this was tested. :(

Best regards,
Krzysztof