[U-Boot] [PATCH 1/2] lcd: allow to not draw the logo when clearing the screen

[Wolfgang Denk]

Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <20090704222746.GE30172 at 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

-- 
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
That was the thing about deserts. They had their  own  gravity.  They
sucked you into the centre.           - Terry Pratchett, _Small Gods_