[RFC 0/2] Add MICFIL DAI support

[RFC 0/2] Add MICFIL DAI support

[Cosmin Samoila]

PDM is a popular way to deliver audio from microphones to the processor
in several applications, such as mobile telephones. However, current
digital-audio systems use multibit audio signal (also known as multibit
PCM) to represent the signal. For this purpose a set of FIR, CIC or/and
Half Band filters are usually implemented on DSPs or software. This block
implements the required digital interface to provide a 16-bit audio signal
from a PDM microphone bitstream in a configurable output sampling rate.
The implementation of this digital interface is based on the application
of digital signal processing techniques in hardware. The PDM Microphone
Interface architecture was designed to gate saving and minimal power
consumption. It implements a bunch of filters to transform a 1-bit PDM
bitstream to a 16-bit PCM signal in the audio band. To avoid aliasing
frequencies in passband, the overall filter has 80 dB stopband attenuation
and passband ripple less than 0.2dB.
The whole module is implemented to work in a multichannel mode. All
channels have the same configuration but each input channel could be turned
on/off independently.


Cosmin-Gabriel Samoila (2):
  ASoC: micfil: Add bindings for MICFIL DAI
  ASoC: Add MICFIL SoC Digital Audio Interface driver.

 .../devicetree/bindings/sound/fsl,micfil.txt       |   38 +
 sound/soc/fsl/Kconfig                              |    9 +
 sound/soc/fsl/Makefile                             |    2 +
 sound/soc/fsl/fsl_micfil.c                         | 2451 ++++++++++++++++++++
 sound/soc/fsl/fsl_micfil.h                         |  317 +++
 5 files changed, 2817 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/fsl,micfil.txt
 create mode 100644 sound/soc/fsl/fsl_micfil.c
 create mode 100644 sound/soc/fsl/fsl_micfil.h

-- 
2.7.4

[RFC 1/2] ASoC: micfil: Add bindings for MICFIL DAI

[Cosmin Samoila]

Document the bindings for MICFIL DAI.

Signed-off-by: Cosmin-Gabriel Samoila <cosmin.samoila@nxp.com>
---
 .../devicetree/bindings/sound/fsl,micfil.txt       | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/fsl,micfil.txt

diff --git a/Documentation/devicetree/bindings/sound/fsl,micfil.txt b/Documentation/devicetree/bindings/sound/fsl,micfil.txt
new file mode 100644
index 0000000..2aa4526
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/fsl,micfil.txt
@@ -0,0 +1,38 @@
+NXP MICFIL Digital Audio Interface (MICFIL).
+
+The MICFIL digital interface provides a 16-bit audio signal from a PDM
+microphone bitstream in a configurable output sampling rate.
+
+Required properties:
+
+  - compatible		: Compatible list, contains "fsl,imx8mm-micfil"
+
+  - reg			: Offset and length of the register set for the device.
+
+  - interrupts		: Contains the micfil interrupts.
+
+  - clocks		: Must contain an entry for each entry in clock-names.
+
+  - clock-names		: Must include the "ipg_clk" for register access and
+			  "ipg_clk_app" for internal micfil clock.
+
+  - dmas		: Generic dma devicetree binding as described in
+			  Documentation/devicetree/bindings/dma/dma.txt.
+
+  - dma-names		: One "rx" dma must be configured.
+
+Example:
+micfil: micfil@30080000 {
+	compatible = "fsl,imx8mm-micfil";
+	reg = <0x0 0x30080000 0x0 0x10000>;
+	interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&clk IMX8MM_CLK_PDM_IPG>,
+		 <&clk IMX8MM_CLK_PDM_ROOT>;
+	clock-names = "ipg_clk", "ipg_clk_app";
+	dmas = <&sdma2 24 26 0x80000000>;
+	dma-names = "rx";
+	status = "disabled";
+};
-- 
2.7.4

Re: [RFC 1/2] ASoC: micfil: Add bindings for MICFIL DAI

[Rob Herring]

On Mon, Dec 10, 2018 at 09:21:14AM +0000, Cosmin Samoila wrote:
> Document the bindings for MICFIL DAI.
> 
> Signed-off-by: Cosmin-Gabriel Samoila <cosmin.samoila@nxp.com>
> ---
>  .../devicetree/bindings/sound/fsl,micfil.txt       | 38 ++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/fsl,micfil.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/fsl,micfil.txt b/Documentation/devicetree/bindings/sound/fsl,micfil.txt
> new file mode 100644
> index 0000000..2aa4526
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/fsl,micfil.txt
> @@ -0,0 +1,38 @@
> +NXP MICFIL Digital Audio Interface (MICFIL).
> +
> +The MICFIL digital interface provides a 16-bit audio signal from a PDM
> +microphone bitstream in a configurable output sampling rate.
> +
> +Required properties:
> +
> +  - compatible		: Compatible list, contains "fsl,imx8mm-micfil"
> +
> +  - reg			: Offset and length of the register set for the device.
> +
> +  - interrupts		: Contains the micfil interrupts.
> +
> +  - clocks		: Must contain an entry for each entry in clock-names.
> +
> +  - clock-names		: Must include the "ipg_clk" for register access and
> +			  "ipg_clk_app" for internal micfil clock.
> +
> +  - dmas		: Generic dma devicetree binding as described in
> +			  Documentation/devicetree/bindings/dma/dma.txt.
> +
> +  - dma-names		: One "rx" dma must be configured.

You don't need -names when there is only one.

> +
> +Example:
> +micfil: micfil@30080000 {
> +	compatible = "fsl,imx8mm-micfil";
> +	reg = <0x0 0x30080000 0x0 0x10000>;
> +	interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
> +	clocks = <&clk IMX8MM_CLK_PDM_IPG>,
> +		 <&clk IMX8MM_CLK_PDM_ROOT>;
> +	clock-names = "ipg_clk", "ipg_clk_app";
> +	dmas = <&sdma2 24 26 0x80000000>;
> +	dma-names = "rx";
> +	status = "disabled";

Don't show status in examples.

> +};
> -- 
> 2.7.4
> 

[RFC 2/2] ASoC: Add MICFIL SoC Digital Audio Interface driver.

[Cosmin Samoila]

Add Digital Audio Interface driver that convers PDM bitstream to PCM
format.

Features:
- Fixed filtering characteristics for audio application.
- Full or partial set of channels operation with individual enable control.
- Programmable PDM clock generator.
- Programmable decimation rate.
- 16-bit signed output result.
- Overall stopband attenuation more than 80dB.
- Overall passband ripple less than 0.2dB.
- Programmable cut-off frequency.
- Hardware Voice Activity Detector (HWVAD)

Signed-off-by: Cosmin-Gabriel Samoila <cosmin.samoila@nxp.com>
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/Kconfig      |    9 +
 sound/soc/fsl/Makefile     |    2 +
 sound/soc/fsl/fsl_micfil.c | 2451 ++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/fsl/fsl_micfil.h |  317 ++++++
 4 files changed, 2779 insertions(+)
 create mode 100644 sound/soc/fsl/fsl_micfil.c
 create mode 100644 sound/soc/fsl/fsl_micfil.h

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index 6ec19fb..fe0f339 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -57,6 +57,15 @@ config SND_SOC_FSL_ESAI
 	  This option is only useful for out-of-tree drivers since
 	  in-tree drivers select it automatically.
 
+config SND_SOC_FSL_MICFIL
+	tristate "Pulse Density Modulation Microphone Interface (MICFIL) module support"
+	select REGMAP_MMIO
+	select SND_SOC_IMX_PCM_DMA if SND_IMX_SOC != n
+	select SND_SOC_GENERIC_DMAENGINE_PCM
+	help
+	  Say Y if you want to add Pulse Density Modulation microphone
+	  interface (MICFIL) support for NXP.
+
 config SND_SOC_FSL_UTILS
 	tristate
 
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
index de94fa0..3c0ff31 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -19,6 +19,7 @@ snd-soc-fsl-ssi-y := fsl_ssi.o
 snd-soc-fsl-ssi-$(CONFIG_DEBUG_FS) += fsl_ssi_dbg.o
 snd-soc-fsl-spdif-objs := fsl_spdif.o
 snd-soc-fsl-esai-objs := fsl_esai.o
+snd-soc-fsl-micfil-objs := fsl_micfil.o
 snd-soc-fsl-utils-objs := fsl_utils.o
 snd-soc-fsl-dma-objs := fsl_dma.o
 obj-$(CONFIG_SND_SOC_FSL_ASOC_CARD) += snd-soc-fsl-asoc-card.o
@@ -27,6 +28,7 @@ obj-$(CONFIG_SND_SOC_FSL_SAI) += snd-soc-fsl-sai.o
 obj-$(CONFIG_SND_SOC_FSL_SSI) += snd-soc-fsl-ssi.o
 obj-$(CONFIG_SND_SOC_FSL_SPDIF) += snd-soc-fsl-spdif.o
 obj-$(CONFIG_SND_SOC_FSL_ESAI) += snd-soc-fsl-esai.o
+obj-$(CONFIG_SND_SOC_FSL_MICFIL) += snd-soc-fsl-micfil.o
 obj-$(CONFIG_SND_SOC_FSL_UTILS) += snd-soc-fsl-utils.o
 obj-$(CONFIG_SND_SOC_POWERPC_DMA) += snd-soc-fsl-dma.o
 
diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
new file mode 100644
index 0000000..ddf188e
--- /dev/null
+++ b/sound/soc/fsl/fsl_micfil.c
@@ -0,0 +1,2451 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright 2018 NXP
+
+#include <linux/atomic.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/interrupt.h>
+#include <linux/kobject.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+#include <sound/dmaengine_pcm.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <sound/tlv.h>
+#include <sound/core.h>
+
+#include "fsl_micfil.h"
+#include "imx-pcm.h"
+
+#define FSL_MICFIL_RATES		SNDRV_PCM_RATE_8000_48000
+#define FSL_MICFIL_FORMATS		(SNDRV_PCM_FMTBIT_S16_LE)
+
+struct fsl_micfil {
+	struct platform_device *pdev;
+	struct regmap *regmap;
+	const struct fsl_micfil_soc_data *soc;
+	struct clk *mclk;
+	struct clk *clk_src[MICFIL_CLK_SRC_NUM];
+	struct snd_dmaengine_dai_dma_data dma_params_rx;
+	struct kobject *hwvad_kobject;
+	unsigned int vad_channel;
+	unsigned int dataline;
+	char name[32];
+	int irq[MICFIL_IRQ_LINES];
+	unsigned int mclk_streams;
+	int quality;	/*QUALITY 2-0 bits */
+	bool slave_mode;
+	int channel_gain[8];
+	int clk_src_id;
+	int vad_sound_gain;
+	int vad_noise_gain;
+	int vad_input_gain;
+	int vad_frame_time;
+	int vad_init_time;
+	int vad_init_mode;
+	int vad_nfil_adjust;
+	int vad_hpf;
+	int vad_zcd_th;
+	int vad_zcd_auto;
+	int vad_zcd_en;
+	int vad_zcd_adj;
+	int vad_rate_index;
+	atomic_t recording_state;
+	atomic_t hwvad_state;
+};
+
+struct fsl_micfil_soc_data {
+	unsigned int fifos;
+	unsigned int fifo_depth;
+	unsigned int dataline;
+	bool imx;
+};
+
+static char *envp[] = {
+		"EVENT=PDM_VOICE_DETECT",
+		NULL,
+	};
+
+static struct fsl_micfil_soc_data fsl_micfil_imx8mm = {
+	.imx = true,
+	.fifos = 8,
+	.fifo_depth = 8,
+	.dataline =  0xf,
+};
+
+static const struct of_device_id fsl_micfil_dt_ids[] = {
+	{ .compatible = "fsl,imx8mm-micfil", .data = &fsl_micfil_imx8mm },
+	{}
+};
+MODULE_DEVICE_TABLE(of, fsl_micfil_dt_ids);
+
+/* Table 5. Quality Modes
+ * Medium	0 0 0
+ * High		0 0 1
+ * Very Low 2	1 0 0
+ * Very Low 1	1 0 1
+ * Very Low 0	1 1 0
+ * Low		1 1 1
+ */
+static const char * const micfil_quality_select_texts[] = {
+	"Medium", "High",
+	"N/A", "N/A",
+	"VLow2", "VLow1",
+	"VLow0", "Low",
+};
+
+static const char * const micfil_hwvad_init_mode[] = {
+	"Envelope mode", "Energy mode",
+};
+
+static const char * const micfil_hwvad_hpf_texts[] = {
+	"Filter bypass",
+	"Cut-off @1750Hz",
+	"Cut-off @215Hz",
+	"Cut-off @102Hz",
+};
+
+static const char * const micfil_hwvad_zcd_enable[] = {
+	"OFF", "ON",
+};
+
+static const char * const micfil_hwvad_zcdauto_enable[] = {
+	"OFF", "ON",
+};
+
+static const char * const micfil_hwvad_noise_decimation[] = {
+	"Disabled", "Enabled",
+};
+
+/* when adding new rate text, also add it to the
+ * micfil_hwvad_rate_ints
+ */
+static const char * const micfil_hwvad_rate[] = {
+	"48KHz", "44.1KHz",
+};
+
+static const int micfil_hwvad_rate_ints[] = {
+	48000, 44100,
+};
+
+static const char * const micfil_clk_src_texts[] = {
+	"Auto", "AudioPLL1", "AudioPLL2", "ExtClk3",
+};
+
+static const struct soc_enum fsl_micfil_quality_enum =
+	SOC_ENUM_SINGLE(REG_MICFIL_CTRL2,
+			MICFIL_CTRL2_QSEL_SHIFT,
+			ARRAY_SIZE(micfil_quality_select_texts),
+			micfil_quality_select_texts);
+static const struct soc_enum fsl_micfil_hwvad_init_mode_enum =
+	SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(micfil_hwvad_init_mode),
+			    micfil_hwvad_init_mode);
+static const struct soc_enum fsl_micfil_hwvad_hpf_enum =
+	SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(micfil_hwvad_hpf_texts),
+			    micfil_hwvad_hpf_texts);
+static const struct soc_enum fsl_micfil_hwvad_zcd_enum =
+	SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(micfil_hwvad_zcd_enable),
+			    micfil_hwvad_zcd_enable);
+static const struct soc_enum fsl_micfil_hwvad_zcdauto_enum =
+	SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(micfil_hwvad_zcdauto_enable),
+			    micfil_hwvad_zcd_enable);
+static const struct soc_enum fsl_micfil_hwvad_ndec_enum =
+	SOC_ENUM_SINGLE(REG_MICFIL_VAD0_NCONFIG,
+			MICFIL_VAD0_NCONFIG_NOREN_SHIFT,
+			ARRAY_SIZE(micfil_hwvad_noise_decimation),
+			micfil_hwvad_noise_decimation);
+static const struct soc_enum fsl_micfil_hwvad_rate_enum =
+	SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(micfil_hwvad_rate),
+			    micfil_hwvad_rate);
+static const struct soc_enum fsl_micfil_clk_src_enum =
+	SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(micfil_clk_src_texts),
+			    micfil_clk_src_texts);
+
+static int micfil_put_clk_src(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+	unsigned int *item = ucontrol->value.enumerated.item;
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+	int val = snd_soc_enum_item_to_val(e, item[0]);
+
+	micfil->clk_src_id = val;
+
+	return 0;
+}
+
+static int micfil_get_clk_src(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	ucontrol->value.enumerated.item[0] = micfil->clk_src_id;
+
+	return 0;
+}
+
+static int hwvad_put_init_mode(struct snd_kcontrol *kcontrol,
+			       struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+	unsigned int *item = ucontrol->value.enumerated.item;
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+	int val = snd_soc_enum_item_to_val(e, item[0]);
+
+	/* 0 - Envelope-based Mode
+	 * 1 - Energy-based Mode
+	 */
+	micfil->vad_init_mode = val;
+	return 0;
+}
+
+static int hwvad_get_init_mode(struct snd_kcontrol *kcontrol,
+			       struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	ucontrol->value.enumerated.item[0] = micfil->vad_init_mode;
+
+	return 0;
+}
+
+static int hwvad_put_hpf(struct snd_kcontrol *kcontrol,
+			 struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+	unsigned int *item = ucontrol->value.enumerated.item;
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+	int val = snd_soc_enum_item_to_val(e, item[0]);
+
+	/* 00 - HPF Bypass
+	 * 01 - Cut-off frequency 1750Hz
+	 * 10 - Cut-off frequency 215Hz
+	 * 11 - Cut-off frequency 102Hz
+	 */
+	micfil->vad_hpf = val;
+
+	return 0;
+}
+
+static int hwvad_get_hpf(struct snd_kcontrol *kcontrol,
+			 struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	ucontrol->value.enumerated.item[0] = micfil->vad_hpf;
+
+	return 0;
+}
+
+static int hwvad_put_zcd_en(struct snd_kcontrol *kcontrol,
+			    struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+	unsigned int *item = ucontrol->value.enumerated.item;
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+	int val = snd_soc_enum_item_to_val(e, item[0]);
+
+	micfil->vad_zcd_en = val;
+
+	return 0;
+}
+
+static int hwvad_get_zcd_en(struct snd_kcontrol *kcontrol,
+			    struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	ucontrol->value.enumerated.item[0] = micfil->vad_zcd_en;
+
+	return 0;
+}
+
+static int hwvad_put_rate(struct snd_kcontrol *kcontrol,
+			  struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+	unsigned int *item = ucontrol->value.enumerated.item;
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+	int val = snd_soc_enum_item_to_val(e, item[0]);
+
+	micfil->vad_rate_index = val;
+
+	return 0;
+}
+
+static int hwvad_get_rate(struct snd_kcontrol *kcontrol,
+			  struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	ucontrol->value.enumerated.item[0] = micfil->vad_rate_index;
+
+	return 0;
+}
+
+static int hwvad_put_zcd_auto(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+	unsigned int *item = ucontrol->value.enumerated.item;
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+	int val = snd_soc_enum_item_to_val(e, item[0]);
+
+	micfil->vad_zcd_auto = val;
+
+	return 0;
+}
+
+static int hwvad_get_zcd_auto(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	ucontrol->value.enumerated.item[0] = micfil->vad_zcd_auto;
+
+	return 0;
+}
+
+static int gain_info(struct snd_kcontrol *kcontrol,
+		     struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = 1;
+	uinfo->value.integer.min = 0;
+	uinfo->value.integer.max = 0xf;
+
+	return 0;
+}
+
+static int put_channel_gain(struct snd_kcontrol *kcontrol,
+			    struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+	struct soc_mixer_control *mc =
+		(struct soc_mixer_control *)kcontrol->private_value;
+	unsigned int shift = mc->shift;
+	int index = shift / 4;
+	int val = ucontrol->value.integer.value[0];
+	int remapped_value;
+	int ret;
+	u32 reg_val;
+
+	/* a value remapping must be done since the gain field have
+	 * the following meaning:
+	 * * 0 : no gain
+	 * * 1 - 7 : +1 to +7 bits gain
+	 * * 8 - 15 : -8 to -1 bits gain
+	 * After the remapp, the scale should start from -8 to +7
+	 */
+
+	micfil->channel_gain[index] = val;
+
+	remapped_value = (val - 8) & 0xF;
+
+	reg_val = remapped_value << shift;
+
+	ret = snd_soc_component_update_bits(comp,
+					    REG_MICFIL_OUT_CTRL,
+					    0xF << shift,
+					    reg_val);
+
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int get_channel_gain(struct snd_kcontrol *kcontrol,
+			    struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+	struct soc_mixer_control *mc =
+		(struct soc_mixer_control *)kcontrol->private_value;
+	int index;
+
+	/* gain bitfield is 4 bits wide */
+	index = mc->shift / 4;
+
+	ucontrol->value.enumerated.item[0] = micfil->channel_gain[index];
+
+	return 0;
+}
+
+static int hwvad_put_input_gain(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	micfil->vad_input_gain = ucontrol->value.integer.value[0];
+
+	return 0;
+}
+
+static int hwvad_get_input_gain(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	ucontrol->value.enumerated.item[0] = micfil->vad_input_gain;
+
+	return 0;
+}
+
+static int hwvad_put_sound_gain(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	micfil->vad_sound_gain = ucontrol->value.integer.value[0];
+
+	return 0;
+}
+
+static int hwvad_get_sound_gain(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	ucontrol->value.enumerated.item[0] = micfil->vad_sound_gain;
+
+	return 0;
+}
+
+static int hwvad_put_noise_gain(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	micfil->vad_noise_gain = ucontrol->value.integer.value[0];
+
+	return 0;
+}
+
+static int hwvad_get_noise_gain(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	ucontrol->value.enumerated.item[0] = micfil->vad_noise_gain;
+
+	return 0;
+}
+
+static int hwvad_framet_info(struct snd_kcontrol *kcontrol,
+			     struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = 1;
+	uinfo->value.integer.min = 1;
+	uinfo->value.integer.max = 64;
+
+	return 0;
+}
+
+static int hwvad_put_frame_time(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	micfil->vad_frame_time = ucontrol->value.integer.value[0];
+
+	return 0;
+}
+
+static int hwvad_get_frame_time(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	ucontrol->value.enumerated.item[0] = micfil->vad_frame_time;
+
+	return 0;
+}
+
+static int hwvad_initt_info(struct snd_kcontrol *kcontrol,
+			    struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = 1;
+	uinfo->value.integer.min = 1;
+	uinfo->value.integer.max = 32;
+
+	return 0;
+}
+
+static int hwvad_put_init_time(struct snd_kcontrol *kcontrol,
+			       struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	micfil->vad_init_time = ucontrol->value.integer.value[0];
+
+	return 0;
+}
+
+static int hwvad_get_init_time(struct snd_kcontrol *kcontrol,
+			       struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	ucontrol->value.enumerated.item[0] = micfil->vad_init_time;
+
+	return 0;
+}
+
+static int hwvad_nfiladj_info(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = 1;
+	uinfo->value.integer.min = 1;
+	uinfo->value.integer.max = 32;
+
+	return 0;
+}
+
+static int hwvad_put_nfil_adjust(struct snd_kcontrol *kcontrol,
+				 struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	micfil->vad_nfil_adjust = ucontrol->value.integer.value[0];
+
+	return 0;
+}
+
+static int hwvad_get_nfil_adjust(struct snd_kcontrol *kcontrol,
+				 struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	ucontrol->value.enumerated.item[0] = micfil->vad_nfil_adjust;
+
+	return 0;
+}
+
+static int hwvad_zcdth_info(struct snd_kcontrol *kcontrol,
+			    struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = 1;
+	uinfo->value.integer.min = 1;
+	uinfo->value.integer.max = 1024;
+
+	return 0;
+}
+
+static int hwvad_put_zcd_th(struct snd_kcontrol *kcontrol,
+			    struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	micfil->vad_zcd_th = ucontrol->value.integer.value[0];
+
+	return 0;
+}
+
+static int hwvad_get_zcd_th(struct snd_kcontrol *kcontrol,
+			    struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	ucontrol->value.enumerated.item[0] = micfil->vad_zcd_th;
+
+	return 0;
+}
+
+static int hwvad_zcdadj_info(struct snd_kcontrol *kcontrol,
+			     struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = 1;
+	uinfo->value.integer.min = 1;
+	uinfo->value.integer.max = 16;
+
+	return 0;
+}
+
+static int hwvad_put_zcd_adj(struct snd_kcontrol *kcontrol,
+			     struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	micfil->vad_zcd_adj = ucontrol->value.integer.value[0];
+
+	return 0;
+}
+
+static int hwvad_get_zcd_adj(struct snd_kcontrol *kcontrol,
+			     struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+	ucontrol->value.enumerated.item[0] = micfil->vad_zcd_adj;
+
+	return 0;
+}
+
+static DECLARE_TLV_DB_SCALE(gain_tlv, 0, 100, 0);
+
+static const struct snd_kcontrol_new fsl_micfil_snd_controls[] = {
+	SOC_SINGLE_RANGE_EXT_TLV("CH0 Gain", -1, MICFIL_OUTGAIN_CHX_SHIFT(0),
+				 0x0, 0xF, 0,
+				 get_channel_gain, put_channel_gain, gain_tlv),
+	SOC_SINGLE_RANGE_EXT_TLV("CH1 Gain", -1, MICFIL_OUTGAIN_CHX_SHIFT(1),
+				 0x0, 0xF, 0,
+				 get_channel_gain, put_channel_gain, gain_tlv),
+	SOC_SINGLE_RANGE_EXT_TLV("CH2 Gain", -1, MICFIL_OUTGAIN_CHX_SHIFT(2),
+				 0x0, 0xF, 0,
+				 get_channel_gain, put_channel_gain, gain_tlv),
+	SOC_SINGLE_RANGE_EXT_TLV("CH3 Gain", -1, MICFIL_OUTGAIN_CHX_SHIFT(3),
+				 0x0, 0xF, 0,
+				 get_channel_gain, put_channel_gain, gain_tlv),
+	SOC_SINGLE_RANGE_EXT_TLV("CH4 Gain", -1, MICFIL_OUTGAIN_CHX_SHIFT(4),
+				 0x0, 0xF, 0,
+				 get_channel_gain, put_channel_gain, gain_tlv),
+	SOC_SINGLE_RANGE_EXT_TLV("CH5 Gain", -1, MICFIL_OUTGAIN_CHX_SHIFT(5),
+				 0x0, 0xF, 0,
+				 get_channel_gain, put_channel_gain, gain_tlv),
+	SOC_SINGLE_RANGE_EXT_TLV("CH6 Gain", -1, MICFIL_OUTGAIN_CHX_SHIFT(6),
+				 0x0, 0xF, 0,
+				 get_channel_gain, put_channel_gain, gain_tlv),
+	SOC_SINGLE_RANGE_EXT_TLV("CH7 Gain", -1, MICFIL_OUTGAIN_CHX_SHIFT(7),
+				 0x0, 0xF, 0,
+				 get_channel_gain, put_channel_gain, gain_tlv),
+
+	SOC_ENUM_EXT("MICFIL Quality Select",
+		     fsl_micfil_quality_enum,
+		     snd_soc_get_enum_double, snd_soc_put_enum_double),
+	SOC_ENUM_EXT("HWVAD Initialization Mode",
+		     fsl_micfil_hwvad_init_mode_enum,
+		     hwvad_get_init_mode, hwvad_put_init_mode),
+	SOC_ENUM_EXT("HWVAD High-Pass Filter",
+		     fsl_micfil_hwvad_hpf_enum,
+		     hwvad_get_hpf, hwvad_put_hpf),
+	SOC_ENUM_EXT("HWVAD Zero-Crossing Detector Enable",
+		     fsl_micfil_hwvad_zcd_enum,
+		     hwvad_get_zcd_en, hwvad_put_zcd_en),
+	SOC_ENUM_EXT("HWVAD Zero-Crossing Detector Auto Threshold",
+		     fsl_micfil_hwvad_zcdauto_enum,
+		     hwvad_get_zcd_auto, hwvad_put_zcd_auto),
+	SOC_ENUM_EXT("HWVAD Noise OR Enable",
+		     fsl_micfil_hwvad_ndec_enum,
+		     snd_soc_get_enum_double, snd_soc_put_enum_double),
+	SOC_ENUM_EXT("HWVAD Sampling Rate",
+		     fsl_micfil_hwvad_rate_enum,
+		     hwvad_get_rate, hwvad_put_rate),
+	SOC_ENUM_EXT("Clock Source",
+		     fsl_micfil_clk_src_enum,
+		     micfil_get_clk_src, micfil_put_clk_src),
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = "HWVAD Input Gain",
+		.access = SNDRV_CTL_ELEM_ACCESS_READ |
+			  SNDRV_CTL_ELEM_ACCESS_WRITE,
+		.info = gain_info,
+		.get = hwvad_get_input_gain,
+		.put = hwvad_put_input_gain,
+	},
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = "HWVAD Sound Gain",
+		.access = SNDRV_CTL_ELEM_ACCESS_READ |
+			  SNDRV_CTL_ELEM_ACCESS_WRITE,
+		.info = gain_info,
+		.get = hwvad_get_sound_gain,
+		.put = hwvad_put_sound_gain,
+	},
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = "HWVAD Noise Gain",
+		.access = SNDRV_CTL_ELEM_ACCESS_READ |
+			  SNDRV_CTL_ELEM_ACCESS_WRITE,
+		.info = gain_info,
+		.get = hwvad_get_noise_gain,
+		.put = hwvad_put_noise_gain,
+	},
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = "HWVAD Detector Frame Time",
+		.access = SNDRV_CTL_ELEM_ACCESS_READ |
+			  SNDRV_CTL_ELEM_ACCESS_WRITE,
+		.info = hwvad_framet_info,
+		.get = hwvad_get_frame_time,
+		.put = hwvad_put_frame_time,
+	},
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = "HWVAD Detector Initialization Time",
+		.access = SNDRV_CTL_ELEM_ACCESS_READ |
+			  SNDRV_CTL_ELEM_ACCESS_WRITE,
+		.info = hwvad_initt_info,
+		.get = hwvad_get_init_time,
+		.put = hwvad_put_init_time,
+	},
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = "HWVAD Noise Filter Adjustment",
+		.access = SNDRV_CTL_ELEM_ACCESS_READ |
+			  SNDRV_CTL_ELEM_ACCESS_WRITE,
+		.info = hwvad_nfiladj_info,
+		.get = hwvad_get_nfil_adjust,
+		.put = hwvad_put_nfil_adjust,
+	},
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = "HWVAD Zero-Crossing Detector Threshold",
+		.access = SNDRV_CTL_ELEM_ACCESS_READ |
+			  SNDRV_CTL_ELEM_ACCESS_WRITE,
+		.info = hwvad_zcdth_info,
+		.get = hwvad_get_zcd_th,
+		.put = hwvad_put_zcd_th,
+	},
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = "HWVAD Zero-Crossing Detector Adjustment",
+		.access = SNDRV_CTL_ELEM_ACCESS_READ |
+			  SNDRV_CTL_ELEM_ACCESS_WRITE,
+		.info = hwvad_zcdadj_info,
+		.get = hwvad_get_zcd_adj,
+		.put = hwvad_put_zcd_adj,
+	},
+
+};
+
+static int disable_hwvad(struct device *dev, bool sync);
+
+static inline int get_pdm_clk(struct fsl_micfil *micfil,
+			      unsigned int rate);
+
+static inline int get_clk_div(struct fsl_micfil *micfil,
+			      unsigned int rate)
+{
+	u32 ctrl2_reg;
+	long mclk_rate;
+	int osr;
+	int clk_div;
+
+	regmap_read(micfil->regmap, REG_MICFIL_CTRL2, &ctrl2_reg);
+	osr = 16 - ((ctrl2_reg & MICFIL_CTRL2_CICOSR_MASK)
+		    >> MICFIL_CTRL2_CICOSR_SHIFT);
+
+	mclk_rate = clk_get_rate(micfil->mclk);
+
+	clk_div = mclk_rate / (get_pdm_clk(micfil, rate) * 2);
+
+	return clk_div;
+}
+
+static inline int get_pdm_clk(struct fsl_micfil *micfil,
+			      unsigned int rate)
+{
+	u32 ctrl2_reg;
+	int qsel, osr;
+	int bclk;
+
+	regmap_read(micfil->regmap, REG_MICFIL_CTRL2, &ctrl2_reg);
+	osr = 16 - ((ctrl2_reg & MICFIL_CTRL2_CICOSR_MASK)
+		    >> MICFIL_CTRL2_CICOSR_SHIFT);
+
+	regmap_read(micfil->regmap, REG_MICFIL_CTRL2, &ctrl2_reg);
+	qsel = ctrl2_reg & MICFIL_CTRL2_QSEL_MASK;
+
+	switch (qsel) {
+	case MICFIL_HIGH_QUALITY:
+		bclk = rate * 8 * osr / 2; /* kfactor = 0.5 */
+		break;
+	case MICFIL_MEDIUM_QUALITY:
+	case MICFIL_VLOW0_QUALITY:
+		bclk = rate * 4 * osr * 1; /* kfactor = 1 */
+		break;
+	case MICFIL_LOW_QUALITY:
+	case MICFIL_VLOW1_QUALITY:
+		bclk = rate * 2 * osr * 2; /* kfactor = 2 */
+		break;
+	case MICFIL_VLOW2_QUALITY:
+		bclk = rate * osr * 4; /* kfactor = 4 */
+		break;
+	default:
+		dev_err(&micfil->pdev->dev,
+			"Please make sure you select a valid quality.\n");
+		bclk = -1;
+		break;
+	}
+
+	return bclk;
+}
+
+/* The SRES is a self-negated bit which provides the CPU with the
+ * capability to initialize the PDM Interface module through the
+ * slave-bus interface. This bit always reads as zero, and this
+ * bit is only effective when MDIS is cleared
+ */
+static int fsl_micfil_reset(struct device *dev)
+{
+	struct fsl_micfil *micfil = dev_get_drvdata(dev);
+	int ret;
+
+	ret = regmap_update_bits(micfil->regmap,
+				 REG_MICFIL_CTRL1,
+				 MICFIL_CTRL1_MDIS_MASK,
+				 0);
+	if (ret) {
+		dev_err(dev, "failed to clear MDIS bit %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_update_bits(micfil->regmap,
+				 REG_MICFIL_CTRL1,
+				 MICFIL_CTRL1_SRES_MASK,
+				 MICFIL_CTRL1_SRES);
+	if (ret) {
+		dev_err(dev, "failed to reset MICFIL: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+/* enable/disable hwvad interrupts */
+static int configure_hwvad_interrupts(struct device *dev,
+				      int enable)
+{
+	struct fsl_micfil *micfil = dev_get_drvdata(dev);
+	int ret;
+	u32 vadie_reg = enable ? MICFIL_VAD0_CTRL1_IE : 0;
+	u32 vaderie_reg = enable ? MICFIL_VAD0_CTRL1_ERIE : 0;
+
+	/* Voice Activity Detector Error Interruption Enable */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_CTRL1,
+				 MICFIL_VAD0_CTRL1_ERIE_MASK,
+				 vaderie_reg);
+	if (ret) {
+		dev_err(dev,
+			"Failed to set/clear VADERIE in CTRL1_VAD0 [%d]\n",
+			ret);
+		return ret;
+	}
+
+	/* Voice Activity Detector Interruption Enable */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_CTRL1,
+				 MICFIL_VAD0_CTRL1_IE_MASK,
+				 vadie_reg);
+	if (ret) {
+		dev_err(dev,
+			"Failed to set/clear VADIE in CTRL1_VAD0 [%d]\n",
+			ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int init_hwvad_internal_filters(struct device *dev)
+{
+	struct fsl_micfil *micfil = dev_get_drvdata(dev);
+	int ret;
+
+	/* Voice Activity Detector Internal Filters Initialization*/
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_CTRL1,
+				 MICFIL_VAD0_CTRL1_ST10_MASK,
+				 MICFIL_VAD0_CTRL1_ST10);
+	if (ret) {
+		dev_err(dev,
+			"Failed to set VADST10 in CTRL1_VAD0 [%d]\n",
+			ret);
+		return ret;
+	}
+
+	/* sleep for 100ms - it should be enough for bit to stay
+	 * pulsed for more than 2 cycles
+	 */
+	usleep_range(MICFIL_SLEEP_MIN, MICFIL_SLEEP_MAX);
+
+	/* Voice Activity Detector Enabled */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_CTRL1,
+				 MICFIL_VAD0_CTRL1_ST10_MASK,
+				 0);
+	if (ret) {
+		dev_err(dev,
+			"Failed to clear VADST10 in CTRL1_VAD0 [%d]\n",
+			ret);
+		return ret;
+	}
+	return 0;
+}
+
+/* Zero-Crossing Detector Initialization
+ * Optionally a Zero-Crossing Detection block (ZCD) could
+ * be enabled to avoid low energy voiced speech be missed,
+ * improving the voice detection performance.
+ * See Section 8.4.3
+ */
+static int __maybe_unused init_zcd(struct device *dev)
+{
+	struct fsl_micfil *micfil = dev_get_drvdata(dev);
+	int ret;
+
+	/* exit if zcd is not enabled from userspace */
+	if (!micfil->vad_zcd_en)
+		return 0;
+
+	if (micfil->vad_zcd_auto) {
+		/* Zero-Crossing Detector Adjustment */
+		ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_ZCD,
+					 MICFIL_VAD0_ZCD_ZCDADJ_MASK,
+					 micfil->vad_zcd_adj);
+		if (ret) {
+			dev_err(dev,
+				"Failed to set ZCDADJ in ZCD_VAD0 [%d]\n",
+				ret);
+			return ret;
+		}
+	}
+
+	/* Zero-Crossing Detector Threshold */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_ZCD,
+				 MICFIL_VAD0_ZCD_ZCDTH_MASK,
+				 MICFIL_VAD0_ZCD_ZCDTH(micfil->vad_zcd_th));
+	if (ret) {
+		dev_err(dev, "Failed to set ZCDTH in ZCD_VAD0 [%d]\n", ret);
+		return ret;
+	}
+
+	/* Zero-Crossing Detector AND Behavior */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_ZCD,
+				 MICFIL_VAD0_ZCD_ZCDAND_MASK,
+				 MICFIL_HWVAD_ZCDAND);
+	if (ret) {
+		dev_err(dev, "Failed to set ZCDAND in ZCD_VAD0 [%d]\n", ret);
+		return ret;
+	}
+
+	/* Zero-Crossing Detector Automatic Threshold */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_ZCD,
+				 MICFIL_VAD0_ZCD_ZCDAUT_MASK,
+				 micfil->vad_zcd_auto);
+	if (ret) {
+		dev_err(dev,
+			"Failed to set/clear ZCDAUT in ZCD_VAD0 [%d]\n",
+			ret);
+		return ret;
+	}
+
+	/* Zero-Crossing Detector Enable */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_ZCD,
+				 MICFIL_VAD0_ZCD_ZCDEN_MASK,
+				 MICFIL_VAD0_ZCD_ZCDEN);
+	if (ret) {
+		dev_err(dev, "Failed to set ZCDEN in ZCD_VAD0 [%d]\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+/* Configuration done only in energy-based initialization mode */
+static int init_hwvad_energy_mode(struct device *dev)
+{
+	struct fsl_micfil *micfil = dev_get_drvdata(dev);
+	int ret, i;
+	u32 stat;
+	u32 flag;
+
+	dev_info(dev, "Energy-based mode initialization\n");
+
+	/* Voice Activity Detector Reset */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_CTRL1,
+				 MICFIL_VAD0_CTRL1_RST_SHIFT,
+				 MICFIL_VAD0_CTRL1_RST);
+	if (ret) {
+		dev_err(dev, "Failed to set VADRST in CTRL1_VAD0 [%d]\n", ret);
+		return ret;
+	}
+
+	/* Voice Activity Detector Enabled */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_CTRL1,
+				 MICFIL_VAD0_CTRL1_EN_MASK,
+				 MICFIL_VAD0_CTRL1_EN);
+	if (ret) {
+		dev_err(dev, "Failed to set VADEN in CTRL1_VAD0 [%d]\n", ret);
+		return ret;
+	}
+
+	/* it would be a good idea to wait some time before VADEN
+	 * is set
+	 */
+	usleep_range(5 * MICFIL_SLEEP_MIN, 5 * MICFIL_SLEEP_MAX);
+
+	/* Enable Interrupts */
+	ret = configure_hwvad_interrupts(dev, 1);
+
+	/* Initialize Zero Crossing Detector */
+	ret = init_zcd(dev);
+	if (ret)
+		return ret;
+
+	/* Enable MICFIL module */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_CTRL1,
+				 MICFIL_CTRL1_PDMIEN_MASK,
+				 MICFIL_CTRL1_PDMIEN);
+	if (ret) {
+		dev_err(dev, "failed to enable the module\n");
+		return ret;
+	}
+
+	/* Wait for INITF to be asserted */
+	for (i = 0; i < MICFIL_MAX_RETRY; i++) {
+		ret = regmap_read(micfil->regmap, REG_MICFIL_VAD0_STAT, &stat);
+		if (ret) {
+			dev_err(dev, "failed to read register %d\n",
+				REG_MICFIL_VAD0_STAT);
+			return ret;
+		}
+
+		flag = (stat & MICFIL_VAD0_STAT_INITF_MASK);
+		if (flag == 0)
+			break;
+
+		usleep_range(MICFIL_SLEEP_MIN, MICFIL_SLEEP_MAX);
+	}
+
+	if (i == MICFIL_MAX_RETRY) {
+		dev_err(dev, "initf not asserted. Failed to init hwvad\n");
+		return -EBUSY;
+	}
+
+	/* Initialize Internal Filters */
+	ret = init_hwvad_internal_filters(dev);
+	if (ret)
+		return ret;
+
+	return ret;
+}
+
+/* Configuration done only in envelope-based initialization mode */
+static int init_hwvad_envelope_mode(struct device *dev)
+{
+	struct fsl_micfil *micfil = dev_get_drvdata(dev);
+	int ret, i;
+	u32 stat;
+	u32 flag;
+
+	/* Frame energy disable */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_CTRL2,
+				 MICFIL_VAD0_CTRL2_FRENDIS_MASK,
+				 MICFIL_VAD0_CTRL2_FRENDIS);
+	if (ret) {
+		dev_err(dev, "Failed to set FRENDIS in CTRL2_VAD0 [%d]\n", ret);
+		return ret;
+	}
+
+	/* Enable pre-filter Noise & Signal */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_CTRL2,
+				 MICFIL_VAD0_CTRL2_PREFEN_MASK,
+				 MICFIL_VAD0_CTRL2_PREFEN);
+	if (ret) {
+		dev_err(dev, "Failed to set PREFEN in CTRL2_VAD0 [%d]\n", ret);
+		return ret;
+	}
+
+	/* Enable Signal Filter */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_SCONFIG,
+				 MICFIL_VAD0_SCONFIG_SFILEN_MASK,
+				 MICFIL_VAD0_SCONFIG_SFILEN);
+	if (ret) {
+		dev_err(dev,
+			"Failed to set SFILEN in SCONFIG_VAD0 [%d]\n",
+			ret);
+		return ret;
+	}
+
+	/* Signal Maximum Enable */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_SCONFIG,
+				 MICFIL_VAD0_SCONFIG_SMAXEN_MASK,
+				 MICFIL_VAD0_SCONFIG_SMAXEN);
+	if (ret) {
+		dev_err(dev,
+			"Failed to set SMAXEN in SCONFIG_VAD0 [%d]\n",
+			ret);
+		return ret;
+	}
+
+	/* Allways enable noise filter, not based on voice activity
+	 * information
+	 */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_NCONFIG,
+				 MICFIL_VAD0_NCONFIG_NFILAUT_MASK,
+				 0);
+	if (ret) {
+		dev_err(dev,
+			"Failed to set NFILAUT in NCONFIG_VAD0 [%d]\n",
+			ret);
+		return ret;
+	}
+
+	/* Noise Minimum Enable */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_NCONFIG,
+				 MICFIL_VAD0_NCONFIG_NMINEN_MASK,
+				 MICFIL_VAD0_NCONFIG_NMINEN);
+	if (ret) {
+		dev_err(dev,
+			"Failed to set NMINEN in NCONFIG_VAD0 [%d]\n",
+			ret);
+		return ret;
+	}
+
+	/* Noise Decimation Enable */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_NCONFIG,
+				 MICFIL_VAD0_NCONFIG_NDECEN_MASK,
+				 MICFIL_VAD0_NCONFIG_NDECEN);
+	if (ret) {
+		dev_err(dev,
+			"Failed to set NDECEN in NCONFIG_VAD0 [%d]\n",
+			ret);
+		return ret;
+	}
+
+	/* Voice Activity Detector Reset */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_CTRL1,
+				 MICFIL_VAD0_CTRL1_RST_MASK,
+				 MICFIL_VAD0_CTRL1_RST);
+	if (ret) {
+		dev_err(dev, "Failed to set VADRST in CTRL1_VAD0 [%d]\n", ret);
+		return ret;
+	}
+
+	/* Initialize Zero Crossing Detector */
+	ret = init_zcd(dev);
+	if (ret)
+		return ret;
+
+	/* Voice Activity Detector Enabled */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_CTRL1,
+				 MICFIL_VAD0_CTRL1_EN_MASK,
+				 MICFIL_VAD0_CTRL1_EN);
+	if (ret) {
+		dev_err(dev, "Failed to set VADEN in CTRL1_VAD0 [%d]\n", ret);
+		return ret;
+	}
+
+	/* Enable MICFIL module */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_CTRL1,
+				 MICFIL_CTRL1_PDMIEN_MASK,
+				 MICFIL_CTRL1_PDMIEN);
+	if (ret) {
+		dev_err(dev, "failed to enable the module\n");
+		return ret;
+	}
+
+	/* it would be a good idea to wait some time before VADEN
+	 * is set
+	 */
+	usleep_range(3 * MICFIL_SLEEP_MIN, 3 * MICFIL_SLEEP_MAX);
+
+	/* Wait for INITF to be asserted */
+	for (i = 0; i < MICFIL_MAX_RETRY; i++) {
+		ret = regmap_read(micfil->regmap, REG_MICFIL_VAD0_STAT, &stat);
+		if (ret) {
+			dev_err(dev, "failed to read register %d\n",
+				REG_MICFIL_VAD0_STAT);
+			return ret;
+		}
+
+		flag = (stat & MICFIL_VAD0_STAT_INITF_MASK);
+		if (flag == 0)
+			break;
+
+		usleep_range(MICFIL_SLEEP_MIN, MICFIL_SLEEP_MAX);
+	}
+
+	if (i == MICFIL_MAX_RETRY) {
+		dev_err(dev, "initf not asserted. Failed to init hwvad\n");
+		return -EBUSY;
+	}
+
+	/* Initialize Internal Filters */
+	ret = init_hwvad_internal_filters(dev);
+	if (ret)
+		return ret;
+
+	/* Enable interrupts */
+	ret = configure_hwvad_interrupts(dev, 1);
+	if (ret)
+		return ret;
+
+	return ret;
+}
+
+/* Hardware Voice Active Detection: The HWVAD takes data from the input
+ * of a selected PDM microphone to detect if there is any
+ * voice activity. When a voice activity is detected, an interrupt could
+ * be delivered to the system. Initialization in section 8.4:
+ * Can work in two modes:
+ *  -> Eneveope-based mode (section 8.4.1)
+ *  -> Energy-based mode (section 8.4.2)
+ *
+ * It is important to remark that the HWVAD detector could be enabled
+ * or reset only when the MICFIL isn't running i.e. when the BSY_FIL
+ * bit in STAT register is cleared
+ */
+static int __maybe_unused init_hwvad(struct device *dev)
+{
+	struct fsl_micfil *micfil = dev_get_drvdata(dev);
+	int ret;
+	u32 reg_val;
+
+	/* configure CIC OSR in VADCICOSR */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_CTRL1,
+				 MICFIL_VAD0_CTRL1_CICOSR_MASK,
+				 MICFIL_CTRL2_OSR_DEFAULT);
+	if (ret) {
+		dev_err(dev, "Failed to set CICOSR in CTRL1_VAD0i [%d]\n", ret);
+		return ret;
+	}
+
+	/* configure source channel in VADCHSEL */
+	reg_val = MICFIL_VAD0_CTRL1_CHSEL(micfil->vad_channel);
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_CTRL1,
+				 MICFIL_VAD0_CTRL1_CHSEL_MASK,
+				 reg_val);
+	if (ret) {
+		dev_err(dev, "Failed to set CHSEL in CTRL1_VAD0 [%d]\n", ret);
+		return ret;
+	}
+
+	/* configure detector frame time VADFRAMET */
+	reg_val = MICFIL_VAD0_CTRL2_FRAMET(micfil->vad_frame_time);
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_CTRL2,
+				 MICFIL_VAD0_CTRL2_FRAMET_MASK,
+				 reg_val);
+	if (ret) {
+		dev_err(dev, "Failed to set FRAMET in CTRL2_VAD0 [%d]\n", ret);
+		return ret;
+	}
+
+	/* configure initialization time in VADINITT */
+	reg_val = MICFIL_VAD0_CTRL1_INITT(micfil->vad_init_time);
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_CTRL1,
+				 MICFIL_VAD0_CTRL1_INITT_MASK,
+				 reg_val);
+	if (ret) {
+		dev_err(dev, "Failed to set INITT in CTRL1_VAD0 [%d]\n", ret);
+		return ret;
+	}
+
+	/* configure input gain in VADINPGAIN */
+	reg_val = MICFIL_VAD0_CTRL2_INPGAIN(micfil->vad_input_gain);
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_CTRL2,
+				 MICFIL_VAD0_CTRL2_INPGAIN_MASK,
+				 reg_val);
+	if (ret) {
+		dev_err(dev, "Failed to set INPGAIN in CTRL2_VAD0 [%d]\n", ret);
+		return ret;
+	}
+
+	/* configure sound gain in SGAIN */
+	reg_val = MICFIL_VAD0_SCONFIG_SGAIN(micfil->vad_sound_gain);
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_SCONFIG,
+				 MICFIL_VAD0_SCONFIG_SGAIN_MASK,
+				 reg_val);
+	if (ret) {
+		dev_err(dev, "Failed to set SGAIN in SCONFIG_VAD0 [%d]\n", ret);
+		return ret;
+	}
+
+	/* configure noise gain in NGAIN */
+	reg_val = MICFIL_VAD0_NCONFIG_NGAIN(micfil->vad_noise_gain);
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_NCONFIG,
+				 MICFIL_VAD0_NCONFIG_NGAIN_MASK,
+				 reg_val);
+	if (ret) {
+		dev_err(dev, "Failed to set NGAIN in NCONFIG_VAD0 [%d]\n", ret);
+		return ret;
+	}
+
+	/* configure or clear the VADNFILADJ based on mode */
+	reg_val = MICFIL_VAD0_NCONFIG_NFILADJ(micfil->vad_nfil_adjust);
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_NCONFIG,
+				 MICFIL_VAD0_NCONFIG_NFILADJ_MASK,
+				 reg_val);
+	if (ret) {
+		dev_err(dev,
+			"Failed to set VADNFILADJ in NCONFIG_VAD0 [%d]\n",
+			ret);
+		return ret;
+	}
+
+	/* enable the high-pass filter in VADHPF */
+	reg_val = MICFIL_VAD0_CTRL2_HPF(micfil->vad_hpf);
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_VAD0_CTRL2,
+				 MICFIL_VAD0_CTRL2_HPF_MASK,
+				 reg_val);
+	if (ret) {
+		dev_err(dev, "Failed to set HPF in CTRL2_VAD0 [%d]\n", ret);
+		return ret;
+	}
+
+	/* envelope-based specific initialization */
+	if (micfil->vad_init_mode == MICFIL_HWVAD_ENVELOPE_MODE) {
+		ret = init_hwvad_envelope_mode(dev);
+		if (ret)
+			return ret;
+	} else {
+		ret = init_hwvad_energy_mode(dev);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static inline bool clk_in_list(struct clk *p, struct clk *clk_src[])
+{
+	int i;
+
+	for (i = 0; i < MICFIL_CLK_SRC_NUM; i++)
+		if (clk_is_match(p, clk_src[i]))
+			return true;
+
+	return false;
+}
+
+static int fsl_micfil_set_mclk_rate(struct fsl_micfil *micfil, int clk_id,
+				    unsigned int freq)
+{
+	struct clk *p = micfil->mclk, *pll = 0, *npll = 0;
+	struct device *dev = &micfil->pdev->dev;
+	u64 ratio = freq;
+	u64 clk_rate;
+	int ret;
+	int i;
+
+	/* Do not touch the clock if hwvad is already enabled
+	 * since you can record only at hwvad rate and clock
+	 * has already been set to the required frequency
+	 */
+	if (atomic_read(&micfil->hwvad_state) == MICFIL_HWVAD_ON)
+		return 0;
+
+	/* check if all clock sources are valid */
+	for (i = 0; i < MICFIL_CLK_SRC_NUM; i++) {
+		if (micfil->clk_src[i])
+			continue;
+
+		dev_err(dev, "Clock Source %d is not valid.\n", i);
+		return -EINVAL;
+	}
+
+	while (p) {
+		struct clk *pp = clk_get_parent(p);
+
+		if (clk_in_list(pp, micfil->clk_src)) {
+			pll = pp;
+			break;
+		}
+		p = pp;
+	}
+
+	if (!pll) {
+		dev_err(dev, "reached a null clock\n");
+		return -EINVAL;
+	}
+
+	if (micfil->clk_src_id == MICFIL_CLK_AUTO) {
+		for (i = 0; i < MICFIL_CLK_SRC_NUM; i++) {
+			clk_rate = clk_get_rate(micfil->clk_src[i]);
+			/* This is an workaround since audio_pll2 clock
+			 * has 722534399 rate and this will never divide
+			 * to any known frequency ???
+			 */
+			clk_rate = round_up(clk_rate, 10);
+			if (do_div(clk_rate, ratio) == 0)
+				npll = micfil->clk_src[i];
+		}
+	} else {
+		/* clock id is offseted by 1 since ID=0 means
+		 * auto clock selection
+		 */
+		npll = micfil->clk_src[micfil->clk_src_id - 1];
+	}
+
+	if (!npll) {
+		dev_err(dev,
+			"failed to find a suitable clock source\n");
+		return -EINVAL;
+	}
+
+	if (!clk_is_match(pll, npll)) {
+		ret = clk_set_parent(p, npll);
+		if (ret < 0)
+			dev_warn(dev,
+				 "failed to set parrent %d\n", ret);
+	}
+
+	clk_disable_unprepare(micfil->mclk);
+	ret = clk_set_rate(micfil->mclk, freq * 1024);
+	if (ret)
+		dev_warn(dev, "failed to set rate (%u): %d\n",
+			 freq * 1024, ret);
+	clk_prepare_enable(micfil->mclk);
+
+	return ret;
+}
+
+static int fsl_micfil_startup(struct snd_pcm_substream *substream,
+			      struct snd_soc_dai *dai)
+{
+	struct fsl_micfil *micfil = snd_soc_dai_get_drvdata(dai);
+
+	if (!micfil) {
+		dev_err(dai->dev,
+			"micfil dai priv_data not set\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int fsl_micfil_trigger(struct snd_pcm_substream *substream, int cmd,
+			      struct snd_soc_dai *dai)
+{
+	struct fsl_micfil *micfil = snd_soc_dai_get_drvdata(dai);
+	struct device *dev = &micfil->pdev->dev;
+	int ret;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		ret = fsl_micfil_reset(dev);
+		if (ret) {
+			dev_err(dev, "failed to soft reset\n");
+			return ret;
+		}
+
+		/* DMA Interrupt Selection - DISEL bits
+		 * 00 - DMA and IRQ disabled
+		 * 01 - DMA req enabled
+		 * 10 - IRQ enabled
+		 * 11 - reserved
+		 */
+		ret = regmap_update_bits(micfil->regmap, REG_MICFIL_CTRL1,
+					 MICFIL_CTRL1_DISEL_MASK,
+					 (1 << MICFIL_CTRL1_DISEL_SHIFT));
+		if (ret) {
+			dev_err(dev, "failed to update DISEL bits\n");
+			return ret;
+		}
+
+		/* Enable the module */
+		ret = regmap_update_bits(micfil->regmap, REG_MICFIL_CTRL1,
+					 MICFIL_CTRL1_PDMIEN_MASK,
+					 MICFIL_CTRL1_PDMIEN);
+		if (ret) {
+			dev_err(dev, "failed to enable the module\n");
+			return ret;
+		}
+
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		/* Disable the module */
+		ret = regmap_update_bits(micfil->regmap, REG_MICFIL_CTRL1,
+					 MICFIL_CTRL1_PDMIEN_MASK,
+					 0);
+		if (ret) {
+			dev_err(dev, "failed to enable the module\n");
+			return ret;
+		}
+
+		ret = regmap_update_bits(micfil->regmap, REG_MICFIL_CTRL1,
+					 MICFIL_CTRL1_DISEL_MASK,
+					 (0 << MICFIL_CTRL1_DISEL_SHIFT));
+		if (ret) {
+			dev_err(dev, "failed to update DISEL bits\n");
+			return ret;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int fsl_set_clock_params(struct device *dev, unsigned int rate)
+{
+	struct fsl_micfil *micfil = dev_get_drvdata(dev);
+	int clk_div;
+	int ret = 0;
+
+	ret = fsl_micfil_set_mclk_rate(micfil, 0, rate);
+	if (ret < 0)
+		dev_err(dev, "failed to set mclk[%lu] to rate %u\n",
+			clk_get_rate(micfil->mclk), rate);
+
+	/* set CICOSR */
+	ret |= regmap_update_bits(micfil->regmap, REG_MICFIL_CTRL2,
+				 MICFIL_CTRL2_CICOSR_MASK,
+				 MICFIL_CTRL2_OSR_DEFAULT);
+	if (ret)
+		dev_err(dev, "failed to set CICOSR in reg 0x%X\n",
+			REG_MICFIL_CTRL2);
+
+	/* set CLK_DIV */
+	clk_div = get_clk_div(micfil, rate);
+	if (clk_div < 0)
+		ret = -EINVAL;
+
+	ret |= regmap_update_bits(micfil->regmap, REG_MICFIL_CTRL2,
+				 MICFIL_CTRL2_CLKDIV_MASK, clk_div);
+	if (ret)
+		dev_err(dev, "failed to set CLKDIV in reg 0x%X\n",
+			REG_MICFIL_CTRL2);
+
+	return ret;
+}
+
+static int fsl_micfil_hw_params(struct snd_pcm_substream *substream,
+				struct snd_pcm_hw_params *params,
+				struct snd_soc_dai *dai)
+{
+	struct fsl_micfil *micfil = snd_soc_dai_get_drvdata(dai);
+	unsigned int channels = params_channels(params);
+	unsigned int rate = params_rate(params);
+	struct device *dev = &micfil->pdev->dev;
+	unsigned int hwvad_rate;
+	int ret;
+	u32 hwvad_state;
+
+	hwvad_rate = micfil_hwvad_rate_ints[micfil->vad_rate_index];
+	hwvad_state = atomic_read(&micfil->hwvad_state);
+
+	/* if hwvad is enabled, make sure you are recording at
+	 * the same rate the hwvad is on or reject it to avoid
+	 * changing the clock rate.
+	 */
+	if (hwvad_state == MICFIL_HWVAD_ON && rate != hwvad_rate) {
+		dev_err(dev, "Record at hwvad rate %u\n", hwvad_rate);
+		return -EINVAL;
+	}
+
+	atomic_set(&micfil->recording_state, MICFIL_RECORDING_ON);
+
+	if (hwvad_state == MICFIL_HWVAD_OFF) {
+		/* 1. Disable the module */
+		ret = regmap_update_bits(micfil->regmap, REG_MICFIL_CTRL1,
+					 MICFIL_CTRL1_PDMIEN_MASK, 0);
+		if (ret) {
+			dev_err(dev, "failed to disable the module\n");
+			return ret;
+		}
+	}
+
+	/* enable channels */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_CTRL1,
+				 0xFF, ((1 << channels) - 1));
+	if (ret) {
+		dev_err(dev, "failed to enable channels %d, reg 0x%X\n", ret,
+			REG_MICFIL_CTRL1);
+		return ret;
+	}
+
+	ret = fsl_set_clock_params(dev, rate);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set clock parameters [%d]\n", ret);
+		return ret;
+	}
+
+	micfil->dma_params_rx.maxburst = channels * MICFIL_DMA_MAXBURST_RX;
+
+	return 0;
+}
+
+static int fsl_micfil_hw_free(struct snd_pcm_substream *substream,
+			      struct snd_soc_dai *dai)
+{
+	struct fsl_micfil *micfil = snd_soc_dai_get_drvdata(dai);
+
+	atomic_set(&micfil->recording_state, MICFIL_RECORDING_OFF);
+
+	return 0;
+}
+
+static int fsl_micfil_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
+				     unsigned int freq, int dir)
+{
+	struct fsl_micfil *micfil = snd_soc_dai_get_drvdata(dai);
+	struct device *dev = &micfil->pdev->dev;
+
+	int ret;
+
+	if (!freq)
+		return 0;
+
+	ret = fsl_micfil_set_mclk_rate(micfil, clk_id, freq);
+	if (ret < 0)
+		dev_err(dev, "failed to set mclk[%lu] to rate %u\n",
+			clk_get_rate(micfil->mclk), freq);
+
+	return ret;
+}
+
+static int fsl_micfil_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt)
+{
+	struct fsl_micfil *micfil = snd_soc_dai_get_drvdata(dai);
+
+	/* DAI MODE */
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* DAI CLK INVERSION */
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_NB_NF:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	micfil->slave_mode = false;
+
+	return 0;
+}
+
+static struct snd_soc_dai_ops fsl_micfil_dai_ops = {
+	.startup = fsl_micfil_startup,
+	.trigger = fsl_micfil_trigger,
+	.hw_params = fsl_micfil_hw_params,
+	.hw_free = fsl_micfil_hw_free,
+	.set_sysclk = fsl_micfil_set_dai_sysclk,
+	.set_fmt = fsl_micfil_set_dai_fmt,
+};
+
+static int fsl_micfil_dai_probe(struct snd_soc_dai *cpu_dai)
+{
+	struct fsl_micfil *micfil = dev_get_drvdata(cpu_dai->dev);
+	struct device *dev = cpu_dai->dev;
+	unsigned int val;
+	int ret;
+	int i;
+
+	/* set qsel to medium */
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_CTRL2,
+				 MICFIL_CTRL2_QSEL_MASK, MICFIL_MEDIUM_QUALITY);
+	if (ret) {
+		dev_err(dev, "failed to set quality mode bits, reg 0x%X\n",
+			REG_MICFIL_CTRL2);
+		return ret;
+	}
+
+	/* set default gain to max_gain */
+	regmap_write(micfil->regmap, REG_MICFIL_OUT_CTRL, 0x77777777);
+	for (i = 0; i < 8; i++)
+		micfil->channel_gain[i] = 0xF;
+
+	snd_soc_dai_init_dma_data(cpu_dai, NULL,
+				  &micfil->dma_params_rx);
+
+	/* FIFO Watermark Control - FIFOWMK*/
+	val = MICFIL_FIFO_CTRL_FIFOWMK(micfil->soc->fifo_depth) - 1;
+	ret = regmap_update_bits(micfil->regmap, REG_MICFIL_FIFO_CTRL,
+				 MICFIL_FIFO_CTRL_FIFOWMK_MASK,
+				 val);
+	if (ret) {
+		dev_err(dev, "failed to set FIFOWMK\n");
+		return ret;
+	}
+
+	snd_soc_dai_set_drvdata(cpu_dai, micfil);
+
+	return 0;
+}
+
+static struct snd_soc_dai_driver fsl_micfil_dai = {
+	.probe = fsl_micfil_dai_probe,
+	.capture = {
+		.stream_name = "CPU-Capture",
+		.channels_min = 1,
+		.channels_max = 8,
+		.rates = FSL_MICFIL_RATES,
+		.formats = FSL_MICFIL_FORMATS,
+	},
+	.ops = &fsl_micfil_dai_ops,
+};
+
+static const struct snd_soc_component_driver fsl_micfil_component = {
+	.name		= "fsl-micfil-dai",
+	.controls       = fsl_micfil_snd_controls,
+	.num_controls   = ARRAY_SIZE(fsl_micfil_snd_controls),
+
+};
+
+/* REGMAP */
+static const struct reg_default fsl_micfil_reg_defaults[] = {
+	{REG_MICFIL_CTRL1,		0x00000000},
+	{REG_MICFIL_CTRL2,		0x00000000},
+	{REG_MICFIL_STAT,		0x00000000},
+	{REG_MICFIL_FIFO_CTRL,		0x00000007},
+	{REG_MICFIL_FIFO_STAT,		0x00000000},
+	{REG_MICFIL_DATACH0,		0x00000000},
+	{REG_MICFIL_DATACH1,		0x00000000},
+	{REG_MICFIL_DATACH2,		0x00000000},
+	{REG_MICFIL_DATACH3,		0x00000000},
+	{REG_MICFIL_DATACH4,		0x00000000},
+	{REG_MICFIL_DATACH5,		0x00000000},
+	{REG_MICFIL_DATACH6,		0x00000000},
+	{REG_MICFIL_DATACH7,		0x00000000},
+	{REG_MICFIL_DC_CTRL,		0x00000000},
+	{REG_MICFIL_OUT_CTRL,		0x00000000},
+	{REG_MICFIL_OUT_STAT,		0x00000000},
+	{REG_MICFIL_VAD0_CTRL1,		0x00000000},
+	{REG_MICFIL_VAD0_CTRL2,		0x000A0000},
+	{REG_MICFIL_VAD0_STAT,		0x00000000},
+	{REG_MICFIL_VAD0_SCONFIG,	0x00000000},
+	{REG_MICFIL_VAD0_NCONFIG,	0x80000000},
+	{REG_MICFIL_VAD0_NDATA,		0x00000000},
+	{REG_MICFIL_VAD0_ZCD,		0x00000004},
+};
+
+static bool fsl_micfil_readable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case REG_MICFIL_CTRL1:
+	case REG_MICFIL_CTRL2:
+	case REG_MICFIL_STAT:
+	case REG_MICFIL_FIFO_CTRL:
+	case REG_MICFIL_FIFO_STAT:
+	case REG_MICFIL_DATACH0:
+	case REG_MICFIL_DATACH1:
+	case REG_MICFIL_DATACH2:
+	case REG_MICFIL_DATACH3:
+	case REG_MICFIL_DATACH4:
+	case REG_MICFIL_DATACH5:
+	case REG_MICFIL_DATACH6:
+	case REG_MICFIL_DATACH7:
+	case REG_MICFIL_DC_CTRL:
+	case REG_MICFIL_OUT_CTRL:
+	case REG_MICFIL_OUT_STAT:
+	case REG_MICFIL_VAD0_CTRL1:
+	case REG_MICFIL_VAD0_CTRL2:
+	case REG_MICFIL_VAD0_STAT:
+	case REG_MICFIL_VAD0_SCONFIG:
+	case REG_MICFIL_VAD0_NCONFIG:
+	case REG_MICFIL_VAD0_NDATA:
+	case REG_MICFIL_VAD0_ZCD:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool fsl_micfil_writeable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case REG_MICFIL_CTRL1:
+	case REG_MICFIL_CTRL2:
+	case REG_MICFIL_STAT:		/* Write 1 to Clear */
+	case REG_MICFIL_FIFO_CTRL:
+	case REG_MICFIL_FIFO_STAT:	/* Write 1 to Clear */
+	case REG_MICFIL_DC_CTRL:
+	case REG_MICFIL_OUT_CTRL:
+	case REG_MICFIL_OUT_STAT:	/* Write 1 to Clear */
+	case REG_MICFIL_VAD0_CTRL1:
+	case REG_MICFIL_VAD0_CTRL2:
+	case REG_MICFIL_VAD0_STAT:	/* Write 1 to Clear */
+	case REG_MICFIL_VAD0_SCONFIG:
+	case REG_MICFIL_VAD0_NCONFIG:
+	case REG_MICFIL_VAD0_ZCD:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool fsl_micfil_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case REG_MICFIL_CTRL1:
+	case REG_MICFIL_CTRL2:
+	case REG_MICFIL_STAT:
+	case REG_MICFIL_DATACH0:
+	case REG_MICFIL_DATACH1:
+	case REG_MICFIL_DATACH2:
+	case REG_MICFIL_DATACH3:
+	case REG_MICFIL_DATACH4:
+	case REG_MICFIL_DATACH5:
+	case REG_MICFIL_DATACH6:
+	case REG_MICFIL_DATACH7:
+	case REG_MICFIL_VAD0_CTRL1:
+	case REG_MICFIL_VAD0_CTRL2:
+	case REG_MICFIL_VAD0_STAT:
+	case REG_MICFIL_VAD0_SCONFIG:
+	case REG_MICFIL_VAD0_NCONFIG:
+	case REG_MICFIL_VAD0_NDATA:
+	case REG_MICFIL_VAD0_ZCD:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config fsl_micfil_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+
+	.max_register = REG_MICFIL_VAD0_ZCD,
+	.reg_defaults = fsl_micfil_reg_defaults,
+	.num_reg_defaults = ARRAY_SIZE(fsl_micfil_reg_defaults),
+	.readable_reg = fsl_micfil_readable_reg,
+	.volatile_reg = fsl_micfil_volatile_reg,
+	.writeable_reg = fsl_micfil_writeable_reg,
+	.cache_type = REGCACHE_RBTREE,
+};
+
+/* END OF REGMAP */
+
+static irqreturn_t voice_detected_fn(int irq, void *devid)
+{
+	struct fsl_micfil *micfil = (struct fsl_micfil *)devid;
+	struct device *dev = &micfil->pdev->dev;
+	int ret;
+
+	/* disable hwvad */
+	ret = disable_hwvad(dev, true);
+	if (ret)
+		dev_err(dev, "Failed to disable HWVAD module\n");
+
+	/* notify userspace that voice was detected */
+	kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t hwvad_isr(int irq, void *devid)
+{
+	struct fsl_micfil *micfil = (struct fsl_micfil *)devid;
+	struct device *dev = &micfil->pdev->dev;
+	int ret;
+	u32 vad0_reg;
+
+	regmap_read(micfil->regmap, REG_MICFIL_VAD0_STAT, &vad0_reg);
+
+	/* The only difference between MICFIL_VAD0_STAT_EF and
+	 * MICFIL_VAD0_STAT_IF is that the former requires Write
+	 * 1 to Clear. Since both flags are set, it is enough
+	 * to only read one of them
+	 */
+	if (vad0_reg & MICFIL_VAD0_STAT_IF_MASK) {
+		/* Write 1 to clear */
+		regmap_write_bits(micfil->regmap, REG_MICFIL_VAD0_STAT,
+				  MICFIL_VAD0_STAT_IF_MASK,
+				  MICFIL_VAD0_STAT_IF);
+
+		/* disable hwvad interrupts */
+		ret = configure_hwvad_interrupts(dev, 0);
+		if (ret)
+			dev_err(dev, "Failed to disable interrupts\n");
+	}
+
+	return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t hwvad_err_isr(int irq, void *devid)
+{
+	struct fsl_micfil *micfil = (struct fsl_micfil *)devid;
+	struct device *dev = &micfil->pdev->dev;
+	u32 vad0_reg;
+
+	regmap_read(micfil->regmap, REG_MICFIL_VAD0_STAT, &vad0_reg);
+
+	if (vad0_reg & MICFIL_VAD0_STAT_INSATF_MASK)
+		dev_dbg(dev, "voice activity input overflow/underflow detected\n");
+
+	if (vad0_reg & MICFIL_VAD0_STAT_INITF_MASK)
+		dev_dbg(dev, "voice activity dectector is initializing\n");
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t micfil_isr(int irq, void *devid)
+{
+	struct fsl_micfil *micfil = (struct fsl_micfil *)devid;
+	struct platform_device *pdev = micfil->pdev;
+	u32 stat_reg;
+	u32 fifo_stat_reg;
+	u32 ctrl1_reg;
+	bool dma_enabled;
+	int i;
+
+	regmap_read(micfil->regmap, REG_MICFIL_STAT, &stat_reg);
+	regmap_read(micfil->regmap, REG_MICFIL_CTRL1, &ctrl1_reg);
+	regmap_read(micfil->regmap, REG_MICFIL_FIFO_STAT, &fifo_stat_reg);
+
+	dma_enabled = MICFIL_DMA_ENABLED(ctrl1_reg);
+
+	/* Channel 0-7 Output Data Flags */
+	for (i = 0; i < MICFIL_OUTPUT_CHANNELS; i++) {
+		if (stat_reg & MICFIL_STAT_CHXF_MASK(i))
+			dev_dbg(&pdev->dev,
+				"Data available in Data Channel %d\n", i);
+		/* if DMA is not enabled, field must be written with 1
+		 * to clear
+		 */
+		if (!dma_enabled)
+			regmap_write_bits(micfil->regmap,
+					  REG_MICFIL_STAT,
+					  MICFIL_STAT_CHXF_MASK(i),
+					  1);
+	}
+
+	for (i = 0; i < MICFIL_FIFO_NUM; i++) {
+		if (fifo_stat_reg & MICFIL_FIFO_STAT_FIFOX_OVER_MASK(i))
+			dev_dbg(&pdev->dev,
+				"FIFO Overflow Exception flag for channel %d\n",
+				i);
+
+		if (fifo_stat_reg & MICFIL_FIFO_STAT_FIFOX_UNDER_MASK(i))
+			dev_dbg(&pdev->dev,
+				"FIFO Underflow Exception flag for channel %d\n",
+				i);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t micfil_err_isr(int irq, void *devid)
+{
+	struct fsl_micfil *micfil = (struct fsl_micfil *)devid;
+	struct platform_device *pdev = micfil->pdev;
+	u32 stat_reg;
+
+	regmap_read(micfil->regmap, REG_MICFIL_STAT, &stat_reg);
+
+	if (stat_reg & MICFIL_STAT_BSY_FIL_MASK)
+		dev_dbg(&pdev->dev, "isr: Decimation Filter is running\n");
+
+	if (stat_reg & MICFIL_STAT_FIR_RDY_MASK)
+		dev_dbg(&pdev->dev, "isr: FIR Filter Data ready\n");
+
+	if (stat_reg & MICFIL_STAT_LOWFREQF_MASK) {
+		dev_dbg(&pdev->dev, "isr: ipg_clk_app is too low\n");
+		regmap_write_bits(micfil->regmap, REG_MICFIL_STAT,
+				  MICFIL_STAT_LOWFREQF_MASK, 1);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int fsl_set_clock_params(struct device *, unsigned int);
+
+static int enable_hwvad(struct device *dev, bool sync)
+{
+	struct fsl_micfil *micfil = dev_get_drvdata(dev);
+	int ret;
+	int rate;
+	u32 state;
+
+	if (sync)
+		pm_runtime_get_sync(dev);
+
+	state = atomic_cmpxchg(&micfil->hwvad_state,
+			       MICFIL_HWVAD_OFF,
+			       MICFIL_HWVAD_ON);
+
+	/* we should not reenable when sync = true because
+	 * this means enable was called for second time by
+	 * user. However state = ON and sync = false can only
+	 * occur when enable is called from system_resume. In
+	 * this case we should enable the hwvad
+	 */
+	if (sync && state == MICFIL_HWVAD_ON) {
+		dev_err(dev, "hwvad already on\n");
+		ret = -EBUSY;
+		goto enable_error;
+	}
+
+	if (micfil->vad_rate_index >= ARRAY_SIZE(micfil_hwvad_rate_ints)) {
+		dev_err(dev, "There are more select texts than rates\n");
+		ret = -EINVAL;
+		goto enable_error;
+	}
+
+	rate = micfil_hwvad_rate_ints[micfil->vad_rate_index];
+
+	/* This is required because if an arecord was done,
+	 * suspend function will mark regmap as cache only
+	 * and reads/writes in volatile regs will fail
+	 */
+	regcache_cache_only(micfil->regmap, false);
+	regcache_mark_dirty(micfil->regmap);
+	regcache_sync(micfil->regmap);
+
+	ret = fsl_set_clock_params(dev, rate);
+	if (ret)
+		return ret;
+
+	ret = fsl_micfil_reset(dev);
+	if (ret)
+		return ret;
+
+	/* Initialize Hardware Voice Activity */
+	ret = init_hwvad(dev);
+
+	return ret;
+enable_error:
+	if (sync)
+		pm_runtime_put_sync(dev);
+	return ret;
+}
+
+static int disable_hwvad(struct device *dev, bool sync)
+{
+	struct fsl_micfil *micfil = dev_get_drvdata(dev);
+	int ret = 0;
+	u32 state;
+
+	/* This is required because if an arecord was done,
+	 * suspend function will mark regmap as cache only
+	 * and reads/writes in volatile regs will fail
+	 */
+	regcache_cache_only(micfil->regmap, false);
+	regcache_mark_dirty(micfil->regmap);
+	regcache_sync(micfil->regmap);
+
+	/* disable is called with sync = false only from
+	 * system suspend and in this case, you should not
+	 * change the hwvad_state so we know at system_resume
+	 * to reenable hwvad
+	 */
+	if (sync)
+		state = atomic_cmpxchg(&micfil->hwvad_state,
+				       MICFIL_HWVAD_ON,
+				       MICFIL_HWVAD_OFF);
+	else
+		state = atomic_read(&micfil->hwvad_state);
+
+	if (state == MICFIL_HWVAD_ON) {
+		/* Voice Activity Detector Reset */
+		ret |= regmap_update_bits(micfil->regmap,
+					  REG_MICFIL_VAD0_CTRL1,
+					  MICFIL_VAD0_CTRL1_RST_SHIFT,
+					  MICFIL_VAD0_CTRL1_RST);
+
+		/* Disable HWVAD */
+		ret |= regmap_update_bits(micfil->regmap,
+					  REG_MICFIL_VAD0_CTRL1,
+					  MICFIL_VAD0_CTRL1_EN_MASK,
+					  0);
+
+		/* Disable Signal Filter */
+		ret |= regmap_update_bits(micfil->regmap,
+					  REG_MICFIL_VAD0_SCONFIG,
+					  MICFIL_VAD0_SCONFIG_SFILEN_MASK,
+					  0);
+
+		/* Signal Maximum Enable */
+		ret |= regmap_update_bits(micfil->regmap,
+					  REG_MICFIL_VAD0_SCONFIG,
+					  MICFIL_VAD0_SCONFIG_SMAXEN_MASK,
+					  0);
+
+		/* Enable pre-filter Noise & Signal */
+		ret |= regmap_update_bits(micfil->regmap,
+					  REG_MICFIL_VAD0_CTRL2,
+					  MICFIL_VAD0_CTRL2_PREFEN_MASK,
+					  0);
+
+		/* Noise Decimation Enable */
+		ret |= regmap_update_bits(micfil->regmap,
+					  REG_MICFIL_VAD0_NCONFIG,
+					  MICFIL_VAD0_NCONFIG_NDECEN_MASK,
+					  0);
+
+		/* disable the module and clock only if recording
+		 * is not done in parallel
+		 */
+		state = atomic_read(&micfil->recording_state);
+		if (state == MICFIL_RECORDING_OFF) {
+		/* Disable MICFIL module */
+			ret |= regmap_update_bits(micfil->regmap,
+						  REG_MICFIL_CTRL1,
+						  MICFIL_CTRL1_PDMIEN_MASK,
+						  0);
+		}
+
+	} else {
+		ret = -EPERM;
+		dev_err(dev, "HWVAD is not enabled %d\n", ret);
+	}
+
+	if (sync)
+		pm_runtime_put_sync(dev);
+
+	return ret;
+}
+
+static ssize_t micfil_hwvad_handler(struct kobject *kobj,
+				    struct kobj_attribute *attr,
+				    const char *buf,
+				    size_t count)
+{
+	struct kobject *nand_kobj = kobj->parent;
+	struct device *dev = container_of(nand_kobj, struct device, kobj);
+	struct fsl_micfil *micfil = dev_get_drvdata(dev);
+	unsigned long vad_channel;
+	int ret;
+
+	ret = kstrtoul(buf, 16, &vad_channel);
+	if (ret < 0)
+		return -EINVAL;
+
+	if (vad_channel <= 7) {
+		micfil->vad_channel = vad_channel;
+		ret = enable_hwvad(dev, true);
+		if (ret)
+			dev_err(dev, "Failed to enable hwvad");
+	} else {
+		micfil->vad_channel = -1;
+		ret = disable_hwvad(dev, true);
+	}
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+static struct kobj_attribute hwvad_en_attr = __ATTR(enable,
+						   0660,
+						   NULL,
+						   micfil_hwvad_handler);
+
+static int fsl_micfil_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	const struct of_device_id *of_id;
+	struct fsl_micfil *micfil;
+	struct resource *res;
+	void __iomem *regs;
+	int ret, i;
+	unsigned long irqflag = 0;
+
+	micfil = devm_kzalloc(&pdev->dev, sizeof(*micfil), GFP_KERNEL);
+	if (!micfil)
+		return -ENOMEM;
+
+	micfil->pdev = pdev;
+	strncpy(micfil->name, np->name, sizeof(micfil->name) - 1);
+
+	of_id = of_match_device(fsl_micfil_dt_ids, &pdev->dev);
+	if (!of_id || !of_id->data)
+		return -EINVAL;
+
+	micfil->soc = of_id->data;
+
+	/* ipg_clk is used to control the registers
+	 * ipg_clk_app is used to operate the filter
+	 */
+	micfil->mclk = devm_clk_get(&pdev->dev, "ipg_clk_app");
+	if (IS_ERR(micfil->mclk)) {
+		dev_err(&pdev->dev, "failed to get core clock: %ld\n",
+			PTR_ERR(micfil->mclk));
+		return PTR_ERR(micfil->mclk);
+	}
+
+	/* get audio pll1 and pll2 */
+	micfil->clk_src[MICFIL_AUDIO_PLL1] = devm_clk_get(&pdev->dev, "pll8k");
+	if (IS_ERR(micfil->clk_src[MICFIL_AUDIO_PLL1]))
+		micfil->clk_src[MICFIL_AUDIO_PLL1] = NULL;
+
+	micfil->clk_src[MICFIL_AUDIO_PLL2] = devm_clk_get(&pdev->dev, "pll11k");
+	if (IS_ERR(micfil->clk_src[MICFIL_AUDIO_PLL2]))
+		micfil->clk_src[MICFIL_AUDIO_PLL2] = NULL;
+
+	micfil->clk_src[MICFIL_CLK_EXT3] = devm_clk_get(&pdev->dev, "clkext3");
+	if (IS_ERR(micfil->clk_src[MICFIL_CLK_EXT3]))
+		micfil->clk_src[MICFIL_CLK_EXT3] = NULL;
+
+	/* init regmap */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
+
+	micfil->regmap = devm_regmap_init_mmio_clk(&pdev->dev,
+						   "ipg_clk",
+						   regs,
+						   &fsl_micfil_regmap_config);
+	if (IS_ERR(micfil->regmap)) {
+		dev_err(&pdev->dev, "failed to init MICFIL regmap: %ld\n",
+			PTR_ERR(micfil->regmap));
+		return PTR_ERR(micfil->regmap);
+	}
+
+	/* dataline mask for RX */
+	ret = of_property_read_u32_index(np,
+					 "fsl,dataline",
+					 0,
+					 &micfil->dataline);
+	if (ret)
+		micfil->dataline = 1;
+
+	if (micfil->dataline & ~micfil->soc->dataline) {
+		dev_err(&pdev->dev, "dataline setting error, Mask is 0x%X\n",
+			micfil->soc->dataline);
+		return -EINVAL;
+	}
+
+	/* get IRQs */
+	for (i = 0; i < MICFIL_IRQ_LINES; i++) {
+		micfil->irq[i] = platform_get_irq(pdev, i);
+		dev_err(&pdev->dev, "GET IRQ: %d\n", micfil->irq[i]);
+		if (micfil->irq[i] < 0) {
+			dev_err(&pdev->dev, "no irq for node %s\n", pdev->name);
+			return micfil->irq[i];
+		}
+	}
+
+	if (of_property_read_bool(np, "fsl,shared-interrupt"))
+		irqflag = IRQF_SHARED;
+
+	/* Digital Microphone interface voice activity detector event
+	 * interrupt - IRQ 44
+	 */
+	ret = devm_request_threaded_irq(&pdev->dev, micfil->irq[0],
+					hwvad_isr, voice_detected_fn,
+					irqflag, micfil->name, micfil);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to claim hwvad event irq %u\n",
+			micfil->irq[0]);
+		return ret;
+	}
+
+	/* Digital Microphone interface voice activity detector error
+	 * interrupt - IRQ 45
+	 */
+	ret = devm_request_irq(&pdev->dev, micfil->irq[1],
+			       hwvad_err_isr, irqflag,
+			       micfil->name, micfil);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to claim hwvad error irq %u\n",
+			micfil->irq[1]);
+		return ret;
+	}
+
+	/* Digital Microphone interface interrupt - IRQ 109 */
+	ret = devm_request_irq(&pdev->dev, micfil->irq[2],
+			       micfil_isr, irqflag,
+			       micfil->name, micfil);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to claim mic interface irq %u\n",
+			micfil->irq[2]);
+		return ret;
+	}
+
+	/* Digital Microphone interface error interrupt - IRQ 110 */
+	ret = devm_request_irq(&pdev->dev, micfil->irq[3],
+			       micfil_err_isr, irqflag,
+			       micfil->name, micfil);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to claim mic interface error irq %u\n",
+			micfil->irq[3]);
+		return ret;
+	}
+
+	micfil->slave_mode = false;
+
+	micfil->dma_params_rx.chan_name = "rx";
+	micfil->dma_params_rx.addr = res->start + REG_MICFIL_DATACH0;
+	micfil->dma_params_rx.maxburst = MICFIL_DMA_MAXBURST_RX;
+
+	/* set default rate to first value in available vad rates */
+	micfil->vad_rate_index = 0;
+
+	platform_set_drvdata(pdev, micfil);
+
+	pm_runtime_enable(&pdev->dev);
+
+	ret = devm_snd_soc_register_component(&pdev->dev, &fsl_micfil_component,
+					      &fsl_micfil_dai, 1);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register component %s\n",
+			fsl_micfil_component.name);
+		return ret;
+	}
+
+	ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to pcm register\n");
+		return ret;
+	}
+
+	/* create sysfs entry used to enable hwvad from userspace */
+	micfil->hwvad_kobject = kobject_create_and_add("hwvad",
+						       &pdev->dev.kobj);
+	if (!micfil->hwvad_kobject)
+		return -ENOMEM;
+
+	ret = sysfs_create_file(micfil->hwvad_kobject,
+				&hwvad_en_attr.attr);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to create file for hwvad_enable\n");
+		kobject_put(micfil->hwvad_kobject);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int __maybe_unused fsl_micfil_runtime_suspend(struct device *dev)
+{
+	struct fsl_micfil *micfil = dev_get_drvdata(dev);
+	u32 state;
+
+	state = atomic_read(&micfil->hwvad_state);
+	if (state == MICFIL_HWVAD_ON)
+		return 0;
+
+	regcache_cache_only(micfil->regmap, true);
+
+	/* Disable the clock only if the hwvad is not enabled */
+	if (state == MICFIL_HWVAD_OFF)
+		clk_disable_unprepare(micfil->mclk);
+
+	return 0;
+}
+
+static int __maybe_unused fsl_micfil_runtime_resume(struct device *dev)
+{
+	struct fsl_micfil *micfil = dev_get_drvdata(dev);
+	int ret;
+	u32 state;
+
+	state = atomic_read(&micfil->hwvad_state);
+
+	/* enable mclk only if the hwvad is not enabled
+	 * When hwvad is enabled, clock won't be disabled
+	 * in suspend since hwvad and recording share the
+	 * same clock
+	 */
+	if (state == MICFIL_HWVAD_ON)
+		return 0;
+
+	ret = clk_prepare_enable(micfil->mclk);
+	if (ret < 0)
+		return ret;
+
+	regcache_cache_only(micfil->regmap, false);
+	regcache_mark_dirty(micfil->regmap);
+	regcache_sync(micfil->regmap);
+
+	return 0;
+}
+#endif /* CONFIG_PM*/
+
+#ifdef CONFIG_PM_SLEEP
+static int __maybe_unused fsl_micfil_suspend(struct device *dev)
+{
+	struct fsl_micfil *micfil = dev_get_drvdata(dev);
+	int ret;
+	u32 state;
+
+	state = atomic_read(&micfil->hwvad_state);
+
+	if (state == MICFIL_HWVAD_ON) {
+		dev_err(dev, "Disabling hwvad on suspend");
+		ret = disable_hwvad(dev, false);
+		if (ret)
+			dev_warn(dev, "Failed to disable hwvad");
+	}
+
+	pm_runtime_force_suspend(dev);
+
+	return 0;
+}
+
+static int __maybe_unused fsl_micfil_resume(struct device *dev)
+{
+	struct fsl_micfil *micfil = dev_get_drvdata(dev);
+	int ret;
+	u32 state;
+
+	pm_runtime_force_resume(dev);
+
+	state = atomic_read(&micfil->hwvad_state);
+	if (state == MICFIL_HWVAD_ON) {
+		dev_err(dev, "Enabling hwvad on resume");
+		ret = enable_hwvad(dev, false);
+		if (ret)
+			dev_warn(dev, "Failed to re-enable hwvad");
+	}
+
+	return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static const struct dev_pm_ops fsl_micfil_pm_ops = {
+	SET_RUNTIME_PM_OPS(fsl_micfil_runtime_suspend,
+			   fsl_micfil_runtime_resume,
+			   NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(fsl_micfil_suspend,
+				fsl_micfil_resume)
+};
+
+static struct platform_driver fsl_micfil_driver = {
+	.probe = fsl_micfil_probe,
+	.driver = {
+		.name = "fsl-micfil-dai",
+		.pm = &fsl_micfil_pm_ops,
+		.of_match_table = fsl_micfil_dt_ids,
+	},
+};
+module_platform_driver(fsl_micfil_driver);
+
+MODULE_AUTHOR("Cosmin-Gabriel Samoila <cosmin.samoila@nxp.com>");
+MODULE_DESCRIPTION("NXP PDM Microphone Interface (MICFIL) driver");
+MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/fsl/fsl_micfil.h b/sound/soc/fsl/fsl_micfil.h
new file mode 100644
index 0000000..792187b
--- /dev/null
+++ b/sound/soc/fsl/fsl_micfil.h
@@ -0,0 +1,317 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * PDM Microphone Interface for the NXP i.MX SoC
+ * Copyright 2018 NXP
+ */
+
+#ifndef _FSL_MICFIL_H
+#define _FSL_MICFIL_H
+
+/* MICFIL Register Map */
+#define REG_MICFIL_CTRL1		0x00
+#define REG_MICFIL_CTRL2		0x04
+#define REG_MICFIL_STAT			0x08
+#define REG_MICFIL_FIFO_CTRL		0x10
+#define REG_MICFIL_FIFO_STAT		0x14
+#define REG_MICFIL_DATACH0		0x24
+#define REG_MICFIL_DATACH1		0x28
+#define REG_MICFIL_DATACH2		0x2C
+#define REG_MICFIL_DATACH3		0x30
+#define REG_MICFIL_DATACH4		0x34
+#define REG_MICFIL_DATACH5		0x38
+#define REG_MICFIL_DATACH6		0x3C
+#define REG_MICFIL_DATACH7		0x40
+#define REG_MICFIL_DC_CTRL		0x64
+#define REG_MICFIL_OUT_CTRL		0x74
+#define REG_MICFIL_OUT_STAT		0x7C
+#define REG_MICFIL_VAD0_CTRL1		0x90
+#define REG_MICFIL_VAD0_CTRL2		0x94
+#define REG_MICFIL_VAD0_STAT		0x98
+#define REG_MICFIL_VAD0_SCONFIG		0x9C
+#define REG_MICFIL_VAD0_NCONFIG		0xA0
+#define REG_MICFIL_VAD0_NDATA		0xA4
+#define REG_MICFIL_VAD0_ZCD		0xA8
+
+/* MICFIL Control Register 1 -- REG_MICFILL_CTRL1 0x00 */
+#define MICFIL_CTRL1_MDIS_SHIFT		31
+#define MICFIL_CTRL1_MDIS_MASK		BIT(MICFIL_CTRL1_MDIS_SHIFT)
+#define MICFIL_CTRL1_MDIS		BIT(MICFIL_CTRL1_MDIS_SHIFT)
+#define MICFIL_CTRL1_DOZEN_SHIFT	30
+#define MICFIL_CTRL1_DOZEN_MASK		BIT(MICFIL_CTRL1_DOZEN_SHIFT)
+#define MICFIL_CTRL1_DOZEN		BIT(MICFIL_CTRL1_DOZEN_SHIFT)
+#define MICFIL_CTRL1_PDMIEN_SHIFT	29
+#define MICFIL_CTRL1_PDMIEN_MASK	BIT(MICFIL_CTRL1_PDMIEN_SHIFT)
+#define MICFIL_CTRL1_PDMIEN		BIT(MICFIL_CTRL1_PDMIEN_SHIFT)
+#define MICFIL_CTRL1_DBG_SHIFT		28
+#define MICFIL_CTRL1_DBG_MASK		BIT(MICFIL_CTRL1_DBG_SHIFT)
+#define MICFIL_CTRL1_DBG		BIT(MICFIL_CTRL1_DBG_SHIFT)
+#define MICFIL_CTRL1_SRES_SHIFT		27
+#define MICFIL_CTRL1_SRES_MASK		BIT(MICFIL_CTRL1_SRES_SHIFT)
+#define MICFIL_CTRL1_SRES		BIT(MICFIL_CTRL1_SRES_SHIFT)
+#define MICFIL_CTRL1_DBGE_SHIFT		26
+#define MICFIL_CTRL1_DBGE_MASK		BIT(MICFIL_CTRL1_DBGE_SHIFT)
+#define MICFIL_CTRL1_DBGE		BIT(MICFIL_CTRL1_DBGE_SHIFT)
+#define MICFIL_CTRL1_DISEL_SHIFT	24
+#define MICFIL_CTRL1_DISEL_WIDTH	2
+#define MICFIL_CTRL1_DISEL_MASK		((BIT(MICFIL_CTRL1_DISEL_WIDTH) - 1) \
+					 << MICFIL_CTRL1_DISEL_SHIFT)
+#define MICFIL_CTRL1_DISEL(v)		(((v) << MICFIL_CTRL1_DISEL_SHIFT) \
+					 & MICFIL_CTRL1_DISEL_MASK)
+#define MICFIL_CTRL1_ERREN_SHIFT	23
+#define MICFIL_CTRL1_ERREN_MASK		BIT(MICFIL_CTRL1_ERREN_SHIFT)
+#define MICFIL_CTRL1_ERREN		BIT(MICFIL_CTRL1_ERREN_SHIFT)
+#define MICFIL_CTRL1_CHEN_SHIFT		0
+#define MICFIL_CTRL1_CHEN_WIDTH		8
+#define MICFIL_CTRL1_CHEN_MASK(x)	(BIT(x) << MICFIL_CTRL1_CHEN_SHIFT)
+#define MICFIL_CTRL1_CHEN(x)		(MICFIL_CTRL1_CHEN_MASK(x))
+
+/* MICFIL Control Register 2 -- REG_MICFILL_CTRL2 0x04 */
+#define MICFIL_CTRL2_QSEL_SHIFT		25
+#define MICFIL_CTRL2_QSEL_WIDTH		3
+#define MICFIL_CTRL2_QSEL_MASK		((BIT(MICFIL_CTRL2_QSEL_WIDTH) - 1) \
+					 << MICFIL_CTRL2_QSEL_SHIFT)
+#define MICFIL_HIGH_QUALITY		BIT(MICFIL_CTRL2_QSEL_SHIFT)
+#define MICFIL_MEDIUM_QUALITY		(0 << MICFIL_CTRL2_QSEL_SHIFT)
+#define MICFIL_LOW_QUALITY		(7 << MICFIL_CTRL2_QSEL_SHIFT)
+#define MICFIL_VLOW0_QUALITY		(6 << MICFIL_CTRL2_QSEL_SHIFT)
+#define MICFIL_VLOW1_QUALITY		(5 << MICFIL_CTRL2_QSEL_SHIFT)
+#define MICFIL_VLOW2_QUALITY		(4 << MICFIL_CTRL2_QSEL_SHIFT)
+
+#define MICFIL_CTRL2_CICOSR_SHIFT	16
+#define MICFIL_CTRL2_CICOSR_WIDTH	4
+#define MICFIL_CTRL2_CICOSR_MASK	((BIT(MICFIL_CTRL2_CICOSR_WIDTH) - 1) \
+					 << MICFIL_CTRL2_CICOSR_SHIFT)
+#define MICFIL_CTRL2_CICOSR(v)		(((v) << MICFIL_CTRL2_CICOSR_SHIFT) \
+					 & MICFIL_CTRL2_CICOSR_MASK)
+#define MICFIL_CTRL2_CLKDIV_SHIFT	0
+#define MICFIL_CTRL2_CLKDIV_WIDTH	8
+#define MICFIL_CTRL2_CLKDIV_MASK	((BIT(MICFIL_CTRL2_CLKDIV_WIDTH) - 1) \
+					 << MICFIL_CTRL2_CLKDIV_SHIFT)
+#define MICFIL_CTRL2_CLKDIV(v)		(((v) << MICFIL_CTRL2_CLKDIV_SHIFT) \
+					 & MICFIL_CTRL2_CLKDIV_MASK)
+
+/* MICFIL Status Register -- REG_MICFIL_STAT 0x08 */
+#define MICFIL_STAT_BSY_FIL_SHIFT	31
+#define MICFIL_STAT_BSY_FIL_MASK	BIT(MICFIL_STAT_BSY_FIL_SHIFT)
+#define MICFIL_STAT_BSY_FIL		BIT(MICFIL_STAT_BSY_FIL_SHIFT)
+#define MICFIL_STAT_FIR_RDY_SHIFT	30
+#define MICFIL_STAT_FIR_RDY_MASK	BIT(MICFIL_STAT_FIR_RDY_SHIFT)
+#define MICFIL_STAT_FIR_RDY		BIT(MICFIL_STAT_FIR_RDY_SHIFT)
+#define MICFIL_STAT_LOWFREQF_SHIFT	29
+#define MICFIL_STAT_LOWFREQF_MASK	BIT(MICFIL_STAT_LOWFREQF_SHIFT)
+#define MICFIL_STAT_LOWFREQF		BIT(MICFIL_STAT_LOWFREQF_SHIFT)
+#define MICFIL_STAT_CHXF_SHIFT(v)	(v)
+#define MICFIL_STAT_CHXF_MASK(v)	BIT(MICFIL_STAT_CHXF_SHIFT(v))
+#define MICFIL_STAT_CHXF(v)		BIT(MICFIL_STAT_CHXF_SHIFT(v))
+
+/* MICFIL FIFO Control Register -- REG_MICFIL_FIFO_CTRL 0x10 */
+#define MICFIL_FIFO_CTRL_FIFOWMK_SHIFT	0
+#define MICFIL_FIFO_CTRL_FIFOWMK_WIDTH	3
+#define MICFIL_FIFO_CTRL_FIFOWMK_MASK	((BIT(MICFIL_FIFO_CTRL_FIFOWMK_WIDTH) - 1) \
+					 << MICFIL_FIFO_CTRL_FIFOWMK_SHIFT)
+#define MICFIL_FIFO_CTRL_FIFOWMK(v)	(((v) << MICFIL_FIFO_CTRL_FIFOWMK_SHIFT) \
+					 & MICFIL_FIFO_CTRL_FIFOWMK_MASK)
+
+/* MICFIL FIFO Status Register -- REG_MICFIL_FIFO_STAT 0x14 */
+#define MICFIL_FIFO_STAT_FIFOX_OVER_SHIFT(v)	(v)
+#define MICFIL_FIFO_STAT_FIFOX_OVER_MASK(v)	BIT(MICFIL_FIFO_STAT_FIFOX_OVER_SHIFT(v))
+#define MICFIL_FIFO_STAT_FIFOX_UNDER_SHIFT(v)	((v) + 8)
+#define MICFIL_FIFO_STAT_FIFOX_UNDER_MASK(v)	BIT(MICFIL_FIFO_STAT_FIFOX_UNDER_SHIFT(v))
+
+/* MICFIL HWVAD0 Control 1 Register -- REG_MICFIL_VAD0_CTRL1*/
+#define MICFIL_VAD0_CTRL1_CHSEL_SHIFT	24
+#define MICFIL_VAD0_CTRL1_CHSEL_WIDTH	3
+#define MICFIL_VAD0_CTRL1_CHSEL_MASK	((BIT(MICFIL_VAD0_CTRL1_CHSEL_WIDTH) - 1) \
+					 << MICFIL_VAD0_CTRL1_CHSEL_SHIFT)
+#define MICFIL_VAD0_CTRL1_CHSEL(v)	(((v) << MICFIL_VAD0_CTRL1_CHSEL_SHIFT) \
+					 & MICFIL_VAD0_CTRL1_CHSEL_MASK)
+#define MICFIL_VAD0_CTRL1_CICOSR_SHIFT	16
+#define MICFIL_VAD0_CTRL1_CICOSR_WIDTH	4
+#define MICFIL_VAD0_CTRL1_CICOSR_MASK	((BIT(MICFIL_VAD0_CTRL1_CICOSR_WIDTH) - 1) \
+					 << MICFIL_VAD0_CTRL1_CICOSR_SHIFT)
+#define MICFIL_VAD0_CTRL1_CICOSR(v)	(((v) << MICFIL_VAD0_CTRL1_CICOSR_SHIFT) \
+					 & MICFIL_VAD0_CTRL1_CICOSR_MASK)
+#define MICFIL_VAD0_CTRL1_INITT_SHIFT	8
+#define MICFIL_VAD0_CTRL1_INITT_WIDTH	5
+#define MICFIL_VAD0_CTRL1_INITT_MASK	((BIT(MICFIL_VAD0_CTRL1_INITT_WIDTH) - 1) \
+					 << MICFIL_VAD0_CTRL1_INITT_SHIFT)
+#define MICFIL_VAD0_CTRL1_INITT(v)	(((v) << MICFIL_VAD0_CTRL1_INITT_SHIFT) \
+					 & MICFIL_VAD0_CTRL1_INITT_MASK)
+#define MICFIL_VAD0_CTRL1_ST10_SHIFT	4
+#define MICFIL_VAD0_CTRL1_ST10_MASK	BIT(MICFIL_VAD0_CTRL1_ST10_SHIFT)
+#define MICFIL_VAD0_CTRL1_ST10		BIT(MICFIL_VAD0_CTRL1_ST10_SHIFT)
+#define MICFIL_VAD0_CTRL1_ERIE_SHIFT	3
+#define MICFIL_VAD0_CTRL1_ERIE_MASK	BIT(MICFIL_VAD0_CTRL1_ERIE_SHIFT)
+#define MICFIL_VAD0_CTRL1_ERIE		BIT(MICFIL_VAD0_CTRL1_ERIE_SHIFT)
+#define MICFIL_VAD0_CTRL1_IE_SHIFT	2
+#define MICFIL_VAD0_CTRL1_IE_MASK	BIT(MICFIL_VAD0_CTRL1_IE_SHIFT)
+#define MICFIL_VAD0_CTRL1_IE		BIT(MICFIL_VAD0_CTRL1_IE_SHIFT)
+#define MICFIL_VAD0_CTRL1_RST_SHIFT	1
+#define MICFIL_VAD0_CTRL1_RST_MASK	BIT(MICFIL_VAD0_CTRL1_RST_SHIFT)
+#define MICFIL_VAD0_CTRL1_RST		BIT(MICFIL_VAD0_CTRL1_RST_SHIFT)
+#define MICFIL_VAD0_CTRL1_EN_SHIFT	0
+#define MICFIL_VAD0_CTRL1_EN_MASK	BIT(MICFIL_VAD0_CTRL1_EN_SHIFT)
+#define MICFIL_VAD0_CTRL1_EN		BIT(MICFIL_VAD0_CTRL1_EN_SHIFT)
+
+/* MICFIL HWVAD0 Control 2 Register -- REG_MICFIL_VAD0_CTRL2*/
+#define MICFIL_VAD0_CTRL2_FRENDIS_SHIFT	31
+#define MICFIL_VAD0_CTRL2_FRENDIS_MASK	BIT(MICFIL_VAD0_CTRL2_FRENDIS_SHIFT)
+#define MICFIL_VAD0_CTRL2_FRENDIS	BIT(MICFIL_VAD0_CTRL2_FRENDIS_SHIFT)
+#define MICFIL_VAD0_CTRL2_PREFEN_SHIFT	30
+#define MICFIL_VAD0_CTRL2_PREFEN_MASK	BIT(MICFIL_VAD0_CTRL2_PREFEN_SHIFT)
+#define MICFIL_VAD0_CTRL2_PREFEN	BIT(MICFIL_VAD0_CTRL2_PREFEN_SHIFT)
+#define MICFIL_VAD0_CTRL2_FOUTDIS_SHIFT	28
+#define MICFIL_VAD0_CTRL2_FOUTDIS_MASK	BIT(MICFIL_VAD0_CTRL2_FOUTDIS_SHIFT)
+#define MICFIL_VAD0_CTRL2_FOUTDIS	BIT(MICFIL_VAD0_CTRL2_FOUTDIS_SHIFT)
+#define MICFIL_VAD0_CTRL2_FRAMET_SHIFT	16
+#define MICFIL_VAD0_CTRL2_FRAMET_WIDTH	6
+#define MICFIL_VAD0_CTRL2_FRAMET_MASK	((BIT(MICFIL_VAD0_CTRL2_FRAMET_WIDTH) - 1) \
+					 << MICFIL_VAD0_CTRL2_FRAMET_SHIFT)
+#define MICFIL_VAD0_CTRL2_FRAMET(v)	(((v) << MICFIL_VAD0_CTRL2_FRAMET_SHIFT) \
+					 & MICFIL_VAD0_CTRL2_FRAMET_MASK)
+#define MICFIL_VAD0_CTRL2_INPGAIN_SHIFT	8
+#define MICFIL_VAD0_CTRL2_INPGAIN_WIDTH	4
+#define MICFIL_VAD0_CTRL2_INPGAIN_MASK	((BIT(MICFIL_VAD0_CTRL2_INPGAIN_WIDTH) - 1) \
+					 << MICFIL_VAD0_CTRL2_INPGAIN_SHIFT)
+#define MICFIL_VAD0_CTRL2_INPGAIN(v)	(((v) << MICFIL_VAD0_CTRL2_INPGAIN_SHIFT) \
+					& MICFIL_VAD0_CTRL2_INPGAIN_MASK)
+#define MICFIL_VAD0_CTRL2_HPF_SHIFT	0
+#define MICFIL_VAD0_CTRL2_HPF_WIDTH	2
+#define MICFIL_VAD0_CTRL2_HPF_MASK	((BIT(MICFIL_VAD0_CTRL2_HPF_WIDTH) - 1) \
+					 << MICFIL_VAD0_CTRL2_HPF_SHIFT)
+#define MICFIL_VAD0_CTRL2_HPF(v)	(((v) << MICFIL_VAD0_CTRL2_HPF_SHIFT) \
+					 & MICFIL_VAD0_CTRL2_HPF_MASK)
+
+/* MICFIL HWVAD0 Signal CONFIG Register -- REG_MICFIL_VAD0_SCONFIG */
+#define MICFIL_VAD0_SCONFIG_SFILEN_SHIFT	31
+#define MICFIL_VAD0_SCONFIG_SFILEN_MASK		BIT(MICFIL_VAD0_SCONFIG_SFILEN_SHIFT)
+#define MICFIL_VAD0_SCONFIG_SFILEN		BIT(MICFIL_VAD0_SCONFIG_SFILEN_SHIFT)
+#define MICFIL_VAD0_SCONFIG_SMAXEN_SHIFT	30
+#define MICFIL_VAD0_SCONFIG_SMAXEN_MASK		BIT(MICFIL_VAD0_SCONFIG_SMAXEN_SHIFT)
+#define MICFIL_VAD0_SCONFIG_SMAXEN		BIT(MICFIL_VAD0_SCONFIG_SMAXEN_SHIFT)
+#define MICFIL_VAD0_SCONFIG_SGAIN_SHIFT		0
+#define MICFIL_VAD0_SCONFIG_SGAIN_WIDTH		4
+#define MICFIL_VAD0_SCONFIG_SGAIN_MASK		((BIT(MICFIL_VAD0_SCONFIG_SGAIN_WIDTH) - 1) \
+						<< MICFIL_VAD0_SCONFIG_SGAIN_SHIFT)
+#define MICFIL_VAD0_SCONFIG_SGAIN(v)		(((v) << MICFIL_VAD0_SCONFIG_SGAIN_SHIFT) \
+						 & MICFIL_VAD0_SCONFIG_SGAIN_MASK)
+
+/* MICFIL HWVAD0 Noise CONFIG Register -- REG_MICFIL_VAD0_NCONFIG */
+#define MICFIL_VAD0_NCONFIG_NFILAUT_SHIFT	31
+#define MICFIL_VAD0_NCONFIG_NFILAUT_MASK	BIT(MICFIL_VAD0_NCONFIG_NFILAUT_SHIFT)
+#define MICFIL_VAD0_NCONFIG_NFILAUT		BIT(MICFIL_VAD0_NCONFIG_NFILAUT_SHIFT)
+#define MICFIL_VAD0_NCONFIG_NMINEN_SHIFT	30
+#define MICFIL_VAD0_NCONFIG_NMINEN_MASK		BIT(MICFIL_VAD0_NCONFIG_NMINEN_SHIFT)
+#define MICFIL_VAD0_NCONFIG_NMINEN		BIT(MICFIL_VAD0_NCONFIG_NMINEN_SHIFT)
+#define MICFIL_VAD0_NCONFIG_NDECEN_SHIFT	29
+#define MICFIL_VAD0_NCONFIG_NDECEN_MASK		BIT(MICFIL_VAD0_NCONFIG_NDECEN_SHIFT)
+#define MICFIL_VAD0_NCONFIG_NDECEN		BIT(MICFIL_VAD0_NCONFIG_NDECEN_SHIFT)
+#define MICFIL_VAD0_NCONFIG_NOREN_SHIFT		28
+#define MICFIL_VAD0_NCONFIG_NOREN		BIT(MICFIL_VAD0_NCONFIG_NOREN_SHIFT)
+#define MICFIL_VAD0_NCONFIG_NFILADJ_SHIFT	8
+#define MICFIL_VAD0_NCONFIG_NFILADJ_WIDTH	5
+#define MICFIL_VAD0_NCONFIG_NFILADJ_MASK	((BIT(MICFIL_VAD0_NCONFIG_NFILADJ_WIDTH) - 1) \
+						 << MICFIL_VAD0_NCONFIG_NFILADJ_SHIFT)
+#define MICFIL_VAD0_NCONFIG_NFILADJ(v)		(((v) << MICFIL_VAD0_NCONFIG_NFILADJ_SHIFT) \
+						 & MICFIL_VAD0_NCONFIG_NFILADJ_MASK)
+#define MICFIL_VAD0_NCONFIG_NGAIN_SHIFT		0
+#define MICFIL_VAD0_NCONFIG_NGAIN_WIDTH		4
+#define MICFIL_VAD0_NCONFIG_NGAIN_MASK		((BIT(MICFIL_VAD0_NCONFIG_NGAIN_WIDTH) - 1) \
+						 << MICFIL_VAD0_NCONFIG_NGAIN_SHIFT)
+#define MICFIL_VAD0_NCONFIG_NGAIN(v)		(((v) << MICFIL_VAD0_NCONFIG_NGAIN_SHIFT) \
+						 & MICFIL_VAD0_NCONFIG_NGAIN_MASK)
+
+/* MICFIL HWVAD0 Zero-Crossing Detector - REG_MICFIL_VAD0_ZCD */
+#define MICFIL_VAD0_ZCD_ZCDTH_SHIFT	16
+#define MICFIL_VAD0_ZCD_ZCDTH_WIDTH	10
+#define MICFIL_VAD0_ZCD_ZCDTH_MASK	((BIT(MICFIL_VAD0_ZCD_ZCDTH_WIDTH) - 1) \
+					 << MICFIL_VAD0_ZCD_ZCDTH_SHIFT)
+#define MICFIL_VAD0_ZCD_ZCDTH(v)	(((v) << MICFIL_VAD0_ZCD_ZCDTH_SHIFT)\
+					 & MICFIL_VAD0_ZCD_ZCDTH_MASK)
+#define MICFIL_VAD0_ZCD_ZCDADJ_SHIFT	8
+#define MICFIL_VAD0_ZCD_ZCDADJ_WIDTH	4
+#define MICFIL_VAD0_ZCD_ZCDADJ_MASK	((BIT(MICFIL_VAD0_ZCD_ZCDADJ_WIDTH) - 1)\
+					 << MICFIL_VAD0_ZCD_ZCDADJ_SHIFT)
+#define MICFIL_VAD0_ZCD_ZCDADJ(v)	(((v) << MICFIL_VAD0_ZCD_ZCDADJ_SHIFT)\
+					 & MICFIL_VAD0_ZCD_ZCDADJ_MASK)
+#define MICFIL_VAD0_ZCD_ZCDAND_SHIFT	4
+#define MICFIL_VAD0_ZCD_ZCDAND_MASK	BIT(MICFIL_VAD0_ZCD_ZCDAND_SHIFT)
+#define MICFIL_VAD0_ZCD_ZCDAND		BIT(MICFIL_VAD0_ZCD_ZCDAND_SHIFT)
+#define MICFIL_VAD0_ZCD_ZCDAUT_SHIFT	2
+#define MICFIL_VAD0_ZCD_ZCDAUT_MASK	BIT(MICFIL_VAD0_ZCD_ZCDAUT_SHIFT)
+#define MICFIL_VAD0_ZCD_ZCDAUT		BIT(MICFIL_VAD0_ZCD_ZCDAUT_SHIFT)
+#define MICFIL_VAD0_ZCD_ZCDEN_SHIFT	0
+#define MICFIL_VAD0_ZCD_ZCDEN_MASK	BIT(MICFIL_VAD0_ZCD_ZCDEN_SHIFT)
+#define MICFIL_VAD0_ZCD_ZCDEN		BIT(MICFIL_VAD0_ZCD_ZCDEN_SHIFT)
+
+/* MICFIL HWVAD0 Status Register - REG_MICFIL_VAD0_STAT */
+#define MICFIL_VAD0_STAT_INITF_SHIFT	31
+#define MICFIL_VAD0_STAT_INITF_MASK	BIT(MICFIL_VAD0_STAT_INITF_SHIFT)
+#define MICFIL_VAD0_STAT_INITF		BIT(MICFIL_VAD0_STAT_INITF_SHIFT)
+#define MICFIL_VAD0_STAT_INSATF_SHIFT	16
+#define MICFIL_VAD0_STAT_INSATF_MASK	BIT(MICFIL_VAD0_STAT_INSATF_SHIFT)
+#define MICFIL_VAD0_STAT_INSATF		BIT(MICFIL_VAD0_STAT_INSATF_SHIFT)
+#define MICFIL_VAD0_STAT_EF_SHIFT	15
+#define MICFIL_VAD0_STAT_EF_MASK	BIT(MICFIL_VAD0_STAT_EF_SHIFT)
+#define MICFIL_VAD0_STAT_EF		BIT(MICFIL_VAD0_STAT_EF_SHIFT)
+#define MICFIL_VAD0_STAT_IF_SHIFT	0
+#define MICFIL_VAD0_STAT_IF_MASK	BIT(MICFIL_VAD0_STAT_IF_SHIFT)
+#define MICFIL_VAD0_STAT_IF		BIT(MICFIL_VAD0_STAT_IF_SHIFT)
+
+/* HWVAD Constants */
+#define MICFIL_HWVAD_ENVELOPE_MODE	0
+#define MICFIL_HWVAD_ENERGY_MODE	1
+#define MICFIL_HWVAD_INIT_FRAMES	10
+#define MICFIL_HWVAD_INPGAIN		0
+#define MICFIL_HWVAD_SGAIN		6
+#define MICFIL_HWVAD_NGAIN		3
+#define MICFIL_HWVAD_NFILADJ		0
+#define MICFIL_HWVAD_ZCDADJ		(1 << (MICFIL_VAD0_ZCD_ZCDADJ_WIDTH - 2))
+#define MICFIL_HWVAD_ZCDTH		10	/* initial threshold value */
+#define MICFIL_HWVAD_ZCDOR		0
+#define MICFIL_HWVAD_ZCDAND		1
+#define MICFIL_HWVAD_ZCD_MANUAL		0
+#define MICFIL_HWVAD_ZCD_AUTO		1
+#define MICFIL_HWVAD_HPF_BYPASS		0
+#define MICFIL_HWVAD_HPF_1750HZ		1
+#define MICFIL_HWVAD_HPF_215HZ		2
+#define MICFIL_HWVAD_HPF_102HZ		3
+#define MICFIL_HWVAD_FRAMET_DEFAULT	10
+
+/* MICFIL Output Control Register */
+#define MICFIL_OUTGAIN_CHX_SHIFT(v)	(4 * (v))
+
+/* Constants */
+#define MICFIL_DMA_IRQ_DISABLED(v)	((v) & MICFIL_CTRL1_DISEL_MASK)
+#define MICFIL_DMA_ENABLED(v)		((0x1 << MICFIL_CTRL1_DISEL_SHIFT) \
+					 == ((v) & MICFIL_CTRL1_DISEL_MASK))
+#define MICFIL_IRQ_ENABLED(v)		((0x2 << MICFIL_CTRL1_DISEL_SHIFT) \
+					 == ((v) & MICFIL_CTRL1_DISEL_MASK))
+#define MICFIL_OUTPUT_CHANNELS		8
+#define MICFIL_FIFO_NUM			8
+
+#define FIFO_PTRWID			3
+#define FIFO_LEN			BIT(FIFO_PTRWID)
+
+#define MICFIL_IRQ_LINES		4
+#define MICFIL_MAX_RETRY		25
+#define MICFIL_SLEEP_MIN		90000 /* in us */
+#define MICFIL_SLEEP_MAX		100000 /* in us */
+#define MICFIL_DMA_MAXBURST_RX		6
+#define MICFIL_CTRL2_OSR_DEFAULT	(0 << MICFIL_CTRL2_CICOSR_SHIFT)
+#define MICFIL_DEFAULT_RATE		48000
+#define MICFIL_CLK_SRC_NUM		3
+#define MICFIL_CLK_AUTO			0
+
+/* clock source ids */
+#define MICFIL_AUDIO_PLL1		0
+#define MICFIL_AUDIO_PLL2		1
+#define MICFIL_CLK_EXT3			2
+
+/* States of micfil */
+#define MICFIL_HWVAD_OFF		0
+#define MICFIL_HWVAD_ON			1
+#define MICFIL_RECORDING_OFF		0
+#define MICFIL_RECORDING_ON		1
+
+#endif /* _FSL_MICFIL_H */
-- 
2.7.4

