[PATCH 0/5] ASoC: (minor) atmel cleanup

[PATCH 0/5] ASoC: (minor) atmel cleanup

[Ladislav Michl]

Hi there!

this patchset is a side product of reading ALSA code while trying to support
new hardware. Nothing special here, just an attempt to make those three
drivers to look more similar. Only patch 4 fixes actual problem, but:
1) this particular issue is present almost everywhere (as far as I looked)
   where of_node_put is used in ALSA. Perhaps coccinelle people could help?
2) seems noone bothered as OF_DYNAMIC is mostly disabled

Compile tested only.

Ladislav Michl (5):
  ASoC: sam9g20_wm8731: use dev_*() logging functions
  ASoC: sam9x5_wm8731: Drop 'ASoC' prefix from error messages
  ASoC: sam9x5_wm8731: Return -ENODEV when probe does not find OF node
  ASoC: sam9x5_wm8731: Fix OF node reference counting
  ASoC: atmel_wm8904: Return -ENODEV when probe does not find OF node
  
 sound/soc/atmel/atmel_wm8904.c   |  6 ++----
 sound/soc/atmel/sam9g20_wm8731.c | 17 +++++++--------
 sound/soc/atmel/sam9x5_wm8731.c  | 45 +++++++++++++++++-----------------------
 3 files changed, 29 insertions(+), 39 deletions(-)

-- 
2.15.1

[PATCH 1/5] ASoC: sam9g20_wm8731: use dev_*() logging functions

[Ladislav Michl]

Use dev_*() logging functions instead of plain printk and as
messages are now properly prefixed drop 'ASoC' prefix as well.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 sound/soc/atmel/sam9g20_wm8731.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/sound/soc/atmel/sam9g20_wm8731.c b/sound/soc/atmel/sam9g20_wm8731.c
index d7469cdd90dc..98f93e79c654 100644
--- a/sound/soc/atmel/sam9g20_wm8731.c
+++ b/sound/soc/atmel/sam9g20_wm8731.c
@@ -110,16 +110,15 @@ static const struct snd_soc_dapm_route intercon[] = {
 static int at91sam9g20ek_wm8731_init(struct snd_soc_pcm_runtime *rtd)
 {
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	struct device *dev = rtd->dev;
 	int ret;
 
-	printk(KERN_DEBUG
-			"at91sam9g20ek_wm8731 "
-			": at91sam9g20ek_wm8731_init() called\n");
+	dev_dbg(dev, "%s called\n", __func__);
 
 	ret = snd_soc_dai_set_sysclk(codec_dai, WM8731_SYSCLK_MCLK,
-		MCLK_RATE, SND_SOC_CLOCK_IN);
+				     MCLK_RATE, SND_SOC_CLOCK_IN);
 	if (ret < 0) {
-		printk(KERN_ERR "Failed to set WM8731 SYSCLK: %d\n", ret);
+		dev_err(dev, "Failed to set WM8731 SYSCLK: %d\n", ret);
 		return ret;
 	}
 
@@ -179,21 +178,21 @@ static int at91sam9g20ek_audio_probe(struct platform_device *pdev)
 	 */
 	mclk = clk_get(NULL, "pck0");
 	if (IS_ERR(mclk)) {
-		printk(KERN_ERR "ASoC: Failed to get MCLK\n");
+		dev_err(&pdev->dev, "Failed to get MCLK\n");
 		ret = PTR_ERR(mclk);
 		goto err;
 	}
 
 	pllb = clk_get(NULL, "pllb");
 	if (IS_ERR(pllb)) {
-		printk(KERN_ERR "ASoC: Failed to get PLLB\n");
+		dev_err(&pdev->dev, "Failed to get PLLB\n");
 		ret = PTR_ERR(pllb);
 		goto err_mclk;
 	}
 	ret = clk_set_parent(mclk, pllb);
 	clk_put(pllb);
 	if (ret != 0) {
-		printk(KERN_ERR "ASoC: Failed to set MCLK parent\n");
+		dev_err(&pdev->dev, "Failed to set MCLK parent\n");
 		goto err_mclk;
 	}
 
@@ -236,7 +235,7 @@ static int at91sam9g20ek_audio_probe(struct platform_device *pdev)
 
 	ret = snd_soc_register_card(card);
 	if (ret) {
-		printk(KERN_ERR "ASoC: snd_soc_register_card() failed\n");
+		dev_err(&pdev->dev, "snd_soc_register_card() failed\n");
 	}
 
 	return ret;
-- 
2.15.1

Applied "ASoC: sam9g20_wm8731: use dev_*() logging functions" to the asoc tree

[Mark Brown]

The patch

   ASoC: sam9g20_wm8731: use dev_*() logging functions

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From edefc8f387a86c2db943db6accd1cd23be2b64a9 Mon Sep 17 00:00:00 2001
From: Ladislav Michl <ladis@linux-mips.org>
Date: Mon, 15 Jan 2018 08:52:54 +0100
Subject: [PATCH] ASoC: sam9g20_wm8731: use dev_*() logging functions

Use dev_*() logging functions instead of plain printk and as
messages are now properly prefixed drop 'ASoC' prefix as well.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/atmel/sam9g20_wm8731.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/sound/soc/atmel/sam9g20_wm8731.c b/sound/soc/atmel/sam9g20_wm8731.c
index d7469cdd90dc..98f93e79c654 100644
--- a/sound/soc/atmel/sam9g20_wm8731.c
+++ b/sound/soc/atmel/sam9g20_wm8731.c
@@ -110,16 +110,15 @@ static const struct snd_soc_dapm_route intercon[] = {
 static int at91sam9g20ek_wm8731_init(struct snd_soc_pcm_runtime *rtd)
 {
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	struct device *dev = rtd->dev;
 	int ret;
 
-	printk(KERN_DEBUG
-			"at91sam9g20ek_wm8731 "
-			": at91sam9g20ek_wm8731_init() called\n");
+	dev_dbg(dev, "%s called\n", __func__);
 
 	ret = snd_soc_dai_set_sysclk(codec_dai, WM8731_SYSCLK_MCLK,
-		MCLK_RATE, SND_SOC_CLOCK_IN);
+				     MCLK_RATE, SND_SOC_CLOCK_IN);
 	if (ret < 0) {
-		printk(KERN_ERR "Failed to set WM8731 SYSCLK: %d\n", ret);
+		dev_err(dev, "Failed to set WM8731 SYSCLK: %d\n", ret);
 		return ret;
 	}
 
@@ -179,21 +178,21 @@ static int at91sam9g20ek_audio_probe(struct platform_device *pdev)
 	 */
 	mclk = clk_get(NULL, "pck0");
 	if (IS_ERR(mclk)) {
-		printk(KERN_ERR "ASoC: Failed to get MCLK\n");
+		dev_err(&pdev->dev, "Failed to get MCLK\n");
 		ret = PTR_ERR(mclk);
 		goto err;
 	}
 
 	pllb = clk_get(NULL, "pllb");
 	if (IS_ERR(pllb)) {
-		printk(KERN_ERR "ASoC: Failed to get PLLB\n");
+		dev_err(&pdev->dev, "Failed to get PLLB\n");
 		ret = PTR_ERR(pllb);
 		goto err_mclk;
 	}
 	ret = clk_set_parent(mclk, pllb);
 	clk_put(pllb);
 	if (ret != 0) {
-		printk(KERN_ERR "ASoC: Failed to set MCLK parent\n");
+		dev_err(&pdev->dev, "Failed to set MCLK parent\n");
 		goto err_mclk;
 	}
 
@@ -236,7 +235,7 @@ static int at91sam9g20ek_audio_probe(struct platform_device *pdev)
 
 	ret = snd_soc_register_card(card);
 	if (ret) {
-		printk(KERN_ERR "ASoC: snd_soc_register_card() failed\n");
+		dev_err(&pdev->dev, "snd_soc_register_card() failed\n");
 	}
 
 	return ret;
-- 
2.15.1

[PATCH 2/5] ASoC: sam9x5_wm8731: Drop 'ASoC' prefix from error messages

[Ladislav Michl]

dev_err already provides messages with prefix, so drop 'ASoC'
prefix.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 sound/soc/atmel/sam9x5_wm8731.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/sound/soc/atmel/sam9x5_wm8731.c b/sound/soc/atmel/sam9x5_wm8731.c
index ccdf547f4d8c..e6c303ab869d 100644
--- a/sound/soc/atmel/sam9x5_wm8731.c
+++ b/sound/soc/atmel/sam9x5_wm8731.c
@@ -49,13 +49,13 @@ static int sam9x5_wm8731_init(struct snd_soc_pcm_runtime *rtd)
 	struct device *dev = rtd->dev;
 	int ret;
 
-	dev_dbg(dev, "ASoC: %s called\n", __func__);
+	dev_dbg(dev, "%s called\n", __func__);
 
 	/* set the codec system clock for DAC and ADC */
 	ret = snd_soc_dai_set_sysclk(codec_dai, WM8731_SYSCLK_XTAL,
 				     MCLK_RATE, SND_SOC_CLOCK_IN);
 	if (ret < 0) {
-		dev_err(dev, "ASoC: Failed to set WM8731 SYSCLK: %d\n", ret);
+		dev_err(dev, "Failed to set WM8731 SYSCLK: %d\n", ret);
 		return ret;
 	}
 
@@ -146,8 +146,7 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
 
 	ret = atmel_ssc_set_audio(priv->ssc_id);
 	if (ret != 0) {
-		dev_err(&pdev->dev,
-			"ASoC: Failed to set SSC %d for audio: %d\n",
+		dev_err(&pdev->dev, "Failed to set SSC %d for audio: %d\n",
 			ret, priv->ssc_id);
 		goto out;
 	}
@@ -157,12 +156,11 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
 
 	ret = snd_soc_register_card(card);
 	if (ret) {
-		dev_err(&pdev->dev,
-			"ASoC: Platform device allocation failed\n");
+		dev_err(&pdev->dev, "Platform device allocation failed\n");
 		goto out_put_audio;
 	}
 
-	dev_dbg(&pdev->dev, "ASoC: %s ok\n", __func__);
+	dev_dbg(&pdev->dev, "%s ok\n", __func__);
 
 	return ret;
 
-- 
2.15.1

Applied "ASoC: sam9x5_wm8731: Drop 'ASoC' prefix from error messages" to the asoc tree

[Mark Brown]

The patch

   ASoC: sam9x5_wm8731: Drop 'ASoC' prefix from error messages

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 2efb8a8f11ba92971551189da91ece8b5040e461 Mon Sep 17 00:00:00 2001
From: Ladislav Michl <ladis@linux-mips.org>
Date: Mon, 15 Jan 2018 08:53:21 +0100
Subject: [PATCH] ASoC: sam9x5_wm8731: Drop 'ASoC' prefix from error messages

dev_err already provides messages with prefix, so drop 'ASoC'
prefix.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/atmel/sam9x5_wm8731.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/sound/soc/atmel/sam9x5_wm8731.c b/sound/soc/atmel/sam9x5_wm8731.c
index ccdf547f4d8c..e6c303ab869d 100644
--- a/sound/soc/atmel/sam9x5_wm8731.c
+++ b/sound/soc/atmel/sam9x5_wm8731.c
@@ -49,13 +49,13 @@ static int sam9x5_wm8731_init(struct snd_soc_pcm_runtime *rtd)
 	struct device *dev = rtd->dev;
 	int ret;
 
-	dev_dbg(dev, "ASoC: %s called\n", __func__);
+	dev_dbg(dev, "%s called\n", __func__);
 
 	/* set the codec system clock for DAC and ADC */
 	ret = snd_soc_dai_set_sysclk(codec_dai, WM8731_SYSCLK_XTAL,
 				     MCLK_RATE, SND_SOC_CLOCK_IN);
 	if (ret < 0) {
-		dev_err(dev, "ASoC: Failed to set WM8731 SYSCLK: %d\n", ret);
+		dev_err(dev, "Failed to set WM8731 SYSCLK: %d\n", ret);
 		return ret;
 	}
 
@@ -146,8 +146,7 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
 
 	ret = atmel_ssc_set_audio(priv->ssc_id);
 	if (ret != 0) {
-		dev_err(&pdev->dev,
-			"ASoC: Failed to set SSC %d for audio: %d\n",
+		dev_err(&pdev->dev, "Failed to set SSC %d for audio: %d\n",
 			ret, priv->ssc_id);
 		goto out;
 	}
@@ -157,12 +156,11 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
 
 	ret = snd_soc_register_card(card);
 	if (ret) {
-		dev_err(&pdev->dev,
-			"ASoC: Platform device allocation failed\n");
+		dev_err(&pdev->dev, "Platform device allocation failed\n");
 		goto out_put_audio;
 	}
 
-	dev_dbg(&pdev->dev, "ASoC: %s ok\n", __func__);
+	dev_dbg(&pdev->dev, "%s ok\n", __func__);
 
 	return ret;
 
-- 
2.15.1

[PATCH 3/5] ASoC: sam9x5_wm8731: Return -ENODEV when probe does not find OF node

[Ladislav Michl]

Probe should fail with -ENODEV when called with NULL pdev->dev.of_node.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 sound/soc/atmel/sam9x5_wm8731.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/sound/soc/atmel/sam9x5_wm8731.c b/sound/soc/atmel/sam9x5_wm8731.c
index e6c303ab869d..9db08826b32a 100644
--- a/sound/soc/atmel/sam9x5_wm8731.c
+++ b/sound/soc/atmel/sam9x5_wm8731.c
@@ -84,10 +84,8 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
 	struct sam9x5_drvdata *priv;
 	int ret;
 
-	if (!np) {
-		dev_err(&pdev->dev, "No device node supplied\n");
-		return -EINVAL;
-	}
+	if (!np)
+		return -ENODEV;
 
 	card = devm_kzalloc(&pdev->dev, sizeof(*card), GFP_KERNEL);
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
-- 
2.15.1

Re: [PATCH 3/5] ASoC: sam9x5_wm8731: Return -ENODEV when probe does not find OF node

[Alexandre Belloni]

On 15/01/2018 at 08:53:51 +0100, Ladislav Michl wrote:
> Probe should fail with -ENODEV when called with NULL pdev->dev.of_node.
> 
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> ---
>  sound/soc/atmel/sam9x5_wm8731.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/atmel/sam9x5_wm8731.c b/sound/soc/atmel/sam9x5_wm8731.c
> index e6c303ab869d..9db08826b32a 100644
> --- a/sound/soc/atmel/sam9x5_wm8731.c
> +++ b/sound/soc/atmel/sam9x5_wm8731.c
> @@ -84,10 +84,8 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
>  	struct sam9x5_drvdata *priv;
>  	int ret;
>  
> -	if (!np) {
> -		dev_err(&pdev->dev, "No device node supplied\n");
> -		return -EINVAL;
> -	}
> +	if (!np)
> +		return -ENODEV;
>  

This will never happen, you may as well just remove the test.

>  	card = devm_kzalloc(&pdev->dev, sizeof(*card), GFP_KERNEL);
>  	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> -- 
> 2.15.1
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 3/5] ASoC: sam9x5_wm8731: Return -ENODEV when probe does not find OF node

[Ladislav Michl]

On Mon, Jan 15, 2018 at 09:20:25AM +0100, Alexandre Belloni wrote:
> On 15/01/2018 at 08:53:51 +0100, Ladislav Michl wrote:
> > Probe should fail with -ENODEV when called with NULL pdev->dev.of_node.
> > 
> > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> > ---
> >  sound/soc/atmel/sam9x5_wm8731.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/sound/soc/atmel/sam9x5_wm8731.c b/sound/soc/atmel/sam9x5_wm8731.c
> > index e6c303ab869d..9db08826b32a 100644
> > --- a/sound/soc/atmel/sam9x5_wm8731.c
> > +++ b/sound/soc/atmel/sam9x5_wm8731.c
> > @@ -84,10 +84,8 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
> >  	struct sam9x5_drvdata *priv;
> >  	int ret;
> >  
> > -	if (!np) {
> > -		dev_err(&pdev->dev, "No device node supplied\n");
> > -		return -EINVAL;
> > -	}
> > +	if (!np)
> > +		return -ENODEV;
> >  
> 
> This will never happen, you may as well just remove the test.

Ah, right, this is DT only driver. And the same for the patch 5.

Btw, all that drivers seems to be for development boards, so what
about moving to simple card instead? (and let these special drivers
die silently)

> >  	card = devm_kzalloc(&pdev->dev, sizeof(*card), GFP_KERNEL);
> >  	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > -- 
> > 2.15.1
> > 
> 
> -- 
> Alexandre Belloni, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

Re: [PATCH 3/5] ASoC: sam9x5_wm8731: Return -ENODEV when probe does not find OF node

[Alexandre Belloni]

On 15/01/2018 at 10:11:14 +0100, Ladislav Michl wrote:
> On Mon, Jan 15, 2018 at 09:20:25AM +0100, Alexandre Belloni wrote:
> > On 15/01/2018 at 08:53:51 +0100, Ladislav Michl wrote:
> > > Probe should fail with -ENODEV when called with NULL pdev->dev.of_node.
> > > 
> > > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> > > ---
> > >  sound/soc/atmel/sam9x5_wm8731.c | 6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/sound/soc/atmel/sam9x5_wm8731.c b/sound/soc/atmel/sam9x5_wm8731.c
> > > index e6c303ab869d..9db08826b32a 100644
> > > --- a/sound/soc/atmel/sam9x5_wm8731.c
> > > +++ b/sound/soc/atmel/sam9x5_wm8731.c
> > > @@ -84,10 +84,8 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
> > >  	struct sam9x5_drvdata *priv;
> > >  	int ret;
> > >  
> > > -	if (!np) {
> > > -		dev_err(&pdev->dev, "No device node supplied\n");
> > > -		return -EINVAL;
> > > -	}
> > > +	if (!np)
> > > +		return -ENODEV;
> > >  
> > 
> > This will never happen, you may as well just remove the test.
> 
> Ah, right, this is DT only driver. And the same for the patch 5.
> 
> Btw, all that drivers seems to be for development boards, so what
> about moving to simple card instead? (and let these special drivers
> die silently)
> 

Yes, that would be good, I'm all for it. However, I think some custom
boards are using those drivers and are not upstream and will also need
to be migrated or at least have a proper migration path.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH 4/5] ASoC: sam9x5_wm8731: Fix OF node reference counting

[Ladislav Michl]

of_node_release() is called as the destructor on a dynamically
allocated node in of_node_put(), but node pointer is still in use.
Fix that by moving of_node_put() into the error path.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 sound/soc/atmel/sam9x5_wm8731.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/sound/soc/atmel/sam9x5_wm8731.c b/sound/soc/atmel/sam9x5_wm8731.c
index 9db08826b32a..bcfb474ee0e8 100644
--- a/sound/soc/atmel/sam9x5_wm8731.c
+++ b/sound/soc/atmel/sam9x5_wm8731.c
@@ -78,7 +78,6 @@ static const struct snd_soc_dapm_widget sam9x5_dapm_widgets[] = {
 static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
-	struct device_node *codec_np, *cpu_np;
 	struct snd_soc_card *card;
 	struct snd_soc_dai_link *dai;
 	struct sam9x5_drvdata *priv;
@@ -122,36 +121,30 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
 		goto out;
 	}
 
-	codec_np = of_parse_phandle(np, "atmel,audio-codec", 0);
-	if (!codec_np) {
+	dai->codec_of_node = of_parse_phandle(np, "atmel,audio-codec", 0);
+	if (!dai->codec_of_node) {
 		dev_err(&pdev->dev, "atmel,audio-codec node missing\n");
 		ret = -EINVAL;
 		goto out;
 	}
 
-	dai->codec_of_node = codec_np;
-
-	cpu_np = of_parse_phandle(np, "atmel,ssc-controller", 0);
-	if (!cpu_np) {
+	dai->cpu_of_node = of_parse_phandle(np, "atmel,ssc-controller", 0);
+	if (!dai->cpu_of_node) {
 		dev_err(&pdev->dev, "atmel,ssc-controller node missing\n");
 		ret = -EINVAL;
-		goto out;
+		goto out_put_codec;
 	}
-	dai->cpu_of_node = cpu_np;
-	dai->platform_of_node = cpu_np;
+	dai->platform_of_node = dai->cpu_of_node;
 
-	priv->ssc_id = of_alias_get_id(cpu_np, "ssc");
+	priv->ssc_id = of_alias_get_id(dai->cpu_of_node, "ssc");
 
 	ret = atmel_ssc_set_audio(priv->ssc_id);
 	if (ret != 0) {
 		dev_err(&pdev->dev, "Failed to set SSC %d for audio: %d\n",
 			ret, priv->ssc_id);
-		goto out;
+		goto out_put_cpu;
 	}
 
-	of_node_put(codec_np);
-	of_node_put(cpu_np);
-
 	ret = snd_soc_register_card(card);
 	if (ret) {
 		dev_err(&pdev->dev, "Platform device allocation failed\n");
@@ -164,6 +157,10 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
 
 out_put_audio:
 	atmel_ssc_put_audio(priv->ssc_id);
+out_put_cpu:
+	of_node_put(dai->cpu_of_node);
+out_put_codec:
+	of_node_put(dai->codec_of_node);
 out:
 	return ret;
 }
-- 
2.15.1

Re: [PATCH 4/5] ASoC: sam9x5_wm8731: Fix OF node reference counting

[Alexandre Belloni]

On 15/01/2018 at 08:54:30 +0100, Ladislav Michl wrote:
> of_node_release() is called as the destructor on a dynamically
> allocated node in of_node_put(), but node pointer is still in use.
> Fix that by moving of_node_put() into the error path.
> 
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> ---
>  sound/soc/atmel/sam9x5_wm8731.c | 27 ++++++++++++---------------
>  1 file changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/sound/soc/atmel/sam9x5_wm8731.c b/sound/soc/atmel/sam9x5_wm8731.c
> index 9db08826b32a..bcfb474ee0e8 100644
> --- a/sound/soc/atmel/sam9x5_wm8731.c
> +++ b/sound/soc/atmel/sam9x5_wm8731.c
> @@ -78,7 +78,6 @@ static const struct snd_soc_dapm_widget sam9x5_dapm_widgets[] = {
>  static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
> -	struct device_node *codec_np, *cpu_np;
>  	struct snd_soc_card *card;
>  	struct snd_soc_dai_link *dai;
>  	struct sam9x5_drvdata *priv;
> @@ -122,36 +121,30 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
>  		goto out;
>  	}
>  
> -	codec_np = of_parse_phandle(np, "atmel,audio-codec", 0);
> -	if (!codec_np) {
> +	dai->codec_of_node = of_parse_phandle(np, "atmel,audio-codec", 0);
> +	if (!dai->codec_of_node) {
>  		dev_err(&pdev->dev, "atmel,audio-codec node missing\n");
>  		ret = -EINVAL;
>  		goto out;
>  	}
>  
> -	dai->codec_of_node = codec_np;
> -
> -	cpu_np = of_parse_phandle(np, "atmel,ssc-controller", 0);
> -	if (!cpu_np) {
> +	dai->cpu_of_node = of_parse_phandle(np, "atmel,ssc-controller", 0);
> +	if (!dai->cpu_of_node) {
>  		dev_err(&pdev->dev, "atmel,ssc-controller node missing\n");
>  		ret = -EINVAL;
> -		goto out;
> +		goto out_put_codec;
>  	}
> -	dai->cpu_of_node = cpu_np;
> -	dai->platform_of_node = cpu_np;
> +	dai->platform_of_node = dai->cpu_of_node;
>  
> -	priv->ssc_id = of_alias_get_id(cpu_np, "ssc");
> +	priv->ssc_id = of_alias_get_id(dai->cpu_of_node, "ssc");
>  
>  	ret = atmel_ssc_set_audio(priv->ssc_id);
>  	if (ret != 0) {
>  		dev_err(&pdev->dev, "Failed to set SSC %d for audio: %d\n",
>  			ret, priv->ssc_id);
> -		goto out;
> +		goto out_put_cpu;
>  	}
>  
> -	of_node_put(codec_np);
> -	of_node_put(cpu_np);
> -
>  	ret = snd_soc_register_card(card);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Platform device allocation failed\n");
> @@ -164,6 +157,10 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
>  
>  out_put_audio:
>  	atmel_ssc_put_audio(priv->ssc_id);
> +out_put_cpu:
> +	of_node_put(dai->cpu_of_node);

Should'nt they be put in sam9x5_wm8731_driver_remove then? Did you test
any of those changes?

> +out_put_codec:
> +	of_node_put(dai->codec_of_node);
>  out:
>  	return ret;
>  }
> -- 
> 2.15.1
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 4/5] ASoC: sam9x5_wm8731: Fix OF node reference counting

[Ladislav Michl]

On Mon, Jan 15, 2018 at 09:46:07AM +0100, Alexandre Belloni wrote:
> On 15/01/2018 at 08:54:30 +0100, Ladislav Michl wrote:
> > of_node_release() is called as the destructor on a dynamically
> > allocated node in of_node_put(), but node pointer is still in use.
> > Fix that by moving of_node_put() into the error path.
> > 
> > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> > ---
> >  sound/soc/atmel/sam9x5_wm8731.c | 27 ++++++++++++---------------
> >  1 file changed, 12 insertions(+), 15 deletions(-)
> > 
> > diff --git a/sound/soc/atmel/sam9x5_wm8731.c b/sound/soc/atmel/sam9x5_wm8731.c
> > index 9db08826b32a..bcfb474ee0e8 100644
> > --- a/sound/soc/atmel/sam9x5_wm8731.c
> > +++ b/sound/soc/atmel/sam9x5_wm8731.c
> > @@ -78,7 +78,6 @@ static const struct snd_soc_dapm_widget sam9x5_dapm_widgets[] = {
> >  static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
> >  {
> >  	struct device_node *np = pdev->dev.of_node;
> > -	struct device_node *codec_np, *cpu_np;
> >  	struct snd_soc_card *card;
> >  	struct snd_soc_dai_link *dai;
> >  	struct sam9x5_drvdata *priv;
> > @@ -122,36 +121,30 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
> >  		goto out;
> >  	}
> >  
> > -	codec_np = of_parse_phandle(np, "atmel,audio-codec", 0);
> > -	if (!codec_np) {
> > +	dai->codec_of_node = of_parse_phandle(np, "atmel,audio-codec", 0);
> > +	if (!dai->codec_of_node) {
> >  		dev_err(&pdev->dev, "atmel,audio-codec node missing\n");
> >  		ret = -EINVAL;
> >  		goto out;
> >  	}
> >  
> > -	dai->codec_of_node = codec_np;
> > -
> > -	cpu_np = of_parse_phandle(np, "atmel,ssc-controller", 0);
> > -	if (!cpu_np) {
> > +	dai->cpu_of_node = of_parse_phandle(np, "atmel,ssc-controller", 0);
> > +	if (!dai->cpu_of_node) {
> >  		dev_err(&pdev->dev, "atmel,ssc-controller node missing\n");
> >  		ret = -EINVAL;
> > -		goto out;
> > +		goto out_put_codec;
> >  	}
> > -	dai->cpu_of_node = cpu_np;
> > -	dai->platform_of_node = cpu_np;
> > +	dai->platform_of_node = dai->cpu_of_node;
> >  
> > -	priv->ssc_id = of_alias_get_id(cpu_np, "ssc");
> > +	priv->ssc_id = of_alias_get_id(dai->cpu_of_node, "ssc");
> >  
> >  	ret = atmel_ssc_set_audio(priv->ssc_id);
> >  	if (ret != 0) {
> >  		dev_err(&pdev->dev, "Failed to set SSC %d for audio: %d\n",
> >  			ret, priv->ssc_id);
> > -		goto out;
> > +		goto out_put_cpu;
> >  	}
> >  
> > -	of_node_put(codec_np);
> > -	of_node_put(cpu_np);
> > -
> >  	ret = snd_soc_register_card(card);
> >  	if (ret) {
> >  		dev_err(&pdev->dev, "Platform device allocation failed\n");
> > @@ -164,6 +157,10 @@ static int sam9x5_wm8731_driver_probe(struct platform_device *pdev)
> >  
> >  out_put_audio:
> >  	atmel_ssc_put_audio(priv->ssc_id);
> > +out_put_cpu:
> > +	of_node_put(dai->cpu_of_node);
> 
> Should'nt they be put in sam9x5_wm8731_driver_remove then?

No, sound core should do clean up as it is done in
asoc_simple_card_clean_reference. I didn't read whole related ALSA code (yet).
Perhaps core developers could bring a bit of light here?

> Did you test any of those changes?

No as I do not have hardware :)

> > +out_put_codec:
> > +	of_node_put(dai->codec_of_node);
> >  out:
> >  	return ret;
> >  }
> > -- 
> > 2.15.1
> > 
> 
> -- 
> Alexandre Belloni, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

[PATCH 5/5] ASoC: atmel_wm8904: Return -ENODEV when probe does not find OF node

[Ladislav Michl]

Probe should fail with -ENODEV when called with NULL pdev->dev.of_node.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 sound/soc/atmel/atmel_wm8904.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/sound/soc/atmel/atmel_wm8904.c b/sound/soc/atmel/atmel_wm8904.c
index fbc10f61eb55..39a73c4d19ec 100644
--- a/sound/soc/atmel/atmel_wm8904.c
+++ b/sound/soc/atmel/atmel_wm8904.c
@@ -85,10 +85,8 @@ static int atmel_asoc_wm8904_dt_init(struct platform_device *pdev)
 	struct snd_soc_dai_link *dailink = &atmel_asoc_wm8904_dailink;
 	int ret;
 
-	if (!np) {
-		dev_err(&pdev->dev, "only device tree supported\n");
-		return -EINVAL;
-	}
+	if (!np)
+		return -ENODEV;
 
 	ret = snd_soc_of_parse_card_name(card, "atmel,model");
 	if (ret) {
-- 
2.15.1