| Message ID | 20240701164952.577277-1-krzysztof.kozlowski@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ARM: dts: microchip: at91: align LED node name with bindings | expand |
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 > >
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
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>; };
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(-)