| Submitter | Prafulla Wadaskar |
|---|---|
| Date | Nov. 30, 2010, 11:02 a.m. |
| Message ID | <1291114965-17100-5-git-send-email-prafulla@marvell.com> |
| Download | mbox | patch |
| Permalink | /patch/73569/ |
| State | Changes Requested |
| Headers | show |
Comments
Dear Prafulla Wadaskar, In message <1291114965-17100-5-git-send-email-prafulla@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 Also, please add a description of the new CONFIG_ variable to the README file. Thanks. Best regards, Wolfgang Denk
> -----Original Message----- > From: Wolfgang Denk [mailto:wd@denx.de] > Sent: Wednesday, December 01, 2010 1:15 PM > To: Prafulla Wadaskar > Cc: u-boot@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@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 . .
Patch
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 32f24de..8dcfcb9 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -26,7 +26,7 @@ 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 */ + struct NS16550 { UART_REG(rbr); /* 0 */ UART_REG(ier); /* 1 */
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. Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com> --- drivers/serial/ns16550.c | 4 ++-- include/ns16550.h | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-)