2013-10-17 09:11:15

by Xiubo Li

[permalink] [raw]
Subject: [PATCHv1 0/8] ALSA: Add SAI driver and enable SGT15000 codec.

This patch series is mostly Freescale's SAI SoC Digital Audio Interface driver implementation. And the implementation is only compatible with device tree definition.

This patch series is based on linux-next and has been tested on Vybrid VF610 Tower board using device tree.


Added in v1:
- Add SAI SoC Digital Audio Interface driver.
- Add Freescale SAI ALSA SoC Digital Audio Interface node for VF610.
- Enables SAI ALSA SoC DAI device for Vybrid VF610 TOWER board.
- Add device tree bindings for Freescale SAI.
- Revise the bugs about the sgt15000 codec.
- Add SGT15000 based audio machine driver.
- Enable SGT15000 codec based audio driver node for VF610.
- Add device tree bindings for Freescale VF610 sound.






2013-10-17 09:11:23

by Xiubo Li

[permalink] [raw]
Subject: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.

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.

Signed-off-by: Alison Wang <[email protected]
Signed-off-by: Xiubo Li <[email protected]>
---
sound/soc/fsl/Kconfig | 19 ++
sound/soc/fsl/Makefile | 7 +
sound/soc/fsl/fsl-pcm-dma.c | 51 +++++
sound/soc/fsl/fsl-sai.c | 515 ++++++++++++++++++++++++++++++++++++++++++++
sound/soc/fsl/fsl-sai.h | 127 +++++++++++
5 files changed, 719 insertions(+)
create mode 100644 sound/soc/fsl/fsl-pcm-dma.c
create mode 100644 sound/soc/fsl/fsl-sai.c
create mode 100644 sound/soc/fsl/fsl-sai.h

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index cd088cc..a49b386 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -202,3 +202,22 @@ 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
+
+config SND_SOC_FSL_PCM
+ tristate
+ select SND_SOC_GENERIC_DMAENGINE_PCM
+
+endif # SND_FSL_SOC
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
index 4b5970e..865ac23 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -54,3 +54,10 @@ obj-$(CONFIG_SND_MXC_SOC_WM1133_EV1) += snd-soc-wm1133-ev1.o
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_MC13783) += snd-soc-imx-mc13783.o
+
+# FSL ARM SAI/SGT15000 Platform Support
+snd-soc-fsl-sai-objs := fsl-sai.o
+snd-soc-fsl-pcm-objs := fsl-pcm-dma.o
+
+obj-$(CONFIG_SND_SOC_FSL_SAI) += snd-soc-fsl-sai.o
+obj-$(CONFIG_SND_SOC_FSL_PCM) += snd-soc-fsl-pcm.o
diff --git a/sound/soc/fsl/fsl-pcm-dma.c b/sound/soc/fsl/fsl-pcm-dma.c
new file mode 100644
index 0000000..c4d925e
--- /dev/null
+++ b/sound/soc/fsl/fsl-pcm-dma.c
@@ -0,0 +1,51 @@
+/*
+ * 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/dmaengine.h>
+#include <sound/dmaengine_pcm.h>
+#include "fsl-sai.h"
+
+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,
+};
+
+static const struct snd_dmaengine_pcm_config fsl_dmaengine_pcm_config = {
+ .pcm_hardware = &snd_fsl_hardware,
+ .prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config,
+ .prealloc_buffer_size = FSL_SAI_DMABUF_SIZE,
+};
+
+int fsl_pcm_dma_init(struct platform_device *pdev)
+{
+ return snd_dmaengine_pcm_register(&pdev->dev, &fsl_dmaengine_pcm_config,
+ SND_DMAENGINE_PCM_FLAG_NO_RESIDUE);
+}
+EXPORT_SYMBOL_GPL(fsl_pcm_dma_init);
+
+void fsl_pcm_dma_exit(struct platform_device *pdev)
+{
+ snd_dmaengine_pcm_unregister(&pdev->dev);
+}
+EXPORT_SYMBOL_GPL(fsl_pcm_dma_exit);
diff --git a/sound/soc/fsl/fsl-sai.c b/sound/soc/fsl/fsl-sai.c
new file mode 100644
index 0000000..d4c8b44
--- /dev/null
+++ b/sound/soc/fsl/fsl-sai.c
@@ -0,0 +1,515 @@
+/*
+ * 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 "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 = SAI_TCR2;
+ else
+ reg_cr2 = SAI_RCR2;
+
+ val_cr2 = readl(sai->base + reg_cr2);
+ switch (clk_id) {
+ case FSL_SAI_CLK_BUS:
+ val_cr2 &= ~SAI_CR2_MSEL_MASK;
+ val_cr2 |= SAI_CR2_MSEL_BUS;
+ break;
+ case FSL_SAI_CLK_MAST1:
+ val_cr2 &= ~SAI_CR2_MSEL_MASK;
+ val_cr2 |= SAI_CR2_MSEL_MCLK1;
+ break;
+ case FSL_SAI_CLK_MAST2:
+ val_cr2 &= ~SAI_CR2_MSEL_MASK;
+ val_cr2 |= SAI_CR2_MSEL_MCLK2;
+ break;
+ case FSL_SAI_CLK_MAST3:
+ val_cr2 &= ~SAI_CR2_MSEL_MASK;
+ val_cr2 |= 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 + SAI_TCR2);
+ tcr2 &= ~SAI_CR2_DIV_MASK;
+ tcr2 |= SAI_CR2_DIV(div);
+ writel(tcr2, sai->base + SAI_TCR2);
+
+ } else if (div_id == FSL_SAI_RX_DIV) {
+ rcr2 = readl(sai->base + SAI_RCR2);
+ rcr2 &= ~SAI_CR2_DIV_MASK;
+ rcr2 |= SAI_CR2_DIV(div);
+ writel(rcr2, sai->base + 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 = SAI_TCR2;
+ reg_cr3 = SAI_TCR3;
+ reg_cr4 = SAI_TCR4;
+ } else {
+ reg_cr2 = SAI_RCR2;
+ reg_cr3 = SAI_RCR3;
+ reg_cr4 = 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 |= SAI_CR4_MF;
+ else if (sai->fbt == FSL_SAI_FBT_LSB)
+ val_cr4 &= ~SAI_CR4_MF;
+
+ switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+ case SND_SOC_DAIFMT_I2S:
+ val_cr4 |= SAI_CR4_FSE;
+ val_cr4 |= SAI_CR4_FSP;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+ case SND_SOC_DAIFMT_IB_IF:
+ val_cr4 |= SAI_CR4_FSP;
+ val_cr2 &= ~SAI_CR2_BCP;
+ break;
+ case SND_SOC_DAIFMT_IB_NF:
+ val_cr4 &= ~SAI_CR4_FSP;
+ val_cr2 &= ~SAI_CR2_BCP;
+ break;
+ case SND_SOC_DAIFMT_NB_IF:
+ val_cr4 |= SAI_CR4_FSP;
+ val_cr2 |= SAI_CR2_BCP;
+ break;
+ case SND_SOC_DAIFMT_NB_NF:
+ val_cr4 &= ~SAI_CR4_FSP;
+ val_cr2 |= SAI_CR2_BCP;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+ case SND_SOC_DAIFMT_CBS_CFS:
+ val_cr2 |= SAI_CR2_BCD_MSTR;
+ val_cr4 |= SAI_CR4_FSD_MSTR;
+ break;
+ case SND_SOC_DAIFMT_CBM_CFM:
+ val_cr2 &= ~SAI_CR2_BCD_MSTR;
+ val_cr4 &= ~SAI_CR4_FSD_MSTR;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ val_cr3 |= SAI_CR3_TRCE;
+
+ if (fsl_dir == FSL_FMT_RECEIVER)
+ val_cr2 |= 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 + SAI_TCR4);
+ tcr4 &= ~SAI_CR4_FRSZ_MASK;
+ tcr4 |= SAI_CR4_FRSZ(2);
+ writel(tcr4, sai->base + SAI_TCR4);
+ writel(tx_mask, sai->base + SAI_TMR);
+
+ rcr4 = readl(sai->base + SAI_RCR4);
+ rcr4 &= ~SAI_CR4_FRSZ_MASK;
+ rcr4 |= SAI_CR4_FRSZ(2);
+ writel(rcr4, sai->base + SAI_RCR4);
+ writel(rx_mask, sai->base + SAI_RMR);
+
+ return 0;
+}
+
+static int fsl_sai_hw_params_tr(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *cpu_dai, int fsl_dir)
+{
+ u32 val_cr4, val_cr5, reg_cr4, reg_cr5, word_width;
+ struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+
+ if (fsl_dir == FSL_FMT_TRANSMITTER) {
+ reg_cr4 = SAI_TCR4;
+ reg_cr5 = SAI_TCR5;
+ } else {
+ reg_cr4 = SAI_RCR4;
+ reg_cr5 = SAI_RCR5;
+ }
+
+ val_cr4 = readl(sai->base + reg_cr4);
+ val_cr4 &= ~SAI_CR4_SYWD_MASK;
+
+ val_cr5 = readl(sai->base + reg_cr5);
+ val_cr5 &= ~SAI_CR5_WNW_MASK;
+ val_cr5 &= ~SAI_CR5_W0W_MASK;
+ val_cr5 &= ~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 |= SAI_CR4_SYWD(word_width);
+ val_cr5 |= SAI_CR5_WNW(word_width);
+ val_cr5 |= SAI_CR5_W0W(word_width);
+
+ if (sai->fbt == FSL_SAI_FBT_MSB)
+ val_cr5 |= SAI_CR5_FBT(word_width - 1);
+ else if (sai->fbt == FSL_SAI_FBT_LSB)
+ val_cr5 |= SAI_CR5_FBT(0);
+
+ writel(val_cr4, sai->base + reg_cr4);
+ writel(val_cr5, sai->base + reg_cr5);
+
+ 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)
+{
+ int ret;
+
+ ret = fsl_sai_hw_params_tr(substream, params, cpu_dai,
+ FSL_FMT_TRANSMITTER);
+ if (ret) {
+ dev_err(cpu_dai->dev,
+ "Cannot set sai transmitter hw params: %d\n",
+ ret);
+ return ret;
+ }
+
+ ret = fsl_sai_hw_params_tr(substream, params, cpu_dai,
+ FSL_FMT_RECEIVER);
+ if (ret) {
+ dev_err(cpu_dai->dev,
+ "Cannot set sai's receiver hw params: %d\n",
+ ret);
+ return ret;
+ }
+
+ 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 + SAI_TCSR);
+ rcsr = readl(sai->base + SAI_RCSR);
+
+ switch (cmd) {
+ case SNDRV_PCM_TRIGGER_START:
+ case SNDRV_PCM_TRIGGER_RESUME:
+ case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+ rcsr |= SAI_CSR_TERE | SAI_CSR_FRDE;
+ tcsr |= SAI_CSR_TERE | SAI_CSR_FRDE;
+ writel(rcsr, sai->base + SAI_RCSR);
+ udelay(10);
+ writel(tcsr, sai->base + SAI_TCSR);
+ break;
+
+ case SNDRV_PCM_TRIGGER_STOP:
+ case SNDRV_PCM_TRIGGER_SUSPEND:
+ case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+ tcsr &= ~(SAI_CSR_TERE | SAI_CSR_FRDE);
+ rcsr &= ~(SAI_CSR_TERE | SAI_CSR_FRDE);
+ writel(tcsr, sai->base + SAI_TCSR);
+ udelay(10);
+ writel(rcsr, sai->base + SAI_RCSR);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static 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 + SAI_RCSR);
+ writel(0x0, sai->base + SAI_TCSR);
+ writel(sai->dma_params_tx.maxburst, sai->base + SAI_TCR1);
+ writel(sai->dma_params_rx.maxburst, sai->base + 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 of_phandle_args dma_args;
+ int index;
+ struct resource *res;
+ struct fsl_sai *sai;
+ int ret = 0;
+ struct device_node *np = pdev->dev.of_node;
+
+ 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)) {
+ ret = PTR_ERR(sai->base);
+ return ret;
+ }
+
+ sai->clk = devm_clk_get(&pdev->dev, "sai");
+ if (IS_ERR(sai->clk)) {
+ ret = PTR_ERR(sai->clk);
+ dev_err(&pdev->dev, "Cannot get sai's clock: %d\n", ret);
+ return ret;
+ }
+
+ sai->dma_params_rx.addr = res->start + SAI_RDR;
+ sai->dma_params_rx.maxburst = 6;
+ index = of_property_match_string(np, "dma-names", "rx");
+ ret = of_parse_phandle_with_args(np, "dmas", "#dma-cells", index,
+ &dma_args);
+ if (ret)
+ return ret;
+ sai->dma_params_rx.slave_id = dma_args.args[1];
+
+ sai->dma_params_tx.addr = res->start + SAI_TDR;
+ sai->dma_params_tx.maxburst = 6;
+ index = of_property_match_string(np, "dma-names", "tx");
+ ret = of_parse_phandle_with_args(np, "dmas", "#dma-cells", index,
+ &dma_args);
+ if (ret)
+ return ret;
+ sai->dma_params_tx.slave_id = dma_args.args[1];
+
+ ret = snd_soc_register_component(&pdev->dev, &fsl_component,
+ &fsl_sai_dai, 1);
+ if (ret)
+ return ret;
+
+ ret = fsl_pcm_dma_init(pdev);
+ if (ret)
+ goto out;
+
+ platform_set_drvdata(pdev, sai);
+
+ return 0;
+
+out:
+ snd_soc_unregister_component(&pdev->dev);
+ return ret;
+}
+
+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);
+
+ clk_disable_unprepare(sai->clk);
+
+ 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, <[email protected]>");
+MODULE_AUTHOR("Alison Wang, <[email protected]>");
+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..ab76a8e
--- /dev/null
+++ b/sound/soc/fsl/fsl-sai.h
@@ -0,0 +1,127 @@
+/*
+ * 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)
+
+#define FSL_SAI_DMABUF_SIZE (32 * 1024)
+#define TCD_NUMBER 4
+#define EDMA_PRIO_HIGH 6
+
+/* SAI Transmit/Recieve Control Register */
+#define SAI_TCSR 0x00
+#define SAI_RCSR 0x80
+#define SAI_CSR_TERE BIT(31)
+#define SAI_CSR_FWF BIT(17)
+#define SAI_CSR_FRIE BIT(8)
+#define SAI_CSR_FRDE BIT(0)
+
+/* SAI Transmit Data/FIFO/MASK Register */
+#define SAI_TDR 0x20
+#define SAI_TFR 0x40
+#define SAI_TMR 0x60
+
+/* SAI Recieve Data/FIFO/MASK Register */
+#define SAI_RDR 0xa0
+#define SAI_RFR 0xc0
+#define SAI_RMR 0xe0
+
+/* SAI Transmit and Recieve Configuration 1 Register */
+#define SAI_TCR1 0x04
+#define SAI_RCR1 0x84
+
+/* SAI Transmit and Recieve Configuration 2 Register */
+#define SAI_TCR2 0x08
+#define SAI_RCR2 0x88
+#define SAI_CR2_SYNC BIT(30)
+#define SAI_CR2_MSEL_MASK (0xff << 26)
+#define SAI_CR2_MSEL_BUS 0
+#define SAI_CR2_MSEL_MCLK1 BIT(26)
+#define SAI_CR2_MSEL_MCLK2 BIT(27)
+#define SAI_CR2_MSEL_MCLK3 (BIT(26)|BIT(27))
+#define SAI_CR2_BCP BIT(25)
+#define SAI_CR2_BCD_MSTR BIT(24)
+#define SAI_CR2_DIV(x) (x)
+#define SAI_CR2_DIV_MASK 0xff
+
+/* SAI Transmit and Recieve Configuration 3 Register */
+#define SAI_TCR3 0x0c
+#define SAI_RCR3 0x8c
+#define SAI_CR3_TRCE BIT(16)
+#define SAI_CR3_WDFL(x) (x)
+#define SAI_CR3_WDFL_MASK 0x1f
+
+/* SAI Transmit and Recieve Configuration 4 Register */
+#define SAI_TCR4 0x10
+#define SAI_RCR4 0x90
+#define SAI_CR4_FRSZ(x) (((x) - 1) << 16)
+#define SAI_CR4_FRSZ_MASK (0x1f << 16)
+#define SAI_CR4_SYWD(x) (((x) - 1) << 8)
+#define SAI_CR4_SYWD_MASK (0x1f << 8)
+#define SAI_CR4_MF BIT(4)
+#define SAI_CR4_FSE BIT(3)
+#define SAI_CR4_FSP BIT(1)
+#define SAI_CR4_FSD_MSTR BIT(0)
+
+/* SAI Transmit and Recieve Configuration 5 Register */
+#define SAI_TCR5 0x14
+#define SAI_RCR5 0x94
+#define SAI_CR5_WNW(x) (((x) - 1) << 24)
+#define SAI_CR5_WNW_MASK (0x1f << 24)
+#define SAI_CR5_W0W(x) (((x) - 1) << 16)
+#define SAI_CR5_W0W_MASK (0x1f << 16)
+#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
+
+/* 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;
+};
+
+int fsl_pcm_dma_init(struct platform_device *pdev);
+void fsl_pcm_dma_exit(struct platform_device *pdev);
+
+#endif /* __FSL_SAI_H */
--
1.8.0

