[PATCH 0/3] ALSA: usb-audio: Small improvements in mixer handling

[PATCH 0/3] ALSA: usb-audio: Small improvements in mixer handling

[Takashi Iwai]

Hi,

this is a patchset with small fixes for improving the USB-audio mixer
behavior.  Instead of treating the error from a control message
continuously, initialize all at the probe and keep the rest working.

The first two patches are rather fixes for error messages in
get_ctl_value_v2() function, the last one implements the new
behavior.


Takashi

===

Takashi Iwai (3):
  ALSA: usb-audio: Downgrade error message in get_ctl_value_v2()
  ALSA: usb-audio: Drop superfluous error message after disconnection
  ALSA: usb-audio: Initialize every feature unit once at probe time

 sound/usb/mixer.c | 42 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 6 deletions(-)

-- 
2.26.2

[PATCH 1/3] ALSA: usb-audio: Downgrade error message in get_ctl_value_v2()

[Takashi Iwai]

The error message in get_ctl_value_v2() (for UAC2/3) is shown via
KERN_ERR level but it was intended to be rather a debug message as
seen in get_ctl_value_v1() (for UAC1).  This patch downgrade the
printk level.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/mixer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index a2ce535df14b..a20af9243155 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -373,7 +373,7 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request,
 
 	if (ret < 0) {
 error:
-		usb_audio_err(chip,
+		usb_audio_dbg(chip,
 			"cannot get ctl value: req = %#x, wValue = %#x, wIndex = %#x, type = %d\n",
 			request, validx, idx, cval->val_type);
 		return ret;
-- 
2.26.2

[PATCH 2/3] ALSA: usb-audio: Drop superfluous error message after disconnection

[Takashi Iwai]

The error from snd_usb_lock_shutdown() indicates that the device is
disconnected, hence it makes no sense to show any further control
message error in get_ctl_value_v2().  Return the error directly
instead.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/mixer.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index a20af9243155..60d394361a43 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -361,9 +361,8 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request,
 
 	memset(buf, 0, sizeof(buf));
 
-	ret = snd_usb_lock_shutdown(chip) ? -EIO : 0;
-	if (ret)
-		goto error;
+	if (snd_usb_lock_shutdown(chip))
+		return -EIO;
 
 	idx = mixer_ctrl_intf(cval->head.mixer) | (cval->head.id << 8);
 	ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), bRequest,
@@ -372,7 +371,6 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request,
 	snd_usb_unlock_shutdown(chip);
 
 	if (ret < 0) {
-error:
 		usb_audio_dbg(chip,
 			"cannot get ctl value: req = %#x, wValue = %#x, wIndex = %#x, type = %d\n",
 			request, validx, idx, cval->val_type);
-- 
2.26.2

[PATCH 3/3] ALSA: usb-audio: Initialize every feature unit once at probe time

[Takashi Iwai]

So far we used to read the current value of the mixer element
dynamically at the first access, and the error from a GET_CUR message
is treated as a fatal error (unless QUIRK_IGNORE_CTL_ERROR is set).
It's rather inconvenient, as most of GET_CUR errors are no fatal, and
we can continue operation with assumption of some fixed value.

This patch makes the USB-audio driver to change the behavior at probe
time; now it tries to initialize the current value of each mixer
element that is built from a feature unit (those for typically for
mixer volumes and switches).  When a read failure happens, it tries to
set the known minimum value.  After that point, a cached value is used
always, hence we won't hit GET_CUR message error any longer.

The error from GET_CUR message is still shown as a warning normally,
but only once at the probe time, and it'll keep operating.  If the
message is confirmed to be harmless, it can be shut up by
QUIRK_IGNORE_CTL_ERROR quirk flag, too.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/mixer.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 60d394361a43..4555de9830c5 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -1199,12 +1199,32 @@ static void volume_control_quirks(struct usb_mixer_elem_info *cval,
 	}
 }
 
+/* forcibly initialize the current mixer value; if GET_CUR fails, set to
+ * the minimum as default
+ */
+static void init_cur_mix_raw(struct usb_mixer_elem_info *cval, int ch, int idx)
+{
+	int val, err;
+
+	err = snd_usb_get_cur_mix_value(cval, ch, idx, &val);
+	if (!err)
+		return;
+	if (!cval->head.mixer->ignore_ctl_error)
+		usb_audio_warn(cval->head.mixer->chip,
+			       "%d:%d: failed to get current value for ch %d (%d)\n",
+			       cval->head.id, mixer_ctrl_intf(cval->head.mixer),
+			       ch, err);
+	snd_usb_set_cur_mix_value(cval, ch, idx, cval->min);
+}
+
 /*
  * retrieve the minimum and maximum values for the specified control
  */
 static int get_min_max_with_quirks(struct usb_mixer_elem_info *cval,
 				   int default_min, struct snd_kcontrol *kctl)
 {
+	int i, idx;
+
 	/* for failsafe */
 	cval->min = default_min;
 	cval->max = cval->min + 1;
@@ -1217,7 +1237,6 @@ static int get_min_max_with_quirks(struct usb_mixer_elem_info *cval,
 	} else {
 		int minchn = 0;
 		if (cval->cmask) {
-			int i;
 			for (i = 0; i < MAX_CHANNELS; i++)
 				if (cval->cmask & (1 << i)) {
 					minchn = i + 1;
@@ -1318,6 +1337,19 @@ static int get_min_max_with_quirks(struct usb_mixer_elem_info *cval,
 		}
 	}
 
+	/* initialize all elements */
+	if (!cval->cmask) {
+		init_cur_mix_raw(cval, 0, 0);
+	} else {
+		idx = 0;
+		for (i = 0; i < MAX_CHANNELS; i++) {
+			if (cval->cmask & (1 << i)) {
+				init_cur_mix_raw(cval, i + 1, idx);
+				idx++;
+			}
+		}
+	}
+
 	return 0;
 }
 
-- 
2.26.2

Re: [PATCH 0/3] ALSA: usb-audio: Small improvements in mixer handling

[Greg Kroah-Hartman]

On Thu, Oct 14, 2021 at 03:06:33PM +0200, Takashi Iwai wrote:
> Hi,
> 
> this is a patchset with small fixes for improving the USB-audio mixer
> behavior.  Instead of treating the error from a control message
> continuously, initialize all at the probe and keep the rest working.
> 
> The first two patches are rather fixes for error messages in
> get_ctl_value_v2() function, the last one implements the new
> behavior.
> 
> 
> Takashi
> 
> ===
> 
> Takashi Iwai (3):
>   ALSA: usb-audio: Downgrade error message in get_ctl_value_v2()
>   ALSA: usb-audio: Drop superfluous error message after disconnection
>   ALSA: usb-audio: Initialize every feature unit once at probe time
> 
>  sound/usb/mixer.c | 42 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 36 insertions(+), 6 deletions(-)
> 
> -- 
> 2.26.2
> 

These work great for me, combined with the quirk patch I sent:

Tested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 0/3] ALSA: usb-audio: Small improvements in mixer handling

[Takashi Iwai]

On Thu, 14 Oct 2021 16:42:14 +0200,
Greg Kroah-Hartman wrote:
> 
> On Thu, Oct 14, 2021 at 03:06:33PM +0200, Takashi Iwai wrote:
> > Hi,
> > 
> > this is a patchset with small fixes for improving the USB-audio mixer
> > behavior.  Instead of treating the error from a control message
> > continuously, initialize all at the probe and keep the rest working.
> > 
> > The first two patches are rather fixes for error messages in
> > get_ctl_value_v2() function, the last one implements the new
> > behavior.
> > 
> > 
> > Takashi
> > 
> > ===
> > 
> > Takashi Iwai (3):
> >   ALSA: usb-audio: Downgrade error message in get_ctl_value_v2()
> >   ALSA: usb-audio: Drop superfluous error message after disconnection
> >   ALSA: usb-audio: Initialize every feature unit once at probe time
> > 
> >  sound/usb/mixer.c | 42 ++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 36 insertions(+), 6 deletions(-)
> > 
> > -- 
> > 2.26.2
> > 
> 
> These work great for me, combined with the quirk patch I sent:
> 
> Tested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Thanks for quick testing!

As those are no urgent fixes, I'm going to queue them for 5.16.


Takashi

Re: [PATCH 0/3] ALSA: usb-audio: Small improvements in mixer handling

[Greg Kroah-Hartman]

On Thu, Oct 14, 2021 at 05:10:11PM +0200, Takashi Iwai wrote:
> On Thu, 14 Oct 2021 16:42:14 +0200,
> Greg Kroah-Hartman wrote:
> > 
> > On Thu, Oct 14, 2021 at 03:06:33PM +0200, Takashi Iwai wrote:
> > > Hi,
> > > 
> > > this is a patchset with small fixes for improving the USB-audio mixer
> > > behavior.  Instead of treating the error from a control message
> > > continuously, initialize all at the probe and keep the rest working.
> > > 
> > > The first two patches are rather fixes for error messages in
> > > get_ctl_value_v2() function, the last one implements the new
> > > behavior.
> > > 
> > > 
> > > Takashi
> > > 
> > > ===
> > > 
> > > Takashi Iwai (3):
> > >   ALSA: usb-audio: Downgrade error message in get_ctl_value_v2()
> > >   ALSA: usb-audio: Drop superfluous error message after disconnection
> > >   ALSA: usb-audio: Initialize every feature unit once at probe time
> > > 
> > >  sound/usb/mixer.c | 42 ++++++++++++++++++++++++++++++++++++------
> > >  1 file changed, 36 insertions(+), 6 deletions(-)
> > > 
> > > -- 
> > > 2.26.2
> > > 
> > 
> > These work great for me, combined with the quirk patch I sent:
> > 
> > Tested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> Thanks for quick testing!
> 
> As those are no urgent fixes, I'm going to queue them for 5.16.

No rush from my side, thanks!

greg k-h