2021-07-12 06:36:28

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH 5.10 007/593] ALSA: usb-audio: scarlett2: Fix wrong resume call

From: Takashi Iwai <[email protected]>

commit 785b6f29a795f109685f286b91e0250c206fbffb upstream.

The current way of the scarlett2 mixer code managing the
usb_mixer_elem_info object is wrong in two ways: it passes its
internal index to the head.id field, and the val_type field is
uninitialized. This ended up with the wrong execution at the resume
because a bogus unit id is passed wrongly. Also, in the later code
extensions, we'll have more mixer elements, and passing the index will
overflow the unit id size (of 256).

This patch corrects those issues. It introduces a new value type,
USB_MIXER_BESPOKEN, which indicates a non-standard mixer element, and
use this type for all scarlett2 mixer elements, as well as
initializing the fixed unit id 0 for avoiding the overflow.

Tested-by: Geoffrey D. Bennett <[email protected]>
Signed-off-by: Geoffrey D. Bennett <[email protected]>
Cc: <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Iwai <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
sound/usb/mixer.c | 3 +++
sound/usb/mixer.h | 1 +
sound/usb/mixer_scarlett_gen2.c | 7 ++++++-
3 files changed, 10 insertions(+), 1 deletion(-)

--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -3631,6 +3631,9 @@ static int restore_mixer_value(struct us
struct usb_mixer_elem_info *cval = mixer_elem_list_to_info(list);
int c, err, idx;

+ if (cval->val_type == USB_MIXER_BESPOKEN)
+ return 0;
+
if (cval->cmask) {
idx = 0;
for (c = 0; c < MAX_CHANNELS; c++) {
--- a/sound/usb/mixer.h
+++ b/sound/usb/mixer.h
@@ -55,6 +55,7 @@ enum {
USB_MIXER_U16,
USB_MIXER_S32,
USB_MIXER_U32,
+ USB_MIXER_BESPOKEN, /* non-standard type */
};

typedef void (*usb_mixer_elem_dump_func_t)(struct snd_info_buffer *buffer,
--- a/sound/usb/mixer_scarlett_gen2.c
+++ b/sound/usb/mixer_scarlett_gen2.c
@@ -949,10 +949,15 @@ static int scarlett2_add_new_ctl(struct
if (!elem)
return -ENOMEM;

+ /* We set USB_MIXER_BESPOKEN type, so that the core USB mixer code
+ * ignores them for resume and other operations.
+ * Also, the head.id field is set to 0, as we don't use this field.
+ */
elem->head.mixer = mixer;
elem->control = index;
- elem->head.id = index;
+ elem->head.id = 0;
elem->channels = channels;
+ elem->val_type = USB_MIXER_BESPOKEN;

kctl = snd_ctl_new1(ncontrol, elem);
if (!kctl) {



2021-07-13 20:49:03

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 5.10 007/593] ALSA: usb-audio: scarlett2: Fix wrong resume call

Hi!

> This patch corrects those issues. It introduces a new value type,
> USB_MIXER_BESPOKEN, which indicates a non-standard mixer element, and
> use this type for all scarlett2 mixer elements, as well as
> initializing the fixed unit id 0 for avoiding the overflow.

New mixer value is introduced, but printing code in mixer.c is not
updated.

Is something like this needed?


> +++ b/sound/usb/mixer.h
> @@ -55,6 +55,7 @@ enum {
> USB_MIXER_U16,
> USB_MIXER_S32,
> USB_MIXER_U32,
> + USB_MIXER_BESPOKEN, /* non-standard type */
> };
>

Best regards,
Pavel

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 2b5281ef8fca..83d5e4d19128 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -3294,7 +3294,7 @@ static void snd_usb_mixer_dump_cval(struct snd_info_buffer *buffer,
{
struct usb_mixer_elem_info *cval = mixer_elem_list_to_info(list);
static const char * const val_types[] = {
- "BOOLEAN", "INV_BOOLEAN", "S8", "U8", "S16", "U16", "S32", "U32",
+ "BOOLEAN", "INV_BOOLEAN", "S8", "U8", "S16", "U16", "S32", "U32", "BESPOKEN",
};

--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


Attachments:
(No filename) (1.23 kB)
signature.asc (188.00 B)
Digital signature
Download all attachments

2021-07-14 07:02:20

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH 5.10 007/593] ALSA: usb-audio: scarlett2: Fix wrong resume call

On Tue, 13 Jul 2021 22:46:43 +0200,
Pavel Machek wrote:
>
> Hi!
>
> > This patch corrects those issues. It introduces a new value type,
> > USB_MIXER_BESPOKEN, which indicates a non-standard mixer element, and
> > use this type for all scarlett2 mixer elements, as well as
> > initializing the fixed unit id 0 for avoiding the overflow.
>
> New mixer value is introduced, but printing code in mixer.c is not
> updated.
>
> Is something like this needed?

Currently BESPOKEN type doesn't use the standard dump callback, hence
this won't hit, but such a change wouldn't hurt.


thanks,

Takashi

>
>
> > +++ b/sound/usb/mixer.h
> > @@ -55,6 +55,7 @@ enum {
> > USB_MIXER_U16,
> > USB_MIXER_S32,
> > USB_MIXER_U32,
> > + USB_MIXER_BESPOKEN, /* non-standard type */
> > };
> >
>
> Best regards,
> Pavel
>
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index 2b5281ef8fca..83d5e4d19128 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -3294,7 +3294,7 @@ static void snd_usb_mixer_dump_cval(struct snd_info_buffer *buffer,
> {
> struct usb_mixer_elem_info *cval = mixer_elem_list_to_info(list);
> static const char * const val_types[] = {
> - "BOOLEAN", "INV_BOOLEAN", "S8", "U8", "S16", "U16", "S32", "U32",
> + "BOOLEAN", "INV_BOOLEAN", "S8", "U8", "S16", "U16", "S32", "U32", "BESPOKEN",
> };
>
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> [2 Digital signature <application/pgp-signature (7bit)>]
>