[PATCH v1 0/4] Add HiFive Unmatched LEDs

[PATCH v1 0/4] Add HiFive Unmatched LEDs

[Emil Renner Berthing]

This series adds support for the two LEDs on the HiFive Unmatched
RISC-V board.

Emil Renner Berthing (4):
  leds: pwm-multicolor: Don't show -EPROBE_DEFER as errors
  dt-bindings: leds: pwm-multicolor: Add active-low property
  leds: pwm-multicolor: Support active-low LEDs
  riscv: dts: sifive unmatched: Add PWM controlled LEDs

 .../bindings/leds/leds-pwm-multicolor.yaml    |  4 ++
 .../boot/dts/sifive/hifive-unmatched-a00.dts  | 42 +++++++++++++++++++
 drivers/leds/rgb/leds-pwm-multicolor.c        |  8 +++-
 3 files changed, 52 insertions(+), 2 deletions(-)

-- 
2.37.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v1 1/4] leds: pwm-multicolor: Don't show -EPROBE_DEFER as errors

[Emil Renner Berthing]

When requesting a PWM it might return -EPROBE_DEFER if it hasn't probed
yet. This is not an error, so just propagate the -EPROBE_DEFER without
logging anything. There is already dev_err_probe for exactly this
situation.

Fixes: 9fa2762110dd ("leds: Add PWM multicolor driver")
Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
---
 drivers/leds/rgb/leds-pwm-multicolor.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/leds/rgb/leds-pwm-multicolor.c b/drivers/leds/rgb/leds-pwm-multicolor.c
index 45e38708ecb1..eb67b89d28e9 100644
--- a/drivers/leds/rgb/leds-pwm-multicolor.c
+++ b/drivers/leds/rgb/leds-pwm-multicolor.c
@@ -72,8 +72,7 @@ static int iterate_subleds(struct device *dev, struct pwm_mc_led *priv,
 		pwmled = &priv->leds[priv->mc_cdev.num_colors];
 		pwmled->pwm = devm_fwnode_pwm_get(dev, fwnode, NULL);
 		if (IS_ERR(pwmled->pwm)) {
-			ret = PTR_ERR(pwmled->pwm);
-			dev_err(dev, "unable to request PWM: %d\n", ret);
+			ret = dev_err_probe(dev, PTR_ERR(pwmled->pwm), "unable to request PWM\n");
 			goto release_fwnode;
 		}
 		pwm_init_state(pwmled->pwm, &pwmled->state);
-- 
2.37.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v1 1/4] leds: pwm-multicolor: Don't show -EPROBE_DEFER as errors

[Geert Uytterhoeven]

On Tue, Jul 5, 2022 at 11:01 PM Emil Renner Berthing
<emil.renner.berthing@canonical.com> wrote:
> When requesting a PWM it might return -EPROBE_DEFER if it hasn't probed
> yet. This is not an error, so just propagate the -EPROBE_DEFER without
> logging anything. There is already dev_err_probe for exactly this
> situation.
>
> Fixes: 9fa2762110dd ("leds: Add PWM multicolor driver")
> Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v1 2/4] dt-bindings: leds: pwm-multicolor: Add active-low property

[Emil Renner Berthing]

Add the active-low property to LEDs that are part of a multicolor LED
just like the regular PWM LEDs have.

Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
---
 .../devicetree/bindings/leds/leds-pwm-multicolor.yaml         | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
index 6625a528f727..8d614eaf5c5e 100644
--- a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
+++ b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
@@ -33,6 +33,10 @@ properties:
 
           pwm-names: true
 
+          active-low:
+            description: For PWMs where the LED is wired to supply rather than ground.
+            type: boolean
+
           color: true
 
         required:
-- 
2.37.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v1 2/4] dt-bindings: leds: pwm-multicolor: Add active-low property

[Geert Uytterhoeven]

On Tue, Jul 5, 2022 at 11:01 PM Emil Renner Berthing
<emil.renner.berthing@canonical.com> wrote:
> Add the active-low property to LEDs that are part of a multicolor LED
> just like the regular PWM LEDs have.
>
> Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v1 2/4] dt-bindings: leds: pwm-multicolor: Add active-low property