2013-10-17 09:11:29

by Xiubo Li

[permalink] [raw]
Subject: [PATCHv1 2/8] ARM: dts: Add Freescale SAI ALSA SoC Digital Audio Interface node for VF610.

This patch add the SAI's edma mux Tx and Rx support.

Signed-off-by: Jingchang Lu <[email protected]>
Signed-off-by: Xiubo Li <[email protected]>
---
arch/arm/boot/dts/vf610.dtsi | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
index 18e3a4c..391f180 100644
--- a/arch/arm/boot/dts/vf610.dtsi
+++ b/arch/arm/boot/dts/vf610.dtsi
@@ -151,9 +151,11 @@
sai2: sai@40031000 {
compatible = "fsl,vf610-sai";
reg = <0x40031000 0x1000>;
- interrupts = <0 86 0x04>;
clocks = <&clks VF610_CLK_SAI2>;
clock-names = "sai";
+ dma-names = "tx", "rx";
+ dmas = <&edma0 0 VF610_EDMA_MUXID0_SAI2_TX>,
+ <&edma0 0 VF610_EDMA_MUXID0_SAI2_RX>;
status = "disabled";
};

--
1.8.0

2013-10-17 09:11:46

by Xiubo Li

[permalink] [raw]
Subject: [PATCHv1 4/8] Documentation: Add device tree bindings for Freescale SAI.

