Extend LP503x driver to support LP50xx series#54594
Extend LP503x driver to support LP50xx series#54594dariis wants to merge 5 commits intozephyrproject-rtos:mainfrom
Conversation
Rename to use driver for different lp50xx led controllers. Signed-off-by: Rico Ganahl <rico.ganahl@bytesatwork.ch>
Prepare defines for LP50xx series. Adapt define in lp503x sample. Signed-off-by: Rico Ganahl <rico.ganahl@bytesatwork.ch>
Add base yml for defining LP50xx. Signed-off-by: Rico Ganahl <rico.ganahl@bytesatwork.ch>
Add support for LP5009, LP5012, LP5018 and LP5024. Changed from original driver to use last set brightness when turning LED on and set enable gpio in init function. Introduce feature to blink with software timer. Needs to be enabled in Kconfig. New optional DT property defalt-brightness to set the initial brightness in percent. Additional optional DT property norm-value to normalize brightness of the RGB LED. The value represents the sum of the RGB color channels which corresponds to 100% brightness. For perfect color representation the brightness normalization should be disabled and handled by a so called color map (or a LUT) introduced by the app. Feel free to implement a more sophisticated brightness adaptation like gamma correction. Chip feature "LED Bank Control" is not supported. Signed-off-by: Rico Ganahl <rico.ganahl@bytesatwork.ch>
Sample usage of LP5018 I2C LED controller. Signed-off-by: Rico Ganahl <rico.ganahl@bytesatwork.ch>
37be5d2 to
45b6101
Compare
|
|
||
| endif # DT_HAS_TI_LP5018_ENABLED | ||
|
|
||
| if LP50XX_ALLOW_SOFTWARE_BLINK |
There was a problem hiding this comment.
This commit should be split into several (and eventually into several PRs): one or more to add controller series support and one commit per feature you want to add (like software blinking).
About software blinking... It is not something we want to implement at the driver level. We want end-driver to expose hardware capabilities through the LED API. If a controller can blink its LEDs, then led_api_blink() should be implemented. If not, then it should be let empty. Software blinking should be considered as a fallback if hardware support is missing. This way, generic software blinking can be implemented in LED core and shared between all the LED drivers.
Please have a look at #50511. The same arguments about color correction have been developed.
There was a problem hiding this comment.
Just some thoughts:
This commit should be split into several ...
Okay, let's wait for progress in #46134
About software blinking...
Are there any plans to do a LED core with software features which are not supported by the driver(s)?
Otherwise we could let the user decide via Kconfig as in this PR. What is your opinion regarding Kconfig?
There was a problem hiding this comment.
Hi @dariis. My preference is to have software LED effects implemented in some LED core code and used as fall-backs when there is no hardware support available (i.e. when the end-driver API function is missing). I am not working on it yet but it is in my todo list.
There was a problem hiding this comment.
just to clarify:
To get it merged I have to remove the software blink part, right?
There was a problem hiding this comment.
For a start, yes. Eventually you also have to split the main commit (if possible). If you want to continue with this PR then I'll start the review process at the next iteration.
There was a problem hiding this comment.
Hi @dariis. My preference is to have software LED effects implemented in some LED core code and used as fall-backs when there is no hardware support available (i.e. when the end-driver API function is missing). I am not working on it yet but it is in my todo list.
Hi is there any progress on this?
There was a problem hiding this comment.
Hi @dariis. My preference is to have software LED effects implemented in some LED core code and used as fall-backs when there is no hardware support available (i.e. when the end-driver API function is missing). I am not working on it yet but it is in my todo list.
Hi is there any progress on this?
No, it is still on my to-do list.
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
|
close due to #59852 |
Tested on custom board with LP5018 connected to Arduino uno connector on stm32h747i_disco.