Received: by 10.192.165.148 with SMTP id m20csp194960imm; Thu, 3 May 2018 17:59:07 -0700 (PDT) X-Google-Smtp-Source: AB8JxZoBxuNj+cySqwecD6+Nz55mIywaeWNEw5CI+Jufbk2o2zq1lYfrR+iW+cTkf9SzrYPlMNoZ X-Received: by 2002:a63:b306:: with SMTP id i6-v6mr20668421pgf.434.1525395547550; Thu, 03 May 2018 17:59:07 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1525395547; cv=none; d=google.com; s=arc-20160816; b=RnlWJsG6Nx+X732fNbs1pKTilW0zVj5t7ryQwqDVk8RpWxeyDvUIFyQ4K2a3+dt539 J147OI9jBGA3R9zGNGsCWXSqZKE8uAxEYWJVpjL1VCDOC0H2OFGDgSdLW2uHvdRPGWRi Nlw4jFk6q0OWkQ+KPUQe+CJ/NH764RZxo+TADyXBL2tJyzGJBnYupjBY0bUXVSDgOJoV sNOBq67OVL2O5+DecUMELKtsI1rzEmZfER8Mgk/TgoeLq5hggUaA0JgxnUdhJSiQZvpp Ed/8PwX6D2v3HRb0Jl5qsQBxeyeM1RL57KNU/kjHvprdmzL2c8rRghayeOShMYKvH5LX SsLg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :references:in-reply-to:mime-version:dkim-signature :arc-authentication-results; bh=R5sPrsRm/TGTYd4TGzlwzsEnYN3fD56LugTxk6Lm77E=; b=t1je5SGH/tUlEv1HRhvQulx2hNvxEcjNsPiVhvRYUnCKuQBZP6nX1Us5xpP+GL/cEp jHqaaIZpsqPtIFeZQ06t4aS9dEEcD14w0R7rb702Eq1wK4GzmAV4qOH8RIEnuPlDleEh ZLPpPbwoxNFOzHzuID7+ZgbFsp8LdZeCS25eky7SGFu/hFnxCtZFxHGtolABf3J4QHPX j3+G6bfbP+KqP7UK3qUKsSpQKf2EAXGEYti1NinVKJOUlBJuUuRsJA4ocB0BoqM3Z0N7 UlSofp4//GXK/OdVOqEpFQeFCF+qrrefv6qHuRJqJm3CeaQ9pd0JGMrhT7yYop5Mbh2w Ovig== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=p4ybVrxu; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id i4-v6si14715707plt.581.2018.05.03.17.58.52; Thu, 03 May 2018 17:59:07 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=p4ybVrxu; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751190AbeEDA5e (ORCPT + 99 others); Thu, 3 May 2018 20:57:34 -0400 Received: from mail-oi0-f67.google.com ([209.85.218.67]:33930 "EHLO mail-oi0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751132AbeEDA5d (ORCPT ); Thu, 3 May 2018 20:57:33 -0400 Received: by mail-oi0-f67.google.com with SMTP id l1-v6so17772986oii.1 for ; Thu, 03 May 2018 17:57:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=R5sPrsRm/TGTYd4TGzlwzsEnYN3fD56LugTxk6Lm77E=; b=p4ybVrxuaZlKjuLt6fCW0+iFYuA4trt83K1lw7bX10Vy78VK47ATj4al4SJ6CLZiSN nYHij/vWkSo3TdvR7j/taavWV+c+NWFkuWuqGUs7vl3Whgzty0Kwxuc+dRx4ui6wSzq6 WL1nsI8n4LSPjbx8JU9Z7pYbY0s1rU4WJ9QTYg4rjMoNMoAUnotUSqn3UINHEamAVvm0 EAR0U6eYUMACC23/x02mtsE+cYw6kO9Z5dHMLBxidQeth2CqPNZxnFo7jPjC8aA136aS D5vNlg+9xoS6n4mQc1WHt1WgMH9yVa7NZVZhEl87A4ZjJMrDGYyrOhCm8pLr289+Ekr6 HD6Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=R5sPrsRm/TGTYd4TGzlwzsEnYN3fD56LugTxk6Lm77E=; b=UYLowTZBv7RHkghJtVBKBqhitVys76lgJJe98W3ORGTxXuiPcqXS7sTpZSWwNBcTI2 VGeJt6shE9FH5WZH8ZTGduisFw1zMc9gGRWZtABTqXll1Ej/fg1s7KKrcFDaPbRHjgc+ ro1EeCyKdLEYwPeRJowSEIGI0BIuKAOyLqhzgNPkqt3hSPIZn/tKCcdyHtyBJlgu7AED CPDl0klrOMegUPqz+SQ+miVj+rz8Ar04Qqv5+g2yFVNbG2EFeMuP3Fc5xl3SNnNSiQKr Acl22Dfl/vmftWKPS3DcmrREDlljeViEz9guib1A1LvpAL4qpuspDmmhBeN2uNniSvhW T67g== X-Gm-Message-State: ALQs6tDEhy37BVYhdUFRGaIEhQGeyHBOdKhaPm2QAvd5b2JpovKZBZnC zbRQo+4QopeRWJrsGnUg9D9bIZTZEHzpT4D9WLg= X-Received: by 2002:aca:68ea:: with SMTP id o103-v6mr4748994oik.93.1525395452787; Thu, 03 May 2018 17:57:32 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a9d:2c03:0:0:0:0:0 with HTTP; Thu, 3 May 2018 17:57:32 -0700 (PDT) In-Reply-To: <20180427170649.27334-1-jorge.sanjuan@codethink.co.uk> References: <20180424172445.31928-2-jorge.sanjuan@codethink.co.uk> <20180427170649.27334-1-jorge.sanjuan@codethink.co.uk> From: Ruslan Bilovol Date: Fri, 4 May 2018 03:57:32 +0300 Message-ID: Subject: Re: [PATCH v3 1/4] ALSA: usb-audio: UAC3. Add support for mixer unit. To: Jorge Sanjuan Cc: Takashi Iwai , alsa-devel@alsa-project.org, Greg Kroah-Hartman , linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Apr 27, 2018 at 8:06 PM, Jorge Sanjuan wrote: > This adds support for the MIXER UNIT in UAC3. All the information > is obtained from the (HIGH CAPABILITY) Cluster's header. We don't > read the rest of the logical cluster to obtain the channel config > as that wont make any difference in the current mixer behaviour. > > The name of the mixer unit is not yet requested as there is not > support for the UAC3 Class Specific String requests. > > Tested in an UAC3 device working as a HEADSET with a basic mixer > unit (same as the one in the BADD spec) with no controls. So, after deeper looking into the code and after testing this patch, in your usecase (mixer with no controls) you'll never execute build_mixer_unit_ctl(), correct? So did you try to just fix issues with incorrect parsing of mixer unit descriptor? > > Signed-off-by: Jorge Sanjuan > --- > include/uapi/linux/usb/audio.h | 19 +++++++-- > sound/usb/mixer.c | 88 ++++++++++++++++++++++++++++++++++++++---- > 2 files changed, 97 insertions(+), 10 deletions(-) > > diff --git a/include/uapi/linux/usb/audio.h b/include/uapi/linux/usb/audio.h > index 3a78e7145689..13d98e6e0db1 100644 > --- a/include/uapi/linux/usb/audio.h > +++ b/include/uapi/linux/usb/audio.h > @@ -285,9 +285,22 @@ static inline __u8 uac_mixer_unit_iChannelNames(struct uac_mixer_unit_descriptor > static inline __u8 *uac_mixer_unit_bmControls(struct uac_mixer_unit_descriptor *desc, > int protocol) > { > - return (protocol == UAC_VERSION_1) ? > - &desc->baSourceID[desc->bNrInPins + 4] : > - &desc->baSourceID[desc->bNrInPins + 6]; > + switch (protocol) { > + case UAC_VERSION_1: > + return &desc->baSourceID[desc->bNrInPins + 4]; > + case UAC_VERSION_2: > + return &desc->baSourceID[desc->bNrInPins + 6]; > + case UAC_VERSION_3: > + return &desc->baSourceID[desc->bNrInPins + 2]; > + default: > + return NULL; > + } > +} > + > +static inline __u16 uac3_mixer_unit_wClusterDescrID(struct uac_mixer_unit_descriptor *desc) > +{ > + return (desc->baSourceID[desc->bNrInPins + 1] << 8) | > + desc->baSourceID[desc->bNrInPins]; > } > > static inline __u8 uac_mixer_unit_iMixer(struct uac_mixer_unit_descriptor *desc) > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c > index 301ad61ed426..3503f4840ec3 100644 > --- a/sound/usb/mixer.c > +++ b/sound/usb/mixer.c > @@ -719,6 +719,66 @@ static int get_term_name(struct mixer_build *state, struct usb_audio_term *iterm > } > > /* > + * Get logical cluster information for UAC3 devices. > + */ > +static int get_cluster_channels_v3(struct mixer_build *state, unsigned int cluster_id) > +{ > + struct uac3_cluster_header_descriptor c_header; > + int err; > + > + err = snd_usb_ctl_msg(state->chip->dev, > + usb_rcvctrlpipe(state->chip->dev, 0), > + UAC3_CS_REQ_HIGH_CAPABILITY_DESCRIPTOR, > + USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN, > + cluster_id, > + snd_usb_ctrl_intf(state->chip), > + &c_header, sizeof(c_header)); > + if (err < 0) > + goto error; > + if (err != sizeof(c_header)) { > + err = -EIO; > + goto error; > + } > + > + return c_header.bNrChannels; > + > +error: > + usb_audio_err(state->chip, "cannot request logical cluster ID: %d (err: %d)\n", cluster_id, err); > + return err; > +} > + > +/* > + * Get number of channels for a Mixer Unit. > + */ > +static int uac_mixer_unit_get_channels(struct mixer_build *state, > + struct uac_mixer_unit_descriptor *desc) > +{ > + int mu_channels; > + > + if (desc->bLength < 11) > + return -EINVAL; > + if (!desc->bNrInPins) > + return -EINVAL; > + > + switch (state->mixer->protocol) { > + case UAC_VERSION_1: > + case UAC_VERSION_2: > + default: > + mu_channels = uac_mixer_unit_bNrChannels(desc); > + break; > + case UAC_VERSION_3: > + mu_channels = get_cluster_channels_v3(state, > + uac3_mixer_unit_wClusterDescrID(desc)); > + break; > + } > + > + if (!mu_channels) > + return -EINVAL; > + > + return mu_channels; > +} > + > +/* > * parse the source unit recursively until it reaches to a terminal > * or a branched unit. > */ > @@ -865,6 +925,18 @@ static int check_input_term(struct mixer_build *state, int id, > term->name = le16_to_cpu(d->wClockSourceStr); > return 0; > } > + case UAC3_MIXER_UNIT: { > + struct uac_mixer_unit_descriptor *d = p1; > + > + err = uac_mixer_unit_get_channels(state, d); > + if (err < 0) > + return err; > + > + term->channels = err; > + term->type = d->bDescriptorSubtype << 16; /* virtual type */ > + > + return 0; > + } > default: > return -ENODEV; > } > @@ -1797,11 +1869,10 @@ static int parse_audio_feature_unit(struct mixer_build *state, int unitid, > */ > static void build_mixer_unit_ctl(struct mixer_build *state, > struct uac_mixer_unit_descriptor *desc, > - int in_pin, int in_ch, int unitid, > - struct usb_audio_term *iterm) > + int in_pin, int in_ch, int num_outs, > + int unitid, struct usb_audio_term *iterm) > { > struct usb_mixer_elem_info *cval; > - unsigned int num_outs = uac_mixer_unit_bNrChannels(desc); > unsigned int i, len; > struct snd_kcontrol *kctl; > const struct usbmix_name_map *map; > @@ -1878,14 +1949,17 @@ static int parse_audio_mixer_unit(struct mixer_build *state, int unitid, > int input_pins, num_ins, num_outs; > int pin, ich, err; > > - if (desc->bLength < 11 || !(input_pins = desc->bNrInPins) || > - !(num_outs = uac_mixer_unit_bNrChannels(desc))) { > + err = uac_mixer_unit_get_channels(state, desc); > + if (err < 0) { > usb_audio_err(state->chip, > "invalid MIXER UNIT descriptor %d\n", > unitid); > - return -EINVAL; > + return err; > } > > + num_outs = err; > + input_pins = desc->bNrInPins; > + > num_ins = 0; > ich = 0; > for (pin = 0; pin < input_pins; pin++) { > @@ -1912,7 +1986,7 @@ static int parse_audio_mixer_unit(struct mixer_build *state, int unitid, > } > } > if (ich_has_controls) > - build_mixer_unit_ctl(state, desc, pin, ich, > + build_mixer_unit_ctl(state, desc, pin, ich, num_outs, > unitid, &iterm); So with current sources we will never reach this place. In your usecase (mixer with no controls) it obviously won't go inside. However, I created a mixer with controls but still build_mixer_unit_ctl() isn't executed. This is because UAC3 input terminal parsing always returns 0 channels, this is what still needs to be implemented (see comment "REVISIT: UAC3 IT doesn't have channels/cfg" in check_input_term) Beside of that, other part of this patch seems to work as expected, at least uac_mixer_unit_get_channels() gives correct results - I checked it for two-channels config, that is different comparing to BADD. Thanks, Ruslan