Re: [alsa-devel] [PATCH] ASoC: Intel: eve: Enable mclk and ssp sclk early

rt5663 and rt5514 needs mclk/sclk early to synchronize its internal clocks.

There are a couple of differences with similar code used in kbl_rt5663_max98927, see below

diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig index 5c27f7a..d5f167e 100644 --- a/sound/soc/intel/boards/Kconfig +++ b/sound/soc/intel/boards/Kconfig @@ -315,6 +315,7 @@ config SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH depends on I2C && ACPI depends on MFD_INTEL_LPSS || COMPILE_TEST depends on SPI

• select SND_SOC_INTEL_SKYLAKE_SSP_CLK

It would be nicer to follow the same order as for kbl_rt5663_max98927, with this SKYLAKE_SSP_CLK added last after HDAC_HDMI

+static int platform_clock_control(struct snd_soc_dapm_widget *w,

• 	struct snd_kcontrol *k, int  event)
  

+{

• struct snd_soc_dapm_context *dapm = w->dapm;
• struct snd_soc_card *card = dapm->card;
• struct kbl_codec_private *priv = snd_soc_card_get_drvdata(card);
• int ret = 0;
• /*

• * MCLK/SCLK need to be ON early for a successful synchronization of
  

• * codec internal clock. And the clocks are turned off during
  

• * POST_PMD after the stream is stopped.
  

• */
  

• switch (event) {
• case SND_SOC_DAPM_PRE_PMU:

• if (__clk_is_enabled(priv->mclk))
  

• 	return 0;
  

Is this if() test needed? it's not part of the code for kbl_rt5663_max98927, despite all the comments and code structure being identical.

• /* Enable MCLK */
  

• ret = clk_set_rate(priv->mclk, 24000000);
  

• if (ret < 0) {
  

• 	dev_err(card->dev, "Can't set rate for mclk, err: %d\n",
  

• 		ret);
  

• 	return ret;
  

• }
  

• ret = clk_prepare_enable(priv->mclk);
  

• if (ret < 0) {
  

• 	dev_err(card->dev, "Can't enable mclk, err: %d\n", ret);
  

• 	return ret;
  

• }
  

• /* Enable SCLK */
  

• ret = clk_set_rate(priv->sclk, 3072000);
  

• if (ret < 0) {
  

• 	dev_err(card->dev, "Can't set rate for sclk, err: %d\n",
  

• 		ret);
  

• 	clk_disable_unprepare(priv->mclk);
  

• 	return ret;
  

• }
  

• ret = clk_prepare_enable(priv->sclk);
  

• if (ret < 0) {
  

• 	dev_err(card->dev, "Can't enable sclk, err: %d\n", ret);
  

• 	clk_disable_unprepare(priv->mclk);
  

• }
  

• break;
  

• case SND_SOC_DAPM_POST_PMD:

• if (!__clk_is_enabled(priv->mclk))
  

• 	return 0;
  

same here, is this if() test needed? If yes, isn't it needed in kbl_rt5663_max98927?

• clk_disable_unprepare(priv->mclk);
  

• clk_disable_unprepare(priv->sclk);
  

• break;
  

• default:

• return 0;
  

• }
• return 0;

+}

While I am at it, this machine driver uses .ignore_pmdown_time, which is typically used to avoid clock issues. Now that you have an explicit control on clocks, is .ignore_pmdown_time actually required?