[U-Boot] [PATCH 1/3] nds32: Use getenv_ulong() in place of getenv(), strtoul

[U-Boot] [PATCH 1/3] nds32: Use getenv_ulong() in place of getenv(), strtoul

[Macpaul Lin]

This changes the board code to use the new getenv_ulong() function.

Signed-off-by: Macpaul Lin <macpaul@andestech.com>
---
 arch/nds32/lib/board.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/arch/nds32/lib/board.c b/arch/nds32/lib/board.c
index 1776a72..2fd0e93 100644
--- a/arch/nds32/lib/board.c
+++ b/arch/nds32/lib/board.c
@@ -50,13 +50,7 @@ ulong monitor_flash_len;
 #endif
 static int init_baudrate(void)
 {
-	char tmp[64];	/* long enough for environment variables */
-	int i = getenv_f("baudrate", tmp, sizeof(tmp));
-
-	gd->bd->bi_baudrate = gd->baudrate = (i > 0)
-			? (int) simple_strtoul(tmp, NULL, 10)
-			: CONFIG_BAUDRATE;
-
+	gd->baudrate = getenv_ulong("baudrate", 10, CONFIG_BAUDRATE);
 	return 0;
 }
 
@@ -400,9 +394,7 @@ void board_init_r(gd_t *id, ulong dest_addr)
 #endif
 
 	/* Initialize from environment */
-	s = getenv("loadaddr");
-	if (s != NULL)
-		load_addr = simple_strtoul(s, NULL, 16);
+	load_addr = getenv_ulong("loadaddr", 16, load_addr);
 
 #if defined(CONFIG_CMD_NET)
 	s = getenv("bootfile");
-- 
1.7.3.5

[U-Boot] [PATCH 2/3] nds32: cache: define ARCH_DMA_MINALIGN for DMA buffer alignment

[Macpaul Lin]

Add ARCH_DMA_MINALIGN definition to asm/cache.h

Signed-off-by: Macpaul Lin <macpaul@andestech.com>
---
 arch/nds32/include/asm/cache.h |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/nds32/include/asm/cache.h b/arch/nds32/include/asm/cache.h
index d769196..fc22c7b 100644
--- a/arch/nds32/include/asm/cache.h
+++ b/arch/nds32/include/asm/cache.h
@@ -51,4 +51,15 @@ DEFINE_GET_SYS_REG(DCM_CFG);
 #define DCM_CFG_OFF_DSZ	6	/* D-cache line size */
 #define DCM_CFG_MSK_DSZ	(0x7UL << DCM_CFG_OFF_DSZ)
 
+/*
+ * The current upper bound for NDS32 L1 data cache line sizes is 32 bytes.
+ * We use that value for aligning DMA buffers unless the board config has
+ * specified an alternate cache line size.
+ */
+#ifdef CONFIG_SYS_CACHELINE_SIZE
+#define ARCH_DMA_MINALIGN	CONFIG_SYS_CACHELINE_SIZE
+#else
+#define ARCH_DMA_MINALIGN	32
+#endif
+
 #endif /* _ASM_CACHE_H */
-- 
1.7.3.5

[U-Boot] [PATCH 2/3] nds32: cache: define ARCH_DMA_MINALIGN for DMA buffer alignment

[Anton Staaf]

On Mon, Oct 24, 2011 at 2:46 AM, Macpaul Lin <macpaul@andestech.com> wrote:
> Add ARCH_DMA_MINALIGN definition to asm/cache.h
>
> Signed-off-by: Macpaul Lin <macpaul@andestech.com>

Acked-by: Anton Staaf <robotboy@chromium.org>

> ---
> ?arch/nds32/include/asm/cache.h | ? 11 +++++++++++
> ?1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/arch/nds32/include/asm/cache.h b/arch/nds32/include/asm/cache.h
> index d769196..fc22c7b 100644
> --- a/arch/nds32/include/asm/cache.h
> +++ b/arch/nds32/include/asm/cache.h
> @@ -51,4 +51,15 @@ DEFINE_GET_SYS_REG(DCM_CFG);
> ?#define DCM_CFG_OFF_DSZ ? ? ? ?6 ? ? ? /* D-cache line size */
> ?#define DCM_CFG_MSK_DSZ ? ? ? ?(0x7UL << DCM_CFG_OFF_DSZ)
>
> +/*
> + * The current upper bound for NDS32 L1 data cache line sizes is 32 bytes.
> + * We use that value for aligning DMA buffers unless the board config has
> + * specified an alternate cache line size.
> + */
> +#ifdef CONFIG_SYS_CACHELINE_SIZE
> +#define ARCH_DMA_MINALIGN ? ? ?CONFIG_SYS_CACHELINE_SIZE
> +#else
> +#define ARCH_DMA_MINALIGN ? ? ?32
> +#endif
> +
> ?#endif /* _ASM_CACHE_H */
> --
> 1.7.3.5
>
>

