[PATCH] usb: gadget: fastboot: fix downoading files of wMaxPacketSize bytes

[PATCH] usb: gadget: fastboot: fix downoading files of wMaxPacketSize bytes

[Sascha Hauer]

File transfers with sizes of exact multiples of wMaxPacketSize up to
EP_BUFFER_SIZE do not work. For a typical scenario that would be files
of 512, 1024 ... 3584 bytes.

This happens because we unconditionally put EP_BUFFER_SIZE into the
initial request length. For non wMaxPacketSize aligned legths this
works well because the transfer is completed with a short packet.
For wMaxPacketSize aligned lengths there is no short packet though,
so the transfer never completes. Instead we have to put the file
size into the initial request length.

Some controllers like the DWC3 do not work when the request length is
not aligned to wMaxPacketSize, so we align up to wMaxPacketSize like
done in U-Boot.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/gadget/f_fastboot.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index 40a78987e4..036365f67d 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -609,6 +609,16 @@ static void cb_getvar(struct f_fastboot *f_fb, const char *cmd)
 	fastboot_tx_print(f_fb, "OKAY");
 }
 
+static int rx_bytes_expected(struct f_fastboot *f_fb)
+{
+	int remaining = f_fb->download_size - f_fb->download_bytes;
+
+	if (remaining >= EP_BUFFER_SIZE)
+		return EP_BUFFER_SIZE;
+
+	return ALIGN(remaining, f_fb->out_ep->maxpacket);
+}
+
 static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req)
 {
 	struct f_fastboot *f_fb = req->context;
@@ -632,9 +642,7 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req)
 
 	f_fb->download_bytes += req->actual;
 
-	req->length = f_fb->download_size - f_fb->download_bytes;
-	if (req->length > EP_BUFFER_SIZE)
-		req->length = EP_BUFFER_SIZE;
+	req->length = rx_bytes_expected(f_fb);
 
 	show_progress(f_fb->download_bytes);
 
@@ -687,9 +695,7 @@ static void cb_download(struct f_fastboot *f_fb, const char *cmd)
 		struct usb_ep *ep = f_fb->out_ep;
 		fastboot_tx_print(f_fb, "DATA%08x", f_fb->download_size);
 		req->complete = rx_handler_dl_image;
-		req->length = EP_BUFFER_SIZE;
-		if (req->length < ep->maxpacket)
-			req->length = ep->maxpacket;
+		req->length = rx_bytes_expected(f_fb);
 	}
 }
 
-- 
2.19.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] usb: gadget: fastboot: fix downoading files of wMaxPacketSize bytes

[Schenk, Gavin]

Hi,

> File transfers with sizes of exact multiples of wMaxPacketSize up to
> EP_BUFFER_SIZE do not work. For a typical scenario that would be files
> of 512, 1024 ... 3584 bytes.
> 
> This happens because we unconditionally put EP_BUFFER_SIZE into the
> initial request length. For non wMaxPacketSize aligned legths this
s/legths/length

> works well because the transfer is completed with a short packet.
> For wMaxPacketSize aligned lengths there is no short packet though,
> so the transfer never completes. Instead we have to put the file
> size into the initial request length.
> 
> Some controllers like the DWC3 do not work when the request length is
> not aligned to wMaxPacketSize, so we align up to wMaxPacketSize like
> done in U-Boot.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reported-by: Gavin Schenk <g.schenk@eckelmann.de>

Thank you for this patch!

Regards
Gavin Schenk


-- 
Eckelmann AG
Vorstand: Dipl.-Ing. Peter Frankenbach (Sprecher) Dipl.-Wi.-Ing. Philipp Eckelmann
Dr.-Ing. Marco Münchhof Dr.-Ing. Frank Uhlemann
Vorsitzender des Aufsichtsrats: Hubertus G. Krossa
Stv. Vorsitzender des Aufsichtsrats: Dr.-Ing. Gerd Eckelmann
Sitz der Gesellschaft: Berliner Str. 161, 65205 Wiesbaden, Amtsgericht Wiesbaden HRB 12636
http://www.eckelmann.de
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] usb: gadget: fastboot: fix downoading files of wMaxPacketSize bytes

[Uwe Kleine-König]

On Mon, Oct 01, 2018 at 08:29:18AM +0000, Schenk, Gavin wrote:
> Hi,
> 
> > File transfers with sizes of exact multiples of wMaxPacketSize up to
> > EP_BUFFER_SIZE do not work. For a typical scenario that would be files
> > of 512, 1024 ... 3584 bytes.
> > 
> > This happens because we unconditionally put EP_BUFFER_SIZE into the
> > initial request length. For non wMaxPacketSize aligned legths this
> s/legths/length

I'd keep the s at the end though.

Oh, and while we're typo squatting: $Subject ~= s/downoading/downloading

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] usb: gadget: fastboot: fix downoading files of wMaxPacketSize bytes

[Uwe Kleine-König]

Hello Sascha,

On Mon, Oct 01, 2018 at 10:17:16AM +0200, Sascha Hauer wrote:
> File transfers with sizes of exact multiples of wMaxPacketSize up to
> EP_BUFFER_SIZE do not work. For a typical scenario that would be files
> of 512, 1024 ... 3584 bytes.
> 
> This happens because we unconditionally put EP_BUFFER_SIZE into the
> initial request length. For non wMaxPacketSize aligned legths this
> works well because the transfer is completed with a short packet.
> For wMaxPacketSize aligned lengths there is no short packet though,
> so the transfer never completes. Instead we have to put the file
> size into the initial request length.
> 
> Some controllers like the DWC3 do not work when the request length is
> not aligned to wMaxPacketSize, so we align up to wMaxPacketSize like
> done in U-Boot.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

I tried transfering files of sizes between 1 and 4100 bytes and with
this patch this actually works fine.

Tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox