Add tlc59731 strip led driver #68617
Conversation
zephyrbot
added
platform: STM32
zephyrbot
requested review from
ABOSTM,
Desvauxm-st,
FRASTM,
GeorgeCGV,
Mani-Sadhasivam,
bbilas,
erwango,
galak,
gautierg-st,
kartben,
nashif,
simonguinot,
vaishnavachath and
vanti
javad123javad
force-pushed
the
add_tlc59731_strip_led_driver
branch
from
f1cfd50 to
7b67864
Compare
There was a problem hiding this comment.
Why is this gpio-leds? That makes it looks as if we have two separate LEDs while in reality there is just one.
What we need is a gpio-hog to force enable it. And that should possibly even be in stm32wb5mm_dk.dts, although I don't know if that is possible in a way that it's only enabled if the rgb_leds is enabled.
Alternatively, it should be a named pin in stm32wb5mm_dk.dts (probably better name it GPIO_SELECT2 like in the schematic) and then driven from the example code like is done below.
There was a problem hiding this comment.
I agree, However, I couldn't find a suitable binding for the chip select, so I used gpio-leds as it is the closest family to it.
There was a problem hiding this comment.
For a GPIO that you use with the GPIO_DT_SPEC_GET macro, you don't need a compatible at all, because you don't bind it to a driver.
simonguinot
requested review from
soburi and
thedjnK
and removed request for
Desvauxm-st and
mbolivar-ampere
There was a problem hiding this comment.
@simonguinot think it would make sense to have a base dts file wit the length property and have include/zephyr/drivers/led_strip.h validate the num_pixels parameter against it (outside the scope of this PR)
drivers/led_strip/tlc59731.c
Outdated
There was a problem hiding this comment.
@simonguinot and also for this to error if not supported in the same include so it can be entirely omitted here
|
@simonguinot can you re-review please? |
The code review is complete and apart from the fact that this driver uses GPIO to configure the LED strip, it is now in good shape. But I am not sure if it should be merged... @javad123javad, are you still working on an SPI implementation ? |
I am implementing the SPI version in another branch. The development is completed, however, there are some problems relating to the pulse timing that prevent it from working properly. |
|
@simonguinot so can this be merged? |
Yes this can be merged. |
Sure, I will apply the changes after #71630 is merged. Because there are some dependencies that I need from this PR. |
|
@javad123javad can be updated |
javad123javad
force-pushed
the
add_tlc59731_strip_led_driver
branch
from
4d07df4 to
ce26763
Compare
TLC59731 is a 3-Channel, 8-Bit, PWM LED Driver with TI Single-Wire interface (EasySet) protocol. Signed-off-by: Javad Rahimipetroudi <[email protected]>
This patch add RGB LED support for the TI TLC59731 RGB controller. Signed-off-by: Javad Rahimipetroudi <[email protected]>
javad123javad
force-pushed
the
add_tlc59731_strip_led_driver
branch
from
ce26763 to
3d31f14
Compare
11 participants
This patch series add driver support for TI TLC59731 RGB controller, along side the changes applied for STM32WB5MM_DK board and a demo samples based on this board.