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

Alexander Niemeyer <[email protected]>
Newsgroups gmane.linux.sound,gmane.linux.kernel
Message-ID <[email protected]>
Hi Rong,

I think we found the reason for the different behavior.

I reproduced the snd-usb-audio initialization sequence step by step with 
direct libusb UAC1 control transfers and isolated the problem to SET_RES 
on the *Mic Capture Volume control (Feature Unit 3)*.

A fresh-device control test looks like this:

Mic GET_RES = 256
no SET_RES
Playback:
GET_CUR = -3840 (-15 dB)
SET_CUR = -2048 (-8 dB)
GET_CUR changes to -2048 after 68.0 ms
Result: PASS

After another power cycle, I repeated the same test but issued just 
*one* SET_RES request to the Mic Feature Unit first:

Mic GET_RES before = 256
Mic SET_RES(128) = success
Mic GET_RES after = 256
Playback:
GET_CUR = -3840 (-15 dB)
SET_CUR = -2048 (-8 dB)
GET_CUR remains -3840 for more than 1200 ms
Result: FAIL

So a single successful |SET_RES(128)| on Feature Unit 3 is sufficient to 
make subsequent |SET_CUR| requests to the Playback Volume control on 
Feature Unit 2 ineffective.

I also tested the complete Mic SET_RES sequence used by snd-usb-audio:

SET_RES 128
SET_RES 64
SET_RES 32
SET_RES 16
SET_RES 8
SET_RES 4
SET_RES 2
SET_RES 1

All requests return success, while GET_RES remains 256. After that 
sequence, Playback SET_CUR also remains ineffective for more than 1200 ms.

Interestingly, the Mic control itself still works after this. In an 
ALSA-like Mic probe I could successfully change Mic Volume from 0 dB to 
-64 dB and then +1 dB, with GET_CUR reflecting those changes essentially 
immediately (~0.3 ms). Playback remained broken afterwards.

I also checked whether SET_RES on the Playback Feature Unit itself 
causes the problem. It does not:

Playback GET_RES = 256
SET_RES 128 -> 64 -> 32 -> 16 -> 8 -> 4 -> 2 -> 1
GET_RES still = 256
Playback SET_CUR(-8 dB)
GET_CUR changes successfully after 87.7 ms

So the problematic operation appears specifically to be *SET_RES on the 
Mic Feature Unit affecting the Playback Feature Unit*.

I also clarified the separate advertised-minimum issue:

Playback SET_CUR(-64 dB):
no change after >1200 ms
followed by SET_CUR(-8 dB):
works normally after 54.9 ms
Playback SET_CUR(-63 dB):
works after 75.8 ms
followed by SET_CUR(-8 dB):
works after 43.6 ms

Therefore the broken -64 dB endpoint does not leave the device in the 
broken state; it is a separate issue. -63 dB works normally.

I also captured usbmon/pcapng traces for both a working direct-libusb 
SET_CUR sequence and the failing snd-usb-audio initialization, so I can 
send those as well if they are useful.

This also seems to explain why the sticky-check changes did not help: by 
the time snd-usb-audio reaches the Playback Volume sticky check, the 
earlier Mic SET_RES sanity test has already put the device into the 
state where Playback SET_CUR no longer takes effect.

Let me know if you would like me to test a patch or capture any 
additional traces.

Thanks,
Alexander


Am 16.08.2026 um 17:08 schrieb Rong Zhang:
> 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
>>>>>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.