Re: [PATCH 06/18] [POWERPC] Fix UARTLITE reg io for little-endian architectures (ie. microblaze)

On 9/28/07, Olof Johansson [off-list ref] wrote:

On Fri, Sep 28, 2007 at 12:17:13PM -0600, Grant Likely wrote:

quoted

From: Grant Likely <redacted>

Signed-off-by: Grant Likely <redacted>
Acked-by: John Williams <redacted>
---

 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 ace4ec0..270ad3a 100644
--- a/arch/ppc/syslib/virtex_devices.c
+++ b/arch/ppc/syslib/virtex_devices.c

@@ -28,7 +28,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);

Hmm, I see the start changed, and you're now reading/writing a full
32-bit word instead of individual bytes. Still, looks a little fishy to
me. Wouldn't it be more appropriate to change the ULITE_RX offset to be
3 higher and still read/write bytes?

Or are the registers defined as 32-bit ones? (I don't remember, it was
so long since I touched uartlite myself. :-)

All the registers are defined as 32 bit ones.  I think it makes more
sense to access the registers as they are documented, and it
eliminates the 'magic' +3 needed to make it work now.

(Same for the other functions below, but the general principle applies.)

Also, I'm not sure you need to cast port->membase to void*, do you? The
math will still be right since it's declared as char *.

membase is now defined as u32*, so the cast is needed.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195