This adds the Document for Freescale SAI driver under
Documentation/devicetree/bindings/sound/.

Signed-off-by: Xiubo Li <[email protected]>
---
.../devicetree/bindings/sound/fsl-sai.txt | 32 ++++++++++++++++++++++
1 file changed, 32 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/fsl-sai.txt

diff --git a/Documentation/devicetree/bindings/sound/fsl-sai.txt b/Documentation/devicetree/bindings/sound/fsl-sai.txt
new file mode 100644
index 0000000..267afbd
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/fsl-sai.txt
@@ -0,0 +1,32 @@
+Freescale Synchronous Audio Interface (SAI).
+
+The SAI is based on I2S module that used communicating with audio codecs,
+which provides a synchronous audio interface that supports fullduplex
+serial interfaces with frame synchronization such as I2S, AC97, TDM, and
+codec/DSP interfaces.
+
+
+Required properties:
+- compatible: Compatible list, contains "fsl,vf610-sai".
+- reg: Offset and length of the register set for the device.
+- clocks: Must contain an entry for each entry in clock-names.
+- clock-names : Must include the "sai" entry.
+- dmas : Generic dma devicetree binding as described in
+ Documentation/devicetree/bindings/dma/dma.txt.
+- dma-names : Two dmas have to be defined, "tx" and "rx".
+- pinctrl-names: Must contain a "default" entry.
+- pinctrl-NNN: One property must exist for each entry in pinctrl-names.
+ See ../pinctrl/pinctrl-bindings.txt for details of the property values.
+
+Example:
+sai2: sai@40031000 {
+ compatible = "fsl,vf610-sai";
+ reg = <0x40031000 0x1000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_sai2_1>;
+ clocks = <&clks VF610_CLK_SAI2>;
+ clock-names = "sai";
+ dma-names = "tx", "rx";
+ dmas = <&edma0 0 VF610_EDMA_MUXID0_SAI2_TX>,
+ <&edma0 0 VF610_EDMA_MUXID0_SAI2_RX>;
+};
--
1.8.0

2013-10-17 09:11:54

by Xiubo Li

[permalink] [raw]
Subject: [PATCHv1 5/8] ASoC: sgtl5000: Revise the bugs about the sgt15000 codec.

When the CONFIG_REGULATOR is disabled there will be some warnings
printed out.

Signed-off-by: Xiubo Li <[email protected]>
---
sound/soc/codecs/sgtl5000.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
index 1f4093f..4e2e4c9 100644
--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -883,14 +883,19 @@ static int ldo_regulator_register(struct snd_soc_codec *codec,
struct regulator_init_data *init_data,
int voltage)
{
+#ifdef CONFIG_SND_SOC_FSL_SGTL5000
+ return 0;
+#else
dev_err(codec->dev, "this setup needs regulator support in the kernel\n");
return -EINVAL;
+#endif
}

