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>

diff --git a/include/linux/usb/audio-v2.h b/include/linux/usb/audio-v2.h
index 2db83a191e78e3a4590794dd7146569c492cb179..aaafecf073ff8770ec7cdf478a2ab1e80b8e46bc 100644 (file)
--- 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 25de7fe285d9f8e34e58e88b767d11d149ba4ece..ab39ccb974c6f276a53003783d2619094afd4f32 100644 (file)
--- 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 1c02f7373edae335f5f2aa957463a960ff461c5e..3075ac50a391b164c356ca882222f6e7d555fd99 100644 (file)
--- 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));
                }
        }