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