static int ldo_regulator_remove(struct snd_soc_codec *codec)
{
return 0;
}
+
#endif

/*
@@ -1137,6 +1142,7 @@ static int sgtl5000_resume(struct snd_soc_codec *codec)
#define sgtl5000_resume NULL
#endif /* CONFIG_SUSPEND */

+#ifdef CONFIG_REGULATOR
/*
* sgtl5000 has 3 internal power supplies:
* 1. VAG, normally set to vdda/2
@@ -1269,6 +1275,7 @@ static int sgtl5000_set_power_regs(struct snd_soc_codec *codec)

return 0;
}
+#endif

static int sgtl5000_replace_vddd_with_ldo(struct snd_soc_codec *codec)
{
@@ -1370,6 +1377,7 @@ err_regulator_free:
sgtl5000->supplies);
if (external_vddd)
ldo_regulator_remove(codec);
+
return ret;

}
@@ -1391,11 +1399,12 @@ static int sgtl5000_probe(struct snd_soc_codec *codec)
if (ret)
return ret;

+#ifdef CONFIG_REGULATOR
/* power up sgtl5000 */
ret = sgtl5000_set_power_regs(codec);
if (ret)
goto err;
-
+#endif
/* enable small pop, introduce 400ms delay in turning off */
snd_soc_update_bits(codec, SGTL5000_CHIP_REF_CTRL,
SGTL5000_SMALL_POP,
@@ -1446,6 +1455,7 @@ err:
sgtl5000->supplies);
regulator_bulk_free(ARRAY_SIZE(sgtl5000->supplies),
sgtl5000->supplies);
+
ldo_regulator_remove(codec);

return ret;
@@ -1461,6 +1471,7 @@ static int sgtl5000_remove(struct snd_soc_codec *codec)
sgtl5000->supplies);
regulator_bulk_free(ARRAY_SIZE(sgtl5000->supplies),
sgtl5000->supplies);
+
ldo_regulator_remove(codec);

return 0;
--
1.8.0

2013-10-17 09:11:59

by Xiubo Li

[permalink] [raw]
Subject: [PATCHv1 6/8] ASoC: fsl: add SGT15000 based audio machine driver.

This is the SGTl5000 codec based audio driver supported with both
playback and capture dai link implemention.

This implementation is only compatible with device tree definition.

Signed-off-by: Alison Wang <[email protected]
Signed-off-by: Xiubo Li <[email protected]>
---
sound/soc/fsl/Kconfig | 10 +++
sound/soc/fsl/Makefile | 2 +
sound/soc/fsl/fsl-sgtl5000.c | 208 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 220 insertions(+)
create mode 100644 sound/soc/fsl/fsl-sgtl5000.c

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index a49b386..3fbbbf2 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -220,4 +220,14 @@ config SND_SOC_FSL_PCM
tristate
select SND_SOC_GENERIC_DMAENGINE_PCM

+config SND_SOC_FSL_SGTL5000
+ tristate "SoC Audio support for FSL boards with sgtl5000"
+ depends on OF && I2C
+ select SND_SOC_FSL_SAI
+ select SND_SOC_FSL_PCM
+ select SND_SOC_SGTL5000
+ help
+ Say Y if you want to add support for SoC audio on an FSL board with
+ a sgtl5000 codec.
+
endif # SND_FSL_SOC
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
index 865ac23..e8bf0bd 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -58,6 +58,8 @@ 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
snd-soc-fsl-pcm-objs := fsl-pcm-dma.o
+snd-soc-fsl-sgtl5000-objs := fsl-sgtl5000.o

