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

[Takashi Iwai]

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.  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.


thanks,

Takashi