[PATCH v3 0/6] Add driver for SAMA7G5's PDMC
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
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);
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... on current master.
Sascha
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... 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
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... 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
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... 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
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__ */
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
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");
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";
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>,
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
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
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
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.
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
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
participants (6)
-
Codrin Ciubotariu
-
Codrin.Ciubotariu@microchip.com
-
Krzysztof Kozlowski
-
Mark Brown
-
Nicolas Ferre
-
Sascha Hauer