* [RFT v2 0/3] ASoC: samsung: s3c24xx: Fixes for error paths
@ 2017-07-27 17:13 Krzysztof Kozlowski
2017-07-27 17:13 ` [RFT v2 1/3] ASoC: samsung: Fix possible double iounmap on s3c24xx driver probe failure Krzysztof Kozlowski
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2017-07-27 17:13 UTC (permalink / raw)
To: Krzysztof Kozlowski, Sangbeom Kim, Sylwester Nawrocki,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
alsa-devel, linux-kernel
Cc: Arvind Yadav
Hi,
Changes since v1:
1. Add patch 2/3.
2. Drop assignment of iis_cclk=iis_pclk to have balance with clk disables error
and remove paths (pointed by Arvind).
Best regards,
Krzysztof
Krzysztof Kozlowski (3):
ASoC: samsung: Fix possible double iounmap on s3c24xx driver probe
failure
ASoC: samsung: Add missing prepare for iis clock of s3c24xx
ASoC: samsung: Add proper error paths to s3c24xx I2S driver
sound/soc/samsung/s3c-i2s-v2.c | 12 ++++++++++--
sound/soc/samsung/s3c-i2s-v2.h | 7 +++++++
sound/soc/samsung/s3c2412-i2s.c | 13 +++++++++----
3 files changed, 26 insertions(+), 6 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFT v2 1/3] ASoC: samsung: Fix possible double iounmap on s3c24xx driver probe failure
2017-07-27 17:13 [RFT v2 0/3] ASoC: samsung: s3c24xx: Fixes for error paths Krzysztof Kozlowski
@ 2017-07-27 17:13 ` Krzysztof Kozlowski
2017-07-28 4:39 ` Arvind Yadav
2017-07-27 17:13 ` [RFT v2 2/3] ASoC: samsung: Add missing prepare for iis clock of s3c24xx Krzysztof Kozlowski
2017-07-27 17:13 ` [RFT v2 3/3] ASoC: samsung: Add proper error paths to s3c24xx I2S driver Krzysztof Kozlowski
2 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2017-07-27 17:13 UTC (permalink / raw)
To: Krzysztof Kozlowski, Sangbeom Kim, Sylwester Nawrocki,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
alsa-devel, linux-kernel
Cc: Arvind Yadav
Commit 87b132bc0315 ("ASoC: samsung: s3c24{xx,12}-i2s: port to use
generic dmaengine API") moved ioremap() call from
s3c-i2s-v2.c:s3c_i2sv2_probe() to s3c2412-i2s.c:s3c2412_iis_dev_probe()
and converted it to devm- resource managed interface.
However the error path in first of them - s3c_i2sv2_probe() - was not
updated. If getting a iis clock in s3c_i2sv2_probe() failed, the
address space would be unmapped there. This could lead to:
1. double iounmap() later from devm-interface of s3c2412_iis_dev_probe()),
2. accessing the memory by other functions in s3c2412-i2s.c unit.
Anyway, the owner of this mapped region should be s3c2412-i2s.c because
it starts the mapping.
Affected are drivers for S3C24xx family although issue was not reproduced.
Fixes: 87b132bc0315 ("ASoC: samsung: s3c24{xx,12}-i2s: port to use generic dmaengine API")
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
Not marking as Cc-stable because this is theoretical problem, not
reproduced and also not tested.
Please, kindly test on S3C24xx hardware.
Changes since v1:
1. None
---
sound/soc/samsung/s3c-i2s-v2.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/sound/soc/samsung/s3c-i2s-v2.c b/sound/soc/samsung/s3c-i2s-v2.c
index ca522a95160b..9b28046eea8e 100644
--- a/sound/soc/samsung/s3c-i2s-v2.c
+++ b/sound/soc/samsung/s3c-i2s-v2.c
@@ -634,7 +634,6 @@ int s3c_i2sv2_probe(struct snd_soc_dai *dai,
i2s->iis_pclk = clk_get(dev, "iis");
if (IS_ERR(i2s->iis_pclk)) {
dev_err(dev, "failed to get iis_clock\n");
- iounmap(i2s->regs);
return -ENOENT;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [RFT v2 2/3] ASoC: samsung: Add missing prepare for iis clock of s3c24xx
2017-07-27 17:13 [RFT v2 0/3] ASoC: samsung: s3c24xx: Fixes for error paths Krzysztof Kozlowski
2017-07-27 17:13 ` [RFT v2 1/3] ASoC: samsung: Fix possible double iounmap on s3c24xx driver probe failure Krzysztof Kozlowski
@ 2017-07-27 17:13 ` Krzysztof Kozlowski
2017-07-28 4:41 ` Arvind Yadav
2017-07-28 10:18 ` Applied "ASoC: samsung: Add missing prepare for iis clock of s3c24xx" to the asoc tree Mark Brown
2017-07-27 17:13 ` [RFT v2 3/3] ASoC: samsung: Add proper error paths to s3c24xx I2S driver Krzysztof Kozlowski
2 siblings, 2 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2017-07-27 17:13 UTC (permalink / raw)
To: Krzysztof Kozlowski, Sangbeom Kim, Sylwester Nawrocki,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
alsa-devel, linux-kernel
Cc: Arvind Yadav
The s3c_i2sv2_probe() only enabled iis clock. Missing prepare isn't
probably fatal, because for SoC clocks this is usually no-op, but for
correctness this clock should be prepared.
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
Changes since v1:
1. New patch
---
sound/soc/samsung/s3c-i2s-v2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/samsung/s3c-i2s-v2.c b/sound/soc/samsung/s3c-i2s-v2.c
index 9b28046eea8e..3894bda06ebb 100644
--- a/sound/soc/samsung/s3c-i2s-v2.c
+++ b/sound/soc/samsung/s3c-i2s-v2.c
@@ -637,7 +637,7 @@ int s3c_i2sv2_probe(struct snd_soc_dai *dai,
return -ENOENT;
}
- clk_enable(i2s->iis_pclk);
+ clk_prepare_enable(i2s->iis_pclk);
/* Mark ourselves as in TXRX mode so we can run through our cleanup
* process without warnings. */
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [RFT v2 3/3] ASoC: samsung: Add proper error paths to s3c24xx I2S driver
2017-07-27 17:13 [RFT v2 0/3] ASoC: samsung: s3c24xx: Fixes for error paths Krzysztof Kozlowski
2017-07-27 17:13 ` [RFT v2 1/3] ASoC: samsung: Fix possible double iounmap on s3c24xx driver probe failure Krzysztof Kozlowski
2017-07-27 17:13 ` [RFT v2 2/3] ASoC: samsung: Add missing prepare for iis clock of s3c24xx Krzysztof Kozlowski
@ 2017-07-27 17:13 ` Krzysztof Kozlowski
2017-07-28 4:47 ` Arvind Yadav
2017-07-28 10:17 ` Applied "ASoC: samsung: Add proper error paths to s3c24xx I2S driver" to the asoc tree Mark Brown
2 siblings, 2 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2017-07-27 17:13 UTC (permalink / raw)
To: Krzysztof Kozlowski, Sangbeom Kim, Sylwester Nawrocki,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
alsa-devel, linux-kernel
Cc: Arvind Yadav
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.
Changes since v1:
1. Drop clk assignment to disable proper clock on remove()
---
sound/soc/samsung/s3c-i2s-v2.c | 9 +++++++++
sound/soc/samsung/s3c-i2s-v2.h | 7 +++++++
sound/soc/samsung/s3c2412-i2s.c | 13 +++++++++----
3 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/sound/soc/samsung/s3c-i2s-v2.c b/sound/soc/samsung/s3c-i2s-v2.c
index 3894bda06ebb..58c3e9bfc6b7 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_unprepare(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..cc0840fff5aa 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,20 +74,24 @@ static int s3c2412_i2s_probe(struct snd_soc_dai *dai)
clk_set_parent(s3c2412_i2s.iis_cclk, clk_get(NULL, "mpll"));
ret = clk_prepare_enable(s3c2412_i2s.iis_cclk);
if (ret)
- return ret;
-
- s3c2412_i2s.iis_cclk = s3c2412_i2s.iis_pclk;
+ goto err;
/* Configure the I2S pins (GPE0...GPE4) in correct mode */
s3c_gpio_cfgall_range(S3C2410_GPE(0), 5, S3C_GPIO_SFN(2),
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);
return 0;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFT v2 1/3] ASoC: samsung: Fix possible double iounmap on s3c24xx driver probe failure
2017-07-27 17:13 ` [RFT v2 1/3] ASoC: samsung: Fix possible double iounmap on s3c24xx driver probe failure Krzysztof Kozlowski
@ 2017-07-28 4:39 ` Arvind Yadav
0 siblings, 0 replies; 11+ messages in thread
From: Arvind Yadav @ 2017-07-28 4:39 UTC (permalink / raw)
To: Krzysztof Kozlowski, Sangbeom Kim, Sylwester Nawrocki,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
alsa-devel, linux-kernel
On Thursday 27 July 2017 10:43 PM, Krzysztof Kozlowski wrote:
> Commit 87b132bc0315 ("ASoC: samsung: s3c24{xx,12}-i2s: port to use
> generic dmaengine API") moved ioremap() call from
> s3c-i2s-v2.c:s3c_i2sv2_probe() to s3c2412-i2s.c:s3c2412_iis_dev_probe()
> and converted it to devm- resource managed interface.
>
> However the error path in first of them - s3c_i2sv2_probe() - was not
> updated. If getting a iis clock in s3c_i2sv2_probe() failed, the
> address space would be unmapped there. This could lead to:
> 1. double iounmap() later from devm-interface of s3c2412_iis_dev_probe()),
> 2. accessing the memory by other functions in s3c2412-i2s.c unit.
>
> Anyway, the owner of this mapped region should be s3c2412-i2s.c because
> it starts the mapping.
>
> Affected are drivers for S3C24xx family although issue was not reproduced.
>
> Fixes: 87b132bc0315 ("ASoC: samsung: s3c24{xx,12}-i2s: port to use generic dmaengine API")
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Acked-by: Arvind Yadav<arvind.yadav.cs@gmail.com>
> ---
>
> Not marking as Cc-stable because this is theoretical problem, not
> reproduced and also not tested.
>
> Please, kindly test on S3C24xx hardware.
>
> Changes since v1:
> 1. None
> ---
> sound/soc/samsung/s3c-i2s-v2.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/sound/soc/samsung/s3c-i2s-v2.c b/sound/soc/samsung/s3c-i2s-v2.c
> index ca522a95160b..9b28046eea8e 100644
> --- a/sound/soc/samsung/s3c-i2s-v2.c
> +++ b/sound/soc/samsung/s3c-i2s-v2.c
> @@ -634,7 +634,6 @@ int s3c_i2sv2_probe(struct snd_soc_dai *dai,
> i2s->iis_pclk = clk_get(dev, "iis");
> if (IS_ERR(i2s->iis_pclk)) {
> dev_err(dev, "failed to get iis_clock\n");
> - iounmap(i2s->regs);
> return -ENOENT;
> }
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFT v2 2/3] ASoC: samsung: Add missing prepare for iis clock of s3c24xx
2017-07-27 17:13 ` [RFT v2 2/3] ASoC: samsung: Add missing prepare for iis clock of s3c24xx Krzysztof Kozlowski
@ 2017-07-28 4:41 ` Arvind Yadav
2017-07-28 5:44 ` Krzysztof Kozlowski
2017-07-28 10:18 ` Applied "ASoC: samsung: Add missing prepare for iis clock of s3c24xx" to the asoc tree Mark Brown
1 sibling, 1 reply; 11+ messages in thread
From: Arvind Yadav @ 2017-07-28 4:41 UTC (permalink / raw)
To: Krzysztof Kozlowski, Sangbeom Kim, Sylwester Nawrocki,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
alsa-devel, linux-kernel
Hi,
On Thursday 27 July 2017 10:43 PM, Krzysztof Kozlowski wrote:
> The s3c_i2sv2_probe() only enabled iis clock. Missing prepare isn't
> probably fatal, because for SoC clocks this is usually no-op, but for
> correctness this clock should be prepared.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> ---
>
> Changes since v1:
> 1. New patch
> ---
> sound/soc/samsung/s3c-i2s-v2.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/soc/samsung/s3c-i2s-v2.c b/sound/soc/samsung/s3c-i2s-v2.c
> index 9b28046eea8e..3894bda06ebb 100644
> --- a/sound/soc/samsung/s3c-i2s-v2.c
> +++ b/sound/soc/samsung/s3c-i2s-v2.c
> @@ -637,7 +637,7 @@ int s3c_i2sv2_probe(struct snd_soc_dai *dai,
> return -ENOENT;
> }
>
> - clk_enable(i2s->iis_pclk);
> + clk_prepare_enable(i2s->iis_pclk);
Please, handle are return value of clk_prepare_enble.
>
> /* Mark ourselves as in TXRX mode so we can run through our cleanup
> * process without warnings. */
~arvind
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFT v2 3/3] ASoC: samsung: Add proper error paths to s3c24xx I2S driver
2017-07-27 17:13 ` [RFT v2 3/3] ASoC: samsung: Add proper error paths to s3c24xx I2S driver Krzysztof Kozlowski
@ 2017-07-28 4:47 ` Arvind Yadav
2017-07-28 10:17 ` Applied "ASoC: samsung: Add proper error paths to s3c24xx I2S driver" to the asoc tree Mark Brown
1 sibling, 0 replies; 11+ messages in thread
From: Arvind Yadav @ 2017-07-28 4:47 UTC (permalink / raw)
To: Krzysztof Kozlowski, Sangbeom Kim, Sylwester Nawrocki,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
alsa-devel, linux-kernel
On Thursday 27 July 2017 10:43 PM, 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>
Acked-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>
> ---
>
> Please, kindly test on S3C24xx hardware.
>
> Changes since v1:
> 1. Drop clk assignment to disable proper clock on remove()
> ---
> sound/soc/samsung/s3c-i2s-v2.c | 9 +++++++++
> sound/soc/samsung/s3c-i2s-v2.h | 7 +++++++
> sound/soc/samsung/s3c2412-i2s.c | 13 +++++++++----
> 3 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/sound/soc/samsung/s3c-i2s-v2.c b/sound/soc/samsung/s3c-i2s-v2.c
> index 3894bda06ebb..58c3e9bfc6b7 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_unprepare(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..cc0840fff5aa 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,20 +74,24 @@ static int s3c2412_i2s_probe(struct snd_soc_dai *dai)
> clk_set_parent(s3c2412_i2s.iis_cclk, clk_get(NULL, "mpll"));
> ret = clk_prepare_enable(s3c2412_i2s.iis_cclk);
> if (ret)
> - return ret;
> -
> - s3c2412_i2s.iis_cclk = s3c2412_i2s.iis_pclk;
> + goto err;
>
> /* Configure the I2S pins (GPE0...GPE4) in correct mode */
> s3c_gpio_cfgall_range(S3C2410_GPE(0), 5, S3C_GPIO_SFN(2),
> 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);
>
> return 0;
> }
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFT v2 2/3] ASoC: samsung: Add missing prepare for iis clock of s3c24xx
2017-07-28 4:41 ` Arvind Yadav
@ 2017-07-28 5:44 ` Krzysztof Kozlowski
2017-07-28 6:05 ` Arvind Yadav
0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2017-07-28 5:44 UTC (permalink / raw)
To: Arvind Yadav
Cc: alsa-devel, Sangbeom Kim, linux-kernel, Takashi Iwai,
Liam Girdwood, Mark Brown, Sylwester Nawrocki
On Fri, Jul 28, 2017 at 10:11:48AM +0530, Arvind Yadav wrote:
> Hi,
>
>
> On Thursday 27 July 2017 10:43 PM, Krzysztof Kozlowski wrote:
> >The s3c_i2sv2_probe() only enabled iis clock. Missing prepare isn't
> >probably fatal, because for SoC clocks this is usually no-op, but for
> >correctness this clock should be prepared.
> >
> >Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> >
> >---
> >
> >Changes since v1:
> >1. New patch
> >---
> > sound/soc/samsung/s3c-i2s-v2.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/sound/soc/samsung/s3c-i2s-v2.c b/sound/soc/samsung/s3c-i2s-v2.c
> >index 9b28046eea8e..3894bda06ebb 100644
> >--- a/sound/soc/samsung/s3c-i2s-v2.c
> >+++ b/sound/soc/samsung/s3c-i2s-v2.c
> >@@ -637,7 +637,7 @@ int s3c_i2sv2_probe(struct snd_soc_dai *dai,
> > return -ENOENT;
> > }
> >- clk_enable(i2s->iis_pclk);
> >+ clk_prepare_enable(i2s->iis_pclk);
> Please, handle are return value of clk_prepare_enble.
Which is a different issue, different bug. We can fix it but it should
be not mixed with this fix here.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFT v2 2/3] ASoC: samsung: Add missing prepare for iis clock of s3c24xx
2017-07-28 5:44 ` Krzysztof Kozlowski
@ 2017-07-28 6:05 ` Arvind Yadav
0 siblings, 0 replies; 11+ messages in thread
From: Arvind Yadav @ 2017-07-28 6:05 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: alsa-devel, Sangbeom Kim, linux-kernel, Takashi Iwai,
Liam Girdwood, Mark Brown, Sylwester Nawrocki
On Friday 28 July 2017 11:14 AM, Krzysztof Kozlowski wrote:
> On Fri, Jul 28, 2017 at 10:11:48AM +0530, Arvind Yadav wrote:
>> Hi,
>>
>>
>> On Thursday 27 July 2017 10:43 PM, Krzysztof Kozlowski wrote:
>>> The s3c_i2sv2_probe() only enabled iis clock. Missing prepare isn't
>>> probably fatal, because for SoC clocks this is usually no-op, but for
>>> correctness this clock should be prepared.
>>>
>>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Acked-by: Arvind Yadav<arvind.yadav.cs@gmail.com
<mailto:arvind.yadav.cs@gmail.com>>
>>>
>>> ---
>>>
>>> Changes since v1:
>>> 1. New patch
>>> ---
>>> sound/soc/samsung/s3c-i2s-v2.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/sound/soc/samsung/s3c-i2s-v2.c b/sound/soc/samsung/s3c-i2s-v2.c
>>> index 9b28046eea8e..3894bda06ebb 100644
>>> --- a/sound/soc/samsung/s3c-i2s-v2.c
>>> +++ b/sound/soc/samsung/s3c-i2s-v2.c
>>> @@ -637,7 +637,7 @@ int s3c_i2sv2_probe(struct snd_soc_dai *dai,
>>> return -ENOENT;
>>> }
>>> - clk_enable(i2s->iis_pclk);
>>> + clk_prepare_enable(i2s->iis_pclk);
>> Please, handle are return value of clk_prepare_enble.
> Which is a different issue, different bug. We can fix it but it should
> be not mixed with this fix here.
>
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Applied "ASoC: samsung: Add proper error paths to s3c24xx I2S driver" to the asoc tree
2017-07-27 17:13 ` [RFT v2 3/3] ASoC: samsung: Add proper error paths to s3c24xx I2S driver Krzysztof Kozlowski
2017-07-28 4:47 ` Arvind Yadav
@ 2017-07-28 10:17 ` Mark Brown
1 sibling, 0 replies; 11+ messages in thread
From: Mark Brown @ 2017-07-28 10:17 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Arvind Yadav, Mark Brown, Sangbeom Kim, Sylwester Nawrocki,
Liam Girdwood
The patch
ASoC: samsung: Add proper error paths to s3c24xx I2S driver
has been applied to the asoc tree at
git://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 6f187f7ef1c15dca9f720b18a7e928718fd1a8b3 Mon Sep 17 00:00:00 2001
From: Krzysztof Kozlowski <krzk@kernel.org>
Date: Thu, 27 Jul 2017 19:13:38 +0200
Subject: [PATCH] ASoC: samsung: Add proper error paths to s3c24xx I2S driver
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>
Acked-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/samsung/s3c-i2s-v2.c | 9 +++++++++
sound/soc/samsung/s3c-i2s-v2.h | 7 +++++++
sound/soc/samsung/s3c2412-i2s.c | 13 +++++++++----
3 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/sound/soc/samsung/s3c-i2s-v2.c b/sound/soc/samsung/s3c-i2s-v2.c
index 3894bda06ebb..58c3e9bfc6b7 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_unprepare(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..cc0840fff5aa 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,20 +74,24 @@ static int s3c2412_i2s_probe(struct snd_soc_dai *dai)
clk_set_parent(s3c2412_i2s.iis_cclk, clk_get(NULL, "mpll"));
ret = clk_prepare_enable(s3c2412_i2s.iis_cclk);
if (ret)
- return ret;
-
- s3c2412_i2s.iis_cclk = s3c2412_i2s.iis_pclk;
+ goto err;
/* Configure the I2S pins (GPE0...GPE4) in correct mode */
s3c_gpio_cfgall_range(S3C2410_GPE(0), 5, S3C_GPIO_SFN(2),
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);
return 0;
}
--
2.13.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Applied "ASoC: samsung: Add missing prepare for iis clock of s3c24xx" to the asoc tree
2017-07-27 17:13 ` [RFT v2 2/3] ASoC: samsung: Add missing prepare for iis clock of s3c24xx Krzysztof Kozlowski
2017-07-28 4:41 ` Arvind Yadav
@ 2017-07-28 10:18 ` Mark Brown
1 sibling, 0 replies; 11+ messages in thread
From: Mark Brown @ 2017-07-28 10:18 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: alsa-devel, Arvind, Sangbeom Kim, linux-kernel, Takashi Iwai,
Liam Girdwood, Mark Brown, Sylwester Nawrocki, Arvind Yadav,
"Yadav <arvind.yadav.cs"
The patch
ASoC: samsung: Add missing prepare for iis clock of s3c24xx
has been applied to the asoc tree at
git://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 81ea6cc73c384e4093519e0c75394aa2a1941a10 Mon Sep 17 00:00:00 2001
From: Krzysztof Kozlowski <krzk@kernel.org>
Date: Thu, 27 Jul 2017 19:13:37 +0200
Subject: [PATCH] ASoC: samsung: Add missing prepare for iis clock of s3c24xx
The s3c_i2sv2_probe() only enabled iis clock. Missing prepare isn't
probably fatal, because for SoC clocks this is usually no-op, but for
correctness this clock should be prepared.
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Acked-by: Arvind Yadav<arvind.yadav.cs@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/samsung/s3c-i2s-v2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/samsung/s3c-i2s-v2.c b/sound/soc/samsung/s3c-i2s-v2.c
index 9b28046eea8e..3894bda06ebb 100644
--- a/sound/soc/samsung/s3c-i2s-v2.c
+++ b/sound/soc/samsung/s3c-i2s-v2.c
@@ -637,7 +637,7 @@ int s3c_i2sv2_probe(struct snd_soc_dai *dai,
return -ENOENT;
}
- clk_enable(i2s->iis_pclk);
+ clk_prepare_enable(i2s->iis_pclk);
/* Mark ourselves as in TXRX mode so we can run through our cleanup
* process without warnings. */
--
2.13.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-07-28 10:18 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-27 17:13 [RFT v2 0/3] ASoC: samsung: s3c24xx: Fixes for error paths Krzysztof Kozlowski
2017-07-27 17:13 ` [RFT v2 1/3] ASoC: samsung: Fix possible double iounmap on s3c24xx driver probe failure Krzysztof Kozlowski
2017-07-28 4:39 ` Arvind Yadav
2017-07-27 17:13 ` [RFT v2 2/3] ASoC: samsung: Add missing prepare for iis clock of s3c24xx Krzysztof Kozlowski
2017-07-28 4:41 ` Arvind Yadav
2017-07-28 5:44 ` Krzysztof Kozlowski
2017-07-28 6:05 ` Arvind Yadav
2017-07-28 10:18 ` Applied "ASoC: samsung: Add missing prepare for iis clock of s3c24xx" to the asoc tree Mark Brown
2017-07-27 17:13 ` [RFT v2 3/3] ASoC: samsung: Add proper error paths to s3c24xx I2S driver Krzysztof Kozlowski
2017-07-28 4:47 ` Arvind Yadav
2017-07-28 10:17 ` Applied "ASoC: samsung: Add proper error paths to s3c24xx I2S driver" to the asoc tree Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).