Re: [PATCH 2/2] ALSA: usb-audio: Do not expose sticky volume control mixers

[Takashi Iwai <tiwai@suse.de>]

On Wed, 08 Apr 2026 20:33:06 +0200,
Rong Zhang wrote:
> 
> Some devices expose sticky mixers that accept SET_CUR but do absolutely
> nothing. Registering mixers for them confuses userspace and results in
> ineffective volume control.
> 
> Check if the volume control is sticky by setting the volume to the
> maximum or minimum value, and prevent the mixer from being registered
> accordingly.
> 
> Quirky device sample:
> 
>   usb 7-1: New USB device found, idVendor=0e0b, idProduct=fa01, bcdDevice= 1.00
>   usb 7-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
>   usb 7-1: Product: Feaulle Rainbow
>   usb 7-1: Manufacturer: Generic
>   usb 7-1: SerialNumber: 20210726905926
>   (Mic Capture Volume)
> 
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
>  sound/usb/mixer.c | 37 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index a25e8145af67..9f0aed36e27d 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -27,6 +27,7 @@
>   *  	- parse available sample rates again when clock sources changed
>   */
>  
> +#include <linux/array_size.h>

It's only for ARRAY_SIZE()?  Then no need for extra inclusion.  It's
already included by others.

>  #include <linux/bitops.h>
>  #include <linux/init.h>
>  #include <linux/list.h>
> @@ -1287,17 +1288,49 @@ static int get_min_max_with_quirks(struct usb_mixer_elem_info *cval,
>  		if (cval->res == 0)
>  			cval->res = 1;
>  
> -		/* Additional checks for the proper resolution
> +		/* Additional checks
> +		 *
> +		 * Some devices expose sticky mixers that accept SET_CUR
> +		 * but do absolutely nothing.
>  		 *
>  		 * Some devices report smaller resolutions than actually
>  		 * reacting.  They don't return errors but simply clip
>  		 * to the lower aligned value.
>  		 */
> -		if (cval->min + cval->res < cval->max) {
> +		if (cval->min < cval->max) {
> +			int sticky_test_values[] = { cval->min, cval->max };
>  			int last_valid_res = cval->res;
>  			int saved, test, check;
> +			bool effective = false;
> +
>  			if (get_cur_mix_raw(cval, minchn, &saved) < 0)
>  				goto no_res_check;
> +
> +			for (i = 0; i < ARRAY_SIZE(sticky_test_values); i++) {
> +				test = sticky_test_values[i];
> +				if (test == saved)
> +					continue;
> +				/* Assume non-sticky on failure. */
> +				if (snd_usb_set_cur_mix_value(cval, minchn, 0, test) ||
> +				    get_cur_mix_raw(cval, minchn, &check) ||
> +				    check != saved) { /* SET_CUR effective, non-sticky. */
> +					effective = true;
> +					break;
> +				}
> +			}
> +			if (!effective) {
> +				usb_audio_warn(cval->head.mixer->chip,
> +					"%d:%d: sticky mixer values (%d/%d/%d => %d), disabling\n",
> +					cval->head.id, mixer_ctrl_intf(cval->head.mixer),
> +					cval->min, cval->max, cval->res, saved);
> +
> +				cval->initialized = 1;
> +				cval->min = cval->max = cval->res = 0;
> +				return -ENODEV;
> +			}
> +
> +			if (cval->min + cval->res >= cval->max)
> +				goto no_res_check;

Hm, it's quite lots of changes, and I'd like this to be factored out
as a function instead.  Maybe the resolution check code could be moved
into a function, too; get_min_max_with_quirks() is too lengthy.

Also, note that currently there is no error check for get_min_max*().
Your code works just because you set cval->min = cval->max, and there
is an additional check at a later point of this.

Ideally speaking, we should have an error check to explicitly handle a
case like this.  But the current code allows the error at init, at
least, we tolerate the errors at reading UAC_GET_MIN and _MAX.  So, if
any, the error check would need to evaluate the error number and
ignore -EINVAL or such...


thanks,

Takashi