[PATCH v3 0/6] Add driver for SAMA7G5's PDMC

[PATCH v3 0/6] Add driver for SAMA7G5's PDMC

[Codrin Ciubotariu]

This patch series adds support for Pulse Density Microphone Controller
(PDMC), present on Microchip's SAMA7G5.
The PDMC interfaces up to 4 digital microphones having Pulse Density
Modulated (PDM) outputs. It generates a single clock line and samples 1 or
2 data lines. The signal path includes an audio grade programmable
decimation filter and outputs 24-bit audio words.
The source of each channel can be independently defined as PDMC_DS0 or
PDMC_DS1, sampled at the rising or falling edge of PDMC_CLK.

The patch series starts with a fix on the ASoC DMA engine support. Then
continues with the bindings and the driver of PDMC. It is followed by the
DT nodes for SAMA7G5 and SAMA7G5-EK. In the end, the drivers for PDMC
and PDM microphones are enabled in sama7_defconfig.

Changes in v3:
 - addressed new DT bindings comments from Krzysztof Kozlowski

Changes in v2:
 - addressed DT bindings comments from Krzysztof Kozlowski

Codrin Ciubotariu (6):
  ASoC: dmaengine: do not use a NULL prepare_slave_config() callback
  ASoC: dt-bindings: Document Microchip's PDMC
  ASoC: atmel: mchp-pdmc: add PDMC driver
  ARM: dts: at91: sama7g5: add nodes for PDMC
  ARM: dts: at91: sama7g5ek: add node for PDMC0
  ARM: configs: at91: sama7_defconfig: add MCHP PDMC and DMIC drivers

 .../bindings/sound/microchip,pdmc.yaml        |  100 ++
 arch/arm/boot/dts/at91-sama7g5ek.dts          |   21 +-
 arch/arm/boot/dts/sama7g5.dtsi                |   24 +
 arch/arm/configs/sama7_defconfig              |    2 +
 include/dt-bindings/sound/microchip,pdmc.h    |   13 +
 sound/soc/atmel/Kconfig                       |   16 +
 sound/soc/atmel/Makefile                      |    2 +
 sound/soc/atmel/mchp-pdmc.c                   | 1084 +++++++++++++++++
 sound/soc/soc-generic-dmaengine-pcm.c         |    6 +-
 9 files changed, 1264 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/microchip,pdmc.yaml
 create mode 100644 include/dt-bindings/sound/microchip,pdmc.h
 create mode 100644 sound/soc/atmel/mchp-pdmc.c

-- 
2.32.0

[PATCH v3 1/6] ASoC: dmaengine: do not use a NULL prepare_slave_config() callback

[Codrin Ciubotariu]

Even if struct snd_dmaengine_pcm_config is used, prepare_slave_config()
callback might not be set. Check if this callback is set before using it.

Fixes: fa654e085300 ("ASoC: dmaengine-pcm: Provide default config")
Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
---

Changes in v2,v3:
 - none

 sound/soc/soc-generic-dmaengine-pcm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
