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 > +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 > + * 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()