Re: [PATCH 3/4] ALSA: hda: Update and expose codec register procedures

[Takashi Iwai <tiwai@suse.de>]

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