Re: [PATCH v2 2/6] mmc: sdhci-of-at91: add option to keep clocks enabled

[Aubin Constans]

Hi Robert,

On 07/09/2026 15:25, Robert Marko wrote:
> sdhci_at91_set_clks_presets() both enables the controller clocks and
> programs its capabilities and preset registers. This prevents callers from
> restoring the registers without changing the clock enable counts.
>
> Move clock enablement to callers and add a SoC data flag for controllers
> that must keep their clocks enabled. Use it in the runtime PM paths while
> keeping register restoration separate from clock enablement.

This change has been required for the SAMA7G54 MPU already, and implemented by
two complementary commits:

https://github.com/linux4microchip/linux/commit/763a3c05c8f9d0feb279c6f308fa5d48d14e07d3
https://github.com/linux4microchip/linux/commit/b5bbc888fb39471d46fe0a4ff28f69533c265dd0

At least they have been validated by test campaigns.
If that existing work seemed OK for you and the LAN969x, then I could squash
these two commits and submit the result.
> Signed-off-by: Robert Marko <robert.marko@xxxxxxxxxx>
>
>   drivers/mmc/host/sdhci-of-at91.c | 39 +++++++++++++++++++-------------
>   1 file changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
> index 7c4ac65f247d..b7e2a89da348 100644
> --- a/drivers/mmc/host/sdhci-of-at91.c
> +++ b/drivers/mmc/host/sdhci-of-at91.c
> @@ -38,6 +38,7 @@
>   struct sdhci_at91_soc_data {
>          const struct sdhci_pltfm_data *pdata;
>          bool baseclk_is_generated_internally;
> +       bool keep_clks_on;
>          unsigned int divider_for_baseclk;
>   };
>
> @@ -164,7 +165,7 @@ static const struct of_device_id sdhci_at91_dt_match[] = {
>   };
>   MODULE_DEVICE_TABLE(of, sdhci_at91_dt_match);
>
> -static int sdhci_at91_set_clks_presets(struct device *dev)
> +static void sdhci_at91_set_clks_presets(struct device *dev)
>   {
>          struct sdhci_host *host = dev_get_drvdata(dev);
>          struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -174,7 +175,6 @@ static int sdhci_at91_set_clks_presets(struct device *dev)
>          unsigned int                    gck_rate, clk_base_rate;
>          unsigned int                    preset_div;
>
> -       clk_prepare_enable(priv->hclock);
>          caps0 = readl(host->ioaddr + SDHCI_CAPABILITIES);
>          caps1 = readl(host->ioaddr + SDHCI_CAPABILITIES_1);
>
> @@ -222,11 +222,6 @@ static int sdhci_at91_set_clks_presets(struct device *dev)
>          preset_div = DIV_ROUND_UP(gck_rate, 50000000) - 1;
>          writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div,
>                 host->ioaddr + SDHCI_PRESET_FOR_DDR50);
> -
> -       clk_prepare_enable(priv->mainck);
> -       clk_prepare_enable(priv->gck);
> -
> -       return 0;
>   }
>
>   static int sdhci_at91_suspend(struct device *dev)
> @@ -254,9 +249,11 @@ static int sdhci_at91_runtime_suspend(struct device *dev)
>          if (host->tuning_mode != SDHCI_TUNING_MODE_3)
>                  mmc_retune_needed(host->mmc);
>
> -       clk_disable_unprepare(priv->gck);
> -       clk_disable_unprepare(priv->hclock);
> -       clk_disable_unprepare(priv->mainck);
> +       if (!priv->soc_data->keep_clks_on) {
> +               clk_disable_unprepare(priv->gck);
> +               clk_disable_unprepare(priv->hclock);
> +               clk_disable_unprepare(priv->mainck);
> +       }
>
>          return 0;
>   }
> @@ -269,14 +266,23 @@ static int sdhci_at91_runtime_resume(struct device *dev)
>          int ret;
>
>          if (priv->restore_needed) {
> -               ret = sdhci_at91_set_clks_presets(dev);
> -               if (ret)
> -                       return ret;
> +               if (!priv->soc_data->keep_clks_on)
> +                       clk_prepare_enable(priv->hclock);
> +
> +               sdhci_at91_set_clks_presets(dev);
> +
> +               if (!priv->soc_data->keep_clks_on) {
> +                       clk_prepare_enable(priv->mainck);
> +                       clk_prepare_enable(priv->gck);
> +               }
>
>                  priv->restore_needed = false;
>                  goto out;
>          }
>
> +       if (priv->soc_data->keep_clks_on)
> +               goto out;
> +
>          ret = clk_prepare_enable(priv->mainck);
>          if (ret) {
>                  dev_err(dev, "can't enable mainck\n");
> @@ -344,9 +350,10 @@ static int sdhci_at91_probe(struct platform_device *pdev)
>                  return dev_err_probe(&pdev->dev, PTR_ERR(priv->gck),
>                                       "failed to get multclk\n");
>
> -       ret = sdhci_at91_set_clks_presets(&pdev->dev);
> -       if (ret)
> -               return ret;
> +       clk_prepare_enable(priv->hclock);
> +       sdhci_at91_set_clks_presets(&pdev->dev);
> +       clk_prepare_enable(priv->mainck);
> +       clk_prepare_enable(priv->gck);
>
>          priv->restore_needed = false;
>
> --
> 2.55.0