Re: [PATCH v4 2/2] mmc: core: Honor keep-power-in-suspend for (e)MMC in suspend/resume

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `MMC_PM_KEEP_POWER` flag is permanently leaked in `host->pm_flags` if the fast suspend path fails during `mmc_deselect_cards()`.
--

--- Patch [2]: [PATCH v4 2/2] mmc: core: Honor keep-power-in-suspend for (e)MMC in suspend/resume ---
commit 575375a652461af659583ed563eb72a97ab24900
Author: Kamal Dasu <kamal.dasu@broadcom.com>

mmc: core: Honor keep-power-in-suspend for (e)MMC in suspend/resume

This commit updates the core MMC code to honor the keep-power-in-suspend
flag for (e)MMC devices. When this capability is present during suspend,
the power-off sequence is skipped and the host is reset to its initial
state instead. The MMC_PM_KEEP_POWER flag is set to inform host controllers
of the preserved power state.

Closes: https://lore.kernel.org/r/20260413180551.3683969-1-florian.fainelli@broadcom.com/

> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c

[ ... ]

> @@ -2157,6 +2157,35 @@ static int _mmc_suspend(struct mmc_host *host, enum mmc_poweroff_type pm_type)
>  			goto out;
>  	}
>  
> +	/*
> +	 * Keep the card powered across an actual suspend; shutdown, unbind
> +	 * and undervoltage still need the normal power-off path below,
> +	 * since they aren't guaranteed a subsequent _mmc_resume().
> +	 *
> +	 * Check pm_caps, not pm_flags: unlike SDIO, (e)MMC has no
> +	 * per-function driver to request this via
> +	 * sdio_set_host_pm_flags(), so it's a fixed platform trait here.
> +	 *
> +	 * Set MMC_PM_KEEP_POWER in pm_flags so host controller drivers
> +	 * know card power was preserved during suspend and perform a
> +	 * soft resume sequence.
> +	 *
> +	 * Reset the host to its initial bus state like _mmc_hw_reset()
> +	 * does.
> +	 */
> +	if (pm_type == MMC_POWEROFF_SUSPEND &&
> +	    (host->pm_caps & MMC_PM_KEEP_POWER)) {
> +		host->pm_flags |= MMC_PM_KEEP_POWER;
> +		if (!mmc_host_is_spi(host))
> +			err = mmc_deselect_cards(host);
> +		if (!err) {
> +			mmc_set_clock(host, host->f_init);
> +			mmc_set_initial_state(host);
> +			mmc_card_set_suspended(host->card);
> +		}
> +		goto out;
> +	}
> +

[Severity: High]
Does this leak the MMC_PM_KEEP_POWER flag if mmc_deselect_cards() fails?

If mmc_deselect_cards() returns an error, the function jumps to out without
marking the card as suspended.

[ ... ]

> @@ -2220,6 +2249,7 @@ static int _mmc_resume(struct mmc_host *host)
>  	mmc_power_up(host, host->card->ocr);
>  	err = mmc_init_card(host, host->card->ocr, host->card);
>  	mmc_card_clr_suspended(host->card);
> +	host->pm_flags &= ~MMC_PM_KEEP_POWER;
>  
>  out:
>  	mmc_release_host(host);

If the system then attempts to recover from the aborted suspend by calling
_mmc_resume(), it checks if the card is suspended and exits early:

_mmc_resume() {
	...
	if (!mmc_card_suspended(host->card))
		goto out;
	...
}

Since the card was never marked suspended during the error path, it will
take this goto out path and bypass the host->pm_flags cleanup, leaving
MMC_PM_KEEP_POWER set permanently for the remainder of the uptime.

Would it be better to clear the flag in the error path of _mmc_suspend(), or
perhaps wait to set the flag until after mmc_deselect_cards() succeeds?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728190314.2325906-1-kamal.dasu@broadcom.com?part=2