zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.96k stars 6.67k forks source link

LED driver for RGB leds + device tree support #51858

Open koffes opened 2 years ago

koffes commented 2 years ago

Zephyr has a LED driver https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/led/led_gpio.c which is suitable for single LEDs. There is however (IMHO) no good way to handle RGB leds.

Describe the solution you'd like It would be great to have native support for RGB leds. That is, having a common API for both RGB and single LEDs. Something on this form:

// Turns on a single color LED or turns on  R, G and B channel
int led_gpio_on(const struct device *dev, uint32_t led_group);

// Turns off single color LED or turns off R, G and B channel
int led_gpio_off(const struct device *dev, uint32_t led_group); 

// Vararg function. Takes a single uint8_t for single_leds or three uint8_t for RGB leds
int led_gpio_set_brightness(const struct device *dev, uint32_t led_group, ...) 

// Will return error if supplied with single-led-group. color_define can be e.g. COLOR_RED, COLOR_WHITE...
int led_gpio_set_color(const struct device *dev, uint32_t led_group, uint8_t color_define) 

This would also mean that the devicetree must have some mechanism to group three GPIOs to form an RGB LED group.

koffes commented 2 years ago

FYI @alexsven @erikrobstad. I discovered this https://github.com/zephyrproject-rtos/zephyr/blob/main/include/zephyr/drivers/led.h which I need to investigate.

nordicjm commented 2 years ago

There is LED strip already, which can be used (with a size of 1) https://docs.zephyrproject.org/latest/hardware/peripherals/led.html#led-strip

koffes commented 2 years ago

I have had a look at this one, as well as others, but it seems to me that in order to use them you need an external driver IC? In the use case we want to solve, the LED channels are connected directly to host GPIOs.

zephyrbot commented 9 months ago

Hi @mbolivar-ampere, @simonguinot, @Mani-Sadhasivam,

This issue, marked as an Enhancement, was opened a while ago and did not get any traction. It was just assigned to you based on the labels. If you don't consider yourself the right person to address this issue, please re-assing it to the right person.

Please take a moment to review if the issue is still relevant to the project. If it is, please provide feedback and direction on how to move forward. If it is not, has already been addressed, is a duplicate, or is no longer relevant, please close it with a short comment explaining the reason.

@koffes you are also encouraged to help moving this issue forward by providing additional information and confirming this request/issue is still relevant to you.

Thanks!

simonguinot commented 9 months ago

The LED DT API already allows to define multi-color (i.e. multi-channel) LEDs. Here is an example:

leds {
        compatible = "gpio-leds";

        /* Multi channel GPIO LED */
        led_0 {
                label = "RGB LED";
                gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>,
                        <&gpio0 2 GPIO_ACTIVE_LOW>,
                        <&gpio0 3 GPIO_ACTIVE_HIGH>;
                color-mapping =
                        <LED_COLOR_ID_RED>,
                        <LED_COLOR_ID_GREEN>,
                        <LED_COLOR_ID_BLUE>;
        };
        /* Single channel GPIO LED */
        led_1 {
                label = "Green LED";
                gpios = <&gpio0 4 GPIO_ACTIVE_HIGH>;
                color-mapping = <LED_COLOR_ID_GREEN>,
        };
};

On the led_gpio driver side, we simply need to implement support for the color-mapping and gpios array properties through the ->set_color() method of the LED API. And probably we will need to disable the ->set_brightness() method for a multi-color LED. And that's it.

Note that the same comment applies to PWM LEDs.

Grouping LEDs is an interesting idea. Some LED controllers provides this feature and it would be nice for the LED API to support it. But I don't think it is the right way to support RGB GPIO LEDs.

simonguinot commented 9 months ago

Related with #63387

koffes commented 2 weeks ago
          Hi @koffes. The status is that we need to implement https://elixir.bootlin.com/linux/v6.10.3/source/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml and eventually https://elixir.bootlin.com/linux/v6.10.3/source/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml to keep compatibility with the LED DT bindings of the Linux kernel.

