[U-Boot] [PATCH] mtest: Fix end address of increment/decrement test

Peter Tyser
2010-05-20T20:11:32Z

<snip>

> > - The output of 'mtest' is misleading:
> > => mtest 0x1000 0x2000 1 1
> > Testing 00001000 ... 00002000:
> >
> > That should be "00001000 ... 00002003" then, correct? (I know it should
>
> No, it should not. The output shows the addresses where data is
> written to. If you write a 32 bit word to address 00002000, this
> writes to the byte addresses 00002000, 00002001, 00002002 and
> 00002003 (assuming a big endian system). So the output actually is
> correct.

The current output leaves a lot of interpretation up to the user.
Without digging into the code they won't know that 32-bits is written at
0x2000. They may think its a byte at 0x2000. Or maybe a 64-bit
quantity, they'd have no idea.

> > be "00001000 ... 00001fff" to be consistent with this patch's
> > implementation, so this argument is weak...)
>
> No, because we do not actually write to this address (which would also
> be misaligned for a word write).

It depends on interpretation. eg:
=> mtest 0x1000 0x1ffd 1 1
Testing 00001000 ... 00001ffd:

This is *really* testing bytes 0x1000-0x1fff. It's impossible for Joe
User to figure that out though...

> > How would you like this cleaned up? Bring the address coverage of the
> > other tests inline with the increment/decrement test? Improve the mtest
> > output so its obvious what exactly is being tested?
>
> Both, of course :-) Although I think the output is even correct, it
> just leaves room for misinterpretation.

As far as the output, my vote would be to align the end address to a
32-bit address and add 3. eg assuming a starting address of 1000 and
ending addresses of:
0x1ffc - output: Testing 00001000 ... 00001fff
0x1ffd - output: Testing 00001000 ... 00001fff
0x1ffe - output: Testing 00001000 ... 00001fff
0x1fff - output: Testing 00001000 ... 00001fff
0x2000 - output: Testing 00001000 ... 00002003
0x2001 - output: Testing 00001000 ... 00002003
...

This would tell the user explicitly what bytes have been tested and they
wouldn't have to calculate alignments or know word sizes.

Another possibility would be to replace the "end address" argument with
a "size" argument. So "mtest 0x1000 0x1000" would test 0x1000 bytes at
address 0x1000, from 0x1000-0x1fff. I think that would be clearer to
most people, but the downside is you'd have to do some math to calculate
the size parameter in some cases (eg testing the region after exception
vectors, but before the U-Boot image in RAM).

Let me know what you think about how to display the tested memory
region. I'm fine with leaving the "end addr" vs "size" debate for the
next release, if at all.

Best,
Peter