Re: [alsa-devel] [PATCH] ALSA: usb-audio: Disable quirks for BOSS Katana amplifiers

On Wed, 14 Oct 2020 15:33:03 +0200, Mailing Lists wrote:

Thanks for the response Takashi,

How should this be distinguishing between Roland and BOSS? They both have the vendor ID 0x0582.

Ah, right, I missed that point :-<

So the question would be rather how to detect BOSS devices effectively...

thanks,

Takashi

Cheers,

Keith

On Wed, 14 Oct 2020 at 14:09, Takashi Iwai tiwai@suse.de wrote:

On Wed, 14 Oct 2020 14:17:35 +0200,
Mailing Lists wrote:
>
> Following up on this, it appears there are a bunch of the
newer-generation
> Roland/Boss devices which need similar treatment.
>
> So far I have tested the GT-1, the GT-001, and the BR-80, and others
have
> reported the RC-300 as working with similar modifications. I have been
using
> the following change to the code in pcm.c set_sync_ep_implicit_fb_quirk:
>
>     case USB_ID(0x0582, 0x01d8): /* BOSS Katana */
>     case USB_ID(0x0582, 0x0130): /* BOSS Micro BR-80 */
>     case USB_ID(0x0582, 0x0138): /* BOSS RC-300 */
>     case USB_ID(0x0582, 0x01d6): /* BOSS GT-1 */
>     case USB_ID(0x0582, 0x01e5): /* BOSS GT-001 */
> /* BOSS Katana amplifiers and many other newer BOSS devices do not need
quirks
> */
>
> There's probably others too, such as the GT-100 (I believe the GT-001
and
> GT-100 have similar hardware).
>
> My question is, should this just be submitted as a patch to pcm.c or
would it
> be better handled in quirks and, if so, how?
>
> Or something else?

Do we really need this change at all?  I looked at the code again, and
I noticed that basically the function should return 0 without setting
anything else even if you don't have the explicit ID checks there.

The function looks like:

static int set_sync_ep_implicit_fb_quirk(struct snd_usb_substream *subs,
                                         struct usb_device *dev,
                                         struct usb_interface_descriptor
*altsd,
                                         unsigned int attr)
{
        ....
        switch (subs->stream->chip->usb_id) {
        ....
        case USB_ID(0x0582, 0x01d8): /* BOSS Katana */
                /* BOSS Katana amplifiers do not need quirks */
                return 0;
        }

        if (attr == USB_ENDPOINT_SYNC_ASYNC &&
            altsd->bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
            altsd->bInterfaceProtocol == 2 &&
            altsd->bNumEndpoints == 1 &&
            USB_ID_VENDOR(subs->stream->chip->usb_id) == 0x0582 /* Roland
*/ &&
            search_roland_implicit_fb(dev, altsd->bInterfaceNumber + 1,
                                      altsd->bAlternateSetting,
                                      &alts, &ep) >= 0) {
                goto add_sync_ep;
        }

        /* No quirk */
        return 0;

... and the lengthy if-conditions after the switch/case is applied
only for Roland devices, hence it shouldn't influence on BOSS
devices.  After that point, the immediate return with 0, which is the
same as we do in switch/case.  So the explicit check of BOSS devices
there looks superfluous.

thanks,

Takashi

--

Keith A Milner