Message-ID: <56d9a7a6-7cdb-8ac0-5e41-f45fad914c55@ti.com>
Date:   Mon, 23 Sep 2019 09:50:14 -0500
From:   Dan Murphy <dmurphy@...com>
To:     Jacek Anaszewski <jacek.anaszewski@...il.com>, <pavel@....cz>
CC:     <linux-leds@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v8 2/9] documention: leds: Add multicolor class
 documentation

Jacek

Thanks for the review

On 9/21/19 7:28 AM, Jacek Anaszewski wrote:
> Dan,
>
> On 9/20/19 7:41 PM, Dan Murphy wrote:
>> Add the support documentation on the multicolor LED framework.
>> This document defines the directores and file generated by the
> Now there will be one directory created.
>
> Apart from that - all documentation should go in the same patch
> as the feature being added. So patches 1,2 and 3 should be melded
> together.

I think only patches 1 & 2 should be squashed into a single patch.

Patch 3 are the dt-bindings which should be separated


>
>> multicolor framework.  It also documents usage.
>>
>> Signed-off-by: Dan Murphy <dmurphy@...com>
>> ---
>>   Documentation/leds/index.rst                 |  1 +
>>   Documentation/leds/leds-class-multicolor.rst | 91 ++++++++++++++++++++
>>   2 files changed, 92 insertions(+)
>>   create mode 100644 Documentation/leds/leds-class-multicolor.rst
>>
>> diff --git a/Documentation/leds/index.rst b/Documentation/leds/index.rst
>> index 060f4e485897..bc70c6aa7138 100644
>> --- a/Documentation/leds/index.rst
>> +++ b/Documentation/leds/index.rst
>> @@ -9,6 +9,7 @@ LEDs
>>   
>>      leds-class
>>      leds-class-flash
>> +   leds-class-multicolor
>>      ledtrig-oneshot
>>      ledtrig-transient
>>      ledtrig-usbport
>> diff --git a/Documentation/leds/leds-class-multicolor.rst b/Documentation/leds/leds-class-multicolor.rst
>> new file mode 100644
>> index 000000000000..063c9a411a1d
>> --- /dev/null
>> +++ b/Documentation/leds/leds-class-multicolor.rst
>> @@ -0,0 +1,91 @@
>> +====================================
>> +Multi Color LED handling under Linux
>> +====================================
>> +
>> +Description
>> +===========
>> +There are varying monochrome LED colors available for application.  These
>> +LEDs can be used as a single use case LED or can be mixed with other color
>> +LEDs to produce the full spectrum of color.
> I'd say it won't be the most frequent use case. We can expect rather
> compound RGB, RGBA[UV] etc. LED elements being connected to iouts of
> multi color LED controllers like LP50xx. TI mentions RGB LEDs in its
> application notes for instance. I'd mention that in the first place
> and leave what you have above as another use case.

Which application notes are you referring to?


>> Color LEDs that are grouped
>> +can be presented under a single LED node with individual color control.
> Let's skip "with individual color control". This is rather a means for
> keeping backward compatibility. Main goal of the MC class is multi color
> control. We can elaborate on how individual control can be achieved,
> namely one needs to set brightness to max and then can use
> the whole 0-<color>_max_intensity intensity scale for given iout.
> But his can be implied from the information provided below.

Ack


>> +The multicolor class groups these LEDs and allows dynamically setting the value
> What does "dynamically" stand for here? I assume you thought of altering
> colors without changing global brightness, but now it is not the case.

I updated this from what you said below


>> +of a single LED or setting the intensity values of the LEDs in the group and
>> +updating the LEDs virtually simultaneously.
> I propose below instead of the above three lines:
>
> The multi color class groups these LEDs and allows controlling two
> aspects of the final combined color: hue and lightness. The former is
> controlled via <color>_intensity files and the latter is controlled
> via brightness file.
>
> For more details on hue and lightness notions please refer to
> https://en.wikipedia.org/wiki/CIECAM02.
>
> Note that intensity files only cache the written value and the actual
> change of hardware state occurs upon writing brightness file. This
> allows for changing many factors of the perceived color in a virtually
> unnoticeable way for the human observer.

