Commit 6aa87001 authored by Karol Kosik's avatar Karol Kosik Committed by Takashi Iwai
Browse files

ALSA: usb-audio: Support multiple control interfaces 

Registering Numark Party Mix II fails with error 'bogus bTerminalLink 1'.
The problem stems from the driver not being able to find input/output
terminals required to configure audio streaming. The information about
those terminals is stored in AudioControl Interface. Numark device
contains 2 AudioControl Interfaces and the driver checks only one of them.

According to the USB standard, a device can have multiple audio functions,
each represented by Audio Interface Collection. Every audio function is
considered to be closed box and will contain unique AudioControl Interface
and zero or more AudioStreaming and MIDIStreaming Interfaces.

The Numark device adheres to the standard and defines two audio functions:
- MIDIStreaming function
- AudioStreaming function
It starts with MIDI function, followed by the audio function. The driver
saves the first AudioControl Interface in `snd_usb_audio` structure
associated with the entire device. It then attempts to use this interface
to query for terminals and clocks. However, this fails because the correct
information is stored in the second AudioControl Interface, defined in the
second Audio Interface Collection.

This patch introduces a structure holding association between each
MIDI/Audio Interface and its corresponding AudioControl Interface,
instead of relying on AudioControl Interface defined for the entire
device. This structure is populated during usb probing phase and leveraged
later when querying for terminals and when sending USB requests.

Alternative solutions considered include:
- defining a quirk for Numark where the order of interface is manually
changed, or terminals are hardcoded in the driver. This solution would
have fixed only this model, though it seems that device is USB compliant,
and it also seems that other devices from this company may be affected.
What's more, it looks like products from other manufacturers have similar
problems, i.e. Rane One DJ console
- keeping a list of all AudioControl Interfaces and querying all of them
to find required information. That would have solved my problem and have
low probability of breaking other devices, as we would always start with
the same logic of querying first AudioControl Interface. This solution
would not have followed the standard though.

This patch preserves the `snd_usb_audio.ctrl_intf` variable, which holds
the first AudioControl Interface, and uses it as a fallback when some
interfaces are not parsed correctly and lack an associated AudioControl
Interface, i.e., when configured via quirks.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217865


Signed-off-by: default avatarKarol Kosik <k.kosik@outlook.com>
Link: https://patch.msgid.link/AS8P190MB1285893F4735C8B32AD3886BEC852@AS8P190MB1285.EURP190.PROD.OUTLOOK.COM


Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
parent ebfb5a57

sound/usb/card.c

+2 −0
Original line number Diff line number Diff line
@@ -206,6 +206,8 @@ static int snd_usb_create_stream(struct snd_usb_audio *chip, int ctrlif, int int 
		return -EINVAL;

	}



	snd_usb_add_ctrl_interface_link(chip, interface, ctrlif);



	if (! snd_usb_parse_audio_interface(chip, interface)) {

		usb_set_interface(dev, interface, 0); /* reset the current interface */

		return usb_driver_claim_interface(&usb_audio_driver, iface,

sound/usb/clock.c

+39 −23
Original line number Diff line number Diff line
@@ -76,11 +76,14 @@ static bool validate_clock_multiplier(void *p, int id, int proto) 
}



#define DEFINE_FIND_HELPER(name, obj, validator, type2, type3)		\

static obj *name(struct snd_usb_audio *chip, int id, int proto)	\

static obj *name(struct snd_usb_audio *chip, int id,	\

				const struct audioformat *fmt)	\

{									\

	return find_uac_clock_desc(chip->ctrl_intf, id, validator,	\

				   proto == UAC_VERSION_3 ? (type3) : (type2), \

				   proto);				\

	struct usb_host_interface *ctrl_intf =	\

		snd_usb_find_ctrl_interface(chip, fmt->iface); \

	return find_uac_clock_desc(ctrl_intf, id, validator,	\

				   fmt->protocol == UAC_VERSION_3 ? (type3) : (type2), \

				   fmt->protocol);				\

}



DEFINE_FIND_HELPER(snd_usb_find_clock_source,
@@ -93,16 +96,19 @@ DEFINE_FIND_HELPER(snd_usb_find_clock_multiplier, 
		   union uac23_clock_multiplier_desc, validate_clock_multiplier,

		   UAC2_CLOCK_MULTIPLIER, UAC3_CLOCK_MULTIPLIER);



static int uac_clock_selector_get_val(struct snd_usb_audio *chip, int selector_id)

static int uac_clock_selector_get_val(struct snd_usb_audio *chip,

				int selector_id, int iface_no)

{

	struct usb_host_interface *ctrl_intf;

	unsigned char buf;

	int ret;



	ctrl_intf = snd_usb_find_ctrl_interface(chip, iface_no);

	ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0),

			      UAC2_CS_CUR,

			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,

			      UAC2_CX_CLOCK_SELECTOR << 8,

			      snd_usb_ctrl_intf(chip) | (selector_id << 8),

			      snd_usb_ctrl_intf(ctrl_intf) | (selector_id << 8),

			      &buf, sizeof(buf));



	if (ret < 0)
@@ -111,16 +117,18 @@ static int uac_clock_selector_get_val(struct snd_usb_audio *chip, int selector_i 
	return buf;

}



static int uac_clock_selector_set_val(struct snd_usb_audio *chip, int selector_id,

					unsigned char pin)

static int uac_clock_selector_set_val(struct snd_usb_audio *chip,

					int selector_id, unsigned char pin, int iface_no)

{

	struct usb_host_interface *ctrl_intf;

	int ret;



	ctrl_intf = snd_usb_find_ctrl_interface(chip, iface_no);

	ret = snd_usb_ctl_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0),

			      UAC2_CS_CUR,

			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_OUT,

			      UAC2_CX_CLOCK_SELECTOR << 8,

			      snd_usb_ctrl_intf(chip) | (selector_id << 8),

			      snd_usb_ctrl_intf(ctrl_intf) | (selector_id << 8),

			      &pin, sizeof(pin));

	if (ret < 0)

		return ret;
@@ -132,7 +140,7 @@ static int uac_clock_selector_set_val(struct snd_usb_audio *chip, int selector_i 
		return -EINVAL;

	}



	ret = uac_clock_selector_get_val(chip, selector_id);

	ret = uac_clock_selector_get_val(chip, selector_id, iface_no);

	if (ret < 0)

		return ret;
@@ -155,8 +163,10 @@ static bool uac_clock_source_is_valid_quirk(struct snd_usb_audio *chip, 
	unsigned char data;

	struct usb_device *dev = chip->dev;

	union uac23_clock_source_desc *cs_desc;

	struct usb_host_interface *ctrl_intf;



	cs_desc = snd_usb_find_clock_source(chip, source_id, fmt->protocol);

	ctrl_intf = snd_usb_find_ctrl_interface(chip, fmt->iface);

	cs_desc = snd_usb_find_clock_source(chip, source_id, fmt);

	if (!cs_desc)

		return false;
@@ -191,7 +201,7 @@ static bool uac_clock_source_is_valid_quirk(struct snd_usb_audio *chip, 
			err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_CUR,

					      USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,

					      UAC2_CS_CONTROL_CLOCK_VALID << 8,

					      snd_usb_ctrl_intf(chip) | (source_id << 8),

					      snd_usb_ctrl_intf(ctrl_intf) | (source_id << 8),

					      &data, sizeof(data));

			if (err < 0) {

				dev_warn(&dev->dev,
@@ -217,8 +227,10 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, 
	struct usb_device *dev = chip->dev;

	u32 bmControls;

	union uac23_clock_source_desc *cs_desc;

	struct usb_host_interface *ctrl_intf;



	cs_desc = snd_usb_find_clock_source(chip, source_id, fmt->protocol);

	ctrl_intf = snd_usb_find_ctrl_interface(chip, fmt->iface);

	cs_desc = snd_usb_find_clock_source(chip, source_id, fmt);

	if (!cs_desc)

		return false;
@@ -235,7 +247,7 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, 
	err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_CUR,

			      USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,

			      UAC2_CS_CONTROL_CLOCK_VALID << 8,

			      snd_usb_ctrl_intf(chip) | (source_id << 8),

			      snd_usb_ctrl_intf(ctrl_intf) | (source_id << 8),

			      &data, sizeof(data));



	if (err < 0) {
@@ -274,7 +286,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, 
	}



	/* first, see if the ID we're looking at is a clock source already */

	source = snd_usb_find_clock_source(chip, entity_id, proto);

	source = snd_usb_find_clock_source(chip, entity_id, fmt);

	if (source) {

		entity_id = GET_VAL(source, proto, bClockID);

		if (validate && !uac_clock_source_is_valid(chip, fmt,
@@ -287,7 +299,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, 
		return entity_id;

	}



	selector = snd_usb_find_clock_selector(chip, entity_id, proto);

	selector = snd_usb_find_clock_selector(chip, entity_id, fmt);

	if (selector) {

		pins = GET_VAL(selector, proto, bNrInPins);

		clock_id = GET_VAL(selector, proto, bClockID);
@@ -317,7 +329,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, 


		/* the entity ID we are looking at is a selector.

		 * find out what it currently selects */

		ret = uac_clock_selector_get_val(chip, clock_id);

		ret = uac_clock_selector_get_val(chip, clock_id, fmt->iface);

		if (ret < 0) {

			if (!chip->autoclock)

				return ret;
@@ -346,7 +358,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, 
			if (chip->quirk_flags & QUIRK_FLAG_SKIP_CLOCK_SELECTOR ||

			    !writeable)

				return ret;

			err = uac_clock_selector_set_val(chip, entity_id, cur);

			err = uac_clock_selector_set_val(chip, entity_id, cur, fmt->iface);

			if (err < 0) {

				if (pins == 1) {

					usb_audio_dbg(chip,
@@ -377,7 +389,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, 
			if (ret < 0)

				continue;



			err = uac_clock_selector_set_val(chip, entity_id, i);

			err = uac_clock_selector_set_val(chip, entity_id, i, fmt->iface);

			if (err < 0)

				continue;
@@ -391,7 +403,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, 
	}



	/* FIXME: multipliers only act as pass-thru element for now */

	multiplier = snd_usb_find_clock_multiplier(chip, entity_id, proto);

	multiplier = snd_usb_find_clock_multiplier(chip, entity_id, fmt);

	if (multiplier)

		return __uac_clock_find_source(chip, fmt,

					       GET_VAL(multiplier, proto, bCSourceID),
@@ -491,11 +503,13 @@ static int get_sample_rate_v2v3(struct snd_usb_audio *chip, int iface, 
	struct usb_device *dev = chip->dev;

	__le32 data;

	int err;

	struct usb_host_interface *ctrl_intf;



	ctrl_intf = snd_usb_find_ctrl_interface(chip, iface);

	err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_CUR,

			      USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,

			      UAC2_CS_CONTROL_SAM_FREQ << 8,

			      snd_usb_ctrl_intf(chip) | (clock << 8),

			      snd_usb_ctrl_intf(ctrl_intf) | (clock << 8),

			      &data, sizeof(data));

	if (err < 0) {

		dev_warn(&dev->dev, "%d:%d: cannot get freq (v2/v3): err %d\n",
@@ -524,8 +538,10 @@ int snd_usb_set_sample_rate_v2v3(struct snd_usb_audio *chip, 
	__le32 data;

	int err;

	union uac23_clock_source_desc *cs_desc;

	struct usb_host_interface *ctrl_intf;



	cs_desc = snd_usb_find_clock_source(chip, clock, fmt->protocol);

	ctrl_intf = snd_usb_find_ctrl_interface(chip, fmt->iface);

	cs_desc = snd_usb_find_clock_source(chip, clock, fmt);



	if (!cs_desc)

		return 0;
@@ -544,7 +560,7 @@ int snd_usb_set_sample_rate_v2v3(struct snd_usb_audio *chip, 
	err = snd_usb_ctl_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0), UAC2_CS_CUR,

			      USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_OUT,

			      UAC2_CS_CONTROL_SAM_FREQ << 8,

			      snd_usb_ctrl_intf(chip) | (clock << 8),

			      snd_usb_ctrl_intf(ctrl_intf) | (clock << 8),

			      &data, sizeof(data));

	if (err < 0)

		return err;

sound/usb/format.c

+4 −2
Original line number Diff line number Diff line
@@ -545,7 +545,9 @@ static int parse_audio_format_rates_v2v3(struct snd_usb_audio *chip, 
	unsigned char tmp[2], *data;

	int nr_triplets, data_size, ret = 0, ret_l6;

	int clock = snd_usb_clock_find_source(chip, fp, false);

	struct usb_host_interface *ctrl_intf;



	ctrl_intf = snd_usb_find_ctrl_interface(chip, fp->iface);

	if (clock < 0) {

		dev_err(&dev->dev,

			"%s(): unable to find clock source (clock %d)\n",
@@ -557,7 +559,7 @@ static int parse_audio_format_rates_v2v3(struct snd_usb_audio *chip, 
	ret = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_RANGE,

			      USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,

			      UAC2_CS_CONTROL_SAM_FREQ << 8,

			      snd_usb_ctrl_intf(chip) | (clock << 8),

			      snd_usb_ctrl_intf(ctrl_intf) | (clock << 8),

			      tmp, sizeof(tmp));



	if (ret < 0) {
@@ -592,7 +594,7 @@ static int parse_audio_format_rates_v2v3(struct snd_usb_audio *chip, 
	ret = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_RANGE,

			      USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,

			      UAC2_CS_CONTROL_SAM_FREQ << 8,

			      snd_usb_ctrl_intf(chip) | (clock << 8),

			      snd_usb_ctrl_intf(ctrl_intf) | (clock << 8),

			      data, data_size);



	if (ret < 0) {

sound/usb/helper.c

+34 −0
Original line number Diff line number Diff line
@@ -130,3 +130,37 @@ snd_usb_get_host_interface(struct snd_usb_audio *chip, int ifnum, int altsetting 
		return NULL;

	return usb_altnum_to_altsetting(iface, altsetting);

}



int snd_usb_add_ctrl_interface_link(struct snd_usb_audio *chip, int ifnum,

		int ctrlif)

{

	struct usb_device *dev = chip->dev;

	struct usb_host_interface *host_iface;



	if (chip->num_intf_to_ctrl >= MAX_CARD_INTERFACES) {

		dev_info(&dev->dev, "Too many interfaces assigned to the single USB-audio card\n");

		return -EINVAL;

	}



	/* find audiocontrol interface */

	host_iface = &usb_ifnum_to_if(dev, ctrlif)->altsetting[0];



	chip->intf_to_ctrl[chip->num_intf_to_ctrl].interface = ifnum;

	chip->intf_to_ctrl[chip->num_intf_to_ctrl].ctrl_intf = host_iface;

	chip->num_intf_to_ctrl++;



	return 0;

}



struct usb_host_interface *snd_usb_find_ctrl_interface(struct snd_usb_audio *chip,

							int ifnum)

{

	int i;



	for (i = 0; i < chip->num_intf_to_ctrl; ++i)

		if (chip->intf_to_ctrl[i].interface == ifnum)

			return chip->intf_to_ctrl[i].ctrl_intf;



	/* Fallback to first audiocontrol interface */

	return chip->ctrl_intf;

}

sound/usb/helper.h

+8 −2
Original line number Diff line number Diff line
@@ -17,6 +17,12 @@ unsigned char snd_usb_parse_datainterval(struct snd_usb_audio *chip, 
struct usb_host_interface *

snd_usb_get_host_interface(struct snd_usb_audio *chip, int ifnum, int altsetting);



int snd_usb_add_ctrl_interface_link(struct snd_usb_audio *chip, int ifnum,

		int ctrlif);



struct usb_host_interface *snd_usb_find_ctrl_interface(struct snd_usb_audio *chip,

								int ifnum);



/*

 * retrieve usb_interface descriptor from the host interface

 * (conditional for compatibility with the older API)
@@ -28,9 +34,9 @@ snd_usb_get_host_interface(struct snd_usb_audio *chip, int ifnum, int altsetting 


#define snd_usb_get_speed(dev) ((dev)->speed)



static inline int snd_usb_ctrl_intf(struct snd_usb_audio *chip)

static inline int snd_usb_ctrl_intf(struct usb_host_interface *ctrl_intf)

{

	return get_iface_desc(chip->ctrl_intf)->bInterfaceNumber;

	return get_iface_desc(ctrl_intf)->bInterfaceNumber;

}



/* in validate.c */