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

[Aubin Constans <aubin.constans@microchip.com>]

On 15/09/2026 13:31, Robert Marko wrote:
> On Mon, Sep 14, 2026 at 3:46 PM Aubin Constans
> <aubin.constans@microchip.com> 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
> 

To follow up on that patch, I have just submitted the original work,
ported to tag next-20260917:

https://lore.kernel.org/linux-mmc/20260918153414.909333-1-aubin.constans@microchip.com/

Hope it will make easier to support the LAN969x, notably.

Regards,
Aubin

>>>
>>> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
>>>
>>>    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@sartura.hr
> Web: www.sartura.hr