Re: endianness swapped

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Greg,

On Sun, Apr 28, 2019 at 3:59 PM Greg Ungerer <gerg@xxxxxxxxxxxxxx> wrote:
On 28/4/19 7:21 pm, Arnd Bergmann wrote:
On Sun, Apr 28, 2019 at 10:46 AM Geert Uytterhoeven
<geert@xxxxxxxxxxxxxx> wrote:
On Sat, Apr 27, 2019 at 10:22 PM Angelo Dureghello <angelo@xxxxxxxx> wrote:
On Sat, Apr 27, 2019 at 05:32:22PM +0200, Angelo Dureghello wrote:
as you may know, i am working on mcf5441x.
Sorry for not following carefully all the threads, but from a certain
kernel version (likely 4.19 or near there), seems ioread32be
reads the bytes swapped in endianness (mcf-edma dma driver not working
anymore).

Has there been a change about this in the architecture I/O access ?
How should i proceed now ? Fixing the DMA driver read/write, or what ?

looks like the reason of my ioread32be now swapped is:

https://patchwork.kernel.org/patch/10766673/

Trying to figure out what to do now.

This is commit aecc787c06f4300f ("iomap: Use non-raw io functions for
io{read|write}XXbe"):

--- a/lib/iomap.c
+++ b/lib/iomap.c
@@ -65,8 +65,8 @@ static void bad_io_access(unsigned long port, const
char *access)
  #endif

  #ifndef mmio_read16be
-#define mmio_read16be(addr) be16_to_cpu(__raw_readw(addr))
-#define mmio_read32be(addr) be32_to_cpu(__raw_readl(addr))
+#define mmio_read16be(addr) swab16(readw(addr))
+#define mmio_read32be(addr) swab32(readl(addr))
  #endif

  unsigned int ioread8(void __iomem *addr)
@@ -106,8 +106,8 @@ EXPORT_SYMBOL(ioread32be);
  #endif

  #ifndef mmio_write16be
-#define mmio_write16be(val,port) __raw_writew(be16_to_cpu(val),port)
-#define mmio_write32be(val,port) __raw_writel(be32_to_cpu(val),port)
+#define mmio_write16be(val,port) writew(swab16(val),port)
+#define mmio_write32be(val,port) writel(swab32(val),port)

On big endian, the raw accessors are assumed to be non-swapping,
while non-raw accessors are assumed to be swapping.
The latter is not true for Coldfire internal registers, cfr.
arch/m68k/include/asm/io_no.h:

static inline u16 readw(const volatile void __iomem *addr)
{
         if (cf_internalio(addr))
                 return __raw_readw(addr);
         return __le16_to_cpu(__raw_readw(addr));
}

Orthogonal to how Coldfire's read[wl]() should be fixed, I find it a bit
questionable to swap data twice on big endian architectures.

I would expect that the compiler is capable of detecting a double
swap and optimize it out. Even if it can't, there are not that many
instances of io{read,write}{16,32}be in the kernel, so the increase
in kernel image size from a double swap should be limited to a
few extra instructions, and the runtime overhead should be
negligible compared to the bus access.

Probably the overhead is not negligible on old m68k...

Fortunately we can avoid that by defining our own
mmio_{read,write}{16,32}be()...

Makes sense.

I've just sent a patch to do that, as a fix for v5.1 or v5.2.

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



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux