Re: [PATCH 3/3] ALSA: usb-audio: Properly match with audio interface class

From: Alexander Tsoy <alexander@tsoy.me>
To: Takashi Iwai <tiwai@suse.de>
Cc: alsa-devel@alsa-project.org
Subject: Re: [PATCH 3/3] ALSA: usb-audio: Properly match with audio interface class
Date: Sun, 23 Aug 2020 23:52:43 +0300	[thread overview]
Message-ID: <713cb4b6b3999fb00781e73bbbe844abe9e0ac83.camel@tsoy.me> (raw)
In-Reply-To: <s5hft8da9eh.wl-tiwai@suse.de>

В Вс, 23/08/2020 в 13:28 +0200, Takashi Iwai пишет:
> On Sat, 22 Aug 2020 18:05:29 +0200,
> Takashi Iwai wrote:
> > On Sat, 22 Aug 2020 17:27:35 +0200,
> > Alexander Tsoy wrote:
> > > В Пн, 17/08/2020 в 10:21 +0200, Takashi Iwai пишет:
> > > > There are a few entries in the quirk table that set the device
> > > > ID
> > > > with
> > > > USB_DEVICE() macro while having an extra bInterfaceClass
> > > > field.  But
> > > > bInterfaceClass field is never checked unless the proper
> > > > match_flags
> > > > is set, so those may match incorrectly with all interfaces.
> > > > 
> > > > Introduce another macro to match with the vid/pid pair and the
> > > > audio
> > > > class interface, and apply it to such entries, so that they can
> > > > match
> > > > properly.
> > > > 
> > > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > > > ---
> > > >  sound/usb/quirks-table.h | 55 +++++++++++++++++---------------
> > > > ------
> > > > ----------
> > > >  1 file changed, 19 insertions(+), 36 deletions(-)
> > > > 
> > > > diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-
> > > > table.h
> > > > index 988bb9d00192..7a80ef31bbe4 100644
> > > > --- a/sound/usb/quirks-table.h
> > > > +++ b/sound/usb/quirks-table.h
> > > > @@ -35,6 +35,14 @@
> > > >  	.bInterfaceClass = USB_CLASS_AUDIO, \
> > > >  	.bInterfaceSubClass = USB_SUBCLASS_AUDIOCONTROL
> > > >  
> > > > +/* Another standard entry matching with vid/pid and the audio
> > > > class
> > > > */
> > > > +#define USB_AUDIO_CLASS(vend, prod) \
> > > > +	.match_flags = USB_DEVICE_ID_MATCH_DEVICE | \
> > > > +		       USB_DEVICE_ID_MATCH_INT_CLASS, \
> > > > +	.idVendor = vend, \
> > > > +	.idProduct = prod, \
> > > > +	.bInterfaceClass = USB_CLASS_AUDIO
> > > > +
> > > >  /* FTDI devices */
> > > >  {
> > > >  	USB_DEVICE(0x0403, 0xb8d8),
> > > > @@ -68,34 +76,14 @@
> > > >  	}
> > > >  },
> > > >  
> > > > -{
> > > > -	/* E-Mu 0202 USB */
> > > > -	.match_flags = USB_DEVICE_ID_MATCH_DEVICE,
> > > > -	.idVendor = 0x041e,
> > > > -	.idProduct = 0x3f02,
> > > > -	.bInterfaceClass = USB_CLASS_AUDIO,
> > > > -},
> > > > -{
> > > > -	/* E-Mu 0404 USB */
> > > > -	.match_flags = USB_DEVICE_ID_MATCH_DEVICE,
> > > > -	.idVendor = 0x041e,
> > > > -	.idProduct = 0x3f04,
> > > > -	.bInterfaceClass = USB_CLASS_AUDIO,
> > > > -},
> > > > -{
> > > > -	/* E-Mu Tracker Pre */
> > > > -	.match_flags = USB_DEVICE_ID_MATCH_DEVICE,
> > > > -	.idVendor = 0x041e,
> > > > -	.idProduct = 0x3f0a,
> > > > -	.bInterfaceClass = USB_CLASS_AUDIO,
> > > > -},
> > > > -{
> > > > -	/* E-Mu 0204 USB */
> > > > -	.match_flags = USB_DEVICE_ID_MATCH_DEVICE,
> > > > -	.idVendor = 0x041e,
> > > > -	.idProduct = 0x3f19,
> > > > -	.bInterfaceClass = USB_CLASS_AUDIO,
> > > > -},
> > > > +/* E-Mu 0202 USB */
> > > > +{ USB_AUDIO_CLASS(0x041e, 0x3f02) },
> > > > +/* E-Mu 0404 USB */
> > > > +{ USB_AUDIO_CLASS(0x041e, 0x3f04) },
> > > > +/* E-Mu Tracker Pre */
> > > > +{ USB_AUDIO_CLASS(0x041e, 0x3f0a) },
> > > > +/* E-Mu 0204 USB */
> > > > +{ USB_AUDIO_CLASS(0x041e, 0x3f19) },
> > > >  
> > > >  /*
> > > >   * HP Wireless Audio
> > > > @@ -2751,10 +2739,7 @@ YAMAHA_DEVICE(0x7010, "UB99"),
> > > >  },
> > > >  
> > > >  /* KeithMcMillen Stringport */
> > > > -{
> > > > -	USB_DEVICE(0x1f38, 0x0001),
> > > > -	.bInterfaceClass = USB_CLASS_AUDIO,
> > > > -},
> > > > +{ USB_AUDIO_CLASS(0x1f38, 0x0001) },
> > > >  
> > > >  /* Miditech devices */
> > > >  {
> > > > @@ -2977,10 +2962,7 @@ AU0828_DEVICE(0x2040, 0x7270,
> > > > "Hauppauge",
> > > > "HVR-950Q"),
> > > >  },
> > > >  {
> > > >  	/* Tascam US122 MKII - playback-only support */
> > > > -	.match_flags = USB_DEVICE_ID_MATCH_DEVICE,
> > > > -	.idVendor = 0x0644,
> > > > -	.idProduct = 0x8021,
> > > > -	.bInterfaceClass = USB_CLASS_AUDIO,
> > > > +	USB_AUDIO_CLASS(0x0644, 0x8021),
> > > >  	.driver_info = (unsigned long) &(const struct
> > > > snd_usb_audio_quirk) {
> > > >  		.vendor_name = "TASCAM",
> > > >  		.product_name = "US122 MKII",
> > > > @@ -3612,3 +3594,4 @@ AU0828_DEVICE(0x2040, 0x7270,
> > > > "Hauppauge",
> > > > "HVR-950Q"),
> > > >  
> > > >  #undef USB_DEVICE_VENDOR_SPEC
> > > >  #undef USB_AUDIO_DEVICE
> > > > +#undef USB_AUDIO_CLASS
> > > 
> > > I don't know anything about KeithMcMillen Stringport, but all
> > > other
> > > devices (US122 MKII and E-mu) seems to have Vendor-specific
> > > Class.
> > 
> > OK, then it must be with USB_DEVICE_VENDOR_SPEC macro instead
> > although
> > they had .bInterfaceClass = USB_CLASS_AUDIO line wrongly.
> > Could you submit the correction patch, or shall I fix in my side?
> 
> Never mind, I prepared a patch by myself.  Will submit now.

Thank you! Yes, I think entries in quirks-table were incorrect. Here
are some proofs:

- E-Mu 0202 USB:
https://github.com/linuxhw/LsUSB/blob/master/Desktop/ASUSTek%20Computer/All/All%20Series/EB6955AE5064/ROSA-2016.1/4.9.20-NRJ-DESKTOP-1ROSA-X86_64/X86_64/7339AF533B#L1073
- E-Mu 0404 USB:
https://github.com/linuxhw/LsUSB/blob/master/Desktop/ASUSTek%20Computer/M5A97/M5A97%20R2.0/4BA2C16B387E/ROSA-2014.1/4.1.15-NRJ-DESKTOP-1ROSA-X86_64/X86_64/E08FEFA436#L997
- E-Mu 0204 USB:
https://github.com/linuxhw/LsUSB/blob/master/Desktop/ASUSTek%20Computer/P5/P5LD2-SE/D614D36EB275/ROSA-2014.1/4.1.15-NRJ-DESKTOP-1ROSA-X86_64/X86_64/09F4577139#L782
- E-MU Tracker Pre:
https://github.com/linuxhw/LsUSB/blob/master/Desktop/ASRock/H61/H61M-HVGS/AD71CCCE2E3F/ROSA-2014.1/3.14.44-NRJ-DESKTOP-2ROSA-X86_64/X86_64/6996B0918E#L2
- TASCAM US-122mkII
https://github.com/linuxhw/LsUSB/blob/master/Desktop/Intel/WX58BP/WX58BP%20AAE64643-203/2B1109DE1986/ROSA-2014.1/4.1.22-NRJ-DESKTOP-2ROSA-X86_64/X86_64/0E7E6022A3#L421