Re: [RFC PATCH 0/2] Multicolor PWM LED support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sven,

On 1/25/22 10:22 AM, sven@xxxxxxxxxxxxxxxx wrote:
From: Sven Schwermer <sven.schwermer@xxxxxxxxxxxxxxxxxxxxxxxxxxx>

Hi,

As previously discussed [1] on the linux-leds list I am missing
multicolor PWM LED support. In the mean time I have put together a
working prototype for such a driver. This is my first Linux driver
so I'm hoping for some feedback. Here are some questions that came up
while putting this thing together:

   1. Currently, the max-brightness property is expected as a property to
      the multi-led node. That seems consistent with the existing
      multicolor class code, but I'm wondering whether it would make
      sense to have a max-brigthness for the individual LEDs as well?

For the proper mixed color calculation all sub-leds should have
the same max_brightness as the global max_brightness.

Look at how sub-led intensities are calculated in
led_mc_calc_color_components().

See also [0] and [1].

   2. The current multi-led node definition calls for a node index which
      would in turn require the reg property to be set within the node.
      In this context, that doesn't seem to make sense. Should this
      requirement be lifted from leds-class-multicolor.yaml?

reg is required for all DT nodes with address unit in the name.
If you skipped the address unit, then reg would be also not required.

   3. I'm not currently reusing any leds-pwm code because there aren't
      too many overlaps. Does anyone have suggestions what could be
      factored out into a common source file?

I think that having a separate pwm driver for multicolor LEDs is a good
idea. leds-pwm.c is old and well tested driver, there's no need to
tinker at it for no vital reason. And there is not much code to share
as you've noticed.

I would appreciate if anyone would test this code. It runs on my
i.MX6ULL-based hardware.

Best regards,
Sven

[1]: https://www.spinics.net/lists/linux-leds/msg19988.html

Sven Schwermer (2):
   dt-bindings: leds: Add multicolor PWM LED bindings
   leds: Add PWM multicolor driver

  .../bindings/leds/leds-pwm-multicolor.yaml    |  73 +++++++
  drivers/leds/Kconfig                          |   8 +
  drivers/leds/Makefile                         |   1 +
  drivers/leds/leds-pwm-multicolor.c            | 184 ++++++++++++++++++
  4 files changed, 266 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
  create mode 100644 drivers/leds/leds-pwm-multicolor.c


[0] Documentation/ABI/testing/sysfs-class-led-multicolor
[1] Documentation/leds/leds-class-multicolor.rst

--
Best regards,
Jacek Anaszewski



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux