Dne ponedeljek, 11. september 2023 ob 16:54:18 CEST je Andre Przywara napisal(a): > On Mon, 11 Sep 2023 22:09:59 +0800 > longqi <longqi90@xxxxxxxxx> wrote: > > Hi, > > please add a few lines here saying why you fix this: it looks like the red > LED wasn't working at all, because of the wrong GPIO, and the colours were > actually swapped? This would be good to know for everyone reading this > patch. Additionally, add "Fixes" tag, since it's a bug in DT. In any case, go through https://www.kernel.org/doc/html/latest/process/submitting-patches.html so your patch will be correctly formatted. Best regards, Jernej > > In general this is a good catch, so thanks for sending this! > > More below: > > Signed-off-by: longqi <longqi90@xxxxxxxxx> > > --- > > > > .../boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dts | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dts > > b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dts index > > 4c3921ac236c..0cb7a07ab1de 100644 > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dts > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dts > > @@ -26,14 +26,15 @@ leds { > > > > compatible = "gpio-leds"; > > > > led-0 { > > > > - label = "nanopi:green:pwr"; > > - gpios = <&r_pio 0 10 GPIO_ACTIVE_HIGH>; > > - default-state = "on"; > > + label = "nanopi:red:pwr"; > > label is now a deprecated property. That's fine for existing DTs, but I > guess touching this node means you should update this, by using "function" > and "color", as described in: > Documentation/devicetree/bindings/leds/common.yaml > > > + gpios = <&r_pio 0 10 GPIO_ACTIVE_HIGH>; /* PL10 */ > > + linux,default-trigger = "default-on"; > > > > }; > > > > led-1 { > > > > - label = "nanopi:red:status"; > > - gpios = <&pio 0 20 GPIO_ACTIVE_HIGH>; > > + label = "nanopi:green:status"; > > Same thing with label here, please use "function" and "color". > > > + gpios = <&pio 0 10 GPIO_ACTIVE_HIGH>; /* PA10 */ > > + linux,default-trigger = "heartbeat"; > > I don't think we assign default triggers for LED merely labelled "status", > as people might have different use cases for this. For instance I'd find > a USB or eMMC trigger more useful. So you can just drop that line. > > Cheers, > Andre
