Dear Jean-Christophe PLAGNIOL-VILLARD,
In message 20090704222746.GE30172@game.jcrosoft.org you wrote:
--- a/common/lcd.c +++ b/common/lcd.c @@ -78,7 +78,7 @@ static inline void lcd_putc_xy (ushort x, ushort y, uchar c);
static int lcd_init (void *lcdbase);
-static int lcd_clear (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]); +static int lcd_clear (int draw_logo); extern void lcd_ctrl_init (void *lcdbase); extern void lcd_enable (void); static void *lcd_logo (void); @@ -379,7 +379,12 @@ int drv_lcd_init (void) }
/*----------------------------------------------------------------------*/ -static int lcd_clear (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) +static int do_lcd_clear (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) +{
- return lcd_clear(1);
+}
Given the fact that we have exactly one place only where we call lcd_clear() [in lcd_init()], this just adds complexity and memory footprint without adding benefits. Please omit this part.
use the patch 2/2 for the ainsi support
I have no idea what "ainsi" is. Do you mean ANSI?
Also, patches have to be orthogonal. Thi scode is not needed (nor useful) here, so omit it here. It it is needed in some other patch, then please add it there.
- /* Paint the logo and retrieve LCD base address */
- debug ("[LCD] Drawing the logo...\n");
- lcd_console_address = lcd_logo ();
- if (draw_logo) {
/* Paint the logo and retrieve LCD base address */debug ("[LCD] Drawing the logo...\n");lcd_console_address = lcd_logo ();- } else {
lcd_console_address = lcd_base;- }
I wonder if there is any user out there who expects that the "cls" command will also draw the logo - I am pretty sure that this is actually a bug introduced when the lcd_clear() code was extracted from lcd_init() in commit 8655b6f8 nearly 5 years ago.
To me the current implementation makes no sense. I suggest to rather fix the origona bug, i. e. move the lcd_logo() call into lcd_init() so it gets done only once, and not when running the lcd_clear() function using the "cls" commandline interface.
so I prefer to remove the cls command and introduce the clear command which will be availlable for all output serial & lcd
Add a new command instead of fixing an (obvious?) bug? This makes no sense to me. Please rather fix the real bug.
Best regards,
Wolfgang Denk