On 4/30/07, John Williams <[EMAIL PROTECTED]> wrote:
Hi Peter,
The attached patch gets your uartlite driver going on MicroBlaze.
All readb/writeb ops are converted to ioread32/iowrite32.
On MicroBlaze readb/writeb are picking up the MSB, instead of LSB, and
thus reading all zeros instead of the 8-bit control/status/FIFO
registers that you intended.
Can you please confirm if this works on PPC?
Yes, I've confirmed this does work on PPC; but I don't think it's
quite the correct fix.
ioread/write32 is mapped to in/out_le32, yet the bootloader driver
must use in/out_be32. This is because the uartlite driver follows the
lead of 8250 and requires an offset of 3 from the base address in
order to find the relevant byte wise address. In fact, I believe the
driver should work as-is on microblaze if the offset-by-3 is not used
when registering it to the platform bus.
However, the uartlite is *not* an 8250. The 8250 turns up all over
the place and it's registers are defined as 8 bit wide. The
offset-by-3 stuff is part of the plat_serial8250_port structure which
is also used to specify .regshift (increment between registers).
Whereas the UARTLITE is defined as a 32 bit device and it doesn't show
up in anywhere near as many designs. Registers are always 4 bytes
wide and are always located at multiples of 4 bytes off the base
address.
The biggest problem with keeping the 3 byte offset and using
ioread/write32 on it makes every register access straddle a 32-bit
boundary. This means 2 bus transactions for every register access.
Absolutely not what we want.
The problem with keeping the byte-wise access as it is now is that it
means the platform bus binding needs to explicitly know what the host
access width is and add the 3 byte offset accordingly (rather than
using the base address as specified in xparameters unmodified and
using the in/out_be32 macro take care of reading it correctly &
efficiently).
(There are also annoyances that will come up when we move to
arch/powerpc and hook it up to the of_platform_bus)
I note that Grant's recent bootloader driver uses in_be32/out_be32 -
would you prefer that instead of ioread32/iowrite32?
I certainly think so. The device is documented as using 32 bit BE
registers; so the driver should access them as 32bit BE registers
IMHO. Or at least, if there is a good reason to continue the bytewise
access, then the driver should contain the smarts to translate from
documented base address to the appropriate offset.
So; starting with your patch and modifying it, I've attached I think
the change should be. It should work for microblaze, but I've only
tested w/ ppc. Unfortunately the (void*) casts are ugly; there might
be a way around that, but it's due to the type used for the (struct
uart_port)->membase variable.
Cheers,
g.
--
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
From cafac17d995fb2ced7102e8fbaad1f9bf3306b8b Mon Sep 17 00:00:00 2001
From: Grant Likely <[email protected]>
Date: Mon, 30 Apr 2007 23:53:03 -0600
Subject: [PATCH] [PPC] Fix UARTLITE register access for little-endian (microblaze) architectures
Signed-off-by: Grant Likely <[email protected]>
---
arch/ppc/syslib/virtex_devices.c | 2 +-
drivers/serial/uartlite.c | 32 ++++++++++++++++----------------
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/arch/ppc/syslib/virtex_devices.c b/arch/ppc/syslib/virtex_devices.c
index 52e2ebb..6c3e31b 100644
--- a/arch/ppc/syslib/virtex_devices.c
+++ b/arch/ppc/syslib/virtex_devices.c
@@ -31,7 +31,7 @@
.num_resources = 2, \
.resource = (struct resource[]) { \
{ \
- .start = XPAR_UARTLITE_##num##_BASEADDR + 3, \
+ .start = XPAR_UARTLITE_##num##_BASEADDR, \
.end = XPAR_UARTLITE_##num##_HIGHADDR, \
.flags = IORESOURCE_MEM, \
}, \
diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
index f5051cf..59b674a 100644
--- a/drivers/serial/uartlite.c
+++ b/drivers/serial/uartlite.c
@@ -61,7 +61,7 @@ static int ulite_receive(struct uart_port *port, int stat)
/* stats */
if (stat & ULITE_STATUS_RXVALID) {
port->icount.rx++;
- ch = readb(port->membase + ULITE_RX);
+ ch = in_be32((void*)port->membase + ULITE_RX);
if (stat & ULITE_STATUS_PARITY)
port->icount.parity++;
@@ -106,7 +106,7 @@ static int ulite_transmit(struct uart_port *port, int stat)
return 0;
if (port->x_char) {
- writeb(port->x_char, port->membase + ULITE_TX);
+ out_be32((void*)port->membase + ULITE_TX, port->x_char);
port->x_char = 0;
port->icount.tx++;
return 1;
@@ -115,7 +115,7 @@ static int ulite_transmit(struct uart_port *port, int stat)
if (uart_circ_empty(xmit) || uart_tx_stopped(port))
return 0;
- writeb(xmit->buf[xmit->tail], port->membase + ULITE_TX);
+ out_be32((void*)port->membase + ULITE_TX, xmit->buf[xmit->tail]);
xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE-1);
port->icount.tx++;
@@ -132,7 +132,7 @@ static irqreturn_t ulite_isr(int irq, void *dev_id)
int busy;
do {
- int stat = readb(port->membase + ULITE_STATUS);
+ int stat = in_be32((void*)port->membase + ULITE_STATUS);
busy = ulite_receive(port, stat);
busy |= ulite_transmit(port, stat);
} while (busy);
@@ -148,7 +148,7 @@ static unsigned int ulite_tx_empty(struct uart_port *port)
unsigned int ret;
spin_lock_irqsave(&port->lock, flags);
- ret = readb(port->membase + ULITE_STATUS);
+ ret = in_be32((void*)port->membase + ULITE_STATUS);
spin_unlock_irqrestore(&port->lock, flags);
return ret & ULITE_STATUS_TXEMPTY ? TIOCSER_TEMT : 0;
@@ -171,7 +171,7 @@ static void ulite_stop_tx(struct uart_port *port)
static void ulite_start_tx(struct uart_port *port)
{
- ulite_transmit(port, readb(port->membase + ULITE_STATUS));
+ ulite_transmit(port, in_be32((void*)port->membase + ULITE_STATUS));
}
static void ulite_stop_rx(struct uart_port *port)
@@ -200,17 +200,17 @@ static int ulite_startup(struct uart_port *port)
if (ret)
return ret;
- writeb(ULITE_CONTROL_RST_RX | ULITE_CONTROL_RST_TX,
- port->membase + ULITE_CONTROL);
- writeb(ULITE_CONTROL_IE, port->membase + ULITE_CONTROL);
+ out_be32((void*)port->membase + ULITE_CONTROL,
+ ULITE_CONTROL_RST_RX | ULITE_CONTROL_RST_TX);
+ out_be32((void*)port->membase + ULITE_CONTROL, ULITE_CONTROL_IE);
return 0;
}
static void ulite_shutdown(struct uart_port *port)
{
- writeb(0, port->membase + ULITE_CONTROL);
- readb(port->membase + ULITE_CONTROL); /* dummy */
+ out_be32((void*)port->membase + ULITE_CONTROL, 0);
+ in_be32((void*)port->membase + ULITE_CONTROL); /* dummy */
free_irq(port->irq, port);
}
@@ -314,7 +314,7 @@ static void ulite_console_wait_tx(struct uart_port *port)
/* wait up to 10ms for the character(s) to be sent */
for (i = 0; i < 10000; i++) {
- if (readb(port->membase + ULITE_STATUS) & ULITE_STATUS_TXEMPTY)
+ if (in_be32((void*)port->membase + ULITE_STATUS) & ULITE_STATUS_TXEMPTY)
break;
udelay(1);
}
@@ -323,7 +323,7 @@ static void ulite_console_wait_tx(struct uart_port *port)
static void ulite_console_putchar(struct uart_port *port, int ch)
{
ulite_console_wait_tx(port);
- writeb(ch, port->membase + ULITE_TX);
+ out_be32((void*)port->membase + ULITE_TX, ch);
}
static void ulite_console_write(struct console *co, const char *s,
@@ -340,8 +340,8 @@ static void ulite_console_write(struct console *co, const char *s,
spin_lock_irqsave(&port->lock, flags);
/* save and disable interrupt */
- ier = readb(port->membase + ULITE_STATUS) & ULITE_STATUS_IE;
- writeb(0, port->membase + ULITE_CONTROL);
+ ier = in_be32((void*)port->membase + ULITE_STATUS) & ULITE_STATUS_IE;
+ out_be32((void*)port->membase + ULITE_CONTROL, 0);
uart_console_write(port, s, count, ulite_console_putchar);
@@ -349,7 +349,7 @@ static void ulite_console_write(struct console *co, const char *s,
/* restore interrupt state */
if (ier)
- writeb(ULITE_CONTROL_IE, port->membase + ULITE_CONTROL);
+ out_be32((void*)port->membase + ULITE_CONTROL, ULITE_CONTROL_IE);
if (locked)
spin_unlock_irqrestore(&port->lock, flags);
--
1.5.1
_______________________________________________
Linuxppc-embedded mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-embedded
