Add driver module for PDM controller on ACP IP block. Expose dai
ops to configure ACP_WOV_PDM_BLOCK registers on ACP. Such dai ops
will be used by platform specific driver module to register dmic
related dai with ASoC.
Signed-off-by: Ajit Kumar Pandey <[email protected]>
---
sound/soc/amd/acp/Kconfig | 3 +
sound/soc/amd/acp/Makefile | 2 +
sound/soc/amd/acp/acp-pdm.c | 181 +++++++++++++++++++++++++++
sound/soc/amd/acp/amd.h | 9 +-
sound/soc/amd/acp/chip_offset_byte.h | 20 +++
5 files changed, 214 insertions(+), 1 deletion(-)
create mode 100644 sound/soc/amd/acp/acp-pdm.c
diff --git a/sound/soc/amd/acp/Kconfig b/sound/soc/amd/acp/Kconfig
index d5838df3064b..2e6d0259f2e9 100644
--- a/sound/soc/amd/acp/Kconfig
+++ b/sound/soc/amd/acp/Kconfig
@@ -15,6 +15,9 @@ config SND_SOC_AMD_ACP_COMMON
if SND_SOC_AMD_ACP_COMMON
+config SND_SOC_AMD_ACP_PDM
+ tristate
+
config SND_SOC_AMD_ACP_I2S
tristate
diff --git a/sound/soc/amd/acp/Makefile b/sound/soc/amd/acp/Makefile
index 16c144c2965c..66cac95432f6 100644
--- a/sound/soc/amd/acp/Makefile
+++ b/sound/soc/amd/acp/Makefile
@@ -7,6 +7,7 @@
#common acp driver
snd-acp-pcm-objs := acp-platform.o
snd-acp-i2s-objs := acp-i2s.o
+snd-acp-pdm-objs := acp-pdm.o
#platform specific driver
snd-acp-renoir-objs := acp-renoir.o
@@ -18,6 +19,7 @@ snd-acp-sof-mach-objs := acp-sof-mach.o
obj-$(CONFIG_SND_SOC_AMD_ACP_PCM) += snd-acp-pcm.o
obj-$(CONFIG_SND_SOC_AMD_ACP_I2S) += snd-acp-i2s.o
+obj-$(CONFIG_SND_SOC_AMD_ACP_PDM) += snd-acp-pdm.o
obj-$(CONFIG_SND_AMD_ASOC_RENOIR) += snd-acp-renoir.o
diff --git a/sound/soc/amd/acp/acp-pdm.c b/sound/soc/amd/acp/acp-pdm.c
new file mode 100644
index 000000000000..cb9bbd795eee
--- /dev/null
+++ b/sound/soc/amd/acp/acp-pdm.c
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+//
+// This file is provided under a dual BSD/GPLv2 license. When using or
+// redistributing this file, you may do so under either license.
+//
+// Copyright(c) 2021 Advanced Micro Devices, Inc.
+//
+// Authors: Ajit Kumar Pandey <[email protected]>
+// Vijendar Mukunda <[email protected]>
+//
+
+/*
+ * Generic Hardware interface for ACP Audio PDM controller
+ */
+
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/soc-dai.h>
+#include <linux/dma-mapping.h>
+
+#include "amd.h"
+
+#define DRV_NAME "acp-pdm"
+
+#define PDM_DMA_STAT 0x10
+#define PDM_DMA_INTR_MASK 0x10000
+#define PDM_DEC_64 0x2
+#define PDM_CLK_FREQ_MASK 0x07
+#define PDM_MISC_CTRL_MASK 0x10
+#define PDM_ENABLE 0x01
+#define PDM_DISABLE 0x00
+#define DMA_EN_MASK 0x02
+#define DELAY_US 5
+#define PDM_TIMEOUT 1000
+
+static int acp_dmic_dai_trigger(struct snd_pcm_substream *substream,
+ int cmd, struct snd_soc_dai *dai)
+{
+ struct acp_stream *stream = substream->runtime->private_data;
+ struct device *dev = dai->component->dev;
+ struct acp_dev_data *adata = dev_get_drvdata(dev);
+ u32 physical_addr, size_dmic, period_bytes;
+ unsigned int dma_enable;
+ int ret = 0;
+
+ period_bytes = frames_to_bytes(substream->runtime,
+ substream->runtime->period_size);
+ size_dmic = frames_to_bytes(substream->runtime,
+ substream->runtime->buffer_size);
+
+ physical_addr = stream->reg_offset + MEM_WINDOW_START;
+
+ /* Init DMIC Ring buffer */
+ writel(physical_addr, adata->acp_base + ACP_WOV_RX_RINGBUFADDR);
+ writel(size_dmic, adata->acp_base + ACP_WOV_RX_RINGBUFSIZE);
+ writel(period_bytes, adata->acp_base + ACP_WOV_RX_INTR_WATERMARK_SIZE);
+ writel(0x01, adata->acp_base + ACPAXI2AXI_ATU_CTRL);
+
+ switch (cmd) {
+ case SNDRV_PCM_TRIGGER_START:
+ case SNDRV_PCM_TRIGGER_RESUME:
+ case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+ dma_enable = readl(adata->acp_base + ACP_WOV_PDM_DMA_ENABLE);
+ if (!(dma_enable & DMA_EN_MASK)) {
+ writel(PDM_ENABLE, adata->acp_base + ACP_WOV_PDM_ENABLE);
+ writel(PDM_ENABLE, adata->acp_base + ACP_WOV_PDM_DMA_ENABLE);
+ }
+
+ ret = readl_poll_timeout_atomic(adata->acp_base + ACP_WOV_PDM_DMA_ENABLE,
+ dma_enable, (dma_enable & DMA_EN_MASK),
+ DELAY_US, PDM_TIMEOUT);
+ break;
+ case SNDRV_PCM_TRIGGER_STOP:
+ case SNDRV_PCM_TRIGGER_SUSPEND:
+ case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+ dma_enable = readl(adata->acp_base + ACP_WOV_PDM_DMA_ENABLE);
+ if ((dma_enable & DMA_EN_MASK)) {
+ writel(PDM_DISABLE, adata->acp_base + ACP_WOV_PDM_ENABLE);
+ writel(PDM_DISABLE, adata->acp_base + ACP_WOV_PDM_DMA_ENABLE);
+
+ }
+
+ ret = readl_poll_timeout_atomic(adata->acp_base + ACP_WOV_PDM_DMA_ENABLE,
+ dma_enable, !(dma_enable & DMA_EN_MASK),
+ DELAY_US, PDM_TIMEOUT);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+
+static int acp_dmic_hwparams(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *hwparams, struct snd_soc_dai *dai)
+{
+ struct device *dev = dai->component->dev;
+ struct acp_dev_data *adata = dev_get_drvdata(dev);
+ unsigned int dmic_ctrl, channels, ch_mask;
+
+ /* Enable default DMIC clk */
+ writel(PDM_CLK_FREQ_MASK, adata->acp_base + ACP_WOV_CLK_CTRL);
+ dmic_ctrl = readl(adata->acp_base + ACP_WOV_MISC_CTRL);
+ dmic_ctrl |= PDM_MISC_CTRL_MASK;
+ writel(dmic_ctrl, adata->acp_base + ACP_WOV_MISC_CTRL);
+
+ channels = params_channels(hwparams);
+ switch (channels) {
+ case 2:
+ ch_mask = 0;
+ break;
+ case 4:
+ ch_mask = 1;
+ break;
+ case 6:
+ ch_mask = 2;
+ break;
+ default:
+ dev_err(dev, "Invalid channels %d\n", channels);
+ return -EINVAL;
+ }
+
+ if (params_format(hwparams) != SNDRV_PCM_FORMAT_S32_LE) {
+ dev_err(dai->dev, "Invalid format:%d\n", params_format(hwparams));
+ return -EINVAL;
+ }
+
+ writel(ch_mask, adata->acp_base + ACP_WOV_PDM_NO_OF_CHANNELS);
+ writel(PDM_DEC_64, adata->acp_base + ACP_WOV_PDM_DECIMATION_FACTOR);
+
+ return 0;
+}
+
+static int acp_dmic_dai_startup(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct acp_stream *stream = substream->runtime->private_data;
+ struct device *dev = dai->component->dev;
+ struct acp_dev_data *adata = dev_get_drvdata(dev);
+ u32 ext_int_ctrl;
+
+ stream->dai_id = DMIC_INSTANCE;
+ stream->irq_bit = BIT(PDM_DMA_STAT);
+ stream->pte_offset = ACP_SRAM_PDM_PTE_OFFSET;
+
+ /* Enable DMIC Interrupts */
+ ext_int_ctrl = readl(adata->acp_base + ACP_EXTERNAL_INTR_CNTL);
+ ext_int_ctrl |= PDM_DMA_INTR_MASK;
+ writel(ext_int_ctrl, adata->acp_base + ACP_EXTERNAL_INTR_CNTL);
+
+ return 0;
+}
+
+static void acp_dmic_dai_shutdown(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct device *dev = dai->component->dev;
+ struct acp_dev_data *adata = dev_get_drvdata(dev);
+ u32 ext_int_ctrl;
+
+ /* Disable DMIC interrrupts */
+ ext_int_ctrl = readl(adata->acp_base + ACP_EXTERNAL_INTR_CNTL);
+ ext_int_ctrl |= ~PDM_DMA_INTR_MASK;
+ writel(ext_int_ctrl, adata->acp_base + ACP_EXTERNAL_INTR_CNTL);
+}
+
+const struct snd_soc_dai_ops acp_dmic_dai_ops = {
+ .hw_params = acp_dmic_hwparams,
+ .trigger = acp_dmic_dai_trigger,
+ .startup = acp_dmic_dai_startup,
+ .shutdown = acp_dmic_dai_shutdown,
+};
+EXPORT_SYMBOL_NS_GPL(acp_dmic_dai_ops, SND_SOC_ACP_COMMON);
+
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:"DRV_NAME);
diff --git a/sound/soc/amd/acp/amd.h b/sound/soc/amd/acp/amd.h
index 8eee3d34774b..567355209a5c 100644
--- a/sound/soc/amd/acp/amd.h
+++ b/sound/soc/amd/acp/amd.h
@@ -17,8 +17,9 @@
#define I2S_SP_INSTANCE 0x00
#define I2S_BT_INSTANCE 0x01
+#define DMIC_INSTANCE 0x02
-#define MEM_WINDOW_START 0x4000000
+#define MEM_WINDOW_START 0x4080000
#define ACP_I2S_REG_START 0x1242400
#define ACP_I2S_REG_END 0x1242810
@@ -38,6 +39,7 @@
#define ACP_SRAM_SP_CP_PTE_OFFSET 0x100
#define ACP_SRAM_BT_PB_PTE_OFFSET 0x200
#define ACP_SRAM_BT_CP_PTE_OFFSET 0x300
+#define ACP_SRAM_PDM_PTE_OFFSET 0x400
#define PAGE_SIZE_4K_ENABLE 0x2
#define I2S_SP_TX_MEM_WINDOW_START 0x4000000
@@ -96,6 +98,7 @@ struct acp_dev_data {
};
extern const struct snd_soc_dai_ops asoc_acp_cpu_dai_ops;
+extern const struct snd_soc_dai_ops acp_dmic_dai_ops;
int asoc_acp_i2s_probe(struct snd_soc_dai *dai);
int acp_platform_register(struct device *dev);
@@ -131,6 +134,10 @@ static inline u64 acp_get_byte_count(struct acp_dev_data *adata, int dai_id, int
high = readl(adata->acp_base + ACP_I2S_RX_LINEARPOSITIONCNTR_HIGH);
low = readl(adata->acp_base + ACP_I2S_RX_LINEARPOSITIONCNTR_LOW);
break;
+ case DMIC_INSTANCE:
+ high = readl(adata->acp_base + ACP_WOV_RX_LINEARPOSITIONCNTR_HIGH);
+ low = readl(adata->acp_base + ACP_WOV_RX_LINEARPOSITIONCNTR_LOW);
+ break;
default:
dev_err(adata->dev, "Invalid dai id %x\n", dai_id);
return -EINVAL;
diff --git a/sound/soc/amd/acp/chip_offset_byte.h b/sound/soc/amd/acp/chip_offset_byte.h
index c7f77e975dc7..e38589a142e9 100644
--- a/sound/soc/amd/acp/chip_offset_byte.h
+++ b/sound/soc/amd/acp/chip_offset_byte.h
@@ -73,4 +73,24 @@
#define ACP_BTTDM_ITER 0x280C
#define ACP_BTTDM_TXFRMT 0x2810
+/* Registers from ACP_WOV_PDM block */
+
+#define ACP_WOV_PDM_ENABLE 0x2C04
+#define ACP_WOV_PDM_DMA_ENABLE 0x2C08
+#define ACP_WOV_RX_RINGBUFADDR 0x2C0C
+#define ACP_WOV_RX_RINGBUFSIZE 0x2C10
+#define ACP_WOV_RX_LINKPOSITIONCNTR 0x2C14
+#define ACP_WOV_RX_LINEARPOSITIONCNTR_HIGH 0x2C18
+#define ACP_WOV_RX_LINEARPOSITIONCNTR_LOW 0x2C1C
+#define ACP_WOV_RX_INTR_WATERMARK_SIZE 0x2C20
+#define ACP_WOV_PDM_FIFO_FLUSH 0x2C24
+#define ACP_WOV_PDM_NO_OF_CHANNELS 0x2C28
+#define ACP_WOV_PDM_DECIMATION_FACTOR 0x2C2C
+#define ACP_WOV_PDM_VAD_CTRL 0x2C30
+#define ACP_WOV_BUFFER_STATUS 0x2C58
+#define ACP_WOV_MISC_CTRL 0x2C5C
+#define ACP_WOV_CLK_CTRL 0x2C60
+#define ACP_PDM_VAD_DYNAMIC_CLK_GATING_EN 0x2C64
+#define ACP_WOV_ERROR_STATUS_REGISTER 0x2C68
+
#endif
--
2.25.1
couple of nit-picks:
> diff --git a/sound/soc/amd/acp/acp-pdm.c b/sound/soc/amd/acp/acp-pdm.c
> new file mode 100644
> index 000000000000..cb9bbd795eee
> --- /dev/null
> +++ b/sound/soc/amd/acp/acp-pdm.c
> @@ -0,0 +1,181 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
> +//
> +// This file is provided under a dual BSD/GPLv2 license. When using or
> +// redistributing this file, you may do so under either license.
> +//
> +// Copyright(c) 2021 Advanced Micro Devices, Inc.
2022?
> +//
> +// Authors: Ajit Kumar Pandey <[email protected]>
> +// Vijendar Mukunda <[email protected]>
> +//
> +
> +/*
> + * Generic Hardware interface for ACP Audio PDM controller
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +#include <sound/soc-dai.h>
> +#include <linux/dma-mapping.h>
alphabetical order?
> +
> +#include "amd.h"
> +
> +#define DRV_NAME "acp-pdm"
> +
> +#define PDM_DMA_STAT 0x10
> +#define PDM_DMA_INTR_MASK 0x10000
> +#define PDM_DEC_64 0x2
> +#define PDM_CLK_FREQ_MASK 0x07
> +#define PDM_MISC_CTRL_MASK 0x10
> +#define PDM_ENABLE 0x01
> +#define PDM_DISABLE 0x00
> +#define DMA_EN_MASK 0x02
> +#define DELAY_US 5
> +#define PDM_TIMEOUT 1000
> +
> +static int acp_dmic_dai_trigger(struct snd_pcm_substream *substream,
> + int cmd, struct snd_soc_dai *dai)
> +{
> + struct acp_stream *stream = substream->runtime->private_data;
> + struct device *dev = dai->component->dev;
> + struct acp_dev_data *adata = dev_get_drvdata(dev);
> + u32 physical_addr, size_dmic, period_bytes;
> + unsigned int dma_enable;
> + int ret = 0;
> +
> + period_bytes = frames_to_bytes(substream->runtime,
> + substream->runtime->period_size);
> + size_dmic = frames_to_bytes(substream->runtime,
> + substream->runtime->buffer_size);
> +
> + physical_addr = stream->reg_offset + MEM_WINDOW_START;
> +
> + /* Init DMIC Ring buffer */
> + writel(physical_addr, adata->acp_base + ACP_WOV_RX_RINGBUFADDR);
> + writel(size_dmic, adata->acp_base + ACP_WOV_RX_RINGBUFSIZE);
> + writel(period_bytes, adata->acp_base + ACP_WOV_RX_INTR_WATERMARK_SIZE);
> + writel(0x01, adata->acp_base + ACPAXI2AXI_ATU_CTRL);
could this be done in a .prepare step?
> +
> + switch (cmd) {
> + case SNDRV_PCM_TRIGGER_START:
> + case SNDRV_PCM_TRIGGER_RESUME:
> + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> + dma_enable = readl(adata->acp_base + ACP_WOV_PDM_DMA_ENABLE);
> + if (!(dma_enable & DMA_EN_MASK)) {
> + writel(PDM_ENABLE, adata->acp_base + ACP_WOV_PDM_ENABLE);
> + writel(PDM_ENABLE, adata->acp_base + ACP_WOV_PDM_DMA_ENABLE);
> + }
> +
> + ret = readl_poll_timeout_atomic(adata->acp_base + ACP_WOV_PDM_DMA_ENABLE,
> + dma_enable, (dma_enable & DMA_EN_MASK),
> + DELAY_US, PDM_TIMEOUT);
> + break;
> + case SNDRV_PCM_TRIGGER_STOP:
> + case SNDRV_PCM_TRIGGER_SUSPEND:
> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> + dma_enable = readl(adata->acp_base + ACP_WOV_PDM_DMA_ENABLE);
> + if ((dma_enable & DMA_EN_MASK)) {
> + writel(PDM_DISABLE, adata->acp_base + ACP_WOV_PDM_ENABLE);
> + writel(PDM_DISABLE, adata->acp_base + ACP_WOV_PDM_DMA_ENABLE);
> +
> + }
> +
> + ret = readl_poll_timeout_atomic(adata->acp_base + ACP_WOV_PDM_DMA_ENABLE,
> + dma_enable, !(dma_enable & DMA_EN_MASK),
> + DELAY_US, PDM_TIMEOUT);
is the _atomic needed?
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static int acp_dmic_hwparams(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *hwparams, struct snd_soc_dai *dai)
> +{
> + struct device *dev = dai->component->dev;
> + struct acp_dev_data *adata = dev_get_drvdata(dev);
> + unsigned int dmic_ctrl, channels, ch_mask;
> +
> + /* Enable default DMIC clk */
> + writel(PDM_CLK_FREQ_MASK, adata->acp_base + ACP_WOV_CLK_CTRL);
> + dmic_ctrl = readl(adata->acp_base + ACP_WOV_MISC_CTRL);
> + dmic_ctrl |= PDM_MISC_CTRL_MASK;
> + writel(dmic_ctrl, adata->acp_base + ACP_WOV_MISC_CTRL);
.hw_params can be called multiple times, should this clock handling be
done in a .prepare step?
Or alternatively in .startup - this doesn't seem to depend on hardware
parameters?
> +
> + channels = params_channels(hwparams);
> + switch (channels) {
> + case 2:
> + ch_mask = 0;
> + break;
> + case 4:
> + ch_mask = 1;
> + break;
> + case 6:
> + ch_mask = 2;
> + break;
> + default:
> + dev_err(dev, "Invalid channels %d\n", channels);
> + return -EINVAL;
> + }
> +
> + if (params_format(hwparams) != SNDRV_PCM_FORMAT_S32_LE) {
> + dev_err(dai->dev, "Invalid format:%d\n", params_format(hwparams));
> + return -EINVAL;
> + }
> +
> + writel(ch_mask, adata->acp_base + ACP_WOV_PDM_NO_OF_CHANNELS);
> + writel(PDM_DEC_64, adata->acp_base + ACP_WOV_PDM_DECIMATION_FACTOR);
> +
> + return 0;
> +}
> +MODULE_LICENSE("GPL v2");
"GPL" is enough
On 1/14/2022 12:04 AM, Pierre-Louis Bossart wrote:
> [CAUTION: External Email]
>
> couple of nit-picks:
>
>
>> diff --git a/sound/soc/amd/acp/acp-pdm.c b/sound/soc/amd/acp/acp-pdm.c
>> new file mode 100644
>> index 000000000000..cb9bbd795eee
>> --- /dev/null
>> +++ b/sound/soc/amd/acp/acp-pdm.c
>> @@ -0,0 +1,181 @@
>> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
>> +//
>> +// This file is provided under a dual BSD/GPLv2 license. When using or
>> +// redistributing this file, you may do so under either license.
>> +//
>> +// Copyright(c) 2021 Advanced Micro Devices, Inc.
>
> 2022?
>
>> +//
>> +// Authors: Ajit Kumar Pandey <[email protected]>
>> +// Vijendar Mukunda <[email protected]>
>> +//
>> +
>> +/*
>> + * Generic Hardware interface for ACP Audio PDM controller
>> + */
>> +
>> +#include <linux/platform_device.h>
>> +#include <linux/module.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <sound/pcm_params.h>
>> +#include <sound/soc.h>
>> +#include <sound/soc-dai.h>
>> +#include <linux/dma-mapping.h>
>
> alphabetical order?
>
>> +
>> +#include "amd.h"
>> +
>> +#define DRV_NAME "acp-pdm"
>> +
>> +#define PDM_DMA_STAT 0x10
>> +#define PDM_DMA_INTR_MASK 0x10000
>> +#define PDM_DEC_64 0x2
>> +#define PDM_CLK_FREQ_MASK 0x07
>> +#define PDM_MISC_CTRL_MASK 0x10
>> +#define PDM_ENABLE 0x01
>> +#define PDM_DISABLE 0x00
>> +#define DMA_EN_MASK 0x02
>> +#define DELAY_US 5
>> +#define PDM_TIMEOUT 1000
>> +
>> +static int acp_dmic_dai_trigger(struct snd_pcm_substream *substream,
>> + int cmd, struct snd_soc_dai *dai)
>> +{
>> + struct acp_stream *stream = substream->runtime->private_data;
>> + struct device *dev = dai->component->dev;
>> + struct acp_dev_data *adata = dev_get_drvdata(dev);
>> + u32 physical_addr, size_dmic, period_bytes;
>> + unsigned int dma_enable;
>> + int ret = 0;
>> +
>> + period_bytes = frames_to_bytes(substream->runtime,
>> + substream->runtime->period_size);
>> + size_dmic = frames_to_bytes(substream->runtime,
>> + substream->runtime->buffer_size);
>> +
>> + physical_addr = stream->reg_offset + MEM_WINDOW_START;
>> +
>> + /* Init DMIC Ring buffer */
>> + writel(physical_addr, adata->acp_base + ACP_WOV_RX_RINGBUFADDR);
>> + writel(size_dmic, adata->acp_base + ACP_WOV_RX_RINGBUFSIZE);
>> + writel(period_bytes, adata->acp_base + ACP_WOV_RX_INTR_WATERMARK_SIZE);
>> + writel(0x01, adata->acp_base + ACPAXI2AXI_ATU_CTRL);
>
> could this be done in a .prepare step?
>
>> +
>> + switch (cmd) {
>> + case SNDRV_PCM_TRIGGER_START:
>> + case SNDRV_PCM_TRIGGER_RESUME:
>> + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>> + dma_enable = readl(adata->acp_base + ACP_WOV_PDM_DMA_ENABLE);
>> + if (!(dma_enable & DMA_EN_MASK)) {
>> + writel(PDM_ENABLE, adata->acp_base + ACP_WOV_PDM_ENABLE);
>> + writel(PDM_ENABLE, adata->acp_base + ACP_WOV_PDM_DMA_ENABLE);
>> + }
>> +
>> + ret = readl_poll_timeout_atomic(adata->acp_base + ACP_WOV_PDM_DMA_ENABLE,
>> + dma_enable, (dma_enable & DMA_EN_MASK),
>> + DELAY_US, PDM_TIMEOUT);
>> + break;
>> + case SNDRV_PCM_TRIGGER_STOP:
>> + case SNDRV_PCM_TRIGGER_SUSPEND:
>> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>> + dma_enable = readl(adata->acp_base + ACP_WOV_PDM_DMA_ENABLE);
>> + if ((dma_enable & DMA_EN_MASK)) {
>> + writel(PDM_DISABLE, adata->acp_base + ACP_WOV_PDM_ENABLE);
>> + writel(PDM_DISABLE, adata->acp_base + ACP_WOV_PDM_DMA_ENABLE);
>> +
>> + }
>> +
>> + ret = readl_poll_timeout_atomic(adata->acp_base + ACP_WOV_PDM_DMA_ENABLE,
>> + dma_enable, !(dma_enable & DMA_EN_MASK),
>> + DELAY_US, PDM_TIMEOUT);
>
> is the _atomic needed?
>
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + break;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int acp_dmic_hwparams(struct snd_pcm_substream *substream,
>> + struct snd_pcm_hw_params *hwparams, struct snd_soc_dai *dai)
>> +{
>> + struct device *dev = dai->component->dev;
>> + struct acp_dev_data *adata = dev_get_drvdata(dev);
>> + unsigned int dmic_ctrl, channels, ch_mask;
>> +
>> + /* Enable default DMIC clk */
>> + writel(PDM_CLK_FREQ_MASK, adata->acp_base + ACP_WOV_CLK_CTRL);
>> + dmic_ctrl = readl(adata->acp_base + ACP_WOV_MISC_CTRL);
>> + dmic_ctrl |= PDM_MISC_CTRL_MASK;
>> + writel(dmic_ctrl, adata->acp_base + ACP_WOV_MISC_CTRL);
>
> .hw_params can be called multiple times, should this clock handling be
> done in a .prepare step?
>
> Or alternatively in .startup - this doesn't seem to depend on hardware
> parameters?
>
>> +
>> + channels = params_channels(hwparams);
>> + switch (channels) {
>> + case 2:
>> + ch_mask = 0;
>> + break;
>> + case 4:
>> + ch_mask = 1;
>> + break;
>> + case 6:
>> + ch_mask = 2;
>> + break;
>> + default:
>> + dev_err(dev, "Invalid channels %d\n", channels);
>> + return -EINVAL;
>> + }
>> +
>> + if (params_format(hwparams) != SNDRV_PCM_FORMAT_S32_LE) {
>> + dev_err(dai->dev, "Invalid format:%d\n", params_format(hwparams));
>> + return -EINVAL;
>> + }
>> +
>> + writel(ch_mask, adata->acp_base + ACP_WOV_PDM_NO_OF_CHANNELS);
>> + writel(PDM_DEC_64, adata->acp_base + ACP_WOV_PDM_DECIMATION_FACTOR);
>> +
>> + return 0;
>> +}
>
>> +MODULE_LICENSE("GPL v2");
>
> "GPL" is enough
>
>
Thanks, will try to adopt commnets in v3 patch chain