usb: gadget: dummy_hcd: check hcd at urb enqueue

Message ID 1436282700-11028-1-git-send-email-rui.silva@linaro.org
State New
Headers show

Commit Message

Rui Miguel Silva July 7, 2015, 3:25 p.m.
When processing urb list in dummy_timer and an ep goes away, the urb
is giveback with an -EPROTO error. However the same urb can be
enqueued once again and the loop in dummy_timer never get out.

To fix that, make sure the dummy_hcd is enabled at enqueue time to
avoid adding urb to the urbp list.

Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
---
 drivers/usb/gadget/udc/dummy_hcd.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Rui Miguel Silva July 8, 2015, 11:03 a.m. | #1
Hi Alan,
On Tue, Jul 07, 2015 at 02:07:48PM -0400, Alan Stern wrote:
>On Tue, 7 Jul 2015, Rui Miguel Silva wrote:
>
>> When processing urb list in dummy_timer and an ep goes away, the urb
>> is giveback with an -EPROTO error. However the same urb can be
>> enqueued once again and the loop in dummy_timer never get out.
>
>I don't understand.  Why would dummy_timer behave differently for the
>second URB submission?  It should follow the same logic and return
>-EPROTO just like the first time.
>
>> To fix that, make sure the dummy_hcd is enabled at enqueue time to
>> avoid adding urb to the urbp list.
>> To fix that, make sure the dummy_hcd is enabled at enqueue time to
>> avoid adding urb to the urbp list.
>
>This is not the right approach.  We want to allow URBs to be enqueued
>even if the device's upstream port is not enabled.  After all, real
>HCDs allow this.

Yes, you are right. Just drop this patch. Sorry for the noise.

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

Patch

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index f5400b8..30f1563 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -926,8 +926,6 @@  static int dummy_udc_stop(struct usb_gadget *g)
 	return 0;
 }
 
-#undef is_enabled
-
 /* The gadget structure is stored inside the hcd structure and will be
  * released along with it. */
 static void init_dummy_udc_hw(struct dummy *dum)
@@ -1157,6 +1155,10 @@  static int dummy_urb_enqueue(
 	urbp->miter_started = 0;
 
 	dum_hcd = hcd_to_dummy_hcd(hcd);
+	if (!is_enabled(dum_hcd)) {
+		kfree(urbp);
+		return -ESHUTDOWN;
+	}
 	spin_lock_irqsave(&dum_hcd->dum->lock, flags);
 
 	rc = dummy_validate_stream(dum_hcd, urb);
@@ -1191,6 +1193,8 @@  static int dummy_urb_enqueue(
 	return rc;
 }
 
+#undef is_enabled
+
 static int dummy_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 {
 	struct dummy_hcd *dum_hcd;