Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-12-23 Thread Jacek Anaszewski

On 12/12/18 7:32 PM, Pavel Machek wrote:

On Wed 2018-12-12 07:56:20, Rob Herring wrote:

On Wed, Dec 12, 2018 at 3:59 AM Pavel Machek  wrote:


Hi!


We would also probably need different DT properties for different
types of devices, since e.g. for network case the network interface
name would fit better for the LED name, than the phy name,
and we would need to know what type of device name we're going
to look for.

Pavel gave following examples:

eth0:green:link
adsl0:green:link
adsl0:red:error

So we would have e.g.:

associated-vl42-device = <>;
associated-network-device = <>;
associated-block-device = <>;


Variable property names are kind of a pain to parse.

Perhaps when LEDs are associated with a device, we shouldn't care
within the context of the LED subsystem what the name is. The
association is more important and if you have that exposed, then you
don't really need to care what the name is. You still have to deal
with a device with more than 1 LED, but that becomes a problem local
to that device.

What I'm getting at is following a more standard binding pattern of
providers and consumers like we have for gpios, clocks, etc. So we'd
have something like this:

ethernet {
   ...
   leds = <_led>, <_led>;
   led-names = "link", "err";
};

We can still support defining LED names as we've done, but we don't
have to come up with some elaborate naming convention that covers
every single case.


I see that it would be more consistent with how gpios work, but I'm
afraid this does not fit LEDs properly.

With power LED, you want to be able to say "this is just on". Some
poeple like heartbeat, and have LED for that. There may be LED for
"disk activity", meaning activity on any harddrive. And there may be
activity LED for specific disk.

Only in the last case it would be suitable to have LED reference as a
child of an device...


Right. For all the other cases, why do you need any link with a
device? What we have today is sufficient and can continue to be
supported. A link to a device is only used if the led is associated
with a particular device.


Right, so the link is only needed in some cases. So unlike your
example, we would have:

led1 { led-meaning = "heartbeat"; }
led2 { led-meaning = "link"; }
led3 { led-meaning = "err"; }
led4 { led-meaning = "activity"; what-activity = "all_harddisks"; }
ethernet { leds = < ,  >; }

Dunno. I'd expect all the LED description in the LED node, not part of
the description there and back link from the device


Driven by the feeling that it's all going in a wrong direction
I've done a bit more analysis - please refer below to my conclusions.

We already have a means for associating LEDs connected to standalone LED
controllers with other devices - this is LED trigger mechanism.
The association can be discovered by reading "trigger" sysfs file.

Device drivers can even create their own custom triggers, comprising
device node name or phy name in their names, which are known only during
driver probing, which allows to better describe the association than the
generic ones.

When it comes to devices with built-in LEDs - let's check how network
devices handle their LEDs. I've looked into ralink wireless driver:

drivers/net/wireless/ralink/rt2x00/rt2x00leds.c

It registers its LEDs in rt2x00leds_register(),
and composes names according to the following pattern:

   snprintf(name, sizeof(name), "%s-%s::assoc",
rt2x00dev->ops->name, phy_name);

   snprintf(name, sizeof(name), "%s-%s::quality",
rt2x00dev->ops->name, phy_name);

   snprintf(name, sizeof(name), "%s-%s::radio",
rt2x00dev->ops->name, phy_name);

which results in the following LED names for wifi dongle:

rt73usb-phy16::assoc -> 
../../devices/pci:00/:00:14.0/usb2/2-8/2-8:1.0/leds/rt73usb-phy16::assoc
rt73usb-phy16::quality -> 
../../devices/pci:00/:00:14.0/usb2/2-8/2-8:1.0/leds/rt73usb-phy16::quality
rt73usb-phy16::radio -> 
../../devices/pci:00/:00:14.0/usb2/2-8/2-8:1.0/leds/rt73usb-phy16::radio


As it is shown, each entry under /sys/class/CLASS_NAME is a symbolic
link to the class device sub-directory located in the parent device
directory. In this case, even if we hadn't phy name in the LED name,
we could retrieve it by traversing parent device directory in the
sysfs:

../../devices/pci:00/:00:14.0/usb2/2-8/2-8:1.0/ieee80211/phy16/

We have also available additional device and driver specific info
in the uevent file:

../../devices/pci:00/:00:14.0/usb2/2-8/2-8:1.0/uevent:DEVTYPE=usb_interface
../../devices/pci:00/:00:14.0/usb2/2-8/2-8:1.0/uevent:DRIVER=rt73usb
../../devices/pci:00/:00:14.0/usb2/2-8/2-8:1.0/uevent:PRODUCT=148f/2573/1
../../devices/pci:00/:00:14.0/usb2/2-8/2-8:1.0/uevent:TYPE=0/0/0
../../devices/pci:00/:00:14.0/usb2/2-8/2-8:1.0/uevent:INTERFACE=255/255/255
../../devices/pci:00/:00:14.0/usb2/2-8/2-8:1.0/uevent:MODALIAS=usb:v148Fp2573d0001dc00dsc00dp00icFFiscFFipFFin00

In addition to 

Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-12-21 Thread Linus Walleij
On Tue, Dec 11, 2018 at 6:27 PM Rob Herring  wrote:
> On Sat, Dec 1, 2018 at 3:17 PM Jacek Anaszewski
>  wrote:

> > This is not true in case of associations where LED controller is
> > an independent device, as in Pavel's example [0].
>
> I'm not really following how the HDD example is different. The parent
> of an LED would be the controller. The link to the led node would be
> in the disk controller node. Though maybe if things get complicated
> enough, we'd want to describe the drives or drive bays in DT.

Not talking about this in specific, but I will need to add DT bindings
for ATA master/slave, in order to add a temperature zone
for each drive (one sensor in each drive) in my in-progress
hwmon-over-SCSI-emulation-ATA-sensors such that:

ata-controller {
  ata-master {
  };
  ata-slave {
  };
};

I haven't looked closer at this topology yet, but we definatelt
need nodes for each physical connector out of an ATA
master, somehow, so they can act as a sensor/temperature
zone.

Yours,
Linus Walleij


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-12-12 Thread Pavel Machek
On Wed 2018-12-12 07:56:20, Rob Herring wrote:
> On Wed, Dec 12, 2018 at 3:59 AM Pavel Machek  wrote:
> >
> > Hi!
> >
> > > > We would also probably need different DT properties for different
> > > > types of devices, since e.g. for network case the network interface
> > > > name would fit better for the LED name, than the phy name,
> > > > and we would need to know what type of device name we're going
> > > > to look for.
> > > >
> > > > Pavel gave following examples:
> > > >
> > > > eth0:green:link
> > > > adsl0:green:link
> > > > adsl0:red:error
> > > >
> > > > So we would have e.g.:
> > > >
> > > > associated-vl42-device = <>;
> > > > associated-network-device = <>;
> > > > associated-block-device = <>;
> > >
> > > Variable property names are kind of a pain to parse.
> > >
> > > Perhaps when LEDs are associated with a device, we shouldn't care
> > > within the context of the LED subsystem what the name is. The
> > > association is more important and if you have that exposed, then you
> > > don't really need to care what the name is. You still have to deal
> > > with a device with more than 1 LED, but that becomes a problem local
> > > to that device.
> > >
> > > What I'm getting at is following a more standard binding pattern of
> > > providers and consumers like we have for gpios, clocks, etc. So we'd
> > > have something like this:
> > >
> > > ethernet {
> > >   ...
> > >   leds = <_led>, <_led>;
> > >   led-names = "link", "err";
> > > };
> > >
> > > We can still support defining LED names as we've done, but we don't
> > > have to come up with some elaborate naming convention that covers
> > > every single case.
> >
> > I see that it would be more consistent with how gpios work, but I'm
> > afraid this does not fit LEDs properly.
> >
> > With power LED, you want to be able to say "this is just on". Some
> > poeple like heartbeat, and have LED for that. There may be LED for
> > "disk activity", meaning activity on any harddrive. And there may be
> > activity LED for specific disk.
> >
> > Only in the last case it would be suitable to have LED reference as a
> > child of an device...
> 
> Right. For all the other cases, why do you need any link with a
> device? What we have today is sufficient and can continue to be
> supported. A link to a device is only used if the led is associated
> with a particular device.

Right, so the link is only needed in some cases. So unlike your
example, we would have:

led1 { led-meaning = "heartbeat"; }
led2 { led-meaning = "link"; }
led3 { led-meaning = "err"; }
led4 { led-meaning = "activity"; what-activity = "all_harddisks"; }
ethernet { leds = < ,  >; }

Dunno. I'd expect all the LED description in the LED node, not part of
the description there and back link from the device...
Pavel


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-12-12 Thread Rob Herring
On Wed, Dec 12, 2018 at 3:59 AM Pavel Machek  wrote:
>
> Hi!
>
> > > We would also probably need different DT properties for different
> > > types of devices, since e.g. for network case the network interface
> > > name would fit better for the LED name, than the phy name,
> > > and we would need to know what type of device name we're going
> > > to look for.
> > >
> > > Pavel gave following examples:
> > >
> > > eth0:green:link
> > > adsl0:green:link
> > > adsl0:red:error
> > >
> > > So we would have e.g.:
> > >
> > > associated-vl42-device = <>;
> > > associated-network-device = <>;
> > > associated-block-device = <>;
> >
> > Variable property names are kind of a pain to parse.
> >
> > Perhaps when LEDs are associated with a device, we shouldn't care
> > within the context of the LED subsystem what the name is. The
> > association is more important and if you have that exposed, then you
> > don't really need to care what the name is. You still have to deal
> > with a device with more than 1 LED, but that becomes a problem local
> > to that device.
> >
> > What I'm getting at is following a more standard binding pattern of
> > providers and consumers like we have for gpios, clocks, etc. So we'd
> > have something like this:
> >
> > ethernet {
> >   ...
> >   leds = <_led>, <_led>;
> >   led-names = "link", "err";
> > };
> >
> > We can still support defining LED names as we've done, but we don't
> > have to come up with some elaborate naming convention that covers
> > every single case.
>
> I see that it would be more consistent with how gpios work, but I'm
> afraid this does not fit LEDs properly.
>
> With power LED, you want to be able to say "this is just on". Some
> poeple like heartbeat, and have LED for that. There may be LED for
> "disk activity", meaning activity on any harddrive. And there may be
> activity LED for specific disk.
>
> Only in the last case it would be suitable to have LED reference as a
> child of an device...

Right. For all the other cases, why do you need any link with a
device? What we have today is sufficient and can continue to be
supported. A link to a device is only used if the led is associated
with a particular device.

Rob


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-12-12 Thread Pavel Machek
Hi!

> > We would also probably need different DT properties for different
> > types of devices, since e.g. for network case the network interface
> > name would fit better for the LED name, than the phy name,
> > and we would need to know what type of device name we're going
> > to look for.
> >
> > Pavel gave following examples:
> >
> > eth0:green:link
> > adsl0:green:link
> > adsl0:red:error
> >
> > So we would have e.g.:
> >
> > associated-vl42-device = <>;
> > associated-network-device = <>;
> > associated-block-device = <>;
> 
> Variable property names are kind of a pain to parse.
> 
> Perhaps when LEDs are associated with a device, we shouldn't care
> within the context of the LED subsystem what the name is. The
> association is more important and if you have that exposed, then you
> don't really need to care what the name is. You still have to deal
> with a device with more than 1 LED, but that becomes a problem local
> to that device.
> 
> What I'm getting at is following a more standard binding pattern of
> providers and consumers like we have for gpios, clocks, etc. So we'd
> have something like this:
> 
> ethernet {
>   ...
>   leds = <_led>, <_led>;
>   led-names = "link", "err";
> };
> 
> We can still support defining LED names as we've done, but we don't
> have to come up with some elaborate naming convention that covers
> every single case.

I see that it would be more consistent with how gpios work, but I'm
afraid this does not fit LEDs properly.

With power LED, you want to be able to say "this is just on". Some
poeple like heartbeat, and have LED for that. There may be LED for
"disk activity", meaning activity on any harddrive. And there may be
activity LED for specific disk.

Only in the last case it would be suitable to have LED reference as a
child of an device...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-12-11 Thread Pavel Machek
Hi!

> > >> Basically every single device could have a LED associated with it
> > >> ("activity"). Would doing it like this mean we'd have to modify every
> > >> single driver to parse leds / led-names properties?
> > >
> > > Normally, that's how properties like this would work. A driver is also
> > > what knows how the leds should function.
> >
> > This is not true in case of associations where LED controller is
> > an independent device, as in Pavel's example [0].
> 
> I'm not really following how the HDD example is different. The parent
> of an LED would be the controller. The link to the led node would be
> in the disk controller node. Though maybe if things get complicated
> enough, we'd want to describe the drives or drive bays in DT.

We were talking case where the LED is on GPIO somewhere. And yes,
drive bays would be similar.

And.. often you have a single LED for multiple harddrives, too. Fun
:-).
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-12-11 Thread Rob Herring
On Sat, Dec 1, 2018 at 3:17 PM Jacek Anaszewski
 wrote:
>
> On 11/30/2018 11:19 PM, Rob Herring wrote:
> > On Fri, Nov 30, 2018 at 3:08 PM Pavel Machek  wrote:
> >>
> >> Hi!
> >>
>  Pavel gave following examples:
> 
>  eth0:green:link
>  adsl0:green:link
>  adsl0:red:error
> 
>  So we would have e.g.:
> 
>  associated-vl42-device = <>;
>  associated-network-device = <>;
>  associated-block-device = <>;
> >>>
> >>> Variable property names are kind of a pain to parse.
> >>
> >> Ok, would it be enough to have associated-device = <>?
> >
> > Yeah, but I though you needed the device type name in there.
> >
> >>> Perhaps when LEDs are associated with a device, we shouldn't care
> >>> within the context of the LED subsystem what the name is. The
> >>> association is more important and if you have that exposed, then you
> >>> don't really need to care what the name is. You still have to deal
> >>> with a device with more than 1 LED, but that becomes a problem local
> >>> to that device.
> >>>
> >>> What I'm getting at is following a more standard binding pattern of
> >>> providers and consumers like we have for gpios, clocks, etc. So we'd
> >>> have something like this:
> >>>
> >>> ethernet {
> >>>   ...
> >>>   leds = <_led>, <_led>;
> >>>   led-names = "link", "err";
> >>> };
> >>
> >> Basically every single device could have a LED associated with it
> >> ("activity"). Would doing it like this mean we'd have to modify every
> >> single driver to parse leds / led-names properties?
> >
> > Normally, that's how properties like this would work. A driver is also
> > what knows how the leds should function.
>
> This is not true in case of associations where LED controller is
> an independent device, as in Pavel's example [0].

I'm not really following how the HDD example is different. The parent
of an LED would be the controller. The link to the led node would be
in the disk controller node. Though maybe if things get complicated
enough, we'd want to describe the drives or drive bays in DT.

> > A driver can retrieve the led
> > and associate it with the 'foo-bar' function. The 'foo-bar' function
> > then doesn't have to be defined in DT nor exposed to userspace. It
> > wouldn't even have to be driver specific. The driver's subsystem could
> > handle it all if the led functions are standardized. Though then you'd
> > be back to needing standard names for 'led-names', but that's no worse
> > that trigger names. This model would also allow getting rid of
> > 'linux,default-trigger' properties in a lot of cases which wouldn't be
> > a bad thing.
> >
> > However, having drivers handle this is not required. You can iterate
> > thru the tree for nodes with 'leds' and find the node which has a
> > phandle to the led node you care about.
>
> This way of discovering associations between devices and LEDs
> would be more reliable than by devicename part of LED class device
> as discussed previously.
>
> However, I've just tried to verify how it works, but
> I can't find the way to get the of_node phandle from sysfs.
>
> The "of_node" link in per-device dirs in /sys/devices point
> to the directories containing DT properties of the node, but
> I see no way to obtain the node phandle.

