Re: [PATCH] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)

[Michal Simek <monstr@monstr.eu>]

On 02/07/2013 08:38 AM, Geert Uytterhoeven wrote:
> On Thu, Feb 7, 2013 at 8:23 AM, Michal Simek <monstr@monstr.eu> wrote:
>>>> #define iowrite16be(v, addr)   iowrite16(be16_to_cpu(v), (addr))
>>>> #define iowrite16(v, addr)      writew((v), (addr))
>>>> #define writew(b,addr) __raw_writew(__cpu_to_le16(b),addr)
>>>>
>>>> static inline void __raw_writew(u16 b, volatile void __iomem *addr)
>>>> {
>>>>          *(volatile u16 __force *) addr = b;
>>>> }
>>>>
>>>> How is this suppose to work on Big Endian?
>>>> be16_to_cpu(v) is (v)
>>>> and
>>>> __cpu_to_le16(b) is swab16(v)
>>>
>>> Yes.
>>
>> But on native BE system ( I expect that v is in big endian)
>> iowrite16be(v, addr) should be just *(volatile u16 __force *) addr =
>> v; not *(volatile u16 __force *) addr = swab16(v);
>
>>>> What I would expect is
>>>> #define iowrite16be(v, addr)   __raw_writew(__cpu_to_be16(v), addr)
>>>
>>> Indeed, it should be "__cpu_to_be16(v)" instead of "be16_to_cpu(v)".
>>
>> What do you mean by that?
>
> Bummer, I missed that current iowrite16be() uses (the little endian)
> iowrite16(),
> not _raw_writew(), and thought the only difference between the original
> and your version was the endianness conversion macro.
>
> Yes,
>
>      #define iowrite16be(v, addr)   __raw_writew(__cpu_to_be16(v), addr)
>
> should be correct.

ok. Can you please confirm with me that the same problem is also for iowrite32be
ioread16be and ioread32be?

This description seems to me correct for BE and LE.
#define ioread16be(addr)       __be16_to_cpu(__raw_readw(addr))
#define ioread32be(addr)       __be32_to_cpu(__raw_readl(addr))
#define iowrite16be(v, addr)   __raw_writew(__cpu_to_be16(v), addr)
#define iowrite32be(v, addr)   __raw_writel(__cpu_to_be32(v), addr)

What do you think?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian