[U-Boot] [PATCH] usb_storage: USB storage transfer size increase for xHCI

Increase xHCI transfer size for USB storage devices. This helps to achieve 10-20x speedup for large transfers
Signed-off-by: Sergey Temerkhanov s.temerkhanov@gmail.com Signed-off-by: Radha Mohan Chintakuntla rchintakuntla@cavium.com ---
common/usb_storage.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/common/usb_storage.c b/common/usb_storage.c index b978430..ee5acca 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -97,13 +97,15 @@ struct us_data { trans_cmnd transport; /* transport routine */ };
-#ifdef CONFIG_USB_EHCI +#if defined(CONFIG_USB_EHCI) /* * 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 +#elif defined(CONFIG_USB_XHCI) +#define USB_MAX_XFER_BLK 4096 #else #define USB_MAX_XFER_BLK 20 #endif

On Thursday, August 13, 2015 at 09:00:04 PM, Sergey Temerkhanov wrote:
Increase xHCI transfer size for USB storage devices. This helps to achieve 10-20x speedup for large transfers
Signed-off-by: Sergey Temerkhanov s.temerkhanov@gmail.com Signed-off-by: Radha Mohan Chintakuntla rchintakuntla@cavium.com
Hi!
common/usb_storage.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/common/usb_storage.c b/common/usb_storage.c index b978430..ee5acca 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -97,13 +97,15 @@ struct us_data { trans_cmnd transport; /* transport routine */ };
-#ifdef CONFIG_USB_EHCI +#if defined(CONFIG_USB_EHCI) /*
- 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 +#elif defined(CONFIG_USB_XHCI)
Why don't you keep the XHCI consistent with EHCI, ie 64k transfers as well ?
+#define USB_MAX_XFER_BLK 4096 #else #define USB_MAX_XFER_BLK 20 #endif
Best regards, Marek Vasut

Tried different values but transfer sizes larger than ~8k blocks never complete on some controllers causing timeouts and crashes. So, 4k blocks is a safe enough xfer size
Regards, Sergey
On Thu, Aug 13, 2015 at 10:09 PM, Marek Vasut marex@denx.de wrote:
On Thursday, August 13, 2015 at 09:00:04 PM, Sergey Temerkhanov wrote:
Increase xHCI transfer size for USB storage devices. This helps to achieve 10-20x speedup for large transfers
Signed-off-by: Sergey Temerkhanov s.temerkhanov@gmail.com Signed-off-by: Radha Mohan Chintakuntla rchintakuntla@cavium.com
Hi!
common/usb_storage.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/common/usb_storage.c b/common/usb_storage.c index b978430..ee5acca 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -97,13 +97,15 @@ struct us_data { trans_cmnd transport; /* transport routine */ };
-#ifdef CONFIG_USB_EHCI +#if defined(CONFIG_USB_EHCI) /*
- 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 +#elif defined(CONFIG_USB_XHCI)
Why don't you keep the XHCI consistent with EHCI, ie 64k transfers as well ?
+#define USB_MAX_XFER_BLK 4096 #else #define USB_MAX_XFER_BLK 20 #endif
Best regards, Marek Vasut

On Thursday, August 13, 2015 at 09:13:52 PM, Sergei Temerkhanov wrote:
Tried different values but transfer sizes larger than ~8k blocks never complete on some controllers causing timeouts and crashes. So, 4k blocks is a safe enough xfer size
Would you please elaborate on this ?
Best regards, Marek Vasut

Well, when I was working on this, setting large transfer sizes resulted in, AFAIR, xhci_wait_for_event() timeout and changing XHCI_TIMEOUT doesn't help. This function returns NULL which is being dereferenced somewhere else (I don't remember where exactly), there are several places in the generic xhci support code where this is possible - I think it was abort_td().
Regards, Sergey
On Thu, Aug 13, 2015 at 10:34 PM, Marek Vasut marex@denx.de wrote:
On Thursday, August 13, 2015 at 09:13:52 PM, Sergei Temerkhanov wrote:
Tried different values but transfer sizes larger than ~8k blocks never complete on some controllers causing timeouts and crashes. So, 4k blocks
is
a safe enough xfer size
Would you please elaborate on this ?
Best regards, Marek Vasut

On Friday, August 14, 2015 at 12:48:57 AM, Sergei Temerkhanov wrote:
Hi,
please stop top-posting :(
Well, when I was working on this, setting large transfer sizes resulted in, AFAIR, xhci_wait_for_event() timeout and changing XHCI_TIMEOUT doesn't help. This function returns NULL which is being dereferenced somewhere else (I don't remember where exactly), there are several places in the generic xhci support code where this is possible - I think it was abort_td().
Can you please send fixes for those null-pointer dereferences you're triggering? Also, I suspect that if your controller cannot support arbitrary lenght of the descriptors, it might be a controller bug and a quirk should be introduced. Does the controller work in Linux ?
Regards, Sergey
On Thu, Aug 13, 2015 at 10:34 PM, Marek Vasut marex@denx.de wrote:
On Thursday, August 13, 2015 at 09:13:52 PM, Sergei Temerkhanov wrote:
Tried different values but transfer sizes larger than ~8k blocks never complete on some controllers causing timeouts and crashes. So, 4k blocks
is
a safe enough xfer size
Would you please elaborate on this ?
Best regards, Marek Vasut
Best regards, Marek Vasut
participants (3)
-
Marek Vasut
-
Sergei Temerkhanov
-
Sergey Temerkhanov