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

["Grant Likely" <grant.likely@secretlab.ca>]

On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Sep 28, 2007 at 12:17:13PM -0600, Grant Likely wrote:
> > From: Grant Likely <grant.likely@secretlab.ca>
> >
> > Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> > Acked-by: John Williams <jwilliams@itee.uq.edu.au>
> > ---
> >
> >  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