Skip to contentSkip to conversation

ALSA: usb-audio: UAC2 jack detection patchset

PATCH Superseded Andrew Chant 2 participants 9 messages Mar 22, 2018 – Mar 24, 2018

Subsystems (2)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover ALSA: usb-audio: UAC2 jack detection patchset

These patches implement UAC2 jack detection.

This allows USB-to-3.5mm adapters and similar USB devices to report jack state via alsa mixers, using the reporting provided by the UAC2 connector control.

The first patch fixes up the function used to query UAC controls. The long lines in the table are unfortunate but keep the table readable. The long lines elsewhere in the patch were in line with existing long lines due to current code structure.

Andrew Chant (2):
ALSA: usb-audio: fix uac control query argument
ALSA: usb-audio: UAC2 jack detection

 include/linux/usb/audio-v2.h |   4 +-
 sound/usb/clock.c            |   5 +-
 sound/usb/mixer.c            | 168 +++++++++++++++++++++++++++++------
 3 files changed, 146 insertions(+), 31 deletions(-)
Signature · 1 line
2.17.0.rc0.231.g781580f067-goog
#1 lore raw

PATCH 1/2 ALSA: usb-audio: fix uac control query argument

This patch fixes code readability and should have no functional change.

Correct uac control query functions to account for the 1-based indexing of USB Audio Class control identifiers.

The function parameter, u8 control, should be the
constant defined in audio-v2.h to identify the control to be checked for readability or writeability.

This patch fixes all callers that had adjusted, and makes explicit the mapping between audio_feature_info[] array index and the associated control identifier.

---
 include/linux/usb/audio-v2.h |  4 +--
 sound/usb/clock.c            |  5 +--
 sound/usb/mixer.c            | 69 ++++++++++++++++++++++--------------
 3 files changed, 48 insertions(+), 30 deletions(-)
include/linux/usb/audio-v2.h +2 -2
index 2db83a191e78..aaafecf073ff 100644
@@ -36,12 +36,12 @@
 
 static inline bool uac_v2v3_control_is_readable(u32 bmControls, u8 control)
 {
-	return (bmControls >> (control * 2)) & 0x1;
+	return (bmControls >> ((control - 1) * 2)) & 0x1;
 }
 
 static inline bool uac_v2v3_control_is_writeable(u32 bmControls, u8 control)
 {
-	return (bmControls >> (control * 2)) & 0x2;
+	return (bmControls >> ((control - 1) * 2)) & 0x2;
 }
 
 /* 4.7.2 Class-Specific AC Interface Descriptor */
sound/usb/clock.c +3 -2
index 25de7fe285d9..ab39ccb974c6 100644
@@ -214,7 +214,7 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip,
 
 	/* If a clock source can't tell us whether it's valid, we assume it is */
 	if (!uac_v2v3_control_is_readable(bmControls,
-				      UAC2_CS_CONTROL_CLOCK_VALID - 1))
+				      UAC2_CS_CONTROL_CLOCK_VALID))
 		return 1;
 
 	err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_CUR,
@@ -552,7 +552,8 @@ static int set_sample_rate_v2v3(struct snd_usb_audio *chip, int iface,
 		bmControls = cs_desc->bmControls;
 	}
 
-	writeable = uac_v2v3_control_is_writeable(bmControls, UAC2_CS_CONTROL_SAM_FREQ - 1);
+	writeable = uac_v2v3_control_is_writeable(bmControls,
+						  UAC2_CS_CONTROL_SAM_FREQ);
 	if (writeable) {
 		data = cpu_to_le32(rate);
 		err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC2_CS_CUR,
sound/usb/mixer.c +43 -26

146 lines not on this page · raw message

Signature · 1 line
2.17.0.rc0.231.g781580f067-goog
#2 lore raw

Re: [PATCH 1/2] ALSA: usb-audio: fix uac control query argument

Quoted 10 lines · Andrew Chant

This patch fixes code readability and should have no functional change.

Correct uac control query functions to account for the 1-based indexing of USB Audio Class control identifiers.

The function parameter, u8 control, should be the
constant defined in audio-v2.h to identify the control to be checked for readability or writeability.

Quoted 5 lines

This patch fixes all callers that had adjusted, and makes explicit the mapping between audio_feature_info[] array index and the associated control identifier.

Looks like a nice clean up and fix.
Would you like me applying this now, or better all together later?

thanks,

Takashi

#3 lore raw

Re: [PATCH 1/2] ALSA: usb-audio: fix uac control query argument

Applying this now would be great, thank you.

On Mar 23, 2018 12:48 AM, "Takashi Iwai" <[email protected]> wrote:

Quoted 10 lines · Andrew Chant

This patch fixes code readability and should have no functional change.

Correct uac control query functions to account for the 1-based indexing of USB Audio Class control identifiers.

The function parameter, u8 control, should be the
constant defined in audio-v2.h to identify the control to be checked for readability or writeability.

Quoted 5 lines

This patch fixes all callers that had adjusted, and makes explicit the mapping between audio_feature_info[] array index and the associated control identifier.

Looks like a nice clean up and fix.
Would you like me applying this now, or better all together later?

thanks,

Takashi

#4 lore raw

Re: [PATCH 1/2] ALSA: usb-audio: fix uac control query argument

Quoted 2 lines · Andrew Chant

Applying this now would be great, thank you.

OK, applied now. Thanks!

Takashi

Quoted 30 lines · Takashi Iwai
Quoted 10 lines · Andrew Chant

This patch fixes code readability and should have no functional change.

Correct uac control query functions to account for the 1-based indexing of USB Audio Class control identifiers.

The function parameter, u8 control, should be the
constant defined in audio-v2.h to identify the control to be checked for readability or writeability.

Quoted 5 lines

This patch fixes all callers that had adjusted, and makes explicit the mapping between audio_feature_info[] array index and the associated control identifier.

Looks like a nice clean up and fix.
Would you like me applying this now, or better all together later?

thanks,

Takashi
[2 <text/html; UTF-8 (quoted-printable)>]

#5 lore raw

PATCH 2/2 ALSA: usb-audio: UAC2 jack detection

In reply to Andrew Chant

This implements UAC2 jack detection support, presenting jack status as a boolean read-only mono mixer.

The presence of any channel in the UAC2_TE_CONNECTOR control for a terminal will result in the mixer saying the jack is connected.

Mixer naming follows the convention in sound/core/ctljack.c, terminating the mixer with " Jack".
For additional clues as to which jack is being presented, the name is prefixed with " - Input Jack" or " - Output Jack" depending on if it's an input or output terminal.

This is required because terminal names are ambiguous between inputs and outputs and often duplicated - Bidirectional terminal types (0x400 -> 0x4FF)
"... may be used separately for input only or output only. These types require two Terminal descriptors. Both have the same type." (quote from "USB Device Class Definition for Terminal Types")

Since bidirectional terminal types are common for headphone adapters, this distinguishes between two otherwise identically-named jack controls.

Tested with a UAC2 audio device with connector control capability.

---
 sound/usb/mixer.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 98 insertions(+), 1 deletion(-)
sound/usb/mixer.c +98 -1
index 3075ac50a391..6cb61307aefe 100644
@@ -1235,6 +1235,21 @@ static int mixer_ctl_feature_put(struct snd_kcontrol *kcontrol,
 	return changed;
 }
 
+/* get the current value from a mixer element */
+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 val, err;
+
+	err = snd_usb_get_cur_mix_value(cval, 0, 0, &val);
+	if (err < 0)
+		return filter_error(cval, err);
+	val = (val != 0);
+	ucontrol->value.integer.value[0] = val;
+	return 0;
+}
+
 static struct snd_kcontrol_new usb_feature_unit_ctl = {
 	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
 	.name = "", /* will be filled later manually */
@@ -1252,6 +1267,16 @@ static const struct snd_kcontrol_new usb_feature_unit_ctl_ro = {
 	.put = NULL,
 };
 
+/* A UAC connector mixer control */
+static struct snd_kcontrol_new usb_connector_ctl_ro = {
+	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+	.name = "", /* will be filled later manually */
+	.access = SNDRV_CTL_ELEM_ACCESS_READ,
+	.info = snd_ctl_boolean_mono_info,
+	.get = mixer_ctl_connector_get,
+	.put = NULL,
+};
+
 /*
  * This symbol is exported in order to allow the mixer quirks to
  * hook up to the standard feature unit control mechanism
@@ -1464,6 +1489,55 @@ static void build_feature_ctl(struct mixer_build *state, void *raw_desc,
 	snd_usb_mixer_add_control(&cval->head, kctl);
 }
 
+static void get_connector_control_name(struct mixer_build *state,
+				       struct usb_audio_term *term,
+				       bool is_input, char *name, int name_size)
+{
+	int name_len = get_term_name(state, term, name, name_size, 0);
+
+	if (name_len == 0)
+		strlcpy(name, "Unknown", name_size);
+
+	/*
+	 *  sound/core/ctljack.c has a convention of naming jack controls
+	 * by ending in " Jack".  Make it slightly more useful by
+	 * indicating Input or Output after the terminal name.
+	 */
+	if (is_input)
+		strlcat(name, " - Input Jack", name_size);
+	else
+		strlcat(name, " - Output Jack", name_size);
+}
+
+/* Build a mixer control for a UAC connector control (jack-detect) */
+static void build_connector_control(struct mixer_build *state,
+				    struct usb_audio_term *term, bool is_input)
+{
+	struct snd_kcontrol *kctl;
+	struct usb_mixer_elem_info *cval;
+
+	cval = kzalloc(sizeof(*cval), GFP_KERNEL);
+	if (!cval)
+		return;
+	snd_usb_mixer_elem_init_std(&cval->head, state->mixer, term->id);
+	cval->control = UAC2_TE_CONNECTOR;
+	cval->val_type = USB_MIXER_BOOLEAN;
+	cval->channels = term->channels;
+	cval->cmask = term->chconfig;
+	cval->min = 0;
+	cval->max = 1;
+	kctl = snd_ctl_new1(&usb_connector_ctl_ro, cval);
+	if (!kctl) {
+		usb_audio_err(state->chip, "cannot malloc kcontrol\n");
+		kfree(cval);
+		return;
+	}
+	get_connector_control_name(state, term, is_input, kctl->id.name,
+				   sizeof(kctl->id.name));
+	kctl->private_free = snd_usb_mixer_elem_free;
+	snd_usb_mixer_add_control(&cval->head, kctl);
+}
+
 static int parse_clock_source_unit(struct mixer_build *state, int unitid,
 				   void *_ftr)
 {
@@ -1770,6 +1844,23 @@ static void build_mixer_unit_ctl(struct mixer_build *state,
 	snd_usb_mixer_add_control(&cval->head, kctl);
 }
 
+static int parse_audio_input_terminal(struct mixer_build *state, int unitid,
+				      void *raw_desc)
+{
+	struct usb_audio_term iterm;
+	struct uac2_input_terminal_descriptor *d = raw_desc;
+
+	check_input_term(state, d->bTerminalID, &iterm);
+	if (state->mixer->protocol == UAC_VERSION_2) {
+		/* Check for jack detection. */
+		if (uac_v2v3_control_is_readable(d->bmControls,
+						 UAC2_TE_CONNECTOR)) {
+			build_connector_control(state, &iterm, true);
+		}
+	}
+	return 0;
+}
+
 /*
  * parse a mixer unit
  */
@@ -2320,7 +2411,7 @@ static int parse_audio_unit(struct mixer_build *state, int unitid)
 	if (protocol == UAC_VERSION_1 || protocol == UAC_VERSION_2) {
 		switch (p1[2]) {
 		case UAC_INPUT_TERMINAL:
-			return 0; /* NOP */
+			return parse_audio_input_terminal(state, unitid, p1);
 		case UAC_MIXER_UNIT:
 			return parse_audio_mixer_unit(state, unitid, p1);
 		case UAC2_CLOCK_SOURCE:
@@ -2463,6 +2554,12 @@ static int snd_usb_mixer_controls(struct usb_mixer_interface *mixer)
 			err = parse_audio_unit(&state, desc->bCSourceID);
 			if (err < 0 && err != -EINVAL)
 				return err;
+
+			if (uac_v2v3_control_is_readable(desc->bmControls,
+							 UAC2_TE_CONNECTOR)) {
+				build_connector_control(&state, &state.oterm,
+							false);
+			}
 		} else {  /* UAC_VERSION_3 */
 			struct uac3_output_terminal_descriptor *desc = p;
 
Signature · 1 line
2.17.0.rc0.231.g781580f067-goog
#6 lore raw

Re: [PATCH 2/2] ALSA: usb-audio: UAC2 jack detection

Quoted 59 lines · Andrew Chant

This implements UAC2 jack detection support, presenting jack status as a boolean read-only mono mixer.

The presence of any channel in the UAC2_TE_CONNECTOR control for a terminal will result in the mixer saying the jack is connected.

Mixer naming follows the convention in sound/core/ctljack.c, terminating the mixer with " Jack".
For additional clues as to which jack is being presented, the name is prefixed with " - Input Jack" or " - Output Jack" depending on if it's an input or output terminal.

This is required because terminal names are ambiguous between inputs and outputs and often duplicated - Bidirectional terminal types (0x400 -> 0x4FF)
"... may be used separately for input only or output only. These types require two Terminal descriptors. Both have the same type." (quote from "USB Device Class Definition for Terminal Types")

Since bidirectional terminal types are common for headphone adapters, this distinguishes between two otherwise identically-named jack controls.

Tested with a UAC2 audio device with connector control capability.

---
 sound/usb/mixer.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 98 insertions(+), 1 deletion(-)
sound/usb/mixer.c +15 -0
index 3075ac50a391..6cb61307aefe 100644
@@ -1235,6 +1235,21 @@ static int mixer_ctl_feature_put(struct snd_kcontrol *kcontrol,
 	return changed;
 }
 
+/* get the current value from a mixer element */
+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 val, err;
+
+	err = snd_usb_get_cur_mix_value(cval, 0, 0, &val);
+	if (err < 0)
+		return filter_error(cval, err);
+	val = (val != 0);
+	ucontrol->value.integer.value[0] = val;
+	return 0;
+}
+
 static struct snd_kcontrol_new usb_feature_unit_ctl = {
 	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
 	.name = "", /* will be filled later manually */

sound/usb/mixer.c static const struct snd_kcontrol_new usb_feature_unit_ctl_ro = { · line 1272

Quoted 6 lines
sound/usb/mixer.c +3 -0
@@ -1252,6 +1267,16 @@ static const struct snd_kcontrol_new usb_feature_unit_ctl_ro = {
 	.put = NULL,
 };
 
+/* A UAC connector mixer control */
+static struct snd_kcontrol_new usb_connector_ctl_ro = {
+	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,

Let's follow the other usages, and here iface is set to SNDRV_CTL_ELEM_IFACE_CARD. This was for avoiding such volatile controls appearing the mixer application.

sound/usb/mixer.c static const struct snd_kcontrol_new usb_feature_unit_ctl_ro = { · line 1274

Quoted 2 lines
sound/usb/mixer.c +2 -0
+	.name = "", /* will be filled later manually */
+	.access = SNDRV_CTL_ELEM_ACCESS_READ,

Actually we should have set SNDRV_CTL_ELEM_ACCESS_VOLATILE, too. We forgot it in ctljack.c, too.

The rest looks good. But one thing to confirm: the value change notification is done in snd_usb_mixer_interrupt()?

Also, is the jack re-detected after suspend/resume? That is, plug off the jack while sleeping, and after the resume, is the jack status change recognized and notified?

thanks,

Takashi

#7 lore raw

Re: [PATCH 2/2] ALSA: usb-audio: UAC2 jack detection

Quoted 3 lines · Takashi Iwai

Let's follow the other usages, and here iface is set to SNDRV_CTL_ELEM_IFACE_CARD. This was for avoiding such volatile controls appearing the mixer application.

Will do - it looks like the "Clock Source Validity" control also needs this. I'll send a patch for that as well.

Quoted 4 lines
Quoted 1 line

+ .access = SNDRV_CTL_ELEM_ACCESS_READ,

Actually we should have set SNDRV_CTL_ELEM_ACCESS_VOLATILE, too. We forgot it in ctljack.c, too.

Sounds good.

Quoted 7 lines

The rest looks good. But one thing to confirm: the value change notification is done in snd_usb_mixer_interrupt()?

Also, is the jack re-detected after suspend/resume? That is, plug off the jack while sleeping, and after the resume, is the jack status change recognized and notified?

Yes - the USB audio device causes a remote wakeup, then after a few ms responds with the interrupt data message indicating the connector control that changed. This ends up calling snd_usb_mixer_interrupt().

On codeaurora kernels for Qualcomm SoCs, there's a
patch which enables autosuspend for any USB device with an audio interface.
https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/sound/usb/card.c?h=LA.HB.1.1.5.c1#n663
so suspend/resume paths are well tested when devices are used on Android.

Two new questions:
#1 - I'd like to report the impedance of the load connected to the jack (generally as an integer). Is a read-only control element the right way to do this?

#2 - In our particular case there's a single jack which can detect presence of a microphone and playback separately. USB descriptors provide a mechanism to show that the input and output terminals are linked in this way (terminal type 4xx along with bAssocTerminal).

Is there a better way of having this reported than two separate mono jack controls?

#8 lore raw

Re: [PATCH 2/2] ALSA: usb-audio: UAC2 jack detection

Quoted 5 lines · Andrew Chant

Two new questions:
#1 - I'd like to report the impedance of the load connected to the jack (generally as an integer). Is a read-only control element the right way to do this?

Yes, it would be suitable.

Quoted 7 lines

#2 - In our particular case there's a single jack which can detect presence of a microphone and playback separately. USB descriptors provide a mechanism to show that the input and output terminals are linked in this way (terminal type 4xx along with bAssocTerminal).

Is there a better way of having this reported than two separate mono jack controls?

This has been discussed in the past. The current two bool kctls are the mapping of the former input devices. One suggestion was to use an enum for the jack control. But practically seen this would break user-space apps, so likely no-go.

I guess we need to live as is -- two bools to represent one full-duplex jack.

thanks,

Takashi