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>

3 files changed, 48 insertions, 30 deletions

diff --git a/include/linux/usb/audio-v2.h b/include/linux/usb/audio-v2.h
index 2db83a191e78..aaafecf073ff 100644
--- a/include/linux/usb/audio-v2.h
+++ b/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 */
diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index 25de7fe285d9..ab39ccb974c6 100644
--- a/sound/usb/clock.c
+++ b/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,
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 1c02f7373eda..3075ac50a391 100644
--- a/sound/usb/mixer.c
+++ b/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));
                 }
         }