lkml.org 
[lkml]   [2017]   [Jul]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFT 2/2] ASoC: samsung: Add proper error paths to s3c24xx I2S driver
From
Date
Hi,


On Thursday 27 July 2017 12:27 AM, Krzysztof Kozlowski wrote:
> s3c2412_i2s_probe() might fail so driver has to revert work done by
> s3c_i2sv2_probe() (clock enabling). Missing doing this would lead to
> clock enable in-balance.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> ---
>
> Please, kindly test on S3C24xx hardware.
> ---
> sound/soc/samsung/s3c-i2s-v2.c | 9 +++++++++
> sound/soc/samsung/s3c-i2s-v2.h | 7 +++++++
> sound/soc/samsung/s3c2412-i2s.c | 11 +++++++++--
> 3 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/samsung/s3c-i2s-v2.c b/sound/soc/samsung/s3c-i2s-v2.c
> index 9b28046eea8e..df83f2730a53 100644
> --- a/sound/soc/samsung/s3c-i2s-v2.c
> +++ b/sound/soc/samsung/s3c-i2s-v2.c
> @@ -651,6 +651,15 @@ int s3c_i2sv2_probe(struct snd_soc_dai *dai,
> }
> EXPORT_SYMBOL_GPL(s3c_i2sv2_probe);
>
> +void s3c_i2sv2_cleanup(struct snd_soc_dai *dai,
> + struct s3c_i2sv2_info *i2s)
> +{
> + clk_disable(i2s->iis_pclk);
> + clk_put(i2s->iis_pclk);
> + i2s->iis_pclk = NULL;
> +}
> +EXPORT_SYMBOL_GPL(s3c_i2sv2_cleanup);
> +
> #ifdef CONFIG_PM
> static int s3c2412_i2s_suspend(struct snd_soc_dai *dai)
> {
> diff --git a/sound/soc/samsung/s3c-i2s-v2.h b/sound/soc/samsung/s3c-i2s-v2.h
> index 182d80564e37..3fca20f7a853 100644
> --- a/sound/soc/samsung/s3c-i2s-v2.h
> +++ b/sound/soc/samsung/s3c-i2s-v2.h
> @@ -92,6 +92,13 @@ extern int s3c_i2sv2_probe(struct snd_soc_dai *dai,
> unsigned long base);
>
> /**
> + * s3c_i2sv2_cleanup - cleanup resources allocated in s3c_i2sv2_probe
> + * @dai: The ASoC DAI structure supplied to the original probe.
> + * @i2s: Our local i2s structure to fill in.
> + */
> +extern void s3c_i2sv2_cleanup(struct snd_soc_dai *dai,
> + struct s3c_i2sv2_info *i2s);
> +/**
> * s3c_i2sv2_register_component - register component and dai with soc core
> * @dev: DAI device
> * @id: DAI ID
> diff --git a/sound/soc/samsung/s3c2412-i2s.c b/sound/soc/samsung/s3c2412-i2s.c
> index bcd1cbdeac93..005cd6db7d07 100644
> --- a/sound/soc/samsung/s3c2412-i2s.c
> +++ b/sound/soc/samsung/s3c2412-i2s.c
> @@ -65,7 +65,8 @@ static int s3c2412_i2s_probe(struct snd_soc_dai *dai)
> s3c2412_i2s.iis_cclk = devm_clk_get(dai->dev, "i2sclk");
> if (IS_ERR(s3c2412_i2s.iis_cclk)) {
> pr_err("failed to get i2sclk clock\n");
> - return PTR_ERR(s3c2412_i2s.iis_cclk);
> + ret = PTR_ERR(s3c2412_i2s.iis_cclk);
> + goto err;
> }
>
> /* Set MPLL as the source for IIS CLK */
> @@ -73,7 +74,7 @@ static int s3c2412_i2s_probe(struct snd_soc_dai *dai)
> clk_set_parent(s3c2412_i2s.iis_cclk, clk_get(NULL, "mpll"));
we can use devm_clk_get for "mpll" clock.
> ret = clk_prepare_enable(s3c2412_i2s.iis_cclk);
> if (ret)
> - return ret;
> + goto err;
>
> s3c2412_i2s.iis_cclk = s3c2412_i2s.iis_pclk;
Now s3c2412_i2s.iis_cclk and s3c2412_i2s.iis_pclk are holding "iis" clock.
Now no one handling "mpll" clock. If we will call s3c2412_i2s_remove.
It's means. we are disabling "iis" clock twice. Which is not correct.
>
> @@ -82,11 +83,17 @@ static int s3c2412_i2s_probe(struct snd_soc_dai *dai)
> S3C_GPIO_PULL_NONE);
>
> return 0;
> +
> +err:
> + s3c_i2sv2_cleanup(dai, &s3c2412_i2s);
> +
> + return ret;
> }
>
> static int s3c2412_i2s_remove(struct snd_soc_dai *dai)
> {
> clk_disable_unprepare(s3c2412_i2s.iis_cclk);
> + s3c_i2sv2_cleanup(dai, &s3c2412_i2s);
here, We are disabling "iis" clock twice. please check my previous comment.
>
> return 0;
> }
~arvind

\
 
 \ /
  Last update: 2017-07-27 07:12    [W:0.119 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site