2022-07-05 21:30:25

by Emil Renner Berthing

[permalink] [raw]
Subject: [PATCH v1 0/4] Add HiFive Unmatched LEDs

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


2022-07-05 21:53:41

by Emil Renner Berthing

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

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 <[email protected]>
---
.../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

2022-07-05 21:53:49

by Emil Renner Berthing

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

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 <[email protected]>
---
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

2022-07-06 07:53:48

by Geert Uytterhoeven

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

On Tue, Jul 5, 2022 at 11:01 PM Emil Renner Berthing
<[email protected]> 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 <[email protected]>

Reviewed-by: Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2022-07-06 08:21:52

by Geert Uytterhoeven

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

Hi Emil,

On Tue, Jul 5, 2022 at 11:01 PM Emil Renner Berthing
<[email protected]> 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 <[email protected]>

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 <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2022-07-08 09:35:28

by Ron Economos

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

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 <[email protected]>
> ---
> .../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 <[email protected]>

2022-07-17 11:08:12

by Pavel Machek

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

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
<[email protected]>

Acked-by: Pavel Machek <[email protected]>

(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.


Attachments:
(No filename) (555.00 B)
signature.asc (201.00 B)
Download all attachments

2022-07-17 11:40:30

by Pavel Machek

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

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.


Attachments:
(No filename) (455.00 B)
signature.asc (201.00 B)
Download all attachments

2022-07-17 14:21:41

by Emil Renner Berthing

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

On Sun, 17 Jul 2022 at 12:59, Pavel Machek <[email protected]> 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
> <[email protected]>
>
> Acked-by: Pavel Machek <[email protected]>
>
> (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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2022-08-11 21:34:52

by Palmer Dabbelt

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

On Sun, 17 Jul 2022 04:02:49 PDT (-0700), [email protected] 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!