Re: [PATCH] ALSA: usb-audio: add Pioneer DJ DDJ-SZ support

[Takashi Iwai <tiwai@suse.de>]

On Tue, 08 Sep 2026 01:47:15 +0200,
Geraldo Nascimento wrote:
> 
> Hi Takashi,
> 
> On Mon, Sep 7, 2026 at 4:57 AM Takashi Iwai <tiwai@suse.de> wrote:
> >
> > On Sat, 05 Sep 2026 08:19:37 +0200,
> > Geraldo Nascimento wrote:
> > >
> > > On Sat, Sep 5, 2026 at 2:35 AM Hanh Kieu <hhkieu@gmail.com> wrote:
> > > >
> > > > Hi Geraldo,
> > > >
> > > > I looked further into the is_pioneer_implicit_fb() check you pointed out.
> > > >
> > > > It seems the DATA case is intentional. In the original Pioneer DJM-900NXS2 work, Fabian Lesniak described the observed behavior as:
> > > >
> > > > “the playback streams also tries to start the capture endpoint as sync source.”
> > > >
> > > > https://gist.github.com/flesniak/074ab23bbc833663b782f44174eae6a4
> > > >
> > > > Takashi Iwai later formalized this in the Pioneer implicit-feedback handling, explicitly accepting either USB_ENDPOINT_USAGE_DATA or USB_ENDPOINT_USAGE_IMPLICIT_FB for the secondary IN endpoint before passing it to add_implicit_fb_sync_ep():
> > > >
> > > > https://github.com/torvalds/linux/commit/167c9dc84ec384c0940359e067301883ad2b42a8
> > > >
> > >
> > > Every other Pioneer device has that USB_ENDPOINT_USAGE_IMPLICIT_FB
> > > on the IN endpoint. Yours would be the first without it in the IN
> > > endpoint. Unless
> > > your hardware doesn't rely on implicit feedback for syncing then there's a good
> > > chance that implicit.c short-circuiting and still picking up implicit
> > > feedback for you
> > > isn't correct, but what you need to do is cool down now and wait for proper
> > > maintainer response which takes time.
> >
> > Accepting both DATA and IMPLICIT_FB usages is intentional, yes.
> > But I don't follow what's the problem there -- in this case, the
> > bmAttributes of the secondary EP shows it's DATA, no?  Then this check
> > must pass.
> 
> If you check the original patch submission by Hanh, you'll see he did not
> bother to include USB_ENDPOINT_USAGE_IMPLICIT_FB as a flag for
> EP attributes, claiming it was unnecessary and the implicit feedback was
> picked-up regardless.
> 
> Hahn already traced the functions return values, he is getting a true value
> out of that function (and thus picking up implicit feedback for his device)
> even though he does not add USB_ENDPOINT_USAGE_IMPLICIT_FB
> to the IN EP Attributes on his original submission.

Ah, OK.

> >
> > The condition is:
> >
> >         if (!usb_endpoint_is_isoc_in(epd) ||
> >             (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) != USB_ENDPOINT_SYNC_ASYNC ||
> >             ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
> >              USB_ENDPOINT_USAGE_DATA &&
> >              (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
> >              USB_ENDPOINT_USAGE_IMPLICIT_FB))
> >                 return false;
> >
> >         return false;
> 
> Yes, and that means when that first inequality is false, C short-circuits the
> logical AND and never checks USB_ENDPOINT_USAGE_IMPLICIT_FB
> like it should.
> 
> >
> > ... and maybe it's better to be rewritten like:
> >
> >         sync = (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE);
> >         usage = (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK);
> >
> >         if (usb_endpoint_is_isoc_in(epd) &&
> >             sync == USB_ENDPOINT_SYNC_ASYNC &&
> >             (usage == USB_ENDPOINT_USAGE_DATA ||
> >              usage == USB_ENDPOINT_USAGE_IMPLICIT_FB))
> >                 return true;
> >
> >         return false;
> 
> But then, again, you would be disregarding the meaning of the flag
> USB_ENDPOINT_USAGE_IMPLICIT_FB and in practice just making
> the first check.

The Pioneer device quirk accepts USAGE_DATA also as an implicit fb
source if it's the secondary EP and ASYNC input, too -- that's the
very intention here because some Pioneer devices use USAGE_DATA
instead of USAGE_IMPLICIT_FB.


Takashi