Re: [PATCH] CHROMIUM: ASoC: Intel: boards: Use FS as nau8825 sysclk in nau88l25_max98357a machine

[Vinod Koul <vinod.koul@intel.com>]

On Thu, Nov 17, 2016 at 02:35:15PM +0800, Grace Kao wrote:
> 256FS sysclk gives the best audio quality per nau8825 datasheet.
> 
> Loud headphone pop happens if there is no sysclk during nau8825
> playback power up sequence. Currently Skylake does not output MCLK/FS
> when the back-end DAI op hw_param is called, so we cannot switch to
> MCLK/FS in hw_param. This patch reduces pop by leting nau8825 keep
> using its internal VCO clock during widget power up sequence, until
> SNDRV_PCM_TRIGGER_START when MCLK/FS is available.

Please CC maintainers Mark and Liam on ASoC patches

The tag CHROMIUM is not fit for upstream please remove

> 
> This is a temporary fix before Intel ADSP can output MCLK/FS early.
> 
> BUG=chrome-os-partner:50471
> TEST=`emerge-sentry sys-kernel/chromeos-kernel-3_18`. Verified a local
> build that could reduce the pop sound.

This is just noise for upstream, please remove

> 
> Change-Id: I4943fffa8eb1c874cc66b50f0897092bf48dd579

This as well

> Signed-off-by: Grace Kao <grace.kao@intel.com>
> Reviewed-on: https://chromium-review.googlesource.com/333180

And this

> Reviewed-by: Ben Zhang <benzh@chromium.org>
> 
> Series-to: alsa-devel@alsa-project.org
> Series-cc: Vinod.koul@intel.com

here too

> ---
> 
> diff --git a/sound/soc/intel/boards/skl_nau88l25_max98357a.c b/sound/soc/intel/boards/skl_nau88l25_max98357a.c
> index 0f89602..6ad8f52 100644
> --- a/sound/soc/intel/boards/skl_nau88l25_max98357a.c
> +++ b/sound/soc/intel/boards/skl_nau88l25_max98357a.c
> @@ -70,14 +70,7 @@
>  		return -EIO;
>  	}
>  
> -	if (SND_SOC_DAPM_EVENT_ON(event)) {
> -		ret = snd_soc_dai_set_sysclk(codec_dai,
> -				NAU8825_CLK_MCLK, 24000000, SND_SOC_CLOCK_IN);
> -		if (ret < 0) {
> -			dev_err(card->dev, "set sysclk err = %d\n", ret);
> -			return -EIO;
> -		}
> -	} else {
> +	if (!SND_SOC_DAPM_EVENT_ON(event)) {

SND_SOC_DAPM_EVENT_OFF ??

> +static int skylake_nau8825_trigger(struct snd_pcm_substream *substream, int cmd)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct snd_pcm_runtime *runtime = substream->runtime;
> +	struct snd_soc_dai *codec_dai = rtd->codec_dai;
> +	int ret = 0;
> +
> +	switch (cmd) {
> +	case SNDRV_PCM_TRIGGER_START:
> +		ret = snd_soc_dai_set_sysclk(codec_dai,	NAU8825_CLK_FLL_FS, 0,
> +			SND_SOC_CLOCK_IN);
> +		if (ret < 0)
> +			dev_err(codec_dai->dev, "can't set FS clock %d\n", ret);
> +		ret = snd_soc_dai_set_pll(codec_dai, 0, 0, runtime->rate,
> +			runtime->rate * 256);
> +		if (ret < 0)
> +			dev_err(codec_dai->dev, "can't set FLL: %d\n", ret);
> +		break;

starting clock on trigger doesnt sound right to me..

-- 
~Vinod