On Tue, 08 Feb 2022 17:34:47 +0100, Cezary Rojewski wrote:
On 2022-02-08 4:54 PM, Kai Vehmanen wrote:
Hi,
On Mon, 7 Feb 2022, Cezary Rojewski wrote:
With few changes, snd_hda_codec_register() and its unregister-counterpart can be re-used by ASoC drivers. While at it, provide kernel doc for the exposed functions.
thanks, there is no doubt room to improve the HDA<->asoc interaction still and increase reuse. Some questions:
Thanks for taking time in reviewing the patches, Kai!
Due to ALSA-device vs ASoC-component organization differences, new 'snddev_managed' argument is specified allowing for better control over codec registration process.
Can you maybe clarify this? The existing code to handle the different paths is already quite hairy (e.g. code in hda_codec.c:snd_hda_codec_dev_free() that does different actions depending on whether "codec->core.type == HDA_DEV_LEGACY) is true or false), so we'd need to have very clear semantics for the "snddev_managed".
It's rather straightforward - ASoC does not provide you with pointer to struct snd_card until all components are accounted for. snd_hda_codec_device_new() in its current form needs such information upfront though. As we want to create only as many DAIs (and other ASoC components like links and routes) as needed, codec's ->pcm_list_head needs to be built before codec's probing can be completed. But in order to have hda codec to fill ->pcm_list_head for, you need to create it. And in order to create it you need snd_card pointer which ASoC won't give you before you complete component probing.
Typical chicken and egg problem. And that's why additional option is provided - it solves that problem.
Hmm, but snd_hda_codec_device_new() also does the actual hardware initialization including the power-up sequence, and there is a call snd_component_add() with the card instance. How the function would work properly before the card instantiation? You seem to have handled only snd_device_new() and not touching other code where the card (or the hardware access) is involved.
If the purpose is to get the fields of snd_hda_codec to be initialized, those init code can be factored out to another function, so that it works certainly without card.
Takashi