Re: [PATCH 1/2] ASoC: sunxi: Add Allwinner H6 Digital MIC driver

Hi,

Mark Brown [off-list ref] 于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:

quoted

+M:   Ban Tao [off-list ref]
+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?

quoted

@@ -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 [off-list ref]
 *
 */
is this OK?

quoted

+static void sun50i_snd_rxctrl_enable(struct snd_pcm_substream *substream,
+                                 struct sun50i_dmic_dev *host, bool enable)
+{
+     if (enable) {

quoted

+     } else {

quoted

+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.

quoted

+     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