All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ALSA: usb-audio: Check sticky mixers precisely
@ 2026-08-15 21:47 Rong Zhang
  2026-08-16  5:14 ` Alexander Niemeyer
  0 siblings, 1 reply; 8+ messages in thread
From: Rong Zhang @ 2026-08-15 21:47 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai
  Cc: Takashi Iwai, linux-sound, linux-kernel, Alexander Niemeyer,
	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 <adventureFAN@gmx.de>
Closes: https://lore.kernel.org/r/6262cbbd-d1f2-4c9d-a1c7-9c5d12636f4b@gmx.de
Signed-off-by: Rong Zhang <i@rong.moe>
---
 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


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

* Re: [PATCH] ALSA: usb-audio: Check sticky mixers precisely
  2026-08-15 21:47 [PATCH] ALSA: usb-audio: Check sticky mixers precisely Rong Zhang
@ 2026-08-16  5:14 ` Alexander Niemeyer
  2026-08-16 13:50   ` Rong Zhang
  0 siblings, 1 reply; 8+ messages in thread
From: Alexander Niemeyer @ 2026-08-16  5:14 UTC (permalink / raw)
  To: Rong Zhang, Jaroslav Kysela, Takashi Iwai
  Cc: Takashi Iwai, linux-sound, linux-kernel

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.

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 <adventureFAN@gmx.de>
> Closes: https://lore.kernel.org/r/6262cbbd-d1f2-4c9d-a1c7-9c5d12636f4b@gmx.de
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
>   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
>

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

* Re: [PATCH] ALSA: usb-audio: Check sticky mixers precisely
  2026-08-16  5:14 ` Alexander Niemeyer
@ 2026-08-16 13:50   ` Rong Zhang
       [not found]     ` <74ca2e17-8fb8-4ede-8e7e-441be815b5b6@gmx.de>
  0 siblings, 1 reply; 8+ messages in thread
From: Rong Zhang @ 2026-08-16 13:50 UTC (permalink / raw)
  To: Alexander Niemeyer, Jaroslav Kysela, Takashi Iwai
  Cc: Takashi Iwai, linux-sound, linux-kernel

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 <adventureFAN@gmx.de>
> > Closes: https://lore.kernel.org/r/6262cbbd-d1f2-4c9d-a1c7-9c5d12636f4b@gmx.de
> > Signed-off-by: Rong Zhang <i@rong.moe>
> > ---
> >   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
> > 

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

* Re: [PATCH] ALSA: usb-audio: Check sticky mixers precisely
       [not found]     ` <74ca2e17-8fb8-4ede-8e7e-441be815b5b6@gmx.de>
@ 2026-08-16 15:08       ` Rong Zhang
  2026-08-18 14:41         ` Alexander Niemeyer
  0 siblings, 1 reply; 8+ messages in thread
From: Rong Zhang @ 2026-08-16 15:08 UTC (permalink / raw)
  To: Alexander Niemeyer, Jaroslav Kysela, Takashi Iwai
  Cc: Takashi Iwai, linux-sound, linux-kernel

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<adventureFAN@gmx.de>
> > > > Closes:https://lore.kernel.org/r/6262cbbd-d1f2-4c9d-a1c7-9c5d12636f4b@gmx.de
> > > > Signed-off-by: Rong Zhang<i@rong.moe>
> > > > ---
> > > >    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
> > > > 

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

* Re: [PATCH] ALSA: usb-audio: Check sticky mixers precisely
  2026-08-16 15:08       ` Rong Zhang
@ 2026-08-18 14:41         ` Alexander Niemeyer
  2026-08-18 15:24           ` Alexander Niemeyer
  0 siblings, 1 reply; 8+ messages in thread
From: Alexander Niemeyer @ 2026-08-18 14:41 UTC (permalink / raw)
  To: Rong Zhang, Jaroslav Kysela, Takashi Iwai
  Cc: Takashi Iwai, linux-sound, linux-kernel

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<adventureFAN@gmx.de>
>>>>> Closes:https://lore.kernel.org/r/6262cbbd-d1f2-4c9d-a1c7-9c5d12636f4b@gmx.de
>>>>> Signed-off-by: Rong Zhang<i@rong.moe>
>>>>> ---
>>>>>     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
>>>>>

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

