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

[Marek Vasut]

On 07/14/2017 11:46 PM, Benoît Thébaudeau wrote:
> On Fri, Jul 14, 2017 at 1:50 PM, Marek Vasut <marex at denx.de> wrote:
>> 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 .
> 
> And even way before the introduction of XHCI into U-Boot, which means
> that this 20 was targeting OHCI or proprietary HCDs, not XHCI.
> USB_MAX_READ_BLK was already set to 20 in the initial revision of
> usb_storage.c. As I said in the commit message, this 20 was certainly
> not optimal for these non-EHCI HCDs, but it restored the previous
> (i.e. pre-5dd95cf) behavior for these HCDs instead of using the 5 * 4
> KiB code, which was specific to ehci-hcd.c at that time. Without
> knowing the rationale for the legacy 20 blocks, the safest approach
> for non-EHCI HCDs was to use this value in order to avoid breaking a
> platform or something. Looking at ohci-hcd.c, it limits the transfer
> size to (N_URB_TD - 2) * 4 KiB, with N_URB_TD set to 48, so the
> maximum number of transfers would depend on the MSC block size.
> dwc2.c, isp116x-hcd.c, r8a66597-hcd.c, and sl811-hcd.c do not seem to
> have any limit caused by these drivers. The limit with the current
> XHCI code seems to be 64 * 64 KiB. So, nowadays, USB_MAX_XFER_BLK
> could be set to 65535 for all HCDs but OHCI and XHCI, which require
> specific rules depending on the MSC block size.

For whatever reason, something tells me that setting the block size to
64k for XHCI broke things, but I cannot locate the thread. But there's
something in the back of my head ...

[...]

-- 
Best regards,
Marek Vasut