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

[Will Porter <mrwillporter@gmail.com>]

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