diff mbox series

ARM: dts: microchip: at91: align LED node name with bindings

Message ID 20240701164952.577277-1-krzysztof.kozlowski@linaro.org (mailing list archive)
State New, archived
Headers show
Series ARM: dts: microchip: at91: align LED node name with bindings | expand

Commit Message

Krzysztof Kozlowski July 1, 2024, 4:49 p.m. UTC
Bindings expect the LED node names to follow certain pattern, see
dtbs_check warnings:

  at91sam9g15ek.dtb: leds: 'pb18', 'pd21' do not match any of the regexes: '(^led-[0-9a-f]$|led)', 'pinctrl-[0-9]+'

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../boot/dts/microchip/at91sam9g20ek_2mmc.dts |  4 ++--
 .../at91sam9g25-gardena-smart-gateway.dts     | 24 +++++++++----------
 arch/arm/boot/dts/microchip/at91sam9n12ek.dts |  6 ++---
 arch/arm/boot/dts/microchip/at91sam9x5cm.dtsi |  4 ++--
 4 files changed, 19 insertions(+), 19 deletions(-)

Comments

Alexander Dahl July 2, 2024, 8:51 a.m. UTC | #1
Hello Krzysztof,

Am Mon, Jul 01, 2024 at 06:49:52PM +0200 schrieb Krzysztof Kozlowski:
> Bindings expect the LED node names to follow certain pattern, see
> dtbs_check warnings:
> 
>   at91sam9g15ek.dtb: leds: 'pb18', 'pd21' do not match any of the regexes: '(^led-[0-9a-f]$|led)', 'pinctrl-[0-9]+'
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../boot/dts/microchip/at91sam9g20ek_2mmc.dts |  4 ++--
>  .../at91sam9g25-gardena-smart-gateway.dts     | 24 +++++++++----------
>  arch/arm/boot/dts/microchip/at91sam9n12ek.dts |  6 ++---
>  arch/arm/boot/dts/microchip/at91sam9x5cm.dtsi |  4 ++--
>  4 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/microchip/at91sam9g20ek_2mmc.dts b/arch/arm/boot/dts/microchip/at91sam9g20ek_2mmc.dts
> index 172af6ff4b18..3e5eab57d1a5 100644
> --- a/arch/arm/boot/dts/microchip/at91sam9g20ek_2mmc.dts
> +++ b/arch/arm/boot/dts/microchip/at91sam9g20ek_2mmc.dts
> @@ -40,13 +40,13 @@ pinctrl_board_mmc0_slot0: mmc0_slot0-board {
>  	leds {
>  		compatible = "gpio-leds";
>  
> -		ds1 {
> +		led-ds1 {
>  			label = "ds1";
>  			gpios = <&pioB 9 GPIO_ACTIVE_HIGH>;
>  			linux,default-trigger = "heartbeat";
>  		};
>  
> -		ds5 {
> +		led-ds5 {
>  			label = "ds5";
>  			gpios = <&pioB 8 GPIO_ACTIVE_LOW>;
>  		};
> diff --git a/arch/arm/boot/dts/microchip/at91sam9g25-gardena-smart-gateway.dts b/arch/arm/boot/dts/microchip/at91sam9g25-gardena-smart-gateway.dts
> index af70eb8a3a02..e0c1e8df81b1 100644
> --- a/arch/arm/boot/dts/microchip/at91sam9g25-gardena-smart-gateway.dts
> +++ b/arch/arm/boot/dts/microchip/at91sam9g25-gardena-smart-gateway.dts
> @@ -37,71 +37,71 @@ button {
>  	leds {
>  		compatible = "gpio-leds";
>  
> -		power_blue {
> +		led-power-blue {
>  			label = "smartgw:power:blue";
>  			gpios = <&pioC 21 GPIO_ACTIVE_HIGH>;
>  			default-state = "off";
>  		};
>  
> -		power_green {
> +		led-power-green {
>  			label = "smartgw:power:green";
>  			gpios = <&pioC 20 GPIO_ACTIVE_HIGH>;
>  			default-state = "on";
>  		};
>  
> -		power_red {
> +		led-power-red {
>  			label = "smartgw:power:red";
>  			gpios = <&pioC 19 GPIO_ACTIVE_HIGH>;
>  			default-state = "off";
>  		};
>  
> -		radio_blue {
> +		led-radio-blue {
>  			label = "smartgw:radio:blue";
>  			gpios = <&pioC 18 GPIO_ACTIVE_HIGH>;
>  			default-state = "off";
>  		};
>  
> -		radio_green {
> +		led-radio-green {
>  			label = "smartgw:radio:green";
>  			gpios = <&pioC 17 GPIO_ACTIVE_HIGH>;
>  			default-state = "off";
>  		};
>  
> -		radio_red {
> +		led-radio-red {
>  			label = "smartgw:radio:red";
>  			gpios = <&pioC 16 GPIO_ACTIVE_HIGH>;
>  			default-state = "off";
>  		};
>  
> -		internet_blue {
> +		led-internet-blue {
>  			label = "smartgw:internet:blue";
>  			gpios = <&pioC 15 GPIO_ACTIVE_HIGH>;
>  			default-state = "off";
>  		};
>  
> -		internet_green {
> +		led-internet-green {
>  			label = "smartgw:internet:green";
>  			gpios = <&pioC 14 GPIO_ACTIVE_HIGH>;
>  			default-state = "off";
>  		};
>  
> -		internet_red {
> +		led-internet-red {
>  			label = "smartgw:internet:red";
>  			gpios = <&pioC 13 GPIO_ACTIVE_HIGH>;
>  			default-state = "off";
>  		};
>  
> -		heartbeat {
> +		led-heartbeat {
>  			label = "smartgw:heartbeat";
>  			gpios = <&pioB 8 GPIO_ACTIVE_HIGH>;
>  			linux,default-trigger = "heartbeat";
>  		};
>  
> -		pb18 {
> +		led-pb18 {
>  			status = "disabled";
>  		};
>  
> -		pd21 {
> +		led-pd21 {
>  			status = "disabled";
>  		};
>  	};
> diff --git a/arch/arm/boot/dts/microchip/at91sam9n12ek.dts b/arch/arm/boot/dts/microchip/at91sam9n12ek.dts
> index 4c644d4c6be7..643c3b2ab97e 100644
> --- a/arch/arm/boot/dts/microchip/at91sam9n12ek.dts
> +++ b/arch/arm/boot/dts/microchip/at91sam9n12ek.dts
> @@ -207,19 +207,19 @@ bl_reg: backlight_regulator {
>  	leds {
>  		compatible = "gpio-leds";
>  
> -		d8 {
> +		led-d8 {
>  			label = "d8";
>  			gpios = <&pioB 4 GPIO_ACTIVE_LOW>;
>  			linux,default-trigger = "mmc0";
>  		};
>  
> -		d9 {
> +		led-d9 {
>  			label = "d9";
>  			gpios = <&pioB 5 GPIO_ACTIVE_LOW>;
>  			linux,default-trigger = "nand-disk";
>  		};
>  
> -		d10 {
> +		led-d10 {
>  			label = "d10";
>  			gpios = <&pioB 6 GPIO_ACTIVE_HIGH>;
>  			linux,default-trigger = "heartbeat";
> diff --git a/arch/arm/boot/dts/microchip/at91sam9x5cm.dtsi b/arch/arm/boot/dts/microchip/at91sam9x5cm.dtsi
> index cdd37f67280b..fb3c19bdfcb6 100644
> --- a/arch/arm/boot/dts/microchip/at91sam9x5cm.dtsi
> +++ b/arch/arm/boot/dts/microchip/at91sam9x5cm.dtsi
> @@ -120,13 +120,13 @@ rootfs@800000 {
>  	leds {
>  		compatible = "gpio-leds";
>  
> -		pb18 {
> +		led-pb18 {
>  			label = "pb18";
>  			gpios = <&pioB 18 GPIO_ACTIVE_LOW>;
>  			linux,default-trigger = "heartbeat";
>  		};
>  
> -		pd21 {
> +		led-pd21 {
>  			label = "pd21";
>  			gpios = <&pioD 21 GPIO_ACTIVE_HIGH>;
>  		};

In this case these are all gpio-leds and the pattern is in the
leds-gpio gpio binding.  I'm wondering however why you chose the very
generic 'led' match over the more strict one requiring the names to
look like 'led-0', 'led-1' an so forth?  The generic match would also
match names like 'knowledge' or 'controlled'.  But besides that:

Reviewed-by: Alexander Dahl <ada@thorsis.com>

Greets
Alex


> -- 
> 2.43.0
> 
>
Krzysztof Kozlowski July 2, 2024, 10:31 a.m. UTC | #2
On 02/07/2024 10:51, Alexander Dahl wrote:
>> diff --git a/arch/arm/boot/dts/microchip/at91sam9x5cm.dtsi b/arch/arm/boot/dts/microchip/at91sam9x5cm.dtsi
>> index cdd37f67280b..fb3c19bdfcb6 100644
>> --- a/arch/arm/boot/dts/microchip/at91sam9x5cm.dtsi
>> +++ b/arch/arm/boot/dts/microchip/at91sam9x5cm.dtsi
>> @@ -120,13 +120,13 @@ rootfs@800000 {
>>  	leds {
>>  		compatible = "gpio-leds";
>>  
>> -		pb18 {
>> +		led-pb18 {
>>  			label = "pb18";
>>  			gpios = <&pioB 18 GPIO_ACTIVE_LOW>;
>>  			linux,default-trigger = "heartbeat";
>>  		};
>>  
>> -		pd21 {
>> +		led-pd21 {
>>  			label = "pd21";
>>  			gpios = <&pioD 21 GPIO_ACTIVE_HIGH>;
>>  		};
> 
> In this case these are all gpio-leds and the pattern is in the
> leds-gpio gpio binding.  I'm wondering however why you chose the very
> generic 'led' match over the more strict one requiring the names to

match? Which match? I did not write the match pattern in the binding, did I?

> look like 'led-0', 'led-1' an so forth?  The generic match would also

Works for me too. The easiest was to add led prefix. I am not the
maintainer of this platform, so I am doing just some random cleanups and
prefix is the easiest cleanup.

> match names like 'knowledge' or 'controlled'.  But besides that:

Best regards,
Krzysztof
Claudiu Beznea Aug. 9, 2024, 6:11 a.m. UTC | #3
On 01.07.2024 19:49, Krzysztof Kozlowski wrote:
> Bindings expect the LED node names to follow certain pattern, see
> dtbs_check warnings:
> 
>   at91sam9g15ek.dtb: leds: 'pb18', 'pd21' do not match any of the regexes: '(^led-[0-9a-f]$|led)', 'pinctrl-[0-9]+'
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Applied to at91-dt, thanks!
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/microchip/at91sam9g20ek_2mmc.dts b/arch/arm/boot/dts/microchip/at91sam9g20ek_2mmc.dts
index 172af6ff4b18..3e5eab57d1a5 100644
--- a/arch/arm/boot/dts/microchip/at91sam9g20ek_2mmc.dts
+++ b/arch/arm/boot/dts/microchip/at91sam9g20ek_2mmc.dts
@@ -40,13 +40,13 @@  pinctrl_board_mmc0_slot0: mmc0_slot0-board {
 	leds {
 		compatible = "gpio-leds";
 
-		ds1 {
+		led-ds1 {
 			label = "ds1";
 			gpios = <&pioB 9 GPIO_ACTIVE_HIGH>;
 			linux,default-trigger = "heartbeat";
 		};
 
-		ds5 {
+		led-ds5 {
 			label = "ds5";
 			gpios = <&pioB 8 GPIO_ACTIVE_LOW>;
 		};
diff --git a/arch/arm/boot/dts/microchip/at91sam9g25-gardena-smart-gateway.dts b/arch/arm/boot/dts/microchip/at91sam9g25-gardena-smart-gateway.dts
index af70eb8a3a02..e0c1e8df81b1 100644
--- a/arch/arm/boot/dts/microchip/at91sam9g25-gardena-smart-gateway.dts
+++ b/arch/arm/boot/dts/microchip/at91sam9g25-gardena-smart-gateway.dts
@@ -37,71 +37,71 @@  button {
 	leds {
 		compatible = "gpio-leds";
 
-		power_blue {
+		led-power-blue {
 			label = "smartgw:power:blue";
 			gpios = <&pioC 21 GPIO_ACTIVE_HIGH>;
 			default-state = "off";
 		};
 
-		power_green {
+		led-power-green {
 			label = "smartgw:power:green";
 			gpios = <&pioC 20 GPIO_ACTIVE_HIGH>;
 			default-state = "on";
 		};
 
-		power_red {
+		led-power-red {
 			label = "smartgw:power:red";
 			gpios = <&pioC 19 GPIO_ACTIVE_HIGH>;
 			default-state = "off";
 		};
 
-		radio_blue {
+		led-radio-blue {
 			label = "smartgw:radio:blue";
 			gpios = <&pioC 18 GPIO_ACTIVE_HIGH>;
 			default-state = "off";
 		};
 
-		radio_green {
+		led-radio-green {
 			label = "smartgw:radio:green";
 			gpios = <&pioC 17 GPIO_ACTIVE_HIGH>;
 			default-state = "off";
 		};
 
-		radio_red {
+		led-radio-red {
 			label = "smartgw:radio:red";
 			gpios = <&pioC 16 GPIO_ACTIVE_HIGH>;
 			default-state = "off";
 		};
 
-		internet_blue {
+		led-internet-blue {
 			label = "smartgw:internet:blue";
 			gpios = <&pioC 15 GPIO_ACTIVE_HIGH>;
 			default-state = "off";
 		};
 
-		internet_green {
+		led-internet-green {
 			label = "smartgw:internet:green";
 			gpios = <&pioC 14 GPIO_ACTIVE_HIGH>;
 			default-state = "off";
 		};
 
-		internet_red {
+		led-internet-red {
 			label = "smartgw:internet:red";
 			gpios = <&pioC 13 GPIO_ACTIVE_HIGH>;
 			default-state = "off";
 		};
 
-		heartbeat {
+		led-heartbeat {
 			label = "smartgw:heartbeat";
 			gpios = <&pioB 8 GPIO_ACTIVE_HIGH>;
 			linux,default-trigger = "heartbeat";
 		};
 
-		pb18 {
+		led-pb18 {
 			status = "disabled";
 		};
 
-		pd21 {
+		led-pd21 {
 			status = "disabled";
 		};
 	};
diff --git a/arch/arm/boot/dts/microchip/at91sam9n12ek.dts b/arch/arm/boot/dts/microchip/at91sam9n12ek.dts
index 4c644d4c6be7..643c3b2ab97e 100644
--- a/arch/arm/boot/dts/microchip/at91sam9n12ek.dts
+++ b/arch/arm/boot/dts/microchip/at91sam9n12ek.dts
@@ -207,19 +207,19 @@  bl_reg: backlight_regulator {
 	leds {
 		compatible = "gpio-leds";
 
-		d8 {
+		led-d8 {
 			label = "d8";
 			gpios = <&pioB 4 GPIO_ACTIVE_LOW>;
 			linux,default-trigger = "mmc0";
 		};
 
-		d9 {
+		led-d9 {
 			label = "d9";
 			gpios = <&pioB 5 GPIO_ACTIVE_LOW>;
 			linux,default-trigger = "nand-disk";
 		};
 
-		d10 {
+		led-d10 {
 			label = "d10";
 			gpios = <&pioB 6 GPIO_ACTIVE_HIGH>;
 			linux,default-trigger = "heartbeat";
diff --git a/arch/arm/boot/dts/microchip/at91sam9x5cm.dtsi b/arch/arm/boot/dts/microchip/at91sam9x5cm.dtsi
index cdd37f67280b..fb3c19bdfcb6 100644
--- a/arch/arm/boot/dts/microchip/at91sam9x5cm.dtsi
+++ b/arch/arm/boot/dts/microchip/at91sam9x5cm.dtsi
@@ -120,13 +120,13 @@  rootfs@800000 {
 	leds {
 		compatible = "gpio-leds";
 
-		pb18 {
+		led-pb18 {
 			label = "pb18";
 			gpios = <&pioB 18 GPIO_ACTIVE_LOW>;
 			linux,default-trigger = "heartbeat";
 		};
 
-		pd21 {
+		led-pd21 {
 			label = "pd21";
 			gpios = <&pioD 21 GPIO_ACTIVE_HIGH>;
 		};