* [PATCH] ASoC: ep93xx: Fix unchecked clk_prepare_enable() and add rollback on failure
@ 2026-03-24 21:09 Jihed Chaibi
2026-03-26 10:02 ` Jihed Chaibi
0 siblings, 1 reply; 3+ messages in thread
From: Jihed Chaibi @ 2026-03-24 21:09 UTC (permalink / raw)
To: linux-sound
Cc: lgirdwood, broonie, perex, tiwai, linux-kernel, jihed.chaibi.dev
ep93xx_i2s_enable() calls clk_prepare_enable() on three clocks in
sequence (mclk, sclk, lrclk) without checking the return value of any
of them. If an intermediate enable fails, the clocks that were already
enabled are never rolled back, leaking them until the next disable cycle
— which may never come if the stream never started cleanly.
Change ep93xx_i2s_enable() from void to int. Add error checking after
each clk_prepare_enable() call and unwind already-enabled clocks on
failure. Propagate the error through ep93xx_i2s_startup() and
ep93xx_i2s_resume(), both of which already return int.
Signed-off-by: Jihed Chaibi <jihed.chaibi.dev@gmail.com>
---
sound/soc/cirrus/ep93xx-i2s.c | 34 ++++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 10 deletions(-)
diff --git a/sound/soc/cirrus/ep93xx-i2s.c b/sound/soc/cirrus/ep93xx-i2s.c
index cca01c03f048..5dba741594fa 100644
--- a/sound/soc/cirrus/ep93xx-i2s.c
+++ b/sound/soc/cirrus/ep93xx-i2s.c
@@ -91,16 +91,28 @@ static inline unsigned ep93xx_i2s_read_reg(struct ep93xx_i2s_info *info,
return __raw_readl(info->regs + reg);
}
-static void ep93xx_i2s_enable(struct ep93xx_i2s_info *info, int stream)
+static int ep93xx_i2s_enable(struct ep93xx_i2s_info *info, int stream)
{
unsigned base_reg;
+ int err;
if ((ep93xx_i2s_read_reg(info, EP93XX_I2S_TX0EN) & 0x1) == 0 &&
(ep93xx_i2s_read_reg(info, EP93XX_I2S_RX0EN) & 0x1) == 0) {
/* Enable clocks */
- clk_prepare_enable(info->mclk);
- clk_prepare_enable(info->sclk);
- clk_prepare_enable(info->lrclk);
+ err = clk_prepare_enable(info->mclk);
+ if (err)
+ return err;
+ err = clk_prepare_enable(info->sclk);
+ if (err) {
+ clk_disable_unprepare(info->mclk);
+ return err;
+ }
+ err = clk_prepare_enable(info->lrclk);
+ if (err) {
+ clk_disable_unprepare(info->sclk);
+ clk_disable_unprepare(info->mclk);
+ return err;
+ }
/* Enable i2s */
ep93xx_i2s_write_reg(info, EP93XX_I2S_GLCTRL, 1);
@@ -119,6 +131,8 @@ static void ep93xx_i2s_enable(struct ep93xx_i2s_info *info, int stream)
ep93xx_i2s_write_reg(info, EP93XX_I2S_TXCTRL,
EP93XX_I2S_TXCTRL_TXEMPTY_LVL |
EP93XX_I2S_TXCTRL_TXUFIE);
+
+ return 0;
}
static void ep93xx_i2s_disable(struct ep93xx_i2s_info *info, int stream)
@@ -195,9 +209,7 @@ static int ep93xx_i2s_startup(struct snd_pcm_substream *substream,
{
struct ep93xx_i2s_info *info = snd_soc_dai_get_drvdata(dai);
- ep93xx_i2s_enable(info, substream->stream);
-
- return 0;
+ return ep93xx_i2s_enable(info, substream->stream);
}
static void ep93xx_i2s_shutdown(struct snd_pcm_substream *substream,
@@ -373,14 +385,16 @@ static int ep93xx_i2s_suspend(struct snd_soc_component *component)
static int ep93xx_i2s_resume(struct snd_soc_component *component)
{
struct ep93xx_i2s_info *info = snd_soc_component_get_drvdata(component);
+ int err;
if (!snd_soc_component_active(component))
return 0;
- ep93xx_i2s_enable(info, SNDRV_PCM_STREAM_PLAYBACK);
- ep93xx_i2s_enable(info, SNDRV_PCM_STREAM_CAPTURE);
+ err = ep93xx_i2s_enable(info, SNDRV_PCM_STREAM_PLAYBACK);
+ if (err)
+ return err;
- return 0;
+ return ep93xx_i2s_enable(info, SNDRV_PCM_STREAM_CAPTURE);
}
#else
#define ep93xx_i2s_suspend NULL
--
2.47.3
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] ASoC: ep93xx: Fix unchecked clk_prepare_enable() and add rollback on failure
2026-03-24 21:09 [PATCH] ASoC: ep93xx: Fix unchecked clk_prepare_enable() and add rollback on failure Jihed Chaibi
@ 2026-03-26 10:02 ` Jihed Chaibi
2026-03-26 10:37 ` Mark Brown
0 siblings, 1 reply; 3+ messages in thread
From: Jihed Chaibi @ 2026-03-26 10:02 UTC (permalink / raw)
To: linux-sound; +Cc: lgirdwood, broonie, perex, tiwai, linux-kernel
On Tue, Mar 24, 2026 at 10:09 PM Jihed Chaibi
<jihed.chaibi.dev@gmail.com> wrote:
>
> ep93xx_i2s_enable() calls clk_prepare_enable() on three clocks in
> sequence (mclk, sclk, lrclk) without checking the return value of any
> of them. If an intermediate enable fails, the clocks that were already
> enabled are never rolled back, leaking them until the next disable cycle
> — which may never come if the stream never started cleanly.
>
> Change ep93xx_i2s_enable() from void to int. Add error checking after
> each clk_prepare_enable() call and unwind already-enabled clocks on
> failure. Propagate the error through ep93xx_i2s_startup() and
> ep93xx_i2s_resume(), both of which already return int.
>
> Signed-off-by: Jihed Chaibi <jihed.chaibi.dev@gmail.com>
> ---
> sound/soc/cirrus/ep93xx-i2s.c | 34 ++++++++++++++++++++++++----------
> 1 file changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/sound/soc/cirrus/ep93xx-i2s.c b/sound/soc/cirrus/ep93xx-i2s.c
> index cca01c03f048..5dba741594fa 100644
> --- a/sound/soc/cirrus/ep93xx-i2s.c
> +++ b/sound/soc/cirrus/ep93xx-i2s.c
> @@ -91,16 +91,28 @@ static inline unsigned ep93xx_i2s_read_reg(struct ep93xx_i2s_info *info,
> return __raw_readl(info->regs + reg);
> }
>
> -static void ep93xx_i2s_enable(struct ep93xx_i2s_info *info, int stream)
> +static int ep93xx_i2s_enable(struct ep93xx_i2s_info *info, int stream)
> {
> unsigned base_reg;
> + int err;
>
> if ((ep93xx_i2s_read_reg(info, EP93XX_I2S_TX0EN) & 0x1) == 0 &&
> (ep93xx_i2s_read_reg(info, EP93XX_I2S_RX0EN) & 0x1) == 0) {
> /* Enable clocks */
> - clk_prepare_enable(info->mclk);
> - clk_prepare_enable(info->sclk);
> - clk_prepare_enable(info->lrclk);
> + err = clk_prepare_enable(info->mclk);
> + if (err)
> + return err;
> + err = clk_prepare_enable(info->sclk);
> + if (err) {
> + clk_disable_unprepare(info->mclk);
> + return err;
> + }
> + err = clk_prepare_enable(info->lrclk);
> + if (err) {
> + clk_disable_unprepare(info->sclk);
> + clk_disable_unprepare(info->mclk);
> + return err;
> + }
>
> /* Enable i2s */
> ep93xx_i2s_write_reg(info, EP93XX_I2S_GLCTRL, 1);
> @@ -119,6 +131,8 @@ static void ep93xx_i2s_enable(struct ep93xx_i2s_info *info, int stream)
> ep93xx_i2s_write_reg(info, EP93XX_I2S_TXCTRL,
> EP93XX_I2S_TXCTRL_TXEMPTY_LVL |
> EP93XX_I2S_TXCTRL_TXUFIE);
> +
> + return 0;
> }
>
> static void ep93xx_i2s_disable(struct ep93xx_i2s_info *info, int stream)
> @@ -195,9 +209,7 @@ static int ep93xx_i2s_startup(struct snd_pcm_substream *substream,
> {
> struct ep93xx_i2s_info *info = snd_soc_dai_get_drvdata(dai);
>
> - ep93xx_i2s_enable(info, substream->stream);
> -
> - return 0;
> + return ep93xx_i2s_enable(info, substream->stream);
> }
>
> static void ep93xx_i2s_shutdown(struct snd_pcm_substream *substream,
> @@ -373,14 +385,16 @@ static int ep93xx_i2s_suspend(struct snd_soc_component *component)
> static int ep93xx_i2s_resume(struct snd_soc_component *component)
> {
> struct ep93xx_i2s_info *info = snd_soc_component_get_drvdata(component);
> + int err;
>
> if (!snd_soc_component_active(component))
> return 0;
>
> - ep93xx_i2s_enable(info, SNDRV_PCM_STREAM_PLAYBACK);
> - ep93xx_i2s_enable(info, SNDRV_PCM_STREAM_CAPTURE);
> + err = ep93xx_i2s_enable(info, SNDRV_PCM_STREAM_PLAYBACK);
> + if (err)
> + return err;
>
> - return 0;
> + return ep93xx_i2s_enable(info, SNDRV_PCM_STREAM_CAPTURE);
> }
> #else
> #define ep93xx_i2s_suspend NULL
> --
> 2.47.3
>
Adding the missing Fixes tag:
Fixes: f4ff6b56bc8a ("ASoC: cirrus: i2s: Prepare clock before using it")
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] ASoC: ep93xx: Fix unchecked clk_prepare_enable() and add rollback on failure
2026-03-26 10:02 ` Jihed Chaibi
@ 2026-03-26 10:37 ` Mark Brown
0 siblings, 0 replies; 3+ messages in thread
From: Mark Brown @ 2026-03-26 10:37 UTC (permalink / raw)
To: Jihed Chaibi; +Cc: linux-sound, lgirdwood, perex, tiwai, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 529 bytes --]
On Thu, Mar 26, 2026 at 11:02:35AM +0100, Jihed Chaibi wrote:
> On Tue, Mar 24, 2026 at 10:09 PM Jihed Chaibi
> <jihed.chaibi.dev@gmail.com> wrote:
> >
> > ep93xx_i2s_enable() calls clk_prepare_enable() on three clocks in
> > sequence (mclk, sclk, lrclk) without checking the return value of any
Please delete unneeded context from mails when replying. Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-03-26 10:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-24 21:09 [PATCH] ASoC: ep93xx: Fix unchecked clk_prepare_enable() and add rollback on failure Jihed Chaibi
2026-03-26 10:02 ` Jihed Chaibi
2026-03-26 10:37 ` Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox