Re: reset_resume() for btusb
On Thu, 13 Mar 2014, Alan Stern wrote:
Ah, now I understand. We are talking about mode-switching devices that
revert to their default mode following a reset, as opposed to losing
their firmware contents (although the mode switch may involve a change
to the descriptors too). This sort of thing doesn't fit cleanly into
the two classes of errors outlined above.
In this situation we have a choice. We could do a reset and then
rebind all the interfaces, or we could do a logical disconnect and then
rediscover the reverted device. I think the second approach is
cleaner, because it would generate the appropriate udev events for
userspace hooks. (For example, it would allow the usb_modeswitch
program to do its job properly.)
This reasoning suggests that when QUIRK_RESET is present, we should
always skip reset-resume and go directly to logical disconnect.
So Oliver, what do you think about this patch?
Alan Stern
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
@@ -3098,10 +3098,20 @@ static int finish_port_resume(struct usb
* operation is carried out here, after the port has been
* resumed.
*/
- if (udev-reset_resume)
+ if (udev-reset_resume) {
+ /*
+* If the device morphs or changes modes when it is reset,
+* we don't want to perform a reset-resume. We'll fail the
+* resume, which will cause a logical disconnect, and then
+* the device will be rediscovered.
+*/
retry_reset_resume:
- status = usb_reset_and_verify_device(udev);
-
+ if (udev-quirks USB_QUIRK_RESET)
+ status = -ENODEV;
+ else
+ status = usb_reset_and_verify_device(udev);
+ }
+
/* 10.5.4.5 says be sure devices in the tree are still there.
* For now let's assume the device didn't go crazy on resume,
* and device drivers will know about any resume quirks.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: reset_resume() for btusb
On Mon, 2014-03-17 at 17:12 -0400, Alan Stern wrote: On Thu, 13 Mar 2014, Alan Stern wrote: This reasoning suggests that when QUIRK_RESET is present, we should always skip reset-resume and go directly to logical disconnect. So Oliver, what do you think about this patch? Looks perfect. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: reset_resume() for btusb
On Wed, 2014-03-12 at 15:18 +, Poulain, Loic wrote: My thought was to fix the usbcore rebind issue (with pm_runtime) to let the core unbind and rebind the device's interfaces for drivers with no reset_resume callback (not only btusb). Those functions seem to be independent. Even if you have reset_resume() it can still fail and the kernel needs to deal with that. Implementing the btusb reset_resume seems risky, a patch implementing this callback has been previously reverted due to HID dual mode device regression. (cf https://lkml.org/lkml/2013/11/26/347) Alan, perhaps the core code should honor QUIRK_RESET and unbind if it is set. Then hid2hci could set the flag. It will cause reset every time, just like Poulain said, some devices may only fail at rare cases. Peter
Re: reset_resume() for btusb
On Thu, 2014-03-13 at 08:05 +, Peter Chen wrote: On Wed, 2014-03-12 at 15:18 +, Poulain, Loic wrote: Implementing the btusb reset_resume seems risky, a patch implementing this callback has been previously reverted due to HID dual mode device regression. (cf https://lkml.org/lkml/2013/11/26/347) Alan, perhaps the core code should honor QUIRK_RESET and unbind if it is set. Then hid2hci could set the flag. It will cause reset every time, just like Poulain said, some devices may only fail at rare cases. True, but that is the point. We cannot tell devices apart. If we know reset_resume() will fail, there's no point in the attempt. If we take the premise that a class driver ought to have reset_resume() then we need a way to identify the hopeless cases. In addition there is the problem that a driver isn't told why reset_resume() is called. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: reset_resume() for btusb
On Thu, 13 Mar 2014, Oliver Neukum wrote: On Wed, 2014-03-12 at 15:18 +, Poulain, Loic wrote: My thought was to fix the usbcore rebind issue (with pm_runtime) to let the core unbind and rebind the device's interfaces for drivers with no reset_resume callback (not only btusb). Those functions seem to be independent. Which functions are you referring to? Even if you have reset_resume() it can still fail and the kernel needs to deal with that. Sure. Implementing the btusb reset_resume seems risky, a patch implementing this callback has been previously reverted due to HID dual mode device regression. (cf https://lkml.org/lkml/2013/11/26/347) Alan, perhaps the core code should honor QUIRK_RESET and unbind if it is set. Then hid2hci could set the flag. You mean, if QUIRK_RESET is set then don't bother doing a reset-resume? Just logically disconnect the device? Why would that be better than unbinding and rebinding the driver? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: reset_resume() for btusb
On Thu, 13 Mar 2014, Oliver Neukum wrote: On Thu, 2014-03-13 at 08:05 +, Peter Chen wrote: On Wed, 2014-03-12 at 15:18 +, Poulain, Loic wrote: Implementing the btusb reset_resume seems risky, a patch implementing this callback has been previously reverted due to HID dual mode device regression. (cf https://lkml.org/lkml/2013/11/26/347) Alan, perhaps the core code should honor QUIRK_RESET and unbind if it is set. Then hid2hci could set the flag. It will cause reset every time, just like Poulain said, some devices may only fail at rare cases. True, but that is the point. We cannot tell devices apart. If we know reset_resume() will fail, there's no point in the attempt. Fair enough. Although we don't really lose much by making the attempt, even if it is doomed to fail. As I see it, the only compelling reason for allowing QUIRK_RESET to prevent reset-resume would be if the device morphs in a very subtle way, which we can't detect. However, I don't know of any devices like that. If we take the premise that a class driver ought to have reset_resume() then we need a way to identify the hopeless cases. Not necessarily. We can try the reset anyway, and then see if it fails. Or if the driver's reset-resume routine fails. Don't forget, it's possible for some of the drivers bound to a composite device to succeed during a reset-resume while others fail. We wouldn't want to disconnect the entire device in that case. In addition there is the problem that a driver isn't told why reset_resume() is called. There's only one reason for calling reset_resume: The device was suspended and it had to be reset during the resume procedure. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: reset_resume() for btusb
On Thu, 2014-03-13 at 10:11 -0400, Alan Stern wrote: On Thu, 13 Mar 2014, Oliver Neukum wrote: On Wed, 2014-03-12 at 15:18 +, Poulain, Loic wrote: My thought was to fix the usbcore rebind issue (with pm_runtime) to let the core unbind and rebind the device's interfaces for drivers with no reset_resume callback (not only btusb). Those functions seem to be independent. Which functions are you referring to? reset_resume() and the ability to deal with errors in the resume code paths. I should have written functionalities. Alan, perhaps the core code should honor QUIRK_RESET and unbind if it is set. Then hid2hci could set the flag. You mean, if QUIRK_RESET is set then don't bother doing a reset-resume? Just logically disconnect the device? No, rebind. This bug https://lkml.org/lkml/2013/11/26/347 seems to indicate a problem in that area. Why would that be better than unbinding and rebinding the driver? It isn't. But now that you are asking, you remind me. What about the port power off code? Does it need to look at QUIRK_RESET? Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: reset_resume() for btusb
On Thu, 13 Mar 2014, Oliver Neukum wrote: On Thu, 2014-03-13 at 10:11 -0400, Alan Stern wrote: On Thu, 13 Mar 2014, Oliver Neukum wrote: On Wed, 2014-03-12 at 15:18 +, Poulain, Loic wrote: My thought was to fix the usbcore rebind issue (with pm_runtime) to let the core unbind and rebind the device's interfaces for drivers with no reset_resume callback (not only btusb). Those functions seem to be independent. Which functions are you referring to? reset_resume() and the ability to deal with errors in the resume code paths. I should have written functionalities. Well, we are talking about two different classes of errors: Errors encountered by usb_port_resume and helpers when resuming the physical device; Errors encountered by the various interface drivers when they come back to life following a resume. Errors in the first class are handled by a reset-resume (when applicable) or by a logical disconnect (as the last resort). Errors in the second class are handled by unbinding and rebinding the driver. Alan, perhaps the core code should honor QUIRK_RESET and unbind if it is set. Then hid2hci could set the flag. You mean, if QUIRK_RESET is set then don't bother doing a reset-resume? Just logically disconnect the device? No, rebind. This bug https://lkml.org/lkml/2013/11/26/347 seems to indicate a problem in that area. Ah, now I understand. We are talking about mode-switching devices that revert to their default mode following a reset, as opposed to losing their firmware contents (although the mode switch may involve a change to the descriptors too). This sort of thing doesn't fit cleanly into the two classes of errors outlined above. In this situation we have a choice. We could do a reset and then rebind all the interfaces, or we could do a logical disconnect and then rediscover the reverted device. I think the second approach is cleaner, because it would generate the appropriate udev events for userspace hooks. (For example, it would allow the usb_modeswitch program to do its job properly.) This reasoning suggests that when QUIRK_RESET is present, we should always skip reset-resume and go directly to logical disconnect. Why would that be better than unbinding and rebinding the driver? It isn't. But now that you are asking, you remind me. What about the port power off code? Does it need to look at QUIRK_RESET? Yes, it should. Dan Williams and I haven't reached that point yet in our patch review. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: reset_resume() for btusb
My thought was to fix the usbcore rebind issue (with pm_runtime)
to let the core unbind and rebind the device's interfaces for drivers
with no reset_resume callback (not only btusb).
Implementing the btusb reset_resume seems risky,
a patch implementing this callback has been previously reverted due to
HID dual mode device regression. (cf https://lkml.org/lkml/2013/11/26/347)
Regards,
Loic Poulain
From: Oliver Neukum [oneu...@suse.de]
Sent: Wednesday, March 12, 2014 12:03 PM
To: Poulain, Loic
Cc: linux-usb@vger.kernel.org
Subject: reset_resume() for btusb
Hi,
I still think it makes little sense to support reset_resume()
in btusb, but if you really want to, you can try this patch.
HTH
Oliver
From 3776765dbd08701c30f45c1849691a16c1077cc3 Mon Sep 17 00:00:00 2001
From: Oliver Neukum oneu...@suse.de
Date: Wed, 12 Mar 2014 12:01:13 +0100
Subject: [PATCH] btusb: implement reset_resume()
This implements reset_resume() to the extent that this is possible
for btusb. It can be done if the HCI is down. In the other cases
the host would be thrown out of the network.
Signed-off-by: Oliver Neukum oneu...@suse.de
---
drivers/bluetooth/btusb.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index baeaaed..e56fa2a 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -1670,6 +1670,25 @@ done:
return err;
}
+
+static int btusb_reset_resume(struct usb_interface *intf)
+{
+ struct btusb_data *data = usb_get_intfdata(intf);
+ struct hci_dev *hdev = data-hdev;
+
+ /*
+* the interface can be recovered only if the HCI
+* is not part of a network because the synchronization
+* is lost as the device is reset
+*/
+ if (test_bit(HCI_RUNNING, hdev-flags))
+ return -EIO;
+
+ if (hdev-setup)
+ return (hdev-setup)(hdev);
+ else
+ return 0;
+}
#endif
static struct usb_driver btusb_driver = {
@@ -1679,6 +1698,7 @@ static struct usb_driver btusb_driver = {
#ifdef CONFIG_PM
.suspend= btusb_suspend,
.resume = btusb_resume,
+ .reset_resume = btusb_reset_resume,
#endif
.id_table = btusb_table,
.supports_autosuspend = 1,
--
1.8.4.5
-
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
