[U-Boot] [PATCH v1 1/3] fastboot: OUT transaction length must be aligned to wMaxPacketSize

[U-Boot] [PATCH v1 1/3] fastboot: OUT transaction length must be aligned to wMaxPacketSize

[Dileep Katta]

OUT transactions must be aligned to wMaxPacketSize for each transfer,
or else transfer will not complete successfully. This patch modifies
rx_bytes_expected to return a transfer length that is aligned to
wMaxPacketSize.

Note that the value of ep->desc->wMaxPacketSize and ep->maxpacket
may not be the same value, and it is the value of ep->desc->wMaxPacketSize
that should be used for alignment.

Signed-off-by: Dileep Katta <dileep.katta@linaro.org>
---
 drivers/usb/gadget/f_fastboot.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index a8d8205..0d53a61 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -370,13 +370,20 @@ static void cb_getvar(struct usb_ep *ep, struct usb_request *req)
 	fastboot_tx_write_str(response);
 }
 
-static unsigned int rx_bytes_expected(void)
+static unsigned int rx_bytes_expected(unsigned maxpacket)
 {
 	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;
 }
 
@@ -425,7 +432,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 {
-		req->length = rx_bytes_expected();
+		req->length = rx_bytes_expected(ep->desc->wMaxPacketSize);
 		if (req->length < ep->maxpacket)
 			req->length = ep->maxpacket;
 	}
@@ -453,7 +460,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;
-		req->length = rx_bytes_expected();
+		req->length = rx_bytes_expected(ep->desc->wMaxPacketSize);
 		if (req->length < ep->maxpacket)
 			req->length = ep->maxpacket;
 	}
-- 
1.8.3.2

[U-Boot] [PATCH v1 2/3] fastboot: Correct fastboot_fail and fastboot_okay strings

[Dileep Katta]

If the string is copied without NULL termination using strncpy(),
then strncat() on the next line, may concatenate the string after
some stale (or random) data, if the response string was not
zero-initialized.

Signed-off-by: Dileep Katta <dileep.katta@linaro.org>
---
 common/fb_mmc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/fb_mmc.c b/common/fb_mmc.c
index 3911989..73055cc 100644
--- a/common/fb_mmc.c
+++ b/common/fb_mmc.c
@@ -23,13 +23,13 @@ static char *response_str;
 
 void fastboot_fail(const char *s)
 {
-	strncpy(response_str, "FAIL", 4);
+	strncpy(response_str, "FAIL\0", 5);
 	strncat(response_str, s, RESPONSE_LEN - 4 - 1);
 }
 
 void fastboot_okay(const char *s)
 {
-	strncpy(response_str, "OKAY", 4);
+	strncpy(response_str, "OKAY\0", 5);
 	strncat(response_str, s, RESPONSE_LEN - 4 - 1);
 }
 
-- 
1.8.3.2

[U-Boot] [PATCH v1 2/3] fastboot: Correct fastboot_fail and fastboot_okay strings

[Steve Rae]

On 15-02-12 10:33 PM, Dileep Katta wrote:
> If the string is copied without NULL termination using strncpy(),
> then strncat() on the next line, may concatenate the string after
> some stale (or random) data, if the response string was not
> zero-initialized.
>
> Signed-off-by: Dileep Katta <dileep.katta@linaro.org>
> ---
>   common/fb_mmc.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/common/fb_mmc.c b/common/fb_mmc.c
> index 3911989..73055cc 100644
> --- a/common/fb_mmc.c
> +++ b/common/fb_mmc.c
> @@ -23,13 +23,13 @@ static char *response_str;
>
>   void fastboot_fail(const char *s)
>   {
> -	strncpy(response_str, "FAIL", 4);
> +	strncpy(response_str, "FAIL\0", 5);
>   	strncat(response_str, s, RESPONSE_LEN - 4 - 1);
>   }
>
>   void fastboot_okay(const char *s)
>   {
> -	strncpy(response_str, "OKAY", 4);
> +	strncpy(response_str, "OKAY\0", 5);
>   	strncat(response_str, s, RESPONSE_LEN - 4 - 1);
>   }
>
>

