From patchwork Thu Nov 30 12:24:32 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vinod Koul X-Patchwork-Id: 10084733 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id EE9F76035E for ; Thu, 30 Nov 2017 12:21:15 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DE3D029F44 for ; Thu, 30 Nov 2017 12:21:15 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D267829F74; Thu, 30 Nov 2017 12:21:15 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE autolearn=unavailable version=3.3.1 Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0B5D329F44 for ; Thu, 30 Nov 2017 12:21:14 +0000 (UTC) Received: from alsa0.perex.cz (localhost [127.0.0.1]) by alsa0.perex.cz (Postfix) with ESMTP id B6D69267996; Thu, 30 Nov 2017 13:21:13 +0100 (CET) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id E78C2267996; Thu, 30 Nov 2017 13:21:11 +0100 (CET) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by alsa0.perex.cz (Postfix) with ESMTP id 5955F2679A7 for ; Thu, 30 Nov 2017 13:21:00 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 30 Nov 2017 04:20:59 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,341,1508828400"; d="scan'208";a="8862648" Received: from vkoul-udesk7.iind.intel.com (HELO localhost) ([10.223.84.143]) by fmsmga001.fm.intel.com with ESMTP; 30 Nov 2017 04:20:56 -0800 Date: Thu, 30 Nov 2017 17:54:32 +0530 From: Vinod Koul To: Pierre-Louis Bossart Message-ID: <20171130122432.GX32417@localhost> References: <20171129014551.30552-1-pierre-louis.bossart@linux.intel.com> <20171129014551.30552-2-pierre-louis.bossart@linux.intel.com> <20171129102734.GA32417@localhost> <8ba6af88-5cb4-8174-2ecc-46f80420ff28@linux.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <8ba6af88-5cb4-8174-2ecc-46f80420ff28@linux.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Cc: alsa-devel@alsa-project.org, arnd@arndb.de, tiwai@suse.de, linux-kernel@vger.kernel.org, liam.r.girdwood@linux.intel.com, broonie@kernel.org, andriy.shevchenko@linux.intel.com, torvalds@linux-foundation.org Subject: Re: [alsa-devel] [RFC PATCH v2 1/7] ASoC: Intel: Fix Kconfig with top-level selector X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 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-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Nov 29, 2017 at 08:52:58AM -0600, Pierre-Louis Bossart wrote: > On 11/29/17 4:27 AM, Vinod Koul wrote: > >On Tue, Nov 28, 2017 at 07:45:45PM -0600, Pierre-Louis Bossart wrote: > > > >I am not sure about top level being default to Y... > > It's standard procedure apparently, see Linus/Mark/Takashi's emails. > > > > >> config SND_SST_ATOM_HIFI2_PLATFORM > >> tristate "Intel ASoC SST driver for HiFi2 platforms (*field, *trail)" > >>- depends on SND_SOC_INTEL_SST_TOPLEVEL && X86 > >>+ depends on X86 > >> select SND_SOC_COMPRESS > >>+ select SND_SOC_INTEL_COMMON > >> config SND_SOC_INTEL_SKYLAKE > >> tristate "Intel ASoC SST driver for SKL/BXT/KBL/GLK/CNL" > >>- depends on SND_SOC_INTEL_SST_TOPLEVEL && PCI && ACPI > >>+ depends on PCI && ACPI > >> select SND_HDA_EXT_CORE > >> select SND_HDA_DSP_LOADER > >> select SND_SOC_TOPOLOGY > >> select SND_SOC_INTEL_SST > >>+ select SND_SOC_INTEL_COMMON > >>+ help > >>+ If you have a Intel Skylake/Broxton/ApolloLake/KabyLake/ > >>+ GeminiLake or CannonLake platform with the DSP enabled in the BIOS > >>+ then enable this option by saying Y or m. > > > >this is good stuff, helps in improving UX vastly. Btw can we have select ALL > >machine also as an option for people who dont want to select a specfic one, > >that one will really help for a better UX > > no, we shouldn't select things, especially with the Baytrail legacy which is > kept to avoid breaking existing setups but shouldn't be used any longer. > And with SOF coming, we may have hybrid configurations with SOF used on > Baytrail/Cherrytrail/APL/CNL but SST used on Skylake/Kabylake due to > firmware authentication stuff. Okay I am checking this out. Btw you need to rebase this series I wasn't able to apply to mark/intel/topic So I grabbed the branch from your github tree. Assuming I have the right branch (topic/fix-kconfig) At top level UX is bit confusing: [ ] Intel ASoC SST drivers [*] Intel ASoC machine drivers And selecting first symbol shows me the platforms and once I select platform then machines appear under second menu. Now this is really not a great way from UX. Why are we adding platforms selectable? We might be able to simplify this by getting rid of first set and let machine then select Platfroms specfied. [*] Intel ASoC SST drivers < > Intel ASoC SST driver for Haswell/Broadwell (NEW) < > Intel ASoC SST driver for Baytrail (legacy) (NEW) < > Intel ASoC SST driver for PCI HiFi2 platforms (Medfield, Merrifield) (NEW) < > Intel ASoC SST driver for ACPI HiFi2 platforms (Baytrail, Cherrytrail) (NEW) <*> Intel ASoC SST driver for SKL/BXT/KBL/GLK/CNL [*] Intel ASoC machine drivers < > ASoC Audio driver for SKL with RT286 I2S mode (NEW) < > ASoC Audio driver for SKL with NAU88L25 and SSM4567 in I2S Mode (NEW) < > ASoC Audio driver for SKL with NAU88L25 and MAX98357A in I2S Mode (NEW) If we still want both I would like the machine to appear below the respective platform, in above example, I would like to see: [*] Intel ASoC SST drivers < > Intel ASoC SST driver for Haswell/Broadwell (NEW) < > Intel ASoC SST driver for Baytrail (legacy) (NEW) < > Intel ASoC SST driver for PCI HiFi2 platforms (Medfield, Merrifield) (NEW) < > Intel ASoC SST driver for ACPI HiFi2 platforms (Baytrail, Cherrytrail) (NEW) <*> Intel ASoC SST driver for SKL/BXT/KBL/GLK/CNL < > ASoC Audio driver for SKL with RT286 I2S mode (NEW) < > ASoC Audio driver for SKL with NAU88L25 and SSM4567 in I2S Mode (NEW) < > ASoC Audio driver for SKL with NAU88L25 and MAX98357A in I2S Mode (NEW) Then you can get rid of global mach symbol and select platform and then respective machine and better UX :). > >>+endif ## SND_SOC_INTEL_SST_TOPLEVEL > >> # ASoC codec drivers > >> source "sound/soc/intel/boards/Kconfig" > >>+ > >>+# configs common to SST and SOF to compile sound/soc/intel/common > >>+# directory and use matching tables > >>+ > >>+config SND_SOC_INTEL_COMMON > >>+ tristate > >>+ select SND_SOC_ACPI_INTEL_MATCH if ACPI > > > >common selects only MATCH > > COMMON is only there to go compile the sound/soc/intel/common directory. > That's not very useful indeed but otherwise the Makefile doesn't compile the > match tables or the sst-ipc stuff. If you find a better solution I am all > ears. Coming to this topic (my original intention to check this), I found that common symbol is not doing anything expect adding common directory, which is not really needed as we have individual bits for files, so I kept this but removed the match one. -- >8 -- -obj-$(CONFIG_SND_SOC_ACPI_INTEL_MATCH) += snd-soc-acpi-intel-match.o +obj-$(CONFIG_SND_SOC_INTEL_COMMON) += snd-soc-acpi-intel-match.o -- >8 -- This way we have one lesser symbol to manage :) I am okay if you want to keep match and remove common. > >>+ # this option controls the compilation of the sound/soc/intel/common > >>+ # directory and is not meant to be selected by the user. It is > >>+ # not filtered out on purpose by the top-level selector since > >>+ # it will be selected by SST or SOF platform driver options > >>+ > >>+config SND_SOC_ACPI_INTEL_MATCH > >>+ tristate > >>+ select SND_SOC_ACPI if ACPI > > > >then why keep common, lets remove one level and have > >SND_SOC_ACPI_INTEL_MATCH selected. ACPI is must have at top level so we can > >add depends on that symbol > > we still have platforms which don't depend on ACPI, so we shouldn't take > this out. For the controllers which are PCI only, yes. But as a platform we won't boot without ACPI :) so take your pick! diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index dee731d42634..33a05aa2cf32 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -99,8 +99,4 @@ source "sound/soc/intel/boards/Kconfig" config SND_SOC_INTEL_COMMON tristate - select SND_SOC_ACPI_INTEL_MATCH if ACPI - -config SND_SOC_ACPI_INTEL_MATCH - tristate - select SND_SOC_ACPI if ACPI + depends on ACPI diff --git a/sound/soc/intel/Makefile b/sound/soc/intel/Makefile index b973d457e834..8160520fd74c 100644 --- a/sound/soc/intel/Makefile +++ b/sound/soc/intel/Makefile @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 # Core support -obj-$(CONFIG_SND_SOC_INTEL_COMMON) += common/ +obj-$(CONFIG_SND_SOC) += common/ # Platform Support obj-$(CONFIG_SND_SOC_INTEL_HASWELL) += haswell/ diff --git a/sound/soc/intel/common/Makefile b/sound/soc/intel/common/Makefile index 7379d8830c39..92c66c47eb78 100644 --- a/sound/soc/intel/common/Makefile +++ b/sound/soc/intel/common/Makefile @@ -8,4 +8,4 @@ snd-soc-acpi-intel-match-objs := soc-acpi-intel-byt-match.o soc-acpi-intel-cht-m obj-$(CONFIG_SND_SOC_INTEL_SST) += snd-soc-sst-dsp.o snd-soc-sst-ipc.o obj-$(CONFIG_SND_SOC_INTEL_SST_ACPI) += snd-soc-sst-acpi.o obj-$(CONFIG_SND_SOC_INTEL_SST_FIRMWARE) += snd-soc-sst-firmware.o