linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] USB: core: skip unconfiguration if device doesn't support it
@ 2022-05-04  8:36 Jose Ignacio Tornos Martinez
  2022-05-04 12:23 ` Marcel Holtmann
  0 siblings, 1 reply; 12+ messages in thread
From: Jose Ignacio Tornos Martinez @ 2022-05-04  8:36 UTC (permalink / raw)
  To: gregkh, stern, linux-usb; +Cc: marcel, Jose Ignacio Tornos Martinez

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.

The reason is 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 for CSR chip, so a new quirk has been created.

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>
---
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                       |  6 ++++++
 include/linux/usb/quirks.h                      |  3 +++
 4 files changed, 20 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..7c86c8d61570 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 */
 			}
 		}
@@ -394,6 +397,9 @@ static const struct usb_device_id usb_quirk_list[] = {
 	/* ELMO L-12F document camera */
 	{ USB_DEVICE(0x09a1, 0x0028), .driver_info = USB_QUIRK_DELAY_CTRL_MSG },
 
+	/* CSR Bluetooth */
+	{ USB_DEVICE(0x0a12, 0x0001), .driver_info = USB_QUIRK_SKIP_UNCONFIGURE },
+
 	/* Broadcom BCM92035DGROM BT dongle */
 	{ USB_DEVICE(0x0a5c, 0x2021), .driver_info = USB_QUIRK_RESET_RESUME },
 
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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] USB: core: skip unconfiguration if device doesn't support it
  2022-05-04  8:36 [PATCH v4] USB: core: skip unconfiguration if device doesn't support it Jose Ignacio Tornos Martinez
@ 2022-05-04 12:23 ` Marcel Holtmann
  2022-05-04 12:32   ` Jose Ignacio Tornos Martinez
  2022-05-05 10:19   ` Oliver Neukum
  0 siblings, 2 replies; 12+ messages in thread
From: Marcel Holtmann @ 2022-05-04 12:23 UTC (permalink / raw)
  To: Jose Ignacio Tornos Martinez; +Cc: gregkh, stern, linux-usb

Hi Jose,

> 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.
> 
> The reason is 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 for CSR chip, so a new quirk has been created.
> 
> 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>
> ---
> 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                       |  6 ++++++
> include/linux/usb/quirks.h                      |  3 +++
> 4 files changed, 20 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..7c86c8d61570 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 */
> 			}
> 		}
> @@ -394,6 +397,9 @@ static const struct usb_device_id usb_quirk_list[] = {
> 	/* ELMO L-12F document camera */
> 	{ USB_DEVICE(0x09a1, 0x0028), .driver_info = USB_QUIRK_DELAY_CTRL_MSG },
> 
> +	/* CSR Bluetooth */
> +	{ USB_DEVICE(0x0a12, 0x0001), .driver_info = USB_QUIRK_SKIP_UNCONFIGURE },
> +

NAK. I said this before.

The VID:PID 0a12:0001 is used in millions of products that work
correctly. Only because some counterfeit products get things wrong
doesn’t mean all get marked as broken.

Regards

Marcel


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] USB: core: skip unconfiguration if device doesn't support it
  2022-05-04 12:23 ` Marcel Holtmann
@ 2022-05-04 12:32   ` Jose Ignacio Tornos Martinez
  2022-05-05 10:19   ` Oliver Neukum
  1 sibling, 0 replies; 12+ messages in thread
From: Jose Ignacio Tornos Martinez @ 2022-05-04 12:32 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Greg KH, Alan Stern, linux-usb

Hello Marcel,

Ok, I am not sure if it is working in other related products ...
Would it be ok if we keep the possibility to use the quirk if necessary?
That is, by default CSR devices are not marked with the quirk but they
can be marked by the users if necessary.

Thanks

Best regards
José Ignacio

On Wed, May 4, 2022 at 2:23 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Jose,
>
> > 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.
> >
> > The reason is 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 for CSR chip, so a new quirk has been created.
> >
> > 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>
> > ---
> > 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                       |  6 ++++++
> > include/linux/usb/quirks.h                      |  3 +++
> > 4 files changed, 20 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..7c86c8d61570 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 */
> >                       }
> >               }
> > @@ -394,6 +397,9 @@ static const struct usb_device_id usb_quirk_list[] = {
> >       /* ELMO L-12F document camera */
> >       { USB_DEVICE(0x09a1, 0x0028), .driver_info = USB_QUIRK_DELAY_CTRL_MSG },
> >
> > +     /* CSR Bluetooth */
> > +     { USB_DEVICE(0x0a12, 0x0001), .driver_info = USB_QUIRK_SKIP_UNCONFIGURE },
> > +
>
> NAK. I said this before.
>
> The VID:PID 0a12:0001 is used in millions of products that work
> correctly. Only because some counterfeit products get things wrong
> doesn’t mean all get marked as broken.
>
> Regards
>
> Marcel
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] USB: core: skip unconfiguration if device doesn't support it
  2022-05-04 12:23 ` Marcel Holtmann
  2022-05-04 12:32   ` Jose Ignacio Tornos Martinez
@ 2022-05-05 10:19   ` Oliver Neukum
  2022-05-05 11:15     ` Jose Ignacio Tornos Martinez
  2022-05-05 11:29     ` Bjørn Mork
  1 sibling, 2 replies; 12+ messages in thread
From: Oliver Neukum @ 2022-05-05 10:19 UTC (permalink / raw)
  To: Marcel Holtmann, Jose Ignacio Tornos Martinez; +Cc: gregkh, stern, linux-usb



On 04.05.22 14:23, Marcel Holtmann wrote:
>
>> @@ -394,6 +397,9 @@ static const struct usb_device_id usb_quirk_list[] = {
>> 	/* ELMO L-12F document camera */
>> 	{ USB_DEVICE(0x09a1, 0x0028), .driver_info = USB_QUIRK_DELAY_CTRL_MSG },
>>
>> +	/* CSR Bluetooth */
>> +	{ USB_DEVICE(0x0a12, 0x0001), .driver_info = USB_QUIRK_SKIP_UNCONFIGURE },
>> +
> NAK. I said this before.
>
> The VID:PID 0a12:0001 is used in millions of products that work
> correctly. Only because some counterfeit products get things wrong
> doesn’t mean all get marked as broken.
>

Hi,

if I may ask, how certain is that? Soft unbinding is probably not
a common operation.

    Regards
        Oliver


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] USB: core: skip unconfiguration if device doesn't support it
  2022-05-05 10:19   ` Oliver Neukum
@ 2022-05-05 11:15     ` Jose Ignacio Tornos Martinez
  2022-05-05 12:06       ` Oliver Neukum
  2022-05-05 11:29     ` Bjørn Mork
  1 sibling, 1 reply; 12+ messages in thread
From: Jose Ignacio Tornos Martinez @ 2022-05-05 11:15 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: Marcel Holtmann, Greg KH, Alan Stern, linux-usb

Hello Oliver,

Of course, I am working with usbip to remotize usb devices, that is
the reason why unbind/bind is needed and with the btusb devices that I
have, it was not working.

Best regards
José Ignacio

On Thu, May 5, 2022 at 12:19 PM Oliver Neukum <oneukum@suse.com> wrote:
>
>
>
> On 04.05.22 14:23, Marcel Holtmann wrote:
> >
> >> @@ -394,6 +397,9 @@ static const struct usb_device_id usb_quirk_list[] = {
> >>      /* ELMO L-12F document camera */
> >>      { USB_DEVICE(0x09a1, 0x0028), .driver_info = USB_QUIRK_DELAY_CTRL_MSG },
> >>
> >> +    /* CSR Bluetooth */
> >> +    { USB_DEVICE(0x0a12, 0x0001), .driver_info = USB_QUIRK_SKIP_UNCONFIGURE },
> >> +
> > NAK. I said this before.
> >
> > The VID:PID 0a12:0001 is used in millions of products that work
> > correctly. Only because some counterfeit products get things wrong
> > doesn’t mean all get marked as broken.
> >
>
> Hi,
>
> if I may ask, how certain is that? Soft unbinding is probably not
> a common operation.
>
>     Regards
>         Oliver
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] USB: core: skip unconfiguration if device doesn't support it
  2022-05-05 10:19   ` Oliver Neukum
  2022-05-05 11:15     ` Jose Ignacio Tornos Martinez
@ 2022-05-05 11:29     ` Bjørn Mork
  2022-05-05 14:14       ` Alan Stern
  1 sibling, 1 reply; 12+ messages in thread
From: Bjørn Mork @ 2022-05-05 11:29 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: Marcel Holtmann, Jose Ignacio Tornos Martinez, gregkh, stern, linux-usb

