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

[Robert Marko]

On Mon, Sep 14, 2026 at 3:46 PM Aubin Constans
<aubin.constans@xxxxxxxxxxxxx> wrote:
>
> 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.

Hi Aubin,
Yeah, I originally used those in our tree as well before trying to
simplify things for upstreaming.

Regards,
Robert

> >
> > 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
>


-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura d.d.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@xxxxxxxxxx
Web: www.sartura.hr