Messages in this thread |  | | From | Geert Uytterhoeven <> | Date | Wed, 6 Jul 2022 09:56:16 +0200 | 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 <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
|  |