[PATCH v3 00/11] ASoC: add imx-sgtl5000 machine driver working with fsl_ssi

[PATCH v3 00/11] ASoC: add imx-sgtl5000 machine driver working with fsl_ssi

[Shawn Guo]

I believe I have addressed all the outstanding comments and requests
reviewers put on previous versions.  So, hopefully this will be the
last iteration of the series.

The current sound/for-next branch seems broken for me, and I have not
tracking the problem down.  But the series works good on SHA below.

  3030763 (Merge tag 'asoc-3.4' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into topic/asoc)

Timur,

I did not pick up you Ack-by tag, because there are some changes since
v2 you will need to take another look and have the testing, I guess.

Regards,
Shawn

Shawn Guo (11):
      ASoC: core: missing set_fmt should not be complaint
      ASoC: fsl: separate SSI and DMA Kconfig options
      ASoC: imx: merge sound/soc/imx into sound/soc/fsl
      ASoC: fsl: rename imx-pcm Kconfig options and filename
      ASoC: fsl: create fsl_utils to accommodate the common functions
      ASoC: fsl: remove helper fsl_asoc_get_codec_dev_name
      ASoC: fsl: check property 'compatible' for the machine name
      ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX
      ASoC: fsl: remove the fatal error checking on codec-handle
      ASoC: fsl: let fsl_ssi work with imx pcm and machine drivers
      ASoC: fsl: add imx-sgtl5000 machine driver

 .../bindings/sound/imx-audio-sgtl5000.txt          |   24 +++
 arch/powerpc/configs/86xx/mpc8610_hpcd_defconfig   |    1 +
 arch/powerpc/configs/mpc85xx_defconfig             |    1 +
 arch/powerpc/configs/mpc85xx_smp_defconfig         |    1 +
 sound/soc/Kconfig                                  |    1 -
 sound/soc/Makefile                                 |    1 -
 sound/soc/fsl/Kconfig                              |  124 ++++++++++++-
 sound/soc/fsl/Makefile                             |   29 +++-
 sound/soc/{imx => fsl}/eukrea-tlv320.c             |    2 +-
 sound/soc/fsl/fsl_ssi.c                            |  146 ++++++++++++----
 sound/soc/fsl/fsl_utils.c                          |   91 ++++++++++
 sound/soc/fsl/fsl_utils.h                          |   26 +++
 sound/soc/{imx => fsl}/imx-audmux.c                |    0
 sound/soc/{imx => fsl}/imx-audmux.h                |    0
 .../{imx/imx-pcm-dma-mx2.c => fsl/imx-pcm-dma.c}   |    0
 sound/soc/{imx => fsl}/imx-pcm-fiq.c               |    0
 sound/soc/{imx => fsl}/imx-pcm.c                   |    0
 sound/soc/{imx => fsl}/imx-pcm.h                   |    0
 sound/soc/fsl/imx-sgtl5000.c                       |  177 ++++++++++++++++++
 sound/soc/{imx => fsl}/imx-ssi.c                   |    2 +-
 sound/soc/{imx => fsl}/imx-ssi.h                   |    0
 sound/soc/fsl/mpc8610_hpcd.c                       |  168 ++----------------
 sound/soc/{imx => fsl}/mx27vis-aic32x4.c           |    0
 sound/soc/fsl/p1022_ds.c                           |  190 ++------------------
 sound/soc/{imx => fsl}/phycore-ac97.c              |    0
 sound/soc/{imx => fsl}/wm1133-ev1.c                |    0
 sound/soc/imx/Kconfig                              |   79 --------
 sound/soc/imx/Makefile                             |   22 ---
 sound/soc/soc-core.c                               |   11 +-
 29 files changed, 616 insertions(+), 480 deletions(-)

[PATCH v3 01/11] ASoC: core: missing set_fmt should not be complaint

[Shawn Guo]

Not having a DAI link set_fmt operation is perfectly normal and
should not be complaint.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/soc-core.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 7978f6c..98b2635 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1531,14 +1531,14 @@ static void snd_soc_instantiate_card(struct snd_soc_card *card)
 		if (dai_link->dai_fmt) {
 			ret = snd_soc_dai_set_fmt(card->rtd[i].codec_dai,
 						  dai_link->dai_fmt);
-			if (ret != 0)
+			if (ret != 0 && ret != -ENOTSUPP)
 				dev_warn(card->rtd[i].codec_dai->dev,
 					 "Failed to set DAI format: %d\n",
 					 ret);
 
 			ret = snd_soc_dai_set_fmt(card->rtd[i].cpu_dai,
 						  dai_link->dai_fmt);
-			if (ret != 0)
+			if (ret != 0 && ret != -ENOTSUPP)
 				dev_warn(card->rtd[i].cpu_dai->dev,
 					 "Failed to set DAI format: %d\n",
 					 ret);
@@ -2971,10 +2971,11 @@ EXPORT_SYMBOL_GPL(snd_soc_codec_set_pll);
  */
 int snd_soc_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 {
-	if (dai->driver && dai->driver->ops->set_fmt)
-		return dai->driver->ops->set_fmt(dai, fmt);
-	else
+	if (dai->driver == NULL)
 		return -EINVAL;
+	if (dai->driver->ops->set_fmt == NULL)
+		return -ENOTSUPP;
+	return dai->driver->ops->set_fmt(dai, fmt);
 }
 EXPORT_SYMBOL_GPL(snd_soc_dai_set_fmt);
 
-- 
1.7.5.4

[PATCH v3 01/11] ASoC: core: missing set_fmt should not be complaint

[Mark Brown]

On Fri, Mar 09, 2012 at 12:59:40AM +0800, Shawn Guo wrote:
> Not having a DAI link set_fmt operation is perfectly normal and
> should not be complaint.

Applied, thanks.  I'll wait for Timur's review on the rest before
applying.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120308/eebd7a5d/attachment.sig>

[PATCH v3 02/11] ASoC: fsl: separate SSI and DMA Kconfig options

[Shawn Guo]

The fsl_ssi driver will possibly be shared between Freescale PowerPC
and ARM/IMX families, so give it a separate Kconfig option.  Then
fsl_ssi driver can possibly be selected independently from selecting
PowerPC DMA based PCM driver.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/fsl/Kconfig  |   15 +++++++++------
 sound/soc/fsl/Makefile |    3 ++-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index d754d34..ca693b2 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -1,10 +1,11 @@
 config SND_MPC52xx_DMA
 	tristate
 
-# ASoC platform support for the Freescale PowerPC SOCs that have an SSI and
-# an Elo DMA controller, such as the MPC8610 and P1022.  You will still need to
-# select a platform driver and a codec driver.
-config SND_SOC_POWERPC_SSI
+config SND_SOC_FSL_SSI
+	tristate
+	depends on FSL_SOC
+
+config SND_SOC_POWERPC_DMA
 	tristate
 	depends on FSL_SOC
 
@@ -12,7 +13,8 @@ config SND_SOC_MPC8610_HPCD
 	tristate "ALSA SoC support for the Freescale MPC8610 HPCD board"
 	# I2C is necessary for the CS4270 driver
 	depends on MPC8610_HPCD && I2C
-	select SND_SOC_POWERPC_SSI
+	select SND_SOC_FSL_SSI
+	select SND_SOC_POWERPC_DMA
 	select SND_SOC_CS4270
 	select SND_SOC_CS4270_VD33_ERRATA
 	default y if MPC8610_HPCD
@@ -23,7 +25,8 @@ config SND_SOC_P1022_DS
 	tristate "ALSA SoC support for the Freescale P1022 DS board"
 	# I2C is necessary for the WM8776 driver
 	depends on P1022_DS && I2C
-	select SND_SOC_POWERPC_SSI
+	select SND_SOC_FSL_SSI
+	select SND_SOC_POWERPC_DMA
 	select SND_SOC_WM8776
 	default y if P1022_DS
 	help
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
index b4a38c0..95d483f 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -9,7 +9,8 @@ obj-$(CONFIG_SND_SOC_P1022_DS) += snd-soc-p1022-ds.o
 # Freescale PowerPC SSI/DMA Platform Support
 snd-soc-fsl-ssi-objs := fsl_ssi.o
 snd-soc-fsl-dma-objs := fsl_dma.o
-obj-$(CONFIG_SND_SOC_POWERPC_SSI) += snd-soc-fsl-ssi.o snd-soc-fsl-dma.o
+obj-$(CONFIG_SND_SOC_FSL_SSI) += snd-soc-fsl-ssi.o
+obj-$(CONFIG_SND_SOC_POWERPC_DMA) += snd-soc-fsl-dma.o
 
 # MPC5200 Platform Support
 obj-$(CONFIG_SND_MPC52xx_DMA) += mpc5200_dma.o
-- 
1.7.5.4

[PATCH v3 03/11] ASoC: imx: merge sound/soc/imx into sound/soc/fsl

[Shawn Guo]

Freescale PowerPC and ARM/IMX families share the same SSI IP block.
The patch merges sound/soc/imx into sound/soc/fsl, so that the possible
code sharing and consolidation can happen.

This is a plain merge, except that menuconfig SND_POWERPC_SOC is added
in Kconfig for PowerPC platform as a correspondence to SND_IMX_SOC for
IMX platform.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 arch/powerpc/configs/86xx/mpc8610_hpcd_defconfig |    1 +
 arch/powerpc/configs/mpc85xx_defconfig           |    1 +
 arch/powerpc/configs/mpc85xx_smp_defconfig       |    1 +
 sound/soc/Kconfig                                |    1 -
 sound/soc/Makefile                               |    1 -
 sound/soc/fsl/Kconfig                            |   92 ++++++++++++++++++++++
 sound/soc/fsl/Makefile                           |   22 +++++
 sound/soc/{imx => fsl}/eukrea-tlv320.c           |    2 +-
 sound/soc/{imx => fsl}/imx-audmux.c              |    0
 sound/soc/{imx => fsl}/imx-audmux.h              |    0
 sound/soc/{imx => fsl}/imx-pcm-dma-mx2.c         |    0
 sound/soc/{imx => fsl}/imx-pcm-fiq.c             |    0
 sound/soc/{imx => fsl}/imx-pcm.c                 |    0
 sound/soc/{imx => fsl}/imx-pcm.h                 |    0
 sound/soc/{imx => fsl}/imx-ssi.c                 |    2 +-
 sound/soc/{imx => fsl}/imx-ssi.h                 |    0
 sound/soc/{imx => fsl}/mx27vis-aic32x4.c         |    0
 sound/soc/{imx => fsl}/phycore-ac97.c            |    0
 sound/soc/{imx => fsl}/wm1133-ev1.c              |    0
 sound/soc/imx/Kconfig                            |   79 -------------------
 sound/soc/imx/Makefile                           |   22 -----
 21 files changed, 119 insertions(+), 105 deletions(-)
 rename sound/soc/{imx => fsl}/eukrea-tlv320.c (99%)
 rename sound/soc/{imx => fsl}/imx-audmux.c (100%)
 rename sound/soc/{imx => fsl}/imx-audmux.h (100%)
 rename sound/soc/{imx => fsl}/imx-pcm-dma-mx2.c (100%)
 rename sound/soc/{imx => fsl}/imx-pcm-fiq.c (100%)
 rename sound/soc/{imx => fsl}/imx-pcm.c (100%)
 rename sound/soc/{imx => fsl}/imx-pcm.h (100%)
 rename sound/soc/{imx => fsl}/imx-ssi.c (99%)
 rename sound/soc/{imx => fsl}/imx-ssi.h (100%)
 rename sound/soc/{imx => fsl}/mx27vis-aic32x4.c (100%)
 rename sound/soc/{imx => fsl}/phycore-ac97.c (100%)
 rename sound/soc/{imx => fsl}/wm1133-ev1.c (100%)
 delete mode 100644 sound/soc/imx/Kconfig
 delete mode 100644 sound/soc/imx/Makefile

diff --git a/arch/powerpc/configs/86xx/mpc8610_hpcd_defconfig b/arch/powerpc/configs/86xx/mpc8610_hpcd_defconfig
index 0db9ba0..c09598b 100644
--- a/arch/powerpc/configs/86xx/mpc8610_hpcd_defconfig
+++ b/arch/powerpc/configs/86xx/mpc8610_hpcd_defconfig
@@ -100,6 +100,7 @@ CONFIG_SND_MIXER_OSS=y
 CONFIG_SND_PCM_OSS=y
 # CONFIG_SND_SUPPORT_OLD_API is not set
 CONFIG_SND_SOC=y
+CONFIG_SND_POWERPC_SOC=y
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_CMOS=y
 CONFIG_EXT2_FS=y
diff --git a/arch/powerpc/configs/mpc85xx_defconfig b/arch/powerpc/configs/mpc85xx_defconfig
index f37a2ab..44605f8 100644
--- a/arch/powerpc/configs/mpc85xx_defconfig
+++ b/arch/powerpc/configs/mpc85xx_defconfig
@@ -139,6 +139,7 @@ CONFIG_SND_INTEL8X0=y
 # CONFIG_SND_PPC is not set
 # CONFIG_SND_USB is not set
 CONFIG_SND_SOC=y
+CONFIG_SND_POWERPC_SOC=y
 CONFIG_HID_A4TECH=y
 CONFIG_HID_APPLE=y
 CONFIG_HID_BELKIN=y
diff --git a/arch/powerpc/configs/mpc85xx_smp_defconfig b/arch/powerpc/configs/mpc85xx_smp_defconfig
index abdcd31..98e227e 100644
--- a/arch/powerpc/configs/mpc85xx_smp_defconfig
+++ b/arch/powerpc/configs/mpc85xx_smp_defconfig
@@ -141,6 +141,7 @@ CONFIG_SND_INTEL8X0=y
 # CONFIG_SND_PPC is not set
 # CONFIG_SND_USB is not set
 CONFIG_SND_SOC=y
+CONFIG_SND_POWERPC_SOC=y
 CONFIG_HID_A4TECH=y
 CONFIG_HID_APPLE=y
 CONFIG_HID_BELKIN=y
diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index 91c9855..0f85f6d 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -35,7 +35,6 @@ source "sound/soc/blackfin/Kconfig"
 source "sound/soc/davinci/Kconfig"
 source "sound/soc/ep93xx/Kconfig"
 source "sound/soc/fsl/Kconfig"
-source "sound/soc/imx/Kconfig"
 source "sound/soc/jz4740/Kconfig"
 source "sound/soc/nuc900/Kconfig"
 source "sound/soc/omap/Kconfig"
diff --git a/sound/soc/Makefile b/sound/soc/Makefile
index 2feaf37..363dfd6 100644
--- a/sound/soc/Makefile
+++ b/sound/soc/Makefile
@@ -12,7 +12,6 @@ obj-$(CONFIG_SND_SOC)	+= blackfin/
 obj-$(CONFIG_SND_SOC)	+= davinci/
 obj-$(CONFIG_SND_SOC)	+= ep93xx/
 obj-$(CONFIG_SND_SOC)	+= fsl/
-obj-$(CONFIG_SND_SOC)   += imx/
 obj-$(CONFIG_SND_SOC)	+= jz4740/
 obj-$(CONFIG_SND_SOC)	+= mid-x86/
 obj-$(CONFIG_SND_SOC)	+= mxs/
diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index ca693b2..19856a0 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -1,3 +1,15 @@
+config SND_SOC_FSL_SSI
+	tristate
+
+menuconfig SND_POWERPC_SOC
+	tristate "SoC Audio for Freescale PowerPC CPUs"
+	depends on FSL_SOC
+	help
+	  Say Y or M if you want to add support for codecs attached to
+	  the PowerPC CPUs.
+
+if SND_POWERPC_SOC
+
 config SND_MPC52xx_DMA
 	tristate
 
@@ -68,3 +80,83 @@ config SND_MPC52xx_SOC_EFIKA
 	help
 	  Say Y if you want to add support for sound on the Efika.
 
+endif # SND_POWERPC_SOC
+
+menuconfig SND_IMX_SOC
+	tristate "SoC Audio for Freescale i.MX CPUs"
+	depends on ARCH_MXC
+	help
+	  Say Y or M if you want to add support for codecs attached to
+	  the i.MX CPUs.
+
+if SND_IMX_SOC
+
+config SND_SOC_IMX_SSI
+	tristate
+
+config SND_SOC_IMX_PCM
+	tristate
+
+config SND_MXC_SOC_FIQ
+	tristate
+	select FIQ
+	select SND_SOC_IMX_PCM
+
+config SND_MXC_SOC_MX2
+	tristate
+	select SND_SOC_DMAENGINE_PCM
+	select SND_SOC_IMX_PCM
+
+config SND_SOC_IMX_AUDMUX
+	tristate
+
+config SND_MXC_SOC_WM1133_EV1
+	tristate "Audio on the the i.MX31ADS with WM1133-EV1 fitted"
+	depends on MACH_MX31ADS_WM1133_EV1 && EXPERIMENTAL
+	select SND_SOC_WM8350
+	select SND_MXC_SOC_FIQ
+	select SND_SOC_IMX_AUDMUX
+	select SND_SOC_IMX_SSI
+	help
+	  Enable support for audio on the i.MX31ADS with the WM1133-EV1
+	  PMIC board with WM8835x fitted.
+
+config SND_SOC_MX27VIS_AIC32X4
+	tristate "SoC audio support for Visstrim M10 boards"
+	depends on MACH_IMX27_VISSTRIM_M10 && I2C
+	select SND_SOC_TLV320AIC32X4
+	select SND_MXC_SOC_MX2
+	select SND_SOC_IMX_AUDMUX
+	select SND_SOC_IMX_SSI
+	help
+	  Say Y if you want to add support for SoC audio on Visstrim SM10
+	  board with TLV320AIC32X4 codec.
+
+config SND_SOC_PHYCORE_AC97
+	tristate "SoC Audio support for Phytec phyCORE (and phyCARD) boards"
+	depends on MACH_PCM043 || MACH_PCA100
+	select SND_SOC_AC97_BUS
+	select SND_SOC_WM9712
+	select SND_MXC_SOC_FIQ
+	select SND_SOC_IMX_AUDMUX
+	select SND_SOC_IMX_SSI
+	help
+	  Say Y if you want to add support for SoC audio on Phytec phyCORE
+	  and phyCARD boards in AC97 mode
+
+config SND_SOC_EUKREA_TLV320
+	tristate "Eukrea TLV320"
+	depends on MACH_EUKREA_MBIMX27_BASEBOARD \
+		|| MACH_EUKREA_MBIMXSD25_BASEBOARD \
+		|| MACH_EUKREA_MBIMXSD35_BASEBOARD \
+		|| MACH_EUKREA_MBIMXSD51_BASEBOARD
+	depends on I2C
+	select SND_SOC_TLV320AIC23
+	select SND_MXC_SOC_FIQ
+	select SND_SOC_IMX_AUDMUX
+	select SND_SOC_IMX_SSI
+	help
+	  Enable I2S based access to the TLV320AIC23B codec attached
+	  to the SSI interface
+
+endif # SND_IMX_SOC
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
index 95d483f..36c257f 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -21,3 +21,25 @@ obj-$(CONFIG_SND_SOC_MPC5200_AC97) += mpc5200_psc_ac97.o
 obj-$(CONFIG_SND_MPC52xx_SOC_PCM030) += pcm030-audio-fabric.o
 obj-$(CONFIG_SND_MPC52xx_SOC_EFIKA) += efika-audio-fabric.o
 
+# i.MX Platform Support
+snd-soc-imx-ssi-objs := imx-ssi.o
+snd-soc-imx-audmux-objs := imx-audmux.o
+
+obj-$(CONFIG_SND_SOC_IMX_SSI) += snd-soc-imx-ssi.o
+obj-$(CONFIG_SND_SOC_IMX_AUDMUX) += snd-soc-imx-audmux.o
+
+obj-$(CONFIG_SND_SOC_IMX_PCM) += snd-soc-imx-pcm.o
+snd-soc-imx-pcm-y := imx-pcm.o
+snd-soc-imx-pcm-$(CONFIG_SND_MXC_SOC_FIQ) += imx-pcm-fiq.o
+snd-soc-imx-pcm-$(CONFIG_SND_MXC_SOC_MX2) += imx-pcm-dma-mx2.o
+
+# i.MX Machine Support
+snd-soc-eukrea-tlv320-objs := eukrea-tlv320.o
+snd-soc-phycore-ac97-objs := phycore-ac97.o
+snd-soc-mx27vis-aic32x4-objs := mx27vis-aic32x4.o
+snd-soc-wm1133-ev1-objs := wm1133-ev1.o
+
+obj-$(CONFIG_SND_SOC_EUKREA_TLV320) += snd-soc-eukrea-tlv320.o
+obj-$(CONFIG_SND_SOC_PHYCORE_AC97) += snd-soc-phycore-ac97.o
+obj-$(CONFIG_SND_SOC_MX27VIS_AIC32X4) += snd-soc-mx27vis-aic32x4.o
+obj-$(CONFIG_SND_MXC_SOC_WM1133_EV1) += snd-soc-wm1133-ev1.o
diff --git a/sound/soc/imx/eukrea-tlv320.c b/sound/soc/fsl/eukrea-tlv320.c
similarity index 99%
rename from sound/soc/imx/eukrea-tlv320.c
rename to sound/soc/fsl/eukrea-tlv320.c
index 7d4475c..efb9ede 100644
--- a/sound/soc/imx/eukrea-tlv320.c
+++ b/sound/soc/fsl/eukrea-tlv320.c
@@ -7,7 +7,7 @@
  * which is Copyright 2009 Simtec Electronics
  * and on sound/soc/imx/phycore-ac97.c which is
  * Copyright 2009 Sascha Hauer, Pengutronix <s.hauer@pengutronix.de>
- * 
+ *
  *  This program is free software; you can redistribute  it and/or modify it
  *  under  the terms of  the GNU General  Public License as published by the
  *  Free Software Foundation;  either version 2 of the  License, or (at your
diff --git a/sound/soc/imx/imx-audmux.c b/sound/soc/fsl/imx-audmux.c
similarity index 100%
rename from sound/soc/imx/imx-audmux.c
rename to sound/soc/fsl/imx-audmux.c
diff --git a/sound/soc/imx/imx-audmux.h b/sound/soc/fsl/imx-audmux.h
similarity index 100%
rename from sound/soc/imx/imx-audmux.h
rename to sound/soc/fsl/imx-audmux.h
diff --git a/sound/soc/imx/imx-pcm-dma-mx2.c b/sound/soc/fsl/imx-pcm-dma-mx2.c
similarity index 100%
rename from sound/soc/imx/imx-pcm-dma-mx2.c
rename to sound/soc/fsl/imx-pcm-dma-mx2.c
diff --git a/sound/soc/imx/imx-pcm-fiq.c b/sound/soc/fsl/imx-pcm-fiq.c
similarity index 100%
rename from sound/soc/imx/imx-pcm-fiq.c
rename to sound/soc/fsl/imx-pcm-fiq.c
diff --git a/sound/soc/imx/imx-pcm.c b/sound/soc/fsl/imx-pcm.c
similarity index 100%
rename from sound/soc/imx/imx-pcm.c
rename to sound/soc/fsl/imx-pcm.c
diff --git a/sound/soc/imx/imx-pcm.h b/sound/soc/fsl/imx-pcm.h
similarity index 100%
rename from sound/soc/imx/imx-pcm.h
rename to sound/soc/fsl/imx-pcm.h
diff --git a/sound/soc/imx/imx-ssi.c b/sound/soc/fsl/imx-ssi.c
similarity index 99%
rename from sound/soc/imx/imx-ssi.c
rename to sound/soc/fsl/imx-ssi.c
index 9203cdd..3e0dac5a 100644
--- a/sound/soc/imx/imx-ssi.c
+++ b/sound/soc/fsl/imx-ssi.c
@@ -28,7 +28,7 @@
  * value. When we read the same register two times (and the register still
  * contains the same value) these status bits are not set. We work
  * around this by not polling these bits but only wait a fixed delay.
- * 
+ *
  */
 
 #include <linux/clk.h>
diff --git a/sound/soc/imx/imx-ssi.h b/sound/soc/fsl/imx-ssi.h
similarity index 100%
rename from sound/soc/imx/imx-ssi.h
rename to sound/soc/fsl/imx-ssi.h
diff --git a/sound/soc/imx/mx27vis-aic32x4.c b/sound/soc/fsl/mx27vis-aic32x4.c
similarity index 100%
rename from sound/soc/imx/mx27vis-aic32x4.c
rename to sound/soc/fsl/mx27vis-aic32x4.c
diff --git a/sound/soc/imx/phycore-ac97.c b/sound/soc/fsl/phycore-ac97.c
similarity index 100%
rename from sound/soc/imx/phycore-ac97.c
rename to sound/soc/fsl/phycore-ac97.c
diff --git a/sound/soc/imx/wm1133-ev1.c b/sound/soc/fsl/wm1133-ev1.c
similarity index 100%
rename from sound/soc/imx/wm1133-ev1.c
rename to sound/soc/fsl/wm1133-ev1.c
diff --git a/sound/soc/imx/Kconfig b/sound/soc/imx/Kconfig
deleted file mode 100644
index 810acaa..0000000
--- a/sound/soc/imx/Kconfig
+++ /dev/null
@@ -1,79 +0,0 @@
-menuconfig SND_IMX_SOC
-	tristate "SoC Audio for Freescale i.MX CPUs"
-	depends on ARCH_MXC
-	help
-	  Say Y or M if you want to add support for codecs attached to
-	  the i.MX SSI interface.
-
-
-if SND_IMX_SOC
-
-config SND_SOC_IMX_SSI
-	tristate
-
-config SND_SOC_IMX_PCM
-	tristate
-
-config SND_MXC_SOC_FIQ
-	tristate
-	select FIQ
-	select SND_SOC_IMX_PCM
-
-config SND_MXC_SOC_MX2
-	select SND_SOC_DMAENGINE_PCM
-	tristate
-	select SND_SOC_IMX_PCM
-
-config SND_SOC_IMX_AUDMUX
-	tristate
-
-config SND_MXC_SOC_WM1133_EV1
-	tristate "Audio on the the i.MX31ADS with WM1133-EV1 fitted"
-	depends on MACH_MX31ADS_WM1133_EV1 && EXPERIMENTAL
-	select SND_SOC_WM8350
-	select SND_MXC_SOC_FIQ
-	select SND_SOC_IMX_AUDMUX
-	select SND_SOC_IMX_SSI
-	help
-	  Enable support for audio on the i.MX31ADS with the WM1133-EV1
-	  PMIC board with WM8835x fitted.
-
-config SND_SOC_MX27VIS_AIC32X4
-	tristate "SoC audio support for Visstrim M10 boards"
-	depends on MACH_IMX27_VISSTRIM_M10 && I2C
-	select SND_SOC_TLV320AIC32X4
-	select SND_MXC_SOC_MX2
-	select SND_SOC_IMX_AUDMUX
-	select SND_SOC_IMX_SSI
-	help
-	  Say Y if you want to add support for SoC audio on Visstrim SM10
-	  board with TLV320AIC32X4 codec.
-
-config SND_SOC_PHYCORE_AC97
-	tristate "SoC Audio support for Phytec phyCORE (and phyCARD) boards"
-	depends on MACH_PCM043 || MACH_PCA100
-	select SND_SOC_AC97_BUS
-	select SND_SOC_WM9712
-	select SND_MXC_SOC_FIQ
-	select SND_SOC_IMX_AUDMUX
-	select SND_SOC_IMX_SSI
-	help
-	  Say Y if you want to add support for SoC audio on Phytec phyCORE
-	  and phyCARD boards in AC97 mode
-
-config SND_SOC_EUKREA_TLV320
-	tristate "Eukrea TLV320"
-	depends on MACH_EUKREA_MBIMX27_BASEBOARD \
-		|| MACH_EUKREA_MBIMXSD25_BASEBOARD \
-		|| MACH_EUKREA_MBIMXSD35_BASEBOARD \
-		|| MACH_EUKREA_MBIMXSD51_BASEBOARD
-	depends on I2C
-	select SND_SOC_TLV320AIC23
-	select SND_MXC_SOC_FIQ
-	select SND_SOC_IMX_AUDMUX
-	select SND_SOC_IMX_SSI
-	help
-	  Enable I2S based access to the TLV320AIC23B codec attached
-	  to the SSI interface
-
-endif	# SND_IMX_SOC
diff --git a/sound/soc/imx/Makefile b/sound/soc/imx/Makefile
deleted file mode 100644
index f5db3e9..0000000
--- a/sound/soc/imx/Makefile
+++ /dev/null
@@ -1,22 +0,0 @@
-# i.MX Platform Support
-snd-soc-imx-ssi-objs := imx-ssi.o
-snd-soc-imx-audmux-objs := imx-audmux.o
-
-obj-$(CONFIG_SND_SOC_IMX_SSI) += snd-soc-imx-ssi.o
-obj-$(CONFIG_SND_SOC_IMX_AUDMUX) += snd-soc-imx-audmux.o
-
-obj-$(CONFIG_SND_SOC_IMX_PCM) += snd-soc-imx-pcm.o
-snd-soc-imx-pcm-y := imx-pcm.o
-snd-soc-imx-pcm-$(CONFIG_SND_MXC_SOC_FIQ) += imx-pcm-fiq.o
-snd-soc-imx-pcm-$(CONFIG_SND_MXC_SOC_MX2) += imx-pcm-dma-mx2.o
-
-# i.MX Machine Support
-snd-soc-eukrea-tlv320-objs := eukrea-tlv320.o
-snd-soc-phycore-ac97-objs := phycore-ac97.o
-snd-soc-mx27vis-aic32x4-objs := mx27vis-aic32x4.o
-snd-soc-wm1133-ev1-objs := wm1133-ev1.o
-
-obj-$(CONFIG_SND_SOC_EUKREA_TLV320) += snd-soc-eukrea-tlv320.o
-obj-$(CONFIG_SND_SOC_PHYCORE_AC97) += snd-soc-phycore-ac97.o
-obj-$(CONFIG_SND_SOC_MX27VIS_AIC32X4) += snd-soc-mx27vis-aic32x4.o
-obj-$(CONFIG_SND_MXC_SOC_WM1133_EV1) += snd-soc-wm1133-ev1.o
-- 
1.7.5.4

[PATCH v3 04/11] ASoC: fsl: rename imx-pcm Kconfig options and filename

[Shawn Guo]

Rename a couple of imx-pcm Kconfig options and filename to get them
well named and less confusing.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/fsl/Kconfig                              |   12 ++++++------
 sound/soc/fsl/Makefile                             |    4 ++--
 sound/soc/fsl/{imx-pcm-dma-mx2.c => imx-pcm-dma.c} |    0
 3 files changed, 8 insertions(+), 8 deletions(-)
 rename sound/soc/fsl/{imx-pcm-dma-mx2.c => imx-pcm-dma.c} (100%)

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index 19856a0..e3f7509 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -97,12 +97,12 @@ config SND_SOC_IMX_SSI
 config SND_SOC_IMX_PCM
 	tristate
 
-config SND_MXC_SOC_FIQ
+config SND_SOC_IMX_PCM_FIQ
 	tristate
 	select FIQ
 	select SND_SOC_IMX_PCM
 
-config SND_MXC_SOC_MX2
+config SND_SOC_IMX_PCM_DMA
 	tristate
 	select SND_SOC_DMAENGINE_PCM
 	select SND_SOC_IMX_PCM
@@ -114,7 +114,7 @@ config SND_MXC_SOC_WM1133_EV1
 	tristate "Audio on the the i.MX31ADS with WM1133-EV1 fitted"
 	depends on MACH_MX31ADS_WM1133_EV1 && EXPERIMENTAL
 	select SND_SOC_WM8350
-	select SND_MXC_SOC_FIQ
+	select SND_SOC_IMX_PCM_FIQ
 	select SND_SOC_IMX_AUDMUX
 	select SND_SOC_IMX_SSI
 	help
@@ -125,7 +125,7 @@ config SND_SOC_MX27VIS_AIC32X4
 	tristate "SoC audio support for Visstrim M10 boards"
 	depends on MACH_IMX27_VISSTRIM_M10 && I2C
 	select SND_SOC_TLV320AIC32X4
-	select SND_MXC_SOC_MX2
+	select SND_SOC_IMX_PCM_DMA
 	select SND_SOC_IMX_AUDMUX
 	select SND_SOC_IMX_SSI
 	help
@@ -137,7 +137,7 @@ config SND_SOC_PHYCORE_AC97
 	depends on MACH_PCM043 || MACH_PCA100
 	select SND_SOC_AC97_BUS
 	select SND_SOC_WM9712
-	select SND_MXC_SOC_FIQ
+	select SND_SOC_IMX_PCM_FIQ
 	select SND_SOC_IMX_AUDMUX
 	select SND_SOC_IMX_SSI
 	help
@@ -152,7 +152,7 @@ config SND_SOC_EUKREA_TLV320
 		|| MACH_EUKREA_MBIMXSD51_BASEBOARD
 	depends on I2C
 	select SND_SOC_TLV320AIC23
-	select SND_MXC_SOC_FIQ
+	select SND_SOC_IMX_PCM_FIQ
 	select SND_SOC_IMX_AUDMUX
 	select SND_SOC_IMX_SSI
 	help
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
index 36c257f..f031409 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -30,8 +30,8 @@ obj-$(CONFIG_SND_SOC_IMX_AUDMUX) += snd-soc-imx-audmux.o
 
 obj-$(CONFIG_SND_SOC_IMX_PCM) += snd-soc-imx-pcm.o
 snd-soc-imx-pcm-y := imx-pcm.o
-snd-soc-imx-pcm-$(CONFIG_SND_MXC_SOC_FIQ) += imx-pcm-fiq.o
-snd-soc-imx-pcm-$(CONFIG_SND_MXC_SOC_MX2) += imx-pcm-dma-mx2.o
+snd-soc-imx-pcm-$(CONFIG_SND_SOC_IMX_PCM_FIQ) += imx-pcm-fiq.o
+snd-soc-imx-pcm-$(CONFIG_SND_SOC_IMX_PCM_DMA) += imx-pcm-dma.o
 
 # i.MX Machine Support
 snd-soc-eukrea-tlv320-objs := eukrea-tlv320.o
diff --git a/sound/soc/fsl/imx-pcm-dma-mx2.c b/sound/soc/fsl/imx-pcm-dma.c
similarity index 100%
rename from sound/soc/fsl/imx-pcm-dma-mx2.c
rename to sound/soc/fsl/imx-pcm-dma.c
-- 
1.7.5.4

[PATCH v3 05/11] ASoC: fsl: create fsl_utils to accommodate the common functions

[Shawn Guo]

There is some amount of code duplication between mpc8610_hpcd and
p1022_ds machine drivers, and the same code will be duplicated again
when another new machine driver is added.  The patch creates fsl_utils
to accommodate the common functions to stop the code duplication.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/fsl/Kconfig        |    5 ++
 sound/soc/fsl/Makefile       |    2 +
 sound/soc/fsl/fsl_utils.c    |  135 ++++++++++++++++++++++++++++++++++++
 sound/soc/fsl/fsl_utils.h    |   27 +++++++
 sound/soc/fsl/mpc8610_hpcd.c |  157 +++---------------------------------------
 sound/soc/fsl/p1022_ds.c     |  149 +++-------------------------------------
 6 files changed, 189 insertions(+), 286 deletions(-)
 create mode 100644 sound/soc/fsl/fsl_utils.c
 create mode 100644 sound/soc/fsl/fsl_utils.h

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index e3f7509..535ee73 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -1,6 +1,9 @@
 config SND_SOC_FSL_SSI
 	tristate
 
+config SND_SOC_FSL_UTILS
+	tristate
+
 menuconfig SND_POWERPC_SOC
 	tristate "SoC Audio for Freescale PowerPC CPUs"
 	depends on FSL_SOC
@@ -26,6 +29,7 @@ config SND_SOC_MPC8610_HPCD
 	# I2C is necessary for the CS4270 driver
 	depends on MPC8610_HPCD && I2C
 	select SND_SOC_FSL_SSI
+	select SND_SOC_FSL_UTILS
 	select SND_SOC_POWERPC_DMA
 	select SND_SOC_CS4270
 	select SND_SOC_CS4270_VD33_ERRATA
@@ -38,6 +42,7 @@ config SND_SOC_P1022_DS
 	# I2C is necessary for the WM8776 driver
 	depends on P1022_DS && I2C
 	select SND_SOC_FSL_SSI
+	select SND_SOC_FSL_UTILS
 	select SND_SOC_POWERPC_DMA
 	select SND_SOC_WM8776
 	default y if P1022_DS
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
index f031409..fbdbed0 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -8,8 +8,10 @@ obj-$(CONFIG_SND_SOC_P1022_DS) += snd-soc-p1022-ds.o
 
 # Freescale PowerPC SSI/DMA Platform Support
 snd-soc-fsl-ssi-objs := fsl_ssi.o
+snd-soc-fsl-utils-objs := fsl_utils.o
 snd-soc-fsl-dma-objs := fsl_dma.o
 obj-$(CONFIG_SND_SOC_FSL_SSI) += snd-soc-fsl-ssi.o
+obj-$(CONFIG_SND_SOC_FSL_UTILS) += snd-soc-fsl-utils.o
 obj-$(CONFIG_SND_SOC_POWERPC_DMA) += snd-soc-fsl-dma.o
 
 # MPC5200 Platform Support
diff --git a/sound/soc/fsl/fsl_utils.c b/sound/soc/fsl/fsl_utils.c
new file mode 100644
index 0000000..4370c28
--- /dev/null
+++ b/sound/soc/fsl/fsl_utils.c
@@ -0,0 +1,135 @@
+/**
+ * Freescale ALSA SoC Machine driver utility
+ *
+ * Author: Timur Tabi <timur@freescale.com>
+ *
+ * Copyright 2010 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/of_address.h>
+#include <linux/of_i2c.h>
+#include <sound/soc.h>
+
+#include "fsl_utils.h"
+
+/**
+ * fsl_asoc_get_codec_dev_name - determine the dev_name for a codec node
+ *
+ * @np: pointer to the I2C device tree node
+ * @buf: buffer to be filled with the dev_name of the I2C device
+ * @len: the length of the buffer
+ *
+ * This function determines the dev_name for an I2C node.  This is the name
+ * that would be returned by dev_name() if this device_node were part of a
+ * 'struct device'  It's ugly and hackish, but it works.
+ *
+ * The dev_name for such devices include the bus number and I2C address. For
+ * example, "cs4270.0-004f".
+ */
+int fsl_asoc_get_codec_dev_name(struct device_node *np, char *buf, size_t len)
+{
+	const u32 *iprop;
+	u32 addr;
+	char temp[DAI_NAME_SIZE];
+	struct i2c_client *i2c;
+
+	of_modalias_node(np, temp, DAI_NAME_SIZE);
+
+	iprop = of_get_property(np, "reg", NULL);
+	if (!iprop)
+		return -EINVAL;
+
+	addr = be32_to_cpup(iprop);
+
+	/* We need the adapter number */
+	i2c = of_find_i2c_device_by_node(np);
+	if (!i2c) {
+		put_device(&i2c->dev);
+		return -ENODEV;
+	}
+
+	snprintf(buf, len, "%s.%u-%04x", temp, i2c->adapter->nr, addr);
+	put_device(&i2c->dev);
+
+	return 0;
+}
+EXPORT_SYMBOL(fsl_asoc_get_codec_dev_name);
+
+/**
+ * fsl_asoc_get_dma_channel - determine the dma channel for a SSI node
+ *
+ * @ssi_np: pointer to the SSI device tree node
+ * @name: name of the phandle pointing to the dma channel
+ * @dai: ASoC DAI link pointer to be filled with platform_name
+ * @dma_channel_id: dma channel id to be returned
+ * @dma_id: dma id to be returned
+ *
+ * This function determines the dma and channel id for given SSI node.  It
+ * also discovers the platform_name for the ASoC DAI link.
+ */
+int fsl_asoc_get_dma_channel(struct device_node *ssi_np,
+			     const char *name,
+			     struct snd_soc_dai_link *dai,
+			     unsigned int *dma_channel_id,
+			     unsigned int *dma_id)
+{
+	struct resource res;
+	struct device_node *dma_channel_np, *dma_np;
+	const u32 *iprop;
+	int ret;
+
+	dma_channel_np = of_parse_phandle(ssi_np, name, 0);
+	if (!dma_channel_np)
+		return -EINVAL;
+
+	if (!of_device_is_compatible(dma_channel_np, "fsl,ssi-dma-channel")) {
+		of_node_put(dma_channel_np);
+		return -EINVAL;
+	}
+
+	/* Determine the dev_name for the device_node.  This code mimics the
+	 * behavior of of_device_make_bus_id(). We need this because ASoC uses
+	 * the dev_name() of the device to match the platform (DMA) device with
+	 * the CPU (SSI) device.  It's all ugly and hackish, but it works (for
+	 * now).
+	 *
+	 * dai->platform name should already point to an allocated buffer.
+	 */
+	ret = of_address_to_resource(dma_channel_np, 0, &res);
+	if (ret) {
+		of_node_put(dma_channel_np);
+		return ret;
+	}
+	snprintf((char *)dai->platform_name, DAI_NAME_SIZE, "%llx.%s",
+		 (unsigned long long) res.start, dma_channel_np->name);
+
+	iprop = of_get_property(dma_channel_np, "cell-index", NULL);
+	if (!iprop) {
+		of_node_put(dma_channel_np);
+		return -EINVAL;
+	}
+	*dma_channel_id = be32_to_cpup(iprop);
+
+	dma_np = of_get_parent(dma_channel_np);
+	iprop = of_get_property(dma_np, "cell-index", NULL);
+	if (!iprop) {
+		of_node_put(dma_np);
+		return -EINVAL;
+	}
+	*dma_id = be32_to_cpup(iprop);
+
+	of_node_put(dma_np);
+	of_node_put(dma_channel_np);
+
+	return 0;
+}
+EXPORT_SYMBOL(fsl_asoc_get_dma_channel);
+
+MODULE_AUTHOR("Timur Tabi <timur@freescale.com>");
+MODULE_DESCRIPTION("Freescale ASoC utility code");
+MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/fsl/fsl_utils.h b/sound/soc/fsl/fsl_utils.h
new file mode 100644
index 0000000..44d1436
--- /dev/null
+++ b/sound/soc/fsl/fsl_utils.h
@@ -0,0 +1,27 @@
+/**
+ * Freescale ALSA SoC Machine driver utility
+ *
+ * Author: Timur Tabi <timur@freescale.com>
+ *
+ * Copyright 2010 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 _FSL_UTILS_H
+#define _FSL_UTILS_H
+
+#define DAI_NAME_SIZE	32
+
+struct snd_soc_dai_link;
+struct device_node;
+
+int fsl_asoc_get_codec_dev_name(struct device_node *np, char *buf, size_t len);
+int fsl_asoc_get_dma_channel(struct device_node *ssi_np, const char *name,
+			     struct snd_soc_dai_link *dai,
+			     unsigned int *dma_channel_id,
+			     unsigned int *dma_id);
+
+#endif /* _FSL_UTILS_H */
diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c
index fcf9302..7ead1e5 100644
--- a/sound/soc/fsl/mpc8610_hpcd.c
+++ b/sound/soc/fsl/mpc8610_hpcd.c
@@ -14,18 +14,16 @@
 #include <linux/interrupt.h>
 #include <linux/of_device.h>
 #include <linux/slab.h>
-#include <linux/of_i2c.h>
 #include <sound/soc.h>
 #include <asm/fsl_guts.h>
 
 #include "fsl_dma.h"
 #include "fsl_ssi.h"
+#include "fsl_utils.h"
 
 /* There's only one global utilities register */
 static phys_addr_t guts_phys;
 
-#define DAI_NAME_SIZE	32
-
 /**
  * mpc8610_hpcd_data: machine-specific ASoC device data
  *
@@ -181,141 +179,6 @@ static struct snd_soc_ops mpc8610_hpcd_ops = {
 };
 
 /**
- * get_node_by_phandle_name - get a node by its phandle name
- *
- * This function takes a node, the name of a property in that node, and a
- * compatible string.  Assuming the property is a phandle to another node,
- * it returns that node, (optionally) if that node is compatible.
- *
- * If the property is not a phandle, or the node it points to is not compatible
- * with the specific string, then NULL is returned.
- */
-static struct device_node *get_node_by_phandle_name(struct device_node *np,
-					       const char *name,
-					       const char *compatible)
-{
-	const phandle *ph;
-	int len;
-
-	ph = of_get_property(np, name, &len);
-	if (!ph || (len != sizeof(phandle)))
-		return NULL;
-
-	np = of_find_node_by_phandle(*ph);
-	if (!np)
-		return NULL;
-
-	if (compatible && !of_device_is_compatible(np, compatible)) {
-		of_node_put(np);
-		return NULL;
-	}
-
-	return np;
-}
-
-/**
- * get_parent_cell_index -- return the cell-index of the parent of a node
- *
- * Return the value of the cell-index property of the parent of the given
- * node.  This is used for DMA channel nodes that need to know the DMA ID
- * of the controller they are on.
- */
-static int get_parent_cell_index(struct device_node *np)
-{
-	struct device_node *parent = of_get_parent(np);
-	const u32 *iprop;
-
-	if (!parent)
-		return -1;
-
-	iprop = of_get_property(parent, "cell-index", NULL);
-	of_node_put(parent);
-
-	if (!iprop)
-		return -1;
-
-	return be32_to_cpup(iprop);
-}
-
-/**
- * codec_node_dev_name - determine the dev_name for a codec node
- *
- * This function determines the dev_name for an I2C node.  This is the name
- * that would be returned by dev_name() if this device_node were part of a
- * 'struct device'  It's ugly and hackish, but it works.
- *
- * The dev_name for such devices include the bus number and I2C address. For
- * example, "cs4270.0-004f".
- */
-static int codec_node_dev_name(struct device_node *np, char *buf, size_t len)
-{
-	const u32 *iprop;
-	int addr;
-	char temp[DAI_NAME_SIZE];
-	struct i2c_client *i2c;
-
-	of_modalias_node(np, temp, DAI_NAME_SIZE);
-
-	iprop = of_get_property(np, "reg", NULL);
-	if (!iprop)
-		return -EINVAL;
-
-	addr = be32_to_cpup(iprop);
-
-	/* We need the adapter number */
-	i2c = of_find_i2c_device_by_node(np);
-	if (!i2c)
-		return -ENODEV;
-
-	snprintf(buf, len, "%s.%u-%04x", temp, i2c->adapter->nr, addr);
-
-	return 0;
-}
-
-static int get_dma_channel(struct device_node *ssi_np,
-			   const char *name,
-			   struct snd_soc_dai_link *dai,
-			   unsigned int *dma_channel_id,
-			   unsigned int *dma_id)
-{
-	struct resource res;
-	struct device_node *dma_channel_np;
-	const u32 *iprop;
-	int ret;
-
-	dma_channel_np = get_node_by_phandle_name(ssi_np, name,
-						  "fsl,ssi-dma-channel");
-	if (!dma_channel_np)
-		return -EINVAL;
-
-	/* Determine the dev_name for the device_node.  This code mimics the
-	 * behavior of of_device_make_bus_id(). We need this because ASoC uses
-	 * the dev_name() of the device to match the platform (DMA) device with
-	 * the CPU (SSI) device.  It's all ugly and hackish, but it works (for
-	 * now).
-	 *
-	 * dai->platform name should already point to an allocated buffer.
-	 */
-	ret = of_address_to_resource(dma_channel_np, 0, &res);
-	if (ret)
-		return ret;
-	snprintf((char *)dai->platform_name, DAI_NAME_SIZE, "%llx.%s",
-		 (unsigned long long) res.start, dma_channel_np->name);
-
-	iprop = of_get_property(dma_channel_np, "cell-index", NULL);
-	if (!iprop) {
-		of_node_put(dma_channel_np);
-		return -EINVAL;
-	}
-
-	*dma_channel_id = be32_to_cpup(iprop);
-	*dma_id = get_parent_cell_index(dma_channel_np);
-	of_node_put(dma_channel_np);
-
-	return 0;
-}
-
-/**
  * mpc8610_hpcd_probe: platform probe function for the machine driver
  *
  * Although this is a machine driver, the SSI node is the "master" node with
@@ -353,8 +216,8 @@ static int mpc8610_hpcd_probe(struct platform_device *pdev)
 	machine_data->dai[0].ops = &mpc8610_hpcd_ops;
 
 	/* Determine the codec name, it will be used as the codec DAI name */
-	ret = codec_node_dev_name(codec_np, machine_data->codec_name,
-				  DAI_NAME_SIZE);
+	ret = fsl_asoc_get_codec_dev_name(codec_np, machine_data->codec_name,
+					  DAI_NAME_SIZE);
 	if (ret) {
 		dev_err(&pdev->dev, "invalid codec node %s\n",
 			codec_np->full_name);
@@ -458,9 +321,10 @@ static int mpc8610_hpcd_probe(struct platform_device *pdev)
 
 	/* Find the playback DMA channel to use. */
 	machine_data->dai[0].platform_name = machine_data->platform_name[0];
-	ret = get_dma_channel(np, "fsl,playback-dma", &machine_data->dai[0],
-			      &machine_data->dma_channel_id[0],
-			      &machine_data->dma_id[0]);
+	ret = fsl_asoc_get_dma_channel(np, "fsl,playback-dma",
+				       &machine_data->dai[0],
+				       &machine_data->dma_channel_id[0],
+				       &machine_data->dma_id[0]);
 	if (ret) {
 		dev_err(&pdev->dev, "missing/invalid playback DMA phandle\n");
 		goto error;
@@ -468,9 +332,10 @@ static int mpc8610_hpcd_probe(struct platform_device *pdev)
 
 	/* Find the capture DMA channel to use. */
 	machine_data->dai[1].platform_name = machine_data->platform_name[1];
-	ret = get_dma_channel(np, "fsl,capture-dma", &machine_data->dai[1],
-			      &machine_data->dma_channel_id[1],
-			      &machine_data->dma_id[1]);
+	ret = fsl_asoc_get_dma_channel(np, "fsl,capture-dma",
+				       &machine_data->dai[1],
+				       &machine_data->dma_channel_id[1],
+				       &machine_data->dma_id[1]);
 	if (ret) {
 		dev_err(&pdev->dev, "missing/invalid capture DMA phandle\n");
 		goto error;
diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c
index d32ec46..60f7bb8 100644
--- a/sound/soc/fsl/p1022_ds.c
+++ b/sound/soc/fsl/p1022_ds.c
@@ -14,12 +14,12 @@
 #include <linux/interrupt.h>
 #include <linux/of_device.h>
 #include <linux/slab.h>
-#include <linux/of_i2c.h>
 #include <sound/soc.h>
 #include <asm/fsl_guts.h>
 
 #include "fsl_dma.h"
 #include "fsl_ssi.h"
+#include "fsl_utils.h"
 
 /* P1022-specific PMUXCR and DMUXCR bit definitions */
 
@@ -57,8 +57,6 @@ static inline void guts_set_dmuxcr(struct ccsr_guts_85xx __iomem *guts,
 /* There's only one global utilities register */
 static phys_addr_t guts_phys;
 
-#define DAI_NAME_SIZE	32
-
 /**
  * machine_data: machine-specific ASoC device data
  *
@@ -191,136 +189,6 @@ static struct snd_soc_ops p1022_ds_ops = {
 };
 
 /**
- * get_node_by_phandle_name - get a node by its phandle name
- *
- * This function takes a node, the name of a property in that node, and a
- * compatible string.  Assuming the property is a phandle to another node,
- * it returns that node, (optionally) if that node is compatible.
- *
- * If the property is not a phandle, or the node it points to is not compatible
- * with the specific string, then NULL is returned.
- */
-static struct device_node *get_node_by_phandle_name(struct device_node *np,
-	const char *name, const char *compatible)
-{
-	np = of_parse_phandle(np, name, 0);
-	if (!np)
-		return NULL;
-
-	if (!of_device_is_compatible(np, compatible)) {
-		of_node_put(np);
-		return NULL;
-	}
-
-	return np;
-}
-
-/**
- * get_parent_cell_index -- return the cell-index of the parent of a node
- *
- * Return the value of the cell-index property of the parent of the given
- * node.  This is used for DMA channel nodes that need to know the DMA ID
- * of the controller they are on.
- */
-static int get_parent_cell_index(struct device_node *np)
-{
-	struct device_node *parent = of_get_parent(np);
-	const u32 *iprop;
-	int ret = -1;
-
-	if (!parent)
-		return -1;
-
-	iprop = of_get_property(parent, "cell-index", NULL);
-	if (iprop)
-		ret = be32_to_cpup(iprop);
-
-	of_node_put(parent);
-
-	return ret;
-}
-
-/**
- * codec_node_dev_name - determine the dev_name for a codec node
- *
- * This function determines the dev_name for an I2C node.  This is the name
- * that would be returned by dev_name() if this device_node were part of a
- * 'struct device'  It's ugly and hackish, but it works.
- *
- * The dev_name for such devices include the bus number and I2C address. For
- * example, "cs4270-codec.0-004f".
- */
-static int codec_node_dev_name(struct device_node *np, char *buf, size_t len)
-{
-	const u32 *iprop;
-	int addr;
-	char temp[DAI_NAME_SIZE];
-	struct i2c_client *i2c;
-
-	of_modalias_node(np, temp, DAI_NAME_SIZE);
-
-	iprop = of_get_property(np, "reg", NULL);
-	if (!iprop)
-		return -EINVAL;
-
-	addr = be32_to_cpup(iprop);
-
-	/* We need the adapter number */
-	i2c = of_find_i2c_device_by_node(np);
-	if (!i2c)
-		return -ENODEV;
-
-	snprintf(buf, len, "%s.%u-%04x", temp, i2c->adapter->nr, addr);
-
-	return 0;
-}
-
-static int get_dma_channel(struct device_node *ssi_np,
-			   const char *name,
-			   struct snd_soc_dai_link *dai,
-			   unsigned int *dma_channel_id,
-			   unsigned int *dma_id)
-{
-	struct resource res;
-	struct device_node *dma_channel_np;
-	const u32 *iprop;
-	int ret;
-
-	dma_channel_np = get_node_by_phandle_name(ssi_np, name,
-						  "fsl,ssi-dma-channel");
-	if (!dma_channel_np)
-		return -EINVAL;
-
-	/* Determine the dev_name for the device_node.  This code mimics the
-	 * behavior of of_device_make_bus_id(). We need this because ASoC uses
-	 * the dev_name() of the device to match the platform (DMA) device with
-	 * the CPU (SSI) device.  It's all ugly and hackish, but it works (for
-	 * now).
-	 *
-	 * dai->platform name should already point to an allocated buffer.
-	 */
-	ret = of_address_to_resource(dma_channel_np, 0, &res);
-	if (ret) {
-		of_node_put(dma_channel_np);
-		return ret;
-	}
-	snprintf((char *)dai->platform_name, DAI_NAME_SIZE, "%llx.%s",
-		 (unsigned long long) res.start, dma_channel_np->name);
-
-	iprop = of_get_property(dma_channel_np, "cell-index", NULL);
-	if (!iprop) {
-		of_node_put(dma_channel_np);
-		return -EINVAL;
-	}
-
-	*dma_channel_id = be32_to_cpup(iprop);
-	*dma_id = get_parent_cell_index(dma_channel_np);
-	of_node_put(dma_channel_np);
-
-	return 0;
-}
-
-/**
  * p1022_ds_probe: platform probe function for the machine driver
  *
  * Although this is a machine driver, the SSI node is the "master" node with
@@ -358,7 +226,8 @@ static int p1022_ds_probe(struct platform_device *pdev)
 	mdata->dai[0].ops = &p1022_ds_ops;
 
 	/* Determine the codec name, it will be used as the codec DAI name */
-	ret = codec_node_dev_name(codec_np, mdata->codec_name, DAI_NAME_SIZE);
+	ret = fsl_asoc_get_codec_dev_name(codec_np, mdata->codec_name,
+					  DAI_NAME_SIZE);
 	if (ret) {
 		dev_err(&pdev->dev, "invalid codec node %s\n",
 			codec_np->full_name);
@@ -454,9 +323,9 @@ static int p1022_ds_probe(struct platform_device *pdev)
 
 	/* Find the playback DMA channel to use. */
 	mdata->dai[0].platform_name = mdata->platform_name[0];
-	ret = get_dma_channel(np, "fsl,playback-dma", &mdata->dai[0],
-			      &mdata->dma_channel_id[0],
-			      &mdata->dma_id[0]);
+	ret = fsl_asoc_get_dma_channel(np, "fsl,playback-dma", &mdata->dai[0],
+				       &mdata->dma_channel_id[0],
+				       &mdata->dma_id[0]);
 	if (ret) {
 		dev_err(&pdev->dev, "missing/invalid playback DMA phandle\n");
 		goto error;
@@ -464,9 +333,9 @@ static int p1022_ds_probe(struct platform_device *pdev)
 
 	/* Find the capture DMA channel to use. */
 	mdata->dai[1].platform_name = mdata->platform_name[1];
-	ret = get_dma_channel(np, "fsl,capture-dma", &mdata->dai[1],
-			      &mdata->dma_channel_id[1],
-			      &mdata->dma_id[1]);
+	ret = fsl_asoc_get_dma_channel(np, "fsl,capture-dma", &mdata->dai[1],
+				       &mdata->dma_channel_id[1],
+				       &mdata->dma_id[1]);
 	if (ret) {
 		dev_err(&pdev->dev, "missing/invalid capture DMA phandle\n");
 		goto error;
-- 
1.7.5.4

[PATCH v3 06/11] ASoC: fsl: remove helper fsl_asoc_get_codec_dev_name

[Shawn Guo]

The ASoC core now can support matching codec with device node besides
name, so we can save helper function fsl_asoc_get_codec_dev_name.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/fsl/fsl_utils.c    |   44 ------------------------------------------
 sound/soc/fsl/fsl_utils.h    |    1 -
 sound/soc/fsl/mpc8610_hpcd.c |   13 +----------
 sound/soc/fsl/p1022_ds.c     |   13 +----------
 4 files changed, 4 insertions(+), 67 deletions(-)

diff --git a/sound/soc/fsl/fsl_utils.c b/sound/soc/fsl/fsl_utils.c
index 4370c28..b9e42b5 100644
--- a/sound/soc/fsl/fsl_utils.c
+++ b/sound/soc/fsl/fsl_utils.c
@@ -12,55 +12,11 @@
 
 #include <linux/module.h>
 #include <linux/of_address.h>
-#include <linux/of_i2c.h>
 #include <sound/soc.h>
 
 #include "fsl_utils.h"
 
 /**
- * fsl_asoc_get_codec_dev_name - determine the dev_name for a codec node
- *
- * @np: pointer to the I2C device tree node
- * @buf: buffer to be filled with the dev_name of the I2C device
- * @len: the length of the buffer
- *
- * This function determines the dev_name for an I2C node.  This is the name
- * that would be returned by dev_name() if this device_node were part of a
- * 'struct device'  It's ugly and hackish, but it works.
- *
- * The dev_name for such devices include the bus number and I2C address. For
- * example, "cs4270.0-004f".
- */
-int fsl_asoc_get_codec_dev_name(struct device_node *np, char *buf, size_t len)
-{
-	const u32 *iprop;
-	u32 addr;
-	char temp[DAI_NAME_SIZE];
-	struct i2c_client *i2c;
-
-	of_modalias_node(np, temp, DAI_NAME_SIZE);
-
-	iprop = of_get_property(np, "reg", NULL);
-	if (!iprop)
-		return -EINVAL;
-
-	addr = be32_to_cpup(iprop);
-
-	/* We need the adapter number */
-	i2c = of_find_i2c_device_by_node(np);
-	if (!i2c) {
-		put_device(&i2c->dev);
-		return -ENODEV;
-	}
-
-	snprintf(buf, len, "%s.%u-%04x", temp, i2c->adapter->nr, addr);
-	put_device(&i2c->dev);
-
-	return 0;
-}
-EXPORT_SYMBOL(fsl_asoc_get_codec_dev_name);
-
-/**
  * fsl_asoc_get_dma_channel - determine the dma channel for a SSI node
  *
  * @ssi_np: pointer to the SSI device tree node
diff --git a/sound/soc/fsl/fsl_utils.h b/sound/soc/fsl/fsl_utils.h
index 44d1436..b295112 100644
--- a/sound/soc/fsl/fsl_utils.h
+++ b/sound/soc/fsl/fsl_utils.h
@@ -18,7 +18,6 @@
 struct snd_soc_dai_link;
 struct device_node;
 
-int fsl_asoc_get_codec_dev_name(struct device_node *np, char *buf, size_t len);
 int fsl_asoc_get_dma_channel(struct device_node *ssi_np, const char *name,
 			     struct snd_soc_dai_link *dai,
 			     unsigned int *dma_channel_id,
diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c
index 7ead1e5..354b753 100644
--- a/sound/soc/fsl/mpc8610_hpcd.c
+++ b/sound/soc/fsl/mpc8610_hpcd.c
@@ -41,7 +41,6 @@ struct mpc8610_hpcd_data {
 	unsigned int dma_id[2];		/* 0 = DMA1, 1 = DMA2, etc */
 	unsigned int dma_channel_id[2]; /* 0 = ch 0, 1 = ch 1, etc*/
 	char codec_dai_name[DAI_NAME_SIZE];
-	char codec_name[DAI_NAME_SIZE];
 	char platform_name[2][DAI_NAME_SIZE]; /* One for each DMA channel */
 };
 
@@ -215,16 +214,8 @@ static int mpc8610_hpcd_probe(struct platform_device *pdev)
 	machine_data->dai[0].cpu_dai_name = dev_name(&ssi_pdev->dev);
 	machine_data->dai[0].ops = &mpc8610_hpcd_ops;
 
-	/* Determine the codec name, it will be used as the codec DAI name */
-	ret = fsl_asoc_get_codec_dev_name(codec_np, machine_data->codec_name,
-					  DAI_NAME_SIZE);
-	if (ret) {
-		dev_err(&pdev->dev, "invalid codec node %s\n",
-			codec_np->full_name);
-		ret = -EINVAL;
-		goto error;
-	}
-	machine_data->dai[0].codec_name = machine_data->codec_name;
+	/* ASoC core can match codec with device node */
+	machine_data->dai[0].codec_of_node = codec_np;
 
 	/* The DAI name from the codec (snd_soc_dai_driver.name) */
 	machine_data->dai[0].codec_dai_name = "cs4270-hifi";
diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c
index 60f7bb8..e87cb41 100644
--- a/sound/soc/fsl/p1022_ds.c
+++ b/sound/soc/fsl/p1022_ds.c
@@ -73,7 +73,6 @@ struct machine_data {
 	unsigned int ssi_id;		/* 0 = SSI1, 1 = SSI2, etc */
 	unsigned int dma_id[2];		/* 0 = DMA1, 1 = DMA2, etc */
 	unsigned int dma_channel_id[2]; /* 0 = ch 0, 1 = ch 1, etc*/
-	char codec_name[DAI_NAME_SIZE];
 	char platform_name[2][DAI_NAME_SIZE]; /* One for each DMA channel */
 };
 
@@ -225,16 +224,8 @@ static int p1022_ds_probe(struct platform_device *pdev)
 	mdata->dai[0].cpu_dai_name = dev_name(&ssi_pdev->dev);
 	mdata->dai[0].ops = &p1022_ds_ops;
 
-	/* Determine the codec name, it will be used as the codec DAI name */
-	ret = fsl_asoc_get_codec_dev_name(codec_np, mdata->codec_name,
-					  DAI_NAME_SIZE);
-	if (ret) {
-		dev_err(&pdev->dev, "invalid codec node %s\n",
-			codec_np->full_name);
-		ret = -EINVAL;
-		goto error;
-	}
-	mdata->dai[0].codec_name = mdata->codec_name;
+	/* ASoC core can match codec with device node */
+	mdata->dai[0].codec_of_node = codec_np;
 
 	/* We register two DAIs per SSI, one for playback and the other for
 	 * capture.  We support codecs that have separate DAIs for both playback
-- 
1.7.5.4

[PATCH v3 07/11] ASoC: fsl: check property 'compatible' for the machine name

[Shawn Guo]

Check /compatible rather than /model to determine the machine name.
The p1022ds older device trees get a different /model from the new
ones, while /compatible is consistent there, so checking /compatible
will save the bother of detecting older p1022ds device trees.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/fsl/fsl_ssi.c      |    6 +++---
 sound/soc/fsl/mpc8610_hpcd.c |    2 +-
 sound/soc/fsl/p1022_ds.c     |   32 +++++---------------------------
 3 files changed, 9 insertions(+), 31 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 3e06696..2eb407f 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -716,12 +716,12 @@ static int __devinit fsl_ssi_probe(struct platform_device *pdev)
 	}
 
 	/* Trigger the machine driver's probe function.  The platform driver
-	 * name of the machine driver is taken from the /model property of the
+	 * name of the machine driver is taken from /compatible property of the
 	 * device tree.  We also pass the address of the CPU DAI driver
 	 * structure.
 	 */
-	sprop = of_get_property(of_find_node_by_path("/"), "model", NULL);
-	/* Sometimes the model name has a "fsl," prefix, so we strip that. */
+	sprop = of_get_property(of_find_node_by_path("/"), "compatible", NULL);
+	/* Sometimes the compatible name has a "fsl," prefix, so we strip it. */
 	p = strrchr(sprop, ',');
 	if (p)
 		sprop = p + 1;
diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c
index 354b753..8fdc430 100644
--- a/sound/soc/fsl/mpc8610_hpcd.c
+++ b/sound/soc/fsl/mpc8610_hpcd.c
@@ -402,7 +402,7 @@ static struct platform_driver mpc8610_hpcd_driver = {
 	.probe = mpc8610_hpcd_probe,
 	.remove = __devexit_p(mpc8610_hpcd_remove),
 	.driver = {
-		/* The name must match the 'model' property in the device tree,
+		/* The name must match 'compatible' property in the device tree,
 		 * in lowercase letters.
 		 */
 		.name = "snd-soc-mpc8610hpcd",
diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c
index e87cb41..0f42f31 100644
--- a/sound/soc/fsl/p1022_ds.c
+++ b/sound/soc/fsl/p1022_ds.c
@@ -403,6 +403,11 @@ static struct platform_driver p1022_ds_driver = {
 	.probe = p1022_ds_probe,
 	.remove = __devexit_p(p1022_ds_remove),
 	.driver = {
+		/*
+		 * The name must match 'compatible' property in the device tree,
+		 * in lowercase letters.
+		 */
+		.name = "snd-soc-p1022ds",
 		.owner = THIS_MODULE,
 	},
 };
@@ -416,33 +421,6 @@ static int __init p1022_ds_init(void)
 {
 	struct device_node *guts_np;
 	struct resource res;
-	const char *sprop;
-
-	/*
-	 * Check if we're actually running on a P1022DS.  Older device trees
-	 * have a model of "fsl,P1022" and newer ones use "fsl,P1022DS", so we
-	 * need to support both.  The SSI driver uses that property to link to
-	 * the machine driver, so have to match it.
-	 */
-	sprop = of_get_property(of_find_node_by_path("/"), "model", NULL);
-	if (!sprop) {
-		pr_err("snd-soc-p1022ds: missing /model node");
-		return -ENODEV;
-	}
-
-	pr_debug("snd-soc-p1022ds: board model name is %s\n", sprop);
-
-	/*
-	 * The name of this board, taken from the device tree.  Normally, this is a*
-	 * fixed string, but some P1022DS device trees have a /model property of
-	 * "fsl,P1022", and others have "fsl,P1022DS".
-	 */
-	if (strcasecmp(sprop, "fsl,p1022ds") == 0)
-		p1022_ds_driver.driver.name = "snd-soc-p1022ds";
-	else if (strcasecmp(sprop, "fsl,p1022") == 0)
-		p1022_ds_driver.driver.name = "snd-soc-p1022";
-	else
-		return -ENODEV;
 
 	/* Get the physical address of the global utilities registers */
 	guts_np = of_find_compatible_node(NULL, NULL, "fsl,p1022-guts");
-- 
1.7.5.4

[PATCH v3 07/11] ASoC: fsl: check property 'compatible' for the machine name

[Timur Tabi]

Shawn Guo wrote:
> Check /compatible rather than /model to determine the machine name.
> The p1022ds older device trees get a different /model from the new
> ones, while /compatible is consistent there, so checking /compatible
> will save the bother of detecting older p1022ds device trees.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---

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

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 07/11] ASoC: fsl: check property 'compatible' for the machine name

[Mark Brown]

On Fri, Mar 09, 2012 at 12:59:46AM +0800, Shawn Guo wrote:

> Check /compatible rather than /model to determine the machine name.
> The p1022ds older device trees get a different /model from the new
> ones, while /compatible is consistent there, so checking /compatible
> will save the bother of detecting older p1022ds device trees.

Applied, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120309/654b53c9/attachment-0001.sig>

[PATCH v3 08/11] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Shawn Guo]

Provide different pair of accessors for accessing SSI registers on
PowerPC and ARM/IMX, so that fsl_ssi driver can be built on both
architectures.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/fsl/fsl_ssi.c |   67 +++++++++++++++++++++++++++++++---------------
 1 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 2eb407f..aee2066 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -11,12 +11,16 @@
  */
 
 #include <linux/init.h>
+#include <linux/io.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/device.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
 #include <linux/of_platform.h>
+#include <linux/spinlock.h>
 
 #include <sound/core.h>
 #include <sound/pcm.h>
@@ -26,6 +30,27 @@
 
 #include "fsl_ssi.h"
 
+#ifdef PPC
+#define read_ssi(addr)			 in_be32(addr)
+#define write_ssi(val, addr)		 out_be32(addr, val)
+#define write_ssi_mask(addr, clear, set) clrsetbits_be32(addr, clear, set)
+#elif defined ARM
+#define read_ssi(addr)			 readl(addr)
+#define write_ssi(val, addr)		 writel(val, addr)
+static DEFINE_SPINLOCK(ssi_reg_lock);
+static inline void write_ssi_mask(u32 __iomem *addr, u32 clear, u32 set)
+{
+	u32 val;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ssi_reg_lock, flags);
+	val = readl(addr);
+	val = (val & ~clear) | set;
+	writel(val, addr);
+	spin_unlock_irqrestore(&ssi_reg_lock, flags);
+}
+#endif
+
 /**
  * FSLSSI_I2S_RATES: sample rates supported by the I2S
  *
@@ -145,7 +170,7 @@ static irqreturn_t fsl_ssi_isr(int irq, void *dev_id)
 	   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) & SIER_FLAGS;
+	sisr = read_ssi(&ssi->sisr) & SIER_FLAGS;
 
 	if (sisr & CCSR_SSI_SISR_RFRC) {
 		ssi_private->stats.rfrc++;
@@ -260,7 +285,7 @@ static irqreturn_t fsl_ssi_isr(int irq, void *dev_id)
 
 	/* Clear the bits that we set */
 	if (sisr2)
-		out_be32(&ssi->sisr, sisr2);
+		write_ssi(sisr2, &ssi->sisr);
 
 	return ret;
 }
@@ -295,7 +320,7 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
 		 * SSI needs to be disabled before updating the registers we set
 		 * here.
 		 */
-		clrbits32(&ssi->scr, CCSR_SSI_SCR_SSIEN);
+		write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_SSIEN, 0);
 
 		/*
 		 * Program the SSI into I2S Slave Non-Network Synchronous mode.
@@ -303,20 +328,18 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
 		 *
 		 * FIXME: Little-endian samples require a different shift dir
 		 */
-		clrsetbits_be32(&ssi->scr,
+		write_ssi_mask(&ssi->scr,
 			CCSR_SSI_SCR_I2S_MODE_MASK | CCSR_SSI_SCR_SYN,
 			CCSR_SSI_SCR_TFR_CLK_DIS | CCSR_SSI_SCR_I2S_MODE_SLAVE
 			| (synchronous ? CCSR_SSI_SCR_SYN : 0));
 
-		out_be32(&ssi->stcr,
-			 CCSR_SSI_STCR_TXBIT0 | CCSR_SSI_STCR_TFEN0 |
+		write_ssi(CCSR_SSI_STCR_TXBIT0 | CCSR_SSI_STCR_TFEN0 |
 			 CCSR_SSI_STCR_TFSI | CCSR_SSI_STCR_TEFS |
-			 CCSR_SSI_STCR_TSCKP);
+			 CCSR_SSI_STCR_TSCKP, &ssi->stcr);
 
-		out_be32(&ssi->srcr,
-			 CCSR_SSI_SRCR_RXBIT0 | CCSR_SSI_SRCR_RFEN0 |
+		write_ssi(CCSR_SSI_SRCR_RXBIT0 | CCSR_SSI_SRCR_RFEN0 |
 			 CCSR_SSI_SRCR_RFSI | CCSR_SSI_SRCR_REFS |
-			 CCSR_SSI_SRCR_RSCKP);
+			 CCSR_SSI_SRCR_RSCKP, &ssi->srcr);
 
 		/*
 		 * The DC and PM bits are only used if the SSI is the clock
@@ -324,7 +347,7 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
 		 */
 
 		/* Enable the interrupts and DMA requests */
-		out_be32(&ssi->sier, SIER_FLAGS);
+		write_ssi(SIER_FLAGS, &ssi->sier);
 
 		/*
 		 * Set the watermark for transmit FIFI 0 and receive FIFO 0. We
@@ -339,9 +362,9 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
 		 * make this value larger (and maybe we should), but this way
 		 * data will be written to memory as soon as it's available.
 		 */
-		out_be32(&ssi->sfcsr,
-			CCSR_SSI_SFCSR_TFWM0(ssi_private->fifo_depth - 2) |
-			CCSR_SSI_SFCSR_RFWM0(ssi_private->fifo_depth - 2));
+		write_ssi(CCSR_SSI_SFCSR_TFWM0(ssi_private->fifo_depth - 2) |
+			CCSR_SSI_SFCSR_RFWM0(ssi_private->fifo_depth - 2),
+			&ssi->sfcsr);
 
 		/*
 		 * We keep the SSI disabled because if we enable it, then the
@@ -417,7 +440,7 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream,
 	unsigned int sample_size =
 		snd_pcm_format_width(params_format(hw_params));
 	u32 wl = CCSR_SSI_SxCCR_WL(sample_size);
-	int enabled = in_be32(&ssi->scr) & CCSR_SSI_SCR_SSIEN;
+	int enabled = read_ssi(&ssi->scr) & CCSR_SSI_SCR_SSIEN;
 
 	/*
 	 * If we're in synchronous mode, and the SSI is already enabled,
@@ -439,9 +462,9 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream,
 	/* In synchronous mode, the SSI uses STCCR for capture */
 	if ((substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ||
 	    ssi_private->cpu_dai_drv.symmetric_rates)
-		clrsetbits_be32(&ssi->stccr, CCSR_SSI_SxCCR_WL_MASK, wl);
+		write_ssi_mask(&ssi->stccr, CCSR_SSI_SxCCR_WL_MASK, wl);
 	else
-		clrsetbits_be32(&ssi->srccr, CCSR_SSI_SxCCR_WL_MASK, wl);
+		write_ssi_mask(&ssi->srccr, CCSR_SSI_SxCCR_WL_MASK, wl);
 
 	return 0;
 }
@@ -466,19 +489,19 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd,
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-			setbits32(&ssi->scr,
+			write_ssi_mask(&ssi->scr, 0,
 				CCSR_SSI_SCR_SSIEN | CCSR_SSI_SCR_TE);
 		else
-			setbits32(&ssi->scr,
+			write_ssi_mask(&ssi->scr, 0,
 				CCSR_SSI_SCR_SSIEN | CCSR_SSI_SCR_RE);
 		break;
 
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-			clrbits32(&ssi->scr, CCSR_SSI_SCR_TE);
+			write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_TE, 0);
 		else
-			clrbits32(&ssi->scr, CCSR_SSI_SCR_RE);
+			write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_RE, 0);
 		break;
 
 	default:
@@ -510,7 +533,7 @@ static void fsl_ssi_shutdown(struct snd_pcm_substream *substream,
 	if (!ssi_private->first_stream) {
 		struct ccsr_ssi __iomem *ssi = ssi_private->ssi;
 
-		clrbits32(&ssi->scr, CCSR_SSI_SCR_SSIEN);
+		write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_SSIEN, 0);
 	}
 }
 
-- 
1.7.5.4

[PATCH v3 08/11] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Timur Tabi]

Shawn Guo wrote:
> +static DEFINE_SPINLOCK(ssi_reg_lock);
> +static inline void write_ssi_mask(u32 __iomem *addr, u32 clear, u32 set)
> +{
> +	u32 val;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&ssi_reg_lock, flags);
> +	val = readl(addr);
> +	val = (val & ~clear) | set;
> +	writel(val, addr);
> +	spin_unlock_irqrestore(&ssi_reg_lock, flags);
> +}
> +#endif

I think this spinlock is the wrong approach.  The problem with
read-modify-write is on the function level, not the register level.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 08/11] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Shawn Guo]

On Thu, Mar 08, 2012 at 02:13:04PM -0600, Timur Tabi wrote:
> Shawn Guo wrote:
> > +static DEFINE_SPINLOCK(ssi_reg_lock);
> > +static inline void write_ssi_mask(u32 __iomem *addr, u32 clear, u32 set)
> > +{
> > +	u32 val;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&ssi_reg_lock, flags);
> > +	val = readl(addr);
> > +	val = (val & ~clear) | set;
> > +	writel(val, addr);
> > +	spin_unlock_irqrestore(&ssi_reg_lock, flags);
> > +}
> > +#endif
> 
> I think this spinlock is the wrong approach.  The problem with
> read-modify-write is on the function level, not the register level.
> 
Something like this:

@@ -496,6 +494,10 @@ 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 = snd_soc_dai_get_drvdata(rtd->cpu_dai);
        struct ccsr_ssi __iomem *ssi = ssi_private->ssi;
+       unsigned long flags;
+       int ret = 0;
+
+       spin_lock_irqsave(&ssi_lock, flags);

        switch (cmd) {
        case SNDRV_PCM_TRIGGER_START:
@@ -517,10 +519,12 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd,
                break;

        default:
-               return -EINVAL;
+               ret = -EINVAL;
        }

-       return 0;
+       spin_unlock_irqrestore(&ssi_lock, flags);
+
+       return ret;
 }

