[U-Boot] [PATCH 1/1] at91: Add command to control up to 3 GPIO LEDs from the console

[Wolfgang Denk]

Dear Daniel Gorsulowski,

In message <4A02792B.8060102 at esd.eu> you wrote:
> 
> > Ummm... common is for, well, for >>common<< stuff. If this code is
> > specific to AT91 only, it should not go into common.
> > 
> IMHO this code is not specific to AT91 only. Well, other architectures does not
> support the CONFIG_CMD_LED yet, but they could be implemented later.

This is not quite correct. Actually several boards already support
this, or very similar functions:

board/amcc/taihu/taihu.c:static int do_led_ctl(cmd_tbl_t* cmd_tp, int flags, int argc, char *argv[])
board/amcc/taishan/lcd.c:static int do_led_test_off(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
board/amcc/taishan/lcd.c:static int do_led_test_on(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
board/cm5200/cmd_cm5200.c:int do_led(char *argv[])
board/pcs440ep/pcs440ep.c:int do_led (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
board/pn62/cmd_pn62.c:int do_led (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
board/tqc/tqm5200/cmd_stk52xx.c:int do_led(char *argv[])
board/trab/trab_fkt.c:int do_led (char **);
board/trab/trab_fkt.c:int do_led (char **argv)


If the code is really generic, then probably it should merge some (or
all?) of the existing implementations, too.

> >> +++ b/include/asm-arm/arch-at91/led.h
> >> @@ -0,0 +1,52 @@
> >> +/*
> >> + * (C) Copyright 2000-2004
> >> + * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> > 
> > You claim that I have written any of this code?  I decline.
> > 
> I took that structure from include/status_led.h, so i picked you up to the list.
> But if you mean, I'll remove these lines.

I did not write this code:

-> git-blame include/status_led.h
...
de74b9ee (Wolfgang Denk    2007-10-13 21:15:39 +0200 389) /*
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 390)  * Coloured LEDs API
de74b9ee (Wolfgang Denk    2007-10-13 21:15:39 +0200 391)  */
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 392) #ifndef       __ASSEMBLY__
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 393) extern void   coloured_LED_init (void);
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 394) extern void   red_LED_on(void);
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 395) extern void   red_LED_off(void);
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 396) extern void   green_LED_on(void);
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 397) extern void   green_LED_off(void);
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 398) extern void   yellow_LED_on(void);
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 399) extern void   yellow_LED_off(void);
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 400) #else
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 401)       .extern LED_init
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 402)       .extern red_LED_on
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 403)       .extern red_LED_off
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 404)       .extern yellow_LED_on
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 405)       .extern yellow_LED_off
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 406)       .extern green_LED_on
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 407)       .extern green_LED_off
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 408) #endif
bd86220f (Peter Pearse     2007-09-18 13:07:54 +0100 409) 
c609719b (wdenk            2002-11-03 00:24:07 +0000 410) #endif        /* CONFIG_STATUS_LED    */
...

As you can see, this stuff was added in commit bd86220f by Peter Pearse.


But... if this is already present in include/status_led.h, then why do
you have to copy the code? Don't do that! Use the existing include
file instead.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Making files is easy under  the  UNIX  operating  system.  Therefore,
users  tend  to  create  numerous  files  using large amounts of file
space. It has been said that the only standard thing about  all  UNIX
systems  is  the  message-of-the-day  telling users to clean up their
files.                             - System V.2 administrator's guide