Re: [RFC PATCH 13/16] ALSA: mixer_audient: Add enumerated controls

Re: [RFC PATCH 13/16] ALSA: mixer_audient: Add enumerated controls

[Will Porter]

Hi Christian,

The bot found a real problem in my patch 13:

> Does advertising ctype->enumeration->items + 1 items in
> snd_ctl_audient_info() while rejecting item index e->items with
> -EINVAL in snd_ctl_audient_enum_set() break the ALSA control UAPI
> contract for writable enumerated controls?

Yes. mixer-test writes every advertised item, and alsactl restore
fails on a value that store saved as the extra item.

The diff below drops the extra item from every enum:

- A device value outside the list makes get return -EIO. I chose an
  error over a fallback item: an unknown routing value shown as Main
  Mix could hide a route that bypasses the speaker volume, cut and
  dim. The device is not known to report such a value.
- A write-only value that no transfer has confirmed (only after a
  failed probe write) reads as its init value, as a write-only switch
  reads off.

Tested on the iD14 (stable 7.2.15) with the diff applied, with a
script that does the enum writes of mixer-test on this card only:
get returned every advertised item after its put (from the cache for
the write-only controls), and the item past the end was rejected.
alsactl store and restore of the card succeeded. I did not run the
full mixer-test, because it writes the controls of every card.

Does -EIO from get look right to you here? If the diff looks good,
I'll fold it into patches 13 and 15 in the next version.

Thanks,
Will

-- >8 --
From: Will Porter <mrwillporter@gmail.com>
Subject: [RFC PATCH] ALSA: mixer_audient: Advertise only selectable enum items

Every enumerated control advertised one item more than a put accepts.
mixer-test writes every advertised item, and alsactl restore fails on
a stored value of that item. Advertise only the real items.

A get of a device value outside the list now returns -EIO: an unknown
routing value shown as Main Mix could hide a route that bypasses the
speaker volume, cut and dim. After a failed probe write, a write-only
value reads as its init value; this does not confirm the device
state.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Will Porter <mrwillporter@gmail.com>
---
Proposed change to patches 13 and 15.

 sound/usb/mixer_audient.c | 41 +++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/sound/usb/mixer_audient.c b/sound/usb/mixer_audient.c
index e65c62e1eac4a..4180d7d8b325c 100644
--- a/sound/usb/mixer_audient.c
+++ b/sound/usb/mixer_audient.c
@@ -67,10 +67,8 @@ static const DECLARE_TLV_DB_MINMAX_MUTE(snd_audient_tlv_mixer, -12800, 600);
 #define SND_AUDIENT_CTYPE_FLAGS_CACHE		BIT(3)
 
 /*
- * An enumerated control: item i sends values[i]. The texts have one item
- * more than the values. A put cannot select that last item: a get shows it
- * for a device value outside the list ("Other"), or for a write-only value
- * that no write has confirmed ("Unknown").
+ * An enumerated control: item i sends values[i]. A get of a device value
+ * outside the list returns -EIO; it does not show a wrong item.
  */
 struct snd_audient_enum {
 	const char *const *texts;
@@ -85,7 +83,7 @@ struct snd_audient_enum {
 		.values = name##_values,				\
 		.items = ARRAY_SIZE(name##_values),			\
 	};								\
-	static_assert(ARRAY_SIZE(texts_) == ARRAY_SIZE(name##_values) + 1)
+	static_assert(ARRAY_SIZE(texts_) == ARRAY_SIZE(name##_values))
 
 /*
  * iD14 MkI units. The monitor controller, output routing, headphone level,
@@ -126,7 +124,7 @@ static const DECLARE_TLV_DB_SCALE(snd_audient_tlv_id14_vol, -12800, 100, 0);
 
 /* Mono mode: one-speaker sum (Left, Right) or phantom centre */
 static const char *const snd_audient_id14_mono_mode_texts[] = {
-	"Left", "Centre", "Right", "Other",
+	"Left", "Centre", "Right",
 };
 SND_AUDIENT_ENUM(snd_audient_id14_mono_mode,
 		 snd_audient_id14_mono_mode_texts, 1, 0, 2);
@@ -139,10 +137,10 @@ SND_AUDIENT_ENUM(snd_audient_id14_mono_mode,
  * mute still act on every headphone source.
  */
 static const char *const snd_audient_id14_route_main_texts[] = {
-	"Main Mix", "Cue Mix", "DAW Thru (full scale)", "Other",
+	"Main Mix", "Cue Mix", "DAW Thru (full scale)",
 };
 static const char *const snd_audient_id14_route_hp_texts[] = {
-	"Main Mix", "Cue Mix", "DAW Thru", "Other",
+	"Main Mix", "Cue Mix", "DAW Thru",
 };
 SND_AUDIENT_ENUM(snd_audient_id14_route_main_l,
 		 snd_audient_id14_route_main_texts, 0x14, 0x10, 0x00);
@@ -156,12 +154,10 @@ SND_AUDIENT_ENUM(snd_audient_id14_route_hp_r,
 /*
  * iD key function: the value is the monitor controller selector the key
  * toggles. ScrollControl (0xffff) toggles nothing: the knob then scrolls
- * through the HID interface. The last item of a write-only enum shows a
- * value that is not known.
+ * through the HID interface.
  */
 static const char *const snd_audient_id14_id_key_texts[] = {
 	"ScrollControl", "Mono", "Mono + Polarity", "Dim", "Talkback",
-	"Unknown",
 };
 SND_AUDIENT_ENUM(snd_audient_id14_id_key, snd_audient_id14_id_key_texts,
 		 0xffff, ID14_MON_MONO, ID14_MON_POLARITY, ID14_MON_DIM,
@@ -170,13 +166,13 @@ SND_AUDIENT_ENUM(snd_audient_id14_id_key, snd_audient_id14_id_key_texts,
 /* Talkback source: the routing source number of the input; none = 0xff */
 static const char *const snd_audient_id14_talkback_texts[] = {
 	"None", "Mic 1", "Mic 2", "Digi 1", "Digi 2", "Digi 3", "Digi 4",
-	"Digi 5", "Digi 6", "Digi 7", "Digi 8", "Unknown",
+	"Digi 5", "Digi 6", "Digi 7", "Digi 8",
 };
 SND_AUDIENT_ENUM(snd_audient_id14_talkback, snd_audient_id14_talkback_texts,
 		 0xff, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13);
 
 static const char *const snd_audient_id14_optical_texts[] = {
-	"ADAT", "S/PDIF", "Unknown",
+	"ADAT", "S/PDIF",
 };
 SND_AUDIENT_ENUM(snd_audient_id14_optical, snd_audient_id14_optical_texts,
 		 0, 1);
@@ -933,22 +929,23 @@ static int snd_ctl_audient_enum_get(struct snd_kcontrol *kctl,
 	guard(mutex)(&mixer->lock);
 
 	for (i = 0; i < hwchan; i++) {
+		unsigned int item;
 		u32 val;
 		int ret;
 
 		ret = snd_audient_raw_get_one(mixer, ctype,
 					      snd_audient_cache_slot(elem, i),
 					      base + i, &val);
-		if (ret == -ENODATA) {	/* unknown write-only value */
-			ctl_val->value.enumerated.item[i] =
-				ctype->enumeration->items;
-			continue;
-		} else if (ret < 0) {
+		if (ret == -ENODATA)	/* unknown write-only value: init */
+			val = ctype->init;
+		else if (ret < 0)
 			return ret;
-		}
 
-		ctl_val->value.enumerated.item[i] =
-			snd_audient_enum_item(ctype->enumeration, val);
+		item = snd_audient_enum_item(ctype->enumeration, val);
+		if (item >= ctype->enumeration->items)
+			return -EIO;
+
+		ctl_val->value.enumerated.item[i] = item;
 	}
 
 	for (; i < nchan; i++)
@@ -1007,7 +1004,7 @@ static int snd_ctl_audient_info(struct snd_kcontrol *kctl,
 
 	if (ctype->type == SNDRV_CTL_ELEM_TYPE_ENUMERATED)
 		return snd_ctl_enum_info(uinfo, snd_audient_kctl_channels(kctl),
-					 ctype->enumeration->items + 1,
+					 ctype->enumeration->items,
 					 ctype->enumeration->texts);
 
 	uinfo->type = ctype->type;

Re: [RFC PATCH 13/16] ALSA: mixer_audient: Add enumerated controls

[Christian Ruppert]

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

Hi Will,

Thanks for this update. I squashed it into patches 13 and 15 before
reviewing. I also tested everything on 7.3.3 and could not find any
issues. Here are my comments about the code, most of them are minor:

patch 12:
- I would declare snd_audient_int_max, snd_audient_int_min and
  snd_audient_int_from_ctl as inline, they are little more than macros.

patch 13:
- I would have snd_audient_enum_item return the negative errno instead of
  e->items and then propagate it. This seems more coherent with the
  convention applied almost everywhere else in the kernel.
- I wonder if it would make the code more readable if we refactor all the
  if-statements snd_ctl_audient_info has grown into something like DIFF 1
  below?

patch 15:
- snd_ctl_audient_enum_get uses ctype->init which is not set by the
  ID14_ENUM() macro. Is this intentionally always constant 0? In this
  case, could the logic be removed?
- Not sure if this is relevant for the iD14 but we might want to apply
  something like DIFF 2 to make update events work on the replaced
  controls 11 and possibly 60 (in case somebody fiddles with the buttons,
  please check if it actually works with the device). BTW: I'll see if we
  can somehow also get update events for the new controls of the EVO4 on
  the occasion. The events the generic mixer driver generates don't work
  for those so I didn't bother initializing the ID until now.
- In mixer_quirks.c: Fallthrough case statements seem to be used almost
  everywhere else, I suggest to replace the repeating call to
  snd_audient_controls_create with a fallthrough as well.

patch 16:
- can we rename 'create' to something which makes it obvious from the name
  that the callback is optional, e.g. 'extra_init' or similar?

  Christian

---- DIFF 1 ----
diff --git a/sound/usb/mixer_audient.c b/sound/usb/mixer_audient.c
index 4180d7d8b325..212a2156087c 100644
--- a/sound/usb/mixer_audient.c
+++ b/sound/usb/mixer_audient.c
@@ -1002,22 +1002,25 @@ static int snd_ctl_audient_info(struct snd_kcontrol *kctl,
 	const struct snd_audient_ctrl_type *const ctype
 						= snd_audient_kctl_type(kctl);
 
-	if (ctype->type == SNDRV_CTL_ELEM_TYPE_ENUMERATED)
-		return snd_ctl_enum_info(uinfo, snd_audient_kctl_channels(kctl),
-					 ctype->enumeration->items,
-					 ctype->enumeration->texts);
+	switch (ctype->type) {
+		case SNDRV_CTL_ELEM_TYPE_ENUMERATED:
+			return snd_ctl_enum_info(uinfo,
+					snd_audient_kctl_channels(kctl),
+					ctype->enumeration->items,
+					ctype->enumeration->texts);
+		case SNDRV_CTL_ELEM_TYPE_BOOLEAN:
+			uinfo->value.integer.min = 0;
+			uinfo->value.integer.max = 1;
+			break;
+		default:
+			uinfo->value.integer.min = snd_audient_int_min(ctype);
+			uinfo->value.integer.max = snd_audient_int_max(ctype);
+			break;
+	}
 
 	uinfo->type = ctype->type;
 	uinfo->count = snd_audient_kctl_channels(kctl);
 
-	if (ctype->type == SNDRV_CTL_ELEM_TYPE_BOOLEAN) {
-		uinfo->value.integer.min = 0;
-		uinfo->value.integer.max = 1;
-	} else {
-		uinfo->value.integer.min = snd_audient_int_min(ctype);
-		uinfo->value.integer.max = snd_audient_int_max(ctype);
-	}
-
 	return 0;
 }
 

---- DIFF 2 ----
diff --git a/sound/usb/mixer_audient.c b/sound/usb/mixer_audient.c
--- a/sound/usb/mixer_audient.c
+++ b/sound/usb/mixer_audient.c
@@ -1083,6 +1083,7 @@ static int snd_audient_add_ctrl(struct usb_mixer_interface *mixer,
                return -ENOMEM;
 
        elem->head.mixer = mixer;
+       elem->head.id = ctype->unit;
        elem->channels = ctrl->channels;
 
        if (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_CACHE)






On Fri, Nov 06, 2026 at 02:29:43PM -0600, Will Porter wrote:
> Hi Christian,
> 
> The bot found a real problem in my patch 13:
> 
> > Does advertising ctype->enumeration->items + 1 items in
> > snd_ctl_audient_info() while rejecting item index e->items with
> > -EINVAL in snd_ctl_audient_enum_set() break the ALSA control UAPI
> > contract for writable enumerated controls?
> 
> Yes. mixer-test writes every advertised item, and alsactl restore
> fails on a value that store saved as the extra item.
> 
> The diff below drops the extra item from every enum:
> 
> - A device value outside the list makes get return -EIO. I chose an
>   error over a fallback item: an unknown routing value shown as Main
>   Mix could hide a route that bypasses the speaker volume, cut and
>   dim. The device is not known to report such a value.
> - A write-only value that no transfer has confirmed (only after a
>   failed probe write) reads as its init value, as a write-only switch
>   reads off.
> 
> Tested on the iD14 (stable 7.2.15) with the diff applied, with a
> script that does the enum writes of mixer-test on this card only:
> get returned every advertised item after its put (from the cache for
> the write-only controls), and the item past the end was rejected.
> alsactl store and restore of the card succeeded. I did not run the
> full mixer-test, because it writes the controls of every card.
> 
> Does -EIO from get look right to you here? If the diff looks good,
> I'll fold it into patches 13 and 15 in the next version.
> 
> Thanks,
> Will
> 
> -- >8 --
> From: Will Porter <mrwillporter@gmail.com>
> Subject: [RFC PATCH] ALSA: mixer_audient: Advertise only selectable enum items
> 
> Every enumerated control advertised one item more than a put accepts.
> mixer-test writes every advertised item, and alsactl restore fails on
> a stored value of that item. Advertise only the real items.
> 
> A get of a device value outside the list now returns -EIO: an unknown
> routing value shown as Main Mix could hide a route that bypasses the
> speaker volume, cut and dim. After a failed probe write, a write-only
> value reads as its init value; this does not confirm the device
> state.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Will Porter <mrwillporter@gmail.com>
> ---
> Proposed change to patches 13 and 15.
> 
>  sound/usb/mixer_audient.c | 41 +++++++++++++++++++----------------------
>  1 file changed, 19 insertions(+), 22 deletions(-)
> 
> diff --git a/sound/usb/mixer_audient.c b/sound/usb/mixer_audient.c
> index e65c62e1eac4a..4180d7d8b325c 100644
> --- a/sound/usb/mixer_audient.c
> +++ b/sound/usb/mixer_audient.c
> @@ -67,10 +67,8 @@ static const DECLARE_TLV_DB_MINMAX_MUTE(snd_audient_tlv_mixer, -12800, 600);
>  #define SND_AUDIENT_CTYPE_FLAGS_CACHE		BIT(3)
>  
>  /*
> - * An enumerated control: item i sends values[i]. The texts have one item
> - * more than the values. A put cannot select that last item: a get shows it
> - * for a device value outside the list ("Other"), or for a write-only value
> - * that no write has confirmed ("Unknown").
> + * An enumerated control: item i sends values[i]. A get of a device value
> + * outside the list returns -EIO; it does not show a wrong item.
>   */
>  struct snd_audient_enum {
>  	const char *const *texts;
> @@ -85,7 +83,7 @@ struct snd_audient_enum {
>  		.values = name##_values,				\
>  		.items = ARRAY_SIZE(name##_values),			\
>  	};								\
> -	static_assert(ARRAY_SIZE(texts_) == ARRAY_SIZE(name##_values) + 1)
> +	static_assert(ARRAY_SIZE(texts_) == ARRAY_SIZE(name##_values))
>  
>  /*
>   * iD14 MkI units. The monitor controller, output routing, headphone level,
> @@ -126,7 +124,7 @@ static const DECLARE_TLV_DB_SCALE(snd_audient_tlv_id14_vol, -12800, 100, 0);
>  
>  /* Mono mode: one-speaker sum (Left, Right) or phantom centre */
>  static const char *const snd_audient_id14_mono_mode_texts[] = {
> -	"Left", "Centre", "Right", "Other",
> +	"Left", "Centre", "Right",
>  };
>  SND_AUDIENT_ENUM(snd_audient_id14_mono_mode,
>  		 snd_audient_id14_mono_mode_texts, 1, 0, 2);
> @@ -139,10 +137,10 @@ SND_AUDIENT_ENUM(snd_audient_id14_mono_mode,
>   * mute still act on every headphone source.
>   */
>  static const char *const snd_audient_id14_route_main_texts[] = {
> -	"Main Mix", "Cue Mix", "DAW Thru (full scale)", "Other",
> +	"Main Mix", "Cue Mix", "DAW Thru (full scale)",
>  };
>  static const char *const snd_audient_id14_route_hp_texts[] = {
> -	"Main Mix", "Cue Mix", "DAW Thru", "Other",
> +	"Main Mix", "Cue Mix", "DAW Thru",
>  };
>  SND_AUDIENT_ENUM(snd_audient_id14_route_main_l,
>  		 snd_audient_id14_route_main_texts, 0x14, 0x10, 0x00);
> @@ -156,12 +154,10 @@ SND_AUDIENT_ENUM(snd_audient_id14_route_hp_r,
>  /*
>   * iD key function: the value is the monitor controller selector the key
>   * toggles. ScrollControl (0xffff) toggles nothing: the knob then scrolls
> - * through the HID interface. The last item of a write-only enum shows a
> - * value that is not known.
> + * through the HID interface.
>   */
>  static const char *const snd_audient_id14_id_key_texts[] = {
>  	"ScrollControl", "Mono", "Mono + Polarity", "Dim", "Talkback",
> -	"Unknown",
>  };
>  SND_AUDIENT_ENUM(snd_audient_id14_id_key, snd_audient_id14_id_key_texts,
>  		 0xffff, ID14_MON_MONO, ID14_MON_POLARITY, ID14_MON_DIM,
> @@ -170,13 +166,13 @@ SND_AUDIENT_ENUM(snd_audient_id14_id_key, snd_audient_id14_id_key_texts,
>  /* Talkback source: the routing source number of the input; none = 0xff */
>  static const char *const snd_audient_id14_talkback_texts[] = {
>  	"None", "Mic 1", "Mic 2", "Digi 1", "Digi 2", "Digi 3", "Digi 4",
> -	"Digi 5", "Digi 6", "Digi 7", "Digi 8", "Unknown",
> +	"Digi 5", "Digi 6", "Digi 7", "Digi 8",
>  };
>  SND_AUDIENT_ENUM(snd_audient_id14_talkback, snd_audient_id14_talkback_texts,
>  		 0xff, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13);
>  
>  static const char *const snd_audient_id14_optical_texts[] = {
> -	"ADAT", "S/PDIF", "Unknown",
> +	"ADAT", "S/PDIF",
>  };
>  SND_AUDIENT_ENUM(snd_audient_id14_optical, snd_audient_id14_optical_texts,
>  		 0, 1);
> @@ -933,22 +929,23 @@ static int snd_ctl_audient_enum_get(struct snd_kcontrol *kctl,
>  	guard(mutex)(&mixer->lock);
>  
>  	for (i = 0; i < hwchan; i++) {
> +		unsigned int item;
>  		u32 val;
>  		int ret;
>  
>  		ret = snd_audient_raw_get_one(mixer, ctype,
>  					      snd_audient_cache_slot(elem, i),
>  					      base + i, &val);
> -		if (ret == -ENODATA) {	/* unknown write-only value */
> -			ctl_val->value.enumerated.item[i] =
> -				ctype->enumeration->items;
> -			continue;
> -		} else if (ret < 0) {
> +		if (ret == -ENODATA)	/* unknown write-only value: init */
> +			val = ctype->init;
> +		else if (ret < 0)
>  			return ret;
> -		}
>  
> -		ctl_val->value.enumerated.item[i] =
> -			snd_audient_enum_item(ctype->enumeration, val);
> +		item = snd_audient_enum_item(ctype->enumeration, val);
> +		if (item >= ctype->enumeration->items)
> +			return -EIO;
> +
> +		ctl_val->value.enumerated.item[i] = item;
>  	}
>  
>  	for (; i < nchan; i++)
> @@ -1007,7 +1004,7 @@ static int snd_ctl_audient_info(struct snd_kcontrol *kctl,
>  
>  	if (ctype->type == SNDRV_CTL_ELEM_TYPE_ENUMERATED)
>  		return snd_ctl_enum_info(uinfo, snd_audient_kctl_channels(kctl),
> -					 ctype->enumeration->items + 1,
> +					 ctype->enumeration->items,
>  					 ctype->enumeration->texts);
>  
>  	uinfo->type = ctype->type;

-- 
  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 --]

Re: [RFC PATCH 13/16] ALSA: mixer_audient: Add enumerated controls

[Will Porter]

Hi Christian,

Thank you for squashing the fix and for testing it on 7.3.3.

> patch 12:
> - I would declare snd_audient_int_max, snd_audient_int_min and
>   snd_audient_int_from_ctl as inline [...]

Done.

> patch 13:
> - I would have snd_audient_enum_item return the negative errno
>   instead of e->items and then propagate it.

Done. It returns the item or -EIO, and get propagates the error.

> - I wonder if it would make the code more readable if we refactor all
>   the if-statements snd_ctl_audient_info has grown into something
>   like DIFF 1 below?

Done. snd_ctl_audient_info() now uses a switch.

> patch 15:
> - snd_ctl_audient_enum_get uses ctype->init which is not set by the
>   ID14_ENUM() macro. Is this intentionally always constant 0? In this
>   case, could the logic be removed?

I think the logic is still needed, because init is not always 0.
ID14_ENUM() builds only the readable enums: Mono Mode and the four
routing controls. They have no cache entry, so
snd_audient_raw_get_one() never returns -ENODATA for them, and no code
reads init for them. The three write-only enums (iD key function,
talkback source, optical mode) do not use the macro. They set .init
explicitly, to 0xffff, 0xff and 1. A comment on the macro now says
this.

> - Not sure if this is relevant for the iD14 but we might want to
>   apply something like DIFF 2 to make update events work on the
>   replaced controls 11 and possibly 60 [...]

I left DIFF 2 out of my patches. I have no objection to it, but I
cannot test it. The audio control interface of the iD14 MkI has no
interrupt endpoint (bNumEndpoints is 0), so
snd_usb_mixer_status_create() returns before it creates the URB. The
device sends no status interrupts, and a knob change shows only by a
read. On the iD14 the diff only moves the table-created controls from
id 0 to their unit in id_elems and in the usbmixer proc file.

Two things in sound/usb/mixer.c may matter for your EVO4 events:

- snd_usb_mixer_interrupt_v2() skips an element when the unit has more
  than one element and info->control differs from the control selector
  in the message. Our elements leave control at 0. Would the event
  reach them?
- The notify paths clear info->cached of the generic mixer, not the
  mixer_audient cache. Would a cached control report its old value
  after the event?

Could you carry DIFF 2 with your EVO4 event change?

> - In mixer_quirks.c: Fallthrough case statements seem to be used
>   almost everywhere else, [...]

Done. The two case labels share one call.

> patch 16:
> - can we rename 'create' to something which makes it obvious from the
>   name that the callback is optional, e.g. 'extra_init' or similar?

Done. It is now extra_init, with a comment that it is optional.

With these changes each of the five patches builds with W=1 without
warnings, and checkpatch --strict reports nothing new.
On the iD14 (stable 7.2.15) the read and write checks of the controls
pass with the new version.

For the next version: unless you would like another RFC round, I
would send it as a normal series, on top of your four mixer_evo fixes
as before. Does that work for you?

Thanks,
Will

Re: [RFC PATCH 13/16] ALSA: mixer_audient: Add enumerated controls

[Christian Ruppert]

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

Hi Will,

On Mon, Nov 09, 2026 at 08:13:02PM -0600, Will Porter wrote:
> [...]
> > - Not sure if this is relevant for the iD14 but we might want to
> >   apply something like DIFF 2 to make update events work on the
> >   replaced controls 11 and possibly 60 [...]
> 
> I left DIFF 2 out of my patches. I have no objection to it, but I
> cannot test it. The audio control interface of the iD14 MkI has no
> interrupt endpoint (bNumEndpoints is 0), so
> snd_usb_mixer_status_create() returns before it creates the URB. The
> device sends no status interrupts, and a knob change shows only by a
> read. On the iD14 the diff only moves the table-created controls from
> id 0 to their unit in id_elems and in the usbmixer proc file.
> 
> Two things in sound/usb/mixer.c may matter for your EVO4 events:
> 
> - snd_usb_mixer_interrupt_v2() skips an element when the unit has more
>   than one element and info->control differs from the control selector
>   in the message. Our elements leave control at 0. Would the event
>   reach them?
> - The notify paths clear info->cached of the generic mixer, not the
>   mixer_audient cache. Would a cached control report its old value
>   after the event?
> 
> Could you carry DIFF 2 with your EVO4 event change?

OK, thanks for the hints, I'll keep them in mind when I start working on
this. I haven't tested the diff either, for the moment it was only a
sketch so it's most probably incomplete. This is actually only required
for the EVO if I decide to replace generic controls - which I might
eventually to keep the philosophy of the driver consistent:
Until now I tried not to touch what was working, even if it's quirky but
as you cleaned up comparable controls for the iD I might just as well do
so for the EVO, too. A higher priority will be to figure out if and how I
can get events for controls not supported by the generic driver, though,
but that will require a few hours on a rainy day...

> [...]
> With these changes each of the five patches builds with W=1 without
> warnings, and checkpatch --strict reports nothing new.
> On the iD14 (stable 7.2.15) the read and write checks of the controls
> pass with the new version.
> 
> For the next version: unless you would like another RFC round, I
> would send it as a normal series, on top of your four mixer_evo fixes
> as before. Does that work for you?

Actually... I was just about to propose the same but when looking at the
code in mixer.c you refer to above (interrupts and cache invalidation) I
realized that usb_mixer_elem_info already contains an integer array for
caching we could use instead of our manually allocated cache. I suspect we
should rework patch 10 accordingly. The positive side-effect of this would
be that info->cached management in the standard notify path would work
should we decide to use it one day.

I'll prepare a new patch 10 but won't be able to test before the week-end.
I'll send a new RFC after that. Do you think it makes sense to refactor
FU12 caching as well?

Cheers,
  Christian

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

[RFC PATCH 0/6] ALSA: mixer_audient: Cache rework and simplifications

[Will Porter]

Hi Christian,

On Tue, Nov 10, 2026 at 09:57:40PM +0100, Christian Ruppert wrote:
> I realized that usb_mixer_elem_info already contains an integer array for
> caching we could use instead of our manually allocated cache. I suspect we
> should rework patch 10 accordingly.

I like the idea. When I read the code for it, I saw a few more places
where the driver could be simpler. It seemed easier to show them as
patches than to describe them, so here they are for you to try on the
EVO4. Please take what you like and drop the rest.

> I'll prepare a new patch 10 but won't be able to test before the week-end.

Patches 3 to 6 need the cache change as a base, so patch 2 is my
version of it, only so that the rest can be tested. Please replace it
with yours. Two questions came up when I wrote it:

- The notify paths clear info->cached unless get_cur_broken is set. A
  write-only control could not read its value back after that. Patch 2
  sets get_cur_broken for the write-only controls. Is that a correct
  use of the field?
- The interrupt path clears the bit of the channel number in the
  message, and channel 0 clears all bits. Patch 2 uses the channel
  index in the element as the bit. Would the channel number of the
  request fit your EVO4 events better?

> Do you think it makes sense to refactor FU12 caching as well?

Yes, I think so: the driver then has one cache. Patch 6 does it.

The patches:

1. Not for merging. It has the changes from your review as one diff,
   so that patches 2 to 6 apply to the RFC with the enum fix.
2. Use cached and cache_val[] of usb_mixer_elem_info.
3. Replace INIT_HW, FORCE_SET and CACHE with an access mode:
   read-back, cached or write-only. The tables use only these three
   combinations of the flags. Do you plan to use another combination
   on the EVO4, for example INIT_HW on a control that can be read? If
   so, I will drop this patch.
4. Fill the cache on the first get or put, not at probe. The probe
   then sends 12 fewer GET requests on the EVO4 and 56 fewer on the
   iD14. Is there a reason for the EVO4 to read at probe?
5. Use one get and one put for booleans, integers and enums. This
   changes the code path of the EVO4 controls, which I cannot test.
6. Use the same cache for the FU 12 control.

Patches 2 to 6 make mixer_audient.c about 100 lines shorter (227
insertions, 324 deletions). Only patch 4 is meant to change behaviour.

Each patch builds with W=1 without warnings, and checkpatch --strict
shows no errors or warnings. On the iD14 MkI (stable 7.2.15) the read
and write checks of the controls pass with all six patches, change
events included. In 7.2.15 cached is still an int, so that build used
bit masks in place of the bitmap calls.

If some of these look useful, please take them into your new RFC in
any form you like. I can also fold them into the earlier patches, if
that saves you time.

Thanks,
Will

Will Porter (6):
  ALSA: mixer_audient: Fold in the review changes to patches 12-16
  ALSA: mixer_audient: Use the cache of usb_mixer_elem_info
  ALSA: mixer_audient: Replace three flags with an access mode
  ALSA: mixer_audient: Fill the cache on first use
  ALSA: mixer_audient: Use one get and one put for all control types
  ALSA: mixer_audient: Use the same cache for the iD14 MkI FU 12 control

 sound/usb/mixer_audient.c | 589 ++++++++++++++++----------------------
 sound/usb/mixer_quirks.c  |   2 -
 2 files changed, 251 insertions(+), 340 deletions(-)

-- 
2.47.3

[RFC PATCH 1/6] ALSA: mixer_audient: Fold in the review changes to patches 12-16

[Will Porter]

This patch is not for merging. It carries the changes from the review
of RFC patches 12 to 16 as one diff, so that the patches after it apply
to the RFC with the enum fix. The next version of the series has these
changes in patches 12, 13, 15 and 16.

- Declare snd_audient_int_max(), snd_audient_int_min() and
  snd_audient_int_from_ctl() inline.
- snd_audient_enum_item() returns the item or -EIO, and get propagates
  the error.
- snd_ctl_audient_info() uses a switch.
- A comment on ID14_ENUM() says that readable enums do not use init.
- The two case labels in mixer_quirks.c share one call.
- Rename the create hook to extra_init and say that it is optional.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Will Porter <mrwillporter@gmail.com>
---
 sound/usb/mixer_audient.c | 58 +++++++++++++++++++++++----------------
 sound/usb/mixer_quirks.c  |  2 --
 2 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/sound/usb/mixer_audient.c b/sound/usb/mixer_audient.c
index 4180d7d8b325c..df1e25a55a7a9 100644
--- a/sound/usb/mixer_audient.c
+++ b/sound/usb/mixer_audient.c
@@ -219,6 +219,11 @@ SND_AUDIENT_ENUM(snd_audient_id14_optical, snd_audient_id14_optical_texts,
 		.sel = (sel_),				\
 	}
 
+/*
+ * A readable enum. It has no cache entry, so a get always reads the device
+ * and never uses init. Write-only enums set .init and .flags without this
+ * macro.
+ */
 #define ID14_ENUM(unit_, sel_, len_, enum_)		\
 	{						\
 		.type = SNDRV_CTL_ELEM_TYPE_ENUMERATED,	\
@@ -494,8 +499,8 @@ static const struct audient_devinfo {
 	const struct snd_audient_ctrl *controls;
 	const char *const *const matrix_names;
 	const unsigned int matrix_channels;
-	/* controls the tables cannot describe; called after the tables */
-	int (*create)(struct usb_mixer_interface *mixer);
+	/* optional: controls the tables cannot describe, created after them */
+	int (*extra_init)(struct usb_mixer_interface *mixer);
 } audient_devinfo[] = {
 	{
 		.usb_id = USB_ID(USB_AUDIENT_VID, USB_EVO4_PID),
@@ -508,7 +513,7 @@ static const struct audient_devinfo {
 		.controls = snd_audient_id14_mk1_controls,
 		.matrix_names = snd_audient_id14_mk1_matrix,
 		.matrix_channels = 2,
-		.create = snd_audient_id14_mk1_create,
+		.extra_init = snd_audient_id14_mk1_create,
 	},
 	{ 0 } /* sentinel */
 };
@@ -779,13 +784,15 @@ static int snd_ctl_audient_boolean_set(struct snd_kcontrol *kctl,
  * With a step size (res), an integer control counts steps from min:
  * 0 ... (max - min) / res. Without one it shows the device value.
  */
-static long snd_audient_int_max(const struct snd_audient_ctrl_type *ctype)
+static inline long
+snd_audient_int_max(const struct snd_audient_ctrl_type *ctype)
 {
 	return ctype->res ? (ctype->max.integer - ctype->min.integer) / ctype->res
 			  : ctype->max.integer;
 }
 
-static long snd_audient_int_min(const struct snd_audient_ctrl_type *ctype)
+static inline long
+snd_audient_int_min(const struct snd_audient_ctrl_type *ctype)
 {
 	return ctype->res ? 0 : ctype->min.integer;
 }
@@ -802,8 +809,8 @@ static long snd_audient_int_to_ctl(const struct snd_audient_ctrl_type *ctype,
 		   snd_audient_int_max(ctype));
 }
 
-static long snd_audient_int_from_ctl(const struct snd_audient_ctrl_type *ctype,
-				     long val)
+static inline long
+snd_audient_int_from_ctl(const struct snd_audient_ctrl_type *ctype, long val)
 {
 	return ctype->res ? ctype->min.integer + val * ctype->res : val;
 }
@@ -896,16 +903,16 @@ static int snd_ctl_audient_integer_set(struct snd_kcontrol *kctl,
 	return changed;
 }
 
-static unsigned int snd_audient_enum_item(const struct snd_audient_enum *e,
-					  u32 value)
+/* The item index of a device value, or -EIO if the value is not in the list */
+static int snd_audient_enum_item(const struct snd_audient_enum *e, u32 value)
 {
 	unsigned int item;
 
 	for (item = 0; item < e->items; item++)
 		if (e->values[item] == value)
-			break;
+			return item;
 
-	return item;	/* e->items: not in the list */
+	return -EIO;
 }
 
 static int snd_ctl_audient_enum_get(struct snd_kcontrol *kctl,
@@ -929,8 +936,8 @@ static int snd_ctl_audient_enum_get(struct snd_kcontrol *kctl,
 	guard(mutex)(&mixer->lock);
 
 	for (i = 0; i < hwchan; i++) {
-		unsigned int item;
 		u32 val;
+		int item;
 		int ret;
 
 		ret = snd_audient_raw_get_one(mixer, ctype,
@@ -942,8 +949,8 @@ static int snd_ctl_audient_enum_get(struct snd_kcontrol *kctl,
 			return ret;
 
 		item = snd_audient_enum_item(ctype->enumeration, val);
-		if (item >= ctype->enumeration->items)
-			return -EIO;
+		if (item < 0)
+			return item;
 
 		ctl_val->value.enumerated.item[i] = item;
 	}
@@ -1002,22 +1009,25 @@ static int snd_ctl_audient_info(struct snd_kcontrol *kctl,
 	const struct snd_audient_ctrl_type *const ctype
 						= snd_audient_kctl_type(kctl);
 
-	if (ctype->type == SNDRV_CTL_ELEM_TYPE_ENUMERATED)
-		return snd_ctl_enum_info(uinfo, snd_audient_kctl_channels(kctl),
+	switch (ctype->type) {
+	case SNDRV_CTL_ELEM_TYPE_ENUMERATED:
+		return snd_ctl_enum_info(uinfo,
+					 snd_audient_kctl_channels(kctl),
 					 ctype->enumeration->items,
 					 ctype->enumeration->texts);
-
-	uinfo->type = ctype->type;
-	uinfo->count = snd_audient_kctl_channels(kctl);
-
-	if (ctype->type == SNDRV_CTL_ELEM_TYPE_BOOLEAN) {
+	case SNDRV_CTL_ELEM_TYPE_BOOLEAN:
 		uinfo->value.integer.min = 0;
 		uinfo->value.integer.max = 1;
-	} else {
+		break;
+	default:
 		uinfo->value.integer.min = snd_audient_int_min(ctype);
 		uinfo->value.integer.max = snd_audient_int_max(ctype);
+		break;
 	}
 
+	uinfo->type = ctype->type;
+	uinfo->count = snd_audient_kctl_channels(kctl);
+
 	return 0;
 }
 
@@ -1332,8 +1342,8 @@ int snd_audient_controls_create(struct usb_mixer_interface *mixer)
 		}
 	}
 
-	if (info->create)
-		return info->create(mixer);
+	if (info->extra_init)
+		return info->extra_init(mixer);
 
 	return 0;
 }
diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
index 41d559d639d22..763bbeb2cd9da 100644
--- a/sound/usb/mixer_quirks.c
+++ b/sound/usb/mixer_quirks.c
@@ -4541,8 +4541,6 @@ int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer)
 		break;
 
 	case USB_ID(USB_AUDIENT_VID, USB_EVO4_PID): /* Audient EVO 4 */
-		err = snd_audient_controls_create(mixer);
-		break;
 	case USB_ID(USB_AUDIENT_VID, USB_ID14_MK1_PID): /* Audient iD14 MkI */
 		err = snd_audient_controls_create(mixer);
 		break;
-- 
2.47.3

[RFC PATCH 2/6] ALSA: mixer_audient: Use the cache of usb_mixer_elem_info

[Will Porter]

A cached control keeps its values in an array that is allocated after
the element and reached through elem->private_data. Every struct
usb_mixer_elem_info already has fields for this: the cached bitmap and
the cache_val[] array.

Use bit i of elem->cached and elem->cache_val[i] for element channel i.
The bit means what the valid flag meant: a transfer confirmed the
value. A successful read or write records the value and sets its bit.
A failed transfer leaves the cached value and the bit unchanged. Remove
the cache struct, its accessor and the extra allocation.

The generic cache holds MAX_CHANNELS values, so refuse a control with
more hardware channels. No control in the tables has more than two.

The generic notify paths clear the cache of an element unless
get_cur_broken is set. A write-only control cannot read its value back
after that, so set get_cur_broken for the cached FORCE_SET controls.
The notify paths do not reach these elements today, because the
elements do not set their unit id.

No functional change for the controls in the tables.

Assisted-by: Claude:claude-fable-5-2
Assisted-by: Claude:claude-sonnet-5-5
Signed-off-by: Will Porter <mrwillporter@gmail.com>
---
 sound/usb/mixer_audient.c | 131 +++++++++++++++++---------------------
 1 file changed, 60 insertions(+), 71 deletions(-)

diff --git a/sound/usb/mixer_audient.c b/sound/usb/mixer_audient.c
index df1e25a55a7a9..35b562b3aac51 100644
--- a/sound/usb/mixer_audient.c
+++ b/sound/usb/mixer_audient.c
@@ -555,24 +555,13 @@ static inline u16 snd_audient_kctl_channels(struct snd_kcontrol *kctl)
 }
 
 /*
- * A cached value is valid once a transfer confirmed it: a read, or a write
- * that succeeded. A probe transfer that fails leaves it invalid. CACHE |
- * FORCE_SET without INIT_HW keeps the earlier behaviour: the init value is
- * assumed, without a transfer.
+ * A cached control keeps its values in the cache of struct
+ * usb_mixer_elem_info: bit i of elem->cached and elem->cache_val[i] for
+ * element channel i. The bit is set once a transfer confirmed the value: a
+ * read, or a write that succeeded. A probe transfer that fails leaves it
+ * clear. CACHE | FORCE_SET without INIT_HW keeps the earlier behaviour: the
+ * init value is assumed, without a transfer.
  */
-struct snd_audient_cache_entry {
-	u32 value;
-	bool valid;
-};
-
-/* Cache entry of element channel i, or NULL for an uncached control. */
-static inline struct snd_audient_cache_entry *
-snd_audient_cache_slot(struct usb_mixer_elem_info *elem, unsigned int i)
-{
-	struct snd_audient_cache_entry *const cache = elem->private_data;
-
-	return cache ? cache + i : NULL;
-}
 
 
 #define AUDIENT_REQTYPE_SET (USB_DIR_OUT | USB_TYPE_CLASS \
@@ -621,26 +610,28 @@ static inline int snd_audient_set_cur(struct usb_mixer_interface *mixer,
 }
 
 
-static int snd_audient_raw_get_one(struct usb_mixer_interface *const mixer,
+static int snd_audient_raw_get_one(struct usb_mixer_elem_info *const elem,
 			const struct snd_audient_ctrl_type *const ctype,
-			struct snd_audient_cache_entry *const slot,
-			const u8 cidx, u32 *const value)
+			const unsigned int ch, const u8 cidx,
+			u32 *const value)
 {
+	struct usb_mixer_interface *const mixer = elem->head.mixer;
+	const bool cache = ctype->flags & SND_AUDIENT_CTYPE_FLAGS_CACHE;
 	int ret;
 	__le32 buf = 0;
 	s32 val;
 
-	if (slot && slot->valid) {
-		*value = slot->value;
+	if (cache && test_bit(ch, elem->cached)) {
+		*value = elem->cache_val[ch];
 
 		return ctype->len;
 	}
 
 	/*
-	 * Cached FORCE_SET controls are treated as write-only: an invalid
-	 * cache entry does not trigger a GET.
+	 * Cached FORCE_SET controls are treated as write-only: a clear
+	 * cache bit does not trigger a GET.
 	 */
-	if (slot && (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_FORCE_SET))
+	if (cache && (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_FORCE_SET))
 		return -ENODATA;
 
 	ret = snd_audient_get_cur(mixer, ctype, cidx, &buf);
@@ -654,27 +645,27 @@ static int snd_audient_raw_get_one(struct usb_mixer_interface *const mixer,
 
 	*value = val;
 
-	if (slot) {
-		slot->value = val;
-		slot->valid = true;
+	if (cache) {
+		elem->cache_val[ch] = val;
+		set_bit(ch, elem->cached);
 	}
 
 	return ret;
 }
 
-static int snd_audient_raw_set_one(struct usb_mixer_interface *const mixer,
+static int snd_audient_raw_set_one(struct usb_mixer_elem_info *const elem,
 			const struct snd_audient_ctrl_type *const ctype,
-			struct snd_audient_cache_entry *const slot,
-			const u8 cidx, const u32 value,
-			bool force)
+			const unsigned int ch, const u8 cidx,
+			const u32 value, bool force)
 {
+	struct usb_mixer_interface *const mixer = elem->head.mixer;
 	int ret;
 	u32 val;
 
 	if (force) {
 		val = !value;		/* just make it different */
 	} else {
-		ret = snd_audient_raw_get_one(mixer, ctype, slot, cidx, &val);
+		ret = snd_audient_raw_get_one(elem, ctype, ch, cidx, &val);
 		if (ret == -ENODATA)	/* FORCE_SET and not cached yet */
 			val = !value;	/* just make it different */
 		else if (ret < 0)
@@ -690,9 +681,9 @@ static int snd_audient_raw_set_one(struct usb_mixer_interface *const mixer,
 		if (ret < 0)
 			return ret;
 
-		if (slot) {
-			slot->value = value;
-			slot->valid = true;
+		if (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_CACHE) {
+			elem->cache_val[ch] = value;
+			set_bit(ch, elem->cached);
 		}
 	}
 
@@ -724,9 +715,8 @@ static int snd_ctl_audient_boolean_get(struct snd_kcontrol *kctl,
 		u32 buf;
 		int ret;
 
-		ret = snd_audient_raw_get_one(mixer, ctype,
-					      snd_audient_cache_slot(elem, i),
-					      base + i, &buf);
+		ret = snd_audient_raw_get_one(elem, ctype, i, base + i,
+					      &buf);
 		if (ret == -ENODATA)	/* unknown write-only value: off */
 			buf = ctype->min.integer;
 		else if (ret < 0)
@@ -767,9 +757,8 @@ static int snd_ctl_audient_boolean_set(struct snd_kcontrol *kctl,
 							  : ctype->min.integer;
 		int ret;
 
-		ret = snd_audient_raw_set_one(mixer, ctype,
-					      snd_audient_cache_slot(elem, i),
-					      base + i, buf, false);
+		ret = snd_audient_raw_set_one(elem, ctype, i, base + i,
+					      buf, false);
 		if (ret < 0)
 			return ret;
 
@@ -839,9 +828,8 @@ static int snd_ctl_audient_integer_get(struct snd_kcontrol *kctl,
 		s32 val;
 		int ret;
 
-		ret = snd_audient_raw_get_one(mixer, ctype,
-					      snd_audient_cache_slot(elem, i),
-					      base + i, &val);
+		ret = snd_audient_raw_get_one(elem, ctype, i, base + i,
+					      &val);
 		if (ret == -ENODATA)	/* unknown write-only value: minimum */
 			val = ctype->min.integer;
 		else if (ret < 0)
@@ -890,9 +878,8 @@ static int snd_ctl_audient_integer_set(struct snd_kcontrol *kctl,
 						   ctl_val->value.integer.value[i]);
 		int ret;
 
-		ret = snd_audient_raw_set_one(mixer, ctype,
-					      snd_audient_cache_slot(elem, i),
-					      base + i, val, false);
+		ret = snd_audient_raw_set_one(elem, ctype, i, base + i,
+					      val, false);
 		if (ret < 0)
 			return ret;
 
@@ -940,9 +927,8 @@ static int snd_ctl_audient_enum_get(struct snd_kcontrol *kctl,
 		int item;
 		int ret;
 
-		ret = snd_audient_raw_get_one(mixer, ctype,
-					      snd_audient_cache_slot(elem, i),
-					      base + i, &val);
+		ret = snd_audient_raw_get_one(elem, ctype, i, base + i,
+					      &val);
 		if (ret == -ENODATA)	/* unknown write-only value: init */
 			val = ctype->init;
 		else if (ret < 0)
@@ -990,9 +976,8 @@ static int snd_ctl_audient_enum_set(struct snd_kcontrol *kctl,
 		const unsigned int item = ctl_val->value.enumerated.item[i];
 		int ret;
 
-		ret = snd_audient_raw_set_one(mixer, ctype,
-					      snd_audient_cache_slot(elem, i),
-					      base + i, e->values[item], false);
+		ret = snd_audient_raw_set_one(elem, ctype, i, base + i,
+					      e->values[item], false);
 		if (ret < 0)
 			return ret;
 
@@ -1072,24 +1057,28 @@ static int snd_audient_add_ctrl(struct usb_mixer_interface *mixer,
 		return -EINVAL;
 	}
 
-	elem = kzalloc(sizeof(*elem)
-		       + ((ctype->flags & SND_AUDIENT_CTYPE_FLAGS_CACHE)
-			  ? (sizeof(struct snd_audient_cache_entry)
-			     * ctrl->hwchannels)
-			  : 0),
-		       GFP_KERNEL);
+	if (ctrl->hwchannels > MAX_CHANNELS)
+		return -EINVAL;
+
+	elem = kzalloc_obj(*elem);
 	if (!elem)
 		return -ENOMEM;
 
 	elem->head.mixer = mixer;
 	elem->channels = ctrl->channels;
 
-	if (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_CACHE)
-		elem->private_data = elem + 1;
+	/*
+	 * The generic notify paths clear the cache of an element unless
+	 * get_cur_broken is set. A write-only control cannot read its value
+	 * back after that.
+	 */
+	if ((ctype->flags & SND_AUDIENT_CTYPE_FLAGS_CACHE) &&
+	    (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_FORCE_SET))
+		elem->get_cur_broken = 1;
 
 	/*
 	 * A failed probe transfer must not fail the card: warn and keep the
-	 * control. Its cache entry stays invalid, so a readable control reads
+	 * control. Its cache bit stays clear, so a readable control reads
 	 * the device again on its next get, and a write-only control shows an
 	 * unknown value until a put succeeds.
 	 */
@@ -1097,8 +1086,7 @@ static int snd_audient_add_ctrl(struct usb_mixer_interface *mixer,
 		int i;
 
 		for (i = 0; i < ctrl->hwchannels; i++) {
-			ret = snd_audient_raw_set_one(mixer, ctype,
-						      snd_audient_cache_slot(elem, i),
+			ret = snd_audient_raw_set_one(elem, ctype, i,
 						      ctrl->val_offs + i,
 						      ctype->init, true);
 			if (ret < 0)
@@ -1106,20 +1094,21 @@ static int snd_audient_add_ctrl(struct usb_mixer_interface *mixer,
 					       "%s: cannot set channel %d (%d)\n",
 					       ctrl->name, i, ret);
 		}
-	} else if (elem->private_data) {
-		struct snd_audient_cache_entry *const cache = elem->private_data;
+	} else if (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_CACHE) {
 		int i;
 
 		for (i = 0; i < ctrl->hwchannels; i++) {
+			u32 val;
+
 			if (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_FORCE_SET) {
-				cache[i].value = ctype->init;
-				cache[i].valid = true;
+				elem->cache_val[i] = ctype->init;
+				set_bit(i, elem->cached);
 				continue;
 			}
 
-			ret = snd_audient_raw_get_one(mixer, ctype, cache + i,
+			ret = snd_audient_raw_get_one(elem, ctype, i,
 						      ctrl->val_offs + i,
-						      &cache[i].value);
+						      &val);
 			if (ret < 0)
 				usb_audio_warn(mixer->chip,
 					       "%s: cannot read channel %d (%d)\n",
-- 
2.47.3

[RFC PATCH 3/6] ALSA: mixer_audient: Replace three flags with an access mode

[Will Porter]

The INIT_HW, FORCE_SET and CACHE flags allow eight combinations. The
control tables use three: no flag, CACHE alone, and all three together
(ID14_WO). Name these three with an enum in a new field, access:

  READBACK:   every get reads the device
  CACHED:     read once, then use the cache
  WRITE_ONLY: never read; show the last write

The SIGNED flag stays.

With a mode, the set helper decides "is it a change" and "send it"
separately. It no longer needs the force argument, or a compare value
that is made different to force a write. The probe write of a
write-only control calls the send helper directly.

The mode also decides the VOLATILE access bit and get_cur_broken.
Remove the ID14_WO macro and the code for the combinations that no
table uses.

No functional change for the controls in the tables.

Assisted-by: Claude:claude-fable-5-2
Assisted-by: Claude:claude-sonnet-5-5
Signed-off-by: Will Porter <mrwillporter@gmail.com>
---
 sound/usb/mixer_audient.c | 172 ++++++++++++++++++++------------------
 1 file changed, 90 insertions(+), 82 deletions(-)

diff --git a/sound/usb/mixer_audient.c b/sound/usb/mixer_audient.c
index 35b562b3aac51..1bc6b4bbd2910 100644
--- a/sound/usb/mixer_audient.c
+++ b/sound/usb/mixer_audient.c
@@ -62,9 +62,23 @@ enum snd_audient_type {
 static const DECLARE_TLV_DB_MINMAX_MUTE(snd_audient_tlv_mixer, -12800, 600);
 
 #define SND_AUDIENT_CTYPE_FLAGS_SIGNED		BIT(0)
-#define SND_AUDIENT_CTYPE_FLAGS_INIT_HW		BIT(1)
-#define SND_AUDIENT_CTYPE_FLAGS_FORCE_SET	BIT(2)
-#define SND_AUDIENT_CTYPE_FLAGS_CACHE		BIT(3)
+
+/*
+ * How a control reaches the device.
+ *
+ * A cached value is valid once a transfer confirmed it. A read or a write
+ * that succeeds records the value and sets its cache bit. A transfer that
+ * fails leaves the cached value and the bit unchanged.
+ *
+ * WRITE_ONLY: GET stalls or does not report writes. The driver writes the
+ * init value at probe and sends every put. A get shows the last value
+ * written, or an unknown value while none was.
+ */
+enum snd_audient_access {
+	SND_AUDIENT_ACCESS_READBACK = 0,	/* every get reads the device */
+	SND_AUDIENT_ACCESS_CACHED,		/* read once, then use the cache */
+	SND_AUDIENT_ACCESS_WRITE_ONLY,		/* never read; show the last write */
+};
 
 /*
  * An enumerated control: item i sends values[i]. A get of a device value
@@ -177,14 +191,6 @@ static const char *const snd_audient_id14_optical_texts[] = {
 SND_AUDIENT_ENUM(snd_audient_id14_optical, snd_audient_id14_optical_texts,
 		 0, 1);
 
-/*
- * Write-only: GET stalls or does not report writes. Show the last value
- * written, write the init value at probe, and send every put.
- */
-#define ID14_WO	(SND_AUDIENT_CTYPE_FLAGS_CACHE		\
-		 | SND_AUDIENT_CTYPE_FLAGS_FORCE_SET	\
-		 | SND_AUDIENT_CTYPE_FLAGS_INIT_HW)
-
 #define ID14_BOOL(unit_, sel_)				\
 	{						\
 		.type = SNDRV_CTL_ELEM_TYPE_BOOLEAN,	\
@@ -220,9 +226,9 @@ SND_AUDIENT_ENUM(snd_audient_id14_optical, snd_audient_id14_optical_texts,
 	}
 
 /*
- * A readable enum. It has no cache entry, so a get always reads the device
- * and never uses init. Write-only enums set .init and .flags without this
- * macro.
+ * A readable enum. Its access mode is READBACK, so a get always reads the
+ * device and never uses init. Write-only enums set .init and .access
+ * without this macro.
  */
 #define ID14_ENUM(unit_, sel_, len_, enum_)		\
 	{						\
@@ -235,6 +241,7 @@ SND_AUDIENT_ENUM(snd_audient_id14_optical, snd_audient_id14_optical_texts,
 
 static const struct snd_audient_ctrl_type {
 	snd_ctl_elem_type_t type;
+	enum snd_audient_access access;
 	unsigned long flags;
 	size_t len;		/* datatype size, can be 1, 2 or 4 bytes */
 	union {
@@ -285,8 +292,8 @@ static const struct snd_audient_ctrl_type {
 	},
 	[SND_AUDIENT_TYPE_MIXER] = {
 		.type = SNDRV_CTL_ELEM_TYPE_INTEGER,
-		.flags = SND_AUDIENT_CTYPE_FLAGS_SIGNED
-			 | SND_AUDIENT_CTYPE_FLAGS_CACHE,
+		.access = SND_AUDIENT_ACCESS_CACHED,
+		.flags = SND_AUDIENT_CTYPE_FLAGS_SIGNED,
 		.len = 2,
 		.min.integer = -128 * 256,
 		.max.integer = 6 * 256,
@@ -330,7 +337,7 @@ static const struct snd_audient_ctrl_type {
 	/* the documented boost is a fixed +10 dB */
 	[SND_AUDIENT_TYPE_ID14_IN_BOOST] = {
 		.type = SNDRV_CTL_ELEM_TYPE_BOOLEAN,
-		.flags = ID14_WO,
+		.access = SND_AUDIENT_ACCESS_WRITE_ONLY,
 		.len = 2,
 		.min.integer = 0,
 		.max.integer = 10 * 256,
@@ -340,7 +347,7 @@ static const struct snd_audient_ctrl_type {
 	},
 	[SND_AUDIENT_TYPE_ID14_IN_POLARITY] = {
 		.type = SNDRV_CTL_ELEM_TYPE_BOOLEAN,
-		.flags = ID14_WO,
+		.access = SND_AUDIENT_ACCESS_WRITE_ONLY,
 		.len = 1,
 		.min.integer = 0,
 		.max.integer = 1,
@@ -350,7 +357,7 @@ static const struct snd_audient_ctrl_type {
 	},
 	[SND_AUDIENT_TYPE_ID14_MON_ID_KEY] = {
 		.type = SNDRV_CTL_ELEM_TYPE_ENUMERATED,
-		.flags = ID14_WO,
+		.access = SND_AUDIENT_ACCESS_WRITE_ONLY,
 		.len = 2,
 		.init = 0xffff,		/* ScrollControl */
 		.enumeration = &snd_audient_id14_id_key,
@@ -359,7 +366,7 @@ static const struct snd_audient_ctrl_type {
 	},
 	[SND_AUDIENT_TYPE_ID14_MON_TALKBACK_SRC] = {
 		.type = SNDRV_CTL_ELEM_TYPE_ENUMERATED,
-		.flags = ID14_WO,
+		.access = SND_AUDIENT_ACCESS_WRITE_ONLY,
 		.len = 1,
 		.init = 0xff,		/* None */
 		.enumeration = &snd_audient_id14_talkback,
@@ -369,7 +376,7 @@ static const struct snd_audient_ctrl_type {
 	/* the power-on mode is S/PDIF */
 	[SND_AUDIENT_TYPE_ID14_OPTICAL_MODE] = {
 		.type = SNDRV_CTL_ELEM_TYPE_ENUMERATED,
-		.flags = ID14_WO,
+		.access = SND_AUDIENT_ACCESS_WRITE_ONLY,
 		.len = 4,
 		.init = 1,		/* S/PDIF */
 		.enumeration = &snd_audient_id14_optical,
@@ -557,10 +564,7 @@ static inline u16 snd_audient_kctl_channels(struct snd_kcontrol *kctl)
 /*
  * A cached control keeps its values in the cache of struct
  * usb_mixer_elem_info: bit i of elem->cached and elem->cache_val[i] for
- * element channel i. The bit is set once a transfer confirmed the value: a
- * read, or a write that succeeded. A probe transfer that fails leaves it
- * clear. CACHE | FORCE_SET without INIT_HW keeps the earlier behaviour: the
- * init value is assumed, without a transfer.
+ * element channel i. See enum snd_audient_access for when the bit is set.
  */
 
 
@@ -611,27 +615,24 @@ static inline int snd_audient_set_cur(struct usb_mixer_interface *mixer,
 
 
 static int snd_audient_raw_get_one(struct usb_mixer_elem_info *const elem,
-			const struct snd_audient_ctrl_type *const ctype,
-			const unsigned int ch, const u8 cidx,
-			u32 *const value)
+				   const struct snd_audient_ctrl_type *const ctype,
+				   const unsigned int ch, const u8 cidx,
+				   u32 *const value)
 {
 	struct usb_mixer_interface *const mixer = elem->head.mixer;
-	const bool cache = ctype->flags & SND_AUDIENT_CTYPE_FLAGS_CACHE;
 	int ret;
 	__le32 buf = 0;
 	s32 val;
 
-	if (cache && test_bit(ch, elem->cached)) {
+	if (ctype->access != SND_AUDIENT_ACCESS_READBACK &&
+	    test_bit(ch, elem->cached)) {
 		*value = elem->cache_val[ch];
 
 		return ctype->len;
 	}
 
-	/*
-	 * Cached FORCE_SET controls are treated as write-only: a clear
-	 * cache bit does not trigger a GET.
-	 */
-	if (cache && (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_FORCE_SET))
+	/* A write-only control never sends a GET: its value is unknown. */
+	if (ctype->access == SND_AUDIENT_ACCESS_WRITE_ONLY)
 		return -ENODATA;
 
 	ret = snd_audient_get_cur(mixer, ctype, cidx, &buf);
@@ -645,7 +646,7 @@ static int snd_audient_raw_get_one(struct usb_mixer_elem_info *const elem,
 
 	*value = val;
 
-	if (cache) {
+	if (ctype->access == SND_AUDIENT_ACCESS_CACHED) {
 		elem->cache_val[ch] = val;
 		set_bit(ch, elem->cached);
 	}
@@ -653,41 +654,55 @@ static int snd_audient_raw_get_one(struct usb_mixer_elem_info *const elem,
 	return ret;
 }
 
-static int snd_audient_raw_set_one(struct usb_mixer_elem_info *const elem,
-			const struct snd_audient_ctrl_type *const ctype,
-			const unsigned int ch, const u8 cidx,
-			const u32 value, bool force)
+/* Send a value and, for a cached control, record that the device has it. */
+static int snd_audient_raw_send_one(struct usb_mixer_elem_info *const elem,
+				    const struct snd_audient_ctrl_type *const ctype,
+				    const unsigned int ch, const u8 cidx,
+				    const u32 value)
 {
-	struct usb_mixer_interface *const mixer = elem->head.mixer;
+	__le32 buf = cpu_to_le32(value);
 	int ret;
-	u32 val;
-
-	if (force) {
-		val = !value;		/* just make it different */
-	} else {
-		ret = snd_audient_raw_get_one(elem, ctype, ch, cidx, &val);
-		if (ret == -ENODATA)	/* FORCE_SET and not cached yet */
-			val = !value;	/* just make it different */
-		else if (ret < 0)
-			return ret;
+
+	ret = snd_audient_set_cur(elem->head.mixer, ctype, cidx, &buf);
+	if (ret < 0)
+		return ret;
+
+	if (ctype->access != SND_AUDIENT_ACCESS_READBACK) {
+		elem->cache_val[ch] = value;
+		set_bit(ch, elem->cached);
 	}
 
-	force = force || (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_FORCE_SET);
+	return 0;
+}
+
+/*
+ * Set one channel. Returns 1 if the value changed, 0 if not, or a negative
+ * error. A write-only control sends every value; the others send only a
+ * changed one.
+ */
+static int snd_audient_raw_set_one(struct usb_mixer_elem_info *const elem,
+				   const struct snd_audient_ctrl_type *const ctype,
+				   const unsigned int ch, const u8 cidx,
+				   const u32 value)
+{
+	bool changed;
+	u32 cur = 0;
+	int ret;
+
+	ret = snd_audient_raw_get_one(elem, ctype, ch, cidx, &cur);
+	if (ret < 0 && ret != -ENODATA)
+		return ret;
 
-	if (force || (val != value)) {
-		__le32 buf = cpu_to_le32(value);
+	/* -ENODATA: write-only and not cached yet */
+	changed = ret == -ENODATA || cur != value;
 
-		ret = snd_audient_set_cur(mixer, ctype, cidx, &buf);
+	if (changed || ctype->access == SND_AUDIENT_ACCESS_WRITE_ONLY) {
+		ret = snd_audient_raw_send_one(elem, ctype, ch, cidx, value);
 		if (ret < 0)
 			return ret;
-
-		if (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_CACHE) {
-			elem->cache_val[ch] = value;
-			set_bit(ch, elem->cached);
-		}
 	}
 
-	return val != value;
+	return changed;
 }
 
 
@@ -758,7 +773,7 @@ static int snd_ctl_audient_boolean_set(struct snd_kcontrol *kctl,
 		int ret;
 
 		ret = snd_audient_raw_set_one(elem, ctype, i, base + i,
-					      buf, false);
+					      buf);
 		if (ret < 0)
 			return ret;
 
@@ -879,7 +894,7 @@ static int snd_ctl_audient_integer_set(struct snd_kcontrol *kctl,
 		int ret;
 
 		ret = snd_audient_raw_set_one(elem, ctype, i, base + i,
-					      val, false);
+					      val);
 		if (ret < 0)
 			return ret;
 
@@ -977,7 +992,7 @@ static int snd_ctl_audient_enum_set(struct snd_kcontrol *kctl,
 		int ret;
 
 		ret = snd_audient_raw_set_one(elem, ctype, i, base + i,
-					      e->values[item], false);
+					      e->values[item]);
 		if (ret < 0)
 			return ret;
 
@@ -1035,9 +1050,9 @@ static int snd_audient_add_ctrl(struct usb_mixer_interface *mixer,
 						   ctrl->hwchannels);
 	knew.tlv.p = ctype->tlv;
 	knew.access = SNDRV_CTL_ELEM_ACCESS_READWRITE
-			| ((ctype->flags & SND_AUDIENT_CTYPE_FLAGS_CACHE)
-			   ? 0
-			   : SNDRV_CTL_ELEM_ACCESS_VOLATILE)
+			| ((ctype->access == SND_AUDIENT_ACCESS_READBACK)
+			   ? SNDRV_CTL_ELEM_ACCESS_VOLATILE
+			   : 0)
 			| (ctype->tlv ? SNDRV_CTL_ELEM_ACCESS_TLV_READ : 0);
 
 	switch (ctype->type) {
@@ -1072,40 +1087,33 @@ static int snd_audient_add_ctrl(struct usb_mixer_interface *mixer,
 	 * get_cur_broken is set. A write-only control cannot read its value
 	 * back after that.
 	 */
-	if ((ctype->flags & SND_AUDIENT_CTYPE_FLAGS_CACHE) &&
-	    (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_FORCE_SET))
+	if (ctype->access == SND_AUDIENT_ACCESS_WRITE_ONLY)
 		elem->get_cur_broken = 1;
 
 	/*
 	 * A failed probe transfer must not fail the card: warn and keep the
-	 * control. Its cache bit stays clear, so a readable control reads
-	 * the device again on its next get, and a write-only control shows an
+	 * control. Its cache bit stays clear, so a cached control reads the
+	 * device again on its next get, and a write-only control shows an
 	 * unknown value until a put succeeds.
 	 */
-	if (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_INIT_HW) {
+	if (ctype->access == SND_AUDIENT_ACCESS_WRITE_ONLY) {
 		int i;
 
 		for (i = 0; i < ctrl->hwchannels; i++) {
-			ret = snd_audient_raw_set_one(elem, ctype, i,
-						      ctrl->val_offs + i,
-						      ctype->init, true);
+			ret = snd_audient_raw_send_one(elem, ctype, i,
+						       ctrl->val_offs + i,
+						       ctype->init);
 			if (ret < 0)
 				usb_audio_warn(mixer->chip,
 					       "%s: cannot set channel %d (%d)\n",
 					       ctrl->name, i, ret);
 		}
-	} else if (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_CACHE) {
+	} else if (ctype->access == SND_AUDIENT_ACCESS_CACHED) {
 		int i;
 
 		for (i = 0; i < ctrl->hwchannels; i++) {
 			u32 val;
 
-			if (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_FORCE_SET) {
-				elem->cache_val[i] = ctype->init;
-				set_bit(i, elem->cached);
-				continue;
-			}
-
 			ret = snd_audient_raw_get_one(elem, ctype, i,
 						      ctrl->val_offs + i,
 						      &val);
-- 
2.47.3

[RFC PATCH 4/6] ALSA: mixer_audient: Fill the cache on first use

[Will Porter]

The probe reads every channel of a cached control. That is not
necessary: a get or put of a channel whose cache bit is clear already
reads the device and fills the cache.

Remove the probe read of cached controls. A failed first read returns
its error to the caller and leaves the cache bit clear, so the next
call reads again.

This changes behaviour. The probe sends no GET for a cached control:
that is 12 fewer requests on the EVO4 and 56 fewer on the iD14 MkI. A
failed read no longer logs "cannot read channel" at probe. A put to a
channel whose cache bit is clear first sends a GET, then sends a SET
only if the value differs.

The probe write of the init value of write-only controls stays.

Assisted-by: Claude:claude-fable-5-2
Assisted-by: Claude:claude-sonnet-5-5
Signed-off-by: Will Porter <mrwillporter@gmail.com>
---
 sound/usb/mixer_audient.c | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/sound/usb/mixer_audient.c b/sound/usb/mixer_audient.c
index 1bc6b4bbd2910..ff7bf518743ee 100644
--- a/sound/usb/mixer_audient.c
+++ b/sound/usb/mixer_audient.c
@@ -1091,10 +1091,10 @@ static int snd_audient_add_ctrl(struct usb_mixer_interface *mixer,
 		elem->get_cur_broken = 1;
 
 	/*
-	 * A failed probe transfer must not fail the card: warn and keep the
-	 * control. Its cache bit stays clear, so a cached control reads the
-	 * device again on its next get, and a write-only control shows an
-	 * unknown value until a put succeeds.
+	 * A cached control reads the device on its first get or put. A
+	 * write-only control is written once at probe. A failed probe write
+	 * must not fail the card: warn and keep the control. Its cache bit
+	 * stays clear, so it shows an unknown value until a put succeeds.
 	 */
 	if (ctype->access == SND_AUDIENT_ACCESS_WRITE_ONLY) {
 		int i;
@@ -1108,20 +1108,6 @@ static int snd_audient_add_ctrl(struct usb_mixer_interface *mixer,
 					       "%s: cannot set channel %d (%d)\n",
 					       ctrl->name, i, ret);
 		}
-	} else if (ctype->access == SND_AUDIENT_ACCESS_CACHED) {
-		int i;
-
-		for (i = 0; i < ctrl->hwchannels; i++) {
-			u32 val;
-
-			ret = snd_audient_raw_get_one(elem, ctype, i,
-						      ctrl->val_offs + i,
-						      &val);
-			if (ret < 0)
-				usb_audio_warn(mixer->chip,
-					       "%s: cannot read channel %d (%d)\n",
-					       ctrl->name, i, ret);
-		}
 	}
 
 	kctl = snd_ctl_new1(&knew, elem);
-- 
2.47.3

[RFC PATCH 5/6] ALSA: mixer_audient: Use one get and one put for all control types

[Will Porter]

The boolean, integer and enum get and put functions differ only in how
they convert a value between the device and the control. Replace the
six functions with snd_ctl_audient_get() and snd_ctl_audient_put().
Three helpers switch on the control type: one converts a device value
to a control value, one converts a control value back and checks its
range, and one fills a channel that has no hardware.

The locking and the order stay. Put checks every hardware channel
before it takes the lock or sends a request. Get returns -EIO for an
enum value that is not in the list.

The get functions showed an unknown write-only value as off, as the
minimum or as init, by type. Every write-only control in the tables is
a boolean whose init equals min, or an enum. So init gives the same
value for each of them: use init for all types.

No functional change for the controls in the tables.

Assisted-by: Claude:claude-fable-5-2
Assisted-by: Claude:claude-sonnet-5-5
Signed-off-by: Will Porter <mrwillporter@gmail.com>
---
 sound/usb/mixer_audient.c | 284 ++++++++++++++------------------------
 1 file changed, 101 insertions(+), 183 deletions(-)

diff --git a/sound/usb/mixer_audient.c b/sound/usb/mixer_audient.c
index ff7bf518743ee..edc568830366c 100644
--- a/sound/usb/mixer_audient.c
+++ b/sound/usb/mixer_audient.c
@@ -706,84 +706,6 @@ static int snd_audient_raw_set_one(struct usb_mixer_elem_info *const elem,
 }
 
 
-static int snd_ctl_audient_boolean_get(struct snd_kcontrol *kctl,
-				       struct snd_ctl_elem_value *ctl_val)
-{
-	struct usb_mixer_elem_info *const elem = snd_kcontrol_chip(kctl);
-	struct usb_mixer_interface *const mixer = elem->head.mixer;
-
-	const unsigned int nchan = snd_audient_kctl_channels(kctl);
-	const unsigned int hwchan = snd_audient_kctl_hwchannels(kctl);
-	const struct snd_audient_ctrl_type *const ctype
-						= snd_audient_kctl_type(kctl);
-	const u8 base = snd_audient_kctl_cidxoffs(kctl);
-
-	int i;
-
-	CLASS(snd_usb_lock, pm)(mixer->chip);
-	if (pm.err < 0)
-		return pm.err;
-	/* serializes the transfers and the cache, also between gets */
-	guard(mutex)(&mixer->lock);
-
-	for (i = 0; i < hwchan; i++) {
-		u32 buf;
-		int ret;
-
-		ret = snd_audient_raw_get_one(elem, ctype, i, base + i,
-					      &buf);
-		if (ret == -ENODATA)	/* unknown write-only value: off */
-			buf = ctype->min.integer;
-		else if (ret < 0)
-			return ret;
-
-		ctl_val->value.integer.value[i] = (buf == ctype->min.integer)
-						  ? false : true;
-	}
-
-	for (; i < nchan; i++)
-		ctl_val->value.integer.value[i] = 0;
-
-	return 0;
-}
-
-static int snd_ctl_audient_boolean_set(struct snd_kcontrol *kctl,
-				       struct snd_ctl_elem_value *ctl_val)
-{
-	struct usb_mixer_elem_info *const elem = snd_kcontrol_chip(kctl);
-	struct usb_mixer_interface *const mixer = elem->head.mixer;
-
-	const unsigned int hwchan = snd_audient_kctl_hwchannels(kctl);
-	const struct snd_audient_ctrl_type *const ctype
-						= snd_audient_kctl_type(kctl);
-	const u8 base = snd_audient_kctl_cidxoffs(kctl);
-
-	int changed = 0;
-	int i;
-
-	CLASS(snd_usb_lock, pm)(mixer->chip);
-	if (pm.err < 0)
-		return pm.err;
-	/* serializes the transfers and the cache, also between gets */
-	guard(mutex)(&mixer->lock);
-
-	for (i = 0; i < hwchan; i++) {
-		u32 buf = ctl_val->value.integer.value[i] ? ctype->max.integer
-							  : ctype->min.integer;
-		int ret;
-
-		ret = snd_audient_raw_set_one(elem, ctype, i, base + i,
-					      buf);
-		if (ret < 0)
-			return ret;
-
-		if (ret)
-			changed = 1;
-	}
-
-	return changed;
-}
-
 /*
  * With a step size (res), an integer control counts steps from min:
  * 0 ... (max - min) / res. Without one it shows the device value.
@@ -819,106 +741,104 @@ snd_audient_int_from_ctl(const struct snd_audient_ctrl_type *ctype, long val)
 	return ctype->res ? ctype->min.integer + val * ctype->res : val;
 }
 
-static int snd_ctl_audient_integer_get(struct snd_kcontrol *kctl,
-				   struct snd_ctl_elem_value *ctl_val)
+/* The item index of a device value, or -EIO if the value is not in the list */
+static int snd_audient_enum_item(const struct snd_audient_enum *e, u32 value)
 {
-	struct usb_mixer_elem_info *const elem = snd_kcontrol_chip(kctl);
-	struct usb_mixer_interface *const mixer = elem->head.mixer;
-
-	const unsigned int nchan = snd_audient_kctl_channels(kctl);
-	const unsigned int hwchan = snd_audient_kctl_hwchannels(kctl);
-	const struct snd_audient_ctrl_type *const ctype
-						= snd_audient_kctl_type(kctl);
-	const u8 base = snd_audient_kctl_cidxoffs(kctl);
-
-	int i;
+	unsigned int item;
 
-	CLASS(snd_usb_lock, pm)(mixer->chip);
-	if (pm.err < 0)
-		return pm.err;
-	/* serializes the transfers and the cache, also between gets */
-	guard(mutex)(&mixer->lock);
+	for (item = 0; item < e->items; item++)
+		if (e->values[item] == value)
+			return item;
 
-	for (i = 0; i < hwchan; i++) {
-		s32 val;
-		int ret;
+	return -EIO;
+}
 
-		ret = snd_audient_raw_get_one(elem, ctype, i, base + i,
-					      &val);
-		if (ret == -ENODATA)	/* unknown write-only value: minimum */
-			val = ctype->min.integer;
-		else if (ret < 0)
-			return ret;
+/*
+ * Convert a device value to the control value of hardware channel i. An
+ * enumerated value that is not in the list returns -EIO.
+ */
+static int snd_audient_value_to_ctl(const struct snd_audient_ctrl_type *ctype,
+				    struct snd_ctl_elem_value *ctl_val,
+				    unsigned int i, u32 raw)
+{
+	int item;
 
+	switch (ctype->type) {
+	case SNDRV_CTL_ELEM_TYPE_BOOLEAN:
+		ctl_val->value.integer.value[i] = raw != ctype->min.integer;
+		break;
+	case SNDRV_CTL_ELEM_TYPE_INTEGER:
 		ctl_val->value.integer.value[i] =
-			snd_audient_int_to_ctl(ctype, val);
-	}
+			snd_audient_int_to_ctl(ctype, (s32)raw);
+		break;
+	default:	/* SNDRV_CTL_ELEM_TYPE_ENUMERATED */
+		item = snd_audient_enum_item(ctype->enumeration, raw);
+		if (item < 0)
+			return item;
 
-	for (; i < nchan; i++)
-		ctl_val->value.integer.value[i] = snd_audient_int_min(ctype);
+		ctl_val->value.enumerated.item[i] = item;
+		break;
+	}
 
 	return 0;
 }
 
-static int snd_ctl_audient_integer_set(struct snd_kcontrol *kctl,
-				   struct snd_ctl_elem_value *ctl_val)
+/* The control value of a channel without hardware: off, minimum, item 0 */
+static void snd_audient_empty_ctl(const struct snd_audient_ctrl_type *ctype,
+				  struct snd_ctl_elem_value *ctl_val,
+				  unsigned int i)
 {
-	struct usb_mixer_elem_info *const elem = snd_kcontrol_chip(kctl);
-	struct usb_mixer_interface *mixer = elem->head.mixer;
-
-	const unsigned int hwchan = snd_audient_kctl_hwchannels(kctl);
-	const struct snd_audient_ctrl_type *const ctype
-						= snd_audient_kctl_type(kctl);
-	const u8 base = snd_audient_kctl_cidxoffs(kctl);
-
-	int changed = 0;
-	int i;
+	switch (ctype->type) {
+	case SNDRV_CTL_ELEM_TYPE_BOOLEAN:
+		ctl_val->value.integer.value[i] = 0;
+		break;
+	case SNDRV_CTL_ELEM_TYPE_INTEGER:
+		ctl_val->value.integer.value[i] = snd_audient_int_min(ctype);
+		break;
+	default:	/* SNDRV_CTL_ELEM_TYPE_ENUMERATED */
+		ctl_val->value.enumerated.item[i] = 0;
+		break;
+	}
+}
 
-	for (i = 0; i < hwchan; i++) {
-		long val = ctl_val->value.integer.value[i];
+/*
+ * Convert the control value of hardware channel i to a device value. A
+ * value out of range returns -EINVAL; a boolean has no range error.
+ */
+static int snd_audient_value_from_ctl(const struct snd_audient_ctrl_type *ctype,
+				      const struct snd_ctl_elem_value *ctl_val,
+				      unsigned int i, u32 *raw)
+{
+	unsigned int item;
+	long val;
 
+	switch (ctype->type) {
+	case SNDRV_CTL_ELEM_TYPE_BOOLEAN:
+		*raw = ctl_val->value.integer.value[i] ? ctype->max.integer
+						       : ctype->min.integer;
+		break;
+	case SNDRV_CTL_ELEM_TYPE_INTEGER:
+		val = ctl_val->value.integer.value[i];
 		if (val < snd_audient_int_min(ctype) ||
 		    val > snd_audient_int_max(ctype))
 			return -EINVAL;
-	}
 
-	CLASS(snd_usb_lock, pm)(mixer->chip);
-	if (pm.err < 0)
-		return pm.err;
-	/* serializes the transfers and the cache, also between gets */
-	guard(mutex)(&mixer->lock);
-
-	for (i = 0; i < hwchan; i++) {
-		s32 val = snd_audient_int_from_ctl(ctype,
-						   ctl_val->value.integer.value[i]);
-		int ret;
-
-		ret = snd_audient_raw_set_one(elem, ctype, i, base + i,
-					      val);
-		if (ret < 0)
-			return ret;
+		*raw = (s32)snd_audient_int_from_ctl(ctype, val);
+		break;
+	default:	/* SNDRV_CTL_ELEM_TYPE_ENUMERATED */
+		item = ctl_val->value.enumerated.item[i];
+		if (item >= ctype->enumeration->items)
+			return -EINVAL;
 
-		if (ret)
-			changed = 1;
+		*raw = ctype->enumeration->values[item];
+		break;
 	}
 
-	return changed;
-}
-
-/* The item index of a device value, or -EIO if the value is not in the list */
-static int snd_audient_enum_item(const struct snd_audient_enum *e, u32 value)
-{
-	unsigned int item;
-
-	for (item = 0; item < e->items; item++)
-		if (e->values[item] == value)
-			return item;
-
-	return -EIO;
+	return 0;
 }
 
-static int snd_ctl_audient_enum_get(struct snd_kcontrol *kctl,
-				    struct snd_ctl_elem_value *ctl_val)
+static int snd_ctl_audient_get(struct snd_kcontrol *kctl,
+			       struct snd_ctl_elem_value *ctl_val)
 {
 	struct usb_mixer_elem_info *const elem = snd_kcontrol_chip(kctl);
 	struct usb_mixer_interface *const mixer = elem->head.mixer;
@@ -938,32 +858,28 @@ static int snd_ctl_audient_enum_get(struct snd_kcontrol *kctl,
 	guard(mutex)(&mixer->lock);
 
 	for (i = 0; i < hwchan; i++) {
-		u32 val;
-		int item;
+		u32 raw;
 		int ret;
 
-		ret = snd_audient_raw_get_one(elem, ctype, i, base + i,
-					      &val);
+		ret = snd_audient_raw_get_one(elem, ctype, i, base + i, &raw);
 		if (ret == -ENODATA)	/* unknown write-only value: init */
-			val = ctype->init;
+			raw = ctype->init;
 		else if (ret < 0)
 			return ret;
 
-		item = snd_audient_enum_item(ctype->enumeration, val);
-		if (item < 0)
-			return item;
-
-		ctl_val->value.enumerated.item[i] = item;
+		ret = snd_audient_value_to_ctl(ctype, ctl_val, i, raw);
+		if (ret < 0)
+			return ret;
 	}
 
 	for (; i < nchan; i++)
-		ctl_val->value.enumerated.item[i] = 0;
+		snd_audient_empty_ctl(ctype, ctl_val, i);
 
 	return 0;
 }
 
-static int snd_ctl_audient_enum_set(struct snd_kcontrol *kctl,
-				    struct snd_ctl_elem_value *ctl_val)
+static int snd_ctl_audient_put(struct snd_kcontrol *kctl,
+			       struct snd_ctl_elem_value *ctl_val)
 {
 	struct usb_mixer_elem_info *const elem = snd_kcontrol_chip(kctl);
 	struct usb_mixer_interface *const mixer = elem->head.mixer;
@@ -971,15 +887,19 @@ static int snd_ctl_audient_enum_set(struct snd_kcontrol *kctl,
 	const unsigned int hwchan = snd_audient_kctl_hwchannels(kctl);
 	const struct snd_audient_ctrl_type *const ctype
 						= snd_audient_kctl_type(kctl);
-	const struct snd_audient_enum *const e = ctype->enumeration;
 	const u8 base = snd_audient_kctl_cidxoffs(kctl);
 
 	unsigned int i;
 	int changed = 0;
 
-	for (i = 0; i < hwchan; i++)
-		if (ctl_val->value.enumerated.item[i] >= e->items)
-			return -EINVAL;
+	for (i = 0; i < hwchan; i++) {
+		u32 raw;
+		int ret;
+
+		ret = snd_audient_value_from_ctl(ctype, ctl_val, i, &raw);
+		if (ret < 0)
+			return ret;
+	}
 
 	CLASS(snd_usb_lock, pm)(mixer->chip);
 	if (pm.err < 0)
@@ -988,11 +908,14 @@ static int snd_ctl_audient_enum_set(struct snd_kcontrol *kctl,
 	guard(mutex)(&mixer->lock);
 
 	for (i = 0; i < hwchan; i++) {
-		const unsigned int item = ctl_val->value.enumerated.item[i];
+		u32 raw;
 		int ret;
 
-		ret = snd_audient_raw_set_one(elem, ctype, i, base + i,
-					      e->values[item]);
+		ret = snd_audient_value_from_ctl(ctype, ctl_val, i, &raw);
+		if (ret < 0)
+			return ret;
+
+		ret = snd_audient_raw_set_one(elem, ctype, i, base + i, raw);
 		if (ret < 0)
 			return ret;
 
@@ -1057,21 +980,16 @@ static int snd_audient_add_ctrl(struct usb_mixer_interface *mixer,
 
 	switch (ctype->type) {
 	case SNDRV_CTL_ELEM_TYPE_INTEGER:
-		knew.get = snd_ctl_audient_integer_get;
-		knew.put = snd_ctl_audient_integer_set;
-		break;
 	case SNDRV_CTL_ELEM_TYPE_BOOLEAN:
-		knew.get = snd_ctl_audient_boolean_get;
-		knew.put = snd_ctl_audient_boolean_set;
-		break;
 	case SNDRV_CTL_ELEM_TYPE_ENUMERATED:
-		knew.get = snd_ctl_audient_enum_get;
-		knew.put = snd_ctl_audient_enum_set;
 		break;
 	default:
 		return -EINVAL;
 	}
 
+	knew.get = snd_ctl_audient_get;
+	knew.put = snd_ctl_audient_put;
+
 	if (ctrl->hwchannels > MAX_CHANNELS)
 		return -EINVAL;
 
-- 
2.47.3

[RFC PATCH 6/6] ALSA: mixer_audient: Use the same cache for the iD14 MkI FU 12 control

[Will Porter]

The FU 12 control keeps its own value array and valid mask on a
struct usb_mixer_elem_list. Make it a struct usb_mixer_elem_info, and
use cache_val[ch] and bit ch of cached as the other controls do. Set
channels and get_cur_broken. Remove the private struct.

The list head is the first member of struct usb_mixer_elem_info, so
snd_usb_mixer_elem_free() still frees the right pointer. The control
is still registered with is_std_info false, so the generic notify
paths do not touch it.

No functional change.

Assisted-by: Claude:claude-fable-5-2
Assisted-by: Claude:claude-sonnet-5-5
Signed-off-by: Will Porter <mrwillporter@gmail.com>
---
 sound/usb/mixer_audient.c | 54 ++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/sound/usb/mixer_audient.c b/sound/usb/mixer_audient.c
index edc568830366c..8c81a4c05995f 100644
--- a/sound/usb/mixer_audient.c
+++ b/sound/usb/mixer_audient.c
@@ -1074,26 +1074,26 @@ static const struct snd_audient_ctrl_type snd_audient_id14_fu12_type = {
 	.sel = UAC_FU_VOLUME,
 };
 
-struct snd_audient_id14_fu12 {
-	struct usb_mixer_elem_list list;	/* first: snd_usb_mixer_elem_free */
-	long val[ID14_FU12_CHANNELS];
-	u8 valid;				/* bit per channel */
-};
-
-static int snd_audient_id14_fu12_write(struct snd_audient_id14_fu12 *f,
+/*
+ * The control is a struct usb_mixer_elem_info whose list head is registered
+ * with is_std_info false. The head is the first member, as
+ * snd_usb_mixer_elem_free needs. cache_val[ch] and bit ch of cached hold
+ * the last value the device accepted.
+ */
+static int snd_audient_id14_fu12_write(struct usb_mixer_elem_info *elem,
 				       unsigned int ch, long val)
 {
 	__le16 buf = cpu_to_le16(val == ID14_FU12_MUTE ? -128 * 256
 				 : (val - ID14_FU12_MAX) * 256);
 	int ret;
 
-	ret = snd_audient_set_cur(f->list.mixer, &snd_audient_id14_fu12_type,
+	ret = snd_audient_set_cur(elem->head.mixer, &snd_audient_id14_fu12_type,
 				  ch + 1, &buf);
 	if (ret < 0)
 		return ret;
 
-	f->val[ch] = val;
-	f->valid |= BIT(ch);
+	elem->cache_val[ch] = val;
+	set_bit(ch, elem->cached);
 
 	return 0;
 }
@@ -1112,13 +1112,13 @@ static int snd_ctl_audient_id14_fu12_info(struct snd_kcontrol *kctl,
 static int snd_ctl_audient_id14_fu12_get(struct snd_kcontrol *kctl,
 					 struct snd_ctl_elem_value *ctl_val)
 {
-	struct snd_audient_id14_fu12 *f = snd_kcontrol_chip(kctl);
+	struct usb_mixer_elem_info *elem = snd_kcontrol_chip(kctl);
 	unsigned int ch;
 
 	/* a channel that was never written reads as mute */
 	for (ch = 0; ch < ID14_FU12_CHANNELS; ch++)
-		ctl_val->value.integer.value[ch] = (f->valid & BIT(ch))
-						   ? f->val[ch]
+		ctl_val->value.integer.value[ch] = test_bit(ch, elem->cached)
+						   ? elem->cache_val[ch]
 						   : ID14_FU12_MUTE;
 
 	return 0;
@@ -1127,7 +1127,7 @@ static int snd_ctl_audient_id14_fu12_get(struct snd_kcontrol *kctl,
 static int snd_ctl_audient_id14_fu12_set(struct snd_kcontrol *kctl,
 					 struct snd_ctl_elem_value *ctl_val)
 {
-	struct snd_audient_id14_fu12 *f = snd_kcontrol_chip(kctl);
+	struct usb_mixer_elem_info *elem = snd_kcontrol_chip(kctl);
 	long *const v = ctl_val->value.integer.value;
 	unsigned int ch;
 	int changed = 0;
@@ -1136,7 +1136,7 @@ static int snd_ctl_audient_id14_fu12_set(struct snd_kcontrol *kctl,
 		if (v[ch] < 0 || v[ch] > ID14_FU12_MAX)
 			return -EINVAL;
 
-	CLASS(snd_usb_lock, pm)(f->list.mixer->chip);
+	CLASS(snd_usb_lock, pm)(elem->head.mixer->chip);
 	if (pm.err < 0)
 		return pm.err;
 
@@ -1147,10 +1147,10 @@ static int snd_ctl_audient_id14_fu12_set(struct snd_kcontrol *kctl,
 	for (ch = 0; ch < ID14_FU12_CHANNELS; ch++) {
 		int ret;
 
-		if (!(f->valid & BIT(ch)) || f->val[ch] != v[ch])
+		if (!test_bit(ch, elem->cached) || elem->cache_val[ch] != v[ch])
 			changed = 1;
 
-		ret = snd_audient_id14_fu12_write(f, ch, v[ch]);
+		ret = snd_audient_id14_fu12_write(elem, ch, v[ch]);
 		if (ret < 0)
 			return ret;
 	}
@@ -1171,19 +1171,21 @@ static int snd_audient_id14_mk1_create(struct usb_mixer_interface *mixer)
 		.put = snd_ctl_audient_id14_fu12_set,
 		.tlv.p = snd_audient_tlv_id14_fu12,
 	};
-	struct snd_audient_id14_fu12 *f;
+	struct usb_mixer_elem_info *elem;
 	struct snd_kcontrol *kctl;
 	unsigned int ch;
 
-	f = kzalloc_obj(*f);
-	if (!f)
+	elem = kzalloc_obj(*elem);
+	if (!elem)
 		return -ENOMEM;
 
-	f->list.mixer = mixer;
-	f->list.id = ID14_FU_MIX;
+	elem->head.mixer = mixer;
+	elem->head.id = ID14_FU_MIX;
+	elem->channels = ID14_FU12_CHANNELS;
+	elem->get_cur_broken = 1;
 
 	for (ch = 0; ch < ID14_FU12_CHANNELS; ch++) {
-		const int ret = snd_audient_id14_fu12_write(f, ch,
+		const int ret = snd_audient_id14_fu12_write(elem, ch,
 				ch < ID14_FU12_MAIN_CHANNELS ? ID14_FU12_MAX
 							     : ID14_FU12_MUTE);
 
@@ -1193,15 +1195,15 @@ static int snd_audient_id14_mk1_create(struct usb_mixer_interface *mixer)
 				       ch + 1, ret);
 	}
 
-	kctl = snd_ctl_new1(&knew, f);
+	kctl = snd_ctl_new1(&knew, elem);
 	if (!kctl) {
-		kfree(f);
+		kfree(elem);
 		return -ENOMEM;
 	}
 
 	kctl->private_free = snd_usb_mixer_elem_free;
 
-	return snd_usb_mixer_add_list(&f->list, kctl, false, true);
+	return snd_usb_mixer_add_list(&elem->head, kctl, false, true);
 }
 
 /* called from mixer_quirks.c */
-- 
2.47.3