mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Christian Ruppert <arc@gmx.li>
To: Will Porter <mrwillporter@gmail.com>
Cc: Takashi Iwai <tiwai@suse.com>, Jaroslav Kysela <perex@perex.cz>,
	linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org,
	Neil Andrews <neil@androos.io>, Christian Ruppert <arc@gmx.li>
Subject: [RFC V2 PATCH 00/11] Re: ALSA: usb-audio: Audient iD14 MkI vendor
Date: Mon,  2 Nov 2026 01:32:46 +0100	[thread overview]
Message-ID: <20261102003257.10134-1-arc@gmx.li> (raw)
In-Reply-To: <20261101173557.221869-1-mrwillporter@gmail.com>

Hi Will,

On Sun, Nov 01, 2026 at 11:35:57AM -0600, Will Porter wrote:
> [...]
> 
> 1. Patch 10, cache index
> 
> The cache has hwchannels entries, and the probe fills it by element
> index (cache[i]). But get, put and the INIT_HW write index it by device
> channel:
> [...]

Good catch and thanks for the fix, applied in RFC V2.

> 2. Probe-time USB errors now fail the whole card
> [...]
> Could we keep the card registering after a failed probe transfer, while
> keeping the cache entry invalid until a transfer succeeds?
> 
> To save a round, I wrote it out; the diff is at the end of this mail, on
> top of your series. Please change or drop anything that doesn't fit your
> plans.
> [...]

Fine with me, I suppose the question if individual reads should be allowed
to fail during driver initialization is more philosophical in this case.
I don't see any harm. Patch applied in RFC V2

> 3. Smaller points
> 
> - Patch 05: would sign_extend32(buf, 8 * ctype->len - 1) work in
>   place of the shift pair?
> - Patch 05: the buffers mix u32/s32 with cpu_to_le32()/le32_to_cpu().
>   Would __le32 buffers keep sparse quiet?

OK, these make the code a bit cleaner, added to RFC V2.

> - Patch 05: the transfer buffer is 4 bytes on the stack, but nothing
>   checks the table's len. Could snd_audient_add_ctrl() return -EINVAL
>   unless 1 <= len <= 4?

I don't think this is necessary as it should be rather obvious to everyone
touching the driver that the u32 argument can't carry more than 4 bytes.
Added a comment to the parameter definition.

> - Patch 09: with FORCE_SET, a put of the cached value still writes
>   (good) but also reports a change, so each alsactl restore sends a
>   change event for every such control. Could it write but return 0
>   when the cache already holds the value?

Makes sense. Added to RFC V2 (in patch 10 where the cache is added). Please
double-check if the logic reflects what you had in mind.

> - Patch 11: a device with no matrix list would dereference NULL in the
>   loop. Would a check on info->matrix_names be worth adding?

Makes sense although this could also be held off until somebody actually
adds such a device. Added to RFC V2.

> - Patch 01: FYI, the file header still says "Audient EVO driver for
>   ALSA".

Fixed in RFC V2.

> 4. iD14 tables on top
> 
> I built the iD14 tables on your series, with the cache fix applied.
> On an iD14 MkI they passed control read and write checks, the range
> checks and a loopback test of the matrix crosspoints. The crosspoints
> also kept their values across a module reload and were read back into
> the cache.
> 
> On top of your patches I needed three generic additions:
> 
> - a step size for integer types (res): the iD14 level controls use
>   s16 in 1/256 dB; I expose them in whole-dB steps, matching the
>   volume knob. The control counts steps from min, as the generic USB
>   mixer does with the RANGE resolution;
> - an enumerated type with a value list, plus one extra last item that
>   a get shows for a device value outside the list;
> - a per-device create hook, for the FU 12 control that does not fit
>   the tables (in the same patch as that control).
> 
> The matrix uses your loop as it is: with 2 channels per element and the
> names in the order Mic 1 Main, Mic 1 Cue, Mic 2 Main, ..., element n
> starts at CN 2 * n, which is input * 4 + output on the iD14. Does that
> fit what you had in mind?

Yep, sounds good.

> 5. Next steps
> 
> Would you prefer to incorporate these changes in an RFC v2, or shall I
> fold them into the relevant patches when I carry your series, with
> notes in the commit messages?

I created RFC V2 with your patch and the fixes you suggested. This will
make things easier to review when we submit the everything, compared to
fixes on top of a long series of patches.

Based on the same commit as before and retested on 7.3.1.

  Christian

Christian Ruppert (11):
  ALSA: rename mixer_evo to mixer_audient
  ALSA: mixer_audient: explicitly define full control names
  ALSA: mixer_audient: clarify USB control message definition
  ALSA: mixer_audient: Add length check to USB transfer
  ALSA: mixer_audient: make USB control transfer size configurable
  ALSA: mixer_audient: Allow arbitrary boolean control values
  ALSA: mixer_audient: Add integer range checks
  ALSA: mixer_audient: Add control initialization at probe
  ALSA: mixer_audient: Add readback-free parameter set
  ALSA: mixer_audient: add control value caching
  ALSA: mixer_audient: Create matrix programmatically

 MAINTAINERS               |   7 +-
 sound/usb/Makefile        |   2 +-
 sound/usb/mixer_audient.c | 702 ++++++++++++++++++++++++++++++++++++++
 sound/usb/mixer_audient.h |  12 +
 sound/usb/mixer_evo.c     | 526 ----------------------------
 sound/usb/mixer_evo.h     |  12 -
 sound/usb/mixer_quirks.c  |   4 +-
 7 files changed, 721 insertions(+), 544 deletions(-)
 create mode 100644 sound/usb/mixer_audient.c
 create mode 100644 sound/usb/mixer_audient.h
 delete mode 100644 sound/usb/mixer_evo.c
 delete mode 100644 sound/usb/mixer_evo.h

-- 
2.56.0


  reply	other threads:[~2026-11-02  0:33 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-19 23:01 [PATCH 0/5] ALSA: usb-audio: Audient iD14 MkI vendor controls Will Porter
2026-10-19 23:01 ` [PATCH 1/5] ALSA: usb-audio: Give the Audient iD14 MkI its own mixer map Will Porter
2026-10-19 23:01 ` [PATCH 2/5] ALSA: usb-audio: Add Audient iD14 MkI monitor and routing controls Will Porter
2026-10-19 23:01 ` [PATCH 3/5] ALSA: usb-audio: Add Audient iD14 MkI mixer matrix Will Porter
2026-10-19 23:01 ` [PATCH 4/5] ALSA: usb-audio: Add Audient iD14 MkI write-only controls Will Porter
2026-10-19 23:01 ` [PATCH 5/5] ALSA: usb-audio: Replace the Audient iD14 MkI FU 12 control Will Porter
2026-10-20 13:10 ` [PATCH 0/5] ALSA: usb-audio: Audient iD14 MkI vendor controls Takashi Iwai
2026-10-26 18:36 ` Christian Ruppert
2026-10-26 21:44   ` Will Porter
2026-10-27  6:35     ` Takashi Iwai
2026-10-27 23:21     ` Christian Ruppert
2026-10-28  2:36       ` Will Porter
2026-11-01  0:00         ` [RFC PATCH 00/11] " Christian Ruppert
2026-11-01  0:00           ` [RFC PATCH 01/11] ALSA: rename mixer_evo to mixer_audient Christian Ruppert
2026-11-01  0:00           ` [RFC PATCH 02/11] ALSA: mixer_audient: explicitly define full control names Christian Ruppert
2026-11-01  0:00           ` [RFC PATCH 03/11] ALSA: mixer_audient: clarify USB control message definition Christian Ruppert
2026-11-01  0:00           ` [RFC PATCH 04/11] ALSA: mixer_audient: Add length check to USB transfer Christian Ruppert
2026-11-01  0:00           ` [RFC PATCH 05/11] ALSA: mixer_audient: make USB control transfer size configurable Christian Ruppert
2026-11-01  0:00           ` [RFC PATCH 06/11] ALSA: mixer_audient: Allow arbitrary boolean control values Christian Ruppert
2026-11-01  0:00           ` [RFC PATCH 07/11] ALSA: mixer_audient: Add integer range checks Christian Ruppert
2026-11-01  0:00           ` [RFC PATCH 08/11] ALSA: mixer_audient: Add control initialization at probe Christian Ruppert
2026-11-01  0:00           ` [RFC PATCH 09/11] ALSA: mixer_audient: Add readback-free parameter set Christian Ruppert
2026-11-01  0:00           ` [RFC PATCH 10/11] ALSA: mixer_audient: add control value caching Christian Ruppert
2026-11-01  0:00           ` [RFC PATCH 11/11] ALSA: mixer_audient: Create matrix programmatically Christian Ruppert
2026-11-01 17:35           ` [RFC PATCH 00/11] Re: ALSA: usb-audio: Audient iD14 MkI vendor controls Will Porter
2026-11-02  0:32             ` Christian Ruppert [this message]
2026-11-02  0:32               ` [RFC V2 PATCH 01/11] ALSA: rename mixer_evo to mixer_audient Christian Ruppert
2026-11-02  0:32               ` [RFC V2 PATCH 02/11] ALSA: mixer_audient: explicitly define full control names Christian Ruppert
2026-11-02  0:32               ` [RFC V2 PATCH 03/11] ALSA: mixer_audient: clarify USB control message definition Christian Ruppert
2026-11-02  0:32               ` [RFC V2 PATCH 04/11] ALSA: mixer_audient: Add length check to USB transfer Christian Ruppert
2026-11-02  0:32               ` [RFC V2 PATCH 05/11] ALSA: mixer_audient: make USB control transfer size configurable Christian Ruppert
2026-11-02  0:32               ` [RFC V2 PATCH 06/11] ALSA: mixer_audient: Allow arbitrary boolean control values Christian Ruppert
2026-11-02  0:32               ` [RFC V2 PATCH 07/11] ALSA: mixer_audient: Add integer range checks Christian Ruppert
2026-11-02  0:32               ` [RFC V2 PATCH 08/11] ALSA: mixer_audient: Add control initialization at probe Christian Ruppert
2026-11-02  0:32               ` [RFC V2 PATCH 09/11] ALSA: mixer_audient: Add readback-free parameter set Christian Ruppert
2026-11-02  0:32               ` [RFC V2 PATCH 10/11] ALSA: mixer_audient: add control value caching Christian Ruppert
2026-11-02  0:32               ` [RFC V2 PATCH 11/11] ALSA: mixer_audient: Create matrix programmatically Christian Ruppert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261102003257.10134-1-arc@gmx.li \
    --to=arc@gmx.li \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=mrwillporter@gmail.com \
    --cc=neil@androos.io \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®