On Tue, 10 Mar 2020 22:48:09 +0100
Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx> 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
