Re: [PATCH v9 2/8] mmc: sdhci-of-k1: enable essential clock infrastructure for SD operation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Incorrect condition used for targeting removable card scenarios, leading to power regressions for SDIO devices.
- [Medium] Missing explicit clear of SD-specific vendor overrides for non-SD controllers.
--

commit acf2ed5de9b1d692d653b998c6c50d3cddd4cff5
Author: Iker Pedrosa <ikerpedrosam@gmail.com>

mmc: sdhci-of-k1: enable essential clock infrastructure for SD operation

This commit enables clock generation for SD operation on the spacemit
sdhci-of-k1 controller. It sets SDHC_GEN_PAD_CLK_ON for all modes and
conditionally overrides automatic clock gating for SD-only controllers
to handle removable card scenarios.

> diff --git a/drivers/mmc/host/sdhci-of-k1.c b/drivers/mmc/host/sdhci-of-k1.c
> index 455656f9842df..0dd06fc19b857 100644
> --- a/drivers/mmc/host/sdhci-of-k1.c
> +++ b/drivers/mmc/host/sdhci-of-k1.c

[ ... ]

> @@ -101,6 +108,12 @@ static void spacemit_sdhci_reset(struct sdhci_host *host, u8 mask)
>  
>  	if (!(host->mmc->caps2 & MMC_CAP2_NO_MMC))
>  		spacemit_sdhci_setbits(host, SDHC_MMC_CARD_MODE, SPACEMIT_SDHC_MMC_CTRL_REG);
> +
> +	spacemit_sdhci_setbits(host, SDHC_GEN_PAD_CLK_ON, SPACEMIT_SDHC_LEGACY_CTRL_REG);
> +
> +	if (host->mmc->caps2 & MMC_CAP2_NO_MMC)
> +		spacemit_sdhci_setbits(host, SDHC_OVRRD_CLK_OEN | SDHC_FORCE_CLK_ON,
> +				       SPACEMIT_SDHC_OP_EXT_REG);
>  }

The commit message states that the SD-specific clock overrides are intended
to handle removable card scenarios. Does checking for MMC_CAP2_NO_MMC
unintentionally apply these overrides to non-removable SDIO devices?

The MMC_CAP2_NO_MMC flag indicates the absence of eMMC support, which will
evaluate to true for both removable SD cards and non-removable SDIO modules
like Wi-Fi chips.

If the clock is forced to remain continuously on for SDIO devices, could this
override automatic clock gating and cause a power regression by preventing
deep sleep states?

Additionally, does this setup leave the vendor-specific clock overrides active
on eMMC interfaces if they were previously set by a bootloader?

Since SPACEMIT_SDHC_OP_EXT_REG is a vendor-specific register located at offset
0x108, it is typically untouched by the standard SDHCI_RESET_ALL mechanism.

If a prior execution stage leaves SDHC_OVRRD_CLK_OEN or SDHC_FORCE_CLK_ON set,
the absence of an explicit clear in this path could prevent automatic clock
gating on eMMC interfaces.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511-orangepi-sd-card-uhs-v9-0-ae48c0b2b2cf@gmail.com?part=2