[U-Boot] [PATCH v2 4/7] Serial: ns16550: Add support for CONFIG_SYS_NS16550_IER macro

[Prafulla Wadaskar]

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de]
> Sent: Wednesday, December 01, 2010 1:15 PM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Eric Miao; Manas Saksena; Lei Wen; Yu Tang;
> Ashish Karkare; Kiran Vedere; Prabhanjan Sarnaik
> Subject: Re: [U-Boot] [PATCH v2 4/7] Serial: ns16550: Add support for
> CONFIG_SYS_NS16550_IER macro
> 
> Dear Prafulla Wadaskar,
> 
> In message <1291114965-17100-5-git-send-email-prafulla at marvell.com> you
> wrote:
> > On some processors this ier register configuration is different
> > for ex. Marvell Armada100
> >
> > This patch introduce CONFIG_SYS_NS16550_IER macro support to
> > unconditionally initialize this register.
> 
> Sorry, but I don't like an implementation detail:
> 
> >  void NS16550_init (NS16550_t com_port, int baud_divisor)
> >  {
> > -	serial_out(0x00, &com_port->ier);
> > +	serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier);
> >  #if defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)
> >  	serial_out(0x7, &com_port->mdr1);	/* mode select reset TL16C750*/
> >  #endif
> > @@ -52,7 +52,7 @@ void NS16550_init (NS16550_t com_port, int
> baud_divisor)
> >  #ifndef CONFIG_NS16550_MIN_FUNCTIONS
> >  void NS16550_reinit (NS16550_t com_port, int baud_divisor)
> >  {
> > -	serial_out(0x00, &com_port->ier);
> > +	serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier);
> >  	serial_out(UART_LCR_BKSE | UART_LCRVAL, &com_port->lcr);
> >  	serial_out(0, &com_port->dll);
> >  	serial_out(0, &com_port->dlm);
> > diff --git a/include/ns16550.h b/include/ns16550.h
> > index 9ea81e9..e9cb449 100644
> > --- a/include/ns16550.h
> > +++ b/include/ns16550.h
> > @@ -33,6 +33,10 @@
> >  	unsigned char postpad_##x[-CONFIG_SYS_NS16550_REG_SIZE - 1];
> >  #endif
> >
> > +#ifndef CONFIG_SYS_NS16550_IER
> > +#define CONFIG_SYS_NS16550_IER	0x00
> > +#endif /* CONFIG_SYS_NS16550_IER */
> 
> As this macro is only used in drivers/serial/ns16550.c, it makes no
> sense to me to move this to ns16550.h - this makes the code just
> harder to read.  Please move this #ifndef... to
> drivers/serial/ns16550.c

Ack, will be done in v3

> 
> Also, please add a description of the new CONFIG_ variable to the
> README file.

Ack,

Thanks and regards..
Prafulla . .