obj-$(CONFIG_SND_SOC_FSL_SAI) += snd-soc-fsl-sai.o
obj-$(CONFIG_SND_SOC_FSL_PCM) += snd-soc-fsl-pcm.o
+obj-$(CONFIG_SND_SOC_FSL_SGTL5000) += snd-soc-fsl-sgtl5000.o
diff --git a/sound/soc/fsl/fsl-sgtl5000.c b/sound/soc/fsl/fsl-sgtl5000.c
new file mode 100644
index 0000000..bab85ec
--- /dev/null
+++ b/sound/soc/fsl/fsl-sgtl5000.c
@@ -0,0 +1,208 @@
+/*
+ * Freeacale ALSA SoC Audio using SGT1500 as codec.
+ *
+ * Copyright 2012-2013 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/i2c.h>
+#include <linux/clk.h>
+
+#include "../codecs/sgtl5000.h"
+#include "fsl-sai.h"
+
+static unsigned int sysclk_rate;
+
+static int fsl_sgtl5000_dai_init(struct snd_soc_pcm_runtime *rtd)
+{
+ int ret;
+ struct device *dev = rtd->card->dev;
+
+ ret = snd_soc_dai_set_sysclk(rtd->codec_dai, SGTL5000_SYSCLK,
+ sysclk_rate, SND_SOC_CLOCK_IN);
+ if (ret) {
+ dev_err(dev, "could not set codec driver clock params :%d\n",
+ ret);
+ return ret;
+ }
+
+ ret = snd_soc_dai_set_sysclk(rtd->cpu_dai, FSL_SAI_CLK_BUS,
+ sysclk_rate, SND_SOC_CLOCK_OUT);
+ if (ret) {
+ dev_err(dev, "could not set cpu dai driver clock params :%d\n",
+ ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int sgtl5000_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params)
+{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ unsigned int channels = params_channels(params);
+
+ /* TODO: The SAI driver should figure this out for us */
+ switch (channels) {
+ case 2:
+ snd_soc_dai_set_tdm_slot(cpu_dai, 0xfffffffc, 0xfffffffc, 2, 0);
+ break;
+ case 1:
+ snd_soc_dai_set_tdm_slot(cpu_dai, 0xfffffffe, 0xfffffffe, 1, 0);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static struct snd_soc_ops fsl_sgtl5000_hifi_ops = {
+ .hw_params = sgtl5000_params,
+};
+
+static struct snd_soc_dai_link fsl_sgtl5000_dai = {
+ .name = "HiFi",
+ .stream_name = "HiFi",
+ .codec_dai_name = "sgtl5000",
+ .init = &fsl_sgtl5000_dai_init,
+ .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+ SND_SOC_DAIFMT_CBM_CFM,
+ .ops = &fsl_sgtl5000_hifi_ops,
+};
+
+static const struct snd_soc_dapm_widget fsl_sgtl5000_dapm_widgets[] = {
+ SND_SOC_DAPM_MIC("Mic Jack", NULL),
+ SND_SOC_DAPM_LINE("Line In Jack", NULL),
+ SND_SOC_DAPM_HP("Headphone Jack", NULL),
+ SND_SOC_DAPM_SPK("Line Out Jack", NULL),
+ SND_SOC_DAPM_SPK("Ext Spk", NULL),
+};
+
+static struct snd_soc_card fsl_sgt1500_card = {
+ .owner = THIS_MODULE,
+ .num_links = 1,
+ .dai_link = &fsl_sgtl5000_dai,
+ .dapm_widgets = fsl_sgtl5000_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(fsl_sgtl5000_dapm_widgets),
+};
+
+static int fsl_sgtl5000_parse_dt(struct platform_device *pdev)
+{
+ int ret;
+ struct device_node *sai_np, *codec_np;
+ struct clk *codec_clk;
+ struct i2c_client *codec_dev;
+ struct device_node *np = pdev->dev.of_node;
+
+ ret = snd_soc_of_parse_card_name(&fsl_sgt1500_card, "model");
+ if (ret)
+ return ret;
+
+ ret = snd_soc_of_parse_audio_routing(&fsl_sgt1500_card,
+ "audio-routing");
+ if (ret)
+ return ret;
+
+ sai_np = of_parse_phandle(np, "saif-controller", 0);
+ if (!sai_np) {
+ dev_err(&pdev->dev, "\"saif-controller\" phandle missing or "
+ "invalid\n");
+ return -EINVAL;
+ }
+ fsl_sgtl5000_dai.cpu_of_node = sai_np;
+ fsl_sgtl5000_dai.platform_of_node = sai_np;
+
+ codec_np = of_parse_phandle(np, "audio-codec", 0);
+ if (!codec_np) {
+ dev_err(&pdev->dev, "\"audio-codec\" phandle missing or "
+ "invalid\n");
+ ret = -EINVAL;
+ goto sai_np_fail;
+ }
+ fsl_sgtl5000_dai.codec_of_node = codec_np;
+
+ codec_dev = of_find_i2c_device_by_node(codec_np);
+ if (!codec_dev) {
+ dev_err(&pdev->dev, "failed to find codec platform device\n");
+ ret = PTR_ERR(codec_dev);
+ goto codec_np_fail;
+ }
+
+ codec_clk = devm_clk_get(&codec_dev->dev, NULL);
+ if (IS_ERR(codec_clk)) {
+ dev_err(&pdev->dev, "failed to get codec clock\n");
+ ret = PTR_ERR(codec_clk);
+ goto codec_np_fail;
+ }
+
+ sysclk_rate = clk_get_rate(codec_clk);
+
+codec_np_fail:
+ of_node_put(codec_np);
+sai_np_fail:
+ of_node_put(sai_np);
+
+ return ret;
+}
+
+static int fsl_sgtl5000_probe(struct platform_device *pdev)
+{
+ int ret;
+
+ fsl_sgt1500_card.dev = &pdev->dev;
+
+ ret = fsl_sgtl5000_parse_dt(pdev);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "parse sgtl5000 device tree failed :%d\n",
+ ret);
+ return ret;
+ }
+
+ ret = snd_soc_register_card(&fsl_sgt1500_card);
+ if (ret) {
+ dev_err(&pdev->dev, "register soc sound card failed :%d\n",
+ ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int fsl_sgtl5000_remove(struct platform_device *pdev)
+{
+ snd_soc_unregister_card(&fsl_sgt1500_card);
+
+ return 0;
+}
+
+static const struct of_device_id fsl_sgtl5000_dt_ids[] = {
+ { .compatible = "fsl,vf610-sgtl5000", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, fsl_sgtl5000_dt_ids);
+
+static struct platform_driver fsl_sgtl5000_driver = {
+ .driver = {
+ .name = "fsl-sgtl5000",
+ .owner = THIS_MODULE,
+ .of_match_table = fsl_sgtl5000_dt_ids,
+ },
+ .probe = fsl_sgtl5000_probe,
+ .remove = fsl_sgtl5000_remove,
+};
+module_platform_driver(fsl_sgtl5000_driver);
+
+MODULE_AUTHOR("Xiubo Li <[email protected]>");
+MODULE_DESCRIPTION("Freescale SGTL5000 ASoC driver");
+MODULE_LICENSE("GPL");
--
1.8.0

2013-10-17 09:12:10

by Xiubo Li

[permalink] [raw]
Subject: [PATCHv1 7/8] ARM: dts: Enable SGT15000 codec based audio driver node for VF610.

This patch add and enable SGT15000 codec support, and also specified
the corresponding SAI node.

Signed-off-by: Xiubo Li <[email protected]>
Signed-off-by: Alison Wang <[email protected]
---
arch/arm/boot/dts/vf610-twr.dts | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
index e4106dd..a2d9214 100644
--- a/arch/arm/boot/dts/vf610-twr.dts
+++ b/arch/arm/boot/dts/vf610-twr.dts
@@ -34,6 +34,19 @@
};
};

+ sound {
+ compatible = "fsl,vf610-sgtl5000";
+ model = "vf610-sgtl5000";
+ saif-controller = <&sai2>;
+ audio-codec = <&codec>;
+ audio-routing =
+ "MIC_IN", "Mic Jack",
+ "Mic Jack", "Mic Bias",
+ "LINE_IN", "Line In Jack",
+ "Headphone Jack", "HP_OUT",
+ "Ext Spk", "LINE_OUT";
+ };
+
};

&fec0 {
@@ -55,6 +68,12 @@
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_i2c0_1>;
status = "okay";
+
+ codec: sgtl5000@0a {
+ compatible = "fsl,sgtl5000";
+ reg = <0x0a>;
+ clocks = <&clks VF610_CLK_SAI2>;
+ };
};

&sai2 {
--
1.8.0

2013-10-17 09:12:15

by Xiubo Li

[permalink] [raw]
Subject: [PATCHv1 8/8] Documentation: Add device tree bindings for Freescale VF610 sound.

This adds the Document for Freescale VF610 sound driver under
Documentation/devicetree/bindings/sound/.

Signed-off-by: Xiubo Li <[email protected]>
---
.../devicetree/bindings/sound/fsl-sgtl5000.txt | 52 ++++++++++++++++++++++
1 file changed, 52 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/fsl-sgtl5000.txt

diff --git a/Documentation/devicetree/bindings/sound/fsl-sgtl5000.txt b/Documentation/devicetree/bindings/sound/fsl-sgtl5000.txt
new file mode 100644
index 0000000..43e350f
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/fsl-sgtl5000.txt
@@ -0,0 +1,52 @@
+Freescale VF610 audio complex with SGTL5000 codec
+
+Required properties:
+- compatible: "fsl,vf610-sgtl5000"
+- model: The user-visible name of this sound complex.
+- saif-controllers: The phandle list of the SAI controller.
+- audio-codec: The phandle of the SGTL5000 audio codec.
+- audio-routing : A list of the connections between audio components.
+ Each entry is a pair of strings, the first being the connection's sink,
+ the second being the connection's source. Valid names could be power
+ supplies, SGTL5000 pins, and the jacks on the board:
+
+ -- Power supplies:
+ * Mic Bias
+
+ -- SGTL5000 pins:
+ * MIC_IN
+ * LINE_IN
+ * HP_OUT
+ * LINE_OUT
+
+ -- Board connectors:
+ * Mic Jack
+ * Line In Jack
+ * Headphone Jack
+ * Line Out Jack
+ * Ext Spk
+
+Example:
+
+sound {
+ compatible = "fsl,vf610-sgtl5000";
+ model = "vf610-sgtl5000";
+ saif-controller = <&sai2>;
+ audio-codec = <&codec>;
+ audio-routing =
+ "MIC_IN", "Mic Jack",
+ "Mic Jack", "Mic Bias",
+ "LINE_IN", "Line In Jack",
+ "Headphone Jack", "HP_OUT",
+ "Ext Spk", "LINE_OUT";
+};
+
+&i2c0 {
+ ...
+
+ codec: sgtl5000@0a {
+ compatible = "fsl,sgtl5000";
+ reg = <0x0a>;
+ clocks = <&clks VF610_CLK_SAI2>;
+ };
+};
--
1.8.0

2013-10-17 09:11:36

by Xiubo Li

[permalink] [raw]
Subject: [PATCHv1 3/8] ARM: dts: Enables SAI ALSA SoC DAI device for Vybrid VF610 TOWER board.

This patch add and enable SAI device.

Signed-off-by: Xiubo Li <[email protected]>
---
arch/arm/boot/dts/vf610-twr.dts | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
index 1a58678..e4106dd 100644
--- a/arch/arm/boot/dts/vf610-twr.dts
+++ b/arch/arm/boot/dts/vf610-twr.dts
@@ -57,6 +57,12 @@
status = "okay";
};

