* Alan Stern | 2012-10-29 12:04:10 [-0400]:
>Was that really the problem? My memory is not what it was three weeks
>ago...
>
>If so, there's a simpler solution. The list_for_each_entry_safe()
>loop in dummy_timer() should not consider URBs that were added after
>the loop started. That ought to be easy enough to implement. (One
>solution: Increment a counter each time dummy_timer() runs, and store a
>copy of the counter value in each urbp during submission. Exit the
>loop when the stored counter value in urbp is equal to the current
>counter.)
>
>This will prevent you from getting into an infinite loop in atomic
>context. Of course, the resubmission problem would still exist, but
>that's true with non-emulated gadgets too.
>
>Alan Stern
Okay. So now I added an interval check. It looks better. However if you
prefer the "don't check URBs that were added during completion" way then
I could try that. This would start that I remove the "restart:" label
which is used after URB completion.
---
dummy_urb_enqueue() now assigns the endpoint to the qh structure if it
finds one. Plus in case of an INT URB we track the interval so we don't
schedule the packet too often.
If UDC disables the endpoint (on the device side) the endpoint information
is removed from the qh as well. I think real HW would timeout on
transfer (and return -EPROTO).
With this change the, the busy loop which occurs if the host always
re-queues an INT URB in the completion is gone because we "skip" this qh
until the interval.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
---
drivers/usb/gadget/dummy_hcd.c | 145 ++++++++++++++++++++++++++++------------
1 file changed, 102 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
index 2c27566..89a4f7f 100644
--- a/drivers/usb/gadget/dummy_hcd.c
+++ b/drivers/usb/gadget/dummy_hcd.c
@@ -94,6 +94,14 @@ struct dummy_request {
struct usb_request req;
};
+struct dummy_qh {
+ struct list_head urbp_list;
+ struct list_head qh_list;
+ struct dummy_ep *ep;
+ unsigned long next_io;
+ unsigned delay_j;
+};
+
static inline struct dummy_ep *usb_ep_to_dummy_ep(struct usb_ep *_ep)
{
return container_of(_ep, struct dummy_ep, ep);
@@ -557,7 +565,9 @@ static int dummy_enable(struct usb_ep *_ep,
static int dummy_disable(struct usb_ep *_ep)
{
struct dummy_ep *ep;
+ struct dummy_qh *qh;
struct dummy *dum;
+ struct dummy_hcd *dum_hcd;
unsigned long flags;
int retval;
@@ -565,8 +575,17 @@ static int dummy_disable(struct usb_ep *_ep)
if (!_ep || !ep->desc || _ep->name == ep0name)
return -EINVAL;
dum = ep_to_dummy(ep);
+ dum_hcd = gadget_to_dummy_hcd(&dum->gadget);
spin_lock_irqsave(&dum->lock, flags);
+
+ list_for_each_entry(qh, &dum_hcd->qh_list, qh_list) {
+ if (qh->ep == ep) {
+ qh->ep = NULL;
+ break;
+ }
+ }
+
ep->desc = NULL;
ep->stream_en = 0;
retval = 0;
@@ -1161,10 +1180,32 @@ static int dummy_validate_stream(struct dummy_hcd *dum_hcd, struct urb *urb)
return 0;
}
-struct dummy_qh {
- struct list_head urbp_list;
- struct list_head qh_list;
-};
+#define is_active(dum_hcd) ((dum_hcd->port_status & \
+ (USB_PORT_STAT_CONNECTION | USB_PORT_STAT_ENABLE | \
+ USB_PORT_STAT_SUSPEND)) \
+ == (USB_PORT_STAT_CONNECTION | USB_PORT_STAT_ENABLE))
+
+static struct dummy_ep *find_endpoint(struct dummy *dum, u8 address)
+{
+ int i;
+
+ if (!is_active((dum->gadget.speed == USB_SPEED_SUPER ?
+ dum->ss_hcd : dum->hs_hcd)))
+ return NULL;
+ if ((address & ~USB_DIR_IN) == 0)
+ return &dum->ep[0];
+ for (i = 1; i < DUMMY_ENDPOINTS; i++) {
+ struct dummy_ep *ep = &dum->ep[i];
+
+ if (!ep->desc)
+ continue;
+ if (ep->desc->bEndpointAddress == address)
+ return ep;
+ }
+ return NULL;
+}
+
+#undef is_active
struct dummy_qh *qh_append_urb(struct dummy_hcd *dum_hcd, struct urb *urb)
{
@@ -1174,12 +1215,13 @@ struct dummy_qh *qh_append_urb(struct dummy_hcd *dum_hcd, struct urb *urb)
if (qh)
return qh;
- qh = kmalloc(sizeof(*qh), GFP_ATOMIC);
+ qh = kzalloc(sizeof(*qh), GFP_ATOMIC);
if (!qh)
return qh;
list_add_tail(&qh->qh_list, &dum_hcd->qh_list);
INIT_LIST_HEAD(&qh->urbp_list);
urb->ep->hcpriv = qh;
+
return qh;
}
@@ -1196,13 +1238,41 @@ static void dummy_disable_ep(struct usb_hcd *hcd, struct usb_host_endpoint *ep)
goto out;
qh = ep->hcpriv;
- list_del(&qh->qh_list);
- kfree(ep->hcpriv);
- ep->hcpriv = NULL;
+ if (qh) {
+ /*
+ * the URBs which might be pending here will be nuked by the
+ * timer.
+ */
+ list_del(&qh->qh_list);
+ kfree(ep->hcpriv);
+ ep->hcpriv = NULL;
+ }
out:
spin_unlock_irqrestore(&dum_hcd->dum->lock, flags);
}
+static unsigned update_int_intv(struct dummy *dum,
+ const struct usb_endpoint_descriptor *desc)
+{
+ unsigned ms;
+
+ switch (dum->gadget.speed) {
+ case USB_SPEED_LOW:
+ case USB_SPEED_FULL:
+ ms = desc->bInterval;
+ break;
+
+ case USB_SPEED_HIGH:
+ case USB_SPEED_SUPER:
+ ms = ((1 << (desc->bInterval - 1)) * 125) / 1000;
+ break;
+ default:
+ ms = 255;
+ WARN_ON_ONCE(1);
+ }
+ return msecs_to_jiffies(ms);
+}
+
static int dummy_urb_enqueue(
struct usb_hcd *hcd,
struct urb *urb,
@@ -1212,6 +1282,7 @@ static int dummy_urb_enqueue(
struct dummy_qh *qh;
struct urbp *urbp;
unsigned long flags;
+ u8 address;
int rc;
urbp = kmalloc(sizeof *urbp, mem_flags);
@@ -1235,6 +1306,17 @@ static int dummy_urb_enqueue(
if (!qh)
goto err_qh;
+ if (!qh->ep) {
+ struct dummy *dum = dum_hcd->dum;
+
+ address = usb_pipeendpoint(urb->pipe);
+ if (usb_pipein(urb->pipe))
+ address |= USB_DIR_IN;
+ qh->ep = find_endpoint(dum, address);
+ if (qh->ep && usb_pipetype(urb->pipe) == PIPE_INTERRUPT)
+ qh->delay_j = update_int_intv(dum, qh->ep->ep.desc);
+ }
+
if (!dum_hcd->udev) {
dum_hcd->udev = urb->dev;
usb_get_dev(dum_hcd->udev);
@@ -1499,32 +1581,6 @@ static int periodic_bytes(struct dummy *dum, struct dummy_ep *ep)
return limit;
}
-#define is_active(dum_hcd) ((dum_hcd->port_status & \
- (USB_PORT_STAT_CONNECTION | USB_PORT_STAT_ENABLE | \
- USB_PORT_STAT_SUSPEND)) \
- == (USB_PORT_STAT_CONNECTION | USB_PORT_STAT_ENABLE))
-
-static struct dummy_ep *find_endpoint(struct dummy *dum, u8 address)
-{
- int i;
-
- if (!is_active((dum->gadget.speed == USB_SPEED_SUPER ?
- dum->ss_hcd : dum->hs_hcd)))
- return NULL;
- if ((address & ~USB_DIR_IN) == 0)
- return &dum->ep[0];
- for (i = 1; i < DUMMY_ENDPOINTS; i++) {
- struct dummy_ep *ep = &dum->ep[i];
-
- if (!ep->desc)
- continue;
- if (ep->desc->bEndpointAddress == address)
- return ep;
- }
- return NULL;
-}
-
-#undef is_active
#define Dev_Request (USB_TYPE_STANDARD | USB_RECIP_DEVICE)
#define Dev_InRequest (Dev_Request | USB_DIR_IN)
@@ -1709,13 +1765,12 @@ static int handle_control_request(struct dummy_hcd *dum_hcd, struct urb *urb,
return ret_val;
}
-static int handle_one_urb(struct urbp *urbp, struct dummy_hcd *dum_hcd, int *total)
+static int handle_one_urb(struct urbp *urbp, struct dummy_ep *ep,
+ struct dummy_hcd *dum_hcd, int *total)
{
struct dummy *dum = dum_hcd->dum;
struct urb *urb;
struct dummy_request *req;
- u8 address;
- struct dummy_ep *ep = NULL;
int type;
int status = -EINPROGRESS;
int limit;
@@ -1734,11 +1789,6 @@ static int handle_one_urb(struct urbp *urbp, struct dummy_hcd *dum_hcd, int *tot
if (*total <= 0 && type == PIPE_BULK)
return 0;
- /* find the gadget's ep for this request (if configured) */
- address = usb_pipeendpoint (urb->pipe);
- if (usb_pipein(urb->pipe))
- address |= USB_DIR_IN;
- ep = find_endpoint(dum, address);
if (!ep) {
/* set_configuration() disagreement */
dev_dbg(dummy_dev(dum_hcd),
@@ -1922,12 +1972,21 @@ static void dummy_timer(unsigned long _dum_hcd)
list_for_each_entry_safe(qh, qh_tmp, &dum_hcd->qh_list, qh_list) {
int ret;
struct urbp *urbp, *urbp_tmp;
+ struct dummy_ep *ep;
+ if (qh->delay_j && qh->next_io == 0)
+ qh->next_io = jiffies + qh->delay_j;
+ if (qh->delay_j) {
+ if (time_before(jiffies, qh->next_io))
+ continue;
+ }
list_for_each_entry_safe(urbp, urbp_tmp, &qh->urbp_list, urbp_list) {
- ret = handle_one_urb(urbp, dum_hcd, &total);
+ ep = qh->ep;
+ ret = handle_one_urb(urbp, ep, dum_hcd, &total);
if (!ret)
continue;
+ qh->next_io = jiffies + qh->delay_j;
goto restart;
}
}
--
1.7.10.4
--
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
