From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.223.130]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CDB5E4E36ED; Mon, 28 Sep 2026 16:25:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=195.135.223.130 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790612718; cv=none; b=SD9BazptQkS9jPVaLEHSNddHwzpeG6eyoveVEOQhfzxyV563Vg9eiALSsk2ozHrV7svrOs83FLlXGg/Ii+3hyLixoOZ1tluWIwJlyQ9qVX277Zkw9xYoOCVMMpni900ED6Jh6Bh6AlE87hwJE5Sjktol7HixQ/GwiDFfv5VFd5g= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790612718; c=relaxed/simple; bh=IeXHQDuG+UlSTHrfck/KhKvmZ0Y8Fcngu2CJvdQNwxU=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=cNAifyXls5qHdqXtp4wxHwoeg/3veux3OWpPHnU2FxMGg8EwGs3kU+0tiFUiEryXLEbaiLVimQ0nNHyw4+XlCsKt+kfvifEA3LuF4eDiWCnkYAMbD49FdLD3M3E41kPIXMe5B3lSG1V9mSq7xfKe4m+mNILX3RajEgE1UUVIE0o= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=suse.de; spf=pass smtp.mailfrom=suse.de; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b=H2WJ+rIR; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b=RpST6PYQ; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b=1WjzO2fT; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b=gw3poFN3; arc=none smtp.client-ip=195.135.223.130 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="H2WJ+rIR"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="RpST6PYQ"; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="1WjzO2fT"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="gw3poFN3" Received: from imap1.dmz-prg2.suse.org (unknown [10.150.64.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 7C61A21D59; Mon, 28 Sep 2026 16:25:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1790612710; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=MUzOaX8sf87HxifU7reLT1NQboWhipfuOHdlKqrfpHE=; b=H2WJ+rIRKkUYdsmM2JwBzqizVpVBBhyqQ+2bPRdgSUcE5FNOSCGZkwIARZUDStw/h1bU33 0vKxkEZ497ACbK4i4QCnsdd9pS7aPinm/CKCLXjG401MIiqL2O1XnZ4+n5RP8lsXvCa4Vn lRHdeF4KqiajWXsVvorE9TkLDdZyPQ4= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1790612710; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=MUzOaX8sf87HxifU7reLT1NQboWhipfuOHdlKqrfpHE=; b=RpST6PYQMSVpqo0VXsT56G5ijoG/pNR/iABjpqe7wlLau9L1KnPIipZFkheOnfWCaL6T2W mJc1Q5SiN14SbkBA== Authentication-Results: smtp-out1.suse.de; none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1790612706; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=MUzOaX8sf87HxifU7reLT1NQboWhipfuOHdlKqrfpHE=; b=1WjzO2fTMSuLHwiXvYhdYmjcHovUgPBHlbY8M43CBH/sdDKBRkhPV6JQU97KjkGyniOGV5 w655unPL8oPF1JvBLCsuJI2gRWW8b4L0GJaE5SNO86vmmzf4Aah05GrzQT6SlU72+BasYb a/NwCepPqU7TsHZNeTX/o2WpduIH3zQ= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1790612706; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=MUzOaX8sf87HxifU7reLT1NQboWhipfuOHdlKqrfpHE=; b=gw3poFN3KOKxuSGssviSZHD4S+PJzn4CXRDaEMhSqmzC3/VUYo8GlTMRWFTljwVkwlzU/m Tv5PKfl1y4mmhnAw== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id F0C13133F1; Mon, 28 Sep 2026 16:25:05 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id wQ+tL+GUumrTdwAAD6G6ig (envelope-from ); Mon, 28 Sep 2026 16:25:05 +0000 Date: Mon, 28 Sep 2026 18:25:05 +0200 Message-ID: <875wzp4a3i.wl-tiwai@suse.de> From: Takashi Iwai To: Frank van de Pol Cc: tiwai@suse.de, perex@perex.cz, corbet@lwn.net, khan@linuxfoundation.org, rdunlap@infradead.org, linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 0/1] ALSA: usb: Add support for Reloop Jockey 3 DJ controllers In-Reply-To: <20260925021915.78909-1-fvdpol@gmail.com> References: <20260925021915.78909-1-fvdpol@gmail.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/30.2 Mule/6.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-Spam-Level: X-Spam-Score: -3.30 X-Spam-Flag: NO X-Spamd-Result: default: False [-3.30 / 50.00]; BAYES_HAM(-3.00)[100.00%]; NEURAL_HAM_LONG(-1.00)[-1.000]; MID_CONTAINS_FROM(1.00)[]; NEURAL_HAM_SHORT(-0.20)[-0.999]; MIME_GOOD(-0.10)[text/plain]; RCPT_COUNT_SEVEN(0.00)[8]; MIME_TRACE(0.00)[0:+]; RCVD_VIA_SMTP_AUTH(0.00)[]; ARC_NA(0.00)[]; FREEMAIL_TO(0.00)[gmail.com]; FREEMAIL_ENVRCPT(0.00)[gmail.com]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; TO_MATCH_ENVRCPT_ALL(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; FROM_EQ_ENVFROM(0.00)[]; RCVD_COUNT_TWO(0.00)[2]; RCVD_TLS_ALL(0.00)[]; DBL_BLOCKED_OPENRESOLVER(0.00)[suse.de:mid,imap1.dmz-prg2.suse.org:helo] 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. Takashi