Re: [RFC 2/2] ASoC: Add MICFIL SoC Digital Audio Interface driver.

[Mark Brown]

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

On Mon, Dec 10, 2018 at 09:21:15AM +0000, Cosmin Samoila wrote:
> Add Digital Audio Interface driver that convers PDM bitstream to PCM
> format.

I'm missing patch 1/2 here - what's going on with dependencies?

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

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

Re: [RFC 2/2] ASoC: Add MICFIL SoC Digital Audio Interface driver.

[Daniel Baluta]

On Tue, Dec 11, 2018 at 3:09 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, Dec 10, 2018 at 09:21:15AM +0000, Cosmin Samoila wrote:
> > Add Digital Audio Interface driver that convers PDM bitstream to PCM
> > format.
>
> I'm missing patch 1/2 here - what's going on with dependencies?

Hmm, that's strange. I got patch 1/2 and it also appears to be on
the mailing list archive.

http://mailman.alsa-project.org/pipermail/alsa-devel/2018-December/142765.html

Maybe it got filtered somehow to another directory inside your email account?

thanks,
Daniel.

Re: [RFC 2/2] ASoC: Add MICFIL SoC Digital Audio Interface driver.

[Mark Brown]

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

On Tue, Dec 11, 2018 at 11:58:37AM +0200, Daniel Baluta wrote:
> On Tue, Dec 11, 2018 at 3:09 AM Mark Brown <broonie@kernel.org> wrote:

> > I'm missing patch 1/2 here - what's going on with dependencies?

> Hmm, that's strange. I got patch 1/2 and it also appears to be on
> the mailing list archive.

> http://mailman.alsa-project.org/pipermail/alsa-devel/2018-December/142765.html

> Maybe it got filtered somehow to another directory inside your email account?

No, it just got really delayed somewhere - it turned up this morning.

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

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

Sound card init

[Jean Manuel JACINTO]

Hello alsa team.
I'm not sure to write at the right place, so please apologize if i'm being off-topic.
I'm using a supposedly  standard-compliant soundcard with alsa on linux, and playback is going very fine without issue on the 4 channels.

However, inputs are not behaving well. Captured sound is 'choppy' (like small high pitched discontinuous chunks).
After many tries i figured a workaround : if i plug the card to a windows pc, even without using it, it will then capture perfectly from the linux station.

So i suppose the windows driver is doing some initialization. Possibly setting a latency value (i'm supposing that because this value is selectable from the driver tray icon)

As some experts might be on this list, i wanted to ask if you know a way to 'sniff' how the win driver is doing the init. So id be able to replay it with alsactl or custom code ?

Thanks, and sorry again if i'm asking in the wrong place.

Regards,
JM

Le 11 décembre 2018 10:58:37 GMT+01:00, Daniel Baluta <daniel.baluta@gmail.com> a écrit :
>On Tue, Dec 11, 2018 at 3:09 AM Mark Brown <broonie@kernel.org> wrote:
>>
>> On Mon, Dec 10, 2018 at 09:21:15AM +0000, Cosmin Samoila wrote:
>> > Add Digital Audio Interface driver that convers PDM bitstream to
>PCM
>> > format.
>>
>> I'm missing patch 1/2 here - what's going on with dependencies?
>
>Hmm, that's strange. I got patch 1/2 and it also appears to be on
>the mailing list archive.
>
>http://mailman.alsa-project.org/pipermail/alsa-devel/2018-December/142765.html
>
>Maybe it got filtered somehow to another directory inside your email
>account?
>
>thanks,
>Daniel.
>_______________________________________________
>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: [RFC 2/2] ASoC: Add MICFIL SoC Digital Audio Interface driver.

[Mark Brown]

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

On Mon, Dec 10, 2018 at 09:21:15AM +0000, Cosmin Samoila wrote:

> +static char *envp[] = {
> +		"EVENT=PDM_VOICE_DETECT",
> +		NULL,
> +	};

The indentation here is weird.

> +static const char * const micfil_hwvad_zcd_enable[] = {
> +	"OFF", "ON",
> +};

Simple on/off switches should just be regular controls with "Switch" at
the end of their name so userspace can handle them.

> +static const char * const micfil_hwvad_noise_decimation[] = {
> +	"Disabled", "Enabled",
> +};

Same here.

> +/* when adding new rate text, also add it to the
> + * micfil_hwvad_rate_ints
> + */
> +static const char * const micfil_hwvad_rate[] = {
> +	"48KHz", "44.1KHz",
> +};
> +
> +static const int micfil_hwvad_rate_ints[] = {
> +	48000, 44100,
> +};

Can the driver not determine this automatically from sample rates?

> +static const char * const micfil_clk_src_texts[] = {
> +	"Auto", "AudioPLL1", "AudioPLL2", "ExtClk3",
> +};

Is this something that should be user selectable or is it going to be
controlled by the board design?

> +static int hwvad_put_hpf(struct snd_kcontrol *kcontrol,
> +			 struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
> +	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
> +	unsigned int *item = ucontrol->value.enumerated.item;
> +	struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
> +	int val = snd_soc_enum_item_to_val(e, item[0]);
> +
> +	/* 00 - HPF Bypass
> +	 * 01 - Cut-off frequency 1750Hz
> +	 * 10 - Cut-off frequency 215Hz
> +	 * 11 - Cut-off frequency 102Hz
> +	 */
> +	micfil->vad_hpf = val;
> +
> +	return 0;
> +}

What happens if this gets changed while a stream is active - the user
will think they changed the configuration but it won't take effect until
the next stream is started AFAICT?

> +	/* a value remapping must be done since the gain field have
> +	 * the following meaning:
> +	 * * 0 : no gain
> +	 * * 1 - 7 : +1 to +7 bits gain
> +	 * * 8 - 15 : -8 to -1 bits gain
> +	 * After the remapp, the scale should start from -8 to +7
> +	 */

This looks like a signed value so one of the _S_VALUE macros should
handle things I think?

> +static const struct snd_kcontrol_new fsl_micfil_snd_controls[] = {
> +	SOC_SINGLE_RANGE_EXT_TLV("CH0 Gain", -1, MICFIL_OUTGAIN_CHX_SHIFT(0),
> +				 0x0, 0xF, 0,
> +				 get_channel_gain, put_channel_gain, gain_tlv),

All volume controls should have names ending in Volume so userspace
knows how to handle them.

> +/* Hardware Voice Active Detection: The HWVAD takes data from the input
> + * of a selected PDM microphone to detect if there is any
> + * voice activity. When a voice activity is detected, an interrupt could
> + * be delivered to the system. Initialization in section 8.4:
> + * Can work in two modes:
> + *  -> Eneveope-based mode (section 8.4.1)
> + *  -> Energy-based mode (section 8.4.2)
> + *
> + * It is important to remark that the HWVAD detector could be enabled
> + * or reset only when the MICFIL isn't running i.e. when the BSY_FIL
> + * bit in STAT register is cleared
> + */
> +static int __maybe_unused init_hwvad(struct device *dev)

Why is this annotated __maybey_unused?

> +static int fsl_micfil_hw_free(struct snd_pcm_substream *substream,
> +			      struct snd_soc_dai *dai)
> +{
> +	struct fsl_micfil *micfil = snd_soc_dai_get_drvdata(dai);
> +
> +	atomic_set(&micfil->recording_state, MICFIL_RECORDING_OFF);
> +
> +	return 0;
> +}

Are you *sure* you need to and want to use atomic_set() here and that
there's no race conditions resulting from trying to use an atomic
variable?  It's much simpler and clearer to use mutexes, if for some
reason atomic variables make sense then the reasoning needs to be
clearly documented as they're quite tricky and easy to break or
misunderstand.

> +static int fsl_micfil_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> +{
> +	struct fsl_micfil *micfil = snd_soc_dai_get_drvdata(dai);
> +
> +	/* DAI MODE */
> +	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> +	case SND_SOC_DAIFMT_I2S:
> +		break;
> +	default:
> +		return -EINVAL;
> +	}

Is this actually an I2S controller?  It looks like a PDM controller to
me and that's what your cover letter said.  Just omit this entirely if
the DAI format isn't configurable in the hardware.

> +	/* set default gain to max_gain */
> +	regmap_write(micfil->regmap, REG_MICFIL_OUT_CTRL, 0x77777777);
> +	for (i = 0; i < 8; i++)
> +		micfil->channel_gain[i] = 0xF;

I'm assuming the hardware has no defaults here but if we've got to pick
a gain wouldn't a low gain be less likely to blast out someone's
eardrums than a maximum gain?

> +static irqreturn_t voice_detected_fn(int irq, void *devid)
> +{
> +	struct fsl_micfil *micfil = (struct fsl_micfil *)devid;
> +	struct device *dev = &micfil->pdev->dev;
> +	int ret;
> +
> +	/* disable hwvad */
> +	ret = disable_hwvad(dev, true);
> +	if (ret)
> +		dev_err(dev, "Failed to disable HWVAD module\n");
> +
> +	/* notify userspace that voice was detected */
> +	kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
> +
> +	return IRQ_HANDLED;
> +}

So, this looks like it's intended to be used for keyword detection type
applications (though without the offload DSP that those tend to have).
What the other implementations I've seen have ended up doing is using a
compressed audio stream to return the audio data to userspace, allowing
the audion stream to be paused when no audio is detected.  Your approach
here is a bit more manual and may be more sensible for systems without
the offload DSP however the decision to go outside ALSA and use a
kobject needs to be thought about a bit, we'd want to ensure that
there's a standard way of handling hardware like this so applications
can work as consistently as possible with them.

It's probably best to split all this VAD handling code out into a
separate patch so that the basic support can get merged and this can be
reviewed separately.  The rest of the driver has some minor issues but
it looks like all the complexity is in this VAD code.

> +static irqreturn_t micfil_err_isr(int irq, void *devid)
> +{
> +	struct fsl_micfil *micfil = (struct fsl_micfil *)devid;
> +	struct platform_device *pdev = micfil->pdev;
> +	u32 stat_reg;
> +
> +	regmap_read(micfil->regmap, REG_MICFIL_STAT, &stat_reg);
> +
> +	if (stat_reg & MICFIL_STAT_BSY_FIL_MASK)
> +		dev_dbg(&pdev->dev, "isr: Decimation Filter is running\n");
> +
> +	if (stat_reg & MICFIL_STAT_FIR_RDY_MASK)
> +		dev_dbg(&pdev->dev, "isr: FIR Filter Data ready\n");
> +
> +	if (stat_reg & MICFIL_STAT_LOWFREQF_MASK) {
> +		dev_dbg(&pdev->dev, "isr: ipg_clk_app is too low\n");
> +		regmap_write_bits(micfil->regmap, REG_MICFIL_STAT,
> +				  MICFIL_STAT_LOWFREQF_MASK, 1);
> +	}
> +
> +	return IRQ_HANDLED;
> +}

This will uncondtionally report the interrupt as handled but if it sees
an error it doesn't recognize it won't log anything - that seems not
ideal, it'd be better to log the value we read in case there's something
else goes wrong to aid debug.

> +static int enable_hwvad(struct device *dev, bool sync)
> +{
> +	struct fsl_micfil *micfil = dev_get_drvdata(dev);
> +	int ret;
> +	int rate;
> +	u32 state;
> +
> +	if (sync)
> +		pm_runtime_get_sync(dev);
> +
> +	state = atomic_cmpxchg(&micfil->hwvad_state,
> +			       MICFIL_HWVAD_OFF,
> +			       MICFIL_HWVAD_ON);

This *really* needs some documentation about what's going on for
concurrency here.

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

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

Re: [RFC 2/2] ASoC: Add MICFIL SoC Digital Audio Interface driver.

[Cosmin Samoila]

Hello Mark,

Thank you for the review.
Please see my comments inline.

Best regards,
Cosmin

On Mi, 2018-12-12 at 18:14 +0000, Mark Brown wrote:
> On Mon, Dec 10, 2018 at 09:21:15AM +0000, Cosmin Samoila wrote:
> 
> > 
> > +static char *envp[] = {
> > +		"EVENT=PDM_VOICE_DETECT",
> > +		NULL,
> > +	};
> The indentation here is weird.

Will fix it.

> 
> > 
> > +static const char * const micfil_hwvad_zcd_enable[] = {
> > +	"OFF", "ON",
> > +};
> Simple on/off switches should just be regular controls with "Switch"
> at
> the end of their name so userspace can handle them.
> 
> > 
> > +static const char * const micfil_hwvad_noise_decimation[] = {
> > +	"Disabled", "Enabled",
> > +};
> Same here.

Will fix it.

> 
> > 
> > +/* when adding new rate text, also add it to the
> > + * micfil_hwvad_rate_ints
> > + */
> > +static const char * const micfil_hwvad_rate[] = {
> > +	"48KHz", "44.1KHz",
> > +};
> > +
> > +static const int micfil_hwvad_rate_ints[] = {
> > +	48000, 44100,
> > +};
> Can the driver not determine this automatically from sample rates?

I think I could add "48000", "44100" instead of "48KHz", "44.1KHz" and
use kstrtol()

> 
> > 
> > +static const char * const micfil_clk_src_texts[] = {
> > +	"Auto", "AudioPLL1", "AudioPLL2", "ExtClk3",
> > +};
> Is this something that should be user selectable or is it going to be
> controlled by the board design?

User should be able to select the clock source since, in theory,
hardware could support any custom rate as long as you can provide the
clock on extclk.
 
> 
> > 
> > +static int hwvad_put_hpf(struct snd_kcontrol *kcontrol,
> > +			 struct snd_ctl_elem_value *ucontrol)
> > +{
> > +	struct snd_soc_component *comp =
> > snd_kcontrol_chip(kcontrol);
> > +	struct soc_enum *e = (struct soc_enum *)kcontrol-
> > >private_value;
> > +	unsigned int *item = ucontrol->value.enumerated.item;
> > +	struct fsl_micfil *micfil =
> > snd_soc_component_get_drvdata(comp);
> > +	int val = snd_soc_enum_item_to_val(e, item[0]);
> > +
> > +	/* 00 - HPF Bypass
> > +	 * 01 - Cut-off frequency 1750Hz
> > +	 * 10 - Cut-off frequency 215Hz
> > +	 * 11 - Cut-off frequency 102Hz
> > +	 */
> > +	micfil->vad_hpf = val;
> > +
> > +	return 0;
> > +}
> What happens if this gets changed while a stream is active - the user
> will think they changed the configuration but it won't take effect
> until
> the next stream is started AFAICT?

If the stream is active, the configuration will indeed take efect on
the next stream but this is the desired behavior. If we would want to
do the config on the fly, we should use sync functions that also resets
the pdm module which is not what we intend.
User must first configure the module then start the recording since
this seems to be the natural flow.

> 
> > 
> > +	/* a value remapping must be done since the gain field
> > have
> > +	 * the following meaning:
> > +	 * * 0 : no gain
> > +	 * * 1 - 7 : +1 to +7 bits gain
> > +	 * * 8 - 15 : -8 to -1 bits gain
> > +	 * After the remapp, the scale should start from -8 to +7
> > +	 */
> This looks like a signed value so one of the _S_VALUE macros should
> handle things I think?
> 

Ok.

> > 
> > +static const struct snd_kcontrol_new fsl_micfil_snd_controls[] = {
> > +	SOC_SINGLE_RANGE_EXT_TLV("CH0 Gain", -1,
> > MICFIL_OUTGAIN_CHX_SHIFT(0),
> > +				 0x0, 0xF, 0,
> > +				 get_channel_gain,
> > put_channel_gain, gain_tlv),
> All volume controls should have names ending in Volume so userspace
> knows how to handle them.

Ok, I will change the name.

> 
> > 
> > +/* Hardware Voice Active Detection: The HWVAD takes data from the
> > input
> > + * of a selected PDM microphone to detect if there is any
> > + * voice activity. When a voice activity is detected, an interrupt
> > could
> > + * be delivered to the system. Initialization in section 8.4:
> > + * Can work in two modes:
> > + *  -> Eneveope-based mode (section 8.4.1)
> > + *  -> Energy-based mode (section 8.4.2)
> > + *
> > + * It is important to remark that the HWVAD detector could be
> > enabled
> > + * or reset only when the MICFIL isn't running i.e. when the
> > BSY_FIL
> > + * bit in STAT register is cleared
> > + */
> > +static int __maybe_unused init_hwvad(struct device *dev)
> Why is this annotated __maybey_unused?
> 

It remained from early stages of development when HWVAD was not
supported in first version and we only defined the API.
I will remove the annotation.

> > 
> > +static int fsl_micfil_hw_free(struct snd_pcm_substream *substream,
> > +			      struct snd_soc_dai *dai)
> > +{
> > +	struct fsl_micfil *micfil = snd_soc_dai_get_drvdata(dai);
> > +
> > +	atomic_set(&micfil->recording_state,
> > MICFIL_RECORDING_OFF);
> > +
> > +	return 0;
> > +}
> Are you *sure* you need to and want to use atomic_set() here and that
> there's no race conditions resulting from trying to use an atomic
> variable?  It's much simpler and clearer to use mutexes, if for some
> reason atomic variables make sense then the reasoning needs to be
> clearly documented as they're quite tricky and easy to break or
> misunderstand.
> 

We want to keep track of the recording state and the hwvad state since
recording and voice detection can work in parallel. The main reason why
we want to keep track is because the recording and hwvad share the same
clock and we should not touch the clock when one or another is on.
Another restriction is that we want to make sure we use the same rate
for recording and voice detection when doing it in parallel.
This was the only solution we found viable at that time and it worked
in any supported scenarios but we are open for suggestions if the
functionality is kept and code will have a better quality.

> > 
> > +static int fsl_micfil_set_dai_fmt(struct snd_soc_dai *dai,
> > unsigned int fmt)
> > +{
> > +	struct fsl_micfil *micfil = snd_soc_dai_get_drvdata(dai);
> > +
> > +	/* DAI MODE */
> > +	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> > +	case SND_SOC_DAIFMT_I2S:
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> Is this actually an I2S controller?  It looks like a PDM controller
> to
> me and that's what your cover letter said.  Just omit this entirely
> if
> the DAI format isn't configurable in the hardware.

Yes, this should be removed

> 
> > 
> > +	/* set default gain to max_gain */
> > +	regmap_write(micfil->regmap, REG_MICFIL_OUT_CTRL,
> > 0x77777777);
> > +	for (i = 0; i < 8; i++)
> > +		micfil->channel_gain[i] = 0xF;
> I'm assuming the hardware has no defaults here but if we've got to
> pick
> a gain wouldn't a low gain be less likely to blast out someone's
> eardrums than a maximum gain?
> 

Fair enough. We did this because the maximum output volume isn't that
high but I guess we should set it to minimum and user should set volume
via amixer.

> > 
> > +static irqreturn_t voice_detected_fn(int irq, void *devid)
> > +{
> > +	struct fsl_micfil *micfil = (struct fsl_micfil *)devid;
> > +	struct device *dev = &micfil->pdev->dev;
> > +	int ret;
> > +
> > +	/* disable hwvad */
> > +	ret = disable_hwvad(dev, true);
> > +	if (ret)
> > +		dev_err(dev, "Failed to disable HWVAD module\n");
> > +
> > +	/* notify userspace that voice was detected */
> > +	kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
> > +
> > +	return IRQ_HANDLED;
> > +}
> So, this looks like it's intended to be used for keyword detection
> type
> applications (though without the offload DSP that those tend to
> have).
> What the other implementations I've seen have ended up doing is using
> a
> compressed audio stream to return the audio data to userspace,
> allowing
> the audion stream to be paused when no audio is detected.  Your
> approach
> here is a bit more manual and may be more sensible for systems
> without
> the offload DSP however the decision to go outside ALSA and use a
> kobject needs to be thought about a bit, we'd want to ensure that
> there's a standard way of handling hardware like this so applications
> can work as consistently as possible with them.
> 
> It's probably best to split all this VAD handling code out into a
> separate patch so that the basic support can get merged and this can
> be
> reviewed separately.  The rest of the driver has some minor issues
> but
> it looks like all the complexity is in this VAD code.

I was also thinking to split the VAD from driver and sent it in a later
version since this hardware has two independent functionalities:
recording and voice detection.
We have chosen this approach because we can also support the recording
in parallel and it is user's choice how to handle the VOICE_DETECTED
event.
However, we are open for suggestions on how to handle the detection
event and how to notify the userspace. I am pretty sure that with the
current hardware it is hard to return the stream back to user since you
will only have access to samples stored in internal buffers, probably
less than 100 in the best case scenario.

If this is ok for you, we will drop the HWVAD changes for the moment
and send the next version only for recording functionality. We will
send a different patch when this gets accepted with voice detection
support.

> 
> > 
> > +static irqreturn_t micfil_err_isr(int irq, void *devid)
> > +{
> > +	struct fsl_micfil *micfil = (struct fsl_micfil *)devid;
> > +	struct platform_device *pdev = micfil->pdev;
> > +	u32 stat_reg;
> > +
> > +	regmap_read(micfil->regmap, REG_MICFIL_STAT, &stat_reg);
> > +
> > +	if (stat_reg & MICFIL_STAT_BSY_FIL_MASK)
> > +		dev_dbg(&pdev->dev, "isr: Decimation Filter is
> > running\n");
> > +
> > +	if (stat_reg & MICFIL_STAT_FIR_RDY_MASK)
> > +		dev_dbg(&pdev->dev, "isr: FIR Filter Data
> > ready\n");
> > +
> > +	if (stat_reg & MICFIL_STAT_LOWFREQF_MASK) {
> > +		dev_dbg(&pdev->dev, "isr: ipg_clk_app is too
> > low\n");
> > +		regmap_write_bits(micfil->regmap, REG_MICFIL_STAT,
> > +				  MICFIL_STAT_LOWFREQF_MASK, 1);
> > +	}
> > +
> > +	return IRQ_HANDLED;
> > +}
> This will uncondtionally report the interrupt as handled but if it
> sees
> an error it doesn't recognize it won't log anything - that seems not
> ideal, it'd be better to log the value we read in case there's
> something
> else goes wrong to aid debug.

Ok. Will do it.

> 
> > 
> > +static int enable_hwvad(struct device *dev, bool sync)
> > +{
> > +	struct fsl_micfil *micfil = dev_get_drvdata(dev);
> > +	int ret;
> > +	int rate;
> > +	u32 state;
> > +
> > +	if (sync)
> > +		pm_runtime_get_sync(dev);
> > +
> > +	state = atomic_cmpxchg(&micfil->hwvad_state,
> > +			       MICFIL_HWVAD_OFF,
> > +			       MICFIL_HWVAD_ON);
> This *really* needs some documentation about what's going on for
> concurrency here.

The hwvad_state is used as I have explained for recoding_state and it
also helps us to decide what happens with hwvad during suspend/resume.
At this moment, we have decided to disable it at suspend and re-enable
at resume but we micfil could also work as wake-up source (but it is
not implemented yet in driver).
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [RFC 2/2] ASoC: Add MICFIL SoC Digital Audio Interface driver.

[Cosmin Samoila]

Hello Mark

One question inline.

Thank you,
Cosmin

On Jo, 2018-12-13 at 12:20 +0200, Cosmin Samoila wrote:
> Hello Mark,
> 
> Thank you for the review.
> Please see my comments inline.
> 
> Best regards,
> Cosmin
> 
> On Mi, 2018-12-12 at 18:14 +0000, Mark Brown wrote:
> > 
> > On Mon, Dec 10, 2018 at 09:21:15AM +0000, Cosmin Samoila wrote:
> > 
> > > 
> > > 
> > > +static char *envp[] = {
> > > +		"EVENT=PDM_VOICE_DETECT",
> > > +		NULL,
> > > +	};
> > The indentation here is weird.
> Will fix it.
> 
> > 
> > 
> > > 
> > > 
> > > +static const char * const micfil_hwvad_zcd_enable[] = {
> > > +	"OFF", "ON",
> > > +};
> > Simple on/off switches should just be regular controls with
> > "Switch"
> > at
> > the end of their name so userspace can handle them.
> > 
> > > 
> > > 
> > > +static const char * const micfil_hwvad_noise_decimation[] = {
> > > +	"Disabled", "Enabled",
> > > +};
> > Same here.
> Will fix it.

What do you mean by "regular controls" with "Switch"?
I can add switch in the naming but we need to save the user
configuration when doing the initialization (i.e. having custom put &
get).

> > 
> > 
> > > 
> > > 
> > > +/* when adding new rate text, also add it to the
> > > + * micfil_hwvad_rate_ints
> > > + */
> > > +static const char * const micfil_hwvad_rate[] = {
> > > +	"48KHz", "44.1KHz",
> > > +};
> > > +
> > > +static const int micfil_hwvad_rate_ints[] = {
> > > +	48000, 44100,
> > > +};
> > Can the driver not determine this automatically from sample rates?
> I think I could add "48000", "44100" instead of "48KHz", "44.1KHz"
> and
> use kstrtol()
> 
> > 
> > 
> > > 
> > > 
> > > +static const char * const micfil_clk_src_texts[] = {
> > > +	"Auto", "AudioPLL1", "AudioPLL2", "ExtClk3",
> > > +};
> > Is this something that should be user selectable or is it going to
> > be
> > controlled by the board design?
> User should be able to select the clock source since, in theory,
> hardware could support any custom rate as long as you can provide the
> clock on extclk.
>  
> > 
> > 
> > > 
> > > 
> > > +static int hwvad_put_hpf(struct snd_kcontrol *kcontrol,
> > > +			 struct snd_ctl_elem_value *ucontrol)
> > > +{
> > > +	struct snd_soc_component *comp =
> > > snd_kcontrol_chip(kcontrol);
> > > +	struct soc_enum *e = (struct soc_enum *)kcontrol-
> > > > 
> > > > private_value;
> > > +	unsigned int *item = ucontrol->value.enumerated.item;
> > > +	struct fsl_micfil *micfil =
> > > snd_soc_component_get_drvdata(comp);
> > > +	int val = snd_soc_enum_item_to_val(e, item[0]);
> > > +
> > > +	/* 00 - HPF Bypass
> > > +	 * 01 - Cut-off frequency 1750Hz
> > > +	 * 10 - Cut-off frequency 215Hz
> > > +	 * 11 - Cut-off frequency 102Hz
> > > +	 */
> > > +	micfil->vad_hpf = val;
> > > +
> > > +	return 0;
> > > +}
> > What happens if this gets changed while a stream is active - the
> > user
> > will think they changed the configuration but it won't take effect
> > until
> > the next stream is started AFAICT?
> If the stream is active, the configuration will indeed take efect on
> the next stream but this is the desired behavior. If we would want to
> do the config on the fly, we should use sync functions that also
> resets
> the pdm module which is not what we intend.
> User must first configure the module then start the recording since
> this seems to be the natural flow.
> 
> > 
> > 
> > > 
> > > 
> > > +	/* a value remapping must be done since the gain field
> > > have
> > > +	 * the following meaning:
> > > +	 * * 0 : no gain
> > > +	 * * 1 - 7 : +1 to +7 bits gain
> > > +	 * * 8 - 15 : -8 to -1 bits gain
> > > +	 * After the remapp, the scale should start from -8 to
> > > +7
> > > +	 */
> > This looks like a signed value so one of the _S_VALUE macros should
> > handle things I think?
> > 
> Ok.
> 
> > 
> > > 
> > > 
> > > +static const struct snd_kcontrol_new fsl_micfil_snd_controls[] =
> > > {
> > > +	SOC_SINGLE_RANGE_EXT_TLV("CH0 Gain", -1,
> > > MICFIL_OUTGAIN_CHX_SHIFT(0),
> > > +				 0x0, 0xF, 0,
> > > +				 get_channel_gain,
> > > put_channel_gain, gain_tlv),
> > All volume controls should have names ending in Volume so userspace
> > knows how to handle them.
> Ok, I will change the name.
> 
> > 
> > 
> > > 
> > > 
> > > +/* Hardware Voice Active Detection: The HWVAD takes data from
> > > the
> > > input
> > > + * of a selected PDM microphone to detect if there is any
> > > + * voice activity. When a voice activity is detected, an
> > > interrupt
> > > could
> > > + * be delivered to the system. Initialization in section 8.4:
> > > + * Can work in two modes:
> > > + *  -> Eneveope-based mode (section 8.4.1)
> > > + *  -> Energy-based mode (section 8.4.2)
> > > + *
> > > + * It is important to remark that the HWVAD detector could be
> > > enabled
> > > + * or reset only when the MICFIL isn't running i.e. when the
> > > BSY_FIL
> > > + * bit in STAT register is cleared
> > > + */
> > > +static int __maybe_unused init_hwvad(struct device *dev)
> > Why is this annotated __maybey_unused?
> > 
> It remained from early stages of development when HWVAD was not
> supported in first version and we only defined the API.
> I will remove the annotation.
> 
> > 
> > > 
> > > 
> > > +static int fsl_micfil_hw_free(struct snd_pcm_substream
> > > *substream,
> > > +			      struct snd_soc_dai *dai)
> > > +{
> > > +	struct fsl_micfil *micfil =
> > > snd_soc_dai_get_drvdata(dai);
> > > +
> > > +	atomic_set(&micfil->recording_state,
> > > MICFIL_RECORDING_OFF);
> > > +
> > > +	return 0;
> > > +}
> > Are you *sure* you need to and want to use atomic_set() here and
> > that
> > there's no race conditions resulting from trying to use an atomic
> > variable?  It's much simpler and clearer to use mutexes, if for
> > some
> > reason atomic variables make sense then the reasoning needs to be
> > clearly documented as they're quite tricky and easy to break or
> > misunderstand.
> > 
> We want to keep track of the recording state and the hwvad state
> since
> recording and voice detection can work in parallel. The main reason
> why
> we want to keep track is because the recording and hwvad share the
> same
> clock and we should not touch the clock when one or another is on.
> Another restriction is that we want to make sure we use the same rate
> for recording and voice detection when doing it in parallel.
> This was the only solution we found viable at that time and it worked
> in any supported scenarios but we are open for suggestions if the
> functionality is kept and code will have a better quality.
> 
> > 
> > > 
> > > 
> > > +static int fsl_micfil_set_dai_fmt(struct snd_soc_dai *dai,
> > > unsigned int fmt)
> > > +{
> > > +	struct fsl_micfil *micfil =
> > > snd_soc_dai_get_drvdata(dai);
> > > +
> > > +	/* DAI MODE */
> > > +	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> > > +	case SND_SOC_DAIFMT_I2S:
> > > +		break;
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > Is this actually an I2S controller?  It looks like a PDM controller
> > to
> > me and that's what your cover letter said.  Just omit this entirely
> > if
> > the DAI format isn't configurable in the hardware.
> Yes, this should be removed
> 
> > 
> > 
> > > 
> > > 
> > > +	/* set default gain to max_gain */
> > > +	regmap_write(micfil->regmap, REG_MICFIL_OUT_CTRL,
> > > 0x77777777);
> > > +	for (i = 0; i < 8; i++)
> > > +		micfil->channel_gain[i] = 0xF;
> > I'm assuming the hardware has no defaults here but if we've got to
> > pick
> > a gain wouldn't a low gain be less likely to blast out someone's
> > eardrums than a maximum gain?
> > 
> Fair enough. We did this because the maximum output volume isn't that
> high but I guess we should set it to minimum and user should set
> volume
> via amixer.
> 
> > 
> > > 
> > > 
> > > +static irqreturn_t voice_detected_fn(int irq, void *devid)
> > > +{
> > > +	struct fsl_micfil *micfil = (struct fsl_micfil *)devid;
> > > +	struct device *dev = &micfil->pdev->dev;
> > > +	int ret;
> > > +
> > > +	/* disable hwvad */
> > > +	ret = disable_hwvad(dev, true);
> > > +	if (ret)
> > > +		dev_err(dev, "Failed to disable HWVAD
> > > module\n");
> > > +
> > > +	/* notify userspace that voice was detected */
> > > +	kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
> > > +
> > > +	return IRQ_HANDLED;
> > > +}
> > So, this looks like it's intended to be used for keyword detection
> > type
> > applications (though without the offload DSP that those tend to
> > have).
> > What the other implementations I've seen have ended up doing is
> > using
> > a
> > compressed audio stream to return the audio data to userspace,
> > allowing
> > the audion stream to be paused when no audio is detected.  Your
> > approach
> > here is a bit more manual and may be more sensible for systems
> > without
> > the offload DSP however the decision to go outside ALSA and use a
> > kobject needs to be thought about a bit, we'd want to ensure that
> > there's a standard way of handling hardware like this so
> > applications
> > can work as consistently as possible with them.
> > 
> > It's probably best to split all this VAD handling code out into a
> > separate patch so that the basic support can get merged and this
> > can
> > be
> > reviewed separately.  The rest of the driver has some minor issues
> > but
> > it looks like all the complexity is in this VAD code.
> I was also thinking to split the VAD from driver and sent it in a
> later
> version since this hardware has two independent functionalities:
> recording and voice detection.
> We have chosen this approach because we can also support the
> recording
> in parallel and it is user's choice how to handle the VOICE_DETECTED
> event.
> However, we are open for suggestions on how to handle the detection
> event and how to notify the userspace. I am pretty sure that with the
> current hardware it is hard to return the stream back to user since
> you
> will only have access to samples stored in internal buffers, probably
> less than 100 in the best case scenario.
> 
> If this is ok for you, we will drop the HWVAD changes for the moment
> and send the next version only for recording functionality. We will
> send a different patch when this gets accepted with voice detection
> support.
> 
> > 
> > 
> > > 
> > > 
> > > +static irqreturn_t micfil_err_isr(int irq, void *devid)
> > > +{
> > > +	struct fsl_micfil *micfil = (struct fsl_micfil *)devid;
> > > +	struct platform_device *pdev = micfil->pdev;
> > > +	u32 stat_reg;
> > > +
> > > +	regmap_read(micfil->regmap, REG_MICFIL_STAT, &stat_reg);
> > > +
> > > +	if (stat_reg & MICFIL_STAT_BSY_FIL_MASK)
> > > +		dev_dbg(&pdev->dev, "isr: Decimation Filter is
> > > running\n");
> > > +
> > > +	if (stat_reg & MICFIL_STAT_FIR_RDY_MASK)
> > > +		dev_dbg(&pdev->dev, "isr: FIR Filter Data
> > > ready\n");
> > > +
> > > +	if (stat_reg & MICFIL_STAT_LOWFREQF_MASK) {
> > > +		dev_dbg(&pdev->dev, "isr: ipg_clk_app is too
> > > low\n");
> > > +		regmap_write_bits(micfil->regmap,
> > > REG_MICFIL_STAT,
> > > +				  MICFIL_STAT_LOWFREQF_MASK, 1);
> > > +	}
> > > +
> > > +	return IRQ_HANDLED;
> > > +}
> > This will uncondtionally report the interrupt as handled but if it
> > sees
> > an error it doesn't recognize it won't log anything - that seems
> > not
> > ideal, it'd be better to log the value we read in case there's
> > something
> > else goes wrong to aid debug.
> Ok. Will do it.
> 
> > 
> > 
> > > 
> > > 
> > > +static int enable_hwvad(struct device *dev, bool sync)
> > > +{
> > > +	struct fsl_micfil *micfil = dev_get_drvdata(dev);
> > > +	int ret;
> > > +	int rate;
> > > +	u32 state;
> > > +
> > > +	if (sync)
> > > +		pm_runtime_get_sync(dev);
> > > +
> > > +	state = atomic_cmpxchg(&micfil->hwvad_state,
> > > +			       MICFIL_HWVAD_OFF,
> > > +			       MICFIL_HWVAD_ON);
> > This *really* needs some documentation about what's going on for
> > concurrency here.
> The hwvad_state is used as I have explained for recoding_state and it
> also helps us to decide what happens with hwvad during
> suspend/resume.
> At this moment, we have decided to disable it at suspend and re-
> enable
> at resume but we micfil could also work as wake-up source (but it is
> not implemented yet in driver).
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [RFC 2/2] ASoC: Add MICFIL SoC Digital Audio Interface driver.

[Mark Brown]

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

On Thu, Dec 13, 2018 at 01:57:56PM +0000, Cosmin Samoila wrote:

> > > > +	"Disabled", "Enabled",
> > > > +};
> > > Same here.

> > Will fix it.

> What do you mean by "regular controls" with "Switch"?
> I can add switch in the naming but we need to save the user
> configuration when doing the initialization (i.e. having custom put &
> get).

Not an enum, a normal number based control.

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

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

Re: [RFC 2/2] ASoC: Add MICFIL SoC Digital Audio Interface driver.

[Mark Brown]

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

On Thu, Dec 13, 2018 at 10:20:47AM +0000, Cosmin Samoila wrote:
> On Mi, 2018-12-12 at 18:14 +0000, Mark Brown wrote:

> > > +static const char * const micfil_hwvad_rate[] = {
> > > +	"48KHz", "44.1KHz",
> > > +};

> > > +static const int micfil_hwvad_rate_ints[] = {
> > > +	48000, 44100,
> > > +};
> > Can the driver not determine this automatically from sample rates?

> I think I could add "48000", "44100" instead of "48KHz", "44.1KHz" and
> use kstrtol()

No, you're missing the point - why not set the sample rate based on the
sample rate the interface is running at?

> > > +static const char * const micfil_clk_src_texts[] = {
> > > +	"Auto", "AudioPLL1", "AudioPLL2", "ExtClk3",
> > > +};

> > Is this something that should be user selectable or is it going to be
> > controlled by the board design?

> User should be able to select the clock source since, in theory,
> hardware could support any custom rate as long as you can provide the
> clock on extclk.

What I'm saying is that this should not be selectable by the user at
runtime.  It's not like the user is going to open their system and start
soldering links onto the board or anything.

> > What happens if this gets changed while a stream is active - the user
> > will think they changed the configuration but it won't take effect
> > until
> > the next stream is started AFAICT?

> If the stream is active, the configuration will indeed take efect on
> the next stream but this is the desired behavior. If we would want to
> do the config on the fly, we should use sync functions that also resets
> the pdm module which is not what we intend.
> User must first configure the module then start the recording since
> this seems to be the natural flow.

If the user can't reconfigure the stream while it's running the user
shouldn't be able to reconfigure the stream while it's running - we
should block the change if it can't be implemented.  Then the user can
make the change while the interface is idle and and 

> > Are you *sure* you need to and want to use atomic_set() here and that
> > there's no race conditions resulting from trying to use an atomic
> > variable?  It's much simpler and clearer to use mutexes, if for some
> > reason atomic variables make sense then the reasoning needs to be
> > clearly documented as they're quite tricky and easy to break or
> > misunderstand.

> We want to keep track of the recording state and the hwvad state since
> recording and voice detection can work in parallel. The main reason why
> we want to keep track is because the recording and hwvad share the same
> clock and we should not touch the clock when one or another is on.
> Another restriction is that we want to make sure we use the same rate
> for recording and voice detection when doing it in parallel.
> This was the only solution we found viable at that time and it worked
> in any supported scenarios but we are open for suggestions if the
> functionality is kept and code will have a better quality.

This explains what the data is but not why you have chosen to use
atomics to do this; what other concurrency primitives were considered,
why were they rejected and what's the analysis that shows how the use of
atomics is safe?

> > > +	/* set default gain to max_gain */
> > > +	regmap_write(micfil->regmap, REG_MICFIL_OUT_CTRL,
> > > 0x77777777);
> > > +	for (i = 0; i < 8; i++)
> > > +		micfil->channel_gain[i] = 0xF;

> > I'm assuming the hardware has no defaults here but if we've got to
> > pick
> > a gain wouldn't a low gain be less likely to blast out someone's
> > eardrums than a maximum gain?

> Fair enough. We did this because the maximum output volume isn't that
> high but I guess we should set it to minimum and user should set volume
> via amixer.

The ideal thing is to just not set it at all and use whatever the
hardware designers picked.

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

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

Re: [RFC 2/2] ASoC: Add MICFIL SoC Digital Audio Interface driver.

[Daniel Baluta]

On Thu, Dec 13, 2018 at 4:31 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Dec 13, 2018 at 10:20:47AM +0000, Cosmin Samoila wrote:
> > On Mi, 2018-12-12 at 18:14 +0000, Mark Brown wrote:
>
> > > > +static const char * const micfil_hwvad_rate[] = {
> > > > + "48KHz", "44.1KHz",
> > > > +};
>
> > > > +static const int micfil_hwvad_rate_ints[] = {
> > > > + 48000, 44100,
> > > > +};
> > > Can the driver not determine this automatically from sample rates?
>
> > I think I could add "48000", "44100" instead of "48KHz", "44.1KHz" and
> > use kstrtol()
>
> No, you're missing the point - why not set the sample rate based on the
> sample rate the interface is running at?

Hmm, definitely we somehow miss the point here. So what happens
if we only do Voice Activity Detection (VAD) ?

This means that there is no sample rate yet configured, so for this reason
we get the sampling rate as above via ALSA kcontrol interface.

My feeling is that somehow we will need to create maybe another type
of stream (VAD?).

This is a thing that we need to think. Suggestions are welcomed!

>
> > > > +static const char * const micfil_clk_src_texts[] = {
> > > > + "Auto", "AudioPLL1", "AudioPLL2", "ExtClk3",
> > > > +};
>
> > > Is this something that should be user selectable or is it going to be
> > > controlled by the board design?
>
> > User should be able to select the clock source since, in theory,
> > hardware could support any custom rate as long as you can provide the
> > clock on extclk.
>
> What I'm saying is that this should not be selectable by the user at
> runtime.  It's not like the user is going to open their system and start
> soldering links onto the board or anything.

Of course, so would this be a better option to select it from dts?

>
> > > What happens if this gets changed while a stream is active - the user
> > > will think they changed the configuration but it won't take effect
> > > until
> > > the next stream is started AFAICT?
>
> > If the stream is active, the configuration will indeed take efect on
> > the next stream but this is the desired behavior. If we would want to
> > do the config on the fly, we should use sync functions that also resets
> > the pdm module which is not what we intend.
> > User must first configure the module then start the recording since
> > this seems to be the natural flow.
>
> If the user can't reconfigure the stream while it's running the user
> shouldn't be able to reconfigure the stream while it's running - we
> should block the change if it can't be implemented.  Then the user can
> make the change while the interface is idle and and
>
> > > Are you *sure* you need to and want to use atomic_set() here and that
> > > there's no race conditions resulting from trying to use an atomic
> > > variable?  It's much simpler and clearer to use mutexes, if for some
> > > reason atomic variables make sense then the reasoning needs to be
> > > clearly documented as they're quite tricky and easy to break or
> > > misunderstand.

We can go back to mutexes no problem, but we thought that atomic vars
are lighter.

>
> > We want to keep track of the recording state and the hwvad state since
> > recording and voice detection can work in parallel. The main reason why
> > we want to keep track is because the recording and hwvad share the same
> > clock and we should not touch the clock when one or another is on.
> > Another restriction is that we want to make sure we use the same rate
> > for recording and voice detection when doing it in parallel.
> > This was the only solution we found viable at that time and it worked
> > in any supported scenarios but we are open for suggestions if the
> > functionality is kept and code will have a better quality.
>
> This explains what the data is but not why you have chosen to use
> atomics to do this; what other concurrency primitives were considered,
> why were they rejected and what's the analysis that shows how the use of
> atomics is safe?

We first used mutexes but I suggested switching to atomic vars because
AFAIK they are lighter than mutexes.

Re: [RFC 2/2] ASoC: Add MICFIL SoC Digital Audio Interface driver.

[Mark Brown]

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

On Fri, Dec 14, 2018 at 04:54:13PM +0200, Daniel Baluta wrote:
> On Thu, Dec 13, 2018 at 4:31 PM Mark Brown <broonie@kernel.org> wrote:

> > No, you're missing the point - why not set the sample rate based on the
> > sample rate the interface is running at?

> Hmm, definitely we somehow miss the point here. So what happens
> if we only do Voice Activity Detection (VAD) ?

> This means that there is no sample rate yet configured, so for this reason
> we get the sampling rate as above via ALSA kcontrol interface.

> My feeling is that somehow we will need to create maybe another type
> of stream (VAD?).

> This is a thing that we need to think. Suggestions are welcomed!

This is really easy if you do what other people have done and provide
the detected voice as a compressed stream.  On the other hand if there's
no stream and people just pick the audio out of the normal record path
is there any great reason why people would configure this at all.

> > > > Is this something that should be user selectable or is it going to be
> > > > controlled by the board design?
> >
> > > User should be able to select the clock source since, in theory,
> > > hardware could support any custom rate as long as you can provide the
> > > clock on extclk.
> >
> > What I'm saying is that this should not be selectable by the user at
> > runtime.  It's not like the user is going to open their system and start
> > soldering links onto the board or anything.
> 
> Of course, so would this be a better option to select it from dts?

Yes, or the machine driver.

> > > > Are you *sure* you need to and want to use atomic_set() here and that
> > > > there's no race conditions resulting from trying to use an atomic
> > > > variable?  It's much simpler and clearer to use mutexes, if for some
> > > > reason atomic variables make sense then the reasoning needs to be
> > > > clearly documented as they're quite tricky and easy to break or
> > > > misunderstand.

> We can go back to mutexes no problem, but we thought that atomic vars
> are lighter.

They do less than you might think unfortunately and are more complicated
to reason about so unless you're getting a useful performance gain from
them they're rarely worth the effort.

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

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

Re: [RFC 2/2] ASoC: Add MICFIL SoC Digital Audio Interface driver.

[Pierre-Louis Bossart]

On 12/12/18 12:14 PM, Mark Brown wrote:
>> +static irqreturn_t voice_detected_fn(int irq, void *devid)
>> +{
>> +	struct fsl_micfil *micfil = (struct fsl_micfil *)devid;
>> +	struct device *dev = &micfil->pdev->dev;
>> +	int ret;
>> +
>> +	/* disable hwvad */
>> +	ret = disable_hwvad(dev, true);
>> +	if (ret)
>> +		dev_err(dev, "Failed to disable HWVAD module\n");
>> +
>> +	/* notify userspace that voice was detected */
>> +	kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
>> +
>> +	return IRQ_HANDLED;
>> +}
> So, this looks like it's intended to be used for keyword detection type
> applications (though without the offload DSP that those tend to have).
> What the other implementations I've seen have ended up doing is using a
> compressed audio stream to return the audio data to userspace, allowing
> the audion stream to be paused when no audio is detected.  Your approach
> here is a bit more manual and may be more sensible for systems without
> the offload DSP however the decision to go outside ALSA and use a
> kobject needs to be thought about a bit, we'd want to ensure that
> there's a standard way of handling hardware like this so applications
> can work as consistently as possible with them.

There's no mention of any buffer so it's likely plain vanilla VAD here. 
We've seen two choices to warn userspace of a acoustic event, either use 
a uevent or a kcontrol. I believe we ended-up chosing the latter on the 
Intel side in the past and that was also the plan for SOF.

In terms of configurations for the PDM we have completely different 
settings since we pass explicit coefficients while this solution passes 
qualifiers (cut-off, quality, etc), not sure if there is any point in 
trying to unify those parameters.

Re: [RFC 2/2] ASoC: Add MICFIL SoC Digital Audio Interface driver.

[Mark Brown]

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

On Fri, Dec 14, 2018 at 02:09:00PM -0600, Pierre-Louis Bossart wrote:

> There's no mention of any buffer so it's likely plain vanilla VAD here.
> We've seen two choices to warn userspace of a acoustic event, either use a
> uevent or a kcontrol. I believe we ended-up chosing the latter on the Intel
> side in the past and that was also the plan for SOF.

It sounded like there were some separate audio streams somewhere along
the line, it wasn't super clear if that just went into a DSP that did
the VAD and wasn't visible to the rest of the system or if it's
something the system can see.  Generally we have gone for kcontrols for
event signalling.

> In terms of configurations for the PDM we have completely different settings
> since we pass explicit coefficients while this solution passes qualifiers
> (cut-off, quality, etc), not sure if there is any point in trying to unify
> those parameters.

Yes, I don't think the specific parameters are going to be joined up any
time soon - it's more the overall shape of the solution.

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

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

Re: [RFC 2/2] ASoC: Add MICFIL SoC Digital Audio Interface driver.

[Daniel Baluta]

On Mon, Dec 17, 2018 at 2:19 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Dec 14, 2018 at 02:09:00PM -0600, Pierre-Louis Bossart wrote:
>
> > There's no mention of any buffer so it's likely plain vanilla VAD here.
> > We've seen two choices to warn userspace of a acoustic event, either use a
> > uevent or a kcontrol. I believe we ended-up chosing the latter on the Intel
> > side in the past and that was also the plan for SOF.
>
> It sounded like there were some separate audio streams somewhere along
> the line, it wasn't super clear if that just went into a DSP that did
> the VAD and wasn't visible to the rest of the system or if it's
> something the system can see.  Generally we have gone for kcontrols for
> event signalling.

Yes, it is plain vanilla VAD here. The MICFIL hardware IP sends an interrupt
when voice is detected.

Can you point us to some code that uses kcontrol interface to signal an event?
This might sound silly because I know there are a lot of places where kcontrol
interface is used but I  just want make sure that we are on the same page.
>
> > In terms of configurations for the PDM we have completely different settings
> > since we pass explicit coefficients while this solution passes qualifiers
> > (cut-off, quality, etc), not sure if there is any point in trying to unify
> > those parameters.
>
> Yes, I don't think the specific parameters are going to be joined up any
> time soon - it's more the overall shape of the solution.

Re: [RFC 2/2] ASoC: Add MICFIL SoC Digital Audio Interface driver.

[Daniel Baluta]

On Fri, Dec 14, 2018 at 10:09 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
> On 12/12/18 12:14 PM, Mark Brown wrote:
> >> +static irqreturn_t voice_detected_fn(int irq, void *devid)
> >> +{
> >> +    struct fsl_micfil *micfil = (struct fsl_micfil *)devid;
> >> +    struct device *dev = &micfil->pdev->dev;
> >> +    int ret;
> >> +
> >> +    /* disable hwvad */
> >> +    ret = disable_hwvad(dev, true);
> >> +    if (ret)
> >> +            dev_err(dev, "Failed to disable HWVAD module\n");
> >> +
> >> +    /* notify userspace that voice was detected */
> >> +    kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
> >> +
> >> +    return IRQ_HANDLED;
> >> +}
> > So, this looks like it's intended to be used for keyword detection type
> > applications (though without the offload DSP that those tend to have).
> > What the other implementations I've seen have ended up doing is using a
> > compressed audio stream to return the audio data to userspace, allowing
> > the audion stream to be paused when no audio is detected.  Your approach
> > here is a bit more manual and may be more sensible for systems without
> > the offload DSP however the decision to go outside ALSA and use a
> > kobject needs to be thought about a bit, we'd want to ensure that
> > there's a standard way of handling hardware like this so applications
> > can work as consistently as possible with them.
>
> There's no mention of any buffer so it's likely plain vanilla VAD here.
> We've seen two choices to warn userspace of a acoustic event, either use
> a uevent or a kcontrol. I believe we ended-up chosing the latter on the
> Intel side in the past and that was also the plan for SOF.

Reviving this discussion about VAD. Do you have any draft patches for
the interface to send events to user space?

We are now trying to upstream VAD for PDM and we are looking
at all our options.

thanks,
Daniel.