[U-Boot] [PATCH v2] usb: fix usb_stor_read/write on DM

[Marek Vasut]

On 07/14/2017 01:03 PM, Masahiro Yamada wrote:
> 2017-07-14 19:07 GMT+09:00 Marek Vasut <marex at denx.de>:
>> On 07/14/2017 04:31 AM, Masahiro Yamada wrote:
>>> Prior to DM, we could not enable different types of USB controllers
>>> at the same time.  DM was supposed to loosen the limitation.  It is
>>> true that we can compile drivers, but they do not work.
>>>
>>> For example, if EHCI is enabled, xHCI fails as follows:
>>>
>>>   => usb read 82000000 0 2000
>>>
>>>   USB read: device 0 block # 0, count 8192 ... WARN halted endpoint, queueing URB anyway.
>>>   Unexpected XHCI event TRB, skipping... (3fb54010 00000001 13000000 01008401)
>>>   BUG: failure at drivers/usb/host/xhci-ring.c:489/abort_td()!
>>>   BUG!
>>>   ### ERROR ### Please RESET the board ###
>>>
>>> The cause of the error seems the following code:
>>>
>>>   #ifdef CONFIG_USB_EHCI_HCD
>>>   /*
>>>    * The U-Boot EHCI driver can handle any transfer length as long as there is
>>>    * enough free heap space left, but the SCSI READ(10) and WRITE(10) commands are
>>>    * limited to 65535 blocks.
>>>    */
>>>   #define USB_MAX_XFER_BLK    65535
>>>   #else
>>>   #define USB_MAX_XFER_BLK    20
>>>   #endif
>>>
>>> To fix the problem, choose the chunk size at run-time for CONFIG_BLK.
>>
>> What happens if CONFIG_BLK is not set ?
> 
> USB_MAX_XFER_BLK is chosen.

And can we fix that even for non-CONFIG_BLK ?

>> Why is it 20 for XHCI anyway ?
> 
> You are the maintainer.
> (I hope) you have better knowledge with this.

Heh, way to deflect the question. I seem to remember some discussion
about the DMA (?) limitation on XHCI, but I'd have to dig through the ML
archives myself.

> Looks like the following commit was picked up by you.

5 years ago, way before DM was what it is today .

> commit cffcc503580907d733e694d505e12ff6ec6c679a
> Author:     Benoît Thébaudeau <benoit.thebaudeau at advansee.com>
> AuthorDate: Fri Aug 10 18:26:50 2012 +0200
> Commit:     Marek Vasut <marex at denx.de>
> CommitDate: Sat Sep 1 16:21:52 2012 +0200
> 
>     usb_storage: Restore non-EHCI support
> 
>     The commit 5dd95cf made the MSC driver EHCI-specific. This patch restores a
>     basic support of non-EHCI HCDs, like before that commit.
> 
>     The fallback transfer size is certainly not optimal, but at least
> it should work
>     like before.
> 
>     Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau at advansee.com>
>     Cc: Marek Vasut <marex at denx.de>
>     Cc: Ilya Yanok <ilya.yanok at cogentembedded.com>
>     Cc: Stefan Herbrechtsmeier <stefan at herbrechtsmeier.net>
> 
> diff --git a/common/usb_storage.c b/common/usb_storage.c
> index bdc306f587fd..0cd6399a3c42 100644
> --- a/common/usb_storage.c
> +++ b/common/usb_storage.c
> @@ -155,11 +155,15 @@ struct us_data {
>         trans_cmnd      transport;              /* transport routine */
>  };
> 
> +#ifdef CONFIG_USB_EHCI
>  /*
>   * The U-Boot EHCI driver cannot handle more than 5 page aligned buffers
>   * of 4096 bytes in a transfer without running itself out of qt_buffers
>   */
>  #define USB_MAX_XFER_BLK(start, blksz) (((4096 * 5) - (start % 4096)) / blksz)
> +#else
> +#define USB_MAX_XFER_BLK(start, blksz) 20
> +#endif
> 
>  static struct us_data usb_stor[USB_MAX_STOR_DEV];
> 
> 
> 
> 
> 
> 
> 


-- 
Best regards,
Marek Vasut