-- 
Regards,
Shawn

[PATCH v3 08/11] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Tabi Timur-B04825]

Shawn Guo wrote:

> Something like this:
>
> @@ -496,6 +494,10 @@ 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 = snd_soc_dai_get_drvdata(rtd->cpu_dai);
>          struct ccsr_ssi __iomem *ssi = ssi_private->ssi;
> +       unsigned long flags;
> +       int ret = 0;
> +
> +       spin_lock_irqsave(&ssi_lock, flags);
>
>          switch (cmd) {
>          case SNDRV_PCM_TRIGGER_START:
> @@ -517,10 +519,12 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd,
>                  break;
>
>          default:
> -               return -EINVAL;
> +               ret = -EINVAL;
>          }
>
> -       return 0;
> +       spin_unlock_irqrestore(&ssi_lock, flags);
> +
> +       return ret;
>   }

That's the general idea, but I think it needs more study.  I don't 
consider this to be a bug that needs to be fixed ASAP, since it's never 
been an actual problem so far.  For instance, do we need to protect 
between hw_params and _trigger?

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 08/11] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Shawn Guo]

On Fri, Mar 09, 2012 at 02:09:57AM +0000, Tabi Timur-B04825 wrote:
> That's the general idea, but I think it needs more study.  I don't 
> consider this to be a bug that needs to be fixed ASAP, since it's never 
> been an actual problem so far.  For instance, do we need to protect 
> between hw_params and _trigger?
> 
Ok, I will leave it to you with a big "FIXME" comment then.

