[alsa-devel] [PATCH V1 1/3] ASoC: fsl_esai: spba clock is needed by esai device

[Nicolin Chen]

Hi,

On Thu, Sep 17, 2015 at 02:09:23PM +0800, Shengjiu Wang wrote:
> esai need to enable the spba clock, when sdma is using share peripheral
> script. If don't enable it, may cause the read/write wrong data from/to
> register.

It would be nicer to elaborate the SPBA as well, especially
the reason why it "may cause the read/write wrong data from/to
register".

> Signed-off-by: Shengjiu Wang <shengjiu.wang at freescale.com>

> diff --git a/Documentation/devicetree/bindings/sound/fsl,esai.txt b/Documentation/devicetree/bindings/sound/fsl,esai.txt
> index d3b6b5f..d677203 100644
> --- a/Documentation/devicetree/bindings/sound/fsl,esai.txt
> +++ b/Documentation/devicetree/bindings/sound/fsl,esai.txt
> @@ -27,6 +27,8 @@ Required properties:
>  			  derive HCK, SCK and FS.
>  	"fsys"		  The system clock derived from ahb clock used to
>  			  derive HCK, SCK and FS.
> +	"dma"		  The spba clock is needed when sdma share peripheral
> +			  script is used.

It would be better to treat it as an optional property as ESAI
itself doesn't require it to be enabled while a specific SDMA
script does so as to access the registers successfully via the
arbiter. There is one of existing SoCs that doesn't have SDMA
and SPBA at all so they don't need it.

> @@ -469,6 +470,9 @@ static int fsl_esai_startup(struct snd_pcm_substream *substream,
>  	ret = clk_prepare_enable(esai_priv->coreclk);
>  	if (ret)
>  		return ret;
> +	ret = clk_prepare_enable(esai_priv->dmaclk);
> +	if (ret)
> +		return ret;

Needs a goto for a failed case in order to disable the coreclk.

> @@ -772,6 +778,13 @@ static int fsl_esai_probe(struct platform_device *pdev)
>  		dev_warn(&pdev->dev, "failed to get fsys clock: %ld\n",
>  				PTR_ERR(esai_priv->fsysclk));
>  
> +	esai_priv->dmaclk = devm_clk_get(&pdev->dev, "dma");
> +	if (IS_ERR(esai_priv->dmaclk)) {
> +		dev_err(&pdev->dev, "Cannot get dma clock: %ld\n",
> +				PTR_ERR(esai_priv->dmaclk));
> +		return PTR_ERR(esai_priv->dmaclk);
> +	}

This will break the old DTB -- would be solved by making the clock
optional though. And it would be better to follow the pattern of
previous error message.

Nicolin