[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 at samsung.com>
> Signed-off-by: Dongwoo Lee <dwoo08.lee at 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;
>  }
>  
>  /**
>