ALSA: usb-audio: Fix parameter block size for UAC2 control requests
diff mbox

Message ID 1439463886-5369-1-git-send-email-julian@jusst.de
State New
Headers show

Commit Message

Julian Scheel Aug. 13, 2015, 11:04 a.m. UTC
USB Audio Class version 2.0 supports three different parameter block sizes for
CUR requests, which are 1 byte (5.2.3.1 Layout 1 Parameter Block), 2 bytes
(5.2.3.2 Layout 2 Parameter Block) and 4 bytes (5.2.3.3 Layout 3 Parameter
Block). Use the correct size according to the specific control as it was
already done for UACv1. The allocated block size for control requests is
increased to support the 4 byte worst case.

Signed-off-by: Julian Scheel <julian@jusst.de>
---
 sound/usb/mixer.c | 83 +++++++++++++++++++++++++++++++++++++++++--------------
 sound/usb/mixer.h |  2 ++
 2 files changed, 65 insertions(+), 20 deletions(-)

Comments

Takashi Iwai Aug. 14, 2015, 1:17 p.m. UTC | #1
On Thu, 13 Aug 2015 13:04:46 +0200,
Julian Scheel wrote:
> 
> USB Audio Class version 2.0 supports three different parameter block sizes for
> CUR requests, which are 1 byte (5.2.3.1 Layout 1 Parameter Block), 2 bytes
> (5.2.3.2 Layout 2 Parameter Block) and 4 bytes (5.2.3.3 Layout 3 Parameter
> Block). Use the correct size according to the specific control as it was
> already done for UACv1. The allocated block size for control requests is
> increased to support the 4 byte worst case.
> 
> Signed-off-by: Julian Scheel <julian@jusst.de>
> ---
>  sound/usb/mixer.c | 83 +++++++++++++++++++++++++++++++++++++++++--------------
>  sound/usb/mixer.h |  2 ++
>  2 files changed, 65 insertions(+), 20 deletions(-)
> 
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index 4a49944..675725b 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -233,6 +233,14 @@ static int convert_signed_value(struct usb_mixer_elem_info *cval, int val)
>  		if (val >= 0x8000)
>  			val -= 0x10000;
>  		break;
> +	case USB_MIXER_U32:
> +		val &= 0xffffffff;
> +		break;
> +	case USB_MIXER_S32:
> +		val &= 0xffffffff;
> +		if (val >= 0x80000000)
> +			val -= 0x100000000;
> +		break;

All these are superfluous, as int on Linux is always 32bit.


>  	}
>  	return val;
>  }
> @@ -253,6 +261,9 @@ static int convert_bytes_value(struct usb_mixer_elem_info *cval, int val)
>  	case USB_MIXER_S16:
>  	case USB_MIXER_U16:
>  		return val & 0xffff;
> +	case USB_MIXER_S32:
> +	case USB_MIXER_U32:
> +		return val & 0xffffffff;

Ditto.


>  	}
>  	return 0; /* not reached */
>  }
> @@ -328,14 +339,26 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request,
>  			    int validx, int *value_ret)
>  {
>  	struct snd_usb_audio *chip = cval->head.mixer->chip;
> -	unsigned char buf[2 + 3 * sizeof(__u16)]; /* enough space for one range */
> +	unsigned char buf[4 + 3 * sizeof(__u32)]; /* enough space for one range */
>  	unsigned char *val;
>  	int idx = 0, ret, size;
>  	__u8 bRequest;
>  
>  	if (request == UAC_GET_CUR) {
>  		bRequest = UAC2_CS_CUR;
> -		size = sizeof(__u16);
> +		switch (cval->val_type) {
> +			case USB_MIXER_S32:
> +			case USB_MIXER_U32:
> +				size = 4;
> +				break;
> +			case USB_MIXER_S16:
> +			case USB_MIXER_U16:
> +				size = 2;
> +				break;
> +			default:
> +				size = 1;
> +				break;

Please split this into a small helper function,
e.g. uac2_ctl_value_size().  The same stuff is used in below, too.

> +		}
>  	} else {
>  		bRequest = UAC2_CS_RANGE;
>  		size = sizeof(buf);
> @@ -446,7 +469,7 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
>  				int request, int validx, int value_set)
>  {
>  	struct snd_usb_audio *chip = cval->head.mixer->chip;
> -	unsigned char buf[2];
> +	unsigned char buf[4];
>  	int idx = 0, val_len, err, timeout = 10;
>  
>  	validx += cval->idx_off;
> @@ -454,8 +477,19 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
>  	if (cval->head.mixer->protocol == UAC_VERSION_1) {
>  		val_len = cval->val_type >= USB_MIXER_S16 ? 2 : 1;
>  	} else { /* UAC_VERSION_2 */
> -		/* audio class v2 controls are always 2 bytes in size */
> -		val_len = sizeof(__u16);
> +		switch (cval->val_type) {
> +			case USB_MIXER_S32:
> +			case USB_MIXER_U32:
> +				val_len = 4;
> +				break;
> +			case USB_MIXER_S16:
> +			case USB_MIXER_U16:
> +				val_len = 2;
> +				break;
> +			default:
> +				val_len = 1;
> +				break;
> +		}
>  
>  		/* FIXME */
>  		if (request != UAC_SET_CUR) {
> @@ -469,6 +503,8 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
>  	value_set = convert_bytes_value(cval, value_set);
>  	buf[0] = value_set & 0xff;
>  	buf[1] = (value_set >> 8) & 0xff;
> +	buf[2] = (value_set >> 16) & 0xff;
> +	buf[3] = (value_set >> 24) & 0xff;

It's not smart but OK, just a subtle matter...

>  	err = snd_usb_autoresume(chip);
>  	if (err < 0)
>  		return -EIO;
> @@ -799,24 +835,25 @@ static int check_input_term(struct mixer_build *state, int id,
>  /* feature unit control information */
>  struct usb_feature_control_info {
>  	const char *name;
> -	unsigned int type;	/* control type (mute, volume, etc.) */
> +	int type;	/* data type for uac1 */
> +	int type_uac2;	/* data type for uac2 if different from uac1, else -1 */
>  };
>  
>  static struct usb_feature_control_info audio_feature_info[] = {
> -	{ "Mute",			USB_MIXER_INV_BOOLEAN },
> -	{ "Volume",			USB_MIXER_S16 },
> -	{ "Tone Control - Bass",	USB_MIXER_S8 },
> -	{ "Tone Control - Mid",		USB_MIXER_S8 },
> -	{ "Tone Control - Treble",	USB_MIXER_S8 },
> -	{ "Graphic Equalizer",		USB_MIXER_S8 }, /* FIXME: not implemeted yet */
> -	{ "Auto Gain Control",		USB_MIXER_BOOLEAN },
> -	{ "Delay Control",		USB_MIXER_U16 }, /* FIXME: U32 in UAC2 */
> -	{ "Bass Boost",			USB_MIXER_BOOLEAN },
> -	{ "Loudness",			USB_MIXER_BOOLEAN },
> +	{ "Mute",			USB_MIXER_INV_BOOLEAN, -1 },
> +	{ "Volume",			USB_MIXER_S16, -1 },
> +	{ "Tone Control - Bass",	USB_MIXER_S8, -1 },
> +	{ "Tone Control - Mid",		USB_MIXER_S8, -1 },
> +	{ "Tone Control - Treble",	USB_MIXER_S8, -1 },
> +	{ "Graphic Equalizer",		USB_MIXER_S8, -1 }, /* FIXME: not implemeted yet */
> +	{ "Auto Gain Control",		USB_MIXER_BOOLEAN, -1 },
> +	{ "Delay Control",		USB_MIXER_U16, USB_MIXER_U32 },
> +	{ "Bass Boost",			USB_MIXER_BOOLEAN, -1 },
> +	{ "Loudness",			USB_MIXER_BOOLEAN, -1 },
>  	/* UAC2 specific */
> -	{ "Input Gain Control",		USB_MIXER_S16 },
> -	{ "Input Gain Pad Control",	USB_MIXER_S16 },
> -	{ "Phase Inverter Control",	USB_MIXER_BOOLEAN },
> +	{ "Input Gain Control",		USB_MIXER_S16, -1 },
> +	{ "Input Gain Pad Control",	USB_MIXER_S16, -1 },
> +	{ "Phase Inverter Control",	USB_MIXER_BOOLEAN, -1 },
>  };
>  
>  /* private_free callback */
> @@ -1241,7 +1278,13 @@ static void build_feature_ctl(struct mixer_build *state, void *raw_desc,
>  	snd_usb_mixer_elem_init_std(&cval->head, state->mixer, unitid);
>  	cval->control = control;
>  	cval->cmask = ctl_mask;
> -	cval->val_type = audio_feature_info[control-1].type;
> +	if (state->mixer->protocol == UAC_VERSION_1)
> +		cval->val_type = audio_feature_info[control-1].type;
> +	else /* UAC_VERSION_2 */
> +		cval->val_type = audio_feature_info[control-1].type_uac2 > -1 ?

Usually ">= 0" is used.

> +			audio_feature_info[control-1].type_uac2 :
> +			audio_feature_info[control-1].type;

Since audio_feature_info[control-1] is referred so many times, it's
better to put in a temporary variable.


thanks,

Takashi
Julian Scheel Aug. 14, 2015, 1:45 p.m. UTC | #2
Am 14.08.2015 um 15:17 schrieb Takashi Iwai:
> On Thu, 13 Aug 2015 13:04:46 +0200,
> Julian Scheel wrote:
>>
>> USB Audio Class version 2.0 supports three different parameter block sizes for
>> CUR requests, which are 1 byte (5.2.3.1 Layout 1 Parameter Block), 2 bytes
>> (5.2.3.2 Layout 2 Parameter Block) and 4 bytes (5.2.3.3 Layout 3 Parameter
>> Block). Use the correct size according to the specific control as it was
>> already done for UACv1. The allocated block size for control requests is
>> increased to support the 4 byte worst case.
>>
>> Signed-off-by: Julian Scheel <julian@jusst.de>
>> ---
>>   sound/usb/mixer.c | 83 +++++++++++++++++++++++++++++++++++++++++--------------
>>   sound/usb/mixer.h |  2 ++
>>   2 files changed, 65 insertions(+), 20 deletions(-)
>>
>> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
>> index 4a49944..675725b 100644
>> --- a/sound/usb/mixer.c
>> +++ b/sound/usb/mixer.c
>> @@ -233,6 +233,14 @@ static int convert_signed_value(struct usb_mixer_elem_info *cval, int val)
>>   		if (val >= 0x8000)
>>   			val -= 0x10000;
>>   		break;
>> +	case USB_MIXER_U32:
>> +		val &= 0xffffffff;
>> +		break;
>> +	case USB_MIXER_S32:
>> +		val &= 0xffffffff;
>> +		if (val >= 0x80000000)
>> +			val -= 0x100000000;
>> +		break;
>
> All these are superfluous, as int on Linux is always 32bit.

Of course. Thanks.

>>   	}
>>   	return val;
>>   }
>> @@ -253,6 +261,9 @@ static int convert_bytes_value(struct usb_mixer_elem_info *cval, int val)
>>   	case USB_MIXER_S16:
>>   	case USB_MIXER_U16:
>>   		return val & 0xffff;
>> +	case USB_MIXER_S32:
>> +	case USB_MIXER_U32:
>> +		return val & 0xffffffff;
>
> Ditto.
>
>
>>   	}
>>   	return 0; /* not reached */
>>   }
>> @@ -328,14 +339,26 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request,
>>   			    int validx, int *value_ret)
>>   {
>>   	struct snd_usb_audio *chip = cval->head.mixer->chip;
>> -	unsigned char buf[2 + 3 * sizeof(__u16)]; /* enough space for one range */
>> +	unsigned char buf[4 + 3 * sizeof(__u32)]; /* enough space for one range */
>>   	unsigned char *val;
>>   	int idx = 0, ret, size;
>>   	__u8 bRequest;
>>
>>   	if (request == UAC_GET_CUR) {
>>   		bRequest = UAC2_CS_CUR;
>> -		size = sizeof(__u16);
>> +		switch (cval->val_type) {
>> +			case USB_MIXER_S32:
>> +			case USB_MIXER_U32:
>> +				size = 4;
>> +				break;
>> +			case USB_MIXER_S16:
>> +			case USB_MIXER_U16:
>> +				size = 2;
>> +				break;
>> +			default:
>> +				size = 1;
>> +				break;
>
> Please split this into a small helper function,
> e.g. uac2_ctl_value_size().  The same stuff is used in below, too.

Ack.

>> +		}
>>   	} else {
>>   		bRequest = UAC2_CS_RANGE;
>>   		size = sizeof(buf);
>> @@ -446,7 +469,7 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
>>   				int request, int validx, int value_set)
>>   {
>>   	struct snd_usb_audio *chip = cval->head.mixer->chip;
>> -	unsigned char buf[2];
>> +	unsigned char buf[4];
>>   	int idx = 0, val_len, err, timeout = 10;
>>
>>   	validx += cval->idx_off;
>> @@ -454,8 +477,19 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
>>   	if (cval->head.mixer->protocol == UAC_VERSION_1) {
>>   		val_len = cval->val_type >= USB_MIXER_S16 ? 2 : 1;
>>   	} else { /* UAC_VERSION_2 */
>> -		/* audio class v2 controls are always 2 bytes in size */
>> -		val_len = sizeof(__u16);
>> +		switch (cval->val_type) {
>> +			case USB_MIXER_S32:
>> +			case USB_MIXER_U32:
>> +				val_len = 4;
>> +				break;
>> +			case USB_MIXER_S16:
>> +			case USB_MIXER_U16:
>> +				val_len = 2;
>> +				break;
>> +			default:
>> +				val_len = 1;
>> +				break;
>> +		}
>>
>>   		/* FIXME */
>>   		if (request != UAC_SET_CUR) {
>> @@ -469,6 +503,8 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
>>   	value_set = convert_bytes_value(cval, value_set);
>>   	buf[0] = value_set & 0xff;
>>   	buf[1] = (value_set >> 8) & 0xff;
>> +	buf[2] = (value_set >> 16) & 0xff;
>> +	buf[3] = (value_set >> 24) & 0xff;
>
> It's not smart but OK, just a subtle matter...

Hm, what would you like better?

>>   	err = snd_usb_autoresume(chip);
>>   	if (err < 0)
>>   		return -EIO;
>> @@ -799,24 +835,25 @@ static int check_input_term(struct mixer_build *state, int id,
>>   /* feature unit control information */
>>   struct usb_feature_control_info {
>>   	const char *name;
>> -	unsigned int type;	/* control type (mute, volume, etc.) */
>> +	int type;	/* data type for uac1 */
>> +	int type_uac2;	/* data type for uac2 if different from uac1, else -1 */
>>   };
>>
>>   static struct usb_feature_control_info audio_feature_info[] = {
>> -	{ "Mute",			USB_MIXER_INV_BOOLEAN },
>> -	{ "Volume",			USB_MIXER_S16 },
>> -	{ "Tone Control - Bass",	USB_MIXER_S8 },
>> -	{ "Tone Control - Mid",		USB_MIXER_S8 },
>> -	{ "Tone Control - Treble",	USB_MIXER_S8 },
>> -	{ "Graphic Equalizer",		USB_MIXER_S8 }, /* FIXME: not implemeted yet */
>> -	{ "Auto Gain Control",		USB_MIXER_BOOLEAN },
>> -	{ "Delay Control",		USB_MIXER_U16 }, /* FIXME: U32 in UAC2 */
>> -	{ "Bass Boost",			USB_MIXER_BOOLEAN },
>> -	{ "Loudness",			USB_MIXER_BOOLEAN },
>> +	{ "Mute",			USB_MIXER_INV_BOOLEAN, -1 },
>> +	{ "Volume",			USB_MIXER_S16, -1 },
>> +	{ "Tone Control - Bass",	USB_MIXER_S8, -1 },
>> +	{ "Tone Control - Mid",		USB_MIXER_S8, -1 },
>> +	{ "Tone Control - Treble",	USB_MIXER_S8, -1 },
>> +	{ "Graphic Equalizer",		USB_MIXER_S8, -1 }, /* FIXME: not implemeted yet */
>> +	{ "Auto Gain Control",		USB_MIXER_BOOLEAN, -1 },
>> +	{ "Delay Control",		USB_MIXER_U16, USB_MIXER_U32 },
>> +	{ "Bass Boost",			USB_MIXER_BOOLEAN, -1 },
>> +	{ "Loudness",			USB_MIXER_BOOLEAN, -1 },
>>   	/* UAC2 specific */
>> -	{ "Input Gain Control",		USB_MIXER_S16 },
>> -	{ "Input Gain Pad Control",	USB_MIXER_S16 },
>> -	{ "Phase Inverter Control",	USB_MIXER_BOOLEAN },
>> +	{ "Input Gain Control",		USB_MIXER_S16, -1 },
>> +	{ "Input Gain Pad Control",	USB_MIXER_S16, -1 },
>> +	{ "Phase Inverter Control",	USB_MIXER_BOOLEAN, -1 },
>>   };
>>
>>   /* private_free callback */
>> @@ -1241,7 +1278,13 @@ static void build_feature_ctl(struct mixer_build *state, void *raw_desc,
>>   	snd_usb_mixer_elem_init_std(&cval->head, state->mixer, unitid);
>>   	cval->control = control;
>>   	cval->cmask = ctl_mask;
>> -	cval->val_type = audio_feature_info[control-1].type;
>> +	if (state->mixer->protocol == UAC_VERSION_1)
>> +		cval->val_type = audio_feature_info[control-1].type;
>> +	else /* UAC_VERSION_2 */
>> +		cval->val_type = audio_feature_info[control-1].type_uac2 > -1 ?
>
> Usually ">= 0" is used.

Ack.

>> +			audio_feature_info[control-1].type_uac2 :
>> +			audio_feature_info[control-1].type;
>
> Since audio_feature_info[control-1] is referred so many times, it's
> better to put in a temporary variable.

Ack.

Thanks for reviewing!
-Julian
Takashi Iwai Aug. 14, 2015, 1:51 p.m. UTC | #3
On Fri, 14 Aug 2015 15:45:58 +0200,
Julian Scheel wrote:
> 
> Am 14.08.2015 um 15:17 schrieb Takashi Iwai:
> > On Thu, 13 Aug 2015 13:04:46 +0200,
> > Julian Scheel wrote:
> >>
> >> @@ -469,6 +503,8 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
> >>   	value_set = convert_bytes_value(cval, value_set);
> >>   	buf[0] = value_set & 0xff;
> >>   	buf[1] = (value_set >> 8) & 0xff;
> >> +	buf[2] = (value_set >> 16) & 0xff;
> >> +	buf[3] = (value_set >> 24) & 0xff;
> >
> > It's not smart but OK, just a subtle matter...
> 
> Hm, what would you like better?

A shorter code would be to to pass __le32 value pointer directly after
calling cpu_to_le32(), for example.  But it's a matter of taste, so
it's OK to keep your code as is.


Takashi
Julian Scheel Aug. 14, 2015, 2:14 p.m. UTC | #4
Am 14.08.2015 um 15:51 schrieb Takashi Iwai:
> On Fri, 14 Aug 2015 15:45:58 +0200,
> Julian Scheel wrote:
>>
>> Am 14.08.2015 um 15:17 schrieb Takashi Iwai:
>>> On Thu, 13 Aug 2015 13:04:46 +0200,
>>> Julian Scheel wrote:
>>>>
>>>> @@ -469,6 +503,8 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
>>>>    	value_set = convert_bytes_value(cval, value_set);
>>>>    	buf[0] = value_set & 0xff;
>>>>    	buf[1] = (value_set >> 8) & 0xff;
>>>> +	buf[2] = (value_set >> 16) & 0xff;
>>>> +	buf[3] = (value_set >> 24) & 0xff;
>>>
>>> It's not smart but OK, just a subtle matter...
>>
>> Hm, what would you like better?
>
> A shorter code would be to to pass __le32 value pointer directly after
> calling cpu_to_le32(), for example.  But it's a matter of taste, so
> it's OK to keep your code as is.

Thanks, I'll leave it as is for now and keep the le32 approach in mind 
for the future.

Patch
diff mbox

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 4a49944..675725b 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -233,6 +233,14 @@  static int convert_signed_value(struct usb_mixer_elem_info *cval, int val)
 		if (val >= 0x8000)
 			val -= 0x10000;
 		break;
+	case USB_MIXER_U32:
+		val &= 0xffffffff;
+		break;
+	case USB_MIXER_S32:
+		val &= 0xffffffff;
+		if (val >= 0x80000000)
+			val -= 0x100000000;
+		break;
 	}
 	return val;
 }
@@ -253,6 +261,9 @@  static int convert_bytes_value(struct usb_mixer_elem_info *cval, int val)
 	case USB_MIXER_S16:
 	case USB_MIXER_U16:
 		return val & 0xffff;
+	case USB_MIXER_S32:
+	case USB_MIXER_U32:
+		return val & 0xffffffff;
 	}
 	return 0; /* not reached */
 }
@@ -328,14 +339,26 @@  static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request,
 			    int validx, int *value_ret)
 {
 	struct snd_usb_audio *chip = cval->head.mixer->chip;
-	unsigned char buf[2 + 3 * sizeof(__u16)]; /* enough space for one range */
+	unsigned char buf[4 + 3 * sizeof(__u32)]; /* enough space for one range */
 	unsigned char *val;
 	int idx = 0, ret, size;
 	__u8 bRequest;
 
 	if (request == UAC_GET_CUR) {
 		bRequest = UAC2_CS_CUR;
-		size = sizeof(__u16);
+		switch (cval->val_type) {
+			case USB_MIXER_S32:
+			case USB_MIXER_U32:
+				size = 4;
+				break;
+			case USB_MIXER_S16:
+			case USB_MIXER_U16:
+				size = 2;
+				break;
+			default:
+				size = 1;
+				break;
+		}
 	} else {
 		bRequest = UAC2_CS_RANGE;
 		size = sizeof(buf);
@@ -446,7 +469,7 @@  int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
 				int request, int validx, int value_set)
 {
 	struct snd_usb_audio *chip = cval->head.mixer->chip;
-	unsigned char buf[2];
+	unsigned char buf[4];
 	int idx = 0, val_len, err, timeout = 10;
 
 	validx += cval->idx_off;
@@ -454,8 +477,19 @@  int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
 	if (cval->head.mixer->protocol == UAC_VERSION_1) {
 		val_len = cval->val_type >= USB_MIXER_S16 ? 2 : 1;
 	} else { /* UAC_VERSION_2 */
-		/* audio class v2 controls are always 2 bytes in size */
-		val_len = sizeof(__u16);
+		switch (cval->val_type) {
+			case USB_MIXER_S32:
+			case USB_MIXER_U32:
+				val_len = 4;
+				break;
+			case USB_MIXER_S16:
+			case USB_MIXER_U16:
+				val_len = 2;
+				break;
+			default:
+				val_len = 1;
+				break;
+		}
 
 		/* FIXME */
 		if (request != UAC_SET_CUR) {
@@ -469,6 +503,8 @@  int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
 	value_set = convert_bytes_value(cval, value_set);
 	buf[0] = value_set & 0xff;
 	buf[1] = (value_set >> 8) & 0xff;
+	buf[2] = (value_set >> 16) & 0xff;
+	buf[3] = (value_set >> 24) & 0xff;
 	err = snd_usb_autoresume(chip);
 	if (err < 0)
 		return -EIO;
@@ -799,24 +835,25 @@  static int check_input_term(struct mixer_build *state, int id,
 /* feature unit control information */
 struct usb_feature_control_info {
 	const char *name;
-	unsigned int type;	/* control type (mute, volume, etc.) */
+	int type;	/* data type for uac1 */
+	int type_uac2;	/* data type for uac2 if different from uac1, else -1 */
 };
 
 static struct usb_feature_control_info audio_feature_info[] = {
-	{ "Mute",			USB_MIXER_INV_BOOLEAN },
-	{ "Volume",			USB_MIXER_S16 },
-	{ "Tone Control - Bass",	USB_MIXER_S8 },
-	{ "Tone Control - Mid",		USB_MIXER_S8 },
-	{ "Tone Control - Treble",	USB_MIXER_S8 },
-	{ "Graphic Equalizer",		USB_MIXER_S8 }, /* FIXME: not implemeted yet */
-	{ "Auto Gain Control",		USB_MIXER_BOOLEAN },
-	{ "Delay Control",		USB_MIXER_U16 }, /* FIXME: U32 in UAC2 */
-	{ "Bass Boost",			USB_MIXER_BOOLEAN },
-	{ "Loudness",			USB_MIXER_BOOLEAN },
+	{ "Mute",			USB_MIXER_INV_BOOLEAN, -1 },
+	{ "Volume",			USB_MIXER_S16, -1 },
+	{ "Tone Control - Bass",	USB_MIXER_S8, -1 },
+	{ "Tone Control - Mid",		USB_MIXER_S8, -1 },
+	{ "Tone Control - Treble",	USB_MIXER_S8, -1 },
+	{ "Graphic Equalizer",		USB_MIXER_S8, -1 }, /* FIXME: not implemeted yet */
+	{ "Auto Gain Control",		USB_MIXER_BOOLEAN, -1 },
+	{ "Delay Control",		USB_MIXER_U16, USB_MIXER_U32 },
+	{ "Bass Boost",			USB_MIXER_BOOLEAN, -1 },
+	{ "Loudness",			USB_MIXER_BOOLEAN, -1 },
 	/* UAC2 specific */
-	{ "Input Gain Control",		USB_MIXER_S16 },
-	{ "Input Gain Pad Control",	USB_MIXER_S16 },
-	{ "Phase Inverter Control",	USB_MIXER_BOOLEAN },
+	{ "Input Gain Control",		USB_MIXER_S16, -1 },
+	{ "Input Gain Pad Control",	USB_MIXER_S16, -1 },
+	{ "Phase Inverter Control",	USB_MIXER_BOOLEAN, -1 },
 };
 
 /* private_free callback */
@@ -1241,7 +1278,13 @@  static void build_feature_ctl(struct mixer_build *state, void *raw_desc,
 	snd_usb_mixer_elem_init_std(&cval->head, state->mixer, unitid);
 	cval->control = control;
 	cval->cmask = ctl_mask;
-	cval->val_type = audio_feature_info[control-1].type;
+	if (state->mixer->protocol == UAC_VERSION_1)
+		cval->val_type = audio_feature_info[control-1].type;
+	else /* UAC_VERSION_2 */
+		cval->val_type = audio_feature_info[control-1].type_uac2 > -1 ?
+			audio_feature_info[control-1].type_uac2 :
+			audio_feature_info[control-1].type;
+
 	if (ctl_mask == 0) {
 		cval->channels = 1;	/* master channel */
 		cval->master_readonly = readonly_mask;
diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h
index d3268f0..3417ef3 100644
--- a/sound/usb/mixer.h
+++ b/sound/usb/mixer.h
@@ -33,6 +33,8 @@  enum {
 	USB_MIXER_U8,
 	USB_MIXER_S16,
 	USB_MIXER_U16,
+	USB_MIXER_S32,
+	USB_MIXER_U32,
 };
 
 typedef void (*usb_mixer_elem_dump_func_t)(struct snd_info_buffer *buffer,