index 285441d6aeed..2ab2ddc1294d 100644
--- a/sound/soc/soc-generic-dmaengine-pcm.c
+++ b/sound/soc/soc-generic-dmaengine-pcm.c
@@ -86,10 +86,10 @@ static int dmaengine_pcm_hw_params(struct snd_soc_component *component,
 
 	memset(&slave_config, 0, sizeof(slave_config));
 
-	if (!pcm->config)
-		prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config;
-	else
+	if (pcm->config && pcm->config->prepare_slave_config)
 		prepare_slave_config = pcm->config->prepare_slave_config;
+	else
+		prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config;
 
 	if (prepare_slave_config) {
 		int ret = prepare_slave_config(substream, params, &slave_config);
-- 
2.32.0

[PATCH v3 2/6] ASoC: dt-bindings: Document Microchip's PDMC

[Codrin Ciubotariu]

Add DT bindings for the new Microchip PDMC embedded in sama7g5 SoCs.

Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
---

Changes in v3:
 - set line length to 80 characters long
 - set 'reg' as the second property

Changes in v2:
 - renamed patch from 'ASoC: add DT bindings for Microchip PDMC' to
   'ASoC: dt-bindings: Document Microchip's PDMC';
 - renamed yaml file from 'mchp,pdmc.yaml' to 'microchip,pdmc.yaml';
 - used imperative mode in commit description;
 - renamed mchp,pdmc.h to microchip,pdmc.h;
 - fixed 'title' to represent HW;
 - made 'compatible' first property;
 - s/microhpone/microphone
 - none name in example set to 'sound'

 .../bindings/sound/microchip,pdmc.yaml        | 100 ++++++++++++++++++
 include/dt-bindings/sound/microchip,pdmc.h    |  13 +++
 2 files changed, 113 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/microchip,pdmc.yaml
 create mode 100644 include/dt-bindings/sound/microchip,pdmc.h

diff --git a/Documentation/devicetree/bindings/sound/microchip,pdmc.yaml b/Documentation/devicetree/bindings/sound/microchip,pdmc.yaml
new file mode 100644
index 000000000000..04414eb4ada9
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/microchip,pdmc.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/microchip,pdmc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip Pulse Density Microphone Controller
+
+maintainers:
+  - Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
+
+description:
+  The Microchip Pulse Density Microphone Controller (PDMC) interfaces up to 4
+  digital microphones having Pulse Density Modulated (PDM) outputs.
+
+properties:
+  compatible:
+    const: microchip,sama7g5-pdmc
+
+  reg:
+    maxItems: 1
+
+  "#sound-dai-cells":
+    const: 0
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Peripheral Bus Clock
+      - description: Generic Clock
+
+  clock-names:
+    items:
+      - const: pclk
+      - const: gclk
+
+  dmas:
+    description: RX DMA Channel
+    maxItems: 1
+
+  dma-names:
+    const: rx
+
+  microchip,mic-pos:
+    description: |
+      Position of PDM microphones on the DS line and the sampling edge (rising
+      or falling) of the CLK line. A microphone is represented as a pair of DS
+      line and the sampling edge. The first microphone is mapped to channel 0,
+      the second to channel 1, etc.
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    items:
+      items:
+        - description: value for DS line
+        - description: value for sampling edge
+      anyOf:
+        - enum:
+            - [0, 0]
+            - [0, 1]
+            - [1, 0]
+            - [1, 1]
+    minItems: 1
+    maxItems: 4
+    uniqueItems: true
+
+required:
+  - compatible
+  - reg
+  - "#sound-dai-cells"
+  - interrupts
+  - clocks
+  - clock-names
+  - dmas
+  - dma-names
+  - microchip,mic-pos
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/at91.h>
+    #include <dt-bindings/dma/at91.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/sound/microchip,pdmc.h>
+
+    pdmc: sound@e1608000 {
+        compatible = "microchip,sama7g5-pdmc";
+        reg = <0xe1608000 0x4000>;
+        #sound-dai-cells = <0>;
+        interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
+        dmas = <&dma0 AT91_XDMAC_DT_PERID(37)>;
+        dma-names = "rx";
+        clocks = <&pmc PMC_TYPE_PERIPHERAL 68>, <&pmc PMC_TYPE_GCK 68>;
+        clock-names = "pclk", "gclk";
+        microchip,mic-pos = <MCHP_PDMC_DS0 MCHP_PDMC_CLK_POSITIVE>,
+                            <MCHP_PDMC_DS0 MCHP_PDMC_CLK_NEGATIVE>,
+                            <MCHP_PDMC_DS1 MCHP_PDMC_CLK_POSITIVE>,
+                            <MCHP_PDMC_DS1 MCHP_PDMC_CLK_NEGATIVE>;
+    };
diff --git a/include/dt-bindings/sound/microchip,pdmc.h b/include/dt-bindings/sound/microchip,pdmc.h
new file mode 100644
index 000000000000..96cde94ce74f
--- /dev/null
+++ b/include/dt-bindings/sound/microchip,pdmc.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __DT_BINDINGS_MICROCHIP_PDMC_H__
+#define __DT_BINDINGS_MICROCHIP_PDMC_H__
+
+/* PDM microphone's pin placement */
+#define MCHP_PDMC_DS0 0
+#define MCHP_PDMC_DS1 1
+
+/* PDM microphone clock edge sampling */
+#define MCHP_PDMC_CLK_POSITIVE 0
+#define MCHP_PDMC_CLK_NEGATIVE 1
+
+#endif /* __DT_BINDINGS_MICROCHIP_PDMC_H__ */
-- 
2.32.0

[PATCH v3 3/6] ASoC: atmel: mchp-pdmc: add PDMC driver

[Codrin Ciubotariu]

The Pulse Density Microphone Controller (PDMC) interfaces up to 4 digital
microphones having Pulse Density Modulated (PDM) outputs. It generates a
single clock line and samples 1 or 2 data lines. The signal path includes
an audio grade programmable decimation filter and outputs 24-bit audio
words on the APB bus.

Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
---

Changes in v3:
 - none

Changes in v2:
 - replaced included file dt-bindings/sound/mchp,pdmc.h wih
   dt-bindings/sound/microchip,pdmc.h

 sound/soc/atmel/Kconfig     |   16 +
 sound/soc/atmel/Makefile    |    2 +
 sound/soc/atmel/mchp-pdmc.c | 1084 +++++++++++++++++++++++++++++++++++
 3 files changed, 1102 insertions(+)
 create mode 100644 sound/soc/atmel/mchp-pdmc.c

diff --git a/sound/soc/atmel/Kconfig b/sound/soc/atmel/Kconfig
index 8617793ed955..795c0b0b527a 100644
--- a/sound/soc/atmel/Kconfig
+++ b/sound/soc/atmel/Kconfig
@@ -160,4 +160,20 @@ config SND_MCHP_SOC_SPDIFRX
 
 	  This S/PDIF RX driver is compliant with IEC-60958 standard and
 	  includes programmable User Data and Channel Status fields.
+
+config SND_MCHP_SOC_PDMC
+	tristate "Microchip ASoC driver for boards using PDMC"
+	depends on OF && (ARCH_AT91 || COMPILE_TEST)
+	select SND_SOC_GENERIC_DMAENGINE_PCM
+	select REGMAP_MMIO
+	help
+	  Say Y or M if you want to add support for Microchip ASoC PDMC driver on the
+	  following Microchip platforms:
+	  - sama7g5
+
+	  The Pulse Density Microphone Controller (PDMC) interfaces up to 4 digital
+	  microphones PDM outputs. It generates a single clock line and samples 1 or
+	  2 data lines. The signal path includes an audio grade programmable
+	  decimation filter and outputs 24-bit audio words.
+
 endif
diff --git a/sound/soc/atmel/Makefile b/sound/soc/atmel/Makefile
index 016188397210..043097a08ea8 100644
--- a/sound/soc/atmel/Makefile
+++ b/sound/soc/atmel/Makefile
@@ -7,6 +7,7 @@ snd-soc-atmel-i2s-objs := atmel-i2s.o
 snd-soc-mchp-i2s-mcc-objs := mchp-i2s-mcc.o
 snd-soc-mchp-spdiftx-objs := mchp-spdiftx.o
 snd-soc-mchp-spdifrx-objs := mchp-spdifrx.o
+snd-soc-mchp-pdmc-objs := mchp-pdmc.o
 
 # pdc and dma need to both be built-in if any user of
 # ssc is built-in.
@@ -21,6 +22,7 @@ obj-$(CONFIG_SND_ATMEL_SOC_I2S) += snd-soc-atmel-i2s.o
 obj-$(CONFIG_SND_MCHP_SOC_I2S_MCC) += snd-soc-mchp-i2s-mcc.o
 obj-$(CONFIG_SND_MCHP_SOC_SPDIFTX) += snd-soc-mchp-spdiftx.o
 obj-$(CONFIG_SND_MCHP_SOC_SPDIFRX) += snd-soc-mchp-spdifrx.o
+obj-$(CONFIG_SND_MCHP_SOC_PDMC) += snd-soc-mchp-pdmc.o
 
 # AT91 Machine Support
 snd-soc-sam9g20-wm8731-objs := sam9g20_wm8731.o
diff --git a/sound/soc/atmel/mchp-pdmc.c b/sound/soc/atmel/mchp-pdmc.c
new file mode 100644
index 000000000000..c44636f6207d
--- /dev/null
+++ b/sound/soc/atmel/mchp-pdmc.c
@@ -0,0 +1,1084 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Driver for Microchip Pulse Density Microphone Controller (PDMC) interfaces
+//
+// Copyright (C) 2019-2022 Microchip Technology Inc. and its subsidiaries
+//
+// Author: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
+
+#include <dt-bindings/sound/microchip,pdmc.h>
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+
+#include <sound/core.h>
+#include <sound/dmaengine_pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/tlv.h>
+
+/*
+ * ---- PDMC Register map ----
+ */
+#define MCHP_PDMC_CR			0x00	/* Control Register */
+#define MCHP_PDMC_MR			0x04	/* Mode Register */
+#define MCHP_PDMC_CFGR			0x08	/* Configuration Register */
+#define MCHP_PDMC_RHR			0x0C	/* Receive Holding Register */
+#define MCHP_PDMC_IER			0x14	/* Interrupt Enable Register */
+#define MCHP_PDMC_IDR			0x18	/* Interrupt Disable Register */
+#define MCHP_PDMC_IMR			0x1C	/* Interrupt Mask Register */
+#define MCHP_PDMC_ISR			0x20	/* Interrupt Status Register */
+#define MCHP_PDMC_VER			0x50	/* Version Register */
+
+/*
+ * ---- Control Register (Write-only) ----
+ */
+#define MCHP_PDMC_CR_SWRST		BIT(0)	/* Software Reset */
+
+/*
+ * ---- Mode Register (Read/Write) ----
+ */
+#define MCHP_PDMC_MR_PDMCEN_MASK	GENMASK(3, 0)
+#define MCHP_PDMC_MR_PDMCEN(ch)		(BIT(ch) & MCHP_PDMC_MR_PDMCEN_MASK)
+
+#define MCHP_PDMC_MR_OSR_MASK		GENMASK(17, 16)
+#define MCHP_PDMC_MR_OSR64		(1 << 16)
+#define MCHP_PDMC_MR_OSR128		(2 << 16)
+#define MCHP_PDMC_MR_OSR256		(3 << 16)
+
+#define MCHP_PDMC_MR_SINCORDER_MASK	GENMASK(23, 20)
+#define MCHP_PDMC_MR_SINCORDER(order)	(((order) << 20) & \
+					 MCHP_PDMC_MR_SINCORDER_MASK)
+
+#define MCHP_PDMC_MR_SINC_OSR_MASK	GENMASK(27, 24)
+#define MCHP_PDMC_MR_SINC_OSR_DIS	(0 << 24)
+#define MCHP_PDMC_MR_SINC_OSR_8		(1 << 24)
+#define MCHP_PDMC_MR_SINC_OSR_16	(2 << 24)
+#define MCHP_PDMC_MR_SINC_OSR_32	(3 << 24)
+#define MCHP_PDMC_MR_SINC_OSR_64	(4 << 24)
+#define MCHP_PDMC_MR_SINC_OSR_128	(5 << 24)
+#define MCHP_PDMC_MR_SINC_OSR_256	(6 << 24)
+
+#define MCHP_PDMC_MR_CHUNK_MASK		GENMASK(31, 28)
+#define MCHP_PDMC_MR_CHUNK(chunk)	(((chunk) << 28) & \
+					 MCHP_PDMC_MR_CHUNK_MASK)
+
+/*
+ * ---- Configuration Register (Read/Write) ----
+ */
+#define MCHP_PDMC_CFGR_BSSEL_MASK	(BIT(0) | BIT(2) | BIT(4) | BIT(6))
+#define MCHP_PDMC_CFGR_BSSEL(ch)	BIT((ch) * 2)
+
+#define MCHP_PDMC_CFGR_PDMSEL_MASK	(BIT(16) | BIT(18) | BIT(20) | BIT(22))
+#define MCHP_PDMC_CFGR_PDMSEL(ch)	BIT((ch) * 2 + 16)
+
+/*
+ * ---- Interrupt Enable/Disable/Mask/Status Registers ----
+ */
+#define MCHP_PDMC_IR_RXRDY		BIT(0)
+#define MCHP_PDMC_IR_RXEMPTY		BIT(1)
+#define MCHP_PDMC_IR_RXFULL		BIT(2)
+#define MCHP_PDMC_IR_RXCHUNK		BIT(3)
+#define MCHP_PDMC_IR_RXUDR		BIT(4)
+#define MCHP_PDMC_IR_RXOVR		BIT(5)
+
+/*
+ * ---- Version Register (Read-only) ----
+ */
+#define MCHP_PDMC_VER_VERSION		GENMASK(11, 0)
+
+#define MCHP_PDMC_MAX_CHANNELS		4
+#define MCHP_PDMC_DS_NO			2
+#define MCHP_PDMC_EDGE_NO		2
+
+struct mic_map {
+	int ds_pos;
+	int clk_edge;
+};
+
+struct mchp_pdmc_chmap {
+	struct snd_pcm_chmap_elem *chmap;
+	struct mchp_pdmc *dd;
+	struct snd_pcm *pcm;
+	struct snd_kcontrol *kctl;
+};
+
+struct mchp_pdmc {
+	struct mic_map channel_mic_map[MCHP_PDMC_MAX_CHANNELS];
+	struct device *dev;
+	struct snd_dmaengine_dai_dma_data addr;
+	struct regmap *regmap;
+	struct clk *pclk;
+	struct clk *gclk;
+	u32 pdmcen;
+	int mic_no;
+	int sinc_order;
+	bool audio_filter_en;
+	u8 gclk_enabled:1;
+};
+
+static const char *const mchp_pdmc_sinc_filter_order_text[] = {
+	"1", "2", "3", "4", "5"
+};
+
+static const unsigned int mchp_pdmc_sinc_filter_order_values[] = {
+	1, 2, 3, 4, 5,
+};
+
+static const struct soc_enum mchp_pdmc_sinc_filter_order_enum = {
+	.items = ARRAY_SIZE(mchp_pdmc_sinc_filter_order_text),
+	.texts = mchp_pdmc_sinc_filter_order_text,
+	.values = mchp_pdmc_sinc_filter_order_values,
+};
+
+static int mchp_pdmc_sinc_order_get(struct snd_kcontrol *kcontrol,
+				    struct snd_ctl_elem_value *uvalue)
+{
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct mchp_pdmc *dd = snd_soc_component_get_drvdata(component);
+	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+	unsigned int item;
+
+	item = snd_soc_enum_val_to_item(e, dd->sinc_order);
+	uvalue->value.enumerated.item[0] = item;
+
+	return 0;
+}
+
+static int mchp_pdmc_sinc_order_put(struct snd_kcontrol *kcontrol,
+				    struct snd_ctl_elem_value *uvalue)
+{
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct mchp_pdmc *dd = snd_soc_component_get_drvdata(component);
+	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+	unsigned int *item = uvalue->value.enumerated.item;
+	unsigned int val;
+
+	if (item[0] >= e->items)
+		return -EINVAL;
+
+	val = snd_soc_enum_item_to_val(e, item[0]) << e->shift_l;
+	if (val == dd->sinc_order)
+		return 0;
+
+	dd->sinc_order = val;
+
+	return 1;
+}
+
+static int mchp_pdmc_af_get(struct snd_kcontrol *kcontrol,
+			    struct snd_ctl_elem_value *uvalue)
+{
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct mchp_pdmc *dd = snd_soc_component_get_drvdata(component);
+
+	uvalue->value.integer.value[0] = !!dd->audio_filter_en;
+
+	return 0;
+}
+
+static int mchp_pdmc_af_put(struct snd_kcontrol *kcontrol,
+			    struct snd_ctl_elem_value *uvalue)
+{
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct mchp_pdmc *dd = snd_soc_component_get_drvdata(component);
+	bool af = uvalue->value.integer.value ? true : false;
+
+	if (dd->audio_filter_en == af)
+		return 0;
+
+	dd->audio_filter_en = af;
+
+	return 1;
+}
+
+static int mchp_pdmc_chmap_ctl_info(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_info *uinfo)
+{
+	struct mchp_pdmc_chmap *info = snd_kcontrol_chip(kcontrol);
+
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = info->dd->mic_no;
+	uinfo->value.integer.min = 0;
+	uinfo->value.integer.max = SNDRV_CHMAP_RR; /* maxmimum 4 channels */
+	return 0;
+}
+
+static inline struct snd_pcm_substream *
+mchp_pdmc_chmap_substream(struct mchp_pdmc_chmap *info, unsigned int idx)
+{
+	struct snd_pcm_substream *s;
+
+	for (s = info->pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream; s; s = s->next)
+		if (s->number == idx)
+			return s;
+	return NULL;
+}
+
+static struct snd_pcm_chmap_elem *mchp_pdmc_chmap_get(struct snd_pcm_substream *substream,
+						      struct mchp_pdmc_chmap *ch_info)
+{
+	struct snd_pcm_chmap_elem *map;
+
+	for (map = ch_info->chmap; map->channels; map++) {
+		if (map->channels == substream->runtime->channels)
+			return map;
+	}
+	return NULL;
+}
+
+static int mchp_pdmc_chmap_ctl_get(struct snd_kcontrol *kcontrol,
+				   struct snd_ctl_elem_value *ucontrol)
+{
+	struct mchp_pdmc_chmap *info = snd_kcontrol_chip(kcontrol);
+	struct mchp_pdmc *dd = info->dd;
+	unsigned int idx = snd_ctl_get_ioffidx(kcontrol, &ucontrol->id);
+	struct snd_pcm_substream *substream;
+	const struct snd_pcm_chmap_elem *map;
+	int i;
+	u32 cfgr_val = 0;
+
+	if (!info->chmap)
+		return -EINVAL;
+	substream = mchp_pdmc_chmap_substream(info, idx);
+	if (!substream)
+		return -ENODEV;
+	memset(ucontrol->value.integer.value, 0, sizeof(long) * info->dd->mic_no);
+	if (!substream->runtime)
+		return 0; /* no channels set */
+
+	map = mchp_pdmc_chmap_get(substream, info);
+	if (!map)
+		return -EINVAL;
+
+	for (i = 0; i < map->channels; i++) {
+		int map_idx = map->channels == 1 ? map->map[i] - SNDRV_CHMAP_MONO :
+						   map->map[i] - SNDRV_CHMAP_FL;
+
+		/* make sure the reported channel map is the real one, so write the map */
+		if (dd->channel_mic_map[map_idx].ds_pos)
+			cfgr_val |= MCHP_PDMC_CFGR_PDMSEL(i);
+		if (dd->channel_mic_map[map_idx].clk_edge)
+			cfgr_val |= MCHP_PDMC_CFGR_BSSEL(i);
+
+		ucontrol->value.integer.value[i] = map->map[i];
+	}
+
+	regmap_write(dd->regmap, MCHP_PDMC_CFGR, cfgr_val);
+
+	return 0;
+}
+
+static int mchp_pdmc_chmap_ctl_put(struct snd_kcontrol *kcontrol,
+				   struct snd_ctl_elem_value *ucontrol)
+{
+	struct mchp_pdmc_chmap *info = snd_kcontrol_chip(kcontrol);
+	struct mchp_pdmc *dd = info->dd;
+	unsigned int idx = snd_ctl_get_ioffidx(kcontrol, &ucontrol->id);
+	struct snd_pcm_substream *substream;
+	struct snd_pcm_chmap_elem *map;
+	u32 cfgr_val = 0;
+	int i;
+
+	if (!info->chmap)
+		return -EINVAL;
+	substream = mchp_pdmc_chmap_substream(info, idx);
+	if (!substream)
+		return -ENODEV;
+
+	map = mchp_pdmc_chmap_get(substream, info);
+	if (!map)
+		return -EINVAL;
+
+	for (i = 0; i < map->channels; i++) {
+		int map_idx;
+
+		map->map[i] = ucontrol->value.integer.value[i];
+		map_idx = map->channels == 1 ? map->map[i] - SNDRV_CHMAP_MONO :
+					       map->map[i] - SNDRV_CHMAP_FL;
+
+		/* configure IP for the desired channel map */
+		if (dd->channel_mic_map[map_idx].ds_pos)
+			cfgr_val |= MCHP_PDMC_CFGR_PDMSEL(i);
+		if (dd->channel_mic_map[map_idx].clk_edge)
+			cfgr_val |= MCHP_PDMC_CFGR_BSSEL(i);
+	}
+
+	regmap_write(dd->regmap, MCHP_PDMC_CFGR, cfgr_val);
+
+	return 0;
+}
+
+static void mchp_pdmc_chmap_ctl_private_free(struct snd_kcontrol *kcontrol)
+{
+	struct mchp_pdmc_chmap *info = snd_kcontrol_chip(kcontrol);
+
+	info->pcm->streams[SNDRV_PCM_STREAM_CAPTURE].chmap_kctl = NULL;
+	kfree(info);
+}
+
+static int mchp_pdmc_chmap_ctl_tlv(struct snd_kcontrol *kcontrol, int op_flag,
+				   unsigned int size, unsigned int __user *tlv)
+{
+	struct mchp_pdmc_chmap *info = snd_kcontrol_chip(kcontrol);
+	const struct snd_pcm_chmap_elem *map;
+	unsigned int __user *dst;
+	int c, count = 0;
+
+	if (!info->chmap)
+		return -EINVAL;
+	if (size < 8)
+		return -ENOMEM;
+	if (put_user(SNDRV_CTL_TLVT_CONTAINER, tlv))
+		return -EFAULT;
+	size -= 8;
+	dst = tlv + 2;
+	for (map = info->chmap; map->channels; map++) {
+		int chs_bytes = map->channels * 4;
+
+		if (size < 8)
+			return -ENOMEM;
+		if (put_user(SNDRV_CTL_TLVT_CHMAP_VAR, dst) ||
+		    put_user(chs_bytes, dst + 1))
+			return -EFAULT;
+		dst += 2;
+		size -= 8;
+		count += 8;
+		if (size < chs_bytes)
+			return -ENOMEM;
+		size -= chs_bytes;
+		count += chs_bytes;
+		for (c = 0; c < map->channels; c++) {
+			if (put_user(map->map[c], dst))
+				return -EFAULT;
+			dst++;
+		}
+	}
+	if (put_user(count, tlv + 1))
+		return -EFAULT;
+	return 0;
+}
+
+static const struct snd_kcontrol_new mchp_pdmc_snd_controls[] = {
+	SOC_SINGLE_BOOL_EXT("Audio Filter", 0, &mchp_pdmc_af_get, &mchp_pdmc_af_put),
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = "SINC Filter Order",
+		.info = snd_soc_info_enum_double,
+		.get = mchp_pdmc_sinc_order_get,
+		.put = mchp_pdmc_sinc_order_put,
+		.private_value = (unsigned long)&mchp_pdmc_sinc_filter_order_enum,
+	},
+};
+
+static int mchp_pdmc_close(struct snd_soc_component *component,
+			   struct snd_pcm_substream *substream)
+{
+	return snd_soc_add_component_controls(component, mchp_pdmc_snd_controls,
+					      ARRAY_SIZE(mchp_pdmc_snd_controls));
+}
+
+static int mchp_pdmc_open(struct snd_soc_component *component,
+			  struct snd_pcm_substream *substream)
+{
+	int i;
+
+	/* remove controls that can't be changed at runtime */
+	for (i = 0; i < ARRAY_SIZE(mchp_pdmc_snd_controls); i++) {
+		const struct snd_kcontrol_new *control = &mchp_pdmc_snd_controls[i];
+		struct snd_ctl_elem_id id;
+		struct snd_kcontrol *kctl;
+		int err;
+
+		if (component->name_prefix)
+			snprintf(id.name, sizeof(id.name), "%s %s", component->name_prefix,
+				 control->name);
+		else
+			strscpy(id.name, control->name, sizeof(id.name));
+
+		id.numid = 0;
+		id.iface = control->iface;
+		id.device = control->device;
+		id.subdevice = control->subdevice;
+		id.index = control->index;
+		kctl = snd_ctl_find_id(component->card->snd_card, &id);
+		if (!kctl) {
+			dev_err(component->dev, "Failed to find %s\n", control->name);
+			continue;
+		}
+		err = snd_ctl_remove(component->card->snd_card, kctl);
+		if (err < 0) {
+			dev_err(component->dev, "%d: Failed to remove %s\n", err,
+				control->name);
+			continue;
+		}
+	}
+
+	return 0;
+}
+
+static const struct snd_soc_component_driver mchp_pdmc_dai_component = {
+	.name = "mchp-pdmc",
+	.controls = mchp_pdmc_snd_controls,
+	.num_controls = ARRAY_SIZE(mchp_pdmc_snd_controls),
+	.open = &mchp_pdmc_open,
+	.close = &mchp_pdmc_close,
+};
+
+static const unsigned int mchp_pdmc_1mic[] = {1};
+static const unsigned int mchp_pdmc_2mic[] = {1, 2};
+static const unsigned int mchp_pdmc_3mic[] = {1, 2, 3};
+static const unsigned int mchp_pdmc_4mic[] = {1, 2, 3, 4};
+
+static const struct snd_pcm_hw_constraint_list mchp_pdmc_chan_constr[] = {
+	{
+		.list = mchp_pdmc_1mic,
+		.count = ARRAY_SIZE(mchp_pdmc_1mic),
+	},
+	{
+		.list = mchp_pdmc_2mic,
+		.count = ARRAY_SIZE(mchp_pdmc_2mic),
+	},
+	{
+		.list = mchp_pdmc_3mic,
+		.count = ARRAY_SIZE(mchp_pdmc_3mic),
+	},
+	{
+		.list = mchp_pdmc_4mic,
+		.count = ARRAY_SIZE(mchp_pdmc_4mic),
+	},
+};
+
+static int mchp_pdmc_startup(struct snd_pcm_substream *substream,
+			     struct snd_soc_dai *dai)
+{
+	struct mchp_pdmc *dd = snd_soc_dai_get_drvdata(dai);
+	int ret;
+
+	ret = clk_prepare_enable(dd->pclk);
+	if (ret) {
+		dev_err(dd->dev, "failed to enable the peripheral clock: %d\n", ret);
+		return ret;
+	}
+
+	regmap_write(dd->regmap, MCHP_PDMC_CR, MCHP_PDMC_CR_SWRST);
+
+	snd_pcm_hw_constraint_list(substream->runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
+				   &mchp_pdmc_chan_constr[dd->mic_no - 1]);
+
+	return 0;
+}
+
+static void mchp_pdmc_shutdown(struct snd_pcm_substream *substream,
+			       struct snd_soc_dai *dai)
+{
+	struct mchp_pdmc *dd = snd_soc_dai_get_drvdata(dai);
+
+	clk_disable_unprepare(dd->pclk);
+}
+
+static int mchp_pdmc_dai_probe(struct snd_soc_dai *dai)
+{
+	struct mchp_pdmc *dd = snd_soc_dai_get_drvdata(dai);
+
+	snd_soc_dai_init_dma_data(dai, NULL, &dd->addr);
+
+	return 0;
+}
+
+static int mchp_pdmc_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
+{
+	unsigned int fmt_master = fmt & SND_SOC_DAIFMT_MASTER_MASK;
+	unsigned int fmt_format = fmt & SND_SOC_DAIFMT_FORMAT_MASK;
+
+	/* IP needs to be bitclock master */
+	if (fmt_master != SND_SOC_DAIFMT_CBS_CFS &&
+	    fmt_master != SND_SOC_DAIFMT_CBS_CFM)
+		return -EINVAL;
+
+	/* IP supports only PDM interface */
+	if (fmt_format != SND_SOC_DAIFMT_PDM)
+		return -EINVAL;
+
+	return 0;
+}
+
+static u32 mchp_pdmc_mr_set_osr(int audio_filter_en, unsigned int osr)
+{
+	if (audio_filter_en) {
+		switch (osr) {
+		case 64:
+			return MCHP_PDMC_MR_OSR64;
+		case 128:
+			return MCHP_PDMC_MR_OSR128;
+		case 256:
+			return MCHP_PDMC_MR_OSR256;
+		}
+	} else {
+		switch (osr) {
+		case 8:
+			return MCHP_PDMC_MR_SINC_OSR_8;
+		case 16:
+			return MCHP_PDMC_MR_SINC_OSR_16;
+		case 32:
+			return MCHP_PDMC_MR_SINC_OSR_32;
+		case 64:
+			return MCHP_PDMC_MR_SINC_OSR_64;
+		case 128:
+			return MCHP_PDMC_MR_SINC_OSR_128;
+		case 256:
+			return MCHP_PDMC_MR_SINC_OSR_256;
+		}
+	}
+	return 0;
+}
+
+static inline int mchp_pdmc_period_to_maxburst(int period_size)
+{
+	if (!(period_size % 8))
+		return 8;
+	if (!(period_size % 4))
+		return 4;
+	if (!(period_size % 2))
+		return 2;
+	return 1;
+}
+
+static struct snd_pcm_chmap_elem mchp_pdmc_std_chmaps[] = {
+	{ .channels = 1,
+	  .map = { SNDRV_CHMAP_MONO } },
+	{ .channels = 2,
+	  .map = { SNDRV_CHMAP_FL, SNDRV_CHMAP_FR } },
+	{ .channels = 3,
+	  .map = { SNDRV_CHMAP_FL, SNDRV_CHMAP_FR,
+		   SNDRV_CHMAP_RL } },
+	{ .channels = 4,
+	  .map = { SNDRV_CHMAP_FL, SNDRV_CHMAP_FR,
+		   SNDRV_CHMAP_RL, SNDRV_CHMAP_RR } },
+	{ }
+};
+
+static int mchp_pdmc_hw_params(struct snd_pcm_substream *substream,
+			       struct snd_pcm_hw_params *params,
+			       struct snd_soc_dai *dai)
+{
+	struct mchp_pdmc *dd = snd_soc_dai_get_drvdata(dai);
+	struct snd_soc_component *comp = dai->component;
+	unsigned long gclk_rate = 0;
+	unsigned long best_diff_rate = ~0UL;
+	unsigned int channels = params_channels(params);
+	unsigned int osr = 0, osr_start;
+	unsigned int fs = params_rate(params);
+	u32 mr_val = 0;
+	u32 cfgr_val = 0;
+	int i;
+	int ret;
+
+	dev_dbg(comp->dev, "%s() rate=%u format=%#x width=%u channels=%u\n",
+		__func__, params_rate(params), params_format(params),
+		params_width(params), params_channels(params));
+
+	if (channels > dd->mic_no) {
+		dev_err(comp->dev, "more channels %u than microphones %d\n",
+			channels, dd->mic_no);
+		return -EINVAL;
+	}
+
+	dd->pdmcen = 0;
+	for (i = 0; i < channels; i++) {
+		dd->pdmcen |= MCHP_PDMC_MR_PDMCEN(i);
+		if (dd->channel_mic_map[i].ds_pos)
+			cfgr_val |= MCHP_PDMC_CFGR_PDMSEL(i);
+		if (dd->channel_mic_map[i].clk_edge)
+			cfgr_val |= MCHP_PDMC_CFGR_BSSEL(i);
+	}
+
+	if (dd->gclk_enabled) {
+		clk_disable_unprepare(dd->gclk);
+		dd->gclk_enabled = 0;
+	}
+
+	for (osr_start = dd->audio_filter_en ? 64 : 8;
+	     osr_start <= 256 && best_diff_rate; osr_start *= 2) {
+		long round_rate;
+		unsigned long diff_rate;
+
+		round_rate = clk_round_rate(dd->gclk,
+					    (unsigned long)fs * 16 * osr_start);
+		if (round_rate < 0)
+			continue;
+		diff_rate = abs((fs * 16 * osr_start) - round_rate);
+		if (diff_rate < best_diff_rate) {
+			best_diff_rate = diff_rate;
+			osr = osr_start;
+			gclk_rate = fs * 16 * osr;
+		}
+	}
+	if (!gclk_rate) {
+		dev_err(comp->dev, "invalid sampling rate: %u\n", fs);
+		return -EINVAL;
+	}
+
+	/* set the rate */
+	ret = clk_set_rate(dd->gclk, gclk_rate);
+	if (ret) {
+		dev_err(comp->dev, "unable to set rate %lu to GCLK: %d\n",
+			gclk_rate, ret);
+		return ret;
+	}
+
+	mr_val |= mchp_pdmc_mr_set_osr(dd->audio_filter_en, osr);
+
+	mr_val |= MCHP_PDMC_MR_SINCORDER(dd->sinc_order);
+
+	dd->addr.maxburst = mchp_pdmc_period_to_maxburst(snd_pcm_lib_period_bytes(substream));
+	mr_val |= MCHP_PDMC_MR_CHUNK(dd->addr.maxburst);
+	dev_dbg(comp->dev, "maxburst set to %d\n", dd->addr.maxburst);
+
+	clk_prepare_enable(dd->gclk);
+	dd->gclk_enabled = 1;
+
+	snd_soc_component_update_bits(comp, MCHP_PDMC_MR,
+				      MCHP_PDMC_MR_OSR_MASK |
+				      MCHP_PDMC_MR_SINCORDER_MASK |
+				      MCHP_PDMC_MR_SINC_OSR_MASK |
+				      MCHP_PDMC_MR_CHUNK_MASK, mr_val);
+
+	snd_soc_component_write(comp, MCHP_PDMC_CFGR, cfgr_val);
+
+	return 0;
+}
+
+static int mchp_pdmc_hw_free(struct snd_pcm_substream *substream,
+			     struct snd_soc_dai *dai)
+{
+	struct mchp_pdmc *dd = snd_soc_dai_get_drvdata(dai);
+
+	if (dd->gclk_enabled) {
+		clk_disable_unprepare(dd->gclk);
+		dd->gclk_enabled = 0;
+	}
+
+	return 0;
+}
+
+static int mchp_pdmc_trigger(struct snd_pcm_substream *substream,
+			     int cmd, struct snd_soc_dai *dai)
+{
+	struct mchp_pdmc *dd = snd_soc_dai_get_drvdata(dai);
+	struct snd_soc_component *cpu = dai->component;
+#ifdef DEBUG
+	u32 val;
+#endif
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		/* Enable overrun and underrun error interrupts */
+		regmap_write(dd->regmap, MCHP_PDMC_IER,
+			     MCHP_PDMC_IR_RXOVR | MCHP_PDMC_IR_RXUDR);
+		snd_soc_component_update_bits(cpu, MCHP_PDMC_MR,
+					      MCHP_PDMC_MR_PDMCEN_MASK,
+					      dd->pdmcen);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		/* Disable overrun and underrun error interrupts */
+		regmap_write(dd->regmap, MCHP_PDMC_IDR,
+			     MCHP_PDMC_IR_RXOVR | MCHP_PDMC_IR_RXUDR);
+		snd_soc_component_update_bits(cpu, MCHP_PDMC_MR,
+					      MCHP_PDMC_MR_PDMCEN_MASK, 0);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+#ifdef DEBUG
+	regmap_read(dd->regmap, MCHP_PDMC_MR, &val);
+	dev_dbg(dd->dev, "MR (0x%02x): 0x%08x\n", MCHP_PDMC_MR, val);
+	regmap_read(dd->regmap, MCHP_PDMC_CFGR, &val);
+	dev_dbg(dd->dev, "CFGR (0x%02x): 0x%08x\n", MCHP_PDMC_CFGR, val);
+	regmap_read(dd->regmap, MCHP_PDMC_IMR, &val);
+	dev_dbg(dd->dev, "IMR (0x%02x): 0x%08x\n", MCHP_PDMC_IMR, val);
+#endif
+
+	return 0;
+}
+
+static const struct snd_soc_dai_ops mchp_pdmc_dai_ops = {
+	.set_fmt	= mchp_pdmc_set_fmt,
+	.startup	= mchp_pdmc_startup,
+	.shutdown	= mchp_pdmc_shutdown,
+	.hw_params	= mchp_pdmc_hw_params,
+	.hw_free	= mchp_pdmc_hw_free,
+	.trigger	= mchp_pdmc_trigger,
+};
+
+static int mchp_pdmc_add_chmap_ctls(struct snd_pcm *pcm, struct mchp_pdmc *dd)
+{
+	struct mchp_pdmc_chmap *info;
+	struct snd_kcontrol_new knew = {
+		.iface = SNDRV_CTL_ELEM_IFACE_PCM,
+		.access = SNDRV_CTL_ELEM_ACCESS_READWRITE |
+			SNDRV_CTL_ELEM_ACCESS_TLV_READ |
+			SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK,
+		.info = mchp_pdmc_chmap_ctl_info,
+		.get = mchp_pdmc_chmap_ctl_get,
+		.put = mchp_pdmc_chmap_ctl_put,
+		.tlv.c = mchp_pdmc_chmap_ctl_tlv,
+	};
+	int err;
+
+	if (WARN_ON(pcm->streams[SNDRV_PCM_STREAM_CAPTURE].chmap_kctl))
+		return -EBUSY;
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+	info->pcm = pcm;
+	info->dd = dd;
+	info->chmap = mchp_pdmc_std_chmaps;
+	knew.name = "Capture Channel Map";
+	knew.device = pcm->device;
+	knew.count = pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream_count;
+	info->kctl = snd_ctl_new1(&knew, info);
+	if (!info->kctl) {
+		kfree(info);
+		return -ENOMEM;
+	}
+	info->kctl->private_free = mchp_pdmc_chmap_ctl_private_free;
+	err = snd_ctl_add(pcm->card, info->kctl);
+	if (err < 0)
+		return err;
+	pcm->streams[SNDRV_PCM_STREAM_CAPTURE].chmap_kctl = info->kctl;
+	return 0;
+}
+
+static int mchp_pdmc_pcm_new(struct snd_soc_pcm_runtime *rtd,
+			     struct snd_soc_dai *dai)
+{
+	struct mchp_pdmc *dd = snd_soc_dai_get_drvdata(dai);
+	int ret;
+
+	ret = mchp_pdmc_add_chmap_ctls(rtd->pcm, dd);
+	if (ret < 0) {
+		dev_err(dd->dev, "failed to add channel map controls: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct snd_soc_dai_driver mchp_pdmc_dai = {
+	.probe	= mchp_pdmc_dai_probe,
+	.capture = {
+		.stream_name	= "Capture",
+		.channels_min	= 1,
+		.channels_max	= 4,
+		.rate_min	= 8000,
+		.rate_max	= 192000,
+		.rates		= SNDRV_PCM_RATE_KNOT,
+		.formats	= SNDRV_PCM_FMTBIT_S24_LE,
+	},
+	.ops = &mchp_pdmc_dai_ops,
+	.pcm_new = &mchp_pdmc_pcm_new,
+};
+
+/* PDMC interrupt handler */
+static irqreturn_t mchp_pdmc_interrupt(int irq, void *dev_id)
+{
+	struct mchp_pdmc *dd = (struct mchp_pdmc *)dev_id;
+	u32 isr, msr, pending;
+	irqreturn_t ret = IRQ_NONE;
+
+	regmap_read(dd->regmap, MCHP_PDMC_ISR, &isr);
+	regmap_read(dd->regmap, MCHP_PDMC_IMR, &msr);
+
+	pending = isr & msr;
+	dev_dbg(dd->dev, "ISR (0x%02x): 0x%08x, IMR (0x%02x): 0x%08x, pending: 0x%08x\n",
+		MCHP_PDMC_ISR, isr, MCHP_PDMC_IMR, msr, pending);
+	if (!pending)
+		return IRQ_NONE;
+
+	if (pending & MCHP_PDMC_IR_RXUDR) {
+		dev_warn(dd->dev, "underrun detected\n");
+		regmap_write(dd->regmap, MCHP_PDMC_IDR, MCHP_PDMC_IR_RXUDR);
+		ret = IRQ_HANDLED;
+	}
+	if (pending & MCHP_PDMC_IR_RXOVR) {
+		dev_warn(dd->dev, "overrun detected\n");
+		regmap_write(dd->regmap, MCHP_PDMC_IDR, MCHP_PDMC_IR_RXOVR);
+		ret = IRQ_HANDLED;
+	}
+
+	return ret;
+}
+
+/* regmap configuration */
+static bool mchp_pdmc_readable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case MCHP_PDMC_MR:
+	case MCHP_PDMC_CFGR:
+	case MCHP_PDMC_IMR:
+	case MCHP_PDMC_ISR:
+	case MCHP_PDMC_VER:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool mchp_pdmc_writeable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case MCHP_PDMC_CR:
+	case MCHP_PDMC_MR:
+	case MCHP_PDMC_CFGR:
+	case MCHP_PDMC_IER:
+	case MCHP_PDMC_IDR:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool mchp_pdmc_precious_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case MCHP_PDMC_RHR:
+	case MCHP_PDMC_ISR:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config mchp_pdmc_regmap_config = {
+	.reg_bits	= 32,
+	.reg_stride	= 4,
+	.val_bits	= 32,
+	.max_register	= MCHP_PDMC_VER,
+	.readable_reg	= mchp_pdmc_readable_reg,
+	.writeable_reg	= mchp_pdmc_writeable_reg,
+	.precious_reg	= mchp_pdmc_precious_reg,
+};
+
+static int mchp_pdmc_dt_init(struct mchp_pdmc *dd)
+{
+	struct device_node *np = dd->dev->of_node;
+	bool mic_ch[MCHP_PDMC_DS_NO][MCHP_PDMC_EDGE_NO] = {0};
+	int i;
+	int ret;
+
+	if (!np) {
+		dev_err(dd->dev, "device node not found\n");
+		return -EINVAL;
+	}
+
+	dd->mic_no = of_property_count_u32_elems(np, "microchip,mic-pos");
+	if (dd->mic_no < 0) {
+		dev_err(dd->dev, "failed to get mchp,mic-pos: %d",
+			dd->mic_no);
+		return dd->mic_no;
+	}
+	if (!dd->mic_no || dd->mic_no % 2 ||
+	    dd->mic_no / 2 > MCHP_PDMC_MAX_CHANNELS) {
+		dev_err(dd->dev, "invalid array length for mchp,mic-pos: %d",
+			dd->mic_no);
+		return -EINVAL;
+	}
+
+	dd->mic_no /= 2;
+
+	dev_info(dd->dev, "%d PDM microchopnes declared\n", dd->mic_no);
+
+	/* by default, we consider the order of microphones in mchp,mic-pos to
+	 * be the same with the channel mapping; 1st microphone channel 0, 2nd
+	 * microphone channel 1, etc.
+	 */
+	for (i = 0; i < dd->mic_no; i++) {
+		int ds;
+		int edge;
+
+		ret = of_property_read_u32_index(np, "microchip,mic-pos", i * 2,
+						 &ds);
+		if (ret) {
+			dev_err(dd->dev,
+				"failed to get value no %d value from microchip,mic-pos: %d",
+				i * 2, ret);
+			return ret;
+		}
+		if (ds >= MCHP_PDMC_DS_NO) {
+			dev_err(dd->dev,
+				"invalid DS index in microchip,mic-pos array: %d",
+				ds);
+			return -EINVAL;
+		}
+
+		ret = of_property_read_u32_index(np, "microchip,mic-pos", i * 2 + 1,
+						 &edge);
+		if (ret) {
+			dev_err(dd->dev,
+				"failed to get value no %d value from microchip,mic-pos: %d",
+				i * 2 + 1, ret);
+			return ret;
+		}
+
+		if (edge != MCHP_PDMC_CLK_POSITIVE &&
+		    edge != MCHP_PDMC_CLK_NEGATIVE) {
+			dev_err(dd->dev,
+				"invalid edge in microchip,mic-pos array: %d", edge);
+			return -EINVAL;
+		}
+		if (mic_ch[ds][edge]) {
+			dev_err(dd->dev,
+				"duplicated mic (DS %d, edge %d) in microchip,mic-pos array",
+				ds, edge);
+			return -EINVAL;
+		}
+		mic_ch[ds][edge] = true;
+		dd->channel_mic_map[i].ds_pos = ds;
+		dd->channel_mic_map[i].clk_edge = edge;
+	}
+
+	return 0;
+}
+
+/* used to clean the channel index found on RHR's MSB */
+static int mchp_pdmc_process(struct snd_pcm_substream *substream,
+			     int channel, unsigned long hwoff,
+			     void *buf, unsigned long bytes)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	u8 *dma_ptr = runtime->dma_area + hwoff +
+		      channel * (runtime->dma_bytes / runtime->channels);
+	u8 *dma_ptr_end = dma_ptr + bytes;
+	unsigned int sample_size = samples_to_bytes(runtime, 1);
+
+	for (; dma_ptr < dma_ptr_end; dma_ptr += sample_size)
+		*dma_ptr = 0;
+
+	return 0;
+}
+
+static struct snd_dmaengine_pcm_config mchp_pdmc_config = {
+	.process = mchp_pdmc_process,
+};
+
+static int mchp_pdmc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mchp_pdmc *dd;
+	struct resource *res;
+	void __iomem *io_base;
+	u32 version;
+	int irq;
+	int ret;
+
+	dd = devm_kzalloc(dev, sizeof(*dd), GFP_KERNEL);
+	if (!dd)
+		return -ENOMEM;
+
+	dd->dev = &pdev->dev;
+	ret =  mchp_pdmc_dt_init(dd);
+	if (ret < 0)
+		return ret;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(dev, "failed to get irq: %d\n", irq);
+		return irq;
+	}
+
+	dd->pclk = devm_clk_get(dev, "pclk");
+	if (IS_ERR(dd->pclk)) {
+		ret = PTR_ERR(dd->pclk);
+		dev_err(dev, "failed to get peripheral clock: %d\n", ret);
+		return ret;
+	}
+
+	dd->gclk = devm_clk_get(dev, "gclk");
+	if (IS_ERR(dd->gclk)) {
+		ret = PTR_ERR(dd->gclk);
+		dev_err(dev, "failed to get GCK: %d\n", ret);
+		return ret;
+	}
+
+	io_base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+	if (IS_ERR(io_base)) {
+		ret = PTR_ERR(io_base);
+		dev_err(dev, "failed to remap register memory: %d\n", ret);
+		return ret;
+	}
+
+	dd->regmap = devm_regmap_init_mmio(dev, io_base,
+					   &mchp_pdmc_regmap_config);
+	if (IS_ERR(dd->regmap)) {
+		ret = PTR_ERR(dd->regmap);
+		dev_err(dev, "failed to init register map: %d\n", ret);
+		return ret;
+	}
+
+	ret = devm_request_irq(dev, irq, mchp_pdmc_interrupt, 0,
+			       dev_name(&pdev->dev), (void *)dd);
+	if (ret < 0) {
+		dev_err(dev, "can't register ISR for IRQ %u (ret=%i)\n",
+			irq, ret);
+		return ret;
+	}
+
+	/* by default audio filter is enabled and the SINC Filter order
+	 * will be set to the recommended value, 3
+	 */
+	dd->audio_filter_en = true;
+	dd->sinc_order = 3;
+
+	dd->addr.addr = (dma_addr_t)res->start + MCHP_PDMC_RHR;
+	platform_set_drvdata(pdev, dd);
+
+	/* register platform */
+	ret = devm_snd_dmaengine_pcm_register(dev, &mchp_pdmc_config, 0);
+	if (ret) {
+		dev_err(dev, "could not register platform: %d\n", ret);
+		return ret;
+	}
+
+	ret = devm_snd_soc_register_component(dev, &mchp_pdmc_dai_component,
+					      &mchp_pdmc_dai, 1);
+	if (ret) {
+		dev_err(dev, "could not register CPU DAI: %d\n", ret);
+		return ret;
+	}
+
+	/* print IP version */
+	regmap_read(dd->regmap, MCHP_PDMC_VER, &version);
+	dev_info(dd->dev, "hw version: %#lx\n",
+		 version & MCHP_PDMC_VER_VERSION);
+
+	return 0;
+}
+
+static const struct of_device_id mchp_pdmc_of_match[] = {
+	{
+		.compatible = "microchip,sama7g5-pdmc",
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(of, mchp_pdmc_of_match);
+
+static struct platform_driver mchp_pdmc_driver = {
+	.driver	= {
+		.name		= "mchp-pdmc",
+		.of_match_table	= of_match_ptr(mchp_pdmc_of_match),
+		.pm		= &snd_soc_pm_ops,
+	},
+	.probe	= mchp_pdmc_probe,
+};
+module_platform_driver(mchp_pdmc_driver);
+
+MODULE_DESCRIPTION("Microchip PDMC driver under ALSA SoC architecture");
+MODULE_AUTHOR("Codrin Ciubotariu <codrin.ciubotariu@microchip.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.32.0

[PATCH v3 4/6] ARM: dts: at91: sama7g5: add nodes for PDMC

[Codrin Ciubotariu]

Microchip's SAMA7G5 embeds two PDMCs. The PDMCs can be used to connect 2x4
PDM microphones.

Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
---

Chnages in v3:
 - none

Changes in v2:
 - set 'sound' as nodes' name

 arch/arm/boot/dts/sama7g5.dtsi | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/sama7g5.dtsi b/arch/arm/boot/dts/sama7g5.dtsi
index eddcfbf4d223..70e86444194f 100644
--- a/arch/arm/boot/dts/sama7g5.dtsi
+++ b/arch/arm/boot/dts/sama7g5.dtsi
@@ -275,6 +275,30 @@ pwm: pwm@e1604000 {
 			status = "disabled";
 		};
 
+		pdmc0: sound@e1608000 {
+			compatible = "microchip,sama7g5-pdmc";
+			reg = <0xe1608000 0x1000>;
+			interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
+			#sound-dai-cells = <0>;
+			dmas = <&dma0 AT91_XDMAC_DT_PERID(37)>;
+			dma-names = "rx";
+			clocks = <&pmc PMC_TYPE_PERIPHERAL 68>, <&pmc PMC_TYPE_GCK 68>;
+			clock-names = "pclk", "gclk";
+			status = "disabled";
+		};
+
+		pdmc1: sound@e160c000 {
+			compatible = "microchip,sama7g5-pdmc";
+			reg = <0xe160c000 0x1000>;
+			interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+			#sound-dai-cells = <0>;
+			dmas = <&dma0 AT91_XDMAC_DT_PERID(38)>;
+			dma-names = "rx";
+			clocks = <&pmc PMC_TYPE_PERIPHERAL 69>, <&pmc PMC_TYPE_GCK 69>;
+			clock-names = "pclk", "gclk";
+			status = "disabled";
+		};
+
 		spdifrx: spdifrx@e1614000 {
 			#sound-dai-cells = <0>;
 			compatible = "microchip,sama7g5-spdifrx";
-- 
2.32.0

[PATCH v3 5/6] ARM: dts: at91: sama7g5ek: add node for PDMC0

[Codrin Ciubotariu]

SAMA7G5-EK has 4 PDM microphones connected to PDMC0. PDMC0 pinmux is in
conflict with gmac1, gmac1 being enabled by default.

Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
---

Changes in v3:
 - none

Changes in v2:
 - replaced included file dt-bindings/sound/mchp,pdmc.h wih
   dt-bindings/sound/microchip,pdmc.h

 arch/arm/boot/dts/at91-sama7g5ek.dts | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/at91-sama7g5ek.dts b/arch/arm/boot/dts/at91-sama7g5ek.dts
index ccf9e224da78..a1fb980d3fc1 100644
--- a/arch/arm/boot/dts/at91-sama7g5ek.dts
+++ b/arch/arm/boot/dts/at91-sama7g5ek.dts
@@ -14,6 +14,7 @@
 #include <dt-bindings/mfd/atmel-flexcom.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/pinctrl/at91.h>
+#include <dt-bindings/sound/microchip,pdmc.h>
 
 / {
 	model = "Microchip SAMA7G5-EK";
@@ -439,7 +440,7 @@ &gmac1 {
 		     &pinctrl_gmac1_mdio_default
 		     &pinctrl_gmac1_phy_irq>;
 	phy-mode = "rmii";
-	status = "okay";
+	status = "okay"; /* Conflict with pdmc0. */
 
 	ethernet-phy@0 {
 		reg = <0x0>;
@@ -453,6 +454,17 @@ &i2s0 {
 	pinctrl-0 = <&pinctrl_i2s0_default>;
 };
 
+&pdmc0 {
+	#sound-dai-cells = <0>;
+	microchip,mic-pos = <MCHP_PDMC_DS0 MCHP_PDMC_CLK_NEGATIVE>, /* MIC 1 */
+			    <MCHP_PDMC_DS1 MCHP_PDMC_CLK_NEGATIVE>, /* MIC 2 */
+			    <MCHP_PDMC_DS0 MCHP_PDMC_CLK_POSITIVE>, /* MIC 3 */
+			    <MCHP_PDMC_DS1 MCHP_PDMC_CLK_POSITIVE>; /* MIC 4 */
+	status = "disabled"; /* Conflict with gmac1. */
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_pdmc0_default>;
+};
+
 &pioA {
 	pinctrl_flx0_default: flx0_default {
 		pinmux = <PIN_PE3__FLEXCOM0_IO0>,
@@ -609,6 +621,13 @@ pinctrl_mikrobus1_spi: mikrobus1_spi {
 		bias-disable;
 	};
 
+	pinctrl_pdmc0_default: pdmc0_default {
+		pinmux = <PIN_PD23__PDMC0_DS0>,
+			 <PIN_PD24__PDMC0_DS1>,
+			 <PIN_PD22__PDMC0_CLK>;
+		bias_disable;
+	};
+
 	pinctrl_qspi: qspi {
 		pinmux = <PIN_PB12__QSPI0_IO0>,
 			 <PIN_PB11__QSPI0_IO1>,
-- 
2.32.0

[PATCH v3 6/6] ARM: configs: at91: sama7_defconfig: add MCHP PDMC and DMIC drivers

[Codrin Ciubotariu]

Enable drivers needed for Microchip's PDMC and PDM microphones.

Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
---

Changes in v2,v3:
 - none;

 arch/arm/configs/sama7_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/sama7_defconfig b/arch/arm/configs/sama7_defconfig
index 0368068e04d9..bc29badab890 100644
--- a/arch/arm/configs/sama7_defconfig
+++ b/arch/arm/configs/sama7_defconfig
@@ -138,6 +138,8 @@ CONFIG_SND_SOC_MIKROE_PROTO=m
 CONFIG_SND_MCHP_SOC_I2S_MCC=y
 CONFIG_SND_MCHP_SOC_SPDIFTX=y
 CONFIG_SND_MCHP_SOC_SPDIFRX=y
+CONFIG_SND_MCHP_SOC_PDMC=y
+CONFIG_SND_SOC_DMIC=y
 CONFIG_SND_SOC_PCM5102A=y
 CONFIG_SND_SOC_SPDIF=y
 CONFIG_SND_SIMPLE_CARD=y
-- 
2.32.0

Re: [PATCH v3 2/6] ASoC: dt-bindings: Document Microchip's PDMC

[Krzysztof Kozlowski]

On 07/03/2022 13:21, Codrin Ciubotariu wrote:
> Add DT bindings for the new Microchip PDMC embedded in sama7g5 SoCs.
> 
> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> ---
> 
> Changes in v3:
>  - set line length to 80 characters long
>  - set 'reg' as the second property
> 
> Changes in v2:
>  - renamed patch from 'ASoC: add DT bindings for Microchip PDMC' to
>    'ASoC: dt-bindings: Document Microchip's PDMC';
>  - renamed yaml file from 'mchp,pdmc.yaml' to 'microchip,pdmc.yaml';
>  - used imperative mode in commit description;
>  - renamed mchp,pdmc.h to microchip,pdmc.h;
>  - fixed 'title' to represent HW;
>  - made 'compatible' first property;
>  - s/microhpone/microphone
>  - none name in example set to 'sound'
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>


Best regards,
Krzysztof

Re: (subset) [PATCH v3 0/6] Add driver for SAMA7G5's PDMC

[Mark Brown]

On Mon, 7 Mar 2022 14:21:56 +0200, Codrin Ciubotariu wrote:
> This patch series adds support for Pulse Density Microphone Controller
> (PDMC), present on Microchip's SAMA7G5.
> The PDMC interfaces up to 4 digital microphones having Pulse Density
> Modulated (PDM) outputs. It generates a single clock line and samples 1 or
> 2 data lines. The signal path includes an audio grade programmable
> decimation filter and outputs 24-bit audio words.
> The source of each channel can be independently defined as PDMC_DS0 or
> PDMC_DS1, sampled at the rising or falling edge of PDMC_CLK.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/6] ASoC: dmaengine: do not use a NULL prepare_slave_config() callback
      commit: 9a1e13440a4f2e7566fd4c5eae6a53e6400e08a4
[2/6] ASoC: dt-bindings: Document Microchip's PDMC
      commit: 015044e9610c8523794ea6cb55d5388bc00ba96a
[3/6] ASoC: atmel: mchp-pdmc: add PDMC driver
      commit: 50291652af5269813baa6024eb0e81b5f0bbb451

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH v3 1/6] ASoC: dmaengine: do not use a NULL prepare_slave_config() callback

[Sascha Hauer]

Hi,

On Mon, Mar 07, 2022 at 02:21:57PM +0200, Codrin Ciubotariu wrote:
> Even if struct snd_dmaengine_pcm_config is used, prepare_slave_config()
> callback might not be set. Check if this callback is set before using it.
> 
> Fixes: fa654e085300 ("ASoC: dmaengine-pcm: Provide default config")
> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> ---
> 
> Changes in v2,v3:
>  - none
> 
>  sound/soc/soc-generic-dmaengine-pcm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
> index 285441d6aeed..2ab2ddc1294d 100644
> --- a/sound/soc/soc-generic-dmaengine-pcm.c
> +++ b/sound/soc/soc-generic-dmaengine-pcm.c
> @@ -86,10 +86,10 @@ static int dmaengine_pcm_hw_params(struct snd_soc_component *component,
>  
>  	memset(&slave_config, 0, sizeof(slave_config));
>  
> -	if (!pcm->config)
> -		prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config;
> -	else
> +	if (pcm->config && pcm->config->prepare_slave_config)
>  		prepare_slave_config = pcm->config->prepare_slave_config;
> +	else
> +		prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config;
>  
>  	if (prepare_slave_config) {
>  		int ret = prepare_slave_config(substream, params, &slave_config);

I wonder if this patch is correct. There are drivers like
sound/soc/mxs/mxs-pcm.c which call snd_dmaengine_pcm_register() with a
config which has the prepare_slave_config callback unset. For these
drivers dmaengine_pcm_hw_params() previously was a no-op. Now with this
patch snd_dmaengine_pcm_prepare_slave_config() and
dmaengine_slave_config() are called. At least for the mxs-pcm driver
calling dmaengine_slave_config() will return -ENOSYS.

At least the "Check if this callback is set before using it" part is
wrong, the callback is checked before using it with

	if (prepare_slave_config) {
		...
	}

I don't have any mxs hardware at hand to test this. I just stumbled upon
the change of behaviour when rebasing
https://patchwork.kernel.org/project/alsa-devel/patch/20220301122111.1073174-1-s.hauer@pengutronix.de/
on current master.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v3 1/6] ASoC: dmaengine: do not use a NULL prepare_slave_config() callback

[Codrin.Ciubotariu]

On 20.04.2022 12:15, Sascha Hauer wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi,

Hi Sascha,

> 
> On Mon, Mar 07, 2022 at 02:21:57PM +0200, Codrin Ciubotariu wrote:
>> Even if struct snd_dmaengine_pcm_config is used, prepare_slave_config()
>> callback might not be set. Check if this callback is set before using it.
>>
>> Fixes: fa654e085300 ("ASoC: dmaengine-pcm: Provide default config")
>> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
>> ---
>>
>> Changes in v2,v3:
>>   - none
>>
>>   sound/soc/soc-generic-dmaengine-pcm.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
>> index 285441d6aeed..2ab2ddc1294d 100644
>> --- a/sound/soc/soc-generic-dmaengine-pcm.c
>> +++ b/sound/soc/soc-generic-dmaengine-pcm.c
>> @@ -86,10 +86,10 @@ static int dmaengine_pcm_hw_params(struct snd_soc_component *component,
>>
>>        memset(&slave_config, 0, sizeof(slave_config));
>>
>> -     if (!pcm->config)
>> -             prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config;
>> -     else
>> +     if (pcm->config && pcm->config->prepare_slave_config)
>>                prepare_slave_config = pcm->config->prepare_slave_config;
>> +     else
>> +             prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config;
>>
>>        if (prepare_slave_config) {
>>                int ret = prepare_slave_config(substream, params, &slave_config);
> 
> I wonder if this patch is correct. There are drivers like
> sound/soc/mxs/mxs-pcm.c which call snd_dmaengine_pcm_register() with a
> config which has the prepare_slave_config callback unset. For these
> drivers dmaengine_pcm_hw_params() previously was a no-op. Now with this
> patch snd_dmaengine_pcm_prepare_slave_config() and
> dmaengine_slave_config() are called. At least for the mxs-pcm driver
> calling dmaengine_slave_config() will return -ENOSYS.
> 
> At least the "Check if this callback is set before using it" part is
> wrong, the callback is checked before using it with
> 
>          if (prepare_slave_config) {
>                  ...
>          }
> 
> I don't have any mxs hardware at hand to test this. I just stumbled upon
> the change of behaviour when rebasing
> https://patchwork.kernel.org/project/alsa-devel/patch/20220301122111.1073174-1-s.hauer@pengutronix.de/
> on current master.

You are right. I changed the behavior from:
if (pmc->config && !pcm->config->prepare_slave_config)
	<do nothing>
to:
if (pmc->config && !pcm->config->prepare_slave_config)
	snd_dmaengine_pcm_prepare_slave_config()

It was not intended and I agree that the commit message is not accurate. 
I guess some drivers might not need dmaengine_slave_config()...
However, in my case, for the mchp-pdmc driver, I do have pcm->config 
with pcm->config->prepare_slave_config NULL, but I still need 
snd_dmaengine_pcm_prepare_slave_config() to be called. Should we add a 
separate flag to call snd_dmaengine_pcm_prepare_slave_config() if 
pcm->config->prepare_slave_config is NULL?

Nice catch!

Best regards,
Codrin

Re: [PATCH v3 1/6] ASoC: dmaengine: do not use a NULL prepare_slave_config() callback

[Sascha Hauer]

On Wed, Apr 20, 2022 at 09:58:06AM +0000, Codrin.Ciubotariu@microchip.com wrote:
> On 20.04.2022 12:15, Sascha Hauer wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > Hi,
> 
> Hi Sascha,
> 
> > 
> > On Mon, Mar 07, 2022 at 02:21:57PM +0200, Codrin Ciubotariu wrote:
> >> Even if struct snd_dmaengine_pcm_config is used, prepare_slave_config()
> >> callback might not be set. Check if this callback is set before using it.
> >>
> >> Fixes: fa654e085300 ("ASoC: dmaengine-pcm: Provide default config")
> >> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> >> ---
> >>
> >> Changes in v2,v3:
> >>   - none
> >>
> >>   sound/soc/soc-generic-dmaengine-pcm.c | 6 +++---
> >>   1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
> >> index 285441d6aeed..2ab2ddc1294d 100644
> >> --- a/sound/soc/soc-generic-dmaengine-pcm.c
> >> +++ b/sound/soc/soc-generic-dmaengine-pcm.c
> >> @@ -86,10 +86,10 @@ static int dmaengine_pcm_hw_params(struct snd_soc_component *component,
> >>
> >>        memset(&slave_config, 0, sizeof(slave_config));
> >>
> >> -     if (!pcm->config)
> >> -             prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config;
> >> -     else
> >> +     if (pcm->config && pcm->config->prepare_slave_config)
> >>                prepare_slave_config = pcm->config->prepare_slave_config;
> >> +     else
> >> +             prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config;
> >>
> >>        if (prepare_slave_config) {
> >>                int ret = prepare_slave_config(substream, params, &slave_config);
> > 
> > I wonder if this patch is correct. There are drivers like
> > sound/soc/mxs/mxs-pcm.c which call snd_dmaengine_pcm_register() with a
> > config which has the prepare_slave_config callback unset. For these
> > drivers dmaengine_pcm_hw_params() previously was a no-op. Now with this
> > patch snd_dmaengine_pcm_prepare_slave_config() and
> > dmaengine_slave_config() are called. At least for the mxs-pcm driver
> > calling dmaengine_slave_config() will return -ENOSYS.
> > 
> > At least the "Check if this callback is set before using it" part is
> > wrong, the callback is checked before using it with
> > 
> >          if (prepare_slave_config) {
> >                  ...
> >          }
> > 
> > I don't have any mxs hardware at hand to test this. I just stumbled upon
> > the change of behaviour when rebasing
> > https://patchwork.kernel.org/project/alsa-devel/patch/20220301122111.1073174-1-s.hauer@pengutronix.de/
> > on current master.
> 
> You are right. I changed the behavior from:
> if (pmc->config && !pcm->config->prepare_slave_config)
> 	<do nothing>
> to:
> if (pmc->config && !pcm->config->prepare_slave_config)
> 	snd_dmaengine_pcm_prepare_slave_config()
> 
> It was not intended and I agree that the commit message is not accurate. 
> I guess some drivers might not need dmaengine_slave_config()...
> However, in my case, for the mchp-pdmc driver, I do have pcm->config 
> with pcm->config->prepare_slave_config NULL, but I still need 
> snd_dmaengine_pcm_prepare_slave_config() to be called. Should we add a 
> separate flag to call snd_dmaengine_pcm_prepare_slave_config() if 
> pcm->config->prepare_slave_config is NULL?

Other drivers set pcm->config->prepare_slave_config to
snd_dmaengine_pcm_prepare_slave_config() explicitly:

sound/soc/fsl/imx-pcm-dma.c:33: .prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config,

I think that's the way to go.

Regards,
Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v3 1/6] ASoC: dmaengine: do not use a NULL prepare_slave_config() callback

[Codrin.Ciubotariu]

On 20.04.2022 13:06, Sascha Hauer wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Wed, Apr 20, 2022 at 09:58:06AM +0000, Codrin.Ciubotariu@microchip.com wrote:
>> On 20.04.2022 12:15, Sascha Hauer wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Hi,
>>
>> Hi Sascha,
>>
>>>
>>> On Mon, Mar 07, 2022 at 02:21:57PM +0200, Codrin Ciubotariu wrote:
>>>> Even if struct snd_dmaengine_pcm_config is used, prepare_slave_config()
>>>> callback might not be set. Check if this callback is set before using it.
>>>>
>>>> Fixes: fa654e085300 ("ASoC: dmaengine-pcm: Provide default config")
>>>> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
>>>> ---
>>>>
>>>> Changes in v2,v3:
>>>>    - none
>>>>
>>>>    sound/soc/soc-generic-dmaengine-pcm.c | 6 +++---
>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
>>>> index 285441d6aeed..2ab2ddc1294d 100644
>>>> --- a/sound/soc/soc-generic-dmaengine-pcm.c
>>>> +++ b/sound/soc/soc-generic-dmaengine-pcm.c
>>>> @@ -86,10 +86,10 @@ static int dmaengine_pcm_hw_params(struct snd_soc_component *component,
>>>>
>>>>         memset(&slave_config, 0, sizeof(slave_config));
>>>>
>>>> -     if (!pcm->config)
>>>> -             prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config;
>>>> -     else
>>>> +     if (pcm->config && pcm->config->prepare_slave_config)
>>>>                 prepare_slave_config = pcm->config->prepare_slave_config;
>>>> +     else
>>>> +             prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config;
>>>>
>>>>         if (prepare_slave_config) {
>>>>                 int ret = prepare_slave_config(substream, params, &slave_config);
>>>
>>> I wonder if this patch is correct. There are drivers like
>>> sound/soc/mxs/mxs-pcm.c which call snd_dmaengine_pcm_register() with a
>>> config which has the prepare_slave_config callback unset. For these
>>> drivers dmaengine_pcm_hw_params() previously was a no-op. Now with this
>>> patch snd_dmaengine_pcm_prepare_slave_config() and
>>> dmaengine_slave_config() are called. At least for the mxs-pcm driver
>>> calling dmaengine_slave_config() will return -ENOSYS.
>>>
>>> At least the "Check if this callback is set before using it" part is
>>> wrong, the callback is checked before using it with
>>>
>>>           if (prepare_slave_config) {
>>>                   ...
>>>           }
>>>
>>> I don't have any mxs hardware at hand to test this. I just stumbled upon
>>> the change of behaviour when rebasing
>>> https://patchwork.kernel.org/project/alsa-devel/patch/20220301122111.1073174-1-s.hauer@pengutronix.de/
>>> on current master.
>>
>> You are right. I changed the behavior from:
>> if (pmc->config && !pcm->config->prepare_slave_config)
>>        <do nothing>
>> to:
>> if (pmc->config && !pcm->config->prepare_slave_config)
>>        snd_dmaengine_pcm_prepare_slave_config()
>>
>> It was not intended and I agree that the commit message is not accurate.
>> I guess some drivers might not need dmaengine_slave_config()...
>> However, in my case, for the mchp-pdmc driver, I do have pcm->config
>> with pcm->config->prepare_slave_config NULL, but I still need
>> snd_dmaengine_pcm_prepare_slave_config() to be called. Should we add a
>> separate flag to call snd_dmaengine_pcm_prepare_slave_config() if
>> pcm->config->prepare_slave_config is NULL?
> 
> Other drivers set pcm->config->prepare_slave_config to
> snd_dmaengine_pcm_prepare_slave_config() explicitly:
> 
> sound/soc/fsl/imx-pcm-dma.c:33: .prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config,
> 
> I think that's the way to go.

That's more elegant, right. I will revert this patch and use your 
suggestion for the mchp-pdmc driver.

Thanks and best regards,
Codrin

Re: [PATCH v3 6/6] ARM: configs: at91: sama7_defconfig: add MCHP PDMC and DMIC drivers

[Nicolas Ferre]

On 07/03/2022 at 13:22, Codrin Ciubotariu wrote:
> Enable drivers needed for Microchip's PDMC and PDM microphones.
> 
> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> ---
> 
> Changes in v2,v3:
>   - none;
> 
>   arch/arm/configs/sama7_defconfig | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/configs/sama7_defconfig b/arch/arm/configs/sama7_defconfig
> index 0368068e04d9..bc29badab890 100644
> --- a/arch/arm/configs/sama7_defconfig
> +++ b/arch/arm/configs/sama7_defconfig
> @@ -138,6 +138,8 @@ CONFIG_SND_SOC_MIKROE_PROTO=m
>   CONFIG_SND_MCHP_SOC_I2S_MCC=y
>   CONFIG_SND_MCHP_SOC_SPDIFTX=y
>   CONFIG_SND_MCHP_SOC_SPDIFRX=y
> +CONFIG_SND_MCHP_SOC_PDMC=y
> +CONFIG_SND_SOC_DMIC=y

I'm fine with that, but I see that some Kconfig entries "select" this 
SND_SOC_DMIC directly (amd, intel, mediatek, stm).
If it's absolutely needed for PDMC to work, what about doing the same as 
it would prevent some broken configurations?

Regards,
   Nicolas

>   CONFIG_SND_SOC_PCM5102A=y
>   CONFIG_SND_SOC_SPDIF=y
>   CONFIG_SND_SIMPLE_CARD=y


-- 
Nicolas Ferre

Re: [PATCH v3 6/6] ARM: configs: at91: sama7_defconfig: add MCHP PDMC and DMIC drivers

[Codrin.Ciubotariu]

On 05.05.2022 16:58, Nicolas Ferre wrote:
> On 07/03/2022 at 13:22, Codrin Ciubotariu wrote:
>> Enable drivers needed for Microchip's PDMC and PDM microphones.
>>
>> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
>> ---
>>
>> Changes in v2,v3:
>>   - none;
>>
>>   arch/arm/configs/sama7_defconfig | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm/configs/sama7_defconfig 
>> b/arch/arm/configs/sama7_defconfig
>> index 0368068e04d9..bc29badab890 100644
>> --- a/arch/arm/configs/sama7_defconfig
>> +++ b/arch/arm/configs/sama7_defconfig
>> @@ -138,6 +138,8 @@ CONFIG_SND_SOC_MIKROE_PROTO=m
>>   CONFIG_SND_MCHP_SOC_I2S_MCC=y
>>   CONFIG_SND_MCHP_SOC_SPDIFTX=y
>>   CONFIG_SND_MCHP_SOC_SPDIFRX=y
>> +CONFIG_SND_MCHP_SOC_PDMC=y
>> +CONFIG_SND_SOC_DMIC=y
> 
> I'm fine with that, but I see that some Kconfig entries "select" this 
> SND_SOC_DMIC directly (amd, intel, mediatek, stm).
> If it's absolutely needed for PDMC to work, what about doing the same as 
> it would prevent some broken configurations?

The only way it makes sense to me to have this driver selected somewhere 
is in a sound card driver, used for a specific board, which we know it 
has PDM microphones. Since, for now, we use the simple sound card for 
our audio interfaces, we have no place to add this select.
The reason I do not like to add this select under the controller driver, 
as some of the vendors did, is because, in the future, we might have 
different PDM microphones that might not work with SND_SOC_DMIC and 
might need a different driver.
I don't have a strong opinion on this. If you think I am overthinking, 
please let me know and I will change this.

Best regards,
Codrin

Re: [PATCH v3 6/6] ARM: configs: at91: sama7_defconfig: add MCHP PDMC and DMIC drivers

[Mark Brown]

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

On Thu, May 05, 2022 at 02:47:04PM +0000, Codrin.Ciubotariu@microchip.com wrote:
> On 05.05.2022 16:58, Nicolas Ferre wrote:

> > I'm fine with that, but I see that some Kconfig entries "select" this 
> > SND_SOC_DMIC directly (amd, intel, mediatek, stm).
> > If it's absolutely needed for PDMC to work, what about doing the same as 
> > it would prevent some broken configurations?

> The only way it makes sense to me to have this driver selected somewhere 
> is in a sound card driver, used for a specific board, which we know it 
> has PDM microphones. Since, for now, we use the simple sound card for 
> our audio interfaces, we have no place to add this select.
> The reason I do not like to add this select under the controller driver, 
> as some of the vendors did, is because, in the future, we might have 
> different PDM microphones that might not work with SND_SOC_DMIC and 
> might need a different driver.
> I don't have a strong opinion on this. If you think I am overthinking, 
> please let me know and I will change this.

It's unlikely but possible that there could be some other device
connected (eg, you could have a DSP or something that generates PDM
output).  If the driver doesn't directly instantiate the DMIC itself
then it's probably reasonable for it to be user controllable if the DMIC
driver is there.

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

Re: [PATCH v3 6/6] ARM: configs: at91: sama7_defconfig: add MCHP PDMC and DMIC drivers

[Nicolas Ferre]

On 05/05/2022 at 17:01, Mark Brown wrote:
> On Thu, May 05, 2022 at 02:47:04PM +0000,Codrin.Ciubotariu@microchip.com  wrote:
>> On 05.05.2022 16:58, Nicolas Ferre wrote:
>>> I'm fine with that, but I see that some Kconfig entries "select" this
>>> SND_SOC_DMIC directly (amd, intel, mediatek, stm).
>>> If it's absolutely needed for PDMC to work, what about doing the same as
>>> it would prevent some broken configurations?
>> The only way it makes sense to me to have this driver selected somewhere
>> is in a sound card driver, used for a specific board, which we know it
>> has PDM microphones. Since, for now, we use the simple sound card for
>> our audio interfaces, we have no place to add this select.
>> The reason I do not like to add this select under the controller driver,
>> as some of the vendors did, is because, in the future, we might have
>> different PDM microphones that might not work with SND_SOC_DMIC and
>> might need a different driver.
>> I don't have a strong opinion on this. If you think I am overthinking,
>> please let me know and I will change this.
> It's unlikely but possible that there could be some other device
> connected (eg, you could have a DSP or something that generates PDM
> output).  If the driver doesn't directly instantiate the DMIC itself
> then it's probably reasonable for it to be user controllable if the DMIC
> driver is there.

Fair enough, It makes perfect sense to me as it is then.
Thanks for the feedback!

Best regards,
   Nicolas

-- 
Nicolas Ferre