+&sai2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_sai2_1>;
+ status = "okay";
+};
+
&uart1 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_uart1_1>;
--
1.8.0

2013-10-17 09:43:18

by Lothar Waßmann

[permalink] [raw]
Subject: Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.

Hi,

Xiubo Li <[email protected]> wrote:
[...]
> diff --git a/sound/soc/fsl/fsl-pcm-dma.c b/sound/soc/fsl/fsl-pcm-dma.c
> new file mode 100644
> index 0000000..c4d925e
> --- /dev/null
> +++ b/sound/soc/fsl/fsl-pcm-dma.c
> @@ -0,0 +1,51 @@
[...]
> +
> +static int fsl_sai_probe(struct platform_device *pdev)
> +{
> + struct of_phandle_args dma_args;
> + int index;
> + struct resource *res;
> + struct fsl_sai *sai;
> + int ret = 0;
> + struct device_node *np = pdev->dev.of_node;
> +
> + 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)) {
> + ret = PTR_ERR(sai->base);
> + return ret;
>
could be:
return PTR_ERR(sai->base);

[...]
> +static const struct of_device_id fsl_sai_ids[] = {
> + { .compatible = "fsl,vf610-sai", },
> + { /*sentinel*/ },
>
The comma after the last entry in a struct initializer is there to make
patches that append another entry cleaner. Since this entry is and
always must be the last entry, the comma is useless here.

> diff --git a/sound/soc/fsl/fsl-sai.h b/sound/soc/fsl/fsl-sai.h
> new file mode 100644
> index 0000000..ab76a8e
> --- /dev/null
> +++ b/sound/soc/fsl/fsl-sai.h
> @@ -0,0 +1,127 @@
> +/*
> + * 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)
> +
> +#define FSL_SAI_DMABUF_SIZE (32 * 1024)
> +#define TCD_NUMBER 4
> +#define EDMA_PRIO_HIGH 6
> +
strange indentation with mixed spaces and tabs.

> +/* SAI Transmit and Recieve Configuration 2 Register */
> +#define SAI_TCR2 0x08
> +#define SAI_RCR2 0x88
> +#define SAI_CR2_SYNC BIT(30)
> +#define SAI_CR2_MSEL_MASK (0xff << 26)
> +#define SAI_CR2_MSEL_BUS 0
> +#define SAI_CR2_MSEL_MCLK1 BIT(26)
> +#define SAI_CR2_MSEL_MCLK2 BIT(27)
> +#define SAI_CR2_MSEL_MCLK3 (BIT(26)|BIT(27))
>
spaces around '|'?


Lothar Waßmann
--
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

http://www.karo-electronics.de | [email protected]
___________________________________________________________

2013-10-17 09:51:16

by Lucas Stach

[permalink] [raw]
Subject: Re: [PATCHv1 8/8] Documentation: Add device tree bindings for Freescale VF610 sound.

Am Donnerstag, den 17.10.2013, 17:01 +0800 schrieb Xiubo Li:
> This adds the Document for Freescale VF610 sound driver under
> Documentation/devicetree/bindings/sound/.
>
> Signed-off-by: Xiubo Li <[email protected]>
> ---
> .../devicetree/bindings/sound/fsl-sgtl5000.txt | 52 ++++++++++++++++++++++
> 1 file changed, 52 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/fsl-sgtl5000.txt
>
> diff --git a/Documentation/devicetree/bindings/sound/fsl-sgtl5000.txt b/Documentation/devicetree/bindings/sound/fsl-sgtl5000.txt
> new file mode 100644
> index 0000000..43e350f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/fsl-sgtl5000.txt

This document name is overly generic, there are more than one FSL
platforms with SGTL5000 codecs. Please include the vf610 here.

> @@ -0,0 +1,52 @@
> +Freescale VF610 audio complex with SGTL5000 codec
> +
> +Required properties:
> +- compatible: "fsl,vf610-sgtl5000"
> +- model: The user-visible name of this sound complex.
> +- saif-controllers: The phandle list of the SAI controller.
> +- audio-codec: The phandle of the SGTL5000 audio codec.
> +- audio-routing : A list of the connections between audio components.
> + Each entry is a pair of strings, the first being the connection's sink,
> + the second being the connection's source. Valid names could be power
> + supplies, SGTL5000 pins, and the jacks on the board:
> +
> + -- Power supplies:
> + * Mic Bias
> +
> + -- SGTL5000 pins:
> + * MIC_IN
> + * LINE_IN
> + * HP_OUT
> + * LINE_OUT
> +
> + -- Board connectors:
> + * Mic Jack
> + * Line In Jack
> + * Headphone Jack
> + * Line Out Jack
> + * Ext Spk
> +
> +Example:
> +
> +sound {
> + compatible = "fsl,vf610-sgtl5000";
> + model = "vf610-sgtl5000";
> + saif-controller = <&sai2>;
> + audio-codec = <&codec>;
> + audio-routing =
> + "MIC_IN", "Mic Jack",
> + "Mic Jack", "Mic Bias",
> + "LINE_IN", "Line In Jack",
> + "Headphone Jack", "HP_OUT",
> + "Ext Spk", "LINE_OUT";
> +};
> +
> +&i2c0 {
> + ...
> +
> + codec: sgtl5000@0a {
> + compatible = "fsl,sgtl5000";
> + reg = <0x0a>;
> + clocks = <&clks VF610_CLK_SAI2>;
> + };
> +};

--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2013-10-17 10:16:47

by Nicolin Chen

[permalink] [raw]
Subject: Re: [PATCHv1 5/8] ASoC: sgtl5000: Revise the bugs about the sgt15000 codec.

Hi,

On Thu, Oct 17, 2013 at 05:01:14PM +0800, Xiubo Li wrote:
> When the CONFIG_REGULATOR is disabled there will be some warnings
> printed out.

A little confused by the title. But after looking at the comments,
is the patch just gonna add some debug info for the case when the
CONFIG_REGULATOR's been un-selected?

Well first, I think at least the title should be more explicit.
And second, the necessity of this patch might just a little...
if CONFIG_REGULATOR is required to power it up, why not turn it on.

>
> Signed-off-by: Xiubo Li <[email protected]>
> ---
> sound/soc/codecs/sgtl5000.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
> index 1f4093f..4e2e4c9 100644
> --- a/sound/soc/codecs/sgtl5000.c
> +++ b/sound/soc/codecs/sgtl5000.c
> @@ -883,14 +883,19 @@ static int ldo_regulator_register(struct snd_soc_codec *codec,
> struct regulator_init_data *init_data,
> int voltage)
> {
> +#ifdef CONFIG_SND_SOC_FSL_SGTL5000

Why there's FSL_SGTL5000 here? Not supposed to be CONFIG_REGULATOR?

> + return 0;
> +#else
> dev_err(codec->dev, "this setup needs regulator support in the kernel\n");
> return -EINVAL;
> +#endif
> }
>
> static int ldo_regulator_remove(struct snd_soc_codec *codec)
> {
> return 0;
> }
> +

I don't think it's fair to add a meaningless line. It doesn't make any sense
according to the title and comments.

> #endif
>
> /*
> @@ -1137,6 +1142,7 @@ static int sgtl5000_resume(struct snd_soc_codec *codec)
> #define sgtl5000_resume NULL
> #endif /* CONFIG_SUSPEND */
>
> +#ifdef CONFIG_REGULATOR

The inline regulator-related functions are already have REGULATOR dependency.
Is that necessary to put an additional one here?

> /*
> * sgtl5000 has 3 internal power supplies:
> * 1. VAG, normally set to vdda/2
> @@ -1269,6 +1275,7 @@ static int sgtl5000_set_power_regs(struct snd_soc_codec *codec)
>
> return 0;
> }
> +#endif
>
> static int sgtl5000_replace_vddd_with_ldo(struct snd_soc_codec *codec)
> {
> @@ -1370,6 +1377,7 @@ err_regulator_free:
> sgtl5000->supplies);
> if (external_vddd)
> ldo_regulator_remove(codec);
> +

Pls drop this.

> return ret;
>
> }
> @@ -1391,11 +1399,12 @@ static int sgtl5000_probe(struct snd_soc_codec *codec)
> if (ret)
> return ret;
>
> +#ifdef CONFIG_REGULATOR
> /* power up sgtl5000 */
> ret = sgtl5000_set_power_regs(codec);
> if (ret)
> goto err;
> -
> +#endif
> /* enable small pop, introduce 400ms delay in turning off */
> snd_soc_update_bits(codec, SGTL5000_CHIP_REF_CTRL,
> SGTL5000_SMALL_POP,
> @@ -1446,6 +1455,7 @@ err:
> sgtl5000->supplies);
> regulator_bulk_free(ARRAY_SIZE(sgtl5000->supplies),
> sgtl5000->supplies);
> +

Ditto

> ldo_regulator_remove(codec);
>
> return ret;
> @@ -1461,6 +1471,7 @@ static int sgtl5000_remove(struct snd_soc_codec *codec)
> sgtl5000->supplies);
> regulator_bulk_free(ARRAY_SIZE(sgtl5000->supplies),
> sgtl5000->supplies);
> +

Ditto

Best regards,
Nicolin Chen

> ldo_regulator_remove(codec);
>
> return 0;
> --
> 1.8.0
>

2013-10-17 10:18:28

by Lothar Waßmann

[permalink] [raw]
Subject: Re: [PATCHv1 5/8] ASoC: sgtl5000: Revise the bugs about the sgt15000 codec.

Hi,

Xiubo Li <[email protected]> wrote:
> When the CONFIG_REGULATOR is disabled there will be some warnings
> printed out.
>
> Signed-off-by: Xiubo Li <[email protected]>
> ---
> sound/soc/codecs/sgtl5000.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
> index 1f4093f..4e2e4c9 100644
> --- a/sound/soc/codecs/sgtl5000.c
> +++ b/sound/soc/codecs/sgtl5000.c
> @@ -883,14 +883,19 @@ static int ldo_regulator_register(struct snd_soc_codec *codec,
> struct regulator_init_data *init_data,
> int voltage)
> {
> +#ifdef CONFIG_SND_SOC_FSL_SGTL5000
> + return 0;
> +#else
> dev_err(codec->dev, "this setup needs regulator support in the kernel\n");
> return -EINVAL;
> +#endif
>
This looks wrong to me, as this will disable the error for unsolicited
platforms in a multi arch kernel!

> static int ldo_regulator_remove(struct snd_soc_codec *codec)
> {
> return 0;
> }
> +
> #endif
>
Why do you add an extra empty line here?


Lothar Waßmann
--
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

http://www.karo-electronics.de | [email protected]
___________________________________________________________

2013-10-17 10:23:43

by Lothar Waßmann

[permalink] [raw]
Subject: Re: [PATCHv1 0/8] ALSA: Add SAI driver and enable SGT15000 codec.

Hi,

Xiubo Li <[email protected]> wrote:

The subject has a wrong name for the codec "SGT1..." instead of
"SGTL...", which will make it difficult to search for this thread in
mail archives or in commit messages once this patches should be applied!


Lothar Waßmann
--
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

http://www.karo-electronics.de | [email protected]
___________________________________________________________

2013-10-17 12:15:10

by Timur Tabi

[permalink] [raw]
Subject: Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.

Xiubo Li wrote:
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + sai->base = devm_ioremap_resource(&pdev->dev, res);

Why not use of_iomap()?

2013-10-17 12:19:52

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.

On 10/17/2013 02:15 PM, Timur Tabi wrote:
> Xiubo Li wrote:
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + sai->base = devm_ioremap_resource(&pdev->dev, res);
>
> Why not use of_iomap()?

Because it won't check for conflicting resource regions.

- Lars

2013-10-17 13:22:24

by Timur Tabi

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.

Lars-Peter Clausen wrote:
>>> >>+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> >>+ sai->base = devm_ioremap_resource(&pdev->dev, res);
>> >
>> >Why not use of_iomap()?
> Because it won't check for conflicting resource regions.

Maybe I've been out of the loop for too long, but why is that a
particular problem with this driver?

2013-10-17 13:31:51

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.

On 10/17/2013 03:22 PM, Timur Tabi wrote:
> Lars-Peter Clausen wrote:
>>>> >>+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> >>+ sai->base = devm_ioremap_resource(&pdev->dev, res);
>>> >
>>> >Why not use of_iomap()?
>> Because it won't check for conflicting resource regions.
>
> Maybe I've been out of the loop for too long, but why is that a particular
> problem with this driver?

It is usually something you'd want to check in general to make sure that you
don't have multiple device that access the same iomem region at the same time.

- Lars

2013-10-17 13:37:30

by Timur Tabi

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.

Lars-Peter Clausen wrote:
>> >Maybe I've been out of the loop for too long, but why is that a particular
>> >problem with this driver?

> It is usually something you'd want to check in general to make sure that you
> don't have multiple device that access the same iomem region at the same time.

I understand that, but I'm trying to figure out why of_iomap() is okay
for hundreds of other drivers, but not this one. I've used it dozens of
times myself, without ever worrying about overlapping regions.

2013-10-17 13:49:56

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.

On 10/17/2013 03:37 PM, Timur Tabi wrote:
> Lars-Peter Clausen wrote:
>>> >Maybe I've been out of the loop for too long, but why is that a particular
>>> >problem with this driver?
>
>> It is usually something you'd want to check in general to make sure that you
>> don't have multiple device that access the same iomem region at the same
>> time.
>
> I understand that, but I'm trying to figure out why of_iomap() is okay for
> hundreds of other drivers, but not this one. I've used it dozens of times
> myself, without ever worrying about overlapping regions.

The driver would work fine with just of_iomap(). But the resource range
check comes basically for free and it does help to catch errors, so I'd
recommend on using it rather than not using it.

- Lars

2013-10-17 14:11:22

by Mark Brown

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.

On Thu, Oct 17, 2013 at 03:51:54PM +0200, Lars-Peter Clausen wrote:
> On 10/17/2013 03:37 PM, Timur Tabi wrote:

> > I understand that, but I'm trying to figure out why of_iomap() is okay for
> > hundreds of other drivers, but not this one. I've used it dozens of times
> > myself, without ever worrying about overlapping regions.

> The driver would work fine with just of_iomap(). But the resource range
> check comes basically for free and it does help to catch errors, so I'd
> recommend on using it rather than not using it.

There's also the fact that it's a devm_ function which means less error
handling code that we can break which is nice. There's probably a case
for an improved OF helper here...


Attachments:
(No filename) (710.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-10-17 17:41:14

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.

On 10/17/2013 11:01 AM, Xiubo Li wrote:
[...]
> +static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params,
> + struct snd_soc_dai *cpu_dai)
> +{
> + int ret;
> +
> + ret = fsl_sai_hw_params_tr(substream, params, cpu_dai,
> + FSL_FMT_TRANSMITTER);
> + if (ret) {
> + dev_err(cpu_dai->dev,
> + "Cannot set sai transmitter hw params: %d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = fsl_sai_hw_params_tr(substream, params, cpu_dai,
> + FSL_FMT_RECEIVER);
> + if (ret) {
> + dev_err(cpu_dai->dev,
> + "Cannot set sai's receiver hw params: %d\n",
> + ret);
> + return ret;
> + }

Shouldn't, depending on the substream direction, either transmit or receiver
be configured, instead of always configuring both?

> +
> + 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 + SAI_TCSR);
> + rcsr = readl(sai->base + SAI_RCSR);
> +
> + switch (cmd) {
> + case SNDRV_PCM_TRIGGER_START:
> + case SNDRV_PCM_TRIGGER_RESUME:
> + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> + rcsr |= SAI_CSR_TERE | SAI_CSR_FRDE;
> + tcsr |= SAI_CSR_TERE | SAI_CSR_FRDE;
> + writel(rcsr, sai->base + SAI_RCSR);
> + udelay(10);
> + writel(tcsr, sai->base + SAI_TCSR);
> + break;
> +
> + case SNDRV_PCM_TRIGGER_STOP:
> + case SNDRV_PCM_TRIGGER_SUSPEND:
> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> + tcsr &= ~(SAI_CSR_TERE | SAI_CSR_FRDE);
> + rcsr &= ~(SAI_CSR_TERE | SAI_CSR_FRDE);
> + writel(tcsr, sai->base + SAI_TCSR);
> + udelay(10);
> + writel(rcsr, sai->base + SAI_RCSR);
> + break;
> + default:
> + return -EINVAL;
> + }
> +

Same here, shouldn't tx and rx be started independently depending on the
substream direction?

> + return 0;
> +}
> +
> +static struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {

const

> + .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 const struct snd_soc_component_driver fsl_component = {
> + .name = "fsl-sai",
> +};
> +
> +static int fsl_sai_probe(struct platform_device *pdev)
> +{
[...]
> +
> + sai->dma_params_rx.addr = res->start + SAI_RDR;
> + sai->dma_params_rx.maxburst = 6;
> + index = of_property_match_string(np, "dma-names", "rx");
> + ret = of_parse_phandle_with_args(np, "dmas", "#dma-cells", index,
> + &dma_args);
> + if (ret)
> + return ret;
> + sai->dma_params_rx.slave_id = dma_args.args[1];
> +
> + sai->dma_params_tx.addr = res->start + SAI_TDR;
> + sai->dma_params_tx.maxburst = 6;
> + index = of_property_match_string(np, "dma-names", "tx");
> + ret = of_parse_phandle_with_args(np, "dmas", "#dma-cells", index,
> + &dma_args);
> + if (ret)
> + return ret;
> + sai->dma_params_tx.slave_id = dma_args.args[1];

The driver should not have to manually parse the dma devicetree properties,
this is something that should be handled by the dma engine driver.

> +
> + ret = snd_soc_register_component(&pdev->dev, &fsl_component,
> + &fsl_sai_dai, 1);
> + if (ret)
> + return ret;
> +
> + ret = fsl_pcm_dma_init(pdev);
> + if (ret)
> + goto out;
> +
> + platform_set_drvdata(pdev, sai);
> +
> + return 0;
> +
> +out:
> + snd_soc_unregister_component(&pdev->dev);
> + return ret;
> +}

2013-10-18 17:28:55

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCHv1 5/8] ASoC: sgtl5000: Revise the bugs about the sgt15000 codec.

On Thu, Oct 17, 2013 at 05:01:14PM +0800, Xiubo Li wrote:

> @@ -883,14 +883,19 @@ static int ldo_regulator_register(struct snd_soc_codec *codec,
> struct regulator_init_data *init_data,
> int voltage)
> {
> +#ifdef CONFIG_SND_SOC_FSL_SGTL5000
> + return 0;
> +#else
> dev_err(codec->dev, "this setup needs regulator support in the kernel\n");
> return -EINVAL;
> +#endif
> }

If these systems don't actually need the internal regulator then should
they not be trying to enable it? Alternatively if it's OK to ignore
this then why is this conditional in the board?

If this is something that it's safe to ignore then it should either be
ignored all the time or should be controlled by platform data not by a
compile time #define.


Attachments:
(No filename) (747.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-10-18 17:32:02

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCHv1 8/8] Documentation: Add device tree bindings for Freescale VF610 sound.

On Thu, Oct 17, 2013 at 05:01:17PM +0800, Xiubo Li wrote:

> + -- Power supplies:
> + * Mic Bias
> +
> + -- SGTL5000 pins:
> + * MIC_IN
> + * LINE_IN
> + * HP_OUT
> + * LINE_OUT

Things that are part of the CODEC should be part of the CODEC binding
and this binding should reference that - this way the information
doesn't have to be replicated by all boards using the CODEC and if new
devices are supported by the CODEC driver then only that needs updating
hopefully.


Attachments:
(No filename) (491.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-10-18 17:33:45

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCHv1 6/8] ASoC: fsl: add SGT15000 based audio machine driver.

On Thu, Oct 17, 2013 at 05:01:15PM +0800, Xiubo Li wrote:

> + ret = snd_soc_register_card(&fsl_sgt1500_card);
> + if (ret) {
> + dev_err(&pdev->dev, "register soc sound card failed :%d\n",
> + ret);
> + return ret;
> + }

Use the newly added devm_snd_soc_register_card() (in -next).


Attachments:
(No filename) (289.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-10-24 11:06:36

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.

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.


Attachments:
(No filename) (2.80 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments