[PATCH 05/15] musb: Do not use musb_read[b|w] / _write[b|w] wrappers in generic fifo functions

Hi,

On 10-03-15 09:50, Arnd Bergmann wrote:

On Tuesday 10 March 2015 08:43:22 Hans de Goede wrote:

quoted

On 09-03-15 22:50, Arnd Bergmann wrote:

quoted

On Monday 09 March 2015 21:40:18 Hans de Goede wrote:

quoted

The generic fifo functions already use non wrapped accesses in various
cases through the iowrite#_rep functions, and all platforms which override
the default musb_read[b|w] / _write[b|w] functions also provide their own
fifo access functions, so we can safely drop the unnecessary indirection
from the fifo access functions.

Signed-off-by: Hans de Goede <redacted>

The patch looks reasonably, but the description seem misleading.
I believe the real reason why it's ok to use __raw_writew for the
FIFO is that a FIFO by definition is using CPU endian access for
copying byte streams from memory, which is unlike any other MMIO
register that requires fixed-endian accessors.

I'm not sure that that is the case here, this fifo allows reading
4 bytes at a time using 32 bit word access, so endianness may come
into play. This patch is safe however since all existing users of
the generic fifo_read / write helpers which this patch touches, are
also using the generic musb_read[b|w] / _write[b|w] functions which
are just __raw_foo wrappers, so there is no functional change, which
is what the commit message tries to say.

This probably means that the generic musb helpers are not safe to use
on big-endian ARM systems (but may work on MIPS). The only one currently
not using the generic helpers is blackfin, which is fixed to little
endian.

quoted

Note that sunxi needs this function because of the register address
translation the sunxi_musb_readb / writeb wrappers are doing which
does not know how to deal with fifo data, and besides that it should
make the generic read / write fifo helpers somewhat faster by removing
an indirect function call.

Your sunxi_musb_readw/writew functions however also are endian-safe
and seem to get this part right, unlike all other platforms that use
the generic __raw_*() accessors. With this patch applied, it should
actually work fine, and it would work on other platforms as well
if we change all __raw_*() calls outside of musb_default_write_fifo()
and musb_default_read_fifo() to use *_relaxed() instead.

I think that that change falls outside of the scope of this patchset.

I agree that it would be probably a good idea to get rid of the
__raw_foo usage in musb, which is why I've used the non __raw
versions in the sunxi glue, but as said I believe this falls outside
of the scope of this patchset. All my preparation patches for adding
sunxi support carefully do not make any functional changes, as I
do not want to cause regressions on hardware which I cannot test.

Regards,

Hans