[U-Boot] [PATCH] usb: Add new command to regress USB devices

[Marek Vasut]

On 03/09/2016 01:33 PM, Rajesh Bhagat wrote:
> 
> 
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Wednesday, March 09, 2016 4:59 PM
>> To: Rajat Srivastava <rajat.srivastava at nxp.com>; u-boot at lists.denx.de
>> Cc: sjg at chromium.org; Rajesh Bhagat <rajesh.bhagat at nxp.com>
>> Subject: Re: [PATCH] usb: Add new command to regress USB devices
>>
>> On 03/09/2016 12:22 PM, Rajat Srivastava wrote:
>>> This patch adds a new 'usb regress' command, that can be used to
>>> regress test a USB device. It performs the following operations:
>>>
>>> 1. starts the USB device
>>> 2. performs read/write operations
>>> 3. stops the USB device
>>> 4. verifies the contents of read/write operations
>>>
>>> Sample Output:
>>> => usb regress 81000000 82000000 32m
>>> regressing USB..
>>> starting USB...
>>> USB0:   Register 200017f NbrPorts 2
>>> Starting the controller
>>> USB XHCI 1.00
>>> scanning bus 0 for devices... 2 USB Device(s) found
>>>        scanning usb for storage devices... 1 Storage Device(s) found
>>> USB write: device 0 block # 0, count 65536 ... 65536 blocks write: OK
>>> USB read: device 0 block # 0, count 65536 ... 65536 blocks read: OK
>>> stopping USB..
>>> verifying data on addresses 0x81000000 and 0x82000000 Total of 65536
>>> word(s) were the same
>>>
>>> Signed-off-by: Rajat Srivastava <rajat.srivastava at nxp.com>
>>> Signed-off-by: Rajesh Bhagat <rajesh.bhagat at nxp.com>
>>
>>
>> Does it do anything which cannot be achieved on the command line itself using "usb
>> reset" "usb write" "usb read" "cmp" commands ?
>>
> 
> Let me share little background and motivation for addition of this command: 
> 
> 1. We need to test different make(from different vendors/model) USB devices to test USB hardware. Where we 
> generally face below issues: 
>    a. USB devices enumeration failure on Nth iteration.
>    b. USB read/write failure (incomplete transfer or data corruption) for particular data size e.g. 12M.
> 2. "usb regress" command takes size/iterations and performs all above operations in single command to reduce 
> manual overhead.
> +	"usb regress waddr raddr size [iterations] - regress a USB device\n"
> +	"    (starts, writes to waddr, reads from raddr, stops and verifies.\n"
> +	"    `size' format 1B/1K/1M/1G)\n "
> 3. We are planning to provide a patch over it to provide summary report as below:
>     Regress Report:     
>     USB enumerate: OK/ERROR (2/20)
>     USB write: OK/ERROR (2/20)
>     USB read: OK/ERROR (2/20)
>     USB verify: OK/ERROR (2/20)
> Above report can be useful to regress a USB devices, and detailed report need to referred only when anything fails. 
> 
> Please provide your opinion on the same. 

Did you measure the overhead ? I believe the overhead of implementing
this in shell is significantly lower than the overhead of the USB and
the USB stack itself, so discussing any overhead concerns here is moot.

Yet still, I cannot find a definitive answer to my question whether this
can be implemented by pure u-boot shell commands, but I think
it can be done that way. So why should I accept this patch over
something like:

while true ; do \
 usb reset & usb write .. && usb read ... && cmp ... && echo OK ; \
done

?
-- 
Best regards,
Marek Vasut