Re: [RFC] uartlite driver MicroBlaze compatability

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

[-- Attachment #1: Type: text/plain, Size: 2943 bytes --]

On 4/30/07, John Williams <jwilliams@itee.uq.edu.au> 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.
grant.likely@secretlab.ca
(403) 399-0195

[-- Attachment #2: 0001-PPC-Fix-UARTLITE-register-access-for-little-endian.patch --]
[-- Type: application/x-patch, Size: 5228 bytes --]