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
