linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* (no subject)
@ 2013-11-01  7:04 Xiubo Li
  2013-11-01  7:04 ` [PATCHv2 1/8] ALSA: Add SAI SoC Digital Audio Interface driver Xiubo Li
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Xiubo Li @ 2013-11-01  7:04 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


Hello,

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

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

Changed in v2:
- Use default settings for the generic dmaengine PCM driver.
- Separate receive and transmit setting in most functions, but some couldn't for the HW limitation.
- Drop some not reduntant code.
- Use devm_snd_soc_register_component() instead of snd_soc_register_component().
- Use devm_snd_soc_register_card() instead of devm_snd_soc_register_card().
- Adjust the code sentences sequence.
- Make the namespacing consistent.
- Rename CONFIG_SND_SOC_FSL_SGTL5000 to CONFIG_SND_SOC_FSL_SGTL5000_VF610.
- Drop some meaningless lines.
- Rename the binding document file.

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





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCHv2 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
  2013-11-01  7:04 Xiubo Li
@ 2013-11-01  7:04 ` Xiubo Li
  2013-11-01  8:59   ` Nicolin Chen
  2013-11-01 18:25   ` Mark Brown
  2013-11-01  7:04 ` [PATCHv2 2/8] ARM: dts: Add Freescale SAI ALSA SoC Digital Audio Interface node for VF610 Xiubo Li
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 17+ messages in thread
From: Xiubo Li @ 2013-11-01  7:04 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   |  16 ++
 sound/soc/fsl/Makefile  |   5 +
 sound/soc/fsl/fsl-sai.c | 472 ++++++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/fsl/fsl-sai.h | 120 ++++++++++++
 4 files changed, 613 insertions(+)
 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 b7ab71f..9a8851e 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -213,3 +213,19 @@ config SND_SOC_IMX_MC13783
 	select SND_SOC_IMX_PCM_DMA
 
 endif # SND_IMX_SOC
+
+menuconfig SND_FSL_SOC
+	tristate "SoC Audio for Freescale FSL CPUs"
+	help
+	  Say Y or M if you want to add support for codecs attached to
+	  the FSL CPUs.
+
+	  This will enable Freeacale SAI, SGT15000 codec.
+
+if SND_FSL_SOC
+
+config SND_SOC_FSL_SAI
+	tristate
+	select SND_SOC_GENERIC_DMAENGINE_PCM
+
+endif # SND_FSL_SOC
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
index 8db705b..e5acc03 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -56,3 +56,8 @@ obj-$(CONFIG_SND_SOC_IMX_SGTL5000) += snd-soc-imx-sgtl5000.o
 obj-$(CONFIG_SND_SOC_IMX_WM8962) += snd-soc-imx-wm8962.o
 obj-$(CONFIG_SND_SOC_IMX_SPDIF) += snd-soc-imx-spdif.o
 obj-$(CONFIG_SND_SOC_IMX_MC13783) += snd-soc-imx-mc13783.o
+
+# FSL ARM SAI/SGT15000 Platform Support
+snd-soc-fsl-sai-objs := fsl-sai.o
+
+obj-$(CONFIG_SND_SOC_FSL_SAI) += snd-soc-fsl-sai.o
diff --git a/sound/soc/fsl/fsl-sai.c b/sound/soc/fsl/fsl-sai.c
new file mode 100644
index 0000000..bb57e67
--- /dev/null
+++ b/sound/soc/fsl/fsl-sai.c
@@ -0,0 +1,472 @@
+/*
+ * Freescale SAI ALSA SoC Digital Audio Interface driver.
+ *
+ * Copyright 2012-2013 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/of_address.h>
+#include <sound/core.h>
+#include <sound/pcm_params.h>
+#include <linux/delay.h>
+#include <linux/dmaengine.h>
+#include <sound/dmaengine_pcm.h>
+
+#include "fsl-sai.h"
+
+static int fsl_sai_set_dai_sysclk_tr(struct snd_soc_dai *cpu_dai,
+		int clk_id, unsigned int freq, int fsl_dir)
+{
+	u32 val_cr2, reg_cr2;
+	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+
+	if (fsl_dir == FSL_FMT_TRANSMITTER)
+		reg_cr2 = FSL_SAI_TCR2;
+	else
+		reg_cr2 = FSL_SAI_RCR2;
+
+	val_cr2 = readl(sai->base + reg_cr2);
+	switch (clk_id) {
+	case FSL_SAI_CLK_BUS:
+		val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK;
+		val_cr2 |= FSL_SAI_CR2_MSEL_BUS;
+		break;
+	case FSL_SAI_CLK_MAST1:
+		val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK;
+		val_cr2 |= FSL_SAI_CR2_MSEL_MCLK1;
+		break;
+	case FSL_SAI_CLK_MAST2:
+		val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK;
+		val_cr2 |= FSL_SAI_CR2_MSEL_MCLK2;
+		break;
+	case FSL_SAI_CLK_MAST3:
+		val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK;
+		val_cr2 |= FSL_SAI_CR2_MSEL_MCLK3;
+		break;
+	default:
+		return -EINVAL;
+	}
+	writel(val_cr2, sai->base + reg_cr2);
+
+	return 0;
+}
+
+static int fsl_sai_set_dai_sysclk(struct snd_soc_dai *cpu_dai,
+		int clk_id, unsigned int freq, int dir)
+{
+	int ret;
+
+	if (dir == SND_SOC_CLOCK_IN)
+		return 0;
+
+	ret = fsl_sai_set_dai_sysclk_tr(cpu_dai, clk_id, freq,
+					FSL_FMT_TRANSMITTER);
+	if (ret) {
+		dev_err(cpu_dai->dev,
+				"Cannot set sai's transmitter sysclk: %d\n",
+				ret);
+		return ret;
+	}
+
+	ret = fsl_sai_set_dai_sysclk_tr(cpu_dai, clk_id, freq,
+					FSL_FMT_RECEIVER);
+	if (ret) {
+		dev_err(cpu_dai->dev,
+				"Cannot set sai's receiver sysclk: %d\n",
+				ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int fsl_sai_set_dai_clkdiv(struct snd_soc_dai *cpu_dai,
+		int div_id, int div)
+{
+	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+	u32 tcr2, rcr2;
+
+	if (div_id == FSL_SAI_TX_DIV) {
+		tcr2 = readl(sai->base + FSL_SAI_TCR2);
+		tcr2 &= ~FSL_SAI_CR2_DIV_MASK;
+		tcr2 |= FSL_SAI_CR2_DIV(div);
+		writel(tcr2, sai->base + FSL_SAI_TCR2);
+
+	} else if (div_id == FSL_SAI_RX_DIV) {
+		rcr2 = readl(sai->base + FSL_SAI_RCR2);
+		rcr2 &= ~FSL_SAI_CR2_DIV_MASK;
+		rcr2 |= FSL_SAI_CR2_DIV(div);
+		writel(rcr2, sai->base + FSL_SAI_RCR2);
+
+	} else
+		return -EINVAL;
+
+	return 0;
+}
+
+static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai *cpu_dai,
+				unsigned int fmt, int fsl_dir)
+{
+	u32 val_cr2, val_cr3, val_cr4, reg_cr2, reg_cr3, reg_cr4;
+	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+
+	if (fsl_dir == FSL_FMT_TRANSMITTER) {
+		reg_cr2 = FSL_SAI_TCR2;
+		reg_cr3 = FSL_SAI_TCR3;
+		reg_cr4 = FSL_SAI_TCR4;
+	} else {
+		reg_cr2 = FSL_SAI_RCR2;
+		reg_cr3 = FSL_SAI_RCR3;
+		reg_cr4 = FSL_SAI_RCR4;
+	}
+
+	val_cr2 = readl(sai->base + reg_cr2);
+	val_cr3 = readl(sai->base + reg_cr3);
+	val_cr4 = readl(sai->base + reg_cr4);
+
+	if (sai->fbt == FSL_SAI_FBT_MSB)
+		val_cr4 |= FSL_SAI_CR4_MF;
+	else if (sai->fbt == FSL_SAI_FBT_LSB)
+		val_cr4 &= ~FSL_SAI_CR4_MF;
+
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		val_cr4 |= FSL_SAI_CR4_FSE;
+		val_cr4 |= FSL_SAI_CR4_FSP;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_IB_IF:
+		val_cr4 |= FSL_SAI_CR4_FSP;
+		val_cr2 &= ~FSL_SAI_CR2_BCP;
+		break;
+	case SND_SOC_DAIFMT_IB_NF:
+		val_cr4 &= ~FSL_SAI_CR4_FSP;
+		val_cr2 &= ~FSL_SAI_CR2_BCP;
+		break;
+	case SND_SOC_DAIFMT_NB_IF:
+		val_cr4 |= FSL_SAI_CR4_FSP;
+		val_cr2 |= FSL_SAI_CR2_BCP;
+		break;
+	case SND_SOC_DAIFMT_NB_NF:
+		val_cr4 &= ~FSL_SAI_CR4_FSP;
+		val_cr2 |= FSL_SAI_CR2_BCP;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBS_CFS:
+		val_cr2 |= FSL_SAI_CR2_BCD_MSTR;
+		val_cr4 |= FSL_SAI_CR4_FSD_MSTR;
+		break;
+	case SND_SOC_DAIFMT_CBM_CFM:
+		val_cr2 &= ~FSL_SAI_CR2_BCD_MSTR;
+		val_cr4 &= ~FSL_SAI_CR4_FSD_MSTR;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	val_cr3 |= FSL_SAI_CR3_TRCE;
+
+	if (fsl_dir == FSL_FMT_RECEIVER)
+		val_cr2 |= FSL_SAI_CR2_SYNC;
+
+	writel(val_cr2, sai->base + reg_cr2);
+	writel(val_cr3, sai->base + reg_cr3);
+	writel(val_cr4, sai->base + reg_cr4);
+
+	return 0;
+
+}
+
+static int fsl_sai_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
+{
+	int ret;
+
+	ret = fsl_sai_set_dai_fmt_tr(cpu_dai, fmt, FSL_FMT_TRANSMITTER);
+	if (ret) {
+		dev_err(cpu_dai->dev,
+				"Cannot set sai's transmitter format: %d\n",
+				ret);
+		return ret;
+	}
+
+	ret = fsl_sai_set_dai_fmt_tr(cpu_dai, fmt, FSL_FMT_RECEIVER);
+	if (ret) {
+		dev_err(cpu_dai->dev,
+				"Cannot set sai's receiver format: %d\n",
+				ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int fsl_sai_set_dai_tdm_slot(struct snd_soc_dai *cpu_dai,
+		unsigned int tx_mask, unsigned int rx_mask,
+		int slots, int slot_width)
+{
+	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+	u32 tcr4, rcr4;
+
+	tcr4 = readl(sai->base + FSL_SAI_TCR4);
+	tcr4 &= ~FSL_SAI_CR4_FRSZ_MASK;
+	tcr4 |= FSL_SAI_CR4_FRSZ(2);
+	writel(tcr4, sai->base + FSL_SAI_TCR4);
+	writel(tx_mask, sai->base + FSL_SAI_TMR);
+
+	rcr4 = readl(sai->base + FSL_SAI_RCR4);
+	rcr4 &= ~FSL_SAI_CR4_FRSZ_MASK;
+	rcr4 |= FSL_SAI_CR4_FRSZ(2);
+	writel(rcr4, sai->base + FSL_SAI_RCR4);
+	writel(rx_mask, sai->base + FSL_SAI_RMR);
+
+	return 0;
+}
+
+static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
+		struct snd_pcm_hw_params *params,
+		struct snd_soc_dai *cpu_dai)
+{
+	u32 val_cr4, val_cr5, reg_cr4, reg_cr5, word_width;
+	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		reg_cr4 = FSL_SAI_TCR4;
+		reg_cr5 = FSL_SAI_TCR5;
+	} else {
+		reg_cr4 = FSL_SAI_RCR4;
+		reg_cr5 = FSL_SAI_RCR5;
+	}
+
+	val_cr4 = readl(sai->base + reg_cr4);
+	val_cr4 &= ~FSL_SAI_CR4_SYWD_MASK;
+
+	val_cr5 = readl(sai->base + reg_cr5);
+	val_cr5 &= ~FSL_SAI_CR5_WNW_MASK;
+	val_cr5 &= ~FSL_SAI_CR5_W0W_MASK;
+	val_cr5 &= ~FSL_SAI_CR5_FBT_MASK;
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_S16_LE:
+		word_width = 16;
+		break;
+	case SNDRV_PCM_FORMAT_S20_3LE:
+		word_width = 20;
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+		word_width = 24;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	val_cr4 |= FSL_SAI_CR4_SYWD(word_width);
+	val_cr5 |= FSL_SAI_CR5_WNW(word_width);
+	val_cr5 |= FSL_SAI_CR5_W0W(word_width);
+
+	if (sai->fbt == FSL_SAI_FBT_MSB)
+		val_cr5 |= FSL_SAI_CR5_FBT(word_width - 1);
+	else if (sai->fbt == FSL_SAI_FBT_LSB)
+		val_cr5 |= FSL_SAI_CR5_FBT(0);
+
+	writel(val_cr4, sai->base + reg_cr4);
+	writel(val_cr5, sai->base + reg_cr5);
+
+	return 0;
+}
+
+static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
+		struct snd_soc_dai *dai)
+{
+	struct fsl_sai *sai = snd_soc_dai_get_drvdata(dai);
+	unsigned int tcsr, rcsr;
+
+	tcsr = readl(sai->base + FSL_SAI_TCSR);
+	rcsr = readl(sai->base + FSL_SAI_RCSR);
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		tcsr |= FSL_SAI_CSR_FRDE;
+		rcsr &= ~FSL_SAI_CSR_FRDE;
+	} else {
+		rcsr |= FSL_SAI_CSR_FRDE;
+		tcsr &= ~FSL_SAI_CSR_FRDE;
+	}
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		tcsr |= FSL_SAI_CSR_TERE;
+		rcsr |= FSL_SAI_CSR_TERE;
+		writel(rcsr, sai->base + FSL_SAI_RCSR);
+		udelay(10);
+		writel(tcsr, sai->base + FSL_SAI_TCSR);
+		break;
+
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		if (!(dai->playback_active || dai->capture_active)) {
+			tcsr &= ~FSL_SAI_CSR_TERE;
+			rcsr &= ~FSL_SAI_CSR_TERE;
+		}
+		writel(rcsr, sai->base + FSL_SAI_RCSR);
+		udelay(10);
+		writel(tcsr, sai->base + FSL_SAI_TCSR);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
+	.set_sysclk	= fsl_sai_set_dai_sysclk,
+	.set_clkdiv	= fsl_sai_set_dai_clkdiv,
+	.set_fmt	= fsl_sai_set_dai_fmt,
+	.set_tdm_slot	= fsl_sai_set_dai_tdm_slot,
+	.hw_params	= fsl_sai_hw_params,
+	.trigger	= fsl_sai_trigger,
+};
+
+static int fsl_sai_dai_probe(struct snd_soc_dai *dai)
+{
+	int ret;
+	struct fsl_sai *sai = dev_get_drvdata(dai->dev);
+
+	ret = clk_prepare_enable(sai->clk);
+	if (ret)
+		return ret;
+
+	writel(0x0, sai->base + FSL_SAI_RCSR);
+	writel(0x0, sai->base + FSL_SAI_TCSR);
+	writel(sai->dma_params_tx.maxburst, sai->base + FSL_SAI_TCR1);
+	writel(sai->dma_params_rx.maxburst, sai->base + FSL_SAI_RCR1);
+
+	dai->playback_dma_data = &sai->dma_params_tx;
+	dai->capture_dma_data = &sai->dma_params_rx;
+
+	snd_soc_dai_set_drvdata(dai, sai);
+
+	return 0;
+}
+
+int fsl_sai_dai_remove(struct snd_soc_dai *dai)
+{
+	struct fsl_sai *sai = dev_get_drvdata(dai->dev);
+
+	clk_disable_unprepare(sai->clk);
+
+	return 0;
+}
+
+static struct snd_soc_dai_driver fsl_sai_dai = {
+	.probe = fsl_sai_dai_probe,
+	.remove = fsl_sai_dai_remove,
+	.playback = {
+		.channels_min = 1,
+		.channels_max = 2,
+		.rates = SNDRV_PCM_RATE_8000_96000,
+		.formats = FSL_SAI_FORMATS,
+	},
+	.capture = {
+		.channels_min = 1,
+		.channels_max = 2,
+		.rates = SNDRV_PCM_RATE_8000_96000,
+		.formats = FSL_SAI_FORMATS,
+	},
+	.ops = &fsl_sai_pcm_dai_ops,
+};
+
+static const struct snd_soc_component_driver fsl_component = {
+	.name           = "fsl-sai",
+};
+
+static int fsl_sai_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct fsl_sai *sai;
+	int ret = 0;
+
+	sai = devm_kzalloc(&pdev->dev, sizeof(*sai), GFP_KERNEL);
+	if (!sai)
+		return -ENOMEM;
+
+	sai->fbt = FSL_SAI_FBT_MSB;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	sai->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(sai->base))
+		return PTR_ERR(sai->base);
+
+	sai->clk = devm_clk_get(&pdev->dev, "sai");
+	if (IS_ERR(sai->clk)) {
+		dev_err(&pdev->dev, "Cannot get sai's clock: %d\n", ret);
+		return PTR_ERR(sai->clk);
+	}
+
+	sai->dma_params_rx.addr = res->start + FSL_SAI_RDR;
+	sai->dma_params_rx.maxburst = 6;
+
+	sai->dma_params_tx.addr = res->start + FSL_SAI_TDR;
+	sai->dma_params_tx.maxburst = 6;
+
+	platform_set_drvdata(pdev, sai);
+
+	ret = devm_snd_soc_register_component(&pdev->dev, &fsl_component,
+			&fsl_sai_dai, 1);
+	if (ret)
+		return ret;
+
+	ret = snd_dmaengine_pcm_register(&pdev->dev, NULL,
+			SND_DMAENGINE_PCM_FLAG_NO_RESIDUE);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int fsl_sai_remove(struct platform_device *pdev)
+{
+	snd_dmaengine_pcm_unregister(&pdev->dev);
+
+	return 0;
+}
+
+static const struct of_device_id fsl_sai_ids[] = {
+	{ .compatible = "fsl,vf610-sai", },
+	{ /*sentinel*/ }
+};
+
+static struct platform_driver fsl_sai_driver = {
+	.probe = fsl_sai_probe,
+	.remove = fsl_sai_remove,
+
+	.driver = {
+		.name = "fsl-sai",
+		.owner = THIS_MODULE,
+		.of_match_table = fsl_sai_ids,
+	},
+};
+module_platform_driver(fsl_sai_driver);
+
+MODULE_AUTHOR("Xiubo Li, <Li.Xiubo@freescale.com>");
+MODULE_AUTHOR("Alison Wang, <b18965@freescale.com>");
+MODULE_DESCRIPTION("Freescale Soc SAI Interface");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/fsl/fsl-sai.h b/sound/soc/fsl/fsl-sai.h
new file mode 100644
index 0000000..1637679
--- /dev/null
+++ b/sound/soc/fsl/fsl-sai.h
@@ -0,0 +1,120 @@
+/*
+ * Copyright 2012-2013 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __FSL_SAI_H
+#define __FSL_SAI_H
+
+#include <sound/dmaengine_pcm.h>
+
+#define FSL_SAI_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |\
+			 SNDRV_PCM_FMTBIT_S20_3LE |\
+			 SNDRV_PCM_FMTBIT_S24_LE)
+
+/* SAI Transmit/Recieve Control Register */
+#define FSL_SAI_TCSR		0x00
+#define FSL_SAI_RCSR		0x80
+#define FSL_SAI_CSR_TERE	BIT(31)
+#define FSL_SAI_CSR_FWF		BIT(17)
+#define FSL_SAI_CSR_FRIE	BIT(8)
+#define FSL_SAI_CSR_FRDE	BIT(0)
+
+/* SAI Transmit Data/FIFO/MASK Register */
+#define FSL_SAI_TDR		0x20
+#define FSL_SAI_TFR		0x40
+#define FSL_SAI_TMR		0x60
+
+/* SAI Recieve Data/FIFO/MASK Register */
+#define FSL_SAI_RDR		0xa0
+#define FSL_SAI_RFR		0xc0
+#define FSL_SAI_RMR		0xe0
+
+/* SAI Transmit and Recieve Configuration 1 Register */
+#define FSL_SAI_TCR1		0x04
+#define FSL_SAI_RCR1		0x84
+
+/* SAI Transmit and Recieve Configuration 2 Register */
+#define FSL_SAI_TCR2		0x08
+#define FSL_SAI_RCR2		0x88
+#define FSL_SAI_CR2_SYNC	BIT(30)
+#define FSL_SAI_CR2_MSEL_MASK	(0xff << 26)
+#define FSL_SAI_CR2_MSEL_BUS	0
+#define FSL_SAI_CR2_MSEL_MCLK1	BIT(26)
+#define FSL_SAI_CR2_MSEL_MCLK2	BIT(27)
+#define FSL_SAI_CR2_MSEL_MCLK3	(BIT(26) | BIT(27))
+#define FSL_SAI_CR2_BCP		BIT(25)
+#define FSL_SAI_CR2_BCD_MSTR	BIT(24)
+#define FSL_SAI_CR2_DIV(x)	(x)
+#define FSL_SAI_CR2_DIV_MASK	0xff
+
+/* SAI Transmit and Recieve Configuration 3 Register */
+#define FSL_SAI_TCR3		0x0c
+#define FSL_SAI_RCR3		0x8c
+#define FSL_SAI_CR3_TRCE	BIT(16)
+#define FSL_SAI_CR3_WDFL(x)	(x)
+#define FSL_SAI_CR3_WDFL_MASK	0x1f
+
+/* SAI Transmit and Recieve Configuration 4 Register */
+#define FSL_SAI_TCR4		0x10
+#define FSL_SAI_RCR4		0x90
+#define FSL_SAI_CR4_FRSZ(x)	(((x) - 1) << 16)
+#define FSL_SAI_CR4_FRSZ_MASK	(0x1f << 16)
+#define FSL_SAI_CR4_SYWD(x)	(((x) - 1) << 8)
+#define FSL_SAI_CR4_SYWD_MASK	(0x1f << 8)
+#define FSL_SAI_CR4_MF		BIT(4)
+#define FSL_SAI_CR4_FSE		BIT(3)
+#define FSL_SAI_CR4_FSP		BIT(1)
+#define FSL_SAI_CR4_FSD_MSTR	BIT(0)
+
+/* SAI Transmit and Recieve Configuration 5 Register */
+#define FSL_SAI_TCR5		0x14
+#define FSL_SAI_RCR5		0x94
+#define FSL_SAI_CR5_WNW(x)	(((x) - 1) << 24)
+#define FSL_SAI_CR5_WNW_MASK	(0x1f << 24)
+#define FSL_SAI_CR5_W0W(x)	(((x) - 1) << 16)
+#define FSL_SAI_CR5_W0W_MASK	(0x1f << 16)
+#define FSL_SAI_CR5_FBT(x)	((x) << 8)
+#define FSL_SAI_CR5_FBT_MASK	(0x1f << 8)
+
+/* SAI audio dividers */
+#define FSL_SAI_TX_DIV		0
+#define FSL_SAI_RX_DIV		1
+
+/* SAI type */
+#define FSL_SAI_DMA		BIT(0)
+#define FSL_SAI_USE_AC97	BIT(1)
+#define FSL_SAI_NET		BIT(2)
+#define FSL_SAI_TRA_SYN		BIT(3)
+#define FSL_SAI_REC_SYN		BIT(4)
+#define FSL_SAI_USE_I2S_SLAVE	BIT(5)
+
+#define FSL_FMT_TRANSMITTER	0
+#define FSL_FMT_RECEIVER	1
+
+/* SAI clock sources */
+#define FSL_SAI_CLK_BUS		0
+#define FSL_SAI_CLK_MAST1	1
+#define FSL_SAI_CLK_MAST2	2
+#define FSL_SAI_CLK_MAST3	3
+
+enum fsl_sai_fbt {
+	FSL_SAI_FBT_MSB,
+	FSL_SAI_FBT_LSB,
+};
+
+struct fsl_sai {
+	struct clk *clk;
+
+	void __iomem *base;
+
+	enum fsl_sai_fbt fbt;
+
+	struct snd_dmaengine_dai_dma_data dma_params_rx;
+	struct snd_dmaengine_dai_dma_data dma_params_tx;
+};
+
+#endif /* __FSL_SAI_H */
-- 
1.8.4



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCHv2 2/8] ARM: dts: Add Freescale SAI ALSA SoC Digital Audio Interface node for VF610.
  2013-11-01  7:04 Xiubo Li
  2013-11-01  7:04 ` [PATCHv2 1/8] ALSA: Add SAI SoC Digital Audio Interface driver Xiubo Li
