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

[Will Porter <mrwillporter@gmail.com>]

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