Re: [PATCH] ASoC: Intel: Unify HDAudio-ext bus initialization

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

>> hmm. It's not clear whether we should initialize the regular hdac_bus
>> fields in the ext_bus_init(). For plain HDA, these are also initialized
>> in the caller. E.g. in sound/pci/hda/hda_controller.c.
>>
>> So if we start cleaning up, should we go further put this in
>> snd_hdac_bus_init()?
>>
>> Then another is what is the right place for settings like "sync_write =
>> 1". While this settings applies to all current users of the extended
>> bus, this is really hw specific setting and not really a property of
>> the extended bus, so this feels a bit out-of-place.
> 
> I'm rather in favor of bigger cleanups. We can definitely move further
> in the future : )

I am not opposed to updates in this hdaudio-ext part, but I am in favor
of less ambitious step-by-step changes.

a) This is legacy code where the initial authors have moved on, and it's
very hard to figure out what the intent was. It's quite common to have
discussion on feature v. bug, see e.g. the discussion we had on this in
https://github.com/thesofproject/linux/pull/3175#pullrequestreview-772674119

b) In addition, this code is shared between two teams with separate
testing/CI capabilities and limited number of commercial/shipping
devices. There is a very large risk of introducing bugs even with the
best intentions.

Before we do any changes in functionality, there are already basic steps
that can be done, e.g. using consistent naming between variables, the
existing code is just confusing as can be:

struct hdac_stream *stream;
struct hdac_ext_stream *stream;
struct hdac_stream *hstream;
struct hdac_ext_stream *hstream;

I started basic cleanups here:
https://github.com/thesofproject/linux/pull/3158, I haven't had time to
complete this because of more important locking issues, but I intend to
send those clarifications for the next cycle.

Again before we do large changes let's think of smaller steps and how we
are going to validate those changes.