[PATCH] asm-generic: io: Fix ioread16/32be and iowrite16/32be

[PATCH] asm-generic: io: Fix ioread16/32be and iowrite16/32be

[Michal Simek]

Fix ioreadXXbe and iowriteXXbe functions which did
additional little endian conversion on native big endian systems.
Using be_to_cpu (cpu_to_be) conversions with __raw_read/write
functions have resolved it.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Geert Uytterhoeven <geert@linux-m68k.org>
CC: Will Deacon <will.deacon@arm.com>
CC: linux-arch@vger.kernel.org
---
 include/asm-generic/io.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 33bbbae..8823581 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -225,15 +225,15 @@ static inline void outsl(unsigned long addr, const void *buffer, int count)
 #ifndef CONFIG_GENERIC_IOMAP
 #define ioread8(addr)		readb(addr)
 #define ioread16(addr)		readw(addr)
-#define ioread16be(addr)	be16_to_cpu(ioread16(addr))
+#define ioread16be(addr)	__be16_to_cpu(__raw_readw(addr))
 #define ioread32(addr)		readl(addr)
-#define ioread32be(addr)	be32_to_cpu(ioread32(addr))
+#define ioread32be(addr)	__be32_to_cpu(__raw_readl(addr))
 
 #define iowrite8(v, addr)	writeb((v), (addr))
 #define iowrite16(v, addr)	writew((v), (addr))
-#define iowrite16be(v, addr)	iowrite16(be16_to_cpu(v), (addr))
+#define iowrite16be(v, addr)	__raw_writew(__cpu_to_be16(v), addr)
 #define iowrite32(v, addr)	writel((v), (addr))
-#define iowrite32be(v, addr)	iowrite32(be32_to_cpu(v), (addr))
+#define iowrite32be(v, addr)	__raw_writel(__cpu_to_be32(v), addr)
 
 #define ioread8_rep(p, dst, count) \
 	insb((unsigned long) (p), (dst), (count))
-- 
1.7.0.4

Re: [PATCH] asm-generic: io: Fix ioread16/32be and iowrite16/32be

[Arnd Bergmann]

On Thursday 07 February 2013 15:18:48 Michal Simek wrote:
> Fix ioreadXXbe and iowriteXXbe functions which did
> additional little endian conversion on native big endian systems.
> Using be_to_cpu (cpu_to_be) conversions with __raw_read/write
> functions have resolved it.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: Arnd Bergmann <arnd@arndb.de>
> CC: Geert Uytterhoeven <geert@linux-m68k.org>
> CC: Will Deacon <will.deacon@arm.com>
> CC: linux-arch@vger.kernel.org

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH] asm-generic: io: Fix ioread16/32be and iowrite16/32be

[Geert Uytterhoeven]

On Thu, Feb 7, 2013 at 3:18 PM, Michal Simek <michal.simek@xilinx.com> wrote:
> Fix ioreadXXbe and iowriteXXbe functions which did
> additional little endian conversion on native big endian systems.
> Using be_to_cpu (cpu_to_be) conversions with __raw_read/write
> functions have resolved it.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: Arnd Bergmann <arnd@arndb.de>
> CC: Geert Uytterhoeven <geert@linux-m68k.org>
> CC: Will Deacon <will.deacon@arm.com>
> CC: linux-arch@vger.kernel.org

I have one question (see below). Apart from that:
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

> -#define ioread16be(addr)       be16_to_cpu(ioread16(addr))
> +#define ioread16be(addr)       __be16_to_cpu(__raw_readw(addr))

Why did you change it to the __beX_to_cpu variant with underscores?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH] asm-generic: io: Fix ioread16/32be and iowrite16/32be

[Michal Simek]

2013/2/8 Geert Uytterhoeven <geert@linux-m68k.org>:
> On Thu, Feb 7, 2013 at 3:18 PM, Michal Simek <michal.simek@xilinx.com> wrote:
>> Fix ioreadXXbe and iowriteXXbe functions which did
>> additional little endian conversion on native big endian systems.
>> Using be_to_cpu (cpu_to_be) conversions with __raw_read/write
>> functions have resolved it.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> CC: Arnd Bergmann <arnd@arndb.de>
>> CC: Geert Uytterhoeven <geert@linux-m68k.org>
>> CC: Will Deacon <will.deacon@arm.com>
>> CC: linux-arch@vger.kernel.org
>
> I have one question (see below). Apart from that:
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
>
>> -#define ioread16be(addr)       be16_to_cpu(ioread16(addr))
>> +#define ioread16be(addr)       __be16_to_cpu(__raw_readw(addr))
>
> Why did you change it to the __beX_to_cpu variant with underscores?

The question could be probably different. Why are they even defined?
I have grepped the kernel and all archs use these generic macros

include/linux/byteorder/generic.h:94:#define be32_to_cpu __be32_to_cpu
include/linux/byteorder/generic.h:106:#define be32_to_cpup __be32_to_cpup
include/linux/byteorder/generic.h:118:#define be32_to_cpus __be32_to_cpus

What about to remove them?

Back to you question. I can't remember particular reason for that maybe
just experience that __ versions should be the fastest kernel implementation.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform