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

[Takashi Iwai <tiwai@suse.de>]

On Mon, 14 Sep 2026 06:00:04 +0200,
Hanh Kieu wrote:
> 
> 
> Hi,
> Following up, anything needed on my end?

Now I took to for-next branch.  Thanks.


Takashi

> - Hanh Kieu 
> 
> On Mon, Sep 7, 2026 at 11:33 PM Takashi Iwai <tiwai@suse.de> wrote:
> 
>     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
>