Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Jernej =?utf-8?B?xaBrcmFiZWM=?= <jernej.skrabec@gmail.com>
To:     Andre Przywara <andre.przywara@arm.com>,
        Samuel Holland <samuel@sholland.org>
Cc:     Chen-Yu Tsai <wens@csie.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Rob Herring <robh+dt@kernel.org>, devicetree@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-sunxi@lists.linux.dev
Subject: Re: Re: [PATCH 1/2] ARM: dts: sunxi: Fix GPIO LED node names
Date:   Mon, 05 Dec 2022 22:07:20 +0100
Message-ID: <2324364.NG923GbCHz@kista>
In-Reply-To: <2979be86-e561-e5ff-b348-367a7c20fab1@sholland.org>
References: <20221125195401.61642-1-samuel@sholland.org> <20221125214050.711997f1@slackpad.lan> <2979be86-e561-e5ff-b348-367a7c20fab1@sholland.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

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@sholland.org> 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@sholland.org>
> > 
> > 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 */
> >>  		
> >>  		};