Re: [PATCH] Revert "regmap-mmio: Use native endianness for read/write"

From: Arnd Bergmann
Date: Mon Jan 25 2016 - 17:25:09 EST


On Monday 25 January 2016 23:07:55 Johannes Berg wrote:
> This reverts commit 29bb45f25ff3051354ed330c0d0f10418a2b8c7c.
>
> Clearly, using "native" endianness is a terrible idea when
> devices are involved, since those devices are different hw
> from the CPU, won't change, and the CPU might be able to
> run in both big and little endian, like ARM and PowerPC can.
> Therefore, "native" endian doesn't really exist.
>
> Consequently, this commit broke my HummingBoard i.MX6 in big
> endian mode since it would now try to talk to the little
> endian hardware with a big endian CPU without conversion.
>
> What the patch really would have to do is introduce some kind
> of "device-endian" readl/writel, that takes the endianness of
> the device as an argument. That seems a bit overkill though,
> and would likely not generate any better code than the double
> byte-swaps that MIPS is getting now.
>
> Therefore, simply revert the commit to fix the breakage.
>
> Fixes: 29bb45f25ff3 ("regmap-mmio: Use native endianness for read/write")
> Signed-off-by: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>

I think it's more complicated than this unfortunately:

Most architectures behave the same way you explain: all I/O registers
are fixed-endian, and the CPU may also be fixed-endian or may
support both using a runtime switch, which we abstract using the
readl/writel etc helpers in Linux.

On MIPS, the CPU endianess is set at through an input signal
on the CPU core, and this cannot change during runtime but can
change between SoCs or can be configurable with a hardware jumper.
Some SoC vendors (notably Broadcom) decided to use the same signal
to control whether there is a byteswap on the device bus or not,
so the on-chip MMIO registers are always the same endianess as
the CPU itself.

This means that the devices are in fact CPU-endian, and we need
some way for Linux to represent this. The patch to
drivers/base/regmap/regmap-mmio.c is clearly wrong, as we
must never use __raw_*() accessors in an architecture independent
driver (for a number of reasons), but we still need a fix for
MIPS so it can specify a way to do the double-swap without
faking the endianess of the registers.

Also, defaulting syscon to "native-endian" when nothing else is
specified sounds like a bad idea, but we may already be stuck there
with the precedent in existing bindings after 6a55244e897d
("regmap: mmio: request native endian formatting"), we'll have
to think about that some more.

Arnd