Re: [PATCH v26 03/15] leds: multicolor: Introduce a multicolor class definition

From: Pavel Machek
Date: Sat Jun 06 2020 - 11:53:29 EST


Hi!

> Introduce a multicolor class that groups colored LEDs
> within a LED node.
>
> The multi color class groups monochrome LEDs and allows controlling two
> aspects of the final combined color: hue and lightness. The former is
> controlled via the intensity file and the latter is controlled
> via brightness file.
>
> Acked-by: Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx>
> Signed-off-by: Dan Murphy <dmurphy@xxxxxx>

> diff --git a/Documentation/ABI/testing/sysfs-class-led-multicolor b/Documentation/ABI/testing/sysfs-class-led-multicolor
> new file mode 100644
> index 000000000000..7d33a82a4b07
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-led-multicolor
> @@ -0,0 +1,34 @@
> +What: /sys/class/leds/<led>/brightness
> +Date: March 2020
> +KernelVersion: 5.8
> +Contact: Dan Murphy <dmurphy@xxxxxx>
> +Description: read/write
> + Writing to this file will update all LEDs within the group to a
> + calculated percentage of what each color LED intensity is set
> + to. The percentage is calculated for each grouped LED via the
> + equation below:

> + led_brightness = brightness * multi_intensity/max_brightness
> +
> + For additional details please refer to
> + Documentation/leds/leds-class-multicolor.rst.
> +
> + The value of the color is from 0 to
> + /sys/class/leds/<led>/max_brightness.

It is not too clear to me what "color" means here.

It would be worth mentioning that this is single integer.

> +What: /sys/class/leds/<led>/multi_index
> +Date: March 2020
> +KernelVersion: 5.8
> +Contact: Dan Murphy <dmurphy@xxxxxx>
> +Description: read
> + The multi_index array, when read, will output the LED colors
> + by name as they are indexed in the multi_intensity file.

This should specify that it is array of strings.

> +What: /sys/class/leds/<led>/multi_intensity
> +Date: March 2020
> +KernelVersion: 5.8
> +Contact: Dan Murphy <dmurphy@xxxxxx>
> +Description: read/write
> + Intensity level for the LED color within the array.
> + The intensities for each color must be entered based on the
> + multi_index array.

I'd mention here that it is array of integers, and what the maximum
values are.

> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9533,6 +9533,14 @@ F: Documentation/devicetree/bindings/leds/
> F: drivers/leds/
> F: include/linux/leds.h
>
> +LED MULTICOLOR FRAMEWORK
> +M: Dan Murphy <dmurphy@xxxxxx>
> +L: linux-leds@xxxxxxxxxxxxxxx

I'd like to be mentioned here, too. "M: Pavel Machek
<pavel@xxxxxx>". And I'm not sure if I should be taking MAINTAINER
file update through a LED tree. Should definitely go to separate
patch.

> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 9cdc4cfc5d11..fe7d90d4fa23 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -30,6 +30,16 @@ config LEDS_CLASS_FLASH
> for the flash related features of a LED device. It can be built
> as a module.
>
> +config LEDS_CLASS_MULTI_COLOR
> + tristate "LED MultiColor LED Class Support"

"LED MultiColor Class Support"

Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Attachment: signature.asc
Description: Digital signature