On Fri, 31 May 2019 22:59:17 +0200, Pierre-Louis Bossart wrote: > > > >> we need everything that was removed in your proposal :-) > >> > >> - memset(bus, 0, sizeof(*bus)); > >> - bus->dev = dev; > >> - > >> - bus->io_ops = &io_ops; > >> - INIT_LIST_HEAD(&bus->stream_list); > >> - > >> - bus->irq = -1; > >> - bus->ext_ops = ext_ops; > >> - > >> - /* > >> - * There is only one HDA bus atm. keep the index as 0. > >> - * Need to fix when there are more than one HDA bus. > >> - */ > >> - bus->idx = 0; > >> - > >> - spin_lock_init(&bus->reg_lock); > >> > >> This is the smallest set of initialization needed when you don't need > >> hdmi/hdaudio codec support. > > > > I don't understand it... Why SOF core needs to initialize the content > > of HD-audio bus object even if you won't use it? > > we do use it left and right, but we only use the 'controller/DMA' > parts of that structure. we have zero use for CORB/RIRB and > codec-specific stuff when I2S and DMIC are the only connections to 3rd > party chips So you want to avoid the dependency on snd-hda-core if the system is with only I2S/DMIC? But, snd-sof-intel-hda-common already has the hard dependency on snd-hda-core if CONFIG_SND_SOC_SOF_HDA is enabled. So the attempt makes little sense as long as CONFIG_SND_SOC_SOF_HDA is set. And, if CONFIG_SND_SOC_SOF_HDA isn't set -- i.e. CONFIG_SND_SOC_HF_HDA_LINK isn't set either; it implies that there can be no user of HD-audio bus. So, I see no big reason we need to care about the stuff... > > IOW, what's the merit of having hda-bus.c with the copy of > > snd-hda-core code? As far as I see, both hda.c and hda-bus.c are > > linked into the same snd-sof-intel-hda-common module. And, the former > > has the direct calls of HD-audio core API (with > > CONFIG_SND_SOC_SOF_HDA); i.e. snd-sof-intel-hda-common already depends > > on snd-hda-core if CONFIG_SND_SOC_SOF_HDA is on, no matter how you > > code hda-bus.c. > > I agree we could implement hda-bus in a cleaner way - but it's a very > small file. Yes, but this is a kind of layer violation. Imagine you do initialize the struct pci_dev or whatever else device object because you want to reduce the dependency on other core helpers; it would be nightmare from the maintenance POV. I've had already hard time to figure out why SOF HDA initializes the HD-audio bus, because it misses the explicit snd_hdac_*_bus_init() call. That's the starting point of this thread. So, let's avoid ugly hacks. Make it more straightforward. Again, if the module size matters, we can split and reduce the part of HD-audio core stuff that is directly linked to SOF core. > A larger core repartitioning would take quite a bit of > time, and in the mean time we already have to sort out all the deltas > between legacy driver and hdac library. > > Anyways, that's it for me this week, enjoy your vacation! Thanks! Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
