[U-Boot] [PATCH 3/3] nds32: asm/io.h: add __iormb and __iowmb support

[Graeme Russ]

Hi Macpaul,

2011/10/27 馬克泡 <macpaul at gmail.com>:
> Hi Graeme,
>
> 在 2011年10月26日下午1:55，Graeme Russ <graeme.russ at gmail.com> 寫道：
>> Hi Macpaul
>
> [snip]
>
>>> I'm going to change writeb/readb series functions from macro into
>>> inline functions.
>>
>> For all arches? Have you seen /arch/x86/include/asm/io.h?
>
> Currently, I have take a looked into arm, mips, and blackfin.
> Because the most probably common problem lead with the function
> replacement might be
> NS16550, so I check NS16550 at first.
>
> According to your reminding, it seem there is no necessary for x86 to
> replace the macro.
> Is that correct?

Yes, I think so - x86 will use inb/outb not readb/writeb. For all other
arch's, I suspect these may be one and the same

> However I have no idea is there any other architecture like x86.

No, x86 is unique - It define 'port mapped I/O' to free up the 64kB of
memory that would otherwise be taken up by device I/O - that was 10% of
the memory on a 640kB PC :)

>>> Is this a necessary for this machine  to access 8-bytes data?
>>
>> outb/inb access 8-bit data in a 16-bit address space and the ns16650 has
>> (for the x86 architecture at least) byte aligned registers
>>
>
> Okay, so it's not actually an "ulong" casting.

No

>>> Since inline functions are type sensitive,
>>> I've suggest to replace the macro for other machines like writeb(x, (uint)y)
>>> but Marek think this is not good enough.
>>> Could you give some comments?
>>
>> Hmm, but eNET (well, all x86 really) would not use writeb/readb - outb/inb
>> would be used instead so if you are only touching the writeb/readb, eNET
>> would never see this. So I don't understand where your type conflicts are
>> occuring
>>
>
> Because the the address passed into writeb and readb in ns16550.c will be
> "unsigned char" if they defined CONFIG_SYS_NS16550_REG_SIZE as "1" or
> "-4" and then
> will lead compile warnings when the address of inline function of
> writeb/readb is "unsigned int".
> (Please refer to include/ns16550.c)

Hmm, I can't provide any insight there

>> Looking at the ulong cast, I was wondering why it was there, and the commit
>> diff says I wrote it which is a bit of a worry :) x86 only has 65535
>> ports accessible via outb and inb, so I would have thought the case should
>> have been a ushort, not a ulong.
>>
>
> Okay, so we can double confirmed it is an ushort not an ulong.

I think so - I'll test if it build when cast as a ushort

>> I'm sorry, but without a patch to see what you are actually doing, I'm
>> having a bit of difficulty picturing what is happening and what could be
>> going wrong...
>>
>
> Sorry not to add you at the beginning when the discussion begin.
> Because when I sent this patch, I thought it is only related to NDS32
> arch at first.
>
> For examples, the origin implementation in macro is this.
> #define __arch_getb(a)                  (*(unsigned char *)(a))
> #define __arch_putb(v, a)               (*(unsigned char *)(a) = (v))
>
> #define dmb()          __asm__ __volatile__ ("" : : : "memory")
> #define __iormb()      dmb()
> #define __iowmb()      dmb()
>
> #define writeb(v, c)   ({ u8  __v = v; __iowmb(); __arch_putb(__v, c); __v; })
> #define readb(a)   __arch_getb(a)
>
> The new implementation in inline function will be like this.
>
> static inline void writeb(unsigned char val, unsigned char *addr)
> {
>       __iowmb();
>       __arch_putb(val, addr);
> }
> static inline unsigned char readb(unsigned char *addr)
> {
>        u8      val;
>
>        val = __arch_getb(addr);
>        __iormb();
>        return val;
> }

In arch/x86/include/io.h:

#define readb(addr) (*(volatile unsigned char *) (addr))

Feel free to change

inb/outb are already inline funtions (via some very ugly macros)

Regards,

Graeme