diff mbox

[U-Boot,03/12] ns16550: change map_sysmem to map_physmem

Message ID 1447684616-10297-4-git-send-email-thomas@wytron.com.tw
State Superseded
Delegated to: Simon Glass
Headers show

Commit Message

Thomas Chou Nov. 16, 2015, 2:36 p.m. UTC
Change map_sysmem() to map_physmem(), because sandbox is the only
arch which define map_sysmem() and it actually only calls
map_physmem(). For some arch like nios2, the flag should be
MAP_NOCACHE for port access.

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
 drivers/serial/ns16550.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Simon Glass Nov. 16, 2015, 9:08 p.m. UTC | #1
Hi Thomas,

On 16 November 2015 at 07:36, Thomas Chou <thomas@wytron.com.tw> wrote:
> Change map_sysmem() to map_physmem(), because sandbox is the only
> arch which define map_sysmem() and it actually only calls
> map_physmem(). For some arch like nios2, the flag should be
> MAP_NOCACHE for port access.

Why change it to map_physmem()? Doesn't that make assumptions about
how sandbox is implemented?

>
> Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
> ---
>  drivers/serial/ns16550.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> index 6433844..8d028de 100644
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -8,7 +8,6 @@
>  #include <dm.h>
>  #include <errno.h>
>  #include <fdtdec.h>
> -#include <mapmem.h>
>  #include <ns16550.h>
>  #include <serial.h>
>  #include <watchdog.h>
> @@ -97,7 +96,7 @@ static void ns16550_writeb(NS16550_t port, int offset, int value)
>         unsigned char *addr;
>
>         offset *= 1 << plat->reg_shift;
> -       addr = map_sysmem(plat->base, 0) + offset;
> +       addr = map_physmem(plat->base, 0, MAP_NOCACHE) + offset;
>         /*
>          * As far as we know it doesn't make sense to support selection of
>          * these options at run-time, so use the existing CONFIG options.
> @@ -111,7 +110,7 @@ static int ns16550_readb(NS16550_t port, int offset)
>         unsigned char *addr;
>
>         offset *= 1 << plat->reg_shift;
> -       addr = map_sysmem(plat->base, 0) + offset;
> +       addr = map_physmem(plat->base, 0, MAP_NOCACHE) + offset;
>
>         return serial_in_shift(addr, plat->reg_shift);
>  }
> --
> 2.5.0
>

Regards,
Simon
Thomas Chou Nov. 17, 2015, 12:12 a.m. UTC | #2
Hi Simon,

On 2015年11月17日 05:08, Simon Glass wrote:
> Hi Thomas,
>
> On 16 November 2015 at 07:36, Thomas Chou <thomas@wytron.com.tw> wrote:
>> Change map_sysmem() to map_physmem(), because sandbox is the only
>> arch which define map_sysmem() and it actually only calls
>> map_physmem(). For some arch like nios2, the flag should be
>> MAP_NOCACHE for port access.
>
> Why change it to map_physmem()? Doesn't that make assumptions about
> how sandbox is implemented?
>

The question might be asked from the other side, why does it use 
map_sysmem()?

In README,

- CONFIG_ARCH_MAP_SYSMEM
Generally U-Boot (and in particular the md command) uses
effective address. It is therefore not necessary to regard
U-Boot address as virtual addresses that need to be translated
to physical addresses. However, sandbox requires this, since
it maintains its own little RAM buffer which contains all
addressable memory. This option causes some memory accesses
to be mapped through map_sysmem() / unmap_sysmem().

The map_physmem() serves the same purpose to translate physical address 
to virtual address with the additional flag to take care of cache 
property. Many drivers use map_physmem() since ports access should be 
uncached. As ns16550 is a driver, it should use map_physmem() rather 
than map_sysmem().

Best regards,
Thomas
diff mbox

Patch

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index 6433844..8d028de 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -8,7 +8,6 @@ 
 #include <dm.h>
 #include <errno.h>
 #include <fdtdec.h>
-#include <mapmem.h>
 #include <ns16550.h>
 #include <serial.h>
 #include <watchdog.h>
@@ -97,7 +96,7 @@  static void ns16550_writeb(NS16550_t port, int offset, int value)
 	unsigned char *addr;
 
 	offset *= 1 << plat->reg_shift;
-	addr = map_sysmem(plat->base, 0) + offset;
+	addr = map_physmem(plat->base, 0, MAP_NOCACHE) + offset;
 	/*
 	 * As far as we know it doesn't make sense to support selection of
 	 * these options at run-time, so use the existing CONFIG options.
@@ -111,7 +110,7 @@  static int ns16550_readb(NS16550_t port, int offset)
 	unsigned char *addr;
 
 	offset *= 1 << plat->reg_shift;
-	addr = map_sysmem(plat->base, 0) + offset;
+	addr = map_physmem(plat->base, 0, MAP_NOCACHE) + offset;
 
 	return serial_in_shift(addr, plat->reg_shift);
 }