[alsa-devel] Why open-coding in sof_hda_bus_init()?

[Takashi Iwai]

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