@ 2013-11-01  7:04 ` Xiubo Li
  2013-11-01  7:04 ` [PATCHv2 3/8] ARM: dts: Enables SAI ALSA SoC DAI device for Vybrid VF610 TOWER board Xiubo Li
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Xiubo Li @ 2013-11-01  7:04 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, Jingchang Lu

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

Signed-off-by: Jingchang Lu <b35083@freescale.com>
Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 arch/arm/boot/dts/vf610.dtsi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCHv2 3/8] ARM: dts: Enables SAI ALSA SoC DAI device for Vybrid VF610 TOWER board.
  2013-11-01  7:04 Xiubo Li
  2013-11-01  7:04 ` [PATCHv2 1/8] ALSA: Add SAI SoC Digital Audio Interface driver Xiubo Li
  2013-11-01  7:04 ` [PATCHv2 2/8] ARM: dts: Add Freescale SAI ALSA SoC Digital Audio Interface node for VF610 Xiubo Li
@ 2013-11-01  7:04 ` Xiubo Li
  2013-11-01  7:04 ` [PATCHv2 4/8] Documentation: Add device tree bindings for Freescale SAI Xiubo Li
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Xiubo Li @ 2013-11-01  7:04 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 patch add and enable SAI device.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 arch/arm/boot/dts/vf610-twr.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
index 1a58678..e4106dd 100644
--- a/arch/arm/boot/dts/vf610-twr.dts
+++ b/arch/arm/boot/dts/vf610-twr.dts
@@ -57,6 +57,12 @@
 	status = "okay";
 };
 
