[PATCH 0/4] ALSA: hda: Expose codec organization functions

[PATCH 0/4] ALSA: hda: Expose codec organization functions

[Cezary Rojewski]

Changes expose several function that are currently unavailable for
HDA-DSP drivers for use. Those functions are:

snd_hda_codec_cleanup_for_unbind()
snd_hda_codec_set_power_save()
snd_hda_codec_register()
snd_hda_codec_unregister()
snd_hda_codec_device_init()

This allows upcoming AVS driver [1] to re-use even mode of HDA related
code that is currently available in sound/pci/hda and sound/hda and
prevent any code duplication within avs-driver that would otherwise had
to happen.

Last patch in the series provides snd_hdac_ext_bus_link_at() - a helper
function which allows for retrieval of HDA segment (link) based on codec
address directly. This is simpler than parsing codec-name first to
extract the address what is the case for snd_hdac_ext_bus_get_link().
The latter function is updated to re-use newly added one so core logic
is not duplicated after the addition.

[1]: https://lore.kernel.org/all/20211208111301.1817725-1-cezary.rojewski@intel.com/


Cezary Rojewski (4):
  ALSA: hda: Add snd_hdac_ext_bus_link_at() helper
  ALSA: hda: Update and expose snd_hda_codec_device_init()
  ALSA: hda: Update and expose codec register procedures
  ALSA: hda: Expose codec cleanup and power-save functions

 include/sound/hda_codec.h           | 11 +++-
 include/sound/hdaudio_ext.h         |  1 +
 sound/hda/ext/hdac_ext_controller.c | 31 +++++++---
 sound/pci/hda/hda_codec.c           | 94 ++++++++++++++++++++---------
 sound/pci/hda/hda_local.h           |  2 -
 sound/soc/codecs/hdac_hda.c         |  2 +-
 6 files changed, 100 insertions(+), 41 deletions(-)

-- 
2.25.1

[PATCH 1/4] ALSA: hda: Add snd_hdac_ext_bus_link_at() helper

[Cezary Rojewski]

This patch exposes a new helper to directly retrieve the link from the
codec address, and makes use of this helper when retrieving the link
from the codec name.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 include/sound/hdaudio_ext.h         |  1 +
 sound/hda/ext/hdac_ext_controller.c | 31 +++++++++++++++++++----------
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
index 77123c3e4095..b0c8e4936168 100644
--- a/include/sound/hdaudio_ext.h
+++ b/include/sound/hdaudio_ext.h
@@ -28,6 +28,7 @@ void snd_hdac_ext_stream_spbcap_enable(struct hdac_bus *chip,
 				 bool enable, int index);
 
 int snd_hdac_ext_bus_get_ml_capabilities(struct hdac_bus *bus);
+struct hdac_ext_link *snd_hdac_ext_bus_link_at(struct hdac_bus *bus, int addr);
 struct hdac_ext_link *snd_hdac_ext_bus_get_link(struct hdac_bus *bus,
 						const char *codec_name);
 
diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c
index b2df7b4f9227..b072392725c7 100644
--- a/sound/hda/ext/hdac_ext_controller.c
+++ b/sound/hda/ext/hdac_ext_controller.c
@@ -132,6 +132,26 @@ void snd_hdac_link_free_all(struct hdac_bus *bus)
 }
 EXPORT_SYMBOL_GPL(snd_hdac_link_free_all);
 
+/**
+ * snd_hdac_ext_bus_link_at - get link at specified address
+ * @bus: link's parent bus device
+ * @addr: codec device address
+ *
+ * Returns link object or NULL if matching link is not found.
+ */
+struct hdac_ext_link *snd_hdac_ext_bus_link_at(struct hdac_bus *bus, int addr)
+{
+	struct hdac_ext_link *hlink;
+	int i;
+
+	list_for_each_entry(hlink, &bus->hlink_list, list)
+		for (i = 0; i < HDA_MAX_CODECS; i++)
+			if (hlink->lsdiid & (0x1 << addr))
+				return hlink;
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_at);
+
 /**
  * snd_hdac_ext_bus_get_link - get link based on codec name
  * @bus: the pointer to HDAC bus object
@@ -140,8 +160,6 @@ EXPORT_SYMBOL_GPL(snd_hdac_link_free_all);
 struct hdac_ext_link *snd_hdac_ext_bus_get_link(struct hdac_bus *bus,
 						 const char *codec_name)
 {
-	int i;
-	struct hdac_ext_link *hlink = NULL;
 	int bus_idx, addr;
 
 	if (sscanf(codec_name, "ehdaudio%dD%d", &bus_idx, &addr) != 2)
@@ -151,14 +169,7 @@ struct hdac_ext_link *snd_hdac_ext_bus_get_link(struct hdac_bus *bus,
 	if (addr < 0 || addr > 31)
 		return NULL;
 
-	list_for_each_entry(hlink, &bus->hlink_list, list) {
-		for (i = 0; i < HDA_MAX_CODECS; i++) {
-			if (hlink->lsdiid & (0x1 << addr))
-				return hlink;
-		}
-	}
-
-	return NULL;
+	return snd_hdac_ext_bus_link_at(bus, addr);
 }
 EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_get_link);
 
-- 
2.25.1

[PATCH 2/4] ALSA: hda: Update and expose snd_hda_codec_device_init()

[Cezary Rojewski]

With few changes, snd_hda_codec_device_init() can be re-used by ASoC
drivers. While at it, provide kernel doc for the exposed function.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 include/sound/hda_codec.h |  3 +++
 sound/pci/hda/hda_codec.c | 45 +++++++++++++++++++++++++--------------
 2 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
index 82d9daa17851..5e3cbcca42f0 100644
--- a/include/sound/hda_codec.h
+++ b/include/sound/hda_codec.h
@@ -306,6 +306,9 @@ struct hda_codec {
 /*
  * constructors
  */
+__printf(3, 4) struct hda_codec *
+snd_hda_codec_device_init(struct hda_bus *bus, unsigned int codec_addr,
+			  const char *fmt, ...);
 int snd_hda_codec_new(struct hda_bus *bus, struct snd_card *card,
 		      unsigned int codec_addr, struct hda_codec **codecp);
 int snd_hda_codec_device_new(struct hda_bus *bus, struct snd_card *card,
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 7016b48227bf..3787060ad77f 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -877,36 +877,48 @@ static void snd_hda_codec_dev_release(struct device *dev)
 
 #define DEV_NAME_LEN 31
 
-static int snd_hda_codec_device_init(struct hda_bus *bus, struct snd_card *card,
-			unsigned int codec_addr, struct hda_codec **codecp)
+/**
+ * snd_hda_codec_device_init - allocate HDA codec device
+ * @bus: codec's parent bus
+ * @codec_addr: the codec address on the parent bus
+ * @fmt: format string for the device's name
+ *
+ * Returns newly allocated codec device or ERR_PTR() on failure.
+ */
+struct hda_codec *
+snd_hda_codec_device_init(struct hda_bus *bus, unsigned int codec_addr,
+			  const char *fmt, ...)
 {
+	va_list vargs;
 	char name[DEV_NAME_LEN];
 	struct hda_codec *codec;
 	int err;
 
-	dev_dbg(card->dev, "%s: entry\n", __func__);
-
 	if (snd_BUG_ON(!bus))
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	if (snd_BUG_ON(codec_addr > HDA_MAX_CODEC_ADDRESS))
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 
 	codec = kzalloc(sizeof(*codec), GFP_KERNEL);
 	if (!codec)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
+
+	va_start(vargs, fmt);
+	vsprintf(name, fmt, vargs);
+	va_end(vargs);
 
-	sprintf(name, "hdaudioC%dD%d", card->number, codec_addr);
 	err = snd_hdac_device_init(&codec->core, &bus->core, name, codec_addr);
 	if (err < 0) {
 		kfree(codec);
-		return err;
+		return ERR_PTR(err);
 	}
 
+	codec->bus = bus;
 	codec->core.type = HDA_DEV_LEGACY;
-	*codecp = codec;
 
-	return err;
+	return codec;
 }
+EXPORT_SYMBOL_GPL(snd_hda_codec_device_init);
 
 /**
  * snd_hda_codec_new - create a HDA codec
@@ -920,11 +932,13 @@ static int snd_hda_codec_device_init(struct hda_bus *bus, struct snd_card *card,
 int snd_hda_codec_new(struct hda_bus *bus, struct snd_card *card,
 		      unsigned int codec_addr, struct hda_codec **codecp)
 {
-	int ret;
+	struct hda_codec *codec;
 
-	ret = snd_hda_codec_device_init(bus, card, codec_addr, codecp);
-	if (ret < 0)
-		return ret;
+	codec = snd_hda_codec_device_init(bus, codec_addr, "hdaudioC%dD%d",
+					  card->number, codec_addr);
+	if (IS_ERR(codec))
+		return PTR_ERR(codec);
+	*codecp = codec;
 
 	return snd_hda_codec_device_new(bus, card, codec_addr, *codecp);
 }
@@ -951,7 +965,6 @@ int snd_hda_codec_device_new(struct hda_bus *bus, struct snd_card *card,
 	codec->core.dev.release = snd_hda_codec_dev_release;
 	codec->core.exec_verb = codec_exec_verb;
 
-	codec->bus = bus;
 	codec->card = card;
 	codec->addr = codec_addr;
 	mutex_init(&codec->spdif_mutex);
-- 
2.25.1

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

[Cezary Rojewski]

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.

Due to ALSA-device vs ASoC-component organization differences, new
'snddev_managed' argument is specified allowing for better control over
codec registration process.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 include/sound/hda_codec.h   |  5 ++++-
 sound/pci/hda/hda_codec.c   | 35 ++++++++++++++++++++++++++---------
 sound/pci/hda/hda_local.h   |  1 -
 sound/soc/codecs/hdac_hda.c |  2 +-
 4 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
index 5e3cbcca42f0..f74abc13414f 100644
--- a/include/sound/hda_codec.h
+++ b/include/sound/hda_codec.h
@@ -312,9 +312,12 @@ snd_hda_codec_device_init(struct hda_bus *bus, unsigned int codec_addr,
 int snd_hda_codec_new(struct hda_bus *bus, struct snd_card *card,
 		      unsigned int codec_addr, struct hda_codec **codecp);
 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);
 int snd_hda_codec_configure(struct hda_codec *codec);
 int snd_hda_codec_update_widgets(struct hda_codec *codec);
+void snd_hda_codec_register(struct hda_codec *codec);
+void snd_hda_codec_unregister(struct hda_codec *codec);
 
 /*
  * low level functions
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 3787060ad77f..4406b7983c07 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -813,7 +813,12 @@ void snd_hda_codec_display_power(struct hda_codec *codec, bool enable)
 		snd_hdac_display_power(&codec->bus->core, codec->addr, enable);
 }
 
-/* also called from hda_bind.c */
+/**
+ * snd_hda_codec_register - Finalize codec initialization
+ * @codec: codec device to register
+ *
+ * Also called from hda_bind.c
+ */
 void snd_hda_codec_register(struct hda_codec *codec)
 {
 	if (codec->registered)
@@ -826,6 +831,7 @@ void snd_hda_codec_register(struct hda_codec *codec)
 		codec->registered = 1;
 	}
 }
+EXPORT_SYMBOL_GPL(snd_hda_codec_register);
 
 static int snd_hda_codec_dev_register(struct snd_device *device)
 {
@@ -833,10 +839,12 @@ static int snd_hda_codec_dev_register(struct snd_device *device)
 	return 0;
 }
 
-static int snd_hda_codec_dev_free(struct snd_device *device)
+/**
+ * snd_hda_codec_unregister - Unregister specified codec device
+ * @codec: codec device to unregister
+ */
+void snd_hda_codec_unregister(struct hda_codec *codec)
 {
-	struct hda_codec *codec = device->device_data;
-
 	codec->in_freeing = 1;
 	/*
 	 * snd_hda_codec_device_new() is used by legacy HDA and ASoC driver.
@@ -853,7 +861,12 @@ static int snd_hda_codec_dev_free(struct snd_device *device)
 	 */
 	if (codec->core.type == HDA_DEV_LEGACY)
 		put_device(hda_codec_dev(codec));
+}
+EXPORT_SYMBOL_GPL(snd_hda_codec_unregister);
 
+static int snd_hda_codec_dev_free(struct snd_device *device)
+{
+	snd_hda_codec_unregister(device->device_data);
 	return 0;
 }
 
@@ -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);
 }
 EXPORT_SYMBOL_GPL(snd_hda_codec_new);
 
 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;
+	}
 
 	/* PM runtime needs to be enabled later after binding codec */
 	pm_runtime_forbid(&codec->core.dev);
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
index 8621f576446b..4c52dfb615bc 100644
--- a/sound/pci/hda/hda_local.h
+++ b/sound/pci/hda/hda_local.h
@@ -135,7 +135,6 @@ int __snd_hda_add_vmaster(struct hda_codec *codec, char *name,
 #define snd_hda_add_vmaster(codec, name, tlv, followers, suffix, access) \
 	__snd_hda_add_vmaster(codec, name, tlv, followers, suffix, true, access, NULL)
 int snd_hda_codec_reset(struct hda_codec *codec);
-void snd_hda_codec_register(struct hda_codec *codec);
 void snd_hda_codec_cleanup_for_unbind(struct hda_codec *codec);
 void snd_hda_codec_disconnect_pcms(struct hda_codec *codec);
 
diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c
index de5955db0a5f..667f3df239c7 100644
--- a/sound/soc/codecs/hdac_hda.c
+++ b/sound/soc/codecs/hdac_hda.c
@@ -413,7 +413,7 @@ static int hdac_hda_codec_probe(struct snd_soc_component *component)
 				       HDA_CODEC_IDX_CONTROLLER, true);
 
 	ret = snd_hda_codec_device_new(hcodec->bus, component->card->snd_card,
-				       hdev->addr, hcodec);
+				       hdev->addr, hcodec, true);
 	if (ret < 0) {
 		dev_err(&hdev->dev, "failed to create hda codec %d\n", ret);
 		goto error_no_pm;
-- 
2.25.1

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

[Kai Vehmanen]

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

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

[Cezary Rojewski]

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.

>> @@ -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?

Good catch! It is supposed to be 'true' by default. I checked previous 
'releases' of this patch: between internal RFC v1 -> RFC v2 this mistake 
got in, and probably because I've rebased the driver during that time 
and run into several conflicts which I had to fix manually.

Will update in v2.

>>   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.

Same as above.


Regards,
Czarek

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

[Takashi Iwai]

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

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

[Cezary Rojewski]

On 2022-02-08 6:14 PM, Takashi Iwai wrote:
> 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.

I was anticipating snd_hda_codec_device_init() related question so much 
and was so ready for it that I answered like an autobot, regardless of 
fact that people were not asking about snd_hda_codec_device_init() vs 
snd_hda_codec_device_new() directly and that's not even the patch where 
the change is added. And I wasn't even drinking coffee for the past few 
days..

Meh..
In regard to snddev_managed, If I'm not mistaken, the problem is 
connected to the 'dev_ops' (.dev_register, .dev_free) provided for 
snd_device_new(). To have basically 0% custom HDA logic in our code, 
rely solely on code found in sound/hda and sound/pci/hda ability to 
control when snd_hda_codec_register() and snd_hda_codec_unregister() is 
called was needed.

snd_soc_bind_card() invokes snd_card_register() which in consequence 
leads to snd_device_register_all() and that to automatic ->dev_register 
call. That call involves PM operations, and at that moment, codec is not 
ready for it. In the end, ability to call these in correct order allowed 
all codecs that communicate with avs-driver to remain on HDA_DEV_LEGACY 
level.

To answer the remaining question found in your message:
The purpose of the change was not to get the fields of snd_hda_codec out 
of the function and have them initialized elsewhere. All other 
operations found in the snd_hda_codec_device_new() are required and 
their ordering is not problematic so they have been left untouched.


Regards,
Czarek

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

[Cezary Rojewski]

On 2022-02-09 11:21 AM, Cezary Rojewski wrote:
> snd_soc_bind_card() invokes snd_card_register() which in consequence 
> leads to snd_device_register_all() and that to automatic ->dev_register 
> call. That call involves PM operations, and at that moment, codec is not 
> ready for it.
By that I mean: bus driver (here, avs-driver) has some saying on the PM 
matter too e.g.: sets their (codecs) status to suspended via 
pm_runtime_set_suspended() so the bus runtime suspend is not blocked by 
codecs that could possibly never complete their probing.


Regards,
Czarek

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

[Takashi Iwai]

On Wed, 09 Feb 2022 11:38:23 +0100,
Cezary Rojewski wrote:
> 
> On 2022-02-09 11:21 AM, Cezary Rojewski wrote:
> > snd_soc_bind_card() invokes snd_card_register() which in consequence
> > leads to snd_device_register_all() and that to automatic
> > ->dev_register call. That call involves PM operations, and at that
> > moment, codec is not ready for it.
> By that I mean: bus driver (here, avs-driver) has some saying on the
> PM matter too e.g.: sets their (codecs) status to suspended via
> pm_runtime_set_suspended() so the bus runtime suspend is not blocked
> by codecs that could possibly never complete their probing.

Hm, but the runtime PM of hda_codec device is suppressed at first in
snd_hda_codec_device_new() via pm_runtime_forbid(), i.e. the codec
should be kept running until it reaches to snd_hda_codec_register(),
it shouldn't be runtime-suspended beforehand.

I might overlook something, but it's a bit hard to follow...


thanks,

Takashi

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

[Pierre-Louis Bossart]

>>> 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.

New capabilities are always welcome, what I am missing is how important
your suggestion is for end users or OEMs.

The main reason for using a DSP-based driver on a HDaudio Intel platform
is when DMICs connected to the PCH, since this link cannot be handled by
the legacy driver. Those sort of form factors typically have analog
playback and capture only. UCM exposes only analog playback and capture.

Desktops without DMICs generally rely on the legacy driver and have
different sorts of problems with jack retasking and other time-consuming
problems.

Exposing additional DAIs in a DSP-based driver when supported by the
codec is a good idea on paper, but it's far from straightforward.

a) it assumes that there are indeed additional DAIs. Is this really the
case on the SKL/KBL devices you are targeting? It's not on newer
CML..ADL devices we've been supporting with SOF.

b) it assumes that what is exposed by the codec actually does work - and
the number of quirks tells us to be cautious. We routinely get reports
that even Intel NUCs don't have the right quirks in the kernel...

c) and then it creates new problems for the topology that may expose
paths that may or may not be supported by the codec. I am not aware of
any existing APIs to take down or enable a path conditionally - though
it's been a problem for a very long time for SSP and DMIC enablement
that are not always correctly described, and any suggestions to improve
this limitation would have my full support.

FWIW, in our latest SOF work we went back to handling ONE DAI with
analog playback and capture and ditched the 'digital playback'. Trying
to do more led us to too many issues of 'works on platform A' and 'does
not work on platform B', and sometimes with different answers depending
on which BIOS version is used.

IMHO what's really problematic for HDaudio is the support for amplifiers
located behind the HDaudio codec, for which we more often than not are
missing the I2C configuration sequences. Suspend-resume is a recurring
problem as well.

I am not saying no for the sake of saying no, I have just never heard of
anyone complain about restrictions on the number of DAIs in the HDaudio
world.

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

[Cezary Rojewski]

On 2022-02-08 6:46 PM, Pierre-Louis Bossart wrote:

> New capabilities are always welcome, what I am missing is how important
> your suggestion is for end users or OEMs.


Users won't notice and why would we like them to notice? The intended 
behavior is 1:1 with hda legacy one.

> The main reason for using a DSP-based driver on a HDaudio Intel platform
> is when DMICs connected to the PCH, since this link cannot be handled by
> the legacy driver. Those sort of form factors typically have analog
> playback and capture only. UCM exposes only analog playback and capture.
> 
> Desktops without DMICs generally rely on the legacy driver and have
> different sorts of problems with jack retasking and other time-consuming
> problems.


I believe we're all aware why and when DSP-drivers are chosen over HDA 
legacy driver. That's orthogonal to the current subject.

> Exposing additional DAIs in a DSP-based driver when supported by the
> codec is a good idea on paper, but it's far from straightforward.


The outcome is reduction in number of DAIs exposed for basically all 
codecs except for HDMIs as DSP-based solutions are hardcoding 3-DAIs. 
Here, the intended behavior is to be 1:1 with hda legacy behavior, not 
hardcoding.

> a) it assumes that there are indeed additional DAIs. Is this really the
> case on the SKL/KBL devices you are targeting? It's not on newer
> CML..ADL devices we've been supporting with SOF.


It does not _assume_, it _reads_. The outcome is reduction of DAIs 
exposed rather than hardcoding Analog/Alt Analog/Digital endpoints what 
is currently the case.

> b) it assumes that what is exposed by the codec actually does work - and
> the number of quirks tells us to be cautious. We routinely get reports
> that even Intel NUCs don't have the right quirks in the kernel...


Patches just expose functions. Logic stays the same. As it is 
sound/pci/hda code we're talking about, that code is quirk-friendly - it 
contains several "patches" and quirks already.

> c) and then it creates new problems for the topology that may expose
> paths that may or may not be supported by the codec. I am not aware of
> any existing APIs to take down or enable a path conditionally - though
> it's been a problem for a very long time for SSP and DMIC enablement
> that are not always correctly described, and any suggestions to improve
> this limitation would have my full support.


The default for topology is to expose just single analog endpoint as 
majority of codecs expose nothing else. Moreover, having just 1 FE 
defined in topology can be used to limit the usecases where we know 
codec says it exposes more but in fact these endpoints aren't 
functional. For specific codecs that expose more, a specific topology 
can be provided just like it's done for any DSP-driver today.

> FWIW, in our latest SOF work we went back to handling ONE DAI with
> analog playback and capture and ditched the 'digital playback'. Trying
> to do more led us to too many issues of 'works on platform A' and 'does
> not work on platform B', and sometimes with different answers depending
> on which BIOS version is used.
> 
> IMHO what's really problematic for HDaudio is the support for amplifiers
> located behind the HDaudio codec, for which we more often than not are
> missing the I2C configuration sequences. Suspend-resume is a recurring
> problem as well.
> 
> I am not saying no for the sake of saying no, I have just never heard of
> anyone complain about restrictions on the number of DAIs in the HDaudio
> world.


I believe our goals align. Rather than hardcoding Analog/Alt 
Analog/Digital endpoints as it's done currently, when codec most of the 
time do not have them working anyway, rely on behavior found in 
sound/hda and sound/pci/hda. If there are some problems there it's 
win:win for us and legacy driver. Fix one spot, have both drivers happy.


Regards,
Czarek

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

[Pierre-Louis Bossart]

On 2/9/22 06:05, Cezary Rojewski wrote:
>> FWIW, in our latest SOF work we went back to handling ONE DAI with
>> analog playback and capture and ditched the 'digital playback'. Trying
>> to do more led us to too many issues of 'works on platform A' and 'does
>> not work on platform B', and sometimes with different answers depending
>> on which BIOS version is used.
>>
>> IMHO what's really problematic for HDaudio is the support for amplifiers
>> located behind the HDaudio codec, for which we more often than not are
>> missing the I2C configuration sequences. Suspend-resume is a recurring
>> problem as well.
>>
>> I am not saying no for the sake of saying no, I have just never heard of
>> anyone complain about restrictions on the number of DAIs in the HDaudio
>> world.
> 
> 
> I believe our goals align. Rather than hardcoding Analog/Alt
> Analog/Digital endpoints as it's done currently, when codec most of the
> time do not have them working anyway, rely on behavior found in
> sound/hda and sound/pci/hda. If there are some problems there it's
> win:win for us and legacy driver. Fix one spot, have both drivers happy.

I don't quite see the alignment: the only thing that we've seen work
reliably and that we do need is the analog part, and we want to get rid
of the other paths which we can't test in the first place. I must admit
I don't recall why we bothered to expose those alt analog and digital
paths back in 2018, they have not been used nor tested by anyone.

Now if this patch helped to make sure we do indeed have an analog path,
that'd be fine. That would indeed avoid a hard-coded decision and report
configuration errors.

I just don't see what exposing additional paths brings, they never
worked reliably for us when we tried. IIRC half of our team gets an
error with the Digital playback stream on Up Extreme and the other half
can play just fine - albeit with no connector to actually see what the
output is.

FWIW there are many things made possible by HDaudio, in practice we
often have to limit ourselves to what is known to work and what is
needed by end-products. You could spend all your time chasing
configuration issues, missing NIDS and bad verbs. Been there, done that.

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

[Cezary Rojewski]

On 2022-02-09 5:08 PM, Pierre-Louis Bossart wrote:
> 
> 
> On 2/9/22 06:05, Cezary Rojewski wrote:
>> I believe our goals align. Rather than hardcoding Analog/Alt
>> Analog/Digital endpoints as it's done currently, when codec most of the
>> time do not have them working anyway, rely on behavior found in
>> sound/hda and sound/pci/hda. If there are some problems there it's
>> win:win for us and legacy driver. Fix one spot, have both drivers happy.
> 
> I don't quite see the alignment: the only thing that we've seen work
> reliably and that we do need is the analog part, and we want to get rid
> of the other paths which we can't test in the first place. I must admit
> I don't recall why we bothered to expose those alt analog and digital
> paths back in 2018, they have not been used nor tested by anyone.
> 
> Now if this patch helped to make sure we do indeed have an analog path,
> that'd be fine. That would indeed avoid a hard-coded decision and report
> configuration errors.
> 
> I just don't see what exposing additional paths brings, they never
> worked reliably for us when we tried. IIRC half of our team gets an
> error with the Digital playback stream on Up Extreme and the other half
> can play just fine - albeit with no connector to actually see what the
> output is.
 >
> FWIW there are many things made possible by HDaudio, in practice we
> often have to limit ourselves to what is known to work and what is
> needed by end-products. You could spend all your time chasing
> configuration issues, missing NIDS and bad verbs. Been there, done that.


Again, we are not trying to force-expose stuff which does not work. In 
majority of the cases, non-HDMI codecs we're dealing with notify about 
just single analog endpoint. For now, it's 100% of the cases, but I'm 
aware of fact that RVPs and a dozen of Dell/Lenovo/Acer laptops do not 
equal to entire market.

Remember that you can always use topology to "gate" userspace from 
streaming through endpoints which we do not work. And right now, we are 
working with topologies supporting single endpoint for non-HDMI devices.

So, this is a clear upgrade when compared to Analog/Alt 
Analog/Digitalh-hardcoded configuration used currently. That's on top of 
aligning with hda legacy behavior.


Regards,
Czarek

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

[Pierre-Louis Bossart]

> Again, we are not trying to force-expose stuff which does not work. In
> majority of the cases, non-HDMI codecs we're dealing with notify about
> just single analog endpoint. For now, it's 100% of the cases, but I'm
> aware of fact that RVPs and a dozen of Dell/Lenovo/Acer laptops do not
> equal to entire market.

We are in agreement, but since we don't have any ability to test those
alt/digital parts my take is 'don't even bother about them'.

> Remember that you can always use topology to "gate" userspace from
> streaming through endpoints which we do not work. And right now, we are
> working with topologies supporting single endpoint for non-HDMI devices.

