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=-2.3 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 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 5F6ADC433E0 for ; Wed, 27 May 2020 01:18:23 +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 DCD3A208FE for ; Wed, 27 May 2020 01:18:22 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=alsa-project.org header.i=@alsa-project.org header.b="chRRoLKh" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DCD3A208FE Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.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 33CBA1759; Wed, 27 May 2020 03:17:31 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 33CBA1759 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1590542301; bh=wXiIZEPgA+o+gtexV3R/og++K1+zsumzx+CVsLYigm0=; h=Subject:To:References:From:Date:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=chRRoLKhgAJokDbC1yi0nAZnHdkUrBm9rcVpqnEzEzgEwK/wgFD7J9+H4IAXMMXRT mA2XTOVhgRTceH4PLydAAd93ZOGyZfcGjmz/9ho6x67pGKd+U6fBrY7/KksFQIMSpp U0k9/361aCegBTeL2AimTpWKNagZEx1e5AGL0EHE= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id BF07CF8014A; Wed, 27 May 2020 03:17:30 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 10456F80150; Wed, 27 May 2020 03:17:29 +0200 (CEST) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) (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 59199F80100 for ; Wed, 27 May 2020 03:17:24 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 59199F80100 IronPort-SDR: 0HqTfDJrA/0kWQQYYOL49O0Cdq4B/pPbMzvLbnladKCAHj5FCpLSlrX1hTYoivhD5YcU0mNJdA +5e7TUCoKERw== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 May 2020 18:17:21 -0700 IronPort-SDR: yGjtoHow/qvLRoXWui+jdWENt1JP375Wm5kR8NkxXI9AFbZoEvjlnQnQDzwvynT24ZuDuT+LOk Ckt36L2ctO1Q== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,439,1583222400"; d="scan'208";a="345363727" Received: from xgu17-mobl.ccr.corp.intel.com (HELO [10.254.210.123]) ([10.254.210.123]) by orsmga001.jf.intel.com with ESMTP; 26 May 2020 18:17:19 -0700 Subject: Re: [PATCH v2 1/2] ASoC: topology: refine and log the header in the correct pass To: Cezary Rojewski , alsa-devel@alsa-project.org References: <20200521074847.71406-1-yang.jie@linux.intel.com> <20200521074847.71406-2-yang.jie@linux.intel.com> <4c05fcc4-2f94-1ca0-2148-af70ef739d00@intel.com> From: Keyon Jie Message-ID: Date: Wed, 27 May 2020 09:17:19 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Cc: tiwai@suse.de, broonie@kernel.org, pierre-louis.bossart@linux.intel.com, ranjani.sridharan@linux.intel.com 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 5/26/20 11:30 PM, Cezary Rojewski wrote: > On 2020-05-26 4:45 PM, Keyon Jie wrote: >> On 5/26/20 8:38 PM, Cezary Rojewski wrote: >>> On 2020-05-21 9:48 AM, Keyon Jie wrote: > >>> By having "log" code here we have one place for hdr validation, >>> rather than two (the second being just an "if" to be fair..) and >>> private array is no longer necessary. Local func ptr variable would >>> take care of storing adequate function to call. >> >> Hi Cezary, so what you suggested above is changing the >> soc_tplg_load_header() to be something like this, right? >> >> >> static int soc_tplg_load_header(struct soc_tplg *tplg, >>      struct snd_soc_tplg_hdr *hdr) >> { >>      unsigned int hdr_pass; >>      int (*elem_load)(struct soc_tplg *, struct snd_soc_tplg_hdr *); >> >>      tplg->pos = tplg->hdr_pos + sizeof(struct snd_soc_tplg_hdr); >> >>      /* check for matching ID */ >>      if (le32_to_cpu(hdr->index) != tplg->req_index && >>          tplg->req_index != SND_SOC_TPLG_INDEX_ALL) >>          return 0; >> >>      tplg->index = le32_to_cpu(hdr->index); >> >>      switch (le32_to_cpu(hdr->type)) { >>      case SND_SOC_TPLG_TYPE_MIXER: >>      case SND_SOC_TPLG_TYPE_ENUM: >>      case SND_SOC_TPLG_TYPE_BYTES: >>          hdr_pass = SOC_TPLG_PASS_MIXER; >>          elem_load = soc_tplg_kcontrol_elems_load; >>          break; >>      case SND_SOC_TPLG_TYPE_DAPM_GRAPH: >>          hdr_pass = SOC_TPLG_PASS_GRAPH; >>          elem_load = soc_tplg_dapm_graph_elems_load; >>          break; >>      case SND_SOC_TPLG_TYPE_DAPM_WIDGET: >>          hdr_pass = SOC_TPLG_PASS_WIDGET; >>          elem_load = soc_tplg_dapm_widget_elems_load; >>          break; >>      case SND_SOC_TPLG_TYPE_PCM: >>          hdr_pass = SOC_TPLG_PASS_PCM_DAI; >>          elem_load = soc_tplg_pcm_elems_load; >>          break; >>      case SND_SOC_TPLG_TYPE_DAI: >>          hdr_pass = SOC_TPLG_PASS_BE_DAI; >>          elem_load = soc_tplg_dai_elems_load; >>          break; >>      case SND_SOC_TPLG_TYPE_DAI_LINK: >>      case SND_SOC_TPLG_TYPE_BACKEND_LINK: >>          /* physical link configurations */ >>          hdr_pass = SOC_TPLG_PASS_LINK; >>          elem_load = soc_tplg_link_elems_load; >>          break; >>      case SND_SOC_TPLG_TYPE_MANIFEST: >>          hdr_pass = SOC_TPLG_PASS_MANIFEST; >>          elem_load = soc_tplg_manifest_load; >>          break; >>      default: >>          /* bespoke vendor data object */ >>          hdr_pass = SOC_TPLG_PASS_VENDOR; >>          elem_load = soc_tplg_vendor_load; >>          break; >>      } >> >>      if (tplg->pass == hdr_pass) { >>          dev_dbg(tplg->dev, >>              "ASoC: Got 0x%x bytes of type %d version %d vendor %d at >> pass %d\n", >>              hdr->payload_size, hdr->type, hdr->version, >>              hdr->vendor_type, tplg->pass); >>          return elem_load(tplg, hdr); >>      } >> >>      return 0; >> } >> >> >> I am also fine with this, though I thought my previous version looks >> more organized and not so error-prone as we need 8 more assignation here. >> >> Mark, Pierre, preference about this? >> >> Thanks, >> ~Keyon >> > > Another option, if you want to reduce assignment count, is to keep > soc_pass_load while still removing the private map. Said soc_pass_load > would require declaration update to accept function ptr on top of what's > already there. > > In consequence, soc_tplg_load_header becomes a switch-case with a bunch of > case X: >     return soc_pass_load(tplg, hdr, >             pass=_MY_PASS >                 (e.g. SOC_TPLG_PASS_VENDOR), >             elem_load=_MY_LOAD_FUNC >                 (e.g. soc_tplg_vendor_load)) I would prefer to use the assignment one, let me update the series now. Thanks, ~Keyon > > Czarek