drivers: led: led_gpio: Add support of blink#89734
drivers: led: led_gpio: Add support of blink#89734anobli wants to merge 2 commits intozephyrproject-rtos:mainfrom
Conversation
|
OK, indeed, this makes sense. I will think about to another implementation more adapted to the fallback. |
The led_api_blink is quite convenient to use. This adds its support to the led_gpio driver. Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
anobli
force-pushed
the
abailon/led-gpio-blink
branch
4 times, most recently
from
1c7dfcf to
43f8722
Compare
The LED blink API i squite useful but this is not supported by all the LED controller. This provides a software implementation of blink that relies on timer to blink a LED. This could be enabled using Kconfig, as well the maximum number of LED that could blink at the same time using blink fallback. This does not requires any changes to driver to be used. Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
anobli
force-pushed
the
abailon/led-gpio-blink
branch
from
43f8722 to
f92a058
Compare
|
|
Hi @anobli, Maybe you should close this PR, and take the time to write a proper driver for software blinking ? And once ready you could open a new PR, with the right title and a clean history ? In addition, are you sure that timer is the right tool to implement software blinking ? There are various backends behind LEDs: GPIO, SPI, I2C, PWM, and many others. Some of them may even sleep during I/O. Is that allowed in timer context in Zephyr ? On all platforms ? In Linux for example, timers are indeed used for software blinking. But if the LED end-driver doesn't provide the This software blinking driver will be quite an important one. So please take the time to make it great and optimized. |
|
|
||
| if (api->blink == NULL) { | ||
| return -ENOSYS; | ||
| return led_blink_fallback(dev, led, delay_on, delay_off); |
There was a problem hiding this comment.
I think it would be better to put this in an ifdef and implement it so that it keeps clean when it's disabled.
| const struct led_driver_api *api = | ||
| (const struct led_driver_api *)dev->api; | ||
|
|
||
| led_blink_fallback(dev, led, 0, 0); |
| const struct led_driver_api *api = | ||
| (const struct led_driver_api *)dev->api; | ||
|
|
||
| led_blink_fallback(dev, led, 0, 0); |
| const struct gpio_dt_spec *led_gpio = &config->led[led]; | ||
|
|
||
| data->blink_state ^= 1 << led; | ||
| gpio_pin_set_dt(led_gpio, data->blink_state & (1 << led)); |
There was a problem hiding this comment.
Can this be replaced with led_on to work with other LED devices?
There was a problem hiding this comment.
I left it by mistake. Actually, all the change made in led_gpio.c should be reverted.
I will close this PR, and open a new one soon. |
|
Closing this PR since the scope has changed. |
5 participants
The led_api_blink is quite convenient to use.
This adds its support to the led_gpio driver.