You probably don't want to be doing this in userspace. It's possible,
but you've got to read the 'leds' property to get the phandle and then
walk the tree to find the node with the matching phandle property
value (i.e. reimplement what the kernel does). You'd probably would
instead want to define a LED specific symlink sysfs attr either in the
associated device to the led device or vice versa.

Rob


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-12-01 Thread Jacek Anaszewski
On 11/30/2018 11:19 PM, Rob Herring wrote:
> On Fri, Nov 30, 2018 at 3:08 PM Pavel Machek  wrote:
>>
>> Hi!
>>
 Pavel gave following examples:

 eth0:green:link
 adsl0:green:link
 adsl0:red:error

 So we would have e.g.:

 associated-vl42-device = <>;
 associated-network-device = <>;
 associated-block-device = <>;
>>>
>>> Variable property names are kind of a pain to parse.
>>
>> Ok, would it be enough to have associated-device = <>?
> 
> Yeah, but I though you needed the device type name in there.
> 
>>> Perhaps when LEDs are associated with a device, we shouldn't care
>>> within the context of the LED subsystem what the name is. The
>>> association is more important and if you have that exposed, then you
>>> don't really need to care what the name is. You still have to deal
>>> with a device with more than 1 LED, but that becomes a problem local
>>> to that device.
>>>
>>> What I'm getting at is following a more standard binding pattern of
>>> providers and consumers like we have for gpios, clocks, etc. So we'd
>>> have something like this:
>>>
>>> ethernet {
>>>   ...
>>>   leds = <_led>, <_led>;
>>>   led-names = "link", "err";
>>> };
>>
>> Basically every single device could have a LED associated with it
>> ("activity"). Would doing it like this mean we'd have to modify every
>> single driver to parse leds / led-names properties?
> 
> Normally, that's how properties like this would work. A driver is also
> what knows how the leds should function. 

This is not true in case of associations where LED controller is
an independent device, as in Pavel's example [0].

> A driver can retrieve the led
> and associate it with the 'foo-bar' function. The 'foo-bar' function
> then doesn't have to be defined in DT nor exposed to userspace. It
> wouldn't even have to be driver specific. The driver's subsystem could
> handle it all if the led functions are standardized. Though then you'd
> be back to needing standard names for 'led-names', but that's no worse
> that trigger names. This model would also allow getting rid of
> 'linux,default-trigger' properties in a lot of cases which wouldn't be
> a bad thing.
> 
> However, having drivers handle this is not required. You can iterate
> thru the tree for nodes with 'leds' and find the node which has a
> phandle to the led node you care about. 

This way of discovering associations between devices and LEDs
would be more reliable than by devicename part of LED class device
as discussed previously.

However, I've just tried to verify how it works, but
I can't find the way to get the of_node phandle from sysfs.

The "of_node" link in per-device dirs in /sys/devices point
to the directories containing DT properties of the node, but
I see no way to obtain the node phandle.

> Sure, it's not that efficient,
> but it does work and it's only done once. Basically, as long as the
> linkage is there, we can make it work. I think using
> 'associated-device' might work better for the current implementation
> of Linux LED support, but leds/led-names would be more inline with
> other DT bindings. The current Linux implementation shouldn't dictate
> the binding design.

[0] https://lkml.org/lkml/2018/11/13/103

-- 
Best regards,
Jacek Anaszewski


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-12-01 Thread Jacek Anaszewski
On 11/30/2018 11:19 PM, Rob Herring wrote:
> On Fri, Nov 30, 2018 at 3:08 PM Pavel Machek  wrote:
>>
>> Hi!
>>
 Pavel gave following examples:

 eth0:green:link
 adsl0:green:link
 adsl0:red:error

 So we would have e.g.:

 associated-vl42-device = <>;
 associated-network-device = <>;
 associated-block-device = <>;
>>>
>>> Variable property names are kind of a pain to parse.
>>
>> Ok, would it be enough to have associated-device = <>?
> 
> Yeah, but I though you needed the device type name in there.
> 
>>> Perhaps when LEDs are associated with a device, we shouldn't care
>>> within the context of the LED subsystem what the name is. The
>>> association is more important and if you have that exposed, then you
>>> don't really need to care what the name is. You still have to deal
>>> with a device with more than 1 LED, but that becomes a problem local
>>> to that device.
>>>
>>> What I'm getting at is following a more standard binding pattern of
>>> providers and consumers like we have for gpios, clocks, etc. So we'd
>>> have something like this:
>>>
>>> ethernet {
>>>   ...
>>>   leds = <_led>, <_led>;
>>>   led-names = "link", "err";
>>> };
>>
>> Basically every single device could have a LED associated with it
>> ("activity"). Would doing it like this mean we'd have to modify every
>> single driver to parse leds / led-names properties?
> 
> Normally, that's how properties like this would work. A driver is also
> what knows how the leds should function. 

This is not true in case of associations where LED controller is
an independent device, as in Pavel's example [0].

> A driver can retrieve the led
> and associate it with the 'foo-bar' function. The 'foo-bar' function
> then doesn't have to be defined in DT nor exposed to userspace. It
> wouldn't even have to be driver specific. The driver's subsystem could
> handle it all if the led functions are standardized. Though then you'd
> be back to needing standard names for 'led-names', but that's no worse
> that trigger names. This model would also allow getting rid of
> 'linux,default-trigger' properties in a lot of cases which wouldn't be
> a bad thing.
> 
> However, having drivers handle this is not required. You can iterate
> thru the tree for nodes with 'leds' and find the node which has a
> phandle to the led node you care about. 

This way of discovering associations between devices and LEDs
would be more reliable than by devicename part of LED class device
as discussed previously.

However, I've just tried to verify how it works, but
I can't find the way to get the of_node phandle from sysfs.

The "of_node" link in per-device dirs in /sys/devices point
to the directories containing DT properties of the node, but
I see no way to obtain the node phandle.

> Sure, it's not that efficient,
> but it does work and it's only done once. Basically, as long as the
> linkage is there, we can make it work. I think using
> 'associated-device' might work better for the current implementation
> of Linux LED support, but leds/led-names would be more inline with
> other DT bindings. The current Linux implementation shouldn't dictate
> the binding design.

[0] https://lkml.org/lkml/2018/11/13/103

-- 
Best regards,
Jacek Anaszewski


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-30 Thread Rob Herring
On Fri, Nov 30, 2018 at 3:08 PM Pavel Machek  wrote:
>
> Hi!
>
> > > Pavel gave following examples:
> > >
> > > eth0:green:link
> > > adsl0:green:link
> > > adsl0:red:error
> > >
> > > So we would have e.g.:
> > >
> > > associated-vl42-device = <>;
> > > associated-network-device = <>;
> > > associated-block-device = <>;
> >
> > Variable property names are kind of a pain to parse.
>
> Ok, would it be enough to have associated-device = <>?

Yeah, but I though you needed the device type name in there.

> > Perhaps when LEDs are associated with a device, we shouldn't care
> > within the context of the LED subsystem what the name is. The
> > association is more important and if you have that exposed, then you
> > don't really need to care what the name is. You still have to deal
> > with a device with more than 1 LED, but that becomes a problem local
> > to that device.
> >
> > What I'm getting at is following a more standard binding pattern of
> > providers and consumers like we have for gpios, clocks, etc. So we'd
> > have something like this:
> >
> > ethernet {
> >   ...
> >   leds = <_led>, <_led>;
> >   led-names = "link", "err";
> > };
>
> Basically every single device could have a LED associated with it
> ("activity"). Would doing it like this mean we'd have to modify every
> single driver to parse leds / led-names properties?

Normally, that's how properties like this would work. A driver is also
what knows how the leds should function. A driver can retrieve the led
and associate it with the 'foo-bar' function. The 'foo-bar' function
then doesn't have to be defined in DT nor exposed to userspace. It
wouldn't even have to be driver specific. The driver's subsystem could
handle it all if the led functions are standardized. Though then you'd
be back to needing standard names for 'led-names', but that's no worse
that trigger names. This model would also allow getting rid of
'linux,default-trigger' properties in a lot of cases which wouldn't be
a bad thing.

However, having drivers handle this is not required. You can iterate
thru the tree for nodes with 'leds' and find the node which has a
phandle to the led node you care about. Sure, it's not that efficient,
but it does work and it's only done once. Basically, as long as the
linkage is there, we can make it work. I think using
'associated-device' might work better for the current implementation
of Linux LED support, but leds/led-names would be more inline with
other DT bindings. The current Linux implementation shouldn't dictate
the binding design.

Rob


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-30 Thread Rob Herring
On Fri, Nov 30, 2018 at 3:08 PM Pavel Machek  wrote:
>
> Hi!
>
> > > Pavel gave following examples:
> > >
> > > eth0:green:link
> > > adsl0:green:link
> > > adsl0:red:error
> > >
> > > So we would have e.g.:
> > >
> > > associated-vl42-device = <>;
> > > associated-network-device = <>;
> > > associated-block-device = <>;
> >
> > Variable property names are kind of a pain to parse.
>
> Ok, would it be enough to have associated-device = <>?

Yeah, but I though you needed the device type name in there.

> > Perhaps when LEDs are associated with a device, we shouldn't care
> > within the context of the LED subsystem what the name is. The
> > association is more important and if you have that exposed, then you
> > don't really need to care what the name is. You still have to deal
> > with a device with more than 1 LED, but that becomes a problem local
> > to that device.
> >
> > What I'm getting at is following a more standard binding pattern of
> > providers and consumers like we have for gpios, clocks, etc. So we'd
> > have something like this:
> >
> > ethernet {
> >   ...
> >   leds = <_led>, <_led>;
> >   led-names = "link", "err";
> > };
>
> Basically every single device could have a LED associated with it
> ("activity"). Would doing it like this mean we'd have to modify every
> single driver to parse leds / led-names properties?

Normally, that's how properties like this would work. A driver is also
what knows how the leds should function. A driver can retrieve the led
and associate it with the 'foo-bar' function. The 'foo-bar' function
then doesn't have to be defined in DT nor exposed to userspace. It
wouldn't even have to be driver specific. The driver's subsystem could
handle it all if the led functions are standardized. Though then you'd
be back to needing standard names for 'led-names', but that's no worse
that trigger names. This model would also allow getting rid of
'linux,default-trigger' properties in a lot of cases which wouldn't be
a bad thing.

However, having drivers handle this is not required. You can iterate
thru the tree for nodes with 'leds' and find the node which has a
phandle to the led node you care about. Sure, it's not that efficient,
but it does work and it's only done once. Basically, as long as the
linkage is there, we can make it work. I think using
'associated-device' might work better for the current implementation
of Linux LED support, but leds/led-names would be more inline with
other DT bindings. The current Linux implementation shouldn't dictate
the binding design.

Rob


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-30 Thread Pavel Machek
Hi!

> > Pavel gave following examples:
> >
> > eth0:green:link
> > adsl0:green:link
> > adsl0:red:error
> >
> > So we would have e.g.:
> >
> > associated-vl42-device = <>;
> > associated-network-device = <>;
> > associated-block-device = <>;
> 
> Variable property names are kind of a pain to parse.

Ok, would it be enough to have associated-device = <>?

> Perhaps when LEDs are associated with a device, we shouldn't care
> within the context of the LED subsystem what the name is. The
> association is more important and if you have that exposed, then you
> don't really need to care what the name is. You still have to deal
> with a device with more than 1 LED, but that becomes a problem local
> to that device.
> 
> What I'm getting at is following a more standard binding pattern of
> providers and consumers like we have for gpios, clocks, etc. So we'd
> have something like this:
> 
> ethernet {
>   ...
>   leds = <_led>, <_led>;
>   led-names = "link", "err";
> };

Basically every single device could have a LED associated with it
("activity"). Would doing it like this mean we'd have to modify every
single driver to parse leds / led-names properties?

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-30 Thread Pavel Machek
Hi!

> > Pavel gave following examples:
> >
> > eth0:green:link
> > adsl0:green:link
> > adsl0:red:error
> >
> > So we would have e.g.:
> >
> > associated-vl42-device = <>;
> > associated-network-device = <>;
> > associated-block-device = <>;
> 
> Variable property names are kind of a pain to parse.

Ok, would it be enough to have associated-device = <>?

> Perhaps when LEDs are associated with a device, we shouldn't care
> within the context of the LED subsystem what the name is. The
> association is more important and if you have that exposed, then you
> don't really need to care what the name is. You still have to deal
> with a device with more than 1 LED, but that becomes a problem local
> to that device.
> 
> What I'm getting at is following a more standard binding pattern of
> providers and consumers like we have for gpios, clocks, etc. So we'd
> have something like this:
> 
> ethernet {
>   ...
>   leds = <_led>, <_led>;
>   led-names = "link", "err";
> };

Basically every single device could have a LED associated with it
("activity"). Would doing it like this mean we'd have to modify every
single driver to parse leds / led-names properties?

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-30 Thread Rob Herring
On Tue, Nov 27, 2018 at 2:38 PM Jacek Anaszewski
 wrote:
>
> On 11/13/2018 09:57 PM, Jacek Anaszewski wrote:
> > On 11/12/2018 07:27 PM, Rob Herring wrote:
> >> On Tue, Nov 06, 2018 at 11:07:12PM +0100, Jacek Anaszewski wrote:
> >>> Introduce dedicated properties for conveying information about
> >>> LED function and color. Mark old "label" property as deprecated.
> >>>
> >>> Signed-off-by: Jacek Anaszewski 
> >>> Cc: Baolin Wang 
> >>> Cc: Daniel Mack 
> >>> Cc: Dan Murphy 
> >>> Cc: Linus Walleij 
> >>> Cc: Oleh Kravchenko 
> >>> Cc: Sakari Ailus 
> >>> Cc: Simon Shields 
> >>> Cc: Xiaotong Lu 
> >>> ---
> >>>  Documentation/devicetree/bindings/leds/common.txt | 52 
> >>> +++
> >>>  1 file changed, 44 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/leds/common.txt 
> >>> b/Documentation/devicetree/bindings/leds/common.txt
> >>> index aa13998..3efc826 100644
> >>> --- a/Documentation/devicetree/bindings/leds/common.txt
> >>> +++ b/Documentation/devicetree/bindings/leds/common.txt
> >>> @@ -10,14 +10,20 @@ can influence the way of the LED device 
> >>> initialization, the LED components
> >>>  have to be tightly coupled with the LED device binding. They are 
> >>> represented
> >>>  by child nodes of the parent LED device binding.
> >>>
> >>> +
> >>>  Optional properties for child nodes:
> >>>  - led-sources : List of device current outputs the LED is connected to. 
> >>> The
> >>> outputs are identified by the numbers that must be defined
> >>> in the LED device binding documentation.
> >>> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed 
> >>> definitions
> >>> +   from the header include/dt-bindings/leds/functions.h.
> >>> +   If there is no matching LED_FUNCTION available, add a new one.
> >>> +- color : Color of the LED.
> >>>  - label : The label for this LED. If omitted, the label is taken from 
> >>> the node
> >>>   name (excluding the unit address). It has to uniquely identify
> >>>   a device, i.e. no other LED class device can be assigned the same
> >>> - label.
> >>> + label. This property is deprecated - use 'function' and 'color'
> >>> + properties instead.
> >>
> >> I don't know if I'd go as far a deprecating.
> >>
> >> One thing to consider is how we handle multiple of the same function? Do
> >> we allow an index on function names? What if an index isn't meaningful
> >> and we need "front" vs. "rear" for example? Maybe label is still needed
> >> there.
> >
> > I believe that 'label' property with its old semantics must be preserved
> > for backwards compatibility - it so far has been used inconsistently for
> > conveying variations of devicename:color:function sections. If provided,
> > then it's been taken as-is for LED class device name, or concatenated
> > with the devicename hard-coded in the driver.
> >
> > Regarding the differentiation between the LEDs with functions of
> > same kind - OK, I agree, we will need another section.
> >
> > What seems to fits the best is the reference to the device it is
> > logically associated with.
> >
> > The question is whether the devicename should be provided in DT
> > literally, or by phandle, and then retrieved in runtime, basing on the
> > sysfs entry, like in case of input-leds bridge which for USB keyboard
> > creates LEDs named e.g.:
> >
> > input5::capslock
> > input5::numlock
> > input5::scrolllock
> >
> > Probably we will have to allow for some flexibility in this regard,
> > to allow for providing devicename literally like "rear", "front",
> > or like above input case.
>
> I must admit I have a dilemma  with this devicename part.
> Thinking more of it - the semantics of that section of LED
> class device name needs to be precise. Currently there seem
> to be two types of device names considered:
>
> 1) descriptive name like in case of flash -
>front-camera::flash, rear-camera::flash, which would allow
>for providing arbitrary devicenames, i.e. it would not improve
>LED naming too much, which was the aim of this patch set
> 2) name representing hardware relation or user-defined relation between
>some device and the LED class device, like in above keyboard
>LED case:
>
>- input5::numlock
>
>or for hard disk:
>
>- sda1::hdderr,
>
>In the hdderr case Pavel mentioned the arrangement where the LED
>is not a part of the hard disk, so the relation would have to be
>defined in DT.
>For the flash case the devicename part would be matching /dev/videoN
>device.
>
> I'm in favour of the option 2) since it seems to be more precise.
> In this case we would need a mechanism for asynchronous LED class
> device registration which would register a LED only after the
> associated device has been probed and its name is known.
> The mechanism would be a simpler form of
> drivers/media/v4l2-core/v4l2-async.c.
>
> In the LED DT node we would need a property holding a phandle
> to 

Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-30 Thread Rob Herring
On Tue, Nov 27, 2018 at 2:38 PM Jacek Anaszewski
 wrote:
