On Tue, 11 Mar 2014, Poulain, Loic wrote: > To test the patch, I tried it with the btusb driver, and forced manually the resume issue > with one interface (intf 0) every five resume call. With this patch, the interface is correctly > unbind/rebind. Then, the upper layer (here android) is able to retrieve the device. > > However, btusb driver itself doesn't seem compatible with the rebind mechanism. > Indeed, When a real resume fails, all the interfaces of a same device are marked for rebind. > The btusb driver works with two interfaces, it claims the "intf 1" in the "intf 0" probe > and releases the two interfaces at the same time in the disconnect callback. > However the rebind mechanism unbind/bind the device's interfaces sequentially. > (cf do_rebind_interfaces), for the btusb case, it means: > 1. unbind intf 0 > 2. bind intf 0 > 3. unbind intf 1 > 4. bind inf 1 > > At point 2. btusb probes intf 0, and tries to claim the intf 1 which is not yet unbind, so > the intf 0 probe fails (interface 1 busy). At point 4. the busb probes the intf 1, since it's > not the "main interface", it fails and returns -ENODEV, waiting for the intf0 probe. > > I don't know if it is a btusb driver implementation issue or a usb core problem. > Two fixes are possible. > - Fix the btusb driver, make it compatible with any probe sequence, "intf 0" first > or "intf 1" first. (However, may several drivers have the same behavior) > - Fix the rebind mechanism, unbind all the interfaces first then probe them all. > > Regarding, the rebind patch itself, it's ok for me with the forced software failure test. > But should be completed with an other patch or new patchset for the btusb driver (at least). You're right about this problem; it is a weakness in the USB core. The patch below should fix it, but I haven't done any testing. Please try it out and see if it helps with your problem. Alan Stern Index: usb-3.14/drivers/usb/core/driver.c =================================================================== --- usb-3.14.orig/drivers/usb/core/driver.c +++ usb-3.14/drivers/usb/core/driver.c @@ -980,8 +980,7 @@ EXPORT_SYMBOL_GPL(usb_deregister); * it doesn't support pre_reset/post_reset/reset_resume or * because it doesn't support suspend/resume. * - * The caller must hold @intf's device's lock, but not its pm_mutex - * and not @intf->dev.sem. + * The caller must hold @intf's device's lock, but not @intf's lock. */ void usb_forced_unbind_intf(struct usb_interface *intf) { @@ -994,16 +993,37 @@ void usb_forced_unbind_intf(struct usb_i intf->needs_binding = 1; } +/* + * Unbind drivers for @udev's marked interfaces. These interfaces have + * the needs_binding flag set, for example by usb_resume_interface(). + * + * The caller must hold @udev's device lock. + */ +static void unbind_marked_interfaces(struct usb_device *udev) +{ + struct usb_host_config *config; + int i; + struct usb_interface *intf; + + config = udev->actconfig; + if (config) { + for (i = 0; i < config->desc.bNumInterfaces; ++i) { + intf = config->interface[i]; + if (intf->dev.driver && intf->needs_binding) + usb_forced_unbind_intf(intf); + } + } +} + /* Delayed forced unbinding of a USB interface driver and scan * for rebinding. * - * The caller must hold @intf's device's lock, but not its pm_mutex - * and not @intf->dev.sem. + * The caller must hold @intf's device's lock, but not @intf's lock. * * Note: Rebinds will be skipped if a system sleep transition is in * progress and the PM "complete" callback hasn't occurred yet. */ -void usb_rebind_intf(struct usb_interface *intf) +static void usb_rebind_intf(struct usb_interface *intf) { int rc; @@ -1020,68 +1040,66 @@ void usb_rebind_intf(struct usb_interfac } } -#ifdef CONFIG_PM - -/* Unbind drivers for @udev's interfaces that don't support suspend/resume - * There is no check for reset_resume here because it can be determined - * only during resume whether reset_resume is needed. +/* + * Rebind drivers to @udev's marked interfaces. These interfaces have + * the needs_binding flag set. * * The caller must hold @udev's device lock. */ -static void unbind_no_pm_drivers_interfaces(struct usb_device *udev) +static void rebind_marked_interfaces(struct usb_device *udev) { struct usb_host_config *config; int i; struct usb_interface *intf; - struct usb_driver *drv; config = udev->actconfig; if (config) { for (i = 0; i < config->desc.bNumInterfaces; ++i) { intf = config->interface[i]; - - if (intf->dev.driver) { - drv = to_usb_driver(intf->dev.driver); - if (!drv->suspend || !drv->resume) - usb_forced_unbind_intf(intf); - } + if (intf->needs_binding) + usb_rebind_intf(intf); } } } -/* Unbind drivers for @udev's interfaces that failed to support reset-resume. - * These interfaces have the needs_binding flag set by usb_resume_interface(). +/* + * Unbind all of @udev's marked interfaces and then rebind all of them. + * This ordering is necessary because some drivers claim several interfaces + * when they are first probed. * * The caller must hold @udev's device lock. */ -static void unbind_no_reset_resume_drivers_interfaces(struct usb_device *udev) +void usb_unbind_and_rebind_marked_interfaces(struct usb_device *udev) { - struct usb_host_config *config; - int i; - struct usb_interface *intf; - - config = udev->actconfig; - if (config) { - for (i = 0; i < config->desc.bNumInterfaces; ++i) { - intf = config->interface[i]; - if (intf->dev.driver && intf->needs_binding) - usb_forced_unbind_intf(intf); - } - } + unbind_marked_interfaces(udev); + rebind_marked_interfaces(udev); } -static void do_rebind_interfaces(struct usb_device *udev) +#ifdef CONFIG_PM + +/* Unbind drivers for @udev's interfaces that don't support suspend/resume + * There is no check for reset_resume here because it can be determined + * only during resume whether reset_resume is needed. + * + * The caller must hold @udev's device lock. + */ +static void unbind_no_pm_drivers_interfaces(struct usb_device *udev) { struct usb_host_config *config; int i; struct usb_interface *intf; + struct usb_driver *drv; config = udev->actconfig; if (config) { for (i = 0; i < config->desc.bNumInterfaces; ++i) { intf = config->interface[i]; - if (intf->needs_binding) - usb_rebind_intf(intf); + + if (intf->dev.driver) { + drv = to_usb_driver(intf->dev.driver); + if (!drv->suspend || !drv->resume) + usb_forced_unbind_intf(intf); + } } } } @@ -1410,7 +1428,7 @@ int usb_resume_complete(struct device *d * whose needs_binding flag is set */ if (udev->state != USB_STATE_NOTATTACHED) - do_rebind_interfaces(udev); + rebind_marked_interfaces(udev); return 0; } @@ -1432,7 +1450,7 @@ int usb_resume(struct device *dev, pm_me pm_runtime_disable(dev); pm_runtime_set_active(dev); pm_runtime_enable(dev); - unbind_no_reset_resume_drivers_interfaces(udev); + unbind_marked_interfaces(udev); } /* Avoid PM error messages for devices disconnected while suspended Index: usb-3.14/drivers/usb/core/hub.c =================================================================== --- usb-3.14.orig/drivers/usb/core/hub.c +++ usb-3.14/drivers/usb/core/hub.c @@ -5345,10 +5345,11 @@ int usb_reset_device(struct usb_device * else if (cintf->condition == USB_INTERFACE_BOUND) rebind = 1; + if (rebind) + cintf->needs_binding = 1; } - if (ret == 0 && rebind) - usb_rebind_intf(cintf); } + usb_unbind_and_rebind_marked_interfaces(udev); } usb_autosuspend_device(udev); Index: usb-3.14/drivers/usb/core/usb.h =================================================================== --- usb-3.14.orig/drivers/usb/core/usb.h +++ usb-3.14/drivers/usb/core/usb.h @@ -56,7 +56,7 @@ extern int usb_match_one_id_intf(struct extern int usb_match_device(struct usb_device *dev, const struct usb_device_id *id); extern void usb_forced_unbind_intf(struct usb_interface *intf); -extern void usb_rebind_intf(struct usb_interface *intf); +extern void usb_unbind_and_rebind_marked_interfaces(struct usb_device *udev); extern int usb_hub_claim_port(struct usb_device *hdev, unsigned port, struct dev_state *owner); -- 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