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

[Paul Walmsley <paul.walmsley@sifive.com>]

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