[U-Boot] [PATCH 2/3] nds32: cache: define ARCH_DMA_MINALIGN for DMA buffer alignment

[馬克泡]

Hi Wolfgang,

2011/10/25 Anton Staaf <robotboy@chromium.org>:
> On Mon, Oct 24, 2011 at 2:46 AM, Macpaul Lin <macpaul@andestech.com> wrote:
>> Add ARCH_DMA_MINALIGN definition to asm/cache.h
>>
>> Signed-off-by: Macpaul Lin <macpaul@andestech.com>
>
> Acked-by: Anton Staaf <robotboy@chromium.org>
>

Since the NDS32 has been come into mainline.
Could I pick up this kind of simple changes of reviewed patches which
are related to NDS32 only by myself,
and then send pull-request to you?
Hope this could reducing your loading and gain some benefits

Thanks.
-- 
Best regards,
Macpaul Lin

[U-Boot] [PATCH 2/3] nds32: cache: define ARCH_DMA_MINALIGN for DMA buffer alignment

[Mike Frysinger]

On Friday 28 October 2011 00:46:38 ??? wrote:
> Since the NDS32 has been come into mainline.
> Could I pick up this kind of simple changes of reviewed patches which
> are related to NDS32 only by myself,
> and then send pull-request to you?
> Hope this could reducing your loading and gain some benefits

you've got a maintainer tree now:
	http://git.denx.de/?p=u-boot/u-boot-nds32.git

so you should be gathering all the NDS32 patches and pushing to wolfgang.  
please see the documentation here for the process:
	http://www.denx.de/wiki/U-Boot/DevelopmentProcess

short answer:
	- fixes should get pushed at any time
	- everything else should get pushed during merge window
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111030/ba4d243b/attachment.pgp 

[U-Boot] [PATCH 2/3] nds32: cache: define ARCH_DMA_MINALIGN for DMA buffer alignment

[馬克泡]

Hi Mike,

2011/10/31 Mike Frysinger <vapier@gentoo.org>:
> On Friday 28 October 2011 00:46:38 ??? wrote:
> you've got a maintainer tree now:
>        http://git.denx.de/?p=u-boot/u-boot-nds32.git
>
> so you should be gathering all the NDS32 patches and pushing to wolfgang.
> please see the documentation here for the process:
>        http://www.denx.de/wiki/U-Boot/DevelopmentProcess
>
> short answer:
>        - fixes should get pushed at any time
>        - everything else should get pushed during merge window
> -mike
>

Thanks for your hint.
Will do patch collections soon.

-- 
Best regards,
Macpaul Lin

[U-Boot] [PATCH 2/3] nds32: cache: define ARCH_DMA_MINALIGN for DMA buffer alignment

[馬克泡]

Hi Macpaul,

2011/10/25 Anton Staaf <robotboy@chromium.org>:
> On Mon, Oct 24, 2011 at 2:46 AM, Macpaul Lin <macpaul@andestech.com> wrote:
>> Add ARCH_DMA_MINALIGN definition to asm/cache.h
>>
>> Signed-off-by: Macpaul Lin <macpaul@andestech.com>
>
> Acked-by: Anton Staaf <robotboy@chromium.org>
>

Applied to u-boot-nds32/master, thanks.

Best regards,
Macpaul Lin

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

[Macpaul Lin]

This patch add required __iormb and __iowmb to io.h.
This also fix some misbehavior to periphal drivers.

This io.h has been fixed with referencing arm/include/asm/io.h.

Signed-off-by: Macpaul Lin <macpaul@andestech.com>
---
 arch/nds32/include/asm/io.h |   20 ++++++++++++++------
 1 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/nds32/include/asm/io.h b/arch/nds32/include/asm/io.h
index 2504c2b..a221a4d 100644
--- a/arch/nds32/include/asm/io.h
+++ b/arch/nds32/include/asm/io.h
@@ -98,13 +98,21 @@ extern void __raw_readsl(unsigned int addr, void *data, int longlen);
 #define __raw_readw(a)			__arch_getw(a)
 #define __raw_readl(a)			__arch_getl(a)
 