+&sai2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_sai2_1>;
+	status = "okay";
+};
+
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1_1>;
-- 
1.8.4



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCHv2 4/8] Documentation: Add device tree bindings for Freescale SAI.
  2013-11-01  7:04 Xiubo Li
                   ` (2 preceding siblings ...)
  2013-11-01  7:04 ` [PATCHv2 3/8] ARM: dts: Enables SAI ALSA SoC DAI device for Vybrid VF610 TOWER board Xiubo Li
@ 2013-11-01  7:04 ` Xiubo Li
  2013-11-01  7:04 ` [PATCHv2 5/8] ASoC: SGTL5000: Enhance the SGTL5000 codec driver about regulator Xiubo Li
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Xiubo Li @ 2013-11-01  7:04 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 the Document for Freescale SAI driver under
Documentation/devicetree/bindings/sound/.

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

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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCHv2 5/8] ASoC: SGTL5000: Enhance the SGTL5000 codec driver about regulator.
  2013-11-01  7:04 Xiubo Li
                   ` (3 preceding siblings ...)
  2013-11-01  7:04 ` [PATCHv2 4/8] Documentation: Add device tree bindings for Freescale SAI Xiubo Li
@ 2013-11-01  7:04 ` Xiubo Li
  2013-11-01 10:02   ` Nicolin Chen
  2013-11-01 18:50   ` Mark Brown
  2013-11-01  7:04 ` [PATCHv2 6/8] ASoC: fsl: add SGTL5000 based audio machine driver Xiubo Li
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 17+ messages in thread
From: Xiubo Li @ 2013-11-01  7:04 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

On VF610 series there are no regulators used, and now whether the
CONFIG_REGULATOR mirco is enabled or not, for the VF610 audio
patch series, the board cannot be probe successfully.
And this patch will solve this issue.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 sound/soc/codecs/sgtl5000.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
index 1f4093f..c2f6d86 100644
--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -61,6 +61,7 @@ static const struct reg_default sgtl5000_reg_defaults[] = {
 	{ SGTL5000_DAP_AVC_DECAY,		0x0050 },
 };
 
+#ifdef CONFIG_REGULATOR
 /* regulator supplies for sgtl5000, VDDD is an optional external supply */
 enum sgtl5000_regulator_supplies {
 	VDDA,
@@ -93,6 +94,9 @@ static struct regulator_init_data ldo_init_data = {
 	.num_consumer_supplies = 1,
 	.consumer_supplies = &ldo_consumer[0],
 };
+#else
+#define SGTL5000_SUPPLY_NUM 0
+#endif
 
 /*
  * sgtl5000 internal ldo regulator,
@@ -112,7 +116,9 @@ struct sgtl5000_priv {
 	int master;	/* i2s master or not */
 	int fmt;	/* i2s data format */
 	struct regulator_bulk_data supplies[SGTL5000_SUPPLY_NUM];
+#ifdef CONFIG_REGULATOR
 	struct ldo_regulator *ldo;
+#endif
 	struct regmap *regmap;
 	struct clk *mclk;
 };
@@ -879,6 +885,7 @@ static int ldo_regulator_remove(struct snd_soc_codec *codec)
 	return 0;
 }
 #else
+#ifndef CONFIG_SND_SOC_FSL_SGTL5000_VF610
 static int ldo_regulator_register(struct snd_soc_codec *codec,
 				struct regulator_init_data *init_data,
 				int voltage)
@@ -886,6 +893,7 @@ static int ldo_regulator_register(struct snd_soc_codec *codec,
 	dev_err(codec->dev, "this setup needs regulator support in the kernel\n");
 	return -EINVAL;
 }
+#endif
 
 static int ldo_regulator_remove(struct snd_soc_codec *codec)
 {
@@ -1137,6 +1145,7 @@ static int sgtl5000_resume(struct snd_soc_codec *codec)
 #define sgtl5000_resume  NULL
 #endif	/* CONFIG_SUSPEND */
 
+#ifdef CONFIG_REGULATOR
 /*
  * sgtl5000 has 3 internal power supplies:
  * 1. VAG, normally set to vdda/2
@@ -1373,6 +1382,7 @@ err_regulator_free:
 	return ret;
 
 }
+#endif
 
 static int sgtl5000_probe(struct snd_soc_codec *codec)
 {
@@ -1387,6 +1397,7 @@ static int sgtl5000_probe(struct snd_soc_codec *codec)
 		return ret;
 	}
 
+#ifdef CONFIG_REGULATOR
 	ret = sgtl5000_enable_regulators(codec);
 	if (ret)
 		return ret;
@@ -1395,6 +1406,7 @@ static int sgtl5000_probe(struct snd_soc_codec *codec)
 	ret = sgtl5000_set_power_regs(codec);
 	if (ret)
 		goto err;
+#endif
 
 	/* enable small pop, introduce 400ms delay in turning off */
 	snd_soc_update_bits(codec, SGTL5000_CHIP_REF_CTRL,
-- 
1.8.4



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCHv2 6/8] ASoC: fsl: add SGTL5000 based audio machine driver.
  2013-11-01  7:04 Xiubo Li
                   ` (4 preceding siblings ...)
  2013-11-01  7:04 ` [PATCHv2 5/8] ASoC: SGTL5000: Enhance the SGTL5000 codec driver about regulator Xiubo Li
@ 2013-11-01  7:04 ` Xiubo Li
  2013-11-01 10:17   ` Oskar Schirmer
                     ` (2 more replies)
  2013-11-01  7:04 ` [PATCHv2 7/8] ARM: dts: Enable SGTL5000 codec based audio driver node for VF610 Xiubo Li
  2013-11-01  7:04 ` [PATCHv2 8/8] Documentation: Add device tree bindings for Freescale VF610 sound Xiubo Li
  7 siblings, 3 replies; 17+ messages in thread
From: Xiubo Li @ 2013-11-01  7:04 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 is the SGTL5000 codec based audio driver supported with both
playback and capture dai link implemention.

This implementation is only compatible with device tree definition.

Signed-off-by: Alison Wang <b18965@freescale.com
Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>

Conflicts:
	sound/soc/fsl/Makefile
---
 sound/soc/fsl/Kconfig              |  10 ++
 sound/soc/fsl/Makefile             |   2 +
 sound/soc/fsl/fsl-sgtl5000-vf610.c | 208 +++++++++++++++++++++++++++++++++++++
 3 files changed, 220 insertions(+)
 create mode 100644 sound/soc/fsl/fsl-sgtl5000-vf610.c

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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCHv2 7/8] ARM: dts: Enable SGTL5000 codec based audio driver node for VF610.
  2013-11-01  7:04 Xiubo Li
                   ` (5 preceding siblings ...)
  2013-11-01  7:04 ` [PATCHv2 6/8] ASoC: fsl: add SGTL5000 based audio machine driver Xiubo Li
@ 2013-11-01  7:04 ` Xiubo Li
  2013-11-01  7:04 ` [PATCHv2 8/8] Documentation: Add device tree bindings for Freescale VF610 sound Xiubo Li
  7 siblings, 0 replies; 17+ messages in thread
From: Xiubo Li @ 2013-11-01  7:04 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 patch add and enable SGTL5000 codec support, and also specified
the corresponding SAI node.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
Signed-off-by: Alison Wang <b18965@freescale.com
---
 arch/arm/boot/dts/vf610-twr.dts | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
index e4106dd..a2d9214 100644
--- a/arch/arm/boot/dts/vf610-twr.dts
+++ b/arch/arm/boot/dts/vf610-twr.dts
@@ -34,6 +34,19 @@
 		};
 	};
 
+	sound {
+		compatible = "fsl,vf610-sgtl5000";
+		model = "vf610-sgtl5000";
+		saif-controller = <&sai2>;
+		audio-codec = <&codec>;
+		audio-routing =
+			"MIC_IN", "Mic Jack",
+			"Mic Jack", "Mic Bias",
+			"LINE_IN", "Line In Jack",
+			"Headphone Jack", "HP_OUT",
+			"Ext Spk", "LINE_OUT";
+	};
+
 };
 
 &fec0 {
@@ -55,6 +68,12 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_i2c0_1>;
 	status = "okay";
+
+	codec: sgtl5000@0a {
+		compatible = "fsl,sgtl5000";
+		reg = <0x0a>;
+		clocks = <&clks VF610_CLK_SAI2>;
+	};
 };
 
 &sai2 {
-- 
1.8.4



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCHv2 8/8] Documentation: Add device tree bindings for Freescale VF610 sound.
  2013-11-01  7:04 Xiubo Li
                   ` (6 preceding siblings ...)
  2013-11-01  7:04 ` [PATCHv2 7/8] ARM: dts: Enable SGTL5000 codec based audio driver node for VF610 Xiubo Li
@ 2013-11-01  7:04 ` Xiubo Li
  7 siblings, 0 replies; 17+ messages in thread
From: Xiubo Li @ 2013-11-01  7:04 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 the Document for Freescale VF610 sound driver under
Documentation/devicetree/bindings/sound/.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 .../bindings/sound/fsl_audio_sgt15000_vf610.txt    | 46 ++++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/fsl_audio_sgt15000_vf610.txt

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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCHv2 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
  2013-11-01  7:04 ` [PATCHv2 1/8] ALSA: Add SAI SoC Digital Audio Interface driver Xiubo Li
@ 2013-11-01  8:59   ` Nicolin Chen
  2013-11-01 18:25   ` Mark Brown
  1 sibling, 0 replies; 17+ messages in thread
From: Nicolin Chen @ 2013-11-01  8:59 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, LW, oskar, shawn.guo,
	b18965, devicetree, linux-doc, linux-kernel, linux-arm-kernel,
	alsa-devel, linuxppc-dev

Hi Xiubo,

On Fri, Nov 01, 2013 at 03:04:48PM +0800, Xiubo Li wrote:
> This adds Freescale SAI ASoC Audio support.
> This implementation is only compatible with device tree definition.
> Features:
> o Supports playback/capture
> o Supports 16/20/24 bit PCM
> o Supports 8k - 96k sample rates
> o Supports slave mode only.
> 

Just for curiosity, I found there're quite a bit code actually support
I2S master mode such as set_sysclk() and register configuration to FMT
SND_SOC_DAIFMT_CBS_CFS.

Is that really "supports slave mode only"? Or just you haven't make
it properly work?

> diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
> index b7ab71f..9a8851e 100644
> --- a/sound/soc/fsl/Kconfig
> +++ b/sound/soc/fsl/Kconfig
> @@ -213,3 +213,19 @@ config SND_SOC_IMX_MC13783
>  	select SND_SOC_IMX_PCM_DMA
>  
>  endif # SND_IMX_SOC
> +
> +menuconfig SND_FSL_SOC
> +	tristate "SoC Audio for Freescale FSL CPUs"
> +	help
> +	  Say Y or M if you want to add support for codecs attached to
> +	  the FSL CPUs.
> +
> +	  This will enable Freeacale SAI, SGT15000 codec.
> +
> +if SND_FSL_SOC

Why check this here? SAI should be a regular IP module which can be used by
other SoC as well. We would never know if next i.MX SoC won't contain SAI.

> +
> +config SND_SOC_FSL_SAI
> +	tristate
> +	select SND_SOC_GENERIC_DMAENGINE_PCM

I think you should keep SAI beside SSI/SPDIF directly.

> +
> +endif # SND_FSL_SOC
> diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
> index 8db705b..e5acc03 100644
> --- a/sound/soc/fsl/Makefile
> +++ b/sound/soc/fsl/Makefile
> @@ -56,3 +56,8 @@ obj-$(CONFIG_SND_SOC_IMX_SGTL5000) += snd-soc-imx-sgtl5000.o
>  obj-$(CONFIG_SND_SOC_IMX_WM8962) += snd-soc-imx-wm8962.o
>  obj-$(CONFIG_SND_SOC_IMX_SPDIF) += snd-soc-imx-spdif.o
>  obj-$(CONFIG_SND_SOC_IMX_MC13783) += snd-soc-imx-mc13783.o
> +
> +# FSL ARM SAI/SGT15000 Platform Support

Should be SGTL5000, not SGT1. And SAI should not be used with SGTL5000 only,
it's a bit confusing to mention SGTL5000 here.

> +snd-soc-fsl-sai-objs := fsl-sai.o

And I think it should be better to put it along with fsl-ssi.o and fsl-spdif.o

> +
> +obj-$(CONFIG_SND_SOC_FSL_SAI) += snd-soc-fsl-sai.o

Ditto

> diff --git a/sound/soc/fsl/fsl-sai.c b/sound/soc/fsl/fsl-sai.c
> new file mode 100644
> index 0000000..bb57e67
> --- /dev/null
> +++ b/sound/soc/fsl/fsl-sai.c
> @@ -0,0 +1,472 @@
> +/*
> + * Freescale SAI ALSA SoC Digital Audio Interface driver.
> + *
> + * Copyright 2012-2013 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.

There're too many double space inside. Could you pls revise it?

> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/of_address.h>
> +#include <sound/core.h>
> +#include <sound/pcm_params.h>
> +#include <linux/delay.h>
> +#include <linux/dmaengine.h>
> +#include <sound/dmaengine_pcm.h>

I think it's better to keep <sound/xxx.h> together. And basically
we can keep header files ordered by alphabet.

> +
> +#include "fsl-sai.h"
> +
> +static int fsl_sai_set_dai_sysclk_tr(struct snd_soc_dai *cpu_dai,
> +		int clk_id, unsigned int freq, int fsl_dir)
> +{
> +	u32 val_cr2, reg_cr2;
> +	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
> +
> +	if (fsl_dir == FSL_FMT_TRANSMITTER)
> +		reg_cr2 = FSL_SAI_TCR2;
> +	else
> +		reg_cr2 = FSL_SAI_RCR2;
> +
> +	val_cr2 = readl(sai->base + reg_cr2);
> +	switch (clk_id) {
> +	case FSL_SAI_CLK_BUS:
> +		val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK;
> +		val_cr2 |= FSL_SAI_CR2_MSEL_BUS;
> +		break;
> +	case FSL_SAI_CLK_MAST1:
> +		val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK;
> +		val_cr2 |= FSL_SAI_CR2_MSEL_MCLK1;
> +		break;
> +	case FSL_SAI_CLK_MAST2:
> +		val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK;
> +		val_cr2 |= FSL_SAI_CR2_MSEL_MCLK2;
> +		break;
> +	case FSL_SAI_CLK_MAST3:
> +		val_cr2 &= ~FSL_SAI_CR2_MSEL_MASK;
> +		val_cr2 |= FSL_SAI_CR2_MSEL_MCLK3;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	writel(val_cr2, sai->base + reg_cr2);
> +
> +	return 0;
> +}
> +
> +static int fsl_sai_set_dai_sysclk(struct snd_soc_dai *cpu_dai,
> +		int clk_id, unsigned int freq, int dir)
> +{
> +	int ret;
> +
> +	if (dir == SND_SOC_CLOCK_IN)
> +		return 0;
> +
> +	ret = fsl_sai_set_dai_sysclk_tr(cpu_dai, clk_id, freq,
> +					FSL_FMT_TRANSMITTER);
> +	if (ret) {
> +		dev_err(cpu_dai->dev,
> +				"Cannot set sai's transmitter sysclk: %d\n",
> +				ret);
> +		return ret;
> +	}
> +
> +	ret = fsl_sai_set_dai_sysclk_tr(cpu_dai, clk_id, freq,
> +					FSL_FMT_RECEIVER);
> +	if (ret) {
> +		dev_err(cpu_dai->dev,
> +				"Cannot set sai's receiver sysclk: %d\n",
> +				ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int fsl_sai_set_dai_clkdiv(struct snd_soc_dai *cpu_dai,
> +		int div_id, int div)
> +{
> +	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
> +	u32 tcr2, rcr2;
> +
> +	if (div_id == FSL_SAI_TX_DIV) {
> +		tcr2 = readl(sai->base + FSL_SAI_TCR2);
> +		tcr2 &= ~FSL_SAI_CR2_DIV_MASK;
> +		tcr2 |= FSL_SAI_CR2_DIV(div);
> +		writel(tcr2, sai->base + FSL_SAI_TCR2);
> +
> +	} else if (div_id == FSL_SAI_RX_DIV) {
> +		rcr2 = readl(sai->base + FSL_SAI_RCR2);
> +		rcr2 &= ~FSL_SAI_CR2_DIV_MASK;
> +		rcr2 |= FSL_SAI_CR2_DIV(div);
> +		writel(rcr2, sai->base + FSL_SAI_RCR2);
> +
> +	} else
> +		return -EINVAL;

It would look better if using switch-case. And the last 'else'
could be 'default:'.

> +
> +	return 0;
> +}
> +
> +static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai *cpu_dai,
> +				unsigned int fmt, int fsl_dir)
> +{
> +	u32 val_cr2, val_cr3, val_cr4, reg_cr2, reg_cr3, reg_cr4;
> +	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
> +
> +	if (fsl_dir == FSL_FMT_TRANSMITTER) {
> +		reg_cr2 = FSL_SAI_TCR2;
> +		reg_cr3 = FSL_SAI_TCR3;
> +		reg_cr4 = FSL_SAI_TCR4;
> +	} else {
> +		reg_cr2 = FSL_SAI_RCR2;
> +		reg_cr3 = FSL_SAI_RCR3;
> +		reg_cr4 = FSL_SAI_RCR4;
> +	}
> +
> +	val_cr2 = readl(sai->base + reg_cr2);
> +	val_cr3 = readl(sai->base + reg_cr3);
> +	val_cr4 = readl(sai->base + reg_cr4);
> +
> +	if (sai->fbt == FSL_SAI_FBT_MSB)
> +		val_cr4 |= FSL_SAI_CR4_MF;
> +	else if (sai->fbt == FSL_SAI_FBT_LSB)
> +		val_cr4 &= ~FSL_SAI_CR4_MF;
> +
> +	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> +	case SND_SOC_DAIFMT_I2S:
> +		val_cr4 |= FSL_SAI_CR4_FSE;
> +		val_cr4 |= FSL_SAI_CR4_FSP;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> +	case SND_SOC_DAIFMT_IB_IF:
> +		val_cr4 |= FSL_SAI_CR4_FSP;
> +		val_cr2 &= ~FSL_SAI_CR2_BCP;
> +		break;
> +	case SND_SOC_DAIFMT_IB_NF:
> +		val_cr4 &= ~FSL_SAI_CR4_FSP;
> +		val_cr2 &= ~FSL_SAI_CR2_BCP;
> +		break;
> +	case SND_SOC_DAIFMT_NB_IF:
> +		val_cr4 |= FSL_SAI_CR4_FSP;
> +		val_cr2 |= FSL_SAI_CR2_BCP;
> +		break;
> +	case SND_SOC_DAIFMT_NB_NF:
> +		val_cr4 &= ~FSL_SAI_CR4_FSP;
> +		val_cr2 |= FSL_SAI_CR2_BCP;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> +	case SND_SOC_DAIFMT_CBS_CFS:
> +		val_cr2 |= FSL_SAI_CR2_BCD_MSTR;
> +		val_cr4 |= FSL_SAI_CR4_FSD_MSTR;
> +		break;
> +	case SND_SOC_DAIFMT_CBM_CFM:
> +		val_cr2 &= ~FSL_SAI_CR2_BCD_MSTR;
> +		val_cr4 &= ~FSL_SAI_CR4_FSD_MSTR;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	val_cr3 |= FSL_SAI_CR3_TRCE;
> +
> +	if (fsl_dir == FSL_FMT_RECEIVER)
> +		val_cr2 |= FSL_SAI_CR2_SYNC;
> +
> +	writel(val_cr2, sai->base + reg_cr2);
> +	writel(val_cr3, sai->base + reg_cr3);
> +	writel(val_cr4, sai->base + reg_cr4);
> +
> +	return 0;

> +

Pls drop this extra line.

> +}
> +
> +static int fsl_sai_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
> +{
> +	int ret;
> +
> +	ret = fsl_sai_set_dai_fmt_tr(cpu_dai, fmt, FSL_FMT_TRANSMITTER);
> +	if (ret) {
> +		dev_err(cpu_dai->dev,
> +				"Cannot set sai's transmitter format: %d\n",
> +				ret);
> +		return ret;
> +	}
> +
> +	ret = fsl_sai_set_dai_fmt_tr(cpu_dai, fmt, FSL_FMT_RECEIVER);
> +	if (ret) {
> +		dev_err(cpu_dai->dev,
> +				"Cannot set sai's receiver format: %d\n",
> +				ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int fsl_sai_set_dai_tdm_slot(struct snd_soc_dai *cpu_dai,
> +		unsigned int tx_mask, unsigned int rx_mask,
> +		int slots, int slot_width)
> +{
> +	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
> +	u32 tcr4, rcr4;
> +
> +	tcr4 = readl(sai->base + FSL_SAI_TCR4);
> +	tcr4 &= ~FSL_SAI_CR4_FRSZ_MASK;
> +	tcr4 |= FSL_SAI_CR4_FRSZ(2);
> +	writel(tcr4, sai->base + FSL_SAI_TCR4);
> +	writel(tx_mask, sai->base + FSL_SAI_TMR);
> +
> +	rcr4 = readl(sai->base + FSL_SAI_RCR4);
> +	rcr4 &= ~FSL_SAI_CR4_FRSZ_MASK;
> +	rcr4 |= FSL_SAI_CR4_FRSZ(2);
> +	writel(rcr4, sai->base + FSL_SAI_RCR4);
> +	writel(rx_mask, sai->base + FSL_SAI_RMR);
> +
> +	return 0;
> +}
> +
> +static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
> +		struct snd_pcm_hw_params *params,
> +		struct snd_soc_dai *cpu_dai)
> +{
> +	u32 val_cr4, val_cr5, reg_cr4, reg_cr5, word_width;
> +	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
> +
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +		reg_cr4 = FSL_SAI_TCR4;
> +		reg_cr5 = FSL_SAI_TCR5;
> +	} else {
> +		reg_cr4 = FSL_SAI_RCR4;
> +		reg_cr5 = FSL_SAI_RCR5;
> +	}
> +
> +	val_cr4 = readl(sai->base + reg_cr4);
> +	val_cr4 &= ~FSL_SAI_CR4_SYWD_MASK;
> +
> +	val_cr5 = readl(sai->base + reg_cr5);
> +	val_cr5 &= ~FSL_SAI_CR5_WNW_MASK;
> +	val_cr5 &= ~FSL_SAI_CR5_W0W_MASK;
> +	val_cr5 &= ~FSL_SAI_CR5_FBT_MASK;
> +
> +	switch (params_format(params)) {
> +	case SNDRV_PCM_FORMAT_S16_LE:
> +		word_width = 16;
> +		break;
> +	case SNDRV_PCM_FORMAT_S20_3LE:
> +		word_width = 20;
> +		break;
> +	case SNDRV_PCM_FORMAT_S24_LE:
> +		word_width = 24;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	val_cr4 |= FSL_SAI_CR4_SYWD(word_width);
> +	val_cr5 |= FSL_SAI_CR5_WNW(word_width);
> +	val_cr5 |= FSL_SAI_CR5_W0W(word_width);
> +
> +	if (sai->fbt == FSL_SAI_FBT_MSB)
> +		val_cr5 |= FSL_SAI_CR5_FBT(word_width - 1);
> +	else if (sai->fbt == FSL_SAI_FBT_LSB)
> +		val_cr5 |= FSL_SAI_CR5_FBT(0);
> +
> +	writel(val_cr4, sai->base + reg_cr4);
> +	writel(val_cr5, sai->base + reg_cr5);
> +
> +	return 0;
> +}
> +
> +static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
> +		struct snd_soc_dai *dai)
> +{
> +	struct fsl_sai *sai = snd_soc_dai_get_drvdata(dai);
> +	unsigned int tcsr, rcsr;
> +
> +	tcsr = readl(sai->base + FSL_SAI_TCSR);
> +	rcsr = readl(sai->base + FSL_SAI_RCSR);
> +
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +		tcsr |= FSL_SAI_CSR_FRDE;
> +		rcsr &= ~FSL_SAI_CSR_FRDE;
> +	} else {
> +		rcsr |= FSL_SAI_CSR_FRDE;
> +		tcsr &= ~FSL_SAI_CSR_FRDE;
> +	}
> +
> +	switch (cmd) {
> +	case SNDRV_PCM_TRIGGER_START:
> +	case SNDRV_PCM_TRIGGER_RESUME:
> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> +		tcsr |= FSL_SAI_CSR_TERE;
> +		rcsr |= FSL_SAI_CSR_TERE;
> +		writel(rcsr, sai->base + FSL_SAI_RCSR);
> +		udelay(10);

Does SAI really needs this udelay() here? Required by IP's operation flow?
If so, I think it's better to add comments here to explain it.

> +		writel(tcsr, sai->base + FSL_SAI_TCSR);
> +		break;
> +
> +	case SNDRV_PCM_TRIGGER_STOP:
> +	case SNDRV_PCM_TRIGGER_SUSPEND:
> +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> +		if (!(dai->playback_active || dai->capture_active)) {
> +			tcsr &= ~FSL_SAI_CSR_TERE;
> +			rcsr &= ~FSL_SAI_CSR_TERE;
> +		}
> +		writel(rcsr, sai->base + FSL_SAI_RCSR);
> +		udelay(10);
> +		writel(tcsr, sai->base + FSL_SAI_TCSR);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
> +	.set_sysclk	= fsl_sai_set_dai_sysclk,
> +	.set_clkdiv	= fsl_sai_set_dai_clkdiv,
> +	.set_fmt	= fsl_sai_set_dai_fmt,
> +	.set_tdm_slot	= fsl_sai_set_dai_tdm_slot,
> +	.hw_params	= fsl_sai_hw_params,
> +	.trigger	= fsl_sai_trigger,
> +};
> +
> +static int fsl_sai_dai_probe(struct snd_soc_dai *dai)
> +{
> +	int ret;
> +	struct fsl_sai *sai = dev_get_drvdata(dai->dev);
> +
> +	ret = clk_prepare_enable(sai->clk);
> +	if (ret)
> +		return ret;
> +
> +	writel(0x0, sai->base + FSL_SAI_RCSR);
> +	writel(0x0, sai->base + FSL_SAI_TCSR);
> +	writel(sai->dma_params_tx.maxburst, sai->base + FSL_SAI_TCR1);
> +	writel(sai->dma_params_rx.maxburst, sai->base + FSL_SAI_RCR1);
> +
> +	dai->playback_dma_data = &sai->dma_params_tx;
> +	dai->capture_dma_data = &sai->dma_params_rx;
> +
> +	snd_soc_dai_set_drvdata(dai, sai);
> +
> +	return 0;
> +}
> +
> +int fsl_sai_dai_remove(struct snd_soc_dai *dai)

static

> +{
> +	struct fsl_sai *sai = dev_get_drvdata(dai->dev);
> +
> +	clk_disable_unprepare(sai->clk);
> +
> +	return 0;
> +}
> +
> +static struct snd_soc_dai_driver fsl_sai_dai = {
> +	.probe = fsl_sai_dai_probe,
> +	.remove = fsl_sai_dai_remove,
> +	.playback = {
> +		.channels_min = 1,
> +		.channels_max = 2,
> +		.rates = SNDRV_PCM_RATE_8000_96000,
> +		.formats = FSL_SAI_FORMATS,
> +	},
> +	.capture = {
> +		.channels_min = 1,
> +		.channels_max = 2,
> +		.rates = SNDRV_PCM_RATE_8000_96000,
> +		.formats = FSL_SAI_FORMATS,
> +	},
> +	.ops = &fsl_sai_pcm_dai_ops,
> +};
> +
> +static const struct snd_soc_component_driver fsl_component = {
> +	.name           = "fsl-sai",
> +};
> +
> +static int fsl_sai_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	struct fsl_sai *sai;
> +	int ret = 0;
> +
> +	sai = devm_kzalloc(&pdev->dev, sizeof(*sai), GFP_KERNEL);
> +	if (!sai)
> +		return -ENOMEM;
> +
> +	sai->fbt = FSL_SAI_FBT_MSB;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	sai->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(sai->base))
> +		return PTR_ERR(sai->base);
> +
> +	sai->clk = devm_clk_get(&pdev->dev, "sai");
> +	if (IS_ERR(sai->clk)) {
> +		dev_err(&pdev->dev, "Cannot get sai's clock: %d\n", ret);
> +		return PTR_ERR(sai->clk);
> +	}
> +
> +	sai->dma_params_rx.addr = res->start + FSL_SAI_RDR;
> +	sai->dma_params_rx.maxburst = 6;
> +
> +	sai->dma_params_tx.addr = res->start + FSL_SAI_TDR;
> +	sai->dma_params_tx.maxburst = 6;
> +
> +	platform_set_drvdata(pdev, sai);
> +
> +	ret = devm_snd_soc_register_component(&pdev->dev, &fsl_component,
> +			&fsl_sai_dai, 1);
> +	if (ret)
> +		return ret;
> +
> +	ret = snd_dmaengine_pcm_register(&pdev->dev, NULL,
> +			SND_DMAENGINE_PCM_FLAG_NO_RESIDUE);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int fsl_sai_remove(struct platform_device *pdev)
> +{
> +	snd_dmaengine_pcm_unregister(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id fsl_sai_ids[] = {
> +	{ .compatible = "fsl,vf610-sai", },

> +	{ /*sentinel*/ }

I think this could be left in blank without comments inside.
And if you really want to add it, pls add like: /* sentinel */
						  ^        ^
> +};
> +
> +static struct platform_driver fsl_sai_driver = {
> +	.probe = fsl_sai_probe,
> +	.remove = fsl_sai_remove,
> +
> +	.driver = {
> +		.name = "fsl-sai",
> +		.owner = THIS_MODULE,
> +		.of_match_table = fsl_sai_ids,
> +	},
> +};
> +module_platform_driver(fsl_sai_driver);
> +
> +MODULE_AUTHOR("Xiubo Li, <Li.Xiubo@freescale.com>");
> +MODULE_AUTHOR("Alison Wang, <b18965@freescale.com>");
> +MODULE_DESCRIPTION("Freescale Soc SAI Interface");
> +MODULE_LICENSE("GPL");

Should be better if added:
MODULE_ALIAS("platform:fsl-sai");

This would support module auto-load feature in some Linux-OS.

Best regards,
Nicolin Chen

> diff --git a/sound/soc/fsl/fsl-sai.h b/sound/soc/fsl/fsl-sai.h
> new file mode 100644
> index 0000000..1637679
> --- /dev/null
> +++ b/sound/soc/fsl/fsl-sai.h
> @@ -0,0 +1,120 @@
> +/*
> + * Copyright 2012-2013 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __FSL_SAI_H
> +#define __FSL_SAI_H
> +
> +#include <sound/dmaengine_pcm.h>
> +
> +#define FSL_SAI_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |\
> +			 SNDRV_PCM_FMTBIT_S20_3LE |\
> +			 SNDRV_PCM_FMTBIT_S24_LE)
> +
> +/* SAI Transmit/Recieve Control Register */
> +#define FSL_SAI_TCSR		0x00
> +#define FSL_SAI_RCSR		0x80
> +#define FSL_SAI_CSR_TERE	BIT(31)
> +#define FSL_SAI_CSR_FWF		BIT(17)
> +#define FSL_SAI_CSR_FRIE	BIT(8)
> +#define FSL_SAI_CSR_FRDE	BIT(0)
> +
> +/* SAI Transmit Data/FIFO/MASK Register */
> +#define FSL_SAI_TDR		0x20
> +#define FSL_SAI_TFR		0x40
> +#define FSL_SAI_TMR		0x60
> +
> +/* SAI Recieve Data/FIFO/MASK Register */
> +#define FSL_SAI_RDR		0xa0
> +#define FSL_SAI_RFR		0xc0
> +#define FSL_SAI_RMR		0xe0
> +
> +/* SAI Transmit and Recieve Configuration 1 Register */
> +#define FSL_SAI_TCR1		0x04
> +#define FSL_SAI_RCR1		0x84
> +
> +/* SAI Transmit and Recieve Configuration 2 Register */
> +#define FSL_SAI_TCR2		0x08
> +#define FSL_SAI_RCR2		0x88
> +#define FSL_SAI_CR2_SYNC	BIT(30)
> +#define FSL_SAI_CR2_MSEL_MASK	(0xff << 26)
> +#define FSL_SAI_CR2_MSEL_BUS	0
> +#define FSL_SAI_CR2_MSEL_MCLK1	BIT(26)
> +#define FSL_SAI_CR2_MSEL_MCLK2	BIT(27)
> +#define FSL_SAI_CR2_MSEL_MCLK3	(BIT(26) | BIT(27))
> +#define FSL_SAI_CR2_BCP		BIT(25)
> +#define FSL_SAI_CR2_BCD_MSTR	BIT(24)
> +#define FSL_SAI_CR2_DIV(x)	(x)
> +#define FSL_SAI_CR2_DIV_MASK	0xff
> +
> +/* SAI Transmit and Recieve Configuration 3 Register */
> +#define FSL_SAI_TCR3		0x0c
> +#define FSL_SAI_RCR3		0x8c
> +#define FSL_SAI_CR3_TRCE	BIT(16)
> +#define FSL_SAI_CR3_WDFL(x)	(x)
> +#define FSL_SAI_CR3_WDFL_MASK	0x1f
> +
> +/* SAI Transmit and Recieve Configuration 4 Register */
> +#define FSL_SAI_TCR4		0x10
> +#define FSL_SAI_RCR4		0x90
> +#define FSL_SAI_CR4_FRSZ(x)	(((x) - 1) << 16)
> +#define FSL_SAI_CR4_FRSZ_MASK	(0x1f << 16)
> +#define FSL_SAI_CR4_SYWD(x)	(((x) - 1) << 8)
> +#define FSL_SAI_CR4_SYWD_MASK	(0x1f << 8)
> +#define FSL_SAI_CR4_MF		BIT(4)
> +#define FSL_SAI_CR4_FSE		BIT(3)
> +#define FSL_SAI_CR4_FSP		BIT(1)
> +#define FSL_SAI_CR4_FSD_MSTR	BIT(0)
> +
> +/* SAI Transmit and Recieve Configuration 5 Register */
> +#define FSL_SAI_TCR5		0x14
> +#define FSL_SAI_RCR5		0x94
> +#define FSL_SAI_CR5_WNW(x)	(((x) - 1) << 24)
> +#define FSL_SAI_CR5_WNW_MASK	(0x1f << 24)
> +#define FSL_SAI_CR5_W0W(x)	(((x) - 1) << 16)
> +#define FSL_SAI_CR5_W0W_MASK	(0x1f << 16)
> +#define FSL_SAI_CR5_FBT(x)	((x) << 8)
> +#define FSL_SAI_CR5_FBT_MASK	(0x1f << 8)
> +
> +/* SAI audio dividers */
> +#define FSL_SAI_TX_DIV		0
> +#define FSL_SAI_RX_DIV		1
> +
> +/* SAI type */
> +#define FSL_SAI_DMA		BIT(0)
> +#define FSL_SAI_USE_AC97	BIT(1)
> +#define FSL_SAI_NET		BIT(2)
> +#define FSL_SAI_TRA_SYN		BIT(3)
> +#define FSL_SAI_REC_SYN		BIT(4)
> +#define FSL_SAI_USE_I2S_SLAVE	BIT(5)
> +
> +#define FSL_FMT_TRANSMITTER	0
> +#define FSL_FMT_RECEIVER	1
> +
> +/* SAI clock sources */
> +#define FSL_SAI_CLK_BUS		0
> +#define FSL_SAI_CLK_MAST1	1
> +#define FSL_SAI_CLK_MAST2	2
> +#define FSL_SAI_CLK_MAST3	3
> +
> +enum fsl_sai_fbt {
> +	FSL_SAI_FBT_MSB,
> +	FSL_SAI_FBT_LSB,
> +};
> +
> +struct fsl_sai {
> +	struct clk *clk;
> +
> +	void __iomem *base;
> +
> +	enum fsl_sai_fbt fbt;
> +
> +	struct snd_dmaengine_dai_dma_data dma_params_rx;
> +	struct snd_dmaengine_dai_dma_data dma_params_tx;
> +};
> +
> +#endif /* __FSL_SAI_H */
> -- 
> 1.8.4
> 



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv2 5/8] ASoC: SGTL5000: Enhance the SGTL5000 codec driver about regulator.
  2013-11-01  7:04 ` [PATCHv2 5/8] ASoC: SGTL5000: Enhance the SGTL5000 codec driver about regulator Xiubo Li
@ 2013-11-01 10:02   ` Nicolin Chen
  2013-11-01 18:50   ` Mark Brown
  1 sibling, 0 replies; 17+ messages in thread
From: Nicolin Chen @ 2013-11-01 10:02 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, LW, oskar, shawn.guo,
	b18965, devicetree, linux-doc, linux-kernel, linux-arm-kernel,
	alsa-devel, linuxppc-dev

On Fri, Nov 01, 2013 at 03:04:52PM +0800, Xiubo Li wrote:
> On VF610 series there are no regulators used, and now whether the
> CONFIG_REGULATOR mirco is enabled or not, for the VF610 audio

micro? or macro?

> patch series, the board cannot be probe successfully.
> And this patch will solve this issue.
> 

I finally got your idea about what this patch does. But it seems to
be more likely a work around. Maybe I here can't think it through
comprehensively, but I think you can try to add a fixed regulator to
sgtl5000 in the devicetree, rather than disabling common functions
even if not breaking others.

> Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> ---
>  sound/soc/codecs/sgtl5000.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
> index 1f4093f..c2f6d86 100644
> --- a/sound/soc/codecs/sgtl5000.c
> +++ b/sound/soc/codecs/sgtl5000.c
> @@ -61,6 +61,7 @@ static const struct reg_default sgtl5000_reg_defaults[] = {
>  	{ SGTL5000_DAP_AVC_DECAY,		0x0050 },
>  };
>  
> +#ifdef CONFIG_REGULATOR
>  /* regulator supplies for sgtl5000, VDDD is an optional external supply */
>  enum sgtl5000_regulator_supplies {
>  	VDDA,
> @@ -93,6 +94,9 @@ static struct regulator_init_data ldo_init_data = {
>  	.num_consumer_supplies = 1,
>  	.consumer_supplies = &ldo_consumer[0],
>  };
> +#else
> +#define SGTL5000_SUPPLY_NUM 0
> +#endif
>  
>  /*
>   * sgtl5000 internal ldo regulator,
> @@ -112,7 +116,9 @@ struct sgtl5000_priv {
>  	int master;	/* i2s master or not */
>  	int fmt;	/* i2s data format */
>  	struct regulator_bulk_data supplies[SGTL5000_SUPPLY_NUM];
> +#ifdef CONFIG_REGULATOR
>  	struct ldo_regulator *ldo;
> +#endif
>  	struct regmap *regmap;
>  	struct clk *mclk;
>  };
> @@ -879,6 +885,7 @@ static int ldo_regulator_remove(struct snd_soc_codec *codec)
>  	return 0;
>  }
>  #else
> +#ifndef CONFIG_SND_SOC_FSL_SGTL5000_VF610

Checking a platform or SoC specific define here is just a bit....

Could you pls find a better solution? Like I said at first, use fixed regulator
or figure out why your System would hang with current sgtl5000 driver. If it
really has a critical bug in its regulator-related code, I think you can then
make a greater contribution by fixing the bug rather than bypassing it.

Best regards,
Nicolin Chen

>  static int ldo_regulator_register(struct snd_soc_codec *codec,
>  				struct regulator_init_data *init_data,
>  				int voltage)
> @@ -886,6 +893,7 @@ static int ldo_regulator_register(struct snd_soc_codec *codec,
>  	dev_err(codec->dev, "this setup needs regulator support in the kernel\n");
>  	return -EINVAL;
>  }
> +#endif
>  
>  static int ldo_regulator_remove(struct snd_soc_codec *codec)
>  {
> @@ -1137,6 +1145,7 @@ static int sgtl5000_resume(struct snd_soc_codec *codec)
>  #define sgtl5000_resume  NULL
>  #endif	/* CONFIG_SUSPEND */
>  
> +#ifdef CONFIG_REGULATOR
>  /*
>   * sgtl5000 has 3 internal power supplies:
>   * 1. VAG, normally set to vdda/2
> @@ -1373,6 +1382,7 @@ err_regulator_free:
>  	return ret;
>  
>  }
> +#endif
>  
>  static int sgtl5000_probe(struct snd_soc_codec *codec)
>  {
> @@ -1387,6 +1397,7 @@ static int sgtl5000_probe(struct snd_soc_codec *codec)
>  		return ret;
>  	}
>  
> +#ifdef CONFIG_REGULATOR
>  	ret = sgtl5000_enable_regulators(codec);
>  	if (ret)
>  		return ret;
> @@ -1395,6 +1406,7 @@ static int sgtl5000_probe(struct snd_soc_codec *codec)
>  	ret = sgtl5000_set_power_regs(codec);
>  	if (ret)
>  		goto err;
> +#endif
>  
>  	/* enable small pop, introduce 400ms delay in turning off */
>  	snd_soc_update_bits(codec, SGTL5000_CHIP_REF_CTRL,
> -- 
> 1.8.4
> 



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv2 6/8] ASoC: fsl: add SGTL5000 based audio machine driver.
  2013-11-01  7:04 ` [PATCHv2 6/8] ASoC: fsl: add SGTL5000 based audio machine driver Xiubo Li
@ 2013-11-01 10:17   ` Oskar Schirmer
  2013-11-01 10:28   ` Nicolin Chen
  2013-11-01 18:40   ` Mark Brown
  2 siblings, 0 replies; 17+ messages in thread
From: Oskar Schirmer @ 2013-11-01 10:17 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, LW, shawn.guo, b42378,
	b18965, devicetree, linux-doc, linux-kernel, linux-arm-kernel,
	alsa-devel, linuxppc-dev

Not that it would improve functionality, but:

On Fri, Nov 01, 2013 at 15:04:53 +0800, Xiubo Li wrote:
[...]
> diff --git a/sound/soc/fsl/fsl-sgtl5000-vf610.c b/sound/soc/fsl/fsl-sgtl5000-vf610.c
> new file mode 100644
> index 0000000..f535b42
> --- /dev/null
> +++ b/sound/soc/fsl/fsl-sgtl5000-vf610.c
> @@ -0,0 +1,208 @@
> +/*
> + * Freeacale ALSA SoC Audio using SGT1500 as codec.
          ^                             ^   ^
     "Freescale"                    "SGTL5000"

regards,
  Oskar

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv2 6/8] ASoC: fsl: add SGTL5000 based audio machine driver.
  2013-11-01  7:04 ` [PATCHv2 6/8] ASoC: fsl: add SGTL5000 based audio machine driver Xiubo Li
  2013-11-01 10:17   ` Oskar Schirmer
@ 2013-11-01 10:28   ` Nicolin Chen
  2013-11-01 12:07     ` Shawn Guo
  2013-11-01 18:40   ` Mark Brown
  2 siblings, 1 reply; 17+ messages in thread
From: Nicolin Chen @ 2013-11-01 10:28 UTC (permalink / raw)
  To: Xiubo Li, shawn.guo
  Cc: r65073, timur, lgirdwood, broonie, r64188, rob.herring,
	pawel.moll, mark.rutland, swarren, ian.campbell, rob, linux,
	perex, tiwai, grant.likely, fabio.estevam, LW, oskar, b18965,
	devicetree, linux-doc, linux-kernel, linux-arm-kernel,
	alsa-devel, linuxppc-dev

Hi Xiubo,

On Fri, Nov 01, 2013 at 03:04:53PM +0800, Xiubo Li wrote:
> This is the SGTL5000 codec based audio driver supported with both
> playback and capture dai link implemention.
> 
> This implementation is only compatible with device tree definition.
> 
> Signed-off-by: Alison Wang <b18965@freescale.com
> Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> 
> Conflicts:
> 	sound/soc/fsl/Makefile
> ---
>  sound/soc/fsl/Kconfig              |  10 ++
>  sound/soc/fsl/Makefile             |   2 +

>  sound/soc/fsl/fsl-sgtl5000-vf610.c | 208 +++++++++++++++++++++++++++++++++++++

I just doubt if this file naming is appropriate. Even if we might not have
rigor rule for the file names, according to existing ones, they are all in
a same pattern: [SoC name]-[codec name].c

"imx-sgtl5000.c" for example

I think it would make user less confused about what this file exactly is if
this machine driver also follow the pattern: vf610-sgtl5000.c


@Shawn

What do you think about the file name?

>  3 files changed, 220 insertions(+)
>  create mode 100644 sound/soc/fsl/fsl-sgtl5000-vf610.c
> 
> diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
> index 9a8851e..1b835ba 100644
> --- a/sound/soc/fsl/Kconfig
> +++ b/sound/soc/fsl/Kconfig
> @@ -228,4 +228,14 @@ config SND_SOC_FSL_SAI
>  	tristate
>  	select SND_SOC_GENERIC_DMAENGINE_PCM
>  
> +config SND_SOC_FSL_SGTL5000_VF610

Same problem with the this define.

> +	tristate "SoC Audio support for FSL boards with sgtl5000"

And 'FSL' here confuses me a lot. Because those boards based on i.MX series
also could be called FSL boards.

> +	depends on OF && I2C
> +	select SND_SOC_FSL_SAI
> +	select SND_SOC_FSL_PCM
> +	select SND_SOC_SGTL5000
> +	help
> +	  Say Y if you want to add support for SoC audio on an FSL board with
> +	  a sgtl5000 codec.
> +
>  endif # SND_FSL_SOC
> diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
> index e5acc03..26fc551 100644
> --- a/sound/soc/fsl/Makefile
> +++ b/sound/soc/fsl/Makefile
> @@ -59,5 +59,7 @@ 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-sgtl5000-vf610-objs := fsl-sgtl5000-vf610.o
>  
>  obj-$(CONFIG_SND_SOC_FSL_SAI) += snd-soc-fsl-sai.o
> +obj-$(CONFIG_SND_SOC_FSL_SGTL5000_VF610) += snd-soc-fsl-sgtl5000-vf610.o
> diff --git a/sound/soc/fsl/fsl-sgtl5000-vf610.c b/sound/soc/fsl/fsl-sgtl5000-vf610.c
> new file mode 100644
> index 0000000..f535b42
> --- /dev/null
> +++ b/sound/soc/fsl/fsl-sgtl5000-vf610.c
> @@ -0,0 +1,208 @@
> +/*
> + * Freeacale ALSA SoC Audio using SGT1500 as codec.
> + *
> + * Copyright 2012-2013 Freescale Semiconductor, Inc.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/i2c.h>
> +#include <linux/clk.h>
> +
> +#include "../codecs/sgtl5000.h"
> +#include "fsl-sai.h"
> +
> +static unsigned int sysclk_rate;
> +
> +static int fsl_sgtl5000_dai_init(struct snd_soc_pcm_runtime *rtd)

Naming issue here again.

At least from my point of view, if you actually merged imx-sgtl5000 with
vf610-sgtl5000 and made it also compatible to other freescale SoCs, you
could then fairly call it fsl_sgtl5000_xxxx.

Well, I might be a little picky here because it's a static function and
won't conflict others. Just the name here doesn't look so explicit to me.

Please reconsider about this whole file's naming.

Best regards,
Nicolin Chen

> +{
> +	int ret;
> +	struct device *dev = rtd->card->dev;
> +
> +	ret = snd_soc_dai_set_sysclk(rtd->codec_dai, SGTL5000_SYSCLK,
> +				     sysclk_rate, SND_SOC_CLOCK_IN);
> +	if (ret) {
> +		dev_err(dev, "could not set codec driver clock params :%d\n",
> +				ret);
> +		return ret;
> +	}
> +
> +	ret = snd_soc_dai_set_sysclk(rtd->cpu_dai, FSL_SAI_CLK_BUS,
> +				     sysclk_rate, SND_SOC_CLOCK_OUT);
> +	if (ret) {
> +		dev_err(dev, "could not set cpu dai driver clock params :%d\n",
> +				ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int sgtl5000_params(struct snd_pcm_substream *substream,
> +		struct snd_pcm_hw_params *params)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> +	unsigned int channels = params_channels(params);
> +
> +	/* TODO: The SAI driver should figure this out for us */
> +	switch (channels) {
> +	case 2:
> +		snd_soc_dai_set_tdm_slot(cpu_dai, 0xfffffffc, 0xfffffffc, 2, 0);
> +		break;
> +	case 1:
> +		snd_soc_dai_set_tdm_slot(cpu_dai, 0xfffffffe, 0xfffffffe, 1, 0);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static struct snd_soc_ops fsl_sgtl5000_hifi_ops = {
> +	.hw_params = sgtl5000_params,
> +};
> +
> +static struct snd_soc_dai_link fsl_sgtl5000_dai = {
> +	.name = "HiFi",
> +	.stream_name = "HiFi",
> +	.codec_dai_name = "sgtl5000",
> +	.init = &fsl_sgtl5000_dai_init,
> +	.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> +		SND_SOC_DAIFMT_CBM_CFM,
> +	.ops = &fsl_sgtl5000_hifi_ops,
> +};
> +
> +static const struct snd_soc_dapm_widget fsl_sgtl5000_dapm_widgets[] = {
> +	SND_SOC_DAPM_MIC("Mic Jack", NULL),
> +	SND_SOC_DAPM_LINE("Line In Jack", NULL),
> +	SND_SOC_DAPM_HP("Headphone Jack", NULL),
> +	SND_SOC_DAPM_SPK("Line Out Jack", NULL),
> +	SND_SOC_DAPM_SPK("Ext Spk", NULL),
> +};
> +
> +static struct snd_soc_card fsl_sgt1500_card = {
> +	.owner = THIS_MODULE,
> +	.num_links = 1,
> +	.dai_link = &fsl_sgtl5000_dai,
> +	.dapm_widgets = fsl_sgtl5000_dapm_widgets,
> +	.num_dapm_widgets = ARRAY_SIZE(fsl_sgtl5000_dapm_widgets),
> +};
> +
> +static int fsl_sgtl5000_parse_dt(struct platform_device *pdev)
> +{
> +	int ret;
> +	struct device_node *sai_np, *codec_np;
> +	struct clk *codec_clk;
> +	struct i2c_client *codec_dev;
> +	struct device_node *np = pdev->dev.of_node;
> +
> +	ret = snd_soc_of_parse_card_name(&fsl_sgt1500_card, "model");
> +	if (ret)
> +		return ret;
> +
> +	ret = snd_soc_of_parse_audio_routing(&fsl_sgt1500_card,
> +			"audio-routing");
> +	if (ret)
> +		return ret;
> +
> +	sai_np = of_parse_phandle(np, "saif-controller", 0);
> +	if (!sai_np) {
> +		dev_err(&pdev->dev, "\"saif-controller\" phandle missing or "
> +				"invalid\n");
> +		return -EINVAL;
> +	}
> +	fsl_sgtl5000_dai.cpu_of_node = sai_np;
> +	fsl_sgtl5000_dai.platform_of_node = sai_np;
> +
> +	codec_np = of_parse_phandle(np, "audio-codec", 0);
> +	if (!codec_np) {
> +		dev_err(&pdev->dev, "\"audio-codec\" phandle missing or "
> +				"invalid\n");
> +		ret = -EINVAL;
> +		goto sai_np_fail;
> +	}
> +	fsl_sgtl5000_dai.codec_of_node = codec_np;
> +
> +	codec_dev = of_find_i2c_device_by_node(codec_np);
> +	if (!codec_dev) {
> +		dev_err(&pdev->dev, "failed to find codec platform device\n");
> +		ret = PTR_ERR(codec_dev);
> +		goto codec_np_fail;
> +	}
> +
> +	codec_clk = devm_clk_get(&codec_dev->dev, NULL);
> +	if (IS_ERR(codec_clk)) {
> +		dev_err(&pdev->dev, "failed to get codec clock\n");
> +		ret = PTR_ERR(codec_clk);
> +		goto codec_np_fail;
> +	}
> +
> +	sysclk_rate = clk_get_rate(codec_clk);
> +
> +codec_np_fail:
> +	of_node_put(codec_np);
> +sai_np_fail:
> +	of_node_put(sai_np);
> +
> +	return ret;
> +}
> +
> +static int fsl_sgtl5000_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +
> +	fsl_sgt1500_card.dev = &pdev->dev;
> +
> +	ret = fsl_sgtl5000_parse_dt(pdev);
> +	if (ret) {
> +		dev_err(&pdev->dev,
> +				"parse sgtl5000 device tree failed :%d\n",
> +				ret);
> +		return ret;
> +	}
> +
> +	ret = devm_snd_soc_register_card(&pdev->dev, &fsl_sgt1500_card);
> +	if (ret) {
> +		dev_err(&pdev->dev, "register soc sound card failed :%d\n",
> +				ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int fsl_sgtl5000_remove(struct platform_device *pdev)
> +{
> +	snd_soc_unregister_card(&fsl_sgt1500_card);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id fsl_sgtl5000_dt_ids[] = {
> +	{ .compatible = "fsl,vf610-sgtl5000", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, fsl_sgtl5000_dt_ids);
> +
> +static struct platform_driver fsl_sgtl5000_driver = {
> +	.driver = {
> +		.name = "fsl-sgtl5000",
> +		.owner = THIS_MODULE,
> +		.of_match_table = fsl_sgtl5000_dt_ids,
> +	},
> +	.probe = fsl_sgtl5000_probe,
> +	.remove = fsl_sgtl5000_remove,
> +};
> +module_platform_driver(fsl_sgtl5000_driver);
> +
> +MODULE_AUTHOR("Xiubo Li <Li.Xiubo@freescale.com>");
> +MODULE_DESCRIPTION("Freescale SGTL5000 ASoC driver");
> +MODULE_LICENSE("GPL");
> -- 
> 1.8.4
> 



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv2 6/8] ASoC: fsl: add SGTL5000 based audio machine driver.
  2013-11-01 10:28   ` Nicolin Chen
@ 2013-11-01 12:07     ` Shawn Guo
  0 siblings, 0 replies; 17+ messages in thread
From: Shawn Guo @ 2013-11-01 12:07 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: Xiubo Li, r65073, timur, lgirdwood, broonie, r64188, rob.herring,
	pawel.moll, mark.rutland, swarren, ian.campbell, rob, linux,
	perex, tiwai, grant.likely, fabio.estevam, LW, oskar, b18965,
	devicetree, linux-doc, linux-kernel, linux-arm-kernel,
	alsa-devel, linuxppc-dev

On Fri, Nov 01, 2013 at 06:28:05PM +0800, Nicolin Chen wrote:
> >  sound/soc/fsl/fsl-sgtl5000-vf610.c | 208 +++++++++++++++++++++++++++++++++++++
> 
> I just doubt if this file naming is appropriate. Even if we might not have
> rigor rule for the file names, according to existing ones, they are all in
> a same pattern: [SoC name]-[codec name].c
> 
> "imx-sgtl5000.c" for example
> 
> I think it would make user less confused about what this file exactly is if
> this machine driver also follow the pattern: vf610-sgtl5000.c
> 
> 
> @Shawn
> 
> What do you think about the file name?

Yeah, it would be better to name the file following the existing the
pattern.

Shawn


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv2 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
  2013-11-01  7:04 ` [PATCHv2 1/8] ALSA: Add SAI SoC Digital Audio Interface driver Xiubo Li
  2013-11-01  8:59   ` Nicolin Chen
@ 2013-11-01 18:25   ` Mark Brown
  1 sibling, 0 replies; 17+ messages in thread
From: Mark Brown @ 2013-11-01 18:25 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: 1005 bytes --]

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

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

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

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

Coding style?

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

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

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

We should have a devm_ version of this.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv2 6/8] ASoC: fsl: add SGTL5000 based audio machine driver.
  2013-11-01  7:04 ` [PATCHv2 6/8] ASoC: fsl: add SGTL5000 based audio machine driver Xiubo Li
  2013-11-01 10:17   ` Oskar Schirmer
  2013-11-01 10:28   ` Nicolin Chen
@ 2013-11-01 18:40   ` Mark Brown
  2 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2013-11-01 18:40 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: 785 bytes --]

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

> Conflicts:
> 	sound/soc/fsl/Makefile

Ahem.

> +	/* TODO: The SAI driver should figure this out for us */
> +	switch (channels) {
> +	case 2:
> +		snd_soc_dai_set_tdm_slot(cpu_dai, 0xfffffffc, 0xfffffffc, 2, 0);
> +		break;
> +	case 1:
> +		snd_soc_dai_set_tdm_slot(cpu_dai, 0xfffffffe, 0xfffffffe, 1, 0);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}

Yes, it should - this code should probably just be copied straight into
the SAI driver.  If we need to support other configurations we can do
that later.

> +static int fsl_sgtl5000_remove(struct platform_device *pdev)
> +{
> +	snd_soc_unregister_card(&fsl_sgt1500_card);
> +
> +	return 0;
> +}

You're using snd_soc_unregister_card() so you don't need to do this.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv2 5/8] ASoC: SGTL5000: Enhance the SGTL5000 codec driver about regulator.
  2013-11-01  7:04 ` [PATCHv2 5/8] ASoC: SGTL5000: Enhance the SGTL5000 codec driver about regulator Xiubo Li
  2013-11-01 10:02   ` Nicolin Chen
@ 2013-11-01 18:50   ` Mark Brown
  1 sibling, 0 replies; 17+ messages in thread
From: Mark Brown @ 2013-11-01 18:50 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: 760 bytes --]

On Fri, Nov 01, 2013 at 03:04:52PM +0800, Xiubo Li wrote:
> On VF610 series there are no regulators used, and now whether the
> CONFIG_REGULATOR mirco is enabled or not, for the VF610 audio
> patch series, the board cannot be probe successfully.
> And this patch will solve this issue.

I don't understand what this is for at all, you're just saying there is
a problem you're trying to solve but you don't explain anything about
what the problem is or how your changes address it.

> +#ifndef CONFIG_SND_SOC_FSL_SGTL5000_VF610
>  static int ldo_regulator_register(struct snd_soc_codec *codec,

This is definitely broken, it won't work with multi-platform kernels,
and I don't understand what this is supposed to do - what is the reason
for making this change?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2013-11-01 18:51 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-01  7:04 Xiubo Li
2013-11-01  7:04 ` [PATCHv2 1/8] ALSA: Add SAI SoC Digital Audio Interface driver Xiubo Li
2013-11-01  8:59   ` Nicolin Chen
2013-11-01 18:25   ` Mark Brown
2013-11-01  7:04 ` [PATCHv2 2/8] ARM: dts: Add Freescale SAI ALSA SoC Digital Audio Interface node for VF610 Xiubo Li
2013-11-01  7:04 ` [PATCHv2 3/8] ARM: dts: Enables SAI ALSA SoC DAI device for Vybrid VF610 TOWER board Xiubo Li
2013-11-01  7:04 ` [PATCHv2 4/8] Documentation: Add device tree bindings for Freescale SAI Xiubo Li
2013-11-01  7:04 ` [PATCHv2 5/8] ASoC: SGTL5000: Enhance the SGTL5000 codec driver about regulator Xiubo Li
2013-11-01 10:02   ` Nicolin Chen
2013-11-01 18:50   ` Mark Brown
2013-11-01  7:04 ` [PATCHv2 6/8] ASoC: fsl: add SGTL5000 based audio machine driver Xiubo Li
2013-11-01 10:17   ` Oskar Schirmer
2013-11-01 10:28   ` Nicolin Chen
2013-11-01 12:07     ` Shawn Guo
2013-11-01 18:40   ` Mark Brown
2013-11-01  7:04 ` [PATCHv2 7/8] ARM: dts: Enable SGTL5000 codec based audio driver node for VF610 Xiubo Li
2013-11-01  7:04 ` [PATCHv2 8/8] Documentation: Add device tree bindings for Freescale VF610 sound Xiubo Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).