>
> On 11/13/2018 09:57 PM, Jacek Anaszewski wrote:
> > On 11/12/2018 07:27 PM, Rob Herring wrote:
> >> On Tue, Nov 06, 2018 at 11:07:12PM +0100, Jacek Anaszewski wrote:
> >>> Introduce dedicated properties for conveying information about
> >>> LED function and color. Mark old "label" property as deprecated.
> >>>
> >>> Signed-off-by: Jacek Anaszewski 
> >>> Cc: Baolin Wang 
> >>> Cc: Daniel Mack 
> >>> Cc: Dan Murphy 
> >>> Cc: Linus Walleij 
> >>> Cc: Oleh Kravchenko 
> >>> Cc: Sakari Ailus 
> >>> Cc: Simon Shields 
> >>> Cc: Xiaotong Lu 
> >>> ---
> >>>  Documentation/devicetree/bindings/leds/common.txt | 52 
> >>> +++
> >>>  1 file changed, 44 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/leds/common.txt 
> >>> b/Documentation/devicetree/bindings/leds/common.txt
> >>> index aa13998..3efc826 100644
> >>> --- a/Documentation/devicetree/bindings/leds/common.txt
> >>> +++ b/Documentation/devicetree/bindings/leds/common.txt
> >>> @@ -10,14 +10,20 @@ can influence the way of the LED device 
> >>> initialization, the LED components
> >>>  have to be tightly coupled with the LED device binding. They are 
> >>> represented
> >>>  by child nodes of the parent LED device binding.
> >>>
> >>> +
> >>>  Optional properties for child nodes:
> >>>  - led-sources : List of device current outputs the LED is connected to. 
> >>> The
> >>> outputs are identified by the numbers that must be defined
> >>> in the LED device binding documentation.
> >>> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed 
> >>> definitions
> >>> +   from the header include/dt-bindings/leds/functions.h.
> >>> +   If there is no matching LED_FUNCTION available, add a new one.
> >>> +- color : Color of the LED.
> >>>  - label : The label for this LED. If omitted, the label is taken from 
> >>> the node
> >>>   name (excluding the unit address). It has to uniquely identify
> >>>   a device, i.e. no other LED class device can be assigned the same
> >>> - label.
> >>> + label. This property is deprecated - use 'function' and 'color'
> >>> + properties instead.
> >>
> >> I don't know if I'd go as far a deprecating.
> >>
> >> One thing to consider is how we handle multiple of the same function? Do
> >> we allow an index on function names? What if an index isn't meaningful
> >> and we need "front" vs. "rear" for example? Maybe label is still needed
> >> there.
> >
> > I believe that 'label' property with its old semantics must be preserved
> > for backwards compatibility - it so far has been used inconsistently for
> > conveying variations of devicename:color:function sections. If provided,
> > then it's been taken as-is for LED class device name, or concatenated
> > with the devicename hard-coded in the driver.
> >
> > Regarding the differentiation between the LEDs with functions of
> > same kind - OK, I agree, we will need another section.
> >
> > What seems to fits the best is the reference to the device it is
> > logically associated with.
> >
> > The question is whether the devicename should be provided in DT
> > literally, or by phandle, and then retrieved in runtime, basing on the
> > sysfs entry, like in case of input-leds bridge which for USB keyboard
> > creates LEDs named e.g.:
> >
> > input5::capslock
> > input5::numlock
> > input5::scrolllock
> >
> > Probably we will have to allow for some flexibility in this regard,
> > to allow for providing devicename literally like "rear", "front",
> > or like above input case.
>
> I must admit I have a dilemma  with this devicename part.
> Thinking more of it - the semantics of that section of LED
> class device name needs to be precise. Currently there seem
> to be two types of device names considered:
>
> 1) descriptive name like in case of flash -
>front-camera::flash, rear-camera::flash, which would allow
>for providing arbitrary devicenames, i.e. it would not improve
>LED naming too much, which was the aim of this patch set
> 2) name representing hardware relation or user-defined relation between
>some device and the LED class device, like in above keyboard
>LED case:
>
>- input5::numlock
>
>or for hard disk:
>
>- sda1::hdderr,
>
>In the hdderr case Pavel mentioned the arrangement where the LED
>is not a part of the hard disk, so the relation would have to be
>defined in DT.
>For the flash case the devicename part would be matching /dev/videoN
>device.
>
> I'm in favour of the option 2) since it seems to be more precise.
> In this case we would need a mechanism for asynchronous LED class
> device registration which would register a LED only after the
> associated device has been probed and its name is known.
> The mechanism would be a simpler form of
> drivers/media/v4l2-core/v4l2-async.c.
>
> In the LED DT node we would need a property holding a phandle
> to 

Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-27 Thread Jacek Anaszewski
On 11/13/2018 09:57 PM, Jacek Anaszewski wrote:
> On 11/12/2018 07:27 PM, Rob Herring wrote:
>> On Tue, Nov 06, 2018 at 11:07:12PM +0100, Jacek Anaszewski wrote:
>>> Introduce dedicated properties for conveying information about
>>> LED function and color. Mark old "label" property as deprecated.
>>>
>>> Signed-off-by: Jacek Anaszewski 
>>> Cc: Baolin Wang 
>>> Cc: Daniel Mack 
>>> Cc: Dan Murphy 
>>> Cc: Linus Walleij 
>>> Cc: Oleh Kravchenko 
>>> Cc: Sakari Ailus 
>>> Cc: Simon Shields 
>>> Cc: Xiaotong Lu 
>>> ---
>>>  Documentation/devicetree/bindings/leds/common.txt | 52 
>>> +++
>>>  1 file changed, 44 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/common.txt 
>>> b/Documentation/devicetree/bindings/leds/common.txt
>>> index aa13998..3efc826 100644
>>> --- a/Documentation/devicetree/bindings/leds/common.txt
>>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>>> @@ -10,14 +10,20 @@ can influence the way of the LED device initialization, 
>>> the LED components
>>>  have to be tightly coupled with the LED device binding. They are 
>>> represented
>>>  by child nodes of the parent LED device binding.
>>>  
>>> +
>>>  Optional properties for child nodes:
>>>  - led-sources : List of device current outputs the LED is connected to. The
>>> outputs are identified by the numbers that must be defined
>>> in the LED device binding documentation.
>>> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
>>> +   from the header include/dt-bindings/leds/functions.h.
>>> +   If there is no matching LED_FUNCTION available, add a new one.
>>> +- color : Color of the LED.
>>>  - label : The label for this LED. If omitted, the label is taken from the 
>>> node
>>>   name (excluding the unit address). It has to uniquely identify
>>>   a device, i.e. no other LED class device can be assigned the same
>>> - label.
>>> + label. This property is deprecated - use 'function' and 'color'
>>> + properties instead.
>>
>> I don't know if I'd go as far a deprecating.
>>
>> One thing to consider is how we handle multiple of the same function? Do 
>> we allow an index on function names? What if an index isn't meaningful 
>> and we need "front" vs. "rear" for example? Maybe label is still needed 
>> there. 
> 
> I believe that 'label' property with its old semantics must be preserved
> for backwards compatibility - it so far has been used inconsistently for
> conveying variations of devicename:color:function sections. If provided,
> then it's been taken as-is for LED class device name, or concatenated
> with the devicename hard-coded in the driver.
> 
> Regarding the differentiation between the LEDs with functions of
> same kind - OK, I agree, we will need another section.
> 
> What seems to fits the best is the reference to the device it is
> logically associated with.
> 
> The question is whether the devicename should be provided in DT
> literally, or by phandle, and then retrieved in runtime, basing on the
> sysfs entry, like in case of input-leds bridge which for USB keyboard
> creates LEDs named e.g.:
> 
> input5::capslock
> input5::numlock
> input5::scrolllock
> 
> Probably we will have to allow for some flexibility in this regard,
> to allow for providing devicename literally like "rear", "front",
> or like above input case.

I must admit I have a dilemma  with this devicename part.
Thinking more of it - the semantics of that section of LED
class device name needs to be precise. Currently there seem
to be two types of device names considered:

1) descriptive name like in case of flash -
   front-camera::flash, rear-camera::flash, which would allow
   for providing arbitrary devicenames, i.e. it would not improve
   LED naming too much, which was the aim of this patch set
2) name representing hardware relation or user-defined relation between
   some device and the LED class device, like in above keyboard
   LED case:

   - input5::numlock

   or for hard disk:

   - sda1::hdderr,

   In the hdderr case Pavel mentioned the arrangement where the LED
   is not a part of the hard disk, so the relation would have to be
   defined in DT.
   For the flash case the devicename part would be matching /dev/videoN
   device.

I'm in favour of the option 2) since it seems to be more precise.
In this case we would need a mechanism for asynchronous LED class
device registration which would register a LED only after the
associated device has been probed and its name is known.
The mechanism would be a simpler form of
drivers/media/v4l2-core/v4l2-async.c.

In the LED DT node we would need a property holding a phandle
to the associated device. Then, in the runtime the related device
would call led_register_associated_device(struct device_node*,
struct device*) which would in turn match the device_node with
the phandle assigned to the property in the LED DT node. Having
the struct device of the 

Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-27 Thread Jacek Anaszewski
On 11/13/2018 09:57 PM, Jacek Anaszewski wrote:
> On 11/12/2018 07:27 PM, Rob Herring wrote:
>> On Tue, Nov 06, 2018 at 11:07:12PM +0100, Jacek Anaszewski wrote:
>>> Introduce dedicated properties for conveying information about
>>> LED function and color. Mark old "label" property as deprecated.
>>>
>>> Signed-off-by: Jacek Anaszewski 
>>> Cc: Baolin Wang 
>>> Cc: Daniel Mack 
>>> Cc: Dan Murphy 
>>> Cc: Linus Walleij 
>>> Cc: Oleh Kravchenko 
>>> Cc: Sakari Ailus 
>>> Cc: Simon Shields 
>>> Cc: Xiaotong Lu 
>>> ---
>>>  Documentation/devicetree/bindings/leds/common.txt | 52 
>>> +++
>>>  1 file changed, 44 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/common.txt 
>>> b/Documentation/devicetree/bindings/leds/common.txt
>>> index aa13998..3efc826 100644
>>> --- a/Documentation/devicetree/bindings/leds/common.txt
>>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>>> @@ -10,14 +10,20 @@ can influence the way of the LED device initialization, 
>>> the LED components
>>>  have to be tightly coupled with the LED device binding. They are 
>>> represented
>>>  by child nodes of the parent LED device binding.
>>>  
>>> +
>>>  Optional properties for child nodes:
>>>  - led-sources : List of device current outputs the LED is connected to. The
>>> outputs are identified by the numbers that must be defined
>>> in the LED device binding documentation.
>>> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
>>> +   from the header include/dt-bindings/leds/functions.h.
>>> +   If there is no matching LED_FUNCTION available, add a new one.
>>> +- color : Color of the LED.
>>>  - label : The label for this LED. If omitted, the label is taken from the 
>>> node
>>>   name (excluding the unit address). It has to uniquely identify
>>>   a device, i.e. no other LED class device can be assigned the same
>>> - label.
>>> + label. This property is deprecated - use 'function' and 'color'
>>> + properties instead.
>>
>> I don't know if I'd go as far a deprecating.
>>
>> One thing to consider is how we handle multiple of the same function? Do 
>> we allow an index on function names? What if an index isn't meaningful 
>> and we need "front" vs. "rear" for example? Maybe label is still needed 
>> there. 
> 
> I believe that 'label' property with its old semantics must be preserved
> for backwards compatibility - it so far has been used inconsistently for
> conveying variations of devicename:color:function sections. If provided,
> then it's been taken as-is for LED class device name, or concatenated
> with the devicename hard-coded in the driver.
> 
> Regarding the differentiation between the LEDs with functions of
> same kind - OK, I agree, we will need another section.
> 
> What seems to fits the best is the reference to the device it is
> logically associated with.
> 
> The question is whether the devicename should be provided in DT
> literally, or by phandle, and then retrieved in runtime, basing on the
> sysfs entry, like in case of input-leds bridge which for USB keyboard
> creates LEDs named e.g.:
> 
> input5::capslock
> input5::numlock
> input5::scrolllock
> 
> Probably we will have to allow for some flexibility in this regard,
> to allow for providing devicename literally like "rear", "front",
> or like above input case.

I must admit I have a dilemma  with this devicename part.
Thinking more of it - the semantics of that section of LED
class device name needs to be precise. Currently there seem
to be two types of device names considered:

1) descriptive name like in case of flash -
   front-camera::flash, rear-camera::flash, which would allow
   for providing arbitrary devicenames, i.e. it would not improve
   LED naming too much, which was the aim of this patch set
2) name representing hardware relation or user-defined relation between
   some device and the LED class device, like in above keyboard
   LED case:

   - input5::numlock

   or for hard disk:

   - sda1::hdderr,

   In the hdderr case Pavel mentioned the arrangement where the LED
   is not a part of the hard disk, so the relation would have to be
   defined in DT.
   For the flash case the devicename part would be matching /dev/videoN
   device.

I'm in favour of the option 2) since it seems to be more precise.
In this case we would need a mechanism for asynchronous LED class
device registration which would register a LED only after the
associated device has been probed and its name is known.
The mechanism would be a simpler form of
drivers/media/v4l2-core/v4l2-async.c.

In the LED DT node we would need a property holding a phandle
to the associated device. Then, in the runtime the related device
would call led_register_associated_device(struct device_node*,
struct device*) which would in turn match the device_node with
the phandle assigned to the property in the LED DT node. Having
the struct device of the 

Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-13 Thread Jacek Anaszewski
On 11/12/2018 07:27 PM, Rob Herring wrote:
> On Tue, Nov 06, 2018 at 11:07:12PM +0100, Jacek Anaszewski wrote:
>> Introduce dedicated properties for conveying information about
>> LED function and color. Mark old "label" property as deprecated.
>>
>> Signed-off-by: Jacek Anaszewski 
>> Cc: Baolin Wang 
>> Cc: Daniel Mack 
>> Cc: Dan Murphy 
>> Cc: Linus Walleij 
>> Cc: Oleh Kravchenko 
>> Cc: Sakari Ailus 
>> Cc: Simon Shields 
>> Cc: Xiaotong Lu 
>> ---
>>  Documentation/devicetree/bindings/leds/common.txt | 52 
>> +++
>>  1 file changed, 44 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/common.txt 
>> b/Documentation/devicetree/bindings/leds/common.txt
>> index aa13998..3efc826 100644
>> --- a/Documentation/devicetree/bindings/leds/common.txt
>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>> @@ -10,14 +10,20 @@ can influence the way of the LED device initialization, 
>> the LED components
>>  have to be tightly coupled with the LED device binding. They are represented
>>  by child nodes of the parent LED device binding.
>>  
>> +
>>  Optional properties for child nodes:
>>  - led-sources : List of device current outputs the LED is connected to. The
>>  outputs are identified by the numbers that must be defined
>>  in the LED device binding documentation.
>> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
>> +from the header include/dt-bindings/leds/functions.h.
>> +If there is no matching LED_FUNCTION available, add a new one.
>> +- color : Color of the LED.
>>  - label : The label for this LED. If omitted, the label is taken from the 
>> node
>>name (excluding the unit address). It has to uniquely identify
>>a device, i.e. no other LED class device can be assigned the same
>> -  label.
>> +  label. This property is deprecated - use 'function' and 'color'
>> +  properties instead.
> 
> I don't know if I'd go as far a deprecating.
> 
> One thing to consider is how we handle multiple of the same function? Do 
> we allow an index on function names? What if an index isn't meaningful 
> and we need "front" vs. "rear" for example? Maybe label is still needed 
> there. 

I believe that 'label' property with its old semantics must be preserved
for backwards compatibility - it so far has been used inconsistently for
conveying variations of devicename:color:function sections. If provided,
then it's been taken as-is for LED class device name, or concatenated
with the devicename hard-coded in the driver.

Regarding the differentiation between the LEDs with functions of
same kind - OK, I agree, we will need another section.

What seems to fits the best is the reference to the device it is
logically associated with.

The question is whether the devicename should be provided in DT
literally, or by phandle, and then retrieved in runtime, basing on the
sysfs entry, like in case of input-leds bridge which for USB keyboard
creates LEDs named e.g.:

input5::capslock
input5::numlock
input5::scrolllock

Probably we will have to allow for some flexibility in this regard,
to allow for providing devicename literally like "rear", "front",
or like above input case.

-- 
Best regards,
Jacek Anaszewski


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-13 Thread Jacek Anaszewski
On 11/12/2018 07:27 PM, Rob Herring wrote:
> On Tue, Nov 06, 2018 at 11:07:12PM +0100, Jacek Anaszewski wrote:
>> Introduce dedicated properties for conveying information about
>> LED function and color. Mark old "label" property as deprecated.
>>
>> Signed-off-by: Jacek Anaszewski 
>> Cc: Baolin Wang 
>> Cc: Daniel Mack 
>> Cc: Dan Murphy 
>> Cc: Linus Walleij 
>> Cc: Oleh Kravchenko 
>> Cc: Sakari Ailus 
>> Cc: Simon Shields 
>> Cc: Xiaotong Lu 
>> ---
>>  Documentation/devicetree/bindings/leds/common.txt | 52 
>> +++
>>  1 file changed, 44 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/common.txt 
>> b/Documentation/devicetree/bindings/leds/common.txt
>> index aa13998..3efc826 100644
>> --- a/Documentation/devicetree/bindings/leds/common.txt
>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>> @@ -10,14 +10,20 @@ can influence the way of the LED device initialization, 
>> the LED components
>>  have to be tightly coupled with the LED device binding. They are represented
>>  by child nodes of the parent LED device binding.
>>  
>> +
>>  Optional properties for child nodes:
>>  - led-sources : List of device current outputs the LED is connected to. The
>>  outputs are identified by the numbers that must be defined
>>  in the LED device binding documentation.
>> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
>> +from the header include/dt-bindings/leds/functions.h.
>> +If there is no matching LED_FUNCTION available, add a new one.
>> +- color : Color of the LED.
>>  - label : The label for this LED. If omitted, the label is taken from the 
>> node
>>name (excluding the unit address). It has to uniquely identify
>>a device, i.e. no other LED class device can be assigned the same
>> -  label.
>> +  label. This property is deprecated - use 'function' and 'color'
>> +  properties instead.
> 
> I don't know if I'd go as far a deprecating.
> 
> One thing to consider is how we handle multiple of the same function? Do 
> we allow an index on function names? What if an index isn't meaningful 
> and we need "front" vs. "rear" for example? Maybe label is still needed 
> there. 

I believe that 'label' property with its old semantics must be preserved
for backwards compatibility - it so far has been used inconsistently for
conveying variations of devicename:color:function sections. If provided,
then it's been taken as-is for LED class device name, or concatenated
with the devicename hard-coded in the driver.

Regarding the differentiation between the LEDs with functions of
same kind - OK, I agree, we will need another section.

What seems to fits the best is the reference to the device it is
logically associated with.

The question is whether the devicename should be provided in DT
literally, or by phandle, and then retrieved in runtime, basing on the
sysfs entry, like in case of input-leds bridge which for USB keyboard
creates LEDs named e.g.:

input5::capslock
input5::numlock
input5::scrolllock

Probably we will have to allow for some flexibility in this regard,
to allow for providing devicename literally like "rear", "front",
or like above input case.

-- 
Best regards,
Jacek Anaszewski


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-13 Thread Jacek Anaszewski
Hi Vesa,

On 11/13/2018 08:10 AM, Vesa Jääskeläinen wrote:
> Hi Jacek,
> 
> On 12/11/2018 18.02, Jacek Anaszewski wrote:
 Support for RGB LEDs, or other variations of LED synchronization
 have been approached several times, but without satisfying result
 so far.

 Generally the problem boils down to the issue of how triggers
 should handle multiple synchronized LED class devices in a backwards
 compatible way with monochrome LEDs.

 At some point the HSV [0] approach was proposed, but there was a
 problem
 with getting uniform colors across devices. Especially white.
 Certainly a calibration mechanism would be required.

 [0] https://lkml.org/lkml/2017/8/30/423
>>>
>>> We do not usually use PWM controlled leds so our calibration has been HW
>>> engineer tuning the resistors for the leds to get acceptable color for
>>> different colors variations.
>>>
>>> If one wants to have fixed colors for such device then one could use
>>> this similar method to define them or/and free form interface to enter
>>> RGB values there. Thou with PWM RGB leds you probably want to have more
>>> animation there which probably would require some additional support
>>> code.
>>>
>>> One way to do atomic PWM RGB color change with sysfs could be:
>>>
>>> echo "21 223 242" > color
>>>
>>> or
>>>
>>> echo "21 223 242" > rgb
>>>
>>> or:
>>>
>>> echo "r:21 g:232 b:242" > color (or something similar)
>>>
>>> and if there is know registered name then just write it to "color" which
>>> would pick registered color rgb values to led instances rgb value.
>>>
>>> Now for PWM RGB led one could use "brightness" and "rgb" value to
>>> calculate actual color with some color space formula (like hsv in [0]).
>>>
>>> Doing white with RGB LED is a bit hard so usually you want to get RGBW
>>> LED (or RGBAW LED) if "real" white is something that is needed. This
>>> could then be "rgbw" entry and "color" to pick from fixed set.
>>>
>>> These presets in device tree for "color" could be considered one way of
>>> doing calibration for particular hardware.
>>>
>>> So in device tree for RGB PWM led it could be like:
>>>
>>> color-orange {
>>>  rgb = <249 197 9>
>>> }
>>>
>>> color-warm-white {
>>>  rgb = <255 253 249>
>>> }
>>>
>>> How would that sound like?
>>
>> Thank you for the description of your approach.
>>
>> Predefined DT color definitions make an interesting option. Nonetheless,
>> your design assumes that for RGB LEDs max_brightness would have to be
>> always 1.
>>
>> While it would solve the issue, it wouldn't allow to benefit from
>> the whole potential of RGB LEDs - think of having adjustable "color
>> brightness" (like in HSV color model).
> 
> What I tried to describe above was that these could also work with HSV
> model also with larger brigtness values too.
> 
> Let's say we do following sequence to change from off state to user
> configured intensity value:
> 
> # Turn off leds
> echo 0 > brightness
> 
> # Select requested color
> echo "orange" > color
> 
> # Use configured LED intensity from user configuration
> echo 84 > brightness
> 
> Driver would now use rgb value <249 197 9> and brightness <84> to
> calculate (with HSV solution) real PWM values for the led component
> colors. With led controller this is probably easier to get linear
> feedback but for generic PWM controller you may want to utilize PWM
> curve feature like defined for backlight driver.
> 
> When thinking this I instantly see my self with thinking PS4's
> sliding/pulsing color animation for orange/blue/white. And controlling
> this animation steps manually from user space probably is not the best
> idea so like with blinking you would need more accurately timed action
> within kernel I suppose (or hardware with the support).
> 
> With that idea forward:
> 
> # Turning animation sequencer off causes instant changes for values
> echo "off" > animation-sequencer
> 
> # User 1 second for animation (no effect yet as sequencer is off)
> echo 1000 > animation-time-ms (or so)
> 
> # Turn off leds (instance change)
> echo 0 > brightness
> 
> # Select requested color (instant change)
> echo "orange" > color
> 
> # Change to linear animation sequencer
> echo "linear" > animation-sequencer
> 
> # Now everything from this point is subject to animation sequencer
> # Sequencer remember active state and next state (as seen in sysfs)
> 
> # Use configured LED intensity from user configuration (no effect yet)
> echo 84 > brightness
> 
> # Trigger transition sequence
> echo load > animation-sequencer
> 
> # This would cause 1 second animation from black to orange with user
> intensivity of 84
> 
> # Wait for transition to complete and some extra
> sleep 4
> 
> # Change color to blue with animation
> echo "blue" > color
> echo load > animation-sequencer
> 
> # This would cause driver to change from orange to blue in current
> intensivity level during 1 second period
> 
> # -- end --
> 
> Now that went a bit ahead of time but 

Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-13 Thread Jacek Anaszewski
Hi Vesa,

On 11/13/2018 08:10 AM, Vesa Jääskeläinen wrote:
> Hi Jacek,
> 
> On 12/11/2018 18.02, Jacek Anaszewski wrote:
 Support for RGB LEDs, or other variations of LED synchronization
 have been approached several times, but without satisfying result
 so far.

 Generally the problem boils down to the issue of how triggers
 should handle multiple synchronized LED class devices in a backwards
 compatible way with monochrome LEDs.

 At some point the HSV [0] approach was proposed, but there was a
 problem
 with getting uniform colors across devices. Especially white.
 Certainly a calibration mechanism would be required.

 [0] https://lkml.org/lkml/2017/8/30/423
>>>
>>> We do not usually use PWM controlled leds so our calibration has been HW
>>> engineer tuning the resistors for the leds to get acceptable color for
>>> different colors variations.
>>>
>>> If one wants to have fixed colors for such device then one could use
>>> this similar method to define them or/and free form interface to enter
>>> RGB values there. Thou with PWM RGB leds you probably want to have more
>>> animation there which probably would require some additional support
>>> code.
>>>
>>> One way to do atomic PWM RGB color change with sysfs could be:
>>>
>>> echo "21 223 242" > color
>>>
>>> or
>>>
>>> echo "21 223 242" > rgb
>>>
>>> or:
>>>
>>> echo "r:21 g:232 b:242" > color (or something similar)
>>>
>>> and if there is know registered name then just write it to "color" which
>>> would pick registered color rgb values to led instances rgb value.
>>>
>>> Now for PWM RGB led one could use "brightness" and "rgb" value to
>>> calculate actual color with some color space formula (like hsv in [0]).
>>>
>>> Doing white with RGB LED is a bit hard so usually you want to get RGBW
>>> LED (or RGBAW LED) if "real" white is something that is needed. This
>>> could then be "rgbw" entry and "color" to pick from fixed set.
>>>
>>> These presets in device tree for "color" could be considered one way of
>>> doing calibration for particular hardware.
>>>
>>> So in device tree for RGB PWM led it could be like:
>>>
>>> color-orange {
>>>  rgb = <249 197 9>
>>> }
>>>
>>> color-warm-white {
>>>  rgb = <255 253 249>
>>> }
>>>
>>> How would that sound like?
>>
>> Thank you for the description of your approach.
>>
>> Predefined DT color definitions make an interesting option. Nonetheless,
>> your design assumes that for RGB LEDs max_brightness would have to be
>> always 1.
>>
>> While it would solve the issue, it wouldn't allow to benefit from
>> the whole potential of RGB LEDs - think of having adjustable "color
>> brightness" (like in HSV color model).
> 
> What I tried to describe above was that these could also work with HSV
> model also with larger brigtness values too.
> 
> Let's say we do following sequence to change from off state to user
> configured intensity value:
> 
> # Turn off leds
> echo 0 > brightness
> 
> # Select requested color
> echo "orange" > color
> 
> # Use configured LED intensity from user configuration
> echo 84 > brightness
> 
> Driver would now use rgb value <249 197 9> and brightness <84> to
> calculate (with HSV solution) real PWM values for the led component
> colors. With led controller this is probably easier to get linear
> feedback but for generic PWM controller you may want to utilize PWM
> curve feature like defined for backlight driver.
> 
> When thinking this I instantly see my self with thinking PS4's
> sliding/pulsing color animation for orange/blue/white. And controlling
> this animation steps manually from user space probably is not the best
> idea so like with blinking you would need more accurately timed action
> within kernel I suppose (or hardware with the support).
> 
> With that idea forward:
> 
> # Turning animation sequencer off causes instant changes for values
> echo "off" > animation-sequencer
> 
> # User 1 second for animation (no effect yet as sequencer is off)
> echo 1000 > animation-time-ms (or so)
> 
> # Turn off leds (instance change)
> echo 0 > brightness
> 
> # Select requested color (instant change)
> echo "orange" > color
> 
> # Change to linear animation sequencer
> echo "linear" > animation-sequencer
> 
> # Now everything from this point is subject to animation sequencer
> # Sequencer remember active state and next state (as seen in sysfs)
> 
> # Use configured LED intensity from user configuration (no effect yet)
> echo 84 > brightness
> 
> # Trigger transition sequence
> echo load > animation-sequencer
> 
> # This would cause 1 second animation from black to orange with user
> intensivity of 84
> 
> # Wait for transition to complete and some extra
> sleep 4
> 
> # Change color to blue with animation
> echo "blue" > color
> echo load > animation-sequencer
> 
> # This would cause driver to change from orange to blue in current
> intensivity level during 1 second period
> 
> # -- end --
> 
> Now that went a bit ahead of time but 

Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-12 Thread Vesa Jääskeläinen

Hi Jacek,

On 12/11/2018 18.02, Jacek Anaszewski wrote:

Support for RGB LEDs, or other variations of LED synchronization
have been approached several times, but without satisfying result
so far.

Generally the problem boils down to the issue of how triggers
should handle multiple synchronized LED class devices in a backwards
compatible way with monochrome LEDs.

At some point the HSV [0] approach was proposed, but there was a problem
with getting uniform colors across devices. Especially white.
Certainly a calibration mechanism would be required.

[0] https://lkml.org/lkml/2017/8/30/423


We do not usually use PWM controlled leds so our calibration has been HW
engineer tuning the resistors for the leds to get acceptable color for
different colors variations.

If one wants to have fixed colors for such device then one could use
this similar method to define them or/and free form interface to enter
RGB values there. Thou with PWM RGB leds you probably want to have more
animation there which probably would require some additional support code.

One way to do atomic PWM RGB color change with sysfs could be:

echo "21 223 242" > color

or

echo "21 223 242" > rgb

or:

echo "r:21 g:232 b:242" > color (or something similar)

and if there is know registered name then just write it to "color" which
would pick registered color rgb values to led instances rgb value.

