* turris omnia leds again: question
@ 2020-03-10 17:38 Marek Behun
2020-03-10 21:48 ` Jacek Anaszewski
0 siblings, 1 reply; 6+ messages in thread
From: Marek Behun @ 2020-03-10 17:38 UTC (permalink / raw)
To: linux-leds; +Cc: Jacek Anaszewski, Pavel Machek
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?
Thanks, Marek.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: turris omnia leds again: question
2020-03-10 17:38 turris omnia leds again: question Marek Behun
@ 2020-03-10 21:48 ` Jacek Anaszewski
2020-03-10 22:23 ` Marek Behun
0 siblings, 1 reply; 6+ messages in thread
From: Jacek Anaszewski @ 2020-03-10 21:48 UTC (permalink / raw)
To: Marek Behun, linux-leds; +Cc: Pavel Machek
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?
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: turris omnia leds again: question
2020-03-10 21:48 ` Jacek Anaszewski
@ 2020-03-10 22:23 ` Marek Behun
2020-03-11 10:59 ` Jacek Anaszewski
0 siblings, 1 reply; 6+ messages in thread
From: Marek Behun @ 2020-03-10 22:23 UTC (permalink / raw)
To: Jacek Anaszewski; +Cc: linux-leds, Pavel Machek
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: turris omnia leds again: question
2020-03-10 22:23 ` Marek Behun
@ 2020-03-11 10:59 ` Jacek Anaszewski
2020-03-19 4:34 ` Marek Behun
2020-03-19 15:27 ` Marek Behun
0 siblings, 2 replies; 6+ messages in thread
From: Jacek Anaszewski @ 2020-03-11 10:59 UTC (permalink / raw)
To: Marek Behun; +Cc: linux-leds, Pavel Machek
On 3/10/20 11:23 PM, Marek Behun wrote:
> 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.
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.
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: turris omnia leds again: question
2020-03-11 10:59 ` Jacek Anaszewski
@ 2020-03-19 4:34 ` Marek Behun
2020-03-19 15:27 ` Marek Behun
1 sibling, 0 replies; 6+ messages in thread
From: Marek Behun @ 2020-03-19 4:34 UTC (permalink / raw)
To: Jacek Anaszewski; +Cc: linux-leds, Pavel Machek
On Wed, 11 Mar 2020 11:59:07 +0100
Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
> On 3/10/20 11:23 PM, Marek Behun wrote:
> > 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.
>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: turris omnia leds again: question
2020-03-11 10:59 ` Jacek Anaszewski
2020-03-19 4:34 ` Marek Behun
@ 2020-03-19 15:27 ` Marek Behun
1 sibling, 0 replies; 6+ messages in thread
From: Marek Behun @ 2020-03-19 15:27 UTC (permalink / raw)
To: Jacek Anaszewski; +Cc: linux-leds, Pavel Machek
On Wed, 11 Mar 2020 11:59:07 +0100
Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-19 15:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-10 17:38 turris omnia leds again: question Marek Behun
2020-03-10 21:48 ` Jacek Anaszewski
2020-03-10 22:23 ` Marek Behun
2020-03-11 10:59 ` Jacek Anaszewski
2020-03-19 4:34 ` Marek Behun
2020-03-19 15:27 ` Marek Behun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).