Dear Jean-Christophe PLAGNIOL-VILLARD,
In message 1245613942-10693-1-git-send-email-plagnioj@jcrosoft.com you wrote:
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
common/lcd.c | 23 ++++++++++++++++------- 1 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/common/lcd.c b/common/lcd.c index 74a5c77..b5e81f1 100644 --- 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.
+static int lcd_clear(int draw_logo) { #if LCD_BPP == LCD_MONOCHROME /* Setting the palette */ @@ -414,9 +419,13 @@ static int lcd_clear (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) COLOR_MASK(lcd_getbgcolor()), lcd_line_length*panel_info.vl_row); #endif
- /* 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.
Please rework, and resubmit. Thanks.
Best regards,
Wolfgang Denk