Hi,
On Fri, Mar 1, 2013 at 11:15 AM, Tom Rini trini@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@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@xilinx.com Tested-by: Jagannadha Sutradharudu Teki jaganna@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