linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ASoC drivers for the Freescale MPC8610 SoC
@ 2007-12-20  0:03 Timur Tabi
  2007-12-20  4:06 ` Olof Johansson
                   ` (4 more replies)
  0 siblings, 5 replies; 89+ messages in thread
From: Timur Tabi @ 2007-12-20  0:03 UTC (permalink / raw)
  To: linuxppc-dev, alsa-devel; +Cc: Timur Tabi

This patch adds ALSA SoC device drivers for the Freescale MPC8610 SoC
and the MPC8610-HPCD reference board.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

This patch applies on top of
git://git.kernel.org/pub/scm/linux/kernel/git/perex/alsa.git.

 Documentation/powerpc/booting-without-of.txt |   40 ++
 arch/powerpc/boot/dts/mpc8610_hpcd.dts       |  107 ++++
 arch/powerpc/configs/mpc8610_hpcd_defconfig  |  171 ++++++-
 arch/powerpc/platforms/86xx/mpc8610_hpcd.c   |   18 +
 sound/soc/Kconfig                            |    1 +
 sound/soc/Makefile                           |    2 +-
 sound/soc/fsl/Kconfig                        |   21 +
 sound/soc/fsl/Makefile                       |    6 +
 sound/soc/fsl/fsl_dma.c                      |  819 ++++++++++++++++++++++++++
 sound/soc/fsl/fsl_dma.h                      |  149 +++++
 sound/soc/fsl/fsl_ssi.c                      |  614 +++++++++++++++++++
 sound/soc/fsl/fsl_ssi.h                      |  224 +++++++
 sound/soc/fsl/mpc8610_hpcd.c                 |  621 +++++++++++++++++++
 13 files changed, 2789 insertions(+), 4 deletions(-)
 create mode 100644 sound/soc/fsl/Kconfig
 create mode 100644 sound/soc/fsl/Makefile
 create mode 100644 sound/soc/fsl/fsl_dma.c
 create mode 100644 sound/soc/fsl/fsl_dma.h
 create mode 100644 sound/soc/fsl/fsl_ssi.c
 create mode 100644 sound/soc/fsl/fsl_ssi.h
 create mode 100644 sound/soc/fsl/mpc8610_hpcd.c

diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index e9a3cb1..826af91 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -53,6 +53,7 @@ Table of Contents
       j) CFI or JEDEC memory-mapped NOR flash
       k) Global Utilities Block
       l) Xilinx IP cores
+      p) Freescale Synchronous Serial Interface
 
   VII - Specifying interrupt information for devices
     1) interrupts property
@@ -2514,6 +2515,45 @@ platforms are moved over to use the flattened-device-tree model.
       Requred properties:
        - current-speed : Baud rate of uartlite
 
+    p) Freescale Synchronous Serial Interface
+
+       The SSI is a serial device that communicates with audio codecs.  It can
+       be programmed in AC97, I2S, left-justified, or right-justified modes.
+
+       Required properties:
+       - compatible	  : compatible list, containing "fsl,ssi"
+       - cell-index	  : the SSI, <0> = SSI1, <1> = SSI2, and so on
+       - reg		  : offset and length of the register set for the device
+       - interrupts	  : <a b> where a is the interrupt number and b is a
+                            field that represents an encoding of the sense and
+			    level information for the interrupt.  This should be
+			    encoded based on the information in section 2)
+			    depending on the type of interrupt controller you
+			    have.
+       - interrupt-parent : the phandle for the interrupt controller that
+                            services interrupts for this device.
+       - fsl,mode	  : the operating mode for the SSI interface
+			    "i2s-slave" - I2S mode, SSI is clock slave
+			    "i2s-master" - I2S mode, SSI is clock master
+			    "lj-slave" - left-justified mode, SSI is clock slave
+			    "lj-master" - l.j. mode, SSI is clock master
+			    "rj-slave" - right-justified mode, SSI is clock slave
+			    "rj-master" - r.j., SSI is clock master
+			    "ac97-slave" - AC97 mode, SSI is clock slave
+			    "ac97-master" - AC97 mode, SSI is clock master
+
+       Optional properties:
+       - codec		  : child node that defines an audio codec connected
+		 	    to this SSI
+
+       Child 'codec' node required properties:
+       - compatible	  : compatible list, contains the name of the codec
+
+       Child 'codec' node optional properties:
+       - bus-frequency	  : The frequency of the input clock, which typically
+                            comes from an on-board dedicated oscillator.
+
+
    More devices will be defined as this spec matures.
 
 VII - Specifying interrupt information for devices
diff --git a/arch/powerpc/boot/dts/mpc8610_hpcd.dts b/arch/powerpc/boot/dts/mpc8610_hpcd.dts
index 966edf1..4f12ede 100644
--- a/arch/powerpc/boot/dts/mpc8610_hpcd.dts
+++ b/arch/powerpc/boot/dts/mpc8610_hpcd.dts
@@ -103,6 +103,113 @@
 			reg = <e0000 1000>;
 			fsl,has-rstcr;
 		};
+
+		ssi@16000 {
+			compatible = "fsl,ssi";
+			cell-index = <0>;
+			reg = <16000 100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <3e 2>;
+			fsl,mode = "i2s-slave";
+			codec {
+				compatible = "cirrus,cs4270";
+				/* MCLK source is a stand-alone oscillator */
+				bus-frequency = <bb8000>;
+			};
+		};
+
+		ssi@16100 {
+			compatible = "fsl,ssi";
+			cell-index = <1>;
+			reg = <16100 100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <3f 2>;
+		};
+
+                dma@21300 {
+                        #address-cells = <1>;
+                        #size-cells = <1>;
+                        compatible = "fsl,mpc8610-dma", "fsl,mpc8540-dma";
+                        cell-index = <0>;
+                        reg = <21300 4>; /* DMA general status register */
+                        ranges = <0 21100 200>;
+
+                        dma-channel@0 {
+				compatible = "fsl,mpc8610-dma-channel",
+					"fsl,mpc8540-dma-channel";
+				cell-index = <0>;
+				reg = <0 80>;
+				interrupt-parent = <&mpic>;
+				interrupts = <14 2>;
+                        };
+                        dma-channel@1 {
+				compatible = "fsl,mpc8610-dma-channel",
+					"fsl,mpc8540-dma-channel";
+				cell-index = <1>;
+				reg = <80 80>;
+				interrupt-parent = <&mpic>;
+				interrupts = <15 2>;
+                        };
+                        dma-channel@2 {
+				compatible = "fsl,mpc8610-dma-channel",
+					"fsl,mpc8540-dma-channel";
+				cell-index = <2>;
+				reg = <100 80>;
+				interrupt-parent = <&mpic>;
+				interrupts = <16 2>;
+                        };
+                        dma-channel@3 {
+				compatible = "fsl,mpc8610-dma-channel",
+					"fsl,mpc8540-dma-channel";
+				cell-index = <3>;
+				reg = <180 80>;
+				interrupt-parent = <&mpic>;
+				interrupts = <17 2>;
+                        };
+                };
+
+                dma@c300 {
+                        #address-cells = <1>;
+                        #size-cells = <1>;
+                        compatible = "fsl,mpc8610-dma", "fsl,mpc8540-dma";
+                        cell-index = <1>;
+                        reg = <c300 4>; /* DMA general status register */
+                        ranges = <0 c100 200>;
+
+                        dma-channel@0 {
+				compatible = "fsl,mpc8610-dma-channel",
+					"fsl,mpc8540-dma-channel";
+				cell-index = <0>;
+				reg = <0 80>;
+				interrupt-parent = <&mpic>;
+				interrupts = <3c 2>;
+                        };
+                        dma-channel@1 {
+				compatible = "fsl,mpc8610-dma-channel",
+					"fsl,mpc8540-dma-channel";
+				cell-index = <1>;
+				reg = <80 80>;
+				interrupt-parent = <&mpic>;
+				interrupts = <3d 2>;
+                        };
+                        dma-channel@2 {
+				compatible = "fsl,mpc8610-dma-channel",
+					"fsl,mpc8540-dma-channel";
+				cell-index = <2>;
+				reg = <100 80>;
+				interrupt-parent = <&mpic>;
+				interrupts = <3e 2>;
+                        };
+                        dma-channel@3 {
+				compatible = "fsl,mpc8610-dma-channel",
+					"fsl,mpc8540-dma-channel";
+				cell-index = <3>;
+				reg = <180 80>;
+				interrupt-parent = <&mpic>;
+				interrupts = <3f 2>;
+                        };
+                };
+
 	};
 
 	pci@e0008000 {
diff --git a/arch/powerpc/configs/mpc8610_hpcd_defconfig b/arch/powerpc/configs/mpc8610_hpcd_defconfig
index 0483211..fb68886 100644
--- a/arch/powerpc/configs/mpc8610_hpcd_defconfig
+++ b/arch/powerpc/configs/mpc8610_hpcd_defconfig
@@ -684,7 +684,7 @@ CONFIG_SERIAL_8250_RSA=y
 CONFIG_SERIAL_CORE=y
 CONFIG_SERIAL_CORE_CONSOLE=y
 # CONFIG_SERIAL_JSM is not set
-CONFIG_SERIAL_OF_PLATFORM=y
+# CONFIG_SERIAL_OF_PLATFORM is not set
 CONFIG_UNIX98_PTYS=y
 # CONFIG_LEGACY_PTYS is not set
 # CONFIG_IPMI_HANDLER is not set
@@ -699,7 +699,60 @@ CONFIG_UNIX98_PTYS=y
 # CONFIG_RAW_DRIVER is not set
 # CONFIG_TCG_TPM is not set
 CONFIG_DEVPORT=y
-# CONFIG_I2C is not set
+CONFIG_I2C=y
+CONFIG_I2C_BOARDINFO=y
+# CONFIG_I2C_CHARDEV is not set
+
+#
+# I2C Algorithms
+#
+# CONFIG_I2C_ALGOBIT is not set
+# CONFIG_I2C_ALGOPCF is not set
+# CONFIG_I2C_ALGOPCA is not set
+
+#
+# I2C Hardware Bus support
+#
+# CONFIG_I2C_ALI1535 is not set
+# CONFIG_I2C_ALI1563 is not set
+# CONFIG_I2C_ALI15X3 is not set
+# CONFIG_I2C_AMD756 is not set
+# CONFIG_I2C_AMD8111 is not set
+# CONFIG_I2C_I801 is not set
+# CONFIG_I2C_I810 is not set
+# CONFIG_I2C_PIIX4 is not set
+CONFIG_I2C_MPC=y
+# CONFIG_I2C_NFORCE2 is not set
+# CONFIG_I2C_OCORES is not set
+# CONFIG_I2C_PARPORT_LIGHT is not set
+# CONFIG_I2C_PROSAVAGE is not set
+# CONFIG_I2C_SAVAGE4 is not set
+# CONFIG_I2C_SIMTEC is not set
+# CONFIG_I2C_SIS5595 is not set
+# CONFIG_I2C_SIS630 is not set
+# CONFIG_I2C_SIS96X is not set
+# CONFIG_I2C_TAOS_EVM is not set
+# CONFIG_I2C_VIA is not set
+# CONFIG_I2C_VIAPRO is not set
+# CONFIG_I2C_VOODOO3 is not set
+
+#
+# Miscellaneous I2C Chip support
+#
+# CONFIG_SENSORS_DS1337 is not set
+# CONFIG_SENSORS_DS1374 is not set
+# CONFIG_DS1682 is not set
+# CONFIG_SENSORS_EEPROM is not set
+# CONFIG_SENSORS_PCF8574 is not set
+# CONFIG_SENSORS_PCA9539 is not set
+# CONFIG_SENSORS_PCF8591 is not set
+# CONFIG_SENSORS_M41T00 is not set
+# CONFIG_SENSORS_MAX6875 is not set
+# CONFIG_SENSORS_TSL2550 is not set
+# CONFIG_I2C_DEBUG_CORE is not set
+# CONFIG_I2C_DEBUG_ALGO is not set
+# CONFIG_I2C_DEBUG_BUS is not set
+# CONFIG_I2C_DEBUG_CHIP is not set
 
 #
 # SPI support
@@ -746,7 +799,119 @@ CONFIG_DUMMY_CONSOLE=y
 #
 # Sound
 #
-# CONFIG_SOUND is not set
+CONFIG_SOUND=y
+
+#
+# Advanced Linux Sound Architecture
+#
+CONFIG_SND=y
+CONFIG_SND_TIMER=y
+CONFIG_SND_PCM=y
+# CONFIG_SND_SEQUENCER is not set
+CONFIG_SND_OSSEMUL=y
+CONFIG_SND_MIXER_OSS=y
+CONFIG_SND_PCM_OSS=y
+# CONFIG_SND_PCM_OSS_PLUGINS is not set
+# CONFIG_SND_DYNAMIC_MINORS is not set
+# CONFIG_SND_SUPPORT_OLD_API is not set
+CONFIG_SND_VERBOSE_PROCFS=y
+# CONFIG_SND_VERBOSE_PRINTK is not set
+# CONFIG_SND_DEBUG is not set
+
+#
+# Generic devices
+#
+# CONFIG_SND_DUMMY is not set
+# CONFIG_SND_MTPAV is not set
+# CONFIG_SND_SERIAL_U16550 is not set
+# CONFIG_SND_MPU401 is not set
+
+#
+# PCI devices
+#
+# CONFIG_SND_AD1889 is not set
+# CONFIG_SND_ALS300 is not set
+# CONFIG_SND_ALS4000 is not set
+# CONFIG_SND_ALI5451 is not set
+# CONFIG_SND_ATIIXP is not set
+# CONFIG_SND_ATIIXP_MODEM is not set
+# CONFIG_SND_AU8810 is not set
+# CONFIG_SND_AU8820 is not set
+# CONFIG_SND_AU8830 is not set
+# CONFIG_SND_AZT3328 is not set
+# CONFIG_SND_BT87X is not set
+# CONFIG_SND_CA0106 is not set
+# CONFIG_SND_CMIPCI is not set
+# CONFIG_SND_CS4281 is not set
+# CONFIG_SND_CS46XX is not set
+# CONFIG_SND_CS5530 is not set
+# CONFIG_SND_DARLA20 is not set
+# CONFIG_SND_GINA20 is not set
+# CONFIG_SND_LAYLA20 is not set
+# CONFIG_SND_DARLA24 is not set
+# CONFIG_SND_GINA24 is not set
+# CONFIG_SND_LAYLA24 is not set
+# CONFIG_SND_MONA is not set
+# CONFIG_SND_MIA is not set
+# CONFIG_SND_ECHO3G is not set
+# CONFIG_SND_INDIGO is not set
+# CONFIG_SND_INDIGOIO is not set
+# CONFIG_SND_INDIGODJ is not set
+# CONFIG_SND_EMU10K1 is not set
+# CONFIG_SND_EMU10K1X is not set
+# CONFIG_SND_ENS1370 is not set
+# CONFIG_SND_ENS1371 is not set
+# CONFIG_SND_ES1938 is not set
+# CONFIG_SND_ES1968 is not set
+# CONFIG_SND_FM801 is not set
+# CONFIG_SND_HDA_INTEL is not set
+# CONFIG_SND_HDSP is not set
+# CONFIG_SND_HDSPM is not set
+# CONFIG_SND_ICE1712 is not set
+# CONFIG_SND_ICE1724 is not set
+# CONFIG_SND_INTEL8X0 is not set
+# CONFIG_SND_INTEL8X0M is not set
+# CONFIG_SND_KORG1212 is not set
+# CONFIG_SND_MAESTRO3 is not set
+# CONFIG_SND_MIXART is not set
+# CONFIG_SND_NM256 is not set
+# CONFIG_SND_PCXHR is not set
+# CONFIG_SND_RIPTIDE is not set
+# CONFIG_SND_RME32 is not set
+# CONFIG_SND_RME96 is not set
+# CONFIG_SND_RME9652 is not set
+# CONFIG_SND_SONICVIBES is not set
+# CONFIG_SND_TRIDENT is not set
+# CONFIG_SND_VIA82XX is not set
+# CONFIG_SND_VIA82XX_MODEM is not set
+# CONFIG_SND_VX222 is not set
+# CONFIG_SND_YMFPCI is not set
+
+#
+# ALSA PowerMac devices
+#
+
+#
+# ALSA PowerPC devices
+#
+
+#
+# System on Chip audio support
+#
+CONFIG_SND_SOC=y
+
+#
+# SoC Audio support for SuperH
+#
+
+#
+# ALSA SoC audio for Freescale SOCs
+#
+CONFIG_SND_SOC_MPC8610=y
+CONFIG_SND_SOC_MPC8610_HPCD=y
+CONFIG_SND_SOC_CS4270=y
+CONFIG_SND_SOC_CS4270_VD33_ERRATA=y
+
 CONFIG_HID_SUPPORT=y
 CONFIG_HID=y
 # CONFIG_HID_DEBUG is not set
diff --git a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
index 6390895..6e1bde3 100644
--- a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
+++ b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
@@ -34,9 +34,27 @@
 
 #include <asm/mpic.h>
 
+#include <asm/of_platform.h>
 #include <sysdev/fsl_pci.h>
 #include <sysdev/fsl_soc.h>
 
+static struct of_device_id mpc8610_ids[] = {
+	{ .type = "soc", },
+	{}
+};
+
+static int __init mpc8610_declare_of_platform_devices(void)
+{
+	if (!machine_is(mpc86xx_hpcd))
+		return 0;
+
+	/* Without this call, the SSI device driver won't get probed. */
+	of_platform_bus_probe(NULL, mpc8610_ids, NULL);
+
+	return 0;
+}
+device_initcall(mpc8610_declare_of_platform_devices);
+
 void __init
 mpc86xx_hpcd_init_irq(void)
 {
diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index 97b2552..43bbc60 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -28,6 +28,7 @@ source "sound/soc/at91/Kconfig"
 source "sound/soc/pxa/Kconfig"
 source "sound/soc/s3c24xx/Kconfig"
 source "sound/soc/sh/Kconfig"
+source "sound/soc/fsl/Kconfig"
 
 # Supported codecs
 source "sound/soc/codecs/Kconfig"
diff --git a/sound/soc/Makefile b/sound/soc/Makefile
index 3041403..4869c9a 100644
--- a/sound/soc/Makefile
+++ b/sound/soc/Makefile
@@ -1,4 +1,4 @@
 snd-soc-core-objs := soc-core.o soc-dapm.o
 
 obj-$(CONFIG_SND_SOC)	+= snd-soc-core.o
-obj-$(CONFIG_SND_SOC)	+= codecs/ at91/ pxa/ s3c24xx/ sh/
+obj-$(CONFIG_SND_SOC)	+= codecs/ at91/ pxa/ s3c24xx/ sh/ fsl/
diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
new file mode 100644
index 0000000..4a5bbd2
--- /dev/null
+++ b/sound/soc/fsl/Kconfig
@@ -0,0 +1,21 @@
+menu "ALSA SoC audio for Freescale SOCs"
+
+config SND_SOC_MPC8610
+	bool "ALSA SoC support for the MPC8610 SOC"
+	depends on SND_SOC #&& MPC8610_HPCD
+	default y #if MPC8610
+	help
+	  Say Y if you want to add support for codecs attached to the SSI
+          device on an MPC8610.
+
+config SND_SOC_MPC8610_HPCD
+	# ALSA SoC support for Freescale MPC8610HPCD
+	bool "ALSA SoC support for the Freescale MPC8610 HPCD board"
+	depends on SND_SOC_MPC8610
+	select SND_SOC_CS4270
+	select SND_SOC_CS4270_VD33_ERRATA
+	default y #if MPC8610_HPCD
+	help
+	  Say Y if you want to enable audio on the Freescale MPC8610 HPCD.
+
+endmenu
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
new file mode 100644
index 0000000..62f680a
--- /dev/null
+++ b/sound/soc/fsl/Makefile
@@ -0,0 +1,6 @@
+# MPC8610 HPCD Machine Support
+obj-$(CONFIG_SND_SOC_MPC8610_HPCD) += mpc8610_hpcd.o
+
+# MPC8610 Platform Support
+obj-$(CONFIG_SND_SOC_MPC8610) += fsl_ssi.o fsl_dma.o
+
diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c
new file mode 100644
index 0000000..6b86be0
--- /dev/null
+++ b/sound/soc/fsl/fsl_dma.c
@@ -0,0 +1,819 @@
+/*
+ * Freescale DMA ALSA SoC PCM driver
+ *
+ * Author: Timur Tabi <timur@freescale.com>
+ *
+ * Copyright 2007 Freescale Semiconductor, Inc.  This file is licensed under
+ * the terms of the GNU General Public License version 2.  This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ *
+ * This driver implements ASoC support for the Elo DMA controller, which is
+ * the DMA controller on Freescale 83xx, 85xx, and 86xx SOCs. In ALSA terms,
+ * the PCM driver is what handles the DMA buffer.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+
+#include <sound/driver.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+#include <asm/io.h>
+
+#include "fsl_dma.h"
+
+/*
+ * The formats that the DMA controller supports, which is anything
+ * that is 8, 16, or 32 bits.
+ */
+#define FSLDMA_PCM_FORMATS (SNDRV_PCM_FMTBIT_S8 	| \
+			    SNDRV_PCM_FMTBIT_U8 	| \
+			    SNDRV_PCM_FMTBIT_S16_LE     | \
+			    SNDRV_PCM_FMTBIT_S16_BE     | \
+			    SNDRV_PCM_FMTBIT_U16_LE     | \
+			    SNDRV_PCM_FMTBIT_U16_BE     | \
+			    SNDRV_PCM_FMTBIT_S24_LE     | \
+			    SNDRV_PCM_FMTBIT_S24_BE     | \
+			    SNDRV_PCM_FMTBIT_U24_LE     | \
+			    SNDRV_PCM_FMTBIT_U24_BE     | \
+			    SNDRV_PCM_FMTBIT_S32_LE     | \
+			    SNDRV_PCM_FMTBIT_S32_BE     | \
+			    SNDRV_PCM_FMTBIT_U32_LE     | \
+			    SNDRV_PCM_FMTBIT_U32_BE)
+
+#define FSLDMA_PCM_RATES (SNDRV_PCM_RATE_5512 | SNDRV_PCM_RATE_8000_192000 | \
+			  SNDRV_PCM_RATE_CONTINUOUS)
+
+/* DMA global data.  This structure is used by fsl_dma_open() to determine
+ * which DMA channels to assign to a substream.  Unfortunately, ASoC V1 does
+ * not allow the machine driver to provide this information to the PCM
+ * driver in advance, and there's no way to differentiate between the two
+ * DMA controllers.  So for now, this driver only supports one SSI device
+ * using two DMA channels.  We cannot support multiple DMA devices.
+ *
+ * ssi_stx_phys: bus address of SSI STX register
+ * ssi_srx_phys: bus address of SSI SRX register
+ * dma_channel: pointer to the DMA channel's registers
+ * irq: IRQ for this DMA channel
+ * assigned: set to 1 if that DMA channel is assigned to a substream
+ */
+static struct {
+	dma_addr_t ssi_stx_phys;
+	dma_addr_t ssi_srx_phys;
+	struct ccsr_dma_channel __iomem *dma_channel[2];
+	unsigned int irq[2];
+	unsigned int assigned[2];
+} dma_global_data;
+
+/*
+ * The number of DMA links to use.  Two is the bare minimum, but if you
+ * have really small links you might need more.
+ */
+#define NUM_DMA_LINKS   2
+
+/* Per-substream DMA data
+ *
+ * Each substream has a 1-to-1 association with a DMA channel.
+ *
+ * The link[] array is first because it needs to be aligned on a 32-byte
+ * boundary, so putting it first will ensure alignment without padding the
+ * structure.
+ *
+ * link[]: array of link descriptors
+ * controller_id: which DMA controller (0, 1, ...)
+ * channel_id: which DMA channel on the controller (0, 1, 2, ...)
+ * dma_channel: pointer to the DMA channel's registers
+ * irq: IRQ for this DMA channel
+ * substream: pointer to the substream object, needed by the ISR
+ * ssi_sxx_phys: bus address of the STX or SRX register to use
+ * ld_buf_phys: physical address of the LD buffer
+ * current_link: index into link[] of the link currently being processed
+ * dma_buf_phys: physical address of the DMA buffer
+ * dma_buf_next: physical address of the next period to process
+ * dma_buf_end: physical address of the byte after the end of the DMA buffer
+ * period_size: the size of a single period
+ */
+struct fsl_dma_private {
+	struct fsl_dma_link_descriptor link[NUM_DMA_LINKS];
+	unsigned int controller_id;
+	unsigned int channel_id;
+	struct ccsr_dma_channel __iomem *dma_channel;
+	unsigned int irq;
+	struct snd_pcm_substream *substream;
+	dma_addr_t ssi_sxx_phys;
+	dma_addr_t ld_buf_phys;
+	unsigned int current_link;
+	dma_addr_t dma_buf_phys;
+	dma_addr_t dma_buf_next;
+	dma_addr_t dma_buf_end;
+	size_t period_size;
+	unsigned int num_periods;
+};
+
+/*
+ * Define the hardware characteristics for the PCM hardware.
+ *
+ * The PCM hardware is the Freescale DMA controller.  This structure defines
+ * the capabilities of that hardware.
+ *
+ * Since the sampling rate and data format are not controlled by the DMA
+ * controller, we specify no limits for those values.  The only exception is
+ * period_bytes_min, which is set to a reasonably low value to prevent the
+ * DMA controller from generating too many interrupts per second.
+ *
+ * Since each link descriptor has a 32-bit byte count field, we set
+ * period_bytes_max to the largest 32-bit number.  We also have no maximum
+ * number of periods.
+ */
+static const struct snd_pcm_hardware fsl_dma_hardware = {
+
+	.info   		= SNDRV_PCM_INFO_INTERLEAVED,
+	.formats		= FSLDMA_PCM_FORMATS,
+	.rates  		= FSLDMA_PCM_RATES,
+	.rate_min       	= 5512,
+	.rate_max       	= 192000,
+	.period_bytes_min       = 512,  	/* A reasonable limit */
+	.period_bytes_max       = (u32) -1,
+	.periods_min    	= NUM_DMA_LINKS,
+	.periods_max    	= (unsigned int) -1,
+	.buffer_bytes_max       = 128 * 1024,   /* A reasonable limit */
+};
+
+/*
+ * Tell ALSA that the DMA transfer has aborted
+ *
+ * This function should be called by the ISR whenever the DMA controller
+ * halts data transfer.
+ */
+static void fsl_dma_abort_stream(struct snd_pcm_substream *substream)
+{
+	unsigned long flags;
+
+	snd_pcm_stream_lock_irqsave(substream, flags);
+
+	if (snd_pcm_running(substream))
+		snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
+
+	snd_pcm_stream_unlock_irqrestore(substream, flags);
+}
+
+/*
+ * Update the link descriptor pointers to point to the next period buffer.
+*/
+static void fsl_dma_update_pointers(struct fsl_dma_private *dma_private)
+{
+	struct fsl_dma_link_descriptor *link =
+		&dma_private->link[dma_private->current_link];
+
+	/* Update our link descriptors to point to the next period */
+	if (dma_private->substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		link->source_addr =
+			cpu_to_be32(dma_private->dma_buf_next);
+	else
+		link->dest_addr =
+			cpu_to_be32(dma_private->dma_buf_next);
+
+	/* Update our variables for next time */
+	dma_private->dma_buf_next += dma_private->period_size;
+
+	if (dma_private->dma_buf_next >= dma_private->dma_buf_end)
+		dma_private->dma_buf_next = dma_private->dma_buf_phys;
+
+	if (++dma_private->current_link >= NUM_DMA_LINKS)
+		dma_private->current_link = 0;
+}
+
+/*
+ * Interrupt handler for the DMA controller
+ */
+static irqreturn_t fsl_dma_isr(int irq, void *dev_id)
+{
+	struct fsl_dma_private *dma_private = dev_id;
+	struct ccsr_dma_channel __iomem *dma_channel = dma_private->dma_channel;
+	irqreturn_t ret = IRQ_NONE;
+	u32 sr, sr2 = 0;
+
+	/* We got an interrupt, so read the status register to see what we
+	   were interrupted for.
+	 */
+	sr = in_be32(&dma_channel->sr);
+
+	if (sr & CCSR_DMA_SR_TE) {
+		dev_err(dma_private->substream->pcm->card->dev,
+			"DMA transmit error (controller=%u channel=%u irq=%u\n",
+			dma_private->controller_id,
+			dma_private->channel_id, irq);
+		fsl_dma_abort_stream(dma_private->substream);
+		sr2 |= CCSR_DMA_SR_TE;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sr & CCSR_DMA_SR_CH)
+		ret = IRQ_HANDLED;
+
+	if (sr & CCSR_DMA_SR_PE) {
+		dev_err(dma_private->substream->pcm->card->dev,
+			"DMA%u programming error (channel=%u irq=%u)\n",
+			dma_private->controller_id,
+			dma_private->channel_id, irq);
+		fsl_dma_abort_stream(dma_private->substream);
+		sr2 |= CCSR_DMA_SR_PE;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sr & CCSR_DMA_SR_EOLNI) {
+		sr2 |= CCSR_DMA_SR_EOLNI;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sr & CCSR_DMA_SR_CB)
+		ret = IRQ_HANDLED;
+
+	if (sr & CCSR_DMA_SR_EOSI) {
+		struct snd_pcm_substream *substream = dma_private->substream;
+
+		/* Tell ALSA we completed a period. */
+		snd_pcm_period_elapsed(substream);
+
+		/*
+		 * Update our link descriptors to point to the next period. We
+		 * only need to do this if the number of periods is not equal to
+		 * the number of links.
+		 */
+		if (dma_private->num_periods != NUM_DMA_LINKS)
+			fsl_dma_update_pointers(dma_private);
+
+		sr2 |= CCSR_DMA_SR_EOSI;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sr & CCSR_DMA_SR_EOLSI) {
+		sr2 |= CCSR_DMA_SR_EOLSI;
+		ret = IRQ_HANDLED;
+	}
+
+	/* Clear the bits that we set */
+	if (sr2)
+		out_be32(&dma_channel->sr, sr2);
+
+	return ret;
+}
+
+/*
+ * Initialize this PCM driver.
+ *
+ * This function is called when the codec driver calls snd_soc_new_pcms(),
+ * once for each .dai_link in the machine driver's snd_soc_machine
+ * structure.
+ */
+static int fsl_dma_new(struct snd_card *card, struct snd_soc_codec_dai *dai,
+	struct snd_pcm *pcm)
+{
+	static u64 fsl_dma_dmamask = 0xffffffff;
+	int ret;
+
+	if (!card->dev->dma_mask)
+		card->dev->dma_mask = &fsl_dma_dmamask;
+
+	if (!card->dev->coherent_dma_mask)
+		card->dev->coherent_dma_mask = fsl_dma_dmamask;
+
+	ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, pcm->dev,
+		fsl_dma_hardware.buffer_bytes_max,
+		&pcm->streams[0].substream->dma_buffer);
+	if (ret) {
+		dev_err(card->dev,
+			"Can't allocate playback DMA buffer (size=%u)\n",
+			fsl_dma_hardware.buffer_bytes_max);
+		return -ENOMEM;
+	}
+
+	ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, pcm->dev,
+		fsl_dma_hardware.buffer_bytes_max,
+		&pcm->streams[1].substream->dma_buffer);
+	if (ret) {
+		snd_dma_free_pages(&pcm->streams[0].substream->dma_buffer);
+		dev_err(card->dev,
+			"Can't allocate capture DMA buffer (size=%u)\n",
+			fsl_dma_hardware.buffer_bytes_max);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+/*
+ * Open a new stream.
+ *
+ * Each stream has its own DMA buffer.
+ */
+static int fsl_dma_open(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct fsl_dma_private *dma_private;
+	dma_addr_t ld_buf_phys;
+	unsigned int channel;
+	int ret = 0;
+
+	/*
+	 * Reject any DMA buffer whose size is not a multiple of the period
+	 * size.  We need to make sure that the DMA buffer can be evenly divided
+	 * into periods.
+	 */
+	ret = snd_pcm_hw_constraint_integer(runtime,
+		SNDRV_PCM_HW_PARAM_PERIODS);
+	if (ret < 0) {
+		dev_err(substream->pcm->card->dev, "invalid buffer size\n");
+		return ret;
+	}
+
+	channel = substream->stream == SNDRV_PCM_STREAM_PLAYBACK ? 0 : 1;
+
+	if (dma_global_data.assigned[channel]) {
+		dev_err(substream->pcm->card->dev,
+			"DMA channel already assigned\n");
+		return -EBUSY;
+	}
+
+	dma_private = dma_alloc_coherent(substream->pcm->dev,
+		sizeof(struct fsl_dma_private), &ld_buf_phys, GFP_KERNEL);
+	if (!dma_private) {
+		dev_err(substream->pcm->card->dev,
+			"can't allocate DMA private data\n");
+		return -ENOMEM;
+	}
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		dma_private->ssi_sxx_phys = dma_global_data.ssi_stx_phys;
+	else
+		dma_private->ssi_sxx_phys = dma_global_data.ssi_srx_phys;
+
+	dma_private->dma_channel = dma_global_data.dma_channel[channel];
+	dma_private->irq = dma_global_data.irq[channel];
+	dma_private->substream = substream;
+	dma_private->ld_buf_phys = ld_buf_phys;
+	dma_private->dma_buf_phys = substream->dma_buffer.addr;
+
+	/* We only support one DMA controller for now */
+	dma_private->controller_id = 0;
+	dma_private->channel_id = channel;
+
+	ret = request_irq(dma_private->irq, fsl_dma_isr, 0, "DMA", dma_private);
+	if (ret) {
+		dev_err(substream->pcm->card->dev,
+			"can't register ISR for IRQ %u (ret=%i)\n",
+			dma_private->irq, ret);
+		dma_free_coherent(substream->pcm->dev,
+			sizeof(struct fsl_dma_private),
+			dma_private, dma_private->ld_buf_phys);
+		return ret;
+	}
+
+	dma_global_data.assigned[channel] = 1;
+
+	snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
+	snd_soc_set_runtime_hwparams(substream, &fsl_dma_hardware);
+	runtime->private_data = dma_private;
+
+	return 0;
+}
+
+/*
+ * Allocate the DMA buffer and the DMA link descriptors.
+ *
+ * ALSA divides the DMA buffer into N periods.  We create NUM_DMA_LINKS link
+ * descriptors that ping-pong from one period to the next.  For example, if
+ * there are six periods and two link descriptors, this is how they look
+ * before playback starts:
+ *
+ *      	   The last link descriptor
+ *   ____________  points back to the first
+ *  |   	 |
+ *  V   	 |
+ *  ___    ___   |
+ * |   |->|   |->|
+ * |___|  |___|
+ *   |      |
+ *   |      |
+ *   V      V
+ *  _________________________________________
+ * |      |      |      |      |      |      |  The DMA buffer is
+ * |      |      |      |      |      |      |    divided into 6 parts
+ * |______|______|______|______|______|______|
+ *
+ * and here's how they look after the first period is finished playing:
+ *
+ *   ____________
+ *  |   	 |
+ *  V   	 |
+ *  ___    ___   |
+ * |   |->|   |->|
+ * |___|  |___|
+ *   |      |
+ *   |______________
+ *          |       |
+ *          V       V
+ *  _________________________________________
+ * |      |      |      |      |      |      |
+ * |      |      |      |      |      |      |
+ * |______|______|______|______|______|______|
+ *
+ * The first link descriptor now points to the third period.  The DMA
+ * controller is currently playing the second period.  When it finishes, it
+ * will jump back to the first descriptor and play the third period.
+ *
+ * There are four reasons we do this:
+ *
+ * 1. The only way to get the DMA controller to automatically restart the
+ *    transfer when it gets to the end of the buffer is to use chaining
+ *    mode.  Basic direct mode doesn't offer that feature.
+ * 2. We need to receive an interrupt at the end of every period.  The DMA
+ *    controller can generate an interrupt at the end of every link transfer
+ *    (aka segment).  Making each period into a DMA segment will give us the
+ *    interrupts we need.
+ * 3. By creating only two link descriptors, regardless of the number of
+ *    periods, we do not need to reallocate the link descriptors if the
+ *    number of periods changes.
+ * 4. All of the audio data is still stored in a single, contiguous DMA
+ *    buffer, which is what ALSA expects.  We're just dividing it into
+ *    contiguous parts, and creating a link descriptor for each one.
+ *
+ * Note that due to a quirk of the SSI's STX register, the target address
+ * for the DMA operations depends on the sample size.  So we don't program
+ * the dest_addr (for playback -- source_addr for capture) fields in the
+ * link descriptors here.  We do that in fsl_dma_prepare()
+ */
+static int fsl_dma_hw_params(struct snd_pcm_substream *substream,
+				 struct snd_pcm_hw_params *hw_params)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct fsl_dma_private *dma_private = runtime->private_data;
+	struct ccsr_dma_channel __iomem *dma_channel = dma_private->dma_channel;
+
+	dma_addr_t temp_addr;   /* Pointer to next period */
+	u64 temp_link;  	/* Pointer to next link descriptor */
+	u32 mr; 		/* Temporary variable for MR register */
+
+	unsigned int i;
+
+	/* Get all the parameters we need */
+	size_t buffer_size = params_buffer_bytes(hw_params);
+	size_t period_size = params_period_bytes(hw_params);
+
+	/* Initialize our DMA tracking variables */
+	dma_private->period_size = period_size;
+	dma_private->num_periods = params_periods(hw_params);
+	dma_private->dma_buf_end = dma_private->dma_buf_phys + buffer_size;
+	dma_private->dma_buf_next = dma_private->dma_buf_phys +
+		(NUM_DMA_LINKS * period_size);
+	if (dma_private->dma_buf_next >= dma_private->dma_buf_end)
+		dma_private->dma_buf_next = dma_private->dma_buf_phys;
+
+	/*
+	 * Initialize each link descriptor.
+	 *
+	 * The actual address in STX0 (destination for playback, source for
+	 * capture) is based on the sample size, but we don't know the sample
+	 * size in this function, so we'll have to adjust that later.  See
+	 * comments in fsl_dma_prepare().
+	 *
+	 * The DMA controller does not have a cache, so the CPU does not
+	 * need to tell it to flush its cache.  However, the DMA
+	 * controller does need to tell the CPU to flush its cache.
+	 * That's what the SNOOP bit does.
+	 *
+	 * Also, even though the DMA controller supports 36-bit addressing, for
+	 * simplicity we currently support only 32-bit addresses for the audio
+	 * buffer itself.
+	 */
+	temp_addr = substream->dma_buffer.addr;
+	temp_link = dma_private->ld_buf_phys +
+		sizeof(struct fsl_dma_link_descriptor);
+
+	for (i = 0; i < NUM_DMA_LINKS; i++) {
+		struct fsl_dma_link_descriptor *link = &dma_private->link[i];
+
+		link->count = cpu_to_be32(period_size);
+		link->source_attr = cpu_to_be32(CCSR_DMA_ATR_SNOOP);
+		link->dest_attr = cpu_to_be32(CCSR_DMA_ATR_SNOOP);
+		link->next = cpu_to_be64(temp_link);
+
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+			link->source_addr = cpu_to_be32(temp_addr);
+		else
+			link->dest_addr = cpu_to_be32(temp_addr);
+
+		temp_addr += period_size;
+		temp_link += sizeof(struct fsl_dma_link_descriptor);
+	}
+	/* The last link descriptor points to the first */
+	dma_private->link[i - 1].next = cpu_to_be64(dma_private->ld_buf_phys);
+
+	/* Tell the DMA controller where the first link descriptor is */
+	out_be32(&dma_channel->clndar,
+		CCSR_DMA_CLNDAR_ADDR(dma_private->ld_buf_phys));
+	out_be32(&dma_channel->eclndar,
+		CCSR_DMA_ECLNDAR_ADDR(dma_private->ld_buf_phys));
+
+	/* The manual says the BCR must be clear before enabling EMP */
+	out_be32(&dma_channel->bcr, 0);
+
+	/* Program the mode register for interrupts, external master control,
+	   and source/destination hold.  Also clear the Channel Abort bit.
+	 */
+	mr = in_be32(&dma_channel->mr) &
+		~(CCSR_DMA_MR_CA | CCSR_DMA_MR_DAHE | CCSR_DMA_MR_SAHE);
+
+	/*
+	 * We want External Master Start and External Master Pause enabled,
+	 * because the SSI is controlling the DMA controller.  We want the DMA
+	 * controller to be set up in advance, and then we signal only the SSI
+	 * to start transfering.
+	 *
+	 * We want End-Of-Segment Interrupts enabled, because this will generate
+	 * an interrupt at the end of each segment (each link descriptor
+	 * represents one segment).  Each DMA segment is the same thing as an
+	 * ALSA period, so this is how we get an interrupt at the end of every
+	 * period.
+	 *
+	 * We want Error Interrupt enabled, so that we can get an error if
+	 * the DMA controller is mis-programmed somehow.
+	 */
+	mr |= CCSR_DMA_MR_EOSIE | CCSR_DMA_MR_EIE | CCSR_DMA_MR_EMP_EN |
+		CCSR_DMA_MR_EMS_EN;
+
+	/* For playback, we want the destination address to be held.  For
+	   capture, set the source address to be held. */
+	mr |= (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
+		CCSR_DMA_MR_DAHE : CCSR_DMA_MR_SAHE;
+
+	out_be32(&dma_channel->mr, mr);
+
+	return 0;
+}
+
+/*
+ * Prepare the DMA registers for playback.
+ *
+ * This function is called after the specifics of the audio data are known,
+ * i.e. snd_pcm_runtime is initialized.
+ *
+ * In this function, we finish programming the registers of the DMA
+ * controller that are dependent on the sample size.
+ *
+ * One of the drawbacks with big-endian is that when copying integers of
+ * different sizes to a fixed-sized register, the address to which the
+ * integer must be copied is dependent on the size of the integer.
+ *
+ * For example, if P is the address of a 32-bit register, and X is a 32-bit
+ * integer, then X should be copied to address P.  However, if X is a 16-bit
+ * integer, then it should be copied to P+2.  If X is an 8-bit register,
+ * then it should be copied to P+3.
+ *
+ * So for playback of 8-bit samples, the DMA controller must transfer single
+ * bytes from the DMA buffer to the last byte of the STX0 register, i.e.
+ * offset by 3 bytes. For 16-bit samples, the offset is two bytes.
+ *
+ * For 24-bit samples, the offset is 1 byte.  However, the DMA controller
+ * does not support 3-byte copies (the DAHTS register supports only 1, 2, 4,
+ * and 8 bytes at a time).  So we do not support packed 24-bit samples.
+ * 24-bit data must be padded to 32 bits.
+ */
+static int fsl_dma_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct fsl_dma_private *dma_private = runtime->private_data;
+	struct ccsr_dma_channel __iomem *dma_channel = dma_private->dma_channel;
+	u32 mr;
+	unsigned int i;
+	dma_addr_t ssi_sxx_phys;	/* Bus address of SSI STX register */
+	unsigned int frame_size;	/* Number of bytes per frame */
+
+	ssi_sxx_phys = dma_private->ssi_sxx_phys;
+
+	mr = in_be32(&dma_channel->mr) & ~(CCSR_DMA_MR_BWC_MASK |
+		  CCSR_DMA_MR_SAHTS_MASK | CCSR_DMA_MR_DAHTS_MASK);
+
+	switch (runtime->sample_bits) {
+	case 8:
+		mr |= CCSR_DMA_MR_DAHTS_1 | CCSR_DMA_MR_SAHTS_1;
+		ssi_sxx_phys += 3;
+		break;
+	case 16:
+		mr |= CCSR_DMA_MR_DAHTS_2 | CCSR_DMA_MR_SAHTS_2;
+		ssi_sxx_phys += 2;
+		break;
+	case 32:
+		mr |= CCSR_DMA_MR_DAHTS_4 | CCSR_DMA_MR_SAHTS_4;
+		break;
+	default:
+		dev_err(substream->pcm->card->dev,
+			"unsupported sample size %u\n", runtime->sample_bits);
+		return -EINVAL;
+	}
+
+	frame_size = runtime->frame_bits / 8;
+	/*
+	 * BWC should always be a multiple of the frame size.  BWC determines
+	 * how many bytes are sent/received before the DMA controller checks the
+	 * SSI to see if it needs to stop.  For playback, the transmit FIFO can
+	 * hold three frames, so we want to send two frames at a time. For
+	 * capture, the receive FIFO is triggered when it contains one frame, so
+	 * we want to receive one frame at a time.
+	 */
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		mr |= CCSR_DMA_MR_BWC(2 * frame_size);
+	else
+		mr |= CCSR_DMA_MR_BWC(frame_size);
+
+	out_be32(&dma_channel->mr, mr);
+
+	/*
+	 * Program the address of the DMA transfer to/from the SSI.
+	 */
+	for (i = 0; i < NUM_DMA_LINKS; i++) {
+		struct fsl_dma_link_descriptor *link = &dma_private->link[i];
+
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+			link->dest_addr = cpu_to_be32(ssi_sxx_phys);
+		else
+			link->source_addr = cpu_to_be32(ssi_sxx_phys);
+	}
+
+	return 0;
+}
+
+/*
+ * fsl_dma_pointer - 'pointer' PCM operation callback function
+ *
+ * This function is called by ALSA when ALSA wants to know where in the
+ * stream buffer the hardware currently is.
+ *
+ * For playback, the SAR register contains the physical address of the most
+ * recent DMA transfer.  For capture, the value is in the DAR register.
+ *
+ * The base address of the buffer is stored in the source_addr field of the
+ * first link descriptor.
+ */
+static snd_pcm_uframes_t fsl_dma_pointer(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct fsl_dma_private *dma_private = runtime->private_data;
+	struct ccsr_dma_channel __iomem *dma_channel = dma_private->dma_channel;
+	dma_addr_t position;
+	snd_pcm_uframes_t frames;
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		position = in_be32(&dma_channel->sar);
+	else
+		position = in_be32(&dma_channel->dar);
+
+	frames = bytes_to_frames(runtime, position - dma_private->dma_buf_phys);
+
+	/*
+	 * If the current address is just past the end of the buffer, wrap it
+	 * around.
+	 */
+	if (frames == runtime->buffer_size)
+		frames = 0;
+
+	return frames;
+}
+
+/*
+ * Release the resources allocated in fsl_dma_hw_params() and
+ * de-program the registers.  This function can be called multiple times.
+ */
+static int fsl_dma_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct fsl_dma_private *dma_private = runtime->private_data;
+
+	if (dma_private) {
+		struct ccsr_dma_channel __iomem *dma_channel;
+
+		dma_channel = dma_private->dma_channel;
+
+		dump_dma_regs(__FUNCTION__, __LINE__, dma_channel);
+		dump_lds(dma_private);
+
+		/* Stop the DMA */
+		out_be32(&dma_channel->mr, CCSR_DMA_MR_CA);
+		out_be32(&dma_channel->mr, 0);
+
+		/* Reset all the other registers */
+		out_be32(&dma_channel->sr, -1);
+		out_be32(&dma_channel->clndar, 0);
+		out_be32(&dma_channel->eclndar, 0);
+		out_be32(&dma_channel->satr, 0);
+		out_be32(&dma_channel->sar, 0);
+		out_be32(&dma_channel->datr, 0);
+		out_be32(&dma_channel->dar, 0);
+		out_be32(&dma_channel->bcr, 0);
+		out_be32(&dma_channel->nlndar, 0);
+		out_be32(&dma_channel->enlndar, 0);
+	}
+
+	return 0;
+}
+
+/*
+ * Close the stream.
+ */
+static int fsl_dma_close(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct fsl_dma_private *dma_private = runtime->private_data;
+	int dir = substream->stream == SNDRV_PCM_STREAM_PLAYBACK ? 0 : 1;
+
+	if (dma_private) {
+		if (dma_private->irq)
+			free_irq(dma_private->irq, dma_private);
+
+		if (dma_private->ld_buf_phys) {
+			dma_unmap_single(substream->pcm->dev,
+				dma_private->ld_buf_phys,
+				sizeof(dma_private->link), DMA_TO_DEVICE);
+		}
+
+		/* Deallocate the fsl_dma_private structure */
+		dma_free_coherent(substream->pcm->dev,
+			sizeof(struct fsl_dma_private),
+			dma_private, dma_private->ld_buf_phys);
+		substream->runtime->private_data = NULL;
+	}
+
+	dma_global_data.assigned[dir] = 0;
+
+	return 0;
+}
+
+/*
+ * Remove this PCM driver.
+ */
+static void fsl_dma_free_dma_buffers(struct snd_pcm *pcm)
+{
+	struct snd_pcm_substream *substream;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(pcm->streams); i++) {
+		substream = pcm->streams[i].substream;
+		if (substream) {
+			snd_dma_free_pages(&substream->dma_buffer);
+			substream->dma_buffer.area = NULL;
+			substream->dma_buffer.addr = 0;
+		}
+	}
+}
+
+static struct snd_pcm_ops fsl_dma_ops = {
+	.open   	= fsl_dma_open,
+	.close  	= fsl_dma_close,
+	.ioctl  	= snd_pcm_lib_ioctl,
+	.hw_params      = fsl_dma_hw_params,
+	.hw_free	= fsl_dma_hw_free,
+	.prepare	= fsl_dma_prepare,
+	.pointer	= fsl_dma_pointer,
+};
+
+struct snd_soc_platform fsl_soc_platform = {
+	.name   	= "fsl-dma",
+	.pcm_ops	= &fsl_dma_ops,
+	.pcm_new	= fsl_dma_new,
+	.pcm_free       = fsl_dma_free_dma_buffers,
+};
+EXPORT_SYMBOL_GPL(fsl_soc_platform);
+
+int fsl_dma_configure(struct fsl_dma_info *dma_info)
+{
+	static int initialized;
+
+	/* We only support one DMA controller for now */
+	if (initialized)
+		return 0;
+
+	dma_global_data.ssi_stx_phys = dma_info->ssi_stx_phys;
+	dma_global_data.ssi_srx_phys = dma_info->ssi_srx_phys;
+	dma_global_data.dma_channel[0] = dma_info->dma_channel[0];
+	dma_global_data.dma_channel[1] = dma_info->dma_channel[1];
+	dma_global_data.irq[0] = dma_info->dma_irq[0];
+	dma_global_data.irq[1] = dma_info->dma_irq[1];
+	dma_global_data.assigned[0] = 0;
+	dma_global_data.assigned[1] = 0;
+
+	initialized = 1;
+	return 1;
+}
+EXPORT_SYMBOL_GPL(fsl_dma_configure);
+
+MODULE_AUTHOR("Timur Tabi <timur@freescale.com>");
+MODULE_DESCRIPTION("Freescale FSL Elo DMA ASoC PCM module");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/fsl/fsl_dma.h b/sound/soc/fsl/fsl_dma.h
new file mode 100644
index 0000000..430a6ce
--- /dev/null
+++ b/sound/soc/fsl/fsl_dma.h
@@ -0,0 +1,149 @@
+/*
+ * mpc8610-pcm.h - ALSA PCM interface for the Freescale MPC8610 SoC
+ *
+ * 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 _MPC8610_PCM_H
+#define _MPC8610_PCM_H
+
+struct ccsr_dma {
+	u8 res0[0x100];
+	struct ccsr_dma_channel {
+		__be32 mr;      /* Mode register */
+		__be32 sr;      /* Status register */
+		__be32 eclndar; /* Current link descriptor extended addr reg */
+		__be32 clndar;  /* Current link descriptor address register */
+		__be32 satr;    /* Source attributes register */
+		__be32 sar;     /* Source address register */
+		__be32 datr;    /* Destination attributes register */
+		__be32 dar;     /* Destination address register */
+		__be32 bcr;     /* Byte count register */
+		__be32 enlndar; /* Next link descriptor extended address reg */
+		__be32 nlndar;  /* Next link descriptor address register */
+		u8 res1[4];
+		__be32 eclsdar; /* Current list descriptor extended addr reg */
+		__be32 clsdar;  /* Current list descriptor address register */
+		__be32 enlsdar; /* Next list descriptor extended address reg */
+		__be32 nlsdar;  /* Next list descriptor address register */
+		__be32 ssr;     /* Source stride register */
+		__be32 dsr;     /* Destination stride register */
+		u8 res2[0x38];
+	} channel[4];
+	__be32 dgsr;
+};
+
+#define CCSR_DMA_MR_BWC_DISABLED	0x0F000000
+#define CCSR_DMA_MR_BWC_SHIFT   	24
+#define CCSR_DMA_MR_BWC_MASK    	0x0F000000
+#define CCSR_DMA_MR_BWC(x) \
+	((ilog2(x) << CCSR_DMA_MR_BWC_SHIFT) & CCSR_DMA_MR_BWC_MASK)
+#define CCSR_DMA_MR_EMP_EN      	0x00200000
+#define CCSR_DMA_MR_EMS_EN      	0x00040000
+#define CCSR_DMA_MR_DAHTS_MASK  	0x00030000
+#define CCSR_DMA_MR_DAHTS_1     	0x00000000
+#define CCSR_DMA_MR_DAHTS_2     	0x00010000
+#define CCSR_DMA_MR_DAHTS_4     	0x00020000
+#define CCSR_DMA_MR_DAHTS_8     	0x00030000
+#define CCSR_DMA_MR_SAHTS_MASK  	0x0000C000
+#define CCSR_DMA_MR_SAHTS_1     	0x00000000
+#define CCSR_DMA_MR_SAHTS_2     	0x00004000
+#define CCSR_DMA_MR_SAHTS_4     	0x00008000
+#define CCSR_DMA_MR_SAHTS_8     	0x0000C000
+#define CCSR_DMA_MR_DAHE		0x00002000
+#define CCSR_DMA_MR_SAHE		0x00001000
+#define CCSR_DMA_MR_SRW 		0x00000400
+#define CCSR_DMA_MR_EOSIE       	0x00000200
+#define CCSR_DMA_MR_EOLNIE      	0x00000100
+#define CCSR_DMA_MR_EOLSIE      	0x00000080
+#define CCSR_DMA_MR_EIE 		0x00000040
+#define CCSR_DMA_MR_XFE 		0x00000020
+#define CCSR_DMA_MR_CDSM_SWSM   	0x00000010
+#define CCSR_DMA_MR_CA  		0x00000008
+#define CCSR_DMA_MR_CTM 		0x00000004
+#define CCSR_DMA_MR_CC  		0x00000002
+#define CCSR_DMA_MR_CS  		0x00000001
+
+#define CCSR_DMA_SR_TE  		0x00000080
+#define CCSR_DMA_SR_CH  		0x00000020
+#define CCSR_DMA_SR_PE  		0x00000010
+#define CCSR_DMA_SR_EOLNI       	0x00000008
+#define CCSR_DMA_SR_CB  		0x00000004
+#define CCSR_DMA_SR_EOSI		0x00000002
+#define CCSR_DMA_SR_EOLSI       	0x00000001
+
+/* ECLNDAR takes bits 32-36 of the CLNDAR register */
+static inline u32 CCSR_DMA_ECLNDAR_ADDR(u64 x)
+{
+	return (x >> 32) & 0xf;
+}
+
+#define CCSR_DMA_CLNDAR_ADDR(x) ((x) & 0xFFFFFFFE)
+#define CCSR_DMA_CLNDAR_EOSIE   	0x00000008
+
+/* SATR and DATR, combined */
+#define CCSR_DMA_ATR_PBATMU     	0x20000000
+#define CCSR_DMA_ATR_TFLOWLVL_0 	0x00000000
+#define CCSR_DMA_ATR_TFLOWLVL_1 	0x06000000
+#define CCSR_DMA_ATR_TFLOWLVL_2 	0x08000000
+#define CCSR_DMA_ATR_TFLOWLVL_3 	0x0C000000
+#define CCSR_DMA_ATR_PCIORDER   	0x02000000
+#define CCSR_DMA_ATR_SME		0x01000000
+#define CCSR_DMA_ATR_NOSNOOP    	0x00040000
+#define CCSR_DMA_ATR_SNOOP      	0x00050000
+#define CCSR_DMA_ATR_ESAD_MASK  	0x0000000F
+
+/**
+ *  List Descriptor for extended chaining mode DMA operations.
+ *
+ *  The CLSDAR register points to the first (in a linked-list) List
+ *  Descriptor.  Each object must be aligned on a 32-byte boundary. Each
+ *  list descriptor points to a linked-list of link Descriptors.
+ */
+struct fsl_dma_list_descriptor {
+	__be64 next;    	/* Address of next list descriptor */
+	__be64 first_link;      /* Address of first link descriptor */
+	__be32 source;  	/* Source stride */
+	__be32 dest;    	/* Destination stride */
+	u8 res[8];      	/* Reserved */
+} __attribute__ ((aligned(32), packed));
+
+/**
+ *  Link Descriptor for basic and extended chaining mode DMA operations.
+ *
+ *  A Link Descriptor points to a single DMA buffer.  Each link descriptor
+ *  must be aligned on a 32-byte boundary.
+ */
+struct fsl_dma_link_descriptor {
+	__be32 source_attr;     /* Programmed into SATR register */
+	__be32 source_addr;     /* Programmed into SAR register */
+	__be32 dest_attr;       /* Programmed into DATR register */
+	__be32 dest_addr;       /* Programmed into DAR register */
+	__be64 next;    /* Address of next link descriptor */
+	__be32 count;   /* Byte count */
+	u8 res[4];      /* Reserved */
+} __attribute__ ((aligned(32), packed));
+
+/* DMA information needed to create a snd_soc_cpu_dai object
+ *
+ * ssi_stx_phys: bus address of SSI STX register to use
+ * ssi_srx_phys: bus address of SSI SRX register to use
+ * dma[0]: points to the DMA channel to use for playback
+ * dma[1]: points to the DMA channel to use for capture
+ * dma_irq[0]: IRQ of the DMA channel to use for playback
+ * dma_irq[1]: IRQ of the DMA channel to use for capture
+ */
+struct fsl_dma_info {
+	dma_addr_t ssi_stx_phys;
+	dma_addr_t ssi_srx_phys;
+	struct ccsr_dma_channel __iomem *dma_channel[2];
+	unsigned int dma_irq[2];
+};
+
+extern struct snd_soc_platform fsl_soc_platform;
+
+int fsl_dma_configure(struct fsl_dma_info *dma_info);
+
+#endif
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
new file mode 100644
index 0000000..a219e58
--- /dev/null
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -0,0 +1,614 @@
+/*
+ * Freescale SSI ALSA SoC Digital Audio Interface (DAI) driver
+ *
+ * Author: Timur Tabi <timur@freescale.com>
+ *
+ * Copyright 2007 Freescale Semiconductor, Inc.  This file is licensed under
+ * the terms of the GNU General Public License version 2.  This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/device.h>
+#include <linux/delay.h>
+
+#include <sound/driver.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/initval.h>
+#include <sound/soc.h>
+
+#include <asm/immap_86xx.h>
+
+#include "fsl_ssi.h"
+
+/*
+ * The sample rates supported by the I2S
+ *
+ * This driver currently only supports the SSI running in I2S slave mode,
+ * which means the codec determines the sample rate.  Therefore, we tell
+ * ALSA that we support all rates and let the codec driver decide what rates
+ * are really supported.
+ */
+#define FSLSSI_I2S_RATES (SNDRV_PCM_RATE_5512 | SNDRV_PCM_RATE_8000_192000 | \
+			  SNDRV_PCM_RATE_CONTINUOUS)
+
+/*
+ * The audio formats supported by the SSI
+ *
+ * This driver currently only supports the SSI running in I2S slave mode.
+ *
+ * The SSI has a limitation in that the samples must be in the same byte
+ * order as the host CPU.  This is because when multiple bytes are written
+ * to the STX register, the bytes and bits must be written in the same
+ * order.  The STX is a shift register, so all the bits need to be aligned
+ * (bit-endianness must match byte-endianness).  Processors typically write
+ * the bits within a byte in the same order that the bytes of a word are
+ * written in.  So if the host CPU is big-endian, then only big-endian
+ * samples will be written to STX properly.
+ */
+#ifdef __BIG_ENDIAN
+#define FSLSSI_I2S_FORMATS (SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_BE | \
+	 SNDRV_PCM_FMTBIT_S18_3BE | SNDRV_PCM_FMTBIT_S20_3BE | \
+	 SNDRV_PCM_FMTBIT_S24_3BE | SNDRV_PCM_FMTBIT_S24_BE)
+#else
+#define FSLSSI_I2S_FORMATS (SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_LE | \
+	 SNDRV_PCM_FMTBIT_S18_3LE | SNDRV_PCM_FMTBIT_S20_3LE | \
+	 SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_LE)
+#endif
+
+/* SSI information.
+ *
+ * name: short name for this device ("SSI0", "SSI1", etc)
+ * ssi: pointer to the SSI's registers
+ * ssi_phys: physical address of the SSI registers
+ * irq: IRQ of this SSI
+ * dev: struct device pointer
+ * playback: the number of playback streams opened
+ * capture: the number of capture streams opened
+ */
+struct fsl_ssi_private {
+	char name[8];
+	struct ccsr_ssi __iomem *ssi;
+	dma_addr_t ssi_phys;
+	unsigned int irq;
+	struct device *dev;
+	unsigned int playback;
+	unsigned int capture;
+	struct snd_soc_cpu_dai cpu_dai;
+	struct device_attribute dev_attr;
+
+	struct {
+		unsigned int rfrc;
+		unsigned int tfrc;
+		unsigned int cmdau;
+		unsigned int cmddu;
+		unsigned int rxt;
+		unsigned int rdr1;
+		unsigned int rdr0;
+		unsigned int tde1;
+		unsigned int tde0;
+		unsigned int roe1;
+		unsigned int roe0;
+		unsigned int tue1;
+		unsigned int tue0;
+		unsigned int tfs;
+		unsigned int rfs;
+		unsigned int tls;
+		unsigned int rls;
+		unsigned int rff1;
+		unsigned int rff0;
+		unsigned int tfe1;
+		unsigned int tfe0;
+	} stats;
+};
+
+/*
+ * SSI interrupt handler.
+ *
+ * Do we even want an SSI interrupt handler?  Maybe for error handling, but
+ * buffer processing is handled via DMA interrupts.
+ */
+static irqreturn_t fsl_ssi_isr(int irq, void *dev_id)
+{
+	struct fsl_ssi_private *ssi_private = dev_id;
+	struct ccsr_ssi __iomem *ssi = ssi_private->ssi;
+	irqreturn_t ret = IRQ_NONE;
+	__be32 sisr;
+	__be32 sisr2 = 0;
+
+	/* We got an interrupt, so read the status register to see what we
+	   were interrupted for.  We mask it with the Interrupt Enable register
+	   so that we only check for events that we're interested in.
+	 */
+	sisr = in_be32(&ssi->sisr) & in_be32(&ssi->sier);
+
+	if (sisr & CCSR_SSI_SISR_RFRC) {
+		ssi_private->stats.rfrc++;
+		sisr2 |= CCSR_SSI_SISR_RFRC;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_TFRC) {
+		ssi_private->stats.tfrc++;
+		sisr2 |= CCSR_SSI_SISR_TFRC;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_CMDAU) {
+		ssi_private->stats.cmdau++;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_CMDDU) {
+		ssi_private->stats.cmddu++;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_RXT) {
+		ssi_private->stats.rxt++;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_RDR1) {
+		ssi_private->stats.rdr1++;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_RDR0) {
+		ssi_private->stats.rdr0++;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_TDE1) {
+		ssi_private->stats.tde1++;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_TDE0) {
+		ssi_private->stats.tde0++;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_ROE1) {
+		ssi_private->stats.roe1++;
+		sisr2 |= CCSR_SSI_SISR_ROE1;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_ROE0) {
+		ssi_private->stats.roe0++;
+		sisr2 |= CCSR_SSI_SISR_ROE0;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_TUE1) {
+		ssi_private->stats.tue1++;
+		sisr2 |= CCSR_SSI_SISR_TUE1;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_TUE0) {
+		ssi_private->stats.tue0++;
+		sisr2 |= CCSR_SSI_SISR_TUE0;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_TFS) {
+		ssi_private->stats.tfs++;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_RFS) {
+		ssi_private->stats.rfs++;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_TLS) {
+		ssi_private->stats.tls++;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_RLS) {
+		ssi_private->stats.rls++;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_RFF1) {
+		ssi_private->stats.rff1++;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_RFF0) {
+		ssi_private->stats.rff0++;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_TFE1) {
+		ssi_private->stats.tfe1++;
+		ret = IRQ_HANDLED;
+	}
+
+	if (sisr & CCSR_SSI_SISR_TFE0) {
+		ssi_private->stats.tfe0++;
+		ret = IRQ_HANDLED;
+	}
+
+	/* Clear the bits that we set */
+	if (sisr2)
+		out_be32(&ssi->sisr, sisr2);
+
+	return ret;
+}
+
+/*
+ * Startup
+ *
+ * This is the first function called when a stream is opened.
+ *
+ * If this is the first stream open, then grab the IRQ and program most of
+ * the SSI registers.
+ */
+static int fsl_ssi_startup(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct fsl_ssi_private *ssi_private = rtd->dai->cpu_dai->private_data;
+
+	/*
+	 * If this is the first stream opened, then request the IRQ
+	 * and initialize the SSI registers.
+	 */
+	if (!ssi_private->playback && !ssi_private->capture) {
+		struct ccsr_ssi __iomem *ssi = ssi_private->ssi;
+		int ret;
+
+		ret = request_irq(ssi_private->irq, fsl_ssi_isr, 0,
+				  ssi_private->name, ssi_private);
+		if (ret < 0) {
+			dev_err(substream->pcm->card->dev,
+				"could not claim irq %u\n", ssi_private->irq);
+			return ret;
+		}
+
+		/*
+		 * Section 16.5 of the MPC8610 reference manual says that the
+		 * SSI needs to be disabled before updating the registers we set
+		 * here.
+		 */
+		clrbits32(&ssi->scr, CCSR_SSI_SCR_SSIEN);
+
+		/*
+		 * Program the SSI into I2S Slave Non-Network Synchronous mode.
+		 * Also enable the transmit and receive FIFO.
+		 *
+		 * FIXME: Little-endian samples require a different shift dir
+		 */
+		clrsetbits_be32(&ssi->scr, CCSR_SSI_SCR_I2S_MODE_MASK,
+			CCSR_SSI_SCR_TFR_CLK_DIS |
+			CCSR_SSI_SCR_I2S_MODE_SLAVE | CCSR_SSI_SCR_SYN);
+
+		out_be32(&ssi->stcr,
+			 CCSR_SSI_STCR_TXBIT0 | CCSR_SSI_STCR_TFEN0 |
+			 CCSR_SSI_STCR_TFSI | CCSR_SSI_STCR_TEFS |
+			 CCSR_SSI_STCR_TSCKP);
+
+		out_be32(&ssi->srcr,
+			 CCSR_SSI_SRCR_RXBIT0 | CCSR_SSI_SRCR_RFEN0 |
+			 CCSR_SSI_SRCR_RFSI | CCSR_SSI_SRCR_REFS |
+			 CCSR_SSI_SRCR_RSCKP);
+
+		/*
+		 * The DC and PM bits are only used if the SSI is the clock
+		 * master.
+		 */
+
+		/* 4. Enable the interrupts and DMA requests */
+		out_be32(&ssi->sier,
+			 CCSR_SSI_SIER_TFRC_EN | CCSR_SSI_SIER_TDMAE |
+			 CCSR_SSI_SIER_TIE | CCSR_SSI_SIER_TUE0_EN |
+			 CCSR_SSI_SIER_TUE1_EN | CCSR_SSI_SIER_RFRC_EN |
+			 CCSR_SSI_SIER_RDMAE | CCSR_SSI_SIER_RIE |
+			 CCSR_SSI_SIER_ROE0_EN | CCSR_SSI_SIER_ROE1_EN);
+
+		/*
+		 * Set the watermark for transmit FIFI 0 and receive FIFO 0. We
+		 * don't use FIFO 1.  Since the SSI only supports stereo, the
+		 * watermark should never be an odd number.
+		 */
+		out_be32(&ssi->sfcsr,
+			 CCSR_SSI_SFCSR_TFWM0(6) | CCSR_SSI_SFCSR_RFWM0(2));
+
+		/*
+		 * We keep the SSI disabled because if we enable it, then the
+		 * DMA controller will start.  It's not supposed to start until
+		 * the SCR.TE (or SCR.RE) bit is set, but it does anyway.  The
+		 * DMA controller will transfer one "BWC" of data (i.e. the
+		 * amount of data that the MR.BWC bits are set to).  The reason
+		 * this is bad is because at this point, the PCM driver has not
+		 * finished initializing the DMA controller.
+		 */
+	}
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		ssi_private->playback++;
+
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+		ssi_private->capture++;
+
+	return 0;
+}
+
+/*
+ * Prepare the SSI.
+ *
+ * Most of the SSI registers have been programmed in the startup function,
+ * but the word length must be programmed here.  Unfortunately, programming
+ * the SxCCR.WL bits requires the SSI to be temporarily disabled.  This can
+ * cause a problem with supporting simultaneous playback and capture.  If
+ * the SSI is already playing a stream, then that stream may be temporarily
+ * stopped when you start capture.
+ *
+ * Note: The SxCCR.DC and SxCCR.PM bits are only used if the SSI is the
+ * clock master.
+ */
+static int fsl_ssi_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct fsl_ssi_private *ssi_private = rtd->dai->cpu_dai->private_data;
+
+	struct ccsr_ssi __iomem *ssi = ssi_private->ssi;
+	u32 wl;
+
+	wl = CCSR_SSI_SxCCR_WL(snd_pcm_format_width(runtime->format));
+
+	clrbits32(&ssi->scr, CCSR_SSI_SCR_SSIEN);
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		clrsetbits_be32(&ssi->stccr, CCSR_SSI_SxCCR_WL_MASK, wl);
+	else
+		clrsetbits_be32(&ssi->srccr, CCSR_SSI_SxCCR_WL_MASK, wl);
+
+	setbits32(&ssi->scr, CCSR_SSI_SCR_SSIEN);
+
+	return 0;
+}
+
+/*
+ * Start and stop the DMA transfer.
+ *
+ * This function is called by ALSA to start, stop, pause, and resume the DMA
+ * transfer of data.
+ *
+ * The SSI has put the DMA in external master start and pause mode, which
+ * means the SSI completely controls the flow of data.
+ */
+static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct fsl_ssi_private *ssi_private = rtd->dai->cpu_dai->private_data;
+	struct ccsr_ssi __iomem *ssi = ssi_private->ssi;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+			setbits32(&ssi->scr, CCSR_SSI_SCR_TE);
+		} else {
+			setbits32(&ssi->scr, CCSR_SSI_SCR_RE);
+
+			/*
+			 * I have no idea why we need a delay here. Without it,
+			 * the DMA won't start (or the SSI won't tell the DMA to
+			 * start).
+			 */
+			msleep(1);
+		}
+		break;
+
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+			clrbits32(&ssi->scr, CCSR_SSI_SCR_TE);
+		else
+			clrbits32(&ssi->scr, CCSR_SSI_SCR_RE);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/*
+ * Shutdown.  Clear DMA parameters and shutdown the SSI if there
+ * are no other substreams open.
+ */
+static void fsl_ssi_shutdown(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct fsl_ssi_private *ssi_private = rtd->dai->cpu_dai->private_data;
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		ssi_private->playback--;
+
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+		ssi_private->capture--;
+
+	/*
+	 * If this is the last active substream, disable the SSI and release
+	 * the IRQ.
+	 */
+	if (!ssi_private->playback && !ssi_private->capture) {
+		struct ccsr_ssi __iomem *ssi = ssi_private->ssi;
+
+		clrbits32(&ssi->scr, CCSR_SSI_SCR_SSIEN);
+
+		free_irq(ssi_private->irq, ssi_private);
+	}
+}
+
+/* Set the clock frequency and direction
+ *
+ * This function is called by the machine driver to tell us what the clock
+ * frequency and direction are.
+ *
+ * Currently, we only support operating as a clock slave (SND_SOC_CLOCK_IN),
+ * and we don't care about the frequency.  Return an error if the direction
+ * is not SND_SOC_CLOCK_IN.
+ */
+static int fsl_ssi_set_sysclk(struct snd_soc_cpu_dai *cpu_dai,
+			      int clk_id, unsigned int freq, int dir)
+{
+
+	return (dir == SND_SOC_CLOCK_IN) ? 0 : -EINVAL;
+}
+
+/* Set the serial format.
+ *
+ * This function is called by the machine driver to tell us what serial
+ * format to use.
+ *
+ * Currently, we only support I2S mode.  Return an error if the format is
+ * not SND_SOC_DAIFMT_I2S.
+ */
+static int fsl_ssi_set_fmt(struct snd_soc_cpu_dai *cpu_dai, unsigned int format)
+{
+	return (format == SND_SOC_DAIFMT_I2S) ? 0 : -EINVAL;
+}
+
+/*
+ * Template CPU DAI for the SSI
+ */
+static struct snd_soc_cpu_dai fsl_ssi_dai_template = {
+	.playback = {
+		/* The SSI does not support monaural audio. */
+		.channels_min = 2,
+		.channels_max = 2,
+		.rates = FSLSSI_I2S_RATES,
+		.formats = FSLSSI_I2S_FORMATS,
+	},
+	.capture = {
+		.channels_min = 2,
+		.channels_max = 2,
+		.rates = FSLSSI_I2S_RATES,
+		.formats = FSLSSI_I2S_FORMATS,
+	},
+	.ops = {
+		.startup = fsl_ssi_startup,
+		.prepare = fsl_ssi_prepare,
+		.shutdown = fsl_ssi_shutdown,
+		.trigger = fsl_ssi_trigger,
+	},
+	.dai_ops = {
+		.set_sysclk = fsl_ssi_set_sysclk,
+		.set_fmt = fsl_ssi_set_fmt,
+	},
+};
+
+static ssize_t fsl_sysfs_ssi_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct fsl_ssi_private *ssi_private =
+	container_of(attr, struct fsl_ssi_private, dev_attr);
+	ssize_t length;
+
+	length = sprintf(buf, "rfrc=%u", ssi_private->stats.rfrc);
+	length += sprintf(buf + length, "\ttfrc=%u", ssi_private->stats.tfrc);
+	length += sprintf(buf + length, "\tcmdau=%u", ssi_private->stats.cmdau);
+	length += sprintf(buf + length, "\tcmddu=%u", ssi_private->stats.cmddu);
+	length += sprintf(buf + length, "\trxt=%u", ssi_private->stats.rxt);
+	length += sprintf(buf + length, "\trdr1=%u", ssi_private->stats.rdr1);
+	length += sprintf(buf + length, "\trdr0=%u", ssi_private->stats.rdr0);
+	length += sprintf(buf + length, "\ttde1=%u", ssi_private->stats.tde1);
+	length += sprintf(buf + length, "\ttde0=%u", ssi_private->stats.tde0);
+	length += sprintf(buf + length, "\troe1=%u", ssi_private->stats.roe1);
+	length += sprintf(buf + length, "\troe0=%u", ssi_private->stats.roe0);
+	length += sprintf(buf + length, "\ttue1=%u", ssi_private->stats.tue1);
+	length += sprintf(buf + length, "\ttue0=%u", ssi_private->stats.tue0);
+	length += sprintf(buf + length, "\ttfs=%u", ssi_private->stats.tfs);
+	length += sprintf(buf + length, "\trfs=%u", ssi_private->stats.rfs);
+	length += sprintf(buf + length, "\ttls=%u", ssi_private->stats.tls);
+	length += sprintf(buf + length, "\trls=%u", ssi_private->stats.rls);
+	length += sprintf(buf + length, "\trff1=%u", ssi_private->stats.rff1);
+	length += sprintf(buf + length, "\trff0=%u", ssi_private->stats.rff0);
+	length += sprintf(buf + length, "\ttfe1=%u", ssi_private->stats.tfe1);
+	length += sprintf(buf + length, "\ttfe0=%u\n", ssi_private->stats.tfe0);
+
+	return length;
+}
+
+/* Create a snd_soc_cpu_dai structure
+ *
+ * This function is called by the machine driver to create a snd_soc_cpu_dai
+ * structure.  A private data structure, with SSI hardware information, is
+ * appended to it.
+ */
+struct snd_soc_cpu_dai *fsl_ssi_create_dai(struct fsl_ssi_info *ssi_info)
+{
+	struct snd_soc_cpu_dai *fsl_ssi_dai;
+	struct fsl_ssi_private *ssi_private;
+	int ret = 0;
+	struct device_attribute *dev_attr;
+
+	ssi_private = kzalloc(sizeof(struct fsl_ssi_private), GFP_KERNEL);
+	if (!ssi_private) {
+		dev_err(ssi_info->dev, "could not allocate DAI object\n");
+		return NULL;
+	}
+	memcpy(&ssi_private->cpu_dai, &fsl_ssi_dai_template,
+	       sizeof(struct snd_soc_cpu_dai));
+
+	fsl_ssi_dai = &ssi_private->cpu_dai;
+	dev_attr = &ssi_private->dev_attr;
+
+	sprintf(ssi_private->name, "ssi%u", (u8) ssi_info->id);
+	ssi_private->ssi = ssi_info->ssi;
+	ssi_private->ssi_phys = ssi_info->ssi_phys;
+	ssi_private->irq = ssi_info->irq;
+	ssi_private->dev = ssi_info->dev;
+
+	ssi_private->dev->driver_data = fsl_ssi_dai;
+
+	/* Initialize the the device_attribute structure */
+	dev_attr->attr.name = "ssi-stats";
+	dev_attr->attr.mode = S_IRUGO;
+	dev_attr->show = fsl_sysfs_ssi_show;
+
+	ret = device_create_file(ssi_private->dev, dev_attr);
+	if (ret) {
+		dev_err(ssi_info->dev, "could not create sysfs %s file\n",
+			ssi_private->dev_attr.attr.name);
+		kfree(fsl_ssi_dai);
+		return NULL;
+	}
+
+	fsl_ssi_dai->private_data = ssi_private;
+	fsl_ssi_dai->name = ssi_private->name;
+	fsl_ssi_dai->id = ssi_info->id;
+
+	return fsl_ssi_dai;
+}
+EXPORT_SYMBOL_GPL(fsl_ssi_create_dai);
+
+void fsl_ssi_destroy_dai(struct snd_soc_cpu_dai *fsl_ssi_dai)
+{
+	struct fsl_ssi_private *ssi_private =
+	container_of(fsl_ssi_dai, struct fsl_ssi_private, cpu_dai);
+
+	device_remove_file(ssi_private->dev, &ssi_private->dev_attr);
+
+	kfree(ssi_private);
+}
+EXPORT_SYMBOL_GPL(fsl_ssi_destroy_dai);
+
+MODULE_AUTHOR("Timur Tabi <timur@freescale.com>");
+MODULE_DESCRIPTION("Freescale Synchronous Serial Interface (SSI) ASoC Driver");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/fsl/fsl_ssi.h b/sound/soc/fsl/fsl_ssi.h
new file mode 100644
index 0000000..f6e33af
--- /dev/null
+++ b/sound/soc/fsl/fsl_ssi.h
@@ -0,0 +1,224 @@
+/*
+ * fsl_ssi.h - ALSA SSI interface for the Freescale MPC8610 SoC
+ *
+ * Author: Timur Tabi <timur@freescale.com>
+ *
+ * Copyright 2007 Freescale Semiconductor, Inc.  This file is licensed under
+ * the terms of the GNU General Public License version 2.  This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ */
+
+#ifndef _MPC8610_I2S_H
+#define _MPC8610_I2S_H
+
+/* SSI Register Map */
+struct ccsr_ssi {
+	__be32 stx0;	/* 0x.0000 - SSI Transmit Data Register 0 */
+	__be32 stx1;	/* 0x.0004 - SSI Transmit Data Register 1 */
+	__be32 srx0;	/* 0x.0008 - SSI Receive Data Register 0 */
+	__be32 srx1;	/* 0x.000C - SSI Receive Data Register 1 */
+	__be32 scr;	/* 0x.0010 - SSI Control Register */
+	__be32 sisr;	/* 0x.0014 - SSI Interrupt Status Register Mixed */
+	__be32 sier;	/* 0x.0018 - SSI Interrupt Enable Register */
+	__be32 stcr;	/* 0x.001C - SSI Transmit Configuration Register */
+	__be32 srcr;	/* 0x.0020 - SSI Receive Configuration Register */
+	__be32 stccr;	/* 0x.0024 - SSI Transmit Clock Control Register */
+	__be32 srccr;	/* 0x.0028 - SSI Receive Clock Control Register */
+	__be32 sfcsr;	/* 0x.002C - SSI FIFO Control/Status Register */
+	__be32 str;	/* 0x.0030 - SSI Test Register */
+	__be32 sor;	/* 0x.0034 - SSI Option Register */
+	__be32 sacnt;	/* 0x.0038 - SSI AC97 Control Register */
+	__be32 sacadd;	/* 0x.003C - SSI AC97 Command Address Register */
+	__be32 sacdat;	/* 0x.0040 - SSI AC97 Command Data Register */
+	__be32 satag;	/* 0x.0044 - SSI AC97 Tag Register */
+	__be32 stmsk;	/* 0x.0048 - SSI Transmit Time Slot Mask Register */
+	__be32 srmsk;	/* 0x.004C - SSI Receive Time Slot Mask Register */
+	__be32 saccst;	/* 0x.0050 - SSI AC97 Channel Status Register */
+	__be32 saccen;	/* 0x.0054 - SSI AC97 Channel Enable Register */
+	__be32 saccdis; /* 0x.0058 - SSI AC97 Channel Disable Register */
+};
+
+#define CCSR_SSI_SCR_RFR_CLK_DIS	0x00000800
+#define CCSR_SSI_SCR_TFR_CLK_DIS	0x00000400
+#define CCSR_SSI_SCR_TCH_EN		0x00000100
+#define CCSR_SSI_SCR_SYS_CLK_EN		0x00000080
+#define CCSR_SSI_SCR_I2S_MODE_MASK	0x00000060
+#define CCSR_SSI_SCR_I2S_MODE_NORMAL	0x00000000
+#define CCSR_SSI_SCR_I2S_MODE_MASTER	0x00000020
+#define CCSR_SSI_SCR_I2S_MODE_SLAVE	0x00000040
+#define CCSR_SSI_SCR_SYN		0x00000010
+#define CCSR_SSI_SCR_NET		0x00000008
+#define CCSR_SSI_SCR_RE			0x00000004
+#define CCSR_SSI_SCR_TE			0x00000002
+#define CCSR_SSI_SCR_SSIEN		0x00000001
+
+#define CCSR_SSI_SISR_RFRC		0x01000000
+#define CCSR_SSI_SISR_TFRC		0x00800000
+#define CCSR_SSI_SISR_CMDAU		0x00040000
+#define CCSR_SSI_SISR_CMDDU		0x00020000
+#define CCSR_SSI_SISR_RXT		0x00010000
+#define CCSR_SSI_SISR_RDR1		0x00008000
+#define CCSR_SSI_SISR_RDR0		0x00004000
+#define CCSR_SSI_SISR_TDE1		0x00002000
+#define CCSR_SSI_SISR_TDE0		0x00001000
+#define CCSR_SSI_SISR_ROE1		0x00000800
+#define CCSR_SSI_SISR_ROE0		0x00000400
+#define CCSR_SSI_SISR_TUE1		0x00000200
+#define CCSR_SSI_SISR_TUE0		0x00000100
+#define CCSR_SSI_SISR_TFS		0x00000080
+#define CCSR_SSI_SISR_RFS		0x00000040
+#define CCSR_SSI_SISR_TLS		0x00000020
+#define CCSR_SSI_SISR_RLS		0x00000010
+#define CCSR_SSI_SISR_RFF1		0x00000008
+#define CCSR_SSI_SISR_RFF0		0x00000004
+#define CCSR_SSI_SISR_TFE1		0x00000002
+#define CCSR_SSI_SISR_TFE0		0x00000001
+
+#define CCSR_SSI_SIER_RFRC_EN		0x01000000
+#define CCSR_SSI_SIER_TFRC_EN		0x00800000
+#define CCSR_SSI_SIER_RDMAE		0x00400000
+#define CCSR_SSI_SIER_RIE		0x00200000
+#define CCSR_SSI_SIER_TDMAE		0x00100000
+#define CCSR_SSI_SIER_TIE		0x00080000
+#define CCSR_SSI_SIER_CMDAU_EN		0x00040000
+#define CCSR_SSI_SIER_CMDDU_EN		0x00020000
+#define CCSR_SSI_SIER_RXT_EN		0x00010000
+#define CCSR_SSI_SIER_RDR1_EN		0x00008000
+#define CCSR_SSI_SIER_RDR0_EN		0x00004000
+#define CCSR_SSI_SIER_TDE1_EN		0x00002000
+#define CCSR_SSI_SIER_TDE0_EN		0x00001000
+#define CCSR_SSI_SIER_ROE1_EN		0x00000800
+#define CCSR_SSI_SIER_ROE0_EN		0x00000400
+#define CCSR_SSI_SIER_TUE1_EN		0x00000200
+#define CCSR_SSI_SIER_TUE0_EN		0x00000100
+#define CCSR_SSI_SIER_TFS_EN		0x00000080
+#define CCSR_SSI_SIER_RFS_EN		0x00000040
+#define CCSR_SSI_SIER_TLS_EN		0x00000020
+#define CCSR_SSI_SIER_RLS_EN		0x00000010
+#define CCSR_SSI_SIER_RFF1_EN		0x00000008
+#define CCSR_SSI_SIER_RFF0_EN		0x00000004
+#define CCSR_SSI_SIER_TFE1_EN		0x00000002
+#define CCSR_SSI_SIER_TFE0_EN		0x00000001
+
+#define CCSR_SSI_STCR_TXBIT0		0x00000200
+#define CCSR_SSI_STCR_TFEN1		0x00000100
+#define CCSR_SSI_STCR_TFEN0		0x00000080
+#define CCSR_SSI_STCR_TFDIR		0x00000040
+#define CCSR_SSI_STCR_TXDIR		0x00000020
+#define CCSR_SSI_STCR_TSHFD		0x00000010
+#define CCSR_SSI_STCR_TSCKP		0x00000008
+#define CCSR_SSI_STCR_TFSI		0x00000004
+#define CCSR_SSI_STCR_TFSL		0x00000002
+#define CCSR_SSI_STCR_TEFS		0x00000001
+
+#define CCSR_SSI_SRCR_RXEXT		0x00000400
+#define CCSR_SSI_SRCR_RXBIT0		0x00000200
+#define CCSR_SSI_SRCR_RFEN1		0x00000100
+#define CCSR_SSI_SRCR_RFEN0		0x00000080
+#define CCSR_SSI_SRCR_RFDIR		0x00000040
+#define CCSR_SSI_SRCR_RXDIR		0x00000020
+#define CCSR_SSI_SRCR_RSHFD		0x00000010
+#define CCSR_SSI_SRCR_RSCKP		0x00000008
+#define CCSR_SSI_SRCR_RFSI		0x00000004
+#define CCSR_SSI_SRCR_RFSL		0x00000002
+#define CCSR_SSI_SRCR_REFS		0x00000001
+
+/* STCCR and SRCCR */
+#define CCSR_SSI_SxCCR_DIV2		0x00040000
+#define CCSR_SSI_SxCCR_PSR		0x00020000
+#define CCSR_SSI_SxCCR_WL_SHIFT		13
+#define CCSR_SSI_SxCCR_WL_MASK		0x0001E000
+#define CCSR_SSI_SxCCR_WL(x) \
+	(((((x) / 2) - 1) << CCSR_SSI_SxCCR_WL_SHIFT) & CCSR_SSI_SxCCR_WL_MASK)
+#define CCSR_SSI_SxCCR_DC_SHIFT		8
+#define CCSR_SSI_SxCCR_DC_MASK		0x00001F00
+#define CCSR_SSI_SxCCR_DC(x) \
+	((((x) - 1) << CCSR_SSI_SxCCR_DC_SHIFT) & CCSR_SSI_SxCCR_DC_MASK)
+#define CCSR_SSI_SxCCR_PM_SHIFT		0
+#define CCSR_SSI_SxCCR_PM_MASK		0x000000FF
+#define CCSR_SSI_SxCCR_PM(x) \
+	((((x) - 1) << CCSR_SSI_SxCCR_PM_SHIFT) & CCSR_SSI_SxCCR_PM_MASK)
+
+/*
+ * The xFCNT bits are read-only, and the xFWM bits are read/write.  Use the
+ * CCSR_SSI_SFCSR_xFCNTy() macros to read the FIFO counters, and use the
+ * CCSR_SSI_SFCSR_xFWMy() macros to set the watermarks.
+ */
+#define CCSR_SSI_SFCSR_RFCNT1_SHIFT	28
+#define CCSR_SSI_SFCSR_RFCNT1_MASK	0xF0000000
+#define CCSR_SSI_SFCSR_RFCNT1(x) \
+	(((x) & CCSR_SSI_SFCSR_RFCNT1_MASK) >> CCSR_SSI_SFCSR_RFCNT1_SHIFT)
+#define CCSR_SSI_SFCSR_TFCNT1_SHIFT	24
+#define CCSR_SSI_SFCSR_TFCNT1_MASK	0x0F000000
+#define CCSR_SSI_SFCSR_TFCNT1(x) \
+	(((x) & CCSR_SSI_SFCSR_TFCNT1_MASK) >> CCSR_SSI_SFCSR_TFCNT1_SHIFT)
+#define CCSR_SSI_SFCSR_RFWM1_SHIFT	20
+#define CCSR_SSI_SFCSR_RFWM1_MASK	0x00F00000
+#define CCSR_SSI_SFCSR_RFWM1(x)	\
+	(((x) << CCSR_SSI_SFCSR_RFWM1_SHIFT) & CCSR_SSI_SFCSR_RFWM1_MASK)
+#define CCSR_SSI_SFCSR_TFWM1_SHIFT	16
+#define CCSR_SSI_SFCSR_TFWM1_MASK	0x000F0000
+#define CCSR_SSI_SFCSR_TFWM1(x)	\
+	(((x) << CCSR_SSI_SFCSR_TFWM1_SHIFT) & CCSR_SSI_SFCSR_TFWM1_MASK)
+#define CCSR_SSI_SFCSR_RFCNT0_SHIFT	12
+#define CCSR_SSI_SFCSR_RFCNT0_MASK	0x0000F000
+#define CCSR_SSI_SFCSR_RFCNT0(x) \
+	(((x) & CCSR_SSI_SFCSR_RFCNT0_MASK) >> CCSR_SSI_SFCSR_RFCNT0_SHIFT)
+#define CCSR_SSI_SFCSR_TFCNT0_SHIFT	8
+#define CCSR_SSI_SFCSR_TFCNT0_MASK	0x00000F00
+#define CCSR_SSI_SFCSR_TFCNT0(x) \
+	(((x) & CCSR_SSI_SFCSR_TFCNT0_MASK) >> CCSR_SSI_SFCSR_TFCNT0_SHIFT)
+#define CCSR_SSI_SFCSR_RFWM0_SHIFT	4
+#define CCSR_SSI_SFCSR_RFWM0_MASK	0x000000F0
+#define CCSR_SSI_SFCSR_RFWM0(x)	\
+	(((x) << CCSR_SSI_SFCSR_RFWM0_SHIFT) & CCSR_SSI_SFCSR_RFWM0_MASK)
+#define CCSR_SSI_SFCSR_TFWM0_SHIFT	0
+#define CCSR_SSI_SFCSR_TFWM0_MASK	0x0000000F
+#define CCSR_SSI_SFCSR_TFWM0(x)	\
+	(((x) << CCSR_SSI_SFCSR_TFWM0_SHIFT) & CCSR_SSI_SFCSR_TFWM0_MASK)
+
+#define CCSR_SSI_STR_TEST		0x00008000
+#define CCSR_SSI_STR_RCK2TCK		0x00004000
+#define CCSR_SSI_STR_RFS2TFS		0x00002000
+#define CCSR_SSI_STR_RXSTATE(x) (((x) >> 8) & 0x1F)
+#define CCSR_SSI_STR_TXD2RXD		0x00000080
+#define CCSR_SSI_STR_TCK2RCK		0x00000040
+#define CCSR_SSI_STR_TFS2RFS		0x00000020
+#define CCSR_SSI_STR_TXSTATE(x) ((x) & 0x1F)
+
+#define CCSR_SSI_SOR_CLKOFF		0x00000040
+#define CCSR_SSI_SOR_RX_CLR		0x00000020
+#define CCSR_SSI_SOR_TX_CLR		0x00000010
+#define CCSR_SSI_SOR_INIT		0x00000008
+#define CCSR_SSI_SOR_WAIT_SHIFT		1
+#define CCSR_SSI_SOR_WAIT_MASK		0x00000006
+#define CCSR_SSI_SOR_WAIT(x) (((x) & 3) << CCSR_SSI_SOR_WAIT_SHIFT)
+#define CCSR_SSI_SOR_SYNRST 		0x00000001
+
+/* Instantiation data for an SSI interface
+ *
+ * This structure contains all the information that the the SSI driver needs
+ * to instantiate an SSI interface with ALSA.  The machine driver should
+ * create this structure, fill it in, call fsl_ssi_create_dai(), and then
+ * delete the structure.
+ *
+ * id: which SSI this is (0, 1, etc. )
+ * ssi: pointer to the SSI's registers
+ * ssi_phys: physical address of the SSI registers
+ * irq: IRQ of this SSI
+ * dev: struct device, used to create the sysfs statistics file
+*/
+struct fsl_ssi_info {
+	unsigned int id;
+	struct ccsr_ssi __iomem *ssi;
+	dma_addr_t ssi_phys;
+	unsigned int irq;
+	struct device *dev;
+};
+
+struct snd_soc_cpu_dai *fsl_ssi_create_dai(struct fsl_ssi_info *ssi_info);
+void fsl_ssi_destroy_dai(struct snd_soc_cpu_dai *fsl_ssi_dai);
+
+#endif
+
diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c
new file mode 100644
index 0000000..5421a5c
--- /dev/null
+++ b/sound/soc/fsl/mpc8610_hpcd.c
@@ -0,0 +1,621 @@
+/**
+ * Freescale MPC8610HPCD ALSA SoC Fabric driver
+ *
+ * Author: Timur Tabi <timur@freescale.com>
+ *
+ * Copyright 2007 Freescale Semiconductor, Inc.  This file is licensed under
+ * the terms of the GNU General Public License version 2.  This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ */
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/version.h>
+#include <linux/kernel.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <asm/of_device.h>
+#include <asm/of_platform.h>
+#include <sound/driver.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <sound/initval.h>
+#include <asm/immap_86xx.h>
+
+#include "../codecs/cs4270.h"
+#include "fsl_dma.h"
+#include "fsl_ssi.h"
+
+/*
+ * Fabric-specific ASoC device data
+ *
+ * This structure contains data for a single sound platform device on an
+ * MPC8610 HPCD.  Some of the data is taken from the device tree.
+ */
+struct mpc8610hpcd_data {
+	struct snd_soc_device sound_devdata;
+	struct snd_soc_dai_link dai;
+	struct snd_soc_machine machine;
+	unsigned int dai_format;
+	unsigned int codec_clk_direction;
+	unsigned int cpu_clk_direction;
+	unsigned int clk_frequency;
+	struct ccsr_guts __iomem *guts;
+	struct ccsr_ssi __iomem *ssi;
+	unsigned int ssi_id;    	/* 0 = SSI1, 1 = SSI2, etc */
+	unsigned int ssi_irq;
+	unsigned int dma_id;    	/* 0 = DMA1, 1 = DMA2, etc */
+	unsigned int dma_irq[2];
+	struct ccsr_dma_channel __iomem *dma[2];
+	unsigned int dma_channel_id[2]; /* 0 = ch 0, 1 = ch 1, etc*/
+};
+
+/*
+ * Initalize the board
+ *
+ * This function is called when platform_device_add() is called.  It is used
+ * to initialize the board-specific hardware.
+ *
+ * Here we program the DMACR and PMUXCR registers.
+ */
+int mpc8610_hpcd_machine_probe(struct platform_device *sound_device)
+{
+	struct mpc8610hpcd_data *machine_data = sound_device->dev.platform_data;
+
+	/* Program the signal routing between the SSI and the DMA */
+	guts_set_dmacr(machine_data->guts, machine_data->dma_id + 1,
+		machine_data->dma_channel_id[0], CCSR_GUTS_DMACR_DEV_SSI);
+	guts_set_dmacr(machine_data->guts, machine_data->dma_id + 1,
+		machine_data->dma_channel_id[1], CCSR_GUTS_DMACR_DEV_SSI);
+
+	guts_set_pmuxcr_dma(machine_data->guts, machine_data->dma_id,
+		machine_data->dma_channel_id[0], 0);
+	guts_set_pmuxcr_dma(machine_data->guts, machine_data->dma_id,
+		machine_data->dma_channel_id[1], 0);
+
+	guts_set_pmuxcr_dma(machine_data->guts, 1, 0, 0);
+	guts_set_pmuxcr_dma(machine_data->guts, 1, 3, 0);
+	guts_set_pmuxcr_dma(machine_data->guts, 0, 3, 0);
+
+	switch (machine_data->ssi_id) {
+	case 0:
+		clrsetbits_be32(&machine_data->guts->pmuxcr,
+			CCSR_GUTS_PMUXCR_SSI1_MASK, CCSR_GUTS_PMUXCR_SSI1_SSI);
+		break;
+	case 1:
+		clrsetbits_be32(&machine_data->guts->pmuxcr,
+			CCSR_GUTS_PMUXCR_SSI2_MASK, CCSR_GUTS_PMUXCR_SSI2_SSI);
+		break;
+	}
+
+	return 0;
+}
+
+/*
+ * Program the board based on various hardware parameters
+ */
+static int mpc8610hpcd_startup(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_codec_dai *codec_dai = rtd->dai->codec_dai;
+	struct snd_soc_cpu_dai *cpu_dai = rtd->dai->cpu_dai;
+	struct mpc8610hpcd_data *machine_data = rtd->socdev->dev->platform_data;
+	int ret = 0;
+
+	/* Tell the CPU driver what the serial protocol is. */
+	if (cpu_dai->dai_ops.set_fmt) {
+		ret = cpu_dai->dai_ops.set_fmt(cpu_dai,
+			machine_data->dai_format);
+		if (ret < 0) {
+			dev_err(substream->pcm->card->dev,
+				"could not set CPU driver audio format\n");
+			return ret;
+		}
+	}
+
+	/* Tell the codec driver what the serial protocol is. */
+	if (codec_dai->dai_ops.set_fmt) {
+		ret = codec_dai->dai_ops.set_fmt(codec_dai,
+			machine_data->dai_format);
+		if (ret < 0) {
+			dev_err(substream->pcm->card->dev,
+				"could not set codec driver audio format\n");
+			return ret;
+		}
+	}
+
+	/*
+	 * Tell the CPU driver what the clock frequency is, and whether it's a
+	 * slave or master.
+	 */
+	if (cpu_dai->dai_ops.set_sysclk) {
+		ret = cpu_dai->dai_ops.set_sysclk(cpu_dai, 0,
+			machine_data->clk_frequency,
+			machine_data->cpu_clk_direction);
+		if (ret < 0) {
+			dev_err(substream->pcm->card->dev,
+				"could not set CPU driver clock parameters\n");
+			return ret;
+		}
+	}
+
+	/*
+	 * Tell the codec driver what the MCLK frequency is, and whether it's
+	 * a slave or master.
+	 */
+	if (codec_dai->dai_ops.set_sysclk) {
+		ret = codec_dai->dai_ops.set_sysclk(codec_dai, 0,
+			machine_data->clk_frequency,
+			machine_data->codec_clk_direction);
+		if (ret < 0) {
+			dev_err(substream->pcm->card->dev,
+				"could not set codec driver clock params\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * Remove the sound device
+ *
+ * This function is called to remove the sound device for one SSI.  We
+ * de-program the DMACR and PMUXCR register.
+ */
+int mpc8610_hpcd_machine_remove(struct platform_device *sound_device)
+{
+	struct mpc8610hpcd_data *machine_data = sound_device->dev.platform_data;
+
+	/* Restore the signal routing */
+
+	guts_set_dmacr(machine_data->guts, machine_data->dma_id + 1,
+		machine_data->dma_channel_id[0], 0);
+	guts_set_dmacr(machine_data->guts, machine_data->dma_id + 1,
+		machine_data->dma_channel_id[1], 0);
+
+	switch (machine_data->ssi_id) {
+	case 0:
+		clrsetbits_be32(&machine_data->guts->pmuxcr,
+			CCSR_GUTS_PMUXCR_SSI1_MASK, CCSR_GUTS_PMUXCR_SSI1_LA);
+		break;
+	case 1:
+		clrsetbits_be32(&machine_data->guts->pmuxcr,
+			CCSR_GUTS_PMUXCR_SSI2_MASK, CCSR_GUTS_PMUXCR_SSI1_LA);
+		break;
+	}
+
+	return 0;
+}
+
+/*
+ * ASoC fabric driver operations
+ */
+static struct snd_soc_ops mpc8610hpcd_ops = {
+	.startup = mpc8610hpcd_startup,
+};
+
+/*
+ * ASoC machine
+ */
+static struct snd_soc_machine mpc8610_hpcd_machine = {
+	.probe = mpc8610_hpcd_machine_probe,
+	.remove = mpc8610_hpcd_machine_remove,
+	.name = "MPC8610 HPCD",
+	.num_links = 1,
+};
+
+/*
+ * Probe function for the fabric driver driver.
+ *
+ * This function gets called when an SSI node is found in the device tree.
+ *
+ * Although this is a fabric driver, the SSI node is the "master" node with
+ * respect to audio hardware connections.  Therefore, we create a new ASoC
+ * device for each new SSI node that has a codec attached.
+ *
+ * FIXME: Currently, we only support one DMA controller, so if there are
+ * multiple SSI nodes with codecs, only the first will be supported.
+ *
+ * FIXME: Even if we did support multiple DMA controllers, we have no
+ * mechanism for assigning DMA controllers and channels to the individual
+ * SSI devices.  We also probably aren't compatible with the generic Elo DMA
+ * device driver.
+ */
+static int mpc8610_hpcd_probe(struct of_device *ofdev,
+	const struct of_device_id *match)
+{
+	struct device_node *np = ofdev->node;
+	struct device_node *codec_np = NULL;
+	struct device_node *guts_np = NULL;
+	struct device_node *dma_np = NULL;
+	struct device_node *dma_channel_np = NULL;
+	const char *sprop;
+	const u32 *iprop;
+	struct resource res;
+	struct platform_device *sound_device = NULL;
+	struct mpc8610hpcd_data *machine_data;
+	struct fsl_ssi_info ssi_info;
+	struct fsl_dma_info dma_info;
+	int ret = -ENODEV;
+
+	machine_data = kzalloc(sizeof(struct mpc8610hpcd_data), GFP_KERNEL);
+	if (!machine_data)
+		return -ENOMEM;
+
+	memset(&ssi_info, 0, sizeof(ssi_info));
+	memset(&dma_info, 0, sizeof(dma_info));
+
+	ssi_info.dev = &ofdev->dev;
+
+	/*
+	 * We are only interested in SSIs with a codec child node in them, so
+	 * let's make sure this SSI has one.
+	 */
+	while ((codec_np = of_get_next_child(np, codec_np)) != NULL) {
+		if (strcmp(codec_np->name, "codec") == 0) {
+			/* Most drivers forget the final of_node_put() call */
+			of_node_put(codec_np);
+			break;
+		}
+	}
+
+	if (!codec_np)
+		goto error;
+
+	/* The MPC8610 HPCD only knows about the CS4270 codec, so reject
+	   anything else. */
+	if (!of_device_is_compatible(codec_np, "cirrus,cs4270"))
+		goto error;
+
+	/* Get the device ID */
+	iprop = of_get_property(np, "cell-index", NULL);
+	if (!iprop) {
+		dev_err(&ofdev->dev, "cell-index property not found\n");
+		ret = -EINVAL;
+		goto error;
+	}
+	machine_data->ssi_id = *iprop;
+	ssi_info.id = *iprop;
+
+	/* Get the serial format and clock direction. */
+	sprop = of_get_property(np, "fsl,mode", NULL);
+	if (!sprop) {
+		dev_err(&ofdev->dev, "fsl,mode property not found\n");
+		ret = -EINVAL;
+		goto error;
+	}
+
+	if (strcasecmp(sprop, "i2s-slave") == 0) {
+		machine_data->dai_format = SND_SOC_DAIFMT_I2S;
+		machine_data->codec_clk_direction = SND_SOC_CLOCK_OUT;
+		machine_data->cpu_clk_direction = SND_SOC_CLOCK_IN;
+
+		/* In i2s-slave mode, the codec has its own clock source, so
+		   we need to get the frequency from the device tree and pass
+		   it to the codec driver. */
+		iprop = of_get_property(codec_np, "bus-frequency", NULL);
+		if (!iprop || !*iprop) {
+			dev_err(&ofdev->dev, "codec bus-frequency property "
+				"is missing or invalid\n");
+			ret = -EINVAL;
+			goto error;
+		}
+		machine_data->clk_frequency = *iprop;
+	} else if (strcasecmp(sprop, "i2s-master") == 0) {
+		machine_data->dai_format = SND_SOC_DAIFMT_I2S;
+		machine_data->codec_clk_direction = SND_SOC_CLOCK_IN;
+		machine_data->cpu_clk_direction = SND_SOC_CLOCK_OUT;
+	} else if (strcasecmp(sprop, "lj-slave") == 0) {
+		machine_data->dai_format = SND_SOC_DAIFMT_LEFT_J;
+		machine_data->codec_clk_direction = SND_SOC_CLOCK_OUT;
+		machine_data->cpu_clk_direction = SND_SOC_CLOCK_IN;
+	} else if (strcasecmp(sprop, "lj-master") == 0) {
+		machine_data->dai_format = SND_SOC_DAIFMT_LEFT_J;
+		machine_data->codec_clk_direction = SND_SOC_CLOCK_IN;
+		machine_data->cpu_clk_direction = SND_SOC_CLOCK_OUT;
+	} else if (strcasecmp(sprop, "rj-master") == 0) {
+		machine_data->dai_format = SND_SOC_DAIFMT_RIGHT_J;
+		machine_data->codec_clk_direction = SND_SOC_CLOCK_OUT;
+		machine_data->cpu_clk_direction = SND_SOC_CLOCK_IN;
+	} else if (strcasecmp(sprop, "rj-master") == 0) {
+		machine_data->dai_format = SND_SOC_DAIFMT_RIGHT_J;
+		machine_data->codec_clk_direction = SND_SOC_CLOCK_IN;
+		machine_data->cpu_clk_direction = SND_SOC_CLOCK_OUT;
+	} else if (strcasecmp(sprop, "ac97-slave") == 0) {
+		machine_data->dai_format = SND_SOC_DAIFMT_AC97;
+		machine_data->codec_clk_direction = SND_SOC_CLOCK_OUT;
+		machine_data->cpu_clk_direction = SND_SOC_CLOCK_IN;
+	} else if (strcasecmp(sprop, "ac97-master") == 0) {
+		machine_data->dai_format = SND_SOC_DAIFMT_AC97;
+		machine_data->codec_clk_direction = SND_SOC_CLOCK_IN;
+		machine_data->cpu_clk_direction = SND_SOC_CLOCK_OUT;
+	} else {
+		dev_err(&ofdev->dev,
+			"unrecognized fsl,mode property \"%s\"\n", sprop);
+		ret = -EINVAL;
+		goto error;
+	}
+
+	if (!machine_data->clk_frequency) {
+		dev_err(&ofdev->dev, "unknown clock frequency\n");
+		ret = -EINVAL;
+		goto error;
+	}
+
+	/* Read the SSI information from the device tree */
+	ret = of_address_to_resource(np, 0, &res);
+	if (ret) {
+		dev_err(&ofdev->dev, "could not obtain SSI address\n");
+		goto error;
+	}
+	if (!res.start) {
+		dev_err(&ofdev->dev, "invalid SSI address\n");
+		goto error;
+	}
+	ssi_info.ssi_phys = res.start;
+
+	machine_data->ssi = ioremap(ssi_info.ssi_phys, sizeof(struct ccsr_ssi));
+	if (!machine_data->ssi) {
+		dev_err(&ofdev->dev, "could not map SSI address %x\n",
+			ssi_info.ssi_phys);
+		ret = -EINVAL;
+		goto error;
+	}
+	ssi_info.ssi = machine_data->ssi;
+
+
+	/* Get the IRQ of the SSI */
+	machine_data->ssi_irq = irq_of_parse_and_map(np, 0);
+	if (!machine_data->ssi_irq) {
+		dev_err(&ofdev->dev, "could not get SSI IRQ\n");
+		ret = -EINVAL;
+		goto error;
+	}
+	ssi_info.irq = machine_data->ssi_irq;
+
+
+	/* Map the global utilities registers. */
+	guts_np = of_find_compatible_node(NULL, NULL, "fsl,mpc8610-guts");
+	if (!guts_np) {
+		dev_err(&ofdev->dev, "could not obtain address of GUTS\n");
+		ret = -EINVAL;
+		goto error;
+	}
+	machine_data->guts = of_iomap(guts_np, 0);
+	of_node_put(guts_np);
+	if (!machine_data->guts) {
+		dev_err(&ofdev->dev, "could not map GUTS\n");
+		ret = -EINVAL;
+		goto error;
+	}
+
+	/* Find the DMA channels to use.  For now, we always use the first DMA
+	   controller. */
+	for_each_compatible_node(dma_np, NULL, "fsl,mpc8610-dma") {
+		iprop = of_get_property(dma_np, "cell-index", NULL);
+		if (iprop && (*iprop == 0)) {
+			of_node_put(dma_np);
+			break;
+		}
+	}
+	if (!dma_np) {
+		dev_err(&ofdev->dev, "could not find DMA node\n");
+		ret = -EINVAL;
+		goto error;
+	}
+	machine_data->dma_id = *iprop;
+
+	/*
+	 * Find the DMA channels to use.  For now, we always use DMA channel 0
+	 * for playback, and DMA channel 1 for capture.
+	 */
+	while ((dma_channel_np = of_get_next_child(dma_np, dma_channel_np))) {
+		iprop = of_get_property(dma_channel_np, "cell-index", NULL);
+		/* Is it DMA channel 0? */
+		if (iprop && (*iprop == 0)) {
+			/* dma_channel[0] and dma_irq[0] are for playback */
+			dma_info.dma_channel[0] = of_iomap(dma_channel_np, 0);
+			dma_info.dma_irq[0] =
+				irq_of_parse_and_map(dma_channel_np, 0);
+			machine_data->dma_channel_id[0] = *iprop;
+			continue;
+		}
+		if (iprop && (*iprop == 1)) {
+			/* dma_channel[1] and dma_irq[1] are for capture */
+			dma_info.dma_channel[1] = of_iomap(dma_channel_np, 0);
+			dma_info.dma_irq[1] =
+				irq_of_parse_and_map(dma_channel_np, 0);
+			machine_data->dma_channel_id[1] = *iprop;
+			continue;
+		}
+	}
+	if (!dma_info.dma_channel[0] || !dma_info.dma_channel[1] ||
+	    !dma_info.dma_irq[0] || !dma_info.dma_irq[1]) {
+		dev_err(&ofdev->dev, "could not find DMA channels\n");
+		ret = -EINVAL;
+		goto error;
+	}
+
+	dma_info.ssi_stx_phys = ssi_info.ssi_phys +
+		offsetof(struct ccsr_ssi, stx0);
+	dma_info.ssi_srx_phys = ssi_info.ssi_phys +
+		offsetof(struct ccsr_ssi, srx0);
+
+	/* We have the DMA information, so tell the DMA driver what it is */
+	if (!fsl_dma_configure(&dma_info)) {
+		dev_err(&ofdev->dev, "could not instantiate DMA device\n");
+		ret = -EBUSY;
+		goto error;
+	}
+
+	/*
+	 * Initialize our DAI data structure.  We should probably get this
+	 * information from the device tree.
+	 */
+	machine_data->dai.name = "CS4270";
+	machine_data->dai.stream_name = "CS4270";
+
+	machine_data->dai.cpu_dai = fsl_ssi_create_dai(&ssi_info);
+	machine_data->dai.codec_dai = &cs4270_dai; /* The codec_dai we want */
+	machine_data->dai.ops = &mpc8610hpcd_ops;
+
+	mpc8610_hpcd_machine.dai_link = &machine_data->dai;
+
+	/* Allocate a new audio platform device structure */
+	sound_device = platform_device_alloc("soc-audio", -1);
+	if (!sound_device) {
+		dev_err(&ofdev->dev, "platform device allocation failed\n");
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	machine_data->sound_devdata.machine = &mpc8610_hpcd_machine;
+	machine_data->sound_devdata.codec_dev = &soc_codec_device_cs4270;
+	machine_data->sound_devdata.platform = &fsl_soc_platform;
+
+	sound_device->dev.platform_data = machine_data;
+
+
+	/* Set the platform device and ASoC device to point to each other */
+	platform_set_drvdata(sound_device, &machine_data->sound_devdata);
+
+	machine_data->sound_devdata.dev = &sound_device->dev;
+
+
+	/* Tell ASoC to probe us.  This will call mpc8610_hpcd_machine.probe(),
+	   if it exists. */
+	ret = platform_device_add(sound_device);
+
+	if (ret) {
+		dev_err(&ofdev->dev, "platform device add failed\n");
+		goto error;
+	}
+
+	dev_set_drvdata(&ofdev->dev, sound_device);
+
+	return 0;
+
+error:
+	if (sound_device)
+		platform_device_unregister(sound_device);
+
+	if (machine_data->dai.cpu_dai)
+		fsl_ssi_destroy_dai(machine_data->dai.cpu_dai);
+
+	if (ssi_info.ssi)
+		iounmap(ssi_info.ssi);
+
+	if (ssi_info.irq)
+		irq_dispose_mapping(ssi_info.irq);
+
+	if (dma_info.dma_channel[0])
+		iounmap(dma_info.dma_channel[0]);
+
+	if (dma_info.dma_channel[1])
+		iounmap(dma_info.dma_channel[1]);
+
+	if (dma_info.dma_irq[0])
+		irq_dispose_mapping(dma_info.dma_irq[0]);
+
+	if (dma_info.dma_irq[1])
+		irq_dispose_mapping(dma_info.dma_irq[1]);
+
+	if (machine_data->guts)
+		iounmap(machine_data->guts);
+
+	kfree(machine_data);
+
+	return ret;
+}
+
+static int mpc8610_hpcd_remove(struct of_device *ofdev)
+{
+	struct platform_device *sound_device = dev_get_drvdata(&ofdev->dev);
+	struct mpc8610hpcd_data *machine_data = sound_device->dev.platform_data;
+
+	platform_device_unregister(sound_device);
+
+	if (machine_data->dai.cpu_dai)
+		fsl_ssi_destroy_dai(machine_data->dai.cpu_dai);
+
+	if (machine_data->ssi)
+		iounmap(machine_data->ssi);
+
+	if (machine_data->dma[0])
+		iounmap(machine_data->dma[0]);
+
+	if (machine_data->dma[1])
+		iounmap(machine_data->dma[1]);
+
+	if (machine_data->dma_irq[0])
+		irq_dispose_mapping(machine_data->dma_irq[0]);
+
+	if (machine_data->dma_irq[1])
+		irq_dispose_mapping(machine_data->dma_irq[1]);
+
+	if (machine_data->guts)
+		iounmap(machine_data->guts);
+
+	kfree(machine_data);
+	sound_device->dev.platform_data = NULL;
+
+	dev_set_drvdata(&ofdev->dev, NULL);
+
+	return 0;
+}
+
+static struct of_device_id mpc8610_hpcd_match[] = {
+	{
+		.compatible = "fsl,ssi",
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, mpc8610_hpcd_match);
+
+static struct of_platform_driver mpc8610_hpcd_of_driver = {
+	.owner  	= THIS_MODULE,
+	.name   	= "mpc8610_hpcd",
+	.match_table    = mpc8610_hpcd_match,
+	.probe  	= mpc8610_hpcd_probe,
+	.remove 	= mpc8610_hpcd_remove,
+};
+
+/**
+ * Fabric driver initialization.
+ *
+ * This function is called when this module is loaded.
+ */
+static int __init mpc8610hpcd_init(void)
+{
+	int ret;
+
+	printk(KERN_INFO "Freescale MPC8610 HPCD ALSA SoC fabric driver\n");
+
+	ret = of_register_platform_driver(&mpc8610_hpcd_of_driver);
+
+	if (ret)
+		printk(KERN_ERR
+			"mpc8610-hpcd: failed to register platform driver\n");
+
+	return ret;
+}
+
+/**
+ * Fabric driver exit
+ *
+ * This function is called when this driver is unloaded.
+ */
+static void __exit mpc8610hpcd_exit(void)
+{
+	of_unregister_platform_driver(&mpc8610_hpcd_of_driver);
+}
+
+module_init(mpc8610hpcd_init);
+module_exit(mpc8610hpcd_exit);
+
+MODULE_AUTHOR("Timur Tabi <timur@freescale.com>");
+MODULE_DESCRIPTION("Freescale MPC8610 HPCD ALSA SoC fabric driver");
+MODULE_LICENSE("GPL");
-- 
1.5.2.4

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20  0:03 [PATCH] ASoC drivers for the Freescale MPC8610 SoC Timur Tabi
@ 2007-12-20  4:06 ` Olof Johansson
  2007-12-20 14:24   ` Timur Tabi
  2007-12-20 14:47 ` Jon Loeliger
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 89+ messages in thread
From: Olof Johansson @ 2007-12-20  4:06 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

Hi,

This is a fairly substantial driver to get through, but here are some
initial comments on some of the simpler stuff:


On Wed, Dec 19, 2007 at 06:03:09PM -0600, Timur Tabi wrote:
> This patch adds ALSA SoC device drivers for the Freescale MPC8610 SoC
> and the MPC8610-HPCD reference board.

[...]

> diff --git a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
> index 6390895..6e1bde3 100644
> --- a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
> +++ b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
> @@ -34,9 +34,27 @@
>  
>  #include <asm/mpic.h>
>  
> +#include <asm/of_platform.h>
>  #include <sysdev/fsl_pci.h>
>  #include <sysdev/fsl_soc.h>
>  
> +static struct of_device_id mpc8610_ids[] = {
> +	{ .type = "soc", },
> +	{}

Please scan based on compatible instead of device_type.

> diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
> new file mode 100644
> index 0000000..4a5bbd2
> --- /dev/null
> +++ b/sound/soc/fsl/Kconfig
> @@ -0,0 +1,21 @@
> +menu "ALSA SoC audio for Freescale SOCs"
> +
> +config SND_SOC_MPC8610
> +	bool "ALSA SoC support for the MPC8610 SOC"
> +	depends on SND_SOC #&& MPC8610_HPCD
> +	default y #if MPC8610
> +	help
> +	  Say Y if you want to add support for codecs attached to the SSI
> +          device on an MPC8610.

Don't default configs to 'y'. Also, what's with the commented-out
dependencies and if?

> +config SND_SOC_MPC8610_HPCD
> +	# ALSA SoC support for Freescale MPC8610HPCD
> +	bool "ALSA SoC support for the Freescale MPC8610 HPCD board"
> +	depends on SND_SOC_MPC8610
> +	select SND_SOC_CS4270
> +	select SND_SOC_CS4270_VD33_ERRATA
> +	default y #if MPC8610_HPCD
> +	help
> +	  Say Y if you want to enable audio on the Freescale MPC8610 HPCD.

Same here w.r.t. defaults and dependencies.

> diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c
> new file mode 100644
> index 0000000..6b86be0
> --- /dev/null
> +++ b/sound/soc/fsl/fsl_dma.c
> @@ -0,0 +1,819 @@
> +/*
> + * Freescale DMA ALSA SoC PCM driver
> + *
> + * Author: Timur Tabi <timur@freescale.com>
> + *
> + * Copyright 2007 Freescale Semiconductor, Inc.  This file is licensed under
> + * the terms of the GNU General Public License version 2.  This program
> + * is licensed "as is" without any warranty of any kind, whether express
> + * or implied.
> + *
> + * This driver implements ASoC support for the Elo DMA controller, which is
> + * the DMA controller on Freescale 83xx, 85xx, and 86xx SOCs. In ALSA terms,
> + * the PCM driver is what handles the DMA buffer.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +
> +#include <sound/driver.h>
> +#include <sound/core.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +
> +#include <asm/io.h>
> +
> +#include "fsl_dma.h"
> +
> +/*
> + * The formats that the DMA controller supports, which is anything
> + * that is 8, 16, or 32 bits.
> + */
> +#define FSLDMA_PCM_FORMATS (SNDRV_PCM_FMTBIT_S8 	| \
> +			    SNDRV_PCM_FMTBIT_U8 	| \
> +			    SNDRV_PCM_FMTBIT_S16_LE     | \
> +			    SNDRV_PCM_FMTBIT_S16_BE     | \
> +			    SNDRV_PCM_FMTBIT_U16_LE     | \
> +			    SNDRV_PCM_FMTBIT_U16_BE     | \
> +			    SNDRV_PCM_FMTBIT_S24_LE     | \
> +			    SNDRV_PCM_FMTBIT_S24_BE     | \
> +			    SNDRV_PCM_FMTBIT_U24_LE     | \
> +			    SNDRV_PCM_FMTBIT_U24_BE     | \
> +			    SNDRV_PCM_FMTBIT_S32_LE     | \
> +			    SNDRV_PCM_FMTBIT_S32_BE     | \
> +			    SNDRV_PCM_FMTBIT_U32_LE     | \
> +			    SNDRV_PCM_FMTBIT_U32_BE)
> +
> +#define FSLDMA_PCM_RATES (SNDRV_PCM_RATE_5512 | SNDRV_PCM_RATE_8000_192000 | \
> +			  SNDRV_PCM_RATE_CONTINUOUS)
> +
> +/* DMA global data.  This structure is used by fsl_dma_open() to determine
> + * which DMA channels to assign to a substream.  Unfortunately, ASoC V1 does
> + * not allow the machine driver to provide this information to the PCM
> + * driver in advance, and there's no way to differentiate between the two
> + * DMA controllers.  So for now, this driver only supports one SSI device
> + * using two DMA channels.  We cannot support multiple DMA devices.
> + *
> + * ssi_stx_phys: bus address of SSI STX register
> + * ssi_srx_phys: bus address of SSI SRX register
> + * dma_channel: pointer to the DMA channel's registers
> + * irq: IRQ for this DMA channel
> + * assigned: set to 1 if that DMA channel is assigned to a substream
> + */

This goes for the whole patch: You've got good documentation, but it's
not in docbook format. Please reformat it since it should be a pretty
simple thing to do.

> +/*
> + * Initialize this PCM driver.
> + *
> + * This function is called when the codec driver calls snd_soc_new_pcms(),
> + * once for each .dai_link in the machine driver's snd_soc_machine
> + * structure.
> + */
> +static int fsl_dma_new(struct snd_card *card, struct snd_soc_codec_dai *dai,
> +	struct snd_pcm *pcm)
> +{
> +	static u64 fsl_dma_dmamask = 0xffffffff;
> +	int ret;
> +
> +	if (!card->dev->dma_mask)
> +		card->dev->dma_mask = &fsl_dma_dmamask;

I haven't read how your channel allocation works, but providing a
pointer to a local static variable is a bit fishy no matter what.

> +	/* Find the DMA channels to use.  For now, we always use the first DMA
> +	   controller. */
> +	for_each_compatible_node(dma_np, NULL, "fsl,mpc8610-dma") {
> +		iprop = of_get_property(dma_np, "cell-index", NULL);
> +		if (iprop && (*iprop == 0)) {
> +			of_node_put(dma_np);
> +			break;
> +		}
> +	}

Do you ever anticipate having other dma users on the system, such as
memcpy offload? You'll probably need allocation support for channels
when that day comes (I ended up writing a simple library for dma channel
management for that very reason on my platform).


-Olof

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20 14:24   ` Timur Tabi
@ 2007-12-20 13:54     ` Takashi Iwai
  2007-12-20 17:04       ` Timur Tabi
  2007-12-21  5:28       ` Lee Revell
  2007-12-20 22:39     ` Olof Johansson
  1 sibling, 2 replies; 89+ messages in thread
From: Takashi Iwai @ 2007-12-20 13:54 UTC (permalink / raw)
  To: Timur Tabi; +Cc: Olof Johansson, linuxppc-dev, alsa-devel

At Thu, 20 Dec 2007 08:24:35 -0600,
Timur Tabi wrote:
> 
> >> +static int fsl_dma_new(struct snd_card *card, struct snd_soc_codec_dai *dai,
> >> +	struct snd_pcm *pcm)
> >> +{
> >> +	static u64 fsl_dma_dmamask = 0xffffffff;
> >> +	int ret;
> >> +
> >> +	if (!card->dev->dma_mask)
> >> +		card->dev->dma_mask = &fsl_dma_dmamask;
> > 
> > I haven't read how your channel allocation works, but providing a
> > pointer to a local static variable is a bit fishy no matter what.
> 
> I just copied this code from another module.  All the ALSA drivers do this, 

All?  No, only a few...
For PCI, usually pci_set_dma_mask() and pci_set_consistent_dma_mask()
are used, of course.


Takashi

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20  4:06 ` Olof Johansson
@ 2007-12-20 14:24   ` Timur Tabi
  2007-12-20 13:54     ` [alsa-devel] " Takashi Iwai
  2007-12-20 22:39     ` Olof Johansson
  0 siblings, 2 replies; 89+ messages in thread
From: Timur Tabi @ 2007-12-20 14:24 UTC (permalink / raw)
  To: Olof Johansson; +Cc: linuxppc-dev, alsa-devel

Olof Johansson wrote:

>> +static struct of_device_id mpc8610_ids[] = {
>> +	{ .type = "soc", },
>> +	{}
> 
> Please scan based on compatible instead of device_type.

I was just following the example from another board file.  However, the 'soc' 
node in the device tree does not have a compatible property, so I don't how to 
change this.

>> +config SND_SOC_MPC8610
>> +	bool "ALSA SoC support for the MPC8610 SOC"
>> +	depends on SND_SOC #&& MPC8610_HPCD
>> +	default y #if MPC8610
>> +	help
>> +	  Say Y if you want to add support for codecs attached to the SSI
>> +          device on an MPC8610.
> 
> Don't default configs to 'y'. Also, what's with the commented-out
> dependencies and if?

Sorry, that was a development change that I forgot to put back.  The "y #" 
should be deleted.


>> + * ssi_stx_phys: bus address of SSI STX register
>> + * ssi_srx_phys: bus address of SSI SRX register
>> + * dma_channel: pointer to the DMA channel's registers
>> + * irq: IRQ for this DMA channel
>> + * assigned: set to 1 if that DMA channel is assigned to a substream
>> + */
> 
> This goes for the whole patch: You've got good documentation, but it's
> not in docbook format. Please reformat it since it should be a pretty
> simple thing to do.

Ok.

>> +static int fsl_dma_new(struct snd_card *card, struct snd_soc_codec_dai *dai,
>> +	struct snd_pcm *pcm)
>> +{
>> +	static u64 fsl_dma_dmamask = 0xffffffff;
>> +	int ret;
>> +
>> +	if (!card->dev->dma_mask)
>> +		card->dev->dma_mask = &fsl_dma_dmamask;
> 
> I haven't read how your channel allocation works, but providing a
> pointer to a local static variable is a bit fishy no matter what.

I just copied this code from another module.  All the ALSA drivers do this, 
but I'll look into it and see if it can't be done different.  I make no 
promises, though!

> Do you ever anticipate having other dma users on the system, such as
> memcpy offload? You'll probably need allocation support for channels
> when that day comes (I ended up writing a simple library for dma channel
> management for that very reason on my platform).

Yes, I plan on updating this driver to work with the standard Freescale "Elo" 
device driver, but that will have to wait until that code is in the kernel and 
stabilized.  The SSI is limited in which DMA channels it can use, anyway.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20  0:03 [PATCH] ASoC drivers for the Freescale MPC8610 SoC Timur Tabi
  2007-12-20  4:06 ` Olof Johansson
@ 2007-12-20 14:47 ` Jon Loeliger
  2007-12-20 22:29 ` Jon Smirl
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 89+ messages in thread
From: Jon Loeliger @ 2007-12-20 14:47 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

So, like, the other day Timur Tabi mumbled:
> diff --git a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c b/arch/powerpc/platforms/86xx/mpc8610_hpc
> d.c
> index 6390895..6e1bde3 100644
> --- a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
> +++ b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
> @@ -34,9 +34,27 @@
>  
>  #include <asm/mpic.h>
>  
> +#include <asm/of_platform.h>
>  #include <sysdev/fsl_pci.h>
>  #include <sysdev/fsl_soc.h>


Please dont' re-introduce these.  I just submitted patches to
move all of the PowerPC instances to using <linux/of_platform.h>



> diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c
> new file mode 100644
> index 0000000..5421a5c
> --- /dev/null
> +++ b/sound/soc/fsl/mpc8610_hpcd.c
> @@ -0,0 +1,621 @@
> +/**
> + * Freescale MPC8610HPCD ALSA SoC Fabric driver
> + *
> + * Author: Timur Tabi <timur@freescale.com>
> + *
> + * Copyright 2007 Freescale Semiconductor, Inc.  This file is licensed under
> + * the terms of the GNU General Public License version 2.  This program
> + * is licensed "as is" without any warranty of any kind, whether express
> + * or implied.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/version.h>
> +#include <linux/kernel.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <asm/of_device.h>
> +#include <asm/of_platform.h>

Again, this should be moved up as <linux/of_platform.h>.


Thanks,
jdl

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20 13:54     ` [alsa-devel] " Takashi Iwai
@ 2007-12-20 17:04       ` Timur Tabi
  2007-12-21  5:28       ` Lee Revell
  1 sibling, 0 replies; 89+ messages in thread
From: Timur Tabi @ 2007-12-20 17:04 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Olof Johansson, linuxppc-dev, alsa-devel

Takashi Iwai wrote:

> All?  No, only a few...
> For PCI, usually pci_set_dma_mask() and pci_set_consistent_dma_mask()
> are used, of course.

Hmm, ok I was wrong.  I took this code from the ASoC at91 driver.

Unfortunately, I don't understand what this code is trying to do.  The AT91 
driver isn't documented, so I don't even know if I need it.  Can someone explain 
what all this is?  What's the alternative to using a static global?

-- 
Timur Tabi
Linux kernel developer at Freescale

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20  0:03 [PATCH] ASoC drivers for the Freescale MPC8610 SoC Timur Tabi
  2007-12-20  4:06 ` Olof Johansson
  2007-12-20 14:47 ` Jon Loeliger
@ 2007-12-20 22:29 ` Jon Smirl
  2007-12-20 22:32   ` Timur Tabi
  2008-01-01 17:25 ` Jon Smirl
  2008-01-02  4:27 ` Jon Smirl
  4 siblings, 1 reply; 89+ messages in thread
From: Jon Smirl @ 2007-12-20 22:29 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On 12/19/07, Timur Tabi <timur@freescale.com> wrote:

> +static struct of_device_id mpc8610_ids[] = {
> +       { .type = "soc", },
> +       {}
> +};
> +
> +static int __init mpc8610_declare_of_platform_devices(void)
> +{
> +       if (!machine_is(mpc86xx_hpcd))
> +               return 0;
> +
> +       /* Without this call, the SSI device driver won't get probed. */
> +       of_platform_bus_probe(NULL, mpc8610_ids, NULL);
> +
> +       return 0;
> +}
> +device_initcall(mpc8610_declare_of_platform_devices);

How is of_platform_bus_probe() supposed to be called? mpc5200/virtex
call it with three NULLs. Is it necessary to name all of the buses in
a of_device_id? If it's not necessary to list the buses the
of_platform_bus_probe() call could be moved to common code.

Are these buses?
        { .compatible = "ibm,plb4", },
        { .compatible = "ibm,opb", },
        { .compatible = "ibm,ebc", },

Could of_platform_bus_probe() be simplified? No one uses the first and
third parameters.

-- 
Jon Smirl
jonsmirl@gmail.com

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20 22:29 ` Jon Smirl
@ 2007-12-20 22:32   ` Timur Tabi
  2007-12-20 22:38     ` Jon Smirl
  0 siblings, 1 reply; 89+ messages in thread
From: Timur Tabi @ 2007-12-20 22:32 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel

Jon Smirl wrote:

> How is of_platform_bus_probe() supposed to be called? mpc5200/virtex
> call it with three NULLs. Is it necessary to name all of the buses in
> a of_device_id? If it's not necessary to list the buses the
> of_platform_bus_probe() call could be moved to common code.

I added the above code because it is the only way I could get my SSI nodes to be 
probed.  If there's a better way to do it, I'm all ears.  I just copied that 
code from the mpc836x_mds.c platform file.

> Are these buses?
>         { .compatible = "ibm,plb4", },
>         { .compatible = "ibm,opb", },
>         { .compatible = "ibm,ebc", },

I have no idea.

> Could of_platform_bus_probe() be simplified? No one uses the first and
> third parameters.

Maybe, but that's not a discussion for this thread!

-- 
Timur Tabi
Linux kernel developer at Freescale

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20 22:39     ` Olof Johansson
@ 2007-12-20 22:37       ` Timur Tabi
  2007-12-20 22:43         ` Scott Wood
  0 siblings, 1 reply; 89+ messages in thread
From: Timur Tabi @ 2007-12-20 22:37 UTC (permalink / raw)
  To: Olof Johansson; +Cc: linuxppc-dev, alsa-devel

Olof Johansson wrote:

>> I was just following the example from another board file.  However, the 'soc' 
>> node in the device tree does not have a compatible property, so I don't how to 
>> change this.
> 
> Then add an appropriate compatible entry to it, please.

None of the SOC nodes in any DTS have a "compatible" entry.  I understand the 
issue, but you're asking me to fix a larger problem, one that is beyond the 
scope of this patch.  You're saying that *all* SOC needs are incorrectly defined 
and need to be probed differently.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20 22:32   ` Timur Tabi
@ 2007-12-20 22:38     ` Jon Smirl
  2007-12-20 22:40       ` Timur Tabi
  0 siblings, 1 reply; 89+ messages in thread
From: Jon Smirl @ 2007-12-20 22:38 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On 12/20/07, Timur Tabi <timur@freescale.com> wrote:
> Jon Smirl wrote:
>
> > How is of_platform_bus_probe() supposed to be called? mpc5200/virtex
> > call it with three NULLs. Is it necessary to name all of the buses in
> > a of_device_id? If it's not necessary to list the buses the
> > of_platform_bus_probe() call could be moved to common code.
>
> I added the above code because it is the only way I could get my SSI nodes to be
> probed.  If there's a better way to do it, I'm all ears.  I just copied that
> code from the mpc836x_mds.c platform file.

mpc5200 does it like this:
of_platform_bus_probe(NULL, NULL, NULL);

No need for the ids.


>
> > Are these buses?
> >         { .compatible = "ibm,plb4", },
> >         { .compatible = "ibm,opb", },
> >         { .compatible = "ibm,ebc", },
>
> I have no idea.
>
> > Could of_platform_bus_probe() be simplified? No one uses the first and
> > third parameters.
>
> Maybe, but that's not a discussion for this thread!
>
> --
> Timur Tabi
> Linux kernel developer at Freescale
>


-- 
Jon Smirl
jonsmirl@gmail.com

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20 14:24   ` Timur Tabi
  2007-12-20 13:54     ` [alsa-devel] " Takashi Iwai
@ 2007-12-20 22:39     ` Olof Johansson
  2007-12-20 22:37       ` Timur Tabi
  1 sibling, 1 reply; 89+ messages in thread
From: Olof Johansson @ 2007-12-20 22:39 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On Thu, Dec 20, 2007 at 08:24:35AM -0600, Timur Tabi wrote:
> Olof Johansson wrote:
> 
> >> +static struct of_device_id mpc8610_ids[] = {
> >> +	{ .type = "soc", },
> >> +	{}
> > 
> > Please scan based on compatible instead of device_type.
> 
> I was just following the example from another board file.  However, the 'soc' 
> node in the device tree does not have a compatible property, so I don't how to 
> change this.

Then add an appropriate compatible entry to it, please.

> > Do you ever anticipate having other dma users on the system, such as
> > memcpy offload? You'll probably need allocation support for channels
> > when that day comes (I ended up writing a simple library for dma channel
> > management for that very reason on my platform).
> 
> Yes, I plan on updating this driver to work with the standard Freescale "Elo" 
> device driver, but that will have to wait until that code is in the kernel and 
> stabilized.  The SSI is limited in which DMA channels it can use, anyway.

Ok.


-Olof

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20 22:38     ` Jon Smirl
@ 2007-12-20 22:40       ` Timur Tabi
  2007-12-20 22:44         ` Scott Wood
  0 siblings, 1 reply; 89+ messages in thread
From: Timur Tabi @ 2007-12-20 22:40 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel

Jon Smirl wrote:

> mpc5200 does it like this:
> of_platform_bus_probe(NULL, NULL, NULL);

I think that tells the OF base code to probe everything in the device tree, 
which is probably overkill.  I think fsl_soc.c covers most of the device tree, 
but the SSI is not defined in fsl_soc.c.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20 22:37       ` Timur Tabi
@ 2007-12-20 22:43         ` Scott Wood
  2007-12-23  2:58           ` Timur Tabi
  0 siblings, 1 reply; 89+ messages in thread
From: Scott Wood @ 2007-12-20 22:43 UTC (permalink / raw)
  To: Timur Tabi; +Cc: Olof Johansson, linuxppc-dev, alsa-devel

Timur Tabi wrote:
> Olof Johansson wrote:
> 
>>> I was just following the example from another board file.  However, the 'soc' 
>>> node in the device tree does not have a compatible property, so I don't how to 
>>> change this.
>> Then add an appropriate compatible entry to it, please.
> 
> None of the SOC nodes in any DTS have a "compatible" entry.

Not quite true; ep88xc, mpc8272ads, and pq2fads have them.

-Scott

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20 22:40       ` Timur Tabi
@ 2007-12-20 22:44         ` Scott Wood
  2007-12-20 23:13           ` Jon Smirl
  0 siblings, 1 reply; 89+ messages in thread
From: Scott Wood @ 2007-12-20 22:44 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

Timur Tabi wrote:
> Jon Smirl wrote:
> 
>> mpc5200 does it like this:
>> of_platform_bus_probe(NULL, NULL, NULL);
> 
> I think that tells the OF base code to probe everything in the device tree, 
> which is probably overkill.  I think fsl_soc.c covers most of the device tree, 
> but the SSI is not defined in fsl_soc.c.

Not quite; it tells it to use a built-in list of bus matches.  Most of 
which are device_type-based, FWIW.

-Scott

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20 22:44         ` Scott Wood
@ 2007-12-20 23:13           ` Jon Smirl
  2007-12-21  0:00             ` David Gibson
  0 siblings, 1 reply; 89+ messages in thread
From: Jon Smirl @ 2007-12-20 23:13 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, alsa-devel, Timur Tabi

On 12/20/07, Scott Wood <scottwood@freescale.com> wrote:
> Timur Tabi wrote:
> > Jon Smirl wrote:
> >
> >> mpc5200 does it like this:
> >> of_platform_bus_probe(NULL, NULL, NULL);
> >
> > I think that tells the OF base code to probe everything in the device tree,
> > which is probably overkill.  I think fsl_soc.c covers most of the device tree,
> > but the SSI is not defined in fsl_soc.c.
>
> Not quite; it tells it to use a built-in list of bus matches.  Most of
> which are device_type-based, FWIW.

Here's the default. Using NULL would work.

static struct of_device_id of_default_bus_ids[] = {
        { .type = "soc", },
        { .compatible = "soc", },
        { .type = "spider", },
        { .type = "axon", },
        { .type = "plb5", },
        { .type = "plb4", },
        { .type = "opb", },
        { .type = "ebc", },
        {},
};


>
> -Scott
>


-- 
Jon Smirl
jonsmirl@gmail.com

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20 23:13           ` Jon Smirl
@ 2007-12-21  0:00             ` David Gibson
  0 siblings, 0 replies; 89+ messages in thread
From: David Gibson @ 2007-12-21  0:00 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Scott Wood, linuxppc-dev, alsa-devel, Timur Tabi

On Thu, Dec 20, 2007 at 06:13:31PM -0500, Jon Smirl wrote:
> On 12/20/07, Scott Wood <scottwood@freescale.com> wrote:
> > Timur Tabi wrote:
> > > Jon Smirl wrote:
> > >
> > >> mpc5200 does it like this:
> > >> of_platform_bus_probe(NULL, NULL, NULL);
> > >
> > > I think that tells the OF base code to probe everything in the device tree,
> > > which is probably overkill.  I think fsl_soc.c covers most of the device tree,
> > > but the SSI is not defined in fsl_soc.c.
> >
> > Not quite; it tells it to use a built-in list of bus matches.  Most of
> > which are device_type-based, FWIW.
> 
> Here's the default. Using NULL would work.

It might work, but using the default list is discouraged.  Pass an
explicit list of match ids for the buses you need to scan instead (and
use compatible to match them, not device_type).

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20 13:54     ` [alsa-devel] " Takashi Iwai
  2007-12-20 17:04       ` Timur Tabi
@ 2007-12-21  5:28       ` Lee Revell
  2007-12-23  3:23         ` Timur Tabi
  1 sibling, 1 reply; 89+ messages in thread
From: Lee Revell @ 2007-12-21  5:28 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Olof Johansson, linuxppc-dev, alsa-devel, Timur Tabi

On Dec 20, 2007 8:54 AM, Takashi Iwai <tiwai@suse.de> wrote:
> At Thu, 20 Dec 2007 08:24:35 -0600,
> Timur Tabi wrote:
> >
> > >> +static int fsl_dma_new(struct snd_card *card, struct snd_soc_codec_dai *dai,
> > >> +  struct snd_pcm *pcm)
> > >> +{
> > >> +  static u64 fsl_dma_dmamask = 0xffffffff;
> > >> +  int ret;
> > >> +
> > >> +  if (!card->dev->dma_mask)
> > >> +          card->dev->dma_mask = &fsl_dma_dmamask;
> > >
> > > I haven't read how your channel allocation works, but providing a
> > > pointer to a local static variable is a bit fishy no matter what.
> >
> > I just copied this code from another module.  All the ALSA drivers do this,
>
> All?  No, only a few...
> For PCI, usually pci_set_dma_mask() and pci_set_consistent_dma_mask()
> are used, of course.

Timur,

Nicely commented driver!  I wish they were all like this ;-)

Please use DMA_32BIT_MASK (see include/linux/dma-mapping.h) instead of
0xffffffff.  I've personally fixed a heisenbug in an ALSA driver
caused by incorrectly typed DMA mask...

Lee

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20 22:43         ` Scott Wood
@ 2007-12-23  2:58           ` Timur Tabi
  2008-01-02 18:08             ` Scott Wood
  0 siblings, 1 reply; 89+ messages in thread
From: Timur Tabi @ 2007-12-23  2:58 UTC (permalink / raw)
  To: Scott Wood; +Cc: Olof Johansson, linuxppc-dev, alsa-devel

Scott Wood wrote:

>> None of the SOC nodes in any DTS have a "compatible" entry.
> 
> Not quite true; ep88xc, mpc8272ads, and pq2fads have them.

Ah ok.  So what should the compatible entry for 8641 be?

	compatible = "fsl,mpc8641"

That looks a lot like what a compatible entry for the CPU should be. 
How are we differentiating between the compatible cores and compatible SOCs?

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-21  5:28       ` Lee Revell
@ 2007-12-23  3:23         ` Timur Tabi
  0 siblings, 0 replies; 89+ messages in thread
From: Timur Tabi @ 2007-12-23  3:23 UTC (permalink / raw)
  To: Lee Revell; +Cc: Takashi Iwai, Olof Johansson, alsa-devel, linuxppc-dev

Lee Revell wrote:

> Please use DMA_32BIT_MASK (see include/linux/dma-mapping.h) instead of
> 0xffffffff. 

No prob.  But did you see this comment:

/*
  * NOTE: do not use the below macros in new code and do not add new 
definitions
  * here.
  *
  * Instead, just open-code DMA_BIT_MASK(n) within your driver
  */

So I guess I should use DMA_BIT_MASK(32) instead.

> I've personally fixed a heisenbug in an ALSA driver
> caused by incorrectly typed DMA mask...

Can you explain to me what all of this does?  Is it okay to use a static 
u64 variable?  Why do so many drivers do it that way?  I don't even know 
if 0xFFFFFFFF is the right number for my platform.

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20  0:03 [PATCH] ASoC drivers for the Freescale MPC8610 SoC Timur Tabi
                   ` (2 preceding siblings ...)
  2007-12-20 22:29 ` Jon Smirl
@ 2008-01-01 17:25 ` Jon Smirl
  2008-01-01 17:42   ` Jon Smirl
                     ` (2 more replies)
  2008-01-02  4:27 ` Jon Smirl
  4 siblings, 3 replies; 89+ messages in thread
From: Jon Smirl @ 2008-01-01 17:25 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On 12/19/07, Timur Tabi <timur@freescale.com> wrote:
> +               ssi@16000 {
> +                       compatible = "fsl,ssi";
> +                       cell-index = <0>;
> +                       reg = <16000 100>;
> +                       interrupt-parent = <&mpic>;
> +                       interrupts = <3e 2>;
> +                       fsl,mode = "i2s-slave";
> +                       codec {
> +                               compatible = "cirrus,cs4270";
> +                               /* MCLK source is a stand-alone oscillator */
> +                               bus-frequency = <bb8000>;
> +                       };
> +               };

Does this need to be bus-frequency? It's always called MCLK in all of
the literature.

In my case the MCLK comes from a chip on the i2c bus that is
programmable How would that be encoded?.

-- 
Jon Smirl
jonsmirl@gmail.com

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-01 17:25 ` Jon Smirl
@ 2008-01-01 17:42   ` Jon Smirl
  2008-01-02 15:19     ` Timur Tabi
  2008-01-02  0:26   ` David Gibson
  2008-01-02 15:10   ` Timur Tabi
  2 siblings, 1 reply; 89+ messages in thread
From: Jon Smirl @ 2008-01-01 17:42 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On 1/1/08, Jon Smirl <jonsmirl@gmail.com> wrote:
> On 12/19/07, Timur Tabi <timur@freescale.com> wrote:
> > +               ssi@16000 {
> > +                       compatible = "fsl,ssi";
> > +                       cell-index = <0>;
> > +                       reg = <16000 100>;
> > +                       interrupt-parent = <&mpic>;
> > +                       interrupts = <3e 2>;
> > +                       fsl,mode = "i2s-slave";
> > +                       codec {
> > +                               compatible = "cirrus,cs4270";
> > +                               /* MCLK source is a stand-alone oscillator */
> > +                               bus-frequency = <bb8000>;
> > +                       };
> > +               };
>
> Does this need to be bus-frequency? It's always called MCLK in all of
> the literature.
>
> In my case the MCLK comes from a chip on the i2c bus that is
> programmable How would that be encoded?.

Looking at the cs4270 codec driver it is controlled by i2c (supports
SPI too).  What happened to the conversation about putting codecs on
the controlling bus and then linking them to the data bus?

If that's the case the cs4270 should be in the i2c bus node (missing
currently) and then a link from the SSI bus would point to it.

cs4270 is still an old style i2c driver which is going to get
deprecated. It takes about thirty minutes to convert it to new style.
If was new style it could pick up its i2c address from the device tree
instead of searching for it.

-- 
Jon Smirl
jonsmirl@gmail.com

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-01 17:25 ` Jon Smirl
  2008-01-01 17:42   ` Jon Smirl
@ 2008-01-02  0:26   ` David Gibson
  2008-01-02 15:10   ` Timur Tabi
  2 siblings, 0 replies; 89+ messages in thread
From: David Gibson @ 2008-01-02  0:26 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel, Timur Tabi

On Tue, Jan 01, 2008 at 12:25:32PM -0500, Jon Smirl wrote:
> On 12/19/07, Timur Tabi <timur@freescale.com> wrote:
> > +               ssi@16000 {
> > +                       compatible = "fsl,ssi";
> > +                       cell-index = <0>;
> > +                       reg = <16000 100>;
> > +                       interrupt-parent = <&mpic>;
> > +                       interrupts = <3e 2>;
> > +                       fsl,mode = "i2s-slave";
> > +                       codec {
> > +                               compatible = "cirrus,cs4270";
> > +                               /* MCLK source is a stand-alone oscillator */
> > +                               bus-frequency = <bb8000>;
> > +                       };
> > +               };
> 
> Does this need to be bus-frequency? It's always called MCLK in all of
> the literature.
> 
> In my case the MCLK comes from a chip on the i2c bus that is
> programmable How would that be encoded?.

Grah!  If there's one obvious frequency for a node, it should always
be "clock-frequency".  This bus-frequency nonsense seems to be a
disease that started as a secondary frequency in Freescale CPU nodes,
and has escaped to all sorts of other places.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-20  0:03 [PATCH] ASoC drivers for the Freescale MPC8610 SoC Timur Tabi
                   ` (3 preceding siblings ...)
  2008-01-01 17:25 ` Jon Smirl
@ 2008-01-02  4:27 ` Jon Smirl
  2008-01-02 15:29   ` Timur Tabi
  4 siblings, 1 reply; 89+ messages in thread
From: Jon Smirl @ 2008-01-02  4:27 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On 12/19/07, Timur Tabi <timur@freescale.com> wrote:
>  sound/soc/fsl/fsl_ssi.c                      |  614 +++++++++++++++++++
>  sound/soc/fsl/fsl_ssi.h                      |  224 +++++++

I'm confused about this part. You built a driver for the mpc8610 ssi
port.  This port has a device tree entry.

+		ssi@16000 {
+			compatible = "fsl,ssi";
+			cell-index = <0>;
+			reg = <16000 100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <3e 2>;
+			fsl,mode = "i2s-slave";
+			codec {
+				compatible = "cirrus,cs4270";
+				/* MCLK source is a stand-alone oscillator */
+				bus-frequency = <bb8000>;
+			};
+		};

But then you don't create an of_platform_driver for this device.
Instead you create one for the fabric driver, struct
of_platform_driver mpc8610_hpcd_of_driver, and directly link the SSI
driver into it.

+static struct of_device_id mpc8610_hpcd_match[] = {
+	{
+		.compatible = "fsl,ssi",
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, mpc8610_hpcd_match);
+
+static struct of_platform_driver mpc8610_hpcd_of_driver = {
+	.owner  	= THIS_MODULE,
+	.name   	= "mpc8610_hpcd",
+	.match_table    = mpc8610_hpcd_match,
+	.probe  	= mpc8610_hpcd_probe,
+	.remove 	= mpc8610_hpcd_remove,
+};

static int mpc8610_hpcd_probe(struct of_device *ofdev,
	const struct of_device_id *match)
{
.....
	machine_data->dai.cpu_dai = fsl_ssi_create_dai(&ssi_info);

Isn't this two separate drivers that have been combined into one
driver? Or does the fsl_ssi channel only work on the mpc8610_hpcd?

This is the problem of knowing how to load the fabric driver that I
was talking about in the other threads. A device that can occur on
more than one chip ".compatible = "fsl,ssi"," is being used to pull in
a platform specific fabric driver, "mpc8610_hpcd". You can use the
kernel config system to select the right driver for ".compatible =
"fsl,ssi"," that matches you hardware and compile it in.

But that doesn't work in my environment. My generic channel is
"fsl,i2s". I have four different systems booting off from a shared
network drive. Each of these systems needs the common "fsl,i2s" driver
but they all four need different fabric drivers.

-- 
Jon Smirl
jonsmirl@gmail.com

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-01 17:25 ` Jon Smirl
  2008-01-01 17:42   ` Jon Smirl
  2008-01-02  0:26   ` David Gibson
@ 2008-01-02 15:10   ` Timur Tabi
  2008-01-02 17:23     ` [alsa-devel] " Mark Brown
  2 siblings, 1 reply; 89+ messages in thread
From: Timur Tabi @ 2008-01-02 15:10 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel

Jon Smirl wrote:
> On 12/19/07, Timur Tabi <timur@freescale.com> wrote:
>> +               ssi@16000 {
>> +                       compatible = "fsl,ssi";
>> +                       cell-index = <0>;
>> +                       reg = <16000 100>;
>> +                       interrupt-parent = <&mpic>;
>> +                       interrupts = <3e 2>;
>> +                       fsl,mode = "i2s-slave";
>> +                       codec {
>> +                               compatible = "cirrus,cs4270";
>> +                               /* MCLK source is a stand-alone oscillator */
>> +                               bus-frequency = <bb8000>;
>> +                       };
>> +               };
> 
> Does this need to be bus-frequency? It's always called MCLK in all of
> the literature.

I'm trying to make this node as generic as possible.  The fabric driver 
is the one that will parse this node and pass the data to the codec 
driver, so I can't use any codec-specific terms.

The API from the fabric driver for passing clock information includes a 
clock ID, a direction, and a frequency.  I can do something like this:

clock1 = <0, bb8000>

Would that be better?

> 
> In my case the MCLK comes from a chip on the i2c bus that is
> programmable How would that be encoded?.

I'm going under the assumption that MCLK does not change once the board 
is up and running.  In your case, you'd need to do something quite 
different, because you're not reading the clock info from the device 
tree and passing it to the codec at initialization once.  If you want to 
define an extension to the 'codec' child node that handles that, I'll 
add it to the documentation.

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-01 17:42   ` Jon Smirl
@ 2008-01-02 15:19     ` Timur Tabi
  2008-01-02 15:34       ` Jon Smirl
  2008-01-02 16:12       ` Grant Likely
  0 siblings, 2 replies; 89+ messages in thread
From: Timur Tabi @ 2008-01-02 15:19 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel

Jon Smirl wrote:
> On 1/1/08, Jon Smirl <jonsmirl@gmail.com> wrote:
>> On 12/19/07, Timur Tabi <timur@freescale.com> wrote:
>>> +               ssi@16000 {
>>> +                       compatible = "fsl,ssi";
>>> +                       cell-index = <0>;
>>> +                       reg = <16000 100>;
>>> +                       interrupt-parent = <&mpic>;
>>> +                       interrupts = <3e 2>;
>>> +                       fsl,mode = "i2s-slave";
>>> +                       codec {
>>> +                               compatible = "cirrus,cs4270";
>>> +                               /* MCLK source is a stand-alone oscillator */
>>> +                               bus-frequency = <bb8000>;
>>> +                       };
>>> +               };
>> Does this need to be bus-frequency? It's always called MCLK in all of
>> the literature.
>>
>> In my case the MCLK comes from a chip on the i2c bus that is
>> programmable How would that be encoded?.
> 
> Looking at the cs4270 codec driver it is controlled by i2c (supports
> SPI too).  What happened to the conversation about putting codecs on
> the controlling bus and then linking them to the data bus?

The current CS4270 driver doesn't support device trees.  When I wrote 
it, the idea of putting I2C info in the device tree was not finalized, 
and since the driver is supposed to be cross-platform, I decided to do 
it the old-fashioned way.  Before I update the code, however, I'm 
waiting for:

1) The current code to be accepted into the tree
2) ASoC is updated to V2
3) The current drivers are updated to support ASoC V2.

I think ASoC V2 will make it easier to support device trees, but I'm not 
ready yet for that.

> If that's the case the cs4270 should be in the i2c bus node (missing
> currently) and then a link from the SSI bus would point to it.

The CS4270 is a child of both the I2C bus *and* the SSI bus.  It needs 
to have two nodes, one under each.  Your're right in that there needs to 
be a link, but until the code is updated to ASoC V2, I think it's 
premature to add that support.

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02  4:27 ` Jon Smirl
@ 2008-01-02 15:29   ` Timur Tabi
  2008-01-02 15:56     ` Jon Smirl
                       ` (2 more replies)
  0 siblings, 3 replies; 89+ messages in thread
From: Timur Tabi @ 2008-01-02 15:29 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel

Jon Smirl wrote:
> On 12/19/07, Timur Tabi <timur@freescale.com> wrote:
>>  sound/soc/fsl/fsl_ssi.c                      |  614 +++++++++++++++++++
>>  sound/soc/fsl/fsl_ssi.h                      |  224 +++++++
> 
> I'm confused about this part. You built a driver for the mpc8610 ssi
> port.  This port has a device tree entry.
> 
> +		ssi@16000 {
> +			compatible = "fsl,ssi";
> +			cell-index = <0>;
> +			reg = <16000 100>;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <3e 2>;
> +			fsl,mode = "i2s-slave";
> +			codec {
> +				compatible = "cirrus,cs4270";
> +				/* MCLK source is a stand-alone oscillator */
> +				bus-frequency = <bb8000>;
> +			};
> +		};
> 
> But then you don't create an of_platform_driver for this device.
> Instead you create one for the fabric driver, struct
> of_platform_driver mpc8610_hpcd_of_driver, and directly link the SSI
> driver into it.

That's the best plan I came up with.  This is apparently fixed in ASoC 
V2.  From ASoC V1's perspective, the fabric driver must be the master. 
However, it doesn't make sense to have a node in the device tree for the 
fabric driver, because there is no such "device".  The fabric driver is 
an abstraction.  So I need to chose some other node to probe the fabric 
driver with.  I chose the SSI, since each SSI can have only one codec.

> 
> +static struct of_device_id mpc8610_hpcd_match[] = {
> +	{
> +		.compatible = "fsl,ssi",
> +	},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, mpc8610_hpcd_match);
> +
> +static struct of_platform_driver mpc8610_hpcd_of_driver = {
> +	.owner  	= THIS_MODULE,
> +	.name   	= "mpc8610_hpcd",
> +	.match_table    = mpc8610_hpcd_match,
> +	.probe  	= mpc8610_hpcd_probe,
> +	.remove 	= mpc8610_hpcd_remove,
> +};
> 
> static int mpc8610_hpcd_probe(struct of_device *ofdev,
> 	const struct of_device_id *match)
> {
> .....
> 	machine_data->dai.cpu_dai = fsl_ssi_create_dai(&ssi_info);
> 
> Isn't this two separate drivers that have been combined into one
> driver? Or does the fsl_ssi channel only work on the mpc8610_hpcd?

Sorry, I don't understand your question.

> This is the problem of knowing how to load the fabric driver that I
> was talking about in the other threads.

Yes, and the decision I made on this topic is to have the fabric driver 
probed on the SSI node.

For ASoC V1, I believe the problem is undefined and each driver should 
be implemented in whatever way works best.

 > A device that can occur on
> more than one chip ".compatible = "fsl,ssi"," is being used to pull in
> a platform specific fabric driver, "mpc8610_hpcd". You can use the
> kernel config system to select the right driver for ".compatible =
> "fsl,ssi"," that matches you hardware and compile it in.

Ok, I think I understand that.

> But that doesn't work in my environment. My generic channel is
> "fsl,i2s". I have four different systems booting off from a shared
> network drive. Each of these systems needs the common "fsl,i2s" driver
> but they all four need different fabric drivers.

That, I don't understand.  fsl,ssi is pretty much the same thing as 
fsl,i2s, since the SSI *is* an I2S device.  It's also an AC97 device, 
which is why I added the fsl,mode property.

The fabric driver is specific to the board.  So you should be using 
Kconfig to select the fabric driver.  There is no node in the device 
tree for fabric drivers.  I thought that was the consensus.

Are you saying that you want to use the same kernel on four different 
systems?  If so, then you need to find a way to compile all fabric 
drivers together, and at boot time each fabric driver will decide 
whether it will do anything.

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 15:19     ` Timur Tabi
@ 2008-01-02 15:34       ` Jon Smirl
  2008-01-03 17:54         ` Timur Tabi
  2008-01-02 16:12       ` Grant Likely
  1 sibling, 1 reply; 89+ messages in thread
From: Jon Smirl @ 2008-01-02 15:34 UTC (permalink / raw)
  To: Timur Tabi, Liam Girdwood; +Cc: linuxppc-dev, alsa-devel

On 1/2/08, Timur Tabi <timur@freescale.com> wrote:
> Jon Smirl wrote:
> > On 1/1/08, Jon Smirl <jonsmirl@gmail.com> wrote:
> >> On 12/19/07, Timur Tabi <timur@freescale.com> wrote:
> >>> +               ssi@16000 {
> >>> +                       compatible = "fsl,ssi";
> >>> +                       cell-index = <0>;
> >>> +                       reg = <16000 100>;
> >>> +                       interrupt-parent = <&mpic>;
> >>> +                       interrupts = <3e 2>;
> >>> +                       fsl,mode = "i2s-slave";
> >>> +                       codec {
> >>> +                               compatible = "cirrus,cs4270";
> >>> +                               /* MCLK source is a stand-alone oscillator */
> >>> +                               bus-frequency = <bb8000>;
> >>> +                       };
> >>> +               };
> >> Does this need to be bus-frequency? It's always called MCLK in all of
> >> the literature.
> >>
> >> In my case the MCLK comes from a chip on the i2c bus that is
> >> programmable How would that be encoded?.
> >
> > Looking at the cs4270 codec driver it is controlled by i2c (supports
> > SPI too).  What happened to the conversation about putting codecs on
> > the controlling bus and then linking them to the data bus?
>
> The current CS4270 driver doesn't support device trees.  When I wrote
> it, the idea of putting I2C info in the device tree was not finalized,
> and since the driver is supposed to be cross-platform, I decided to do
> it the old-fashioned way.  Before I update the code, however, I'm
> waiting for:
>
> 1) The current code to be accepted into the tree
> 2) ASoC is updated to V2
> 3) The current drivers are updated to support ASoC V2.

I've been trying to get the i2c code in for two months. Hopefully it
will go in soon, no one had made any comments on it recently. Have you
tried your code with it?

There is nothing stopping your from putting a node for the CS4270 on
the i2c bus today. It just won't trigger the loading of the driver.

Don't we want to follow the device tree policy of putting the device
on the controlling bus and then link it to the data bus?

> I think ASoC V2 will make it easier to support device trees, but I'm not
> ready yet for that.

It makes it a little easier but it doesn't fix everything. We need to
start looking at it since none of the example driver for it are device
tree based. It still has problems with wanting 'struct
platform_device' when we have 'struct of_platform_device' pointers. It
also doesn't know how to dynamically load codecs based on device
trees.

Liam messed up all of my code when he refactored it in late December.
I've switched over to the current SOC code for the moment. The big
thing that v2 fixes is that SOC is changed to being a subsystem
instead of platform driver. Being a subsystem is the correct model.

It would be good if more pieces of v2 get push forward. Then we can
sort out the device tree issues in it.


>
> > If that's the case the cs4270 should be in the i2c bus node (missing
> > currently) and then a link from the SSI bus would point to it.
>
> The CS4270 is a child of both the I2C bus *and* the SSI bus.  It needs
> to have two nodes, one under each.  Your're right in that there needs to
> be a link, but until the code is updated to ASoC V2, I think it's
> premature to add that support.

Adding the second device tree node doesn't have anything to do with
ASOC v2. It's specific to powerpc and device trees.

-- 
Jon Smirl
jonsmirl@gmail.com

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 15:29   ` Timur Tabi
@ 2008-01-02 15:56     ` Jon Smirl
  2008-01-02 16:32       ` Grant Likely
  2008-01-03 17:57       ` Timur Tabi
  2008-01-02 16:28     ` Grant Likely
  2008-01-03  4:44     ` David Gibson
  2 siblings, 2 replies; 89+ messages in thread
From: Jon Smirl @ 2008-01-02 15:56 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On 1/2/08, Timur Tabi <timur@freescale.com> wrote:
> Are you saying that you want to use the same kernel on four different
> systems?  If so, then you need to find a way to compile all fabric
> drivers together, and at boot time each fabric driver will decide
> whether it will do anything.

Yes, I have four different but similar systems. They only differer in
the codec chips used. I want to make a single kernel image and then
use the device tree to dynamically load the correct codec driver from
initrd. That will let me ship a single kernel image that services all
four machines. The codecs implement different sound systems from low
end to high end.

The correct solution for this is to use kernel modules and trigger
their loading based on the device tree. This is the same mechanism
used by USB and PCI.

For this model to work you need to split your driver. fsl-ssi and
mpc8610_hpcd need to be in  two separate drivers. fsl-ssi  is easy
enough to load since it has a device tree entry.

mpc8610_hpcd is the harder one to load since it doesn't have a device
tree entry. What you want to do it match on the compatible field of
the root node.

static struct of_device_id fabric_of_match[] = {
	{
		.compatible	= "fsl,MPC8610HPCD",
	},
	{},
};

But this doesn't work since the root is the device tree isn't passed
down into the device probe code. (Could this be fixed?)

Instead we could make the separated mpc8610_hpcd fabric driver attach
to fsl,ssi.

static struct of_device_id fabric_of_match[] = {
	{
		.compatible	= "fsl,ssi",
	},
	{},
};

Then in it's probe code check for the right platform.

unsigned long node = of_get_flat_dt_root();
if (!of_flat_dt_is_compatible(node, "fsl,MPC8610HPCD"))
	return 0;
.. activate the code ...

You also need a static flag to make sure you don't active the driver
more than once.

This isn't the best solution since my four fabric drivers will still
load and check what platform they are on before exiting but at least
it works.

-- 
Jon Smirl
jonsmirl@gmail.com

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 15:19     ` Timur Tabi
  2008-01-02 15:34       ` Jon Smirl
@ 2008-01-02 16:12       ` Grant Likely
  2008-01-03 18:08         ` Timur Tabi
  1 sibling, 1 reply; 89+ messages in thread
From: Grant Likely @ 2008-01-02 16:12 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On 1/2/08, Timur Tabi <timur@freescale.com> wrote:
> Jon Smirl wrote:
> > If that's the case the cs4270 should be in the i2c bus node (missing
> > currently) and then a link from the SSI bus would point to it.
>
> The CS4270 is a child of both the I2C bus *and* the SSI bus.  It needs
> to have two nodes, one under each.  Your're right in that there needs to
> be a link, but until the code is updated to ASoC V2, I think it's
> premature to add that support.

Why not be a child of the i2c bus with a phandle to the ssi bus?  That
is the direction we've gone with other multi attachment devices.  (ie.
Ethernet PHYs.  Child of the MDIO node, phandle to link the Ethernet
controller with the PHY)

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 15:29   ` Timur Tabi
  2008-01-02 15:56     ` Jon Smirl
