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

Peter Tyser
2010-05-20T19:07:21Z

Hi Wolfgang,

> In message <1274375283-13004-1-git-send-email-ptyser@xes-inc.com> you wrote:
> >
> > The incrememt/decrement test has an off-by-one error which results in an
> > extra 4 bytes being tested past the specified end address. For
> > instance, when running "mtest 0x1000 0x2000", the bytes 0x2000-0x2003
> > would be tested which is counterintuitive and at odds with the end
> > address calculation of other U-Boot memory tests.
>
> I disagree. I understand your reasoning, but actually it has always
> been the case that commands that take an address reagion specify as
> end address the last address to be used, not oni behind the range.
> You may not like this, but that's how it has been implemented > 10
> years ago, and many people are trained on this behaviour. See for
> example the flash erase command, wehre you will type something like
>
> => erase 40040000 4007FFFF
>
> Here, like in other places, we really use the end address.
>
> So for the sake of consisteny I tend to reject your patch.

I can see your point but the current memtest code is not consistent with
your description.
- Every other test other than the increment/decrement tests the region <
end address. Eg in the start:0x1000 end:0x2000 example, the *only* test
that touches 0x2000-0x2003 region is the increment/decrement test.
Either its broken, or the other memory test functions are.

- 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
be "00001000 ... 00001fff" to be consistent with this patch's
implementation, so this argument is weak...)

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?

Best,
Peter