[PATCH 1/4] ASoC: imx: let SND_MXC_SOC_FIQ select FIQ

[PATCH 1/4] ASoC: imx: let SND_MXC_SOC_FIQ select FIQ

[Shawn Guo]

CONFIG_FIQ is only needed when CONFIG_SND_MXC_SOC_FIQ is selected to
build imx-pcm-fiq.c, so let SND_MXC_SOC_FIQ select FIQ.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/imx/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/soc/imx/Kconfig b/sound/soc/imx/Kconfig
index 91b66ef..17810d9 100644
--- a/sound/soc/imx/Kconfig
+++ b/sound/soc/imx/Kconfig
@@ -1,7 +1,6 @@
 menuconfig SND_IMX_SOC
 	tristate "SoC Audio for Freescale i.MX CPUs"
 	depends on ARCH_MXC
-	select FIQ
 	select SND_SOC_AC97_BUS
 	help
 	  Say Y or M if you want to add support for codecs attached to
@@ -12,6 +11,7 @@ if SND_IMX_SOC
 
 config SND_MXC_SOC_FIQ
 	tristate
+	select FIQ
 
 config SND_MXC_SOC_MX2
 	tristate
-- 
1.7.5.4

[PATCH 2/4] ASoC: imx: move SND_SOC_AC97_BUS selection down to machine driver

[Shawn Guo]

SND_SOC_AC97_BUS is selected to enable the AC97 support in soc-core.
Rather than selecting the option under SND_IMX_SOC, it's better to
leave the selection to individual machine driver which knows if AC97
support is needed or not.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/imx/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/soc/imx/Kconfig b/sound/soc/imx/Kconfig
index 17810d9..bca5d34 100644
--- a/sound/soc/imx/Kconfig
+++ b/sound/soc/imx/Kconfig
@@ -1,7 +1,6 @@
 menuconfig SND_IMX_SOC
 	tristate "SoC Audio for Freescale i.MX CPUs"
 	depends on ARCH_MXC
-	select SND_SOC_AC97_BUS
 	help
 	  Say Y or M if you want to add support for codecs attached to
 	  the i.MX SSI interface.
@@ -37,6 +36,7 @@ config SND_SOC_MX27VIS_AIC32X4
 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
 	help
-- 
1.7.5.4

[PATCH 3/4] ASoC: imx: initialize dma_params burstsize just in imx-ssi

[Shawn Guo]

It's not necessary for imx-pcm-dma-mx2 to access imx_ssi.dma_params
for burstsize initialization.  Instead, it can just be done in imx-ssi
probe function once.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/imx/imx-pcm-dma-mx2.c |    5 -----
 sound/soc/imx/imx-ssi.c         |    2 +-
 2 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/sound/soc/imx/imx-pcm-dma-mx2.c b/sound/soc/imx/imx-pcm-dma-mx2.c
index f974e61..1737141 100644
--- a/sound/soc/imx/imx-pcm-dma-mx2.c
+++ b/sound/soc/imx/imx-pcm-dma-mx2.c
@@ -271,11 +271,6 @@ static struct snd_soc_platform_driver imx_soc_platform_mx2 = {
 
 static int __devinit imx_soc_platform_probe(struct platform_device *pdev)
 {
-	struct imx_ssi *ssi = platform_get_drvdata(pdev);
-
-	ssi->dma_params_tx.burstsize = 6;
-	ssi->dma_params_rx.burstsize = 4;
-
 	return snd_soc_register_platform(&pdev->dev, &imx_soc_platform_mx2);
 }
 
diff --git a/sound/soc/imx/imx-ssi.c b/sound/soc/imx/imx-ssi.c
index dbf43f5..25c6231 100644
--- a/sound/soc/imx/imx-ssi.c
+++ b/sound/soc/imx/imx-ssi.c
@@ -668,7 +668,7 @@ static int imx_ssi_probe(struct platform_device *pdev)
 	ssi->dma_params_rx.dma_addr = res->start + SSI_SRX0;
 	ssi->dma_params_tx.dma_addr = res->start + SSI_STX0;
 
-	ssi->dma_params_tx.burstsize = 4;
+	ssi->dma_params_tx.burstsize = 6;
 	ssi->dma_params_rx.burstsize = 4;
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx0");
-- 
1.7.5.4

[PATCH 4/4] ASoC: imx: separate imx-pcm bits from imx-ssi driver

[Shawn Guo]

Currently the imx-ssi.c[h] accommodates the imx-pcm common bits which
are shared between imx-pcm-dma-mx2 and imx-pcm-fiq drivers.  It assumes
that imx-pcm-dma-mx2 and imx-pcm-fiq will always be used together with
imx-ssi driver.  However this becomes untrue when we see that driver
sound/soc/fsl/fsl_ssi could possibly work with imx-pcm-dma-mx2 too.

The patch moves the imx-pcm common bits from imx-ssi.c[h] into new
files imx-pcm.c[h], and let imx-pcm-dma-mx2 and imx-pcm-fiq drivers
build it in, so that imx-pcm-dma-mx2 can work with no dependency on
imx-ssi driver.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/imx/Kconfig           |    5 ++
 sound/soc/imx/Makefile          |   10 ++--
 sound/soc/imx/imx-pcm-dma-mx2.c |    2 +-
 sound/soc/imx/imx-pcm.c         |  105 +++++++++++++++++++++++++++++++++++++++
 sound/soc/imx/imx-pcm.h         |   32 ++++++++++++
 sound/soc/imx/imx-ssi.c         |   88 --------------------------------
 sound/soc/imx/imx-ssi.h         |   16 +------
 7 files changed, 149 insertions(+), 109 deletions(-)
 create mode 100644 sound/soc/imx/imx-pcm.c
 create mode 100644 sound/soc/imx/imx-pcm.h

diff --git a/sound/soc/imx/Kconfig b/sound/soc/imx/Kconfig
index bca5d34..863a6b4 100644
--- a/sound/soc/imx/Kconfig
+++ b/sound/soc/imx/Kconfig
@@ -8,12 +8,17 @@ menuconfig SND_IMX_SOC
 
 if SND_IMX_SOC
 
+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_IMX_PCM
 
 config SND_MXC_SOC_WM1133_EV1
 	tristate "Audio on the the i.MX31ADS with WM1133-EV1 fitted"
diff --git a/sound/soc/imx/Makefile b/sound/soc/imx/Makefile
index d6d609b..303bc05 100644
--- a/sound/soc/imx/Makefile
+++ b/sound/soc/imx/Makefile
@@ -1,11 +1,11 @@
 # i.MX Platform Support
 snd-soc-imx-objs := imx-ssi.o
-snd-soc-imx-fiq-objs := imx-pcm-fiq.o
-snd-soc-imx-mx2-objs := imx-pcm-dma-mx2.o
-
 obj-$(CONFIG_SND_IMX_SOC) += snd-soc-imx.o
-obj-$(CONFIG_SND_MXC_SOC_FIQ) += snd-soc-imx-fiq.o
-obj-$(CONFIG_SND_MXC_SOC_MX2) += snd-soc-imx-mx2.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
diff --git a/sound/soc/imx/imx-pcm-dma-mx2.c b/sound/soc/imx/imx-pcm-dma-mx2.c
index 1737141..e2d7287 100644
--- a/sound/soc/imx/imx-pcm-dma-mx2.c
+++ b/sound/soc/imx/imx-pcm-dma-mx2.c
@@ -30,7 +30,7 @@
 
 #include <mach/dma.h>
 
-#include "imx-ssi.h"
+#include "imx-pcm.h"
 
 struct imx_pcm_runtime_data {
 	int period_bytes;
diff --git a/sound/soc/imx/imx-pcm.c b/sound/soc/imx/imx-pcm.c
new file mode 100644
index 0000000..93dc360
--- /dev/null
+++ b/sound/soc/imx/imx-pcm.c
@@ -0,0 +1,105 @@
+/*
+ * Copyright 2009 Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * This code is based on code copyrighted by Freescale,
+ * Liam Girdwood, Javier Martin and probably others.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/module.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include "imx-pcm.h"
+
+int snd_imx_pcm_mmap(struct snd_pcm_substream *substream,
+		struct vm_area_struct *vma)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	int ret;
+
+	ret = dma_mmap_writecombine(substream->pcm->card->dev, vma,
+		runtime->dma_area, runtime->dma_addr, runtime->dma_bytes);
+
+	pr_debug("%s: ret: %d %p 0x%08x 0x%08x\n", __func__, ret,
+			runtime->dma_area,
+			runtime->dma_addr,
+			runtime->dma_bytes);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snd_imx_pcm_mmap);
+
+static int imx_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
+{
+	struct snd_pcm_substream *substream = pcm->streams[stream].substream;
+	struct snd_dma_buffer *buf = &substream->dma_buffer;
+	size_t size = IMX_SSI_DMABUF_SIZE;
+
+	buf->dev.type = SNDRV_DMA_TYPE_DEV;
+	buf->dev.dev = pcm->card->dev;
+	buf->private_data = NULL;
+	buf->area = dma_alloc_writecombine(pcm->card->dev, size,
+					   &buf->addr, GFP_KERNEL);
+	if (!buf->area)
+		return -ENOMEM;
+	buf->bytes = size;
+
+	return 0;
+}
+
+static u64 imx_pcm_dmamask = DMA_BIT_MASK(32);
+
+int imx_pcm_new(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_card *card = rtd->card->snd_card;
+	struct snd_pcm *pcm = rtd->pcm;
+	int ret = 0;
+
+	if (!card->dev->dma_mask)
+		card->dev->dma_mask = &imx_pcm_dmamask;
+	if (!card->dev->coherent_dma_mask)
+		card->dev->coherent_dma_mask = DMA_BIT_MASK(32);
+	if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream) {
+		ret = imx_pcm_preallocate_dma_buffer(pcm,
+			SNDRV_PCM_STREAM_PLAYBACK);
+		if (ret)
+			goto out;
+	}
+
+	if (pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream) {
+		ret = imx_pcm_preallocate_dma_buffer(pcm,
+			SNDRV_PCM_STREAM_CAPTURE);
+		if (ret)
+			goto out;
+	}
+
+out:
+	return ret;
+}
+EXPORT_SYMBOL_GPL(imx_pcm_new);
+
+void imx_pcm_free(struct snd_pcm *pcm)
+{
+	struct snd_pcm_substream *substream;
+	struct snd_dma_buffer *buf;
+	int stream;
+
+	for (stream = 0; stream < 2; stream++) {
+		substream = pcm->streams[stream].substream;
+		if (!substream)
+			continue;
+
+		buf = &substream->dma_buffer;
+		if (!buf->area)
+			continue;
+
+		dma_free_writecombine(pcm->card->dev, buf->bytes,
+				      buf->area, buf->addr);
+		buf->area = NULL;
+	}
+}
+EXPORT_SYMBOL_GPL(imx_pcm_free);
diff --git a/sound/soc/imx/imx-pcm.h b/sound/soc/imx/imx-pcm.h
new file mode 100644
index 0000000..b5f5c3a
--- /dev/null
+++ b/sound/soc/imx/imx-pcm.h
@@ -0,0 +1,32 @@
+/*
+ * Copyright 2009 Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * This code is based on code copyrighted by Freescale,
+ * Liam Girdwood, Javier Martin and probably others.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#ifndef _IMX_PCM_H
+#define _IMX_PCM_H
+
+/*
+ * Do not change this as the FIQ handler depends on this size
+ */
+#define IMX_SSI_DMABUF_SIZE	(64 * 1024)
+
+struct imx_pcm_dma_params {
+	int dma;
+	unsigned long dma_addr;
+	int burstsize;
+};
+
+int snd_imx_pcm_mmap(struct snd_pcm_substream *substream,
+		     struct vm_area_struct *vma);
+int imx_pcm_new(struct snd_soc_pcm_runtime *rtd);
+void imx_pcm_free(struct snd_pcm *pcm);
+
+#endif /* _IMX_PCM_H */
diff --git a/sound/soc/imx/imx-ssi.c b/sound/soc/imx/imx-ssi.c
index 25c6231..9203cdd 100644
--- a/sound/soc/imx/imx-ssi.c
+++ b/sound/soc/imx/imx-ssi.c
@@ -363,94 +363,6 @@ static const struct snd_soc_dai_ops imx_ssi_pcm_dai_ops = {
 	.trigger	= imx_ssi_trigger,
 };
 
-int snd_imx_pcm_mmap(struct snd_pcm_substream *substream,
-		struct vm_area_struct *vma)
-{
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	int ret;
-
-	ret = dma_mmap_writecombine(substream->pcm->card->dev, vma,
-		runtime->dma_area, runtime->dma_addr, runtime->dma_bytes);
-
-	pr_debug("%s: ret: %d %p 0x%08x 0x%08x\n", __func__, ret,
-			runtime->dma_area,
-			runtime->dma_addr,
-			runtime->dma_bytes);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(snd_imx_pcm_mmap);
-
-static int imx_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
-{
-	struct snd_pcm_substream *substream = pcm->streams[stream].substream;
-	struct snd_dma_buffer *buf = &substream->dma_buffer;
-	size_t size = IMX_SSI_DMABUF_SIZE;
-
-	buf->dev.type = SNDRV_DMA_TYPE_DEV;
-	buf->dev.dev = pcm->card->dev;
-	buf->private_data = NULL;
-	buf->area = dma_alloc_writecombine(pcm->card->dev, size,
-					   &buf->addr, GFP_KERNEL);
-	if (!buf->area)
-		return -ENOMEM;
-	buf->bytes = size;
-
-	return 0;
-}
-
-static u64 imx_pcm_dmamask = DMA_BIT_MASK(32);
-
-int imx_pcm_new(struct snd_soc_pcm_runtime *rtd)
-{
-	struct snd_card *card = rtd->card->snd_card;
-	struct snd_pcm *pcm = rtd->pcm;
-	int ret = 0;
-
-	if (!card->dev->dma_mask)
-		card->dev->dma_mask = &imx_pcm_dmamask;
-	if (!card->dev->coherent_dma_mask)
-		card->dev->coherent_dma_mask = DMA_BIT_MASK(32);
-	if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream) {
-		ret = imx_pcm_preallocate_dma_buffer(pcm,
-			SNDRV_PCM_STREAM_PLAYBACK);
-		if (ret)
-			goto out;
-	}
-
-	if (pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream) {
-		ret = imx_pcm_preallocate_dma_buffer(pcm,
-			SNDRV_PCM_STREAM_CAPTURE);
-		if (ret)
-			goto out;
-	}
-
-out:
-	return ret;
-}
-EXPORT_SYMBOL_GPL(imx_pcm_new);
-
-void imx_pcm_free(struct snd_pcm *pcm)
-{
-	struct snd_pcm_substream *substream;
-	struct snd_dma_buffer *buf;
-	int stream;
-
-	for (stream = 0; stream < 2; stream++) {
-		substream = pcm->streams[stream].substream;
-		if (!substream)
-			continue;
-
-		buf = &substream->dma_buffer;
-		if (!buf->area)
-			continue;
-
-		dma_free_writecombine(pcm->card->dev, buf->bytes,
-				      buf->area, buf->addr);
-		buf->area = NULL;
-	}
-}
-EXPORT_SYMBOL_GPL(imx_pcm_free);
-
 static int imx_ssi_dai_probe(struct snd_soc_dai *dai)
 {
 	struct imx_ssi *ssi = dev_get_drvdata(dai->dev);
diff --git a/sound/soc/imx/imx-ssi.h b/sound/soc/imx/imx-ssi.h
index 1072dfb..5744e86 100644
--- a/sound/soc/imx/imx-ssi.h
+++ b/sound/soc/imx/imx-ssi.h
@@ -187,12 +187,7 @@
 
 #include <linux/dmaengine.h>
 #include <mach/dma.h>
-
-struct imx_pcm_dma_params {
-	int dma;
-	unsigned long dma_addr;
-	int burstsize;
-};
+#include "imx-pcm.h"
 
 struct imx_ssi {
 	struct platform_device *ac97_dev;
@@ -218,13 +213,4 @@ struct imx_ssi {
 	struct platform_device *soc_platform_pdev_fiq;
 };
 
-int snd_imx_pcm_mmap(struct snd_pcm_substream *substream, struct vm_area_struct *vma);
-int imx_pcm_new(struct snd_soc_pcm_runtime *rtd);
-void imx_pcm_free(struct snd_pcm *pcm);
-
-/*
- * Do not change this as the FIQ handler depends on this size
- */
-#define IMX_SSI_DMABUF_SIZE	(64 * 1024)
-
 #endif /* _IMX_SSI_H */
-- 
1.7.5.4

[PATCH 0/4] ASoC: merge imx into fsl

[Shawn Guo]

This is the second step towards reusing fsl_ssi driver on device tree
based IMX platforms.  The series merges sound/soc/imx into sound/soc/fsl
and makes fsl_ssi driver compilable on ARM/IMX.

Compile-tested for PowerPC.

Shawn Guo (4):
      ASoC: imx: add an explicit Kconfig option for imx-ssi driver
      ASoC: fsl: separate SSI and DMA Kconfig options
      ASoC: imx: merge sound/soc/imx into sound/soc/fsl
      ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

 sound/soc/Makefile                       |    1 -
 sound/soc/fsl/Kconfig                    |   99 ++++++++++++++++++++++++++++--
 sound/soc/fsl/Makefile                   |   22 ++++++-
 sound/soc/{imx => fsl}/eukrea-tlv320.c   |    2 +-
 sound/soc/fsl/fsl_ssi.c                  |   75 ++++++++++++++---------
 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                    |   64 -------------------
 sound/soc/imx/Makefile                   |   19 ------
 17 files changed, 162 insertions(+), 123 deletions(-)

[PATCH 1/4] ASoC: imx: add an explicit Kconfig option for imx-ssi driver

[Shawn Guo]

Currently ASoC:imx uses menuconfig option SND_IMX_SOC selects imx-ssi
driver, and it works because all the machine driver covered by the
menuconfig need to build imx-ssi driver in.  However, it will not work
any more if we have a imx based machine driver going into the menuconfig
while working with fsl_ssi driver (sound/soc/fsl/fsl_ssi.c) rather than
imx-ssi one.

The patch adds an explicit Kconfig option SND_SOC_IMX_SSI for imx-ssi
driver, so that it can be selected independently from the menuconfig
option SND_IMX_SOC.

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

diff --git a/sound/soc/imx/Kconfig b/sound/soc/imx/Kconfig
index 863a6b4..b9de8a8 100644
--- a/sound/soc/imx/Kconfig
+++ b/sound/soc/imx/Kconfig
@@ -8,6 +8,9 @@ menuconfig SND_IMX_SOC
 
 if SND_IMX_SOC
 
+config SND_SOC_IMX_SSI
+	tristate
+
 config SND_SOC_IMX_PCM
 	tristate
 
@@ -25,6 +28,7 @@ config SND_MXC_SOC_WM1133_EV1
 	depends on MACH_MX31ADS_WM1133_EV1 && EXPERIMENTAL
 	select SND_SOC_WM8350
 	select SND_MXC_SOC_FIQ
+	select SND_SOC_IMX_SSI
 	help
 	  Enable support for audio on the i.MX31ADS with the WM1133-EV1
 	  PMIC board with WM8835x fitted.
@@ -34,6 +38,7 @@ config SND_SOC_MX27VIS_AIC32X4
 	depends on MACH_IMX27_VISSTRIM_M10 && I2C
 	select SND_SOC_TLV320AIC32X4
 	select SND_MXC_SOC_MX2
+	select SND_SOC_IMX_SSI
 	help
 	  Say Y if you want to add support for SoC audio on Visstrim SM10
 	  board with TLV320AIC32X4 codec.
@@ -44,6 +49,7 @@ config SND_SOC_PHYCORE_AC97
 	select SND_SOC_AC97_BUS
 	select SND_SOC_WM9712
 	select SND_MXC_SOC_FIQ
+	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
@@ -57,6 +63,7 @@ config SND_SOC_EUKREA_TLV320
 	depends on I2C
 	select SND_SOC_TLV320AIC23
 	select SND_MXC_SOC_FIQ
+	select SND_SOC_IMX_SSI
 	help
 	  Enable I2S based access to the TLV320AIC23B codec attached
 	  to the SSI interface
diff --git a/sound/soc/imx/Makefile b/sound/soc/imx/Makefile
index 303bc05..981c9da 100644
--- a/sound/soc/imx/Makefile
+++ b/sound/soc/imx/Makefile
@@ -1,6 +1,6 @@
 # i.MX Platform Support
-snd-soc-imx-objs := imx-ssi.o
-obj-$(CONFIG_SND_IMX_SOC) += snd-soc-imx.o
+snd-soc-imx-ssi-objs := imx-ssi.o
+obj-$(CONFIG_SND_SOC_IMX_SSI) += snd-soc-imx-ssi.o
 
 obj-$(CONFIG_SND_SOC_IMX_PCM) += snd-soc-imx-pcm.o
 snd-soc-imx-pcm-y := imx-pcm.o
-- 
1.7.5.4

[PATCH 1/4] ASoC: imx: add an explicit Kconfig option for imx-ssi driver

[Timur Tabi]

Shawn Guo wrote:

> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Since when do you work for Linaro?

> ---
>  sound/soc/imx/Kconfig  |    7 +++++++
>  sound/soc/imx/Makefile |    4 ++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/imx/Kconfig b/sound/soc/imx/Kconfig
> index 863a6b4..b9de8a8 100644
> --- a/sound/soc/imx/Kconfig
> +++ b/sound/soc/imx/Kconfig
> @@ -8,6 +8,9 @@ menuconfig SND_IMX_SOC
>  
>  if SND_IMX_SOC
>  
> +config SND_SOC_IMX_SSI
> +	tristate
> +

I was hoping to see sound/soc/imx disappear completely, and all IMX
support move into sound/soc/fsl.  After all, i.MX is a Freescale name.

Anyway, I'm really glad to see this patchset, but I won't be able to
devote much time to studying it until next week, at the earliest.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 1/4] ASoC: imx: add an explicit Kconfig option for imx-ssi driver

[Shawn Guo]

On Thu, Feb 23, 2012 at 09:40:00AM -0600, Timur Tabi wrote:
> Shawn Guo wrote:
> 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> 
> Since when do you work for Linaro?
> 
Since one year ago, as Freescale assignee to Linaro.  So I'm still
employed by Freescale.

> > ---
> >  sound/soc/imx/Kconfig  |    7 +++++++
> >  sound/soc/imx/Makefile |    4 ++--
> >  2 files changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/sound/soc/imx/Kconfig b/sound/soc/imx/Kconfig
> > index 863a6b4..b9de8a8 100644
> > --- a/sound/soc/imx/Kconfig
> > +++ b/sound/soc/imx/Kconfig
> > @@ -8,6 +8,9 @@ menuconfig SND_IMX_SOC
> >  
> >  if SND_IMX_SOC
> >  
> > +config SND_SOC_IMX_SSI
> > +	tristate
> > +
> 
> I was hoping to see sound/soc/imx disappear completely, and all IMX
> support move into sound/soc/fsl.  After all, i.MX is a Freescale name.
> 
That's what I'm doing.  But to be clear, imx-ssi driver will still
exist for all those non-DT i.MX platforms.  We are supporting DT for
imx5 and imx6, but have not seen a plan to migrate all those old i.MX
platforms to DT.

-- 
Regards,
Shawn

[PATCH 1/4] ASoC: imx: add an explicit Kconfig option for imx-ssi driver

[Tabi Timur-B04825]

Shawn Guo wrote:
> That's what I'm doing.  But to be clear, imx-ssi driver will still
> exist for all those non-DT i.MX platforms.  We are supporting DT for
> imx5 and imx6, but have not seen a plan to migrate all those old i.MX
> platforms to DT.

On PowerPC, we handle this by creating cuImages.  These are kernel images 
that have a DTB built-in.  Perhaps something like that is needed for ARM?

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 1/4] ASoC: imx: add an explicit Kconfig option for imx-ssi driver

[Shawn Guo]

On Fri, Feb 24, 2012 at 02:59:22AM +0000, Tabi Timur-B04825 wrote:
> Shawn Guo wrote:
> > That's what I'm doing.  But to be clear, imx-ssi driver will still
> > exist for all those non-DT i.MX platforms.  We are supporting DT for
> > imx5 and imx6, but have not seen a plan to migrate all those old i.MX
> > platforms to DT.
> 
> On PowerPC, we handle this by creating cuImages.  These are kernel images 
> that have a DTB built-in.  Perhaps something like that is needed for ARM?
> 
We have the similar support on ARM (dtb can be appended to zImage).
It resolves the problem that boot loader does not support DT.  However
what I'm saying is that we do not have a plan to add DT support for
those old i.MX SoCs like imx1, imx2, imx3 in kernel.  That said, the
kernel running on those platforms are not aware of DT.

-- 
Regards,
Shawn

[PATCH 2/4] 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 3/4] 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>
---
 sound/soc/Kconfig                        |    1 -
 sound/soc/Makefile                       |    1 -
 sound/soc/fsl/Kconfig                    |   84 ++++++++++++++++++++++++++++++
 sound/soc/fsl/Makefile                   |   19 +++++++
 sound/soc/{imx => fsl}/eukrea-tlv320.c   |    2 +-
 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                    |   71 -------------------------
 sound/soc/imx/Makefile                   |   19 -------
 16 files changed, 105 insertions(+), 94 deletions(-)
 rename sound/soc/{imx => fsl}/eukrea-tlv320.c (99%)
 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/sound/soc/Kconfig b/sound/soc/Kconfig
index 35e662d..2b95e6b 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -32,7 +32,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 9ea8ac8..d95a119 100644
--- a/sound/soc/Makefile
+++ b/sound/soc/Makefile
@@ -9,7 +9,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..a62dc28 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,75 @@ 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_IMX_PCM
+
+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_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_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_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_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..65087b1 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -21,3 +21,22 @@ 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
+obj-$(CONFIG_SND_SOC_IMX_SSI) += snd-soc-imx-ssi.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 1c1fdd1..c33a303 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-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 b9de8a8..0000000
--- a/sound/soc/imx/Kconfig
+++ /dev/null
@@ -1,71 +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
-	tristate
-	select SND_SOC_IMX_PCM
-
-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_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_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_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_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 981c9da..0000000
--- a/sound/soc/imx/Makefile
+++ /dev/null
@@ -1,19 +0,0 @@
-# i.MX Platform Support
-snd-soc-imx-ssi-objs := imx-ssi.o
-obj-$(CONFIG_SND_SOC_IMX_SSI) += snd-soc-imx-ssi.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 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Shawn Guo]

Change PowerPC dependent IO accessors to architecture independent
ones, so that fsl_ssi driver can be built on both PowerPC and ARM/IMX.

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

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 3e06696..32fc84b 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -11,11 +11,14 @@
  */
 
 #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 <sound/core.h>
@@ -145,7 +148,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 = readl(&ssi->sisr) & SIER_FLAGS;
 
 	if (sisr & CCSR_SSI_SISR_RFRC) {
 		ssi_private->stats.rfrc++;
@@ -260,7 +263,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);
+		writel(sisr2, &ssi->sisr);
 
 	return ret;
 }
@@ -280,6 +283,7 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
 	struct fsl_ssi_private *ssi_private =
 		snd_soc_dai_get_drvdata(rtd->cpu_dai);
 	int synchronous = ssi_private->cpu_dai_drv.symmetric_rates;
+	u32 val;
 
 	/*
 	 * If this is the first stream opened, then request the IRQ
@@ -295,7 +299,9 @@ 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);
+		val = readl(&ssi->scr);
+		val &= ~CCSR_SSI_SCR_SSIEN;
+		writel(val, &ssi->scr);
 
 		/*
 		 * Program the SSI into I2S Slave Non-Network Synchronous mode.
@@ -303,20 +309,19 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
 		 *
 		 * FIXME: Little-endian samples require a different shift dir
 		 */
-		clrsetbits_be32(&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));
+		val = readl(&ssi->scr);
+		val &= ~(CCSR_SSI_SCR_I2S_MODE_MASK | CCSR_SSI_SCR_SYN);
+		val |= CCSR_SSI_SCR_TFR_CLK_DIS | CCSR_SSI_SCR_I2S_MODE_SLAVE |
+		       (synchronous ? CCSR_SSI_SCR_SYN : 0);
+		writel(val, &ssi->scr);
 
-		out_be32(&ssi->stcr,
-			 CCSR_SSI_STCR_TXBIT0 | CCSR_SSI_STCR_TFEN0 |
+		writel(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 |
+		writel(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 +329,7 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
 		 */
 
 		/* Enable the interrupts and DMA requests */
-		out_be32(&ssi->sier, SIER_FLAGS);
+		writel(SIER_FLAGS, &ssi->sier);
 
 		/*
 		 * Set the watermark for transmit FIFI 0 and receive FIFO 0. We
@@ -339,9 +344,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));
+		writel(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 +422,8 @@ 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 = readl(&ssi->scr) & CCSR_SSI_SCR_SSIEN;
+	u32 val;
 
 	/*
 	 * If we're in synchronous mode, and the SSI is already enabled,
@@ -438,10 +444,17 @@ 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);
-	else
-		clrsetbits_be32(&ssi->srccr, CCSR_SSI_SxCCR_WL_MASK, wl);
+	    ssi_private->cpu_dai_drv.symmetric_rates) {
+		val = readl(&ssi->stccr);
+		val &= CCSR_SSI_SxCCR_WL_MASK;
+		val |= wl;
+		writel(val, &ssi->stccr);
+	} else {
+		val = readl(&ssi->srccr);
+		val &= CCSR_SSI_SxCCR_WL_MASK;
+		val |= wl;
+		writel(val, &ssi->srccr);
+	}
 
 	return 0;
 }
@@ -461,29 +474,30 @@ 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;
+	u32 val;
 
+	val = readl(&ssi->scr);
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-			setbits32(&ssi->scr,
-				CCSR_SSI_SCR_SSIEN | CCSR_SSI_SCR_TE);
+			val |= CCSR_SSI_SCR_SSIEN | CCSR_SSI_SCR_TE;
 		else
-			setbits32(&ssi->scr,
-				CCSR_SSI_SCR_SSIEN | CCSR_SSI_SCR_RE);
+			val |= 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);
+			val &= ~CCSR_SSI_SCR_TE;
 		else
-			clrbits32(&ssi->scr, CCSR_SSI_SCR_RE);
+			val &= ~CCSR_SSI_SCR_RE;
 		break;
 
 	default:
 		return -EINVAL;
 	}
+	writel(val, &ssi->scr);
 
 	return 0;
 }
@@ -498,6 +512,7 @@ static void fsl_ssi_shutdown(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(rtd->cpu_dai);
+	u32 val;
 
 	if (ssi_private->first_stream == substream)
 		ssi_private->first_stream = ssi_private->second_stream;
@@ -510,7 +525,9 @@ 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);
+		val = readl(&ssi->scr);
+		val &= ~CCSR_SSI_SCR_SSIEN;
+		writel(val, &ssi->scr);
 	}
 }
 
-- 
1.7.5.4

[PATCH 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Sergei Shtylyov]

Hello.

On 23-02-2012 18:48, Shawn Guo wrote:

> Change PowerPC dependent IO accessors to architecture independent
> ones, so that fsl_ssi driver can be built on both PowerPC and ARM/IMX.

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

> diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
> index 3e06696..32fc84b 100644
> --- a/sound/soc/fsl/fsl_ssi.c
> +++ b/sound/soc/fsl/fsl_ssi.c
[...]
> @@ -145,7 +148,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 = readl(&ssi->sisr) & SIER_FLAGS;

    These functions are not equivalent concerning endianness. You should 
probably have used readl_be() instead, else it won't work on PPC anymore.

WBR, Sergei

[PATCH 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Timur Tabi]

Sergei Shtylyov wrote:
>     These functions are not equivalent concerning endianness. You should 
> probably have used readl_be() instead, else it won't work on PPC anymore.

readl_be() is not good enough, either.  The in/out_be functions have
specific syncronization instructions in them, so that the operations
conform to the PowerPC instruction ordering architecture.

You probably need to do something like this:

#ifdef PPC
#define read_ssi(x) in_be32(x)
#else #if defined ARM
#define read_ssi(x) readl(x)
#endif

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Russell King - ARM Linux]

On Thu, Feb 23, 2012 at 09:44:37AM -0600, Timur Tabi wrote:
> Sergei Shtylyov wrote:
> >     These functions are not equivalent concerning endianness. You should 
> > probably have used readl_be() instead, else it won't work on PPC anymore.
> 
> readl_be() is not good enough, either.  The in/out_be functions have
> specific syncronization instructions in them, so that the operations
> conform to the PowerPC instruction ordering architecture.

Is your readl() or readl_be() unordered then?  Wouldn't that be buggy
between coherent DMA accesses and accessing, eg, a PCI peripheral to
enable or read DMA status?

[PATCH 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Timur Tabi]

Russell King - ARM Linux wrote:
> Is your readl() or readl_be() unordered then?  

Yes.  At least, it sure looks like it is.

> Wouldn't that be buggy
> between coherent DMA accesses and accessing, eg, a PCI peripheral to
> enable or read DMA status?

I think so.

We almost never use readl.  I see that it is used in some places, but
honestly I can't see how it can be valid.  For instance, the mpic driver
uses it in mpic_startup_ht_interrupt().  I don't understand how that's
valid, since there's nothing preventing the readl() from happening
*before* the writeb.

I'll have to ask around, because I'm sure I'm missing something.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Russell King - ARM Linux]

On Thu, Feb 23, 2012 at 11:04:59AM -0600, Timur Tabi wrote:
> Russell King - ARM Linux wrote:
> > Is your readl() or readl_be() unordered then?  
> 
> Yes.  At least, it sure looks like it is.
> 
> > Wouldn't that be buggy
> > between coherent DMA accesses and accessing, eg, a PCI peripheral to
> > enable or read DMA status?
> 
> I think so.
> 
> We almost never use readl.  I see that it is used in some places, but
> honestly I can't see how it can be valid.  For instance, the mpic driver
> uses it in mpic_startup_ht_interrupt().  I don't understand how that's
> valid, since there's nothing preventing the readl() from happening
> *before* the writeb.

That just sounds broken.  If readl() can happen before a writeb() issued
in program order, how do most of the PCI drivers work?

> I'll have to ask around, because I'm sure I'm missing something.

If what you say is true, it means we have different semantics for the
same accessors on different architectures.  No wonder it's needing
drivers to gain ifdefs to select appropriate accessors.

This needs sorting out properly.  It needs the semantics of readl() etc
to be fully defined, and all architectures to implement those semantics
rather than what they think would be appropriate for them.

[alsa-devel] [PATCH 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Trent Piepho]

On Thu, Feb 23, 2012 at 12:14 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Feb 23, 2012 at 11:04:59AM -0600, Timur Tabi wrote:
>> Russell King - ARM Linux wrote:
>> We almost never use readl. ?I see that it is used in some places, but
>> honestly I can't see how it can be valid. ?For instance, the mpic driver
>> uses it in mpic_startup_ht_interrupt(). ?I don't understand how that's
>> valid, since there's nothing preventing the readl() from happening
>> *before* the writeb.
>
> That just sounds broken. ?If readl() can happen before a writeb() issued
> in program order, how do most of the PCI drivers work?

I was under the impression that on powerpc io memory was mapped
cache-inhibited and guarded, which provides most ordered, but not
store to load ordering without an additional synchronization
instruction.  See
http://cache.freescale.com/files/32bit/doc/app_note/AN3441.pdf on page
8.

PCI adds some additional complexity, since the PCI device is not
connected directly to the CPU.  One must consider the actions of the
PCI host controller.

[PATCH 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Timur Tabi]

Russell King - ARM Linux wrote:
>> > I'll have to ask around, because I'm sure I'm missing something.
> If what you say is true, it means we have different semantics for the
> same accessors on different architectures.  No wonder it's needing
> drivers to gain ifdefs to select appropriate accessors.
> 
> This needs sorting out properly.  It needs the semantics of readl() etc
> to be fully defined, and all architectures to implement those semantics
> rather than what they think would be appropriate for them.

So I asked around, and sure enough, my macro-fu was weak.  On PowerPC,
readl() maps to in_le32().  Even though PowerPC is big-endian, some
devices are little endian (e.g. PCI and some PICs).

in_le32() is the byte-swapping version of in_be32().  Both functions have
instruction order synchronization instructions in them.

So replacing in_be32() with readl() will break on PowerPC.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Russell King - ARM Linux]

On Fri, Feb 24, 2012 at 03:29:04PM -0600, Timur Tabi wrote:
> Russell King - ARM Linux wrote:
> >> > I'll have to ask around, because I'm sure I'm missing something.
> > If what you say is true, it means we have different semantics for the
> > same accessors on different architectures.  No wonder it's needing
> > drivers to gain ifdefs to select appropriate accessors.
> > 
> > This needs sorting out properly.  It needs the semantics of readl() etc
> > to be fully defined, and all architectures to implement those semantics
> > rather than what they think would be appropriate for them.
> 
> So I asked around, and sure enough, my macro-fu was weak.  On PowerPC,
> readl() maps to in_le32().  Even though PowerPC is big-endian, some
> devices are little endian (e.g. PCI and some PICs).
> 
> in_le32() is the byte-swapping version of in_be32().  Both functions have
> instruction order synchronization instructions in them.
> 
> So replacing in_be32() with readl() will break on PowerPC.

But not a BE variant.  Like ioread32be().

BenH tells me that he'll accept patches which converts in_32be() to
ioread32be(), as in_32be() is ancient PPC cruft that needs to die.

[PATCH 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Timur Tabi]

Russell King - ARM Linux wrote:
> BenH tells me that he'll accept patches which converts in_32be() to
> ioread32be(), as in_32be() is ancient PPC cruft that needs to die.

Where did he say this?  in_be32() (and its variants) are used EVERYWHERE
in PowerPC code.

Also, keep in mind that on ARM, ioread32be() is probably wrong, because we
want to read the register as little-endian.  The SSI audio controller is
little-endian on i.MX chips and big-endian on PowerPC chip.  So we really
need a native-endian variant of ioread32be().  ioread32() is little-endian
on PowerPC, so we can't use that.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Russell King - ARM Linux]

On Fri, Feb 24, 2012 at 04:00:17PM -0600, Timur Tabi wrote:
> Russell King - ARM Linux wrote:
> > BenH tells me that he'll accept patches which converts in_32be() to
> > ioread32be(), as in_32be() is ancient PPC cruft that needs to die.
> 
> Where did he say this?  in_be32() (and its variants) are used EVERYWHERE
> in PowerPC code.

It was actually Grant Likely who said it after Ben H said that drivers
using in_be* is historical PPC stuff that needs fixing.

> Also, keep in mind that on ARM, ioread32be() is probably wrong, because we
> want to read the register as little-endian.  The SSI audio controller is
> little-endian on i.MX chips and big-endian on PowerPC chip.  So we really
> need a native-endian variant of ioread32be().  ioread32() is little-endian
> on PowerPC, so we can't use that.

Ho hum.  So, what that's saying to me is that even implementing in_32be()
on ARM results in rubbish because it wouldn't be big endian there.

So are we heading towards driver specific sets of IO accessors? :-(  It
sounds like that's what it needs because of these different endian-ness
issues.

And lets not forget that we're talking just about this - there's also
the issue of on-SoC devices on ARM now appearing behind PCI buses on
x86, which would make those appear as LE.  I bet if any of those
appearing on PPC would appear in BE mode.  So, 'native endian' doesn't
work either.

So, I'm getting the feeling that something as simple as IO accessors are
just a huge mountain of crap waiting to bite.

[PATCH 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Timur Tabi]

Russell King - ARM Linux wrote:

> Ho hum.  So, what that's saying to me is that even implementing in_32be()
> on ARM results in rubbish because it wouldn't be big endian there.

Well, the "be" in in_be32() means to assume that 32-bit integer is stored
in big-endian format, and to convert it to native format during the read.
 An ARM implementation of in_be32() would include byte-swapping.

> So are we heading towards driver specific sets of IO accessors? :-(  It
> sounds like that's what it needs because of these different endian-ness
> issues.

I'm surprised that ioread32() on PowerPC always does a little-endian read,
but I'm guessing that was done for compatibility.  I don't know what to do
about that.

> And lets not forget that we're talking just about this - there's also
> the issue of on-SoC devices on ARM now appearing behind PCI buses on
> x86, which would make those appear as LE.  I bet if any of those
> appearing on PPC would appear in BE mode.  So, 'native endian' doesn't
> work either.

I think PCI is a special-case on most PowerPC parts, in that the registers
are still little-endian.  But I don't know how pervasive that is.  In
fact, I think on some Freescale PowerPC SOCs, the PCI registers are
little-endian, and other others they're big-endian.  You can see that in
fsl_pci.c.

> So, I'm getting the feeling that something as simple as IO accessors are
> just a huge mountain of crap waiting to bite.

Very few devices are shared between Freescale's PowerPC and ARM SOCs, so
the impact is only a few drivers.  That's why it might be okay (for now)
to have "dual" I/O accessors in specific drivers.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Russell King - ARM Linux]

On Fri, Feb 24, 2012 at 05:05:16PM -0600, Timur Tabi wrote:
> Russell King - ARM Linux wrote:
> 
> > Ho hum.  So, what that's saying to me is that even implementing in_32be()
> > on ARM results in rubbish because it wouldn't be big endian there.
> 
> Well, the "be" in in_be32() means to assume that 32-bit integer is stored
> in big-endian format, and to convert it to native format during the read.
>  An ARM implementation of in_be32() would include byte-swapping.

So, let's go around this loop again.

What would be wrong with converting in_be32() in these drivers to
instead ioread32be()?

[PATCH 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Timur Tabi]

Russell King - ARM Linux wrote:
> What would be wrong with converting in_be32() in these drivers to
> instead ioread32be()?

Well, it wouldn't actually fix any problems.   It's just a stylistic
change for PowerPC.  On ARM, however, ioread32be() is wrong because the
SSI registers are little-endian.

In case you're confused:  the SSI is an I2S audio controller developed by
the i.MX (ARM) division on Freescale.  When the PowerPC division wanted to
add an I2S controller to its SOCs, they just took the same SSI device and
dropped in the PowerPC SOC.  But since PowerPC is big-endian, the
registers are byte-swapped in hardware.  So even though the i.MX SSI and
the PowerPC SSI have the same registers and do the same thing, those
actual registers are little-endian on i.MX and big-endian on PowerPC.

As far as I know, there is no single function in the kernel that will
perform an ordered 32-bit big-endian read on PowerPC and an ordered 32-bit
little-endian read on ARM.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Russell King - ARM Linux]

On Fri, Feb 24, 2012 at 05:22:14PM -0600, Timur Tabi wrote:
> Russell King - ARM Linux wrote:
> > What would be wrong with converting in_be32() in these drivers to
> > instead ioread32be()?
> 
> Well, it wouldn't actually fix any problems.   It's just a stylistic
> change for PowerPC.  On ARM, however, ioread32be() is wrong because the
> SSI registers are little-endian.

So, to go around the loop a little further: to make this driver work
_unaltered_ on ARM, we'd need an in_32be() that read the register
_without_ doing the byte swapping.

But then, such an implementation of in_32be() would be absurd because
it's not doing what it says.

So, the driver needs to change its accessor method.  There is no
accessor in existance at present which does the right thing.

And then I went on to explain that there's more complicated cases too,
where it seems that in theory you can end up with the same IP connected
in BE mode and sitting behind a PCI bus, which presents the need for
accessors which know the bus type by some means.

[PATCH 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX

[Timur Tabi]

Russell King - ARM Linux wrote:

> So, to go around the loop a little further: to make this driver work
> _unaltered_ on ARM, we'd need an in_32be() that read the register
> _without_ doing the byte swapping.
> 
> But then, such an implementation of in_32be() would be absurd because
> it's not doing what it says.

Exactly.

> So, the driver needs to change its accessor method.  There is no
> accessor in existance at present which does the right thing.

Yes, that is my assessment.

> And then I went on to explain that there's more complicated cases too,
> where it seems that in theory you can end up with the same IP connected
> in BE mode and sitting behind a PCI bus, which presents the need for
> accessors which know the bus type by some means.

And since most PCI devices are little-endian, I think that's why readl()
is always little-endian.

We might be able to modify the PowerPC version of ioread32() to be
native-endian without too much difficulty, but that's something that Ben
Herrenschmidt would need to approve.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

[Shawn Guo]

This is the third series towards reusing fsl_ssi driver on device tree
based IMX platforms.  It makes a few cleanups on existing PowerPC code
to ease the later addition of an IMX machine driver that works on
fsl_ssi.

Shawn Guo (6):
      ASoC: fsl: correct get_dma_channel parameter name
      ASoC: fsl: align mpc8610_hpcd with p1022_ds on getting codec node
      ASoC: Remove unnecessary -codec from cs4270 driver name
      ASoC: fsl: create fsl_utils to accommodate the common functions
      ASoC: fsl: use platform_device_id table to match p1022_ds device
      ASoC: fsl: check property 'compatible' for the machine name

 sound/soc/codecs/cs4270.c    |    2 +-
 sound/soc/fsl/Kconfig        |    5 +
 sound/soc/fsl/Makefile       |    2 +
 sound/soc/fsl/fsl_ssi.c      |    6 +-
 sound/soc/fsl/fsl_utils.c    |  153 ++++++++++++++++++++++++++++++++++
 sound/soc/fsl/fsl_utils.h    |   29 +++++++
 sound/soc/fsl/mpc8610_hpcd.c |  167 +++----------------------------------
 sound/soc/fsl/p1022_ds.c     |  185 ++++-------------------------------------
 sound/soc/pxa/raumfeld.c     |    2 +-
 9 files changed, 226 insertions(+), 325 deletions(-)

[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

[Mark Brown]

On Fri, Feb 24, 2012 at 10:09:35PM +0800, Shawn Guo wrote:
> This is the third series towards reusing fsl_ssi driver on device tree
> based IMX platforms.  It makes a few cleanups on existing PowerPC code
> to ease the later addition of an IMX machine driver that works on
> fsl_ssi.

Please don't post new threads as followups to old threads, it makes
things harder to follow in threaded mail readers.
-------------- 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/20120224/02e85cf2/attachment.sig>

[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

[Shawn Guo]

On Fri, Feb 24, 2012 at 02:02:01PM +0000, Mark Brown wrote:
> On Fri, Feb 24, 2012 at 10:09:35PM +0800, Shawn Guo wrote:
> > This is the third series towards reusing fsl_ssi driver on device tree
> > based IMX platforms.  It makes a few cleanups on existing PowerPC code
> > to ease the later addition of an IMX machine driver that works on
> > fsl_ssi.
> 
> Please don't post new threads as followups to old threads, it makes
> things harder to follow in threaded mail readers.

I did that because they are a big series, though some of the patches
can be standalone.

-- 
Regards,
Shawn

[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

[Mark Brown]

On Fri, Feb 24, 2012 at 10:23:45PM +0800, Shawn Guo wrote:
> On Fri, Feb 24, 2012 at 02:02:01PM +0000, Mark Brown wrote:

> > Please don't post new threads as followups to old threads, it makes
> > things harder to follow in threaded mail readers.

> I did that because they are a big series, though some of the patches
> can be standalone.

That doesn't seem to be related to my point?  I'm asking you to not
reply to an old version of your patch series in order to post a new
version of it, just post the new version.
-------------- 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/20120224/56cc550d/attachment.sig>

[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

[Shawn Guo]

On Fri, Feb 24, 2012 at 02:17:06PM +0000, Mark Brown wrote:
> On Fri, Feb 24, 2012 at 10:23:45PM +0800, Shawn Guo wrote:
> > On Fri, Feb 24, 2012 at 02:02:01PM +0000, Mark Brown wrote:
> 
> > > Please don't post new threads as followups to old threads, it makes
> > > things harder to follow in threaded mail readers.
> 
> > I did that because they are a big series, though some of the patches
> > can be standalone.
> 
> That doesn't seem to be related to my point?  I'm asking you to not
> reply to an old version of your patch series in order to post a new
> version of it, just post the new version.

This is the first version of this series.

-- 
Regards,
Shawn

[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

[Timur Tabi]

Shawn Guo wrote:
>> > That doesn't seem to be related to my point?  I'm asking you to not
>> > reply to an old version of your patch series in order to post a new
>> > version of it, just post the new version.

> This is the first version of this series.

I think there's a communication breakdown.  Shawn, take a look at this:

http://mailman.alsa-project.org/pipermail/alsa-devel/2012-February/thread.html

Search for "[alsa-devel] [PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl".

You'll see that this email is indented, because it was posted to the
mailing list AS A REPLY to "[alsa-devel] [PATCH 1/4] ASoC: imx: let
SND_MXC_SOC_FIQ select FIQ".  That's because it has this line in the
message header:

In-Reply-To: <1329979644-31046-1-git-send-email-shawn.guo@linaro.org>

This is what Mark is talking about.  He wants new patch sets to be posted
NOT as replies to other patch sets.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

[Shawn Guo]

On Fri, Feb 24, 2012 at 05:14:12PM -0600, Timur Tabi wrote:
> Shawn Guo wrote:
> >> > That doesn't seem to be related to my point?  I'm asking you to not
> >> > reply to an old version of your patch series in order to post a new
> >> > version of it, just post the new version.
> 
> > This is the first version of this series.
> 
> I think there's a communication breakdown.  Shawn, take a look at this:
> 
> http://mailman.alsa-project.org/pipermail/alsa-devel/2012-February/thread.html
> 
> Search for "[alsa-devel] [PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl".
> 
> You'll see that this email is indented, because it was posted to the
> mailing list AS A REPLY to "[alsa-devel] [PATCH 1/4] ASoC: imx: let
> SND_MXC_SOC_FIQ select FIQ".  That's because it has this line in the
> message header:
> 
> In-Reply-To: <1329979644-31046-1-git-send-email-shawn.guo@linaro.org>
> 
> This is what Mark is talking about.  He wants new patch sets to be posted
> NOT as replies to other patch sets.
> 
Yes, I did something wrong.  But my point is the following patches
should belong to one big series.  They can not be applied separately.

[PATCH 1/4] ASoC: imx: let SND_MXC_SOC_FIQ select FIQ
[PATCH 2/4] ASoC: imx: move SND_SOC_AC97_BUS selection down to machine driver
[PATCH 3/4] ASoC: imx: initialize dma_params burstsize just in imx-ssi
[PATCH 4/4] ASoC: imx: separate imx-pcm bits from imx-ssi driver
[PATCH 0/4] ASoC: merge imx into fsl
[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

-- 
Regards,
Shawn

[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

[Tabi Timur-B04825]

Shawn Guo wrote:
> Yes, I did something wrong.  But my point is the following patches
> should belong to one big series.  They can not be applied separately.
>
> [PATCH 1/4] ASoC: imx: let SND_MXC_SOC_FIQ select FIQ
> [PATCH 2/4] ASoC: imx: move SND_SOC_AC97_BUS selection down to machine driver
> [PATCH 3/4] ASoC: imx: initialize dma_params burstsize just in imx-ssi
> [PATCH 4/4] ASoC: imx: separate imx-pcm bits from imx-ssi driver
> [PATCH 0/4] ASoC: merge imx into fsl
> [PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

That's not what Mark is talking about.  You somehow posted patch 0/6 as a 
reply to the *previous* patchset, so the two patchsets were merged into 
one email thread.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

[Russell King - ARM Linux]

On Sat, Feb 25, 2012 at 01:39:05AM +0000, Tabi Timur-B04825 wrote:
> Shawn Guo wrote:
> > Yes, I did something wrong.  But my point is the following patches
> > should belong to one big series.  They can not be applied separately.
> >
> > [PATCH 1/4] ASoC: imx: let SND_MXC_SOC_FIQ select FIQ
> > [PATCH 2/4] ASoC: imx: move SND_SOC_AC97_BUS selection down to machine driver
> > [PATCH 3/4] ASoC: imx: initialize dma_params burstsize just in imx-ssi
> > [PATCH 4/4] ASoC: imx: separate imx-pcm bits from imx-ssi driver
> > [PATCH 0/4] ASoC: merge imx into fsl
> > [PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl
> 
> That's not what Mark is talking about.  You somehow posted patch 0/6 as a 
> reply to the *previous* patchset, so the two patchsets were merged into 
> one email thread.

Sorry, you and Mark are not listening to Shawn.  Even worse, you're
telling Shawn that what he's trying to explain to you is not the point
when it is actually the whole point.

The "[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl" was posted as a
follow-up to "[PATCH 1/4] ASoC: imx: let SND_MXC_SOC_FIQ select FIQ".

So, the "*previous* patchset" _was_ "[PATCH 1/4] ASoC: imx: let
SND_MXC_SOC_FIQ select FIQ" and not some other random patch set.  That
is a fundamental fact, which can't be argued.  Anyone can verify it, and
it can be verified by checking the email headers.

[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

[Mark Brown]

On Sat, Feb 25, 2012 at 10:17:58AM +0000, Russell King - ARM Linux wrote:
> On Sat, Feb 25, 2012 at 01:39:05AM +0000, Tabi Timur-B04825 wrote:

> > That's not what Mark is talking about.  You somehow posted patch 0/6 as a 
> > reply to the *previous* patchset, so the two patchsets were merged into 
> > one email thread.

> Sorry, you and Mark are not listening to Shawn.  Even worse, you're
> telling Shawn that what he's trying to explain to you is not the point
> when it is actually the whole point.

> The "[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl" was posted as a
> follow-up to "[PATCH 1/4] ASoC: imx: let SND_MXC_SOC_FIQ select FIQ".

> So, the "*previous* patchset" _was_ "[PATCH 1/4] ASoC: imx: let
> SND_MXC_SOC_FIQ select FIQ" and not some other random patch set.  That
> is a fundamental fact, which can't be argued.  Anyone can verify it, and
> it can be verified by checking the email headers.

No, I get that it's a new version of the same patch set rather than a
totally unrelated patch set that was followed up to - I'd still rather
it weren't posted like that as the threading ends up being less helpful
than is desirable.
-------------- 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/20120225/63a85b3a/attachment.sig>

[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

[Russell King - ARM Linux]

On Sat, Feb 25, 2012 at 11:44:15AM +0000, Mark Brown wrote:
> On Sat, Feb 25, 2012 at 10:17:58AM +0000, Russell King - ARM Linux wrote:
> > On Sat, Feb 25, 2012 at 01:39:05AM +0000, Tabi Timur-B04825 wrote:
> 
> > > That's not what Mark is talking about.  You somehow posted patch 0/6 as a 
> > > reply to the *previous* patchset, so the two patchsets were merged into 
> > > one email thread.
> 
> > Sorry, you and Mark are not listening to Shawn.  Even worse, you're
> > telling Shawn that what he's trying to explain to you is not the point
> > when it is actually the whole point.
> 
> > The "[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl" was posted as a
> > follow-up to "[PATCH 1/4] ASoC: imx: let SND_MXC_SOC_FIQ select FIQ".
> 
> > So, the "*previous* patchset" _was_ "[PATCH 1/4] ASoC: imx: let
> > SND_MXC_SOC_FIQ select FIQ" and not some other random patch set.  That
> > is a fundamental fact, which can't be argued.  Anyone can verify it, and
> > it can be verified by checking the email headers.
> 
> No, I get that it's a new version of the same patch set rather than a
> totally unrelated patch set that was followed up to - I'd still rather
> it weren't posted like that as the threading ends up being less helpful
> than is desirable.

Take a look at the diffstat, and you'll see that there's no way that it's
a "new version of the same patch set".  Tell me, how can the set of six
patches be a new version of the previous set of four patches when the
previous set of four patches moves a bunch of files and the following set
of six patches does not?

And if you bother to read Shawn's reply to your question, you'll see that
the whole reason he grouped the two together is because they follow on
from each other.

Here's the diffstats from his emails from the interlinked thread:

[PATCH 1/4] ASoC: imx: add an explicit Kconfig option for imx-ssi driver
 sound/soc/imx/Kconfig  |    7 +++++++
 sound/soc/imx/Makefile |    4 ++--
 2 files changed, 9 insertions(+), 2 deletions(-)

[PATCH 2/4] ASoC: fsl: separate SSI and DMA Kconfig options
 sound/soc/fsl/Kconfig  |   15 +++++++++------
 sound/soc/fsl/Makefile |    3 ++-
 2 files changed, 11 insertions(+), 7 deletions(-)

[PATCH 3/4] ASoC: imx: merge sound/soc/imx into sound/soc/fsl
 sound/soc/Kconfig                        |    1 -
 sound/soc/Makefile                       |    1 -
 sound/soc/fsl/Kconfig                    |   84 ++++++++++++++++++++++++++++++
 sound/soc/fsl/Makefile                   |   19 +++++++
 sound/soc/{imx => fsl}/eukrea-tlv320.c   |    2 +-
 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}/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                    |   71 -------------------------
 sound/soc/imx/Makefile                   |   19 -------
 16 files changed, 105 insertions(+), 94 deletions(-)

[PATCH 4/4] ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX
 sound/soc/fsl/fsl_ssi.c |   75 ++++++++++++++++++++++++++++------------------

[PATCH 1/6] ASoC: fsl: correct get_dma_channel parameter name
 sound/soc/fsl/mpc8610_hpcd.c |    4 ++--
 sound/soc/fsl/p1022_ds.c     |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

[PATCH 2/6] ASoC: fsl: align mpc8610_hpcd with p1022_ds on getting codec node
 sound/soc/fsl/mpc8610_hpcd.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

[PATCH 3/6] ASoC: Remove unnecessary -codec from cs4270 driver name
 sound/soc/codecs/cs4270.c    |    2 +-
 sound/soc/fsl/mpc8610_hpcd.c |    4 ++--
 sound/soc/pxa/raumfeld.c     |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

[PATCH 4/6] ASoC: fsl: create fsl_utils to accommodate the common functions
 sound/soc/fsl/Kconfig        |    5 ++
 sound/soc/fsl/Makefile       |    2 +
 sound/soc/fsl/fsl_utils.c    |  153 ++++++++++++++++++++++++++++++++++++++++
 sound/soc/fsl/fsl_utils.h    |   29 ++++++++
 sound/soc/fsl/mpc8610_hpcd.c |  157 +++---------------------------------------
 sound/soc/fsl/p1022_ds.c     |  149 +++-------------------------------------
 6 files changed, 209 insertions(+), 286 deletions(-)

[PATCH 5/6] ASoC: fsl: use platform_device_id table to match p1022_ds device
 sound/soc/fsl/p1022_ds.c |   36 +++++++++---------------------------
 1 files changed, 9 insertions(+), 27 deletions(-)

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name
 sound/soc/fsl/fsl_ssi.c      |    6 +++---
 sound/soc/fsl/mpc8610_hpcd.c |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

See - not the 6 patch patchset is IN NO WAY a new version of the 4 patch
patchset like you're trying to say it is.

So my point that you're not listening stands.

[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

[Mark Brown]

On Sat, Feb 25, 2012 at 12:12:17PM +0000, Russell King - ARM Linux wrote:

> Take a look at the diffstat, and you'll see that there's no way that it's
> a "new version of the same patch set".  Tell me, how can the set of six

Yeah, sorry - I misremembered the specific reason they were related when
I replied to you.

> And if you bother to read Shawn's reply to your question, you'll see that
> the whole reason he grouped the two together is because they follow on
> from each other.

No, I understand that the patch sets are related, like I say I had
misremembered the relation when replying to you.

> So my point that you're not listening stands.

I am, what I'm asking is that people don't send new patch sets in reply
to old ones even if they are related as while the thought is good in
that case the software tends not to do a great job with it.  Either just
mentioning things in the cover letter or resending the previous stuff as
part of a new, combined series tends to work better.
-------------- 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/20120225/722e71b2/attachment-0001.sig>

[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

[Shawn Guo]

On Sat, Feb 25, 2012 at 11:44:15AM +0000, Mark Brown wrote:
> On Sat, Feb 25, 2012 at 10:17:58AM +0000, Russell King - ARM Linux wrote:
> > On Sat, Feb 25, 2012 at 01:39:05AM +0000, Tabi Timur-B04825 wrote:
> 
> > > That's not what Mark is talking about.  You somehow posted patch 0/6 as a 
> > > reply to the *previous* patchset, so the two patchsets were merged into 
> > > one email thread.
> 
> > Sorry, you and Mark are not listening to Shawn.  Even worse, you're
> > telling Shawn that what he's trying to explain to you is not the point
> > when it is actually the whole point.
> 
> > The "[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl" was posted as a
> > follow-up to "[PATCH 1/4] ASoC: imx: let SND_MXC_SOC_FIQ select FIQ".
> 
> > So, the "*previous* patchset" _was_ "[PATCH 1/4] ASoC: imx: let
> > SND_MXC_SOC_FIQ select FIQ" and not some other random patch set.  That
> > is a fundamental fact, which can't be argued.  Anyone can verify it, and
> > it can be verified by checking the email headers.
> 
> No, I get that it's a new version of the same patch set

I know it's confusing.  But "[PATCH 0/6] ASoC: a few cleanups on
sound/soc/fsl" is not a new version of the series "[PATCH 1/4] ASoC:
imx: let SND_MXC_SOC_FIQ select FIQ".  Instead, it's a follow-up
series which depends on previously posted patches.

Anyway, I will stop doing that to confuse people.

Regards,
Shawn

> rather than a
> totally unrelated patch set that was followed up to - I'd still rather
> it weren't posted like that as the threading ends up being less helpful
> than is desirable.

[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

[Russell King - ARM Linux]

On Sat, Feb 25, 2012 at 07:21:51AM +0800, Shawn Guo wrote:
> On Fri, Feb 24, 2012 at 02:17:06PM +0000, Mark Brown wrote:
> > On Fri, Feb 24, 2012 at 10:23:45PM +0800, Shawn Guo wrote:
> > > On Fri, Feb 24, 2012 at 02:02:01PM +0000, Mark Brown wrote:
> > 
> > > > Please don't post new threads as followups to old threads, it makes
> > > > things harder to follow in threaded mail readers.
> > 
> > > I did that because they are a big series, though some of the patches
> > > can be standalone.
> > 
> > That doesn't seem to be related to my point?  I'm asking you to not
> > reply to an old version of your patch series in order to post a new
> > version of it, just post the new version.
> 
> This is the first version of this series.

Your covering message for this series of six patches:

From: Shawn Guo <shawn.guo@linaro.org>
To: alsa-devel at alsa-project.org
Subject: [PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl
Date: Fri, 24 Feb 2012 22:09:35 +0800
Message-Id: <1330092582-21180-1-git-send-email-shawn.guo@linaro.org>
X-Mailer: git-send-email 1.7.5.4
In-Reply-To: <1329979644-31046-1-git-send-email-shawn.guo@linaro.org>
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
References: <1329979644-31046-1-git-send-email-shawn.guo@linaro.org>
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Your patch 1 from a previous series:

From: Shawn Guo <shawn.guo@linaro.org>
To: alsa-devel at alsa-project.org
Subject: [PATCH 1/4] ASoC: imx: let SND_MXC_SOC_FIQ select FIQ
Date: Thu, 23 Feb 2012 14:47:21 +0800
Message-Id: <1329979644-31046-1-git-send-email-shawn.guo@linaro.org>
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Notice the same message ids.  That means your covering letter with
subject "[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl" is a reply
to your "[PATCH 1/4] ASoC: imx: let SND_MXC_SOC_FIQ select FIQ" message.

Consequently anyone who reads list mail by threads gets your new series
of six patches attached to your previous series of four patches.  That
can be extremely annoying, especially when it ends up being buried in
peoples mailboxes as a direct result of threaded sorting - and it's
a recipe for your patches to be ignored.

[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

[Shawn Guo]

On Fri, Feb 24, 2012 at 11:23:29PM +0000, Russell King - ARM Linux wrote:
> On Sat, Feb 25, 2012 at 07:21:51AM +0800, Shawn Guo wrote:
> > On Fri, Feb 24, 2012 at 02:17:06PM +0000, Mark Brown wrote:
> > > On Fri, Feb 24, 2012 at 10:23:45PM +0800, Shawn Guo wrote:
> > > > On Fri, Feb 24, 2012 at 02:02:01PM +0000, Mark Brown wrote:
> > > 
> > > > > Please don't post new threads as followups to old threads, it makes
> > > > > things harder to follow in threaded mail readers.
> > > 
> > > > I did that because they are a big series, though some of the patches
> > > > can be standalone.
> > > 
> > > That doesn't seem to be related to my point?  I'm asking you to not
> > > reply to an old version of your patch series in order to post a new
> > > version of it, just post the new version.
> > 
> > This is the first version of this series.
> 
> Your covering message for this series of six patches:
> 
> From: Shawn Guo <shawn.guo@linaro.org>
> To: alsa-devel at alsa-project.org
> Subject: [PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl
> Date: Fri, 24 Feb 2012 22:09:35 +0800
> Message-Id: <1330092582-21180-1-git-send-email-shawn.guo@linaro.org>
> X-Mailer: git-send-email 1.7.5.4
> In-Reply-To: <1329979644-31046-1-git-send-email-shawn.guo@linaro.org>
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> References: <1329979644-31046-1-git-send-email-shawn.guo@linaro.org>
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> Your patch 1 from a previous series:
> 
> From: Shawn Guo <shawn.guo@linaro.org>
> To: alsa-devel at alsa-project.org
> Subject: [PATCH 1/4] ASoC: imx: let SND_MXC_SOC_FIQ select FIQ
> Date: Thu, 23 Feb 2012 14:47:21 +0800
> Message-Id: <1329979644-31046-1-git-send-email-shawn.guo@linaro.org>
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> Notice the same message ids.  That means your covering letter with
> subject "[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl" is a reply
> to your "[PATCH 1/4] ASoC: imx: let SND_MXC_SOC_FIQ select FIQ" message.
> 
> Consequently anyone who reads list mail by threads gets your new series
> of six patches attached to your previous series of four patches.  That
> can be extremely annoying, especially when it ends up being buried in
> peoples mailboxes as a direct result of threaded sorting - and it's
> a recipe for your patches to be ignored.

Sorry about that.  But they are not separate patch series but belong
to one big series.  It seems it's a bad idea to post one big series
in such a way.  Will stop doing it.

-- 
Regards,
Shawn

[PATCH 1/6] ASoC: fsl: correct get_dma_channel parameter name

[Shawn Guo]

The second parameter of function get_dma_channel is actually a property
name rather than a compatible string, so rename it for less confusing.

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

diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c
index 0ea4a5a..15e06e9d 100644
--- a/sound/soc/fsl/mpc8610_hpcd.c
+++ b/sound/soc/fsl/mpc8610_hpcd.c
@@ -273,7 +273,7 @@ static int codec_node_dev_name(struct device_node *np, char *buf, size_t len)
 }
 
 static int get_dma_channel(struct device_node *ssi_np,
-			   const char *compatible,
+			   const char *name,
 			   struct snd_soc_dai_link *dai,
 			   unsigned int *dma_channel_id,
 			   unsigned int *dma_id)
@@ -283,7 +283,7 @@ static int get_dma_channel(struct device_node *ssi_np,
 	const u32 *iprop;
 	int ret;
 
-	dma_channel_np = get_node_by_phandle_name(ssi_np, compatible,
+	dma_channel_np = get_node_by_phandle_name(ssi_np, name,
 						  "fsl,ssi-dma-channel");
 	if (!dma_channel_np)
 		return -EINVAL;
diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c
index a5d4e80..d32ec46 100644
--- a/sound/soc/fsl/p1022_ds.c
+++ b/sound/soc/fsl/p1022_ds.c
@@ -276,7 +276,7 @@ static int codec_node_dev_name(struct device_node *np, char *buf, size_t len)
 }
 
 static int get_dma_channel(struct device_node *ssi_np,
-			   const char *compatible,
+			   const char *name,
 			   struct snd_soc_dai_link *dai,
 			   unsigned int *dma_channel_id,
 			   unsigned int *dma_id)
@@ -286,7 +286,7 @@ static int get_dma_channel(struct device_node *ssi_np,
 	const u32 *iprop;
 	int ret;
 
-	dma_channel_np = get_node_by_phandle_name(ssi_np, compatible,
+	dma_channel_np = get_node_by_phandle_name(ssi_np, name,
 						  "fsl,ssi-dma-channel");
 	if (!dma_channel_np)
 		return -EINVAL;
-- 
1.7.5.4

[PATCH 1/6] ASoC: fsl: correct get_dma_channel parameter name

[Timur Tabi]

Shawn Guo wrote:
> The second parameter of function get_dma_channel is actually a property
> name rather than a compatible string, so rename it for less confusing.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---

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

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 1/6] ASoC: fsl: correct get_dma_channel parameter name

[Mark Brown]

On Fri, Feb 24, 2012 at 10:09:36PM +0800, Shawn Guo wrote:
> The second parameter of function get_dma_channel is actually a property
> name rather than a compatible string, so rename it for less confusing.

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/20120228/6998310a/attachment.sig>

[PATCH 2/6] ASoC: fsl: align mpc8610_hpcd with p1022_ds on getting codec node

[Shawn Guo]

Align mpc8610_hpcd with p1022_ds on getting codec node by just calling
of_parse_phandle.  The bonus point of doing that is we can save
exporting get_node_by_phandle_name() when we consolidate the common
bits between mpc8610_hpcd and p1022_ds into a module, which can be
shared by more machine drivers added later.

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

diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c
index 15e06e9d..93256b3 100644
--- a/sound/soc/fsl/mpc8610_hpcd.c
+++ b/sound/soc/fsl/mpc8610_hpcd.c
@@ -336,12 +336,8 @@ static int mpc8610_hpcd_probe(struct platform_device *pdev)
 	const char *sprop;
 	const u32 *iprop;
 
-	/* We are only interested in SSIs with a codec phandle in them,
-	 * so let's make sure this SSI has one. The MPC8610 HPCD only
-	 * knows about the CS4270 codec, so reject anything else.
-	 */
-	codec_np = get_node_by_phandle_name(np, "codec-handle",
-					    "cirrus,cs4270");
+	/* Find the codec node for this SSI. */
+	codec_np = of_parse_phandle(np, "codec-handle", 0);
 	if (!codec_np) {
 		dev_err(dev, "invalid codec node\n");
 		return -EINVAL;
-- 
1.7.5.4

[PATCH 2/6] ASoC: fsl: align mpc8610_hpcd with p1022_ds on getting codec node

[Timur Tabi]

Shawn Guo wrote:
> Align mpc8610_hpcd with p1022_ds on getting codec node by just calling
> of_parse_phandle.  The bonus point of doing that is we can save
> exporting get_node_by_phandle_name() when we consolidate the common
> bits between mpc8610_hpcd and p1022_ds into a module, which can be
> shared by more machine drivers added later.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

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

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 2/6] ASoC: fsl: align mpc8610_hpcd with p1022_ds on getting codec node

[Mark Brown]

On Fri, Feb 24, 2012 at 10:09:37PM +0800, Shawn Guo wrote:
> Align mpc8610_hpcd with p1022_ds on getting codec node by just calling
> of_parse_phandle.  The bonus point of doing that is we can save
> exporting get_node_by_phandle_name() when we consolidate the common
> bits between mpc8610_hpcd and p1022_ds into a module, which can be
> shared by more machine drivers added later.

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/20120228/f1a4631b/attachment.sig>

[PATCH 3/6] ASoC: Remove unnecessary -codec from cs4270 driver name

[Shawn Guo]

Similar to what commit 1e3ad57 (ASoC: Remove redundant -codec from
WM8776 driver name) does for wm8776 driver, this patch does the same
thing for cs4270 driver.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/codecs/cs4270.c    |    2 +-
 sound/soc/fsl/mpc8610_hpcd.c |    4 ++--
 sound/soc/pxa/raumfeld.c     |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/cs4270.c b/sound/soc/codecs/cs4270.c
index 6baccd2..1d672f5 100644
--- a/sound/soc/codecs/cs4270.c
+++ b/sound/soc/codecs/cs4270.c
@@ -715,7 +715,7 @@ MODULE_DEVICE_TABLE(i2c, cs4270_id);
  */
 static struct i2c_driver cs4270_i2c_driver = {
 	.driver = {
-		.name = "cs4270-codec",
+		.name = "cs4270",
 		.owner = THIS_MODULE,
 	},
 	.id_table = cs4270_id,
diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c
index 93256b3..fcf9302 100644
--- a/sound/soc/fsl/mpc8610_hpcd.c
+++ b/sound/soc/fsl/mpc8610_hpcd.c
@@ -245,7 +245,7 @@ static int get_parent_cell_index(struct device_node *np)
  * '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".
+ * example, "cs4270.0-004f".
  */
 static int codec_node_dev_name(struct device_node *np, char *buf, size_t len)
 {
@@ -267,7 +267,7 @@ static int codec_node_dev_name(struct device_node *np, char *buf, size_t len)
 	if (!i2c)
 		return -ENODEV;
 
-	snprintf(buf, len, "%s-codec.%u-%04x", temp, i2c->adapter->nr, addr);
+	snprintf(buf, len, "%s.%u-%04x", temp, i2c->adapter->nr, addr);
 
 	return 0;
 }
diff --git a/sound/soc/pxa/raumfeld.c b/sound/soc/pxa/raumfeld.c
index ba15451..0837065 100644
--- a/sound/soc/pxa/raumfeld.c
+++ b/sound/soc/pxa/raumfeld.c
@@ -232,7 +232,7 @@ static struct snd_soc_ops raumfeld_ak4104_ops = {
 	.cpu_dai_name	= "pxa-ssp-dai.0",		\
 	.platform_name	= "pxa-pcm-audio",		\
 	.codec_dai_name	= "cs4270-hifi",		\
-	.codec_name	= "cs4270-codec.0-0048",	\
+	.codec_name	= "cs4270.0-0048",	\
 	.ops		= &raumfeld_cs4270_ops,		\
 }
 
-- 
1.7.5.4

[PATCH 3/6] ASoC: Remove unnecessary -codec from cs4270 driver name

[Timur Tabi]

Shawn Guo wrote:
> Similar to what commit 1e3ad57 (ASoC: Remove redundant -codec from
> WM8776 driver name) does for wm8776 driver, this patch does the same
> thing for cs4270 driver.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---

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

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 3/6] ASoC: Remove unnecessary -codec from cs4270 driver name

[Mark Brown]

On Fri, Feb 24, 2012 at 10:09:38PM +0800, Shawn Guo wrote:
> Similar to what commit 1e3ad57 (ASoC: Remove redundant -codec from
> WM8776 driver name) does for wm8776 driver, this patch does the same
> thing for cs4270 driver.

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/20120228/cdbc41cc/attachment.sig>

[PATCH 4/6] ASoC: fsl: create fsl_utils to accommodate the common functions

[Shawn Guo]

There are 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    |  153 ++++++++++++++++++++++++++++++++++++++++
 sound/soc/fsl/fsl_utils.h    |   29 ++++++++
 sound/soc/fsl/mpc8610_hpcd.c |  157 +++---------------------------------------
 sound/soc/fsl/p1022_ds.c     |  149 +++-------------------------------------
 6 files changed, 209 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 a62dc28..337c3d1 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 65087b1..46752af 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..90722aa
--- /dev/null
+++ b/sound/soc/fsl/fsl_utils.c
@@ -0,0 +1,153 @@
+/**
+ * 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_i2c.h>
+#include <sound/soc.h>
+
+#include "fsl_utils.h"
+
+/**
+ * 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;
+}
+
+/**
+ * fsl_asoc_get_codec_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".
+ */
+int fsl_asoc_get_codec_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;
+}
+EXPORT_SYMBOL_GPL(fsl_asoc_get_codec_dev_name);
+
+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;
+	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;
+}
+EXPORT_SYMBOL_GPL(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..c928a15
--- /dev/null
+++ b/sound/soc/fsl/fsl_utils.h
@@ -0,0 +1,29 @@
+/**
+ * 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;
+
+extern int fsl_asoc_get_codec_dev_name(struct device_node *np,
+				       char *buf, size_t len);
+extern 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 4/6] ASoC: fsl: create fsl_utils to accommodate the common functions

[Timur Tabi]

Shawn Guo wrote:
> There are 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>
> ---

What repository is this for?  When I apply it to Mark's for-next branch, I
get this:

Applying: ASoC: fsl: create fsl_utils to accommodate the common functions
error: patch failed: sound/soc/fsl/Kconfig:1
error: sound/soc/fsl/Kconfig: patch does not apply
error: patch failed: sound/soc/fsl/Makefile:8
error: sound/soc/fsl/Makefile: patch does not apply
Patch failed at 0004 ASoC: fsl: create fsl_utils to accommodate the common
functions
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 4/6] ASoC: fsl: create fsl_utils to accommodate the common functions

[Timur Tabi]

Shawn Guo wrote:
> There are some amount of code duplication between mpc8610_hpcd and

"There is"

> ---
> diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
> index 65087b1..46752af 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

I think it would be easier to do this:

obj-$(CONFIG_SND_SOC_FSL_SSI) += snd-soc-fsl-ssi.o snd-soc-fsl-utils.o
obj-$(CONFIG_SND_SOC_POWERPC_DMA) += snd-soc-fsl-dma.o snd-soc-fsl-utils.o

Then you don't need to change the Kconfig.

> +static struct device_node *get_node_by_phandle_name(struct device_node *np,
> +	const char *name, const char *compatible)

...

> +static int get_parent_cell_index(struct device_node *np)

I think you should merge these two functions into their callers.  There's
not much point in keeping them separate now.  That will also allow you to
add dev_err() messages when returning error codes.

> +/**
> + * fsl_asoc_get_codec_dev_name - determine the dev_name for a codec node

Can you add kerneldoc descriptions of the parameters?

 * @np: pointer to the I2C device tree 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".
> + */
> +int fsl_asoc_get_codec_dev_name(struct device_node *np, char *buf, size_t len)
> +{
> +	const u32 *iprop;
> +	int addr;

'addr' should be a u32, actually.  I'm not sure why I made it a signed
integer.

> +
> +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)
> +{

Can you add kerneldoc comments to this function?

> +MODULE_AUTHOR("Timur Tabi <timur@freescale.com>");
> +MODULE_DESCRIPTION("Freescale ASoC utility code");
> +MODULE_LICENSE("GPL v2");

Is this really a module?

> diff --git a/sound/soc/fsl/fsl_utils.h b/sound/soc/fsl/fsl_utils.h
> new file mode 100644
> index 0000000..c928a15
> --- /dev/null
> +++ b/sound/soc/fsl/fsl_utils.h
> @@ -0,0 +1,29 @@
> +/**
> + * 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;
> +
> +extern int fsl_asoc_get_codec_dev_name(struct device_node *np,
> +				       char *buf, size_t len);
> +extern 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);

No 'extern' for function prototypes, please.


-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 4/6] ASoC: fsl: create fsl_utils to accommodate the common functions

[Shawn Guo]

On Mon, Feb 27, 2012 at 03:36:48PM -0600, Timur Tabi wrote:
> Shawn Guo wrote:
> > There are some amount of code duplication between mpc8610_hpcd and
> 
> "There is"
> 
Ok.

> > ---
> > diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
> > index 65087b1..46752af 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
> 
> I think it would be easier to do this:
> 
> obj-$(CONFIG_SND_SOC_FSL_SSI) += snd-soc-fsl-ssi.o snd-soc-fsl-utils.o
> obj-$(CONFIG_SND_SOC_POWERPC_DMA) += snd-soc-fsl-dma.o snd-soc-fsl-utils.o
> 
Since fsl_utils.c is created to accommodate the common functions used
by mpc8610_hpcd.c and p1022_ds.c, I guess you actually meant something
like the following?

# MPC8610 HPCD Machine Support
snd-soc-mpc8610-hpcd-objs := mpc8610_hpcd.o fsl_utils.o
obj-$(CONFIG_SND_SOC_MPC8610_HPCD) += snd-soc-mpc8610-hpcd.o

# P1022 DS Machine Support
snd-soc-p1022-ds-objs := p1022_ds.o fsl_utils.o
obj-$(CONFIG_SND_SOC_P1022_DS) += snd-soc-p1022-ds.o

I actually started with doing so, but later changed to make fsl_utils
as a module.  It seems that mpc8610_hpcd and p1022_ds will not be
built in a single kernel image, but on ARM/IMX, it's very likely that
multiple machine drivers will be built in a single image, as the ARM
community is moving to support single image for multiple SoCs.  In
that case, fsl_utils can not be linked like that but has to be a module.

> Then you don't need to change the Kconfig.
> 
> > +static struct device_node *get_node_by_phandle_name(struct device_node *np,
> > +	const char *name, const char *compatible)
> 
> ...
> 
> > +static int get_parent_cell_index(struct device_node *np)
> 
> I think you should merge these two functions into their callers.  There's
> not much point in keeping them separate now.  That will also allow you to
> add dev_err() messages when returning error codes.
> 
Ok.

> > +/**
> > + * fsl_asoc_get_codec_dev_name - determine the dev_name for a codec node
> 
> Can you add kerneldoc descriptions of the parameters?
> 
Ok.

>  * @np: pointer to the I2C device tree 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".
> > + */
> > +int fsl_asoc_get_codec_dev_name(struct device_node *np, char *buf, size_t len)
> > +{
> > +	const u32 *iprop;
> > +	int addr;
> 
> 'addr' should be a u32, actually.  I'm not sure why I made it a signed
> integer.
> 
Ok.

> > +
> > +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)
> > +{
> 
> Can you add kerneldoc comments to this function?
> 
Ok.

> > +MODULE_AUTHOR("Timur Tabi <timur@freescale.com>");
> > +MODULE_DESCRIPTION("Freescale ASoC utility code");
> > +MODULE_LICENSE("GPL v2");
> 
> Is this really a module?
> 
As I explained above, yes, it has to be a module.

> > diff --git a/sound/soc/fsl/fsl_utils.h b/sound/soc/fsl/fsl_utils.h
> > new file mode 100644
> > index 0000000..c928a15
> > --- /dev/null
> > +++ b/sound/soc/fsl/fsl_utils.h
> > @@ -0,0 +1,29 @@
> > +/**
> > + * 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;
> > +
> > +extern int fsl_asoc_get_codec_dev_name(struct device_node *np,
> > +				       char *buf, size_t len);
> > +extern 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);
> 
> No 'extern' for function prototypes, please.
> 
Ok.

-- 
Regards,
Shawn

[PATCH 4/6] ASoC: fsl: create fsl_utils to accommodate the common functions

[Tabi Timur-B04825]

Shawn Guo wrote:

> Since fsl_utils.c is created to accommodate the common functions used
> by mpc8610_hpcd.c and p1022_ds.c, I guess you actually meant something
> like the following?
>
> # MPC8610 HPCD Machine Support
> snd-soc-mpc8610-hpcd-objs := mpc8610_hpcd.o fsl_utils.o
> obj-$(CONFIG_SND_SOC_MPC8610_HPCD) += snd-soc-mpc8610-hpcd.o
>
> # P1022 DS Machine Support
> snd-soc-p1022-ds-objs := p1022_ds.o fsl_utils.o
> obj-$(CONFIG_SND_SOC_P1022_DS) += snd-soc-p1022-ds.o

Yes. :-)

>
> I actually started with doing so, but later changed to make fsl_utils
> as a module.  It seems that mpc8610_hpcd and p1022_ds will not be
> built in a single kernel image, but on ARM/IMX, it's very likely that
> multiple machine drivers will be built in a single image, as the ARM
> community is moving to support single image for multiple SoCs.  In
> that case, fsl_utils can not be linked like that but has to be a module.

Ah yes, good point.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 4/6] ASoC: fsl: create fsl_utils to accommodate the common functions

[Timur Tabi]

Shawn Guo wrote:

> +EXPORT_SYMBOL_GPL(fsl_asoc_get_codec_dev_name);

Please use EXPORT_SYMBOL() instead of EXPORT_SYMBOL_GPL().  Freescale does
not want to impose license restrictions on its customers.

Also, I think there's a bug in this function that I never got around to
fixing.  I think you need to call put_device() somewhere.
of_find_i2c_device_by_node() calls bus_find_device(), which calls
get_device(), which increments the reference count.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 5/6] ASoC: fsl: use platform_device_id table to match p1022_ds device

[Shawn Guo]

Instead of run-time detecting driver name from device tree model
property for binding the correct platform device created in fsl_ssi
probe function, we can simply use platform_device_id table to match
the p1022_ds device.

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

diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c
index 60f7bb8..a6f4a8a 100644
--- a/sound/soc/fsl/p1022_ds.c
+++ b/sound/soc/fsl/p1022_ds.c
@@ -408,10 +408,19 @@ static int __devexit p1022_ds_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct platform_device_id p1022_ds_ids[] = {
+	{ .name = "snd-soc-p1022ds", },
+	{ .name = "snd-soc-p1022", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, p1022_ds_ids);
+
 static struct platform_driver p1022_ds_driver = {
 	.probe = p1022_ds_probe,
 	.remove = __devexit_p(p1022_ds_remove),
+	.id_table = p1022_ds_ids,
 	.driver = {
+		.name = "p1022_ds",
 		.owner = THIS_MODULE,
 	},
 };
@@ -425,33 +434,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 5/6] ASoC: fsl: use platform_device_id table to match p1022_ds device

[Timur Tabi]

Shawn Guo wrote:

> -
> -	/*
> -	 * 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.
> -	 */

Please retain this paragraph somewhere.  It's important to explain why
we're checking for both strings.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 5/6] ASoC: fsl: use platform_device_id table to match p1022_ds device

[Timur Tabi]

Shawn Guo wrote:

> -
> -	/*
> -	 * 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.
> -	 */

Please retain this paragraph somewhere.  It's important to explain why
we're checking for both strings.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 5/6] ASoC: fsl: use platform_device_id table to match p1022_ds device

[Shawn Guo]

On Mon, Feb 27, 2012 at 03:39:17PM -0600, Timur Tabi wrote:
> Shawn Guo wrote:
> 
> > -
> > -	/*
> > -	 * 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.
> > -	 */
> 
> Please retain this paragraph somewhere.  It's important to explain why
> we're checking for both strings.
> 

Ok, it will be something like the following.

/*
 * 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.
 */
static struct platform_device_id p1022_ds_ids[] = {
	{ .name = "snd-soc-p1022ds", },
	{ .name = "snd-soc-p1022", },
	{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(platform, p1022_ds_ids);

-- 
Regards,
Shawn

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Shawn Guo]

The fsl_ssi probe function detects the client machine name using model
property of the device tree.  But it will not work for the ARM client
machines which are to be added.  The model property is given as a
descriptive string on all ARM device tree (arch/arm/boot/*.dts).
Instead, the compatible property gives the machine name.

It seems the fsl_ssi PowerPC users, machine mpc8610_hpcd and p1022ds,
basically have the same thing in model and compatible properties.
Detecting machine name using compatible property becomes a solution
working for both PowerPC and ARM users of fsl_ssi.

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

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 32fc84b..ff570a6 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -733,12 +733,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 7ead1e5..4195182 100644
--- a/sound/soc/fsl/mpc8610_hpcd.c
+++ b/sound/soc/fsl/mpc8610_hpcd.c
@@ -411,7 +411,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",
-- 
1.7.5.4

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Mark Brown]

On Fri, Feb 24, 2012 at 10:09:41PM +0800, Shawn Guo wrote:
> The fsl_ssi probe function detects the client machine name using model
> property of the device tree.  But it will not work for the ARM client
> machines which are to be added.  The model property is given as a
> descriptive string on all ARM device tree (arch/arm/boot/*.dts).
> Instead, the compatible property gives the machine name.

The ARM machines really shouldn't be using the bindings that PowerPC is
using for the machine as they conflate the machine driver with the SSI
binding.  The idiom for new systems is that machine drivers should be
first class devices in the device tree referencing the SSI and CODEC.

The current bindings should be preserved for existing systems of course.
-------------- 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/20120224/e1f0c88e/attachment.sig>

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Timur Tabi]

Mark Brown wrote:
> The ARM machines really shouldn't be using the bindings that PowerPC is
> using for the machine as they conflate the machine driver with the SSI
> binding.  The idiom for new systems is that machine drivers should be
> first class devices in the device tree referencing the SSI and CODEC.

Where is this documented?

I don't remember participating in the discussion for this new binding.

> The current bindings should be preserved for existing systems of course.

There's no reason we can't update the PowerPC device trees.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Shawn Guo]

On Fri, Feb 24, 2012 at 10:30:05AM -0600, Timur Tabi wrote:
> Mark Brown wrote:
> > The ARM machines really shouldn't be using the bindings that PowerPC is
> > using for the machine as they conflate the machine driver with the SSI
> > binding.  The idiom for new systems is that machine drivers should be
> > first class devices in the device tree referencing the SSI and CODEC.
> 
> Where is this documented?
> 
Documentation/devicetree/bindings/sound/tegra*, I guess.

-- 
Regards,
Shawn

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Mark Brown]

On Fri, Feb 24, 2012 at 10:30:05AM -0600, Timur Tabi wrote:
> Mark Brown wrote:
> > The ARM machines really shouldn't be using the bindings that PowerPC is
> > using for the machine as they conflate the machine driver with the SSI
> > binding.  The idiom for new systems is that machine drivers should be
> > first class devices in the device tree referencing the SSI and CODEC.

> Where is this documented?

> I don't remember participating in the discussion for this new binding.

It's not a specific binding but rather a general idea of how these
bindings should be done - I don't know how one would go about
documenting general ideas like that, I have asked people to do that but
I don't think there's a mechanism.  There's a couple of such bindings in
mainline at present (the nVidia ones).

I do recall you being involved in some of the discussions, though it's
possible I have these confused with other device tree threads.

> > The current bindings should be preserved for existing systems of course.

> There's no reason we can't update the PowerPC device trees.

We could, yes, but it would make people's existing device trees buggy
which would be sad.  It'd be nicer to keep the old binding around and
support it for people who are already using it.
-------------- 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/20120225/be8b9fec/attachment-0001.sig>

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Shawn Guo]

On Fri, Feb 24, 2012 at 02:12:07PM +0000, Mark Brown wrote:
> On Fri, Feb 24, 2012 at 10:09:41PM +0800, Shawn Guo wrote:
> > The fsl_ssi probe function detects the client machine name using model
> > property of the device tree.  But it will not work for the ARM client
> > machines which are to be added.  The model property is given as a
> > descriptive string on all ARM device tree (arch/arm/boot/*.dts).
> > Instead, the compatible property gives the machine name.
> 
> The ARM machines really shouldn't be using the bindings that PowerPC is
> using for the machine as they conflate the machine driver with the SSI
> binding.  The idiom for new systems is that machine drivers should be
> first class devices in the device tree referencing the SSI and CODEC.
> 
> The current bindings should be preserved for existing systems of course.

Per discussion [1], the whole point we move to fsl_ssi is that we can
reuse the DT support already in fsl_ssi.  If it's something we should
not use for IMX, I fail to see why we should make this move.

Regards,
Shawn

[1] http://article.gmane.org/gmane.linux.drivers.devicetree/6637

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Mark Brown]

On Sat, Feb 25, 2012 at 09:28:47AM +0800, Shawn Guo wrote:

> Per discussion [1], the whole point we move to fsl_ssi is that we can
> reuse the DT support already in fsl_ssi.  If it's something we should
> not use for IMX, I fail to see why we should make this move.

I was assuming that the idea was to share the code for device control
as the same IP block was being deployed in the various Freescale SoCs
- if the only thing they have in common is the device tree support it
would be a bit strange to merge them.

Note that it's purely the bit where the machine driver is instantiated
from the SSI (which is just one or two of several properties) that I'm
talking about here, the bindings for the SSI itself look good.
-------------- 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/20120225/2640971a/attachment.sig>

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Shawn Guo]

On Sat, Feb 25, 2012 at 11:42:28AM +0000, Mark Brown wrote:
> On Sat, Feb 25, 2012 at 09:28:47AM +0800, Shawn Guo wrote:
> 
> > Per discussion [1], the whole point we move to fsl_ssi is that we can
> > reuse the DT support already in fsl_ssi.  If it's something we should
> > not use for IMX, I fail to see why we should make this move.
> 
> I was assuming that the idea was to share the code for device control
> as the same IP block was being deployed in the various Freescale SoCs
> - if the only thing they have in common is the device tree support it
> would be a bit strange to merge them.
> 
Here is the thing.

For historical reason, there are two drivers for the same SSI block
found on PowerPC and IMX, fsl_ssi and imx-ssi.  The fsl_ssi driver is
used on PowerPC with DT support, while imx-ssi is used on IMX with no
DT support.  Along with the work of supporting DT for the modern IMX
SoCs, I was planning to add DT support into imx-ssi.  But I was told
by Timur that instead of doing that, I should reuse fsl_ssi for those
DT aware IMX platforms.  From his POV, one big reason of why imx-ssi
was created is that ARM/IMX did not support DT before while fsl_ssi was
born to support DT only.  Since ARM/IMX starts supporting DT, fsl_ssi
should be reused there.

I'm personally fine with either reusing fsl_ssi or adding DT support
into imx-ssi.  I do not see any problem with reusing fsl_ssi so far,
but I can quickly turn around to adding new binding for imx-ssi if
you are strong on this position.

> Note that it's purely the bit where the machine driver is instantiated
> from the SSI (which is just one or two of several properties) that I'm
> talking about here, the bindings for the SSI itself look good.

Hmm, there is no SSI binding property contributing to instantiating
the machine driver.  It looks /model property for the device name used
to instantiate the machine driver, which I personally think is fine.

-- 
Regards,
Shawn

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Mark Brown]

On Sat, Feb 25, 2012 at 09:09:53PM +0800, Shawn Guo wrote:

> DT aware IMX platforms.  From his POV, one big reason of why imx-ssi
> was created is that ARM/IMX did not support DT before while fsl_ssi was
> born to support DT only.  Since ARM/IMX starts supporting DT, fsl_ssi
> should be reused there.

No, that's not really a major reason - the big issue was more the lack
of any support from Freescale for the i.MX in mainline at that time and
the lack of any information on the similarities or differences between
the IP implementations in the two processor families, there's quite a
few quirks in the SSI register interface and it wasn't clear what was
shared and what wasn't.  It was more important to get things working in
mainline, the hope was that someone would come along later and merge if
that was possible (which you're now doing!).

> I'm personally fine with either reusing fsl_ssi or adding DT support
> into imx-ssi.  I do not see any problem with reusing fsl_ssi so far,
> but I can quickly turn around to adding new binding for imx-ssi if
> you are strong on this position.

Nobody's suggesting that they don't get merged if they really are the
same IP and can use the same code.  The device tree bindings are (and
should be) a really small proportion of the code.

> > Note that it's purely the bit where the machine driver is instantiated
> > from the SSI (which is just one or two of several properties) that I'm
> > talking about here, the bindings for the SSI itself look good.

> Hmm, there is no SSI binding property contributing to instantiating
> the machine driver.  It looks /model property for the device name used
> to instantiate the machine driver, which I personally think is fine.

The CODEC binding for the machine driver is part of the SSI binding, and
a mandatory one at that.  It looks like the code should be refactored to
register the SSI unconditionally and then optionally also register a
machine driver if there's a CODEC property present.  That way existing
systems should continue to work fine with their current bindings and new
ones can use the modern binding style.

Just using the plain machine name can get a bit inconvenient when you
get reference designs cloned which share the audio but differ in other
ways - the nVidia WM8903 driver is a good example of this, there's a
large number of boards that are definitely distinct boards but are very
similar from an audio point of view.  You'd have to add all the
machines to the machine driver compatible list.
-------------- 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/20120225/73011a5c/attachment.sig>

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Tabi Timur-B04825]

Mark Brown wrote:

> Nobody's suggesting that they don't get merged if they really are the
> same IP and can use the same code.  The device tree bindings are (and
> should be) a really small proportion of the code.

My understanding is that they really are the same exact IP.

> The CODEC binding for the machine driver is part of the SSI binding, and
> a mandatory one at that.  It looks like the code should be refactored to
> register the SSI unconditionally

Ok.

> and then optionally also register a
> machine driver if there's a CODEC property present.

Ah, that's not a bad idea.  I like that.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Timur Tabi]

Shawn Guo wrote:
> It seems the fsl_ssi PowerPC users, machine mpc8610_hpcd and p1022ds,
> basically have the same thing in model and compatible properties.
> Detecting machine name using compatible property becomes a solution
> working for both PowerPC and ARM users of fsl_ssi.

Do you actually have an 8610 or P1022 to test this?  I'm guessing the
answer is no.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Shawn Guo]

On Fri, Feb 24, 2012 at 10:32:50AM -0600, Timur Tabi wrote:
> Shawn Guo wrote:
> > It seems the fsl_ssi PowerPC users, machine mpc8610_hpcd and p1022ds,
> > basically have the same thing in model and compatible properties.
> > Detecting machine name using compatible property becomes a solution
> > working for both PowerPC and ARM users of fsl_ssi.
> 
> Do you actually have an 8610 or P1022 to test this?  I'm guessing the
> answer is no.
> 
No, I do not.  I only compile-tested it.  I need your favor here.

-- 
Regards,
Shawn

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Timur Tabi]

Shawn Guo wrote:
>> > Do you actually have an 8610 or P1022 to test this?  I'm guessing the
>> > answer is no.
>> > 
> No, I do not.  I only compile-tested it.  I need your favor here.

No problem.  I will get to it as soon as I can.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Timur Tabi]

Shawn Guo wrote:
> The fsl_ssi probe function detects the client machine name using model
> property of the device tree.  But it will not work for the ARM client
> machines which are to be added.  The model property is given as a
> descriptive string on all ARM device tree (arch/arm/boot/*.dts).
> Instead, the compatible property gives the machine name.

If you apply this change to the P1022DS as well, then you can eliminate
the need to check for "P1022" and "P1022DS", since the compatible string
in the P1022DS DTS has always said, "fsl,P1022DS".

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Shawn Guo]

On Mon, Feb 27, 2012 at 03:54:01PM -0600, Timur Tabi wrote:
> Shawn Guo wrote:
> > The fsl_ssi probe function detects the client machine name using model
> > property of the device tree.  But it will not work for the ARM client
> > machines which are to be added.  The model property is given as a
> > descriptive string on all ARM device tree (arch/arm/boot/*.dts).
> > Instead, the compatible property gives the machine name.
> 
> If you apply this change to the P1022DS as well, then you can eliminate
> the need to check for "P1022" and "P1022DS", since the compatible string
> in the P1022DS DTS has always said, "fsl,P1022DS".
> 
Since I was asked by Mark to use new binding for new machine driver,
I would drop this patch and keep PowerPC machine drivers working with
/model property.

-- 
Regards,
Shawn

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Tabi Timur-B04825]

Shawn Guo wrote:
> Since I was asked by Mark to use new binding for new machine driver,
> I would drop this patch and keep PowerPC machine drivers working with
> /model property.

I think Mark is only concerned with not breaking compatibility with 
existing device trees.  Both the P1022 and MPC8610 device trees have had 
this compatible property.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Shawn Guo]

On Tue, Feb 28, 2012 at 02:12:53AM +0000, Tabi Timur-B04825 wrote:
> Shawn Guo wrote:
> > Since I was asked by Mark to use new binding for new machine driver,
> > I would drop this patch and keep PowerPC machine drivers working with
> > /model property.
> 
> I think Mark is only concerned with not breaking compatibility with 
> existing device trees.  Both the P1022 and MPC8610 device trees have had 
> this compatible property.
> 
Are you saying that both older and new P1022 device trees have
compatible = "fsl,P1022DS", so that changing to check /compatible
could save the check on /model of P1022 device tree?  Otherwise,
the check between "fsl,P1022" and "fsl,P1022DS" can not be saved.

-- 
Regards,
Shawn

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Tabi Timur-B04825]

Shawn Guo wrote:
> Are you saying that both older and new P1022 device trees have
> compatible = "fsl,P1022DS", so that changing to check /compatible
> could save the check on /model of P1022 device tree?

Yes, that's what I'm saying.

You can verify for yourself by looking at the git history of the device trees.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH 6/6] ASoC: fsl: check property 'compatible' for the machine name

[Shawn Guo]

On Tue, Feb 28, 2012 at 03:42:57AM +0000, Tabi Timur-B04825 wrote:
> Shawn Guo wrote:
> > Are you saying that both older and new P1022 device trees have
> > compatible = "fsl,P1022DS", so that changing to check /compatible
> > could save the check on /model of P1022 device tree?
> 
> Yes, that's what I'm saying.
> 
Ok, I will drop "[PATCH 5/6] ASoC: fsl: use platform_device_id table
to match p1022_ds device" and move to this then.

-- 
Regards,
Shawn

[PATCH 0/6] ASoC: a few cleanups on sound/soc/fsl

[Timur Tabi]

Shawn Guo wrote:
> This is the third series towards reusing fsl_ssi driver on device tree
> based IMX platforms.  It makes a few cleanups on existing PowerPC code
> to ease the later addition of an IMX machine driver that works on
> fsl_ssi.

These patches don't apply 100% cleanly, and there are some issues that
need to be resolved, but I'll review each one and ACK those that I think
are okay.  It would probably be better if none were applied until the
whole patchset is okay, though.

-- 
Timur Tabi
Linux kernel developer at Freescale