[U-Boot] [PATCH] AT91: Add support for blue_LED_* and add coloured_LED_init to at91/led.c
Currently, at91/led.c only provides _on and _off functions for green, yellow and red LEDs. This patch provides a generic coloured_LED_init function, which is a first step towards getting rid of the board-specific (and duplicated) board/*/*/led.c files on at91 This patch alos adds similar support for blue LEDs, mostly for the sake of completeness.
Signed-off-by: Albin Tonnerre albin.tonnerre@free-electrons.com --- cpu/arm926ejs/at91/led.c | 32 ++++++++++++++++++++++++++++++++ 1 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/cpu/arm926ejs/at91/led.c b/cpu/arm926ejs/at91/led.c index be68f59..f6b1912 100644 --- a/cpu/arm926ejs/at91/led.c +++ b/cpu/arm926ejs/at91/led.c @@ -27,6 +27,26 @@ #include <asm/arch/gpio.h> #include <asm/arch/io.h>
+void coloured_LED_init(void) +{ +#ifdef CONFIG_RED_LED + at91_set_gpio_output(CONFIG_RED_LED, 0); + red_LED_off(); +#endif +#ifdef CONFIG_GREEN_LED + at91_set_gpio_output(CONFIG_GREEN_LED, 0); + green_LED_off(); +#endif +#ifdef CONFIG_YELLOW_LED + at91_set_gpio_output(CONFIG_YELLOW_LED, 0); + yellow_LED_off(); +#endif +#ifdef CONFIG_BLUE_LED + at91_set_gpio_output(CONFIG_BLUE_LED, 0); + blue_LED_off(); +#endif +} + #ifdef CONFIG_RED_LED void red_LED_on(void) { @@ -62,3 +82,15 @@ void yellow_LED_off(void) at91_set_gpio_value(CONFIG_YELLOW_LED, 1); } #endif + +#ifdef CONFIG_BLUE_LED +void blue_LED_on(void) +{ + at91_set_gpio_value(CONFIG_BLUE_LED, 0); +} + +void blue_LED_off(void) +{ + at91_set_gpio_value(CONFIG_BLUE_LED, 1); +} +#endif
On 18:10 Wed 12 Aug , Albin Tonnerre wrote:
Currently, at91/led.c only provides _on and _off functions for green, yellow and red LEDs. This patch provides a generic coloured_LED_init function, which is a first step towards getting rid of the board-specific (and duplicated) board/*/*/led.c files on at91 This patch alos adds similar support for blue LEDs, mostly for the sake of completeness.
we do need to stop adding more and more specific led api we do need to have a common api that we can use on any arch for c & asm
Best Regards, J.
On Wed, Aug 12, 2009 at 11:15:26PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote :
On 18:10 Wed 12 Aug , Albin Tonnerre wrote:
Currently, at91/led.c only provides _on and _off functions for green, yellow and red LEDs. This patch provides a generic coloured_LED_init function, which is a first step towards getting rid of the board-specific (and duplicated) board/*/*/led.c files on at91 This patch alos adds similar support for blue LEDs, mostly for the sake of completeness.
we do need to stop adding more and more specific led api we do need to have a common api that we can use on any arch for c & asm
This does not *add* specific LED API, it merely implements the existing one. There /is/ a common led API, which currently has no support for ARM, as you probably know. Does you message mean you'd accept a patch which drops the coloured-LED API (whose only user is currently ARM) and use the one described in include/status_led.h ?
Regards,
On 23:39 Wed 12 Aug , Albin Tonnerre wrote:
On Wed, Aug 12, 2009 at 11:15:26PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote :
On 18:10 Wed 12 Aug , Albin Tonnerre wrote:
Currently, at91/led.c only provides _on and _off functions for green, yellow and red LEDs. This patch provides a generic coloured_LED_init function, which is a first step towards getting rid of the board-specific (and duplicated) board/*/*/led.c files on at91 This patch alos adds similar support for blue LEDs, mostly for the sake of completeness.
we do need to stop adding more and more specific led api we do need to have a common api that we can use on any arch for c & asm
This does not *add* specific LED API, it merely implements the existing one. There /is/ a common led API, which currently has no support for ARM, as you probably know. Does you message mean you'd accept a patch which drops the coloured-LED API (whose only user is currently ARM) and use the one described in include/status_led.h ?
no please take a look on the other LED thread
I want to hape coloured-LED api and numbered led handle by the same api with the less size impact
for c & assembly
Best Regards, J.
On Tue, Aug 18, 2009 at 12:51:48AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote :
no please take a look on the other LED thread
Would you please provide a pointer to this thread ? THe only one remotely related I can find is http://lists.denx.de/pipermail/u-boot/2009-May/052160.html, and you did not participate in this one ...
I want to hape coloured-LED api and numbered led handle by the same api with the less size impact
When you have this same API, you'll still need code to turn LEDs on and off, and you'll still have to beat the LED code out of cards-specific code because it really ought to be in a common file. My patch does exactly that, does *not* impact size, and is an actual step towards whatever you plan next. I'd prefer if you were rejecting this on actual, justified grounds.
Regards,
On 10:49 Tue 18 Aug , Albin Tonnerre wrote:
On Tue, Aug 18, 2009 at 12:51:48AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote :
no please take a look on the other LED thread
Would you please provide a pointer to this thread ? THe only one remotely related I can find is http://lists.denx.de/pipermail/u-boot/2009-May/052160.html, and you did not participate in this one ...
I've as I'm the one who ask Daniel Gorsulowski to base his new code (this patch) against Ulf precedent patch and as other people have done the same comment as I will do no need to repeat it
I want to hape coloured-LED api and numbered led handle by the same api with the less size impact
When you have this same API, you'll still need code to turn LEDs on and off, and you'll still have to beat the LED code out of cards-specific code because it really ought to be in a common file. My patch does exactly that, does *not* impact size, and is an actual step towards whatever you plan next. I'd prefer if you were rejecting this on actual, justified grounds.
your patch does not do it 1) you force the init led to be common which must be board specific as every one can do it how he want
2) you do brake all at91 boards too
3) as already said multiple time we need to cleanup it and stop adding new color support for eachi board or arch specialy when no board use it
Best Regards, J.
On Thu, Aug 20, 2009 at 02:00:45AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote :
On 10:49 Tue 18 Aug , Albin Tonnerre wrote:
On Tue, Aug 18, 2009 at 12:51:48AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote :
no please take a look on the other LED thread
Would you please provide a pointer to this thread ? THe only one remotely related I can find is http://lists.denx.de/pipermail/u-boot/2009-May/052160.html, and you did not participate in this one ...
I've as I'm the one who ask Daniel Gorsulowski to base his new code (this patch) against Ulf precedent patch and as other people have done the same comment as I will do no need to repeat it
Ok, so I'm really confused now. This patch does exactly what you're arguing against in the rest of your mail, and you still don't provide a pointer to Ulf's patch. Would you mind *explaining* to me what your plan is? I just can't get it.
Regards,
Dear Albin Tonnerre,
In message 20090820083619.GB5086@pc-ras4041.res.insa you wrote:
Ok, so I'm really confused now. This patch does exactly what you're arguing against in the rest of your mail, and you still don't provide a pointer to Ulf's patch. Would you mind *explaining* to me what your plan is? I just can't get it.
I know this doesn't help you, but maybe it solaces you: I'm confused as well. I completely los track of what might be needed to get changed to make this patch acceptable.
Jean-Christophe, would you please be so kind and elucidate?
Best regards,
Wolfgang Denk
On 10:36 Thu 20 Aug , Albin Tonnerre wrote:
On Thu, Aug 20, 2009 at 02:00:45AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote :
On 10:49 Tue 18 Aug , Albin Tonnerre wrote:
On Tue, Aug 18, 2009 at 12:51:48AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote :
no please take a look on the other LED thread
Would you please provide a pointer to this thread ? THe only one remotely related I can find is http://lists.denx.de/pipermail/u-boot/2009-May/052160.html, and you did not participate in this one ...
I've as I'm the one who ask Daniel Gorsulowski to base his new code (this patch) against Ulf precedent patch and as other people have done the same comment as I will do no need to repeat it
Ok, so I'm really confused now. This patch does exactly what you're arguing against in the rest of your mail, and you still don't provide a pointer to Ulf's patch. Would you mind *explaining* to me what your plan is? I just can't get it.
something like this for assembly
.macro set_led \num, \state call (c or assembly) set_led_state num state .endm
.macore set_led_red \state call (c or assembly) set_led_state CONFIG_SYS_LED_RED state .endm
and for c
void set_led(int num, int state) { .... }
void set_reg_led(int state) { set_led(CONFIG_SYS_LED_RED, state); }
etc... for all colour
maybe an array could help
struct leds [] = { #ifdef CONFIG_SYS_LED_RED { .name = "red", .num = CONFIG_SYS_LED_RED, }, #endif #ifdef CONFIG_SYS_LED_GREEN { .name = "green", .num = CONFIG_SYS_LED_GREEN, }, #endif };
so we can create a command to manage them
static int do_set_led(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) { int i;
if argc[1] is num && < array_size of leds then set_led(num, argv[2]); else if argc[1] is a colour led then set_led(get_led_num(argc[1]), argc[2]); fi
}
so this will not be arm specific anymore
Best Regards, J.
On Sat, 05 Sep 2009 01:47 +0200, Jean-Christophe PLAGNIOL-VILLARD wrote :
On 10:36 Thu 20 Aug , Albin Tonnerre wrote:
On Thu, Aug 20, 2009 at 02:00:45AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote :
On 10:49 Tue 18 Aug , Albin Tonnerre wrote:
On Tue, Aug 18, 2009 at 12:51:48AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote :
no please take a look on the other LED thread
Would you please provide a pointer to this thread ? THe only one remotely related I can find is http://lists.denx.de/pipermail/u-boot/2009-May/052160.html, and you did not participate in this one ...
I've as I'm the one who ask Daniel Gorsulowski to base his new code (this patch) against Ulf precedent patch and as other people have done the same comment as I will do no need to repeat it
Ok, so I'm really confused now. This patch does exactly what you're arguing against in the rest of your mail, and you still don't provide a pointer to Ulf's patch. Would you mind *explaining* to me what your plan is? I just can't get it.
something like this for assembly
.macro set_led \num, \state call (c or assembly) set_led_state num state .endm
.macore set_led_red \state call (c or assembly) set_led_state CONFIG_SYS_LED_RED state .endm
and for c
void set_led(int num, int state) { .... }
void set_reg_led(int state) { set_led(CONFIG_SYS_LED_RED, state); }
etc... for all colour
Ok. So what about implementing the current, existing LED API ? you'd get:
void status_led_set (int led, int state); that sound a lot like your set_led function. And then
status_led_set(STATUS_LED_{YELLOW,BLUE,RED}, state)
which is basically the same as your set_reg_led?
Cheers,
On 13:20 Sat 05 Sep , Albin Tonnerre wrote:
On Sat, 05 Sep 2009 01:47 +0200, Jean-Christophe PLAGNIOL-VILLARD wrote :
On 10:36 Thu 20 Aug , Albin Tonnerre wrote:
On Thu, Aug 20, 2009 at 02:00:45AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote :
On 10:49 Tue 18 Aug , Albin Tonnerre wrote:
On Tue, Aug 18, 2009 at 12:51:48AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote :
no please take a look on the other LED thread
Would you please provide a pointer to this thread ? THe only one remotely related I can find is http://lists.denx.de/pipermail/u-boot/2009-May/052160.html, and you did not participate in this one ...
I've as I'm the one who ask Daniel Gorsulowski to base his new code (this patch) against Ulf precedent patch and as other people have done the same comment as I will do no need to repeat it
Ok, so I'm really confused now. This patch does exactly what you're arguing against in the rest of your mail, and you still don't provide a pointer to Ulf's patch. Would you mind *explaining* to me what your plan is? I just can't get it.
something like this for assembly
.macro set_led \num, \state call (c or assembly) set_led_state num state .endm
.macore set_led_red \state call (c or assembly) set_led_state CONFIG_SYS_LED_RED state .endm
and for c
void set_led(int num, int state) { .... }
void set_reg_led(int state) { set_led(CONFIG_SYS_LED_RED, state); }
etc... for all colour
Ok. So what about implementing the current, existing LED API ? you'd get:
void status_led_set (int led, int state); that sound a lot like your set_led function. And then
status_led_set(STATUS_LED_{YELLOW,BLUE,RED}, state)
which is basically the same as your set_reg_led?
so you use this one instead
Best Regards, J.
participants (3)
-
Albin Tonnerre -
Jean-Christophe PLAGNIOL-VILLARD -
Wolfgang Denk