Re: Re: [PATCH 1/2] ARM: dts: sunxi: Fix GPIO LED node names

[Jernej Škrabec]

Hi Samuel,

Dne petek, 25. november 2022 ob 22:50:07 CET je Samuel Holland napisal(a):
> Hi Andre,
> 
> On 11/25/22 15:40, Andre Przywara wrote:
> > On Fri, 25 Nov 2022 13:54:00 -0600
> > Samuel Holland <samuel@xxxxxxxxxxxx> wrote:
> > 
> > Hi Samuel,
> > 
> >> These board devicetrees fail to validate because the gpio-leds schema
> >> requires its child nodes to have "led" in the node name.
> >> 
> >> Signed-off-by: Samuel Holland <samuel@xxxxxxxxxxxx>
> > 
> > That looks alright, though the comment in the binding says that we
> > should just have led-0, led-1 instead, so just (hex) numbers. The
> > "status" name is also in the label, so we wouldn't lose information.
> 
> I am not a fan of giving the LEDs meaningless enumerators, but I can do
> that if the maintainers insist.

I'm not a fan of that either, but binding really wants enumerator. So let's 
conform to that.

Best regards,
Jernej

> 
> > Actually, also "label" is deprecated, in favour of "color" and
> > "function", shall this be fixed on the way? Or is there anything that
> > breaks (older kernels) when removing the label property?
> 
> The label is exposed to userspace as the path in sysfs, so we cannot
> change it. There is no way to construct that exact label using function
> and color -- see led_compose_name().
> 
> Regards,
> Samuel
> 
> >> ---
> >> 
> >>  arch/arm/boot/dts/sun5i-gr8-chip-pro.dts | 2 +-
> >>  arch/arm/boot/dts/sun5i-r8-chip.dts      | 2 +-
> >>  arch/arm/boot/dts/sun6i-a31s-sina31s.dts | 2 +-
> >>  3 files changed, 3 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts
> >> b/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts index
> >> a32cde3e32eb..3222f1490716 100644
> >> --- a/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts
> >> +++ b/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts
> >> @@ -70,7 +70,7 @@ chosen {
> >> 
> >>  	leds {
> >>  	
> >>  		compatible = "gpio-leds";
> >> 
> >> -		status {
> >> +		led-status {
> >> 
> >>  			label = "chip-pro:white:status";
> >>  			gpios = <&axp_gpio 2 GPIO_ACTIVE_HIGH>;
> >>  			default-state = "on";
> >> 
> >> diff --git a/arch/arm/boot/dts/sun5i-r8-chip.dts
> >> b/arch/arm/boot/dts/sun5i-r8-chip.dts index 4bf4943d4eb7..303191c926c2
> >> 100644
> >> --- a/arch/arm/boot/dts/sun5i-r8-chip.dts
> >> +++ b/arch/arm/boot/dts/sun5i-r8-chip.dts
> >> @@ -70,7 +70,7 @@ chosen {
> >> 
> >>  	leds {
> >>  	
> >>  		compatible = "gpio-leds";
> >> 
> >> -		status {
> >> +		led-status {
> >> 
> >>  			label = "chip:white:status";
> >>  			gpios = <&axp_gpio 2 GPIO_ACTIVE_HIGH>;
> >>  			default-state = "on";
> >> 
> >> diff --git a/arch/arm/boot/dts/sun6i-a31s-sina31s.dts
> >> b/arch/arm/boot/dts/sun6i-a31s-sina31s.dts index
> >> 0af48e143b66..b84822453381 100644
> >> --- a/arch/arm/boot/dts/sun6i-a31s-sina31s.dts
> >> +++ b/arch/arm/boot/dts/sun6i-a31s-sina31s.dts
> >> @@ -67,7 +67,7 @@ hdmi_con_in: endpoint {
> >> 
> >>  	leds {
> >>  	
> >>  		compatible = "gpio-leds";
> >> 
> >> -		status {
> >> +		led-status {
> >> 
> >>  			label = "sina31s:status:usr";
> >>  			gpios = <&pio 7 13 GPIO_ACTIVE_HIGH>; /* 
PH13 */
> >>  		
> >>  		};