Re: [PATCH v2] ASoC: Intel: boards: eve: Fix DMIC records zero

/* * MCLK/SCLK need to be ON early for a successful synchronization of * codec internal clock. And the clocks are turned off during @@ -91,38 +108,48 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w, */ switch (event) { case SND_SOC_DAPM_PRE_PMU:

• /* 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;
  

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

• 		ssp_num, ret);
  

• 		return ret;
  

nit-pick: alignment is off for the '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;
  

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

• 	/* Enable MCLK */
  

• 	ret = clk_prepare_enable(priv->mclk);
  

That seems correct since you share the mclk between two resources but see [1] below

• 	if (ret < 0) {
  

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

• 			ssp_num, ret);
  

• 		return ret;
  

• 	}
  

  }

• /* Enable SCLK */
  

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

• ret = clk_set_rate(sclk, sclk_rate);
  

  if (ret < 0) {

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

• 		ret);
  

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

• 		ssp_num, 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);
  

• if (!__clk_is_enabled(sclk)) {
  

Why do you need this test? the sclocks are not shared? see also [2] below

• 	/* Enable SCLK */
  

• 	ret = clk_prepare_enable(sclk);
  

• 	if (ret < 0) {
  

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

• 			ssp_num, ret);
  

• 		clk_disable_unprepare(priv->mclk);
  

• 		return ret;
  

• 	}
  

  } break; case SND_SOC_DAPM_POST_PMD:

• clk_disable_unprepare(priv->mclk);
  

• clk_disable_unprepare(priv->sclk);
  

• if (__clk_is_enabled(priv->mclk))
  

• 	clk_disable_unprepare(priv->mclk);
  

[1] this seems wrong in case you have two SSPs working, and stop one. This would turn off the mclk while one of the two SSPs is still working.

• if (__clk_is_enabled(sclk))
  

[2] Again is this test needed since sclk is not shared between SSPs

• 	clk_disable_unprepare(sclk);