| Message ID | 1458694241-15455-1-git-send-email-srae@broadcom.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
On 03/23/2016 01:50 AM, Steve Rae wrote: > This reverts commit 9e4b510d40310bf46e09f4edd0a0b6356213df47. > > Signed-off-by: Steve Rae <srae@broadcom.com> > --- > As discussed on the mailing list, this change breaks the download portion > of fastboot by causing the server (the device running U-Boot) to wait forever > for bytes which are never sent by the client (the host connected via USB). It'd be real cool if you could reference the ML discussion next time. > Does anyone know how to contact: > Dileep Katta <dileep.katta@linaro.org> > (this email bounces) ? No clue, sorry. Applied for now as it fixes real bug. > drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- > 1 file changed, 5 insertions(+), 22 deletions(-) > > diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c > index a54b4ee..887cf4f 100644 > --- a/drivers/usb/gadget/f_fastboot.c > +++ b/drivers/usb/gadget/f_fastboot.c > @@ -57,7 +57,6 @@ static struct f_fastboot *fastboot_func; > static unsigned int fastboot_flash_session_id; > static unsigned int download_size; > static unsigned int download_bytes; > -static bool is_high_speed; > > static struct usb_endpoint_descriptor fs_ep_in = { > .bLength = USB_DT_ENDPOINT_SIZE, > @@ -241,13 +240,10 @@ static int fastboot_set_alt(struct usb_function *f, > __func__, f->name, interface, alt); > > /* make sure we don't enable the ep twice */ > - if (gadget->speed == USB_SPEED_HIGH) { > + if (gadget->speed == USB_SPEED_HIGH) > ret = usb_ep_enable(f_fb->out_ep, &hs_ep_out); > - is_high_speed = true; > - } else { > + else > ret = usb_ep_enable(f_fb->out_ep, &fs_ep_out); > - is_high_speed = false; > - } > if (ret) { > puts("failed to enable out ep\n"); > return ret; > @@ -419,20 +415,13 @@ static void cb_getvar(struct usb_ep *ep, struct usb_request *req) > fastboot_tx_write_str(response); > } > > -static unsigned int rx_bytes_expected(unsigned int maxpacket) > +static unsigned int rx_bytes_expected(void) > { > int rx_remain = download_size - download_bytes; > - int rem = 0; > if (rx_remain < 0) > return 0; > if (rx_remain > EP_BUFFER_SIZE) > return EP_BUFFER_SIZE; > - if (rx_remain < maxpacket) { > - rx_remain = maxpacket; > - } else if (rx_remain % maxpacket != 0) { > - rem = rx_remain % maxpacket; > - rx_remain = rx_remain + (maxpacket - rem); > - } > return rx_remain; > } > > @@ -444,7 +433,6 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req) > const unsigned char *buffer = req->buf; > unsigned int buffer_size = req->actual; > unsigned int pre_dot_num, now_dot_num; > - unsigned int max; > > if (req->status != 0) { > printf("Bad status: %d\n", req->status); > @@ -482,9 +470,7 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req) > > printf("\ndownloading of %d bytes finished\n", download_bytes); > } else { > - max = is_high_speed ? hs_ep_out.wMaxPacketSize : > - fs_ep_out.wMaxPacketSize; > - req->length = rx_bytes_expected(max); > + req->length = rx_bytes_expected(); > if (req->length < ep->maxpacket) > req->length = ep->maxpacket; > } > @@ -497,7 +483,6 @@ static void cb_download(struct usb_ep *ep, struct usb_request *req) > { > char *cmd = req->buf; > char response[FASTBOOT_RESPONSE_LEN]; > - unsigned int max; > > strsep(&cmd, ":"); > download_size = simple_strtoul(cmd, NULL, 16); > @@ -513,9 +498,7 @@ static void cb_download(struct usb_ep *ep, struct usb_request *req) > } else { > sprintf(response, "DATA%08x", download_size); > req->complete = rx_handler_dl_image; > - max = is_high_speed ? hs_ep_out.wMaxPacketSize : > - fs_ep_out.wMaxPacketSize; > - req->length = rx_bytes_expected(max); > + req->length = rx_bytes_expected(); > if (req->length < ep->maxpacket) > req->length = ep->maxpacket; > } >
Hi Marek, > On 03/23/2016 01:50 AM, Steve Rae wrote: > > This reverts commit 9e4b510d40310bf46e09f4edd0a0b6356213df47. > > > > Signed-off-by: Steve Rae <srae@broadcom.com> > > --- > > As discussed on the mailing list, this change breaks the download > > portion of fastboot by causing the server (the device running > > U-Boot) to wait forever for bytes which are never sent by the > > client (the host connected via USB). > > It'd be real cool if you could reference the ML discussion next time. > > > Does anyone know how to contact: > > Dileep Katta <dileep.katta@linaro.org> > > (this email bounces) ? > > No clue, sorry. Probably he is no longer employed by Linaro. > > 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". > > > drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- > > 1 file changed, 5 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/usb/gadget/f_fastboot.c > > b/drivers/usb/gadget/f_fastboot.c index a54b4ee..887cf4f 100644 > > --- a/drivers/usb/gadget/f_fastboot.c > > +++ b/drivers/usb/gadget/f_fastboot.c > > @@ -57,7 +57,6 @@ static struct f_fastboot *fastboot_func; > > static unsigned int fastboot_flash_session_id; > > static unsigned int download_size; > > static unsigned int download_bytes; > > -static bool is_high_speed; > > > > static struct usb_endpoint_descriptor fs_ep_in = { > > .bLength = USB_DT_ENDPOINT_SIZE, > > @@ -241,13 +240,10 @@ static int fastboot_set_alt(struct > > usb_function *f, __func__, f->name, interface, alt); > > > > /* make sure we don't enable the ep twice */ > > - if (gadget->speed == USB_SPEED_HIGH) { > > + if (gadget->speed == USB_SPEED_HIGH) > > ret = usb_ep_enable(f_fb->out_ep, &hs_ep_out); > > - is_high_speed = true; > > - } else { > > + else > > ret = usb_ep_enable(f_fb->out_ep, &fs_ep_out); > > - is_high_speed = false; > > - } > > if (ret) { > > puts("failed to enable out ep\n"); > > return ret; > > @@ -419,20 +415,13 @@ static void cb_getvar(struct usb_ep *ep, > > struct usb_request *req) fastboot_tx_write_str(response); > > } > > > > -static unsigned int rx_bytes_expected(unsigned int maxpacket) > > +static unsigned int rx_bytes_expected(void) > > { > > int rx_remain = download_size - download_bytes; > > - int rem = 0; > > if (rx_remain < 0) > > return 0; > > if (rx_remain > EP_BUFFER_SIZE) > > return EP_BUFFER_SIZE; > > - if (rx_remain < maxpacket) { > > - rx_remain = maxpacket; > > - } else if (rx_remain % maxpacket != 0) { > > - rem = rx_remain % maxpacket; > > - rx_remain = rx_remain + (maxpacket - rem); > > - } > > return rx_remain; > > } > > > > @@ -444,7 +433,6 @@ static void rx_handler_dl_image(struct usb_ep > > *ep, struct usb_request *req) const unsigned char *buffer = > > req->buf; unsigned int buffer_size = req->actual; > > unsigned int pre_dot_num, now_dot_num; > > - unsigned int max; > > > > if (req->status != 0) { > > printf("Bad status: %d\n", req->status); > > @@ -482,9 +470,7 @@ static void rx_handler_dl_image(struct usb_ep > > *ep, struct usb_request *req) > > printf("\ndownloading of %d bytes finished\n", > > download_bytes); } else { > > - max = is_high_speed ? hs_ep_out.wMaxPacketSize : > > - fs_ep_out.wMaxPacketSize; > > - req->length = rx_bytes_expected(max); > > + req->length = rx_bytes_expected(); > > if (req->length < ep->maxpacket) > > req->length = ep->maxpacket; > > } > > @@ -497,7 +483,6 @@ static void cb_download(struct usb_ep *ep, > > struct usb_request *req) { > > char *cmd = req->buf; > > char response[FASTBOOT_RESPONSE_LEN]; > > - unsigned int max; > > > > strsep(&cmd, ":"); > > download_size = simple_strtoul(cmd, NULL, 16); > > @@ -513,9 +498,7 @@ static void cb_download(struct usb_ep *ep, > > struct usb_request *req) } else { > > sprintf(response, "DATA%08x", download_size); > > req->complete = rx_handler_dl_image; > > - max = is_high_speed ? hs_ep_out.wMaxPacketSize : > > - fs_ep_out.wMaxPacketSize; > > - req->length = rx_bytes_expected(max); > > + req->length = rx_bytes_expected(); > > if (req->length < ep->maxpacket) > > req->length = ep->maxpacket; > > } > > > >
On Wed, Mar 23, 2016 at 11:49 AM, Lukasz Majewski <l.majewski@samsung.com> wrote: > Hi Marek, > >> On 03/23/2016 01:50 AM, Steve Rae wrote: >> > This reverts commit 9e4b510d40310bf46e09f4edd0a0b6356213df47. >> > >> > Signed-off-by: Steve Rae <srae@broadcom.com> >> > --- >> > As discussed on the mailing list, this change breaks the download >> > portion of fastboot by causing the server (the device running >> > U-Boot) to wait forever for bytes which are never sent by the >> > client (the host connected via USB). >> >> It'd be real cool if you could reference the ML discussion next time. >> >> > Does anyone know how to contact: >> > Dileep Katta <dileep.katta@linaro.org> >> > (this email bounces) ? >> >> No clue, sorry. > > Probably he is no longer employed by Linaro. > >> >> 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". > >> >> > drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- >> > 1 file changed, 5 insertions(+), 22 deletions(-) >> > >> > diff --git a/drivers/usb/gadget/f_fastboot.c >> > b/drivers/usb/gadget/f_fastboot.c index a54b4ee..887cf4f 100644 >> > --- a/drivers/usb/gadget/f_fastboot.c >> > +++ b/drivers/usb/gadget/f_fastboot.c >> > @@ -57,7 +57,6 @@ static struct f_fastboot *fastboot_func; >> > static unsigned int fastboot_flash_session_id; >> > static unsigned int download_size; >> > static unsigned int download_bytes; >> > -static bool is_high_speed; >> > >> > static struct usb_endpoint_descriptor fs_ep_in = { >> > .bLength = USB_DT_ENDPOINT_SIZE, >> > @@ -241,13 +240,10 @@ static int fastboot_set_alt(struct >> > usb_function *f, __func__, f->name, interface, alt); >> > >> > /* make sure we don't enable the ep twice */ >> > - if (gadget->speed == USB_SPEED_HIGH) { >> > + if (gadget->speed == USB_SPEED_HIGH) >> > ret = usb_ep_enable(f_fb->out_ep, &hs_ep_out); >> > - is_high_speed = true; >> > - } else { >> > + else >> > ret = usb_ep_enable(f_fb->out_ep, &fs_ep_out); >> > - is_high_speed = false; >> > - } >> > if (ret) { >> > puts("failed to enable out ep\n"); >> > return ret; >> > @@ -419,20 +415,13 @@ static void cb_getvar(struct usb_ep *ep, >> > struct usb_request *req) fastboot_tx_write_str(response); >> > } >> > >> > -static unsigned int rx_bytes_expected(unsigned int maxpacket) >> > +static unsigned int rx_bytes_expected(void) >> > { >> > int rx_remain = download_size - download_bytes; >> > - int rem = 0; >> > if (rx_remain < 0) >> > return 0; >> > if (rx_remain > EP_BUFFER_SIZE) >> > return EP_BUFFER_SIZE; >> > - if (rx_remain < maxpacket) { >> > - rx_remain = maxpacket; >> > - } else if (rx_remain % maxpacket != 0) { >> > - rem = rx_remain % maxpacket; >> > - rx_remain = rx_remain + (maxpacket - rem); >> > - } >> > return rx_remain; >> > } >> > >> > @@ -444,7 +433,6 @@ static void rx_handler_dl_image(struct usb_ep >> > *ep, struct usb_request *req) const unsigned char *buffer = >> > req->buf; unsigned int buffer_size = req->actual; >> > unsigned int pre_dot_num, now_dot_num; >> > - unsigned int max; >> > >> > if (req->status != 0) { >> > printf("Bad status: %d\n", req->status); >> > @@ -482,9 +470,7 @@ static void rx_handler_dl_image(struct usb_ep >> > *ep, struct usb_request *req) >> > printf("\ndownloading of %d bytes finished\n", >> > download_bytes); } else { >> > - max = is_high_speed ? hs_ep_out.wMaxPacketSize : >> > - fs_ep_out.wMaxPacketSize; >> > - req->length = rx_bytes_expected(max); >> > + req->length = rx_bytes_expected(); >> > if (req->length < ep->maxpacket) >> > req->length = ep->maxpacket; >> > } >> > @@ -497,7 +483,6 @@ static void cb_download(struct usb_ep *ep, >> > struct usb_request *req) { >> > char *cmd = req->buf; >> > char response[FASTBOOT_RESPONSE_LEN]; >> > - unsigned int max; >> > >> > strsep(&cmd, ":"); >> > download_size = simple_strtoul(cmd, NULL, 16); >> > @@ -513,9 +498,7 @@ static void cb_download(struct usb_ep *ep, >> > struct usb_request *req) } else { >> > sprintf(response, "DATA%08x", download_size); >> > req->complete = rx_handler_dl_image; >> > - max = is_high_speed ? hs_ep_out.wMaxPacketSize : >> > - fs_ep_out.wMaxPacketSize; >> > - req->length = rx_bytes_expected(max); >> > + req->length = rx_bytes_expected(); >> > if (req->length < ep->maxpacket) >> > req->length = ep->maxpacket; >> > } >> > >> >> > > > > -- > Best regards, > > Lukasz Majewski > > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group > _______________________________________________ > U-Boot mailing list > U-Boot@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
On Tue, Mar 22, 2016 at 05:50:41PM -0700, Steve Rae wrote: > This reverts commit 9e4b510d40310bf46e09f4edd0a0b6356213df47. > > Signed-off-by: Steve Rae <srae@broadcom.com> > --- > As discussed on the mailing list, this change breaks the download portion > of fastboot by causing the server (the device running U-Boot) to wait forever > for bytes which are never sent by the client (the host connected via USB). > > Does anyone know how to contact: > Dileep Katta <dileep.katta@linaro.org> > (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! > > drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- > 1 file changed, 5 insertions(+), 22 deletions(-) > > diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c > index a54b4ee..887cf4f 100644 > --- a/drivers/usb/gadget/f_fastboot.c > +++ b/drivers/usb/gadget/f_fastboot.c > @@ -57,7 +57,6 @@ static struct f_fastboot *fastboot_func; > static unsigned int fastboot_flash_session_id; > static unsigned int download_size; > static unsigned int download_bytes; > -static bool is_high_speed; > > static struct usb_endpoint_descriptor fs_ep_in = { > .bLength = USB_DT_ENDPOINT_SIZE, > @@ -241,13 +240,10 @@ static int fastboot_set_alt(struct usb_function *f, > __func__, f->name, interface, alt); > > /* make sure we don't enable the ep twice */ > - if (gadget->speed == USB_SPEED_HIGH) { > + if (gadget->speed == USB_SPEED_HIGH) > ret = usb_ep_enable(f_fb->out_ep, &hs_ep_out); > - is_high_speed = true; > - } else { > + else > ret = usb_ep_enable(f_fb->out_ep, &fs_ep_out); > - is_high_speed = false; > - } > if (ret) { > puts("failed to enable out ep\n"); > return ret; > @@ -419,20 +415,13 @@ static void cb_getvar(struct usb_ep *ep, struct usb_request *req) > fastboot_tx_write_str(response); > } > > -static unsigned int rx_bytes_expected(unsigned int maxpacket) > +static unsigned int rx_bytes_expected(void) > { > int rx_remain = download_size - download_bytes; > - int rem = 0; > if (rx_remain < 0) > return 0; > if (rx_remain > EP_BUFFER_SIZE) > return EP_BUFFER_SIZE; > - if (rx_remain < maxpacket) { > - rx_remain = maxpacket; > - } else if (rx_remain % maxpacket != 0) { > - rem = rx_remain % maxpacket; > - rx_remain = rx_remain + (maxpacket - rem); > - } > return rx_remain; > } > > @@ -444,7 +433,6 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req) > const unsigned char *buffer = req->buf; > unsigned int buffer_size = req->actual; > unsigned int pre_dot_num, now_dot_num; > - unsigned int max; > > if (req->status != 0) { > printf("Bad status: %d\n", req->status); > @@ -482,9 +470,7 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req) > > printf("\ndownloading of %d bytes finished\n", download_bytes); > } else { > - max = is_high_speed ? hs_ep_out.wMaxPacketSize : > - fs_ep_out.wMaxPacketSize; > - req->length = rx_bytes_expected(max); > + req->length = rx_bytes_expected(); > if (req->length < ep->maxpacket) > req->length = ep->maxpacket; > } > @@ -497,7 +483,6 @@ static void cb_download(struct usb_ep *ep, struct usb_request *req) > { > char *cmd = req->buf; > char response[FASTBOOT_RESPONSE_LEN]; > - unsigned int max; > > strsep(&cmd, ":"); > download_size = simple_strtoul(cmd, NULL, 16); > @@ -513,9 +498,7 @@ static void cb_download(struct usb_ep *ep, struct usb_request *req) > } else { > sprintf(response, "DATA%08x", download_size); > req->complete = rx_handler_dl_image; > - max = is_high_speed ? hs_ep_out.wMaxPacketSize : > - fs_ep_out.wMaxPacketSize; > - req->length = rx_bytes_expected(max); > + req->length = rx_bytes_expected(); > if (req->length < ep->maxpacket) > req->length = ep->maxpacket; > } > -- > 1.8.5 >
On Thu, Mar 24, 2016 at 7:15 PM, Tom Rini <trini@konsulko.com> wrote: > On Tue, Mar 22, 2016 at 05:50:41PM -0700, Steve Rae wrote: > >> This reverts commit 9e4b510d40310bf46e09f4edd0a0b6356213df47. >> >> Signed-off-by: Steve Rae <srae@broadcom.com> >> --- >> As discussed on the mailing list, this change breaks the download portion >> of fastboot by causing the server (the device running U-Boot) to wait forever >> for bytes which are never sent by the client (the host connected via USB). >> >> Does anyone know how to contact: >> Dileep Katta <dileep.katta@linaro.org> >> (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. >> >> drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- >> 1 file changed, 5 insertions(+), 22 deletions(-) >> >> diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c >> index a54b4ee..887cf4f 100644 >> --- a/drivers/usb/gadget/f_fastboot.c >> +++ b/drivers/usb/gadget/f_fastboot.c >> @@ -57,7 +57,6 @@ static struct f_fastboot *fastboot_func; >> static unsigned int fastboot_flash_session_id; >> static unsigned int download_size; >> static unsigned int download_bytes; >> -static bool is_high_speed; >> >> static struct usb_endpoint_descriptor fs_ep_in = { >> .bLength = USB_DT_ENDPOINT_SIZE, >> @@ -241,13 +240,10 @@ static int fastboot_set_alt(struct usb_function *f, >> __func__, f->name, interface, alt); >> >> /* make sure we don't enable the ep twice */ >> - if (gadget->speed == USB_SPEED_HIGH) { >> + if (gadget->speed == USB_SPEED_HIGH) >> ret = usb_ep_enable(f_fb->out_ep, &hs_ep_out); >> - is_high_speed = true; >> - } else { >> + else >> ret = usb_ep_enable(f_fb->out_ep, &fs_ep_out); >> - is_high_speed = false; >> - } >> if (ret) { >> puts("failed to enable out ep\n"); >> return ret; >> @@ -419,20 +415,13 @@ static void cb_getvar(struct usb_ep *ep, struct usb_request *req) >> fastboot_tx_write_str(response); >> } >> >> -static unsigned int rx_bytes_expected(unsigned int maxpacket) >> +static unsigned int rx_bytes_expected(void) >> { >> int rx_remain = download_size - download_bytes; >> - int rem = 0; >> if (rx_remain < 0) >> return 0; >> if (rx_remain > EP_BUFFER_SIZE) >> return EP_BUFFER_SIZE; >> - if (rx_remain < maxpacket) { >> - rx_remain = maxpacket; >> - } else if (rx_remain % maxpacket != 0) { >> - rem = rx_remain % maxpacket; >> - rx_remain = rx_remain + (maxpacket - rem); >> - } >> return rx_remain; >> } >> >> @@ -444,7 +433,6 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req) >> const unsigned char *buffer = req->buf; >> unsigned int buffer_size = req->actual; >> unsigned int pre_dot_num, now_dot_num; >> - unsigned int max; >> >> if (req->status != 0) { >> printf("Bad status: %d\n", req->status); >> @@ -482,9 +470,7 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req) >> >> printf("\ndownloading of %d bytes finished\n", download_bytes); >> } else { >> - max = is_high_speed ? hs_ep_out.wMaxPacketSize : >> - fs_ep_out.wMaxPacketSize; >> - req->length = rx_bytes_expected(max); >> + req->length = rx_bytes_expected(); >> if (req->length < ep->maxpacket) >> req->length = ep->maxpacket; >> } >> @@ -497,7 +483,6 @@ static void cb_download(struct usb_ep *ep, struct usb_request *req) >> { >> char *cmd = req->buf; >> char response[FASTBOOT_RESPONSE_LEN]; >> - unsigned int max; >> >> strsep(&cmd, ":"); >> download_size = simple_strtoul(cmd, NULL, 16); >> @@ -513,9 +498,7 @@ static void cb_download(struct usb_ep *ep, struct usb_request *req) >> } else { >> sprintf(response, "DATA%08x", download_size); >> req->complete = rx_handler_dl_image; >> - max = is_high_speed ? hs_ep_out.wMaxPacketSize : >> - fs_ep_out.wMaxPacketSize; >> - req->length = rx_bytes_expected(max); >> + req->length = rx_bytes_expected(); >> if (req->length < ep->maxpacket) >> req->length = ep->maxpacket; >> } >> -- >> 1.8.5 >> > > -- > Tom
diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c index a54b4ee..887cf4f 100644 --- a/drivers/usb/gadget/f_fastboot.c +++ b/drivers/usb/gadget/f_fastboot.c @@ -57,7 +57,6 @@ static struct f_fastboot *fastboot_func; static unsigned int fastboot_flash_session_id; static unsigned int download_size; static unsigned int download_bytes; -static bool is_high_speed; static struct usb_endpoint_descriptor fs_ep_in = { .bLength = USB_DT_ENDPOINT_SIZE, @@ -241,13 +240,10 @@ static int fastboot_set_alt(struct usb_function *f, __func__, f->name, interface, alt); /* make sure we don't enable the ep twice */ - if (gadget->speed == USB_SPEED_HIGH) { + if (gadget->speed == USB_SPEED_HIGH) ret = usb_ep_enable(f_fb->out_ep, &hs_ep_out); - is_high_speed = true; - } else { + else ret = usb_ep_enable(f_fb->out_ep, &fs_ep_out); - is_high_speed = false; - } if (ret) { puts("failed to enable out ep\n"); return ret; @@ -419,20 +415,13 @@ static void cb_getvar(struct usb_ep *ep, struct usb_request *req) fastboot_tx_write_str(response); } -static unsigned int rx_bytes_expected(unsigned int maxpacket) +static unsigned int rx_bytes_expected(void) { int rx_remain = download_size - download_bytes; - int rem = 0; if (rx_remain < 0) return 0; if (rx_remain > EP_BUFFER_SIZE) return EP_BUFFER_SIZE; - if (rx_remain < maxpacket) { - rx_remain = maxpacket; - } else if (rx_remain % maxpacket != 0) { - rem = rx_remain % maxpacket; - rx_remain = rx_remain + (maxpacket - rem); - } return rx_remain; } @@ -444,7 +433,6 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req) const unsigned char *buffer = req->buf; unsigned int buffer_size = req->actual; unsigned int pre_dot_num, now_dot_num; - unsigned int max; if (req->status != 0) { printf("Bad status: %d\n", req->status); @@ -482,9 +470,7 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req) printf("\ndownloading of %d bytes finished\n", download_bytes); } else { - max = is_high_speed ? hs_ep_out.wMaxPacketSize : - fs_ep_out.wMaxPacketSize; - req->length = rx_bytes_expected(max); + req->length = rx_bytes_expected(); if (req->length < ep->maxpacket) req->length = ep->maxpacket; } @@ -497,7 +483,6 @@ static void cb_download(struct usb_ep *ep, struct usb_request *req) { char *cmd = req->buf; char response[FASTBOOT_RESPONSE_LEN]; - unsigned int max; strsep(&cmd, ":"); download_size = simple_strtoul(cmd, NULL, 16); @@ -513,9 +498,7 @@ static void cb_download(struct usb_ep *ep, struct usb_request *req) } else { sprintf(response, "DATA%08x", download_size); req->complete = rx_handler_dl_image; - max = is_high_speed ? hs_ep_out.wMaxPacketSize : - fs_ep_out.wMaxPacketSize; - req->length = rx_bytes_expected(max); + req->length = rx_bytes_expected(); if (req->length < ep->maxpacket) req->length = ep->maxpacket; }
This reverts commit 9e4b510d40310bf46e09f4edd0a0b6356213df47. Signed-off-by: Steve Rae <srae@broadcom.com> --- As discussed on the mailing list, this change breaks the download portion of fastboot by causing the server (the device running U-Boot) to wait forever for bytes which are never sent by the client (the host connected via USB). Does anyone know how to contact: Dileep Katta <dileep.katta@linaro.org> (this email bounces) ? drivers/usb/gadget/f_fastboot.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-)