From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: Why open-coding in sof_hda_bus_init()? Date: Fri, 31 May 2019 21:06:25 +0200 Message-ID: References: <1f3059d6-b271-f612-c670-e7214674892f@linux.intel.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx1.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 91C45F80757 for ; Fri, 31 May 2019 21:06:26 +0200 (CEST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" To: Pierre-Louis Bossart Cc: alsa-devel@alsa-project.org, Keyon Jie List-Id: alsa-devel@alsa-project.org On Fri, 31 May 2019 20:31:33 +0200, Pierre-Louis Bossart wrote: > > On 5/31/19 1:22 PM, Takashi Iwai wrote: > > On Fri, 31 May 2019 19:43:59 +0200, > > Pierre-Louis Bossart wrote: > >> > >> On 5/31/19 12:11 PM, Takashi Iwai wrote: > >>> Hi, > >>> > >>> while looking at SOF code due to the recent debugging session, I > >>> noticed that sof_hda_bus_init() is basically an open-code of the > >>> existing snd_hdac_ext_bus_init(). Why don't we simply call > >>> snd_hdac_ext_bus_init() like below? > >> > >> It's intentional. > >> We've been asked since Day1 of SOF on ApolloLake to provide a > >> 'self-contained' controller-only support that has no dependency on the > >> snd_hdac library for solutions where HDaudio links+codecs are not used > >> (typically IOT devices). This was driven by the lack of separation > >> between layers in that library as well as a desire to have a > >> dual-license. That's why you see the init and some of the basic > >> utilities re-implemented for SOF. > >> > >> However for cases where HDaudio+HDMI are required, we didn't want to > >> reinvent the wheel - HDaudio is complicated enough - and do make use > >> of this snd_hdac library. > >> > >> We have a config SND_SOC_SOF_HDA that controls in which mode we > >> operate, and it enables HDMI by default (for I2S+HDMI solutions). To > >> get external HDaudio codecs you need the additional SOF_HDAUDIO_CODEC > >> kconfig. > >> > >> Does this help? > > > > Well, what's wrong with the conditional build with Kconfig? > > You can just wrap the call snd_hdac_ext_bus_init() with #if/endif, > > e.g. in soc/sof/intel/hda.h, > > > > static inline void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev, > > const struct hdac_ext_bus_ops *ext_ops) > > { > > #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) > > snd_hdac_ext_bus_init(bus, dev, NULL, NULL, ext_ops); > > #endif > > } > > We still need initializations for some of the data structures when > SOF_HDA is not defined. Which data structure? The function above is only initializing the given struct hdac_bus object. I'm not suggesting to change the caller site, hda_init() of sound/soc/sof/intel/hda.c. thanks, Takashi