Now for PWM RGB led one could use "brightness" and "rgb" value to
calculate actual color with some color space formula (like hsv in [0]).

Doing white with RGB LED is a bit hard so usually you want to get RGBW
LED (or RGBAW LED) if "real" white is something that is needed. This
could then be "rgbw" entry and "color" to pick from fixed set.

These presets in device tree for "color" could be considered one way of
doing calibration for particular hardware.

So in device tree for RGB PWM led it could be like:

color-orange {
     rgb = <249 197 9>
}

color-warm-white {
     rgb = <255 253 249>
}

How would that sound like?


Thank you for the description of your approach.

Predefined DT color definitions make an interesting option. Nonetheless,
your design assumes that for RGB LEDs max_brightness would have to be
always 1.

While it would solve the issue, it wouldn't allow to benefit from
the whole potential of RGB LEDs - think of having adjustable "color
brightness" (like in HSV color model).


What I tried to describe above was that these could also work with HSV 
model also with larger brigtness values too.


Let's say we do following sequence to change from off state to user 
configured intensity value:


# Turn off leds
echo 0 > brightness

# Select requested color
echo "orange" > color

# Use configured LED intensity from user configuration
echo 84 > brightness

Driver would now use rgb value <249 197 9> and brightness <84> to 
calculate (with HSV solution) real PWM values for the led component 
colors. With led controller this is probably easier to get linear 
feedback but for generic PWM controller you may want to utilize PWM 
curve feature like defined for backlight driver.


When thinking this I instantly see my self with thinking PS4's 
sliding/pulsing color animation for orange/blue/white. And controlling 
this animation steps manually from user space probably is not the best 
idea so like with blinking you would need more accurately timed action 
within kernel I suppose (or hardware with the support).


With that idea forward:

# Turning animation sequencer off causes instant changes for values
echo "off" > animation-sequencer

# User 1 second for animation (no effect yet as sequencer is off)
echo 1000 > animation-time-ms (or so)

# Turn off leds (instance change)
echo 0 > brightness

# Select requested color (instant change)
echo "orange" > color

# Change to linear animation sequencer
echo "linear" > animation-sequencer

# Now everything from this point is subject to animation sequencer
# Sequencer remember active state and next state (as seen in sysfs)

# Use configured LED intensity from user configuration (no effect yet)
echo 84 > brightness

# Trigger transition sequence
echo load > animation-sequencer

# This would cause 1 second animation from black to orange with user 
intensivity of 84


# Wait for transition to complete and some extra
sleep 4

# Change color to blue with animation
echo "blue" > color
echo load > animation-sequencer

# This would cause driver to change from orange to blue in current 
intensivity level during 1 second period


# -- end --

Now that went a bit ahead of time but I believe it demonstrates the 
potential for this approach with combined with HSL functionality for future.



How abut allowing for providing an array of color intensity/brightness
levels per each color? In the simplest case there could be a single
level per color, but it should be possible to provide up to 255 levels.


I believe that brightness and color should be separate topics.

We have in our devices option in user interface to 

Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-12 Thread Vesa Jääskeläinen

Hi Jacek,

On 12/11/2018 18.02, Jacek Anaszewski wrote:

Support for RGB LEDs, or other variations of LED synchronization
have been approached several times, but without satisfying result
so far.

Generally the problem boils down to the issue of how triggers
should handle multiple synchronized LED class devices in a backwards
compatible way with monochrome LEDs.

At some point the HSV [0] approach was proposed, but there was a problem
with getting uniform colors across devices. Especially white.
Certainly a calibration mechanism would be required.

[0] https://lkml.org/lkml/2017/8/30/423


We do not usually use PWM controlled leds so our calibration has been HW
engineer tuning the resistors for the leds to get acceptable color for
different colors variations.

If one wants to have fixed colors for such device then one could use
this similar method to define them or/and free form interface to enter
RGB values there. Thou with PWM RGB leds you probably want to have more
animation there which probably would require some additional support code.

One way to do atomic PWM RGB color change with sysfs could be:

echo "21 223 242" > color

or

echo "21 223 242" > rgb

or:

echo "r:21 g:232 b:242" > color (or something similar)

and if there is know registered name then just write it to "color" which
would pick registered color rgb values to led instances rgb value.

Now for PWM RGB led one could use "brightness" and "rgb" value to
calculate actual color with some color space formula (like hsv in [0]).

Doing white with RGB LED is a bit hard so usually you want to get RGBW
LED (or RGBAW LED) if "real" white is something that is needed. This
could then be "rgbw" entry and "color" to pick from fixed set.

These presets in device tree for "color" could be considered one way of
doing calibration for particular hardware.

So in device tree for RGB PWM led it could be like:

color-orange {
     rgb = <249 197 9>
}

color-warm-white {
     rgb = <255 253 249>
}

How would that sound like?


Thank you for the description of your approach.

Predefined DT color definitions make an interesting option. Nonetheless,
your design assumes that for RGB LEDs max_brightness would have to be
always 1.

While it would solve the issue, it wouldn't allow to benefit from
the whole potential of RGB LEDs - think of having adjustable "color
brightness" (like in HSV color model).


What I tried to describe above was that these could also work with HSV 
model also with larger brigtness values too.


Let's say we do following sequence to change from off state to user 
configured intensity value:


# Turn off leds
echo 0 > brightness

# Select requested color
echo "orange" > color

# Use configured LED intensity from user configuration
echo 84 > brightness

Driver would now use rgb value <249 197 9> and brightness <84> to 
calculate (with HSV solution) real PWM values for the led component 
colors. With led controller this is probably easier to get linear 
feedback but for generic PWM controller you may want to utilize PWM 
curve feature like defined for backlight driver.


When thinking this I instantly see my self with thinking PS4's 
sliding/pulsing color animation for orange/blue/white. And controlling 
this animation steps manually from user space probably is not the best 
idea so like with blinking you would need more accurately timed action 
within kernel I suppose (or hardware with the support).


With that idea forward:

# Turning animation sequencer off causes instant changes for values
echo "off" > animation-sequencer

# User 1 second for animation (no effect yet as sequencer is off)
echo 1000 > animation-time-ms (or so)

# Turn off leds (instance change)
echo 0 > brightness

# Select requested color (instant change)
echo "orange" > color

# Change to linear animation sequencer
echo "linear" > animation-sequencer

# Now everything from this point is subject to animation sequencer
# Sequencer remember active state and next state (as seen in sysfs)

# Use configured LED intensity from user configuration (no effect yet)
echo 84 > brightness

# Trigger transition sequence
echo load > animation-sequencer

# This would cause 1 second animation from black to orange with user 
intensivity of 84


# Wait for transition to complete and some extra
sleep 4

# Change color to blue with animation
echo "blue" > color
echo load > animation-sequencer

# This would cause driver to change from orange to blue in current 
intensivity level during 1 second period


# -- end --

Now that went a bit ahead of time but I believe it demonstrates the 
potential for this approach with combined with HSL functionality for future.



How abut allowing for providing an array of color intensity/brightness
levels per each color? In the simplest case there could be a single
level per color, but it should be possible to provide up to 255 levels.


I believe that brightness and color should be separate topics.

We have in our devices option in user interface to 

Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-12 Thread Jacek Anaszewski
Hi Vesa,

On 11/10/2018 06:19 PM, Vesa Jääskeläinen wrote:
> Hi Jacek,
> 
> On 09/11/2018 22.42, Jacek Anaszewski wrote:
>> Hi Vesa,
>>
>> On 11/09/2018 09:32 AM, Vesa Jääskeläinen wrote:
>>> On 07/11/2018 0.07, Jacek Anaszewski wrote:
 Introduce dedicated properties for conveying information about
 LED function and color. Mark old "label" property as deprecated.

 Signed-off-by: Jacek Anaszewski 
 Cc: Baolin Wang 
 Cc: Daniel Mack 
 Cc: Dan Murphy 
 Cc: Linus Walleij 
 Cc: Oleh Kravchenko 
 Cc: Sakari Ailus 
 Cc: Simon Shields 
 Cc: Xiaotong Lu 
 ---
    Documentation/devicetree/bindings/leds/common.txt | 52
 +++
    1 file changed, 44 insertions(+), 8 deletions(-)

 diff --git a/Documentation/devicetree/bindings/leds/common.txt
 b/Documentation/devicetree/bindings/leds/common.txt
 index aa13998..3efc826 100644
 --- a/Documentation/devicetree/bindings/leds/common.txt
 +++ b/Documentation/devicetree/bindings/leds/common.txt
 @@ -10,14 +10,20 @@ can influence the way of the LED device
 initialization, the LED components
    have to be tightly coupled with the LED device binding. They are
 represented
    by child nodes of the parent LED device binding.
    +
    Optional properties for child nodes:
    - led-sources : List of device current outputs the LED is connected
 to. The
    outputs are identified by the numbers that must be defined
    in the LED device binding documentation.
 +- function: LED functon. Use one of the LED_FUNCTION_* prefixed
 definitions
 +    from the header include/dt-bindings/leds/functions.h.
 +    If there is no matching LED_FUNCTION available, add a new one.
 +- color : Color of the LED.
>>>
>>> We have had for years out-of-tree patch for multi color gpio led driver
>>> which extends this concept with multiple colors. Then in sysfs there has
>>> been possibility to control the color and otherwise use blinking or
>>> other features.
>>>
>>> Our need is multi color status led of the device which includes
>>> different kind of blinkings and colors on different situations.
>>>
>>> Current in-tree gpio led driver just wasn't atomic enough and a bit
>>> clumsy interface for handling this.
>>>
>>> Now that this is being looked at could we come up with solution that we
>>> could define multiple colors for one led in device tree and then we
>>> could work on getting the driver upstreamed?
>>>
>>> What we did was generally:
>>>
>>> leds-multi {
>>>  compatible = "gpio-multi-leds";
>>>
>>>  status {
>>>  gpios = <...>;
>>>  linux,default-trigger = "none";
>>>  deafult-state = "keep";
>>>
>>>  color-red {
>>>  pin-mask = <0x01>;
>>>  };
>>>
>>>  color-green {
>>>  pin-mask = <0x02>;
>>>  };
>>>
>>>  color-orange {
>>>  pin-mask = <0x03>;
>>>  };
>>>  };
>>> };
>>>
>>
>> Device Tree node implementation doesn't actually say too much
>> about the sysfs interface you came up with, which is most vital
>> in this case.
> 
> In our case it is very simple. There is "color" named sysfs node under
> gpio instance.
> 
> It creates own led instance for each children in this case it is named
> as "status" and then uses properties under it to define operation.
> 
> Currently we do have fixed list of color names in driver to require
> "standardized" names but that could be easily changed to be dynamic.
> 
> Driver registers all GPIOs defined in "gpios" under the instance.
> 
> So one can say:
> 
> echo "orange" > color
> 
> This setting goes to the driver, it figures out logical name "orange"
> and the configure all listed GPIO's to its "pin-mask" state. Polarity of
> the GPIO's is configured in GPIO definition so this is more or less turn
> this particular pin to activate state.
> 
> So in this case it changes led's color to orange and still lets one to
> use standard led triggers. Eg. set periodic blinking or so.
> 
> If one cat's "color" it lists all available colors for user and shows
> which one is active.
> 
> When brightness is configured as zero the all registered go to off state
> and when non-zero then it goes to state what ever is configured with
> "color" sysfs entry.
> 
>> Support for RGB LEDs, or other variations of LED synchronization
>> have been approached several times, but without satisfying result
>> so far.
>>
>> Generally the problem boils down to the issue of how triggers
>> should handle multiple synchronized LED class devices in a backwards
>> compatible way with monochrome LEDs.
>>
>> At some point the HSV [0] approach was proposed, but there was a problem
>> with getting uniform colors across devices. Especially white.
>> Certainly a calibration mechanism would be required.
>>
>> [0] https://lkml.org/lkml/2017/8/30/423
> 
> We do not usually use PWM controlled leds 

Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-12 Thread Jacek Anaszewski
Hi Vesa,

On 11/10/2018 06:19 PM, Vesa Jääskeläinen wrote:
> Hi Jacek,
> 
> On 09/11/2018 22.42, Jacek Anaszewski wrote:
>> Hi Vesa,
>>
>> On 11/09/2018 09:32 AM, Vesa Jääskeläinen wrote:
>>> On 07/11/2018 0.07, Jacek Anaszewski wrote:
 Introduce dedicated properties for conveying information about
 LED function and color. Mark old "label" property as deprecated.

 Signed-off-by: Jacek Anaszewski 
 Cc: Baolin Wang 
 Cc: Daniel Mack 
 Cc: Dan Murphy 
 Cc: Linus Walleij 
 Cc: Oleh Kravchenko 
 Cc: Sakari Ailus 
 Cc: Simon Shields 
 Cc: Xiaotong Lu 
 ---
    Documentation/devicetree/bindings/leds/common.txt | 52
 +++
    1 file changed, 44 insertions(+), 8 deletions(-)

 diff --git a/Documentation/devicetree/bindings/leds/common.txt
 b/Documentation/devicetree/bindings/leds/common.txt
 index aa13998..3efc826 100644
 --- a/Documentation/devicetree/bindings/leds/common.txt
 +++ b/Documentation/devicetree/bindings/leds/common.txt
 @@ -10,14 +10,20 @@ can influence the way of the LED device
 initialization, the LED components
    have to be tightly coupled with the LED device binding. They are
 represented
    by child nodes of the parent LED device binding.
    +
    Optional properties for child nodes:
    - led-sources : List of device current outputs the LED is connected
 to. The
    outputs are identified by the numbers that must be defined
    in the LED device binding documentation.
 +- function: LED functon. Use one of the LED_FUNCTION_* prefixed
 definitions
 +    from the header include/dt-bindings/leds/functions.h.
 +    If there is no matching LED_FUNCTION available, add a new one.
 +- color : Color of the LED.
>>>
>>> We have had for years out-of-tree patch for multi color gpio led driver
>>> which extends this concept with multiple colors. Then in sysfs there has
>>> been possibility to control the color and otherwise use blinking or
>>> other features.
>>>
>>> Our need is multi color status led of the device which includes
>>> different kind of blinkings and colors on different situations.
>>>
>>> Current in-tree gpio led driver just wasn't atomic enough and a bit
>>> clumsy interface for handling this.
>>>
>>> Now that this is being looked at could we come up with solution that we
>>> could define multiple colors for one led in device tree and then we
>>> could work on getting the driver upstreamed?
>>>
>>> What we did was generally:
>>>
>>> leds-multi {
>>>  compatible = "gpio-multi-leds";
>>>
>>>  status {
>>>  gpios = <...>;
>>>  linux,default-trigger = "none";
>>>  deafult-state = "keep";
>>>
>>>  color-red {
>>>  pin-mask = <0x01>;
>>>  };
>>>
>>>  color-green {
>>>  pin-mask = <0x02>;
>>>  };
>>>
>>>  color-orange {
>>>  pin-mask = <0x03>;
>>>  };
>>>  };
>>> };
>>>
>>
>> Device Tree node implementation doesn't actually say too much
>> about the sysfs interface you came up with, which is most vital
>> in this case.
> 
> In our case it is very simple. There is "color" named sysfs node under
> gpio instance.
> 
> It creates own led instance for each children in this case it is named
> as "status" and then uses properties under it to define operation.
> 
> Currently we do have fixed list of color names in driver to require
> "standardized" names but that could be easily changed to be dynamic.
> 
> Driver registers all GPIOs defined in "gpios" under the instance.
> 
> So one can say:
> 
> echo "orange" > color
> 
> This setting goes to the driver, it figures out logical name "orange"
> and the configure all listed GPIO's to its "pin-mask" state. Polarity of
> the GPIO's is configured in GPIO definition so this is more or less turn
> this particular pin to activate state.
> 
> So in this case it changes led's color to orange and still lets one to
> use standard led triggers. Eg. set periodic blinking or so.
> 
> If one cat's "color" it lists all available colors for user and shows
> which one is active.
> 
> When brightness is configured as zero the all registered go to off state
> and when non-zero then it goes to state what ever is configured with
> "color" sysfs entry.
> 
>> Support for RGB LEDs, or other variations of LED synchronization
>> have been approached several times, but without satisfying result
>> so far.
>>
>> Generally the problem boils down to the issue of how triggers
>> should handle multiple synchronized LED class devices in a backwards
>> compatible way with monochrome LEDs.
>>
>> At some point the HSV [0] approach was proposed, but there was a problem
>> with getting uniform colors across devices. Especially white.
>> Certainly a calibration mechanism would be required.
>>
>> [0] https://lkml.org/lkml/2017/8/30/423
> 
> We do not usually use PWM controlled leds 

Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-10 Thread Vesa Jääskeläinen

