Re: turris omnia leds again: question

[Marek Behun <kabel@blackhole.sk>]

On Tue, 10 Mar 2020 22:48:09 +0100
Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:

> Hi Marek,
> 
> On 3/10/20 6:38 PM, Marek Behun wrote:
> > Hi,
> > 
> > I am going to try to send driver for Omnia LEDs again. The last time
> > there was a problem: on 05/01/2019 Jacek wrote:
> >   
> >> I wonder if we're doing right merging this driver in this form.
> >> We break the rule one-led-class-device-per-one-channel. We don't
> >> have LED multi color support yet, so this should support RGB LEDs
> >> in the old manner. Or switch to using LED multi color class.  
> >   
> >> Once we will have LED multi color class, we will be able to add the
> >> support for it to the driver and make the driver configurable to be
> >> able to expose old interface or the LED multi color one.  
> >   
> >> Moreover, the bindings should use led-sources property for grouping
> >> three channels under single LED class device. This is certainly to be
> >> fixed.  
> > 
> > So I am going to try to modify the driver so that each channel creates
> > one LED class device. Do I understand this correctly then, that this
> > way when there are three channels (RGB) on one LED, all the 3 device
> > tree nodes for should have the same reg property, but different
> > led-sources property? Eg:
> > 
> >   led@0,0 {
> >     reg = <0>;
> >     led-sources = <0>;
> >     label = "omnia::heartbeat::red";
> >   };
> > 
> >   led@0,1 {
> >     reg = <0>;
> >     led-sources = <1>;
> >     label = "omnia::heartbeat::green";
> >   };
> > 
> >   led@0,2 {
> >     reg = <0>;
> >     led-sources = <2>;
> >     label = "omnia::heartbeat::blue";
> >   };
> > 
> > Or did I misinterpret the led-sources property?  
> 
> This is what I proposed back then, strangely that message wasn't
> archived by bots, or maybe it resides only in my outbox...
> 
> --------------
> 
> LED sub-node properties:
>  - reg :                Must be from 0x0 to 0xb, since there are 12 RGB
> LEDs on this
>                         controller.
>  - label :              (optional)
>    see Documentation/devicetree/bindings/leds/common.txt
>  - linux,default-trigger : (optional)
>    see Documentation/devicetree/bindings/leds/common.txt
>  - led-sources : Each child node should describe RGB LED it controls,
>                  by listing corresponding iout identifiers:
>         0 - RGB LED 0: red
>         1 - RGB LED 0: green
>         2 - RGB LED 0: blue
>         3 - RGB LED 1: red
>         4 - RGB LED 1: green
>         5 - RGB LED 1: blue
>         6 - RGB LED 2: red
>         7 - RGB LED 2: green
>         8 - RGB LED 2: blue
>         9 - RGB LED 3: red
>         10 - RGB LED 3: green
>         11 - RGB LED 3: blue
>     ... and list all the iouts, maybe other names will be more
>             appropriate for this device, feel free to propose something
> 
> 
> 
> Example:
> 
>         led-controller@2b {
>                 compatible = "cznic,turris-omnia-leds";
>                 reg = <0x2b>;
>                 #address-cells = <1>;
>                 #size-cells = <0>;
> 
>                 led@0 {
>                         reg = <0x0>;
>                         label = "userB";
>                         linux,default-trigger = "heartbeat";
>                         led-sources = <0 1 2>;
>                 };
> 
>                 led@1 {
>                         reg = <0x1>;
>                         label = "userA";
>                         led-sources = <3 4 5>;
>                 };
> 
>                 led@2 {
>                         reg = <0x2>;
>                         label = "pci3";
>                         led-sources = <6 7 8>;
>                 };
> 
>                 led@3 {
>                         reg = <0x3>;
>                         label = "pci2";
>                         led-sources = <9 10 11>;
>                 };
>                 ...
> --------------
> 
> 
> Of course now label should be replaced with color and function
> properties. I've just reviewed that patch set and realized that
> we agreed upon setting max_brightness to 1 for all LEDs, right?
> 

No, there were subsequent patches which added support for
max_brightness = 255.

So the driver will register one LED class device per node, with color
ID = WHITE. Once RGB LED class is merged, the driver can be remade, but
the device tree won't need to be changed.

Marek