Re: [PATCH] ALSA: usb-audio: Set the value of potential sticky mixers to maximum

[Rong Zhang]

Hi Takashi,

On Sun, 2026-05-31 at 16:07 +0200, Takashi Iwai wrote:
> On Sun, 31 May 2026 15:55:46 +0200,
> Rong Zhang wrote:
> > > 
> > Hi Takashi,
> > 
> > On Sun, 2026-05-31 at 15:23 +0200, Takashi Iwai wrote:
> > > On Sat, 30 May 2026 21:52:49 +0200,
> > > Rong Zhang wrote:
> > > > 
> > > > It makes no sense to restore the saved value for a sticky mixer, since
> > > > setting any value is a no-op.
> > > > 
> > > > However, in some rare cases, SET_CUR is effective despite GET_CUR always
> > > > returns a constant value. These mixers are not sticky, but there's no
> > > > way to distinguish them. Without any additional information, the best
> > > > thing we can do is to set the mixer value to the maximum before bailing
> > > > out, so that a soft mixer can still reach the maximum hardware volume if
> > > > the mixer turns out to be non-sticky. Meanwhile, all channels must be
> > > > synchronized to prevent imbalance volume.
> > > > 
> > > > Fixes: 86aa1ea1f15c ("ALSA: usb-audio: Do not expose sticky mixers")
> > > > Signed-off-by: Rong Zhang <i@xxxxxxxx>
> > > > ---
> > > >  sound/usb/mixer.c | 33 +++++++++++++++++++++++++++++----
> > > >  1 file changed, 29 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> > > > index 5fba456eb4a9..fb37bb8ad9a9 100644
> > > > --- a/sound/usb/mixer.c
> > > > +++ b/sound/usb/mixer.c
> > > > @@ -1371,10 +1371,8 @@ static int get_min_max_with_quirks(struct usb_mixer_elem_info *cval,
> > > >  				goto no_checks;
> > > >  
> > > >  			ret = check_sticky_volume_control(cval, minchn, saved);
> > > > -			if (ret < 0) {
> > > > -				snd_usb_set_cur_mix_value(cval, minchn, 0, saved);
> > > > -				return ret;
> > > > -			}
> > > > +			if (ret < 0)
> > > > +				goto sticky;
> > > >  
> > > >  			if (cval->min + cval->res < cval->max)
> > > >  				check_volume_control_res(cval, minchn, saved);
> > > > @@ -1431,6 +1429,33 @@ static int get_min_max_with_quirks(struct usb_mixer_elem_info *cval,
> > > >  	}
> > > >  
> > > >  	return 0;
> > > > +
> > > > +sticky:
> > > > +	/*
> > > > +	 * It makes no sense to restore the saved value for a sticky mixer,
> > > > +	 * since setting any value is a no-op.
> > > > +	 *
> > > > +	 * However, in some rare cases, SET_CUR is effective despite GET_CUR
> > > > +	 * always returns a constant value. These mixers are not sticky, but
> > > > +	 * there's no way to distinguish them. Without any additional
> > > > +	 * information, the best thing we can do is to set the mixer value to
> > > > +	 * the maximum before bailing out, so that a soft mixer can still reach
> > > > +	 * the maximum hardware volume if the mixer turns out to be non-sticky.
> > > > +	 * Meanwhile, all channels must be synchronized to prevent imbalance
> > > > +	 * volume.
> > > > +	 */
> > > > +	if (!cval->cmask) {
> > > > +		snd_usb_set_cur_mix_value(cval, 0, 0, cval->max);
> > > > +	} else {
> > > > +		for (i = 0; i < MAX_CHANNELS; i++) {
> > > > +			idx = 0;
> > > > +			if (cval->cmask & BIT(i)) {
> > > > +				snd_usb_set_cur_mix_value(cval, i + 1, idx, cval->max);
> > > > +				idx++;
> > > > +			}
> > > > +		}
> > > > +	}
> > > > +	return ret;
> > > 
> > > This change is supposed to be applied only to 7.1, 
> > > 
> > 
> > This patch is meant for both 7.1 and 7.2.
> > 
> > > and 7.2 should take
> > > another your series ("ALSA: usb-audio: MIXER_GET_CUR_BROKEN related
> > > changes for 7.2") instead, right?
> > > 
> > 
> > Right.
> > 
> > >   If so, at merging the latter, I'll
> > > revert this one as a preliminary.
> > 
> > Please don't revert this one in 7.2.
> > 
> > It is needed to make any future devices like that to at least have
> > audible volume out-of-box. Without it, users will have to wait for an
> > quirk table entry for their devices (or add one themselves). Since
> > setting the mixer value to maximum should be safe on both categories of
> > devices (sticky, effective SET_CUR but broken GET_CUR), I'd prefer to
> > retain this patch in 7.2 too.
> > 
> > Another reason of retaining it is that the current error path only
> > "restores" the garbage GET_CUR value for minchn, so it could lead to
> > imbalance volume if the mixer has multiple channels. In the effective
> > SET_CUR but broken GET_CUR case, we would still have to fix the error
> > path anyway to synchronize all channels. Since this patch does the job
> > and synchronize all channels using a non-garbage value, it should be
> > fine to retain it in 7.2.
> > 
> > That being said, this patch has a trivial single-line conflict in 7.2
> > because my series adding QUIRK_FLAG_MIXER_GET_CUR_BROKEN narrowed the
> > sticky path down to `ret == -ENODEV'. I can resubmit the series for 7.2
> > to include a rebased version of this patch, should you prefer.
> 
> OK, then let me merge this one to for-linus, and merge to for-next
> branch at first.  
> 

Many thanks.

> Then resubmit the patches that can be applied
> cleanly on for-next branch.  I can merge and resolve conflicts
> locally, but this would become a problem when backporting to older
> stable kernels in future.

Oh, I just noticed that you haven't pushed anything to your for-next
branch after merging my QUIRK_FLAG_MIXER_GET_CUR_BROKEN series, so the
ship hasn't sailed yet. Did you mean that you will drop my "ALSA: usb-
audio: Add QUIRK_FLAG_MIXER_GET_CUR_BROKEN for Sennheiser MOMENTUM 3"
from your local branch so that I can rebase and resubmit the series
then?

It sounds good to me. In this way I can also integrate the "ALSA: usb-
audio: MIXER_GET_CUR_BROKEN related changes for 7.2" series into the
QUIRK_FLAG_MIXER_GET_CUR_BROKEN series so things get tidier.

Thanks,
Rong

> 
> 
> thanks,
> 
> Takashi