Hi Jacek,

On 09/11/2018 22.42, Jacek Anaszewski wrote:

Hi Vesa,

On 11/09/2018 09:32 AM, Vesa Jääskeläinen wrote:

On 07/11/2018 0.07, Jacek Anaszewski wrote:

Introduce dedicated properties for conveying information about
LED function and color. Mark old "label" property as deprecated.

Signed-off-by: Jacek Anaszewski 
Cc: Baolin Wang 
Cc: Daniel Mack 
Cc: Dan Murphy 
Cc: Linus Walleij 
Cc: Oleh Kravchenko 
Cc: Sakari Ailus 
Cc: Simon Shields 
Cc: Xiaotong Lu 
---
   Documentation/devicetree/bindings/leds/common.txt | 52
+++
   1 file changed, 44 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/common.txt
b/Documentation/devicetree/bindings/leds/common.txt
index aa13998..3efc826 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -10,14 +10,20 @@ can influence the way of the LED device
initialization, the LED components
   have to be tightly coupled with the LED device binding. They are
represented
   by child nodes of the parent LED device binding.
   +
   Optional properties for child nodes:
   - led-sources : List of device current outputs the LED is connected
to. The
   outputs are identified by the numbers that must be defined
   in the LED device binding documentation.
+- function: LED functon. Use one of the LED_FUNCTION_* prefixed
definitions
+    from the header include/dt-bindings/leds/functions.h.
+    If there is no matching LED_FUNCTION available, add a new one.
+- color : Color of the LED.


We have had for years out-of-tree patch for multi color gpio led driver
which extends this concept with multiple colors. Then in sysfs there has
been possibility to control the color and otherwise use blinking or
other features.

Our need is multi color status led of the device which includes
different kind of blinkings and colors on different situations.

Current in-tree gpio led driver just wasn't atomic enough and a bit
clumsy interface for handling this.

Now that this is being looked at could we come up with solution that we
could define multiple colors for one led in device tree and then we
could work on getting the driver upstreamed?

What we did was generally:

leds-multi {
 compatible = "gpio-multi-leds";

 status {
     gpios = <...>;
     linux,default-trigger = "none";
     deafult-state = "keep";

     color-red {
     pin-mask = <0x01>;
     };

     color-green {
     pin-mask = <0x02>;
     };

     color-orange {
     pin-mask = <0x03>;
     };
 };
};



Device Tree node implementation doesn't actually say too much
about the sysfs interface you came up with, which is most vital
in this case.


In our case it is very simple. There is "color" named sysfs node under 
gpio instance.


It creates own led instance for each children in this case it is named 
as "status" and then uses properties under it to define operation.


Currently we do have fixed list of color names in driver to require 
"standardized" names but that could be easily changed to be dynamic.


Driver registers all GPIOs defined in "gpios" under the instance.

So one can say:

echo "orange" > color

This setting goes to the driver, it figures out logical name "orange" 
and the configure all listed GPIO's to its "pin-mask" state. Polarity of 
the GPIO's is configured in GPIO definition so this is more or less turn 
this particular pin to activate state.


So in this case it changes led's color to orange and still lets one to 
use standard led triggers. Eg. set periodic blinking or so.


If one cat's "color" it lists all available colors for user and shows 
which one is active.


When brightness is configured as zero the all registered go to off state 
and when non-zero then it goes to state what ever is configured with 
"color" sysfs entry.



Support for RGB LEDs, or other variations of LED synchronization
have been approached several times, but without satisfying result
so far.

Generally the problem boils down to the issue of how triggers
should handle multiple synchronized LED class devices in a backwards
compatible way with monochrome LEDs.

At some point the HSV [0] approach was proposed, but there was a problem
with getting uniform colors across devices. Especially white.
Certainly a calibration mechanism would be required.

[0] https://lkml.org/lkml/2017/8/30/423


We do not usually use PWM controlled leds so our calibration has been HW 
engineer tuning the resistors for the leds to get acceptable color for 
different colors variations.


If one wants to have fixed colors for such device then one could use 
this similar method to define them or/and free form interface to enter 
RGB values there. Thou with PWM RGB leds you probably want to have more 
animation there which probably would require some additional support code.


One way to do atomic PWM RGB color change with sysfs could be:

echo "21 

Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-10 Thread Vesa Jääskeläinen

Hi Jacek,

On 09/11/2018 22.42, Jacek Anaszewski wrote:

Hi Vesa,

On 11/09/2018 09:32 AM, Vesa Jääskeläinen wrote:

On 07/11/2018 0.07, Jacek Anaszewski wrote:

Introduce dedicated properties for conveying information about
LED function and color. Mark old "label" property as deprecated.

Signed-off-by: Jacek Anaszewski 
Cc: Baolin Wang 
Cc: Daniel Mack 
Cc: Dan Murphy 
Cc: Linus Walleij 
Cc: Oleh Kravchenko 
Cc: Sakari Ailus 
Cc: Simon Shields 
Cc: Xiaotong Lu 
---
   Documentation/devicetree/bindings/leds/common.txt | 52
+++
   1 file changed, 44 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/common.txt
b/Documentation/devicetree/bindings/leds/common.txt
index aa13998..3efc826 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -10,14 +10,20 @@ can influence the way of the LED device
initialization, the LED components
   have to be tightly coupled with the LED device binding. They are
represented
   by child nodes of the parent LED device binding.
   +
   Optional properties for child nodes:
   - led-sources : List of device current outputs the LED is connected
to. The
   outputs are identified by the numbers that must be defined
   in the LED device binding documentation.
+- function: LED functon. Use one of the LED_FUNCTION_* prefixed
definitions
+    from the header include/dt-bindings/leds/functions.h.
+    If there is no matching LED_FUNCTION available, add a new one.
+- color : Color of the LED.


We have had for years out-of-tree patch for multi color gpio led driver
which extends this concept with multiple colors. Then in sysfs there has
been possibility to control the color and otherwise use blinking or
other features.

Our need is multi color status led of the device which includes
different kind of blinkings and colors on different situations.

Current in-tree gpio led driver just wasn't atomic enough and a bit
clumsy interface for handling this.

Now that this is being looked at could we come up with solution that we
could define multiple colors for one led in device tree and then we
could work on getting the driver upstreamed?

What we did was generally:

leds-multi {
 compatible = "gpio-multi-leds";

 status {
     gpios = <...>;
     linux,default-trigger = "none";
     deafult-state = "keep";

     color-red {
     pin-mask = <0x01>;
     };

     color-green {
     pin-mask = <0x02>;
     };

     color-orange {
     pin-mask = <0x03>;
     };
 };
};



Device Tree node implementation doesn't actually say too much
about the sysfs interface you came up with, which is most vital
in this case.


In our case it is very simple. There is "color" named sysfs node under 
gpio instance.


It creates own led instance for each children in this case it is named 
as "status" and then uses properties under it to define operation.


Currently we do have fixed list of color names in driver to require 
"standardized" names but that could be easily changed to be dynamic.


Driver registers all GPIOs defined in "gpios" under the instance.

So one can say:

echo "orange" > color

This setting goes to the driver, it figures out logical name "orange" 
and the configure all listed GPIO's to its "pin-mask" state. Polarity of 
the GPIO's is configured in GPIO definition so this is more or less turn 
this particular pin to activate state.


So in this case it changes led's color to orange and still lets one to 
use standard led triggers. Eg. set periodic blinking or so.


If one cat's "color" it lists all available colors for user and shows 
which one is active.


When brightness is configured as zero the all registered go to off state 
and when non-zero then it goes to state what ever is configured with 
"color" sysfs entry.



Support for RGB LEDs, or other variations of LED synchronization
have been approached several times, but without satisfying result
so far.

Generally the problem boils down to the issue of how triggers
should handle multiple synchronized LED class devices in a backwards
compatible way with monochrome LEDs.

At some point the HSV [0] approach was proposed, but there was a problem
with getting uniform colors across devices. Especially white.
Certainly a calibration mechanism would be required.

[0] https://lkml.org/lkml/2017/8/30/423


We do not usually use PWM controlled leds so our calibration has been HW 
engineer tuning the resistors for the leds to get acceptable color for 
different colors variations.


If one wants to have fixed colors for such device then one could use 
this similar method to define them or/and free form interface to enter 
RGB values there. Thou with PWM RGB leds you probably want to have more 
animation there which probably would require some additional support code.


One way to do atomic PWM RGB color change with sysfs could be:

echo "21 

Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-09 Thread Jacek Anaszewski
Hi Vesa,

On 11/09/2018 09:32 AM, Vesa Jääskeläinen wrote:
> On 07/11/2018 0.07, Jacek Anaszewski wrote:
>> Introduce dedicated properties for conveying information about
>> LED function and color. Mark old "label" property as deprecated.
>>
>> Signed-off-by: Jacek Anaszewski 
>> Cc: Baolin Wang 
>> Cc: Daniel Mack 
>> Cc: Dan Murphy 
>> Cc: Linus Walleij 
>> Cc: Oleh Kravchenko 
>> Cc: Sakari Ailus 
>> Cc: Simon Shields 
>> Cc: Xiaotong Lu 
>> ---
>>   Documentation/devicetree/bindings/leds/common.txt | 52
>> +++
>>   1 file changed, 44 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/common.txt
>> b/Documentation/devicetree/bindings/leds/common.txt
>> index aa13998..3efc826 100644
>> --- a/Documentation/devicetree/bindings/leds/common.txt
>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>> @@ -10,14 +10,20 @@ can influence the way of the LED device
>> initialization, the LED components
>>   have to be tightly coupled with the LED device binding. They are
>> represented
>>   by child nodes of the parent LED device binding.
>>   +
>>   Optional properties for child nodes:
>>   - led-sources : List of device current outputs the LED is connected
>> to. The
>>   outputs are identified by the numbers that must be defined
>>   in the LED device binding documentation.
>> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed
>> definitions
>> +    from the header include/dt-bindings/leds/functions.h.
>> +    If there is no matching LED_FUNCTION available, add a new one.
>> +- color : Color of the LED.
> 
> We have had for years out-of-tree patch for multi color gpio led driver
> which extends this concept with multiple colors. Then in sysfs there has
> been possibility to control the color and otherwise use blinking or
> other features.
> 
> Our need is multi color status led of the device which includes
> different kind of blinkings and colors on different situations.
> 
> Current in-tree gpio led driver just wasn't atomic enough and a bit
> clumsy interface for handling this.
> 
> Now that this is being looked at could we come up with solution that we
> could define multiple colors for one led in device tree and then we
> could work on getting the driver upstreamed?
> 
> What we did was generally:
> 
> leds-multi {
> compatible = "gpio-multi-leds";
> 
> status {
>     gpios = <...>;
>     linux,default-trigger = "none";
>     deafult-state = "keep";
> 
>     color-red {
>     pin-mask = <0x01>;
>     };
> 
>     color-green {
>     pin-mask = <0x02>;
>     };
> 
>     color-orange {
>     pin-mask = <0x03>;
>     };
> };
> };
> 

Device Tree node implementation doesn't actually say too much
about the sysfs interface you came up with, which is most vital
in this case.

Support for RGB LEDs, or other variations of LED synchronization
have been approached several times, but without satisfying result
so far.

Generally the problem boils down to the issue of how triggers
should handle multiple synchronized LED class devices in a backwards
compatible way with monochrome LEDs.

At some point the HSV [0] approach was proposed, but there was a problem
with getting uniform colors across devices. Especially white.
Certainly a calibration mechanism would be required.

[0] https://lkml.org/lkml/2017/8/30/423

-- 
Best regards,
Jacek Anaszewski


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-09 Thread Jacek Anaszewski
Hi Vesa,

On 11/09/2018 09:32 AM, Vesa Jääskeläinen wrote:
> On 07/11/2018 0.07, Jacek Anaszewski wrote:
>> Introduce dedicated properties for conveying information about
>> LED function and color. Mark old "label" property as deprecated.
>>
>> Signed-off-by: Jacek Anaszewski 
>> Cc: Baolin Wang 
>> Cc: Daniel Mack 
>> Cc: Dan Murphy 
>> Cc: Linus Walleij 
>> Cc: Oleh Kravchenko 
>> Cc: Sakari Ailus 
>> Cc: Simon Shields 
>> Cc: Xiaotong Lu 
>> ---
>>   Documentation/devicetree/bindings/leds/common.txt | 52
>> +++
>>   1 file changed, 44 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/common.txt
>> b/Documentation/devicetree/bindings/leds/common.txt
>> index aa13998..3efc826 100644
>> --- a/Documentation/devicetree/bindings/leds/common.txt
>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>> @@ -10,14 +10,20 @@ can influence the way of the LED device
>> initialization, the LED components
>>   have to be tightly coupled with the LED device binding. They are
>> represented
>>   by child nodes of the parent LED device binding.
>>   +
>>   Optional properties for child nodes:
>>   - led-sources : List of device current outputs the LED is connected
>> to. The
>>   outputs are identified by the numbers that must be defined
>>   in the LED device binding documentation.
>> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed
>> definitions
>> +    from the header include/dt-bindings/leds/functions.h.
>> +    If there is no matching LED_FUNCTION available, add a new one.
>> +- color : Color of the LED.
> 
> We have had for years out-of-tree patch for multi color gpio led driver
> which extends this concept with multiple colors. Then in sysfs there has
> been possibility to control the color and otherwise use blinking or
> other features.
> 
> Our need is multi color status led of the device which includes
> different kind of blinkings and colors on different situations.
> 
> Current in-tree gpio led driver just wasn't atomic enough and a bit
> clumsy interface for handling this.
> 
> Now that this is being looked at could we come up with solution that we
> could define multiple colors for one led in device tree and then we
> could work on getting the driver upstreamed?
> 
> What we did was generally:
> 
> leds-multi {
> compatible = "gpio-multi-leds";
> 
> status {
>     gpios = <...>;
>     linux,default-trigger = "none";
>     deafult-state = "keep";
> 
>     color-red {
>     pin-mask = <0x01>;
>     };
> 
>     color-green {
>     pin-mask = <0x02>;
>     };
> 
>     color-orange {
>     pin-mask = <0x03>;
>     };
> };
> };
> 

Device Tree node implementation doesn't actually say too much
about the sysfs interface you came up with, which is most vital
in this case.

Support for RGB LEDs, or other variations of LED synchronization
have been approached several times, but without satisfying result
so far.

Generally the problem boils down to the issue of how triggers
should handle multiple synchronized LED class devices in a backwards
compatible way with monochrome LEDs.

At some point the HSV [0] approach was proposed, but there was a problem
with getting uniform colors across devices. Especially white.
Certainly a calibration mechanism would be required.

[0] https://lkml.org/lkml/2017/8/30/423

-- 
Best regards,
Jacek Anaszewski


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-09 Thread Jacek Anaszewski
Dan,

