[U-Boot] [PATCH] Enable port-mapped access to 16550 UART

[U-Boot] [PATCH] Enable port-mapped access to 16550 UART

[Graeme Russ]

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

[U-Boot] [PING][PATCH] Enable port-mapped access to 16550 UART

[Graeme Russ]

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
>
>

[U-Boot] [PATCH] Enable port-mapped access to 16550 UART

[Detlev Zundel]

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

-- 
More than any other time in history, mankind faces a crossroads.  One
path leads  to despair  and utter  hopelessness.   The other to total
extinction.  Let us pray, we have the wisdom to choose correctly.
                                        -- Woody Allen
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] Enable port-mapped access to 16550 UART

[Graeme Russ]

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

[U-Boot] [PATCH] Enable port-mapped access to 16550 UART

[Detlev Zundel]

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

-- 
Spelling is a lossed art
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] Enable port-mapped access to 16550 UART

[Graeme Russ]

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

[U-Boot] [PATCH] Enable port-mapped access to 16550 UART

[Detlev Zundel]

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

-- 
The fact is, volatile on data structures is a bug. It's a wart in the C 
language. It shouldn't be used.
                                   -- Linus Torvalds 
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de