[PATCH v3 1/2] ALSA: usb-audio: Carve out connector value checking into a helper

[PATCH v3 1/2] ALSA: usb-audio: Carve out connector value checking into a helper

[Kai-Heng Feng]

This is preparation for next patch, no functional change intended.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v3:
 - No change.
v2:
 - Only return early when ret < 0.

 sound/usb/mixer.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index b004b2e63a5d..5a2d9a768f70 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -1446,13 +1446,11 @@ static int mixer_ctl_master_bool_get(struct snd_kcontrol *kcontrol,
 	return 0;
 }
 
-/* get the connectors status and report it as boolean type */
-static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol,
-				   struct snd_ctl_elem_value *ucontrol)
+static int get_connector_value(struct usb_mixer_elem_info *cval,
+			       char *name, int *val)
 {
-	struct usb_mixer_elem_info *cval = kcontrol->private_data;
 	struct snd_usb_audio *chip = cval->head.mixer->chip;
-	int idx = 0, validx, ret, val;
+	int idx = 0, validx, ret;
 
 	validx = cval->control << 8 | 0;
 
@@ -1467,21 +1465,24 @@ static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol,
 		ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), UAC2_CS_CUR,
 				      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
 				      validx, idx, &uac2_conn, sizeof(uac2_conn));
-		val = !!uac2_conn.bNrChannels;
+		if (val)
+			*val = !!uac2_conn.bNrChannels;
 	} else { /* UAC_VERSION_3 */
 		struct uac3_insertion_ctl_blk uac3_conn;
 
 		ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), UAC2_CS_CUR,
 				      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
 				      validx, idx, &uac3_conn, sizeof(uac3_conn));
-		val = !!uac3_conn.bmConInserted;
+		if (val)
+			*val = !!uac3_conn.bmConInserted;
 	}
 
 	snd_usb_unlock_shutdown(chip);
 
 	if (ret < 0) {
-		if (strstr(kcontrol->id.name, "Speaker")) {
-			ucontrol->value.integer.value[0] = 1;
+		if (name && strstr(name, "Speaker")) {
+			if (val)
+				*val = 1;
 			return 0;
 		}
 error:
@@ -1491,6 +1492,21 @@ static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol,
 		return filter_error(cval, ret);
 	}
 
+	return ret;
+}
+
+/* get the connectors status and report it as boolean type */
+static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol,
+				   struct snd_ctl_elem_value *ucontrol)
+{
+	struct usb_mixer_elem_info *cval = kcontrol->private_data;
+	int ret, val;
+
+	ret = get_connector_value(cval, kcontrol->id.name, &val);
+
+	if (ret < 0)
+		return ret;
+
 	ucontrol->value.integer.value[0] = val;
 	return 0;
 }
-- 
2.30.2

[PATCH v3 1/2] ALSA: usb-audio: Carve out connector value checking into a helper

[Kai-Heng Feng]

This is preparation for next patch, no functional change intended.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v3:
 - No change.
v2:
 - Only return early when ret < 0.

 sound/usb/mixer.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index b004b2e63a5d..5a2d9a768f70 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -1446,13 +1446,11 @@ static int mixer_ctl_master_bool_get(struct snd_kcontrol *kcontrol,
 	return 0;
 }
 
-/* get the connectors status and report it as boolean type */
-static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol,
-				   struct snd_ctl_elem_value *ucontrol)
+static int get_connector_value(struct usb_mixer_elem_info *cval,
+			       char *name, int *val)
 {
-	struct usb_mixer_elem_info *cval = kcontrol->private_data;
 	struct snd_usb_audio *chip = cval->head.mixer->chip;
-	int idx = 0, validx, ret, val;
+	int idx = 0, validx, ret;
 
 	validx = cval->control << 8 | 0;
 
@@ -1467,21 +1465,24 @@ static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol,
 		ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), UAC2_CS_CUR,
 				      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
 				      validx, idx, &uac2_conn, sizeof(uac2_conn));
-		val = !!uac2_conn.bNrChannels;
+		if (val)
+			*val = !!uac2_conn.bNrChannels;
 	} else { /* UAC_VERSION_3 */
 		struct uac3_insertion_ctl_blk uac3_conn;
 
 		ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), UAC2_CS_CUR,
 				      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
 				      validx, idx, &uac3_conn, sizeof(uac3_conn));
-		val = !!uac3_conn.bmConInserted;
+		if (val)
+			*val = !!uac3_conn.bmConInserted;
 	}
 
 	snd_usb_unlock_shutdown(chip);
 
 	if (ret < 0) {
-		if (strstr(kcontrol->id.name, "Speaker")) {
-			ucontrol->value.integer.value[0] = 1;
+		if (name && strstr(name, "Speaker")) {
+			if (val)
+				*val = 1;
 			return 0;
 		}
 error:
@@ -1491,6 +1492,21 @@ static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol,
 		return filter_error(cval, ret);
 	}
 
+	return ret;
+}
+
+/* get the connectors status and report it as boolean type */
+static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol,
+				   struct snd_ctl_elem_value *ucontrol)
+{
+	struct usb_mixer_elem_info *cval = kcontrol->private_data;
+	int ret, val;
+
+	ret = get_connector_value(cval, kcontrol->id.name, &val);
+
+	if (ret < 0)
+		return ret;
+
 	ucontrol->value.integer.value[0] = val;
 	return 0;
 }
-- 
2.30.2

[PATCH v3 2/2] ALSA: usb-audio: Check connector value on resume

[Kai-Heng Feng]

Rear Mic on Lenovo P620 cannot record after S3, despite that there's no
error and the other two functions of the USB audio, Line In and Line
Out, work just fine.

The mic starts to work again after running userspace app like "alsactl
store". Following the lead, the evidence shows that as soon as connector
status is queried, the mic can work again.

So also check connector value on resume to "wake up" the USB audio to
make it functional.

This can be device specific, however I think this generic approach may
benefit more than one device.

Now the resume callback checks connector, and a new callback,
reset_resume, to also restore switches and volumes.

Suggested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v3:
 - New callback to handle resume and reset-resume separately.

v2:
 - Remove reset-resume.
 - Fold the connector checking to the mixer resume callback.

 sound/usb/mixer.c        | 44 +++++++++++++++++++++++++++++++---------
 sound/usb/mixer.h        |  1 +
 sound/usb/mixer_quirks.c |  2 +-
 3 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 5a2d9a768f70..2faf5767c7f8 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -3631,20 +3631,43 @@ static int restore_mixer_value(struct usb_mixer_elem_list *list)
 	return 0;
 }
 
+static int default_mixer_resume(struct usb_mixer_elem_list *list)
+{
+	struct usb_mixer_elem_info *cval = mixer_elem_list_to_info(list);
+
+	/* get connector value to "wake up" the USB audio */
+	if (cval->val_type == USB_MIXER_BOOLEAN && cval->channels == 1)
+		get_connector_value(cval, NULL, NULL);
+
+	return 0;
+}
+
+static int default_mixer_reset_resume(struct usb_mixer_elem_list *list)
+{
+	int err = default_mixer_resume(list);
+
+	if (err < 0)
+		return err;
+	return restore_mixer_value(list);
+}
+
 int snd_usb_mixer_resume(struct usb_mixer_interface *mixer, bool reset_resume)
 {
 	struct usb_mixer_elem_list *list;
+	usb_mixer_elem_resume_func_t f;
 	int id, err;
 
-	if (reset_resume) {
-		/* restore cached mixer values */
-		for (id = 0; id < MAX_ID_ELEMS; id++) {
-			for_each_mixer_elem(list, mixer, id) {
-				if (list->resume) {
-					err = list->resume(list);
-					if (err < 0)
-						return err;
-				}
+	/* restore cached mixer values */
+	for (id = 0; id < MAX_ID_ELEMS; id++) {
+		for_each_mixer_elem(list, mixer, id) {
+			if (reset_resume)
+				f = list->reset_resume;
+			else
+				f = list->resume;
+			if (f) {
+				err = f(list);
+				if (err < 0)
+					return err;
 			}
 		}
 	}
@@ -3663,6 +3686,7 @@ void snd_usb_mixer_elem_init_std(struct usb_mixer_elem_list *list,
 	list->id = unitid;
 	list->dump = snd_usb_mixer_dump_cval;
 #ifdef CONFIG_PM
-	list->resume = restore_mixer_value;
+	list->resume = default_mixer_resume;
+	list->reset_resume = default_mixer_reset_resume;
 #endif
 }
diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h
index c29e27ac43a7..e5a01f17bf3c 100644
--- a/sound/usb/mixer.h
+++ b/sound/usb/mixer.h
@@ -69,6 +69,7 @@ struct usb_mixer_elem_list {
 	bool is_std_info;
 	usb_mixer_elem_dump_func_t dump;
 	usb_mixer_elem_resume_func_t resume;
+	usb_mixer_elem_resume_func_t reset_resume;
 };
 
 /* iterate over mixer element list of the given unit id */
diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
index ffd922327ae4..b7f9c2fded05 100644
--- a/sound/usb/mixer_quirks.c
+++ b/sound/usb/mixer_quirks.c
@@ -151,7 +151,7 @@ static int add_single_ctl_with_resume(struct usb_mixer_interface *mixer,
 		*listp = list;
 	list->mixer = mixer;
 	list->id = id;
-	list->resume = resume;
+	list->reset_resume = resume;
 	kctl = snd_ctl_new1(knew, list);
 	if (!kctl) {
 		kfree(list);
-- 
2.30.2

[PATCH v3 2/2] ALSA: usb-audio: Check connector value on resume

[Kai-Heng Feng]

Rear Mic on Lenovo P620 cannot record after S3, despite that there's no
error and the other two functions of the USB audio, Line In and Line
Out, work just fine.

The mic starts to work again after running userspace app like "alsactl
store". Following the lead, the evidence shows that as soon as connector
status is queried, the mic can work again.

So also check connector value on resume to "wake up" the USB audio to
make it functional.

This can be device specific, however I think this generic approach may
benefit more than one device.

Now the resume callback checks connector, and a new callback,
reset_resume, to also restore switches and volumes.

Suggested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v3:
 - New callback to handle resume and reset-resume separately.

v2:
 - Remove reset-resume.
 - Fold the connector checking to the mixer resume callback.

 sound/usb/mixer.c        | 44 +++++++++++++++++++++++++++++++---------
 sound/usb/mixer.h        |  1 +
 sound/usb/mixer_quirks.c |  2 +-
 3 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 5a2d9a768f70..2faf5767c7f8 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -3631,20 +3631,43 @@ static int restore_mixer_value(struct usb_mixer_elem_list *list)
 	return 0;
 }
 
+static int default_mixer_resume(struct usb_mixer_elem_list *list)
+{
+	struct usb_mixer_elem_info *cval = mixer_elem_list_to_info(list);
+
+	/* get connector value to "wake up" the USB audio */
+	if (cval->val_type == USB_MIXER_BOOLEAN && cval->channels == 1)
+		get_connector_value(cval, NULL, NULL);
+
+	return 0;
+}
+
+static int default_mixer_reset_resume(struct usb_mixer_elem_list *list)
+{
+	int err = default_mixer_resume(list);
+
+	if (err < 0)
+		return err;
+	return restore_mixer_value(list);
+}
+
 int snd_usb_mixer_resume(struct usb_mixer_interface *mixer, bool reset_resume)
 {
 	struct usb_mixer_elem_list *list;
+	usb_mixer_elem_resume_func_t f;
 	int id, err;
 
-	if (reset_resume) {
-		/* restore cached mixer values */
-		for (id = 0; id < MAX_ID_ELEMS; id++) {
-			for_each_mixer_elem(list, mixer, id) {
-				if (list->resume) {
-					err = list->resume(list);
-					if (err < 0)
-						return err;
-				}
+	/* restore cached mixer values */
+	for (id = 0; id < MAX_ID_ELEMS; id++) {
+		for_each_mixer_elem(list, mixer, id) {
+			if (reset_resume)
+				f = list->reset_resume;
+			else
+				f = list->resume;
+			if (f) {
+				err = f(list);
+				if (err < 0)
+					return err;
 			}
 		}
 	}
@@ -3663,6 +3686,7 @@ void snd_usb_mixer_elem_init_std(struct usb_mixer_elem_list *list,
 	list->id = unitid;
 	list->dump = snd_usb_mixer_dump_cval;
 #ifdef CONFIG_PM
-	list->resume = restore_mixer_value;
+	list->resume = default_mixer_resume;
+	list->reset_resume = default_mixer_reset_resume;
 #endif
 }
diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h
index c29e27ac43a7..e5a01f17bf3c 100644
--- a/sound/usb/mixer.h
+++ b/sound/usb/mixer.h
@@ -69,6 +69,7 @@ struct usb_mixer_elem_list {
 	bool is_std_info;
 	usb_mixer_elem_dump_func_t dump;
 	usb_mixer_elem_resume_func_t resume;
+	usb_mixer_elem_resume_func_t reset_resume;
 };
 
 /* iterate over mixer element list of the given unit id */
diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
index ffd922327ae4..b7f9c2fded05 100644
--- a/sound/usb/mixer_quirks.c
+++ b/sound/usb/mixer_quirks.c
@@ -151,7 +151,7 @@ static int add_single_ctl_with_resume(struct usb_mixer_interface *mixer,
 		*listp = list;
 	list->mixer = mixer;
 	list->id = id;
-	list->resume = resume;
+	list->reset_resume = resume;
 	kctl = snd_ctl_new1(knew, list);
 	if (!kctl) {
 		kfree(list);
-- 
2.30.2

Re: [PATCH v3 1/2] ALSA: usb-audio: Carve out connector value checking into a helper

[Takashi Iwai]

On Thu, 25 Mar 2021 17:59:12 +0100,
Kai-Heng Feng wrote:
> 
> This is preparation for next patch, no functional change intended.
> 
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

Applied now.  Thanks.


Takashi

> ---
> v3:
>  - No change.
> v2:
>  - Only return early when ret < 0.
> 
>  sound/usb/mixer.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index b004b2e63a5d..5a2d9a768f70 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -1446,13 +1446,11 @@ static int mixer_ctl_master_bool_get(struct snd_kcontrol *kcontrol,
>  	return 0;
>  }
>  
> -/* get the connectors status and report it as boolean type */
> -static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol,
> -				   struct snd_ctl_elem_value *ucontrol)
> +static int get_connector_value(struct usb_mixer_elem_info *cval,
> +			       char *name, int *val)
>  {
> -	struct usb_mixer_elem_info *cval = kcontrol->private_data;
>  	struct snd_usb_audio *chip = cval->head.mixer->chip;
> -	int idx = 0, validx, ret, val;
> +	int idx = 0, validx, ret;
>  
>  	validx = cval->control << 8 | 0;
>  
> @@ -1467,21 +1465,24 @@ static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol,
>  		ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), UAC2_CS_CUR,
>  				      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
>  				      validx, idx, &uac2_conn, sizeof(uac2_conn));
> -		val = !!uac2_conn.bNrChannels;
> +		if (val)
> +			*val = !!uac2_conn.bNrChannels;
>  	} else { /* UAC_VERSION_3 */
>  		struct uac3_insertion_ctl_blk uac3_conn;
>  
>  		ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), UAC2_CS_CUR,
>  				      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
>  				      validx, idx, &uac3_conn, sizeof(uac3_conn));
> -		val = !!uac3_conn.bmConInserted;
> +		if (val)
> +			*val = !!uac3_conn.bmConInserted;
>  	}
>  
>  	snd_usb_unlock_shutdown(chip);
>  
>  	if (ret < 0) {
> -		if (strstr(kcontrol->id.name, "Speaker")) {
> -			ucontrol->value.integer.value[0] = 1;
> +		if (name && strstr(name, "Speaker")) {
> +			if (val)
> +				*val = 1;
>  			return 0;
>  		}
>  error:
> @@ -1491,6 +1492,21 @@ static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol,
>  		return filter_error(cval, ret);
>  	}
>  
> +	return ret;
> +}
> +
> +/* get the connectors status and report it as boolean type */
> +static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol,
> +				   struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct usb_mixer_elem_info *cval = kcontrol->private_data;
> +	int ret, val;
> +
> +	ret = get_connector_value(cval, kcontrol->id.name, &val);
> +
> +	if (ret < 0)
> +		return ret;
> +
>  	ucontrol->value.integer.value[0] = val;
>  	return 0;
>  }
> -- 
> 2.30.2
> 

Re: [PATCH v3 1/2] ALSA: usb-audio: Carve out connector value checking into a helper

[Takashi Iwai]

On Thu, 25 Mar 2021 17:59:12 +0100,
Kai-Heng Feng wrote:
> 
> This is preparation for next patch, no functional change intended.
> 
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

Applied now.  Thanks.


Takashi

> ---
> v3:
>  - No change.
> v2:
>  - Only return early when ret < 0.
> 
>  sound/usb/mixer.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index b004b2e63a5d..5a2d9a768f70 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -1446,13 +1446,11 @@ static int mixer_ctl_master_bool_get(struct snd_kcontrol *kcontrol,
>  	return 0;
>  }
>  
> -/* get the connectors status and report it as boolean type */
> -static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol,
> -				   struct snd_ctl_elem_value *ucontrol)
> +static int get_connector_value(struct usb_mixer_elem_info *cval,
> +			       char *name, int *val)
>  {
> -	struct usb_mixer_elem_info *cval = kcontrol->private_data;
>  	struct snd_usb_audio *chip = cval->head.mixer->chip;
> -	int idx = 0, validx, ret, val;
> +	int idx = 0, validx, ret;
>  
>  	validx = cval->control << 8 | 0;
>  
> @@ -1467,21 +1465,24 @@ static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol,
>  		ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), UAC2_CS_CUR,
>  				      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
>  				      validx, idx, &uac2_conn, sizeof(uac2_conn));
> -		val = !!uac2_conn.bNrChannels;
> +		if (val)
> +			*val = !!uac2_conn.bNrChannels;
>  	} else { /* UAC_VERSION_3 */
>  		struct uac3_insertion_ctl_blk uac3_conn;
>  
>  		ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), UAC2_CS_CUR,
>  				      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
>  				      validx, idx, &uac3_conn, sizeof(uac3_conn));
> -		val = !!uac3_conn.bmConInserted;
> +		if (val)
> +			*val = !!uac3_conn.bmConInserted;
>  	}
>  
>  	snd_usb_unlock_shutdown(chip);
>  
>  	if (ret < 0) {
> -		if (strstr(kcontrol->id.name, "Speaker")) {
> -			ucontrol->value.integer.value[0] = 1;
> +		if (name && strstr(name, "Speaker")) {
> +			if (val)
> +				*val = 1;
>  			return 0;
>  		}
>  error:
> @@ -1491,6 +1492,21 @@ static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol,
>  		return filter_error(cval, ret);
>  	}
>  
> +	return ret;
> +}
> +
> +/* get the connectors status and report it as boolean type */
> +static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol,
> +				   struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct usb_mixer_elem_info *cval = kcontrol->private_data;
> +	int ret, val;
> +
> +	ret = get_connector_value(cval, kcontrol->id.name, &val);
> +
> +	if (ret < 0)
> +		return ret;
> +
>  	ucontrol->value.integer.value[0] = val;
>  	return 0;
>  }
> -- 
> 2.30.2
> 

Re: [PATCH v3 2/2] ALSA: usb-audio: Check connector value on resume

[Takashi Iwai]

On Thu, 25 Mar 2021 17:59:13 +0100,
Kai-Heng Feng wrote:
> 
> Rear Mic on Lenovo P620 cannot record after S3, despite that there's no
> error and the other two functions of the USB audio, Line In and Line
> Out, work just fine.
> 
> The mic starts to work again after running userspace app like "alsactl
> store". Following the lead, the evidence shows that as soon as connector
> status is queried, the mic can work again.
> 
> So also check connector value on resume to "wake up" the USB audio to
> make it functional.
> 
> This can be device specific, however I think this generic approach may
> benefit more than one device.
> 
> Now the resume callback checks connector, and a new callback,
> reset_resume, to also restore switches and volumes.
> 
> Suggested-by: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

Applied now.  Thanks.


Takashi

> ---
> v3:
>  - New callback to handle resume and reset-resume separately.
> 
> v2:
>  - Remove reset-resume.
>  - Fold the connector checking to the mixer resume callback.
> 
>  sound/usb/mixer.c        | 44 +++++++++++++++++++++++++++++++---------
>  sound/usb/mixer.h        |  1 +
>  sound/usb/mixer_quirks.c |  2 +-
>  3 files changed, 36 insertions(+), 11 deletions(-)
> 
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index 5a2d9a768f70..2faf5767c7f8 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -3631,20 +3631,43 @@ static int restore_mixer_value(struct usb_mixer_elem_list *list)
>  	return 0;
>  }
>  
> +static int default_mixer_resume(struct usb_mixer_elem_list *list)
> +{
> +	struct usb_mixer_elem_info *cval = mixer_elem_list_to_info(list);
> +
> +	/* get connector value to "wake up" the USB audio */
> +	if (cval->val_type == USB_MIXER_BOOLEAN && cval->channels == 1)
> +		get_connector_value(cval, NULL, NULL);
> +
> +	return 0;
> +}
> +
> +static int default_mixer_reset_resume(struct usb_mixer_elem_list *list)
> +{
> +	int err = default_mixer_resume(list);
> +
> +	if (err < 0)
> +		return err;
> +	return restore_mixer_value(list);
> +}
> +
>  int snd_usb_mixer_resume(struct usb_mixer_interface *mixer, bool reset_resume)
>  {
>  	struct usb_mixer_elem_list *list;
> +	usb_mixer_elem_resume_func_t f;
>  	int id, err;
>  
> -	if (reset_resume) {
> -		/* restore cached mixer values */
> -		for (id = 0; id < MAX_ID_ELEMS; id++) {
> -			for_each_mixer_elem(list, mixer, id) {
> -				if (list->resume) {
> -					err = list->resume(list);
> -					if (err < 0)
> -						return err;
> -				}
> +	/* restore cached mixer values */
> +	for (id = 0; id < MAX_ID_ELEMS; id++) {
> +		for_each_mixer_elem(list, mixer, id) {
> +			if (reset_resume)
> +				f = list->reset_resume;
> +			else
> +				f = list->resume;
> +			if (f) {
> +				err = f(list);
> +				if (err < 0)
> +					return err;
>  			}
>  		}
>  	}
> @@ -3663,6 +3686,7 @@ void snd_usb_mixer_elem_init_std(struct usb_mixer_elem_list *list,
>  	list->id = unitid;
>  	list->dump = snd_usb_mixer_dump_cval;
>  #ifdef CONFIG_PM
> -	list->resume = restore_mixer_value;
> +	list->resume = default_mixer_resume;
> +	list->reset_resume = default_mixer_reset_resume;
>  #endif
>  }
> diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h
> index c29e27ac43a7..e5a01f17bf3c 100644
> --- a/sound/usb/mixer.h
> +++ b/sound/usb/mixer.h
> @@ -69,6 +69,7 @@ struct usb_mixer_elem_list {
>  	bool is_std_info;
>  	usb_mixer_elem_dump_func_t dump;
>  	usb_mixer_elem_resume_func_t resume;
> +	usb_mixer_elem_resume_func_t reset_resume;
>  };
>  
>  /* iterate over mixer element list of the given unit id */
> diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
> index ffd922327ae4..b7f9c2fded05 100644
> --- a/sound/usb/mixer_quirks.c
> +++ b/sound/usb/mixer_quirks.c
> @@ -151,7 +151,7 @@ static int add_single_ctl_with_resume(struct usb_mixer_interface *mixer,
>  		*listp = list;
>  	list->mixer = mixer;
>  	list->id = id;
> -	list->resume = resume;
> +	list->reset_resume = resume;
>  	kctl = snd_ctl_new1(knew, list);
>  	if (!kctl) {
>  		kfree(list);
> -- 
> 2.30.2
> 

Re: [PATCH v3 2/2] ALSA: usb-audio: Check connector value on resume

[Takashi Iwai]

On Thu, 25 Mar 2021 17:59:13 +0100,
Kai-Heng Feng wrote:
> 
> Rear Mic on Lenovo P620 cannot record after S3, despite that there's no
> error and the other two functions of the USB audio, Line In and Line
> Out, work just fine.
> 
> The mic starts to work again after running userspace app like "alsactl
> store". Following the lead, the evidence shows that as soon as connector
> status is queried, the mic can work again.
> 
> So also check connector value on resume to "wake up" the USB audio to
> make it functional.
> 
> This can be device specific, however I think this generic approach may
> benefit more than one device.
> 
> Now the resume callback checks connector, and a new callback,
> reset_resume, to also restore switches and volumes.
> 
> Suggested-by: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

Applied now.  Thanks.


Takashi

> ---
> v3:
>  - New callback to handle resume and reset-resume separately.
> 
> v2:
>  - Remove reset-resume.
>  - Fold the connector checking to the mixer resume callback.
> 
>  sound/usb/mixer.c        | 44 +++++++++++++++++++++++++++++++---------
>  sound/usb/mixer.h        |  1 +
>  sound/usb/mixer_quirks.c |  2 +-
>  3 files changed, 36 insertions(+), 11 deletions(-)
> 
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index 5a2d9a768f70..2faf5767c7f8 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -3631,20 +3631,43 @@ static int restore_mixer_value(struct usb_mixer_elem_list *list)
>  	return 0;
>  }
>  
> +static int default_mixer_resume(struct usb_mixer_elem_list *list)
> +{
> +	struct usb_mixer_elem_info *cval = mixer_elem_list_to_info(list);
> +
> +	/* get connector value to "wake up" the USB audio */
> +	if (cval->val_type == USB_MIXER_BOOLEAN && cval->channels == 1)
> +		get_connector_value(cval, NULL, NULL);
> +
> +	return 0;
> +}
> +
> +static int default_mixer_reset_resume(struct usb_mixer_elem_list *list)
> +{
> +	int err = default_mixer_resume(list);
> +
> +	if (err < 0)
> +		return err;
> +	return restore_mixer_value(list);
> +}
> +
>  int snd_usb_mixer_resume(struct usb_mixer_interface *mixer, bool reset_resume)
>  {
>  	struct usb_mixer_elem_list *list;
> +	usb_mixer_elem_resume_func_t f;
>  	int id, err;
>  
> -	if (reset_resume) {
> -		/* restore cached mixer values */
> -		for (id = 0; id < MAX_ID_ELEMS; id++) {
> -			for_each_mixer_elem(list, mixer, id) {
> -				if (list->resume) {
> -					err = list->resume(list);
> -					if (err < 0)
> -						return err;
> -				}
> +	/* restore cached mixer values */
> +	for (id = 0; id < MAX_ID_ELEMS; id++) {
> +		for_each_mixer_elem(list, mixer, id) {
> +			if (reset_resume)
> +				f = list->reset_resume;
> +			else
> +				f = list->resume;
> +			if (f) {
> +				err = f(list);
> +				if (err < 0)
> +					return err;
>  			}
>  		}
>  	}
> @@ -3663,6 +3686,7 @@ void snd_usb_mixer_elem_init_std(struct usb_mixer_elem_list *list,
>  	list->id = unitid;
>  	list->dump = snd_usb_mixer_dump_cval;
>  #ifdef CONFIG_PM
> -	list->resume = restore_mixer_value;
> +	list->resume = default_mixer_resume;
> +	list->reset_resume = default_mixer_reset_resume;
>  #endif
>  }
> diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h
> index c29e27ac43a7..e5a01f17bf3c 100644
> --- a/sound/usb/mixer.h
> +++ b/sound/usb/mixer.h
> @@ -69,6 +69,7 @@ struct usb_mixer_elem_list {
>  	bool is_std_info;
>  	usb_mixer_elem_dump_func_t dump;
>  	usb_mixer_elem_resume_func_t resume;
> +	usb_mixer_elem_resume_func_t reset_resume;
>  };
>  
>  /* iterate over mixer element list of the given unit id */
> diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
> index ffd922327ae4..b7f9c2fded05 100644
> --- a/sound/usb/mixer_quirks.c
> +++ b/sound/usb/mixer_quirks.c
> @@ -151,7 +151,7 @@ static int add_single_ctl_with_resume(struct usb_mixer_interface *mixer,
>  		*listp = list;
>  	list->mixer = mixer;
>  	list->id = id;
> -	list->resume = resume;
> +	list->reset_resume = resume;
>  	kctl = snd_ctl_new1(knew, list);
>  	if (!kctl) {
>  		kfree(list);
> -- 
> 2.30.2
>