Re: [PATCH v4] mmc: sdhci-pxav3: use managed clock enablement

[Ulf Hansson]

On Mon, Oct 19, 2026 at 12:55 AM Rosen Penev <rosenp@xxxxxxxxx> wrote:
>
> Replace devm_clk_get() plus clk_prepare_enable() with the managed
> variants devm_clk_get_enabled() and devm_clk_get_optional_enabled(),
> using the same io clock lookup as sdhci-pxav2: the "io" clock if
> present, otherwise the unnamed clock. Clock lookup errors, including
> -EPROBE_DEFER, are now propagated instead of being ignored.
>
> The core clock becomes optional, which removes the IS_ERR() guards in
> runtime suspend/resume, as clk_prepare_enable() and friends are NULL
> safe. devm now unwinds the clocks, so the manual clk_disable_unprepare()
> calls in the probe error paths and remove() are dropped.
>
> Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>

Applied for next, thanks!

Kind regards
Uffe


> ---
>  v4: v1 + sdhci-pxav2-style io clock lookup
>  v3: fix sashiko errors.
>  v2: fix resume paths.
>  drivers/mmc/host/sdhci-pxav3.c | 38 +++++++++++-----------------------
>  1 file changed, 12 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
> index f39764ffd4a0..0bbf782fc235 100644
> --- a/drivers/mmc/host/sdhci-pxav3.c
> +++ b/drivers/mmc/host/sdhci-pxav3.c
> @@ -413,19 +413,16 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>         pltfm_host = sdhci_priv(host);
>         pxa = sdhci_pltfm_priv(pltfm_host);
>
> -       pxa->clk_io = devm_clk_get(dev, "io");
> +       pxa->clk_io = devm_clk_get_optional_enabled(dev, "io");
> +       if (!pxa->clk_io)
> +               pxa->clk_io = devm_clk_get_enabled(dev, NULL);
>         if (IS_ERR(pxa->clk_io))
> -               pxa->clk_io = devm_clk_get(dev, NULL);
> -       if (IS_ERR(pxa->clk_io)) {
> -               dev_err(dev, "failed to get io clock\n");
> -               return PTR_ERR(pxa->clk_io);
> -       }
> +               return dev_err_probe(dev, PTR_ERR(pxa->clk_io), "failed to get io clock\n");
>         pltfm_host->clk = pxa->clk_io;
> -       clk_prepare_enable(pxa->clk_io);
>
> -       pxa->clk_core = devm_clk_get(dev, "core");
> -       if (!IS_ERR(pxa->clk_core))
> -               clk_prepare_enable(pxa->clk_core);
> +       pxa->clk_core = devm_clk_get_optional_enabled(dev, "core");
> +       if (IS_ERR(pxa->clk_core))
> +               return PTR_ERR(pxa->clk_core);
>
>         host->mmc->caps |= MMC_CAP_NEED_RSP_BUSY;
>         /* enable 1/8V DDR capable */
> @@ -434,17 +431,17 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>         if (device_is_compatible(dev, "marvell,armada-380-sdhci")) {
>                 ret = armada_38x_quirks(pdev, host);
>                 if (ret < 0)
> -                       goto err_mbus_win;
> +                       return ret;
>                 ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
>                 if (ret < 0)
> -                       goto err_mbus_win;
> +                       return ret;
>         }
>
>         match = of_match_device(of_match_ptr(sdhci_pxav3_of_match), &pdev->dev);
>         if (match) {
>                 ret = mmc_of_parse(host->mmc);
>                 if (ret)
> -                       goto err_of_parse;
> +                       return ret;
>                 sdhci_get_of_property(pdev);
>                 pdata = pxav3_get_mmc_pdata(dev);
>                 pdev->dev.platform_data = pdata;
> @@ -499,27 +496,18 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>  err_add_host:
>         pm_runtime_disable(&pdev->dev);
>         pm_runtime_put_noidle(&pdev->dev);
> -err_of_parse:
> -err_mbus_win:
> -       clk_disable_unprepare(pxa->clk_io);
> -       clk_disable_unprepare(pxa->clk_core);
>         return ret;
>  }
>
>  static void sdhci_pxav3_remove(struct platform_device *pdev)
>  {
>         struct sdhci_host *host = platform_get_drvdata(pdev);
> -       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> -       struct sdhci_pxa *pxa = sdhci_pltfm_priv(pltfm_host);
>
>         pm_runtime_get_sync(&pdev->dev);
>         pm_runtime_disable(&pdev->dev);
>         pm_runtime_put_noidle(&pdev->dev);
>
>         sdhci_remove_host(host, 1);
> -
> -       clk_disable_unprepare(pxa->clk_io);
> -       clk_disable_unprepare(pxa->clk_core);
>  }
>
>  static int sdhci_pxav3_suspend(struct device *dev)
> @@ -560,8 +548,7 @@ static int sdhci_pxav3_runtime_suspend(struct device *dev)
>                 mmc_retune_needed(host->mmc);
>
>         clk_disable_unprepare(pxa->clk_io);
> -       if (!IS_ERR(pxa->clk_core))
> -               clk_disable_unprepare(pxa->clk_core);
> +       clk_disable_unprepare(pxa->clk_core);
>
>         return 0;
>  }
> @@ -573,8 +560,7 @@ static int sdhci_pxav3_runtime_resume(struct device *dev)
>         struct sdhci_pxa *pxa = sdhci_pltfm_priv(pltfm_host);
>
>         clk_prepare_enable(pxa->clk_io);
> -       if (!IS_ERR(pxa->clk_core))
> -               clk_prepare_enable(pxa->clk_core);
> +       clk_prepare_enable(pxa->clk_core);
>
>         sdhci_runtime_resume_host(host, 0);
>         return 0;
> --
> 2.56.0
>