From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753495AbdIDKDt (ORCPT ); Mon, 4 Sep 2017 06:03:49 -0400 Received: from mx08-00178001.pphosted.com ([91.207.212.93]:38549 "EHLO mx07-00178001.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753422AbdIDKDr (ORCPT ); Mon, 4 Sep 2017 06:03:47 -0400 Subject: Re: [PATCH v2 1/2] ASoC: codec: use enable pin to control dmic start and stop To: Lin Huang , "broonie@kernel.org" , "dgreif@chromium.org" CC: "alsa-devel@alsa-project.org" , "lgirdwood@gmail.com" , "briannorris@chromium.org" , "linux-kernel@vger.kernel.org" , "tiwai@suse.com" , "mka@chromium.org" , "dianders@chromium.org" References: <1502936685-24414-1-git-send-email-hl@rock-chips.com> From: Arnaud Pouliquen Message-ID: <63c1d120-9c50-93ef-9ea0-be882ac8de1c@st.com> Date: Mon, 4 Sep 2017 12:03:21 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <1502936685-24414-1-git-send-email-hl@rock-chips.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [10.75.127.46] X-ClientProxiedBy: SFHDAG1NODE1.st.com (10.75.127.1) To SFHDAG3NODE1.st.com (10.75.127.7) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-09-04_05:,, signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello Lin, Sorry for this late answer. I'm not maintainer, just a contributor... but as some update seems strange for me, so i prefer to highlight it to clarify them. On 08/17/2017 04:24 AM, Lin Huang wrote: > From: huang lin > > on some board use enable pin to control dmic start and stop, > so add this feature in dmic driver. Please, Could you give data-sheet reference of your DMIC, to help me to understand your use-case? > > Signed-off-by: Lin Huang > --- > sound/soc/codecs/Kconfig | 2 +- > sound/soc/codecs/dmic.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 47 insertions(+), 1 deletion(-) > > diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig > index 010811e..d98233b 100644 > --- a/sound/soc/codecs/Kconfig > +++ b/sound/soc/codecs/Kconfig > @@ -71,7 +71,7 @@ config SND_SOC_ALL_CODECS > select SND_SOC_DA732X if I2C > select SND_SOC_DA9055 if I2C > select SND_SOC_DIO2125 > - select SND_SOC_DMIC > + select SND_SOC_DMIC if GPIOLIB Dependency also for DMIC without GPIO to handle gating? > select SND_SOC_ES8316 if I2C > select SND_SOC_ES8328_SPI if SPI_MASTER > select SND_SOC_ES8328_I2C if I2C > diff --git a/sound/soc/codecs/dmic.c b/sound/soc/codecs/dmic.c > index 12e07f9..b88a1ee 100644 > --- a/sound/soc/codecs/dmic.c > +++ b/sound/soc/codecs/dmic.c > @@ -19,6 +19,8 @@ > * > */ > > +#include > +#include > #include > #include > #include > @@ -27,6 +29,34 @@ > #include > #include > > +static int dmic_daiops_trigger(struct snd_pcm_substream *substream, > + int cmd, struct snd_soc_dai *dai) > +{ > + struct gpio_desc *dmic_en = snd_soc_dai_get_drvdata(dai); > + > + if (!dmic_en) > + return 0; > + > + switch (cmd) { > + case SNDRV_PCM_TRIGGER_START: > + case SNDRV_PCM_TRIGGER_RESUME: > + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: > + gpiod_set_value(dmic_en, 1); > + break; > + case SNDRV_PCM_TRIGGER_STOP: > + case SNDRV_PCM_TRIGGER_SUSPEND: > + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: > + gpiod_set_value(dmic_en, 0); > + break; > + } > + > + return 0; > +} > + > +static const struct snd_soc_dai_ops dmic_dai_ops = { > + .trigger = dmic_daiops_trigger, > +}; > + should it be handle by trigger or DAPM? > static struct snd_soc_dai_driver dmic_dai = { > .name = "dmic-hifi", > .capture = { > @@ -38,8 +68,23 @@ static struct snd_soc_dai_driver dmic_dai = { > | SNDRV_PCM_FMTBIT_S24_LE > | SNDRV_PCM_FMTBIT_S16_LE, > }, > + .ops = &dmic_dai_ops, > }; > > +static int dmic_codec_probe(struct snd_soc_codec *codec) > +{ > + struct gpio_desc *dmic_en; > + > + dmic_en = devm_gpiod_get_optional(codec->dev, > + "dmicen", GPIOD_OUT_LOW); Hypothesis here is that GPIO is always set to low? seems too limiting. Regards Arnaud > + if (IS_ERR(dmic_en)) > + return PTR_ERR(dmic_en); > + > + snd_soc_codec_set_drvdata(codec, dmic_en); > + > + return 0; > +} > + > static const struct snd_soc_dapm_widget dmic_dapm_widgets[] = { > SND_SOC_DAPM_AIF_OUT("DMIC AIF", "Capture", 0, > SND_SOC_NOPM, 0, 0), > @@ -51,6 +96,7 @@ static const struct snd_soc_dapm_route intercon[] = { > }; > > static const struct snd_soc_codec_driver soc_dmic = { > + .probe = dmic_codec_probe, > .component_driver = { > .dapm_widgets = dmic_dapm_widgets, > .num_dapm_widgets = ARRAY_SIZE(dmic_dapm_widgets), >