diff mbox series

[3/5] arm64: dts: rockchip: Improve LEDs on NanoPi R6C/R6S

Message ID 20240612205056.397204-4-seb-dev@mail.de (mailing list archive)
State New
Headers show
Series Refactor, fix and improve NanoPi R6 series | expand

Commit Message

Sebastian Kropatsch June 12, 2024, 8:48 p.m. UTC
Move led-3 node into NanoPi R6C/R6S's source files since they have
different functionalities on each board: On the R6S this LED is used
to signal LAN2 link up, while on the R6C this LED does not have a
pre-defined purpose.

In addition to that:
    - Remove deprecated label property
    - Add color and function properties
    - Add linux,default-trigger to trigger on Ethernet link

Signed-off-by: Sebastian Kropatsch <seb-dev@mail.de>
---

DT validation doesn't like the 'linux,default-trigger = "stmmac-0:01:link"'
properties, since "*:link" is not a valid value according to
[Documentation/devicetree/bindings/leds/common.yaml]. These LEDs do
have the specific purpose to show if an Ethernet link is up though.
There is one LED for each Ethernet port and they are labeled WAN and
LAN.
Using the 'linux,default-trigger' like this does work perfectly fine
with this solution. I could not find another way to achieve this. Please
let me know if there is a better way.
Maybe it would also be valid to add an entry to the DT bindings file to
allow "*:link" as a value for 'linux,default-trigger'?

The same problem also applies to the NanoPi R5C/R5S which also has
these LEDs to show Ethernet link up, although in their current DT
these LEDs are simply not functional.

---
 .../boot/dts/rockchip/rk3588s-nanopi-r6.dtsi  | 32 +++++++------------
 .../boot/dts/rockchip/rk3588s-nanopi-r6c.dts  | 22 ++++++++++---
 .../boot/dts/rockchip/rk3588s-nanopi-r6s.dts  | 26 +++++++++++++--
 3 files changed, 54 insertions(+), 26 deletions(-)

Comments

kernel test robot June 14, 2024, 3:10 p.m. UTC | #1
Hi Sebastian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rockchip/for-next]
[also build test WARNING on linus/master v6.10-rc3 next-20240613]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Sebastian-Kropatsch/arm64-dts-rockchip-Add-common-definitions-for-NanoPi-R6C-and-R6S/20240613-045639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
patch link:    https://lore.kernel.org/r/20240612205056.397204-4-seb-dev%40mail.de
patch subject: [PATCH 3/5] arm64: dts: rockchip: Improve LEDs on NanoPi R6C/R6S
config: arm64-randconfig-051-20240614 (https://download.01.org/0day-ci/archive/20240614/202406142337.SdnyldQ8-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
dtschema version: 2024.6.dev1+g833054f
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240614/202406142337.SdnyldQ8-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406142337.SdnyldQ8-lkp@intel.com/

dtcheck warnings: (new ones prefixed by >>)
>> arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6s.dtb: leds: led-1:linux,default-trigger: 'oneOf' conditional failed, one must be fixed:
   	'stmmac-0:01:link' is not one of ['backlight', 'default-on', 'heartbeat', 'disk-activity', 'disk-read', 'disk-write', 'timer', 'pattern', 'audio-micmute', 'audio-mute', 'bluetooth-power', 'flash', 'kbd-capslock', 'mtd', 'nand-disk', 'none', 'torch', 'usb-gadget', 'usb-host', 'usbport']
   	'stmmac-0:01:link' does not match '^cpu[0-9]*$'
   	'stmmac-0:01:link' does not match '^hci[0-9]+-power$'
   	'stmmac-0:01:link' does not match '^mmc[0-9]+$'
   	'stmmac-0:01:link' does not match '^phy[0-9]+tx$'
   	from schema $id: http://devicetree.org/schemas/leds/leds-gpio.yaml#
>> arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6s.dtb: leds: led-1: Unevaluated properties are not allowed ('linux,default-trigger' was unexpected)
   	from schema $id: http://devicetree.org/schemas/leds/leds-gpio.yaml#
   arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6s.dtb: leds: led-2:linux,default-trigger: 'oneOf' conditional failed, one must be fixed:
   	'r8169-3-3100:00:link' is not one of ['backlight', 'default-on', 'heartbeat', 'disk-activity', 'disk-read', 'disk-write', 'timer', 'pattern', 'audio-micmute', 'audio-mute', 'bluetooth-power', 'flash', 'kbd-capslock', 'mtd', 'nand-disk', 'none', 'torch', 'usb-gadget', 'usb-host', 'usbport']
   	'r8169-3-3100:00:link' does not match '^cpu[0-9]*$'
   	'r8169-3-3100:00:link' does not match '^hci[0-9]+-power$'
   	'r8169-3-3100:00:link' does not match '^mmc[0-9]+$'
   	'r8169-3-3100:00:link' does not match '^phy[0-9]+tx$'
   	from schema $id: http://devicetree.org/schemas/leds/leds-gpio.yaml#
   arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6s.dtb: leds: led-2: Unevaluated properties are not allowed ('linux,default-trigger' was unexpected)
   	from schema $id: http://devicetree.org/schemas/leds/leds-gpio.yaml#
--
>> arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6c.dtb: leds: led-1:linux,default-trigger: 'oneOf' conditional failed, one must be fixed:
   	'stmmac-0:01:link' is not one of ['backlight', 'default-on', 'heartbeat', 'disk-activity', 'disk-read', 'disk-write', 'timer', 'pattern', 'audio-micmute', 'audio-mute', 'bluetooth-power', 'flash', 'kbd-capslock', 'mtd', 'nand-disk', 'none', 'torch', 'usb-gadget', 'usb-host', 'usbport']
   	'stmmac-0:01:link' does not match '^cpu[0-9]*$'
   	'stmmac-0:01:link' does not match '^hci[0-9]+-power$'
   	'stmmac-0:01:link' does not match '^mmc[0-9]+$'
   	'stmmac-0:01:link' does not match '^phy[0-9]+tx$'
   	from schema $id: http://devicetree.org/schemas/leds/leds-gpio.yaml#
>> arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6c.dtb: leds: led-1: Unevaluated properties are not allowed ('linux,default-trigger' was unexpected)
   	from schema $id: http://devicetree.org/schemas/leds/leds-gpio.yaml#
   arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6c.dtb: leds: led-2:linux,default-trigger: 'oneOf' conditional failed, one must be fixed:
   	'r8169-3-3100:00:link' is not one of ['backlight', 'default-on', 'heartbeat', 'disk-activity', 'disk-read', 'disk-write', 'timer', 'pattern', 'audio-micmute', 'audio-mute', 'bluetooth-power', 'flash', 'kbd-capslock', 'mtd', 'nand-disk', 'none', 'torch', 'usb-gadget', 'usb-host', 'usbport']
   	'r8169-3-3100:00:link' does not match '^cpu[0-9]*$'
   	'r8169-3-3100:00:link' does not match '^hci[0-9]+-power$'
   	'r8169-3-3100:00:link' does not match '^mmc[0-9]+$'
   	'r8169-3-3100:00:link' does not match '^phy[0-9]+tx$'
   	from schema $id: http://devicetree.org/schemas/leds/leds-gpio.yaml#
   arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6c.dtb: leds: led-2: Unevaluated properties are not allowed ('linux,default-trigger' was unexpected)
   	from schema $id: http://devicetree.org/schemas/leds/leds-gpio.yaml#
Heiko Stübner June 20, 2024, 6:42 p.m. UTC | #2
Am Mittwoch, 12. Juni 2024, 22:48:12 CEST schrieb Sebastian Kropatsch:
> Move led-3 node into NanoPi R6C/R6S's source files since they have
> different functionalities on each board: On the R6S this LED is used
> to signal LAN2 link up, while on the R6C this LED does not have a
> pre-defined purpose.
> 
> In addition to that:
>     - Remove deprecated label property
>     - Add color and function properties
>     - Add linux,default-trigger to trigger on Ethernet link

Again, please don't group unrelated changes together into one patch.
It makes things hard to read and hard to follow the changes.

> 
> Signed-off-by: Sebastian Kropatsch <seb-dev@mail.de>
> ---
> 
> DT validation doesn't like the 'linux,default-trigger = "stmmac-0:01:link"'
> properties, since "*:link" is not a valid value according to
> [Documentation/devicetree/bindings/leds/common.yaml]. These LEDs do
> have the specific purpose to show if an Ethernet link is up though.
> There is one LED for each Ethernet port and they are labeled WAN and
> LAN.
> Using the 'linux,default-trigger' like this does work perfectly fine
> with this solution. I could not find another way to achieve this. Please
> let me know if there is a better way.
> Maybe it would also be valid to add an entry to the DT bindings file to
> allow "*:link" as a value for 'linux,default-trigger'?

correct. If needed, things should be added to binding.


Heiko
Sebastian Kropatsch June 20, 2024, 9:51 p.m. UTC | #3
Am 20.06.2024 um 20:42 schrieb Heiko Stübner:
> Am Mittwoch, 12. Juni 2024, 22:48:12 CEST schrieb Sebastian Kropatsch:
>> DT validation doesn't like the 'linux,default-trigger = "stmmac-0:01:link"'
>> properties, since "*:link" is not a valid value according to
>> [Documentation/devicetree/bindings/leds/common.yaml]. These LEDs do
>> have the specific purpose to show if an Ethernet link is up though.
>> There is one LED for each Ethernet port and they are labeled WAN and
>> LAN.
>> Using the 'linux,default-trigger' like this does work perfectly fine
>> with this solution. I could not find another way to achieve this. Please
>> let me know if there is a better way.
>> Maybe it would also be valid to add an entry to the DT bindings file to
>> allow "*:link" as a value for 'linux,default-trigger'?
> 
> correct. If needed, things should be added to binding.

Alright, I'll add this to the binding at
[Documentation/devicetree/bindings/leds/common.yaml]

Should this be a completely separate patch or is it preferred
to add a patch to this NanoPi patch series?

Cheers,
Sebastian
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6.dtsi
index 8b90bae28302..69d0f1f2349b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6.dtsi
@@ -8,6 +8,7 @@ 
 #include <dt-bindings/pinctrl/rockchip.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
+#include <dt-bindings/leds/common.h>
 #include "rk3588s.dtsi"
 
 / {
@@ -52,7 +53,8 @@  leds {
 		compatible = "gpio-leds";
 
 		sys_led: led-0 {
-			label = "sys_led";
+			color = <LED_COLOR_ID_RED>;
+			function = LED_FUNCTION_HEARTBEAT;
 			gpios = <&gpio1 RK_PC1 GPIO_ACTIVE_HIGH>;
 			linux,default-trigger = "heartbeat";
 			pinctrl-names = "default";
@@ -60,24 +62,22 @@  sys_led: led-0 {
 		};
 
 		wan_led: led-1 {
-			label = "wan_led";
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_WAN;
 			gpios = <&gpio1 RK_PC2 GPIO_ACTIVE_HIGH>;
+			linux,default-trigger = "stmmac-0:01:link";
 			pinctrl-names = "default";
 			pinctrl-0 = <&wan_led_pin>;
 		};
 
 		lan1_led: led-2 {
-			label = "lan1_led";
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_LAN;
 			gpios = <&gpio1 RK_PC3 GPIO_ACTIVE_HIGH>;
+			linux,default-trigger = "r8169-3-3100:00:link";
 			pinctrl-names = "default";
 			pinctrl-0 = <&lan1_led_pin>;
 		};
-
-		lan2_led: led-3 {
-			gpios = <&gpio1 RK_PC4 GPIO_ACTIVE_HIGH>;
-			pinctrl-names = "default";
-			pinctrl-0 = <&lan2_led_pin>;
-		};
 	};
 
 	vcc_5v0: regulator-vcc-5v0 {
@@ -328,23 +328,15 @@  key1_pin: key1-pin {
 
 	gpio-leds {
 		sys_led_pin: sys-led-pin {
-			rockchip,pins =
-				<1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_none>;
+			rockchip,pins = <1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_none>;
 		};
 
 		wan_led_pin: wan-led-pin {
-			rockchip,pins =
-				<1 RK_PC2 RK_FUNC_GPIO &pcfg_pull_none>;
+			rockchip,pins = <1 RK_PC2 RK_FUNC_GPIO &pcfg_pull_none>;
 		};
 
 		lan1_led_pin: lan1-led-pin {
-			rockchip,pins =
-				<1 RK_PC3 RK_FUNC_GPIO &pcfg_pull_none>;
-		};
-
-		lan2_led_pin: lan2-led-pin {
-			rockchip,pins =
-				<1 RK_PC4 RK_FUNC_GPIO &pcfg_pull_none>;
+			rockchip,pins = <1 RK_PC3 RK_FUNC_GPIO &pcfg_pull_none>;
 		};
 	};
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6c.dts b/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6c.dts
index 24dcd3e07ea7..d1b6aa033abc 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6c.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6c.dts
@@ -8,6 +8,18 @@  / {
 	model = "FriendlyElec NanoPi R6C";
 	compatible = "friendlyarm,nanopi-r6c", "rockchip,rk3588s";
 
+	leds {
+		compatible = "gpio-leds";
+
+		led1_led: led-3 {
+			color = <LED_COLOR_ID_GREEN>;
+			function = "led1";
+			gpios = <&gpio1 RK_PC4 GPIO_ACTIVE_HIGH>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&led1_led_pin>;
+		};
+	};
+
 	/* MP2143DJ power switch (U9536 in schematics) */
 	vcc3v3_pcie: regulator-vcc3v3-pcie {
 		compatible = "regulator-fixed";
@@ -24,16 +36,18 @@  vcc3v3_pcie: regulator-vcc3v3-pcie {
 	};
 };
 
-&lan2_led {
-	label = "user_led";
-};
-
 /* M.2 M-Key socket */
 &pcie2x1l2 {
 	vpcie3v3-supply = <&vcc3v3_pcie>;
 };
 
 &pinctrl {
+	gpio-leds {
+		led1_led_pin: led1-led-pin {
+			rockchip,pins = <1 RK_PC4 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
 	pcie {
 		pcie20x1_2_con_pwren: pcie20x1-2-con-pwren {
 			rockchip,pins = <3 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6s.dts b/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6s.dts
index d24110b6cf5d..09afbc0a2581 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6s.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6s.dts
@@ -7,13 +7,35 @@ 
 / {
 	model = "FriendlyElec NanoPi R6S";
 	compatible = "friendlyarm,nanopi-r6s", "rockchip,rk3588s";
+
+	leds {
+		compatible = "gpio-leds";
+
+		lan2_led: led-3 {
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_LAN;
+			function-enumerator = <2>;
+			gpios = <&gpio1 RK_PC4 GPIO_ACTIVE_HIGH>;
+			linux,default-trigger = "r8169-4-4100:00:link";
+			pinctrl-names = "default";
+			pinctrl-0 = <&lan2_led_pin>;
+		};
+	};
 };
 
-&lan2_led {
-	label = "lan2_led";
+&lan1_led {
+	function-enumerator = <1>;
 };
 
 /* RTL8125BG Ethernet */
 &pcie2x1l2 {
 	vpcie3v3-supply = <&vcc_3v3_s3>;
 };
+
+&pinctrl {
+	gpio-leds {
+		lan2_led_pin: lan2-led-pin {
+			rockchip,pins = <1 RK_PC4 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+};