[U-Boot] [PATCH 1/2] add generic stubs for GPIO LEDs

[Simon Glass]

Hi Bernard,

On 21 August 2015 at 07:13, Bernhard Nortmann <bernhard.nortmann at web.de> wrote:
> For boards that support LEDs driven via GPIO (CONFIG_GPIO_LED),
> it may be useful to have some generic stubs (wrapper functions)
> for the "colored" LEDs.
>
> This allows defining STATUS_LED_* values directly to GPIO numbers,
> e.g.: #define STATUS_LED_GREEN  248 /* = PH24 */
>
> To keep those optional, it's probably best to introduce an additional
> configuration setting. I've chosen CONFIG_GPIO_LED_STUBS for that.
> Placing the code in drivers/misc/gpio_led.c also ensures that it
> automatically depends on CONFIG_GPIO_LED too.
>
> Signed-off-by: Bernhard Nortmann <bernhard.nortmann at web.de>
> ---
>
>  drivers/misc/gpio_led.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)

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

Reviewed-by: Simon Glass <sjg at chromium.org>

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)

BTW there is a device-tree-enabled LED driver node (see drivers/led).
It might be worth considering using this on some sunxi boards.

>
> diff --git a/drivers/misc/gpio_led.c b/drivers/misc/gpio_led.c
> index 3e95727..164c30d 100644
> --- a/drivers/misc/gpio_led.c
> +++ b/drivers/misc/gpio_led.c
> @@ -51,3 +51,57 @@ void __led_toggle(led_id_t mask)
>  {
>         gpio_set_value(mask, !gpio_get_value(mask));
>  }
> +
> +#ifdef CONFIG_GPIO_LED_STUBS
> +
> +/* 'generic' override of colored LED stubs, to use GPIO functions instead */
> +
> +#ifdef STATUS_LED_RED
> +void red_led_on(void)
> +{
> +       __led_set(STATUS_LED_RED, STATUS_LED_ON);
> +}
> +
> +void red_led_off(void)
> +{
> +       __led_set(STATUS_LED_RED, STATUS_LED_OFF);
> +}
> +#endif
> +
> +#ifdef STATUS_LED_GREEN
> +void green_led_on(void)
> +{
> +       __led_set(STATUS_LED_GREEN, STATUS_LED_ON);
> +}
> +
> +void green_led_off(void)
> +{
> +       __led_set(STATUS_LED_GREEN, STATUS_LED_OFF);
> +}
> +#endif
> +
> +#ifdef STATUS_LED_YELLOW
> +void yellow_led_on(void)
> +{
> +       __led_set(STATUS_LED_YELLOW, STATUS_LED_ON);
> +}
> +
> +void yellow_led_off(void)
> +{
> +       __led_set(STATUS_LED_YELLOW, STATUS_LED_OFF);
> +}
> +#endif
> +
> +#ifdef STATUS_LED_BLUE
> +void blue_led_on(void)
> +{
> +       __led_set(STATUS_LED_BLUE, STATUS_LED_ON);
> +}
> +
> +void blue_led_off(void)
> +{
> +       __led_set(STATUS_LED_BLUE, STATUS_LED_OFF);
> +}
> +#endif
> +
> +#endif /* CONFIG_GPIO_LED_STUBS */
> --
> 2.4.6
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

Regards,
Simon