Oliver Neukum <oneukum@suse.com> writes:
> On 04.05.22 14:23, Marcel Holtmann wrote:
>>
>>> @@ -394,6 +397,9 @@ static const struct usb_device_id usb_quirk_list[] = {
>>> 	/* ELMO L-12F document camera */
>>> 	{ USB_DEVICE(0x09a1, 0x0028), .driver_info = USB_QUIRK_DELAY_CTRL_MSG },
>>>
>>> +	/* CSR Bluetooth */
>>> +	{ USB_DEVICE(0x0a12, 0x0001), .driver_info = USB_QUIRK_SKIP_UNCONFIGURE },
>>> +
>> NAK. I said this before.
>>
>> The VID:PID 0a12:0001 is used in millions of products that work
>> correctly. Only because some counterfeit products get things wrong
>> doesn’t mean all get marked as broken.
>>
>
> Hi,
>
> if I may ask, how certain is that?

100%.  Just get a real CSR device and try it.


canardo:/tmp# sed -ne '/ Port=07 /,/^$/p' /sys/kernel/debug/usb/devices 
T:  Bus=01 Lev=01 Prnt=01 Port=07 Cnt=05 Dev#=  6 Spd=12   MxCh= 0
D:  Ver= 2.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=0a12 ProdID=0001 Rev=88.91
C:* #Ifs= 2 Cfg#= 1 Atr=c0 MxPwr=  0mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=81(I) Atr=03(Int.) MxPS=  16 Ivl=1ms
E:  Ad=02(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   0 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   0 Ivl=1ms
I:  If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   9 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   9 Ivl=1ms
I:  If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  17 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  17 Ivl=1ms
I:  If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  25 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  25 Ivl=1ms
I:  If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  33 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  33 Ivl=1ms
I:  If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  49 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  49 Ivl=1ms

canardo:/tmp# hciconfig hci0
hci0:   Type: Primary  Bus: USB
        BD Address: 00:1A:7D:DA:71:15  ACL MTU: 310:10  SCO MTU: 64:8
        UP RUNNING 
        RX bytes:660 acl:0 sco:0 events:43 errors:0
        TX bytes:2178 acl:0 sco:0 commands:43 errors:0

canardo:/tmp# ls -l /sys/bus/usb/drivers/btusb/
total 0
lrwxrwxrwx 1 root root    0 May  5 13:23 1-8:1.0 -> ../../../../devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.0
lrwxrwxrwx 1 root root    0 May  5 13:23 1-8:1.1 -> ../../../../devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.1
--w------- 1 root root 4096 May  5 13:23 bind
lrwxrwxrwx 1 root root    0 May  5 13:22 module -> ../../../../module/btusb
-rw-r--r-- 1 root root 4096 May  5 13:22 new_id
-rw-r--r-- 1 root root 4096 May  5 13:22 remove_id
--w------- 1 root root 4096 May  5 13:22 uevent
--w------- 1 root root 4096 May  5 13:23 unbind


canardo:/tmp# echo 1-8:1.0 > /sys/bus/usb/drivers/btusb/unbind
canardo:/tmp# sed -ne '/ Port=07 /,/^$/p' /sys/kernel/debug/usb/devices 
T:  Bus=01 Lev=01 Prnt=01 Port=07 Cnt=05 Dev#=  6 Spd=12   MxCh= 0
D:  Ver= 2.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=0a12 ProdID=0001 Rev=88.91
C:* #Ifs= 2 Cfg#= 1 Atr=c0 MxPwr=  0mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=(none)
E:  Ad=81(I) Atr=03(Int.) MxPS=  16 Ivl=1ms
E:  Ad=02(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=(none)
E:  Ad=03(O) Atr=01(Isoc) MxPS=   0 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   0 Ivl=1ms
I:  If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=(none)
E:  Ad=03(O) Atr=01(Isoc) MxPS=   9 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   9 Ivl=1ms
I:  If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=(none)
E:  Ad=03(O) Atr=01(Isoc) MxPS=  17 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  17 Ivl=1ms
I:  If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=(none)
E:  Ad=03(O) Atr=01(Isoc) MxPS=  25 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  25 Ivl=1ms
I:  If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=(none)
E:  Ad=03(O) Atr=01(Isoc) MxPS=  33 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  33 Ivl=1ms
I:  If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=(none)
E:  Ad=03(O) Atr=01(Isoc) MxPS=  49 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  49 Ivl=1ms

canardo:/tmp# hciconfig hci0
Can't get device info: No such device
canardo:/tmp# ls -l /sys/bus/usb/drivers/btusb/
total 0
--w------- 1 root root 4096 May  5 13:23 bind
lrwxrwxrwx 1 root root    0 May  5 13:22 module -> ../../../../module/btusb
-rw-r--r-- 1 root root 4096 May  5 13:22 new_id
-rw-r--r-- 1 root root 4096 May  5 13:22 remove_id
--w------- 1 root root 4096 May  5 13:22 uevent
--w------- 1 root root 4096 May  5 13:26 unbind


canardo:/tmp# echo 1-8:1.0 > /sys/bus/usb/drivers/btusb/bind
canardo:/tmp# sed -ne '/ Port=07 /,/^$/p' /sys/kernel/debug/usb/devices 
T:  Bus=01 Lev=01 Prnt=01 Port=07 Cnt=05 Dev#=  6 Spd=12   MxCh= 0
D:  Ver= 2.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=0a12 ProdID=0001 Rev=88.91
C:* #Ifs= 2 Cfg#= 1 Atr=c0 MxPwr=  0mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=81(I) Atr=03(Int.) MxPS=  16 Ivl=1ms
E:  Ad=02(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   0 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   0 Ivl=1ms
I:  If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   9 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   9 Ivl=1ms
I:  If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  17 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  17 Ivl=1ms
I:  If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  25 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  25 Ivl=1ms
I:  If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  33 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  33 Ivl=1ms
I:  If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  49 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  49 Ivl=1ms

canardo:/tmp# hciconfig hci0
hci0:   Type: Primary  Bus: USB
        BD Address: 00:1A:7D:DA:71:15  ACL MTU: 310:10  SCO MTU: 64:8
        UP RUNNING 
        RX bytes:660 acl:0 sco:0 events:43 errors:0
        TX bytes:2178 acl:0 sco:0 commands:43 errors:0

canardo:/tmp# ls -l /sys/bus/usb/drivers/btusb/
total 0
lrwxrwxrwx 1 root root    0 May  5 13:27 1-8:1.0 -> ../../../../devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.0
lrwxrwxrwx 1 root root    0 May  5 13:27 1-8:1.1 -> ../../../../devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.1
--w------- 1 root root 4096 May  5 13:27 bind
lrwxrwxrwx 1 root root    0 May  5 13:22 module -> ../../../../module/btusb
-rw-r--r-- 1 root root 4096 May  5 13:22 new_id
-rw-r--r-- 1 root root 4096 May  5 13:22 remove_id
--w------- 1 root root 4096 May  5 13:22 uevent
--w------- 1 root root 4096 May  5 13:26 unbind


I do have a couple of fake ones too.  They are mostly interesting from a
"why the h... would anyone do that?" perspective



Bjørn

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] USB: core: skip unconfiguration if device doesn't support it
  2022-05-05 11:15     ` Jose Ignacio Tornos Martinez
@ 2022-05-05 12:06       ` Oliver Neukum
  2022-05-05 12:35         ` Jose Ignacio Tornos Martinez
  0 siblings, 1 reply; 12+ messages in thread
From: Oliver Neukum @ 2022-05-05 12:06 UTC (permalink / raw)
  To: Jose Ignacio Tornos Martinez, Oliver Neukum
  Cc: Marcel Holtmann, Greg KH, Alan Stern, linux-usb



On 05.05.22 13:15, Jose Ignacio Tornos Martinez wrote:
> Hello Oliver,
>
> Of course, I am working with usbip to remotize usb devices, that is
> the reason why unbind/bind is needed and with the btusb devices that I
> have, it was not working.
>
>

Hi,

sorry for being unclear. I was not referring to positive
knowledge about the devices you are testing with.
I was having dark thoughts about the other devices
they are sharing an ID with.
But Bjorn's testing has resolved that. In that case
we can indeed not penalize the compliant devices
for the broken ones.

One question, though. Your approach of simply doing
nothing if config 0 is to be selected again is a bit
brutal. Have you considered resetting the device
and stopping the reenumeration right as a config
is supposed to be chosen?

    Regards
        Oliver


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] USB: core: skip unconfiguration if device doesn't support it
  2022-05-05 12:06       ` Oliver Neukum
@ 2022-05-05 12:35         ` Jose Ignacio Tornos Martinez
  0 siblings, 0 replies; 12+ messages in thread
From: Jose Ignacio Tornos Martinez @ 2022-05-05 12:35 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: Marcel Holtmann, Greg KH, Alan Stern, linux-usb

Hello Oliver,

No worries, as you said, Bjorn has provided a perfect test.

Yes, I have considered but I have thought that a clean and fast
solution is enough, that is, suspending everything from the driver,
because a new configuration is going to be applied when binding again
from local or remote.
Device reset may have other implications and as you commented soft
unbinding is not a common operation at least without binding again.

Best regards
José Ignacio

On Thu, May 5, 2022 at 2:07 PM Oliver Neukum <oneukum@suse.com> wrote:
>
>
>
> On 05.05.22 13:15, Jose Ignacio Tornos Martinez wrote:
> > Hello Oliver,
> >
> > Of course, I am working with usbip to remotize usb devices, that is
> > the reason why unbind/bind is needed and with the btusb devices that I
> > have, it was not working.
> >
> >
>
> Hi,
>
> sorry for being unclear. I was not referring to positive
> knowledge about the devices you are testing with.
> I was having dark thoughts about the other devices
> they are sharing an ID with.
> But Bjorn's testing has resolved that. In that case
> we can indeed not penalize the compliant devices
> for the broken ones.
>
> One question, though. Your approach of simply doing
> nothing if config 0 is to be selected again is a bit
> brutal. Have you considered resetting the device
> and stopping the reenumeration right as a config
> is supposed to be chosen?
>
>     Regards
>         Oliver
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] USB: core: skip unconfiguration if device doesn't support it
  2022-05-05 11:29     ` Bjørn Mork
@ 2022-05-05 14:14       ` Alan Stern
  2022-05-05 15:22         ` Bjørn Mork
  0 siblings, 1 reply; 12+ messages in thread
From: Alan Stern @ 2022-05-05 14:14 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: Oliver Neukum, Marcel Holtmann, Jose Ignacio Tornos Martinez,
	gregkh, linux-usb

On Thu, May 05, 2022 at 01:29:21PM +0200, Bjørn Mork wrote:
> Oliver Neukum <oneukum@suse.com> writes:
> > On 04.05.22 14:23, Marcel Holtmann wrote:
> >>
> >>> @@ -394,6 +397,9 @@ static const struct usb_device_id usb_quirk_list[] = {
> >>> 	/* ELMO L-12F document camera */
> >>> 	{ USB_DEVICE(0x09a1, 0x0028), .driver_info = USB_QUIRK_DELAY_CTRL_MSG },
> >>>
> >>> +	/* CSR Bluetooth */
> >>> +	{ USB_DEVICE(0x0a12, 0x0001), .driver_info = USB_QUIRK_SKIP_UNCONFIGURE },
> >>> +
> >> NAK. I said this before.
> >>
> >> The VID:PID 0a12:0001 is used in millions of products that work
> >> correctly. Only because some counterfeit products get things wrong
> >> doesn’t mean all get marked as broken.
> >>
> >
> > Hi,
> >
> > if I may ask, how certain is that?
> 
> 100%.  Just get a real CSR device and try it.

Please pardon me for butting in, but I don't see how this tests the 
condition that Jose is worried about.

You start with the device being configured and working:

> canardo:/tmp# sed -ne '/ Port=07 /,/^$/p' /sys/kernel/debug/usb/devices 
> T:  Bus=01 Lev=01 Prnt=01 Port=07 Cnt=05 Dev#=  6 Spd=12   MxCh= 0
> D:  Ver= 2.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=0a12 ProdID=0001 Rev=88.91
> C:* #Ifs= 2 Cfg#= 1 Atr=c0 MxPwr=  0mA
> I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=81(I) Atr=03(Int.) MxPS=  16 Ivl=1ms
> E:  Ad=02(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
> E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
> I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=03(O) Atr=01(Isoc) MxPS=   0 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=   0 Ivl=1ms
> I:  If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=03(O) Atr=01(Isoc) MxPS=   9 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=   9 Ivl=1ms
> I:  If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=03(O) Atr=01(Isoc) MxPS=  17 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=  17 Ivl=1ms
> I:  If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=03(O) Atr=01(Isoc) MxPS=  25 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=  25 Ivl=1ms
> I:  If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=03(O) Atr=01(Isoc) MxPS=  33 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=  33 Ivl=1ms
> I:  If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=03(O) Atr=01(Isoc) MxPS=  49 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=  49 Ivl=1ms
> 
> canardo:/tmp# hciconfig hci0
> hci0:   Type: Primary  Bus: USB
>         BD Address: 00:1A:7D:DA:71:15  ACL MTU: 310:10  SCO MTU: 64:8
>         UP RUNNING 
>         RX bytes:660 acl:0 sco:0 events:43 errors:0
>         TX bytes:2178 acl:0 sco:0 commands:43 errors:0
> 
> canardo:/tmp# ls -l /sys/bus/usb/drivers/btusb/
> total 0
> lrwxrwxrwx 1 root root    0 May  5 13:23 1-8:1.0 -> ../../../../devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.0
> lrwxrwxrwx 1 root root    0 May  5 13:23 1-8:1.1 -> ../../../../devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.1
> --w------- 1 root root 4096 May  5 13:23 bind
> lrwxrwxrwx 1 root root    0 May  5 13:22 module -> ../../../../module/btusb
> -rw-r--r-- 1 root root 4096 May  5 13:22 new_id
> -rw-r--r-- 1 root root 4096 May  5 13:22 remove_id
> --w------- 1 root root 4096 May  5 13:22 uevent
> --w------- 1 root root 4096 May  5 13:23 unbind

Then you unbind the Bluetooth driver:

> canardo:/tmp# echo 1-8:1.0 > /sys/bus/usb/drivers/btusb/unbind

But the device is still configured, as proved by the fact that there is 
a '*' following the "C:" below:

> canardo:/tmp# sed -ne '/ Port=07 /,/^$/p' /sys/kernel/debug/usb/devices 
> T:  Bus=01 Lev=01 Prnt=01 Port=07 Cnt=05 Dev#=  6 Spd=12   MxCh= 0
> D:  Ver= 2.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=0a12 ProdID=0001 Rev=88.91
> C:* #Ifs= 2 Cfg#= 1 Atr=c0 MxPwr=  0mA
> I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=(none)
> E:  Ad=81(I) Atr=03(Int.) MxPS=  16 Ivl=1ms
> E:  Ad=02(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
> E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
> I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=(none)
> E:  Ad=03(O) Atr=01(Isoc) MxPS=   0 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=   0 Ivl=1ms
> I:  If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=(none)
> E:  Ad=03(O) Atr=01(Isoc) MxPS=   9 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=   9 Ivl=1ms
> I:  If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=(none)
> E:  Ad=03(O) Atr=01(Isoc) MxPS=  17 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=  17 Ivl=1ms
> I:  If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=(none)
> E:  Ad=03(O) Atr=01(Isoc) MxPS=  25 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=  25 Ivl=1ms
> I:  If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=(none)
> E:  Ad=03(O) Atr=01(Isoc) MxPS=  33 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=  33 Ivl=1ms
> I:  If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=(none)
> E:  Ad=03(O) Atr=01(Isoc) MxPS=  49 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=  49 Ivl=1ms

Of course, since the btusb driver isn't bound, there is no corresponding 
Bluetooth interface in the kernel:

> canardo:/tmp# hciconfig hci0
> Can't get device info: No such device
> canardo:/tmp# ls -l /sys/bus/usb/drivers/btusb/
> total 0
> --w------- 1 root root 4096 May  5 13:23 bind
> lrwxrwxrwx 1 root root    0 May  5 13:22 module -> ../../../../module/btusb
> -rw-r--r-- 1 root root 4096 May  5 13:22 new_id
> -rw-r--r-- 1 root root 4096 May  5 13:22 remove_id
> --w------- 1 root root 4096 May  5 13:22 uevent
> --w------- 1 root root 4096 May  5 13:26 unbind

Then you rebind the driver:

> canardo:/tmp# echo 1-8:1.0 > /sys/bus/usb/drivers/btusb/bind
> canardo:/tmp# sed -ne '/ Port=07 /,/^$/p' /sys/kernel/debug/usb/devices 
> T:  Bus=01 Lev=01 Prnt=01 Port=07 Cnt=05 Dev#=  6 Spd=12   MxCh= 0
> D:  Ver= 2.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=0a12 ProdID=0001 Rev=88.91
> C:* #Ifs= 2 Cfg#= 1 Atr=c0 MxPwr=  0mA
> I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=81(I) Atr=03(Int.) MxPS=  16 Ivl=1ms
> E:  Ad=02(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
> E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
> I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=03(O) Atr=01(Isoc) MxPS=   0 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=   0 Ivl=1ms
> I:  If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=03(O) Atr=01(Isoc) MxPS=   9 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=   9 Ivl=1ms
> I:  If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=03(O) Atr=01(Isoc) MxPS=  17 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=  17 Ivl=1ms
> I:  If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=03(O) Atr=01(Isoc) MxPS=  25 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=  25 Ivl=1ms
> I:  If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=03(O) Atr=01(Isoc) MxPS=  33 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=  33 Ivl=1ms
> I:  If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=03(O) Atr=01(Isoc) MxPS=  49 Ivl=1ms
> E:  Ad=83(I) Atr=01(Isoc) MxPS=  49 Ivl=1ms
> 
> canardo:/tmp# hciconfig hci0
> hci0:   Type: Primary  Bus: USB
>         BD Address: 00:1A:7D:DA:71:15  ACL MTU: 310:10  SCO MTU: 64:8
>         UP RUNNING 
>         RX bytes:660 acl:0 sco:0 events:43 errors:0
>         TX bytes:2178 acl:0 sco:0 commands:43 errors:0
> 
> canardo:/tmp# ls -l /sys/bus/usb/drivers/btusb/
> total 0
> lrwxrwxrwx 1 root root    0 May  5 13:27 1-8:1.0 -> ../../../../devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.0
> lrwxrwxrwx 1 root root    0 May  5 13:27 1-8:1.1 -> ../../../../devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.1
> --w------- 1 root root 4096 May  5 13:27 bind
> lrwxrwxrwx 1 root root    0 May  5 13:22 module -> ../../../../module/btusb
> -rw-r--r-- 1 root root 4096 May  5 13:22 new_id
> -rw-r--r-- 1 root root 4096 May  5 13:22 remove_id
> --w------- 1 root root 4096 May  5 13:22 uevent
> --w------- 1 root root 4096 May  5 13:26 unbind

And presumably the device is working again.  However, none of this shows 
what happens when the device is unconfigured.  To test that, you would 
have to do:

	echo 0 >/sys/bus/usb/devices/1-8/bConfigurationValue
	echo 1 >/sys/bus/usb/devices/1-8/bConfigurationValue

and then see if the device continues to work.

Alan Stern

> I do have a couple of fake ones too.  They are mostly interesting from a
> "why the h... would anyone do that?" perspective
> 
> 
> 
> Bjørn

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] USB: core: skip unconfiguration if device doesn't support it
  2022-05-05 14:14       ` Alan Stern
@ 2022-05-05 15:22         ` Bjørn Mork
  2022-05-05 16:48           ` Jose Ignacio Tornos Martinez
  2022-05-05 17:04           ` Alan Stern
  0 siblings, 2 replies; 12+ messages in thread
From: Bjørn Mork @ 2022-05-05 15:22 UTC (permalink / raw)
  To: Alan Stern
  Cc: Oliver Neukum, Marcel Holtmann, Jose Ignacio Tornos Martinez,
	gregkh, linux-usb

Alan Stern <stern@rowland.harvard.edu> writes:

> Please pardon me for butting in, but I don't see how this tests the 
> condition that Jose is worried about.
..
> And presumably the device is working again.  However, none of this shows 
> what happens when the device is unconfigured.  To test that, you would 
> have to do:
>
> 	echo 0 >/sys/bus/usb/devices/1-8/bConfigurationValue
> 	echo 1 >/sys/bus/usb/devices/1-8/bConfigurationValue
>
> and then see if the device continues to work.

Ah, sorry. Scanned briefly, saw "bind", and assumed too much.  Making an
ass out of... you know.

Actually I didn't understand the part about unconfiguration since I
can't see how that would happen during normal usage?  Anyway,
unconfiguring also works for me:

canardo:/tmp# hciconfig hci0
hci0:   Type: Primary  Bus: USB
        BD Address: 00:1A:7D:DA:71:15  ACL MTU: 310:10  SCO MTU: 64:8
        UP RUNNING 
        RX bytes:660 acl:0 sco:0 events:43 errors:0
        TX bytes:2178 acl:0 sco:0 commands:43 errors:0

canardo:/tmp# echo 0 >/sys/bus/usb/devices/1-8/bConfigurationValue
canardo:/tmp# hciconfig hci0
Can't get device info: No such device
canardo:/tmp# echo 1 >/sys/bus/usb/devices/1-8/bConfigurationValue


Not entirely sure how to validate that "everything" works at this point?
I can use the rfcomm session the adapter usually handles and also run
lescan after this.  So I guess both BLE and BDR works.

And the counters count something:

canardo:/tmp# hciconfig hci0
hci0:   Type: Primary  Bus: USB
        BD Address: 00:1A:7D:DA:71:15  ACL MTU: 310:10  SCO MTU: 64:8
        UP RUNNING 
        RX bytes:3883 acl:40 sco:0 events:81 errors:0
        TX bytes:2518 acl:19 sco:0 commands:50 errors:0


I can see an error like this logged every time I unconfigure the device:

Bluetooth: hci0: urb 00000000e66a2492 failed to resubmit (2)

There is nothing else logged in kernel log


Bjørn

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] USB: core: skip unconfiguration if device doesn't support it
  2022-05-05 15:22         ` Bjørn Mork
@ 2022-05-05 16:48           ` Jose Ignacio Tornos Martinez
  2022-05-05 17:04           ` Alan Stern
  1 sibling, 0 replies; 12+ messages in thread
From: Jose Ignacio Tornos Martinez @ 2022-05-05 16:48 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: Alan Stern, Oliver Neukum, Marcel Holtmann, Greg KH, linux-usb

Hello,

Just repeating the same sequence here without the 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
$ hciconfig
$
< bluetooth device is not found in Bluetooth Settings from GNOME >
$ echo 1 >/sys/bus/usb/devices/1-3/bConfigurationValue
< it takes a while>
bash: echo: write error: Connection timed out
$ hciconfig
$
< bluetooth device is not found in Bluetooth Settings from GNOME >

Definitely, Bjorn, your device is working better than mine.

And the same sequence with the patch, to show that it is working:

$ cat /proc/cmdline
BOOT_IMAGE=... usbcore.quirks=0a12:0001:p
$ cat /sys/module/usbcore/parameters/quirks
0a12:0001:p
$ 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:806 acl:0 sco:0 events:66 errors:0
    TX bytes:5168 acl:0 sco:0 commands:65 errors:0
$
< scanning is working from Bluetooth Settings from GNOME >
$ echo 0 >/sys/bus/usb/devices/1-3/bConfigurationValue
$ hciconfig
$
< bluetooth device is not found in Bluetooth Settings from GNOME >
$ echo 1 >/sys/bus/usb/devices/1-3/bConfigurationValue
$ 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 >

In both cases, I also see similar log errors: Bluetooth: hci0: urb
00000000exxxxxxx failed to resubmit (2)

Best regards
José Ignacio




On Thu, May 5, 2022 at 5:23 PM Bjørn Mork <bjorn@mork.no> wrote:
>
> Alan Stern <stern@rowland.harvard.edu> writes:
>
> > Please pardon me for butting in, but I don't see how this tests the
> > condition that Jose is worried about.
> ..
> > And presumably the device is working again.  However, none of this shows
> > what happens when the device is unconfigured.  To test that, you would
> > have to do:
> >
> >       echo 0 >/sys/bus/usb/devices/1-8/bConfigurationValue
> >       echo 1 >/sys/bus/usb/devices/1-8/bConfigurationValue
> >
> > and then see if the device continues to work.
>
> Ah, sorry. Scanned briefly, saw "bind", and assumed too much.  Making an
> ass out of... you know.
>
> Actually I didn't understand the part about unconfiguration since I
> can't see how that would happen during normal usage?  Anyway,
> unconfiguring also works for me:
>
> canardo:/tmp# hciconfig hci0
> hci0:   Type: Primary  Bus: USB
>         BD Address: 00:1A:7D:DA:71:15  ACL MTU: 310:10  SCO MTU: 64:8
>         UP RUNNING
>         RX bytes:660 acl:0 sco:0 events:43 errors:0
>         TX bytes:2178 acl:0 sco:0 commands:43 errors:0
>
> canardo:/tmp# echo 0 >/sys/bus/usb/devices/1-8/bConfigurationValue
> canardo:/tmp# hciconfig hci0
> Can't get device info: No such device
> canardo:/tmp# echo 1 >/sys/bus/usb/devices/1-8/bConfigurationValue
>
>
> Not entirely sure how to validate that "everything" works at this point?
> I can use the rfcomm session the adapter usually handles and also run
> lescan after this.  So I guess both BLE and BDR works.
>
> And the counters count something:
>
> canardo:/tmp# hciconfig hci0
> hci0:   Type: Primary  Bus: USB
>         BD Address: 00:1A:7D:DA:71:15  ACL MTU: 310:10  SCO MTU: 64:8
>         UP RUNNING
>         RX bytes:3883 acl:40 sco:0 events:81 errors:0
>         TX bytes:2518 acl:19 sco:0 commands:50 errors:0
>
>
> I can see an error like this logged every time I unconfigure the device:
>
> Bluetooth: hci0: urb 00000000e66a2492 failed to resubmit (2)
>
> There is nothing else logged in kernel log
>
>
> Bjørn
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] USB: core: skip unconfiguration if device doesn't support it
  2022-05-05 15:22         ` Bjørn Mork
  2022-05-05 16:48           ` Jose Ignacio Tornos Martinez
@ 2022-05-05 17:04           ` Alan Stern
  1 sibling, 0 replies; 12+ messages in thread
From: Alan Stern @ 2022-05-05 17:04 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: Oliver Neukum, Marcel Holtmann, Jose Ignacio Tornos Martinez,
	gregkh, linux-usb

On Thu, May 05, 2022 at 05:22:45PM +0200, Bjørn Mork wrote:
> Alan Stern <stern@rowland.harvard.edu> writes:
> 
> > Please pardon me for butting in, but I don't see how this tests the 
> > condition that Jose is worried about.
> ..
> > And presumably the device is working again.  However, none of this shows 
> > what happens when the device is unconfigured.  To test that, you would 
> > have to do:
> >
> > 	echo 0 >/sys/bus/usb/devices/1-8/bConfigurationValue
> > 	echo 1 >/sys/bus/usb/devices/1-8/bConfigurationValue
> >
> > and then see if the device continues to work.
> 
> Ah, sorry. Scanned briefly, saw "bind", and assumed too much.  Making an
> ass out of... you know.
> 
> Actually I didn't understand the part about unconfiguration since I
> can't see how that would happen during normal usage?  Anyway,

Jose was exporting his device over usbip, which perhaps does not count 
as "normal" usage.

> unconfiguring also works for me:
> 
> canardo:/tmp# hciconfig hci0
> hci0:   Type: Primary  Bus: USB
>         BD Address: 00:1A:7D:DA:71:15  ACL MTU: 310:10  SCO MTU: 64:8
>         UP RUNNING 
>         RX bytes:660 acl:0 sco:0 events:43 errors:0
>         TX bytes:2178 acl:0 sco:0 commands:43 errors:0
> 
> canardo:/tmp# echo 0 >/sys/bus/usb/devices/1-8/bConfigurationValue
> canardo:/tmp# hciconfig hci0
> Can't get device info: No such device
> canardo:/tmp# echo 1 >/sys/bus/usb/devices/1-8/bConfigurationValue
> 
> 
> Not entirely sure how to validate that "everything" works at this point?

If it weren't, you would know.  Error messages would have shown up at 
this point.

> I can use the rfcomm session the adapter usually handles and also run
> lescan after this.  So I guess both BLE and BDR works.
> 
> And the counters count something:
> 
> canardo:/tmp# hciconfig hci0
> hci0:   Type: Primary  Bus: USB
>         BD Address: 00:1A:7D:DA:71:15  ACL MTU: 310:10  SCO MTU: 64:8
>         UP RUNNING 
>         RX bytes:3883 acl:40 sco:0 events:81 errors:0
>         TX bytes:2518 acl:19 sco:0 commands:50 errors:0
> 
> 
> I can see an error like this logged every time I unconfigure the device:
> 
> Bluetooth: hci0: urb 00000000e66a2492 failed to resubmit (2)

That's normal.  The driver probably could avoid giving that error 
message under these circumstances, but it's not a big deal.

> There is nothing else logged in kernel log

Okay, thanks for testing.

Alan Stern

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-05-05 17:04 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-04  8:36 [PATCH v4] USB: core: skip unconfiguration if device doesn't support it Jose Ignacio Tornos Martinez
2022-05-04 12:23 ` Marcel Holtmann
2022-05-04 12:32   ` Jose Ignacio Tornos Martinez
2022-05-05 10:19   ` Oliver Neukum
2022-05-05 11:15     ` Jose Ignacio Tornos Martinez
2022-05-05 12:06       ` Oliver Neukum
2022-05-05 12:35         ` Jose Ignacio Tornos Martinez
2022-05-05 11:29     ` Bjørn Mork
2022-05-05 14:14       ` Alan Stern
2022-05-05 15:22         ` Bjørn Mork
2022-05-05 16:48           ` Jose Ignacio Tornos Martinez
2022-05-05 17:04           ` Alan Stern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).