Re: [PATCH] ALSA: usb-audio: Check sticky mixers precisely

Hi Alexander,

On Sun, 2026-08-16 at 16:09 +0200, Alexander Niemeyer wrote:
> Hi Rong,
> 
> Sure. The libusb tests were direct USB Audio Class 1 control transfers 
> to the headset using libusb/PyUSB, not ALSA mixer operations.
> 
> I accessed Feature Unit 2 on AudioControl interface 0, master channel 0, 
> with the UAC1 Volume control selector:
> 
> wValue = 0x0200 /* Volume control, master channel */
> wIndex = 0x0200 /* Feature Unit 2, interface 0 */
> 
> I used the standard class-specific requests directly, including GET_CUR, 
> GET_MIN, GET_MAX, GET_RES and SET_CUR, with signed 16-bit little-endian 
> volume values in 1/256 dB units.
> 
> The device reported:
> 
> GET_CUR: 0 ( 0 dB in that test)
> GET_MIN: -16384 (-64 dB)
> GET_MAX: 0 ( 0 dB)
> GET_RES: 256 ( 1 dB)
> 
> For the timing tests I issued SET_CUR for a target value and then 
> repeatedly queried GET_CUR until the value changed or the timeout expired.
> 
> Valid values became visible after roughly:
> 
> -1 dB ~81 ms
> -2 dB ~52 ms
> -4 dB ~47 ms
> -8 dB ~47 ms
> -16 dB ~52 ms
> -32 dB ~47 ms
> 
> The advertised -64 dB minimum behaved differently: SET_CUR returned 
> successfully, but GET_CUR did not change even after 1000 ms.
> 
> To access the AudioControl interface with libusb, I unbound the 
> AudioControl interface from snd-usb-audio for the duration of the test.
> 
> I did not intentionally open a playback stream during those libusb 
> tests. Because the AudioControl interface had been unbound from 
> snd-usb-audio, I also do not believe there was an active ALSA playback 
> stream at that point.

Thanks for the information.

Unfortunately, I still don't exactly see why the device behaved
differently when GET_CUR/SET_CUR requests were sent from snd-usb-audio
compared to your libusb tests.

snd-usb-audio also tries SET_RES to test the sanity of GET_RES. Could you
test if it breaks your device's GET_CUR? 

Maybe comparing them with usbmon can show some clues. You can use
Wireshark to sniff /dev/usbmon*. 

Hint: a Thunderbolt port usually corresponds to a dedicated USB root hub.
If you have one, plug the device to it to get pure usbmon trace results
with no noisy URBs from other devices.

Thanks,
Rong

