[PATCH] USB: g_file_storage: fix handling zero-length packet

[PATCH] USB: g_file_storage: fix handling zero-length packet

[sangsu]

According to "CH 5.5.3. Control Transfer Packet Size Constraints",
Zero-length packet should be sent when the last data payload size
is equal to the endpoint's MaxPacketSize. This patch helps
g_file_storage send zero-length packet properly.

This is tested in s3c2440/s5c7329 architectures and works well.

Signed-off-by: SangSu Park<sangsu@gmail.com>
---
driver/usb/gadget/file_storage.c | 3++-
1 files changed, 2 insertion(+), 1 deletions(-)

--- a/drivers/usb/gadget/file_storage.c	2008-09-01 11:13:03.000000000 -0400
+++ b/drivers/usb/gadget/file_storage.c	2008-09-01 11:16:41.000000000 -0400
@@ -1463,7 +1463,8 @@
	if (rc >= 0 && rc != DELAYED_STATUS) {
		rc = min(rc, w_length);
		fsg->ep0req->length = rc;
-		fsg->ep0req->zero = rc < w_length;
+		fsg->ep0req->zero = rc < w_length
+			&& (rc % gadget->ep0->maxpacket) == 0;
		fsg->ep0req_name = (ctrl->bRequestType & USB_DIR_IN ?
				"ep0-in" : "ep0-out");
		rc = ep0_queue(fsg);

Re: [PATCH] USB: g_file_storage: fix handling zero-length packet

[Alan Stern]

On Sat, 6 Sep 2008, sangsu wrote:

> According to "CH 5.5.3. Control Transfer Packet Size Constraints",
> Zero-length packet should be sent when the last data payload size
> is equal to the endpoint's MaxPacketSize.

Not so.  5.5.3 actually says this:

	The Data stage of a control transfer from an endpoint to the
	host is complete when the endpoint does one of the following:

	 - Has transferred exactly the amount of data specified during 
	the Setup stage

	 - Transfers a packet with a payload size less than
	wMaxPacketSize or transfers a zero-length packet

This clearly indicates that if the amount of data specified in the
Setup stage is a multiple of the wMaxPacketSize then there should not
be a zero-length packet, because the transfer ends as soon as the last 
wMaxPacketSize packet is sent.

> This patch helps
> g_file_storage send zero-length packet properly.
> 
> This is tested in s3c2440/s5c7329 architectures and works well.
> 
> Signed-off-by: SangSu Park<sangsu@gmail.com>
> ---
> driver/usb/gadget/file_storage.c | 3++-
> 1 files changed, 2 insertion(+), 1 deletions(-)
> 
> --- a/drivers/usb/gadget/file_storage.c	2008-09-01 11:13:03.000000000 -0400
> +++ b/drivers/usb/gadget/file_storage.c	2008-09-01 11:16:41.000000000 -0400
> @@ -1463,7 +1463,8 @@
> 	if (rc >= 0 && rc != DELAYED_STATUS) {
> 		rc = min(rc, w_length);
> 		fsg->ep0req->length = rc;
> -		fsg->ep0req->zero = rc < w_length;
> +		fsg->ep0req->zero = rc < w_length
> +			&& (rc % gadget->ep0->maxpacket) == 0;
> 		fsg->ep0req_name = (ctrl->bRequestType & USB_DIR_IN ?
> 				"ep0-in" : "ep0-out");
> 		rc = ep0_queue(fsg);

Firstly, this patch does not do what your description says it does.

Secondly, the patch is not needed.  g_file_storage doesn't have to 
check whether the data size is a multiple of MaxPacketSize because the 
device controller driver already is supposed to make that check.

Alan Stern

Re: [PATCH] USB: g_file_storage: fix handling zero-length packet

[David Brownell]

On Saturday 06 September 2008, Alan Stern wrote:
> Secondly, the patch is not needed.  g_file_storage doesn't have to 
> check whether the data size is a multiple of MaxPacketSize because the 
> device controller driver already is supposed to make that check.

Exactly.

The only fuzziness here is what to do if the host misbehaves and
sends another IN packet when all its data has been sent, instead
of the OUT/STATUS packet.  (Host error...)

Most peripheral controller hardware is well enough designed that
it can detect this and send a zero length packet rather than STALL
the transfer.  It's polite to do so, because that host bug isn't
exactly unknown.  (Some old versions of Linux did that...)