arc: add stubs for map_physmem() and unmap_physmem()

PATCH New Alexey Brodkin 3 participants 12 messages Nov 12, 2015 – Nov 17, 2015 Synopsys ARC architecture

lore

Share & tools

lore

Alexey Brodkin

author

2015-11-12 21:56 UTC #0 lore raw

PATCH 1/1 arc: add stubs for map_physmem() and unmap_physmem()

Up until now there was no need in those stubs.

But since following commit compilation of U-Boot on ARC is broken:
------------------------>8----------------------
commit 7861204c9af7fec1ea9b41541c272516235a6c93

Author: Stephen Warren <[email protected]>
Date:   Sat Oct 3 13:56:46 2015 -0600

itest: make memory access work under sandbox

itest accesses memory, and hence must map/unmap it. Without doing so, it accesses invalid addresses and crashes.

Signed-off-by Stephen Warren <[email protected]>

Reviewed-by Simon Glass <[email protected]>

------------------------>8----------------------

That's because CMD_ITEST is enabled by default in common/Kconfig and now map_physmem()/unmap_physmem() is used there.

So this patch adds missing stubs for ARC.

Signed-off-by Alexey Brodkin <[email protected]>

Cc Stephen Warren <[email protected]>

Cc Simon Glass <[email protected]>

---
 arch/arc/include/asm/io.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

arch/arc/include/asm/io.h +24 -0

index 24b7337..281682c 100644
@@ -10,6 +10,30 @@
 #include <linux/types.h>
 #include <asm/byteorder.h>
 
