| Message ID | 20131021094613.2a07bd79@armhf |
|---|---|
| State | New |
| Headers | show |
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.
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?
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.
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.
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 */
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(-)