[RFC PATCH 0/3] ep93xx i2s audio

[RFC PATCH 0/3] ep93xx i2s audio

[Ryan Mallon]

Hi all,

The following patch series adds support for i2s audio on the ep93xx,
with the Snapper CL15 used as an example implementation. The driver is
basically working, but still incomplete and needs some tidy up before it
will be accepted for mainline. I am posting the patches to get feedback
from others as to how well it works on their boards and ideas to improve
the code.

The following issues still exist:
 - The setting of SDIV/LRDIV is ugly since it requires modifying one of
the clock registers from the audio driver.
 - Audio capture has not been tested
 - Sometimes playback (ie aplay) results in noise. Killing the process
and trying again will result in correct playback.
 - Some empty functions can probably be removed
 - Not sure if I2SONSSP/AC97 should be a function argument, or a Kconfig
option (opinions please).

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934

[RFC PATCH 1/3] ep93xx i2s driver

[Ryan Mallon]

Add ep93xx i2s audio driver

Signed-off-by: Ryan Mallon <ryan@bluewatersys.com>
---

diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index b1749bc..f7cb451 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -28,6 +28,7 @@ source "sound/soc/atmel/Kconfig"
 source "sound/soc/au1x/Kconfig"
 source "sound/soc/blackfin/Kconfig"
 source "sound/soc/davinci/Kconfig"
+source "sound/soc/ep93xx/Kconfig"
 source "sound/soc/fsl/Kconfig"
 source "sound/soc/imx/Kconfig"
 source "sound/soc/omap/Kconfig"
diff --git a/sound/soc/Makefile b/sound/soc/Makefile
index 1470141..55b711a 100644
--- a/sound/soc/Makefile
+++ b/sound/soc/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_SND_SOC)	+= atmel/
 obj-$(CONFIG_SND_SOC)	+= au1x/
 obj-$(CONFIG_SND_SOC)	+= blackfin/
 obj-$(CONFIG_SND_SOC)	+= davinci/
+obj-$(CONFIG_SND_SOC)	+= ep93xx/
 obj-$(CONFIG_SND_SOC)	+= fsl/
 obj-$(CONFIG_SND_SOC)   += imx/
 obj-$(CONFIG_SND_SOC)	+= omap/
diff --git a/sound/soc/ep93xx/Kconfig b/sound/soc/ep93xx/Kconfig
new file mode 100644
index 0000000..6af5dd8
--- /dev/null
+++ b/sound/soc/ep93xx/Kconfig
@@ -0,0 +1,15 @@
+config SND_EP93XX_SOC
+	tristate "SoC Audio support for the Cirrus Logic EP93xx series"
+	depends on ARCH_EP93XX && SND_SOC
+	help
+	  Say Y or M if you want to add support for codecs attached to
+	  the EP93xx I2S interface.
+
+config SND_EP93XX_SOC_I2S
+	tristate
+
+config SND_EP93XX_SOC_SNAPPERCL15
+        tristate "SoC Audio support for Bluewater Systems Snapper CL15 module"
+        depends on SND_EP93XX_SOC && MACH_SNAPPER_CL15
+        select SND_EP93XX_SOC_I2S
+        select SND_SOC_TLV320AIC23
diff --git a/sound/soc/ep93xx/Makefile b/sound/soc/ep93xx/Makefile
new file mode 100644
index 0000000..272e60f
--- /dev/null
+++ b/sound/soc/ep93xx/Makefile
@@ -0,0 +1,11 @@
+# EP93xx Platform Support
+snd-soc-ep93xx-objs				:= ep93xx-pcm.o
+snd-soc-ep93xx-i2s-objs	 			:= ep93xx-i2s.o
+
+obj-$(CONFIG_SND_EP93XX_SOC)			+= snd-soc-ep93xx.o
+obj-$(CONFIG_SND_EP93XX_SOC_I2S)		+= snd-soc-ep93xx-i2s.o
+
+# EP93XX Machine Support
+snd-soc-snappercl15-objs			:= snappercl15.o
+
+obj-$(CONFIG_SND_EP93XX_SOC_SNAPPERCL15)	+= snd-soc-snappercl15.o
diff --git a/sound/soc/ep93xx/ep93xx-i2s.c b/sound/soc/ep93xx/ep93xx-i2s.c
new file mode 100644
index 0000000..a81a18d
--- /dev/null
+++ b/sound/soc/ep93xx/ep93xx-i2s.c
@@ -0,0 +1,497 @@
+/*
+ * linux/sound/soc/ep93xx-i2s.c
+ * EP93xx I2S driver
+ *
+ * Copyright (C) 2008 Ryan Mallon <ryan@bluewatersys.com>
+ * 
+ * Based on the original driver by:
+ *   Copyright (C) 2007 Chase Douglas <chasedouglas@gmail>
+ *   Copyright (C) 2006 Lennert Buytenhek <buytenh@wantstofly.org>
+ * 
+ * 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.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/initval.h>
+#include <sound/soc.h>
+
+#include <mach/hardware.h>
+#include <mach/ep93xx-regs.h>
+#include <mach/dma.h>
+
+#include "ep93xx-pcm.h"
+#include "ep93xx-i2s.h"
+
+#define EP93XX_I2S_TXCLKCFG		0x00
+#define EP93XX_I2S_RXCLKCFG		0x04
+#define EP93XX_I2S_GLSTS		0x08
+#define EP93XX_I2S_GLCTRL		0x0C
+
+#define EP93XX_I2S_TX0LFT		0x10
+#define EP93XX_I2S_TX0RT		0x14
+#define EP93XX_I2S_TX1LFT		0x18
+#define EP93XX_I2S_TX1RT		0x1C
+#define EP93XX_I2S_TX2LFT		0x20
+#define EP93XX_I2S_TX2RT		0x24
+#define EP93XX_I2S_TXLINCTRLDATA	0x28
+#define EP93XX_I2S_TXCTRL		0x2C
+#define EP93XX_I2S_TXWRDLEN		0x30
+#define EP93XX_I2S_TX0EN		0x34
+#define EP93XX_I2S_TX1EN		0x38
+#define EP93XX_I2S_TX2EN		0x3C
+
+#define EP93XX_I2S_RX0LFT		0x40
+#define EP93XX_I2S_RX0RT		0x44
+#define EP93XX_I2S_RX1LFT		0x48
+#define EP93XX_I2S_RX1RT		0x4C
+#define EP93XX_I2S_RX2LFT		0x50
+#define EP93XX_I2S_RX2RT		0x54
+
+#define EP93XX_I2S_RXLINCTRLDATA	0x58
+#define EP93XX_I2S_RXCTRL		0x5C
+#define EP93XX_I2S_RXWRDLEN		0x60
+#define EP93XX_I2S_RX0EN		0x64
+#define EP93XX_I2S_RX1EN		0x68
+#define EP93XX_I2S_RX2EN		0x6C
+
+#define EP93XX_I2S_WRDLEN_16		(0 << 0)
+#define EP93XX_I2S_WRDLEN_24		(1 << 0)
+#define EP93XX_I2S_WRDLEN_32		(2 << 0)
+
+#define EP93XX_I2S_LINCTRLDATA_R_JUST	(1 << 2) /* Right justify */
+
+#define EP93XX_I2S_CLKCFG_LRS		(1 << 0) /* lrclk polarity */
+#define EP93XX_I2S_CLKCFG_CKP		(1 << 1) /* Bit clock polarity */
+#define EP93XX_I2S_CLKCFG_REL		(1 << 2) /*  */
+#define EP93XX_I2S_CLKCFG_MASTER	(1 << 3) /* Master mode */
+#define EP93XX_I2S_CLKCFG_NBCG		(1 << 4) /* Not bit clock gating */
+
+struct ep93xx_i2s_info {
+	struct clk			*i2s_clk;
+	struct ep93xx_pcm_dma_params	*dma_params[2];
+	struct resource			*mem;
+	void __iomem			*regs;
+};
+
+static struct ep93xx_pcm_dma_params ep93xx_i2s_pcm_out = {
+	.name		= "i2s-pcm-out",
+	.dma_port	= EP93XX_DMA_M2P_PORT_I2S1,
+};
+
+static struct ep93xx_pcm_dma_params ep93xx_i2s_pcm_in = {
+	.name		= "i2s-pcm-in",
+	.dma_port	= EP93XX_DMA_M2P_PORT_I2S1,
+};
+
+static inline void ep93xx_i2s_write_reg(struct ep93xx_i2s_info *info,
+					unsigned reg, unsigned val)
+{
+	__raw_writel(val, info->regs + reg);
+}
+
+static inline unsigned ep93xx_i2s_read_reg(struct ep93xx_i2s_info *info,
+					   unsigned reg)
+{
+	return __raw_readl(info->regs + reg);
+}
+
+static inline void ep93xx_i2s_enable(struct ep93xx_i2s_info *info, int enable)
+{
+	ep93xx_i2s_write_reg(info, EP93XX_I2S_GLCTRL, enable);
+}
+
+static inline void ep93xx_i2s_enable_fifos(struct ep93xx_i2s_info *info,
+					   int enable)
+{
+	ep93xx_i2s_write_reg(info, EP93XX_I2S_RX0EN, enable);
+	ep93xx_i2s_write_reg(info, EP93XX_I2S_RX1EN, enable);
+	ep93xx_i2s_write_reg(info, EP93XX_I2S_RX2EN, enable);
+	ep93xx_i2s_write_reg(info, EP93XX_I2S_TX0EN, enable);
+	ep93xx_i2s_write_reg(info, EP93XX_I2S_TX1EN, enable);
+	ep93xx_i2s_write_reg(info, EP93XX_I2S_TX2EN, enable);
+}
+
+static int ep93xx_i2s_startup(struct snd_pcm_substream *substream,
+			      struct snd_soc_dai *dai)
+{	
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
+	struct ep93xx_i2s_info *info = rtd->dai->cpu_dai->private_data;
+	
+	clk_enable(info->i2s_clk);
+	cpu_dai->dma_data = info->dma_params[substream->stream];	
+	return 0;
+}
+
+static void ep93xx_i2s_shutdown(struct snd_pcm_substream *substream,
+				struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct ep93xx_i2s_info *info = rtd->dai->cpu_dai->private_data;
+
+	clk_disable(info->i2s_clk);
+}
+
+static int ep93xx_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
+			      struct snd_soc_dai *dai)
+{
+	return 0;
+}
+
+static int ep93xx_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
+				  unsigned int fmt)
+{
+	struct ep93xx_i2s_info *info = cpu_dai->private_data;
+	unsigned int clk_cfg, lin_ctrl;
+	
+	clk_cfg  = ep93xx_i2s_read_reg(info, EP93XX_I2S_RXCLKCFG);
+	lin_ctrl = ep93xx_i2s_read_reg(info, EP93XX_I2S_RXLINCTRLDATA);
+
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		clk_cfg |= EP93XX_I2S_CLKCFG_REL;
+		lin_ctrl &= ~EP93XX_I2S_LINCTRLDATA_R_JUST;
+		break;
+		
+	case SND_SOC_DAIFMT_LEFT_J:
+		clk_cfg &= ~EP93XX_I2S_CLKCFG_REL;
+		lin_ctrl &= ~EP93XX_I2S_LINCTRLDATA_R_JUST;
+		break;
+		
+	case SND_SOC_DAIFMT_RIGHT_J:
+		clk_cfg &= ~EP93XX_I2S_CLKCFG_REL;
+		lin_ctrl |= EP93XX_I2S_LINCTRLDATA_R_JUST;	
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBS_CFS:
+		/* CPU is master */
+		clk_cfg |= EP93XX_I2S_CLKCFG_MASTER;
+		break;
+
+	case SND_SOC_DAIFMT_CBM_CFM:
+		/* Codec is master */
+		clk_cfg &= ~EP93XX_I2S_CLKCFG_MASTER;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_NB_NF:
+		/* Negative bit clock, lrclk low on left word */
+		clk_cfg &= ~(EP93XX_I2S_CLKCFG_CKP | EP93XX_I2S_CLKCFG_REL);
+		break;
+
+	case SND_SOC_DAIFMT_NB_IF:
+		/* Negative bit clock, lrclk low on right word */
+		clk_cfg &= ~EP93XX_I2S_CLKCFG_CKP;
+		clk_cfg |= EP93XX_I2S_CLKCFG_REL;
+		break;
+		
+	case SND_SOC_DAIFMT_IB_NF:
+		/* Positive bit clock, lrclk low on left word */
+		clk_cfg |= EP93XX_I2S_CLKCFG_CKP;
+		clk_cfg &= ~EP93XX_I2S_CLKCFG_REL;
+		break;
+
+	case SND_SOC_DAIFMT_IB_IF:
+		/* Positive bit clock, lrclk low on right word */
+		clk_cfg |= EP93XX_I2S_CLKCFG_CKP | EP93XX_I2S_CLKCFG_REL;
+		break;
+	}
+
+	/* Write new register values */
+	ep93xx_i2s_write_reg(info, EP93XX_I2S_RXCLKCFG, clk_cfg);
+	ep93xx_i2s_write_reg(info, EP93XX_I2S_TXCLKCFG, clk_cfg);
+	ep93xx_i2s_write_reg(info, EP93XX_I2S_RXLINCTRLDATA, lin_ctrl);
+	ep93xx_i2s_write_reg(info, EP93XX_I2S_TXLINCTRLDATA, lin_ctrl);
+	return 0;
+}
+
+static int ep93xx_i2s_hw_params(struct snd_pcm_substream *substream,
+				struct snd_pcm_hw_params *params,
+				struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
+	struct ep93xx_i2s_info *info = cpu_dai->private_data;
+	unsigned word_len;
+	
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_S16_LE:
+		word_len = EP93XX_I2S_WRDLEN_16;
+		break;
+
+	case SNDRV_PCM_FORMAT_S24_LE:
+		word_len = EP93XX_I2S_WRDLEN_24;
+		break;
+
+	case SNDRV_PCM_FORMAT_S32_LE:
+		word_len = EP93XX_I2S_WRDLEN_32;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		ep93xx_i2s_write_reg(info, EP93XX_I2S_TXWRDLEN, word_len);
+	else
+		ep93xx_i2s_write_reg(info, EP93XX_I2S_TXWRDLEN, word_len);
+	return 0;
+}
+
+static int ep93xx_i2s_set_sysclk(struct snd_soc_dai *cpu_dai, int clk_id,
+				 unsigned int freq, int dir)
+{
+	struct ep93xx_i2s_info *info = cpu_dai->private_data;
+
+	if (dir == SND_SOC_CLOCK_IN || clk_id != 0)
+		return -EINVAL;
+
+	return clk_set_rate(info->i2s_clk, freq);
+}
+
+static int ep93xx_i2s_set_clkdiv(struct snd_soc_dai *cpu_dai, int div_id,
+				 int div)
+{
+	unsigned val;
+
+	/*
+	 * The LRDIV (frame clock) and SDIV (bit clock) controls are in the
+	 * EP93XX_SYSCON_I2SCLKDIV register, which is also used by clock.c
+	 * to set the I2S master clock.
+	 *
+	 * FIXME - This functions is potentially racy with the clock api for
+	 * the I2S master clock. Its also ugly modifying the syscon registers
+	 * here.
+	 */
+	val = __raw_readl( EP93XX_SYSCON_I2SCLKDIV);
+
+	switch (div_id) {
+	case EP93XX_I2S_SDIV:
+		/* SCLK = MCLK / div */
+		switch (div) {
+		case 2: 
+			val &= ~(0x1 << EP93XX_I2S_SDIV);
+			break;
+			
+		case 4:
+			val |= 0x1 << EP93XX_I2S_SDIV;
+			break;
+
+		default:
+			return -EINVAL;
+		}
+		break;
+
+	case EP93XX_I2S_LRDIV:
+		/* LRCLK = SCLK / div */
+		switch (div) {
+		case 32:
+			val &= ~(0x3 << EP93XX_I2S_LRDIV);
+			break;
+
+		case 64:
+			val &= ~(0x3 << EP93XX_I2S_LRDIV);
+			val |= 0x1 << EP93XX_I2S_LRDIV;
+			break;
+
+		case 128:
+			val &= ~(0x3 << EP93XX_I2S_LRDIV);
+			val |= 0x2 << EP93XX_I2S_LRDIV;
+			break;
+
+		default:
+			return -EINVAL;
+		}
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	ep93xx_syscon_swlocked_write(val, EP93XX_SYSCON_I2SCLKDIV);
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int ep93xx_i2s_suspend(struct snd_soc_dai *dai)
+{
+	struct ep93xx_i2s_info *info = dai->private_data;
+
+	if (!dai->active)
+		return;
+	
+	ep93xx_i2s_enable(info, 0);
+}
+
+static int ep93xx_i2s_resume(struct snd_soc_dai *dai)
+{
+	struct ep93xx_i2s_info *info = dai->private_data;
+
+	if (!dai->active)
+		return;
+
+	ep93xx_i2s_enable(info, 1);
+}
+#else
+#define ep93xx_i2s_suspend	NULL
+#define ep93xx_i2s_resume	NULL
+#endif
+
+#define EP93XX_I2S_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
+			    SNDRV_PCM_FMTBIT_S24_LE | \
+			    SNDRV_PCM_FMTBIT_S32_LE)
+
+static struct snd_soc_dai_ops ep93xx_i2s_dai_ops = {
+	.startup	= ep93xx_i2s_startup,
+	.shutdown	= ep93xx_i2s_shutdown,
+	.trigger	= ep93xx_i2s_trigger,
+	.hw_params	= ep93xx_i2s_hw_params,
+	.set_sysclk	= ep93xx_i2s_set_sysclk,
+	.set_clkdiv	= ep93xx_i2s_set_clkdiv,
+	.set_fmt	= ep93xx_i2s_set_dai_fmt,
+};
+
+struct snd_soc_dai ep93xx_i2s_dai = {
+	.name		= "ep93xx-i2s",
+	.id		= 0,
+	.suspend	= ep93xx_i2s_suspend,
+	.resume		= ep93xx_i2s_resume,
+	.playback	= {
+		.channels_min	= 2,
+		.channels_max	= 2,
+		.rates		= SNDRV_PCM_RATE_8000_48000,
+		.formats	= EP93XX_I2S_FORMATS,
+	},
+	.capture	= {
+		 .channels_min	= 2,
+		 .channels_max	= 2,
+		 .rates		= SNDRV_PCM_RATE_8000_48000,
+		 .formats	= EP93XX_I2S_FORMATS,
+	},
+	.ops		= &ep93xx_i2s_dai_ops,
+};
+EXPORT_SYMBOL_GPL(ep93xx_i2s_dai);
+
+static int ep93xx_i2s_probe(struct platform_device *pdev)
+{
+	struct ep93xx_i2s_info *info;
+	struct resource *res;
+	int err;
+
+	info = kzalloc(sizeof(struct ep93xx_i2s_info), GFP_KERNEL);
+	if (!info) {
+		err = -ENOMEM;
+		goto fail;
+	}
+
+	ep93xx_i2s_dai.dev = &pdev->dev;
+	ep93xx_i2s_dai.private_data = info;
+	
+	info->dma_params[SNDRV_PCM_STREAM_PLAYBACK] = &ep93xx_i2s_pcm_out;
+	info->dma_params[SNDRV_PCM_STREAM_CAPTURE]  = &ep93xx_i2s_pcm_in;
+	
+	err = snd_soc_register_dai(&ep93xx_i2s_dai);
+	if (err)
+		goto fail_free_info;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		err = -ENODEV;
+		goto fail_unregister_dai;
+	}
+		
+	info->mem = request_mem_region(res->start, resource_size(res), 
+				       pdev->name);
+	if (!info->mem) {		
+		err = -EBUSY;
+		goto fail_free_info;
+	}
+	
+	info->regs = ioremap(info->mem->start, resource_size(info->mem));
+	if (!info->regs) {
+		err = -ENXIO;
+		goto fail_release_mem;
+	}
+
+	info->i2s_clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(info->i2s_clk)) {
+		err = PTR_ERR(info->i2s_clk);
+		goto fail_unmap_mem;
+	}
+	
+	ep93xx_i2s_enable_fifos(info, 1);
+	ep93xx_i2s_enable(info, 1);
+	return 0;
+
+fail_unmap_mem:	
+	iounmap(info->regs);
+fail_release_mem:
+	release_mem_region(info->mem->start, resource_size(info->mem));
+fail_unregister_dai:
+	snd_soc_unregister_dai(&ep93xx_i2s_dai);
+fail_free_info:
+	kfree(info);
+fail:
+	return err;
+}
+
+static int __devexit ep93xx_i2s_remove(struct platform_device *pdev)
+{
+	struct ep93xx_i2s_info *info = ep93xx_i2s_dai.private_data;
+	
+	ep93xx_i2s_enable_fifos(info, 0);
+	ep93xx_i2s_enable(info, 0);
+
+	clk_put(info->i2s_clk);
+	iounmap(info->regs);
+	release_mem_region(info->mem->start, resource_size(info->mem));
+	snd_soc_unregister_dai(&ep93xx_i2s_dai);
+	kfree(info);
+	return 0;
+}
+
+static struct platform_driver ep93xx_i2s_driver = {
+	.probe	= ep93xx_i2s_probe,
+	.remove	= __devexit_p(ep93xx_i2s_remove),
+	.driver	= {
+		.name	= "ep93xx-i2s",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init ep93xx_i2s_init(void)
+{
+	return platform_driver_register(&ep93xx_i2s_driver);
+}
+
+static void __exit ep93xx_i2s_exit(void)
+{
+	platform_driver_unregister(&ep93xx_i2s_driver);
+}
+
+module_init(ep93xx_i2s_init);
+module_exit(ep93xx_i2s_exit);
+
+MODULE_ALIAS("platform:ep93xx-i2s");
+MODULE_AUTHOR("Ryan Mallon <ryan@bluewatersys.com>");
+MODULE_DESCRIPTION("EP93XX I2S driver");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/ep93xx/ep93xx-i2s.h b/sound/soc/ep93xx/ep93xx-i2s.h
new file mode 100644
index 0000000..37e6370
--- /dev/null
+++ b/sound/soc/ep93xx/ep93xx-i2s.h
@@ -0,0 +1,13 @@
+#ifndef _EP93XX_SND_SOC_I2S_H
+#define _EP93XX_SND_SOC_I2S_H
+
+/*
+ * These are the offsets for the LRDIV and SDIV fields in the syscon i2sclkdiv
+ * register.
+ */
+#define EP93XX_I2S_LRDIV	17
+#define EP93XX_I2S_SDIV		16
+
+extern struct snd_soc_dai ep93xx_i2s_dai;
+
+#endif /* _EP93XX_SND_SOC_I2S_H */
diff --git a/sound/soc/ep93xx/ep93xx-pcm.c b/sound/soc/ep93xx/ep93xx-pcm.c
new file mode 100644
index 0000000..c071b5c
--- /dev/null
+++ b/sound/soc/ep93xx/ep93xx-pcm.c
@@ -0,0 +1,321 @@
+/*
+ * linux/sound/arm/ep93xx-pcm.c - EP93xx ALSA PCM interface
+ *
+ * Copyright (C) 2006 Lennert Buytenhek <buytenh@wantstofly.org>
+ * Copyright (C) 2006 Applied Data Systems
+ *
+ * Rewritten for the SoC audio subsystem (Based on PXA2xx code):
+ *   Copyright (c) 2008 Ryan Mallon <ryan@bluewatersys.com>
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/dma-mapping.h>
+
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+#include <mach/dma.h>
+#include <mach/hardware.h>
+#include <mach/ep93xx-regs.h>
+
+#include "ep93xx-pcm.h"
+
+static const struct snd_pcm_hardware ep93xx_pcm_hardware = {
+	.info			= (SNDRV_PCM_INFO_MMAP		|
+				   SNDRV_PCM_INFO_MMAP_VALID	|
+				   SNDRV_PCM_INFO_INTERLEAVED	|
+				   SNDRV_PCM_INFO_BLOCK_TRANSFER),
+				   
+	.rates			= SNDRV_PCM_RATE_8000_48000,
+	.rate_min		= SNDRV_PCM_RATE_8000,
+	.rate_max		= SNDRV_PCM_RATE_48000,
+	
+	.formats		= (SNDRV_PCM_FMTBIT_S16_LE |
+				   SNDRV_PCM_FMTBIT_S24_LE |
+				   SNDRV_PCM_FMTBIT_S32_LE),
+	
+	.buffer_bytes_max	= 131072,
+	.period_bytes_min	= 32,
+	.period_bytes_max	= 32768,
+	.periods_min		= 1,
+	.periods_max		= 32,
+	.fifo_size		= 32,
+};
+
+struct ep93xx_runtime_data
+{
+	struct ep93xx_dma_m2p_client	cl;
+	struct ep93xx_pcm_dma_params	*params;
+	int				pointer_bytes;
+	struct tasklet_struct		period_tasklet;
+	int				periods;
+	struct ep93xx_dma_buffer	buf[32];
+};
+
+static void ep93xx_pcm_period_elapsed(unsigned long data)
+{
+	struct snd_pcm_substream *substream = (struct snd_pcm_substream *)data;
+	snd_pcm_period_elapsed(substream);
+}
+
+static void ep93xx_pcm_buffer_started(void *cookie,
+				      struct ep93xx_dma_buffer *buf)
+{
+}
+
+static void ep93xx_pcm_buffer_finished(void *cookie, 
+				       struct ep93xx_dma_buffer *buf, 
+				       int bytes, int error)
+{
+	struct snd_pcm_substream *substream = cookie;
+	struct ep93xx_runtime_data *rtd = substream->runtime->private_data;
+
+	if (buf == rtd->buf + rtd->periods - 1)
+		rtd->pointer_bytes = 0;
+	else
+		rtd->pointer_bytes += buf->size;
+
+	if (!error) {
+		ep93xx_dma_m2p_submit_recursive(&rtd->cl, buf);
+		tasklet_schedule(&rtd->period_tasklet);
+	} else {
+		snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
+	}
+}
+
+static int ep93xx_pcm_open(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *soc_rtd = substream->private_data;
+	struct ep93xx_pcm_dma_params *dma_params = soc_rtd->dai->cpu_dai->dma_data;
+	struct ep93xx_runtime_data *rtd;    
+	int ret;
+
+	snd_soc_set_runtime_hwparams(substream, &ep93xx_pcm_hardware);
+
+	rtd = kmalloc(sizeof(*rtd), GFP_KERNEL);
+	if (!rtd) 
+		return -ENOMEM;
+
+	memset(&rtd->period_tasklet, 0, sizeof(rtd->period_tasklet));
+	rtd->period_tasklet.func = ep93xx_pcm_period_elapsed;
+	rtd->period_tasklet.data = (unsigned long)substream;
+
+	rtd->cl.name = dma_params->name;
+	rtd->cl.flags = dma_params->dma_port | EP93XX_DMA_M2P_IGNORE_ERROR |
+		((substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
+		 EP93XX_DMA_M2P_TX : EP93XX_DMA_M2P_RX);
+	rtd->cl.cookie = substream;
+	rtd->cl.buffer_started = ep93xx_pcm_buffer_started;
+	rtd->cl.buffer_finished = ep93xx_pcm_buffer_finished;
+	ret = ep93xx_dma_m2p_client_register(&rtd->cl);
+	if (ret < 0) {
+		kfree(rtd);
+		return ret;
+	}
+	
+	substream->runtime->private_data = rtd;
+	return 0;
+}
+
+static int ep93xx_pcm_close(struct snd_pcm_substream *substream)
+{
+	struct ep93xx_runtime_data *rtd = substream->runtime->private_data;
+
+	ep93xx_dma_m2p_client_unregister(&rtd->cl);
+	kfree(rtd);
+	return 0;
+}
+
+static int ep93xx_pcm_hw_params(struct snd_pcm_substream *substream,
+				struct snd_pcm_hw_params *params)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct ep93xx_runtime_data *rtd = runtime->private_data;
+	size_t totsize = params_buffer_bytes(params);
+	size_t period = params_period_bytes(params);
+	int i;
+
+	snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
+	runtime->dma_bytes = totsize;
+
+	rtd->periods = (totsize + period - 1) / period;
+	for (i = 0; i < rtd->periods; i++) {
+		rtd->buf[i].bus_addr = runtime->dma_addr + (i * period);
+		rtd->buf[i].size = period;
+		if ((i + 1) * period > totsize)
+			rtd->buf[i].size = totsize - (i * period);
+	}
+
+	return 0;
+}
+
+static int ep93xx_pcm_hw_free(struct snd_pcm_substream *substream)
+{
+	snd_pcm_set_runtime_buffer(substream, NULL);
+	return 0;
+}
+
+static int ep93xx_pcm_prepare(struct snd_pcm_substream *substream)
+{
+	return 0;
+}
+
+static int ep93xx_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct ep93xx_runtime_data *rtd = substream->runtime->private_data;
+	int ret;
+	int i;
+
+	ret = 0;
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		rtd->pointer_bytes = 0;
+		for (i = 0; i < rtd->periods; i++)
+			ep93xx_dma_m2p_submit(&rtd->cl, rtd->buf + i);
+		break;
+
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		ep93xx_dma_m2p_flush(&rtd->cl);
+		break;
+
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static snd_pcm_uframes_t ep93xx_pcm_pointer(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct ep93xx_runtime_data *rtd = substream->runtime->private_data;
+
+	/* FIXME: implement this with sub-period granularity */
+	return bytes_to_frames(runtime, rtd->pointer_bytes);
+}
+
+static int ep93xx_pcm_mmap(struct snd_pcm_substream *substream,
+			   struct vm_area_struct *vma)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+
+	return dma_mmap_writecombine(substream->pcm->card->dev, vma,
+				     runtime->dma_area,
+				     runtime->dma_addr,
+				     runtime->dma_bytes);
+}
+
+static struct snd_pcm_ops ep93xx_pcm_ops = {
+	.open		= ep93xx_pcm_open,
+	.close		= ep93xx_pcm_close,
+	.ioctl		= snd_pcm_lib_ioctl,
+	.hw_params	= ep93xx_pcm_hw_params,
+	.hw_free	= ep93xx_pcm_hw_free,
+	.prepare	= ep93xx_pcm_prepare,
+	.trigger	= ep93xx_pcm_trigger,
+	.pointer	= ep93xx_pcm_pointer,
+	.mmap		= ep93xx_pcm_mmap,
+};
+
+static int ep93xx_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
+{
+	struct snd_pcm_substream *substream = pcm->streams[stream].substream;
+	struct snd_dma_buffer *buf = &substream->dma_buffer;
+	size_t size = ep93xx_pcm_hardware.buffer_bytes_max;
+
+	buf->dev.type = SNDRV_DMA_TYPE_DEV;
+	buf->dev.dev = pcm->card->dev;
+	buf->private_data = NULL;
+	buf->area = dma_alloc_writecombine(pcm->card->dev, size,
+					   &buf->addr, GFP_KERNEL);
+	buf->bytes = size;
+
+	return (buf->area == NULL) ? -ENOMEM : 0;
+}
+
+static void ep93xx_pcm_free_dma_buffers(struct snd_pcm *pcm)
+{
+	struct snd_pcm_substream *substream;
+	struct snd_dma_buffer *buf;
+	int stream;
+
+	for (stream = 0; stream < 2; stream++) {		
+		substream = pcm->streams[stream].substream;
+		if (!substream)
+			continue;
+		
+		buf = &substream->dma_buffer;
+		if (!buf->area)
+			continue;
+
+		dma_free_writecombine(pcm->card->dev, buf->bytes, buf->area,
+				      buf->addr);
+		buf->area = NULL;
+	}
+}
+
+static u64 ep93xx_pcm_dmamask = 0xffffffff;
+
+static int ep93xx_pcm_new(struct snd_card *card, struct snd_soc_dai *dai,
+			  struct snd_pcm *pcm)
+{
+	int ret = 0;
+
+	if (!card->dev->dma_mask)
+		card->dev->dma_mask = &ep93xx_pcm_dmamask;
+	if (!card->dev->coherent_dma_mask)
+		card->dev->coherent_dma_mask = 0xffffffff;
+
+	if (dai->playback.channels_min) {
+		ret = ep93xx_pcm_preallocate_dma_buffer(pcm,
+					SNDRV_PCM_STREAM_PLAYBACK);
+		if (ret)
+			return ret;
+
+		ret = ep93xx_pcm_preallocate_dma_buffer(pcm,
+					SNDRV_PCM_STREAM_CAPTURE);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+struct snd_soc_platform ep93xx_soc_platform = {
+	.name		= "ep93xx-audio",
+	.pcm_ops	= &ep93xx_pcm_ops,
+	.pcm_new	= &ep93xx_pcm_new,
+	.pcm_free	= &ep93xx_pcm_free_dma_buffers,
+};
+EXPORT_SYMBOL_GPL(ep93xx_soc_platform);
+
+static int __init ep93xx_soc_platform_init(void)
+{
+	return snd_soc_register_platform(&ep93xx_soc_platform);
+}
+
+static void __exit ep93xx_soc_platform_exit(void)
+{
+	snd_soc_unregister_platform(&ep93xx_soc_platform);
+}
+
+module_init(ep93xx_soc_platform_init);
+module_exit(ep93xx_soc_platform_exit);
+
+MODULE_AUTHOR("Lennert Buytenhek <buytenh@wantstofly.org>");
+MODULE_DESCRIPTION("EP93xx ALSA PCM interface");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/ep93xx/ep93xx-pcm.h b/sound/soc/ep93xx/ep93xx-pcm.h
new file mode 100644
index 0000000..4ffdd3f
--- /dev/null
+++ b/sound/soc/ep93xx/ep93xx-pcm.h
@@ -0,0 +1,22 @@
+/*
+ * sound/soc/ep93xx/ep93xx-pcm.h - EP93xx ALSA PCM interface
+ *
+ * Copyright (C) 2006 Lennert Buytenhek <buytenh@wantstofly.org>
+ * Copyright (C) 2006 Applied Data Systems
+ *
+ * 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 _EP93XX_SND_SOC_PCM_H
+#define _EP93XX_SND_SOC_PCM_H
+
+struct ep93xx_pcm_dma_params {
+	char	*name;
+	int	dma_port;
+};
+
+extern struct snd_soc_platform ep93xx_soc_platform;
+
+#endif /* _EP93XX_SND_SOC_PCM_H */

[RFC PATCH 2/3] ep93xx i2s core support

[Ryan Mallon]

Add ep93xx core support for i2s audio

Signed-off-by: Ryan Mallon <ryan@bluewatersys.com>
---

diff --git a/arch/arm/mach-ep93xx/clock.c b/arch/arm/mach-ep93xx/clock.c
index 5f80092..df88233 100644
--- a/arch/arm/mach-ep93xx/clock.c
+++ b/arch/arm/mach-ep93xx/clock.c
@@ -108,6 +108,16 @@ static struct clk clk_video = {
 	.set_rate	= set_div_rate,
 };
 
+static struct clk clk_i2s = {
+	.sw_locked	= 1,
+	.enable_reg	= EP93XX_SYSCON_I2SCLKDIV,
+	.enable_mask	= (EP93XX_SYSCON_CLKDIV_ENABLE  |
+			   EP93XX_SYSCON_I2SCLKDIV_SENA |
+			   EP93XX_SYSCON_I2SCLKDIV_SPOL |
+			   EP93XX_SYSCON_I2SCLKDIV_ORIDE),
+	.set_rate	= set_div_rate,
+};
+
 /* DMA Clocks */
 static struct clk clk_m2p0 = {
 	.parent		= &clk_h,
@@ -186,6 +196,7 @@ static struct clk_lookup clocks[] = {
 	INIT_CK("ep93xx-ohci",		NULL,		&clk_usb_host),
 	INIT_CK("ep93xx-keypad",	NULL,		&clk_keypad),
 	INIT_CK("ep93xx-fb",		NULL,		&clk_video),
+	INIT_CK("ep93xx-i2s",		NULL,		&clk_i2s),
 	INIT_CK(NULL,			"pwm_clk",	&clk_pwm),
 	INIT_CK(NULL,			"m2p0",		&clk_m2p0),
 	INIT_CK(NULL,			"m2p1",		&clk_m2p1),
diff --git a/arch/arm/mach-ep93xx/core.c b/arch/arm/mach-ep93xx/core.c
index 90fb591..cceeac2 100644
--- a/arch/arm/mach-ep93xx/core.c
+++ b/arch/arm/mach-ep93xx/core.c
@@ -617,6 +617,37 @@ void ep93xx_keypad_release_gpio(struct platform_device *pdev)
 }
 EXPORT_SYMBOL(ep93xx_keypad_release_gpio);
 
+/*************************************************************************
+ * EP93xx I2S audio peripheral handling
+ *************************************************************************/
+static struct resource ep93xx_i2s_resource[] = {
+	{
+		.start	= EP93XX_I2S_PHYS_BASE,
+		.end	= EP93XX_I2S_PHYS_BASE + 0x100 - 1,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+static struct platform_device ep93xx_i2s_device = {
+	.name		= "ep93xx-i2s",
+	.id		= -1,
+	.num_resources	= ARRAY_SIZE(ep93xx_i2s_resource),
+	.resource	= ep93xx_i2s_resource,
+};
+
+void __init ep93xx_register_i2s(unsigned pins)
+{
+	if (pins != EP93XX_SYSCON_DEVCFG_I2SONSSP &&
+	    pins != EP93XX_SYSCON_DEVCFG_I2SONAC97) {
+		pr_err("Invalid I2S pin configuration - not registering\n");
+		return;
+	}
+
+	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_I2SONAC97 |
+				 EP93XX_SYSCON_DEVCFG_I2SONAC97);
+	ep93xx_devcfg_set_bits(pins);
+	platform_device_register(&ep93xx_i2s_device);
+}
 
 extern void ep93xx_gpio_init(void);
 
diff --git a/arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h b/arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h
index 93e2ecc..074d99b 100644
--- a/arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h
+++ b/arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h
@@ -93,6 +93,7 @@
 /* APB peripherals */
 #define EP93XX_TIMER_BASE		EP93XX_APB_IOMEM(0x00010000)
 
+#define EP93XX_I2S_PHYS_BASE		EP93XX_APB_PHYS(0x00020000)
 #define EP93XX_I2S_BASE			EP93XX_APB_IOMEM(0x00020000)
 
 #define EP93XX_SECURITY_BASE		EP93XX_APB_IOMEM(0x00030000)
@@ -193,6 +194,10 @@
 #define EP93XX_SYSCON_CLKDIV_ESEL	(1<<14)
 #define EP93XX_SYSCON_CLKDIV_PSEL	(1<<13)
 #define EP93XX_SYSCON_CLKDIV_PDIV_SHIFT	8
+#define EP93XX_SYSCON_I2SCLKDIV		EP93XX_SYSCON_REG(0x8c)
+#define EP93XX_SYSCON_I2SCLKDIV_SENA	(1<<31)
+#define EP93XX_SYSCON_I2SCLKDIV_ORIDE   (1<<29)
+#define EP93XX_SYSCON_I2SCLKDIV_SPOL	(1<<19)
 #define EP93XX_SYSCON_KEYTCHCLKDIV	EP93XX_SYSCON_REG(0x90)
 #define EP93XX_SYSCON_KEYTCHCLKDIV_TSEN	(1<<31)
 #define EP93XX_SYSCON_KEYTCHCLKDIV_ADIV	(1<<16)
diff --git a/arch/arm/mach-ep93xx/include/mach/platform.h b/arch/arm/mach-ep93xx/include/mach/platform.h
index c6dc14d..9252adf 100644
--- a/arch/arm/mach-ep93xx/include/mach/platform.h
+++ b/arch/arm/mach-ep93xx/include/mach/platform.h
@@ -43,6 +43,7 @@ void ep93xx_pwm_release_gpio(struct platform_device *pdev);
 void ep93xx_register_keypad(struct ep93xx_keypad_platform_data *data);
 int ep93xx_keypad_acquire_gpio(struct platform_device *pdev);
 void ep93xx_keypad_release_gpio(struct platform_device *pdev);
+void ep93xx_register_i2s(unsigned pins);
 
 void ep93xx_init_devices(void);
 extern struct sys_timer ep93xx_timer;

[RFC PATCH 3/3] ep93xx i2s snapper cl15 support

[Ryan Mallon]

Added support for i2s audio on Snapper CL15

Signed-off-by: Ryan Mallon <ryan@bluewatersys.com>
---

diff --git a/arch/arm/mach-ep93xx/snappercl15.c b/arch/arm/mach-ep93xx/snappercl15.c
index 51134b0..d4ca5cc 100644
--- a/arch/arm/mach-ep93xx/snappercl15.c
+++ b/arch/arm/mach-ep93xx/snappercl15.c
@@ -157,6 +157,7 @@ static void __init snappercl15_init_machine(void)
 	ep93xx_register_i2c(&snappercl15_i2c_gpio_data, snappercl15_i2c_data,
 			    ARRAY_SIZE(snappercl15_i2c_data));
 	ep93xx_register_fb(&snappercl15_fb_info);
+	ep93xx_register_i2s(EP93XX_SYSCON_DEVCFG_I2SONAC97);
 	platform_device_register(&snappercl15_nand_device);
 }
 
diff --git a/sound/soc/ep93xx/snappercl15.c b/sound/soc/ep93xx/snappercl15.c
new file mode 100644
index 0000000..3fae86e
--- /dev/null
+++ b/sound/soc/ep93xx/snappercl15.c
@@ -0,0 +1,186 @@
+/*
+ * snappercl15.c -- SoC audio for Bluewater Systems Snapper CL15 module
+ *
+ * Copyright (C) 2008 Bluewater Systems Ltd
+ * Author: Ryan Mallon <ryan@bluewatersys.com>
+ *
+ *  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/platform_device.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <sound/soc-dapm.h>
+
+#include <asm/mach-types.h>
+#include <mach/hardware.h>
+
+#include "../codecs/tlv320aic23.h"
+#include "ep93xx-pcm.h"
+#include "ep93xx-i2s.h"
+
+//#define CODEC_CLOCK 11289600
+#define CODEC_CLOCK 5644800
+
+static int snappercl15_startup(struct snd_pcm_substream *substream)
+{
+	return 0;
+}
+
+static void snappercl15_shutdown(struct snd_pcm_substream *substream)
+{
+}
+
+static int snappercl15_hw_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 *codec_dai = rtd->dai->codec_dai;
+	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
+	int sdiv, lrdiv, err;
+
+	switch (params_rate(params)) {
+	case 44100: 
+		sdiv = 4;
+		lrdiv = 64;
+		break;
+		
+	case 22050: 
+		sdiv = 4; 
+		lrdiv = 128; 
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	err = snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_I2S |
+				  SND_SOC_DAIFMT_NB_IF |
+				  SND_SOC_DAIFMT_CBS_CFS);
+
+	err = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_I2S | 
+				  SND_SOC_DAIFMT_NB_IF |		  
+				  SND_SOC_DAIFMT_CBS_CFS);
+	if (err)
+		return err;
+
+	err = snd_soc_dai_set_sysclk(codec_dai, 0, CODEC_CLOCK, 
+				     SND_SOC_CLOCK_IN);
+	if (err)
+		return err;
+
+	err = snd_soc_dai_set_sysclk(cpu_dai, 0, CODEC_CLOCK, 
+				     SND_SOC_CLOCK_OUT);
+	if (err)
+		return err;
+
+	err = snd_soc_dai_set_clkdiv(cpu_dai, EP93XX_I2S_SDIV, sdiv);
+	if (err)
+		return err;
+
+	err = snd_soc_dai_set_clkdiv(cpu_dai, EP93XX_I2S_LRDIV, lrdiv);
+	if (err)
+		return err;
+	
+	return 0;
+}
+
+static struct snd_soc_ops snappercl15_ops = {
+	.startup	= snappercl15_startup,
+	.shutdown	= snappercl15_shutdown,
+	.hw_params	= snappercl15_hw_params,
+};
+
+static const struct snd_soc_dapm_widget tlv320aic23_dapm_widgets[] = {
+	SND_SOC_DAPM_HP("Headphone Jack", NULL),
+	SND_SOC_DAPM_LINE("Line In", NULL),
+	SND_SOC_DAPM_MIC("Mic Jack", NULL),
+};
+
+static const struct snd_soc_dapm_route audio_map[] = {
+	{"Headphone Jack", NULL, "LHPOUT"},
+	{"Headphone Jack", NULL, "RHPOUT"},
+
+	{"LLINEIN", NULL, "Line In"},
+	{"RLINEIN", NULL, "Line In"},
+
+	{"MICIN", NULL, "Mic Jack"},
+};
+
+static int snappercl15_tlv320aic23_init(struct snd_soc_codec *codec)
+{
+	printk(KERN_INFO "%s - here\n", __FUNCTION__);
+
+	snd_soc_dapm_new_controls(codec, tlv320aic23_dapm_widgets,
+				  ARRAY_SIZE(tlv320aic23_dapm_widgets));
+
+	snd_soc_dapm_add_routes(codec, audio_map, ARRAY_SIZE(audio_map));
+	
+	snd_soc_dapm_enable_pin(codec, "Headphone Jack");
+	snd_soc_dapm_enable_pin(codec, "Line In");
+	snd_soc_dapm_enable_pin(codec, "Mic Jack");
+
+	snd_soc_dapm_sync(codec);
+	return 0;
+}
+
+static struct snd_soc_dai_link snappercl15_dai = {
+	.name		= "tlv320aic23",
+	.stream_name	= "AIC23",
+	.cpu_dai	= &ep93xx_i2s_dai,
+	.codec_dai	= &tlv320aic23_dai,
+	.init		= snappercl15_tlv320aic23_init,
+	.ops		= &snappercl15_ops,
+};
+
+static struct snd_soc_card snd_soc_snappercl15 = {
+	.name		= "Snapper CL15",
+	.platform	= &ep93xx_soc_platform,
+	.dai_link	= &snappercl15_dai,
+	.num_links	= 1,
+};
+
+static struct snd_soc_device snappercl15_snd_devdata = {
+	.card		= &snd_soc_snappercl15,
+	.codec_dev	= &soc_codec_dev_tlv320aic23,
+};
+
+static struct platform_device *snappercl15_snd_device;
+
+static int __init snappercl15_init(void)
+{
+	int ret;
+
+	if (!machine_is_snapper_cl15())
+		return -ENODEV;
+
+	snappercl15_snd_device = platform_device_alloc("soc-audio", -1);
+	if (!snappercl15_snd_device)
+		return -ENOMEM;
+	
+	platform_set_drvdata(snappercl15_snd_device, &snappercl15_snd_devdata);
+	snappercl15_snd_devdata.dev = &snappercl15_snd_device->dev;
+	ret = platform_device_add(snappercl15_snd_device);
+	if (ret)
+		platform_device_put(snappercl15_snd_device);
+
+	return ret;
+}
+
+static void __exit snappercl15_exit(void)
+{
+	platform_device_unregister(snappercl15_snd_device);
+}
+
+module_init(snappercl15_init);
+module_exit(snappercl15_exit);
+
+MODULE_AUTHOR("Ryan Mallon <ryan@bluewatersys.com>");
+MODULE_DESCRIPTION("ALSA SoC Snapper CL15");
+MODULE_LICENSE("GPL");
+

[RFC PATCH 0/3] ep93xx i2s audio

[Chase Douglas]

On Tue, May 18, 2010 at 12:45 AM, Ryan Mallon <ryan@bluewatersys.com> wrote:
> Hi all,
>
> The following patch series adds support for i2s audio on the ep93xx,
> with the Snapper CL15 used as an example implementation. The driver is
> basically working, but still incomplete and needs some tidy up before it
> will be accepted for mainline. I am posting the patches to get feedback
> from others as to how well it works on their boards and ideas to improve
> the code.
>
> The following issues still exist:
> ?- The setting of SDIV/LRDIV is ugly since it requires modifying one of
> the clock registers from the audio driver.

I'm not too sure what to do here myself, this isn't my normal domain.

> ?- Audio capture has not been tested

Can you test it? I would think it should be somewhat simple to test,
and it's better to have working code in the kernel.

> ?- Sometimes playback (ie aplay) results in noise. Killing the process
> and trying again will result in correct playback.

Hrm. I don't remember having this issue way back.

> ?- Some empty functions can probably be removed

I tried to find stuff you can remove, but I only found one.

> ?- Not sure if I2SONSSP/AC97 should be a function argument, or a Kconfig
> option (opinions please).

The function argument is better. Think of the case where you have one
kernel that runs on two separate machines. This option needs to be
settable at runtime.

Awesome work! I'm glad someone else has been able to take Lennert and
my code and make it useful again. I've got some notes interspersed
with your patches, please review them.

Thanks!

-- Chase

[RFC PATCH 1/3] ep93xx i2s driver

[Chase Douglas]

On Tue, May 18, 2010 at 12:53 AM, Ryan Mallon <ryan@bluewatersys.com> wrote:
> Add ep93xx i2s audio driver
>
> Signed-off-by: Ryan Mallon <ryan@bluewatersys.com>
> ---
>
> diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
> index b1749bc..f7cb451 100644
> --- a/sound/soc/Kconfig
> +++ b/sound/soc/Kconfig
> @@ -28,6 +28,7 @@ source "sound/soc/atmel/Kconfig"
> ?source "sound/soc/au1x/Kconfig"
> ?source "sound/soc/blackfin/Kconfig"
> ?source "sound/soc/davinci/Kconfig"
> +source "sound/soc/ep93xx/Kconfig"
> ?source "sound/soc/fsl/Kconfig"
> ?source "sound/soc/imx/Kconfig"
> ?source "sound/soc/omap/Kconfig"
> diff --git a/sound/soc/Makefile b/sound/soc/Makefile
> index 1470141..55b711a 100644
> --- a/sound/soc/Makefile
> +++ b/sound/soc/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_SND_SOC) ? += atmel/
> ?obj-$(CONFIG_SND_SOC) ?+= au1x/
> ?obj-$(CONFIG_SND_SOC) ?+= blackfin/
> ?obj-$(CONFIG_SND_SOC) ?+= davinci/
> +obj-$(CONFIG_SND_SOC) ?+= ep93xx/
> ?obj-$(CONFIG_SND_SOC) ?+= fsl/
> ?obj-$(CONFIG_SND_SOC) ? += imx/
> ?obj-$(CONFIG_SND_SOC) ?+= omap/
> diff --git a/sound/soc/ep93xx/Kconfig b/sound/soc/ep93xx/Kconfig
> new file mode 100644
> index 0000000..6af5dd8
> --- /dev/null
> +++ b/sound/soc/ep93xx/Kconfig
> @@ -0,0 +1,15 @@
> +config SND_EP93XX_SOC
> + ? ? ? tristate "SoC Audio support for the Cirrus Logic EP93xx series"
> + ? ? ? depends on ARCH_EP93XX && SND_SOC
> + ? ? ? help
> + ? ? ? ? Say Y or M if you want to add support for codecs attached to
> + ? ? ? ? the EP93xx I2S interface.
> +
> +config SND_EP93XX_SOC_I2S
> + ? ? ? tristate
> +
> +config SND_EP93XX_SOC_SNAPPERCL15
> + ? ? ? ?tristate "SoC Audio support for Bluewater Systems Snapper CL15 module"
> + ? ? ? ?depends on SND_EP93XX_SOC && MACH_SNAPPER_CL15
> + ? ? ? ?select SND_EP93XX_SOC_I2S
> + ? ? ? ?select SND_SOC_TLV320AIC23

This last config should be in patch 3/3 instead of this patch.

> diff --git a/sound/soc/ep93xx/Makefile b/sound/soc/ep93xx/Makefile
> new file mode 100644
> index 0000000..272e60f
> --- /dev/null
> +++ b/sound/soc/ep93xx/Makefile
> @@ -0,0 +1,11 @@
> +# EP93xx Platform Support
> +snd-soc-ep93xx-objs ? ? ? ? ? ? ? ? ? ? ? ? ? ?:= ep93xx-pcm.o
> +snd-soc-ep93xx-i2s-objs ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?:= ep93xx-i2s.o
> +
> +obj-$(CONFIG_SND_EP93XX_SOC) ? ? ? ? ? ? ? ? ? += snd-soc-ep93xx.o
> +obj-$(CONFIG_SND_EP93XX_SOC_I2S) ? ? ? ? ? ? ? += snd-soc-ep93xx-i2s.o

Any reason not to just do:

obj-$(CONFIG_SND_EP93XX_SOC) ? ? ? ? ? ? ? ? ? += ep93xx-pcm.o

instead of indirection through a separate .o file? I'm no Makefile
master, so there may be a real reason I'm unaware of.

> +
> +# EP93XX Machine Support
> +snd-soc-snappercl15-objs ? ? ? ? ? ? ? ? ? ? ? := snappercl15.o
> +
> +obj-$(CONFIG_SND_EP93XX_SOC_SNAPPERCL15) ? ? ? += snd-soc-snappercl15.o

Same as above, should be in patch 3/3.

> diff --git a/sound/soc/ep93xx/ep93xx-i2s.c b/sound/soc/ep93xx/ep93xx-i2s.c
> new file mode 100644
> index 0000000..a81a18d
> --- /dev/null
> +++ b/sound/soc/ep93xx/ep93xx-i2s.c
> @@ -0,0 +1,497 @@
> +/*
> + * linux/sound/soc/ep93xx-i2s.c
> + * EP93xx I2S driver
> + *
> + * Copyright (C) 2008 Ryan Mallon <ryan@bluewatersys.com>

You should update your copyright to include 2010.

> + *
> + * Based on the original driver by:
> + * ? Copyright (C) 2007 Chase Douglas <chasedouglas@gmail>

Please use my full email "chasedouglas at gmail.com".

> + * ? Copyright (C) 2006 Lennert Buytenhek <buytenh@wantstofly.org>
> + *
> + * 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.
> + *
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +
> +#include <sound/core.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/initval.h>
> +#include <sound/soc.h>
> +
> +#include <mach/hardware.h>
> +#include <mach/ep93xx-regs.h>
> +#include <mach/dma.h>
> +
> +#include "ep93xx-pcm.h"
> +#include "ep93xx-i2s.h"
> +
> +#define EP93XX_I2S_TXCLKCFG ? ? ? ? ? ?0x00
> +#define EP93XX_I2S_RXCLKCFG ? ? ? ? ? ?0x04
> +#define EP93XX_I2S_GLSTS ? ? ? ? ? ? ? 0x08
> +#define EP93XX_I2S_GLCTRL ? ? ? ? ? ? ?0x0C
> +
> +#define EP93XX_I2S_TX0LFT ? ? ? ? ? ? ?0x10
> +#define EP93XX_I2S_TX0RT ? ? ? ? ? ? ? 0x14
> +#define EP93XX_I2S_TX1LFT ? ? ? ? ? ? ?0x18
> +#define EP93XX_I2S_TX1RT ? ? ? ? ? ? ? 0x1C
> +#define EP93XX_I2S_TX2LFT ? ? ? ? ? ? ?0x20
> +#define EP93XX_I2S_TX2RT ? ? ? ? ? ? ? 0x24
> +#define EP93XX_I2S_TXLINCTRLDATA ? ? ? 0x28
> +#define EP93XX_I2S_TXCTRL ? ? ? ? ? ? ?0x2C
> +#define EP93XX_I2S_TXWRDLEN ? ? ? ? ? ?0x30
> +#define EP93XX_I2S_TX0EN ? ? ? ? ? ? ? 0x34
> +#define EP93XX_I2S_TX1EN ? ? ? ? ? ? ? 0x38
> +#define EP93XX_I2S_TX2EN ? ? ? ? ? ? ? 0x3C
> +
> +#define EP93XX_I2S_RX0LFT ? ? ? ? ? ? ?0x40
> +#define EP93XX_I2S_RX0RT ? ? ? ? ? ? ? 0x44
> +#define EP93XX_I2S_RX1LFT ? ? ? ? ? ? ?0x48
> +#define EP93XX_I2S_RX1RT ? ? ? ? ? ? ? 0x4C
> +#define EP93XX_I2S_RX2LFT ? ? ? ? ? ? ?0x50
> +#define EP93XX_I2S_RX2RT ? ? ? ? ? ? ? 0x54
> +
> +#define EP93XX_I2S_RXLINCTRLDATA ? ? ? 0x58
> +#define EP93XX_I2S_RXCTRL ? ? ? ? ? ? ?0x5C
> +#define EP93XX_I2S_RXWRDLEN ? ? ? ? ? ?0x60
> +#define EP93XX_I2S_RX0EN ? ? ? ? ? ? ? 0x64
> +#define EP93XX_I2S_RX1EN ? ? ? ? ? ? ? 0x68
> +#define EP93XX_I2S_RX2EN ? ? ? ? ? ? ? 0x6C
> +
> +#define EP93XX_I2S_WRDLEN_16 ? ? ? ? ? (0 << 0)
> +#define EP93XX_I2S_WRDLEN_24 ? ? ? ? ? (1 << 0)
> +#define EP93XX_I2S_WRDLEN_32 ? ? ? ? ? (2 << 0)
> +
> +#define EP93XX_I2S_LINCTRLDATA_R_JUST ?(1 << 2) /* Right justify */
> +
> +#define EP93XX_I2S_CLKCFG_LRS ? ? ? ? ?(1 << 0) /* lrclk polarity */
> +#define EP93XX_I2S_CLKCFG_CKP ? ? ? ? ?(1 << 1) /* Bit clock polarity */
> +#define EP93XX_I2S_CLKCFG_REL ? ? ? ? ?(1 << 2) /* ?*/
> +#define EP93XX_I2S_CLKCFG_MASTER ? ? ? (1 << 3) /* Master mode */
> +#define EP93XX_I2S_CLKCFG_NBCG ? ? ? ? (1 << 4) /* Not bit clock gating */
> +
> +struct ep93xx_i2s_info {
> + ? ? ? struct clk ? ? ? ? ? ? ? ? ? ? ?*i2s_clk;
> + ? ? ? struct ep93xx_pcm_dma_params ? ?*dma_params[2];
> + ? ? ? struct resource ? ? ? ? ? ? ? ? *mem;
> + ? ? ? void __iomem ? ? ? ? ? ? ? ? ? ?*regs;
> +};

These defines and struct definitions should be in a header file,
probably ep93xx-i2s.h.

I think the struct name should be something like ep93xx_i2s_priv.
*_info is very vague, but *_priv tells us it's passed around as an
opaque structure filled with device specific data.

> +
> +static struct ep93xx_pcm_dma_params ep93xx_i2s_pcm_out = {
> + ? ? ? .name ? ? ? ? ? = "i2s-pcm-out",
> + ? ? ? .dma_port ? ? ? = EP93XX_DMA_M2P_PORT_I2S1,
> +};
> +
> +static struct ep93xx_pcm_dma_params ep93xx_i2s_pcm_in = {
> + ? ? ? .name ? ? ? ? ? = "i2s-pcm-in",
> + ? ? ? .dma_port ? ? ? = EP93XX_DMA_M2P_PORT_I2S1,
> +};
> +
> +static inline void ep93xx_i2s_write_reg(struct ep93xx_i2s_info *info,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned reg, unsigned val)
> +{
> + ? ? ? __raw_writel(val, info->regs + reg);
> +}
> +
> +static inline unsigned ep93xx_i2s_read_reg(struct ep93xx_i2s_info *info,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned reg)
> +{
> + ? ? ? return __raw_readl(info->regs + reg);
> +}
> +
> +static inline void ep93xx_i2s_enable(struct ep93xx_i2s_info *info, int enable)
> +{
> + ? ? ? ep93xx_i2s_write_reg(info, EP93XX_I2S_GLCTRL, enable);
> +}
> +
> +static inline void ep93xx_i2s_enable_fifos(struct ep93xx_i2s_info *info,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int enable)
> +{
> + ? ? ? ep93xx_i2s_write_reg(info, EP93XX_I2S_RX0EN, enable);
> + ? ? ? ep93xx_i2s_write_reg(info, EP93XX_I2S_RX1EN, enable);
> + ? ? ? ep93xx_i2s_write_reg(info, EP93XX_I2S_RX2EN, enable);
> + ? ? ? ep93xx_i2s_write_reg(info, EP93XX_I2S_TX0EN, enable);
> + ? ? ? ep93xx_i2s_write_reg(info, EP93XX_I2S_TX1EN, enable);
> + ? ? ? ep93xx_i2s_write_reg(info, EP93XX_I2S_TX2EN, enable);
> +}
> +
> +static int ep93xx_i2s_startup(struct snd_pcm_substream *substream,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct snd_soc_dai *dai)
> +{
> + ? ? ? struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + ? ? ? struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> + ? ? ? struct ep93xx_i2s_info *info = rtd->dai->cpu_dai->private_data;
> +
> + ? ? ? clk_enable(info->i2s_clk);
> + ? ? ? cpu_dai->dma_data = info->dma_params[substream->stream];
> + ? ? ? return 0;
> +}
> +
> +static void ep93xx_i2s_shutdown(struct snd_pcm_substream *substream,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct snd_soc_dai *dai)
> +{
> + ? ? ? struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + ? ? ? struct ep93xx_i2s_info *info = rtd->dai->cpu_dai->private_data;
> +
> + ? ? ? clk_disable(info->i2s_clk);
> +}
> +
> +static int ep93xx_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct snd_soc_dai *dai)
> +{
> + ? ? ? return 0;
> +}

Unless there's a platform or cpu trigger function, this function is
unnecesssary. See soc_pcm_trigger in sound/soc/soc-core.c.

> +
> +static int ep93xx_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int fmt)
> +{
> + ? ? ? struct ep93xx_i2s_info *info = cpu_dai->private_data;
> + ? ? ? unsigned int clk_cfg, lin_ctrl;
> +
> + ? ? ? clk_cfg ?= ep93xx_i2s_read_reg(info, EP93XX_I2S_RXCLKCFG);
> + ? ? ? lin_ctrl = ep93xx_i2s_read_reg(info, EP93XX_I2S_RXLINCTRLDATA);
> +
> + ? ? ? switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> + ? ? ? case SND_SOC_DAIFMT_I2S:
> + ? ? ? ? ? ? ? clk_cfg |= EP93XX_I2S_CLKCFG_REL;
> + ? ? ? ? ? ? ? lin_ctrl &= ~EP93XX_I2S_LINCTRLDATA_R_JUST;
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? case SND_SOC_DAIFMT_LEFT_J:
> + ? ? ? ? ? ? ? clk_cfg &= ~EP93XX_I2S_CLKCFG_REL;
> + ? ? ? ? ? ? ? lin_ctrl &= ~EP93XX_I2S_LINCTRLDATA_R_JUST;
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? case SND_SOC_DAIFMT_RIGHT_J:
> + ? ? ? ? ? ? ? clk_cfg &= ~EP93XX_I2S_CLKCFG_REL;
> + ? ? ? ? ? ? ? lin_ctrl |= EP93XX_I2S_LINCTRLDATA_R_JUST;
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? default:
> + ? ? ? ? ? ? ? return -EINVAL;
> + ? ? ? }
> +
> + ? ? ? switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> + ? ? ? case SND_SOC_DAIFMT_CBS_CFS:
> + ? ? ? ? ? ? ? /* CPU is master */
> + ? ? ? ? ? ? ? clk_cfg |= EP93XX_I2S_CLKCFG_MASTER;
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? case SND_SOC_DAIFMT_CBM_CFM:
> + ? ? ? ? ? ? ? /* Codec is master */
> + ? ? ? ? ? ? ? clk_cfg &= ~EP93XX_I2S_CLKCFG_MASTER;
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? default:
> + ? ? ? ? ? ? ? return -EINVAL;
> + ? ? ? }
> +
> + ? ? ? switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> + ? ? ? case SND_SOC_DAIFMT_NB_NF:
> + ? ? ? ? ? ? ? /* Negative bit clock, lrclk low on left word */
> + ? ? ? ? ? ? ? clk_cfg &= ~(EP93XX_I2S_CLKCFG_CKP | EP93XX_I2S_CLKCFG_REL);
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? case SND_SOC_DAIFMT_NB_IF:
> + ? ? ? ? ? ? ? /* Negative bit clock, lrclk low on right word */
> + ? ? ? ? ? ? ? clk_cfg &= ~EP93XX_I2S_CLKCFG_CKP;
> + ? ? ? ? ? ? ? clk_cfg |= EP93XX_I2S_CLKCFG_REL;
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? case SND_SOC_DAIFMT_IB_NF:
> + ? ? ? ? ? ? ? /* Positive bit clock, lrclk low on left word */
> + ? ? ? ? ? ? ? clk_cfg |= EP93XX_I2S_CLKCFG_CKP;
> + ? ? ? ? ? ? ? clk_cfg &= ~EP93XX_I2S_CLKCFG_REL;
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? case SND_SOC_DAIFMT_IB_IF:
> + ? ? ? ? ? ? ? /* Positive bit clock, lrclk low on right word */
> + ? ? ? ? ? ? ? clk_cfg |= EP93XX_I2S_CLKCFG_CKP | EP93XX_I2S_CLKCFG_REL;
> + ? ? ? ? ? ? ? break;
> + ? ? ? }
> +
> + ? ? ? /* Write new register values */
> + ? ? ? ep93xx_i2s_write_reg(info, EP93XX_I2S_RXCLKCFG, clk_cfg);
> + ? ? ? ep93xx_i2s_write_reg(info, EP93XX_I2S_TXCLKCFG, clk_cfg);
> + ? ? ? ep93xx_i2s_write_reg(info, EP93XX_I2S_RXLINCTRLDATA, lin_ctrl);
> + ? ? ? ep93xx_i2s_write_reg(info, EP93XX_I2S_TXLINCTRLDATA, lin_ctrl);
> + ? ? ? return 0;
> +}
> +
> +static int ep93xx_i2s_hw_params(struct snd_pcm_substream *substream,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct snd_pcm_hw_params *params,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct snd_soc_dai *dai)
> +{
> + ? ? ? struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + ? ? ? struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> + ? ? ? struct ep93xx_i2s_info *info = cpu_dai->private_data;
> + ? ? ? unsigned word_len;
> +
> + ? ? ? switch (params_format(params)) {
> + ? ? ? case SNDRV_PCM_FORMAT_S16_LE:
> + ? ? ? ? ? ? ? word_len = EP93XX_I2S_WRDLEN_16;
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? case SNDRV_PCM_FORMAT_S24_LE:
> + ? ? ? ? ? ? ? word_len = EP93XX_I2S_WRDLEN_24;
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? case SNDRV_PCM_FORMAT_S32_LE:
> + ? ? ? ? ? ? ? word_len = EP93XX_I2S_WRDLEN_32;
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? default:
> + ? ? ? ? ? ? ? return -EINVAL;
> + ? ? ? }
> +
> + ? ? ? if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> + ? ? ? ? ? ? ? ep93xx_i2s_write_reg(info, EP93XX_I2S_TXWRDLEN, word_len);
> + ? ? ? else
> + ? ? ? ? ? ? ? ep93xx_i2s_write_reg(info, EP93XX_I2S_TXWRDLEN, word_len);

The conditional is unnecessary here.

> + ? ? ? return 0;
> +}
> +
> +static int ep93xx_i2s_set_sysclk(struct snd_soc_dai *cpu_dai, int clk_id,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned int freq, int dir)
> +{
> + ? ? ? struct ep93xx_i2s_info *info = cpu_dai->private_data;
> +
> + ? ? ? if (dir == SND_SOC_CLOCK_IN || clk_id != 0)
> + ? ? ? ? ? ? ? return -EINVAL;
> +
> + ? ? ? return clk_set_rate(info->i2s_clk, freq);
> +}
> +
> +static int ep93xx_i2s_set_clkdiv(struct snd_soc_dai *cpu_dai, int div_id,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int div)
> +{
> + ? ? ? unsigned val;
> +
> + ? ? ? /*
> + ? ? ? ?* The LRDIV (frame clock) and SDIV (bit clock) controls are in the
> + ? ? ? ?* EP93XX_SYSCON_I2SCLKDIV register, which is also used by clock.c
> + ? ? ? ?* to set the I2S master clock.
> + ? ? ? ?*
> + ? ? ? ?* FIXME - This functions is potentially racy with the clock api for
> + ? ? ? ?* the I2S master clock. Its also ugly modifying the syscon registers
> + ? ? ? ?* here.

Yeah, this is ugly, but if there's a potential race condition can we
put a mutex in there?

> + ? ? ? ?*/
> + ? ? ? val = __raw_readl( EP93XX_SYSCON_I2SCLKDIV);
> +
> + ? ? ? switch (div_id) {
> + ? ? ? case EP93XX_I2S_SDIV:
> + ? ? ? ? ? ? ? /* SCLK = MCLK / div */
> + ? ? ? ? ? ? ? switch (div) {
> + ? ? ? ? ? ? ? case 2:
> + ? ? ? ? ? ? ? ? ? ? ? val &= ~(0x1 << EP93XX_I2S_SDIV);
> + ? ? ? ? ? ? ? ? ? ? ? break;
> +
> + ? ? ? ? ? ? ? case 4:
> + ? ? ? ? ? ? ? ? ? ? ? val |= 0x1 << EP93XX_I2S_SDIV;
> + ? ? ? ? ? ? ? ? ? ? ? break;
> +
> + ? ? ? ? ? ? ? default:
> + ? ? ? ? ? ? ? ? ? ? ? return -EINVAL;
> + ? ? ? ? ? ? ? }
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? case EP93XX_I2S_LRDIV:
> + ? ? ? ? ? ? ? /* LRCLK = SCLK / div */
> + ? ? ? ? ? ? ? switch (div) {
> + ? ? ? ? ? ? ? case 32:
> + ? ? ? ? ? ? ? ? ? ? ? val &= ~(0x3 << EP93XX_I2S_LRDIV);
> + ? ? ? ? ? ? ? ? ? ? ? break;
> +
> + ? ? ? ? ? ? ? case 64:
> + ? ? ? ? ? ? ? ? ? ? ? val &= ~(0x3 << EP93XX_I2S_LRDIV);
> + ? ? ? ? ? ? ? ? ? ? ? val |= 0x1 << EP93XX_I2S_LRDIV;
> + ? ? ? ? ? ? ? ? ? ? ? break;
> +
> + ? ? ? ? ? ? ? case 128:
> + ? ? ? ? ? ? ? ? ? ? ? val &= ~(0x3 << EP93XX_I2S_LRDIV);
> + ? ? ? ? ? ? ? ? ? ? ? val |= 0x2 << EP93XX_I2S_LRDIV;
> + ? ? ? ? ? ? ? ? ? ? ? break;
> +
> + ? ? ? ? ? ? ? default:
> + ? ? ? ? ? ? ? ? ? ? ? return -EINVAL;
> + ? ? ? ? ? ? ? }
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? default:
> + ? ? ? ? ? ? ? return -EINVAL;
> + ? ? ? }
> +
> + ? ? ? ep93xx_syscon_swlocked_write(val, EP93XX_SYSCON_I2SCLKDIV);
> + ? ? ? return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int ep93xx_i2s_suspend(struct snd_soc_dai *dai)
> +{
> + ? ? ? struct ep93xx_i2s_info *info = dai->private_data;
> +
> + ? ? ? if (!dai->active)
> + ? ? ? ? ? ? ? return;
> +
> + ? ? ? ep93xx_i2s_enable(info, 0);
> +}
> +
> +static int ep93xx_i2s_resume(struct snd_soc_dai *dai)
> +{
> + ? ? ? struct ep93xx_i2s_info *info = dai->private_data;
> +
> + ? ? ? if (!dai->active)
> + ? ? ? ? ? ? ? return;
> +
> + ? ? ? ep93xx_i2s_enable(info, 1);
> +}
> +#else
> +#define ep93xx_i2s_suspend ? ? NULL
> +#define ep93xx_i2s_resume ? ? ?NULL
> +#endif
> +
> +#define EP93XX_I2S_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
> + ? ? ? ? ? ? ? ? ? ? ? ? ? SNDRV_PCM_FMTBIT_S24_LE | \
> + ? ? ? ? ? ? ? ? ? ? ? ? ? SNDRV_PCM_FMTBIT_S32_LE)

I'd either move this to be just above its usage, or in a header file.
Right here it's an island of its own.

> +
> +static struct snd_soc_dai_ops ep93xx_i2s_dai_ops = {
> + ? ? ? .startup ? ? ? ?= ep93xx_i2s_startup,
> + ? ? ? .shutdown ? ? ? = ep93xx_i2s_shutdown,
> + ? ? ? .trigger ? ? ? ?= ep93xx_i2s_trigger,
> + ? ? ? .hw_params ? ? ?= ep93xx_i2s_hw_params,
> + ? ? ? .set_sysclk ? ? = ep93xx_i2s_set_sysclk,
> + ? ? ? .set_clkdiv ? ? = ep93xx_i2s_set_clkdiv,
> + ? ? ? .set_fmt ? ? ? ?= ep93xx_i2s_set_dai_fmt,
> +};
> +
> +struct snd_soc_dai ep93xx_i2s_dai = {
> + ? ? ? .name ? ? ? ? ? = "ep93xx-i2s",
> + ? ? ? .id ? ? ? ? ? ? = 0,
> + ? ? ? .suspend ? ? ? ?= ep93xx_i2s_suspend,
> + ? ? ? .resume ? ? ? ? = ep93xx_i2s_resume,
> + ? ? ? .playback ? ? ? = {
> + ? ? ? ? ? ? ? .channels_min ? = 2,
> + ? ? ? ? ? ? ? .channels_max ? = 2,
> + ? ? ? ? ? ? ? .rates ? ? ? ? ?= SNDRV_PCM_RATE_8000_48000,
> + ? ? ? ? ? ? ? .formats ? ? ? ?= EP93XX_I2S_FORMATS,
> + ? ? ? },
> + ? ? ? .capture ? ? ? ?= {
> + ? ? ? ? ? ? ? ?.channels_min ?= 2,
> + ? ? ? ? ? ? ? ?.channels_max ?= 2,
> + ? ? ? ? ? ? ? ?.rates ? ? ? ? = SNDRV_PCM_RATE_8000_48000,
> + ? ? ? ? ? ? ? ?.formats ? ? ? = EP93XX_I2S_FORMATS,
> + ? ? ? },
> + ? ? ? .ops ? ? ? ? ? ?= &ep93xx_i2s_dai_ops,
> +};
> +EXPORT_SYMBOL_GPL(ep93xx_i2s_dai);
> +
> +static int ep93xx_i2s_probe(struct platform_device *pdev)
> +{
> + ? ? ? struct ep93xx_i2s_info *info;
> + ? ? ? struct resource *res;
> + ? ? ? int err;
> +
> + ? ? ? info = kzalloc(sizeof(struct ep93xx_i2s_info), GFP_KERNEL);
> + ? ? ? if (!info) {
> + ? ? ? ? ? ? ? err = -ENOMEM;
> + ? ? ? ? ? ? ? goto fail;
> + ? ? ? }
> +
> + ? ? ? ep93xx_i2s_dai.dev = &pdev->dev;
> + ? ? ? ep93xx_i2s_dai.private_data = info;
> +
> + ? ? ? info->dma_params[SNDRV_PCM_STREAM_PLAYBACK] = &ep93xx_i2s_pcm_out;
> + ? ? ? info->dma_params[SNDRV_PCM_STREAM_CAPTURE] ?= &ep93xx_i2s_pcm_in;
> +
> + ? ? ? err = snd_soc_register_dai(&ep93xx_i2s_dai);
> + ? ? ? if (err)
> + ? ? ? ? ? ? ? goto fail_free_info;
> +
> + ? ? ? res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + ? ? ? if (!res) {
> + ? ? ? ? ? ? ? err = -ENODEV;
> + ? ? ? ? ? ? ? goto fail_unregister_dai;
> + ? ? ? }
> +
> + ? ? ? info->mem = request_mem_region(res->start, resource_size(res),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?pdev->name);
> + ? ? ? if (!info->mem) {
> + ? ? ? ? ? ? ? err = -EBUSY;
> + ? ? ? ? ? ? ? goto fail_free_info;
> + ? ? ? }
> +
> + ? ? ? info->regs = ioremap(info->mem->start, resource_size(info->mem));
> + ? ? ? if (!info->regs) {
> + ? ? ? ? ? ? ? err = -ENXIO;
> + ? ? ? ? ? ? ? goto fail_release_mem;
> + ? ? ? }
> +
> + ? ? ? info->i2s_clk = clk_get(&pdev->dev, NULL);
> + ? ? ? if (IS_ERR(info->i2s_clk)) {
> + ? ? ? ? ? ? ? err = PTR_ERR(info->i2s_clk);
> + ? ? ? ? ? ? ? goto fail_unmap_mem;
> + ? ? ? }
> +
> + ? ? ? ep93xx_i2s_enable_fifos(info, 1);
> + ? ? ? ep93xx_i2s_enable(info, 1);
> + ? ? ? return 0;
> +
> +fail_unmap_mem:
> + ? ? ? iounmap(info->regs);
> +fail_release_mem:
> + ? ? ? release_mem_region(info->mem->start, resource_size(info->mem));
> +fail_unregister_dai:
> + ? ? ? snd_soc_unregister_dai(&ep93xx_i2s_dai);
> +fail_free_info:
> + ? ? ? kfree(info);
> +fail:
> + ? ? ? return err;
> +}
> +
> +static int __devexit ep93xx_i2s_remove(struct platform_device *pdev)
> +{
> + ? ? ? struct ep93xx_i2s_info *info = ep93xx_i2s_dai.private_data;
> +
> + ? ? ? ep93xx_i2s_enable_fifos(info, 0);
> + ? ? ? ep93xx_i2s_enable(info, 0);
> +
> + ? ? ? clk_put(info->i2s_clk);
> + ? ? ? iounmap(info->regs);
> + ? ? ? release_mem_region(info->mem->start, resource_size(info->mem));
> + ? ? ? snd_soc_unregister_dai(&ep93xx_i2s_dai);
> + ? ? ? kfree(info);
> + ? ? ? return 0;
> +}
> +
> +static struct platform_driver ep93xx_i2s_driver = {
> + ? ? ? .probe ?= ep93xx_i2s_probe,
> + ? ? ? .remove = __devexit_p(ep93xx_i2s_remove),
> + ? ? ? .driver = {
> + ? ? ? ? ? ? ? .name ? = "ep93xx-i2s",
> + ? ? ? ? ? ? ? .owner ?= THIS_MODULE,
> + ? ? ? },
> +};
> +
> +static int __init ep93xx_i2s_init(void)
> +{
> + ? ? ? return platform_driver_register(&ep93xx_i2s_driver);
> +}
> +
> +static void __exit ep93xx_i2s_exit(void)
> +{
> + ? ? ? platform_driver_unregister(&ep93xx_i2s_driver);
> +}
> +
> +module_init(ep93xx_i2s_init);
> +module_exit(ep93xx_i2s_exit);
> +
> +MODULE_ALIAS("platform:ep93xx-i2s");
> +MODULE_AUTHOR("Ryan Mallon <ryan@bluewatersys.com>");
> +MODULE_DESCRIPTION("EP93XX I2S driver");
> +MODULE_LICENSE("GPL");
> diff --git a/sound/soc/ep93xx/ep93xx-i2s.h b/sound/soc/ep93xx/ep93xx-i2s.h
> new file mode 100644
> index 0000000..37e6370
> --- /dev/null
> +++ b/sound/soc/ep93xx/ep93xx-i2s.h

It's my opinion that every file, unless really really trivial, should
have a license header. I suggest adding one here.

> @@ -0,0 +1,13 @@
> +#ifndef _EP93XX_SND_SOC_I2S_H
> +#define _EP93XX_SND_SOC_I2S_H
> +
> +/*
> + * These are the offsets for the LRDIV and SDIV fields in the syscon i2sclkdiv
> + * register.
> + */
> +#define EP93XX_I2S_LRDIV ? ? ? 17
> +#define EP93XX_I2S_SDIV ? ? ? ? ? ? ? ?16
> +
> +extern struct snd_soc_dai ep93xx_i2s_dai;
> +
> +#endif /* _EP93XX_SND_SOC_I2S_H */
> diff --git a/sound/soc/ep93xx/ep93xx-pcm.c b/sound/soc/ep93xx/ep93xx-pcm.c
> new file mode 100644
> index 0000000..c071b5c
> --- /dev/null
> +++ b/sound/soc/ep93xx/ep93xx-pcm.c
> @@ -0,0 +1,321 @@
> +/*
> + * linux/sound/arm/ep93xx-pcm.c - EP93xx ALSA PCM interface
> + *
> + * Copyright (C) 2006 Lennert Buytenhek <buytenh@wantstofly.org>
> + * Copyright (C) 2006 Applied Data Systems
> + *
> + * Rewritten for the SoC audio subsystem (Based on PXA2xx code):
> + * ? Copyright (c) 2008 Ryan Mallon <ryan@bluewatersys.com>
> + *
> + * 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.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <linux/dma-mapping.h>
> +
> +#include <sound/core.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +
> +#include <mach/dma.h>
> +#include <mach/hardware.h>
> +#include <mach/ep93xx-regs.h>
> +
> +#include "ep93xx-pcm.h"
> +
> +static const struct snd_pcm_hardware ep93xx_pcm_hardware = {
> + ? ? ? .info ? ? ? ? ? ? ? ? ? = (SNDRV_PCM_INFO_MMAP ? ? ? ? ?|
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?SNDRV_PCM_INFO_MMAP_VALID ? ?|
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?SNDRV_PCM_INFO_INTERLEAVED ? |
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?SNDRV_PCM_INFO_BLOCK_TRANSFER),
> +
> + ? ? ? .rates ? ? ? ? ? ? ? ? ?= SNDRV_PCM_RATE_8000_48000,
> + ? ? ? .rate_min ? ? ? ? ? ? ? = SNDRV_PCM_RATE_8000,
> + ? ? ? .rate_max ? ? ? ? ? ? ? = SNDRV_PCM_RATE_48000,
> +
> + ? ? ? .formats ? ? ? ? ? ? ? ?= (SNDRV_PCM_FMTBIT_S16_LE |
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?SNDRV_PCM_FMTBIT_S24_LE |
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?SNDRV_PCM_FMTBIT_S32_LE),
> +
> + ? ? ? .buffer_bytes_max ? ? ? = 131072,
> + ? ? ? .period_bytes_min ? ? ? = 32,
> + ? ? ? .period_bytes_max ? ? ? = 32768,
> + ? ? ? .periods_min ? ? ? ? ? ?= 1,
> + ? ? ? .periods_max ? ? ? ? ? ?= 32,
> + ? ? ? .fifo_size ? ? ? ? ? ? ?= 32,
> +};
> +
> +struct ep93xx_runtime_data
> +{
> + ? ? ? struct ep93xx_dma_m2p_client ? ?cl;
> + ? ? ? struct ep93xx_pcm_dma_params ? ?*params;
> + ? ? ? int ? ? ? ? ? ? ? ? ? ? ? ? ? ? pointer_bytes;
> + ? ? ? struct tasklet_struct ? ? ? ? ? period_tasklet;
> + ? ? ? int ? ? ? ? ? ? ? ? ? ? ? ? ? ? periods;
> + ? ? ? struct ep93xx_dma_buffer ? ? ? ?buf[32];
> +};

I'd rather this be named something like "ep93xx_pcm_priv".

> +
> +static void ep93xx_pcm_period_elapsed(unsigned long data)
> +{
> + ? ? ? struct snd_pcm_substream *substream = (struct snd_pcm_substream *)data;
> + ? ? ? snd_pcm_period_elapsed(substream);
> +}
> +
> +static void ep93xx_pcm_buffer_started(void *cookie,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct ep93xx_dma_buffer *buf)
> +{
> +}

Hrm, this seems rather pointless, but the dma code doesn't check for a
valid function pointer before calling this. It seems necessary for
now.

> +
> +static void ep93xx_pcm_buffer_finished(void *cookie,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct ep93xx_dma_buffer *buf,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int bytes, int error)
> +{
> + ? ? ? struct snd_pcm_substream *substream = cookie;
> + ? ? ? struct ep93xx_runtime_data *rtd = substream->runtime->private_data;
> +
> + ? ? ? if (buf == rtd->buf + rtd->periods - 1)
> + ? ? ? ? ? ? ? rtd->pointer_bytes = 0;
> + ? ? ? else
> + ? ? ? ? ? ? ? rtd->pointer_bytes += buf->size;
> +
> + ? ? ? if (!error) {
> + ? ? ? ? ? ? ? ep93xx_dma_m2p_submit_recursive(&rtd->cl, buf);
> + ? ? ? ? ? ? ? tasklet_schedule(&rtd->period_tasklet);
> + ? ? ? } else {
> + ? ? ? ? ? ? ? snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
> + ? ? ? }
> +}
> +
> +static int ep93xx_pcm_open(struct snd_pcm_substream *substream)
> +{
> + ? ? ? struct snd_soc_pcm_runtime *soc_rtd = substream->private_data;
> + ? ? ? struct ep93xx_pcm_dma_params *dma_params = soc_rtd->dai->cpu_dai->dma_data;
> + ? ? ? struct ep93xx_runtime_data *rtd;
> + ? ? ? int ret;
> +
> + ? ? ? snd_soc_set_runtime_hwparams(substream, &ep93xx_pcm_hardware);
> +
> + ? ? ? rtd = kmalloc(sizeof(*rtd), GFP_KERNEL);
> + ? ? ? if (!rtd)
> + ? ? ? ? ? ? ? return -ENOMEM;
> +
> + ? ? ? memset(&rtd->period_tasklet, 0, sizeof(rtd->period_tasklet));
> + ? ? ? rtd->period_tasklet.func = ep93xx_pcm_period_elapsed;
> + ? ? ? rtd->period_tasklet.data = (unsigned long)substream;
> +
> + ? ? ? rtd->cl.name = dma_params->name;
> + ? ? ? rtd->cl.flags = dma_params->dma_port | EP93XX_DMA_M2P_IGNORE_ERROR |
> + ? ? ? ? ? ? ? ((substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
> + ? ? ? ? ? ? ? ?EP93XX_DMA_M2P_TX : EP93XX_DMA_M2P_RX);
> + ? ? ? rtd->cl.cookie = substream;
> + ? ? ? rtd->cl.buffer_started = ep93xx_pcm_buffer_started;
> + ? ? ? rtd->cl.buffer_finished = ep93xx_pcm_buffer_finished;
> + ? ? ? ret = ep93xx_dma_m2p_client_register(&rtd->cl);
> + ? ? ? if (ret < 0) {
> + ? ? ? ? ? ? ? kfree(rtd);
> + ? ? ? ? ? ? ? return ret;
> + ? ? ? }
> +
> + ? ? ? substream->runtime->private_data = rtd;
> + ? ? ? return 0;
> +}
> +
> +static int ep93xx_pcm_close(struct snd_pcm_substream *substream)
> +{
> + ? ? ? struct ep93xx_runtime_data *rtd = substream->runtime->private_data;
> +
> + ? ? ? ep93xx_dma_m2p_client_unregister(&rtd->cl);
> + ? ? ? kfree(rtd);
> + ? ? ? return 0;
> +}
> +
> +static int ep93xx_pcm_hw_params(struct snd_pcm_substream *substream,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct snd_pcm_hw_params *params)
> +{
> + ? ? ? struct snd_pcm_runtime *runtime = substream->runtime;
> + ? ? ? struct ep93xx_runtime_data *rtd = runtime->private_data;
> + ? ? ? size_t totsize = params_buffer_bytes(params);
> + ? ? ? size_t period = params_period_bytes(params);
> + ? ? ? int i;
> +
> + ? ? ? snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
> + ? ? ? runtime->dma_bytes = totsize;
> +
> + ? ? ? rtd->periods = (totsize + period - 1) / period;
> + ? ? ? for (i = 0; i < rtd->periods; i++) {
> + ? ? ? ? ? ? ? rtd->buf[i].bus_addr = runtime->dma_addr + (i * period);
> + ? ? ? ? ? ? ? rtd->buf[i].size = period;
> + ? ? ? ? ? ? ? if ((i + 1) * period > totsize)
> + ? ? ? ? ? ? ? ? ? ? ? rtd->buf[i].size = totsize - (i * period);
> + ? ? ? }
> +
> + ? ? ? return 0;
> +}
> +
> +static int ep93xx_pcm_hw_free(struct snd_pcm_substream *substream)
> +{
> + ? ? ? snd_pcm_set_runtime_buffer(substream, NULL);
> + ? ? ? return 0;
> +}
> +
> +static int ep93xx_pcm_prepare(struct snd_pcm_substream *substream)
> +{
> + ? ? ? return 0;
> +}
> +
> +static int ep93xx_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
> +{
> + ? ? ? struct ep93xx_runtime_data *rtd = substream->runtime->private_data;
> + ? ? ? int ret;
> + ? ? ? int i;
> +
> + ? ? ? ret = 0;
> + ? ? ? switch (cmd) {
> + ? ? ? case SNDRV_PCM_TRIGGER_START:
> + ? ? ? case SNDRV_PCM_TRIGGER_RESUME:
> + ? ? ? case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> + ? ? ? ? ? ? ? rtd->pointer_bytes = 0;
> + ? ? ? ? ? ? ? for (i = 0; i < rtd->periods; i++)
> + ? ? ? ? ? ? ? ? ? ? ? ep93xx_dma_m2p_submit(&rtd->cl, rtd->buf + i);
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? case SNDRV_PCM_TRIGGER_STOP:
> + ? ? ? case SNDRV_PCM_TRIGGER_SUSPEND:
> + ? ? ? case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> + ? ? ? ? ? ? ? ep93xx_dma_m2p_flush(&rtd->cl);
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? default:
> + ? ? ? ? ? ? ? ret = -EINVAL;
> + ? ? ? ? ? ? ? break;
> + ? ? ? }
> +
> + ? ? ? return ret;
> +}
> +
> +static snd_pcm_uframes_t ep93xx_pcm_pointer(struct snd_pcm_substream *substream)
> +{
> + ? ? ? struct snd_pcm_runtime *runtime = substream->runtime;
> + ? ? ? struct ep93xx_runtime_data *rtd = substream->runtime->private_data;
> +
> + ? ? ? /* FIXME: implement this with sub-period granularity */
> + ? ? ? return bytes_to_frames(runtime, rtd->pointer_bytes);
> +}
> +
> +static int ep93xx_pcm_mmap(struct snd_pcm_substream *substream,
> + ? ? ? ? ? ? ? ? ? ? ? ? ?struct vm_area_struct *vma)
> +{
> + ? ? ? struct snd_pcm_runtime *runtime = substream->runtime;
> +
> + ? ? ? return dma_mmap_writecombine(substream->pcm->card->dev, vma,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?runtime->dma_area,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?runtime->dma_addr,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?runtime->dma_bytes);
> +}
> +
> +static struct snd_pcm_ops ep93xx_pcm_ops = {
> + ? ? ? .open ? ? ? ? ? = ep93xx_pcm_open,
> + ? ? ? .close ? ? ? ? ?= ep93xx_pcm_close,
> + ? ? ? .ioctl ? ? ? ? ?= snd_pcm_lib_ioctl,
> + ? ? ? .hw_params ? ? ?= ep93xx_pcm_hw_params,
> + ? ? ? .hw_free ? ? ? ?= ep93xx_pcm_hw_free,
> + ? ? ? .prepare ? ? ? ?= ep93xx_pcm_prepare,
> + ? ? ? .trigger ? ? ? ?= ep93xx_pcm_trigger,
> + ? ? ? .pointer ? ? ? ?= ep93xx_pcm_pointer,
> + ? ? ? .mmap ? ? ? ? ? = ep93xx_pcm_mmap,
> +};
> +
> +static int ep93xx_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
> +{
> + ? ? ? struct snd_pcm_substream *substream = pcm->streams[stream].substream;
> + ? ? ? struct snd_dma_buffer *buf = &substream->dma_buffer;
> + ? ? ? size_t size = ep93xx_pcm_hardware.buffer_bytes_max;
> +
> + ? ? ? buf->dev.type = SNDRV_DMA_TYPE_DEV;
> + ? ? ? buf->dev.dev = pcm->card->dev;
> + ? ? ? buf->private_data = NULL;
> + ? ? ? buf->area = dma_alloc_writecombine(pcm->card->dev, size,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&buf->addr, GFP_KERNEL);
> + ? ? ? buf->bytes = size;
> +
> + ? ? ? return (buf->area == NULL) ? -ENOMEM : 0;
> +}
> +
> +static void ep93xx_pcm_free_dma_buffers(struct snd_pcm *pcm)
> +{
> + ? ? ? struct snd_pcm_substream *substream;
> + ? ? ? struct snd_dma_buffer *buf;
> + ? ? ? int stream;
> +
> + ? ? ? for (stream = 0; stream < 2; stream++) {
> + ? ? ? ? ? ? ? substream = pcm->streams[stream].substream;
> + ? ? ? ? ? ? ? if (!substream)
> + ? ? ? ? ? ? ? ? ? ? ? continue;
> +
> + ? ? ? ? ? ? ? buf = &substream->dma_buffer;
> + ? ? ? ? ? ? ? if (!buf->area)
> + ? ? ? ? ? ? ? ? ? ? ? continue;
> +
> + ? ? ? ? ? ? ? dma_free_writecombine(pcm->card->dev, buf->bytes, buf->area,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? buf->addr);
> + ? ? ? ? ? ? ? buf->area = NULL;
> + ? ? ? }
> +}
> +
> +static u64 ep93xx_pcm_dmamask = 0xffffffff;
> +
> +static int ep93xx_pcm_new(struct snd_card *card, struct snd_soc_dai *dai,
> + ? ? ? ? ? ? ? ? ? ? ? ? struct snd_pcm *pcm)
> +{
> + ? ? ? int ret = 0;
> +
> + ? ? ? if (!card->dev->dma_mask)
> + ? ? ? ? ? ? ? card->dev->dma_mask = &ep93xx_pcm_dmamask;
> + ? ? ? if (!card->dev->coherent_dma_mask)
> + ? ? ? ? ? ? ? card->dev->coherent_dma_mask = 0xffffffff;
> +
> + ? ? ? if (dai->playback.channels_min) {
> + ? ? ? ? ? ? ? ret = ep93xx_pcm_preallocate_dma_buffer(pcm,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? SNDRV_PCM_STREAM_PLAYBACK);
> + ? ? ? ? ? ? ? if (ret)
> + ? ? ? ? ? ? ? ? ? ? ? return ret;
> +
> + ? ? ? ? ? ? ? ret = ep93xx_pcm_preallocate_dma_buffer(pcm,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? SNDRV_PCM_STREAM_CAPTURE);
> + ? ? ? ? ? ? ? if (ret)
> + ? ? ? ? ? ? ? ? ? ? ? return ret;
> + ? ? ? }
> +
> + ? ? ? return 0;
> +}
> +
> +struct snd_soc_platform ep93xx_soc_platform = {
> + ? ? ? .name ? ? ? ? ? = "ep93xx-audio",
> + ? ? ? .pcm_ops ? ? ? ?= &ep93xx_pcm_ops,
> + ? ? ? .pcm_new ? ? ? ?= &ep93xx_pcm_new,
> + ? ? ? .pcm_free ? ? ? = &ep93xx_pcm_free_dma_buffers,
> +};
> +EXPORT_SYMBOL_GPL(ep93xx_soc_platform);
> +
> +static int __init ep93xx_soc_platform_init(void)
> +{
> + ? ? ? return snd_soc_register_platform(&ep93xx_soc_platform);
> +}
> +
> +static void __exit ep93xx_soc_platform_exit(void)
> +{
> + ? ? ? snd_soc_unregister_platform(&ep93xx_soc_platform);
> +}
> +
> +module_init(ep93xx_soc_platform_init);
> +module_exit(ep93xx_soc_platform_exit);
> +
> +MODULE_AUTHOR("Lennert Buytenhek <buytenh@wantstofly.org>");

Lennert may be the original author, but if you're proposing this for
upstream you probably want to put yourself here. That is, unless
you've chatted with Lennert about this. It seems presumptuous to get
code in the kernel and point people to someone else when it breaks :).

> +MODULE_DESCRIPTION("EP93xx ALSA PCM interface");
> +MODULE_LICENSE("GPL");
> diff --git a/sound/soc/ep93xx/ep93xx-pcm.h b/sound/soc/ep93xx/ep93xx-pcm.h
> new file mode 100644
> index 0000000..4ffdd3f
> --- /dev/null
> +++ b/sound/soc/ep93xx/ep93xx-pcm.h
> @@ -0,0 +1,22 @@
> +/*
> + * sound/soc/ep93xx/ep93xx-pcm.h - EP93xx ALSA PCM interface
> + *
> + * Copyright (C) 2006 Lennert Buytenhek <buytenh@wantstofly.org>
> + * Copyright (C) 2006 Applied Data Systems
> + *
> + * 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 _EP93XX_SND_SOC_PCM_H
> +#define _EP93XX_SND_SOC_PCM_H
> +
> +struct ep93xx_pcm_dma_params {
> + ? ? ? char ? ?*name;
> + ? ? ? int ? ? dma_port;
> +};
> +
> +extern struct snd_soc_platform ep93xx_soc_platform;
> +
> +#endif /* _EP93XX_SND_SOC_PCM_H */

-- Chase

[RFC PATCH 2/3] ep93xx i2s core support

[Chase Douglas]

On Tue, May 18, 2010 at 12:54 AM, Ryan Mallon <ryan@bluewatersys.com> wrote:
> Add ep93xx core support for i2s audio
>
> Signed-off-by: Ryan Mallon <ryan@bluewatersys.com>
> ---
>
> diff --git a/arch/arm/mach-ep93xx/clock.c b/arch/arm/mach-ep93xx/clock.c
> index 5f80092..df88233 100644
> --- a/arch/arm/mach-ep93xx/clock.c
> +++ b/arch/arm/mach-ep93xx/clock.c
> @@ -108,6 +108,16 @@ static struct clk clk_video = {
> ? ? ? ?.set_rate ? ? ? = set_div_rate,
> ?};
>
> +static struct clk clk_i2s = {
> + ? ? ? .sw_locked ? ? ?= 1,
> + ? ? ? .enable_reg ? ? = EP93XX_SYSCON_I2SCLKDIV,
> + ? ? ? .enable_mask ? ?= (EP93XX_SYSCON_CLKDIV_ENABLE ?|
> + ? ? ? ? ? ? ? ? ? ? ? ? ?EP93XX_SYSCON_I2SCLKDIV_SENA |
> + ? ? ? ? ? ? ? ? ? ? ? ? ?EP93XX_SYSCON_I2SCLKDIV_SPOL |
> + ? ? ? ? ? ? ? ? ? ? ? ? ?EP93XX_SYSCON_I2SCLKDIV_ORIDE),
> + ? ? ? .set_rate ? ? ? = set_div_rate,
> +};
> +
> ?/* DMA Clocks */
> ?static struct clk clk_m2p0 = {
> ? ? ? ?.parent ? ? ? ? = &clk_h,
> @@ -186,6 +196,7 @@ static struct clk_lookup clocks[] = {
> ? ? ? ?INIT_CK("ep93xx-ohci", ? ? ? ? ?NULL, ? ? ? ? ? &clk_usb_host),
> ? ? ? ?INIT_CK("ep93xx-keypad", ? ? ? ?NULL, ? ? ? ? ? &clk_keypad),
> ? ? ? ?INIT_CK("ep93xx-fb", ? ? ? ? ? ?NULL, ? ? ? ? ? &clk_video),
> + ? ? ? INIT_CK("ep93xx-i2s", ? ? ? ? ? NULL, ? ? ? ? ? &clk_i2s),
> ? ? ? ?INIT_CK(NULL, ? ? ? ? ? ? ? ? ? "pwm_clk", ? ? ?&clk_pwm),
> ? ? ? ?INIT_CK(NULL, ? ? ? ? ? ? ? ? ? "m2p0", ? ? ? ? &clk_m2p0),
> ? ? ? ?INIT_CK(NULL, ? ? ? ? ? ? ? ? ? "m2p1", ? ? ? ? &clk_m2p1),
> diff --git a/arch/arm/mach-ep93xx/core.c b/arch/arm/mach-ep93xx/core.c
> index 90fb591..cceeac2 100644
> --- a/arch/arm/mach-ep93xx/core.c
> +++ b/arch/arm/mach-ep93xx/core.c
> @@ -617,6 +617,37 @@ void ep93xx_keypad_release_gpio(struct platform_device *pdev)
> ?}
> ?EXPORT_SYMBOL(ep93xx_keypad_release_gpio);
>
> +/*************************************************************************
> + * EP93xx I2S audio peripheral handling
> + *************************************************************************/
> +static struct resource ep93xx_i2s_resource[] = {
> + ? ? ? {
> + ? ? ? ? ? ? ? .start ?= EP93XX_I2S_PHYS_BASE,
> + ? ? ? ? ? ? ? .end ? ?= EP93XX_I2S_PHYS_BASE + 0x100 - 1,
> + ? ? ? ? ? ? ? .flags ?= IORESOURCE_MEM,
> + ? ? ? },
> +};
> +
> +static struct platform_device ep93xx_i2s_device = {
> + ? ? ? .name ? ? ? ? ? = "ep93xx-i2s",
> + ? ? ? .id ? ? ? ? ? ? = -1,
> + ? ? ? .num_resources ?= ARRAY_SIZE(ep93xx_i2s_resource),
> + ? ? ? .resource ? ? ? = ep93xx_i2s_resource,
> +};
> +
> +void __init ep93xx_register_i2s(unsigned pins)
> +{
> + ? ? ? if (pins != EP93XX_SYSCON_DEVCFG_I2SONSSP &&
> + ? ? ? ? ? pins != EP93XX_SYSCON_DEVCFG_I2SONAC97) {
> + ? ? ? ? ? ? ? pr_err("Invalid I2S pin configuration - not registering\n");
> + ? ? ? ? ? ? ? return;
> + ? ? ? }
> +
> + ? ? ? ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_I2SONAC97 |
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?EP93XX_SYSCON_DEVCFG_I2SONAC97);

I think one of the above should be *_I2SONSSP?

> + ? ? ? ep93xx_devcfg_set_bits(pins);
> + ? ? ? platform_device_register(&ep93xx_i2s_device);
> +}
>
> ?extern void ep93xx_gpio_init(void);
>
> diff --git a/arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h b/arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h
> index 93e2ecc..074d99b 100644
> --- a/arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h
> +++ b/arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h
> @@ -93,6 +93,7 @@
> ?/* APB peripherals */
> ?#define EP93XX_TIMER_BASE ? ? ? ? ? ? ?EP93XX_APB_IOMEM(0x00010000)
>
> +#define EP93XX_I2S_PHYS_BASE ? ? ? ? ? EP93XX_APB_PHYS(0x00020000)
> ?#define EP93XX_I2S_BASE ? ? ? ? ? ? ? ? ? ? ? ?EP93XX_APB_IOMEM(0x00020000)
>
> ?#define EP93XX_SECURITY_BASE ? ? ? ? ? EP93XX_APB_IOMEM(0x00030000)
> @@ -193,6 +194,10 @@
> ?#define EP93XX_SYSCON_CLKDIV_ESEL ? ? ?(1<<14)
> ?#define EP93XX_SYSCON_CLKDIV_PSEL ? ? ?(1<<13)
> ?#define EP93XX_SYSCON_CLKDIV_PDIV_SHIFT ? ? ? ?8
> +#define EP93XX_SYSCON_I2SCLKDIV ? ? ? ? ? ? ? ?EP93XX_SYSCON_REG(0x8c)
> +#define EP93XX_SYSCON_I2SCLKDIV_SENA ? (1<<31)
> +#define EP93XX_SYSCON_I2SCLKDIV_ORIDE ? (1<<29)
> +#define EP93XX_SYSCON_I2SCLKDIV_SPOL ? (1<<19)
> ?#define EP93XX_SYSCON_KEYTCHCLKDIV ? ? EP93XX_SYSCON_REG(0x90)
> ?#define EP93XX_SYSCON_KEYTCHCLKDIV_TSEN ? ? ? ?(1<<31)
> ?#define EP93XX_SYSCON_KEYTCHCLKDIV_ADIV ? ? ? ?(1<<16)
> diff --git a/arch/arm/mach-ep93xx/include/mach/platform.h b/arch/arm/mach-ep93xx/include/mach/platform.h
> index c6dc14d..9252adf 100644
> --- a/arch/arm/mach-ep93xx/include/mach/platform.h
> +++ b/arch/arm/mach-ep93xx/include/mach/platform.h
> @@ -43,6 +43,7 @@ void ep93xx_pwm_release_gpio(struct platform_device *pdev);
> ?void ep93xx_register_keypad(struct ep93xx_keypad_platform_data *data);
> ?int ep93xx_keypad_acquire_gpio(struct platform_device *pdev);
> ?void ep93xx_keypad_release_gpio(struct platform_device *pdev);
> +void ep93xx_register_i2s(unsigned pins);
>
> ?void ep93xx_init_devices(void);
> ?extern struct sys_timer ep93xx_timer;

-- Chase

[RFC PATCH 3/3] ep93xx i2s snapper cl15 support

[Chase Douglas]

On Tue, May 18, 2010 at 12:55 AM, Ryan Mallon <ryan@bluewatersys.com> wrote:
> Added support for i2s audio on Snapper CL15
>
> Signed-off-by: Ryan Mallon <ryan@bluewatersys.com>
> ---
>
> diff --git a/arch/arm/mach-ep93xx/snappercl15.c b/arch/arm/mach-ep93xx/snappercl15.c
> index 51134b0..d4ca5cc 100644
> --- a/arch/arm/mach-ep93xx/snappercl15.c
> +++ b/arch/arm/mach-ep93xx/snappercl15.c
> @@ -157,6 +157,7 @@ static void __init snappercl15_init_machine(void)
> ? ? ? ?ep93xx_register_i2c(&snappercl15_i2c_gpio_data, snappercl15_i2c_data,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?ARRAY_SIZE(snappercl15_i2c_data));
> ? ? ? ?ep93xx_register_fb(&snappercl15_fb_info);
> + ? ? ? ep93xx_register_i2s(EP93XX_SYSCON_DEVCFG_I2SONAC97);
> ? ? ? ?platform_device_register(&snappercl15_nand_device);
> ?}
>
> diff --git a/sound/soc/ep93xx/snappercl15.c b/sound/soc/ep93xx/snappercl15.c
> new file mode 100644
> index 0000000..3fae86e
> --- /dev/null
> +++ b/sound/soc/ep93xx/snappercl15.c
> @@ -0,0 +1,186 @@
> +/*
> + * snappercl15.c -- SoC audio for Bluewater Systems Snapper CL15 module
> + *
> + * Copyright (C) 2008 Bluewater Systems Ltd
> + * Author: Ryan Mallon <ryan@bluewatersys.com>
> + *
> + * ?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/platform_device.h>
> +#include <sound/core.h>
> +#include <sound/pcm.h>
> +#include <sound/soc.h>
> +#include <sound/soc-dapm.h>
> +
> +#include <asm/mach-types.h>
> +#include <mach/hardware.h>
> +
> +#include "../codecs/tlv320aic23.h"
> +#include "ep93xx-pcm.h"
> +#include "ep93xx-i2s.h"
> +
> +//#define CODEC_CLOCK 11289600
> +#define CODEC_CLOCK 5644800

Pick one and stick to it! :)

> +
> +static int snappercl15_startup(struct snd_pcm_substream *substream)
> +{
> + ? ? ? return 0;
> +}
> +
> +static void snappercl15_shutdown(struct snd_pcm_substream *substream)
> +{
> +}
> +
> +static int snappercl15_hw_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 *codec_dai = rtd->dai->codec_dai;
> + ? ? ? struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> + ? ? ? int sdiv, lrdiv, err;
> +
> + ? ? ? switch (params_rate(params)) {
> + ? ? ? case 44100:
> + ? ? ? ? ? ? ? sdiv = 4;
> + ? ? ? ? ? ? ? lrdiv = 64;
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? case 22050:
> + ? ? ? ? ? ? ? sdiv = 4;
> + ? ? ? ? ? ? ? lrdiv = 128;
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? default:
> + ? ? ? ? ? ? ? return -EINVAL;
> + ? ? ? }
> +
> + ? ? ? err = snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_I2S |
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? SND_SOC_DAIFMT_NB_IF |
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? SND_SOC_DAIFMT_CBS_CFS);
> +
> + ? ? ? err = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_I2S |
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? SND_SOC_DAIFMT_NB_IF |
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? SND_SOC_DAIFMT_CBS_CFS);
> + ? ? ? if (err)
> + ? ? ? ? ? ? ? return err;
> +
> + ? ? ? err = snd_soc_dai_set_sysclk(codec_dai, 0, CODEC_CLOCK,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?SND_SOC_CLOCK_IN);
> + ? ? ? if (err)
> + ? ? ? ? ? ? ? return err;
> +
> + ? ? ? err = snd_soc_dai_set_sysclk(cpu_dai, 0, CODEC_CLOCK,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?SND_SOC_CLOCK_OUT);
> + ? ? ? if (err)
> + ? ? ? ? ? ? ? return err;
> +
> + ? ? ? err = snd_soc_dai_set_clkdiv(cpu_dai, EP93XX_I2S_SDIV, sdiv);
> + ? ? ? if (err)
> + ? ? ? ? ? ? ? return err;
> +
> + ? ? ? err = snd_soc_dai_set_clkdiv(cpu_dai, EP93XX_I2S_LRDIV, lrdiv);
> + ? ? ? if (err)
> + ? ? ? ? ? ? ? return err;
> +
> + ? ? ? return 0;
> +}
> +
> +static struct snd_soc_ops snappercl15_ops = {
> + ? ? ? .startup ? ? ? ?= snappercl15_startup,
> + ? ? ? .shutdown ? ? ? = snappercl15_shutdown,
> + ? ? ? .hw_params ? ? ?= snappercl15_hw_params,
> +};
> +
> +static const struct snd_soc_dapm_widget tlv320aic23_dapm_widgets[] = {
> + ? ? ? SND_SOC_DAPM_HP("Headphone Jack", NULL),
> + ? ? ? SND_SOC_DAPM_LINE("Line In", NULL),
> + ? ? ? SND_SOC_DAPM_MIC("Mic Jack", NULL),
> +};
> +
> +static const struct snd_soc_dapm_route audio_map[] = {
> + ? ? ? {"Headphone Jack", NULL, "LHPOUT"},
> + ? ? ? {"Headphone Jack", NULL, "RHPOUT"},
> +
> + ? ? ? {"LLINEIN", NULL, "Line In"},
> + ? ? ? {"RLINEIN", NULL, "Line In"},
> +
> + ? ? ? {"MICIN", NULL, "Mic Jack"},
> +};
> +
> +static int snappercl15_tlv320aic23_init(struct snd_soc_codec *codec)
> +{
> + ? ? ? printk(KERN_INFO "%s - here\n", __FUNCTION__);

Delete this.

> +
> + ? ? ? snd_soc_dapm_new_controls(codec, tlv320aic23_dapm_widgets,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ARRAY_SIZE(tlv320aic23_dapm_widgets));
> +
> + ? ? ? snd_soc_dapm_add_routes(codec, audio_map, ARRAY_SIZE(audio_map));
> +
> + ? ? ? snd_soc_dapm_enable_pin(codec, "Headphone Jack");
> + ? ? ? snd_soc_dapm_enable_pin(codec, "Line In");
> + ? ? ? snd_soc_dapm_enable_pin(codec, "Mic Jack");
> +
> + ? ? ? snd_soc_dapm_sync(codec);
> + ? ? ? return 0;
> +}
> +
> +static struct snd_soc_dai_link snappercl15_dai = {
> + ? ? ? .name ? ? ? ? ? = "tlv320aic23",
> + ? ? ? .stream_name ? ?= "AIC23",
> + ? ? ? .cpu_dai ? ? ? ?= &ep93xx_i2s_dai,
> + ? ? ? .codec_dai ? ? ?= &tlv320aic23_dai,
> + ? ? ? .init ? ? ? ? ? = snappercl15_tlv320aic23_init,
> + ? ? ? .ops ? ? ? ? ? ?= &snappercl15_ops,
> +};
> +
> +static struct snd_soc_card snd_soc_snappercl15 = {
> + ? ? ? .name ? ? ? ? ? = "Snapper CL15",
> + ? ? ? .platform ? ? ? = &ep93xx_soc_platform,
> + ? ? ? .dai_link ? ? ? = &snappercl15_dai,
> + ? ? ? .num_links ? ? ?= 1,
> +};
> +
> +static struct snd_soc_device snappercl15_snd_devdata = {
> + ? ? ? .card ? ? ? ? ? = &snd_soc_snappercl15,
> + ? ? ? .codec_dev ? ? ?= &soc_codec_dev_tlv320aic23,
> +};
> +
> +static struct platform_device *snappercl15_snd_device;
> +
> +static int __init snappercl15_init(void)
> +{
> + ? ? ? int ret;
> +
> + ? ? ? if (!machine_is_snapper_cl15())
> + ? ? ? ? ? ? ? return -ENODEV;
> +
> + ? ? ? snappercl15_snd_device = platform_device_alloc("soc-audio", -1);
> + ? ? ? if (!snappercl15_snd_device)
> + ? ? ? ? ? ? ? return -ENOMEM;
> +
> + ? ? ? platform_set_drvdata(snappercl15_snd_device, &snappercl15_snd_devdata);
> + ? ? ? snappercl15_snd_devdata.dev = &snappercl15_snd_device->dev;
> + ? ? ? ret = platform_device_add(snappercl15_snd_device);
> + ? ? ? if (ret)
> + ? ? ? ? ? ? ? platform_device_put(snappercl15_snd_device);
> +
> + ? ? ? return ret;
> +}
> +
> +static void __exit snappercl15_exit(void)
> +{
> + ? ? ? platform_device_unregister(snappercl15_snd_device);
> +}
> +
> +module_init(snappercl15_init);
> +module_exit(snappercl15_exit);
> +
> +MODULE_AUTHOR("Ryan Mallon <ryan@bluewatersys.com>");
> +MODULE_DESCRIPTION("ALSA SoC Snapper CL15");
> +MODULE_LICENSE("GPL");
> +

-- Chase

[RFC PATCH 1/3] ep93xx i2s driver

[H Hartley Sweeten]

On Monday, May 17, 2010 9:53 PM, Ryan Mallon wrote:
> Add ep93xx i2s audio driver
>
> Signed-off-by: Ryan Mallon <ryan@bluewatersys.com>
> ---
>
> diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
> index b1749bc..f7cb451 100644
> --- a/sound/soc/Kconfig
> +++ b/sound/soc/Kconfig
> @@ -28,6 +28,7 @@ source "sound/soc/atmel/Kconfig"
>  source "sound/soc/au1x/Kconfig"
>  source "sound/soc/blackfin/Kconfig"
>  source "sound/soc/davinci/Kconfig"
> +source "sound/soc/ep93xx/Kconfig"
>  source "sound/soc/fsl/Kconfig"
>  source "sound/soc/imx/Kconfig"
>  source "sound/soc/omap/Kconfig"
> diff --git a/sound/soc/Makefile b/sound/soc/Makefile
> index 1470141..55b711a 100644
> --- a/sound/soc/Makefile
> +++ b/sound/soc/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_SND_SOC) += atmel/
>  obj-$(CONFIG_SND_SOC)        += au1x/
>  obj-$(CONFIG_SND_SOC)        += blackfin/
>  obj-$(CONFIG_SND_SOC)        += davinci/
> +obj-$(CONFIG_SND_SOC)        += ep93xx/
>  obj-$(CONFIG_SND_SOC)        += fsl/
>  obj-$(CONFIG_SND_SOC)   += imx/
>  obj-$(CONFIG_SND_SOC)        += omap/
> diff --git a/sound/soc/ep93xx/Kconfig b/sound/soc/ep93xx/Kconfig
> new file mode 100644
> index 0000000..6af5dd8
> --- /dev/null
> +++ b/sound/soc/ep93xx/Kconfig
> @@ -0,0 +1,15 @@
> +config SND_EP93XX_SOC
> +     tristate "SoC Audio support for the Cirrus Logic EP93xx series"
> +     depends on ARCH_EP93XX && SND_SOC
> +     help
> +       Say Y or M if you want to add support for codecs attached to
> +       the EP93xx I2S interface.
> +
> +config SND_EP93XX_SOC_I2S
> +     tristate
> +
> +config SND_EP93XX_SOC_SNAPPERCL15
> +        tristate "SoC Audio support for Bluewater Systems Snapper CL15 module"
> +        depends on SND_EP93XX_SOC && MACH_SNAPPER_CL15
> +        select SND_EP93XX_SOC_I2S
> +        select SND_SOC_TLV320AIC23

Missing help text?

> diff --git a/sound/soc/ep93xx/Makefile b/sound/soc/ep93xx/Makefile
> new file mode 100644
> index 0000000..272e60f
> --- /dev/null
> +++ b/sound/soc/ep93xx/Makefile
> @@ -0,0 +1,11 @@
> +# EP93xx Platform Support
> +snd-soc-ep93xx-objs                          := ep93xx-pcm.o
> +snd-soc-ep93xx-i2s-objs                              := ep93xx-i2s.o
> +
> +obj-$(CONFIG_SND_EP93XX_SOC)                 += snd-soc-ep93xx.o
> +obj-$(CONFIG_SND_EP93XX_SOC_I2S)             += snd-soc-ep93xx-i2s.o
> +
> +# EP93XX Machine Support
> +snd-soc-snappercl15-objs                     := snappercl15.o
> +
> +obj-$(CONFIG_SND_EP93XX_SOC_SNAPPERCL15)     += snd-soc-snappercl15.o
> diff --git a/sound/soc/ep93xx/ep93xx-i2s.c b/sound/soc/ep93xx/ep93xx-i2s.c
> new file mode 100644
> index 0000000..a81a18d
> --- /dev/null
> +++ b/sound/soc/ep93xx/ep93xx-i2s.c
> @@ -0,0 +1,497 @@
> +/*
> + * linux/sound/soc/ep93xx-i2s.c
> + * EP93xx I2S driver
> + *
> + * Copyright (C) 2008 Ryan Mallon <ryan@bluewatersys.com>
> + *
> + * Based on the original driver by:
> + *   Copyright (C) 2007 Chase Douglas <chasedouglas@gmail>
> + *   Copyright (C) 2006 Lennert Buytenhek <buytenh@wantstofly.org>
> + *
> + * 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.
> + *
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +
> +#include <sound/core.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/initval.h>
> +#include <sound/soc.h>
> +
> +#include <mach/hardware.h>
> +#include <mach/ep93xx-regs.h>
> +#include <mach/dma.h>
> +
> +#include "ep93xx-pcm.h"
> +#include "ep93xx-i2s.h"
> +
> +#define EP93XX_I2S_TXCLKCFG          0x00
> +#define EP93XX_I2S_RXCLKCFG          0x04
> +#define EP93XX_I2S_GLSTS             0x08
> +#define EP93XX_I2S_GLCTRL            0x0C
> +
> +#define EP93XX_I2S_TX0LFT            0x10
> +#define EP93XX_I2S_TX0RT             0x14
> +#define EP93XX_I2S_TX1LFT            0x18
> +#define EP93XX_I2S_TX1RT             0x1C
> +#define EP93XX_I2S_TX2LFT            0x20
> +#define EP93XX_I2S_TX2RT             0x24
> +#define EP93XX_I2S_TXLINCTRLDATA     0x28
> +#define EP93XX_I2S_TXCTRL            0x2C
> +#define EP93XX_I2S_TXWRDLEN          0x30
> +#define EP93XX_I2S_TX0EN             0x34
> +#define EP93XX_I2S_TX1EN             0x38
> +#define EP93XX_I2S_TX2EN             0x3C
> +
> +#define EP93XX_I2S_RX0LFT            0x40
> +#define EP93XX_I2S_RX0RT             0x44
> +#define EP93XX_I2S_RX1LFT            0x48
> +#define EP93XX_I2S_RX1RT             0x4C
> +#define EP93XX_I2S_RX2LFT            0x50
> +#define EP93XX_I2S_RX2RT             0x54
> +
> +#define EP93XX_I2S_RXLINCTRLDATA     0x58
> +#define EP93XX_I2S_RXCTRL            0x5C
> +#define EP93XX_I2S_RXWRDLEN          0x60
> +#define EP93XX_I2S_RX0EN             0x64
> +#define EP93XX_I2S_RX1EN             0x68
> +#define EP93XX_I2S_RX2EN             0x6C
> +
> +#define EP93XX_I2S_WRDLEN_16         (0 << 0)
> +#define EP93XX_I2S_WRDLEN_24         (1 << 0)
> +#define EP93XX_I2S_WRDLEN_32         (2 << 0)
> +
> +#define EP93XX_I2S_LINCTRLDATA_R_JUST        (1 << 2) /* Right justify */
> +
> +#define EP93XX_I2S_CLKCFG_LRS                (1 << 0) /* lrclk polarity */
> +#define EP93XX_I2S_CLKCFG_CKP                (1 << 1) /* Bit clock polarity */
> +#define EP93XX_I2S_CLKCFG_REL                (1 << 2) /*  */
> +#define EP93XX_I2S_CLKCFG_MASTER     (1 << 3) /* Master mode */
> +#define EP93XX_I2S_CLKCFG_NBCG               (1 << 4) /* Not bit clock gating */
> +
> +struct ep93xx_i2s_info {
> +     struct clk                      *i2s_clk;

Comment about the clocks below.

> +     struct ep93xx_pcm_dma_params    *dma_params[2];
> +     struct resource                 *mem;
> +     void __iomem                    *regs;
> +};

Also, I saw Chase Douglas' comment on this struct and the defines.

If they are "only" used in this source file please leave them here.  If there
is not a reason to expose the information globally, keep it private to the
driver.

As far as the name of the struct, I really don't care.

> +
> +static struct ep93xx_pcm_dma_params ep93xx_i2s_pcm_out = {
> +     .name           = "i2s-pcm-out",
> +     .dma_port       = EP93XX_DMA_M2P_PORT_I2S1,
> +};
> +
> +static struct ep93xx_pcm_dma_params ep93xx_i2s_pcm_in = {
> +     .name           = "i2s-pcm-in",
> +     .dma_port       = EP93XX_DMA_M2P_PORT_I2S1,
> +};
> +
> +static inline void ep93xx_i2s_write_reg(struct ep93xx_i2s_info *info,
> +                                     unsigned reg, unsigned val)
> +{
> +     __raw_writel(val, info->regs + reg);
> +}
> +
> +static inline unsigned ep93xx_i2s_read_reg(struct ep93xx_i2s_info *info,
> +                                        unsigned reg)
> +{
> +     return __raw_readl(info->regs + reg);
> +}
> +
> +static inline void ep93xx_i2s_enable(struct ep93xx_i2s_info *info, int enable)
> +{
> +     ep93xx_i2s_write_reg(info, EP93XX_I2S_GLCTRL, enable);
> +}
> +
> +static inline void ep93xx_i2s_enable_fifos(struct ep93xx_i2s_info *info,
> +                                        int enable)

Not sure if this should be 'inline'

> +{
> +     ep93xx_i2s_write_reg(info, EP93XX_I2S_RX0EN, enable);
> +     ep93xx_i2s_write_reg(info, EP93XX_I2S_RX1EN, enable);
> +     ep93xx_i2s_write_reg(info, EP93XX_I2S_RX2EN, enable);
> +     ep93xx_i2s_write_reg(info, EP93XX_I2S_TX0EN, enable);
> +     ep93xx_i2s_write_reg(info, EP93XX_I2S_TX1EN, enable);
> +     ep93xx_i2s_write_reg(info, EP93XX_I2S_TX2EN, enable);
> +}
> +
> +static int ep93xx_i2s_startup(struct snd_pcm_substream *substream,
> +                           struct snd_soc_dai *dai)
> +{
> +     struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +     struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> +     struct ep93xx_i2s_info *info = rtd->dai->cpu_dai->private_data;
> +
> +     clk_enable(info->i2s_clk);

Comment about this below.

> +     cpu_dai->dma_data = info->dma_params[substream->stream];
> +     return 0;
> +}
> +
> +static void ep93xx_i2s_shutdown(struct snd_pcm_substream *substream,
> +                             struct snd_soc_dai *dai)
> +{
> +     struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +     struct ep93xx_i2s_info *info = rtd->dai->cpu_dai->private_data;
> +
> +     clk_disable(info->i2s_clk);

Again, comment about this below.

> +}
> +
> +static int ep93xx_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
> +                           struct snd_soc_dai *dai)
> +{
> +     return 0;
> +}
> +
> +static int ep93xx_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
> +                               unsigned int fmt)
> +{
> +     struct ep93xx_i2s_info *info = cpu_dai->private_data;
> +     unsigned int clk_cfg, lin_ctrl;
> +
> +     clk_cfg  = ep93xx_i2s_read_reg(info, EP93XX_I2S_RXCLKCFG);
> +     lin_ctrl = ep93xx_i2s_read_reg(info, EP93XX_I2S_RXLINCTRLDATA);
> +
> +     switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> +     case SND_SOC_DAIFMT_I2S:
> +             clk_cfg |= EP93XX_I2S_CLKCFG_REL;
> +             lin_ctrl &= ~EP93XX_I2S_LINCTRLDATA_R_JUST;
> +             break;
> +
> +     case SND_SOC_DAIFMT_LEFT_J:
> +             clk_cfg &= ~EP93XX_I2S_CLKCFG_REL;
> +             lin_ctrl &= ~EP93XX_I2S_LINCTRLDATA_R_JUST;
> +             break;
> +
> +     case SND_SOC_DAIFMT_RIGHT_J:
> +             clk_cfg &= ~EP93XX_I2S_CLKCFG_REL;
> +             lin_ctrl |= EP93XX_I2S_LINCTRLDATA_R_JUST;
> +             break;
> +
> +     default:
> +             return -EINVAL;
> +     }
> +
> +     switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> +     case SND_SOC_DAIFMT_CBS_CFS:
> +             /* CPU is master */
> +             clk_cfg |= EP93XX_I2S_CLKCFG_MASTER;
> +             break;
> +
> +     case SND_SOC_DAIFMT_CBM_CFM:
> +             /* Codec is master */
> +             clk_cfg &= ~EP93XX_I2S_CLKCFG_MASTER;
> +             break;
> +
> +     default:
> +             return -EINVAL;
> +     }
> +
> +     switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> +     case SND_SOC_DAIFMT_NB_NF:
> +             /* Negative bit clock, lrclk low on left word */
> +             clk_cfg &= ~(EP93XX_I2S_CLKCFG_CKP | EP93XX_I2S_CLKCFG_REL);
> +             break;
> +
> +     case SND_SOC_DAIFMT_NB_IF:
> +             /* Negative bit clock, lrclk low on right word */
> +             clk_cfg &= ~EP93XX_I2S_CLKCFG_CKP;
> +             clk_cfg |= EP93XX_I2S_CLKCFG_REL;
> +             break;
> +
> +     case SND_SOC_DAIFMT_IB_NF:
> +             /* Positive bit clock, lrclk low on left word */
> +             clk_cfg |= EP93XX_I2S_CLKCFG_CKP;
> +             clk_cfg &= ~EP93XX_I2S_CLKCFG_REL;
> +             break;
> +
> +     case SND_SOC_DAIFMT_IB_IF:
> +             /* Positive bit clock, lrclk low on right word */
> +             clk_cfg |= EP93XX_I2S_CLKCFG_CKP | EP93XX_I2S_CLKCFG_REL;
> +             break;
> +     }
> +
> +     /* Write new register values */
> +     ep93xx_i2s_write_reg(info, EP93XX_I2S_RXCLKCFG, clk_cfg);
> +     ep93xx_i2s_write_reg(info, EP93XX_I2S_TXCLKCFG, clk_cfg);
> +     ep93xx_i2s_write_reg(info, EP93XX_I2S_RXLINCTRLDATA, lin_ctrl);
> +     ep93xx_i2s_write_reg(info, EP93XX_I2S_TXLINCTRLDATA, lin_ctrl);
> +     return 0;
> +}
> +
> +static int ep93xx_i2s_hw_params(struct snd_pcm_substream *substream,
> +                             struct snd_pcm_hw_params *params,
> +                             struct snd_soc_dai *dai)
> +{
> +     struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +     struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> +     struct ep93xx_i2s_info *info = cpu_dai->private_data;
> +     unsigned word_len;
> +
> +     switch (params_format(params)) {
> +     case SNDRV_PCM_FORMAT_S16_LE:
> +             word_len = EP93XX_I2S_WRDLEN_16;
> +             break;
> +
> +     case SNDRV_PCM_FORMAT_S24_LE:
> +             word_len = EP93XX_I2S_WRDLEN_24;
> +             break;
> +
> +     case SNDRV_PCM_FORMAT_S32_LE:
> +             word_len = EP93XX_I2S_WRDLEN_32;
> +             break;
> +
> +     default:
> +             return -EINVAL;
> +     }
> +
> +     if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> +             ep93xx_i2s_write_reg(info, EP93XX_I2S_TXWRDLEN, word_len);
> +     else
> +             ep93xx_i2s_write_reg(info, EP93XX_I2S_TXWRDLEN, word_len);
> +     return 0;
> +}
> +
> +static int ep93xx_i2s_set_sysclk(struct snd_soc_dai *cpu_dai, int clk_id,
> +                              unsigned int freq, int dir)
> +{
> +     struct ep93xx_i2s_info *info = cpu_dai->private_data;
> +
> +     if (dir == SND_SOC_CLOCK_IN || clk_id != 0)
> +             return -EINVAL;
> +
> +     return clk_set_rate(info->i2s_clk, freq);
> +}
> +
> +static int ep93xx_i2s_set_clkdiv(struct snd_soc_dai *cpu_dai, int div_id,
> +                              int div)
> +{
> +     unsigned val;
> +
> +     /*
> +      * The LRDIV (frame clock) and SDIV (bit clock) controls are in the
> +      * EP93XX_SYSCON_I2SCLKDIV register, which is also used by clock.c
> +      * to set the I2S master clock.
> +      *
> +      * FIXME - This functions is potentially racy with the clock api for
> +      * the I2S master clock. Its also ugly modifying the syscon registers
> +      * here.
> +      */
> +     val = __raw_readl( EP93XX_SYSCON_I2SCLKDIV);
> +
> +     switch (div_id) {
> +     case EP93XX_I2S_SDIV:
> +             /* SCLK = MCLK / div */
> +             switch (div) {
> +             case 2:
> +                     val &= ~(0x1 << EP93XX_I2S_SDIV);
> +                     break;
> +
> +             case 4:
> +                     val |= 0x1 << EP93XX_I2S_SDIV;
> +                     break;
> +
> +             default:
> +                     return -EINVAL;
> +             }
> +             break;
> +
> +     case EP93XX_I2S_LRDIV:
> +             /* LRCLK = SCLK / div */
> +             switch (div) {
> +             case 32:
> +                     val &= ~(0x3 << EP93XX_I2S_LRDIV);
> +                     break;
> +
> +             case 64:
> +                     val &= ~(0x3 << EP93XX_I2S_LRDIV);
> +                     val |= 0x1 << EP93XX_I2S_LRDIV;
> +                     break;
> +
> +             case 128:
> +                     val &= ~(0x3 << EP93XX_I2S_LRDIV);
> +                     val |= 0x2 << EP93XX_I2S_LRDIV;
> +                     break;
> +
> +             default:
> +                     return -EINVAL;
> +             }
> +             break;
> +
> +     default:
> +             return -EINVAL;
> +     }
> +
> +     ep93xx_syscon_swlocked_write(val, EP93XX_SYSCON_I2SCLKDIV);
> +     return 0;
> +}

As you noted, this is a bit ugly.

If you create two pseudo clocks in clock.c for the sclk and lrclk this can
all be changed to:

static int ep93xx_i2s_set_clkdiv(struct snd_soc_dai *cpu_dai, int div_id,
                                 int div)
{
        struct ep93xx_i2s_info *info = dai->private_data;
        unsigned long rate;

        switch (div_id) {
        case EP93XX_I2S_SDIV:
                /* SCLK = MCLK / div */
                rate = clk_get_rate(info->i2s_clk) / div;
                return clk_set_rate(info->sclk, rate);

        case EP93XX_I2S_LRDIV:
                /* LRCLK = SCLK / div */
                rate = clk_get_rate(info->sclk) / div;
                return clk_set_rate(info->lrclk, rate);

        default:
                return -EINVAL;
        }
}

If the "parents" of the clocks are set so i2s_clk is the parent of sclk which
is the parent of lrclk then you just need to enable/disable the lrclk to get
the others enabled/disabled.

These pseudo clocks will also have to be part of your struct ep93xx_i2s_info and
will need to the appropriate clk_get's in the probe and clk_put's as needed.

The only tricky part about the pseudo clocks are the set_rate routines so that
changing the MCLK (i2s_clk) propagates the correct "rate" down to the children.
Technically it's not even a big deal since you never really query those rates,
internally the rates will be "correct" based on the new MCLK rate.

I'll propose a patch for clock.c in PATCH 2/3.

> +
> +#ifdef CONFIG_PM
> +static int ep93xx_i2s_suspend(struct snd_soc_dai *dai)
> +{
> +     struct ep93xx_i2s_info *info = dai->private_data;
> +
> +     if (!dai->active)
> +             return;
> +
> +     ep93xx_i2s_enable(info, 0);
> +}
> +
> +static int ep93xx_i2s_resume(struct snd_soc_dai *dai)
> +{
> +     struct ep93xx_i2s_info *info = dai->private_data;
> +
> +     if (!dai->active)
> +             return;
> +
> +     ep93xx_i2s_enable(info, 1);
> +}
> +#else
> +#define ep93xx_i2s_suspend   NULL
> +#define ep93xx_i2s_resume    NULL
> +#endif
> +
> +#define EP93XX_I2S_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
> +                         SNDRV_PCM_FMTBIT_S24_LE | \
> +                         SNDRV_PCM_FMTBIT_S32_LE)
> +
> +static struct snd_soc_dai_ops ep93xx_i2s_dai_ops = {
> +     .startup        = ep93xx_i2s_startup,
> +     .shutdown       = ep93xx_i2s_shutdown,
> +     .trigger        = ep93xx_i2s_trigger,
> +     .hw_params      = ep93xx_i2s_hw_params,
> +     .set_sysclk     = ep93xx_i2s_set_sysclk,
> +     .set_clkdiv     = ep93xx_i2s_set_clkdiv,
> +     .set_fmt        = ep93xx_i2s_set_dai_fmt,
> +};
> +
> +struct snd_soc_dai ep93xx_i2s_dai = {
> +     .name           = "ep93xx-i2s",
> +     .id             = 0,
> +     .suspend        = ep93xx_i2s_suspend,
> +     .resume         = ep93xx_i2s_resume,
> +     .playback       = {
> +             .channels_min   = 2,
> +             .channels_max   = 2,
> +             .rates          = SNDRV_PCM_RATE_8000_48000,
> +             .formats        = EP93XX_I2S_FORMATS,
> +     },
> +     .capture        = {
> +              .channels_min  = 2,
> +              .channels_max  = 2,
> +              .rates         = SNDRV_PCM_RATE_8000_48000,
> +              .formats       = EP93XX_I2S_FORMATS,
> +     },
> +     .ops            = &ep93xx_i2s_dai_ops,
> +};
> +EXPORT_SYMBOL_GPL(ep93xx_i2s_dai);
> +
> +static int ep93xx_i2s_probe(struct platform_device *pdev)
> +{
> +     struct ep93xx_i2s_info *info;
> +     struct resource *res;
> +     int err;
> +
> +     info = kzalloc(sizeof(struct ep93xx_i2s_info), GFP_KERNEL);
> +     if (!info) {
> +             err = -ENOMEM;
> +             goto fail;
> +     }
> +
> +     ep93xx_i2s_dai.dev = &pdev->dev;
> +     ep93xx_i2s_dai.private_data = info;
> +
> +     info->dma_params[SNDRV_PCM_STREAM_PLAYBACK] = &ep93xx_i2s_pcm_out;
> +     info->dma_params[SNDRV_PCM_STREAM_CAPTURE]  = &ep93xx_i2s_pcm_in;
> +
> +     err = snd_soc_register_dai(&ep93xx_i2s_dai);
> +     if (err)
> +             goto fail_free_info;
> +
> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +     if (!res) {
> +             err = -ENODEV;
> +             goto fail_unregister_dai;
> +     }
> +
> +     info->mem = request_mem_region(res->start, resource_size(res),
> +                                    pdev->name);
> +     if (!info->mem) {
> +             err = -EBUSY;
> +             goto fail_free_info;
> +     }
> +
> +     info->regs = ioremap(info->mem->start, resource_size(info->mem));
> +     if (!info->regs) {
> +             err = -ENXIO;
> +             goto fail_release_mem;
> +     }
> +
> +     info->i2s_clk = clk_get(&pdev->dev, NULL);
> +     if (IS_ERR(info->i2s_clk)) {
> +             err = PTR_ERR(info->i2s_clk);
> +             goto fail_unmap_mem;
> +     }

See comment above.

> +
> +     ep93xx_i2s_enable_fifos(info, 1);
> +     ep93xx_i2s_enable(info, 1);
> +     return 0;
> +
> +fail_unmap_mem:
> +     iounmap(info->regs);
> +fail_release_mem:
> +     release_mem_region(info->mem->start, resource_size(info->mem));
> +fail_unregister_dai:
> +     snd_soc_unregister_dai(&ep93xx_i2s_dai);
> +fail_free_info:
> +     kfree(info);
> +fail:
> +     return err;
> +}
> +
> +static int __devexit ep93xx_i2s_remove(struct platform_device *pdev)
> +{
> +     struct ep93xx_i2s_info *info = ep93xx_i2s_dai.private_data;
> +
> +     ep93xx_i2s_enable_fifos(info, 0);
> +     ep93xx_i2s_enable(info, 0);
> +
> +     clk_put(info->i2s_clk);

See comment above.

> +     iounmap(info->regs);
> +     release_mem_region(info->mem->start, resource_size(info->mem));
> +     snd_soc_unregister_dai(&ep93xx_i2s_dai);
> +     kfree(info);
> +     return 0;
> +}
> +
> +static struct platform_driver ep93xx_i2s_driver = {
> +     .probe  = ep93xx_i2s_probe,
> +     .remove = __devexit_p(ep93xx_i2s_remove),
> +     .driver = {
> +             .name   = "ep93xx-i2s",
> +             .owner  = THIS_MODULE,
> +     },
> +};
> +
> +static int __init ep93xx_i2s_init(void)
> +{
> +     return platform_driver_register(&ep93xx_i2s_driver);
> +}
> +
> +static void __exit ep93xx_i2s_exit(void)
> +{
> +     platform_driver_unregister(&ep93xx_i2s_driver);
> +}
> +
> +module_init(ep93xx_i2s_init);
> +module_exit(ep93xx_i2s_exit);
> +
> +MODULE_ALIAS("platform:ep93xx-i2s");
> +MODULE_AUTHOR("Ryan Mallon <ryan@bluewatersys.com>");
> +MODULE_DESCRIPTION("EP93XX I2S driver");
> +MODULE_LICENSE("GPL");
> diff --git a/sound/soc/ep93xx/ep93xx-i2s.h b/sound/soc/ep93xx/ep93xx-i2s.h
> new file mode 100644
> index 0000000..37e6370
> --- /dev/null
> +++ b/sound/soc/ep93xx/ep93xx-i2s.h
> @@ -0,0 +1,13 @@
> +#ifndef _EP93XX_SND_SOC_I2S_H
> +#define _EP93XX_SND_SOC_I2S_H
> +
> +/*
> + * These are the offsets for the LRDIV and SDIV fields in the syscon i2sclkdiv
> + * register.
> + */
> +#define EP93XX_I2S_LRDIV     17
> +#define EP93XX_I2S_SDIV              16

Using the pseudo clocks, these defines can go away.

But, since they are also used as the "div_id" in ep93xx_i2s_set_clkdiv they should
be changed to something more logical (i.e. 0, 1 not 16, 17).

> +
> +extern struct snd_soc_dai ep93xx_i2s_dai;
> +
> +#endif /* _EP93XX_SND_SOC_I2S_H */
> diff --git a/sound/soc/ep93xx/ep93xx-pcm.c b/sound/soc/ep93xx/ep93xx-pcm.c
> new file mode 100644
> index 0000000..c071b5c
> --- /dev/null
> +++ b/sound/soc/ep93xx/ep93xx-pcm.c

Is this pcm driver generic enough to work with an AC97 driver?  Looking at
some of the other soc drivers it appears there are different pcm drivers for
different codec interfaces.  If it's not completely generic please rename this
file as appropriate.

> @@ -0,0 +1,321 @@
> +/*
> + * linux/sound/arm/ep93xx-pcm.c - EP93xx ALSA PCM interface
> + *
> + * Copyright (C) 2006 Lennert Buytenhek <buytenh@wantstofly.org>
> + * Copyright (C) 2006 Applied Data Systems
> + *
> + * Rewritten for the SoC audio subsystem (Based on PXA2xx code):
> + *   Copyright (c) 2008 Ryan Mallon <ryan@bluewatersys.com>
> + *
> + * 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.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <linux/dma-mapping.h>
> +
> +#include <sound/core.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +
> +#include <mach/dma.h>
> +#include <mach/hardware.h>
> +#include <mach/ep93xx-regs.h>
> +
> +#include "ep93xx-pcm.h"
> +
> +static const struct snd_pcm_hardware ep93xx_pcm_hardware = {
> +     .info                   = (SNDRV_PCM_INFO_MMAP          |
> +                                SNDRV_PCM_INFO_MMAP_VALID    |
> +                                SNDRV_PCM_INFO_INTERLEAVED   |
> +                                SNDRV_PCM_INFO_BLOCK_TRANSFER),
> +
> +     .rates                  = SNDRV_PCM_RATE_8000_48000,
> +     .rate_min               = SNDRV_PCM_RATE_8000,
> +     .rate_max               = SNDRV_PCM_RATE_48000,
> +
> +     .formats                = (SNDRV_PCM_FMTBIT_S16_LE |
> +                                SNDRV_PCM_FMTBIT_S24_LE |
> +                                SNDRV_PCM_FMTBIT_S32_LE),
> +
> +     .buffer_bytes_max       = 131072,
> +     .period_bytes_min       = 32,
> +     .period_bytes_max       = 32768,
> +     .periods_min            = 1,
> +     .periods_max            = 32,
> +     .fifo_size              = 32,
> +};
> +
> +struct ep93xx_runtime_data
> +{
> +     struct ep93xx_dma_m2p_client    cl;
> +     struct ep93xx_pcm_dma_params    *params;
> +     int                             pointer_bytes;
> +     struct tasklet_struct           period_tasklet;
> +     int                             periods;
> +     struct ep93xx_dma_buffer        buf[32];
> +};
> +
> +static void ep93xx_pcm_period_elapsed(unsigned long data)
> +{
> +     struct snd_pcm_substream *substream = (struct snd_pcm_substream *)data;
> +     snd_pcm_period_elapsed(substream);
> +}
> +
> +static void ep93xx_pcm_buffer_started(void *cookie,
> +                                   struct ep93xx_dma_buffer *buf)
> +{
> +}
> +
> +static void ep93xx_pcm_buffer_finished(void *cookie,
> +                                    struct ep93xx_dma_buffer *buf,
> +                                    int bytes, int error)
> +{
> +     struct snd_pcm_substream *substream = cookie;
> +     struct ep93xx_runtime_data *rtd = substream->runtime->private_data;
> +
> +     if (buf == rtd->buf + rtd->periods - 1)
> +             rtd->pointer_bytes = 0;
> +     else
> +             rtd->pointer_bytes += buf->size;
> +
> +     if (!error) {
> +             ep93xx_dma_m2p_submit_recursive(&rtd->cl, buf);
> +             tasklet_schedule(&rtd->period_tasklet);
> +     } else {
> +             snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
> +     }
> +}
> +
> +static int ep93xx_pcm_open(struct snd_pcm_substream *substream)
> +{
> +     struct snd_soc_pcm_runtime *soc_rtd = substream->private_data;
> +     struct ep93xx_pcm_dma_params *dma_params = soc_rtd->dai->cpu_dai->dma_data;
> +     struct ep93xx_runtime_data *rtd;
> +     int ret;
> +
> +     snd_soc_set_runtime_hwparams(substream, &ep93xx_pcm_hardware);
> +
> +     rtd = kmalloc(sizeof(*rtd), GFP_KERNEL);
> +     if (!rtd)
> +             return -ENOMEM;
> +
> +     memset(&rtd->period_tasklet, 0, sizeof(rtd->period_tasklet));
> +     rtd->period_tasklet.func = ep93xx_pcm_period_elapsed;
> +     rtd->period_tasklet.data = (unsigned long)substream;
> +
> +     rtd->cl.name = dma_params->name;
> +     rtd->cl.flags = dma_params->dma_port | EP93XX_DMA_M2P_IGNORE_ERROR |
> +             ((substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
> +              EP93XX_DMA_M2P_TX : EP93XX_DMA_M2P_RX);
> +     rtd->cl.cookie = substream;
> +     rtd->cl.buffer_started = ep93xx_pcm_buffer_started;
> +     rtd->cl.buffer_finished = ep93xx_pcm_buffer_finished;
> +     ret = ep93xx_dma_m2p_client_register(&rtd->cl);
> +     if (ret < 0) {
> +             kfree(rtd);
> +             return ret;
> +     }
> +
> +     substream->runtime->private_data = rtd;
> +     return 0;
> +}
> +
> +static int ep93xx_pcm_close(struct snd_pcm_substream *substream)
> +{
> +     struct ep93xx_runtime_data *rtd = substream->runtime->private_data;
> +
> +     ep93xx_dma_m2p_client_unregister(&rtd->cl);
> +     kfree(rtd);
> +     return 0;
> +}
> +
> +static int ep93xx_pcm_hw_params(struct snd_pcm_substream *substream,
> +                             struct snd_pcm_hw_params *params)
> +{
> +     struct snd_pcm_runtime *runtime = substream->runtime;
> +     struct ep93xx_runtime_data *rtd = runtime->private_data;
> +     size_t totsize = params_buffer_bytes(params);
> +     size_t period = params_period_bytes(params);
> +     int i;
> +
> +     snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
> +     runtime->dma_bytes = totsize;
> +
> +     rtd->periods = (totsize + period - 1) / period;
> +     for (i = 0; i < rtd->periods; i++) {
> +             rtd->buf[i].bus_addr = runtime->dma_addr + (i * period);
> +             rtd->buf[i].size = period;
> +             if ((i + 1) * period > totsize)
> +                     rtd->buf[i].size = totsize - (i * period);
> +     }
> +
> +     return 0;
> +}
> +
> +static int ep93xx_pcm_hw_free(struct snd_pcm_substream *substream)
> +{
> +     snd_pcm_set_runtime_buffer(substream, NULL);
> +     return 0;
> +}
> +
> +static int ep93xx_pcm_prepare(struct snd_pcm_substream *substream)
> +{
> +     return 0;
> +}
> +
> +static int ep93xx_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
> +{
> +     struct ep93xx_runtime_data *rtd = substream->runtime->private_data;
> +     int ret;
> +     int i;
> +
> +     ret = 0;
> +     switch (cmd) {
> +     case SNDRV_PCM_TRIGGER_START:
> +     case SNDRV_PCM_TRIGGER_RESUME:
> +     case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> +             rtd->pointer_bytes = 0;
> +             for (i = 0; i < rtd->periods; i++)
> +                     ep93xx_dma_m2p_submit(&rtd->cl, rtd->buf + i);
> +             break;
> +
> +     case SNDRV_PCM_TRIGGER_STOP:
> +     case SNDRV_PCM_TRIGGER_SUSPEND:
> +     case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> +             ep93xx_dma_m2p_flush(&rtd->cl);
> +             break;
> +
> +     default:
> +             ret = -EINVAL;
> +             break;
> +     }
> +
> +     return ret;
> +}
> +
> +static snd_pcm_uframes_t ep93xx_pcm_pointer(struct snd_pcm_substream *substream)
> +{
> +     struct snd_pcm_runtime *runtime = substream->runtime;
> +     struct ep93xx_runtime_data *rtd = substream->runtime->private_data;
> +
> +     /* FIXME: implement this with sub-period granularity */
> +     return bytes_to_frames(runtime, rtd->pointer_bytes);
> +}
> +
> +static int ep93xx_pcm_mmap(struct snd_pcm_substream *substream,
> +                        struct vm_area_struct *vma)
> +{
> +     struct snd_pcm_runtime *runtime = substream->runtime;
> +
> +     return dma_mmap_writecombine(substream->pcm->card->dev, vma,
> +                                  runtime->dma_area,
> +                                  runtime->dma_addr,
> +                                  runtime->dma_bytes);
> +}
> +
> +static struct snd_pcm_ops ep93xx_pcm_ops = {
> +     .open           = ep93xx_pcm_open,
> +     .close          = ep93xx_pcm_close,
> +     .ioctl          = snd_pcm_lib_ioctl,
> +     .hw_params      = ep93xx_pcm_hw_params,
> +     .hw_free        = ep93xx_pcm_hw_free,
> +     .prepare        = ep93xx_pcm_prepare,
> +     .trigger        = ep93xx_pcm_trigger,
> +     .pointer        = ep93xx_pcm_pointer,
> +     .mmap           = ep93xx_pcm_mmap,
> +};
> +
> +static int ep93xx_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
> +{
> +     struct snd_pcm_substream *substream = pcm->streams[stream].substream;
> +     struct snd_dma_buffer *buf = &substream->dma_buffer;
> +     size_t size = ep93xx_pcm_hardware.buffer_bytes_max;
> +
> +     buf->dev.type = SNDRV_DMA_TYPE_DEV;
> +     buf->dev.dev = pcm->card->dev;
> +     buf->private_data = NULL;
> +     buf->area = dma_alloc_writecombine(pcm->card->dev, size,
> +                                        &buf->addr, GFP_KERNEL);
> +     buf->bytes = size;
> +
> +     return (buf->area == NULL) ? -ENOMEM : 0;
> +}
> +
> +static void ep93xx_pcm_free_dma_buffers(struct snd_pcm *pcm)
> +{
> +     struct snd_pcm_substream *substream;
> +     struct snd_dma_buffer *buf;
> +     int stream;
> +
> +     for (stream = 0; stream < 2; stream++) {
> +             substream = pcm->streams[stream].substream;
> +             if (!substream)
> +                     continue;
> +
> +             buf = &substream->dma_buffer;
> +             if (!buf->area)
> +                     continue;
> +
> +             dma_free_writecombine(pcm->card->dev, buf->bytes, buf->area,
> +                                   buf->addr);
> +             buf->area = NULL;
> +     }
> +}
> +
> +static u64 ep93xx_pcm_dmamask = 0xffffffff;
> +
> +static int ep93xx_pcm_new(struct snd_card *card, struct snd_soc_dai *dai,
> +                       struct snd_pcm *pcm)
> +{
> +     int ret = 0;
> +
> +     if (!card->dev->dma_mask)
> +             card->dev->dma_mask = &ep93xx_pcm_dmamask;
> +     if (!card->dev->coherent_dma_mask)
> +             card->dev->coherent_dma_mask = 0xffffffff;
> +
> +     if (dai->playback.channels_min) {
> +             ret = ep93xx_pcm_preallocate_dma_buffer(pcm,
> +                                     SNDRV_PCM_STREAM_PLAYBACK);
> +             if (ret)
> +                     return ret;
> +
> +             ret = ep93xx_pcm_preallocate_dma_buffer(pcm,
> +                                     SNDRV_PCM_STREAM_CAPTURE);
> +             if (ret)
> +                     return ret;
> +     }
> +
> +     return 0;
> +}
> +
> +struct snd_soc_platform ep93xx_soc_platform = {
> +     .name           = "ep93xx-audio",
> +     .pcm_ops        = &ep93xx_pcm_ops,
> +     .pcm_new        = &ep93xx_pcm_new,
> +     .pcm_free       = &ep93xx_pcm_free_dma_buffers,
> +};
> +EXPORT_SYMBOL_GPL(ep93xx_soc_platform);
> +
> +static int __init ep93xx_soc_platform_init(void)
> +{
> +     return snd_soc_register_platform(&ep93xx_soc_platform);
> +}
> +
> +static void __exit ep93xx_soc_platform_exit(void)
> +{
> +     snd_soc_unregister_platform(&ep93xx_soc_platform);
> +}
> +
> +module_init(ep93xx_soc_platform_init);
> +module_exit(ep93xx_soc_platform_exit);
> +
> +MODULE_AUTHOR("Lennert Buytenhek <buytenh@wantstofly.org>");
> +MODULE_DESCRIPTION("EP93xx ALSA PCM interface");
> +MODULE_LICENSE("GPL");
> diff --git a/sound/soc/ep93xx/ep93xx-pcm.h b/sound/soc/ep93xx/ep93xx-pcm.h
> new file mode 100644
> index 0000000..4ffdd3f
> --- /dev/null
> +++ b/sound/soc/ep93xx/ep93xx-pcm.h
> @@ -0,0 +1,22 @@
> +/*
> + * sound/soc/ep93xx/ep93xx-pcm.h - EP93xx ALSA PCM interface
> + *
> + * Copyright (C) 2006 Lennert Buytenhek <buytenh@wantstofly.org>
> + * Copyright (C) 2006 Applied Data Systems
> + *
> + * 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 _EP93XX_SND_SOC_PCM_H
> +#define _EP93XX_SND_SOC_PCM_H
> +
> +struct ep93xx_pcm_dma_params {
> +     char    *name;
> +     int     dma_port;
> +};
> +
> +extern struct snd_soc_platform ep93xx_soc_platform;
> +
> +#endif /* _EP93XX_SND_SOC_PCM_H */


Ugh... Sound drivers make my head hurt...

Regards,
Hartley

[RFC PATCH 0/3] ep93xx i2s audio

[Mark Brown]

On Tue, May 18, 2010 at 04:45:27PM +1200, Ryan Mallon wrote:

> The following patch series adds support for i2s audio on the ep93xx,
> with the Snapper CL15 used as an example implementation. The driver is
> basically working, but still incomplete and needs some tidy up before it
> will be accepted for mainline. I am posting the patches to get feedback
> from others as to how well it works on their boards and ideas to improve
> the code.

Please do always CC the maintainers for relevant subsystems and also the
relevant mailing lists when posting patches, even if just as RFC.  In
this case you've done neither.

> The following issues still exist:
>  - The setting of SDIV/LRDIV is ugly since it requires modifying one of
> the clock registers from the audio driver.

Clock API perhaps?

>  - Audio capture has not been tested
>  - Sometimes playback (ie aplay) results in noise. Killing the process
> and trying again will result in correct playback.
>  - Some empty functions can probably be removed

Yes.

>  - Not sure if I2SONSSP/AC97 should be a function argument, or a Kconfig
> option (opinions please).

[RFC PATCH 2/3] ep93xx i2s core support

[H Hartley Sweeten]

On Monday, May 17, 2010 9:54 PM, Ryan Mallon wrote:
> Add ep93xx core support for i2s audio
>
> Signed-off-by: Ryan Mallon <ryan@bluewatersys.com>
> ---
>
> diff --git a/arch/arm/mach-ep93xx/clock.c b/arch/arm/mach-ep93xx/clock.c
> index 5f80092..df88233 100644
> --- a/arch/arm/mach-ep93xx/clock.c
> +++ b/arch/arm/mach-ep93xx/clock.c
> @@ -108,6 +108,16 @@ static struct clk clk_video = {
>  	.set_rate	= set_div_rate,
>  };
>  
> +static struct clk clk_i2s = {
> +	.sw_locked	= 1,
> +	.enable_reg	= EP93XX_SYSCON_I2SCLKDIV,
> +	.enable_mask	= (EP93XX_SYSCON_CLKDIV_ENABLE  |
> +			   EP93XX_SYSCON_I2SCLKDIV_SENA |
> +			   EP93XX_SYSCON_I2SCLKDIV_SPOL |
> +			   EP93XX_SYSCON_I2SCLKDIV_ORIDE),
> +	.set_rate	= set_div_rate,
> +};

This clock should not be enabling the slave clocks.

Not sure about the SPOL and ORIDE bits... Maybe they should be
part of the platform_data (see below) for the i2s driver.

> +
>  /* DMA Clocks */
>  static struct clk clk_m2p0 = {
>  	.parent		= &clk_h,
> @@ -186,6 +196,7 @@ static struct clk_lookup clocks[] = {
>  	INIT_CK("ep93xx-ohci",		NULL,		&clk_usb_host),
>  	INIT_CK("ep93xx-keypad",	NULL,		&clk_keypad),
>  	INIT_CK("ep93xx-fb",		NULL,		&clk_video),
> +	INIT_CK("ep93xx-i2s",		NULL,		&clk_i2s),
>  	INIT_CK(NULL,			"pwm_clk",	&clk_pwm),
>  	INIT_CK(NULL,			"m2p0",		&clk_m2p0),
>  	INIT_CK(NULL,			"m2p1",		&clk_m2p1),


For the pseudo clocks mentioned in PATCH 1/3, how about this?

static struct clk clk_i2s = {
	.sw_locked	= 1,
	.enable_reg	= EP93XX_SYSCON_I2SCLKDIV,
	.enable_mask	= EP93XX_SYSCON_CLKDIV_ENABLE,
	.set_rate	= set_i2s_rate,
};
static struct clk clk_sclk = {
	.sw_locked	= 1,
	.parent		= &clk_i2s,
	.enable_reg	= EP93XX_SYSCON_I2SCLKDIV,
	.enable_mask	= EP93XX_SYSCON_I2SCLKDIV_SENA,
	.set_rate	= set_sclk_rate,
};
static struct clk clk_lrclk = {
	.sw_locked	= 1,
	.parent		= &clk_sclk,
	.enable_reg	= EP93XX_SYSCON_I2SCLKDIV,
	.enable_mask	= EP93XX_SYSCON_I2SCLKDIV_SENA,
	.set_rate	= set_lrclk_rate,
};

	INIT_CK("ep93xx-i2s",		NULL,		&clk_i2s),
	INIT_CK("ep93xx-i2s",		"sclk",		&clk_sclk),
	INIT_CK("ep93xx-i2s",		"lrclk",	&clk_lrclk),

static int set_i2s_rate(struct clk *clk, unsigned long rate)
{
	u32 val;
	int ret;

	/* First set the I2S master clock to the desired rate */
	ret = set_div_rate(clk, rate);
	if (ret)
		return ret;

	/* Now update the child clock rates based on the current setting */
	val = __raw_readl(clk->enable_reg);

	if (val & EP93XX_SYSCON_I2SCLKDIV_SDIV)
		clk_sclk.rate = clk->rate / 4;
	else
		clk_sclk.rate = clk->rate / 2;

	val &= EP93XX_SYSCON_I2SCLKDIV_LRDIV_MASK;
	switch (val >> EP93XX_SYSCON_I2SCLKDIV_LRDIV_SHIFT) {
	case 0:
		clk_lrclk.rate = clk_sclk.rate / 32;
		break;
	case 1:
		clk_lrclk.rate = clk_sclk.rate / 64;
		break;
	case 2:
		clk_lrclk.rate = clk_sclk.rate / 128;
		break;
	default:
		return -EINVAL;
	}

	return 0;
}

static int set_sclk_rate(struct clk *clk, unsigned long rate)
{
	unsigned long i2s_rate = clk_get_rate(clk->parent);
	u32 val;

	val = __raw_readl(clk->enable_reg);

	if (rate == i2s_rate / 2)
		val &= ~EP93XX_SYSCON_I2SCLKDIV_SDIV;
	else if (rate == i2s_rate / 4)
		val |= EP93XX_SYSCON_I2SCLKDIV_SDIV;
	else
		return -EINVAL;

	ep93xx_syscon_swlocked_write(val, clk->enable_reg);
	clk->rate = rate;
	return 0;
}

static int set_lrclk_rate(struct clk *clk, unsigned long rate)
{
	unsigned long sclk_rate = clk_get_rate(clk->parent);
	u32 val;

	val = __raw_readl(clk->enable_reg);
	val &= ~EP93XX_SYSCON_I2SCLKDIV_LRDIV_MASK;

	if (rate == sclk_rate / 32)
		val |= 0x0 << EP93XX_SYSCON_I2SCLKDIV_LRDIV_SHIFT;
	else if (rate == sclk_rate / 64)
		val |= 0x1 << EP93XX_SYSCON_I2SCLKDIV_LRDIV_SHIFT;
	else if (rate == sclk_rate / 128)
		val |= 0x2 << EP93XX_SYSCON_I2SCLKDIV_LRDIV_SHIFT;
	else
		return -EINVAL;

	ep93xx_syscon_swlocked_write(val, clk->enable_reg);
	clk->rate = rate;
	return 0;
}

I know, it's not a patch ;-), but hopefully you get the idea.

I'm not sure about the INIT_CK definitions for the sclk and lrclk.
They might need a NULL dev_id and just get matched based on the con_id.

> diff --git a/arch/arm/mach-ep93xx/core.c b/arch/arm/mach-ep93xx/core.c
> index 90fb591..cceeac2 100644
> --- a/arch/arm/mach-ep93xx/core.c
> +++ b/arch/arm/mach-ep93xx/core.c
> @@ -617,6 +617,37 @@ void ep93xx_keypad_release_gpio(struct platform_device *pdev)
>  }
>  EXPORT_SYMBOL(ep93xx_keypad_release_gpio);
>  
> +/*************************************************************************
> + * EP93xx I2S audio peripheral handling
> + *************************************************************************/
> +static struct resource ep93xx_i2s_resource[] = {
> +	{
> +		.start	= EP93XX_I2S_PHYS_BASE,
> +		.end	= EP93XX_I2S_PHYS_BASE + 0x100 - 1,

The last I2S register is at + 0x6C, so this should be + 0x70 - 1

> +		.flags	= IORESOURCE_MEM,
> +	},

What about the interrupt?  Is there any reason not to make the driver use it?

> +};
> +
> +static struct platform_device ep93xx_i2s_device = {
> +	.name		= "ep93xx-i2s",
> +	.id		= -1,
> +	.num_resources	= ARRAY_SIZE(ep93xx_i2s_resource),
> +	.resource	= ep93xx_i2s_resource,

Should the dma_mask and coherent_dma_mask be defined here instead of in the
driver?

> +};
> +
> +void __init ep93xx_register_i2s(unsigned pins)
> +{
> +	if (pins != EP93XX_SYSCON_DEVCFG_I2SONSSP &&
> +	    pins != EP93XX_SYSCON_DEVCFG_I2SONAC97) {
> +		pr_err("Invalid I2S pin configuration - not registering\n");
> +		return;
> +	}
> +
> +	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_I2SONAC97 |
> +				 EP93XX_SYSCON_DEVCFG_I2SONAC97);
> +	ep93xx_devcfg_set_bits(pins);
> +	platform_device_register(&ep93xx_i2s_device);
> +}

Hmm.. Not sure if the registration function is the correct place to mess with
the Syscon bits.  Since the driver "could" be built as a module it might be
better to just register the device here.

And, since the i2s port 1 use egpio[5:4] and i2s port 2 use egpio[13,6] you
probably should add an ep93xx_acquire_i2s and ep93xx_release_i2s function.

This probably needs some sort of platform_data that can be passed to the
driver to establish the configuration.  Then the acquire function can setup
the proper configuration at runtime. 

>  
>  extern void ep93xx_gpio_init(void);
>  
> diff --git a/arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h b/arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h
> index 93e2ecc..074d99b 100644
> --- a/arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h
> +++ b/arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h
> @@ -93,6 +93,7 @@
>  /* APB peripherals */
>  #define EP93XX_TIMER_BASE		EP93XX_APB_IOMEM(0x00010000)
>  
> +#define EP93XX_I2S_PHYS_BASE		EP93XX_APB_PHYS(0x00020000)
>  #define EP93XX_I2S_BASE			EP93XX_APB_IOMEM(0x00020000)
>  
>  #define EP93XX_SECURITY_BASE		EP93XX_APB_IOMEM(0x00030000)
> @@ -193,6 +194,10 @@
>  #define EP93XX_SYSCON_CLKDIV_ESEL	(1<<14)
>  #define EP93XX_SYSCON_CLKDIV_PSEL	(1<<13)
>  #define EP93XX_SYSCON_CLKDIV_PDIV_SHIFT	8
> +#define EP93XX_SYSCON_I2SCLKDIV		EP93XX_SYSCON_REG(0x8c)
> +#define EP93XX_SYSCON_I2SCLKDIV_SENA	(1<<31)
> +#define EP93XX_SYSCON_I2SCLKDIV_ORIDE   (1<<29)
> +#define EP93XX_SYSCON_I2SCLKDIV_SPOL	(1<<19)

Please reorder these a bit.

Put the *_REG define after the *_VIDCLKDIV one (since they share the *_CLKDIV_*
defines).  Then put the bit defines in decending order like the reset of the
file (i.e. the _I2SCLKDIV_* specific ones come right before the *_CLKDIV_* ones).

>  #define EP93XX_SYSCON_KEYTCHCLKDIV	EP93XX_SYSCON_REG(0x90)
>  #define EP93XX_SYSCON_KEYTCHCLKDIV_TSEN	(1<<31)
>  #define EP93XX_SYSCON_KEYTCHCLKDIV_ADIV	(1<<16)
> diff --git a/arch/arm/mach-ep93xx/include/mach/platform.h b/arch/arm/mach-ep93xx/include/mach/platform.h
> index c6dc14d..9252adf 100644
> --- a/arch/arm/mach-ep93xx/include/mach/platform.h
> +++ b/arch/arm/mach-ep93xx/include/mach/platform.h
> @@ -43,6 +43,7 @@ void ep93xx_pwm_release_gpio(struct platform_device *pdev);
>  void ep93xx_register_keypad(struct ep93xx_keypad_platform_data *data);
>  int ep93xx_keypad_acquire_gpio(struct platform_device *pdev);
>  void ep93xx_keypad_release_gpio(struct platform_device *pdev);
> +void ep93xx_register_i2s(unsigned pins);
>  
>  void ep93xx_init_devices(void);
>  extern struct sys_timer ep93xx_timer;

I need to figure out a way to test this on the EDB9307A board.  I'll try
to get more feedback to you later.

Regards,
Hartley

[RFC PATCH 1/3] ep93xx i2s driver

[Mark Brown]

On Tue, May 18, 2010 at 04:53:28PM +1200, Ryan Mallon wrote:

> index 0000000..6af5dd8
> --- /dev/null
> +++ b/sound/soc/ep93xx/Kconfig
> @@ -0,0 +1,15 @@
> +config SND_EP93XX_SOC
> +	tristate "SoC Audio support for the Cirrus Logic EP93xx series"
> +	depends on ARCH_EP93XX && SND_SOC
> +	help
> +	  Say Y or M if you want to add support for codecs attached to
> +	  the EP93xx I2S interface.
> +
> +config SND_EP93XX_SOC_I2S
> +	tristate
> +
> +config SND_EP93XX_SOC_SNAPPERCL15
> +        tristate "SoC Audio support for Bluewater Systems Snapper CL15 module"
> +        depends on SND_EP93XX_SOC && MACH_SNAPPER_CL15
> +        select SND_EP93XX_SOC_I2S
> +        select SND_SOC_TLV320AIC23

Please split at least the machine driver out into a separate patch.
Probably a good idea to split the DMA driver for legibility.

> +static int ep93xx_i2s_startup(struct snd_pcm_substream *substream,
> +			      struct snd_soc_dai *dai)
> +{	
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> +	struct ep93xx_i2s_info *info = rtd->dai->cpu_dai->private_data;
> +	
> +	clk_enable(info->i2s_clk);
> +	cpu_dai->dma_data = info->dma_params[substream->stream];	

You'll want to rebase against the for-2.6.36 branch of

 git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound-2.6.git

> +static int ep93xx_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
> +			      struct snd_soc_dai *dai)
> +{
> +	return 0;
> +}

Remove empty functins like this.

> +static int ep93xx_i2s_set_clkdiv(struct snd_soc_dai *cpu_dai, int div_id,
> +				 int div)
> +{
> +	unsigned val;
> +
> +	/*
> +	 * The LRDIV (frame clock) and SDIV (bit clock) controls are in the
> +	 * EP93XX_SYSCON_I2SCLKDIV register, which is also used by clock.c
> +	 * to set the I2S master clock.
> +	 *
> +	 * FIXME - This functions is potentially racy with the clock api for
> +	 * the I2S master clock. Its also ugly modifying the syscon registers
> +	 * here.
> +	 */

Exposing LRCLK and BCLK via the clock API seems like a sensible way of
resolving this.  You probably also want to consider making the
configuration of the LRCLK and BCLK automatic, at least by default,
since the configuration can be inferred from the hw_params for most
setups.  This makes machine drivers easier to write.

> +#ifdef CONFIG_PM
> +static int ep93xx_i2s_suspend(struct snd_soc_dai *dai)
> +{
> +	struct ep93xx_i2s_info *info = dai->private_data;
> +
> +	if (!dai->active)
> +		return;
> +	
> +	ep93xx_i2s_enable(info, 0);

This doesn't seem to tie in with the way i2s_enable() is called
elsewhere - it's not conditional on the DAI being active, it's done
unconditionally in probe().  I'd expect to see some handling of the
clocks here, though.

> +static int ep93xx_pcm_prepare(struct snd_pcm_substream *substream)
> +{
> +	return 0;
> +}

Remove this.

> +static int ep93xx_pcm_mmap(struct snd_pcm_substream *substream,
> +			   struct vm_area_struct *vma)
> +{
> +	struct snd_pcm_runtime *runtime = substream->runtime;
> +
> +	return dma_mmap_writecombine(substream->pcm->card->dev, vma,
> +				     runtime->dma_area,
> +				     runtime->dma_addr,
> +				     runtime->dma_bytes);
> +}

We really ought to provide a helper for this, it's cookie cuttered far
too often.

[RFC PATCH 3/3] ep93xx i2s snapper cl15 support

[H Hartley Sweeten]

On Monday, May 17, 2010 9:55 PM, Ryan Mallon wrote:
> Added support for i2s audio on Snapper CL15
>
> Signed-off-by: Ryan Mallon <ryan@bluewatersys.com>
> ---
> 
> diff --git a/arch/arm/mach-ep93xx/snappercl15.c b/arch/arm/mach-ep93xx/snappercl15.c
> index 51134b0..d4ca5cc 100644
> --- a/arch/arm/mach-ep93xx/snappercl15.c
> +++ b/arch/arm/mach-ep93xx/snappercl15.c
> @@ -157,6 +157,7 @@ static void __init snappercl15_init_machine(void)
>  	ep93xx_register_i2c(&snappercl15_i2c_gpio_data, snappercl15_i2c_data,
>  			    ARRAY_SIZE(snappercl15_i2c_data));
>  	ep93xx_register_fb(&snappercl15_fb_info);
> +	ep93xx_register_i2s(EP93XX_SYSCON_DEVCFG_I2SONAC97);
>  	platform_device_register(&snappercl15_nand_device);
>  }
>  
> diff --git a/sound/soc/ep93xx/snappercl15.c b/sound/soc/ep93xx/snappercl15.c
> new file mode 100644
> index 0000000..3fae86e
> --- /dev/null
> +++ b/sound/soc/ep93xx/snappercl15.c
> @@ -0,0 +1,186 @@
> +/*
> + * snappercl15.c -- SoC audio for Bluewater Systems Snapper CL15 module
> + *
> + * Copyright (C) 2008 Bluewater Systems Ltd
> + * Author: Ryan Mallon <ryan@bluewatersys.com>
> + *
> + *  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/platform_device.h>
> +#include <sound/core.h>
> +#include <sound/pcm.h>
> +#include <sound/soc.h>
> +#include <sound/soc-dapm.h>
> +
> +#include <asm/mach-types.h>
> +#include <mach/hardware.h>
> +
> +#include "../codecs/tlv320aic23.h"
> +#include "ep93xx-pcm.h"
> +#include "ep93xx-i2s.h"
> +
> +//#define CODEC_CLOCK 11289600
> +#define CODEC_CLOCK 5644800
> +
> +static int snappercl15_startup(struct snd_pcm_substream *substream)
> +{
> +	return 0;
> +}
> +
> +static void snappercl15_shutdown(struct snd_pcm_substream *substream)
> +{
> +}
> +
> +static int snappercl15_hw_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 *codec_dai = rtd->dai->codec_dai;
> +	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> +	int sdiv, lrdiv, err;
> +
> +	switch (params_rate(params)) {
> +	case 44100: 
> +		sdiv = 4;
> +		lrdiv = 64;
> +		break;
> +		
> +	case 22050: 
> +		sdiv = 4; 
> +		lrdiv = 128; 
> +		break;
> +

These don't look right (unless I'm missing something).

CODEC_CLOCK = 5644800
SCLK = MCLK / sdiv (4) = 1411200
LRCLK = SCLK / lrdiv (64) = 22050 for 44100
LRCLK = SCLK / lrdiv (128) = 11025 for 22050

> +	default:
> +		return -EINVAL;
> +	}
> +
> +	err = snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_I2S |
+				  SND_SOC_DAIFMT_NB_IF |
+				  SND_SOC_DAIFMT_CBS_CFS);
+
+	err = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_I2S | 
+				  SND_SOC_DAIFMT_NB_IF |		  
+				  SND_SOC_DAIFMT_CBS_CFS);
+	if (err)
+		return err;
+
+	err = snd_soc_dai_set_sysclk(codec_dai, 0, CODEC_CLOCK, 
+				     SND_SOC_CLOCK_IN);
+	if (err)
+		return err;
+
+	err = snd_soc_dai_set_sysclk(cpu_dai, 0, CODEC_CLOCK, 
+				     SND_SOC_CLOCK_OUT);
+	if (err)
+		return err;
+
+	err = snd_soc_dai_set_clkdiv(cpu_dai, EP93XX_I2S_SDIV, sdiv);
+	if (err)
+		return err;
+
+	err = snd_soc_dai_set_clkdiv(cpu_dai, EP93XX_I2S_LRDIV, lrdiv);
+	if (err)
+		return err;
+	
+	return 0;
+}
+
+static struct snd_soc_ops snappercl15_ops = {
+	.startup	= snappercl15_startup,
+	.shutdown	= snappercl15_shutdown,
+	.hw_params	= snappercl15_hw_params,
+};
+
+static const struct snd_soc_dapm_widget tlv320aic23_dapm_widgets[] = {
+	SND_SOC_DAPM_HP("Headphone Jack", NULL),
+	SND_SOC_DAPM_LINE("Line In", NULL),
+	SND_SOC_DAPM_MIC("Mic Jack", NULL),
+};
+
+static const struct snd_soc_dapm_route audio_map[] = {
+	{"Headphone Jack", NULL, "LHPOUT"},
+	{"Headphone Jack", NULL, "RHPOUT"},
+
+	{"LLINEIN", NULL, "Line In"},
+	{"RLINEIN", NULL, "Line In"},
+
+	{"MICIN", NULL, "Mic Jack"},
+};
+
+static int snappercl15_tlv320aic23_init(struct snd_soc_codec *codec)
+{
+	printk(KERN_INFO "%s - here\n", __FUNCTION__);
+
+	snd_soc_dapm_new_controls(codec, tlv320aic23_dapm_widgets,
+				  ARRAY_SIZE(tlv320aic23_dapm_widgets));
+
+	snd_soc_dapm_add_routes(codec, audio_map, ARRAY_SIZE(audio_map));
+	
+	snd_soc_dapm_enable_pin(codec, "Headphone Jack");
+	snd_soc_dapm_enable_pin(codec, "Line In");
+	snd_soc_dapm_enable_pin(codec, "Mic Jack");
+
+	snd_soc_dapm_sync(codec);
+	return 0;
+}
+
+static struct snd_soc_dai_link snappercl15_dai = {
+	.name		= "tlv320aic23",
+	.stream_name	= "AIC23",
+	.cpu_dai	= &ep93xx_i2s_dai,
+	.codec_dai	= &tlv320aic23_dai,
+	.init		= snappercl15_tlv320aic23_init,
+	.ops		= &snappercl15_ops,
+};
+
+static struct snd_soc_card snd_soc_snappercl15 = {
+	.name		= "Snapper CL15",
+	.platform	= &ep93xx_soc_platform,
+	.dai_link	= &snappercl15_dai,
+	.num_links	= 1,
+};
+
+static struct snd_soc_device snappercl15_snd_devdata = {
+	.card		= &snd_soc_snappercl15,
+	.codec_dev	= &soc_codec_dev_tlv320aic23,
+};
+
+static struct platform_device *snappercl15_snd_device;
+
+static int __init snappercl15_init(void)
+{
+	int ret;
+
+	if (!machine_is_snapper_cl15())
+		return -ENODEV;
+
+	snappercl15_snd_device = platform_device_alloc("soc-audio", -1);
+	if (!snappercl15_snd_device)
+		return -ENOMEM;
+	
+	platform_set_drvdata(snappercl15_snd_device, &snappercl15_snd_devdata);
+	snappercl15_snd_devdata.dev = &snappercl15_snd_device->dev;
+	ret = platform_device_add(snappercl15_snd_device);
+	if (ret)
+		platform_device_put(snappercl15_snd_device);
+
+	return ret;
+}
+
+static void __exit snappercl15_exit(void)
+{
+	platform_device_unregister(snappercl15_snd_device);
+}
+
+module_init(snappercl15_init);
+module_exit(snappercl15_exit);
+
+MODULE_AUTHOR("Ryan Mallon <ryan@bluewatersys.com>");
+MODULE_DESCRIPTION("ALSA SoC Snapper CL15");
+MODULE_LICENSE("GPL");
+

Regards,
Hartley

[RFC PATCH 1/3] ep93xx i2s driver

[Mark Brown]

On Tue, May 18, 2010 at 08:45:41AM -0400, Chase Douglas wrote:

Please cut text which is not relevant from your replies, it makes it
*much* easier to read them - it's hard to spot new text while deleting
lots of old text, and it scales much more nicely onto mobile devices
too.

> On Tue, May 18, 2010 at 12:53 AM, Ryan Mallon <ryan@bluewatersys.com> wrote:
> > Add ep93xx i2s audio driver

> > +# EP93xx Platform Support
> > +snd-soc-ep93xx-objs ? ? ? ? ? ? ? ? ? ? ? ? ? ?:= ep93xx-pcm.o
> > +snd-soc-ep93xx-i2s-objs ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?:= ep93xx-i2s.o
> > +
> > +obj-$(CONFIG_SND_EP93XX_SOC) ? ? ? ? ? ? ? ? ? += snd-soc-ep93xx.o
> > +obj-$(CONFIG_SND_EP93XX_SOC_I2S) ? ? ? ? ? ? ? += snd-soc-ep93xx-i2s.o

> Any reason not to just do:

> obj-$(CONFIG_SND_EP93XX_SOC) ? ? ? ? ? ? ? ? ? += ep93xx-pcm.o

> instead of indirection through a separate .o file? I'm no Makefile
> master, so there may be a real reason I'm unaware of.

This renaming of the object is idiomatic for ASoC.

> > + ? ? ? struct clk ? ? ? ? ? ? ? ? ? ? ?*i2s_clk;
> > + ? ? ? struct ep93xx_pcm_dma_params ? ?*dma_params[2];
> > + ? ? ? struct resource ? ? ? ? ? ? ? ? *mem;
> > + ? ? ? void __iomem ? ? ? ? ? ? ? ? ? ?*regs;
> > +};

> These defines and struct definitions should be in a header file,
> probably ep93xx-i2s.h.

Having them in the source file is fine from an ASoC point of view if
they're only referenced here but either way is fine.

> > + ? ? ? if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> > + ? ? ? ? ? ? ? ep93xx_i2s_write_reg(info, EP93XX_I2S_TXWRDLEN, word_len);
> > + ? ? ? else
> > + ? ? ? ? ? ? ? ep93xx_i2s_write_reg(info, EP93XX_I2S_TXWRDLEN, word_len);

> The conditional is unnecessary here.

...or possibly (from the register name) indicates that the second one
should be _RXWRDLEN?  If the configuration is the same for both TX and
RX then the DAI should have the symmetric_rates flag set.

[RFC PATCH 3/3] ep93xx i2s snapper cl15 support

[Mark Brown]

On Tue, May 18, 2010 at 04:55:12PM +1200, Ryan Mallon wrote:

> +//#define CODEC_CLOCK 11289600
> +#define CODEC_CLOCK 5644800

Hrm?

> +
> +static int snappercl15_startup(struct snd_pcm_substream *substream)
> +{
> +	return 0;
> +}

Remove this and all other empty functions.

> +static int snappercl15_tlv320aic23_init(struct snd_soc_codec *codec)
> +{
> +	printk(KERN_INFO "%s - here\n", __FUNCTION__);

Debug code, should probably be removed or at least make pr_debug().

> +	snd_soc_dapm_enable_pin(codec, "Headphone Jack");
> +	snd_soc_dapm_enable_pin(codec, "Line In");
> +	snd_soc_dapm_enable_pin(codec, "Mic Jack");

No need to enable pins, this is the default condition.  You could mark
any unused pins on the CODEC as _nc_pin() though.

> +	snd_soc_dapm_sync(codec);

The core should take care of this for you (so many machines need it that
it was pulled in).

[RFC PATCH 0/3] ep93xx i2s audio

[Mark Brown]

On Tue, May 18, 2010 at 04:45:27PM +1200, Ryan Mallon wrote:

>  - Sometimes playback (ie aplay) results in noise. Killing the process
> and trying again will result in correct playback.

What does the output data look like in a scope, is it bit shifted
relative to the clocks for example?

[RFC PATCH 0/3] ep93xx i2s audio

[Ryan Mallon]

Mark Brown wrote:
> On Tue, May 18, 2010 at 04:45:27PM +1200, Ryan Mallon wrote:
> 
>>  - Sometimes playback (ie aplay) results in noise. Killing the process
>> and trying again will result in correct playback.
> 
> What does the output data look like in a scope, is it bit shifted
> relative to the clocks for example?

I haven't looked into this too closely yet, but on a scope it is just
complete noise. It doesn't appear to be shifted or anything like that.

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934

[RFC PATCH 0/3] ep93xx i2s audio

[Ryan Mallon]

Mark Brown wrote:
> On Tue, May 18, 2010 at 04:45:27PM +1200, Ryan Mallon wrote:
> 
>> The following patch series adds support for i2s audio on the ep93xx,
>> with the Snapper CL15 used as an example implementation. The driver is
>> basically working, but still incomplete and needs some tidy up before it
>> will be accepted for mainline. I am posting the patches to get feedback
>> from others as to how well it works on their boards and ideas to improve
>> the code.
> 
> Please do always CC the maintainers for relevant subsystems and also the
> relevant mailing lists when posting patches, even if just as RFC.  In
> this case you've done neither.

Sorry about this, I was mostly getting this out to a bunch of people who
had expressed interested in it. I should have CC'ed the alsa people
though. Thanks for your comments.

>> The following issues still exist:
>>  - The setting of SDIV/LRDIV is ugly since it requires modifying one of
>> the clock registers from the audio driver.
> 
> Clock API perhaps?

Yes, Hartley has suggested the same thing.

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934

[RFC PATCH 1/3] ep93xx i2s driver

[Ryan Mallon]

Chase Douglas wrote:
> On Tue, May 18, 2010 at 12:53 AM, Ryan Mallon <ryan@bluewatersys.com> wrote:
>> Add ep93xx i2s audio driver
>>
>> Signed-off-by: Ryan Mallon <ryan@bluewatersys.com>
>> ---

>> diff --git a/sound/soc/ep93xx/Kconfig b/sound/soc/ep93xx/Kconfig
>> new file mode 100644
>> index 0000000..6af5dd8
>> --- /dev/null
>> +++ b/sound/soc/ep93xx/Kconfig
>> @@ -0,0 +1,15 @@
>> +config SND_EP93XX_SOC
>> +       tristate "SoC Audio support for the Cirrus Logic EP93xx series"
>> +       depends on ARCH_EP93XX && SND_SOC
>> +       help
>> +         Say Y or M if you want to add support for codecs attached to
>> +         the EP93xx I2S interface.
>> +
>> +config SND_EP93XX_SOC_I2S
>> +       tristate
>> +
>> +config SND_EP93XX_SOC_SNAPPERCL15
>> +        tristate "SoC Audio support for Bluewater Systems Snapper CL15 module"
>> +        depends on SND_EP93XX_SOC && MACH_SNAPPER_CL15
>> +        select SND_EP93XX_SOC_I2S
>> +        select SND_SOC_TLV320AIC23
> 
> This last config should be in patch 3/3 instead of this patch.

Yeah, I generated the patches in a bit of a hurry :-)

>> diff --git a/sound/soc/ep93xx/Makefile b/sound/soc/ep93xx/Makefile
>> new file mode 100644
>> index 0000000..272e60f
>> --- /dev/null
>> +++ b/sound/soc/ep93xx/Makefile
>> @@ -0,0 +1,11 @@
>> +# EP93xx Platform Support
>> +snd-soc-ep93xx-objs                            := ep93xx-pcm.o
>> +snd-soc-ep93xx-i2s-objs                                := ep93xx-i2s.o
>> +
>> +obj-$(CONFIG_SND_EP93XX_SOC)                   += snd-soc-ep93xx.o
>> +obj-$(CONFIG_SND_EP93XX_SOC_I2S)               += snd-soc-ep93xx-i2s.o
> 
> Any reason not to just do:
> 
> obj-$(CONFIG_SND_EP93XX_SOC)                   += ep93xx-pcm.o
> 
> instead of indirection through a separate .o file? I'm no Makefile
> master, so there may be a real reason I'm unaware of.

The alsa soc object files are all done this way so that they get
prefixed with snd-soc-

>> + *
>> + * Based on the original driver by:
>> + *   Copyright (C) 2007 Chase Douglas <chasedouglas@gmail>
> 
> Please use my full email "chasedouglas at gmail.com".

Will do, thanks.

>> +#define EP93XX_I2S_CLKCFG_LRS          (1 << 0) /* lrclk polarity */
>> +#define EP93XX_I2S_CLKCFG_CKP          (1 << 1) /* Bit clock polarity */
>> +#define EP93XX_I2S_CLKCFG_REL          (1 << 2) /*  */
>> +#define EP93XX_I2S_CLKCFG_MASTER       (1 << 3) /* Master mode */
>> +#define EP93XX_I2S_CLKCFG_NBCG         (1 << 4) /* Not bit clock gating */
>> +
>> +struct ep93xx_i2s_info {
>> +       struct clk                      *i2s_clk;
>> +       struct ep93xx_pcm_dma_params    *dma_params[2];
>> +       struct resource                 *mem;
>> +       void __iomem                    *regs;
>> +};
> 
> These defines and struct definitions should be in a header file,
> probably ep93xx-i2s.h.

I would much rather leave them here since they are only required by this
file. The ep93xx-i2s.h header should only include the public interface
to the i2s driver.

> I think the struct name should be something like ep93xx_i2s_priv.
> *_info is very vague, but *_priv tells us it's passed around as an
> opaque structure filled with device specific data.

I think info is fine, it should be relatively clear what it is used for.

>> +
>> +static int ep93xx_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
>> +                             struct snd_soc_dai *dai)
>> +{
>> +       return 0;
>> +}
> 
> Unless there's a platform or cpu trigger function, this function is
> unnecesssary. See soc_pcm_trigger in sound/soc/soc-core.c.

Fixed.

>> +
>> +       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>> +               ep93xx_i2s_write_reg(info, EP93XX_I2S_TXWRDLEN, word_len);
>> +       else
>> +               ep93xx_i2s_write_reg(info, EP93XX_I2S_TXWRDLEN, word_len);
> 
> The conditional is unnecessary here.

Oops, this is a cut and paste bug. The else case should write to the
EP93XX_I2S_RXWRDLEN register instead. However, the EP93xx users' guide
says that if the tx and rx channels are both in master mode then
txclkcfg is used for both and the word length for both must be the same.
 Possibly we should just write the word length for both here.

>> +static int ep93xx_i2s_set_clkdiv(struct snd_soc_dai *cpu_dai, int div_id,
>> +                                int div)
>> +{
>> +       unsigned val;
>> +
>> +       /*
>> +        * The LRDIV (frame clock) and SDIV (bit clock) controls are in the
>> +        * EP93XX_SYSCON_I2SCLKDIV register, which is also used by clock.c
>> +        * to set the I2S master clock.
>> +        *
>> +        * FIXME - This functions is potentially racy with the clock api for
>> +        * the I2S master clock. Its also ugly modifying the syscon registers
>> +        * here.
> 
> Yeah, this is ugly, but if there's a potential race condition can we
> put a mutex in there?

The problem is that it races with code inside
arch/arm/mach-ep93xx/clock.c so it would need to be an exported mutex,
which is just as ugly. I think I will end up going with Hartley's/Mark's
idea to use the clock api for this.

>> +
>> +#define EP93XX_I2S_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
>> +                           SNDRV_PCM_FMTBIT_S24_LE | \
>> +                           SNDRV_PCM_FMTBIT_S32_LE)
> 
> I'd either move this to be just above its usage, or in a header file.
> Right here it's an island of its own.

Okay, moved it down a few lines so it is just aboce ep93xx_i2s_dai.

>> diff --git a/sound/soc/ep93xx/ep93xx-i2s.h b/sound/soc/ep93xx/ep93xx-i2s.h
>> new file mode 100644
>> index 0000000..37e6370
>> --- /dev/null
>> +++ b/sound/soc/ep93xx/ep93xx-i2s.h
> 
> It's my opinion that every file, unless really really trivial, should
> have a license header. I suggest adding one here.

Okay, will fix.

>> +
>> +struct ep93xx_runtime_data
>> +{
>> +       struct ep93xx_dma_m2p_client    cl;
>> +       struct ep93xx_pcm_dma_params    *params;
>> +       int                             pointer_bytes;
>> +       struct tasklet_struct           period_tasklet;
>> +       int                             periods;
>> +       struct ep93xx_dma_buffer        buf[32];
>> +};
> 
> I'd rather this be named something like "ep93xx_pcm_priv".

Other drivers are using runtime_data, I think it is more descriptive
than priv.

>> +
>> +module_init(ep93xx_soc_platform_init);
>> +module_exit(ep93xx_soc_platform_exit);
>> +
>> +MODULE_AUTHOR("Lennert Buytenhek <buytenh@wantstofly.org>");
> Lennert may be the original author, but if you're proposing this for
> upstream you probably want to put yourself here. That is, unless
> you've chatted with Lennert about this. It seems presumptuous to get
> code in the kernel and point people to someone else when it breaks :).

Changed it to me, and left Lennert's original copyright in the comment
at the top of the file.

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934

[RFC PATCH 2/3] ep93xx i2s core support

[Ryan Mallon]

Chase Douglas wrote:
> On Tue, May 18, 2010 at 12:54 AM, Ryan Mallon <ryan@bluewatersys.com> wrote:
>> Add ep93xx core support for i2s audio
>>
>> Signed-off-by: Ryan Mallon <ryan@bluewatersys.com>
>> ---
>>
>> +
>> +       ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_I2SONAC97 |
>> +                                EP93XX_SYSCON_DEVCFG_I2SONAC97);
> 
> I think one of the above should be *_I2SONSSP?

Yup, stupid cut and paste error :-). Fixed.

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934

[RFC PATCH 1/3] ep93xx i2s driver

[Ryan Mallon]

H Hartley Sweeten wrote:
> On Monday, May 17, 2010 9:53 PM, Ryan Mallon wrote:
>> Add ep93xx i2s audio driver
>>
>> Signed-off-by: Ryan Mallon <ryan@bluewatersys.com>
>> ---
>>

>> +config SND_EP93XX_SOC_SNAPPERCL15
>> +        tristate "SoC Audio support for Bluewater Systems Snapper CL15 module"
>> +        depends on SND_EP93XX_SOC && MACH_SNAPPER_CL15
>> +        select SND_EP93XX_SOC_I2S
>> +        select SND_SOC_TLV320AIC23
> 
> Missing help text?

Thanks. Fixed.

>> +     struct ep93xx_pcm_dma_params    *dma_params[2];
>> +     struct resource                 *mem;
>> +     void __iomem                    *regs;
>> +};
> 
> Also, I saw Chase Douglas' comment on this struct and the defines.
> 
> If they are "only" used in this source file please leave them here.  If there
> is not a reason to expose the information globally, keep it private to the
> driver.

Yes, I'm also inclined to leave this all here since it keeps the file
self contained.

>> +static inline void ep93xx_i2s_enable_fifos(struct ep93xx_i2s_info *info,
>> +                                        int enable)
> 
> Not sure if this should be 'inline'

Removed.

>> +
>> +     ep93xx_syscon_swlocked_write(val, EP93XX_SYSCON_I2SCLKDIV);
>> +     return 0;
>> +}
> 
> As you noted, this is a bit ugly.
> 
> If you create two pseudo clocks in clock.c for the sclk and lrclk this can
> all be changed to:

I'm going to try and rework this a little. I think the pseudo clock
approach is the way to go. As Mark pointed out, the clkdiv function can
probably be removed and the sdiv/lrdiv calculation done as part of
ep93xx_i2s_hw_params.

>> diff --git a/sound/soc/ep93xx/ep93xx-pcm.c b/sound/soc/ep93xx/ep93xx-pcm.c
>> new file mode 100644
>> index 0000000..c071b5c
>> --- /dev/null
>> +++ b/sound/soc/ep93xx/ep93xx-pcm.c
> 
> Is this pcm driver generic enough to work with an AC97 driver?  Looking at
> some of the other soc drivers it appears there are different pcm drivers for
> different codec interfaces.  If it's not completely generic please rename this
> file as appropriate.

I don't actually know, I think it is. The PCM driver is originally
written by Lennert and the only changes I have really made are to update
it for the alsa soc interface. IIRC, Lennert was using it with an ac97
driver.

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934