THANKS!

Reviewed-by: Steve Rae <srae@broadcom.com>

[U-Boot] [PATCH v1 2/3] fastboot: Correct fastboot_fail and fastboot_okay strings

[Lukasz Majewski]

Hi Dileep,

> If the string is copied without NULL termination using strncpy(),
> then strncat() on the next line, may concatenate the string after
> some stale (or random) data, if the response string was not
> zero-initialized.
> 
> Signed-off-by: Dileep Katta <dileep.katta@linaro.org>
> ---
>  common/fb_mmc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/common/fb_mmc.c b/common/fb_mmc.c
> index 3911989..73055cc 100644
> --- a/common/fb_mmc.c
> +++ b/common/fb_mmc.c
> @@ -23,13 +23,13 @@ static char *response_str;
>  
>  void fastboot_fail(const char *s)
>  {
> -	strncpy(response_str, "FAIL", 4);
> +	strncpy(response_str, "FAIL\0", 5);
>  	strncat(response_str, s, RESPONSE_LEN - 4 - 1);
>  }
>  
>  void fastboot_okay(const char *s)
>  {
> -	strncpy(response_str, "OKAY", 4);
> +	strncpy(response_str, "OKAY\0", 5);
>  	strncat(response_str, s, RESPONSE_LEN - 4 - 1);
>  }
>  

Applied to u-boot-dfu branch.

Thanks for the patch!

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

[U-Boot] [PATCH v1 3/3] usb: gadget: fastboot: Set the Serial Number for Fastboot Gadget

[Dileep Katta]

Configure the serial number using the serial# environment variable
during the fastboot bind.

This enables "fastboot devices" to return the serial number for
the attached devices.

Signed-off-by: Dileep Katta <dileep.katta@linaro.org>
---
 drivers/usb/gadget/f_fastboot.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index 0d53a61..d114c07 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -136,6 +136,7 @@ static int fastboot_bind(struct usb_configuration *c, struct usb_function *f)
 	int id;
 	struct usb_gadget *gadget = c->cdev->gadget;
 	struct f_fastboot *f_fb = func_to_fastboot(f);
+	const char *s;
 
 	/* DYNAMIC interface numbers assignments */
 	id = usb_interface_id(c, f);
@@ -161,6 +162,10 @@ static int fastboot_bind(struct usb_configuration *c, struct usb_function *f)
 
 	hs_ep_out.bEndpointAddress = fs_ep_out.bEndpointAddress;
 
+	s = getenv("serial#");
+	if (s)
+		g_dnl_set_serialnumber((char *)s);
+
 	return 0;
 }
 
-- 
1.8.3.2

[U-Boot] [PATCH v1 3/3] usb: gadget: fastboot: Set the Serial Number for Fastboot Gadget

[Steve Rae]

On 15-02-12 10:33 PM, Dileep Katta wrote:
> Configure the serial number using the serial# environment variable
> during the fastboot bind.
>
> This enables "fastboot devices" to return the serial number for
> the attached devices.
>
> Signed-off-by: Dileep Katta <dileep.katta@linaro.org>
> ---
>   drivers/usb/gadget/f_fastboot.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
> index 0d53a61..d114c07 100644
> --- a/drivers/usb/gadget/f_fastboot.c
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -136,6 +136,7 @@ static int fastboot_bind(struct usb_configuration *c, struct usb_function *f)
>   	int id;
>   	struct usb_gadget *gadget = c->cdev->gadget;
>   	struct f_fastboot *f_fb = func_to_fastboot(f);
> +	const char *s;
>
>   	/* DYNAMIC interface numbers assignments */
>   	id = usb_interface_id(c, f);
> @@ -161,6 +162,10 @@ static int fastboot_bind(struct usb_configuration *c, struct usb_function *f)
>
>   	hs_ep_out.bEndpointAddress = fs_ep_out.bEndpointAddress;
>
> +	s = getenv("serial#");
> +	if (s)
> +		g_dnl_set_serialnumber((char *)s);
> +
>   	return 0;
>   }
>
>

Acked-by: Steve Rae <srae@broadcom.com>

[U-Boot] [PATCH v1 3/3] usb: gadget: fastboot: Set the Serial Number for Fastboot Gadget

[Lukasz Majewski]

Hi Dileep,

> Configure the serial number using the serial# environment variable
> during the fastboot bind.
> 
> This enables "fastboot devices" to return the serial number for
> the attached devices.
> 
> Signed-off-by: Dileep Katta <dileep.katta@linaro.org>
> ---
>  drivers/usb/gadget/f_fastboot.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/gadget/f_fastboot.c
> b/drivers/usb/gadget/f_fastboot.c index 0d53a61..d114c07 100644
> --- a/drivers/usb/gadget/f_fastboot.c
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -136,6 +136,7 @@ static int fastboot_bind(struct usb_configuration
> *c, struct usb_function *f) int id;
>  	struct usb_gadget *gadget = c->cdev->gadget;
>  	struct f_fastboot *f_fb = func_to_fastboot(f);
> +	const char *s;
>  
>  	/* DYNAMIC interface numbers assignments */
>  	id = usb_interface_id(c, f);
> @@ -161,6 +162,10 @@ static int fastboot_bind(struct
> usb_configuration *c, struct usb_function *f) 
>  	hs_ep_out.bEndpointAddress = fs_ep_out.bEndpointAddress;
>  
> +	s = getenv("serial#");
> +	if (s)
> +		g_dnl_set_serialnumber((char *)s);
> +
>  	return 0;
>  }
>  

Applied to u-boot-dfu branch.

Thanks for the patch!

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

[U-Boot] [PATCH v2 1/3] fastboot: OUT transaction length must be aligned to wMaxPacketSize

[Dileep Katta]

OUT transactions must be aligned to wMaxPacketSize for each transfer,
or else transfer will not complete successfully. This patch modifies
rx_bytes_expected to return a transfer length that is aligned to
wMaxPacketSize.

Note that the value of wMaxPacketSize and ep->maxpacket may not be
the same value, and it is the value of wMaxPacketSize that should be
used for alignment.

Signed-off-by: Dileep Katta <dileep.katta@linaro.org>
---
Changes from v1:
	- Corrected source of wMaxPacketSize
 drivers/usb/gadget/f_fastboot.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index a8d8205..b18452e 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -370,13 +370,20 @@ static void cb_getvar(struct usb_ep *ep, struct usb_request *req)
 	fastboot_tx_write_str(response);
 }
 
-static unsigned int rx_bytes_expected(void)
+static unsigned int rx_bytes_expected(unsigned maxpacket)
 {
 	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;
 }
 
@@ -425,7 +432,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 {
-		req->length = rx_bytes_expected();
+		req->length = rx_bytes_expected(fs_ep_out.wMaxPacketSize);
 		if (req->length < ep->maxpacket)
 			req->length = ep->maxpacket;
 	}
@@ -453,7 +460,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;
-		req->length = rx_bytes_expected();
+		req->length = rx_bytes_expected(fs_ep_out.wMaxPacketSize);
 		if (req->length < ep->maxpacket)
 			req->length = ep->maxpacket;
 	}
-- 
1.8.3.2

[U-Boot] [PATCH v2 1/3] fastboot: OUT transaction length must be aligned to wMaxPacketSize

[Stegmaier, Angela]

Hi Dileep,

> -----Original Message-----
> From: Dileep Katta [mailto:dileep.katta at linaro.org]
> Sent: Friday, February 13, 2015 11:59 AM
> To: u-boot at lists.denx.de; robherring2 at gmail.com; Rini, Tom;
> rob.herring at linaro.org; srae at broadcom.com; l.majewski at samsung.com;
> Stegmaier, Angela
> Cc: Dileep Katta
> Subject: [U-Boot][PATCH v2 1/3] fastboot: OUT transaction length must be
> aligned to wMaxPacketSize
> 
> OUT transactions must be aligned to wMaxPacketSize for each transfer, or
> else transfer will not complete successfully. This patch modifies
> rx_bytes_expected to return a transfer length that is aligned to
> wMaxPacketSize.
> 
> Note that the value of wMaxPacketSize and ep->maxpacket may not be the
> same value, and it is the value of wMaxPacketSize that should be used for
> alignment.
> 
> Signed-off-by: Dileep Katta <dileep.katta@linaro.org>
> ---
> Changes from v1:
> 	- Corrected source of wMaxPacketSize
>  drivers/usb/gadget/f_fastboot.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/gadget/f_fastboot.c
> b/drivers/usb/gadget/f_fastboot.c index a8d8205..b18452e 100644
> --- a/drivers/usb/gadget/f_fastboot.c
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -370,13 +370,20 @@ static void cb_getvar(struct usb_ep *ep, struct
> usb_request *req)
>  	fastboot_tx_write_str(response);
>  }
> 
> -static unsigned int rx_bytes_expected(void)
> +static unsigned int rx_bytes_expected(unsigned maxpacket)
>  {
>  	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;
>  }
> 
> @@ -425,7 +432,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 {
> -		req->length = rx_bytes_expected();
> +		req->length =
> rx_bytes_expected(fs_ep_out.wMaxPacketSize);

This will not be the correct value in the case of HS connections.

Thanks,
Angela

>  		if (req->length < ep->maxpacket)
>  			req->length = ep->maxpacket;
>  	}
> @@ -453,7 +460,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;
> -		req->length = rx_bytes_expected();
> +		req->length =
> rx_bytes_expected(fs_ep_out.wMaxPacketSize);
>  		if (req->length < ep->maxpacket)
>  			req->length = ep->maxpacket;
>  	}
> --
> 1.8.3.2

[U-Boot] [PATCH v3 1/3] fastboot: OUT transaction length must be aligned to wMaxPacketSize

[Dileep Katta]

OUT transactions must be aligned to wMaxPacketSize for each transfer,
or else transfer will not complete successfully. This patch modifies
rx_bytes_expected to return a transfer length that is aligned to
wMaxPacketSize.

Note that the value of wMaxPacketSize and ep->maxpacket may not be
the same value, and it is the value of wMaxPacketSize that should be
used for alignment. wMaxPacketSize is passed depending on the speed of
connection.

Signed-off-by: Dileep Katta <dileep.katta@linaro.org>
---
Changes in v2:
	- Corrected source of wMaxPacketSize
Changes in v3:
	- Corrected the logic to accomodate both HS and FS speeds

 drivers/usb/gadget/f_fastboot.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index a8d8205..2793590 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -55,6 +55,7 @@ static inline struct f_fastboot *func_to_fastboot(struct usb_function *f)
 static struct f_fastboot *fastboot_func;
 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,
@@ -219,10 +220,13 @@ 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);
-	else
+		is_high_speed = true;
+	} 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;
@@ -370,13 +374,20 @@ static void cb_getvar(struct usb_ep *ep, struct usb_request *req)
 	fastboot_tx_write_str(response);
 }
 
-static unsigned int rx_bytes_expected(void)
+static unsigned int rx_bytes_expected(unsigned int maxpacket)
 {
 	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;
 }
 
@@ -388,6 +399,7 @@ 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);
@@ -425,7 +437,9 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req)
 
 		printf("\ndownloading of %d bytes finished\n", download_bytes);
 	} else {
-		req->length = rx_bytes_expected();
+		max = is_high_speed ? hs_ep_out.wMaxPacketSize :
+				fs_ep_out.wMaxPacketSize;
+		req->length = rx_bytes_expected(max);
 		if (req->length < ep->maxpacket)
 			req->length = ep->maxpacket;
 	}
