On 2021-10-21 7:19 PM, Pierre-Louis Bossart wrote:
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.
Ok. Will leave the naming part for future patches while leaving argument list as is.
...
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?
Not sure I understand the question. If you are talking about changing string 'sklbus' to something else, then I don't believe mixing changes: update to actual values assigned and assignment relocation in one patch is good. I used 'sklbus' as that's what is being currently assigned to ->modelname within skl_create(). Such approach makes the change more transparent.
Regards, Czarek