| Message ID | 20240206185400.596979-4-aren@peacevolution.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/4] leds: rgb: leds-group-multicolor: allow leds to stay on in suspend | expand |
On Thu, 22 Feb 2024, 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. Works for me - I'll go apply it now.
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? 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>; > > }; > > > > > > > >
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>; > > > }; > > > > > > > > > > > > > > >
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>; };
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> --- 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(-)