On Mon, Jan 25, 2016 at 11:34:41PM +0100, Johannes Berg wrote: > Or, perhaps, the code should just WARN() when endianness is unspecified > (assuming you can actually specify "native-endian;") and we need to fix > all of the instances...
That is probably going to be too much work even if just for MMIO regmaps, there's too many of them out there. So, recapping our discussion on IRC a bit here there's two things going on. We've got how the regmaps specify their configuration to regmap which is currently less obvious than it should be for MMIO buses but boiled down to native/default being little endian in the past since we were using readl() and writel(). There's also the issue of how we write out to the device which is currently wrong since __raw accessors are not protected against things like spinlocks. This is currently a mess since this code was originally written on little endian systems and it never registered that readl() and writel() did any byte swapping so the code has in fact always been writing out little endian values. I also note that currently regmap-mmio is using the byte stream APIs originally intended for I2C and SPI rather than reg_read() and reg_write() which are a much better fit but were added later. I think where we want to end up is with little endian being the default for MMIO regmaps since that's what we were doing up until this change and with the MIPS DT changes applied since the DT that was there was obvious nonsense. Unfortunately MIPS complicates this since it really does want native endianness. I need to think this through properly but I think what we want to do here is: - Make the default for MMIO regmaps be explicitly little endian with either an ifdef for MIPS to keep it working or an explict native endianness tag in the DT instead of the straight revert to LE (the latter seems better). - Convert the MMIO regmap to use reg_read() and reg_write() with implementations using either readX() or ioread_*be() and equivalents for write. This means the core does no endianness swapping and it's all in the bus. Unfortunately that all sounds a bit too big for v4.5... perhaps a combination of a revert of the implementation and the addition of the native tag to the DT for v4.5 followed by the reworking of the bus for v4.6, I really would rather keep the DT change in v4.5 since specifying LE is just bad and we don't want that to propagate any more than it has to. From this I also conclude that we need to improve our testing of big endian ARM systems since nobody managed to notice this all the time this was cooking in -next.
signature.asc
Description: PGP signature