On 11/08/2018 10:08 PM, Dan Murphy wrote:
> Jacek
> 
> On 11/08/2018 02:47 PM, Jacek Anaszewski wrote:
>> Dan,
>>
>> On 11/08/2018 07:00 PM, Dan Murphy wrote:
>>> Jacek
>>>
>>> On 11/06/2018 04:07 PM, Jacek Anaszewski wrote:
 Introduce dedicated properties for conveying information about
 LED function and color. Mark old "label" property as deprecated.

 Signed-off-by: Jacek Anaszewski 
 Cc: Baolin Wang 
 Cc: Daniel Mack 
 Cc: Dan Murphy 
 Cc: Linus Walleij 
 Cc: Oleh Kravchenko 
 Cc: Sakari Ailus 
 Cc: Simon Shields 
 Cc: Xiaotong Lu 
 ---
  Documentation/devicetree/bindings/leds/common.txt | 52 
 +++
  1 file changed, 44 insertions(+), 8 deletions(-)

 diff --git a/Documentation/devicetree/bindings/leds/common.txt 
 b/Documentation/devicetree/bindings/leds/common.txt
 index aa13998..3efc826 100644
 --- a/Documentation/devicetree/bindings/leds/common.txt
 +++ b/Documentation/devicetree/bindings/leds/common.txt
 @@ -10,14 +10,20 @@ can influence the way of the LED device 
 initialization, the LED components
  have to be tightly coupled with the LED device binding. They are 
 represented
  by child nodes of the parent LED device binding.
  
 +
  Optional properties for child nodes:
  - led-sources : List of device current outputs the LED is connected to. 
 The
outputs are identified by the numbers that must be defined
in the LED device binding documentation.
 +- function: LED functon. Use one of the LED_FUNCTION_* prefixed 
 definitions
 +  from the header include/dt-bindings/leds/functions.h.
 +  If there is no matching LED_FUNCTION available, add a new one.
 +- color : Color of the LED.
>>>
>>> Should we define the colors too?  There are only really 4.  Red, green, 
>>> blue and white.
>>>
>>> Generally varying colors are created base on the primary colors.  Even the 
>>> amber color
>>
>> No problem, I can add LED colors. However, I don't quite follow how
>> the mix of base color strings would give "amber" ? :-)
>>
> 
> Amber or yellow.  Red and green with a hint of blue or no blue depending on 
> how dark you want it.
> 
> You don't always have to turn the LED on full to mix.  Sometimes lowering the 
> level of the stronger colors like red 
> and upping weaker colors like green you can different color

That's obvious. I was rather curious what you had on mind by mentioning
four colors (red, green, blue and white) in the context of "amber".
Just to remind: we're still talking about LED names, not the values.

-- 
Best regards,
Jacek Anaszewski


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-09 Thread Jacek Anaszewski
Dan,

On 11/08/2018 10:08 PM, Dan Murphy wrote:
> Jacek
> 
> On 11/08/2018 02:47 PM, Jacek Anaszewski wrote:
>> Dan,
>>
>> On 11/08/2018 07:00 PM, Dan Murphy wrote:
>>> Jacek
>>>
>>> On 11/06/2018 04:07 PM, Jacek Anaszewski wrote:
 Introduce dedicated properties for conveying information about
 LED function and color. Mark old "label" property as deprecated.

 Signed-off-by: Jacek Anaszewski 
 Cc: Baolin Wang 
 Cc: Daniel Mack 
 Cc: Dan Murphy 
 Cc: Linus Walleij 
 Cc: Oleh Kravchenko 
 Cc: Sakari Ailus 
 Cc: Simon Shields 
 Cc: Xiaotong Lu 
 ---
  Documentation/devicetree/bindings/leds/common.txt | 52 
 +++
  1 file changed, 44 insertions(+), 8 deletions(-)

 diff --git a/Documentation/devicetree/bindings/leds/common.txt 
 b/Documentation/devicetree/bindings/leds/common.txt
 index aa13998..3efc826 100644
 --- a/Documentation/devicetree/bindings/leds/common.txt
 +++ b/Documentation/devicetree/bindings/leds/common.txt
 @@ -10,14 +10,20 @@ can influence the way of the LED device 
 initialization, the LED components
  have to be tightly coupled with the LED device binding. They are 
 represented
  by child nodes of the parent LED device binding.
  
 +
  Optional properties for child nodes:
  - led-sources : List of device current outputs the LED is connected to. 
 The
outputs are identified by the numbers that must be defined
in the LED device binding documentation.
 +- function: LED functon. Use one of the LED_FUNCTION_* prefixed 
 definitions
 +  from the header include/dt-bindings/leds/functions.h.
 +  If there is no matching LED_FUNCTION available, add a new one.
 +- color : Color of the LED.
>>>
>>> Should we define the colors too?  There are only really 4.  Red, green, 
>>> blue and white.
>>>
>>> Generally varying colors are created base on the primary colors.  Even the 
>>> amber color
>>
>> No problem, I can add LED colors. However, I don't quite follow how
>> the mix of base color strings would give "amber" ? :-)
>>
> 
> Amber or yellow.  Red and green with a hint of blue or no blue depending on 
> how dark you want it.
> 
> You don't always have to turn the LED on full to mix.  Sometimes lowering the 
> level of the stronger colors like red 
> and upping weaker colors like green you can different color

That's obvious. I was rather curious what you had on mind by mentioning
four colors (red, green, blue and white) in the context of "amber".
Just to remind: we're still talking about LED names, not the values.

-- 
Best regards,
Jacek Anaszewski


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-09 Thread Vesa Jääskeläinen

On 07/11/2018 0.07, Jacek Anaszewski wrote:

Introduce dedicated properties for conveying information about
LED function and color. Mark old "label" property as deprecated.

Signed-off-by: Jacek Anaszewski 
Cc: Baolin Wang 
Cc: Daniel Mack 
Cc: Dan Murphy 
Cc: Linus Walleij 
Cc: Oleh Kravchenko 
Cc: Sakari Ailus 
Cc: Simon Shields 
Cc: Xiaotong Lu 
---
  Documentation/devicetree/bindings/leds/common.txt | 52 +++
  1 file changed, 44 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/common.txt 
b/Documentation/devicetree/bindings/leds/common.txt
index aa13998..3efc826 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -10,14 +10,20 @@ can influence the way of the LED device initialization, the 
LED components
  have to be tightly coupled with the LED device binding. They are represented
  by child nodes of the parent LED device binding.
  
+

  Optional properties for child nodes:
  - led-sources : List of device current outputs the LED is connected to. The
outputs are identified by the numbers that must be defined
in the LED device binding documentation.
+- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
+   from the header include/dt-bindings/leds/functions.h.
+   If there is no matching LED_FUNCTION available, add a new one.
+- color : Color of the LED.


We have had for years out-of-tree patch for multi color gpio led driver 
which extends this concept with multiple colors. Then in sysfs there has 
been possibility to control the color and otherwise use blinking or 
other features.


Our need is multi color status led of the device which includes 
different kind of blinkings and colors on different situations.


Current in-tree gpio led driver just wasn't atomic enough and a bit 
clumsy interface for handling this.


Now that this is being looked at could we come up with solution that we 
could define multiple colors for one led in device tree and then we 
could work on getting the driver upstreamed?


What we did was generally:

leds-multi {
compatible = "gpio-multi-leds";

status {
gpios = <...>;
linux,default-trigger = "none";
deafult-state = "keep";

color-red {
pin-mask = <0x01>;
};

color-green {
pin-mask = <0x02>;
};

color-orange {
pin-mask = <0x03>;
};
};
};

Thanks,
Vesa Jääskeläinen


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-09 Thread Vesa Jääskeläinen

On 07/11/2018 0.07, Jacek Anaszewski wrote:

Introduce dedicated properties for conveying information about
LED function and color. Mark old "label" property as deprecated.

Signed-off-by: Jacek Anaszewski 
Cc: Baolin Wang 
Cc: Daniel Mack 
Cc: Dan Murphy 
Cc: Linus Walleij 
Cc: Oleh Kravchenko 
Cc: Sakari Ailus 
Cc: Simon Shields 
Cc: Xiaotong Lu 
---
  Documentation/devicetree/bindings/leds/common.txt | 52 +++
  1 file changed, 44 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/common.txt 
b/Documentation/devicetree/bindings/leds/common.txt
index aa13998..3efc826 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -10,14 +10,20 @@ can influence the way of the LED device initialization, the 
LED components
  have to be tightly coupled with the LED device binding. They are represented
  by child nodes of the parent LED device binding.
  
+

  Optional properties for child nodes:
  - led-sources : List of device current outputs the LED is connected to. The
outputs are identified by the numbers that must be defined
in the LED device binding documentation.
+- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
+   from the header include/dt-bindings/leds/functions.h.
+   If there is no matching LED_FUNCTION available, add a new one.
+- color : Color of the LED.


We have had for years out-of-tree patch for multi color gpio led driver 
which extends this concept with multiple colors. Then in sysfs there has 
been possibility to control the color and otherwise use blinking or 
other features.


Our need is multi color status led of the device which includes 
different kind of blinkings and colors on different situations.


Current in-tree gpio led driver just wasn't atomic enough and a bit 
clumsy interface for handling this.


Now that this is being looked at could we come up with solution that we 
could define multiple colors for one led in device tree and then we 
could work on getting the driver upstreamed?


What we did was generally:

leds-multi {
compatible = "gpio-multi-leds";

status {
gpios = <...>;
linux,default-trigger = "none";
deafult-state = "keep";

color-red {
pin-mask = <0x01>;
};

color-green {
pin-mask = <0x02>;
};

color-orange {
pin-mask = <0x03>;
};
};
};

Thanks,
Vesa Jääskeläinen


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-08 Thread Dan Murphy
Jacek

On 11/08/2018 02:47 PM, Jacek Anaszewski wrote:
> Dan,
> 
> On 11/08/2018 07:00 PM, Dan Murphy wrote:
>> Jacek
>>
>> On 11/06/2018 04:07 PM, Jacek Anaszewski wrote:
>>> Introduce dedicated properties for conveying information about
>>> LED function and color. Mark old "label" property as deprecated.
>>>
>>> Signed-off-by: Jacek Anaszewski 
>>> Cc: Baolin Wang 
>>> Cc: Daniel Mack 
>>> Cc: Dan Murphy 
>>> Cc: Linus Walleij 
>>> Cc: Oleh Kravchenko 
>>> Cc: Sakari Ailus 
>>> Cc: Simon Shields 
>>> Cc: Xiaotong Lu 
>>> ---
>>>  Documentation/devicetree/bindings/leds/common.txt | 52 
>>> +++
>>>  1 file changed, 44 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/common.txt 
>>> b/Documentation/devicetree/bindings/leds/common.txt
>>> index aa13998..3efc826 100644
>>> --- a/Documentation/devicetree/bindings/leds/common.txt
>>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>>> @@ -10,14 +10,20 @@ can influence the way of the LED device initialization, 
>>> the LED components
>>>  have to be tightly coupled with the LED device binding. They are 
>>> represented
>>>  by child nodes of the parent LED device binding.
>>>  
>>> +
>>>  Optional properties for child nodes:
>>>  - led-sources : List of device current outputs the LED is connected to. The
>>> outputs are identified by the numbers that must be defined
>>> in the LED device binding documentation.
>>> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
>>> +   from the header include/dt-bindings/leds/functions.h.
>>> +   If there is no matching LED_FUNCTION available, add a new one.
>>> +- color : Color of the LED.
>>
>> Should we define the colors too?  There are only really 4.  Red, green, blue 
>> and white.
>>
>> Generally varying colors are created base on the primary colors.  Even the 
>> amber color
> 
> No problem, I can add LED colors. However, I don't quite follow how
> the mix of base color strings would give "amber" ? :-)
> 

Amber or yellow.  Red and green with a hint of blue or no blue depending on how 
dark you want it.

You don't always have to turn the LED on full to mix.  Sometimes lowering the 
level of the stronger colors like red 
and upping weaker colors like green you can different color

Dan

-- 
--
Dan Murphy


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-08 Thread Dan Murphy
Jacek

On 11/08/2018 02:47 PM, Jacek Anaszewski wrote:
> Dan,
> 
> On 11/08/2018 07:00 PM, Dan Murphy wrote:
>> Jacek
>>
>> On 11/06/2018 04:07 PM, Jacek Anaszewski wrote:
>>> Introduce dedicated properties for conveying information about
>>> LED function and color. Mark old "label" property as deprecated.
>>>
>>> Signed-off-by: Jacek Anaszewski 
>>> Cc: Baolin Wang 
>>> Cc: Daniel Mack 
>>> Cc: Dan Murphy 
>>> Cc: Linus Walleij 
>>> Cc: Oleh Kravchenko 
>>> Cc: Sakari Ailus 
>>> Cc: Simon Shields 
>>> Cc: Xiaotong Lu 
>>> ---
>>>  Documentation/devicetree/bindings/leds/common.txt | 52 
>>> +++
>>>  1 file changed, 44 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/common.txt 
>>> b/Documentation/devicetree/bindings/leds/common.txt
>>> index aa13998..3efc826 100644
>>> --- a/Documentation/devicetree/bindings/leds/common.txt
>>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>>> @@ -10,14 +10,20 @@ can influence the way of the LED device initialization, 
>>> the LED components
>>>  have to be tightly coupled with the LED device binding. They are 
>>> represented
>>>  by child nodes of the parent LED device binding.
>>>  
>>> +
>>>  Optional properties for child nodes:
>>>  - led-sources : List of device current outputs the LED is connected to. The
>>> outputs are identified by the numbers that must be defined
>>> in the LED device binding documentation.
>>> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
>>> +   from the header include/dt-bindings/leds/functions.h.
>>> +   If there is no matching LED_FUNCTION available, add a new one.
>>> +- color : Color of the LED.
>>
>> Should we define the colors too?  There are only really 4.  Red, green, blue 
>> and white.
>>
>> Generally varying colors are created base on the primary colors.  Even the 
>> amber color
> 
> No problem, I can add LED colors. However, I don't quite follow how
> the mix of base color strings would give "amber" ? :-)
> 

Amber or yellow.  Red and green with a hint of blue or no blue depending on how 
dark you want it.

You don't always have to turn the LED on full to mix.  Sometimes lowering the 
level of the stronger colors like red 
and upping weaker colors like green you can different color

Dan

-- 
--
Dan Murphy


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-08 Thread Jacek Anaszewski
Dan,

On 11/08/2018 07:00 PM, Dan Murphy wrote:
> Jacek
> 
> On 11/06/2018 04:07 PM, Jacek Anaszewski wrote:
>> Introduce dedicated properties for conveying information about
>> LED function and color. Mark old "label" property as deprecated.
>>
>> Signed-off-by: Jacek Anaszewski 
>> Cc: Baolin Wang 
>> Cc: Daniel Mack 
>> Cc: Dan Murphy 
>> Cc: Linus Walleij 
>> Cc: Oleh Kravchenko 
>> Cc: Sakari Ailus 
>> Cc: Simon Shields 
>> Cc: Xiaotong Lu 
>> ---
>>  Documentation/devicetree/bindings/leds/common.txt | 52 
>> +++
>>  1 file changed, 44 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/common.txt 
>> b/Documentation/devicetree/bindings/leds/common.txt
>> index aa13998..3efc826 100644
>> --- a/Documentation/devicetree/bindings/leds/common.txt
>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>> @@ -10,14 +10,20 @@ can influence the way of the LED device initialization, 
>> the LED components
>>  have to be tightly coupled with the LED device binding. They are represented
>>  by child nodes of the parent LED device binding.
>>  
>> +
>>  Optional properties for child nodes:
>>  - led-sources : List of device current outputs the LED is connected to. The
>>  outputs are identified by the numbers that must be defined
>>  in the LED device binding documentation.
>> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
>> +from the header include/dt-bindings/leds/functions.h.
>> +If there is no matching LED_FUNCTION available, add a new one.
>> +- color : Color of the LED.
> 
> Should we define the colors too?  There are only really 4.  Red, green, blue 
> and white.
> 
> Generally varying colors are created base on the primary colors.  Even the 
> amber color

No problem, I can add LED colors. However, I don't quite follow how
the mix of base color strings would give "amber" ? :-)

-- 
Best regards,
Jacek Anaszewski


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-08 Thread Jacek Anaszewski
Dan,

On 11/08/2018 07:00 PM, Dan Murphy wrote:
> Jacek
> 
> On 11/06/2018 04:07 PM, Jacek Anaszewski wrote:
>> Introduce dedicated properties for conveying information about
>> LED function and color. Mark old "label" property as deprecated.
>>
>> Signed-off-by: Jacek Anaszewski 
>> Cc: Baolin Wang 
>> Cc: Daniel Mack 
>> Cc: Dan Murphy 
>> Cc: Linus Walleij 
>> Cc: Oleh Kravchenko 
>> Cc: Sakari Ailus 
>> Cc: Simon Shields 
>> Cc: Xiaotong Lu 
>> ---
>>  Documentation/devicetree/bindings/leds/common.txt | 52 
>> +++
>>  1 file changed, 44 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/common.txt 
>> b/Documentation/devicetree/bindings/leds/common.txt
>> index aa13998..3efc826 100644
>> --- a/Documentation/devicetree/bindings/leds/common.txt
>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>> @@ -10,14 +10,20 @@ can influence the way of the LED device initialization, 
>> the LED components
>>  have to be tightly coupled with the LED device binding. They are represented
>>  by child nodes of the parent LED device binding.
>>  
>> +
>>  Optional properties for child nodes:
>>  - led-sources : List of device current outputs the LED is connected to. The
>>  outputs are identified by the numbers that must be defined
>>  in the LED device binding documentation.
>> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
>> +from the header include/dt-bindings/leds/functions.h.
>> +If there is no matching LED_FUNCTION available, add a new one.
>> +- color : Color of the LED.
> 
> Should we define the colors too?  There are only really 4.  Red, green, blue 
> and white.
> 
> Generally varying colors are created base on the primary colors.  Even the 
> amber color

No problem, I can add LED colors. However, I don't quite follow how
the mix of base color strings would give "amber" ? :-)

-- 
Best regards,
Jacek Anaszewski


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-08 Thread Dan Murphy
Jacek

On 11/06/2018 04:07 PM, Jacek Anaszewski wrote:
> Introduce dedicated properties for conveying information about
> LED function and color. Mark old "label" property as deprecated.
> 
> Signed-off-by: Jacek Anaszewski 
> Cc: Baolin Wang 
> Cc: Daniel Mack 
> Cc: Dan Murphy 
> Cc: Linus Walleij 
> Cc: Oleh Kravchenko 
> Cc: Sakari Ailus 
> Cc: Simon Shields 
> Cc: Xiaotong Lu 
> ---
>  Documentation/devicetree/bindings/leds/common.txt | 52 
> +++
>  1 file changed, 44 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/common.txt 
> b/Documentation/devicetree/bindings/leds/common.txt
> index aa13998..3efc826 100644
> --- a/Documentation/devicetree/bindings/leds/common.txt
> +++ b/Documentation/devicetree/bindings/leds/common.txt
> @@ -10,14 +10,20 @@ can influence the way of the LED device initialization, 
> the LED components
>  have to be tightly coupled with the LED device binding. They are represented
>  by child nodes of the parent LED device binding.
>  
> +
>  Optional properties for child nodes:
>  - led-sources : List of device current outputs the LED is connected to. The
>   outputs are identified by the numbers that must be defined
>   in the LED device binding documentation.
> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
> + from the header include/dt-bindings/leds/functions.h.
> + If there is no matching LED_FUNCTION available, add a new one.
> +- color : Color of the LED.

Should we define the colors too?  There are only really 4.  Red, green, blue 
and white.

Generally varying colors are created base on the primary colors.  Even the 
amber color

Dan

>  - label : The label for this LED. If omitted, the label is taken from the 
> node
> name (excluding the unit address). It has to uniquely identify
> a device, i.e. no other LED class device can be assigned the same
> -   label.
> +   label. This property is deprecated - use 'function' and 'color'
> +   properties instead.
>  
>  - default-state : The initial state of the LED. Valid values are "on", "off",
>and "keep". If the LED is already on or off and the default-state property 
> is
> @@ -87,29 +93,59 @@ Required properties for trigger source:
>  
>  * Examples
>  
> -gpio-leds {
> +#include 
> +
> +led-controller@0 {
>   compatible = "gpio-leds";
>  
> - system-status {
> - label = "Status";
> + led0 {
> + function = LED_FUNCTION_STATUS;
>   linux,default-trigger = "heartbeat";
>   gpios = < 0 GPIO_ACTIVE_HIGH>;
>   };
>  
> - usb {
> + led1 {
> + function = LED_FUNCTION_USB;
>   gpios = < 1 GPIO_ACTIVE_HIGH>;
>   trigger-sources = <_port1>, <_port1>;
>   };
>  };
>  
> -max77693-led {
> +led-controller@0 {
>   compatible = "maxim,max77693-led";
>  
> - camera-flash {
> - label = "Flash";
> + led {
> + function = LED_FUNCTION_FLASH;
> + color = "white";
>   led-sources = <0>, <1>;
>   led-max-microamp = <5>;
>   flash-max-microamp = <32>;
>   flash-max-timeout-us = <50>;
>   };
>  };
> +
> +led-controller@68 {
> + compatible = "ti,tlc59116";
> + reg = <0x68>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + led@0 {
> + function = LED_FUNCTION_WAN;
> + color = "amber";
> + reg = <0x0>;
> + };
> +
> + led@2 {
> + function = LED_FUNCTION_2G;
> + color = "white";
> + reg = <0x2>;
> + };
> +
> + led@9 {
> + function = LED_FUNCTION_ALIVE;
> + color = "green";
> + reg = <0x9>;
> + linux,default_trigger = "heartbeat";
> + };
> +};
> 


-- 
--
Dan Murphy


Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-08 Thread Dan Murphy
Jacek

On 11/06/2018 04:07 PM, Jacek Anaszewski wrote:
> Introduce dedicated properties for conveying information about
> LED function and color. Mark old "label" property as deprecated.
> 
> Signed-off-by: Jacek Anaszewski 
> Cc: Baolin Wang 
> Cc: Daniel Mack 
> Cc: Dan Murphy 
> Cc: Linus Walleij 
> Cc: Oleh Kravchenko 
> Cc: Sakari Ailus 
> Cc: Simon Shields 
> Cc: Xiaotong Lu 
> ---
>  Documentation/devicetree/bindings/leds/common.txt | 52 
> +++
>  1 file changed, 44 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/common.txt 
> b/Documentation/devicetree/bindings/leds/common.txt
> index aa13998..3efc826 100644
> --- a/Documentation/devicetree/bindings/leds/common.txt
> +++ b/Documentation/devicetree/bindings/leds/common.txt
> @@ -10,14 +10,20 @@ can influence the way of the LED device initialization, 
> the LED components
>  have to be tightly coupled with the LED device binding. They are represented
>  by child nodes of the parent LED device binding.
>  
> +
>  Optional properties for child nodes:
>  - led-sources : List of device current outputs the LED is connected to. The
>   outputs are identified by the numbers that must be defined
>   in the LED device binding documentation.
> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
> + from the header include/dt-bindings/leds/functions.h.
> + If there is no matching LED_FUNCTION available, add a new one.
> +- color : Color of the LED.

Should we define the colors too?  There are only really 4.  Red, green, blue 
and white.

Generally varying colors are created base on the primary colors.  Even the 
amber color

Dan

>  - label : The label for this LED. If omitted, the label is taken from the 
> node
> name (excluding the unit address). It has to uniquely identify
> a device, i.e. no other LED class device can be assigned the same
> -   label.
> +   label. This property is deprecated - use 'function' and 'color'
> +   properties instead.
>  
>  - default-state : The initial state of the LED. Valid values are "on", "off",
>and "keep". If the LED is already on or off and the default-state property 
> is
> @@ -87,29 +93,59 @@ Required properties for trigger source:
>  
>  * Examples
>  
> -gpio-leds {
> +#include 
> +
> +led-controller@0 {
>   compatible = "gpio-leds";
>  
> - system-status {
> - label = "Status";
> + led0 {
> + function = LED_FUNCTION_STATUS;
>   linux,default-trigger = "heartbeat";
>   gpios = < 0 GPIO_ACTIVE_HIGH>;
>   };
>  
> - usb {
> + led1 {
> + function = LED_FUNCTION_USB;
>   gpios = < 1 GPIO_ACTIVE_HIGH>;
>   trigger-sources = <_port1>, <_port1>;
>   };
>  };
>  
> -max77693-led {
> +led-controller@0 {
>   compatible = "maxim,max77693-led";
>  
> - camera-flash {
> - label = "Flash";
> + led {
> + function = LED_FUNCTION_FLASH;
> + color = "white";
>   led-sources = <0>, <1>;
>   led-max-microamp = <5>;
>   flash-max-microamp = <32>;
>   flash-max-timeout-us = <50>;
>   };
>  };
> +
> +led-controller@68 {
> + compatible = "ti,tlc59116";
> + reg = <0x68>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + led@0 {
> + function = LED_FUNCTION_WAN;
> + color = "amber";
> + reg = <0x0>;
> + };
> +
> + led@2 {
> + function = LED_FUNCTION_2G;
> + color = "white";
> + reg = <0x2>;
> + };
> +
> + led@9 {
> + function = LED_FUNCTION_ALIVE;
> + color = "green";
> + reg = <0x9>;
> + linux,default_trigger = "heartbeat";
> + };
> +};
> 


-- 
--
Dan Murphy


[PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-06 Thread Jacek Anaszewski
Introduce dedicated properties for conveying information about
LED function and color. Mark old "label" property as deprecated.

Signed-off-by: Jacek Anaszewski 
Cc: Baolin Wang 
Cc: Daniel Mack 
Cc: Dan Murphy 
Cc: Linus Walleij 
Cc: Oleh Kravchenko 
Cc: Sakari Ailus 
Cc: Simon Shields 
Cc: Xiaotong Lu 
---
 Documentation/devicetree/bindings/leds/common.txt | 52 +++
 1 file changed, 44 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/common.txt 
b/Documentation/devicetree/bindings/leds/common.txt
index aa13998..3efc826 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -10,14 +10,20 @@ can influence the way of the LED device initialization, the 
LED components
 have to be tightly coupled with the LED device binding. They are represented
 by child nodes of the parent LED device binding.
 
+
 Optional properties for child nodes:
 - led-sources : List of device current outputs the LED is connected to. The
outputs are identified by the numbers that must be defined
in the LED device binding documentation.
+- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
+   from the header include/dt-bindings/leds/functions.h.
+   If there is no matching LED_FUNCTION available, add a new one.
+- color : Color of the LED.
 - label : The label for this LED. If omitted, the label is taken from the node
  name (excluding the unit address). It has to uniquely identify
  a device, i.e. no other LED class device can be assigned the same
- label.
+ label. This property is deprecated - use 'function' and 'color'
+ properties instead.
 
 - default-state : The initial state of the LED. Valid values are "on", "off",
   and "keep". If the LED is already on or off and the default-state property is
@@ -87,29 +93,59 @@ Required properties for trigger source:
 
 * Examples
 
-gpio-leds {
+#include 
+
+led-controller@0 {
compatible = "gpio-leds";
 
-   system-status {
-   label = "Status";
+   led0 {
+   function = LED_FUNCTION_STATUS;
linux,default-trigger = "heartbeat";
gpios = < 0 GPIO_ACTIVE_HIGH>;
};
 
-   usb {
+   led1 {
+   function = LED_FUNCTION_USB;
gpios = < 1 GPIO_ACTIVE_HIGH>;
trigger-sources = <_port1>, <_port1>;
};
 };
 
-max77693-led {
+led-controller@0 {
compatible = "maxim,max77693-led";
 
-   camera-flash {
-   label = "Flash";
+   led {
+   function = LED_FUNCTION_FLASH;
+   color = "white";
led-sources = <0>, <1>;
led-max-microamp = <5>;
flash-max-microamp = <32>;
flash-max-timeout-us = <50>;
};
 };
+
+led-controller@68 {
+   compatible = "ti,tlc59116";
+   reg = <0x68>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   led@0 {
+   function = LED_FUNCTION_WAN;
+   color = "amber";
+   reg = <0x0>;
+   };
+
+   led@2 {
+   function = LED_FUNCTION_2G;
+   color = "white";
+   reg = <0x2>;
+   };
+
+   led@9 {
+   function = LED_FUNCTION_ALIVE;
+   color = "green";
+   reg = <0x9>;
+   linux,default_trigger = "heartbeat";
+   };
+};
-- 
2.1.4



[PATCH 04/24] dt-bindings: leds: Add function and color properties

2018-11-06 Thread Jacek Anaszewski
Introduce dedicated properties for conveying information about
LED function and color. Mark old "label" property as deprecated.

Signed-off-by: Jacek Anaszewski 
Cc: Baolin Wang 
Cc: Daniel Mack 
Cc: Dan Murphy 
Cc: Linus Walleij 
Cc: Oleh Kravchenko 
Cc: Sakari Ailus 
Cc: Simon Shields 
Cc: Xiaotong Lu 
---
 Documentation/devicetree/bindings/leds/common.txt | 52 +++
 1 file changed, 44 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/common.txt 
b/Documentation/devicetree/bindings/leds/common.txt
index aa13998..3efc826 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -10,14 +10,20 @@ can influence the way of the LED device initialization, the 
LED components
 have to be tightly coupled with the LED device binding. They are represented
 by child nodes of the parent LED device binding.
 
+
 Optional properties for child nodes:
 - led-sources : List of device current outputs the LED is connected to. The
outputs are identified by the numbers that must be defined
in the LED device binding documentation.
+- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
+   from the header include/dt-bindings/leds/functions.h.
+   If there is no matching LED_FUNCTION available, add a new one.
+- color : Color of the LED.
 - label : The label for this LED. If omitted, the label is taken from the node
  name (excluding the unit address). It has to uniquely identify
  a device, i.e. no other LED class device can be assigned the same
- label.
+ label. This property is deprecated - use 'function' and 'color'
+ properties instead.
 
 - default-state : The initial state of the LED. Valid values are "on", "off",
   and "keep". If the LED is already on or off and the default-state property is
@@ -87,29 +93,59 @@ Required properties for trigger source:
 
 * Examples
 
-gpio-leds {
+#include 
+
+led-controller@0 {
compatible = "gpio-leds";
 
-   system-status {
-   label = "Status";
+   led0 {
+   function = LED_FUNCTION_STATUS;
linux,default-trigger = "heartbeat";
gpios = < 0 GPIO_ACTIVE_HIGH>;
};
 
-   usb {
+   led1 {
+   function = LED_FUNCTION_USB;
gpios = < 1 GPIO_ACTIVE_HIGH>;
trigger-sources = <_port1>, <_port1>;
};
 };
 
-max77693-led {
+led-controller@0 {
compatible = "maxim,max77693-led";
 
-   camera-flash {
-   label = "Flash";
+   led {
+   function = LED_FUNCTION_FLASH;
+   color = "white";
led-sources = <0>, <1>;
led-max-microamp = <5>;
flash-max-microamp = <32>;
flash-max-timeout-us = <50>;
};
 };
+
+led-controller@68 {
+   compatible = "ti,tlc59116";
+   reg = <0x68>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   led@0 {
+   function = LED_FUNCTION_WAN;
+   color = "amber";
+   reg = <0x0>;
+   };
+
+   led@2 {
+   function = LED_FUNCTION_2G;
+   color = "white";
+   reg = <0x2>;
+   };
+
+   led@9 {
+   function = LED_FUNCTION_ALIVE;
+   color = "green";
+   reg = <0x9>;
+   linux,default_trigger = "heartbeat";
+   };
+};
-- 
2.1.4