| Message ID | 1418653194-16873-19-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Stefan Roese |
| Headers | show |
Hi Simon, On Mon, 15 Dec 2014 07:19:53 -0700 Simon Glass <sjg@chromium.org> wrote: > +#ifdef CONFIG_DEBUG_UART_NS16550 > +void debug_uart_init(void) > +{ > + struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE; > + int baud_divisor; > + > + baud_divisor = calc_divisor(com_port, CONFIG_DEBUG_UART_CLOCK, > + CONFIG_BAUDRATE); > + > + writeb(CONFIG_SYS_NS16550_IER, &com_port->ier); > + writeb(UART_MCRVAL, &com_port->mcr); > + writeb(UART_FCRVAL, &com_port->fcr); > + > + writeb(UART_LCR_BKSE | UART_LCRVAL, &com_port->lcr); > + writeb(baud_divisor & 0xff, &com_port->dll); > + writeb((baud_divisor >> 8) & 0xff, &com_port->dlm); > + writeb(UART_LCRVAL, &com_port->lcr); > +} This function is duplicating NS16550_init() and NS16550_setbrg(). > +static inline void _debug_uart_putc(int ch) > +{ > + struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE; > + > + while (!(readb(&com_port->lsr) & UART_LSR_THRE)) > + ; > + writeb(ch, &com_port->thr); > +} This is duplicating ns16550_serial_putc(). What does this patch save? It looks like the driver code is just getting dirtier. Best Regards Masahiro Yamada
Hi Masahiro, On 15 December 2014 at 21:39, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote: > Hi Simon, > > > On Mon, 15 Dec 2014 07:19:53 -0700 > Simon Glass <sjg@chromium.org> wrote: > >> +#ifdef CONFIG_DEBUG_UART_NS16550 >> +void debug_uart_init(void) >> +{ >> + struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE; >> + int baud_divisor; >> + >> + baud_divisor = calc_divisor(com_port, CONFIG_DEBUG_UART_CLOCK, >> + CONFIG_BAUDRATE); >> + >> + writeb(CONFIG_SYS_NS16550_IER, &com_port->ier); >> + writeb(UART_MCRVAL, &com_port->mcr); >> + writeb(UART_FCRVAL, &com_port->fcr); >> + >> + writeb(UART_LCR_BKSE | UART_LCRVAL, &com_port->lcr); >> + writeb(baud_divisor & 0xff, &com_port->dll); >> + writeb((baud_divisor >> 8) & 0xff, &com_port->dlm); >> + writeb(UART_LCRVAL, &com_port->lcr); >> +} > > This function is duplicating NS16550_init() and NS16550_setbrg(). Yes I should fix that. > > > >> +static inline void _debug_uart_putc(int ch) >> +{ >> + struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE; >> + >> + while (!(readb(&com_port->lsr) & UART_LSR_THRE)) >> + ; >> + writeb(ch, &com_port->thr); >> +} > > This is duplicating ns16550_serial_putc(). Ditto. > > > > What does this patch save? > > It looks like the driver code is just getting dirtier. See my other patch for an explanation. We could add this feature to any serial driver. Regards, Simon
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index 268bb42..9d0d4c3 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -31,6 +31,19 @@ config DEBUG_UART serial drivers are up and running (done in serial_init()). Otherwise the drivers may conflict and you will get strange output. +choice + prompt "Select which UART will provide the debug UART" + depends on DEBUG_UART + +config DEBUG_UART_NS16550 + bool "ns16550" + help + Select this to enable a debug UART using the ns16550 driver. You + will need to provide parameters to make this work. The driver will + be available until the real driver model serial is running. + +endchoice + config DEBUG_UART_BASE hex "Base address of UART" depends on DEBUG_UART diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index af5beba..b04c7d0 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -5,6 +5,7 @@ */ #include <common.h> +#include <debug_uart.h> #include <dm.h> #include <errno.h> #include <fdtdec.h> @@ -106,10 +107,15 @@ static int ns16550_readb(NS16550_t port, int offset) ns16550_readb(com_port, addr - (unsigned char *)com_port) #endif -int ns16550_calc_divisor(NS16550_t port, int clock, int baudrate) +static inline int calc_divisor(NS16550_t port, int clock, int baudrate) { const unsigned int mode_x_div = 16; + return DIV_ROUND_CLOSEST(clock, mode_x_div * baudrate); +} + +int ns16550_calc_divisor(NS16550_t port, int clock, int baudrate) +{ #ifdef CONFIG_OMAP1510 /* If can't cleanly clock 115200 set div to 1 */ if ((clock == 12000000) && (baudrate == 115200)) { @@ -119,7 +125,7 @@ int ns16550_calc_divisor(NS16550_t port, int clock, int baudrate) port->osc_12m_sel = 0; /* clear if previsouly set */ #endif - return DIV_ROUND_CLOSEST(clock, mode_x_div * baudrate); + return calc_divisor(port, clock, baudrate); } static void NS16550_setbrg(NS16550_t com_port, int baud_divisor) @@ -219,6 +225,38 @@ int NS16550_tstc(NS16550_t com_port) #endif /* CONFIG_NS16550_MIN_FUNCTIONS */ +#ifdef CONFIG_DEBUG_UART_NS16550 +void debug_uart_init(void) +{ + struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE; + int baud_divisor; + + baud_divisor = calc_divisor(com_port, CONFIG_DEBUG_UART_CLOCK, + CONFIG_BAUDRATE); + + writeb(CONFIG_SYS_NS16550_IER, &com_port->ier); + writeb(UART_MCRVAL, &com_port->mcr); + writeb(UART_FCRVAL, &com_port->fcr); + + writeb(UART_LCR_BKSE | UART_LCRVAL, &com_port->lcr); + writeb(baud_divisor & 0xff, &com_port->dll); + writeb((baud_divisor >> 8) & 0xff, &com_port->dlm); + writeb(UART_LCRVAL, &com_port->lcr); +} + +static inline void _debug_uart_putc(int ch) +{ + struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE; + + while (!(readb(&com_port->lsr) & UART_LSR_THRE)) + ; + writeb(ch, &com_port->thr); +} + +DEBUG_UART_FUNCS + +#endif + #ifdef CONFIG_DM_SERIAL static int ns16550_serial_putc(struct udevice *dev, const char ch) {
Add debug UART functions to permit ns16550 to provide an early debug UART. Try to avoid using the stack so that this can be called from assembler before a stack is set up (at least on ARM and PowerPC). Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/serial/Kconfig | 13 +++++++++++++ drivers/serial/ns16550.c | 42 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 2 deletions(-)