+/*
+ * Given a physical address and a length, return a virtual address
+ * that can be used to access the memory range with the caching
+ * properties specified by "flags".
+ */
+#define MAP_NOCACHE	(0)
+#define MAP_WRCOMBINE	(0)
+#define MAP_WRBACK	(0)
+#define MAP_WRTHROUGH	(0)
+
+static inline void *
+map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
+{
+	return (void *)((unsigned long)paddr);
+}
+
+/*
+ * Take down a mapping set up by map_physmem().
+ */
+static inline void unmap_physmem(void *vaddr, unsigned long flags)
+{
+
+}
+
 static inline void sync(void)
 {
 	/* Not yet implemented */

Signature · 1 line

2.4.3

Stephen Warren

2015-11-12 23:00 UTC #1 lore raw

PATCH 1/1 arc: add stubs for map_physmem() and unmap_physmem()

Quoted 6 lines · Alexey Brodkin

Up until now there was no need in those stubs.
But since following commit compilation of U-Boot on ARC is broken:
commit 7861204c9af7fec1ea9b41541c272516235a6c93
itest: make memory access work under sandbox

...

Quoted 4 lines

That's because CMD_ITEST is enabled by default in common/Kconfig and now map_physmem()/unmap_physmem() is used there.
So this patch adds missing stubs for ARC.

This looks OK, but rather than cut/pasting this exact same code yet another time, why not create e.g. include/io-base.h that contains this, and share it amongst all architectures?

Alexey Brodkin

author

2015-11-13 13:40 UTC #2 lore raw

PATCH 1/1 arc: add stubs for map_physmem() and unmap_physmem()

Hi Stephen,

Quoted 8 lines · Stephen Warren

Quoted 6 lines · Alexey Brodkin
Up until now there was no need in those stubs.
But since following commit compilation of U-Boot on ARC is broken:
commit 7861204c9af7fec1ea9b41541c272516235a6c93
itest: make memory access work under sandbox
...

Quoted 8 lines

Quoted 4 lines
That's because CMD_ITEST is enabled by default in common/Kconfig and now map_physmem()/unmap_physmem() is used there.
So this patch adds missing stubs for ARC.
This looks OK, but rather than cut/pasting this exact same code yet another time, why not create e.g. include/io-base.h that contains this, and share it amongst all architectures?

I did think about that.

But the problem is "asm/io.h" is included in lots of sources and it's hard to tell a reason for that inclusion - if it's only because of map_physmem() or other stuff that might exist in the same header.

For example lots of accessors are described in the same "asm/io.h" like readl(), writeb() etc.

Frankly I'd prefer in that particular case to limit a change to my architecture.

Still thoughts are welcome.

-Alexey

Stephen Warren

2015-11-13 16:06 UTC #3 lore raw

PATCH 1/1 arc: add stubs for map_physmem() and unmap_physmem()

Quoted 26 lines · Alexey Brodkin

Hi Stephen,
Quoted 8 lines · Stephen Warren
Quoted 6 lines · Alexey Brodkin
Up until now there was no need in those stubs.
But since following commit compilation of U-Boot on ARC is broken:
commit 7861204c9af7fec1ea9b41541c272516235a6c93
itest: make memory access work under sandbox
...
Quoted 8 lines
Quoted 4 lines
That's because CMD_ITEST is enabled by default in common/Kconfig and now map_physmem()/unmap_physmem() is used there.
So this patch adds missing stubs for ARC.
This looks OK, but rather than cut/pasting this exact same code yet another time, why not create e.g. include/io-base.h that contains this, and share it amongst all architectures?
I did think about that.
But the problem is "asm/io.h" is included in lots of sources and it's hard to tell a reason for that inclusion - if it's only because of map_physmem() or other stuff that might exist in the same header.

Quoted 7 lines

For example lots of accessors are described in the same "asm/io.h" like readl(), writeb() etc.
Frankly I'd prefer in that particular case to limit a change to my architecture.
Still thoughts are welcome.

FWIW, I expected all the existing <asm/io.h> to simply include that new header in place of the duplicated code. Certainly, going through the entire source tree and adding #include statements for that new header would not be a good approach.

Simon Glass

2015-11-13 18:14 UTC #4 lore raw

PATCH 1/1 arc: add stubs for map_physmem() and unmap_physmem()

In reply to Alexey Brodkin

Hi Alexey,

Quoted 65 lines · Alexey Brodkin

Up until now there was no need in those stubs.
But since following commit compilation of U-Boot on ARC is broken:
------------------------>8----------------------
commit 7861204c9af7fec1ea9b41541c272516235a6c93
Author: Stephen Warren <[email protected]>
Date:   Sat Oct 3 13:56:46 2015 -0600
itest: make memory access work under sandbox
itest accesses memory, and hence must map/unmap it. Without doing so, it accesses invalid addresses and crashes.
Signed-off-by Stephen Warren <[email protected]>
Reviewed-by Simon Glass <[email protected]>
------------------------>8----------------------
That's because CMD_ITEST is enabled by default in common/Kconfig and now map_physmem()/unmap_physmem() is used there.
So this patch adds missing stubs for ARC.
Signed-off-by Alexey Brodkin <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Simon Glass <[email protected]>
---
 arch/arc/include/asm/io.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
arch/arc/include/asm/io.h +24 -1
index 24b7337..281682c 100644
@@ -10,6 +10,30 @@
 #include <linux/types.h>
 #include <asm/byteorder.h>
 
+/*
+ * Given a physical address and a length, return a virtual address
+ * that can be used to access the memory range with the caching
+ * properties specified by "flags".
+ */
+#define MAP_NOCACHE    (0)
+#define MAP_WRCOMBINE  (0)
+#define MAP_WRBACK     (0)
+#define MAP_WRTHROUGH  (0)
+
+static inline void *
+map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
+{
+       return (void *)((unsigned long)paddr);
+}
+
+/*
+ * Take down a mapping set up by map_physmem().
+ */
+static inline void unmap_physmem(void *vaddr, unsigned long flags)
+{
+
+}
+
 static inline void sync(void)
 {
        /* Not yet implemented */
--

Quoted 2 lines

2.4.3

What error does this solve?

Is CONFIG_ARCH_MAP_SYSMEM defined for arc?

Regards,
Simon

Alexey Brodkin

author

2015-11-13 18:23 UTC #5 lore raw

PATCH 1/1 arc: add stubs for map_physmem() and unmap_physmem()

Hi Simon,

Quoted 65 lines · Simon Glass

Hi Alexey,
Quoted 61 lines · Alexey Brodkin
Up until now there was no need in those stubs.
But since following commit compilation of U-Boot on ARC is broken:
------------------------>8----------------------
commit 7861204c9af7fec1ea9b41541c272516235a6c93
Author: Stephen Warren <[email protected]>
Date:   Sat Oct 3 13:56:46 2015 -0600
itest: make memory access work under sandbox
itest accesses memory, and hence must map/unmap it. Without doing so, it accesses invalid addresses and crashes.
Signed-off-by Stephen Warren <[email protected]>
Reviewed-by Simon Glass <[email protected]>
------------------------>8----------------------
That's because CMD_ITEST is enabled by default in common/Kconfig and now map_physmem()/unmap_physmem() is used there.
So this patch adds missing stubs for ARC.
Signed-off-by Alexey Brodkin <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Simon Glass <[email protected]>
---
 arch/arc/include/asm/io.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
arch/arc/include/asm/io.h +24 -0
index 24b7337..281682c 100644
@@ -10,6 +10,30 @@
 #include <linux/types.h>
 #include <asm/byteorder.h>
 
+/*
+ * Given a physical address and a length, return a virtual address
+ * that can be used to access the memory range with the caching
+ * properties specified by "flags".
+ */
+#define MAP_NOCACHE    (0)
+#define MAP_WRCOMBINE  (0)
+#define MAP_WRBACK     (0)
+#define MAP_WRTHROUGH  (0)
+
+static inline void *
+map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
+{
+       return (void *)((unsigned long)paddr);
+}
+
+/*
+ * Take down a mapping set up by map_physmem().
+ */
+static inline void unmap_physmem(void *vaddr, unsigned long flags)
+{
+
+}
+

Quoted 8 lines

Quoted 6 lines
arch/arc/include/asm/io.h +0 -1
 static inline void sync(void)
 {
        /* Not yet implemented */
--
2.4.3
What error does this solve?

---------------------->8-------------------
CC common/cmd_itest.o
common/cmd_itest.c: In function 'evalexp':
common/cmd_itest.c:61:3: warning: implicit
declaration of function 'map_physmem' [-Wimplicit-function-declaration]
buf = map_physmem(addr, w, MAP_WRBACK);
^
com
mon/cmd_itest.c:61:30: error: 'MAP_WRBACK' undeclared (first use in this function)
buf = map_physmem(addr, w,

MAP_WRBACK);
                              ^

common/cmd_itest.c:61:30: note: each undeclared identifier is reported only once for each function it appears in
common/cmd_itest.c:71:3: warning: implicit declaration of function 'unmap_physmem' [ -Wimplicit-function-declaration]
unmap_physmem(buf, w);
^
scripts/Makefile.build:277: recipe for target
'common/cmd_itest.o' failed
make[1]: *** [common/cmd_itest.o] Error 1
Makefile:1211: recipe for target 'common' failed
make
: *** [common] Error 2
---------------------->8-------------------

Quoted 1 line

Is CONFIG_ARCH_MAP_SYSMEM defined for arc?

No
---------------------->8-------------------

$ cat .config | grep CONFIG_ARCH_MAP_SYSMEM---------------------->8-------------------

-Alexey

Simon Glass

2015-11-14 02:03 UTC #6 lore raw

PATCH 1/1 arc: add stubs for map_physmem() and unmap_physmem()

Quoted 102 lines · Alexey Brodkin

Hi Simon,
Quoted 65 lines · Simon Glass
Hi Alexey,
Quoted 61 lines · Alexey Brodkin
Up until now there was no need in those stubs.
But since following commit compilation of U-Boot on ARC is broken:
------------------------>8----------------------
commit 7861204c9af7fec1ea9b41541c272516235a6c93
Author: Stephen Warren <[email protected]>
Date:   Sat Oct 3 13:56:46 2015 -0600
itest: make memory access work under sandbox
itest accesses memory, and hence must map/unmap it. Without doing so, it accesses invalid addresses and crashes.
Signed-off-by Stephen Warren <[email protected]>
Reviewed-by Simon Glass <[email protected]>
------------------------>8----------------------
That's because CMD_ITEST is enabled by default in common/Kconfig and now map_physmem()/unmap_physmem() is used there.
So this patch adds missing stubs for ARC.
Signed-off-by Alexey Brodkin <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Simon Glass <[email protected]>
---
 arch/arc/include/asm/io.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
arch/arc/include/asm/io.h +24 -0
index 24b7337..281682c 100644
@@ -10,6 +10,30 @@
 #include <linux/types.h>
 #include <asm/byteorder.h>
 
+/*
+ * Given a physical address and a length, return a virtual address
+ * that can be used to access the memory range with the caching
+ * properties specified by "flags".
+ */
+#define MAP_NOCACHE    (0)
+#define MAP_WRCOMBINE  (0)
+#define MAP_WRBACK     (0)
+#define MAP_WRTHROUGH  (0)
+
+static inline void *
+map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
+{
+       return (void *)((unsigned long)paddr);
+}
+
+/*
+ * Take down a mapping set up by map_physmem().
+ */
+static inline void unmap_physmem(void *vaddr, unsigned long flags)
+{
+
+}
+
Quoted 8 lines
Quoted 6 lines
arch/arc/include/asm/io.h +0 -1
 static inline void sync(void)
 {
        /* Not yet implemented */
--
2.4.3
What error does this solve?
---------------------->8-------------------
CC common/cmd_itest.o
common/cmd_itest.c: In function 'evalexp':
common/cmd_itest.c:61:3: warning: implicit
declaration of function 'map_physmem' [-Wimplicit-function-declaration]
   buf = map_physmem(addr, w, MAP_WRBACK);
   ^
com
mon/cmd_itest.c:61:30: error: 'MAP_WRBACK' undeclared (first use in this function)
   buf = map_physmem(addr, w,
MAP_WRBACK);
                              ^
common/cmd_itest.c:61:30: note: each undeclared identifier is reported only once for each function it appears in
common/cmd_itest.c:71:3: warning: implicit declaration of function 'unmap_physmem' [ -Wimplicit-function-declaration]
   unmap_physmem(buf, w);
   ^
scripts/Makefile.build:277: recipe for target
'common/cmd_itest.o' failed
make[1]: *** [common/cmd_itest.o] Error 1
Makefile:1211: recipe for target 'common' failed
make
: *** [common] Error 2
---------------------->8-------------------

Quoted 7 lines

Quoted 1 line
Is CONFIG_ARCH_MAP_SYSMEM defined for arc?
No
---------------------->8-------------------
$ cat .config | grep CONFIG_ARCH_MAP_SYSMEM---------------------->8-------------------
-Alexey

Reviewed-by Simon Glass <[email protected]>

Alexey Brodkin

author

2015-11-16 13:47 UTC #7 lore raw

PATCH 1/1 arc: add stubs for map_physmem() and unmap_physmem()

Hi Simon, Stephen,

Quoted 9 lines · Simon Glass

Quoted 7 lines · Alexey Brodkin
Hi Simon,
Quoted 4 lines · Simon Glass
Hi Alexey,
On 12 November 2015 at 14:56, Alexey Brodkin
<[email protected]> wrote:

[snip]

Quoted 20 lines

Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
Up until now there was no need in those stubs.
But since following commit compilation of U-Boot on ARC is broken:
------------------------>8----------------------
commit 7861204c9af7fec1ea9b41541c272516235a6c93
Author: Stephen Warren <[email protected]>
Date:   Sat Oct 3 13:56:46 2015 -0600
itest: make memory access work under sandbox
itest accesses memory, and hence must map/unmap it. Without doing so, it accesses invalid addresses and crashes.
Signed-off-by Stephen Warren <[email protected]>
Reviewed-by Simon Glass <[email protected]>
------------------------>8----------------------
That's because CMD_ITEST is enabled by default in common/Kconfig and now map_physmem()/unmap_physmem() is used there.

Quoted 8 lines

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
So this patch adds missing stubs for ARC.
Signed-off-by Alexey Brodkin <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Simon Glass <[email protected]>
---
Reviewed-by Simon Glass <[email protected]>

So should I move map_physmem()/unmap_physmem() in generic header and include it in asm/io.h for every arch (as suggested by Stephen) or for starters I may have stubs for ARC and once this patch is accepted do clean-up for all arches at once?

-Alexey

Simon Glass

2015-11-16 21:08 UTC #8 lore raw

PATCH 1/1 arc: add stubs for map_physmem() and unmap_physmem()

Hi,

Quoted 42 lines · Alexey Brodkin

Hi Simon, Stephen,
Quoted 9 lines · Simon Glass
Quoted 7 lines · Alexey Brodkin
Hi Simon,
Quoted 4 lines · Simon Glass
Hi Alexey,
On 12 November 2015 at 14:56, Alexey Brodkin
<[email protected]> wrote:
[snip]
Quoted 27 lines
Quoted 26 lines
Quoted 26 lines
Quoted 26 lines
Up until now there was no need in those stubs.
But since following commit compilation of U-Boot on ARC is broken:
------------------------>8----------------------
commit 7861204c9af7fec1ea9b41541c272516235a6c93
Author: Stephen Warren <[email protected]>
Date:   Sat Oct 3 13:56:46 2015 -0600
itest: make memory access work under sandbox
itest accesses memory, and hence must map/unmap it. Without doing so, it accesses invalid addresses and crashes.
Signed-off-by Stephen Warren <[email protected]>
Reviewed-by Simon Glass <[email protected]>
------------------------>8----------------------
That's because CMD_ITEST is enabled by default in common/Kconfig and now map_physmem()/unmap_physmem() is used there.
So this patch adds missing stubs for ARC.
Signed-off-by Alexey Brodkin <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Simon Glass <[email protected]>
---

Quoted 6 lines

Quoted 1 line
Reviewed-by Simon Glass <[email protected]>
So should I move map_physmem()/unmap_physmem() in generic header and include it in asm/io.h for every arch (as suggested by Stephen) or for starters I may have stubs for ARC and once this patch is accepted do clean-up for all arches at once?

Either is fine with me. If you do a shared file it should be in include/asm-generic I think.

Regards,
Simon

Alexey Brodkin

author

2015-11-16 22:15 UTC #9 lore raw

PATCH 1/1 arc: add stubs for map_physmem() and unmap_physmem()

Hi Stephen,

Quoted 48 lines · Simon Glass

Hi,
Quoted 44 lines · Alexey Brodkin
Hi Simon, Stephen,
Quoted 9 lines · Simon Glass
Quoted 7 lines · Alexey Brodkin
Hi Simon,
Quoted 4 lines · Simon Glass
Hi Alexey,
On 12 November 2015 at 14:56, Alexey Brodkin
<[email protected]> wrote:
[snip]
Quoted 28 lines
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
Up until now there was no need in those stubs.
But since following commit compilation of U-Boot on ARC is broken:
------------------------>8----------------------
commit 7861204c9af7fec1ea9b41541c272516235a6c93
Author: Stephen Warren <[email protected]>
Date:   Sat Oct 3 13:56:46 2015 -0600
itest: make memory access work under sandbox
itest accesses memory, and hence must map/unmap it. Without doing so, it accesses invalid addresses and crashes.
Signed-off-by Stephen Warren <[email protected]>
Reviewed-by Simon Glass <[email protected]>
------------------------>8----------------------
That's because CMD_ITEST is enabled by default in common/Kconfig and now map_physmem()/unmap_physmem() is used there.
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
So this patch adds missing stubs for ARC.
Signed-off-by Alexey Brodkin <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Simon Glass <[email protected]>
---
Reviewed-by Simon Glass <[email protected]>

Quoted 7 lines

Quoted 4 lines
So should I move map_physmem()/unmap_physmem() in generic header and include it in asm/io.h for every arch (as suggested by Stephen) or for starters I may have stubs for ARC and once this patch is accepted do clean-up for all arches at once?
Either is fine with me. If you do a shared file it should be in include/asm-generic I think.

Please let me know if you're fine as well with local patch for ARC for starters. This quick fix is really necessary because as of today U-Boot for ARC couldn't be built at all from current master branch.

-Alexey

Stephen Warren

2015-11-16 23:15 UTC #10 lore raw

PATCH 1/1 arc: add stubs for map_physmem() and unmap_physmem()

Quoted 56 lines · Alexey Brodkin

Hi Stephen,
Quoted 53 lines · Simon Glass
Hi,
Quoted 48 lines · Alexey Brodkin
Hi Simon, Stephen,
Quoted 9 lines · Simon Glass
Quoted 7 lines · Alexey Brodkin
Hi Simon,
Quoted 4 lines · Simon Glass
Hi Alexey,
On 12 November 2015 at 14:56, Alexey Brodkin
<[email protected]> wrote:
[snip]
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
Up until now there was no need in those stubs.
But since following commit compilation of U-Boot on ARC is broken:
------------------------>8----------------------
commit 7861204c9af7fec1ea9b41541c272516235a6c93
Author: Stephen Warren <[email protected]>
Date:   Sat Oct 3 13:56:46 2015 -0600
itest: make memory access work under sandbox
itest accesses memory, and hence must map/unmap it. Without doing so, it accesses invalid addresses and crashes.
Signed-off-by Stephen Warren <[email protected]>
Reviewed-by Simon Glass <[email protected]>
------------------------>8----------------------
That's because CMD_ITEST is enabled by default in common/Kconfig and now map_physmem()/unmap_physmem() is used there.
Quoted 8 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
So this patch adds missing stubs for ARC.
Signed-off-by Alexey Brodkin <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Simon Glass <[email protected]>
---
Reviewed-by Simon Glass <[email protected]>
So should I move map_physmem()/unmap_physmem() in generic header and include it in asm/io.h for every arch (as suggested by Stephen) or for starters I may have stubs for ARC and once this patch is accepted do clean-up for all arches at once?

Quoted 6 lines

Quoted 2 lines
Either is fine with me. If you do a shared file it should be in include/asm-generic I think.
Please let me know if you're fine as well with local patch for ARC for starters. This quick fix is really necessary because as of today U-Boot for ARC couldn't be built at all from current master branch.

Yes, that seems fine; my suggestion was more for post-patch cleanup.

Alexey Brodkin

author Applied

2015-11-17 21:43 UTC #11 lore raw

PATCH 1/1 arc: add stubs for map_physmem() and unmap_physmem()

Hi,

Quoted 16 lines · Stephen Warren

Quoted 15 lines · Alexey Brodkin
Hi Stephen,
Quoted 12 lines · Simon Glass
Hi,
Quoted 7 lines · Alexey Brodkin
Hi Simon, Stephen,
Quoted 4 lines · Simon Glass
Quoted 2 lines · Alexey Brodkin
Hi Simon,

Quoted 8 lines

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
Quoted 5 lines · Simon Glass
Quoted 4 lines
Hi Alexey,
On 12 November 2015 at 14:56, Alexey Brodkin
<[email protected]> wrote:
[snip]
Yes, that seems fine; my suggestion was more for post-patch cleanup.

Applied, thanks.

-Alexey