[U-Boot] [PATCH] Enable port-mapped access to 16550 UART
This patch does two things: - Changes default behaviour to use proper memory accessors - Allows port-mapped access (using inb/outb) for the x86 architecture
Signed-off-by: Graeme Russ graeme.russ@gmail.com --- drivers/serial/ns16550.c | 69 ++++++++++++++++++++++++++-------------------- 1 files changed, 39 insertions(+), 30 deletions(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 2fcc8c3..c41ca0d 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -6,6 +6,8 @@
#include <config.h> #include <ns16550.h> +#include <linux/types.h> +#include <asm/io.h>
#define UART_LCRVAL UART_LCR_8N1 /* 8 data, 1 stop, no parity */ #define UART_MCRVAL (UART_MCR_DTR | \ @@ -13,28 +15,35 @@ #define UART_FCRVAL (UART_FCR_FIFO_EN | \ UART_FCR_RXSR | \ UART_FCR_TXSR) /* Clear & enable FIFOs */ +#ifdef CONFIG_X86 +#define uart_writeb(x,y) outb(x,(ulong)y) +#define uart_readb(y) inb((ulong)y) +#else +#define uart_writeb(x,y) writeb(x,y) +#define uart_readb(y) readb(y) +#endif
void NS16550_init (NS16550_t com_port, int baud_divisor) { - com_port->ier = 0x00; + uart_writeb(0x00, &com_port->ier); #if defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2) - com_port->mdr1 = 0x7; /* mode select reset TL16C750*/ + uart_writeb(0x7, &com_port->mdr1); /* mode select reset TL16C750*/ #endif - com_port->lcr = UART_LCR_BKSE | UART_LCRVAL; - com_port->dll = 0; - com_port->dlm = 0; - com_port->lcr = UART_LCRVAL; - com_port->mcr = UART_MCRVAL; - com_port->fcr = UART_FCRVAL; - com_port->lcr = UART_LCR_BKSE | UART_LCRVAL; - com_port->dll = baud_divisor & 0xff; - com_port->dlm = (baud_divisor >> 8) & 0xff; - com_port->lcr = UART_LCRVAL; + uart_writeb(UART_LCR_BKSE | UART_LCRVAL, (ulong)&com_port->lcr); + uart_writeb(0, &com_port->dll); + uart_writeb(0, &com_port->dlm); + uart_writeb(UART_LCRVAL, &com_port->lcr); + uart_writeb(UART_MCRVAL, &com_port->mcr); + uart_writeb(UART_FCRVAL, &com_port->fcr); + uart_writeb(UART_LCR_BKSE | UART_LCRVAL, &com_port->lcr); + uart_writeb(baud_divisor & 0xff, &com_port->dll); + uart_writeb((baud_divisor >> 8) & 0xff, &com_port->dlm); + uart_writeb(UART_LCRVAL, &com_port->lcr); #if defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2) #if defined(CONFIG_APTIX) - com_port->mdr1 = 3; /* /13 mode so Aptix 6MHz can hit 115200 */ + uart_writeb(3, &com_port->mdr1); /* /13 mode so Aptix 6MHz can hit 115200 */ #else - com_port->mdr1 = 0; /* /16 is proper to hit 115200 with 48MHz */ + uart_writeb(0, &com_port->mdr1); /* /16 is proper to hit 115200 with 48MHz */ #endif #endif /* CONFIG_OMAP */ } @@ -42,41 +51,41 @@ 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) { - com_port->ier = 0x00; - com_port->lcr = UART_LCR_BKSE | UART_LCRVAL; - com_port->dll = 0; - com_port->dlm = 0; - com_port->lcr = UART_LCRVAL; - com_port->mcr = UART_MCRVAL; - com_port->fcr = UART_FCRVAL; - com_port->lcr = UART_LCR_BKSE; - com_port->dll = baud_divisor & 0xff; - com_port->dlm = (baud_divisor >> 8) & 0xff; - com_port->lcr = UART_LCRVAL; + uart_writeb(0x00, &com_port->ier); + uart_writeb(UART_LCR_BKSE | UART_LCRVAL, &com_port->lcr); + uart_writeb(0, &com_port->dll); + uart_writeb(0, &com_port->dlm); + uart_writeb(UART_LCRVAL, &com_port->lcr); + uart_writeb(UART_MCRVAL, &com_port->mcr); + uart_writeb(UART_FCRVAL, &com_port->fcr); + uart_writeb(UART_LCR_BKSE, &com_port->lcr); + uart_writeb(baud_divisor & 0xff, &com_port->dll); + uart_writeb((baud_divisor >> 8) & 0xff, &com_port->dlm); + uart_writeb(UART_LCRVAL, &com_port->lcr); } #endif /* CONFIG_NS16550_MIN_FUNCTIONS */
void NS16550_putc (NS16550_t com_port, char c) { - while ((com_port->lsr & UART_LSR_THRE) == 0); - com_port->thr = c; + while ((uart_readb(&com_port->lsr) & UART_LSR_THRE) == 0); + uart_writeb(c, &com_port->thr); }
#ifndef CONFIG_NS16550_MIN_FUNCTIONS char NS16550_getc (NS16550_t com_port) { - while ((com_port->lsr & UART_LSR_DR) == 0) { + while ((uart_readb(&com_port->lsr) & UART_LSR_DR) == 0) { #ifdef CONFIG_USB_TTY extern void usbtty_poll(void); usbtty_poll(); #endif } - return (com_port->rbr); + return uart_readb(&com_port->rbr); }
int NS16550_tstc (NS16550_t com_port) { - return ((com_port->lsr & UART_LSR_DR) != 0); + return ((uart_readb(&com_port->lsr) & UART_LSR_DR) != 0); }
#endif /* CONFIG_NS16550_MIN_FUNCTIONS */
Has anyone had a chance to look at this?
This patch is a pre-requisite for converting my x86 board to use this driver (and CONFIG_SERIAL_MULTI) and droping one more redundant source file from the u-boot tree (cpu/i386/serial.c)
Regards,
Graeme
On Sat, Oct 24, 2009 at 12:27 PM, Graeme Russ graeme.russ@gmail.com wrote:
This patch does two things:
- Changes default behaviour to use proper memory accessors
- Allows port-mapped access (using inb/outb) for the x86 architecture
Signed-off-by: Graeme Russ graeme.russ@gmail.com
drivers/serial/ns16550.c | 69 ++++++++++++++++++++++++++-------------------- 1 files changed, 39 insertions(+), 30 deletions(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 2fcc8c3..c41ca0d 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -6,6 +6,8 @@
#include <config.h> #include <ns16550.h> +#include <linux/types.h> +#include <asm/io.h>
#define UART_LCRVAL UART_LCR_8N1 /* 8 data, 1 stop, no parity */ #define UART_MCRVAL (UART_MCR_DTR | \ @@ -13,28 +15,35 @@ #define UART_FCRVAL (UART_FCR_FIFO_EN | \ UART_FCR_RXSR | \ UART_FCR_TXSR) /* Clear & enable FIFOs */ +#ifdef CONFIG_X86 +#define uart_writeb(x,y) outb(x,(ulong)y) +#define uart_readb(y) inb((ulong)y) +#else +#define uart_writeb(x,y) writeb(x,y) +#define uart_readb(y) readb(y) +#endif
void NS16550_init (NS16550_t com_port, int baud_divisor) {
com_port->ier = 0x00;
uart_writeb(0x00, &com_port->ier);#if defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)
com_port->mdr1 = 0x7; /* mode select reset TL16C750*/
uart_writeb(0x7, &com_port->mdr1); /* mode select reset TL16C750*/#endif
com_port->lcr = UART_LCR_BKSE | UART_LCRVAL;com_port->dll = 0;com_port->dlm = 0;com_port->lcr = UART_LCRVAL;com_port->mcr = UART_MCRVAL;com_port->fcr = UART_FCRVAL;com_port->lcr = UART_LCR_BKSE | UART_LCRVAL;com_port->dll = baud_divisor & 0xff;com_port->dlm = (baud_divisor >> 8) & 0xff;com_port->lcr = UART_LCRVAL;
uart_writeb(UART_LCR_BKSE | UART_LCRVAL, (ulong)&com_port->lcr);uart_writeb(0, &com_port->dll);uart_writeb(0, &com_port->dlm);uart_writeb(UART_LCRVAL, &com_port->lcr);uart_writeb(UART_MCRVAL, &com_port->mcr);uart_writeb(UART_FCRVAL, &com_port->fcr);uart_writeb(UART_LCR_BKSE | UART_LCRVAL, &com_port->lcr);uart_writeb(baud_divisor & 0xff, &com_port->dll);uart_writeb((baud_divisor >> 8) & 0xff, &com_port->dlm);uart_writeb(UART_LCRVAL, &com_port->lcr);#if defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2) #if defined(CONFIG_APTIX)
com_port->mdr1 = 3; /* /13 mode so Aptix 6MHz can hit 115200 */
uart_writeb(3, &com_port->mdr1); /* /13 mode so Aptix 6MHz can hit 115200 */#else
com_port->mdr1 = 0; /* /16 is proper to hit 115200 with 48MHz */
uart_writeb(0, &com_port->mdr1); /* /16 is proper to hit 115200 with 48MHz */#endif #endif /* CONFIG_OMAP */ } @@ -42,41 +51,41 @@ 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) {
com_port->ier = 0x00;com_port->lcr = UART_LCR_BKSE | UART_LCRVAL;com_port->dll = 0;com_port->dlm = 0;com_port->lcr = UART_LCRVAL;com_port->mcr = UART_MCRVAL;com_port->fcr = UART_FCRVAL;com_port->lcr = UART_LCR_BKSE;com_port->dll = baud_divisor & 0xff;com_port->dlm = (baud_divisor >> 8) & 0xff;com_port->lcr = UART_LCRVAL;
uart_writeb(0x00, &com_port->ier);uart_writeb(UART_LCR_BKSE | UART_LCRVAL, &com_port->lcr);uart_writeb(0, &com_port->dll);uart_writeb(0, &com_port->dlm);uart_writeb(UART_LCRVAL, &com_port->lcr);uart_writeb(UART_MCRVAL, &com_port->mcr);uart_writeb(UART_FCRVAL, &com_port->fcr);uart_writeb(UART_LCR_BKSE, &com_port->lcr);uart_writeb(baud_divisor & 0xff, &com_port->dll);uart_writeb((baud_divisor >> 8) & 0xff, &com_port->dlm);uart_writeb(UART_LCRVAL, &com_port->lcr);} #endif /* CONFIG_NS16550_MIN_FUNCTIONS */
void NS16550_putc (NS16550_t com_port, char c) {
while ((com_port->lsr & UART_LSR_THRE) == 0);com_port->thr = c;
while ((uart_readb(&com_port->lsr) & UART_LSR_THRE) == 0);uart_writeb(c, &com_port->thr);}
#ifndef CONFIG_NS16550_MIN_FUNCTIONS char NS16550_getc (NS16550_t com_port) {
while ((com_port->lsr & UART_LSR_DR) == 0) {
while ((uart_readb(&com_port->lsr) & UART_LSR_DR) == 0) {#ifdef CONFIG_USB_TTY extern void usbtty_poll(void); usbtty_poll(); #endif }
return (com_port->rbr);
return uart_readb(&com_port->rbr);}
int NS16550_tstc (NS16550_t com_port) {
return ((com_port->lsr & UART_LSR_DR) != 0);
return ((uart_readb(&com_port->lsr) & UART_LSR_DR) != 0);}
#endif /* CONFIG_NS16550_MIN_FUNCTIONS */
1.6.4.1.174.g32f4c
Hello Graeme,
This patch does two things:
- Changes default behaviour to use proper memory accessors
- Allows port-mapped access (using inb/outb) for the x86 architecture
Signed-off-by: Graeme Russ graeme.russ@gmail.com
drivers/serial/ns16550.c | 69 ++++++++++++++++++++++++++-------------------- 1 files changed, 39 insertions(+), 30 deletions(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 2fcc8c3..c41ca0d 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -6,6 +6,8 @@
#include <config.h> #include <ns16550.h> +#include <linux/types.h> +#include <asm/io.h>
#define UART_LCRVAL UART_LCR_8N1 /* 8 data, 1 stop, no parity */ #define UART_MCRVAL (UART_MCR_DTR | \ @@ -13,28 +15,35 @@ #define UART_FCRVAL (UART_FCR_FIFO_EN | \ UART_FCR_RXSR | \ UART_FCR_TXSR) /* Clear & enable FIFOs */ +#ifdef CONFIG_X86 +#define uart_writeb(x,y) outb(x,(ulong)y) +#define uart_readb(y) inb((ulong)y) +#else +#define uart_writeb(x,y) writeb(x,y) +#define uart_readb(y) readb(y) +#endif
Why do you need a specific variant for X86 instead of implementing writeb and readb correctly in the first place?
If this was in place, all the accessors should only switch to using readb/writeb and from looking at it, this should not brak e.g. PowerPC boards with weird register layouts.
When you post a patch with only these changes, I'll test it on a few of the usual suspects on PowerPC.
Cheers Detlev
Detlev,
Thanks for havnig a look at this
On Thu, Oct 29, 2009 at 3:13 AM, Detlev Zundel dzu@denx.de wrote:
Hello Graeme,
This patch does two things:
- Changes default behaviour to use proper memory accessors
- Allows port-mapped access (using inb/outb) for the x86 architecture
Signed-off-by: Graeme Russ graeme.russ@gmail.com
drivers/serial/ns16550.c | 69 ++++++++++++++++++++++++++-------------------- 1 files changed, 39 insertions(+), 30 deletions(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 2fcc8c3..c41ca0d 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -6,6 +6,8 @@
#include <config.h> #include <ns16550.h> +#include <linux/types.h> +#include <asm/io.h>
#define UART_LCRVAL UART_LCR_8N1 /* 8 data, 1 stop, no parity */ #define UART_MCRVAL (UART_MCR_DTR | \ @@ -13,28 +15,35 @@ #define UART_FCRVAL (UART_FCR_FIFO_EN | \ UART_FCR_RXSR | \ UART_FCR_TXSR) /* Clear & enable FIFOs */ +#ifdef CONFIG_X86 +#define uart_writeb(x,y) outb(x,(ulong)y) +#define uart_readb(y) inb((ulong)y) +#else +#define uart_writeb(x,y) writeb(x,y) +#define uart_readb(y) readb(y) +#endif
Why do you need a specific variant for X86 instead of implementing writeb and readb correctly in the first place?
For x86 readb and writeb provide volatile accessors to memory - These are used for memory-mapped devices (i.e. devices which are attached directly to the memory bus such as PCI devices etc). inb and outb provide access to I/O Ports. For example:
writeb(0x12, 0x00001000) will generate something like: movb $0x12, al movl $0x00001000, ebx movb al, ebx
outb(0x12, 0x00001000) will generate something like: movb $0x12, al movl $0x00001000, ebx outb al, ebx
Looking at include/asm/asm-ppc/io.h it seems to me that, for PPC, there is no differentiation between readb/writeb and inb/outb other than that the user may define an optional IOBASE for inb/outb which shifts where in memory inb/outb accesses, but they are still memory accesses. So, for PPC, if IOBASE is 0, the above two examples will compile to identical code.
(Having a look at the other arches, it appears that x86 is very unique in that inb/outb do not access memory)
If this was in place, all the accessors should only switch to using readb/writeb and from looking at it, this should not brak e.g. PowerPC boards with weird register layouts.
This patch should not change the behaviour for non x86 boards other than to use proper I/O accessors which prevents any risk that gcc will optimise them into some other order (or completely out)
For x86, this patch causes the driver to use I/O rather than memory which is how PC UARTS (and interrupt controllers and nearly anything else which is not 'memory') have always been accessed
When you post a patch with only these changes, I'll test it on a few of the usual suspects on PowerPC.
The only other option is my even uglier first patch:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/70250
Cheers Detlev
Regards,
Graeme
Hi Graeme,
[...]
+#ifdef CONFIG_X86 +#define uart_writeb(x,y) outb(x,(ulong)y) +#define uart_readb(y) inb((ulong)y) +#else +#define uart_writeb(x,y) writeb(x,y) +#define uart_readb(y) readb(y) +#endif
Why do you need a specific variant for X86 instead of implementing writeb and readb correctly in the first place?
For x86 readb and writeb provide volatile accessors to memory - These are used for memory-mapped devices (i.e. devices which are attached directly to the memory bus such as PCI devices etc). inb and outb provide access to I/O Ports. For example:
writeb(0x12, 0x00001000) will generate something like: movb $0x12, al movl $0x00001000, ebx movb al, ebx
outb(0x12, 0x00001000) will generate something like: movb $0x12, al movl $0x00001000, ebx outb al, ebx
Looking at include/asm/asm-ppc/io.h it seems to me that, for PPC, there is no differentiation between readb/writeb and inb/outb other than that the user may define an optional IOBASE for inb/outb which shifts where in memory inb/outb accesses, but they are still memory accesses. So, for PPC, if IOBASE is 0, the above two examples will compile to identical code.
(Having a look at the other arches, it appears that x86 is very unique in that inb/outb do not access memory)
Ok, I remember this icky in/out stuff from x86 now that you come to mention it.
So it seems we have to keep the distinction somehow. Looking at drivers/serial/8250.c to see what Linux does, we could start including something like the "port.iotype" layer from there, although I feel this is somewhat too heavy currently. So in the meantime, I'd suggest that we at least start using the Linux convention and turn all the register accesses into "serial_{in,out}" and define these for X86 and !X86 like you did.
This way should be somewhat clearer than defining a "writeb" not to be a writeb after all, which I find confusing.
What do you think?
Cheers Detlev
On Fri, Oct 30, 2009 at 2:42 AM, Detlev Zundel dzu@denx.de wrote:
Hi Graeme,
[...]
+#ifdef CONFIG_X86 +#define uart_writeb(x,y) outb(x,(ulong)y) +#define uart_readb(y) inb((ulong)y) +#else +#define uart_writeb(x,y) writeb(x,y) +#define uart_readb(y) readb(y) +#endif
Why do you need a specific variant for X86 instead of implementing writeb and readb correctly in the first place?
For x86 readb and writeb provide volatile accessors to memory - These are used for memory-mapped devices (i.e. devices which are attached directly to the memory bus such as PCI devices etc). inb and outb provide access to I/O Ports. For example:
writeb(0x12, 0x00001000) will generate something like: movb $0x12, al movl $0x00001000, ebx movb al, ebx
outb(0x12, 0x00001000) will generate something like: movb $0x12, al movl $0x00001000, ebx outb al, ebx
Looking at include/asm/asm-ppc/io.h it seems to me that, for PPC, there is no differentiation between readb/writeb and inb/outb other than that the user may define an optional IOBASE for inb/outb which shifts where in memory inb/outb accesses, but they are still memory accesses. So, for PPC, if IOBASE is 0, the above two examples will compile to identical code.
(Having a look at the other arches, it appears that x86 is very unique in that inb/outb do not access memory)
Ok, I remember this icky in/out stuff from x86 now that you come to mention it.
So it seems we have to keep the distinction somehow. Looking at drivers/serial/8250.c to see what Linux does, we could start including something like the "port.iotype" layer from there, although I feel this is somewhat too heavy currently. So in the meantime, I'd suggest that
Yes, I just had a look now - Although very elegant, it is very heavy
we at least start using the Linux convention and turn all the register accesses into "serial_{in,out}" and define these for X86 and !X86 like you did.
This way should be somewhat clearer than defining a "writeb" not to be a writeb after all, which I find confusing.
What do you think?
So essentially go with my patch but rename uart_writeb to serial_out and uart_readb to serial_in? If so, I'll re-spin
Cheers Detlev
Regards,
Graeme
Hi Graeme,
we at least start using the Linux convention and turn all the register accesses into "serial_{in,out}" and define these for X86 and !X86 like you did.
This way should be somewhat clearer than defining a "writeb" not to be a writeb after all, which I find confusing.
What do you think?
So essentially go with my patch but rename uart_writeb to serial_out and uart_readb to serial_in? If so, I'll re-spin
Yes please.
Thanks Detlev
participants (2)
-
Detlev Zundel -
Graeme Russ