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

[馬克泡]

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?
However I have no idea is there any other architecture like x86.

>> 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.

>> 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)

> 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'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;
}


-- 
Best regards,
Macpaul Lin