USB: Add quirk for WORLDE easykey.25 MIDI keyboard
diff mbox series

Message ID 1484937994-8076-1-git-send-email-lukas@oxygene.sk
State New, archived
Headers show
Series
  • USB: Add quirk for WORLDE easykey.25 MIDI keyboard
Related show

Commit Message

Lukáš Lalinský Jan. 20, 2017, 6:46 p.m. UTC
Add a quirk for WORLDE easykey.25 MIDI keyboard (idVendor=0218,
idProduct=0401). The device reports that it has config string
descriptor at index 3, but when the system selects the configuration
and tries to get the description, it returns a -EPROTO error,
the communication restarts and this keeps repeating over and over again.
Not requesting the string descriptor makes the device work correctly.

Relevant info from Wireshark:

[...]

CONFIGURATION DESCRIPTOR
    bLength: 9
    bDescriptorType: 0x02 (CONFIGURATION)
    wTotalLength: 101
    bNumInterfaces: 2
    bConfigurationValue: 1
    iConfiguration: 3
    Configuration bmAttributes: 0xc0  SELF-POWERED  NO REMOTE-WAKEUP
        1... .... = Must be 1: Must be 1 for USB 1.1 and higher
        .1.. .... = Self-Powered: This device is SELF-POWERED
        ..0. .... = Remote Wakeup: This device does NOT support remote wakeup
    bMaxPower: 50  (100mA)

[...]

     45 0.369104       host                  2.38.0                USB      64     GET DESCRIPTOR Request STRING

[...]

URB setup
    bmRequestType: 0x80
        1... .... = Direction: Device-to-host
        .00. .... = Type: Standard (0x00)
        ...0 0000 = Recipient: Device (0x00)
    bRequest: GET DESCRIPTOR (6)
    Descriptor Index: 0x03
    bDescriptorType: 0x03
    Language Id: English (United States) (0x0409)
    wLength: 255

     46 0.369255       2.38.0                host                  USB      64     GET DESCRIPTOR Response STRING[Malformed Packet]

[...]

Frame 46: 64 bytes on wire (512 bits), 64 bytes captured (512 bits) on interface 0
USB URB
    [Source: 2.38.0]
    [Destination: host]
    URB id: 0xffff88021f62d480
    URB type: URB_COMPLETE ('C')
    URB transfer type: URB_CONTROL (0x02)
    Endpoint: 0x80, Direction: IN
    Device: 38
    URB bus id: 2
    Device setup request: not relevant ('-')
    Data: present (0)
    URB sec: 1484896277
    URB usec: 455031
    URB status: Protocol error (-EPROTO) (-71)
    URB length [bytes]: 0
    Data length [bytes]: 0
    [Request in: 45]
    [Time from request: 0.000151000 seconds]
    Unused Setup Header
    Interval: 0
    Start frame: 0
    Copy of Transfer Flags: 0x00000200
    Number of ISO descriptors: 0
[Malformed Packet: USB]
    [Expert Info (Error/Malformed): Malformed Packet (Exception occurred)]
        [Malformed Packet (Exception occurred)]
        [Severity level: Error]
        [Group: Malformed]

Signed-off-by: Lukáš Lalinský <lukas@oxygene.sk>
---
 drivers/usb/core/quirks.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Greg KH Jan. 21, 2017, 9:08 a.m. UTC | #1
On Fri, Jan 20, 2017 at 07:46:34PM +0100, Lukáš Lalinský wrote:
> Add a quirk for WORLDE easykey.25 MIDI keyboard (idVendor=0218,
> idProduct=0401). The device reports that it has config string
> descriptor at index 3, but when the system selects the configuration
> and tries to get the description, it returns a -EPROTO error,
> the communication restarts and this keeps repeating over and over again.
> Not requesting the string descriptor makes the device work correctly.

Always use scripts/get_maintainer.pl to determine who to send patches
to, and what mailing list.  You forgot linux-usb@vger, which I've now
added...

> 
> Relevant info from Wireshark:
> 
> [...]
> 
> CONFIGURATION DESCRIPTOR
>     bLength: 9
>     bDescriptorType: 0x02 (CONFIGURATION)
>     wTotalLength: 101
>     bNumInterfaces: 2
>     bConfigurationValue: 1
>     iConfiguration: 3
>     Configuration bmAttributes: 0xc0  SELF-POWERED  NO REMOTE-WAKEUP
>         1... .... = Must be 1: Must be 1 for USB 1.1 and higher
>         .1.. .... = Self-Powered: This device is SELF-POWERED
>         ..0. .... = Remote Wakeup: This device does NOT support remote wakeup
>     bMaxPower: 50  (100mA)
> 
> [...]
> 
>      45 0.369104       host                  2.38.0                USB      64     GET DESCRIPTOR Request STRING
> 
> [...]
> 
> URB setup
>     bmRequestType: 0x80
>         1... .... = Direction: Device-to-host
>         .00. .... = Type: Standard (0x00)
>         ...0 0000 = Recipient: Device (0x00)
>     bRequest: GET DESCRIPTOR (6)
>     Descriptor Index: 0x03
>     bDescriptorType: 0x03
>     Language Id: English (United States) (0x0409)
>     wLength: 255
> 
>      46 0.369255       2.38.0                host                  USB      64     GET DESCRIPTOR Response STRING[Malformed Packet]
> 
> [...]
> 
> Frame 46: 64 bytes on wire (512 bits), 64 bytes captured (512 bits) on interface 0
> USB URB
>     [Source: 2.38.0]
>     [Destination: host]
>     URB id: 0xffff88021f62d480
>     URB type: URB_COMPLETE ('C')
>     URB transfer type: URB_CONTROL (0x02)
>     Endpoint: 0x80, Direction: IN
>     Device: 38
>     URB bus id: 2
>     Device setup request: not relevant ('-')
>     Data: present (0)
>     URB sec: 1484896277
>     URB usec: 455031
>     URB status: Protocol error (-EPROTO) (-71)
>     URB length [bytes]: 0
>     Data length [bytes]: 0
>     [Request in: 45]
>     [Time from request: 0.000151000 seconds]
>     Unused Setup Header
>     Interval: 0
>     Start frame: 0
>     Copy of Transfer Flags: 0x00000200
>     Number of ISO descriptors: 0
> [Malformed Packet: USB]
>     [Expert Info (Error/Malformed): Malformed Packet (Exception occurred)]
>         [Malformed Packet (Exception occurred)]
>         [Severity level: Error]
>         [Group: Malformed]
> 
> Signed-off-by: Lukáš Lalinský <lukas@oxygene.sk>
> ---
>  drivers/usb/core/quirks.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
> index d2e50a2..24f9f98 100644
> --- a/drivers/usb/core/quirks.c
> +++ b/drivers/usb/core/quirks.c
> @@ -37,6 +37,10 @@ static const struct usb_device_id usb_quirk_list[] = {
>  	/* CBM - Flash disk */
>  	{ USB_DEVICE(0x0204, 0x6025), .driver_info = USB_QUIRK_RESET_RESUME },
>  
> +	/* WORLDE easy key (easykey.25) MIDI controller  */
> +	{ USB_DEVICE(0x0218, 0x0401), .driver_info =
> +			USB_QUIRK_CONFIG_INTF_STRINGS },

That's odd, how does other operating systems handle this device?

thanks,

greg k-h
Lukáš Lalinský Jan. 23, 2017, 6:36 p.m. UTC | #2
On Sat, Jan 21, 2017 at 10:08 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> Always use scripts/get_maintainer.pl to determine who to send patches
> to, and what mailing list.  You forgot linux-usb@vger, which I've now
> added...

I'm sorry about that. I actually did use scripts/get_maintainer.pl,
but it only returned your email address and I added linux-kernel@vger
myself. I guess I ran it with wrong options.

> That's odd, how does other operating systems handle this device?

I'm not sure how realistic this test is, but I ran a Wireshark capture
from Windows 8.1 VM and it seems that for this particular device,
Windows doesn't try to set the current configuration (since there is
only one) and it does not even attempt to get the configuration string
descriptor. I'm not sure if Windows does this in general, but it seems
to work around the problem for this device by not caring about the
configuration string descriptor in the first place.

I have uploaded both captures here -
https://gist.github.com/lalinsky/83148a827d5cd43e79e377d8e1b5ed0d

Lukas
Oliver Neukum Jan. 24, 2017, 7:32 a.m. UTC | #3
Am Montag, den 23.01.2017, 19:36 +0100 schrieb Lukáš Lalinský:
> On Sat, Jan 21, 2017 at 10:08 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > 
> > Always use scripts/get_maintainer.pl to determine who to send
> > patches
> > to, and what mailing list.  You forgot linux-usb@vger, which I've
> > now
> > added...
> 
> I'm sorry about that. I actually did use scripts/get_maintainer.pl,
> but it only returned your email address and I added linux-kernel@vger
> myself. I guess I ran it with wrong options.
> 
> > 
> > That's odd, how does other operating systems handle this device?
> 
> I'm not sure how realistic this test is, but I ran a Wireshark
> capture
> from Windows 8.1 VM and it seems that for this particular device,
> Windows doesn't try to set the current configuration (since there is
> only one) and it does not even attempt to get the configuration
> string
> descriptor. I'm not sure if Windows does this in general, but it
> seems
> to work around the problem for this device by not caring about the
> configuration string descriptor in the first place.
> 
> I have uploaded both captures here -
> https://gist.github.com/lalinsky/83148a827d5cd43e79e377d8e1b5ed0d

Indeed it is does not set a configuration. Either the capture
is incomplete or device and host violate the standard. A device
may be left unconfigured. We need to read the descriptors even if we
see only one configuration to get the power budgeting right.

Does the device work without any .ini file?

	Regards
		Oliver
