From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 256D8C2B9F4 for ; Tue, 22 Jun 2021 09:02:49 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id EFA2061353 for ; Tue, 22 Jun 2021 09:02:47 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EFA2061353 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id A9C8E1677; Tue, 22 Jun 2021 11:01:55 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz A9C8E1677 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1624352565; bh=jOkg1XrrVydpr8F5cE5nj4Nz6TPLL/28piFyYA9r1jI=; h=Date:From:To:Subject:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=bazeBso4PebpMx0QjVCx2WQyDXN0wA3jRrMCN1pV2duRG4p0RHcuQ1uI9HGwt75ND iThk2gScqqC5HdtHihZ3+VqquGn4bBFIXEh2hrlvVtFSOHol9tPa1wSLHdMOYJCq90 62sBIBj+p9f/GJGk3Xsg5WSbwKW1aZoWUZoGomFc= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id C386DF8025F; Tue, 22 Jun 2021 11:01:54 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id A3F3DF80268; Tue, 22 Jun 2021 11:01:53 +0200 (CEST) Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 33F7FF80161 for ; Tue, 22 Jun 2021 11:01:50 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 33F7FF80161 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="MaLevsz2"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="HVLt743C" Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id A8D6E1FD5D; Tue, 22 Jun 2021 09:01:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1624352510; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=5fYcpaOJ9VD/NjzWFRUWGSwDUZnJ+wBjN1dOjE0HI3M=; b=MaLevsz2l8WHJREP+0IMO+euOn9zN3LQwJZjiSjrCR2/oQ6NmMs4xFrGU268p7jseZyp0J QEzU6yoiLtJpPRR+M5rKfpeuT/PmsG1czKwOp2ahciZPm4OOEYZ9XEZT+b7RJAJpyN+y/R WqFAt7gY7/+ASw2w42UjkRCwumOg7WQ= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1624352510; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=5fYcpaOJ9VD/NjzWFRUWGSwDUZnJ+wBjN1dOjE0HI3M=; b=HVLt743CZ+9ZbZCDqaMFn4H7MpkFxvQuBNICbAyg1SQHxOyT8TrynFSyXjlXPfBm3i/1A4 /KX490rCUXHG9nBA== Received: from alsa1.suse.de (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id A14D5A3B84; Tue, 22 Jun 2021 09:01:50 +0000 (UTC) Date: Tue, 22 Jun 2021 11:01:50 +0200 Message-ID: From: Takashi Iwai To: "Geoffrey D. Bennett" Subject: Re: [PATCH 16/31] ALSA: usb-audio: scarlett2: Add Gen 3 mixer support In-Reply-To: References: <0b00f3a5-fe31-0ad5-c723-d354dc724e58@gmail.com> <20210622074454.GB13614@m.b4.vu> <20210622081839.GA13849@m.b4.vu> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Cc: Hin-Tak Leung , alsa-devel@alsa-project.org, Vladimir Sadovnikov X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On Tue, 22 Jun 2021 10:34:54 +0200, Takashi Iwai wrote: > > On Tue, 22 Jun 2021 10:18:39 +0200, > Geoffrey D. Bennett wrote: > > > > On Tue, Jun 22, 2021 at 09:58:27AM +0200, Takashi Iwai wrote: > > > On Tue, 22 Jun 2021 09:44:54 +0200, > > > Geoffrey D. Bennett wrote: > > > > > > > > On Tue, Jun 22, 2021 at 09:34:25AM +0200, Takashi Iwai wrote: > > > > > On Tue, 22 Jun 2021 09:07:20 +0200, > > > > > Vladimir Sadovnikov wrote: > > > > > > > > > > > > Hello Takashi! > > > > > > > > > > > > Since Focusrite devices are too advanced in settings, the overall > > > > > > amount of 256 controls is not enough for these devices (like 18i20). > > > > > > I would like also to extend this constant up to 1024 or even more > > > > > > since adding support of software configuration of the device also > > > > > > can exceed the amount of 512 control elements. > > > > > > > > > > This define isn't for the total number of mixer elements. Instead, > > > > > it's just a size of the bitmap table that contains the head of the > > > > > linked list for each unit id (in the sense of USB mixer spec). > > > > > So the number of mixer elements is unlimited. > > > > > > > > Sorry I don't understand what's going on then. Am I calling > > > > snd_usb_mixer_add_control() wrong? Because when I called it more than > > > > MAX_ID_ELEMS times I got a buffer overflow in mixer->id_elems[] (from > > > > memory, I can confirm tonight). > > > > > > > > snd_usb_create_mixer() has: > > > > > > > > mixer->id_elems = kcalloc(MAX_ID_ELEMS, sizeof(*mixer->id_elems), > > > > GFP_KERNEL); > > > > > > > > snd_usb_mixer_add_control() called from mixer_scarlett_gen2.c ends up > > > > at snd_usb_mixer_add_list() which does: > > > > > > > > list->next_id_elem = mixer->id_elems[list->id]; > > > > mixer->id_elems[list->id] = list; > > > > > > > > And list->id was going over MAX_ID_ELEMS. > > > > > > Here, list->id is the *USB* mixer unit id, not the ALSA control id or > > > whatever the internal index. The former should be a byte, hence it > > > can't be over 256. > > > > > > That said, the scarlett2 code calls the function in a wrong way. It > > > has worked casually, so far, just because the core code doesn't use > > > the unit id number for significant roles. > > > > > > So, as a quick workaround, simply pass 0 or any fixed number under 256 > > > to list->id (i.e. elem->head.id in scarlett2_add_new_ctl()). That's > > > all, and the elements are chained in the linked list. > > > > Okay, I will fix that tonight. > > > > Were patches 1-15 of this set of 31 acceptable? If so, I will send a > > new set with this fix and the remainder of the patches. > > I don't see other issues through a quick glance. > > And, the additional fix is below. > If this works, please include this at first. It was missing one piece. The revised patch is below. Takashi -- 8< -- From: Takashi Iwai Subject: [PATCH v2] ALSA: usb-audio: scarlett2: Fix wrong resume call The current way of the scarlett2 mixer code managing the usb_mixer_elem_info object is wrong in two ways: it passes its internal index to the head.id field, and the val_type field is uninitialized. This ended up with the wrong execution at the resume because a bogus unit id is passed wrongly. Also, in the later code extensions, we'll have more mixer elements, and passing the index will overflow the unit id size (of 256). This patch corrects those issues. It introduces a new value type, USB_MIXER_BESPOKEN, which indicates a non-standard mixer element, and use this type for all scarlett2 mixer elements, as well as initializing the fixed unit id 0 for avoiding the overflow. Cc: Signed-off-by: Takashi Iwai --- sound/usb/mixer.c | 3 +++ sound/usb/mixer.h | 1 + sound/usb/mixer_scarlett_gen2.c | 7 ++++++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index 428d581f988f..0f578dabd094 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -3605,6 +3605,9 @@ static int restore_mixer_value(struct usb_mixer_elem_list *list) struct usb_mixer_elem_info *cval = mixer_elem_list_to_info(list); int c, err, idx; + if (cval->val_type == USB_MIXER_BESPOKEN) + return 0; + if (cval->cmask) { idx = 0; for (c = 0; c < MAX_CHANNELS; c++) { diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h index e5a01f17bf3c..ea41e7a1f7bf 100644 --- a/sound/usb/mixer.h +++ b/sound/usb/mixer.h @@ -55,6 +55,7 @@ enum { USB_MIXER_U16, USB_MIXER_S32, USB_MIXER_U32, + USB_MIXER_BESPOKEN, /* non-standard type */ }; typedef void (*usb_mixer_elem_dump_func_t)(struct snd_info_buffer *buffer, diff --git a/sound/usb/mixer_scarlett_gen2.c b/sound/usb/mixer_scarlett_gen2.c index 2e1937b072ee..ed2f16a5fc87 100644 --- a/sound/usb/mixer_scarlett_gen2.c +++ b/sound/usb/mixer_scarlett_gen2.c @@ -1070,10 +1070,15 @@ static int scarlett2_add_new_ctl(struct usb_mixer_interface *mixer, if (!elem) return -ENOMEM; + /* We set USB_MIXER_BESPOKEN type, so that the core USB mixer code + * ignores them for resume and other operations. + * Also, the head.id field is set to 0, as we don't use this field. + */ elem->head.mixer = mixer; elem->control = index; - elem->head.id = index; + elem->head.id = 0; elem->channels = channels; + elem->val_type = USB_MIXER_BESPOKEN; kctl = snd_ctl_new1(ncontrol, elem); if (!kctl) { -- 2.26.2