[PATCH 1/5] [v3] ASoC: fsl: use snd_soc_register_card to register the card

[PATCH 1/5] [v3] ASoC: fsl: use snd_soc_register_card to register the card

[Timur Tabi]

Use snd_soc_register_card() instead of platform_device_alloc("soc-audio")
to register the sound card from the machine drivers.  The use of
platform_device_alloc is deprecated.

Although several other drivers still use platform_device_alloc(), the
Freescale drivers were not using it to pass driver data.  Instead of fixing
the driver data usage, it's better to replace the deprecated code.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 sound/soc/fsl/mpc8610_hpcd.c |   32 +++++++-------------------------
 sound/soc/fsl/p1022_ds.c     |   31 ++++++-------------------------
 2 files changed, 13 insertions(+), 50 deletions(-)

diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c
index 60bcba1..9ff9318 100644
--- a/sound/soc/fsl/mpc8610_hpcd.c
+++ b/sound/soc/fsl/mpc8610_hpcd.c
@@ -192,7 +192,6 @@ static int mpc8610_hpcd_probe(struct platform_device *pdev)
 		container_of(dev, struct platform_device, dev);
 	struct device_node *np = ssi_pdev->dev.of_node;
 	struct device_node *codec_np = NULL;
-	struct platform_device *sound_device = NULL;
 	struct mpc8610_hpcd_data *machine_data;
 	int ret = -ENODEV;
 	const char *sprop;
@@ -341,34 +340,22 @@ static int mpc8610_hpcd_probe(struct platform_device *pdev)
 	machine_data->card.probe = mpc8610_hpcd_machine_probe;
 	machine_data->card.remove = mpc8610_hpcd_machine_remove;
 	machine_data->card.name = pdev->name; /* The platform driver name */
+	machine_data->card.owner = THIS_MODULE;
+	machine_data->card.dev = &pdev->dev;
 	machine_data->card.num_links = 2;
 	machine_data->card.dai_link = machine_data->dai;
 
-	/* Allocate a new audio platform device structure */
-	sound_device = platform_device_alloc("soc-audio", -1);
-	if (!sound_device) {
-		dev_err(&pdev->dev, "platform device alloc failed\n");
-		ret = -ENOMEM;
-		goto error;
-	}
-
-	/* Associate the card data with the sound device */
-	platform_set_drvdata(sound_device, &machine_data->card);
-
 	/* Register with ASoC */
-	ret = platform_device_add(sound_device);
+	ret = snd_soc_register_card(&machine_data->card);
 	if (ret) {
-		dev_err(&pdev->dev, "platform device add failed\n");
-		goto error_sound;
+		dev_err(&pdev->dev, "could not register card\n");
+		goto error;
 	}
-	dev_set_drvdata(&pdev->dev, sound_device);
 
 	of_node_put(codec_np);
 
 	return 0;
 
-error_sound:
-	platform_device_put(sound_device);
 error:
 	kfree(machine_data);
 error_alloc:
@@ -383,17 +370,12 @@ error_alloc:
  */
 static int __devexit mpc8610_hpcd_remove(struct platform_device *pdev)
 {
-	struct platform_device *sound_device = dev_get_drvdata(&pdev->dev);
-	struct snd_soc_card *card = platform_get_drvdata(sound_device);
+	struct snd_soc_card *card = platform_get_drvdata(pdev);
 	struct mpc8610_hpcd_data *machine_data =
 		container_of(card, struct mpc8610_hpcd_data, card);
 
-	platform_device_unregister(sound_device);
-
+	snd_soc_unregister_card(card);
 	kfree(machine_data);
-	sound_device->dev.platform_data = NULL;
-
-	dev_set_drvdata(&pdev->dev, NULL);
 
 	return 0;
 }
diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c
index 50adf40..144d496 100644
--- a/sound/soc/fsl/p1022_ds.c
+++ b/sound/soc/fsl/p1022_ds.c
@@ -202,7 +202,6 @@ static int p1022_ds_probe(struct platform_device *pdev)
 		container_of(dev, struct platform_device, dev);
 	struct device_node *np = ssi_pdev->dev.of_node;
 	struct device_node *codec_np = NULL;
-	struct platform_device *sound_device = NULL;
 	struct machine_data *mdata;
 	int ret = -ENODEV;
 	const char *sprop;
@@ -349,36 +348,23 @@ static int p1022_ds_probe(struct platform_device *pdev)
 	mdata->card.probe = p1022_ds_machine_probe;
 	mdata->card.remove = p1022_ds_machine_remove;
 	mdata->card.name = pdev->name; /* The platform driver name */
+	mdata->card.owner = THIS_MODULE;
+	mdata->card.dev = &pdev->dev;
 	mdata->card.num_links = 2;
 	mdata->card.dai_link = mdata->dai;
 
-	/* Allocate a new audio platform device structure */
-	sound_device = platform_device_alloc("soc-audio", -1);
-	if (!sound_device) {
-		dev_err(&pdev->dev, "platform device alloc failed\n");
-		ret = -ENOMEM;
-		goto error;
-	}
-
-	/* Associate the card data with the sound device */
-	platform_set_drvdata(sound_device, &mdata->card);
-
 	/* Register with ASoC */
-	ret = platform_device_add(sound_device);
+	ret = snd_soc_register_card(&mdata->card);
 	if (ret) {
-		dev_err(&pdev->dev, "platform device add failed\n");
+		dev_err(&pdev->dev, "could not register card\n");
 		goto error;
 	}
-	dev_set_drvdata(&pdev->dev, sound_device);
 
 	of_node_put(codec_np);
 
 	return 0;
 
 error:
-	if (sound_device)
-		platform_device_put(sound_device);
-
 	kfree(mdata);
 error_put:
 	of_node_put(codec_np);
@@ -392,17 +378,12 @@ error_put:
  */
 static int __devexit p1022_ds_remove(struct platform_device *pdev)
 {
-	struct platform_device *sound_device = dev_get_drvdata(&pdev->dev);
-	struct snd_soc_card *card = platform_get_drvdata(sound_device);
+	struct snd_soc_card *card = platform_get_drvdata(pdev);
 	struct machine_data *mdata =
 		container_of(card, struct machine_data, card);
 
-	platform_device_unregister(sound_device);
-
+	snd_soc_unregister_card(card);
 	kfree(mdata);
-	sound_device->dev.platform_data = NULL;
-
-	dev_set_drvdata(&pdev->dev, NULL);
 
 	return 0;
 }
-- 
1.7.3.4

[PATCH 2/5] ASoC: fsl: move machine drivers to late_initcall()

[Timur Tabi]

Transition the module initialization to late_initcall(), so that
the machine driver loads last.  Although this is technically not
necessary, it does avoid this error message at boot time:

	ALSA device list:
	  No soundcards found.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 sound/soc/fsl/mpc8610_hpcd.c |    2 +-
 sound/soc/fsl/p1022_ds.c     |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c
index 9ff9318..cc45086 100644
--- a/sound/soc/fsl/mpc8610_hpcd.c
+++ b/sound/soc/fsl/mpc8610_hpcd.c
@@ -425,7 +425,7 @@ static void __exit mpc8610_hpcd_exit(void)
 	platform_driver_unregister(&mpc8610_hpcd_driver);
 }
 
-module_init(mpc8610_hpcd_init);
+late_initcall(mpc8610_hpcd_init);
 module_exit(mpc8610_hpcd_exit);
 
 MODULE_AUTHOR("Timur Tabi <timur@freescale.com>");
diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c
index 144d496..be7db47 100644
--- a/sound/soc/fsl/p1022_ds.c
+++ b/sound/soc/fsl/p1022_ds.c
@@ -434,7 +434,7 @@ static void __exit p1022_ds_exit(void)
 	platform_driver_unregister(&p1022_ds_driver);
 }
 
-module_init(p1022_ds_init);
+late_initcall(p1022_ds_init);
 module_exit(p1022_ds_exit);
 
 MODULE_AUTHOR("Timur Tabi <timur@freescale.com>");
-- 
1.7.3.4

Re: [PATCH 2/5] ASoC: fsl: move machine drivers to late_initcall()

[Mark Brown]

On Fri, Sep 14, 2012 at 04:14:35PM -0500, Timur Tabi wrote:
> Transition the module initialization to late_initcall(), so that
> the machine driver loads last.  Although this is technically not
> necessary, it does avoid this error message at boot time:
> 
> 	ALSA device list:
> 	  No soundcards found.

As I said previously I think this needs a change that applies to all
drivers, like I say removing the above print seems the most sensible one
to me.

[PATCH 3/5] ASoC: fsl: remove unnecessary call to dma_unmap_single

[Timur Tabi]

Remove a call to dma_unmap_single() from the PowerPC ASoC DMA driver.  The
buffer is allocated and not actually mapped, so the unmap call doesn't
make sense.  It was probably left over from some early version of the driver.

This bug was unnoticed for so long because the DMA mapping functions normally
don't do anything on PowerPC.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 sound/soc/fsl/fsl_dma.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c
index 96bb92d..6feb265 100644
--- a/sound/soc/fsl/fsl_dma.c
+++ b/sound/soc/fsl/fsl_dma.c
@@ -823,12 +823,6 @@ static int fsl_dma_close(struct snd_pcm_substream *substream)
 		if (dma_private->irq)
 			free_irq(dma_private->irq, dma_private);
 
-		if (dma_private->ld_buf_phys) {
-			dma_unmap_single(dev, dma_private->ld_buf_phys,
-					 sizeof(dma_private->link),
-					 DMA_TO_DEVICE);
-		}
-
 		/* Deallocate the fsl_dma_private structure */
 		dma_free_coherent(dev, sizeof(struct fsl_dma_private),
 				  dma_private, dma_private->ld_buf_phys);
-- 
1.7.3.4

Re: [PATCH 3/5] ASoC: fsl: remove unnecessary call to dma_unmap_single

[Mark Brown]

On Fri, Sep 14, 2012 at 04:14:36PM -0500, Timur Tabi wrote:
> Remove a call to dma_unmap_single() from the PowerPC ASoC DMA driver.  The
> buffer is allocated and not actually mapped, so the unmap call doesn't
> make sense.  It was probably left over from some early version of the driver.
> 
> This bug was unnoticed for so long because the DMA mapping functions normally
> don't do anything on PowerPC.

Applied, thanks.

[PATCH 4/5] ASoC: wm8960: add support for big-endian audio samples

[Timur Tabi]

PowerPC ASoC drivers frequently use the _BE variants of the SNDRV_PCM_FORMAT
macros, so we need to look for those as well.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 sound/soc/codecs/wm8960.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
index 8740423..782faa0 100644
--- a/sound/soc/codecs/wm8960.c
+++ b/sound/soc/codecs/wm8960.c
@@ -557,18 +557,25 @@ static int wm8960_hw_params(struct snd_pcm_substream *substream,
 	struct snd_soc_codec *codec = dai->codec;
 	struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec);
 	u16 iface = snd_soc_read(codec, WM8960_IFACE1) & 0xfff3;
+	snd_pcm_format_t format = params_format(params);
 	int i;
 
 	/* bit size */
-	switch (params_format(params)) {
+	switch (format) {
 	case SNDRV_PCM_FORMAT_S16_LE:
+	case SNDRV_PCM_FORMAT_S16_BE:
 		break;
 	case SNDRV_PCM_FORMAT_S20_3LE:
+	case SNDRV_PCM_FORMAT_S20_3BE:
 		iface |= 0x0004;
 		break;
 	case SNDRV_PCM_FORMAT_S24_LE:
+	case SNDRV_PCM_FORMAT_S24_BE:
 		iface |= 0x0008;
 		break;
+	default:
+		dev_err(codec->dev, "unsupported format %i\n", format);
+		return -EINVAL;
 	}
 
 	/* Update filters for the new rate */
-- 
1.7.3.4

Re: [PATCH 4/5] ASoC: wm8960: add support for big-endian audio samples

[Mark Brown]

On Fri, Sep 14, 2012 at 04:14:37PM -0500, Timur Tabi wrote:
> PowerPC ASoC drivers frequently use the _BE variants of the SNDRV_PCM_FORMAT
> macros, so we need to look for those as well.

Applied but...

> +	snd_pcm_format_t format = params_format(params);
>  	int i;
>  
>  	/* bit size */
> -	switch (params_format(params)) {
> +	switch (format) {

...please don't introduce unrelated changes, especially ones you don't
mention in the changelog.

Re: [PATCH 4/5] ASoC: wm8960: add support for big-endian audio samples

[Tabi Timur-B04825]

Mark Brown wrote:
>> >+	snd_pcm_format_t format = params_format(params);
>> >  	int i;
>> >
>> >  	/* bit size */
>> >-	switch (params_format(params)) {
>> >+	switch (format) {
> ...please don't introduce unrelated changes, especially ones you don't
> mention in the changelog.

Well, the introduction of variable 'format' is for the default case in the 
switch statement:

+       default:
+               dev_err(codec->dev, "unsupported format %i\n", format);
+               return -EINVAL;

I would not call this change "unrelated".

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH 4/5] ASoC: wm8960: add support for big-endian audio samples

[Mark Brown]

On Wed, Sep 19, 2012 at 02:58:29AM +0000, Tabi Timur-B04825 wrote:
> Mark Brown wrote:

> > ...please don't introduce unrelated changes, especially ones you don't
> > mention in the changelog.

> Well, the introduction of variable 'format' is for the default case in the 
> switch statement:

I saw that, that's why I didn't write "random stylistic changes" (as I
had originally, I nearly discarded the patch before I read that far).

> +       default:
> +               dev_err(codec->dev, "unsupported format %i\n", format);
> +               return -EINVAL;

> I would not call this change "unrelated".

It's not adding a new format, it's changing the error reporting.  That's
a sensible thing to do and could have been done as part of the same
patch but it should've been mentioned in the changelog.

[PATCH 5/5] ASoC: add support for the Freescale / iVeia P1022 RDK reference board

[Timur Tabi]

This small reference boards has a Freescale P1022 dual-core PowerPC SOC
and a Wolfson Microelectronics WM8960 codec.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 sound/soc/fsl/Kconfig     |   14 ++
 sound/soc/fsl/Makefile    |    4 +
 sound/soc/fsl/p1022_rdk.c |  455 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 473 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/fsl/p1022_rdk.c

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index d701330..c6f582f 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -46,6 +46,20 @@ config SND_SOC_P1022_DS
 	  This will also include the Wolfson Microelectronics WM8776 codec
 	  driver.
 
+config SND_SOC_P1022_RDK
+	tristate "ALSA SoC support for the Freescale / iVeia P1022 RDK board"
+	# I2C is necessary for the WM8960 driver
+	depends on P1022_RDK && I2C
+	select SND_SOC_FSL_SSI
+	select SND_SOC_FSL_UTILS
+	select SND_SOC_POWERPC_DMA
+	select SND_SOC_WM8960
+	default y if P1022_RDK
+	help
+	  Say Y if you want to enable audio on the Freescale / iVeia
+	  P1022 RDK board.  This will also include the Wolfson
+	  Microelectronics WM8960 codec driver.
+
 config SND_SOC_MPC5200_I2S
 	tristate "Freescale MPC5200 PSC in I2S mode driver"
 	depends on PPC_MPC52xx && PPC_BESTCOMM
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
index 5f3cf3f..515ba66 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -6,6 +6,10 @@ obj-$(CONFIG_SND_SOC_MPC8610_HPCD) += snd-soc-mpc8610-hpcd.o
 snd-soc-p1022-ds-objs := p1022_ds.o
 obj-$(CONFIG_SND_SOC_P1022_DS) += snd-soc-p1022-ds.o
 
+# P1022 RDK Machine Support
+snd-soc-p1022-rdk-objs := p1022_rdk.o
+obj-$(CONFIG_SND_SOC_P1022_RDK) += snd-soc-p1022-rdk.o
+
 # Freescale PowerPC SSI/DMA Platform Support
 snd-soc-fsl-ssi-objs := fsl_ssi.o
 snd-soc-fsl-utils-objs := fsl_utils.o
diff --git a/sound/soc/fsl/p1022_rdk.c b/sound/soc/fsl/p1022_rdk.c
new file mode 100644
index 0000000..82d49f5
--- /dev/null
+++ b/sound/soc/fsl/p1022_rdk.c
@@ -0,0 +1,455 @@
+/**
+ * Freescale P1022RDK ALSA SoC Machine driver
+ *
+ * Author: Timur Tabi <timur@freescale.com>
+ *
+ * Copyright 2012 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.
+ *
+ * Note: in order for audio to work correctly, the output controls need
+ * to be enabled, because they control the clock.  So for playback, for
+ * example:
+ *
+ *      amixer sset 'Left Output Mixer PCM' on
+ *      amixer sset 'Right Output Mixer PCM' on
+ */
+
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/of_device.h>
+#include <linux/slab.h>
+#include <sound/soc.h>
+#include <asm/fsl_guts.h>
+
+#include "fsl_dma.h"
+#include "fsl_ssi.h"
+#include "fsl_utils.h"
+
+#include "../codecs/wm8960.h"
+
+/* P1022-specific PMUXCR and DMUXCR bit definitions */
+
+#define CCSR_GUTS_PMUXCR_UART0_I2C1_MASK	0x0001c000
+#define CCSR_GUTS_PMUXCR_UART0_I2C1_UART0_SSI	0x00010000
+#define CCSR_GUTS_PMUXCR_UART0_I2C1_SSI		0x00018000
+
+#define CCSR_GUTS_PMUXCR_SSI_DMA_TDM_MASK	0x00000c00
+#define CCSR_GUTS_PMUXCR_SSI_DMA_TDM_SSI	0x00000000
+
+#define CCSR_GUTS_DMUXCR_PAD	1	/* DMA controller/channel set to pad */
+#define CCSR_GUTS_DMUXCR_SSI	2	/* DMA controller/channel set to SSI */
+
+/*
+ * Set the DMACR register in the GUTS
+ *
+ * The DMACR register determines the source of initiated transfers for each
+ * channel on each DMA controller.  Rather than have a bunch of repetitive
+ * macros for the bit patterns, we just have a function that calculates
+ * them.
+ *
+ * guts: Pointer to GUTS structure
+ * co: The DMA controller (0 or 1)
+ * ch: The channel on the DMA controller (0, 1, 2, or 3)
+ * device: The device to set as the target (CCSR_GUTS_DMUXCR_xxx)
+ */
+static inline void guts_set_dmuxcr(struct ccsr_guts __iomem *guts,
+	unsigned int co, unsigned int ch, unsigned int device)
+{
+	unsigned int shift = 16 + (8 * (1 - co) + 2 * (3 - ch));
+
+	clrsetbits_be32(&guts->dmuxcr, 3 << shift, device << shift);
+}
+
+/* There's only one global utilities register */
+static phys_addr_t guts_phys;
+
+/**
+ * machine_data: machine-specific ASoC device data
+ *
+ * This structure contains data for a single sound platform device on an
+ * P1022 RDK.  Some of the data is taken from the device tree.
+ */
+struct machine_data {
+	struct snd_soc_dai_link dai[2];
+	struct snd_soc_card card;
+	unsigned int dai_format;
+	unsigned int codec_clk_direction;
+	unsigned int cpu_clk_direction;
+	unsigned int clk_frequency;
+	unsigned int ssi_id;		/* 0 = SSI1, 1 = SSI2, etc */
+	unsigned int dma_id[2];		/* 0 = DMA1, 1 = DMA2, etc */
+	unsigned int dma_channel_id[2]; /* 0 = ch 0, 1 = ch 1, etc*/
+	char platform_name[2][DAI_NAME_SIZE]; /* One for each DMA channel */
+};
+
+/**
+ * p1022_rdk_machine_probe: initialize the board
+ *
+ * This function is used to initialize the board-specific hardware.
+ *
+ * Here we program the DMACR and PMUXCR registers.
+ */
+static int p1022_rdk_machine_probe(struct snd_soc_card *card)
+{
+	struct machine_data *mdata =
+		container_of(card, struct machine_data, card);
+	struct ccsr_guts __iomem *guts;
+
+	guts = ioremap(guts_phys, sizeof(struct ccsr_guts));
+	if (!guts) {
+		dev_err(card->dev, "could not map global utilities\n");
+		return -ENOMEM;
+	}
+
+	/* Enable SSI Tx signal */
+	clrsetbits_be32(&guts->pmuxcr, CCSR_GUTS_PMUXCR_UART0_I2C1_MASK,
+			CCSR_GUTS_PMUXCR_UART0_I2C1_UART0_SSI);
+
+	/* Enable SSI Rx signal */
+	clrsetbits_be32(&guts->pmuxcr, CCSR_GUTS_PMUXCR_SSI_DMA_TDM_MASK,
+			CCSR_GUTS_PMUXCR_SSI_DMA_TDM_SSI);
+
+	/* Enable DMA Channel for SSI */
+	guts_set_dmuxcr(guts, mdata->dma_id[0], mdata->dma_channel_id[0],
+			CCSR_GUTS_DMUXCR_SSI);
+
+	guts_set_dmuxcr(guts, mdata->dma_id[1], mdata->dma_channel_id[1],
+			CCSR_GUTS_DMUXCR_SSI);
+
+	iounmap(guts);
+
+	return 0;
+}
+
+/**
+ * p1022_rdk_startup: program the board with various hardware parameters
+ *
+ * This function takes board-specific information, like clock frequencies
+ * and serial data formats, and passes that information to the codec and
+ * transport drivers.
+ */
+static int p1022_rdk_startup(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct machine_data *mdata =
+		container_of(rtd->card, struct machine_data, card);
+	struct device *dev = rtd->card->dev;
+	int ret = 0;
+
+	/* Tell the codec driver what the serial protocol is. */
+	ret = snd_soc_dai_set_fmt(rtd->codec_dai, mdata->dai_format);
+	if (ret < 0) {
+		dev_err(dev, "could not set codec driver audio format\n");
+		return ret;
+	}
+
+	ret = snd_soc_dai_set_pll(rtd->codec_dai, 0, 0, mdata->clk_frequency,
+		mdata->clk_frequency);
+	if (ret < 0) {
+		dev_err(dev, "could not set codec PLL frequency\n");
+		return ret;
+	}
+
+	/* Tell the WM8960 to set SYSCLK equal to MCLK input (bypass PLL) */
+	ret = snd_soc_dai_set_clkdiv(rtd->codec_dai, WM8960_SYSCLKDIV,
+		WM8960_SYSCLK_MCLK | WM8960_SYSCLK_DIV_1);
+	if (ret < 0) {
+		dev_err(dev, "could not set codec driver clock source\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * p1022_rdk_machine_remove: Remove the sound device
+ *
+ * This function is called to remove the sound device for one SSI.  We
+ * de-program the DMACR and PMUXCR register.
+ */
+static int p1022_rdk_machine_remove(struct snd_soc_card *card)
+{
+	struct machine_data *mdata =
+		container_of(card, struct machine_data, card);
+	struct ccsr_guts __iomem *guts;
+
+	guts = ioremap(guts_phys, sizeof(struct ccsr_guts));
+	if (!guts) {
+		dev_err(card->dev, "could not map global utilities\n");
+		return -ENOMEM;
+	}
+
+	/* Restore the signal routing */
+	clrbits32(&guts->pmuxcr, CCSR_GUTS_PMUXCR_UART0_I2C1_MASK);
+	clrbits32(&guts->pmuxcr, CCSR_GUTS_PMUXCR_SSI_DMA_TDM_MASK);
+	guts_set_dmuxcr(guts, mdata->dma_id[0], mdata->dma_channel_id[0], 0);
+	guts_set_dmuxcr(guts, mdata->dma_id[1], mdata->dma_channel_id[1], 0);
+
+	iounmap(guts);
+
+	return 0;
+}
+
+/**
+ * p1022_rdk_ops: ASoC machine driver operations
+ */
+static struct snd_soc_ops p1022_rdk_ops = {
+	.startup = p1022_rdk_startup,
+};
+
+/**
+ * p1022_rdk_probe: platform probe function for the machine driver
+ *
+ * Although this is a machine driver, the SSI node is the "master" node with
+ * respect to audio hardware connections.  Therefore, we create a new ASoC
+ * device for each new SSI node that has a codec attached.
+ */
+static int p1022_rdk_probe(struct platform_device *pdev)
+{
+	struct device *dev = pdev->dev.parent;
+	/* ssi_pdev is the platform device for the SSI node that probed us */
+	struct platform_device *ssi_pdev =
+		container_of(dev, struct platform_device, dev);
+	struct device_node *np = ssi_pdev->dev.of_node;
+	struct device_node *codec_np = NULL;
+	struct machine_data *mdata;
+	int ret = -ENODEV;
+	const char *sprop;
+	const u32 *iprop;
+
+	/* Find the codec node for this SSI. */
+	codec_np = of_parse_phandle(np, "codec-handle", 0);
+	if (!codec_np) {
+		dev_err(dev, "could not find codec node\n");
+		return -EINVAL;
+	}
+
+	mdata = kzalloc(sizeof(struct machine_data), GFP_KERNEL);
+	if (!mdata) {
+		ret = -ENOMEM;
+		goto error_put;
+	}
+
+	mdata->dai[0].cpu_dai_name = dev_name(&ssi_pdev->dev);
+	mdata->dai[0].ops = &p1022_rdk_ops;
+
+	/* ASoC core can match codec with device node */
+	mdata->dai[0].codec_of_node = codec_np;
+
+	/* We register two DAIs per SSI, one for playback and the other for
+	 * capture.  We support codecs that have separate DAIs for both playback
+	 * and capture.
+	 */
+	memcpy(&mdata->dai[1], &mdata->dai[0], sizeof(struct snd_soc_dai_link));
+
+	/* The DAI names from the codec (snd_soc_dai_driver.name) */
+	mdata->dai[0].codec_dai_name = "wm8960-hifi";
+	mdata->dai[1].codec_dai_name = mdata->dai[0].codec_dai_name;
+
+	/* Get the device ID */
+	iprop = of_get_property(np, "cell-index", NULL);
+	if (!iprop) {
+		dev_err(&pdev->dev, "cell-index property not found\n");
+		ret = -EINVAL;
+		goto error;
+	}
+	mdata->ssi_id = be32_to_cpup(iprop);
+
+	/* Get the serial format and clock direction. */
+	sprop = of_get_property(np, "fsl,mode", NULL);
+	if (!sprop) {
+		dev_err(&pdev->dev, "fsl,mode property not found\n");
+		ret = -EINVAL;
+		goto error;
+	}
+
+	if (strcasecmp(sprop, "i2s-slave") == 0) {
+		mdata->dai_format = SND_SOC_DAIFMT_NB_NF |
+			SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM;
+		mdata->codec_clk_direction = SND_SOC_CLOCK_OUT;
+		mdata->cpu_clk_direction = SND_SOC_CLOCK_IN;
+
+		/* In i2s-slave mode, the codec has its own clock source, so we
+		 * need to get the frequency from the device tree and pass it to
+		 * the codec driver.
+		 */
+		iprop = of_get_property(codec_np, "clock-frequency", NULL);
+		if (!iprop || !*iprop) {
+			dev_err(&pdev->dev, "codec bus-frequency "
+				"property is missing or invalid\n");
+			ret = -EINVAL;
+			goto error;
+		}
+		mdata->clk_frequency = be32_to_cpup(iprop);
+	} else if (strcasecmp(sprop, "i2s-master") == 0) {
+		mdata->dai_format = SND_SOC_DAIFMT_NB_NF |
+			SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBS_CFS;
+		mdata->codec_clk_direction = SND_SOC_CLOCK_IN;
+		mdata->cpu_clk_direction = SND_SOC_CLOCK_OUT;
+	} else if (strcasecmp(sprop, "lj-slave") == 0) {
+		mdata->dai_format = SND_SOC_DAIFMT_NB_NF |
+			SND_SOC_DAIFMT_LEFT_J | SND_SOC_DAIFMT_CBM_CFM;
+		mdata->codec_clk_direction = SND_SOC_CLOCK_OUT;
+		mdata->cpu_clk_direction = SND_SOC_CLOCK_IN;
+	} else if (strcasecmp(sprop, "lj-master") == 0) {
+		mdata->dai_format = SND_SOC_DAIFMT_NB_NF |
+			SND_SOC_DAIFMT_LEFT_J | SND_SOC_DAIFMT_CBS_CFS;
+		mdata->codec_clk_direction = SND_SOC_CLOCK_IN;
+		mdata->cpu_clk_direction = SND_SOC_CLOCK_OUT;
+	} else if (strcasecmp(sprop, "rj-slave") == 0) {
+		mdata->dai_format = SND_SOC_DAIFMT_NB_NF |
+			SND_SOC_DAIFMT_RIGHT_J | SND_SOC_DAIFMT_CBM_CFM;
+		mdata->codec_clk_direction = SND_SOC_CLOCK_OUT;
+		mdata->cpu_clk_direction = SND_SOC_CLOCK_IN;
+	} else if (strcasecmp(sprop, "rj-master") == 0) {
+		mdata->dai_format = SND_SOC_DAIFMT_NB_NF |
+			SND_SOC_DAIFMT_RIGHT_J | SND_SOC_DAIFMT_CBS_CFS;
+		mdata->codec_clk_direction = SND_SOC_CLOCK_IN;
+		mdata->cpu_clk_direction = SND_SOC_CLOCK_OUT;
+	} else if (strcasecmp(sprop, "ac97-slave") == 0) {
+		mdata->dai_format = SND_SOC_DAIFMT_NB_NF |
+			SND_SOC_DAIFMT_AC97 | SND_SOC_DAIFMT_CBM_CFM;
+		mdata->codec_clk_direction = SND_SOC_CLOCK_OUT;
+		mdata->cpu_clk_direction = SND_SOC_CLOCK_IN;
+	} else if (strcasecmp(sprop, "ac97-master") == 0) {
+		mdata->dai_format = SND_SOC_DAIFMT_NB_NF |
+			SND_SOC_DAIFMT_AC97 | SND_SOC_DAIFMT_CBS_CFS;
+		mdata->codec_clk_direction = SND_SOC_CLOCK_IN;
+		mdata->cpu_clk_direction = SND_SOC_CLOCK_OUT;
+	} else {
+		dev_err(&pdev->dev,
+			"unrecognized fsl,mode property '%s'\n", sprop);
+		ret = -EINVAL;
+		goto error;
+	}
+
+	if (!mdata->clk_frequency) {
+		dev_err(&pdev->dev, "unknown clock frequency\n");
+		ret = -EINVAL;
+		goto error;
+	}
+
+	/* Find the playback DMA channel to use. */
+	mdata->dai[0].platform_name = mdata->platform_name[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;
+	}
+
+	/* Find the capture DMA channel to use. */
+	mdata->dai[1].platform_name = mdata->platform_name[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;
+	}
+
+	/* Initialize our DAI data structure.  */
+	mdata->dai[0].stream_name = "playback";
+	mdata->dai[1].stream_name = "capture";
+	mdata->dai[0].name = mdata->dai[0].stream_name;
+	mdata->dai[1].name = mdata->dai[1].stream_name;
+
+	mdata->card.probe = p1022_rdk_machine_probe;
+	mdata->card.remove = p1022_rdk_machine_remove;
+	mdata->card.name = pdev->name; /* The platform driver name */
+	mdata->card.owner = THIS_MODULE;
+	mdata->card.dev = &pdev->dev;
+	mdata->card.num_links = 2;
+	mdata->card.dai_link = mdata->dai;
+
+	/* Register with ASoC */
+	ret = snd_soc_register_card(&mdata->card);
+	if (ret) {
+		dev_err(&pdev->dev, "could not register card\n");
+		goto error;
+	}
+
+	of_node_put(codec_np);
+
+	return 0;
+
+error:
+	kfree(mdata);
+error_put:
+	of_node_put(codec_np);
+	return ret;
+}
+
+/**
+ * p1022_rdk_remove: remove the platform device
+ *
+ * This function is called when the platform device is removed.
+ */
+static int __devexit p1022_rdk_remove(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = platform_get_drvdata(pdev);
+	struct machine_data *mdata =
+		container_of(card, struct machine_data, card);
+
+	snd_soc_unregister_card(card);
+	kfree(mdata);
+
+	return 0;
+}
+
+static struct platform_driver p1022_rdk_driver = {
+	.probe = p1022_rdk_probe,
+	.remove = __devexit_p(p1022_rdk_remove),
+	.driver = {
+		/*
+		 * The name must match 'compatible' property in the device tree,
+		 * in lowercase letters.
+		 */
+		.name = "snd-soc-p1022rdk",
+		.owner = THIS_MODULE,
+	},
+};
+
+/**
+ * p1022_rdk_init: machine driver initialization.
+ *
+ * This function is called when this module is loaded.
+ */
+static int __init p1022_rdk_init(void)
+{
+	struct device_node *guts_np;
+	struct resource res;
+
+	/* Get the physical address of the global utilities registers */
+	guts_np = of_find_compatible_node(NULL, NULL, "fsl,p1022-guts");
+	if (of_address_to_resource(guts_np, 0, &res)) {
+		pr_err("snd-soc-p1022rdk: missing/invalid global utils node\n");
+		of_node_put(guts_np);
+		return -EINVAL;
+	}
+	guts_phys = res.start;
+	of_node_put(guts_np);
+
+	return platform_driver_register(&p1022_rdk_driver);
+}
+
+/**
+ * p1022_rdk_exit: machine driver exit
+ *
+ * This function is called when this driver is unloaded.
+ */
+static void __exit p1022_rdk_exit(void)
+{
+	platform_driver_unregister(&p1022_rdk_driver);
+}
+
+late_initcall(p1022_rdk_init);
+module_exit(p1022_rdk_exit);
+
+MODULE_AUTHOR("Timur Tabi <timur@freescale.com>");
+MODULE_DESCRIPTION("Freescale / iVeia P1022 RDK ALSA SoC machine driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.3.4

Re: [PATCH 5/5] ASoC: add support for the Freescale / iVeia P1022 RDK reference board

[Mark Brown]

On Fri, Sep 14, 2012 at 04:14:38PM -0500, Timur Tabi wrote:

>  sound/soc/fsl/Kconfig     |   14 ++
>  sound/soc/fsl/Makefile    |    4 +
>  sound/soc/fsl/p1022_rdk.c |  455 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 473 insertions(+), 0 deletions(-)
>  create mode 100644 sound/soc/fsl/p1022_rdk.c

This adds a new device tree binding so it should be documented.

> +	/* Tell the codec driver what the serial protocol is. */
> +	ret = snd_soc_dai_set_fmt(rtd->codec_dai, mdata->dai_format);
> +	if (ret < 0) {
> +		dev_err(dev, "could not set codec driver audio format\n");
> +		return ret;
> +	}

This should normally be done via the dai_fmt member of the dai_link
structure.

> +	ret = snd_soc_dai_set_pll(rtd->codec_dai, 0, 0, mdata->clk_frequency,
> +		mdata->clk_frequency);
> +	if (ret < 0) {
> +		dev_err(dev, "could not set codec PLL frequency\n");
> +		return ret;
> +	}

We never stop the PLL - normally you'd want to stop it when the audio
stops.  But..

> +	/* Tell the WM8960 to set SYSCLK equal to MCLK input (bypass PLL) */
> +	ret = snd_soc_dai_set_clkdiv(rtd->codec_dai, WM8960_SYSCLKDIV,
> +		WM8960_SYSCLK_MCLK | WM8960_SYSCLK_DIV_1);
> +	if (ret < 0) {
> +		dev_err(dev, "could not set codec driver clock source\n");
> +		return ret;
> +	}

...we don't appear to use the PLL output anyway so why not just leave it
off?  As this is fixed it's better style to do the setup in late_init()
or the init function for the DAI link.

Also we should log the return codes when we log failures.

> +	/* Get the serial format and clock direction. */
> +	sprop = of_get_property(np, "fsl,mode", NULL);
> +	if (!sprop) {
> +		dev_err(&pdev->dev, "fsl,mode property not found\n");
> +		ret = -EINVAL;
> +		goto error;
> +	}
> +
> +	if (strcasecmp(sprop, "i2s-slave") == 0) {
> +		mdata->dai_format = SND_SOC_DAIFMT_NB_NF |
> +			SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM;
> +		mdata->codec_clk_direction = SND_SOC_CLOCK_OUT;
> +		mdata->cpu_clk_direction = SND_SOC_CLOCK_IN;

Why would this be something that individual systems would want to
change, and if it is something it's useful to vary why not at runtime?
I suspect the best thing here is just to pick a format and use it, if
there is a reason to vary this it's probably a higher level thing.

> +	snd_soc_unregister_card(card);
> +	kfree(mdata);

devm_kzalloc().

Re: [PATCH 5/5] ASoC: add support for the Freescale / iVeia P1022 RDK reference board

[Tabi Timur-B04825]

Mark Brown wrote:
> On Fri, Sep 14, 2012 at 04:14:38PM -0500, Timur Tabi wrote:
>
>>   sound/soc/fsl/Kconfig     |   14 ++
>>   sound/soc/fsl/Makefile    |    4 +
>>   sound/soc/fsl/p1022_rdk.c |  455 +++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 473 insertions(+), 0 deletions(-)
>>   create mode 100644 sound/soc/fsl/p1022_rdk.c
>
> This adds a new device tree binding so it should be documented.

It does?  Where?  It uses the same binding as p1022_ds.c.

>> +	/* Tell the codec driver what the serial protocol is. */
>> +	ret = snd_soc_dai_set_fmt(rtd->codec_dai, mdata->dai_format);
>> +	if (ret < 0) {
>> +		dev_err(dev, "could not set codec driver audio format\n");
>> +		return ret;
>> +	}
>
> This should normally be done via the dai_fmt member of the dai_link
> structure.

Ok, that's new.  The other PowerPC SSI drivers do it this way.

>> +	ret = snd_soc_dai_set_pll(rtd->codec_dai, 0, 0, mdata->clk_frequency,
>> +		mdata->clk_frequency);
>> +	if (ret < 0) {
>> +		dev_err(dev, "could not set codec PLL frequency\n");
>> +		return ret;
>> +	}
>
> We never stop the PLL - normally you'd want to stop it when the audio
> stops.  But..
>
>> +	/* Tell the WM8960 to set SYSCLK equal to MCLK input (bypass PLL) */
>> +	ret = snd_soc_dai_set_clkdiv(rtd->codec_dai, WM8960_SYSCLKDIV,
>> +		WM8960_SYSCLK_MCLK | WM8960_SYSCLK_DIV_1);
>> +	if (ret < 0) {
>> +		dev_err(dev, "could not set codec driver clock source\n");
>> +		return ret;
>> +	}
>
> ...we don't appear to use the PLL output anyway so why not just leave it
> off?  As this is fixed it's better style to do the setup in late_init()
> or the init function for the DAI link.

I just copied this code from iVeia BSP.  I'm not really sure what it does.

> Also we should log the return codes when we log failures.

You mean like this:

if (ret < 0) {
	dev_err(dev, "could not set codec driver clock source (ret=%i)\n", ret);
	return ret;
}

I almost never do this, and there are plenty of cases in my existing code 
where I don't.  Is this a new policy?

>> +	/* Get the serial format and clock direction. */
>> +	sprop = of_get_property(np, "fsl,mode", NULL);
>> +	if (!sprop) {
>> +		dev_err(&pdev->dev, "fsl,mode property not found\n");
>> +		ret = -EINVAL;
>> +		goto error;
>> +	}
>> +
>> +	if (strcasecmp(sprop, "i2s-slave") == 0) {
>> +		mdata->dai_format = SND_SOC_DAIFMT_NB_NF |
>> +			SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM;
>> +		mdata->codec_clk_direction = SND_SOC_CLOCK_OUT;
>> +		mdata->cpu_clk_direction = SND_SOC_CLOCK_IN;
>
> Why would this be something that individual systems would want to
> change, and if it is something it's useful to vary why not at runtime?
> I suspect the best thing here is just to pick a format and use it, if
> there is a reason to vary this it's probably a higher level thing.

This is the same code as my other two machine drivers that I've been using 
for years.  It's part of the SSI binding.  Do you want me to change the 
binding and drop the properties?

I understand what you're saying, and you're probably right that it's not 
really a configuration option.  But it's a change in the binding, so I 
can't just drop it here.  I have to fix the other two machine drivers.

>
>> +	snd_soc_unregister_card(card);
>> +	kfree(mdata);
>
> devm_kzalloc().

Ok.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH 5/5] ASoC: add support for the Freescale / iVeia P1022 RDK reference board

[Mark Brown]

On Wed, Sep 19, 2012 at 02:47:48AM +0000, Tabi Timur-B04825 wrote:
> Mark Brown wrote:
> > On Fri, Sep 14, 2012 at 04:14:38PM -0500, Timur Tabi wrote:

> >>   sound/soc/fsl/Kconfig     |   14 ++
> >>   sound/soc/fsl/Makefile    |    4 +
> >>   sound/soc/fsl/p1022_rdk.c |  455 +++++++++++++++++++++++++++++++++++++++++++++
> >>   3 files changed, 473 insertions(+), 0 deletions(-)
> >>   create mode 100644 sound/soc/fsl/p1022_rdk.c

> > This adds a new device tree binding so it should be documented.

> It does?  Where?  It uses the same binding as p1022_ds.c.

It's adding a new machine driver, that generally means a new binding.
If it's really an identical binding then it ought to at least add a new
compatible property to the existing binding (and extend the set of
supported CODECs for that binding).

Personally given the thing with the interface formats and the fact that
it's not sharing code with the other bindings for implementation of the
binding I'd be inclined to just document it as a new binding and use it
as the model going forwards, leaving the older bindings as legacy.

> >> +	/* Tell the WM8960 to set SYSCLK equal to MCLK input (bypass PLL) */
> >> +	ret = snd_soc_dai_set_clkdiv(rtd->codec_dai, WM8960_SYSCLKDIV,
> >> +		WM8960_SYSCLK_MCLK | WM8960_SYSCLK_DIV_1);
> >> +	if (ret < 0) {
> >> +		dev_err(dev, "could not set codec driver clock source\n");
> >> +		return ret;
> >> +	}

> > ...we don't appear to use the PLL output anyway so why not just leave it
> > off?  As this is fixed it's better style to do the setup in late_init()
> > or the init function for the DAI link.

> I just copied this code from iVeia BSP.  I'm not really sure what it does.

You should just be able to drop the call to set_pll(), I expect you'll
find that all it does is consume extra power.

> > Also we should log the return codes when we log failures.

> You mean like this:

> if (ret < 0) {
> 	dev_err(dev, "could not set codec driver clock source (ret=%i)\n", ret);
> 	return ret;
> }

Yes.

> I almost never do this, and there are plenty of cases in my existing code 
> where I don't.  Is this a new policy?

It's not new, it's always been good practice - if something fails we
should do as much as possible to tell the user why.

> >> +	if (strcasecmp(sprop, "i2s-slave") == 0) {
> >> +		mdata->dai_format = SND_SOC_DAIFMT_NB_NF |
> >> +			SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM;
> >> +		mdata->codec_clk_direction = SND_SOC_CLOCK_OUT;
> >> +		mdata->cpu_clk_direction = SND_SOC_CLOCK_IN;

> > Why would this be something that individual systems would want to
> > change, and if it is something it's useful to vary why not at runtime?
> > I suspect the best thing here is just to pick a format and use it, if
> > there is a reason to vary this it's probably a higher level thing.

> This is the same code as my other two machine drivers that I've been using 
> for years.  It's part of the SSI binding.  Do you want me to change the 
> binding and drop the properties?

Well, the existing bindings are out there so can't really be changed but
we should do something better going forwards.

> I understand what you're saying, and you're probably right that it's not 
> really a configuration option.  But it's a change in the binding, so I 
> can't just drop it here.  I have to fix the other two machine drivers.

Like I said above it looks awfully like a new binding to me.

Re: [PATCH 5/5] ASoC: add support for the Freescale / iVeia P1022 RDK reference board

[Timur Tabi]

Mark Brown wrote:

> It's adding a new machine driver, that generally means a new binding.

Not for the way I defined the SSI bindings.  All of the information is
stored in the SSI and codec nodes.  That's why I did it that way.  There
is no binding for the machine driver.

> If it's really an identical binding then it ought to at least add a new
> compatible property to the existing binding (and extend the set of
> supported CODECs for that binding).

Here's the device tree:

http://git.kernel.org/?p=linux/kernel/git/galak/powerpc.git;a=blob;f=arch/powerpc/boot/dts/p1022rdk.dts;h=51d82de223f37c12dfbb33906cac89ce551abcd2;hb=34f84b5b5bc83f4fc208cc278f572e6d926f976b

The only thing that's new is a node for the wm8960.  I could create a
binding for that.  I asked you in a previous email about the binding for
that codec, but you never replied.

> Personally given the thing with the interface formats and the fact that
> it's not sharing code with the other bindings for implementation of the
> binding I'd be inclined to just document it as a new binding and use it
> as the model going forwards, leaving the older bindings as legacy.

Again, the only new binding is the codec.

>>>> +	/* Tell the WM8960 to set SYSCLK equal to MCLK input (bypass PLL) */
>>>> +	ret = snd_soc_dai_set_clkdiv(rtd->codec_dai, WM8960_SYSCLKDIV,
>>>> +		WM8960_SYSCLK_MCLK | WM8960_SYSCLK_DIV_1);
>>>> +	if (ret < 0) {
>>>> +		dev_err(dev, "could not set codec driver clock source\n");
>>>> +		return ret;
>>>> +	}
> 
>>> ...we don't appear to use the PLL output anyway so why not just leave it
>>> off?  As this is fixed it's better style to do the setup in late_init()
>>> or the init function for the DAI link.
> 
>> I just copied this code from iVeia BSP.  I'm not really sure what it does.
> 
> You should just be able to drop the call to set_pll(), I expect you'll
> find that all it does is consume extra power.

Ok, I'll try it.  I have no way to measure power consumption, however.

>> I almost never do this, and there are plenty of cases in my existing code 
>> where I don't.  Is this a new policy?
> 
> It's not new, it's always been good practice - if something fails we
> should do as much as possible to tell the user why.

Ok.

>> This is the same code as my other two machine drivers that I've been using 
>> for years.  It's part of the SSI binding.  Do you want me to change the 
>> binding and drop the properties?
> 
> Well, the existing bindings are out there so can't really be changed but
> we should do something better going forwards.

Fair enough.  I created those bindings because, back then, I had no idea
whether they would be needed or not.  Now that I've created three boards
that use the SSI, I see that it's not helpful.  I can fix that.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH 1/5] [v3] ASoC: fsl: use snd_soc_register_card to register the card

[Mark Brown]

On Fri, Sep 14, 2012 at 04:14:34PM -0500, Timur Tabi wrote:
> Use snd_soc_register_card() instead of platform_device_alloc("soc-audio")
> to register the sound card from the machine drivers.  The use of
> platform_device_alloc is deprecated.

Applied, thanks.