Re: [PATCH 2/4] ALSA: hda: Make snd_hda_codec_device_init() the only codec constructor

[Cezary Rojewski <cezary.rojewski@intel.com>]

On 2022-07-20 6:00 PM, Pierre-Louis Bossart wrote:
> On 7/20/22 10:39, Cezary Rojewski wrote:
>> On 2022-07-20 3:13 PM, Takashi Iwai wrote:
>>> On Wed, 20 Jul 2022 15:06:20 +0200,
>>> Cezary Rojewski wrote:
>>>>
>>>> Refactor snd_hdac_ext_bus_device_init() so that it makes use of
>>>> snd_hda_codec_device_init() to create and initialize new codec device.
>>>> Causes the latter to become the sole codec device constructor.
>>>>
>>>> Users of the refactored function are updated accordingly and now also
>>>> take responsibility for assigning driver's private data as that task is
>>>> no longer performed by hdac_hda_dev_probe().
>>>
>>> Hrm, this doesn't look really right.  It means you'll introduce a hard
>>> dependency chain in a reverse order: snd-hda-ext-core ->
>>> snd-hda-codec.
>>>
>>> Originally, the ext bus code was written completely independent from
>>> the legacy HD-audio implementations, and hdac-hda driver was a kind of
>>> wrapper / bridge for the legacy codec over the ext bus.  If we want
>>> change this rule and make the legacy HD-audio codec always tied with
>>> the ext bus, a likely better way would be to call
>>> snd_hda_codec_device_init() in the caller's side (e.g. skl or sof),
>>> then pass the newly created codec object to
>>> snd_hdac_ext_bus_device_init() for further initialization.
>>
>>
>> Agree. That or drop the snd_hdac_ext_bus_device_init() entirely. Will
>> send an update soon.
>>
>> In regard to the other subject, my plan:
>> - separate code used by both ALSA/ASoC into sound/hda (this includes
>> many hda_codec functions)
>> - combine hda_bus and hdac_bus
>> - combine hda_codec and hdac_device
>> - drop HDA_DEV_ASOC
>> - drop hdac_hda/hdac_hdmi (once skylake-driver is gone; sof will be
>> updated accordingly)
> 
> the skylake driver cannot be removed until you have evidence that users
> have switched, and SOF has other priorities that will likely conflict
> with that goal. I don't even know what this 'drop hdac_hda' idea means
> in detail, we need to keep an ASoC-based codec and the split between
> platform/codec/machine. We are not going to move the HDaudio codec
> management logic inside the SOF driver if that was the intent. The SOF
> driver will focus on host/controller/DSP handling.


The evidence will be there : )
Also, there is nothing stopping us from adjusting skylake-driver however 
we see fit along the road. sound/soc/codecs/hda.c is a clear winner here.

And SOF is just breaking compatibility in several places due to IPC4 and 
stuff, no? There is no reason not to update sof along the road too - so 
it makes use of the aforementioned codec driver.

>> <story does not end here>
> 
> I strongly recommend that we add no dependencies between hdac_ext and
> hda_codec. To be clearer, we don't want to limit the hdac_ext bus and
> stream management to platforms with HDaudio codecs.

Yeah, we all agree here - snd_hdac_ext_bus_device_init() will be 
removed. snd_hda_codec_device_init() + registration will be used 
directly instead.