mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE
@ 2026-09-15  8:54 Zipdox
  2026-09-15 11:08 ` Takashi Iwai
  0 siblings, 1 reply; 14+ messages in thread
From: Zipdox @ 2026-09-15  8:54 UTC (permalink / raw)
  To: linux-sound; +Cc: perex, linux-kernel, tiwai

Hello,

The USB Audio Mixer Unit driver (sound/usb/mixer.c) sends audio class
1.0 format control messages to devices, regardless of the actual audio
class version. This means audio class 2.0 devices will receive incorrect
messages, stall, and cause the driver to error (e.g. alsamixer exits
with a broken pipe error). This patch implements the GET_CUR, SET_CUR,
and RANGE control requests for mixer units according to the Audio Class
2.0 specification.

I have one question though, which has to do with the get_cur_mix_raw
function. It states "channel = 0: master, 1 = first channel", but as far
as I know the spec doesn't specify a master control for mixer units. I
implemented it so that channel 0 results the channel 1 value, but maybe
some other solution is desirable.

From: Zipdox <zipdox@zipdox.net>
Date: Tue, 15 Sep 2026 10:12:01 +0200
Subject: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and
  RANGE

Signed-off-by: Zipdox <zipdox@zipdox.net>
---
  sound/usb/mixer.c | 191 ++++++++++++++++++++++++++++++++--------------
  1 file changed, 133 insertions(+), 58 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index ecaa8bc08d7c..99270731d409 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -360,12 +360,12 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request,
  	{
  		CLASS(snd_usb_lock, pm)(chip);
  		if (pm.err)
-			return -EIO;
+		return -EIO;
  
-		idx = mixer_ctrl_intf(cval->head.mixer) | (cval->head.id << 8);
-		ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), bRequest,
-				      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
-				      validx, idx, buf, size);
+	idx = mixer_ctrl_intf(cval->head.mixer) | (cval->head.id << 8);
+	ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), bRequest,
+			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
+			      validx, idx, buf, size);
  	}
  
  	if (ret < 0) {
@@ -410,6 +410,53 @@ static int get_ctl_value(struct usb_mixer_elem_info *cval, int request,
  		get_ctl_value_v2(cval, request, validx, value_ret);
  }
  
+static int get_ctl_range_16(struct usb_mixer_elem_info *cval)
+{
+	struct snd_usb_audio *chip = cval->head.mixer->chip;
+	int idx;
+	int ret;
+	__u16 wNumSubRanges;
+	__u8 range_res[8] = {0};
+
+	__u8 mcn = (cval->control - 1) * cval->channels;
+	__u16 wValue = (UAC2_MU_MIXER << 8) | mcn;
+
+	if (snd_usb_lock_shutdown(chip))
+		return -EIO;
+
+	idx = mixer_ctrl_intf(cval->head.mixer) | (cval->head.id << 8);
+	ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), UAC2_CS_RANGE,
+			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
+			      wValue, idx, range_res, sizeof(range_res));
+	snd_usb_unlock_shutdown(chip);
+
+	if (ret < 0) {
+		usb_audio_dbg(chip,
+			"cannot get ctl range: req = %#x, wValue = %#x, wIndex = %#x\n",
+			UAC2_CS_RANGE, wValue, idx);
+		return ret;
+	}
+
+	wNumSubRanges = combine_word(&range_res[0]);
+	if(wNumSubRanges != 1) {
+		usb_audio_dbg(chip,
+			"unexpected wNumSubRanges (%u) for ctl range: req = %#x, wValue = %#x, wIndex = %#x\n",
+			(unsigned int)wNumSubRanges, UAC2_CS_RANGE, wValue, idx);
+		return -EIO;
+	}
+
+	cval->min = combine_word(&range_res[2]);
+	if (cval->min >= 0x8000) cval->min -= 0x10000;
+	cval->max = combine_word(&range_res[4]);
+	if (cval->max >= 0x8000) cval->max -= 0x10000;
+	cval->res = combine_word(&range_res[6]);
+	usb_audio_dbg(chip,
+		"range for mcn = %u: min = %d, max = %d, res = %d\n",
+		(unsigned int)mcn, cval->min, cval->max, cval->res);
+
+	return 0;
+}
+
  static int get_cur_ctl_value(struct usb_mixer_elem_info *cval,
  			     int validx, int *value)
  {
@@ -420,9 +467,18 @@ static int get_cur_ctl_value(struct usb_mixer_elem_info *cval,
  static inline int get_cur_mix_raw(struct usb_mixer_elem_info *cval,
  				  int channel, int *value)
  {
-	return get_ctl_value(cval, UAC_GET_CUR,
+	if(cval->head.mixer->protocol == UAC_VERSION_1){
+		return get_ctl_value(cval, UAC_GET_CUR,
  			     (cval->control << 8) | channel,
  			     value);
+	}else{
+		__u8 mcn = (cval->control - 1) * cval->channels + ((channel == 0 ? 1 : channel) - 1);
+		__u16 wValue = (UAC2_MU_MIXER << 8) | mcn;
+		return get_ctl_value(cval, UAC_GET_CUR,
+			     wValue,
+			     value);
+	}
+	
  }
  
  int snd_usb_get_cur_mix_value(struct usb_mixer_elem_info *cval,
@@ -527,9 +583,18 @@ int snd_usb_set_cur_mix_value(struct usb_mixer_elem_info *cval, int channel,
  		return 0;
  	}
  
-	err = snd_usb_mixer_set_ctl_value(cval,
-					  UAC_SET_CUR, (cval->control << 8) | channel,
-					  value);
+	if(cval->head.mixer->protocol == UAC_VERSION_1){
+		err = snd_usb_mixer_set_ctl_value(cval,
+						UAC_SET_CUR, (cval->control << 8) | channel,
+						value);
+	}else{
+		__u8 mcn = (cval->control - 1) * cval->channels + ((channel == 0 ? 1 : channel) - 1);
+		__u16 wValue = (UAC2_MU_MIXER << 8) | mcn;
+		err = snd_usb_mixer_set_ctl_value(cval,
+						UAC_SET_CUR, wValue,
+						value);
+	}
+
  	if (err < 0)
  		return err;
  	cval->cached |= BIT(channel);
@@ -1336,59 +1401,69 @@ static int get_min_max_with_quirks(struct usb_mixer_elem_info *cval,
  	    cval->val_type == USB_MIXER_INV_BOOLEAN) {
  		cval->initialized = 1;
  	} else {
-		int minchn = 0;
-		if (cval->cmask) {
-			for (i = 0; i < MAX_CHANNELS; i++)
-				if (cval->cmask & BIT(i)) {
-					minchn = i + 1;
-					break;
-				}
-		}
-		if (get_ctl_value(cval, UAC_GET_MAX, (cval->control << 8) | minchn, &cval->max) < 0 ||
-		    get_ctl_value(cval, UAC_GET_MIN, (cval->control << 8) | minchn, &cval->min) < 0) {
-			usb_audio_err(cval->head.mixer->chip,
-				      "%d:%d: cannot get min/max values for control %d (id %d)\n",
-				   cval->head.id, mixer_ctrl_intf(cval->head.mixer),
-							       cval->control, cval->head.id);
-			return -EAGAIN;
-		}
-		if (get_ctl_value(cval, UAC_GET_RES,
-				  (cval->control << 8) | minchn,
-				  &cval->res) < 0) {
-			cval->res = 1;
-		} else if (cval->head.mixer->protocol == UAC_VERSION_1) {
-			int last_valid_res = cval->res;
-
-			while (cval->res > 1) {
-				if (snd_usb_mixer_set_ctl_value(cval, UAC_SET_RES,
-								(cval->control << 8) | minchn,
-								cval->res / 2) < 0)
-					break;
-				cval->res /= 2;
+		if(cval->head.mixer->protocol == UAC_VERSION_1){
+			int minchn = 0;
+			if (cval->cmask) {
+				for (i = 0; i < MAX_CHANNELS; i++)
+					if (cval->cmask & BIT(i)) {
+						minchn = i + 1;
+						break;
+					}
+			}
+			if (get_ctl_value(cval, UAC_GET_MAX, (cval->control << 8) | minchn, &cval->max) < 0 ||
+				get_ctl_value(cval, UAC_GET_MIN, (cval->control << 8) | minchn, &cval->min) < 0) {
+				usb_audio_err(cval->head.mixer->chip,
+						"%d:%d: cannot get min/max values for control %d (id %d)\n",
+					cval->head.id, mixer_ctrl_intf(cval->head.mixer),
+									cval->control, cval->head.id);
+				return -EAGAIN;
  			}
  			if (get_ctl_value(cval, UAC_GET_RES,
-					  (cval->control << 8) | minchn, &cval->res) < 0)
-				cval->res = last_valid_res;
-		}
-		if (cval->res == 0)
-			cval->res = 1;
+					(cval->control << 8) | minchn,
+					&cval->res) < 0) {
+				cval->res = 1;
+			} else if (cval->head.mixer->protocol == UAC_VERSION_1) {
+				int last_valid_res = cval->res;
+
+				while (cval->res > 1) {
+					if (snd_usb_mixer_set_ctl_value(cval, UAC_SET_RES,
+									(cval->control << 8) | minchn,
+									cval->res / 2) < 0)
+						break;
+					cval->res /= 2;
+				}
+				if (get_ctl_value(cval, UAC_GET_RES,
+						(cval->control << 8) | minchn, &cval->res) < 0)
+					cval->res = last_valid_res;
+			}
+			if (cval->res == 0)
+				cval->res = 1;
  
-		if (cval->min < cval->max) {
-			int saved;
+			if (cval->min < cval->max) {
+				int saved;
  
-			if (get_cur_mix_raw(cval, minchn, &saved) < 0)
-				goto no_checks;
+				if (get_cur_mix_raw(cval, minchn, &saved) < 0)
+					goto no_checks;
  
-			ret = check_sticky_volume_control(cval, minchn, saved);
-			if (ret)
-				goto no_checks;
+				ret = check_sticky_volume_control(cval, minchn, saved);
+				if (ret)
+					goto no_checks;
  
-			if (cval->min + cval->res < cval->max)
-				check_volume_control_res(cval, minchn, saved);
+				if (cval->min + cval->res < cval->max)
+					check_volume_control_res(cval, minchn, saved);
  
-			snd_usb_set_cur_mix_value(cval, minchn, 0, saved);
-		}
+				snd_usb_set_cur_mix_value(cval, minchn, 0, saved);
+			}
  
+		}else{
+			if(get_ctl_range_16(cval) < 0){
+				usb_audio_err(cval->head.mixer->chip,
+				"%d:%d: cannot get min/max values for control %d (id %d)\n",
+					cval->head.id, mixer_ctrl_intf(cval->head.mixer),
+									cval->control, cval->head.id);
+				return -EINVAL;
+			}
+		}
  no_checks:
  		/*
  		 * Got a non-fatal failure during sanity checks.
@@ -1983,9 +2058,9 @@ static void __build_feature_ctl(struct usb_mixer_interface *mixer,
  			       cval->head.id, kctl->id.name, cval->channels,
  			       cval->min, cval->max, cval->res);
  	} else {
-		usb_audio_dbg(mixer->chip, "[%d] FU [%s] ch = %d, val = %d/%d/%d\n",
-			      cval->head.id, kctl->id.name, cval->channels,
-			      cval->min, cval->max, cval->res);
+	usb_audio_dbg(mixer->chip, "[%d] FU [%s] ch = %d, val = %d/%d/%d\n",
+		      cval->head.id, kctl->id.name, cval->channels,
+		      cval->min, cval->max, cval->res);
  	}
  
  	snd_usb_mixer_add_control(&cval->head, kctl);
@@ -3550,7 +3625,7 @@ void snd_usb_mixer_notify_id(struct usb_mixer_interface *mixer, int unitid)
  		info = mixer_elem_list_to_info(list);
  		/* invalidate cache, so the value is read from the device */
  		if (!info->get_cur_broken)
-			info->cached = 0;
+		info->cached = 0;
  		snd_ctl_notify(mixer->chip->card, SNDRV_CTL_EVENT_MASK_VALUE,
  			       &list->kctl->id);
  	}
-- 
2.47.3

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE
  2026-09-15  8:54 [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE Zipdox
@ 2026-09-15 11:08 ` Takashi Iwai
  2026-09-15 12:54   ` Zipdox
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Iwai @ 2026-09-15 11:08 UTC (permalink / raw)
  To: Zipdox; +Cc: linux-sound, perex, linux-kernel, tiwai

On Tue, 15 Sep 2026 10:54:00 +0200,
Zipdox wrote:
> 
> Hello,
> 
> The USB Audio Mixer Unit driver (sound/usb/mixer.c) sends audio class
> 1.0 format control messages to devices, regardless of the actual audio
> class version. This means audio class 2.0 devices will receive incorrect
> messages, stall, and cause the driver to error (e.g. alsamixer exits
> with a broken pipe error).

Hmm, for UAC2, UAC_GET_CUR & co isn't passed as is at all, but the
call is translated to UAC2_CS_CUR and UAC2_CS_RANGE accordingly in
get_ctl_value_v2() (that is called from get_ctl_value()
conditionally).  So this translation path was skipped somehow with
your device...?


> This patch implements the GET_CUR, SET_CUR,
> and RANGE control requests for mixer units according to the Audio Class
> 2.0 specification.
> 
> I have one question though, which has to do with the get_cur_mix_raw
> function. It states "channel = 0: master, 1 = first channel", but as far
> as I know the spec doesn't specify a master control for mixer units. I
> implemented it so that channel 0 results the channel 1 value, but maybe
> some other solution is desirable.
> 
> From: Zipdox <zipdox@zipdox.net>
> Date: Tue, 15 Sep 2026 10:12:01 +0200
> Subject: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and
>  RANGE
> 
> Signed-off-by: Zipdox <zipdox@zipdox.net>

Could you try to reformat in a proper patch format?
Also, it looks containing many unnecessary / unrelated changes that
make review more difficult.  Please try to concentrate only on the
change you really need.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE
  2026-09-15 11:08 ` Takashi Iwai
