Despite the needs_rebind flag, the interface rebind was never
done for the PM runtime resume. This patch fixes this issue
by triggering the rebind in usb runtime resume.
The rebind procedure needs to be called with the device lock.
However, depending the call path (remote wakeup, local resume),
the device lock may or may not already be held in usb runtime
resume. So, use a work queue to take the lock unconditionally.
Protect this work against any deadlock in the same way as
reset_device.
Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxx>
---
drivers/usb/core/driver.c | 30 ++++++++++++++++++++++++++----
drivers/usb/core/message.c | 37 +++++++++++++++++++++++++++++++++++++
include/linux/usb.h | 7 +++++++
3 files changed, 70 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index ab90a01..0f5296d 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -290,6 +290,18 @@ static void usb_cancel_queued_reset(struct usb_interface *iface)
cancel_work_sync(&iface->reset_ws);
}
+/*
+ * Cancel any pending scheduled rebind
+ *
+ * See comments in __usb_queue_rebind_device() regarding
+ * udev->rebind_running.
+ */
+static void usb_cancel_queued_rebind(struct usb_interface *iface)
+{
+ if (iface->rebind_running == 0)
+ cancel_work_sync(&iface->rebind_ws);
+}
+
/* called from driver core with dev locked */
static int usb_probe_interface(struct device *dev)
{
@@ -381,6 +393,7 @@ static int usb_probe_interface(struct device *dev)
intf->needs_remote_wakeup = 0;
intf->condition = USB_INTERFACE_UNBOUND;
usb_cancel_queued_reset(intf);
+ usb_cancel_queued_rebind(intf);
/* If the LPM disable succeeded, balance the ref counts. */
if (!lpm_disable_error)
@@ -449,6 +462,8 @@ static int usb_unbind_interface(struct device *dev)
intf->condition = USB_INTERFACE_UNBOUND;
intf->needs_remote_wakeup = 0;
+ usb_cancel_queued_rebind(intf);
+
/* Attempt to re-enable USB3 LPM, if the disable succeeded. */
if (!lpm_disable_error)
usb_unlocked_enable_lpm(udev);
@@ -1080,7 +1095,7 @@ static void unbind_no_reset_resume_drivers_interfaces(struct usb_device *udev)
}
}
-static void do_rebind_interfaces(struct usb_device *udev)
+static void do_rebind_interfaces(struct usb_device *udev, int use_workqueue)
{
struct usb_host_config *config;
int i;
@@ -1090,8 +1105,12 @@ static void do_rebind_interfaces(struct usb_device *udev)
if (config) {
for (i = 0; i < config->desc.bNumInterfaces; ++i) {
intf = config->interface[i];
- if (intf->needs_binding)
- usb_rebind_intf(intf);
+ if (intf->needs_binding) {
+ if (use_workqueue)
+ schedule_work(&intf->rebind_ws);
+ else
+ usb_rebind_intf(intf);
+ }
}
}
}
@@ -1420,7 +1439,7 @@ int usb_resume_complete(struct device *dev)
* whose needs_binding flag is set
*/
if (udev->state != USB_STATE_NOTATTACHED)
- do_rebind_interfaces(udev);
+ do_rebind_interfaces(udev, 0);
return 0;
}
@@ -1800,6 +1819,9 @@ int usb_runtime_resume(struct device *dev)
* and all its interfaces.
*/
status = usb_resume_both(udev, PMSG_AUTO_RESUME);
+
+ do_rebind_interfaces(udev, 1);
+
return status;
}
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index f829a1a..3d95867 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1661,6 +1661,42 @@ static void __usb_queue_reset_device(struct work_struct *ws)
}
}
+/*
+ * Internal function to queue an interface rebind
+ *
+ * This is initialized into the workstruct in 'struct
+ * usb_device->rebind_ws' that is launched by
+ * message.c:usb_set_configuration() when initializing each 'struct
+ * usb_interface'.
+ *
+ * It is safe to get the USB device without reference counts because
+ * the life cycle of @iface is bound to the life cycle of @udev. Then,
+ * this function will be ran only if @iface is alive (and before
+ * freeing it any scheduled instances of it will have been cancelled).
+ *
+ * We need to set a flag (usb_dev->rebind_running) because when we call
+ * the rebind, the interfaces will be unbound. The current interface
+ * cannot try to remove the queued work as it would cause a deadlock
+ * (you cannot remove your work from within your executing workqueue).
+ * This flag lets it know, so that usb_cancel_queued_rebind() doesn't
+ * try to do it.
+ */
+static void __usb_queue_rebind_intf(struct work_struct *ws)
+{
+ int rc;
+ struct usb_interface *iface =
+ container_of(ws, struct usb_interface, rebind_ws);
+ struct usb_device *udev = interface_to_usbdev(iface);
+
+ /* Same deadlock conditions as reset device apply here */
+ rc = usb_lock_device_for_reset(udev, iface);
+ if (rc >= 0) {
+ iface->rebind_running = 1;
+ usb_rebind_intf(iface);
+ iface->rebind_running = 0;
+ usb_unlock_device(udev);
+ }
+}
/*
* usb_set_configuration - Makes a particular device setting be current
@@ -1847,6 +1883,7 @@ free_interfaces:
intf->dev.groups = usb_interface_groups;
intf->dev.dma_mask = dev->dev.dma_mask;
INIT_WORK(&intf->reset_ws, __usb_queue_reset_device);
+ INIT_WORK(&intf->rebind_ws, __usb_queue_rebind_intf);
intf->minor = -1;
device_initialize(&intf->dev);
pm_runtime_no_callbacks(&intf->dev);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 7f6eb85..f6accec 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -131,6 +131,11 @@ enum usb_interface_condition {
* thread. See __usb_queue_reset_device().
* @resetting_device: USB core reset the device, so use alt setting 0 as
* current; needs bandwidth alloc after reset.
+ * @rebind_ws: Used for scheduling interface rebind with device lock.
+ * @rebind_running: set to 1 if the interface is currently running a
+ * queued rebind so that usb_cancel_queued_rebind() doesn't try to
+ * remove from the workqueue when running inside the worker
+ * thread. See __usb_queue_rebind_device().
*
* USB device drivers attach to interfaces on a physical device. Each
* interface encapsulates a single high level function, such as feeding
@@ -181,11 +186,13 @@ struct usb_interface {
unsigned needs_binding:1; /* needs delayed unbind/rebind */
unsigned reset_running:1;
unsigned resetting_device:1; /* true: bandwidth alloc after reset */
+ unsigned rebind_running:1;
struct device dev; /* interface specific device info */
struct device *usb_dev;
atomic_t pm_usage_cnt; /* usage counter for autosuspend */
struct work_struct reset_ws; /* for resets in atomic context */
+ struct work_struct rebind_ws; /* for rebind interface with device lock */
};
#define to_usb_interface(d) container_of(d, struct usb_interface, dev)
--
1.8.3.2
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
--
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
