| Message ID | 1383289495-24523-2-git-send-email-Li.Xiubo@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Xiubo, On Fri, Nov 01, 2013 at 03:04:48PM +0800, Xiubo Li wrote: > This adds Freescale SAI ASoC Audio support. > This implementation is only compatible with device tree definition. > Features: > o Supports playback/capture > o Supports 16/20/24 bit PCM > o Supports 8k - 96k sample rates > o Supports slave mode only. > Just for curiosity, I found there're quite a bit code actually support I2S master mode such as set_sysclk() and register configuration to FMT SND_SOC_DAIFMT_CBS_CFS. Is that really "supports slave mode only"? Or just you haven't make it properly work? > diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig > index b7ab71f..9a8851e 100644 > --- a/sound/soc/fsl/Kconfig > +++ b/sound/soc/fsl/Kconfig > @@ -213,3 +213,19 @@ config SND_SOC_IMX_MC13783 > select SND_SOC_IMX_PCM_DMA > > endif # SND_IMX_SOC > + > +menuconfig SND_FSL_SOC > + tristate "SoC Audio for Freescale FSL CPUs" > + help > + Say Y or M if you want to add support for codecs attached to > + the FSL CPUs. > + > + This will enable Freeacale SAI, SGT15000 codec. > + > +if SND_FSL_SOC Why check this here? SAI should be a regular IP module which can be used by other SoC as well. We would never know if next i.MX SoC won't contain SAI. > + > +config SND_SOC_FSL_SAI > + tristate > + select SND_SOC_GENERIC_DMAENGINE_PCM I think you should keep SAI beside SSI/SPDIF directly. > + > +endif # SND_FSL_SOC > diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile > index 8db705b..e5acc03 100644 > --- a/sound/soc/fsl/Makefile > +++ b/sound/soc/fsl/Makefile > @@ -56,3 +56,8 @@ obj-$(CONFIG_SND_SOC_IMX_SGTL5000) += snd-soc-imx-sgtl5000.o > obj-$(CONFIG_SND_SOC_IMX_WM8962) += snd-soc-imx-wm8962.o > obj-$(CONFIG_SND_SOC_IMX_SPDIF) += snd-soc-imx-spdif.o > obj-$(CONFIG_SND_SOC_IMX_MC13783) += snd-soc-imx-mc13783.o > + > +# FSL ARM SAI/SGT15000 Platform Support Should be SGTL5000, not SGT1. And SAI should not be used with SGTL5000 only, it's a bit confusing to mention SGTL5000 here. > +snd-soc-fsl-sai-objs := fsl-sai.o And I think it should be better to put it along with fsl-ssi.o and fsl-spdif.o > + > +obj-$(CONFIG_SND_SOC_FSL_SAI) += snd-soc-fsl-sai.o Ditto > diff --git a/sound/soc/fsl/fsl-sai.c b/sound/soc/fsl/fsl-sai.c > new file mode 100644 > index 0000000..bb57e67 > --- /dev/null > +++ b/sound/soc/fsl/fsl-sai.c > @@ -0,0 +1,472 @@ > +/* > + * Freescale SAI ALSA SoC Digital Audio Interface driver. > + * > + * Copyright 2012-2013 Freescale Semiconductor, Inc. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. There're too many double space inside. Could you pls revise it? > + * > + */ > + > +#include <linux/clk.h> > +#include <linux/module.h> > +#include <linux/slab.h> > +#include <linux/of_address.h> > +#include <sound/core.h> > +#include <sound/pcm_params.h> > +#include <linux/delay.h> > +#include <linux/dmaengine.h> > +#include <sound/dmaengine_pcm.h> I think it's better to keep <sound/xxx.h> together. And basically we can keep header files ordered by alphabet. > + > +#include "fsl-sai.h" > + > +static int fsl_sai_set_dai_sysclk_tr(struct snd_soc_dai *cpu_dai, > + int clk_id, unsigned int freq, int fsl_dir) > +{ > + u32 val_cr2, reg_cr2; > + struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); > + > + if (fsl_dir == FSL_FMT_TRANSMITTER) > + reg_cr2 = FSL_SAI_TCR2; > + else > + reg_cr2 = FSL_SAI_RCR2; > + > + val_cr2 = readl(sai->base + reg_cr2); > + switch (clk_id) { > + case FSL_SAI_CLK_BUS: > + val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK; > + val_cr2 |= FSL_SAI_CR2_MSEL_BUS; > + break; > + case FSL_SAI_CLK_MAST1: > + val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK; > + val_cr2 |= FSL_SAI_CR2_MSEL_MCLK1; > + break; > + case FSL_SAI_CLK_MAST2: > + val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK; > + val_cr2 |= FSL_SAI_CR2_MSEL_MCLK2; > + break; > + case FSL_SAI_CLK_MAST3: > + val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK; > + val_cr2 |= FSL_SAI_CR2_MSEL_MCLK3; > + break; > + default: > + return -EINVAL; > + } > + writel(val_cr2, sai->base + reg_cr2); > + > + return 0; > +} > + > +static int fsl_sai_set_dai_sysclk(struct snd_soc_dai *cpu_dai, > + int clk_id, unsigned int freq, int dir) > +{ > + int ret; > + > + if (dir == SND_SOC_CLOCK_IN) > + return 0; > + > + 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); > + if (ret) { > + dev_err(cpu_dai->dev, > + "Cannot set sai's receiver sysclk: %d\n", > + ret); > + return ret; > + } > + > + return 0; > +} > + > +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); > + > + } else if (div_id == FSL_SAI_RX_DIV) { > + rcr2 = readl(sai->base + FSL_SAI_RCR2); > + rcr2 &= ~FSL_SAI_CR2_DIV_MASK; > + rcr2 |= FSL_SAI_CR2_DIV(div); > + writel(rcr2, sai->base + FSL_SAI_RCR2); > + > + } else > + return -EINVAL; It would look better if using switch-case. And the last 'else' could be 'default:'. > + > + return 0; > +} > + > +static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai *cpu_dai, > + unsigned int fmt, int fsl_dir) > +{ > + u32 val_cr2, val_cr3, val_cr4, reg_cr2, reg_cr3, reg_cr4; > + struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); > + > + if (fsl_dir == FSL_FMT_TRANSMITTER) { > + reg_cr2 = FSL_SAI_TCR2; > + reg_cr3 = FSL_SAI_TCR3; > + reg_cr4 = FSL_SAI_TCR4; > + } else { > + reg_cr2 = FSL_SAI_RCR2; > + reg_cr3 = FSL_SAI_RCR3; > + reg_cr4 = FSL_SAI_RCR4; > + } > + > + val_cr2 = readl(sai->base + reg_cr2); > + val_cr3 = readl(sai->base + reg_cr3); > + val_cr4 = readl(sai->base + reg_cr4); > + > + if (sai->fbt == FSL_SAI_FBT_MSB) > + val_cr4 |= FSL_SAI_CR4_MF; > + else if (sai->fbt == FSL_SAI_FBT_LSB) > + val_cr4 &= ~FSL_SAI_CR4_MF; > + > + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { > + case SND_SOC_DAIFMT_I2S: > + val_cr4 |= FSL_SAI_CR4_FSE; > + val_cr4 |= FSL_SAI_CR4_FSP; > + break; > + default: > + return -EINVAL; > + } > + > + switch (fmt & SND_SOC_DAIFMT_INV_MASK) { > + case SND_SOC_DAIFMT_IB_IF: > + val_cr4 |= FSL_SAI_CR4_FSP; > + val_cr2 &= ~FSL_SAI_CR2_BCP; > + break; > + case SND_SOC_DAIFMT_IB_NF: > + val_cr4 &= ~FSL_SAI_CR4_FSP; > + val_cr2 &= ~FSL_SAI_CR2_BCP; > + break; > + case SND_SOC_DAIFMT_NB_IF: > + val_cr4 |= FSL_SAI_CR4_FSP; > + val_cr2 |= FSL_SAI_CR2_BCP; > + break; > + case SND_SOC_DAIFMT_NB_NF: > + val_cr4 &= ~FSL_SAI_CR4_FSP; > + val_cr2 |= FSL_SAI_CR2_BCP; > + break; > + default: > + return -EINVAL; > + } > + > + switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { > + case SND_SOC_DAIFMT_CBS_CFS: > + val_cr2 |= FSL_SAI_CR2_BCD_MSTR; > + val_cr4 |= FSL_SAI_CR4_FSD_MSTR; > + break; > + case SND_SOC_DAIFMT_CBM_CFM: > + val_cr2 &= ~FSL_SAI_CR2_BCD_MSTR; > + val_cr4 &= ~FSL_SAI_CR4_FSD_MSTR; > + break; > + default: > + return -EINVAL; > + } > + > + val_cr3 |= FSL_SAI_CR3_TRCE; > + > + if (fsl_dir == FSL_FMT_RECEIVER) > + val_cr2 |= FSL_SAI_CR2_SYNC; > + > + writel(val_cr2, sai->base + reg_cr2); > + writel(val_cr3, sai->base + reg_cr3); > + writel(val_cr4, sai->base + reg_cr4); > + > + return 0; > + Pls drop this extra line. > +} > + > +static int fsl_sai_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) > +{ > + int ret; > + > + ret = fsl_sai_set_dai_fmt_tr(cpu_dai, fmt, FSL_FMT_TRANSMITTER); > + if (ret) { > + dev_err(cpu_dai->dev, > + "Cannot set sai's transmitter format: %d\n", > + ret); > + return ret; > + } > + > + ret = fsl_sai_set_dai_fmt_tr(cpu_dai, fmt, FSL_FMT_RECEIVER); > + if (ret) { > + dev_err(cpu_dai->dev, > + "Cannot set sai's receiver format: %d\n", > + ret); > + return ret; > + } > + > + return 0; > +} > + > +static int fsl_sai_set_dai_tdm_slot(struct snd_soc_dai *cpu_dai, > + unsigned int tx_mask, unsigned int rx_mask, > + int slots, int slot_width) > +{ > + struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); > + u32 tcr4, rcr4; > + > + tcr4 = readl(sai->base + FSL_SAI_TCR4); > + tcr4 &= ~FSL_SAI_CR4_FRSZ_MASK; > + tcr4 |= FSL_SAI_CR4_FRSZ(2); > + writel(tcr4, sai->base + FSL_SAI_TCR4); > + writel(tx_mask, sai->base + FSL_SAI_TMR); > + > + rcr4 = readl(sai->base + FSL_SAI_RCR4); > + rcr4 &= ~FSL_SAI_CR4_FRSZ_MASK; > + rcr4 |= FSL_SAI_CR4_FRSZ(2); > + writel(rcr4, sai->base + FSL_SAI_RCR4); > + writel(rx_mask, sai->base + FSL_SAI_RMR); > + > + return 0; > +} > + > +static int fsl_sai_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params, > + struct snd_soc_dai *cpu_dai) > +{ > + u32 val_cr4, val_cr5, reg_cr4, reg_cr5, word_width; > + struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); > + > + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { > + reg_cr4 = FSL_SAI_TCR4; > + reg_cr5 = FSL_SAI_TCR5; > + } else { > + reg_cr4 = FSL_SAI_RCR4; > + reg_cr5 = FSL_SAI_RCR5; > + } > + > + val_cr4 = readl(sai->base + reg_cr4); > + val_cr4 &= ~FSL_SAI_CR4_SYWD_MASK; > + > + val_cr5 = readl(sai->base + reg_cr5); > + val_cr5 &= ~FSL_SAI_CR5_WNW_MASK; > + val_cr5 &= ~FSL_SAI_CR5_W0W_MASK; > + val_cr5 &= ~FSL_SAI_CR5_FBT_MASK; > + > + switch (params_format(params)) { > + case SNDRV_PCM_FORMAT_S16_LE: > + word_width = 16; > + break; > + case SNDRV_PCM_FORMAT_S20_3LE: > + word_width = 20; > + break; > + case SNDRV_PCM_FORMAT_S24_LE: > + word_width = 24; > + break; > + default: > + return -EINVAL; > + } > + > + val_cr4 |= FSL_SAI_CR4_SYWD(word_width); > + val_cr5 |= FSL_SAI_CR5_WNW(word_width); > + val_cr5 |= FSL_SAI_CR5_W0W(word_width); > + > + if (sai->fbt == FSL_SAI_FBT_MSB) > + val_cr5 |= FSL_SAI_CR5_FBT(word_width - 1); > + else if (sai->fbt == FSL_SAI_FBT_LSB) > + val_cr5 |= FSL_SAI_CR5_FBT(0); > + > + writel(val_cr4, sai->base + reg_cr4); > + writel(val_cr5, sai->base + reg_cr5); > + > + return 0; > +} > + > +static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, > + struct snd_soc_dai *dai) > +{ > + struct fsl_sai *sai = snd_soc_dai_get_drvdata(dai); > + unsigned int tcsr, rcsr; > + > + tcsr = readl(sai->base + FSL_SAI_TCSR); > + rcsr = readl(sai->base + FSL_SAI_RCSR); > + > + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { > + tcsr |= FSL_SAI_CSR_FRDE; > + rcsr &= ~FSL_SAI_CSR_FRDE; > + } else { > + rcsr |= FSL_SAI_CSR_FRDE; > + tcsr &= ~FSL_SAI_CSR_FRDE; > + } > + > + switch (cmd) { > + case SNDRV_PCM_TRIGGER_START: > + case SNDRV_PCM_TRIGGER_RESUME: > + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: > + tcsr |= FSL_SAI_CSR_TERE; > + rcsr |= FSL_SAI_CSR_TERE; > + writel(rcsr, sai->base + FSL_SAI_RCSR); > + udelay(10); Does SAI really needs this udelay() here? Required by IP's operation flow? If so, I think it's better to add comments here to explain it. > + writel(tcsr, sai->base + FSL_SAI_TCSR); > + break; > + > + case SNDRV_PCM_TRIGGER_STOP: > + case SNDRV_PCM_TRIGGER_SUSPEND: > + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: > + if (!(dai->playback_active || dai->capture_active)) { > + tcsr &= ~FSL_SAI_CSR_TERE; > + rcsr &= ~FSL_SAI_CSR_TERE; > + } > + writel(rcsr, sai->base + FSL_SAI_RCSR); > + udelay(10); > + writel(tcsr, sai->base + FSL_SAI_TCSR); > + break; > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = { > + .set_sysclk = fsl_sai_set_dai_sysclk, > + .set_clkdiv = fsl_sai_set_dai_clkdiv, > + .set_fmt = fsl_sai_set_dai_fmt, > + .set_tdm_slot = fsl_sai_set_dai_tdm_slot, > + .hw_params = fsl_sai_hw_params, > + .trigger = fsl_sai_trigger, > +}; > + > +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; > + > + writel(0x0, sai->base + FSL_SAI_RCSR); > + writel(0x0, sai->base + FSL_SAI_TCSR); > + writel(sai->dma_params_tx.maxburst, sai->base + FSL_SAI_TCR1); > + writel(sai->dma_params_rx.maxburst, sai->base + FSL_SAI_RCR1); > + > + dai->playback_dma_data = &sai->dma_params_tx; > + dai->capture_dma_data = &sai->dma_params_rx; > + > + snd_soc_dai_set_drvdata(dai, sai); > + > + return 0; > +} > + > +int fsl_sai_dai_remove(struct snd_soc_dai *dai) static > +{ > + struct fsl_sai *sai = dev_get_drvdata(dai->dev); > + > + clk_disable_unprepare(sai->clk); > + > + return 0; > +} > + > +static struct snd_soc_dai_driver fsl_sai_dai = { > + .probe = fsl_sai_dai_probe, > + .remove = fsl_sai_dai_remove, > + .playback = { > + .channels_min = 1, > + .channels_max = 2, > + .rates = SNDRV_PCM_RATE_8000_96000, > + .formats = FSL_SAI_FORMATS, > + }, > + .capture = { > + .channels_min = 1, > + .channels_max = 2, > + .rates = SNDRV_PCM_RATE_8000_96000, > + .formats = FSL_SAI_FORMATS, > + }, > + .ops = &fsl_sai_pcm_dai_ops, > +}; > + > +static const struct snd_soc_component_driver fsl_component = { > + .name = "fsl-sai", > +}; > + > +static int fsl_sai_probe(struct platform_device *pdev) > +{ > + struct resource *res; > + struct fsl_sai *sai; > + int ret = 0; > + > + sai = devm_kzalloc(&pdev->dev, sizeof(*sai), GFP_KERNEL); > + if (!sai) > + return -ENOMEM; > + > + sai->fbt = FSL_SAI_FBT_MSB; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + sai->base = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(sai->base)) > + return PTR_ERR(sai->base); > + > + sai->clk = devm_clk_get(&pdev->dev, "sai"); > + if (IS_ERR(sai->clk)) { > + dev_err(&pdev->dev, "Cannot get sai's clock: %d\n", ret); > + return PTR_ERR(sai->clk); > + } > + > + sai->dma_params_rx.addr = res->start + FSL_SAI_RDR; > + sai->dma_params_rx.maxburst = 6; > + > + sai->dma_params_tx.addr = res->start + FSL_SAI_TDR; > + sai->dma_params_tx.maxburst = 6; > + > + platform_set_drvdata(pdev, sai); > + > + ret = devm_snd_soc_register_component(&pdev->dev, &fsl_component, > + &fsl_sai_dai, 1); > + if (ret) > + return ret; > + > + ret = snd_dmaengine_pcm_register(&pdev->dev, NULL, > + SND_DMAENGINE_PCM_FLAG_NO_RESIDUE); > + if (ret) > + return ret; > + > + return 0; > +} > + > +static int fsl_sai_remove(struct platform_device *pdev) > +{ > + snd_dmaengine_pcm_unregister(&pdev->dev); > + > + return 0; > +} > + > +static const struct of_device_id fsl_sai_ids[] = { > + { .compatible = "fsl,vf610-sai", }, > + { /*sentinel*/ } I think this could be left in blank without comments inside. And if you really want to add it, pls add like: /* sentinel */ ^ ^ > +}; > + > +static struct platform_driver fsl_sai_driver = { > + .probe = fsl_sai_probe, > + .remove = fsl_sai_remove, > + > + .driver = { > + .name = "fsl-sai", > + .owner = THIS_MODULE, > + .of_match_table = fsl_sai_ids, > + }, > +}; > +module_platform_driver(fsl_sai_driver); > + > +MODULE_AUTHOR("Xiubo Li, <Li.Xiubo@freescale.com>"); > +MODULE_AUTHOR("Alison Wang, <b18965@freescale.com>"); > +MODULE_DESCRIPTION("Freescale Soc SAI Interface"); > +MODULE_LICENSE("GPL"); Should be better if added: MODULE_ALIAS("platform:fsl-sai"); This would support module auto-load feature in some Linux-OS. Best regards, Nicolin Chen > diff --git a/sound/soc/fsl/fsl-sai.h b/sound/soc/fsl/fsl-sai.h > new file mode 100644 > index 0000000..1637679 > --- /dev/null > +++ b/sound/soc/fsl/fsl-sai.h > @@ -0,0 +1,120 @@ > +/* > + * Copyright 2012-2013 Freescale Semiconductor, Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#ifndef __FSL_SAI_H > +#define __FSL_SAI_H > + > +#include <sound/dmaengine_pcm.h> > + > +#define FSL_SAI_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |\ > + SNDRV_PCM_FMTBIT_S20_3LE |\ > + SNDRV_PCM_FMTBIT_S24_LE) > + > +/* SAI Transmit/Recieve Control Register */ > +#define FSL_SAI_TCSR 0x00 > +#define FSL_SAI_RCSR 0x80 > +#define FSL_SAI_CSR_TERE BIT(31) > +#define FSL_SAI_CSR_FWF BIT(17) > +#define FSL_SAI_CSR_FRIE BIT(8) > +#define FSL_SAI_CSR_FRDE BIT(0) > + > +/* SAI Transmit Data/FIFO/MASK Register */ > +#define FSL_SAI_TDR 0x20 > +#define FSL_SAI_TFR 0x40 > +#define FSL_SAI_TMR 0x60 > + > +/* SAI Recieve Data/FIFO/MASK Register */ > +#define FSL_SAI_RDR 0xa0 > +#define FSL_SAI_RFR 0xc0 > +#define FSL_SAI_RMR 0xe0 > + > +/* SAI Transmit and Recieve Configuration 1 Register */ > +#define FSL_SAI_TCR1 0x04 > +#define FSL_SAI_RCR1 0x84 > + > +/* SAI Transmit and Recieve Configuration 2 Register */ > +#define FSL_SAI_TCR2 0x08 > +#define FSL_SAI_RCR2 0x88 > +#define FSL_SAI_CR2_SYNC BIT(30) > +#define FSL_SAI_CR2_MSEL_MASK (0xff << 26) > +#define FSL_SAI_CR2_MSEL_BUS 0 > +#define FSL_SAI_CR2_MSEL_MCLK1 BIT(26) > +#define FSL_SAI_CR2_MSEL_MCLK2 BIT(27) > +#define FSL_SAI_CR2_MSEL_MCLK3 (BIT(26) | BIT(27)) > +#define FSL_SAI_CR2_BCP BIT(25) > +#define FSL_SAI_CR2_BCD_MSTR BIT(24) > +#define FSL_SAI_CR2_DIV(x) (x) > +#define FSL_SAI_CR2_DIV_MASK 0xff > + > +/* SAI Transmit and Recieve Configuration 3 Register */ > +#define FSL_SAI_TCR3 0x0c > +#define FSL_SAI_RCR3 0x8c > +#define FSL_SAI_CR3_TRCE BIT(16) > +#define FSL_SAI_CR3_WDFL(x) (x) > +#define FSL_SAI_CR3_WDFL_MASK 0x1f > + > +/* SAI Transmit and Recieve Configuration 4 Register */ > +#define FSL_SAI_TCR4 0x10 > +#define FSL_SAI_RCR4 0x90 > +#define FSL_SAI_CR4_FRSZ(x) (((x) - 1) << 16) > +#define FSL_SAI_CR4_FRSZ_MASK (0x1f << 16) > +#define FSL_SAI_CR4_SYWD(x) (((x) - 1) << 8) > +#define FSL_SAI_CR4_SYWD_MASK (0x1f << 8) > +#define FSL_SAI_CR4_MF BIT(4) > +#define FSL_SAI_CR4_FSE BIT(3) > +#define FSL_SAI_CR4_FSP BIT(1) > +#define FSL_SAI_CR4_FSD_MSTR BIT(0) > + > +/* SAI Transmit and Recieve Configuration 5 Register */ > +#define FSL_SAI_TCR5 0x14 > +#define FSL_SAI_RCR5 0x94 > +#define FSL_SAI_CR5_WNW(x) (((x) - 1) << 24) > +#define FSL_SAI_CR5_WNW_MASK (0x1f << 24) > +#define FSL_SAI_CR5_W0W(x) (((x) - 1) << 16) > +#define FSL_SAI_CR5_W0W_MASK (0x1f << 16) > +#define FSL_SAI_CR5_FBT(x) ((x) << 8) > +#define FSL_SAI_CR5_FBT_MASK (0x1f << 8) > + > +/* SAI audio dividers */ > +#define FSL_SAI_TX_DIV 0 > +#define FSL_SAI_RX_DIV 1 > + > +/* SAI type */ > +#define FSL_SAI_DMA BIT(0) > +#define FSL_SAI_USE_AC97 BIT(1) > +#define FSL_SAI_NET BIT(2) > +#define FSL_SAI_TRA_SYN BIT(3) > +#define FSL_SAI_REC_SYN BIT(4) > +#define FSL_SAI_USE_I2S_SLAVE BIT(5) > + > +#define FSL_FMT_TRANSMITTER 0 > +#define FSL_FMT_RECEIVER 1 > + > +/* SAI clock sources */ > +#define FSL_SAI_CLK_BUS 0 > +#define FSL_SAI_CLK_MAST1 1 > +#define FSL_SAI_CLK_MAST2 2 > +#define FSL_SAI_CLK_MAST3 3 > + > +enum fsl_sai_fbt { > + FSL_SAI_FBT_MSB, > + FSL_SAI_FBT_LSB, > +}; > + > +struct fsl_sai { > + struct clk *clk; > + > + void __iomem *base; > + > + enum fsl_sai_fbt fbt; > + > + struct snd_dmaengine_dai_dma_data dma_params_rx; > + struct snd_dmaengine_dai_dma_data dma_params_tx; > +}; > + > +#endif /* __FSL_SAI_H */ > -- > 1.8.4 >
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.
> > This adds Freescale SAI ASoC Audio support. > > This implementation is only compatible with device tree definition. > > Features: > > o Supports playback/capture > > o Supports 16/20/24 bit PCM > > o Supports 8k - 96k sample rates > > o Supports slave mode only. > > > > Just for curiosity, I found there're quite a bit code actually support > I2S master mode such as set_sysclk() and register configuration to FMT > SND_SOC_DAIFMT_CBS_CFS. > > Is that really "supports slave mode only"? Or just you haven't make it > properly work? > Sorry, this comments is not very consistent with the driver implementation. On VF610 there only supports slave mode. So, I will revise this. > > diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig index > > b7ab71f..9a8851e 100644 > > --- a/sound/soc/fsl/Kconfig > > +++ b/sound/soc/fsl/Kconfig > > @@ -213,3 +213,19 @@ config SND_SOC_IMX_MC13783 > > select SND_SOC_IMX_PCM_DMA > > > > endif # SND_IMX_SOC > > + > > +menuconfig SND_FSL_SOC > > + tristate "SoC Audio for Freescale FSL CPUs" > > + help > > + Say Y or M if you want to add support for codecs attached to > > + the FSL CPUs. > > + > > + This will enable Freeacale SAI, SGT15000 codec. > > + > > +if SND_FSL_SOC > > Why check this here? SAI should be a regular IP module which can be used > by other SoC as well. We would never know if next i.MX SoC won't contain > SAI. > > > + > > +config SND_SOC_FSL_SAI > > + tristate > > + select SND_SOC_GENERIC_DMAENGINE_PCM > > I think you should keep SAI beside SSI/SPDIF directly. > That's right. > > + > > +endif # SND_FSL_SOC > > diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile index > > 8db705b..e5acc03 100644 > > --- a/sound/soc/fsl/Makefile > > +++ b/sound/soc/fsl/Makefile > > @@ -56,3 +56,8 @@ obj-$(CONFIG_SND_SOC_IMX_SGTL5000) += > > snd-soc-imx-sgtl5000.o > > obj-$(CONFIG_SND_SOC_IMX_WM8962) += snd-soc-imx-wm8962.o > > obj-$(CONFIG_SND_SOC_IMX_SPDIF) += snd-soc-imx-spdif.o > > obj-$(CONFIG_SND_SOC_IMX_MC13783) += snd-soc-imx-mc13783.o > > + > > +# FSL ARM SAI/SGT15000 Platform Support > > Should be SGTL5000, not SGT1. And SAI should not be used with SGTL5000 > only, it's a bit confusing to mention SGTL5000 here. > Yes, this will be revised then. > > +snd-soc-fsl-sai-objs := fsl-sai.o > > And I think it should be better to put it along with fsl-ssi.o and fsl- > spdif.o > But fsl-ssi.o and fsl-spdif.o is based PowrePC platform? Which we can see from the comments. While fsl-sai.o is base ARM platform. Despite whether there is any different between ARM and PPC or not, the comments won't be correct. > > + > > +obj-$(CONFIG_SND_SOC_FSL_SAI) += snd-soc-fsl-sai.o > > Ditto > > > diff --git a/sound/soc/fsl/fsl-sai.c b/sound/soc/fsl/fsl-sai.c new > > file mode 100644 index 0000000..bb57e67 > > --- /dev/null > > +++ b/sound/soc/fsl/fsl-sai.c > > @@ -0,0 +1,472 @@ > > +/* > > + * Freescale SAI ALSA SoC Digital Audio Interface driver. > > + * > > + * Copyright 2012-2013 Freescale Semiconductor, Inc. > > + * > > + * This program is free software; you can redistribute it and/or > > +modify it > > + * under the terms of the GNU General Public License as published > > +by the > > + * Free Software Foundation; either version 2 of the License, or > > +(at your > > + * option) any later version. > > There're too many double space inside. Could you pls revise it? > Yes, see the next version. > > + * > > + */ > > + > > +#include <linux/clk.h> > > +#include <linux/module.h> > > +#include <linux/slab.h> > > +#include <linux/of_address.h> > > +#include <sound/core.h> > > +#include <sound/pcm_params.h> > > +#include <linux/delay.h> > > +#include <linux/dmaengine.h> > > +#include <sound/dmaengine_pcm.h> > > I think it's better to keep <sound/xxx.h> together. And basically we can > keep header files ordered by alphabet. > Okey. > > + > > +#include "fsl-sai.h" > > + > > +static int fsl_sai_set_dai_sysclk_tr(struct snd_soc_dai *cpu_dai, > > + int clk_id, unsigned int freq, int fsl_dir) { > > + u32 val_cr2, reg_cr2; > > + struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); > > + > > + if (fsl_dir == FSL_FMT_TRANSMITTER) > > + reg_cr2 = FSL_SAI_TCR2; > > + else > > + reg_cr2 = FSL_SAI_RCR2; > > + > > + val_cr2 = readl(sai->base + reg_cr2); > > + switch (clk_id) { > > + case FSL_SAI_CLK_BUS: > > + val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK; > > + val_cr2 |= FSL_SAI_CR2_MSEL_BUS; > > + break; > > + case FSL_SAI_CLK_MAST1: > > + val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK; > > + val_cr2 |= FSL_SAI_CR2_MSEL_MCLK1; > > + break; > > + case FSL_SAI_CLK_MAST2: > > + val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK; > > + val_cr2 |= FSL_SAI_CR2_MSEL_MCLK2; > > + break; > > + case FSL_SAI_CLK_MAST3: > > + val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK; > > + val_cr2 |= FSL_SAI_CR2_MSEL_MCLK3; > > + break; > > + default: > > + return -EINVAL; > > + } > > + writel(val_cr2, sai->base + reg_cr2); > > + > > + return 0; > > +} > > + > > +static int fsl_sai_set_dai_sysclk(struct snd_soc_dai *cpu_dai, > > + int clk_id, unsigned int freq, int dir) { > > + int ret; > > + > > + if (dir == SND_SOC_CLOCK_IN) > > + return 0; > > + > > + 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); > > + if (ret) { > > + dev_err(cpu_dai->dev, > > + "Cannot set sai's receiver sysclk: %d\n", > > + ret); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +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); > > + > > + } else if (div_id == FSL_SAI_RX_DIV) { > > + rcr2 = readl(sai->base + FSL_SAI_RCR2); > > + rcr2 &= ~FSL_SAI_CR2_DIV_MASK; > > + rcr2 |= FSL_SAI_CR2_DIV(div); > > + writel(rcr2, sai->base + FSL_SAI_RCR2); > > + > > + } else > > + return -EINVAL; > > It would look better if using switch-case. And the last 'else' > could be 'default:'. > I'll think it over. > > + > > + return 0; > > +} > > + > > +static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai *cpu_dai, > > + unsigned int fmt, int fsl_dir) > > +{ > > + u32 val_cr2, val_cr3, val_cr4, reg_cr2, reg_cr3, reg_cr4; > > + struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); > > + > > + if (fsl_dir == FSL_FMT_TRANSMITTER) { > > + reg_cr2 = FSL_SAI_TCR2; > > + reg_cr3 = FSL_SAI_TCR3; > > + reg_cr4 = FSL_SAI_TCR4; > > + } else { > > + reg_cr2 = FSL_SAI_RCR2; > > + reg_cr3 = FSL_SAI_RCR3; > > + reg_cr4 = FSL_SAI_RCR4; > > + } > > + > > + val_cr2 = readl(sai->base + reg_cr2); > > + val_cr3 = readl(sai->base + reg_cr3); > > + val_cr4 = readl(sai->base + reg_cr4); > > + > > + if (sai->fbt == FSL_SAI_FBT_MSB) > > + val_cr4 |= FSL_SAI_CR4_MF; > > + else if (sai->fbt == FSL_SAI_FBT_LSB) > > + val_cr4 &= ~FSL_SAI_CR4_MF; > > + > > + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { > > + case SND_SOC_DAIFMT_I2S: > > + val_cr4 |= FSL_SAI_CR4_FSE; > > + val_cr4 |= FSL_SAI_CR4_FSP; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + switch (fmt & SND_SOC_DAIFMT_INV_MASK) { > > + case SND_SOC_DAIFMT_IB_IF: > > + val_cr4 |= FSL_SAI_CR4_FSP; > > + val_cr2 &= ~FSL_SAI_CR2_BCP; > > + break; > > + case SND_SOC_DAIFMT_IB_NF: > > + val_cr4 &= ~FSL_SAI_CR4_FSP; > > + val_cr2 &= ~FSL_SAI_CR2_BCP; > > + break; > > + case SND_SOC_DAIFMT_NB_IF: > > + val_cr4 |= FSL_SAI_CR4_FSP; > > + val_cr2 |= FSL_SAI_CR2_BCP; > > + break; > > + case SND_SOC_DAIFMT_NB_NF: > > + val_cr4 &= ~FSL_SAI_CR4_FSP; > > + val_cr2 |= FSL_SAI_CR2_BCP; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { > > + case SND_SOC_DAIFMT_CBS_CFS: > > + val_cr2 |= FSL_SAI_CR2_BCD_MSTR; > > + val_cr4 |= FSL_SAI_CR4_FSD_MSTR; > > + break; > > + case SND_SOC_DAIFMT_CBM_CFM: > > + val_cr2 &= ~FSL_SAI_CR2_BCD_MSTR; > > + val_cr4 &= ~FSL_SAI_CR4_FSD_MSTR; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + val_cr3 |= FSL_SAI_CR3_TRCE; > > + > > + if (fsl_dir == FSL_FMT_RECEIVER) > > + val_cr2 |= FSL_SAI_CR2_SYNC; > > + > > + writel(val_cr2, sai->base + reg_cr2); > > + writel(val_cr3, sai->base + reg_cr3); > > + writel(val_cr4, sai->base + reg_cr4); > > + > > + return 0; > > > + > > Pls drop this extra line. > See the next version. > > > +} > > + > > +static int fsl_sai_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned > > +int fmt) { > > + int ret; > > + > > + ret = fsl_sai_set_dai_fmt_tr(cpu_dai, fmt, FSL_FMT_TRANSMITTER); > > + if (ret) { > > + dev_err(cpu_dai->dev, > > + "Cannot set sai's transmitter format: %d\n", > > + ret); > > + return ret; > > + } > > + > > + ret = fsl_sai_set_dai_fmt_tr(cpu_dai, fmt, FSL_FMT_RECEIVER); > > + if (ret) { > > + dev_err(cpu_dai->dev, > > + "Cannot set sai's receiver format: %d\n", > > + ret); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int fsl_sai_set_dai_tdm_slot(struct snd_soc_dai *cpu_dai, > > + unsigned int tx_mask, unsigned int rx_mask, > > + int slots, int slot_width) > > +{ > > + struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); > > + u32 tcr4, rcr4; > > + > > + tcr4 = readl(sai->base + FSL_SAI_TCR4); > > + tcr4 &= ~FSL_SAI_CR4_FRSZ_MASK; > > + tcr4 |= FSL_SAI_CR4_FRSZ(2); > > + writel(tcr4, sai->base + FSL_SAI_TCR4); > > + writel(tx_mask, sai->base + FSL_SAI_TMR); > > + > > + rcr4 = readl(sai->base + FSL_SAI_RCR4); > > + rcr4 &= ~FSL_SAI_CR4_FRSZ_MASK; > > + rcr4 |= FSL_SAI_CR4_FRSZ(2); > > + writel(rcr4, sai->base + FSL_SAI_RCR4); > > + writel(rx_mask, sai->base + FSL_SAI_RMR); > > + > > + return 0; > > +} > > + > > +static int fsl_sai_hw_params(struct snd_pcm_substream *substream, > > + struct snd_pcm_hw_params *params, > > + struct snd_soc_dai *cpu_dai) > > +{ > > + u32 val_cr4, val_cr5, reg_cr4, reg_cr5, word_width; > > + struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); > > + > > + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { > > + reg_cr4 = FSL_SAI_TCR4; > > + reg_cr5 = FSL_SAI_TCR5; > > + } else { > > + reg_cr4 = FSL_SAI_RCR4; > > + reg_cr5 = FSL_SAI_RCR5; > > + } > > + > > + val_cr4 = readl(sai->base + reg_cr4); > > + val_cr4 &= ~FSL_SAI_CR4_SYWD_MASK; > > + > > + val_cr5 = readl(sai->base + reg_cr5); > > + val_cr5 &= ~FSL_SAI_CR5_WNW_MASK; > > + val_cr5 &= ~FSL_SAI_CR5_W0W_MASK; > > + val_cr5 &= ~FSL_SAI_CR5_FBT_MASK; > > + > > + switch (params_format(params)) { > > + case SNDRV_PCM_FORMAT_S16_LE: > > + word_width = 16; > > + break; > > + case SNDRV_PCM_FORMAT_S20_3LE: > > + word_width = 20; > > + break; > > + case SNDRV_PCM_FORMAT_S24_LE: > > + word_width = 24; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + val_cr4 |= FSL_SAI_CR4_SYWD(word_width); > > + val_cr5 |= FSL_SAI_CR5_WNW(word_width); > > + val_cr5 |= FSL_SAI_CR5_W0W(word_width); > > + > > + if (sai->fbt == FSL_SAI_FBT_MSB) > > + val_cr5 |= FSL_SAI_CR5_FBT(word_width - 1); > > + else if (sai->fbt == FSL_SAI_FBT_LSB) > > + val_cr5 |= FSL_SAI_CR5_FBT(0); > > + > > + writel(val_cr4, sai->base + reg_cr4); > > + writel(val_cr5, sai->base + reg_cr5); > > + > > + return 0; > > +} > > + > > +static int fsl_sai_trigger(struct snd_pcm_substream *substream, int > cmd, > > + struct snd_soc_dai *dai) > > +{ > > + struct fsl_sai *sai = snd_soc_dai_get_drvdata(dai); > > + unsigned int tcsr, rcsr; > > + > > + tcsr = readl(sai->base + FSL_SAI_TCSR); > > + rcsr = readl(sai->base + FSL_SAI_RCSR); > > + > > + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { > > + tcsr |= FSL_SAI_CSR_FRDE; > > + rcsr &= ~FSL_SAI_CSR_FRDE; > > + } else { > > + rcsr |= FSL_SAI_CSR_FRDE; > > + tcsr &= ~FSL_SAI_CSR_FRDE; > > + } > > + > > + switch (cmd) { > > + case SNDRV_PCM_TRIGGER_START: > > + case SNDRV_PCM_TRIGGER_RESUME: > > + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: > > + tcsr |= FSL_SAI_CSR_TERE; > > + rcsr |= FSL_SAI_CSR_TERE; > > + writel(rcsr, sai->base + FSL_SAI_RCSR); > > + udelay(10); > > Does SAI really needs this udelay() here? Required by IP's operation flow? > If so, I think it's better to add comments here to explain it. > +++++++++++++++++ Synchronous mode The SAI transmitter and receiver can be configured to operate with synchronous bit clock and frame sync. 1, If the transmitter bit clock and frame sync are to be used by both the transmitter and receiver: ... * It is recommended that the transmitter is the last enabled and the first disabled. 2, If the receiver bit clock and frame sync are to be used by both the transmitter and receiver: ... * It is recommended that the receiver is the last enabled and the first disabled. ------------------ So I think the delay is needed, and I still tunning it. > > + writel(tcsr, sai->base + FSL_SAI_TCSR); > > + break; > > + > > + case SNDRV_PCM_TRIGGER_STOP: > > + case SNDRV_PCM_TRIGGER_SUSPEND: > > + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: > > + if (!(dai->playback_active || dai->capture_active)) { > > + tcsr &= ~FSL_SAI_CSR_TERE; > > + rcsr &= ~FSL_SAI_CSR_TERE; > > + } > > + writel(rcsr, sai->base + FSL_SAI_RCSR); > > + udelay(10); > > + writel(tcsr, sai->base + FSL_SAI_TCSR); > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > + > > +static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = { > > + .set_sysclk = fsl_sai_set_dai_sysclk, > > + .set_clkdiv = fsl_sai_set_dai_clkdiv, > > + .set_fmt = fsl_sai_set_dai_fmt, > > + .set_tdm_slot = fsl_sai_set_dai_tdm_slot, > > + .hw_params = fsl_sai_hw_params, > > + .trigger = fsl_sai_trigger, > > +}; > > + > > +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; > > + > > + writel(0x0, sai->base + FSL_SAI_RCSR); > > + writel(0x0, sai->base + FSL_SAI_TCSR); > > + writel(sai->dma_params_tx.maxburst, sai->base + FSL_SAI_TCR1); > > + writel(sai->dma_params_rx.maxburst, sai->base + FSL_SAI_RCR1); > > + > > + dai->playback_dma_data = &sai->dma_params_tx; > > + dai->capture_dma_data = &sai->dma_params_rx; > > + > > + snd_soc_dai_set_drvdata(dai, sai); > > + > > + return 0; > > +} > > + > > +int fsl_sai_dai_remove(struct snd_soc_dai *dai) > > static > Yes. > > +{ > > + struct fsl_sai *sai = dev_get_drvdata(dai->dev); > > + > > + clk_disable_unprepare(sai->clk); > > + > > + return 0; > > +} > > + > > +static struct snd_soc_dai_driver fsl_sai_dai = { > > + .probe = fsl_sai_dai_probe, > > + .remove = fsl_sai_dai_remove, > > + .playback = { > > + .channels_min = 1, > > + .channels_max = 2, > > + .rates = SNDRV_PCM_RATE_8000_96000, > > + .formats = FSL_SAI_FORMATS, > > + }, > > + .capture = { > > + .channels_min = 1, > > + .channels_max = 2, > > + .rates = SNDRV_PCM_RATE_8000_96000, > > + .formats = FSL_SAI_FORMATS, > > + }, > > + .ops = &fsl_sai_pcm_dai_ops, > > +}; > > + > > +static const struct snd_soc_component_driver fsl_component = { > > + .name = "fsl-sai", > > +}; > > + > > +static int fsl_sai_probe(struct platform_device *pdev) { > > + struct resource *res; > > + struct fsl_sai *sai; > > + int ret = 0; > > + > > + sai = devm_kzalloc(&pdev->dev, sizeof(*sai), GFP_KERNEL); > > + if (!sai) > > + return -ENOMEM; > > + > > + sai->fbt = FSL_SAI_FBT_MSB; > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + sai->base = devm_ioremap_resource(&pdev->dev, res); > > + if (IS_ERR(sai->base)) > > + return PTR_ERR(sai->base); > > + > > + sai->clk = devm_clk_get(&pdev->dev, "sai"); > > + if (IS_ERR(sai->clk)) { > > + dev_err(&pdev->dev, "Cannot get sai's clock: %d\n", ret); > > + return PTR_ERR(sai->clk); > > + } > > + > > + sai->dma_params_rx.addr = res->start + FSL_SAI_RDR; > > + sai->dma_params_rx.maxburst = 6; > > + > > + sai->dma_params_tx.addr = res->start + FSL_SAI_TDR; > > + sai->dma_params_tx.maxburst = 6; > > + > > + platform_set_drvdata(pdev, sai); > > + > > + ret = devm_snd_soc_register_component(&pdev->dev, &fsl_component, > > + &fsl_sai_dai, 1); > > + if (ret) > > + return ret; > > + > > + ret = snd_dmaengine_pcm_register(&pdev->dev, NULL, > > + SND_DMAENGINE_PCM_FLAG_NO_RESIDUE); > > + if (ret) > > + return ret; > > + > > + return 0; > > +} > > + > > +static int fsl_sai_remove(struct platform_device *pdev) { > > + snd_dmaengine_pcm_unregister(&pdev->dev); > > + > > + return 0; > > +} > > + > > +static const struct of_device_id fsl_sai_ids[] = { > > + { .compatible = "fsl,vf610-sai", }, > > > + { /*sentinel*/ } > > I think this could be left in blank without comments inside. > And if you really want to add it, pls add like: /* sentinel */ > ^ ^ Okey. > > +}; > > + > > +static struct platform_driver fsl_sai_driver = { > > + .probe = fsl_sai_probe, > > + .remove = fsl_sai_remove, > > + > > + .driver = { > > + .name = "fsl-sai", > > + .owner = THIS_MODULE, > > + .of_match_table = fsl_sai_ids, > > + }, > > +}; > > +module_platform_driver(fsl_sai_driver); > > + > > +MODULE_AUTHOR("Xiubo Li, <Li.Xiubo@freescale.com>"); > > +MODULE_AUTHOR("Alison Wang, <b18965@freescale.com>"); > > +MODULE_DESCRIPTION("Freescale Soc SAI Interface"); > > +MODULE_LICENSE("GPL"); > > Should be better if added: > MODULE_ALIAS("platform:fsl-sai"); > > This would support module auto-load feature in some Linux-OS. > I will think it over. BRS, Xiubo
On Mon, Nov 04, 2013 at 11:45:10AM +0800, Xiubo Li-B47053 wrote: > > > +snd-soc-fsl-sai-objs := fsl-sai.o > > > > And I think it should be better to put it along with fsl-ssi.o and fsl- > > spdif.o > > > > But fsl-ssi.o and fsl-spdif.o is based PowrePC platform? Which we can see from the comments. No. fsl-ssi.c is compatible to both ARM and PPC. And fsl-spdif.c is currently used on ARM only. So please just put along with them. > > > + case SNDRV_PCM_TRIGGER_START: > > > + case SNDRV_PCM_TRIGGER_RESUME: > > > + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: > > > + tcsr |= FSL_SAI_CSR_TERE; > > > + rcsr |= FSL_SAI_CSR_TERE; > > > + writel(rcsr, sai->base + FSL_SAI_RCSR); > > > + udelay(10); > > > > Does SAI really needs this udelay() here? Required by IP's operation flow? > > If so, I think it's better to add comments here to explain it. > > > +++++++++++++++++ > Synchronous mode > The SAI transmitter and receiver can be configured to operate with synchronous bit clock > and frame sync. > > 1, > If the transmitter bit clock and frame sync are to be used by both the transmitter and > receiver: > ... > * It is recommended that the transmitter is the last enabled and the first disabled. > > 2, > If the receiver bit clock and frame sync are to be used by both the transmitter and > receiver: > ... > * It is recommended that the receiver is the last enabled and the first disabled. > ------------------ > > So I think the delay is needed, and I still tunning it. > The udelay just doesn't make sense to what you are talking about. Does SAI really need 10us delay between two register-updating? We basically use udelay only if the IP hardware actually needs it: some IP needs time to boot itself up after doing software reset for example. But it doesn't look reasonable to me by using udelay to make sure "the last enabled". And from the 'Synchronous mode' you just provided, there're another issue: + case SNDRV_PCM_TRIGGER_START: + case SNDRV_PCM_TRIGGER_RESUME: + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: + tcsr |= FSL_SAI_CSR_TERE; + rcsr |= FSL_SAI_CSR_TERE; + writel(rcsr, sai->base + FSL_SAI_RCSR); + udelay(10); + writel(tcsr, sai->base + FSL_SAI_TCSR); + break; + + case SNDRV_PCM_TRIGGER_STOP: + case SNDRV_PCM_TRIGGER_SUSPEND: + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: + if (!(dai->playback_active || dai->capture_active)) { + tcsr &= ~FSL_SAI_CSR_TERE; + rcsr &= ~FSL_SAI_CSR_TERE; + } + writel(rcsr, sai->base + FSL_SAI_RCSR); + udelay(10); + writel(tcsr, sai->base + FSL_SAI_TCSR); + break; ISSUE 1: You might make sure transmitter is the last enabled. However, it's not the first disabled. Is this okay? ISSUE 2: There are two cases listed in 'Synchronous mode'. However, your driver doesn't take care of them. The SAI's synchronous mode looks like more flexible than SSI's. The driver needs to be more sophisticated so that it can handle multiple cases when TX/RX clocks are controlled by either TX or RX, and surely, the asynchronous mode as well. And there's another personal tip: I think you can first try to focus on this SAI driver and pend the others. There might be two many things you need to refine if you are doing them at the same time. Best regards, Nicolin Chen
> > +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? > This is the bit clock divider. I'll add some comments or rename them to be more readable. From the IP spec: ++++++ Bit Clock Divide Divides down the audio master clock to generate the bit clock when configured for an internal bit clock. ------ From the ASoC subsystem comments we can see that: ++++++ Configures the clock dividers. This is used to derive the best DAI bit and frame clocks from the system or master clock. It's best to set the DAI bit and frame clocks as low as possible to save system power. ------ > > +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. > While if the module clock is not enabled here, the followed registers cannot read/write in the same function. And this _probe function is the _dai_probe not the driver's module _probe. If the clk_prepare_enable(sai->clk) is not here, where should it be will be nicer ? One of the following functions ? .set_sysclk = fsl_sai_set_dai_sysclk, .set_clkdiv = fsl_sai_set_dai_clkdiv, .set_fmt = fsl_sai_set_dai_fmt, .set_tdm_slot = fsl_sai_set_dai_tdm_slot, .hw_params = fsl_sai_hw_params, .trigger = fsl_sai_trigger, > > + 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. Sorry, is there one patch for adding the devm_ version of snd_dmaengine_pcm_register() already ? In the -next and other topics branches I could not find it. Best Regards, Xiubo
On Mon, Nov 04, 2013 at 07:35:12AM +0000, Li Xiubo wrote: > From the ASoC subsystem comments we can see that: > ++++++ > Configures the clock dividers. This is used to derive the best DAI bit and > frame clocks from the system or master clock. It's best to set the DAI bit > and frame clocks as low as possible to save system power. > ------ You should never use this unless you have to, there is no point in every single machine driver using your driver having to duplicate the same calculations. > > > > +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. > While if the module clock is not enabled here, the followed registers cannot read/write in the same function. > And this _probe function is the _dai_probe not the driver's module _probe. So you can enable the clock when you explicitly need to write to the registers... > If the clk_prepare_enable(sai->clk) is not here, where should it be will be nicer ? > One of the following functions ? > .set_sysclk = fsl_sai_set_dai_sysclk, > .set_clkdiv = fsl_sai_set_dai_clkdiv, > .set_fmt = fsl_sai_set_dai_fmt, > .set_tdm_slot = fsl_sai_set_dai_tdm_slot, > .hw_params = fsl_sai_hw_params, > .trigger = fsl_sai_trigger, It could be in any or all of them except trigger (where the core should hold a runtime PM reference anyway). You can always take a reference for the duration of the function if you're concerned it may be called when the referent isn't otherwise held. > > > + 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. > Sorry, is there one patch for adding the devm_ version of snd_dmaengine_pcm_register() already ? > In the -next and other topics branches I could not find it. No, there isn't one but there should be one.
> > From the ASoC subsystem comments we can see that: > > ++++++ > > Configures the clock dividers. This is used to derive the best DAI bit > > and frame clocks from the system or master clock. It's best to set the > > DAI bit and frame clocks as low as possible to save system power. > > ------ > > You should never use this unless you have to, there is no point in every > single machine driver using your driver having to duplicate the same > calculations. > Okey, I'll think it over, if not have to, I will revise this. > > > > > > +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. > > > While if the module clock is not enabled here, the followed registers > cannot read/write in the same function. > > And this _probe function is the _dai_probe not the driver's module > _probe. > > So you can enable the clock when you explicitly need to write to the > registers... > > > If the clk_prepare_enable(sai->clk) is not here, where should it be > will be nicer ? > > One of the following functions ? > > .set_sysclk = fsl_sai_set_dai_sysclk, > > .set_clkdiv = fsl_sai_set_dai_clkdiv, > > .set_fmt = fsl_sai_set_dai_fmt, > > .set_tdm_slot = fsl_sai_set_dai_tdm_slot, > > .hw_params = fsl_sai_hw_params, > > .trigger = fsl_sai_trigger, > > It could be in any or all of them except trigger (where the core should > hold a runtime PM reference anyway). You can always take a reference for > the duration of the function if you're concerned it may be called when > the referent isn't otherwise held. > While in this _sai_dai_probe function just followed the clock enable sentence, there are some register writing operations: The PATCH: +++++++++++++++++++++++ +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); =====> clock enable here + if (ret) + return ret; + + writel(0x0, sai->base + FSL_SAI_RCSR); ======>registers writing here. + writel(0x0, sai->base + FSL_SAI_TCSR); ======> and here + writel(sai->dma_params_tx.maxburst, sai->base + FSL_SAI_TCR1); =======>and here + writel(sai->dma_params_rx.maxburst, sai->base + FSL_SAI_RCR1); =======>and here + + dai->playback_dma_data = &sai->dma_params_tx; + dai->capture_dma_data = &sai->dma_params_rx; + + snd_soc_dai_set_drvdata(dai, sai); + + return 0; +} ------------------------- As your opinions, should I move the four register writing operations to .set_sysclk/set_clkdiv/... functions too ? Or just add a clk_disable_unprepare() after them here, and then add clk_prepare_enable in one of .set_sysclk/set_clkdiv/...? And the first two of this four registers must be initialize as early as possible, and if move them to one of the .set_sysclk/set_clkdiv/... functions, How can I very ensure which one is the first to be called ? Won't the calling sequence be changed in the feature ? From the debug logs, we can see that: 1, _sai_probe This is called when the machine brings up and has one SAI device. 2, _sai_dai_probe 3, .set_sysclk 4, .set_fmt Are called in order when the machine has Audio driver and is enabled, and also while the machine brings up. The above four steps only be called one time in order. When aplay/arecord is runs the following will be called in order: 5, .set_tdm_slot 6, .hw_param 7, .trigger -->begain 8, .trigger --> end The 2,3,4 are always called almost the same time, and they are all have register read/write operations. Now the clk_prepare_enable() is in step 2, and it won't be any different moving to step 3 or 4. So, only could move it to step 5 or 6, if so every time the aplay/arecord runs, clk_prepare_enable() will be called, and there has no chance to call clk_disable_unprepare(). Now from the code we can see that I have add clk_prepare_enable() in _sai_dai_probe() and clk_disable_unprepare() in _sai_dai_remove(). Isn't this okey ? > > > > + 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. > > > Sorry, is there one patch for adding the devm_ version of > snd_dmaengine_pcm_register() already ? > > In the -next and other topics branches I could not find it. > > No, there isn't one but there should be one. > And if it has existed then I will use it. BRs, Xiubo
Li Xiubo wrote:
> But fsl-ssi.o and fsl-spdif.o is based PowrePC platform? Which we can see from the comments.
fsl_ssi was originally PPC-only, but it now supports PPC and ARM. You
can see that from the git history.
If there are any comments that say PPC but are not PPC-specific, that
should be fixed.
> > But fsl-ssi.o and fsl-spdif.o is based PowrePC platform? Which we can > see from the comments. > > fsl_ssi was originally PPC-only, but it now supports PPC and ARM. You > can see that from the git history. > > If there are any comments that say PPC but are not PPC-specific, that > should be fixed. Yes, find it. The comments is in "sound/soc/fsl/Makefile" : +++++++++++ "# Freescale PowerPC SSI/DMA Platform Support" ----------- But fsl-spdif.o is also under it. And this is also support ARM and PowerPC platforms at the same time ? If so, the comments should be modified to : +++++++++++ "# Freescale PowerPC and ARM SSI/DMA Platform Support" ----------- Best Regards, Xiubo
Li Xiubo wrote: >> If there are any comments that say PPC but are not PPC-specific, that >> >should be fixed. > Yes, find it. > > The comments is in "sound/soc/fsl/Makefile" : > +++++++++++ > "# Freescale PowerPC SSI/DMA Platform Support" > ----------- > > But fsl-spdif.o is also under it. > And this is also support ARM and PowerPC platforms at the same time ? > If so, the comments should be modified to : > +++++++++++ > "# Freescale PowerPC and ARM SSI/DMA Platform Support" > ----------- Yes, this should be changed.
> >> If there are any comments that say PPC but are not PPC-specific, that > >> >should be fixed. > > Yes, find it. > > > > The comments is in "sound/soc/fsl/Makefile" : > > +++++++++++ > > "# Freescale PowerPC SSI/DMA Platform Support" > > ----------- > > > > But fsl-spdif.o is also under it. > > And this is also support ARM and PowerPC platforms at the same time ? > > If so, the comments should be modified to : > > +++++++++++ > > "# Freescale PowerPC and ARM SSI/DMA Platform Support" > > ----------- > > Yes, this should be changed. > How about : +++++++++ "# Freescale PowerPC and ARM SSI/DMA/SAI/SPDIF Platform Support" --------- ?
On Wed, Nov 06, 2013 at 03:53:24AM +0000, Li Xiubo wrote: > > >> If there are any comments that say PPC but are not PPC-specific, that > > >> >should be fixed. > > > Yes, find it. > > > > > > The comments is in "sound/soc/fsl/Makefile" : > > > +++++++++++ > > > "# Freescale PowerPC SSI/DMA Platform Support" > > > ----------- > > > > > > But fsl-spdif.o is also under it. > > > And this is also support ARM and PowerPC platforms at the same time ? > > > If so, the comments should be modified to : > > > +++++++++++ > > > "# Freescale PowerPC and ARM SSI/DMA Platform Support" > > > ----------- > > > > Yes, this should be changed. > > > > How about : > +++++++++ > "# Freescale PowerPC and ARM SSI/DMA/SAI/SPDIF Platform Support" > --------- > ? Or we can just drop 'PowerPC and ARM'? Shawn
> > > >> If there are any comments that say PPC but are not PPC-specific, > that > > > >> >should be fixed. > > > > Yes, find it. > > > > > > > > The comments is in "sound/soc/fsl/Makefile" : > > > > +++++++++++ > > > > "# Freescale PowerPC SSI/DMA Platform Support" > > > > ----------- > > > > > > > > But fsl-spdif.o is also under it. > > > > And this is also support ARM and PowerPC platforms at the same > time ? > > > > If so, the comments should be modified to : > > > > +++++++++++ > > > > "# Freescale PowerPC and ARM SSI/DMA Platform Support" > > > > ----------- > > > > > > Yes, this should be changed. > > > > > > > How about : > > +++++++++ > > "# Freescale PowerPC and ARM SSI/DMA/SAI/SPDIF Platform Support" > > --------- > > ? > > Or we can just drop 'PowerPC and ARM'? > Yes, this will be better. Best Regards, Xiubo
On Tue, Nov 05, 2013 at 03:21:49AM +0000, Li Xiubo wrote: > As your opinions, should I move the four register writing operations to .set_sysclk/set_clkdiv/... functions too ? > Or just add a clk_disable_unprepare() after them here, and then add clk_prepare_enable in one of .set_sysclk/set_clkdiv/...? The latter.
> The udelay just doesn't make sense to what you are talking about. > > Does SAI really need 10us delay between two register-updating? > No, this is not must be. > We basically use udelay only if the IP hardware actually needs it: some > IP needs time to boot itself up after doing software reset for example. > But it doesn't look reasonable to me by using udelay to make sure "the > last enabled". > > And from the 'Synchronous mode' you just provided, there're another issue: > > + case SNDRV_PCM_TRIGGER_START: > + case SNDRV_PCM_TRIGGER_RESUME: > + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: > + tcsr |= FSL_SAI_CSR_TERE; > + rcsr |= FSL_SAI_CSR_TERE; > + writel(rcsr, sai->base + FSL_SAI_RCSR); > + udelay(10); > + writel(tcsr, sai->base + FSL_SAI_TCSR); > + break; > + > + case SNDRV_PCM_TRIGGER_STOP: > + case SNDRV_PCM_TRIGGER_SUSPEND: > + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: > + if (!(dai->playback_active || dai->capture_active)) { > + tcsr &= ~FSL_SAI_CSR_TERE; > + rcsr &= ~FSL_SAI_CSR_TERE; > + } > + writel(rcsr, sai->base + FSL_SAI_RCSR); > + udelay(10); > + writel(tcsr, sai->base + FSL_SAI_TCSR); > + break; > > ISSUE 1: You might make sure transmitter is the last enabled. > However, it's not the first disabled. Is this okay? > Yes, this is just programming mistake. I'll revise it. In this case the transmitter should be the last enabled and the first disabled. > ISSUE 2: There are two cases listed in 'Synchronous mode'. > However, your driver doesn't take care of them. > The SAI's synchronous mode looks like more flexible > than SSI's. The driver needs to be more sophisticated > so that it can handle multiple cases when TX/RX clocks > are controlled by either TX or RX, and surely, the > asynchronous mode as well. > Because in Vybrid the transmitter bit clock and frame sync are to be used by both the transmitter and receiver, and only this case can be used here, so now I only handle this case. > > And there's another personal tip: I think you can first try to focus on > this SAI driver and pend the others. There might be two many things you > need to refine if you are doing them at the same time. > I'll implement them later if needed. -- Best Regards, Xiubo
On Wed, Nov 20, 2013 at 11:37:45AM +0800, Xiubo Li-B47053 wrote: > > > The udelay just doesn't make sense to what you are talking about. > > > > Does SAI really need 10us delay between two register-updating? > > > > No, this is not must be. Then you should explain in your comments why you really put it here or just drop it if it's just a mistake. > > We basically use udelay only if the IP hardware actually needs it: some > > IP needs time to boot itself up after doing software reset for example. > > But it doesn't look reasonable to me by using udelay to make sure "the > > last enabled". > > > > And from the 'Synchronous mode' you just provided, there're another issue: > > > > + case SNDRV_PCM_TRIGGER_START: > > + case SNDRV_PCM_TRIGGER_RESUME: > > + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: > > + tcsr |= FSL_SAI_CSR_TERE; > > + rcsr |= FSL_SAI_CSR_TERE; > > + writel(rcsr, sai->base + FSL_SAI_RCSR); > > + udelay(10); > > + writel(tcsr, sai->base + FSL_SAI_TCSR); > > + break; > > + > > + case SNDRV_PCM_TRIGGER_STOP: > > + case SNDRV_PCM_TRIGGER_SUSPEND: > > + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: > > + if (!(dai->playback_active || dai->capture_active)) { > > + tcsr &= ~FSL_SAI_CSR_TERE; > > + rcsr &= ~FSL_SAI_CSR_TERE; > > + } > > + writel(rcsr, sai->base + FSL_SAI_RCSR); > > + udelay(10); > > + writel(tcsr, sai->base + FSL_SAI_TCSR); > > + break; > > > > ISSUE 1: You might make sure transmitter is the last enabled. > > However, it's not the first disabled. Is this okay? > > > > Yes, this is just programming mistake. I'll revise it. > > In this case the transmitter should be the last enabled and the first disabled. > > > > ISSUE 2: There are two cases listed in 'Synchronous mode'. > > However, your driver doesn't take care of them. > > The SAI's synchronous mode looks like more flexible > > than SSI's. The driver needs to be more sophisticated > > so that it can handle multiple cases when TX/RX clocks > > are controlled by either TX or RX, and surely, the > > asynchronous mode as well. > > > > Because in Vybrid the transmitter bit clock and frame sync are to be used by > both the transmitter and receiver, and only this case can be used here, so > now I only handle this case. It's fairly okay if adding explicit comments to indicate that currently the driver only supports its Synchronous mode with clocks controlled by TX only. Best, Nicolin Chen > > > > And there's another personal tip: I think you can first try to focus on > > this SAI driver and pend the others. There might be two many things you > > need to refine if you are doing them at the same time. > > > > I'll implement them later if needed. > > > -- > Best Regards, > Xiubo >
> > > The udelay just doesn't make sense to what you are talking about. > > > > > > Does SAI really need 10us delay between two register-updating? > > > > > > > No, this is not must be. > > Then you should explain in your comments why you really put it here or > just drop it if it's just a mistake. > The udelay will be removed then. > > > ISSUE 2: There are two cases listed in 'Synchronous mode'. > > > However, your driver doesn't take care of them. > > > The SAI's synchronous mode looks like more flexible > > > than SSI's. The driver needs to be more sophisticated > > > so that it can handle multiple cases when TX/RX clocks > > > are controlled by either TX or RX, and surely, the > > > asynchronous mode as well. > > > > > > > Because in Vybrid the transmitter bit clock and frame sync are to be > > used by both the transmitter and receiver, and only this case can be > > used here, so now I only handle this case. > > It's fairly okay if adding explicit comments to indicate that currently > the driver only supports its Synchronous mode with clocks controlled by > TX only. > Just think, on other platforms maybe only the Rx's clock is available. Thus I think there should be one DT property to control this, and then the SAI driver can be more flexible. Or could you give me some more practical ideas ? -- Best Regards, Xiubo
diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig index b7ab71f..9a8851e 100644 --- a/sound/soc/fsl/Kconfig +++ b/sound/soc/fsl/Kconfig @@ -213,3 +213,19 @@ config SND_SOC_IMX_MC13783 select SND_SOC_IMX_PCM_DMA endif # SND_IMX_SOC + +menuconfig SND_FSL_SOC + tristate "SoC Audio for Freescale FSL CPUs" + help + Say Y or M if you want to add support for codecs attached to + the FSL CPUs. + + This will enable Freeacale SAI, SGT15000 codec. + +if SND_FSL_SOC + +config SND_SOC_FSL_SAI + tristate + select SND_SOC_GENERIC_DMAENGINE_PCM + +endif # SND_FSL_SOC diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile index 8db705b..e5acc03 100644 --- a/sound/soc/fsl/Makefile +++ b/sound/soc/fsl/Makefile @@ -56,3 +56,8 @@ obj-$(CONFIG_SND_SOC_IMX_SGTL5000) += snd-soc-imx-sgtl5000.o obj-$(CONFIG_SND_SOC_IMX_WM8962) += snd-soc-imx-wm8962.o obj-$(CONFIG_SND_SOC_IMX_SPDIF) += snd-soc-imx-spdif.o obj-$(CONFIG_SND_SOC_IMX_MC13783) += snd-soc-imx-mc13783.o + +# FSL ARM SAI/SGT15000 Platform Support +snd-soc-fsl-sai-objs := fsl-sai.o + +obj-$(CONFIG_SND_SOC_FSL_SAI) += snd-soc-fsl-sai.o diff --git a/sound/soc/fsl/fsl-sai.c b/sound/soc/fsl/fsl-sai.c new file mode 100644 index 0000000..bb57e67 --- /dev/null +++ b/sound/soc/fsl/fsl-sai.c @@ -0,0 +1,472 @@ +/* + * Freescale SAI ALSA SoC Digital Audio Interface driver. + * + * Copyright 2012-2013 Freescale Semiconductor, Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + * + */ + +#include <linux/clk.h> +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/of_address.h> +#include <sound/core.h> +#include <sound/pcm_params.h> +#include <linux/delay.h> +#include <linux/dmaengine.h> +#include <sound/dmaengine_pcm.h> + +#include "fsl-sai.h" + +static int fsl_sai_set_dai_sysclk_tr(struct snd_soc_dai *cpu_dai, + int clk_id, unsigned int freq, int fsl_dir) +{ + u32 val_cr2, reg_cr2; + struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); + + if (fsl_dir == FSL_FMT_TRANSMITTER) + reg_cr2 = FSL_SAI_TCR2; + else + reg_cr2 = FSL_SAI_RCR2; + + val_cr2 = readl(sai->base + reg_cr2); + switch (clk_id) { + case FSL_SAI_CLK_BUS: + val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK; + val_cr2 |= FSL_SAI_CR2_MSEL_BUS; + break; + case FSL_SAI_CLK_MAST1: + val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK; + val_cr2 |= FSL_SAI_CR2_MSEL_MCLK1; + break; + case FSL_SAI_CLK_MAST2: + val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK; + val_cr2 |= FSL_SAI_CR2_MSEL_MCLK2; + break; + case FSL_SAI_CLK_MAST3: + val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK; + val_cr2 |= FSL_SAI_CR2_MSEL_MCLK3; + break; + default: + return -EINVAL; + } + writel(val_cr2, sai->base + reg_cr2); + + return 0; +} + +static int fsl_sai_set_dai_sysclk(struct snd_soc_dai *cpu_dai, + int clk_id, unsigned int freq, int dir) +{ + int ret; + + if (dir == SND_SOC_CLOCK_IN) + return 0; + + 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); + if (ret) { + dev_err(cpu_dai->dev, + "Cannot set sai's receiver sysclk: %d\n", + ret); + return ret; + } + + return 0; +} + +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); + + } else if (div_id == FSL_SAI_RX_DIV) { + rcr2 = readl(sai->base + FSL_SAI_RCR2); + rcr2 &= ~FSL_SAI_CR2_DIV_MASK; + rcr2 |= FSL_SAI_CR2_DIV(div); + writel(rcr2, sai->base + FSL_SAI_RCR2); + + } else + return -EINVAL; + + return 0; +} + +static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai *cpu_dai, + unsigned int fmt, int fsl_dir) +{ + u32 val_cr2, val_cr3, val_cr4, reg_cr2, reg_cr3, reg_cr4; + struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); + + if (fsl_dir == FSL_FMT_TRANSMITTER) { + reg_cr2 = FSL_SAI_TCR2; + reg_cr3 = FSL_SAI_TCR3; + reg_cr4 = FSL_SAI_TCR4; + } else { + reg_cr2 = FSL_SAI_RCR2; + reg_cr3 = FSL_SAI_RCR3; + reg_cr4 = FSL_SAI_RCR4; + } + + val_cr2 = readl(sai->base + reg_cr2); + val_cr3 = readl(sai->base + reg_cr3); + val_cr4 = readl(sai->base + reg_cr4); + + if (sai->fbt == FSL_SAI_FBT_MSB) + val_cr4 |= FSL_SAI_CR4_MF; + else if (sai->fbt == FSL_SAI_FBT_LSB) + val_cr4 &= ~FSL_SAI_CR4_MF; + + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { + case SND_SOC_DAIFMT_I2S: + val_cr4 |= FSL_SAI_CR4_FSE; + val_cr4 |= FSL_SAI_CR4_FSP; + break; + default: + return -EINVAL; + } + + switch (fmt & SND_SOC_DAIFMT_INV_MASK) { + case SND_SOC_DAIFMT_IB_IF: + val_cr4 |= FSL_SAI_CR4_FSP; + val_cr2 &= ~FSL_SAI_CR2_BCP; + break; + case SND_SOC_DAIFMT_IB_NF: + val_cr4 &= ~FSL_SAI_CR4_FSP; + val_cr2 &= ~FSL_SAI_CR2_BCP; + break; + case SND_SOC_DAIFMT_NB_IF: + val_cr4 |= FSL_SAI_CR4_FSP; + val_cr2 |= FSL_SAI_CR2_BCP; + break; + case SND_SOC_DAIFMT_NB_NF: + val_cr4 &= ~FSL_SAI_CR4_FSP; + val_cr2 |= FSL_SAI_CR2_BCP; + break; + default: + return -EINVAL; + } + + switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { + case SND_SOC_DAIFMT_CBS_CFS: + val_cr2 |= FSL_SAI_CR2_BCD_MSTR; + val_cr4 |= FSL_SAI_CR4_FSD_MSTR; + break; + case SND_SOC_DAIFMT_CBM_CFM: + val_cr2 &= ~FSL_SAI_CR2_BCD_MSTR; + val_cr4 &= ~FSL_SAI_CR4_FSD_MSTR; + break; + default: + return -EINVAL; + } + + val_cr3 |= FSL_SAI_CR3_TRCE; + + if (fsl_dir == FSL_FMT_RECEIVER) + val_cr2 |= FSL_SAI_CR2_SYNC; + + writel(val_cr2, sai->base + reg_cr2); + writel(val_cr3, sai->base + reg_cr3); + writel(val_cr4, sai->base + reg_cr4); + + return 0; + +} + +static int fsl_sai_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) +{ + int ret; + + ret = fsl_sai_set_dai_fmt_tr(cpu_dai, fmt, FSL_FMT_TRANSMITTER); + if (ret) { + dev_err(cpu_dai->dev, + "Cannot set sai's transmitter format: %d\n", + ret); + return ret; + } + + ret = fsl_sai_set_dai_fmt_tr(cpu_dai, fmt, FSL_FMT_RECEIVER); + if (ret) { + dev_err(cpu_dai->dev, + "Cannot set sai's receiver format: %d\n", + ret); + return ret; + } + + return 0; +} + +static int fsl_sai_set_dai_tdm_slot(struct snd_soc_dai *cpu_dai, + unsigned int tx_mask, unsigned int rx_mask, + int slots, int slot_width) +{ + struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); + u32 tcr4, rcr4; + + tcr4 = readl(sai->base + FSL_SAI_TCR4); + tcr4 &= ~FSL_SAI_CR4_FRSZ_MASK; + tcr4 |= FSL_SAI_CR4_FRSZ(2); + writel(tcr4, sai->base + FSL_SAI_TCR4); + writel(tx_mask, sai->base + FSL_SAI_TMR); + + rcr4 = readl(sai->base + FSL_SAI_RCR4); + rcr4 &= ~FSL_SAI_CR4_FRSZ_MASK; + rcr4 |= FSL_SAI_CR4_FRSZ(2); + writel(rcr4, sai->base + FSL_SAI_RCR4); + writel(rx_mask, sai->base + FSL_SAI_RMR); + + return 0; +} + +static int fsl_sai_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *cpu_dai) +{ + u32 val_cr4, val_cr5, reg_cr4, reg_cr5, word_width; + struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); + + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { + reg_cr4 = FSL_SAI_TCR4; + reg_cr5 = FSL_SAI_TCR5; + } else { + reg_cr4 = FSL_SAI_RCR4; + reg_cr5 = FSL_SAI_RCR5; + } + + val_cr4 = readl(sai->base + reg_cr4); + val_cr4 &= ~FSL_SAI_CR4_SYWD_MASK; + + val_cr5 = readl(sai->base + reg_cr5); + val_cr5 &= ~FSL_SAI_CR5_WNW_MASK; + val_cr5 &= ~FSL_SAI_CR5_W0W_MASK; + val_cr5 &= ~FSL_SAI_CR5_FBT_MASK; + + switch (params_format(params)) { + case SNDRV_PCM_FORMAT_S16_LE: + word_width = 16; + break; + case SNDRV_PCM_FORMAT_S20_3LE: + word_width = 20; + break; + case SNDRV_PCM_FORMAT_S24_LE: + word_width = 24; + break; + default: + return -EINVAL; + } + + val_cr4 |= FSL_SAI_CR4_SYWD(word_width); + val_cr5 |= FSL_SAI_CR5_WNW(word_width); + val_cr5 |= FSL_SAI_CR5_W0W(word_width); + + if (sai->fbt == FSL_SAI_FBT_MSB) + val_cr5 |= FSL_SAI_CR5_FBT(word_width - 1); + else if (sai->fbt == FSL_SAI_FBT_LSB) + val_cr5 |= FSL_SAI_CR5_FBT(0); + + writel(val_cr4, sai->base + reg_cr4); + writel(val_cr5, sai->base + reg_cr5); + + return 0; +} + +static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, + struct snd_soc_dai *dai) +{ + struct fsl_sai *sai = snd_soc_dai_get_drvdata(dai); + unsigned int tcsr, rcsr; + + tcsr = readl(sai->base + FSL_SAI_TCSR); + rcsr = readl(sai->base + FSL_SAI_RCSR); + + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { + tcsr |= FSL_SAI_CSR_FRDE; + rcsr &= ~FSL_SAI_CSR_FRDE; + } else { + rcsr |= FSL_SAI_CSR_FRDE; + tcsr &= ~FSL_SAI_CSR_FRDE; + } + + switch (cmd) { + case SNDRV_PCM_TRIGGER_START: + case SNDRV_PCM_TRIGGER_RESUME: + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: + tcsr |= FSL_SAI_CSR_TERE; + rcsr |= FSL_SAI_CSR_TERE; + writel(rcsr, sai->base + FSL_SAI_RCSR); + udelay(10); + writel(tcsr, sai->base + FSL_SAI_TCSR); + break; + + case SNDRV_PCM_TRIGGER_STOP: + case SNDRV_PCM_TRIGGER_SUSPEND: + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: + if (!(dai->playback_active || dai->capture_active)) { + tcsr &= ~FSL_SAI_CSR_TERE; + rcsr &= ~FSL_SAI_CSR_TERE; + } + writel(rcsr, sai->base + FSL_SAI_RCSR); + udelay(10); + writel(tcsr, sai->base + FSL_SAI_TCSR); + break; + default: + return -EINVAL; + } + + return 0; +} + +static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = { + .set_sysclk = fsl_sai_set_dai_sysclk, + .set_clkdiv = fsl_sai_set_dai_clkdiv, + .set_fmt = fsl_sai_set_dai_fmt, + .set_tdm_slot = fsl_sai_set_dai_tdm_slot, + .hw_params = fsl_sai_hw_params, + .trigger = fsl_sai_trigger, +}; + +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; + + writel(0x0, sai->base + FSL_SAI_RCSR); + writel(0x0, sai->base + FSL_SAI_TCSR); + writel(sai->dma_params_tx.maxburst, sai->base + FSL_SAI_TCR1); + writel(sai->dma_params_rx.maxburst, sai->base + FSL_SAI_RCR1); + + dai->playback_dma_data = &sai->dma_params_tx; + dai->capture_dma_data = &sai->dma_params_rx; + + snd_soc_dai_set_drvdata(dai, sai); + + return 0; +} + +int fsl_sai_dai_remove(struct snd_soc_dai *dai) +{ + struct fsl_sai *sai = dev_get_drvdata(dai->dev); + + clk_disable_unprepare(sai->clk); + + return 0; +} + +static struct snd_soc_dai_driver fsl_sai_dai = { + .probe = fsl_sai_dai_probe, + .remove = fsl_sai_dai_remove, + .playback = { + .channels_min = 1, + .channels_max = 2, + .rates = SNDRV_PCM_RATE_8000_96000, + .formats = FSL_SAI_FORMATS, + }, + .capture = { + .channels_min = 1, + .channels_max = 2, + .rates = SNDRV_PCM_RATE_8000_96000, + .formats = FSL_SAI_FORMATS, + }, + .ops = &fsl_sai_pcm_dai_ops, +}; + +static const struct snd_soc_component_driver fsl_component = { + .name = "fsl-sai", +}; + +static int fsl_sai_probe(struct platform_device *pdev) +{ + struct resource *res; + struct fsl_sai *sai; + int ret = 0; + + sai = devm_kzalloc(&pdev->dev, sizeof(*sai), GFP_KERNEL); + if (!sai) + return -ENOMEM; + + sai->fbt = FSL_SAI_FBT_MSB; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + sai->base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(sai->base)) + return PTR_ERR(sai->base); + + sai->clk = devm_clk_get(&pdev->dev, "sai"); + if (IS_ERR(sai->clk)) { + dev_err(&pdev->dev, "Cannot get sai's clock: %d\n", ret); + return PTR_ERR(sai->clk); + } + + sai->dma_params_rx.addr = res->start + FSL_SAI_RDR; + sai->dma_params_rx.maxburst = 6; + + sai->dma_params_tx.addr = res->start + FSL_SAI_TDR; + sai->dma_params_tx.maxburst = 6; + + platform_set_drvdata(pdev, sai); + + ret = devm_snd_soc_register_component(&pdev->dev, &fsl_component, + &fsl_sai_dai, 1); + if (ret) + return ret; + + ret = snd_dmaengine_pcm_register(&pdev->dev, NULL, + SND_DMAENGINE_PCM_FLAG_NO_RESIDUE); + if (ret) + return ret; + + return 0; +} + +static int fsl_sai_remove(struct platform_device *pdev) +{ + snd_dmaengine_pcm_unregister(&pdev->dev); + + return 0; +} + +static const struct of_device_id fsl_sai_ids[] = { + { .compatible = "fsl,vf610-sai", }, + { /*sentinel*/ } +}; + +static struct platform_driver fsl_sai_driver = { + .probe = fsl_sai_probe, + .remove = fsl_sai_remove, + + .driver = { + .name = "fsl-sai", + .owner = THIS_MODULE, + .of_match_table = fsl_sai_ids, + }, +}; +module_platform_driver(fsl_sai_driver); + +MODULE_AUTHOR("Xiubo Li, <Li.Xiubo@freescale.com>"); +MODULE_AUTHOR("Alison Wang, <b18965@freescale.com>"); +MODULE_DESCRIPTION("Freescale Soc SAI Interface"); +MODULE_LICENSE("GPL"); diff --git a/sound/soc/fsl/fsl-sai.h b/sound/soc/fsl/fsl-sai.h new file mode 100644 index 0000000..1637679 --- /dev/null +++ b/sound/soc/fsl/fsl-sai.h @@ -0,0 +1,120 @@ +/* + * Copyright 2012-2013 Freescale Semiconductor, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#ifndef __FSL_SAI_H +#define __FSL_SAI_H + +#include <sound/dmaengine_pcm.h> + +#define FSL_SAI_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |\ + SNDRV_PCM_FMTBIT_S20_3LE |\ + SNDRV_PCM_FMTBIT_S24_LE) + +/* SAI Transmit/Recieve Control Register */ +#define FSL_SAI_TCSR 0x00 +#define FSL_SAI_RCSR 0x80 +#define FSL_SAI_CSR_TERE BIT(31) +#define FSL_SAI_CSR_FWF BIT(17) +#define FSL_SAI_CSR_FRIE BIT(8) +#define FSL_SAI_CSR_FRDE BIT(0) + +/* SAI Transmit Data/FIFO/MASK Register */ +#define FSL_SAI_TDR 0x20 +#define FSL_SAI_TFR 0x40 +#define FSL_SAI_TMR 0x60 + +/* SAI Recieve Data/FIFO/MASK Register */ +#define FSL_SAI_RDR 0xa0 +#define FSL_SAI_RFR 0xc0 +#define FSL_SAI_RMR 0xe0 + +/* SAI Transmit and Recieve Configuration 1 Register */ +#define FSL_SAI_TCR1 0x04 +#define FSL_SAI_RCR1 0x84 + +/* SAI Transmit and Recieve Configuration 2 Register */ +#define FSL_SAI_TCR2 0x08 +#define FSL_SAI_RCR2 0x88 +#define FSL_SAI_CR2_SYNC BIT(30) +#define FSL_SAI_CR2_MSEL_MASK (0xff << 26) +#define FSL_SAI_CR2_MSEL_BUS 0 +#define FSL_SAI_CR2_MSEL_MCLK1 BIT(26) +#define FSL_SAI_CR2_MSEL_MCLK2 BIT(27) +#define FSL_SAI_CR2_MSEL_MCLK3 (BIT(26) | BIT(27)) +#define FSL_SAI_CR2_BCP BIT(25) +#define FSL_SAI_CR2_BCD_MSTR BIT(24) +#define FSL_SAI_CR2_DIV(x) (x) +#define FSL_SAI_CR2_DIV_MASK 0xff + +/* SAI Transmit and Recieve Configuration 3 Register */ +#define FSL_SAI_TCR3 0x0c +#define FSL_SAI_RCR3 0x8c +#define FSL_SAI_CR3_TRCE BIT(16) +#define FSL_SAI_CR3_WDFL(x) (x) +#define FSL_SAI_CR3_WDFL_MASK 0x1f + +/* SAI Transmit and Recieve Configuration 4 Register */ +#define FSL_SAI_TCR4 0x10 +#define FSL_SAI_RCR4 0x90 +#define FSL_SAI_CR4_FRSZ(x) (((x) - 1) << 16) +#define FSL_SAI_CR4_FRSZ_MASK (0x1f << 16) +#define FSL_SAI_CR4_SYWD(x) (((x) - 1) << 8) +#define FSL_SAI_CR4_SYWD_MASK (0x1f << 8) +#define FSL_SAI_CR4_MF BIT(4) +#define FSL_SAI_CR4_FSE BIT(3) +#define FSL_SAI_CR4_FSP BIT(1) +#define FSL_SAI_CR4_FSD_MSTR BIT(0) + +/* SAI Transmit and Recieve Configuration 5 Register */ +#define FSL_SAI_TCR5 0x14 +#define FSL_SAI_RCR5 0x94 +#define FSL_SAI_CR5_WNW(x) (((x) - 1) << 24) +#define FSL_SAI_CR5_WNW_MASK (0x1f << 24) +#define FSL_SAI_CR5_W0W(x) (((x) - 1) << 16) +#define FSL_SAI_CR5_W0W_MASK (0x1f << 16) +#define FSL_SAI_CR5_FBT(x) ((x) << 8) +#define FSL_SAI_CR5_FBT_MASK (0x1f << 8) + +/* SAI audio dividers */ +#define FSL_SAI_TX_DIV 0 +#define FSL_SAI_RX_DIV 1 + +/* SAI type */ +#define FSL_SAI_DMA BIT(0) +#define FSL_SAI_USE_AC97 BIT(1) +#define FSL_SAI_NET BIT(2) +#define FSL_SAI_TRA_SYN BIT(3) +#define FSL_SAI_REC_SYN BIT(4) +#define FSL_SAI_USE_I2S_SLAVE BIT(5) + +#define FSL_FMT_TRANSMITTER 0 +#define FSL_FMT_RECEIVER 1 + +/* SAI clock sources */ +#define FSL_SAI_CLK_BUS 0 +#define FSL_SAI_CLK_MAST1 1 +#define FSL_SAI_CLK_MAST2 2 +#define FSL_SAI_CLK_MAST3 3 + +enum fsl_sai_fbt { + FSL_SAI_FBT_MSB, + FSL_SAI_FBT_LSB, +}; + +struct fsl_sai { + struct clk *clk; + + void __iomem *base; + + enum fsl_sai_fbt fbt; + + struct snd_dmaengine_dai_dma_data dma_params_rx; + struct snd_dmaengine_dai_dma_data dma_params_tx; +}; + +#endif /* __FSL_SAI_H */