[PATCH 0/6] Add device tree support for mxs audio

[PATCH 0/6] Add device tree support for mxs audio

[Shawn Guo]

This series adds device tree probe for mxs-saif driver and mxs-sgtl5000
ASoC machine driver, and is tested on imx28-evk board.

It's based on sound, clkdev, mxs pinctrl and DT branches, and probably
easier to have it go through arm-soc with sound/for-3.5 pulled in as
dependency.

Shawn Guo (6):
      ASoC: mxs: add __devinit for mxs_saif_probe
      ASoC: mxs: use devm_clk_get for mxs_saif_probe
      ASoC: mxs: mxs-pcm does not need to be a plaform_driver
      ASoC: mxs: add device tree support for mxs-saif
      ASoC: mxs: add device tree support for mxs-sgtl5000
      ARM: dts: enable audio support for imx28-evk

 .../bindings/sound/mxs-audio-sgtl5000.txt          |   17 +++
 .../devicetree/bindings/sound/mxs-saif.txt         |   36 ++++++
 arch/arm/boot/dts/imx28-evk.dts                    |   41 +++++++
 arch/arm/boot/dts/imx28.dtsi                       |   23 ++++
 sound/soc/mxs/mxs-pcm.c                            |   24 +---
 sound/soc/mxs/mxs-pcm.h                            |    3 +
 sound/soc/mxs/mxs-saif.c                           |  119 +++++++++++---------
 sound/soc/mxs/mxs-saif.h                           |    1 -
 sound/soc/mxs/mxs-sgtl5000.c                       |   61 ++++++++++-
 9 files changed, 253 insertions(+), 72 deletions(-)

[PATCH 1/6] ASoC: mxs: add __devinit for mxs_saif_probe

[Shawn Guo]

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

diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c
index 7fd224b..40d0775 100644
--- a/sound/soc/mxs/mxs-saif.c
+++ b/sound/soc/mxs/mxs-saif.c
@@ -621,7 +621,7 @@ static irqreturn_t mxs_saif_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int mxs_saif_probe(struct platform_device *pdev)
+static int __devinit mxs_saif_probe(struct platform_device *pdev)
 {
 	struct resource *iores, *dmares;
 	struct mxs_saif *saif;
-- 
1.7.5.4

Re: [PATCH 1/6] ASoC: mxs: add __devinit for mxs_saif_probe

[Dong Aisheng]

On Thu, May 10, 2012 at 04:42:08PM +0800, Shawn Guo wrote:
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Acked-by: Dong Aisheng <dong.aisheng@linaro.org>

Regards
Dong Aisheng

Re: [PATCH 1/6] ASoC: mxs: add __devinit for mxs_saif_probe

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 127 bytes --]

On Thu, May 10, 2012 at 04:42:08PM +0800, Shawn Guo wrote:
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 2/6] ASoC: mxs: use devm_clk_get for mxs_saif_probe

[Shawn Guo]

Use devm_clk_get for mxs_saif_probe to ease the cleanup.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/mxs/mxs-saif.c |   20 +++++++-------------
 1 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c
index 40d0775..5192db6 100644
--- a/sound/soc/mxs/mxs-saif.c
+++ b/sound/soc/mxs/mxs-saif.c
@@ -658,7 +658,7 @@ static int __devinit mxs_saif_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	saif->clk = clk_get(&pdev->dev, NULL);
+	saif->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(saif->clk)) {
 		ret = PTR_ERR(saif->clk);
 		dev_err(&pdev->dev, "Cannot get the clock: %d\n",
@@ -671,25 +671,22 @@ static int __devinit mxs_saif_probe(struct platform_device *pdev)
 	saif->base = devm_request_and_ioremap(&pdev->dev, iores);
 	if (!saif->base) {
 		dev_err(&pdev->dev, "ioremap failed\n");
-		ret = -ENODEV;
-		goto failed_get_resource;
+		return -ENODEV;
 	}
 
 	dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
 	if (!dmares) {
-		ret = -ENODEV;
 		dev_err(&pdev->dev, "failed to get dma resource: %d\n",
 			ret);
-		goto failed_get_resource;
+		return -ENODEV;
 	}
 	saif->dma_param.chan_num = dmares->start;
 
 	saif->irq = platform_get_irq(pdev, 0);
 	if (saif->irq < 0) {
-		ret = saif->irq;
 		dev_err(&pdev->dev, "failed to get irq resource: %d\n",
 			ret);
-		goto failed_get_resource;
+		return saif->irq;
 	}
 
 	saif->dev = &pdev->dev;
@@ -697,7 +694,7 @@ static int __devinit mxs_saif_probe(struct platform_device *pdev)
 			       "mxs-saif", saif);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to request irq\n");
-		goto failed_get_resource;
+		return ret;
 	}
 
 	saif->dma_param.chan_irq = platform_get_irq(pdev, 1);
@@ -705,7 +702,7 @@ static int __devinit mxs_saif_probe(struct platform_device *pdev)
 		ret = saif->dma_param.chan_irq;
 		dev_err(&pdev->dev, "failed to get dma irq resource: %d\n",
 			ret);
-		goto failed_get_resource;
+		return saif->dma_param.chan_irq;
 	}
 
 	platform_set_drvdata(pdev, saif);
@@ -713,7 +710,7 @@ static int __devinit mxs_saif_probe(struct platform_device *pdev)
 	ret = snd_soc_register_dai(&pdev->dev, &mxs_saif_dai);
 	if (ret) {
 		dev_err(&pdev->dev, "register DAI failed\n");
-		goto failed_get_resource;
+		return ret;
 	}
 
 	saif->soc_platform_pdev = platform_device_alloc(
@@ -736,8 +733,6 @@ failed_pdev_add:
 	platform_device_put(saif->soc_platform_pdev);
 failed_pdev_alloc:
 	snd_soc_unregister_dai(&pdev->dev);
-failed_get_resource:
-	clk_put(saif->clk);
 
 	return ret;
 }
@@ -748,7 +743,6 @@ static int __devexit mxs_saif_remove(struct platform_device *pdev)
 
 	platform_device_unregister(saif->soc_platform_pdev);
 	snd_soc_unregister_dai(&pdev->dev);
-	clk_put(saif->clk);
 
 	return 0;
 }
-- 
1.7.5.4

Re: [PATCH 2/6] ASoC: mxs: use devm_clk_get for mxs_saif_probe

[Dong Aisheng]

On Thu, May 10, 2012 at 04:42:09PM +0800, Shawn Guo wrote:
> Use devm_clk_get for mxs_saif_probe to ease the cleanup.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  sound/soc/mxs/mxs-saif.c |   20 +++++++-------------
>  1 files changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c
> index 40d0775..5192db6 100644
> --- a/sound/soc/mxs/mxs-saif.c
> +++ b/sound/soc/mxs/mxs-saif.c
> @@ -658,7 +658,7 @@ static int __devinit mxs_saif_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	saif->clk = clk_get(&pdev->dev, NULL);
> +	saif->clk = devm_clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(saif->clk)) {
>  		ret = PTR_ERR(saif->clk);
>  		dev_err(&pdev->dev, "Cannot get the clock: %d\n",
> @@ -671,25 +671,22 @@ static int __devinit mxs_saif_probe(struct platform_device *pdev)
>  	saif->base = devm_request_and_ioremap(&pdev->dev, iores);
>  	if (!saif->base) {
>  		dev_err(&pdev->dev, "ioremap failed\n");
> -		ret = -ENODEV;
> -		goto failed_get_resource;
> +		return -ENODEV;
>  	}
>  
>  	dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
>  	if (!dmares) {
> -		ret = -ENODEV;
The 'ret' is used below.
The following has the same issue.

>  		dev_err(&pdev->dev, "failed to get dma resource: %d\n",
>  			ret);
I think we can remove the ret here since it does not help too much.

> -		goto failed_get_resource;
> +		return -ENODEV;
>  	}
>  	saif->dma_param.chan_num = dmares->start;
>  
>  	saif->irq = platform_get_irq(pdev, 0);
>  	if (saif->irq < 0) {
> -		ret = saif->irq;
ditto

>  		dev_err(&pdev->dev, "failed to get irq resource: %d\n",
>  			ret);
> -		goto failed_get_resource;
> +		return saif->irq;
>  	}
>  
>  	saif->dev = &pdev->dev;
> @@ -697,7 +694,7 @@ static int __devinit mxs_saif_probe(struct platform_device *pdev)
>  			       "mxs-saif", saif);
>  	if (ret) {
>  		dev_err(&pdev->dev, "failed to request irq\n");
> -		goto failed_get_resource;
> +		return ret;
>  	}
>  
>  	saif->dma_param.chan_irq = platform_get_irq(pdev, 1);
> @@ -705,7 +702,7 @@ static int __devinit mxs_saif_probe(struct platform_device *pdev)
>  		ret = saif->dma_param.chan_irq;
>  		dev_err(&pdev->dev, "failed to get dma irq resource: %d\n",
>  			ret);
ditto
Just remove it.

Regards
Dong Aisheng
> -		goto failed_get_resource;
> +		return saif->dma_param.chan_irq;
>  	}
>  
>  	platform_set_drvdata(pdev, saif);
> @@ -713,7 +710,7 @@ static int __devinit mxs_saif_probe(struct platform_device *pdev)
>  	ret = snd_soc_register_dai(&pdev->dev, &mxs_saif_dai);
>  	if (ret) {
>  		dev_err(&pdev->dev, "register DAI failed\n");
> -		goto failed_get_resource;
> +		return ret;
>  	}
>  
>  	saif->soc_platform_pdev = platform_device_alloc(
> @@ -736,8 +733,6 @@ failed_pdev_add:
>  	platform_device_put(saif->soc_platform_pdev);
>  failed_pdev_alloc:
>  	snd_soc_unregister_dai(&pdev->dev);
> -failed_get_resource:
> -	clk_put(saif->clk);
>  
>  	return ret;
>  }
> @@ -748,7 +743,6 @@ static int __devexit mxs_saif_remove(struct platform_device *pdev)
>  
>  	platform_device_unregister(saif->soc_platform_pdev);
>  	snd_soc_unregister_dai(&pdev->dev);
> -	clk_put(saif->clk);
>  
>  	return 0;
>  }
> -- 
> 1.7.5.4
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [PATCH 2/6] ASoC: mxs: use devm_clk_get for mxs_saif_probe

[Shawn Guo]

On 10 May 2012 17:12, Dong Aisheng <aisheng.dong@freescale.com> wrote:
>>       dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
>>       if (!dmares) {
>> -             ret = -ENODEV;
> The 'ret' is used below.

Right, good catch.

> The following has the same issue.
>
>>               dev_err(&pdev->dev, "failed to get dma resource: %d\n",
>>                       ret);
> I think we can remove the ret here since it does not help too much.
>
Ok, will do.

Regards,
Shawn

Re: [PATCH 2/6] ASoC: mxs: use devm_clk_get for mxs_saif_probe

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 295 bytes --]

On Thu, May 10, 2012 at 04:42:09PM +0800, Shawn Guo wrote:
> Use devm_clk_get for mxs_saif_probe to ease the cleanup.

Please stop submitting devm_clk_get() changes until it's in mainline, as
I've said with all the similar changs this can't be applied until that
API can be merged up into ASoC.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/6] ASoC: mxs: use devm_clk_get for mxs_saif_probe

[Shawn Guo]

On 10 May 2012 17:13, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, May 10, 2012 at 04:42:09PM +0800, Shawn Guo wrote:
>> Use devm_clk_get for mxs_saif_probe to ease the cleanup.
>
> Please stop submitting devm_clk_get() changes until it's in mainline, as
> I've said with all the similar changs this can't be applied until that
> API can be merged up into ASoC.

Russell's clkdev branch will show up in arm-soc as a dependency
branch.  So if you are ok with the series go through arm-soc, we will
be fine.

Regards,
Shawn

Re: [PATCH 2/6] ASoC: mxs: use devm_clk_get for mxs_saif_probe

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 800 bytes --]

On Thu, May 10, 2012 at 05:17:27PM +0800, Shawn Guo wrote:
> On 10 May 2012 17:13, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> > Please stop submitting devm_clk_get() changes until it's in mainline, as
> > I've said with all the similar changs this can't be applied until that
> > API can be merged up into ASoC.

> Russell's clkdev branch will show up in arm-soc as a dependency
> branch.  So if you are ok with the series go through arm-soc, we will
> be fine.

Well, if Russell's happy with that being merged into other trees I could
just pull it into ASoC?  Or we could just wait, or if this is really
separate to everything else why not just apply it on the clkdev branch
rather than worrying about some complicated arrangement?

This is just a trivial cleanup to use new APIs...

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/6] ASoC: mxs: use devm_clk_get for mxs_saif_probe

[Shawn Guo]

On Thu, May 10, 2012 at 10:30:33AM +0100, Mark Brown wrote:
> Well, if Russell's happy with that being merged into other trees I could
> just pull it into ASoC?  Or we could just wait, or if this is really
> separate to everything else why not just apply it on the clkdev branch
> rather than worrying about some complicated arrangement?
> 
> This is just a trivial cleanup to use new APIs...

Ok, giving up ...

-- 
Regards,
Shawn

[PATCH 3/6] ASoC: mxs: mxs-pcm does not need to be a plaform_driver

[Shawn Guo]

Same as the commit 518de86 (ASoC: tegra: register 'platform' from DAIs,
get rid of pdev), it makes mxs-pcm not a platform_driver but helper to
register "platform", so that the platform_device for mxs-pcm can be
saved completely.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/mxs/mxs-pcm.c      |   24 ++++++------------------
 sound/soc/mxs/mxs-pcm.h      |    3 +++
 sound/soc/mxs/mxs-saif.c     |   25 ++++---------------------
 sound/soc/mxs/mxs-saif.h     |    1 -
 sound/soc/mxs/mxs-sgtl5000.c |    4 ++--
 5 files changed, 15 insertions(+), 42 deletions(-)

diff --git a/sound/soc/mxs/mxs-pcm.c b/sound/soc/mxs/mxs-pcm.c
index e373fbb..373dec9 100644
--- a/sound/soc/mxs/mxs-pcm.c
+++ b/sound/soc/mxs/mxs-pcm.c
@@ -220,28 +220,16 @@ static struct snd_soc_platform_driver mxs_soc_platform = {
 	.pcm_free	= mxs_pcm_free,
 };
 
-static int __devinit mxs_soc_platform_probe(struct platform_device *pdev)
+int __devinit mxs_pcm_platform_register(struct device *dev)
 {
-	return snd_soc_register_platform(&pdev->dev, &mxs_soc_platform);
+	return snd_soc_register_platform(dev, &mxs_soc_platform);
 }
+EXPORT_SYMBOL_GPL(mxs_pcm_platform_register);
 
-static int __devexit mxs_soc_platform_remove(struct platform_device *pdev)
+void __devexit mxs_pcm_platform_unregister(struct device *dev)
 {
-	snd_soc_unregister_platform(&pdev->dev);
-
-	return 0;
+	snd_soc_unregister_platform(dev);
 }
-
-static struct platform_driver mxs_pcm_driver = {
-	.driver = {
-		.name = "mxs-pcm-audio",
-		.owner = THIS_MODULE,
-	},
-	.probe = mxs_soc_platform_probe,
-	.remove = __devexit_p(mxs_soc_platform_remove),
-};
-
-module_platform_driver(mxs_pcm_driver);
+EXPORT_SYMBOL_GPL(mxs_pcm_platform_unregister);
 
 MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:mxs-pcm-audio");
diff --git a/sound/soc/mxs/mxs-pcm.h b/sound/soc/mxs/mxs-pcm.h
index 5f01a91..35ba2ca 100644
--- a/sound/soc/mxs/mxs-pcm.h
+++ b/sound/soc/mxs/mxs-pcm.h
@@ -24,4 +24,7 @@ struct mxs_pcm_dma_params {
 	int chan_num;
 };
 
+int mxs_pcm_platform_register(struct device *dev);
+void mxs_pcm_platform_unregister(struct device *dev);
+
 #endif
diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c
index 5192db6..fa80ee6 100644
--- a/sound/soc/mxs/mxs-saif.c
+++ b/sound/soc/mxs/mxs-saif.c
@@ -713,35 +713,18 @@ static int __devinit mxs_saif_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	saif->soc_platform_pdev = platform_device_alloc(
-					"mxs-pcm-audio", pdev->id);
-	if (!saif->soc_platform_pdev) {
-		ret = -ENOMEM;
-		goto failed_pdev_alloc;
-	}
-
-	platform_set_drvdata(saif->soc_platform_pdev, saif);
-	ret = platform_device_add(saif->soc_platform_pdev);
+	ret = mxs_pcm_platform_register(&pdev->dev);
 	if (ret) {
-		dev_err(&pdev->dev, "failed to add soc platform device\n");
-		goto failed_pdev_add;
+		dev_err(&pdev->dev, "register PCM failed: %d\n", ret);
+		return ret;
 	}
 
 	return 0;
-
-failed_pdev_add:
-	platform_device_put(saif->soc_platform_pdev);
-failed_pdev_alloc:
-	snd_soc_unregister_dai(&pdev->dev);
-
-	return ret;
 }
 
 static int __devexit mxs_saif_remove(struct platform_device *pdev)
 {
-	struct mxs_saif *saif = platform_get_drvdata(pdev);
-
-	platform_device_unregister(saif->soc_platform_pdev);
+	mxs_pcm_platform_unregister(&pdev->dev);
 	snd_soc_unregister_dai(&pdev->dev);
 
 	return 0;
diff --git a/sound/soc/mxs/mxs-saif.h b/sound/soc/mxs/mxs-saif.h
index 12c91e4..3cb342e 100644
--- a/sound/soc/mxs/mxs-saif.h
+++ b/sound/soc/mxs/mxs-saif.h
@@ -123,7 +123,6 @@ struct mxs_saif {
 	unsigned int cur_rate;
 	unsigned int ongoing;
 
-	struct platform_device *soc_platform_pdev;
 	u32 fifo_underrun;
 	u32 fifo_overrun;
 };
diff --git a/sound/soc/mxs/mxs-sgtl5000.c b/sound/soc/mxs/mxs-sgtl5000.c
index 60f052b..e9e6112 100644
--- a/sound/soc/mxs/mxs-sgtl5000.c
+++ b/sound/soc/mxs/mxs-sgtl5000.c
@@ -90,7 +90,7 @@ static struct snd_soc_dai_link mxs_sgtl5000_dai[] = {
 		.codec_dai_name	= "sgtl5000",
 		.codec_name	= "sgtl5000.0-000a",
 		.cpu_dai_name	= "mxs-saif.0",
-		.platform_name	= "mxs-pcm-audio.0",
+		.platform_name	= "mxs-saif.0",
 		.ops		= &mxs_sgtl5000_hifi_ops,
 	}, {
 		.name		= "HiFi Rx",
@@ -98,7 +98,7 @@ static struct snd_soc_dai_link mxs_sgtl5000_dai[] = {
 		.codec_dai_name	= "sgtl5000",
 		.codec_name	= "sgtl5000.0-000a",
 		.cpu_dai_name	= "mxs-saif.1",
-		.platform_name	= "mxs-pcm-audio.1",
+		.platform_name	= "mxs-saif.1",
 		.ops		= &mxs_sgtl5000_hifi_ops,
 	},
 };
-- 
1.7.5.4

Re: [PATCH 3/6] ASoC: mxs: mxs-pcm does not need to be a plaform_driver

[Dong Aisheng]

On Thu, May 10, 2012 at 04:42:10PM +0800, Shawn Guo wrote:
> Same as the commit 518de86 (ASoC: tegra: register 'platform' from DAIs,
> get rid of pdev), it makes mxs-pcm not a platform_driver but helper to
> register "platform", so that the platform_device for mxs-pcm can be
> saved completely.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  sound/soc/mxs/mxs-pcm.c      |   24 ++++++------------------
>  sound/soc/mxs/mxs-pcm.h      |    3 +++
>  sound/soc/mxs/mxs-saif.c     |   25 ++++---------------------
>  sound/soc/mxs/mxs-saif.h     |    1 -
>  sound/soc/mxs/mxs-sgtl5000.c |    4 ++--
>  5 files changed, 15 insertions(+), 42 deletions(-)
> 
...

> +int mxs_pcm_platform_register(struct device *dev);
> +void mxs_pcm_platform_unregister(struct device *dev);
> +
>  #endif
> diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c
> index 5192db6..fa80ee6 100644
> --- a/sound/soc/mxs/mxs-saif.c
> +++ b/sound/soc/mxs/mxs-saif.c
> @@ -713,35 +713,18 @@ static int __devinit mxs_saif_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	saif->soc_platform_pdev = platform_device_alloc(
> -					"mxs-pcm-audio", pdev->id);
> -	if (!saif->soc_platform_pdev) {
> -		ret = -ENOMEM;
> -		goto failed_pdev_alloc;
> -	}
> -
> -	platform_set_drvdata(saif->soc_platform_pdev, saif);
> -	ret = platform_device_add(saif->soc_platform_pdev);
> +	ret = mxs_pcm_platform_register(&pdev->dev);
>  	if (ret) {
> -		dev_err(&pdev->dev, "failed to add soc platform device\n");
> -		goto failed_pdev_add;
> +		dev_err(&pdev->dev, "register PCM failed: %d\n", ret);
> +		return ret;
>  	}
>  
>  	return 0;
> -
> -failed_pdev_add:
> -	platform_device_put(saif->soc_platform_pdev);
> -failed_pdev_alloc:
> -	snd_soc_unregister_dai(&pdev->dev);
Do not need to unregister_dai in case mxs_pcm_platform_register failed?

Regards
Dong Aisheng

Re: [PATCH 3/6] ASoC: mxs: mxs-pcm does not need to be a plaform_driver

[Shawn Guo]

On Thu, May 10, 2012 at 05:25:55PM +0800, Dong Aisheng wrote:
> > -failed_pdev_alloc:
> > -	snd_soc_unregister_dai(&pdev->dev);
> Do not need to unregister_dai in case mxs_pcm_platform_register failed?
> 
Yeah, my fingers moved so fast to remove code.  Thanks for catching it.

-- 
Regards,
Shawn

[PATCH 4/6] ASoC: mxs: add device tree support for mxs-saif

[Shawn Guo]

Add device tree probe for mxs-saif driver.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 .../devicetree/bindings/sound/mxs-saif.txt         |   36 ++++++++++
 sound/soc/mxs/mxs-saif.c                           |   74 +++++++++++++++-----
 2 files changed, 93 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/mxs-saif.txt

diff --git a/Documentation/devicetree/bindings/sound/mxs-saif.txt b/Documentation/devicetree/bindings/sound/mxs-saif.txt
new file mode 100644
index 0000000..c37ba61
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/mxs-saif.txt
@@ -0,0 +1,36 @@
+* Freescale MXS Serial Audio Interface (SAIF)
+
+Required properties:
+- compatible: Should be "fsl,<chip>-saif"
+- reg: Should contain registers location and length
+- interrupts: Should contain ERROR and DMA interrupts
+- fsl,saif-dma-channel: APBX DMA channel for the SAIF
+
+Optional properties:
+- fsl,saif-master: phandle to the master SAIF.  It's only required for
+  the slave SAIF.
+
+Note: Each SAIF controller should have an alias correctly numbered
+in "aliases" node.
+
+Example:
+
+aliases {
+	saif0 = &saif0;
+	saif1 = &saif1;
+};
+
+saif0: saif@80042000 {
+	compatible = "fsl,imx28-saif";
+	reg = <0x80042000 2000>;
+	interrupts = <59 80>;
+	fsl,saif-dma-channel = <4>;
+};
+
+saif1: saif@80046000 {
+	compatible = "fsl,imx28-saif";
+	reg = <0x80046000 2000>;
+	interrupts = <58 81>;
+	fsl,saif-dma-channel = <5>;
+	fsl,saif-master = <&saif0>;
+};
diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c
index fa80ee6..ca1e77c 100644
--- a/sound/soc/mxs/mxs-saif.c
+++ b/sound/soc/mxs/mxs-saif.c
@@ -18,6 +18,8 @@
 
 #include <linux/module.h>
 #include <linux/init.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/dma-mapping.h>
@@ -623,35 +625,58 @@ static irqreturn_t mxs_saif_irq(int irq, void *dev_id)
 
 static int __devinit mxs_saif_probe(struct platform_device *pdev)
 {
+	struct device_node *np = pdev->dev.of_node;
 	struct resource *iores, *dmares;
 	struct mxs_saif *saif;
 	struct mxs_saif_platform_data *pdata;
 	struct pinctrl *pinctrl;
 	int ret = 0;
 
-	if (pdev->id >= ARRAY_SIZE(mxs_saif))
+
+	if (!np && pdev->id >= ARRAY_SIZE(mxs_saif))
 		return -EINVAL;
 
 	saif = devm_kzalloc(&pdev->dev, sizeof(*saif), GFP_KERNEL);
 	if (!saif)
 		return -ENOMEM;
 
-	mxs_saif[pdev->id] = saif;
-	saif->id = pdev->id;
-
-	pdata = pdev->dev.platform_data;
-	if (pdata && !pdata->master_mode) {
-		saif->master_id = pdata->master_id;
-		if (saif->master_id < 0 ||
-			saif->master_id >= ARRAY_SIZE(mxs_saif) ||
-			saif->master_id == saif->id) {
-			dev_err(&pdev->dev, "get wrong master id\n");
-			return -EINVAL;
+	if (np) {
+		struct device_node *master;
+		saif->id = of_alias_get_id(np, "saif");
+		if (saif->id < 0)
+			return saif->id;
+		/*
+		 * If there is no "fsl,saif-master" phandle, it's a saif
+		 * master.  Otherwise, it's a slave and its phandle points
+		 * to the master.
+		 */
+		master = of_parse_phandle(np, "fsl,saif-master", 0);
+		if (!master) {
+			saif->master_id = saif->id;
+		} else {
+			saif->master_id = of_alias_get_id(master, "saif");
+			if (saif->master_id < 0)
+				return saif->master_id;
 		}
 	} else {
-		saif->master_id = saif->id;
+		saif->id = pdev->id;
+
+		pdata = pdev->dev.platform_data;
+		if (pdata && !pdata->master_mode) {
+			saif->master_id = pdata->master_id;
+			if (saif->master_id < 0 ||
+				saif->master_id >= ARRAY_SIZE(mxs_saif) ||
+				saif->master_id == saif->id) {
+				dev_err(&pdev->dev, "get wrong master id\n");
+				return -EINVAL;
+			}
+		} else {
+			saif->master_id = saif->id;
+		}
 	}
 
+	mxs_saif[saif->id] = saif;
+
 	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
 	if (IS_ERR(pinctrl)) {
 		ret = PTR_ERR(pinctrl);
@@ -676,11 +701,19 @@ static int __devinit mxs_saif_probe(struct platform_device *pdev)
 
 	dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
 	if (!dmares) {
-		dev_err(&pdev->dev, "failed to get dma resource: %d\n",
-			ret);
-		return -ENODEV;
+		/*
+		 * TODO: This is a temporary solution and should be changed
+		 * to use generic DMA binding later when the helplers get in.
+		 */
+		ret = of_property_read_u32(np, "fsl,saif-dma-channel",
+					   &saif->dma_param.chan_num);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to get dma channel\n");
+			return ret;
+		}
+	} else {
+		saif->dma_param.chan_num = dmares->start;
 	}
-	saif->dma_param.chan_num = dmares->start;
 
 	saif->irq = platform_get_irq(pdev, 0);
 	if (saif->irq < 0) {
@@ -730,6 +763,12 @@ static int __devexit mxs_saif_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id mxs_saif_dt_ids[] = {
+	{ .compatible = "fsl,imx28-saif", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxs_saif_dt_ids);
+
 static struct platform_driver mxs_saif_driver = {
 	.probe = mxs_saif_probe,
 	.remove = __devexit_p(mxs_saif_remove),
@@ -737,6 +776,7 @@ static struct platform_driver mxs_saif_driver = {
 	.driver = {
 		.name = "mxs-saif",
 		.owner = THIS_MODULE,
+		.of_match_table = mxs_saif_dt_ids,
 	},
 };
 
-- 
1.7.5.4

Re: [PATCH 4/6] ASoC: mxs: add device tree support for mxs-saif

[Dong Aisheng]

On Thu, May 10, 2012 at 04:42:11PM +0800, Shawn Guo wrote:
> Add device tree probe for mxs-saif driver.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  .../devicetree/bindings/sound/mxs-saif.txt         |   36 ++++++++++
>  sound/soc/mxs/mxs-saif.c                           |   74 +++++++++++++++-----
>  2 files changed, 93 insertions(+), 17 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/sound/mxs-saif.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/mxs-saif.txt b/Documentation/devicetree/bindings/sound/mxs-saif.txt
> new file mode 100644
> index 0000000..c37ba61
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/mxs-saif.txt
> @@ -0,0 +1,36 @@
> +* Freescale MXS Serial Audio Interface (SAIF)
> +
> +Required properties:
> +- compatible: Should be "fsl,<chip>-saif"
> +- reg: Should contain registers location and length
> +- interrupts: Should contain ERROR and DMA interrupts
> +- fsl,saif-dma-channel: APBX DMA channel for the SAIF
> +
> +Optional properties:
> +- fsl,saif-master: phandle to the master SAIF.  It's only required for
> +  the slave SAIF.
> +
> +Note: Each SAIF controller should have an alias correctly numbered
> +in "aliases" node.
> +
> +Example:
> +
> +aliases {
> +	saif0 = &saif0;
> +	saif1 = &saif1;
> +};
> +
> +saif0: saif@80042000 {
> +	compatible = "fsl,imx28-saif";
> +	reg = <0x80042000 2000>;
> +	interrupts = <59 80>;
> +	fsl,saif-dma-channel = <4>;
> +};
> +
> +saif1: saif@80046000 {
> +	compatible = "fsl,imx28-saif";
> +	reg = <0x80046000 2000>;
> +	interrupts = <58 81>;
> +	fsl,saif-dma-channel = <5>;
> +	fsl,saif-master = <&saif0>;
> +};
> diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c
> index fa80ee6..ca1e77c 100644
> --- a/sound/soc/mxs/mxs-saif.c
> +++ b/sound/soc/mxs/mxs-saif.c
> @@ -18,6 +18,8 @@
>  
>  #include <linux/module.h>
>  #include <linux/init.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/dma-mapping.h>
> @@ -623,35 +625,58 @@ static irqreturn_t mxs_saif_irq(int irq, void *dev_id)
>  
>  static int __devinit mxs_saif_probe(struct platform_device *pdev)
>  {
> +	struct device_node *np = pdev->dev.of_node;
>  	struct resource *iores, *dmares;
>  	struct mxs_saif *saif;
>  	struct mxs_saif_platform_data *pdata;
>  	struct pinctrl *pinctrl;
>  	int ret = 0;
>  
> -	if (pdev->id >= ARRAY_SIZE(mxs_saif))
> +
> +	if (!np && pdev->id >= ARRAY_SIZE(mxs_saif))
>  		return -EINVAL;
>  
>  	saif = devm_kzalloc(&pdev->dev, sizeof(*saif), GFP_KERNEL);
>  	if (!saif)
>  		return -ENOMEM;
>  
> -	mxs_saif[pdev->id] = saif;
> -	saif->id = pdev->id;
> -
> -	pdata = pdev->dev.platform_data;
> -	if (pdata && !pdata->master_mode) {
> -		saif->master_id = pdata->master_id;
> -		if (saif->master_id < 0 ||
> -			saif->master_id >= ARRAY_SIZE(mxs_saif) ||
> -			saif->master_id == saif->id) {
> -			dev_err(&pdev->dev, "get wrong master id\n");
> -			return -EINVAL;
> +	if (np) {
> +		struct device_node *master;
> +		saif->id = of_alias_get_id(np, "saif");
> +		if (saif->id < 0)
> +			return saif->id;
> +		/*
> +		 * If there is no "fsl,saif-master" phandle, it's a saif
> +		 * master.  Otherwise, it's a slave and its phandle points
> +		 * to the master.
> +		 */
> +		master = of_parse_phandle(np, "fsl,saif-master", 0);
> +		if (!master) {
> +			saif->master_id = saif->id;
> +		} else {
> +			saif->master_id = of_alias_get_id(master, "saif");
> +			if (saif->master_id < 0)
> +				return saif->master_id;
>  		}
>  	} else {
> -		saif->master_id = saif->id;
> +		saif->id = pdev->id;
> +
> +		pdata = pdev->dev.platform_data;
> +		if (pdata && !pdata->master_mode) {
> +			saif->master_id = pdata->master_id;
> +			if (saif->master_id < 0 ||
> +				saif->master_id >= ARRAY_SIZE(mxs_saif) ||
> +				saif->master_id == saif->id) {
> +				dev_err(&pdev->dev, "get wrong master id\n");
> +				return -EINVAL;
Maybe we could put the master_id validation code out to below since for dt case
it also needs it.

Otherwise the code looks ok to me.

Regards
Dong Aisheng

Re: [PATCH 4/6] ASoC: mxs: add device tree support for mxs-saif

[Shawn Guo]

On Thu, May 10, 2012 at 05:45:25PM +0800, Dong Aisheng wrote:
> Maybe we could put the master_id validation code out to below since for dt case
> it also needs it.
> 
Reasonable.  Will do.

-- 
Regards,
Shawn

[PATCH 5/6] ASoC: mxs: add device tree support for mxs-sgtl5000

[Shawn Guo]

Add device tree probe for mxs-sgtl5000 machine driver.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 .../bindings/sound/mxs-audio-sgtl5000.txt          |   17 ++++++
 sound/soc/mxs/mxs-sgtl5000.c                       |   57 ++++++++++++++++++++
 2 files changed, 74 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/mxs-audio-sgtl5000.txt

diff --git a/Documentation/devicetree/bindings/sound/mxs-audio-sgtl5000.txt b/Documentation/devicetree/bindings/sound/mxs-audio-sgtl5000.txt
new file mode 100644
index 0000000..601c518
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/mxs-audio-sgtl5000.txt
@@ -0,0 +1,17 @@
+* Freescale MXS audio complex with SGTL5000 codec
+
+Required properties:
+- compatible: "fsl,mxs-audio-sgtl5000"
+- model: The user-visible name of this sound complex
+- saif-controllers: The phandle list of the MXS SAIF controller
+- audio-codec: The phandle of the SGTL5000 audio codec
+
+Example:
+
+sound {
+	compatible = "fsl,imx28-evk-sgtl5000",
+		     "fsl,mxs-audio-sgtl5000";
+	model = "imx28-evk-sgtl5000";
+	saif-controllers = <&saif0 &saif1>;
+	audio-codec = <&sgtl5000>;
+};
diff --git a/sound/soc/mxs/mxs-sgtl5000.c b/sound/soc/mxs/mxs-sgtl5000.c
index e9e6112..d420f11 100644
--- a/sound/soc/mxs/mxs-sgtl5000.c
+++ b/sound/soc/mxs/mxs-sgtl5000.c
@@ -18,6 +18,8 @@
 
 #include <linux/module.h>
 #include <linux/device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/soc.h>
@@ -110,11 +112,58 @@ static struct snd_soc_card mxs_sgtl5000 = {
 	.num_links	= ARRAY_SIZE(mxs_sgtl5000_dai),
 };
 
+static int __devinit mxs_sgtl5000_probe_dt(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *saif_np[2], *codec_np;
+	struct platform_device *saif_pdev[2];
+	int i, ret = 0;
+
+	if (!np)
+		return 1; /* no device tree */
+
+	saif_np[0] = of_parse_phandle(np, "saif-controllers", 0);
+	saif_np[1] = of_parse_phandle(np, "saif-controllers", 1);
+	codec_np = of_parse_phandle(np, "audio-codec", 0);
+	if (!saif_np[0] || !saif_np[1] || !codec_np) {
+		dev_err(&pdev->dev, "phandle missing or invalid\n");
+		return -EINVAL;
+	}
+
+	saif_pdev[0] = of_find_device_by_node(saif_np[0]);
+	saif_pdev[1] = of_find_device_by_node(saif_np[1]);
+	if (!saif_pdev[0] || !saif_pdev[1]) {
+		dev_err(&pdev->dev, "failed to find saif platform device\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	for (i = 0; i < 2; i++) {
+		mxs_sgtl5000_dai[i].codec_name = NULL;
+		mxs_sgtl5000_dai[i].codec_of_node = codec_np;
+		mxs_sgtl5000_dai[i].cpu_dai_name = NULL;
+		mxs_sgtl5000_dai[i].cpu_dai_of_node = saif_np[i];
+		mxs_sgtl5000_dai[i].platform_name = NULL;
+		mxs_sgtl5000_dai[i].platform_of_node = saif_np[i];
+	}
+
+out:
+	of_node_put(codec_np);
+	of_node_put(saif_np[0]);
+	of_node_put(saif_np[1]);
+
+	return ret;
+}
+
 static int __devinit mxs_sgtl5000_probe(struct platform_device *pdev)
 {
 	struct snd_soc_card *card = &mxs_sgtl5000;
 	int ret;
 
+	ret = mxs_sgtl5000_probe_dt(pdev);
+	if (ret < 0)
+		return ret;
+
 	/*
 	 * Set an init clock(11.28Mhz) for sgtl5000 initialization(i2c r/w).
 	 * The Sgtl5000 sysclk is derived from saif0 mclk and it's range
@@ -127,6 +176,7 @@ static int __devinit mxs_sgtl5000_probe(struct platform_device *pdev)
 	card->dev = &pdev->dev;
 	platform_set_drvdata(pdev, card);
 
+	card->dev = &pdev->dev;
 	ret = snd_soc_register_card(card);
 	if (ret) {
 		dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n",
@@ -148,10 +198,17 @@ static int __devexit mxs_sgtl5000_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id mxs_sgtl5000_dt_ids[] = {
+	{ .compatible = "fsl,mxs-audio-sgtl5000", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxs_sgtl5000_dt_ids);
+
 static struct platform_driver mxs_sgtl5000_audio_driver = {
 	.driver = {
 		.name = "mxs-sgtl5000",
 		.owner = THIS_MODULE,
+		.of_match_table = mxs_sgtl5000_dt_ids,
 	},
 	.probe = mxs_sgtl5000_probe,
 	.remove = __devexit_p(mxs_sgtl5000_remove),
-- 
1.7.5.4

Re: [PATCH 5/6] ASoC: mxs: add device tree support for mxs-sgtl5000

[Dong Aisheng]

On Thu, May 10, 2012 at 04:42:12PM +0800, Shawn Guo wrote:
> Add device tree probe for mxs-sgtl5000 machine driver.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Mostly it looks okay to me.
A few comments.

> ---
>  .../bindings/sound/mxs-audio-sgtl5000.txt          |   17 ++++++
>  sound/soc/mxs/mxs-sgtl5000.c                       |   57 ++++++++++++++++++++
>  2 files changed, 74 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/sound/mxs-audio-sgtl5000.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/mxs-audio-sgtl5000.txt b/Documentation/devicetree/bindings/sound/mxs-audio-sgtl5000.txt
> new file mode 100644
> index 0000000..601c518
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/mxs-audio-sgtl5000.txt
> @@ -0,0 +1,17 @@
> +* Freescale MXS audio complex with SGTL5000 codec
> +
> +Required properties:
> +- compatible: "fsl,mxs-audio-sgtl5000"
> +- model: The user-visible name of this sound complex
> +- saif-controllers: The phandle list of the MXS SAIF controller
> +- audio-codec: The phandle of the SGTL5000 audio codec
> +
> +Example:
> +
> +sound {
> +	compatible = "fsl,imx28-evk-sgtl5000",
> +		     "fsl,mxs-audio-sgtl5000";
> +	model = "imx28-evk-sgtl5000";
> +	saif-controllers = <&saif0 &saif1>;
> +	audio-codec = <&sgtl5000>;
> +};
> diff --git a/sound/soc/mxs/mxs-sgtl5000.c b/sound/soc/mxs/mxs-sgtl5000.c
> index e9e6112..d420f11 100644
> --- a/sound/soc/mxs/mxs-sgtl5000.c
> +++ b/sound/soc/mxs/mxs-sgtl5000.c
> @@ -18,6 +18,8 @@
>  
>  #include <linux/module.h>
>  #include <linux/device.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <sound/core.h>
>  #include <sound/pcm.h>
>  #include <sound/soc.h>
> @@ -110,11 +112,58 @@ static struct snd_soc_card mxs_sgtl5000 = {
>  	.num_links	= ARRAY_SIZE(mxs_sgtl5000_dai),
>  };
>  
> +static int __devinit mxs_sgtl5000_probe_dt(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *saif_np[2], *codec_np;
> +	struct platform_device *saif_pdev[2];
> +	int i, ret = 0;
> +
> +	if (!np)
> +		return 1; /* no device tree */
> +
> +	saif_np[0] = of_parse_phandle(np, "saif-controllers", 0);
> +	saif_np[1] = of_parse_phandle(np, "saif-controllers", 1);
> +	codec_np = of_parse_phandle(np, "audio-codec", 0);
> +	if (!saif_np[0] || !saif_np[1] || !codec_np) {
> +		dev_err(&pdev->dev, "phandle missing or invalid\n");
> +		return -EINVAL;
> +	}
...

> +
> +	saif_pdev[0] = of_find_device_by_node(saif_np[0]);
> +	saif_pdev[1] = of_find_device_by_node(saif_np[1]);
> +	if (!saif_pdev[0] || !saif_pdev[1]) {
> +		dev_err(&pdev->dev, "failed to find saif platform device\n");
> +		ret = -EINVAL;
> +		goto out;
> +	}
Do we really need this checking?

> +
> +	for (i = 0; i < 2; i++) {
> +		mxs_sgtl5000_dai[i].codec_name = NULL;
> +		mxs_sgtl5000_dai[i].codec_of_node = codec_np;
> +		mxs_sgtl5000_dai[i].cpu_dai_name = NULL;
> +		mxs_sgtl5000_dai[i].cpu_dai_of_node = saif_np[i];
> +		mxs_sgtl5000_dai[i].platform_name = NULL;
> +		mxs_sgtl5000_dai[i].platform_of_node = saif_np[i];
> +	}
> +
> +out:
> +	of_node_put(codec_np);
> +	of_node_put(saif_np[0]);
> +	of_node_put(saif_np[1]);
> +
> +	return ret;
> +}
> +
>  static int __devinit mxs_sgtl5000_probe(struct platform_device *pdev)
>  {
>  	struct snd_soc_card *card = &mxs_sgtl5000;
>  	int ret;
>  
> +	ret = mxs_sgtl5000_probe_dt(pdev);
> +	if (ret < 0)
> +		return ret;
> +
>  	/*
>  	 * Set an init clock(11.28Mhz) for sgtl5000 initialization(i2c r/w).
>  	 * The Sgtl5000 sysclk is derived from saif0 mclk and it's range
> @@ -127,6 +176,7 @@ static int __devinit mxs_sgtl5000_probe(struct platform_device *pdev)
>  	card->dev = &pdev->dev;
>  	platform_set_drvdata(pdev, card);
>  
> +	card->dev = &pdev->dev;
Repeat assignment?

Regards
Dong Aisheng

>  	ret = snd_soc_register_card(card);
>  	if (ret) {
>  		dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n",
> @@ -148,10 +198,17 @@ static int __devexit mxs_sgtl5000_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct of_device_id mxs_sgtl5000_dt_ids[] = {
> +	{ .compatible = "fsl,mxs-audio-sgtl5000", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mxs_sgtl5000_dt_ids);
> +
>  static struct platform_driver mxs_sgtl5000_audio_driver = {
>  	.driver = {
>  		.name = "mxs-sgtl5000",
>  		.owner = THIS_MODULE,
> +		.of_match_table = mxs_sgtl5000_dt_ids,
>  	},
>  	.probe = mxs_sgtl5000_probe,
>  	.remove = __devexit_p(mxs_sgtl5000_remove),
> -- 
> 1.7.5.4
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [PATCH 5/6] ASoC: mxs: add device tree support for mxs-sgtl5000

[Shawn Guo]

On Thu, May 10, 2012 at 07:16:46PM +0800, Dong Aisheng wrote:
> > +static int __devinit mxs_sgtl5000_probe_dt(struct platform_device *pdev)
> > +{
> > +	struct device_node *np = pdev->dev.of_node;
> > +	struct device_node *saif_np[2], *codec_np;
> > +	struct platform_device *saif_pdev[2];
> > +	int i, ret = 0;
> > +
> > +	if (!np)
> > +		return 1; /* no device tree */
> > +
> > +	saif_np[0] = of_parse_phandle(np, "saif-controllers", 0);
> > +	saif_np[1] = of_parse_phandle(np, "saif-controllers", 1);
> > +	codec_np = of_parse_phandle(np, "audio-codec", 0);
> > +	if (!saif_np[0] || !saif_np[1] || !codec_np) {
> > +		dev_err(&pdev->dev, "phandle missing or invalid\n");
> > +		return -EINVAL;
> > +	}
> ...
> 
> > +
> > +	saif_pdev[0] = of_find_device_by_node(saif_np[0]);
> > +	saif_pdev[1] = of_find_device_by_node(saif_np[1]);
> > +	if (!saif_pdev[0] || !saif_pdev[1]) {
> > +		dev_err(&pdev->dev, "failed to find saif platform device\n");
> > +		ret = -EINVAL;
> > +		goto out;
> > +	}
> Do we really need this checking?
> 
Hmm, why not?  We need to ensure that the phandles are correctly
represented in device tree.

> > +	card->dev = &pdev->dev;
> Repeat assignment?
> 
Good catch, again.  It's a rebasing error.  Thanks.

Regards,
Shawn

Re: [PATCH 5/6] ASoC: mxs: add device tree support for mxs-sgtl5000

[Dong Aisheng]

On Thu, May 10, 2012 at 08:20:04PM +0800, Shawn Guo wrote:
> On Thu, May 10, 2012 at 07:16:46PM +0800, Dong Aisheng wrote:
> > > +static int __devinit mxs_sgtl5000_probe_dt(struct platform_device *pdev)
> > > +{
> > > +	struct device_node *np = pdev->dev.of_node;
> > > +	struct device_node *saif_np[2], *codec_np;
> > > +	struct platform_device *saif_pdev[2];
> > > +	int i, ret = 0;
> > > +
> > > +	if (!np)
> > > +		return 1; /* no device tree */
> > > +
> > > +	saif_np[0] = of_parse_phandle(np, "saif-controllers", 0);
> > > +	saif_np[1] = of_parse_phandle(np, "saif-controllers", 1);
> > > +	codec_np = of_parse_phandle(np, "audio-codec", 0);
> > > +	if (!saif_np[0] || !saif_np[1] || !codec_np) {
> > > +		dev_err(&pdev->dev, "phandle missing or invalid\n");
> > > +		return -EINVAL;
> > > +	}
> > ...
> > 
> > > +
> > > +	saif_pdev[0] = of_find_device_by_node(saif_np[0]);
> > > +	saif_pdev[1] = of_find_device_by_node(saif_np[1]);
> > > +	if (!saif_pdev[0] || !saif_pdev[1]) {
> > > +		dev_err(&pdev->dev, "failed to find saif platform device\n");
> > > +		ret = -EINVAL;
> > > +		goto out;
> > > +	}
> > Do we really need this checking?
> > 
> Hmm, why not?  We need to ensure that the phandles are correctly
> represented in device tree.
> 
Hmm, even we need to do sanity check, i guess this could be done in
core since the core will take these nodes as parameter.
However, after checking the core a bit, i guess the core has already
done this:

/* no, then find CPU DAI from registered DAIs*/
list_for_each_entry(cpu_dai, &dai_list, list) {
	if (dai_link->cpu_dai_of_node) {
		if (cpu_dai->dev->of_node != dai_link->cpu_dai_of_node)
			continue;
	} else {
		if (strcmp(cpu_dai->name, dai_link->cpu_dai_name))
			continue;
	}

	rtd->cpu_dai = cpu_dai;
	goto find_codec;
}

It looks if the phandles are not correctly represented, the dai binding
will fail, right?

Regards
Dong Aisheng

Re: [PATCH 5/6] ASoC: mxs: add device tree support for mxs-sgtl5000

[Shawn Guo]

On Fri, May 11, 2012 at 10:33:03AM +0800, Dong Aisheng wrote:
> Hmm, even we need to do sanity check, i guess this could be done in
> core since the core will take these nodes as parameter.
> However, after checking the core a bit, i guess the core has already
> done this:
> 
> /* no, then find CPU DAI from registered DAIs*/
> list_for_each_entry(cpu_dai, &dai_list, list) {
> 	if (dai_link->cpu_dai_of_node) {
> 		if (cpu_dai->dev->of_node != dai_link->cpu_dai_of_node)
> 			continue;
> 	} else {
> 		if (strcmp(cpu_dai->name, dai_link->cpu_dai_name))
> 			continue;
> 	}
> 
> 	rtd->cpu_dai = cpu_dai;
> 	goto find_codec;
> }
> 
> It looks if the phandles are not correctly represented, the dai binding
> will fail, right?
> 
Yes, the core will have the sanity check and fail out without reporting
what's exactly going wrong, unless you turn on ASoC core debug option.
So what's harm to have the check in driver to fail out as early as
possible and with the exact message reporting failure cause?

-- 
Regards,
Shawn

Re: [PATCH 5/6] ASoC: mxs: add device tree support for mxs-sgtl5000

[Shawn Guo]

On Thu, May 10, 2012 at 04:42:12PM +0800, Shawn Guo wrote:
> Add device tree probe for mxs-sgtl5000 machine driver.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  .../bindings/sound/mxs-audio-sgtl5000.txt          |   17 ++++++
>  sound/soc/mxs/mxs-sgtl5000.c                       |   57 ++++++++++++++++++++
>  2 files changed, 74 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/sound/mxs-audio-sgtl5000.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/mxs-audio-sgtl5000.txt b/Documentation/devicetree/bindings/sound/mxs-audio-sgtl5000.txt
> new file mode 100644
> index 0000000..601c518
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/mxs-audio-sgtl5000.txt
> @@ -0,0 +1,17 @@
> +* Freescale MXS audio complex with SGTL5000 codec
> +
> +Required properties:
> +- compatible: "fsl,mxs-audio-sgtl5000"
> +- model: The user-visible name of this sound complex
> +- saif-controllers: The phandle list of the MXS SAIF controller
> +- audio-codec: The phandle of the SGTL5000 audio codec
> +
> +Example:
> +
> +sound {
> +	compatible = "fsl,imx28-evk-sgtl5000",
> +		     "fsl,mxs-audio-sgtl5000";
> +	model = "imx28-evk-sgtl5000";
> +	saif-controllers = <&saif0 &saif1>;
> +	audio-codec = <&sgtl5000>;
> +};
> diff --git a/sound/soc/mxs/mxs-sgtl5000.c b/sound/soc/mxs/mxs-sgtl5000.c
> index e9e6112..d420f11 100644
> --- a/sound/soc/mxs/mxs-sgtl5000.c
> +++ b/sound/soc/mxs/mxs-sgtl5000.c
> @@ -18,6 +18,8 @@
>  
>  #include <linux/module.h>
>  #include <linux/device.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <sound/core.h>
>  #include <sound/pcm.h>
>  #include <sound/soc.h>
> @@ -110,11 +112,58 @@ static struct snd_soc_card mxs_sgtl5000 = {
>  	.num_links	= ARRAY_SIZE(mxs_sgtl5000_dai),
>  };
>  
> +static int __devinit mxs_sgtl5000_probe_dt(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *saif_np[2], *codec_np;
> +	struct platform_device *saif_pdev[2];

The saif_pdev is a left-over which can be removed completely now.

> +	int i, ret = 0;
> +
> +	if (!np)
> +		return 1; /* no device tree */
> +
> +	saif_np[0] = of_parse_phandle(np, "saif-controllers", 0);
> +	saif_np[1] = of_parse_phandle(np, "saif-controllers", 1);
> +	codec_np = of_parse_phandle(np, "audio-codec", 0);
> +	if (!saif_np[0] || !saif_np[1] || !codec_np) {
> +		dev_err(&pdev->dev, "phandle missing or invalid\n");
> +		return -EINVAL;
> +	}
> +
> +	saif_pdev[0] = of_find_device_by_node(saif_np[0]);
> +	saif_pdev[1] = of_find_device_by_node(saif_np[1]);
> +	if (!saif_pdev[0] || !saif_pdev[1]) {
> +		dev_err(&pdev->dev, "failed to find saif platform device\n");
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	for (i = 0; i < 2; i++) {
> +		mxs_sgtl5000_dai[i].codec_name = NULL;
> +		mxs_sgtl5000_dai[i].codec_of_node = codec_np;
> +		mxs_sgtl5000_dai[i].cpu_dai_name = NULL;
> +		mxs_sgtl5000_dai[i].cpu_dai_of_node = saif_np[i];
> +		mxs_sgtl5000_dai[i].platform_name = NULL;
> +		mxs_sgtl5000_dai[i].platform_of_node = saif_np[i];
> +	}
> +
> +out:

The label is used nowhere now.  Will remove it in v2.

> +	of_node_put(codec_np);
> +	of_node_put(saif_np[0]);
> +	of_node_put(saif_np[1]);
> +
> +	return ret;
> +}
> +

-- 
Regards,
Shawn

[PATCH 6/6] ARM: dts: enable audio support for imx28-evk

[Shawn Guo]

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 arch/arm/boot/dts/imx28-evk.dts |   41 +++++++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/imx28.dtsi    |   23 +++++++++++++++++++++
 2 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/imx28-evk.dts b/arch/arm/boot/dts/imx28-evk.dts
index c4b717f..a07fc92 100644
--- a/arch/arm/boot/dts/imx28-evk.dts
+++ b/arch/arm/boot/dts/imx28-evk.dts
@@ -41,10 +41,31 @@
 		};
 
 		apbx@80040000 {
+			saif0: saif@80042000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&saif0_pins_a>;
+				status = "okay";
+			};
+
+			saif1: saif@80046000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&saif1_pins_a>;
+				fsl,saif-master = <&saif0>;
+				status = "okay";
+			};
+
 			i2c0: i2c@80058000 {
 				pinctrl-names = "default";
 				pinctrl-0 = <&i2c0_pins_a>;
 				status = "okay";
+
+				sgtl5000: codec@0a {
+					compatible = "fsl,sgtl5000";
+					reg = <0x0a>;
+					VDDA-supply = <&reg_3p3v>;
+					VDDIO-supply = <&reg_3p3v>;
+
+				};
 			};
 
 			duart: serial@80074000 {
@@ -70,4 +91,24 @@
 			status = "okay";
 		};
 	};
+
+	regulators {
+		compatible = "simple-bus";
+
+		reg_3p3v: 3p3v {
+			compatible = "regulator-fixed";
+			regulator-name = "3P3V";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-always-on;
+		};
+	};
+
+	sound {
+		compatible = "fsl,imx28-evk-sgtl5000",
+			     "fsl,mxs-audio-sgtl5000";
+		model = "imx28-evk-sgtl5000";
+		saif-controllers = <&saif0 &saif1>;
+		audio-codec = <&sgtl5000>;
+	};
 };
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index fd73fc2..4634cb8 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -20,6 +20,8 @@
 		gpio2 = &gpio2;
 		gpio3 = &gpio3;
 		gpio4 = &gpio4;
+		saif0 = &saif0;
+		saif1 = &saif1;
 	};
 
 	cpus {
@@ -212,6 +214,23 @@
 					fsl,voltage = <1>;
 					fsl,pull-up = <1>;
 				};
+
+				saif0_pins_a: saif0@0 {
+					reg = <0>;
+					fsl,pinmux-ids =
+						<0x3140 0x3150 0x3160 0x3170>;
+					fsl,drive-strength = <2>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <1>;
+				};
+
+				saif1_pins_a: saif1@0 {
+					reg = <0>;
+					fsl,pinmux-ids = <0x31a0>;
+					fsl,drive-strength = <2>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <1>;
+				};
 			};
 
 			digctl@8001c000 {
@@ -319,8 +338,10 @@
 			};
 
 			saif0: saif@80042000 {
+				compatible = "fsl,imx28-saif";
 				reg = <0x80042000 2000>;
 				interrupts = <59 80>;
+				fsl,saif-dma-channel = <4>;
 				status = "disabled";
 			};
 
@@ -330,8 +351,10 @@
 			};
 
 			saif1: saif@80046000 {
+				compatible = "fsl,imx28-saif";
 				reg = <0x80046000 2000>;
 				interrupts = <58 81>;
+				fsl,saif-dma-channel = <5>;
 				status = "disabled";
 			};
 
-- 
1.7.5.4

Re: [PATCH 6/6] ARM: dts: enable audio support for imx28-evk

[Dong Aisheng]

On Thu, May 10, 2012 at 04:42:13PM +0800, Shawn Guo wrote:
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  arch/arm/boot/dts/imx28-evk.dts |   41 +++++++++++++++++++++++++++++++++++++++
>  arch/arm/boot/dts/imx28.dtsi    |   23 +++++++++++++++++++++
>  2 files changed, 64 insertions(+), 0 deletions(-)
> 
Looks ok to me.

Acked-by: Dong Aisheng <dong.aisheng@linaro.org>

Regards
Dong Aisheng