Hi Arnd,
On Mon, Apr 29, 2019 at 2:40 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
On Mon, Apr 29, 2019 at 10:19 AM Geert Uytterhoeven
<geert@xxxxxxxxxxxxxx> wrote:
The generic definitions of mmio_{read,write}{16,32}be() in lib/iomap.c
assume that the {read,write}[wl]() I/O accessors always use little
endian accesses, and swap the result.
However, the Coldfire versions of the {read,write}[wl]() I/O accessors are
special, in that they use native big endian instead of little endian for
accesses to the on-SoC peripheral block, thus violating the assumption.
Fix this by providing our own variants, using the raw accessors,
reinstating the old behavior. This is fine on m68k, as no special
barriers are needed, and also avoids swapping data twice.
Reported-by: Angelo Dureghello <angelo@xxxxxxxx>
Fixes: aecc787c06f4300f ("iomap: Use non-raw io functions for io{read|write}XXbe")
Signed-off-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
---
This can be reverted later, after this oddity of the Coldfire I/O
support has been fixed, and drivers have been updated.
---
arch/m68k/include/asm/io.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/m68k/include/asm/io.h b/arch/m68k/include/asm/io.h
index aabe6420ead2a599..d47e7384681ab1cd 100644
--- a/arch/m68k/include/asm/io.h
+++ b/arch/m68k/include/asm/io.h
@@ -8,6 +8,12 @@
#include <asm/io_mm.h>
#endif
+#define mmio_read16be(addr) __raw_readw(addr)
+#define mmio_read32be(addr) __raw_readl(addr)
+
+#define mmio_write16be(val, port) __raw_writew((val), (port))
+#define mmio_write32be(val, port) __raw_writel((val), (port))
+
#include <asm-generic/io.h>
This looks correct to me, but there are two points that stick out to me:
- why do you only do this for mmio and not for pio?
Because no one had a need for it? ;-)
Now seriously, m68k only has MMIO, no PIO. Any PIO, if used, is for ISA or
PCMCIA I/O accesses, which are little endian.
- why do you even use the generic_iomap wrappers rather than
the trivial asm-generic versions of those functions?
Looking at git history, that was done to fix some link errors, which no
longer seem to happen with the more mature asm-generic infrastructure we
have now.
So probably we can drop the selection of GENERIC_IOMAP and inclusion
of <asm-generic/iomap.h>, after fixing a few compiler warnings like:
include/asm-generic/io.h: In function ‘ioread8_rep’
arch/m68k/include/asm/io_mm.h:371:44: warning: passing argument 1
of ‘raw_insb’ discard ‘const’ qualifier from pointer target type
[-Wdiscarded-qualifiers]
#define readsb(port, buf, nr) raw_insb((port), (u8 *)(buf), (nr))
arch/m68k/include/asm/raw_io.h:101:50: note: expected ‘volatile u8
*’ {aka ‘volatile unsigned char *’} but argument is of type ‘const
volatile void *’
static inline void raw_insb(volatile u8 __iomem *port, u8 *buf,
unsigned int len)
and some regression testing, of course.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
