diff mbox series

[v1,4/4] riscv: dts: sifive unmatched: Add PWM controlled LEDs

Message ID 20220705210143.315151-5-emil.renner.berthing@canonical.com (mailing list archive)
State New, archived
Headers show
Series Add HiFive Unmatched LEDs | expand

Commit Message

Emil Renner Berthing July 5, 2022, 9:01 p.m. UTC
This adds the two PWM controlled LEDs to the HiFive Unmatched device
tree. D12 is just a regular green diode, but D2 is an RGB diode with 3
PWM inputs controlling the three different colours.

Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
---
 .../boot/dts/sifive/hifive-unmatched-a00.dts  | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)

Comments

Geert Uytterhoeven July 6, 2022, 7:56 a.m. UTC | #1
Hi Emil,

On Tue, Jul 5, 2022 at 11:01 PM Emil Renner Berthing
<emil.renner.berthing@canonical.com> wrote:
> This adds the two PWM controlled LEDs to the HiFive Unmatched device
> tree. D12 is just a regular green diode, but D2 is an RGB diode with 3
> PWM inputs controlling the three different colours.
>
> Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

Thanks for your patch!

> --- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> +++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> @@ -44,6 +46,46 @@ gpio-poweroff {
>                 compatible = "gpio-poweroff";
>                 gpios = <&gpio 2 GPIO_ACTIVE_LOW>;
>         };
> +
> +       led-controller-1 {
> +               compatible = "pwm-leds";
> +
> +               led-d12 {
> +                       pwms = <&pwm0 0 7812500 PWM_POLARITY_INVERTED>;
> +                       active-low;

The first thing that came into my mind was "why not drop the
PWM_POLARITY_INVERTED flag instead?".

But it turns out drivers/pwm/pwm-sifive.c does not support
non-inverted PWMs, and returns -EINVAL if PWM_POLARITY_INVERSED
(no typo) is not set.  I think it would be good if
Documentation/devicetree/bindings/pwm/pwm-sifive.yaml would mention
this limitation, and perhaps even enforce it, if possible?

I didn't check this against the schematics, but the generic structure
LGTM, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Ron Economos July 8, 2022, 9:21 a.m. UTC | #2
On 7/5/22 2:01 PM, Emil Renner Berthing wrote:
> This adds the two PWM controlled LEDs to the HiFive Unmatched device
> tree. D12 is just a regular green diode, but D2 is an RGB diode with 3
> PWM inputs controlling the three different colours.
>
> Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> ---
>   .../boot/dts/sifive/hifive-unmatched-a00.dts  | 42 +++++++++++++++++++
>   1 file changed, 42 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> index c4ed9efdff03..beaefe74755a 100644
> --- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> +++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> @@ -4,6 +4,8 @@
>   #include "fu740-c000.dtsi"
>   #include <dt-bindings/gpio/gpio.h>
>   #include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pwm/pwm.h>
>   
>   /* Clock frequency (in Hz) of the PCB crystal for rtcclk */
>   #define RTCCLK_FREQ		1000000
> @@ -44,6 +46,46 @@ gpio-poweroff {
>   		compatible = "gpio-poweroff";
>   		gpios = <&gpio 2 GPIO_ACTIVE_LOW>;
>   	};
> +
> +	led-controller-1 {
> +		compatible = "pwm-leds";
> +
> +		led-d12 {
> +			pwms = <&pwm0 0 7812500 PWM_POLARITY_INVERTED>;
> +			active-low;
> +			color = <LED_COLOR_ID_GREEN>;
> +			max-brightness = <255>;
> +			label = "d12";
> +		};
> +	};
> +
> +	led-controller-2 {
> +		compatible = "pwm-leds-multicolor";
> +
> +		multi-led {
> +			color = <LED_COLOR_ID_RGB>;
> +			max-brightness = <255>;
> +			label = "d2";
> +
> +			led-red {
> +				pwms = <&pwm0 2 7812500 PWM_POLARITY_INVERTED>;
> +				active-low;
> +				color = <LED_COLOR_ID_RED>;
> +			};
> +
> +			led-green {
> +				pwms = <&pwm0 1 7812500 PWM_POLARITY_INVERTED>;
> +				active-low;
> +				color = <LED_COLOR_ID_GREEN>;
> +			};
> +
> +			led-blue {
> +				pwms = <&pwm0 3 7812500 PWM_POLARITY_INVERTED>;
> +				active-low;
> +				color = <LED_COLOR_ID_BLUE>;
> +			};
> +		};
> +	};
>   };
>   
>   &uart0 {

Tested on HiFive Unmatched with both udev and systemd. Works good.

Tested-by: Ron Economos <re@w6rz.net>
Pavel Machek July 17, 2022, 10:57 a.m. UTC | #3
On Tue 2022-07-05 23:01:43, Emil Renner Berthing wrote:
> This adds the two PWM controlled LEDs to the HiFive Unmatched device
> tree. D12 is just a regular green diode, but D2 is an RGB diode with 3
> PWM inputs controlling the three different colours.
> 
> Signed-off-by: Emil Renner Berthing
<emil.renner.berthing@canonical.com>

Acked-by: Pavel Machek <pavel@ucw.cz>

(This is dts change, I'd rather not take it through the LED tree).

Best regards,
							Pavel
Emil Renner Berthing July 17, 2022, 2:05 p.m. UTC | #4
On Sun, 17 Jul 2022 at 12:59, Pavel Machek <pavel@ucw.cz> wrote:
> On Tue 2022-07-05 23:01:43, Emil Renner Berthing wrote:
> > This adds the two PWM controlled LEDs to the HiFive Unmatched device
> > tree. D12 is just a regular green diode, but D2 is an RGB diode with 3
> > PWM inputs controlling the three different colours.
> >
> > Signed-off-by: Emil Renner Berthing
> <emil.renner.berthing@canonical.com>
>
> Acked-by: Pavel Machek <pavel@ucw.cz>
>
> (This is dts change, I'd rather not take it through the LED tree).

Makes sense. Palmer, will you consider this?

/Emil
> Best regards,
>                                                         Pavel
>
> --
> People of Russia, stop Putin before his war on Ukraine escalates.
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
diff mbox series

Patch

diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
index c4ed9efdff03..beaefe74755a 100644
--- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
+++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
@@ -4,6 +4,8 @@ 
 #include "fu740-c000.dtsi"
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/pwm/pwm.h>
 
 /* Clock frequency (in Hz) of the PCB crystal for rtcclk */
 #define RTCCLK_FREQ		1000000
@@ -44,6 +46,46 @@  gpio-poweroff {
 		compatible = "gpio-poweroff";
 		gpios = <&gpio 2 GPIO_ACTIVE_LOW>;
 	};
+
+	led-controller-1 {
+		compatible = "pwm-leds";
+
+		led-d12 {
+			pwms = <&pwm0 0 7812500 PWM_POLARITY_INVERTED>;
+			active-low;
+			color = <LED_COLOR_ID_GREEN>;
+			max-brightness = <255>;
+			label = "d12";
+		};
+	};
+
+	led-controller-2 {
+		compatible = "pwm-leds-multicolor";
+
+		multi-led {
+			color = <LED_COLOR_ID_RGB>;
+			max-brightness = <255>;
+			label = "d2";
+
+			led-red {
+				pwms = <&pwm0 2 7812500 PWM_POLARITY_INVERTED>;
+				active-low;
+				color = <LED_COLOR_ID_RED>;
+			};
+
+			led-green {
+				pwms = <&pwm0 1 7812500 PWM_POLARITY_INVERTED>;
+				active-low;
+				color = <LED_COLOR_ID_GREEN>;
+			};
+
+			led-blue {
+				pwms = <&pwm0 3 7812500 PWM_POLARITY_INVERTED>;
+				active-low;
+				color = <LED_COLOR_ID_BLUE>;
+			};
+		};
+	};
 };
 
 &uart0 {