When dummy_hcd is created as a SuperSpeed controller (configurable via
a module parameter) dummy_start() is called twice, once to make the
SuperSpeed HCD and once to make the Highspeed HCD. When it is created
as a HighSpeed controller, then dummy_start() is only called once.
The way dummy_start() was written, the URB list and the timer are
initialized twice (in the superspeed case). Besides looking weird
it is also possible that this can be buggy, because the way it works
is that we initialize the timer and the list when starting the
SuperSpeed HCD, we return all the way back to the USB core (so
possibly the timer and list start to get used due to requests
by the hub) and then we initialize them again while starting
the HighSpeed HCD. It is not impossible for this to result at
least in lost/leaked URBs.
So, rework dummy_start() to only initialize dummy_hcd struct
fields once and delete dummy_start_ss() helper function
as it is no longer useful.
Signed-off-by: Pantelis Koukousoulas <pktoss@xxxxxxxxx>
---
drivers/usb/gadget/dummy_hcd.c | 50 ++++++++++++++----------------------------
1 file changed, 17 insertions(+), 33 deletions(-)
diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
index 8c06430..dbbee3e 100644
--- a/drivers/usb/gadget/dummy_hcd.c
+++ b/drivers/usb/gadget/dummy_hcd.c
@@ -2314,45 +2314,23 @@ static ssize_t urbs_show(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_RO(urbs);
-static int dummy_start_ss(struct dummy_hcd *dum_hcd)
-{
- init_timer(&dum_hcd->timer);
- dum_hcd->timer.function = dummy_timer;
- dum_hcd->timer.data = (unsigned long)dum_hcd;
- dum_hcd->rh_state = DUMMY_RH_RUNNING;
- dum_hcd->stream_en_ep = 0;
- INIT_LIST_HEAD(&dum_hcd->urbp_list);
- dummy_hcd_to_hcd(dum_hcd)->power_budget = POWER_BUDGET;
- dummy_hcd_to_hcd(dum_hcd)->state = HC_STATE_RUNNING;
- dummy_hcd_to_hcd(dum_hcd)->uses_new_polling = 1;
-#ifdef CONFIG_USB_OTG
- dummy_hcd_to_hcd(dum_hcd)->self.otg_port = 1;
-#endif
- return 0;
-
- /* FIXME 'urbs' should be a per-device thing, maybe in usbcore */
- return device_create_file(dummy_dev(dum_hcd), &dev_attr_urbs);
-}
-
static int dummy_start(struct usb_hcd *hcd)
{
struct dummy_hcd *dum_hcd = hcd_to_dummy_hcd(hcd);
/*
- * MASTER side init ... we emulate a root hub that'll only ever
- * talk to one device (the slave side). Also appears in sysfs,
- * just like more familiar pci-based HCDs.
+ * These are per-device, not per HCD, so they should only be
+ * initialized once although in the superspeed case this
+ * function will be called twice. Thus we use a conditional.
*/
- if (!usb_hcd_is_primary_hcd(hcd))
- return dummy_start_ss(dum_hcd);
-
- spin_lock_init(&dum_hcd->dum->lock);
- init_timer(&dum_hcd->timer);
- dum_hcd->timer.function = dummy_timer;
- dum_hcd->timer.data = (unsigned long)dum_hcd;
- dum_hcd->rh_state = DUMMY_RH_RUNNING;
-
- INIT_LIST_HEAD(&dum_hcd->urbp_list);
+ if (dum_hcd->rh_state != DUMMY_RH_RUNNING) {
+ spin_lock_init(&dum_hcd->dum->lock);
+ init_timer(&dum_hcd->timer);
+ dum_hcd->timer.function = dummy_timer;
+ dum_hcd->timer.data = (unsigned long)dum_hcd;
+ INIT_LIST_HEAD(&dum_hcd->urbp_list);
+ dum_hcd->rh_state = DUMMY_RH_RUNNING;
+ }
hcd->power_budget = POWER_BUDGET;
hcd->state = HC_STATE_RUNNING;
@@ -2362,6 +2340,12 @@ static int dummy_start(struct usb_hcd *hcd)
hcd->self.otg_port = 1;
#endif
+ /* In dummy_hcd the SuperSpeed HCD is the secondary one */
+ if (!usb_hcd_is_primary_hcd(hcd)) {
+ dum_hcd->stream_en_ep = 0;
+ return 0;
+ }
+
/* FIXME 'urbs' should be a per-device thing, maybe in usbcore */
return device_create_file(dummy_dev(dum_hcd), &dev_attr_urbs);
}
--
1.9.1
--
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
