| Message ID | 1321120932-25786-1-git-send-email-galak@kernel.crashing.org |
|---|---|
| State | Accepted |
| Commit | 94c50f1176b711ce16ac95b58fb11d643f4b0ce8 |
| Delegated to: | Wolfgang Denk |
| Headers | show |
Hi Kumar, On Sat, Nov 12, 2011 at 10:02 AM, Kumar Gala <galak@kernel.crashing.org> wrote: > This reverts commit 355a835747c6f7c5071ead295a7dfc489c73cb03. > > The original commit broke long standing assumption that md commands work > on effective addresses. This normally isn't an issue for most systems > that map 1:1, however on systems with a 36-bit address map it breaks. What is the system that shows this problem? With sandbox we need some sort of mapping here - what do you suggest? map_virtmem? It would be good to get a solution to this rather than just reverting. Regards, Simon > > Signed-off-by: Kumar Gala <galak@kernel.crashing.org> > --- > common/cmd_mem.c | 9 ++------- > 1 files changed, 2 insertions(+), 7 deletions(-) > > diff --git a/common/cmd_mem.c b/common/cmd_mem.c > index 461ee19..28476d7 100644 > --- a/common/cmd_mem.c > +++ b/common/cmd_mem.c > @@ -33,7 +33,6 @@ > #include <dataflash.h> > #endif > #include <watchdog.h> > -#include <asm/io.h> > > #ifdef CMD_MEM_DEBUG > #define PRINTF(fmt,args...) printf (fmt ,##args) > @@ -142,13 +141,9 @@ int do_mem_md ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > # endif > > { > - ulong bytes = size * length; > - void *buf = map_physmem(addr, bytes, MAP_WRBACK); > - > /* Print the lines. */ > - print_buffer(addr, buf, size, length, DISP_LINE_LEN / size); > - addr += bytes; > - unmap_physmem(buf, bytes); > + print_buffer(addr, (void*)addr, size, length, DISP_LINE_LEN/size); > + addr += size*length; > } > #endif > > -- > 1.7.3.4 > >
On Nov 12, 2011, at 2:07 PM, Simon Glass wrote: > Hi Kumar, > > On Sat, Nov 12, 2011 at 10:02 AM, Kumar Gala <galak@kernel.crashing.org> wrote: >> This reverts commit 355a835747c6f7c5071ead295a7dfc489c73cb03. >> >> The original commit broke long standing assumption that md commands work >> on effective addresses. This normally isn't an issue for most systems >> that map 1:1, however on systems with a 36-bit address map it breaks. > > What is the system that shows this problem? Any PPC system that have _36BIT and some that imply it in boards.cfg > With sandbox we need some sort of mapping here - what do you suggest? > map_virtmem? Not sure, what the need is w/sandbox so hard to suggest a solution. > It would be good to get a solution to this rather than just reverting. Sure, but I'd rather revert in short term so the mainline tree works for all existing boards, rather than for the new feature until we come up with a solution. > > Regards, > Simon > >> >> Signed-off-by: Kumar Gala <galak@kernel.crashing.org> >> --- >> common/cmd_mem.c | 9 ++------- >> 1 files changed, 2 insertions(+), 7 deletions(-) >> >> diff --git a/common/cmd_mem.c b/common/cmd_mem.c >> index 461ee19..28476d7 100644 >> --- a/common/cmd_mem.c >> +++ b/common/cmd_mem.c >> @@ -33,7 +33,6 @@ >> #include <dataflash.h> >> #endif >> #include <watchdog.h> >> -#include <asm/io.h> >> >> #ifdef CMD_MEM_DEBUG >> #define PRINTF(fmt,args...) printf (fmt ,##args) >> @@ -142,13 +141,9 @@ int do_mem_md ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >> # endif >> >> { >> - ulong bytes = size * length; >> - void *buf = map_physmem(addr, bytes, MAP_WRBACK); >> - >> /* Print the lines. */ >> - print_buffer(addr, buf, size, length, DISP_LINE_LEN / size); >> - addr += bytes; >> - unmap_physmem(buf, bytes); >> + print_buffer(addr, (void*)addr, size, length, DISP_LINE_LEN/size); >> + addr += size*length; >> } >> #endif >> >> -- >> 1.7.3.4 >> >>
Hi Kumar, On Mon, Nov 14, 2011 at 1:11 PM, Kumar Gala <galak@kernel.crashing.org> wrote: > > On Nov 12, 2011, at 2:07 PM, Simon Glass wrote: > >> Hi Kumar, >> >> On Sat, Nov 12, 2011 at 10:02 AM, Kumar Gala <galak@kernel.crashing.org> wrote: >>> This reverts commit 355a835747c6f7c5071ead295a7dfc489c73cb03. >>> >>> The original commit broke long standing assumption that md commands work >>> on effective addresses. This normally isn't an issue for most systems >>> that map 1:1, however on systems with a 36-bit address map it breaks. >> >> What is the system that shows this problem? > > Any PPC system that have _36BIT and some that imply it in boards.cfg > >> With sandbox we need some sort of mapping here - what do you suggest? >> map_virtmem? > > Not sure, what the need is w/sandbox so hard to suggest a solution. OK well please go ahead with the revert. I could do a little patch, but perhaps we should clarify a few things first. Let me try to explain how I understand it now. We have physical and virtual address and I assume from your email that an effective address is the same as virtual. My assumption that the two are generally the same in U-Boot is false. Sandbox needs to provide a simulated memory bank to U-Boot. This can be thought of as physical memory, hence my implementation. In fact sandbox only sees part of the memory of its process, in this case 128MB of memory mmap'd from the OS. We cannot use a simple address in sandbox - we could request one but there is no real guarantee that we will get it, and it might lead to people hard-coding addresses in the header file leading to inexplicable failures. It is preferably to provide a simulated memory bank that starts at zero and extends for 128MB, say. Then the addresses for 'md' and the like are mapped into the mmap'd area. I wonder whether something like this might work? ptr = map_mem(addr, bytes, MAP_WRBACK); <do things with ptr> unmap_mem() These could be macros which #define to nothing for normal architectures. It is a little ugly, but there are not that many places which read/write memory without going through read/write macros. Regards, Simon > >> It would be good to get a solution to this rather than just reverting. > > Sure, but I'd rather revert in short term so the mainline tree works for all existing boards, rather than for the new feature until we come up with a solution. > >> >> Regards, >> Simon >> >>> >>> Signed-off-by: Kumar Gala <galak@kernel.crashing.org> >>> --- >>> common/cmd_mem.c | 9 ++------- >>> 1 files changed, 2 insertions(+), 7 deletions(-) >>> >>> diff --git a/common/cmd_mem.c b/common/cmd_mem.c >>> index 461ee19..28476d7 100644 >>> --- a/common/cmd_mem.c >>> +++ b/common/cmd_mem.c >>> @@ -33,7 +33,6 @@ >>> #include <dataflash.h> >>> #endif >>> #include <watchdog.h> >>> -#include <asm/io.h> >>> >>> #ifdef CMD_MEM_DEBUG >>> #define PRINTF(fmt,args...) printf (fmt ,##args) >>> @@ -142,13 +141,9 @@ int do_mem_md ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >>> # endif >>> >>> { >>> - ulong bytes = size * length; >>> - void *buf = map_physmem(addr, bytes, MAP_WRBACK); >>> - >>> /* Print the lines. */ >>> - print_buffer(addr, buf, size, length, DISP_LINE_LEN / size); >>> - addr += bytes; >>> - unmap_physmem(buf, bytes); >>> + print_buffer(addr, (void*)addr, size, length, DISP_LINE_LEN/size); >>> + addr += size*length; >>> } >>> #endif >>> >>> -- >>> 1.7.3.4 >>> >>> > >
Dear Kumar Gala, In message <1321120932-25786-1-git-send-email-galak@kernel.crashing.org> you wrote: > This reverts commit 355a835747c6f7c5071ead295a7dfc489c73cb03. > > The original commit broke long standing assumption that md commands work > on effective addresses. This normally isn't an issue for most systems > that map 1:1, however on systems with a 36-bit address map it breaks. > > Signed-off-by: Kumar Gala <galak@kernel.crashing.org> > --- > common/cmd_mem.c | 9 ++------- > 1 files changed, 2 insertions(+), 7 deletions(-) Applied, thanks. Best regards, Wolfgang Denk
diff --git a/common/cmd_mem.c b/common/cmd_mem.c index 461ee19..28476d7 100644 --- a/common/cmd_mem.c +++ b/common/cmd_mem.c @@ -33,7 +33,6 @@ #include <dataflash.h> #endif #include <watchdog.h> -#include <asm/io.h> #ifdef CMD_MEM_DEBUG #define PRINTF(fmt,args...) printf (fmt ,##args) @@ -142,13 +141,9 @@ int do_mem_md ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) # endif { - ulong bytes = size * length; - void *buf = map_physmem(addr, bytes, MAP_WRBACK); - /* Print the lines. */ - print_buffer(addr, buf, size, length, DISP_LINE_LEN / size); - addr += bytes; - unmap_physmem(buf, bytes); + print_buffer(addr, (void*)addr, size, length, DISP_LINE_LEN/size); + addr += size*length; } #endif
This reverts commit 355a835747c6f7c5071ead295a7dfc489c73cb03. The original commit broke long standing assumption that md commands work on effective addresses. This normally isn't an issue for most systems that map 1:1, however on systems with a 36-bit address map it breaks. Signed-off-by: Kumar Gala <galak@kernel.crashing.org> --- common/cmd_mem.c | 9 ++------- 1 files changed, 2 insertions(+), 7 deletions(-)