Received: by 10.192.165.148 with SMTP id m20csp1263024imm; Wed, 25 Apr 2018 15:37:24 -0700 (PDT) X-Google-Smtp-Source: AIpwx4+u/p1/Vjpm6U4vJOCOXGXiXmfhYDUg/JJs00sOQDSXvIf1LrZRz99eo1QCiZB/Dp01s5DA X-Received: by 10.99.96.193 with SMTP id u184mr21373336pgb.122.1524695844806; Wed, 25 Apr 2018 15:37:24 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1524695844; cv=none; d=google.com; s=arc-20160816; b=PsORLoKbzV2Rb3Pfwv7/EDpfB6lRhoo8S45PFYekXat8VpZGD4uYJ6eet4/+MFJo5O SgZH1s04nX14zf/Y5lZ1a5SoqsPaaP5jOf3yUAZI9ZhwZY8ziGXgpNkVsmv5qbt/cJIA oh0xTHn6RlQKpiab4crVWjzBx+MRnrkClGP3MGvE+1kAQQ9Sf2hbJBjF3SX3pAs/6A3m q8pi1uqm99TcBjamjRRhxGvlDv0uRxWI6yMLjhEtauP6HLUp0mjgY03Mf6NrMYsUn19T 7DuG9p4YqH1nSXnfx/Vs6jnxyI4Nr0L29V1ZsQ+WWOMdVQw6HgZoc0Z4yULMTX5YbJZe jdYA== 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=KyR/yJfkanXQWrCN6t4LO7jOmrz9emLaM97NaI8gLg8=; b=mcTnX4tgqmaW5vhsHTh+SnmX5up0obf/c7af4VflMezgz25EgcT4a5cydfAfR4Aasl wGi8dMlFBtQ0UH8pCtXkvZixRnA+vK+ntgCiOSPk3A7WtTuGeaomJmAu+Z6bMfAbI/MT hlb9CKaYPyqoONbDIbUK7mXa+bDBEXRxCsqAiO751/NqtbhEOW6OlqvvlSMFCCIhWc+A CZX3ySLD021HnA9M313v4MK0ygsvMaG6vPBAw4sY7DmDzCMTPXHi3iIFG/KSzMJzLG8W AhatYLcB+kUXAKznK3u3u/m2W4Jew9zlNb7fCYe9pxitOjrMJKZjVwSyeAN9cPVKSPxV Nf7Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=ZiaazV5F; 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 m6si14288639pgc.234.2018.04.25.15.37.10; Wed, 25 Apr 2018 15:37:24 -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=ZiaazV5F; 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 S1753683AbeDYWf6 (ORCPT + 99 others); Wed, 25 Apr 2018 18:35:58 -0400 Received: from mail-ot0-f195.google.com ([74.125.82.195]:37548 "EHLO mail-ot0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751563AbeDYWf4 (ORCPT ); Wed, 25 Apr 2018 18:35:56 -0400 Received: by mail-ot0-f195.google.com with SMTP id 77-v6so21100246otd.4 for ; Wed, 25 Apr 2018 15:35:56 -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=KyR/yJfkanXQWrCN6t4LO7jOmrz9emLaM97NaI8gLg8=; b=ZiaazV5FH2lvlPwihiO7xb167Et34c76jcoO6gUurCLZQiv07hhsvjC+hlz6zl6c1x 7lVsmdQAmsjIolnInnlDoYKhkDFVsPRMK4BjEj19AhttHgK6uYKhElYXpsIGITapSRlM oA60PCBK2bXtyAx5nlyNrJJioYKdtaW8Czv3rXGXLMSfRNl/MrF1JennOglOnz1LD+Zu 3bAgcgoCdaQghleTLmRaQ6zd1Ds/Y8dz9/PskEzO4hdrraneQCChfCYlGYyEe/ERbTnK c8Rg+XRa22igqHDywe1kH3QfzM2JzCBSILI4qsPKXQ5ZznbX1A+SXRePsO2XBOyO1ViS BBEA== 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=KyR/yJfkanXQWrCN6t4LO7jOmrz9emLaM97NaI8gLg8=; b=sMIoktVoAqSbWCrH6i/5eib9+Ha/ILU3FzSK7WG15dN9pIlwgRE1ViVtIlUNgd8qaE 1jYpmoh+n/gNJG5wu+GperZ2dCEQWKUSF1ZmX28iR17OU2N08/vZY0dJjixXcy+Ag2vk kTxBNEsCE4yQicKu1h5UIrWUCt/8ypFTOiZYl8C5Q7YCtinfoZCOVYt9tQaDidoKI+oz PbspTuUO6n69ZKR5uBUOouhtX6h090MtUKSM6T6Klt8XGxkWHlWSM9FTpmykO+Bkpg/8 1O3ZhBT5rE4K1KdXUnWkKoWL0A2QNDH7XGYFwfPpcxzi/tDK4QftG5n5UopH4AiMqxcF hJBA== X-Gm-Message-State: ALQs6tBirGSAtA3mRvdYexW5JgZF5GBB+IRofnJSz2C/xL29DkzgBW/w JAHRCJOwvIxEgTVL7/+chSpXcQPUSxcJIPJ6Bf4= X-Received: by 2002:a9d:2d83:: with SMTP id g3-v6mr19158699otb.259.1524695755938; Wed, 25 Apr 2018 15:35:55 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a9d:2c03:0:0:0:0:0 with HTTP; Wed, 25 Apr 2018 15:35:55 -0700 (PDT) In-Reply-To: <20180424172445.31928-2-jorge.sanjuan@codethink.co.uk> References: <20180420170327.31569-1-jorge.sanjuan@codethink.co.uk> <20180424172445.31928-1-jorge.sanjuan@codethink.co.uk> <20180424172445.31928-2-jorge.sanjuan@codethink.co.uk> From: Ruslan Bilovol Date: Thu, 26 Apr 2018 01:35:55 +0300 Message-ID: Subject: Re: [alsa-devel] [PATCH v2 1/4] ALSA: usb-audio: UAC3. Add support for mixer unit. To: Jorge Sanjuan Cc: Takashi Iwai , Greg Kroah-Hartman , alsa-devel@alsa-project.org, 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 Tue, Apr 24, 2018 at 8:24 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. The patch looks OK in general, but I have few comments > > Signed-off-by: Jorge Sanjuan > --- > include/uapi/linux/usb/audio.h | 13 +++++-- > sound/usb/mixer.c | 87 ++++++++++++++++++++++++++++++++++++++++-- > 2 files changed, 93 insertions(+), 7 deletions(-) > > diff --git a/include/uapi/linux/usb/audio.h b/include/uapi/linux/usb/audio.h > index 3a78e7145689..f9be472cd025 100644 > --- a/include/uapi/linux/usb/audio.h > +++ b/include/uapi/linux/usb/audio.h > @@ -285,9 +285,16 @@ 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) Name of this function is ambiguous, that's because UAC1 mixer unit has only bmControls bitmap, whereas UAC2/3 mixer unit has two bitmaps (bmMixerControls and bmControls), in latter case this func returns pointer to bmMixerControls. Maybe in the future we will need to rename it, but at least having comment which clarifies that in case of UAC2/3 this function actually returns pointer to bmMixerControls here will be helpful for code readers. > { > - 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 __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..bf701b466a4e 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, > + le16_to_cpu(desc->baSourceID[desc->bNrInPins])); It would be good to create and use some helper here, for example implement uac3_mixer_unit_wClusterDescrID() similar to uac_mixer_unit_bNrChannels(). It will put all this conversation to a single place and will improve code readability. > + 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; > } > @@ -1801,7 +1873,7 @@ static void build_mixer_unit_ctl(struct mixer_build *state, > struct usb_audio_term *iterm) > { > struct usb_mixer_elem_info *cval; > - unsigned int num_outs = uac_mixer_unit_bNrChannels(desc); > + unsigned int num_outs; > unsigned int i, len; > struct snd_kcontrol *kctl; > const struct usbmix_name_map *map; > @@ -1814,6 +1886,10 @@ static void build_mixer_unit_ctl(struct mixer_build *state, > if (!cval) > return; > > + num_outs = uac_mixer_unit_get_channels(state, desc); > + if (num_outs < 0) > + return; This will produce an extra USB control request in UAC3 case, which we can avoid if add num_outs to input parameters of build_mixer_unit_ctl() function. This function is used only once inside of parse_audio_mixer_unit() which already has channels number request and all needed checks. Thanks, Ruslan > + > snd_usb_mixer_elem_init_std(&cval->head, state->mixer, unitid); > cval->control = in_ch + 1; /* based on 1 */ > cval->val_type = USB_MIXER_S16; > @@ -1878,14 +1954,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++) { > -- > 2.11.0 > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel