Re: xsysace driver support on arches other than PPC/Microblaze

From: Arnd Bergmann
Date: Wed Jun 19 2013 - 10:51:49 EST


On Wednesday 19 June 2013, Alexey Brodkin wrote:
> ============
> static void ace_out_be16(struct ace_device *ace, int reg, u16 val)
> {
> out_be16(ace->baseaddr + reg, val);
> }
>
> static void ace_dataout_be16(struct ace_device *ace)
> {
> int i = ACE_FIFO_SIZE / 2;
> u16 *src = ace->data_ptr;
> while (i--)
> out_le16(ace->baseaddr + 0x40, *src++);
> ace->data_ptr = src;
> }
> ============
>
> From it you may see that one high-level big-endian accessor
> ("ace_out_be16") uses big-endian low-level accessor ("out_be16") while
> another high-level big-endian accessor ("ace_dataout_be16") uses
> little-endian low-level accessor ("out_be16").
>
> It seems like access to 16-bit data words should be done always with LE
> accessors (after all it's always just a window to a device's memory).

This is a very long story, but I think the code is right. The point is
that ace_out_be16 accesses a single register that is defined as big-endian,
while ace_dataout_be16 accesses a byte stream in 16-bit units but has to
copy each byte in the same order that they are stored in memory on the
CPU. It usually takes me half an hour to wrap my head around this,
but yes it is this crazy and a lot of drivers do the same thing.

To simplify the above, ace_dataout_be16 should just call ioread16_rep()
which does the loop in the right endianess, while ace_out_be16
could be changed to use iowrite16_be(), which is the similar to
out_be16 but more portable.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/