@ 2008-01-02 16:28     ` Grant Likely
  2008-01-02 18:49       ` [alsa-devel] " Mark Brown
  2008-01-03 18:14       ` Timur Tabi
  2008-01-03  4:44     ` David Gibson
  2 siblings, 2 replies; 89+ messages in thread
From: Grant Likely @ 2008-01-02 16:28 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On 1/2/08, Timur Tabi <timur@freescale.com> wrote:
> That's the best plan I came up with.  This is apparently fixed in ASoC
> V2.  From ASoC V1's perspective, the fabric driver must be the master.
> However, it doesn't make sense to have a node in the device tree for the
> fabric driver, because there is no such "device".  The fabric driver is
> an abstraction.  So I need to chose some other node to probe the fabric
> driver with.  I chose the SSI, since each SSI can have only one codec.

Does that mean with ASoC V2 you can instantiate it with the board
specific platform code instead?  I think that is the consensus we were
leaning towards in the last discussion about this issue.

> > But that doesn't work in my environment. My generic channel is
> > "fsl,i2s". I have four different systems booting off from a shared
> > network drive. Each of these systems needs the common "fsl,i2s" driver
> > but they all four need different fabric drivers.
>
> That, I don't understand.  fsl,ssi is pretty much the same thing as
> fsl,i2s, since the SSI *is* an I2S device.  It's also an AC97 device,
> which is why I added the fsl,mode property.

This is one of the examples of where the compatible properties are
trying to be far to generic about what they are.  How do you define
what "fsl,ssi" is?  What happens when Freescale produces another
peripheral that can do ssi but isn't register level compatible?

In my opinion, it is far better to be specific in the device tree and
teach the driver about what versions it is able to bind against.  In
this case, I would use "fsl,mpc8610-ssi" or maybe better yet:
"fsl,mpc8610-ssi,i2s" (MPC8610 SSI device in I2S mode).

I don't like the idea of a separate fsl,mode property to describe the
behaviour of multifunction peripherals.  It makes probing more
difficult when there is a different driver for each mode.

>
> The fabric driver is specific to the board.  So you should be using
> Kconfig to select the fabric driver.  There is no node in the device
> tree for fabric drivers.  I thought that was the consensus.

No, the desire is to go multiplatform in ppc.  That means you cannot
use Kconfig to select the correct fabric driver.

> Are you saying that you want to use the same kernel on four different
> systems?  If so, then you need to find a way to compile all fabric
> drivers together, and at boot time each fabric driver will decide
> whether it will do anything.

Yes!  That is exactly what we want to support in arch/powerpc.  Use
platform code to select the correct fabric driver.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 15:56     ` Jon Smirl
@ 2008-01-02 16:32       ` Grant Likely
  2008-01-02 17:12         ` Jon Smirl
  2008-01-03 17:57       ` Timur Tabi
  1 sibling, 1 reply; 89+ messages in thread
From: Grant Likely @ 2008-01-02 16:32 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel, Timur Tabi

On 1/2/08, Jon Smirl <jonsmirl@gmail.com> wrote:
> On 1/2/08, Timur Tabi <timur@freescale.com> wrote:
> mpc8610_hpcd is the harder one to load since it doesn't have a device
> tree entry. What you want to do it match on the compatible field of
> the root node.
>
> static struct of_device_id fabric_of_match[] = {
>         {
>                 .compatible     = "fsl,MPC8610HPCD",
>         },
>         {},
> };
>
> But this doesn't work since the root is the device tree isn't passed
> down into the device probe code. (Could this be fixed?)

The driver can always get the root node.  But better yet, instantiate
the correct fabric device (probably as a platform_device) from the
platform code.  Then the correct fabric driver can probe against it.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 16:32       ` Grant Likely
@ 2008-01-02 17:12         ` Jon Smirl
  2008-01-02 17:22           ` Grant Likely
  2008-01-03  4:46           ` David Gibson
  0 siblings, 2 replies; 89+ messages in thread
From: Jon Smirl @ 2008-01-02 17:12 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, alsa-devel, Timur Tabi

On 1/2/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> On 1/2/08, Jon Smirl <jonsmirl@gmail.com> wrote:
> > On 1/2/08, Timur Tabi <timur@freescale.com> wrote:
> > mpc8610_hpcd is the harder one to load since it doesn't have a device
> > tree entry. What you want to do it match on the compatible field of
> > the root node.
> >
> > static struct of_device_id fabric_of_match[] = {
> >         {
> >                 .compatible     = "fsl,MPC8610HPCD",
> >         },
> >         {},
> > };
> >
> > But this doesn't work since the root is the device tree isn't passed
> > down into the device probe code. (Could this be fixed?)
>
> The driver can always get the root node.  But better yet, instantiate
> the correct fabric device (probably as a platform_device) from the
> platform code.  Then the correct fabric driver can probe against it.

The meaning of this has finally sunk into my consciousness. The
platform code can create a device that isn't bound to a driver. So why
not make this an of_platform_device?  This is basically a pseudo
device that isn't in the device tree.

Alternatively, the best place for this device would be on the ASOC
bus, but the ASOC bus hasn't been created when the platform code runs.
Maybe I can figure out a place in the platform code to create this
device after the ASOC driver has loaded and created the bus. Does the
platform code get control back after loading all of the device
drivers?

In the longer term I'd like to kill platform_bus on powerpc and only
use of_platform_bus. Platform_bus seems to be functioning like a
catch-all and collecting junk from lots of different platforms.


-- 
Jon Smirl
jonsmirl@gmail.com

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 17:12         ` Jon Smirl
@ 2008-01-02 17:22           ` Grant Likely
  2008-01-02 18:43             ` Jon Smirl
  2008-01-03  4:46           ` David Gibson
  1 sibling, 1 reply; 89+ messages in thread
From: Grant Likely @ 2008-01-02 17:22 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel, Timur Tabi

On 1/2/08, Jon Smirl <jonsmirl@gmail.com> wrote:
> On 1/2/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> > On 1/2/08, Jon Smirl <jonsmirl@gmail.com> wrote:
> > > On 1/2/08, Timur Tabi <timur@freescale.com> wrote:
> > > mpc8610_hpcd is the harder one to load since it doesn't have a device
> > > tree entry. What you want to do it match on the compatible field of
> > > the root node.
> > >
> > > static struct of_device_id fabric_of_match[] = {
> > >         {
> > >                 .compatible     = "fsl,MPC8610HPCD",
> > >         },
> > >         {},
> > > };
> > >
> > > But this doesn't work since the root is the device tree isn't passed
> > > down into the device probe code. (Could this be fixed?)
> >
> > The driver can always get the root node.  But better yet, instantiate
> > the correct fabric device (probably as a platform_device) from the
> > platform code.  Then the correct fabric driver can probe against it.
>
> The meaning of this has finally sunk into my consciousness. The
> platform code can create a device that isn't bound to a driver. So why
> not make this an of_platform_device?  This is basically a pseudo
> device that isn't in the device tree.

Simply because it doesn't have a device node.  That's the prime
characteristc that differentiates platform_bus from of_platform_bus.
What do you bind against in of_platform_bus if you don't have a
specific node for it?

> Alternatively, the best place for this device would be on the ASOC
> bus, but the ASOC bus hasn't been created when the platform code runs.
> Maybe I can figure out a place in the platform code to create this
> device after the ASOC driver has loaded and created the bus. Does the
> platform code get control back after loading all of the device
> drivers?

Yes, but it requires the core ASoC code to not be a module.  Then you
can use machine_device_initcall() to register the device at a later
time.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 15:10   ` Timur Tabi
@ 2008-01-02 17:23     ` Mark Brown
  2008-01-03 18:23       ` Timur Tabi
  0 siblings, 1 reply; 89+ messages in thread
From: Mark Brown @ 2008-01-02 17:23 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On Wed, Jan 02, 2008 at 09:10:44AM -0600, Timur Tabi wrote:
> Jon Smirl wrote:

> > Does this need to be bus-frequency? It's always called MCLK in all of
> > the literature.

> I'm trying to make this node as generic as possible.  The fabric driver 
> is the one that will parse this node and pass the data to the codec 
> driver, so I can't use any codec-specific terms.

> The API from the fabric driver for passing clock information includes a 
> clock ID, a direction, and a frequency.  I can do something like this:

> clock1 = <0, bb8000>

> Would that be better?

To cover everything you'd need to be able to specify all the clocking
parameters, especially a PLL configuration, and also specify more than
one of each item.  Even then you'd still have problems like...

> > In my case the MCLK comes from a chip on the i2c bus that is
> > programmable How would that be encoded?.

> I'm going under the assumption that MCLK does not change once the board 
> is up and running.  In your case, you'd need to do something quite 
> different, because you're not reading the clock info from the device 
> tree and passing it to the codec at initialization once.  If you want to 
> define an extension to the 'codec' child node that handles that, I'll 
> add it to the documentation.

According to the documentation in your patch the bus frequency should
already be optional (though I don't immediately see that in the code,
but then I'm entirely unfamiliar with OpenFirmware device trees).
Boards that reconfigure the clocking at run time can then provide
code to set the clocking up at the appropriate times, which is probably
what they want anyway.

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2007-12-23  2:58           ` Timur Tabi
@ 2008-01-02 18:08             ` Scott Wood
  0 siblings, 0 replies; 89+ messages in thread
From: Scott Wood @ 2008-01-02 18:08 UTC (permalink / raw)
  To: Timur Tabi; +Cc: Olof Johansson, linuxppc-dev, alsa-devel

On Sat, Dec 22, 2007 at 08:58:21PM -0600, Timur Tabi wrote:
> Scott Wood wrote:
>
>>> None of the SOC nodes in any DTS have a "compatible" entry.
>>
>> Not quite true; ep88xc, mpc8272ads, and pq2fads have them.
>
> Ah ok.  So what should the compatible entry for 8641 be?
>
> 	compatible = "fsl,mpc8641"

Yes.

> That looks a lot like what a compatible entry for the CPU should be.

I guess technically the cpu should list something like fsl,e600 (or
whatever suffix is relevant).

-Scott

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 17:22           ` Grant Likely
@ 2008-01-02 18:43             ` Jon Smirl
  2008-01-02 18:50               ` Grant Likely
  0 siblings, 1 reply; 89+ messages in thread
From: Jon Smirl @ 2008-01-02 18:43 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, alsa-devel, Timur Tabi

On 1/2/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> > Alternatively, the best place for this device would be on the ASOC
> > bus, but the ASOC bus hasn't been created when the platform code runs.
> > Maybe I can figure out a place in the platform code to create this
> > device after the ASOC driver has loaded and created the bus. Does the
> > platform code get control back after loading all of the device
> > drivers?
>
> Yes, but it requires the core ASoC code to not be a module.  Then you
> can use machine_device_initcall() to register the device at a later
> time.

How about this for a simpler solution? My mpc5200-psc-ac97 and
mpc5200-psc-i2c drivers can create a device on the ASOC bus named
after the first entry in the compatible field of the root node. That
will cause the correct driver to get activated. I'm in the process of
making ASOC drivers dynamically loadable like the i2c ones.


-- 
Jon Smirl
jonsmirl@gmail.com

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 16:28     ` Grant Likely
@ 2008-01-02 18:49       ` Mark Brown
  2008-01-03 18:16         ` Timur Tabi
  2008-01-03 18:14       ` Timur Tabi
  1 sibling, 1 reply; 89+ messages in thread
From: Mark Brown @ 2008-01-02 18:49 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, alsa-devel, Timur Tabi

On Wed, Jan 02, 2008 at 09:28:12AM -0700, Grant Likely wrote:
> On 1/2/08, Timur Tabi <timur@freescale.com> wrote:

> > However, it doesn't make sense to have a node in the device tree for the
> > fabric driver, because there is no such "device".  The fabric driver is
> > an abstraction.  So I need to chose some other node to probe the fabric
> > driver with.  I chose the SSI, since each SSI can have only one codec.

I'm not sure I'd go so far as to say that the fabric/machine driver is
purely an abstraction.  It does represent real hardware, often with
software control.

> Does that mean with ASoC V2 you can instantiate it with the board
> specific platform code instead?  I think that is the consensus we were
> leaning towards in the last discussion about this issue.

With ASoC v2 rather than having a single monolithic ASoC device which
probes everything ASoC is converted into a proper subsystem with each
component (codec, SoC CPU port, whatever) having a sysfs-visible driver.
These drivers register with the core as they are probed with the probing
happening through whatever mechanism is appropriate for the driver.

The machine support code (fabric driver in PowerPC terms, I think?)
tells the core how everything is connected together by registering
devices representing the links (eg, I2S) between the codecs, CPU and
other devices.  The ASoC core is then responsible for ensuring that all
the required components are present before it registers with the ALSA
core.

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 18:43             ` Jon Smirl
@ 2008-01-02 18:50               ` Grant Likely
  2008-01-02 18:56                 ` Jon Smirl
  0 siblings, 1 reply; 89+ messages in thread
From: Grant Likely @ 2008-01-02 18:50 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel, Timur Tabi

On 1/2/08, Jon Smirl <jonsmirl@gmail.com> wrote:
> On 1/2/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> > > Alternatively, the best place for this device would be on the ASOC
> > > bus, but the ASOC bus hasn't been created when the platform code runs.
> > > Maybe I can figure out a place in the platform code to create this
> > > device after the ASOC driver has loaded and created the bus. Does the
> > > platform code get control back after loading all of the device
> > > drivers?
> >
> > Yes, but it requires the core ASoC code to not be a module.  Then you
> > can use machine_device_initcall() to register the device at a later
> > time.
>
> How about this for a simpler solution? My mpc5200-psc-ac97 and
> mpc5200-psc-i2c drivers can create a device on the ASOC bus named
> after the first entry in the compatible field of the root node. That
> will cause the correct driver to get activated. I'm in the process of
> making ASOC drivers dynamically loadable like the i2c ones.

I little icky, but it doesn't sound dangerous (as in shouldn't cause
any name conflicts).  That may be the best we can do for the time
being.  But I don't think it is a good idea for the long term.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 18:50               ` Grant Likely
@ 2008-01-02 18:56                 ` Jon Smirl
  0 siblings, 0 replies; 89+ messages in thread
From: Jon Smirl @ 2008-01-02 18:56 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, alsa-devel, Timur Tabi

On 1/2/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> On 1/2/08, Jon Smirl <jonsmirl@gmail.com> wrote:
> > On 1/2/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> > > > Alternatively, the best place for this device would be on the ASOC
> > > > bus, but the ASOC bus hasn't been created when the platform code runs.
> > > > Maybe I can figure out a place in the platform code to create this
> > > > device after the ASOC driver has loaded and created the bus. Does the
> > > > platform code get control back after loading all of the device
> > > > drivers?
> > >
> > > Yes, but it requires the core ASoC code to not be a module.  Then you
> > > can use machine_device_initcall() to register the device at a later
> > > time.
> >
> > How about this for a simpler solution? My mpc5200-psc-ac97 and
> > mpc5200-psc-i2c drivers can create a device on the ASOC bus named
> > after the first entry in the compatible field of the root node. That
> > will cause the correct driver to get activated. I'm in the process of
> > making ASOC drivers dynamically loadable like the i2c ones.
>
> I little icky, but it doesn't sound dangerous (as in shouldn't cause
> any name conflicts).  That may be the best we can do for the time
> being.  But I don't think it is a good idea for the long term.

Simplest long term fix is to allow drivers to bind on the root node.
Make this work:

> static struct of_device_id fabric_of_match[] = {
>         {
>                 .compatible     = "fsl,MPC8610HPCD",
>         },
>         {},
> };



>
> Cheers,
> g.
>
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
> grant.likely@secretlab.ca
> (403) 399-0195
>


-- 
Jon Smirl
jonsmirl@gmail.com

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 15:29   ` Timur Tabi
  2008-01-02 15:56     ` Jon Smirl
  2008-01-02 16:28     ` Grant Likely
@ 2008-01-03  4:44     ` David Gibson
  2008-01-03 14:54       ` Jon Smirl
  2008-01-03 18:16       ` Timur Tabi
  2 siblings, 2 replies; 89+ messages in thread
From: David Gibson @ 2008-01-03  4:44 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On Wed, Jan 02, 2008 at 09:29:57AM -0600, Timur Tabi wrote:
> Jon Smirl wrote:
> > On 12/19/07, Timur Tabi <timur@freescale.com> wrote:
> >>  sound/soc/fsl/fsl_ssi.c                      |  614 +++++++++++++++++++
> >>  sound/soc/fsl/fsl_ssi.h                      |  224 +++++++
> > 
> > I'm confused about this part. You built a driver for the mpc8610 ssi
> > port.  This port has a device tree entry.
> > 
> > +		ssi@16000 {
> > +			compatible = "fsl,ssi";
> > +			cell-index = <0>;
> > +			reg = <16000 100>;
> > +			interrupt-parent = <&mpic>;
> > +			interrupts = <3e 2>;
> > +			fsl,mode = "i2s-slave";
> > +			codec {
> > +				compatible = "cirrus,cs4270";
> > +				/* MCLK source is a stand-alone oscillator */
> > +				bus-frequency = <bb8000>;
> > +			};
> > +		};
> > 
> > But then you don't create an of_platform_driver for this device.
> > Instead you create one for the fabric driver, struct
> > of_platform_driver mpc8610_hpcd_of_driver, and directly link the SSI
> > driver into it.
> 
> That's the best plan I came up with.  This is apparently fixed in ASoC 
> V2.  From ASoC V1's perspective, the fabric driver must be the master. 
> However, it doesn't make sense to have a node in the device tree for the 
> fabric driver, because there is no such "device".  The fabric driver is 
> an abstraction.  So I need to chose some other node to probe the fabric 
> driver with.  I chose the SSI, since each SSI can have only one
> codec.

Instantiating the fabric driver off any node is wrong, precisely
because it is an abstraction.  The fabric driver should be
instantiated by the platform code.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 17:12         ` Jon Smirl
  2008-01-02 17:22           ` Grant Likely
@ 2008-01-03  4:46           ` David Gibson
  2008-01-03 14:33             ` Jon Smirl
  1 sibling, 1 reply; 89+ messages in thread
From: David Gibson @ 2008-01-03  4:46 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel, Timur Tabi

On Wed, Jan 02, 2008 at 12:12:00PM -0500, Jon Smirl wrote:
> On 1/2/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> > On 1/2/08, Jon Smirl <jonsmirl@gmail.com> wrote:
> > > On 1/2/08, Timur Tabi <timur@freescale.com> wrote:
> > > mpc8610_hpcd is the harder one to load since it doesn't have a device
> > > tree entry. What you want to do it match on the compatible field of
> > > the root node.
> > >
> > > static struct of_device_id fabric_of_match[] = {
> > >         {
> > >                 .compatible     = "fsl,MPC8610HPCD",
> > >         },
> > >         {},
> > > };
> > >
> > > But this doesn't work since the root is the device tree isn't passed
> > > down into the device probe code. (Could this be fixed?)
> >
> > The driver can always get the root node.  But better yet, instantiate
> > the correct fabric device (probably as a platform_device) from the
> > platform code.  Then the correct fabric driver can probe against it.
> 
> The meaning of this has finally sunk into my consciousness. The
> platform code can create a device that isn't bound to a driver. So why
> not make this an of_platform_device?  This is basically a pseudo
> device that isn't in the device tree.
> 
> Alternatively, the best place for this device would be on the ASOC
> bus, but the ASOC bus hasn't been created when the platform code runs.
> Maybe I can figure out a place in the platform code to create this
> device after the ASOC driver has loaded and created the bus. Does the
> platform code get control back after loading all of the device
> drivers?
> 
> In the longer term I'd like to kill platform_bus on powerpc and only
> use of_platform_bus. Platform_bus seems to be functioning like a
> catch-all and collecting junk from lots of different platforms.

Not going to happen.  of_platform_bus is not the right solution, and
in fact we're looking at moving (gradually) away from using
of_platform_bus, and instead using platform devices (along with the
device node being available for *any* struct device via the
arch_sysdata).

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03  4:46           ` David Gibson
@ 2008-01-03 14:33             ` Jon Smirl
  0 siblings, 0 replies; 89+ messages in thread
From: Jon Smirl @ 2008-01-03 14:33 UTC (permalink / raw)
  To: Jon Smirl, Grant Likely, linuxppc-dev, alsa-devel, Timur Tabi

On 1/2/08, David Gibson <david@gibson.dropbear.id.au> wrote:
> On Wed, Jan 02, 2008 at 12:12:00PM -0500, Jon Smirl wrote:
> > On 1/2/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> > > On 1/2/08, Jon Smirl <jonsmirl@gmail.com> wrote:
> > > > On 1/2/08, Timur Tabi <timur@freescale.com> wrote:
> > > > mpc8610_hpcd is the harder one to load since it doesn't have a device
> > > > tree entry. What you want to do it match on the compatible field of
> > > > the root node.
> > > >
> > > > static struct of_device_id fabric_of_match[] = {
> > > >         {
> > > >                 .compatible     = "fsl,MPC8610HPCD",
> > > >         },
> > > >         {},
> > > > };
> > > >
> > > > But this doesn't work since the root is the device tree isn't passed
> > > > down into the device probe code. (Could this be fixed?)
> > >
> > > The driver can always get the root node.  But better yet, instantiate
> > > the correct fabric device (probably as a platform_device) from the
> > > platform code.  Then the correct fabric driver can probe against it.
> >
> > The meaning of this has finally sunk into my consciousness. The
> > platform code can create a device that isn't bound to a driver. So why
> > not make this an of_platform_device?  This is basically a pseudo
> > device that isn't in the device tree.
> >
> > Alternatively, the best place for this device would be on the ASOC
> > bus, but the ASOC bus hasn't been created when the platform code runs.
> > Maybe I can figure out a place in the platform code to create this
> > device after the ASOC driver has loaded and created the bus. Does the
> > platform code get control back after loading all of the device
> > drivers?
> >
> > In the longer term I'd like to kill platform_bus on powerpc and only
> > use of_platform_bus. Platform_bus seems to be functioning like a
> > catch-all and collecting junk from lots of different platforms.
>
> Not going to happen.  of_platform_bus is not the right solution, and
> in fact we're looking at moving (gradually) away from using
> of_platform_bus, and instead using platform devices (along with the
> device node being available for *any* struct device via the
> arch_sysdata).

I do agree that of_platform_bus should have been derived from
platform_bus, not a separate structure. This is causing problems in
the ASLA code  where they want typed pointers.

In my little test patch, I disabled platform_bus on my MPC5200. This
generated some compiler errors which exposed a bunch of MPC83xxx and
Apple code that was getting compiled into my MPC5200 kernel.  These
were platform bus drivers that weren't properly ifdef'd.

So I guess my objection is more along the lines of getting rid of
driver code inside the arch directory and switching everything to
modules. Then we could periodically turn off platform bus on each
platform and make sure everything still builds.


>
> --
> David Gibson                    | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
>                                 | _way_ _around_!
> http://www.ozlabs.org/~dgibson
>


-- 
Jon Smirl
jonsmirl@gmail.com

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03  4:44     ` David Gibson
@ 2008-01-03 14:54       ` Jon Smirl
  2008-01-04  5:01         ` David Gibson
  2008-01-03 18:16       ` Timur Tabi
  1 sibling, 1 reply; 89+ messages in thread
From: Jon Smirl @ 2008-01-03 14:54 UTC (permalink / raw)
  To: Timur Tabi, Jon Smirl, linuxppc-dev, alsa-devel

On 1/2/08, David Gibson <david@gibson.dropbear.id.au> wrote:
> On Wed, Jan 02, 2008 at 09:29:57AM -0600, Timur Tabi wrote:
> > Jon Smirl wrote:
> > > On 12/19/07, Timur Tabi <timur@freescale.com> wrote:
> > >>  sound/soc/fsl/fsl_ssi.c                      |  614 +++++++++++++++++++
> > >>  sound/soc/fsl/fsl_ssi.h                      |  224 +++++++
> > >
> > > I'm confused about this part. You built a driver for the mpc8610 ssi
> > > port.  This port has a device tree entry.
> > >
> > > +           ssi@16000 {
> > > +                   compatible = "fsl,ssi";
> > > +                   cell-index = <0>;
> > > +                   reg = <16000 100>;
> > > +                   interrupt-parent = <&mpic>;
> > > +                   interrupts = <3e 2>;
> > > +                   fsl,mode = "i2s-slave";
> > > +                   codec {
> > > +                           compatible = "cirrus,cs4270";
> > > +                           /* MCLK source is a stand-alone oscillator */
> > > +                           bus-frequency = <bb8000>;
> > > +                   };
> > > +           };
> > >
> > > But then you don't create an of_platform_driver for this device.
> > > Instead you create one for the fabric driver, struct
> > > of_platform_driver mpc8610_hpcd_of_driver, and directly link the SSI
> > > driver into it.
> >
> > That's the best plan I came up with.  This is apparently fixed in ASoC
> > V2.  From ASoC V1's perspective, the fabric driver must be the master.
> > However, it doesn't make sense to have a node in the device tree for the
> > fabric driver, because there is no such "device".  The fabric driver is
> > an abstraction.  So I need to chose some other node to probe the fabric
> > driver with.  I chose the SSI, since each SSI can have only one
> > codec.
>
> Instantiating the fabric driver off any node is wrong, precisely
> because it is an abstraction.  The fabric driver should be
> instantiated by the platform code.

Instantiating it from the platform code forces me to put it either the
of_platform_bus or the platform_bus since there aren't any other buses
around when the platform code runs. Platform bus doesn't implement
dynamic module loading. So that means it has to go onto the
of_platform_bus. That implies that is it a pseudo-device without a
pseudo-device entry in the device tree which is fine with me. I'll
need to poke around in the of_bus code and see if the driver will load
without a device tree entry.

A simple fix to this would be to let me instantiate the driver off
from the root node of the tree. That's the conceptually correct place
for instantiating a driver that extends the platform code. Should I
try adjusting the of probing code to pass the node in, or are there
major objections?

Also, as others have pointed out, this driver is not an abstraction.
It represents the mess of wires hooking the codec up to the jacks on
the back panel and possibly GPIO pins that control the wiring. You
need this because the pins on HD audio codecs are completely
reconfigurable and the same chip can be wired in a thousand different
ways. It lets you have a generic codec driver and the move the
platform specific code out of the driver.


>
> --
> David Gibson                    | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
>                                 | _way_ _around_!
> http://www.ozlabs.org/~dgibson
>


-- 
Jon Smirl
jonsmirl@gmail.com

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 15:34       ` Jon Smirl
@ 2008-01-03 17:54         ` Timur Tabi
  2008-01-03 18:13           ` Grant Likely
  2008-01-03 23:51           ` David Gibson
  0 siblings, 2 replies; 89+ messages in thread
From: Timur Tabi @ 2008-01-03 17:54 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Liam Girdwood, alsa-devel, linuxppc-dev

Jon Smirl wrote:
> On 1/2/08, Timur Tabi <timur@freescale.com> wrote:
>> Jon Smirl wrote:
>>> On 1/1/08, Jon Smirl <jonsmirl@gmail.com> wrote:
>>>> On 12/19/07, Timur Tabi <timur@freescale.com> wrote:
>>>>> +               ssi@16000 {
>>>>> +                       compatible = "fsl,ssi";
>>>>> +                       cell-index = <0>;
>>>>> +                       reg = <16000 100>;
>>>>> +                       interrupt-parent = <&mpic>;
>>>>> +                       interrupts = <3e 2>;
>>>>> +                       fsl,mode = "i2s-slave";
>>>>> +                       codec {
>>>>> +                               compatible = "cirrus,cs4270";
>>>>> +                               /* MCLK source is a stand-alone oscillator */
>>>>> +                               bus-frequency = <bb8000>;
>>>>> +                       };
>>>>> +               };
>>>> Does this need to be bus-frequency? It's always called MCLK in all of
>>>> the literature.
>>>>
>>>> In my case the MCLK comes from a chip on the i2c bus that is
>>>> programmable How would that be encoded?.
>>> Looking at the cs4270 codec driver it is controlled by i2c (supports
>>> SPI too).  What happened to the conversation about putting codecs on
>>> the controlling bus and then linking them to the data bus?
>> The current CS4270 driver doesn't support device trees.  When I wrote
>> it, the idea of putting I2C info in the device tree was not finalized,
>> and since the driver is supposed to be cross-platform, I decided to do
>> it the old-fashioned way.  Before I update the code, however, I'm
>> waiting for:
>>
>> 1) The current code to be accepted into the tree
>> 2) ASoC is updated to V2
>> 3) The current drivers are updated to support ASoC V2.
> 
> I've been trying to get the i2c code in for two months. Hopefully it
> will go in soon, no one had made any comments on it recently. Have you
> tried your code with it?

No.  I don't like updating my patches with new features while they're 
undergoing review.  If something is clearly wrong with the patch, then I'll 
fix it and resubmit.  But I really don't like to support new stuff just 
because it's there.

> There is nothing stopping your from putting a node for the CS4270 on
> the i2c bus today. It just won't trigger the loading of the driver.

Yes, the thing that's stopping me is that I don't want to do 20 things at 
once.  I already have pending patches that I'm trying to get in.  Once those 
are in, then I will consider additional work.

> Don't we want to follow the device tree policy of putting the device
> on the controlling bus and then link it to the data bus?

Normally, that sounds like a good idea, but the cs4270 is an I2S device first, 
and an I2C device second.  I need to be able to find that codec from the I2S 
node.  My I2S driver would not know to scan all I2C devices to find the codec.

> It makes it a little easier but it doesn't fix everything. We need to
> start looking at it since none of the example driver for it are device
> tree based.

I will look at it, *after* my current V1 driver has been applied to the tree.

 > It still has problems with wanting 'struct
> platform_device' when we have 'struct of_platform_device' pointers. It
> also doesn't know how to dynamically load codecs based on device
> trees.

I agree that these things need to be fixed.  I look forward to thinking about 
these problems, *after* my V1 patches have been applied.

> Liam messed up all of my code when he refactored it in late December.

Bummer.

> I've switched over to the current SOC code for the moment. The big
> thing that v2 fixes is that SOC is changed to being a subsystem
> instead of platform driver. Being a subsystem is the correct model.
> 
> It would be good if more pieces of v2 get push forward. Then we can
> sort out the device tree issues in it.

I agree.

> Adding the second device tree node doesn't have anything to do with
> ASOC v2. It's specific to powerpc and device trees.

Ok, but making my CS4270 driver device-tree aware is a completely separate 
task from what this patchset is addressing.


-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 15:56     ` Jon Smirl
  2008-01-02 16:32       ` Grant Likely
@ 2008-01-03 17:57       ` Timur Tabi
  1 sibling, 0 replies; 89+ messages in thread
From: Timur Tabi @ 2008-01-03 17:57 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel

Jon Smirl wrote:

> For this model to work you need to split your driver. fsl-ssi and
> mpc8610_hpcd need to be in  two separate drivers. 

They are two separate drivers.  sound/soc/fsl/fsl_ssi.c and 
sound/soc/fsl/mpc8610_hpcd.c

 > fsl-ssi  is easy
> enough to load since it has a device tree entry.
> 
> mpc8610_hpcd is the harder one to load since it doesn't have a device
> tree entry. What you want to do it match on the compatible field of
> the root node.
> 
> static struct of_device_id fabric_of_match[] = {
> 	{
> 		.compatible	= "fsl,MPC8610HPCD",
> 	},
> 	{},
> };
> 
> But this doesn't work since the root is the device tree isn't passed
> down into the device probe code. (Could this be fixed?)

I don't understand that sentence.  Is there a typo?

> Instead we could make the separated mpc8610_hpcd fabric driver attach
> to fsl,ssi.
> 
> static struct of_device_id fabric_of_match[] = {
> 	{
> 		.compatible	= "fsl,ssi",
> 	},
> 	{},
> };
> 
> Then in it's probe code check for the right platform.

That's what I do.  I attach to fsl,ssi, gather the information from the device 
tree, and then call a private API to initialize the SSI driver.


-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 16:12       ` Grant Likely
@ 2008-01-03 18:08         ` Timur Tabi
  2008-01-03 18:17           ` Grant Likely
  0 siblings, 1 reply; 89+ messages in thread
From: Timur Tabi @ 2008-01-03 18:08 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, alsa-devel

Grant Likely wrote:

> Why not be a child of the i2c bus with a phandle to the ssi bus? 

Because when I probe the SSI node, I want to know what the attached codec is. 
  So if anything, I would need a pointer from the SSI bus *to* the respective 
child on the I2C bus.

I know little about platform devices/drivers, so I don't know how to use them.

Currently, I have a design flaw in my driver in that if I have two SSIs, and 
each one is attached to a CS4270, I don't have any way of making sure that the 
right CS4270 is using the right I2C address.  I'm guessing that if I switch to 
a platform-based model, I can resolve this issue.  But for now, the CS4270 
doesn't support that, so that patchset I have submitted works with what I 
have.  After my patchset has been applied, I'll be more than happy to look 
into updating the CS4270 (and everything else) to use the platform model for I2C.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 17:54         ` Timur Tabi
@ 2008-01-03 18:13           ` Grant Likely
  2008-01-03 18:20             ` Timur Tabi
  2008-01-03 23:51           ` David Gibson
  1 sibling, 1 reply; 89+ messages in thread
From: Grant Likely @ 2008-01-03 18:13 UTC (permalink / raw)
  To: Timur Tabi; +Cc: Liam Girdwood, alsa-devel, linuxppc-dev

On 1/3/08, Timur Tabi <timur@freescale.com> wrote:
> Jon Smirl wrote:
> > Don't we want to follow the device tree policy of putting the device
> > on the controlling bus and then link it to the data bus?
>
> Normally, that sounds like a good idea, but the cs4270 is an I2S device first,
> and an I2C device second.  I need to be able to find that codec from the I2S
> node.  My I2S driver would not know to scan all I2C devices to find the codec.

The device tree is a description of the hardware; not software.  It's
not a good idea to break with convention due to current driver
architecture.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 16:28     ` Grant Likely
  2008-01-02 18:49       ` [alsa-devel] " Mark Brown
@ 2008-01-03 18:14       ` Timur Tabi
  2008-01-03 18:25         ` Grant Likely
  1 sibling, 1 reply; 89+ messages in thread
From: Timur Tabi @ 2008-01-03 18:14 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, alsa-devel

Grant Likely wrote:

> Does that mean with ASoC V2 you can instantiate it with the board
> specific platform code instead? 

I don't know.  I haven't really looked at V2 yet.  You'll have to ask Liam 
Girdwood.

> This is one of the examples of where the compatible properties are
> trying to be far to generic about what they are.  How do you define
> what "fsl,ssi" is? 

The SSI is a specific Freescale device, so I think it's pretty well defined.

> What happens when Freescale produces another
> peripheral that can do ssi but isn't register level compatible?

It won't be called the SSI.  It will be called something else.

> In my opinion, it is far better to be specific in the device tree and
> teach the driver about what versions it is able to bind against.  In
> this case, I would use "fsl,mpc8610-ssi" or maybe better yet:
> "fsl,mpc8610-ssi,i2s" (MPC8610 SSI device in I2S mode).

I can work with that, but the SSI could be placed into any future 83xx, 85xx, 
or 86xx SOC, and the driver will still work with it as-is.

> I don't like the idea of a separate fsl,mode property to describe the
> behaviour of multifunction peripherals.  It makes probing more
> difficult when there is a different driver for each mode.

Can you propose an alternative?  The driver needs to know what mode to use 
when communicating with its codec.  How am I supposed to know if I have an I2S 
codec or an AC97 codec?

>> The fabric driver is specific to the board.  So you should be using
>> Kconfig to select the fabric driver.  There is no node in the device
>> tree for fabric drivers.  I thought that was the consensus.
> 
> No, the desire is to go multiplatform in ppc.  That means you cannot
> use Kconfig to select the correct fabric driver.

I don't see any way of avoiding this with ASoC V1.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 18:49       ` [alsa-devel] " Mark Brown
@ 2008-01-03 18:16         ` Timur Tabi
  2008-01-03 23:47           ` David Gibson
  0 siblings, 1 reply; 89+ messages in thread
From: Timur Tabi @ 2008-01-03 18:16 UTC (permalink / raw)
  To: Grant Likely, Timur Tabi, linuxppc-dev, alsa-devel

Mark Brown wrote:

> The machine support code (fabric driver in PowerPC terms, I think?)
> tells the core how everything is connected together by registering
> devices representing the links (eg, I2S) between the codecs, CPU and
> other devices.  The ASoC core is then responsible for ensuring that all
> the required components are present before it registers with the ALSA
> core.

I'm no expert on this, but I think from the PowerPC point-of-view, the *ideal* 
situation would be if the ASoC fabric driver were generic, maybe even part of 
ASoC itself, and everything it needed could be obtained from the device tree.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03  4:44     ` David Gibson
  2008-01-03 14:54       ` Jon Smirl
@ 2008-01-03 18:16       ` Timur Tabi
  1 sibling, 0 replies; 89+ messages in thread
From: Timur Tabi @ 2008-01-03 18:16 UTC (permalink / raw)
  To: Timur Tabi, Jon Smirl, linuxppc-dev, alsa-devel

David Gibson wrote:

> Instantiating the fabric driver off any node is wrong, precisely
> because it is an abstraction.  The fabric driver should be
> instantiated by the platform code.

Can you tell me how to do that?


-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 18:08         ` Timur Tabi
@ 2008-01-03 18:17           ` Grant Likely
  2008-01-03 18:54             ` Scott Wood
  0 siblings, 1 reply; 89+ messages in thread
From: Grant Likely @ 2008-01-03 18:17 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On 1/3/08, Timur Tabi <timur@freescale.com> wrote:
> Grant Likely wrote:
>
> > Why not be a child of the i2c bus with a phandle to the ssi bus?
>
> Because when I probe the SSI node, I want to know what the attached codec is.
>   So if anything, I would need a pointer from the SSI bus *to* the respective
> child on the I2C bus.

That's fine too (it's what is done with Ethernet PHYs).  My preference
is the other way around, but it's not a big issue in this case.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 18:13           ` Grant Likely
@ 2008-01-03 18:20             ` Timur Tabi
  2008-01-03 18:32               ` Grant Likely
  0 siblings, 1 reply; 89+ messages in thread
From: Timur Tabi @ 2008-01-03 18:20 UTC (permalink / raw)
  To: Grant Likely; +Cc: Liam Girdwood, alsa-devel, linuxppc-dev

Grant Likely wrote:

> The device tree is a description of the hardware; not software.  It's
> not a good idea to break with convention due to current driver
> architecture.

I believe that with ASoC V1, I'm stuck between a rock and a hard place, and so 
the only way to make this code work is to bend some rules.  Right now, the 
CS4270 driver does not support platform drivers or the device tree, so there's 
no point in putting a child I2C node for it.  As I mentioned in other posts, I 
will be more than happy to update the CS4270 driver to support this new 
paradigm (which was invented after the CS4270 driver was written) *after* this 
current patchset is applied.


-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-02 17:23     ` [alsa-devel] " Mark Brown
@ 2008-01-03 18:23       ` Timur Tabi
  2008-01-03 23:00         ` Mark Brown
  0 siblings, 1 reply; 89+ messages in thread
From: Timur Tabi @ 2008-01-03 18:23 UTC (permalink / raw)
  To: Timur Tabi, Jon Smirl, linuxppc-dev, alsa-devel

Mark Brown wrote:

>> clock1 = <0, bb8000>
> 
>> Would that be better?
> 
> To cover everything you'd need to be able to specify all the clocking
> parameters, especially a PLL configuration, and also specify more than
> one of each item.  Even then you'd still have problems like...

The ASoC V1 API for communicating clock data from the fabric driver to the 
codec driver only allows for three parameters.

> According to the documentation in your patch the bus frequency should
> already be optional 

My code does not currently support that configuration, and I don't have any 
hardware that works that way, so I don't know what it would look like.  I'm 
just trying to make the driver as flexible as possible, given ASoC V1 constraints.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 18:14       ` Timur Tabi
@ 2008-01-03 18:25         ` Grant Likely
  2008-01-03 18:28           ` Timur Tabi
  0 siblings, 1 reply; 89+ messages in thread
From: Grant Likely @ 2008-01-03 18:25 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On 1/3/08, Timur Tabi <timur@freescale.com> wrote:
> Grant Likely wrote:
>
> > Does that mean with ASoC V2 you can instantiate it with the board
> > specific platform code instead?
>
> I don't know.  I haven't really looked at V2 yet.  You'll have to ask Liam
> Girdwood.
>
> > This is one of the examples of where the compatible properties are
> > trying to be far to generic about what they are.  How do you define
> > what "fsl,ssi" is?
>
> The SSI is a specific Freescale device, so I think it's pretty well defined.
>
> > What happens when Freescale produces another
> > peripheral that can do ssi but isn't register level compatible?
>
> It won't be called the SSI.  It will be called something else.

Heh, I've seen enough to know that it's virtually impossible for a
company to maintain a consistent naming scheme all the time.  Better
to be specific now and add generic names sometime in the future rather
than the other way around.

> > In my opinion, it is far better to be specific in the device tree and
> > teach the driver about what versions it is able to bind against.  In
> > this case, I would use "fsl,mpc8610-ssi" or maybe better yet:
> > "fsl,mpc8610-ssi,i2s" (MPC8610 SSI device in I2S mode).
>
> I can work with that, but the SSI could be placed into any future 83xx, 85xx,
> or 86xx SOC, and the driver will still work with it as-is.

The have the device trees claim compatibility with the older
fsl,mpc8610-ssi device specifically.  ie: compatible =
"fsl,mpc83<whatever>-ssi,ac97", "fsl,mpc8610-ssi,ac97";

>
> > I don't like the idea of a separate fsl,mode property to describe the
> > behaviour of multifunction peripherals.  It makes probing more
> > difficult when there is a different driver for each mode.
>
> Can you propose an alternative?  The driver needs to know what mode to use
> when communicating with its codec.  How am I supposed to know if I have an I2S
> codec or an AC97 codec?

Make the compatible property tell you!  :-)  If it's connected to an
I2S codec, then it could be compatible = "fsl,mpc8610-ssi,i2s".  Or
for AC7, compatible = "fsl,mpc8610-ssi,ac97"

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 18:25         ` Grant Likely
@ 2008-01-03 18:28           ` Timur Tabi
  2008-01-03 18:38             ` Grant Likely
  0 siblings, 1 reply; 89+ messages in thread
From: Timur Tabi @ 2008-01-03 18:28 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, alsa-devel

Grant Likely wrote:

> Make the compatible property tell you!  :-)  If it's connected to an
> I2S codec, then it could be compatible = "fsl,mpc8610-ssi,i2s".  Or
> for AC7, compatible = "fsl,mpc8610-ssi,ac97"

That won't work.  There are too many variations.  I think a separate property 
just makes more sense.  Frankly, I don't see what's wrong with it.



-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 18:20             ` Timur Tabi
@ 2008-01-03 18:32               ` Grant Likely
  0 siblings, 0 replies; 89+ messages in thread
From: Grant Likely @ 2008-01-03 18:32 UTC (permalink / raw)
  To: Timur Tabi; +Cc: Liam Girdwood, alsa-devel, linuxppc-dev

On 1/3/08, Timur Tabi <timur@freescale.com> wrote:
> Grant Likely wrote:
>
> > The device tree is a description of the hardware; not software.  It's
> > not a good idea to break with convention due to current driver
> > architecture.
>
> I believe that with ASoC V1, I'm stuck between a rock and a hard place, and so
> the only way to make this code work is to bend some rules.  Right now, the
> CS4270 driver does not support platform drivers or the device tree, so there's
> no point in putting a child I2C node for it.  As I mentioned in other posts, I
> will be more than happy to update the CS4270 driver to support this new
> paradigm (which was invented after the CS4270 driver was written) *after* this
> current patchset is applied.

If you need to bend rules, then do it in a place where it won't bite
us in the butt down the road.  (ie. not with the device tree).  Do
hacky stuff in the platform code if you need to because it can be
changed easily down the road.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 18:28           ` Timur Tabi
@ 2008-01-03 18:38             ` Grant Likely
  0 siblings, 0 replies; 89+ messages in thread
From: Grant Likely @ 2008-01-03 18:38 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On 1/3/08, Timur Tabi <timur@freescale.com> wrote:
> Grant Likely wrote:
>
> > Make the compatible property tell you!  :-)  If it's connected to an
> > I2S codec, then it could be compatible = "fsl,mpc8610-ssi,i2s".  Or
> > for AC7, compatible = "fsl,mpc8610-ssi,ac97"
>
> That won't work.  There are too many variations.  I think a separate property
> just makes more sense.  Frankly, I don't see what's wrong with it.

Sure it will, that's exactly what I'm doing with the 5200, but I won't
argue the point.  My *opinion* is that using compatible is a more
elegant approach for this type of multifunction device, but using a
mode property is neither wrong or bad.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 18:17           ` Grant Likely
@ 2008-01-03 18:54             ` Scott Wood
  2008-01-03 19:13               ` Grant Likely
  0 siblings, 1 reply; 89+ messages in thread
From: Scott Wood @ 2008-01-03 18:54 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, alsa-devel, Timur Tabi

Grant Likely wrote:
> On 1/3/08, Timur Tabi <timur@freescale.com> wrote:
>> Grant Likely wrote:
>>
>>> Why not be a child of the i2c bus with a phandle to the ssi bus?
>> Because when I probe the SSI node, I want to know what the attached codec is.
>>   So if anything, I would need a pointer from the SSI bus *to* the respective
>> child on the I2C bus.
> 
> That's fine too (it's what is done with Ethernet PHYs).  My preference
> is the other way around, but it's not a big issue in this case.

I'd just link in both directions, and let software follow it in 
whichever direction it prefers.

-Scott

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 18:54             ` Scott Wood
@ 2008-01-03 19:13               ` Grant Likely
  2008-01-03 19:18                 ` Scott Wood
  2008-01-05  2:35                 ` Timur Tabi
  0 siblings, 2 replies; 89+ messages in thread
From: Grant Likely @ 2008-01-03 19:13 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, alsa-devel, Timur Tabi

On 1/3/08, Scott Wood <scottwood@freescale.com> wrote:
> Grant Likely wrote:
> > On 1/3/08, Timur Tabi <timur@freescale.com> wrote:
> >> Grant Likely wrote:
> >>
> >>> Why not be a child of the i2c bus with a phandle to the ssi bus?
> >> Because when I probe the SSI node, I want to know what the attached codec is.
> >>   So if anything, I would need a pointer from the SSI bus *to* the respective
> >> child on the I2C bus.
> >
> > That's fine too (it's what is done with Ethernet PHYs).  My preference
> > is the other way around, but it's not a big issue in this case.
>
> I'd just link in both directions, and let software follow it in
> whichever direction it prefers.

Gah!  Don't do that!  Then you need to maintain both directions in the
dts file.  Software is good at generating reverse mappings.  Don't put
that burden on the dts author.  (the software principle of defining
things in one place only applies here)

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 19:13               ` Grant Likely
@ 2008-01-03 19:18                 ` Scott Wood
  2008-01-03 23:13                   ` [alsa-devel] " Mark Brown
  2008-01-05  2:35                 ` Timur Tabi
  1 sibling, 1 reply; 89+ messages in thread
From: Scott Wood @ 2008-01-03 19:18 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, alsa-devel, Timur Tabi

Grant Likely wrote:
> On 1/3/08, Scott Wood <scottwood@freescale.com> wrote:
>> I'd just link in both directions, and let software follow it in
>> whichever direction it prefers.
> 
> Gah!  Don't do that!  Then you need to maintain both directions in the
> dts file.  Software is good at generating reverse mappings.

Software is, however, lousy at correctly wading through 
poorly-structured data (which device trees are full of) to figure out 
how to locate the link it wants to follow backwards.

-Scott

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 18:23       ` Timur Tabi
@ 2008-01-03 23:00         ` Mark Brown
  2008-01-05  2:43           ` Timur Tabi
  0 siblings, 1 reply; 89+ messages in thread
From: Mark Brown @ 2008-01-03 23:00 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On Thu, Jan 03, 2008 at 12:23:08PM -0600, Timur Tabi wrote:
> Mark Brown wrote:

> > To cover everything you'd need to be able to specify all the clocking
> > parameters, especially a PLL configuration, and also specify more than
> > one of each item.  Even then you'd still have problems like...

> The ASoC V1 API for communicating clock data from the fabric driver to the 
> codec driver only allows for three parameters.

Each individual call to set_sysclk() only takes three parameters but it
can be called repeatedly and some configurations are going to require
this.  There's also the set_pll() call which will be required by some
things too (and again that can support multiple PLLs).  

For example, something like this isn't unknown:

 - Set PLL input to pin A.
 - Configure PLL input/output frequencies.
 - Set codec system clock source to be the PLL

and of course the ordering matters.  You can also have other dividers
and clock sources within the codec which need configuring and other
components outside the codec which need configuring to supply the clocks
to the codec.

> > According to the documentation in your patch the bus frequency should
> > already be optional 

> My code does not currently support that configuration, and I don't have any 
> hardware that works that way, so I don't know what it would look like.  I'm 
> just trying to make the driver as flexible as possible, given ASoC V1 constraints.

Indeed.  Providing the device tree stuff doesn't get set in stone I'm
not sure we need to nail this down perfectly for ASoC v1 when we're
running into trouble working around it.

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 19:18                 ` Scott Wood
@ 2008-01-03 23:13                   ` Mark Brown
  0 siblings, 0 replies; 89+ messages in thread
From: Mark Brown @ 2008-01-03 23:13 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, alsa-devel, Timur Tabi

On Thu, Jan 03, 2008 at 01:18:31PM -0600, Scott Wood wrote:
> Grant Likely wrote:

> > Gah!  Don't do that!  Then you need to maintain both directions in the
> > dts file.  Software is good at generating reverse mappings.

> Software is, however, lousy at correctly wading through 
> poorly-structured data (which device trees are full of) to figure out 
> how to locate the link it wants to follow backwards.

Thinking about that from an ASoC v2 perspective the approach that this
immediately suggests is to represent the links between the devices in
the device tree and then have those links reference the attached
devices.

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 18:16         ` Timur Tabi
@ 2008-01-03 23:47           ` David Gibson
  2008-01-04 13:39             ` Mark Brown
  0 siblings, 1 reply; 89+ messages in thread
From: David Gibson @ 2008-01-03 23:47 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On Thu, Jan 03, 2008 at 12:16:19PM -0600, Timur Tabi wrote:
> Mark Brown wrote:
> 
> > The machine support code (fabric driver in PowerPC terms, I think?)
> > tells the core how everything is connected together by registering
> > devices representing the links (eg, I2S) between the codecs, CPU and
> > other devices.  The ASoC core is then responsible for ensuring that all
> > the required components are present before it registers with the ALSA
> > core.
> 
> I'm no expert on this, but I think from the PowerPC point-of-view,
> the *ideal* situation would be if the ASoC fabric driver were
> generic, maybe even part of ASoC itself, and everything it needed
> could be obtained from the device tree.

Nice idea in principle, and may be the way to go ultimately, but very
tricky in practice.  The whole reason the fabric driver concept exists
(from other archs) is that there are an awful lot of variants on how
to wire the sound components together.  Devising a way of expressing
those connections in the device tree that's sufficient will be very
curly.  Then we'd have to build the fabric driver that can parse and
process them all.  And then, people will no doubt produce device trees
with errors in the connection information, so we'll still need
platform-specific workarounds.

If we want sound working any time soon, we'll want to stick with the
platform based fabric drivers for the time being.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 17:54         ` Timur Tabi
  2008-01-03 18:13           ` Grant Likely
@ 2008-01-03 23:51           ` David Gibson
  2008-01-05  2:39             ` [alsa-devel] " Timur Tabi
  1 sibling, 1 reply; 89+ messages in thread
From: David Gibson @ 2008-01-03 23:51 UTC (permalink / raw)
  To: Timur Tabi; +Cc: Liam Girdwood, alsa-devel, linuxppc-dev

On Thu, Jan 03, 2008 at 11:54:24AM -0600, Timur Tabi wrote:
> Jon Smirl wrote:
> > On 1/2/08, Timur Tabi <timur@freescale.com> wrote:
> >> Jon Smirl wrote:
> >>> On 1/1/08, Jon Smirl <jonsmirl@gmail.com> wrote:
> >>>> On 12/19/07, Timur Tabi <timur@freescale.com> wrote:
[snip]
> > Don't we want to follow the device tree policy of putting the device
> > on the controlling bus and then link it to the data bus?
> 
> Normally, that sounds like a good idea, but the cs4270 is an I2S
> device first, and an I2C device second.  I need to be able to find
> that codec from the I2S node.  My I2S driver would not know to scan
> all I2C devices to find the codec.

And what distinction are you drawing between "first" and "second"
here?  For the device tree, the primary bus should the the one by
which it's addressable - i.e. where the control registers are, not
the data path.

Why would the I2S need to scan for codecs?  Wouldn't it be up to the
codec driver to register with I2S?

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 14:54       ` Jon Smirl
@ 2008-01-04  5:01         ` David Gibson
  0 siblings, 0 replies; 89+ messages in thread
From: David Gibson @ 2008-01-04  5:01 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel, Timur Tabi

On Thu, Jan 03, 2008 at 09:54:45AM -0500, Jon Smirl wrote:
> On 1/2/08, David Gibson <david@gibson.dropbear.id.au> wrote:
[snip]
> > Instantiating the fabric driver off any node is wrong, precisely
> > because it is an abstraction.  The fabric driver should be
> > instantiated by the platform code.
> 
> Instantiating it from the platform code forces me to put it either the
> of_platform_bus or the platform_bus since there aren't any other buses
> around when the platform code runs. Platform bus doesn't implement
> dynamic module loading. So that means it has to go onto the
> of_platform_bus. That implies that is it a pseudo-device without a
> pseudo-device entry in the device tree which is fine with me. I'll
> need to poke around in the of_bus code and see if the driver will load
> without a device tree entry.

You're letting implementation warts influence basic design decisions.
This is not sensible.  Step back and think for a moment, work out a
sane organization *then* think how you might need to fix or workaround
limitations of existing infrastructure.

> A simple fix to this would be to let me instantiate the driver off
> from the root node of the tree. That's the conceptually correct place
> for instantiating a driver that extends the platform code. Should I
> try adjusting the of probing code to pass the node in, or are there
> major objections?

The current probing system can't instantiate a device for the root
node in any sane way, since it takes a list of suitable busses.

The constructor based approach we're looking at implementing could do
it.  It should, in any case, be constructing a platform_device - so
the platform bus code would still need to be extended to handle the
module loading.  Creating it as an of_platform device bound to the
root node might be a workable interim solution though.

of_platform_device simply does not *ever* make sense conceptually: the
type of struct device wrapper in use depends on the bus the device is
attached to, not on how we figured out the device was there.  OF can
potentially give information about any sort of device be it
simple-bus, i2c, PCI or whatever connected.

> Also, as others have pointed out, this driver is not an abstraction.
> It represents the mess of wires hooking the codec up to the jacks on
> the back panel and possibly GPIO pins that control the wiring. You
> need this because the pins on HD audio codecs are completely
> reconfigurable and the same chip can be wired in a thousand different
> ways. It lets you have a generic codec driver and the move the
> platform specific code out of the driver.

Well, "abstraction" is maybe not the right word.  But the point is
that the fabric driver doesn't represent a neatly isolated device with
well defined bus connections.  Instead it represents the tangle of
essentially every link between audio devices in the platform.  About
the clearest possible example of a true "platform device" (as opposed
to a device on some bus that just doesn't have any bus-specific
logic).

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 23:47           ` David Gibson
@ 2008-01-04 13:39             ` Mark Brown
  0 siblings, 0 replies; 89+ messages in thread
From: Mark Brown @ 2008-01-04 13:39 UTC (permalink / raw)
  To: Timur Tabi, Grant Likely, linuxppc-dev, alsa-devel

On Fri, Jan 04, 2008 at 10:47:25AM +1100, David Gibson wrote:
> On Thu, Jan 03, 2008 at 12:16:19PM -0600, Timur Tabi wrote:

> > I'm no expert on this, but I think from the PowerPC point-of-view,
> > the *ideal* situation would be if the ASoC fabric driver were
> > generic, maybe even part of ASoC itself, and everything it needed
> > could be obtained from the device tree.

> Nice idea in principle, and may be the way to go ultimately, but very
> tricky in practice.  The whole reason the fabric driver concept exists
> (from other archs) is that there are an awful lot of variants on how
> to wire the sound components together.  Devising a way of expressing
> those connections in the device tree that's sufficient will be very
> curly.  Then we'd have to build the fabric driver that can parse and
> process them all.

Yes, there's an issue with complexity here.  Some of the individual
components are going to have quite a lot of different things to
configure by themselves even for static use and the choices made may
depend on the usage at run time rather than being a static property of
the hardware.  It's also more than just connections - many machine
drivers will provide control for components like analogue switches or
simple amplifiers controlled through GPIO lines or memory mapped
registers (these are generally specific to the board).

As a result I would expect that you will always have systems using
platform based drivers.  I don't think that this is a bad thing -
something that can completely replace them would be able to do anything
that can be done in C in the kernel.

>                    And then, people will no doubt produce device trees
> with errors in the connection information, so we'll still need
> platform-specific workarounds.

The other concern with this is that it risks turning the interface to
the codec and controller drivers into an ABI which isn't expected at the
minute and might cause problems in the future.  At the minute the
drivers export constants to their users defining the parameters they
can configure and (for things like source selection) the possible
values.  These can currently be changed at will and there's no great
consistency in their values between drivers.

There would also be difficulties in writing the device tree - without
the symbolic names you're going to end up with strings of numeric
constants in the device tree which are not going to be terribly readable
and will be error prone.

> If we want sound working any time soon, we'll want to stick with the
> platform based fabric drivers for the time being.

Like I say, I would expect that you're always going to want to have
platform based drivers.  Even if a given board can be represented in a
device tree some users will find it more straightforward or convenient
to write C code for their platform and have the device tree specify more
basic configuration options that correspond to the things they want to
vary between boards.

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 19:13               ` Grant Likely
  2008-01-03 19:18                 ` Scott Wood
@ 2008-01-05  2:35                 ` Timur Tabi
  2008-01-05  3:28                   ` Grant Likely
  1 sibling, 1 reply; 89+ messages in thread
From: Timur Tabi @ 2008-01-05  2:35 UTC (permalink / raw)
  To: Grant Likely; +Cc: Scott Wood, linuxppc-dev, alsa-devel

Grant Likely wrote:
> On 1/3/08, Scott Wood <scottwood@freescale.com> wrote:
>> Grant Likely wrote:
>>> On 1/3/08, Timur Tabi <timur@freescale.com> wrote:
>>>> Grant Likely wrote:
>>>>
>>>>> Why not be a child of the i2c bus with a phandle to the ssi bus?
>>>> Because when I probe the SSI node, I want to know what the attached codec is.
>>>>   So if anything, I would need a pointer from the SSI bus *to* the respective
>>>> child on the I2C bus.
>>> That's fine too (it's what is done with Ethernet PHYs).  My preference
>>> is the other way around, but it's not a big issue in this case.
>> I'd just link in both directions, and let software follow it in
>> whichever direction it prefers.
> 
> Gah!  Don't do that!  Then you need to maintain both directions in the
> dts file. 

So?  What's wrong with that?

> Software is good at generating reverse mappings. 

What software would that be?  Currently, there is no software that will do 
that?  Or are you saying that you want my driver to search the entire device 
tree until it finds the reverse mapping?  I don't think *that* is a good idea.

> Don't put
> that burden on the dts author. 

As the DTS author in question, I hereby declare that such a requirement is not 
a burden in the slightest.  Thank you.


-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 23:51           ` David Gibson
@ 2008-01-05  2:39             ` Timur Tabi
  2008-01-06  0:46               ` David Gibson
  0 siblings, 1 reply; 89+ messages in thread
From: Timur Tabi @ 2008-01-05  2:39 UTC (permalink / raw)
  To: Timur Tabi, Jon Smirl, Liam Girdwood, alsa-devel, linuxppc-dev

David Gibson wrote:

> And what distinction are you drawing between "first" and "second"
> here? 

Oh, that's an easy one:  The CS4270 can work without an I2C or SPI connection, 
but it will never work without an I2S connection.

> Why would the I2S need to scan for codecs?  Wouldn't it be up to the
> codec driver to register with I2S?

Not in ASoC V1.  The codec driver registers with ASoC, but the actual 
connection to other devices (e.g. the I2S driver) is done either in the I2S 
driver or in the fabric driver, depending on your mood.  And that connection 
is done via a pointer to a structure in the codec driver.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-03 23:00         ` Mark Brown
@ 2008-01-05  2:43           ` Timur Tabi
  2008-01-07 13:37             ` Mark Brown
  0 siblings, 1 reply; 89+ messages in thread
From: Timur Tabi @ 2008-01-05  2:43 UTC (permalink / raw)
  To: Jon Smirl, linuxppc-dev, alsa-devel

Mark Brown wrote:

> Each individual call to set_sysclk() only takes three parameters but it
> can be called repeatedly and some configurations are going to require
> this. 

In other words, ...

clock1 = <0, bb8000>
clock2 = <1, 653230>
clock23 = <0, ab2372>

> and of course the ordering matters.  

Ok, you got me there.  But then, isn't this just another example where the 
device tree is incapable of describing a complex configuration, and so we need 
a platform driver?


> Indeed.  Providing the device tree stuff doesn't get set in stone I'm
> not sure we need to nail this down perfectly for ASoC v1 when we're
> running into trouble working around it.

I definitely agree with that.  I'll be the first to admit that this driver, 
much like ASoC V1, is a prototype.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-05  2:35                 ` Timur Tabi
@ 2008-01-05  3:28                   ` Grant Likely
  0 siblings, 0 replies; 89+ messages in thread
From: Grant Likely @ 2008-01-05  3:28 UTC (permalink / raw)
  To: Timur Tabi; +Cc: Scott Wood, linuxppc-dev, alsa-devel

On 1/4/08, Timur Tabi <timur@freescale.com> wrote:
> Grant Likely wrote:
> > Don't put
> > that burden on the dts author.
>
> As the DTS author in question, I hereby declare that such a requirement is not
> a burden in the slightest.  Thank you.

Dude, you work for *Freescale*.  The set of dts authors affected
include every engineer writing a device tree for a board that uses
this part.  :-)

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-05  2:39             ` [alsa-devel] " Timur Tabi
@ 2008-01-06  0:46               ` David Gibson
  2008-01-07 14:24                 ` Mark Brown
  2008-01-07 15:52                 ` Timur Tabi
  0 siblings, 2 replies; 89+ messages in thread
From: David Gibson @ 2008-01-06  0:46 UTC (permalink / raw)
  To: Timur Tabi; +Cc: Liam Girdwood, alsa-devel, linuxppc-dev

On Fri, Jan 04, 2008 at 08:39:54PM -0600, Timur Tabi wrote:
> David Gibson wrote:
> 
> > And what distinction are you drawing between "first" and "second"
> > here? 
> 
> Oh, that's an easy one:  The CS4270 can work without an I2C or SPI connection, 
> but it will never work without an I2S connection.
> 
> > Why would the I2S need to scan for codecs?  Wouldn't it be up to the
> > codec driver to register with I2S?
> 
> Not in ASoC V1.  The codec driver registers with ASoC, but the actual 
> connection to other devices (e.g. the I2S driver) is done either in the I2S 
> driver or in the fabric driver, depending on your mood.  And that connection 
> is done via a pointer to a structure in the codec driver.

Ok, but couldn't you strucutre your I2S or fabric driver so that it
only becomes fully operational once the codec driver has registered
with it?

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-05  2:43           ` Timur Tabi
@ 2008-01-07 13:37             ` Mark Brown
  0 siblings, 0 replies; 89+ messages in thread
From: Mark Brown @ 2008-01-07 13:37 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel

On Fri, Jan 04, 2008 at 08:43:17PM -0600, Timur Tabi wrote:
> Mark Brown wrote:

> In other words, ...

> clock1 = <0, bb8000>
> clock2 = <1, 653230>
> clock23 = <0, ab2372>

Yes, something like that would cover it.  I'm not sure what is idiomatic
for the device tree.

> > and of course the ordering matters.  

> Ok, you got me there.  But then, isn't this just another example where the 
> device tree is incapable of describing a complex configuration, and so we need 
> a platform driver?

Yes, you could certainly do that - as you say, any device tree based
configuration would be optional so it's not a blocker if some things
aren't supported.

It'd be nice to have some idea of how to handle it should someone want
to do it but I wouldn't think it's essential.  The most common case
where specific ordering is required is that a PLL will need to have all
its inputs configured before the PLL is activated so it'd probably cover
a large proportion of cases to do that last.

> > Indeed.  Providing the device tree stuff doesn't get set in stone I'm
> > not sure we need to nail this down perfectly for ASoC v1 when we're
> > running into trouble working around it.

> I definitely agree with that.  I'll be the first to admit that this driver, 
> much like ASoC V1, is a prototype.

Yes, from an ASoC point of view the driver looks good as it is.  The
only discussion is about the PowerPC probing stuff.

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-06  0:46               ` David Gibson
@ 2008-01-07 14:24                 ` Mark Brown
  2008-01-07 15:52                 ` Timur Tabi
  1 sibling, 0 replies; 89+ messages in thread
From: Mark Brown @ 2008-01-07 14:24 UTC (permalink / raw)
  To: David Gibson; +Cc: Liam Girdwood, alsa-devel, Timur Tabi, linuxppc-dev

On Sun, Jan 06, 2008 at 11:46:37AM +1100, David Gibson wrote:

> Ok, but couldn't you strucutre your I2S or fabric driver so that it
> only becomes fully operational once the codec driver has registered
> with it?

That's what ASoC v2 is doing, more or less (the core brings things on
line rather than drivers doing it).  For v1 so long as it doesn't cause
any problems in practice I'm not sure I'd worry about it too much.

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-06  0:46               ` David Gibson
  2008-01-07 14:24                 ` Mark Brown
@ 2008-01-07 15:52                 ` Timur Tabi
  2008-01-07 18:28                   ` Mark Brown
  2008-01-07 18:44                   ` Liam Girdwood
  1 sibling, 2 replies; 89+ messages in thread
From: Timur Tabi @ 2008-01-07 15:52 UTC (permalink / raw)
  To: Timur Tabi, Jon Smirl, Liam Girdwood, alsa-devel, linuxppc-dev

David Gibson wrote:

> Ok, but couldn't you strucutre your I2S or fabric driver so that it
> only becomes fully operational once the codec driver has registered
> with it?

Not in ASoC V1.  You have to understand, ASoC V1 was designed without any 
consideration for runtime-bindings and other OF goodies.  All connections 
between the drivers are static, literally.  In fact, I wouldn't be surprised if 
some ASoC drivers cannot be compiled as modules.

So all I'm trying to do now is get my driver, with warts and all, into the tree 
so that I can focus with Liam et al to get a real ASoC V2 up and running.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-07 15:52                 ` Timur Tabi
@ 2008-01-07 18:28                   ` Mark Brown
  2008-01-10  3:49                     ` David Gibson
  2008-01-07 18:44                   ` Liam Girdwood
  1 sibling, 1 reply; 89+ messages in thread
From: Mark Brown @ 2008-01-07 18:28 UTC (permalink / raw)
  To: Timur Tabi; +Cc: Liam Girdwood, alsa-devel, linuxppc-dev

On Mon, Jan 07, 2008 at 09:52:03AM -0600, Timur Tabi wrote:
> David Gibson wrote:

> > Ok, but couldn't you strucutre your I2S or fabric driver so that it
> > only becomes fully operational once the codec driver has registered
> > with it?

> Not in ASoC V1.  You have to understand, ASoC V1 was designed without any 
> consideration for runtime-bindings and other OF goodies.  All connections 
> between the drivers are static, literally.  In fact, I wouldn't be surprised if 
> some ASoC drivers cannot be compiled as modules.

I'd just like to emphasise this point - ASoC v1 really doesn't
understand the idea that the components of the sound subsystem might be
probed separately.  It's set up to handle bare hardware with everything
being probed from code in the machine/fabric driver.  This makes life
very messy for platforms with something like the device tree.

As has been said, handling this properly is one of the major motivations
behind ASoC v2.

> So all I'm trying to do now is get my driver, with warts and all, into the tree 
> so that I can focus with Liam et al to get a real ASoC V2 up and running.

This is certainly the approach we want to take from an ASoC point of
view.

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-07 15:52                 ` Timur Tabi
  2008-01-07 18:28                   ` Mark Brown
@ 2008-01-07 18:44                   ` Liam Girdwood
  2008-01-07 18:45                     ` Timur Tabi
  1 sibling, 1 reply; 89+ messages in thread
From: Liam Girdwood @ 2008-01-07 18:44 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel, Mark Brown

On Mon, 2008-01-07 at 09:52 -0600, Timur Tabi wrote:
> 
> So all I'm trying to do now is get my driver, with warts and all, into the tree 
> so that I can focus with Liam et al to get a real ASoC V2 up and running.
> 

I'll commit the MPC8610 into the ASoC (v1) dev tree soon (hopefully
tonight). This will allow folks to use it in the meantime whilst we sort
out any changes.

I'll then port (what I can) to V2, although I may need some assistance
with some of the PPC sections.

Fwiw we are looking at submitting V2 in March/April time. 


Liam

PS. Sorry for the silence lately. We've just moved to a new opensource
server over the holidays and have been without some services i.e. mail.



Privacy & Confidentiality Notice
-------------------------------------------------
This message and any attachments contain privileged and confidential information that is intended solely for the person(s) to whom it is addressed. If you are not an intended recipient you must not: read; copy; distribute; discuss; take any action in or make any reliance upon the contents of this message; nor open or read any attachment. If you have received this message in error, please notify us as soon as possible on the following telephone number and destroy this message including any attachments. Thank you.
-------------------------------------------------
Wolfson Microelectronics plc
Tel: +44 (0)131 272 7000
Fax: +44 (0)131 272 7001
Web: www.wolfsonmicro.com

Registered in Scotland

Company number SC089839

Registered office: 

Westfield House, 26 Westfield Road, Edinburgh, EH11 2QB, UK

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-07 18:44                   ` Liam Girdwood
@ 2008-01-07 18:45                     ` Timur Tabi
  0 siblings, 0 replies; 89+ messages in thread
From: Timur Tabi @ 2008-01-07 18:45 UTC (permalink / raw)
  To: Liam Girdwood; +Cc: linuxppc-dev, alsa-devel, Mark Brown

Liam Girdwood wrote:
> On Mon, 2008-01-07 at 09:52 -0600, Timur Tabi wrote:
>> So all I'm trying to do now is get my driver, with warts and all, into the tree 
>> so that I can focus with Liam et al to get a real ASoC V2 up and running.
>>
> 
> I'll commit the MPC8610 into the ASoC (v1) dev tree soon (hopefully
> tonight). This will allow folks to use it in the meantime whilst we sort
> out any changes.

I'm working on some minor updates, so hold off for now.  I'll post a new patch 
later this afternoon.

> I'll then port (what I can) to V2, although I may need some assistance
> with some of the PPC sections.

I'll be 100% available for that.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-07 18:28                   ` Mark Brown
@ 2008-01-10  3:49                     ` David Gibson
  2008-01-10  5:41                       ` Jon Smirl
  0 siblings, 1 reply; 89+ messages in thread
From: David Gibson @ 2008-01-10  3:49 UTC (permalink / raw)
  To: Timur Tabi, Jon Smirl, Liam Girdwood, alsa-devel, linuxppc-dev

On Mon, Jan 07, 2008 at 06:28:54PM +0000, Mark Brown wrote:
> On Mon, Jan 07, 2008 at 09:52:03AM -0600, Timur Tabi wrote:
> > David Gibson wrote:
> 
> > > Ok, but couldn't you strucutre your I2S or fabric driver so that it
> > > only becomes fully operational once the codec driver has registered
> > > with it?
> 
> > Not in ASoC V1.  You have to understand, ASoC V1 was designed without any 
> > consideration for runtime-bindings and other OF goodies.  All connections 
> > between the drivers are static, literally.  In fact, I wouldn't be surprised if 
> > some ASoC drivers cannot be compiled as modules.
> 
> I'd just like to emphasise this point - ASoC v1 really doesn't
> understand the idea that the components of the sound subsystem might be
> probed separately.  It's set up to handle bare hardware with everything
> being probed from code in the machine/fabric driver.  This makes life
> very messy for platforms with something like the device tree.
> 
> As has been said, handling this properly is one of the major motivations
> behind ASoC v2.

Ick.  Ok.

Nonetheless, messing up the device tree to workaround ASoC V1's silly
limitations is not a good idea.  The device tree must represent the
hardware as much as possible.  If that means we have to have a bunch
of platform-specific hacks to instatiate the drivers in the correct
order / combination, that's unfortunate, but there you go.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-10  3:49                     ` David Gibson
@ 2008-01-10  5:41                       ` Jon Smirl
  2008-01-10 10:30                         ` Liam Girdwood
  0 siblings, 1 reply; 89+ messages in thread
From: Jon Smirl @ 2008-01-10  5:41 UTC (permalink / raw)
  To: Timur Tabi, Jon Smirl, Liam Girdwood, alsa-devel, linuxppc-dev

On 1/9/08, David Gibson <david@gibson.dropbear.id.au> wrote:
> On Mon, Jan 07, 2008 at 06:28:54PM +0000, Mark Brown wrote:
> > On Mon, Jan 07, 2008 at 09:52:03AM -0600, Timur Tabi wrote:
> > > David Gibson wrote:
> >
> > > > Ok, but couldn't you strucutre your I2S or fabric driver so that it
> > > > only becomes fully operational once the codec driver has registered
> > > > with it?
> >
> > > Not in ASoC V1.  You have to understand, ASoC V1 was designed without any
> > > consideration for runtime-bindings and other OF goodies.  All connections
> > > between the drivers are static, literally.  In fact, I wouldn't be surprised if
> > > some ASoC drivers cannot be compiled as modules.
> >
> > I'd just like to emphasise this point - ASoC v1 really doesn't
> > understand the idea that the components of the sound subsystem might be
> > probed separately.  It's set up to handle bare hardware with everything
> > being probed from code in the machine/fabric driver.  This makes life
> > very messy for platforms with something like the device tree.
> >
> > As has been said, handling this properly is one of the major motivations
> > behind ASoC v2.
>
> Ick.  Ok.
>
> Nonetheless, messing up the device tree to workaround ASoC V1's silly
> limitations is not a good idea.  The device tree must represent the
> hardware as much as possible.  If that means we have to have a bunch
> of platform-specific hacks to instatiate the drivers in the correct
> order / combination, that's unfortunate, but there you go.

ASOC v2 is sitting on a Wolfson server out of tree. I have been using
it for several months without problem. The pace of it being merged
could probably be sped up.

>
> --
> David Gibson                    | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
>                                 | _way_ _around_!
> http://www.ozlabs.org/~dgibson
>


-- 
Jon Smirl
jonsmirl@gmail.com

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-10  5:41                       ` Jon Smirl
@ 2008-01-10 10:30                         ` Liam Girdwood
  2008-01-10 15:39                           ` Timur Tabi
  0 siblings, 1 reply; 89+ messages in thread
From: Liam Girdwood @ 2008-01-10 10:30 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel, Timur Tabi

On Thu, 2008-01-10 at 00:41 -0500, Jon Smirl wrote:

> ASOC v2 is sitting on a Wolfson server out of tree. I have been using
> it for several months without problem. The pace of it being merged
> could probably be sped up.

I think we are probably looking at submission in the next 8 - 10 weeks.
Currently most of the core code is complete, however some platforms and
codecs still need porting.

Liam

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-10 10:30                         ` Liam Girdwood
@ 2008-01-10 15:39                           ` Timur Tabi
  2008-01-10 16:01                             ` Grant Likely
  0 siblings, 1 reply; 89+ messages in thread
From: Timur Tabi @ 2008-01-10 15:39 UTC (permalink / raw)
  To: Jon Smirl, david, Grant Likely, Olof Johansson
  Cc: linuxppc-dev, alsa-devel, Liam Girdwood

Liam Girdwood wrote:

> I think we are probably looking at submission in the next 8 - 10 weeks.
> Currently most of the core code is complete, however some platforms and
> codecs still need porting.

With that in mind, can I get some kind of consensus from the PPC side as to 
whether this ASoC V1 driver is okay?  I want to get it into 2.6.25.

Keep in mind:

1) ASoC V1 is not PowerPC-friendly, so it's impossible to make an ASoC V1 
PowerPC driver "100% correct".

2) The CS4270 driver does not support I2C nodes in the device tree, so there's 
not point in adding any to the 8610 DTS.

3) Liam and I are working on porting this driver to ASoC V2 and resolving all 
open PPC issue, but that won't be done in time for 2.6.25.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-10 15:39                           ` Timur Tabi
@ 2008-01-10 16:01                             ` Grant Likely
  2008-01-10 16:03                               ` Timur Tabi
  0 siblings, 1 reply; 89+ messages in thread
From: Grant Likely @ 2008-01-10 16:01 UTC (permalink / raw)
  To: Timur Tabi; +Cc: alsa-devel, Liam Girdwood, linuxppc-dev, Olof Johansson, david

On 1/10/08, Timur Tabi <timur@freescale.com> wrote:
> Liam Girdwood wrote:
>
> > I think we are probably looking at submission in the next 8 - 10 weeks.
> > Currently most of the core code is complete, however some platforms and
> > codecs still need porting.
>
> With that in mind, can I get some kind of consensus from the PPC side as to
> whether this ASoC V1 driver is okay?  I want to get it into 2.6.25.
>
> Keep in mind:
>
> 1) ASoC V1 is not PowerPC-friendly, so it's impossible to make an ASoC V1
> PowerPC driver "100% correct".

The driver doesn't need to be 100% correct.  Drivers are easy to
change if they aren't quite right.  There are no long term
consequences.

However, the device tree issues must be addressed before it is merged
and deployed.  Otherwise we end up having to support poorly designed
trees over the long term.

So, I'm okay with merging the driver *minus* the .dts and
booting-without-of.txt changes.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-10 16:01                             ` Grant Likely
@ 2008-01-10 16:03                               ` Timur Tabi
  2008-01-10 20:10                                 ` Jon Smirl
  0 siblings, 1 reply; 89+ messages in thread
From: Timur Tabi @ 2008-01-10 16:03 UTC (permalink / raw)
  To: Grant Likely
  Cc: alsa-devel, Liam Girdwood, linuxppc-dev, Olof Johansson, david

Grant Likely wrote:

> The driver doesn't need to be 100% correct.  Drivers are easy to
> change if they aren't quite right.  There are no long term
> consequences.
 >
> However, the device tree issues must be addressed before it is merged
> and deployed.  Otherwise we end up having to support poorly designed
> trees over the long term.

I agree.  Correct me if I'm wrong, but I think the only device tree "issue" is 
the definition of the 'codec' node under the SSI node.  If so, I'm not sure what 
other changes need to be mode.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-10 16:03                               ` Timur Tabi
@ 2008-01-10 20:10                                 ` Jon Smirl
  2008-01-10 20:13                                   ` Timur Tabi
  0 siblings, 1 reply; 89+ messages in thread
From: Jon Smirl @ 2008-01-10 20:10 UTC (permalink / raw)
  To: Timur Tabi; +Cc: alsa-devel, Liam Girdwood, linuxppc-dev, Olof Johansson, david

On 1/10/08, Timur Tabi <timur@freescale.com> wrote:
> Grant Likely wrote:
>
> > The driver doesn't need to be 100% correct.  Drivers are easy to
> > change if they aren't quite right.  There are no long term
> > consequences.
>  >
> > However, the device tree issues must be addressed before it is merged
> > and deployed.  Otherwise we end up having to support poorly designed
> > trees over the long term.
>
> I agree.  Correct me if I'm wrong, but I think the only device tree "issue" is
> the definition of the 'codec' node under the SSI node.  If so, I'm not sure what
> other changes need to be mode.

Isn't your codec is i2c controlled? Where does the main node for the
code live, i2c bus or ssi bus? What does the link between the buses
look like for pointing to the codec entry?

What about fsl,ssi being too generic for a compatible type?

>
> --
> Timur Tabi
> Linux kernel developer at Freescale
>


-- 
Jon Smirl
jonsmirl@gmail.com

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-10 20:10                                 ` Jon Smirl
@ 2008-01-10 20:13                                   ` Timur Tabi
  2008-01-10 20:24                                     ` Grant Likely
  2008-01-10 20:39                                     ` Jon Smirl
  0 siblings, 2 replies; 89+ messages in thread
From: Timur Tabi @ 2008-01-10 20:13 UTC (permalink / raw)
  To: Jon Smirl; +Cc: alsa-devel, Liam Girdwood, linuxppc-dev, Olof Johansson, david

Jon Smirl wrote:

> Isn't your codec is i2c controlled? Where does the main node for the
> code live, i2c bus or ssi bus? What does the link between the buses
> look like for pointing to the codec entry?

The CS4270 driver is "old style", which means it probes all possible I2C 
addresses until it finds a hit, and then stops.  This has all the obvious 
drawbacks, but I'm stuck with that design for now.

> What about fsl,ssi being too generic for a compatible type?

Already fixed:

		ssi@16000 {
			compatible = "fsl,mpc8610-ssi";
-- 
Timur Tabi
Linux kernel developer at Freescale

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-10 20:13                                   ` Timur Tabi
@ 2008-01-10 20:24                                     ` Grant Likely
  2008-01-10 20:35                                       ` Timur Tabi
  2008-01-10 20:39                                     ` Jon Smirl
  1 sibling, 1 reply; 89+ messages in thread
From: Grant Likely @ 2008-01-10 20:24 UTC (permalink / raw)
  To: Timur Tabi; +Cc: alsa-devel, Liam Girdwood, linuxppc-dev, Olof Johansson, david

On 1/10/08, Timur Tabi <timur@freescale.com> wrote:
> Jon Smirl wrote:
>
> > Isn't your codec is i2c controlled? Where does the main node for the
> > code live, i2c bus or ssi bus? What does the link between the buses
> > look like for pointing to the codec entry?
>
> The CS4270 driver is "old style", which means it probes all possible I2C
> addresses until it finds a hit, and then stops.  This has all the obvious
> drawbacks, but I'm stuck with that design for now.
>
> > What about fsl,ssi being too generic for a compatible type?
>
> Already fixed:
>
>                 ssi@16000 {
>                         compatible = "fsl,mpc8610-ssi";

Nit: node name should be either i2s@16000 (the mode that it is in), or
sound@16000 (if you feel that this node encapsulates the concept of a
sound device enough).

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-10 20:24                                     ` Grant Likely
@ 2008-01-10 20:35                                       ` Timur Tabi
  0 siblings, 0 replies; 89+ messages in thread
From: Timur Tabi @ 2008-01-10 20:35 UTC (permalink / raw)
  To: Grant Likely
  Cc: alsa-devel, Liam Girdwood, linuxppc-dev, Olof Johansson, david

Grant Likely wrote:

> Nit: node name should be either i2s@16000 (the mode that it is in), or
> sound@16000 (if you feel that this node encapsulates the concept of a
> sound device enough).

Well, SSI stands for "Synchronous Serial  Interface" (although it's capable of 
asynchronous communication as well).  From the manual:

"The SSI is a full-duplex, serial port that allows the chip to communicate with 
a variety of serial devices.  These serial devices can be standard CODer-DECoder 
(CODECs), Digital Signal Processors (DSPs), microprocessors, peripherals, and 
popular industry audio CODECs that implement the inter-IC sound bus standard 
(I2S) and Intel AC97 standard."

It might an I2S device in this case, but it could be an AC97 device in some 
other case.  It all depends on how the board is wired.  Do we really want to 
change the name of an SOC device based on what it's connected to?

-- 
Timur Tabi
Linux kernel developer at Freescale

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-10 20:13                                   ` Timur Tabi
  2008-01-10 20:24                                     ` Grant Likely
@ 2008-01-10 20:39                                     ` Jon Smirl
  2008-01-10 20:44                                       ` Timur Tabi
  1 sibling, 1 reply; 89+ messages in thread
From: Jon Smirl @ 2008-01-10 20:39 UTC (permalink / raw)
  To: Timur Tabi; +Cc: alsa-devel, Liam Girdwood, linuxppc-dev, Olof Johansson, david

On 1/10/08, Timur Tabi <timur@freescale.com> wrote:
> Jon Smirl wrote:
>
> > Isn't your codec is i2c controlled? Where does the main node for the
> > code live, i2c bus or ssi bus? What does the link between the buses
> > look like for pointing to the codec entry?
>
> The CS4270 driver is "old style", which means it probes all possible I2C
> addresses until it finds a hit, and then stops.  This has all the obvious
> drawbacks, but I'm stuck with that design for now.

So the codec is controlled from the i2c bus and SSI is used to supply
it with data. Based on what has been said on this list, the device
tree node for the codec should be on the i2c bus with a link from the
ssi bus to it.

>
> > What about fsl,ssi being too generic for a compatible type?
>
> Already fixed:
>
>                 ssi@16000 {
>                         compatible = "fsl,mpc8610-ssi";
> --
> Timur Tabi
> Linux kernel developer at Freescale
>


-- 
Jon Smirl
jonsmirl@gmail.com

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

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
  2008-01-10 20:39                                     ` Jon Smirl
@ 2008-01-10 20:44                                       ` Timur Tabi
  0 siblings, 0 replies; 89+ messages in thread
From: Timur Tabi @ 2008-01-10 20:44 UTC (permalink / raw)
  To: Jon Smirl; +Cc: alsa-devel, Liam Girdwood, linuxppc-dev, Olof Johansson, david

Jon Smirl wrote:

> So the codec is controlled from the i2c bus and SSI is used to supply
> it with data. Based on what has been said on this list, the device
> tree node for the codec should be on the i2c bus with a link from the
> ssi bus to it.

I'm working on that now.  I'll have a new patch with this exact change this 
afternoon.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

end of thread, other threads:[~2008-01-10 20:45 UTC | newest]

Thread overview: 89+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-12-20  0:03 [PATCH] ASoC drivers for the Freescale MPC8610 SoC Timur Tabi
2007-12-20  4:06 ` Olof Johansson
2007-12-20 14:24   ` Timur Tabi
2007-12-20 13:54     ` [alsa-devel] " Takashi Iwai
2007-12-20 17:04       ` Timur Tabi
2007-12-21  5:28       ` Lee Revell
2007-12-23  3:23         ` Timur Tabi
2007-12-20 22:39     ` Olof Johansson
2007-12-20 22:37       ` Timur Tabi
2007-12-20 22:43         ` Scott Wood
2007-12-23  2:58           ` Timur Tabi
2008-01-02 18:08             ` Scott Wood
2007-12-20 14:47 ` Jon Loeliger
2007-12-20 22:29 ` Jon Smirl
2007-12-20 22:32   ` Timur Tabi
2007-12-20 22:38     ` Jon Smirl
2007-12-20 22:40       ` Timur Tabi
2007-12-20 22:44         ` Scott Wood
2007-12-20 23:13           ` Jon Smirl
2007-12-21  0:00             ` David Gibson
2008-01-01 17:25 ` Jon Smirl
2008-01-01 17:42   ` Jon Smirl
2008-01-02 15:19     ` Timur Tabi
2008-01-02 15:34       ` Jon Smirl
2008-01-03 17:54         ` Timur Tabi
2008-01-03 18:13           ` Grant Likely
2008-01-03 18:20             ` Timur Tabi
2008-01-03 18:32               ` Grant Likely
2008-01-03 23:51           ` David Gibson
2008-01-05  2:39             ` [alsa-devel] " Timur Tabi
2008-01-06  0:46               ` David Gibson
2008-01-07 14:24                 ` Mark Brown
2008-01-07 15:52                 ` Timur Tabi
2008-01-07 18:28                   ` Mark Brown
2008-01-10  3:49                     ` David Gibson
2008-01-10  5:41                       ` Jon Smirl
2008-01-10 10:30                         ` Liam Girdwood
2008-01-10 15:39                           ` Timur Tabi
2008-01-10 16:01                             ` Grant Likely
2008-01-10 16:03                               ` Timur Tabi
2008-01-10 20:10                                 ` Jon Smirl
2008-01-10 20:13                                   ` Timur Tabi
2008-01-10 20:24                                     ` Grant Likely
2008-01-10 20:35                                       ` Timur Tabi
2008-01-10 20:39                                     ` Jon Smirl
2008-01-10 20:44                                       ` Timur Tabi
2008-01-07 18:44                   ` Liam Girdwood
2008-01-07 18:45                     ` Timur Tabi
2008-01-02 16:12       ` Grant Likely
2008-01-03 18:08         ` Timur Tabi
2008-01-03 18:17           ` Grant Likely
2008-01-03 18:54             ` Scott Wood
2008-01-03 19:13               ` Grant Likely
2008-01-03 19:18                 ` Scott Wood
2008-01-03 23:13                   ` [alsa-devel] " Mark Brown
2008-01-05  2:35                 ` Timur Tabi
2008-01-05  3:28                   ` Grant Likely
2008-01-02  0:26   ` David Gibson
2008-01-02 15:10   ` Timur Tabi
2008-01-02 17:23     ` [alsa-devel] " Mark Brown
2008-01-03 18:23       ` Timur Tabi
2008-01-03 23:00         ` Mark Brown
2008-01-05  2:43           ` Timur Tabi
2008-01-07 13:37             ` Mark Brown
2008-01-02  4:27 ` Jon Smirl
2008-01-02 15:29   ` Timur Tabi
2008-01-02 15:56     ` Jon Smirl
2008-01-02 16:32       ` Grant Likely
2008-01-02 17:12         ` Jon Smirl
2008-01-02 17:22           ` Grant Likely
2008-01-02 18:43             ` Jon Smirl
2008-01-02 18:50               ` Grant Likely
2008-01-02 18:56                 ` Jon Smirl
2008-01-03  4:46           ` David Gibson
2008-01-03 14:33             ` Jon Smirl
2008-01-03 17:57       ` Timur Tabi
2008-01-02 16:28     ` Grant Likely
2008-01-02 18:49       ` [alsa-devel] " Mark Brown
2008-01-03 18:16         ` Timur Tabi
2008-01-03 23:47           ` David Gibson
2008-01-04 13:39             ` Mark Brown
2008-01-03 18:14       ` Timur Tabi
2008-01-03 18:25         ` Grant Likely
2008-01-03 18:28           ` Timur Tabi
2008-01-03 18:38             ` Grant Likely
2008-01-03  4:44     ` David Gibson
2008-01-03 14:54       ` Jon Smirl
2008-01-04  5:01         ` David Gibson
2008-01-03 18:16       ` Timur Tabi

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