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:
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".
@@ -940,12 +953,13 @@ int snd_hda_codec_new(struct hda_bus *bus, struct snd_card *card, return PTR_ERR(codec); *codecp = codec;
- return snd_hda_codec_device_new(bus, card, codec_addr, *codecp);
- return snd_hda_codec_device_new(bus, card, codec_addr, *codecp, false);
So this sets snddev_managed to "false" for snd-hda-intel? How is this expected to work?
int snd_hda_codec_device_new(struct hda_bus *bus, struct snd_card *card,
unsigned int codec_addr, struct hda_codec *codec)
unsigned int codec_addr, struct hda_codec *codec,bool snddev_managed){ char component[31]; hda_nid_t fg; @@ -1020,9 +1034,12 @@ int snd_hda_codec_device_new(struct hda_bus *bus, struct snd_card *card, codec->core.subsystem_id, codec->core.revision_id); snd_component_add(card, component);
[...]
- err = snd_device_new(card, SNDRV_DEV_CODEC, codec, &dev_ops);
- if (err < 0)
goto error;
- if (snddev_managed) {
/* ASoC features component management instead */err = snd_device_new(card, SNDRV_DEV_CODEC, codec, &dev_ops);if (err < 0)goto error;- }
I might misunderstand semantics of snddev_managed here, but given in case of non-ASoC use, snddev_managed is false, this would mean we don't call snd_device_new() at all...? I don't see where this is added elsewhere in the series, so this would seem to break the flow for non-ASoC case.
Br, Kai