Re: [PATCH] ASoC: ep93xx: Fix unchecked clk_prepare_enable() and add rollback on failure

[Jihed Chaibi]

On Tue, Mar 24, 2026 at 10:09 PM Jihed Chaibi
<jihed.chaibi.dev@xxxxxxxxx> 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@xxxxxxxxx>
> ---
>  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")