Originally posted by @simonguinot in https://github.com/zephyrproject-rtos/zephyr/issues/74013#issuecomment-2468442939

koffes commented 2 weeks ago

FYI @gWacey

simonguinot commented 2 weeks ago

Also I am also considering reopening https://github.com/zephyrproject-rtos/zephyr/pull/69227 and https://github.com/zephyrproject-rtos/zephyr/pull/74013.

Indeed they introduce the use of several items for the gpios and pwms properties (for the gpio-leds and pwm-leds DT bindings), while it is limited to 1 for Linux. But it is an elegant and cheap solution to bring support for GPIO/PWM RGB LEDs in Zephyr. The alternative of stacking drivers through pwm-leds-multicolor is not super appealing for an embedded OS...

gWacey commented 1 week ago

I like this approach and I have been looking at something similar. It would sit well with the blink implementation #57791.

gWacey commented 1 week ago

@simonguinot will you be reopening the PRs as it is an elegant solution to the problem I am facing?

simonguinot commented 6 days ago

I don't know @gWacey.

Here is an attempt to describe the situation.

I wrote several Linux LED drivers in the past and at the time there was no support for multi-color LEDs. One LED was equal to one channel. And each color/channel of an RGB LED was handled independently from the others via a dedicated sysfs entry. It was actually not great at all.

So in 2020, when I started working on LEDs in Zephyr, I brought support for multi-color LEDs via the color-mapping DT property and ->set_color() API function. color-mapping introduces the notion that 1 LED can be composed of several color channels. And we grew from there. It is now used by 3 drivers: is31fl3194, lp50xx, ncp5623. The #69227 and #74013 PRs result directly from this.

But in the meantime, Linux took a different path. The color and function DT properties have been introduced and the ability to "compose" a multi-color LED with several monochrome LEDs was added.

In Linux, a LED controller with RGB or multi-color LEDs uses the following DT representation:

led-controller {
    compatible = "...";
    reg = <...>;

    multi-led@0 {
        color = <LED_COLOR_ID_RGB or LED_COLOR_ID_MULTI>;

        led@x {
            reg = <x>;
            color = <LED_COLOR_ID_RED>;
        };

        led@y {
            reg = <y>;
            color = <LED_COLOR_ID_GREEN>;
        };

        led@z {
            reg = <z>;
            color = <LED_COLOR_ID_BLUE>;
        };
    };
    multi-led@1 {
    ...
    };
    ...
};

While in Zephyr, we use:

led-controller {
    compatible = "...";
    reg = <...>;

    led@0 {
        index = <...>;
        color-mapping =
            <LED_COLOR_ID_RED>,
            <LED_COLOR_ID_GREEN>,
            <LED_COLOR_ID_BLUE>;
    };
    led@1 {
        ...
    };
    ...
};

So we are actually not aligned with Linux. For example, the DT bindings for the Texas Instruments LP50XX controllers ("ti,lp50xx") are not compatible.

For GPIO RGB LEDs, Linux introduced the leds-group-multicolor DT binding: https://elixir.bootlin.com/linux/v6.12/source/Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml. This allows to compose an RGB LED by stacking a leds-group-multicolor driver on the top of any other LED driver (led-gpio, led-pwm, ...). This allows to add support for RGB GPIO LED to existing boards by adding a new DT node and without modifying the existing DT LED nodes.

While leds-group-multicolor can be used to compose a PWM RGB LED, another DT binding has been introduced: leds-pwm-multicolor (https://elixir.bootlin.com/linux/v6.12/source/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml). It uses the multi-led DT representation I described above and it allows to drive a PWM RGB LED directly, without stacking the leds-group-multicolor driver. So there are two drivers/bindings for PWM LEDs in Linux: led-pwm and leds-pwm-multicolor. A new board can use leds-pwm-multicolor to support PWM RGB LEDs. And an old board can go with led-pwm + leds-group-multicolor.

So to summarize, it's a mess :)

From where we are now, #69227 and #74013 are the right path. Stacking drivers is not really efficient for an embedded OS. But this takes us further away from Linux and it must be discussed.