linux-kernel - Re: [PATCH v2] ASoC: rk3399_gru_sound: fix recording pop at first attempt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160923212737.GA17658@rob-hp-laptop>
Date:   Fri, 23 Sep 2016 16:27:37 -0500
From:   Rob Herring <robh@...nel.org>
To:     Xing Zheng <zhengxing@...k-chips.com>
Cc:     linux-rockchip@...ts.infradead.org, woojoo.lee@...sung.com,
        cychiang@...gle.com, hychao@...gle.com, smbarber@...gle.com,
        heiko@...ech.de, dianders@...gle.com,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, alsa-devel@...a-project.org,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] ASoC: rk3399_gru_sound: fix recording pop at first
 attempt

On Thu, Sep 22, 2016 at 09:50:06PM +0800, Xing Zheng wrote:
> From: Wonjoon Lee <woojoo.lee@...sung.com>
> 
> Pop happens when mclk applied but dmic's own boot-time

This statement doesn't make sense.

> Specify dmic delay times in dt to make sure
> clocks are ready earlier than dmic working

I don't think this really belongs in DT. Seems like better control of 
the init order is needed. Or why not just a fixed delay in the driver?

> 
> Signed-off-by: Wonjoon Lee <woojoo.lee@...sung.com>
> Signed-off-by: Xing Zheng <zhengxing@...k-chips.com>
> ---
> 
> Changes in v2:
> - rename dmic-delay to dmic-wakeup-delay-ms
> 
>  .../bindings/sound/rockchip,rk3399-gru-sound.txt           |  7 +++++++
>  sound/soc/rockchip/rk3399_gru_sound.c                      | 14 ++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt b/Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt
> index f19b6c8..eac91db 100644
> --- a/Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt
> +++ b/Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt
> @@ -6,10 +6,17 @@ Required properties:
>    connected to the codecs
>  - rockchip,codec: The phandle of the MAX98357A/RT5514/DA7219 codecs
>  
> +Optional properties:
> +- dmic-wakeup-delay-ms : specify delay time (ms) for DMIC ready.
> +  If this option is specified, which means it's required dmic need
> +  delay for DMIC to ready so that rt5514 can avoid recording before
> +  DMIC send valid data
> +
>  Example:
>  
>  sound {
>  	compatible = "rockchip,rk3399-gru-sound";
>  	rockchip,cpu = <&i2s0>;
>  	rockchip,codec = <&max98357a &rt5514 &da7219>;
> +	dmic-wakeup-delay-ms = <20>;
>  };
> diff --git a/sound/soc/rockchip/rk3399_gru_sound.c b/sound/soc/rockchip/rk3399_gru_sound.c
> index ee06489..9ed735a 100644
> --- a/sound/soc/rockchip/rk3399_gru_sound.c
> +++ b/sound/soc/rockchip/rk3399_gru_sound.c
> @@ -38,6 +38,8 @@
>  
>  #define SOUND_FS	256
>  
> +unsigned int rt5514_dmic_delay;
> +
>  static struct snd_soc_jack rockchip_sound_jack;
>  
>  static const struct snd_soc_dapm_widget rockchip_dapm_widgets[] = {
> @@ -123,6 +125,9 @@ static int rockchip_sound_rt5514_hw_params(struct snd_pcm_substream *substream,
>  		return ret;
>  	}
>  
> +	/* Wait for DMIC stable */
> +	msleep(rt5514_dmic_delay);
> +
>  	return 0;
>  }
>  
> @@ -343,6 +348,15 @@ static int rockchip_sound_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> +	/* Set DMIC delay */
> +	ret = device_property_read_u32(&pdev->dev, "dmic-delay",

Obviously, this doesn't work now...

> +					&rt5514_dmic_delay);
> +	if (ret) {
> +		rt5514_dmic_delay = 0;
> +		dev_dbg(&pdev->dev,
> +			"no optional property 'dmic-delay' found, default: no delay\n");
> +	}
> +
>  	rockchip_dailinks[DAILINK_RT5514_DSP].cpu_name = kstrdup_const(dev_name(dev), GFP_KERNEL);
>  	rockchip_dailinks[DAILINK_RT5514_DSP].cpu_dai_name = kstrdup_const(dev_name(dev), GFP_KERNEL);
>  	rockchip_dailinks[DAILINK_RT5514_DSP].platform_name = kstrdup_const(dev_name(dev), GFP_KERNEL);
> -- 
> 1.9.1
> 
>