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=-8.3 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable 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 BECFAC54FD0 for ; Mon, 27 Apr 2020 10:59:43 +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 491E32063A for ; Mon, 27 Apr 2020 10:59:43 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=alsa-project.org header.i=@alsa-project.org header.b="qNqGVK4b" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 491E32063A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com 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 936EC167F; Mon, 27 Apr 2020 12:58:51 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 936EC167F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1587985181; bh=ZfB9uUkFhCkVOUFfWE58+63IF+yvAk8PBrRM1eB0XqA=; h=Subject:To:References:From:Date:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=qNqGVK4b/w0Ai3jYsGtV+CvFBSVDr/AZg20/NU2bBoA7yck1fRpAgQ3/jIc9yVygv w0vcLqSQRCsIlSHcbErN81iUkzkaQhZ4/lh9a11ktekbVlRC/su9F5yy2lcCWqhCff A70bdnNaApBWKCVNs9EtfG7SFEKegzW3w4Pb97RA= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 0F515F8022B; Mon, 27 Apr 2020 12:58:51 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 97331F80232; Mon, 27 Apr 2020 12:58:49 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id BE42FF8010A for ; Mon, 27 Apr 2020 12:58:46 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz BE42FF8010A IronPort-SDR: tBzRSvOIOOPXxXwvDUDCu+j14VEGcUoaorqZ420aoGQ/8Ja+EPvFPTL9XFWj1YHAxKFup0e7TT 3tfzYQMPNETQ== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Apr 2020 03:58:45 -0700 IronPort-SDR: /VxuhP7/0fY2Rl+Xtnu7E/Iqt4QccWwkH0YPxd1SHVuRptIaVtx9bKCeRZBO98W066wv3Ml9tx v25cl6sXNh0w== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,323,1583222400"; d="scan'208";a="302340862" Received: from crojewsk-mobl1.ger.corp.intel.com (HELO [10.213.7.127]) ([10.213.7.127]) by FMSMGA003.fm.intel.com with ESMTP; 27 Apr 2020 03:58:42 -0700 Subject: Re: [PATCH 2/3] ASoC: bdw-rt5650: channel constraint support To: Brent Lu , alsa-devel@alsa-project.org References: <1587976638-29806-1-git-send-email-brent.lu@intel.com> <1587976638-29806-3-git-send-email-brent.lu@intel.com> From: Cezary Rojewski Message-ID: Date: Mon, 27 Apr 2020 12:58:41 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <1587976638-29806-3-git-send-email-brent.lu@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Cc: Guennadi Liakhovetski , Kuninori Morimoto , linux-kernel@vger.kernel.org, Jie Yang , Takashi Iwai , Pierre-Louis Bossart , Liam Girdwood , Ben Zhang , Mac Chiang , Mark Brown 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 2020-04-27 10:37, Brent Lu wrote: > BDW boards using this machine driver supports only 2 or 4-channel capture. > Implement a constraint to enforce it. > What about playback configurations? Title for the overall series fits better than the one chosen for actual patches. "channel constraint support" is misleading. Constraints are added or removed but certainly not supported. > Signed-off-by: Brent Lu > --- > sound/soc/intel/boards/bdw-rt5650.c | 34 ++++++++++++++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > > diff --git a/sound/soc/intel/boards/bdw-rt5650.c b/sound/soc/intel/boards/bdw-rt5650.c > index af2f502..dd4f219 100644 > --- a/sound/soc/intel/boards/bdw-rt5650.c > +++ b/sound/soc/intel/boards/bdw-rt5650.c > @@ -21,6 +21,9 @@ > > #include "../../codecs/rt5645.h" > > +#define DUAL_CHANNEL 2 > +#define QUAD_CHANNEL 4 > + Remove, we need not additional too-obvious macro. One could argue 'STEREO' is a better choice for '2' channel configuration too. > struct bdw_rt5650_priv { > struct gpio_desc *gpio_hp_en; > struct snd_soc_component *component; > @@ -162,6 +165,36 @@ static int bdw_rt5650_rtd_init(struct snd_soc_pcm_runtime *rtd) > } > #endif > > +static const unsigned int channels[] = { > + DUAL_CHANNEL, QUAD_CHANNEL, Inline as stated above. > +}; > + > +static const struct snd_pcm_hw_constraint_list constraints_channels = { > + .count = ARRAY_SIZE(channels), > + .list = channels, > + .mask = 0, > +}; > + > +static int bdw_fe_startup(struct snd_pcm_substream *substream) Entire file uses 'bdw_rt5650_' naming convention. Let's not stray away from that path now. > +{ > + struct snd_pcm_runtime *runtime = substream->runtime; > + Missing hw.channels_max assignment from rt5677 - inconsistency/ copy error? > + /* > + * On this platform for PCM device we support, > + * 2 or 4 channel capture > + */ Sometimes you add a newline add and before, while other times just one, before the comment. Please streamline the format across all patches in the series. Comment can be more strict too /* Board supports stereo and quad configurations */ > + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) > + snd_pcm_hw_constraint_list(runtime, 0, > + SNDRV_PCM_HW_PARAM_CHANNELS, > + &constraints_channels); Redesign to reduce indentation and improve readability - if (stream != capture) return 0; return snd_pcm_hw_contraint_list(...); > + > + return 0; > +} > + > +static const struct snd_soc_ops bdw_rt5650_fe_ops = { > + .startup = bdw_fe_startup, > +}; > + > static int bdw_rt5650_init(struct snd_soc_pcm_runtime *rtd) > { > struct bdw_rt5650_priv *bdw_rt5650 = > @@ -234,6 +267,7 @@ static struct snd_soc_dai_link bdw_rt5650_dais[] = { > .name = "System PCM", > .stream_name = "System Playback", > .dynamic = 1, > + .ops = &bdw_rt5650_fe_ops, > #if !IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL) > .init = bdw_rt5650_rtd_init, > #endif >