Re: [PATCH 04/24] dt-bindings: leds: Add function and color properties
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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