On Thu, Oct 17, 2013 at 05:01:10PM +0800, Xiubo Li wrote:
+static struct snd_pcm_hardware snd_fsl_hardware = {
- .info = SNDRV_PCM_INFO_INTERLEAVED |
SNDRV_PCM_INFO_BLOCK_TRANSFER |SNDRV_PCM_INFO_MMAP |SNDRV_PCM_INFO_MMAP_VALID |SNDRV_PCM_INFO_PAUSE |SNDRV_PCM_INFO_RESUME,- .formats = SNDRV_PCM_FMTBIT_S16_LE,
- .rate_min = 8000,
- .channels_min = 2,
- .channels_max = 2,
- .buffer_bytes_max = FSL_SAI_DMABUF_SIZE,
- .period_bytes_min = 4096,
- .period_bytes_max = FSL_SAI_DMABUF_SIZE / TCD_NUMBER,
- .periods_min = TCD_NUMBER,
- .periods_max = TCD_NUMBER,
- .fifo_size = 0,
+};
There's a patch in -next that lets the generic dmaengine code figure out some settings from the dmacontroller rather than requiring the driver to explicitly provide configuration - it's "ASoC: dmaengine-pcm: Provide default config". Please update your driver to use this, or let's work out what it doesn't do any try to fix it.
- ret = fsl_sai_set_dai_sysclk_tr(cpu_dai, clk_id, freq,
FSL_FMT_TRANSMITTER);- if (ret) {
dev_err(cpu_dai->dev,"Cannot set sai's transmitter sysclk: %d\n",ret);return ret;- }
- ret = fsl_sai_set_dai_sysclk_tr(cpu_dai, clk_id, freq,
FSL_FMT_RECEIVER);
As other people have commented these should be exposed as separate clocks rather than set in sync, unless there's some hardware reason they need to be identical. If that is the case then a comment explaining the limitation would be good.
Similarly with several of the other functions.
+int fsl_sai_dai_remove(struct snd_soc_dai *dai) +{
- struct fsl_sai *sai = dev_get_drvdata(dai->dev);
- clk_disable_unprepare(sai->clk);
It'd be a bit nicer to only enable the clock while the driver is actively being used rather than all the time the system is powered up but it's not a blocker for merge.
- ret = snd_soc_register_component(&pdev->dev, &fsl_component,
&fsl_sai_dai, 1);- if (ret)
return ret;
There's a devm_snd_soc_register_component() in -next, please use that.
- ret = fsl_pcm_dma_init(pdev);
- if (ret)
goto out;- platform_set_drvdata(pdev, sai);
These should go before the driver is registered with the subsystem otherwise you've got a race where something might try to use the driver before init is finished.
+static int fsl_sai_remove(struct platform_device *pdev) +{
- struct fsl_sai *sai = platform_get_drvdata(pdev);
- fsl_pcm_dma_exit(pdev);
- snd_soc_unregister_component(&pdev->dev);
Similarly here, unregister from the subsystem then clean up after.
+#define SAI_CR5_FBT(x) ((x) << 8) +#define SAI_CR5_FBT_MASK (0x1f << 8)
+/* SAI audio dividers */ +#define FSL_SAI_TX_DIV 0 +#define FSL_SAI_RX_DIV 1
Make the namespacing consistent please - for preference use FSL_SAI always.