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