@ 2026-09-15 12:54   ` Zipdox
  2026-09-15 14:19     ` Takashi Iwai
  0 siblings, 1 reply; 14+ messages in thread
From: Zipdox @ 2026-09-15 12:54 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: linux-sound, perex, linux-kernel, tiwai

On 9/15/26 1:08 PM, Takashi Iwai wrote:
> Hmm, for UAC2, UAC_GET_CUR & co isn't passed as is at all, but the
> call is translated to UAC2_CS_CUR and UAC2_CS_RANGE accordingly in
> get_ctl_value_v2() (that is called from get_ctl_value()
> conditionally).  So this translation path was skipped somehow with
> your device...?

The request is translated yes, but wValue is incorrect. UAC2 uses a
Mixer Control Number ("mcn") in the low byte to identify the mixer
control, along with a control selector in the high byte (UAC2_MU_MIXER
in this case). UAC1 on the other hand uses the low and high byte to
identify the mixer control. It seems get_ctl_value_v2 doesn't change
wValue to the appropriate format (mcn and control selector). This code
was, as far as I understand it, never correct to begin with for UAC2.

To get the MIN, MAX, and RES, get_min_max_with_quirks currently calls
get_ctl_value three times to get the values separately. It looks like
get_ctl_value_v2 was written to send RANGE requests and extract the
appropriate part of the RANGE response, depending on the request. This
doesn't actually work though, because wValue is incorrect as I stated
above. It also unnecessarily sends three requests instead of one. That's
why I bypassed it entirely and wrote the get_ctl_range_16 function. This
could in theory be consolidated, but there's quite a bit of other code
so I'm hesitant to touch it. Supplying the correct wValue to
get_ctl_value in get_cur_mix_raw fixed getting the current value, and
the same applies to the call to snd_usb_mixer_set_ctl_value in
snd_usb_set_cur_mix_value.

> Could you try to reformat in a proper patch format?
> Also, it looks containing many unnecessary / unrelated changes that
> make review more difficult.  Please try to concentrate only on the
> change you really need.

Perhaps you are referring to the indentation changes resulting from
wrapping existing code in an if statement? I can't really do anything
about that. I don't think I changed any unrelated code itself. If there
are unrelated changes, please point them out to me. In any case, I
changed the patch to the "canonical patch format" as written in the
kernel docs.

From: Zipdox <zipdox@zipdox.net>

The USB Audio Mixer Unit driver (sound/usb/mixer.c) sends audio class
1.0 format control messages to devices, regardless of the actual audio
class version. This means audio class 2.0 devices will receive incorrect
messages, stall, and cause the driver to error (e.g. alsamixer exits
with a broken pipe error). This patch implements the GET_CUR, SET_CUR,
and RANGE control requests for mixer units according to the Audio Class
2.0 specification.

Subject: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE

Signed-off-by: Zipdox <zipdox@zipdox.net>
---
  sound/usb/mixer.c | 191 ++++++++++++++++++++++++++++++++--------------
  1 file changed, 133 insertions(+), 58 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index ecaa8bc08d7c..99270731d409 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -360,12 +360,12 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request,
  	{
  		CLASS(snd_usb_lock, pm)(chip);
  		if (pm.err)
-			return -EIO;
+		return -EIO;
  
-		idx = mixer_ctrl_intf(cval->head.mixer) | (cval->head.id << 8);
-		ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), bRequest,
-				      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
-				      validx, idx, buf, size);
+	idx = mixer_ctrl_intf(cval->head.mixer) | (cval->head.id << 8);
+	ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), bRequest,
+			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
+			      validx, idx, buf, size);
  	}
  
  	if (ret < 0) {
@@ -410,6 +410,53 @@ static int get_ctl_value(struct usb_mixer_elem_info *cval, int request,
  		get_ctl_value_v2(cval, request, validx, value_ret);
  }
  
+static int get_ctl_range_16(struct usb_mixer_elem_info *cval)
+{
+	struct snd_usb_audio *chip = cval->head.mixer->chip;
+	int idx;
+	int ret;
+	__u16 wNumSubRanges;
+	__u8 range_res[8] = {0};
+
+	__u8 mcn = (cval->control - 1) * cval->channels;
+	__u16 wValue = (UAC2_MU_MIXER << 8) | mcn;
+
+	if (snd_usb_lock_shutdown(chip))
+		return -EIO;
+
+	idx = mixer_ctrl_intf(cval->head.mixer) | (cval->head.id << 8);
+	ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), UAC2_CS_RANGE,
+			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
+			      wValue, idx, range_res, sizeof(range_res));
+	snd_usb_unlock_shutdown(chip);
+
+	if (ret < 0) {
+		usb_audio_dbg(chip,
+			"cannot get ctl range: req = %#x, wValue = %#x, wIndex = %#x\n",
+			UAC2_CS_RANGE, wValue, idx);
+		return ret;
+	}
+
+	wNumSubRanges = combine_word(&range_res[0]);
+	if(wNumSubRanges != 1) {
+		usb_audio_dbg(chip,
+			"unexpected wNumSubRanges (%u) for ctl range: req = %#x, wValue = %#x, wIndex = %#x\n",
+			(unsigned int)wNumSubRanges, UAC2_CS_RANGE, wValue, idx);
+		return -EIO;
+	}
+
+	cval->min = combine_word(&range_res[2]);
+	if (cval->min >= 0x8000) cval->min -= 0x10000;
+	cval->max = combine_word(&range_res[4]);
+	if (cval->max >= 0x8000) cval->max -= 0x10000;
+	cval->res = combine_word(&range_res[6]);
+	usb_audio_dbg(chip,
+		"range for mcn = %u: min = %d, max = %d, res = %d\n",
+		(unsigned int)mcn, cval->min, cval->max, cval->res);
+
+	return 0;
+}
+
  static int get_cur_ctl_value(struct usb_mixer_elem_info *cval,
  			     int validx, int *value)
  {
@@ -420,9 +467,18 @@ static int get_cur_ctl_value(struct usb_mixer_elem_info *cval,
  static inline int get_cur_mix_raw(struct usb_mixer_elem_info *cval,
  				  int channel, int *value)
  {
-	return get_ctl_value(cval, UAC_GET_CUR,
+	if(cval->head.mixer->protocol == UAC_VERSION_1){
+		return get_ctl_value(cval, UAC_GET_CUR,
  			     (cval->control << 8) | channel,
  			     value);
+	}else{
+		__u8 mcn = (cval->control - 1) * cval->channels + ((channel == 0 ? 1 : channel) - 1);
+		__u16 wValue = (UAC2_MU_MIXER << 8) | mcn;
+		return get_ctl_value(cval, UAC_GET_CUR,
+			     wValue,
+			     value);
+	}
+	
  }
  
  int snd_usb_get_cur_mix_value(struct usb_mixer_elem_info *cval,
@@ -527,9 +583,18 @@ int snd_usb_set_cur_mix_value(struct usb_mixer_elem_info *cval, int channel,
  		return 0;
  	}
  
-	err = snd_usb_mixer_set_ctl_value(cval,
-					  UAC_SET_CUR, (cval->control << 8) | channel,
-					  value);
+	if(cval->head.mixer->protocol == UAC_VERSION_1){
+		err = snd_usb_mixer_set_ctl_value(cval,
+						UAC_SET_CUR, (cval->control << 8) | channel,
+						value);
+	}else{
+		__u8 mcn = (cval->control - 1) * cval->channels + ((channel == 0 ? 1 : channel) - 1);
+		__u16 wValue = (UAC2_MU_MIXER << 8) | mcn;
+		err = snd_usb_mixer_set_ctl_value(cval,
+						UAC_SET_CUR, wValue,
+						value);
+	}
+
  	if (err < 0)
  		return err;
  	cval->cached |= BIT(channel);
@@ -1336,59 +1401,69 @@ static int get_min_max_with_quirks(struct usb_mixer_elem_info *cval,
  	    cval->val_type == USB_MIXER_INV_BOOLEAN) {
  		cval->initialized = 1;
  	} else {
-		int minchn = 0;
-		if (cval->cmask) {
-			for (i = 0; i < MAX_CHANNELS; i++)
-				if (cval->cmask & BIT(i)) {
-					minchn = i + 1;
-					break;
-				}
-		}
-		if (get_ctl_value(cval, UAC_GET_MAX, (cval->control << 8) | minchn, &cval->max) < 0 ||
-		    get_ctl_value(cval, UAC_GET_MIN, (cval->control << 8) | minchn, &cval->min) < 0) {
-			usb_audio_err(cval->head.mixer->chip,
-				      "%d:%d: cannot get min/max values for control %d (id %d)\n",
-				   cval->head.id, mixer_ctrl_intf(cval->head.mixer),
-							       cval->control, cval->head.id);
-			return -EAGAIN;
-		}
-		if (get_ctl_value(cval, UAC_GET_RES,
-				  (cval->control << 8) | minchn,
-				  &cval->res) < 0) {
-			cval->res = 1;
-		} else if (cval->head.mixer->protocol == UAC_VERSION_1) {
-			int last_valid_res = cval->res;
-
-			while (cval->res > 1) {
-				if (snd_usb_mixer_set_ctl_value(cval, UAC_SET_RES,
-								(cval->control << 8) | minchn,
-								cval->res / 2) < 0)
-					break;
-				cval->res /= 2;
+		if(cval->head.mixer->protocol == UAC_VERSION_1){
+			int minchn = 0;
+			if (cval->cmask) {
+				for (i = 0; i < MAX_CHANNELS; i++)
+					if (cval->cmask & BIT(i)) {
+						minchn = i + 1;
+						break;
+					}
+			}
+			if (get_ctl_value(cval, UAC_GET_MAX, (cval->control << 8) | minchn, &cval->max) < 0 ||
+				get_ctl_value(cval, UAC_GET_MIN, (cval->control << 8) | minchn, &cval->min) < 0) {
+				usb_audio_err(cval->head.mixer->chip,
+						"%d:%d: cannot get min/max values for control %d (id %d)\n",
+					cval->head.id, mixer_ctrl_intf(cval->head.mixer),
+									cval->control, cval->head.id);
+				return -EAGAIN;
  			}
  			if (get_ctl_value(cval, UAC_GET_RES,
-					  (cval->control << 8) | minchn, &cval->res) < 0)
-				cval->res = last_valid_res;
-		}
-		if (cval->res == 0)
-			cval->res = 1;
+					(cval->control << 8) | minchn,
+					&cval->res) < 0) {
+				cval->res = 1;
+			} else if (cval->head.mixer->protocol == UAC_VERSION_1) {
+				int last_valid_res = cval->res;
+
+				while (cval->res > 1) {
+					if (snd_usb_mixer_set_ctl_value(cval, UAC_SET_RES,
+									(cval->control << 8) | minchn,
+									cval->res / 2) < 0)
+						break;
+					cval->res /= 2;
+				}
+				if (get_ctl_value(cval, UAC_GET_RES,
+						(cval->control << 8) | minchn, &cval->res) < 0)
+					cval->res = last_valid_res;
+			}
+			if (cval->res == 0)
+				cval->res = 1;
  
-		if (cval->min < cval->max) {
-			int saved;
+			if (cval->min < cval->max) {
+				int saved;
  
-			if (get_cur_mix_raw(cval, minchn, &saved) < 0)
-				goto no_checks;
+				if (get_cur_mix_raw(cval, minchn, &saved) < 0)
+					goto no_checks;
  
-			ret = check_sticky_volume_control(cval, minchn, saved);
-			if (ret)
-				goto no_checks;
+				ret = check_sticky_volume_control(cval, minchn, saved);
+				if (ret)
+					goto no_checks;
  
-			if (cval->min + cval->res < cval->max)
-				check_volume_control_res(cval, minchn, saved);
+				if (cval->min + cval->res < cval->max)
+					check_volume_control_res(cval, minchn, saved);
  
-			snd_usb_set_cur_mix_value(cval, minchn, 0, saved);
-		}
+				snd_usb_set_cur_mix_value(cval, minchn, 0, saved);
+			}
  
+		}else{
+			if(get_ctl_range_16(cval) < 0){
+				usb_audio_err(cval->head.mixer->chip,
+				"%d:%d: cannot get min/max values for control %d (id %d)\n",
+					cval->head.id, mixer_ctrl_intf(cval->head.mixer),
+									cval->control, cval->head.id);
+				return -EINVAL;
+			}
+		}
  no_checks:
  		/*
  		 * Got a non-fatal failure during sanity checks.
@@ -1983,9 +2058,9 @@ static void __build_feature_ctl(struct usb_mixer_interface *mixer,
  			       cval->head.id, kctl->id.name, cval->channels,
  			       cval->min, cval->max, cval->res);
  	} else {
-		usb_audio_dbg(mixer->chip, "[%d] FU [%s] ch = %d, val = %d/%d/%d\n",
-			      cval->head.id, kctl->id.name, cval->channels,
-			      cval->min, cval->max, cval->res);
+	usb_audio_dbg(mixer->chip, "[%d] FU [%s] ch = %d, val = %d/%d/%d\n",
+		      cval->head.id, kctl->id.name, cval->channels,
+		      cval->min, cval->max, cval->res);
  	}
  
  	snd_usb_mixer_add_control(&cval->head, kctl);
@@ -3550,7 +3625,7 @@ void snd_usb_mixer_notify_id(struct usb_mixer_interface *mixer, int unitid)
  		info = mixer_elem_list_to_info(list);
  		/* invalidate cache, so the value is read from the device */
  		if (!info->get_cur_broken)
-			info->cached = 0;
+		info->cached = 0;
  		snd_ctl_notify(mixer->chip->card, SNDRV_CTL_EVENT_MASK_VALUE,
  			       &list->kctl->id);
  	}
-- 
2.47.3

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE
  2026-09-15 12:54   ` Zipdox
@ 2026-09-15 14:19     ` Takashi Iwai
  2026-09-16  7:43       ` Zipdox
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Iwai @ 2026-09-15 14:19 UTC (permalink / raw)
  To: Zipdox; +Cc: Takashi Iwai, linux-sound, perex, linux-kernel, tiwai

On Tue, 15 Sep 2026 14:54:38 +0200,
Zipdox wrote:
> 
> On 9/15/26 1:08 PM, Takashi Iwai wrote:
> > Hmm, for UAC2, UAC_GET_CUR & co isn't passed as is at all, but the
> > call is translated to UAC2_CS_CUR and UAC2_CS_RANGE accordingly in
> > get_ctl_value_v2() (that is called from get_ctl_value()
> > conditionally).  So this translation path was skipped somehow with
> > your device...?
> 
> The request is translated yes, but wValue is incorrect. UAC2 uses a
> Mixer Control Number ("mcn") in the low byte to identify the mixer
> control, along with a control selector in the high byte (UAC2_MU_MIXER
> in this case). UAC1 on the other hand uses the low and high byte to
> identify the mixer control. It seems get_ctl_value_v2 doesn't change
> wValue to the appropriate format (mcn and control selector). This code
> was, as far as I understand it, never correct to begin with for UAC2.

OK, point taken.

> To get the MIN, MAX, and RES, get_min_max_with_quirks currently calls
> get_ctl_value three times to get the values separately. It looks like
> get_ctl_value_v2 was written to send RANGE requests and extract the
> appropriate part of the RANGE response, depending on the request. This
> doesn't actually work though, because wValue is incorrect as I stated
> above. It also unnecessarily sends three requests instead of one. That's
> why I bypassed it entirely and wrote the get_ctl_range_16 function. This
> could in theory be consolidated, but there's quite a bit of other code
> so I'm hesitant to touch it. Supplying the correct wValue to
> get_ctl_value in get_cur_mix_raw fixed getting the current value, and
> the same applies to the call to snd_usb_mixer_set_ctl_value in
> snd_usb_set_cur_mix_value.

But your code also blindly assumes the MU_MIXER_CONTROL that doesn't
fit with other units like feature unit?  I guess we have to set the
proper Control Selector value depending on the unit type.

After all, this is about the correct setup of wValue.  Let's try to be
minimalistic at first -- just tweak wValue in get_ctl_value_v2() and
snd_usb_mixer_set_ctl_value() at first.  If this becomes too ugly, we
can think of redesigning.


> > Could you try to reformat in a proper patch format?
> > Also, it looks containing many unnecessary / unrelated changes that
> > make review more difficult.  Please try to concentrate only on the
> > change you really need.
> 
> Perhaps you are referring to the indentation changes resulting from
> wrapping existing code in an if statement? I can't really do anything
> about that. I don't think I changed any unrelated code itself. If there
> are unrelated changes, please point them out to me.

Well, get_ctl_value_v2() changes look all unrelated and just breaking
indentations.

> In any case, I
> changed the patch to the "canonical patch format" as written in the
> kernel docs.
> 
> From: Zipdox <zipdox@zipdox.net>
> 
> The USB Audio Mixer Unit driver (sound/usb/mixer.c) sends audio class
> 1.0 format control messages to devices, regardless of the actual audio
> class version. This means audio class 2.0 devices will receive incorrect
> messages, stall, and cause the driver to error (e.g. alsamixer exits
> with a broken pipe error). This patch implements the GET_CUR, SET_CUR,
> and RANGE control requests for mixer units according to the Audio Class
> 2.0 specification.
> 
> Subject: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE

The subject line must be at the beginning right after From tag.


> Signed-off-by: Zipdox <zipdox@zipdox.net>

... and both From and Signed-off-by should be with a real name (or a
known identity).  It's a legal requirement.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE
  2026-09-15 14:19     ` Takashi Iwai
@ 2026-09-16  7:43       ` Zipdox
  2026-09-16 15:40         ` Takashi Iwai
  0 siblings, 1 reply; 14+ messages in thread
From: Zipdox @ 2026-09-16  7:43 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: linux-sound, perex, linux-kernel, tiwai

On 9/15/26 4:19 PM, Takashi Iwai wrote:
> But your code also blindly assumes the MU_MIXER_CONTROL that doesn't
> fit with other units like feature unit?  I guess we have to set the
> proper Control Selector value depending on the unit type.
> 
> After all, this is about the correct setup of wValue.  Let's try to be
> minimalistic at first -- just tweak wValue in get_ctl_value_v2() and
> snd_usb_mixer_set_ctl_value() at first.  If this becomes too ugly, we
> can think of redesigning.
I assumed that mixer.c was just for the mixer unit, and I seem to have 
completely overlooked the fact that that it also controls feature units. 
I'm not sure how to proceed from here. Do you have any suggestions for 
how we can store the control selector in the usb_mixer_elem_info struct? 
The control field is presently used as the input channel number, so 
maybe we should add a separate field for the input channel? I think 
maybe it's better that someone else fixes this, I'm kind of out of my 
element here.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE
  2026-09-16  7:43       ` Zipdox
@ 2026-09-16 15:40         ` Takashi Iwai
  2026-09-16 16:58           ` Takashi Iwai
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Iwai @ 2026-09-16 15:40 UTC (permalink / raw)
  To: Zipdox; +Cc: Takashi Iwai, linux-sound, perex, linux-kernel, tiwai

On Wed, 16 Sep 2026 09:43:14 +0200,
Zipdox wrote:
> 
> On 9/15/26 4:19 PM, Takashi Iwai wrote:
> > But your code also blindly assumes the MU_MIXER_CONTROL that doesn't
> > fit with other units like feature unit?  I guess we have to set the
> > proper Control Selector value depending on the unit type.
> > 
> > After all, this is about the correct setup of wValue.  Let's try to be
> > minimalistic at first -- just tweak wValue in get_ctl_value_v2() and
> > snd_usb_mixer_set_ctl_value() at first.  If this becomes too ugly, we
> > can think of redesigning.
> I assumed that mixer.c was just for the mixer unit, and I seem to have
> completely overlooked the fact that that it also controls feature
> units. I'm not sure how to proceed from here. Do you have any
> suggestions for how we can store the control selector in the
> usb_mixer_elem_info struct? The control field is presently used as the
> input channel number, so maybe we should add a separate field for the
> input channel? I think maybe it's better that someone else fixes this,
> I'm kind of out of my element here.

I took a look back at the current code, and at least, the parse of
UAC2 mixer unit itself is OK -- it handles the bitmap properly.
The problem is the exceptional way to set up wValue for the mixer
unit, and I guess something like below could work.  It just stores the
part of wValue in an additional field, and use it for calculating the
correct wValue with MCN + UAC2_MU_MIXER in high byte.

Just compile tested, so not 100% sure whether it's correct.


thanks,

Takashi

--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -347,6 +347,10 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request,
 
 	val_size = uac2_ctl_value_size(cval->val_type);
 
+	/* correct wValue for UAC2 mixer control with MCN */
+	if (cval->v2_mixer)
+		validx = cval->v2_mixer | ((validx & 0xff) - 1);
+
 	if (request == UAC_GET_CUR) {
 		bRequest = UAC2_CS_CUR;
 		size = val_size;
@@ -2345,6 +2349,10 @@ static void build_mixer_unit_ctl(struct mixer_build *state,
 
 	snd_usb_mixer_elem_init_std(&cval->head, state->mixer, unitid);
 	cval->control = in_ch + 1; /* based on 1 */
+	/* MCN is calculated later with the input channel based on v2_mixer */
+	if (state->mixer->protocol == UAC_VERSION_2 ||
+	    state->mixer->protocol == UAC_VERSION_3)
+		cval->v2_mixer = (UAC2_MU_MIXER << 8) | (in_ch * num_outs);
 	cval->val_type = USB_MIXER_S16;
 	for (i = 0; i < num_outs; i++) {
 		__u8 *c = uac_mixer_unit_bmControls(desc, state->mixer->protocol);
diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h
index 037b446d8b6f..650f30026177 100644
--- a/sound/usb/mixer.h
+++ b/sound/usb/mixer.h
@@ -97,6 +97,7 @@ struct usb_mixer_elem_info {
 	u8 initialized;
 	u8 min_mute;
 	u8 get_cur_broken;
+	u16 v2_mixer;
 	void *private_data;
 };
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE
  2026-09-16 15:40         ` Takashi Iwai
@ 2026-09-16 16:58           ` Takashi Iwai
  2026-09-16 18:22             ` Takashi Iwai
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Iwai @ 2026-09-16 16:58 UTC (permalink / raw)
  To: Zipdox; +Cc: linux-sound, perex, linux-kernel, tiwai

On Wed, 16 Sep 2026 17:40:53 +0200,
Takashi Iwai wrote:
> 
> On Wed, 16 Sep 2026 09:43:14 +0200,
> Zipdox wrote:
> > 
> > On 9/15/26 4:19 PM, Takashi Iwai wrote:
> > > But your code also blindly assumes the MU_MIXER_CONTROL that doesn't
> > > fit with other units like feature unit?  I guess we have to set the
> > > proper Control Selector value depending on the unit type.
> > > 
> > > After all, this is about the correct setup of wValue.  Let's try to be
> > > minimalistic at first -- just tweak wValue in get_ctl_value_v2() and
> > > snd_usb_mixer_set_ctl_value() at first.  If this becomes too ugly, we
> > > can think of redesigning.
> > I assumed that mixer.c was just for the mixer unit, and I seem to have
> > completely overlooked the fact that that it also controls feature
> > units. I'm not sure how to proceed from here. Do you have any
> > suggestions for how we can store the control selector in the
> > usb_mixer_elem_info struct? The control field is presently used as the
> > input channel number, so maybe we should add a separate field for the
> > input channel? I think maybe it's better that someone else fixes this,
> > I'm kind of out of my element here.
> 
> I took a look back at the current code, and at least, the parse of
> UAC2 mixer unit itself is OK -- it handles the bitmap properly.
> The problem is the exceptional way to set up wValue for the mixer
> unit, and I guess something like below could work.  It just stores the
> part of wValue in an additional field, and use it for calculating the
> correct wValue with MCN + UAC2_MU_MIXER in high byte.
> 
> Just compile tested, so not 100% sure whether it's correct.

... and this version was obviously half-baked, forgotten the
counterpart.  A revised patch is below.


Takashi

-- 8< --
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 33a6a1281410..0977fc58d0b5 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -347,6 +347,10 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request,
 
 	val_size = uac2_ctl_value_size(cval->val_type);
 
+	/* correct wValue for UAC2 mixer control with MCN */
+	if (cval->v2_mixer)
+		validx = cval->v2_mixer | ((validx & 0xff) - 1);
+
 	if (request == UAC_GET_CUR) {
 		bRequest = UAC2_CS_CUR;
 		size = val_size;
@@ -478,6 +482,10 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
 		}
 
 		request = UAC2_CS_CUR;
+
+		/* correct wValue for UAC2 mixer control with MCN */
+		if (cval->v2_mixer)
+			validx = cval->v2_mixer | ((validx & 0xff) - 1);
 	}
 
 	value_set = convert_bytes_value(cval, value_set);
@@ -2345,6 +2353,10 @@ static void build_mixer_unit_ctl(struct mixer_build *state,
 
 	snd_usb_mixer_elem_init_std(&cval->head, state->mixer, unitid);
 	cval->control = in_ch + 1; /* based on 1 */
+	/* MCN is calculated later with the input channel based on v2_mixer */
+	if (state->mixer->protocol == UAC_VERSION_2 ||
+	    state->mixer->protocol == UAC_VERSION_3)
+		cval->v2_mixer = (UAC2_MU_MIXER << 8) | (in_ch * num_outs);
 	cval->val_type = USB_MIXER_S16;
 	for (i = 0; i < num_outs; i++) {
 		__u8 *c = uac_mixer_unit_bmControls(desc, state->mixer->protocol);
diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h
index 037b446d8b6f..650f30026177 100644
--- a/sound/usb/mixer.h
+++ b/sound/usb/mixer.h
@@ -97,6 +97,7 @@ struct usb_mixer_elem_info {
 	u8 initialized;
 	u8 min_mute;
 	u8 get_cur_broken;
+	u16 v2_mixer;
 	void *private_data;
 };
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE
  2026-09-16 16:58           ` Takashi Iwai
@ 2026-09-16 18:22             ` Takashi Iwai
  2026-09-17 14:23               ` Zipdox
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Iwai @ 2026-09-16 18:22 UTC (permalink / raw)
  To: Zipdox; +Cc: linux-sound, perex, linux-kernel, tiwai

On Wed, 16 Sep 2026 18:58:50 +0200,
Takashi Iwai wrote:
> 
> On Wed, 16 Sep 2026 17:40:53 +0200,
> Takashi Iwai wrote:
> > 
> > On Wed, 16 Sep 2026 09:43:14 +0200,
> > Zipdox wrote:
> > > 
> > > On 9/15/26 4:19 PM, Takashi Iwai wrote:
> > > > But your code also blindly assumes the MU_MIXER_CONTROL that doesn't
> > > > fit with other units like feature unit?  I guess we have to set the
> > > > proper Control Selector value depending on the unit type.
> > > > 
> > > > After all, this is about the correct setup of wValue.  Let's try to be
> > > > minimalistic at first -- just tweak wValue in get_ctl_value_v2() and
> > > > snd_usb_mixer_set_ctl_value() at first.  If this becomes too ugly, we
> > > > can think of redesigning.
> > > I assumed that mixer.c was just for the mixer unit, and I seem to have
> > > completely overlooked the fact that that it also controls feature
> > > units. I'm not sure how to proceed from here. Do you have any
> > > suggestions for how we can store the control selector in the
> > > usb_mixer_elem_info struct? The control field is presently used as the
> > > input channel number, so maybe we should add a separate field for the
> > > input channel? I think maybe it's better that someone else fixes this,
> > > I'm kind of out of my element here.
> > 
> > I took a look back at the current code, and at least, the parse of
> > UAC2 mixer unit itself is OK -- it handles the bitmap properly.
> > The problem is the exceptional way to set up wValue for the mixer
> > unit, and I guess something like below could work.  It just stores the
> > part of wValue in an additional field, and use it for calculating the
> > correct wValue with MCN + UAC2_MU_MIXER in high byte.
> > 
> > Just compile tested, so not 100% sure whether it's correct.
> 
> ... and this version was obviously half-baked, forgotten the
> counterpart.  A revised patch is below.

... or maybe a less cryptic version like below.


Takashi

-- 8< --
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 33a6a1281410..c6639a45fb24 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -335,6 +335,16 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request,
 	return -EINVAL;
 }
 
+/* convert the given UAC1 wValue (ICN+1|OCN+1) to UAC2 MCN */
+static unsigned int to_mcn(const struct usb_mixer_elem_info *cval,
+			   unsigned int validx)
+{
+	unsigned char m = (validx >> 8) & 0xff;
+	unsigned char v = validx & 0xff;
+
+	return (m - 1) * cval->num_outputs * (v - 1);
+}
+
 static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request,
 			    int validx, int *value_ret)
 {
@@ -347,6 +357,10 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request,
 
 	val_size = uac2_ctl_value_size(cval->val_type);
 
+	/* correct wValue for UAC2 mixer control with MCN */
+	if (cval->v2_mixer)
+		validx =(UAC2_MU_MIXER << 8) | to_mcn(cval, validx);
+
 	if (request == UAC_GET_CUR) {
 		bRequest = UAC2_CS_CUR;
 		size = val_size;
@@ -478,6 +492,10 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
 		}
 
 		request = UAC2_CS_CUR;
+
+		/* correct wValue for UAC2 mixer control with MCN */
+		if (cval->v2_mixer)
+			validx =(UAC2_MU_MIXER << 8) | to_mcn(cval, validx);
 	}
 
 	value_set = convert_bytes_value(cval, value_set);
@@ -2345,6 +2363,10 @@ static void build_mixer_unit_ctl(struct mixer_build *state,
 
 	snd_usb_mixer_elem_init_std(&cval->head, state->mixer, unitid);
 	cval->control = in_ch + 1; /* based on 1 */
+	if (state->mixer->protocol == UAC_VERSION_2 ||
+	    state->mixer->protocol == UAC_VERSION_3)
+		cval->v2_mixer = true;
+	cval->num_outputs = num_outs;
 	cval->val_type = USB_MIXER_S16;
 	for (i = 0; i < num_outs; i++) {
 		__u8 *c = uac_mixer_unit_bmControls(desc, state->mixer->protocol);
diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h
index 037b446d8b6f..cf45c39cbccc 100644
--- a/sound/usb/mixer.h
+++ b/sound/usb/mixer.h
@@ -97,6 +97,8 @@ struct usb_mixer_elem_info {
 	u8 initialized;
 	u8 min_mute;
 	u8 get_cur_broken;
+	u8 num_outputs;
+	bool v2_mixer;
 	void *private_data;
 };
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE
  2026-09-16 18:22             ` Takashi Iwai
@ 2026-09-17 14:23               ` Zipdox
  2026-09-17 14:49                 ` Takashi Iwai
  2026-09-17 14:54                 ` Zipdox
  0 siblings, 2 replies; 14+ messages in thread
From: Zipdox @ 2026-09-17 14:23 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: linux-sound, perex, linux-kernel, tiwai



On 9/16/26 8:22 PM, Takashi Iwai wrote:
> 
> ... or maybe a less cryptic version like below.
> 
> 
> Takashi
> 
> -- 8< --
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index 33a6a1281410..c6639a45fb24 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -335,6 +335,16 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request,
>   	return -EINVAL;
>   }
>   
> +/* convert the given UAC1 wValue (ICN+1|OCN+1) to UAC2 MCN */
> +static unsigned int to_mcn(const struct usb_mixer_elem_info *cval,
> +			   unsigned int validx)
> +{
> +	unsigned char m = (validx >> 8) & 0xff;
> +	unsigned char v = validx & 0xff;
> +
> +	return (m - 1) * cval->num_outputs * (v - 1);
> +}
You used multiplication instead of addition for the last part. I assume 
this is a mistake? Also you should probably also bitwise and that with 
0xFF to make sure it's not more than one byte. The product of the number 
of input and output channels must be no greater than 256 according to 
the spec. I don't know if this is checked elsewhere. Other than that it 
looks good for a patch to get it working. I'll try it out and send a 
follow-up email shortly.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE
  2026-09-17 14:23               ` Zipdox
@ 2026-09-17 14:49                 ` Takashi Iwai
  2026-09-17 14:54                 ` Zipdox
  1 sibling, 0 replies; 14+ messages in thread
From: Takashi Iwai @ 2026-09-17 14:49 UTC (permalink / raw)
  To: Zipdox; +Cc: Takashi Iwai, linux-sound, perex, linux-kernel, tiwai

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

On Thu, 17 Sep 2026 16:23:41 +0200,
Zipdox wrote:
> 
> 
> 
> On 9/16/26 8:22 PM, Takashi Iwai wrote:
> > 
> > ... or maybe a less cryptic version like below.
> > 
> > 
> > Takashi
> > 
> > -- 8< --
> > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> > index 33a6a1281410..c6639a45fb24 100644
> > --- a/sound/usb/mixer.c
> > +++ b/sound/usb/mixer.c
> > @@ -335,6 +335,16 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request,
> >   	return -EINVAL;
> >   }
> >   +/* convert the given UAC1 wValue (ICN+1|OCN+1) to UAC2 MCN */
> > +static unsigned int to_mcn(const struct usb_mixer_elem_info *cval,
> > +			   unsigned int validx)
> > +{
> > +	unsigned char m = (validx >> 8) & 0xff;
> > +	unsigned char v = validx & 0xff;
> > +
> > +	return (m - 1) * cval->num_outputs * (v - 1);
> > +}
> You used multiplication instead of addition for the last part. I
> assume this is a mistake?

Yes, an obvious typo.

> Also you should probably also bitwise and
> that with 0xFF to make sure it's not more than one byte. The product
> of the number of input and output channels must be no greater than 256
> according to the spec. I don't know if this is checked
> elsewhere.

Right, that's already done in another patch, I didn't send the whole
series.  Now attached below.

> Other than that it looks good for a patch to get it
> working. I'll try it out and send a follow-up email shortly.


thanks,

Takashi


[-- Attachment #2: 0001-ALSA-usb-audio-Check-mixer-matrix-size-for-UAC2-3-at.patch --]
[-- Type: application/octet-stream, Size: 1204 bytes --]

From f4276ae989dbfc13209c7630779068dcdec917e8 Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Thu, 17 Sep 2026 10:02:02 +0200
Subject: [PATCH 1/3] ALSA: usb-audio: Check mixer matrix size for UAC2/3 at
 parsing

The matrix of input/output channels specified in a UAC2/3 mixer unit
must fit to the upper limit 256.  Add a sanity check and returns an
error if an invalid size is detected.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/mixer.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 33a6a1281410..ddfd7e01a3ef 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -2436,6 +2436,16 @@ static int parse_audio_mixer_unit(struct mixer_build *state, int unitid,
 	num_outs = err;
 	input_pins = desc->bNrInPins;
 
+	if (state->mixer->protocol == UAC_VERSION_2 ||
+	    state->mixer->protocol == UAC_VERSION_3) {
+		if (input_pins * num_outs > 256) {
+			usb_audio_err(state->chip,
+				      "invalid channels for MIXER UNIT %d: input=%d, output=%d\n",
+				      unitid, input_pins, num_outs);
+			return -EINVAL;
+		}
+	}
+
 	num_ins = 0;
 	ich = 0;
 	for (pin = 0; pin < input_pins; pin++) {
-- 
2.55.0


[-- Attachment #3: 0002-ALSA-usb-audio-Fix-UAC2-mixer-unit-request-handling.patch --]
[-- Type: application/octet-stream, Size: 3532 bytes --]

From be27b714aaa57aa5f07a0c5ebcfc63743231b984 Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Thu, 17 Sep 2026 09:49:19 +0200
Subject: [PATCH 2/3] ALSA: usb-audio: Fix UAC2 mixer unit request handling

For a request for a Mixer Unit on UAC2 (also UAC3), the wValue is
different from UAC1 and an incompatible value must be passed.
Namely, UAC1 takes a word consisting of 1-based input channel in the
high byte and 1-based output channel in the low byte.
Meanwhile, UAC2/3 takes UAC2_MU_MIXER in the high byte and a MCN
(0-based bit position of input/output channels) in the low byte.
The current driver implementation blindly assumes the UAC1 way, hence
it would cause a firmware error.

This patch attempts to implement the conversion to UAC2 MCN at
get_ctl_value_v2() and snd_usb_mixer_set_ctl_value() for mixer units.
At the points above, the old wValue containing ICN and OCN is
converted to the corresponding MCN, and it's used as the proper
wValue.

Reported-by: Zipdox <zipdox@zipdox.net>
Closes: https://lore.kernel.org/d46fcac6-bd7e-4fc4-95e1-4e8d39f92ad3@zipdox.net
Fixes: 23caaf19b11e ("ALSA: usb-mixer: Add support for Audio Class v2.0")
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/mixer.c | 22 ++++++++++++++++++++++
 sound/usb/mixer.h |  2 ++
 2 files changed, 24 insertions(+)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index ddfd7e01a3ef..54a512fb5fa8 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -335,6 +335,16 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request,
 	return -EINVAL;
 }
 
+/* convert the given UAC1 wValue (ICN|OCN) to UAC2 MCN */
+static unsigned int to_mcn(const struct usb_mixer_elem_info *cval,
+			   unsigned int validx)
+{
+	unsigned char m = (validx >> 8) & 0xff; /* 1-based input channel */
+	unsigned char v = validx & 0xff; /* 1-based output channel */
+
+	return (m - 1) * cval->num_outputs + (v - 1);
+}
+
 static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request,
 			    int validx, int *value_ret)
 {
@@ -347,6 +357,10 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request,
 
 	val_size = uac2_ctl_value_size(cval->val_type);
 
+	/* correct wValue for UAC2 mixer control with MCN */
+	if (cval->v2_mixer)
+		validx = (UAC2_MU_MIXER << 8) | to_mcn(cval, validx);
+
 	if (request == UAC_GET_CUR) {
 		bRequest = UAC2_CS_CUR;
 		size = val_size;
@@ -478,6 +492,10 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
 		}
 
 		request = UAC2_CS_CUR;
+
+		/* correct wValue for UAC2 mixer control with MCN */
+		if (cval->v2_mixer)
+			validx = (UAC2_MU_MIXER << 8) | to_mcn(cval, validx);
 	}
 
 	value_set = convert_bytes_value(cval, value_set);
@@ -2345,6 +2363,10 @@ static void build_mixer_unit_ctl(struct mixer_build *state,
 
 	snd_usb_mixer_elem_init_std(&cval->head, state->mixer, unitid);
 	cval->control = in_ch + 1; /* based on 1 */
+	if (state->mixer->protocol == UAC_VERSION_2 ||
+	    state->mixer->protocol == UAC_VERSION_3)
+		cval->v2_mixer = true;
+	cval->num_outputs = num_outs;
 	cval->val_type = USB_MIXER_S16;
 	for (i = 0; i < num_outs; i++) {
 		__u8 *c = uac_mixer_unit_bmControls(desc, state->mixer->protocol);
diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h
index 037b446d8b6f..cf45c39cbccc 100644
--- a/sound/usb/mixer.h
+++ b/sound/usb/mixer.h
@@ -97,6 +97,8 @@ struct usb_mixer_elem_info {
 	u8 initialized;
 	u8 min_mute;
 	u8 get_cur_broken;
+	u8 num_outputs;
+	bool v2_mixer;
 	void *private_data;
 };
 
-- 
2.55.0


[-- Attachment #4: 0003-ALSA-usb-audio-Optimize-min-max-res-parse-for-UAC2.patch --]
[-- Type: application/octet-stream, Size: 9015 bytes --]

From 0f8d881464f94d46687854251056156e492fe35e Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Thu, 17 Sep 2026 11:20:30 +0200
Subject: [PATCH 3/3] ALSA: usb-audio: Optimize min/max/res parse for UAC2

UAC2 feature and mixer units provide the mixer information about
minimum and max channels as well as the resolution in a single
UAC2_CS_RANGE request, but the current code tries to extract each of
them in an old way of UAC1.

This patch refactors the code to optimize the range info extraction
for UAC2.  Now the code for obtaining min/max/res info is done in
get_ctl_range() function.  For UAC1, this will call UAC_GET_MIN,
UAC_GET_MAX and UAC_GET_RES requests, while it calls a single
UAC2_CS_RANGE for UAC2/3.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/mixer.c | 209 ++++++++++++++++++++++++++--------------------
 1 file changed, 117 insertions(+), 92 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 54a512fb5fa8..6af51c50a542 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -304,8 +304,9 @@ static inline int mixer_ctrl_intf(struct usb_mixer_interface *mixer)
 	return get_iface_desc(mixer->hostif)->bInterfaceNumber;
 }
 
-static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request,
-			    int validx, int *value_ret)
+/* send a request for UAC1 feature & mixer unit */
+static int request_ctl_v1(struct usb_mixer_elem_info *cval, u8 request,
+			  int validx, int *value_ret)
 {
 	struct snd_usb_audio *chip = cval->head.mixer->chip;
 	unsigned char buf[2];
@@ -345,98 +346,72 @@ static unsigned int to_mcn(const struct usb_mixer_elem_info *cval,
 	return (m - 1) * cval->num_outputs + (v - 1);
 }
 
-static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request,
-			    int validx, int *value_ret)
+/* send a request for UAC2 feature & mixer unit */
+static int request_ctl_v2(struct usb_mixer_elem_info *cval, u8 request,
+			  int validx, unsigned char *buf, int size)
 {
 	struct snd_usb_audio *chip = cval->head.mixer->chip;
-	/* enough space for one range */
-	unsigned char buf[sizeof(__u16) + 3 * sizeof(__u32)];
-	unsigned char *val;
-	int idx = 0, ret, val_size, size;
-	__u8 bRequest;
+	int idx, ret;
 
-	val_size = uac2_ctl_value_size(cval->val_type);
+	CLASS(snd_usb_lock, pm)(chip);
+	if (pm.err)
+		return -EIO;
 
 	/* correct wValue for UAC2 mixer control with MCN */
 	if (cval->v2_mixer)
 		validx = (UAC2_MU_MIXER << 8) | to_mcn(cval, validx);
 
-	if (request == UAC_GET_CUR) {
-		bRequest = UAC2_CS_CUR;
-		size = val_size;
-	} else {
-		bRequest = UAC2_CS_RANGE;
-		size = sizeof(__u16) + 3 * val_size;
-	}
-
-	memset(buf, 0, sizeof(buf));
-
-	{
-		CLASS(snd_usb_lock, pm)(chip);
-		if (pm.err)
-			return -EIO;
-
-		idx = mixer_ctrl_intf(cval->head.mixer) | (cval->head.id << 8);
-		ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), bRequest,
-				      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
-				      validx, idx, buf, size);
-	}
-
-	if (ret < 0) {
+	memset(buf, 0, size);
+	idx = mixer_ctrl_intf(cval->head.mixer) | (cval->head.id << 8);
+	ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0),
+			      request,
+			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
+			      validx, idx, buf, size);
+	if (ret < 0)
 		usb_audio_dbg(chip,
 			"cannot get ctl value: req = %#x, wValue = %#x, wIndex = %#x, type = %d\n",
 			request, validx, idx, cval->val_type);
+
+	return ret;
+}
+
+/* read the current value for UAC2 */
+static int get_ctl_value_v2(struct usb_mixer_elem_info *cval,
+			    int validx, int *value_ret)
+{
+	/* enough space for one value */
+	unsigned char buf[sizeof(__u32)];
+	int ret, val_size;
+
+	val_size = uac2_ctl_value_size(cval->val_type);
+
+	ret = request_ctl_v2(cval, UAC2_CS_CUR, validx, buf, val_size);
+	if (ret < 0)
 		return ret;
-	}
-
-	/* FIXME: how should we handle multiple triplets here? */
-
-	switch (request) {
-	case UAC_GET_CUR:
-		val = buf;
-		break;
-	case UAC_GET_MIN:
-		val = buf + sizeof(__u16);
-		break;
-	case UAC_GET_MAX:
-		val = buf + sizeof(__u16) + val_size;
-		break;
-	case UAC_GET_RES:
-		val = buf + sizeof(__u16) + val_size * 2;
-		break;
-	default:
-		return -EINVAL;
-	}
 
 	*value_ret = convert_signed_value(cval,
-					  snd_usb_combine_bytes(val, val_size));
-
+					  snd_usb_combine_bytes(buf, val_size));
 	return 0;
 }
 
-static int get_ctl_value(struct usb_mixer_elem_info *cval, int request,
-			 int validx, int *value_ret)
+/* read the current value */
+static int get_cur_ctl_value(struct usb_mixer_elem_info *cval,
+			     int validx, int *value_ret)
 {
 	validx += cval->idx_off;
 
 	return (cval->head.mixer->protocol == UAC_VERSION_1) ?
-		get_ctl_value_v1(cval, request, validx, value_ret) :
-		get_ctl_value_v2(cval, request, validx, value_ret);
-}
-
-static int get_cur_ctl_value(struct usb_mixer_elem_info *cval,
-			     int validx, int *value)
-{
-	return get_ctl_value(cval, UAC_GET_CUR, validx, value);
+		request_ctl_v1(cval, UAC_GET_CUR, validx, value_ret) :
+		get_ctl_value_v2(cval, validx, value_ret);
 }
 
 /* channel = 0: master, 1 = first channel */
 static inline int get_cur_mix_raw(struct usb_mixer_elem_info *cval,
 				  int channel, int *value)
 {
-	return get_ctl_value(cval, UAC_GET_CUR,
-			     (cval->control << 8) | channel,
-			     value);
+	return get_cur_ctl_value(cval,
+				 (cval->control << 8) | channel,
+				 value);
 }
 
 int snd_usb_get_cur_mix_value(struct usb_mixer_elem_info *cval,
@@ -466,6 +441,77 @@ int snd_usb_get_cur_mix_value(struct usb_mixer_elem_info *cval,
 	return 0;
 }
 
+/* extract the mixer min/max/res info from UAC1 feature / mixer unit */
+static int get_ctl_range_v1(struct usb_mixer_elem_info *cval, int validx)
+{
+	int last_valid_res = cval->res;
+
+	if (request_ctl_v1(cval, UAC_GET_MAX, validx, &cval->max) < 0 ||
+	    request_ctl_v1(cval, UAC_GET_MIN, validx, &cval->min) < 0) {
+		usb_audio_err(cval->head.mixer->chip,
+			      "%d:%d: cannot get min/max values for control %d (id %d)\n",
+			      cval->head.id, mixer_ctrl_intf(cval->head.mixer),
+			      cval->control, cval->head.id);
+		return -EAGAIN;
+	}
+
+	if (request_ctl_v1(cval, UAC_GET_RES, validx, &cval->res) < 0) {
+		cval->res = 1;
+		return 0;
+	}
+
+	last_valid_res = cval->res;
+	while (cval->res > 1) {
+		if (snd_usb_mixer_set_ctl_value(cval, UAC_SET_RES,
+						validx, cval->res / 2) < 0)
+			break;
+		cval->res /= 2;
+	}
+	if (request_ctl_v1(cval, UAC_GET_RES, validx, &cval->res) < 0)
+		cval->res = last_valid_res;
+
+	return 0;
+}
+
+/* extract the mixer min/max/res info from UAC2 feature / mixer unit */
+static int get_ctl_range_v2(struct usb_mixer_elem_info *cval, int validx)
+{
+	/* enough space for one range */
+	unsigned char buf[sizeof(__u16) + 3 * sizeof(__u32)];
+	unsigned char *val;
+	int ret, val_size, size;
+
+	val_size = uac2_ctl_value_size(cval->val_type);
+	size = sizeof(__u16) + 3 * val_size;
+
+	ret = request_ctl_v2(cval, UAC2_CS_RANGE, validx, buf, size);
+	if (ret < 0)
+		return ret;
+
+	/* FIXME: how should we handle multiple triplets here? */
+	val = buf + 2;
+	cval->min = convert_signed_value(cval, snd_usb_combine_bytes(val, val_size));
+	val += val_size;
+	cval->max = convert_signed_value(cval, snd_usb_combine_bytes(val, val_size));
+	val += val_size;
+	cval->res = convert_signed_value(cval, snd_usb_combine_bytes(val, val_size));
+	return 0;
+}
+
+/* extract the mixer min/max/res info */
+static int get_ctl_range(struct usb_mixer_elem_info *cval, int validx)
+{
+	switch (cval->head.mixer->protocol) {
+	case UAC_VERSION_1:
+		return get_ctl_range_v1(cval, validx);
+	case UAC_VERSION_2:
+	case UAC_VERSION_3:
+		return get_ctl_range_v2(cval, validx);
+	default:
+		return -EINVAL;
+	}
+}
+
 /*
  * set a mixer value
  */
@@ -1362,32 +1408,11 @@ static int get_min_max_with_quirks(struct usb_mixer_elem_info *cval,
 					break;
 				}
 		}
-		if (get_ctl_value(cval, UAC_GET_MAX, (cval->control << 8) | minchn, &cval->max) < 0 ||
-		    get_ctl_value(cval, UAC_GET_MIN, (cval->control << 8) | minchn, &cval->min) < 0) {
-			usb_audio_err(cval->head.mixer->chip,
-				      "%d:%d: cannot get min/max values for control %d (id %d)\n",
-				   cval->head.id, mixer_ctrl_intf(cval->head.mixer),
-							       cval->control, cval->head.id);
-			return -EAGAIN;
-		}
-		if (get_ctl_value(cval, UAC_GET_RES,
-				  (cval->control << 8) | minchn,
-				  &cval->res) < 0) {
-			cval->res = 1;
-		} else if (cval->head.mixer->protocol == UAC_VERSION_1) {
-			int last_valid_res = cval->res;
 
-			while (cval->res > 1) {
-				if (snd_usb_mixer_set_ctl_value(cval, UAC_SET_RES,
-								(cval->control << 8) | minchn,
-								cval->res / 2) < 0)
-					break;
-				cval->res /= 2;
-			}
-			if (get_ctl_value(cval, UAC_GET_RES,
-					  (cval->control << 8) | minchn, &cval->res) < 0)
-				cval->res = last_valid_res;
-		}
+		ret = get_ctl_range(cval, (cval->control << 8) | minchn);
+		if (ret < 0)
+			return ret;
+
 		if (cval->res == 0)
 			cval->res = 1;
 
-- 
2.55.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE
  2026-09-17 14:23               ` Zipdox
  2026-09-17 14:49                 ` Takashi Iwai
@ 2026-09-17 14:54                 ` Zipdox
  2026-09-17 16:15                   ` Takashi Iwai
  1 sibling, 1 reply; 14+ messages in thread
From: Zipdox @ 2026-09-17 14:54 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: linux-sound, perex, linux-kernel, tiwai

On 9/17/26 4:23 PM, Zipdox wrote:
> 
> 
> On 9/16/26 8:22 PM, Takashi Iwai wrote:
>>
>> ... or maybe a less cryptic version like below.
>>
>>
>> Takashi
>>
>> -- 8< --
>> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
>> index 33a6a1281410..c6639a45fb24 100644
>> --- a/sound/usb/mixer.c
>> +++ b/sound/usb/mixer.c
>> @@ -335,6 +335,16 @@ static int get_ctl_value_v1(struct 
>> usb_mixer_elem_info *cval, int request,
>>       return -EINVAL;
>>   }
>> +/* convert the given UAC1 wValue (ICN+1|OCN+1) to UAC2 MCN */
>> +static unsigned int to_mcn(const struct usb_mixer_elem_info *cval,
>> +               unsigned int validx)
>> +{
>> +    unsigned char m = (validx >> 8) & 0xff;
>> +    unsigned char v = validx & 0xff;
>> +
>> +    return (m - 1) * cval->num_outputs * (v - 1);
>> +}
> You used multiplication instead of addition for the last part. I assume 
> this is a mistake? Also you should probably also bitwise and that with 
> 0xFF to make sure it's not more than one byte. The product of the number 
> of input and output channels must be no greater than 256 according to 
> the spec. I don't know if this is checked elsewhere. Other than that it 
> looks good for a patch to get it working. I'll try it out and send a 
> follow-up email shortly.
> 
If you fix the code like this:

static unsigned int to_mcn(const struct usb_mixer_elem_info *cval,
			   unsigned int validx)
{
	unsigned char icn = (validx >> 8) & 0xff;
	unsigned char ocn = validx & 0xff;

	return (icn - 1) * cval->num_outputs + (ocn - 1);
}

then it works correctly. As far as I'm concerned you can merge that 
as-is. It would be good to try on some other hardware as well to check 
for regressions though.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE
  2026-09-17 14:54                 ` Zipdox
@ 2026-09-17 16:15                   ` Takashi Iwai
  2026-09-17 16:33                     ` Zipdox
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Iwai @ 2026-09-17 16:15 UTC (permalink / raw)
  To: Zipdox; +Cc: Takashi Iwai, linux-sound, perex, linux-kernel, tiwai

On Thu, 17 Sep 2026 16:54:50 +0200,
Zipdox wrote:
> 
> On 9/17/26 4:23 PM, Zipdox wrote:
> > 
> > 
> > On 9/16/26 8:22 PM, Takashi Iwai wrote:
> >> 
> >> ... or maybe a less cryptic version like below.
> >> 
> >> 
> >> Takashi
> >> 
> >> -- 8< --
> >> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> >> index 33a6a1281410..c6639a45fb24 100644
> >> --- a/sound/usb/mixer.c
> >> +++ b/sound/usb/mixer.c
> >> @@ -335,6 +335,16 @@ static int get_ctl_value_v1(struct
> >> usb_mixer_elem_info *cval, int request,
> >>       return -EINVAL;
> >>   }
> >> +/* convert the given UAC1 wValue (ICN+1|OCN+1) to UAC2 MCN */
> >> +static unsigned int to_mcn(const struct usb_mixer_elem_info *cval,
> >> +               unsigned int validx)
> >> +{
> >> +    unsigned char m = (validx >> 8) & 0xff;
> >> +    unsigned char v = validx & 0xff;
> >> +
> >> +    return (m - 1) * cval->num_outputs * (v - 1);
> >> +}
> > You used multiplication instead of addition for the last part. I
> > assume this is a mistake? Also you should probably also bitwise and
> > that with 0xFF to make sure it's not more than one byte. The product
> > of the number of input and output channels must be no greater than
> > 256 according to the spec. I don't know if this is checked
> > elsewhere. Other than that it looks good for a patch to get it
> > working. I'll try it out and send a follow-up email shortly.
> > 
> If you fix the code like this:
> 
> static unsigned int to_mcn(const struct usb_mixer_elem_info *cval,
> 			   unsigned int validx)
> {
> 	unsigned char icn = (validx >> 8) & 0xff;
> 	unsigned char ocn = validx & 0xff;
> 
> 	return (icn - 1) * cval->num_outputs + (ocn - 1);
> }
> 
> then it works correctly. As far as I'm concerned you can merge that
> as-is. It would be good to try on some other hardware as well to check
> for regressions though.

Good to hear.  If you have a chance to test my last series of patches,
it'd be appreciated.

BTW, I'll be off from tomorrow, and the issue is no new bug to be
urgently fixed, so I'll handle after back to work again :)


thanks,

Takashi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE
  2026-09-17 16:15                   ` Takashi Iwai
@ 2026-09-17 16:33                     ` Zipdox
  2026-09-17 16:40                       ` Takashi Iwai
  0 siblings, 1 reply; 14+ messages in thread
From: Zipdox @ 2026-09-17 16:33 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: linux-sound, perex, linux-kernel, tiwai

On 9/17/26 6:15 PM, Takashi Iwai wrote:
> Good to hear.  If you have a chance to test my last series of patches,
> it'd be appreciated.
> 
> BTW, I'll be off from tomorrow, and the issue is no new bug to be
> urgently fixed, so I'll handle after back to work again :)
> 
> 
> thanks,
> 
> Takashi

I have compiled your patches and the mixer unit control seems to work 
correctly.

Another thing on my mind is the following section from section 5.2.5.5.1 
from the spec:
> A Mixer Unit consists of a number of Mixer Controls, either programmable or fixed. A Mixer Control must
> support the CUR and RANGE(MIN, MAX, RES) attributes. The settings for the CUR, MIN, and MAX
> attributes can range from +127.9961 dB (0x7FFF) down to -127.9961 dB (0x8001) in steps of 1/256 dB or
> 0.00390625 dB (0x0001). The settings for the RES attribute can only have positive values and range from
> 1/256 dB (0x0001) to +127.9961 dB (0x7FFF).
> In addition, code 0x8000, representing silence (i.e., -∞ dB), must always be implemented. However, it must
> never be reported as the MIN attribute value.

Is this presently handled? For reference, 0x8000 is -32768. When I use 
qasmixer or alsamixer, for example, it only goes down to -32767. This is 
the same in UAC1 by the way. Should we add a mute control to mixer 
nodes? Or perhaps extend the reported range downward by one, and 
overwrite the lowest value to -32768 when writing to a mixer unit. The 
latter is my preferred solution.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE
  2026-09-17 16:33                     ` Zipdox
@ 2026-09-17 16:40                       ` Takashi Iwai
  0 siblings, 0 replies; 14+ messages in thread
From: Takashi Iwai @ 2026-09-17 16:40 UTC (permalink / raw)
  To: Zipdox; +Cc: Takashi Iwai, linux-sound, perex, linux-kernel, tiwai

On Thu, 17 Sep 2026 18:33:50 +0200,
Zipdox wrote:
> 
> On 9/17/26 6:15 PM, Takashi Iwai wrote:
> > Good to hear.  If you have a chance to test my last series of patches,
> > it'd be appreciated.
> > 
> > BTW, I'll be off from tomorrow, and the issue is no new bug to be
> > urgently fixed, so I'll handle after back to work again :)
> > 
> > 
> > thanks,
> > 
> > Takashi
> 
> I have compiled your patches and the mixer unit control seems to work
> correctly.
> 
> Another thing on my mind is the following section from section
> 5.2.5.5.1 from the spec:
> > A Mixer Unit consists of a number of Mixer Controls, either programmable or fixed. A Mixer Control must
> > support the CUR and RANGE(MIN, MAX, RES) attributes. The settings for the CUR, MIN, and MAX
> > attributes can range from +127.9961 dB (0x7FFF) down to -127.9961 dB (0x8001) in steps of 1/256 dB or
> > 0.00390625 dB (0x0001). The settings for the RES attribute can only have positive values and range from
> > 1/256 dB (0x0001) to +127.9961 dB (0x7FFF).
> > In addition, code 0x8000, representing silence (i.e., -∞ dB), must always be implemented. However, it must
> > never be reported as the MIN attribute value.
> 
> Is this presently handled? For reference, 0x8000 is -32768. When I use
> qasmixer or alsamixer, for example, it only goes down to -32767. This
> is the same in UAC1 by the way. Should we add a mute control to mixer
> nodes? Or perhaps extend the reported range downward by one, and
> overwrite the lowest value to -32768 when writing to a mixer unit. The
> latter is my preferred solution.

I guess it's not; there is cval->min_mute flag, but it's for another
case (the minimal volume = mute) for the statically mapped mixer
elements, and not for this mixer unit case.

Maybe they should be implemented by additional mute switches.

Overall, the mixer unit controls have been rarely used by the devices,
so far, and that's the reason we didn't hit this over years :)


Takashi

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-09-17 16:41 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-15  8:54 [PATCH] USB Audio Class 2 Mixer unit support for GET_CUR, SET_CUR and RANGE Zipdox
2026-09-15 11:08 ` Takashi Iwai
2026-09-15 12:54   ` Zipdox
2026-09-15 14:19     ` Takashi Iwai
2026-09-16  7:43       ` Zipdox
2026-09-16 15:40         ` Takashi Iwai
2026-09-16 16:58           ` Takashi Iwai
2026-09-16 18:22             ` Takashi Iwai
2026-09-17 14:23               ` Zipdox
2026-09-17 14:49                 ` Takashi Iwai
2026-09-17 14:54                 ` Zipdox
2026-09-17 16:15                   ` Takashi Iwai
2026-09-17 16:33                     ` Zipdox
2026-09-17 16:40                       ` Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®