[PATCH 0/2] Introduce dmic mode switch delay parameter

[PATCH 0/2] Introduce dmic mode switch delay parameter

[Jenny TC]

Some DMICs need clock to be running for a specified duration as per the
DMIC spec to complete the mode transition like sleep to mormal, off to normal
etc.

Jenny TC (2):
  ASoC: dmic: Enable ACPI device entry
  ASoC: dmic: Introduce mode switch delay

 sound/soc/codecs/dmic.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

-- 
1.9.1

[PATCH 1/2] ASoC: dmic: Enable ACPI device entry

[Jenny TC]

Enable ACPI device probing for dmic so that DMIC parameters can be passed
from ACPI.

Signed-off-by: Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>
Signed-off-by: Jairaj Arava <jairaj.arava@intel.com>
Signed-off-by: Harsha Priya <harshapriya.n@intel.com>
---
 sound/soc/codecs/dmic.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/sound/soc/codecs/dmic.c b/sound/soc/codecs/dmic.c
index 8c4926d..ab1aa01 100644
--- a/sound/soc/codecs/dmic.c
+++ b/sound/soc/codecs/dmic.c
@@ -19,6 +19,7 @@
  *
  */
 
+#include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/gpio.h>
 #include <linux/gpio/consumer.h>
@@ -144,15 +145,26 @@ static int dmic_dev_probe(struct platform_device *pdev)
 
 MODULE_ALIAS("platform:dmic-codec");
 
+#ifdef CONFIG_OF
 static const struct of_device_id dmic_dev_match[] = {
 	{.compatible = "dmic-codec"},
 	{}
 };
+#endif
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id dmic_acpi_match[] = {
+	{ "DMIC", 0 },
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, dmic_acpi_match);
+#endif
 
 static struct platform_driver dmic_driver = {
 	.driver = {
 		.name = "dmic-codec",
 		.of_match_table = dmic_dev_match,
+		.acpi_match_table = ACPI_PTR(dmic_acpi_match),
 	},
 	.probe = dmic_dev_probe,
 };
-- 
1.9.1

Re: [PATCH 1/2] ASoC: dmic: Enable ACPI device entry

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 333 bytes --]

On Tue, Oct 02, 2018 at 11:27:30AM +0530, Jenny TC wrote:

> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id dmic_acpi_match[] = {
> +	{ "DMIC", 0 },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(acpi, dmic_acpi_match);
> +#endif

This doesn't look like a standards conforming ACPI identifier, I thought
they were all 8 characters?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/2] ASoC: dmic: Enable ACPI device entry

[Matthias Kaehlcke]

Hi Jenny,

On Tue, Oct 02, 2018 at 11:27:30AM +0530, Jenny TC wrote:
> Enable ACPI device probing for dmic so that DMIC parameters can be passed
> from ACPI.
> 
> Signed-off-by: Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>
> Signed-off-by: Jairaj Arava <jairaj.arava@intel.com>
> Signed-off-by: Harsha Priya <harshapriya.n@intel.com>
> ---
>  sound/soc/codecs/dmic.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/sound/soc/codecs/dmic.c b/sound/soc/codecs/dmic.c
> index 8c4926d..ab1aa01 100644
> --- a/sound/soc/codecs/dmic.c
> +++ b/sound/soc/codecs/dmic.c
> @@ -19,6 +19,7 @@
>   *
>   */
>  
> +#include <linux/acpi.h>
>  #include <linux/delay.h>
>  #include <linux/gpio.h>
>  #include <linux/gpio/consumer.h>
> @@ -144,15 +145,26 @@ static int dmic_dev_probe(struct platform_device *pdev)
>  
>  MODULE_ALIAS("platform:dmic-codec");
>  
> +#ifdef CONFIG_OF
>  static const struct of_device_id dmic_dev_match[] = {
>  	{.compatible = "dmic-codec"},
>  	{}
>  };
> +#endif
> +
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id dmic_acpi_match[] = {
> +	{ "DMIC", 0 },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(acpi, dmic_acpi_match);
> +#endif
>  
>  static struct platform_driver dmic_driver = {
>  	.driver = {
>  		.name = "dmic-codec",
>  		.of_match_table = dmic_dev_match,

Above you make the definition of dmic_dev_match depend on CONFIG_OF,
you'll want to change the initialization here to
'of_match_ptr(dmic_dev_match)'

[PATCH 2/2] ASoC: dmic: Introduce mode switch delay

[Jenny TC]

Some DMICs require delay before stream capture to complete the mode
switching. Different modes can be power off, sleep etc. The DMIC clock
for a specified duration is needed for the DMIC to complete the mode
switching and ready for stream capture.

Signed-off-by: Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>
Signed-off-by: Jairaj Arava <jairaj.arava@intel.com>
Signed-off-by: Harsha Priya <harshapriya.n@intel.com>
---
 sound/soc/codecs/dmic.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/sound/soc/codecs/dmic.c b/sound/soc/codecs/dmic.c
index ab1aa01..38d41a2 100644
--- a/sound/soc/codecs/dmic.c
+++ b/sound/soc/codecs/dmic.c
@@ -34,6 +34,29 @@
 struct dmic {
 	struct gpio_desc *gpio_en;
 	int wakeup_delay;
+	/* Delay after DMIC mode switch */
+	int modeswitch_delay_ms;
+};
+
+int dmic_daiops_trigger(struct snd_pcm_substream *substream,
+		int cmd, struct snd_soc_dai *dai)
+{
+	struct snd_soc_component *component = dai->component;
+	struct dmic *dmic = snd_soc_component_get_drvdata(component);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_STOP:
+		if (dmic->modeswitch_delay_ms)
+			mdelay(dmic->modeswitch_delay_ms);
+
+		break;
+	}
+
+	return 0;
+}
+
+static const struct snd_soc_dai_ops dmic_dai_ops = {
+	.trigger	= dmic_daiops_trigger,
 };
 
 static int dmic_aif_event(struct snd_soc_dapm_widget *w,
@@ -69,6 +92,7 @@ static int dmic_aif_event(struct snd_soc_dapm_widget *w,
 			| SNDRV_PCM_FMTBIT_S24_LE
 			| SNDRV_PCM_FMTBIT_S16_LE,
 	},
+	.ops    = &dmic_dai_ops,
 };
 
 static int dmic_component_probe(struct snd_soc_component *component)
@@ -86,6 +110,8 @@ static int dmic_component_probe(struct snd_soc_component *component)
 
 	device_property_read_u32(component->dev, "wakeup-delay-ms",
 				 &dmic->wakeup_delay);
+	device_property_read_u32(component->dev, "modeswitch_delay_ms",
+				 &dmic->modeswitch_delay_ms);
 
 	snd_soc_component_set_drvdata(component, dmic);
 
-- 
1.9.1

Re: [PATCH 0/2] Introduce dmic mode switch delay parameter

[Pierre-Louis Bossart]

On 10/2/18 12:57 AM, Jenny TC wrote:
> Some DMICs need clock to be running for a specified duration as per the
> DMIC spec to complete the mode transition like sleep to mormal, off to normal
> etc.
>
> Jenny TC (2):
>    ASoC: dmic: Enable ACPI device entry
>    ASoC: dmic: Introduce mode switch delay
>
>   sound/soc/codecs/dmic.c | 38 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 38 insertions(+)
>
Sorry for the late feedback.

Allowing some timing adjustments for the clock transitions is a good 
thing. The way it's done is questionable and raises a number of concerns.

First, there was an Intel internal discussion before my extended break 
on why this 'dmic-codec' is needed on Intel platforms. To the best of my 
knowledge we don't control the mics with GPIOs, which was the initial 
purpose of this driver. We have experimental evidence on ApolloLake and 
GeminiLake that using the soc-dummy/soc-dummy-dai definitions are 
enough, and it may be a good thing to agree on the direction here. If 
you want a parameter, you can still use a machine driver DMI-based 
kernel quirk and/or pass a kernel parameter, the need to extend this 
dmic-codec is far from obvious to me.

Assuming you still want to use this codec, then there are still major 
concerns about the ACPI directions. As Mark noted it, "DMIC" does not 
follow any of the guidelines or accepted definitions with an unambiguous 
vendor and part ID. We know we already have conflicts between 
Intel-defined ACPI IDs, e.g. for RT298 on multiple platforms, let's be 
careful here, shall we?

And I am coming to my last point. The Skylake driver already contains 
code to create the dmic devices by hand (see below the git grep 
results). So I wonder what happens if you use both ACPI-based 
enumeration AND manually create the dmic device - I view these solutions 
as mutually incompatible. Either you have not tested against the 
upstream code or something is missing from your patchset. What am I missing?

-Pierre

skl.c:static int skl_dmic_device_register(struct skl *skl)
skl.c:  /* SKL has one dmic port, so allocate dmic device for this */
skl.c:  pdev = platform_device_alloc("dmic-codec", -1);
skl.c:          dev_err(bus->dev, "failed to allocate dmic device\n");
skl.c:          dev_err(bus->dev, "failed to add dmic device: %d\n", ret);
skl.c:  skl->dmic_dev = pdev;
skl.c:static void skl_dmic_device_unregister(struct skl *skl)
skl.c:  if (skl->dmic_dev)
skl.c:          platform_device_unregister(skl->dmic_dev);
skl.c:  /* create device for soc dmic */
skl.c:  err = skl_dmic_device_register(skl);
skl.c:  skl_dmic_device_unregister(skl);

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 0/2] Introduce dmic mode switch delay parameter

[Pierre-Louis Bossart]

On 10/21/18 10:42 PM, Pierre-Louis Bossart wrote:
>
> On 10/2/18 12:57 AM, Jenny TC wrote:
>> Some DMICs need clock to be running for a specified duration as per the
>> DMIC spec to complete the mode transition like sleep to mormal, off 
>> to normal
>> etc.
>>
>> Jenny TC (2):
>>    ASoC: dmic: Enable ACPI device entry
>>    ASoC: dmic: Introduce mode switch delay
>>
>>   sound/soc/codecs/dmic.c | 38 ++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 38 insertions(+)
>>
> Sorry for the late feedback.
>
> Allowing some timing adjustments for the clock transitions is a good 
> thing. The way it's done is questionable and raises a number of concerns.
>
> First, there was an Intel internal discussion before my extended break 
> on why this 'dmic-codec' is needed on Intel platforms. To the best of 
> my knowledge we don't control the mics with GPIOs, which was the 
> initial purpose of this driver. We have experimental evidence on 
> ApolloLake and GeminiLake that using the soc-dummy/soc-dummy-dai 
> definitions are enough, and it may be a good thing to agree on the 
> direction here. If you want a parameter, you can still use a machine 
> driver DMI-based kernel quirk and/or pass a kernel parameter, the need 
> to extend this dmic-codec is far from obvious to me.
>
> Assuming you still want to use this codec, then there are still major 
> concerns about the ACPI directions. As Mark noted it, "DMIC" does not 
> follow any of the guidelines or accepted definitions with an 
> unambiguous vendor and part ID. We know we already have conflicts 
> between Intel-defined ACPI IDs, e.g. for RT298 on multiple platforms, 
> let's be careful here, shall we?
>
> And I am coming to my last point. The Skylake driver already contains 
> code to create the dmic devices by hand (see below the git grep 
> results). So I wonder what happens if you use both ACPI-based 
> enumeration AND manually create the dmic device - I view these 
> solutions as mutually incompatible. Either you have not tested against 
> the upstream code or something is missing from your patchset. What am 
> I missing?

I forgot to add another open on ACPI support: what would be the scope of 
the "DMIC" device? With ACPI we typically have a parent-child 
relationship, e.g. we put audio codec below the relevant I2C/SPI 
controller in the DSDT definitions. In the absence of a DMIC bus, you 
need to be careful how the DMIC device is added in the DSDT - it'd 
likely need to be below the scope of the HDaudio controller.

>
> -Pierre
>
> skl.c:static int skl_dmic_device_register(struct skl *skl)
> skl.c:  /* SKL has one dmic port, so allocate dmic device for this */
> skl.c:  pdev = platform_device_alloc("dmic-codec", -1);
> skl.c:          dev_err(bus->dev, "failed to allocate dmic device\n");
> skl.c:          dev_err(bus->dev, "failed to add dmic device: %d\n", 
> ret);
> skl.c:  skl->dmic_dev = pdev;
> skl.c:static void skl_dmic_device_unregister(struct skl *skl)
> skl.c:  if (skl->dmic_dev)
> skl.c:          platform_device_unregister(skl->dmic_dev);
> skl.c:  /* create device for soc dmic */
> skl.c:  err = skl_dmic_device_register(skl);
> skl.c:  skl_dmic_device_unregister(skl);
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 0/2] Introduce dmic mode switch delay parameter

[Tc, Jenny]

> Allowing some timing adjustments for the clock transitions is a good 
> thing. The way it's done is questionable and raises a number of concerns.
>
> First, there was an Intel internal discussion before my extended break 
> on why this 'dmic-codec' is needed on Intel platforms. To the best of 
> my knowledge we don't control the mics with GPIOs, which was the 
> initial purpose of this driver. We have experimental evidence on 
> ApolloLake and GeminiLake that using the soc-dummy/soc-dummy-dai 
> definitions are enough, and it may be a good thing to agree on the 
> direction here. If you want a parameter, you can still use a machine 
> driver DMI-based kernel quirk and/or pass a kernel parameter, the need 
> to extend this dmic-codec is far from obvious to me.

The driver already exposes another parameter (wakeup-delay-ms) using device tree. 
Enabling ACPI device enumeration provides a way to pass existing parameter
and also cover the new parameter(modeswitch_delay_ms) introduced in this patch set.
Isn't it good to adopt ACPI enumeration if the driver has multiple parameters to handle?
 
> Assuming you still want to use this codec, then there are still major 
> concerns about the ACPI directions. As Mark noted it, "DMIC" does not 
> follow any of the guidelines or accepted definitions with an 
> unambiguous vendor and part ID. We know we already have conflicts 
> between Intel-defined ACPI IDs, e.g. for RT298 on multiple platforms, 
> let's be careful here, shall we?

Agree, need to find proper ACPI ID for the device.
 
>
> And I am coming to my last point. The Skylake driver already contains 
> code to create the dmic devices by hand (see below the git grep 
> results). So I wonder what happens if you use both ACPI-based 
> enumeration AND manually create the dmic device - I view these 
> solutions as mutually incompatible. Either you have not tested against 
> the upstream code or something is missing from your patchset. What am 
> I missing?

Skl driver already registers a DMIC (dmic-codec) device and with ACPI enumeration
one more device (DMIC:00) gets registered. The snd_soc_dai_link  structure populated
in the machine driver decides which codec device to be used in the capture path and 
there by handles the compatibility issue you pointed out. 
 

>I forgot to add another open on ACPI support: what would be the scope of the "DMIC" device? 
>With ACPI we typically have a parent-child relationship, e.g. we put audio codec below the relevant
>I2C/SPI controller in the DSDT definitions. In the absence of a DMIC bus, you need to be careful how
>the DMIC device is added in the DSDT - it'd likely need to be below the scope of the HDaudio controller.

In DSDT, the device is added under the Intel HDA (1f.3 for SKL/KBL) parent device.

-Jenny

Re: [PATCH 0/2] Introduce dmic mode switch delay parameter

[Pierre-Louis Bossart]

On 10/23/18 12:11 PM, Tc, Jenny wrote:
>> Allowing some timing adjustments for the clock transitions is a good
>> thing. The way it's done is questionable and raises a number of concerns.
>>
>> First, there was an Intel internal discussion before my extended break
>> on why this 'dmic-codec' is needed on Intel platforms. To the best of
>> my knowledge we don't control the mics with GPIOs, which was the
>> initial purpose of this driver. We have experimental evidence on
>> ApolloLake and GeminiLake that using the soc-dummy/soc-dummy-dai
>> definitions are enough, and it may be a good thing to agree on the
>> direction here. If you want a parameter, you can still use a machine
>> driver DMI-based kernel quirk and/or pass a kernel parameter, the need
>> to extend this dmic-codec is far from obvious to me.
> The driver already exposes another parameter (wakeup-delay-ms) using device tree.
> Enabling ACPI device enumeration provides a way to pass existing parameter
> and also cover the new parameter(modeswitch_delay_ms) introduced in this patch set.
> Isn't it good to adopt ACPI enumeration if the driver has multiple parameters to handle?

ACPI enumeration has nothing to do with multiple parameters. You use 
ACPI enumeration when you want the devices to be created based on a set 
of descriptors exposed by the DSDT instead of hard-coding the device 
support in the kernel.

>   
>> Assuming you still want to use this codec, then there are still major
>> concerns about the ACPI directions. As Mark noted it, "DMIC" does not
>> follow any of the guidelines or accepted definitions with an
>> unambiguous vendor and part ID. We know we already have conflicts
>> between Intel-defined ACPI IDs, e.g. for RT298 on multiple platforms,
>> let's be careful here, shall we?
> Agree, need to find proper ACPI ID for the device.

ok, but not sure what to define. You don't want too many identifiers 
either, this generated lots of patches for no good reason. What are the 
needs here? You probably don't want to identify the DMIC vendor so this 
could be an Intel-defined ID. But I wonder if this might be reused on 
AMD platforms?

>   
>> And I am coming to my last point. The Skylake driver already contains
>> code to create the dmic devices by hand (see below the git grep
>> results). So I wonder what happens if you use both ACPI-based
>> enumeration AND manually create the dmic device - I view these
>> solutions as mutually incompatible. Either you have not tested against
>> the upstream code or something is missing from your patchset. What am
>> I missing?
> Skl driver already registers a DMIC (dmic-codec) device and with ACPI enumeration
> one more device (DMIC:00) gets registered. The snd_soc_dai_link  structure populated
> in the machine driver decides which codec device to be used in the capture path and
> there by handles the compatibility issue you pointed out.

Wow. What's the point of having two devices? I am not against your 
solution but at the very least there should be something in the SKL 
driver to detect the presence of DMIC ACPI identifiers and only manually 
register the dmic-codec if no identifier was found.

Changing the dailink to point to one device instead of another is not a 
good idea, the machine driver should be independent from all this, and 
be reusable between the SKL driver or SOF drivers. The last thing you 
want is a hack in there.

>   
>
>> I forgot to add another open on ACPI support: what would be the scope of the "DMIC" device?
>> With ACPI we typically have a parent-child relationship, e.g. we put audio codec below the relevant
>> I2C/SPI controller in the DSDT definitions. In the absence of a DMIC bus, you need to be careful how
>> the DMIC device is added in the DSDT - it'd likely need to be below the scope of the HDaudio controller.
> In DSDT, the device is added under the Intel HDA (1f.3 for SKL/KBL) parent device.

ok, makes sense. Do you think it'd be possible to use ACPI initrd 
overlays to add support for those parameters if they don't natively 
exist in the BIOS?

Re: [PATCH 0/2] Introduce dmic mode switch delay parameter

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1255 bytes --]

On Tue, Oct 23, 2018 at 01:22:18PM -0500, Pierre-Louis Bossart wrote:

> ok, but not sure what to define. You don't want too many identifiers either,
> this generated lots of patches for no good reason. What are the needs here?
> You probably don't want to identify the DMIC vendor so this could be an
> Intel-defined ID. But I wonder if this might be reused on AMD platforms?

