Re: [PATCH v5 00/13] ALSA: jockey3: Add support for Reloop Jockey 3 DJ controllers

[Takashi Iwai <tiwai@suse.de>]

On Mon, 19 Oct 2026 19:44:32 +0200,
Frank van de Pol wrote:
> 
> Hi Takashi, Jaroslav, and the ALSA community,
> 
> v5 is v4 split into a series, as Takashi asked. While splitting it,
> reviewing each patch on its own turned up a few small opportunities to
> harden the driver further. They are listed below and folded into the
> patch that introduces the code they touch. Apart from those and the
> KUnit fix, the code at the end of the series is the code of v4.
> 
> Link: https://lore.kernel.org/all/20260925021915.78909-1-fvdpol@gmail.com/
> 
> Changes v4 -> v5:
> 
>  - Split the single patch into 13 patches (see below).
>  - Move the KUnit batch-equals-single test buffers to the heap. On the
>    stack they exceeded -Wframe-larger-than on 32-bit (1440 > 1280 bytes
>    on arm-allyesconfig). Reported by the kernel test robot; the fix is in
>    patch #5.
>    Link: https://lore.kernel.org/all/202609260711.YnOvPZGc-lkp@intel.com/
>  - Replace the per-model Assisted-by: lines with a single
>    "Assisted-by: LLM" as suggested.
>  - Probe (patch 1):
>    - initialize fw_version before it is logged;
>    - validate the endpoints on altsetting 1 only, the setting the driver
>      selects;
>    - move the card slot handling out of probe() into helpers;
>    - stop the initialization retry loop as soon as EP0 stops answering,
>      instead of retrying for ~20 s;
>    - free the card with snd_card_free_when_closed() on the probe error
>      path, because snd_card_register() can fail after it has created
>      device nodes.
>  - Ratelimit the short capture URB error (patch 4).
>  - Build the codec lookup tables only once (patch 6).
>  - Every stall recovery now reports an xrun on both directions,
>    including a recovery started from .prepare while the sibling stream
>    is running (patch 8).
>  - .prepare waits for a recovery that is already in flight before it
>    lets a stream start (patch 8).
>  - An idle capture stall no longer sets the watchdog's tick rate
>    (patch 9).
> 
> How the series is split:
> 
>    1  probe skeleton: device IDs, endpoint checks, the Ploytec
>       initialization handshake, a card with no devices
>    2  bulk streams: the three URB rings, running with idle payloads
>    3  PCM playback, with the portable encoder
>    4  PCM capture, with the portable decoder
>    5  KUnit tests for the codec
>    6  optimized 64-bit and 32-bit codec variants
>    7  several packets per URB optimization
>    8  recovery of a stalled stream, and USB reset handling
>    9  watchdog for stalls in a running stream
>   10  sample rate switching
>   11  MIDI
>   12  power management
>   13  documentation
> 
> Takashi's list also named mixer support. The device has no hardware
> mixer or volume controls that the driver could expose, so there is no
> patch for it. The only control elements are the channel maps, which
> arrive with PCM in patches 3 and 4.
> 
> Notes on the split:
> 
>  - Every patch builds on x86_64, i386, arm64 and armhf clean at W=1,
>    and is checkpatch --strict and kernel-doc clean. At W=12 and under
>    sparse the only warnings are shadowing reports caused by kernel
>    macros: the index[] module parameter shadowing the index() builtin,
>    and the __ret locals of the wait_event*() family. The KUnit suite passes at
>    every step from patch 5 on, and from patch 6 on that holds for both
>    the optimized and the portable codec.
>  - From patch 2 on, the driver services all three bulk endpoints.
>    Later patches only change what goes into the packets and what is
>    taken out of them. I did this because the firmware is sensitive to
>    deviations from the vendor sequence, and someone bisecting should be
>    able to boot any patch with the device attached.
>  - The rate is fixed at 44.1 kHz until patch 10.
>  - Two function arguments exist before their second caller, to avoid
>    changing signatures inside the series: bounce_alt0 in
>    ploytec_initialize_device() (patch 1) and cold_init in the rate
>    setting path (patch 2). Both get their other caller in patch 10, and
>    the commit messages say so.
>  - The comment blocks at the top of jockey3.c (device model, resource
>    lifetime, locking) grow with the code they describe.
> 
> Testing:
> 
>  - The series was tested cumulatively, to check that it is bisectable.
>    For each N from 1 to 12, I built the tree with patches 1..N applied,
>    loaded the module on x86_64 with the device attached, and ran the
>    checks that the functionality present at that point allows: a smoke
>    run for patches 1-6, the functional suite for patches 7-12, with an
>    operator for the MIDI cases. At every step the device is detected and
>    bound without errors, and what a step adds works as described in its
>    commit message. Patch 13 is documentation only.
>  - These runs are on the final content of each patch, including the
>    fixes listed above.
> 
> Best regards,
> Frank
> 
> 
> Frank van de Pol (13):
>   ALSA: jockey3: Add probe skeleton for Reloop Jockey 3 DJ controllers
>   ALSA: jockey3: Start the bulk streams
>   ALSA: jockey3: Add PCM playback
>   ALSA: jockey3: Add PCM capture
>   ALSA: jockey3: Add KUnit tests for the Ploytec codec
>   ALSA: jockey3: Add optimized codec variants
>   ALSA: jockey3: Coalesce several packets into one URB
>   ALSA: jockey3: Recover a stalled stream
>   ALSA: jockey3: Add a watchdog for mid-stream stalls
>   ALSA: jockey3: Add sample rate switching
>   ALSA: jockey3: Add MIDI support
>   ALSA: jockey3: Add power management
>   ALSA: jockey3: Add driver documentation

Thanks, this makes it much easier to digest and review.
Now it's already a time for 7.4 merge window, so I'll check the series
after merge window is closed.  Meanwhile, Sashiko already reviewed,
and you can enjoy its complaints :)


Takashi