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.59k stars 6.48k forks source link

LED api can't be called from devicetree phandle #42015

Closed caspermeijn closed 2 years ago

caspermeijn commented 2 years ago

Describe the bug The led.h api requires a device * and uint32_t led. If you have a phandle to a led node, you can get the device via DT_PARENT, but the led number is unavailable.

To Reproduce Devicetree dts:

aliases {
    led0 = &blled0; /* backlight low */
};

leds {
    compatible = "gpio-leds";
    blled0: bl_led_0 {
        gpios = <&gpio0 14 GPIO_ACTIVE_LOW>;
        label = "Backlight Low";
    };
};

main.c:

#define LED_NODE DT_ALIAS(led0)
#define LED_PARENT DT_PARENT(LED_NODE)
    const struct device *dev = device_get_binding(LED_PARENT);
    const uint8_t led = 0; // <== This hardcoded constant is the problem
    led_on(dev, led);

kconfig:

CONFIG_LED=y
CONFIG_LED_GPIO=y

Expected behavior I expect a way to find the led number for a phandle.

Impact The workaround is to use the led as a gpio. There are DT macros to obtain the pin settings. But this is not a generic led solution.

As part of #41047 I want to use the led api, but I don't have a way to convert the phandle to the led settings

Logs and console output

Environment (please complete the following information):

Additional context Board: pinetime_devkit0

henrikbrixandersen commented 2 years ago

compatible = "gpio-leds" is just a GPIO, not a LED device. Using it as a GPIO is not a workaround, it the intention.

caspermeijn commented 2 years ago

@henrikbrixandersen There is a LED driver for gpio-leds. You need to enable CONFIG_LED_GPIO. Then https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/led/led_gpio.c will convert the compatible = "gpio-leds" devices to LED devices. From then on you could use the code example. I will add the kconfig options to the example.

mbolivar-nordic commented 2 years ago

@caspermeijn something like this should work:

#define DEVICE_AND_COMMA(node_id) DEVICE_DT_GET(node_id),

const struct device *my_leds[] = { DT_FOREACH_CHILD(LED_PARENT, DEVICE_AND_COMMA) };

Then you can use the index of each device in the array as its LED number.

Mani-Sadhasivam commented 2 years ago

@caspermeijn

    const struct device *dev = device_get_binding(LED_PARENT);
    const uint8_t led = 0; // <== This hardcoded constant is the problem

0 is the index of the GPIO LED and not the actual gpio number. I don't properly get your question. If you want to access an LED on a board using an application, then it is obvious that you'd know how many LEDs are connected on-board via GPIOs and declared in the board dts. Then a simple indexing won't be a problem.

caspermeijn commented 2 years ago

I don't think we understand each other. I will try to rephrase the problem in the hope that it is more clear.

My board has a display. That display has a backlight. The backlight is controlled by a GPIO of the main processor. I want to make the display driver understand how to set the backlight brightness. The led api seems like the perfect abstraction for this.

To prevent hardcoded values in the display driver, I want to use the device tree to indicate which led is the backlight. When I make a phandle from the display to the single led, I can't obtain the led number that is needed for the led api.

So I want to reference a single led (not the array) and I want to do this in a driver (so no application specific knowledge available).

In https://github.com/zephyrproject-rtos/zephyr/pull/41303 I created the phandle I want to use in the driver code.

@Mani-Sadhasivam I hope this makes my problem more clear.

mbolivar-nordic commented 2 years ago

Hi there. I still think that it's possible to synthesize child indexes in C from array offsets, but I agree that's inconvenient enough that this feature is probably worth just adding to the DT API: https://github.com/zephyrproject-rtos/zephyr/pull/44151

caspermeijn commented 2 years ago

@mbolivar-nordic Thanks, this fixes my issue. I had prototyped something similar, but I couldn't find a way to handle it properly. DT_NODE_CHILD_IDX is the way to go.