On Wed, 11 Mar 2020 11:59:07 +0100
Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx> wrote:
> On 3/10/20 11:23 PM, Marek Behun wrote:
> > 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.
>
> Right.
>
> > 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.
>
> Device Tree will need to be changed to LED mc specific bindings,
> which at current state introduces one more level or nesting
> and LED_COLOR_ID_MULTI for the top level DT node.
>
> And the driver will need to still support this approach as well
> as the new LED mc class.
>
Hi Jacek,
I have used the led-sources in such a way that the user can either set
led-sources = <0 1 2>;
color = <LED_COLOR_ID_WHITE>;
in which case all three channels will be grouped into one led cdev,
or the user can use just one led-source, for example
led-sources = <0>;
color = <LED_COLOR_ID_RED>;
and in this case they can have one led cdev per channel.
Is this acceptable? Or should I just go with the WHITE approach?
In case that this is acceptable I wonder what should be the suggested
device-tree node naming and reg property, when using one led cdev per
channel, for example:
led@1,0 {
reg = <1>;
led-sources = <3>;
color = <LED_COLOR_ID_RED>;
};
led@1,1 {
reg = <1>;
led-sources = <4>;
color = <LED_COLOR_ID_GREEN>;
};
led@1,2 {
reg = <1>;
led-sources = <5>;
color = <LED_COLOR_ID_BLUE>;
};
I don't think different nodes should have the same reg property. Should
in this case the reg property have two values?
Marek
