On Fri, Jul 14, 2017 at 1:50 PM, Marek Vasut <ma...@denx.de> wrote:
> On 07/14/2017 01:03 PM, Masahiro Yamada wrote:
>> 2017-07-14 19:07 GMT+09:00 Marek Vasut <ma...@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.
>> commit cffcc503580907d733e694d505e12ff6ec6c679a
>> Author: Benoît Thébaudeau <benoit.thebaud...@advansee.com>
>> AuthorDate: Fri Aug 10 18:26:50 2012 +0200
>> Commit: Marek Vasut <ma...@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.thebaud...@advansee.com>
>> Cc: Marek Vasut <ma...@denx.de>
>> Cc: Ilya Yanok <ilya.ya...@cogentembedded.com>
>> Cc: Stefan Herbrechtsmeier <ste...@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,
Benoît
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
