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

2016-11-28 Thread 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2016-11-27 Thread 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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2016-11-25 Thread 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 4080 {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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2016-11-21 Thread 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 4080 {a file exceeding 4032KB}

About HTML email, I just mailed with git-send-email, but I will double-check 
the setting.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2016-11-18 Thread 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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2016-11-17 Thread 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 
> Signed-off-by: Dongwoo Lee 
> ---
>  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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2016-11-17 Thread 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 
Signed-off-by: Dongwoo Lee 
---
 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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot