On Fri, Jul 14, 2017 at 1:50 PM, Marek Vasut marex@denx.de wrote:
On 07/14/2017 01:03 PM, Masahiro Yamada wrote:
2017-07-14 19:07 GMT+09:00 Marek Vasut marex@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.thebaudeau@advansee.com AuthorDate: Fri Aug 10 18:26:50 2012 +0200 Commit: Marek Vasut marex@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 leastit should work like before.
Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> Cc: Marek Vasut <marex@denx.de> Cc: Ilya Yanok <ilya.yanok@cogentembedded.com> Cc: Stefan Herbrechtsmeier <stefan@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