[PATCH] ASoC: kirkwood: cleanup the external clock probe

[PATCH] ASoC: kirkwood: cleanup the external clock probe

[Jean-Francois Moine]

This patch checks the return value of the clk_prepare_enable of the
external clock and removes the test about a same internal and external
clock which would quite never occur and won't work in most cases
it would occur.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
This patch applies against broonie/sound for-next.
---
 sound/soc/kirkwood/kirkwood-i2s.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/sound/soc/kirkwood/kirkwood-i2s.c.old b/sound/soc/kirkwood/kirkwood-i2s.c.pat2
index d0504a2..c2433ea 100644
--- a/sound/soc/kirkwood/kirkwood-i2s.c.old
+++ b/sound/soc/kirkwood/kirkwood-i2s.c.pat2
@@ -500,14 +500,11 @@ static int kirkwood_i2s_dev_probe(struct platform_device *pdev)
 		if (PTR_ERR(priv->extclk) == -EPROBE_DEFER)
 			return -EPROBE_DEFER;
 	} else {
-		if (priv->extclk == priv->clk) {
-			devm_clk_put(&pdev->dev, priv->extclk);
-			priv->extclk = ERR_PTR(-EINVAL);
-		} else {
-			dev_info(&pdev->dev, "found external clock\n");
-			clk_prepare_enable(priv->extclk);
-			soc_dai = &kirkwood_i2s_dai_extclk;
-		}
+		dev_info(&pdev->dev, "found external clock\n");
+		ret = clk_prepare_enable(priv->extclk);
+		if (err < 0)
+			return err;
+		soc_dai = kirkwood_i2s_dai_extclk;
 	}
 
 	/* Some sensible defaults - this reflects the powerup values */

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ASoC: kirkwood: cleanup the external clock probe

[Russell King - ARM Linux]

On Mon, Oct 21, 2013 at 09:46:13AM +0200, Jean-Francois Moine wrote:
> This patch checks the return value of the clk_prepare_enable of the
> external clock and removes the test about a same internal and external
> clock which would quite never occur and won't work in most cases
> it would occur.

NAK.  It can occur.

Re: [PATCH] ASoC: kirkwood: cleanup the external clock probe

[Jean-Francois Moine]

On Mon, 21 Oct 2013 09:06:57 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Mon, Oct 21, 2013 at 09:46:13AM +0200, Jean-Francois Moine wrote:
> > This patch checks the return value of the clk_prepare_enable of the
> > external clock and removes the test about a same internal and external
> > clock which would quite never occur and won't work in most cases
> > it would occur.
> 
> NAK.  It can occur.

In which case? And, what would be the right treatment?

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ASoC: kirkwood: cleanup the external clock probe

[Russell King - ARM Linux]

On Mon, Oct 21, 2013 at 10:28:34AM +0200, Jean-Francois Moine wrote:
> On Mon, 21 Oct 2013 09:06:57 +0100
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> 
> > On Mon, Oct 21, 2013 at 09:46:13AM +0200, Jean-Francois Moine wrote:
> > > This patch checks the return value of the clk_prepare_enable of the
> > > external clock and removes the test about a same internal and external
> > > clock which would quite never occur and won't work in most cases
> > > it would occur.
> > 
> > NAK.  It can occur.
> 
> In which case? And, what would be the right treatment?

        priv->clk = devm_clk_get(&pdev->dev, NULL);
        priv->extclk = devm_clk_get(&pdev->dev, "extclk");

Supplying the first clock to this driver without a separate "extclk"
via clkdev will return it as the second clock.

Again, NAK.  Your change is against the principles of the clk API.

Re: [PATCH] ASoC: kirkwood: cleanup the external clock probe

[Jean-Francois Moine]

On Mon, 21 Oct 2013 09:52:00 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> > > NAK.  It can occur.  
> > 
> > In which case? And, what would be the right treatment?  
> 
>         priv->clk = devm_clk_get(&pdev->dev, NULL);
>         priv->extclk = devm_clk_get(&pdev->dev, "extclk");
> 
> Supplying the first clock to this driver without a separate "extclk"
> via clkdev will return it as the second clock.
> 
> Again, NAK.  Your change is against the principles of the clk API.

You are right.

Mark, please forget about this patch.

Thanks.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel