Re: [PATCH v3] mmc: sdhci-of-dwcmshc: check bus clock enable result in the probe() method

On Mon, Jun 1, 2026 at 4:49 PM Sergey Shtylyov <[email protected]> wrote:
>
> In the driver's probe() method, clk_disable_unprepare() for the bus clock
> is called on the error path even if the prior clk_prepare_enable() call has
> failed (and the same thing happens in the remove() method as well) -- that
> would cause the prepare/enable counter imbalance.  Also, the same problem
> can happen in the driver's suspend() method; note that the resume() method
> does check the clk_prepare_enable()'s result -- let's be consistent and do
> that in probe() method as well.  BTW, I don't know for sure what does the
> bus clock control -- if it affects the register accesses, the driver will
> likely cause (e.g. on ARM) a kernel oops if it fails to prepare/enable the
> bus clock in the probe() method...
>
> Found by Linux Verification Center (linuxtesting.org) with the Svace static
> analysis tool.
>
> Fixes: e438cf49b305 ("mmc: sdhci-of-dwcmshc: add SDHCI OF Synopsys DWC MSHC driver")
> Fixes: bccce2ec7790 ("mmc: sdhci-of-dwcmshc: add suspend/resume support")
> Signed-off-by: Sergey Shtylyov <[email protected]>

Applied for fixes and by adding a stable tag, thanks!

Kind regards
Uffe


>
> ---
> This patch is against the fixes branch of Ulf Hansson's mmc.git repo.
>
> Changes in version 3:
> - dropped stray comma in the subject;
> - dropped unneeded word in the description.
>
> Changes in version 2:
> - mentioned the problem in the remove() method as well, somewhat rephrased
>   the patch description.
>
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index b9ecd91f44ad..29af07561a86 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -2441,13 +2441,16 @@ static int dwcmshc_probe(struct platform_device *pdev)
>                         return err;
>
>                 priv->bus_clk = devm_clk_get(dev, "bus");
> -               if (!IS_ERR(priv->bus_clk))
> -                       clk_prepare_enable(priv->bus_clk);
> +               if (!IS_ERR(priv->bus_clk)) {
> +                       err = clk_prepare_enable(priv->bus_clk);
> +                       if (err)
> +                               goto err_clk;
> +               }
>         }
>
>         err = mmc_of_parse(host->mmc);
>         if (err)
> -               goto err_clk;
> +               goto err_bus_clk;
>
>         sdhci_get_of_property(pdev);
>
> @@ -2461,7 +2464,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
>         if (pltfm_data->init) {
>                 err = pltfm_data->init(&pdev->dev, host, priv);
>                 if (err)
> -                       goto err_clk;
> +                       goto err_bus_clk;
>         }
>
>  #ifdef CONFIG_ACPI
> @@ -2507,9 +2510,10 @@ static int dwcmshc_probe(struct platform_device *pdev)
>  err_rpm:
>         pm_runtime_disable(dev);
>         pm_runtime_put_noidle(dev);
> +err_bus_clk:
> +       clk_disable_unprepare(priv->bus_clk);
>  err_clk:
>         clk_disable_unprepare(pltfm_host->clk);
> -       clk_disable_unprepare(priv->bus_clk);
>         clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
>         return err;
>  }
> --
> 2.54.0