2019-09-22 19:19:06

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH v8 3/9] dt: bindings: Add multicolor class dt bindings documention

Dan,

On 9/20/19 7:41 PM, Dan Murphy wrote:
> Add DT bindings for the LEDs multicolor class framework.
>
> Signed-off-by: Dan Murphy <[email protected]>
> ---
> .../bindings/leds/leds-class-multicolor.txt | 95 +++++++++++++++++++
> 1 file changed, 95 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.txt
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt
> new file mode 100644
> index 000000000000..215d3c90f351
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt
> @@ -0,0 +1,95 @@
> +* Multicolor LED properties
> +
> +Monochrome LEDs can grouped into LED clusters. These clusters can provide a
> +visible output that have a wide range of colors and intensities.

Here is the same issue as I spotted in a review for the patch 2/9
from this set:

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

How about:

Bindings for multi color LEDs show how to describe current outputs of
either integrated multi-color LED elements (like RGB, RGBW, RGBWA-UV
etc.) or standalone LEDs, to achieve logically grouped multi-color LED
modules. This is achieved by adding multi-led nodes layer to the
monochrome LED bindings.

> +
> +The nodes and properties defined in this document are unique to the multicolor
> +LED class. Common LED nodes and properties are inherited from the common.txt
> +within this documentation directory.
> +
> +Required LED Child properties:
> + - color : For multicolor LED support this property should be defined as
> + LED_COLOR_ID_MULTI and further definition can be found in
> + include/linux/leds/common.h.
> +
> +led-controller@30 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "ti,lp5024";
> + reg = <0x29>;
> +
> + multi-led@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> + color = <LED_COLOR_ID_MULTI>;
> + function = LED_FUNCTION_STATUS;
> +
> +
> + led@3 {
> + reg = <3>;
> + color = <LED_COLOR_ID_RED>;
> + };
> +
> + led@4 {
> + reg = <4>;
> + color = <LED_COLOR_ID_GREEN>;
> + };
> +
> + led@5 {
> + reg = <5>;
> + color = <LED_COLOR_ID_BLUE>;
> + };
> + };
> +
> + multi-led@2 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + color = <LED_COLOR_ID_MULTI>;
> + function = LED_FUNCTION_ACTIVITY;
> + reg = <2>;
> + ti,led-bank = <2 3 5>;
> +
> + led@6 {
> + reg = <0x6>;
> + color = <LED_COLOR_ID_RED>;
> + led-sources = <6 9 15>;
> + };
> +
> + led@7 {
> + reg = <0x7>;
> + color = <LED_COLOR_ID_GREEN>;
> + led-sources = <7 10 16>;
> + };
> +
> + led@8 {
> + reg = <0x8>;
> + color = <LED_COLOR_ID_BLUE>;
> + led-sources = <8 11 17>;
> + };
> + };
> +
> + multi-led@4 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <4>;
> + color = <LED_COLOR_ID_MULTI>;
> + function = LED_FUNCTION_ACTIVITY;
> +
> + led@12 {
> + reg = <12>;
> + color = <LED_COLOR_ID_RED>;
> + };
> +
> + led@13 {
> + reg = <13>;
> + color = <LED_COLOR_ID_GREEN>;
> + };
> +
> + led@14 {
> + reg = <14>;
> + color = <LED_COLOR_ID_BLUE>;
> + };
> + };
> +
> +};
>

--
Best regards,
Jacek Anaszewski


2019-09-25 14:52:41

by Dan Murphy

[permalink] [raw]
Subject: Re: [PATCH v8 3/9] dt: bindings: Add multicolor class dt bindings documention

Jacek

On 9/21/19 7:57 AM, Jacek Anaszewski wrote:
> Dan,
>
> On 9/20/19 7:41 PM, Dan Murphy wrote:
>> Add DT bindings for the LEDs multicolor class framework.
>>
>> Signed-off-by: Dan Murphy <[email protected]>
>> ---
>> .../bindings/leds/leds-class-multicolor.txt | 95 +++++++++++++++++++
>> 1 file changed, 95 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.txt
>>
>> diff --git a/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt
>> new file mode 100644
>> index 000000000000..215d3c90f351
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt
>> @@ -0,0 +1,95 @@
>> +* Multicolor LED properties
>> +
>> +Monochrome LEDs can grouped into LED clusters. These clusters can provide a
>> +visible output that have a wide range of colors and intensities.
> Here is the same issue as I spotted in a review for the patch 2/9
> from this set:
>
> "
> 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.
> "
>
> How about:
>
> Bindings for multi color LEDs show how to describe current outputs of
> either integrated multi-color LED elements (like RGB, RGBW, RGBWA-UV
> etc.) or standalone LEDs, to achieve logically grouped multi-color LED
> modules. This is achieved by adding multi-led nodes layer to the
> monochrome LED bindings.

Ack

Dan

>> +
>> +The nodes and properties defined in this document are unique to the multicolor
>> +LED class. Common LED nodes and properties are inherited from the common.txt
>> +within this documentation directory.
>> +
>> +Required LED Child properties:
>> + - color : For multicolor LED support this property should be defined as
>> + LED_COLOR_ID_MULTI and further definition can be found in
>> + include/linux/leds/common.h.
>> +
>> +led-controller@30 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "ti,lp5024";
>> + reg = <0x29>;
>> +
>> + multi-led@1 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <1>;
>> + color = <LED_COLOR_ID_MULTI>;
>> + function = LED_FUNCTION_STATUS;
>> +
>> +
>> + led@3 {
>> + reg = <3>;
>> + color = <LED_COLOR_ID_RED>;
>> + };
>> +
>> + led@4 {
>> + reg = <4>;
>> + color = <LED_COLOR_ID_GREEN>;
>> + };
>> +
>> + led@5 {
>> + reg = <5>;
>> + color = <LED_COLOR_ID_BLUE>;
>> + };
>> + };
>> +
>> + multi-led@2 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + color = <LED_COLOR_ID_MULTI>;
>> + function = LED_FUNCTION_ACTIVITY;
>> + reg = <2>;
>> + ti,led-bank = <2 3 5>;
>> +
>> + led@6 {
>> + reg = <0x6>;
>> + color = <LED_COLOR_ID_RED>;
>> + led-sources = <6 9 15>;
>> + };
>> +
>> + led@7 {
>> + reg = <0x7>;
>> + color = <LED_COLOR_ID_GREEN>;
>> + led-sources = <7 10 16>;
>> + };
>> +
>> + led@8 {
>> + reg = <0x8>;
>> + color = <LED_COLOR_ID_BLUE>;
>> + led-sources = <8 11 17>;
>> + };
>> + };
>> +
>> + multi-led@4 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <4>;
>> + color = <LED_COLOR_ID_MULTI>;
>> + function = LED_FUNCTION_ACTIVITY;
>> +
>> + led@12 {
>> + reg = <12>;
>> + color = <LED_COLOR_ID_RED>;
>> + };
>> +
>> + led@13 {
>> + reg = <13>;
>> + color = <LED_COLOR_ID_GREEN>;
>> + };
>> +
>> + led@14 {
>> + reg = <14>;
>> + color = <LED_COLOR_ID_BLUE>;
>> + };
>> + };
>> +
>> +};
>>