* [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-15 13:03 ` Ban Tao
0 siblings, 0 replies; 36+ messages in thread
From: Ban Tao @ 2021-06-15 13:03 UTC (permalink / raw)
To: lgirdwood, broonie, perex, tiwai, mripard, wens, jernej.skrabec,
fengzheng923, p.zabel, samuel, krzk
Cc: linux-kernel, alsa-devel, linux-arm-kernel, linux-sunxi
The Allwinner H6 and later SoCs have an DMIC block
which is capable of capture.
Signed-off-by: Ban Tao <fengzheng923@gmail.com>
---
MAINTAINERS | 7 +
sound/soc/sunxi/Kconfig | 8 +
sound/soc/sunxi/Makefile | 1 +
sound/soc/sunxi/sun50i-dmic.c | 408 ++++++++++++++++++++++++++++++++++
4 files changed, 424 insertions(+)
create mode 100644 sound/soc/sunxi/sun50i-dmic.c
diff --git a/MAINTAINERS b/MAINTAINERS
index b4094f07214e..1b87225c39b0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -760,6 +760,13 @@ L: linux-media@vger.kernel.org
S: Maintained
F: drivers/staging/media/sunxi/cedrus/
+ALLWINNER DMIC DRIVERS
+M: Ban Tao <fengzheng923@gmail.com>
+L: alsa-devel@alsa-project.org (moderated for non-subscribers)
+S: Maintained
+F: Documentation/devicetree/bindings/sound/allwinner,sun50i-h6-dmic.yaml
+F: sound/soc/sunxi/sun50i-dmic.c
+
ALPHA PORT
M: Richard Henderson <rth@twiddle.net>
M: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
index ddcaaa98d3cb..2a3bf7722e11 100644
--- a/sound/soc/sunxi/Kconfig
+++ b/sound/soc/sunxi/Kconfig
@@ -56,6 +56,14 @@ config SND_SUN4I_SPDIF
Say Y or M to add support for the S/PDIF audio block in the Allwinner
A10 and affiliated SoCs.
+config SND_SUN50I_DMIC
+ tristate "Allwinner H6 DMIC Support"
+ depends on (OF && ARCH_SUNXI) || COMPILE_TEST
+ select SND_SOC_GENERIC_DMAENGINE_PCM
+ help
+ Say Y or M to add support for the DMIC audio block in the Allwinner
+ H6 and affiliated SoCs.
+
config SND_SUN8I_ADDA_PR_REGMAP
tristate
select REGMAP
diff --git a/sound/soc/sunxi/Makefile b/sound/soc/sunxi/Makefile
index a86be340a076..4483fe9c94ef 100644
--- a/sound/soc/sunxi/Makefile
+++ b/sound/soc/sunxi/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_SND_SUN8I_CODEC_ANALOG) += sun8i-codec-analog.o
obj-$(CONFIG_SND_SUN50I_CODEC_ANALOG) += sun50i-codec-analog.o
obj-$(CONFIG_SND_SUN8I_CODEC) += sun8i-codec.o
obj-$(CONFIG_SND_SUN8I_ADDA_PR_REGMAP) += sun8i-adda-pr-regmap.o
+obj-$(CONFIG_SND_SUN50I_DMIC) += sun50i-dmic.o
diff --git a/sound/soc/sunxi/sun50i-dmic.c b/sound/soc/sunxi/sun50i-dmic.c
new file mode 100644
index 000000000000..78d512d93974
--- /dev/null
+++ b/sound/soc/sunxi/sun50i-dmic.c
@@ -0,0 +1,408 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * ALSA SoC DMIC Audio Layer
+ *
+ * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/of_device.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+#include <sound/dmaengine_pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+
+#define SUN50I_DMIC_EN_CTL (0x00)
+ #define SUN50I_DMIC_EN_CTL_GLOBE BIT(8)
+ #define SUN50I_DMIC_EN_CTL_CHAN(v) ((v) << 0)
+ #define SUN50I_DMIC_EN_CTL_CHAN_MASK GENMASK(7, 0)
+#define SUN50I_DMIC_SR (0x04)
+ #define SUN50I_DMIC_SR_SAMOLE_RATE(v) ((v) << 0)
+ #define SUN50I_DMIC_SR_SAMOLE_RATE_MASK GENMASK(2, 0)
+#define SUN50I_DMIC_CTL (0x08)
+ #define SUN50I_DMIC_CTL_OVERSAMPLE_RATE BIT(0)
+#define SUN50I_DMIC_DATA (0x10)
+#define SUN50I_DMIC_INTC (0x14)
+ #define SUN50I_DMIC_FIFO_DRQ_EN BIT(2)
+#define SUN50I_DMIC_INT_STA (0x18)
+ #define SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING BIT(1)
+ #define SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING BIT(0)
+#define SUN50I_DMIC_RXFIFO_CTL (0x1c)
+ #define SUN50I_DMIC_RXFIFO_CTL_FLUSH BIT(31)
+ #define SUN50I_DMIC_RXFIFO_CTL_MODE BIT(9)
+ #define SUN50I_DMIC_RXFIFO_CTL_RESOLUTION BIT(8)
+#define SUN50I_DMIC_CH_NUM (0x24)
+ #define SUN50I_DMIC_CH_NUM_N(v) ((v) << 0)
+ #define SUN50I_DMIC_CH_NUM_N_MASK GENMASK(2, 0)
+#define SUN50I_DMIC_CNT (0x2c)
+ #define SUN50I_DMIC_CNT_N BIT(0)
+#define SUN50I_DMIC_HPF_CTRL (0x38)
+#define SUN50I_DMIC_VERSION (0x50)
+
+
+struct sun50i_dmic_dev {
+ struct platform_device *pdev;
+ struct clk *dmic_clk;
+ struct clk *apb_clk;
+ struct reset_control *rst;
+ struct regmap *regmap;
+ struct snd_dmaengine_dai_dma_data dma_params_rx;
+ unsigned int chan_en;
+};
+
+struct dmic_rate {
+ unsigned int samplerate;
+ unsigned int rate_bit;
+};
+
+static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream *substream,
+ struct sun50i_dmic_dev *host, bool enable)
+{
+ if (enable) {
+ /* DRQ ENABLE */
+ regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
+ SUN50I_DMIC_FIFO_DRQ_EN,
+ SUN50I_DMIC_FIFO_DRQ_EN);
+ regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
+ SUN50I_DMIC_EN_CTL_CHAN_MASK,
+ SUN50I_DMIC_EN_CTL_CHAN(host->chan_en));
+ /* Global enable */
+ regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
+ SUN50I_DMIC_EN_CTL_GLOBE,
+ SUN50I_DMIC_EN_CTL_GLOBE);
+ } else {
+ /* DRQ DISABLE */
+ regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
+ SUN50I_DMIC_FIFO_DRQ_EN, 0);
+ regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
+ SUN50I_DMIC_EN_CTL_CHAN_MASK,
+ SUN50I_DMIC_EN_CTL_CHAN(0));
+ /* Global disable */
+ regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
+ SUN50I_DMIC_EN_CTL_GLOBE, 0);
+ }
+}
+
+static int sun50i_dmic_startup(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *cpu_dai)
+{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
+
+ /* only support capture */
+ if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
+ return -EINVAL;
+
+ regmap_write(host->regmap, SUN50I_DMIC_INT_STA,
+ SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING |
+ SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING);
+ regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
+ SUN50I_DMIC_RXFIFO_CTL_FLUSH,
+ SUN50I_DMIC_RXFIFO_CTL_FLUSH);
+ regmap_write(host->regmap, SUN50I_DMIC_CNT, SUN50I_DMIC_CNT_N);
+
+ return 0;
+}
+
+static int sun50i_dmic_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *cpu_dai)
+{
+ int i = 0;
+ unsigned long rate = params_rate(params);
+ unsigned int channels = params_channels(params);
+ struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
+ struct platform_device *pdev = host->pdev;
+ struct dmic_rate dmic_rate_s[] = {
+ {44100, 0x0},
+ {48000, 0x0},
+ {22050, 0x2},
+ {24000, 0x2},
+ {11025, 0x4},
+ {12000, 0x4},
+ {32000, 0x1},
+ {16000, 0x3},
+ {8000, 0x5},
+ };
+
+ /* DMIC num is N+1 */
+ regmap_update_bits(host->regmap, SUN50I_DMIC_CH_NUM,
+ SUN50I_DMIC_CH_NUM_N_MASK,
+ SUN50I_DMIC_CH_NUM_N(channels - 1));
+ host->chan_en = (1 << channels) - 1;
+ regmap_write(host->regmap, SUN50I_DMIC_HPF_CTRL, host->chan_en);
+
+ switch (params_format(params)) {
+ case SNDRV_PCM_FORMAT_S16_LE:
+ regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
+ SUN50I_DMIC_RXFIFO_CTL_MODE,
+ SUN50I_DMIC_RXFIFO_CTL_MODE);
+ regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
+ SUN50I_DMIC_RXFIFO_CTL_RESOLUTION, 0);
+ break;
+ case SNDRV_PCM_FORMAT_S24_LE:
+ regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
+ SUN50I_DMIC_RXFIFO_CTL_MODE, 0);
+ regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
+ SUN50I_DMIC_RXFIFO_CTL_RESOLUTION,
+ SUN50I_DMIC_RXFIFO_CTL_RESOLUTION);
+ break;
+ default:
+ dev_err(&pdev->dev, "Invalid format!\n");
+ return -EINVAL;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(dmic_rate_s); i++) {
+ if (dmic_rate_s[i].samplerate == rate) {
+ regmap_update_bits(host->regmap, SUN50I_DMIC_SR,
+ SUN50I_DMIC_SR_SAMOLE_RATE_MASK,
+ SUN50I_DMIC_SR_SAMOLE_RATE(dmic_rate_s[i].rate_bit));
+ break;
+ }
+ }
+ if (i == ARRAY_SIZE(dmic_rate_s)) {
+ dev_err(&pdev->dev, "Invalid rate!\n");
+ return -EINVAL;
+ }
+
+ /* oversamplerate adjust */
+ if (params_rate(params) >= 24000)
+ regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
+ SUN50I_DMIC_CTL_OVERSAMPLE_RATE,
+ SUN50I_DMIC_CTL_OVERSAMPLE_RATE);
+ else
+ regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
+ SUN50I_DMIC_CTL_OVERSAMPLE_RATE, 0);
+
+ return 0;
+}
+
+static int sun50i_dmic_trigger(struct snd_pcm_substream *substream, int cmd,
+ struct snd_soc_dai *dai)
+{
+ int ret = 0;
+ struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
+
+ if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
+ return -EINVAL;
+
+ switch (cmd) {
+ case SNDRV_PCM_TRIGGER_START:
+ case SNDRV_PCM_TRIGGER_RESUME:
+ case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+ sun50i_snd_rxctrl_enable(substream, host, true);
+ break;
+
+ case SNDRV_PCM_TRIGGER_STOP:
+ case SNDRV_PCM_TRIGGER_SUSPEND:
+ case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+ sun50i_snd_rxctrl_enable(substream, host, false);
+ break;
+
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ return ret;
+}
+
+static int sun50i_dmic_set_sysclk(struct snd_soc_dai *dai, int clk_id,
+ unsigned int freq, int dir)
+{
+ struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
+
+ if (clk_set_rate(host->dmic_clk, freq)) {
+ dev_err(dai->dev, "Freq : %u not support\n", freq);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int sun50i_dmic_soc_dai_probe(struct snd_soc_dai *dai)
+{
+ struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
+
+ snd_soc_dai_init_dma_data(dai, NULL, &host->dma_params_rx);
+
+ return 0;
+}
+
+static const struct snd_soc_dai_ops sun50i_dmic_dai_ops = {
+ .startup = sun50i_dmic_startup,
+ .trigger = sun50i_dmic_trigger,
+ .hw_params = sun50i_dmic_hw_params,
+ .set_sysclk = sun50i_dmic_set_sysclk,
+};
+
+static const struct regmap_config sun50i_dmic_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = SUN50I_DMIC_VERSION,
+ .cache_type = REGCACHE_NONE,
+};
+
+#define SUN50I_DMIC_RATES (SNDRV_PCM_RATE_8000_48000 | SNDRV_PCM_RATE_KNOT)
+#define SUN50I_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE)
+
+static struct snd_soc_dai_driver sun50i_dmic_dai = {
+ .capture = {
+ .channels_min = 1,
+ .channels_max = 8,
+ .rates = SUN50I_DMIC_RATES,
+ .formats = SUN50I_FORMATS,
+ },
+ .probe = sun50i_dmic_soc_dai_probe,
+ .ops = &sun50i_dmic_dai_ops,
+ .name = "dmic",
+};
+
+static const struct of_device_id sun50i_dmic_of_match[] = {
+ {
+ .compatible = "allwinner,sun50i-h6-dmic",
+ },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, sun50i_dmic_of_match);
+
+static const struct snd_soc_component_driver sun50i_dmic_component = {
+ .name = "sun50i-dmic",
+};
+
+static int sun50i_dmic_runtime_suspend(struct device *dev)
+{
+ struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(host->dmic_clk);
+ clk_disable_unprepare(host->apb_clk);
+
+ return 0;
+}
+
+static int sun50i_dmic_runtime_resume(struct device *dev)
+{
+ struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
+ int ret;
+
+ ret = clk_prepare_enable(host->dmic_clk);
+ if (ret)
+ return ret;
+ ret = clk_prepare_enable(host->apb_clk);
+ if (ret)
+ clk_disable_unprepare(host->dmic_clk);
+
+ return ret;
+}
+
+static int sun50i_dmic_probe(struct platform_device *pdev)
+{
+ struct sun50i_dmic_dev *host;
+ struct resource *res;
+ int ret;
+ void __iomem *base;
+
+ host = devm_kzalloc(&pdev->dev, sizeof(*host), GFP_KERNEL);
+ if (!host)
+ return -ENOMEM;
+
+ host->pdev = pdev;
+
+ /* Get the addresses */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(base))
+ return dev_err_probe(&pdev->dev, PTR_ERR(base),
+ "get resource failed.\n");
+
+ host->regmap = devm_regmap_init_mmio(&pdev->dev, base,
+ &sun50i_dmic_regmap_config);
+
+ /* Clocks */
+ host->apb_clk = devm_clk_get(&pdev->dev, "apb");
+ if (IS_ERR(host->apb_clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(host->apb_clk),
+ "failed to get apb clock.\n");
+
+ host->dmic_clk = devm_clk_get(&pdev->dev, "dmic");
+ if (IS_ERR(host->dmic_clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(host->dmic_clk),
+ "failed to get dmic clock.\n");
+
+ host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
+ host->dma_params_rx.maxburst = 8;
+ host->dma_params_rx.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+
+ platform_set_drvdata(pdev, host);
+
+ host->rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
+ if (IS_ERR(host->rst) && PTR_ERR(host->rst) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ dev_err(&pdev->dev, "Failed to get reset: %d\n", ret);
+ return ret;
+ }
+ if (!IS_ERR(host->rst))
+ reset_control_deassert(host->rst);
+
+ ret = devm_snd_soc_register_component(&pdev->dev,
+ &sun50i_dmic_component, &sun50i_dmic_dai, 1);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to register component.\n");
+
+ pm_runtime_enable(&pdev->dev);
+ if (!pm_runtime_enabled(&pdev->dev)) {
+ ret = sun50i_dmic_runtime_resume(&pdev->dev);
+ if (ret)
+ goto err_unregister;
+ }
+
+ ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
+ if (ret)
+ goto err_suspend;
+
+ return 0;
+err_suspend:
+ if (!pm_runtime_status_suspended(&pdev->dev))
+ sun50i_dmic_runtime_suspend(&pdev->dev);
+err_unregister:
+ pm_runtime_disable(&pdev->dev);
+ return ret;
+}
+
+static int sun50i_dmic_remove(struct platform_device *pdev)
+{
+ pm_runtime_disable(&pdev->dev);
+ if (!pm_runtime_status_suspended(&pdev->dev))
+ sun50i_dmic_runtime_suspend(&pdev->dev);
+
+ return 0;
+}
+
+static const struct dev_pm_ops sun50i_dmic_pm = {
+ SET_RUNTIME_PM_OPS(sun50i_dmic_runtime_suspend,
+ sun50i_dmic_runtime_resume, NULL)
+};
+
+static struct platform_driver sun50i_dmic_driver = {
+ .driver = {
+ .name = "sun50i-dmic",
+ .of_match_table = of_match_ptr(sun50i_dmic_of_match),
+ .pm = &sun50i_dmic_pm,
+ },
+ .probe = sun50i_dmic_probe,
+ .remove = sun50i_dmic_remove,
+};
+
+module_platform_driver(sun50i_dmic_driver);
+
+MODULE_DESCRIPTION("Allwinner sun50i DMIC SoC Interface");
+MODULE_AUTHOR("Ban Tao <fengzheng923@gmail.com>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:sun50i-dmic");
--
2.29.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-15 13:03 ` Ban Tao
0 siblings, 0 replies; 36+ messages in thread
From: Ban Tao @ 2021-06-15 13:03 UTC (permalink / raw)
To: lgirdwood, broonie, perex, tiwai, mripard, wens, jernej.skrabec,
fengzheng923, p.zabel, samuel, krzk
Cc: alsa-devel, linux-kernel, linux-arm-kernel, linux-sunxi
The Allwinner H6 and later SoCs have an DMIC block
which is capable of capture.
Signed-off-by: Ban Tao <fengzheng923@gmail.com>
---
MAINTAINERS | 7 +
sound/soc/sunxi/Kconfig | 8 +
sound/soc/sunxi/Makefile | 1 +
sound/soc/sunxi/sun50i-dmic.c | 408 ++++++++++++++++++++++++++++++++++
4 files changed, 424 insertions(+)
create mode 100644 sound/soc/sunxi/sun50i-dmic.c
diff --git a/MAINTAINERS b/MAINTAINERS
index b4094f07214e..1b87225c39b0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -760,6 +760,13 @@ L: linux-media@vger.kernel.org
S: Maintained
F: drivers/staging/media/sunxi/cedrus/
+ALLWINNER DMIC DRIVERS
+M: Ban Tao <fengzheng923@gmail.com>
+L: alsa-devel@alsa-project.org (moderated for non-subscribers)
+S: Maintained
+F: Documentation/devicetree/bindings/sound/allwinner,sun50i-h6-dmic.yaml
+F: sound/soc/sunxi/sun50i-dmic.c
+
ALPHA PORT
M: Richard Henderson <rth@twiddle.net>
M: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
index ddcaaa98d3cb..2a3bf7722e11 100644
--- a/sound/soc/sunxi/Kconfig
+++ b/sound/soc/sunxi/Kconfig
@@ -56,6 +56,14 @@ config SND_SUN4I_SPDIF
Say Y or M to add support for the S/PDIF audio block in the Allwinner
A10 and affiliated SoCs.
+config SND_SUN50I_DMIC
+ tristate "Allwinner H6 DMIC Support"
+ depends on (OF && ARCH_SUNXI) || COMPILE_TEST
+ select SND_SOC_GENERIC_DMAENGINE_PCM
+ help
+ Say Y or M to add support for the DMIC audio block in the Allwinner
+ H6 and affiliated SoCs.
+
config SND_SUN8I_ADDA_PR_REGMAP
tristate
select REGMAP
diff --git a/sound/soc/sunxi/Makefile b/sound/soc/sunxi/Makefile
index a86be340a076..4483fe9c94ef 100644
--- a/sound/soc/sunxi/Makefile
+++ b/sound/soc/sunxi/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_SND_SUN8I_CODEC_ANALOG) += sun8i-codec-analog.o
obj-$(CONFIG_SND_SUN50I_CODEC_ANALOG) += sun50i-codec-analog.o
obj-$(CONFIG_SND_SUN8I_CODEC) += sun8i-codec.o
obj-$(CONFIG_SND_SUN8I_ADDA_PR_REGMAP) += sun8i-adda-pr-regmap.o
+obj-$(CONFIG_SND_SUN50I_DMIC) += sun50i-dmic.o
diff --git a/sound/soc/sunxi/sun50i-dmic.c b/sound/soc/sunxi/sun50i-dmic.c
new file mode 100644
index 000000000000..78d512d93974
--- /dev/null
+++ b/sound/soc/sunxi/sun50i-dmic.c
@@ -0,0 +1,408 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * ALSA SoC DMIC Audio Layer
+ *
+ * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/of_device.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+#include <sound/dmaengine_pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+
+#define SUN50I_DMIC_EN_CTL (0x00)
+ #define SUN50I_DMIC_EN_CTL_GLOBE BIT(8)
+ #define SUN50I_DMIC_EN_CTL_CHAN(v) ((v) << 0)
+ #define SUN50I_DMIC_EN_CTL_CHAN_MASK GENMASK(7, 0)
+#define SUN50I_DMIC_SR (0x04)
+ #define SUN50I_DMIC_SR_SAMOLE_RATE(v) ((v) << 0)
+ #define SUN50I_DMIC_SR_SAMOLE_RATE_MASK GENMASK(2, 0)
+#define SUN50I_DMIC_CTL (0x08)
+ #define SUN50I_DMIC_CTL_OVERSAMPLE_RATE BIT(0)
+#define SUN50I_DMIC_DATA (0x10)
+#define SUN50I_DMIC_INTC (0x14)
+ #define SUN50I_DMIC_FIFO_DRQ_EN BIT(2)
+#define SUN50I_DMIC_INT_STA (0x18)
+ #define SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING BIT(1)
+ #define SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING BIT(0)
+#define SUN50I_DMIC_RXFIFO_CTL (0x1c)
+ #define SUN50I_DMIC_RXFIFO_CTL_FLUSH BIT(31)
+ #define SUN50I_DMIC_RXFIFO_CTL_MODE BIT(9)
+ #define SUN50I_DMIC_RXFIFO_CTL_RESOLUTION BIT(8)
+#define SUN50I_DMIC_CH_NUM (0x24)
+ #define SUN50I_DMIC_CH_NUM_N(v) ((v) << 0)
+ #define SUN50I_DMIC_CH_NUM_N_MASK GENMASK(2, 0)
+#define SUN50I_DMIC_CNT (0x2c)
+ #define SUN50I_DMIC_CNT_N BIT(0)
+#define SUN50I_DMIC_HPF_CTRL (0x38)
+#define SUN50I_DMIC_VERSION (0x50)
+
+
+struct sun50i_dmic_dev {
+ struct platform_device *pdev;
+ struct clk *dmic_clk;
+ struct clk *apb_clk;
+ struct reset_control *rst;
+ struct regmap *regmap;
+ struct snd_dmaengine_dai_dma_data dma_params_rx;
+ unsigned int chan_en;
+};
+
+struct dmic_rate {
+ unsigned int samplerate;
+ unsigned int rate_bit;
+};
+
+static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream *substream,
+ struct sun50i_dmic_dev *host, bool enable)
+{
+ if (enable) {
+ /* DRQ ENABLE */
+ regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
+ SUN50I_DMIC_FIFO_DRQ_EN,
+ SUN50I_DMIC_FIFO_DRQ_EN);
+ regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
+ SUN50I_DMIC_EN_CTL_CHAN_MASK,
+ SUN50I_DMIC_EN_CTL_CHAN(host->chan_en));
+ /* Global enable */
+ regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
+ SUN50I_DMIC_EN_CTL_GLOBE,
+ SUN50I_DMIC_EN_CTL_GLOBE);
+ } else {
+ /* DRQ DISABLE */
+ regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
+ SUN50I_DMIC_FIFO_DRQ_EN, 0);
+ regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
+ SUN50I_DMIC_EN_CTL_CHAN_MASK,
+ SUN50I_DMIC_EN_CTL_CHAN(0));
+ /* Global disable */
+ regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
+ SUN50I_DMIC_EN_CTL_GLOBE, 0);
+ }
+}
+
+static int sun50i_dmic_startup(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *cpu_dai)
+{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
+
+ /* only support capture */
+ if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
+ return -EINVAL;
+
+ regmap_write(host->regmap, SUN50I_DMIC_INT_STA,
+ SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING |
+ SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING);
+ regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
+ SUN50I_DMIC_RXFIFO_CTL_FLUSH,
+ SUN50I_DMIC_RXFIFO_CTL_FLUSH);
+ regmap_write(host->regmap, SUN50I_DMIC_CNT, SUN50I_DMIC_CNT_N);
+
+ return 0;
+}
+
+static int sun50i_dmic_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *cpu_dai)
+{
+ int i = 0;
+ unsigned long rate = params_rate(params);
+ unsigned int channels = params_channels(params);
+ struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
+ struct platform_device *pdev = host->pdev;
+ struct dmic_rate dmic_rate_s[] = {
+ {44100, 0x0},
+ {48000, 0x0},
+ {22050, 0x2},
+ {24000, 0x2},
+ {11025, 0x4},
+ {12000, 0x4},
+ {32000, 0x1},
+ {16000, 0x3},
+ {8000, 0x5},
+ };
+
+ /* DMIC num is N+1 */
+ regmap_update_bits(host->regmap, SUN50I_DMIC_CH_NUM,
+ SUN50I_DMIC_CH_NUM_N_MASK,
+ SUN50I_DMIC_CH_NUM_N(channels - 1));
+ host->chan_en = (1 << channels) - 1;
+ regmap_write(host->regmap, SUN50I_DMIC_HPF_CTRL, host->chan_en);
+
+ switch (params_format(params)) {
+ case SNDRV_PCM_FORMAT_S16_LE:
+ regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
+ SUN50I_DMIC_RXFIFO_CTL_MODE,
+ SUN50I_DMIC_RXFIFO_CTL_MODE);
+ regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
+ SUN50I_DMIC_RXFIFO_CTL_RESOLUTION, 0);
+ break;
+ case SNDRV_PCM_FORMAT_S24_LE:
+ regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
+ SUN50I_DMIC_RXFIFO_CTL_MODE, 0);
+ regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
+ SUN50I_DMIC_RXFIFO_CTL_RESOLUTION,
+ SUN50I_DMIC_RXFIFO_CTL_RESOLUTION);
+ break;
+ default:
+ dev_err(&pdev->dev, "Invalid format!\n");
+ return -EINVAL;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(dmic_rate_s); i++) {
+ if (dmic_rate_s[i].samplerate == rate) {
+ regmap_update_bits(host->regmap, SUN50I_DMIC_SR,
+ SUN50I_DMIC_SR_SAMOLE_RATE_MASK,
+ SUN50I_DMIC_SR_SAMOLE_RATE(dmic_rate_s[i].rate_bit));
+ break;
+ }
+ }
+ if (i == ARRAY_SIZE(dmic_rate_s)) {
+ dev_err(&pdev->dev, "Invalid rate!\n");
+ return -EINVAL;
+ }
+
+ /* oversamplerate adjust */
+ if (params_rate(params) >= 24000)
+ regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
+ SUN50I_DMIC_CTL_OVERSAMPLE_RATE,
+ SUN50I_DMIC_CTL_OVERSAMPLE_RATE);
+ else
+ regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
+ SUN50I_DMIC_CTL_OVERSAMPLE_RATE, 0);
+
+ return 0;
+}
+
+static int sun50i_dmic_trigger(struct snd_pcm_substream *substream, int cmd,
+ struct snd_soc_dai *dai)
+{
+ int ret = 0;
+ struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
+
+ if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
+ return -EINVAL;
+
+ switch (cmd) {
+ case SNDRV_PCM_TRIGGER_START:
+ case SNDRV_PCM_TRIGGER_RESUME:
+ case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+ sun50i_snd_rxctrl_enable(substream, host, true);
+ break;
+
+ case SNDRV_PCM_TRIGGER_STOP:
+ case SNDRV_PCM_TRIGGER_SUSPEND:
+ case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+ sun50i_snd_rxctrl_enable(substream, host, false);
+ break;
+
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ return ret;
+}
+
+static int sun50i_dmic_set_sysclk(struct snd_soc_dai *dai, int clk_id,
+ unsigned int freq, int dir)
+{
+ struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
+
+ if (clk_set_rate(host->dmic_clk, freq)) {
+ dev_err(dai->dev, "Freq : %u not support\n", freq);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int sun50i_dmic_soc_dai_probe(struct snd_soc_dai *dai)
+{
+ struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
+
+ snd_soc_dai_init_dma_data(dai, NULL, &host->dma_params_rx);
+
+ return 0;
+}
+
+static const struct snd_soc_dai_ops sun50i_dmic_dai_ops = {
+ .startup = sun50i_dmic_startup,
+ .trigger = sun50i_dmic_trigger,
+ .hw_params = sun50i_dmic_hw_params,
+ .set_sysclk = sun50i_dmic_set_sysclk,
+};
+
+static const struct regmap_config sun50i_dmic_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = SUN50I_DMIC_VERSION,
+ .cache_type = REGCACHE_NONE,
+};
+
+#define SUN50I_DMIC_RATES (SNDRV_PCM_RATE_8000_48000 | SNDRV_PCM_RATE_KNOT)
+#define SUN50I_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE)
+
+static struct snd_soc_dai_driver sun50i_dmic_dai = {
+ .capture = {
+ .channels_min = 1,
+ .channels_max = 8,
+ .rates = SUN50I_DMIC_RATES,
+ .formats = SUN50I_FORMATS,
+ },
+ .probe = sun50i_dmic_soc_dai_probe,
+ .ops = &sun50i_dmic_dai_ops,
+ .name = "dmic",
+};
+
+static const struct of_device_id sun50i_dmic_of_match[] = {
+ {
+ .compatible = "allwinner,sun50i-h6-dmic",
+ },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, sun50i_dmic_of_match);
+
+static const struct snd_soc_component_driver sun50i_dmic_component = {
+ .name = "sun50i-dmic",
+};
+
+static int sun50i_dmic_runtime_suspend(struct device *dev)
+{
+ struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(host->dmic_clk);
+ clk_disable_unprepare(host->apb_clk);
+
+ return 0;
+}
+
+static int sun50i_dmic_runtime_resume(struct device *dev)
+{
+ struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
+ int ret;
+
+ ret = clk_prepare_enable(host->dmic_clk);
+ if (ret)
+ return ret;
+ ret = clk_prepare_enable(host->apb_clk);
+ if (ret)
+ clk_disable_unprepare(host->dmic_clk);
+
+ return ret;
+}
+
+static int sun50i_dmic_probe(struct platform_device *pdev)
+{
+ struct sun50i_dmic_dev *host;
+ struct resource *res;
+ int ret;
+ void __iomem *base;
+
+ host = devm_kzalloc(&pdev->dev, sizeof(*host), GFP_KERNEL);
+ if (!host)
+ return -ENOMEM;
+
+ host->pdev = pdev;
+
+ /* Get the addresses */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(base))
+ return dev_err_probe(&pdev->dev, PTR_ERR(base),
+ "get resource failed.\n");
+
+ host->regmap = devm_regmap_init_mmio(&pdev->dev, base,
+ &sun50i_dmic_regmap_config);
+
+ /* Clocks */
+ host->apb_clk = devm_clk_get(&pdev->dev, "apb");
+ if (IS_ERR(host->apb_clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(host->apb_clk),
+ "failed to get apb clock.\n");
+
+ host->dmic_clk = devm_clk_get(&pdev->dev, "dmic");
+ if (IS_ERR(host->dmic_clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(host->dmic_clk),
+ "failed to get dmic clock.\n");
+
+ host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
+ host->dma_params_rx.maxburst = 8;
+ host->dma_params_rx.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+
+ platform_set_drvdata(pdev, host);
+
+ host->rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
+ if (IS_ERR(host->rst) && PTR_ERR(host->rst) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ dev_err(&pdev->dev, "Failed to get reset: %d\n", ret);
+ return ret;
+ }
+ if (!IS_ERR(host->rst))
+ reset_control_deassert(host->rst);
+
+ ret = devm_snd_soc_register_component(&pdev->dev,
+ &sun50i_dmic_component, &sun50i_dmic_dai, 1);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to register component.\n");
+
+ pm_runtime_enable(&pdev->dev);
+ if (!pm_runtime_enabled(&pdev->dev)) {
+ ret = sun50i_dmic_runtime_resume(&pdev->dev);
+ if (ret)
+ goto err_unregister;
+ }
+
+ ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
+ if (ret)
+ goto err_suspend;
+
+ return 0;
+err_suspend:
+ if (!pm_runtime_status_suspended(&pdev->dev))
+ sun50i_dmic_runtime_suspend(&pdev->dev);
+err_unregister:
+ pm_runtime_disable(&pdev->dev);
+ return ret;
+}
+
+static int sun50i_dmic_remove(struct platform_device *pdev)
+{
+ pm_runtime_disable(&pdev->dev);
+ if (!pm_runtime_status_suspended(&pdev->dev))
+ sun50i_dmic_runtime_suspend(&pdev->dev);
+
+ return 0;
+}
+
+static const struct dev_pm_ops sun50i_dmic_pm = {
+ SET_RUNTIME_PM_OPS(sun50i_dmic_runtime_suspend,
+ sun50i_dmic_runtime_resume, NULL)
+};
+
+static struct platform_driver sun50i_dmic_driver = {
+ .driver = {
+ .name = "sun50i-dmic",
+ .of_match_table = of_match_ptr(sun50i_dmic_of_match),
+ .pm = &sun50i_dmic_pm,
+ },
+ .probe = sun50i_dmic_probe,
+ .remove = sun50i_dmic_remove,
+};
+
+module_platform_driver(sun50i_dmic_driver);
+
+MODULE_DESCRIPTION("Allwinner sun50i DMIC SoC Interface");
+MODULE_AUTHOR("Ban Tao <fengzheng923@gmail.com>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:sun50i-dmic");
--
2.29.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
2021-06-15 13:03 ` Ban Tao
(?)
(?)
@ 2021-06-15 13:18 ` Philipp Zabel
-1 siblings, 0 replies; 36+ messages in thread
From: Philipp Zabel @ 2021-06-15 13:18 UTC (permalink / raw)
To: Ban Tao, lgirdwood, broonie, perex, tiwai, mripard, wens,
jernej.skrabec, samuel, krzk
Cc: linux-kernel, alsa-devel, linux-arm-kernel, linux-sunxi
Hi Ban,
On Tue, 2021-06-15 at 21:03 +0800, Ban Tao wrote:
> The Allwinner H6 and later SoCs have an DMIC block
> which is capable of capture.
>
> Signed-off-by: Ban Tao <fengzheng923@gmail.com>
[...]
> diff --git a/sound/soc/sunxi/sun50i-dmic.c b/sound/soc/sunxi/sun50i-dmic.c
> new file mode 100644
> index 000000000000..78d512d93974
> --- /dev/null
> +++ b/sound/soc/sunxi/sun50i-dmic.c
> @@ -0,0 +1,408 @@
[...]
> + host->rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
> + if (IS_ERR(host->rst) && PTR_ERR(host->rst) == -EPROBE_DEFER) {
> + ret = -EPROBE_DEFER;
> + dev_err(&pdev->dev, "Failed to get reset: %d\n", ret);
> + return ret;
> + }
Please don't ignore errors. For example:
if (IS_ERR(host->rst))
return dev_err_probe(&pdev->dev, PTR_ERR(host->rst),
"Failed to get reset\n");
devm_reset_control_get_optional_exclusive() will return NULL if no reset
is specified in the device tree.
> + if (!IS_ERR(host->rst))
> + reset_control_deassert(host->rst);
Then this is not necessary. Just use:
reset_control_deassert(host->rst);
regards
Philipp
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-15 13:18 ` Philipp Zabel
0 siblings, 0 replies; 36+ messages in thread
From: Philipp Zabel @ 2021-06-15 13:18 UTC (permalink / raw)
To: Ban Tao, lgirdwood, broonie, perex, tiwai, mripard, wens,
jernej.skrabec, samuel, krzk
Cc: linux-kernel, alsa-devel, linux-arm-kernel, linux-sunxi
Hi Ban,
On Tue, 2021-06-15 at 21:03 +0800, Ban Tao wrote:
> The Allwinner H6 and later SoCs have an DMIC block
> which is capable of capture.
>
> Signed-off-by: Ban Tao <fengzheng923@gmail.com>
[...]
> diff --git a/sound/soc/sunxi/sun50i-dmic.c b/sound/soc/sunxi/sun50i-dmic.c
> new file mode 100644
> index 000000000000..78d512d93974
> --- /dev/null
> +++ b/sound/soc/sunxi/sun50i-dmic.c
> @@ -0,0 +1,408 @@
[...]
> + host->rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
> + if (IS_ERR(host->rst) && PTR_ERR(host->rst) == -EPROBE_DEFER) {
> + ret = -EPROBE_DEFER;
> + dev_err(&pdev->dev, "Failed to get reset: %d\n", ret);
> + return ret;
> + }
Please don't ignore errors. For example:
if (IS_ERR(host->rst))
return dev_err_probe(&pdev->dev, PTR_ERR(host->rst),
"Failed to get reset\n");
devm_reset_control_get_optional_exclusive() will return NULL if no reset
is specified in the device tree.
> + if (!IS_ERR(host->rst))
> + reset_control_deassert(host->rst);
Then this is not necessary. Just use:
reset_control_deassert(host->rst);
regards
Philipp
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-15 13:18 ` Philipp Zabel
0 siblings, 0 replies; 36+ messages in thread
From: Philipp Zabel @ 2021-06-15 13:18 UTC (permalink / raw)
To: Ban Tao, lgirdwood, broonie, perex, tiwai, mripard, wens,
jernej.skrabec, samuel, krzk
Cc: alsa-devel, linux-kernel, linux-arm-kernel, linux-sunxi
Hi Ban,
On Tue, 2021-06-15 at 21:03 +0800, Ban Tao wrote:
> The Allwinner H6 and later SoCs have an DMIC block
> which is capable of capture.
>
> Signed-off-by: Ban Tao <fengzheng923@gmail.com>
[...]
> diff --git a/sound/soc/sunxi/sun50i-dmic.c b/sound/soc/sunxi/sun50i-dmic.c
> new file mode 100644
> index 000000000000..78d512d93974
> --- /dev/null
> +++ b/sound/soc/sunxi/sun50i-dmic.c
> @@ -0,0 +1,408 @@
[...]
> + host->rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
> + if (IS_ERR(host->rst) && PTR_ERR(host->rst) == -EPROBE_DEFER) {
> + ret = -EPROBE_DEFER;
> + dev_err(&pdev->dev, "Failed to get reset: %d\n", ret);
> + return ret;
> + }
Please don't ignore errors. For example:
if (IS_ERR(host->rst))
return dev_err_probe(&pdev->dev, PTR_ERR(host->rst),
"Failed to get reset\n");
devm_reset_control_get_optional_exclusive() will return NULL if no reset
is specified in the device tree.
> + if (!IS_ERR(host->rst))
> + reset_control_deassert(host->rst);
Then this is not necessary. Just use:
reset_control_deassert(host->rst);
regards
Philipp
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-15 13:18 ` Philipp Zabel
0 siblings, 0 replies; 36+ messages in thread
From: Philipp Zabel @ 2021-06-15 13:18 UTC (permalink / raw)
To: Ban Tao, lgirdwood, broonie, perex, tiwai, mripard, wens,
jernej.skrabec, samuel, krzk
Cc: linux-kernel, alsa-devel, linux-arm-kernel, linux-sunxi
Hi Ban,
On Tue, 2021-06-15 at 21:03 +0800, Ban Tao wrote:
> The Allwinner H6 and later SoCs have an DMIC block
> which is capable of capture.
>
> Signed-off-by: Ban Tao <fengzheng923@gmail.com>
[...]
> diff --git a/sound/soc/sunxi/sun50i-dmic.c b/sound/soc/sunxi/sun50i-dmic.c
> new file mode 100644
> index 000000000000..78d512d93974
> --- /dev/null
> +++ b/sound/soc/sunxi/sun50i-dmic.c
> @@ -0,0 +1,408 @@
[...]
> + host->rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
> + if (IS_ERR(host->rst) && PTR_ERR(host->rst) == -EPROBE_DEFER) {
> + ret = -EPROBE_DEFER;
> + dev_err(&pdev->dev, "Failed to get reset: %d\n", ret);
> + return ret;
> + }
Please don't ignore errors. For example:
if (IS_ERR(host->rst))
return dev_err_probe(&pdev->dev, PTR_ERR(host->rst),
"Failed to get reset\n");
devm_reset_control_get_optional_exclusive() will return NULL if no reset
is specified in the device tree.
> + if (!IS_ERR(host->rst))
> + reset_control_deassert(host->rst);
Then this is not necessary. Just use:
reset_control_deassert(host->rst);
regards
Philipp
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
2021-06-15 13:03 ` Ban Tao
(?)
@ 2021-06-15 13:22 ` Mark Brown
-1 siblings, 0 replies; 36+ messages in thread
From: Mark Brown @ 2021-06-15 13:22 UTC (permalink / raw)
To: Ban Tao
Cc: lgirdwood, perex, tiwai, mripard, wens, jernej.skrabec, p.zabel,
samuel, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 1763 bytes --]
On Tue, Jun 15, 2021 at 09:03:26PM +0800, Ban Tao wrote:
Other than a few small things this looks good:
> +M: Ban Tao <fengzheng923@gmail.com>
> +L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> +S: Maintained
> +F: Documentation/devicetree/bindings/sound/allwinner,sun50i-h6-dmic.yaml
> +F: sound/soc/sunxi/sun50i-dmic.c
Not the binding document?
> @@ -0,0 +1,408 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * ALSA SoC DMIC Audio Layer
> + *
> + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> + *
Please make the entire comment a C++ one so things look more
intentional.
> +static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream *substream,
> + struct sun50i_dmic_dev *host, bool enable)
> +{
> + if (enable) {
> + } else {
> +static int sun50i_dmic_trigger(struct snd_pcm_substream *substream, int cmd,
> + struct snd_soc_dai *dai)
> +{
> + int ret = 0;
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> +
> + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
> + return -EINVAL;
> +
> + switch (cmd) {
> + case SNDRV_PCM_TRIGGER_START:
> + case SNDRV_PCM_TRIGGER_RESUME:
> + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> + sun50i_snd_rxctrl_enable(substream, host, true);
> + break;
> +
> + case SNDRV_PCM_TRIGGER_STOP:
> + case SNDRV_PCM_TRIGGER_SUSPEND:
> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> + sun50i_snd_rxctrl_enable(substream, host, false);
> + break;
This is the only caller of _rxctrl_enable() and _rxctrl_enable() shares
no code between the two cases - just inline _rxctrl_enable() here, it's
clearer what's going on.
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, res);
devm_platform_ioremap_resource()
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-15 13:22 ` Mark Brown
0 siblings, 0 replies; 36+ messages in thread
From: Mark Brown @ 2021-06-15 13:22 UTC (permalink / raw)
To: Ban Tao
Cc: lgirdwood, perex, tiwai, mripard, wens, jernej.skrabec, p.zabel,
samuel, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
[-- Attachment #1.1: Type: text/plain, Size: 1763 bytes --]
On Tue, Jun 15, 2021 at 09:03:26PM +0800, Ban Tao wrote:
Other than a few small things this looks good:
> +M: Ban Tao <fengzheng923@gmail.com>
> +L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> +S: Maintained
> +F: Documentation/devicetree/bindings/sound/allwinner,sun50i-h6-dmic.yaml
> +F: sound/soc/sunxi/sun50i-dmic.c
Not the binding document?
> @@ -0,0 +1,408 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * ALSA SoC DMIC Audio Layer
> + *
> + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> + *
Please make the entire comment a C++ one so things look more
intentional.
> +static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream *substream,
> + struct sun50i_dmic_dev *host, bool enable)
> +{
> + if (enable) {
> + } else {
> +static int sun50i_dmic_trigger(struct snd_pcm_substream *substream, int cmd,
> + struct snd_soc_dai *dai)
> +{
> + int ret = 0;
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> +
> + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
> + return -EINVAL;
> +
> + switch (cmd) {
> + case SNDRV_PCM_TRIGGER_START:
> + case SNDRV_PCM_TRIGGER_RESUME:
> + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> + sun50i_snd_rxctrl_enable(substream, host, true);
> + break;
> +
> + case SNDRV_PCM_TRIGGER_STOP:
> + case SNDRV_PCM_TRIGGER_SUSPEND:
> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> + sun50i_snd_rxctrl_enable(substream, host, false);
> + break;
This is the only caller of _rxctrl_enable() and _rxctrl_enable() shares
no code between the two cases - just inline _rxctrl_enable() here, it's
clearer what's going on.
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, res);
devm_platform_ioremap_resource()
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-15 13:22 ` Mark Brown
0 siblings, 0 replies; 36+ messages in thread
From: Mark Brown @ 2021-06-15 13:22 UTC (permalink / raw)
To: Ban Tao
Cc: alsa-devel, krzk, samuel, linux-kernel, tiwai, jernej.skrabec,
lgirdwood, wens, mripard, p.zabel, linux-sunxi, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 1763 bytes --]
On Tue, Jun 15, 2021 at 09:03:26PM +0800, Ban Tao wrote:
Other than a few small things this looks good:
> +M: Ban Tao <fengzheng923@gmail.com>
> +L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> +S: Maintained
> +F: Documentation/devicetree/bindings/sound/allwinner,sun50i-h6-dmic.yaml
> +F: sound/soc/sunxi/sun50i-dmic.c
Not the binding document?
> @@ -0,0 +1,408 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * ALSA SoC DMIC Audio Layer
> + *
> + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> + *
Please make the entire comment a C++ one so things look more
intentional.
> +static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream *substream,
> + struct sun50i_dmic_dev *host, bool enable)
> +{
> + if (enable) {
> + } else {
> +static int sun50i_dmic_trigger(struct snd_pcm_substream *substream, int cmd,
> + struct snd_soc_dai *dai)
> +{
> + int ret = 0;
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> +
> + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
> + return -EINVAL;
> +
> + switch (cmd) {
> + case SNDRV_PCM_TRIGGER_START:
> + case SNDRV_PCM_TRIGGER_RESUME:
> + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> + sun50i_snd_rxctrl_enable(substream, host, true);
> + break;
> +
> + case SNDRV_PCM_TRIGGER_STOP:
> + case SNDRV_PCM_TRIGGER_SUSPEND:
> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> + sun50i_snd_rxctrl_enable(substream, host, false);
> + break;
This is the only caller of _rxctrl_enable() and _rxctrl_enable() shares
no code between the two cases - just inline _rxctrl_enable() here, it's
clearer what's going on.
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, res);
devm_platform_ioremap_resource()
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
2021-06-15 13:22 ` Mark Brown
(?)
(?)
@ 2021-06-17 7:42 ` 班涛
-1 siblings, 0 replies; 36+ messages in thread
From: 班涛 @ 2021-06-17 7:42 UTC (permalink / raw)
To: Mark Brown
Cc: lgirdwood, perex, tiwai, mripard, wens, jernej.skrabec, p.zabel,
Samuel Holland, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
Hi,
Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
>
> On Tue, Jun 15, 2021 at 09:03:26PM +0800, Ban Tao wrote:
>
> Other than a few small things this looks good:
>
> > +M: Ban Tao <fengzheng923@gmail.com>
> > +L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/sound/allwinner,sun50i-h6-dmic.yaml
> > +F: sound/soc/sunxi/sun50i-dmic.c
>
> Not the binding document?
>
> > @@ -0,0 +1,408 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * ALSA SoC DMIC Audio Layer
> > + *
> > + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> > + *
>
> Please make the entire comment a C++ one so things look more
> intentional.
>
For example;
// SPDX-License-Identifier: GPL-2.0-or-later
/*
* This driver supports the DMIC in Allwinner's H6 SoCs.
*
* Copyright 2021 Ban Tao <fengzheng923@gmail.com>
*
*/
is this OK?
> > +static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream *substream,
> > + struct sun50i_dmic_dev *host, bool enable)
> > +{
> > + if (enable) {
>
> > + } else {
>
> > +static int sun50i_dmic_trigger(struct snd_pcm_substream *substream, int cmd,
> > + struct snd_soc_dai *dai)
> > +{
> > + int ret = 0;
> > + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> > +
> > + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
> > + return -EINVAL;
> > +
> > + switch (cmd) {
> > + case SNDRV_PCM_TRIGGER_START:
> > + case SNDRV_PCM_TRIGGER_RESUME:
> > + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> > + sun50i_snd_rxctrl_enable(substream, host, true);
> > + break;
> > +
> > + case SNDRV_PCM_TRIGGER_STOP:
> > + case SNDRV_PCM_TRIGGER_SUSPEND:
> > + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> > + sun50i_snd_rxctrl_enable(substream, host, false);
> > + break;
>
> This is the only caller of _rxctrl_enable() and _rxctrl_enable() shares
> no code between the two cases - just inline _rxctrl_enable() here, it's
> clearer what's going on.
>
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + base = devm_ioremap_resource(&pdev->dev, res);
>
> devm_platform_ioremap_resource()
But I need to get the register base address of DMIC. E.g res->start.
host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-17 7:42 ` 班涛
0 siblings, 0 replies; 36+ messages in thread
From: 班涛 @ 2021-06-17 7:42 UTC (permalink / raw)
To: Mark Brown
Cc: lgirdwood, perex, tiwai, mripard, wens, jernej.skrabec, p.zabel,
Samuel Holland, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
Hi,
Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
>
> On Tue, Jun 15, 2021 at 09:03:26PM +0800, Ban Tao wrote:
>
> Other than a few small things this looks good:
>
> > +M: Ban Tao <fengzheng923@gmail.com>
> > +L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/sound/allwinner,sun50i-h6-dmic.yaml
> > +F: sound/soc/sunxi/sun50i-dmic.c
>
> Not the binding document?
>
> > @@ -0,0 +1,408 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * ALSA SoC DMIC Audio Layer
> > + *
> > + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> > + *
>
> Please make the entire comment a C++ one so things look more
> intentional.
>
For example;
// SPDX-License-Identifier: GPL-2.0-or-later
/*
* This driver supports the DMIC in Allwinner's H6 SoCs.
*
* Copyright 2021 Ban Tao <fengzheng923@gmail.com>
*
*/
is this OK?
> > +static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream *substream,
> > + struct sun50i_dmic_dev *host, bool enable)
> > +{
> > + if (enable) {
>
> > + } else {
>
> > +static int sun50i_dmic_trigger(struct snd_pcm_substream *substream, int cmd,
> > + struct snd_soc_dai *dai)
> > +{
> > + int ret = 0;
> > + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> > +
> > + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
> > + return -EINVAL;
> > +
> > + switch (cmd) {
> > + case SNDRV_PCM_TRIGGER_START:
> > + case SNDRV_PCM_TRIGGER_RESUME:
> > + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> > + sun50i_snd_rxctrl_enable(substream, host, true);
> > + break;
> > +
> > + case SNDRV_PCM_TRIGGER_STOP:
> > + case SNDRV_PCM_TRIGGER_SUSPEND:
> > + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> > + sun50i_snd_rxctrl_enable(substream, host, false);
> > + break;
>
> This is the only caller of _rxctrl_enable() and _rxctrl_enable() shares
> no code between the two cases - just inline _rxctrl_enable() here, it's
> clearer what's going on.
>
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + base = devm_ioremap_resource(&pdev->dev, res);
>
> devm_platform_ioremap_resource()
But I need to get the register base address of DMIC. E.g res->start.
host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-17 7:42 ` 班涛
0 siblings, 0 replies; 36+ messages in thread
From: 班涛 @ 2021-06-17 7:42 UTC (permalink / raw)
To: Mark Brown
Cc: alsa-devel, krzk, Samuel Holland, linux-kernel, tiwai,
jernej.skrabec, lgirdwood, wens, mripard, p.zabel, linux-sunxi,
linux-arm-kernel
Hi,
Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
>
> On Tue, Jun 15, 2021 at 09:03:26PM +0800, Ban Tao wrote:
>
> Other than a few small things this looks good:
>
> > +M: Ban Tao <fengzheng923@gmail.com>
> > +L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/sound/allwinner,sun50i-h6-dmic.yaml
> > +F: sound/soc/sunxi/sun50i-dmic.c
>
> Not the binding document?
>
> > @@ -0,0 +1,408 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * ALSA SoC DMIC Audio Layer
> > + *
> > + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> > + *
>
> Please make the entire comment a C++ one so things look more
> intentional.
>
For example;
// SPDX-License-Identifier: GPL-2.0-or-later
/*
* This driver supports the DMIC in Allwinner's H6 SoCs.
*
* Copyright 2021 Ban Tao <fengzheng923@gmail.com>
*
*/
is this OK?
> > +static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream *substream,
> > + struct sun50i_dmic_dev *host, bool enable)
> > +{
> > + if (enable) {
>
> > + } else {
>
> > +static int sun50i_dmic_trigger(struct snd_pcm_substream *substream, int cmd,
> > + struct snd_soc_dai *dai)
> > +{
> > + int ret = 0;
> > + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> > +
> > + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
> > + return -EINVAL;
> > +
> > + switch (cmd) {
> > + case SNDRV_PCM_TRIGGER_START:
> > + case SNDRV_PCM_TRIGGER_RESUME:
> > + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> > + sun50i_snd_rxctrl_enable(substream, host, true);
> > + break;
> > +
> > + case SNDRV_PCM_TRIGGER_STOP:
> > + case SNDRV_PCM_TRIGGER_SUSPEND:
> > + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> > + sun50i_snd_rxctrl_enable(substream, host, false);
> > + break;
>
> This is the only caller of _rxctrl_enable() and _rxctrl_enable() shares
> no code between the two cases - just inline _rxctrl_enable() here, it's
> clearer what's going on.
>
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + base = devm_ioremap_resource(&pdev->dev, res);
>
> devm_platform_ioremap_resource()
But I need to get the register base address of DMIC. E.g res->start.
host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-17 7:42 ` 班涛
0 siblings, 0 replies; 36+ messages in thread
From: 班涛 @ 2021-06-17 7:42 UTC (permalink / raw)
To: Mark Brown
Cc: lgirdwood, perex, tiwai, mripard, wens, jernej.skrabec, p.zabel,
Samuel Holland, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
Hi,
Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
>
> On Tue, Jun 15, 2021 at 09:03:26PM +0800, Ban Tao wrote:
>
> Other than a few small things this looks good:
>
> > +M: Ban Tao <fengzheng923@gmail.com>
> > +L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/sound/allwinner,sun50i-h6-dmic.yaml
> > +F: sound/soc/sunxi/sun50i-dmic.c
>
> Not the binding document?
>
> > @@ -0,0 +1,408 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * ALSA SoC DMIC Audio Layer
> > + *
> > + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> > + *
>
> Please make the entire comment a C++ one so things look more
> intentional.
>
For example;
// SPDX-License-Identifier: GPL-2.0-or-later
/*
* This driver supports the DMIC in Allwinner's H6 SoCs.
*
* Copyright 2021 Ban Tao <fengzheng923@gmail.com>
*
*/
is this OK?
> > +static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream *substream,
> > + struct sun50i_dmic_dev *host, bool enable)
> > +{
> > + if (enable) {
>
> > + } else {
>
> > +static int sun50i_dmic_trigger(struct snd_pcm_substream *substream, int cmd,
> > + struct snd_soc_dai *dai)
> > +{
> > + int ret = 0;
> > + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> > +
> > + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
> > + return -EINVAL;
> > +
> > + switch (cmd) {
> > + case SNDRV_PCM_TRIGGER_START:
> > + case SNDRV_PCM_TRIGGER_RESUME:
> > + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> > + sun50i_snd_rxctrl_enable(substream, host, true);
> > + break;
> > +
> > + case SNDRV_PCM_TRIGGER_STOP:
> > + case SNDRV_PCM_TRIGGER_SUSPEND:
> > + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> > + sun50i_snd_rxctrl_enable(substream, host, false);
> > + break;
>
> This is the only caller of _rxctrl_enable() and _rxctrl_enable() shares
> no code between the two cases - just inline _rxctrl_enable() here, it's
> clearer what's going on.
>
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + base = devm_ioremap_resource(&pdev->dev, res);
>
> devm_platform_ioremap_resource()
But I need to get the register base address of DMIC. E.g res->start.
host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
2021-06-17 7:42 ` 班涛
(?)
@ 2021-06-17 10:48 ` Mark Brown
-1 siblings, 0 replies; 36+ messages in thread
From: Mark Brown @ 2021-06-17 10:48 UTC (permalink / raw)
To: 班涛
Cc: lgirdwood, perex, tiwai, mripard, wens, jernej.skrabec, p.zabel,
Samuel Holland, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 1052 bytes --]
On Thu, Jun 17, 2021 at 03:42:43PM +0800, 班涛 wrote:
> Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
> > > @@ -0,0 +1,408 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/*
> > > + * ALSA SoC DMIC Audio Layer
> > > + *
> > > + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> > > + *
> > Please make the entire comment a C++ one so things look more
> > intentional.
> For example;
> // SPDX-License-Identifier: GPL-2.0-or-later
> /*
> * This driver supports the DMIC in Allwinner's H6 SoCs.
> *
> * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> *
> */
> is this OK?
No, that's what you have already make the entire thing a C++ comment
with //s.
> > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > + base = devm_ioremap_resource(&pdev->dev, res);
> >
> > devm_platform_ioremap_resource()
>
> But I need to get the register base address of DMIC. E.g res->start.
> host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
OK.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-17 10:48 ` Mark Brown
0 siblings, 0 replies; 36+ messages in thread
From: Mark Brown @ 2021-06-17 10:48 UTC (permalink / raw)
To: 班涛
Cc: lgirdwood, perex, tiwai, mripard, wens, jernej.skrabec, p.zabel,
Samuel Holland, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
[-- Attachment #1.1: Type: text/plain, Size: 1052 bytes --]
On Thu, Jun 17, 2021 at 03:42:43PM +0800, 班涛 wrote:
> Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
> > > @@ -0,0 +1,408 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/*
> > > + * ALSA SoC DMIC Audio Layer
> > > + *
> > > + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> > > + *
> > Please make the entire comment a C++ one so things look more
> > intentional.
> For example;
> // SPDX-License-Identifier: GPL-2.0-or-later
> /*
> * This driver supports the DMIC in Allwinner's H6 SoCs.
> *
> * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> *
> */
> is this OK?
No, that's what you have already make the entire thing a C++ comment
with //s.
> > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > + base = devm_ioremap_resource(&pdev->dev, res);
> >
> > devm_platform_ioremap_resource()
>
> But I need to get the register base address of DMIC. E.g res->start.
> host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
OK.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-17 10:48 ` Mark Brown
0 siblings, 0 replies; 36+ messages in thread
From: Mark Brown @ 2021-06-17 10:48 UTC (permalink / raw)
To: 班涛
Cc: alsa-devel, krzk, Samuel Holland, linux-kernel, tiwai,
jernej.skrabec, lgirdwood, wens, mripard, p.zabel, linux-sunxi,
linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 1052 bytes --]
On Thu, Jun 17, 2021 at 03:42:43PM +0800, 班涛 wrote:
> Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
> > > @@ -0,0 +1,408 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/*
> > > + * ALSA SoC DMIC Audio Layer
> > > + *
> > > + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> > > + *
> > Please make the entire comment a C++ one so things look more
> > intentional.
> For example;
> // SPDX-License-Identifier: GPL-2.0-or-later
> /*
> * This driver supports the DMIC in Allwinner's H6 SoCs.
> *
> * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> *
> */
> is this OK?
No, that's what you have already make the entire thing a C++ comment
with //s.
> > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > + base = devm_ioremap_resource(&pdev->dev, res);
> >
> > devm_platform_ioremap_resource()
>
> But I need to get the register base address of DMIC. E.g res->start.
> host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
OK.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
2021-06-17 10:48 ` Mark Brown
(?)
(?)
@ 2021-06-17 11:50 ` 班涛
-1 siblings, 0 replies; 36+ messages in thread
From: 班涛 @ 2021-06-17 11:50 UTC (permalink / raw)
To: Mark Brown
Cc: lgirdwood, perex, tiwai, mripard, wens, jernej.skrabec, p.zabel,
Samuel Holland, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
Hi,
Mark Brown <broonie@kernel.org> 于2021年6月17日周四 下午6:48写道:
>
> On Thu, Jun 17, 2021 at 03:42:43PM +0800, 班涛 wrote:
> > Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
>
> > > > @@ -0,0 +1,408 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > +/*
> > > > + * ALSA SoC DMIC Audio Layer
> > > > + *
> > > > + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> > > > + *
>
> > > Please make the entire comment a C++ one so things look more
> > > intentional.
>
> > For example;
> > // SPDX-License-Identifier: GPL-2.0-or-later
> > /*
> > * This driver supports the DMIC in Allwinner's H6 SoCs.
> > *
> > * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> > *
> > */
> > is this OK?
>
> No, that's what you have already make the entire thing a C++ comment
> with //s.
I don’t understand. For example, sun4i-codec.c sun4i-i2s.c
sun8i-codec.c and sun4i-spdif.c files are the same as mine.
Which file can I refer to? what should I do......
>
> > > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > > + base = devm_ioremap_resource(&pdev->dev, res);
> > >
> > > devm_platform_ioremap_resource()
> >
> > But I need to get the register base address of DMIC. E.g res->start.
> > host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
>
> OK.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-17 11:50 ` 班涛
0 siblings, 0 replies; 36+ messages in thread
From: 班涛 @ 2021-06-17 11:50 UTC (permalink / raw)
To: Mark Brown
Cc: lgirdwood, perex, tiwai, mripard, wens, jernej.skrabec, p.zabel,
Samuel Holland, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
Hi,
Mark Brown <broonie@kernel.org> 于2021年6月17日周四 下午6:48写道:
>
> On Thu, Jun 17, 2021 at 03:42:43PM +0800, 班涛 wrote:
> > Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
>
> > > > @@ -0,0 +1,408 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > +/*
> > > > + * ALSA SoC DMIC Audio Layer
> > > > + *
> > > > + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> > > > + *
>
> > > Please make the entire comment a C++ one so things look more
> > > intentional.
>
> > For example;
> > // SPDX-License-Identifier: GPL-2.0-or-later
> > /*
> > * This driver supports the DMIC in Allwinner's H6 SoCs.
> > *
> > * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> > *
> > */
> > is this OK?
>
> No, that's what you have already make the entire thing a C++ comment
> with //s.
I don’t understand. For example, sun4i-codec.c sun4i-i2s.c
sun8i-codec.c and sun4i-spdif.c files are the same as mine.
Which file can I refer to? what should I do......
>
> > > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > > + base = devm_ioremap_resource(&pdev->dev, res);
> > >
> > > devm_platform_ioremap_resource()
> >
> > But I need to get the register base address of DMIC. E.g res->start.
> > host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
>
> OK.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-17 11:50 ` 班涛
0 siblings, 0 replies; 36+ messages in thread
From: 班涛 @ 2021-06-17 11:50 UTC (permalink / raw)
To: Mark Brown
Cc: alsa-devel, krzk, Samuel Holland, linux-kernel, tiwai,
jernej.skrabec, lgirdwood, wens, mripard, p.zabel, linux-sunxi,
linux-arm-kernel
Hi,
Mark Brown <broonie@kernel.org> 于2021年6月17日周四 下午6:48写道:
>
> On Thu, Jun 17, 2021 at 03:42:43PM +0800, 班涛 wrote:
> > Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
>
> > > > @@ -0,0 +1,408 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > +/*
> > > > + * ALSA SoC DMIC Audio Layer
> > > > + *
> > > > + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> > > > + *
>
> > > Please make the entire comment a C++ one so things look more
> > > intentional.
>
> > For example;
> > // SPDX-License-Identifier: GPL-2.0-or-later
> > /*
> > * This driver supports the DMIC in Allwinner's H6 SoCs.
> > *
> > * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> > *
> > */
> > is this OK?
>
> No, that's what you have already make the entire thing a C++ comment
> with //s.
I don’t understand. For example, sun4i-codec.c sun4i-i2s.c
sun8i-codec.c and sun4i-spdif.c files are the same as mine.
Which file can I refer to? what should I do......
>
> > > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > > + base = devm_ioremap_resource(&pdev->dev, res);
> > >
> > > devm_platform_ioremap_resource()
> >
> > But I need to get the register base address of DMIC. E.g res->start.
> > host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
>
> OK.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-17 11:50 ` 班涛
0 siblings, 0 replies; 36+ messages in thread
From: 班涛 @ 2021-06-17 11:50 UTC (permalink / raw)
To: Mark Brown
Cc: lgirdwood, perex, tiwai, mripard, wens, jernej.skrabec, p.zabel,
Samuel Holland, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
Hi,
Mark Brown <broonie@kernel.org> 于2021年6月17日周四 下午6:48写道:
>
> On Thu, Jun 17, 2021 at 03:42:43PM +0800, 班涛 wrote:
> > Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
>
> > > > @@ -0,0 +1,408 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > +/*
> > > > + * ALSA SoC DMIC Audio Layer
> > > > + *
> > > > + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> > > > + *
>
> > > Please make the entire comment a C++ one so things look more
> > > intentional.
>
> > For example;
> > // SPDX-License-Identifier: GPL-2.0-or-later
> > /*
> > * This driver supports the DMIC in Allwinner's H6 SoCs.
> > *
> > * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> > *
> > */
> > is this OK?
>
> No, that's what you have already make the entire thing a C++ comment
> with //s.
I don’t understand. For example, sun4i-codec.c sun4i-i2s.c
sun8i-codec.c and sun4i-spdif.c files are the same as mine.
Which file can I refer to? what should I do......
>
> > > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > > + base = devm_ioremap_resource(&pdev->dev, res);
> > >
> > > devm_platform_ioremap_resource()
> >
> > But I need to get the register base address of DMIC. E.g res->start.
> > host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
>
> OK.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
2021-06-17 11:50 ` 班涛
(?)
@ 2021-06-17 12:07 ` Mark Brown
-1 siblings, 0 replies; 36+ messages in thread
From: Mark Brown @ 2021-06-17 12:07 UTC (permalink / raw)
To: 班涛
Cc: lgirdwood, perex, tiwai, mripard, wens, jernej.skrabec, p.zabel,
Samuel Holland, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 699 bytes --]
On Thu, Jun 17, 2021 at 07:50:42PM +0800, 班涛 wrote:
> Mark Brown <broonie@kernel.org> 于2021年6月17日周四 下午6:48写道:
> > On Thu, Jun 17, 2021 at 03:42:43PM +0800, 班涛 wrote:
> > > Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
> > No, that's what you have already make the entire thing a C++ comment
> > with //s.
> I don’t understand. For example, sun4i-codec.c sun4i-i2s.c
> sun8i-codec.c and sun4i-spdif.c files are the same as mine.
Other people doing a bad job is no excuse for doing a bad job yourself.
> Which file can I refer to? what should I do......
Make every line of the comment start with //. See soc-core.c
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-17 12:07 ` Mark Brown
0 siblings, 0 replies; 36+ messages in thread
From: Mark Brown @ 2021-06-17 12:07 UTC (permalink / raw)
To: 班涛
Cc: lgirdwood, perex, tiwai, mripard, wens, jernej.skrabec, p.zabel,
Samuel Holland, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
[-- Attachment #1.1: Type: text/plain, Size: 699 bytes --]
On Thu, Jun 17, 2021 at 07:50:42PM +0800, 班涛 wrote:
> Mark Brown <broonie@kernel.org> 于2021年6月17日周四 下午6:48写道:
> > On Thu, Jun 17, 2021 at 03:42:43PM +0800, 班涛 wrote:
> > > Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
> > No, that's what you have already make the entire thing a C++ comment
> > with //s.
> I don’t understand. For example, sun4i-codec.c sun4i-i2s.c
> sun8i-codec.c and sun4i-spdif.c files are the same as mine.
Other people doing a bad job is no excuse for doing a bad job yourself.
> Which file can I refer to? what should I do......
Make every line of the comment start with //. See soc-core.c
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-17 12:07 ` Mark Brown
0 siblings, 0 replies; 36+ messages in thread
From: Mark Brown @ 2021-06-17 12:07 UTC (permalink / raw)
To: 班涛
Cc: alsa-devel, krzk, Samuel Holland, linux-kernel, tiwai,
jernej.skrabec, lgirdwood, wens, mripard, p.zabel, linux-sunxi,
linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 699 bytes --]
On Thu, Jun 17, 2021 at 07:50:42PM +0800, 班涛 wrote:
> Mark Brown <broonie@kernel.org> 于2021年6月17日周四 下午6:48写道:
> > On Thu, Jun 17, 2021 at 03:42:43PM +0800, 班涛 wrote:
> > > Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
> > No, that's what you have already make the entire thing a C++ comment
> > with //s.
> I don’t understand. For example, sun4i-codec.c sun4i-i2s.c
> sun8i-codec.c and sun4i-spdif.c files are the same as mine.
Other people doing a bad job is no excuse for doing a bad job yourself.
> Which file can I refer to? what should I do......
Make every line of the comment start with //. See soc-core.c
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
2021-06-17 12:07 ` Mark Brown
(?)
(?)
@ 2021-06-17 12:18 ` 班涛
-1 siblings, 0 replies; 36+ messages in thread
From: 班涛 @ 2021-06-17 12:18 UTC (permalink / raw)
To: Mark Brown
Cc: lgirdwood, perex, tiwai, mripard, wens, jernej.skrabec, p.zabel,
Samuel Holland, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
Hi,
Mark Brown <broonie@kernel.org> 于2021年6月17日周四 下午8:08写道:
>
> On Thu, Jun 17, 2021 at 07:50:42PM +0800, 班涛 wrote:
> > Mark Brown <broonie@kernel.org> 于2021年6月17日周四 下午6:48写道:
> > > On Thu, Jun 17, 2021 at 03:42:43PM +0800, 班涛 wrote:
> > > > Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
>
> > > No, that's what you have already make the entire thing a C++ comment
> > > with //s.
>
> > I don’t understand. For example, sun4i-codec.c sun4i-i2s.c
> > sun8i-codec.c and sun4i-spdif.c files are the same as mine.
>
> Other people doing a bad job is no excuse for doing a bad job yourself.
>
> > Which file can I refer to? what should I do......
>
> Make every line of the comment start with //. See soc-core.c
Thanks, I understand, I will fix it in v2.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-17 12:18 ` 班涛
0 siblings, 0 replies; 36+ messages in thread
From: 班涛 @ 2021-06-17 12:18 UTC (permalink / raw)
To: Mark Brown
Cc: lgirdwood, perex, tiwai, mripard, wens, jernej.skrabec, p.zabel,
Samuel Holland, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
Hi,
Mark Brown <broonie@kernel.org> 于2021年6月17日周四 下午8:08写道:
>
> On Thu, Jun 17, 2021 at 07:50:42PM +0800, 班涛 wrote:
> > Mark Brown <broonie@kernel.org> 于2021年6月17日周四 下午6:48写道:
> > > On Thu, Jun 17, 2021 at 03:42:43PM +0800, 班涛 wrote:
> > > > Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
>
> > > No, that's what you have already make the entire thing a C++ comment
> > > with //s.
>
> > I don’t understand. For example, sun4i-codec.c sun4i-i2s.c
> > sun8i-codec.c and sun4i-spdif.c files are the same as mine.
>
> Other people doing a bad job is no excuse for doing a bad job yourself.
>
> > Which file can I refer to? what should I do......
>
> Make every line of the comment start with //. See soc-core.c
Thanks, I understand, I will fix it in v2.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-17 12:18 ` 班涛
0 siblings, 0 replies; 36+ messages in thread
From: 班涛 @ 2021-06-17 12:18 UTC (permalink / raw)
To: Mark Brown
Cc: alsa-devel, krzk, Samuel Holland, linux-kernel, tiwai,
jernej.skrabec, lgirdwood, wens, mripard, p.zabel, linux-sunxi,
linux-arm-kernel
Hi,
Mark Brown <broonie@kernel.org> 于2021年6月17日周四 下午8:08写道:
>
> On Thu, Jun 17, 2021 at 07:50:42PM +0800, 班涛 wrote:
> > Mark Brown <broonie@kernel.org> 于2021年6月17日周四 下午6:48写道:
> > > On Thu, Jun 17, 2021 at 03:42:43PM +0800, 班涛 wrote:
> > > > Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
>
> > > No, that's what you have already make the entire thing a C++ comment
> > > with //s.
>
> > I don’t understand. For example, sun4i-codec.c sun4i-i2s.c
> > sun8i-codec.c and sun4i-spdif.c files are the same as mine.
>
> Other people doing a bad job is no excuse for doing a bad job yourself.
>
> > Which file can I refer to? what should I do......
>
> Make every line of the comment start with //. See soc-core.c
Thanks, I understand, I will fix it in v2.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-17 12:18 ` 班涛
0 siblings, 0 replies; 36+ messages in thread
From: 班涛 @ 2021-06-17 12:18 UTC (permalink / raw)
To: Mark Brown
Cc: lgirdwood, perex, tiwai, mripard, wens, jernej.skrabec, p.zabel,
Samuel Holland, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
Hi,
Mark Brown <broonie@kernel.org> 于2021年6月17日周四 下午8:08写道:
>
> On Thu, Jun 17, 2021 at 07:50:42PM +0800, 班涛 wrote:
> > Mark Brown <broonie@kernel.org> 于2021年6月17日周四 下午6:48写道:
> > > On Thu, Jun 17, 2021 at 03:42:43PM +0800, 班涛 wrote:
> > > > Mark Brown <broonie@kernel.org> 于2021年6月15日周二 下午9:22写道:
>
> > > No, that's what you have already make the entire thing a C++ comment
> > > with //s.
>
> > I don’t understand. For example, sun4i-codec.c sun4i-i2s.c
> > sun8i-codec.c and sun4i-spdif.c files are the same as mine.
>
> Other people doing a bad job is no excuse for doing a bad job yourself.
>
> > Which file can I refer to? what should I do......
>
> Make every line of the comment start with //. See soc-core.c
Thanks, I understand, I will fix it in v2.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
2021-06-15 13:03 ` Ban Tao
(?)
@ 2021-06-16 3:06 ` Samuel Holland
-1 siblings, 0 replies; 36+ messages in thread
From: Samuel Holland @ 2021-06-16 3:06 UTC (permalink / raw)
To: Ban Tao, lgirdwood, broonie, perex, tiwai, mripard, wens,
jernej.skrabec, p.zabel, krzk
Cc: linux-kernel, alsa-devel, linux-arm-kernel, linux-sunxi
Hello,
On 6/15/21 8:03 AM, Ban Tao wrote:
> The Allwinner H6 and later SoCs have an DMIC block
> which is capable of capture.
>
> Signed-off-by: Ban Tao <fengzheng923@gmail.com>
> ---
> MAINTAINERS | 7 +
> sound/soc/sunxi/Kconfig | 8 +
> sound/soc/sunxi/Makefile | 1 +
> sound/soc/sunxi/sun50i-dmic.c | 408 ++++++++++++++++++++++++++++++++++
> 4 files changed, 424 insertions(+)
> create mode 100644 sound/soc/sunxi/sun50i-dmic.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b4094f07214e..1b87225c39b0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -760,6 +760,13 @@ L: linux-media@vger.kernel.org
> S: Maintained
> F: drivers/staging/media/sunxi/cedrus/
>
> +ALLWINNER DMIC DRIVERS
> +M: Ban Tao <fengzheng923@gmail.com>
> +L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> +S: Maintained
> +F: Documentation/devicetree/bindings/sound/allwinner,sun50i-h6-dmic.yaml
> +F: sound/soc/sunxi/sun50i-dmic.c
> +
> ALPHA PORT
> M: Richard Henderson <rth@twiddle.net>
> M: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
> index ddcaaa98d3cb..2a3bf7722e11 100644
> --- a/sound/soc/sunxi/Kconfig
> +++ b/sound/soc/sunxi/Kconfig
> @@ -56,6 +56,14 @@ config SND_SUN4I_SPDIF
> Say Y or M to add support for the S/PDIF audio block in the Allwinner
> A10 and affiliated SoCs.
>
> +config SND_SUN50I_DMIC
> + tristate "Allwinner H6 DMIC Support"
> + depends on (OF && ARCH_SUNXI) || COMPILE_TEST
> + select SND_SOC_GENERIC_DMAENGINE_PCM
> + help
> + Say Y or M to add support for the DMIC audio block in the Allwinner
> + H6 and affiliated SoCs.
> +
> config SND_SUN8I_ADDA_PR_REGMAP
> tristate
> select REGMAP
> diff --git a/sound/soc/sunxi/Makefile b/sound/soc/sunxi/Makefile
> index a86be340a076..4483fe9c94ef 100644
> --- a/sound/soc/sunxi/Makefile
> +++ b/sound/soc/sunxi/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_SND_SUN8I_CODEC_ANALOG) += sun8i-codec-analog.o
> obj-$(CONFIG_SND_SUN50I_CODEC_ANALOG) += sun50i-codec-analog.o
> obj-$(CONFIG_SND_SUN8I_CODEC) += sun8i-codec.o
> obj-$(CONFIG_SND_SUN8I_ADDA_PR_REGMAP) += sun8i-adda-pr-regmap.o
> +obj-$(CONFIG_SND_SUN50I_DMIC) += sun50i-dmic.o
> diff --git a/sound/soc/sunxi/sun50i-dmic.c b/sound/soc/sunxi/sun50i-dmic.c
> new file mode 100644
> index 000000000000..78d512d93974
> --- /dev/null
> +++ b/sound/soc/sunxi/sun50i-dmic.c
> @@ -0,0 +1,408 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * ALSA SoC DMIC Audio Layer
> + *
> + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/of_device.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +#include <sound/dmaengine_pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +
> +
> +#define SUN50I_DMIC_EN_CTL (0x00)
> + #define SUN50I_DMIC_EN_CTL_GLOBE BIT(8)
> + #define SUN50I_DMIC_EN_CTL_CHAN(v) ((v) << 0)
> + #define SUN50I_DMIC_EN_CTL_CHAN_MASK GENMASK(7, 0)
> +#define SUN50I_DMIC_SR (0x04)
> + #define SUN50I_DMIC_SR_SAMOLE_RATE(v) ((v) << 0)
> + #define SUN50I_DMIC_SR_SAMOLE_RATE_MASK GENMASK(2, 0)
> +#define SUN50I_DMIC_CTL (0x08)
> + #define SUN50I_DMIC_CTL_OVERSAMPLE_RATE BIT(0)
> +#define SUN50I_DMIC_DATA (0x10)
> +#define SUN50I_DMIC_INTC (0x14)
> + #define SUN50I_DMIC_FIFO_DRQ_EN BIT(2)
> +#define SUN50I_DMIC_INT_STA (0x18)
> + #define SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING BIT(1)
> + #define SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING BIT(0)
> +#define SUN50I_DMIC_RXFIFO_CTL (0x1c)
> + #define SUN50I_DMIC_RXFIFO_CTL_FLUSH BIT(31)
> + #define SUN50I_DMIC_RXFIFO_CTL_MODE BIT(9)
> + #define SUN50I_DMIC_RXFIFO_CTL_RESOLUTION BIT(8)
> +#define SUN50I_DMIC_CH_NUM (0x24)
> + #define SUN50I_DMIC_CH_NUM_N(v) ((v) << 0)
> + #define SUN50I_DMIC_CH_NUM_N_MASK GENMASK(2, 0)
> +#define SUN50I_DMIC_CNT (0x2c)
> + #define SUN50I_DMIC_CNT_N BIT(0)
> +#define SUN50I_DMIC_HPF_CTRL (0x38)
> +#define SUN50I_DMIC_VERSION (0x50)
> +
> +
> +struct sun50i_dmic_dev {
> + struct platform_device *pdev;
> + struct clk *dmic_clk;
> + struct clk *apb_clk;
> + struct reset_control *rst;
> + struct regmap *regmap;
> + struct snd_dmaengine_dai_dma_data dma_params_rx;
> + unsigned int chan_en;
> +};
> +
> +struct dmic_rate {
> + unsigned int samplerate;
> + unsigned int rate_bit;
> +};
> +
> +static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream *substream,
> + struct sun50i_dmic_dev *host, bool enable)
> +{
> + if (enable) {
> + /* DRQ ENABLE */
> + regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
> + SUN50I_DMIC_FIFO_DRQ_EN,
> + SUN50I_DMIC_FIFO_DRQ_EN);
> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
> + SUN50I_DMIC_EN_CTL_CHAN_MASK,
> + SUN50I_DMIC_EN_CTL_CHAN(host->chan_en));
> + /* Global enable */
> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
> + SUN50I_DMIC_EN_CTL_GLOBE,
> + SUN50I_DMIC_EN_CTL_GLOBE);
> + } else {
> + /* DRQ DISABLE */
> + regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
> + SUN50I_DMIC_FIFO_DRQ_EN, 0);
> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
> + SUN50I_DMIC_EN_CTL_CHAN_MASK,
> + SUN50I_DMIC_EN_CTL_CHAN(0));
> + /* Global disable */
> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
> + SUN50I_DMIC_EN_CTL_GLOBE, 0);
> + }
> +}
> +
> +static int sun50i_dmic_startup(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *cpu_dai)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
This is equivalent to the simpler code you use below:
struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
> +
> + /* only support capture */
> + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
> + return -EINVAL;
> +
> + regmap_write(host->regmap, SUN50I_DMIC_INT_STA,
> + SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING |
> + SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING);
The interrupt is never enabled, so do we really care about IRQ status?
> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> + SUN50I_DMIC_RXFIFO_CTL_FLUSH,
> + SUN50I_DMIC_RXFIFO_CTL_FLUSH);
> + regmap_write(host->regmap, SUN50I_DMIC_CNT, SUN50I_DMIC_CNT_N);
> +
> + return 0;
> +}
> +
> +static int sun50i_dmic_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params,
> + struct snd_soc_dai *cpu_dai)
> +{
> + int i = 0;
> + unsigned long rate = params_rate(params);
> + unsigned int channels = params_channels(params);
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
> + struct platform_device *pdev = host->pdev;
> + struct dmic_rate dmic_rate_s[] = {
> + {44100, 0x0},
> + {48000, 0x0},
> + {22050, 0x2},
> + {24000, 0x2},
> + {11025, 0x4},
> + {12000, 0x4},
> + {32000, 0x1},
> + {16000, 0x3},
> + {8000, 0x5},
> + };
> +
> + /* DMIC num is N+1 */
> + regmap_update_bits(host->regmap, SUN50I_DMIC_CH_NUM,
> + SUN50I_DMIC_CH_NUM_N_MASK,
> + SUN50I_DMIC_CH_NUM_N(channels - 1));
> + host->chan_en = (1 << channels) - 1;
> + regmap_write(host->regmap, SUN50I_DMIC_HPF_CTRL, host->chan_en);
> +
> + switch (params_format(params)) {
> + case SNDRV_PCM_FORMAT_S16_LE:
> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> + SUN50I_DMIC_RXFIFO_CTL_MODE,
> + SUN50I_DMIC_RXFIFO_CTL_MODE);
> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION, 0);
> + break;
> + case SNDRV_PCM_FORMAT_S24_LE:
> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> + SUN50I_DMIC_RXFIFO_CTL_MODE, 0);
In SNDRV_PCM_FORMAT_S24_LE, the sample is in the three least significant
bytes, so wouldn't we want to still use mode 1 here? To me, mode 0 only
makes sense when using SNDRV_PCM_FORMAT_S32_LE.
> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION,
> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION);
> + break;
> + default:
> + dev_err(&pdev->dev, "Invalid format!\n");
The calls to dev_err() can use cpu_dai->dev. Then you don't need to keep
a pointer to pdev in the driver data.
> + return -EINVAL;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(dmic_rate_s); i++) {
> + if (dmic_rate_s[i].samplerate == rate) {
> + regmap_update_bits(host->regmap, SUN50I_DMIC_SR,
> + SUN50I_DMIC_SR_SAMOLE_RATE_MASK,
> + SUN50I_DMIC_SR_SAMOLE_RATE(dmic_rate_s[i].rate_bit));
> + break;
> + }
> + }
> + if (i == ARRAY_SIZE(dmic_rate_s)) {
> + dev_err(&pdev->dev, "Invalid rate!\n");
> + return -EINVAL;
> + }
> +
> + /* oversamplerate adjust */
> + if (params_rate(params) >= 24000)
> + regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE,
> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE);
> + else
> + regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE, 0);
> +
> + return 0;
> +}
> +
> +static int sun50i_dmic_trigger(struct snd_pcm_substream *substream, int cmd,
> + struct snd_soc_dai *dai)
> +{
> + int ret = 0;
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> +
> + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
> + return -EINVAL;
> +
> + switch (cmd) {
> + case SNDRV_PCM_TRIGGER_START:
> + case SNDRV_PCM_TRIGGER_RESUME:
> + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> + sun50i_snd_rxctrl_enable(substream, host, true);
> + break;
> +
> + case SNDRV_PCM_TRIGGER_STOP:
> + case SNDRV_PCM_TRIGGER_SUSPEND:
> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> + sun50i_snd_rxctrl_enable(substream, host, false);
> + break;
> +
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + return ret;
> +}
> +
> +static int sun50i_dmic_set_sysclk(struct snd_soc_dai *dai, int clk_id,
> + unsigned int freq, int dir)
> +{
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> +
> + if (clk_set_rate(host->dmic_clk, freq)) {
Currently, mainline does not have any sunxi-specific machine drivers. We
are using simple-audio-card for sun50i. And asoc_simple_hw_params() only
calls .set_sysclk if mclk-fs is set to some fixed value, from the device
tree. However, for sunxi, mclk-fs cannot be a fixed value, because
sysclk is the same 22/24 MHz regardless of sample rate. So .set_sysclk
will never be called.
So for now I would suggest calling clk_set_rate() from .hw_params, using
something like sun8i_codec_get_sysclk_rate() to get the right frequency.
> + dev_err(dai->dev, "Freq : %u not support\n", freq);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int sun50i_dmic_soc_dai_probe(struct snd_soc_dai *dai)
> +{
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> +
> + snd_soc_dai_init_dma_data(dai, NULL, &host->dma_params_rx);
> +
> + return 0;
> +}
> +
> +static const struct snd_soc_dai_ops sun50i_dmic_dai_ops = {
> + .startup = sun50i_dmic_startup,
> + .trigger = sun50i_dmic_trigger,
> + .hw_params = sun50i_dmic_hw_params,
> + .set_sysclk = sun50i_dmic_set_sysclk,
> +};
> +
> +static const struct regmap_config sun50i_dmic_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .max_register = SUN50I_DMIC_VERSION,
> + .cache_type = REGCACHE_NONE,
> +};
> +
> +#define SUN50I_DMIC_RATES (SNDRV_PCM_RATE_8000_48000 | SNDRV_PCM_RATE_KNOT)
If you use SNDRV_PCM_RATE_KNOT, you also need to provide a list of
sample rates as a constraint in .startup.
> +#define SUN50I_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE)
> +
> +static struct snd_soc_dai_driver sun50i_dmic_dai = {
> + .capture = {
> + .channels_min = 1,
> + .channels_max = 8,
> + .rates = SUN50I_DMIC_RATES,
> + .formats = SUN50I_FORMATS,
> + },
> + .probe = sun50i_dmic_soc_dai_probe,
> + .ops = &sun50i_dmic_dai_ops,
> + .name = "dmic",
> +};
> +
> +static const struct of_device_id sun50i_dmic_of_match[] = {
> + {
> + .compatible = "allwinner,sun50i-h6-dmic",
> + },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, sun50i_dmic_of_match);
> +
> +static const struct snd_soc_component_driver sun50i_dmic_component = {
> + .name = "sun50i-dmic",
> +};
> +
> +static int sun50i_dmic_runtime_suspend(struct device *dev)
> +{
> + struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
> +
> + clk_disable_unprepare(host->dmic_clk);
> + clk_disable_unprepare(host->apb_clk);
> +
> + return 0;
> +}
> +
> +static int sun50i_dmic_runtime_resume(struct device *dev)
> +{
> + struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = clk_prepare_enable(host->dmic_clk);
> + if (ret)
> + return ret;
> + ret = clk_prepare_enable(host->apb_clk);
> + if (ret)
> + clk_disable_unprepare(host->dmic_clk);
> +
> + return ret;
> +}
> +
> +static int sun50i_dmic_probe(struct platform_device *pdev)
> +{
> + struct sun50i_dmic_dev *host;
> + struct resource *res;
> + int ret;
> + void __iomem *base;
> +
> + host = devm_kzalloc(&pdev->dev, sizeof(*host), GFP_KERNEL);
> + if (!host)
> + return -ENOMEM;
> +
> + host->pdev = pdev;
> +
> + /* Get the addresses */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(base))
> + return dev_err_probe(&pdev->dev, PTR_ERR(base),
> + "get resource failed.\n");
> +
> + host->regmap = devm_regmap_init_mmio(&pdev->dev, base,
> + &sun50i_dmic_regmap_config);
> +
> + /* Clocks */
> + host->apb_clk = devm_clk_get(&pdev->dev, "apb");
> + if (IS_ERR(host->apb_clk))
> + return dev_err_probe(&pdev->dev, PTR_ERR(host->apb_clk),
> + "failed to get apb clock.\n");
> +
> + host->dmic_clk = devm_clk_get(&pdev->dev, "dmic");
> + if (IS_ERR(host->dmic_clk))
> + return dev_err_probe(&pdev->dev, PTR_ERR(host->dmic_clk),
> + "failed to get dmic clock.\n");
> +
> + host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
> + host->dma_params_rx.maxburst = 8;
> + host->dma_params_rx.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
This needs to be DMA_SLAVE_BUSWIDTH_4_BYTES if you are using larger than
16-bit samples. See for example sun4i_i2s_hw_params().
Regards,
Samuel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-16 3:06 ` Samuel Holland
0 siblings, 0 replies; 36+ messages in thread
From: Samuel Holland @ 2021-06-16 3:06 UTC (permalink / raw)
To: Ban Tao, lgirdwood, broonie, perex, tiwai, mripard, wens,
jernej.skrabec, p.zabel, krzk
Cc: linux-kernel, alsa-devel, linux-arm-kernel, linux-sunxi
Hello,
On 6/15/21 8:03 AM, Ban Tao wrote:
> The Allwinner H6 and later SoCs have an DMIC block
> which is capable of capture.
>
> Signed-off-by: Ban Tao <fengzheng923@gmail.com>
> ---
> MAINTAINERS | 7 +
> sound/soc/sunxi/Kconfig | 8 +
> sound/soc/sunxi/Makefile | 1 +
> sound/soc/sunxi/sun50i-dmic.c | 408 ++++++++++++++++++++++++++++++++++
> 4 files changed, 424 insertions(+)
> create mode 100644 sound/soc/sunxi/sun50i-dmic.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b4094f07214e..1b87225c39b0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -760,6 +760,13 @@ L: linux-media@vger.kernel.org
> S: Maintained
> F: drivers/staging/media/sunxi/cedrus/
>
> +ALLWINNER DMIC DRIVERS
> +M: Ban Tao <fengzheng923@gmail.com>
> +L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> +S: Maintained
> +F: Documentation/devicetree/bindings/sound/allwinner,sun50i-h6-dmic.yaml
> +F: sound/soc/sunxi/sun50i-dmic.c
> +
> ALPHA PORT
> M: Richard Henderson <rth@twiddle.net>
> M: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
> index ddcaaa98d3cb..2a3bf7722e11 100644
> --- a/sound/soc/sunxi/Kconfig
> +++ b/sound/soc/sunxi/Kconfig
> @@ -56,6 +56,14 @@ config SND_SUN4I_SPDIF
> Say Y or M to add support for the S/PDIF audio block in the Allwinner
> A10 and affiliated SoCs.
>
> +config SND_SUN50I_DMIC
> + tristate "Allwinner H6 DMIC Support"
> + depends on (OF && ARCH_SUNXI) || COMPILE_TEST
> + select SND_SOC_GENERIC_DMAENGINE_PCM
> + help
> + Say Y or M to add support for the DMIC audio block in the Allwinner
> + H6 and affiliated SoCs.
> +
> config SND_SUN8I_ADDA_PR_REGMAP
> tristate
> select REGMAP
> diff --git a/sound/soc/sunxi/Makefile b/sound/soc/sunxi/Makefile
> index a86be340a076..4483fe9c94ef 100644
> --- a/sound/soc/sunxi/Makefile
> +++ b/sound/soc/sunxi/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_SND_SUN8I_CODEC_ANALOG) += sun8i-codec-analog.o
> obj-$(CONFIG_SND_SUN50I_CODEC_ANALOG) += sun50i-codec-analog.o
> obj-$(CONFIG_SND_SUN8I_CODEC) += sun8i-codec.o
> obj-$(CONFIG_SND_SUN8I_ADDA_PR_REGMAP) += sun8i-adda-pr-regmap.o
> +obj-$(CONFIG_SND_SUN50I_DMIC) += sun50i-dmic.o
> diff --git a/sound/soc/sunxi/sun50i-dmic.c b/sound/soc/sunxi/sun50i-dmic.c
> new file mode 100644
> index 000000000000..78d512d93974
> --- /dev/null
> +++ b/sound/soc/sunxi/sun50i-dmic.c
> @@ -0,0 +1,408 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * ALSA SoC DMIC Audio Layer
> + *
> + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/of_device.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +#include <sound/dmaengine_pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +
> +
> +#define SUN50I_DMIC_EN_CTL (0x00)
> + #define SUN50I_DMIC_EN_CTL_GLOBE BIT(8)
> + #define SUN50I_DMIC_EN_CTL_CHAN(v) ((v) << 0)
> + #define SUN50I_DMIC_EN_CTL_CHAN_MASK GENMASK(7, 0)
> +#define SUN50I_DMIC_SR (0x04)
> + #define SUN50I_DMIC_SR_SAMOLE_RATE(v) ((v) << 0)
> + #define SUN50I_DMIC_SR_SAMOLE_RATE_MASK GENMASK(2, 0)
> +#define SUN50I_DMIC_CTL (0x08)
> + #define SUN50I_DMIC_CTL_OVERSAMPLE_RATE BIT(0)
> +#define SUN50I_DMIC_DATA (0x10)
> +#define SUN50I_DMIC_INTC (0x14)
> + #define SUN50I_DMIC_FIFO_DRQ_EN BIT(2)
> +#define SUN50I_DMIC_INT_STA (0x18)
> + #define SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING BIT(1)
> + #define SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING BIT(0)
> +#define SUN50I_DMIC_RXFIFO_CTL (0x1c)
> + #define SUN50I_DMIC_RXFIFO_CTL_FLUSH BIT(31)
> + #define SUN50I_DMIC_RXFIFO_CTL_MODE BIT(9)
> + #define SUN50I_DMIC_RXFIFO_CTL_RESOLUTION BIT(8)
> +#define SUN50I_DMIC_CH_NUM (0x24)
> + #define SUN50I_DMIC_CH_NUM_N(v) ((v) << 0)
> + #define SUN50I_DMIC_CH_NUM_N_MASK GENMASK(2, 0)
> +#define SUN50I_DMIC_CNT (0x2c)
> + #define SUN50I_DMIC_CNT_N BIT(0)
> +#define SUN50I_DMIC_HPF_CTRL (0x38)
> +#define SUN50I_DMIC_VERSION (0x50)
> +
> +
> +struct sun50i_dmic_dev {
> + struct platform_device *pdev;
> + struct clk *dmic_clk;
> + struct clk *apb_clk;
> + struct reset_control *rst;
> + struct regmap *regmap;
> + struct snd_dmaengine_dai_dma_data dma_params_rx;
> + unsigned int chan_en;
> +};
> +
> +struct dmic_rate {
> + unsigned int samplerate;
> + unsigned int rate_bit;
> +};
> +
> +static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream *substream,
> + struct sun50i_dmic_dev *host, bool enable)
> +{
> + if (enable) {
> + /* DRQ ENABLE */
> + regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
> + SUN50I_DMIC_FIFO_DRQ_EN,
> + SUN50I_DMIC_FIFO_DRQ_EN);
> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
> + SUN50I_DMIC_EN_CTL_CHAN_MASK,
> + SUN50I_DMIC_EN_CTL_CHAN(host->chan_en));
> + /* Global enable */
> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
> + SUN50I_DMIC_EN_CTL_GLOBE,
> + SUN50I_DMIC_EN_CTL_GLOBE);
> + } else {
> + /* DRQ DISABLE */
> + regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
> + SUN50I_DMIC_FIFO_DRQ_EN, 0);
> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
> + SUN50I_DMIC_EN_CTL_CHAN_MASK,
> + SUN50I_DMIC_EN_CTL_CHAN(0));
> + /* Global disable */
> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
> + SUN50I_DMIC_EN_CTL_GLOBE, 0);
> + }
> +}
> +
> +static int sun50i_dmic_startup(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *cpu_dai)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
This is equivalent to the simpler code you use below:
struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
> +
> + /* only support capture */
> + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
> + return -EINVAL;
> +
> + regmap_write(host->regmap, SUN50I_DMIC_INT_STA,
> + SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING |
> + SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING);
The interrupt is never enabled, so do we really care about IRQ status?
> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> + SUN50I_DMIC_RXFIFO_CTL_FLUSH,
> + SUN50I_DMIC_RXFIFO_CTL_FLUSH);
> + regmap_write(host->regmap, SUN50I_DMIC_CNT, SUN50I_DMIC_CNT_N);
> +
> + return 0;
> +}
> +
> +static int sun50i_dmic_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params,
> + struct snd_soc_dai *cpu_dai)
> +{
> + int i = 0;
> + unsigned long rate = params_rate(params);
> + unsigned int channels = params_channels(params);
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
> + struct platform_device *pdev = host->pdev;
> + struct dmic_rate dmic_rate_s[] = {
> + {44100, 0x0},
> + {48000, 0x0},
> + {22050, 0x2},
> + {24000, 0x2},
> + {11025, 0x4},
> + {12000, 0x4},
> + {32000, 0x1},
> + {16000, 0x3},
> + {8000, 0x5},
> + };
> +
> + /* DMIC num is N+1 */
> + regmap_update_bits(host->regmap, SUN50I_DMIC_CH_NUM,
> + SUN50I_DMIC_CH_NUM_N_MASK,
> + SUN50I_DMIC_CH_NUM_N(channels - 1));
> + host->chan_en = (1 << channels) - 1;
> + regmap_write(host->regmap, SUN50I_DMIC_HPF_CTRL, host->chan_en);
> +
> + switch (params_format(params)) {
> + case SNDRV_PCM_FORMAT_S16_LE:
> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> + SUN50I_DMIC_RXFIFO_CTL_MODE,
> + SUN50I_DMIC_RXFIFO_CTL_MODE);
> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION, 0);
> + break;
> + case SNDRV_PCM_FORMAT_S24_LE:
> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> + SUN50I_DMIC_RXFIFO_CTL_MODE, 0);
In SNDRV_PCM_FORMAT_S24_LE, the sample is in the three least significant
bytes, so wouldn't we want to still use mode 1 here? To me, mode 0 only
makes sense when using SNDRV_PCM_FORMAT_S32_LE.
> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION,
> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION);
> + break;
> + default:
> + dev_err(&pdev->dev, "Invalid format!\n");
The calls to dev_err() can use cpu_dai->dev. Then you don't need to keep
a pointer to pdev in the driver data.
> + return -EINVAL;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(dmic_rate_s); i++) {
> + if (dmic_rate_s[i].samplerate == rate) {
> + regmap_update_bits(host->regmap, SUN50I_DMIC_SR,
> + SUN50I_DMIC_SR_SAMOLE_RATE_MASK,
> + SUN50I_DMIC_SR_SAMOLE_RATE(dmic_rate_s[i].rate_bit));
> + break;
> + }
> + }
> + if (i == ARRAY_SIZE(dmic_rate_s)) {
> + dev_err(&pdev->dev, "Invalid rate!\n");
> + return -EINVAL;
> + }
> +
> + /* oversamplerate adjust */
> + if (params_rate(params) >= 24000)
> + regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE,
> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE);
> + else
> + regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE, 0);
> +
> + return 0;
> +}
> +
> +static int sun50i_dmic_trigger(struct snd_pcm_substream *substream, int cmd,
> + struct snd_soc_dai *dai)
> +{
> + int ret = 0;
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> +
> + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
> + return -EINVAL;
> +
> + switch (cmd) {
> + case SNDRV_PCM_TRIGGER_START:
> + case SNDRV_PCM_TRIGGER_RESUME:
> + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> + sun50i_snd_rxctrl_enable(substream, host, true);
> + break;
> +
> + case SNDRV_PCM_TRIGGER_STOP:
> + case SNDRV_PCM_TRIGGER_SUSPEND:
> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> + sun50i_snd_rxctrl_enable(substream, host, false);
> + break;
> +
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + return ret;
> +}
> +
> +static int sun50i_dmic_set_sysclk(struct snd_soc_dai *dai, int clk_id,
> + unsigned int freq, int dir)
> +{
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> +
> + if (clk_set_rate(host->dmic_clk, freq)) {
Currently, mainline does not have any sunxi-specific machine drivers. We
are using simple-audio-card for sun50i. And asoc_simple_hw_params() only
calls .set_sysclk if mclk-fs is set to some fixed value, from the device
tree. However, for sunxi, mclk-fs cannot be a fixed value, because
sysclk is the same 22/24 MHz regardless of sample rate. So .set_sysclk
will never be called.
So for now I would suggest calling clk_set_rate() from .hw_params, using
something like sun8i_codec_get_sysclk_rate() to get the right frequency.
> + dev_err(dai->dev, "Freq : %u not support\n", freq);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int sun50i_dmic_soc_dai_probe(struct snd_soc_dai *dai)
> +{
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> +
> + snd_soc_dai_init_dma_data(dai, NULL, &host->dma_params_rx);
> +
> + return 0;
> +}
> +
> +static const struct snd_soc_dai_ops sun50i_dmic_dai_ops = {
> + .startup = sun50i_dmic_startup,
> + .trigger = sun50i_dmic_trigger,
> + .hw_params = sun50i_dmic_hw_params,
> + .set_sysclk = sun50i_dmic_set_sysclk,
> +};
> +
> +static const struct regmap_config sun50i_dmic_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .max_register = SUN50I_DMIC_VERSION,
> + .cache_type = REGCACHE_NONE,
> +};
> +
> +#define SUN50I_DMIC_RATES (SNDRV_PCM_RATE_8000_48000 | SNDRV_PCM_RATE_KNOT)
If you use SNDRV_PCM_RATE_KNOT, you also need to provide a list of
sample rates as a constraint in .startup.
> +#define SUN50I_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE)
> +
> +static struct snd_soc_dai_driver sun50i_dmic_dai = {
> + .capture = {
> + .channels_min = 1,
> + .channels_max = 8,
> + .rates = SUN50I_DMIC_RATES,
> + .formats = SUN50I_FORMATS,
> + },
> + .probe = sun50i_dmic_soc_dai_probe,
> + .ops = &sun50i_dmic_dai_ops,
> + .name = "dmic",
> +};
> +
> +static const struct of_device_id sun50i_dmic_of_match[] = {
> + {
> + .compatible = "allwinner,sun50i-h6-dmic",
> + },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, sun50i_dmic_of_match);
> +
> +static const struct snd_soc_component_driver sun50i_dmic_component = {
> + .name = "sun50i-dmic",
> +};
> +
> +static int sun50i_dmic_runtime_suspend(struct device *dev)
> +{
> + struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
> +
> + clk_disable_unprepare(host->dmic_clk);
> + clk_disable_unprepare(host->apb_clk);
> +
> + return 0;
> +}
> +
> +static int sun50i_dmic_runtime_resume(struct device *dev)
> +{
> + struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = clk_prepare_enable(host->dmic_clk);
> + if (ret)
> + return ret;
> + ret = clk_prepare_enable(host->apb_clk);
> + if (ret)
> + clk_disable_unprepare(host->dmic_clk);
> +
> + return ret;
> +}
> +
> +static int sun50i_dmic_probe(struct platform_device *pdev)
> +{
> + struct sun50i_dmic_dev *host;
> + struct resource *res;
> + int ret;
> + void __iomem *base;
> +
> + host = devm_kzalloc(&pdev->dev, sizeof(*host), GFP_KERNEL);
> + if (!host)
> + return -ENOMEM;
> +
> + host->pdev = pdev;
> +
> + /* Get the addresses */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(base))
> + return dev_err_probe(&pdev->dev, PTR_ERR(base),
> + "get resource failed.\n");
> +
> + host->regmap = devm_regmap_init_mmio(&pdev->dev, base,
> + &sun50i_dmic_regmap_config);
> +
> + /* Clocks */
> + host->apb_clk = devm_clk_get(&pdev->dev, "apb");
> + if (IS_ERR(host->apb_clk))
> + return dev_err_probe(&pdev->dev, PTR_ERR(host->apb_clk),
> + "failed to get apb clock.\n");
> +
> + host->dmic_clk = devm_clk_get(&pdev->dev, "dmic");
> + if (IS_ERR(host->dmic_clk))
> + return dev_err_probe(&pdev->dev, PTR_ERR(host->dmic_clk),
> + "failed to get dmic clock.\n");
> +
> + host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
> + host->dma_params_rx.maxburst = 8;
> + host->dma_params_rx.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
This needs to be DMA_SLAVE_BUSWIDTH_4_BYTES if you are using larger than
16-bit samples. See for example sun4i_i2s_hw_params().
Regards,
Samuel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-16 3:06 ` Samuel Holland
0 siblings, 0 replies; 36+ messages in thread
From: Samuel Holland @ 2021-06-16 3:06 UTC (permalink / raw)
To: Ban Tao, lgirdwood, broonie, perex, tiwai, mripard, wens,
jernej.skrabec, p.zabel, krzk
Cc: alsa-devel, linux-kernel, linux-arm-kernel, linux-sunxi
Hello,
On 6/15/21 8:03 AM, Ban Tao wrote:
> The Allwinner H6 and later SoCs have an DMIC block
> which is capable of capture.
>
> Signed-off-by: Ban Tao <fengzheng923@gmail.com>
> ---
> MAINTAINERS | 7 +
> sound/soc/sunxi/Kconfig | 8 +
> sound/soc/sunxi/Makefile | 1 +
> sound/soc/sunxi/sun50i-dmic.c | 408 ++++++++++++++++++++++++++++++++++
> 4 files changed, 424 insertions(+)
> create mode 100644 sound/soc/sunxi/sun50i-dmic.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b4094f07214e..1b87225c39b0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -760,6 +760,13 @@ L: linux-media@vger.kernel.org
> S: Maintained
> F: drivers/staging/media/sunxi/cedrus/
>
> +ALLWINNER DMIC DRIVERS
> +M: Ban Tao <fengzheng923@gmail.com>
> +L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> +S: Maintained
> +F: Documentation/devicetree/bindings/sound/allwinner,sun50i-h6-dmic.yaml
> +F: sound/soc/sunxi/sun50i-dmic.c
> +
> ALPHA PORT
> M: Richard Henderson <rth@twiddle.net>
> M: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
> index ddcaaa98d3cb..2a3bf7722e11 100644
> --- a/sound/soc/sunxi/Kconfig
> +++ b/sound/soc/sunxi/Kconfig
> @@ -56,6 +56,14 @@ config SND_SUN4I_SPDIF
> Say Y or M to add support for the S/PDIF audio block in the Allwinner
> A10 and affiliated SoCs.
>
> +config SND_SUN50I_DMIC
> + tristate "Allwinner H6 DMIC Support"
> + depends on (OF && ARCH_SUNXI) || COMPILE_TEST
> + select SND_SOC_GENERIC_DMAENGINE_PCM
> + help
> + Say Y or M to add support for the DMIC audio block in the Allwinner
> + H6 and affiliated SoCs.
> +
> config SND_SUN8I_ADDA_PR_REGMAP
> tristate
> select REGMAP
> diff --git a/sound/soc/sunxi/Makefile b/sound/soc/sunxi/Makefile
> index a86be340a076..4483fe9c94ef 100644
> --- a/sound/soc/sunxi/Makefile
> +++ b/sound/soc/sunxi/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_SND_SUN8I_CODEC_ANALOG) += sun8i-codec-analog.o
> obj-$(CONFIG_SND_SUN50I_CODEC_ANALOG) += sun50i-codec-analog.o
> obj-$(CONFIG_SND_SUN8I_CODEC) += sun8i-codec.o
> obj-$(CONFIG_SND_SUN8I_ADDA_PR_REGMAP) += sun8i-adda-pr-regmap.o
> +obj-$(CONFIG_SND_SUN50I_DMIC) += sun50i-dmic.o
> diff --git a/sound/soc/sunxi/sun50i-dmic.c b/sound/soc/sunxi/sun50i-dmic.c
> new file mode 100644
> index 000000000000..78d512d93974
> --- /dev/null
> +++ b/sound/soc/sunxi/sun50i-dmic.c
> @@ -0,0 +1,408 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * ALSA SoC DMIC Audio Layer
> + *
> + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/of_device.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +#include <sound/dmaengine_pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +
> +
> +#define SUN50I_DMIC_EN_CTL (0x00)
> + #define SUN50I_DMIC_EN_CTL_GLOBE BIT(8)
> + #define SUN50I_DMIC_EN_CTL_CHAN(v) ((v) << 0)
> + #define SUN50I_DMIC_EN_CTL_CHAN_MASK GENMASK(7, 0)
> +#define SUN50I_DMIC_SR (0x04)
> + #define SUN50I_DMIC_SR_SAMOLE_RATE(v) ((v) << 0)
> + #define SUN50I_DMIC_SR_SAMOLE_RATE_MASK GENMASK(2, 0)
> +#define SUN50I_DMIC_CTL (0x08)
> + #define SUN50I_DMIC_CTL_OVERSAMPLE_RATE BIT(0)
> +#define SUN50I_DMIC_DATA (0x10)
> +#define SUN50I_DMIC_INTC (0x14)
> + #define SUN50I_DMIC_FIFO_DRQ_EN BIT(2)
> +#define SUN50I_DMIC_INT_STA (0x18)
> + #define SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING BIT(1)
> + #define SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING BIT(0)
> +#define SUN50I_DMIC_RXFIFO_CTL (0x1c)
> + #define SUN50I_DMIC_RXFIFO_CTL_FLUSH BIT(31)
> + #define SUN50I_DMIC_RXFIFO_CTL_MODE BIT(9)
> + #define SUN50I_DMIC_RXFIFO_CTL_RESOLUTION BIT(8)
> +#define SUN50I_DMIC_CH_NUM (0x24)
> + #define SUN50I_DMIC_CH_NUM_N(v) ((v) << 0)
> + #define SUN50I_DMIC_CH_NUM_N_MASK GENMASK(2, 0)
> +#define SUN50I_DMIC_CNT (0x2c)
> + #define SUN50I_DMIC_CNT_N BIT(0)
> +#define SUN50I_DMIC_HPF_CTRL (0x38)
> +#define SUN50I_DMIC_VERSION (0x50)
> +
> +
> +struct sun50i_dmic_dev {
> + struct platform_device *pdev;
> + struct clk *dmic_clk;
> + struct clk *apb_clk;
> + struct reset_control *rst;
> + struct regmap *regmap;
> + struct snd_dmaengine_dai_dma_data dma_params_rx;
> + unsigned int chan_en;
> +};
> +
> +struct dmic_rate {
> + unsigned int samplerate;
> + unsigned int rate_bit;
> +};
> +
> +static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream *substream,
> + struct sun50i_dmic_dev *host, bool enable)
> +{
> + if (enable) {
> + /* DRQ ENABLE */
> + regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
> + SUN50I_DMIC_FIFO_DRQ_EN,
> + SUN50I_DMIC_FIFO_DRQ_EN);
> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
> + SUN50I_DMIC_EN_CTL_CHAN_MASK,
> + SUN50I_DMIC_EN_CTL_CHAN(host->chan_en));
> + /* Global enable */
> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
> + SUN50I_DMIC_EN_CTL_GLOBE,
> + SUN50I_DMIC_EN_CTL_GLOBE);
> + } else {
> + /* DRQ DISABLE */
> + regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
> + SUN50I_DMIC_FIFO_DRQ_EN, 0);
> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
> + SUN50I_DMIC_EN_CTL_CHAN_MASK,
> + SUN50I_DMIC_EN_CTL_CHAN(0));
> + /* Global disable */
> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
> + SUN50I_DMIC_EN_CTL_GLOBE, 0);
> + }
> +}
> +
> +static int sun50i_dmic_startup(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *cpu_dai)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
This is equivalent to the simpler code you use below:
struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
> +
> + /* only support capture */
> + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
> + return -EINVAL;
> +
> + regmap_write(host->regmap, SUN50I_DMIC_INT_STA,
> + SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING |
> + SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING);
The interrupt is never enabled, so do we really care about IRQ status?
> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> + SUN50I_DMIC_RXFIFO_CTL_FLUSH,
> + SUN50I_DMIC_RXFIFO_CTL_FLUSH);
> + regmap_write(host->regmap, SUN50I_DMIC_CNT, SUN50I_DMIC_CNT_N);
> +
> + return 0;
> +}
> +
> +static int sun50i_dmic_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params,
> + struct snd_soc_dai *cpu_dai)
> +{
> + int i = 0;
> + unsigned long rate = params_rate(params);
> + unsigned int channels = params_channels(params);
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
> + struct platform_device *pdev = host->pdev;
> + struct dmic_rate dmic_rate_s[] = {
> + {44100, 0x0},
> + {48000, 0x0},
> + {22050, 0x2},
> + {24000, 0x2},
> + {11025, 0x4},
> + {12000, 0x4},
> + {32000, 0x1},
> + {16000, 0x3},
> + {8000, 0x5},
> + };
> +
> + /* DMIC num is N+1 */
> + regmap_update_bits(host->regmap, SUN50I_DMIC_CH_NUM,
> + SUN50I_DMIC_CH_NUM_N_MASK,
> + SUN50I_DMIC_CH_NUM_N(channels - 1));
> + host->chan_en = (1 << channels) - 1;
> + regmap_write(host->regmap, SUN50I_DMIC_HPF_CTRL, host->chan_en);
> +
> + switch (params_format(params)) {
> + case SNDRV_PCM_FORMAT_S16_LE:
> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> + SUN50I_DMIC_RXFIFO_CTL_MODE,
> + SUN50I_DMIC_RXFIFO_CTL_MODE);
> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION, 0);
> + break;
> + case SNDRV_PCM_FORMAT_S24_LE:
> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> + SUN50I_DMIC_RXFIFO_CTL_MODE, 0);
In SNDRV_PCM_FORMAT_S24_LE, the sample is in the three least significant
bytes, so wouldn't we want to still use mode 1 here? To me, mode 0 only
makes sense when using SNDRV_PCM_FORMAT_S32_LE.
> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION,
> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION);
> + break;
> + default:
> + dev_err(&pdev->dev, "Invalid format!\n");
The calls to dev_err() can use cpu_dai->dev. Then you don't need to keep
a pointer to pdev in the driver data.
> + return -EINVAL;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(dmic_rate_s); i++) {
> + if (dmic_rate_s[i].samplerate == rate) {
> + regmap_update_bits(host->regmap, SUN50I_DMIC_SR,
> + SUN50I_DMIC_SR_SAMOLE_RATE_MASK,
> + SUN50I_DMIC_SR_SAMOLE_RATE(dmic_rate_s[i].rate_bit));
> + break;
> + }
> + }
> + if (i == ARRAY_SIZE(dmic_rate_s)) {
> + dev_err(&pdev->dev, "Invalid rate!\n");
> + return -EINVAL;
> + }
> +
> + /* oversamplerate adjust */
> + if (params_rate(params) >= 24000)
> + regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE,
> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE);
> + else
> + regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE, 0);
> +
> + return 0;
> +}
> +
> +static int sun50i_dmic_trigger(struct snd_pcm_substream *substream, int cmd,
> + struct snd_soc_dai *dai)
> +{
> + int ret = 0;
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> +
> + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
> + return -EINVAL;
> +
> + switch (cmd) {
> + case SNDRV_PCM_TRIGGER_START:
> + case SNDRV_PCM_TRIGGER_RESUME:
> + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> + sun50i_snd_rxctrl_enable(substream, host, true);
> + break;
> +
> + case SNDRV_PCM_TRIGGER_STOP:
> + case SNDRV_PCM_TRIGGER_SUSPEND:
> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> + sun50i_snd_rxctrl_enable(substream, host, false);
> + break;
> +
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + return ret;
> +}
> +
> +static int sun50i_dmic_set_sysclk(struct snd_soc_dai *dai, int clk_id,
> + unsigned int freq, int dir)
> +{
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> +
> + if (clk_set_rate(host->dmic_clk, freq)) {
Currently, mainline does not have any sunxi-specific machine drivers. We
are using simple-audio-card for sun50i. And asoc_simple_hw_params() only
calls .set_sysclk if mclk-fs is set to some fixed value, from the device
tree. However, for sunxi, mclk-fs cannot be a fixed value, because
sysclk is the same 22/24 MHz regardless of sample rate. So .set_sysclk
will never be called.
So for now I would suggest calling clk_set_rate() from .hw_params, using
something like sun8i_codec_get_sysclk_rate() to get the right frequency.
> + dev_err(dai->dev, "Freq : %u not support\n", freq);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int sun50i_dmic_soc_dai_probe(struct snd_soc_dai *dai)
> +{
> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> +
> + snd_soc_dai_init_dma_data(dai, NULL, &host->dma_params_rx);
> +
> + return 0;
> +}
> +
> +static const struct snd_soc_dai_ops sun50i_dmic_dai_ops = {
> + .startup = sun50i_dmic_startup,
> + .trigger = sun50i_dmic_trigger,
> + .hw_params = sun50i_dmic_hw_params,
> + .set_sysclk = sun50i_dmic_set_sysclk,
> +};
> +
> +static const struct regmap_config sun50i_dmic_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .max_register = SUN50I_DMIC_VERSION,
> + .cache_type = REGCACHE_NONE,
> +};
> +
> +#define SUN50I_DMIC_RATES (SNDRV_PCM_RATE_8000_48000 | SNDRV_PCM_RATE_KNOT)
If you use SNDRV_PCM_RATE_KNOT, you also need to provide a list of
sample rates as a constraint in .startup.
> +#define SUN50I_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE)
> +
> +static struct snd_soc_dai_driver sun50i_dmic_dai = {
> + .capture = {
> + .channels_min = 1,
> + .channels_max = 8,
> + .rates = SUN50I_DMIC_RATES,
> + .formats = SUN50I_FORMATS,
> + },
> + .probe = sun50i_dmic_soc_dai_probe,
> + .ops = &sun50i_dmic_dai_ops,
> + .name = "dmic",
> +};
> +
> +static const struct of_device_id sun50i_dmic_of_match[] = {
> + {
> + .compatible = "allwinner,sun50i-h6-dmic",
> + },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, sun50i_dmic_of_match);
> +
> +static const struct snd_soc_component_driver sun50i_dmic_component = {
> + .name = "sun50i-dmic",
> +};
> +
> +static int sun50i_dmic_runtime_suspend(struct device *dev)
> +{
> + struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
> +
> + clk_disable_unprepare(host->dmic_clk);
> + clk_disable_unprepare(host->apb_clk);
> +
> + return 0;
> +}
> +
> +static int sun50i_dmic_runtime_resume(struct device *dev)
> +{
> + struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = clk_prepare_enable(host->dmic_clk);
> + if (ret)
> + return ret;
> + ret = clk_prepare_enable(host->apb_clk);
> + if (ret)
> + clk_disable_unprepare(host->dmic_clk);
> +
> + return ret;
> +}
> +
> +static int sun50i_dmic_probe(struct platform_device *pdev)
> +{
> + struct sun50i_dmic_dev *host;
> + struct resource *res;
> + int ret;
> + void __iomem *base;
> +
> + host = devm_kzalloc(&pdev->dev, sizeof(*host), GFP_KERNEL);
> + if (!host)
> + return -ENOMEM;
> +
> + host->pdev = pdev;
> +
> + /* Get the addresses */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(base))
> + return dev_err_probe(&pdev->dev, PTR_ERR(base),
> + "get resource failed.\n");
> +
> + host->regmap = devm_regmap_init_mmio(&pdev->dev, base,
> + &sun50i_dmic_regmap_config);
> +
> + /* Clocks */
> + host->apb_clk = devm_clk_get(&pdev->dev, "apb");
> + if (IS_ERR(host->apb_clk))
> + return dev_err_probe(&pdev->dev, PTR_ERR(host->apb_clk),
> + "failed to get apb clock.\n");
> +
> + host->dmic_clk = devm_clk_get(&pdev->dev, "dmic");
> + if (IS_ERR(host->dmic_clk))
> + return dev_err_probe(&pdev->dev, PTR_ERR(host->dmic_clk),
> + "failed to get dmic clock.\n");
> +
> + host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
> + host->dma_params_rx.maxburst = 8;
> + host->dma_params_rx.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
This needs to be DMA_SLAVE_BUSWIDTH_4_BYTES if you are using larger than
16-bit samples. See for example sun4i_i2s_hw_params().
Regards,
Samuel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
2021-06-16 3:06 ` Samuel Holland
(?)
(?)
@ 2021-06-16 6:21 ` 班涛
2021-06-19 19:52 ` Samuel Holland
-1 siblings, 1 reply; 36+ messages in thread
From: 班涛 @ 2021-06-16 6:21 UTC (permalink / raw)
To: Samuel Holland
Cc: alsa-devel, krzk, linux-kernel, tiwai, mripard, lgirdwood, wens,
broonie, jernej.skrabec, p.zabel, linux-sunxi, linux-arm-kernel
Hi,
Samuel Holland <samuel@sholland.org> 于2021年6月16日周三 上午11:06写道:
> Hello,
>
> On 6/15/21 8:03 AM, Ban Tao wrote:
> > The Allwinner H6 and later SoCs have an DMIC block
> > which is capable of capture.
> >
> > Signed-off-by: Ban Tao <fengzheng923@gmail.com>
> > ---
> > MAINTAINERS | 7 +
> > sound/soc/sunxi/Kconfig | 8 +
> > sound/soc/sunxi/Makefile | 1 +
> > sound/soc/sunxi/sun50i-dmic.c | 408 ++++++++++++++++++++++++++++++++++
> > 4 files changed, 424 insertions(+)
> > create mode 100644 sound/soc/sunxi/sun50i-dmic.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index b4094f07214e..1b87225c39b0 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -760,6 +760,13 @@ L: linux-media@vger.kernel.org
> > S: Maintained
> > F: drivers/staging/media/sunxi/cedrus/
> >
> > +ALLWINNER DMIC DRIVERS
> > +M: Ban Tao <fengzheng923@gmail.com>
> > +L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> > +S: Maintained
> > +F:
> Documentation/devicetree/bindings/sound/allwinner,sun50i-h6-dmic.yaml
> > +F: sound/soc/sunxi/sun50i-dmic.c
> > +
> > ALPHA PORT
> > M: Richard Henderson <rth@twiddle.net>
> > M: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> > diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
> > index ddcaaa98d3cb..2a3bf7722e11 100644
> > --- a/sound/soc/sunxi/Kconfig
> > +++ b/sound/soc/sunxi/Kconfig
> > @@ -56,6 +56,14 @@ config SND_SUN4I_SPDIF
> > Say Y or M to add support for the S/PDIF audio block in the
> Allwinner
> > A10 and affiliated SoCs.
> >
> > +config SND_SUN50I_DMIC
> > + tristate "Allwinner H6 DMIC Support"
> > + depends on (OF && ARCH_SUNXI) || COMPILE_TEST
> > + select SND_SOC_GENERIC_DMAENGINE_PCM
> > + help
> > + Say Y or M to add support for the DMIC audio block in the
> Allwinner
> > + H6 and affiliated SoCs.
> > +
> > config SND_SUN8I_ADDA_PR_REGMAP
> > tristate
> > select REGMAP
> > diff --git a/sound/soc/sunxi/Makefile b/sound/soc/sunxi/Makefile
> > index a86be340a076..4483fe9c94ef 100644
> > --- a/sound/soc/sunxi/Makefile
> > +++ b/sound/soc/sunxi/Makefile
> > @@ -6,3 +6,4 @@ obj-$(CONFIG_SND_SUN8I_CODEC_ANALOG) +=
> sun8i-codec-analog.o
> > obj-$(CONFIG_SND_SUN50I_CODEC_ANALOG) += sun50i-codec-analog.o
> > obj-$(CONFIG_SND_SUN8I_CODEC) += sun8i-codec.o
> > obj-$(CONFIG_SND_SUN8I_ADDA_PR_REGMAP) += sun8i-adda-pr-regmap.o
> > +obj-$(CONFIG_SND_SUN50I_DMIC) += sun50i-dmic.o
> > diff --git a/sound/soc/sunxi/sun50i-dmic.c
> b/sound/soc/sunxi/sun50i-dmic.c
> > new file mode 100644
> > index 000000000000..78d512d93974
> > --- /dev/null
> > +++ b/sound/soc/sunxi/sun50i-dmic.c
> > @@ -0,0 +1,408 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * ALSA SoC DMIC Audio Layer
> > + *
> > + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
> > + *
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/device.h>
> > +#include <linux/of_device.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/reset.h>
> > +#include <sound/dmaengine_pcm.h>
> > +#include <sound/pcm_params.h>
> > +#include <sound/soc.h>
> > +
> > +
> > +#define SUN50I_DMIC_EN_CTL (0x00)
> > + #define SUN50I_DMIC_EN_CTL_GLOBE BIT(8)
> > + #define SUN50I_DMIC_EN_CTL_CHAN(v) ((v) << 0)
> > + #define SUN50I_DMIC_EN_CTL_CHAN_MASK GENMASK(7, 0)
> > +#define SUN50I_DMIC_SR (0x04)
> > + #define SUN50I_DMIC_SR_SAMOLE_RATE(v) ((v) << 0)
> > + #define SUN50I_DMIC_SR_SAMOLE_RATE_MASK GENMASK(2, 0)
> > +#define SUN50I_DMIC_CTL (0x08)
> > + #define SUN50I_DMIC_CTL_OVERSAMPLE_RATE BIT(0)
> > +#define SUN50I_DMIC_DATA (0x10)
> > +#define SUN50I_DMIC_INTC (0x14)
> > + #define SUN50I_DMIC_FIFO_DRQ_EN BIT(2)
> > +#define SUN50I_DMIC_INT_STA (0x18)
> > + #define SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING BIT(1)
> > + #define SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING BIT(0)
> > +#define SUN50I_DMIC_RXFIFO_CTL (0x1c)
> > + #define SUN50I_DMIC_RXFIFO_CTL_FLUSH BIT(31)
> > + #define SUN50I_DMIC_RXFIFO_CTL_MODE BIT(9)
> > + #define SUN50I_DMIC_RXFIFO_CTL_RESOLUTION BIT(8)
> > +#define SUN50I_DMIC_CH_NUM (0x24)
> > + #define SUN50I_DMIC_CH_NUM_N(v) ((v) << 0)
> > + #define SUN50I_DMIC_CH_NUM_N_MASK GENMASK(2, 0)
> > +#define SUN50I_DMIC_CNT (0x2c)
> > + #define SUN50I_DMIC_CNT_N BIT(0)
> > +#define SUN50I_DMIC_HPF_CTRL (0x38)
> > +#define SUN50I_DMIC_VERSION (0x50)
> > +
> > +
> > +struct sun50i_dmic_dev {
> > + struct platform_device *pdev;
> > + struct clk *dmic_clk;
> > + struct clk *apb_clk;
> > + struct reset_control *rst;
> > + struct regmap *regmap;
> > + struct snd_dmaengine_dai_dma_data dma_params_rx;
> > + unsigned int chan_en;
> > +};
> > +
> > +struct dmic_rate {
> > + unsigned int samplerate;
> > + unsigned int rate_bit;
> > +};
> > +
> > +static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream
> *substream,
> > + struct sun50i_dmic_dev *host, bool
> enable)
> > +{
> > + if (enable) {
> > + /* DRQ ENABLE */
> > + regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
> > + SUN50I_DMIC_FIFO_DRQ_EN,
> > + SUN50I_DMIC_FIFO_DRQ_EN);
> > + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
> > + SUN50I_DMIC_EN_CTL_CHAN_MASK,
> > + SUN50I_DMIC_EN_CTL_CHAN(host->chan_en));
> > + /* Global enable */
> > + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
> > + SUN50I_DMIC_EN_CTL_GLOBE,
> > + SUN50I_DMIC_EN_CTL_GLOBE);
> > + } else {
> > + /* DRQ DISABLE */
> > + regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
> > + SUN50I_DMIC_FIFO_DRQ_EN, 0);
> > + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
> > + SUN50I_DMIC_EN_CTL_CHAN_MASK,
> > + SUN50I_DMIC_EN_CTL_CHAN(0));
> > + /* Global disable */
> > + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
> > + SUN50I_DMIC_EN_CTL_GLOBE, 0);
> > + }
> > +}
> > +
> > +static int sun50i_dmic_startup(struct snd_pcm_substream *substream,
> > + struct snd_soc_dai *cpu_dai)
> > +{
> > + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > + struct sun50i_dmic_dev *host =
> snd_soc_dai_get_drvdata(rtd->cpu_dai);
>
> This is equivalent to the simpler code you use below:
>
> struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
>
> > +
> > + /* only support capture */
> > + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
> > + return -EINVAL;
> > +
> > + regmap_write(host->regmap, SUN50I_DMIC_INT_STA,
> > + SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING |
> > + SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING);
>
> The interrupt is never enabled, so do we really care about IRQ status?
>
Ok, I will delete it .
>
> > + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> > + SUN50I_DMIC_RXFIFO_CTL_FLUSH,
> > + SUN50I_DMIC_RXFIFO_CTL_FLUSH);
> > + regmap_write(host->regmap, SUN50I_DMIC_CNT, SUN50I_DMIC_CNT_N);
> > +
> > + return 0;
> > +}
> > +
> > +static int sun50i_dmic_hw_params(struct snd_pcm_substream *substream,
> > + struct snd_pcm_hw_params *params,
> > + struct snd_soc_dai *cpu_dai)
> > +{
> > + int i = 0;
> > + unsigned long rate = params_rate(params);
> > + unsigned int channels = params_channels(params);
> > + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
> > + struct platform_device *pdev = host->pdev;
> > + struct dmic_rate dmic_rate_s[] = {
> > + {44100, 0x0},
> > + {48000, 0x0},
> > + {22050, 0x2},
> > + {24000, 0x2},
> > + {11025, 0x4},
> > + {12000, 0x4},
> > + {32000, 0x1},
> > + {16000, 0x3},
> > + {8000, 0x5},
> > + };
> > +
> > + /* DMIC num is N+1 */
> > + regmap_update_bits(host->regmap, SUN50I_DMIC_CH_NUM,
> > + SUN50I_DMIC_CH_NUM_N_MASK,
> > + SUN50I_DMIC_CH_NUM_N(channels - 1));
> > + host->chan_en = (1 << channels) - 1;
> > + regmap_write(host->regmap, SUN50I_DMIC_HPF_CTRL, host->chan_en);
> > +
> > + switch (params_format(params)) {
> > + case SNDRV_PCM_FORMAT_S16_LE:
> > + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> > + SUN50I_DMIC_RXFIFO_CTL_MODE,
> > + SUN50I_DMIC_RXFIFO_CTL_MODE);
> > + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> > + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION, 0);
> > + break;
> > + case SNDRV_PCM_FORMAT_S24_LE:
> > + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> > + SUN50I_DMIC_RXFIFO_CTL_MODE, 0);
>
> In SNDRV_PCM_FORMAT_S24_LE, the sample is in the three least significant
> bytes, so wouldn't we want to still use mode 1 here? To me, mode 0 only
> makes sense when using SNDRV_PCM_FORMAT_S32_LE.
>
I don't understand. The DMIC does not support 32bits, why use
SNDRV_PCM_FORMAT_S32_LE ?
>
> > + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
> > + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION,
> > + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION);
> > + break;
> > + default:
> > + dev_err(&pdev->dev, "Invalid format!\n");
>
> The calls to dev_err() can use cpu_dai->dev. Then you don't need to keep
> a pointer to pdev in the driver data.
>
> > + return -EINVAL;
> > + }
> > +
> > + for (i = 0; i < ARRAY_SIZE(dmic_rate_s); i++) {
> > + if (dmic_rate_s[i].samplerate == rate) {
> > + regmap_update_bits(host->regmap, SUN50I_DMIC_SR,
> > + SUN50I_DMIC_SR_SAMOLE_RATE_MASK,
> > +
> SUN50I_DMIC_SR_SAMOLE_RATE(dmic_rate_s[i].rate_bit));
> > + break;
> > + }
> > + }
> > + if (i == ARRAY_SIZE(dmic_rate_s)) {
> > + dev_err(&pdev->dev, "Invalid rate!\n");
> > + return -EINVAL;
> > + }
> > +
> > + /* oversamplerate adjust */
> > + if (params_rate(params) >= 24000)
> > + regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
> > + SUN50I_DMIC_CTL_OVERSAMPLE_RATE,
> > + SUN50I_DMIC_CTL_OVERSAMPLE_RATE);
> > + else
> > + regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
> > + SUN50I_DMIC_CTL_OVERSAMPLE_RATE, 0);
> > +
> > + return 0;
> > +}
> > +
> > +static int sun50i_dmic_trigger(struct snd_pcm_substream *substream, int
> cmd,
> > + struct snd_soc_dai *dai)
> > +{
> > + int ret = 0;
> > + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> > +
> > + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
> > + return -EINVAL;
> > +
> > + switch (cmd) {
> > + case SNDRV_PCM_TRIGGER_START:
> > + case SNDRV_PCM_TRIGGER_RESUME:
> > + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> > + sun50i_snd_rxctrl_enable(substream, host, true);
> > + break;
> > +
> > + case SNDRV_PCM_TRIGGER_STOP:
> > + case SNDRV_PCM_TRIGGER_SUSPEND:
> > + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> > + sun50i_snd_rxctrl_enable(substream, host, false);
> > + break;
> > +
> > + default:
> > + ret = -EINVAL;
> > + break;
> > + }
> > + return ret;
> > +}
> > +
> > +static int sun50i_dmic_set_sysclk(struct snd_soc_dai *dai, int clk_id,
> > + unsigned int freq, int dir)
> > +{
> > + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> > +
> > + if (clk_set_rate(host->dmic_clk, freq)) {
>
> Currently, mainline does not have any sunxi-specific machine drivers. We
> are using simple-audio-card for sun50i. And asoc_simple_hw_params() only
> calls .set_sysclk if mclk-fs is set to some fixed value, from the device
> tree. However, for sunxi, mclk-fs cannot be a fixed value, because
> sysclk is the same 22/24 MHz regardless of sample rate. So .set_sysclk
> will never be called.
>
> So for now I would suggest calling clk_set_rate() from .hw_params, using
> something like sun8i_codec_get_sysclk_rate() to get the right frequency.
>
Thanks, i will do it
>
> > + dev_err(dai->dev, "Freq : %u not support\n", freq);
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int sun50i_dmic_soc_dai_probe(struct snd_soc_dai *dai)
> > +{
> > + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
> > +
> > + snd_soc_dai_init_dma_data(dai, NULL, &host->dma_params_rx);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct snd_soc_dai_ops sun50i_dmic_dai_ops = {
> > + .startup = sun50i_dmic_startup,
> > + .trigger = sun50i_dmic_trigger,
> > + .hw_params = sun50i_dmic_hw_params,
> > + .set_sysclk = sun50i_dmic_set_sysclk,
> > +};
> > +
> > +static const struct regmap_config sun50i_dmic_regmap_config = {
> > + .reg_bits = 32,
> > + .reg_stride = 4,
> > + .val_bits = 32,
> > + .max_register = SUN50I_DMIC_VERSION,
> > + .cache_type = REGCACHE_NONE,
> > +};
> > +
> > +#define SUN50I_DMIC_RATES (SNDRV_PCM_RATE_8000_48000 |
> SNDRV_PCM_RATE_KNOT)
>
> If you use SNDRV_PCM_RATE_KNOT, you also need to provide a list of
> sample rates as a constraint in .startup.
>
> > +#define SUN50I_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |
> SNDRV_PCM_FMTBIT_S24_LE)
> > +
> > +static struct snd_soc_dai_driver sun50i_dmic_dai = {
> > + .capture = {
> > + .channels_min = 1,
> > + .channels_max = 8,
> > + .rates = SUN50I_DMIC_RATES,
> > + .formats = SUN50I_FORMATS,
> > + },
> > + .probe = sun50i_dmic_soc_dai_probe,
> > + .ops = &sun50i_dmic_dai_ops,
> > + .name = "dmic",
> > +};
> > +
> > +static const struct of_device_id sun50i_dmic_of_match[] = {
> > + {
> > + .compatible = "allwinner,sun50i-h6-dmic",
> > + },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, sun50i_dmic_of_match);
> > +
> > +static const struct snd_soc_component_driver sun50i_dmic_component = {
> > + .name = "sun50i-dmic",
> > +};
> > +
> > +static int sun50i_dmic_runtime_suspend(struct device *dev)
> > +{
> > + struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
> > +
> > + clk_disable_unprepare(host->dmic_clk);
> > + clk_disable_unprepare(host->apb_clk);
> > +
> > + return 0;
> > +}
> > +
> > +static int sun50i_dmic_runtime_resume(struct device *dev)
> > +{
> > + struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
> > + int ret;
> > +
> > + ret = clk_prepare_enable(host->dmic_clk);
> > + if (ret)
> > + return ret;
> > + ret = clk_prepare_enable(host->apb_clk);
> > + if (ret)
> > + clk_disable_unprepare(host->dmic_clk);
> > +
> > + return ret;
> > +}
> > +
> > +static int sun50i_dmic_probe(struct platform_device *pdev)
> > +{
> > + struct sun50i_dmic_dev *host;
> > + struct resource *res;
> > + int ret;
> > + void __iomem *base;
> > +
> > + host = devm_kzalloc(&pdev->dev, sizeof(*host), GFP_KERNEL);
> > + if (!host)
> > + return -ENOMEM;
> > +
> > + host->pdev = pdev;
> > +
> > + /* Get the addresses */
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + base = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(base))
> > + return dev_err_probe(&pdev->dev, PTR_ERR(base),
> > + "get resource failed.\n");
> > +
> > + host->regmap = devm_regmap_init_mmio(&pdev->dev, base,
> > +
> &sun50i_dmic_regmap_config);
> > +
> > + /* Clocks */
> > + host->apb_clk = devm_clk_get(&pdev->dev, "apb");
> > + if (IS_ERR(host->apb_clk))
> > + return dev_err_probe(&pdev->dev, PTR_ERR(host->apb_clk),
> > + "failed to get apb clock.\n");
> > +
> > + host->dmic_clk = devm_clk_get(&pdev->dev, "dmic");
> > + if (IS_ERR(host->dmic_clk))
> > + return dev_err_probe(&pdev->dev, PTR_ERR(host->dmic_clk),
> > + "failed to get dmic clock.\n");
> > +
> > + host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
> > + host->dma_params_rx.maxburst = 8;
> > + host->dma_params_rx.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>
> This needs to be DMA_SLAVE_BUSWIDTH_4_BYTES if you are using larger than
> 16-bit samples. See for example sun4i_i2s_hw_params().
>
> Regards,
> Samuel
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
2021-06-16 6:21 ` 班涛
2021-06-19 19:52 ` Samuel Holland
@ 2021-06-19 19:52 ` Samuel Holland
0 siblings, 0 replies; 36+ messages in thread
From: Samuel Holland @ 2021-06-19 19:52 UTC (permalink / raw)
To: 班涛
Cc: lgirdwood, broonie, perex, tiwai, mripard, wens, jernej.skrabec,
p.zabel, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
On 6/16/21 1:21 AM, 班涛 wrote:
> Hi,
>
> Samuel Holland <samuel@sholland.org> 于2021年6月16日周三 上午11:06写道:
>
>> Hello,
>>
>> On 6/15/21 8:03 AM, Ban Tao wrote:
>>> The Allwinner H6 and later SoCs have an DMIC block
>>> which is capable of capture.
>>>
>>> Signed-off-by: Ban Tao <fengzheng923@gmail.com>
>>> ---
>>> MAINTAINERS | 7 +
>>> sound/soc/sunxi/Kconfig | 8 +
>>> sound/soc/sunxi/Makefile | 1 +
>>> sound/soc/sunxi/sun50i-dmic.c | 408 ++++++++++++++++++++++++++++++++++
>>> 4 files changed, 424 insertions(+)
>>> create mode 100644 sound/soc/sunxi/sun50i-dmic.c
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index b4094f07214e..1b87225c39b0 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -760,6 +760,13 @@ L: linux-media@vger.kernel.org
>>> S: Maintained
>>> F: drivers/staging/media/sunxi/cedrus/
>>>
>>> +ALLWINNER DMIC DRIVERS
>>> +M: Ban Tao <fengzheng923@gmail.com>
>>> +L: alsa-devel@alsa-project.org (moderated for non-subscribers)
>>> +S: Maintained
>>> +F:
>> Documentation/devicetree/bindings/sound/allwinner,sun50i-h6-dmic.yaml
>>> +F: sound/soc/sunxi/sun50i-dmic.c
>>> +
>>> ALPHA PORT
>>> M: Richard Henderson <rth@twiddle.net>
>>> M: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
>>> diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
>>> index ddcaaa98d3cb..2a3bf7722e11 100644
>>> --- a/sound/soc/sunxi/Kconfig
>>> +++ b/sound/soc/sunxi/Kconfig
>>> @@ -56,6 +56,14 @@ config SND_SUN4I_SPDIF
>>> Say Y or M to add support for the S/PDIF audio block in the
>> Allwinner
>>> A10 and affiliated SoCs.
>>>
>>> +config SND_SUN50I_DMIC
>>> + tristate "Allwinner H6 DMIC Support"
>>> + depends on (OF && ARCH_SUNXI) || COMPILE_TEST
>>> + select SND_SOC_GENERIC_DMAENGINE_PCM
>>> + help
>>> + Say Y or M to add support for the DMIC audio block in the
>> Allwinner
>>> + H6 and affiliated SoCs.
>>> +
>>> config SND_SUN8I_ADDA_PR_REGMAP
>>> tristate
>>> select REGMAP
>>> diff --git a/sound/soc/sunxi/Makefile b/sound/soc/sunxi/Makefile
>>> index a86be340a076..4483fe9c94ef 100644
>>> --- a/sound/soc/sunxi/Makefile
>>> +++ b/sound/soc/sunxi/Makefile
>>> @@ -6,3 +6,4 @@ obj-$(CONFIG_SND_SUN8I_CODEC_ANALOG) +=
>> sun8i-codec-analog.o
>>> obj-$(CONFIG_SND_SUN50I_CODEC_ANALOG) += sun50i-codec-analog.o
>>> obj-$(CONFIG_SND_SUN8I_CODEC) += sun8i-codec.o
>>> obj-$(CONFIG_SND_SUN8I_ADDA_PR_REGMAP) += sun8i-adda-pr-regmap.o
>>> +obj-$(CONFIG_SND_SUN50I_DMIC) += sun50i-dmic.o
>>> diff --git a/sound/soc/sunxi/sun50i-dmic.c
>> b/sound/soc/sunxi/sun50i-dmic.c
>>> new file mode 100644
>>> index 000000000000..78d512d93974
>>> --- /dev/null
>>> +++ b/sound/soc/sunxi/sun50i-dmic.c
>>> @@ -0,0 +1,408 @@
>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>> +/*
>>> + * ALSA SoC DMIC Audio Layer
>>> + *
>>> + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
>>> + *
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/device.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/pm_runtime.h>
>>> +#include <linux/reset.h>
>>> +#include <sound/dmaengine_pcm.h>
>>> +#include <sound/pcm_params.h>
>>> +#include <sound/soc.h>
>>> +
>>> +
>>> +#define SUN50I_DMIC_EN_CTL (0x00)
>>> + #define SUN50I_DMIC_EN_CTL_GLOBE BIT(8)
>>> + #define SUN50I_DMIC_EN_CTL_CHAN(v) ((v) << 0)
>>> + #define SUN50I_DMIC_EN_CTL_CHAN_MASK GENMASK(7, 0)
>>> +#define SUN50I_DMIC_SR (0x04)
>>> + #define SUN50I_DMIC_SR_SAMOLE_RATE(v) ((v) << 0)
>>> + #define SUN50I_DMIC_SR_SAMOLE_RATE_MASK GENMASK(2, 0)
>>> +#define SUN50I_DMIC_CTL (0x08)
>>> + #define SUN50I_DMIC_CTL_OVERSAMPLE_RATE BIT(0)
>>> +#define SUN50I_DMIC_DATA (0x10)
>>> +#define SUN50I_DMIC_INTC (0x14)
>>> + #define SUN50I_DMIC_FIFO_DRQ_EN BIT(2)
>>> +#define SUN50I_DMIC_INT_STA (0x18)
>>> + #define SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING BIT(1)
>>> + #define SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING BIT(0)
>>> +#define SUN50I_DMIC_RXFIFO_CTL (0x1c)
>>> + #define SUN50I_DMIC_RXFIFO_CTL_FLUSH BIT(31)
>>> + #define SUN50I_DMIC_RXFIFO_CTL_MODE BIT(9)
>>> + #define SUN50I_DMIC_RXFIFO_CTL_RESOLUTION BIT(8)
>>> +#define SUN50I_DMIC_CH_NUM (0x24)
>>> + #define SUN50I_DMIC_CH_NUM_N(v) ((v) << 0)
>>> + #define SUN50I_DMIC_CH_NUM_N_MASK GENMASK(2, 0)
>>> +#define SUN50I_DMIC_CNT (0x2c)
>>> + #define SUN50I_DMIC_CNT_N BIT(0)
>>> +#define SUN50I_DMIC_HPF_CTRL (0x38)
>>> +#define SUN50I_DMIC_VERSION (0x50)
>>> +
>>> +
>>> +struct sun50i_dmic_dev {
>>> + struct platform_device *pdev;
>>> + struct clk *dmic_clk;
>>> + struct clk *apb_clk;
>>> + struct reset_control *rst;
>>> + struct regmap *regmap;
>>> + struct snd_dmaengine_dai_dma_data dma_params_rx;
>>> + unsigned int chan_en;
>>> +};
>>> +
>>> +struct dmic_rate {
>>> + unsigned int samplerate;
>>> + unsigned int rate_bit;
>>> +};
>>> +
>>> +static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream
>> *substream,
>>> + struct sun50i_dmic_dev *host, bool
>> enable)
>>> +{
>>> + if (enable) {
>>> + /* DRQ ENABLE */
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
>>> + SUN50I_DMIC_FIFO_DRQ_EN,
>>> + SUN50I_DMIC_FIFO_DRQ_EN);
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
>>> + SUN50I_DMIC_EN_CTL_CHAN_MASK,
>>> + SUN50I_DMIC_EN_CTL_CHAN(host->chan_en));
>>> + /* Global enable */
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
>>> + SUN50I_DMIC_EN_CTL_GLOBE,
>>> + SUN50I_DMIC_EN_CTL_GLOBE);
>>> + } else {
>>> + /* DRQ DISABLE */
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
>>> + SUN50I_DMIC_FIFO_DRQ_EN, 0);
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
>>> + SUN50I_DMIC_EN_CTL_CHAN_MASK,
>>> + SUN50I_DMIC_EN_CTL_CHAN(0));
>>> + /* Global disable */
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
>>> + SUN50I_DMIC_EN_CTL_GLOBE, 0);
>>> + }
>>> +}
>>> +
>>> +static int sun50i_dmic_startup(struct snd_pcm_substream *substream,
>>> + struct snd_soc_dai *cpu_dai)
>>> +{
>>> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
>>> + struct sun50i_dmic_dev *host =
>> snd_soc_dai_get_drvdata(rtd->cpu_dai);
>>
>> This is equivalent to the simpler code you use below:
>>
>> struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
>>
>>> +
>>> + /* only support capture */
>>> + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
>>> + return -EINVAL;
>>> +
>>> + regmap_write(host->regmap, SUN50I_DMIC_INT_STA,
>>> + SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING |
>>> + SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING);
>>
>> The interrupt is never enabled, so do we really care about IRQ status?
>>
>
> Ok, I will delete it .
>
>>
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
>>> + SUN50I_DMIC_RXFIFO_CTL_FLUSH,
>>> + SUN50I_DMIC_RXFIFO_CTL_FLUSH);
>>> + regmap_write(host->regmap, SUN50I_DMIC_CNT, SUN50I_DMIC_CNT_N);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sun50i_dmic_hw_params(struct snd_pcm_substream *substream,
>>> + struct snd_pcm_hw_params *params,
>>> + struct snd_soc_dai *cpu_dai)
>>> +{
>>> + int i = 0;
>>> + unsigned long rate = params_rate(params);
>>> + unsigned int channels = params_channels(params);
>>> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
>>> + struct platform_device *pdev = host->pdev;
>>> + struct dmic_rate dmic_rate_s[] = {
>>> + {44100, 0x0},
>>> + {48000, 0x0},
>>> + {22050, 0x2},
>>> + {24000, 0x2},
>>> + {11025, 0x4},
>>> + {12000, 0x4},
>>> + {32000, 0x1},
>>> + {16000, 0x3},
>>> + {8000, 0x5},
>>> + };
>>> +
>>> + /* DMIC num is N+1 */
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_CH_NUM,
>>> + SUN50I_DMIC_CH_NUM_N_MASK,
>>> + SUN50I_DMIC_CH_NUM_N(channels - 1));
>>> + host->chan_en = (1 << channels) - 1;
>>> + regmap_write(host->regmap, SUN50I_DMIC_HPF_CTRL, host->chan_en);
>>> +
>>> + switch (params_format(params)) {
>>> + case SNDRV_PCM_FORMAT_S16_LE:
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
>>> + SUN50I_DMIC_RXFIFO_CTL_MODE,
>>> + SUN50I_DMIC_RXFIFO_CTL_MODE);
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
>>> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION, 0);
>>> + break;
>>> + case SNDRV_PCM_FORMAT_S24_LE:
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
>>> + SUN50I_DMIC_RXFIFO_CTL_MODE, 0);
>>
>> In SNDRV_PCM_FORMAT_S24_LE, the sample is in the three least significant
>> bytes, so wouldn't we want to still use mode 1 here? To me, mode 0 only
>> makes sense when using SNDRV_PCM_FORMAT_S32_LE.
>>
>
> I don't understand. The DMIC does not support 32bits, why use
> SNDRV_PCM_FORMAT_S32_LE ?
The hardware supports extending the samples to 32 bits. So if userspace
wants 32-bit samples (for whatever reason), it is more efficient to do
the extension in hardware than in software. You do not need to add
support for SNDRV_PCM_FORMAT_S32_LE; the driver will work fine without it.
But this is unrelated to the fact that mode 0 is wrong for
SNDRV_PCM_FORMAT_S24_LE.
SNDRV_PCM_FORMAT_S16_LE => samples must be in 2 lower bytes.
Mode 1 does this. The upper 2 bytes are ignored.
SNDRV_PCM_FORMAT_S24_LE => samples must be in 3 lower bytes.
Mode 1 does this. The 21-bit sample is extended with 3 zeroes.
In fact, since only 21 bits in the 24-bit sample are meaningful, you
should set the sig_bits field in the DAI driver so userspace can know.
(Does it make sense that extending 21 -> 24 bits to fit in a standard
PCM format serves the same purpose as extending 21 -> 32 bits?)
>>
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
>>> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION,
>>> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION);
>>> + break;
>>> + default:
>>> + dev_err(&pdev->dev, "Invalid format!\n");
>>
>> The calls to dev_err() can use cpu_dai->dev. Then you don't need to keep
>> a pointer to pdev in the driver data.
>>
>>> + return -EINVAL;
>>> + }
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(dmic_rate_s); i++) {
>>> + if (dmic_rate_s[i].samplerate == rate) {
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_SR,
>>> + SUN50I_DMIC_SR_SAMOLE_RATE_MASK,
>>> +
>> SUN50I_DMIC_SR_SAMOLE_RATE(dmic_rate_s[i].rate_bit));
>>> + break;
>>> + }
>>> + }
>>> + if (i == ARRAY_SIZE(dmic_rate_s)) {
>>> + dev_err(&pdev->dev, "Invalid rate!\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + /* oversamplerate adjust */
>>> + if (params_rate(params) >= 24000)
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
>>> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE,
>>> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE);
>>> + else
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
>>> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE, 0);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sun50i_dmic_trigger(struct snd_pcm_substream *substream, int
>> cmd,
>>> + struct snd_soc_dai *dai)
>>> +{
>>> + int ret = 0;
>>> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
>>> +
>>> + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
>>> + return -EINVAL;
>>> +
>>> + switch (cmd) {
>>> + case SNDRV_PCM_TRIGGER_START:
>>> + case SNDRV_PCM_TRIGGER_RESUME:
>>> + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>>> + sun50i_snd_rxctrl_enable(substream, host, true);
>>> + break;
>>> +
>>> + case SNDRV_PCM_TRIGGER_STOP:
>>> + case SNDRV_PCM_TRIGGER_SUSPEND:
>>> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>>> + sun50i_snd_rxctrl_enable(substream, host, false);
>>> + break;
>>> +
>>> + default:
>>> + ret = -EINVAL;
>>> + break;
>>> + }
>>> + return ret;
>>> +}
>>> +
>>> +static int sun50i_dmic_set_sysclk(struct snd_soc_dai *dai, int clk_id,
>>> + unsigned int freq, int dir)
>>> +{
>>> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
>>> +
>>> + if (clk_set_rate(host->dmic_clk, freq)) {
>>
>> Currently, mainline does not have any sunxi-specific machine drivers. We
>> are using simple-audio-card for sun50i. And asoc_simple_hw_params() only
>> calls .set_sysclk if mclk-fs is set to some fixed value, from the device
>> tree. However, for sunxi, mclk-fs cannot be a fixed value, because
>> sysclk is the same 22/24 MHz regardless of sample rate. So .set_sysclk
>> will never be called.
>>
>> So for now I would suggest calling clk_set_rate() from .hw_params, using
>> something like sun8i_codec_get_sysclk_rate() to get the right frequency.
>>
>
> Thanks, i will do it
>
>>
>>> + dev_err(dai->dev, "Freq : %u not support\n", freq);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sun50i_dmic_soc_dai_probe(struct snd_soc_dai *dai)
>>> +{
>>> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
>>> +
>>> + snd_soc_dai_init_dma_data(dai, NULL, &host->dma_params_rx);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct snd_soc_dai_ops sun50i_dmic_dai_ops = {
>>> + .startup = sun50i_dmic_startup,
>>> + .trigger = sun50i_dmic_trigger,
>>> + .hw_params = sun50i_dmic_hw_params,
>>> + .set_sysclk = sun50i_dmic_set_sysclk,
>>> +};
>>> +
>>> +static const struct regmap_config sun50i_dmic_regmap_config = {
>>> + .reg_bits = 32,
>>> + .reg_stride = 4,
>>> + .val_bits = 32,
>>> + .max_register = SUN50I_DMIC_VERSION,
>>> + .cache_type = REGCACHE_NONE,
>>> +};
>>> +
>>> +#define SUN50I_DMIC_RATES (SNDRV_PCM_RATE_8000_48000 |
>> SNDRV_PCM_RATE_KNOT)
>>
>> If you use SNDRV_PCM_RATE_KNOT, you also need to provide a list of
>> sample rates as a constraint in .startup.
>>
>>> +#define SUN50I_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |
>> SNDRV_PCM_FMTBIT_S24_LE)
>>> +
>>> +static struct snd_soc_dai_driver sun50i_dmic_dai = {
>>> + .capture = {
>>> + .channels_min = 1,
>>> + .channels_max = 8,
>>> + .rates = SUN50I_DMIC_RATES,
>>> + .formats = SUN50I_FORMATS,
".sig_bits = 21," should go here.
Regards,
Samuel
>>> + },
>>> + .probe = sun50i_dmic_soc_dai_probe,
>>> + .ops = &sun50i_dmic_dai_ops,
>>> + .name = "dmic",
>>> +};
>>> +
>>> +static const struct of_device_id sun50i_dmic_of_match[] = {
>>> + {
>>> + .compatible = "allwinner,sun50i-h6-dmic",
>>> + },
>>> + { /* sentinel */ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, sun50i_dmic_of_match);
>>> +
>>> +static const struct snd_soc_component_driver sun50i_dmic_component = {
>>> + .name = "sun50i-dmic",
>>> +};
>>> +
>>> +static int sun50i_dmic_runtime_suspend(struct device *dev)
>>> +{
>>> + struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
>>> +
>>> + clk_disable_unprepare(host->dmic_clk);
>>> + clk_disable_unprepare(host->apb_clk);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sun50i_dmic_runtime_resume(struct device *dev)
>>> +{
>>> + struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
>>> + int ret;
>>> +
>>> + ret = clk_prepare_enable(host->dmic_clk);
>>> + if (ret)
>>> + return ret;
>>> + ret = clk_prepare_enable(host->apb_clk);
>>> + if (ret)
>>> + clk_disable_unprepare(host->dmic_clk);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int sun50i_dmic_probe(struct platform_device *pdev)
>>> +{
>>> + struct sun50i_dmic_dev *host;
>>> + struct resource *res;
>>> + int ret;
>>> + void __iomem *base;
>>> +
>>> + host = devm_kzalloc(&pdev->dev, sizeof(*host), GFP_KERNEL);
>>> + if (!host)
>>> + return -ENOMEM;
>>> +
>>> + host->pdev = pdev;
>>> +
>>> + /* Get the addresses */
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> + base = devm_ioremap_resource(&pdev->dev, res);
>>> + if (IS_ERR(base))
>>> + return dev_err_probe(&pdev->dev, PTR_ERR(base),
>>> + "get resource failed.\n");
>>> +
>>> + host->regmap = devm_regmap_init_mmio(&pdev->dev, base,
>>> +
>> &sun50i_dmic_regmap_config);
>>> +
>>> + /* Clocks */
>>> + host->apb_clk = devm_clk_get(&pdev->dev, "apb");
>>> + if (IS_ERR(host->apb_clk))
>>> + return dev_err_probe(&pdev->dev, PTR_ERR(host->apb_clk),
>>> + "failed to get apb clock.\n");
>>> +
>>> + host->dmic_clk = devm_clk_get(&pdev->dev, "dmic");
>>> + if (IS_ERR(host->dmic_clk))
>>> + return dev_err_probe(&pdev->dev, PTR_ERR(host->dmic_clk),
>>> + "failed to get dmic clock.\n");
>>> +
>>> + host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
>>> + host->dma_params_rx.maxburst = 8;
>>> + host->dma_params_rx.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>>
>> This needs to be DMA_SLAVE_BUSWIDTH_4_BYTES if you are using larger than
>> 16-bit samples. See for example sun4i_i2s_hw_params().
>>
>> Regards,
>> Samuel
>>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-19 19:52 ` Samuel Holland
0 siblings, 0 replies; 36+ messages in thread
From: Samuel Holland @ 2021-06-19 19:52 UTC (permalink / raw)
To: 班涛
Cc: lgirdwood, broonie, perex, tiwai, mripard, wens, jernej.skrabec,
p.zabel, krzk, linux-kernel, alsa-devel, linux-arm-kernel,
linux-sunxi
On 6/16/21 1:21 AM, 班涛 wrote:
> Hi,
>
> Samuel Holland <samuel@sholland.org> 于2021年6月16日周三 上午11:06写道:
>
>> Hello,
>>
>> On 6/15/21 8:03 AM, Ban Tao wrote:
>>> The Allwinner H6 and later SoCs have an DMIC block
>>> which is capable of capture.
>>>
>>> Signed-off-by: Ban Tao <fengzheng923@gmail.com>
>>> ---
>>> MAINTAINERS | 7 +
>>> sound/soc/sunxi/Kconfig | 8 +
>>> sound/soc/sunxi/Makefile | 1 +
>>> sound/soc/sunxi/sun50i-dmic.c | 408 ++++++++++++++++++++++++++++++++++
>>> 4 files changed, 424 insertions(+)
>>> create mode 100644 sound/soc/sunxi/sun50i-dmic.c
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index b4094f07214e..1b87225c39b0 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -760,6 +760,13 @@ L: linux-media@vger.kernel.org
>>> S: Maintained
>>> F: drivers/staging/media/sunxi/cedrus/
>>>
>>> +ALLWINNER DMIC DRIVERS
>>> +M: Ban Tao <fengzheng923@gmail.com>
>>> +L: alsa-devel@alsa-project.org (moderated for non-subscribers)
>>> +S: Maintained
>>> +F:
>> Documentation/devicetree/bindings/sound/allwinner,sun50i-h6-dmic.yaml
>>> +F: sound/soc/sunxi/sun50i-dmic.c
>>> +
>>> ALPHA PORT
>>> M: Richard Henderson <rth@twiddle.net>
>>> M: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
>>> diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
>>> index ddcaaa98d3cb..2a3bf7722e11 100644
>>> --- a/sound/soc/sunxi/Kconfig
>>> +++ b/sound/soc/sunxi/Kconfig
>>> @@ -56,6 +56,14 @@ config SND_SUN4I_SPDIF
>>> Say Y or M to add support for the S/PDIF audio block in the
>> Allwinner
>>> A10 and affiliated SoCs.
>>>
>>> +config SND_SUN50I_DMIC
>>> + tristate "Allwinner H6 DMIC Support"
>>> + depends on (OF && ARCH_SUNXI) || COMPILE_TEST
>>> + select SND_SOC_GENERIC_DMAENGINE_PCM
>>> + help
>>> + Say Y or M to add support for the DMIC audio block in the
>> Allwinner
>>> + H6 and affiliated SoCs.
>>> +
>>> config SND_SUN8I_ADDA_PR_REGMAP
>>> tristate
>>> select REGMAP
>>> diff --git a/sound/soc/sunxi/Makefile b/sound/soc/sunxi/Makefile
>>> index a86be340a076..4483fe9c94ef 100644
>>> --- a/sound/soc/sunxi/Makefile
>>> +++ b/sound/soc/sunxi/Makefile
>>> @@ -6,3 +6,4 @@ obj-$(CONFIG_SND_SUN8I_CODEC_ANALOG) +=
>> sun8i-codec-analog.o
>>> obj-$(CONFIG_SND_SUN50I_CODEC_ANALOG) += sun50i-codec-analog.o
>>> obj-$(CONFIG_SND_SUN8I_CODEC) += sun8i-codec.o
>>> obj-$(CONFIG_SND_SUN8I_ADDA_PR_REGMAP) += sun8i-adda-pr-regmap.o
>>> +obj-$(CONFIG_SND_SUN50I_DMIC) += sun50i-dmic.o
>>> diff --git a/sound/soc/sunxi/sun50i-dmic.c
>> b/sound/soc/sunxi/sun50i-dmic.c
>>> new file mode 100644
>>> index 000000000000..78d512d93974
>>> --- /dev/null
>>> +++ b/sound/soc/sunxi/sun50i-dmic.c
>>> @@ -0,0 +1,408 @@
>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>> +/*
>>> + * ALSA SoC DMIC Audio Layer
>>> + *
>>> + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
>>> + *
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/device.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/pm_runtime.h>
>>> +#include <linux/reset.h>
>>> +#include <sound/dmaengine_pcm.h>
>>> +#include <sound/pcm_params.h>
>>> +#include <sound/soc.h>
>>> +
>>> +
>>> +#define SUN50I_DMIC_EN_CTL (0x00)
>>> + #define SUN50I_DMIC_EN_CTL_GLOBE BIT(8)
>>> + #define SUN50I_DMIC_EN_CTL_CHAN(v) ((v) << 0)
>>> + #define SUN50I_DMIC_EN_CTL_CHAN_MASK GENMASK(7, 0)
>>> +#define SUN50I_DMIC_SR (0x04)
>>> + #define SUN50I_DMIC_SR_SAMOLE_RATE(v) ((v) << 0)
>>> + #define SUN50I_DMIC_SR_SAMOLE_RATE_MASK GENMASK(2, 0)
>>> +#define SUN50I_DMIC_CTL (0x08)
>>> + #define SUN50I_DMIC_CTL_OVERSAMPLE_RATE BIT(0)
>>> +#define SUN50I_DMIC_DATA (0x10)
>>> +#define SUN50I_DMIC_INTC (0x14)
>>> + #define SUN50I_DMIC_FIFO_DRQ_EN BIT(2)
>>> +#define SUN50I_DMIC_INT_STA (0x18)
>>> + #define SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING BIT(1)
>>> + #define SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING BIT(0)
>>> +#define SUN50I_DMIC_RXFIFO_CTL (0x1c)
>>> + #define SUN50I_DMIC_RXFIFO_CTL_FLUSH BIT(31)
>>> + #define SUN50I_DMIC_RXFIFO_CTL_MODE BIT(9)
>>> + #define SUN50I_DMIC_RXFIFO_CTL_RESOLUTION BIT(8)
>>> +#define SUN50I_DMIC_CH_NUM (0x24)
>>> + #define SUN50I_DMIC_CH_NUM_N(v) ((v) << 0)
>>> + #define SUN50I_DMIC_CH_NUM_N_MASK GENMASK(2, 0)
>>> +#define SUN50I_DMIC_CNT (0x2c)
>>> + #define SUN50I_DMIC_CNT_N BIT(0)
>>> +#define SUN50I_DMIC_HPF_CTRL (0x38)
>>> +#define SUN50I_DMIC_VERSION (0x50)
>>> +
>>> +
>>> +struct sun50i_dmic_dev {
>>> + struct platform_device *pdev;
>>> + struct clk *dmic_clk;
>>> + struct clk *apb_clk;
>>> + struct reset_control *rst;
>>> + struct regmap *regmap;
>>> + struct snd_dmaengine_dai_dma_data dma_params_rx;
>>> + unsigned int chan_en;
>>> +};
>>> +
>>> +struct dmic_rate {
>>> + unsigned int samplerate;
>>> + unsigned int rate_bit;
>>> +};
>>> +
>>> +static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream
>> *substream,
>>> + struct sun50i_dmic_dev *host, bool
>> enable)
>>> +{
>>> + if (enable) {
>>> + /* DRQ ENABLE */
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
>>> + SUN50I_DMIC_FIFO_DRQ_EN,
>>> + SUN50I_DMIC_FIFO_DRQ_EN);
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
>>> + SUN50I_DMIC_EN_CTL_CHAN_MASK,
>>> + SUN50I_DMIC_EN_CTL_CHAN(host->chan_en));
>>> + /* Global enable */
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
>>> + SUN50I_DMIC_EN_CTL_GLOBE,
>>> + SUN50I_DMIC_EN_CTL_GLOBE);
>>> + } else {
>>> + /* DRQ DISABLE */
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
>>> + SUN50I_DMIC_FIFO_DRQ_EN, 0);
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
>>> + SUN50I_DMIC_EN_CTL_CHAN_MASK,
>>> + SUN50I_DMIC_EN_CTL_CHAN(0));
>>> + /* Global disable */
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
>>> + SUN50I_DMIC_EN_CTL_GLOBE, 0);
>>> + }
>>> +}
>>> +
>>> +static int sun50i_dmic_startup(struct snd_pcm_substream *substream,
>>> + struct snd_soc_dai *cpu_dai)
>>> +{
>>> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
>>> + struct sun50i_dmic_dev *host =
>> snd_soc_dai_get_drvdata(rtd->cpu_dai);
>>
>> This is equivalent to the simpler code you use below:
>>
>> struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
>>
>>> +
>>> + /* only support capture */
>>> + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
>>> + return -EINVAL;
>>> +
>>> + regmap_write(host->regmap, SUN50I_DMIC_INT_STA,
>>> + SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING |
>>> + SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING);
>>
>> The interrupt is never enabled, so do we really care about IRQ status?
>>
>
> Ok, I will delete it .
>
>>
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
>>> + SUN50I_DMIC_RXFIFO_CTL_FLUSH,
>>> + SUN50I_DMIC_RXFIFO_CTL_FLUSH);
>>> + regmap_write(host->regmap, SUN50I_DMIC_CNT, SUN50I_DMIC_CNT_N);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sun50i_dmic_hw_params(struct snd_pcm_substream *substream,
>>> + struct snd_pcm_hw_params *params,
>>> + struct snd_soc_dai *cpu_dai)
>>> +{
>>> + int i = 0;
>>> + unsigned long rate = params_rate(params);
>>> + unsigned int channels = params_channels(params);
>>> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
>>> + struct platform_device *pdev = host->pdev;
>>> + struct dmic_rate dmic_rate_s[] = {
>>> + {44100, 0x0},
>>> + {48000, 0x0},
>>> + {22050, 0x2},
>>> + {24000, 0x2},
>>> + {11025, 0x4},
>>> + {12000, 0x4},
>>> + {32000, 0x1},
>>> + {16000, 0x3},
>>> + {8000, 0x5},
>>> + };
>>> +
>>> + /* DMIC num is N+1 */
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_CH_NUM,
>>> + SUN50I_DMIC_CH_NUM_N_MASK,
>>> + SUN50I_DMIC_CH_NUM_N(channels - 1));
>>> + host->chan_en = (1 << channels) - 1;
>>> + regmap_write(host->regmap, SUN50I_DMIC_HPF_CTRL, host->chan_en);
>>> +
>>> + switch (params_format(params)) {
>>> + case SNDRV_PCM_FORMAT_S16_LE:
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
>>> + SUN50I_DMIC_RXFIFO_CTL_MODE,
>>> + SUN50I_DMIC_RXFIFO_CTL_MODE);
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
>>> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION, 0);
>>> + break;
>>> + case SNDRV_PCM_FORMAT_S24_LE:
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
>>> + SUN50I_DMIC_RXFIFO_CTL_MODE, 0);
>>
>> In SNDRV_PCM_FORMAT_S24_LE, the sample is in the three least significant
>> bytes, so wouldn't we want to still use mode 1 here? To me, mode 0 only
>> makes sense when using SNDRV_PCM_FORMAT_S32_LE.
>>
>
> I don't understand. The DMIC does not support 32bits, why use
> SNDRV_PCM_FORMAT_S32_LE ?
The hardware supports extending the samples to 32 bits. So if userspace
wants 32-bit samples (for whatever reason), it is more efficient to do
the extension in hardware than in software. You do not need to add
support for SNDRV_PCM_FORMAT_S32_LE; the driver will work fine without it.
But this is unrelated to the fact that mode 0 is wrong for
SNDRV_PCM_FORMAT_S24_LE.
SNDRV_PCM_FORMAT_S16_LE => samples must be in 2 lower bytes.
Mode 1 does this. The upper 2 bytes are ignored.
SNDRV_PCM_FORMAT_S24_LE => samples must be in 3 lower bytes.
Mode 1 does this. The 21-bit sample is extended with 3 zeroes.
In fact, since only 21 bits in the 24-bit sample are meaningful, you
should set the sig_bits field in the DAI driver so userspace can know.
(Does it make sense that extending 21 -> 24 bits to fit in a standard
PCM format serves the same purpose as extending 21 -> 32 bits?)
>>
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
>>> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION,
>>> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION);
>>> + break;
>>> + default:
>>> + dev_err(&pdev->dev, "Invalid format!\n");
>>
>> The calls to dev_err() can use cpu_dai->dev. Then you don't need to keep
>> a pointer to pdev in the driver data.
>>
>>> + return -EINVAL;
>>> + }
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(dmic_rate_s); i++) {
>>> + if (dmic_rate_s[i].samplerate == rate) {
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_SR,
>>> + SUN50I_DMIC_SR_SAMOLE_RATE_MASK,
>>> +
>> SUN50I_DMIC_SR_SAMOLE_RATE(dmic_rate_s[i].rate_bit));
>>> + break;
>>> + }
>>> + }
>>> + if (i == ARRAY_SIZE(dmic_rate_s)) {
>>> + dev_err(&pdev->dev, "Invalid rate!\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + /* oversamplerate adjust */
>>> + if (params_rate(params) >= 24000)
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
>>> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE,
>>> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE);
>>> + else
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
>>> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE, 0);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sun50i_dmic_trigger(struct snd_pcm_substream *substream, int
>> cmd,
>>> + struct snd_soc_dai *dai)
>>> +{
>>> + int ret = 0;
>>> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
>>> +
>>> + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
>>> + return -EINVAL;
>>> +
>>> + switch (cmd) {
>>> + case SNDRV_PCM_TRIGGER_START:
>>> + case SNDRV_PCM_TRIGGER_RESUME:
>>> + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>>> + sun50i_snd_rxctrl_enable(substream, host, true);
>>> + break;
>>> +
>>> + case SNDRV_PCM_TRIGGER_STOP:
>>> + case SNDRV_PCM_TRIGGER_SUSPEND:
>>> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>>> + sun50i_snd_rxctrl_enable(substream, host, false);
>>> + break;
>>> +
>>> + default:
>>> + ret = -EINVAL;
>>> + break;
>>> + }
>>> + return ret;
>>> +}
>>> +
>>> +static int sun50i_dmic_set_sysclk(struct snd_soc_dai *dai, int clk_id,
>>> + unsigned int freq, int dir)
>>> +{
>>> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
>>> +
>>> + if (clk_set_rate(host->dmic_clk, freq)) {
>>
>> Currently, mainline does not have any sunxi-specific machine drivers. We
>> are using simple-audio-card for sun50i. And asoc_simple_hw_params() only
>> calls .set_sysclk if mclk-fs is set to some fixed value, from the device
>> tree. However, for sunxi, mclk-fs cannot be a fixed value, because
>> sysclk is the same 22/24 MHz regardless of sample rate. So .set_sysclk
>> will never be called.
>>
>> So for now I would suggest calling clk_set_rate() from .hw_params, using
>> something like sun8i_codec_get_sysclk_rate() to get the right frequency.
>>
>
> Thanks, i will do it
>
>>
>>> + dev_err(dai->dev, "Freq : %u not support\n", freq);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sun50i_dmic_soc_dai_probe(struct snd_soc_dai *dai)
>>> +{
>>> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
>>> +
>>> + snd_soc_dai_init_dma_data(dai, NULL, &host->dma_params_rx);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct snd_soc_dai_ops sun50i_dmic_dai_ops = {
>>> + .startup = sun50i_dmic_startup,
>>> + .trigger = sun50i_dmic_trigger,
>>> + .hw_params = sun50i_dmic_hw_params,
>>> + .set_sysclk = sun50i_dmic_set_sysclk,
>>> +};
>>> +
>>> +static const struct regmap_config sun50i_dmic_regmap_config = {
>>> + .reg_bits = 32,
>>> + .reg_stride = 4,
>>> + .val_bits = 32,
>>> + .max_register = SUN50I_DMIC_VERSION,
>>> + .cache_type = REGCACHE_NONE,
>>> +};
>>> +
>>> +#define SUN50I_DMIC_RATES (SNDRV_PCM_RATE_8000_48000 |
>> SNDRV_PCM_RATE_KNOT)
>>
>> If you use SNDRV_PCM_RATE_KNOT, you also need to provide a list of
>> sample rates as a constraint in .startup.
>>
>>> +#define SUN50I_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |
>> SNDRV_PCM_FMTBIT_S24_LE)
>>> +
>>> +static struct snd_soc_dai_driver sun50i_dmic_dai = {
>>> + .capture = {
>>> + .channels_min = 1,
>>> + .channels_max = 8,
>>> + .rates = SUN50I_DMIC_RATES,
>>> + .formats = SUN50I_FORMATS,
".sig_bits = 21," should go here.
Regards,
Samuel
>>> + },
>>> + .probe = sun50i_dmic_soc_dai_probe,
>>> + .ops = &sun50i_dmic_dai_ops,
>>> + .name = "dmic",
>>> +};
>>> +
>>> +static const struct of_device_id sun50i_dmic_of_match[] = {
>>> + {
>>> + .compatible = "allwinner,sun50i-h6-dmic",
>>> + },
>>> + { /* sentinel */ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, sun50i_dmic_of_match);
>>> +
>>> +static const struct snd_soc_component_driver sun50i_dmic_component = {
>>> + .name = "sun50i-dmic",
>>> +};
>>> +
>>> +static int sun50i_dmic_runtime_suspend(struct device *dev)
>>> +{
>>> + struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
>>> +
>>> + clk_disable_unprepare(host->dmic_clk);
>>> + clk_disable_unprepare(host->apb_clk);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sun50i_dmic_runtime_resume(struct device *dev)
>>> +{
>>> + struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
>>> + int ret;
>>> +
>>> + ret = clk_prepare_enable(host->dmic_clk);
>>> + if (ret)
>>> + return ret;
>>> + ret = clk_prepare_enable(host->apb_clk);
>>> + if (ret)
>>> + clk_disable_unprepare(host->dmic_clk);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int sun50i_dmic_probe(struct platform_device *pdev)
>>> +{
>>> + struct sun50i_dmic_dev *host;
>>> + struct resource *res;
>>> + int ret;
>>> + void __iomem *base;
>>> +
>>> + host = devm_kzalloc(&pdev->dev, sizeof(*host), GFP_KERNEL);
>>> + if (!host)
>>> + return -ENOMEM;
>>> +
>>> + host->pdev = pdev;
>>> +
>>> + /* Get the addresses */
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> + base = devm_ioremap_resource(&pdev->dev, res);
>>> + if (IS_ERR(base))
>>> + return dev_err_probe(&pdev->dev, PTR_ERR(base),
>>> + "get resource failed.\n");
>>> +
>>> + host->regmap = devm_regmap_init_mmio(&pdev->dev, base,
>>> +
>> &sun50i_dmic_regmap_config);
>>> +
>>> + /* Clocks */
>>> + host->apb_clk = devm_clk_get(&pdev->dev, "apb");
>>> + if (IS_ERR(host->apb_clk))
>>> + return dev_err_probe(&pdev->dev, PTR_ERR(host->apb_clk),
>>> + "failed to get apb clock.\n");
>>> +
>>> + host->dmic_clk = devm_clk_get(&pdev->dev, "dmic");
>>> + if (IS_ERR(host->dmic_clk))
>>> + return dev_err_probe(&pdev->dev, PTR_ERR(host->dmic_clk),
>>> + "failed to get dmic clock.\n");
>>> +
>>> + host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
>>> + host->dma_params_rx.maxburst = 8;
>>> + host->dma_params_rx.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>>
>> This needs to be DMA_SLAVE_BUSWIDTH_4_BYTES if you are using larger than
>> 16-bit samples. See for example sun4i_i2s_hw_params().
>>
>> Regards,
>> Samuel
>>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
@ 2021-06-19 19:52 ` Samuel Holland
0 siblings, 0 replies; 36+ messages in thread
From: Samuel Holland @ 2021-06-19 19:52 UTC (permalink / raw)
To: 班涛
Cc: alsa-devel, krzk, linux-kernel, tiwai, mripard, lgirdwood, wens,
broonie, jernej.skrabec, p.zabel, linux-sunxi, linux-arm-kernel
On 6/16/21 1:21 AM, 班涛 wrote:
> Hi,
>
> Samuel Holland <samuel@sholland.org> 于2021年6月16日周三 上午11:06写道:
>
>> Hello,
>>
>> On 6/15/21 8:03 AM, Ban Tao wrote:
>>> The Allwinner H6 and later SoCs have an DMIC block
>>> which is capable of capture.
>>>
>>> Signed-off-by: Ban Tao <fengzheng923@gmail.com>
>>> ---
>>> MAINTAINERS | 7 +
>>> sound/soc/sunxi/Kconfig | 8 +
>>> sound/soc/sunxi/Makefile | 1 +
>>> sound/soc/sunxi/sun50i-dmic.c | 408 ++++++++++++++++++++++++++++++++++
>>> 4 files changed, 424 insertions(+)
>>> create mode 100644 sound/soc/sunxi/sun50i-dmic.c
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index b4094f07214e..1b87225c39b0 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -760,6 +760,13 @@ L: linux-media@vger.kernel.org
>>> S: Maintained
>>> F: drivers/staging/media/sunxi/cedrus/
>>>
>>> +ALLWINNER DMIC DRIVERS
>>> +M: Ban Tao <fengzheng923@gmail.com>
>>> +L: alsa-devel@alsa-project.org (moderated for non-subscribers)
>>> +S: Maintained
>>> +F:
>> Documentation/devicetree/bindings/sound/allwinner,sun50i-h6-dmic.yaml
>>> +F: sound/soc/sunxi/sun50i-dmic.c
>>> +
>>> ALPHA PORT
>>> M: Richard Henderson <rth@twiddle.net>
>>> M: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
>>> diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
>>> index ddcaaa98d3cb..2a3bf7722e11 100644
>>> --- a/sound/soc/sunxi/Kconfig
>>> +++ b/sound/soc/sunxi/Kconfig
>>> @@ -56,6 +56,14 @@ config SND_SUN4I_SPDIF
>>> Say Y or M to add support for the S/PDIF audio block in the
>> Allwinner
>>> A10 and affiliated SoCs.
>>>
>>> +config SND_SUN50I_DMIC
>>> + tristate "Allwinner H6 DMIC Support"
>>> + depends on (OF && ARCH_SUNXI) || COMPILE_TEST
>>> + select SND_SOC_GENERIC_DMAENGINE_PCM
>>> + help
>>> + Say Y or M to add support for the DMIC audio block in the
>> Allwinner
>>> + H6 and affiliated SoCs.
>>> +
>>> config SND_SUN8I_ADDA_PR_REGMAP
>>> tristate
>>> select REGMAP
>>> diff --git a/sound/soc/sunxi/Makefile b/sound/soc/sunxi/Makefile
>>> index a86be340a076..4483fe9c94ef 100644
>>> --- a/sound/soc/sunxi/Makefile
>>> +++ b/sound/soc/sunxi/Makefile
>>> @@ -6,3 +6,4 @@ obj-$(CONFIG_SND_SUN8I_CODEC_ANALOG) +=
>> sun8i-codec-analog.o
>>> obj-$(CONFIG_SND_SUN50I_CODEC_ANALOG) += sun50i-codec-analog.o
>>> obj-$(CONFIG_SND_SUN8I_CODEC) += sun8i-codec.o
>>> obj-$(CONFIG_SND_SUN8I_ADDA_PR_REGMAP) += sun8i-adda-pr-regmap.o
>>> +obj-$(CONFIG_SND_SUN50I_DMIC) += sun50i-dmic.o
>>> diff --git a/sound/soc/sunxi/sun50i-dmic.c
>> b/sound/soc/sunxi/sun50i-dmic.c
>>> new file mode 100644
>>> index 000000000000..78d512d93974
>>> --- /dev/null
>>> +++ b/sound/soc/sunxi/sun50i-dmic.c
>>> @@ -0,0 +1,408 @@
>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>> +/*
>>> + * ALSA SoC DMIC Audio Layer
>>> + *
>>> + * Copyright 2021 Ban Tao <fengzheng923@gmail.com>
>>> + *
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/device.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/pm_runtime.h>
>>> +#include <linux/reset.h>
>>> +#include <sound/dmaengine_pcm.h>
>>> +#include <sound/pcm_params.h>
>>> +#include <sound/soc.h>
>>> +
>>> +
>>> +#define SUN50I_DMIC_EN_CTL (0x00)
>>> + #define SUN50I_DMIC_EN_CTL_GLOBE BIT(8)
>>> + #define SUN50I_DMIC_EN_CTL_CHAN(v) ((v) << 0)
>>> + #define SUN50I_DMIC_EN_CTL_CHAN_MASK GENMASK(7, 0)
>>> +#define SUN50I_DMIC_SR (0x04)
>>> + #define SUN50I_DMIC_SR_SAMOLE_RATE(v) ((v) << 0)
>>> + #define SUN50I_DMIC_SR_SAMOLE_RATE_MASK GENMASK(2, 0)
>>> +#define SUN50I_DMIC_CTL (0x08)
>>> + #define SUN50I_DMIC_CTL_OVERSAMPLE_RATE BIT(0)
>>> +#define SUN50I_DMIC_DATA (0x10)
>>> +#define SUN50I_DMIC_INTC (0x14)
>>> + #define SUN50I_DMIC_FIFO_DRQ_EN BIT(2)
>>> +#define SUN50I_DMIC_INT_STA (0x18)
>>> + #define SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING BIT(1)
>>> + #define SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING BIT(0)
>>> +#define SUN50I_DMIC_RXFIFO_CTL (0x1c)
>>> + #define SUN50I_DMIC_RXFIFO_CTL_FLUSH BIT(31)
>>> + #define SUN50I_DMIC_RXFIFO_CTL_MODE BIT(9)
>>> + #define SUN50I_DMIC_RXFIFO_CTL_RESOLUTION BIT(8)
>>> +#define SUN50I_DMIC_CH_NUM (0x24)
>>> + #define SUN50I_DMIC_CH_NUM_N(v) ((v) << 0)
>>> + #define SUN50I_DMIC_CH_NUM_N_MASK GENMASK(2, 0)
>>> +#define SUN50I_DMIC_CNT (0x2c)
>>> + #define SUN50I_DMIC_CNT_N BIT(0)
>>> +#define SUN50I_DMIC_HPF_CTRL (0x38)
>>> +#define SUN50I_DMIC_VERSION (0x50)
>>> +
>>> +
>>> +struct sun50i_dmic_dev {
>>> + struct platform_device *pdev;
>>> + struct clk *dmic_clk;
>>> + struct clk *apb_clk;
>>> + struct reset_control *rst;
>>> + struct regmap *regmap;
>>> + struct snd_dmaengine_dai_dma_data dma_params_rx;
>>> + unsigned int chan_en;
>>> +};
>>> +
>>> +struct dmic_rate {
>>> + unsigned int samplerate;
>>> + unsigned int rate_bit;
>>> +};
>>> +
>>> +static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream
>> *substream,
>>> + struct sun50i_dmic_dev *host, bool
>> enable)
>>> +{
>>> + if (enable) {
>>> + /* DRQ ENABLE */
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
>>> + SUN50I_DMIC_FIFO_DRQ_EN,
>>> + SUN50I_DMIC_FIFO_DRQ_EN);
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
>>> + SUN50I_DMIC_EN_CTL_CHAN_MASK,
>>> + SUN50I_DMIC_EN_CTL_CHAN(host->chan_en));
>>> + /* Global enable */
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
>>> + SUN50I_DMIC_EN_CTL_GLOBE,
>>> + SUN50I_DMIC_EN_CTL_GLOBE);
>>> + } else {
>>> + /* DRQ DISABLE */
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_INTC,
>>> + SUN50I_DMIC_FIFO_DRQ_EN, 0);
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
>>> + SUN50I_DMIC_EN_CTL_CHAN_MASK,
>>> + SUN50I_DMIC_EN_CTL_CHAN(0));
>>> + /* Global disable */
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_EN_CTL,
>>> + SUN50I_DMIC_EN_CTL_GLOBE, 0);
>>> + }
>>> +}
>>> +
>>> +static int sun50i_dmic_startup(struct snd_pcm_substream *substream,
>>> + struct snd_soc_dai *cpu_dai)
>>> +{
>>> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
>>> + struct sun50i_dmic_dev *host =
>> snd_soc_dai_get_drvdata(rtd->cpu_dai);
>>
>> This is equivalent to the simpler code you use below:
>>
>> struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
>>
>>> +
>>> + /* only support capture */
>>> + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
>>> + return -EINVAL;
>>> +
>>> + regmap_write(host->regmap, SUN50I_DMIC_INT_STA,
>>> + SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING |
>>> + SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING);
>>
>> The interrupt is never enabled, so do we really care about IRQ status?
>>
>
> Ok, I will delete it .
>
>>
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
>>> + SUN50I_DMIC_RXFIFO_CTL_FLUSH,
>>> + SUN50I_DMIC_RXFIFO_CTL_FLUSH);
>>> + regmap_write(host->regmap, SUN50I_DMIC_CNT, SUN50I_DMIC_CNT_N);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sun50i_dmic_hw_params(struct snd_pcm_substream *substream,
>>> + struct snd_pcm_hw_params *params,
>>> + struct snd_soc_dai *cpu_dai)
>>> +{
>>> + int i = 0;
>>> + unsigned long rate = params_rate(params);
>>> + unsigned int channels = params_channels(params);
>>> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
>>> + struct platform_device *pdev = host->pdev;
>>> + struct dmic_rate dmic_rate_s[] = {
>>> + {44100, 0x0},
>>> + {48000, 0x0},
>>> + {22050, 0x2},
>>> + {24000, 0x2},
>>> + {11025, 0x4},
>>> + {12000, 0x4},
>>> + {32000, 0x1},
>>> + {16000, 0x3},
>>> + {8000, 0x5},
>>> + };
>>> +
>>> + /* DMIC num is N+1 */
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_CH_NUM,
>>> + SUN50I_DMIC_CH_NUM_N_MASK,
>>> + SUN50I_DMIC_CH_NUM_N(channels - 1));
>>> + host->chan_en = (1 << channels) - 1;
>>> + regmap_write(host->regmap, SUN50I_DMIC_HPF_CTRL, host->chan_en);
>>> +
>>> + switch (params_format(params)) {
>>> + case SNDRV_PCM_FORMAT_S16_LE:
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
>>> + SUN50I_DMIC_RXFIFO_CTL_MODE,
>>> + SUN50I_DMIC_RXFIFO_CTL_MODE);
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
>>> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION, 0);
>>> + break;
>>> + case SNDRV_PCM_FORMAT_S24_LE:
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
>>> + SUN50I_DMIC_RXFIFO_CTL_MODE, 0);
>>
>> In SNDRV_PCM_FORMAT_S24_LE, the sample is in the three least significant
>> bytes, so wouldn't we want to still use mode 1 here? To me, mode 0 only
>> makes sense when using SNDRV_PCM_FORMAT_S32_LE.
>>
>
> I don't understand. The DMIC does not support 32bits, why use
> SNDRV_PCM_FORMAT_S32_LE ?
The hardware supports extending the samples to 32 bits. So if userspace
wants 32-bit samples (for whatever reason), it is more efficient to do
the extension in hardware than in software. You do not need to add
support for SNDRV_PCM_FORMAT_S32_LE; the driver will work fine without it.
But this is unrelated to the fact that mode 0 is wrong for
SNDRV_PCM_FORMAT_S24_LE.
SNDRV_PCM_FORMAT_S16_LE => samples must be in 2 lower bytes.
Mode 1 does this. The upper 2 bytes are ignored.
SNDRV_PCM_FORMAT_S24_LE => samples must be in 3 lower bytes.
Mode 1 does this. The 21-bit sample is extended with 3 zeroes.
In fact, since only 21 bits in the 24-bit sample are meaningful, you
should set the sig_bits field in the DAI driver so userspace can know.
(Does it make sense that extending 21 -> 24 bits to fit in a standard
PCM format serves the same purpose as extending 21 -> 32 bits?)
>>
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
>>> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION,
>>> + SUN50I_DMIC_RXFIFO_CTL_RESOLUTION);
>>> + break;
>>> + default:
>>> + dev_err(&pdev->dev, "Invalid format!\n");
>>
>> The calls to dev_err() can use cpu_dai->dev. Then you don't need to keep
>> a pointer to pdev in the driver data.
>>
>>> + return -EINVAL;
>>> + }
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(dmic_rate_s); i++) {
>>> + if (dmic_rate_s[i].samplerate == rate) {
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_SR,
>>> + SUN50I_DMIC_SR_SAMOLE_RATE_MASK,
>>> +
>> SUN50I_DMIC_SR_SAMOLE_RATE(dmic_rate_s[i].rate_bit));
>>> + break;
>>> + }
>>> + }
>>> + if (i == ARRAY_SIZE(dmic_rate_s)) {
>>> + dev_err(&pdev->dev, "Invalid rate!\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + /* oversamplerate adjust */
>>> + if (params_rate(params) >= 24000)
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
>>> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE,
>>> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE);
>>> + else
>>> + regmap_update_bits(host->regmap, SUN50I_DMIC_CTL,
>>> + SUN50I_DMIC_CTL_OVERSAMPLE_RATE, 0);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sun50i_dmic_trigger(struct snd_pcm_substream *substream, int
>> cmd,
>>> + struct snd_soc_dai *dai)
>>> +{
>>> + int ret = 0;
>>> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
>>> +
>>> + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
>>> + return -EINVAL;
>>> +
>>> + switch (cmd) {
>>> + case SNDRV_PCM_TRIGGER_START:
>>> + case SNDRV_PCM_TRIGGER_RESUME:
>>> + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>>> + sun50i_snd_rxctrl_enable(substream, host, true);
>>> + break;
>>> +
>>> + case SNDRV_PCM_TRIGGER_STOP:
>>> + case SNDRV_PCM_TRIGGER_SUSPEND:
>>> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>>> + sun50i_snd_rxctrl_enable(substream, host, false);
>>> + break;
>>> +
>>> + default:
>>> + ret = -EINVAL;
>>> + break;
>>> + }
>>> + return ret;
>>> +}
>>> +
>>> +static int sun50i_dmic_set_sysclk(struct snd_soc_dai *dai, int clk_id,
>>> + unsigned int freq, int dir)
>>> +{
>>> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
>>> +
>>> + if (clk_set_rate(host->dmic_clk, freq)) {
>>
>> Currently, mainline does not have any sunxi-specific machine drivers. We
>> are using simple-audio-card for sun50i. And asoc_simple_hw_params() only
>> calls .set_sysclk if mclk-fs is set to some fixed value, from the device
>> tree. However, for sunxi, mclk-fs cannot be a fixed value, because
>> sysclk is the same 22/24 MHz regardless of sample rate. So .set_sysclk
>> will never be called.
>>
>> So for now I would suggest calling clk_set_rate() from .hw_params, using
>> something like sun8i_codec_get_sysclk_rate() to get the right frequency.
>>
>
> Thanks, i will do it
>
>>
>>> + dev_err(dai->dev, "Freq : %u not support\n", freq);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sun50i_dmic_soc_dai_probe(struct snd_soc_dai *dai)
>>> +{
>>> + struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(dai);
>>> +
>>> + snd_soc_dai_init_dma_data(dai, NULL, &host->dma_params_rx);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct snd_soc_dai_ops sun50i_dmic_dai_ops = {
>>> + .startup = sun50i_dmic_startup,
>>> + .trigger = sun50i_dmic_trigger,
>>> + .hw_params = sun50i_dmic_hw_params,
>>> + .set_sysclk = sun50i_dmic_set_sysclk,
>>> +};
>>> +
>>> +static const struct regmap_config sun50i_dmic_regmap_config = {
>>> + .reg_bits = 32,
>>> + .reg_stride = 4,
>>> + .val_bits = 32,
>>> + .max_register = SUN50I_DMIC_VERSION,
>>> + .cache_type = REGCACHE_NONE,
>>> +};
>>> +
>>> +#define SUN50I_DMIC_RATES (SNDRV_PCM_RATE_8000_48000 |
>> SNDRV_PCM_RATE_KNOT)
>>
>> If you use SNDRV_PCM_RATE_KNOT, you also need to provide a list of
>> sample rates as a constraint in .startup.
>>
>>> +#define SUN50I_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |
>> SNDRV_PCM_FMTBIT_S24_LE)
>>> +
>>> +static struct snd_soc_dai_driver sun50i_dmic_dai = {
>>> + .capture = {
>>> + .channels_min = 1,
>>> + .channels_max = 8,
>>> + .rates = SUN50I_DMIC_RATES,
>>> + .formats = SUN50I_FORMATS,
".sig_bits = 21," should go here.
Regards,
Samuel
>>> + },
>>> + .probe = sun50i_dmic_soc_dai_probe,
>>> + .ops = &sun50i_dmic_dai_ops,
>>> + .name = "dmic",
>>> +};
>>> +
>>> +static const struct of_device_id sun50i_dmic_of_match[] = {
>>> + {
>>> + .compatible = "allwinner,sun50i-h6-dmic",
>>> + },
>>> + { /* sentinel */ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, sun50i_dmic_of_match);
>>> +
>>> +static const struct snd_soc_component_driver sun50i_dmic_component = {
>>> + .name = "sun50i-dmic",
>>> +};
>>> +
>>> +static int sun50i_dmic_runtime_suspend(struct device *dev)
>>> +{
>>> + struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
>>> +
>>> + clk_disable_unprepare(host->dmic_clk);
>>> + clk_disable_unprepare(host->apb_clk);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sun50i_dmic_runtime_resume(struct device *dev)
>>> +{
>>> + struct sun50i_dmic_dev *host = dev_get_drvdata(dev);
>>> + int ret;
>>> +
>>> + ret = clk_prepare_enable(host->dmic_clk);
>>> + if (ret)
>>> + return ret;
>>> + ret = clk_prepare_enable(host->apb_clk);
>>> + if (ret)
>>> + clk_disable_unprepare(host->dmic_clk);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int sun50i_dmic_probe(struct platform_device *pdev)
>>> +{
>>> + struct sun50i_dmic_dev *host;
>>> + struct resource *res;
>>> + int ret;
>>> + void __iomem *base;
>>> +
>>> + host = devm_kzalloc(&pdev->dev, sizeof(*host), GFP_KERNEL);
>>> + if (!host)
>>> + return -ENOMEM;
>>> +
>>> + host->pdev = pdev;
>>> +
>>> + /* Get the addresses */
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> + base = devm_ioremap_resource(&pdev->dev, res);
>>> + if (IS_ERR(base))
>>> + return dev_err_probe(&pdev->dev, PTR_ERR(base),
>>> + "get resource failed.\n");
>>> +
>>> + host->regmap = devm_regmap_init_mmio(&pdev->dev, base,
>>> +
>> &sun50i_dmic_regmap_config);
>>> +
>>> + /* Clocks */
>>> + host->apb_clk = devm_clk_get(&pdev->dev, "apb");
>>> + if (IS_ERR(host->apb_clk))
>>> + return dev_err_probe(&pdev->dev, PTR_ERR(host->apb_clk),
>>> + "failed to get apb clock.\n");
>>> +
>>> + host->dmic_clk = devm_clk_get(&pdev->dev, "dmic");
>>> + if (IS_ERR(host->dmic_clk))
>>> + return dev_err_probe(&pdev->dev, PTR_ERR(host->dmic_clk),
>>> + "failed to get dmic clock.\n");
>>> +
>>> + host->dma_params_rx.addr = res->start + SUN50I_DMIC_DATA;
>>> + host->dma_params_rx.maxburst = 8;
>>> + host->dma_params_rx.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>>
>> This needs to be DMA_SLAVE_BUSWIDTH_4_BYTES if you are using larger than
>> 16-bit samples. See for example sun4i_i2s_hw_params().
>>
>> Regards,
>> Samuel
>>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver
2021-06-15 13:03 ` Ban Tao
` (4 preceding siblings ...)
(?)
@ 2021-06-17 0:10 ` kernel test robot
-1 siblings, 0 replies; 36+ messages in thread
From: kernel test robot @ 2021-06-17 0:10 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2867 bytes --]
Hi Ban,
I love your patch! Yet something to improve:
[auto build test ERROR on sunxi/sunxi/for-next]
[also build test ERROR on asoc/for-next pza/reset/next linus/master v5.13-rc6 next-20210616]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Ban-Tao/ASoC-sunxi-Add-Allwinner-H6-Digital-MIC-driver/20210616-140744
base: https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git sunxi/for-next
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/49b9fed77fea75987ffd3230684bff94e1f22138
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ban-Tao/ASoC-sunxi-Add-Allwinner-H6-Digital-MIC-driver/20210616-140744
git checkout 49b9fed77fea75987ffd3230684bff94e1f22138
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
sound/soc/sunxi/sun50i-dmic.c: In function 'sun50i_dmic_startup':
>> sound/soc/sunxi/sun50i-dmic.c:96:60: error: 'struct snd_soc_pcm_runtime' has no member named 'cpu_dai'
96 | struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
| ^~
vim +96 sound/soc/sunxi/sun50i-dmic.c
91
92 static int sun50i_dmic_startup(struct snd_pcm_substream *substream,
93 struct snd_soc_dai *cpu_dai)
94 {
95 struct snd_soc_pcm_runtime *rtd = substream->private_data;
> 96 struct sun50i_dmic_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
97
98 /* only support capture */
99 if (substream->stream != SNDRV_PCM_STREAM_CAPTURE)
100 return -EINVAL;
101
102 regmap_write(host->regmap, SUN50I_DMIC_INT_STA,
103 SUN50I_DMIC_INT_STA_OVERRUN_IRQ_PENDING |
104 SUN50I_DMIC_INT_STA_DATA_IRQ_PENDING);
105 regmap_update_bits(host->regmap, SUN50I_DMIC_RXFIFO_CTL,
106 SUN50I_DMIC_RXFIFO_CTL_FLUSH,
107 SUN50I_DMIC_RXFIFO_CTL_FLUSH);
108 regmap_write(host->regmap, SUN50I_DMIC_CNT, SUN50I_DMIC_CNT_N);
109
110 return 0;
111 }
112
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 68087 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread