Revert "fastboot: OUT transaction length must be aligned to wMaxPacketSize"
PATCH 1/1 Revert "fastboot: OUT transaction length must be aligned to wMaxPacketSize"
This reverts commit 9e4b510d40310bf46e09f4edd0a0b6356213df47.
---
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).
Does anyone know how to contact:
Dileep Katta <[email protected]>
(this email bounces) ?
drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-)
drivers/usb/gadget/f_fastboot.c +5 -22
@@ -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; }
Signature · 1 line
1.8.5
PATCH 1/1 Revert "fastboot: OUT transaction length must be aligned to wMaxPacketSize"
Quoted 7 lines · Steve Rae
This reverts commit
9e4b510d40310bf46e09f4edd0a0b6356213df47.Signed-off-by Steve Rae <[email protected]>---
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.
Quoted 3 lines
Does anyone know how to contact:
Dileep Katta <[email protected]>
(this email bounces) ?
No clue, sorry.
Applied for now as it fixes real bug.
Quoted 92 lines
drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-)drivers/usb/gadget/f_fastboot.c +5
-22@@ -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; }
Signature · 2 lines
Best regards, Marek Vasut
PATCH 1/1 Revert "fastboot: OUT transaction length must be aligned to wMaxPacketSize"
Hi Marek,
Quoted 10 lines · Steve Rae
Quoted 8 lines
This reverts commit
9e4b510d40310bf46e09f4edd0a0b6356213df47.Signed-off-by Steve Rae <[email protected]>---
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).
Quoted 7 lines
It'd be real cool if you could reference the ML discussion next time.
Quoted 3 lines
Does anyone know how to contact:
Dileep Katta <[email protected]>
(this email bounces) ?No clue, sorry.
Probably he is no longer employed by Linaro.
Quoted 2 lines
Applied for now as it fixes real bug.
I've a gut feeling that there are different versions of "fastboot" protocol.
What Steve is describing is that HOST is waiting for ZLP ACK.
Anyway, I'm not able to test fastboot gadget since our (I mean Samsung's) boards aren't supporting fastboot - we use UMS, DFU, LTHOR.
Hence, I can only rely on somebody's testing and review the code without the ability to know about implementation "quirks".
Quoted 95 lines
Quoted 93 lines
drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex a54b4ee..887cf4f 100644drivers/usb/gadget/f_fastboot.c +5
-22@@ -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; }
Signature · 5 lines
Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
PATCH 1/1 Revert "fastboot: OUT transaction length must be aligned to wMaxPacketSize"
Quoted 137 lines · Lukasz Majewski
Hi Marek,
Quoted 10 lines · Steve Rae
Quoted 8 lines
This reverts commit
9e4b510d40310bf46e09f4edd0a0b6356213df47.Signed-off-by Steve Rae <[email protected]>---
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).Quoted 7 lines
It'd be real cool if you could reference the ML discussion next time.
Quoted 3 lines
Does anyone know how to contact:
Dileep Katta <[email protected]>
(this email bounces) ?No clue, sorry.
Probably he is no longer employed by Linaro.
Quoted 2 lines
Applied for now as it fixes real bug.
I've a gut feeling that there are different versions of "fastboot" protocol.
What Steve is describing is that HOST is waiting for ZLP ACK.
Anyway, I'm not able to test fastboot gadget since our (I mean Samsung's) boards aren't supporting fastboot - we use UMS, DFU, LTHOR.
Hence, I can only rely on somebody's testing and review the code without the ability to know about implementation "quirks".
Quoted 79 lines
Quoted 79 lines
drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex a54b4ee..887cf4f 100644drivers/usb/gadget/f_fastboot.c +4
-19@@ -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;Quoted 16 lines
Quoted 4 lines
drivers/usb/gadget/f_fastboot.c +0
-0strsep (&cmd,":" ); download_size =simple_strtoul (cmd,NULL ,16 );drivers/usb/gadget/f_fastboot.c static void cb_download(struct usb_ep *ep, · line 504
Quoted 10 lines
drivers/usb/gadget/f_fastboot.c +1
-3@@ -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,
Quoted 7 lines
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
+ Angela Stegmaier
Steve,
Actually this code was originally added by Angela Stegmaier (see [1]). Before reverting it, we should ask her opinion on that.
I'm gonna verify this change on DRA7 EVM board today and see if "fastboot flash" works fine (and it certainly was, before this revert).
[1] http://omapzoom.org/?p=repo/u-boot.git;a=commit;h=92471c8af13493491a4118473279077fa6b765cf
PATCH 1/1 Revert "fastboot: OUT transaction length must be aligned to wMaxPacketSize"
Lukasz & Sam:
Quoted 23 lines · Sam Protsenko
Quoted 20 lines · Lukasz Majewski
Hi Marek,
Quoted 17 lines · Steve Rae
Quoted 8 lines
This reverts commit
9e4b510d40310bf46e09f4edd0a0b6356213df47.Signed-off-by Steve Rae <[email protected]>---
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.
Quoted 3 lines
Does anyone know how to contact:
Dileep Katta <[email protected]>
(this email bounces) ?No clue, sorry.
Quoted 8 lines
Quoted 8 lines
Probably he is no longer employed by Linaro.
Quoted 2 lines
Applied for now as it fixes real bug.
I've a gut feeling that there are different versions of "fastboot" protocol.
Yes - I suspect that also, which is why I test with two (2) fastboot clients:
(1) from 2013 (and I don't know its origin), and
(2) from the Adroid SDK (r24.4.1) for Linux - from developer.android.com; which requires
the "Android SDK Platform-tools (rev 23.1)" to be installed...
[ this is the current version - checked yesterday! ]
Quoted 1 line
Quoted 1 line
What Steve is describing is that HOST is waiting for ZLP ACK.
Lukasz:
Is there any debugging I can enable to verify this?
Quoted 118 lines
Quoted 108 lines
Anyway, I'm not able to test fastboot gadget since our (I mean Samsung's) boards aren't supporting fastboot - we use UMS, DFU, LTHOR.
Hence, I can only rely on somebody's testing and review the code without the ability to know about implementation "quirks".
Quoted 79 lines
Quoted 79 lines
drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex a54b4ee..887cf4f 100644drivers/usb/gadget/f_fastboot.c +4
-19@@ -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;Quoted 16 lines
Quoted 4 lines
drivers/usb/gadget/f_fastboot.c +0
-0strsep (&cmd,":" ); download_size =simple_strtoul (cmd,NULL ,16 );drivers/usb/gadget/f_fastboot.c static void cb_download(struct usb_ep *ep, · line 504
Quoted 10 lines
drivers/usb/gadget/f_fastboot.c +1
-3@@ -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,Quoted 7 lines
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot+ Angela Stegmaier
Quoted 8 lines
Steve,
Actually this code was originally added by Angela Stegmaier (see [1]). Before reverting it, we should ask her opinion on that.
I'm gonna verify this change on DRA7 EVM board today and see if "fastboot flash" works fine (and it certainly was, before this revert).
Sam:
Thanks!
Could you please add the following printf lines, and let me know the results?
drivers/usb/gadget/f_fastboot.c +0 -0
@@ -427,12 +427,14 @@ static unsigned int rx_bytes_expected(unsigned
int maxpacket)
return 0;
if (rx_remain > EP_BUFFER_SIZE)
return EP_BUFFER_SIZE;
+ printf("\nFIXME: rx_remain (before) = %d\n", rx_remain);
if (rx_remain < maxpacket) {
rx_remain = maxpacket;
} else if (rx_remain % maxpacket != 0) {
rem = rx_remain % maxpacket;
rx_remain = rx_remain + (maxpacket - rem);
}
+ printf("\nFIXME: rx_remain (after) = %d\n", rx_remain);
return rx_remain;}
mine show:
FIXME: rx_remain (before) = 4072
FIXME: rx_remain (after) = 4096
and then it just hangs forever....
PATCH 1/1 Revert "fastboot: OUT transaction length must be aligned to wMaxPacketSize"
Quoted 200 lines · Steve Rae
Lukasz & Sam:
Quoted 23 lines · Sam Protsenko
Quoted 20 lines · Lukasz Majewski
Hi Marek,
Quoted 17 lines · Steve Rae
Quoted 8 lines
This reverts commit
9e4b510d40310bf46e09f4edd0a0b6356213df47.Signed-off-by Steve Rae <[email protected]>---
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.
Quoted 3 lines
Does anyone know how to contact:
Dileep Katta <[email protected]>
(this email bounces) ?No clue, sorry.
Quoted 8 lines
Quoted 8 lines
Probably he is no longer employed by Linaro.
Quoted 2 lines
Applied for now as it fixes real bug.
I've a gut feeling that there are different versions of "fastboot" protocol.
Yes - I suspect that also, which is why I test with two (2) fastboot clients:
(1) from 2013 (and I don't know its origin), and
(2) from the Adroid SDK (r24.4.1) for Linux - from developer.android.com; which requires
the "Android SDK Platform-tools (rev 23.1)" to be installed...
[ this is the current version - checked yesterday! ]Quoted 1 line
Quoted 1 line
What Steve is describing is that HOST is waiting for ZLP ACK.
Lukasz:
Is there any debugging I can enable to verify this?Quoted 118 lines
Quoted 108 lines
Anyway, I'm not able to test fastboot gadget since our (I mean Samsung's) boards aren't supporting fastboot - we use UMS, DFU, LTHOR.
Hence, I can only rely on somebody's testing and review the code without the ability to know about implementation "quirks".
Quoted 79 lines
Quoted 79 lines
drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex a54b4ee..887cf4f 100644drivers/usb/gadget/f_fastboot.c +4
-19@@ -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;Quoted 16 lines
Quoted 4 lines
drivers/usb/gadget/f_fastboot.c +0
-0strsep (&cmd,":" ); download_size =simple_strtoul (cmd,NULL ,16 );drivers/usb/gadget/f_fastboot.c static void cb_download(struct usb_ep *ep, · line 504
Quoted 10 lines
drivers/usb/gadget/f_fastboot.c +1
-3@@ -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,Quoted 7 lines
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot+ Angela Stegmaier
Quoted 8 lines
Steve,
Actually this code was originally added by Angela Stegmaier (see [1]). Before reverting it, we should ask her opinion on that.
I'm gonna verify this change on DRA7 EVM board today and see if "fastboot flash" works fine (and it certainly was, before this revert).
Sam:
Thanks!Could you please add the following printf lines, and let me know the results?
drivers/usb/gadget/f_fastboot.c +2
-0@@ -427,12 +427,14 @@ static unsigned int rx_bytes_expected(unsigned int maxpacket)return 0 ;if (rx_remain >EP_BUFFER_SIZE )return EP_BUFFER_SIZE ; +printf ("\nFIXME: rx_remain (before) = %d\n" , rx_remain);if (rx_remain < maxpacket) { rx_remain = maxpacket; }else if (rx_remain % maxpacket !=0 ) { rem = rx_remain % maxpacket; rx_remain = rx_remain + (maxpacket - rem); } +printf ("\nFIXME: rx_remain (after) = %d\n" , rx_remain);return rx_remain; }
Quoted 4 lines
mine show:
FIXME:rx_remain(before) = 4072
FIXME:rx_remain(after) = 4096
and then it just hangs forever....
First of all, with this revert my fastboot hangs forever (on "flash" command). So we probably shouldn't merge it as is (although it may fix fastboot on your platform, it also breaks fastboot on TI platforms).
Next, I collected debug output that you asked me for.
1. Without your patch (my fastboot works):
1.1 fastboot flash xloader MLO
Starting download of 69316 bytes
FIXME: rx_remain (before) = 3780
FIXME: rx_remain (after) = 4096
downloading of 69316 bytes finished
Flashing Raw Image
........ wrote 69632 bytes to 'xloader'
1.2 fastboot flash bootloader u-boot.img
Starting download of 350280 bytes
FIXME: rx_remain (before) = 2120
FIXME: rx_remain (after) = 2560
downloading of 350280 bytes finished
Flashing Raw Image
........ wrote 350720 bytes to 'bootloader'
2. With your patch (my fastboot hangs on "flash" command):
2.1 fastboot flash xloader MLO
Starting download of 69316 bytes
FIXME: rx_remain (after) = 3780
2.2 fastboot flash bootloader u-boot
Starting download of 350072 bytes
FIXME: rx_remain (after) = 1912
Probably you use some different host-side fastboot tool than me.
Quoted 4 lines
PATCH 1/1 Revert "fastboot: OUT transaction length must be aligned to wMaxPacketSize"
Quoted 239 lines · Sam Protsenko
Quoted 200 lines · Steve Rae
Lukasz & Sam:
Quoted 23 lines · Sam Protsenko
Quoted 20 lines · Lukasz Majewski
Hi Marek,
Quoted 17 lines · Steve Rae
Quoted 8 lines
This reverts commit
9e4b510d40310bf46e09f4edd0a0b6356213df47.Signed-off-by Steve Rae <[email protected]>---
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.
Quoted 3 lines
Does anyone know how to contact:
Dileep Katta <[email protected]>
(this email bounces) ?No clue, sorry.
Quoted 8 lines
Quoted 8 lines
Probably he is no longer employed by Linaro.
Quoted 2 lines
Applied for now as it fixes real bug.
I've a gut feeling that there are different versions of "fastboot" protocol.
Yes - I suspect that also, which is why I test with two (2) fastboot clients:
(1) from 2013 (and I don't know its origin), and
(2) from the Adroid SDK (r24.4.1) for Linux - from developer.android.com; which requires
the "Android SDK Platform-tools (rev 23.1)" to be installed...
[ this is the current version - checked yesterday! ]Quoted 1 line
Quoted 1 line
What Steve is describing is that HOST is waiting for ZLP ACK.
Lukasz:
Is there any debugging I can enable to verify this?Quoted 118 lines
Quoted 108 lines
Anyway, I'm not able to test fastboot gadget since our (I mean Samsung's) boards aren't supporting fastboot - we use UMS, DFU, LTHOR.
Hence, I can only rely on somebody's testing and review the code without the ability to know about implementation "quirks".
Quoted 79 lines
Quoted 79 lines
drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex a54b4ee..887cf4f 100644drivers/usb/gadget/f_fastboot.c +4
-19@@ -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;Quoted 16 lines
Quoted 4 lines
drivers/usb/gadget/f_fastboot.c +0
-0strsep (&cmd,":" ); download_size =simple_strtoul (cmd,NULL ,16 );drivers/usb/gadget/f_fastboot.c static void cb_download(struct usb_ep *ep, · line 504
Quoted 10 lines
drivers/usb/gadget/f_fastboot.c +1
-3@@ -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,Quoted 7 lines
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot+ Angela Stegmaier
Quoted 8 lines
Steve,
Actually this code was originally added by Angela Stegmaier (see [1]). Before reverting it, we should ask her opinion on that.
I'm gonna verify this change on DRA7 EVM board today and see if "fastboot flash" works fine (and it certainly was, before this revert).
Sam:
Thanks!Could you please add the following printf lines, and let me know the results?
drivers/usb/gadget/f_fastboot.c +2
-0@@ -427,12 +427,14 @@ static unsigned int rx_bytes_expected(unsigned int maxpacket)return 0 ;if (rx_remain >EP_BUFFER_SIZE )return EP_BUFFER_SIZE ; +printf ("\nFIXME: rx_remain (before) = %d\n" , rx_remain);if (rx_remain < maxpacket) { rx_remain = maxpacket; }else if (rx_remain % maxpacket !=0 ) { rem = rx_remain % maxpacket; rx_remain = rx_remain + (maxpacket - rem); } +printf ("\nFIXME: rx_remain (after) = %d\n" , rx_remain);return rx_remain; }Quoted 4 lines
mine show:
FIXME:rx_remain(before) = 4072
FIXME:rx_remain(after) = 4096
and then it just hangs forever....First of all, with this revert my fastboot hangs forever (on "flash" command). So we probably shouldn't merge it as is (although it may fix fastboot on your platform, it also breaks fastboot on TI platforms).
Next, I collected debug output that you asked me for.
1. Without your patch (my fastboot works):
1.1 fastboot flash xloader MLO
Starting download of 69316 bytes
FIXME:rx_remain(before) = 3780
FIXME:rx_remain(after) = 4096
downloading of 69316 bytes finished
Flashing Raw Image
........ wrote 69632 bytes to 'xloader'1.2 fastboot flash bootloader u-boot.img
Starting download of 350280 bytes
FIXME:rx_remain(before) = 2120
FIXME:rx_remain(after) = 2560
downloading of 350280 bytes finished
Flashing Raw Image
........ wrote 350720 bytes to 'bootloader'2. With your patch (my fastboot hangs on "flash" command):
2.1 fastboot flash xloader MLO
Starting download of 69316 bytes
FIXME:rx_remain(after) = 3780
Quoted 7 lines
2.2 fastboot flash bootloader u-boot
Starting download of 350072 bytes
FIXME:rx_remain(after) = 1912Probably you use some different host-side fastboot tool than me.
Appreciate these tests -- now I'm totally confused.... ( as documented above, I'm using: )
from the Adroid SDK (r24.4.1) for Linux - from developer.android.com; which requires
the "Android SDK Platform-tools (rev 23.1)" to be installed... [ this is the current version - checked yesterday! ]
what is your fastboot?
Quoted 5 lines
Quoted 5 lines
PATCH 1/1 Revert "fastboot: OUT transaction length must be aligned to wMaxPacketSize"
Quoted 249 lines · Steve Rae
Quoted 239 lines · Sam Protsenko
Quoted 200 lines · Steve Rae
Lukasz & Sam:
Quoted 23 lines · Sam Protsenko
Quoted 20 lines · Lukasz Majewski
Hi Marek,
Quoted 17 lines · Steve Rae
Quoted 8 lines
This reverts commit
9e4b510d40310bf46e09f4edd0a0b6356213df47.Signed-off-by Steve Rae <[email protected]>---
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.
Quoted 3 lines
Does anyone know how to contact:
Dileep Katta <[email protected]>
(this email bounces) ?No clue, sorry.
Quoted 8 lines
Quoted 8 lines
Probably he is no longer employed by Linaro.
Quoted 2 lines
Applied for now as it fixes real bug.
I've a gut feeling that there are different versions of "fastboot" protocol.
Yes - I suspect that also, which is why I test with two (2) fastboot clients:
(1) from 2013 (and I don't know its origin), and
(2) from the Adroid SDK (r24.4.1) for Linux - from developer.android.com; which requires
the "Android SDK Platform-tools (rev 23.1)" to be installed...
[ this is the current version - checked yesterday! ]Quoted 1 line
Quoted 1 line
What Steve is describing is that HOST is waiting for ZLP ACK.
Lukasz:
Is there any debugging I can enable to verify this?Quoted 118 lines
Quoted 108 lines
Anyway, I'm not able to test fastboot gadget since our (I mean Samsung's) boards aren't supporting fastboot - we use UMS, DFU, LTHOR.
Hence, I can only rely on somebody's testing and review the code without the ability to know about implementation "quirks".
Quoted 79 lines
Quoted 79 lines
drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex a54b4ee..887cf4f 100644drivers/usb/gadget/f_fastboot.c +4
-19@@ -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;Quoted 16 lines
Quoted 4 lines
drivers/usb/gadget/f_fastboot.c +0
-0strsep (&cmd,":" ); download_size =simple_strtoul (cmd,NULL ,16 );drivers/usb/gadget/f_fastboot.c static void cb_download(struct usb_ep *ep, · line 504
Quoted 10 lines
drivers/usb/gadget/f_fastboot.c +1
-3@@ -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,Quoted 7 lines
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot+ Angela Stegmaier
Quoted 8 lines
Steve,
Actually this code was originally added by Angela Stegmaier (see [1]). Before reverting it, we should ask her opinion on that.
I'm gonna verify this change on DRA7 EVM board today and see if "fastboot flash" works fine (and it certainly was, before this revert).
Sam:
Thanks!Could you please add the following printf lines, and let me know the results?
drivers/usb/gadget/f_fastboot.c +2
-0@@ -427,12 +427,14 @@ static unsigned int rx_bytes_expected(unsigned int maxpacket)return 0 ;if (rx_remain >EP_BUFFER_SIZE )return EP_BUFFER_SIZE ; +printf ("\nFIXME: rx_remain (before) = %d\n" , rx_remain);if (rx_remain < maxpacket) { rx_remain = maxpacket; }else if (rx_remain % maxpacket !=0 ) { rem = rx_remain % maxpacket; rx_remain = rx_remain + (maxpacket - rem); } +printf ("\nFIXME: rx_remain (after) = %d\n" , rx_remain);return rx_remain; }Quoted 4 lines
mine show:
FIXME:rx_remain(before) = 4072
FIXME:rx_remain(after) = 4096
and then it just hangs forever....First of all, with this revert my fastboot hangs forever (on "flash" command). So we probably shouldn't merge it as is (although it may fix fastboot on your platform, it also breaks fastboot on TI platforms).
Next, I collected debug output that you asked me for.
1. Without your patch (my fastboot works):
1.1 fastboot flash xloader MLO
Starting download of 69316 bytes
FIXME:rx_remain(before) = 3780
FIXME:rx_remain(after) = 4096
downloading of 69316 bytes finished
Flashing Raw Image
........ wrote 69632 bytes to 'xloader'1.2 fastboot flash bootloader u-boot.img
Starting download of 350280 bytes
FIXME:rx_remain(before) = 2120
FIXME:rx_remain(after) = 2560
downloading of 350280 bytes finished
Flashing Raw Image
........ wrote 350720 bytes to 'bootloader'2. With your patch (my fastboot hangs on "flash" command):
2.1 fastboot flash xloader MLO
Starting download of 69316 bytes
FIXME:rx_remain(after) = 3780Quoted 7 lines
2.2 fastboot flash bootloader u-boot
Starting download of 350072 bytes
FIXME:rx_remain(after) = 1912Probably you use some different host-side fastboot tool than me.
Quoted 9 lines
Appreciate these tests -- now I'm totally confused.... ( as documented above, I'm using: )
from the Adroid SDK (r24.4.1) for Linux - from developer.android.com; which requires
the "Android SDK Platform-tools (rev 23.1)" to be installed... [ this is the current version - checked yesterday! ]what is your fastboot?
I'm using fastboot tool from Android Lollipop build (using [1] instructions). And looking into git log for my system/core/fastboot/fastboot.c, it seems like it doesn't have any vendor-specific commits on top of it, which means it's mainline Google fastboot.
So basically you can build mainline Android Lollipop AFS and get the same fastboot tool as I have.
[1] http://omappedia.org/wiki/6AL.1.2_Release_Notes
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
PATCH 1/1 Revert "fastboot: OUT transaction length must be aligned to wMaxPacketSize"
In reply to Steve RaeQuoted 11 lines · Steve Rae
This reverts commit
9e4b510d40310bf46e09f4edd0a0b6356213df47.Signed-off-by Steve Rae <[email protected]>---
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).Does anyone know how to contact:
Dileep Katta <[email protected]>
(this email bounces) ?
Sam, can you comment / ack this? Dileep isn't with Linaro anymore and I assume isn't overly interested in this area anymore. Thanks!
Quoted 95 lines
drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-)drivers/usb/gadget/f_fastboot.c +5
-22@@ -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; }Signature · 1 line
1.8.5
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160324/4d8214e1/attachment.sig>
PATCH 1/1 Revert "fastboot: OUT transaction length must be aligned to wMaxPacketSize"
Quoted 10 lines · Tom Rini
Quoted 8 lines · Steve Rae
This reverts commit
9e4b510d40310bf46e09f4edd0a0b6356213df47.Signed-off-by Steve Rae <[email protected]>---
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).
Quoted 7 lines
Quoted 3 lines
Does anyone know how to contact:
Dileep Katta <[email protected]>
(this email bounces) ?Sam, can you comment / ack this? Dileep isn't with Linaro anymore and I assume isn't overly interested in this area anymore. Thanks!
Yes, Dileep is not with Linaro anymore. But I don't really know how to contact him, if it's what you are asking. As for this revert -- I already mentioned in this thread that it breaks TI platforms, causing them to hang forever on "fastboot flash" command. So I guess we need to root cause the problem itself instead of reverting useful code.
Steve,
Can you please try fastboot tool from "android-tools-fastboot" package (available in Debian and Ubuntu)? It works for me as well as fastboot tool from Android Lollipop build, but this way you don't have to build the whole AFS just to test fastboot tool.
Thanks.
Quoted 98 lines
Quoted 93 lines
drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-)drivers/usb/gadget/f_fastboot.c +5
-23@@ -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; } --Quoted 2 lines
1.8.5
--
Tom
PATCH 1/1 Revert "fastboot: OUT transaction length must be aligned to wMaxPacketSize"
That one fails for me too....
So I suspect that a conditional is required - something like:
(quoted diff) +0 -0
b/drivers/usb/gadget/f_fastboot.c
index a54b4ee..41ec211 100644
drivers/usb/gadget/f_fastboot.c +0 -0
@@ -422,17 +422,19 @@ static void cb_getvar(struct usb_ep *ep, struct
usb_request *req)
static unsigned int rx_bytes_expected(unsigned int maxpacket)
{
int rx_remain = download_size - download_bytes;
- int rem = 0;
+ int __maybe_unused rem = 0;
if (rx_remain < 0)
return 0;
if (rx_remain > EP_BUFFER_SIZE)
return EP_BUFFER_SIZE;
+#if XXX_alignment_required
if (rx_remain < maxpacket) {
rx_remain = maxpacket;
} else if (rx_remain % maxpacket != 0) {
rem = rx_remain % maxpacket;
rx_remain = rx_remain + (maxpacket - rem);
}
+#endif
return rx_remain;}
If you submit a PATCH like this, then I will abort the request to "revert" this PATCH....
Thanks, Steve
Quoted 144 lines · Sam Protsenko
Quoted 7 lines · Tom Rini
Quoted 5 lines · Steve Rae
This reverts commit
9e4b510d40310bf46e09f4edd0a0b6356213df47.Signed-off-by Steve Rae <[email protected]>---
As discussed on the mailing list, this change breaks the downloadportion
Quoted 1 line
Quoted 1 line
of fastboot by causing the server (the device running U-Boot) to wait
forever
Quoted 1 line
Quoted 1 line
for bytes which are never sent by the client (the host connected via
USB).
Quoted 8 lines
Quoted 4 lines
Does anyone know how to contact:
Dileep Katta <[email protected]>
(this email bounces) ?Sam, can you comment / ack this? Dileep isn't with Linaro anymore and I assume isn't overly interested in this area anymore. Thanks!
Yes, Dileep is not with Linaro anymore. But I don't really know how to contact him, if it's what you are asking. As for this revert -- I already mentioned in this thread that it breaks TI platforms, causing them to hang forever on "fastboot flash" command. So I guess we need to root cause the problem itself instead of reverting useful code.
Steve,
Can you please try fastboot tool from "android-tools-fastboot" package (available in Debian and Ubuntu)? It works for me as well as fastboot tool from Android Lollipop build, but this way you don't have to build the whole AFS just to test fastboot tool.
Thanks.
Quoted 5 lines
Review
Quoted 5 lines
drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cQuoted 9 lines
Quoted 9 lines
index a54b4ee..887cf4f 100644
drivers/usb/gadget/f_fastboot.c +0
-1@@ -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;Quoted 3 lines
drivers/usb/gadget/f_fastboot.c static int fastboot_set_alt(struct usb_function
Quoted 3 lines
drivers/usb/gadget/f_fastboot.c +0
-0static 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,
Quoted 16 lines
drivers/usb/gadget/f_fastboot.c static void cb_getvar(struct usb_ep *ep, struct
Quoted 16 lines
drivers/usb/gadget/f_fastboot.c +2
-5__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)Quoted 21 lines
Quoted 21 lines
drivers/usb/gadget/f_fastboot.c +1
-8fastboot_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; }Quoted 1 line
drivers/usb/gadget/f_fastboot.c static void rx_handler_dl_image(struct usb_ep *ep,
Quoted 1 line
drivers/usb/gadget/f_fastboot.c +0
-0@@ -444,7 +433,6 @@ static void rx_handler_dl_image(struct usb_ep *ep,struct
usb_request*req)Quoted 8 lines
drivers/usb/gadget/f_fastboot.c static void rx_handler_dl_image(struct usb_ep *ep,
Quoted 8 lines
drivers/usb/gadget/f_fastboot.c +0
-1const 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)Quoted 2 lines
drivers/usb/gadget/f_fastboot.c static void rx_handler_dl_image(struct usb_ep *ep, · line 470
Quoted 2 lines
drivers/usb/gadget/f_fastboot.c +0
-0printf("\ndownloading of %d bytes finished\n",
download_bytes);Quoted 9 lines
drivers/usb/gadget/f_fastboot.c static void cb_download(struct usb_ep *ep, struct
Quoted 9 lines
drivers/usb/gadget/f_fastboot.c +1
-3}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)Quoted 8 lines
drivers/usb/gadget/f_fastboot.c static void cb_download(struct usb_ep *ep, struct
Quoted 8 lines
drivers/usb/gadget/f_fastboot.c +0
-1{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)Quoted 16 lines
Quoted 11 lines
drivers/usb/gadget/f_fastboot.c +1
-4}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; } --Quoted 2 lines
1.8.5
--
Tom
PATCH 1/1 Revert "fastboot: OUT transaction length must be aligned to wMaxPacketSize"
Quoted 31 lines · Steve Rae
That one fails for me too....
So I suspect that a conditional is required - something like:
drivers/usb/gadget/f_fastboot.c +3
-1@@ -422,17 +422,19 @@ static void cb_getvar(struct usb_ep *ep, struct usb_request *req)static unsigned int rx_bytes_expected (unsigned int maxpacket) {int rx_remain = download_size - download_bytes; -int rem =0 ; +int __maybe_unused rem =0 ;if (rx_remain <0 )return 0 ;if (rx_remain >EP_BUFFER_SIZE )return EP_BUFFER_SIZE ; +#if XXX_alignment_requiredif (rx_remain < maxpacket) { rx_remain = maxpacket; }else if (rx_remain % maxpacket !=0 ) { rem = rx_remain % maxpacket; rx_remain = rx_remain + (maxpacket - rem); } +#endif return rx_remain; }
Quoted 3 lines
If you submit a PATCH like this, then I will abort the request to "revert" this PATCH....
Thanks, Steve
Well, I'm pretty busy doing another work right now, and obviously revert shouldn't be merged anyway (as it breaks another platforms). But you can go ahead and send v2 of this patch, adding corresponding options (perhaps to Kconfig) and testing them via #ifdef, and I will verify if it works fine for TI platforms.
Also, it would be good to know why that stuck is happening for you in the first place, so we can come up with real solution to this problem instead of fixing it with hack.
Quoted 151 lines
Quoted 144 lines · Sam Protsenko
Quoted 13 lines · Tom Rini
Quoted 11 lines · Steve Rae
This reverts commit
9e4b510d40310bf46e09f4edd0a0b6356213df47.Signed-off-by Steve Rae <[email protected]>---
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).Quoted 7 lines
Quoted 3 lines
Does anyone know how to contact:
Dileep Katta <[email protected]>
(this email bounces) ?Sam, can you comment / ack this? Dileep isn't with Linaro anymore and I assume isn't overly interested in this area anymore. Thanks!
Yes, Dileep is not with Linaro anymore. But I don't really know how to contact him, if it's what you are asking. As for this revert -- I already mentioned in this thread that it breaks TI platforms, causing them to hang forever on "fastboot flash" command. So I guess we need to root cause the problem itself instead of reverting useful code.
Steve,
Can you please try fastboot tool from "android-tools-fastboot" package (available in Debian and Ubuntu)? It works for me as well as fastboot tool from Android Lollipop build, but this way you don't have to build the whole AFS just to test fastboot tool.
Thanks.
Quoted 106 lines
Quoted 100 lines
drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-)drivers/usb/gadget/f_fastboot.c +2
-6@@ -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;drivers/usb/gadget/f_fastboot.c +1
-9@@ -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);drivers/usb/gadget/f_fastboot.c +1
-3@@ -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; }drivers/usb/gadget/f_fastboot.c +0
-1@@ -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);drivers/usb/gadget/f_fastboot.c +1
-3@@ -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;}Quoted 3 lines
--
1.8.5--
Tom
PATCH 1/1 Revert "fastboot: OUT transaction length must be aligned to wMaxPacketSize"
Quoted 32 lines · Sam Protsenko
Quoted 31 lines · Steve Rae
That one fails for me too....
So I suspect that a conditional is required - something like:
drivers/usb/gadget/f_fastboot.c +3
-1@@ -422,17 +422,19 @@ static void cb_getvar(struct usb_ep *ep, struct usb_request *req)static unsigned int rx_bytes_expected (unsigned int maxpacket) {int rx_remain = download_size - download_bytes; -int rem =0 ; +int __maybe_unused rem =0 ;if (rx_remain <0 )return 0 ;if (rx_remain >EP_BUFFER_SIZE )return EP_BUFFER_SIZE ; +#if XXX_alignment_requiredif (rx_remain < maxpacket) { rx_remain = maxpacket; }else if (rx_remain % maxpacket !=0 ) { rem = rx_remain % maxpacket; rx_remain = rx_remain + (maxpacket - rem); } +#endif return rx_remain; }
Quoted 6 lines
Quoted 3 lines
If you submit a PATCH like this, then I will abort the request to "revert" this PATCH....
Thanks, SteveWell, I'm pretty busy doing another work right now, and obviously revert shouldn't be merged anyway (as it breaks another platforms).
The revert is not applied.
Quoted 7 lines
But you can go ahead and send v2 of this patch, adding corresponding options (perhaps to
Kconfig) and testing them via #ifdef, and I will verify if it works fine for TI platforms.Also, it would be good to know why that stuck is happening for you in the first place, so we can come up with real solution to this problem instead of fixing it with hack.
Right
Best regards,
Marek Vasut