May I ask how you 'gate' parts of a topology? Or did you mean that you
use different topologies?

> 
> So, this is a clear upgrade when compared to Analog/Alt
> Analog/Digitalh-hardcoded configuration used currently. That's on top of
> aligning with hda legacy behavior.

Agree, but it still leaves the door open to exposing those paths which
may or may not work - no one has ever tested them. It's better IMHO to
only allow for the analog path. If we can detect the presence of this
path, great.

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

[Cezary Rojewski]

On 2022-02-11 4:23 PM, Pierre-Louis Bossart wrote:
> 
>> Again, we are not trying to force-expose stuff which does not work. In
>> majority of the cases, non-HDMI codecs we're dealing with notify about
>> just single analog endpoint. For now, it's 100% of the cases, but I'm
>> aware of fact that RVPs and a dozen of Dell/Lenovo/Acer laptops do not
>> equal to entire market.
> 
> We are in agreement, but since we don't have any ability to test those
> alt/digital parts my take is 'don't even bother about them'.
> 
>> Remember that you can always use topology to "gate" userspace from
>> streaming through endpoints which we do not work. And right now, we are
>> working with topologies supporting single endpoint for non-HDMI devices.
> 
> May I ask how you 'gate' parts of a topology? Or did you mean that you
> use different topologies?
> 
>>
>> So, this is a clear upgrade when compared to Analog/Alt
>> Analog/Digitalh-hardcoded configuration used currently. That's on top of
>> aligning with hda legacy behavior.
> 
> Agree, but it still leaves the door open to exposing those paths which
> may or may not work - no one has ever tested them. It's better IMHO to
> only allow for the analog path. If we can detect the presence of this
> path, great.

You may define topology with lower number of FE, thus limiting number of 
options available to user, possibly shielding them from untested 
scenarios. E.g.: Have a 3-DAI HDMI codec yet provide 2-FE topology file 
for it because you know 3rd endpoint is faulty. Only two devices will 
show up in userspace effectively making users unable to play on the 3rd one.

Again, that's a default - expose fewer endpoints (here, just single 
analog one) for non-HDMI codecs.

Of course, there is small number of users who may want to stream on 
their  Alt Analog/Digital endpoints. We do not want 'Conexant story' to 
repeat all over again. So, for them, non-default (topology) could be 
provided to enable streaming on whatever they want.

The chosen approach does not hinder either of the sides.


Regards,
Czarek

[PATCH 4/4] ALSA: hda: Expose codec cleanup and power-save functions

[Cezary Rojewski]

With few changes, snd_hda_codec_set_power_save() and
snd_hda_codec_cleanup_for_unbind() can be re-used by ASoC drivers.
While at it, provide kernel doc for the exposed functions.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 include/sound/hda_codec.h |  3 +++
 sound/pci/hda/hda_codec.c | 14 ++++++++++++--
 sound/pci/hda/hda_local.h |  1 -
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
index f74abc13414f..77426ff58338 100644
--- a/include/sound/hda_codec.h
+++ b/include/sound/hda_codec.h
@@ -318,6 +318,7 @@ int snd_hda_codec_configure(struct hda_codec *codec);
 int snd_hda_codec_update_widgets(struct hda_codec *codec);
 void snd_hda_codec_register(struct hda_codec *codec);
 void snd_hda_codec_unregister(struct hda_codec *codec);
+void snd_hda_codec_cleanup_for_unbind(struct hda_codec *codec);
 
 /*
  * low level functions
@@ -496,9 +497,11 @@ int hda_call_check_power_status(struct hda_codec *codec, hda_nid_t nid)
 #define snd_hda_power_down(codec)	snd_hdac_power_down(&(codec)->core)
 #define snd_hda_power_down_pm(codec)	snd_hdac_power_down_pm(&(codec)->core)
 #ifdef CONFIG_PM
+void snd_hda_codec_set_power_save(struct hda_codec *codec, int delay);
 void snd_hda_set_power_save(struct hda_bus *bus, int delay);
 void snd_hda_update_power_acct(struct hda_codec *codec);
 #else
+static inline void snd_hda_codec_set_power_save(struct hda_codec *codec, int delay) {}
 static inline void snd_hda_set_power_save(struct hda_bus *bus, int delay) {}
 #endif
 
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 4406b7983c07..34db37ce8847 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -766,6 +766,10 @@ static void codec_release_pcms(struct hda_codec *codec)
 	}
 }
 
+/**
+ * snd_hda_codec_cleanup_for_unbind - Prepare codec for removal
+ * @codec: codec device to cleanup
+ */
 void snd_hda_codec_cleanup_for_unbind(struct hda_codec *codec)
 {
 	if (codec->registered) {
@@ -3397,7 +3401,12 @@ int snd_hda_add_new_ctls(struct hda_codec *codec,
 EXPORT_SYMBOL_GPL(snd_hda_add_new_ctls);
 
 #ifdef CONFIG_PM
-static void codec_set_power_save(struct hda_codec *codec, int delay)
+/**
+ * snd_hda_codec_set_power_save - Configure codec's runtime PM
+ * @codec: codec device to configure
+ * @delay: autosuspend delay
+ */
+void snd_hda_codec_set_power_save(struct hda_codec *codec, int delay)
 {
 	struct device *dev = hda_codec_dev(codec);
 
@@ -3415,6 +3424,7 @@ static void codec_set_power_save(struct hda_codec *codec, int delay)
 		pm_runtime_forbid(dev);
 	}
 }
+EXPORT_SYMBOL_GPL(snd_hda_codec_set_power_save);
 
 /**
  * snd_hda_set_power_save - reprogram autosuspend for the given delay
@@ -3428,7 +3438,7 @@ void snd_hda_set_power_save(struct hda_bus *bus, int delay)
 	struct hda_codec *c;
 
 	list_for_each_codec(c, bus)
-		codec_set_power_save(c, delay);
+		snd_hda_codec_set_power_save(c, delay);
 }
 EXPORT_SYMBOL_GPL(snd_hda_set_power_save);
 
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
index 4c52dfb615bc..aca592651870 100644
--- a/sound/pci/hda/hda_local.h
+++ b/sound/pci/hda/hda_local.h
@@ -135,7 +135,6 @@ int __snd_hda_add_vmaster(struct hda_codec *codec, char *name,
 #define snd_hda_add_vmaster(codec, name, tlv, followers, suffix, access) \
 	__snd_hda_add_vmaster(codec, name, tlv, followers, suffix, true, access, NULL)
 int snd_hda_codec_reset(struct hda_codec *codec);
-void snd_hda_codec_cleanup_for_unbind(struct hda_codec *codec);
 void snd_hda_codec_disconnect_pcms(struct hda_codec *codec);
 
 #define snd_hda_regmap_sync(codec)	snd_hdac_regmap_sync(&(codec)->core)
-- 
2.25.1

Re: [PATCH 0/4] ALSA: hda: Expose codec organization functions

[Pierre-Louis Bossart]

On 2/7/22 05:49, Cezary Rojewski wrote:
> Changes expose several function that are currently unavailable for
> HDA-DSP drivers for use. Those functions are:
> 
> snd_hda_codec_cleanup_for_unbind()
> snd_hda_codec_set_power_save()
> snd_hda_codec_register()
> snd_hda_codec_unregister()
> snd_hda_codec_device_init()


It would be useful to explain why a platform driver would need to make
use of codec-management related routines, which would typically be
needed only in a codec or machine driver, or hidden as part of a generic
bus layer.

In addition, both the Skylake and SOF/HDA drivers make use of e.g.,
snd_hdac_ext_bus_device_init(), what was wrong with this approach that's
been used since 2018?

Re: [PATCH 0/4] ALSA: hda: Expose codec organization functions

[Cezary Rojewski]

On 2022-02-07 1:35 PM, Pierre-Louis Bossart wrote:
> 
> 
> On 2/7/22 05:49, Cezary Rojewski wrote:
>> Changes expose several function that are currently unavailable for
>> HDA-DSP drivers for use. Those functions are:
>>
>> snd_hda_codec_cleanup_for_unbind()
>> snd_hda_codec_set_power_save()
>> snd_hda_codec_register()
>> snd_hda_codec_unregister()
>> snd_hda_codec_device_init()
> 
> 
> It would be useful to explain why a platform driver would need to make
> use of codec-management related routines, which would typically be
> needed only in a codec or machine driver, or hidden as part of a generic
> bus layer.
> 
> In addition, both the Skylake and SOF/HDA drivers make use of e.g.,
> snd_hdac_ext_bus_device_init(), what was wrong with this approach that's
> been used since 2018?

Thanks for chiming in! So, all HDA drivers currently available in ASoC 
are _assuming_ codec resources, they are not _reading_ them. To be 
efficient and only create DAIs and other components when needed, codec's 
->pcm_list_head need to be filled in with data before ASoC sound card 
can be fully enumerated.

Initialization routines for HDA device require pointer to instance of 
struct snd_card upfront whereas ASoC framework gives you such pointer 
only once all components are accounted for. Also, component granulation 
seen in ASoC causes need for adjustment for order of operations when 
registering/probing codec device to achieve correctness (resource wise) 
I'd mentioned above. We could have coded that logic ourselves but that's 
a duplication as the logic is already there.


Regards,
Czarek