Skip to contentSkip to conversation

usb_storage: USB storage transfer size increase for xHCI

PATCH New Sergey Temerkhanov 2 participants 9 messages Aug 13, 2015 – Aug 14, 2015 1 message not in the archive

lore
Share & tools
lore

Sergey Temerkhanov

author
#0 lore raw

PATCH 1/1 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 <[email protected]>
Signed-off-by Radha Mohan Chintakuntla <[email protected]>

---

 common/usb_storage.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
common/usb_storage.c +3 -1
index b978430..ee5acca 100644
@@ -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
Signature · 1 line
2.2.0
#1 lore raw

PATCH 1/1 usb_storage: USB storage transfer size increase for xHCI

Quoted 5 lines · Sergey Temerkhanov

Increase xHCI transfer size for USB storage devices. This helps to achieve 10-20x speedup for large transfers

Signed-off-by Sergey Temerkhanov <[email protected]>
Signed-off-by Radha Mohan Chintakuntla <[email protected]>

Hi!

Quoted 18 lines

---

 common/usb_storage.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
common/usb_storage.c +1 -1
index b978430..ee5acca 100644
@@ -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)
Quoted 4 lines

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 ?

Quoted 4 lines

+#define USB_MAX_XFER_BLK 4096
#else
#define USB_MAX_XFER_BLK 20
#endif

Best regards,
Marek Vasut

author
#2 lore raw

PATCH 1/1 usb_storage: USB storage transfer size increase for xHCI

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

Quoted 44 lines · Marek Vasut
Quoted 5 lines · Sergey Temerkhanov

Increase xHCI transfer size for USB storage devices. This helps to achieve 10-20x speedup for large transfers

Signed-off-by Sergey Temerkhanov <[email protected]>
Signed-off-by Radha Mohan Chintakuntla <[email protected]>

Hi!

Quoted 10 lines

---

 common/usb_storage.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
common/usb_storage.c +0 -0
index b978430..ee5acca 100644

common/usb_storage.c struct us_data { · line 102

Quoted 7 lines
common/usb_storage.c +1 -1
@@ -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

Quoted 5 lines
common/usb_storage.c +0 -0
 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 ?

Quoted 4 lines

+#define USB_MAX_XFER_BLK 4096
#else
#define USB_MAX_XFER_BLK 20
#endif

Best regards,
Marek Vasut

#3 lore raw

PATCH 1/1 usb_storage: USB storage transfer size increase for xHCI

Quoted 3 lines · Sergei Temerkhanov

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

A message that is not in the archive (sent off-list or not yet received): CAPEA6dY71DZhHb61c_+RfdvsM0PvzXRxY1nSw9zFPaqvTYzUgQ@mail.gmail.com

#5 lore raw

PATCH 1/1 usb_storage: USB storage transfer size increase for xHCI

On Thursday, August 13, 2015 at 10:56:27 PM, Sergei Temerkhanov wrote:

Hi!

(please stop top-posting ; please keep the list in the loop)

Quoted 5 lines

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 suspect we might need a quirk for the XHCI instead them (quirk that the controller gets stuck when you try to queue arbitrary number of descriptors). But I wonder, don't you have some cache problems instead ? Try disabling dcache (CONFIG_SYS_DCACHE_OFF) on your machine and see if that fixes the problem .

Best regards,
Marek Vasut

author
#6 lore raw

PATCH 1/1 usb_storage: USB storage transfer size increase for xHCI

OK, I'll try to find some time for this. I remember trying it with DCACHE disabled but for no luck

Regards,
Sergey

Quoted 24 lines · Marek Vasut

On Thursday, August 13, 2015 at 10:56:27 PM, Sergei Temerkhanov wrote:

Hi!

(please stop top-posting ; please keep the list in the loop)

Quoted 1 line

Well, when I was working on this, setting large transfer sizes resulted

in,

Quoted 2 lines

AFAIR, xhci_wait_for_event() timeout and changing XHCI_TIMEOUT doesn't help. This function returns NULL which is being dereferenced somewhere

else

Quoted 2 lines

(I don't remember where exactly), there are several places in the generic xhci support code where this is possible.

I suspect we might need a quirk for the XHCI instead them (quirk that the controller gets stuck when you try to queue arbitrary number of descriptors).
But I wonder, don't you have some cache problems instead ? Try disabling dcache (CONFIG_SYS_DCACHE_OFF) on your machine and see if that fixes the problem .

Best regards,
Marek Vasut

#7 lore raw

PATCH 1/1 usb_storage: USB storage transfer size increase for xHCI

Quoted 2 lines · Sergei Temerkhanov

OK, I'll try to find some time for this. I remember trying it with DCACHE disabled but for no luck

Cool, thanks. Btw please stop top-posting ;)

Quoted 31 lines

Regards,
Sergey

Quoted 27 lines · Marek Vasut

On Thursday, August 13, 2015 at 10:56:27 PM, Sergei Temerkhanov wrote:

Hi!

(please stop top-posting ; please keep the list in the loop)

Quoted 1 line

Well, when I was working on this, setting large transfer sizes resulted

in,

Quoted 2 lines

AFAIR, xhci_wait_for_event() timeout and changing XHCI_TIMEOUT doesn't help. This function returns NULL which is being dereferenced somewhere

else

Quoted 2 lines

(I don't remember where exactly), there are several places in the generic xhci support code where this is possible.

I suspect we might need a quirk for the XHCI instead them (quirk that the controller gets stuck when you try to queue arbitrary number of descriptors).
But I wonder, don't you have some cache problems instead ? Try disabling dcache (CONFIG_SYS_DCACHE_OFF) on your machine and see if that fixes the problem .

Best regards,
Marek Vasut

Best regards,
Marek Vasut

author
#8 lore raw

PATCH 1/1 usb_storage: USB storage transfer size increase for xHCI

In reply to 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

Quoted 11 lines · Marek Vasut
Quoted 2 lines · Sergei Temerkhanov

Tried different values but transfer sizes larger than ~8k blocks never complete on some controllers causing timeouts and crashes. So, 4k blocks

is

Quoted 1 line

a safe enough xfer size

Would you please elaborate on this ?

Best regards,
Marek Vasut

#9 lore raw

PATCH 1/1 usb_storage: USB storage transfer size increase for xHCI

On Friday, August 14, 2015 at 12:48:57 AM, Sergei Temerkhanov wrote:

Hi,

please stop top-posting :(

Quoted 5 lines

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 ?

Quoted 17 lines

Regards,
Sergey

Quoted 13 lines · Marek Vasut
Quoted 3 lines · Sergei Temerkhanov

Tried different values but transfer sizes larger than ~8k blocks never complete on some controllers causing timeouts and crashes. So, 4k blocks

is

Quoted 1 line

a safe enough xfer size

Would you please elaborate on this ?

Best regards,
Marek Vasut

Best regards,
Marek Vasut