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