-#define writeb(v, a)			__arch_putb(v, a)
-#define writew(v, a)			__arch_putw(v, a)
-#define writel(v, a)			__arch_putl(v, a)
+/*
+ * TODO: The kernel offers some more advanced versions of barriers, it might
+ * have some advantages to use them instead of the simple one here.
+ */
+#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 writew(v, c)	({ u16 __v = v; __iowmb(); __arch_putw(__v, c); __v; })
+#define writel(v, c)	({ u32 __v = v; __iowmb(); __arch_putl(__v, c); __v; })
 
-#define readb(a)			__arch_getb(a)
-#define readw(a)			__arch_getw(a)
-#define readl(a)			__arch_getl(a)
+#define readb(c)	({ u8  __v = __arch_getb(c); __iormb(); __v; })
+#define readw(c)	({ u16 __v = __arch_getw(c); __iormb(); __v; })
+#define readl(c)	({ u32 __v = __arch_getl(c); __iormb(); __v; })
 
 /*
  * The compiler seems to be incapable of optimising constants
-- 
1.7.3.5

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

[Marek Vasut]

> This patch add required __iormb and __iowmb to io.h.
> This also fix some misbehavior to periphal drivers.
> 
> This io.h has been fixed with referencing arm/include/asm/io.h.
> 
> Signed-off-by: Macpaul Lin <macpaul@andestech.com>

Hi,

can you possibly implement those as inline functions ? It'd be also great if you 
could convert the other macros to inline functions.

Thanks!

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

[馬克泡]

Hi Marek,

2011/10/24 Marek Vasut <marek.vasut@gmail.com>

>  Hi,
>
> can you possibly implement those as inline functions ? It'd be also great
> if you
> could convert the other macros to inline functions.
>
> Thanks!
>

Thanks for your comment.
I'll try to do it tomorrow (GMT +0800).

-- 
Best regards,
Macpaul Lin

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

[馬克泡]

Hi Marek,

> 2011/10/24 Marek Vasut <marek.vasut@gmail.com>
>>
>> Hi,
>>
>> can you possibly implement those as inline functions ? It'd be also great if you
>> could convert the other macros to inline functions.
>>
>> Thanks!

I've found there are some different data type warning occur when I'm
translating inline function
since all the driver originately call write and read marco.

Most of them is easy to deal with sicne the address is usually a
32-bit address "unsigned int *".
However, the driver/serial/ns16550.c used readb/writeb but the
"address" passed in ns16550.c is
byte-wised (unsigned char *).

And there are also address casted in (ulong) in ns16550.c.
serial_out(UART_LCR_BKSE | UART_LCRVAL, (ulong)&com_port->lcr);
I'm not sure to fix this way would affect ns16550 in other architecutre?.
#define serial_out(x, y)       writeb(x, (unsigned int *)y)

And there are also problems when translating readb from macro to
inline function.
Some driver cannot be compiled with a return value inside the readb.
I think I can post the fix of writeb for NDS32 architecture and see if
you give some comments
then I do for other architecture later.

Hi Wolfgang,
Could you please pickup these 2 patches since it was independent to
r/w inline functions?
I'm not sure if the merge windows has been closed.
Or should I pick these 2 patches into nds32 repo?

nds32: Use getenv_ulong() in place of getenv(), strtoul
http://patchwork.ozlabs.org/patch/121312/

nds32: cache: define ARCH_DMA_MINALIGN for DMA buffer alignment
http://patchwork.ozlabs.org/patch/121313/

Thanks!

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

[Marek Vasut]

> Hi Marek,
> 
> > 2011/10/24 Marek Vasut <marek.vasut@gmail.com>
> > 
> >> Hi,
> >> 
> >> can you possibly implement those as inline functions ? It'd be also
> >> great if you could convert the other macros to inline functions.
> >> 
> >> Thanks!
> 
> I've found there are some different data type warning occur when I'm
> translating inline function
> since all the driver originately call write and read marco.
> 
> Most of them is easy to deal with sicne the address is usually a
> 32-bit address "unsigned int *".
> However, the driver/serial/ns16550.c used readb/writeb but the
> "address" passed in ns16550.c is
> byte-wised (unsigned char *).

Well please fix. You can send a cleanup series?

> 
> And there are also address casted in (ulong) in ns16550.c.
> serial_out(UART_LCR_BKSE | UART_LCRVAL, (ulong)&com_port->lcr);
> I'm not sure to fix this way would affect ns16550 in other architecutre?.
> #define serial_out(x, y)       writeb(x, (unsigned int *)y)

This is definitelly wrong. I'd like to see others comment on this one.
> 
> And there are also problems when translating readb from macro to
> inline function.

Why?

> Some driver cannot be compiled with a return value inside the readb.
> I think I can post the fix of writeb for NDS32 architecture and see if
> you give some comments
> then I do for other architecture later.
> 
> Hi Wolfgang,
> Could you please pickup these 2 patches since it was independent to
> r/w inline functions?
> I'm not sure if the merge windows has been closed.
> Or should I pick these 2 patches into nds32 repo?
> 
> nds32: Use getenv_ulong() in place of getenv(), strtoul
> http://patchwork.ozlabs.org/patch/121312/
> 
> nds32: cache: define ARCH_DMA_MINALIGN for DMA buffer alignment
> http://patchwork.ozlabs.org/patch/121313/
> 
> Thanks!

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

[馬克泡]

Hi Marek,

2011/10/25 Marek Vasut <marek.vasut@gmail.com>:

[snip]

>> Hi Marek,
>>
>> And there are also address casted in (ulong) in ns16550.c.
>> serial_out(UART_LCR_BKSE | UART_LCRVAL, (ulong)&com_port->lcr);
>> I'm not sure to fix this way would affect ns16550 in other architecutre?.
>> #define serial_out(x, y) ? ? ? writeb(x, (unsigned int *)y)
>
> This is definitelly wrong. I'd like to see others comment on this one.

I'm agree with you but I'm not sure why dose it need a ulong casting
(64 bit data?) since I'm not familiar
with ns16550 hardware.

Please check ns16550.c, current code has another such kind of casting
#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
#define serial_out(x, y)        outb(x, (ulong)y)
#define serial_in(y)            inb((ulong)y)
[...]
#else
#define serial_out(x, y)        writeb(x, y)
#define serial_in(y)            readb(y)
#endif

>>
>> And there are also problems when translating readb from macro to
>> inline function.
>
> Why?

Oh I've just found where the problem is inside the io.h
There are the same problem in other io.h (and in linux) if the specific
macro wasn't defined, the header will use a kind of default macro.
I'll post the patch for NDS32 later which also fix this problem.

Plesse give comment to the PATCH v2 later and see if everyone like
such kind of implement.

-- 
Best regards,
Macpaul Lin

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

[馬克泡]

Hi Graeme,

2011/10/25 ??? <macpaul@gmail.com>:
> Hi Marek,
>
> 2011/10/25 Marek Vasut <marek.vasut@gmail.com>:
>
> [snip]
>
> Please check ns16550.c, current code has another such kind of casting
> #ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> #define serial_out(x, y)        outb(x, (ulong)y)
> #define serial_in(y)            inb((ulong)y)
> [...]
> #else
> #define serial_out(x, y)        writeb(x, y)
> #define serial_in(y)            readb(y)
> #endif
>

I'm going to change writeb/readb series functions from macro into
inline functions.
However, I've found data type casting problem in NS16550.
Only here the serial_out with CONFIG_SYS_NS16550_PORT_MAPPED is used
for board "eNET".
Is this a necessary for this machine  to access 8-bytes data?

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?

-- 
Best regards,
Macpaul Lin

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

[Graeme Russ]

Hi Macpaul

On Wed, Oct 26, 2011 at 4:19 PM, ??? <macpaul@gmail.com> wrote:
> Hi Graeme,
>
> 2011/10/25 ??? <macpaul@gmail.com>:
>> Hi Marek,
>>
>> 2011/10/25 Marek Vasut <marek.vasut@gmail.com>:
>>
>> [snip]
>>
>> Please check ns16550.c, current code has another such kind of casting
>> #ifdef CONFIG_SYS_NS16550_PORT_MAPPED
>> #define serial_out(x, y)        outb(x, (ulong)y)
>> #define serial_in(y)            inb((ulong)y)
>> [...]
>> #else
>> #define serial_out(x, y)        writeb(x, y)
>> #define serial_in(y)            readb(y)
>> #endif
>>
>
> 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?

> However, I've found data type casting problem in NS16550.
> Only here the serial_out with CONFIG_SYS_NS16550_PORT_MAPPED is used
> for board "eNET".
> 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

> 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

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.

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

Regards,

Graeme

[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@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

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

[Graeme Russ]

Hi Macpaul,

2011/10/27 ??? <macpaul@gmail.com>:
> Hi Graeme,
>
> ? 2011?10?26???1:55?Graeme Russ <graeme.russ@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

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

[Macpaul Lin]

1. This patch add required __iormb and __iowmb to io.h.
   This also fix some misbehavior to periphal drivers.
   This io.h has been fixed with referencing arm/include/asm/io.h.
2. This patch replaced macro writeb and readb into inline function.

Signed-off-by: Macpaul Lin <macpaul@andestech.com>
---
Changes for v2:
  - translate writeb macro into inline function.

 arch/nds32/include/asm/io.h |   72 +++++++++++++++++++++++++++++++------------
 1 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/arch/nds32/include/asm/io.h b/arch/nds32/include/asm/io.h
index 2504c2b..2c105f7 100644
--- a/arch/nds32/include/asm/io.h
+++ b/arch/nds32/include/asm/io.h
@@ -98,13 +98,59 @@ extern void __raw_readsl(unsigned int addr, void *data, int longlen);
 #define __raw_readw(a)			__arch_getw(a)
 #define __raw_readl(a)			__arch_getl(a)
 
-#define writeb(v, a)			__arch_putb(v, a)
-#define writew(v, a)			__arch_putw(v, a)
-#define writel(v, a)			__arch_putl(v, a)
+/*
+ * TODO: The kernel offers some more advanced versions of barriers, it might
+ * have some advantages to use them instead of the simple one here.
+ */
+#define dmb()		__asm__ __volatile__ ("" : : : "memory")
+#define __iormb()	dmb()
+#define __iowmb()	dmb()
+
+static inline void writeb(unsigned char val, unsigned char *addr)
+{
+	__iowmb();
+	__arch_putb(val, addr);
+}
+
+static inline void writew(unsigned short val, unsigned short *addr)
+{
+	__iowmb();
+	__arch_putw(val, addr);
+
+}
+
+static inline void writel(unsigned int val, unsigned int *addr)
+{
+	__iowmb();
+	__arch_putl(val, addr);
+}
+
+static inline unsigned char readb(unsigned char *addr)
+{
+	u8	val;
 
-#define readb(a)			__arch_getb(a)
-#define readw(a)			__arch_getw(a)
-#define readl(a)			__arch_getl(a)
+	val = __arch_getb(addr);
+	__iormb();
+	return val;
+}
+
+static inline unsigned short readw(unsigned short *addr)
+{
+	u16	val;
+
+	val = __arch_getw(addr);
+	__iormb();
+	return val;
+}
+
+static inline unsigned int readl(unsigned int *addr)
+{
+	u32	val;
+
+	val = __arch_getl(addr);
+	__iormb();
+	return val;
+}
 
 /*
  * The compiler seems to be incapable of optimising constants
@@ -338,20 +384,6 @@ check_signature(unsigned long io_addr, const unsigned char *signature,
 out:
 	return retval;
 }
-
-#elif !defined(readb)
-
-#define readb(addr)			(__readwrite_bug("readb"), 0)
-#define readw(addr)			(__readwrite_bug("readw"), 0)
-#define readl(addr)			(__readwrite_bug("readl"), 0)
-#define writeb(v, addr)			__readwrite_bug("writeb")
-#define writew(v, addr)			__readwrite_bug("writew")
-#define writel(v, addr)			__readwrite_bug("writel")
-
-#define eth_io_copy_and_sum(a, b, c, d)	__readwrite_bug("eth_io_copy_and_sum")
-
-#define check_signature(io, sig, len)	(0)
-
 #endif	/* __mem_pci */
 
 /*
-- 
1.7.3.5

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

[馬克泡]

Hi Macpaul,

2011/10/25 Macpaul Lin <macpaul@andestech.com>:
> 1. This patch add required __iormb and __iowmb to io.h.
> ? This also fix some misbehavior to periphal drivers.
> ? This io.h has been fixed with referencing arm/include/asm/io.h.
> 2. This patch replaced macro writeb and readb into inline function.
>
> Signed-off-by: Macpaul Lin <macpaul@andestech.com>
> ---
> Changes for v2:
> ?- translate writeb macro into inline function.
>
> ?arch/nds32/include/asm/io.h | ? 72 +++++++++++++++++++++++++++++++------------
> ?1 files changed, 52 insertions(+), 20 deletions(-)
>

Applied to u-boot-nds32/master, thanks.

Best regards,
Macpaul Lin

[U-Boot] [PATCH 1/3] nds32: Use getenv_ulong() in place of getenv(), strtoul

[馬克泡]

Hi Macpaul,

2011/10/24 Macpaul Lin <macpaul@andestech.com>:
> This changes the board code to use the new getenv_ulong() function.
>
> Signed-off-by: Macpaul Lin <macpaul@andestech.com>
> ---
> ?arch/nds32/lib/board.c | ? 12 ++----------
> ?1 files changed, 2 insertions(+), 10 deletions(-)

Applied to u-boot-nds32/master, thanks.

Best regards,
Macpaul Lin