If it was just moving common parts, I would have no issue. My main objection is that you went one step further and started renaming stuff in rather confusing ways, e.g.
-void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev) +void sof_hda_bus_init(struct hda_bus *bus, struct pci_dev *pdev,
- snd_hdac_ext_bus_init - initialize a HD-audio extended bus
- snd_hda_ext_bus_init - initialize a HD-audio extended bus
Reason for renaming snd_hdac_ext_bus_init() to snd_hda_ext_bus_init() is argument type change: 'struct hdac_bus *' to struct hda_bus *'. Don't believe that's confusing to anybody.
it is confusing to me, myself and I. The main point is that it blurs layers.
The hdaudio_ext library deals with Intel controller extensions for DMA management and does not need to know about a larger container.
No problem with reverting naming change for now - we can streamline naming later.
In regard to sof_hda_bus_init(): I don't see any renaming here, just argument type changes to match new snd_hda_ext_bus_init() usage.
hda_bus is a definition from hda_codec.h, I don't see a reason why we should use this structure when the vast majority of the code uses hdac_bus. And the purpose of hdac_ext is really to deal with *controller* extensions to couple/decouple DMAs. There is no dependency on codecs at that level.
hda_bus is the base for all HDAudio drivers: struct azx struct skl_dev struct sof_intel_hda_dev
So no, what vast majority of code actually does is hda_bus/codec to hdac_bus/codec (and vice-versa) translation when in fact all the drivers are hda_* based. If you speak of confusion, that's the confusion that should be addressed in the future..
I maintain that the hdaudio_ext library is about Intel-specific changes on the controller level and only with DMAs.
/** * hdac_ext_stream: HDAC extended stream for extended HDA caps * * @hstream: hdac_stream * @pphc_addr: processing pipe host stream pointer * @pplc_addr: processing pipe link stream pointer * @spib_addr: software position in buffers stream pointer * @fifo_addr: software position Max fifos stream pointer * @dpibr_addr: DMA position in buffer resume pointer * @dpib: DMA position in buffer * @lpib: Linear position in buffer * @decoupled: stream host and link is decoupled * @link_locked: link is locked * @link_prepared: link is prepared * @link_substream: link substream */
It's not really a surprise that there's confusion, even the HDaudio spec describes controller, DMA, link and codec without clearly calling out boundaries between layers.
Even if there was, I also don't really see why/when we should start using hda_ext v. hdac_ext, the naming convention completely escapes me. It would seem logical to me to only use hdac_ext as an extension of hdac_, no?
I also don't get what this means: + snd_hda_ext_bus_init(hbus, pci, NULL, ext_ops, "sklbus");
what is 'sklbus' and what purpose are you trying to accomplish with this change?
Well, please see the updated declaration of snd_hda_ext_bus_init() in this very patch and then the existing code of sound/soc/intel/skylake/skl.c - skl_create(). Last argument in updated declaration reads 'modelname'. Skylake-driver has its own, SOF initializes it differently.
Not sure why you have your own?