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

[Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>]

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