Multicolor Framework update
Subsystems (3)
v3 cover Multicolor Framework update
Hello
In this series I have done quite a bit of rework and since there was not to many review comments we can pretty much treat this as unreviewed.
I have removed the brightness-model definition from this series as it is an enhancement to the multi color framework but is not required in the initial submission. And this seems to be the biggest source of contention in the patch set. So the idea is to get the base MC FW in place and let developers play with it.
I have converted the LP5523 to use the MC framework and will update a couple more drivers once I have the test harnesses to verify the implementation.
The biggest change to this series is that the parent brightness file now controls all the color LEDs brightness within the directory.
I have written a script that writes brightness values to the colors and then proceeds to write the parent brightness from 0->255 and then back to 0. Basically pulsing the LED group. I found that the color is uniform across the brightness ranges.
Dan
Dan Murphy (9):
leds: multicolor: Add sysfs interface definition
dt: bindings: Add multicolor class dt bindings documention
documention: leds: Add multicolor class documentation
dt-bindings: leds: Add multicolor ID to the color ID list
leds: Add multicolor ID to the color ID list
leds: multicolor: Introduce a multicolor class definition
dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers
leds: lp50xx: Add the LP50XX family of the RGB LED driver
leds: Update the lp55xx to use the multi color framework
.../ABI/testing/sysfs-class-led-multicolor | 57 ++ .../bindings/leds/leds-class-multicolor.txt | 97 +++ .../devicetree/bindings/leds/leds-lp50xx.txt | 142 ++++ Documentation/leds/leds-class-multicolor.txt | 99 +++ drivers/leds/Kconfig | 17 + drivers/leds/Makefile | 2 + drivers/leds/led-class-multicolor.c | 421 +++++++++++ drivers/leds/led-core.c | 1 + drivers/leds/leds-lp50xx.c | 714 ++++++++++++++++++ drivers/leds/leds-lp5523.c | 13 + drivers/leds/leds-lp55xx-common.c | 133 +++- drivers/leds/leds-lp55xx-common.h | 10 + include/dt-bindings/leds/common.h | 3 +- include/linux/led-class-multicolor.h | 95 +++ include/linux/platform_data/leds-lp55xx.h | 5 + 15 files changed, 1789 insertions(+), 20 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-class-led-multicolor create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.txt create mode 100644 Documentation/devicetree/bindings/leds/leds-lp50xx.txt create mode 100644 Documentation/leds/leds-class-multicolor.txt create mode 100644 drivers/leds/led-class-multicolor.c create mode 100644 drivers/leds/leds-lp50xx.c create mode 100644 include/linux/led-class-multicolor.h
Signature · 1 line
2.21.0.5.gaeb582a983
PATCH v3 1/9 leds: multicolor: Add sysfs interface definition
Add a documentation of LED Multicolor LED class specific sysfs attributes.
--- .../ABI/testing/sysfs-class-led-multicolor | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-led-multicolor
Documentation/ABI/testing/sysfs-class-led-multicolor new file +57 -0
@@ -0,0 +1,57 @@ +What: /sys/class/leds/<led>/colors/sync_enable +Date: April 2019 +KernelVersion: 5.2 +Contact: Dan Murphy <[email protected]> +Description: read/write + Writing a 1 to this file will enable the synchronization of all + the defined color LEDs within the LED node. Brightness values + for each LED will be stored and written when sync is set to 1. + Writing a 0 to this file will disable syncing and allow + individual control of the LEDs brightness settings. + +What: /sys/class/leds/<led>/colors/sync +Date: April 2019 +KernelVersion: 5.2 +Contact: Dan Murphy <[email protected]> +Description: write only + Writing a 1 to this file while sync_enable is set to 1 will + write the current brightness values to all defined LEDs within + the LED node. All LEDs defined will be configured based + on the brightness that has been requested. + + If sync_enable is set to 0 then writing a 1 to sync has no + affect on the LEDs. + +What: /sys/class/leds/<led>/colors/<led_color>/brightness +Date: April 2019 +KernelVersion: 5.2 +Contact: Dan Murphy <[email protected]> +Description: read/write + The led_color directory is dynamically created based on the + colors defined by the registrar of the class. + The led_color can be but not limited to red, green, blue, + white, amber, yellow and violet. Drivers can also declare a + LED color for presentation. There is one directory per color + presented. The brightness file is created under each + led_color directory and controls the individual LED color + setting. + + If sync is enabled then writing the brightness value of the LED + is deferred until a 1 is written to + /sys/class/leds/<led>/color/sync. If syncing is + disabled then the LED brightness value will be written + immediately to the LED driver. + + The value of the color is from 0 to + /sys/class/leds/<led>/colors/<led_color>/max_brightness. + +What: /sys/class/leds/<led>/colors/<led_color>/max_brightness +Date: April 2019 +KernelVersion: 5.2 +Contact: Dan Murphy <[email protected]> +Description: read only + Maximum brightness level for the LED color, default is + 255 (LED_FULL). + + If the LED does not support different brightness levels, this + should be 1.
Signature · 1 line
2.21.0.5.gaeb582a983
Re: [PATCH v3 1/9] leds: multicolor: Add sysfs interface definition
Quoted 15 lines · Dan Murphy
Add a documentation of LED Multicolor LED class specific sysfs attributes.
Signed-off-by Dan Murphy <[email protected]>--- .../ABI/testing/sysfs-class-led-multicolor | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-led-multicolorDocumentation/ABI/testing/sysfs-class-led-multicolor new file +0
-0
Documentation/ABI/testing/sysfs-class-led-multicolor line 2
Quoted 2 lines
Documentation/ABI/testing/sysfs-class-led-multicolor new file +2
-0@@ -0,0 +1,57 @@ +What: /sys/class/leds/<led>/colors/sync_enable +Date: April 2019
I believe I suggested more reasonable interface. Why not use that?
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH v3 1/9] leds: multicolor: Add sysfs interface definition
Pavel
Quoted 10 lines · Pavel Machek
Quoted 9 lines · Dan Murphy
Add a documentation of LED Multicolor LED class specific sysfs attributes.
Signed-off-by Dan Murphy <[email protected]>--- .../ABI/testing/sysfs-class-led-multicolor | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+)create mode 100644
Documentation/ABI/testing/sysfs-class-led-multicolor
Quoted 10 lines
Documentation/ABI/testing/sysfs-class-led-multicolor line 2
Quoted 8 lines
Documentation/ABI/testing/sysfs-class-led-multicolor new file +2
-0@@ -0,0 +1,57 @@ +What: /sys/class/leds/<led>/colors/sync_enable +Date: April 2019I believe I suggested more reasonable interface. Why not use that?
Can you please provide the reference to your interface?
These patchsets have been around for a while (Decemeber 2018) and I cannot seem to find the reference to your suggestion.
The suggestion may have been mired in the brightness model discussions.
So I don't want to over look what you suggested as it may be more reasonable then what I have implemented.
Dan
Re: [PATCH v3 1/9] leds: multicolor: Add sysfs interface definition
Pavel
Quoted 25 lines · Dan Murphy
Pavel
Quoted 11 lines · Pavel Machek
Quoted 10 lines · Dan Murphy
Add a documentation of LED Multicolor LED class specific sysfs attributes.
Signed-off-by Dan Murphy <[email protected]>---
.../ABI/testing/sysfs-class-led-multicolor | 57
+++++++++++++++++++1 file changed, 57 insertions(+)create mode 100644
Documentation/ABI/testing/sysfs-class-led-multicolorQuoted 10 lines
Documentation/ABI/testing/sysfs-class-led-multicolor line 2
Quoted 8 lines
Documentation/ABI/testing/sysfs-class-led-multicolor new file +2
-0@@ -0,0 +1,57 @@ +What: /sys/class/leds/<led>/colors/sync_enable +Date: April 2019I believe I suggested more reasonable interface. Why not use that?
Quoted 2 lines
Can you please provide the reference to your interface?
I think I found the suggestion [0]. Assuming that was the suggestion it violates the kernel 1 value/file and there was agreement that this interface had value. In testing the interface, it made sense to be able to setup a color after writing each color brightness file and then syncing the color LEDs with the new brightness levels. It also provides flexibility to be able to set a single LED color without writing sync. The decision to do either or should really be a user space decision. If I have referenced the wrong thread please let me know which thread you are referring to.
[0] https://lore.kernel.org/patchwork/patch/1057044/
<snip>
Dan
Re: [PATCH v3 1/9] leds: multicolor: Add sysfs interface definition
Quoted 30 lines · Dan Murphy
Pavel
Quoted 27 lines · Dan Murphy
Pavel
Quoted 23 lines · Pavel Machek
Quoted 18 lines · Dan Murphy
Add a documentation of LED Multicolor LED class specific sysfs attributes.
Signed-off-by Dan Murphy <[email protected]>---
.../ABI/testing/sysfs-class-led-multicolor | 57
+++++++++++++++++++1 file changed, 57 insertions(+)create mode 100644
Documentation/ABI/testing/sysfs-class-led-multicolorDocumentation/ABI/testing/sysfs-class-led-multicolor new file +0
-0Documentation/ABI/testing/sysfs-class-led-multicolor line 2
Quoted 2 lines
Documentation/ABI/testing/sysfs-class-led-multicolor new file +2
-0@@ -0,0 +1,57 @@ +What: /sys/class/leds/<led>/colors/sync_enable +Date: April 2019I believe I suggested more reasonable interface. Why not use that?
Quoted 5 lines
Quoted 2 lines
Can you please provide the reference to your interface?
I think I found the suggestion [0]. Assuming that was the suggestion it violates the kernel 1 value/file and there was agreement that this interface had value. In testing the interface, it made sense to be able to
1 value/file is actually slightly more complex rule:
Attributes should be ASCII text files, preferably with only one value per file. It is noted that it may not be efficient to contain only one value per file, so it is socially acceptable to express an array of values of the same type.
See sysfs.txt. Proposed "sync_enable" is ugly enough, and the values really are array of values of same type, so we should be ok with nicer interface.
Best regards,
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH v3 1/9] leds: multicolor: Add sysfs interface definition
Pavel
Quoted 37 lines · Pavel Machek
Quoted 29 lines · Dan Murphy
Pavel
Quoted 26 lines · Dan Murphy
Pavel
Quoted 23 lines · Pavel Machek
Quoted 18 lines · Dan Murphy
Add a documentation of LED Multicolor LED class specific sysfs attributes.
Signed-off-by Dan Murphy <[email protected]>--- .../ABI/testing/sysfs-class-led-multicolor | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+)create mode 100644
Documentation/ABI/testing/sysfs-class-led-multicolorDocumentation/ABI/testing/sysfs-class-led-multicolor new file +0
-0Documentation/ABI/testing/sysfs-class-led-multicolor line 2
Quoted 2 lines
Documentation/ABI/testing/sysfs-class-led-multicolor new file +2
-0@@ -0,0 +1,57 @@ +What: /sys/class/leds/<led>/colors/sync_enable +Date: April 2019I believe I suggested more reasonable interface. Why not use that?
Quoted 5 lines
Quoted 2 lines
Can you please provide the reference to your interface?
I think I found the suggestion [0]. Assuming that was the suggestion it violates the kernel 1 value/file and there was agreement that this interface had value. In testing the interface, it made sense to be able to
1 value/file is actually slightly more complex rule:
Quoted 8 lines
Attributes should be ASCII text files, preferably with only one value per file. It is noted that it may not be efficient to contain only one value per file, so it is socially acceptable to express an array of values of the same type.
See sysfs.txt. Proposed "
sync_enable" is ugly enough, and the values really are array of values of same type, so we should be ok with nicer interface.
sync_enable is a binary input 0 or 1, but I could change that to ASCII "enabled/disabled". And the sole purpose of this file is to enable or disable the synchronization of the grouped monochrome color LED brightness settings within that given directory. So not sure why the sync_enable is ugly since it is meant to enable a feature of the class. Honestly I could drop the whole sync feature but I have found value in this interface through my testing both with a pulsing script I wrote and the notint.py you wrote. This feature is meant for slower processors or processes with low priority that could be interrupted or switched out during writing the LED and cause LED color flicker. Priming the colors and then updating all the available LEDs in the group in the kernel space with a single write to the sync file is a little more intuitive. I could change both the sync and sync_enable to module_params instead.
After mulling over the proposed solution of passing in the array of values I came to the conclusion this is actually more complicated than it needs to be and this implementation is not flexible.
There would need to be additional files that need to present information to the user space on what LEDs available, how many LEDs are available and what the LED array order is. The user interface cannot always assume that the array is RGB.
There could be RGBW, RGBA, RGBWA, GBW or WIR or any combination of varying LED colors. The interface that is proposed would need to have a varying array as the input.
I have posted v4 of the documentation of this patchset with updated sysfs documentation and ABI description.
Dan
Quoted 3 lines
Best regards,
Pavel
Re: [PATCH v3 1/9] leds: multicolor: Add sysfs interface definition
In reply to Dan MurphyHi Dan,
Thank you for the update.
One thing is missing here - we need to document how legacy brightness levels map to the sub-LED color levels, i.e. what you do in multicolor_set_brightness().
Best regards,
Jacek Anaszewski
Quoted 73 lines · Dan Murphy
Add a documentation of LED Multicolor LED class specific sysfs attributes.
Signed-off-by Dan Murphy <[email protected]>--- .../ABI/testing/sysfs-class-led-multicolor | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+)create mode 100644
Documentation/ABI/testing/sysfs-class-led-multicolorDocumentation/ABI/testing/sysfs-class-led-multicolor new file +57
-0@@ -0,0 +1,57 @@ +What: /sys/class/leds/<led>/colors/sync_enable +Date: April 2019 +KernelVersion: 5.2 +Contact: Dan Murphy <[email protected]> +Description: read/write + Writing a 1 to this file will enable the synchronization of all + the defined color LEDs within the LED node. Brightness values + for each LED will be stored and written when sync is set to 1. + Writing a 0 to this file will disable syncing and allow + individual control of the LEDs brightness settings. + +What: /sys/class/leds/<led>/colors/sync +Date: April 2019 +KernelVersion: 5.2 +Contact: Dan Murphy <[email protected]> +Description: write only + Writing a 1 to this file while sync_enable is set to 1 will + write the current brightness values to all defined LEDs within + the LED node. All LEDs defined will be configured based + on the brightness that has been requested. + + If sync_enable is set to 0 then writing a 1 to sync has no + affect on the LEDs. + +What: /sys/class/leds/<led>/colors/<led_color>/brightness +Date: April 2019 +KernelVersion: 5.2 +Contact: Dan Murphy <[email protected]> +Description: read/write + The led_color directory is dynamically created based on the + colors defined by the registrar of the class. + The led_color can be but not limited to red, green, blue, + white, amber, yellow and violet. Drivers can also declare a + LED color for presentation. There is one directory per color + presented. The brightness file is created under each + led_color directory and controls the individual LED color + setting. + + If sync is enabled then writing the brightness value of the LED + is deferred until a 1 is written to + /sys/class/leds/<led>/color/sync. If syncing is + disabled then the LED brightness value will be written + immediately to the LED driver. + + The value of the color is from 0 to + /sys/class/leds/<led>/colors/<led_color>/max_brightness. + +What: /sys/class/leds/<led>/colors/<led_color>/max_brightness +Date: April 2019 +KernelVersion: 5.2 +Contact: Dan Murphy <[email protected]> +Description: read only + Maximum brightness level for the LED color, default is + 255 (LED_FULL). + + If the LED does not support different brightness levels, this + should be 1.
Re: [PATCH v3 1/9] leds: multicolor: Add sysfs interface definition
Jacek
Quoted 7 lines · Jacek Anaszewski
Hi Dan,
Thank you for the update.
One thing is missing here - we need to document how legacy brightness levels map to the sub-LED color levels, i.e. what you do in
multicolor_set_brightness().
Ok so i will need to document the algorithm that is used to determine the color LED brightness.
Dan
Quoted 4 lines
Best regards,
Jacek Anaszewski
<snip>
Quoted 1 line
Re: [PATCH v3 1/9] leds: multicolor: Add sysfs interface definition
Dan,
Quoted 5 lines · Dan Murphy
Jacek
Quoted 2 lines · Jacek Anaszewski
Hi Dan,
Quoted 8 lines
Quoted 5 lines
Thank you for the update.
One thing is missing here - we need to document how legacy brightness levels map to the sub-LED color levels, i.e. what you do in
multicolor_set_brightness().Ok so i will need to document the algorithm that is used to determine the color LED brightness.
Right, and please send just an update of that single patch.
Signature · 2 lines
Best regards, Jacek Anaszewski
Re: [PATCH v3 1/9] leds: multicolor: Add sysfs interface definition
Jacek
Quoted 10 lines · Jacek Anaszewski
Dan,
Quoted 7 lines · Dan Murphy
Jacek
Quoted 4 lines · Jacek Anaszewski
Hi Dan,
Thank you for the update.
Quoted 9 lines
Quoted 6 lines
Quoted 3 lines
One thing is missing here - we need to document how legacy brightness levels map to the sub-LED color levels, i.e. what you do in
multicolor_set_brightness().Ok so i will need to document the algorithm that is used to determine the color LED brightness.
Right, and please send just an update of that single patch.
So you are asking for v4 with only this patch updated.
Dan
Re: [PATCH v3 1/9] leds: multicolor: Add sysfs interface definition
Quoted 18 lines · Dan Murphy
Jacek
Quoted 15 lines · Jacek Anaszewski
Dan,
Quoted 12 lines · Dan Murphy
Jacek
Quoted 7 lines · Jacek Anaszewski
Hi Dan,
Thank you for the update.
One thing is missing here - we need to document how legacy brightness levels map to the sub-LED color levels, i.e. what you do in
multicolor_set_brightness().
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Ok so i will need to document the algorithm that is used to determine the color LED brightness.
Right, and please send just an update of that single patch.
So you are asking for v4 with only this patch updated.
Not exactly - I am asking for v4 of this patch. We don't need to spam the lists with the rest of unaltered patches from the series.
The interface seems to be the most vital part of this patch set, and it is possible that it will undergo at least slight modifications.
We will move to discussing the code once we achieve a total consensus.
Signature · 2 lines
Best regards, Jacek Anaszewski
Re: [PATCH v3 1/9] leds: multicolor: Add sysfs interface definition
Jacek
Quoted 26 lines · Jacek Anaszewski
Quoted 18 lines · Dan Murphy
Jacek
Quoted 15 lines · Jacek Anaszewski
Dan,
Quoted 12 lines · Dan Murphy
Jacek
Quoted 7 lines · Jacek Anaszewski
Hi Dan,
Thank you for the update.
One thing is missing here - we need to document how legacy brightness levels map to the sub-LED color levels, i.e. what you do in
multicolor_set_brightness().Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Ok so i will need to document the algorithm that is used to determine the color LED brightness.
Right, and please send just an update of that single patch.
So you are asking for v4 with only this patch updated.
Quoted 8 lines
Not exactly - I am asking for v4 of this patch. We don't need to spam the lists with the rest of unaltered patches from the series.
The interface seems to be the most vital part of this patch set, and it is possible that it will undergo at least slight modifications.
We will move to discussing the code once we achieve a total consensus.
Sorry for the late reply. OK I have updated the sysfs documentation but in doing so I am also going to send in v4 of the sysfs documentation as it explains the interfaces in more detail. Also I will be adding more technical information into that sysfs doc on what is done and what to expect and it's usage.
Dan
PATCH v3 2/9 dt: bindings: Add multicolor class dt bindings documention
In reply to Dan MurphyAdd DT bindings for the LEDs multicolor class framework.
--- .../bindings/leds/leds-class-multicolor.txt | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.txt
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +97 -0
@@ -0,0 +1,97 @@ +* Multicolor LED properties + +Multicolor LEDs can consist of a RGB, RGBW or a RGBA LED clusters. These devices +can be grouped together and also provide a modeling mechanism so that the +cluster LEDs can vary in hue and intensity to produce a wide range of colors. + +The nodes and properties defined in this document are unique to the multicolor +LED class. Common LED nodes and properties are inherited from the common.txt +within this documentation directory. + +Required LED Child properties: + - color : For multicolor LED support this property should be defined as + LED_COLOR_ID_MULTI and further definition can be found in + include/linux/leds/common.h. + +led-controller@30 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,lp5024"; + reg = <0x29>; + + multi-led@4 { + #address-cells = <1>; + #size-cells = <0>; + reg = <4>; + color = <LED_COLOR_ID_MULTI>; + function = LED_FUNCTION_ACTIVITY; + + led@12 { + reg = <12>; + color = <LED_COLOR_ID_RED>; + }; + + led@13 { + reg = <13>; + color = <LED_COLOR_ID_GREEN>; + }; + + led@14 { + reg = <14>; + color = <LED_COLOR_ID_BLUE>; + }; + }; + + /* Only support RGB no model defined */ + multi-led@1 { + #address-cells = <1>; + #size-cells = <0>; + reg = <1>; + color = <LED_COLOR_ID_MULTI>; + function = LED_FUNCTION_; + + + led@3 { + reg = <3>; + color = <LED_COLOR_ID_RED>; + }; + + led@4 { + reg = <4>; + color = <LED_COLOR_ID_GREEN>; + }; + + led@5 { + reg = <5>; + color = <LED_COLOR_ID_BLUE>; + }; + }; + + multi-led@2 { + #address-cells = <1>; + #size-cells = <0>; + color = <LED_COLOR_ID_MULTI>; + function = LED_FUNCTION_ACTIVITY; + reg = <2>; + ti,led-bank = <2 3 5>; + + led@6 { + reg = <0x6>; + color = <LED_COLOR_ID_RED>; + led-sources = <6 9 15>; + }; + + led@7 { + reg = <0x7>; + color = <LED_COLOR_ID_GREEN>; + led-sources = <7 10 16>; + }; + + led@8 { + reg = <0x8>; + color = <LED_COLOR_ID_BLUE>; + led-sources = <8 11 17>; + }; + }; + +};
Signature · 1 line
2.21.0.5.gaeb582a983
Rob Herring
maintainerRe: [PATCH v3 2/9] dt: bindings: Add multicolor class dt bindings documention
Quoted 26 lines · Dan Murphy
Add DT bindings for the LEDs multicolor class framework.
Signed-off-by Dan Murphy <[email protected]>--- .../bindings/leds/leds-class-multicolor.txt | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.txtDocumentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +12
-0@@ -0,0 +1,97 @@ +* Multicolor LED properties + +Multicolor LEDs can consist of a RGB, RGBW or a RGBA LED clusters. These devices +can be grouped together and also provide a modeling mechanism so that the +cluster LEDs can vary in hue and intensity to produce a wide range of colors. + +The nodes and properties defined in this document are unique to the multicolor +LED class. Common LED nodes and properties are inherited from the common.txt +within this documentation directory. + +Required LED Child properties: + - color : For multicolor LED support this property should be defined as
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt line 22
Quoted 10 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +10
-0+ LED_COLOR_ID_MULTI and further definition can be found in + include/linux/leds/common.h. + +led-controller@30 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,lp5024"; + reg = <0x29>; + + multi-led@4 {
Typically we sort by address order.
Quoted 13 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +13
-0+ #address-cells = <1>; + #size-cells = <0>; + reg = <4>; + color = <LED_COLOR_ID_MULTI>; + function = LED_FUNCTION_ACTIVITY; + + led@12 { + reg = <12>; + color = <LED_COLOR_ID_RED>; + }; + + led@13 { + reg = <13>;
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt line 45
Quoted 10 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +10
-0+ color = <LED_COLOR_ID_GREEN>; + }; + + led@14 { + reg = <14>; + color = <LED_COLOR_ID_BLUE>; + }; + }; + + /* Only support RGB no model defined */
I don't understand this comment.
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt line 51
Quoted 6 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +6
-0+ multi-led@1 { + #address-cells = <1>; + #size-cells = <0>; + reg = <1>; + color = <LED_COLOR_ID_MULTI>; + function = LED_FUNCTION_;
??
Quoted 49 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +46
-0+ + + led@3 { + reg = <3>; + color = <LED_COLOR_ID_RED>; + }; + + led@4 { + reg = <4>; + color = <LED_COLOR_ID_GREEN>; + }; + + led@5 { + reg = <5>; + color = <LED_COLOR_ID_BLUE>; + }; + }; + + multi-led@2 { + #address-cells = <1>; + #size-cells = <0>; + color = <LED_COLOR_ID_MULTI>; + function = LED_FUNCTION_ACTIVITY; + reg = <2>; + ti,led-bank = <2 3 5>; + + led@6 { + reg = <0x6>; + color = <LED_COLOR_ID_RED>; + led-sources = <6 9 15>; + }; + + led@7 { + reg = <0x7>; + color = <LED_COLOR_ID_GREEN>; + led-sources = <7 10 16>; + }; + + led@8 { + reg = <0x8>; + color = <LED_COLOR_ID_BLUE>; + led-sources = <8 11 17>; + }; + }; + +};Signature · 1 line
2.21.0.5.gaeb582a983
Re: [PATCH v3 2/9] dt: bindings: Add multicolor class dt bindings documention
Rob
Thanks for the review
Quoted 30 lines · Rob Herring
Quoted 29 lines · Dan Murphy
Add DT bindings for the LEDs multicolor class framework.
Signed-off-by Dan Murphy <[email protected]>--- .../bindings/leds/leds-class-multicolor.txt | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+)create mode 100644
Documentation/devicetree/bindings/leds/leds-class-multicolor.txtDocumentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +15
-0@@ -0,0 +1,97 @@ +* Multicolor LED properties + +Multicolor LEDs can consist of a RGB, RGBW or a RGBA LED clusters. These devices +can be grouped together and also provide a modeling mechanism so that the +cluster LEDs can vary in hue and intensity to produce a wide range of colors. + +The nodes and properties defined in this document are unique to the multicolor +LED class. Common LED nodes and properties are inherited from the common.txt +within this documentation directory. + +Required LED Child properties: + - color : For multicolor LED support this property should be defined as + LED_COLOR_ID_MULTI and further definition can be found in + include/linux/leds/common.h. +
Quoted 8 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt line 22
Quoted 7 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +7
-0+led-controller@30 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,lp5024"; + reg = <0x29>; + + multi-led@4 {Typically we sort by address order.
These are not addresses these end up being the "module" number that the LEDs below are associated to.
Quoted 17 lines
Quoted 17 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +16
-0+ #address-cells = <1>; + #size-cells = <0>; + reg = <4>; + color = <LED_COLOR_ID_MULTI>; + function = LED_FUNCTION_ACTIVITY; + + led@12 { + reg = <12>; + color = <LED_COLOR_ID_RED>; + }; + + led@13 { + reg = <13>; + color = <LED_COLOR_ID_GREEN>; + }; +
Quoted 8 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt line 45
Quoted 7 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +7
-0+ led@14 { + reg = <14>; + color = <LED_COLOR_ID_BLUE>; + }; + }; + + /* Only support RGB no model defined */I don't understand this comment.
Artifact can be removed
Quoted 8 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt line 51
Quoted 7 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +6
-0+ multi-led@1 { + #address-cells = <1>; + #size-cells = <0>; + reg = <1>; + color = <LED_COLOR_ID_MULTI>; + function = LED_FUNCTION_;??
I meant to change that but missed it in the example.
Dan
Quoted 50 lines
Quoted 50 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +46
-0+ + + led@3 { + reg = <3>; + color = <LED_COLOR_ID_RED>; + }; + + led@4 { + reg = <4>; + color = <LED_COLOR_ID_GREEN>; + }; + + led@5 { + reg = <5>; + color = <LED_COLOR_ID_BLUE>; + }; + }; + + multi-led@2 { + #address-cells = <1>; + #size-cells = <0>; + color = <LED_COLOR_ID_MULTI>; + function = LED_FUNCTION_ACTIVITY; + reg = <2>; + ti,led-bank = <2 3 5>; + + led@6 { + reg = <0x6>; + color = <LED_COLOR_ID_RED>; + led-sources = <6 9 15>; + }; + + led@7 { + reg = <0x7>; + color = <LED_COLOR_ID_GREEN>; + led-sources = <7 10 16>; + }; + + led@8 { + reg = <0x8>; + color = <LED_COLOR_ID_BLUE>; + led-sources = <8 11 17>; + }; + }; + +};Signature · 1 line
2.21.0.5.gaeb582a983
Rob Herring
maintainerRe: [PATCH v3 2/9] dt: bindings: Add multicolor class dt bindings documention
Quoted 45 lines · Dan Murphy
Rob
Thanks for the review
Quoted 30 lines · Rob Herring
Quoted 29 lines · Dan Murphy
Add DT bindings for the LEDs multicolor class framework.
Signed-off-by Dan Murphy <[email protected]>--- .../bindings/leds/leds-class-multicolor.txt | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+)create mode 100644
Documentation/devicetree/bindings/leds/leds-class-multicolor.txtDocumentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +15
-0@@ -0,0 +1,97 @@ +* Multicolor LED properties + +Multicolor LEDs can consist of a RGB, RGBW or a RGBA LED clusters. These devices +can be grouped together and also provide a modeling mechanism so that the +cluster LEDs can vary in hue and intensity to produce a wide range of colors. + +The nodes and properties defined in this document are unique to the multicolor +LED class. Common LED nodes and properties are inherited from the common.txt +within this documentation directory. + +Required LED Child properties: + - color : For multicolor LED support this property should be defined as + LED_COLOR_ID_MULTI and further definition can be found in + include/linux/leds/common.h. +Quoted 8 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt line 22
Quoted 7 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +7
-0+led-controller@30 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,lp5024"; + reg = <0x29>; + + multi-led@4 {Typically we sort by address order.
Quoted 2 lines
These are not addresses these end up being the "module" number that the LEDs below are associated to.
'reg' (and the unit-address) is an address in the sense that is how you identify a device or sub-device. It doesn't matter what type of 'address' it is, DT practice is to sort node in unit-address numerical order.
'module' is a h/w thing, right? A bank or instance within the device? If not, using 'reg' here is not appropriate.
Rob
Re: [PATCH v3 2/9] dt: bindings: Add multicolor class dt bindings documention
Quoted 49 lines · Rob Herring
Quoted 45 lines · Dan Murphy
Rob
Thanks for the review
Quoted 30 lines · Rob Herring
Quoted 29 lines · Dan Murphy
Add DT bindings for the LEDs multicolor class framework.
Signed-off-by Dan Murphy <[email protected]>--- .../bindings/leds/leds-class-multicolor.txt | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+)create mode 100644
Documentation/devicetree/bindings/leds/leds-class-multicolor.txtDocumentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +15
-0@@ -0,0 +1,97 @@ +* Multicolor LED properties + +Multicolor LEDs can consist of a RGB, RGBW or a RGBA LED clusters. These devices +can be grouped together and also provide a modeling mechanism so that the +cluster LEDs can vary in hue and intensity to produce a wide range of colors. + +The nodes and properties defined in this document are unique to the multicolor +LED class. Common LED nodes and properties are inherited from the common.txt +within this documentation directory. + +Required LED Child properties: + - color : For multicolor LED support this property should be defined as + LED_COLOR_ID_MULTI and further definition can be found in + include/linux/leds/common.h. +Quoted 8 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt line 22
Quoted 7 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +7
-0+led-controller@30 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,lp5024"; + reg = <0x29>; + + multi-led@4 {Typically we sort by address order.
Quoted 2 lines
These are not addresses these end up being the "module" number that the LEDs below are associated to.
Quoted 7 lines
'reg' (and the unit-address) is an address in the sense that is how you identify a device or sub-device. It doesn't matter what type of 'address' it is, DT practice is to sort node in unit-address numerical order.
'module' is a h/w thing, right? A bank or instance within the device? If not, using 'reg' here is not appropriate.
In this case reg represents LEDn_BRIGHTNESS register which controls a group of three LEDs. The thing is that those registers' addresses start from 0x07, i.e. the formula for calculating the RGB LED module address is: LEDn_BRIGHTNESS = 0x07 + n.
From the above it seems that we should have multi-led@7 and reg = 0x07 for LED0_BRIGHTNESS register governing the brightness of RGB LED module 0, right?
And regarding sorting by address order I think that Rob was asking for placing whole multi-led@4 sub-node after multi-led@2 (here sticking to the numeration from the patch).
Signature · 2 lines
Best regards, Jacek Anaszewski
Rob Herring
maintainerRe: [PATCH v3 2/9] dt: bindings: Add multicolor class dt bindings documention
Quoted 59 lines · Jacek Anaszewski
Quoted 49 lines · Rob Herring
Quoted 45 lines · Dan Murphy
Rob
Thanks for the review
Quoted 30 lines · Rob Herring
Quoted 29 lines · Dan Murphy
Add DT bindings for the LEDs multicolor class framework.
Signed-off-by Dan Murphy <[email protected]>--- .../bindings/leds/leds-class-multicolor.txt | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+)create mode 100644
Documentation/devicetree/bindings/leds/leds-class-multicolor.txtDocumentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +15
-0@@ -0,0 +1,97 @@ +* Multicolor LED properties + +Multicolor LEDs can consist of a RGB, RGBW or a RGBA LED clusters. These devices +can be grouped together and also provide a modeling mechanism so that the +cluster LEDs can vary in hue and intensity to produce a wide range of colors. + +The nodes and properties defined in this document are unique to the multicolor +LED class. Common LED nodes and properties are inherited from the common.txt +within this documentation directory. + +Required LED Child properties: + - color : For multicolor LED support this property should be defined as + LED_COLOR_ID_MULTI and further definition can be found in + include/linux/leds/common.h. +Quoted 8 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt line 22
Quoted 7 lines
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt new file +7
-0+led-controller@30 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,lp5024"; + reg = <0x29>; + + multi-led@4 {Typically we sort by address order.
Quoted 2 lines
These are not addresses these end up being the "module" number that the LEDs below are associated to.
Quoted 7 lines
'reg' (and the unit-address) is an address in the sense that is how you identify a device or sub-device. It doesn't matter what type of 'address' it is, DT practice is to sort node in unit-address numerical order.
'module' is a h/w thing, right? A bank or instance within the device? If not, using 'reg' here is not appropriate.
Quoted 8 lines
In this case reg represents
LEDn_BRIGHTNESSregister which controls a group of three LEDs. The thing is that those registers' addresses start from0x07, i.e. the formula for calculating the RGB LED module address is:LEDn_BRIGHTNESS=0x07+ n.From the above it seems that we should have multi-led@7 and reg =
0x07
forLED0_BRIGHTNESSregister governing the brightness of RGB LED module 0, right?
Use whatever makes the most sense from a h/w perspective. If 'module N' is something that I'd read about in the datasheet, then I'd stick with 'N'.
Quoted 4 lines
And regarding sorting by address order I think that Rob was asking for placing whole multi-led@4 sub-node after multi-led@2 (here sticking to the numeration from the patch).
Right.
Rob
PATCH v3 3/9 documention: leds: Add multicolor class documentation
In reply to Dan MurphyAdd the support documentation on the multicolor LED framework. This document defines the directores and file generated by the multicolor framework. It also documents usage.
--- Documentation/leds/leds-class-multicolor.txt | 99 ++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 Documentation/leds/leds-class-multicolor.txt
Documentation/leds/leds-class-multicolor.txt new file +99 -0
@@ -0,0 +1,99 @@ + +Multi Color LED handling under Linux +================================================= + +Author: Dan Murphy <[email protected]> + +Description +----------- +There are varying monochrome LED colors available for application. These +LEDs can be used as a single use case LED or can be mixed with other color +LEDs to produce the full spectrum of color. Color LEDs that are grouped +can be presented under a single LED node with individual color control. +The multicolor class groups these LEDs and allows dynamically setting the value +of a single LED or setting the brightness values of the LEDs in the group and +updating the LEDs virtually simultaneously. + +Multicolor Class Control +------------------------- +The multicolor class presents the LED groups under a directory called "colors". +This directory is a child under the LED parent node created by the led_class +framework. The led_class framework is documented in led-class.txt within this +documentation directory. + +Each colored LED is given its own directory. These directories can be, but not +limited to red, green, blue, white, amber, yellow and violet. Under these +directories the brightness and max_brightness files are presented for each LED. + +Under the "colors" directory there are two files created: "sync" and +"sync_enable". The sync_enable file controls whether the LED brightness +value is set real time or if the LED brightness value setting is deferred until +the "sync" file is written. If sync_enable is set then writing to each LED +"brightness" file will store the brightness value. Once the "sync" file is +written then each LED color defined in the node will write the brightness of +the LED in the device driver. + +If "sync_enable" is not set then writing the brightness value of the LED to the +device driver is done immediately. Writing the "sync" file has no affect. + +Directory Layout Example +------------------------ +root:/sys/class/leds/rgb:grouped_leds# ls -lR colors/ +colors/: +drwxr-xr-x 2 root root 0 Jun 28 20:21 blue +drwxr-xr-x 2 root root 0 Jun 28 20:21 green +drwxr-xr-x 2 root root 0 Jun 28 20:21 red +--w------- 1 root root 4096 Jun 28 20:21 sync +-rw------- 1 root root 4096 Jun 28 20:22 sync_enable + +colors/blue: +-rw------- 1 root root 4096 Jun 28 20:21 brightness +-r-------- 1 root root 4096 Jun 28 20:27 max_brightness + +colors/green: +-rw------- 1 root root 4096 Jun 28 20:22 brightness +-r-------- 1 root root 4096 Jun 28 20:27 max_brightness + +colors/red: +-rw------- 1 root root 4096 Jun 28 20:21 brightness +-r-------- 1 root root 4096 Jun 28 20:27 max_brightness + +Example of Writing LEDs with Sync Enabled +----------------------------------------- +Below the red, green and blue LEDs are set to corresponding values. These +values are stored and not written until the sync file is written. + +cd /sys/class/leds/rgb:grouped_leds/colors + +echo 1 > sync_enable + +echo 100 > red/brightness +echo 80 > green/brightness +echo 180 > blue/brightness + +* LED device driver has not been updated and the LED states have not changed. +* Writing the LED brightness files again will only change the stored value and +* not the device driver value. + +echo 1 > sync + +* LED device driver has been updated; the LEDs should present the brightness +* levels that have been set. Since sync_enable is still enabled writing to the +* LED brightness files will not change the current brightnesses. + +Example of Writing LEDs with Sync Disabled +------------------------------------------ +Below the values of each LED are written to the device driver immediately upon +request. + +cd /sys/class/leds/rgb:grouped_leds/colors + +echo 0 > sync_enable + +echo 100 > red/brightness // Red LED should be on with the current brightness +echo 80 > green/brightness // Green LED should be on with the current brightness +echo 180 > blue/brightness // Blue LED should be on with the current brightness +. +. +. +echo 0 > green/brightness // Green LED should be off
Signature · 1 line
2.21.0.5.gaeb582a983
PATCH v3 4/9 dt-bindings: leds: Add multicolor ID to the color ID list
In reply to Dan MurphyAdd a new color ID that is declared as MULTICOLOR as with the multicolor framework declaring a definitive color is not accurate as the node can contain multiple colors.
--- include/dt-bindings/leds/common.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
include/dt-bindings/leds/common.h +2 -1
@@ -65,6 +65,7 @@#define LED_COLOR_ID_VIOLET 5#define LED_COLOR_ID_YELLOW 6#define LED_COLOR_ID_IR 7 -#define LED_COLOR_ID_COUNT 8 +#define LED_COLOR_ID_MULTI 8 +#define LED_COLOR_ID_COUNT 9#endif /* __DT_BINDINGS_LEDS_H */
Signature · 1 line
2.21.0.5.gaeb582a983
Re: [PATCH v3 4/9] dt-bindings: leds: Add multicolor ID to the color ID list
Quoted 9 lines · Dan Murphy
Add a new color ID that is declared as MULTICOLOR as with the multicolor framework declaring a definitive color is not accurate as the node can contain multiple colors.
Signed-off-by Dan Murphy <[email protected]>--- include/dt-bindings/leds/common.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
PATCH v3 5/9 leds: Add multicolor ID to the color ID list
In reply to Dan MurphyAdd a new color ID that is declared as MULTICOLOR as with the multicolor framework declaring a definitive color is not accurate as the node can contain multiple colors.
--- drivers/leds/led-core.c | 1 + 1 file changed, 1 insertion(+)
drivers/leds/led-core.c +1 -0
@@ -38,6 +38,7 @@ const char *led_colors[LED_COLOR_ID_COUNT] = { [LED_COLOR_ID_VIOLET] = "violet", [LED_COLOR_ID_YELLOW] = "yellow", [LED_COLOR_ID_IR] = "ir", + [LED_COLOR_ID_MULTI ] ="multicolor" , };EXPORT_SYMBOL_GPL (led_colors);
Signature · 1 line
2.21.0.5.gaeb582a983
PATCH v3 6/9 leds: multicolor: Introduce a multicolor class definition
In reply to Dan MurphyIntroduce a multicolor class that groups colored LEDs within a LED node.
The framework allows for dynamically setting individual LEDs or setting brightness levels of LEDs and updating them virtually simultaneously.
--- drivers/leds/Kconfig | 10 + drivers/leds/Makefile | 1 + drivers/leds/led-class-multicolor.c | 421 +++++++++++++++++++++++++++ include/linux/led-class-multicolor.h | 95 ++++++ 4 files changed, 527 insertions(+) create mode 100644 drivers/leds/led-class-multicolor.c create mode 100644 include/linux/led-class-multicolor.h
drivers/leds/Kconfig +10 -0
@@ -29,6 +29,16 @@ config LEDS_CLASS_FLASH for the flash related features of a LED device. It can be built as a module. +config LEDS_CLASS_MULTI_COLOR + tristate "LED Mulit Color LED Class Support" + depends on LEDS_CLASS + help + This option enables the multicolor LED sysfs class in /sys/class/leds. + It wraps LED Class and adds multicolor LED specific sysfs attributes + and kernel internal API to it. You'll need this to provide support + for multicolor LEDs that are grouped together. This class is not + intended for single color LEDs. It can be built as a module. + config LEDS_BRIGHTNESS_HW_CHANGED bool "LED Class brightness_hw_changed attribute support" depends on LEDS_CLASS
drivers/leds/Makefile +1 -0
@@ -4,6 +4,7 @@ obj-$(CONFIG_NEW_LEDS) += led-core.o obj-$(CONFIG_LEDS_CLASS) += led-class.o obj-$(CONFIG_LEDS_CLASS_FLASH) += led-class-flash.o +obj-$(CONFIG_LEDS_CLASS_MULTI_COLOR) += led-class-multicolor.o obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o # LED Platform Drivers
drivers/leds/led-class-multicolor.c new file +421 -0
421 lines not on this page · raw message
include/linux/led-class-multicolor.h new file +95 -0
95 lines not on this page · raw message
Signature · 1 line
2.21.0.5.gaeb582a983
Re: [PATCH v3 6/9] leds: multicolor: Introduce a multicolor class definition
Hi Dan,
Thank you for the v5.
I will confine myself to commenting only some parts since the rest will undergo rework due to removal of sync API.
Quoted 24 lines · Dan Murphy
Introduce a multicolor class that groups colored LEDs within a LED node.
The framework allows for dynamically setting individual LEDs or setting brightness levels of LEDs and updating them virtually simultaneously.
Signed-off-by Dan Murphy <[email protected]>--- drivers/leds/Kconfig | 10 + drivers/leds/Makefile | 1 + drivers/leds/led-class-multicolor.c | 421 +++++++++++++++++++++++++++ include/linux/led-class-multicolor.h | 95 ++++++ 4 files changed, 527 insertions(+)create mode 100644
drivers/leds/led-class-multicolor.ccreate mode 100644include/linux/led-class-multicolor.hdrivers/leds/Kconfig +0
-0@@ -29,6 +29,16 @@ config LEDS_CLASS_FLASH for the flash related features of a LED device. It can be built as a module.
drivers/leds/Kconfig config LEDS_CLASS_FLASH · line 40
Quoted 10 lines
drivers/leds/Kconfig +9
-0+config LEDS_CLASS_MULTI_COLOR + tristate "LED Mulit Color LED Class Support" + depends on LEDS_CLASS + help + This option enables the multicolor LED sysfs class in /sys/class/leds. + It wraps LED Class and adds multicolor LED specific sysfs attributes + and kernel internal API to it. You'll need this to provide support + for multicolor LEDs that are grouped together. This class is not + intended for single color LEDs. It can be built as a module.
extra whitespace:
s/ It can/It can/
[...]
Quoted 13 lines
drivers/leds/Kconfig +13
-0+ +static int multicolor_set_brightness(struct led_classdev *led_cdev, + enum led_brightness brightness) +{ + struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev); + struct led_classdev_mc_data *data = mcled_cdev->data; + struct led_multicolor_ops *ops = mcled_cdev->ops; + struct led_classdev_mc_priv *priv; + unsigned long state = brightness; + int adj_value; + ssize_t ret = -EINVAL; + + mutex_lock(&led_cdev->led_access);
drivers/leds/Kconfig config LEDS_CLASS_FLASH
Quoted 10 lines
drivers/leds/Kconfig +10
-0+ + if (ops->set_module_brightness) { + ret = ops->set_module_brightness(mcled_cdev, state); + goto unlock; + } + + list_for_each_entry(priv, &data->color_list, list) { + if (state && priv->brightness && priv->max_brightness) { + adj_value = state * ((priv->brightness * 100) / priv->max_brightness); + adj_value = adj_value / 100;
Why the multiplication an then division by 100? And priv->max_brightness stays unaltered? This changes the proportions. My python script works just fine without those.
Quoted 13 lines
drivers/leds/Kconfig +13
-0+ } else + adj_value = LED_OFF; + + ret = ops->set_color_brightness(priv->mcled_cdev, + priv->color_id, adj_value); + if (ret < 0) + goto unlock; + } + +unlock: + mutex_unlock(&led_cdev->led_access); + return ret; +}
[...]
Quoted 16 lines
drivers/leds/Kconfig +16
-0+int led_classdev_multicolor_register_ext(struct device *parent, + struct led_classdev_mc *mcled_cdev, + struct led_init_data *init_data) +{ + struct led_classdev *led_cdev; + struct led_multicolor_ops *ops; + struct led_classdev_mc_data *data; + int ret; + int i; + + if (!mcled_cdev) + return -EINVAL; + + ops = mcled_cdev->ops; + if (!ops || !ops->set_color_brightness) + return -EINVAL;
drivers/leds/Kconfig
Quoted 10 lines
drivers/leds/Kconfig +10
-0+ + data = kzalloc(sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + mcled_cdev->data = data; + led_cdev = &mcled_cdev->led_cdev; + + if (led_cdev->brightness_set_blocking) + led_cdev->brightness_set_blocking = multicolor_set_brightness;
This is weird. In leds-lp50xx.c you don't initialize brightness_set_blocking and this still works?
I believe this is kind of omission.
And it is not reasonable to just override driver supplied op with generic one just like that.
I propose to initialize brightness_set or brightness_set_blocking op as we used to do it for monochrome LEDs. Those function(s) on driver side will either use device's hardware support for setting color lightness, or will call a generic function provided by LED multi color class for calculating intensities of all colors it comprises in the cluster.
I know this is different to what we've discussed on IRC, but now it looks for me the most reasonable way to go.
Quoted 102 lines
drivers/leds/Kconfig +96
-0+ INIT_LIST_HEAD(&data->color_list); + + /* Register led class device */ + ret = led_classdev_register_ext(parent, led_cdev, init_data); + if (ret) + return ret; + + ret = led_multicolor_init_color_dir(data, mcled_cdev); + if (ret) + return ret; + + /* Select the sysfs attributes to be created for the device */ + for (i = 0; i < mcled_cdev->num_leds; i++) { + ret = led_multicolor_init_color(data, mcled_cdev, + mcled_cdev->available_colors[i]); + if (ret) + break; + } + + return ret; +} +EXPORT_SYMBOL_GPL(led_classdev_multicolor_register_ext); + +void led_classdev_multicolor_unregister(struct led_classdev_mc *mcled_cdev) +{ + if (!mcled_cdev) + return; + + led_classdev_unregister(&mcled_cdev->led_cdev); +} +EXPORT_SYMBOL_GPL(led_classdev_multicolor_unregister); + +static void devm_led_classdev_multicolor_release(struct device *dev, void *res) +{ + led_classdev_multicolor_unregister(*(struct led_classdev_mc **)res); +} + +/** + * devm_of_led_classdev_register - resource managed led_classdev_register() + * + * @parent: parent of LED device + * @led_cdev: the led_classdev structure for this device. + */ +int devm_led_classdev_multicolor_register(struct device *parent, + struct led_classdev_mc *mcled_cdev) +{ + struct led_classdev_mc **dr; + int ret; + + dr = devres_alloc(devm_led_classdev_multicolor_release, + sizeof(*dr), GFP_KERNEL); + if (!dr) + return -ENOMEM; + + ret = led_classdev_multicolor_register(parent, mcled_cdev); + if (ret) { + devres_free(dr); + return ret; + } + + *dr = mcled_cdev; + devres_add(parent, dr); + + return 0; +} +EXPORT_SYMBOL_GPL(devm_led_classdev_multicolor_register); + +static int devm_led_classdev_multicolor_match(struct device *dev, + void *res, void *data) +{ + struct mcled_cdev **p = res; + + if (WARN_ON(!p || !*p)) + return 0; + + return *p == data; +} + +/** + * devm_led_classdev_multicolor_unregister() - resource managed + * led_classdev_multicolor_unregister() + * @parent: The device to unregister. + * @mcled_cdev: the led_classdev_mc structure for this device. + */ +void devm_led_classdev_multicolor_unregister(struct device *dev, + struct led_classdev_mc *mcled_cdev) +{ + WARN_ON(devres_release(dev, + devm_led_classdev_multicolor_release, + devm_led_classdev_multicolor_match, mcled_cdev)); +} +EXPORT_SYMBOL_GPL(devm_led_classdev_multicolor_unregister); + +MODULE_AUTHOR("Dan Murphy <[email protected]>"); +MODULE_DESCRIPTION("Multi Color LED class interface"); +MODULE_LICENSE("GPL v2");include/linux/led-class-multicolor.h new file +0
-0
include/linux/led-class-multicolor.h line 5
Quoted 5 lines
include/linux/led-class-multicolor.h new file +5
-0@@ -0,0 +1,95 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* LED Multicolor class interface +* Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ +*/
Let's have C++ comment style also here.
Quoted 5 lines
include/linux/led-class-multicolor.h new file +5
-0+#ifndef __LINUX_MULTICOLOR_LEDS_H_INCLUDED +#define __LINUX_MULTICOLOR_LEDS_H_INCLUDED + +#include <linux/leds.h> +#include <dt-bindings/leds/common.h>
[...]
Signature · 2 lines
Best regards, Jacek Anaszewski
Re: [PATCH v3 6/9] leds: multicolor: Introduce a multicolor class definition
Jacek
Thanks for the review
Quoted 47 lines · Jacek Anaszewski
Hi Dan,
Thank you for the v5.
I will confine myself to commenting only some parts since the rest will undergo rework due to removal of sync API.
Quoted 28 lines · Dan Murphy
Introduce a multicolor class that groups colored LEDs within a LED node.
The framework allows for dynamically setting individual LEDs or setting brightness levels of LEDs and updating them virtually simultaneously.
Signed-off-by Dan Murphy <[email protected]>---
drivers/leds/Kconfig | 10 + drivers/leds/Makefile | 1 + drivers/leds/led-class-multicolor.c | 421 +++++++++++++++++++++++++++include/linux/led-class-multicolor.h| 95 ++++++4 files changed, 527 insertions(+)create mode 100644
drivers/leds/led-class-multicolor.ccreate mode 100644include/linux/led-class-multicolor.hdrivers/leds/Kconfig +0
-0@@ -29,6 +29,16 @@ config LEDS_CLASS_FLASHfor the flash related features of a LED device. It can be built as a module.
+configLEDS_CLASS_MULTI_COLOR
+ tristate "LED Mulit Color LED Class Support"
+ depends onLEDS_CLASS
+ helpQuoted 8 lines
+ This option enables the multicolor LED sysfs class in
/sys/class/leds.
+ It wraps LED Class and adds multicolor LED specific sysfs attributes
+ and kernel internal API to it. You'll need this to provide support
+ for multicolor LEDs that are grouped together. This class is not + intended for single color LEDs. It can be built as a module.extra whitespace:
Quoted 1 line
s/ It can/It can/
Ack
Quoted 22 lines
[...]
Quoted 20 lines
drivers/leds/Kconfig +20
-0+ +static int multicolor_set_brightness(struct led_classdev *led_cdev, + enum led_brightness brightness) +{ + struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev); + struct led_classdev_mc_data *data = mcled_cdev->data; + struct led_multicolor_ops *ops = mcled_cdev->ops; + struct led_classdev_mc_priv *priv; + unsigned long state = brightness; + int adj_value; + ssize_t ret = -EINVAL; + + mutex_lock(&led_cdev->led_access); + + if (ops->set_module_brightness) { + ret = ops->set_module_brightness(mcled_cdev, state); + goto unlock; + } + + list_for_each_entry(priv, &data->color_list, list) {
Quoted 8 lines
Quoted 4 lines
+ if (state &&
priv->brightness&&priv->max_brightness) { +adj_value= state * ((priv->brightness* 100) /priv->max_brightness);
+adj_value=adj_value/ 100;Why the multiplication an then division by 100? And
priv->max_brightnessstays unaltered? This changes the proportions. My python script works just fine without those.
Because the kernel does not do floating point math and the calculation is using the ratio
between the intensity and max_intensity and multiplying against the requested brightness.
priv->intensity = 100 (This is the current intensity of the color LED)
priv->max_intensity = 255
state = 80 (This is the requested cluster brightness)
100/255 = 0.392 which is 0.
0 * 80 = 0 this is not what the value should be
But with the multiplier.
10000/255 = 39.2 which is 39 which means that the intensity is only 39% of the
max_intensity.
39 * 80 = 3120 So to preserve the 39% from the 80 we multiply the percentage * requested cluster brightness
3120 / 100 = 31 then we normalize back
I am not sure how your script is working without the multiplier.
Quoted 36 lines
Quoted 14 lines
+ } else
+adj_value=LED_OFF;drivers/leds/Kconfig +11
-0+ + ret = ops->set_color_brightness(priv->mcled_cdev, + priv->color_id, adj_value); + if (ret < 0) + goto unlock; + } + +unlock: + mutex_unlock(&led_cdev->led_access); + return ret; +}[...]
Quoted 21 lines
drivers/leds/Kconfig +21
-0+int led_classdev_multicolor_register_ext(struct device *parent, + struct led_classdev_mc *mcled_cdev, + struct led_init_data *init_data) +{ + struct led_classdev *led_cdev; + struct led_multicolor_ops *ops; + struct led_classdev_mc_data *data; + int ret; + int i; + + if (!mcled_cdev) + return -EINVAL; + + ops = mcled_cdev->ops; + if (!ops || !ops->set_color_brightness) + return -EINVAL; + + data = kzalloc(sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; +
Quoted 8 lines
Quoted 5 lines
+
mcled_cdev->data= data;
+led_cdev=&mcled_cdev->led_cdev;
+
+ if (led_cdev->brightness_set_blocking)
+led_cdev->brightness_set_blocking=multicolor_set_brightness;This is weird. In
leds-lp50xx.cyou don't initializebrightness_set_blockingand this still works?
I will have to look. I don't believe I retested this on lp50xx only the lp55xx code.
Quoted 13 lines
I believe this is kind of omission.
And it is not reasonable to just override driver supplied op with generic one just like that.
I propose to initialize
brightness_setorbrightness_set_blockingop as we used to do it for monochrome LEDs. Those function(s) on driver side will either use device's hardware support for setting color lightness, or will call a generic function provided by LED multi color class for calculating intensities of all colors it comprises in the cluster.
Quoted 2 lines
I know this is different to what we've discussed on IRC, but now it looks for me the most reasonable way to go.
So you want the device driver to handle the brightness request and call into the framework for
calculating the color intensities?
That would work as well and solves a problem of HW supported brightness control like the LP50xx.
The LP50xx would not need to call into the function for calculated intensities.
Quoted 107 lines
Quoted 107 lines
+
INIT_LIST_HEAD(&data->color_list);drivers/leds/Kconfig +23
-0+ + /* Register led class device */ + ret = led_classdev_register_ext(parent, led_cdev, init_data); + if (ret) + return ret; + + ret = led_multicolor_init_color_dir(data, mcled_cdev); + if (ret) + return ret; + + /* Select the sysfs attributes to be created for the device */ + for (i = 0; i < mcled_cdev->num_leds; i++) { + ret = led_multicolor_init_color(data, mcled_cdev, + mcled_cdev->available_colors[i]); + if (ret) + break; + } + + return ret; +} +EXPORT_SYMBOL_GPL(led_classdev_multicolor_register_ext); + +void led_classdev_multicolor_unregister(struct led_classdev_mc
*mcled_cdev)drivers/leds/Kconfig +9
-0+{ + if (!mcled_cdev) + return; + + led_classdev_unregister(&mcled_cdev->led_cdev); +} +EXPORT_SYMBOL_GPL(led_classdev_multicolor_unregister); + +static void devm_led_classdev_multicolor_release(struct device *dev,void *res)
+{
+led_classdev_multicolor_unregister(*(struct led_classdev_mc **)res);drivers/leds/Kconfig +4
-0+} + +/** + * devm_of_led_classdev_register - resource managed
led_classdev_register()
+ *
+ * @parent: parent of LED device
+ * @led_cdev: theled_classdevstructure for this device.
+ */drivers/leds/Kconfig +53
-0+int devm_led_classdev_multicolor_register(struct device *parent, + struct led_classdev_mc *mcled_cdev) +{ + struct led_classdev_mc **dr; + int ret; + + dr = devres_alloc(devm_led_classdev_multicolor_release, + sizeof(*dr), GFP_KERNEL); + if (!dr) + return -ENOMEM; + + ret = led_classdev_multicolor_register(parent, mcled_cdev); + if (ret) { + devres_free(dr); + return ret; + } + + *dr = mcled_cdev; + devres_add(parent, dr); + + return 0; +} +EXPORT_SYMBOL_GPL(devm_led_classdev_multicolor_register); + +static int devm_led_classdev_multicolor_match(struct device *dev, + void *res, void *data) +{ + struct mcled_cdev **p = res; + + if (WARN_ON(!p || !*p)) + return 0; + + return *p == data; +} + +/** + * devm_led_classdev_multicolor_unregister() - resource managed + * led_classdev_multicolor_unregister() + * @parent: The device to unregister. + * @mcled_cdev: the led_classdev_mc structure for this device. + */ +void devm_led_classdev_multicolor_unregister(struct device *dev, + struct led_classdev_mc *mcled_cdev) +{ + WARN_ON(devres_release(dev, + devm_led_classdev_multicolor_release, + devm_led_classdev_multicolor_match, mcled_cdev)); +} +EXPORT_SYMBOL_GPL(devm_led_classdev_multicolor_unregister); + +MODULE_AUTHOR("Dan Murphy <[email protected]>"); +MODULE_DESCRIPTION("Multi Color LED class interface"); +MODULE_LICENSE("GPL v2");include/linux/led-class-multicolor.h new file +0
-0@@ -0,0 +1,95 @@
Quoted 9 lines
Quoted 6 lines
include/linux/led-class-multicolor.h new file +4
-0+// SPDX-License-Identifier: GPL-2.0 + +/* LED Multicolor class interface +* Copyright (C) 2019 Texas Instruments Incorporated - Let's have C++ comment style also here.
ACK
Quoted 8 lines
Quoted 5 lines
include/linux/led-class-multicolor.h new file +5
-0+#ifndef __LINUX_MULTICOLOR_LEDS_H_INCLUDED +#define __LINUX_MULTICOLOR_LEDS_H_INCLUDED + +#include <linux/leds.h> +#include <dt-bindings/leds/common.h> [...]
Re: [PATCH v3 6/9] leds: multicolor: Introduce a multicolor class definition
Dan,
Quoted 3 lines · Dan Murphy
Jacek
Thanks for the review
You're welcome.
Quoted 101 lines · Jacek Anaszewski
Quoted 47 lines
Hi Dan,
Thank you for the v5.
I will confine myself to commenting only some parts since the rest will undergo rework due to removal of sync API.
Quoted 28 lines · Dan Murphy
Introduce a multicolor class that groups colored LEDs within a LED node.
The framework allows for dynamically setting individual LEDs or setting brightness levels of LEDs and updating them virtually simultaneously.
Signed-off-by Dan Murphy <[email protected]>---
drivers/leds/Kconfig | 10 + drivers/leds/Makefile | 1 + drivers/leds/led-class-multicolor.c | 421 +++++++++++++++++++++++++++include/linux/led-class-multicolor.h| 95 ++++++4 files changed, 527 insertions(+)create mode 100644
drivers/leds/led-class-multicolor.ccreate mode 100644include/linux/led-class-multicolor.hdrivers/leds/Kconfig +0
-0@@ -29,6 +29,16 @@ config LEDS_CLASS_FLASHfor the flash related features of a LED device. It can be built as a module.
+configLEDS_CLASS_MULTI_COLOR
+ tristate "LED Mulit Color LED Class Support"
+ depends onLEDS_CLASS
+ helpQuoted 8 lines
+ This option enables the multicolor LED sysfs class in
/sys/class/leds.
+ It wraps LED Class and adds multicolor LED specific sysfs attributes
+ and kernel internal API to it. You'll need this to provide support
+ for multicolor LEDs that are grouped together. This class is not + intended for single color LEDs. It can be built as a module.extra whitespace:
Quoted 1 line
s/ It can/It can/
Ack
Quoted 22 lines
[...]
Quoted 20 lines
drivers/leds/Kconfig +20
-0+ +static int multicolor_set_brightness(struct led_classdev *led_cdev, + enum led_brightness brightness) +{ + struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev); + struct led_classdev_mc_data *data = mcled_cdev->data; + struct led_multicolor_ops *ops = mcled_cdev->ops; + struct led_classdev_mc_priv *priv; + unsigned long state = brightness; + int adj_value; + ssize_t ret = -EINVAL; + + mutex_lock(&led_cdev->led_access); + + if (ops->set_module_brightness) { + ret = ops->set_module_brightness(mcled_cdev, state); + goto unlock; + } + + list_for_each_entry(priv, &data->color_list, list) {Quoted 8 lines
Quoted 4 lines
+ if (state &&
priv->brightness&&priv->max_brightness) { +adj_value= state * ((priv->brightness* 100) /priv->max_brightness);
+adj_value=adj_value/ 100;Why the multiplication an then division by 100? And
priv->max_brightnessstays unaltered? This changes the proportions. My python script works just fine without those.Because the kernel does not do floating point math and the calculation is using the ratio
between the intensity and
max_intensityand multiplying against the requested brightness.
priv->intensity= 100 (This is the current intensity of the color LED)
priv->max_intensity= 255state = 80 (This is the requested cluster brightness)
100/255 = 0.392 which is 0.
0 * 80 = 0 this is not what the value should be
But with the multiplier.
10000/255 = 39.2 which is 39 which means that the intensity is only 39% of the
Quoted 7 lines
max_intensity.39 * 80 = 3120 So to preserve the 39% from the 80 we multiply the percentage * requested cluster brightness
3120 / 100 = 31 then we normalize back
I am not sure how your script is working without the multiplier.
Try to remove brackets around division operation. Then first we are multiplying and only after that dividing.
$ echo "80 * 100 / 255" | bc 31
Quoted 59 lines
Quoted 35 lines
Quoted 13 lines
+ } else
+adj_value=LED_OFF;drivers/leds/Kconfig +11
-0+ + ret = ops->set_color_brightness(priv->mcled_cdev, + priv->color_id, adj_value); + if (ret < 0) + goto unlock; + } + +unlock: + mutex_unlock(&led_cdev->led_access); + return ret; +}[...]
Quoted 21 lines
drivers/leds/Kconfig +21
-0+int led_classdev_multicolor_register_ext(struct device *parent, + struct led_classdev_mc *mcled_cdev, + struct led_init_data *init_data) +{ + struct led_classdev *led_cdev; + struct led_multicolor_ops *ops; + struct led_classdev_mc_data *data; + int ret; + int i; + + if (!mcled_cdev) + return -EINVAL; + + ops = mcled_cdev->ops; + if (!ops || !ops->set_color_brightness) + return -EINVAL; + + data = kzalloc(sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; +Quoted 8 lines
Quoted 5 lines
+
mcled_cdev->data= data;
+led_cdev=&mcled_cdev->led_cdev;
+
+ if (led_cdev->brightness_set_blocking)
+led_cdev->brightness_set_blocking=multicolor_set_brightness;This is weird. In
leds-lp50xx.cyou don't initializebrightness_set_blockingand this still works?I will have to look. I don't believe I retested this on lp50xx only the lp55xx code.
Quoted 13 lines
I believe this is kind of omission.
And it is not reasonable to just override driver supplied op with generic one just like that.
I propose to initialize
brightness_setorbrightness_set_blockingop as we used to do it for monochrome LEDs. Those function(s) on driver side will either use device's hardware support for setting color lightness, or will call a generic function provided by LED multi color class for calculating intensities of all colors it comprises in the cluster.
Quoted 6 lines
Quoted 2 lines
I know this is different to what we've discussed on IRC, but now it looks for me the most reasonable way to go.
So you want the device driver to handle the brightness request and call into the framework for
calculating the color intensities?
Exactly.
Quoted 3 lines
That would work as well and solves a problem of HW supported brightness control like the LP50xx.
The LP50xx would not need to call into the function for calculated intensities.
True.
Signature · 2 lines
Best regards, Jacek Anaszewski
PATCH v3 7/9 dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers
In reply to Dan MurphyIntroduce the bindings for the Texas Instruments LP5036, LP5030, LP5024 and LP5018 RGB LED device driver. The LP5036/30/24/18 can control RGB LEDs individually or as part of a control bank group. These devices have the ability to adjust the mixing control for the RGB LEDs to obtain different colors independent of the overall brightness of the LED grouping.
Datasheet:
http://www.ti.com/lit/ds/symlink/lp5024.pdf
http://www.ti.com/lit/ds/symlink/lp5036.pdf
--- .../devicetree/bindings/leds/leds-lp50xx.txt | 142 ++++++++++++++++++ 1 file changed, 142 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-lp50xx.txt
Documentation/devicetree/bindings/leds/leds-lp50xx.txt new file +142 -0
@@ -0,0 +1,142 @@ +* Texas Instruments - LP5018/24/30/36 RGB LED driver + +The LP50XX is multi-channel, I2C RGB LED Drivers that can group RGB LEDs into +a LED group or control them individually. + +The difference in these RGB LED drivers is the number of supported RGB modules. + +Required parent properties: + - compatible: + "ti,lp5018" + "ti,lp5024" + "ti,lp5030" + "ti,lp5036" + - reg : I2C slave address + lp5018/24 - 0x28 + lp5030/36 - 0x30 + - #address-cells : 1 + - #size-cells : 0 + +Optional parent properties: + - enable-gpios : gpio pin to enable/disable the device. + - vled-supply : LED supply + +Required child properties: + - #address-cells : 1 + - #size-cells : 0 + - reg : This is the LED module number. + - color : see Documentation/devicetree/bindings/leds/common.txt + - function : see Documentation/devicetree/bindings/leds/common.txt + +Required child properties only is LED modules will be banked: + - ti,led-bank : This property denotes the LED module numbers that will + be controlled as a single RGB cluster. Each LED module + number will be controlled by a single LED class instance. + There can only be one instance of the ti,led-bank + property for each device node. + +Required grandchildren properties: + - reg : A single entry denoting the LED module that controls + the RGB cluster. + - color : see Documentation/devicetree/bindings/leds/leds-multicolor.txt + - led-sources : see Documentation/devicetree/bindings/leds/common.txt + +The LED outputs associated with the LED modules are defined in Table 1 of the +corresponding data sheets. + +LP5018 - 6 Total RGB cluster LED outputs 0-5 +LP5024 - 8 Total RGB cluster LED outputs 0-7 +LP5030 - 10 Total RGB cluster LED outputs 0-9 +LP5036 - 12 Total RGB cluster LED outputs 0-11 + +Optional child properties: + - label : see Documentation/devicetree/bindings/leds/common.txt + - linux,default-trigger : + see Documentation/devicetree/bindings/leds/common.txt + +Examples: +led-controller@29 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,lp5024"; + reg = <0x29>; + enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>; + vled-supply = <&vmmcsd_fixed>; + + multi-led@2 { + #address-cells = <1>; + #size-cells = <0>; + reg = <2>; + color = <LED_COLOR_ID_MULTI>; + function = LED_FUNCTION_STANDBY; + ti,led-bank = <2 3 5>; + + led@6 { + reg = <0x6>; + color = <LED_COLOR_ID_RED>; + led-sources = <6 9 15>; + }; + + led@7 { + reg = <0x7>; + color = <LED_COLOR_ID_GREEN>; + led-sources = <7 10 16>; + }; + + led@8 { + reg = <0x8>; + color = <LED_COLOR_ID_BLUE>; + led-sources = <8 11 17>; + }; + }; + + multi-led@1 { + #address-cells = <1>; + #size-cells = <0>; + reg = <1>; + color = <LED_COLOR_ID_MULTI>; + function = LED_FUNCTION_STATUS; + + led@3 { + reg = <3>; + color = <LED_COLOR_ID_RED>; + }; + + led@4 { + reg = <4>; + color = <LED_COLOR_ID_GREEN>; + }; + + led@5 { + reg = <5>; + color = <LED_COLOR_ID_BLUE>; + }; + }; + + multi-led@4 { + #address-cells = <1>; + #size-cells = <0>; + reg = <4>; + color = <LED_COLOR_ID_MULTI>; + function = LED_FUNCTION_ACTIVITY; + + led@12 { + reg = <12>; + color = <LED_COLOR_ID_RED>; + }; + + led@13 { + reg = <13>; + color = <LED_COLOR_ID_GREEN>; + }; + + led@14 { + reg = <14>; + color = <LED_COLOR_ID_BLUE>; + }; + }; +}; + +For more product information please see the link below: +http://www.ti.com/lit/ds/symlink/lp5024.pdf +http://www.ti.com/lit/ds/symlink/lp5036.pdf
Signature · 1 line
2.21.0.5.gaeb582a983
Rob Herring
maintainerRe: [PATCH v3 7/9] dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers
Quoted 10 lines · Dan Murphy
Introduce the bindings for the Texas Instruments LP5036, LP5030, LP5024 and LP5018 RGB LED device driver. The LP5036/30/24/18 can control RGB LEDs individually or as part of a control bank group. These devices have the ability to adjust the mixing control for the RGB LEDs to obtain different colors independent of the overall brightness of the LED grouping.
Datasheet:
http://www.ti.com/lit/ds/symlink/lp5024.pdf
http://www.ti.com/lit/ds/symlink/lp5036.pdf
Quoted 6 lines
Signed-off-by Dan Murphy <[email protected]>--- .../devicetree/bindings/leds/leds-lp50xx.txt | 142 ++++++++++++++++++ 1 file changed, 142 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-lp50xx.txt
Please add Acked-by/Reviewed-by tags when posting new versions. However, there's no need to repost patches *only* to add the tags. The upstream maintainer will do that for acks received on the version they apply.
If a tag was not added on purpose, please state why and what changed.
Re: [PATCH v3 7/9] dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers
Rob
Quoted 17 lines · Rob Herring
Quoted 10 lines · Dan Murphy
Introduce the bindings for the Texas Instruments LP5036, LP5030, LP5024 and LP5018 RGB LED device driver. The LP5036/30/24/18 can control RGB LEDs individually or as part of a control bank group. These devices have the ability to adjust the mixing control for the RGB LEDs to obtain different colors independent of the overall brightness of the LED grouping.
Datasheet:
http://www.ti.com/lit/ds/symlink/lp5024.pdf
http://www.ti.com/lit/ds/symlink/lp5036.pdfQuoted 6 lines
Signed-off-by Dan Murphy <[email protected]>--- .../devicetree/bindings/leds/leds-lp50xx.txt | 142 ++++++++++++++++++ 1 file changed, 142 insertions(+)create mode 100644
Documentation/devicetree/bindings/leds/leds-lp50xx.txt
Quoted 5 lines
Please add Acked-by/Reviewed-by tags when posting new versions. However, there's no need to repost patches *only* to add the tags. The upstream maintainer will do that for acks received on the version they apply.
If a tag was not added on purpose, please state why and what changed.
I usually provide the tags but in my cover letter I did indicate that these patchsets have changed pretty dramatically so I did not feel right about pulling in anyones tags.
Adding your reviewed by on a binding that changed did not seem right to me.
Dan
Re: [PATCH v3 7/9] dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers
In reply to Dan MurphyQuoted 10 lines · Dan Murphy
Introduce the bindings for the Texas Instruments LP5036, LP5030, LP5024 and LP5018 RGB LED device driver. The LP5036/30/24/18 can control RGB LEDs individually or as part of a control bank group. These devices have the ability to adjust the mixing control for the RGB LEDs to obtain different colors independent of the overall brightness of the LED grouping.
Datasheet:
http://www.ti.com/lit/ds/symlink/lp5024.pdf
http://www.ti.com/lit/ds/symlink/lp5036.pdf
Quoted 5 lines
Signed-off-by Dan Murphy <[email protected]>--- .../devicetree/bindings/leds/leds-lp50xx.txt | 142 ++++++++++++++++++ 1 file changed, 142 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-lp50xx.txt
PATCH v3 8/9 leds: lp50xx: Add the LP50XX family of the RGB LED driver
In reply to Dan MurphyIntroduce the LP5036/30/24/18 RGB LED driver.
The difference in these parts are the number of
LED outputs where the:
LP5036 can control 36 LEDs
LP5030 can control 30 LEDs
LP5024 can control 24 LEDs
LP5018 can control 18 LEDs
The device has the ability to group LED output into control banks so that multiple LED banks can be controlled with the same mixing and brightness. Inversely the LEDs can also be controlled independently.
--- drivers/leds/Kconfig | 7 + drivers/leds/Makefile | 1 + drivers/leds/leds-lp50xx.c | 714 +++++++++++++++++++++++++++++++++++++ 3 files changed, 722 insertions(+) create mode 100644 drivers/leds/leds-lp50xx.c
drivers/leds/Kconfig +7 -0
@@ -336,6 +336,13 @@ config LEDS_LP3952 To compile this driver as a module, choose M here: the module will be called leds-lp3952. +config LEDS_LP50XX + tristate "LED Support for TI LP5036/30/24/18 LED driver chip" + depends on LEDS_CLASS && REGMAP_I2C + help + If you say yes here you get support for the Texas Instruments + LP5036, LP5030, LP5024 and LP5018 LED driver. + config LEDS_LP55XX_COMMON tristate "Common Driver for TI/National LP5521/5523/55231/5562/8501" depends on LEDS_LP5521 || LEDS_LP5523 || LEDS_LP5562 || LEDS_LP8501
drivers/leds/Makefile +1 -0
@@ -33,6 +33,7 @@ obj-$(CONFIG_LEDS_GPIO_REGISTER) += leds-gpio-register.o obj-$(CONFIG_LEDS_GPIO) += leds-gpio.o obj-$(CONFIG_LEDS_LP3944) += leds-lp3944.o obj-$(CONFIG_LEDS_LP3952) += leds-lp3952.o +obj-$(CONFIG_LEDS_LP50XX) += leds-lp50xx.o obj-$(CONFIG_LEDS_LP55XX_COMMON) += leds-lp55xx-common.o obj-$(CONFIG_LEDS_LP5521) += leds-lp5521.o obj-$(CONFIG_LEDS_LP5523) += leds-lp5523.o
drivers/leds/leds-lp50xx.c new file +714 -0
714 lines not on this page · raw message
Signature · 1 line
2.21.0.5.gaeb582a983
PATCH v3 9/9 leds: Update the lp55xx to use the multi color framework
In reply to Dan MurphyUpdate the lp5523 to use the multi color framework.
--- drivers/leds/leds-lp5523.c | 13 +++ drivers/leds/leds-lp55xx-common.c | 133 ++++++++++++++++++---- drivers/leds/leds-lp55xx-common.h | 10 ++ include/linux/platform_data/leds-lp55xx.h | 5 + 4 files changed, 142 insertions(+), 19 deletions(-)
drivers/leds/leds-lp5523.c +13 -0
@@ -804,6 +804,18 @@ static ssize_t store_master_fader_leds(struct device *dev,return ret; } +static int lp5523_led_intensity (struct lp55xx_led *led,int chan_num) +{ +struct lp55xx_chip *chip = led->chip; +int ret; + +mutex_lock (&chip->lock); + ret =lp55xx_write (chip,LP5523_REG_LED_PWM_BASE + chan_num, + led->brightness); +mutex_unlock (&chip->lock); +return ret; +} +static int lp5523_led_brightness (struct lp55xx_led *led) {struct lp55xx_chip *chip = led->chip;
@@ -870,6 +882,7 @@ static struct lp55xx_device_config lp5523_cfg = { .max_channel = LP5523_MAX_LEDS, .post_init_device = lp5523_post_init_device, .brightness_fn = lp5523_led_brightness, + .intensity_fn = lp5523_led_intensity, .set_led_current = lp5523_set_led_current, .firmware_cb = lp5523_firmware_loaded, .run_engine = lp5523_run_engine,
drivers/leds/leds-lp55xx-common.c +114 -19
188 lines not on this page · raw message
drivers/leds/leds-lp55xx-common.h +10 -0
@@ -15,6 +15,8 @@#ifndef _LEDS_LP55XX_COMMON_H#define _LEDS_LP55XX_COMMON_H +#include <linux/led-class-multicolor.h> +enum lp55xx_engine_index {LP55XX_ENGINE_INVALID , LP55XX_ENGINE_1,
@@ -112,6 +114,9 @@ struct lp55xx_device_config {/* access brightness register */ int (*brightness_fn)(struct lp55xx_led *led); +/* access brightness register */ +int (*intensity_fn)(struct lp55xx_led *led,int chan_num); +/* current setting function */ void (*set_led_current) (struct lp55xx_led *led,u8 led_current);
@@ -162,6 +167,7 @@ struct lp55xx_chip {* struct lp55xx_led * @chan_nr : Channel number * @cdev : LED class device +* @mc_cdev : Multi color class device * @led_current : Current setting at each led channel * @max_current : Maximun current at each led channel * @brightness : Brightness value
@@ -170,9 +176,13 @@ struct lp55xx_chip {struct lp55xx_led {int chan_nr;struct led_classdev cdev; +struct led_classdev_mc mc_cdev;u8 led_current;u8 max_current;u8 brightness; +int num_colors; +int channel_color[10 ]; +int grouped_channels[10 ];struct lp55xx_chip *chip; };
include/linux/platform_data/leds-lp55xx.h +5 -0
@@ -15,6 +15,8 @@#ifndef _LEDS_LP55XX_H#define _LEDS_LP55XX_H +#include <linux/led-class-multicolor.h> +/* Clock configuration */ #define LP55XX_CLOCK_AUTO 0#define LP55XX_CLOCK_INT 1
@@ -26,6 +28,9 @@ struct lp55xx_led_config {u8 chan_nr;u8 led_current;/* mA x10, 0 if led is not connected */ u8 max_current; +int num_colors; +int channel_color[10 ]; +int grouped_channels[10 ]; };struct lp55xx_predef_pattern {
Signature · 1 line
2.21.0.5.gaeb582a983
Hello Dan,
Am Donnerstag, 23. Mai 2019, 14:08:11 CEST schrieb Dan Murphy:
Quoted 9 lines
leds: multicolor: Add sysfs interface definition
dt: bindings: Add multicolor class dt bindings documention
documention: leds: Add multicolor class documentation
dt-bindings: leds: Add multicolor ID to the color ID list
leds: Add multicolor ID to the color ID list
leds: multicolor: Introduce a multicolor class definition
dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers
leds: lp50xx: Add the LP50XX family of the RGB LED driver
leds: Update the lp55xx to use the multi color framework
While not having much experience with the implementation of the LED subsystem, I've had a short look at those. Curious question: would it be possible to take three gpio-leds and group those together to one multicolor-led? I know at least one board, where things are wired up like this, see e.g. at91-sama5d27_som1_ek.dts
Greets
Alex
Hello
Quoted 12 lines · Alexander Dahl
Hello Dan,
Am Donnerstag, 23. Mai 2019, 14:08:11 CEST schrieb Dan Murphy:
Quoted 9 lines
leds: multicolor: Add sysfs interface definition
dt: bindings: Add multicolor class dt bindings documention
documention: leds: Add multicolor class documentation
dt-bindings: leds: Add multicolor ID to the color ID list
leds: Add multicolor ID to the color ID list
leds: multicolor: Introduce a multicolor class definition
dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers
leds: lp50xx: Add the LP50XX family of the RGB LED driver
leds: Update the lp55xx to use the multi color framework
Quoted 5 lines
While not having much experience with the implementation of the LED subsystem, I've had a short look at those. Curious question: would it be possible to take three gpio-leds and group those together to one multicolor-led? I know at least one board, where things are wired up like this, see e.g.
at91-sama5d27_som1_ek.dts
I have been thinking a lot about how gpio LEDs would fit into the mix here.
The leds-gpio.c would need to be extended to register to the multicolor framework for this to work.
I would need to get or create a gpio led cluster to test it out.
Dan
Quoted 3 lines
Greets
Alex
Hi!
Quoted 11 lines
Am Donnerstag, 23. Mai 2019, 14:08:11 CEST schrieb Dan Murphy:
Quoted 9 lines
leds: multicolor: Add sysfs interface definition
dt: bindings: Add multicolor class dt bindings documention
documention: leds: Add multicolor class documentation
dt-bindings: leds: Add multicolor ID to the color ID list
leds: Add multicolor ID to the color ID list
leds: multicolor: Introduce a multicolor class definition
dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers
leds: lp50xx: Add the LP50XX family of the RGB LED driver
leds: Update the lp55xx to use the multi color framework
Quoted 5 lines
While not having much experience with the implementation of the LED subsystem, I've had a short look at those. Curious question: would it be possible to take three gpio-leds and group those together to one multicolor-led? I know at least one board, where things are wired up like this, see e.g.
at91-sama5d27_som1_ek.dts
Probably could be done, but is not really a good match. Multicolor was meant for LEDs with many brightness levels.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hello
Quoted 12 lines · Pavel Machek
Hi!
Quoted 10 lines
Am Donnerstag, 23. Mai 2019, 14:08:11 CEST schrieb Dan Murphy:
Quoted 9 lines
leds: multicolor: Add sysfs interface definition
dt: bindings: Add multicolor class dt bindings documention
documention: leds: Add multicolor class documentation
dt-bindings: leds: Add multicolor ID to the color ID list
leds: Add multicolor ID to the color ID list
leds: multicolor: Introduce a multicolor class definition
dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers
leds: lp50xx: Add the LP50XX family of the RGB LED driver
leds: Update the lp55xx to use the multi color framework
Quoted 7 lines
Quoted 5 lines
While not having much experience with the implementation of the LED subsystem, I've had a short look at those. Curious question: would it be possible to take three gpio-leds and group those together to one multicolor-led? I know at least one board, where things are wired up like this, see e.g.
at91-sama5d27_som1_ek.dtsProbably could be done, but is not really a good match. Multicolor was meant for LEDs with many brightness levels.
Well that is one aspect of the MC FW. The other aspect was to collate cluster RGB modules into a single
LED node as that RGB cluster would inherently display a functional color (ie RGB battery LED, multi-color
notification LEDs, Multicolor LED keyboards...)
Dan
Quoted 1 line
Pavel