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=-3.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no 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 F36D5C433DB for ; Fri, 29 Jan 2021 15:13:58 +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 6394164E02 for ; Fri, 29 Jan 2021 15:13:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6394164E02 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 EF13516CF; Fri, 29 Jan 2021 16:13:06 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz EF13516CF DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1611933237; bh=9WwsOeID7TfkDSLybcL3EaHmzd7I17eczuuIVbJilDo=; h=Date:From:To:Subject:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=X3c3dZ+NtTFYmbIz8wi3dWPcCNLxwNciKBZnan/HRebDHFduHX4KfCp4XUTHJlZL/ AhutaIzK2vmFaQ24gWgN/c3rhuYPYekefvmN8PAiMKWsONoJ1MBWkqP2z03dmZMZMg uY3zRLpUvL0wMqWFf/i1fAlpST4leZEiqZW/9wi0= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 8270BF80259; Fri, 29 Jan 2021 16:13:06 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id AD554F8025F; Fri, 29 Jan 2021 16:13:04 +0100 (CET) Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 14BBCF80130 for ; Fri, 29 Jan 2021 16:13:01 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 14BBCF80130 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 28155ABD6; Fri, 29 Jan 2021 15:13:01 +0000 (UTC) Date: Fri, 29 Jan 2021 16:13:01 +0100 Message-ID: From: Takashi Iwai To: Fabian Lesniak Subject: Re: [PATCH] ALSA: usb-audio: Add DJM750 to Pioneer mixer quirk In-Reply-To: <3031135.XsSY7s2paC@artex> References: <20210128160338.dac4vrj7wjiykcxm@base.nu> <3031135.XsSY7s2paC@artex> 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: alsa-devel@alsa-project.org, Olivia Mackintosh , =?UTF-8?B?RnJhbnRp77+977+9ZWsgS3Xvv73vv71lcmE=?= 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 Fri, 29 Jan 2021 15:09:11 +0100, Fabian Lesniak wrote: > > Hi Olivia, > > perfect time for this patch since I'm currently working on similar quirks for > the DJM-900NXS2 model. I will stick to your method for now. I do have some > minor comments below. > > In general, I'm wondering whether it is a good way to implement more and more > Pioneer devices in such a hard coded way. mixer_quirks.c already has >3k LOC, > and the 900NXS2 support will add at least 100 more if written in the same > scheme. It may be good to either dynamically create controls depending on the > model or move pioneer support to an extra file. I'd like to hear what Takashi > thinks about that. If we can reduce the code, it's really appreciated. But I'm afraid that we won't reduce much for now, and the current amount is still manageable. Let's see how much we need for 900NXS2. thanks, Takashi > > Cheers > Fabian > > > +static const struct snd_pioneer_djm_device snd_pioneer_djm_devices[] = { > > + { .name = "DJM-250Mk2", .controls = snd_pioneer_djm250mk2_option_groups, .ncontrols = 7}, > > + { .name = "DJM-750", .controls = snd_pioneer_djm750_option_groups, .ncontrols = 5} > > +}; > These fixed values for ncontrols can easily be overlooked, consider ARRAY_SIZE > instead. Maybe introduce a macro similar to snd_pioneer_djm_option_group_item. > > > + const struct snd_pioneer_djm_device device = snd_pioneer_djm_devices[device_idx]; > This makes a local copy, which can be avoided by using a pointer instead: > const struct snd_pioneer_djm_device *device = &snd_pioneer_djm_devices[device_idx]; > > > usb_mixer_interface *mixer, u1 err = snd_usb_ctl_msg( > > mixer->chip->dev, usb_sndctrlpipe(mixer->chip->dev, 0), > > USB_REQ_SET_FEATURE, > > - USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE, > > - snd_pioneer_djm_option_groups[group].options[value].wValue, > > - snd_pioneer_djm_option_groups[group].options[value].wIndex, > > + USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE, > > device.controls[group].options[value].wValue, > > + device.controls[group].options[value].wIndex, > > NULL, 0); > Rather keep these arguments aligned. > > > - err = snd_pioneer_djm_controls_create(mixer); > > + err = snd_pioneer_djm_controls_create(mixer, 0x00); > > + break; > > + case USB_ID(0x08e4, 0x017f): /* Pioneer DJ DJM-750 */ > > + err = snd_pioneer_djm_controls_create(mixer, 0x01); > > break; > I'd introduce defines for the different models instead of raw values. > >