> 
> If the open-stream state is important, I can repeat the experiment 
> specifically controlling for playback-stream-open versus 
> playback-stream-closed.
> 
> Thanks,
> Alexander
> 
> Am 16.08.2026 um 15:50 schrieb Rong Zhang:
> > Hi Alexander,
> > 
> > On Sun, 2026-08-16 at 07:14 +0200, Alexander Niemeyer wrote:
> > > Hi Rong,
> > > 
> > > I tested the sticky-check part of your patch on the Logitech PRO X
> > > Wireless (046d:0aba) on Fedora 44, kernel 7.1.8-200.fc44.x86_64.
> > > 
> > > Since your patch is based on a newer tree, I used a minimal backport of
> > > the new ~16-value / 10 ms sticky-check logic to the 7.1.8 code. The
> > > GET_CUR-broken handling from the newer tree was not included; GET_CUR
> > > itself succeeds on this device.
> > > 
> > > Unfortunately, the playback control is still classified as sticky:
> > > 
> > > 2:0: sticky mixer values (-16384/0/256 => -3840), disabling
> > > 
> > > I then instrumented the check and tried an additional diagnostic: after
> > > every successful SET_CUR, wait 100 ms and perform another GET_CUR before
> > > issuing the next SET_CUR.
> > > 
> > > For the playback volume, the saved value was -3840 and GET_CUR remained
> > > at -3840 for every tested value, even after 100 ms, for example:
> > > 
> > > test=-15104 immediate=-3840 after100ms=-3840
> > > test=-13824 immediate=-3840 after100ms=-3840
> > > test=-3584 immediate=-3840 after100ms=-3840
> > > test=-2304 immediate=-3840 after100ms=-3840
> > > test=-1024 immediate=-3840 after100ms=-3840
> > > test=0 immediate=-3840 after100ms=-3840
> > > 
> > > So in this case the issue does not appear to be simply that the
> > > accumulated 10 ms sleeps are too short. During the probe-time sticky
> > > check, SET_CUR succeeds but GET_CUR for the playback control remains
> > > unchanged even when each SET_CUR is given 100 ms before the next one.
> > > 
> > > This differs from my previous direct libusb tests with the AudioControl
> > > interface unbound, where valid SET_CUR values became visible through
> > > GET_CUR after roughly 47–81 ms.
> > Really interesting. Maybe the mixer changes its value only when there is
> > an opened playback stream.
> > 
> > Could you clarify your "libusb tests"?
> > 
> > Thanks,
> > Rong
> > 
> > > The first debug line I saw with |saved=0| was from the Mic Capture
> > > Volume control; that control changed immediately and returned as
> > > non-sticky. The sequence above with |saved=-3840| is the problematic PCM
> > > Playback Volume control.
> > > 
> > > I'd be happy to test another version or run additional diagnostics if
> > > useful.
> > > 
> > > Best regards,
> > > Alexander
> > > 
> > > 
> > > Am 15.08.2026 um 23:47 schrieb Rong Zhang:
> > > > Some mixers are asynchronous, and some have broken min/max. They are
> > > > mistakenly considered sticky due to how the check is implemented.
> > > > 
> > > > Check sticky mixers more precisely by checking approximately 16 values
> > > > and adding a msleep(10) between each check, so that asynchronous mixers
> > > > have enough time to change the value and mixers with broken min/max are
> > > > checked properly. Additionally, mark GET_CUR as broken when
> > > > get_cur_mix_raw() fails, instead of returning successfully.
> > > > 
> > > > Reported-by: Alexander Niemeyer<[email protected]>
> > > > Closes:https://lore.kernel.org/r/[email protected]
> > > > Signed-off-by: Rong Zhang<[email protected]>
> > > > ---
> > > >    sound/usb/mixer.c | 51 ++++++++++++++++++++++++++++++++++++++++++++-------
> > > >    1 file changed, 44 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> > > > index 703c118f9d4e..3d0f97730a06 100644
> > > > --- a/sound/usb/mixer.c
> > > > +++ b/sound/usb/mixer.c
> > > > @@ -1256,22 +1256,59 @@ static void init_cur_mix_raw(struct usb_mixer_elem_info *cval, int ch, int idx)
> > > >    static int check_sticky_volume_control(struct usb_mixer_elem_info *cval,
> > > >    				       int channel, int saved)
> > > >    {
> > > > -	int sticky_test_values[] = { cval->min, cval->max };
> > > > -	int test, check, i;
> > > > +	int test, check, res;
> > > > +
> > > > +	/*
> > > > +	 * Check approximately 16 values (15 intervals).
> > > > +	 * If the resolution is not fine enough, check fewer values.
> > > > +	 */
> > > > +	res = DIV_ROUND_UP(cval->max - cval->min, 15);
> > > > +	res = res ? roundup(res, cval->res) : cval->res;
> > > > +
> > > > +	/*
> > > > +	 * If (cval->max - cval->min) is not a multiple of cval->res, we still
> > > > +	 * want to test cval->max anyway.
> > > > +	 */
> > > > +	for (test = cval->min; test < cval->max + res; test += res) {
> > > > +		if (test > cval->max)
> > > > +			test = cval->max;
> > > >    
> > > > -	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, channel, 0, test) ||
> > > > -		    get_cur_mix_raw(cval, channel, &check) ||
> > > > -		    check != saved) /* SET_CUR effective, non-sticky. */
> > > > +		if (snd_usb_set_cur_mix_value(cval, channel, 0, test))
> > > > +			return 0;
> > > > +
> > > > +		if (get_cur_mix_raw(cval, channel, &check))
> > > > +			goto get_cur_broken;
> > > > +		if (check != saved) /* SET_CUR effective, non-sticky. */
> > > >    			return 0;
> > > > +
> > > > +		/*
> > > > +		 * Leave some time for asynchronous mixers to change the value.
> > > > +		 *
> > > > +		 * Note that there is no need to wait between SET_CUR and
> > > > +		 * GET_CUR, as we don't care whether the GET_CUR value matches
> > > > +		 * the SET_CUR one. IOW, what we expect is just a GET_CUR value
> > > > +		 * differing from the saved one.
> > > > +		 *
> > > > +		 * Mixers of most devices are synchronous. The should have
> > > > +		 * returned early without extra sleep. Asynchronous mixers will
> > > > +		 * return once the accumulated time is enough for them to change
> > > > +		 * the value.
> > > > +		 */
> > > > +		msleep(10);
> > > >    	}
> > > >    
> > > > +	/* Check again after the last msleep(). */
> > > > +	if (get_cur_mix_raw(cval, channel, &check))
> > > > +		goto get_cur_broken;
> > > > +	if (check != saved)
> > > > +		return 0;
> > > > +
> > > >    	if (cval->head.mixer->chip->quirk_flags & QUIRK_FLAG_MIXER_GET_CUR_BROKEN) {
> > > > +get_cur_broken:
> > > >    		usb_audio_info(cval->head.mixer->chip,
> > > >    			       "%d:%d: broken mixer GET_CUR (%d/%d/%d => %d)\n",
> > > >    			       cval->head.id, mixer_ctrl_intf(cval->head.mixer),
> > > > 
> > > > ---
> > > > base-commit: 3eb40771c00a8488fa6ed2cc1fe203477908bf38
> > > > change-id: 74676fce-uac-precise-sticky-check-94474a22b57d
> > > > 
> > > > Thanks,
> > > > Rong
> > > >