Re: [PATCH v4 0/1] ALSA: usb: Add support for Reloop Jockey 3 DJ controllers

[Takashi Iwai <tiwai@suse.de>]

On Mon, 28 Sep 2026 18:25:05 +0200,
Takashi Iwai wrote:
> 
> On Fri, 25 Sep 2026 04:19:13 +0200,
> Frank van de Pol wrote:
> > 
> > Hi Takashi, Jaroslav, and the ALSA community,
> > 
> > Apologies for the long gap since v3. This is not simply v3 with the review
> > comments addressed -- I went back to the reverse engineering, and a fair
> > amount of what came out of it changed the driver structurally.
> > 
> > The patch has grown accordingly: v3 was 8 files and 1613 insertions, this is
> > 17 and 7326.  Most of that is not new logic: of the roughly 5700 added
> > lines, about 2100 are comments, 1660 are KUnit tests and generated test
> > vectors for the codec, 249 are the new jockey3.rst documentation, and only
> > about 1300 are executable code.  The rest is blank lines and Kconfig
> > plumbing.
> > 
> > The relatively large number of comments comes from the deliberate decision
> > to include what I learned from the protocol analysis with the relevant
> > section of code.  The parts that most need explaining (the bit-plane wire
> > format, device initialization, why the URBs run for the device's whole
> > lifetime, the locking order etc.) cannot be verified by reading the code
> > alone.  ploytec_codec.c is over half comment for that reason: the DOC: block
> > deriving the format from the board's I2S and DMA geometry is effectively the
> > specification the optimized variants implement.
> > 
> > Previous version:
> > v3: https://lore.kernel.org/all/20260622011131.1748298-1-fvdpol@gmail.com/
> > 
> > Changes v3 -> v4:
> > 
> >  - Reworked the locking architecture around an explicit hierarchy: a
> >    process-context rate_mutex outermost, then IRQ-safe leaf spinlocks for
> >    playback, capture and MIDI that are never nested in one another.
> >    Documented in jockey3.c.
> > 
> >  - Fixed the capture stall after a sample-rate change, eliminating the USB
> >    device reset that recovery used to require.  The reset rate per rate
> >    change went from 19.3% to zero, with a 98k clean streak on arm64 and 61k
> >    on x86_64.
> > 
> >  - Rewrote the bit-plane codec: a portable reference implementation plus
> >    32-bit and 64-bit SWAR variants selected at compile time, called through a
> >    batch API at the driver's real batch sizes. Against the same machine's
> >    reference build, the optimized path measures 11.0x encode / 7.8x decode on
> >    x86_64 and 6.9x / 6.1x on arm64. KUnit tests validate all variants against
> >    an independently derived model of the wire format.
> > 
> >  - Added URB coalescing.  The driver used to submit one 512-byte packet per
> >    URB, fixing the completion rate at 9923/s at 44.1 kHz and 21600/s at 96
> >    kHz for as long as the device was plugged in, used or not.  N packets per
> >    URB (N a power of two, 1 to 8) are now chosen per PCM open from the
> >    requested period size.
> > 
> >  - Fixed a cold-boot initialization race. Straight after power-on the device
> >    accepts the entire init sequence, reports success on every transfer, takes
> >    playback samples, but its audio engine never starts. Bisected over 100
> >    cold boots to between 144 and 156 ms after enumeration; the driver now
> >    waits 250 ms before its first control transfer, close to the 297-308 ms
> >    the Windows driver waits.
> > 
> >  - Added a URB liveness watchdog. Every error path hangs off a URB
> >    completion, so a device that stops completing URBs produces no error, no
> >    xrun and no log line. A work item now reports either direction silent for
> >    20 ms and enters the recovery ladder on a stall onset.
> > 
> >  - Hardened handling of an unresponsive device: an EP0 error-class predicate
> >    aborts the handshake before usb_set_interface() is reached, since the USB
> >    core disables an interface's endpoints as its first action and does not
> >    re-enable them on the failure return.
> > 
> >  - Added Documentation/sound/cards/jockey3.rst
> > 
> >  - Addressed the v3 review comments.
> > 
> > 
> > Testing:
> > 
> > Validated with an automated hardware-in-the-loop framework written for the
> > purpose: probe and unbind endurance, PCM cycling, every rate in both
> > directions, period and buffer boundary matrices, duplex, xrun injection,
> > rate-change soaks, suspend/resume, USB disconnects via hub port power
> > switching, and cold-boot cycling through a network-controlled mains switch. 
> > Some of the figures above come from runs of tens of thousands of iterations.
> > 
> > Exercised on real hardware on x86_64, i386, arm64 (Raspberry Pi 4) and
> > armv6/armhf (Raspberry Pi 1B -- functional but tight at 88.2/96 kHz), with
> > x86_64 and arm64 also run under a KASAN + lockdep debug kernel.  The codec
> > KUnit suite additionally tested under UML and QEMU on i386, arm, arm64,
> > riscv and s390 (test big/little endianness).
> > 
> > 
> > Two things I would rather state than have found:
> > 
> >  - The mitigation stack around the rate-change stall is now practically
> >    dormant since the rate-change improvement.  I kept it as cheap insurance;
> >    happy to remove it.
> > 
> >  - The 250 ms settling delay in jockey3_initialize() runs on the
> >    USB hub thread.  I can move initialization off the probe path if you
> >    prefer, though that introduces complexity to avoid races.
> > 
> > 
> > I look forward to your review/feedback.
> > 
> > Best regards,
> > Frank
> > 
> > Frank van de Pol (1):
> >   ALSA: usb: Add support for Reloop Jockey 3 DJ controllers
> > 
> >  Documentation/sound/cards/index.rst           |    1 +
> >  Documentation/sound/cards/jockey3.rst         |  249 +
> >  MAINTAINERS                                   |    8 +
> >  sound/usb/Kconfig                             |    1 +
> >  sound/usb/Makefile                            |    1 +
> >  sound/usb/jockey3/.kunitconfig                |   13 +
> >  sound/usb/jockey3/Kconfig                     |   60 +
> >  sound/usb/jockey3/Makefile                    |    7 +
> >  sound/usb/jockey3/jockey3.c                   | 4060 +++++++++++++++++
> >  sound/usb/jockey3/ploytec_codec.c             |  656 +++
> >  sound/usb/jockey3/ploytec_codec.h             |   44 +
> >  sound/usb/jockey3/ploytec_codec_kunit.c       |  871 ++++
> >  .../usb/jockey3/ploytec_codec_test_vectors.h  |  793 ++++
> >  sound/usb/jockey3/ploytec_midi.c              |   83 +
> >  sound/usb/jockey3/ploytec_midi.h              |   36 +
> >  sound/usb/jockey3/ploytec_proto.c             |  365 ++
> >  sound/usb/jockey3/ploytec_proto.h             |   78 +
> >  17 files changed, 7326 insertions(+)
> >  create mode 100644 Documentation/sound/cards/jockey3.rst
> >  create mode 100644 sound/usb/jockey3/.kunitconfig
> >  create mode 100644 sound/usb/jockey3/Kconfig
> >  create mode 100644 sound/usb/jockey3/Makefile
> >  create mode 100644 sound/usb/jockey3/jockey3.c
> >  create mode 100644 sound/usb/jockey3/ploytec_codec.c
> >  create mode 100644 sound/usb/jockey3/ploytec_codec.h
> >  create mode 100644 sound/usb/jockey3/ploytec_codec_kunit.c
> >  create mode 100644 sound/usb/jockey3/ploytec_codec_test_vectors.h
> >  create mode 100644 sound/usb/jockey3/ploytec_midi.c
> >  create mode 100644 sound/usb/jockey3/ploytec_midi.h
> >  create mode 100644 sound/usb/jockey3/ploytec_proto.c
> >  create mode 100644 sound/usb/jockey3/ploytec_proto.h
> 
> Thanks for the update.  I think overall the code would be OK, but
> the problem is that it's a way too big single patch to digest.
> 
> Could you try to instruct LLM for splitting logically for easier
> reviews?  It could be like starting with the core probe skeleton,
> polytec core stuff, basic PCM loops, the mixer support, the MIDI
> support, kunit, documentation, something else I forgot, etc.

And, you don't have to list up all models you've used in Assisted-by
tag; just put a single "Assisted-by: LLM" should suffice (and even
preferred).


Takashi