[U-Boot] usb_stor_BBB_transport 5 ms delay - performance

[Benoît Thébaudeau]

Hi Jim,

On Thu, Jul 26, 2012 at 10:20:48 PM, Jim Shimer wrote:
> I'm seeing a 5ms delay in usb_stor_BBB_transport, which occurs every
> 10K of
> data for fatload usb or 500ms of delay per 1MB of image size.  This
> adds up
> to quite a bit of delay if you're loading a large ramdisk.
> 
> Does anyone know what the reason for the 5ms delay really is?  I'm
> assuming
> that this delay is to debounce the 5V/100ma USB power up.  I made
> some
> modification, where the delay is skipped if the device has already
> been
> queried as ready.  This has save me 500ms/M on fatload times (eg,
> 140M=70seconds).  Is there anything wrong with this tweak?
> 
> Here's a diff of what I've done to get the performance I need:
> 
> --- usb_storage.c.orig  2012-07-26 16:06:40.775251000 -0400
> +++ usb_storage.c       2012-07-26 13:49:36.000000000 -0400
> @@ -132,6 +132,7 @@ static block_dev_desc_t usb_dev_desc[USB
>  struct us_data;
>  typedef int (*trans_cmnd)(ccb *cb, struct us_data *data);
>  typedef int (*trans_reset)(struct us_data *data);
> +typedef enum us_status { USB_NOT_READY, USB_READY} us_status;
> 
>  struct us_data {
>         struct usb_device *pusb_dev;     /* this usb_device */
> @@ -154,6 +155,7 @@ struct us_data {
>         ccb             *srb;                   /* current srb */
>         trans_reset     transport_reset;        /* reset routine */
>         trans_cmnd      transport;              /* transport routine
>         */
> +       us_status       status;
>  };
> 
>  static struct us_data usb_stor[USB_MAX_STOR_DEV];
> @@ -691,7 +693,10 @@ int usb_stor_BBB_transport(ccb *srb, str
>                 usb_stor_BBB_reset(us);
>                 return USB_STOR_TRANSPORT_FAILED;
>         }
> -       wait_ms(5);
> +       if(us->status != USB_READY)
> +       {
> +               wait_ms(5);
> +       }
>         pipein = usb_rcvbulkpipe(us->pusb_dev, us->ep_in);
>         pipeout = usb_sndbulkpipe(us->pusb_dev, us->ep_out);
>         /* DATA phase + error handling */
> @@ -957,7 +962,10 @@ static int usb_test_unit_ready(ccb *srb,
>                 srb->datalen = 0;
>                 srb->cmdlen = 12;
>                 if (ss->transport(srb, ss) ==
>                 USB_STOR_TRANSPORT_GOOD)
> +               {
> +                       ss->status = USB_READY;
>                         return 0;
> +               }
>                 usb_request_sense(srb, ss);
>                 wait_ms(100);
>         } while (retries--);
> @@ -965,6 +973,11 @@ static int usb_test_unit_ready(ccb *srb,
>         return -1;
>  }
> 
> +static void usb_set_unit_not_ready(struct us_data *ss)
> +{
> +       ss->status = USB_NOT_READY;
> +}
> +
>  static int usb_read_capacity(ccb *srb, struct us_data *ss)
>  {
>         int retry;
> @@ -1108,6 +1121,7 @@ retry_it:
>                 blks -= smallblks;
>                 buf_addr += srb->datalen;
>         } while (blks != 0);
> +       usb_set_unit_not_ready((struct us_data *)dev->privptr);
> 
>         USB_STOR_PRINTF("usb_read: end startblk %lx, blccnt %x buffer
> %lx\n",
>                         start, smallblks, buf_addr);
> @@ -1188,6 +1202,7 @@ retry_it:
>                 blks -= smallblks;
>                 buf_addr += srb->datalen;
>         } while (blks != 0);
> +       usb_set_unit_not_ready((struct us_data *)dev->privptr);
> 
>         USB_STOR_PRINTF("usb_write: end startblk %lx, blccnt %x
>         buffer
> %lx\n",
>                         start, smallblks, buf_addr);
> @@ -1398,6 +1413,7 @@ int usb_stor_get_info(struct usb_device
>                 cap[0] = 2880;
>                 cap[1] = 0x200;
>         }
> +       usb_set_unit_not_ready((struct us_data *)dev->privptr);
>         USB_STOR_PRINTF("Read Capacity returns: 0x%lx, 0x%lx\n",
>         cap[0],
>                         cap[1]);
>  #if 0
> 
> 
> I'd appreciate any feedback.
> Regards

I have not looked into this delay issue, but I had similar performance issues
that I fixed with the following series:
http://patchwork.ozlabs.org/patch/172052/
http://patchwork.ozlabs.org/patch/172204/
http://patchwork.ozlabs.org/patch/172054/
http://patchwork.ozlabs.org/patch/172055/
http://patchwork.ozlabs.org/patch/172056/

Your suggestion is interesting and might be a complement to my series. I don't
have time to check its correctness right now, but I'll try soon.

Best regards,
Benoît