* [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 9:01 ` Xiubo Li
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li @ 2013-10-17 9:01 UTC (permalink / raw)
To: r65073, timur, lgirdwood, broonie
Cc: r64188, rob.herring, pawel.moll, mark.rutland, swarren,
ian.campbell, rob, linux, perex, tiwai, grant.likely,
fabio.estevam, LW, oskar, shawn.guo, b42378, b18965, devicetree,
linux-doc, linux-kernel, linux-arm-kernel, alsa-devel,
linuxppc-dev
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 <b18965@freescale.com
Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
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, <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..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
^ permalink raw reply related [flat|nested] 158+ messages in thread* [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 9:01 ` Xiubo Li
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li @ 2013-10-17 9:01 UTC (permalink / raw)
To: linux-arm-kernel
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 <b18965 at freescale.com
Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
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, <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..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
^ permalink raw reply related [flat|nested] 158+ messages in thread* [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 9:01 ` Xiubo Li
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li @ 2013-10-17 9:01 UTC (permalink / raw)
To: r65073, timur, lgirdwood, broonie
Cc: mark.rutland, alsa-devel, linux-doc, tiwai, b18965, perex, LW,
linux, b42378, oskar, grant.likely, devicetree, ian.campbell,
pawel.moll, swarren, rob.herring, linux-arm-kernel, fabio.estevam,
linux-kernel, rob, r64188, shawn.guo, linuxppc-dev
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 <b18965@freescale.com
Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
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, <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..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
^ permalink raw reply related [flat|nested] 158+ messages in thread* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-17 9:01 ` Xiubo Li
(?)
(?)
@ 2013-10-17 9:42 ` Lothar Waßmann
-1 siblings, 0 replies; 158+ messages in thread
From: Lothar Waßmann @ 2013-10-17 9:42 UTC (permalink / raw)
To: Xiubo Li
Cc: r65073, timur, lgirdwood, broonie, r64188, rob.herring,
pawel.moll, mark.rutland, swarren, ian.campbell, rob, linux,
perex, tiwai, grant.likely, fabio.estevam, oskar, shawn.guo,
b42378, b18965, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, alsa-devel, linuxppc-dev
Hi,
Xiubo Li <Li.Xiubo@freescale.com> 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
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 158+ messages in thread* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 9:42 ` Lothar Waßmann
0 siblings, 0 replies; 158+ messages in thread
From: Lothar Waßmann @ 2013-10-17 9:42 UTC (permalink / raw)
To: Xiubo Li
Cc: r65073, timur, lgirdwood, broonie, r64188, rob.herring,
pawel.moll, mark.rutland, swarren, ian.campbell, rob, linux,
perex, tiwai, grant.likely, fabio.estevam, oskar, shawn.guo,
b42378, b18965, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, alsa-devel, linuxppc-dev
Hi,
Xiubo Li <Li.Xiubo@freescale.com> 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
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 158+ messages in thread* [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 9:42 ` Lothar Waßmann
0 siblings, 0 replies; 158+ messages in thread
From: Lothar Waßmann @ 2013-10-17 9:42 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Xiubo Li <Li.Xiubo@freescale.com> 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
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 158+ messages in thread* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 9:42 ` Lothar Waßmann
0 siblings, 0 replies; 158+ messages in thread
From: Lothar Waßmann @ 2013-10-17 9:42 UTC (permalink / raw)
To: Xiubo Li
Cc: mark.rutland, alsa-devel, linux-doc, tiwai, b18965, timur, perex,
r65073, linux, b42378, linux-arm-kernel, grant.likely, devicetree,
ian.campbell, pawel.moll, swarren, rob.herring, broonie, oskar,
fabio.estevam, lgirdwood, linux-kernel, rob, r64188, shawn.guo,
linuxppc-dev
Hi,
Xiubo Li <Li.Xiubo@freescale.com> 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 =3D 0;
> + struct device_node *np =3D pdev->dev.of_node;
> +
> + sai =3D devm_kzalloc(&pdev->dev, sizeof(*sai), GFP_KERNEL);
> + if (!sai)
> + return -ENOMEM;
> +
> + sai->fbt =3D FSL_SAI_FBT_MSB;
> +
> + res =3D platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + sai->base =3D devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(sai->base)) {
> + ret =3D PTR_ERR(sai->base);
> + return ret;
>
could be:
return PTR_ERR(sai->base);
[...]
> +static const struct of_device_id fsl_sai_ids[] =3D {
> + { .compatible =3D "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=C3=9Fmann
--=20
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra=C3=9Fe 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch=C3=A4ftsf=C3=BChrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 158+ messages in thread* RE: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-17 9:42 ` Lothar Waßmann
(?)
@ 2013-10-18 3:19 ` Xiubo Li-B47053
-1 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-18 3:19 UTC (permalink / raw)
To: Lothar Waßmann
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, perex@perex.cz, Guo Shawn-R65073,
linux@arm.linux.org.uk, Chen Guangyu-B42378,
linux-arm-kernel@lists.infradead.org, grant.likely@linaro.org,
devicetree@vger.kernel.org, ian.campbell@citrix.com,
pawel.moll@arm.com, swarren@wwwdotorg.org,
rob.herring@calxeda.com, broonie@kernel.org, osk
Hi,
> > + 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);
>
Yes，This looks better.
> > +#define FSL_SAI_DMABUF_SIZE (32 * 1024)
> > +#define TCD_NUMBER 4
> > +#define EDMA_PRIO_HIGH 6
> > +
> strange indentation with mixed spaces and tabs.
>
This will be revised in the next version.
> > +#define SAI_CR2_MSEL_MCLK3 (BIT(26)|BIT(27))
> >
> spaces around '|'?
>
And this too.
--
BRS,
Xiubo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 158+ messages in thread* [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-18 3:19 ` Xiubo Li-B47053
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-18 3:19 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
> > + 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);
>
Yes?This looks better.
> > +#define FSL_SAI_DMABUF_SIZE (32 * 1024)
> > +#define TCD_NUMBER 4
> > +#define EDMA_PRIO_HIGH 6
> > +
> strange indentation with mixed spaces and tabs.
>
This will be revised in the next version.
> > +#define SAI_CR2_MSEL_MCLK3 (BIT(26)|BIT(27))
> >
> spaces around '|'?
>
And this too.
--
BRS,
Xiubo
^ permalink raw reply [flat|nested] 158+ messages in thread* RE: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-18 3:19 ` Xiubo Li-B47053
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-18 3:19 UTC (permalink / raw)
To: Lothar Waßmann
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, perex@perex.cz, Guo Shawn-R65073,
linux@arm.linux.org.uk, Chen Guangyu-B42378,
linux-arm-kernel@lists.infradead.org, grant.likely@linaro.org,
devicetree@vger.kernel.org, ian.campbell@citrix.com,
pawel.moll@arm.com, swarren@wwwdotorg.org,
rob.herring@calxeda.com, broonie@kernel.org, oskar@scara.com,
Estevam Fabio-R49496, lgirdwood@gmail.com,
linux-kernel@vger.kernel.org, rob@landley.net,
Jin Zhengxiong-R64188, shawn.guo@linaro.org,
linuxppc-dev@lists.ozlabs.org
SGksDQoNCj4gPiArCXJlcyA9IHBsYXRmb3JtX2dldF9yZXNvdXJjZShwZGV2LCBJT1JFU09VUkNF
X01FTSwgMCk7DQo+ID4gKwlzYWktPmJhc2UgPSBkZXZtX2lvcmVtYXBfcmVzb3VyY2UoJnBkZXYt
PmRldiwgcmVzKTsNCj4gPiArCWlmIChJU19FUlIoc2FpLT5iYXNlKSkgew0KPiA+ICsJCXJldCA9
IFBUUl9FUlIoc2FpLT5iYXNlKTsNCj4gPiArCQlyZXR1cm4gcmV0Ow0KPiA+DQo+IGNvdWxkIGJl
Og0KPiAJCXJldHVybiBQVFJfRVJSKHNhaS0+YmFzZSk7DQo+DQoNClllc++8jFRoaXMgbG9va3Mg
YmV0dGVyLg0KDQogDQo+ID4gKyNkZWZpbmUgRlNMX1NBSV9ETUFCVUZfU0laRQkoMzIgKiAxMDI0
KQ0KPiA+ICsjZGVmaW5lIFRDRF9OVU1CRVIJCTQNCj4gPiArI2RlZmluZSBFRE1BX1BSSU9fSElH
SCAgICAgICAgICA2DQo+ID4gKw0KPiBzdHJhbmdlIGluZGVudGF0aW9uIHdpdGggbWl4ZWQgc3Bh
Y2VzIGFuZCB0YWJzLg0KPiANCg0KVGhpcyB3aWxsIGJlIHJldmlzZWQgaW4gdGhlIG5leHQgdmVy
c2lvbi4NCg0KDQoNCj4gPiArI2RlZmluZSBTQUlfQ1IyX01TRUxfTUNMSzMJKEJJVCgyNil8QklU
KDI3KSkNCj4gPg0KPiBzcGFjZXMgYXJvdW5kICd8Jz8NCj4gDQoNCkFuZCB0aGlzIHRvby4NCg0K
DQoNCg0KLS0NCkJSUywNClhpdWJvDQo=
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-17 9:01 ` Xiubo Li
(?)
(?)
@ 2013-10-17 12:15 ` Timur Tabi
-1 siblings, 0 replies; 158+ messages in thread
From: Timur Tabi @ 2013-10-17 12:15 UTC (permalink / raw)
To: Xiubo Li, r65073, lgirdwood, broonie
Cc: mark.rutland, alsa-devel, linux-doc, tiwai, b18965, LW, linux,
b42378, oskar, grant.likely, devicetree, ian.campbell, pawel.moll,
swarren, rob.herring, linux-arm-kernel, fabio.estevam,
linux-kernel, rob, r64188, shawn.guo, linuxppc-dev
Xiubo Li wrote:
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + sai->base = devm_ioremap_resource(&pdev->dev, res);
Why not use of_iomap()?
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 12:15 ` Timur Tabi
0 siblings, 0 replies; 158+ messages in thread
From: Timur Tabi @ 2013-10-17 12:15 UTC (permalink / raw)
To: Xiubo Li, r65073, lgirdwood, broonie
Cc: r64188, rob.herring, pawel.moll, mark.rutland, swarren,
ian.campbell, rob, linux, perex, tiwai, grant.likely,
fabio.estevam, LW, oskar, shawn.guo, b42378, b18965, devicetree,
linux-doc, linux-kernel, linux-arm-kernel, alsa-devel,
linuxppc-dev
Xiubo Li wrote:
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + sai->base = devm_ioremap_resource(&pdev->dev, res);
Why not use of_iomap()?
^ permalink raw reply [flat|nested] 158+ messages in thread
* [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 12:15 ` Timur Tabi
0 siblings, 0 replies; 158+ messages in thread
From: Timur Tabi @ 2013-10-17 12:15 UTC (permalink / raw)
To: linux-arm-kernel
Xiubo Li wrote:
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + sai->base = devm_ioremap_resource(&pdev->dev, res);
Why not use of_iomap()?
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 12:15 ` Timur Tabi
0 siblings, 0 replies; 158+ messages in thread
From: Timur Tabi @ 2013-10-17 12:15 UTC (permalink / raw)
To: Xiubo Li, r65073, lgirdwood, broonie
Cc: mark.rutland, alsa-devel, linux-doc, tiwai, b18965, perex, LW,
linux, b42378, oskar, grant.likely, devicetree, ian.campbell,
pawel.moll, swarren, rob.herring, linux-arm-kernel, fabio.estevam,
linux-kernel, rob, r64188, shawn.guo, linuxppc-dev
Xiubo Li wrote:
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + sai->base = devm_ioremap_resource(&pdev->dev, res);
Why not use of_iomap()?
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-17 12:15 ` Timur Tabi
(?)
@ 2013-10-17 12:21 ` Lars-Peter Clausen
-1 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-10-17 12:21 UTC (permalink / raw)
To: Timur Tabi
Cc: Xiubo Li, r65073, lgirdwood, broonie, mark.rutland, alsa-devel,
linux-doc, tiwai, b18965, LW, linux, b42378, oskar, grant.likely,
devicetree, ian.campbell, pawel.moll, swarren, rob.herring,
linux-arm-kernel, fabio.estevam, linux-kernel, rob, r64188,
shawn.guo, linuxppc-dev
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
^ permalink raw reply [flat|nested] 158+ messages in thread
* [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 12:21 ` Lars-Peter Clausen
0 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-10-17 12:21 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 12:21 ` Lars-Peter Clausen
0 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-10-17 12:21 UTC (permalink / raw)
To: Timur Tabi
Cc: mark.rutland, alsa-devel, linux-doc, tiwai, b18965, linux-kernel,
r65073, LW, linux, b42378, Xiubo Li, oskar, grant.likely,
devicetree, ian.campbell, pawel.moll, swarren, rob.herring,
broonie, linux-arm-kernel, fabio.estevam, lgirdwood, rob, r64188,
shawn.guo, linuxppc-dev
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
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-17 12:21 ` Lars-Peter Clausen
(?)
@ 2013-10-17 13:22 ` Timur Tabi
-1 siblings, 0 replies; 158+ messages in thread
From: Timur Tabi @ 2013-10-17 13:22 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Xiubo Li, r65073, lgirdwood, broonie, mark.rutland, alsa-devel,
linux-doc, tiwai, b18965, LW, linux, b42378, oskar, grant.likely,
devicetree, ian.campbell, pawel.moll, swarren, rob.herring,
linux-arm-kernel, fabio.estevam, linux-kernel, rob, r64188,
shawn.guo, linuxppc-dev
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?
^ permalink raw reply [flat|nested] 158+ messages in thread
* [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 13:22 ` Timur Tabi
0 siblings, 0 replies; 158+ messages in thread
From: Timur Tabi @ 2013-10-17 13:22 UTC (permalink / raw)
To: linux-arm-kernel
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?
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 13:22 ` Timur Tabi
0 siblings, 0 replies; 158+ messages in thread
From: Timur Tabi @ 2013-10-17 13:22 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: mark.rutland, alsa-devel, linux-doc, tiwai, b18965, linux-kernel,
r65073, LW, linux, b42378, Xiubo Li, oskar, grant.likely,
devicetree, ian.campbell, pawel.moll, swarren, rob.herring,
broonie, linux-arm-kernel, fabio.estevam, lgirdwood, rob, r64188,
shawn.guo, linuxppc-dev
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?
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-17 13:22 ` Timur Tabi
(?)
@ 2013-10-17 13:33 ` Lars-Peter Clausen
-1 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-10-17 13:33 UTC (permalink / raw)
To: Timur Tabi
Cc: Xiubo Li, r65073, lgirdwood, broonie, mark.rutland, alsa-devel,
linux-doc, tiwai, b18965, LW, linux, b42378, oskar, grant.likely,
devicetree, ian.campbell, pawel.moll, swarren, rob.herring,
linux-arm-kernel, fabio.estevam, linux-kernel, rob, r64188,
shawn.guo, linuxppc-dev
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
^ permalink raw reply [flat|nested] 158+ messages in thread
* [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 13:33 ` Lars-Peter Clausen
0 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-10-17 13:33 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 13:33 ` Lars-Peter Clausen
0 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-10-17 13:33 UTC (permalink / raw)
To: Timur Tabi
Cc: mark.rutland, alsa-devel, linux-doc, tiwai, b18965, linux-kernel,
r65073, LW, linux, b42378, Xiubo Li, oskar, grant.likely,
devicetree, ian.campbell, pawel.moll, swarren, rob.herring,
broonie, linux-arm-kernel, fabio.estevam, lgirdwood, rob, r64188,
shawn.guo, linuxppc-dev
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
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-17 13:33 ` Lars-Peter Clausen
(?)
@ 2013-10-17 13:37 ` Timur Tabi
-1 siblings, 0 replies; 158+ messages in thread
From: Timur Tabi @ 2013-10-17 13:37 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Xiubo Li, r65073, lgirdwood, broonie, mark.rutland, alsa-devel,
linux-doc, tiwai, b18965, LW, linux, b42378, oskar, grant.likely,
devicetree, ian.campbell, pawel.moll, swarren, rob.herring,
linux-arm-kernel, fabio.estevam, linux-kernel, rob, r64188,
shawn.guo, linuxppc-dev
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.
^ permalink raw reply [flat|nested] 158+ messages in thread
* [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 13:37 ` Timur Tabi
0 siblings, 0 replies; 158+ messages in thread
From: Timur Tabi @ 2013-10-17 13:37 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 13:37 ` Timur Tabi
0 siblings, 0 replies; 158+ messages in thread
From: Timur Tabi @ 2013-10-17 13:37 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: mark.rutland, alsa-devel, linux-doc, tiwai, b18965, linux-kernel,
r65073, LW, linux, b42378, Xiubo Li, oskar, grant.likely,
devicetree, ian.campbell, pawel.moll, swarren, rob.herring,
broonie, linux-arm-kernel, fabio.estevam, lgirdwood, rob, r64188,
shawn.guo, linuxppc-dev
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.
^ permalink raw reply [flat|nested] 158+ messages in thread
[parent not found: <525FE815.1040300-N01EOCouUvQ@public.gmane.org>]
* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-17 13:37 ` Timur Tabi
(?)
(?)
@ 2013-10-17 13:51 ` Lars-Peter Clausen
-1 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-10-17 13:51 UTC (permalink / raw)
To: Timur Tabi
Cc: Xiubo Li, r65073-KZfg59tc24xl57MIdRCFDg,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
linux-doc-u79uwXL29TY76Z2rM5mHXA, tiwai-l3A5Bk7waGM,
b18965-KZfg59tc24xl57MIdRCFDg,
LW-bxm8fMRDkQLDiMYJYoSAnRvVK+yQ3ZXh, linux-lFZ/pmaqli7XmaaqVzeoHQ,
b42378-KZfg59tc24xl57MIdRCFDg, oskar-fYPSZ7JpQqsAvxtiuMwx3w,
grant.likely-QSEj5FYQhm4dnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA,
ian.campbell-Sxgqhf6Nn4DQT0dZR+AlfA, pawel.moll-5wv7dgnIgG8,
swarren-3lzwWm7+Weoh9ZMKESR00Q,
rob.herring-bsGFqQB8/DxBDgjK7y7TUQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
fabio.estevam-KZfg59tc24xl57MIdRCFDg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, rob-VoJi6FS/r0vR7s880joybQ,
r64188-KZfg59tc24xl57MIdRCFDg, shawn.guo-QSEj5FYQhm4dnm+yROfE0A,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 13:51 ` Lars-Peter Clausen
0 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-10-17 13:51 UTC (permalink / raw)
To: Timur Tabi
Cc: Xiubo Li, r65073, lgirdwood, broonie, mark.rutland, alsa-devel,
linux-doc, tiwai, b18965, LW, linux, b42378, oskar, grant.likely,
devicetree, ian.campbell, pawel.moll, swarren, rob.herring,
linux-arm-kernel, fabio.estevam, linux-kernel, rob, r64188,
shawn.guo, linuxppc-dev
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
^ permalink raw reply [flat|nested] 158+ messages in thread
* [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 13:51 ` Lars-Peter Clausen
0 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-10-17 13:51 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 13:51 ` Lars-Peter Clausen
0 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-10-17 13:51 UTC (permalink / raw)
To: Timur Tabi
Cc: mark.rutland, alsa-devel, linux-doc, tiwai, b18965, linux-kernel,
r65073, LW, linux, b42378, Xiubo Li, oskar, grant.likely,
devicetree, ian.campbell, pawel.moll, swarren, rob.herring,
broonie, linux-arm-kernel, fabio.estevam, lgirdwood, rob, r64188,
shawn.guo, linuxppc-dev
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
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-17 13:51 ` Lars-Peter Clausen
(?)
(?)
@ 2013-10-17 14:10 ` Mark Brown
-1 siblings, 0 replies; 158+ messages in thread
From: Mark Brown @ 2013-10-17 14:10 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: mark.rutland, alsa-devel, linux-doc, tiwai, b18965, Timur Tabi,
lgirdwood, r65073, LW, linux, b42378, Xiubo Li, oskar,
grant.likely, devicetree, ian.campbell, pawel.moll, swarren,
rob.herring, linux-arm-kernel, fabio.estevam, linux-kernel, rob,
r64188, shawn.guo, linuxppc-dev
[-- Attachment #1.1: Type: text/plain, Size: 710 bytes --]
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...
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 14:10 ` Mark Brown
0 siblings, 0 replies; 158+ messages in thread
From: Mark Brown @ 2013-10-17 14:10 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Timur Tabi, Xiubo Li, r65073, lgirdwood, mark.rutland, alsa-devel,
linux-doc, tiwai, b18965, LW, linux, b42378, oskar, grant.likely,
devicetree, ian.campbell, pawel.moll, swarren, rob.herring,
linux-arm-kernel, fabio.estevam, linux-kernel, rob, r64188,
shawn.guo, linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 710 bytes --]
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...
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 158+ messages in thread
* [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 14:10 ` Mark Brown
0 siblings, 0 replies; 158+ messages in thread
From: Mark Brown @ 2013-10-17 14:10 UTC (permalink / raw)
To: linux-arm-kernel
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...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131017/204d832d/attachment.sig>
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 14:10 ` Mark Brown
0 siblings, 0 replies; 158+ messages in thread
From: Mark Brown @ 2013-10-17 14:10 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: mark.rutland, alsa-devel, linux-doc, tiwai, b18965, Timur Tabi,
lgirdwood, r65073, LW, linux, b42378, Xiubo Li, oskar,
grant.likely, devicetree, ian.campbell, pawel.moll, swarren,
rob.herring, linux-arm-kernel, fabio.estevam, linux-kernel, rob,
r64188, shawn.guo, linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 710 bytes --]
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...
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 158+ messages in thread
* RE: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-17 14:10 ` Mark Brown
(?)
@ 2013-10-18 3:42 ` Xiubo Li-B47053
-1 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-18 3:42 UTC (permalink / raw)
To: Mark Brown, Lars-Peter Clausen
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
Timur Tabi, linux-kernel@vger.kernel.org, Guo Shawn-R65073,
LW@KARO-electronics.de, linux@arm.linux.org.uk,
Chen Guangyu-B42378, oskar@scara.com, grant.likely@linaro.org,
devicetree@vger.kernel.org, ian.campbell@citrix.com,
pawel.moll@arm.com, swarren@wwwdotorg.org,
rob.herring@calxeda.com, linux-arm-ker
> > > 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...
Using this instead of of_iomap() is because "devm_" and resource range check
as Lars and Mark said, and there are more than one SAI device here which will
be added later, maybe the resource range check is needed.
Thanks.
--
BRS
Xiubo
^ permalink raw reply [flat|nested] 158+ messages in thread
* [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-18 3:42 ` Xiubo Li-B47053
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-18 3:42 UTC (permalink / raw)
To: linux-arm-kernel
> > > 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...
Using this instead of of_iomap() is because "devm_" and resource range check
as Lars and Mark said, and there are more than one SAI device here which will
be added later, maybe the resource range check is needed.
Thanks.
--
BRS
Xiubo
^ permalink raw reply [flat|nested] 158+ messages in thread
* RE: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-18 3:42 ` Xiubo Li-B47053
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-18 3:42 UTC (permalink / raw)
To: Mark Brown, Lars-Peter Clausen
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
Timur Tabi, linux-kernel@vger.kernel.org, Guo Shawn-R65073,
LW@KARO-electronics.de, linux@arm.linux.org.uk,
Chen Guangyu-B42378, oskar@scara.com, grant.likely@linaro.org,
devicetree@vger.kernel.org, ian.campbell@citrix.com,
pawel.moll@arm.com, swarren@wwwdotorg.org,
rob.herring@calxeda.com, linux-arm-kernel@lists.infradead.org,
Estevam Fabio-R49496, lgirdwood@gmail.com, rob@landley.net,
Jin Zhengxiong-R64188, shawn.guo@linaro.org,
linuxppc-dev@lists.ozlabs.org
> > > 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.
>=20
> > 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.
>=20
> 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...
Using this instead of of_iomap() is because "devm_" and resource range chec=
k
as Lars and Mark said, and there are more than one SAI device here which wi=
ll
be added later, maybe the resource range check is needed.
Thanks.
--
BRS
Xiubo
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-17 9:01 ` Xiubo Li
(?)
(?)
@ 2013-10-17 17:43 ` Lars-Peter Clausen
-1 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-10-17 17:43 UTC (permalink / raw)
To: Xiubo Li
Cc: mark.rutland, alsa-devel, linux-doc, tiwai, b18965, timur,
lgirdwood, r65073, LW, linux, b42378, oskar, grant.likely,
devicetree, ian.campbell, pawel.moll, swarren, rob.herring,
broonie, linux-arm-kernel, fabio.estevam, linux-kernel, rob,
r64188, shawn.guo, linuxppc-dev
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;
> +}
^ permalink raw reply [flat|nested] 158+ messages in thread* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 17:43 ` Lars-Peter Clausen
0 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-10-17 17:43 UTC (permalink / raw)
To: Xiubo Li
Cc: r65073, timur, lgirdwood, broonie, mark.rutland, alsa-devel,
linux-doc, tiwai, b18965, LW, linux, b42378, oskar, grant.likely,
devicetree, ian.campbell, pawel.moll, swarren, rob.herring,
linux-arm-kernel, fabio.estevam, linux-kernel, rob, r64188,
shawn.guo, linuxppc-dev
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;
> +}
^ permalink raw reply [flat|nested] 158+ messages in thread* [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 17:43 ` Lars-Peter Clausen
0 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-10-17 17:43 UTC (permalink / raw)
To: linux-arm-kernel
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;
> +}
^ permalink raw reply [flat|nested] 158+ messages in thread* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-17 17:43 ` Lars-Peter Clausen
0 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-10-17 17:43 UTC (permalink / raw)
To: Xiubo Li
Cc: mark.rutland, alsa-devel, linux-doc, tiwai, b18965, timur,
lgirdwood, r65073, LW, linux, b42378, oskar, grant.likely,
devicetree, ian.campbell, pawel.moll, swarren, rob.herring,
broonie, linux-arm-kernel, fabio.estevam, linux-kernel, rob,
r64188, shawn.guo, linuxppc-dev
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;
> +}
^ permalink raw reply [flat|nested] 158+ messages in thread* RE: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-17 17:43 ` Lars-Peter Clausen
(?)
@ 2013-10-21 6:59 ` Xiubo Li-B47053
-1 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-21 6:59 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, linux-kernel@vger.kernel.org, Guo Shawn-R65073,
LW@KARO-electronics.de, linux@arm.linux.org.uk,
Chen Guangyu-B42378, oskar@scara.com, grant.likely@linaro.org,
devicetree@vger.kernel.org, ian.campbell@citrix.com,
pawel.moll@arm.com, swarren@wwwdotorg.org,
rob.herring@calxeda.com, broonie
> > +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?
>
Yes, this can be configed separately. Please see the next version.
> > +
> > + 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?
>
But this couldn't, from the SAI's spec we can see that:
------
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:
* The transmitter must be configured for asynchronous operation and the receiver for
synchronous operation.
* In synchronous mode, the receiver is enabled only when both the transmitter and
receiver are enabled.
* 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:
* The receiver must be configured for asynchronous operation and the transmitter for
synchronous operation.
* In synchronous mode, the transmitter is enabled only when both the receiver and
transmitter are both enabled.
* It is recommended that the receiver is the last enabled and the first disabled.
------
The receiver and transmitter should be both enabled at the same time if any of them is alive.
> > + return 0;
> > +}
> > +
> > +static struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
>
> const
>
Please see the next version.
^ permalink raw reply [flat|nested] 158+ messages in thread* [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-21 6:59 ` Xiubo Li-B47053
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-21 6:59 UTC (permalink / raw)
To: linux-arm-kernel
> > +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?
>
Yes, this can be configed separately. Please see the next version.
> > +
> > + 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?
>
But this couldn't, from the SAI's spec we can see that:
------
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:
* The transmitter must be configured for asynchronous operation and the receiver for
synchronous operation.
* In synchronous mode, the receiver is enabled only when both the transmitter and
receiver are enabled.
* 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:
* The receiver must be configured for asynchronous operation and the transmitter for
synchronous operation.
* In synchronous mode, the transmitter is enabled only when both the receiver and
transmitter are both enabled.
* It is recommended that the receiver is the last enabled and the first disabled.
------
The receiver and transmitter should be both enabled at the same time if any of them is alive.
> > + return 0;
> > +}
> > +
> > +static struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
>
> const
>
Please see the next version.
^ permalink raw reply [flat|nested] 158+ messages in thread* RE: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-21 6:59 ` Xiubo Li-B47053
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-21 6:59 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, linux-kernel@vger.kernel.org, Guo Shawn-R65073,
LW@KARO-electronics.de, linux@arm.linux.org.uk,
Chen Guangyu-B42378, oskar@scara.com, grant.likely@linaro.org,
devicetree@vger.kernel.org, ian.campbell@citrix.com,
pawel.moll@arm.com, swarren@wwwdotorg.org,
rob.herring@calxeda.com, broonie@kernel.org,
linux-arm-kernel@lists.infradead.org, Estevam Fabio-R49496,
lgirdwood@gmail.com, rob@landley.net, Jin Zhengxiong-R64188,
shawn.guo@linaro.org, linuxppc-dev@lists.ozlabs.org
> > +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 =3D 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 =3D 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;
> > + }
>=20
> Shouldn't, depending on the substream direction, either transmit or
> receiver be configured, instead of always configuring both?
>=20
Yes, this can be configed separately. Please see the next version.
> > +
> > + return 0;
> > +}
> > +
> > +static int fsl_sai_trigger(struct snd_pcm_substream *substream, int
> cmd,
> > + struct snd_soc_dai *dai)
> > +{
> > + struct fsl_sai *sai =3D snd_soc_dai_get_drvdata(dai);
> > + unsigned int tcsr, rcsr;
> > +
> > + tcsr =3D readl(sai->base + SAI_TCSR);
> > + rcsr =3D readl(sai->base + SAI_RCSR);
> > +
> > + switch (cmd) {
> > + case SNDRV_PCM_TRIGGER_START:
> > + case SNDRV_PCM_TRIGGER_RESUME:
> > + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> > + rcsr |=3D SAI_CSR_TERE | SAI_CSR_FRDE;
> > + tcsr |=3D 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 &=3D ~(SAI_CSR_TERE | SAI_CSR_FRDE);
> > + rcsr &=3D ~(SAI_CSR_TERE | SAI_CSR_FRDE);
> > + writel(tcsr, sai->base + SAI_TCSR);
> > + udelay(10);
> > + writel(rcsr, sai->base + SAI_RCSR);
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
>=20
> Same here, shouldn't tx and rx be started independently depending on the
> substream direction?
>=20
But this couldn't, from the SAI's spec we can see that:
------
The SAI transmitter and receiver can be configured to operate with synchron=
ous bit clock
and frame sync.
1),=20
If the transmitter bit clock and frame sync are to be used by both the tran=
smitter and
receiver:
* The transmitter must be configured for asynchronous operation and the rec=
eiver for
synchronous operation.
* In synchronous mode, the receiver is enabled only when both the transmitt=
er and
receiver are enabled.
* 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 transmi=
tter and
receiver:
* The receiver must be configured for asynchronous operation and the transm=
itter for
synchronous operation.
* In synchronous mode, the transmitter is enabled only when both the receiv=
er and
transmitter are both enabled.
* It is recommended that the receiver is the last enabled and the first dis=
abled.
------
The receiver and transmitter should be both enabled at the same time if any=
of them is alive.
> > + return 0;
> > +}
> > +
> > +static struct snd_soc_dai_ops fsl_sai_pcm_dai_ops =3D {
>=20
> const
>=20
Please see the next version.
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-17 17:43 ` Lars-Peter Clausen
(?)
@ 2013-10-22 2:20 ` Xiubo Li-B47053
-1 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-22 2:20 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, linux-kernel@vger.kernel.org, Guo Shawn-R65073,
LW@KARO-electronics.de, linux@arm.linux.org.uk,
Chen Guangyu-B42378, oskar@scara.com, grant.likely@linaro.org,
devicetree@vger.kernel.org, ian.campbell@citrix.com,
pawel.moll@arm.com, swarren@wwwdotorg.org,
rob.herring@calxeda.com, broonie
> > +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.
>
Yes, the dma engine interface has already parsed the slave_id while
the dma customer requesting one dma channel.
Though this also could be a way to pass the slave_id to dma driver, but the
dma driver uses the way while requesting dma channels.
So I'll drop this code later.
^ permalink raw reply [flat|nested] 158+ messages in thread* [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-22 2:20 ` Xiubo Li-B47053
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-22 2:20 UTC (permalink / raw)
To: linux-arm-kernel
> > +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.
>
Yes, the dma engine interface has already parsed the slave_id while
the dma customer requesting one dma channel.
Though this also could be a way to pass the slave_id to dma driver, but the
dma driver uses the way while requesting dma channels.
So I'll drop this code later.
^ permalink raw reply [flat|nested] 158+ messages in thread* RE: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-22 2:20 ` Xiubo Li-B47053
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-22 2:20 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, linux-kernel@vger.kernel.org, Guo Shawn-R65073,
LW@KARO-electronics.de, linux@arm.linux.org.uk,
Chen Guangyu-B42378, oskar@scara.com, grant.likely@linaro.org,
devicetree@vger.kernel.org, ian.campbell@citrix.com,
pawel.moll@arm.com, swarren@wwwdotorg.org,
rob.herring@calxeda.com, broonie@kernel.org,
linux-arm-kernel@lists.infradead.org, Estevam Fabio-R49496,
lgirdwood@gmail.com, rob@landley.net, Jin Zhengxiong-R64188,
shawn.guo@linaro.org, linuxppc-dev@lists.ozlabs.org
> > +static int fsl_sai_probe(struct platform_device *pdev) {
> [...]
> > +
> > + sai->dma_params_rx.addr =3D res->start + SAI_RDR;
> > + sai->dma_params_rx.maxburst =3D 6;
> > + index =3D of_property_match_string(np, "dma-names", "rx");
> > + ret =3D of_parse_phandle_with_args(np, "dmas", "#dma-cells", index,
> > + &dma_args);
> > + if (ret)
> > + return ret;
> > + sai->dma_params_rx.slave_id =3D dma_args.args[1];
> > +
> > + sai->dma_params_tx.addr =3D res->start + SAI_TDR;
> > + sai->dma_params_tx.maxburst =3D 6;
> > + index =3D of_property_match_string(np, "dma-names", "tx");
> > + ret =3D of_parse_phandle_with_args(np, "dmas", "#dma-cells", index,
> > + &dma_args);
> > + if (ret)
> > + return ret;
> > + sai->dma_params_tx.slave_id =3D dma_args.args[1];
>=20
> The driver should not have to manually parse the dma devicetree
> properties, this is something that should be handled by the dma engine
> driver.
>=20
Yes, the dma engine interface has already parsed the slave_id while
the dma customer requesting one dma channel.
Though this also could be a way to pass the slave_id to dma driver, but the=
=20
dma driver uses the way while requesting dma channels.
So I'll drop this code later.
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-17 17:43 ` Lars-Peter Clausen
(?)
@ 2013-10-28 5:58 ` Xiubo Li-B47053
-1 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-28 5:58 UTC (permalink / raw)
To: djbw@fb.com, vinod.koul@intel.com
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, linux-kernel@vger.kernel.org, Guo Shawn-R65073,
LW@KARO-electronics.de, Lars-Peter Clausen,
linux@arm.linux.org.uk, Chen Guangyu-B42378, oskar@scara.com,
grant.likely@linaro.org, devicetree@vger.kernel.org,
ian.campbell@citrix.com, pawel.moll@arm.com,
swarren@wwwdotorg.org, rob.herring
Hi Dan, Vinod,
> > +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.
>
What do you think about the DMA slave_id ?
I have been noticed by one colleague that this should be parsed here, which
is from your opinions ?
> > +
> > + 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;
^ permalink raw reply [flat|nested] 158+ messages in thread* [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-28 5:58 ` Xiubo Li-B47053
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-28 5:58 UTC (permalink / raw)
To: linux-arm-kernel
Hi Dan, Vinod,
> > +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.
>
What do you think about the DMA slave_id ?
I have been noticed by one colleague that this should be parsed here, which
is from your opinions ?
> > +
> > + 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;
^ permalink raw reply [flat|nested] 158+ messages in thread* RE: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-28 5:58 ` Xiubo Li-B47053
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-28 5:58 UTC (permalink / raw)
To: djbw@fb.com, vinod.koul@intel.com
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, linux-kernel@vger.kernel.org, Guo Shawn-R65073,
LW@KARO-electronics.de, Lars-Peter Clausen,
linux@arm.linux.org.uk, Chen Guangyu-B42378, oskar@scara.com,
grant.likely@linaro.org, devicetree@vger.kernel.org,
ian.campbell@citrix.com, pawel.moll@arm.com,
swarren@wwwdotorg.org, rob.herring@calxeda.com,
broonie@kernel.org, linux-arm-kernel@lists.infradead.org,
Estevam Fabio-R49496, lgirdwood@gmail.com, rob@landley.net,
Jin Zhengxiong-R64188, shawn.guo@linaro.org,
linuxppc-dev@lists.ozlabs.org
Hi Dan, Vinod,
> > +static int fsl_sai_probe(struct platform_device *pdev) {
> [...]
> > +
> > + sai->dma_params_rx.addr =3D res->start + SAI_RDR;
> > + sai->dma_params_rx.maxburst =3D 6;
> > + index =3D of_property_match_string(np, "dma-names", "rx");
> > + ret =3D of_parse_phandle_with_args(np, "dmas", "#dma-cells", index,
> > + &dma_args);
> > + if (ret)
> > + return ret;
> > + sai->dma_params_rx.slave_id =3D dma_args.args[1];
> > +
> > + sai->dma_params_tx.addr =3D res->start + SAI_TDR;
> > + sai->dma_params_tx.maxburst =3D 6;
> > + index =3D of_property_match_string(np, "dma-names", "tx");
> > + ret =3D of_parse_phandle_with_args(np, "dmas", "#dma-cells", index,
> > + &dma_args);
> > + if (ret)
> > + return ret;
> > + sai->dma_params_tx.slave_id =3D dma_args.args[1];
>=20
> The driver should not have to manually parse the dma devicetree
> properties, this is something that should be handled by the dma engine
> driver.
>=20
What do you think about the DMA slave_id ?
I have been noticed by one colleague that this should be parsed here, which
is from your opinions ?
> > +
> > + ret =3D snd_soc_register_component(&pdev->dev, &fsl_component,
> > + &fsl_sai_dai, 1);
> > + if (ret)
> > + return ret;
> > +
> > + ret =3D fsl_pcm_dma_init(pdev);
> > + if (ret)
> > + goto out;
^ permalink raw reply [flat|nested] 158+ messages in thread* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-28 5:58 ` Xiubo Li-B47053
(?)
@ 2013-11-12 5:02 ` Vinod Koul
-1 siblings, 0 replies; 158+ messages in thread
From: Vinod Koul @ 2013-11-12 5:02 UTC (permalink / raw)
To: Xiubo Li-B47053
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, linux-kernel@vger.kernel.org, Guo Shawn-R65073,
LW@KARO-electronics.de, Lars-Peter Clausen,
linux@arm.linux.org.uk, Chen Guangyu-B42378, oskar@scara.com,
grant.likely@linaro.org, devicetree@vger.kernel.org,
ian.campbell@citrix.com, pawel.moll@arm.com,
swarren@wwwdotorg.org, rob.herring
On Mon, Oct 28, 2013 at 05:58:42AM +0000, Xiubo Li-B47053 wrote:
> Hi Dan, Vinod,
>
>
> > > +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.
> >
>
> What do you think about the DMA slave_id ?
> I have been noticed by one colleague that this should be parsed here, which
> is from your opinions ?
Sure slave_id can be parsed here, but IMO it should be programmed via the
dma_slave_confog into the respective channel
--
~Vinod
>
>
> > > +
> > > + 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;
>
>
--
^ permalink raw reply [flat|nested] 158+ messages in thread* [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-11-12 5:02 ` Vinod Koul
0 siblings, 0 replies; 158+ messages in thread
From: Vinod Koul @ 2013-11-12 5:02 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Oct 28, 2013 at 05:58:42AM +0000, Xiubo Li-B47053 wrote:
> Hi Dan, Vinod,
>
>
> > > +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.
> >
>
> What do you think about the DMA slave_id ?
> I have been noticed by one colleague that this should be parsed here, which
> is from your opinions ?
Sure slave_id can be parsed here, but IMO it should be programmed via the
dma_slave_confog into the respective channel
--
~Vinod
>
>
> > > +
> > > + 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;
>
>
--
^ permalink raw reply [flat|nested] 158+ messages in thread* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-11-12 5:02 ` Vinod Koul
0 siblings, 0 replies; 158+ messages in thread
From: Vinod Koul @ 2013-11-12 5:02 UTC (permalink / raw)
To: Xiubo Li-B47053
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, linux-kernel@vger.kernel.org, Guo Shawn-R65073,
LW@KARO-electronics.de, Lars-Peter Clausen,
linux@arm.linux.org.uk, Chen Guangyu-B42378, oskar@scara.com,
grant.likely@linaro.org, devicetree@vger.kernel.org,
ian.campbell@citrix.com, pawel.moll@arm.com,
swarren@wwwdotorg.org, rob.herring@calxeda.com,
broonie@kernel.org, linux-arm-kernel@lists.infradead.org,
Estevam Fabio-R49496, lgirdwood@gmail.com, rob@landley.net,
djbw@fb.com, Jin Zhengxiong-R64188, shawn.guo@linaro.org,
linuxppc-dev@lists.ozlabs.org
On Mon, Oct 28, 2013 at 05:58:42AM +0000, Xiubo Li-B47053 wrote:
> Hi Dan, Vinod,
>
>
> > > +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.
> >
>
> What do you think about the DMA slave_id ?
> I have been noticed by one colleague that this should be parsed here, which
> is from your opinions ?
Sure slave_id can be parsed here, but IMO it should be programmed via the
dma_slave_confog into the respective channel
--
~Vinod
>
>
> > > +
> > > + 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;
>
>
--
^ permalink raw reply [flat|nested] 158+ messages in thread* RE: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-11-12 5:02 ` Vinod Koul
(?)
@ 2013-11-12 7:35 ` Li Xiubo
-1 siblings, 0 replies; 158+ messages in thread
From: Li Xiubo @ 2013-11-12 7:35 UTC (permalink / raw)
To: Vinod Koul
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, timur@tabi.org,
linux-kernel@vger.kernel.org, Huan Wang, LW@KARO-electronics.de,
Lars-Peter Clausen, linux@arm.linux.org.uk,
linux-arm-kernel@lists.infradead.org, grant.likely@linaro.org,
devicetree@vger.kernel.org, ian.campbell@citrix.com,
pawel.moll@arm.com, swarren@wwwdotorg.org, Shawn Guo, djbw@fb.com,
rob.
> > > > +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.
> > >
> >
> > What do you think about the DMA slave_id ?
> > I have been noticed by one colleague that this should be parsed here,
> > which is from your opinions ?
> Sure slave_id can be parsed here, but IMO it should be programmed via the
> dma_slave_confog into the respective channel
>
Actually, these are parsed for cpu_dai->playback_dma_data and cpu_dai->capture_dma_data dynamically, whose type is struct dma_slave_config.
And now I must parse them here, because the platform eDMA driver's newest version will check and use the slave_ids to select and configure the eDMA channels via dma_device->device_control().
--
Xiubo
^ permalink raw reply [flat|nested] 158+ messages in thread* [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-11-12 7:35 ` Li Xiubo
0 siblings, 0 replies; 158+ messages in thread
From: Li Xiubo @ 2013-11-12 7:35 UTC (permalink / raw)
To: linux-arm-kernel
> > > > +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.
> > >
> >
> > What do you think about the DMA slave_id ?
> > I have been noticed by one colleague that this should be parsed here,
> > which is from your opinions ?
> Sure slave_id can be parsed here, but IMO it should be programmed via the
> dma_slave_confog into the respective channel
>
Actually, these are parsed for cpu_dai->playback_dma_data and cpu_dai->capture_dma_data dynamically, whose type is struct dma_slave_config.
And now I must parse them here, because the platform eDMA driver's newest version will check and use the slave_ids to select and configure the eDMA channels via dma_device->device_control().
--
Xiubo
^ permalink raw reply [flat|nested] 158+ messages in thread* RE: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-11-12 7:35 ` Li Xiubo
0 siblings, 0 replies; 158+ messages in thread
From: Li Xiubo @ 2013-11-12 7:35 UTC (permalink / raw)
To: Vinod Koul, lars@metafoo.de
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, timur@tabi.org,
linux-kernel@vger.kernel.org, Huan Wang, LW@KARO-electronics.de,
Lars-Peter Clausen, linux@arm.linux.org.uk,
linux-arm-kernel@lists.infradead.org, grant.likely@linaro.org,
devicetree@vger.kernel.org, ian.campbell@citrix.com,
pawel.moll@arm.com, swarren@wwwdotorg.org, Shawn Guo, djbw@fb.com,
rob.herring@calxeda.com, broonie@kernel.org, Zhengxiong Jin,
oskar@scara.com, Fabio Estevam, lgirdwood@gmail.com,
rob@landley.net, Guangyu Chen, shawn.guo@linaro.org,
linuxppc-dev@lists.ozlabs.org
> > > > +static int fsl_sai_probe(struct platform_device *pdev) {
> > > [...]
> > > > +
> > > > + sai->dma_params_rx.addr =3D res->start + SAI_RDR;
> > > > + sai->dma_params_rx.maxburst =3D 6;
> > > > + index =3D of_property_match_string(np, "dma-names", "rx");
> > > > + ret =3D of_parse_phandle_with_args(np, "dmas", "#dma-cells",
> index,
> > > > + &dma_args);
> > > > + if (ret)
> > > > + return ret;
> > > > + sai->dma_params_rx.slave_id =3D dma_args.args[1];
> > > > +
> > > > + sai->dma_params_tx.addr =3D res->start + SAI_TDR;
> > > > + sai->dma_params_tx.maxburst =3D 6;
> > > > + index =3D of_property_match_string(np, "dma-names", "tx");
> > > > + ret =3D of_parse_phandle_with_args(np, "dmas", "#dma-cells",
> index,
> > > > + &dma_args);
> > > > + if (ret)
> > > > + return ret;
> > > > + sai->dma_params_tx.slave_id =3D 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.
> > >
> >
> > What do you think about the DMA slave_id ?
> > I have been noticed by one colleague that this should be parsed here,
> > which is from your opinions ?
> Sure slave_id can be parsed here, but IMO it should be programmed via the
> dma_slave_confog into the respective channel
>=20
Actually, these are parsed for cpu_dai->playback_dma_data and cpu_dai->capt=
ure_dma_data dynamically, whose type is struct dma_slave_config.
And now I must parse them here, because the platform eDMA driver's newest v=
ersion will check and use the slave_ids to select and configure the eDMA ch=
annels via dma_device->device_control().=20
--
Xiubo
^ permalink raw reply [flat|nested] 158+ messages in thread* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-11-12 7:35 ` Li Xiubo
(?)
@ 2013-11-12 7:59 ` Lars-Peter Clausen
-1 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-11-12 7:59 UTC (permalink / raw)
To: Li Xiubo
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, timur@tabi.org,
linux-kernel@vger.kernel.org, Huan Wang, LW@KARO-electronics.de,
linux@arm.linux.org.uk, Vinod Koul,
linux-arm-kernel@lists.infradead.org, grant.likely@linaro.org,
devicetree@vger.kernel.org, ian.campbell@citrix.com,
pawel.moll@arm.com, swarren@wwwdotorg.org, Shawn Guo, djbw@fb.com,
rob.her
On 11/12/2013 08:35 AM, Li Xiubo wrote:
>>>>> +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.
>>>>
>>>
>>> What do you think about the DMA slave_id ?
>>> I have been noticed by one colleague that this should be parsed here,
>>> which is from your opinions ?
>> Sure slave_id can be parsed here, but IMO it should be programmed via the
>> dma_slave_confog into the respective channel
>>
>
> Actually, these are parsed for cpu_dai->playback_dma_data and cpu_dai->capture_dma_data dynamically, whose type is struct dma_slave_config.
>
> And now I must parse them here, because the platform eDMA driver's newest version will check and use the slave_ids to select and configure the eDMA channels via dma_device->device_control().
Parsing them here is a layering violation. The format of the DMA specifier
depends on the DMA controller. A DMA slave should not make any assumptions
about how the specifier looks like, it should not even look at them. You should
fix the DMA controller driver to work without slave_id in the devicetree case.
- Lars
^ permalink raw reply [flat|nested] 158+ messages in thread* [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-11-12 7:59 ` Lars-Peter Clausen
0 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-11-12 7:59 UTC (permalink / raw)
To: linux-arm-kernel
On 11/12/2013 08:35 AM, Li Xiubo wrote:
>>>>> +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.
>>>>
>>>
>>> What do you think about the DMA slave_id ?
>>> I have been noticed by one colleague that this should be parsed here,
>>> which is from your opinions ?
>> Sure slave_id can be parsed here, but IMO it should be programmed via the
>> dma_slave_confog into the respective channel
>>
>
> Actually, these are parsed for cpu_dai->playback_dma_data and cpu_dai->capture_dma_data dynamically, whose type is struct dma_slave_config.
>
> And now I must parse them here, because the platform eDMA driver's newest version will check and use the slave_ids to select and configure the eDMA channels via dma_device->device_control().
Parsing them here is a layering violation. The format of the DMA specifier
depends on the DMA controller. A DMA slave should not make any assumptions
about how the specifier looks like, it should not even look at them. You should
fix the DMA controller driver to work without slave_id in the devicetree case.
- Lars
^ permalink raw reply [flat|nested] 158+ messages in thread* Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-11-12 7:59 ` Lars-Peter Clausen
0 siblings, 0 replies; 158+ messages in thread
From: Lars-Peter Clausen @ 2013-11-12 7:59 UTC (permalink / raw)
To: Li Xiubo
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, timur@tabi.org,
linux-kernel@vger.kernel.org, Huan Wang, LW@KARO-electronics.de,
linux@arm.linux.org.uk, Vinod Koul,
linux-arm-kernel@lists.infradead.org, grant.likely@linaro.org,
devicetree@vger.kernel.org, ian.campbell@citrix.com,
pawel.moll@arm.com, swarren@wwwdotorg.org, Shawn Guo, djbw@fb.com,
rob.herring@calxeda.com, broonie@kernel.org, Zhengxiong Jin,
oskar@scara.com, Fabio Estevam, lgirdwood@gmail.com,
rob@landley.net, Guangyu Chen, shawn.guo@linaro.org,
linuxppc-dev@lists.ozlabs.org
On 11/12/2013 08:35 AM, Li Xiubo wrote:
>>>>> +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.
>>>>
>>>
>>> What do you think about the DMA slave_id ?
>>> I have been noticed by one colleague that this should be parsed here,
>>> which is from your opinions ?
>> Sure slave_id can be parsed here, but IMO it should be programmed via the
>> dma_slave_confog into the respective channel
>>
>
> Actually, these are parsed for cpu_dai->playback_dma_data and cpu_dai->capture_dma_data dynamically, whose type is struct dma_slave_config.
>
> And now I must parse them here, because the platform eDMA driver's newest version will check and use the slave_ids to select and configure the eDMA channels via dma_device->device_control().
Parsing them here is a layering violation. The format of the DMA specifier
depends on the DMA controller. A DMA slave should not make any assumptions
about how the specifier looks like, it should not even look at them. You should
fix the DMA controller driver to work without slave_id in the devicetree case.
- Lars
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-17 9:01 ` Xiubo Li
(?)
@ 2013-10-24 11:05 ` Mark Brown
-1 siblings, 0 replies; 158+ messages in thread
From: Mark Brown @ 2013-10-24 11:05 UTC (permalink / raw)
To: Xiubo Li
Cc: r65073, timur, lgirdwood, r64188, rob.herring, pawel.moll,
mark.rutland, swarren, ian.campbell, rob, linux, perex, tiwai,
grant.likely, fabio.estevam, LW, oskar, shawn.guo, b42378, b18965,
devicetree, linux-doc, linux-kernel, linux-arm-kernel, alsa-devel,
linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 2866 bytes --]
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.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 158+ messages in thread* [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-24 11:05 ` Mark Brown
0 siblings, 0 replies; 158+ messages in thread
From: Mark Brown @ 2013-10-24 11:05 UTC (permalink / raw)
To: linux-arm-kernel
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131024/0827af43/attachment.sig>
^ permalink raw reply [flat|nested] 158+ messages in thread* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-24 11:05 ` Mark Brown
0 siblings, 0 replies; 158+ messages in thread
From: Mark Brown @ 2013-10-24 11:05 UTC (permalink / raw)
To: Xiubo Li
Cc: mark.rutland, alsa-devel, linux-doc, tiwai, b18965, timur, perex,
r65073, LW, linux, b42378, linux-arm-kernel, grant.likely,
devicetree, ian.campbell, pawel.moll, swarren, rob.herring, oskar,
fabio.estevam, lgirdwood, linux-kernel, rob, r64188, shawn.guo,
linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 2866 bytes --]
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.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 158+ messages in thread* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-24 11:05 ` Mark Brown
(?)
@ 2013-10-28 7:15 ` Xiubo Li-B47053
-1 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-28 7:15 UTC (permalink / raw)
To: Mark Brown
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, Guo Shawn-R65073, LW@KARO-electronics.de,
linux@arm.linux.org.uk, Chen Guangyu-B42378,
linux-arm-kernel@lists.infradead.org, grant.likely@linaro.org,
devicetree@vger.kernel.org, ian.campbell@citrix.com,
pawel.moll@arm.com, swarren@wwwdotorg.org,
rob.herring@calxeda.com, oskar@scara.com
> > +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.
>
I will do a research.
> > + 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.
>
As I have replied before, there is one function couldn't be separated for the hardware limitation.
> > +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.
>
Actully there are to "XXX_probe" functions and two "XXX_remove" functions:
fsl_sai_dai_probe() and fsl_sai_dai_remove() are callbacks of the ASoC subsystem.
And in fsl_sai_dai_probe() needs to read/write the SAI controller's registers, so
the clk_enable_prepare() must be here and clk_disable_unprepare() in fsl_sai_dai_remove().
fsl_sai_probe() and fsl_sai_remove() are the driver's probe and remove interfaces.
So the "+ clk_disable_unprepare(sai->clk);" sentence in fsl_sai_remove() will be removed later.
> > + 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.
>
See the next version.
> > +
> > + 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.
>
See the next version.
> > +#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.
>
See the next version.
^ permalink raw reply [flat|nested] 158+ messages in thread* [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-28 7:15 ` Xiubo Li-B47053
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-28 7:15 UTC (permalink / raw)
To: linux-arm-kernel
> > +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.
>
I will do a research.
> > + 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.
>
As I have replied before, there is one function couldn't be separated for the hardware limitation.
> > +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.
>
Actully there are to "XXX_probe" functions and two "XXX_remove" functions:
fsl_sai_dai_probe() and fsl_sai_dai_remove() are callbacks of the ASoC subsystem.
And in fsl_sai_dai_probe() needs to read/write the SAI controller's registers, so
the clk_enable_prepare() must be here and clk_disable_unprepare() in fsl_sai_dai_remove().
fsl_sai_probe() and fsl_sai_remove() are the driver's probe and remove interfaces.
So the "+ clk_disable_unprepare(sai->clk);" sentence in fsl_sai_remove() will be removed later.
> > + 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.
>
See the next version.
> > +
> > + 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.
>
See the next version.
> > +#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.
>
See the next version.
^ permalink raw reply [flat|nested] 158+ messages in thread* RE: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-28 7:15 ` Xiubo Li-B47053
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-28 7:15 UTC (permalink / raw)
To: Mark Brown
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, perex@perex.cz, Guo Shawn-R65073,
LW@KARO-electronics.de, linux@arm.linux.org.uk,
Chen Guangyu-B42378, linux-arm-kernel@lists.infradead.org,
grant.likely@linaro.org, devicetree@vger.kernel.org,
ian.campbell@citrix.com, pawel.moll@arm.com,
swarren@wwwdotorg.org, rob.herring@calxeda.com, oskar@scara.com,
Estevam Fabio-R49496, lgirdwood@gmail.com,
linux-kernel@vger.kernel.org, rob@landley.net,
Jin Zhengxiong-R64188, shawn.guo@linaro.org,
linuxppc-dev@lists.ozlabs.org
> > +static struct snd_pcm_hardware snd_fsl_hardware =3D {
> > + .info =3D 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 =3D SNDRV_PCM_FMTBIT_S16_LE,
> > + .rate_min =3D 8000,
> > + .channels_min =3D 2,
> > + .channels_max =3D 2,
> > + .buffer_bytes_max =3D FSL_SAI_DMABUF_SIZE,
> > + .period_bytes_min =3D 4096,
> > + .period_bytes_max =3D FSL_SAI_DMABUF_SIZE / TCD_NUMBER,
> > + .periods_min =3D TCD_NUMBER,
> > + .periods_max =3D TCD_NUMBER,
> > + .fifo_size =3D 0,
> > +};
>=20
> 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.
>
I will do a research.
=20
> > + ret =3D 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 =3D fsl_sai_set_dai_sysclk_tr(cpu_dai, clk_id, freq,
> > + FSL_FMT_RECEIVER);
>=20
> 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.
>=20
> Similarly with several of the other functions.
>=20
As I have replied before, there is one function couldn't be separated for t=
he hardware limitation.
> > +int fsl_sai_dai_remove(struct snd_soc_dai *dai) {
> > + struct fsl_sai *sai =3D dev_get_drvdata(dai->dev);
> > +
> > + clk_disable_unprepare(sai->clk);
>=20
> 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.
>=20
Actully there are to "XXX_probe" functions and two "XXX_remove" functions:
fsl_sai_dai_probe() and fsl_sai_dai_remove() are callbacks of the ASoC subs=
ystem.
And in fsl_sai_dai_probe() needs to read/write the SAI controller's registe=
rs, so
the clk_enable_prepare() must be here and clk_disable_unprepare() in fsl_sa=
i_dai_remove().
fsl_sai_probe() and fsl_sai_remove() are the driver's probe and remove inte=
rfaces.
So the "+ clk_disable_unprepare(sai->clk);" sentence in fsl_sai_remove() wi=
ll be removed later.
> > + ret =3D snd_soc_register_component(&pdev->dev, &fsl_component,
> > + &fsl_sai_dai, 1);
> > + if (ret)
> > + return ret;
>=20
> There's a devm_snd_soc_register_component() in -next, please use that.
>=20
See the next version.
> > +
> > + ret =3D fsl_pcm_dma_init(pdev);
> > + if (ret)
> > + goto out;
> > +
> > + platform_set_drvdata(pdev, sai);
>=20
> 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.
>=20
> > +static int fsl_sai_remove(struct platform_device *pdev) {
> > + struct fsl_sai *sai =3D platform_get_drvdata(pdev);
> > +
> > + fsl_pcm_dma_exit(pdev);
> > +
> > + snd_soc_unregister_component(&pdev->dev);
>=20
> Similarly here, unregister from the subsystem then clean up after.
>=20
See the next version.
> > +#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
>=20
> Make the namespacing consistent please - for preference use FSL_SAI
> always.
>
See the next version.
^ permalink raw reply [flat|nested] 158+ messages in thread
* RE: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-24 11:05 ` Mark Brown
(?)
@ 2013-10-29 4:00 ` Xiubo Li-B47053
-1 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-29 4:00 UTC (permalink / raw)
To: Mark Brown
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, perex@perex.cz, Guo Shawn-R65073,
LW@KARO-electronics.de, linux@arm.linux.org.uk,
Chen Guangyu-B42378, linux-arm-kernel@lists.infradead.org,
grant.likely@linaro.org, devicetree@vger.kernel.org,
ian.campbell@citrix.com, pawel.moll@arm.com,
swarren@wwwdotorg.org, rob.herring@calxeda.com
> > +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.
>
I couldn't find the patch in the next and other trees.
Does this patch has been submitted to the -next tree ?
Or could you tell me how to find the patch please?
Thanks very much.
^ permalink raw reply [flat|nested] 158+ messages in thread* [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-29 4:00 ` Xiubo Li-B47053
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-29 4:00 UTC (permalink / raw)
To: linux-arm-kernel
> > +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.
>
I couldn't find the patch in the next and other trees.
Does this patch has been submitted to the -next tree ?
Or could you tell me how to find the patch please?
Thanks very much.
^ permalink raw reply [flat|nested] 158+ messages in thread* RE: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-29 4:00 ` Xiubo Li-B47053
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-29 4:00 UTC (permalink / raw)
To: Mark Brown
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, perex@perex.cz, Guo Shawn-R65073,
LW@KARO-electronics.de, linux@arm.linux.org.uk,
Chen Guangyu-B42378, linux-arm-kernel@lists.infradead.org,
grant.likely@linaro.org, devicetree@vger.kernel.org,
ian.campbell@citrix.com, pawel.moll@arm.com,
swarren@wwwdotorg.org, rob.herring@calxeda.com, oskar@scara.com,
Estevam Fabio-R49496, lgirdwood@gmail.com,
linux-kernel@vger.kernel.org, rob@landley.net,
Jin Zhengxiong-R64188, shawn.guo@linaro.org,
linuxppc-dev@lists.ozlabs.org
> > +static struct snd_pcm_hardware snd_fsl_hardware =3D {
> > + .info =3D 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 =3D SNDRV_PCM_FMTBIT_S16_LE,
> > + .rate_min =3D 8000,
> > + .channels_min =3D 2,
> > + .channels_max =3D 2,
> > + .buffer_bytes_max =3D FSL_SAI_DMABUF_SIZE,
> > + .period_bytes_min =3D 4096,
> > + .period_bytes_max =3D FSL_SAI_DMABUF_SIZE / TCD_NUMBER,
> > + .periods_min =3D TCD_NUMBER,
> > + .periods_max =3D TCD_NUMBER,
> > + .fifo_size =3D 0,
> > +};
>=20
> 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.
>=20
I couldn't find the patch in the next and other trees.
Does this patch has been submitted to the -next tree ?
Or could you tell me how to find the patch please?
Thanks very much.
^ permalink raw reply [flat|nested] 158+ messages in thread* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-29 4:00 ` Xiubo Li-B47053
(?)
@ 2013-10-29 4:02 ` Nicolin Chen
-1 siblings, 0 replies; 158+ messages in thread
From: Nicolin Chen @ 2013-10-29 4:02 UTC (permalink / raw)
To: Xiubo Li-B47053
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, Guo Shawn-R65073, LW@KARO-electronics.de,
linux@arm.linux.org.uk, linux-arm-kernel@lists.infradead.org,
grant.likely@linaro.org, devicetree@vger.kernel.org,
ian.campbell@citrix.com, pawel.moll@arm.com,
swarren@wwwdotorg.org, rob.herring@calxeda.com, Mark Brown,
oskar@scara.com, Estev
On Tue, Oct 29, 2013 at 12:00:57PM +0800, Xiubo Li-B47053 wrote:
> > 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.
> >
>
> I couldn't find the patch in the next and other trees.
> Does this patch has been submitted to the -next tree ?
> Or could you tell me how to find the patch please?
>
Are you using broonie's repository?
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
If you searched the title in for-next branch, you should have found it.
Best regards,
Nicolin Chen
^ permalink raw reply [flat|nested] 158+ messages in thread
* [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-29 4:02 ` Nicolin Chen
0 siblings, 0 replies; 158+ messages in thread
From: Nicolin Chen @ 2013-10-29 4:02 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Oct 29, 2013 at 12:00:57PM +0800, Xiubo Li-B47053 wrote:
> > 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.
> >
>
> I couldn't find the patch in the next and other trees.
> Does this patch has been submitted to the -next tree ?
> Or could you tell me how to find the patch please?
>
Are you using broonie's repository?
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
If you searched the title in for-next branch, you should have found it.
Best regards,
Nicolin Chen
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-29 4:02 ` Nicolin Chen
0 siblings, 0 replies; 158+ messages in thread
From: Nicolin Chen @ 2013-10-29 4:02 UTC (permalink / raw)
To: Xiubo Li-B47053
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, perex@perex.cz, Guo Shawn-R65073,
LW@KARO-electronics.de, linux@arm.linux.org.uk,
linux-arm-kernel@lists.infradead.org, grant.likely@linaro.org,
devicetree@vger.kernel.org, ian.campbell@citrix.com,
pawel.moll@arm.com, swarren@wwwdotorg.org,
rob.herring@calxeda.com, Mark Brown, oskar@scara.com,
Estevam Fabio-R49496, lgirdwood@gmail.com,
linux-kernel@vger.kernel.org, rob@landley.net,
Jin Zhengxiong-R64188, shawn.guo@linaro.org,
linuxppc-dev@lists.ozlabs.org
On Tue, Oct 29, 2013 at 12:00:57PM +0800, Xiubo Li-B47053 wrote:
> > 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.
> >
>
> I couldn't find the patch in the next and other trees.
> Does this patch has been submitted to the -next tree ?
> Or could you tell me how to find the patch please?
>
Are you using broonie's repository?
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
If you searched the title in for-next branch, you should have found it.
Best regards,
Nicolin Chen
^ permalink raw reply [flat|nested] 158+ messages in thread
* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
2013-10-29 4:02 ` Nicolin Chen
(?)
@ 2013-10-29 9:31 ` Xiubo Li-B47053
-1 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-29 9:31 UTC (permalink / raw)
To: Chen Guangyu-B42378
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, Guo Shawn-R65073, LW@KARO-electronics.de,
linux@arm.linux.org.uk, linux-arm-kernel@lists.infradead.org,
grant.likely@linaro.org, devicetree@vger.kernel.org,
ian.campbell@citrix.com, pawel.moll@arm.com,
swarren@wwwdotorg.org, rob.herring@calxeda.com, Mark Brown,
oskar@scara.com, Estev
> -----Original Message-----
> From: Chen Guangyu-B42378
> Sent: Tuesday, October 29, 2013 12:02 PM
> To: Xiubo Li-B47053
> Cc: Mark Brown; Guo Shawn-R65073; timur@tabi.org; lgirdwood@gmail.com;
> Jin Zhengxiong-R64188; rob.herring@calxeda.com; pawel.moll@arm.com;
> mark.rutland@arm.com; swarren@wwwdotorg.org; ian.campbell@citrix.com;
> rob@landley.net; linux@arm.linux.org.uk; perex@perex.cz; tiwai@suse.de;
> grant.likely@linaro.org; Estevam Fabio-R49496; LW@KARO-electronics.de;
> oskar@scara.com; shawn.guo@linaro.org; Wang Huan-B18965;
> devicetree@vger.kernel.org; linux-doc@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; alsa-
> devel@alsa-project.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface
> driver.
>
> On Tue, Oct 29, 2013 at 12:00:57PM +0800, Xiubo Li-B47053 wrote:
> > > 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.
> > >
> >
> > I couldn't find the patch in the next and other trees.
> > Does this patch has been submitted to the -next tree ?
> > Or could you tell me how to find the patch please?
> >
>
> Are you using broonie's repository?
NO.
> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
>
> If you searched the title in for-next branch, you should have found it.
>
Yes, find it.
Thanks very much.
--
Xiubo
^ permalink raw reply [flat|nested] 158+ messages in thread
* [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-29 9:31 ` Xiubo Li-B47053
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-29 9:31 UTC (permalink / raw)
To: linux-arm-kernel
> -----Original Message-----
> From: Chen Guangyu-B42378
> Sent: Tuesday, October 29, 2013 12:02 PM
> To: Xiubo Li-B47053
> Cc: Mark Brown; Guo Shawn-R65073; timur at tabi.org; lgirdwood at gmail.com;
> Jin Zhengxiong-R64188; rob.herring at calxeda.com; pawel.moll at arm.com;
> mark.rutland at arm.com; swarren at wwwdotorg.org; ian.campbell at citrix.com;
> rob at landley.net; linux at arm.linux.org.uk; perex at perex.cz; tiwai at suse.de;
> grant.likely at linaro.org; Estevam Fabio-R49496; LW at KARO-electronics.de;
> oskar at scara.com; shawn.guo at linaro.org; Wang Huan-B18965;
> devicetree at vger.kernel.org; linux-doc at vger.kernel.org; linux-
> kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org; alsa-
> devel at alsa-project.org; linuxppc-dev at lists.ozlabs.org
> Subject: Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface
> driver.
>
> On Tue, Oct 29, 2013 at 12:00:57PM +0800, Xiubo Li-B47053 wrote:
> > > 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.
> > >
> >
> > I couldn't find the patch in the next and other trees.
> > Does this patch has been submitted to the -next tree ?
> > Or could you tell me how to find the patch please?
> >
>
> Are you using broonie's repository?
NO.
> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
>
> If you searched the title in for-next branch, you should have found it.
>
Yes, find it.
Thanks very much.
--
Xiubo
^ permalink raw reply [flat|nested] 158+ messages in thread
* RE: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
@ 2013-10-29 9:31 ` Xiubo Li-B47053
0 siblings, 0 replies; 158+ messages in thread
From: Xiubo Li-B47053 @ 2013-10-29 9:31 UTC (permalink / raw)
To: Chen Guangyu-B42378
Cc: mark.rutland@arm.com, alsa-devel@alsa-project.org,
linux-doc@vger.kernel.org, tiwai@suse.de, Wang Huan-B18965,
timur@tabi.org, perex@perex.cz, Guo Shawn-R65073,
LW@KARO-electronics.de, linux@arm.linux.org.uk,
linux-arm-kernel@lists.infradead.org, grant.likely@linaro.org,
devicetree@vger.kernel.org, ian.campbell@citrix.com,
pawel.moll@arm.com, swarren@wwwdotorg.org,
rob.herring@calxeda.com, Mark Brown, oskar@scara.com,
Estevam Fabio-R49496, lgirdwood@gmail.com,
linux-kernel@vger.kernel.org, rob@landley.net,
Jin Zhengxiong-R64188, shawn.guo@linaro.org,
linuxppc-dev@lists.ozlabs.org
> -----Original Message-----
> From: Chen Guangyu-B42378
> Sent: Tuesday, October 29, 2013 12:02 PM
> To: Xiubo Li-B47053
> Cc: Mark Brown; Guo Shawn-R65073; timur@tabi.org; lgirdwood@gmail.com;
> Jin Zhengxiong-R64188; rob.herring@calxeda.com; pawel.moll@arm.com;
> mark.rutland@arm.com; swarren@wwwdotorg.org; ian.campbell@citrix.com;
> rob@landley.net; linux@arm.linux.org.uk; perex@perex.cz; tiwai@suse.de;
> grant.likely@linaro.org; Estevam Fabio-R49496; LW@KARO-electronics.de;
> oskar@scara.com; shawn.guo@linaro.org; Wang Huan-B18965;
> devicetree@vger.kernel.org; linux-doc@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; alsa-
> devel@alsa-project.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface
> driver.
>=20
> On Tue, Oct 29, 2013 at 12:00:57PM +0800, Xiubo Li-B47053 wrote:
> > > 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.
> > >
> >
> > I couldn't find the patch in the next and other trees.
> > Does this patch has been submitted to the -next tree ?
> > Or could you tell me how to find the patch please?
> >
>=20
> Are you using broonie's repository?
NO.
> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
>=20
> If you searched the title in for-next branch, you should have found it.
>=20
Yes, find it.
Thanks very much.
--
Xiubo
^ permalink raw reply [flat|nested] 158+ messages in thread