-- 
Regards,
Shawn

[PATCH v3 08/11] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Tabi Timur-B04825]

Shawn Guo wrote:
> On Fri, Mar 09, 2012 at 02:09:57AM +0000, Tabi Timur-B04825 wrote:
>> >  That's the general idea, but I think it needs more study.  I don't
>> >  consider this to be a bug that needs to be fixed ASAP, since it's never
>> >  been an actual problem so far.  For instance, do we need to protect
>> >  between hw_params and _trigger?
>> >
> Ok, I will leave it to you with a big "FIXME" comment then.

Ok.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 08/11] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Mark Brown]

On Fri, Mar 09, 2012 at 11:21:31AM +0800, Shawn Guo wrote:
> On Fri, Mar 09, 2012 at 02:09:57AM +0000, Tabi Timur-B04825 wrote:

> > That's the general idea, but I think it needs more study.  I don't 
> > consider this to be a bug that needs to be fixed ASAP, since it's never 
> > been an actual problem so far.  For instance, do we need to protect 
> > between hw_params and _trigger?

Yes, the capture and playback streams may run in parallel.

> Ok, I will leave it to you with a big "FIXME" comment then.

You probably just need a read/modify/write function to do the locking in
the style of update_bits().
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120309/1a035262/attachment-0001.sig>

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Shawn Guo]

As Documentation/devicetree/bindings/powerpc/fsl/ssi.txt documented,
codec-handle is an optional property, so missing it should not be a
fatal error.

More importantly, the imx machine drivers to be added working with
fsl_ssi will not have this property, as they use new ASoC machine
driver binding mechanism.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/fsl/fsl_ssi.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index aee2066..e5903cb 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -645,12 +645,6 @@ static int __devinit fsl_ssi_probe(struct platform_device *pdev)
 	if (!of_device_is_available(np))
 		return -ENODEV;
 
-	/* Check for a codec-handle property. */
-	if (!of_get_property(np, "codec-handle", NULL)) {
-		dev_err(&pdev->dev, "missing codec-handle property\n");
-		return -ENODEV;
-	}
-
 	/* We only support the SSI in "I2S Slave" mode */
 	sprop = of_get_property(np, "fsl,mode", NULL);
 	if (!sprop || strcmp(sprop, "i2s-slave")) {
-- 
1.7.5.4

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Timur Tabi]

Shawn Guo wrote:
> As Documentation/devicetree/bindings/powerpc/fsl/ssi.txt documented,
> codec-handle is an optional property, so missing it should not be a
> fatal error.]

It's optional in the sense that if your SSI is not connected to anything,
then you don't need to specify the codec handle.  If the SSI is connected
to a codec, then the SSI node needs to point to it.

> More importantly, the imx machine drivers to be added working with
> fsl_ssi will not have this property, as they use new ASoC machine
> driver binding mechanism.

I want to see that binding documented in Documentation/devicetree/bindings
first.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Shawn Guo]

On Thu, Mar 08, 2012 at 02:50:21PM -0600, Timur Tabi wrote:
> Shawn Guo wrote:
> > As Documentation/devicetree/bindings/powerpc/fsl/ssi.txt documented,
> > codec-handle is an optional property, so missing it should not be a
> > fatal error.]
> 
> It's optional in the sense that if your SSI is not connected to anything,
> then you don't need to specify the codec handle.  If the SSI is connected
> to a codec, then the SSI node needs to point to it.
> 
It's true for the binding that mpc8610_hpcd and p1022_ds use.  But per
request from Mark, we need to use the new binding for imx machine
drivers, which will have the codec phandle in "audio complex" node.

> > More importantly, the imx machine drivers to be added working with
> > fsl_ssi will not have this property, as they use new ASoC machine
> > driver binding mechanism.
> 
> I want to see that binding documented in Documentation/devicetree/bindings
> first.

Documentation/devicetree/bindings/sound/imx-audio-sgtl5000.txt, which
takes Documentation/devicetree/bindings/sound/tegra-audio-wm8903.txt
as the example.

-- 
Regards,
Shawn

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Timur Tabi]

Shawn Guo wrote:
> Documentation/devicetree/bindings/sound/imx-audio-sgtl5000.txt, which
> takes Documentation/devicetree/bindings/sound/tegra-audio-wm8903.txt
> as the example.

sound {
	compatible = "fsl,imx51-babbage-sgtl5000",
		     "fsl,imx-audio-sgtl5000";
	model = "imx51-babbage-sgtl5000";
	ssi-controller = <&ssi1>;
	audio-codec = <&sgtl5000>;
	mux-int-port = <1>;
	mux-ext-port = <3>;
};

How does this handle multiple SSIs, each connected to its own codec?

And where's the binding for the DMA controller?

I honestly don't see how this is an improvement over what I came up with.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Mark Brown]

On Tue, Mar 13, 2012 at 06:23:50PM -0500, Timur Tabi wrote:

> sound {
> 	compatible = "fsl,imx51-babbage-sgtl5000",
> 		     "fsl,imx-audio-sgtl5000";
> 	model = "imx51-babbage-sgtl5000";
> 	ssi-controller = <&ssi1>;
> 	audio-codec = <&sgtl5000>;
> 	mux-int-port = <1>;
> 	mux-ext-port = <3>;
> };

> How does this handle multiple SSIs, each connected to its own codec?

With this card those would just be separate CODECs.  This is a binding
for a simple card.

> And where's the binding for the DMA controller?

The DMA controller is transparently instantiated on Tegra systems, there
really isn't a meaningfully separate DMA controller - the I2S controller
specifies the signal to use.  Currently the card takes care of the ASoC
level driver stuff but it really should be done by the I2S controller.

> I honestly don't see how this is an improvement over what I came up with.

The biggest improvement is that the SoC binding knows nothing about the
card binding at all, cards are completely separate and are free to
define any binding they choose using any number of on-SoC and off-SoC
devices.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120313/db40adc5/attachment.sig>

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Tabi Timur-B04825]

Mark Brown wrote:

> With this card those would just be separate CODECs.  This is a binding
> for a simple card.

But that is insufficient for the SSI driver, which is expected to handle 
multiple SSIs.  If you're going to document a binding for the SSI driver, 
then you need to document how to handle multiple SSIs.

> The biggest improvement is that the SoC binding knows nothing about the
> card binding at all, cards are completely separate and are free to
> define any binding they choose using any number of on-SoC and off-SoC
> devices.

Ok, I don't understand that at all.

I also don't understand why you insist on perverting my driver to support 
two incompatible bindings, especially since I still haven't heard an 
explanation as to what's wrong with the binding that I defined.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Mark Brown]

On Wed, Mar 14, 2012 at 02:57:00AM +0000, Tabi Timur-B04825 wrote:
> Mark Brown wrote:

> > With this card those would just be separate CODECs.  This is a binding
> > for a simple card.

> But that is insufficient for the SSI driver, which is expected to handle 
> multiple SSIs.  If you're going to document a binding for the SSI driver, 
> then you need to document how to handle multiple SSIs.

For almost all SoC audio ports this works in exactly the same way it
works for multiple instances of any other device - you do a binding that
covers a single device and then you bind multiple instances of that
device in the system each of which need know nothing about any other
instances of the IP.  

If your SoC has IP which provides more than one port in a single IP
block then the driver for that IP would need to register multiple DAIs
from a single binding which is again exactly the same thing as you'd do
with any other IP block that behaved similarly.

All of which is exactly the same as for any other device and essentially
irrelevant to how we define the bindings for the cards.

> > The biggest improvement is that the SoC binding knows nothing about the
> > card binding at all, cards are completely separate and are free to
> > define any binding they choose using any number of on-SoC and off-SoC
> > devices.

> Ok, I don't understand that at all.

Do you have some questions about the above?  What is it that you find
unclear?   

> I also don't understand why you insist on perverting my driver to support 
> two incompatible bindings, especially since I still haven't heard an 
> explanation as to what's wrong with the binding that I defined.

We've been through this *repeatedly*, including in the message you're
replying to, and we're still going back to square one every time you
decide to start talking about device tree again.  This is frustrating
in the extreme.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120314/52007cc3/attachment.sig>

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Timur Tabi]

Mark Brown wrote:

> All of which is exactly the same as for any other device and essentially
> irrelevant to how we define the bindings for the cards.

So using imx-audio-sgtl5000.txt as an example, you're saying that if I
have two SSIs, I should do this in my device tree:

sound1 {
	compatible = "fsl,imx51-babbage-sgtl5000",
		     "fsl,imx-audio-sgtl5000";
	model = "imx51-babbage-sgtl5000";
	ssi-controller = <&ssi1>;
	audio-codec = <&sgtl5000_1>;
	mux-int-port = <1>;
	mux-ext-port = <3>;
};

sound2 {
	compatible = "fsl,imx51-babbage-sgtl5000",
		     "fsl,imx-audio-sgtl5000";
	model = "imx51-babbage-sgtl5000";
	ssi-controller = <&ssi2>;
	audio-codec = <&sgtl5000_2>;
	mux-int-port = <1>;
	mux-ext-port = <3>;
};

>>> The biggest improvement is that the SoC binding knows nothing about the
>>> card binding at all, cards are completely separate and are free to
>>> define any binding they choose using any number of on-SoC and off-SoC
>>> devices.
> 
>> Ok, I don't understand that at all.
> 
> Do you have some questions about the above?  What is it that you find
> unclear?   

What's the difference between an "SoC binding" and a "card binding"?

> We've been through this *repeatedly*, including in the message you're
> replying to, and we're still going back to square one every time you
> decide to start talking about device tree again.  This is frustrating
> in the extreme.

I'm sorry you're frustrated, but so am I.  I still don't understand why
the binding that I invented for the SSI driver isn't good enough for i.MX.
 The binding and the code was documented and approved a long time ago.
Why can't we keep the same binding on i.MX, to keep the code simpler?

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Shawn Guo]

On Wed, Mar 14, 2012 at 06:00:41PM -0500, Timur Tabi wrote:
> Mark Brown wrote:
> 
> > All of which is exactly the same as for any other device and essentially
> > irrelevant to how we define the bindings for the cards.
> 
> So using imx-audio-sgtl5000.txt as an example, you're saying that if I
> have two SSIs, I should do this in my device tree:
> 
> sound1 {
> 	compatible = "fsl,imx51-babbage-sgtl5000",
> 		     "fsl,imx-audio-sgtl5000";
> 	model = "imx51-babbage-sgtl5000";
> 	ssi-controller = <&ssi1>;
> 	audio-codec = <&sgtl5000_1>;
> 	mux-int-port = <1>;
> 	mux-ext-port = <3>;
> };
> 
> sound2 {
> 	compatible = "fsl,imx51-babbage-sgtl5000",
> 		     "fsl,imx-audio-sgtl5000";
> 	model = "imx51-babbage-sgtl5000";
> 	ssi-controller = <&ssi2>;
> 	audio-codec = <&sgtl5000_2>;
> 	mux-int-port = <1>;
> 	mux-ext-port = <3>;
> };
> 
Are you putting an example that has two sgtl5000 codecs connected to
SSI1 and SSI2 on a particular board?

I have never seen a real case for such setup.  Have you actually seen?
I do not even see the need to use two SSI controllers for all imx
platforms I have seen.

The real case I have seen is imx28 saif driver, which has to use two
saif controllers connected to one sgtl5000 to support full-duplex.
I would expect something like below for that machine driver.

sound2 {
	compatible = "fsl,imx28-evk-sgtl5000",
		     "fsl,mxs-audio-sgtl5000";
	model = "imx28-evk-sgtl5000";
	saif-controller = <&saif1, &saif2>;
};

We can have that controller property be a phandle array in the binding.

> >>> The biggest improvement is that the SoC binding knows nothing about the
> >>> card binding at all, cards are completely separate and are free to
> >>> define any binding they choose using any number of on-SoC and off-SoC
> >>> devices.
> > 
> >> Ok, I don't understand that at all.
> > 
> > Do you have some questions about the above?  What is it that you find
> > unclear?   
> 
> What's the difference between an "SoC binding" and a "card binding"?
> 
I guess "SoC binding" means the binding for fsl_ssi while "card
binding" means the binding for imx-sgtl5000 in this particular case.

> > We've been through this *repeatedly*, including in the message you're
> > replying to, and we're still going back to square one every time you
> > decide to start talking about device tree again.  This is frustrating
> > in the extreme.
> 
> I'm sorry you're frustrated, but so am I.

And, so am I :(

I thought you have agreed that we can go for new binding for imx
machine drivers per the discussion you and Mark had on the first
version of the series.  That's why I changed to use the new binding
in v2, and you even had given your ack tag on v2.  Now you are
suddenly against that so strongly.  I really do not understand why.
Can you share your concern regarding to that?  What's the problem
with imx using new binding exactly?

> I still don't understand why
> the binding that I invented for the SSI driver isn't good enough for i.MX.

I guess Mark had made it clear that having "codec-handle" encoded in
ssi node is not as good as what new binding does, having it encoded
in "audio complex" node.

>  The binding and the code was documented and approved a long time ago.
> Why can't we keep the same binding on i.MX,
> to keep the code simpler?

I hardly agree with that.  The PowerPC binding will require:

1. fsl_ssi's probe function needs to trigger machine driver's probe
2. machine driver needs to find ssi's node to access codec-handle

which actually makes it complexer than the new binding.

Anyway, either way works, and I have tried both.  But unfortunately
I still have not made everyone happy.  Are we going to make the series
miss the v3.4 merge window just because of this argument, which does
not make much point to me at all.

-- 
Regards,
Shawn

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Tabi Timur-B04825]

Shawn Guo wrote:

> Are you putting an example that has two sgtl5000 codecs connected to
> SSI1 and SSI2 on a particular board?

No, this is just a theoretical example.

> I have never seen a real case for such setup.  Have you actually seen?
> I do not even see the need to use two SSI controllers for all imx
> platforms I have seen.

The PowerPC MPC8610 has two SSIs, and I have a hacked up board that has 
both connected to two CS4270s.  This is how I tested multiple SSI support 
in my driver.

Since my driver supports any number of SSIs, I expect any changes to it to 
continue that support.

> The real case I have seen is imx28 saif driver, which has to use two
> saif controllers connected to one sgtl5000 to support full-duplex.
> I would expect something like below for that machine driver.
>
> sound2 {
> 	compatible = "fsl,imx28-evk-sgtl5000",
> 		     "fsl,mxs-audio-sgtl5000";
> 	model = "imx28-evk-sgtl5000";
> 	saif-controller =<&saif1,&saif2>;
> };
>
> We can have that controller property be a phandle array in the binding.

I can see how this style of binding would make it easier to support an 
N-to-M mapping of controllers to codecs.  However, this is not a concern 
for the SSI driver.

> I thought you have agreed that we can go for new binding for imx
> machine drivers per the discussion you and Mark had on the first
> version of the series.  That's why I changed to use the new binding
> in v2, and you even had given your ack tag on v2.  Now you are
> suddenly against that so strongly.  I really do not understand why.
> Can you share your concern regarding to that?  What's the problem
> with imx using new binding exactly?

I admit I may have been too hasty in giving that ACK, because I did not 
take the time to study the binding.  I figured that the new binding was 
superior to what I came up with years ago.

Now that I've studied the binding, I no longer believe that.  I don't 
understand what's so wonderful about the new binding that my driver has to 
support it *and* the old binding.  I think it would be easier if i.MX uses 
the original binding.

>> I still don't understand why
>> the binding that I invented for the SSI driver isn't good enough for i.MX.
>
> I guess Mark had made it clear that having "codec-handle" encoded in
> ssi node is not as good as what new binding does, having it encoded
> in "audio complex" node.

Even if the new binding is "better", I do not see how it's SOOOOO much 
better that my driver has to support it.  The drawback in supporting both 
is the added complexity.

>> Why can't we keep the same binding on i.MX,
>> to keep the code simpler?
>
> I hardly agree with that.  The PowerPC binding will require:
>
> 1. fsl_ssi's probe function needs to trigger machine driver's probe
> 2. machine driver needs to find ssi's node to access codec-handle
>
> which actually makes it complexer than the new binding.

But the driver already does this!  No one's asking you to add the code to 
perform these tasks.  Just use the existing code.

> Anyway, either way works, and I have tried both.  But unfortunately
> I still have not made everyone happy.  Are we going to make the series
> miss the v3.4 merge window just because of this argument, which does
> not make much point to me at all.

I can respect that concern, but at the same time, I still have to push for 
what I believe makes the most sense.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Shawn Guo]

On Thu, Mar 15, 2012 at 01:37:06PM +0000, Tabi Timur-B04825 wrote:
...
> The PowerPC MPC8610 has two SSIs, and I have a hacked up board that has 
> both connected to two CS4270s.  This is how I tested multiple SSI support 
> in my driver.
> 
> Since my driver supports any number of SSIs, I expect any changes to it to 
> continue that support.
> 
I think it can still easily get supported with the binding like below.

sound {
      compatible = "fsl,imx51-babbage-sgtl5000",
                   "fsl,imx-audio-sgtl5000";
      model = "imx51-babbage-sgtl5000";
      ssi-controller = <&ssi1, &ssi2>;
      audio-codec = <&sgtl5000_1, &sgtl5000_2>;
      mux-int-port = <1>;
      mux-ext-port = <3>;
};

The only difference will be both ssi-controller and audio-codec become
an array of phandles.

...
> I admit I may have been too hasty in giving that ACK, because I did not 
> take the time to study the binding.  I figured that the new binding was 
> superior to what I came up with years ago.
> 
> Now that I've studied the binding, I no longer believe that.  I don't 
> understand what's so wonderful about the new binding that my driver has to 
> support it *and* the old binding.  I think it would be easier if i.MX uses 
> the original binding.
> 
I do not think it would be easier for imx.  Looking at the imx-sgtl5000
binding, you will find we have audmux port configuration encoded there.
How would you suggest to get that fit into the PowerPC binding?

...
> Even if the new binding is "better", I do not see how it's SOOOOO much 
> better that my driver has to support it.  The drawback in supporting both 
> is the added complexity.

If you insist on that the driver should only support single binding,
I would rather change mpc8610_hpcd and p1022_ds to use the new binding.

...
> But the driver already does this!  No one's asking you to add the code to 
> perform these tasks.  Just use the existing code.

Again, what's your suggestion to support audmux configuration with the
existing binding?

-- 
Regards,
Shawn

[alsa-devel] [PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Trent Piepho]

On Thu, Mar 15, 2012 at 10:21 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Thu, Mar 15, 2012 at 01:37:06PM +0000, Tabi Timur-B04825 wrote:
> ...
> > The PowerPC MPC8610 has two SSIs, and I have a hacked up board that has
> > both connected to two CS4270s. ?This is how I tested multiple SSI support
> > in my driver.
> >
> > Since my driver supports any number of SSIs, I expect any changes to it to
> > continue that support.
> >
> I think it can still easily get supported with the binding like below.
>
> sound {
> ? ? ?compatible = "fsl,imx51-babbage-sgtl5000",
> ? ? ? ? ? ? ? ? ? "fsl,imx-audio-sgtl5000";
> ? ? ?model = "imx51-babbage-sgtl5000";
> ? ? ?ssi-controller = <&ssi1, &ssi2>;
> ? ? ?audio-codec = <&sgtl5000_1, &sgtl5000_2>;
> ? ? ?mux-int-port = <1>;
> ? ? ?mux-ext-port = <3>;
> };

How would you support binding the imx51 ESAI controller?

>
> The only difference will be both ssi-controller and audio-codec become
> an array of phandles.
>
> ...
> > I admit I may have been too hasty in giving that ACK, because I did not
> > take the time to study the binding. ?I figured that the new binding was
> > superior to what I came up with years ago.
> >
> > Now that I've studied the binding, I no longer believe that. ?I don't
> > understand what's so wonderful about the new binding that my driver has to
> > support it *and* the old binding. ?I think it would be easier if i.MX uses
> > the original binding.
> >
> I do not think it would be easier for imx. ?Looking at the imx-sgtl5000
> binding, you will find we have audmux port configuration encoded there.
> How would you suggest to get that fit into the PowerPC binding?
>
> ...
> > Even if the new binding is "better", I do not see how it's SOOOOO much
> > better that my driver has to support it. ?The drawback in supporting both
> > is the added complexity.
>
> If you insist on that the driver should only support single binding,
> I would rather change mpc8610_hpcd and p1022_ds to use the new binding.
>
> ...
> > But the driver already does this! ?No one's asking you to add the code to
> > perform these tasks. ?Just use the existing code.
>
> Again, what's your suggestion to support audmux configuration with the
> existing binding?
>
> --
> Regards,
> Shawn
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Trent Piepho]

On Thu, Mar 15, 2012 at 11:39 AM, Trent Piepho <tpiepho@gmail.com> wrote:
> On Thu, Mar 15, 2012 at 10:21 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
>> On Thu, Mar 15, 2012 at 01:37:06PM +0000, Tabi Timur-B04825 wrote:
>> ...
>> > The PowerPC MPC8610 has two SSIs, and I have a hacked up board that has
>> > both connected to two CS4270s. ?This is how I tested multiple SSI support
>> > in my driver.
>> >
>> > Since my driver supports any number of SSIs, I expect any changes to it to
>> > continue that support.
>> >
>> I think it can still easily get supported with the binding like below.
>>
>> sound {
>> ? ? ?compatible = "fsl,imx51-babbage-sgtl5000",
>> ? ? ? ? ? ? ? ? ? "fsl,imx-audio-sgtl5000";
>> ? ? ?model = "imx51-babbage-sgtl5000";
>> ? ? ?ssi-controller = <&ssi1, &ssi2>;
>> ? ? ?audio-codec = <&sgtl5000_1, &sgtl5000_2>;
>> ? ? ?mux-int-port = <1>;
>> ? ? ?mux-ext-port = <3>;
>> };
>
> How would you support binding the imx51 ESAI controller?

Sorry about previous email, laptop touchpad emitted a spurious click,
which caused text window in gmail to become deselected, which caused a
subsequent enter to send the email rather than creating a new line.

For esai, would you add something like:
    esai-controller = <&esai1>;
    esai-audio-codec = <&sgtl5000_3>;

That seems rather ugly to me.

It seems to me in most device bindings where one piece of hardware is
attached to another, than the one upstream has in its binding a
phandle to the one downstream.  But here we have a third binding with
two lists of devices and an implied link between devices in
corresponding positions in the lists.  Does any other device binding
work like that?

[alsa-devel] [PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Mark Brown]

On Thu, Mar 15, 2012 at 11:57:52AM -0400, Trent Piepho wrote:

> which caused text window in gmail to become deselected, which caused a
> subsequent enter to send the email rather than creating a new line.

> For esai, would you add something like:
>     esai-controller = <&esai1>;
>     esai-audio-codec = <&sgtl5000_3>;

> That seems rather ugly to me.

None of these examples seem particularly to reflect realistic audio
hardware; they're more describing a board with multiple distinct audio
subsystems on it than a single card.  Remember that nothing at all is
fixed about how the card binding works internally, the card driver is
free to pick any binding that makes sense for the systems it supports.
Shoehorning every possible card into the same binding is definitely a
really bad idea.

> It seems to me in most device bindings where one piece of hardware is
> attached to another, than the one upstream has in its binding a
> phandle to the one downstream.  But here we have a third binding with
> two lists of devices and an implied link between devices in
> corresponding positions in the lists.  Does any other device binding
> work like that?

There's no upstream and downstream here, really.  But like I say none of
this looks like realistic hardware anyway.

*Please* go and read the previous discussions of audio bindings for
SoCs, while it's more understandable with people who haven't been
involved before its still tiresome to have to go back to square one.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120315/8ed7c6cc/attachment.sig>

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Timur Tabi]

Shawn Guo wrote:
> sound {
>       compatible = "fsl,imx51-babbage-sgtl5000",
>                    "fsl,imx-audio-sgtl5000";
>       model = "imx51-babbage-sgtl5000";
>       ssi-controller = <&ssi1, &ssi2>;
>       audio-codec = <&sgtl5000_1, &sgtl5000_2>;
>       mux-int-port = <1>;
>       mux-ext-port = <3>;
> };

Based on Mark's other email, I would rather see this:

sound1 {
      compatible = "fsl,imx51-babbage-sgtl5000",
                   "fsl,imx-audio-sgtl5000";
      model = "imx51-babbage-sgtl5000";
      ssi-controller = <&ssi1>;
      audio-codec = <&sgtl5000_1>;
      mux-int-port = <1>;
      mux-ext-port = <3>;
};

sound2 {
      compatible = "fsl,imx51-babbage-sgtl5000",
                   "fsl,imx-audio-sgtl5000";
      model = "imx51-babbage-sgtl5000";
      ssi-controller = <&ssi2>;
      audio-codec = <&sgtl5000_2>;
      mux-int-port = <1>;
      mux-ext-port = <3>;
};

(ignoring for the moment that I have no idea what the mux-xxx-port values
in sound2 should be)

This makes it clear that ssi1 connects only to sgtl5000_1.

If you're okay with this, then please update the documentation accordingly.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Shawn Guo]

On Thu, Mar 15, 2012 at 11:47:05AM -0500, Timur Tabi wrote:
> Shawn Guo wrote:
> > sound {
> >       compatible = "fsl,imx51-babbage-sgtl5000",
> >                    "fsl,imx-audio-sgtl5000";
> >       model = "imx51-babbage-sgtl5000";
> >       ssi-controller = <&ssi1, &ssi2>;
> >       audio-codec = <&sgtl5000_1, &sgtl5000_2>;
> >       mux-int-port = <1>;
> >       mux-ext-port = <3>;
> > };
> 
> Based on Mark's other email, I would rather see this:
> 
> sound1 {
>       compatible = "fsl,imx51-babbage-sgtl5000",
>                    "fsl,imx-audio-sgtl5000";
>       model = "imx51-babbage-sgtl5000";
>       ssi-controller = <&ssi1>;
>       audio-codec = <&sgtl5000_1>;
>       mux-int-port = <1>;
>       mux-ext-port = <3>;
> };
> 
> sound2 {
>       compatible = "fsl,imx51-babbage-sgtl5000",
>                    "fsl,imx-audio-sgtl5000";
>       model = "imx51-babbage-sgtl5000";
>       ssi-controller = <&ssi2>;
>       audio-codec = <&sgtl5000_2>;
>       mux-int-port = <1>;
>       mux-ext-port = <3>;
> };
> 
> (ignoring for the moment that I have no idea what the mux-xxx-port values
> in sound2 should be)
> 
> This makes it clear that ssi1 connects only to sgtl5000_1.
> 
> If you're okay with this, then please update the documentation accordingly.
> 
Hmm, Documentation/devicetree/bindings/sound/imx-audio-sgtl5000.txt
is the binding document specifically for imx-sgtl5000 driver rather
than for all fsl_ssi based machine drivers.  Since I have never seen
and doubt will ever see the setup that two SSIs connect to two
sgtl5000 codecs on any imx board, I do not see much point to update
the documentation for that.

If this is indeed a practical setup for PowerPC, it can be documented
in the binding for PowerPC machine driver when there is future PowerPC
machine drivers adopting the new binding.

-- 
Regards,
Shawn

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Tabi Timur-B04825]

Shawn Guo wrote:
> Hmm, Documentation/devicetree/bindings/sound/imx-audio-sgtl5000.txt
> is the binding document specifically for imx-sgtl5000 driver rather
> than for all fsl_ssi based machine drivers.  Since I have never seen
> and doubt will ever see the setup that two SSIs connect to two
> sgtl5000 codecs on any imx board, I do not see much point to update
> the documentation for that.
>
> If this is indeed a practical setup for PowerPC, it can be documented
> in the binding for PowerPC machine driver when there is future PowerPC
> machine drivers adopting the new binding.

Ok.  I was going to say that I'd like to see a template that applies to 
all SOCs with an SSI, but I guess that won't be that useful.

I'm just concerned that if there's no standard way to create bindings for 
machine drivers, we're going to end up with some kind of mess down the road.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Mark Brown]

On Fri, Mar 16, 2012 at 01:55:01AM +0000, Tabi Timur-B04825 wrote:

> I'm just concerned that if there's no standard way to create bindings for 
> machine drivers, we're going to end up with some kind of mess down the road.

I think that we'll be able to deal with that by adding helpers for
common cases (as has already started to happen with some of the nVidia
inspired changes like the auto_nc_pins() stuff) - approach the problem
by extracting the good practice and putting it into helpers to encourage
use.  Fortunately machine drivers are by their nature system specific so
so if they are messed up they impact a limited subset of systems.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120317/97ff8154/attachment.sig>

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Mark Brown]

On Wed, Mar 14, 2012 at 06:00:41PM -0500, Timur Tabi wrote:
> Mark Brown wrote:

> So using imx-audio-sgtl5000.txt as an example, you're saying that if I
> have two SSIs, I should do this in my device tree:

> sound1 {
> };

> sound2 {
> };

That's a totally sensible option if you've got two unrelated audio
subsystems on your board for some reason.  If you've got a system where
that's not applicable and the two devices are related in some way then
you'd define a new sound card binding that reflects that.

> >>> The biggest improvement is that the SoC binding knows nothing about the
> >>> card binding at all, cards are completely separate and are free to
> >>> define any binding they choose using any number of on-SoC and off-SoC
> >>> devices.

> >> Ok, I don't understand that at all.

> > Do you have some questions about the above?  What is it that you find
> > unclear?   

> What's the difference between an "SoC binding" and a "card binding"?

The SoC is the bit of silicon with the CPU and other devices on it like
the DMA controller and SSI or whatever ports.  The card is the PCB this
has been soldered down onto.

> > We've been through this *repeatedly*, including in the message you're
> > replying to, and we're still going back to square one every time you
> > decide to start talking about device tree again.  This is frustrating
> > in the extreme.

> I'm sorry you're frustrated, but so am I.  I still don't understand why
> the binding that I invented for the SSI driver isn't good enough for i.MX.
>  The binding and the code was documented and approved a long time ago.
> Why can't we keep the same binding on i.MX, to keep the code simpler?

The problem with your binding has always been, and continues to be, that
it's based on the idea that there's one CODEC per SSI and that the CODEC
is a simple appendage of that SSI.  There's no real real binding for the
machine driver, it's just silently created by a single SSI port.  Really
there's nothing new with this issue, there's always been this absence of
a representation for the machine distinct from the individual devices it
is built up from.

This means that when you get systems which have auxiliary devices (like
most of the Wolfson reference boards which have a power amplifier for
the sub speaker not connected to an SSI port) or that need to set up
things like complex accessory detection mechanisms there's no real place
in your binding to support those systems.  Things like the configuration
of the input and output connections which the nVidia WM8903 drivers are
able to do over multiple boards are another example.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120315/5a7763df/attachment.sig>

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Shawn Guo]

On Thu, Mar 15, 2012 at 02:27:03PM +0000, Mark Brown wrote:
> On Wed, Mar 14, 2012 at 06:00:41PM -0500, Timur Tabi wrote:
...
> > I'm sorry you're frustrated, but so am I.  I still don't understand why
> > the binding that I invented for the SSI driver isn't good enough for i.MX.
> >  The binding and the code was documented and approved a long time ago.
> > Why can't we keep the same binding on i.MX, to keep the code simpler?
> 
> The problem with your binding has always been, and continues to be, that
> it's based on the idea that there's one CODEC per SSI and that the CODEC
> is a simple appendage of that SSI.  There's no real real binding for the
> machine driver, it's just silently created by a single SSI port.  Really
> there's nothing new with this issue, there's always been this absence of
> a representation for the machine distinct from the individual devices it
> is built up from.
> 
> This means that when you get systems which have auxiliary devices (like
> most of the Wolfson reference boards which have a power amplifier for
> the sub speaker not connected to an SSI port) or that need to set up
> things like complex accessory detection mechanisms there's no real place
> in your binding to support those systems.  Things like the configuration
> of the input and output connections which the nVidia WM8903 drivers are
> able to do over multiple boards are another example.

Yes, we will have the exactly same thing for imx-sgtl5000 to be added
later.  The current imx-sgtl5000 driver just an initial support.

-- 
Regards,
Shawn

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Timur Tabi]

Mark Brown wrote:
> On Wed, Mar 14, 2012 at 06:00:41PM -0500, Timur Tabi wrote:
>> Mark Brown wrote:
> 
>> So using imx-audio-sgtl5000.txt as an example, you're saying that if I
>> have two SSIs, I should do this in my device tree:
> 
>> sound1 {
>> };
> 
>> sound2 {
>> };
> 
> That's a totally sensible option if you've got two unrelated audio
> subsystems on your board for some reason.  If you've got a system where
> that's not applicable and the two devices are related in some way then
> you'd define a new sound card binding that reflects that.

Well, the two SSIs are related only in that they're the same kind of
device.  But SSI1 connected to CS4270_1 is completely independent from
SSI2 connected to CS4270_2.

So I guess that means that we'd have two soundX {} top-level nodes.

> The SoC is the bit of silicon with the CPU and other devices on it like
> the DMA controller and SSI or whatever ports.  The card is the PCB this
> has been soldered down onto.

Well, I asked about the difference between the soc BINDING and the card
BINDING.  On PowerPC, at least, there's no distinct binding for either.
For example, I2C devices are on the PCB, but they're listed as child nodes
of the I2C controller, which is on the SoC.

> The problem with your binding has always been, and continues to be, that
> it's based on the idea that there's one CODEC per SSI and that the CODEC
> is a simple appendage of that SSI.  There's no real real binding for the
> machine driver, it's just silently created by a single SSI port.  Really
> there's nothing new with this issue, there's always been this absence of
> a representation for the machine distinct from the individual devices it
> is built up from.
> 
> This means that when you get systems which have auxiliary devices (like
> most of the Wolfson reference boards which have a power amplifier for
> the sub speaker not connected to an SSI port) or that need to set up
> things like complex accessory detection mechanisms there's no real place
> in your binding to support those systems.  Things like the configuration
> of the input and output connections which the nVidia WM8903 drivers are
> able to do over multiple boards are another example.

Ok, fair enough.  I understand that my binding has no way of specifying
board-specific properties, or anything that isn't directly related to the
SSI, codec, or DMA controller.

However, I don't like the way this is being represented as a PowerPC vs.
ARM issue, because that's just not correct.  It's an "old binding" vs "new
binding" issue.  For example:

+	/*
+	 * In case of imx, the machine driver uses new binding which does
+	 * not require SSI driver to trigger machine driver's probe, but
+	 * the pcm device needs to be registered here.
+	 */
+	if (ssi_private->ssi_on_imx) {
+		ssi_private->imx_pcm_pdev =
+			platform_device_register_simple("imx-pcm-audio",

This prohibits me from using the new binding on any future PowerPC parts,
because it clearly says "iMX" on everything.
-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Mark Brown]

On Thu, Mar 15, 2012 at 11:44:16AM -0500, Timur Tabi wrote:
> Mark Brown wrote:

> > The SoC is the bit of silicon with the CPU and other devices on it like
> > the DMA controller and SSI or whatever ports.  The card is the PCB this
> > has been soldered down onto.

> Well, I asked about the difference between the soc BINDING and the card
> BINDING.  On PowerPC, at least, there's no distinct binding for either.
> For example, I2C devices are on the PCB, but they're listed as child nodes
> of the I2C controller, which is on the SoC.

Right, but this is the problem in a nutshell - there's nowhere to put
the board binding so the SoC code has to know about and do it.

> However, I don't like the way this is being represented as a PowerPC vs.
> ARM issue, because that's just not correct.  It's an "old binding" vs "new
> binding" issue.  For example:

I think that's just creeping in because the old binding is the existing
practice on PowerPC.

> +	/*
> +	 * In case of imx, the machine driver uses new binding which does
> +	 * not require SSI driver to trigger machine driver's probe, but
> +	 * the pcm device needs to be registered here.
> +	 */
> +	if (ssi_private->ssi_on_imx) {
> +		ssi_private->imx_pcm_pdev =
> +			platform_device_register_simple("imx-pcm-audio",

> This prohibits me from using the new binding on any future PowerPC parts,
> because it clearly says "iMX" on everything.

So long as it's internal to the code (which the above is) I don't think
it really matters, the code can be changed later.  We need to do
something about DMA controllers in general, and there was some recent
discussion on the DT list about a generic binding for them, so leaving
the code as-is with nothing in the DT itself seems like a good place to
leave things initially.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120315/896de2a3/attachment.sig>

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Shawn Guo]

On Thu, Mar 15, 2012 at 11:44:16AM -0500, Timur Tabi wrote:
...
> Ok, fair enough.  I understand that my binding has no way of specifying
> board-specific properties, or anything that isn't directly related to the
> SSI, codec, or DMA controller.
> 
> However, I don't like the way this is being represented as a PowerPC vs.
> ARM issue, because that's just not correct.  It's an "old binding" vs "new
> binding" issue.  For example:
> 
> +	/*
> +	 * In case of imx, the machine driver uses new binding which does
> +	 * not require SSI driver to trigger machine driver's probe, but
> +	 * the pcm device needs to be registered here.
> +	 */
> +	if (ssi_private->ssi_on_imx) {

This could probably change to check whether property codec-handle is
present under SSI node to decide if it runs with "old binding" or
"new binding".

> +		ssi_private->imx_pcm_pdev =
> +			platform_device_register_simple("imx-pcm-audio",
> 
This is still IMX vs. PowerPC thing rather than "old binding" vs. "new
binding" one.

> This prohibits me from using the new binding on any future PowerPC parts,
> because it clearly says "iMX" on everything.

I think we can change it later when the future PowerPC parts come to
use the new binding.  In any case, I do not see this is an issue that
should block the whole series.

-- 
Regards,
Shawn

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Tabi Timur-B04825]

Shawn Guo wrote:

> This could probably change to check whether property codec-handle is
> present under SSI node to decide if it runs with "old binding" or
> "new binding".

I guess that's how we need to do it.  The SSI driver should not be looking 
for /sound node, but that's the most reliable way to know if we have a new 
or old binding.

>> +		ssi_private->imx_pcm_pdev =
>> +			platform_device_register_simple("imx-pcm-audio",
>>
> This is still IMX vs. PowerPC thing rather than "old binding" vs. "new
> binding" one.

What is "imx-pcm-audio"?

>> This prohibits me from using the new binding on any future PowerPC parts,
>> because it clearly says "iMX" on everything.
>
> I think we can change it later when the future PowerPC parts come to
> use the new binding.  In any case, I do not see this is an issue that
> should block the whole series.

At least get rid of any unnecessary references to "imx".

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Shawn Guo]

On Fri, Mar 16, 2012 at 02:07:42AM +0000, Tabi Timur-B04825 wrote:
...
> >> +		ssi_private->imx_pcm_pdev =
> >> +			platform_device_register_simple("imx-pcm-audio",
> >>
> > This is still IMX vs. PowerPC thing rather than "old binding" vs. "new
> > binding" one.
> 
> What is "imx-pcm-audio"?
> 
Something like "fsl-pcm-audio", but different from "fsl-pcm-audio"
will be instantiated as a hardware block by device tree,
"imx-pcm-audio" is pretty much like a ASoC wrapper for calling
dmaengine driver.  So there is no hardware block presenting this
"device" in device tree, and we need to instantiate it from cpu DAI
driver as Mark suggested.

-- 
Regards,
Shawn

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Tabi Timur-B04825]

Shawn Guo wrote:
>> >
> Something like "fsl-pcm-audio", but different from "fsl-pcm-audio"
> will be instantiated as a hardware block by device tree,
> "imx-pcm-audio" is pretty much like a ASoC wrapper for calling
> dmaengine driver.  So there is no hardware block presenting this
> "device" in device tree, and we need to instantiate it from cpu DAI
> driver as Mark suggested.

Wait, is this the DMA controller?  Because on PowerPC, we have device tree 
nodes for the DMA controller.  Why don't you have one on i.MX?

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Shawn Guo]

On Fri, Mar 16, 2012 at 03:44:43AM +0000, Tabi Timur-B04825 wrote:
> Shawn Guo wrote:
> >> >
> > Something like "fsl-pcm-audio", but different from "fsl-pcm-audio"
> > will be instantiated as a hardware block by device tree,
> > "imx-pcm-audio" is pretty much like a ASoC wrapper for calling
> > dmaengine driver.  So there is no hardware block presenting this
> > "device" in device tree, and we need to instantiate it from cpu DAI
> > driver as Mark suggested.
> 
> Wait, is this the DMA controller?  Because on PowerPC, we have device tree 
> nodes for the DMA controller.  Why don't you have one on i.MX?
> 
We do have a DMA controller node for IMX.

	sdma at 83fb0000 {
		compatible = "fsl,imx51-sdma", "fsl,imx35-sdma";
		reg = <0x83fb0000 0x4000>;
		interrupts = <6>;
	};

But it's instantiated as a dmaengine device backed by dmaengine driver
drivers/dma/imx-dma.c.  "imx-pcm-audio" will just be an ASoC dmaengine
client using dmaengine API.

-- 
Regards,
Shawn

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Tabi Timur-B04825]

Shawn Guo wrote:
> But it's instantiated as a dmaengine device backed by dmaengine driver
> drivers/dma/imx-dma.c.  "imx-pcm-audio" will just be an ASoC dmaengine
> client using dmaengine API.

So you don't have the equivalent of sound/soc/fsl_dma.c?  Don't you need 
to program the DMA controller specifically for audio?  There's no way I 
could use drivers/dma/fsldma.c for audio.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Shawn Guo]

On Fri, Mar 16, 2012 at 04:08:32AM +0000, Tabi Timur-B04825 wrote:
> Shawn Guo wrote:
> > But it's instantiated as a dmaengine device backed by dmaengine driver
> > drivers/dma/imx-dma.c.  "imx-pcm-audio" will just be an ASoC dmaengine
> > client using dmaengine API.
> 
> So you don't have the equivalent of sound/soc/fsl_dma.c?  Don't you need 
> to program the DMA controller specifically for audio?  There's no way I 
> could use drivers/dma/fsldma.c for audio.
> 
I need to program the DMA controller for audio.  But that's done through
generic dmaengine API with audio specific parameters passed in.  That's
pretty much what all snd_imx_pcm_hw_params (sound/soc/fsl/imx-pcm-dma.c)
does there.

-- 
Regards,
Shawn

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Tabi Timur-B04825]

Shawn Guo wrote:
> I need to program the DMA controller for audio.  But that's done through
> generic dmaengine API with audio specific parameters passed in.  That's
> pretty much what all snd_imx_pcm_hw_params (sound/soc/fsl/imx-pcm-dma.c)
> does there.

Interesting.  I'll have to check that out.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Shawn Guo]

On Fri, Mar 16, 2012 at 02:07:42AM +0000, Tabi Timur-B04825 wrote:
> Shawn Guo wrote:
> 
> > This could probably change to check whether property codec-handle is
> > present under SSI node to decide if it runs with "old binding" or
> > "new binding".
> 
> I guess that's how we need to do it.  The SSI driver should not be looking 
> for /sound node, but that's the most reliable way to know if we have a new 
> or old binding.
> 
...
> >> This prohibits me from using the new binding on any future PowerPC parts,
> >> because it clearly says "iMX" on everything.
> >
> > I think we can change it later when the future PowerPC parts come to
> > use the new binding.  In any case, I do not see this is an issue that
> > should block the whole series.
> 
> At least get rid of any unnecessary references to "imx".
> 
So something like this?

        /*
         * If codec-handle property is missing from SSI node, we assume
         * that the machine driver uses new binding which does not require
         * SSI driver to trigger machine driver's probe.
         */
        if (!of_get_property(np, "codec-handle", NULL)) {
                if (ssi_private->ssi_on_imx) {
                        ssi_private->imx_pcm_pdev =
                                platform_device_register_simple("imx-pcm-audio",
                                                                -1, NULL, 0);
                        if (IS_ERR(ssi_private->imx_pcm_pdev)) {
                                ret = PTR_ERR(ssi_private->imx_pcm_pdev);
                                goto error_dev;
                        }
                }
                /* success for new binding case */
                return 0;
        }

It does not reduce any reference to "imx" actually.  If you think it's
worth another iteration of the series, I will post v5 for it.

-- 
Regards,
Shawn

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Tabi Timur-B04825]

Shawn Guo wrote:
> So something like this?
>
>          /*
>           * If codec-handle property is missing from SSI node, we assume
>           * that the machine driver uses new binding which does not require
>           * SSI driver to trigger machine driver's probe.
>           */
>          if (!of_get_property(np, "codec-handle", NULL)) {
>                  if (ssi_private->ssi_on_imx) {
>                          ssi_private->imx_pcm_pdev =
>                                  platform_device_register_simple("imx-pcm-audio",
>                                                                  -1, NULL, 0);
>                          if (IS_ERR(ssi_private->imx_pcm_pdev)) {
>                                  ret = PTR_ERR(ssi_private->imx_pcm_pdev);
>                                  goto error_dev;
>                          }
>                  }
>                  /* success for new binding case */
>                  return 0;
>          }
>
> It does not reduce any reference to "imx" actually.  If you think it's
> worth another iteration of the series, I will post v5 for it.

I had something more like this in mind:

         /*
          * If codec-handle property is missing from SSI node, we assume
          * that the machine driver uses new binding which does not require
          * SSI driver to trigger machine driver's probe.
          */
         if (!of_get_property(np, "codec-handle", NULL))
		ssi_private->new_binding = true;


	if (ssi_private->ssi_on_imx) {
                 ssi_private->imx_pcm_pdev =
                 	platform_device_register_simple("imx-pcm-audio",
                                                                 -1, NULL, 0);
                         if (IS_ERR(ssi_private->imx_pcm_pdev)) {
                                 ret = PTR_ERR(ssi_private->imx_pcm_pdev);
                                 goto error_dev;
                         }
                 }
                 /* success for new binding case */
                 return 0;
         }

Well, it's not perfect, but the idea is that we keep track of new vs. old 
binding separately from imx vs. powerpc.  Although I'm thinking there 
might be a way to general the call to platform_device_register_simple. 
Maybe we could put "imx-pcm-audio" in the device tree?

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Shawn Guo]

On Fri, Mar 16, 2012 at 03:53:39AM +0000, Tabi Timur-B04825 wrote:
> Shawn Guo wrote:
> > So something like this?
> >
> >          /*
> >           * If codec-handle property is missing from SSI node, we assume
> >           * that the machine driver uses new binding which does not require
> >           * SSI driver to trigger machine driver's probe.
> >           */
> >          if (!of_get_property(np, "codec-handle", NULL)) {
> >                  if (ssi_private->ssi_on_imx) {
> >                          ssi_private->imx_pcm_pdev =
> >                                  platform_device_register_simple("imx-pcm-audio",
> >                                                                  -1, NULL, 0);
> >                          if (IS_ERR(ssi_private->imx_pcm_pdev)) {
> >                                  ret = PTR_ERR(ssi_private->imx_pcm_pdev);
> >                                  goto error_dev;
> >                          }
> >                  }
> >                  /* success for new binding case */
> >                  return 0;
> >          }
> >
> > It does not reduce any reference to "imx" actually.  If you think it's
> > worth another iteration of the series, I will post v5 for it.
> 
> I had something more like this in mind:
> 
>          /*
>           * If codec-handle property is missing from SSI node, we assume
>           * that the machine driver uses new binding which does not require
>           * SSI driver to trigger machine driver's probe.
>           */
>          if (!of_get_property(np, "codec-handle", NULL))
> 		ssi_private->new_binding = true;
> 
> 
> 	if (ssi_private->ssi_on_imx) {
>                  ssi_private->imx_pcm_pdev =
>                  	platform_device_register_simple("imx-pcm-audio",
>                                                                  -1, NULL, 0);
>                          if (IS_ERR(ssi_private->imx_pcm_pdev)) {
>                                  ret = PTR_ERR(ssi_private->imx_pcm_pdev);
>                                  goto error_dev;
>                          }
>                  }
>                  /* success for new binding case */
>                  return 0;
>          }
> 
> Well, it's not perfect, but the idea is that we keep track of new vs. old 
> binding separately from imx vs. powerpc.

Fine with me.

> Although I'm thinking there 
> might be a way to general the call to platform_device_register_simple. 
> Maybe we could put "imx-pcm-audio" in the device tree?
> 
It seems a little bit abuse of device tree to me.  My first choice was
to do that in imx-sgtl5000 machine driver.  But Mark disagrees.

-- 
Regards,
Shawn

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Mark Brown]

On Fri, Mar 16, 2012 at 03:53:39AM +0000, Tabi Timur-B04825 wrote:

> might be a way to general the call to platform_device_register_simple. 
> Maybe we could put "imx-pcm-audio" in the device tree?

We might end up doing that but as this adaption of the DMA controller to
the audio stack isn't really hardware so much as an implementation
detail of the audio stack it'd be nice to come up with some neater way
of doing it, ideally not involving the platform device at all.

I'm starting to think we should have a framework thing that lets DAI
drivers say they're a combined DMA/DAI driver for cases like this where
there is a generic DMA device that's being shared by lots of different
subsystems.  It's a common situation and it makes the device registration
clearer.  Adding Stephen Warren as he was thinking about this stuff in
the context of the nVidia drivers.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120316/e687354c/attachment.sig>

[PATCH v3 09/11] ASoC: fsl: remove the fatal error checking on codec-handle

[Mark Brown]

On Thu, Mar 08, 2012 at 02:50:21PM -0600, Timur Tabi wrote:
> Shawn Guo wrote:

> > More importantly, the imx machine drivers to be added working with
> > fsl_ssi will not have this property, as they use new ASoC machine
> > driver binding mechanism.

> I want to see that binding documented in Documentation/devicetree/bindings
> first.

That binding will generally be per machine driver.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120309/942ac0dc/attachment-0001.sig>

[PATCH v3 10/11] ASoC: fsl: let fsl_ssi work with imx pcm and machine drivers

[Shawn Guo]

Makes necessary changes on fsl_ssi to let it work with imx pcm and
machine drivers.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/fsl/fsl_ssi.c |   67 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 67 insertions(+), 0 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index e5903cb..c251ee9 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -29,6 +29,7 @@
 #include <sound/soc.h>
 
 #include "fsl_ssi.h"
+#include "imx-pcm.h"
 
 #ifdef PPC
 #define read_ssi(addr)			 in_be32(addr)
@@ -119,6 +120,11 @@ struct fsl_ssi_private {
 	struct device_attribute dev_attr;
 	struct platform_device *pdev;
 
+	int ssi_on_imx;
+	struct platform_device *imx_pcm_pdev;
+	struct imx_pcm_dma_params dma_params_tx;
+	struct imx_pcm_dma_params dma_params_rx;
+
 	struct {
 		unsigned int rfrc;
 		unsigned int tfrc;
@@ -416,6 +422,12 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
 		ssi_private->second_stream = substream;
 	}
 
+	if (ssi_private->ssi_on_imx)
+		snd_soc_dai_set_dma_data(dai, substream,
+			(substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
+				&ssi_private->dma_params_tx :
+				&ssi_private->dma_params_rx);
+
 	return 0;
 }
 
@@ -709,6 +721,38 @@ static int __devinit fsl_ssi_probe(struct platform_device *pdev)
                 /* Older 8610 DTs didn't have the fifo-depth property */
 		ssi_private->fifo_depth = 8;
 
+	if (of_device_is_compatible(pdev->dev.of_node, "fsl,imx1-ssi")) {
+		u32 dma_events[2];
+		ssi_private->ssi_on_imx = 1;
+		/*
+		 * FIXME: The dma burst size must equal to ssi watermark
+		 * setting on imx.  We have burstsize be "fifo_depth - 2"
+		 * here because "fifo_depth - 2" rather than fifo_depth is
+		 * being written into watermark register in fsl_ssi_startup().
+		 * Is this something needs to be fixed for PowerPC?
+		 */
+		ssi_private->dma_params_tx.burstsize =
+			ssi_private->fifo_depth - 2;
+		ssi_private->dma_params_rx.burstsize =
+			ssi_private->fifo_depth - 2;
+		ssi_private->dma_params_tx.dma_addr =
+			ssi_private->ssi_phys + offsetof(struct ccsr_ssi, stx0);
+		ssi_private->dma_params_rx.dma_addr =
+			ssi_private->ssi_phys + offsetof(struct ccsr_ssi, srx0);
+		/*
+		 * TODO: This is a temporary solution and should be changed
+		 * to use generic DMA binding later when the helplers get in.
+		 */
+		ret = of_property_read_u32_array(pdev->dev.of_node,
+					"fsl,ssi-dma-events", dma_events, 2);
+		if (ret) {
+			dev_err(&pdev->dev, "could not get dma events\n");
+			goto error_irq;
+		}
+		ssi_private->dma_params_tx.dma = dma_events[0];
+		ssi_private->dma_params_rx.dma = dma_events[1];
+	}
+
 	/* Initialize the the device_attribute structure */
 	dev_attr = &ssi_private->dev_attr;
 	sysfs_attr_init(&dev_attr->attr);
@@ -732,6 +776,24 @@ static int __devinit fsl_ssi_probe(struct platform_device *pdev)
 		goto error_dev;
 	}
 
+	/*
+	 * In case of imx, the machine driver uses new binding which does
+	 * not require SSI driver to trigger machine driver's probe, but
+	 * the pcm device needs to be registered here.
+	 */
+	if (ssi_private->ssi_on_imx) {
+		ssi_private->imx_pcm_pdev =
+			platform_device_register_simple("imx-pcm-audio",
+							-1, NULL, 0);
+		if (IS_ERR(ssi_private->imx_pcm_pdev)) {
+			ret = PTR_ERR(ssi_private->imx_pcm_pdev);
+			goto error_dev;
+		} else {
+			/* success for imx */
+			return 0;
+		}
+	}
+
 	/* Trigger the machine driver's probe function.  The platform driver
 	 * name of the machine driver is taken from /compatible property of the
 	 * device tree.  We also pass the address of the CPU DAI driver
@@ -757,6 +819,8 @@ static int __devinit fsl_ssi_probe(struct platform_device *pdev)
 
 error_dai:
 	snd_soc_unregister_dai(&pdev->dev);
+	if (ssi_private->ssi_on_imx)
+		platform_device_unregister(ssi_private->imx_pcm_pdev);
 
 error_dev:
 	dev_set_drvdata(&pdev->dev, NULL);
@@ -783,6 +847,8 @@ static int fsl_ssi_remove(struct platform_device *pdev)
 
 	platform_device_unregister(ssi_private->pdev);
 	snd_soc_unregister_dai(&pdev->dev);
+	if (ssi_private->ssi_on_imx)
+		platform_device_unregister(ssi_private->imx_pcm_pdev);
 	device_remove_file(&pdev->dev, &ssi_private->dev_attr);
 
 	free_irq(ssi_private->irq, ssi_private);
@@ -796,6 +862,7 @@ static int fsl_ssi_remove(struct platform_device *pdev)
 
 static const struct of_device_id fsl_ssi_ids[] = {
 	{ .compatible = "fsl,mpc8610-ssi", },
+	{ .compatible = "fsl,imx1-ssi", },
 	{}
 };
 MODULE_DEVICE_TABLE(of, fsl_ssi_ids);
-- 
1.7.5.4

[PATCH v3 10/11] ASoC: fsl: let fsl_ssi work with imx pcm and machine drivers

[Sascha Hauer]

On Fri, Mar 09, 2012 at 12:59:49AM +0800, Shawn Guo wrote:
> Makes necessary changes on fsl_ssi to let it work with imx pcm and
> machine drivers.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  sound/soc/fsl/fsl_ssi.c |   67 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 67 insertions(+), 0 deletions(-)
> 
> diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
> index e5903cb..c251ee9 100644
> --- a/sound/soc/fsl/fsl_ssi.c
> +++ b/sound/soc/fsl/fsl_ssi.c
> @@ -29,6 +29,7 @@
>  #include <sound/soc.h>
>  
>  #include "fsl_ssi.h"
> +#include "imx-pcm.h"
>  
>  #ifdef PPC
>  #define read_ssi(addr)			 in_be32(addr)
> @@ -119,6 +120,11 @@ struct fsl_ssi_private {
>  	struct device_attribute dev_attr;
>  	struct platform_device *pdev;
>  
> +	int ssi_on_imx;
> +	struct platform_device *imx_pcm_pdev;
> +	struct imx_pcm_dma_params dma_params_tx;
> +	struct imx_pcm_dma_params dma_params_rx;
> +
>  	struct {
>  		unsigned int rfrc;
>  		unsigned int tfrc;
> @@ -416,6 +422,12 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
>  		ssi_private->second_stream = substream;
>  	}
>  
> +	if (ssi_private->ssi_on_imx)
> +		snd_soc_dai_set_dma_data(dai, substream,
> +			(substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
> +				&ssi_private->dma_params_tx :
> +				&ssi_private->dma_params_rx);
> +
>  	return 0;
>  }
>  
> @@ -709,6 +721,38 @@ static int __devinit fsl_ssi_probe(struct platform_device *pdev)
>                  /* Older 8610 DTs didn't have the fifo-depth property */
>  		ssi_private->fifo_depth = 8;
>  
> +	if (of_device_is_compatible(pdev->dev.of_node, "fsl,imx1-ssi")) {
> +		u32 dma_events[2];
> +		ssi_private->ssi_on_imx = 1;
> +		/*
> +		 * FIXME: The dma burst size must equal to ssi watermark
> +		 * setting on imx.  We have burstsize be "fifo_depth - 2"
> +		 * here because "fifo_depth - 2" rather than fifo_depth is
> +		 * being written into watermark register in fsl_ssi_startup().
> +		 * Is this something needs to be fixed for PowerPC?
> +		 */
> +		ssi_private->dma_params_tx.burstsize =
> +			ssi_private->fifo_depth - 2;
> +		ssi_private->dma_params_rx.burstsize =
> +			ssi_private->fifo_depth - 2;
> +		ssi_private->dma_params_tx.dma_addr =
> +			ssi_private->ssi_phys + offsetof(struct ccsr_ssi, stx0);
> +		ssi_private->dma_params_rx.dma_addr =
> +			ssi_private->ssi_phys + offsetof(struct ccsr_ssi, srx0);
> +		/*
> +		 * TODO: This is a temporary solution and should be changed
> +		 * to use generic DMA binding later when the helplers get in.
> +		 */
> +		ret = of_property_read_u32_array(pdev->dev.of_node,
> +					"fsl,ssi-dma-events", dma_events, 2);
> +		if (ret) {
> +			dev_err(&pdev->dev, "could not get dma events\n");
> +			goto error_irq;
> +		}
> +		ssi_private->dma_params_tx.dma = dma_events[0];
> +		ssi_private->dma_params_rx.dma = dma_events[1];
> +	}
> +
>  	/* Initialize the the device_attribute structure */
>  	dev_attr = &ssi_private->dev_attr;
>  	sysfs_attr_init(&dev_attr->attr);
> @@ -732,6 +776,24 @@ static int __devinit fsl_ssi_probe(struct platform_device *pdev)
>  		goto error_dev;
>  	}
>  
> +	/*
> +	 * In case of imx, the machine driver uses new binding which does
> +	 * not require SSI driver to trigger machine driver's probe, but
> +	 * the pcm device needs to be registered here.
> +	 */
> +	if (ssi_private->ssi_on_imx) {
> +		ssi_private->imx_pcm_pdev =
> +			platform_device_register_simple("imx-pcm-audio",
> +							-1, NULL, 0);
> +		if (IS_ERR(ssi_private->imx_pcm_pdev)) {
> +			ret = PTR_ERR(ssi_private->imx_pcm_pdev);
> +			goto error_dev;
> +		} else {
> +			/* success for imx */
> +			return 0;
> +		}
> +	}
> +
>  	/* Trigger the machine driver's probe function.  The platform driver
>  	 * name of the machine driver is taken from /compatible property of the
>  	 * device tree.  We also pass the address of the CPU DAI driver
> @@ -757,6 +819,8 @@ static int __devinit fsl_ssi_probe(struct platform_device *pdev)
>  
>  error_dai:
>  	snd_soc_unregister_dai(&pdev->dev);
> +	if (ssi_private->ssi_on_imx)
> +		platform_device_unregister(ssi_private->imx_pcm_pdev);
>  
>  error_dev:
>  	dev_set_drvdata(&pdev->dev, NULL);
> @@ -783,6 +847,8 @@ static int fsl_ssi_remove(struct platform_device *pdev)
>  
>  	platform_device_unregister(ssi_private->pdev);
>  	snd_soc_unregister_dai(&pdev->dev);
> +	if (ssi_private->ssi_on_imx)
> +		platform_device_unregister(ssi_private->imx_pcm_pdev);
>  	device_remove_file(&pdev->dev, &ssi_private->dev_attr);
>  
>  	free_irq(ssi_private->irq, ssi_private);
> @@ -796,6 +862,7 @@ static int fsl_ssi_remove(struct platform_device *pdev)
>  
>  static const struct of_device_id fsl_ssi_ids[] = {
>  	{ .compatible = "fsl,mpc8610-ssi", },
> +	{ .compatible = "fsl,imx1-ssi", },

imx1 seems wrong here. The register layout has changed significantly between
i.MX1 and i.MX21.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v3 10/11] ASoC: fsl: let fsl_ssi work with imx pcm and machine drivers

[Shawn Guo]

On Thu, Mar 08, 2012 at 08:15:26PM +0100, Sascha Hauer wrote:
...
> > @@ -796,6 +862,7 @@ static int fsl_ssi_remove(struct platform_device *pdev)
> >  
> >  static const struct of_device_id fsl_ssi_ids[] = {
> >  	{ .compatible = "fsl,mpc8610-ssi", },
> > +	{ .compatible = "fsl,imx1-ssi", },
> 
> imx1 seems wrong here. The register layout has changed significantly between
> i.MX1 and i.MX21.
> 
Yeah, that's right.  You indeed know i.MX1 much better than me.  Will
change to:

  .compatible = "fsl,imx21-ssi"

-- 
Regards,
Shawn

[PATCH v3 10/11] ASoC: fsl: let fsl_ssi work with imx pcm and machine drivers

[Timur Tabi]

Shawn Guo wrote:

> +	if (of_device_is_compatible(pdev->dev.of_node, "fsl,imx1-ssi")) {

I wonder if the i.MX code should be protected by an #ifdef.  There's no
point in compiling this code on PowerPC.

> +		/*
> +		 * FIXME: The dma burst size must equal to ssi watermark
> +		 * setting on imx.  We have burstsize be "fifo_depth - 2"
> +		 * here because "fifo_depth - 2" rather than fifo_depth is
> +		 * being written into watermark register in fsl_ssi_startup().
> +		 * Is this something needs to be fixed for PowerPC?

Setting the watermark to fifo_depth-2 allows the SSI to continue storing
data into the FIFO (during capture) in the case the DMA engine isn't fast
enough.  I don't want the FIFO to be completely full before we start the DMA.

For playback, it's the same thing.  I can't be sure the FIFO is completely
empty when it's time to DMA more data.

> +		 */
> +		ssi_private->dma_params_tx.burstsize =
> +			ssi_private->fifo_depth - 2;
> +		ssi_private->dma_params_rx.burstsize =
> +			ssi_private->fifo_depth - 2;
> +		ssi_private->dma_params_tx.dma_addr =
> +			ssi_private->ssi_phys + offsetof(struct ccsr_ssi, stx0);
> +		ssi_private->dma_params_rx.dma_addr =
> +			ssi_private->ssi_phys + offsetof(struct ccsr_ssi, srx0);

So I'm a little confused by this new binding.  I went through a lot of
work to remove DMA-specific data from the SSI driver, and here you are
adding a bunch of it back.  What you're doing here -- I'm doing it in the
DMA driver where it belongs.

> +		/*
> +		 * TODO: This is a temporary solution and should be changed
> +		 * to use generic DMA binding later when the helplers get in.
> +		 */
> +		ret = of_property_read_u32_array(pdev->dev.of_node,
> +					"fsl,ssi-dma-events", dma_events, 2);
> +		if (ret) {
> +			dev_err(&pdev->dev, "could not get dma events\n");
> +			goto error_irq;
> +		}
> +		ssi_private->dma_params_tx.dma = dma_events[0];
> +		ssi_private->dma_params_rx.dma = dma_events[1];

of_property_read_u32_array seems overkill for just two integers.

ssi_private->dma_params_tx.dma = be32_to_cpu(prop[0]);
ssi_private->dma_params_rx.dma = be32_to_cpu(prop[1]);

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 10/11] ASoC: fsl: let fsl_ssi work with imx pcm and machine drivers

[Shawn Guo]

On Thu, Mar 08, 2012 at 02:45:29PM -0600, Timur Tabi wrote:
> Shawn Guo wrote:
> 
> > +	if (of_device_is_compatible(pdev->dev.of_node, "fsl,imx1-ssi")) {
> 
> I wonder if the i.MX code should be protected by an #ifdef.  There's no
> point in compiling this code on PowerPC.
> 
I'm not really fond of #ifdef.  The i.MX code is not that much after
all.

> > +		/*
> > +		 * FIXME: The dma burst size must equal to ssi watermark
> > +		 * setting on imx.  We have burstsize be "fifo_depth - 2"
> > +		 * here because "fifo_depth - 2" rather than fifo_depth is
> > +		 * being written into watermark register in fsl_ssi_startup().
> > +		 * Is this something needs to be fixed for PowerPC?
> 
> Setting the watermark to fifo_depth-2 allows the SSI to continue storing
> data into the FIFO (during capture) in the case the DMA engine isn't fast
> enough.  I don't want the FIFO to be completely full before we start the DMA.
> 
> For playback, it's the same thing.  I can't be sure the FIFO is completely
> empty when it's time to DMA more data.
> 
Ok, got you.  We used to set fifo_depth on imx-ssi not the value given
by hardware manual, but the one already tuned.

So all we need is to encode the hardware value in device tree property
"fsl,fifo-depth", and it just works.  I will remove this "FIXME". 

> > +		 */
> > +		ssi_private->dma_params_tx.burstsize =
> > +			ssi_private->fifo_depth - 2;
> > +		ssi_private->dma_params_rx.burstsize =
> > +			ssi_private->fifo_depth - 2;
> > +		ssi_private->dma_params_tx.dma_addr =
> > +			ssi_private->ssi_phys + offsetof(struct ccsr_ssi, stx0);
> > +		ssi_private->dma_params_rx.dma_addr =
> > +			ssi_private->ssi_phys + offsetof(struct ccsr_ssi, srx0);
> 
> So I'm a little confused by this new binding.  I went through a lot of
> work to remove DMA-specific data from the SSI driver, and here you are
> adding a bunch of it back.  What you're doing here -- I'm doing it in the
> DMA driver where it belongs.

I'm not sure these all belong to DMA driver.  These are all
configurations/parameters of SSI, which are related to DMA though.
I see fsl_dma are handling these, but I do not fully agree with that,
because we end up with fsl_dma driver accessing SSI node and
"struct ccsr_ssi" which should be SSI private data.

Secondly, this is the way that how imx-ssi and imx-pcm-dma works and
interacts.  If we want to move these stuff into imx-pcm-dma, we need
a good story to convince imx-ssi users that why it's better than what
they currently do.

> 
> > +		/*
> > +		 * TODO: This is a temporary solution and should be changed
> > +		 * to use generic DMA binding later when the helplers get in.
> > +		 */
> > +		ret = of_property_read_u32_array(pdev->dev.of_node,
> > +					"fsl,ssi-dma-events", dma_events, 2);
> > +		if (ret) {
> > +			dev_err(&pdev->dev, "could not get dma events\n");
> > +			goto error_irq;
> > +		}
> > +		ssi_private->dma_params_tx.dma = dma_events[0];
> > +		ssi_private->dma_params_rx.dma = dma_events[1];
> 
> of_property_read_u32_array seems overkill for just two integers.
> 
Hmm, I do not think so.

Device tree maintainers created helpers of_property_read_u32 and
of_property_read_u32_array to ease the user and make the operation
less error prone.  Looking at the of_property_read_u32 implementation,
it's even calling into of_property_read_u32_array.  So I do not think
it's overkill for reading two integers in any way.

static inline int of_property_read_u32(const struct device_node *np,
                                       const char *propname,
                                       u32 *out_value)
{
        return of_property_read_u32_array(np, propname, out_value, 1);
}

> ssi_private->dma_params_tx.dma = be32_to_cpu(prop[0]);
> ssi_private->dma_params_rx.dma = be32_to_cpu(prop[1]);
> 

-- 
Regards,
Shawn

[PATCH v3 10/11] ASoC: fsl: let fsl_ssi work with imx pcm and machine drivers

[Tabi Timur-B04825]

Shawn Guo wrote:

> Ok, got you.  We used to set fifo_depth on imx-ssi not the value given
> by hardware manual, but the one already tuned.

What do you mean by "already tuned".  Tuned by whom?

>>> +		 */
>>> +		ssi_private->dma_params_tx.burstsize =
>>> +			ssi_private->fifo_depth - 2;
>>> +		ssi_private->dma_params_rx.burstsize =
>>> +			ssi_private->fifo_depth - 2;
>>> +		ssi_private->dma_params_tx.dma_addr =
>>> +			ssi_private->ssi_phys + offsetof(struct ccsr_ssi, stx0);
>>> +		ssi_private->dma_params_rx.dma_addr =
>>> +			ssi_private->ssi_phys + offsetof(struct ccsr_ssi, srx0);
>>
>> So I'm a little confused by this new binding.  I went through a lot of
>> work to remove DMA-specific data from the SSI driver, and here you are
>> aFrom arnd at arndb.de  Mon Mar 12 12:43:25 2012
From: arnd@arndb.de (Arnd Bergmann)
Date: Mon, 12 Mar 2012 16:43:25 +0000
Subject: [PATCH 1/2] ARM: kirkwood: Basic support for DNS-320 and DNS-325
In-Reply-To: <20120312161004.GB5050@titan.lakedaemon.net>
References: <1331476406-9844-1-git-send-email-jm@lentin.co.uk>
 <201203120811.23137.arnd@arndb.de>
 <20120312161004.GB5050@titan.lakedaemon.net>
Message-ID: <201203121643.26133.arnd@arndb.de>

On Monday 12 March 2012, Jason Cooper wrote:
> Okay, that's what smelled funny.  wdt and intc both only have registers
> in virtual address space defined.  Can I assume they map like everything
> else to corresponding physical addresses?
> 
> eg wdt at fed20300 -> wdt at f1020300 ?

Yes, correct. Or just uses 20300 as the address if you remap everything
using the ranges property.

	Arnd

[PATCH v3 10/11] ASoC: fsl: let fsl_ssi work with imx pcm and machine drivers

[Shawn Guo]

On Fri, Mar 09, 2012 at 04:02:40AM +0000, Tabi Timur-B04825 wrote:
> Shawn Guo wrote:
> 
> > Ok, got you.  We used to set fifo_depth on imx-ssi not the value given
> > by hardware manual, but the one already tuned.
> 
> What do you mean by "already tuned".  Tuned by whom?
> 
For example, we have SSI fifo depth 15 on i.MX51 design.  Instead of
having fifo_depth as 15 and then writing 15 - 2 into burstsize and
watermark, we used to have fifo_depth as 13, and then burstsize and
watermark can just be fifo_depth.

I agree that fsl_ssi does the right thing, having fifo_depth be the
hardware value.

> >>> +		 */
> >>> +		ssi_private->dma_params_tx.burstsize =
> >>> +			ssi_private->fifo_depth - 2;
> >>> +		ssi_private->dma_params_rx.burstsize =
> >>> +			ssi_private->fifo_depth - 2;
> >>> +		ssi_private->dma_params_tx.dma_addr =
> >>> +			ssi_private->ssi_phys + offsetof(struct ccsr_ssi, stx0);
> >>> +		ssi_private->dma_params_rx.dma_addr =
> >>> +			ssi_private->ssi_phys + offsetof(struct ccsr_ssi, srx0);
> >>
> >> So I'm a little confused by this new binding.  I went through a lot of
> >> work to remove DMA-specific data from the SSI driver, and here you are
> >> adding a bunch of it back.  What you're doing here -- I'm doing it in the
> >> DMA driver where it belongs.
> >
> > I'm not sure these all belong to DMA driver.  These are all
> > configurations/parameters of SSI, which are related to DMA though.
> > I see fsl_dma are handling these, but I do not fully agree with that,
> > because we end up with fsl_dma driver accessing SSI node and
> > "struct ccsr_ssi" which should be SSI private data.
> 
> Well, the SSI needs to tell the DMA driver how to program itself.  On 
> PowerPC, the DMA's burst size is based on the SSI's FIFO depth.  You're 
> effectively doing what I already do, except you do more work in the SSI 
> driver.
> 
> I understand that we need to support old and new bindings, but I already 
> created an infrastructure that supports passing SSI information to the DMA 
> driver.

I do not see an infrastructure, but that fsl_dma driver accesses SSI
private data structure.  Instead, snd_soc_dai_set[get]_dma_data looks
more like an infrastructure to me.

> It seems that the above code is different only for the sake of 
> being different, not because it's better.
> 
> This is not really an i.MX vs. PowerPC issue, which is why I'm not sure 
> this change belong in this patchset.
> 
Without this change, fsl_ssi can not work with imx-pcm-dma.  I do not
understand how it can possibly not belong in this patchset?

> > Secondly, this is the way that how imx-ssi and imx-pcm-dma works and
> > interacts.  If we want to move these stuff into imx-pcm-dma, we need
> > a good story to convince imx-ssi users that why it's better than what
> > they currently do.
> 
> I don't understand that.  You're already rewriting these drivers.  Who 
> else needs to be convinced?
> 
As I told before, even now we have fsl_ssi working for DT based IMX
platforms, imx5 and imx6, imx-ssi will still exist for those non-DT
IMX, like imx21, imx25, imx27, imx31, imx35.  That said, imx-pcm-dma
will need to work with both fsl_ssi and imx-ssi.  Moving those stuff
into imx-pcm-dma will require significant changes on imx-ssi too.

-- 
Regards,
Shawn

[PATCH v3 11/11] ASoC: fsl: add imx-sgtl5000 machine driver

[Shawn Guo]

This is the initial imx-sgtl5000 machine driver support.  It's a device
tree only machine driver working with fsl_ssi driver.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 .../bindings/sound/imx-audio-sgtl5000.txt          |   24 +++
 sound/soc/fsl/Kconfig                              |   12 ++
 sound/soc/fsl/Makefile                             |    2 +
 sound/soc/fsl/imx-sgtl5000.c                       |  177 ++++++++++++++++++++
 4 files changed, 215 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/imx-audio-sgtl5000.txt
 create mode 100644 sound/soc/fsl/imx-sgtl5000.c

diff --git a/Documentation/devicetree/bindings/sound/imx-audio-sgtl5000.txt b/Documentation/devicetree/bindings/sound/imx-audio-sgtl5000.txt
new file mode 100644
index 0000000..421a374
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/imx-audio-sgtl5000.txt
@@ -0,0 +1,24 @@
+Freescale i.MX audio complex with SGTL5000 codec
+
+Required properties:
+- compatible : "fsl,imx-audio-sgtl5000"
+- model : The user-visible name of this sound complex
+- ssi-controller : The phandle of the i.MX SSI controller
+- audio-codec : The phandle of the SGTL5000 audio codec
+- mux-int-port : The internal port of the i.MX audio muxer (AUDMUX)
+- mux-ext-port : The external port of the i.MX audio muxer
+
+Note: The AUDMUX port numbering should start at 1, which is consistent with
+hardware manual.
+
+Example:
+
+sound {
+	compatible = "fsl,imx51-babbage-sgtl5000",
+		     "fsl,imx-audio-sgtl5000";
+	model = "imx51-babbage-sgtl5000";
+	ssi-controller = <&ssi1>;
+	audio-codec = <&sgtl5000>;
+	mux-int-port = <1>;
+	mux-ext-port = <3>;
+};
diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index 535ee73..aa14fc9 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -164,4 +164,16 @@ config SND_SOC_EUKREA_TLV320
 	  Enable I2S based access to the TLV320AIC23B codec attached
 	  to the SSI interface
 
+config SND_SOC_IMX_SGTL5000
+	tristate "SoC Audio support for i.MX boards with sgtl5000"
+	depends on OF && I2C
+	select SND_SOC_SGTL5000
+	select SND_SOC_IMX_PCM_DMA
+	select SND_SOC_IMX_AUDMUX
+	select SND_SOC_FSL_SSI
+	select SND_SOC_FSL_UTILS
+	help
+	  Say Y if you want to add support for SoC audio on an i.MX board with
+	  a sgtl5000 codec.
+
 endif # SND_IMX_SOC
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
index fbdbed0..638d385 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -40,8 +40,10 @@ snd-soc-eukrea-tlv320-objs := eukrea-tlv320.o
 snd-soc-phycore-ac97-objs := phycore-ac97.o
 snd-soc-mx27vis-aic32x4-objs := mx27vis-aic32x4.o
 snd-soc-wm1133-ev1-objs := wm1133-ev1.o
+snd-soc-imx-sgtl5000-objs := imx-sgtl5000.o
 
 obj-$(CONFIG_SND_SOC_EUKREA_TLV320) += snd-soc-eukrea-tlv320.o
 obj-$(CONFIG_SND_SOC_PHYCORE_AC97) += snd-soc-phycore-ac97.o
 obj-$(CONFIG_SND_SOC_MX27VIS_AIC32X4) += snd-soc-mx27vis-aic32x4.o
 obj-$(CONFIG_SND_MXC_SOC_WM1133_EV1) += snd-soc-wm1133-ev1.o
+obj-$(CONFIG_SND_SOC_IMX_SGTL5000) += snd-soc-imx-sgtl5000.o
diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c
new file mode 100644
index 0000000..3786b61
--- /dev/null
+++ b/sound/soc/fsl/imx-sgtl5000.c
@@ -0,0 +1,177 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ * Copyright 2012 Linaro Ltd.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <sound/soc.h>
+
+#include "../codecs/sgtl5000.h"
+#include "imx-audmux.h"
+
+#define DAI_NAME_SIZE	32
+
+struct imx_sgtl5000_data {
+	struct snd_soc_dai_link dai;
+	struct snd_soc_card card;
+	char codec_dai_name[DAI_NAME_SIZE];
+	char platform_name[DAI_NAME_SIZE];
+	unsigned int clk_frequency;
+};
+
+static int imx_sgtl5000_dai_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct imx_sgtl5000_data *data = container_of(rtd->card,
+					struct imx_sgtl5000_data, card);
+	struct device *dev = rtd->card->dev;
+	int ret;
+
+	ret = snd_soc_dai_set_sysclk(rtd->codec_dai, SGTL5000_SYSCLK,
+				     data->clk_frequency, SND_SOC_CLOCK_IN);
+	if (ret) {
+		dev_err(dev, "could not set codec driver clock params\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *ssi_np, *codec_np;
+	struct platform_device *ssi_pdev;
+	struct imx_sgtl5000_data *data;
+	int int_port, ext_port;
+	int ret;
+
+	ret = of_property_read_u32(np, "mux-int-port", &int_port);
+	if (ret) {
+		dev_err(&pdev->dev, "mux-int-port missing or invalid\n");
+		return ret;
+	}
+	ret = of_property_read_u32(np, "mux-ext-port", &ext_port);
+	if (ret) {
+		dev_err(&pdev->dev, "mux-ext-port missing or invalid\n");
+		return ret;
+	}
+
+	/*
+	 * The port numbering in the hardware manual starts at 1, while
+	 * the audmux API expects it starts at 0.
+	 */
+	int_port--;
+	ext_port--;
+	ret = imx_audmux_v2_configure_port(int_port,
+			IMX_AUDMUX_V2_PTCR_SYN |
+			IMX_AUDMUX_V2_PTCR_TFSEL(ext_port) |
+			IMX_AUDMUX_V2_PTCR_TCSEL(ext_port) |
+			IMX_AUDMUX_V2_PTCR_TFSDIR |
+			IMX_AUDMUX_V2_PTCR_TCLKDIR,
+			IMX_AUDMUX_V2_PDCR_RXDSEL(ext_port));
+	if (ret) {
+		dev_err(&pdev->dev, "audmux internal port setup failed\n");
+		return ret;
+	}
+	imx_audmux_v2_configure_port(ext_port,
+			IMX_AUDMUX_V2_PTCR_SYN |
+			IMX_AUDMUX_V2_PTCR_TCSEL(int_port),
+			IMX_AUDMUX_V2_PDCR_RXDSEL(int_port));
+	if (ret) {
+		dev_err(&pdev->dev, "audmux external port setup failed\n");
+		return ret;
+	}
+
+	ssi_np = of_parse_phandle(pdev->dev.of_node, "ssi-controller", 0);
+	codec_np = of_parse_phandle(pdev->dev.of_node, "audio-codec", 0);
+	if (!ssi_np || !codec_np) {
+		dev_err(&pdev->dev, "phandle missing or invalid\n");
+		return -EINVAL;
+	}
+
+	ssi_pdev = of_find_device_by_node(ssi_np);
+	if (!ssi_pdev) {
+		dev_err(&pdev->dev, "failed to find SSI platform device\n");
+		return -EINVAL;
+	}
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	ret = of_property_read_u32(codec_np, "clock-frequency",
+				   &data->clk_frequency);
+	if (ret) {
+		dev_err(&pdev->dev, "clock-frequency missing or invalid\n");
+		return ret;
+	}
+
+	data->dai.name = "HiFi";
+	data->dai.stream_name = "HiFi";
+	data->dai.codec_dai_name = "sgtl5000";
+	data->dai.codec_of_node = codec_np;
+	data->dai.cpu_dai_name = dev_name(&ssi_pdev->dev);
+	data->dai.platform_name = "imx-pcm-audio";
+	data->dai.init = &imx_sgtl5000_dai_init;
+	data->dai.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+			    SND_SOC_DAIFMT_CBM_CFM;
+
+	data->card.dev = &pdev->dev;
+	ret = snd_soc_of_parse_card_name(&data->card, "model");
+	if (ret)
+		return ret;
+	data->card.num_links = 1;
+	data->card.dai_link = &data->dai;
+
+	ret = snd_soc_register_card(&data->card);
+	if (ret) {
+		dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, data);
+	of_node_put(ssi_np);
+	of_node_put(codec_np);
+
+	return 0;
+}
+
+static int __devexit imx_sgtl5000_remove(struct platform_device *pdev)
+{
+	struct imx_sgtl5000_data *data = platform_get_drvdata(pdev);
+
+	snd_soc_unregister_card(&data->card);
+
+	return 0;
+}
+
+static const struct of_device_id imx_sgtl5000_dt_ids[] = {
+	{ .compatible = "fsl,imx-audio-sgtl5000", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx_sgtl5000_dt_ids);
+
+static struct platform_driver imx_sgtl5000_driver = {
+	.driver = {
+		.name = "imx-sgtl5000",
+		.owner = THIS_MODULE,
+		.of_match_table = imx_sgtl5000_dt_ids,
+	},
+	.probe = imx_sgtl5000_probe,
+	.remove = __devexit_p(imx_sgtl5000_remove),
+};
+module_platform_driver(imx_sgtl5000_driver);
+
+MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");
+MODULE_DESCRIPTION("Freescale i.MX SGTL5000 ASoC machine driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:imx-sgtl5000");
-- 
1.7.5.4

[PATCH v3 00/11] ASoC: add imx-sgtl5000 machine driver working with fsl_ssi

[Timur Tabi]

Shawn Guo wrote:
> I did not pick up you Ack-by tag, because there are some changes since
> v2 you will need to take another look and have the testing, I guess.

Unfortunately, although it works on the MPC8610, it's broken on the P1022:

# speaker-test -t sine -c 2 -r 44100


speaker-test 1.0.11rc2


Playback device is default
Stream parameters are 44100Hz, S16_BE, 2 channels
Sine wave rate is 440.0000Hz
Rate set to 44100Hz (requested 44100Hz)
Buffer size range from 256 to 32768
Period size range from 128 to 16384
Periods = 4
Buffer time size 575
To choose buffer_size = 32768
To choose period_size = 8192
was set period_size = 8192
was set buffer_size = 32768

At this point, nothing else happens, there's no audio, and even Ctrl-C
doesn't work.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 00/11] ASoC: add imx-sgtl5000 machine driver working with fsl_ssi

[Shawn Guo]

On Thu, Mar 08, 2012 at 02:05:02PM -0600, Timur Tabi wrote:
> Shawn Guo wrote:
> > I did not pick up you Ack-by tag, because there are some changes since
> > v2 you will need to take another look and have the testing, I guess.
> 
> Unfortunately, although it works on the MPC8610, it's broken on the P1022:
> 
> # speaker-test -t sine -c 2 -r 44100
> 
> 
> speaker-test 1.0.11rc2
> 
> 
> Playback device is default
> Stream parameters are 44100Hz, S16_BE, 2 channels
> Sine wave rate is 440.0000Hz
> Rate set to 44100Hz (requested 44100Hz)
> Buffer size range from 256 to 32768
> Period size range from 128 to 16384
> Periods = 4
> Buffer time size 575
> To choose buffer_size = 32768
> To choose period_size = 8192
> was set period_size = 8192
> was set buffer_size = 32768
> 
> At this point, nothing else happens, there's no audio, and even Ctrl-C
> doesn't work.
> 
Are you testing the patch set on top of the SHA below, which I
mentioned in the cover letter?

  3030763 (Merge tag 'asoc-3.4' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into topic/asoc)

I see the similar problem on the following SHA with my testing on
imx-sgtl5000.

  1a8feca (Merge branch 'for-3.4' into asoc-next)

-- 
Regards,
Shawn

[PATCH v3 00/11] ASoC: add imx-sgtl5000 machine driver working with fsl_ssi

[Tabi Timur-B04825]

Shawn Guo wrote:
> Are you testing the patch set on top of the SHA below, which I
> mentioned in the cover letter?
>
>    3030763 (Merge tag 'asoc-3.4' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into topic/asoc)
>
> I see the similar problem on the following SHA with my testing on
> imx-sgtl5000.
>
>    1a8feca (Merge branch 'for-3.4' into asoc-next)

It appears the problem is with for-3.4 itself.  I'm running git-bisect now 
to narrow it down.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH v3 00/11] ASoC: add imx-sgtl5000 machine driver working with fsl_ssi

[Shawn Guo]

On Fri, Mar 09, 2012 at 02:11:41AM +0000, Tabi Timur-B04825 wrote:
> Shawn Guo wrote:
> > Are you testing the patch set on top of the SHA below, which I
> > mentioned in the cover letter?
> >
> >    3030763 (Merge tag 'asoc-3.4' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into topic/asoc)
> >
> > I see the similar problem on the following SHA with my testing on
> > imx-sgtl5000.
> >
> >    1a8feca (Merge branch 'for-3.4' into asoc-next)
> 
> It appears the problem is with for-3.4 itself.  I'm running git-bisect now 
> to narrow it down.
> 
My bisect tells that the offending commit is:

  96acc35 (ASoC: DAPM: Make sure DAPM widget IO ops hold the component mutex)

But I do not understand why yet.

-- 
Regards,
Shawn

[PATCH v3 00/11] ASoC: add imx-sgtl5000 machine driver working with fsl_ssi

[Shawn Guo]

On 9 March 2012 15:13, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Fri, Mar 09, 2012 at 02:11:41AM +0000, Tabi Timur-B04825 wrote:
>> Shawn Guo wrote:
>> > Are you testing the patch set on top of the SHA below, which I
>> > mentioned in the cover letter?
>> >
>> > ? ?3030763 (Merge tag 'asoc-3.4' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into topic/asoc)
>> >
>> > I see the similar problem on the following SHA with my testing on
>> > imx-sgtl5000.
>> >
>> > ? ?1a8feca (Merge branch 'for-3.4' into asoc-next)
>>
>> It appears the problem is with for-3.4 itself. ?I'm running git-bisect now
>> to narrow it down.
>>
> My bisect tells that the offending commit is:
>
> ?96acc35 (ASoC: DAPM: Make sure DAPM widget IO ops hold the component mutex)
>
> But I do not understand why yet.

=============================================
[ INFO: possible recursive locking detected ]
3.3.0-rc4+ #159 Not tainted
---------------------------------------------
aplay/395 is trying to acquire lock:
 (&codec->mutex){+.+...}, at: [<802f7414>] soc_widget_update_bits_locked+0x88/0x
1a0

but task is already holding lock:
 (&codec->mutex){+.+...}, at: [<802f9df4>] snd_soc_dapm_stream_event+0x38/0xc0

[PATCH v3 00/11] ASoC: add imx-sgtl5000 machine driver working with fsl_ssi

[Mark Brown]

On Fri, Mar 09, 2012 at 03:28:22PM +0800, Shawn Guo wrote:
> On 9 March 2012 15:13, Shawn Guo <shawn.guo@linaro.org> wrote:
> > On Fri, Mar 09, 2012 at 02:11:41AM +0000, Tabi Timur-B04825 wrote:
> >> Shawn Guo wrote:
> >> > Are you testing the patch set on top of the SHA below, which I
> >> > mentioned in the cover letter?
> >> >
> >> > ? ?3030763 (Merge tag 'asoc-3.4' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into topic/asoc)

Adding Liam who I just noticed isn't on the CCs.

> >> It appears the problem is with for-3.4 itself. ?I'm running git-bisect now
> >> to narrow it down.
> >>
> > My bisect tells that the offending commit is:
> >
> > ?96acc35 (ASoC: DAPM: Make sure DAPM widget IO ops hold the component mutex)
> >
> > But I do not understand why yet.
> 
> =============================================
> [ INFO: possible recursive locking detected ]
> 3.3.0-rc4+ #159 Not tainted
> ---------------------------------------------
> aplay/395 is trying to acquire lock:
>  (&codec->mutex){+.+...}, at: [<802f7414>] soc_widget_update_bits_locked+0x88/0x
> 1a0
> 
> but task is already holding lock:
>  (&codec->mutex){+.+...}, at: [<802f9df4>] snd_soc_dapm_stream_event+0x38/0xc0

Ick, right.  We need to create that separate I/O mutex I think...
Perhaps we should punt the locking change until 3.5 too, there's no
non-CODEC DAPM in 3.4?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120309/a4ff54f0/attachment-0001.sig>

[PATCH v3 00/11] ASoC: add imx-sgtl5000 machine driver working with fsl_ssi

[Mark Brown]

On Fri, Mar 09, 2012 at 03:13:49PM +0800, Shawn Guo wrote:

> My bisect tells that the offending commit is:

>   96acc35 (ASoC: DAPM: Make sure DAPM widget IO ops hold the component mutex)

> But I do not understand why yet.

It'll be deadlocking somewhere I expect - turning on the soft lockup
detector will probably make it obvious.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120309/33c86013/attachment-0001.sig>