El Thu, Feb 15, 2018 at 06:47:29PM -0800 Brian Norris ha dit: > Hi, > > On Thu, Feb 15, 2018 at 06:24:16PM -0800, Matthias Kaehlcke wrote: > > On some systems a delay is needed after switching on the clocks, to allow > > the 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 after enabling the mic. A blocking delay > > can't be applied in dmic_daiops_trigger() since the function is called > > in atomic context. Instead use a DAPM event handler to set the enable > > GPIO and apply the delay in the handler. > > > > Signed-off-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx> > > --- > > Changes in v2: > > - use DAPM event handler instead of _prepare() and get rid of > > _trigger() > > - skip error check for optional 'wakeup-delay-ms' property > > - updated commit message > > Looks good to me in general: > > Brian Norris <briannorris@xxxxxxxxxxxx> > > But I don't know much about the audio subsystem. One comment below. > > > > > .../devicetree/bindings/sound/dmic.txt | 2 + > > sound/soc/codecs/dmic.c | 63 ++++++++++--------- > > 2 files changed, 37 insertions(+), 28 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/sound/dmic.txt b/Documentation/devicetree/bindings/sound/dmic.txt > > index f7bf65611453..e957b4136716 100644 > > --- a/Documentation/devicetree/bindings/sound/dmic.txt > > +++ b/Documentation/devicetree/bindings/sound/dmic.txt > > @@ -8,6 +8,7 @@ Required properties: > > Optional properties: > > - dmicen-gpios: GPIO specifier for dmic to control start and stop > > - num-channels: Number of microphones on this DAI > > + - wakeup-delay-ms: Delay (in ms) after enabling the DMIC > > > > Example node: > > > > @@ -15,4 +16,5 @@ Example node: > > compatible = "dmic-codec"; > > dmicen-gpios = <&gpio4 3 GPIO_ACTIVE_HIGH>; > > num-channels = <1>; > > + wakeup-delay-ms <50>; > > }; > > diff --git a/sound/soc/codecs/dmic.c b/sound/soc/codecs/dmic.c > > index cf83c423394d..b5f60ac22ace 100644 > > --- a/sound/soc/codecs/dmic.c > > +++ b/sound/soc/codecs/dmic.c > [...] > > > static const struct snd_soc_dapm_widget dmic_dapm_widgets[] = { > > - SND_SOC_DAPM_AIF_OUT("DMIC AIF", "Capture", 0, > > - SND_SOC_NOPM, 0, 0), > > + SND_SOC_DAPM_AIF_OUT_E("DMIC AIF", "Capture", 0, > > + SND_SOC_NOPM, 0, 0, dmic_aif_event, > > + SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD), > > Any good reason both are _POST_? It seems like you'd want them to be > inverses (e.g., power-power-up, and pre-power-down). Post-power-up is needed to avoid a popping noise when the clock is enabled (see 3a6f9dce6116 ("ASoC: rk3399_gru_sound: fix recording pop at first attempt") and disabling the mic before switching off the clock (pre-power-down) could cause noise at the end of the recording. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html