@@ -438,6 +452,7 @@ static void cb_download(struct usb_ep *ep, struct usb_request *req)
 {
 	char *cmd = req->buf;
 	char response[RESPONSE_LEN];
+	unsigned int max;
 
 	strsep(&cmd, ":");
 	download_size = simple_strtoul(cmd, NULL, 16);
@@ -453,7 +468,9 @@ 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;
-		req->length = rx_bytes_expected();
+		max = is_high_speed ? hs_ep_out.wMaxPacketSize :
+			fs_ep_out.wMaxPacketSize;
+		req->length = rx_bytes_expected(max);
 		if (req->length < ep->maxpacket)
 			req->length = ep->maxpacket;
 	}
-- 
1.8.3.2

[U-Boot] [PATCH v3 1/3] fastboot: OUT transaction length must be aligned to wMaxPacketSize

[Lukasz Majewski]

Hi Dileep,

> OUT transactions must be aligned to wMaxPacketSize for each transfer,
> or else transfer will not complete successfully. This patch modifies
> rx_bytes_expected to return a transfer length that is aligned to
> wMaxPacketSize.
> 
> Note that the value of wMaxPacketSize and ep->maxpacket may not be
> the same value, and it is the value of wMaxPacketSize that should be
> used for alignment. wMaxPacketSize is passed depending on the speed of
> connection.
> 
> Signed-off-by: Dileep Katta <dileep.katta@linaro.org>
> ---
> Changes in v2:
> 	- Corrected source of wMaxPacketSize
> Changes in v3:
> 	- Corrected the logic to accomodate both HS and FS speeds
> 
>  drivers/usb/gadget/f_fastboot.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/gadget/f_fastboot.c
> b/drivers/usb/gadget/f_fastboot.c index a8d8205..2793590 100644
> --- a/drivers/usb/gadget/f_fastboot.c
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -55,6 +55,7 @@ static inline struct f_fastboot
> *func_to_fastboot(struct usb_function *f) static struct f_fastboot
> *fastboot_func; 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,
> @@ -219,10 +220,13 @@ 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);
> -	else
> +		is_high_speed = true;
> +	} 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;
> @@ -370,13 +374,20 @@ static void cb_getvar(struct usb_ep *ep, struct
> usb_request *req) fastboot_tx_write_str(response);
>  }
>  
> -static unsigned int rx_bytes_expected(void)
> +static unsigned int rx_bytes_expected(unsigned int maxpacket)
>  {
>  	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;
>  }
>  
> @@ -388,6 +399,7 @@ 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);
> @@ -425,7 +437,9 @@ static void rx_handler_dl_image(struct usb_ep
> *ep, struct usb_request *req) 
>  		printf("\ndownloading of %d bytes finished\n",
> download_bytes); } else {
> -		req->length = rx_bytes_expected();
> +		max = is_high_speed ? hs_ep_out.wMaxPacketSize :
> +				fs_ep_out.wMaxPacketSize;
> +		req->length = rx_bytes_expected(max);
>  		if (req->length < ep->maxpacket)
>  			req->length = ep->maxpacket;
>  	}
> @@ -438,6 +452,7 @@ static void cb_download(struct usb_ep *ep, struct
> usb_request *req) {
>  	char *cmd = req->buf;
>  	char response[RESPONSE_LEN];
> +	unsigned int max;
>  
>  	strsep(&cmd, ":");
>  	download_size = simple_strtoul(cmd, NULL, 16);
> @@ -453,7 +468,9 @@ 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;
> -		req->length = rx_bytes_expected();
> +		max = is_high_speed ? hs_ep_out.wMaxPacketSize :
> +			fs_ep_out.wMaxPacketSize;
> +		req->length = rx_bytes_expected(max);
>  		if (req->length < ep->maxpacket)
>  			req->length = ep->maxpacket;
>  	}

Applied to u-boot-dfu branch.

Thanks for the patch!

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

[U-Boot] Anyone else having issues with "fastboot flash" - because of this change?

[Steve Rae]

... updated the subject line, was:
Re: [U-Boot][PATCH v3 1/3] fastboot: OUT transaction length must be 
aligned to wMaxPacketSize


On 15-02-24 02:28 AM, Lukasz Majewski wrote:
> Hi Dileep,
>
>> OUT transactions must be aligned to wMaxPacketSize for each transfer,
>> or else transfer will not complete successfully. This patch modifies
>> rx_bytes_expected to return a transfer length that is aligned to
>> wMaxPacketSize.
>>
>> Note that the value of wMaxPacketSize and ep->maxpacket may not be
>> the same value, and it is the value of wMaxPacketSize that should be
>> used for alignment. wMaxPacketSize is passed depending on the speed of
>> connection.
>>
>> Signed-off-by: Dileep Katta <dileep.katta@linaro.org>
>> ---
>> Changes in v2:
>> 	- Corrected source of wMaxPacketSize
>> Changes in v3:
>> 	- Corrected the logic to accomodate both HS and FS speeds
>>
>>   drivers/usb/gadget/f_fastboot.c | 27 ++++++++++++++++++++++-----
>>   1 file changed, 22 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/f_fastboot.c
>> b/drivers/usb/gadget/f_fastboot.c index a8d8205..2793590 100644
>> --- a/drivers/usb/gadget/f_fastboot.c
>> +++ b/drivers/usb/gadget/f_fastboot.c
>> @@ -55,6 +55,7 @@ static inline struct f_fastboot
>> *func_to_fastboot(struct usb_function *f) static struct f_fastboot
>> *fastboot_func; 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,
>> @@ -219,10 +220,13 @@ 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);
>> -	else
>> +		is_high_speed = true;
>> +	} 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;
>> @@ -370,13 +374,20 @@ static void cb_getvar(struct usb_ep *ep, struct
>> usb_request *req) fastboot_tx_write_str(response);
>>   }
>>
>> -static unsigned int rx_bytes_expected(void)
>> +static unsigned int rx_bytes_expected(unsigned int maxpacket)
>>   {
>>   	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);
>> +	}

Is anyone else having problems with this code???
I need to remove these six (newly added) lines in order to get my boards 
to work -- otherwise, they just "hang" druing the download phase of 
"fastboot flash"....

Thanks in advance, Steve

>>   	return rx_remain;
>>   }
>>
>> @@ -388,6 +399,7 @@ 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);
>> @@ -425,7 +437,9 @@ static void rx_handler_dl_image(struct usb_ep
>> *ep, struct usb_request *req)
>>   		printf("\ndownloading of %d bytes finished\n",
>> download_bytes); } else {
>> -		req->length = rx_bytes_expected();
>> +		max = is_high_speed ? hs_ep_out.wMaxPacketSize :
>> +				fs_ep_out.wMaxPacketSize;
>> +		req->length = rx_bytes_expected(max);
>>   		if (req->length < ep->maxpacket)
>>   			req->length = ep->maxpacket;
>>   	}
>> @@ -438,6 +452,7 @@ static void cb_download(struct usb_ep *ep, struct
>> usb_request *req) {
>>   	char *cmd = req->buf;
>>   	char response[RESPONSE_LEN];
>> +	unsigned int max;
>>
>>   	strsep(&cmd, ":");
>>   	download_size = simple_strtoul(cmd, NULL, 16);
>> @@ -453,7 +468,9 @@ 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;
>> -		req->length = rx_bytes_expected();
>> +		max = is_high_speed ? hs_ep_out.wMaxPacketSize :
>> +			fs_ep_out.wMaxPacketSize;
>> +		req->length = rx_bytes_expected(max);
>>   		if (req->length < ep->maxpacket)
>>   			req->length = ep->maxpacket;
>>   	}
>
> Applied to u-boot-dfu branch.
>
> Thanks for the patch!
>

[U-Boot] Anyone else having issues with "fastboot flash" - because of this change?

[Steve Rae]

On Thu, Mar 10, 2016 at 3:46 PM, Steve Rae <srae@broadcom.com> wrote:
> ... updated the subject line, was:
> Re: [U-Boot][PATCH v3 1/3] fastboot: OUT transaction length must be aligned
> to wMaxPacketSize
>
>


>>> -static unsigned int rx_bytes_expected(void)
>>> +static unsigned int rx_bytes_expected(unsigned int maxpacket)
>>>   {
>>>         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);
>>> +       }
>
>
> Is anyone else having problems with this code???
> I need to remove these six (newly added) lines in order to get my boards to
> work -- otherwise, they just "hang" druing the download phase of "fastboot
> flash"....
>
> Thanks in advance, Steve
>


MORE INFO:

I added some logs to see what is happening:

diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index f87aae7..824430f 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -427,12 +427,17 @@ static unsigned int rx_bytes_expected(unsigned
int maxpacket)
                return 0;
        if (rx_remain > EP_BUFFER_SIZE)
                return EP_BUFFER_SIZE;
+#if 1
        if (rx_remain < maxpacket) {
+               printf("\nVALIDATE: %s: %d (%d) %d\n", __func__,
rx_remain, maxpacket, maxpacket);
                rx_remain = maxpacket;
        } else if (rx_remain % maxpacket != 0) {
                rem = rx_remain % maxpacket;
+               printf("\nVALIDATE: %s: %d (%d) %d %d\n", __func__,
rx_remain, maxpacket, rem, rx_remain + (maxpacket - rem));
                rx_remain = rx_remain + (maxpacket - rem);
        }
+#endif
+       printf("\nVALIDATE: using %d\n", rx_remain);
        return rx_remain;
 }

RESULTS:

In the case that fails, (#if 1), it reports:
VALIDATE: rx_bytes_expected: 812 (512) 300 1024
VALIDATE: using 1024

In the case that works (#if 0), it reports:
VALIDATE: using 812

To me, it looks like there is only 812 bytes more to be downloaded,
but this code modifies that value to expect 1024 bytes;
and I guess it hangs after the 812 have been received (while waiting
for the rest of the 1024)....

Please advise!

[U-Boot] [PATCH v2 1/3] fastboot: OUT transaction length must be aligned to wMaxPacketSize

[Steve Rae]

On 15-02-13 09:59 AM, Dileep Katta wrote:
> OUT transactions must be aligned to wMaxPacketSize for each transfer,
> or else transfer will not complete successfully. This patch modifies
> rx_bytes_expected to return a transfer length that is aligned to
> wMaxPacketSize.
>
> Note that the value of wMaxPacketSize and ep->maxpacket may not be
> the same value, and it is the value of wMaxPacketSize that should be
> used for alignment.
>
> Signed-off-by: Dileep Katta <dileep.katta@linaro.org>
> ---
> Changes from v1:
> 	- Corrected source of wMaxPacketSize
>   drivers/usb/gadget/f_fastboot.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
> index a8d8205..b18452e 100644
> --- a/drivers/usb/gadget/f_fastboot.c
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -370,13 +370,20 @@ static void cb_getvar(struct usb_ep *ep, struct usb_request *req)
>   	fastboot_tx_write_str(response);
>   }
>
> -static unsigned int rx_bytes_expected(void)
> +static unsigned int rx_bytes_expected(unsigned maxpacket)
>   {
>   	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;
>   }
>
> @@ -425,7 +432,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 {
> -		req->length = rx_bytes_expected();
> +		req->length = rx_bytes_expected(fs_ep_out.wMaxPacketSize);
>   		if (req->length < ep->maxpacket)
>   			req->length = ep->maxpacket;
>   	}
> @@ -453,7 +460,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;
> -		req->length = rx_bytes_expected();
> +		req->length = rx_bytes_expected(fs_ep_out.wMaxPacketSize);
>   		if (req->length < ep->maxpacket)
>   			req->length = ep->maxpacket;
>   	}
>

I don't understand...
Why are we always limiting this to the "Full Speed" size?
( see fs_ep_out versus hs_ep_out defined previously in this file... )

Thanks, Steve