* Re: [PATCH] ALSA: usb-audio: Check sticky mixers precisely
  2026-08-18 14:41         ` Alexander Niemeyer
@ 2026-08-18 15:24           ` Alexander Niemeyer
  2026-08-19 16:39             ` Rong Zhang
  0 siblings, 1 reply; 8+ messages in thread
From: Alexander Niemeyer @ 2026-08-18 15:24 UTC (permalink / raw)
  To: Rong Zhang, Jaroslav Kysela, Takashi Iwai
  Cc: Takashi Iwai, linux-sound, linux-kernel

Hi Rong,

one important follow-up to my previous message:

I realized that the detachable microphone boom was not physically connected
to the headset during the direct libusb tests I reported earlier.

I therefore repeated the relevant tests with the boom microphone attached,
and the result changes in an important way.

With the boom microphone attached, on a fresh device:

Mic GET_RES = 256
no SET_RES

Playback:
   GET_CUR = -3840 (-15 dB)
   SET_CUR = -2048 (-8 dB)
   GET_CUR changes to -2048 after 65.3 ms

Result: PASS

After another power cycle, with the boom still attached:

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 changes to -2048 after 66.3 ms

Result: PASS

I also repeated the complete Mic SET_RES sequence:

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, GET_RES remains 256, and Playback 
SET_CUR(-8 dB)
still works afterwards, with GET_CUR changing after 76.0 ms.

However, with the boom microphone attached, the stock Fedora 7.1.8
snd-usb-audio driver still reproduces the original failure:

   sticky mixer values (-16384/0/256 => -3840), disabling

and no PCM Playback Volume control is exposed.

So I need to correct the conclusion from my previous message:

The Mic SET_RES behavior is real, but it depends on whether the detachable
microphone boom is physically connected.

With the boom disconnected:
   a single Mic SET_RES(128) is sufficient to make subsequent Playback
   SET_CUR ineffective for more than 1200 ms.

With the boom connected:
   both a single Mic SET_RES(128) and the full SET_RES sequence leave
   Playback SET_CUR working normally.

Therefore Mic SET_RES alone does not explain the complete snd-usb-audio
failure in all device configurations. There must be another difference in
the full snd-usb-audio initialization path when the boom is attached.

The separate Playback -64 dB minimum issue I reported remains unchanged:
-64 dB is ineffective, -63 dB works, and attempting -64 dB does not prevent
subsequent valid Playback SET_CUR requests from working.

Sorry for not noticing the detachable-microphone state earlier. I wanted to
send this correction as soon as I confirmed it.

I still have the usbmon/pcapng captures and can run further targeted tests
if useful.

Thanks,
Alexander

Am 18.08.2026 um 16:41 schrieb Alexander Niemeyer:
> 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<adventureFAN@gmx.de>
>>>>>> Closes:https://lore.kernel.org/r/6262cbbd-d1f2-4c9d-a1c7-9c5d12636f4b@gmx.de 
>>>>>>
>>>>>> Signed-off-by: Rong Zhang<i@rong.moe>
>>>>>> ---
>>>>>>     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
>>>>>>

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

* Re: [PATCH] ALSA: usb-audio: Check sticky mixers precisely
  2026-08-18 15:24           ` Alexander Niemeyer
@ 2026-08-19 16:39             ` Rong Zhang
  2026-08-19 18:37               ` Alexander Niemeyer
  0 siblings, 1 reply; 8+ messages in thread
From: Rong Zhang @ 2026-08-19 16:39 UTC (permalink / raw)
  To: Alexander Niemeyer, Jaroslav Kysela, Takashi Iwai
  Cc: Takashi Iwai, linux-sound, linux-kernel

Hi Alexander,

On Tue, 2026-08-18 at 17:24 +0200, Alexander Niemeyer wrote:
> Hi Rong,
> 
> one important follow-up to my previous message:
> 
> I realized that the detachable microphone boom was not physically connected
> to the headset during the direct libusb tests I reported earlier.
> 
> I therefore repeated the relevant tests with the boom microphone attached,
> and the result changes in an important way.
> 
> With the boom microphone attached, on a fresh device:
> 
> Mic GET_RES = 256
> no SET_RES
> 
> Playback:
>    GET_CUR = -3840 (-15 dB)
>    SET_CUR = -2048 (-8 dB)
>    GET_CUR changes to -2048 after 65.3 ms
> 
> Result: PASS
> 
> After another power cycle, with the boom still attached:
> 
> 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 changes to -2048 after 66.3 ms
> 
> Result: PASS
> 
> I also repeated the complete Mic SET_RES sequence:
> 
> 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, GET_RES remains 256, and Playback 
> SET_CUR(-8 dB)
> still works afterwards, with GET_CUR changing after 76.0 ms.
> 
> However, with the boom microphone attached, the stock Fedora 7.1.8
> snd-usb-audio driver still reproduces the original failure:
> 
>    sticky mixer values (-16384/0/256 => -3840), disabling
> 
> and no PCM Playback Volume control is exposed.