Ack


>> +Multicolor Class Control
>> +========================
>> +The multicolor class presents the LED groups under a directory called "colors".
>> +This directory is a child under the LED parent node created by the led_class
>> +framework.  The led_class framework is documented in led-class.rst within this
>> +documentation directory.
>> +
>> +Each colored LED will have two files created under the colors directory
>> +<led_color>_intensity and <led_color>_max_intensity. These files will contain
> s/led_color/color/
Ack
>
>> +one of LED_COLOR_ID_* definitions from the header
>> +include/dt-bindings/leds/common.h.
>> +
>> +Directory Layout Example
>> +========================
>> +root:/sys/class/leds/rgb:grouped_leds# ls -lR colors/
>> +-rw-rwxr-- 1 root root 4096 Jul 7 03:10 red_max_intensity
>> +--w--wx-w- 1 root root 4096 Jul 7 03:10 red_intensity
>> +-rw-rwxr-- 1 root root 4096 Jul 7 03:10 green_max_intensity
>> +--w--wx-w- 1 root root 4096 Jul 7 03:10 green_intensity
>> +-rw-rwxr-- 1 root root 4096 Jul 7 03:10 blue_max_intensity
>> +--w--wx-w- 1 root root 4096 Jul 7 03:10 blue_intensity
>> +
>> +Multicolor Class Brightness Control
>> +===================================
>> +The multiclor class framework will calculate each monochrome LEDs intensity.
>> +
>> +The brightness level for each LED is calculated based on the color LED
>> +intensity setting divided by the color LED max intensity setting multiplied by
>> +the requested value.
> s/value/brightness/

Ack


>> +
>> +led_brightness = requested_value * led_color_intensity/led_color_max_intensity
> led_brightness = brightness * <color>_intensity/<color>_max_intensity

Ack


>> +
>> +Example:
>> +Three LEDs are present in the group as defined in "Directory Layout Example"
>> +within this document.
>> +
>> +A user first writes the color LED brightness file with the brightness level that
>> +is necessary to achieve a blueish violet output from the RGB LED group.
>> +
>> +echo 138 > /sys/class/leds/rgb:grouped_leds/red_intensity
>> +echo 43 > /sys/class/leds/rgb:grouped_leds/green_intensity
>> +echo 226 > /sys/class/leds/rgb:grouped_leds/blue_intensity
>> +
>> +red -
>> +	intensity = 138
>> +	max_intensity = 255
>> +green -
>> +	intensity = 43
>> +	max_intensity = 255
>> +blue -
>> +	intensity = 226
>> +	max_intensity = 255
>> +
>> +The user can control the brightness of that RGB group by writing the parent
>> +'brightness' control.  Assuming a parent max_brightness of 255 the user may want
>> +to dim the LED color group to half.  The user would write a value of 128 to the
>> +parent brightness file then the values written to each LED will be adjusted
>> +base on this value
>> +
>> +cat /sys/class/leds/rgb:grouped_leds/max_brightness
>> +255
>> +echo 128 > /sys/class/leds/rgb:grouped_leds/brightness
>> +
>> +adjusted_red_value = 128 * 138/255 = 69
>> +adjusted_green_value = 128 * 43/255 = 21
>> +adjusted_blue_value = 128 * 226/255 = 113
>> +
>> +Reading the parent brightness file will return the current brightness value of
>> +the color LED group.
>> +
>> +cat /sys/class/leds/rgb:grouped_leds/max_brightness
>> +255
>> +
>> +echo 128 > /sys/class/leds/rgb:grouped_leds/brightness
>> +
>> +cat /sys/class/leds/rgb:grouped_leds/max_brightness
> s/max_brightness/brightness/
>
Ack

Dan

>> +128
>>

Powered by blists - more mailing lists