Lukáš Lalinský Jan. 24, 2017, 7:37 a.m. UTC | #4
On Tue, Jan 24, 2017 at 8:32 AM, Oliver Neukum <oneukum@suse.com> wrote:
> Am Montag, den 23.01.2017, 19:36 +0100 schrieb Lukáš Lalinský:
>> I have uploaded both captures here -
>> https://gist.github.com/lalinsky/83148a827d5cd43e79e377d8e1b5ed0d
>
> Indeed it is does not set a configuration. Either the capture
> is incomplete or device and host violate the standard. A device
> may be left unconfigured.

Is this may or may not? I'm not familiar with USB, so I assumed if
there is only one configuration and there is always one active, it
does not need to be set explicitly because the correct one is already
active.

> We need to read the descriptors even if we
> see only one configuration to get the power budgeting right.

Aren't those in the CONFIGURATION descriptors? Reading the STRING
descriptor is probably only useful if you need to print the
configuration details somewhere.

> Does the device work without any .ini file?

Yes. It's a standard MIDI device, no configuration is required.

Lukas
Oliver Neukum Jan. 24, 2017, 8:30 a.m. UTC | #5
Am Dienstag, den 24.01.2017, 08:37 +0100 schrieb Lukáš Lalinský:
> On Tue, Jan 24, 2017 at 8:32 AM, Oliver Neukum <oneukum@suse.com>
> wrote:
> > 
> > Am Montag, den 23.01.2017, 19:36 +0100 schrieb Lukáš Lalinský:
> > > 
> > > I have uploaded both captures here -
> > > https://gist.github.com/lalinsky/83148a827d5cd43e79e377d8e1b5ed0d
> > 
> > Indeed it is does not set a configuration. Either the capture
> > is incomplete or device and host violate the standard. A device
> > may be left unconfigured.
> 
> Is this may or may not? I'm not familiar with USB, so I assumed if

You can leave a device unconfigured. In that case it stays in the
addressed state, where its power draw is strictly limited.
That is exactly what the kernel does when it is confronted with
a device that would overdraw the power budget.

> there is only one configuration and there is always one active, it
> does not need to be set explicitly because the correct one is already
> active.

No, it is not. Or rather it should not be. Can you please recheck
that you are capturing the whole exchange?

> > We need to read the descriptors even if we
> > see only one configuration to get the power budgeting right.
> 
> Aren't those in the CONFIGURATION descriptors? Reading the STRING

True.

> descriptor is probably only useful if you need to print the
> configuration details somewhere.

Also true.

> > 
> > Does the device work without any .ini file?
> 
> Yes. It's a standard MIDI device, no configuration is required.

So Windows by default does not read string descriptors. That is worth
a thought, but we need to check. Yet I am not sure how useful that is.
The first lsusb would crash the device. We'd need a quirk anyway.

	Regards
		Oliver
Lukáš Lalinský Jan. 24, 2017, 8:31 a.m. UTC | #6
On Tue, Jan 24, 2017 at 8:37 AM, Lukáš Lalinský <lukas@oxygene.sk> wrote:
> On Tue, Jan 24, 2017 at 8:32 AM, Oliver Neukum <oneukum@suse.com> wrote:
>> Am Montag, den 23.01.2017, 19:36 +0100 schrieb Lukáš Lalinský:
>>> I have uploaded both captures here -
>>> https://gist.github.com/lalinsky/83148a827d5cd43e79e377d8e1b5ed0d
>>
>> Indeed it is does not set a configuration. Either the capture
>> is incomplete or device and host violate the standard. A device
>> may be left unconfigured.
>
> Is this may or may not? I'm not familiar with USB, so I assumed if
> there is only one configuration and there is always one active, it
> does not need to be set explicitly because the correct one is already
> active.
>
>> We need to read the descriptors even if we
>> see only one configuration to get the power budgeting right.
>
> Aren't those in the CONFIGURATION descriptors? Reading the STRING
> descriptor is probably only useful if you need to print the
> configuration details somewhere.

I re-ran the capture on a Windows 7 host. The previous capture was
missing data, probably due to interactions of the Linux host and
Windows VM.

https://gist.github.com/lalinsky/2ec7d74b049b448b1f7032d8861ca4a2

It does set the configuration, but does not request the string descriptor.

Lukas

Patch
diff mbox series

diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index d2e50a2..24f9f98 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -37,6 +37,10 @@  static const struct usb_device_id usb_quirk_list[] = {
 	/* CBM - Flash disk */
 	{ USB_DEVICE(0x0204, 0x6025), .driver_info = USB_QUIRK_RESET_RESUME },
 
+	/* WORLDE easy key (easykey.25) MIDI controller  */
+	{ USB_DEVICE(0x0218, 0x0401), .driver_info =
+			USB_QUIRK_CONFIG_INTF_STRINGS },
+
 	/* HP 5300/5370C scanner */
 	{ USB_DEVICE(0x03f0, 0x0701), .driver_info =
 			USB_QUIRK_STRING_FETCH_255 },