[U-Boot] [PATCH] usb: xhci: Limit transfer length in a single TD

[U-Boot] [PATCH] usb: xhci: Limit transfer length in a single TD

[Dongwoo Lee]

The transfer request exceeding 4032KB (the maximum number of TRBs per
TD * the maximum size of transfer buffer on TRB) fails on xhci host
with timed out error or babble error state. This failure occurs when
accessing large files on USB mass-storage. Currently with xhci as well
as ehci host, the driver requests maximum 30MB (65536 blks * 512 byte)
to storage at once. However, xhci cannot handle this request because
of the reason mentioned above, even though ehci can handle this. Thus,
transfer request larger than this size should be splitted in order to
limit the length of data in a single TD.

Even though the single request is splitted into multiple requests,
the transfer speed has affected insignificantly in comparison with
ehci host: 22.6 MB/s on ehci and 22.3 MB/s on xhci for 100MB tranfer.

Reported-by: Jaehoon Chung <jh80.chung@samsung.com>
Signed-off-by: Dongwoo Lee <dwoo08.lee@samsung.com>
---
 drivers/usb/host/xhci.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 3201177..594026e 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -907,12 +907,40 @@ static int _xhci_submit_int_msg(struct usb_device *udev, unsigned long pipe,
 static int _xhci_submit_bulk_msg(struct usb_device *udev, unsigned long pipe,
 				 void *buffer, int length)
 {
+	int ret;
+	int xfer_max_per_td, xfer_length, buf_pos;
+
 	if (usb_pipetype(pipe) != PIPE_BULK) {
 		printf("non-bulk pipe (type=%lu)", usb_pipetype(pipe));
 		return -EINVAL;
 	}
 
-	return xhci_bulk_tx(udev, pipe, length, buffer);
+	/*
+	 * When transfering data exceeding the maximum number of TRBs per
+	 * TD (default 64) is requested, the transfer fails with babble
+	 * error or time out.
+	 *
+	 * Thus, huge data transfer should be splitted into multiple TDs.
+	 */
+	xfer_max_per_td = TRB_MAX_BUFF_SIZE * (TRBS_PER_SEGMENT - 1);
+
+	buf_pos = 0;
+	do {
+		if (length > xfer_max_per_td)
+			xfer_length = xfer_max_per_td;
+		else
+			xfer_length = length;
+
+		ret = xhci_bulk_tx(udev, pipe, xfer_length, buffer + buf_pos);
+		if (ret < 0)
+			return ret;
+
+		buf_pos += xfer_length;
+		length -= xfer_length;
+
+	} while (length > 0);
+
+	return ret;
 }
 
 /**
-- 
1.9.1

[U-Boot] [PATCH] usb: xhci: Limit transfer length in a single TD

[Jaehoon Chung]

Hi,

Added Marek as USB maintainer.

On 11/17/2016 01:21 PM, Dongwoo Lee wrote:
> The transfer request exceeding 4032KB (the maximum number of TRBs per
> TD * the maximum size of transfer buffer on TRB) fails on xhci host
> with timed out error or babble error state. This failure occurs when
> accessing large files on USB mass-storage. Currently with xhci as well
> as ehci host, the driver requests maximum 30MB (65536 blks * 512 byte)
> to storage at once. However, xhci cannot handle this request because
> of the reason mentioned above, even though ehci can handle this. Thus,
> transfer request larger than this size should be splitted in order to
> limit the length of data in a single TD.
> 
> Even though the single request is splitted into multiple requests,
> the transfer speed has affected insignificantly in comparison with
> ehci host: 22.6 MB/s on ehci and 22.3 MB/s on xhci for 100MB tranfer.

I don't have USB knowledge..So i wonder that this is correct way.
Have other guys ever seen the similar issue?

> 
> Reported-by: Jaehoon Chung <jh80.chung@samsung.com>
> Signed-off-by: Dongwoo Lee <dwoo08.lee@samsung.com>
> ---
>  drivers/usb/host/xhci.c | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 3201177..594026e 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -907,12 +907,40 @@ static int _xhci_submit_int_msg(struct usb_device *udev, unsigned long pipe,
>  static int _xhci_submit_bulk_msg(struct usb_device *udev, unsigned long pipe,
>  				 void *buffer, int length)
>  {
> +	int ret;
> +	int xfer_max_per_td, xfer_length, buf_pos;
> +
>  	if (usb_pipetype(pipe) != PIPE_BULK) {
>  		printf("non-bulk pipe (type=%lu)", usb_pipetype(pipe));
>  		return -EINVAL;
>  	}
>  
> -	return xhci_bulk_tx(udev, pipe, length, buffer);
> +	/*
> +	 * When transfering data exceeding the maximum number of TRBs per
> +	 * TD (default 64) is requested, the transfer fails with babble
> +	 * error or time out.
> +	 *
> +	 * Thus, huge data transfer should be splitted into multiple TDs.
> +	 */
> +	xfer_max_per_td = TRB_MAX_BUFF_SIZE * (TRBS_PER_SEGMENT - 1);

xfer_ma_per_td is constant? Then why don't define "XFER_MAX_PER_TD"?
Then can remove xfer_max_per_td  variable.

> +
> +	buf_pos = 0;

can be assigned to 0 when buf_pos is defined?

Best Regards,
Jaehoon Chung

> +	do {
> +		if (length > xfer_max_per_td)
> +			xfer_length = xfer_max_per_td;
> +		else
> +			xfer_length = length;
> +
> +		ret = xhci_bulk_tx(udev, pipe, xfer_length, buffer + buf_pos);
> +		if (ret < 0)
> +			return ret;
> +
> +		buf_pos += xfer_length;
> +		length -= xfer_length;
> +
> +	} while (length > 0);
> +
> +	return ret;
>  }
>  
>  /**
> 

[U-Boot] [PATCH] usb: xhci: Limit transfer length in a single TD

[Marek Vasut]

On 11/18/2016 08:24 AM, Jaehoon Chung wrote:
> Hi,
> 
> Added Marek as USB maintainer.
> 
> On 11/17/2016 01:21 PM, Dongwoo Lee wrote:
>> The transfer request exceeding 4032KB (the maximum number of TRBs per
>> TD * the maximum size of transfer buffer on TRB) fails on xhci host
>> with timed out error or babble error state. This failure occurs when
>> accessing large files on USB mass-storage. Currently with xhci as well
>> as ehci host, the driver requests maximum 30MB (65536 blks * 512 byte)
>> to storage at once. However, xhci cannot handle this request because
>> of the reason mentioned above, even though ehci can handle this. Thus,
>> transfer request larger than this size should be splitted in order to
>> limit the length of data in a single TD.
>>
>> Even though the single request is splitted into multiple requests,
>> the transfer speed has affected insignificantly in comparison with
>> ehci host: 22.6 MB/s on ehci and 22.3 MB/s on xhci for 100MB tranfer.
> 
> I don't have USB knowledge..So i wonder that this is correct way.
> Have other guys ever seen the similar issue?

Is this a controller limitation ?

btw can you fix your mailer to NOT send HTML email to the list?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] usb: xhci: Limit transfer length in a single TD

[Dongwoo Lee]

On 2016? 11? 18? 23:01, Marek Vasut wrote:
> On 11/18/2016 08:24 AM, Jaehoon Chung wrote:
>> Hi,
>>
>> Added Marek as USB maintainer.
>>
>> On 11/17/2016 01:21 PM, Dongwoo Lee wrote:
>>> The transfer request exceeding 4032KB (the maximum number of TRBs per
>>> TD * the maximum size of transfer buffer on TRB) fails on xhci host
>>> with timed out error or babble error state. This failure occurs when
>>> accessing large files on USB mass-storage. Currently with xhci as well
>>> as ehci host, the driver requests maximum 30MB (65536 blks * 512 byte)
>>> to storage at once. However, xhci cannot handle this request because
>>> of the reason mentioned above, even though ehci can handle this. Thus,
>>> transfer request larger than this size should be splitted in order to
>>> limit the length of data in a single TD.
>>>
>>> Even though the single request is splitted into multiple requests,
>>> the transfer speed has affected insignificantly in comparison with
>>> ehci host: 22.6 MB/s on ehci and 22.3 MB/s on xhci for 100MB tranfer.
>>
>> I don't have USB knowledge..So i wonder that this is correct way.
>> Have other guys ever seen the similar issue?
> 
> Is this a controller limitation ?
> 
> btw can you fix your mailer to NOT send HTML email to the list?
> 

If my understanding for xhci spec.(rev. 1.1) 4.9.2 is right, the controller 
has no limitation for transfer size because it can support a very large TRB 
ring with multiple Ring Segments. 

However, the xhci driver seems not to be implemented for supporting it; 
the TRB ring is comprised of only a single segment. As a result, it cannot 
handle the request exceeding 4032KB (TRB_MAX_BUFF_SIZE(64KB) * 
(TRBS_PER_SEGMENT(64) - link TRB(1)), thus the request should be divided.  

This issue can be reproduced by using the following command on Odroid-XU3/XU4
with USB mass-storage connected to xhci host:

	>fatload usb 0 40800000 {a file exceeding 4032KB}

About HTML email, I just mailed with git-send-email, but I will double-check 
the setting.

[U-Boot] [PATCH] usb: xhci: Limit transfer length in a single TD

[Marek Vasut]

On 11/22/2016 03:42 AM, Dongwoo Lee wrote:
> On 2016? 11? 18? 23:01, Marek Vasut wrote:
>> On 11/18/2016 08:24 AM, Jaehoon Chung wrote:
>>> Hi,
>>>
>>> Added Marek as USB maintainer.
>>>
>>> On 11/17/2016 01:21 PM, Dongwoo Lee wrote:
>>>> The transfer request exceeding 4032KB (the maximum number of TRBs per
>>>> TD * the maximum size of transfer buffer on TRB) fails on xhci host
>>>> with timed out error or babble error state. This failure occurs when
>>>> accessing large files on USB mass-storage. Currently with xhci as well
>>>> as ehci host, the driver requests maximum 30MB (65536 blks * 512 byte)
>>>> to storage at once. However, xhci cannot handle this request because
>>>> of the reason mentioned above, even though ehci can handle this. Thus,
>>>> transfer request larger than this size should be splitted in order to
>>>> limit the length of data in a single TD.
>>>>
>>>> Even though the single request is splitted into multiple requests,
>>>> the transfer speed has affected insignificantly in comparison with
>>>> ehci host: 22.6 MB/s on ehci and 22.3 MB/s on xhci for 100MB tranfer.
>>>
>>> I don't have USB knowledge..So i wonder that this is correct way.
>>> Have other guys ever seen the similar issue?
>>
>> Is this a controller limitation ?
>>
>> btw can you fix your mailer to NOT send HTML email to the list?
>>
> 
> If my understanding for xhci spec.(rev. 1.1) 4.9.2 is right, the controller 
> has no limitation for transfer size because it can support a very large TRB 
> ring with multiple Ring Segments. 
> 
> However, the xhci driver seems not to be implemented for supporting it; 
> the TRB ring is comprised of only a single segment. As a result, it cannot 
> handle the request exceeding 4032KB (TRB_MAX_BUFF_SIZE(64KB) * 
> (TRBS_PER_SEGMENT(64) - link TRB(1)), thus the request should be divided.  

Can we update the driver ?

> This issue can be reproduced by using the following command on Odroid-XU3/XU4
> with USB mass-storage connected to xhci host:
> 
> 	>fatload usb 0 40800000 {a file exceeding 4032KB}
> 
> About HTML email, I just mailed with git-send-email, but I will double-check 
> the setting.

OK

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] usb: xhci: Limit transfer length in a single TD

[Dongwoo Lee]

On 11/26/2016 03:25 AM, Marek Vasut wrote:
> On 11/22/2016 03:42 AM, Dongwoo Lee wrote:
>> On 2016? 11? 18? 23:01, Marek Vasut wrote:
>>> On 11/18/2016 08:24 AM, Jaehoon Chung wrote:
>>>> Hi,
>>>>
>>>> Added Marek as USB maintainer.
>>>>
>>>> On 11/17/2016 01:21 PM, Dongwoo Lee wrote:
>>>>> The transfer request exceeding 4032KB (the maximum number of TRBs per
>>>>> TD * the maximum size of transfer buffer on TRB) fails on xhci host
>>>>> with timed out error or babble error state. This failure occurs when
>>>>> accessing large files on USB mass-storage. Currently with xhci as well
>>>>> as ehci host, the driver requests maximum 30MB (65536 blks * 512 byte)
>>>>> to storage at once. However, xhci cannot handle this request because
>>>>> of the reason mentioned above, even though ehci can handle this. Thus,
>>>>> transfer request larger than this size should be splitted in order to
>>>>> limit the length of data in a single TD.
>>>>>
>>>>> Even though the single request is splitted into multiple requests,
>>>>> the transfer speed has affected insignificantly in comparison with
>>>>> ehci host: 22.6 MB/s on ehci and 22.3 MB/s on xhci for 100MB tranfer.
>>>>
>>>> I don't have USB knowledge..So i wonder that this is correct way.
>>>> Have other guys ever seen the similar issue?
>>>
>>> Is this a controller limitation ?
>>>
>>> btw can you fix your mailer to NOT send HTML email to the list?
>>>
>>
>> If my understanding for xhci spec.(rev. 1.1) 4.9.2 is right, the controller 
>> has no limitation for transfer size because it can support a very large TRB 
>> ring with multiple Ring Segments. 
>>
>> However, the xhci driver seems not to be implemented for supporting it; 
>> the TRB ring is comprised of only a single segment. As a result, it cannot 
>> handle the request exceeding 4032KB (TRB_MAX_BUFF_SIZE(64KB) * 
>> (TRBS_PER_SEGMENT(64) - link TRB(1)), thus the request should be divided.  
> 
> Can we update the driver ?
> 

Yes, I agree. 
I think also updating driver is more reasonable.

Though I think it takes some time since I just started xhci, I will
prepare a patch for enabling multiple ring segments for the driver.

[U-Boot] [PATCH] usb: xhci: Limit transfer length in a single TD

[Marek Vasut]

On 11/28/2016 07:15 AM, Dongwoo Lee wrote:
> On 11/26/2016 03:25 AM, Marek Vasut wrote:
>> On 11/22/2016 03:42 AM, Dongwoo Lee wrote:
>>> On 2016? 11? 18? 23:01, Marek Vasut wrote:
>>>> On 11/18/2016 08:24 AM, Jaehoon Chung wrote:
>>>>> Hi,
>>>>>
>>>>> Added Marek as USB maintainer.
>>>>>
>>>>> On 11/17/2016 01:21 PM, Dongwoo Lee wrote:
>>>>>> The transfer request exceeding 4032KB (the maximum number of TRBs per
>>>>>> TD * the maximum size of transfer buffer on TRB) fails on xhci host
>>>>>> with timed out error or babble error state. This failure occurs when
>>>>>> accessing large files on USB mass-storage. Currently with xhci as well
>>>>>> as ehci host, the driver requests maximum 30MB (65536 blks * 512 byte)
>>>>>> to storage at once. However, xhci cannot handle this request because
>>>>>> of the reason mentioned above, even though ehci can handle this. Thus,
>>>>>> transfer request larger than this size should be splitted in order to
>>>>>> limit the length of data in a single TD.
>>>>>>
>>>>>> Even though the single request is splitted into multiple requests,
>>>>>> the transfer speed has affected insignificantly in comparison with
>>>>>> ehci host: 22.6 MB/s on ehci and 22.3 MB/s on xhci for 100MB tranfer.
>>>>>
>>>>> I don't have USB knowledge..So i wonder that this is correct way.
>>>>> Have other guys ever seen the similar issue?
>>>>
>>>> Is this a controller limitation ?
>>>>
>>>> btw can you fix your mailer to NOT send HTML email to the list?
>>>>
>>>
>>> If my understanding for xhci spec.(rev. 1.1) 4.9.2 is right, the controller 
>>> has no limitation for transfer size because it can support a very large TRB 
>>> ring with multiple Ring Segments. 
>>>
>>> However, the xhci driver seems not to be implemented for supporting it; 
>>> the TRB ring is comprised of only a single segment. As a result, it cannot 
>>> handle the request exceeding 4032KB (TRB_MAX_BUFF_SIZE(64KB) * 
>>> (TRBS_PER_SEGMENT(64) - link TRB(1)), thus the request should be divided.  
>>
>> Can we update the driver ?
>>
> 
> Yes, I agree. 
> I think also updating driver is more reasonable.
> 
> Though I think it takes some time since I just started xhci, I will
> prepare a patch for enabling multiple ring segments for the driver.

Great, thanks!


-- 
Best regards,
Marek Vasut