[U-Boot] [PATCH] Revert "sandbox: Change md command to use map_physmem"

[Simon Glass]

Hi Kumar,

On Mon, Nov 14, 2011 at 1:11 PM, Kumar Gala <galak at 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 at 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 at 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
>>>
>>>
>
>