[Rob Herring]

On Tue, 05 Jul 2022 23:01:41 +0200, Emil Renner Berthing wrote:
> Add the active-low property to LEDs that are part of a multicolor LED
> just like the regular PWM LEDs have.
> 
> Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> ---
>  .../devicetree/bindings/leds/leds-pwm-multicolor.yaml         | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v1 3/4] leds: pwm-multicolor: Support active-low LEDs

[Emil Renner Berthing]

Add support for LEDs wired up to light when the PWM output is low, just
like the regular PWM LEDs.

Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
---
 drivers/leds/rgb/leds-pwm-multicolor.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/leds/rgb/leds-pwm-multicolor.c b/drivers/leds/rgb/leds-pwm-multicolor.c
index eb67b89d28e9..da9d2218ae18 100644
--- a/drivers/leds/rgb/leds-pwm-multicolor.c
+++ b/drivers/leds/rgb/leds-pwm-multicolor.c
@@ -19,6 +19,7 @@
 struct pwm_led {
 	struct pwm_device *pwm;
 	struct pwm_state state;
+	bool active_low;
 };
 
 struct pwm_mc_led {
@@ -45,6 +46,9 @@ static int led_pwm_mc_set(struct led_classdev *cdev,
 		duty *= mc_cdev->subled_info[i].brightness;
 		do_div(duty, cdev->max_brightness);
 
+		if (priv->leds[i].active_low)
+			duty = priv->leds[i].state.period - duty;
+
 		priv->leds[i].state.duty_cycle = duty;
 		priv->leds[i].state.enabled = duty > 0;
 		ret = pwm_apply_state(priv->leds[i].pwm,
@@ -76,6 +80,7 @@ static int iterate_subleds(struct device *dev, struct pwm_mc_led *priv,
 			goto release_fwnode;
 		}
 		pwm_init_state(pwmled->pwm, &pwmled->state);
+		pwmled->active_low = fwnode_property_read_bool(fwnode, "active-low");
 
 		ret = fwnode_property_read_u32(fwnode, "color", &color);
 		if (ret) {
-- 
2.37.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v1 3/4] leds: pwm-multicolor: Support active-low LEDs

[Geert Uytterhoeven]

On Tue, Jul 5, 2022 at 11:01 PM Emil Renner Berthing
<emil.renner.berthing@canonical.com> wrote:
> Add support for LEDs wired up to light when the PWM output is low, just
> like the regular PWM LEDs.
>
> Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Emil Renner Berthing]

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 {
-- 
2.37.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Geert Uytterhoeven]

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Ron Economos]

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>


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Pavel Machek]

[-- Attachment #1.1: Type: text/plain, Size: 555 bytes --]

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

-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Emil Renner Berthing]

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v1 0/4] Add HiFive Unmatched LEDs

[Pavel Machek]

[-- Attachment #1.1: Type: text/plain, Size: 455 bytes --]

Hi!

> This series adds support for the two LEDs on the HiFive Unmatched
> RISC-V board.
> 
> Emil Renner Berthing (4):
>   leds: pwm-multicolor: Don't show -EPROBE_DEFER as errors
>   dt-bindings: leds: pwm-multicolor: Add active-low property
>   leds: pwm-multicolor: Support active-low LEDs

Thank you, applied. Not taking the dts change

Best regards,
							Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v1 0/4] Add HiFive Unmatched LEDs

[Palmer Dabbelt]

On Sun, 17 Jul 2022 04:02:49 PDT (-0700), pavel@ucw.cz wrote:
> Hi!
>
>> This series adds support for the two LEDs on the HiFive Unmatched
>> RISC-V board.
>> 
>> Emil Renner Berthing (4):
>>   leds: pwm-multicolor: Don't show -EPROBE_DEFER as errors
>>   dt-bindings: leds: pwm-multicolor: Add active-low property
>>   leds: pwm-multicolor: Support active-low LEDs
>
> Thank you, applied. Not taking the dts change

I took the DTS change (#4) on riscv/for-next.  Thanks!

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv