diff mbox

[1/1] ALSA: usb: initial USB Audio Device Class 3.0 support

Message ID 1510020080-15849-2-git-send-email-ruslan.bilovol@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ruslan Bilovol Nov. 7, 2017, 2:01 a.m. UTC
Recently released USB Audio Class 3.0 specification
introduces many significant changes comparing to
previous versions, like
 - new Power Domains, support for LPM/L1
 - new Cluster descriptor
 - changed layout of all class-specific descriptors
 - new High Capability descriptors
 - New class-specific String descriptors
 - new and removed units
 - additional sources for interrupts
 - removed Type II Audio Data Formats
 - ... and many other things (check spec)

It also provides backward compatibility through
multiple configurations, as well as requires
mandatory support for BADD (Basic Audio Device
Definition) on each ADC3.0 compliant device

This patch adds initial support of UAC3 specification
that is enough for Generic I/O Profile (BAOF, BAIF)
device support from BADD document.

Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
---
 include/linux/usb/audio-v2.h   |   4 +-
 include/linux/usb/audio-v3.h   | 343 +++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/usb/audio.h |   1 +
 sound/usb/card.c               |   7 +-
 sound/usb/card.h               |   2 +-
 sound/usb/clock.c              | 228 ++++++++++++++++++++++++---
 sound/usb/clock.h              |   4 +-
 sound/usb/format.c             |  87 +++++++++--
 sound/usb/format.h             |   6 +-
 sound/usb/mixer.c              | 332 ++++++++++++++++++++++++++-------------
 sound/usb/stream.c             | 296 ++++++++++++++++++++++++++++++-----
 11 files changed, 1117 insertions(+), 193 deletions(-)
 create mode 100644 include/linux/usb/audio-v3.h

Comments

Takashi Iwai Nov. 8, 2017, 2:38 p.m. UTC | #1
On Tue, 07 Nov 2017 03:01:20 +0100,
Ruslan Bilovol wrote:
> 
> Recently released USB Audio Class 3.0 specification
> introduces many significant changes comparing to
> previous versions, like
>  - new Power Domains, support for LPM/L1
>  - new Cluster descriptor
>  - changed layout of all class-specific descriptors
>  - new High Capability descriptors
>  - New class-specific String descriptors
>  - new and removed units
>  - additional sources for interrupts
>  - removed Type II Audio Data Formats
>  - ... and many other things (check spec)
> 
> It also provides backward compatibility through
> multiple configurations, as well as requires
> mandatory support for BADD (Basic Audio Device
> Definition) on each ADC3.0 compliant device
> 
> This patch adds initial support of UAC3 specification
> that is enough for Generic I/O Profile (BAOF, BAIF)
> device support from BADD document.
> 
> Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>

The patch looks good, but the timing is fairly late for merging to
4.15.

So from my side, the primary question is whether the changes in USB
(audio) header files are OK for USB guys.

Greg, could you check these changes and give an ack if it's OK to
merge?  Or if you prefer postpone, just let me know.


Below are some nitpicking:

> --- a/include/linux/usb/audio-v2.h
> +++ b/include/linux/usb/audio-v2.h
> @@ -33,12 +33,12 @@
>   *
>   */
>  
> -static inline bool uac2_control_is_readable(u32 bmControls, u8 control)
> +static inline bool uac_v2v3_control_is_readable(u32 bmControls, u8 control)
>  {
>  	return (bmControls >> (control * 2)) & 0x1;
>  }
>  
> -static inline bool uac2_control_is_writeable(u32 bmControls, u8 control)
> +static inline bool uac_v2v3_control_is_writeable(u32 bmControls, u8 control)
>  {
>  	return (bmControls >> (control * 2)) & 0x2;
>  }

This change looks a bit strange, but later one I noticed the reason
you didn't copy these functions into usb/audio-v3.h.  Though, I guess
a compiler can optimize out even if you call the same inline code
twice for v2 and v3...

In anyway, if we do rename, let's mention about it in the comment.


> diff --git a/include/linux/usb/audio-v3.h b/include/linux/usb/audio-v3.h
> new file mode 100644
> index 0000000..cbbe51e
> --- /dev/null
> +++ b/include/linux/usb/audio-v3.h
> @@ -0,0 +1,343 @@
> +// SPDX-License-Identifier: GPL-2.0+

You can use the C-style comment for SPDX line,
  /* SPDX-License-Identifier: GPL-2.0+ */

> +static struct uac3_clock_source_descriptor *
> +	snd_usb_find_clock_source_v3(struct usb_host_interface *ctrl_iface,
> +				  int clock_id)
> +{
> +	struct uac3_clock_source_descriptor *cs = NULL;
> +
> +	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
> +					     ctrl_iface->extralen,
> +					     cs, UAC3_CLOCK_SOURCE))) {
> +		if (cs->bClockID == clock_id)
> +			return cs;
> +	}
> +
> +	return NULL;
> +}

We may clean up the whole find_clock_xxx stuff later, but let's keep
it dumb and straight for now...


> +static int __uac3_clock_find_source(struct snd_usb_audio *chip, int entity_id,
> +				   unsigned long *visited, bool validate)
> +{
(snip)
> +		/* the entity ID we are looking for is a selector.
> +		 * find out what it currently selects */

The multi-line comment should be properly formatted like
  /*
   * XXX
   */

> +		/* The current clock source is invalid, try others. */
> +		for (i = 1; i <= selector->bNrInPins; i++) {
> +			int err;
> +
> +			if (i == cur)
> +				continue;
> +
> +			ret = __uac3_clock_find_source(chip, selector->baCSourceID[i - 1],
> +				visited, true);
> +			if (ret < 0)
> +				continue;
> +
> +			err = uac_clock_selector_set_val(chip, entity_id, i);
> +			if (err < 0)
> +				continue;
> +
> +			usb_audio_info(chip,
> +				 "found and selected valid clock source %d\n",
> +				 ret);

Do we need to print this at each time?


>  static int parse_audio_format_i(struct snd_usb_audio *chip,
> -				struct audioformat *fp, unsigned int format,
> -				struct uac_format_type_i_continuous_descriptor *fmt)
> +				struct audioformat *fp, u64 format,
> +				void *_fmt)
>  {
>  	snd_pcm_format_t pcm_format;
> +	unsigned int fmt_type;
>  	int ret;
>  
> -	if (fmt->bFormatType == UAC_FORMAT_TYPE_III) {
> +	switch (fp->protocol) {
> +	default:
> +	case UAC_VERSION_1:
> +	case UAC_VERSION_2: {
> +		struct uac_format_type_i_continuous_descriptor *fmt = _fmt;
> +
> +		fmt_type = fmt->bFormatType;
> +		break;
> +	}
> +	case UAC_VERSION_3: {
> +		/* fp->fmt_type is already set in this case */
> +		fmt_type = fp->fmt_type;
> +		break;
> +	}
> +	}

The double-braces don't look beautiful.  And in this case it's just
for the temporary fmt variable, so it can be moved to the top level
and drop the internal whole braces.


> @@ -405,15 +435,20 @@ static int parse_audio_format_i(struct snd_usb_audio *chip,
>  	 */
>  	switch (fp->protocol) {
>  	default:
> -	case UAC_VERSION_1:
> +	case UAC_VERSION_1: {
> +		struct uac_format_type_i_continuous_descriptor *fmt = _fmt;
> +
>  		fp->channels = fmt->bNrChannels;
>  		ret = parse_audio_format_rates_v1(chip, fp, (unsigned char *) fmt, 7);
>  		break;
> +	}
>  	case UAC_VERSION_2:
> +	case UAC_VERSION_3: {
>  		/* fp->channels is already set in this case */
> -		ret = parse_audio_format_rates_v2(chip, fp);
> +		ret = parse_audio_format_rates_v2v3(chip, fp);
>  		break;
>  	}
> +	}

DItto.

> @@ -520,3 +555,23 @@ int snd_usb_parse_audio_format(struct snd_usb_audio *chip,
>  	return 0;
>  }
>  
> +int snd_usb_parse_audio_format_v3(struct snd_usb_audio *chip,
> +			       struct audioformat *fp,
> +			       struct uac3_as_header_descriptor *as,
> +			       int stream)
> +{
> +	u64 format = le64_to_cpu(as->bmFormats);
> +	int err;
> +
> +	/* Type I format bits are D0..D6 */
> +	if (format & 0x7f)
> +		fp->fmt_type = UAC_FORMAT_TYPE_I;
> +	else
> +		fp->fmt_type = UAC_FORMAT_TYPE_III;
> +
> +	err = parse_audio_format_i(chip, fp, format, as);
> +	if (err < 0)
> +		return err;
> +
> +	return 0;

You can simply return from parse_audio_format_i() without err check.

> @@ -460,9 +461,10 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
>  
>  	validx += cval->idx_off;
>  
> +

Superfluous blank line.

> @@ -718,16 +721,104 @@ static int check_input_term(struct mixer_build *state, int id,
> +				} else { /* UAC_VERSION_2 */
> +					struct uac2_input_terminal_descriptor *d = p1;
> +
> +					/* call recursively to verify that the
> +					 * referenced clock entity is valid */

Fix the comment style.

> @@ -624,38 +708,158 @@ int snd_usb_parse_audio_interface(struct snd_usb_audio *chip, int iface_no)
>  				iface_no, altno, as->bTerminalLink);
>  			continue;
>  		}
> -		}
>  
> -		/* get format type */
> -		fmt = snd_usb_find_csint_desc(alts->extra, alts->extralen, NULL, UAC_FORMAT_TYPE);
> -		if (!fmt) {
> +		case UAC_VERSION_3: {
> +			struct uac3_input_terminal_descriptor *input_term;
> +			struct uac3_output_terminal_descriptor *output_term;
> +			struct uac3_as_header_descriptor *as;
> +			struct uac3_cluster_header_descriptor *cluster;
> +			struct uac3_hc_descriptor_header hc_header;
> +			u16 cluster_id, wLength;
(snip)

Now snd_usb_parse_audio_interface() becomes too lengthy and hard to
read through.  It'd be great if you can split / cleanup this in
another patch.


thanks,

Takashi
kernel test robot Nov. 8, 2017, 4:46 p.m. UTC | #2
Hi Ruslan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on sound/for-next]
[also build test WARNING on v4.14-rc8 next-20171108]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ruslan-Bilovol/USB-Audio-Device-Class-3-0-support/20171108-235800
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next
config: mips-allyesconfig (attached as .config)
compiler: mips-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=mips 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   sound/usb/stream.c: In function 'snd_usb_parse_audio_interface':
>> sound/usb/stream.c:917:8: warning: 'fmt' may be used uninitialized in this function [-Wmaybe-uninitialized]
       if (snd_usb_parse_audio_format(chip, fp, format,
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           fmt, stream) < 0) {
           ~~~~~~~~~~~~

vim +/fmt +917 sound/usb/stream.c

   561	
   562	int snd_usb_parse_audio_interface(struct snd_usb_audio *chip, int iface_no)
   563	{
   564		struct usb_device *dev;
   565		struct usb_interface *iface;
   566		struct usb_host_interface *alts;
   567		struct usb_interface_descriptor *altsd;
   568		int i, altno, err, stream;
   569		u64 format = 0;
   570		unsigned int num_channels = 0;
   571		struct audioformat *fp = NULL;
   572		int num, protocol, clock = 0;
   573		struct uac_format_type_i_continuous_descriptor *fmt;
   574		struct snd_pcm_chmap_elem *chmap_v3 = NULL;
   575		unsigned int chconfig;
   576	
   577		dev = chip->dev;
   578	
   579		/* parse the interface's altsettings */
   580		iface = usb_ifnum_to_if(dev, iface_no);
   581	
   582		num = iface->num_altsetting;
   583	
   584		/*
   585		 * Dallas DS4201 workaround: It presents 5 altsettings, but the last
   586		 * one misses syncpipe, and does not produce any sound.
   587		 */
   588		if (chip->usb_id == USB_ID(0x04fa, 0x4201))
   589			num = 4;
   590	
   591		for (i = 0; i < num; i++) {
   592			alts = &iface->altsetting[i];
   593			altsd = get_iface_desc(alts);
   594			protocol = altsd->bInterfaceProtocol;
   595			/* skip invalid one */
   596			if (((altsd->bInterfaceClass != USB_CLASS_AUDIO ||
   597			      (altsd->bInterfaceSubClass != USB_SUBCLASS_AUDIOSTREAMING &&
   598			       altsd->bInterfaceSubClass != USB_SUBCLASS_VENDOR_SPEC)) &&
   599			     altsd->bInterfaceClass != USB_CLASS_VENDOR_SPEC) ||
   600			    altsd->bNumEndpoints < 1 ||
   601			    le16_to_cpu(get_endpoint(alts, 0)->wMaxPacketSize) == 0)
   602				continue;
   603			/* must be isochronous */
   604			if ((get_endpoint(alts, 0)->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) !=
   605			    USB_ENDPOINT_XFER_ISOC)
   606				continue;
   607			/* check direction */
   608			stream = (get_endpoint(alts, 0)->bEndpointAddress & USB_DIR_IN) ?
   609				SNDRV_PCM_STREAM_CAPTURE : SNDRV_PCM_STREAM_PLAYBACK;
   610			altno = altsd->bAlternateSetting;
   611	
   612			if (snd_usb_apply_interface_quirk(chip, iface_no, altno))
   613				continue;
   614	
   615			/*
   616			 * Roland audio streaming interfaces are marked with protocols
   617			 * 0/1/2, but are UAC 1 compatible.
   618			 */
   619			if (USB_ID_VENDOR(chip->usb_id) == 0x0582 &&
   620			    altsd->bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
   621			    protocol <= 2)
   622				protocol = UAC_VERSION_1;
   623	
   624			chconfig = 0;
   625			/* get audio formats */
   626			switch (protocol) {
   627			default:
   628				dev_dbg(&dev->dev, "%u:%d: unknown interface protocol %#02x, assuming v1\n",
   629					iface_no, altno, protocol);
   630				protocol = UAC_VERSION_1;
   631				/* fall through */
   632	
   633			case UAC_VERSION_1: {
   634				struct uac1_as_header_descriptor *as =
   635					snd_usb_find_csint_desc(alts->extra, alts->extralen, NULL, UAC_AS_GENERAL);
   636				struct uac_input_terminal_descriptor *iterm;
   637	
   638				if (!as) {
   639					dev_err(&dev->dev,
   640						"%u:%d : UAC_AS_GENERAL descriptor not found\n",
   641						iface_no, altno);
   642					continue;
   643				}
   644	
   645				if (as->bLength < sizeof(*as)) {
   646					dev_err(&dev->dev,
   647						"%u:%d : invalid UAC_AS_GENERAL desc\n",
   648						iface_no, altno);
   649					continue;
   650				}
   651	
   652				format = le16_to_cpu(as->wFormatTag); /* remember the format value */
   653	
   654				iterm = snd_usb_find_input_terminal_descriptor(chip->ctrl_intf,
   655									       as->bTerminalLink);
   656				if (iterm) {
   657					num_channels = iterm->bNrChannels;
   658					chconfig = le16_to_cpu(iterm->wChannelConfig);
   659				}
   660	
   661				break;
   662			}
   663	
   664			case UAC_VERSION_2: {
   665				struct uac2_input_terminal_descriptor *input_term;
   666				struct uac2_output_terminal_descriptor *output_term;
   667				struct uac2_as_header_descriptor *as =
   668					snd_usb_find_csint_desc(alts->extra, alts->extralen, NULL, UAC_AS_GENERAL);
   669	
   670				if (!as) {
   671					dev_err(&dev->dev,
   672						"%u:%d : UAC_AS_GENERAL descriptor not found\n",
   673						iface_no, altno);
   674					continue;
   675				}
   676	
   677				if (as->bLength < sizeof(*as)) {
   678					dev_err(&dev->dev,
   679						"%u:%d : invalid UAC_AS_GENERAL desc\n",
   680						iface_no, altno);
   681					continue;
   682				}
   683	
   684				num_channels = as->bNrChannels;
   685				format = le32_to_cpu(as->bmFormats);
   686				chconfig = le32_to_cpu(as->bmChannelConfig);
   687	
   688				/* lookup the terminal associated to this interface
   689				 * to extract the clock */
   690				input_term = snd_usb_find_input_terminal_descriptor(chip->ctrl_intf,
   691										    as->bTerminalLink);
   692				if (input_term) {
   693					clock = input_term->bCSourceID;
   694					if (!chconfig && (num_channels == input_term->bNrChannels))
   695						chconfig = le32_to_cpu(input_term->bmChannelConfig);
   696					break;
   697				}
   698	
   699				output_term = snd_usb_find_output_terminal_descriptor(chip->ctrl_intf,
   700										      as->bTerminalLink);
   701				if (output_term) {
   702					clock = output_term->bCSourceID;
   703					break;
   704				}
   705	
   706				dev_err(&dev->dev,
   707					"%u:%d : bogus bTerminalLink %d\n",
   708					iface_no, altno, as->bTerminalLink);
   709				continue;
   710			}
   711	
   712			case UAC_VERSION_3: {
   713				struct uac3_input_terminal_descriptor *input_term;
   714				struct uac3_output_terminal_descriptor *output_term;
   715				struct uac3_as_header_descriptor *as;
   716				struct uac3_cluster_header_descriptor *cluster;
   717				struct uac3_hc_descriptor_header hc_header;
   718				u16 cluster_id, wLength;
   719	
   720				as = snd_usb_find_csint_desc(alts->extra,
   721								alts->extralen,
   722								NULL, UAC_AS_GENERAL);
   723	
   724				if (!as) {
   725					dev_err(&dev->dev,
   726						"%u:%d : UAC_AS_GENERAL descriptor not found\n",
   727						iface_no, altno);
   728					continue;
   729				}
   730	
   731				if (as->bLength < sizeof(*as)) {
   732					dev_err(&dev->dev,
   733						"%u:%d : invalid UAC_AS_GENERAL desc\n",
   734						iface_no, altno);
   735					continue;
   736				}
   737	
   738				cluster_id = le16_to_cpu(as->wClusterDescrID);
   739				if (!cluster_id) {
   740					dev_err(&dev->dev,
   741						"%u:%d : no cluster descriptor\n",
   742						iface_no, altno);
   743					continue;
   744				}
   745	
   746				/*
   747				 * Get number of channels and channel map through
   748				 * High Capability Cluster Descriptor
   749				 *
   750				 * First step: get High Capability header and
   751				 * read size of Cluster Descriptor
   752				 */
   753				err = snd_usb_ctl_msg(chip->dev,
   754						usb_rcvctrlpipe(chip->dev, 0),
   755						UAC3_CS_REQ_HIGH_CAPABILITY_DESCRIPTOR,
   756						USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
   757						cluster_id,
   758						snd_usb_ctrl_intf(chip),
   759						&hc_header, sizeof(hc_header));
   760				if (err < 0)
   761					return err;
   762				else if (err != sizeof(hc_header)) {
   763					dev_err(&dev->dev,
   764						"%u:%d : can't get High Capability descriptor\n",
   765						iface_no, altno);
   766					return -EIO;
   767				}
   768	
   769				/*
   770				 * Second step: allocate needed amount of memory
   771				 * and request Cluster Descriptor
   772				 */
   773				wLength = le16_to_cpu(hc_header.wLength);
   774				cluster = kzalloc(wLength, GFP_KERNEL);
   775				if (!cluster)
   776					return -ENOMEM;
   777				err = snd_usb_ctl_msg(chip->dev,
   778						usb_rcvctrlpipe(chip->dev, 0),
   779						UAC3_CS_REQ_HIGH_CAPABILITY_DESCRIPTOR,
   780						USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
   781						cluster_id,
   782						snd_usb_ctrl_intf(chip),
   783						cluster, wLength);
   784				if (err < 0) {
   785					kfree(cluster);
   786					return err;
   787				} else if (err != wLength) {
   788					dev_err(&dev->dev,
   789						"%u:%d : can't get Cluster Descriptor\n",
   790						iface_no, altno);
   791					kfree(cluster);
   792					return -EIO;
   793				}
   794	
   795				num_channels = cluster->bNrChannels;
   796				chmap_v3 = convert_chmap_v3(cluster);
   797	
   798				kfree(cluster);
   799	
   800				format = le64_to_cpu(as->bmFormats);
   801	
   802				/* lookup the terminal associated to this interface
   803				 * to extract the clock */
   804				input_term = snd_usb_find_input_terminal_descriptor(
   805								chip->ctrl_intf,
   806								as->bTerminalLink);
   807	
   808				if (input_term) {
   809					clock = input_term->bCSourceID;
   810					break;
   811				}
   812	
   813				output_term = snd_usb_find_output_terminal_descriptor(chip->ctrl_intf,
   814										      as->bTerminalLink);
   815				if (output_term) {
   816					clock = output_term->bCSourceID;
   817					break;
   818				}
   819	
   820				dev_err(&dev->dev,
   821					"%u:%d : bogus bTerminalLink %d\n",
   822					iface_no, altno, as->bTerminalLink);
   823				continue;
   824			}
   825			}
   826	
   827			if (protocol == UAC_VERSION_1 || protocol == UAC_VERSION_2) {
   828				/* get format type */
   829				fmt = snd_usb_find_csint_desc(alts->extra,
   830							      alts->extralen,
   831							      NULL, UAC_FORMAT_TYPE);
   832				if (!fmt) {
   833					dev_err(&dev->dev,
   834						"%u:%d : no UAC_FORMAT_TYPE desc\n",
   835						iface_no, altno);
   836					continue;
   837				}
   838				if (((protocol == UAC_VERSION_1) && (fmt->bLength < 8))
   839						|| ((protocol == UAC_VERSION_2) &&
   840								(fmt->bLength < 6))) {
   841					dev_err(&dev->dev,
   842						"%u:%d : invalid UAC_FORMAT_TYPE desc\n",
   843						iface_no, altno);
   844					continue;
   845				}
   846	
   847				/*
   848				 * Blue Microphones workaround: The last altsetting is
   849				 * identical with the previous one, except for a larger
   850				 * packet size, but is actually a mislabeled two-channel
   851				 * setting; ignore it.
   852				 */
   853				if (fmt->bNrChannels == 1 &&
   854				    fmt->bSubframeSize == 2 &&
   855				    altno == 2 && num == 3 &&
   856				    fp && fp->altsetting == 1 && fp->channels == 1 &&
   857				    fp->formats == SNDRV_PCM_FMTBIT_S16_LE &&
   858				    protocol == UAC_VERSION_1 &&
   859				    le16_to_cpu(get_endpoint(alts, 0)->wMaxPacketSize) ==
   860								fp->maxpacksize * 2)
   861					continue;
   862			}
   863	
   864			fp = kzalloc(sizeof(*fp), GFP_KERNEL);
   865			if (!fp)
   866				return -ENOMEM;
   867	
   868			fp->iface = iface_no;
   869			fp->altsetting = altno;
   870			fp->altset_idx = i;
   871			fp->endpoint = get_endpoint(alts, 0)->bEndpointAddress;
   872			fp->ep_attr = get_endpoint(alts, 0)->bmAttributes;
   873			fp->datainterval = snd_usb_parse_datainterval(chip, alts);
   874			fp->protocol = protocol;
   875			fp->maxpacksize = le16_to_cpu(get_endpoint(alts, 0)->wMaxPacketSize);
   876			fp->channels = num_channels;
   877			if (snd_usb_get_speed(dev) == USB_SPEED_HIGH)
   878				fp->maxpacksize = (((fp->maxpacksize >> 11) & 3) + 1)
   879						* (fp->maxpacksize & 0x7ff);
   880			fp->attributes = parse_uac_endpoint_attributes(chip, alts, protocol, iface_no);
   881			fp->clock = clock;
   882			INIT_LIST_HEAD(&fp->list);
   883	
   884			/* some quirks for attributes here */
   885	
   886			switch (chip->usb_id) {
   887			case USB_ID(0x0a92, 0x0053): /* AudioTrak Optoplay */
   888				/* Optoplay sets the sample rate attribute although
   889				 * it seems not supporting it in fact.
   890				 */
   891				fp->attributes &= ~UAC_EP_CS_ATTR_SAMPLE_RATE;
   892				break;
   893			case USB_ID(0x041e, 0x3020): /* Creative SB Audigy 2 NX */
   894			case USB_ID(0x0763, 0x2003): /* M-Audio Audiophile USB */
   895				/* doesn't set the sample rate attribute, but supports it */
   896				fp->attributes |= UAC_EP_CS_ATTR_SAMPLE_RATE;
   897				break;
   898			case USB_ID(0x0763, 0x2001):  /* M-Audio Quattro USB */
   899			case USB_ID(0x0763, 0x2012):  /* M-Audio Fast Track Pro USB */
   900			case USB_ID(0x047f, 0x0ca1): /* plantronics headset */
   901			case USB_ID(0x077d, 0x07af): /* Griffin iMic (note that there is
   902							an older model 77d:223) */
   903			/*
   904			 * plantronics headset and Griffin iMic have set adaptive-in
   905			 * although it's really not...
   906			 */
   907				fp->ep_attr &= ~USB_ENDPOINT_SYNCTYPE;
   908				if (stream == SNDRV_PCM_STREAM_PLAYBACK)
   909					fp->ep_attr |= USB_ENDPOINT_SYNC_ADAPTIVE;
   910				else
   911					fp->ep_attr |= USB_ENDPOINT_SYNC_SYNC;
   912				break;
   913			}
   914	
   915			/* ok, let's parse further... */
   916			if (protocol == UAC_VERSION_1 || protocol == UAC_VERSION_2) {
 > 917				if (snd_usb_parse_audio_format(chip, fp, format,
   918								fmt, stream) < 0) {
   919					kfree(fp->rate_table);
   920					kfree(fp);
   921					fp = NULL;
   922					continue;
   923				}
   924			} else {
   925				struct uac3_as_header_descriptor *as;
   926	
   927				as = snd_usb_find_csint_desc(alts->extra,
   928							     alts->extralen,
   929							     NULL, UAC_AS_GENERAL);
   930	
   931				if (snd_usb_parse_audio_format_v3(chip, fp, as,
   932									stream) < 0) {
   933					kfree(fp->rate_table);
   934					kfree(fp);
   935					fp = NULL;
   936					continue;
   937				}
   938			}
   939	
   940			/* Create chmap */
   941			if (fp->channels != num_channels)
   942				chconfig = 0;
   943	
   944			if (protocol == UAC_VERSION_3)
   945				fp->chmap = chmap_v3;
   946			else
   947				fp->chmap = convert_chmap(fp->channels, chconfig,
   948							  protocol);
   949	
   950			dev_dbg(&dev->dev, "%u:%d: add audio endpoint %#x\n", iface_no, altno, fp->endpoint);
   951			err = snd_usb_add_audio_stream(chip, stream, fp);
   952			if (err < 0) {
   953				list_del(&fp->list); /* unlink for avoiding double-free */
   954				kfree(fp->rate_table);
   955				kfree(fp->chmap);
   956				kfree(fp);
   957				return err;
   958			}
   959			/* try to set the interface... */
   960			usb_set_interface(chip->dev, iface_no, altno);
   961			snd_usb_init_pitch(chip, iface_no, alts, fp);
   962			snd_usb_init_sample_rate(chip, iface_no, alts, fp, fp->rate_max);
   963		}
   964		return 0;
   965	}
   966	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Pierre-Louis Bossart Nov. 8, 2017, 7:38 p.m. UTC | #3
Nice work, thanks! I double-checked all the descriptors and values and 
didn't find anything problematic, the main comment I have is that the 
clock source/selection could probably be refactored since the 
differences are really minor with UAC2, and there is work to do to 
select the right Audio Interface Association.

On 11/6/17 8:01 PM, Ruslan Bilovol wrote:
> Recently released USB Audio Class 3.0 specification
> introduces many significant changes comparing to
> previous versions, like
>   - new Power Domains, support for LPM/L1
>   - new Cluster descriptor
>   - changed layout of all class-specific descriptors
>   - new High Capability descriptors
>   - New class-specific String descriptors
>   - new and removed units
>   - additional sources for interrupts
>   - removed Type II Audio Data Formats
>   - ... and many other things (check spec)
> 
> It also provides backward compatibility through
> multiple configurations, as well as requires
> mandatory support for BADD (Basic Audio Device
> Definition) on each ADC3.0 compliant device
> 
> This patch adds initial support of UAC3 specification
> that is enough for Generic I/O Profile (BAOF, BAIF)
> device support from BADD document.

Do you mean to say that the selection of the BADD profile or full-blown 
descriptor capabilities will be handled in a follow-up patch?

It's my understanding from Section 3.3 that a UAC3 device is required to 
expose
- a backwards-compatible configuration (UAC1 or UAC2),
- a UAC3 BADD profile
- one or more AIA compliant with UAC3 (which may have additional 
features not present in the baseline description).

And I am not sure how a driver would make the selection...

In theory though, a UAC3 device should work out of the box even if the 
host only supports UAC1 or UAC2.


> +/*
> + * v1.0, v2.0 and v3.0 of this standard have many things in common. For the rest
> + * of the definitions, please refer to audio.h and audio-v2.h
> + */
> +
> +/* All High Capability descriptors have these 2 fields at the beginning */
> +struct uac3_hc_descriptor_header {
> +	__le16 wLength;
> +	__u8 bDescriptorType;
> +	__u8 bDescriptorSubtype;
> +	__le16 wDescriptorID;
> +} __attribute__ ((packed));
> +
> +/* 4.3.1 CLUSTER DESCRIPTOR HEADER */
> +struct uac3_cluster_header_descriptor {
> +	__le16 wLength;
> +	__u8 bDescriptorType;
> +	__u8 bDescriptorSubtype;
> +	__le16 wDescriptorID;
> +	__u8 bNrChannels;
> +} __attribute__ ((packed));
> +
> +/* 4.3.2.1 SEGMENTS */
> +struct uac3_cluster_segment_descriptor {
> +	__le16 wLength;
> +	__u8 bSegmentType;
> +	/* __u8[0]; segment-specific data */
> +} __attribute__ ((packed));
> +
> +/* 4.3.2.1.1 END SEGMENT */
> +struct uac3_cluster_end_segment_descriptor {
> +	__le16 wLength;
> +	__u8 bSegmentType;		/* Constant END_SEGMENT */
> +} __attribute__ ((packed));
> +

you didn't include the definitions in 4.3.2.1.2
Cluster Descriptor Segment
Vendor Defined Segment

> +/* 4.3.2.1.3.1 INFORMATION SEGMENT */
> +struct uac3_cluster_information_segment_descriptor {
> +	__le16 wLength;
> +	__u8 bSegmentType;

this field is a CHANNEL_INFORMATION constant.

> +	__u8 bChPurpose;
> +	__u8 bChRelationship;
> +	__u8 bChGroupID;
> +} __attribute__ ((packed));
> +
> +/* 4.5.2 CLASS-SPECIFIC AC INTERFACE DESCRIPTOR */
> +struct uac3_ac_header_descriptor {
> +	__u8 bLength;			/* 10 */
> +	__u8 bDescriptorType;		/* CS_INTERFACE descriptor type */
> +	__u8 bDescriptorSubtype;	/* HEADER descriptor subtype */
> +	__u8 bCategory;
> +
> +	/* includes Clock Source, Unit, Terminal, and Power Domain desc. */
> +	__le16 wTotalLength;
> +
> +	__le32 bmControls;
> +} __attribute__ ((packed));
> +

Missing extended terminal descriptor header from 4.5.2.3.2?

Missing multi-function processing unit descriptor header from 4.5.2.10.3 ?

> +
> +/* bmAttribute fields */
> +#define UAC3_CLOCK_SOURCE_TYPE_EXT	0x0
> +#define UAC3_CLOCK_SOURCE_TYPE_INT	0x1
> +#define UAC3_CLOCK_SOURCE_ASYNC		(0 << 2)
> +#define UAC3_CLOCK_SOURCE_SYNCED_TO_SOF	(1 << 1)
> +
> +/* 4.5.2.13 CLOCK SELECTOR DESCRIPTOR */
> +struct uac3_clock_selector_descriptor {
> +	__u8 bLength;
> +	__u8 bDescriptorType;
> +	__u8 bDescriptorSubtype;
> +	__u8 bClockID;
> +	__u8 bNrInPins;
> +	__u8 baCSourceID[];
> +	/* bmControls and wCSelectorDescrStr omitted */

why is this omitted here and not below in the clock multiplier descriptor?

> +} __attribute__((packed) > +
> +/* 4.5.2.14 CLOCK MULTIPLIER DESCRIPTOR */
> +struct uac3_clock_multiplier_descriptor {
> +	__u8 bLength;
> +	__u8 bDescriptorType;
> +	__u8 bDescriptorSubtype;
> +	__u8 bClockID;
> +	__u8 bCSourceID;
> +	__le32 bmControls;
> +	__le16 wCMultiplierDescrStr;
> +} __attribute__((packed));

[snip]

> +
> +/* 4.5.2.15 POWER DOMAIN DESCRIPTOR */
> +struct uac3_power_domain_descriptor {
> +	__u8 bLength;
> +	__u8 bDescriptorType;
> +	__u8 bDescriptorSubtype;
> +	__u8 bPowerDomainID;
> +	__le16 waRecoveryTime1;
> +	__le16 waRecoveryTime2;
> +	__u8 bNrEntities;
> +	__u8 baEntityID[];
> +	/* wPDomainDescrStr omitted */
why?
> +} __attribute__((packed));
> +
> +/* As above, but more useful for defining your own descriptors */
> +#define DECLARE_UAC3_POWER_DOMAIN_DESCRIPTOR(n)			\
> +struct uac3_power_domain_descriptor_##n {			\
> +	__u8 bLength;						\
> +	__u8 bDescriptorType;					\
> +	__u8 bDescriptorSubtype;				\
> +	__u8 bPowerDomainID;					\
> +	__le16 waRecoveryTime1;					\
> +	__le16 waRecoveryTime2;					\
> +	__u8 bNrEntities;					\
> +	__u8 baEntityID[n];					\
> +	__le16 wPDomainDescrStr;					\
> +} __attribute__ ((packed))
> +

any specific reason why the descriptors are not added in linear order, 
following the spec definition? all the descriptors below could be added 
earlier.

> +/* 4.5.2.1 INPUT TERMINAL DESCRIPTOR */
> +struct uac3_input_terminal_descriptor {
> +	__u8 bLength;
> +	__u8 bDescriptorType;
> +	__u8 bDescriptorSubtype;
> +	__u8 bTerminalID;
> +	__le16 wTerminalType;
> +	__u8 bAssocTerminal;
> +	__u8 bCSourceID;
> +	__le32 bmControls;
> +	__le16 wClusterDescrID;
> +	__le16 wExTerminalDescrID;
> +	__le16 wConnectorsDescrID;
> +	__le16 wTerminalDescrStr;
> +} __attribute__((packed));
> +

> +
> +/* 4.7.2 CLASS-SPECIFIC AS INTERFACE DESCRIPTOR */
> +struct uac3_as_header_descriptor {
> +	__u8 bLength;
> +	__u8 bDescriptorType;
> +	__u8 bDescriptorSubtype;
> +	__u8 bTerminalLink;
> +	__le32 bmControls;
> +	__le16 wClusterDescrID;
> +	__le64 bmFormats;
> +	__u8 bSubslotSize;
> +	__u8 bBitResolution;
> +	__le16 bmAuxProtocols;
> +	__u8 bControlSize;
> +} __attribute__((packed));
> +
> +#define UAC3_FORMAT_TYPE_I_RAW_DATA	(1 << 6)
This seems to come from Table A1 in the formats document, is it inserted 
here because of the relationship with the bmFormats? If yes, we should 
probably add the full list from Table A1 (as done below for the other codes)

> +
> +/* 4.8.1.2 CLASS-SPECIFIC AS ISOCHRONOUS AUDIO DATA ENDPOINT DESCRIPTOR */
> +struct uac3_iso_endpoint_descriptor {
> +	__u8 bLength;
> +	__u8 bDescriptorType;
> +	__u8 bDescriptorSubtype;
> +	__le32 bmControls;
> +	__u8 bLockDelayUnits;
> +	__le16 wLockDelay;
> +} __attribute__((packed));
> +
> +#define UAC3_CONTROL_PITCH		(3 << 0)
> +#define UAC3_CONTROL_DATA_OVERRUN	(3 << 2)
> +#define UAC3_CONTROL_DATA_UNDERRUN	(3 << 4)

these are really masks, no?

> +
> +/* 6.1 INTERRUPT DATA MESSAGE */
> +#define UAC3_INTERRUPT_DATA_MSG_VENDOR	(1 << 0)
> +#define UAC3_INTERRUPT_DATA_MSG_EP	(1 << 1)

this is the same as in UAC2

> +
> +struct uac3_interrupt_data_msg {
> +	__u8 bInfo;
> +	__u8 bSourceType;
> +	__le16 wValue;
> +	__le16 wIndex;
> +} __attribute__((packed));

This seems identical to UAC2, the difference being the bSourceType with 
additional values (was bAttribute).

> +
> +/* A.2 AUDIO AUDIO FUNCTION SUBCLASS CODES */
> +#define UAC3_FUNCTION_SUBCLASS_UNDEFINED	0x00
> +#define UAC3_FUNCTION_SUBCLASS_FULL_ADC_3_0	0x01
> +#define UAC3_FUNCTION_SUBCLASS_GENERIC_IO	0x20
> +#define UAC3_FUNCTION_SUBCLASS_HEADPHONE	0x21
> +#define UAC3_FUNCTION_SUBCLASS_SPEAKER		0x22
> +#define UAC3_FUNCTION_SUBCLASS_MICROPHONE	0x23
> +#define UAC3_FUNCTION_SUBCLASS_HEADSET		0x24
> +#define UAC3_FUNCTION_SUBCLASS_HEADSET_ADAPTER	0x25
> +#define UAC3_FUNCTION_SUBCLASS_SPEAKERPHONE	0x26

mention that these are linked to BADD profiles.

> +
> +/* A.7 AUDIO FUNCTION CATEGORY CODES */
> +#define UAC3_FUNCTION_SUBCLASS_UNDEFINED	0x00
> +#define UAC3_FUNCTION_DESKTOP_SPEAKER		0x01
> +#define UAC3_FUNCTION_HOME_THEATER		0x02
> +#define UAC3_FUNCTION_MICROPHONE		0x03
> +#define UAC3_FUNCTION_HEADSET			0x04
> +#define UAC3_FUNCTION_TELEPHONE			0x05
> +#define UAC3_FUNCTION_CONVERTER			0x06
> +#define UAC3_FUNCTION_SOUND_RECORDER		0x07
> +#define UAC3_FUNCTION_IO_BOX			0x08
> +#define UAC3_FUNCTION_MUSICAL_INSTRUMENT	0x09
> +#define UAC3_FUNCTION_PRO_AUDIO			0x0a
> +#define UAC3_FUNCTION_AUDIO_VIDEO		0x0b
> +#define UAC3_FUNCTION_CONTROL_PANEL		0x0c
> +#define UAC3_FUNCTION_HEADPHONE			0x0d
> +#define UAC3_FUNCTION_GENERIC_SPEAKER		0x0e
> +#define UAC3_FUNCTION_HEADSET_ADAPTER		0x0f
> +#define UAC3_FUNCTION_SPEAKERPHONE		0x10
> +#define UAC3_FUNCTION_OTHER			0xff
> +
> +/* A.8 AUDIO CLASS-SPECIFIC DESCRIPTOR TYPES */
> +#define UAC3_CS_UNDEFINED		0x20
> +#define UAC3_CS_DEVICE			0x21
> +#define UAC3_CS_CONFIGURATION		0x22
> +#define UAC3_CS_STRING			0x23
> +#define UAC3_CS_INTERFACE		0x24
> +#define UAC3_CS_ENDPOINT		0x25
> +#define UAC3_CS_CLUSTER			0x26
> +
> +/* A.10 CLUSTER DESCRIPTOR SEGMENT TYPES */
> +#define UAC3_SEGMENT_UNDEFINED		0x00
> +#define UAC3_CLUSTER_DESCRIPTION	0x01
> +#define UAC3_CLUSTER_VENDOR_DEFINED	0x1F
> +#define UAC3_CHANNEL_INFORMATION	0x20
> +#define UAC3_CHANNEL_AMBISONIC		0x21
> +#define UAC3_CHANNEL_DESCRIPTION	0x22
> +#define UAC3_CHANNEL_VENDOR_DEFINED	0xFE
> +#define UAC3_END_SEGMENT		0xFF
> +
> +/* A.11 CHANNEL PURPOSE DEFINITIONS */
> +#define UAC3_PURPOSE_UNDEFINED		0x00
> +#define UAC3_PURPOSE_GENERIC_AUDIO	0x01
> +#define UAC3_PURPOSE_VOICE		0x02
> +#define UAC3_PURPOSE_SPEECH		0x03
> +#define UAC3_PURPOSE_AMBIENT		0x04
> +#define UAC3_PURPOSE_REFERENCE		0x05
> +#define UAC3_PURPOSE_ULTRASONIC		0x06
> +#define UAC3_PURPOSE_VIBROKINETIC	0x07
> +#define UAC3_PURPOSE_NON_AUDIO		0xFF
> +
> +/* A.12 CHANNEL RELATIONSHIP DEFINITIONS */
> +/* FIXME: spec is missing these constants. Few found in BasicAudioDevice3.pdf */

yes, this is a known bug in the released UAC3 document, the values were 
removed by accident. If this helps, here's what I have from the last 
release candidate, this will hopefully be corrected soon.


RELATIONSHIP_UNDEFINED     UND  0x00
MONO			   M    0x01
LEFT			   L    0x02
RIGHT			   R    0x03
ARRAY			   AR   0x04
PATTERN_X		   PX   0x20
PATTERN_Y		   PY   0x21
PATTERN_A		   PA   0x22
PATTERN_B		   PB   0x23
PATTERN_M		   PM   0x24
PATTERN_S		   PS   0x25
Front Left		   FL   0x80
Front Right		   FR   0x81
Front Center		   FC   0x82
Front Left of Center	   FLC  0x83
Front Right of Center	   FRC  0x84
Front Wide Left		   FWL  0x85
Front Wide Right	   FWR  0x86
Side Left		   SL   0x87
Side Right		   SR   0x88
Surround Array Left	   SAL  0x89
Surround Array Right	   SAR  0x8A

Back Left                      BL      0x8B
Back Right		       BR      0x8C
Back Center		       BC      0x8D
Back Left of Center	       BLC     0x8E
Back Right of Center	       BRC     0x8F
Back Wide Left		       BWL     0x90
Back Wide Right		       BWR     0x91
Top Center		       TC      0x92
Top Front Left		       TFL     0x93
Top Front Right		       TFR     0x94
Top Front Center	       TFC     0x95
Top Front Left of Center       TFLC    0x96
Top Front Right of Center      TFRC    0x97
Top Front Wide Left	       TFWL    0x98
Top Front Wide Right	       TFWR    0x99
Top Side Left		       TSL     0x9A
Top Side Right		       TSR     0x9B
Top Surround Array Left	       TSAL    0x9C
Top Surround Array Right       TSAR    0x9D
Top Back Left		       TBL     0x9E
Top Back Right		       TBR     0x9F
Top Back Center		       TBC     0xA0
Top Back Left Of Center	       TBLC    0xA1
Top Back Right Of Center       TBRC    0xA2
Top Back Wide Left	       TBWL    0xA3
Top Back Wide Right	       TBWR    0xA4
Bottom Center 		       BC      0xA5
Bottom Front Left	       BFL     0xA6
Bottom Front Right	       BFR     0xA7
Bottom Front Center	       BFC     0xA8
Bottom Front Left Of Center    BFLC    0xA9
Bottom Front Right Of Center   BFRC    0xAA
Bottom Front Wide Left	       BFWL    0xAB
Bottom Front Wide Right	       BFWR    0xAC

Bottom Side Left             BSL    0xAD
Bottom Side Right	     BSR    0xAE
Bottom Surround Array Left   BSAL   0xAF
Bottom Surround Array Right  BSAR   0xB0
Bottom Back Left	     BBL    0xB1
Bottom Back Right	     BBR    0xB2
Bottom Back Center	     BBC    0xB3
Bottom Back Left Of Center   BBLC   0xB4
Bottom Back Right Of Center  BBRC   0xB5
Bottom Back Wide Left	     BBWL   0xB6
Bottom Back Wide Right	     BBWR   0xB7
Low Frequency Effects	     LFE    0xB8
Low Frequency Effects Left   LFEL   0xB9
Low Frequency Effects Right  LFER   0xBA
Headphone Left		     HPL    0xBB
Headphone Right		     HPR    0xBC


> +#define UAC3_CH_RELATIONSHIP_UNDEFINED	0x00
> +#define UAC3_CH_MONO			0x01
> +#define UAC3_CH_LEFT			0x02
> +#define UAC3_CH_RIGHT			0x03
> +#define UAC3_CH_FRONT_LEFT		0x80
> +#define UAC3_CH_FRONT_RIGHT		0x81
> +#define UAC3_CH_FRONT_CENTER		0x82
> +#define UAC3_CH_SURROUND_ARRAY_LEFT	0x89
> +#define UAC3_CH_SURROUND_ARRAY_RIGHT	0x8A
> +#define UAC3_CH_LOW_FREQUENCY_EFFECTS	0xB8
> +
> +/* A.15 AUDIO CLASS-SPECIFIC AC INTERFACE DESCRIPTOR SUBTYPES */
> +/* see audio.h for the rest, which is identical to v1 */
> +#define UAC3_EXTENDED_TERMINAL		0x04
> +#define UAC3_MIXER_UNIT			0x05
> +#define UAC3_SELECTOR_UNIT		0x06
> +#define UAC3_FEATURE_UNIT		0x07
> +#define UAC3_EFFECT_UNIT		0x08
> +#define UAC3_PROCESSING_UNIT		0x09
> +#define UAC3_EXTENSION_UNIT		0x0a
> +#define UAC3_CLOCK_SOURCE		0x0b
> +#define UAC3_CLOCK_SELECTOR		0x0c
> +#define UAC3_CLOCK_MULTIPLIER		0x0d
> +#define UAC3_SAMPLE_RATE_CONVERTER	0x0e
> +#define UAC3_CONNECTORS			0x0f
> +#define UAC3_POWER_DOMAIN		0x10
> +
> +/* A.22 AUDIO CLASS-SPECIFIC REQUEST CODES */
> +#define UAC3_CS_REQ_CUR				0x01
> +#define UAC3_CS_REQ_RANGE			0x02
> +#define UAC3_CS_REQ_MEM				0x03

these first 3 are already in UAC2

> +#define UAC3_CS_REQ_INTEN			0x04
> +#define UAC3_CS_REQ_STRING			0x05
> +#define UAC3_CS_REQ_HIGH_CAPABILITY_DESCRIPTOR	0x06

the last 3 are really UAC3-specific

> +
> +/* A.23.1 AUDIOCONTROL INTERFACE CONTROL SELECTORS */
> +#define UAC3_AC_CONTROL_UNDEFINED		0x00
> +#define UAC3_AC_ACTIVE_INTERFACE_CONTROL	0x01
> +#define UAC3_AC_POWER_DOMAIN_CONTROL		0x02
> +
> +#endif /* __LINUX_USB_AUDIO_V3_H */
> diff --git a/include/uapi/linux/usb/audio.h b/include/uapi/linux/usb/audio.h
> index a4680a5..66ec2ae 100644
> --- a/include/uapi/linux/usb/audio.h
> +++ b/include/uapi/linux/usb/audio.h
> @@ -26,6 +26,7 @@
>   /* bInterfaceProtocol values to denote the version of the standard used */
>   #define UAC_VERSION_1			0x00
>   #define UAC_VERSION_2			0x20
> +#define UAC_VERSION_3			0x30
>   
>   /* A.2 Audio Interface Subclass Codes */
>   #define USB_SUBCLASS_AUDIOCONTROL	0x01
> diff --git a/sound/usb/card.c b/sound/usb/card.c
> index 3dc36d9..df3f0ab 100644
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
> @@ -7,6 +7,7 @@
>    *	    Alan Cox (alan@lxorguk.ukuu.org.uk)
>    *	    Thomas Sailer (sailer@ife.ee.ethz.ch)
>    *
> + *   Audio Class 3.0 support by Ruslan Bilovol <ruslan.bilovol@gmail.com>

does this mean new copyright?

>    *
>    *   This program is free software; you can redistribute it and/or modify
>    *   it under the terms of the GNU General Public License as published by
> @@ -44,6 +45,7 @@
>   #include <linux/mutex.h>
>   #include <linux/usb/audio.h>
>   #include <linux/usb/audio-v2.h>
> +#include <linux/usb/audio-v3.h>
>   #include <linux/module.h>
>   
>   #include <sound/control.h>
> @@ -261,7 +263,8 @@ static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif)
>   		break;
>   	}
>   
> -	case UAC_VERSION_2: {
> +	case UAC_VERSION_2:
> +	case UAC_VERSION_3: {
>   		struct usb_interface_assoc_descriptor *assoc =
>   			usb_ifnum_to_if(dev, ctrlif)->intf_assoc;
>   
> @@ -281,7 +284,7 @@ static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif)
>   		}
>   
>   		if (!assoc) {
> -			dev_err(&dev->dev, "Audio class v2 interfaces need an interface association\n");
> +			dev_err(&dev->dev, "Audio class v2/v3 interfaces need an interface association\n");

yes, but they can have more than one, so how do you handle the selection 
(see 3.3)?

>   			return -EINVAL;
>   		}
>   
> diff --git a/sound/usb/card.h b/sound/usb/card.h
> index 111b0f0..5e2934f 100644
> --- a/sound/usb/card.h
> +++ b/sound/usb/card.h
> @@ -21,7 +21,7 @@ struct audioformat {
>   	unsigned char endpoint;		/* endpoint */
>   	unsigned char ep_attr;		/* endpoint attributes */
>   	unsigned char datainterval;	/* log_2 of data packet interval */
> -	unsigned char protocol;		/* UAC_VERSION_1/2 */
> +	unsigned char protocol;		/* UAC_VERSION_1/2/3 */
>   	unsigned int maxpacksize;	/* max. packet size */
>   	unsigned int rates;		/* rate bitmasks */
>   	unsigned int rate_min, rate_max;	/* min/max rates */
> diff --git a/sound/usb/clock.c b/sound/usb/clock.c
> index 26dd5f2..04361d7 100644
> --- a/sound/usb/clock.c
> +++ b/sound/usb/clock.c
> @@ -23,6 +23,7 @@
>   #include <linux/usb.h>
>   #include <linux/usb/audio.h>
>   #include <linux/usb/audio-v2.h>
> +#include <linux/usb/audio-v3.h>
>   
>   #include <sound/core.h>
>   #include <sound/info.h>
> @@ -50,6 +51,22 @@
>   	return NULL;
>   }
>   
> +static struct uac3_clock_source_descriptor *
> +	snd_usb_find_clock_source_v3(struct usb_host_interface *ctrl_iface,
> +				  int clock_id)
> +{
> +	struct uac3_clock_source_descriptor *cs = NULL;
> +
> +	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
> +					     ctrl_iface->extralen,
> +					     cs, UAC3_CLOCK_SOURCE))) {
> +		if (cs->bClockID == clock_id)
> +			return cs;
> +	}
> +
> +	return NULL;
> +}
> +
>   static struct uac_clock_selector_descriptor *
>   	snd_usb_find_clock_selector(struct usb_host_interface *ctrl_iface,
>   				    int clock_id)
> @@ -66,6 +83,22 @@
>   	return NULL;
>   }
>   
> +static struct uac3_clock_selector_descriptor *
> +	snd_usb_find_clock_selector_v3(struct usb_host_interface *ctrl_iface,
> +				    int clock_id)
> +{
> +	struct uac3_clock_selector_descriptor *cs = NULL;
> +
> +	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
> +					     ctrl_iface->extralen,
> +					     cs, UAC3_CLOCK_SELECTOR))) {
> +		if (cs->bClockID == clock_id)
> +			return cs;
> +	}
> +
> +	return NULL;
> +}
> +
>   static struct uac_clock_multiplier_descriptor *
>   	snd_usb_find_clock_multiplier(struct usb_host_interface *ctrl_iface,
>   				      int clock_id)
> @@ -82,6 +115,22 @@
>   	return NULL;
>   }
>   
> +static struct uac3_clock_multiplier_descriptor *
> +	snd_usb_find_clock_multiplier_v3(struct usb_host_interface *ctrl_iface,
> +				      int clock_id)
> +{
> +	struct uac3_clock_multiplier_descriptor *cs = NULL;
> +
> +	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
> +					     ctrl_iface->extralen,
> +					     cs, UAC3_CLOCK_MULTIPLIER))) {
> +		if (cs->bClockID == clock_id)
> +			return cs;
> +	}
> +
> +	return NULL;
> +}
> +
>   static int uac_clock_selector_get_val(struct snd_usb_audio *chip, int selector_id)
>   {
>   	unsigned char buf;
> @@ -135,19 +184,33 @@ static int uac_clock_selector_set_val(struct snd_usb_audio *chip, int selector_i
>   	return ret;
>   }
>   
> -static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, int source_id)
> +static bool uac_clock_source_is_valid(struct snd_usb_audio *chip,
> +				      int protocol,
> +				      int source_id)
>   {
>   	int err;
>   	unsigned char data;
>   	struct usb_device *dev = chip->dev;
> -	struct uac_clock_source_descriptor *cs_desc =
> -		snd_usb_find_clock_source(chip->ctrl_intf, source_id);
> -
> -	if (!cs_desc)
> -		return 0;
> +	u32 bmControls;
> +
> +	if (protocol == UAC_VERSION_3) {
> +		struct uac3_clock_source_descriptor *cs_desc =
> +			snd_usb_find_clock_source_v3(chip->ctrl_intf, source_id);
> +
> +		if (!cs_desc)
> +			return 0;
> +		bmControls = le32_to_cpu(cs_desc->bmControls);
> +	} else { /* UAC_VERSION_1/2 */

no, clock sources are only in UAC2.

> +		struct uac_clock_source_descriptor *cs_desc =
> +			snd_usb_find_clock_source(chip->ctrl_intf, source_id);
> +
> +		if (!cs_desc)
> +			return 0;
> +		bmControls = cs_desc->bmControls;
> +	}
>   
>   	/* If a clock source can't tell us whether it's valid, we assume it is */
> -	if (!uac2_control_is_readable(cs_desc->bmControls,
> +	if (!uac_v2v3_control_is_readable(bmControls,
>   				      UAC2_CS_CONTROL_CLOCK_VALID - 1))
>   		return 1;
>   
> @@ -167,9 +230,8 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, int source_id)
>   	return !!data;
>   }
>   
> -static int __uac_clock_find_source(struct snd_usb_audio *chip,
> -				   int entity_id, unsigned long *visited,
> -				   bool validate)
> +static int __uac_clock_find_source(struct snd_usb_audio *chip, int entity_id,
> +				   unsigned long *visited, bool validate)
>   {
>   	struct uac_clock_source_descriptor *source;
>   	struct uac_clock_selector_descriptor *selector;
> @@ -188,7 +250,8 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip,
>   	source = snd_usb_find_clock_source(chip->ctrl_intf, entity_id);
>   	if (source) {
>   		entity_id = source->bClockID;
> -		if (validate && !uac_clock_source_is_valid(chip, entity_id)) {
> +		if (validate && !uac_clock_source_is_valid(chip, UAC_VERSION_2,
> +								entity_id)) {
>   			usb_audio_err(chip,
>   				"clock source %d is not valid, cannot use\n",
>   				entity_id);
> @@ -257,6 +320,97 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip,
>   	return -EINVAL;
>   }
>   
> +static int __uac3_clock_find_source(struct snd_usb_audio *chip, int entity_id,
> +				   unsigned long *visited, bool validate)
> +{
> +	struct uac3_clock_source_descriptor *source;
> +	struct uac3_clock_selector_descriptor *selector;
> +	struct uac3_clock_multiplier_descriptor *multiplier;
> +
> +	entity_id &= 0xff;
> +
> +	if (test_and_set_bit(entity_id, visited)) {
> +		usb_audio_warn(chip,
> +			 "%s(): recursive clock topology detected, id %d.\n",
> +			 __func__, entity_id);
> +		return -EINVAL;
> +	}
> +
> +	/* first, see if the ID we're looking for is a clock source already */
> +	source = snd_usb_find_clock_source_v3(chip->ctrl_intf, entity_id);
> +	if (source) {
> +		entity_id = source->bClockID;
> +		if (validate && !uac_clock_source_is_valid(chip, UAC_VERSION_3,
> +								entity_id)) {
> +			usb_audio_err(chip,
> +				"clock source %d is not valid, cannot use\n",
> +				entity_id);
> +			return -ENXIO;
> +		}
> +		return entity_id;
> +	}
> +
> +	selector = snd_usb_find_clock_selector_v3(chip->ctrl_intf, entity_id);
> +	if (selector) {
> +		int ret, i, cur;
> +
> +		/* the entity ID we are looking for is a selector.
> +		 * find out what it currently selects */
> +		ret = uac_clock_selector_get_val(chip, selector->bClockID);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* Selector values are one-based */
> +
> +		if (ret > selector->bNrInPins || ret < 1) {
> +			usb_audio_err(chip,
> +				"%s(): selector reported illegal value, id %d, ret %d\n",
> +				__func__, selector->bClockID, ret);
> +
> +			return -EINVAL;
> +		}
> +
> +		cur = ret;
> +		ret = __uac3_clock_find_source(chip, selector->baCSourceID[ret - 1],
> +					       visited, validate);
> +		if (!validate || ret > 0 || !chip->autoclock)
> +			return ret;
> +
> +		/* The current clock source is invalid, try others. */
> +		for (i = 1; i <= selector->bNrInPins; i++) {
> +			int err;
> +
> +			if (i == cur)
> +				continue;
> +
> +			ret = __uac3_clock_find_source(chip, selector->baCSourceID[i - 1],
> +				visited, true);
> +			if (ret < 0)
> +				continue;
> +
> +			err = uac_clock_selector_set_val(chip, entity_id, i);
> +			if (err < 0)
> +				continue;
> +
> +			usb_audio_info(chip,
> +				 "found and selected valid clock source %d\n",
> +				 ret);
> +			return ret;
> +		}
> +
> +		return -ENXIO;
> +	}
> +
> +	/* FIXME: multipliers only act as pass-thru element for now */
> +	multiplier = snd_usb_find_clock_multiplier_v3(chip->ctrl_intf,
> +						      entity_id);
> +	if (multiplier)
> +		return __uac3_clock_find_source(chip, multiplier->bCSourceID,
> +						visited, validate);
> +
> +	return -EINVAL;

the difference between clock structures is not that big really between 
UAC2 and UAC3 (wider bmcontrols, change to reference terminal and 
wClockSourceStr_), could this code be refactored to handle both UAC2 and 
UAC3?


> +}
> +
>   /*
>    * For all kinds of sample rate settings and other device queries,
>    * the clock source (end-leaf) must be used. However, clock selectors,
> @@ -268,12 +422,22 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip,
>    *
>    * Returns the clock source UnitID (>=0) on success, or an error.
>    */
> -int snd_usb_clock_find_source(struct snd_usb_audio *chip, int entity_id,
> -			      bool validate)
> +int snd_usb_clock_find_source(struct snd_usb_audio *chip, int protocol,
> +			      int entity_id, bool validate)
>   {
>   	DECLARE_BITMAP(visited, 256);
>   	memset(visited, 0, sizeof(visited));
> -	return __uac_clock_find_source(chip, entity_id, visited, validate);
> +
> +	switch (protocol) {
> +	case UAC_VERSION_2:
> +		return __uac_clock_find_source(chip, entity_id, visited,
> +					       validate);
> +	case UAC_VERSION_3:
> +		return __uac3_clock_find_source(chip, entity_id, visited,
> +					       validate);

this looks again very similar, could this be optimized?


>   #include <sound/core.h>
>   #include <sound/pcm.h>
> @@ -39,11 +40,11 @@
>    * @dev: usb device
>    * @fp: audioformat record
>    * @format: the format tag (wFormatTag)
> - * @fmt: the format type descriptor
> + * @fmt: the format type descriptor (v1/v2) or AudioStreaming descriptor (v3)
>    */
>   static u64 parse_audio_format_i_type(struct snd_usb_audio *chip,
>   				     struct audioformat *fp,
> -				     unsigned int format, void *_fmt)
> +				     u64 format, void *_fmt)
>   {
>   	int sample_width, sample_bytes;
>   	u64 pcm_formats = 0;
> @@ -69,6 +70,17 @@ static u64 parse_audio_format_i_type(struct snd_usb_audio *chip,
>   		format <<= 1;
>   		break;
>   	}
> +	case UAC_VERSION_3: {
> +		struct uac3_as_header_descriptor *as = _fmt;
> +		sample_width = as->bBitResolution;
> +		sample_bytes = as->bSubslotSize;
> +
> +		if (format & UAC3_FORMAT_TYPE_I_RAW_DATA)
> +			pcm_formats |= SNDRV_PCM_FMTBIT_SPECIAL;

could this be expanded to add at least basic PCM (D0 set).

> +int snd_usb_parse_audio_format_v3(struct snd_usb_audio *chip,
> +			       struct audioformat *fp,
> +			       struct uac3_as_header_descriptor *as,
> +			       int stream)
> +{
> +	u64 format = le64_to_cpu(as->bmFormats);
> +	int err;
> +
> +	/* Type I format bits are D0..D6 */
> +	if (format & 0x7f)
> +		fp->fmt_type = UAC_FORMAT_TYPE_I;
> +	else
> +		fp->fmt_type = UAC_FORMAT_TYPE_III;
maybe mention that this test only work because type IV is not supported, 
otherwise this wouldn't quite right.

> +int snd_usb_parse_audio_format_v3(struct snd_usb_audio *chip,
> +			       struct audioformat *fp,
> +			       struct uac3_as_header_descriptor *as,
> +			       int stream);
>   #endif /*  __USBAUDIO_FORMAT_H */
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index 9732edf..5b5dfe4 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -51,6 +51,7 @@
>   #include <linux/usb.h>
>   #include <linux/usb/audio.h>
>   #include <linux/usb/audio-v2.h>
> +#include <linux/usb/audio-v3.h>
>   
>   #include <sound/core.h>
>   #include <sound/control.h>
> @@ -189,7 +190,7 @@ static void *find_audio_control_unit(struct mixer_build *state,
>   					USB_DT_CS_INTERFACE)) != NULL) {
>   		if (hdr->bLength >= 4 &&
>   		    hdr->bDescriptorSubtype >= UAC_INPUT_TERMINAL &&
> -		    hdr->bDescriptorSubtype <= UAC2_SAMPLE_RATE_CONVERTER &&
> +		    hdr->bDescriptorSubtype <= UAC3_SAMPLE_RATE_CONVERTER &&
>   		    hdr->bUnitID == unit)

this looks like a mistake, the definitions are

#define UAC3_SAMPLE_RATE_CONVERTER	0x0e
#define UAC3_CONNECTORS			0x0f
#define UAC3_POWER_DOMAIN		0x10

You will miss the last two with this test.


> -		case UAC1_PROCESSING_UNIT:
> -		case UAC1_EXTENSION_UNIT:
> -		/* UAC2_PROCESSING_UNIT_V2 */
> -		/* UAC2_EFFECT_UNIT */
> -		case UAC2_EXTENSION_UNIT_V2: {
> -			struct uac_processing_unit_descriptor *d = p1;
> -
> -			if (state->mixer->protocol == UAC_VERSION_2 &&
> -				hdr[2] == UAC2_EFFECT_UNIT) {
> -				/* UAC2/UAC1 unit IDs overlap here in an
> -				 * uncompatible way. Ignore this unit for now.
> -				 */
> +
> +				/* REVISIT: UAC3 IT doesn't have channels/cfg */
> +				term->channels = 0;
> +				term->chconfig = 0;

It does, but you need to get the information from the wClusterDescrID

[snip]

> +/* UAC3 device stores channels information in Cluster Descriptors */
> +static struct
> +snd_pcm_chmap_elem *convert_chmap_v3(struct uac3_cluster_header_descriptor
> +								*cluster)
> +{
> +	unsigned int channels = cluster->bNrChannels;
> +	struct snd_pcm_chmap_elem *chmap;
> +	void *p = cluster;
> +	int len, c;
> +
> +	if (channels > ARRAY_SIZE(chmap->map))
> +		return NULL;
> +
> +	chmap = kzalloc(sizeof(*chmap), GFP_KERNEL);
> +	if (!chmap)
> +		return NULL;
> +
> +	len = le16_to_cpu(cluster->wLength);
> +	c = 0;
> +	p += sizeof(struct uac3_cluster_header_descriptor);
> +
> +	while (((p - (void *)cluster) < len) && (c < channels)) {
> +		struct uac3_cluster_segment_descriptor *cs_desc = p;
> +		u16 cs_len;
> +		u8 cs_type;
> +
> +		cs_len = le16_to_cpu(cs_desc->wLength);
> +		cs_type = cs_desc->bSegmentType;
> +
> +		if (cs_type == UAC3_CHANNEL_INFORMATION) {
> +			struct uac3_cluster_information_segment_descriptor *is = p;
> +			unsigned char map;
> +
> +			/*
> +			 * FIXME: this conversion is simplified, because
> +			 * asound.h doesn't have UAC3 values yet


yes, we need to have a mapping between USB and CEA definitions (which 
are aligned to some extent btw)

> +			 */
> +			switch (is->bChPurpose) {
> +			case UAC3_CH_MONO:
> +				map = SNDRV_CHMAP_MONO;
> +				break;
> +			case UAC3_CH_LEFT:
> +			case UAC3_CH_FRONT_LEFT:
> +			case UAC3_CH_SURROUND_ARRAY_LEFT:
> +				map = SNDRV_CHMAP_FL;
> +				break;
> +			case UAC3_CH_RIGHT:
> +			case UAC3_CH_FRONT_RIGHT:
> +			case UAC3_CH_SURROUND_ARRAY_RIGHT:
> +				map = SNDRV_CHMAP_FR;
> +				break;
> +			case UAC3_CH_FRONT_CENTER:
> +				map = SNDRV_CHMAP_FC;
> +				break;
> +			case UAC3_CH_LOW_FREQUENCY_EFFECTS:
> +				map = SNDRV_CHMAP_LFE;
> +				break;
> +			case UAC3_CH_RELATIONSHIP_UNDEFINED:
> +			default:
> +				map = SNDRV_CHMAP_UNKNOWN;
> +				break;
Greg KH Nov. 9, 2017, 8:04 a.m. UTC | #4
On Wed, Nov 08, 2017 at 03:38:35PM +0100, Takashi Iwai wrote:
> On Tue, 07 Nov 2017 03:01:20 +0100,
> Ruslan Bilovol wrote:
> > 
> > Recently released USB Audio Class 3.0 specification
> > introduces many significant changes comparing to
> > previous versions, like
> >  - new Power Domains, support for LPM/L1
> >  - new Cluster descriptor
> >  - changed layout of all class-specific descriptors
> >  - new High Capability descriptors
> >  - New class-specific String descriptors
> >  - new and removed units
> >  - additional sources for interrupts
> >  - removed Type II Audio Data Formats
> >  - ... and many other things (check spec)
> > 
> > It also provides backward compatibility through
> > multiple configurations, as well as requires
> > mandatory support for BADD (Basic Audio Device
> > Definition) on each ADC3.0 compliant device
> > 
> > This patch adds initial support of UAC3 specification
> > that is enough for Generic I/O Profile (BAOF, BAIF)
> > device support from BADD document.
> > 
> > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> 
> The patch looks good, but the timing is fairly late for merging to
> 4.15.

Isn't kbuild barfing all over these?  Is that because of the cross-tree
changes needed?

> So from my side, the primary question is whether the changes in USB
> (audio) header files are OK for USB guys.
> 
> Greg, could you check these changes and give an ack if it's OK to
> merge?  Or if you prefer postpone, just let me know.

I don't object to the USB header changes, as long as the fixes you point
out are made :)

thanks,

greg k-h
Takashi Iwai Nov. 9, 2017, 8:16 a.m. UTC | #5
On Thu, 09 Nov 2017 09:04:58 +0100,
Greg Kroah-Hartman wrote:
> 
> On Wed, Nov 08, 2017 at 03:38:35PM +0100, Takashi Iwai wrote:
> > On Tue, 07 Nov 2017 03:01:20 +0100,
> > Ruslan Bilovol wrote:
> > > 
> > > Recently released USB Audio Class 3.0 specification
> > > introduces many significant changes comparing to
> > > previous versions, like
> > >  - new Power Domains, support for LPM/L1
> > >  - new Cluster descriptor
> > >  - changed layout of all class-specific descriptors
> > >  - new High Capability descriptors
> > >  - New class-specific String descriptors
> > >  - new and removed units
> > >  - additional sources for interrupts
> > >  - removed Type II Audio Data Formats
> > >  - ... and many other things (check spec)
> > > 
> > > It also provides backward compatibility through
> > > multiple configurations, as well as requires
> > > mandatory support for BADD (Basic Audio Device
> > > Definition) on each ADC3.0 compliant device
> > > 
> > > This patch adds initial support of UAC3 specification
> > > that is enough for Generic I/O Profile (BAOF, BAIF)
> > > device support from BADD document.
> > > 
> > > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > 
> > The patch looks good, but the timing is fairly late for merging to
> > 4.15.
> 
> Isn't kbuild barfing all over these?  Is that because of the cross-tree
> changes needed?

No, it's just local to the audio driver, plus a few
include/linux/usb/*.h modification / addition.
So there shouldn't be a big breakage in that regard.


> > So from my side, the primary question is whether the changes in USB
> > (audio) header files are OK for USB guys.
> > 
> > Greg, could you check these changes and give an ack if it's OK to
> > merge?  Or if you prefer postpone, just let me know.
> 
> I don't object to the USB header changes, as long as the fixes you point
> out are made :)

Good to hear.
Could you give your acked-by once after reviewing the patch?
I think this should go through sound tree.

Thanks!


Takashi
Greg KH Nov. 9, 2017, 8:33 a.m. UTC | #6
On Thu, Nov 09, 2017 at 09:16:52AM +0100, Takashi Iwai wrote:
> On Thu, 09 Nov 2017 09:04:58 +0100,
> Greg Kroah-Hartman wrote:
> > 
> > On Wed, Nov 08, 2017 at 03:38:35PM +0100, Takashi Iwai wrote:
> > > On Tue, 07 Nov 2017 03:01:20 +0100,
> > > Ruslan Bilovol wrote:
> > > > 
> > > > Recently released USB Audio Class 3.0 specification
> > > > introduces many significant changes comparing to
> > > > previous versions, like
> > > >  - new Power Domains, support for LPM/L1
> > > >  - new Cluster descriptor
> > > >  - changed layout of all class-specific descriptors
> > > >  - new High Capability descriptors
> > > >  - New class-specific String descriptors
> > > >  - new and removed units
> > > >  - additional sources for interrupts
> > > >  - removed Type II Audio Data Formats
> > > >  - ... and many other things (check spec)
> > > > 
> > > > It also provides backward compatibility through
> > > > multiple configurations, as well as requires
> > > > mandatory support for BADD (Basic Audio Device
> > > > Definition) on each ADC3.0 compliant device
> > > > 
> > > > This patch adds initial support of UAC3 specification
> > > > that is enough for Generic I/O Profile (BAOF, BAIF)
> > > > device support from BADD document.
> > > > 
> > > > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > > 
> > > The patch looks good, but the timing is fairly late for merging to
> > > 4.15.
> > 
> > Isn't kbuild barfing all over these?  Is that because of the cross-tree
> > changes needed?
> 
> No, it's just local to the audio driver, plus a few
> include/linux/usb/*.h modification / addition.
> So there shouldn't be a big breakage in that regard.
> 
> 
> > > So from my side, the primary question is whether the changes in USB
> > > (audio) header files are OK for USB guys.
> > > 
> > > Greg, could you check these changes and give an ack if it's OK to
> > > merge?  Or if you prefer postpone, just let me know.
> > 
> > I don't object to the USB header changes, as long as the fixes you point
> > out are made :)
> 
> Good to hear.
> Could you give your acked-by once after reviewing the patch?

I'm guessing there will be a new patch?  I'll be glad to review that
one.

thanks,

greg k-h
Takashi Iwai Nov. 9, 2017, 8:58 a.m. UTC | #7
On Thu, 09 Nov 2017 09:33:44 +0100,
Greg Kroah-Hartman wrote:
> 
> On Thu, Nov 09, 2017 at 09:16:52AM +0100, Takashi Iwai wrote:
> > On Thu, 09 Nov 2017 09:04:58 +0100,
> > Greg Kroah-Hartman wrote:
> > > 
> > > On Wed, Nov 08, 2017 at 03:38:35PM +0100, Takashi Iwai wrote:
> > > > On Tue, 07 Nov 2017 03:01:20 +0100,
> > > > Ruslan Bilovol wrote:
> > > > > 
> > > > > Recently released USB Audio Class 3.0 specification
> > > > > introduces many significant changes comparing to
> > > > > previous versions, like
> > > > >  - new Power Domains, support for LPM/L1
> > > > >  - new Cluster descriptor
> > > > >  - changed layout of all class-specific descriptors
> > > > >  - new High Capability descriptors
> > > > >  - New class-specific String descriptors
> > > > >  - new and removed units
> > > > >  - additional sources for interrupts
> > > > >  - removed Type II Audio Data Formats
> > > > >  - ... and many other things (check spec)
> > > > > 
> > > > > It also provides backward compatibility through
> > > > > multiple configurations, as well as requires
> > > > > mandatory support for BADD (Basic Audio Device
> > > > > Definition) on each ADC3.0 compliant device
> > > > > 
> > > > > This patch adds initial support of UAC3 specification
> > > > > that is enough for Generic I/O Profile (BAOF, BAIF)
> > > > > device support from BADD document.
> > > > > 
> > > > > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > > > 
> > > > The patch looks good, but the timing is fairly late for merging to
> > > > 4.15.
> > > 
> > > Isn't kbuild barfing all over these?  Is that because of the cross-tree
> > > changes needed?
> > 
> > No, it's just local to the audio driver, plus a few
> > include/linux/usb/*.h modification / addition.
> > So there shouldn't be a big breakage in that regard.
> > 
> > 
> > > > So from my side, the primary question is whether the changes in USB
> > > > (audio) header files are OK for USB guys.
> > > > 
> > > > Greg, could you check these changes and give an ack if it's OK to
> > > > merge?  Or if you prefer postpone, just let me know.
> > > 
> > > I don't object to the USB header changes, as long as the fixes you point
> > > out are made :)
> > 
> > Good to hear.
> > Could you give your acked-by once after reviewing the patch?
> 
> I'm guessing there will be a new patch?  I'll be glad to review that
> one.

Sure, no problem.


thanks,

Takashi
Ruslan Bilovol Nov. 10, 2017, 11:12 a.m. UTC | #8
On Wed, Nov 8, 2017 at 4:38 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Tue, 07 Nov 2017 03:01:20 +0100,
> Ruslan Bilovol wrote:
>>
>> Recently released USB Audio Class 3.0 specification
>> introduces many significant changes comparing to
>> previous versions, like
>>  - new Power Domains, support for LPM/L1
>>  - new Cluster descriptor
>>  - changed layout of all class-specific descriptors
>>  - new High Capability descriptors
>>  - New class-specific String descriptors
>>  - new and removed units
>>  - additional sources for interrupts
>>  - removed Type II Audio Data Formats
>>  - ... and many other things (check spec)
>>
>> It also provides backward compatibility through
>> multiple configurations, as well as requires
>> mandatory support for BADD (Basic Audio Device
>> Definition) on each ADC3.0 compliant device
>>
>> This patch adds initial support of UAC3 specification
>> that is enough for Generic I/O Profile (BAOF, BAIF)
>> device support from BADD document.
>>
>> Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
>
> The patch looks good, but the timing is fairly late for merging to
> 4.15.

That's OK

>
> So from my side, the primary question is whether the changes in USB
> (audio) header files are OK for USB guys.
>
> Greg, could you check these changes and give an ack if it's OK to
> merge?  Or if you prefer postpone, just let me know.
>
>
> Below are some nitpicking:
>
>> --- a/include/linux/usb/audio-v2.h
>> +++ b/include/linux/usb/audio-v2.h
>> @@ -33,12 +33,12 @@
>>   *
>>   */
>>
>> -static inline bool uac2_control_is_readable(u32 bmControls, u8 control)
>> +static inline bool uac_v2v3_control_is_readable(u32 bmControls, u8 control)
>>  {
>>       return (bmControls >> (control * 2)) & 0x1;
>>  }
>>
>> -static inline bool uac2_control_is_writeable(u32 bmControls, u8 control)
>> +static inline bool uac_v2v3_control_is_writeable(u32 bmControls, u8 control)
>>  {
>>       return (bmControls >> (control * 2)) & 0x2;
>>  }
>
> This change looks a bit strange, but later one I noticed the reason
> you didn't copy these functions into usb/audio-v3.h.  Though, I guess
> a compiler can optimize out even if you call the same inline code
> twice for v2 and v3...

The main idea of this rename was to make code more clear.
Since with UAC3 clock handling (and many other things) still is the
same as in UAC2 but discriptor layouts and some constatns numbers
changed. So it will possible to not introduce extra comparisons
to run the same function.
On the other hand we still can use uac2 named function, but
programming becomes pure hell in this case. This is because
some part of constatns are common for UAC1 and UAC2, some
are common UAC2 and UAC3, and some are common for
all three of them. To make the code more understandable,
I tried to change common function names to see in the code
what is common.

By the way, I'll try to do it another way in this particular case

>
> In anyway, if we do rename, let's mention about it in the comment.
>
>
>> diff --git a/include/linux/usb/audio-v3.h b/include/linux/usb/audio-v3.h
>> new file mode 100644
>> index 0000000..cbbe51e
>> --- /dev/null
>> +++ b/include/linux/usb/audio-v3.h
>> @@ -0,0 +1,343 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>
> You can use the C-style comment for SPDX line,
>   /* SPDX-License-Identifier: GPL-2.0+ */
>
>> +static struct uac3_clock_source_descriptor *
>> +     snd_usb_find_clock_source_v3(struct usb_host_interface *ctrl_iface,
>> +                               int clock_id)
>> +{
>> +     struct uac3_clock_source_descriptor *cs = NULL;
>> +
>> +     while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
>> +                                          ctrl_iface->extralen,
>> +                                          cs, UAC3_CLOCK_SOURCE))) {
>> +             if (cs->bClockID == clock_id)
>> +                     return cs;
>> +     }
>> +
>> +     return NULL;
>> +}
>
> We may clean up the whole find_clock_xxx stuff later, but let's keep
> it dumb and straight for now...
>
>
>> +static int __uac3_clock_find_source(struct snd_usb_audio *chip, int entity_id,
>> +                                unsigned long *visited, bool validate)
>> +{
> (snip)
>> +             /* the entity ID we are looking for is a selector.
>> +              * find out what it currently selects */
>
> The multi-line comment should be properly formatted like
>   /*
>    * XXX
>    */

Correct. I used it only because other places in clock.c has
same coding style. Will fix it.

>
>> +             /* The current clock source is invalid, try others. */
>> +             for (i = 1; i <= selector->bNrInPins; i++) {
>> +                     int err;
>> +
>> +                     if (i == cur)
>> +                             continue;
>> +
>> +                     ret = __uac3_clock_find_source(chip, selector->baCSourceID[i - 1],
>> +                             visited, true);
>> +                     if (ret < 0)
>> +                             continue;
>> +
>> +                     err = uac_clock_selector_set_val(chip, entity_id, i);
>> +                     if (err < 0)
>> +                             continue;
>> +
>> +                     usb_audio_info(chip,
>> +                              "found and selected valid clock source %d\n",
>> +                              ret);
>
> Do we need to print this at each time?

We have the same info print in UAC2 version of function, let's keep
it consistent (or change in both places)

>
>
>>  static int parse_audio_format_i(struct snd_usb_audio *chip,
>> -                             struct audioformat *fp, unsigned int format,
>> -                             struct uac_format_type_i_continuous_descriptor *fmt)
>> +                             struct audioformat *fp, u64 format,
>> +                             void *_fmt)
>>  {
>>       snd_pcm_format_t pcm_format;
>> +     unsigned int fmt_type;
>>       int ret;
>>
>> -     if (fmt->bFormatType == UAC_FORMAT_TYPE_III) {
>> +     switch (fp->protocol) {
>> +     default:
>> +     case UAC_VERSION_1:
>> +     case UAC_VERSION_2: {
>> +             struct uac_format_type_i_continuous_descriptor *fmt = _fmt;
>> +
>> +             fmt_type = fmt->bFormatType;
>> +             break;
>> +     }
>> +     case UAC_VERSION_3: {
>> +             /* fp->fmt_type is already set in this case */
>> +             fmt_type = fp->fmt_type;
>> +             break;
>> +     }
>> +     }
>
> The double-braces don't look beautiful.  And in this case it's just
> for the temporary fmt variable, so it can be moved to the top level
> and drop the internal whole braces.

Agree.

>
>
>> @@ -405,15 +435,20 @@ static int parse_audio_format_i(struct snd_usb_audio *chip,
>>        */
>>       switch (fp->protocol) {
>>       default:
>> -     case UAC_VERSION_1:
>> +     case UAC_VERSION_1: {
>> +             struct uac_format_type_i_continuous_descriptor *fmt = _fmt;
>> +
>>               fp->channels = fmt->bNrChannels;
>>               ret = parse_audio_format_rates_v1(chip, fp, (unsigned char *) fmt, 7);
>>               break;
>> +     }
>>       case UAC_VERSION_2:
>> +     case UAC_VERSION_3: {
>>               /* fp->channels is already set in this case */
>> -             ret = parse_audio_format_rates_v2(chip, fp);
>> +             ret = parse_audio_format_rates_v2v3(chip, fp);
>>               break;
>>       }
>> +     }
>
> DItto.
>
>> @@ -520,3 +555,23 @@ int snd_usb_parse_audio_format(struct snd_usb_audio *chip,
>>       return 0;
>>  }
>>
>> +int snd_usb_parse_audio_format_v3(struct snd_usb_audio *chip,
>> +                            struct audioformat *fp,
>> +                            struct uac3_as_header_descriptor *as,
>> +                            int stream)
>> +{
>> +     u64 format = le64_to_cpu(as->bmFormats);
>> +     int err;
>> +
>> +     /* Type I format bits are D0..D6 */
>> +     if (format & 0x7f)
>> +             fp->fmt_type = UAC_FORMAT_TYPE_I;
>> +     else
>> +             fp->fmt_type = UAC_FORMAT_TYPE_III;
>> +
>> +     err = parse_audio_format_i(chip, fp, format, as);
>> +     if (err < 0)
>> +             return err;
>> +
>> +     return 0;
>
> You can simply return from parse_audio_format_i() without err check.

Agree, will fix it

>
>> @@ -460,9 +461,10 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
>>
>>       validx += cval->idx_off;
>>
>> +
>
> Superfluous blank line.

Yes, somehow I missed this before submitting the patch. Will fix it

>
>> @@ -718,16 +721,104 @@ static int check_input_term(struct mixer_build *state, int id,
>> +                             } else { /* UAC_VERSION_2 */
>> +                                     struct uac2_input_terminal_descriptor *d = p1;
>> +
>> +                                     /* call recursively to verify that the
>> +                                      * referenced clock entity is valid */
>
> Fix the comment style.
>
>> @@ -624,38 +708,158 @@ int snd_usb_parse_audio_interface(struct snd_usb_audio *chip, int iface_no)
>>                               iface_no, altno, as->bTerminalLink);
>>                       continue;
>>               }
>> -             }
>>
>> -             /* get format type */
>> -             fmt = snd_usb_find_csint_desc(alts->extra, alts->extralen, NULL, UAC_FORMAT_TYPE);
>> -             if (!fmt) {
>> +             case UAC_VERSION_3: {
>> +                     struct uac3_input_terminal_descriptor *input_term;
>> +                     struct uac3_output_terminal_descriptor *output_term;
>> +                     struct uac3_as_header_descriptor *as;
>> +                     struct uac3_cluster_header_descriptor *cluster;
>> +                     struct uac3_hc_descriptor_header hc_header;
>> +                     u16 cluster_id, wLength;
> (snip)
>
> Now snd_usb_parse_audio_interface() becomes too lengthy and hard to
> read through.  It'd be great if you can split / cleanup this in
> another patch.

Okay, I'll do that.

Thanks,
Ruslan Bilovol
Ruslan Bilovol Nov. 11, 2017, 2:48 a.m. UTC | #9
On Wed, Nov 8, 2017 at 9:38 PM, Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
> Nice work, thanks! I double-checked all the descriptors and values and
> didn't find anything problematic, the main comment I have is that the clock
> source/selection could probably be refactored since the differences are
> really minor with UAC2, and there is work to do to select the right Audio
> Interface Association.
>
> On 11/6/17 8:01 PM, Ruslan Bilovol wrote:
>>
>> Recently released USB Audio Class 3.0 specification
>> introduces many significant changes comparing to
>> previous versions, like
>>   - new Power Domains, support for LPM/L1
>>   - new Cluster descriptor
>>   - changed layout of all class-specific descriptors
>>   - new High Capability descriptors
>>   - New class-specific String descriptors
>>   - new and removed units
>>   - additional sources for interrupts
>>   - removed Type II Audio Data Formats
>>   - ... and many other things (check spec)
>>
>> It also provides backward compatibility through
>> multiple configurations, as well as requires
>> mandatory support for BADD (Basic Audio Device
>> Definition) on each ADC3.0 compliant device
>>
>> This patch adds initial support of UAC3 specification
>> that is enough for Generic I/O Profile (BAOF, BAIF)
>> device support from BADD document.
>
>
> Do you mean to say that the selection of the BADD profile or full-blown
> descriptor capabilities will be handled in a follow-up patch?

I have no follow-up patch for configuration switching.
I mean only that it is enough to support BAOF, BAIF profiles.
however, BAIOF profile won't work correctly, becaus it contains mixer unit
which is not supported yet in this patch.

This is because of way I use UAC3. As UAC3 device, I use an UAC3
gadget implementation which I sent before to linux-usb mailing list:
http://www.spinics.net/lists/linux-usb/msg162482.html

The UAC3 gadget implements BAOF+BAIF profile; but not BAIOF profile
which doesn't make sense in this case (there is no reason to mix Audio IN
with Audio Out)

Thus I can't implement and test UAC3 mixer handling in current patch,
so BAIOF profile isn't supported by it yet.

>
> It's my understanding from Section 3.3 that a UAC3 device is required to
> expose
> - a backwards-compatible configuration (UAC1 or UAC2),
> - a UAC3 BADD profile
> - one or more AIA compliant with UAC3 (which may have additional features
> not present in the baseline description).

My understanding of Section 3.3 is slightly different. An UAC3 device is
required to expose:
 - a backward-compatible first configuration (UAC1 or UAC2)
 - an UAC3 BADD profile on another configuration, with only one AIA inside
 - and _may_ have one or more configurations with UAC3 support that
   provide functionality beyond what is available in BADD

>
> And I am not sure how a driver would make the selection...

In order to test this patch, I created UAC2+UAC3 gadget and manually
switched to UAC3 configuration on Host by:
$ echo 2 > /sys/bus/usb/devices/1-1/bConfigurationValue

I don't think ALSA driver can make decision which configuration to select,
maybe some userspace tool can handle it, like usb_modeswitch does
for networking devices.

>
> In theory though, a UAC3 device should work out of the box even if the host
> only supports UAC1 or UAC2.

Yes, this is correct, I verified it as well.

>
>
>> +/*
>> + * v1.0, v2.0 and v3.0 of this standard have many things in common. For
>> the rest
>> + * of the definitions, please refer to audio.h and audio-v2.h
>> + */
>> +
>> +/* All High Capability descriptors have these 2 fields at the beginning
>> */
>> +struct uac3_hc_descriptor_header {
>> +       __le16 wLength;
>> +       __u8 bDescriptorType;
>> +       __u8 bDescriptorSubtype;
>> +       __le16 wDescriptorID;
>> +} __attribute__ ((packed));
>> +
>> +/* 4.3.1 CLUSTER DESCRIPTOR HEADER */
>> +struct uac3_cluster_header_descriptor {
>> +       __le16 wLength;
>> +       __u8 bDescriptorType;
>> +       __u8 bDescriptorSubtype;
>> +       __le16 wDescriptorID;
>> +       __u8 bNrChannels;
>> +} __attribute__ ((packed));
>> +
>> +/* 4.3.2.1 SEGMENTS */
>> +struct uac3_cluster_segment_descriptor {
>> +       __le16 wLength;
>> +       __u8 bSegmentType;
>> +       /* __u8[0]; segment-specific data */
>> +} __attribute__ ((packed));
>> +
>> +/* 4.3.2.1.1 END SEGMENT */
>> +struct uac3_cluster_end_segment_descriptor {
>> +       __le16 wLength;
>> +       __u8 bSegmentType;              /* Constant END_SEGMENT */
>> +} __attribute__ ((packed));
>> +
>
>
> you didn't include the definitions in 4.3.2.1.2
> Cluster Descriptor Segment
> Vendor Defined Segment

Correct, they are not used in BADD so I didn't care much, but I can
add it in next patchset

>
>> +/* 4.3.2.1.3.1 INFORMATION SEGMENT */
>> +struct uac3_cluster_information_segment_descriptor {
>> +       __le16 wLength;
>> +       __u8 bSegmentType;
>
>
> this field is a CHANNEL_INFORMATION constant.
>
>> +       __u8 bChPurpose;
>> +       __u8 bChRelationship;
>> +       __u8 bChGroupID;
>> +} __attribute__ ((packed));
>> +
>> +/* 4.5.2 CLASS-SPECIFIC AC INTERFACE DESCRIPTOR */
>> +struct uac3_ac_header_descriptor {
>> +       __u8 bLength;                   /* 10 */
>> +       __u8 bDescriptorType;           /* CS_INTERFACE descriptor type */
>> +       __u8 bDescriptorSubtype;        /* HEADER descriptor subtype */
>> +       __u8 bCategory;
>> +
>> +       /* includes Clock Source, Unit, Terminal, and Power Domain desc.
>> */
>> +       __le16 wTotalLength;
>> +
>> +       __le32 bmControls;
>> +} __attribute__ ((packed));
>> +
>
>
> Missing extended terminal descriptor header from 4.5.2.3.2?
>
> Missing multi-function processing unit descriptor header from 4.5.2.10.3 ?

Yes, I can add them as well.

>
>> +
>> +/* bmAttribute fields */
>> +#define UAC3_CLOCK_SOURCE_TYPE_EXT     0x0
>> +#define UAC3_CLOCK_SOURCE_TYPE_INT     0x1
>> +#define UAC3_CLOCK_SOURCE_ASYNC                (0 << 2)
>> +#define UAC3_CLOCK_SOURCE_SYNCED_TO_SOF        (1 << 1)
>> +
>> +/* 4.5.2.13 CLOCK SELECTOR DESCRIPTOR */
>> +struct uac3_clock_selector_descriptor {
>> +       __u8 bLength;
>> +       __u8 bDescriptorType;
>> +       __u8 bDescriptorSubtype;
>> +       __u8 bClockID;
>> +       __u8 bNrInPins;
>> +       __u8 baCSourceID[];
>> +       /* bmControls and wCSelectorDescrStr omitted */
>
>
> why is this omitted here and not below in the clock multiplier descriptor?

That's because in this descriptor ther is variable (unknown) field baCSourceID,
so we can't define anythyng else after it as per C standard.
In the clock multiplier descriptor below we don't have such issue.

>
>> +} __attribute__((packed) > +
>> +/* 4.5.2.14 CLOCK MULTIPLIER DESCRIPTOR */
>> +struct uac3_clock_multiplier_descriptor {
>> +       __u8 bLength;
>> +       __u8 bDescriptorType;
>> +       __u8 bDescriptorSubtype;
>> +       __u8 bClockID;
>> +       __u8 bCSourceID;
>> +       __le32 bmControls;
>> +       __le16 wCMultiplierDescrStr;
>> +} __attribute__((packed));
>
>
> [snip]
>
>> +
>> +/* 4.5.2.15 POWER DOMAIN DESCRIPTOR */
>> +struct uac3_power_domain_descriptor {
>> +       __u8 bLength;
>> +       __u8 bDescriptorType;
>> +       __u8 bDescriptorSubtype;
>> +       __u8 bPowerDomainID;
>> +       __le16 waRecoveryTime1;
>> +       __le16 waRecoveryTime2;
>> +       __u8 bNrEntities;
>> +       __u8 baEntityID[];
>> +       /* wPDomainDescrStr omitted */
>
> why?
>>
>> +} __attribute__((packed));
>> +
>> +/* As above, but more useful for defining your own descriptors */
>> +#define DECLARE_UAC3_POWER_DOMAIN_DESCRIPTOR(n)                        \
>> +struct uac3_power_domain_descriptor_##n {                      \
>> +       __u8 bLength;                                           \
>> +       __u8 bDescriptorType;                                   \
>> +       __u8 bDescriptorSubtype;                                \
>> +       __u8 bPowerDomainID;                                    \
>> +       __le16 waRecoveryTime1;                                 \
>> +       __le16 waRecoveryTime2;                                 \
>> +       __u8 bNrEntities;                                       \
>> +       __u8 baEntityID[n];                                     \
>> +       __le16 wPDomainDescrStr;                                        \
>> +} __attribute__ ((packed))
>> +
>
>
> any specific reason why the descriptors are not added in linear order,
> following the spec definition? all the descriptors below could be added
> earlier.

That's because of way I wrote this header. These descritors are added
in same order as in UAC2 header, then it was more easy to compare
both headers in order to understand if we can reuse anything from UAC2.

If course I should reorganize it, but forgot to do :D

>
>> +/* 4.5.2.1 INPUT TERMINAL DESCRIPTOR */
>> +struct uac3_input_terminal_descriptor {
>> +       __u8 bLength;
>> +       __u8 bDescriptorType;
>> +       __u8 bDescriptorSubtype;
>> +       __u8 bTerminalID;
>> +       __le16 wTerminalType;
>> +       __u8 bAssocTerminal;
>> +       __u8 bCSourceID;
>> +       __le32 bmControls;
>> +       __le16 wClusterDescrID;
>> +       __le16 wExTerminalDescrID;
>> +       __le16 wConnectorsDescrID;
>> +       __le16 wTerminalDescrStr;
>> +} __attribute__((packed));
>> +
>
>
>> +
>> +/* 4.7.2 CLASS-SPECIFIC AS INTERFACE DESCRIPTOR */
>> +struct uac3_as_header_descriptor {
>> +       __u8 bLength;
>> +       __u8 bDescriptorType;
>> +       __u8 bDescriptorSubtype;
>> +       __u8 bTerminalLink;
>> +       __le32 bmControls;
>> +       __le16 wClusterDescrID;
>> +       __le64 bmFormats;
>> +       __u8 bSubslotSize;
>> +       __u8 bBitResolution;
>> +       __le16 bmAuxProtocols;
>> +       __u8 bControlSize;
>> +} __attribute__((packed));
>> +
>> +#define UAC3_FORMAT_TYPE_I_RAW_DATA    (1 << 6)
>
> This seems to come from Table A1 in the formats document, is it inserted
> here because of the relationship with the bmFormats? If yes, we should
> probably add the full list from Table A1 (as done below for the other codes)

Yes, this comes from Frmts3.0. However, for bits 0..5 we reuse same
values from UAC1 spec, thay are same for UAC2 as well.
See UAC_FORMAT_TYPE_xxx

>
>> +
>> +/* 4.8.1.2 CLASS-SPECIFIC AS ISOCHRONOUS AUDIO DATA ENDPOINT DESCRIPTOR
>> */
>> +struct uac3_iso_endpoint_descriptor {
>> +       __u8 bLength;
>> +       __u8 bDescriptorType;
>> +       __u8 bDescriptorSubtype;
>> +       __le32 bmControls;
>> +       __u8 bLockDelayUnits;
>> +       __le16 wLockDelay;
>> +} __attribute__((packed));
>> +
>> +#define UAC3_CONTROL_PITCH             (3 << 0)
>> +#define UAC3_CONTROL_DATA_OVERRUN      (3 << 2)
>> +#define UAC3_CONTROL_DATA_UNDERRUN     (3 << 4)
>
>
> these are really masks, no?

Yes, same as in UAC2, we can drop them as they are not used yet.

>
>> +
>> +/* 6.1 INTERRUPT DATA MESSAGE */
>> +#define UAC3_INTERRUPT_DATA_MSG_VENDOR (1 << 0)
>> +#define UAC3_INTERRUPT_DATA_MSG_EP     (1 << 1)
>
>
> this is the same as in UAC2
>
>> +
>> +struct uac3_interrupt_data_msg {
>> +       __u8 bInfo;
>> +       __u8 bSourceType;
>> +       __le16 wValue;
>> +       __le16 wIndex;
>> +} __attribute__((packed));
>
>
> This seems identical to UAC2, the difference being the bSourceType with
> additional values (was bAttribute).

Yes, this is correct. I made separate structure just to make code reading
easier. Same is for UAC3_INTERRUPT_DATA_xxx above.

>
>> +
>> +/* A.2 AUDIO AUDIO FUNCTION SUBCLASS CODES */
>> +#define UAC3_FUNCTION_SUBCLASS_UNDEFINED       0x00
>> +#define UAC3_FUNCTION_SUBCLASS_FULL_ADC_3_0    0x01
>> +#define UAC3_FUNCTION_SUBCLASS_GENERIC_IO      0x20
>> +#define UAC3_FUNCTION_SUBCLASS_HEADPHONE       0x21
>> +#define UAC3_FUNCTION_SUBCLASS_SPEAKER         0x22
>> +#define UAC3_FUNCTION_SUBCLASS_MICROPHONE      0x23
>> +#define UAC3_FUNCTION_SUBCLASS_HEADSET         0x24
>> +#define UAC3_FUNCTION_SUBCLASS_HEADSET_ADAPTER 0x25
>> +#define UAC3_FUNCTION_SUBCLASS_SPEAKERPHONE    0x26
>
>
> mention that these are linked to BADD profiles.

OK, will do it

>
>> +
>> +/* A.7 AUDIO FUNCTION CATEGORY CODES */
>> +#define UAC3_FUNCTION_SUBCLASS_UNDEFINED       0x00
>> +#define UAC3_FUNCTION_DESKTOP_SPEAKER          0x01
>> +#define UAC3_FUNCTION_HOME_THEATER             0x02
>> +#define UAC3_FUNCTION_MICROPHONE               0x03
>> +#define UAC3_FUNCTION_HEADSET                  0x04
>> +#define UAC3_FUNCTION_TELEPHONE                        0x05
>> +#define UAC3_FUNCTION_CONVERTER                        0x06
>> +#define UAC3_FUNCTION_SOUND_RECORDER           0x07
>> +#define UAC3_FUNCTION_IO_BOX                   0x08
>> +#define UAC3_FUNCTION_MUSICAL_INSTRUMENT       0x09
>> +#define UAC3_FUNCTION_PRO_AUDIO                        0x0a
>> +#define UAC3_FUNCTION_AUDIO_VIDEO              0x0b
>> +#define UAC3_FUNCTION_CONTROL_PANEL            0x0c
>> +#define UAC3_FUNCTION_HEADPHONE                        0x0d
>> +#define UAC3_FUNCTION_GENERIC_SPEAKER          0x0e
>> +#define UAC3_FUNCTION_HEADSET_ADAPTER          0x0f
>> +#define UAC3_FUNCTION_SPEAKERPHONE             0x10
>> +#define UAC3_FUNCTION_OTHER                    0xff
>> +
>> +/* A.8 AUDIO CLASS-SPECIFIC DESCRIPTOR TYPES */
>> +#define UAC3_CS_UNDEFINED              0x20
>> +#define UAC3_CS_DEVICE                 0x21
>> +#define UAC3_CS_CONFIGURATION          0x22
>> +#define UAC3_CS_STRING                 0x23
>> +#define UAC3_CS_INTERFACE              0x24
>> +#define UAC3_CS_ENDPOINT               0x25
>> +#define UAC3_CS_CLUSTER                        0x26
>> +
>> +/* A.10 CLUSTER DESCRIPTOR SEGMENT TYPES */
>> +#define UAC3_SEGMENT_UNDEFINED         0x00
>> +#define UAC3_CLUSTER_DESCRIPTION       0x01
>> +#define UAC3_CLUSTER_VENDOR_DEFINED    0x1F
>> +#define UAC3_CHANNEL_INFORMATION       0x20
>> +#define UAC3_CHANNEL_AMBISONIC         0x21
>> +#define UAC3_CHANNEL_DESCRIPTION       0x22
>> +#define UAC3_CHANNEL_VENDOR_DEFINED    0xFE
>> +#define UAC3_END_SEGMENT               0xFF
>> +
>> +/* A.11 CHANNEL PURPOSE DEFINITIONS */
>> +#define UAC3_PURPOSE_UNDEFINED         0x00
>> +#define UAC3_PURPOSE_GENERIC_AUDIO     0x01
>> +#define UAC3_PURPOSE_VOICE             0x02
>> +#define UAC3_PURPOSE_SPEECH            0x03
>> +#define UAC3_PURPOSE_AMBIENT           0x04
>> +#define UAC3_PURPOSE_REFERENCE         0x05
>> +#define UAC3_PURPOSE_ULTRASONIC                0x06
>> +#define UAC3_PURPOSE_VIBROKINETIC      0x07
>> +#define UAC3_PURPOSE_NON_AUDIO         0xFF
>> +
>> +/* A.12 CHANNEL RELATIONSHIP DEFINITIONS */
>> +/* FIXME: spec is missing these constants. Few found in
>> BasicAudioDevice3.pdf */
>
>
> yes, this is a known bug in the released UAC3 document, the values were
> removed by accident. If this helps, here's what I have from the last release
> candidate, this will hopefully be corrected soon.

My next step was to send a follow-up email to usb.org about this issue,
but since you already have the answer, I'll include these values in the
next version of patch, thanks.

>
>
> RELATIONSHIP_UNDEFINED     UND  0x00
> MONO                       M    0x01
> LEFT                       L    0x02
> RIGHT                      R    0x03
> ARRAY                      AR   0x04
> PATTERN_X                  PX   0x20
> PATTERN_Y                  PY   0x21
> PATTERN_A                  PA   0x22
> PATTERN_B                  PB   0x23
> PATTERN_M                  PM   0x24
> PATTERN_S                  PS   0x25
> Front Left                 FL   0x80
> Front Right                FR   0x81
> Front Center               FC   0x82
> Front Left of Center       FLC  0x83
> Front Right of Center      FRC  0x84
> Front Wide Left            FWL  0x85
> Front Wide Right           FWR  0x86
> Side Left                  SL   0x87
> Side Right                 SR   0x88
> Surround Array Left        SAL  0x89
> Surround Array Right       SAR  0x8A
>
> Back Left                      BL      0x8B
> Back Right                     BR      0x8C
> Back Center                    BC      0x8D
> Back Left of Center            BLC     0x8E
> Back Right of Center           BRC     0x8F
> Back Wide Left                 BWL     0x90
> Back Wide Right                BWR     0x91
> Top Center                     TC      0x92
> Top Front Left                 TFL     0x93
> Top Front Right                TFR     0x94
> Top Front Center               TFC     0x95
> Top Front Left of Center       TFLC    0x96
> Top Front Right of Center      TFRC    0x97
> Top Front Wide Left            TFWL    0x98
> Top Front Wide Right           TFWR    0x99
> Top Side Left                  TSL     0x9A
> Top Side Right                 TSR     0x9B
> Top Surround Array Left        TSAL    0x9C
> Top Surround Array Right       TSAR    0x9D
> Top Back Left                  TBL     0x9E
> Top Back Right                 TBR     0x9F
> Top Back Center                TBC     0xA0
> Top Back Left Of Center        TBLC    0xA1
> Top Back Right Of Center       TBRC    0xA2
> Top Back Wide Left             TBWL    0xA3
> Top Back Wide Right            TBWR    0xA4
> Bottom Center                  BC      0xA5
> Bottom Front Left              BFL     0xA6
> Bottom Front Right             BFR     0xA7
> Bottom Front Center            BFC     0xA8
> Bottom Front Left Of Center    BFLC    0xA9
> Bottom Front Right Of Center   BFRC    0xAA
> Bottom Front Wide Left         BFWL    0xAB
> Bottom Front Wide Right        BFWR    0xAC
>
> Bottom Side Left             BSL    0xAD
> Bottom Side Right            BSR    0xAE
> Bottom Surround Array Left   BSAL   0xAF
> Bottom Surround Array Right  BSAR   0xB0
> Bottom Back Left             BBL    0xB1
> Bottom Back Right            BBR    0xB2
> Bottom Back Center           BBC    0xB3
> Bottom Back Left Of Center   BBLC   0xB4
> Bottom Back Right Of Center  BBRC   0xB5
> Bottom Back Wide Left        BBWL   0xB6
> Bottom Back Wide Right       BBWR   0xB7
> Low Frequency Effects        LFE    0xB8
> Low Frequency Effects Left   LFEL   0xB9
> Low Frequency Effects Right  LFER   0xBA
> Headphone Left               HPL    0xBB
> Headphone Right              HPR    0xBC
>
>
>> +#define UAC3_CH_RELATIONSHIP_UNDEFINED 0x00
>> +#define UAC3_CH_MONO                   0x01
>> +#define UAC3_CH_LEFT                   0x02
>> +#define UAC3_CH_RIGHT                  0x03
>> +#define UAC3_CH_FRONT_LEFT             0x80
>> +#define UAC3_CH_FRONT_RIGHT            0x81
>> +#define UAC3_CH_FRONT_CENTER           0x82
>> +#define UAC3_CH_SURROUND_ARRAY_LEFT    0x89
>> +#define UAC3_CH_SURROUND_ARRAY_RIGHT   0x8A
>> +#define UAC3_CH_LOW_FREQUENCY_EFFECTS  0xB8
>> +
>> +/* A.15 AUDIO CLASS-SPECIFIC AC INTERFACE DESCRIPTOR SUBTYPES */
>> +/* see audio.h for the rest, which is identical to v1 */
>> +#define UAC3_EXTENDED_TERMINAL         0x04
>> +#define UAC3_MIXER_UNIT                        0x05
>> +#define UAC3_SELECTOR_UNIT             0x06
>> +#define UAC3_FEATURE_UNIT              0x07
>> +#define UAC3_EFFECT_UNIT               0x08
>> +#define UAC3_PROCESSING_UNIT           0x09
>> +#define UAC3_EXTENSION_UNIT            0x0a
>> +#define UAC3_CLOCK_SOURCE              0x0b
>> +#define UAC3_CLOCK_SELECTOR            0x0c
>> +#define UAC3_CLOCK_MULTIPLIER          0x0d
>> +#define UAC3_SAMPLE_RATE_CONVERTER     0x0e
>> +#define UAC3_CONNECTORS                        0x0f
>> +#define UAC3_POWER_DOMAIN              0x10
>> +
>> +/* A.22 AUDIO CLASS-SPECIFIC REQUEST CODES */
>> +#define UAC3_CS_REQ_CUR                                0x01
>> +#define UAC3_CS_REQ_RANGE                      0x02
>> +#define UAC3_CS_REQ_MEM                                0x03
>
>
> these first 3 are already in UAC2
>
>> +#define UAC3_CS_REQ_INTEN                      0x04
>> +#define UAC3_CS_REQ_STRING                     0x05
>> +#define UAC3_CS_REQ_HIGH_CAPABILITY_DESCRIPTOR 0x06
>
>
> the last 3 are really UAC3-specific

Tha'ts correct. Do you want to say we can reuse UAC2 values?
This brings an issue I described previously in this mailing list.
It is related to UAC1/2/3 specs has some common values, sometimes
use different values for same things and this makes understanding
of sources quite complex if for example we will use UAC2_xx for
UAC3 in some cases for common things but UAC3_xx for UAC3-specifc.

I see 3 solutions:
1) use UAC2_xxx for common things (hard to read/write the code, you
    have to remember what is common and what is not, or leave
    a comment near each such place saying it's OK to use it for UAC3
2) rename UAC2_xxx to something like UAC_V2V3_xxx, so it will be
    self-explanatory
3) create UAC3_xxx for thouse things that are common with UAC2,
    so no need to do either 1 or 2, but it can lead to code duplication

In this patch I did some mix of 2 and 3, but additional comments are
welcome, as it variabl naming is the hardest part of programming :)
>
>> +
>> +/* A.23.1 AUDIOCONTROL INTERFACE CONTROL SELECTORS */
>> +#define UAC3_AC_CONTROL_UNDEFINED              0x00
>> +#define UAC3_AC_ACTIVE_INTERFACE_CONTROL       0x01
>> +#define UAC3_AC_POWER_DOMAIN_CONTROL           0x02
>> +
>> +#endif /* __LINUX_USB_AUDIO_V3_H */
>> diff --git a/include/uapi/linux/usb/audio.h
>> b/include/uapi/linux/usb/audio.h
>> index a4680a5..66ec2ae 100644
>> --- a/include/uapi/linux/usb/audio.h
>> +++ b/include/uapi/linux/usb/audio.h
>> @@ -26,6 +26,7 @@
>>   /* bInterfaceProtocol values to denote the version of the standard used
>> */
>>   #define UAC_VERSION_1                 0x00
>>   #define UAC_VERSION_2                 0x20
>> +#define UAC_VERSION_3                  0x30
>>     /* A.2 Audio Interface Subclass Codes */
>>   #define USB_SUBCLASS_AUDIOCONTROL     0x01
>> diff --git a/sound/usb/card.c b/sound/usb/card.c
>> index 3dc36d9..df3f0ab 100644
>> --- a/sound/usb/card.c
>> +++ b/sound/usb/card.c
>> @@ -7,6 +7,7 @@
>>    *        Alan Cox (alan@lxorguk.ukuu.org.uk)
>>    *        Thomas Sailer (sailer@ife.ee.ethz.ch)
>>    *
>> + *   Audio Class 3.0 support by Ruslan Bilovol <ruslan.bilovol@gmail.com>
>
>
> does this mean new copyright?

That's a good qestion, I don't know the answer. Just wanted to leave
a comment about work I did for USB audio.

>
>>    *
>>    *   This program is free software; you can redistribute it and/or
>> modify
>>    *   it under the terms of the GNU General Public License as published
>> by
>> @@ -44,6 +45,7 @@
>>   #include <linux/mutex.h>
>>   #include <linux/usb/audio.h>
>>   #include <linux/usb/audio-v2.h>
>> +#include <linux/usb/audio-v3.h>
>>   #include <linux/module.h>
>>     #include <sound/control.h>
>> @@ -261,7 +263,8 @@ static int snd_usb_create_streams(struct snd_usb_audio
>> *chip, int ctrlif)
>>                 break;
>>         }
>>   -     case UAC_VERSION_2: {
>> +       case UAC_VERSION_2:
>> +       case UAC_VERSION_3: {
>>                 struct usb_interface_assoc_descriptor *assoc =
>>                         usb_ifnum_to_if(dev, ctrlif)->intf_assoc;
>>   @@ -281,7 +284,7 @@ static int snd_usb_create_streams(struct
>> snd_usb_audio *chip, int ctrlif)
>>                 }
>>                 if (!assoc) {
>> -                       dev_err(&dev->dev, "Audio class v2 interfaces need
>> an interface association\n");
>> +                       dev_err(&dev->dev, "Audio class v2/v3 interfaces
>> need an interface association\n");
>
>
> yes, but they can have more than one, so how do you handle the selection
> (see 3.3)?

Current implentation of USB audio driver is to create only one ALSA
card per USB device, that works with UAC1/UAC2 and now with UAC3;
so if USB device hase more that one interface association, only first
will be used.

But if you mean configuration switching (between backward
compatible UAC1/UAC2 and UAC3 configuration) that will not limit us,
since after switching to another configuration  UAC1/UAC2 interfaces
will be released and UAC3 will be probed, this is done by USB core.

As described above, currently I switching between configs manually.

>
>>                         return -EINVAL;
>>                 }
>>   diff --git a/sound/usb/card.h b/sound/usb/card.h
>> index 111b0f0..5e2934f 100644
>> --- a/sound/usb/card.h
>> +++ b/sound/usb/card.h
>> @@ -21,7 +21,7 @@ struct audioformat {
>>         unsigned char endpoint;         /* endpoint */
>>         unsigned char ep_attr;          /* endpoint attributes */
>>         unsigned char datainterval;     /* log_2 of data packet interval
>> */
>> -       unsigned char protocol;         /* UAC_VERSION_1/2 */
>> +       unsigned char protocol;         /* UAC_VERSION_1/2/3 */
>>         unsigned int maxpacksize;       /* max. packet size */
>>         unsigned int rates;             /* rate bitmasks */
>>         unsigned int rate_min, rate_max;        /* min/max rates */
>> diff --git a/sound/usb/clock.c b/sound/usb/clock.c
>> index 26dd5f2..04361d7 100644
>> --- a/sound/usb/clock.c
>> +++ b/sound/usb/clock.c
>> @@ -23,6 +23,7 @@
>>   #include <linux/usb.h>
>>   #include <linux/usb/audio.h>
>>   #include <linux/usb/audio-v2.h>
>> +#include <linux/usb/audio-v3.h>
>>     #include <sound/core.h>
>>   #include <sound/info.h>
>> @@ -50,6 +51,22 @@
>>         return NULL;
>>   }
>>   +static struct uac3_clock_source_descriptor *
>> +       snd_usb_find_clock_source_v3(struct usb_host_interface
>> *ctrl_iface,
>> +                                 int clock_id)
>> +{
>> +       struct uac3_clock_source_descriptor *cs = NULL;
>> +
>> +       while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
>> +                                            ctrl_iface->extralen,
>> +                                            cs, UAC3_CLOCK_SOURCE))) {
>> +               if (cs->bClockID == clock_id)
>> +                       return cs;
>> +       }
>> +
>> +       return NULL;
>> +}
>> +
>>   static struct uac_clock_selector_descriptor *
>>         snd_usb_find_clock_selector(struct usb_host_interface *ctrl_iface,
>>                                     int clock_id)
>> @@ -66,6 +83,22 @@
>>         return NULL;
>>   }
>>   +static struct uac3_clock_selector_descriptor *
>> +       snd_usb_find_clock_selector_v3(struct usb_host_interface
>> *ctrl_iface,
>> +                                   int clock_id)
>> +{
>> +       struct uac3_clock_selector_descriptor *cs = NULL;
>> +
>> +       while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
>> +                                            ctrl_iface->extralen,
>> +                                            cs, UAC3_CLOCK_SELECTOR))) {
>> +               if (cs->bClockID == clock_id)
>> +                       return cs;
>> +       }
>> +
>> +       return NULL;
>> +}
>> +
>>   static struct uac_clock_multiplier_descriptor *
>>         snd_usb_find_clock_multiplier(struct usb_host_interface
>> *ctrl_iface,
>>                                       int clock_id)
>> @@ -82,6 +115,22 @@
>>         return NULL;
>>   }
>>   +static struct uac3_clock_multiplier_descriptor *
>> +       snd_usb_find_clock_multiplier_v3(struct usb_host_interface
>> *ctrl_iface,
>> +                                     int clock_id)
>> +{
>> +       struct uac3_clock_multiplier_descriptor *cs = NULL;
>> +
>> +       while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
>> +                                            ctrl_iface->extralen,
>> +                                            cs, UAC3_CLOCK_MULTIPLIER)))
>> {
>> +               if (cs->bClockID == clock_id)
>> +                       return cs;
>> +       }
>> +
>> +       return NULL;
>> +}
>> +
>>   static int uac_clock_selector_get_val(struct snd_usb_audio *chip, int
>> selector_id)
>>   {
>>         unsigned char buf;
>> @@ -135,19 +184,33 @@ static int uac_clock_selector_set_val(struct
>> snd_usb_audio *chip, int selector_i
>>         return ret;
>>   }
>>   -static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, int
>> source_id)
>> +static bool uac_clock_source_is_valid(struct snd_usb_audio *chip,
>> +                                     int protocol,
>> +                                     int source_id)
>>   {
>>         int err;
>>         unsigned char data;
>>         struct usb_device *dev = chip->dev;
>> -       struct uac_clock_source_descriptor *cs_desc =
>> -               snd_usb_find_clock_source(chip->ctrl_intf, source_id);
>> -
>> -       if (!cs_desc)
>> -               return 0;
>> +       u32 bmControls;
>> +
>> +       if (protocol == UAC_VERSION_3) {
>> +               struct uac3_clock_source_descriptor *cs_desc =
>> +                       snd_usb_find_clock_source_v3(chip->ctrl_intf,
>> source_id);
>> +
>> +               if (!cs_desc)
>> +                       return 0;
>> +               bmControls = le32_to_cpu(cs_desc->bmControls);
>> +       } else { /* UAC_VERSION_1/2 */
>
>
> no, clock sources are only in UAC2.

Correct, will fix in next patch set

>
>> +               struct uac_clock_source_descriptor *cs_desc =
>> +                       snd_usb_find_clock_source(chip->ctrl_intf,
>> source_id);
>> +
>> +               if (!cs_desc)
>> +                       return 0;
>> +               bmControls = cs_desc->bmControls;
>> +       }
>>         /* If a clock source can't tell us whether it's valid, we assume
>> it is */
>> -       if (!uac2_control_is_readable(cs_desc->bmControls,
>> +       if (!uac_v2v3_control_is_readable(bmControls,
>>                                       UAC2_CS_CONTROL_CLOCK_VALID - 1))
>>                 return 1;
>>   @@ -167,9 +230,8 @@ static bool uac_clock_source_is_valid(struct
>> snd_usb_audio *chip, int source_id)
>>         return !!data;
>>   }
>>   -static int __uac_clock_find_source(struct snd_usb_audio *chip,
>> -                                  int entity_id, unsigned long *visited,
>> -                                  bool validate)
>> +static int __uac_clock_find_source(struct snd_usb_audio *chip, int
>> entity_id,
>> +                                  unsigned long *visited, bool validate)
>>   {
>>         struct uac_clock_source_descriptor *source;
>>         struct uac_clock_selector_descriptor *selector;
>> @@ -188,7 +250,8 @@ static int __uac_clock_find_source(struct
>> snd_usb_audio *chip,
>>         source = snd_usb_find_clock_source(chip->ctrl_intf, entity_id);
>>         if (source) {
>>                 entity_id = source->bClockID;
>> -               if (validate && !uac_clock_source_is_valid(chip,
>> entity_id)) {
>> +               if (validate && !uac_clock_source_is_valid(chip,
>> UAC_VERSION_2,
>> +
>> entity_id)) {
>>                         usb_audio_err(chip,
>>                                 "clock source %d is not valid, cannot
>> use\n",
>>                                 entity_id);
>> @@ -257,6 +320,97 @@ static int __uac_clock_find_source(struct
>> snd_usb_audio *chip,
>>         return -EINVAL;
>>   }
>>   +static int __uac3_clock_find_source(struct snd_usb_audio *chip, int
>> entity_id,
>> +                                  unsigned long *visited, bool validate)
>> +{
>> +       struct uac3_clock_source_descriptor *source;
>> +       struct uac3_clock_selector_descriptor *selector;
>> +       struct uac3_clock_multiplier_descriptor *multiplier;
>> +
>> +       entity_id &= 0xff;
>> +
>> +       if (test_and_set_bit(entity_id, visited)) {
>> +               usb_audio_warn(chip,
>> +                        "%s(): recursive clock topology detected, id
>> %d.\n",
>> +                        __func__, entity_id);
>> +               return -EINVAL;
>> +       }
>> +
>> +       /* first, see if the ID we're looking for is a clock source
>> already */
>> +       source = snd_usb_find_clock_source_v3(chip->ctrl_intf, entity_id);
>> +       if (source) {
>> +               entity_id = source->bClockID;
>> +               if (validate && !uac_clock_source_is_valid(chip,
>> UAC_VERSION_3,
>> +
>> entity_id)) {
>> +                       usb_audio_err(chip,
>> +                               "clock source %d is not valid, cannot
>> use\n",
>> +                               entity_id);
>> +                       return -ENXIO;
>> +               }
>> +               return entity_id;
>> +       }
>> +
>> +       selector = snd_usb_find_clock_selector_v3(chip->ctrl_intf,
>> entity_id);
>> +       if (selector) {
>> +               int ret, i, cur;
>> +
>> +               /* the entity ID we are looking for is a selector.
>> +                * find out what it currently selects */
>> +               ret = uac_clock_selector_get_val(chip,
>> selector->bClockID);
>> +               if (ret < 0)
>> +                       return ret;
>> +
>> +               /* Selector values are one-based */
>> +
>> +               if (ret > selector->bNrInPins || ret < 1) {
>> +                       usb_audio_err(chip,
>> +                               "%s(): selector reported illegal value, id
>> %d, ret %d\n",
>> +                               __func__, selector->bClockID, ret);
>> +
>> +                       return -EINVAL;
>> +               }
>> +
>> +               cur = ret;
>> +               ret = __uac3_clock_find_source(chip,
>> selector->baCSourceID[ret - 1],
>> +                                              visited, validate);
>> +               if (!validate || ret > 0 || !chip->autoclock)
>> +                       return ret;
>> +
>> +               /* The current clock source is invalid, try others. */
>> +               for (i = 1; i <= selector->bNrInPins; i++) {
>> +                       int err;
>> +
>> +                       if (i == cur)
>> +                               continue;
>> +
>> +                       ret = __uac3_clock_find_source(chip,
>> selector->baCSourceID[i - 1],
>> +                               visited, true);
>> +                       if (ret < 0)
>> +                               continue;
>> +
>> +                       err = uac_clock_selector_set_val(chip, entity_id,
>> i);
>> +                       if (err < 0)
>> +                               continue;
>> +
>> +                       usb_audio_info(chip,
>> +                                "found and selected valid clock source
>> %d\n",
>> +                                ret);
>> +                       return ret;
>> +               }
>> +
>> +               return -ENXIO;
>> +       }
>> +
>> +       /* FIXME: multipliers only act as pass-thru element for now */
>> +       multiplier = snd_usb_find_clock_multiplier_v3(chip->ctrl_intf,
>> +                                                     entity_id);
>> +       if (multiplier)
>> +               return __uac3_clock_find_source(chip,
>> multiplier->bCSourceID,
>> +                                               visited, validate);
>> +
>> +       return -EINVAL;
>
>
> the difference between clock structures is not that big really between UAC2
> and UAC3 (wider bmcontrols, change to reference terminal and
> wClockSourceStr_), could this code be refactored to handle both UAC2 and
> UAC3?

Yes, that's the issue I described above. Will see what I can do here.

>
>
>> +}
>> +
>>   /*
>>    * For all kinds of sample rate settings and other device queries,
>>    * the clock source (end-leaf) must be used. However, clock selectors,
>> @@ -268,12 +422,22 @@ static int __uac_clock_find_source(struct
>> snd_usb_audio *chip,
>>    *
>>    * Returns the clock source UnitID (>=0) on success, or an error.
>>    */
>> -int snd_usb_clock_find_source(struct snd_usb_audio *chip, int entity_id,
>> -                             bool validate)
>> +int snd_usb_clock_find_source(struct snd_usb_audio *chip, int protocol,
>> +                             int entity_id, bool validate)
>>   {
>>         DECLARE_BITMAP(visited, 256);
>>         memset(visited, 0, sizeof(visited));
>> -       return __uac_clock_find_source(chip, entity_id, visited,
>> validate);
>> +
>> +       switch (protocol) {
>> +       case UAC_VERSION_2:
>> +               return __uac_clock_find_source(chip, entity_id, visited,
>> +                                              validate);
>> +       case UAC_VERSION_3:
>> +               return __uac3_clock_find_source(chip, entity_id, visited,
>> +                                              validate);
>
>
> this looks again very similar, could this be optimized?

Same here

>
>
>>   #include <sound/core.h>
>>   #include <sound/pcm.h>
>> @@ -39,11 +40,11 @@
>>    * @dev: usb device
>>    * @fp: audioformat record
>>    * @format: the format tag (wFormatTag)
>> - * @fmt: the format type descriptor
>> + * @fmt: the format type descriptor (v1/v2) or AudioStreaming descriptor
>> (v3)
>>    */
>>   static u64 parse_audio_format_i_type(struct snd_usb_audio *chip,
>>                                      struct audioformat *fp,
>> -                                    unsigned int format, void *_fmt)
>> +                                    u64 format, void *_fmt)
>>   {
>>         int sample_width, sample_bytes;
>>         u64 pcm_formats = 0;
>> @@ -69,6 +70,17 @@ static u64 parse_audio_format_i_type(struct
>> snd_usb_audio *chip,
>>                 format <<= 1;
>>                 break;
>>         }
>> +       case UAC_VERSION_3: {
>> +               struct uac3_as_header_descriptor *as = _fmt;
>> +               sample_width = as->bBitResolution;
>> +               sample_bytes = as->bSubslotSize;
>> +
>> +               if (format & UAC3_FORMAT_TYPE_I_RAW_DATA)
>> +                       pcm_formats |= SNDRV_PCM_FMTBIT_SPECIAL;
>
>
> could this be expanded to add at least basic PCM (D0 set).

Basic PCM, and more (D0..D5) are handled, this is needed for special
case of raw data, same as for UAC2, see commit 717bfb5 ("ALSA:
snd-usb: handle raw data format of UAC2 devices")

>
>> +int snd_usb_parse_audio_format_v3(struct snd_usb_audio *chip,
>> +                              struct audioformat *fp,
>> +                              struct uac3_as_header_descriptor *as,
>> +                              int stream)
>> +{
>> +       u64 format = le64_to_cpu(as->bmFormats);
>> +       int err;
>> +
>> +       /* Type I format bits are D0..D6 */
>> +       if (format & 0x7f)
>> +               fp->fmt_type = UAC_FORMAT_TYPE_I;
>> +       else
>> +               fp->fmt_type = UAC_FORMAT_TYPE_III;
>
> maybe mention that this test only work because type IV is not supported,
> otherwise this wouldn't quite right.

Yes, will do it.

>
>> +int snd_usb_parse_audio_format_v3(struct snd_usb_audio *chip,
>> +                              struct audioformat *fp,
>> +                              struct uac3_as_header_descriptor *as,
>> +                              int stream);
>>   #endif /*  __USBAUDIO_FORMAT_H */
>> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
>> index 9732edf..5b5dfe4 100644
>> --- a/sound/usb/mixer.c
>> +++ b/sound/usb/mixer.c
>> @@ -51,6 +51,7 @@
>>   #include <linux/usb.h>
>>   #include <linux/usb/audio.h>
>>   #include <linux/usb/audio-v2.h>
>> +#include <linux/usb/audio-v3.h>
>>     #include <sound/core.h>
>>   #include <sound/control.h>
>> @@ -189,7 +190,7 @@ static void *find_audio_control_unit(struct
>> mixer_build *state,
>>                                         USB_DT_CS_INTERFACE)) != NULL) {
>>                 if (hdr->bLength >= 4 &&
>>                     hdr->bDescriptorSubtype >= UAC_INPUT_TERMINAL &&
>> -                   hdr->bDescriptorSubtype <= UAC2_SAMPLE_RATE_CONVERTER
>> &&
>> +                   hdr->bDescriptorSubtype <= UAC3_SAMPLE_RATE_CONVERTER
>> &&
>>                     hdr->bUnitID == unit)
>
>
> this looks like a mistake, the definitions are
>
> #define UAC3_SAMPLE_RATE_CONVERTER      0x0e
> #define UAC3_CONNECTORS                 0x0f
> #define UAC3_POWER_DOMAIN               0x10
>
> You will miss the last two with this test.

That's because connectors descriptor uses High Capability descriptor for
replresentation, thus it can't be compared here.
Power Domain descriptor isn't used yet by this driver, so let's check for
it once this functionality will be added.

Probably it makes sense to make this comparison UAC version depended
to not break existing UAC1/2 functionality, I'll check it.

>
>
>> -               case UAC1_PROCESSING_UNIT:
>> -               case UAC1_EXTENSION_UNIT:
>> -               /* UAC2_PROCESSING_UNIT_V2 */
>> -               /* UAC2_EFFECT_UNIT */
>> -               case UAC2_EXTENSION_UNIT_V2: {
>> -                       struct uac_processing_unit_descriptor *d = p1;
>> -
>> -                       if (state->mixer->protocol == UAC_VERSION_2 &&
>> -                               hdr[2] == UAC2_EFFECT_UNIT) {
>> -                               /* UAC2/UAC1 unit IDs overlap here in an
>> -                                * uncompatible way. Ignore this unit for
>> now.
>> -                                */
>> +
>> +                               /* REVISIT: UAC3 IT doesn't have
>> channels/cfg */
>> +                               term->channels = 0;
>> +                               term->chconfig = 0;
>
>
> It does, but you need to get the information from the wClusterDescrID

Correct. Channels are used during mixer parsing which we don't support
yet, chconfig is set in many places but nobody uses it, so we can remove it from
usb_audio_term struct.

>
> [snip]
>
>> +/* UAC3 device stores channels information in Cluster Descriptors */
>> +static struct
>> +snd_pcm_chmap_elem *convert_chmap_v3(struct
>> uac3_cluster_header_descriptor
>> +                                                               *cluster)
>> +{
>> +       unsigned int channels = cluster->bNrChannels;
>> +       struct snd_pcm_chmap_elem *chmap;
>> +       void *p = cluster;
>> +       int len, c;
>> +
>> +       if (channels > ARRAY_SIZE(chmap->map))
>> +               return NULL;
>> +
>> +       chmap = kzalloc(sizeof(*chmap), GFP_KERNEL);
>> +       if (!chmap)
>> +               return NULL;
>> +
>> +       len = le16_to_cpu(cluster->wLength);
>> +       c = 0;
>> +       p += sizeof(struct uac3_cluster_header_descriptor);
>> +
>> +       while (((p - (void *)cluster) < len) && (c < channels)) {
>> +               struct uac3_cluster_segment_descriptor *cs_desc = p;
>> +               u16 cs_len;
>> +               u8 cs_type;
>> +
>> +               cs_len = le16_to_cpu(cs_desc->wLength);
>> +               cs_type = cs_desc->bSegmentType;
>> +
>> +               if (cs_type == UAC3_CHANNEL_INFORMATION) {
>> +                       struct uac3_cluster_information_segment_descriptor
>> *is = p;
>> +                       unsigned char map;
>> +
>> +                       /*
>> +                        * FIXME: this conversion is simplified, because
>> +                        * asound.h doesn't have UAC3 values yet
>
>
>
> yes, we need to have a mapping between USB and CEA definitions (which are
> aligned to some extent btw)
>
>> +                        */
>> +                       switch (is->bChPurpose) {
>> +                       case UAC3_CH_MONO:
>> +                               map = SNDRV_CHMAP_MONO;
>> +                               break;
>> +                       case UAC3_CH_LEFT:
>> +                       case UAC3_CH_FRONT_LEFT:
>> +                       case UAC3_CH_SURROUND_ARRAY_LEFT:
>> +                               map = SNDRV_CHMAP_FL;
>> +                               break;
>> +                       case UAC3_CH_RIGHT:
>> +                       case UAC3_CH_FRONT_RIGHT:
>> +                       case UAC3_CH_SURROUND_ARRAY_RIGHT:
>> +                               map = SNDRV_CHMAP_FR;
>> +                               break;
>> +                       case UAC3_CH_FRONT_CENTER:
>> +                               map = SNDRV_CHMAP_FC;
>> +                               break;
>> +                       case UAC3_CH_LOW_FREQUENCY_EFFECTS:
>> +                               map = SNDRV_CHMAP_LFE;
>> +                               break;
>> +                       case UAC3_CH_RELATIONSHIP_UNDEFINED:
>> +                       default:
>> +                               map = SNDRV_CHMAP_UNKNOWN;
>> +                               break;


Best regards,
Ruslan Bilovol
Ruslan Bilovol Nov. 11, 2017, 2:56 a.m. UTC | #10
On Thu, Nov 9, 2017 at 10:33 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Nov 09, 2017 at 09:16:52AM +0100, Takashi Iwai wrote:
>> On Thu, 09 Nov 2017 09:04:58 +0100,
>> Greg Kroah-Hartman wrote:
>> >
>> > On Wed, Nov 08, 2017 at 03:38:35PM +0100, Takashi Iwai wrote:
>> > > On Tue, 07 Nov 2017 03:01:20 +0100,
>> > > Ruslan Bilovol wrote:
>> > > >
>> > > > Recently released USB Audio Class 3.0 specification
>> > > > introduces many significant changes comparing to
>> > > > previous versions, like
>> > > >  - new Power Domains, support for LPM/L1
>> > > >  - new Cluster descriptor
>> > > >  - changed layout of all class-specific descriptors
>> > > >  - new High Capability descriptors
>> > > >  - New class-specific String descriptors
>> > > >  - new and removed units
>> > > >  - additional sources for interrupts
>> > > >  - removed Type II Audio Data Formats
>> > > >  - ... and many other things (check spec)
>> > > >
>> > > > It also provides backward compatibility through
>> > > > multiple configurations, as well as requires
>> > > > mandatory support for BADD (Basic Audio Device
>> > > > Definition) on each ADC3.0 compliant device
>> > > >
>> > > > This patch adds initial support of UAC3 specification
>> > > > that is enough for Generic I/O Profile (BAOF, BAIF)
>> > > > device support from BADD document.
>> > > >
>> > > > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
>> > >
>> > > The patch looks good, but the timing is fairly late for merging to
>> > > 4.15.
>> >
>> > Isn't kbuild barfing all over these?  Is that because of the cross-tree
>> > changes needed?
>>
>> No, it's just local to the audio driver, plus a few
>> include/linux/usb/*.h modification / addition.
>> So there shouldn't be a big breakage in that regard.

The UAC3 gadget driver (which I sent to linux-usb mailing list) depends on
a new audio-v3.h header which is a part of this patch.

So Felipe's tree will have dependency on Takashi's tree; and that's why
kbuild notifies about build failure of UAC3 gadget driver.

>>
>>
>> > > So from my side, the primary question is whether the changes in USB
>> > > (audio) header files are OK for USB guys.
>> > >
>> > > Greg, could you check these changes and give an ack if it's OK to
>> > > merge?  Or if you prefer postpone, just let me know.
>> >
>> > I don't object to the USB header changes, as long as the fixes you point
>> > out are made :)
>>
>> Good to hear.
>> Could you give your acked-by once after reviewing the patch?
>
> I'm guessing there will be a new patch?  I'll be glad to review that
> one.

Sure, there will be a new patch set soon.

Thanks,
Ruslan
Takashi Iwai Nov. 11, 2017, 7:29 a.m. UTC | #11
On Sat, 11 Nov 2017 03:56:17 +0100,
Ruslan Bilovol wrote:
> 
> On Thu, Nov 9, 2017 at 10:33 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Thu, Nov 09, 2017 at 09:16:52AM +0100, Takashi Iwai wrote:
> >> On Thu, 09 Nov 2017 09:04:58 +0100,
> >> Greg Kroah-Hartman wrote:
> >> >
> >> > On Wed, Nov 08, 2017 at 03:38:35PM +0100, Takashi Iwai wrote:
> >> > > On Tue, 07 Nov 2017 03:01:20 +0100,
> >> > > Ruslan Bilovol wrote:
> >> > > >
> >> > > > Recently released USB Audio Class 3.0 specification
> >> > > > introduces many significant changes comparing to
> >> > > > previous versions, like
> >> > > >  - new Power Domains, support for LPM/L1
> >> > > >  - new Cluster descriptor
> >> > > >  - changed layout of all class-specific descriptors
> >> > > >  - new High Capability descriptors
> >> > > >  - New class-specific String descriptors
> >> > > >  - new and removed units
> >> > > >  - additional sources for interrupts
> >> > > >  - removed Type II Audio Data Formats
> >> > > >  - ... and many other things (check spec)
> >> > > >
> >> > > > It also provides backward compatibility through
> >> > > > multiple configurations, as well as requires
> >> > > > mandatory support for BADD (Basic Audio Device
> >> > > > Definition) on each ADC3.0 compliant device
> >> > > >
> >> > > > This patch adds initial support of UAC3 specification
> >> > > > that is enough for Generic I/O Profile (BAOF, BAIF)
> >> > > > device support from BADD document.
> >> > > >
> >> > > > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> >> > >
> >> > > The patch looks good, but the timing is fairly late for merging to
> >> > > 4.15.
> >> >
> >> > Isn't kbuild barfing all over these?  Is that because of the cross-tree
> >> > changes needed?
> >>
> >> No, it's just local to the audio driver, plus a few
> >> include/linux/usb/*.h modification / addition.
> >> So there shouldn't be a big breakage in that regard.
> 
> The UAC3 gadget driver (which I sent to linux-usb mailing list) depends on
> a new audio-v3.h header which is a part of this patch.
> 
> So Felipe's tree will have dependency on Takashi's tree; and that's why
> kbuild notifies about build failure of UAC3 gadget driver.

OK, I see.  I can prepare an immutable branch once when the patches
are ready to merge.


Takashi
Pierre-Louis Bossart Nov. 13, 2017, 4:07 p.m. UTC | #12
On 11/10/17 8:48 PM, Ruslan Bilovol wrote:
> On Wed, Nov 8, 2017 at 9:38 PM, Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com> wrote:
>> Nice work, thanks! I double-checked all the descriptors and values and
>> didn't find anything problematic, the main comment I have is that the clock
>> source/selection could probably be refactored since the differences are
>> really minor with UAC2, and there is work to do to select the right Audio
>> Interface Association.
>>
>> On 11/6/17 8:01 PM, Ruslan Bilovol wrote:
>>>
>>> Recently released USB Audio Class 3.0 specification
>>> introduces many significant changes comparing to
>>> previous versions, like
>>>    - new Power Domains, support for LPM/L1
>>>    - new Cluster descriptor
>>>    - changed layout of all class-specific descriptors
>>>    - new High Capability descriptors
>>>    - New class-specific String descriptors
>>>    - new and removed units
>>>    - additional sources for interrupts
>>>    - removed Type II Audio Data Formats
>>>    - ... and many other things (check spec)
>>>
>>> It also provides backward compatibility through
>>> multiple configurations, as well as requires
>>> mandatory support for BADD (Basic Audio Device
>>> Definition) on each ADC3.0 compliant device
>>>
>>> This patch adds initial support of UAC3 specification
>>> that is enough for Generic I/O Profile (BAOF, BAIF)
>>> device support from BADD document.
>>
>>
>> Do you mean to say that the selection of the BADD profile or full-blown
>> descriptor capabilities will be handled in a follow-up patch?
> 
> I have no follow-up patch for configuration switching.
> I mean only that it is enough to support BAOF, BAIF profiles.
> however, BAIOF profile won't work correctly, becaus it contains mixer unit
> which is not supported yet in this patch.
> 
> This is because of way I use UAC3. As UAC3 device, I use an UAC3
> gadget implementation which I sent before to linux-usb mailing list:
> http://www.spinics.net/lists/linux-usb/msg162482.html
> 
> The UAC3 gadget implements BAOF+BAIF profile; but not BAIOF profile
> which doesn't make sense in this case (there is no reason to mix Audio IN
> with Audio Out)

sidetone is the main reason for mixing input on output, very useful too 
avoid speaking too loudly into the mic or when you use well-insulated 
headsets. most USB headsets provide this capability.

> 
> Thus I can't implement and test UAC3 mixer handling in current patch,
> so BAIOF profile isn't supported by it yet.
> 
>>
>> It's my understanding from Section 3.3 that a UAC3 device is required to
>> expose
>> - a backwards-compatible configuration (UAC1 or UAC2),
>> - a UAC3 BADD profile
>> - one or more AIA compliant with UAC3 (which may have additional features
>> not present in the baseline description).
> 
> My understanding of Section 3.3 is slightly different. An UAC3 device is
> required to expose:
>   - a backward-compatible first configuration (UAC1 or UAC2)
>   - an UAC3 BADD profile on another configuration, with only one AIA inside
>   - and _may_ have one or more configurations with UAC3 support that
>     provide functionality beyond what is available in BADD

yes, i should have written 0 or more AIA compliant with UAC3

> 
>>
>> And I am not sure how a driver would make the selection...
> 
> In order to test this patch, I created UAC2+UAC3 gadget and manually
> switched to UAC3 configuration on Host by:
> $ echo 2 > /sys/bus/usb/devices/1-1/bConfigurationValue
> 
> I don't think ALSA driver can make decision which configuration to select,
> maybe some userspace tool can handle it, like usb_modeswitch does
> for networking devices.

The BADD profiles were intended to provide a basic descriptors for hosts 
that couldn't handle descriptor parsing. That was a very controversial 
work item, since both the Linux and Windows stacks can parse descriptors 
the usefulness of BADD profiles remains questionable.

I think at some point we should have the following policy
if one UAC3 is available, use the first one
else
     if there is a BADD profile use it
     else
         fall back to UAC1/2

I don't see how a user will ever choose between multiple UAC3 
configurations, should they ever exist, except maybe by trial and error.

> 
>>
>> In theory though, a UAC3 device should work out of the box even if the host
>> only supports UAC1 or UAC2.
> 
> Yes, this is correct, I verified it as well.
> 
>>
>>
>>> +/*
>>> + * v1.0, v2.0 and v3.0 of this standard have many things in common. For
>>> the rest
>>> + * of the definitions, please refer to audio.h and audio-v2.h
>>> + */
>>> +
>>> +/* All High Capability descriptors have these 2 fields at the beginning
>>> */
>>> +struct uac3_hc_descriptor_header {
>>> +       __le16 wLength;
>>> +       __u8 bDescriptorType;
>>> +       __u8 bDescriptorSubtype;
>>> +       __le16 wDescriptorID;
>>> +} __attribute__ ((packed));
>>> +
>>> +/* 4.3.1 CLUSTER DESCRIPTOR HEADER */
>>> +struct uac3_cluster_header_descriptor {
>>> +       __le16 wLength;
>>> +       __u8 bDescriptorType;
>>> +       __u8 bDescriptorSubtype;
>>> +       __le16 wDescriptorID;
>>> +       __u8 bNrChannels;
>>> +} __attribute__ ((packed));
>>> +
>>> +/* 4.3.2.1 SEGMENTS */
>>> +struct uac3_cluster_segment_descriptor {
>>> +       __le16 wLength;
>>> +       __u8 bSegmentType;
>>> +       /* __u8[0]; segment-specific data */
>>> +} __attribute__ ((packed));
>>> +
>>> +/* 4.3.2.1.1 END SEGMENT */
>>> +struct uac3_cluster_end_segment_descriptor {
>>> +       __le16 wLength;
>>> +       __u8 bSegmentType;              /* Constant END_SEGMENT */
>>> +} __attribute__ ((packed));
>>> +
>>
>>
>> you didn't include the definitions in 4.3.2.1.2
>> Cluster Descriptor Segment
>> Vendor Defined Segment
> 
> Correct, they are not used in BADD so I didn't care much, but I can
> add it in next patchset

ok

> 
>>
>>> +/* 4.3.2.1.3.1 INFORMATION SEGMENT */
>>> +struct uac3_cluster_information_segment_descriptor {
>>> +       __le16 wLength;
>>> +       __u8 bSegmentType;
>>
>>
>> this field is a CHANNEL_INFORMATION constant.
>>
>>> +       __u8 bChPurpose;
>>> +       __u8 bChRelationship;
>>> +       __u8 bChGroupID;
>>> +} __attribute__ ((packed));
>>> +
>>> +/* 4.5.2 CLASS-SPECIFIC AC INTERFACE DESCRIPTOR */
>>> +struct uac3_ac_header_descriptor {
>>> +       __u8 bLength;                   /* 10 */
>>> +       __u8 bDescriptorType;           /* CS_INTERFACE descriptor type */
>>> +       __u8 bDescriptorSubtype;        /* HEADER descriptor subtype */
>>> +       __u8 bCategory;
>>> +
>>> +       /* includes Clock Source, Unit, Terminal, and Power Domain desc.
>>> */
>>> +       __le16 wTotalLength;
>>> +
>>> +       __le32 bmControls;
>>> +} __attribute__ ((packed));
>>> +
>>
>>
>> Missing extended terminal descriptor header from 4.5.2.3.2?
>>
>> Missing multi-function processing unit descriptor header from 4.5.2.10.3 ?
> 
> Yes, I can add them as well.

ok

> 
>>
>>> +
>>> +/* bmAttribute fields */
>>> +#define UAC3_CLOCK_SOURCE_TYPE_EXT     0x0
>>> +#define UAC3_CLOCK_SOURCE_TYPE_INT     0x1
>>> +#define UAC3_CLOCK_SOURCE_ASYNC                (0 << 2)
>>> +#define UAC3_CLOCK_SOURCE_SYNCED_TO_SOF        (1 << 1)
>>> +
>>> +/* 4.5.2.13 CLOCK SELECTOR DESCRIPTOR */
>>> +struct uac3_clock_selector_descriptor {
>>> +       __u8 bLength;
>>> +       __u8 bDescriptorType;
>>> +       __u8 bDescriptorSubtype;
>>> +       __u8 bClockID;
>>> +       __u8 bNrInPins;
>>> +       __u8 baCSourceID[];
>>> +       /* bmControls and wCSelectorDescrStr omitted */
>>
>>
>> why is this omitted here and not below in the clock multiplier descriptor?
> 
> That's because in this descriptor ther is variable (unknown) field baCSourceID,
> so we can't define anythyng else after it as per C standard.
> In the clock multiplier descriptor below we don't have such issue.

ah, then maybe explain it, I wasn't sure if there was a technical reason 
or a filter you applied for some other reason.

> 
>>
>>> +} __attribute__((packed) > +
>>> +/* 4.5.2.14 CLOCK MULTIPLIER DESCRIPTOR */
>>> +struct uac3_clock_multiplier_descriptor {
>>> +       __u8 bLength;
>>> +       __u8 bDescriptorType;
>>> +       __u8 bDescriptorSubtype;
>>> +       __u8 bClockID;
>>> +       __u8 bCSourceID;
>>> +       __le32 bmControls;
>>> +       __le16 wCMultiplierDescrStr;
>>> +} __attribute__((packed));
>>
>>
>> [snip]
>>
>>> +
>>> +/* 4.5.2.15 POWER DOMAIN DESCRIPTOR */
>>> +struct uac3_power_domain_descriptor {
>>> +       __u8 bLength;
>>> +       __u8 bDescriptorType;
>>> +       __u8 bDescriptorSubtype;
>>> +       __u8 bPowerDomainID;
>>> +       __le16 waRecoveryTime1;
>>> +       __le16 waRecoveryTime2;
>>> +       __u8 bNrEntities;
>>> +       __u8 baEntityID[];
>>> +       /* wPDomainDescrStr omitted */
>>
>> why?
>>>
>>> +} __attribute__((packed));
>>> +
>>> +/* As above, but more useful for defining your own descriptors */
>>> +#define DECLARE_UAC3_POWER_DOMAIN_DESCRIPTOR(n)                        \
>>> +struct uac3_power_domain_descriptor_##n {                      \
>>> +       __u8 bLength;                                           \
>>> +       __u8 bDescriptorType;                                   \
>>> +       __u8 bDescriptorSubtype;                                \
>>> +       __u8 bPowerDomainID;                                    \
>>> +       __le16 waRecoveryTime1;                                 \
>>> +       __le16 waRecoveryTime2;                                 \
>>> +       __u8 bNrEntities;                                       \
>>> +       __u8 baEntityID[n];                                     \
>>> +       __le16 wPDomainDescrStr;                                        \
>>> +} __attribute__ ((packed))
>>> +
>>
>>
>> any specific reason why the descriptors are not added in linear order,
>> following the spec definition? all the descriptors below could be added
>> earlier.
> 
> That's because of way I wrote this header. These descritors are added
> in same order as in UAC2 header, then it was more easy to compare
> both headers in order to understand if we can reuse anything from UAC2.
> 
> If course I should reorganize it, but forgot to do :D

I don't know why this was modified, we should have kept the initial 
order in the spec when possible.

> 
>>
>>> +/* 4.5.2.1 INPUT TERMINAL DESCRIPTOR */
>>> +struct uac3_input_terminal_descriptor {
>>> +       __u8 bLength;
>>> +       __u8 bDescriptorType;
>>> +       __u8 bDescriptorSubtype;
>>> +       __u8 bTerminalID;
>>> +       __le16 wTerminalType;
>>> +       __u8 bAssocTerminal;
>>> +       __u8 bCSourceID;
>>> +       __le32 bmControls;
>>> +       __le16 wClusterDescrID;
>>> +       __le16 wExTerminalDescrID;
>>> +       __le16 wConnectorsDescrID;
>>> +       __le16 wTerminalDescrStr;
>>> +} __attribute__((packed));
>>> +
>>
>>
>>> +
>>> +/* 4.7.2 CLASS-SPECIFIC AS INTERFACE DESCRIPTOR */
>>> +struct uac3_as_header_descriptor {
>>> +       __u8 bLength;
>>> +       __u8 bDescriptorType;
>>> +       __u8 bDescriptorSubtype;
>>> +       __u8 bTerminalLink;
>>> +       __le32 bmControls;
>>> +       __le16 wClusterDescrID;
>>> +       __le64 bmFormats;
>>> +       __u8 bSubslotSize;
>>> +       __u8 bBitResolution;
>>> +       __le16 bmAuxProtocols;
>>> +       __u8 bControlSize;
>>> +} __attribute__((packed));
>>> +
>>> +#define UAC3_FORMAT_TYPE_I_RAW_DATA    (1 << 6)
>>
>> This seems to come from Table A1 in the formats document, is it inserted
>> here because of the relationship with the bmFormats? If yes, we should
>> probably add the full list from Table A1 (as done below for the other codes)
> 
> Yes, this comes from Frmts3.0. However, for bits 0..5 we reuse same
> values from UAC1 spec, thay are same for UAC2 as well.
> See UAC_FORMAT_TYPE_xxx

yes

> 
>>
>>> +
>>> +/* 4.8.1.2 CLASS-SPECIFIC AS ISOCHRONOUS AUDIO DATA ENDPOINT DESCRIPTOR
>>> */
>>> +struct uac3_iso_endpoint_descriptor {
>>> +       __u8 bLength;
>>> +       __u8 bDescriptorType;
>>> +       __u8 bDescriptorSubtype;
>>> +       __le32 bmControls;
>>> +       __u8 bLockDelayUnits;
>>> +       __le16 wLockDelay;
>>> +} __attribute__((packed));
>>> +
>>> +#define UAC3_CONTROL_PITCH             (3 << 0)
>>> +#define UAC3_CONTROL_DATA_OVERRUN      (3 << 2)
>>> +#define UAC3_CONTROL_DATA_UNDERRUN     (3 << 4)
>>
>>
>> these are really masks, no?
> 
> Yes, same as in UAC2, we can drop them as they are not used yet.

ok

> 
>>
>>> +
>>> +/* 6.1 INTERRUPT DATA MESSAGE */
>>> +#define UAC3_INTERRUPT_DATA_MSG_VENDOR (1 << 0)
>>> +#define UAC3_INTERRUPT_DATA_MSG_EP     (1 << 1)
>>
>>
>> this is the same as in UAC2
>>
>>> +
>>> +struct uac3_interrupt_data_msg {
>>> +       __u8 bInfo;
>>> +       __u8 bSourceType;
>>> +       __le16 wValue;
>>> +       __le16 wIndex;
>>> +} __attribute__((packed));
>>
>>
>> This seems identical to UAC2, the difference being the bSourceType with
>> additional values (was bAttribute).
> 
> Yes, this is correct. I made separate structure just to make code reading
> easier. Same is for UAC3_INTERRUPT_DATA_xxx above.

ok. maybe just mention that this is for readability

> 
>>
>>> +
>>> +/* A.2 AUDIO AUDIO FUNCTION SUBCLASS CODES */
>>> +#define UAC3_FUNCTION_SUBCLASS_UNDEFINED       0x00
>>> +#define UAC3_FUNCTION_SUBCLASS_FULL_ADC_3_0    0x01
>>> +#define UAC3_FUNCTION_SUBCLASS_GENERIC_IO      0x20
>>> +#define UAC3_FUNCTION_SUBCLASS_HEADPHONE       0x21
>>> +#define UAC3_FUNCTION_SUBCLASS_SPEAKER         0x22
>>> +#define UAC3_FUNCTION_SUBCLASS_MICROPHONE      0x23
>>> +#define UAC3_FUNCTION_SUBCLASS_HEADSET         0x24
>>> +#define UAC3_FUNCTION_SUBCLASS_HEADSET_ADAPTER 0x25
>>> +#define UAC3_FUNCTION_SUBCLASS_SPEAKERPHONE    0x26
>>
>>
>> mention that these are linked to BADD profiles.
> 
> OK, will do it
> 
>>
>>> +
>>> +/* A.7 AUDIO FUNCTION CATEGORY CODES */
>>> +#define UAC3_FUNCTION_SUBCLASS_UNDEFINED       0x00
>>> +#define UAC3_FUNCTION_DESKTOP_SPEAKER          0x01
>>> +#define UAC3_FUNCTION_HOME_THEATER             0x02
>>> +#define UAC3_FUNCTION_MICROPHONE               0x03
>>> +#define UAC3_FUNCTION_HEADSET                  0x04
>>> +#define UAC3_FUNCTION_TELEPHONE                        0x05
>>> +#define UAC3_FUNCTION_CONVERTER                        0x06
>>> +#define UAC3_FUNCTION_SOUND_RECORDER           0x07
>>> +#define UAC3_FUNCTION_IO_BOX                   0x08
>>> +#define UAC3_FUNCTION_MUSICAL_INSTRUMENT       0x09
>>> +#define UAC3_FUNCTION_PRO_AUDIO                        0x0a
>>> +#define UAC3_FUNCTION_AUDIO_VIDEO              0x0b
>>> +#define UAC3_FUNCTION_CONTROL_PANEL            0x0c
>>> +#define UAC3_FUNCTION_HEADPHONE                        0x0d
>>> +#define UAC3_FUNCTION_GENERIC_SPEAKER          0x0e
>>> +#define UAC3_FUNCTION_HEADSET_ADAPTER          0x0f
>>> +#define UAC3_FUNCTION_SPEAKERPHONE             0x10
>>> +#define UAC3_FUNCTION_OTHER                    0xff
>>> +
>>> +/* A.8 AUDIO CLASS-SPECIFIC DESCRIPTOR TYPES */
>>> +#define UAC3_CS_UNDEFINED              0x20
>>> +#define UAC3_CS_DEVICE                 0x21
>>> +#define UAC3_CS_CONFIGURATION          0x22
>>> +#define UAC3_CS_STRING                 0x23
>>> +#define UAC3_CS_INTERFACE              0x24
>>> +#define UAC3_CS_ENDPOINT               0x25
>>> +#define UAC3_CS_CLUSTER                        0x26
>>> +
>>> +/* A.10 CLUSTER DESCRIPTOR SEGMENT TYPES */
>>> +#define UAC3_SEGMENT_UNDEFINED         0x00
>>> +#define UAC3_CLUSTER_DESCRIPTION       0x01
>>> +#define UAC3_CLUSTER_VENDOR_DEFINED    0x1F
>>> +#define UAC3_CHANNEL_INFORMATION       0x20
>>> +#define UAC3_CHANNEL_AMBISONIC         0x21
>>> +#define UAC3_CHANNEL_DESCRIPTION       0x22
>>> +#define UAC3_CHANNEL_VENDOR_DEFINED    0xFE
>>> +#define UAC3_END_SEGMENT               0xFF
>>> +
>>> +/* A.11 CHANNEL PURPOSE DEFINITIONS */
>>> +#define UAC3_PURPOSE_UNDEFINED         0x00
>>> +#define UAC3_PURPOSE_GENERIC_AUDIO     0x01
>>> +#define UAC3_PURPOSE_VOICE             0x02
>>> +#define UAC3_PURPOSE_SPEECH            0x03
>>> +#define UAC3_PURPOSE_AMBIENT           0x04
>>> +#define UAC3_PURPOSE_REFERENCE         0x05
>>> +#define UAC3_PURPOSE_ULTRASONIC                0x06
>>> +#define UAC3_PURPOSE_VIBROKINETIC      0x07
>>> +#define UAC3_PURPOSE_NON_AUDIO         0xFF
>>> +
>>> +/* A.12 CHANNEL RELATIONSHIP DEFINITIONS */
>>> +/* FIXME: spec is missing these constants. Few found in
>>> BasicAudioDevice3.pdf */
>>
>>
>> yes, this is a known bug in the released UAC3 document, the values were
>> removed by accident. If this helps, here's what I have from the last release
>> candidate, this will hopefully be corrected soon.
> 
> My next step was to send a follow-up email to usb.org about this issue,
> but since you already have the answer, I'll include these values in the
> next version of patch, thanks.

It doesn't hurt to provide feedback to USB since it'll show the need for 
a spec update.

[snip]

>>
>>
>>> +#define UAC3_CH_RELATIONSHIP_UNDEFINED 0x00
>>> +#define UAC3_CH_MONO                   0x01
>>> +#define UAC3_CH_LEFT                   0x02
>>> +#define UAC3_CH_RIGHT                  0x03
>>> +#define UAC3_CH_FRONT_LEFT             0x80
>>> +#define UAC3_CH_FRONT_RIGHT            0x81
>>> +#define UAC3_CH_FRONT_CENTER           0x82
>>> +#define UAC3_CH_SURROUND_ARRAY_LEFT    0x89
>>> +#define UAC3_CH_SURROUND_ARRAY_RIGHT   0x8A
>>> +#define UAC3_CH_LOW_FREQUENCY_EFFECTS  0xB8
>>> +
>>> +/* A.15 AUDIO CLASS-SPECIFIC AC INTERFACE DESCRIPTOR SUBTYPES */
>>> +/* see audio.h for the rest, which is identical to v1 */
>>> +#define UAC3_EXTENDED_TERMINAL         0x04
>>> +#define UAC3_MIXER_UNIT                        0x05
>>> +#define UAC3_SELECTOR_UNIT             0x06
>>> +#define UAC3_FEATURE_UNIT              0x07
>>> +#define UAC3_EFFECT_UNIT               0x08
>>> +#define UAC3_PROCESSING_UNIT           0x09
>>> +#define UAC3_EXTENSION_UNIT            0x0a
>>> +#define UAC3_CLOCK_SOURCE              0x0b
>>> +#define UAC3_CLOCK_SELECTOR            0x0c
>>> +#define UAC3_CLOCK_MULTIPLIER          0x0d
>>> +#define UAC3_SAMPLE_RATE_CONVERTER     0x0e
>>> +#define UAC3_CONNECTORS                        0x0f
>>> +#define UAC3_POWER_DOMAIN              0x10
>>> +
>>> +/* A.22 AUDIO CLASS-SPECIFIC REQUEST CODES */
>>> +#define UAC3_CS_REQ_CUR                                0x01
>>> +#define UAC3_CS_REQ_RANGE                      0x02
>>> +#define UAC3_CS_REQ_MEM                                0x03
>>
>>
>> these first 3 are already in UAC2
>>
>>> +#define UAC3_CS_REQ_INTEN                      0x04
>>> +#define UAC3_CS_REQ_STRING                     0x05
>>> +#define UAC3_CS_REQ_HIGH_CAPABILITY_DESCRIPTOR 0x06
>>
>>
>> the last 3 are really UAC3-specific
> 
> Tha'ts correct. Do you want to say we can reuse UAC2 values?
> This brings an issue I described previously in this mailing list.
> It is related to UAC1/2/3 specs has some common values, sometimes
> use different values for same things and this makes understanding
> of sources quite complex if for example we will use UAC2_xx for
> UAC3 in some cases for common things but UAC3_xx for UAC3-specifc.
> 
> I see 3 solutions:
> 1) use UAC2_xxx for common things (hard to read/write the code, you
>      have to remember what is common and what is not, or leave
>      a comment near each such place saying it's OK to use it for UAC3
> 2) rename UAC2_xxx to something like UAC_V2V3_xxx, so it will be
>      self-explanatory

I vote for this option.

> 3) create UAC3_xxx for thouse things that are common with UAC2,
>      so no need to do either 1 or 2, but it can lead to code duplication
> 
> In this patch I did some mix of 2 and 3, but additional comments are
> welcome, as it variabl naming is the hardest part of programming :)
>>
>>> +
>>> +/* A.23.1 AUDIOCONTROL INTERFACE CONTROL SELECTORS */
>>> +#define UAC3_AC_CONTROL_UNDEFINED              0x00
>>> +#define UAC3_AC_ACTIVE_INTERFACE_CONTROL       0x01
>>> +#define UAC3_AC_POWER_DOMAIN_CONTROL           0x02
>>> +
>>> +#endif /* __LINUX_USB_AUDIO_V3_H */
>>> diff --git a/include/uapi/linux/usb/audio.h
>>> b/include/uapi/linux/usb/audio.h
>>> index a4680a5..66ec2ae 100644
>>> --- a/include/uapi/linux/usb/audio.h
>>> +++ b/include/uapi/linux/usb/audio.h
>>> @@ -26,6 +26,7 @@
>>>    /* bInterfaceProtocol values to denote the version of the standard used
>>> */
>>>    #define UAC_VERSION_1                 0x00
>>>    #define UAC_VERSION_2                 0x20
>>> +#define UAC_VERSION_3                  0x30
>>>      /* A.2 Audio Interface Subclass Codes */
>>>    #define USB_SUBCLASS_AUDIOCONTROL     0x01
>>> diff --git a/sound/usb/card.c b/sound/usb/card.c
>>> index 3dc36d9..df3f0ab 100644
>>> --- a/sound/usb/card.c
>>> +++ b/sound/usb/card.c
>>> @@ -7,6 +7,7 @@
>>>     *        Alan Cox (alan@lxorguk.ukuu.org.uk)
>>>     *        Thomas Sailer (sailer@ife.ee.ethz.ch)
>>>     *
>>> + *   Audio Class 3.0 support by Ruslan Bilovol <ruslan.bilovol@gmail.com>
>>
>>
>> does this mean new copyright?
> 
> That's a good qestion, I don't know the answer. Just wanted to leave
> a comment about work I did for USB audio.

Don't know the answer either.

> 
>>
>>>     *
>>>     *   This program is free software; you can redistribute it and/or
>>> modify
>>>     *   it under the terms of the GNU General Public License as published
>>> by
>>> @@ -44,6 +45,7 @@
>>>    #include <linux/mutex.h>
>>>    #include <linux/usb/audio.h>
>>>    #include <linux/usb/audio-v2.h>
>>> +#include <linux/usb/audio-v3.h>
>>>    #include <linux/module.h>
>>>      #include <sound/control.h>
>>> @@ -261,7 +263,8 @@ static int snd_usb_create_streams(struct snd_usb_audio
>>> *chip, int ctrlif)
>>>                  break;
>>>          }
>>>    -     case UAC_VERSION_2: {
>>> +       case UAC_VERSION_2:
>>> +       case UAC_VERSION_3: {
>>>                  struct usb_interface_assoc_descriptor *assoc =
>>>                          usb_ifnum_to_if(dev, ctrlif)->intf_assoc;
>>>    @@ -281,7 +284,7 @@ static int snd_usb_create_streams(struct
>>> snd_usb_audio *chip, int ctrlif)
>>>                  }
>>>                  if (!assoc) {
>>> -                       dev_err(&dev->dev, "Audio class v2 interfaces need
>>> an interface association\n");
>>> +                       dev_err(&dev->dev, "Audio class v2/v3 interfaces
>>> need an interface association\n");
>>
>>
>> yes, but they can have more than one, so how do you handle the selection
>> (see 3.3)?
> 
> Current implentation of USB audio driver is to create only one ALSA
> card per USB device, that works with UAC1/UAC2 and now with UAC3;
> so if USB device hase more that one interface association, only first
> will be used.
> 
> But if you mean configuration switching (between backward
> compatible UAC1/UAC2 and UAC3 configuration) that will not limit us,
> since after switching to another configuration  UAC1/UAC2 interfaces
> will be released and UAC3 will be probed, this is done by USB core.
> 
> As described above, currently I switching between configs manually.

My hope is that we can have a basic policy to select the configurations, 
as detailed above.

[snip]

> 
>>
>>
>>>    #include <sound/core.h>
>>>    #include <sound/pcm.h>
>>> @@ -39,11 +40,11 @@
>>>     * @dev: usb device
>>>     * @fp: audioformat record
>>>     * @format: the format tag (wFormatTag)
>>> - * @fmt: the format type descriptor
>>> + * @fmt: the format type descriptor (v1/v2) or AudioStreaming descriptor
>>> (v3)
>>>     */
>>>    static u64 parse_audio_format_i_type(struct snd_usb_audio *chip,
>>>                                       struct audioformat *fp,
>>> -                                    unsigned int format, void *_fmt)
>>> +                                    u64 format, void *_fmt)
>>>    {
>>>          int sample_width, sample_bytes;
>>>          u64 pcm_formats = 0;
>>> @@ -69,6 +70,17 @@ static u64 parse_audio_format_i_type(struct
>>> snd_usb_audio *chip,
>>>                  format <<= 1;
>>>                  break;
>>>          }
>>> +       case UAC_VERSION_3: {
>>> +               struct uac3_as_header_descriptor *as = _fmt;
>>> +               sample_width = as->bBitResolution;
>>> +               sample_bytes = as->bSubslotSize;
>>> +
>>> +               if (format & UAC3_FORMAT_TYPE_I_RAW_DATA)
>>> +                       pcm_formats |= SNDRV_PCM_FMTBIT_SPECIAL;
>>
>>
>> could this be expanded to add at least basic PCM (D0 set).
> 
> Basic PCM, and more (D0..D5) are handled, this is needed for special
> case of raw data, same as for UAC2, see commit 717bfb5 ("ALSA:
> snd-usb: handle raw data format of UAC2 devices")

I don't get this one, you are only handing D6 here? where is D0..D5 handled?

> 
>>
>>> +int snd_usb_parse_audio_format_v3(struct snd_usb_audio *chip,
>>> +                              struct audioformat *fp,
>>> +                              struct uac3_as_header_descriptor *as,
>>> +                              int stream)
>>> +{
>>> +       u64 format = le64_to_cpu(as->bmFormats);
>>> +       int err;
>>> +
>>> +       /* Type I format bits are D0..D6 */
>>> +       if (format & 0x7f)
>>> +               fp->fmt_type = UAC_FORMAT_TYPE_I;
>>> +       else
>>> +               fp->fmt_type = UAC_FORMAT_TYPE_III;
>>
>> maybe mention that this test only work because type IV is not supported,
>> otherwise this wouldn't quite right.
> 
> Yes, will do it.

ok

> 
>>
>>> +int snd_usb_parse_audio_format_v3(struct snd_usb_audio *chip,
>>> +                              struct audioformat *fp,
>>> +                              struct uac3_as_header_descriptor *as,
>>> +                              int stream);
>>>    #endif /*  __USBAUDIO_FORMAT_H */
>>> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
>>> index 9732edf..5b5dfe4 100644
>>> --- a/sound/usb/mixer.c
>>> +++ b/sound/usb/mixer.c
>>> @@ -51,6 +51,7 @@
>>>    #include <linux/usb.h>
>>>    #include <linux/usb/audio.h>
>>>    #include <linux/usb/audio-v2.h>
>>> +#include <linux/usb/audio-v3.h>
>>>      #include <sound/core.h>
>>>    #include <sound/control.h>
>>> @@ -189,7 +190,7 @@ static void *find_audio_control_unit(struct
>>> mixer_build *state,
>>>                                          USB_DT_CS_INTERFACE)) != NULL) {
>>>                  if (hdr->bLength >= 4 &&
>>>                      hdr->bDescriptorSubtype >= UAC_INPUT_TERMINAL &&
>>> -                   hdr->bDescriptorSubtype <= UAC2_SAMPLE_RATE_CONVERTER
>>> &&
>>> +                   hdr->bDescriptorSubtype <= UAC3_SAMPLE_RATE_CONVERTER
>>> &&
>>>                      hdr->bUnitID == unit)
>>
>>
>> this looks like a mistake, the definitions are
>>
>> #define UAC3_SAMPLE_RATE_CONVERTER      0x0e
>> #define UAC3_CONNECTORS                 0x0f
>> #define UAC3_POWER_DOMAIN               0x10
>>
>> You will miss the last two with this test.
> 
> That's because connectors descriptor uses High Capability descriptor for
> replresentation, thus it can't be compared here.
> Power Domain descriptor isn't used yet by this driver, so let's check for
> it once this functionality will be added.

Not sure I understand, the goal of this test is to check if the 
descriptors for the audio function look correct by looking at the range 
of their subType, which is also supported in High Capability descriptors?
> 
> Probably it makes sense to make this comparison UAC version depended
> to not break existing UAC1/2 functionality, I'll check it.
> 
>>
>>
>>> -               case UAC1_PROCESSING_UNIT:
>>> -               case UAC1_EXTENSION_UNIT:
>>> -               /* UAC2_PROCESSING_UNIT_V2 */
>>> -               /* UAC2_EFFECT_UNIT */
>>> -               case UAC2_EXTENSION_UNIT_V2: {
>>> -                       struct uac_processing_unit_descriptor *d = p1;
>>> -
>>> -                       if (state->mixer->protocol == UAC_VERSION_2 &&
>>> -                               hdr[2] == UAC2_EFFECT_UNIT) {
>>> -                               /* UAC2/UAC1 unit IDs overlap here in an
>>> -                                * uncompatible way. Ignore this unit for
>>> now.
>>> -                                */
>>> +
>>> +                               /* REVISIT: UAC3 IT doesn't have
>>> channels/cfg */
>>> +                               term->channels = 0;
>>> +                               term->chconfig = 0;
>>
>>
>> It does, but you need to get the information from the wClusterDescrID
> 
> Correct. Channels are used during mixer parsing which we don't support
> yet, chconfig is set in many places but nobody uses it, so we can remove it from
> usb_audio_term struct.

ok
diff mbox

Patch

diff --git a/include/linux/usb/audio-v2.h b/include/linux/usb/audio-v2.h
index fd73bc0..8ab3fb0 100644
--- a/include/linux/usb/audio-v2.h
+++ b/include/linux/usb/audio-v2.h
@@ -33,12 +33,12 @@ 
  *
  */
 
-static inline bool uac2_control_is_readable(u32 bmControls, u8 control)
+static inline bool uac_v2v3_control_is_readable(u32 bmControls, u8 control)
 {
 	return (bmControls >> (control * 2)) & 0x1;
 }
 
-static inline bool uac2_control_is_writeable(u32 bmControls, u8 control)
+static inline bool uac_v2v3_control_is_writeable(u32 bmControls, u8 control)
 {
 	return (bmControls >> (control * 2)) & 0x2;
 }
diff --git a/include/linux/usb/audio-v3.h b/include/linux/usb/audio-v3.h
new file mode 100644
index 0000000..cbbe51e
--- /dev/null
+++ b/include/linux/usb/audio-v3.h
@@ -0,0 +1,343 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2017 Ruslan Bilovol <ruslan.bilovol@gmail.com>
+ *
+ * This file holds USB constants and structures defined
+ * by the USB DEVICE CLASS DEFINITION FOR AUDIO DEVICES Release 3.0.
+ */
+
+#ifndef __LINUX_USB_AUDIO_V3_H
+#define __LINUX_USB_AUDIO_V3_H
+
+#include <linux/types.h>
+
+/*
+ * v1.0, v2.0 and v3.0 of this standard have many things in common. For the rest
+ * of the definitions, please refer to audio.h and audio-v2.h
+ */
+
+/* All High Capability descriptors have these 2 fields at the beginning */
+struct uac3_hc_descriptor_header {
+	__le16 wLength;
+	__u8 bDescriptorType;
+	__u8 bDescriptorSubtype;
+	__le16 wDescriptorID;
+} __attribute__ ((packed));
+
+/* 4.3.1 CLUSTER DESCRIPTOR HEADER */
+struct uac3_cluster_header_descriptor {
+	__le16 wLength;
+	__u8 bDescriptorType;
+	__u8 bDescriptorSubtype;
+	__le16 wDescriptorID;
+	__u8 bNrChannels;
+} __attribute__ ((packed));
+
+/* 4.3.2.1 SEGMENTS */
+struct uac3_cluster_segment_descriptor {
+	__le16 wLength;
+	__u8 bSegmentType;
+	/* __u8[0]; segment-specific data */
+} __attribute__ ((packed));
+
+/* 4.3.2.1.1 END SEGMENT */
+struct uac3_cluster_end_segment_descriptor {
+	__le16 wLength;
+	__u8 bSegmentType;		/* Constant END_SEGMENT */
+} __attribute__ ((packed));
+
+/* 4.3.2.1.3.1 INFORMATION SEGMENT */
+struct uac3_cluster_information_segment_descriptor {
+	__le16 wLength;
+	__u8 bSegmentType;
+	__u8 bChPurpose;
+	__u8 bChRelationship;
+	__u8 bChGroupID;
+} __attribute__ ((packed));
+
+/* 4.5.2 CLASS-SPECIFIC AC INTERFACE DESCRIPTOR */
+struct uac3_ac_header_descriptor {
+	__u8 bLength;			/* 10 */
+	__u8 bDescriptorType;		/* CS_INTERFACE descriptor type */
+	__u8 bDescriptorSubtype;	/* HEADER descriptor subtype */
+	__u8 bCategory;
+
+	/* includes Clock Source, Unit, Terminal, and Power Domain desc. */
+	__le16 wTotalLength;
+
+	__le32 bmControls;
+} __attribute__ ((packed));
+
+/* 4.5.2.12 CLOCK SOURCE DESCRIPTOR */
+struct uac3_clock_source_descriptor {
+	__u8 bLength;
+	__u8 bDescriptorType;
+	__u8 bDescriptorSubtype;
+	__u8 bClockID;
+	__u8 bmAttributes;
+	__le32 bmControls;
+	__u8 bReferenceTerminal;
+	__le16 wClockSourceStr;
+} __attribute__((packed));
+
+/* bmAttribute fields */
+#define UAC3_CLOCK_SOURCE_TYPE_EXT	0x0
+#define UAC3_CLOCK_SOURCE_TYPE_INT	0x1
+#define UAC3_CLOCK_SOURCE_ASYNC		(0 << 2)
+#define UAC3_CLOCK_SOURCE_SYNCED_TO_SOF	(1 << 1)
+
+/* 4.5.2.13 CLOCK SELECTOR DESCRIPTOR */
+struct uac3_clock_selector_descriptor {
+	__u8 bLength;
+	__u8 bDescriptorType;
+	__u8 bDescriptorSubtype;
+	__u8 bClockID;
+	__u8 bNrInPins;
+	__u8 baCSourceID[];
+	/* bmControls and wCSelectorDescrStr omitted */
+} __attribute__((packed));
+
+/* 4.5.2.14 CLOCK MULTIPLIER DESCRIPTOR */
+struct uac3_clock_multiplier_descriptor {
+	__u8 bLength;
+	__u8 bDescriptorType;
+	__u8 bDescriptorSubtype;
+	__u8 bClockID;
+	__u8 bCSourceID;
+	__le32 bmControls;
+	__le16 wCMultiplierDescrStr;
+} __attribute__((packed));
+
+/* 4.5.2.15 POWER DOMAIN DESCRIPTOR */
+struct uac3_power_domain_descriptor {
+	__u8 bLength;
+	__u8 bDescriptorType;
+	__u8 bDescriptorSubtype;
+	__u8 bPowerDomainID;
+	__le16 waRecoveryTime1;
+	__le16 waRecoveryTime2;
+	__u8 bNrEntities;
+	__u8 baEntityID[];
+	/* wPDomainDescrStr omitted */
+} __attribute__((packed));
+
+/* As above, but more useful for defining your own descriptors */
+#define DECLARE_UAC3_POWER_DOMAIN_DESCRIPTOR(n)			\
+struct uac3_power_domain_descriptor_##n {			\
+	__u8 bLength;						\
+	__u8 bDescriptorType;					\
+	__u8 bDescriptorSubtype;				\
+	__u8 bPowerDomainID;					\
+	__le16 waRecoveryTime1;					\
+	__le16 waRecoveryTime2;					\
+	__u8 bNrEntities;					\
+	__u8 baEntityID[n];					\
+	__le16 wPDomainDescrStr;					\
+} __attribute__ ((packed))
+
+/* 4.5.2.1 INPUT TERMINAL DESCRIPTOR */
+struct uac3_input_terminal_descriptor {
+	__u8 bLength;
+	__u8 bDescriptorType;
+	__u8 bDescriptorSubtype;
+	__u8 bTerminalID;
+	__le16 wTerminalType;
+	__u8 bAssocTerminal;
+	__u8 bCSourceID;
+	__le32 bmControls;
+	__le16 wClusterDescrID;
+	__le16 wExTerminalDescrID;
+	__le16 wConnectorsDescrID;
+	__le16 wTerminalDescrStr;
+} __attribute__((packed));
+
+/* 4.5.2.2 OUTPUT TERMINAL DESCRIPTOR */
+struct uac3_output_terminal_descriptor {
+	__u8 bLength;
+	__u8 bDescriptorType;
+	__u8 bDescriptorSubtype;
+	__u8 bTerminalID;
+	__le16 wTerminalType;
+	__u8 bAssocTerminal;
+	__u8 bSourceID;
+	__u8 bCSourceID;
+	__le32 bmControls;
+	__le16 wExTerminalDescrID;
+	__le16 wConnectorsDescrID;
+	__le16 wTerminalDescrStr;
+} __attribute__((packed));
+
+/* 4.5.2.7 FEATURE UNIT DESCRIPTOR */
+struct uac3_feature_unit_descriptor {
+	__u8 bLength;
+	__u8 bDescriptorType;
+	__u8 bDescriptorSubtype;
+	__u8 bUnitID;
+	__u8 bSourceID;
+	/* bmaControls is actually u32,
+	 * but u8 is needed for the hybrid parser */
+	__u8 bmaControls[0]; /* variable length */
+	/* wFeatureDescrStr omitted */
+} __attribute__((packed));
+
+#define UAC3_DT_FEATURE_UNIT_SIZE(ch)		(7 + ((ch) + 1) * 4)
+
+/* As above, but more useful for defining your own descriptors */
+#define DECLARE_UAC3_FEATURE_UNIT_DESCRIPTOR(ch)		\
+struct uac3_feature_unit_descriptor_##ch {			\
+	__u8 bLength;						\
+	__u8 bDescriptorType;					\
+	__u8 bDescriptorSubtype;				\
+	__u8 bUnitID;						\
+	__u8 bSourceID;						\
+	__u8 bControlSize;					\
+	__le32 bmaControls[ch + 1];				\
+	__le16 wFeatureDescrStr;				\
+} __attribute__ ((packed))
+
+/* 4.7.2 CLASS-SPECIFIC AS INTERFACE DESCRIPTOR */
+struct uac3_as_header_descriptor {
+	__u8 bLength;
+	__u8 bDescriptorType;
+	__u8 bDescriptorSubtype;
+	__u8 bTerminalLink;
+	__le32 bmControls;
+	__le16 wClusterDescrID;
+	__le64 bmFormats;
+	__u8 bSubslotSize;
+	__u8 bBitResolution;
+	__le16 bmAuxProtocols;
+	__u8 bControlSize;
+} __attribute__((packed));
+
+#define UAC3_FORMAT_TYPE_I_RAW_DATA	(1 << 6)
+
+/* 4.8.1.2 CLASS-SPECIFIC AS ISOCHRONOUS AUDIO DATA ENDPOINT DESCRIPTOR */
+struct uac3_iso_endpoint_descriptor {
+	__u8 bLength;
+	__u8 bDescriptorType;
+	__u8 bDescriptorSubtype;
+	__le32 bmControls;
+	__u8 bLockDelayUnits;
+	__le16 wLockDelay;
+} __attribute__((packed));
+
+#define UAC3_CONTROL_PITCH		(3 << 0)
+#define UAC3_CONTROL_DATA_OVERRUN	(3 << 2)
+#define UAC3_CONTROL_DATA_UNDERRUN	(3 << 4)
+
+/* 6.1 INTERRUPT DATA MESSAGE */
+#define UAC3_INTERRUPT_DATA_MSG_VENDOR	(1 << 0)
+#define UAC3_INTERRUPT_DATA_MSG_EP	(1 << 1)
+
+struct uac3_interrupt_data_msg {
+	__u8 bInfo;
+	__u8 bSourceType;
+	__le16 wValue;
+	__le16 wIndex;
+} __attribute__((packed));
+
+/* A.2 AUDIO AUDIO FUNCTION SUBCLASS CODES */
+#define UAC3_FUNCTION_SUBCLASS_UNDEFINED	0x00
+#define UAC3_FUNCTION_SUBCLASS_FULL_ADC_3_0	0x01
+#define UAC3_FUNCTION_SUBCLASS_GENERIC_IO	0x20
+#define UAC3_FUNCTION_SUBCLASS_HEADPHONE	0x21
+#define UAC3_FUNCTION_SUBCLASS_SPEAKER		0x22
+#define UAC3_FUNCTION_SUBCLASS_MICROPHONE	0x23
+#define UAC3_FUNCTION_SUBCLASS_HEADSET		0x24
+#define UAC3_FUNCTION_SUBCLASS_HEADSET_ADAPTER	0x25
+#define UAC3_FUNCTION_SUBCLASS_SPEAKERPHONE	0x26
+
+/* A.7 AUDIO FUNCTION CATEGORY CODES */
+#define UAC3_FUNCTION_SUBCLASS_UNDEFINED	0x00
+#define UAC3_FUNCTION_DESKTOP_SPEAKER		0x01
+#define UAC3_FUNCTION_HOME_THEATER		0x02
+#define UAC3_FUNCTION_MICROPHONE		0x03
+#define UAC3_FUNCTION_HEADSET			0x04
+#define UAC3_FUNCTION_TELEPHONE			0x05
+#define UAC3_FUNCTION_CONVERTER			0x06
+#define UAC3_FUNCTION_SOUND_RECORDER		0x07
+#define UAC3_FUNCTION_IO_BOX			0x08
+#define UAC3_FUNCTION_MUSICAL_INSTRUMENT	0x09
+#define UAC3_FUNCTION_PRO_AUDIO			0x0a
+#define UAC3_FUNCTION_AUDIO_VIDEO		0x0b
+#define UAC3_FUNCTION_CONTROL_PANEL		0x0c
+#define UAC3_FUNCTION_HEADPHONE			0x0d
+#define UAC3_FUNCTION_GENERIC_SPEAKER		0x0e
+#define UAC3_FUNCTION_HEADSET_ADAPTER		0x0f
+#define UAC3_FUNCTION_SPEAKERPHONE		0x10
+#define UAC3_FUNCTION_OTHER			0xff
+
+/* A.8 AUDIO CLASS-SPECIFIC DESCRIPTOR TYPES */
+#define UAC3_CS_UNDEFINED		0x20
+#define UAC3_CS_DEVICE			0x21
+#define UAC3_CS_CONFIGURATION		0x22
+#define UAC3_CS_STRING			0x23
+#define UAC3_CS_INTERFACE		0x24
+#define UAC3_CS_ENDPOINT		0x25
+#define UAC3_CS_CLUSTER			0x26
+
+/* A.10 CLUSTER DESCRIPTOR SEGMENT TYPES */
+#define UAC3_SEGMENT_UNDEFINED		0x00
+#define UAC3_CLUSTER_DESCRIPTION	0x01
+#define UAC3_CLUSTER_VENDOR_DEFINED	0x1F
+#define UAC3_CHANNEL_INFORMATION	0x20
+#define UAC3_CHANNEL_AMBISONIC		0x21
+#define UAC3_CHANNEL_DESCRIPTION	0x22
+#define UAC3_CHANNEL_VENDOR_DEFINED	0xFE
+#define UAC3_END_SEGMENT		0xFF
+
+/* A.11 CHANNEL PURPOSE DEFINITIONS */
+#define UAC3_PURPOSE_UNDEFINED		0x00
+#define UAC3_PURPOSE_GENERIC_AUDIO	0x01
+#define UAC3_PURPOSE_VOICE		0x02
+#define UAC3_PURPOSE_SPEECH		0x03
+#define UAC3_PURPOSE_AMBIENT		0x04
+#define UAC3_PURPOSE_REFERENCE		0x05
+#define UAC3_PURPOSE_ULTRASONIC		0x06
+#define UAC3_PURPOSE_VIBROKINETIC	0x07
+#define UAC3_PURPOSE_NON_AUDIO		0xFF
+
+/* A.12 CHANNEL RELATIONSHIP DEFINITIONS */
+/* FIXME: spec is missing these constants. Few found in BasicAudioDevice3.pdf */
+#define UAC3_CH_RELATIONSHIP_UNDEFINED	0x00
+#define UAC3_CH_MONO			0x01
+#define UAC3_CH_LEFT			0x02
+#define UAC3_CH_RIGHT			0x03
+#define UAC3_CH_FRONT_LEFT		0x80
+#define UAC3_CH_FRONT_RIGHT		0x81
+#define UAC3_CH_FRONT_CENTER		0x82
+#define UAC3_CH_SURROUND_ARRAY_LEFT	0x89
+#define UAC3_CH_SURROUND_ARRAY_RIGHT	0x8A
+#define UAC3_CH_LOW_FREQUENCY_EFFECTS	0xB8
+
+/* A.15 AUDIO CLASS-SPECIFIC AC INTERFACE DESCRIPTOR SUBTYPES */
+/* see audio.h for the rest, which is identical to v1 */
+#define UAC3_EXTENDED_TERMINAL		0x04
+#define UAC3_MIXER_UNIT			0x05
+#define UAC3_SELECTOR_UNIT		0x06
+#define UAC3_FEATURE_UNIT		0x07
+#define UAC3_EFFECT_UNIT		0x08
+#define UAC3_PROCESSING_UNIT		0x09
+#define UAC3_EXTENSION_UNIT		0x0a
+#define UAC3_CLOCK_SOURCE		0x0b
+#define UAC3_CLOCK_SELECTOR		0x0c
+#define UAC3_CLOCK_MULTIPLIER		0x0d
+#define UAC3_SAMPLE_RATE_CONVERTER	0x0e
+#define UAC3_CONNECTORS			0x0f
+#define UAC3_POWER_DOMAIN		0x10
+
+/* A.22 AUDIO CLASS-SPECIFIC REQUEST CODES */
+#define UAC3_CS_REQ_CUR				0x01
+#define UAC3_CS_REQ_RANGE			0x02
+#define UAC3_CS_REQ_MEM				0x03
+#define UAC3_CS_REQ_INTEN			0x04
+#define UAC3_CS_REQ_STRING			0x05
+#define UAC3_CS_REQ_HIGH_CAPABILITY_DESCRIPTOR	0x06
+
+/* A.23.1 AUDIOCONTROL INTERFACE CONTROL SELECTORS */
+#define UAC3_AC_CONTROL_UNDEFINED		0x00
+#define UAC3_AC_ACTIVE_INTERFACE_CONTROL	0x01
+#define UAC3_AC_POWER_DOMAIN_CONTROL		0x02
+
+#endif /* __LINUX_USB_AUDIO_V3_H */
diff --git a/include/uapi/linux/usb/audio.h b/include/uapi/linux/usb/audio.h
index a4680a5..66ec2ae 100644
--- a/include/uapi/linux/usb/audio.h
+++ b/include/uapi/linux/usb/audio.h
@@ -26,6 +26,7 @@ 
 /* bInterfaceProtocol values to denote the version of the standard used */
 #define UAC_VERSION_1			0x00
 #define UAC_VERSION_2			0x20
+#define UAC_VERSION_3			0x30
 
 /* A.2 Audio Interface Subclass Codes */
 #define USB_SUBCLASS_AUDIOCONTROL	0x01
diff --git a/sound/usb/card.c b/sound/usb/card.c
index 3dc36d9..df3f0ab 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -7,6 +7,7 @@ 
  *	    Alan Cox (alan@lxorguk.ukuu.org.uk)
  *	    Thomas Sailer (sailer@ife.ee.ethz.ch)
  *
+ *   Audio Class 3.0 support by Ruslan Bilovol <ruslan.bilovol@gmail.com>
  *
  *   This program is free software; you can redistribute it and/or modify
  *   it under the terms of the GNU General Public License as published by
@@ -44,6 +45,7 @@ 
 #include <linux/mutex.h>
 #include <linux/usb/audio.h>
 #include <linux/usb/audio-v2.h>
+#include <linux/usb/audio-v3.h>
 #include <linux/module.h>
 
 #include <sound/control.h>
@@ -261,7 +263,8 @@  static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif)
 		break;
 	}
 
-	case UAC_VERSION_2: {
+	case UAC_VERSION_2:
+	case UAC_VERSION_3: {
 		struct usb_interface_assoc_descriptor *assoc =
 			usb_ifnum_to_if(dev, ctrlif)->intf_assoc;
 
@@ -281,7 +284,7 @@  static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif)
 		}
 
 		if (!assoc) {
-			dev_err(&dev->dev, "Audio class v2 interfaces need an interface association\n");
+			dev_err(&dev->dev, "Audio class v2/v3 interfaces need an interface association\n");
 			return -EINVAL;
 		}
 
diff --git a/sound/usb/card.h b/sound/usb/card.h
index 111b0f0..5e2934f 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -21,7 +21,7 @@  struct audioformat {
 	unsigned char endpoint;		/* endpoint */
 	unsigned char ep_attr;		/* endpoint attributes */
 	unsigned char datainterval;	/* log_2 of data packet interval */
-	unsigned char protocol;		/* UAC_VERSION_1/2 */
+	unsigned char protocol;		/* UAC_VERSION_1/2/3 */
 	unsigned int maxpacksize;	/* max. packet size */
 	unsigned int rates;		/* rate bitmasks */
 	unsigned int rate_min, rate_max;	/* min/max rates */
diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index 26dd5f2..04361d7 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -23,6 +23,7 @@ 
 #include <linux/usb.h>
 #include <linux/usb/audio.h>
 #include <linux/usb/audio-v2.h>
+#include <linux/usb/audio-v3.h>
 
 #include <sound/core.h>
 #include <sound/info.h>
@@ -50,6 +51,22 @@ 
 	return NULL;
 }
 
+static struct uac3_clock_source_descriptor *
+	snd_usb_find_clock_source_v3(struct usb_host_interface *ctrl_iface,
+				  int clock_id)
+{
+	struct uac3_clock_source_descriptor *cs = NULL;
+
+	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
+					     ctrl_iface->extralen,
+					     cs, UAC3_CLOCK_SOURCE))) {
+		if (cs->bClockID == clock_id)
+			return cs;
+	}
+
+	return NULL;
+}
+
 static struct uac_clock_selector_descriptor *
 	snd_usb_find_clock_selector(struct usb_host_interface *ctrl_iface,
 				    int clock_id)
@@ -66,6 +83,22 @@ 
 	return NULL;
 }
 
+static struct uac3_clock_selector_descriptor *
+	snd_usb_find_clock_selector_v3(struct usb_host_interface *ctrl_iface,
+				    int clock_id)
+{
+	struct uac3_clock_selector_descriptor *cs = NULL;
+
+	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
+					     ctrl_iface->extralen,
+					     cs, UAC3_CLOCK_SELECTOR))) {
+		if (cs->bClockID == clock_id)
+			return cs;
+	}
+
+	return NULL;
+}
+
 static struct uac_clock_multiplier_descriptor *
 	snd_usb_find_clock_multiplier(struct usb_host_interface *ctrl_iface,
 				      int clock_id)
@@ -82,6 +115,22 @@ 
 	return NULL;
 }
 
+static struct uac3_clock_multiplier_descriptor *
+	snd_usb_find_clock_multiplier_v3(struct usb_host_interface *ctrl_iface,
+				      int clock_id)
+{
+	struct uac3_clock_multiplier_descriptor *cs = NULL;
+
+	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
+					     ctrl_iface->extralen,
+					     cs, UAC3_CLOCK_MULTIPLIER))) {
+		if (cs->bClockID == clock_id)
+			return cs;
+	}
+
+	return NULL;
+}
+
 static int uac_clock_selector_get_val(struct snd_usb_audio *chip, int selector_id)
 {
 	unsigned char buf;
@@ -135,19 +184,33 @@  static int uac_clock_selector_set_val(struct snd_usb_audio *chip, int selector_i
 	return ret;
 }
 
-static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, int source_id)
+static bool uac_clock_source_is_valid(struct snd_usb_audio *chip,
+				      int protocol,
+				      int source_id)
 {
 	int err;
 	unsigned char data;
 	struct usb_device *dev = chip->dev;
-	struct uac_clock_source_descriptor *cs_desc =
-		snd_usb_find_clock_source(chip->ctrl_intf, source_id);
-
-	if (!cs_desc)
-		return 0;
+	u32 bmControls;
+
+	if (protocol == UAC_VERSION_3) {
+		struct uac3_clock_source_descriptor *cs_desc =
+			snd_usb_find_clock_source_v3(chip->ctrl_intf, source_id);
+
+		if (!cs_desc)
+			return 0;
+		bmControls = le32_to_cpu(cs_desc->bmControls);
+	} else { /* UAC_VERSION_1/2 */
+		struct uac_clock_source_descriptor *cs_desc =
+			snd_usb_find_clock_source(chip->ctrl_intf, source_id);
+
+		if (!cs_desc)
+			return 0;
+		bmControls = cs_desc->bmControls;
+	}
 
 	/* If a clock source can't tell us whether it's valid, we assume it is */
-	if (!uac2_control_is_readable(cs_desc->bmControls,
+	if (!uac_v2v3_control_is_readable(bmControls,
 				      UAC2_CS_CONTROL_CLOCK_VALID - 1))
 		return 1;
 
@@ -167,9 +230,8 @@  static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, int source_id)
 	return !!data;
 }
 
-static int __uac_clock_find_source(struct snd_usb_audio *chip,
-				   int entity_id, unsigned long *visited,
-				   bool validate)
+static int __uac_clock_find_source(struct snd_usb_audio *chip, int entity_id,
+				   unsigned long *visited, bool validate)
 {
 	struct uac_clock_source_descriptor *source;
 	struct uac_clock_selector_descriptor *selector;
@@ -188,7 +250,8 @@  static int __uac_clock_find_source(struct snd_usb_audio *chip,
 	source = snd_usb_find_clock_source(chip->ctrl_intf, entity_id);
 	if (source) {
 		entity_id = source->bClockID;
-		if (validate && !uac_clock_source_is_valid(chip, entity_id)) {
+		if (validate && !uac_clock_source_is_valid(chip, UAC_VERSION_2,
+								entity_id)) {
 			usb_audio_err(chip,
 				"clock source %d is not valid, cannot use\n",
 				entity_id);
@@ -257,6 +320,97 @@  static int __uac_clock_find_source(struct snd_usb_audio *chip,
 	return -EINVAL;
 }
 
+static int __uac3_clock_find_source(struct snd_usb_audio *chip, int entity_id,
+				   unsigned long *visited, bool validate)
+{
+	struct uac3_clock_source_descriptor *source;
+	struct uac3_clock_selector_descriptor *selector;
+	struct uac3_clock_multiplier_descriptor *multiplier;
+
+	entity_id &= 0xff;
+
+	if (test_and_set_bit(entity_id, visited)) {
+		usb_audio_warn(chip,
+			 "%s(): recursive clock topology detected, id %d.\n",
+			 __func__, entity_id);
+		return -EINVAL;
+	}
+
+	/* first, see if the ID we're looking for is a clock source already */
+	source = snd_usb_find_clock_source_v3(chip->ctrl_intf, entity_id);
+	if (source) {
+		entity_id = source->bClockID;
+		if (validate && !uac_clock_source_is_valid(chip, UAC_VERSION_3,
+								entity_id)) {
+			usb_audio_err(chip,
+				"clock source %d is not valid, cannot use\n",
+				entity_id);
+			return -ENXIO;
+		}
+		return entity_id;
+	}
+
+	selector = snd_usb_find_clock_selector_v3(chip->ctrl_intf, entity_id);
+	if (selector) {
+		int ret, i, cur;
+
+		/* the entity ID we are looking for is a selector.
+		 * find out what it currently selects */
+		ret = uac_clock_selector_get_val(chip, selector->bClockID);
+		if (ret < 0)
+			return ret;
+
+		/* Selector values are one-based */
+
+		if (ret > selector->bNrInPins || ret < 1) {
+			usb_audio_err(chip,
+				"%s(): selector reported illegal value, id %d, ret %d\n",
+				__func__, selector->bClockID, ret);
+
+			return -EINVAL;
+		}
+
+		cur = ret;
+		ret = __uac3_clock_find_source(chip, selector->baCSourceID[ret - 1],
+					       visited, validate);
+		if (!validate || ret > 0 || !chip->autoclock)
+			return ret;
+
+		/* The current clock source is invalid, try others. */
+		for (i = 1; i <= selector->bNrInPins; i++) {
+			int err;
+
+			if (i == cur)
+				continue;
+
+			ret = __uac3_clock_find_source(chip, selector->baCSourceID[i - 1],
+				visited, true);
+			if (ret < 0)
+				continue;
+
+			err = uac_clock_selector_set_val(chip, entity_id, i);
+			if (err < 0)
+				continue;
+
+			usb_audio_info(chip,
+				 "found and selected valid clock source %d\n",
+				 ret);
+			return ret;
+		}
+
+		return -ENXIO;
+	}
+
+	/* FIXME: multipliers only act as pass-thru element for now */
+	multiplier = snd_usb_find_clock_multiplier_v3(chip->ctrl_intf,
+						      entity_id);
+	if (multiplier)
+		return __uac3_clock_find_source(chip, multiplier->bCSourceID,
+						visited, validate);
+
+	return -EINVAL;
+}
+
 /*
  * For all kinds of sample rate settings and other device queries,
  * the clock source (end-leaf) must be used. However, clock selectors,
@@ -268,12 +422,22 @@  static int __uac_clock_find_source(struct snd_usb_audio *chip,
  *
  * Returns the clock source UnitID (>=0) on success, or an error.
  */
-int snd_usb_clock_find_source(struct snd_usb_audio *chip, int entity_id,
-			      bool validate)
+int snd_usb_clock_find_source(struct snd_usb_audio *chip, int protocol,
+			      int entity_id, bool validate)
 {
 	DECLARE_BITMAP(visited, 256);
 	memset(visited, 0, sizeof(visited));
-	return __uac_clock_find_source(chip, entity_id, visited, validate);
+
+	switch (protocol) {
+	case UAC_VERSION_2:
+		return __uac_clock_find_source(chip, entity_id, visited,
+					       validate);
+	case UAC_VERSION_3:
+		return __uac3_clock_find_source(chip, entity_id, visited,
+					       validate);
+	default:
+		return -EINVAL;
+	}
 }
 
 static int set_sample_rate_v1(struct snd_usb_audio *chip, int iface,
@@ -332,7 +496,7 @@  static int set_sample_rate_v1(struct snd_usb_audio *chip, int iface,
 	return 0;
 }
 
-static int get_sample_rate_v2(struct snd_usb_audio *chip, int iface,
+static int get_sample_rate_v2v3(struct snd_usb_audio *chip, int iface,
 			      int altsetting, int clock)
 {
 	struct usb_device *dev = chip->dev;
@@ -345,7 +509,7 @@  static int get_sample_rate_v2(struct snd_usb_audio *chip, int iface,
 			      snd_usb_ctrl_intf(chip) | (clock << 8),
 			      &data, sizeof(data));
 	if (err < 0) {
-		dev_warn(&dev->dev, "%d:%d: cannot get freq (v2): err %d\n",
+		dev_warn(&dev->dev, "%d:%d: cannot get freq (v2/v3): err %d\n",
 			 iface, altsetting, err);
 		return 0;
 	}
@@ -353,7 +517,7 @@  static int get_sample_rate_v2(struct snd_usb_audio *chip, int iface,
 	return le32_to_cpu(data);
 }
 
-static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
+static int set_sample_rate_v2v3(struct snd_usb_audio *chip, int iface,
 			      struct usb_host_interface *alts,
 			      struct audioformat *fmt, int rate)
 {
@@ -362,18 +526,30 @@  static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
 	int err, cur_rate, prev_rate;
 	int clock;
 	bool writeable;
-	struct uac_clock_source_descriptor *cs_desc;
+	u32 bmControls;
 
-	clock = snd_usb_clock_find_source(chip, fmt->clock, true);
+	clock = snd_usb_clock_find_source(chip, fmt->protocol,
+					  fmt->clock, true);
 	if (clock < 0)
 		return clock;
 
-	prev_rate = get_sample_rate_v2(chip, iface, fmt->altsetting, clock);
+	prev_rate = get_sample_rate_v2v3(chip, iface, fmt->altsetting, clock);
 	if (prev_rate == rate)
 		return 0;
 
-	cs_desc = snd_usb_find_clock_source(chip->ctrl_intf, clock);
-	writeable = uac2_control_is_writeable(cs_desc->bmControls, UAC2_CS_CONTROL_SAM_FREQ - 1);
+	if (fmt->protocol == UAC_VERSION_3) {
+		struct uac3_clock_source_descriptor *cs_desc;
+
+		cs_desc = snd_usb_find_clock_source_v3(chip->ctrl_intf, clock);
+		bmControls = le32_to_cpu(cs_desc->bmControls);
+	} else {
+		struct uac_clock_source_descriptor *cs_desc;
+
+		cs_desc = snd_usb_find_clock_source(chip->ctrl_intf, clock);
+		bmControls = cs_desc->bmControls;
+	}
+
+	writeable = uac_v2v3_control_is_writeable(bmControls, UAC2_CS_CONTROL_SAM_FREQ - 1);
 	if (writeable) {
 		data = cpu_to_le32(rate);
 		err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC2_CS_CUR,
@@ -383,12 +559,13 @@  static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
 				      &data, sizeof(data));
 		if (err < 0) {
 			usb_audio_err(chip,
-				"%d:%d: cannot set freq %d (v2): err %d\n",
+				"%d:%d: cannot set freq %d (v2/v3): err %d\n",
 				iface, fmt->altsetting, rate, err);
 			return err;
 		}
 
-		cur_rate = get_sample_rate_v2(chip, iface, fmt->altsetting, clock);
+		cur_rate = get_sample_rate_v2v3(chip, iface,
+						fmt->altsetting, clock);
 	} else {
 		cur_rate = prev_rate;
 	}
@@ -427,7 +604,8 @@  int snd_usb_init_sample_rate(struct snd_usb_audio *chip, int iface,
 		return set_sample_rate_v1(chip, iface, alts, fmt, rate);
 
 	case UAC_VERSION_2:
-		return set_sample_rate_v2(chip, iface, alts, fmt, rate);
+	case UAC_VERSION_3:
+		return set_sample_rate_v2v3(chip, iface, alts, fmt, rate);
 	}
 }
 
diff --git a/sound/usb/clock.h b/sound/usb/clock.h
index d592e4a..e465ff5 100644
--- a/sound/usb/clock.h
+++ b/sound/usb/clock.h
@@ -5,7 +5,7 @@  int snd_usb_init_sample_rate(struct snd_usb_audio *chip, int iface,
 			     struct usb_host_interface *alts,
 			     struct audioformat *fmt, int rate);
 
-int snd_usb_clock_find_source(struct snd_usb_audio *chip, int entity_id,
-			     bool validate);
+int snd_usb_clock_find_source(struct snd_usb_audio *chip, int protocol,
+			     int entity_id, bool validate);
 
 #endif /* __USBAUDIO_CLOCK_H */
diff --git a/sound/usb/format.c b/sound/usb/format.c
index 2c44386..6f85892 100644
--- a/sound/usb/format.c
+++ b/sound/usb/format.c
@@ -20,6 +20,7 @@ 
 #include <linux/usb.h>
 #include <linux/usb/audio.h>
 #include <linux/usb/audio-v2.h>
+#include <linux/usb/audio-v3.h>
 
 #include <sound/core.h>
 #include <sound/pcm.h>
@@ -39,11 +40,11 @@ 
  * @dev: usb device
  * @fp: audioformat record
  * @format: the format tag (wFormatTag)
- * @fmt: the format type descriptor
+ * @fmt: the format type descriptor (v1/v2) or AudioStreaming descriptor (v3)
  */
 static u64 parse_audio_format_i_type(struct snd_usb_audio *chip,
 				     struct audioformat *fp,
-				     unsigned int format, void *_fmt)
+				     u64 format, void *_fmt)
 {
 	int sample_width, sample_bytes;
 	u64 pcm_formats = 0;
@@ -69,6 +70,17 @@  static u64 parse_audio_format_i_type(struct snd_usb_audio *chip,
 		format <<= 1;
 		break;
 	}
+	case UAC_VERSION_3: {
+		struct uac3_as_header_descriptor *as = _fmt;
+		sample_width = as->bBitResolution;
+		sample_bytes = as->bSubslotSize;
+
+		if (format & UAC3_FORMAT_TYPE_I_RAW_DATA)
+			pcm_formats |= SNDRV_PCM_FMTBIT_SPECIAL;
+
+		format <<= 1;
+		break;
+	}
 	}
 
 	if ((pcm_formats == 0) &&
@@ -137,7 +149,7 @@  static u64 parse_audio_format_i_type(struct snd_usb_audio *chip,
 	}
 	if (format & ~0x3f) {
 		usb_audio_info(chip,
-			 "%u:%d : unsupported format bits %#x\n",
+			 "%u:%d : unsupported format bits %#llx\n",
 			 fp->iface, fp->altsetting, format);
 	}
 
@@ -281,15 +293,16 @@  static int parse_uac2_sample_rate_range(struct snd_usb_audio *chip,
 
 /*
  * parse the format descriptor and stores the possible sample rates
- * on the audioformat table (audio class v2).
+ * on the audioformat table (audio class v2 and v3).
  */
-static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
+static int parse_audio_format_rates_v2v3(struct snd_usb_audio *chip,
 				       struct audioformat *fp)
 {
 	struct usb_device *dev = chip->dev;
 	unsigned char tmp[2], *data;
 	int nr_triplets, data_size, ret = 0;
-	int clock = snd_usb_clock_find_source(chip, fp->clock, false);
+	int clock = snd_usb_clock_find_source(chip, fp->protocol,
+					      fp->clock, false);
 
 	if (clock < 0) {
 		dev_err(&dev->dev,
@@ -368,13 +381,30 @@  static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
  * parse the format type I and III descriptors
  */
 static int parse_audio_format_i(struct snd_usb_audio *chip,
-				struct audioformat *fp, unsigned int format,
-				struct uac_format_type_i_continuous_descriptor *fmt)
+				struct audioformat *fp, u64 format,
+				void *_fmt)
 {
 	snd_pcm_format_t pcm_format;
+	unsigned int fmt_type;
 	int ret;
 
-	if (fmt->bFormatType == UAC_FORMAT_TYPE_III) {
+	switch (fp->protocol) {
+	default:
+	case UAC_VERSION_1:
+	case UAC_VERSION_2: {
+		struct uac_format_type_i_continuous_descriptor *fmt = _fmt;
+
+		fmt_type = fmt->bFormatType;
+		break;
+	}
+	case UAC_VERSION_3: {
+		/* fp->fmt_type is already set in this case */
+		fmt_type = fp->fmt_type;
+		break;
+	}
+	}
+
+	if (fmt_type == UAC_FORMAT_TYPE_III) {
 		/* FIXME: the format type is really IECxxx
 		 *        but we give normal PCM format to get the existing
 		 *        apps working...
@@ -393,7 +423,7 @@  static int parse_audio_format_i(struct snd_usb_audio *chip,
 		}
 		fp->formats = pcm_format_to_bits(pcm_format);
 	} else {
-		fp->formats = parse_audio_format_i_type(chip, fp, format, fmt);
+		fp->formats = parse_audio_format_i_type(chip, fp, format, _fmt);
 		if (!fp->formats)
 			return -EINVAL;
 	}
@@ -405,15 +435,20 @@  static int parse_audio_format_i(struct snd_usb_audio *chip,
 	 */
 	switch (fp->protocol) {
 	default:
-	case UAC_VERSION_1:
+	case UAC_VERSION_1: {
+		struct uac_format_type_i_continuous_descriptor *fmt = _fmt;
+
 		fp->channels = fmt->bNrChannels;
 		ret = parse_audio_format_rates_v1(chip, fp, (unsigned char *) fmt, 7);
 		break;
+	}
 	case UAC_VERSION_2:
+	case UAC_VERSION_3: {
 		/* fp->channels is already set in this case */
-		ret = parse_audio_format_rates_v2(chip, fp);
+		ret = parse_audio_format_rates_v2v3(chip, fp);
 		break;
 	}
+	}
 
 	if (fp->channels < 1) {
 		usb_audio_err(chip,
@@ -430,7 +465,7 @@  static int parse_audio_format_i(struct snd_usb_audio *chip,
  */
 static int parse_audio_format_ii(struct snd_usb_audio *chip,
 				 struct audioformat *fp,
-				 int format, void *_fmt)
+				 u64 format, void *_fmt)
 {
 	int brate, framesize, ret;
 
@@ -445,7 +480,7 @@  static int parse_audio_format_ii(struct snd_usb_audio *chip,
 		break;
 	default:
 		usb_audio_info(chip,
-			 "%u:%d : unknown format tag %#x is detected.  processed as MPEG.\n",
+			 "%u:%d : unknown format tag %#llx is detected.  processed as MPEG.\n",
 			 fp->iface, fp->altsetting, format);
 		fp->formats = SNDRV_PCM_FMTBIT_MPEG;
 		break;
@@ -470,7 +505,7 @@  static int parse_audio_format_ii(struct snd_usb_audio *chip,
 		framesize = le16_to_cpu(fmt->wSamplesPerFrame);
 		usb_audio_info(chip, "found format II with max.bitrate = %d, frame size=%d\n", brate, framesize);
 		fp->frame_size = framesize;
-		ret = parse_audio_format_rates_v2(chip, fp);
+		ret = parse_audio_format_rates_v2v3(chip, fp);
 		break;
 	}
 	}
@@ -479,7 +514,7 @@  static int parse_audio_format_ii(struct snd_usb_audio *chip,
 }
 
 int snd_usb_parse_audio_format(struct snd_usb_audio *chip,
-			       struct audioformat *fp, unsigned int format,
+			       struct audioformat *fp, u64 format,
 			       struct uac_format_type_i_continuous_descriptor *fmt,
 			       int stream)
 {
@@ -520,3 +555,23 @@  int snd_usb_parse_audio_format(struct snd_usb_audio *chip,
 	return 0;
 }
 
+int snd_usb_parse_audio_format_v3(struct snd_usb_audio *chip,
+			       struct audioformat *fp,
+			       struct uac3_as_header_descriptor *as,
+			       int stream)
+{
+	u64 format = le64_to_cpu(as->bmFormats);
+	int err;
+
+	/* Type I format bits are D0..D6 */
+	if (format & 0x7f)
+		fp->fmt_type = UAC_FORMAT_TYPE_I;
+	else
+		fp->fmt_type = UAC_FORMAT_TYPE_III;
+
+	err = parse_audio_format_i(chip, fp, format, as);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
diff --git a/sound/usb/format.h b/sound/usb/format.h
index 4b8a011..46c9de7 100644
--- a/sound/usb/format.h
+++ b/sound/usb/format.h
@@ -2,8 +2,12 @@ 
 #define __USBAUDIO_FORMAT_H
 
 int snd_usb_parse_audio_format(struct snd_usb_audio *chip,
-			       struct audioformat *fp, unsigned int format,
+			       struct audioformat *fp, u64 format,
 			       struct uac_format_type_i_continuous_descriptor *fmt,
 			       int stream);
 
+int snd_usb_parse_audio_format_v3(struct snd_usb_audio *chip,
+			       struct audioformat *fp,
+			       struct uac3_as_header_descriptor *as,
+			       int stream);
 #endif /*  __USBAUDIO_FORMAT_H */
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 9732edf..5b5dfe4 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -51,6 +51,7 @@ 
 #include <linux/usb.h>
 #include <linux/usb/audio.h>
 #include <linux/usb/audio-v2.h>
+#include <linux/usb/audio-v3.h>
 
 #include <sound/core.h>
 #include <sound/control.h>
@@ -189,7 +190,7 @@  static void *find_audio_control_unit(struct mixer_build *state,
 					USB_DT_CS_INTERFACE)) != NULL) {
 		if (hdr->bLength >= 4 &&
 		    hdr->bDescriptorSubtype >= UAC_INPUT_TERMINAL &&
-		    hdr->bDescriptorSubtype <= UAC2_SAMPLE_RATE_CONVERTER &&
+		    hdr->bDescriptorSubtype <= UAC3_SAMPLE_RATE_CONVERTER &&
 		    hdr->bUnitID == unit)
 			return hdr;
 	}
@@ -460,9 +461,10 @@  int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
 
 	validx += cval->idx_off;
 
+
 	if (cval->head.mixer->protocol == UAC_VERSION_1) {
 		val_len = cval->val_type >= USB_MIXER_S16 ? 2 : 1;
-	} else { /* UAC_VERSION_2 */
+	} else { /* UAC_VERSION_2/3 */
 		val_len = uac2_ctl_value_size(cval->val_type);
 
 		/* FIXME */
@@ -711,6 +713,7 @@  static int get_term_name(struct mixer_build *state, struct usb_audio_term *iterm
 static int check_input_term(struct mixer_build *state, int id,
 			    struct usb_audio_term *term)
 {
+	int protocol = state->mixer->protocol;
 	int err;
 	void *p1;
 
@@ -718,16 +721,104 @@  static int check_input_term(struct mixer_build *state, int id,
 	while ((p1 = find_audio_control_unit(state, id)) != NULL) {
 		unsigned char *hdr = p1;
 		term->id = id;
-		switch (hdr[2]) {
-		case UAC_INPUT_TERMINAL:
-			if (state->mixer->protocol == UAC_VERSION_1) {
-				struct uac_input_terminal_descriptor *d = p1;
-				term->type = le16_to_cpu(d->wTerminalType);
-				term->channels = d->bNrChannels;
-				term->chconfig = le16_to_cpu(d->wChannelConfig);
-				term->name = d->iTerminal;
-			} else { /* UAC_VERSION_2 */
-				struct uac2_input_terminal_descriptor *d = p1;
+
+		if ((protocol == UAC_VERSION_1) || (protocol == UAC_VERSION_2)) {
+			switch (hdr[2]) {
+			case UAC_INPUT_TERMINAL:
+				if (protocol == UAC_VERSION_1) {
+					struct uac_input_terminal_descriptor *d = p1;
+
+					term->type = le16_to_cpu(d->wTerminalType);
+					term->channels = d->bNrChannels;
+					term->chconfig = le16_to_cpu(d->wChannelConfig);
+					term->name = d->iTerminal;
+				} else { /* UAC_VERSION_2 */
+					struct uac2_input_terminal_descriptor *d = p1;
+
+					/* call recursively to verify that the
+					 * referenced clock entity is valid */
+					err = check_input_term(state, d->bCSourceID, term);
+					if (err < 0)
+						return err;
+
+					/* save input term properties after recursion,
+					 * to ensure they are not overriden by the
+					 * recursion calls */
+					term->id = id;
+					term->type = le16_to_cpu(d->wTerminalType);
+					term->channels = d->bNrChannels;
+					term->chconfig = le32_to_cpu(d->bmChannelConfig);
+					term->name = d->iTerminal;
+				}
+				return 0;
+			case UAC_FEATURE_UNIT: {
+				/* the header is the same for v1 and v2 */
+				struct uac_feature_unit_descriptor *d = p1;
+
+				id = d->bSourceID;
+				break; /* continue to parse */
+			}
+			case UAC_MIXER_UNIT: {
+				struct uac_mixer_unit_descriptor *d = p1;
+
+				term->type = d->bDescriptorSubtype << 16; /* virtual type */
+				term->channels = uac_mixer_unit_bNrChannels(d);
+				term->chconfig = uac_mixer_unit_wChannelConfig(d, protocol);
+				term->name = uac_mixer_unit_iMixer(d);
+				return 0;
+			}
+			case UAC_SELECTOR_UNIT:
+			case UAC2_CLOCK_SELECTOR: {
+				struct uac_selector_unit_descriptor *d = p1;
+				/* call recursively to retrieve the channel info */
+				err = check_input_term(state, d->baSourceID[0], term);
+				if (err < 0)
+					return err;
+				term->type = d->bDescriptorSubtype << 16; /* virtual type */
+				term->id = id;
+				term->name = uac_selector_unit_iSelector(d);
+				return 0;
+			}
+			case UAC1_PROCESSING_UNIT:
+			case UAC1_EXTENSION_UNIT:
+			/* UAC2_PROCESSING_UNIT_V2 */
+			/* UAC2_EFFECT_UNIT */
+			case UAC2_EXTENSION_UNIT_V2: {
+				struct uac_processing_unit_descriptor *d = p1;
+
+				if (protocol == UAC_VERSION_2 &&
+					hdr[2] == UAC2_EFFECT_UNIT) {
+					/* UAC2/UAC1 unit IDs overlap here in an
+					 * uncompatible way. Ignore this unit for now.
+					 */
+					return 0;
+				}
+
+				if (d->bNrInPins) {
+					id = d->baSourceID[0];
+					break; /* continue to parse */
+				}
+				term->type = d->bDescriptorSubtype << 16; /* virtual type */
+				term->channels = uac_processing_unit_bNrChannels(d);
+				term->chconfig = uac_processing_unit_wChannelConfig(d, protocol);
+				term->name = uac_processing_unit_iProcessing(d, protocol);
+				return 0;
+			}
+			case UAC2_CLOCK_SOURCE: {
+				struct uac_clock_source_descriptor *d = p1;
+
+				term->type = d->bDescriptorSubtype << 16; /* virtual type */
+				term->id = id;
+				term->name = d->iClockSource;
+				return 0;
+			}
+			default:
+				return -ENODEV;
+			}
+		} else { /* UAC_VERSION_3 */
+			switch (hdr[2]) {
+			case UAC_INPUT_TERMINAL: {
+				struct uac3_input_terminal_descriptor *d = p1;
 
 				/* call recursively to verify that the
 				 * referenced clock entity is valid */
@@ -740,71 +831,31 @@  static int check_input_term(struct mixer_build *state, int id,
 				 * recursion calls */
 				term->id = id;
 				term->type = le16_to_cpu(d->wTerminalType);
-				term->channels = d->bNrChannels;
-				term->chconfig = le32_to_cpu(d->bmChannelConfig);
-				term->name = d->iTerminal;
-			}
-			return 0;
-		case UAC_FEATURE_UNIT: {
-			/* the header is the same for v1 and v2 */
-			struct uac_feature_unit_descriptor *d = p1;
-			id = d->bSourceID;
-			break; /* continue to parse */
-		}
-		case UAC_MIXER_UNIT: {
-			struct uac_mixer_unit_descriptor *d = p1;
-			term->type = d->bDescriptorSubtype << 16; /* virtual type */
-			term->channels = uac_mixer_unit_bNrChannels(d);
-			term->chconfig = uac_mixer_unit_wChannelConfig(d, state->mixer->protocol);
-			term->name = uac_mixer_unit_iMixer(d);
-			return 0;
-		}
-		case UAC_SELECTOR_UNIT:
-		case UAC2_CLOCK_SELECTOR: {
-			struct uac_selector_unit_descriptor *d = p1;
-			/* call recursively to retrieve the channel info */
-			err = check_input_term(state, d->baSourceID[0], term);
-			if (err < 0)
-				return err;
-			term->type = d->bDescriptorSubtype << 16; /* virtual type */
-			term->id = id;
-			term->name = uac_selector_unit_iSelector(d);
-			return 0;
-		}
-		case UAC1_PROCESSING_UNIT:
-		case UAC1_EXTENSION_UNIT:
-		/* UAC2_PROCESSING_UNIT_V2 */
-		/* UAC2_EFFECT_UNIT */
-		case UAC2_EXTENSION_UNIT_V2: {
-			struct uac_processing_unit_descriptor *d = p1;
-
-			if (state->mixer->protocol == UAC_VERSION_2 &&
-				hdr[2] == UAC2_EFFECT_UNIT) {
-				/* UAC2/UAC1 unit IDs overlap here in an
-				 * uncompatible way. Ignore this unit for now.
-				 */
+
+				/* REVISIT: UAC3 IT doesn't have channels/cfg */
+				term->channels = 0;
+				term->chconfig = 0;
+
+				term->name = le16_to_cpu(d->wTerminalDescrStr);
 				return 0;
 			}
+			case UAC3_FEATURE_UNIT: {
+				struct uac3_feature_unit_descriptor *d = p1;
 
-			if (d->bNrInPins) {
-				id = d->baSourceID[0];
+				id = d->bSourceID;
 				break; /* continue to parse */
 			}
-			term->type = d->bDescriptorSubtype << 16; /* virtual type */
-			term->channels = uac_processing_unit_bNrChannels(d);
-			term->chconfig = uac_processing_unit_wChannelConfig(d, state->mixer->protocol);
-			term->name = uac_processing_unit_iProcessing(d, state->mixer->protocol);
-			return 0;
-		}
-		case UAC2_CLOCK_SOURCE: {
-			struct uac_clock_source_descriptor *d = p1;
-			term->type = d->bDescriptorSubtype << 16; /* virtual type */
-			term->id = id;
-			term->name = d->iClockSource;
-			return 0;
-		}
-		default:
-			return -ENODEV;
+			case UAC3_CLOCK_SOURCE: {
+				struct uac3_clock_source_descriptor *d = p1;
+
+				term->type = d->bDescriptorSubtype << 16; /* virtual type */
+				term->id = id;
+				term->name = le16_to_cpu(d->wClockSourceStr);
+				return 0;
+			}
+			default:
+				return -ENODEV;
+			}
 		}
 	}
 	return -ENODEV;
@@ -1411,7 +1462,7 @@  static int parse_clock_source_unit(struct mixer_build *state, int unitid,
 	 * The only property of this unit we are interested in is the
 	 * clock source validity. If that isn't readable, just bail out.
 	 */
-	if (!uac2_control_is_readable(hdr->bmControls,
+	if (!uac_v2v3_control_is_readable(hdr->bmControls,
 				      ilog2(UAC2_CS_CONTROL_CLOCK_VALID)))
 		return 0;
 
@@ -1427,7 +1478,7 @@  static int parse_clock_source_unit(struct mixer_build *state, int unitid,
 	cval->val_type = USB_MIXER_BOOLEAN;
 	cval->control = UAC2_CS_CONTROL_CLOCK_VALID;
 
-	if (uac2_control_is_writeable(hdr->bmControls,
+	if (uac_v2v3_control_is_writeable(hdr->bmControls,
 				      ilog2(UAC2_CS_CONTROL_CLOCK_VALID)))
 		kctl = snd_ctl_new1(&usb_feature_unit_ctl, cval);
 	else {
@@ -1484,8 +1535,9 @@  static int parse_audio_feature_unit(struct mixer_build *state, int unitid,
 				      unitid);
 			return -EINVAL;
 		}
-	} else {
+	} else if (state->mixer->protocol == UAC_VERSION_2) {
 		struct uac2_feature_unit_descriptor *ftr = _ftr;
+
 		csize = 4;
 		channels = (hdr->bLength - 6) / 4 - 1;
 		bmaControls = ftr->bmaControls;
@@ -1495,6 +1547,18 @@  static int parse_audio_feature_unit(struct mixer_build *state, int unitid,
 				      unitid);
 			return -EINVAL;
 		}
+	} else { /* UAC_VERSION_3 */
+		struct uac3_feature_unit_descriptor *ftr = _ftr;
+
+		csize = 4;
+		channels = (ftr->bLength - 7) / 4 - 1;
+		bmaControls = ftr->bmaControls;
+		if (hdr->bLength < 6 + csize) {
+			usb_audio_err(state->chip,
+				      "unit %u: invalid UAC3_FEATURE_UNIT descriptor\n",
+				      unitid);
+			return -EINVAL;
+		}
 	}
 
 	/* parse the source unit */
@@ -1553,7 +1617,7 @@  static int parse_audio_feature_unit(struct mixer_build *state, int unitid,
 				build_feature_ctl(state, _ftr, 0, i, &iterm,
 						  unitid, 0);
 		}
-	} else { /* UAC_VERSION_2 */
+	} else { /* UAC_VERSION_2/3 */
 		for (i = 0; i < ARRAY_SIZE(audio_feature_info); i++) {
 			unsigned int ch_bits = 0;
 			unsigned int ch_read_only = 0;
@@ -1563,9 +1627,9 @@  static int parse_audio_feature_unit(struct mixer_build *state, int unitid,
 
 				mask = snd_usb_combine_bytes(bmaControls +
 							     csize * (j+1), csize);
-				if (uac2_control_is_readable(mask, i)) {
+				if (uac_v2v3_control_is_readable(mask, i)) {
 					ch_bits |= (1 << j);
-					if (!uac2_control_is_writeable(mask, i))
+					if (!uac_v2v3_control_is_writeable(mask, i))
 						ch_read_only |= (1 << j);
 				}
 			}
@@ -1586,9 +1650,9 @@  static int parse_audio_feature_unit(struct mixer_build *state, int unitid,
 			if (ch_bits & 1)
 				build_feature_ctl(state, _ftr, ch_bits, i,
 						  &iterm, unitid, ch_read_only);
-			if (uac2_control_is_readable(master_bits, i))
+			if (uac_v2v3_control_is_readable(master_bits, i))
 				build_feature_ctl(state, _ftr, 0, i, &iterm, unitid,
-						  !uac2_control_is_writeable(master_bits, i));
+						  !uac_v2v3_control_is_writeable(master_bits, i));
 		}
 	}
 
@@ -2189,6 +2253,7 @@  static int parse_audio_selector_unit(struct mixer_build *state, int unitid,
 static int parse_audio_unit(struct mixer_build *state, int unitid)
 {
 	unsigned char *p1;
+	int protocol = state->mixer->protocol;
 
 	if (test_and_set_bit(unitid, state->unitbitmap))
 		return 0; /* the unit already visited */
@@ -2199,36 +2264,61 @@  static int parse_audio_unit(struct mixer_build *state, int unitid)
 		return -EINVAL;
 	}
 
-	switch (p1[2]) {
-	case UAC_INPUT_TERMINAL:
-		return 0; /* NOP */
-	case UAC_MIXER_UNIT:
-		return parse_audio_mixer_unit(state, unitid, p1);
-	case UAC2_CLOCK_SOURCE:
-		return parse_clock_source_unit(state, unitid, p1);
-	case UAC_SELECTOR_UNIT:
-	case UAC2_CLOCK_SELECTOR:
-		return parse_audio_selector_unit(state, unitid, p1);
-	case UAC_FEATURE_UNIT:
-		return parse_audio_feature_unit(state, unitid, p1);
-	case UAC1_PROCESSING_UNIT:
-	/*   UAC2_EFFECT_UNIT has the same value */
-		if (state->mixer->protocol == UAC_VERSION_1)
-			return parse_audio_processing_unit(state, unitid, p1);
-		else
-			return 0; /* FIXME - effect units not implemented yet */
-	case UAC1_EXTENSION_UNIT:
-	/*   UAC2_PROCESSING_UNIT_V2 has the same value */
-		if (state->mixer->protocol == UAC_VERSION_1)
+	if ((protocol == UAC_VERSION_1) || (protocol == UAC_VERSION_2)) {
+		switch (p1[2]) {
+		case UAC_INPUT_TERMINAL:
+			return 0; /* NOP */
+		case UAC_MIXER_UNIT:
+			return parse_audio_mixer_unit(state, unitid, p1);
+		case UAC2_CLOCK_SOURCE:
+			return parse_clock_source_unit(state, unitid, p1);
+		case UAC_SELECTOR_UNIT:
+		case UAC2_CLOCK_SELECTOR:
+			return parse_audio_selector_unit(state, unitid, p1);
+		case UAC_FEATURE_UNIT:
+			return parse_audio_feature_unit(state, unitid, p1);
+		case UAC1_PROCESSING_UNIT:
+		/*   UAC2_EFFECT_UNIT has the same value */
+			if (protocol == UAC_VERSION_1)
+				return parse_audio_processing_unit(state, unitid, p1);
+			else
+				return 0; /* FIXME - effect units not implemented yet */
+		case UAC1_EXTENSION_UNIT:
+		/*   UAC2_PROCESSING_UNIT_V2 has the same value */
+			if (protocol == UAC_VERSION_1)
+				return parse_audio_extension_unit(state, unitid, p1);
+			else /* UAC_VERSION_2 */
+				return parse_audio_processing_unit(state, unitid, p1);
+		case UAC2_EXTENSION_UNIT_V2:
 			return parse_audio_extension_unit(state, unitid, p1);
-		else /* UAC_VERSION_2 */
+		default:
+			usb_audio_err(state->chip,
+				"unit %u: unexpected type 0x%02x\n", unitid, p1[2]);
+			return -EINVAL;
+		}
+	} else { /* UAC_VERSION_3 */
+		switch (p1[2]) {
+		case UAC_INPUT_TERMINAL:
+			return 0; /* NOP */
+		case UAC3_MIXER_UNIT:
+			return parse_audio_mixer_unit(state, unitid, p1);
+		case UAC3_CLOCK_SOURCE:
+			return parse_clock_source_unit(state, unitid, p1);
+		case UAC3_CLOCK_SELECTOR:
+			return parse_audio_selector_unit(state, unitid, p1);
+		case UAC3_FEATURE_UNIT:
+			return parse_audio_feature_unit(state, unitid, p1);
+		case UAC3_EFFECT_UNIT:
+			return 0; /* FIXME - effect units not implemented yet */
+		case UAC3_PROCESSING_UNIT:
 			return parse_audio_processing_unit(state, unitid, p1);
-	case UAC2_EXTENSION_UNIT_V2:
-		return parse_audio_extension_unit(state, unitid, p1);
-	default:
-		usb_audio_err(state->chip,
-			"unit %u: unexpected type 0x%02x\n", unitid, p1[2]);
-		return -EINVAL;
+		case UAC3_EXTENSION_UNIT:
+			return parse_audio_extension_unit(state, unitid, p1);
+		default:
+			usb_audio_err(state->chip,
+				"unit %u: unexpected type 0x%02x\n", unitid, p1[2]);
+			return -EINVAL;
+		}
 	}
 }
 
@@ -2296,7 +2386,7 @@  static int snd_usb_mixer_controls(struct usb_mixer_interface *mixer)
 			err = parse_audio_unit(&state, desc->bSourceID);
 			if (err < 0 && err != -EINVAL)
 				return err;
-		} else { /* UAC_VERSION_2 */
+		} else if (mixer->protocol == UAC_VERSION_2) {
 			struct uac2_output_terminal_descriptor *desc = p;
 
 			if (desc->bLength < sizeof(*desc))
@@ -2317,6 +2407,27 @@  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;
+		} else {  /* UAC_VERSION_3 */
+			struct uac3_output_terminal_descriptor *desc = p;
+
+			if (desc->bLength < sizeof(*desc))
+				continue; /* invalid descriptor? */
+			/* mark terminal ID as visited */
+			set_bit(desc->bTerminalID, state.unitbitmap);
+			state.oterm.id = desc->bTerminalID;
+			state.oterm.type = le16_to_cpu(desc->wTerminalType);
+			state.oterm.name = le16_to_cpu(desc->wTerminalDescrStr);
+			err = parse_audio_unit(&state, desc->bSourceID);
+			if (err < 0 && err != -EINVAL)
+				return err;
+
+			/*
+			 * For UAC3, use the same approach to also add the
+			 * clock selectors
+			 */
+			err = parse_audio_unit(&state, desc->bCSourceID);
+			if (err < 0 && err != -EINVAL)
+				return err;
 		}
 	}
 
@@ -2558,6 +2669,9 @@  int snd_usb_create_mixer(struct snd_usb_audio *chip, int ctrlif,
 	case UAC_VERSION_2:
 		mixer->protocol = UAC_VERSION_2;
 		break;
+	case UAC_VERSION_3:
+		mixer->protocol = UAC_VERSION_3;
+		break;
 	}
 
 	if ((err = snd_usb_mixer_controls(mixer)) < 0 ||
diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index d1776e5..738ee87 100644
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -20,6 +20,7 @@ 
 #include <linux/usb.h>
 #include <linux/usb/audio.h>
 #include <linux/usb/audio-v2.h>
+#include <linux/usb/audio-v3.h>
 
 #include <sound/core.h>
 #include <sound/pcm.h>
@@ -311,6 +312,86 @@  static struct snd_pcm_chmap_elem *convert_chmap(int channels, unsigned int bits,
 	return chmap;
 }
 
+/* UAC3 device stores channels information in Cluster Descriptors */
+static struct
+snd_pcm_chmap_elem *convert_chmap_v3(struct uac3_cluster_header_descriptor
+								*cluster)
+{
+	unsigned int channels = cluster->bNrChannels;
+	struct snd_pcm_chmap_elem *chmap;
+	void *p = cluster;
+	int len, c;
+
+	if (channels > ARRAY_SIZE(chmap->map))
+		return NULL;
+
+	chmap = kzalloc(sizeof(*chmap), GFP_KERNEL);
+	if (!chmap)
+		return NULL;
+
+	len = le16_to_cpu(cluster->wLength);
+	c = 0;
+	p += sizeof(struct uac3_cluster_header_descriptor);
+
+	while (((p - (void *)cluster) < len) && (c < channels)) {
+		struct uac3_cluster_segment_descriptor *cs_desc = p;
+		u16 cs_len;
+		u8 cs_type;
+
+		cs_len = le16_to_cpu(cs_desc->wLength);
+		cs_type = cs_desc->bSegmentType;
+
+		if (cs_type == UAC3_CHANNEL_INFORMATION) {
+			struct uac3_cluster_information_segment_descriptor *is = p;
+			unsigned char map;
+
+			/*
+			 * FIXME: this conversion is simplified, because
+			 * asound.h doesn't have UAC3 values yet
+			 */
+			switch (is->bChPurpose) {
+			case UAC3_CH_MONO:
+				map = SNDRV_CHMAP_MONO;
+				break;
+			case UAC3_CH_LEFT:
+			case UAC3_CH_FRONT_LEFT:
+			case UAC3_CH_SURROUND_ARRAY_LEFT:
+				map = SNDRV_CHMAP_FL;
+				break;
+			case UAC3_CH_RIGHT:
+			case UAC3_CH_FRONT_RIGHT:
+			case UAC3_CH_SURROUND_ARRAY_RIGHT:
+				map = SNDRV_CHMAP_FR;
+				break;
+			case UAC3_CH_FRONT_CENTER:
+				map = SNDRV_CHMAP_FC;
+				break;
+			case UAC3_CH_LOW_FREQUENCY_EFFECTS:
+				map = SNDRV_CHMAP_LFE;
+				break;
+			case UAC3_CH_RELATIONSHIP_UNDEFINED:
+			default:
+				map = SNDRV_CHMAP_UNKNOWN;
+				break;
+			}
+
+			chmap->map[c++] = map;
+		}
+		p += cs_len;
+
+	}
+
+	if (channels < c)
+		pr_err("%s: channel number mismatch\n", __func__);
+
+	chmap->channels = channels;
+
+	for (; c < channels; c++)
+		chmap->map[c] = SNDRV_CHMAP_UNKNOWN;
+
+	return chmap;
+}
+
 /*
  * add this endpoint to the chip instance.
  * if a stream with the same endpoint already exists, append to it.
@@ -461,10 +542,11 @@  static int parse_uac_endpoint_attributes(struct snd_usb_audio *chip,
 	return NULL;
 }
 
-static struct uac2_output_terminal_descriptor *
-	snd_usb_find_output_terminal_descriptor(struct usb_host_interface *ctrl_iface,
-						int terminal_id)
+static void *
+snd_usb_find_output_terminal_descriptor(struct usb_host_interface *ctrl_iface,
+					int terminal_id)
 {
+	/* OK to use with both UAC2 and UAC3 */
 	struct uac2_output_terminal_descriptor *term = NULL;
 
 	while ((term = snd_usb_find_csint_desc(ctrl_iface->extra,
@@ -484,10 +566,12 @@  int snd_usb_parse_audio_interface(struct snd_usb_audio *chip, int iface_no)
 	struct usb_host_interface *alts;
 	struct usb_interface_descriptor *altsd;
 	int i, altno, err, stream;
-	unsigned int format = 0, num_channels = 0;
+	u64 format = 0;
+	unsigned int num_channels = 0;
 	struct audioformat *fp = NULL;
 	int num, protocol, clock = 0;
 	struct uac_format_type_i_continuous_descriptor *fmt;
+	struct snd_pcm_chmap_elem *chmap_v3 = NULL;
 	unsigned int chconfig;
 
 	dev = chip->dev;
@@ -624,38 +708,158 @@  int snd_usb_parse_audio_interface(struct snd_usb_audio *chip, int iface_no)
 				iface_no, altno, as->bTerminalLink);
 			continue;
 		}
-		}
 
-		/* get format type */
-		fmt = snd_usb_find_csint_desc(alts->extra, alts->extralen, NULL, UAC_FORMAT_TYPE);
-		if (!fmt) {
+		case UAC_VERSION_3: {
+			struct uac3_input_terminal_descriptor *input_term;
+			struct uac3_output_terminal_descriptor *output_term;
+			struct uac3_as_header_descriptor *as;
+			struct uac3_cluster_header_descriptor *cluster;
+			struct uac3_hc_descriptor_header hc_header;
+			u16 cluster_id, wLength;
+
+			as = snd_usb_find_csint_desc(alts->extra,
+							alts->extralen,
+							NULL, UAC_AS_GENERAL);
+
+			if (!as) {
+				dev_err(&dev->dev,
+					"%u:%d : UAC_AS_GENERAL descriptor not found\n",
+					iface_no, altno);
+				continue;
+			}
+
+			if (as->bLength < sizeof(*as)) {
+				dev_err(&dev->dev,
+					"%u:%d : invalid UAC_AS_GENERAL desc\n",
+					iface_no, altno);
+				continue;
+			}
+
+			cluster_id = le16_to_cpu(as->wClusterDescrID);
+			if (!cluster_id) {
+				dev_err(&dev->dev,
+					"%u:%d : no cluster descriptor\n",
+					iface_no, altno);
+				continue;
+			}
+
+			/*
+			 * Get number of channels and channel map through
+			 * High Capability Cluster Descriptor
+			 *
+			 * First step: get High Capability header and
+			 * read size of Cluster Descriptor
+			 */
+			err = snd_usb_ctl_msg(chip->dev,
+					usb_rcvctrlpipe(chip->dev, 0),
+					UAC3_CS_REQ_HIGH_CAPABILITY_DESCRIPTOR,
+					USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
+					cluster_id,
+					snd_usb_ctrl_intf(chip),
+					&hc_header, sizeof(hc_header));
+			if (err < 0)
+				return err;
+			else if (err != sizeof(hc_header)) {
+				dev_err(&dev->dev,
+					"%u:%d : can't get High Capability descriptor\n",
+					iface_no, altno);
+				return -EIO;
+			}
+
+			/*
+			 * Second step: allocate needed amount of memory
+			 * and request Cluster Descriptor
+			 */
+			wLength = le16_to_cpu(hc_header.wLength);
+			cluster = kzalloc(wLength, GFP_KERNEL);
+			if (!cluster)
+				return -ENOMEM;
+			err = snd_usb_ctl_msg(chip->dev,
+					usb_rcvctrlpipe(chip->dev, 0),
+					UAC3_CS_REQ_HIGH_CAPABILITY_DESCRIPTOR,
+					USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
+					cluster_id,
+					snd_usb_ctrl_intf(chip),
+					cluster, wLength);
+			if (err < 0) {
+				kfree(cluster);
+				return err;
+			} else if (err != wLength) {
+				dev_err(&dev->dev,
+					"%u:%d : can't get Cluster Descriptor\n",
+					iface_no, altno);
+				kfree(cluster);
+				return -EIO;
+			}
+
+			num_channels = cluster->bNrChannels;
+			chmap_v3 = convert_chmap_v3(cluster);
+
+			kfree(cluster);
+
+			format = le64_to_cpu(as->bmFormats);
+
+			/* lookup the terminal associated to this interface
+			 * to extract the clock */
+			input_term = snd_usb_find_input_terminal_descriptor(
+							chip->ctrl_intf,
+							as->bTerminalLink);
+
+			if (input_term) {
+				clock = input_term->bCSourceID;
+				break;
+			}
+
+			output_term = snd_usb_find_output_terminal_descriptor(chip->ctrl_intf,
+									      as->bTerminalLink);
+			if (output_term) {
+				clock = output_term->bCSourceID;
+				break;
+			}
+
 			dev_err(&dev->dev,
-				"%u:%d : no UAC_FORMAT_TYPE desc\n",
-				iface_no, altno);
+				"%u:%d : bogus bTerminalLink %d\n",
+				iface_no, altno, as->bTerminalLink);
 			continue;
 		}
-		if (((protocol == UAC_VERSION_1) && (fmt->bLength < 8)) ||
-		    ((protocol == UAC_VERSION_2) && (fmt->bLength < 6))) {
-			dev_err(&dev->dev,
-				"%u:%d : invalid UAC_FORMAT_TYPE desc\n",
-				iface_no, altno);
-			continue;
 		}
 
-		/*
-		 * Blue Microphones workaround: The last altsetting is identical
-		 * with the previous one, except for a larger packet size, but
-		 * is actually a mislabeled two-channel setting; ignore it.
-		 */
-		if (fmt->bNrChannels == 1 &&
-		    fmt->bSubframeSize == 2 &&
-		    altno == 2 && num == 3 &&
-		    fp && fp->altsetting == 1 && fp->channels == 1 &&
-		    fp->formats == SNDRV_PCM_FMTBIT_S16_LE &&
-		    protocol == UAC_VERSION_1 &&
-		    le16_to_cpu(get_endpoint(alts, 0)->wMaxPacketSize) ==
+		if (protocol == UAC_VERSION_1 || protocol == UAC_VERSION_2) {
+			/* get format type */
+			fmt = snd_usb_find_csint_desc(alts->extra,
+						      alts->extralen,
+						      NULL, UAC_FORMAT_TYPE);
+			if (!fmt) {
+				dev_err(&dev->dev,
+					"%u:%d : no UAC_FORMAT_TYPE desc\n",
+					iface_no, altno);
+				continue;
+			}
+			if (((protocol == UAC_VERSION_1) && (fmt->bLength < 8))
+					|| ((protocol == UAC_VERSION_2) &&
+							(fmt->bLength < 6))) {
+				dev_err(&dev->dev,
+					"%u:%d : invalid UAC_FORMAT_TYPE desc\n",
+					iface_no, altno);
+				continue;
+			}
+
+			/*
+			 * Blue Microphones workaround: The last altsetting is
+			 * identical with the previous one, except for a larger
+			 * packet size, but is actually a mislabeled two-channel
+			 * setting; ignore it.
+			 */
+			if (fmt->bNrChannels == 1 &&
+			    fmt->bSubframeSize == 2 &&
+			    altno == 2 && num == 3 &&
+			    fp && fp->altsetting == 1 && fp->channels == 1 &&
+			    fp->formats == SNDRV_PCM_FMTBIT_S16_LE &&
+			    protocol == UAC_VERSION_1 &&
+			    le16_to_cpu(get_endpoint(alts, 0)->wMaxPacketSize) ==
 							fp->maxpacksize * 2)
-			continue;
+				continue;
+		}
 
 		fp = kzalloc(sizeof(*fp), GFP_KERNEL);
 		if (!fp)
@@ -709,17 +913,39 @@  int snd_usb_parse_audio_interface(struct snd_usb_audio *chip, int iface_no)
 		}
 
 		/* ok, let's parse further... */
-		if (snd_usb_parse_audio_format(chip, fp, format, fmt, stream) < 0) {
-			kfree(fp->rate_table);
-			kfree(fp);
-			fp = NULL;
-			continue;
+		if (protocol == UAC_VERSION_1 || protocol == UAC_VERSION_2) {
+			if (snd_usb_parse_audio_format(chip, fp, format,
+							fmt, stream) < 0) {
+				kfree(fp->rate_table);
+				kfree(fp);
+				fp = NULL;
+				continue;
+			}
+		} else {
+			struct uac3_as_header_descriptor *as;
+
+			as = snd_usb_find_csint_desc(alts->extra,
+						     alts->extralen,
+						     NULL, UAC_AS_GENERAL);
+
+			if (snd_usb_parse_audio_format_v3(chip, fp, as,
+								stream) < 0) {
+				kfree(fp->rate_table);
+				kfree(fp);
+				fp = NULL;
+				continue;
+			}
 		}
 
 		/* Create chmap */
 		if (fp->channels != num_channels)
 			chconfig = 0;
-		fp->chmap = convert_chmap(fp->channels, chconfig, protocol);
+
+		if (protocol == UAC_VERSION_3)
+			fp->chmap = chmap_v3;
+		else
+			fp->chmap = convert_chmap(fp->channels, chconfig,
+						  protocol);
 
 		dev_dbg(&dev->dev, "%u:%d: add audio endpoint %#x\n", iface_no, altno, fp->endpoint);
 		err = snd_usb_add_audio_stream(chip, stream, fp);