[U-Boot] [PATCH] 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 swarren@wwwdotorg.org 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 swarren@wwwdotorg.org Reviewed-by: Simon Glass sjg@chromium.org ------------------------>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 abrodkin@synopsys.com Cc: Stephen Warren swarren@wwwdotorg.org Cc: Simon Glass sjg@chromium.org --- arch/arc/include/asm/io.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h index 24b7337..281682c 100644 --- a/arch/arc/include/asm/io.h +++ b/arch/arc/include/asm/io.h @@ -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 */
On 11/12/2015 02:56 PM, Alexey Brodkin wrote:
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
...
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?
Hi Stephen,
On Thu, 2015-11-12 at 16:00 -0700, Stephen Warren wrote:
On 11/12/2015 02:56 PM, Alexey Brodkin wrote:
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
...
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
On 11/13/2015 06:40 AM, Alexey Brodkin wrote:
Hi Stephen,
On Thu, 2015-11-12 at 16:00 -0700, Stephen Warren wrote:
On 11/12/2015 02:56 PM, Alexey Brodkin wrote:
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
...
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.
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.
Hi Alexey,
On 12 November 2015 at 14:56, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
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 swarren@wwwdotorg.org 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 <swarren@wwwdotorg.org> Reviewed-by: Simon Glass <sjg@chromium.org>------------------------>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 abrodkin@synopsys.com Cc: Stephen Warren swarren@wwwdotorg.org Cc: Simon Glass sjg@chromium.org
arch/arc/include/asm/io.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h index 24b7337..281682c 100644 --- a/arch/arc/include/asm/io.h +++ b/arch/arc/include/asm/io.h @@ -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 */ -- 2.4.3
What error does this solve?
Is CONFIG_ARCH_MAP_SYSMEM defined for arc?
Regards, Simon
Hi Simon,
On Fri, 2015-11-13 at 11:14 -0700, Simon Glass wrote:
Hi Alexey,
On 12 November 2015 at 14:56, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
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 swarren@wwwdotorg.org 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 <swarren@wwwdotorg.org> Reviewed-by: Simon Glass <sjg@chromium.org>------------------------>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 abrodkin@synopsys.com Cc: Stephen Warren swarren@wwwdotorg.org Cc: Simon Glass sjg@chromium.org
arch/arc/include/asm/io.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h index 24b7337..281682c 100644 --- a/arch/arc/include/asm/io.h +++ b/arch/arc/include/asm/io.h @@ -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 */ -- 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-------------------
Is CONFIG_ARCH_MAP_SYSMEM defined for arc?
No ---------------------->8------------------- $ cat .config | grep CONFIG_ARCH_MAP_SYSMEM---------------------->8-------------------
-Alexey
On 13 November 2015 at 11:23, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Hi Simon,
On Fri, 2015-11-13 at 11:14 -0700, Simon Glass wrote:
Hi Alexey,
On 12 November 2015 at 14:56, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
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 swarren@wwwdotorg.org 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 <swarren@wwwdotorg.org> Reviewed-by: Simon Glass <sjg@chromium.org>------------------------>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 abrodkin@synopsys.com Cc: Stephen Warren swarren@wwwdotorg.org Cc: Simon Glass sjg@chromium.org
arch/arc/include/asm/io.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h index 24b7337..281682c 100644 --- a/arch/arc/include/asm/io.h +++ b/arch/arc/include/asm/io.h @@ -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 */ -- 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-------------------
Is CONFIG_ARCH_MAP_SYSMEM defined for arc?
No ---------------------->8------------------- $ cat .config | grep CONFIG_ARCH_MAP_SYSMEM---------------------->8-------------------
-Alexey
Reviewed-by: Simon Glass sjg@chromium.org
Hi Simon, Stephen,
On Fri, 2015-11-13 at 19:03 -0700, Simon Glass wrote:
On 13 November 2015 at 11:23, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Hi Simon,
On Fri, 2015-11-13 at 11:14 -0700, Simon Glass wrote:
Hi Alexey,
On 12 November 2015 at 14:56, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
[snip]
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 swarren@wwwdotorg.org 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 <swarren@wwwdotorg.org> Reviewed-by: Simon Glass <sjg@chromium.org>------------------------>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 abrodkin@synopsys.com Cc: Stephen Warren swarren@wwwdotorg.org Cc: Simon Glass sjg@chromium.org
Reviewed-by: Simon Glass sjg@chromium.org
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
Hi,
On 16 November 2015 at 06:47, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Hi Simon, Stephen,
On Fri, 2015-11-13 at 19:03 -0700, Simon Glass wrote:
On 13 November 2015 at 11:23, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Hi Simon,
On Fri, 2015-11-13 at 11:14 -0700, Simon Glass wrote:
Hi Alexey,
On 12 November 2015 at 14:56, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
[snip]
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 swarren@wwwdotorg.org 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 <swarren@wwwdotorg.org> Reviewed-by: Simon Glass <sjg@chromium.org>------------------------>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 abrodkin@synopsys.com Cc: Stephen Warren swarren@wwwdotorg.org Cc: Simon Glass sjg@chromium.org
Reviewed-by: Simon Glass sjg@chromium.org
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
Hi Stephen,
On Mon, 2015-11-16 at 14:08 -0700, Simon Glass wrote:
Hi,
On 16 November 2015 at 06:47, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Hi Simon, Stephen,
On Fri, 2015-11-13 at 19:03 -0700, Simon Glass wrote:
On 13 November 2015 at 11:23, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Hi Simon,
On Fri, 2015-11-13 at 11:14 -0700, Simon Glass wrote:
Hi Alexey,
On 12 November 2015 at 14:56, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
[snip]
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 swarren@wwwdotorg.org 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 <swarren@wwwdotorg.org> Reviewed-by: Simon Glass <sjg@chromium.org>------------------------>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 abrodkin@synopsys.com Cc: Stephen Warren swarren@wwwdotorg.org Cc: Simon Glass sjg@chromium.org
Reviewed-by: Simon Glass sjg@chromium.org
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
On 11/16/2015 03:15 PM, Alexey Brodkin wrote:
Hi Stephen,
On Mon, 2015-11-16 at 14:08 -0700, Simon Glass wrote:
Hi,
On 16 November 2015 at 06:47, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Hi Simon, Stephen,
On Fri, 2015-11-13 at 19:03 -0700, Simon Glass wrote:
On 13 November 2015 at 11:23, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Hi Simon,
On Fri, 2015-11-13 at 11:14 -0700, Simon Glass wrote:
Hi Alexey,
On 12 November 2015 at 14:56, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
[snip]
> 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 swarren@wwwdotorg.org > 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 swarren@wwwdotorg.org > Reviewed-by: Simon Glass sjg@chromium.org > ------------------------>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 abrodkin@synopsys.com > Cc: Stephen Warren swarren@wwwdotorg.org > Cc: Simon Glass sjg@chromium.org > ---
Reviewed-by: Simon Glass sjg@chromium.org
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.
Yes, that seems fine; my suggestion was more for post-patch cleanup.
Hi,
On Mon, 2015-11-16 at 16:15 -0700, Stephen Warren wrote:
On 11/16/2015 03:15 PM, Alexey Brodkin wrote:
Hi Stephen,
On Mon, 2015-11-16 at 14:08 -0700, Simon Glass wrote:
Hi,
On 16 November 2015 at 06:47, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Hi Simon, Stephen,
On Fri, 2015-11-13 at 19:03 -0700, Simon Glass wrote:
On 13 November 2015 at 11:23, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Hi Simon,
On Fri, 2015-11-13 at 11:14 -0700, Simon Glass wrote: > Hi Alexey, > > On 12 November 2015 at 14:56, Alexey Brodkin > Alexey.Brodkin@synopsys.com wrote:
[snip]
Yes, that seems fine; my suggestion was more for post-patch cleanup.
Applied, thanks.
-Alexey
participants (3)
-
Alexey Brodkin -
Simon Glass -
Stephen Warren