* [PATCH v9 0/3] mmc: core: Keep the card powered across suspend when firmware needs it live
@ 2026-08-11 16:38 Kamal Dasu
2026-08-11 16:38 ` [PATCH v9 1/3] dt-bindings: mmc: Extend keep-power-in-suspend beyond SDIO Kamal Dasu
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Kamal Dasu @ 2026-08-11 16:38 UTC (permalink / raw)
To: Ulf Hansson
Cc: Kamal Dasu, Florian Fainelli, Wolfram Sang, Oleksij Rempel,
Avri Altman, Pedro Demarchi Gomes, Erick Shepherd, Adrian Hunter,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-mmc,
devicetree, linux-kernel
This is v9.
Background: on brcmstb boards with a Kioxia 016G01 eMMC, firmware
accesses the card directly during resume from Suspend-to-DRAM, before
the kernel's own resume path runs, in order to load boot code using
hard wired logic that is not field updatable. The card needs to stay
powered and responsive for that access to succeed, and since it is
never power-cycled, it also needs to be reset before the kernel
reuses it after resume.
Changes in v9:
- Sashiko's AI review found two more High severity issues in the
same shutdown/undervoltage fallback added in v8:
* Calling mmc_select_card() straight after finding the card
still marked suspended assumed the card would respond to
CMD7 as-is, but that's exactly the assumption
reset-card-at-resume exists to distrust -- firmware may have
left the card unable to decode commands at whatever
clock/timing was negotiated before suspend. A CMD7 timeout
there hit "goto out" and skipped the power-off sequence
entirely, the same failure mode this fallback was added to
close in v8. Fixed by resetting the host to its initial bus
state before mmc_select_card(), the same way _mmc_resume()
already does before touching the card.
* Even with that reset, mmc_select_card() can still legitimately
fail (card genuinely gone). The code still unconditionally
"goto out" in that case, skipping mmc_power_off() entirely --
but mmc_power_off() is host/regulator-side only and doesn't
need a responsive card, so there's no reason a select failure
should leave the supply on indefinitely for a shutdown or
undervoltage event, or leave pm_flags/suspended state stuck
retrying the same failing select on every later call. Fixed
by falling through to mmc_power_off() directly on select
failure instead of aborting.
- Patches 1/3 and 2/3 are unchanged from v8.
Changes in v8:
- Sashiko's AI review of v7 found a High severity issue on patch
3/3: the keep-power fast path marks the card suspended without
powering it off, but the pre-existing early exit at the top of
_mmc_suspend() for an already-suspended card doesn't account for
that -- a shutdown, unbind or undervoltage event landing before
the card's next real access (which is what lazily triggers
_mmc_resume() via runtime PM) would hit that early exit and
silently skip mmc_poweroff_notify()/mmc_power_off() entirely.
Fixed by reselecting the card and continuing into the normal
power-off sequence in that case, instead of a bare early exit.
- Sashiko also flagged a Low severity gap on patch 2/3: nothing in
the schema enforced reset-card-at-resume's own stated pairing
with keep-power-in-suspend, so a DT could set it alone and still
pass dt_binding_check. Rob Herring asked for this to be addressed
on the list. Added a dependencies entry for it.
- Patch 1/3 is unchanged from v7.
Changes in v7:
- Sashiko's AI review of v6 found two real, complementary bugs from
treating keep-power-in-suspend and reset-card-at-resume as fully
independent in the driver: keep-power-in-suspend without
reset-card-at-resume left mmc_power_up() no-oping on resume,
which hangs on real hardware (confirmed); reset-card-at-resume
without keep-power-in-suspend drove the clock and bus lines ahead
of mmc_power_up() while the card's supply was still off from a
normal power-off. Patch 3/3 now requires both capabilities
together for the suspend fast path, and gates the resume-side
reset on pm_flags (only ever set when the fast path actually ran)
rather than the raw capability. Verified on hardware: the
previously-hanging combination now falls through safely to a
normal power-off/power-on cycle, and the paired-capability case
(brcmstb's actual configuration) is unaffected.
- Patches 1/3 and 2/3 gained Krzysztof's Reviewed-by; their content
is otherwise unchanged from v6.
Changes in v6:
- Split keeping the card powered and needing a reset before reuse
into two independent DT properties, per Ulf: extending
keep-power-in-suspend beyond SDIO (patch 1) no longer carries any
brcmstb-specific rationale, and a new reset-card-at-resume
property (patch 2) covers that instead. brcmstb sets both; SDIO's
existing keep-power-in-suspend users are unaffected.
- Patch 3 (the driver patch) reflects the split: the
mmc_set_clock()/mmc_set_initial_state() reset moved out of the
suspend-side fast path and into _mmc_resume(), gated on the new
MMC_CAP2_RESET_AT_RESUME, matching reset-card-at-resume's name
and description.
- Also per Ulf (raised on v4, applies equally to v5): dropped the
mention of sdio_set_host_pm_flags() and how Linux's SDIO stack
happens to expose this at runtime from the binding description --
that's a software implementation detail, not a hardware/platform
description.
Changes in v5:
- Patch 1: added Krzysztof's Reviewed-by.
- Patch 2: only set host->pm_flags |= MMC_PM_KEEP_POWER after
mmc_deselect_cards() succeeds, instead of unconditionally before
it. Otherwise, if the deselect fails, the card is never marked
suspended, _mmc_resume() takes its early exit, and the flag never
gets cleared -- leaking it for the rest of uptime.
Changes in v4:
- Dropped the no-mmc-poweroff-suspend DT property and
MMC_CAP2_NO_POWEROFF_SUSPEND host capability entirely. Krzysztof
pointed out they described exactly the same contract as the
existing keep-power-in-suspend property (don't power off the card
across suspend/resume). Extended keep-power-in-suspend's scope
beyond SDIO instead, and reworked _mmc_suspend() to check
host->pm_caps & MMC_PM_KEEP_POWER directly rather than adding a
new capability.
- Gated the fast path on pm_type == MMC_POWEROFF_SUSPEND; it was
previously unconditional, so it wrongly skipped the required
power-off/notify handling during shutdown, unbind and
undervoltage as well.
- Set/clear host->pm_flags |= MMC_PM_KEEP_POWER around the suspend/
resume, mirroring the SDIO convention, so host drivers can tell
power was preserved if they need to.
Changes in v3:
- Reworked the fix in _mmc_suspend() (drivers/mmc/core/mmc.c) to
skip the poweroff-notify/sleep/power-off sequence entirely.
- Renamed no-mmc-sleep/MMC_CAP2_NO_SLEEP_CMD to
no-mmc-poweroff-suspend/MMC_CAP2_NO_POWEROFF_SUSPEND.
Changes in v2:
- Replaced v1's card-level MMC_QUIRK_BROKEN_SLEEP quirk with a host
capability and matching DT property, per Ulf's suggestion.
- Added Reported-by/Closes tags crediting Florian.
Kamal Dasu (3):
dt-bindings: mmc: Extend keep-power-in-suspend beyond SDIO
dt-bindings: mmc: Add reset-card-at-resume property
mmc: core: Honor keep-power-in-suspend and reset-card-at-resume
for (e)MMC
.../bindings/mmc/mmc-controller-common.yaml | 10 +++-
drivers/mmc/core/host.c | 2 +
drivers/mmc/core/mmc.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/mmc/host.h | 1 +
4 files changed, 108 insertions(+), 3 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v9 1/3] dt-bindings: mmc: Extend keep-power-in-suspend beyond SDIO 2026-08-11 16:38 [PATCH v9 0/3] mmc: core: Keep the card powered across suspend when firmware needs it live Kamal Dasu @ 2026-08-11 16:38 ` Kamal Dasu 2026-08-11 16:38 ` [PATCH v9 2/3] dt-bindings: mmc: Add reset-card-at-resume property Kamal Dasu 2026-08-11 16:38 ` [PATCH v9 3/3] mmc: core: Honor keep-power-in-suspend and reset-card-at-resume for (e)MMC Kamal Dasu 2 siblings, 0 replies; 5+ messages in thread From: Kamal Dasu @ 2026-08-11 16:38 UTC (permalink / raw) To: Ulf Hansson Cc: Kamal Dasu, Florian Fainelli, Wolfram Sang, Oleksij Rempel, Avri Altman, Pedro Demarchi Gomes, Erick Shepherd, Adrian Hunter, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-mmc, devicetree, linux-kernel, Krzysztof Kozlowski keep-power-in-suspend is currently documented as SDIO-only, but the requirement it describes -- preserving card power across a suspend/resume cycle -- applies just as well to any card type. Drop the SDIO-only restriction so eMMC and SD platforms can use it too. Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com> --- Changes in v9: - No change. Changes in v8: - No change. Changes in v7: - Added Krzysztof's Reviewed-by. Changes in v6: - Split into a separate, minimal patch per Ulf: just extend keep-power-in-suspend's scope, with no brcmstb/firmware-specific rationale in its description. That rationale now lives in the new reset-card-at-resume property instead (patch 2/3), and the two are set together on brcmstb rather than folded into one property. - Also per Ulf (applies to v4 and v5 alike): dropped the mention of sdio_set_host_pm_flags() and how Linux's SDIO stack happens to expose this at runtime -- that's a software implementation detail, not a hardware/platform description. Changes in v5: - Added Krzysztof's Reviewed-by. Changes in v4: - Dropped no-mmc-poweroff-suspend entirely and extended keep-power-in-suspend instead, per Krzysztof: the two properties described the same "don't power off across suspend/resume" contract. Changes in v3: - Renamed from no-mmc-sleep; dropped S_A_TIMEOUT framing per Ulf. Changes in v2: - New patch, replacing v1's card-level quirk, per Ulf. Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml index 3d7195e9461c..c18bf0d6a56e 100644 --- a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +++ b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml @@ -291,7 +291,7 @@ properties: keep-power-in-suspend: $ref: /schemas/types.yaml#/definitions/flag description: - SDIO only. Preserves card power during a suspend/resume cycle. + Preserves card power during a suspend/resume cycle. wakeup-source: $ref: /schemas/types.yaml#/definitions/flag -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v9 2/3] dt-bindings: mmc: Add reset-card-at-resume property 2026-08-11 16:38 [PATCH v9 0/3] mmc: core: Keep the card powered across suspend when firmware needs it live Kamal Dasu 2026-08-11 16:38 ` [PATCH v9 1/3] dt-bindings: mmc: Extend keep-power-in-suspend beyond SDIO Kamal Dasu @ 2026-08-11 16:38 ` Kamal Dasu 2026-08-11 16:38 ` [PATCH v9 3/3] mmc: core: Honor keep-power-in-suspend and reset-card-at-resume for (e)MMC Kamal Dasu 2 siblings, 0 replies; 5+ messages in thread From: Kamal Dasu @ 2026-08-11 16:38 UTC (permalink / raw) To: Ulf Hansson Cc: Kamal Dasu, Florian Fainelli, Wolfram Sang, Oleksij Rempel, Avri Altman, Pedro Demarchi Gomes, Erick Shepherd, Adrian Hunter, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-mmc, devicetree, linux-kernel, Krzysztof Kozlowski On some platforms, firmware or other hardware accesses the card during suspend/resume, before the kernel's own resume path has run, leaving the card in a state the kernel can no longer assume it knows. Add a flag property so such boards can tell the mmc core the card needs to be reset before it can be used again. This is expected to be paired with keep-power-in-suspend on boards whose firmware needs the card to stay powered and live throughout suspend: since the card is never power-cycled, nothing else would force it back to a known state on resume. Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com> --- Changes in v9: - No change. Changes in v8: - Sashiko's AI review of v7 pointed out this property's own description says it's "expected to be paired with keep-power-in-suspend", and patch 3/3 now enforces exactly that pairing in the driver, but nothing enforced it in the schema -- a DT could set reset-card-at-resume alone and still pass dt_binding_check. Rob Herring asked for this to be addressed on the list. Added a dependencies entry requiring keep-power-in-suspend whenever reset-card-at-resume is present. Changes in v7: - No other change here; patch 3/3 now requires this property alongside keep-power-in-suspend for (e)MMC rather than treating them as fully independent in the driver -- see that patch's changelog. Changes in v6: - New patch, per Ulf's suggestion: rather than fold "needs a reset at resume" into keep-power-in-suspend's own meaning, describe it as its own independent property, so the two can be combined only where actually needed (brcmstb sets both; SDIO's existing keep-power-in-suspend users are unaffected and set neither this nor a reset). Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml index c18bf0d6a56e..c51e9184384d 100644 --- a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +++ b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml @@ -293,6 +293,13 @@ properties: description: Preserves card power during a suspend/resume cycle. + reset-card-at-resume: + $ref: /schemas/types.yaml#/definitions/flag + description: + The HW/FW may have accessed the card during suspend/resume, + leaving it in an unknown state. Hence, before the card can be + used, it must be reset. + wakeup-source: $ref: /schemas/types.yaml#/definitions/flag description: @@ -361,5 +368,6 @@ patternProperties: dependencies: cd-debounce-delay-ms: [ cd-gpios ] fixed-emmc-driver-type: [ non-removable ] + reset-card-at-resume: [ keep-power-in-suspend ] additionalProperties: true -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v9 3/3] mmc: core: Honor keep-power-in-suspend and reset-card-at-resume for (e)MMC 2026-08-11 16:38 [PATCH v9 0/3] mmc: core: Keep the card powered across suspend when firmware needs it live Kamal Dasu 2026-08-11 16:38 ` [PATCH v9 1/3] dt-bindings: mmc: Extend keep-power-in-suspend beyond SDIO Kamal Dasu 2026-08-11 16:38 ` [PATCH v9 2/3] dt-bindings: mmc: Add reset-card-at-resume property Kamal Dasu @ 2026-08-11 16:38 ` Kamal Dasu 2026-09-07 13:58 ` Ulf Hansson 2 siblings, 1 reply; 5+ messages in thread From: Kamal Dasu @ 2026-08-11 16:38 UTC (permalink / raw) To: Ulf Hansson Cc: Kamal Dasu, Florian Fainelli, Wolfram Sang, Oleksij Rempel, Avri Altman, Pedro Demarchi Gomes, Erick Shepherd, Adrian Hunter, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-mmc, devicetree, linux-kernel On some platforms, firmware accesses the (e)MMC card directly during resume from Suspend-to-DRAM, before the kernel's own resume path has run, in order to load boot code. This requires the card to remain powered and responsive throughout suspend: putting it to sleep, sending it a power-off notification, or removing its supply is not safe, since firmware needs to talk to a live card. Since the card is never power-cycled, nothing else resets it back to a known state before the kernel reuses it after resume. keep-power-in-suspend / MMC_PM_KEEP_POWER already exist for the first part, but are only consumed in the SDIO suspend/resume path (mmc_sdio_suspend()/mmc_sdio_resume()), gated on a per-function runtime request via sdio_set_host_pm_flags(). (e)MMC has no equivalent function-driver layer to make that request, and the requirement here is a fixed platform characteristic rather than a per-cycle one, so _mmc_suspend() checks host->pm_caps directly instead of pm_flags. reset-card-at-resume covers the second part: when set, _mmc_resume() resets the host to its initial bus state the same way _mmc_hw_reset() does for a non-power-cycle reset, before mmc_power_up() and mmc_init_card() re-identify the card. _mmc_suspend()'s fast path requires both MMC_PM_KEEP_POWER and MMC_CAP2_RESET_AT_RESUME to be set. Keeping the card powered without also resetting it at resume is not safe for this driver: skipping mmc_power_off() leaves power_mode at MMC_POWER_ON, so mmc_power_up() no-ops in _mmc_resume(), and without an explicit reset first, mmc_init_card() runs against whatever bus speed/width was active before suspend instead of the initial state it expects. Conversely, _mmc_resume()'s reset checks pm_flags rather than the MMC_CAP2_RESET_AT_RESUME capability directly, since pm_flags is only set when the fast path actually ran -- the only time power_mode is guaranteed to still be MMC_POWER_ON, and so the only time resetting the bus before mmc_power_up() is both necessary and safe. Without that check, MMC_CAP2_RESET_AT_RESUME set on its own would drive the clock and bus lines while the card's supply is still off following a normal mmc_power_off(). The fast path also leaves the card marked suspended without powering it off, so _mmc_suspend()'s pre-existing early exit for an already-suspended card can no longer assume there is nothing left to do: a shutdown, unbind or undervoltage event can still arrive before the card's next real access lazily triggers _mmc_resume() via runtime PM, and those events need the normal power-off sequence regardless. Reselect the card and continue into that sequence in that case, instead of returning early and silently skipping mmc_poweroff_notify() and mmc_power_off(). Reset the host to its initial bus state first, the same way _mmc_resume() does, since firmware may have left the card in a state that doesn't decode reliably at whatever clock/timing was negotiated before suspend -- otherwise mmc_select_card() itself can fail and abort into that same early exit, before ever reaching the sequence it was supposed to fall through to. If mmc_select_card() still fails after that reset -- the card may genuinely be gone -- fall through to mmc_power_off() directly rather than aborting again: unlike the other steps here, powering off doesn't need a responsive card, and shutdown/undervoltage need the supply actually removed regardless. host->pm_flags is set alongside marking the card suspended, and cleared in _mmc_resume(), so host controller resume handlers can tell power was preserved if they need to. Reported-by: Florian Fainelli <florian.fainelli@broadcom.com> Closes: https://lore.kernel.org/r/20260413180551.3683969-1-florian.fainelli@broadcom.com/ Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com> --- Changes in v9: - Sashiko's AI review found two more High severity issues in the same shutdown/undervoltage fallback added in v8: * Calling mmc_select_card() straight after finding the card still marked suspended assumed the card would respond to CMD7 as-is, but that's exactly the assumption reset-card-at-resume exists to distrust -- firmware may have left the card unable to decode commands at whatever clock/timing was negotiated before suspend. A CMD7 timeout there hit "goto out" and skipped the power-off sequence entirely, the same failure mode this fallback was added to close in v8. Fixed by resetting the host to its initial bus state before mmc_select_card(), the same way _mmc_resume() already does before touching the card. * Even with that reset, mmc_select_card() can still legitimately fail (card genuinely gone). The code still unconditionally "goto out" in that case, skipping mmc_power_off() entirely -- but mmc_power_off() is host/regulator-side only and doesn't need a responsive card, so there's no reason a select failure should leave the supply on indefinitely for a shutdown or undervoltage event, or leave pm_flags/suspended state stuck retrying the same failing select on every later call. Fixed by falling through to mmc_power_off() directly on select failure instead of aborting. - Patches 1/3 and 2/3 are unchanged from v8. Changes in v8: - Sashiko's AI review of v7 found a High severity issue: the keep-power fast path marks the card suspended without powering it off, but the pre-existing early exit at the top of _mmc_suspend() (for an already-suspended card) doesn't know that -- it just returns immediately regardless of pm_type. Since the system-resume path only re-enables runtime PM rather than calling _mmc_resume() directly (that happens lazily, via runtime PM, on the card's next real access), a shutdown, unbind or undervoltage event landing before that next access would hit the early exit and silently skip mmc_poweroff_notify()/mmc_power_off() entirely. Fixed by reselecting the card and continuing into the normal power-off sequence whenever the card is suspended via our fast path but pm_type isn't a plain suspend, instead of a bare early exit. Calling _mmc_resume() itself from here isn't an option, since _mmc_suspend() already holds the host claim. Changes in v7: - Sashiko's AI review of v6 found two real, complementary bugs in treating MMC_PM_KEEP_POWER and MMC_CAP2_RESET_AT_RESUME as fully independent in this driver: * keep-power-in-suspend without reset-card-at-resume: confirmed on hardware to hang -- _mmc_resume()'s mmc_power_up() no-ops since power_mode never left MMC_POWER_ON, so mmc_init_card() runs at the pre-suspend bus speed and CMD1 times out. * reset-card-at-resume without keep-power-in-suspend: the reset block ran mmc_set_clock()/mmc_set_initial_state() ahead of mmc_power_up(), while power_mode was still MMC_POWER_OFF from a normal suspend-time mmc_power_off() -- driving the clock and bus lines before the card's supply is enabled. Fixed by requiring both capabilities together for the suspend fast path, and checking pm_flags (not the raw capability) for the resume-side reset, so it only ever runs when power was actually kept this cycle. Verified on hardware: the keep-power-in-suspend- without-reset-card-at-resume case now correctly falls through to a normal power-off/power-on cycle instead of hanging, and the paired-capability case is unaffected (still hardware-verified, now with an extra confirmation run after this fix). Changes in v6: - Reworked around Ulf's two-property split: dropped the unconditional mmc_set_clock()/mmc_set_initial_state() reset from the suspend-side fast path, and instead perform it in _mmc_resume(), gated on the new MMC_CAP2_RESET_AT_RESUME (from reset-card-at-resume), matching the property's name and description ("before the card can be used, it must be reset"). - No longer touches MMC_CAP2_NO_POWEROFF_SUSPEND/no-mmc-poweroff- suspend at all -- that capability and property are gone, per the v4 rework; this patch only adds MMC_CAP2_RESET_AT_RESUME. Changes in v5: - Only set host->pm_flags |= MMC_PM_KEEP_POWER after mmc_deselect_cards() succeeds, instead of unconditionally before it. Otherwise, if the deselect fails, the card is never marked suspended, _mmc_resume() takes its early exit, and the flag never gets cleared -- leaking it for the rest of uptime. Changes in v4: - Gated the fast path on pm_type == MMC_POWEROFF_SUSPEND; it was previously unconditional, so it wrongly skipped the required power-off/notify handling during shutdown, unbind and undervoltage as well. - Set/clear host->pm_flags |= MMC_PM_KEEP_POWER around the suspend/ resume, mirroring the SDIO convention, so host controller resume handlers can tell power was preserved and perform a soft resume sequence instead of assuming power was lost. - Dropped MMC_CAP2_NO_POWEROFF_SUSPEND and the no-mmc-poweroff- suspend DT property entirely. Reuse keep-power-in-suspend / MMC_PM_KEEP_POWER instead, per Krzysztof's point that the new property described the same contract as the existing one. _mmc_suspend() now checks host->pm_caps directly rather than pm_flags, since (e)MMC has no per-function driver to make the dynamic sdio_set_host_pm_flags()-style request SDIO uses. Changes in v3: - Reworked _mmc_suspend() to skip poweroff-notify/sleep/power-off entirely, not just SLEEP, per Ulf. - Renamed to MMC_CAP2_NO_POWEROFF_SUSPEND/no-mmc-poweroff-suspend. Changes in v2: - Replaced the card-level MMC_QUIRK_BROKEN_SLEEP quirk with a host capability, per Ulf. - Added Reported-by/Closes crediting Florian. drivers/mmc/core/host.c | 2 + drivers/mmc/core/mmc.c | 98 +++++++++++++++++++++++++++++++++++++++- include/linux/mmc/host.h | 1 + 3 files changed, 99 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c index b7ce3137d452..1622f7846441 100644 --- a/drivers/mmc/core/host.c +++ b/drivers/mmc/core/host.c @@ -400,6 +400,8 @@ int mmc_of_parse(struct mmc_host *host) if (device_property_read_bool(dev, "no-mmc-hs400")) host->caps2 &= ~(MMC_CAP2_HS400_1_8V | MMC_CAP2_HS400_1_2V | MMC_CAP2_HS400_ES); + if (device_property_read_bool(dev, "reset-card-at-resume")) + host->caps2 |= MMC_CAP2_RESET_AT_RESUME; /* Must be after "non-removable" check */ if (device_property_read_u32(dev, "fixed-emmc-driver-type", &drv_type) == 0) { diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 05444ecf3909..3a0855a4ca44 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -2144,8 +2144,55 @@ static int _mmc_suspend(struct mmc_host *host, enum mmc_poweroff_type pm_type) mmc_claim_host(host); - if (mmc_card_suspended(host->card)) - goto out; + if (mmc_card_suspended(host->card)) { + /* + * Nothing to do for a redundant suspend call. Otherwise, the + * card can only still be marked suspended here because the + * keep-power fast path below left it powered and merely + * deselected -- reselect it and continue into the normal + * power-off sequence below, since shutdown, unbind and + * undervoltage need mmc_power_off() regardless of how the + * card got here. + */ + if (pm_type == MMC_POWEROFF_SUSPEND || + !(host->pm_flags & MMC_PM_KEEP_POWER)) + goto out; + + /* + * Firmware may have accessed the card while it stayed + * powered through suspend, leaving it in a state the kernel + * can no longer assume it knows. Reset the host to its + * initial bus state first, same as _mmc_resume() does before + * touching the card, so mmc_select_card() below is talking + * at a clock/timing every card is guaranteed to decode + * rather than whatever mode was negotiated before suspend. + */ + mmc_set_clock(host, host->f_init); + mmc_set_initial_state(host); + + if (!mmc_host_is_spi(host)) + err = mmc_select_card(host->card); + + if (err) { + /* + * The card still isn't responding even after the + * reset above -- it may genuinely be gone. Nothing + * else below needs a responsive card except + * mmc_power_off() itself, which is host/regulator + * side only, so cut power directly instead of + * leaving the card's supply on indefinitely (and + * pm_flags/suspended state stuck retrying this same + * failing select on every later call). + */ + mmc_power_off(host); + host->pm_flags &= ~MMC_PM_KEEP_POWER; + mmc_card_set_suspended(host->card); + goto out; + } + + mmc_card_clr_suspended(host->card); + host->pm_flags &= ~MMC_PM_KEEP_POWER; + } /* * For the undervoltage case, we care more about device integrity. @@ -2157,6 +2204,33 @@ 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. + * + * Require MMC_CAP2_RESET_AT_RESUME too: without it, _mmc_resume() + * has no way to bring the host back to a state mmc_init_card() can + * use, since mmc_power_up() no-ops when power_mode is already + * MMC_POWER_ON. Keeping power without also resetting at resume is + * not a safe combination for this driver. + */ + if (pm_type == MMC_POWEROFF_SUSPEND && + (host->pm_caps & MMC_PM_KEEP_POWER) && + (host->caps2 & MMC_CAP2_RESET_AT_RESUME)) { + if (!mmc_host_is_spi(host)) + err = mmc_deselect_cards(host); + if (!err) { + host->pm_flags |= MMC_PM_KEEP_POWER; + 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); @@ -2217,9 +2291,29 @@ static int _mmc_resume(struct mmc_host *host) if (!mmc_card_suspended(host->card)) goto out; + /* + * Firmware or other hardware may have accessed the card while it + * stayed powered through suspend, leaving it in a state the kernel + * can no longer assume it knows. Reset the host to its initial bus + * state like _mmc_hw_reset() does for a non-power-cycle reset, + * before mmc_init_card() re-identifies the card. + * + * Check pm_flags, not the MMC_CAP2_RESET_AT_RESUME capability + * directly: pm_flags only ends up set here when _mmc_suspend() + * actually took the keep-power fast path this cycle, which is the + * only time power_mode is guaranteed to still be MMC_POWER_ON (and + * so the only time this reset is both necessary and safe to do + * before mmc_power_up() touches the bus). + */ + if (host->pm_flags & MMC_PM_KEEP_POWER) { + mmc_set_clock(host, host->f_init); + mmc_set_initial_state(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); diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index ba84f02c2a10..14a407a9f9b7 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -463,6 +463,7 @@ struct mmc_host { #define MMC_CAP2_CRYPTO 0 #endif #define MMC_CAP2_ALT_GPT_TEGRA (1 << 28) /* Host with eMMC that has GPT entry at a non-standard location */ +#define MMC_CAP2_RESET_AT_RESUME (1 << 29) /* Card must be reset before use at resume */ bool uhs2_sd_tran; /* UHS-II flag for SD_TRAN state */ bool uhs2_app_cmd; /* UHS-II flag for APP command */ -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v9 3/3] mmc: core: Honor keep-power-in-suspend and reset-card-at-resume for (e)MMC 2026-08-11 16:38 ` [PATCH v9 3/3] mmc: core: Honor keep-power-in-suspend and reset-card-at-resume for (e)MMC Kamal Dasu @ 2026-09-07 13:58 ` Ulf Hansson 0 siblings, 0 replies; 5+ messages in thread From: Ulf Hansson @ 2026-09-07 13:58 UTC (permalink / raw) To: Kamal Dasu Cc: Ulf Hansson, Florian Fainelli, Wolfram Sang, Oleksij Rempel, Avri Altman, Pedro Demarchi Gomes, Erick Shepherd, Adrian Hunter, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-mmc, devicetree, linux-kernel On Tue, Aug 11, 2026 at 6:40 PM Kamal Dasu <kamal.dasu@broadcom.com> wrote: > > On some platforms, firmware accesses the (e)MMC card directly during > resume from Suspend-to-DRAM, before the kernel's own resume path has > run, in order to load boot code. This requires the card to remain > powered and responsive throughout suspend: putting it to sleep, > sending it a power-off notification, or removing its supply is not > safe, since firmware needs to talk to a live card. Since the card is > never power-cycled, nothing else resets it back to a known state > before the kernel reuses it after resume. > > keep-power-in-suspend / MMC_PM_KEEP_POWER already exist for the first > part, but are only consumed in the SDIO suspend/resume path > (mmc_sdio_suspend()/mmc_sdio_resume()), gated on a per-function > runtime request via sdio_set_host_pm_flags(). (e)MMC has no > equivalent function-driver layer to make that request, and the > requirement here is a fixed platform characteristic rather than a > per-cycle one, so _mmc_suspend() checks host->pm_caps directly > instead of pm_flags. > > reset-card-at-resume covers the second part: when set, _mmc_resume() > resets the host to its initial bus state the same way _mmc_hw_reset() > does for a non-power-cycle reset, before mmc_power_up() and > mmc_init_card() re-identify the card. > > _mmc_suspend()'s fast path requires both MMC_PM_KEEP_POWER and > MMC_CAP2_RESET_AT_RESUME to be set. Keeping the card powered without > also resetting it at resume is not safe for this driver: skipping > mmc_power_off() leaves power_mode at MMC_POWER_ON, so mmc_power_up() > no-ops in _mmc_resume(), and without an explicit reset first, > mmc_init_card() runs against whatever bus speed/width was active > before suspend instead of the initial state it expects. Conversely, > _mmc_resume()'s reset checks pm_flags rather than the > MMC_CAP2_RESET_AT_RESUME capability directly, since pm_flags is only > set when the fast path actually ran -- the only time power_mode is > guaranteed to still be MMC_POWER_ON, and so the only time resetting > the bus before mmc_power_up() is both necessary and safe. Without > that check, MMC_CAP2_RESET_AT_RESUME set on its own would drive the > clock and bus lines while the card's supply is still off following a > normal mmc_power_off(). > > The fast path also leaves the card marked suspended without powering > it off, so _mmc_suspend()'s pre-existing early exit for an > already-suspended card can no longer assume there is nothing left to > do: a shutdown, unbind or undervoltage event can still arrive before > the card's next real access lazily triggers _mmc_resume() via runtime > PM, and those events need the normal power-off sequence regardless. > Reselect the card and continue into that sequence in that case, > instead of returning early and silently skipping mmc_poweroff_notify() > and mmc_power_off(). Reset the host to its initial bus state first, > the same way _mmc_resume() does, since firmware may have left the > card in a state that doesn't decode reliably at whatever clock/timing > was negotiated before suspend -- otherwise mmc_select_card() itself > can fail and abort into that same early exit, before ever reaching > the sequence it was supposed to fall through to. If mmc_select_card() > still fails after that reset -- the card may genuinely be gone -- fall > through to mmc_power_off() directly rather than aborting again: > unlike the other steps here, powering off doesn't need a responsive > card, and shutdown/undervoltage need the supply actually removed > regardless. > > host->pm_flags is set alongside marking the card suspended, and > cleared in _mmc_resume(), so host controller resume handlers can tell > power was preserved if they need to. While I certainly appreciate a descriptive commit message, this is a bit overwhelming. Please consider making commit messages a bit less wordy and more to the point. By reading the above, it becomes clear to me that we should really split this patch. Both for reviewing purposes, but also to use the step-by-step approach. I believe the first the patch should take care of the updated interpretation of MMC_PM_KEEP_POWER for eMMC and a second patch on top should deal with the "reset-card-at-resume" thing. Anyway, I understand the code in the mmc core for power management is somewhat tricky to follow. Rather than continue to provide you with in-depth guidance and review comments, allow me to spend a couple of days trying to make the implementation for the above and send out a few patches that you can test. Kind regards Uffe > > Reported-by: Florian Fainelli <florian.fainelli@broadcom.com> > Closes: https://lore.kernel.org/r/20260413180551.3683969-1-florian.fainelli@broadcom.com/ > Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com> > --- > Changes in v9: > - Sashiko's AI review found two more High severity issues in the > same shutdown/undervoltage fallback added in v8: > * Calling mmc_select_card() straight after finding the card > still marked suspended assumed the card would respond to > CMD7 as-is, but that's exactly the assumption > reset-card-at-resume exists to distrust -- firmware may have > left the card unable to decode commands at whatever > clock/timing was negotiated before suspend. A CMD7 timeout > there hit "goto out" and skipped the power-off sequence > entirely, the same failure mode this fallback was added to > close in v8. Fixed by resetting the host to its initial bus > state before mmc_select_card(), the same way _mmc_resume() > already does before touching the card. > * Even with that reset, mmc_select_card() can still legitimately > fail (card genuinely gone). The code still unconditionally > "goto out" in that case, skipping mmc_power_off() entirely -- > but mmc_power_off() is host/regulator-side only and doesn't > need a responsive card, so there's no reason a select failure > should leave the supply on indefinitely for a shutdown or > undervoltage event, or leave pm_flags/suspended state stuck > retrying the same failing select on every later call. Fixed > by falling through to mmc_power_off() directly on select > failure instead of aborting. > - Patches 1/3 and 2/3 are unchanged from v8. > > Changes in v8: > - Sashiko's AI review of v7 found a High severity issue: the > keep-power fast path marks the card suspended without powering > it off, but the pre-existing early exit at the top of > _mmc_suspend() (for an already-suspended card) doesn't know that > -- it just returns immediately regardless of pm_type. Since the > system-resume path only re-enables runtime PM rather than calling > _mmc_resume() directly (that happens lazily, via runtime PM, on > the card's next real access), a shutdown, unbind or undervoltage > event landing before that next access would hit the early exit > and silently skip mmc_poweroff_notify()/mmc_power_off() entirely. > Fixed by reselecting the card and continuing into the normal > power-off sequence whenever the card is suspended via our fast > path but pm_type isn't a plain suspend, instead of a bare early > exit. Calling _mmc_resume() itself from here isn't an option, since > _mmc_suspend() already holds the host claim. > > Changes in v7: > - Sashiko's AI review of v6 found two real, complementary bugs in > treating MMC_PM_KEEP_POWER and MMC_CAP2_RESET_AT_RESUME as fully > independent in this driver: > * keep-power-in-suspend without reset-card-at-resume: confirmed > on hardware to hang -- _mmc_resume()'s mmc_power_up() no-ops > since power_mode never left MMC_POWER_ON, so mmc_init_card() > runs at the pre-suspend bus speed and CMD1 times out. > * reset-card-at-resume without keep-power-in-suspend: the reset > block ran mmc_set_clock()/mmc_set_initial_state() ahead of > mmc_power_up(), while power_mode was still MMC_POWER_OFF from > a normal suspend-time mmc_power_off() -- driving the clock > and bus lines before the card's supply is enabled. > Fixed by requiring both capabilities together for the suspend > fast path, and checking pm_flags (not the raw capability) for the > resume-side reset, so it only ever runs when power was actually > kept this cycle. Verified on hardware: the keep-power-in-suspend- > without-reset-card-at-resume case now correctly falls through to > a normal power-off/power-on cycle instead of hanging, and the > paired-capability case is unaffected (still hardware-verified, > now with an extra confirmation run after this fix). > > Changes in v6: > - Reworked around Ulf's two-property split: dropped the > unconditional mmc_set_clock()/mmc_set_initial_state() reset from > the suspend-side fast path, and instead perform it in > _mmc_resume(), gated on the new MMC_CAP2_RESET_AT_RESUME (from > reset-card-at-resume), matching the property's name and > description ("before the card can be used, it must be reset"). > - No longer touches MMC_CAP2_NO_POWEROFF_SUSPEND/no-mmc-poweroff- > suspend at all -- that capability and property are gone, per the > v4 rework; this patch only adds MMC_CAP2_RESET_AT_RESUME. > > Changes in v5: > - Only set host->pm_flags |= MMC_PM_KEEP_POWER after > mmc_deselect_cards() succeeds, instead of unconditionally before > it. Otherwise, if the deselect fails, the card is never marked > suspended, _mmc_resume() takes its early exit, and the flag never > gets cleared -- leaking it for the rest of uptime. > > Changes in v4: > - Gated the fast path on pm_type == MMC_POWEROFF_SUSPEND; it was > previously unconditional, so it wrongly skipped the required > power-off/notify handling during shutdown, unbind and > undervoltage as well. > - Set/clear host->pm_flags |= MMC_PM_KEEP_POWER around the suspend/ > resume, mirroring the SDIO convention, so host controller resume > handlers can tell power was preserved and perform a soft resume > sequence instead of assuming power was lost. > - Dropped MMC_CAP2_NO_POWEROFF_SUSPEND and the no-mmc-poweroff- > suspend DT property entirely. Reuse keep-power-in-suspend / > MMC_PM_KEEP_POWER instead, per Krzysztof's point that the new > property described the same contract as the existing one. > _mmc_suspend() now checks host->pm_caps directly rather than > pm_flags, since (e)MMC has no per-function driver to make the > dynamic sdio_set_host_pm_flags()-style request SDIO uses. > > Changes in v3: > - Reworked _mmc_suspend() to skip poweroff-notify/sleep/power-off > entirely, not just SLEEP, per Ulf. > - Renamed to MMC_CAP2_NO_POWEROFF_SUSPEND/no-mmc-poweroff-suspend. > > Changes in v2: > - Replaced the card-level MMC_QUIRK_BROKEN_SLEEP quirk with a host > capability, per Ulf. > - Added Reported-by/Closes crediting Florian. > > drivers/mmc/core/host.c | 2 + > drivers/mmc/core/mmc.c | 98 +++++++++++++++++++++++++++++++++++++++- > include/linux/mmc/host.h | 1 + > 3 files changed, 99 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c > index b7ce3137d452..1622f7846441 100644 > --- a/drivers/mmc/core/host.c > +++ b/drivers/mmc/core/host.c > @@ -400,6 +400,8 @@ int mmc_of_parse(struct mmc_host *host) > if (device_property_read_bool(dev, "no-mmc-hs400")) > host->caps2 &= ~(MMC_CAP2_HS400_1_8V | MMC_CAP2_HS400_1_2V | > MMC_CAP2_HS400_ES); > + if (device_property_read_bool(dev, "reset-card-at-resume")) > + host->caps2 |= MMC_CAP2_RESET_AT_RESUME; > > /* Must be after "non-removable" check */ > if (device_property_read_u32(dev, "fixed-emmc-driver-type", &drv_type) == 0) { > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index 05444ecf3909..3a0855a4ca44 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -2144,8 +2144,55 @@ static int _mmc_suspend(struct mmc_host *host, enum mmc_poweroff_type pm_type) > > mmc_claim_host(host); > > - if (mmc_card_suspended(host->card)) > - goto out; > + if (mmc_card_suspended(host->card)) { > + /* > + * Nothing to do for a redundant suspend call. Otherwise, the > + * card can only still be marked suspended here because the > + * keep-power fast path below left it powered and merely > + * deselected -- reselect it and continue into the normal > + * power-off sequence below, since shutdown, unbind and > + * undervoltage need mmc_power_off() regardless of how the > + * card got here. > + */ > + if (pm_type == MMC_POWEROFF_SUSPEND || > + !(host->pm_flags & MMC_PM_KEEP_POWER)) > + goto out; > + > + /* > + * Firmware may have accessed the card while it stayed > + * powered through suspend, leaving it in a state the kernel > + * can no longer assume it knows. Reset the host to its > + * initial bus state first, same as _mmc_resume() does before > + * touching the card, so mmc_select_card() below is talking > + * at a clock/timing every card is guaranteed to decode > + * rather than whatever mode was negotiated before suspend. > + */ > + mmc_set_clock(host, host->f_init); > + mmc_set_initial_state(host); > + > + if (!mmc_host_is_spi(host)) > + err = mmc_select_card(host->card); > + > + if (err) { > + /* > + * The card still isn't responding even after the > + * reset above -- it may genuinely be gone. Nothing > + * else below needs a responsive card except > + * mmc_power_off() itself, which is host/regulator > + * side only, so cut power directly instead of > + * leaving the card's supply on indefinitely (and > + * pm_flags/suspended state stuck retrying this same > + * failing select on every later call). > + */ > + mmc_power_off(host); > + host->pm_flags &= ~MMC_PM_KEEP_POWER; > + mmc_card_set_suspended(host->card); > + goto out; > + } > + > + mmc_card_clr_suspended(host->card); > + host->pm_flags &= ~MMC_PM_KEEP_POWER; > + } > > /* > * For the undervoltage case, we care more about device integrity. > @@ -2157,6 +2204,33 @@ 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. > + * > + * Require MMC_CAP2_RESET_AT_RESUME too: without it, _mmc_resume() > + * has no way to bring the host back to a state mmc_init_card() can > + * use, since mmc_power_up() no-ops when power_mode is already > + * MMC_POWER_ON. Keeping power without also resetting at resume is > + * not a safe combination for this driver. > + */ > + if (pm_type == MMC_POWEROFF_SUSPEND && > + (host->pm_caps & MMC_PM_KEEP_POWER) && > + (host->caps2 & MMC_CAP2_RESET_AT_RESUME)) { > + if (!mmc_host_is_spi(host)) > + err = mmc_deselect_cards(host); > + if (!err) { > + host->pm_flags |= MMC_PM_KEEP_POWER; > + 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); > @@ -2217,9 +2291,29 @@ static int _mmc_resume(struct mmc_host *host) > if (!mmc_card_suspended(host->card)) > goto out; > > + /* > + * Firmware or other hardware may have accessed the card while it > + * stayed powered through suspend, leaving it in a state the kernel > + * can no longer assume it knows. Reset the host to its initial bus > + * state like _mmc_hw_reset() does for a non-power-cycle reset, > + * before mmc_init_card() re-identifies the card. > + * > + * Check pm_flags, not the MMC_CAP2_RESET_AT_RESUME capability > + * directly: pm_flags only ends up set here when _mmc_suspend() > + * actually took the keep-power fast path this cycle, which is the > + * only time power_mode is guaranteed to still be MMC_POWER_ON (and > + * so the only time this reset is both necessary and safe to do > + * before mmc_power_up() touches the bus). > + */ > + if (host->pm_flags & MMC_PM_KEEP_POWER) { > + mmc_set_clock(host, host->f_init); > + mmc_set_initial_state(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); > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h > index ba84f02c2a10..14a407a9f9b7 100644 > --- a/include/linux/mmc/host.h > +++ b/include/linux/mmc/host.h > @@ -463,6 +463,7 @@ struct mmc_host { > #define MMC_CAP2_CRYPTO 0 > #endif > #define MMC_CAP2_ALT_GPT_TEGRA (1 << 28) /* Host with eMMC that has GPT entry at a non-standard location */ > +#define MMC_CAP2_RESET_AT_RESUME (1 << 29) /* Card must be reset before use at resume */ > > bool uhs2_sd_tran; /* UHS-II flag for SD_TRAN state */ > bool uhs2_app_cmd; /* UHS-II flag for APP command */ > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-07 13:59 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-11 16:38 [PATCH v9 0/3] mmc: core: Keep the card powered across suspend when firmware needs it live Kamal Dasu 2026-08-11 16:38 ` [PATCH v9 1/3] dt-bindings: mmc: Extend keep-power-in-suspend beyond SDIO Kamal Dasu 2026-08-11 16:38 ` [PATCH v9 2/3] dt-bindings: mmc: Add reset-card-at-resume property Kamal Dasu 2026-08-11 16:38 ` [PATCH v9 3/3] mmc: core: Honor keep-power-in-suspend and reset-card-at-resume for (e)MMC Kamal Dasu 2026-09-07 13:58 ` Ulf Hansson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox