Re: [RFC V2 PATCH 00/11] Re: ALSA: usb-audio: Audient iD14 MkI vendor

[Christian Ruppert <arc@gmx.li>]

[-- Attachment #1: Type: text/plain, Size: 7205 bytes --]

On Mon, Nov 02, 2026 at 08:45:18PM -0600, Will Porter wrote:
> Hi Christian,
> 
> Thanks for RFC V2, and for folding everything in so quickly.
> 
> On Mon, Nov 02, 2026 at 01:32:46AM +0100, Christian Ruppert wrote:
> > 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.
> 
> Yes, that is what I had in mind: a put of the cached value still
> writes it, but returns 0, so alsactl restore sends no change events.
> On an iD14 MkI, a put of the held value sent no change event on a
> write-only switch, a volume or a matrix control. A put of a new value
> on the write-only switch sent one.
> 
> One question on the same function. A FORCE_SET control without CACHE
> now reads the device before each write; in RFC v1 the put skipped
> that read. If the device refuses the read, the put fails before it
> writes. No table uses FORCE_SET without CACHE today, so nothing
> breaks now.
> 
> Should that combination work? If yes, the put could skip the read
> when the control has no cache slot, as the diff below does. If not,
> a comment or a check in snd_audient_add_ctrl() would say so, and you
> can drop the diff.

I guess that depends how we define force_set. In my mind it's literally
force set, i.e. for write-only values we must explicitly specify both
FORCE_SET and CACHE. The reasoning is that I don't see how write-only
controls can work correctly without cache. But force-setting uncached
read-write controls might be of value if the set operation as such
triggers an event inside a device, even if the value is the same as the
one just read back on some volatile control. I imagine this could become
useful for certain enumeration types, for example to load a preset.

The question is now what the put function should return in such a case and
until we encounter one it's rather hypothetical to say what's preferable:
- We could systematically return 1 because an event was triggered in the
  device. If you think this is the right thing to do, feel free to weave
  your diff into patch 10. I don't think it makes sense to reiterate
  RFC V3 for such a small change and I'll test the full patch set anwyay.
- We could return 1 if the value is changed and 0 if it isn't which is the
  current behaviour and designed to be consistent with FORCE_SET in case we
  do have a cache. I imagine (cached) write-only controls are more probable
  to trigger events on writing than volatile read-write controls so this
  seemed the reasonable choice.

> > 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.
> 
> Fine with me.
> 
> checkpatch --strict on the 11 patches gives no errors or warnings,
> only CHECK-level style notes, mostly unneeded parentheses (04, 05,
> 07, 10) and alignment (01, 05). I'll leave those to your discretion.

Not sure if we need to reiterate because of those, Takashi please advise.
I would like to keep the parentheses for readability if possible and the
alignment issues can be fixed little by little when we touch the file over
time. Or otherwise, you can also weave in fixes if you prefer.

> iD14 patches on RFC V2
> 
> I rebased my iD14 patch series on RFC V2. Three of my patches are now
> part of yours, so seven remain:
> 
>   1. a step size for integer controls
>   2. enumerated controls
>   3. a separate mixer map for the iD14 MkI
>   4. iD14 MkI monitor and routing controls
>   5. iD14 MkI mixer matrix
>   6. iD14 MkI write-only controls
>   7. the iD14 MkI FU 12 control

Is there a reason 4, 5 and 6 are separate? They should all be one big
update in the device definition tables without any code changes, right?

> All 18 patches build one by one with W=1 without warnings.
> On an iD14 MkI (7.2.11 stable), the controls passed the same read and
> write checks as on RFC v1.
> 
> Next steps
> 
> As agreed, I'll send the final series: your 11 patches, then the
> seven iD14 patches. Two questions first:
> 
> - Your four mixer_evo fixes ("ALSA: mixer_evo: minor fixes and tlv
>   addition") are still open on Patchwork. Should they still go in
>   separately, before this series? Then I'd base the final series on
>   sound for-next once Takashi applies them. Or should I put them at
>   the front of the final series?

In principle they are bugfixes and the intention was to keep them small
enough so they can still squeeze into 7.4. If Takashi decides that it's
already too late for this type of fixes I'm happy remove the ones which
are fixed by our new patch set anyway and resubmit the remaining ones at
the beginning of our serie.

Happy to review and test an RFC of the complete series until he's back and
takes a decision. I expect your seven patches aren't massively impacted by
the four pending fixes so starting to review them as soon as you're ready
would allow for one or two iterations if required, without losing any time
while waiting.

> - If the diff below looks right to you, shall I fold it into patch
>   10 when I send, or would you prefer to take it into your own tree
>   first?

See above. If you think it makes sense to apply it please just fold it in,
I don't think we need a full iteration for this.

> Thanks,
> Will
> 
> -- >8 --
> From: Will Porter <mrwillporter@gmail.com>
> Subject: [RFC PATCH] ALSA: mixer_audient: Skip the read for an uncached FORCE_SET put
> 
> A FORCE_SET put without a cache slot reads the device before the
> write, and fails without writing if that read fails. Skip the read
> in that case, as RFC v1 did. Puts on cached controls keep their
> change reporting.
> 
> Also fix a typo in a comment and drop a blank line.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Will Porter <mrwillporter@gmail.com>
> ---
> Proposed change to patch 10.
> 
> diff --git a/sound/usb/mixer_audient.c b/sound/usb/mixer_audient.c
> index 483ab8f..33c2d1f 100644
> --- a/sound/usb/mixer_audient.c
> +++ b/sound/usb/mixer_audient.c
> @@ -329,11 +329,12 @@ static int snd_audient_raw_set_one(struct usb_mixer_interface *const mixer,
>  	int ret;
>  	u32 val;
>  
> -	if (force) {
> +	if (force ||
> +	    (!slot && (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_FORCE_SET))) {
>  		val = !value;		/* just make it different */
>  	} else {
>  		ret = snd_audient_raw_get_one(mixer, ctype, slot, cidx, &val);
> -		if (ret == -ENODATA)	/* FORCED_SET and not cached yet */
> +		if (ret == -ENODATA)	/* FORCE_SET and not cached yet */
>  			val = !value;	/* just make it different */
>  		else if (ret < 0)
>  			return ret;
> @@ -352,7 +353,6 @@ static int snd_audient_raw_set_one(struct usb_mixer_interface *const mixer,
>  			slot->value = value;
>  			slot->valid = true;
>  		}
> -
>  	}
>  
>  	return val != value;

-- 
  Christian Ruppert                                             <arc@gmx.li>

This message is digitally signed with GnuPG.
GPG/PGP Key fingerprint = 4492 2196 75F0 EAA0 981E  B661 1476 F17A 975F 587B

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 870 bytes --]