Hi Max,
On Tue, Jun 7, 2016 at 12:57 PM, Max Krummenacher <max.oss...@gmail.com> wrote:
> Hi Benoît,
>
> Thank you for your review.
You're welcome.
> I wanted to wait for Scott's patchseries to make it into master to
> allow for potential needed
> changes.
No problem.
> 2016-05-31 22:21 GMT+02:00 Benoît Thébaudeau
> <benoit.thebaudeau....@gmail.com>:
> ...
>>> Extend this by allowing for a second parameter specifying the byte offset
>>> to the last block to be tested.
>>>
>>
>> End offsets are always ambiguous because users can hesitate between
>> the offset of the first byte of the last block, the offset of the last
>> byte of the last block, and the offset of the first byte of the block
>> following the last one (if any). A byte size would probably be better
>> here, and it would also be more consistent with the other nand
>> commands.
>
> Ok. I will change the interface to use offset/size.
Good.
> ...
>>> NAND torture: device 0 offset 0x1000000 size 0x20000
>>> passed 2, failed 0
>>>
>>
>> With more than one block to test, the printed size becomes ambiguous
>> here. It would be better to indicate that it is the erase size of the
>> block. The total test size could also be printed, either instead of
>> the erase size, or besides it.
>>
> Ok. I will change this to print test size and block size, e.g. like:
> NAND torture: device 0 offset 0x1000000 size 0x40000 (nand block size 0x20000)
Good.
> ...
>>> - return ret == 0 ? 0 : 1;
>>> + ret = nand_torture(nand, off);
>>> + if (ret) {
> ...
>>
>> A size parameter could probably be added to nand_torture() instead of
>> handling the range in the command, so that the direct usages of
>> nand_torture() (in or out of tree) can also benefit from this
>> enhancement.
>
> I disagree here.
> Likely one uses the extended functionality only during HW bringup and
> only interactively. If one would want to test multiple blocks from code one
> would also want to know the testresult of each individual block rather
> than only having a return parameter indicating a 'all good' or
> 'at least one block failed'.
> Even here in the interactive 'nand torture' cmd the printf of a failed
> block in the loop
> is a usecase of this.
Makes sense. Agreed.
Best regards,
Benoît
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
