All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/2] mmc: core: Keep the card powered across suspend when firmware needs it live
@ 2026-07-29 20:14 Kamal Dasu
  2026-07-29 20:14 ` [PATCH v5 1/2] dt-bindings: mmc: Extend keep-power-in-suspend beyond SDIO Kamal Dasu
  2026-07-29 20:14 ` [PATCH v5 2/2] mmc: core: Honor keep-power-in-suspend for (e)MMC in suspend/resume Kamal Dasu
  0 siblings, 2 replies; 5+ messages in thread
From: Kamal Dasu @ 2026-07-29 20:14 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 v5.

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.

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. (e)MMC has no per-function driver to make the
    dynamic sdio_set_host_pm_flags() request SDIO uses, so this reads
    pm_caps -- the DT-derived, fixed platform characteristic -- not
    pm_flags.
  - 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.
  - Reset the host to its initial bus state (mmc_set_clock() +
    mmc_set_initial_state(), the same helpers _mmc_hw_reset() uses for
    a non-power-cycle reset) before marking the card suspended.
  - 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 (2):
  dt-bindings: mmc: Extend keep-power-in-suspend beyond SDIO
  mmc: core: Honor keep-power-in-suspend for (e)MMC in suspend/resume

 .../bindings/mmc/mmc-controller-common.yaml |  7 ++++-
 drivers/mmc/core/mmc.c                      | 30 ++++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)

--
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v5 1/2] dt-bindings: mmc: Extend keep-power-in-suspend beyond SDIO
  2026-07-29 20:14 [PATCH v5 0/2] mmc: core: Keep the card powered across suspend when firmware needs it live Kamal Dasu
@ 2026-07-29 20:14 ` Kamal Dasu
  2026-07-29 20:23   ` sashiko-bot
  2026-07-29 20:14 ` [PATCH v5 2/2] mmc: core: Honor keep-power-in-suspend for (e)MMC in suspend/resume Kamal Dasu
  1 sibling, 1 reply; 5+ messages in thread
From: Kamal Dasu @ 2026-07-29 20:14 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 already documents exactly the requirement
needed on (e)MMC platforms where firmware accesses the card directly
during resume. Rather than add a separate property describing the
same contract, drop its "SDIO only" restriction and document the
(e)MMC use case alongside the existing SDIO one.

Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
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 | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml
index 3d7195e9461c..f4663b318435 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml
+++ b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml
@@ -291,7 +291,12 @@ 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. For SDIO,
+      this is a request an individual function driver can make at
+      runtime via sdio_set_host_pm_flags(). For (e)MMC, it is instead
+      a fixed platform characteristic, needed on boards where firmware
+      accesses the card directly during system resume, before the
+      kernel's own resume path runs.
 
   wakeup-source:
     $ref: /schemas/types.yaml#/definitions/flag
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v5 2/2] mmc: core: Honor keep-power-in-suspend for (e)MMC in suspend/resume
  2026-07-29 20:14 [PATCH v5 0/2] mmc: core: Keep the card powered across suspend when firmware needs it live Kamal Dasu
  2026-07-29 20:14 ` [PATCH v5 1/2] dt-bindings: mmc: Extend keep-power-in-suspend beyond SDIO Kamal Dasu
@ 2026-07-29 20:14 ` Kamal Dasu
  2026-07-29 20:29   ` sashiko-bot
  1 sibling, 1 reply; 5+ messages in thread
From: Kamal Dasu @ 2026-07-29 20:14 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.

keep-power-in-suspend / MMC_PM_KEEP_POWER already exist for this
purpose, 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.

When pm_caps has MMC_PM_KEEP_POWER set and pm_type is
MMC_POWEROFF_SUSPEND, skip the poweroff-notify/sleep/power-off
sequence entirely: deselect the card, reset the host to its initial
bus state the same way _mmc_hw_reset() does for a non-power-cycle
reset, mark the card suspended, and set host->pm_flags |=
MMC_PM_KEEP_POWER.

Setting MMC_PM_KEEP_POWER in host->pm_flags during suspend ensures
that host controller resume handlers are aware that card power was
preserved across suspend, allowing them to perform a soft resume
sequence instead of assuming power was lost. Clear this flag in
_mmc_resume() upon completion.

Only set pm_flags and mark the card suspended after
mmc_deselect_cards() succeeds. If it fails, the card is never marked
suspended, so _mmc_resume() takes its early "not suspended" exit and
never reaches the code that clears MMC_PM_KEEP_POWER -- setting the
flag unconditionally would leak it for the remainder of uptime.

The pm_type check matters because _mmc_suspend() is also called for
shutdown, driver unbind, and undervoltage, none of which are
guaranteed a subsequent _mmc_resume() call, so those still need the
normal power-off path.

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 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.
  - Reset the host to its initial bus state (mmc_set_clock() +
    mmc_set_initial_state(), matching _mmc_hw_reset()'s non-power-
    cycle path) before marking the card suspended.
  - 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/mmc.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 05444ecf3909..7cbe65532726 100644
--- 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)) {
+		if (!mmc_host_is_spi(host))
+			err = mmc_deselect_cards(host);
+		if (!err) {
+			host->pm_flags |= MMC_PM_KEEP_POWER;
+			mmc_set_clock(host, host->f_init);
+			mmc_set_initial_state(host);
+			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);
@@ -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);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v5 1/2] dt-bindings: mmc: Extend keep-power-in-suspend beyond SDIO
  2026-07-29 20:14 ` [PATCH v5 1/2] dt-bindings: mmc: Extend keep-power-in-suspend beyond SDIO Kamal Dasu
@ 2026-07-29 20:23   ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-07-29 20:23 UTC (permalink / raw)
  To: Kamal Dasu; +Cc: robh, conor+dt, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The commit documents `keep-power-in-suspend` as a supported contract for (e)MMC platforms to preserve power during suspend, but the MMC core does not implement this behavior for non-SDIO cards, resulting in a false contract. The MMC core will unconditionally power off the card during suspend, breaking firmware that relies on the card remaining powered.
--

--- Patch [1]: [PATCH v5 1/2] dt-bindings: mmc: Extend keep-power-in-suspend beyond SDIO ---
commit 4c90177be3ef2dae76dde526bf579376039fcb17
Author: Kamal Dasu <kamal.dasu@broadcom.com>

dt-bindings: mmc: Extend keep-power-in-suspend beyond SDIO

This commit drops the "SDIO only" restriction for the keep-power-in-suspend
property in the MMC device tree bindings. It updates the documentation to
state that for (e)MMC devices, this property indicates a platform requirement
where firmware accesses the card directly during resume.

> diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml
> --- a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml
> +++ b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml
> @@ -291,7 +291,12 @@ 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. For SDIO,
> +      this is a request an individual function driver can make at
> +      runtime via sdio_set_host_pm_flags(). For (e)MMC, it is instead
> +      a fixed platform characteristic, needed on boards where firmware
> +      accesses the card directly during system resume, before the
> +      kernel's own resume path runs.

[Severity: High]
Does the MMC core actually support keeping power for non-SDIO cards during
suspend?

Looking at the implementation in drivers/mmc/core/mmc.c, _mmc_suspend()
appears to unconditionally power off the card:

drivers/mmc/core/mmc.c:_mmc_suspend() {
        ...
        if (!err) {
                mmc_power_off(host);
                mmc_card_set_suspended(host->card);
        }
}

Unlike the SDIO suspend path, it lacks a check for mmc_card_keep_power(host).
Without this check, won't the kernel still power off the card during suspend
even if keep-power-in-suspend is defined in the device tree? 

If firmware attempts to access the unpowered MMC device during early resume
because it expects this DT contract to be honored, could this result in a
system crash or resume hang?

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5 2/2] mmc: core: Honor keep-power-in-suspend for (e)MMC in suspend/resume
  2026-07-29 20:14 ` [PATCH v5 2/2] mmc: core: Honor keep-power-in-suspend for (e)MMC in suspend/resume Kamal Dasu
@ 2026-07-29 20:29   ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-07-29 20:29 UTC (permalink / raw)
  To: Kamal Dasu; +Cc: conor+dt, robh, devicetree

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Leaving the eMMC card in high-speed mode (e.g., HS200/HS400) while abruptly resetting the host controller to legacy timing causes DLL desynchronization and breaks communication upon resume.
- [High] Missing eMMC Power-Off Notification during shutdown, unbind, and undervoltage when MMC_PM_KEEP_POWER is active.
- [Medium] The static MMC_PM_KEEP_POWER capability unintentionally disables aggressive runtime power management (Runtime PM) for the eMMC.
--

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

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

This commit introduces support for preserving eMMC power during suspend-to-DRAM
on platforms where firmware accesses the card directly during resume. It skips
the sleep and power-off notify sequence if MMC_PM_KEEP_POWER is set.

> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 05444ecf3909f..7cbe655327264 100644
> --- 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)) {

[Severity: Medium]
Since mmc_runtime_suspend() also calls _mmc_suspend() with
MMC_POWEROFF_SUSPEND, does matching the static host->pm_caps here
inadvertently skip the power down sequence during routine runtime idle
suspend, effectively disabling aggressive runtime power management?

> +		if (!mmc_host_is_spi(host))
> +			err = mmc_deselect_cards(host);
> +		if (!err) {
> +			host->pm_flags |= MMC_PM_KEEP_POWER;
> +			mmc_set_clock(host, host->f_init);
> +			mmc_set_initial_state(host);

[Severity: High]
Does this abruptly drop the host controller's clock to f_init and switch
to legacy 1-bit timing while the eMMC card remains fully powered and
stranded in its high-speed state?

Could this cause the card's internal DLL to lose lock, resulting in
communication failure upon resume when mmc_init_card() sends CMD0?

> +			mmc_card_set_suspended(host->card);

[Severity: High]
Since the new MMC_PM_KEEP_POWER fast path marks the card as suspended
without powering it down, will the early exit in _mmc_suspend() mistakenly
skip subsequent power-off notifications?

If a shutdown, unbind, or undervoltage occurs later, it hits this check
in _mmc_suspend():

	if (mmc_card_suspended(host->card))
		goto out;

and skips sending EXT_CSD_POWER_OFF_LONG and powering off the card?

> +		}
> +		goto out;
> +	}
> +

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-29 20:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-29 20:14 [PATCH v5 0/2] mmc: core: Keep the card powered across suspend when firmware needs it live Kamal Dasu
2026-07-29 20:14 ` [PATCH v5 1/2] dt-bindings: mmc: Extend keep-power-in-suspend beyond SDIO Kamal Dasu
2026-07-29 20:23   ` sashiko-bot
2026-07-29 20:14 ` [PATCH v5 2/2] mmc: core: Honor keep-power-in-suspend for (e)MMC in suspend/resume Kamal Dasu
2026-07-29 20:29   ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.