Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Kamal Dasu" <kamal.dasu@broadcom.com>
Cc: devicetree@vger.kernel.org, robh@kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH v3 2/2] mmc: core: Add MMC_CAP2_NO_POWEROFF_SUSPEND host capability
Date: Wed, 22 Jul 2026 21:24:57 +0000	[thread overview]
Message-ID: <20260722212458.2914A1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260722211417.3702709-3-kamal.dasu@broadcom.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The MMC_CAP2_NO_POWEROFF_SUSPEND bypass unconditionally applies to all power-off types in `_mmc_suspend()`, preventing power-off notifications and emergency actions during system shutdown, driver unbind, and undervoltage protection.
- [High] Skipping `mmc_power_off` during suspend leaves `host->ios.power_mode` as `MMC_POWER_ON`, causing `mmc_power_up` to return early on resume, which breaks card re-initialization by failing to reset the clock and bus state.
--

--- Patch [2]: [PATCH v3 2/2] mmc: core: Add MMC_CAP2_NO_POWEROFF_SUSPEND host capability ---
commit d167c50cff315ae6b0704bc19e48556178313b8a
Author: Kamal Dasu <kamal.dasu@broadcom.com>

mmc: core: Add MMC_CAP2_NO_POWEROFF_SUSPEND host capability

On some platforms, firmware accesses the (e)MMC card directly during resume
from Suspend-to-DRAM. This requires the card to remain powered and responsive
throughout suspend. This patch adds MMC_CAP2_NO_POWEROFF_SUSPEND to skip the
poweroff-notify/sleep/power-off sequence during _mmc_suspend().

Link: 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
> index 05444ecf3909f..27a559e6fcc0b 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -2157,6 +2157,21 @@ static int _mmc_suspend(struct mmc_host *host, enum mmc_poweroff_type pm_type)
>  			goto out;
>  	}
>  
> +	/*
> +	 * Some platforms need the card to stay powered and responsive
> +	 * throughout suspend, since firmware accesses it directly during
> +	 * resume, before the kernel's own resume path has run. Just
> +	 * deselect it and leave power alone; mmc_init_card() will fully
> +	 * re-initialize the card once the kernel resumes.
> +	 */
> +	if (host->caps2 & MMC_CAP2_NO_POWEROFF_SUSPEND) {

[Severity: High]
Does this unconditionally bypass the power-off and notification sequence for
all pm_type values, including MMC_POWEROFF_SHUTDOWN and 
MMC_POWEROFF_UNDERVOLTAGE?

Since this doesn't check if pm_type == MMC_POWEROFF_SUSPEND, could this
prevent required power-off notifications and emergency actions during system
shutdown, driver unbind, or undervoltage events?

[Severity: High]
If we skip mmc_power_off() here, won't host->ios.power_mode remain
MMC_POWER_ON?

During resume, _mmc_resume() calls mmc_power_up(), which checks the
power_mode:

drivers/mmc/core/core.c:mmc_power_up() {
    if (host->ios.power_mode == MMC_POWER_ON)
        return;
    ...
}

Will mmc_power_up() return early, skipping the clock and bus reset in
mmc_set_initial_state()?

If so, could this cause mmc_init_card() to subsequently execute using an
invalid high-speed host bus state instead of the required reset state?

> +		if (!mmc_host_is_spi(host))
> +			err = mmc_deselect_cards(host);
> +		if (!err)
> +			mmc_card_set_suspended(host->card);
> +		goto out;
> +	}
> +
>  	if (mmc_card_can_poweroff_notify(host->card) &&
>  	    mmc_host_can_poweroff_notify(host, pm_type))
>  		err = mmc_poweroff_notify(host->card, notify_type);

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

      reply	other threads:[~2026-07-22 21:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-22 21:14 [PATCH v3 0/2] mmc: core: Keep the card powered across suspend when firmware needs it live Kamal Dasu
2026-07-22 21:14 ` [PATCH v3 1/2] dt-bindings: mmc: Document no-mmc-poweroff-suspend property Kamal Dasu
2026-07-22 21:14 ` [PATCH v3 2/2] mmc: core: Add MMC_CAP2_NO_POWEROFF_SUSPEND host capability Kamal Dasu
2026-07-22 21:24   ` sashiko-bot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260722212458.2914A1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=kamal.dasu@broadcom.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox