Re: [PATCH] usb: gadget: dummy_hcd: fix status in transfer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Alan,
On Tue, Jul 07, 2015 at 01:53:14PM -0400, Alan Stern wrote:
On Tue, 7 Jul 2015, Rui Miguel Silva wrote:

When the request actual state is equal to lenght, besides setting the

"state"?  Do you mean "status"?

"lenght" is a typo.

request status to done, the status variable also need to be updated.
If not, the status will be EINPROGRESS and the transfer will never be
set as completed.

The status variable points to the URB, not to the request.

Signed-off-by: Rui Miguel Silva <rui.silva@xxxxxxxxxx>
---
 drivers/usb/gadget/udc/dummy_hcd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 181112c..f5400b8 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1367,8 +1367,10 @@ top:
 		/* many requests terminate without a short packet */
 		} else {
 			if (req->req.length == req->req.actual
-					&& !req->req.zero)
+					&& !req->req.zero) {
 				req->req.status = 0;
+				*status = 0;
+			}
 			if (urb->transfer_buffer_length == urb->actual_length
 					&& !(urb->transfer_flags
 						& URB_ZERO_PACKET))

This patch is wrong.  *status should indeed remain -EINPROGRESS and the
transfer should remain incomplete.

Here's an example to illustrate what I mean.  Let's say the endpoint's
maxpacket value is 512, and suppose the host queues a 1024-byte IN URB.
In response, suppose the gadget queues two 512-byte requests, each with
req->req.zero clear.  Consider what happens when the first request is
processed.

len, req->req.length and req->req.actual are all equal to 512, and
is_short is false.  req therefore completes with req->req.status set to
0.  But the URB is still in progress!  It still has 512 bytes left to
transfer, and it won't complete until the second request finishes.
Therefore *status (which is the URB's status) should remain set to
-EINPROGRESS.

Yes, in this case the patch breaks the flow and remove the URB status
of -EINPROGRESS to soon. But what about, and taken your example above,
the gadget queues only one 512-byte request? is_short is false, and
urb->transfer_buffer_length != urb->actual_length and so the URB
status is never changed.

Cheers,
   Rui

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux