All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v1] Revert "fastboot: OUT transaction length must be aligned to wMaxPacketSize"
Date: Wed, 23 Mar 2016 03:04:40 +0100	[thread overview]
Message-ID: <56F1F9B8.8090607@denx.de> (raw)
In-Reply-To: <1458694241-15455-1-git-send-email-srae@broadcom.com>

On 03/23/2016 01:50 AM, Steve Rae wrote:
> This reverts commit 9e4b510d40310bf46e09f4edd0a0b6356213df47.
> 
> Signed-off-by: Steve Rae <srae@broadcom.com>
> ---
> As discussed on the mailing list, this change breaks the download portion
> of fastboot by causing the server (the device running U-Boot) to wait forever
> for bytes which are never sent by the client (the host connected via USB).

It'd be real cool if you could reference the ML discussion next time.

> Does anyone know how to contact:
>   Dileep Katta <dileep.katta@linaro.org>
> (this email bounces) ?

No clue, sorry.

Applied for now as it fixes real bug.

>  drivers/usb/gadget/f_fastboot.c | 27 +++++----------------------
>  1 file changed, 5 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
> index a54b4ee..887cf4f 100644
> --- a/drivers/usb/gadget/f_fastboot.c
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -57,7 +57,6 @@ static struct f_fastboot *fastboot_func;
>  static unsigned int fastboot_flash_session_id;
>  static unsigned int download_size;
>  static unsigned int download_bytes;
> -static bool is_high_speed;
>  
>  static struct usb_endpoint_descriptor fs_ep_in = {
>  	.bLength            = USB_DT_ENDPOINT_SIZE,
> @@ -241,13 +240,10 @@ static int fastboot_set_alt(struct usb_function *f,
>  	      __func__, f->name, interface, alt);
>  
>  	/* make sure we don't enable the ep twice */
> -	if (gadget->speed == USB_SPEED_HIGH) {
> +	if (gadget->speed == USB_SPEED_HIGH)
>  		ret = usb_ep_enable(f_fb->out_ep, &hs_ep_out);
> -		is_high_speed = true;
> -	} else {
> +	else
>  		ret = usb_ep_enable(f_fb->out_ep, &fs_ep_out);
> -		is_high_speed = false;
> -	}
>  	if (ret) {
>  		puts("failed to enable out ep\n");
>  		return ret;
> @@ -419,20 +415,13 @@ static void cb_getvar(struct usb_ep *ep, struct usb_request *req)
>  	fastboot_tx_write_str(response);
>  }
>  
> -static unsigned int rx_bytes_expected(unsigned int maxpacket)
> +static unsigned int rx_bytes_expected(void)
>  {
>  	int rx_remain = download_size - download_bytes;
> -	int rem = 0;
>  	if (rx_remain < 0)
>  		return 0;
>  	if (rx_remain > EP_BUFFER_SIZE)
>  		return EP_BUFFER_SIZE;
> -	if (rx_remain < maxpacket) {
> -		rx_remain = maxpacket;
> -	} else if (rx_remain % maxpacket != 0) {
> -		rem = rx_remain % maxpacket;
> -		rx_remain = rx_remain + (maxpacket - rem);
> -	}
>  	return rx_remain;
>  }
>  
> @@ -444,7 +433,6 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req)
>  	const unsigned char *buffer = req->buf;
>  	unsigned int buffer_size = req->actual;
>  	unsigned int pre_dot_num, now_dot_num;
> -	unsigned int max;
>  
>  	if (req->status != 0) {
>  		printf("Bad status: %d\n", req->status);
> @@ -482,9 +470,7 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req)
>  
>  		printf("\ndownloading of %d bytes finished\n", download_bytes);
>  	} else {
> -		max = is_high_speed ? hs_ep_out.wMaxPacketSize :
> -				fs_ep_out.wMaxPacketSize;
> -		req->length = rx_bytes_expected(max);
> +		req->length = rx_bytes_expected();
>  		if (req->length < ep->maxpacket)
>  			req->length = ep->maxpacket;
>  	}
> @@ -497,7 +483,6 @@ static void cb_download(struct usb_ep *ep, struct usb_request *req)
>  {
>  	char *cmd = req->buf;
>  	char response[FASTBOOT_RESPONSE_LEN];
> -	unsigned int max;
>  
>  	strsep(&cmd, ":");
>  	download_size = simple_strtoul(cmd, NULL, 16);
> @@ -513,9 +498,7 @@ static void cb_download(struct usb_ep *ep, struct usb_request *req)
>  	} else {
>  		sprintf(response, "DATA%08x", download_size);
>  		req->complete = rx_handler_dl_image;
> -		max = is_high_speed ? hs_ep_out.wMaxPacketSize :
> -			fs_ep_out.wMaxPacketSize;
> -		req->length = rx_bytes_expected(max);
> +		req->length = rx_bytes_expected();
>  		if (req->length < ep->maxpacket)
>  			req->length = ep->maxpacket;
>  	}
> 


-- 
Best regards,
Marek Vasut

  reply	other threads:[~2016-03-23  2:04 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-23  0:50 [U-Boot] [PATCH v1] Revert "fastboot: OUT transaction length must be aligned to wMaxPacketSize" Steve Rae
2016-03-23  2:04 ` Marek Vasut [this message]
2016-03-23  9:49   ` Lukasz Majewski
2016-03-23 15:54     ` Sam Protsenko
2016-03-23 17:40       ` Steve Rae
2016-03-23 19:40         ` Sam Protsenko
2016-03-23 19:49           ` Steve Rae
2016-04-03 14:07             ` Sam Protsenko
2016-03-24 17:15 ` Tom Rini
2016-04-03 14:19   ` Sam Protsenko
2016-04-04 17:16     ` Steve Rae
2016-04-05 12:09       ` Sam Protsenko
2016-04-05 12:59         ` Marek Vasut

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56F1F9B8.8090607@denx.de \
    --to=marex@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.