Re: [PATCH v2 4/4] arm64: dts: sun50i-a64-pinephone: change led type to status

From: "Jernej Škrabec" <jernej.skrabec@gmail.com>
To: Aren <aren@peacevolution.org>
Cc: linux-kernel@vger.kernel.org,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	devicetree@vger.kernel.org,
	Jean-Jacques Hiblot <jjhiblot@traphandler.com>,
	Chen-Yu Tsai <wens@csie.org>, Ondrej Jirman <megi@xff.cz>,
	linux-sunxi@lists.linux.dev, Lee Jones <lee@kernel.org>,
	Pavel Machek <pavel@ucw.cz>,
	linux-arm-kernel@lists.infradead.org, linux-leds@vger.kernel.org,
	Conor Dooley <conor+dt@kernel.org>, Miles Alan <m@milesalan.com>,
	Samuel Holland <samuel@sholland.org>
Subject: Re: [PATCH v2 4/4] arm64: dts: sun50i-a64-pinephone: change led type to status
Date: Fri, 23 Feb 2024 17:36:42 +0100	[thread overview]
Message-ID: <4545459.LvFx2qVVIh@jernej-laptop> (raw)
In-Reply-To: <n5rmhx7ez7xoqainjqxpdk47e3bw2pvtsgswofnhjdxtrk72j2@debhbdxsxz4m>

Dne petek, 23. februar 2024 ob 17:30:00 CET je Aren napisal(a):
> On Thu, Feb 22, 2024 at 09:57:00PM +0100, Jernej Škrabec wrote:
> > Dne torek, 06. februar 2024 ob 19:13:20 CET je Aren Moynihan napisal(a):
> > > The status function is described in the documentation as being a rgb led
> > > used for system notifications on phones[1][2]. This is exactly what this
> > > led is used for on the PinePhone, so using status is probably more
> > > accurate than indicator.
> > > 
> > > 1: Documentation/leds/well-known-leds.txt
> > > 2: include/dt-bindings/leds/common.h
> > > 
> > > Signed-off-by: Aren Moynihan <aren@peacevolution.org>
> > 
> > Sorry for late review.
> > 
> > Please update subject in patches 2-3. Instead of "sun50i-a64-pinephone:"
> > use "allwinner: pinephone:" (check commit history of sun50i-a64-pinephone.dtsi).
> > Also rgb -> RGB, led -> LED. Last, please reword commit message to exclude
> > links and just say DT bindings documentation.
> > 
> > Note that I'll merge patches 2-3 once patch 1 is merged.
> 
> Would you like me to reword and resend the patches, or is it quicker
> for you to just do it when you apply them?

Since Ondřej raised concerns, let's finish that discussion first. It's possible
that this patch will be rejected. That would also mean new revision of patches.

Sadly, this means DT patches will miss v6.9 window.

Best regards,
Jernej

> 
> Thanks for taking a look at this,
>  - Aren
> 
> > Best regards,
> > Jernej
> > 
> > > ---
> > > I can't find any documentation describing the indicator function, so
> > > it's definitely less specific than status, but besides that I'm not sure
> > > how it compares. Please ignore this patch if it's not useful.
> > > 
> > > (no changes since v1)
> > > 
> > >  arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> > > index e53e0d4579a7..6d327266e6cc 100644
> > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> > > @@ -61,7 +61,7 @@ led2: led-2 {
> > >  	multi-led {
> > >  		compatible = "leds-group-multicolor";
> > >  		color = <LED_COLOR_ID_RGB>;
> > > -		function = LED_FUNCTION_INDICATOR;
> > > +		function = LED_FUNCTION_STATUS;
> > >  		leds = <&led0>, <&led1>, <&led2>;
> > >  	};
> > >  
> > > 
> > 
> > 
> > 
> > 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel