Re: [PATCH] riscv: Add gpio and pwmleds to DTS(/arch/riscv/boot/dts/sifive/)

[JaeJoon Jung <rgbi3307@gmail.com>]

I agree with you because LEDs are user defined using method.

And, I am sorry about that I confirmed lately below posted by Yash.

https://lore.kernel.org/linux-riscv/mhng-cb360722-bdb6-4cf7-9fa7-1d92f6b6bbfa@palmerdabbelt-glaptop1/T/#madb19f55bac11a9a675b1ca73ca3f0c2d88c57cf

It is helpful for me and I am testing it.
If I find a bug, I am going to share about it.

Thanks a lot, Have a nice day.

Yours,
JaeJoon Jung

On Thu, 30 Jan 2020 at 12:35, Paul Walmsley <paul.walmsley@sifive.com> wrote:
>
> On Tue, 21 Jan 2020, JaeJoon Jung wrote:
>
> > I added below DTS to act gpio and pwmleds for SiFive FU540 Unleashed board.
> >
> > diff --git a/arch/riscv/boot/dts/sifive/fu540-c000.dtsi
> > b/arch/riscv/boot/dts/sifive/fu540-c000.dtsi
> > index a2e3d54e830c..b03bf570020c 100644
> > --- a/arch/riscv/boot/dts/sifive/fu540-c000.dtsi
> > +++ b/arch/riscv/boot/dts/sifive/fu540-c000.dtsi
> >
> > +                gpio0: gpio@10060000 {
> > +                        compatible = "sifive,fu540-c000-gpio", "sifive,gpio0";
> > +                        reg = <0x0 0x10060000 0x0 0x1000>;
> > +                        reg-names = "control";
> > +                        gpio-controller;
> > +                        #gpio-cells = <2>;
> > +                        ngpios = <16>;
> > +                        interrupt-controller;
> > +                        #interrupt-cells = <2>;
> > +                        interrupt-parent = <&plic0>;
> > +                        interrupts = <15 16 17 18 19 20 21 22 23 24
> > 25 26 27 28 29 30>;
> > +                        status = "disabled";
> > +                };
>
> Yash posted this a while ago:
>
> https://lore.kernel.org/linux-riscv/mhng-cb360722-bdb6-4cf7-9fa7-1d92f6b6bbfa@palmerdabbelt-glaptop1/T/#madb19f55bac11a9a675b1ca73ca3f0c2d88c57cf
>
>
> >
> > diff --git a/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> > b/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> > index 88cfcb96bf23..f3f55dbbf737 100644
> > --- a/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> > +++ b/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> >
> >         cpus {
> > @@ -41,6 +41,39 @@
> >                 clock-frequency = <RTCCLK_FREQ>;
> >                 clock-output-names = "rtcclk";
> >         };
> > +
> > +
> > +        pwmleds {
> > +                compatible = "pwm-leds";
> > +                heartbeat {
> > +                        label = "led1";
> > +                        max-brightness = <255>;
> > +                        active-low = <1>;
> > +                        pwms = <&pwm0 0 7812500 0>;
> > +                        linux,default-trigger = "heartbeat";
> > +                };
> > +                mtd {
> > +                        label = "led2";
> > +                        max-brightness = <255>;
> > +                        active-low = <1>;
> > +                        pwms = <&pwm0 1 7812500 0>;
> > +                        linux,default-trigger = "mtd";
> > +                };
> > +                netdev {
> > +                        label = "led3";
> > +                        max-brightness = <255>;
> > +                        active-low = <1>;
> > +                        pwms = <&pwm0 2 7812500 0>;
> > +                        linux,default-trigger = "netdev";
> > +                };
> > +                panic {
> > +                        label = "led4";
> > +                        max-brightness = <255>;
> > +                        active-low = <1>;
> > +                        pwms = <&pwm0 3 7812500 0>;
> > +                        linux,default-trigger = "panic";
> > +                };
> > +        };
> >  };
>
> I don't think it's good to add these pwmleds to the default board DTS
> file.  I realize that many upstream ARM development boards have added this
> type of configuration, but it gets in the way of reusing this DT file when
> integrators wish to have the LEDs used for different purposes.  If the
> Unleashed silkscreen had text on it describing the LEDs as having these
> specific functions, or if Unleashed was sold in a case with similar
> markings on the outside, it'd be a different story, and then a change like
> the above could make sense.
>
>
> - Paul