Could you test my patch with Mic attached?

> 
> So I need to correct the conclusion from my previous message:
> 
> The Mic SET_RES behavior is real, but it depends on whether the detachable
> microphone boom is physically connected.
> 
> With the boom disconnected:
>    a single Mic SET_RES(128) is sufficient to make subsequent Playback
>    SET_CUR ineffective for more than 1200 ms.
> 
> With the boom connected:
>    both a single Mic SET_RES(128) and the full SET_RES sequence leave
>    Playback SET_CUR working normally.
> 

Thanks for your tests.

Let me conclude:

- Mic detached: SET_RES on Mic breaks Playback
- Mic attached: SET_RES on Mic breaks nothing

Still, I doubt if Playback SET_CUR is really broken in the first case.
There is a chance that Playback SET_CUR is effective while GET_CUR
becomes broken.

Could you test:

- Leave the Mic detached
- SET_RES on Mic
- GET_CUR on Playback
- Play some audio, listen to it
- SET_CUR on Playback
- GET_CUR on Playback
- Play some audio again, listen to it, and tell if the physical volume
has changed

The procedure can be achieved by hacking into the snd-usb-audio driver
and nullifying its cache mechanism, so that you can fetch GET_CUR values
from standard ALSA mixer interface.

If you can notice physical volume changes, it implies SET_RES on Mic
break Playback GET_CUR without breaking SET_CUR. Otherwise, it break
Playback SET_CUR.

If it only breaks GET_CUR, it implies the methodology of sticky check is
problematic. In this case I would probably consider demoting the sticky
check's severity.

I am also thinking about how we should demote the severity. Could you
also test if applying QUIRK_FLAG_MIXER_GET_CUR_BROKEN (without this
patch) breaks mixer change notifications? I.e., tune the volume with the
buttons/knobs on the device and see if the audio stack reflects the
change. I guess it will break the notifications, but I would still like
to wait for your confirmation. 

Thanks,
Rong

> Therefore Mic SET_RES alone does not explain the complete snd-usb-audio
> failure in all device configurations. There must be another difference in
> the full snd-usb-audio initialization path when the boom is attached.
> 
> The separate Playback -64 dB minimum issue I reported remains unchanged:
> -64 dB is ineffective, -63 dB works, and attempting -64 dB does not prevent
> subsequent valid Playback SET_CUR requests from working.
> 
> Sorry for not noticing the detachable-microphone state earlier. I wanted to
> send this correction as soon as I confirmed it.
> 
> I still have the usbmon/pcapng captures and can run further targeted tests
> if useful.
> 
> Thanks,
> Alexander
> 
> Am 18.08.2026 um 16:41 schrieb Alexander Niemeyer:
> > 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<adventureFAN@gmx.de>
> > > > > > > Closes:https://lore.kernel.org/r/6262cbbd-d1f2-4c9d-a1c7-9c5d12636f4b@gmx.de 
> > > > > > > 
> > > > > > > Signed-off-by: Rong Zhang<i@rong.moe>
> > > > > > > ---
> > > > > > >     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
> > > > > > > 

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

* Re: [PATCH] ALSA: usb-audio: Check sticky mixers precisely
  2026-08-19 16:39             ` Rong Zhang
@ 2026-08-19 18:37               ` Alexander Niemeyer
  0 siblings, 0 replies; 8+ messages in thread
From: Alexander Niemeyer @ 2026-08-19 18:37 UTC (permalink / raw)
  To: Rong Zhang, Jaroslav Kysela, Takashi Iwai
  Cc: Takashi Iwai, linux-sound, linux-kernel

Hi Rong,

thanks. I have now completed the three tests you asked for.

1. Your sticky-check patch with the detachable Mic boom attached

I tested the same minimal 7.1.8 backport of your ~16-value / 10 ms
sticky-check logic again, this time with the detachable microphone boom
physically attached.

The external snd-usb-audio module was rebuilt and loaded successfully
(the module taint was "O"), and the old 100 ms diagnostic changes were
removed before this test.

The result is still a failure:

   sticky mixer values (-16384/0/256 => -3840), disabling

The PCM control exposes only:

   Capabilities: pswitch pswitch-joined

and no Playback Volume control.

I repeated this once more with another dongle replug, without changing
the module or touching the volume wheel, and got the same result again.

So:

   your sticky-check patch + Mic boom attached
   -> still classified as sticky
   -> reproduced twice


2. Is Playback SET_CUR really broken after Mic SET_RES with the boom 
detached?

For this test I detached the microphone boom.

I used a test version of snd-usb-audio which:

- performed the Mic UAC1 resolution probe,
- kept the Playback Volume control exposed,
- clamped the known-broken advertised -64 dB minimum to -63 dB,
- and bypassed the normal ALSA mixer cache so that mixer reads fetch
   GET_CUR from the device.

The initial Playback value was:

   43 = -20 dB

I played a 3-second 1 kHz test tone and listened to its level.

Then I requested:

   23 = -40 dB

The immediate output from the cset operation still showed:

   values=43

I waited two seconds and issued a fresh cget. It then returned:

   values=23

I played the exact same test tone again, and the second tone was clearly
quieter.

So Playback SET_CUR is physically effective.

At least in this test, the effect after Mic SET_RES with the boom
detached is not that Playback SET_CUR stops working. Instead, GET_CUR
can remain stale for some time after SET_CUR and later reflect the
actual value.

In short:

   before:             43 (-20 dB)
   SET_CUR requested:  23 (-40 dB)
   immediate result:   43
   GET_CUR after 2 s:  23
   physical volume:    clearly quieter


3. QUIRK_FLAG_MIXER_GET_CUR_BROKEN and mixer change notifications

Linux 7.1.8 does not contain QUIRK_FLAG_MIXER_GET_CUR_BROKEN yet, so for
this test I made a minimal backport of the flag behavior to the otherwise
stock 7.1.8 mixer code, without your new sticky-check patch.

I loaded snd-usb-audio with:

   quirk_flags=046d:0aba:mixer_get_cur_broken

The kernel detected the constant GET_CUR condition and kept the Playback
Volume control exposed:

   sticky mixer values (-16384/0/256 => -3840), disabling
   PROXTESTC: treating constant GET_CUR as broken

For this test the microphone boom was attached.

Before turning the physical headset wheel:

   ALSA cget: 56
   wpctl:     Volume 0.72

I then deliberately turned the physical volume wheel further down.

Afterwards:

   ALSA cget: 47
   wpctl:     Volume 0.52

KDE's volume OSD also reacted normally while I turned the wheel.

So, on this device, using MIXER_GET_CUR_BROKEN did not break propagation
of physical mixer changes through the audio stack:

   physical wheel
   -> ALSA value changed
   -> PipeWire value changed
   -> KDE OSD reacted normally


Therefore the current picture seems to be:

- Your revised sticky check still rejects the Playback Volume control,
   both with the detachable Mic boom attached and detached.

- With the Mic boom detached, Mic SET_RES can make Playback GET_CUR stay
   stale after a Playback SET_CUR, but Playback SET_CUR itself is still
   physically effective.

- At least on this Logitech device, treating GET_CUR as broken did not
   prevent physical wheel changes from reaching ALSA/PipeWire/KDE.

Let me know what you would like me to test next. I have kept the Fedora
test environment and module-signing setup intact.

Thanks,
Alexander

Am 19.08.2026 um 18:39 schrieb Rong Zhang:
> Hi Alexander,
>
> On Tue, 2026-08-18 at 17:24 +0200, Alexander Niemeyer wrote:
>> Hi Rong,
>>
>> one important follow-up to my previous message:
>>
>> I realized that the detachable microphone boom was not physically connected
>> to the headset during the direct libusb tests I reported earlier.
>>
>> I therefore repeated the relevant tests with the boom microphone attached,
>> and the result changes in an important way.
>>
>> With the boom microphone attached, on a fresh device:
>>
>> Mic GET_RES = 256
>> no SET_RES
>>
>> Playback:
>>     GET_CUR = -3840 (-15 dB)
>>     SET_CUR = -2048 (-8 dB)
>>     GET_CUR changes to -2048 after 65.3 ms
>>
>> Result: PASS
>>
>> After another power cycle, with the boom still attached:
>>
>> 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 changes to -2048 after 66.3 ms
>>
>> Result: PASS
>>
>> I also repeated the complete Mic SET_RES sequence:
>>
>> 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, GET_RES remains 256, and Playback
>> SET_CUR(-8 dB)
>> still works afterwards, with GET_CUR changing after 76.0 ms.
>>
>> However, with the boom microphone attached, the stock Fedora 7.1.8
>> snd-usb-audio driver still reproduces the original failure:
>>
>>     sticky mixer values (-16384/0/256 => -3840), disabling
>>
>> and no PCM Playback Volume control is exposed.
> Could you test my patch with Mic attached?
>
>> So I need to correct the conclusion from my previous message:
>>
>> The Mic SET_RES behavior is real, but it depends on whether the detachable
>> microphone boom is physically connected.
>>
>> With the boom disconnected:
>>     a single Mic SET_RES(128) is sufficient to make subsequent Playback
>>     SET_CUR ineffective for more than 1200 ms.
>>
>> With the boom connected:
>>     both a single Mic SET_RES(128) and the full SET_RES sequence leave
>>     Playback SET_CUR working normally.
>>
> Thanks for your tests.
>
> Let me conclude:
>
> - Mic detached: SET_RES on Mic breaks Playback
> - Mic attached: SET_RES on Mic breaks nothing
>
> Still, I doubt if Playback SET_CUR is really broken in the first case.
> There is a chance that Playback SET_CUR is effective while GET_CUR
> becomes broken.
>
> Could you test:
>
> - Leave the Mic detached
> - SET_RES on Mic
> - GET_CUR on Playback
> - Play some audio, listen to it
> - SET_CUR on Playback
> - GET_CUR on Playback
> - Play some audio again, listen to it, and tell if the physical volume
> has changed
>
> The procedure can be achieved by hacking into the snd-usb-audio driver
> and nullifying its cache mechanism, so that you can fetch GET_CUR values
> from standard ALSA mixer interface.
>
> If you can notice physical volume changes, it implies SET_RES on Mic
> break Playback GET_CUR without breaking SET_CUR. Otherwise, it break
> Playback SET_CUR.
>
> If it only breaks GET_CUR, it implies the methodology of sticky check is
> problematic. In this case I would probably consider demoting the sticky
> check's severity.
>
> I am also thinking about how we should demote the severity. Could you
> also test if applying QUIRK_FLAG_MIXER_GET_CUR_BROKEN (without this
> patch) breaks mixer change notifications? I.e., tune the volume with the
> buttons/knobs on the device and see if the audio stack reflects the
> change. I guess it will break the notifications, but I would still like
> to wait for your confirmation.
>
> Thanks,
> Rong
>
>> Therefore Mic SET_RES alone does not explain the complete snd-usb-audio
>> failure in all device configurations. There must be another difference in
>> the full snd-usb-audio initialization path when the boom is attached.
>>
>> The separate Playback -64 dB minimum issue I reported remains unchanged:
>> -64 dB is ineffective, -63 dB works, and attempting -64 dB does not prevent
>> subsequent valid Playback SET_CUR requests from working.
>>
>> Sorry for not noticing the detachable-microphone state earlier. I wanted to
>> send this correction as soon as I confirmed it.
>>
>> I still have the usbmon/pcapng captures and can run further targeted tests
>> if useful.
>>
>> Thanks,
>> Alexander
>>
>> Am 18.08.2026 um 16:41 schrieb Alexander Niemeyer:
>>> 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<adventureFAN@gmx.de>
>>>>>>>> Closes:https://lore.kernel.org/r/6262cbbd-d1f2-4c9d-a1c7-9c5d12636f4b@gmx.de
>>>>>>>>
>>>>>>>> Signed-off-by: Rong Zhang<i@rong.moe>
>>>>>>>> ---
>>>>>>>>      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
>>>>>>>>

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

end of thread, other threads:[~2026-08-19 18:37 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-15 21:47 [PATCH] ALSA: usb-audio: Check sticky mixers precisely Rong Zhang
2026-08-16  5:14 ` Alexander Niemeyer
2026-08-16 13:50   ` Rong Zhang
     [not found]     ` <74ca2e17-8fb8-4ede-8e7e-441be815b5b6@gmx.de>
2026-08-16 15:08       ` Rong Zhang
2026-08-18 14:41         ` Alexander Niemeyer
2026-08-18 15:24           ` Alexander Niemeyer
2026-08-19 16:39             ` Rong Zhang
2026-08-19 18:37               ` Alexander Niemeyer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.