[PATCH v2 0/3] ASoC: mchp-pdmc: fix poc noises when starting capture

[PATCH v2 0/3] ASoC: mchp-pdmc: fix poc noises when starting capture

[Claudiu Beznea]

To start capture on Microchip PDMC the enable bits for each supported
microphone need to be set. After this bit is set the PDMC starts to
receive data from microphones and it considers this data as valid data.
Thus if microphones are not ready the PDMC captures anyway data from its
lines. This data is interpreted by the human ear as poc noises.

To avoid this the following software workaround need to be applied when
starting capture:
1/ enable PDMC channel
2/ wait 150ms
3/ execute 16 dummy reads from RHR
4/ clear interrupts
5/ enable interrupts
6/ enable DMA channel

For this workaround to work step 6 need to be executed at the end.
For step 6 was added patch 1/3 from this series. With this, component
DAI driver sets its struct snd_soc_component_driver::start_dma_last = 1
and proper action is taken based on this flag when starting DAI trigger
vs DMA.

Thank you,
Claudiu Beznea

Changes in v2:
- patch 1/3 from v1 is now "ASoC: soc-pcm: add option to start DMA after DAI"
- pass start_dma_last from component DAI driver object
  (struct snd_soc_component_driver::start_dma_last); adapt patch 3/3 after this;
- in patch 1/3 s/Do we need to start dma first/Do we need to start dma last
  in comment from soc_pcm_trigger()
- collect review tag from Krzysztof

Claudiu Beznea (3):
  ASoC: soc-pcm: add option to start DMA after DAI
  ASoC: dt-bindings: sama7g5-pdmc: add microchip,startup-delay-us
    binding
  ASoC: mchp-pdmc: fix poc noise at capture startup

 .../sound/microchip,sama7g5-pdmc.yaml         |  6 ++
 include/sound/soc-component.h                 |  2 +
 sound/soc/atmel/mchp-pdmc.c                   | 55 +++++++++++++++++--
 sound/soc/soc-pcm.c                           | 27 +++++++--
 4 files changed, 80 insertions(+), 10 deletions(-)

-- 
2.34.1

[PATCH v2 1/3] ASoC: soc-pcm: add option to start DMA after DAI

[Claudiu Beznea]

Add option to start DMA component after DAI trigger. This is done
by filling the new struct snd_soc_component_driver::start_dma_last.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 include/sound/soc-component.h |  2 ++
 sound/soc/soc-pcm.c           | 27 ++++++++++++++++++++++-----
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h
index 3203d35bc8c1..0814ed143864 100644
--- a/include/sound/soc-component.h
+++ b/include/sound/soc-component.h
@@ -190,6 +190,8 @@ struct snd_soc_component_driver {
 	bool use_dai_pcm_id;	/* use DAI link PCM ID as PCM device number */
 	int be_pcm_base;	/* base device ID for all BE PCMs */
 
+	unsigned int start_dma_last;
+
 #ifdef CONFIG_DEBUG_FS
 	const char *debugfs_prefix;
 #endif
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 005b179a770a..5eb056b942ce 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1088,22 +1088,39 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
 static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 {
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
-	int ret = -EINVAL, _ret = 0;
+	struct snd_soc_component *component;
+	int ret = -EINVAL, _ret = 0, start_dma_last = 0, i;
 	int rollback = 0;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		/* Do we need to start dma last? */
+		for_each_rtd_components(rtd, i, component) {
+			if (component->driver->start_dma_last) {
+				start_dma_last = 1;
+				break;
+			}
+		}
+
 		ret = snd_soc_link_trigger(substream, cmd, 0);
 		if (ret < 0)
 			goto start_err;
 
-		ret = snd_soc_pcm_component_trigger(substream, cmd, 0);
-		if (ret < 0)
-			goto start_err;
+		if (start_dma_last) {
+			ret = snd_soc_pcm_dai_trigger(substream, cmd, 0);
+			if (ret < 0)
+				goto start_err;
+
+			ret = snd_soc_pcm_component_trigger(substream, cmd, 0);
+		} else {
+			ret = snd_soc_pcm_component_trigger(substream, cmd, 0);
+			if (ret < 0)
+				goto start_err;
 
-		ret = snd_soc_pcm_dai_trigger(substream, cmd, 0);
+			ret = snd_soc_pcm_dai_trigger(substream, cmd, 0);
+		}
 start_err:
 		if (ret < 0)
 			rollback = 1;
-- 
2.34.1

Re: [PATCH v2 1/3] ASoC: soc-pcm: add option to start DMA after DAI

[Amadeusz Sławiński]

On 2/17/2023 1:41 PM, Claudiu Beznea wrote:
> Add option to start DMA component after DAI trigger. This is done
> by filling the new struct snd_soc_component_driver::start_dma_last.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>   include/sound/soc-component.h |  2 ++
>   sound/soc/soc-pcm.c           | 27 ++++++++++++++++++++++-----
>   2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h
> index 3203d35bc8c1..0814ed143864 100644
> --- a/include/sound/soc-component.h
> +++ b/include/sound/soc-component.h
> @@ -190,6 +190,8 @@ struct snd_soc_component_driver {
>   	bool use_dai_pcm_id;	/* use DAI link PCM ID as PCM device number */
>   	int be_pcm_base;	/* base device ID for all BE PCMs */
>   
> +	unsigned int start_dma_last;
> +
>   #ifdef CONFIG_DEBUG_FS
>   	const char *debugfs_prefix;
>   #endif
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 005b179a770a..5eb056b942ce 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -1088,22 +1088,39 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
>   static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
>   {
>   	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
> -	int ret = -EINVAL, _ret = 0;
> +	struct snd_soc_component *component;
> +	int ret = -EINVAL, _ret = 0, start_dma_last = 0, i;
>   	int rollback = 0;
>   
>   	switch (cmd) {
>   	case SNDRV_PCM_TRIGGER_START:
>   	case SNDRV_PCM_TRIGGER_RESUME:
>   	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> +		/* Do we need to start dma last? */
> +		for_each_rtd_components(rtd, i, component) {
> +			if (component->driver->start_dma_last) {
> +				start_dma_last = 1;
> +				break;
> +			}
> +		}
> +
>   		ret = snd_soc_link_trigger(substream, cmd, 0);
>   		if (ret < 0)
>   			goto start_err;
>   
> -		ret = snd_soc_pcm_component_trigger(substream, cmd, 0);
> -		if (ret < 0)
> -			goto start_err;
> +		if (start_dma_last) {
> +			ret = snd_soc_pcm_dai_trigger(substream, cmd, 0);
> +			if (ret < 0)
> +				goto start_err;
> +
> +			ret = snd_soc_pcm_component_trigger(substream, cmd, 0);
> +		} else {
> +			ret = snd_soc_pcm_component_trigger(substream, cmd, 0);
> +			if (ret < 0)
> +				goto start_err;
>   
> -		ret = snd_soc_pcm_dai_trigger(substream, cmd, 0);
> +			ret = snd_soc_pcm_dai_trigger(substream, cmd, 0);
> +		}
>   start_err:
>   		if (ret < 0)
>   			rollback = 1;

Can all of the above be implemented similarly to already present 
stop_dma_first? It looks similar and I don't see reason to have one flag 
in snd_soc_component_driver and other in snd_soc_dai_link.

Re: [PATCH v2 1/3] ASoC: soc-pcm: add option to start DMA after DAI

[Claudiu.Beznea]

On 17.02.2023 15:09, Amadeusz Sławiński wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On 2/17/2023 1:41 PM, Claudiu Beznea wrote:
>> Add option to start DMA component after DAI trigger. This is done
>> by filling the new struct snd_soc_component_driver::start_dma_last.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>   include/sound/soc-component.h |  2 ++
>>   sound/soc/soc-pcm.c           | 27 ++++++++++++++++++++++-----
>>   2 files changed, 24 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h
>> index 3203d35bc8c1..0814ed143864 100644
>> --- a/include/sound/soc-component.h
>> +++ b/include/sound/soc-component.h
>> @@ -190,6 +190,8 @@ struct snd_soc_component_driver {
>>       bool use_dai_pcm_id;    /* use DAI link PCM ID as PCM device number */
>>       int be_pcm_base;        /* base device ID for all BE PCMs */
>>
>> +     unsigned int start_dma_last;
>> +
>>   #ifdef CONFIG_DEBUG_FS
>>       const char *debugfs_prefix;
>>   #endif
>> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
>> index 005b179a770a..5eb056b942ce 100644
>> --- a/sound/soc/soc-pcm.c
>> +++ b/sound/soc/soc-pcm.c
>> @@ -1088,22 +1088,39 @@ static int soc_pcm_hw_params(struct
>> snd_pcm_substream *substream,
>>   static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
>>   {
>>       struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
>> -     int ret = -EINVAL, _ret = 0;
>> +     struct snd_soc_component *component;
>> +     int ret = -EINVAL, _ret = 0, start_dma_last = 0, i;
>>       int rollback = 0;
>>
>>       switch (cmd) {
>>       case SNDRV_PCM_TRIGGER_START:
>>       case SNDRV_PCM_TRIGGER_RESUME:
>>       case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>> +             /* Do we need to start dma last? */
>> +             for_each_rtd_components(rtd, i, component) {
>> +                     if (component->driver->start_dma_last) {
>> +                             start_dma_last = 1;
>> +                             break;
>> +                     }
>> +             }
>> +
>>               ret = snd_soc_link_trigger(substream, cmd, 0);
>>               if (ret < 0)
>>                       goto start_err;
>>
>> -             ret = snd_soc_pcm_component_trigger(substream, cmd, 0);
>> -             if (ret < 0)
>> -                     goto start_err;
>> +             if (start_dma_last) {
>> +                     ret = snd_soc_pcm_dai_trigger(substream, cmd, 0);
>> +                     if (ret < 0)
>> +                             goto start_err;
>> +
>> +                     ret = snd_soc_pcm_component_trigger(substream, cmd,
>> 0);
>> +             } else {
>> +                     ret = snd_soc_pcm_component_trigger(substream, cmd,
>> 0);
>> +                     if (ret < 0)
>> +                             goto start_err;
>>
>> -             ret = snd_soc_pcm_dai_trigger(substream, cmd, 0);
>> +                     ret = snd_soc_pcm_dai_trigger(substream, cmd, 0);
>> +             }
>>   start_err:
>>               if (ret < 0)
>>                       rollback = 1;
> 
> Can all of the above be implemented similarly to already present
> stop_dma_first? It looks similar and I don't see reason to have one flag
> in snd_soc_component_driver and other in snd_soc_dai_link.

That was the other solution identified; I mentioned it in v1; from v1 cover
letter/discussions:

The other solution that was identified for this was to extend the already
existing mechanism around struct snd_soc_dai_link::stop_dma_first. The downside
of this was that a potential struct snd_soc_dai_link::start_dma_last
would have to be populated on sound card driver thus, had to be taken
into account in all sound card drivers. At the moment, the mchp-pdmc is
used only with simple-audio-card. In case of simple-audio-card a new DT
binding would had to be introduced to specify this action on dai-link
descriptions (as of my investigation).

> 
> 

[PATCH v2 2/3] ASoC: dt-bindings: sama7g5-pdmc: add microchip,startup-delay-us binding

[Claudiu Beznea]

Add microchip,startup-delay-us binding to let PDMC users to specify
startup delay.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml   | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml b/Documentation/devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml
index c4cf1e5ab84b..9b40268537cb 100644
--- a/Documentation/devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml
+++ b/Documentation/devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml
@@ -67,6 +67,12 @@ properties:
     maxItems: 4
     uniqueItems: true
 
+  microchip,startup-delay-us:
+    description: |
+      Specifies the delay in microseconds that needs to be applied after
+      enabling the PDMC microphones to avoid unwanted noise due to microphones
+      not being ready.
+
 required:
   - compatible
   - reg
-- 
2.34.1

Re: [PATCH v2 2/3] ASoC: dt-bindings: sama7g5-pdmc: add microchip,startup-delay-us binding

[Rob Herring]

On Fri, Feb 17, 2023 at 02:41:50PM +0200, Claudiu Beznea wrote:
> Add microchip,startup-delay-us binding to let PDMC users to specify
> startup delay.

The diff tells me all this. Why does this need to be per platform?

> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml   | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml b/Documentation/devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml
> index c4cf1e5ab84b..9b40268537cb 100644
> --- a/Documentation/devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml
> +++ b/Documentation/devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml
> @@ -67,6 +67,12 @@ properties:
>      maxItems: 4
>      uniqueItems: true
>  
> +  microchip,startup-delay-us:
> +    description: |
> +      Specifies the delay in microseconds that needs to be applied after
> +      enabling the PDMC microphones to avoid unwanted noise due to microphones
> +      not being ready.
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.34.1
> 

Re: [PATCH v2 2/3] ASoC: dt-bindings: sama7g5-pdmc: add microchip,startup-delay-us binding

[Claudiu.Beznea]

On 21.02.2023 00:56, Rob Herring wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, Feb 17, 2023 at 02:41:50PM +0200, Claudiu Beznea wrote:
>> Add microchip,startup-delay-us binding to let PDMC users to specify
>> startup delay.
> 
> The diff tells me all this. Why does this need to be per platform?

PDMC can work with different kind of microphones, thus different boards
could have different microphones. Depending on microphone type the PDMC
would need to wait longer or shorter period than the default chosen period
to filter unwanted noise. Thus the need of having this specified though
device tree. Would you prefer to have this in commit message?

Thank you,
Claudiu

> 
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>  .../devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml   | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml b/Documentation/devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml
>> index c4cf1e5ab84b..9b40268537cb 100644
>> --- a/Documentation/devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml
>> +++ b/Documentation/devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml
>> @@ -67,6 +67,12 @@ properties:
>>      maxItems: 4
>>      uniqueItems: true
>>
>> +  microchip,startup-delay-us:
>> +    description: |
>> +      Specifies the delay in microseconds that needs to be applied after
>> +      enabling the PDMC microphones to avoid unwanted noise due to microphones
>> +      not being ready.
>> +
>>  required:
>>    - compatible
>>    - reg
>> --
>> 2.34.1
>>

Re: [PATCH v2 2/3] ASoC: dt-bindings: sama7g5-pdmc: add microchip,startup-delay-us binding

[Krzysztof Kozlowski]

On 21/02/2023 09:10, Claudiu.Beznea@microchip.com wrote:
> On 21.02.2023 00:56, Rob Herring wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On Fri, Feb 17, 2023 at 02:41:50PM +0200, Claudiu Beznea wrote:
>>> Add microchip,startup-delay-us binding to let PDMC users to specify
>>> startup delay.
>>
>> The diff tells me all this. Why does this need to be per platform?
> 
> PDMC can work with different kind of microphones, thus different boards
> could have different microphones. Depending on microphone type the PDMC
> would need to wait longer or shorter period than the default chosen period
> to filter unwanted noise. Thus the need of having this specified though
> device tree. Would you prefer to have this in commit message?

I believe you also had explain it to me, thus as you can see having it
in commit msg would spare you two questions...

Best regards,
Krzysztof

Re: [PATCH v2 2/3] ASoC: dt-bindings: sama7g5-pdmc: add microchip,startup-delay-us binding

[Claudiu.Beznea]

On 21.02.2023 11:23, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 21/02/2023 09:10, Claudiu.Beznea@microchip.com wrote:
>> On 21.02.2023 00:56, Rob Herring wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On Fri, Feb 17, 2023 at 02:41:50PM +0200, Claudiu Beznea wrote:
>>>> Add microchip,startup-delay-us binding to let PDMC users to specify
>>>> startup delay.
>>>
>>> The diff tells me all this. Why does this need to be per platform?
>>
>> PDMC can work with different kind of microphones, thus different boards
>> could have different microphones. Depending on microphone type the PDMC
>> would need to wait longer or shorter period than the default chosen period
>> to filter unwanted noise. Thus the need of having this specified though
>> device tree. Would you prefer to have this in commit message?
> 
> I believe you also had explain it to me, thus as you can see having it
> in commit msg would spare you two questions...

Right, I'll add it in the next version.

Thank you,
Claudiu

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 2/3] ASoC: dt-bindings: sama7g5-pdmc: add microchip,startup-delay-us binding

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 795 bytes --]

On Tue, Feb 21, 2023 at 10:52:54AM +0000, Claudiu.Beznea@microchip.com wrote:
> On 21.02.2023 11:23, Krzysztof Kozlowski wrote:

> >> PDMC can work with different kind of microphones, thus different boards
> >> could have different microphones. Depending on microphone type the PDMC
> >> would need to wait longer or shorter period than the default chosen period
> >> to filter unwanted noise. Thus the need of having this specified though
> >> device tree. Would you prefer to have this in commit message?

> > I believe you also had explain it to me, thus as you can see having it
> > in commit msg would spare you two questions...

> Right, I'll add it in the next version.

TBH this is really standard stuff for audio hardware, having to leave
board specific settling delays is very normal.

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

[PATCH v2 3/3] ASoC: mchp-pdmc: fix poc noise at capture startup

[Claudiu Beznea]

Microchip PDMC IP doesn't filter microphone noises on startup. By default,
it captures data received from digital microphones after
the MCHP_PDMC_MR.EN bits are set. Thus when enable is set on PDMC side the
digital microphones might not be ready yet and PDMC captures data from then
in this time. This data captured is poc noise. To avoid this the software
workaround is to the following:
1/ enable PDMC channel
2/ wait 150ms (on SAMA7G5-EK setup)
3/ execute 16 dummy reads from RHR
4/ clear interrupts
5/ enable interrupts
6/ enable DMA channel

Fixes: 50291652af52 ("ASoC: atmel: mchp-pdmc: add PDMC driver")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---

Hi, Mark,

If this is applied as is please add proper Depends-on tag to point to
patch 1/3 commit id to ease the backporting on older kernels, if any.

Thank you,
Claudiu

 sound/soc/atmel/mchp-pdmc.c | 55 +++++++++++++++++++++++++++++++++----
 1 file changed, 50 insertions(+), 5 deletions(-)

diff --git a/sound/soc/atmel/mchp-pdmc.c b/sound/soc/atmel/mchp-pdmc.c
index cf4084dcbd5e..6461e2741a33 100644
--- a/sound/soc/atmel/mchp-pdmc.c
+++ b/sound/soc/atmel/mchp-pdmc.c
@@ -114,6 +114,7 @@ struct mchp_pdmc {
 	struct clk *gclk;
 	u32 pdmcen;
 	u32 suspend_irq;
+	u32 startup_delay_us;
 	int mic_no;
 	int sinc_order;
 	bool audio_filter_en;
@@ -425,6 +426,7 @@ static const struct snd_soc_component_driver mchp_pdmc_dai_component = {
 	.open = &mchp_pdmc_open,
 	.close = &mchp_pdmc_close,
 	.legacy_dai_naming = 1,
+	.start_dma_last = 1,
 };
 
 static const unsigned int mchp_pdmc_1mic[] = {1};
@@ -632,6 +634,29 @@ static int mchp_pdmc_hw_params(struct snd_pcm_substream *substream,
 	return 0;
 }
 
+static void mchp_pdmc_noise_filter_workaround(struct mchp_pdmc *dd)
+{
+	u32 tmp, steps = 16;
+
+	/*
+	 * PDMC doesn't wait for microphones' startup time thus the acquisition
+	 * may start before the microphones are ready leading to poc noises at
+	 * the beginning of capture. To avoid this, we need to wait 50ms (in
+	 * normal startup procedure) or 150 ms (worst case after resume from sleep
+	 * states) after microphones are enabled and then clear the FIFOs (by
+	 * reading the RHR 16 times) and possible interrupts before continuing.
+	 * Also, for this to work the DMA needs to be started after interrupts
+	 * are enabled.
+	 */
+	usleep_range(dd->startup_delay_us, dd->startup_delay_us + 5);
+
+	while (steps--)
+		regmap_read(dd->regmap, MCHP_PDMC_RHR, &tmp);
+
+	/* Clear interrupts. */
+	regmap_read(dd->regmap, MCHP_PDMC_ISR, &tmp);
+}
+
 static int mchp_pdmc_trigger(struct snd_pcm_substream *substream,
 			     int cmd, struct snd_soc_dai *dai)
 {
@@ -644,15 +669,17 @@ static int mchp_pdmc_trigger(struct snd_pcm_substream *substream,
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_START:
-		/* Enable overrun and underrun error interrupts */
-		regmap_write(dd->regmap, MCHP_PDMC_IER, dd->suspend_irq |
-			     MCHP_PDMC_IR_RXOVR | MCHP_PDMC_IR_RXUDR);
-		dd->suspend_irq = 0;
-		fallthrough;
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		snd_soc_component_update_bits(cpu, MCHP_PDMC_MR,
 					      MCHP_PDMC_MR_PDMCEN_MASK,
 					      dd->pdmcen);
+
+		mchp_pdmc_noise_filter_workaround(dd);
+
+		/* Enable interrupts. */
+		regmap_write(dd->regmap, MCHP_PDMC_IER, dd->suspend_irq |
+			     MCHP_PDMC_IR_RXOVR | MCHP_PDMC_IR_RXUDR);
+		dd->suspend_irq = 0;
 		break;
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 		regmap_read(dd->regmap, MCHP_PDMC_IMR, &dd->suspend_irq);
@@ -796,6 +823,7 @@ static bool mchp_pdmc_readable_reg(struct device *dev, unsigned int reg)
 	case MCHP_PDMC_CFGR:
 	case MCHP_PDMC_IMR:
 	case MCHP_PDMC_ISR:
+	case MCHP_PDMC_RHR:
 	case MCHP_PDMC_VER:
 		return true;
 	default:
@@ -817,6 +845,17 @@ static bool mchp_pdmc_writeable_reg(struct device *dev, unsigned int reg)
 	}
 }
 
+static bool mchp_pdmc_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case MCHP_PDMC_ISR:
+	case MCHP_PDMC_RHR:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static bool mchp_pdmc_precious_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
@@ -836,6 +875,7 @@ static const struct regmap_config mchp_pdmc_regmap_config = {
 	.readable_reg	= mchp_pdmc_readable_reg,
 	.writeable_reg	= mchp_pdmc_writeable_reg,
 	.precious_reg	= mchp_pdmc_precious_reg,
+	.volatile_reg	= mchp_pdmc_volatile_reg,
 	.cache_type	= REGCACHE_FLAT,
 };
 
@@ -918,6 +958,11 @@ static int mchp_pdmc_dt_init(struct mchp_pdmc *dd)
 		dd->channel_mic_map[i].clk_edge = edge;
 	}
 
+	ret = of_property_read_u32(np, "microchip,startup-delay-us",
+				   &dd->startup_delay_us);
+	if (ret)
+		dd->startup_delay_us = 150000;
+
 	return 0;
 }
 
-- 
2.34.1