[U-Boot] [PATCH] cmd_mem: Decode the mtest start and end values from fdt

[Simon Glass]

Hi,

On Fri, Mar 1, 2013 at 11:15 AM, Tom Rini <trini at ti.com> wrote:
> On Fri, Mar 01, 2013 at 05:06:48PM +0100, Wolfgang Denk wrote:
>> Dear Jagannadha Sutradharudu Teki,
>>
>> In message <10597224-d520-4a3f-8185-5de018ee5046 at TX2EHSMHS026.ehs.local> you wrote:
>> > This patch provides a support to decode the mtest start
>> > and end values from fdt config node.
>> >
>> > Signed-off-by: Jagannadha Sutradharudu Teki <jaganna at xilinx.com>
>> > Tested-by: Jagannadha Sutradharudu Teki <jaganna at xilinx.com>
>> > ---
>> >  common/cmd_mem.c |    6 ++++--
>> >  common/main.c    |   18 ++++++++++++++++++
>> >  2 files changed, 22 insertions(+), 2 deletions(-)
>>
>> You are adding code here which may bot be user and/or wanted by the
>> majority of boards, so please make it configureable (and document the
>> new config option).
>
> In addition, if we're going to start whacking around in here, can we
> just make this function bail if not passed a start and end address?  I'm
> a little torn since on the one hand this is a known function, but on the
> other it's not something one would expect to be used outside of testing
> environments.

I am not sure that storing the memory addresses in an env variable is
all that useful, but OK. If there is an env variable feature then it
should override the FDT. So perhaps you could create a function which
returns the mtest start and end address, something like this perhaps:

/* Should this return an error? */
int memtest_get_region(ulong *start, ulong *end)
{
   *start = CONFIG_SYS_MEMTEST_START;
   *end = CONFIG_SYS_MEMTEST_END;
#ifdef CONFIG_OF_CONTROL
   fdtdec_get_config_int() with default
   (actually fdt_get_addr() might be better if enhanced to support a default)
#endif
   /* maybe read from environment var here, if CONFIG enabled for that */
   return 0;
}

Regards,
Simon