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

[Mark Brown <broonie@kernel.org>]

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

On Mon, Jan 25, 2016 at 11:24:44PM +0100, Arnd Bergmann wrote:
> 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

Please send patches to maintainers and please use subject lines
reflecting the style for the subsystem.

> > 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 I'd expect to be happening here is that either the driver or the
DT should be specifying the endianness of the hardware.  If the device
is always a given endianness then I'd expect that to turn up in the
driver rather than the DT.

> > 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.

The problem here is that regmap already has that functionality (it needs
it for non-MMIO buses anyway) and so it knows what's going on really
wants the I/O accessors to get out of the way.

> 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.

I can't identify *anything* which says we shouldn't use the __raw
accessors in architecture neutral code with the possible exception of
the __.  Seriously, how is anyone supposed to use this stuff if we have
hidden assumptions like this?

> 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.

Yeah, the native endian formatting is causing a lot of trouble.  The
MIPS folks really should also have come and talked to me rather than
writing such obvious nonsense in the DT in the first place.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]