On Sun, 24 Jan 2016 17:36:17 +0100,
Guillaume Fougnies wrote:
>
>
> Hello,
>
> This is a small patch against sound/usb/quirks.c to correct "Teac
> Corp." products delay.
> Tested on Teac UD-501 and UD-503 products, could not harm other
> Teac products.
> It corrects the "clock source 41 is not valid, cannot use" error
> for both devices.
> And more critically on the new UD-503, a complete freeze of the
> teac usb interface when switching between different rate/format.
> (Freeze only solved by a power switch of the device...)
>
> The patch matches the problem of the "Playback Design" products. I just
> refactored a bit the code to include Teac products properly.
The changes look almost good to me. Could you fix small nitpick below
and resubmit with a proper changelog, and most importantly, with your
sign-off, in order to merge to upstream?
>
> Regards,
> Guillaume
>
>
> ---
> sound/usb/quirks.c | 58 +++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 33 insertions(+), 25 deletions(-)
>
> diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
> index 23ea6d8..f3faaef 100644
> --- a/sound/usb/quirks.c
> +++ b/sound/usb/quirks.c
> @@ -1202,40 +1202,48 @@ void snd_usb_endpoint_start_quirk(struct snd_usb_endpoint *ep)
> void snd_usb_set_interface_quirk(struct usb_device *dev)
> {
> /*
> - * "Playback Design" products need a 50ms delay after setting the
> + * Products needing a 50ms delay after setting the
> * USB interface.
> */
> - if (le16_to_cpu(dev->descriptor.idVendor) == 0x23ba)
> - mdelay(50);
> + switch (le16_to_cpu(dev->descriptor.idVendor)) {
> + case 0x23ba: /* Playback Design */
> + case 0x0644: /* TEAC Corp. */
In the kernel code, case is has the same indent level as switch.
> + mdelay(50);
> + }
Put break after mdelay() call as the common practice.
> }
>
> void snd_usb_ctl_msg_quirk(struct usb_device *dev, unsigned int pipe,
> __u8 request, __u8 requesttype, __u16 value,
> __u16 index, void *data, __u16 size)
> {
> - /*
> - * "Playback Design" products need a 20ms delay after each
> - * class compliant request
> - */
> - if ((le16_to_cpu(dev->descriptor.idVendor) == 0x23ba) &&
> - (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS)
> - mdelay(20);
> -
> - /* Marantz/Denon devices with USB DAC functionality need a delay
> - * after each class compliant request
> - */
> - if (is_marantz_denon_dac(USB_ID(le16_to_cpu(dev->descriptor.idVendor),
> - le16_to_cpu(dev->descriptor.idProduct)))
> - && (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS)
> - mdelay(20);
> + if ((requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS) {
> + switch (le16_to_cpu(dev->descriptor.idVendor)) {
> + /*
> + * "Playback Design"/"TEAC Corp." products need a 20ms delay after each
> + * class compliant request
> + */
> + case 0x23ba: /* "Playback Design" */
> + case 0x0644: /* TEAC Corp. */
> + mdelay(20);
> + return;
> +
> + /* Zoom R16/24 needs a tiny delay here, otherwise requests like
> + * get/set frequency return as failed despite actually succeeding.
> + */
> + case 0x1686:
> + /* restrict to 0x00dd */
> + if (le16_to_cpu(dev->descriptor.idProduct) == 0x00dd)
> + mdelay(1);
> + return;
> + }
>
> - /* Zoom R16/24 needs a tiny delay here, otherwise requests like
> - * get/set frequency return as failed despite actually succeeding.
> - */
> - if ((le16_to_cpu(dev->descriptor.idVendor) == 0x1686) &&
> - (le16_to_cpu(dev->descriptor.idProduct) == 0x00dd) &&
> - (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS)
> - mdelay(1);
> + /* Marantz/Denon devices with USB DAC functionality need a delay
> + * after each class compliant request
> + */
> + if (is_marantz_denon_dac(USB_ID(le16_to_cpu(dev->descriptor.idVendor),
> + le16_to_cpu(dev->descriptor.idProduct))))
> + mdelay(20);
> + }
In this case, I prefer keeping the old code as is.
Just add the check for your device there in a new if block.
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
