| Message ID | 20220504151647.471885-1-jtornosm@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5] USB: core: skip unconfiguration if device doesn't support it | expand |
On Wed, May 04, 2022 at 05:16:47PM +0200, Jose Ignacio Tornos Martinez wrote: > Some devices like Bluetooth Dongles with CSR chip (i.e. USB > Bluetooth V4.0 Dongle by Trust) hang when they are unbound from > 'unbind' sysfs entry and can not be bound again. > > For these devices, CSR chip hangs when usb configuration command > with index 0 (used to unconfigure) is sent during disconnection. > > To avoid this unwanted result, it is necessary not to send this > command, so a new quirk has been created. By default, quirk is > not applied for any device and needs to be enabled by user. > > For these devices, athough device is not unconfigured, it is > better to avoid device hanging to be able to operate. Even > bluetooth can be previously turned off. > On the other hand, this is not important if usb device is going to > be bound again (normal behavior), i.e. with usbip. > > Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com> > --- Acked-by: Alan Stern <stern@rowland.harvard.edu>
On Wed, May 04, 2022 at 05:16:47PM +0200, Jose Ignacio Tornos Martinez wrote: > Some devices like Bluetooth Dongles with CSR chip (i.e. USB > Bluetooth V4.0 Dongle by Trust) hang when they are unbound from > 'unbind' sysfs entry and can not be bound again. > > For these devices, CSR chip hangs when usb configuration command > with index 0 (used to unconfigure) is sent during disconnection. > > To avoid this unwanted result, it is necessary not to send this > command, so a new quirk has been created. By default, quirk is > not applied for any device and needs to be enabled by user. > > For these devices, athough device is not unconfigured, it is > better to avoid device hanging to be able to operate. Even > bluetooth can be previously turned off. > On the other hand, this is not important if usb device is going to > be bound again (normal behavior), i.e. with usbip. > > Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com> > --- > V4 -> V5: > - By default, quirk is not applied for any device and needs to be enabled > by user if necessary. > V3 -> V4: > - Reorder quirk entries to be in numerical order according to the vendor > ID and product ID. > - Add patch version information. > V2 -> V3: > - Change subject (Bluetooth: btusb: CSR chip hangs when unbound -> > USB: core: skip unconfiguration if device doesn't support it). > - Improve quirk checking. > - Allow to test quirk interactively. > V1 -> V2: > - Use quirk feature for the exception. > > Documentation/admin-guide/kernel-parameters.txt | 2 ++ > drivers/usb/core/message.c | 12 +++++++++--- > drivers/usb/core/quirks.c | 3 +++ > include/linux/usb/quirks.h | 3 +++ > 4 files changed, 17 insertions(+), 3 deletions(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index 3f1cc5e317ed..71651b888d14 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -6183,6 +6183,8 @@ > pause after every control message); > o = USB_QUIRK_HUB_SLOW_RESET (Hub needs extra > delay after resetting its port); > + p = USB_QUIRK_SKIP_UNCONFIGURE (device doesn't > + support unconfigure); > Example: quirks=0781:5580:bk,0a5c:5834:gij > > usbhid.mousepoll= > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c > index 4d59d927ae3e..9c6cd0c75f4f 100644 > --- a/drivers/usb/core/message.c > +++ b/drivers/usb/core/message.c > @@ -2108,9 +2108,15 @@ int usb_set_configuration(struct usb_device *dev, int configuration) > } > kfree(new_interfaces); > > - ret = usb_control_msg_send(dev, 0, USB_REQ_SET_CONFIGURATION, 0, > - configuration, 0, NULL, 0, > - USB_CTRL_SET_TIMEOUT, GFP_NOIO); > + if (configuration == 0 && !cp > + && (dev->quirks & USB_QUIRK_SKIP_UNCONFIGURE)) { > + dev_warn(&dev->dev, "device is not unconfigured!\n"); > + ret = 0; > + } else > + ret = usb_control_msg_send(dev, 0, USB_REQ_SET_CONFIGURATION, 0, > + configuration, 0, NULL, 0, > + USB_CTRL_SET_TIMEOUT, GFP_NOIO); > + > if (ret && cp) { > /* > * All the old state is gone, so what else can we do? > diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c > index d3c14b5ed4a1..f4cdf85a9eb6 100644 > --- a/drivers/usb/core/quirks.c > +++ b/drivers/usb/core/quirks.c > @@ -138,6 +138,9 @@ static int quirks_param_set(const char *value, const struct kernel_param *kp) > case 'o': > flags |= USB_QUIRK_HUB_SLOW_RESET; > break; > + case 'p': > + flags |= USB_QUIRK_SKIP_UNCONFIGURE; > + break; > /* Ignore unrecognized flag characters */ > } > } > diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h > index eeb7c2157c72..79cb0616f394 100644 > --- a/include/linux/usb/quirks.h > +++ b/include/linux/usb/quirks.h > @@ -72,4 +72,7 @@ > /* device has endpoints that should be ignored */ > #define USB_QUIRK_ENDPOINT_IGNORE BIT(15) > > +/* device doesn't support unconfigure. */ > +#define USB_QUIRK_SKIP_UNCONFIGURE BIT(16) > + > #endif /* __LINUX_USB_QUIRKS_H */ > -- > 2.35.1 > I'll drop this for now as there are no in-kernel users for this quirk yet. When there is a need for one, please resubmit it. thanks, greg k-h
Ok, I will try to identify the "bad" devices in some way. Thanks José Ignacio On Thu, May 12, 2022 at 1:48 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Wed, May 04, 2022 at 05:16:47PM +0200, Jose Ignacio Tornos Martinez wrote: > > Some devices like Bluetooth Dongles with CSR chip (i.e. USB > > Bluetooth V4.0 Dongle by Trust) hang when they are unbound from > > 'unbind' sysfs entry and can not be bound again. > > > > For these devices, CSR chip hangs when usb configuration command > > with index 0 (used to unconfigure) is sent during disconnection. > > > > To avoid this unwanted result, it is necessary not to send this > > command, so a new quirk has been created. By default, quirk is > > not applied for any device and needs to be enabled by user. > > > > For these devices, athough device is not unconfigured, it is > > better to avoid device hanging to be able to operate. Even > > bluetooth can be previously turned off. > > On the other hand, this is not important if usb device is going to > > be bound again (normal behavior), i.e. with usbip. > > > > Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com> > > --- > > V4 -> V5: > > - By default, quirk is not applied for any device and needs to be enabled > > by user if necessary. > > V3 -> V4: > > - Reorder quirk entries to be in numerical order according to the vendor > > ID and product ID. > > - Add patch version information. > > V2 -> V3: > > - Change subject (Bluetooth: btusb: CSR chip hangs when unbound -> > > USB: core: skip unconfiguration if device doesn't support it). > > - Improve quirk checking. > > - Allow to test quirk interactively. > > V1 -> V2: > > - Use quirk feature for the exception. > > > > Documentation/admin-guide/kernel-parameters.txt | 2 ++ > > drivers/usb/core/message.c | 12 +++++++++--- > > drivers/usb/core/quirks.c | 3 +++ > > include/linux/usb/quirks.h | 3 +++ > > 4 files changed, 17 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > > index 3f1cc5e317ed..71651b888d14 100644 > > --- a/Documentation/admin-guide/kernel-parameters.txt > > +++ b/Documentation/admin-guide/kernel-parameters.txt > > @@ -6183,6 +6183,8 @@ > > pause after every control message); > > o = USB_QUIRK_HUB_SLOW_RESET (Hub needs extra > > delay after resetting its port); > > + p = USB_QUIRK_SKIP_UNCONFIGURE (device doesn't > > + support unconfigure); > > Example: quirks=0781:5580:bk,0a5c:5834:gij > > > > usbhid.mousepoll= > > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c > > index 4d59d927ae3e..9c6cd0c75f4f 100644 > > --- a/drivers/usb/core/message.c > > +++ b/drivers/usb/core/message.c > > @@ -2108,9 +2108,15 @@ int usb_set_configuration(struct usb_device *dev, int configuration) > > } > > kfree(new_interfaces); > > > > - ret = usb_control_msg_send(dev, 0, USB_REQ_SET_CONFIGURATION, 0, > > - configuration, 0, NULL, 0, > > - USB_CTRL_SET_TIMEOUT, GFP_NOIO); > > + if (configuration == 0 && !cp > > + && (dev->quirks & USB_QUIRK_SKIP_UNCONFIGURE)) { > > + dev_warn(&dev->dev, "device is not unconfigured!\n"); > > + ret = 0; > > + } else > > + ret = usb_control_msg_send(dev, 0, USB_REQ_SET_CONFIGURATION, 0, > > + configuration, 0, NULL, 0, > > + USB_CTRL_SET_TIMEOUT, GFP_NOIO); > > + > > if (ret && cp) { > > /* > > * All the old state is gone, so what else can we do? > > diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c > > index d3c14b5ed4a1..f4cdf85a9eb6 100644 > > --- a/drivers/usb/core/quirks.c > > +++ b/drivers/usb/core/quirks.c > > @@ -138,6 +138,9 @@ static int quirks_param_set(const char *value, const struct kernel_param *kp) > > case 'o': > > flags |= USB_QUIRK_HUB_SLOW_RESET; > > break; > > + case 'p': > > + flags |= USB_QUIRK_SKIP_UNCONFIGURE; > > + break; > > /* Ignore unrecognized flag characters */ > > } > > } > > diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h > > index eeb7c2157c72..79cb0616f394 100644 > > --- a/include/linux/usb/quirks.h > > +++ b/include/linux/usb/quirks.h > > @@ -72,4 +72,7 @@ > > /* device has endpoints that should be ignored */ > > #define USB_QUIRK_ENDPOINT_IGNORE BIT(15) > > > > +/* device doesn't support unconfigure. */ > > +#define USB_QUIRK_SKIP_UNCONFIGURE BIT(16) > > + > > #endif /* __LINUX_USB_QUIRKS_H */ > > -- > > 2.35.1 > > > > I'll drop this for now as there are no in-kernel users for this quirk > yet. When there is a need for one, please resubmit it. > > thanks, > > greg k-h >
On Fri, May 13, 2022 at 11:50:26AM +0200, Jose Ignacio Tornos Martinez wrote: > Ok, I will try to identify the "bad" devices in some way. > > Thanks > > José Ignacio > > > On Thu, May 12, 2022 at 1:48 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > I'll drop this for now as there are no in-kernel users for this quirk > > yet. When there is a need for one, please resubmit it. Hold on; Greg's comment doesn't seem fair. There are no in-kernel users for this quirk because it is meant to be a user API. (Just as there are no in-kernel users for read(2) -- it is there so that userspace can call it). Jose does have users for the new quirk: Anybody with one of the bad Bluetooth CSR knockoff chips. Now I agree; it would be great if there was some way to identify them automatically. But if that's not possible, the only alternative is to allow userspace to set the quirk flag whenever it knows the quirk is needed. Alan Stern
On Fri, May 13, 2022 at 10:09:26AM -0400, Alan Stern wrote: > On Fri, May 13, 2022 at 11:50:26AM +0200, Jose Ignacio Tornos Martinez wrote: > > Ok, I will try to identify the "bad" devices in some way. > > > > Thanks > > > > José Ignacio > > > > > > On Thu, May 12, 2022 at 1:48 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > I'll drop this for now as there are no in-kernel users for this quirk > > > yet. When there is a need for one, please resubmit it. > > Hold on; Greg's comment doesn't seem fair. There are no in-kernel > users for this quirk because it is meant to be a user API. (Just as > there are no in-kernel users for read(2) -- it is there so that > userspace can call it). True, but the kernel calls read(2) itself as well in places, it just looks a bit different, kernel_read_file() :) > Jose does have users for the new quirk: Anybody with one of the bad > Bluetooth CSR knockoff chips. Now I agree; it would be great if there > was some way to identify them automatically. But if that's not > possible, the only alternative is to allow userspace to set the quirk > flag whenever it knows the quirk is needed. Is that the case here that we know how to identify this? I thought Marcel said something else was happening here. If the bluetooth developers/maintainers say this is needed for some devices to work properly and they will be handled in userspace somehow through a udev rule or the like, I will gladly add this. But I thought this thread died out as it was determined that this wasn't needed at this point in time which is why I dropped it. thanks, greg k-h
On Fri, May 13, 2022 at 04:13:07PM +0200, Greg KH wrote: > On Fri, May 13, 2022 at 10:09:26AM -0400, Alan Stern wrote: > > On Fri, May 13, 2022 at 11:50:26AM +0200, Jose Ignacio Tornos Martinez wrote: > > > Ok, I will try to identify the "bad" devices in some way. > > > > > > Thanks > > > > > > José Ignacio > > > > > > > > > On Thu, May 12, 2022 at 1:48 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > I'll drop this for now as there are no in-kernel users for this quirk > > > > yet. When there is a need for one, please resubmit it. > > > > Hold on; Greg's comment doesn't seem fair. There are no in-kernel > > users for this quirk because it is meant to be a user API. (Just as > > there are no in-kernel users for read(2) -- it is there so that > > userspace can call it). > > True, but the kernel calls read(2) itself as well in places, it just > looks a bit different, kernel_read_file() :) Okay, but you get the point. :-) > > Jose does have users for the new quirk: Anybody with one of the bad > > Bluetooth CSR knockoff chips. Now I agree; it would be great if there > > was some way to identify them automatically. But if that's not > > possible, the only alternative is to allow userspace to set the quirk > > flag whenever it knows the quirk is needed. > > Is that the case here that we know how to identify this? I thought > Marcel said something else was happening here. > > If the bluetooth developers/maintainers say this is needed for some > devices to work properly and they will be handled in userspace somehow > through a udev rule or the like, I will gladly add this. But I thought > this thread died out as it was determined that this wasn't needed at > this point in time which is why I dropped it. It's kind of an odd situation. In ordinary usage the device works okay. But it stops working after it has been exported over usbip; that is what Jose wants to fix. Come to think of it, maybe there is a simple workaround. If userspace resets the device after it is unconfigured, there's a good chance that will get it to start working again. Jose, can you try this? There is a usbreset program you can use, floating around on the web. (Greg, did that program or something like it ever get added to the usbutils package?) I don't know how much attention the bluetooth people have given to this issue so far. Marcel should be able to provide more information. Alan Stern
On Fri, May 13, 2022 at 10:26:02AM -0400, Alan Stern wrote: > On Fri, May 13, 2022 at 04:13:07PM +0200, Greg KH wrote: > > On Fri, May 13, 2022 at 10:09:26AM -0400, Alan Stern wrote: > > > On Fri, May 13, 2022 at 11:50:26AM +0200, Jose Ignacio Tornos Martinez wrote: > > > > Ok, I will try to identify the "bad" devices in some way. > > > > > > > > Thanks > > > > > > > > José Ignacio > > > > > > > > > > > > On Thu, May 12, 2022 at 1:48 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > > > I'll drop this for now as there are no in-kernel users for this quirk > > > > > yet. When there is a need for one, please resubmit it. > > > > > > Hold on; Greg's comment doesn't seem fair. There are no in-kernel > > > users for this quirk because it is meant to be a user API. (Just as > > > there are no in-kernel users for read(2) -- it is there so that > > > userspace can call it). > > > > True, but the kernel calls read(2) itself as well in places, it just > > looks a bit different, kernel_read_file() :) > > Okay, but you get the point. :-) > > > > Jose does have users for the new quirk: Anybody with one of the bad > > > Bluetooth CSR knockoff chips. Now I agree; it would be great if there > > > was some way to identify them automatically. But if that's not > > > possible, the only alternative is to allow userspace to set the quirk > > > flag whenever it knows the quirk is needed. > > > > Is that the case here that we know how to identify this? I thought > > Marcel said something else was happening here. > > > > If the bluetooth developers/maintainers say this is needed for some > > devices to work properly and they will be handled in userspace somehow > > through a udev rule or the like, I will gladly add this. But I thought > > this thread died out as it was determined that this wasn't needed at > > this point in time which is why I dropped it. > > It's kind of an odd situation. In ordinary usage the device works okay. > But it stops working after it has been exported over usbip; that is what > Jose wants to fix. > > Come to think of it, maybe there is a simple workaround. If userspace > resets the device after it is unconfigured, there's a good chance that > will get it to start working again. Jose, can you try this? There is a > usbreset program you can use, floating around on the web. (Greg, did > that program or something like it ever get added to the usbutils > package?) Yes, it is in the usbutils package. I don't think many distros package the prebuilt binary, but the .c file can be found here: https://github.com/gregkh/usbutils/blob/master/usbreset.c
>> Come to think of it, maybe there is a simple workaround. If userspace >> resets the device after it is unconfigured, there's a good chance that >> will get it to start working again. Jose, can you try this? There is a >> usbreset program you can use, floating around on the web. (Greg, did >> that program or something like it ever get added to the usbutils >> package?) > Yes, it is in the usbutils package. I don't think many distros package > the prebuilt binary, but the .c file can be found here: > https://github.com/gregkh/usbutils/blob/master/usbreset.c Of course, I can also try that way and I will comment on the result. I would like to identify in some way, I have some ideas related to the initial configuration but I have to research more. If not possible, if there is a user workaround like this, it would be great. Thank you very much José Ignacio On Fri, May 13, 2022 at 4:46 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Fri, May 13, 2022 at 10:26:02AM -0400, Alan Stern wrote: > > On Fri, May 13, 2022 at 04:13:07PM +0200, Greg KH wrote: > > > On Fri, May 13, 2022 at 10:09:26AM -0400, Alan Stern wrote: > > > > On Fri, May 13, 2022 at 11:50:26AM +0200, Jose Ignacio Tornos Martinez wrote: > > > > > Ok, I will try to identify the "bad" devices in some way. > > > > > > > > > > Thanks > > > > > > > > > > José Ignacio > > > > > > > > > > > > > > > On Thu, May 12, 2022 at 1:48 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > > > > > I'll drop this for now as there are no in-kernel users for this quirk > > > > > > yet. When there is a need for one, please resubmit it. > > > > > > > > Hold on; Greg's comment doesn't seem fair. There are no in-kernel > > > > users for this quirk because it is meant to be a user API. (Just as > > > > there are no in-kernel users for read(2) -- it is there so that > > > > userspace can call it). > > > > > > True, but the kernel calls read(2) itself as well in places, it just > > > looks a bit different, kernel_read_file() :) > > > > Okay, but you get the point. :-) > > > > > > Jose does have users for the new quirk: Anybody with one of the bad > > > > Bluetooth CSR knockoff chips. Now I agree; it would be great if there > > > > was some way to identify them automatically. But if that's not > > > > possible, the only alternative is to allow userspace to set the quirk > > > > flag whenever it knows the quirk is needed. > > > > > > Is that the case here that we know how to identify this? I thought > > > Marcel said something else was happening here. > > > > > > If the bluetooth developers/maintainers say this is needed for some > > > devices to work properly and they will be handled in userspace somehow > > > through a udev rule or the like, I will gladly add this. But I thought > > > this thread died out as it was determined that this wasn't needed at > > > this point in time which is why I dropped it. > > > > It's kind of an odd situation. In ordinary usage the device works okay. > > But it stops working after it has been exported over usbip; that is what > > Jose wants to fix. > > > > Come to think of it, maybe there is a simple workaround. If userspace > > resets the device after it is unconfigured, there's a good chance that > > will get it to start working again. Jose, can you try this? There is a > > usbreset program you can use, floating around on the web. (Greg, did > > that program or something like it ever get added to the usbutils > > package?) > > Yes, it is in the usbutils package. I don't think many distros package > the prebuilt binary, but the .c file can be found here: > https://github.com/gregkh/usbutils/blob/master/usbreset.c >
Alan Stern <stern@rowland.harvard.edu> writes: > (Greg, did that program or something like it ever get added to the > usbutils package?) (your) usbreset.c was added to usbutils in 2016 :-) Bjørn
I have just tested usbreset program (without my previous patch):
$ hciconfig
hci0: Type: Primary Bus: USB
BD Address: 00:1A:7D:DA:71:13 ACL MTU: 310:10 SCO MTU: 64:8
UP RUNNING
RX bytes:696 acl:0 sco:0 events:49 errors:0
TX bytes:3168 acl:0 sco:0 commands:49 errors:0
< scanning is working from Bluetooth Settings from GNOME >
$ echo 0 >/sys/bus/usb/devices/1-3/bConfigurationValue
$ ./usbreset 001/003
Resetting CSR8510 A10 ... ok
$ echo 1 >/sys/bus/usb/devices/1-3/bConfigurationValue
< it takes a while>
bash: echo: write error: Connection timed out
$
Unfortunately, it is not solving the issue and I get the same result.
I will try to identify the device or the situation in some way.
Best regards
José Ignacio
On Fri, May 13, 2022 at 5:01 PM Bjørn Mork <bjorn@mork.no> wrote:
>
> Alan Stern <stern@rowland.harvard.edu> writes:
>
> > (Greg, did that program or something like it ever get added to the
> > usbutils package?)
>
> (your) usbreset.c was added to usbutils in 2016 :-)
>
>
> Bjørn
>
I have been trying to identify the chips that I have in some way, but as I do not have a big sampling or more information, it is difficult. And I have verified again the datasheets for CSR (Qualcomm) and they do not comment anything related to the problem. All the devices contain this information: manufacturer 10 hci_rev 22bb lmp_subver 22bb hci_ver 6 (BLUETOOTH_VER_4_0) And two of them contain bcdDevice=88.91 (Trust and unknown) and the other 75.58 (EDR) Following the code and comments to detect fakes in btusb.c, the devices are correct although it comments that 0x7558, 0x8891 are known fake bcdDevices. In fact, the unconfigure issue is the only problem that I have found. So, I think the only solution, at least to be able to work, would be to add the user quirk with the patch. Of course, if you consider. Thanks again Best regards José Ignacio On Fri, May 13, 2022 at 4:58 PM Jose Ignacio Tornos Martinez <jtornosm@redhat.com> wrote: > > >> Come to think of it, maybe there is a simple workaround. If userspace > >> resets the device after it is unconfigured, there's a good chance that > >> will get it to start working again. Jose, can you try this? There is a > >> usbreset program you can use, floating around on the web. (Greg, did > >> that program or something like it ever get added to the usbutils > >> package?) > > > Yes, it is in the usbutils package. I don't think many distros package > > the prebuilt binary, but the .c file can be found here: > > https://github.com/gregkh/usbutils/blob/master/usbreset.c > > Of course, I can also try that way and I will comment on the result. > I would like to identify in some way, I have some ideas related to the > initial configuration but I have to research more. > If not possible, if there is a user workaround like this, it would be great. > > Thank you very much > > José Ignacio > > On Fri, May 13, 2022 at 4:46 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > On Fri, May 13, 2022 at 10:26:02AM -0400, Alan Stern wrote: > > > On Fri, May 13, 2022 at 04:13:07PM +0200, Greg KH wrote: > > > > On Fri, May 13, 2022 at 10:09:26AM -0400, Alan Stern wrote: > > > > > On Fri, May 13, 2022 at 11:50:26AM +0200, Jose Ignacio Tornos Martinez wrote: > > > > > > Ok, I will try to identify the "bad" devices in some way. > > > > > > > > > > > > Thanks > > > > > > > > > > > > José Ignacio > > > > > > > > > > > > > > > > > > On Thu, May 12, 2022 at 1:48 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > > > > > > > I'll drop this for now as there are no in-kernel users for this quirk > > > > > > > yet. When there is a need for one, please resubmit it. > > > > > > > > > > Hold on; Greg's comment doesn't seem fair. There are no in-kernel > > > > > users for this quirk because it is meant to be a user API. (Just as > > > > > there are no in-kernel users for read(2) -- it is there so that > > > > > userspace can call it). > > > > > > > > True, but the kernel calls read(2) itself as well in places, it just > > > > looks a bit different, kernel_read_file() :) > > > > > > Okay, but you get the point. :-) > > > > > > > > Jose does have users for the new quirk: Anybody with one of the bad > > > > > Bluetooth CSR knockoff chips. Now I agree; it would be great if there > > > > > was some way to identify them automatically. But if that's not > > > > > possible, the only alternative is to allow userspace to set the quirk > > > > > flag whenever it knows the quirk is needed. > > > > > > > > Is that the case here that we know how to identify this? I thought > > > > Marcel said something else was happening here. > > > > > > > > If the bluetooth developers/maintainers say this is needed for some > > > > devices to work properly and they will be handled in userspace somehow > > > > through a udev rule or the like, I will gladly add this. But I thought > > > > this thread died out as it was determined that this wasn't needed at > > > > this point in time which is why I dropped it. > > > > > > It's kind of an odd situation. In ordinary usage the device works okay. > > > But it stops working after it has been exported over usbip; that is what > > > Jose wants to fix. > > > > > > Come to think of it, maybe there is a simple workaround. If userspace > > > resets the device after it is unconfigured, there's a good chance that > > > will get it to start working again. Jose, can you try this? There is a > > > usbreset program you can use, floating around on the web. (Greg, did > > > that program or something like it ever get added to the usbutils > > > package?) > > > > Yes, it is in the usbutils package. I don't think many distros package > > the prebuilt binary, but the .c file can be found here: > > https://github.com/gregkh/usbutils/blob/master/usbreset.c > >
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 3f1cc5e317ed..71651b888d14 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -6183,6 +6183,8 @@ pause after every control message); o = USB_QUIRK_HUB_SLOW_RESET (Hub needs extra delay after resetting its port); + p = USB_QUIRK_SKIP_UNCONFIGURE (device doesn't + support unconfigure); Example: quirks=0781:5580:bk,0a5c:5834:gij usbhid.mousepoll= diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 4d59d927ae3e..9c6cd0c75f4f 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -2108,9 +2108,15 @@ int usb_set_configuration(struct usb_device *dev, int configuration) } kfree(new_interfaces); - ret = usb_control_msg_send(dev, 0, USB_REQ_SET_CONFIGURATION, 0, - configuration, 0, NULL, 0, - USB_CTRL_SET_TIMEOUT, GFP_NOIO); + if (configuration == 0 && !cp + && (dev->quirks & USB_QUIRK_SKIP_UNCONFIGURE)) { + dev_warn(&dev->dev, "device is not unconfigured!\n"); + ret = 0; + } else + ret = usb_control_msg_send(dev, 0, USB_REQ_SET_CONFIGURATION, 0, + configuration, 0, NULL, 0, + USB_CTRL_SET_TIMEOUT, GFP_NOIO); + if (ret && cp) { /* * All the old state is gone, so what else can we do? diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c index d3c14b5ed4a1..f4cdf85a9eb6 100644 --- a/drivers/usb/core/quirks.c +++ b/drivers/usb/core/quirks.c @@ -138,6 +138,9 @@ static int quirks_param_set(const char *value, const struct kernel_param *kp) case 'o': flags |= USB_QUIRK_HUB_SLOW_RESET; break; + case 'p': + flags |= USB_QUIRK_SKIP_UNCONFIGURE; + break; /* Ignore unrecognized flag characters */ } } diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h index eeb7c2157c72..79cb0616f394 100644 --- a/include/linux/usb/quirks.h +++ b/include/linux/usb/quirks.h @@ -72,4 +72,7 @@ /* device has endpoints that should be ignored */ #define USB_QUIRK_ENDPOINT_IGNORE BIT(15) +/* device doesn't support unconfigure. */ +#define USB_QUIRK_SKIP_UNCONFIGURE BIT(16) + #endif /* __LINUX_USB_QUIRKS_H */
Some devices like Bluetooth Dongles with CSR chip (i.e. USB Bluetooth V4.0 Dongle by Trust) hang when they are unbound from 'unbind' sysfs entry and can not be bound again. For these devices, CSR chip hangs when usb configuration command with index 0 (used to unconfigure) is sent during disconnection. To avoid this unwanted result, it is necessary not to send this command, so a new quirk has been created. By default, quirk is not applied for any device and needs to be enabled by user. For these devices, athough device is not unconfigured, it is better to avoid device hanging to be able to operate. Even bluetooth can be previously turned off. On the other hand, this is not important if usb device is going to be bound again (normal behavior), i.e. with usbip. Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com> --- V4 -> V5: - By default, quirk is not applied for any device and needs to be enabled by user if necessary. V3 -> V4: - Reorder quirk entries to be in numerical order according to the vendor ID and product ID. - Add patch version information. V2 -> V3: - Change subject (Bluetooth: btusb: CSR chip hangs when unbound -> USB: core: skip unconfiguration if device doesn't support it). - Improve quirk checking. - Allow to test quirk interactively. V1 -> V2: - Use quirk feature for the exception. Documentation/admin-guide/kernel-parameters.txt | 2 ++ drivers/usb/core/message.c | 12 +++++++++--- drivers/usb/core/quirks.c | 3 +++ include/linux/usb/quirks.h | 3 +++ 4 files changed, 17 insertions(+), 3 deletions(-)