Hello Simon!

Am 23.08.2015 23:21, schrieb Simon Glass:
Hi Bernard,

[...]
If this is a new option it should be added to Kconfig. Otherwise:

Reviewed-by: Simon Glass <s...@chromium.org>
Right. Kconfig wasn't on my agenda, but I agree that it should go in there.
Unfortunately this points out further problems. The existing CONFIG_GPIO_LED
is not part of Kconfig, but gets set via board-specific (.h) includes. Kconfig
has/supports CONFIG_LED_GPIO, but that is related to the driver model
variant, and doesn't affect inclusion of drivers/misc/gpio_led.c ...
admittedly it's a bit of a mess.

I am not a huge fan of the existing API. I would suggest that if you
have the energy you could replace it with something like:

enum led_colour_t {
    led_red,
    red_green,
    ...
};

int led_set_on(enum led_colour_t colour, bool on)
Yes, I've been code-dancing a bit back and forth to get this stuff to fit in
with the existing API. One reason I have selected these "stub" functions
is that they are meant to match (and replace) the existing weak definitions
in arch/arm/lib/board.c. Also, my impression is that they exist as separate
functions to allow common/cmd_led.c to use them as function pointers,
namely for the construction of the led_commands[] array.

The API may not be pretty, but I have in fact tried to keep it as close to
the original as possible, with the goal of allowing to simply substitute
GPIO numbers for the "LED bits".

BTW there is a device-tree-enabled LED driver node (see drivers/led).
It might be worth considering using this on some sunxi boards.
Thanks! I'll definitely have a look into it.
(That one is related to the CONFIG_LED_GPIO mentioned above.)

Regards, B. Nortmann
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to