There's nothing stopping AMD systems using the Intel device IDs.  We
already get some of that with the other non-Intel components that have
been assigned Intel IDs due to their presence on Intel reference
systems.

> Changing the dailink to point to one device instead of another is not a good
> idea, the machine driver should be independent from all this, and be
> reusable between the SKL driver or SOF drivers. The last thing you want is a
> hack in there.

The firmware binding really ought to be OS neutral, never mind driver
neutral :(

> ok, makes sense. Do you think it'd be possible to use ACPI initrd overlays
> to add support for those parameters if they don't natively exist in the
> BIOS?

Don't know about that (I'm not familiar enough with how this stuff gets
shipped on x86 systems) but the traditionl solution for ACPI appears to
be to have DMI based quirks.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 0/2] Introduce dmic mode switch delay parameter

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 741 bytes --]

On Tue, Oct 23, 2018 at 05:11:57PM +0000, Tc, Jenny wrote:

> The driver already exposes another parameter (wakeup-delay-ms) using device tree. 
> Enabling ACPI device enumeration provides a way to pass existing parameter
> and also cover the new parameter(modeswitch_delay_ms) introduced in this patch set.
> Isn't it good to adopt ACPI enumeration if the driver has multiple parameters to handle?

If you want to adopt device tree bindings in ACPI systems there is
already the _DSD based mapping for this.  However please be aware that
ACPI and DT systems have radically different approaches in a number of
areas, including describing sound cards, and trying to combine the two
in the same system might lead to disappointment.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 0/2] Introduce dmic mode switch delay parameter

[Vinod Koul]

On 21-10-18, 22:42, Pierre-Louis Bossart wrote:
> 
> On 10/2/18 12:57 AM, Jenny TC wrote:
> > Some DMICs need clock to be running for a specified duration as per the
> > DMIC spec to complete the mode transition like sleep to mormal, off to normal
> > etc.
> > 
> > Jenny TC (2):
> >    ASoC: dmic: Enable ACPI device entry
> >    ASoC: dmic: Introduce mode switch delay
> > 
> >   sound/soc/codecs/dmic.c | 38 ++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 38 insertions(+)
> > 
> Sorry for the late feedback.
> 
> Allowing some timing adjustments for the clock transitions is a good thing.
> The way it's done is questionable and raises a number of concerns.
> 
> First, there was an Intel internal discussion before my extended break on
> why this 'dmic-codec' is needed on Intel platforms. To the best of my
> knowledge we don't control the mics with GPIOs, which was the initial
> purpose of this driver. We have experimental evidence on ApolloLake and
> GeminiLake that using the soc-dummy/soc-dummy-dai definitions are enough,
> and it may be a good thing to agree on the direction here. If you want a
> parameter, you can still use a machine driver DMI-based kernel quirk and/or
> pass a kernel parameter, the need to extend this dmic-codec is far from
> obvious to me.

I think in this case you can go with dummy codec. You are modelling an
endpoint here, nothing else

> Assuming you still want to use this codec, then there are still major
> concerns about the ACPI directions. As Mark noted it, "DMIC" does not follow
> any of the guidelines or accepted definitions with an unambiguous vendor and
> part ID. We know we already have conflicts between Intel-defined ACPI IDs,
> e.g. for RT298 on multiple platforms, let's be careful here, shall we?
> 
> And I am coming to my last point. The Skylake driver already contains code
> to create the dmic devices by hand (see below the git grep results). So I
> wonder what happens if you use both ACPI-based enumeration AND manually
> create the dmic device - I view these solutions as mutually incompatible.
> Either you have not tested against the upstream code or something is missing
> from your patchset. What am I missing?

That is very valid point. Also DMIC clock is generated by Audio block
and not really a system clock, so why shouldn't this be described in
audio block.

Furthermore I recall we have NHLT table and some description for DMICs.
The driver currently parses that information to get number of dmics to
use (see skl_get_dmic_geo). I would think that adding this delay to NHLT
would be more sensible and pass it on to whatever entity wishes to use
it.

-- 
~Vinod