From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1166241AbeBOSSG (ORCPT ); Thu, 15 Feb 2018 13:18:06 -0500 Received: from mail-it0-f65.google.com ([209.85.214.65]:54866 "EHLO mail-it0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1163568AbeBOSSC (ORCPT ); Thu, 15 Feb 2018 13:18:02 -0500 X-Google-Smtp-Source: AH8x227COh2aNrv64yy+ZdoEsA1nnJsRJCGBnYJefSlLASyAGha9wZV1DRD8MiFwx0E65jnMbc+ZrA== Date: Thu, 15 Feb 2018 10:18:00 -0800 From: Matthias Kaehlcke To: Peter Ujfalusi Cc: Liam Girdwood , Mark Brown , Rob Herring , Mark Rutland , Jaroslav Kysela , Takashi Iwai , devicetree@vger.kernel.org, alsa-devel@alsa-project.org, huang lin , Brian Norris , linux-kernel@vger.kernel.org, Dylan Reid Subject: Re: [alsa-devel] [PATCH] ASoC: dmic: Add optional wakeup delay Message-ID: <20180215181800.GB99727@google.com> References: <20180214235156.81589-1-mka@chromium.org> <7c9d5202-bcac-f989-6be2-f286de15a5de@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <7c9d5202-bcac-f989-6be2-f286de15a5de@ti.com> User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org El Thu, Feb 15, 2018 at 09:42:01AM +0200 Peter Ujfalusi ha dit: > > > On 2018-02-15 01:51, Matthias Kaehlcke wrote: > > On some systems a delay is needed after switching on the clocks, to allow > > the DMIC output to stabilize and avoid a popping noise at the beginning > > of the recording. Add the optional device tree property 'wakeup-delay-ms' > > and apply the specified delay in the new dmic_daiops_prepare(). > > > > Signed-off-by: Matthias Kaehlcke > > --- > > .../devicetree/bindings/sound/dmic.txt | 2 + > > sound/soc/codecs/dmic.c | 54 ++++++++++++++----- > > 2 files changed, 42 insertions(+), 14 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/sound/dmic.txt b/Documentation/devicetree/bindings/sound/dmic.txt > > index 54c8ef6498a8..de741c6609d0 100644 > > --- a/Documentation/devicetree/bindings/sound/dmic.txt > > +++ b/Documentation/devicetree/bindings/sound/dmic.txt > > @@ -7,10 +7,12 @@ Required properties: > > > > Optional properties: > > - dmicen-gpios: GPIO specifier for dmic to control start and stop > > + - wakeup-delay-ms: Delay (in ms) after enabling the DMIC > > > > Example node: > > > > dmic_codec: dmic@0 { > > compatible = "dmic-codec"; > > dmicen-gpios = <&gpio4 3 GPIO_ACTIVE_HIGH>; > > + wakeup-delay-ms <50>; > > }; > > diff --git a/sound/soc/codecs/dmic.c b/sound/soc/codecs/dmic.c > > index b88a1ee66f80..11f6abf11074 100644 > > --- a/sound/soc/codecs/dmic.c > > +++ b/sound/soc/codecs/dmic.c > > @@ -19,6 +19,7 @@ > > * > > */ > > > > +#include > > #include > > #include > > #include > > @@ -29,24 +30,38 @@ > > #include > > #include > > > > +struct dmic { > > + struct gpio_desc *gpio_en; > > + int wakeup_delay; > > +}; > > + > > +static int dmic_daiops_prepare(struct snd_pcm_substream *substream, > > + struct snd_soc_dai *dai) > > +{ > > + struct dmic *dmic = snd_soc_dai_get_drvdata(dai); > > + > > + if (dmic->gpio_en) > > + gpiod_set_value(dmic->gpio_en, 1); > > You don't need the 'if (dmic->gpio_en)' True, personally I think the if statement makes it clearer that the GPIO is optional. I'm fine with removing it if the general sense is that it's just noise. > > + > > + if (dmic->wakeup_delay) > > + msleep(dmic->wakeup_delay); > > + > > + return 0; > > +} > > + > > 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); > > + struct dmic *dmic = snd_soc_dai_get_drvdata(dai); > > > > - if (!dmic_en) > > + if (!dmic->gpio_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); > > + gpiod_set_value(dmic->gpio_en, 0); > > break; > > } > > What if instead of this trickery we try to handle the gpio/delay via > DAPM? SND_SOC_DAPM_AIF_OUT_E() might be just what we need? > > We could remove the trigger, and will have no need for the prepare > callback either.. SND_SOC_DAPM_AIF_OUT_E() looks promising, thanks for the pointer! > > @@ -54,6 +69,7 @@ static int dmic_daiops_trigger(struct snd_pcm_substream *substream, > > } > > > > static const struct snd_soc_dai_ops dmic_dai_ops = { > > + .prepare = dmic_daiops_prepare, > > .trigger = dmic_daiops_trigger, > > }; > > > > @@ -73,14 +89,24 @@ static struct snd_soc_dai_driver dmic_dai = { > > > > static int dmic_codec_probe(struct snd_soc_codec *codec) > > { > > - struct gpio_desc *dmic_en; > > + struct dmic *dmic; > > + int err; > > + > > + dmic = devm_kzalloc(codec->dev, sizeof(*dmic), GFP_KERNEL); > > + if (!dmic) > > + return -ENOMEM; > > + > > + dmic->gpio_en = devm_gpiod_get_optional(codec->dev, > > + "dmicen", GPIOD_OUT_LOW); > > + if (IS_ERR(dmic->gpio_en)) > > + return PTR_ERR(dmic->gpio_en); > > > > - dmic_en = devm_gpiod_get_optional(codec->dev, > > - "dmicen", GPIOD_OUT_LOW); > > - if (IS_ERR(dmic_en)) > > - return PTR_ERR(dmic_en); > > + err = device_property_read_u32(codec->dev, "wakeup-delay-ms", > > + &dmic->wakeup_delay); > > + if (err && (err != -EINVAL)) > > + return err; > > > > - snd_soc_codec_set_drvdata(codec, dmic_en); > > + snd_soc_codec_set_drvdata(codec, dmic); > > > > return 0; > > } > > > > - Péter > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matthias Kaehlcke Subject: Re: [PATCH] ASoC: dmic: Add optional wakeup delay Date: Thu, 15 Feb 2018 10:18:00 -0800 Message-ID: <20180215181800.GB99727@google.com> References: <20180214235156.81589-1-mka@chromium.org> <7c9d5202-bcac-f989-6be2-f286de15a5de@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <7c9d5202-bcac-f989-6be2-f286de15a5de@ti.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Peter Ujfalusi Cc: Mark Rutland , devicetree@vger.kernel.org, alsa-devel@alsa-project.org, huang lin , linux-kernel@vger.kernel.org, Brian Norris , Takashi Iwai , Rob Herring , Liam Girdwood , Mark Brown , Dylan Reid List-Id: devicetree@vger.kernel.org RWwgVGh1LCBGZWIgMTUsIDIwMTggYXQgMDk6NDI6MDFBTSArMDIwMCBQZXRlciBVamZhbHVzaSBo YSBkaXQ6Cgo+IAo+IAo+IE9uIDIwMTgtMDItMTUgMDE6NTEsIE1hdHRoaWFzIEthZWhsY2tlIHdy b3RlOgo+ID4gT24gc29tZSBzeXN0ZW1zIGEgZGVsYXkgaXMgbmVlZGVkIGFmdGVyIHN3aXRjaGlu ZyBvbiB0aGUgY2xvY2tzLCB0byBhbGxvdwo+ID4gdGhlIERNSUMgb3V0cHV0IHRvIHN0YWJpbGl6 ZSBhbmQgYXZvaWQgYSBwb3BwaW5nIG5vaXNlIGF0IHRoZSBiZWdpbm5pbmcKPiA+IG9mIHRoZSBy ZWNvcmRpbmcuIEFkZCB0aGUgb3B0aW9uYWwgZGV2aWNlIHRyZWUgcHJvcGVydHkgJ3dha2V1cC1k ZWxheS1tcycKPiA+IGFuZCBhcHBseSB0aGUgc3BlY2lmaWVkIGRlbGF5IGluIHRoZSBuZXcgZG1p Y19kYWlvcHNfcHJlcGFyZSgpLgo+ID4gCj4gPiBTaWduZWQtb2ZmLWJ5OiBNYXR0aGlhcyBLYWVo bGNrZSA8bWthQGNocm9taXVtLm9yZz4KPiA+IC0tLQo+ID4gIC4uLi9kZXZpY2V0cmVlL2JpbmRp bmdzL3NvdW5kL2RtaWMudHh0ICAgICAgICB8ICAyICsKPiA+ICBzb3VuZC9zb2MvY29kZWNzL2Rt aWMuYyAgICAgICAgICAgICAgICAgICAgICAgfCA1NCArKysrKysrKysrKysrKy0tLS0tCj4gPiAg MiBmaWxlcyBjaGFuZ2VkLCA0MiBpbnNlcnRpb25zKCspLCAxNCBkZWxldGlvbnMoLSkKPiA+IAo+ ID4gZGlmZiAtLWdpdCBhL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9zb3VuZC9k bWljLnR4dCBiL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9zb3VuZC9kbWljLnR4 dAo+ID4gaW5kZXggNTRjOGVmNjQ5OGE4Li5kZTc0MWM2NjA5ZDAgMTAwNjQ0Cj4gPiAtLS0gYS9E b2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3Mvc291bmQvZG1pYy50eHQKPiA+ICsrKyBi L0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9zb3VuZC9kbWljLnR4dAo+ID4gQEAg LTcsMTAgKzcsMTIgQEAgUmVxdWlyZWQgcHJvcGVydGllczoKPiA+ICAKPiA+ICBPcHRpb25hbCBw cm9wZXJ0aWVzOgo+ID4gIAktIGRtaWNlbi1ncGlvczogR1BJTyBzcGVjaWZpZXIgZm9yIGRtaWMg dG8gY29udHJvbCBzdGFydCBhbmQgc3RvcAo+ID4gKwktIHdha2V1cC1kZWxheS1tczogRGVsYXkg KGluIG1zKSBhZnRlciBlbmFibGluZyB0aGUgRE1JQwo+ID4gIAo+ID4gIEV4YW1wbGUgbm9kZToK PiA+ICAKPiA+ICAJZG1pY19jb2RlYzogZG1pY0AwIHsKPiA+ICAJCWNvbXBhdGlibGUgPSAiZG1p Yy1jb2RlYyI7Cj4gPiAgCQlkbWljZW4tZ3Bpb3MgPSA8JmdwaW80IDMgR1BJT19BQ1RJVkVfSElH SD47Cj4gPiArCQl3YWtldXAtZGVsYXktbXMgPDUwPjsKPiA+ICAJfTsKPiA+IGRpZmYgLS1naXQg YS9zb3VuZC9zb2MvY29kZWNzL2RtaWMuYyBiL3NvdW5kL3NvYy9jb2RlY3MvZG1pYy5jCj4gPiBp bmRleCBiODhhMWVlNjZmODAuLjExZjZhYmYxMTA3NCAxMDA2NDQKPiA+IC0tLSBhL3NvdW5kL3Nv Yy9jb2RlY3MvZG1pYy5jCj4gPiArKysgYi9zb3VuZC9zb2MvY29kZWNzL2RtaWMuYwo+ID4gQEAg LTE5LDYgKzE5LDcgQEAKPiA+ICAgKgo+ID4gICAqLwo+ID4gIAo+ID4gKyNpbmNsdWRlIDxsaW51 eC9kZWxheS5oPgo+ID4gICNpbmNsdWRlIDxsaW51eC9ncGlvLmg+Cj4gPiAgI2luY2x1ZGUgPGxp bnV4L2dwaW8vY29uc3VtZXIuaD4KPiA+ICAjaW5jbHVkZSA8bGludXgvcGxhdGZvcm1fZGV2aWNl Lmg+Cj4gPiBAQCAtMjksMjQgKzMwLDM4IEBACj4gPiAgI2luY2x1ZGUgPHNvdW5kL3NvYy5oPgo+ ID4gICNpbmNsdWRlIDxzb3VuZC9zb2MtZGFwbS5oPgo+ID4gIAo+ID4gK3N0cnVjdCBkbWljIHsK PiA+ICsJc3RydWN0IGdwaW9fZGVzYyAqZ3Bpb19lbjsKPiA+ICsJaW50IHdha2V1cF9kZWxheTsK PiA+ICt9Owo+ID4gKwo+ID4gK3N0YXRpYyBpbnQgZG1pY19kYWlvcHNfcHJlcGFyZShzdHJ1Y3Qg c25kX3BjbV9zdWJzdHJlYW0gKnN1YnN0cmVhbSwKPiA+ICsJCQkJc3RydWN0IHNuZF9zb2NfZGFp ICpkYWkpCj4gPiArewo+ID4gKwlzdHJ1Y3QgZG1pYyAqZG1pYyA9IHNuZF9zb2NfZGFpX2dldF9k cnZkYXRhKGRhaSk7Cj4gPiArCj4gPiArCWlmIChkbWljLT5ncGlvX2VuKQo+ID4gKwkJZ3Bpb2Rf c2V0X3ZhbHVlKGRtaWMtPmdwaW9fZW4sIDEpOwo+IAo+IFlvdSBkb24ndCBuZWVkIHRoZSAnaWYg KGRtaWMtPmdwaW9fZW4pJwoKVHJ1ZSwgcGVyc29uYWxseSBJIHRoaW5rIHRoZSBpZiBzdGF0ZW1l bnQgbWFrZXMgaXQgY2xlYXJlciB0aGF0IHRoZQpHUElPIGlzIG9wdGlvbmFsLiBJJ20gZmluZSB3 aXRoIHJlbW92aW5nIGl0IGlmIHRoZSBnZW5lcmFsIHNlbnNlIGlzCnRoYXQgaXQncyBqdXN0IG5v aXNlLgoKPiA+ICsKPiA+ICsJaWYgKGRtaWMtPndha2V1cF9kZWxheSkKPiA+ICsJCW1zbGVlcChk bWljLT53YWtldXBfZGVsYXkpOwo+ID4gKwo+ID4gKwlyZXR1cm4gMDsKPiA+ICt9Cj4gPiArCj4g PiAgc3RhdGljIGludCBkbWljX2RhaW9wc190cmlnZ2VyKHN0cnVjdCBzbmRfcGNtX3N1YnN0cmVh bSAqc3Vic3RyZWFtLAo+ID4gIAkJaW50IGNtZCwgc3RydWN0IHNuZF9zb2NfZGFpICpkYWkpCj4g PiAgewo+ID4gLQlzdHJ1Y3QgZ3Bpb19kZXNjICpkbWljX2VuID0gc25kX3NvY19kYWlfZ2V0X2Ry dmRhdGEoZGFpKTsKPiA+ICsJc3RydWN0IGRtaWMgKmRtaWMgPSBzbmRfc29jX2RhaV9nZXRfZHJ2 ZGF0YShkYWkpOwo+ID4gIAo+ID4gLQlpZiAoIWRtaWNfZW4pCj4gPiArCWlmICghZG1pYy0+Z3Bp b19lbikKPiA+ICAJCXJldHVybiAwOwo+ID4gIAo+ID4gIAlzd2l0Y2ggKGNtZCkgewo+ID4gLQlj YXNlIFNORFJWX1BDTV9UUklHR0VSX1NUQVJUOgo+ID4gLQljYXNlIFNORFJWX1BDTV9UUklHR0VS X1JFU1VNRToKPiA+IC0JY2FzZSBTTkRSVl9QQ01fVFJJR0dFUl9QQVVTRV9SRUxFQVNFOgo+ID4g LQkJZ3Bpb2Rfc2V0X3ZhbHVlKGRtaWNfZW4sIDEpOwo+ID4gLQkJYnJlYWs7Cj4gPiAgCWNhc2Ug U05EUlZfUENNX1RSSUdHRVJfU1RPUDoKPiA+ICAJY2FzZSBTTkRSVl9QQ01fVFJJR0dFUl9TVVNQ RU5EOgo+ID4gIAljYXNlIFNORFJWX1BDTV9UUklHR0VSX1BBVVNFX1BVU0g6Cj4gPiAtCQlncGlv ZF9zZXRfdmFsdWUoZG1pY19lbiwgMCk7Cj4gPiArCQlncGlvZF9zZXRfdmFsdWUoZG1pYy0+Z3Bp b19lbiwgMCk7Cj4gPiAgCQlicmVhazsKPiA+ICAJfQo+IAo+IFdoYXQgaWYgaW5zdGVhZCBvZiB0 aGlzIHRyaWNrZXJ5IHdlIHRyeSB0byBoYW5kbGUgdGhlIGdwaW8vZGVsYXkgdmlhCj4gREFQTT8g U05EX1NPQ19EQVBNX0FJRl9PVVRfRSgpIG1pZ2h0IGJlIGp1c3Qgd2hhdCB3ZSBuZWVkPwo+IAo+ IFdlIGNvdWxkIHJlbW92ZSB0aGUgdHJpZ2dlciwgYW5kIHdpbGwgaGF2ZSBubyBuZWVkIGZvciB0 aGUgcHJlcGFyZQo+IGNhbGxiYWNrIGVpdGhlci4uCgpTTkRfU09DX0RBUE1fQUlGX09VVF9FKCkg bG9va3MgcHJvbWlzaW5nLCB0aGFua3MgZm9yIHRoZSBwb2ludGVyIQoKPiA+IEBAIC01NCw2ICs2 OSw3IEBAIHN0YXRpYyBpbnQgZG1pY19kYWlvcHNfdHJpZ2dlcihzdHJ1Y3Qgc25kX3BjbV9zdWJz dHJlYW0gKnN1YnN0cmVhbSwKPiA+ICB9Cj4gPiAgCj4gPiAgc3RhdGljIGNvbnN0IHN0cnVjdCBz bmRfc29jX2RhaV9vcHMgZG1pY19kYWlfb3BzID0gewo+ID4gKwkucHJlcGFyZQk9IGRtaWNfZGFp b3BzX3ByZXBhcmUsCj4gPiAgCS50cmlnZ2VyCT0gZG1pY19kYWlvcHNfdHJpZ2dlciwKPiA+ICB9 Owo+ID4gIAo+ID4gQEAgLTczLDE0ICs4OSwyNCBAQCBzdGF0aWMgc3RydWN0IHNuZF9zb2NfZGFp X2RyaXZlciBkbWljX2RhaSA9IHsKPiA+ICAKPiA+ICBzdGF0aWMgaW50IGRtaWNfY29kZWNfcHJv YmUoc3RydWN0IHNuZF9zb2NfY29kZWMgKmNvZGVjKQo+ID4gIHsKPiA+IC0Jc3RydWN0IGdwaW9f ZGVzYyAqZG1pY19lbjsKPiA+ICsJc3RydWN0IGRtaWMgKmRtaWM7Cj4gPiArCWludCBlcnI7Cj4g PiArCj4gPiArCWRtaWMgPSBkZXZtX2t6YWxsb2MoY29kZWMtPmRldiwgc2l6ZW9mKCpkbWljKSwg R0ZQX0tFUk5FTCk7Cj4gPiArCWlmICghZG1pYykKPiA+ICsJCXJldHVybiAtRU5PTUVNOwo+ID4g Kwo+ID4gKwlkbWljLT5ncGlvX2VuID0gZGV2bV9ncGlvZF9nZXRfb3B0aW9uYWwoY29kZWMtPmRl diwKPiA+ICsJCQkJCQkiZG1pY2VuIiwgR1BJT0RfT1VUX0xPVyk7Cj4gPiArCWlmIChJU19FUlIo ZG1pYy0+Z3Bpb19lbikpCj4gPiArCQlyZXR1cm4gUFRSX0VSUihkbWljLT5ncGlvX2VuKTsKPiA+ ICAKPiA+IC0JZG1pY19lbiA9IGRldm1fZ3Bpb2RfZ2V0X29wdGlvbmFsKGNvZGVjLT5kZXYsCj4g PiAtCQkJCQkiZG1pY2VuIiwgR1BJT0RfT1VUX0xPVyk7Cj4gPiAtCWlmIChJU19FUlIoZG1pY19l bikpCj4gPiAtCQlyZXR1cm4gUFRSX0VSUihkbWljX2VuKTsKPiA+ICsJZXJyID0gZGV2aWNlX3By b3BlcnR5X3JlYWRfdTMyKGNvZGVjLT5kZXYsICJ3YWtldXAtZGVsYXktbXMiLAo+ID4gKwkJCQkg ICAgICAgJmRtaWMtPndha2V1cF9kZWxheSk7Cj4gPiArCWlmIChlcnIgJiYgKGVyciAhPSAtRUlO VkFMKSkKPiA+ICsJCXJldHVybiBlcnI7Cj4gPiAgCj4gPiAtCXNuZF9zb2NfY29kZWNfc2V0X2Ry dmRhdGEoY29kZWMsIGRtaWNfZW4pOwo+ID4gKwlzbmRfc29jX2NvZGVjX3NldF9kcnZkYXRhKGNv ZGVjLCBkbWljKTsKPiA+ICAKPiA+ICAJcmV0dXJuIDA7Cj4gPiAgfQo+ID4gCj4gCj4gLSBQw6l0 ZXIKPiAKPiBUZXhhcyBJbnN0cnVtZW50cyBGaW5sYW5kIE95LCBQb3Jra2FsYW5rYXR1IDIyLCAw MDE4MCBIZWxzaW5raS4KPiBZLXR1bm51cy9CdXNpbmVzcyBJRDogMDYxNTUyMS00LiBLb3RpcGFp a2thL0RvbWljaWxlOiBIZWxzaW5raQpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fXwpBbHNhLWRldmVsIG1haWxpbmcgbGlzdApBbHNhLWRldmVsQGFsc2EtcHJv amVjdC5vcmcKaHR0cDovL21haWxtYW4uYWxzYS1wcm9qZWN0Lm9yZy9tYWlsbWFuL2xpc3RpbmZv L2Fsc2EtZGV2ZWwK