Re: [PATCHv2 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.

From: Mark Brown
Date: Fri Nov 01 2013 - 14:26:30 EST


On Fri, Nov 01, 2013 at 03:04:48PM +0800, Xiubo Li wrote:

> +static int fsl_sai_set_dai_clkdiv(struct snd_soc_dai *cpu_dai,
> + int div_id, int div)
> +{
> + struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
> + u32 tcr2, rcr2;
> +
> + if (div_id == FSL_SAI_TX_DIV) {
> + tcr2 = readl(sai->base + FSL_SAI_TCR2);
> + tcr2 &= ~FSL_SAI_CR2_DIV_MASK;
> + tcr2 |= FSL_SAI_CR2_DIV(div);
> + writel(tcr2, sai->base + FSL_SAI_TCR2);

What is this divider and why does the user have to set it manually?

> + } else
> + return -EINVAL;
> +

Coding style?

> +static int fsl_sai_dai_probe(struct snd_soc_dai *dai)
> +{
> + int ret;
> + struct fsl_sai *sai = dev_get_drvdata(dai->dev);
> +
> + ret = clk_prepare_enable(sai->clk);
> + if (ret)
> + return ret;

It'd be nicer to only enable the clock while the device is in active
use.

> + ret = snd_dmaengine_pcm_register(&pdev->dev, NULL,
> + SND_DMAENGINE_PCM_FLAG_NO_RESIDUE);
> + if (ret)
> + return ret;

We should have a devm_ version of this.

Attachment: signature.asc
Description: Digital signature