ALSA: usb-audio: fix uac control query argument

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

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

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

This patch fixes all callers that had adjusted, and makes explicit
the mapping between audio_feature_info[] array index and the associated
control identifier.
Signed-off-by: Andrew Chant <achant@google.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>

include/linux/usb/audio-v2.h

@@ -36,12 +36,12 @@
 
 static inline bool uac_v2v3_control_is_readable(u32 bmControls, u8 control)
 {
-	return (bmControls >> (control * 2)) & 0x1;
+	return (bmControls >> ((control - 1) * 2)) & 0x1;
 }
 
 static inline bool uac_v2v3_control_is_writeable(u32 bmControls, u8 control)
 {
-	return (bmControls >> (control * 2)) & 0x2;
+	return (bmControls >> ((control - 1) * 2)) & 0x2;
 }
 
 /* 4.7.2 Class-Specific AC Interface Descriptor */

sound/usb/clock.c

@@ -214,7 +214,7 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip,
 
 	/* If a clock source can't tell us whether it's valid, we assume it is */
 	if (!uac_v2v3_control_is_readable(bmControls,
-				      UAC2_CS_CONTROL_CLOCK_VALID - 1))
+				      UAC2_CS_CONTROL_CLOCK_VALID))
 		return 1;
 
 	err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_CUR,
@@ -552,7 +552,8 @@ static int set_sample_rate_v2v3(struct snd_usb_audio *chip, int iface,
 		bmControls = cs_desc->bmControls;
 	}
 
-	writeable = uac_v2v3_control_is_writeable(bmControls, UAC2_CS_CONTROL_SAM_FREQ - 1);
+	writeable = uac_v2v3_control_is_writeable(bmControls,
+						  UAC2_CS_CONTROL_SAM_FREQ);
 	if (writeable) {
 		data = cpu_to_le32(rate);
 		err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC2_CS_CUR,

sound/usb/mixer.c

@@ -879,26 +879,27 @@ static int check_input_term(struct mixer_build *state, int id,
 
 /* feature unit control information */
 struct usb_feature_control_info {
+	int control;
 	const char *name;
 	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, -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 },
+	{ UAC_FU_MUTE,			"Mute",			USB_MIXER_INV_BOOLEAN, -1 },
+	{ UAC_FU_VOLUME,		"Volume",		USB_MIXER_S16, -1 },
+	{ UAC_FU_BASS,			"Tone Control - Bass",	USB_MIXER_S8, -1 },
+	{ UAC_FU_MID,			"Tone Control - Mid",	USB_MIXER_S8, -1 },
+	{ UAC_FU_TREBLE,		"Tone Control - Treble", USB_MIXER_S8, -1 },
+	{ UAC_FU_GRAPHIC_EQUALIZER,	"Graphic Equalizer",	USB_MIXER_S8, -1 }, /* FIXME: not implemented yet */
+	{ UAC_FU_AUTOMATIC_GAIN,	"Auto Gain Control",	USB_MIXER_BOOLEAN, -1 },
+	{ UAC_FU_DELAY,			"Delay Control",	USB_MIXER_U16, USB_MIXER_U32 },
+	{ UAC_FU_BASS_BOOST,		"Bass Boost",		USB_MIXER_BOOLEAN, -1 },
+	{ UAC_FU_LOUDNESS,		"Loudness",		USB_MIXER_BOOLEAN, -1 },
 	/* UAC2 specific */
-	{ "Input Gain Control",		USB_MIXER_S16, -1 },
-	{ "Input Gain Pad Control",	USB_MIXER_S16, -1 },
-	{ "Phase Inverter Control",	USB_MIXER_BOOLEAN, -1 },
+	{ UAC2_FU_INPUT_GAIN,		"Input Gain Control",	USB_MIXER_S16, -1 },
+	{ UAC2_FU_INPUT_GAIN_PAD,	"Input Gain Pad Control", USB_MIXER_S16, -1 },
+	{ UAC2_FU_PHASE_INVERTER,	 "Phase Inverter Control", USB_MIXER_BOOLEAN, -1 },
 };
 
 /* private_free callback */
@@ -1293,6 +1294,17 @@ static void check_no_speaker_on_headset(struct snd_kcontrol *kctl,
 	strlcpy(kctl->id.name, "Headphone", sizeof(kctl->id.name));
 }
 
+static struct usb_feature_control_info *get_feature_control_info(int control)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(audio_feature_info); ++i) {
+		if (audio_feature_info[i].control == control)
+			return &audio_feature_info[i];
+	}
+	return NULL;
+}
+
 static void build_feature_ctl(struct mixer_build *state, void *raw_desc,
 			      unsigned int ctl_mask, int control,
 			      struct usb_audio_term *iterm, int unitid,
@@ -1308,8 +1320,6 @@ static void build_feature_ctl(struct mixer_build *state, void *raw_desc,
 	const struct usbmix_name_map *map;
 	unsigned int range;
 
-	control++; /* change from zero-based to 1-based value */
-
 	if (control == UAC_FU_GRAPHIC_EQUALIZER) {
 		/* FIXME: not supported yet */
 		return;
@@ -1325,7 +1335,10 @@ 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;
-	ctl_info = &audio_feature_info[control-1];
+
+	ctl_info = get_feature_control_info(control);
+	if (!ctl_info)
+		return;
 	if (state->mixer->protocol == UAC_VERSION_1)
 		cval->val_type = ctl_info->type;
 	else /* UAC_VERSION_2 */
@@ -1475,7 +1488,7 @@ static int parse_clock_source_unit(struct mixer_build *state, int unitid,
 	 * clock source validity. If that isn't readable, just bail out.
 	 */
 	if (!uac_v2v3_control_is_readable(hdr->bmControls,
-				      ilog2(UAC2_CS_CONTROL_CLOCK_VALID)))
+				      UAC2_CS_CONTROL_CLOCK_VALID))
 		return 0;
 
 	cval = kzalloc(sizeof(*cval), GFP_KERNEL);
@@ -1491,7 +1504,7 @@ static int parse_clock_source_unit(struct mixer_build *state, int unitid,
 	cval->control = UAC2_CS_CONTROL_CLOCK_VALID;
 
 	if (uac_v2v3_control_is_writeable(hdr->bmControls,
-				      ilog2(UAC2_CS_CONTROL_CLOCK_VALID)))
+				      UAC2_CS_CONTROL_CLOCK_VALID))
 		kctl = snd_ctl_new1(&usb_feature_unit_ctl, cval);
 	else {
 		cval->master_readonly = 1;
@@ -1625,6 +1638,8 @@ static int parse_audio_feature_unit(struct mixer_build *state, int unitid,
 		/* check all control types */
 		for (i = 0; i < 10; i++) {
 			unsigned int ch_bits = 0;
+			int control = audio_feature_info[i].control;
+
 			for (j = 0; j < channels; j++) {
 				unsigned int mask;
 
@@ -1640,25 +1655,26 @@ static int parse_audio_feature_unit(struct mixer_build *state, int unitid,
 			 * (for ease of programming).
 			 */
 			if (ch_bits & 1)
-				build_feature_ctl(state, _ftr, ch_bits, i,
+				build_feature_ctl(state, _ftr, ch_bits, control,
 						  &iterm, unitid, 0);
 			if (master_bits & (1 << i))
-				build_feature_ctl(state, _ftr, 0, i, &iterm,
-						  unitid, 0);
+				build_feature_ctl(state, _ftr, 0, control,
+						  &iterm, unitid, 0);
 		}
 	} 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;
+			int control = audio_feature_info[i].control;
 
 			for (j = 0; j < channels; j++) {
 				unsigned int mask;
 
 				mask = snd_usb_combine_bytes(bmaControls +
 							     csize * (j+1), csize);
-				if (uac_v2v3_control_is_readable(mask, i)) {
+				if (uac_v2v3_control_is_readable(mask, control)) {
 					ch_bits |= (1 << j);
-					if (!uac_v2v3_control_is_writeable(mask, i))
+					if (!uac_v2v3_control_is_writeable(mask, control))
 						ch_read_only |= (1 << j);
 				}
 			}
@@ -1677,11 +1693,12 @@ static int parse_audio_feature_unit(struct mixer_build *state, int unitid,
 			 * (for ease of programming).
 			 */
 			if (ch_bits & 1)
-				build_feature_ctl(state, _ftr, ch_bits, i,
+				build_feature_ctl(state, _ftr, ch_bits, control,
 						  &iterm, unitid, ch_read_only);
-			if (uac_v2v3_control_is_readable(master_bits, i))
+			if (uac_v2v3_control_is_readable(master_bits, control))
 				build_feature_ctl(state, _ftr, 0, i, &iterm, unitid,
-						  !uac_v2v3_control_is_writeable(master_bits, i));
+						  !uac_v2v3_control_is_writeable(master_bits,
+										 control));
 		}
 	}