mmc: core: Keep the card powered across suspend when firmware needs it live
Subsystems (2)
v3 cover mmc: core: Keep the card powered across suspend when firmware needs it live
This is v3, following Ulf's review of v2:
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 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: Document no-mmc-poweroff-suspend property
mmc: core: Add MMC_CAP2_NO_POWEROFF_SUSPEND host capability
.../bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++ drivers/mmc/core/host.c | 2 ++ drivers/mmc/core/mmc.c | 15 +++++++++++++++ include/linux/mmc/host.h | 1 + 4 files changed, 28 insertions(+)
Signature · 1 line
2.34.1
PATCH v3 1/2 dt-bindings: mmc: Document no-mmc-poweroff-suspend property
On some platforms, firmware accesses the (e)MMC card directly during system resume from Suspend-to-DRAM, before the kernel's own resume path has run, in order to load boot code using hard wired logic that is not field updatable. This requires the card to remain powered and responsive throughout suspend: it cannot be put to sleep, sent a power-off notification, or have its supply removed.
Add a no-mmc-poweroff-suspend flag property so affected boards can tell the mmc core to keep the card powered across suspend instead of sleeping or powering it off.
---
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.
.../devicetree/bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+)
Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +10 -0
@@ -232,6 +232,16 @@ properties: description: All eMMC HS400 modes are not supported. + no-mmc-poweroff-suspend: + $ref: /schemas/types.yaml#/definitions/flag + description: + On this platform, firmware accesses the attached (e)MMC card + directly during system resume, before the kernel's own resume + path runs. The card must therefore stay powered throughout + suspend, and the controller must not send it a sleep or + power-off notification. The kernel re-initializes the card + after resume, since firmware may have already reset it. + dsr: description: Value the card Driver Stage Register (DSR) should be programmed
Signature · 1 line
2.34.1
Re: [PATCH v3 1/2] dt-bindings: mmc: Document no-mmc-poweroff-suspend property
Quoted 29 lines · Kamal Dasu
On some platforms, firmware accesses the (e)MMC card directly during system resume from Suspend-to-DRAM, before the kernel's own resume path has run, in order to load boot code using hard wired logic that is not field updatable. This requires the card to remain powered and responsive throughout suspend: it cannot be put to sleep, sent a power-off notification, or have its supply removed.
Add a no-mmc-poweroff-suspend flag property so affected boards can tell the mmc core to keep the card powered across suspend instead of sleeping or powering it off.
Signed-off-by Kamal Dasu <[email protected]>---
Changes in v3:
- Renamed from no-mmc-sleep; droppedS_A_TIMEOUTframing per Ulf.Changes in v2:
- New patch, replacing v1's card-level quirk, per Ulf..../devicetree/bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+)Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +0
-0@@ -232,6 +232,16 @@ properties: description: All eMMC HS400 modes are not supported.
Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml properties: · line 243
Quoted 10 lines
Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +9
-0+ no-mmc-poweroff-suspend: + $ref: /schemas/types.yaml#/definitions/flag + description: + On this platform, firmware accesses the attached (e)MMC card + directly during system resume, before the kernel's own resume + path runs. The card must therefore stay powered throughout + suspend, and the controller must not send it a sleep or + power-off notification. The kernel re-initializes the card + after resume, since firmware may have already reset it.
Isn't this exactly the same as keep-power-in-suspend?
Best regards,
Krzysztof
Re: [PATCH v3 1/2] dt-bindings: mmc: Document no-mmc-poweroff-suspend property
Quoted 33 lines · Krzysztof Kozlowski
Quoted 16 lines · Kamal Dasu
On some platforms, firmware accesses the (e)MMC card directly during system resume from Suspend-to-DRAM, before the kernel's own resume path has run, in order to load boot code using hard wired logic that is not field updatable. This requires the card to remain powered and responsive throughout suspend: it cannot be put to sleep, sent a power-off notification, or have its supply removed.
Add a no-mmc-poweroff-suspend flag property so affected boards can tell the mmc core to keep the card powered across suspend instead of sleeping or powering it off.
Signed-off-by Kamal Dasu <[email protected]>---
Changes in v3:
- Renamed from no-mmc-sleep; droppedS_A_TIMEOUTframing per Ulf.Quoted 7 lines
Changes in v2:
- New patch, replacing v1's card-level quirk, per Ulf..../devicetree/bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+)diff --git
a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yamlb/Documentation/devicetree/bindings/mmc/mmc-controller-common.yamlQuoted 7 lines
index
3d7195e9461c..5f7b3f8ef1dd100644Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +0
-0@@ -232,6 +232,16 @@ properties: description: All eMMC HS400 modes are not supported.
Quoted 13 lines
Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml properties: · line 243
Quoted 9 lines
Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +9
-0+ no-mmc-poweroff-suspend: + $ref: /schemas/types.yaml#/definitions/flag + description: + On this platform, firmware accesses the attached (e)MMC card + directly during system resume, before the kernel's own resume + path runs. The card must therefore stay powered throughout + suspend, and the controller must not send it a sleep or + power-off notification. The kernel re-initializes the card + after resume, since firmware may have already reset it.Isn't this exactly the same as keep-power-in-suspend?
Not quite, though I see why it reads that way from the description alone.
keep-power-in-suspend only takes effect through host->pm_flags, which gets set at runtime via sdio_set_host_pm_flags() -- an SDIO function driver opts in from its own suspend() callback, per suspend cycle. The consuming logic lives entirely in mmc_sdio_suspend()/ mmc_sdio_resume() (drivers/mmc/core/sdio.c); pm_flags is never read in mmc.c, which is what this series touches.
no-mmc-poweroff-suspend differs in two ways:
- It's a fixed platform characteristic, not a per-driver runtime request. There's no SDIO-style function driver to call an opt-in API from for a plain (e)MMC storage card -- the requirement here (firmware always touches the card at resume, on this board) is always true, not something decided per suspend cycle.
- The resume-side behavior differs. keep-power-in-suspend without a wake IRQ does nothing at resume: it assumes the card's state survived untouched. This flag needs the opposite -- keep power, but still fully re-initialize the card via mmc_init_card(), since firmware may have reconfigured it while the kernel was suspended.
Given those differences I went with a host capability following the existing no-sdio/no-sd/no-mmc/no-mmc-hs400 pattern in mmc_of_parse() rather than extending pm_flags/keep-power-in-suspend, but happy to hear if you see a cleaner way to fold this in.
Kamal
Quoted 5 lines
Best regards,
Krzysztof
Re: [PATCH v3 1/2] dt-bindings: mmc: Document no-mmc-poweroff-suspend property
Quoted 60 lines · Kamal Dasu
Quoted 33 lines · Krzysztof Kozlowski
Quoted 16 lines · Kamal Dasu
On some platforms, firmware accesses the (e)MMC card directly during system resume from Suspend-to-DRAM, before the kernel's own resume path has run, in order to load boot code using hard wired logic that is not field updatable. This requires the card to remain powered and responsive throughout suspend: it cannot be put to sleep, sent a power-off notification, or have its supply removed.
Add a no-mmc-poweroff-suspend flag property so affected boards can tell the mmc core to keep the card powered across suspend instead of sleeping or powering it off.
Signed-off-by Kamal Dasu <[email protected]>---
Changes in v3:
- Renamed from no-mmc-sleep; droppedS_A_TIMEOUTframing per Ulf.Quoted 7 lines
Changes in v2:
- New patch, replacing v1's card-level quirk, per Ulf..../devicetree/bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+)diff --git
a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yamlb/Documentation/devicetree/bindings/mmc/mmc-controller-common.yamlQuoted 7 lines
index
3d7195e9461c..5f7b3f8ef1dd100644Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +0
-0@@ -232,6 +232,16 @@ properties: description: All eMMC HS400 modes are not supported.Quoted 13 lines
Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml properties: · line 243
Quoted 9 lines
Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +9
-0+ no-mmc-poweroff-suspend: + $ref: /schemas/types.yaml#/definitions/flag + description: + On this platform, firmware accesses the attached (e)MMC card + directly during system resume, before the kernel's own resume + path runs. The card must therefore stay powered throughout + suspend, and the controller must not send it a sleep or + power-off notification. The kernel re-initializes the card + after resume, since firmware may have already reset it.Isn't this exactly the same as keep-power-in-suspend?
Not quite, though I see why it reads that way from the description alone.
keep-power-in-suspend only takes effect through
host->pm_flags, which gets set at runtime viasdio_set_host_pm_flags()-- an SDIO function driver opts in from its ownsuspend()callback, per suspend cycle. The consuming logic lives entirely in mmc_sdio_suspend()/mmc_sdio_resume()(drivers/mmc/core/sdio.c);pm_flagsis never read inmmc.c, which is what this series touches.no-mmc-poweroff-suspend differs in two ways:
Quoted 10 lines
- It's a fixed platform characteristic, not a per-driver runtime request. There's no SDIO-style function driver to call an opt-in API from for a plain (e)MMC storage card -- the requirement here (firmware always touches the card at resume, on this board) is always true, not something decided per suspend cycle.
- The resume-side behavior differs. keep-power-in-suspend without a wake IRQ does nothing at resume: it assumes the card's state survived untouched. This flag needs the opposite -- keep power, but still fully re-initialize the card viammc_init_card(), since firmware may have reconfigured it while the kernel was suspended.
All this is just driver stuff which I did not ask about.
The property is described pretty the same - do not power off during suspend/resume.
Best regards,
Krzysztof
Re: [PATCH v3 1/2] dt-bindings: mmc: Document no-mmc-poweroff-suspend property
Quoted 73 lines · Krzysztof Kozlowski
Quoted 60 lines · Kamal Dasu
Quoted 33 lines · Krzysztof Kozlowski
Quoted 16 lines · Kamal Dasu
On some platforms, firmware accesses the (e)MMC card directly during system resume from Suspend-to-DRAM, before the kernel's own resume path has run, in order to load boot code using hard wired logic that is not field updatable. This requires the card to remain powered and responsive throughout suspend: it cannot be put to sleep, sent a power-off notification, or have its supply removed.
Add a no-mmc-poweroff-suspend flag property so affected boards can tell the mmc core to keep the card powered across suspend instead of sleeping or powering it off.
Signed-off-by Kamal Dasu <[email protected]>---
Changes in v3:
- Renamed from no-mmc-sleep; droppedS_A_TIMEOUTframing per Ulf.Quoted 7 lines
Changes in v2:
- New patch, replacing v1's card-level quirk, per Ulf..../devicetree/bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+)diff --git
a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yamlb/Documentation/devicetree/bindings/mmc/mmc-controller-common.yamlQuoted 7 lines
index
3d7195e9461c..5f7b3f8ef1dd100644Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +0
-0@@ -232,6 +232,16 @@ properties: description: All eMMC HS400 modes are not supported.Quoted 13 lines
Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml properties: · line 243
Quoted 9 lines
Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +9
-0+ no-mmc-poweroff-suspend: + $ref: /schemas/types.yaml#/definitions/flag + description: + On this platform, firmware accesses the attached (e)MMC card + directly during system resume, before the kernel's own resume + path runs. The card must therefore stay powered throughout + suspend, and the controller must not send it a sleep or + power-off notification. The kernel re-initializes the card + after resume, since firmware may have already reset it.Isn't this exactly the same as keep-power-in-suspend?
Not quite, though I see why it reads that way from the description alone.
keep-power-in-suspend only takes effect through
host->pm_flags, which gets set at runtime viasdio_set_host_pm_flags()-- an SDIO function driver opts in from its ownsuspend()callback, per suspend cycle. The consuming logic lives entirely in mmc_sdio_suspend()/mmc_sdio_resume()(drivers/mmc/core/sdio.c);pm_flagsis never read inmmc.c, which is what this series touches.no-mmc-poweroff-suspend differs in two ways:
Quoted 10 lines
- It's a fixed platform characteristic, not a per-driver runtime request. There's no SDIO-style function driver to call an opt-in API from for a plain (e)MMC storage card -- the requirement here (firmware always touches the card at resume, on this board) is always true, not something decided per suspend cycle.
- The resume-side behavior differs. keep-power-in-suspend without a wake IRQ does nothing at resume: it assumes the card's state survived untouched. This flag needs the opposite -- keep power, but still fully re-initialize the card viammc_init_card(), since firmware may have reconfigured it while the kernel was suspended.
Quoted 5 lines
All this is just driver stuff which I did not ask about.
The property is described pretty the same - do not power off during suspend/resume.
Do you want me to fold this change into using 'keep-power-in-suspend' for mmc as well ?.
Quoted 5 lines
Best regards,
Krzysztof
Re: [PATCH v3 1/2] dt-bindings: mmc: Document no-mmc-poweroff-suspend property
Quoted 74 lines · Kamal Dasu
Quoted 73 lines · Krzysztof Kozlowski
Quoted 70 lines · Kamal Dasu
Quoted 33 lines · Krzysztof Kozlowski
Quoted 16 lines · Kamal Dasu
On some platforms, firmware accesses the (e)MMC card directly during system resume from Suspend-to-DRAM, before the kernel's own resume path has run, in order to load boot code using hard wired logic that is not field updatable. This requires the card to remain powered and responsive throughout suspend: it cannot be put to sleep, sent a power-off notification, or have its supply removed.
Add a no-mmc-poweroff-suspend flag property so affected boards can tell the mmc core to keep the card powered across suspend instead of sleeping or powering it off.
Signed-off-by Kamal Dasu <[email protected]>---
Changes in v3:
- Renamed from no-mmc-sleep; droppedS_A_TIMEOUTframing per Ulf.Quoted 7 lines
Changes in v2:
- New patch, replacing v1's card-level quirk, per Ulf..../devicetree/bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+)diff --git
a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yamlb/Documentation/devicetree/bindings/mmc/mmc-controller-common.yamlQuoted 7 lines
index
3d7195e9461c..5f7b3f8ef1dd100644Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +0
-0@@ -232,6 +232,16 @@ properties: description: All eMMC HS400 modes are not supported.Quoted 13 lines
Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml properties: · line 243
Quoted 9 lines
Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +9
-0+ no-mmc-poweroff-suspend: + $ref: /schemas/types.yaml#/definitions/flag + description: + On this platform, firmware accesses the attached (e)MMC card + directly during system resume, before the kernel's own resume + path runs. The card must therefore stay powered throughout + suspend, and the controller must not send it a sleep or + power-off notification. The kernel re-initializes the card + after resume, since firmware may have already reset it.Isn't this exactly the same as keep-power-in-suspend?
Not quite, though I see why it reads that way from the description alone.
keep-power-in-suspend only takes effect through
host->pm_flags, which gets set at runtime viasdio_set_host_pm_flags()-- an SDIO function driver opts in from its ownsuspend()callback, per suspend cycle. The consuming logic lives entirely in mmc_sdio_suspend()/mmc_sdio_resume()(drivers/mmc/core/sdio.c);pm_flagsis never read inmmc.c, which is what this series touches.no-mmc-poweroff-suspend differs in two ways:
- It's a fixed platform characteristic, not a per-driver runtime request. There's no SDIO-style function driver to call an opt-in API from for a plain (e)MMC storage card -- the requirement here (firmware always touches the card at resume, on this board) is always true, not something decided per suspend cycle.
- The resume-side behavior differs. keep-power-in-suspend without a wake IRQ does nothing at resume: it assumes the card's state survived untouched. This flag needs the opposite -- keep power, but still fully re-initialize the card viammc_init_card(), since firmware may have reconfigured it while the kernel was suspended.
Quoted 8 lines
Quoted 5 lines
All this is just driver stuff which I did not ask about.
The property is described pretty the same - do not power off during suspend/resume.
Do you want me to fold this change into using 'keep-power-in-suspend' for mmc as well ?.
I don't want a new property if it has exactly the same meaning. Of course I am happy to hear why the property has different meaning in terms of hardware description. We do not talk here about drivers, obviously.
Best regards,
Krzysztof
Re: [PATCH v3 1/2] dt-bindings: mmc: Document no-mmc-poweroff-suspend property
Quoted 82 lines · Krzysztof Kozlowski
Quoted 80 lines · Kamal Dasu
Quoted 78 lines · Krzysztof Kozlowski
Quoted 60 lines · Kamal Dasu
Quoted 33 lines · Krzysztof Kozlowski
Quoted 16 lines · Kamal Dasu
On some platforms, firmware accesses the (e)MMC card directly during system resume from Suspend-to-DRAM, before the kernel's own resume path has run, in order to load boot code using hard wired logic that is not field updatable. This requires the card to remain powered and responsive throughout suspend: it cannot be put to sleep, sent a power-off notification, or have its supply removed.
Add a no-mmc-poweroff-suspend flag property so affected boards can tell the mmc core to keep the card powered across suspend instead of sleeping or powering it off.
Signed-off-by Kamal Dasu <[email protected]>---
Changes in v3:
- Renamed from no-mmc-sleep; droppedS_A_TIMEOUTframing per Ulf.Quoted 7 lines
Changes in v2:
- New patch, replacing v1's card-level quirk, per Ulf..../devicetree/bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+)diff --git
a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yamlb/Documentation/devicetree/bindings/mmc/mmc-controller-common.yamlQuoted 7 lines
index
3d7195e9461c..5f7b3f8ef1dd100644Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +0
-0@@ -232,6 +232,16 @@ properties: description: All eMMC HS400 modes are not supported.Quoted 13 lines
Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml properties: · line 243
Quoted 9 lines
Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +9
-0+ no-mmc-poweroff-suspend: + $ref: /schemas/types.yaml#/definitions/flag + description: + On this platform, firmware accesses the attached (e)MMC card + directly during system resume, before the kernel's own resume + path runs. The card must therefore stay powered throughout + suspend, and the controller must not send it a sleep or + power-off notification. The kernel re-initializes the card + after resume, since firmware may have already reset it.Isn't this exactly the same as keep-power-in-suspend?
Not quite, though I see why it reads that way from the description alone.
keep-power-in-suspend only takes effect through
host->pm_flags, which gets set at runtime viasdio_set_host_pm_flags()-- an SDIO function driver opts in from its ownsuspend()callback, per suspend cycle. The consuming logic lives entirely in mmc_sdio_suspend()/mmc_sdio_resume()(drivers/mmc/core/sdio.c);pm_flagsis never read inmmc.c, which is what this series touches.no-mmc-poweroff-suspend differs in two ways:
Quoted 10 lines
- It's a fixed platform characteristic, not a per-driver runtime request. There's no SDIO-style function driver to call an opt-in API from for a plain (e)MMC storage card -- the requirement here (firmware always touches the card at resume, on this board) is always true, not something decided per suspend cycle.
- The resume-side behavior differs. keep-power-in-suspend without a wake IRQ does nothing at resume: it assumes the card's state survived untouched. This flag needs the opposite -- keep power, but still fully re-initialize the card viammc_init_card(), since firmware may have reconfigured it while the kernel was suspended.All this is just driver stuff which I did not ask about.
The property is described pretty the same - do not power off during suspend/resume.
Quoted 7 lines
Quoted 2 lines
Do you want me to fold this change into using 'keep-power-in-suspend' for mmc as well ?.
I don't want a new property if it has exactly the same meaning. Of course I am happy to hear why the property has different meaning in terms of hardware description. We do not talk here about drivers, obviously.
In terms of hardware description, yes, it's the same meaning: don't power off the card across suspend/resume. I've folded this into keep-power-in-suspend rather than keeping a separate property -- extended its description to cover (e)MMC alongside the existing SDIO wording, and reworked the mmc core side to honor it for (e)MMC too.
Sending v4 shortly.
Quoted 2 lines
Best regards,
Krzysztof
Re: [PATCH v3 1/2] dt-bindings: mmc: Document no-mmc-poweroff-suspend property
Quoted 92 lines · Kamal Dasu
Quoted 82 lines · Krzysztof Kozlowski
Quoted 80 lines · Kamal Dasu
Quoted 78 lines · Krzysztof Kozlowski
Quoted 60 lines · Kamal Dasu
Quoted 33 lines · Krzysztof Kozlowski
Quoted 16 lines · Kamal Dasu
On some platforms, firmware accesses the (e)MMC card directly during system resume from Suspend-to-DRAM, before the kernel's own resume path has run, in order to load boot code using hard wired logic that is not field updatable. This requires the card to remain powered and responsive throughout suspend: it cannot be put to sleep, sent a power-off notification, or have its supply removed.
Add a no-mmc-poweroff-suspend flag property so affected boards can tell the mmc core to keep the card powered across suspend instead of sleeping or powering it off.
Signed-off-by Kamal Dasu <[email protected]>---
Changes in v3:
- Renamed from no-mmc-sleep; droppedS_A_TIMEOUTframing per Ulf.Quoted 7 lines
Changes in v2:
- New patch, replacing v1's card-level quirk, per Ulf..../devicetree/bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+)diff --git
a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yamlb/Documentation/devicetree/bindings/mmc/mmc-controller-common.yamlQuoted 7 lines
index
3d7195e9461c..5f7b3f8ef1dd100644Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +0
-0@@ -232,6 +232,16 @@ properties: description: All eMMC HS400 modes are not supported.Quoted 13 lines
Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml properties: · line 243
Quoted 9 lines
Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml +9
-0+ no-mmc-poweroff-suspend: + $ref: /schemas/types.yaml#/definitions/flag + description: + On this platform, firmware accesses the attached (e)MMC card + directly during system resume, before the kernel's own resume + path runs. The card must therefore stay powered throughout + suspend, and the controller must not send it a sleep or + power-off notification. The kernel re-initializes the card + after resume, since firmware may have already reset it.Isn't this exactly the same as keep-power-in-suspend?
Not quite, though I see why it reads that way from the description alone.
keep-power-in-suspend only takes effect through
host->pm_flags, which gets set at runtime viasdio_set_host_pm_flags()-- an SDIO function driver opts in from its ownsuspend()callback, per suspend cycle. The consuming logic lives entirely in mmc_sdio_suspend()/mmc_sdio_resume()(drivers/mmc/core/sdio.c);pm_flagsis never read inmmc.c, which is what this series touches.no-mmc-poweroff-suspend differs in two ways:
Quoted 10 lines
- It's a fixed platform characteristic, not a per-driver runtime request. There's no SDIO-style function driver to call an opt-in API from for a plain (e)MMC storage card -- the requirement here (firmware always touches the card at resume, on this board) is always true, not something decided per suspend cycle.
- The resume-side behavior differs. keep-power-in-suspend without a wake IRQ does nothing at resume: it assumes the card's state survived untouched. This flag needs the opposite -- keep power, but still fully re-initialize the card viammc_init_card(), since firmware may have reconfigured it while the kernel was suspended.All this is just driver stuff which I did not ask about.
The property is described pretty the same - do not power off during suspend/resume.
Quoted 7 lines
Quoted 2 lines
Do you want me to fold this change into using 'keep-power-in-suspend' for mmc as well ?.
I don't want a new property if it has exactly the same meaning. Of course I am happy to hear why the property has different meaning in terms of hardware description. We do not talk here about drivers, obviously.
Quoted 7 lines
In terms of hardware description, yes, it's the same meaning: don't power off the card across suspend/resume. I've folded this into keep-power-in-suspend rather than keeping a separate property -- extended its description to cover (e)MMC alongside the existing SDIO wording, and reworked the mmc core side to honor it for (e)MMC too.
Sending v4 shortly.
Hold on, this isn't what is needed from the HW point of view. At least, according to what Florian told me from the earlier discussions.
What we need is a property that means: "Keep power in suspend/resume, but also re-initialize the eMMC/card at resume since the FW have fiddled with the card before the kernel resumes".
Kind regards
Uffe
PATCH v3 2/2 mmc: core: Add MMC_CAP2_NO_POWEROFF_SUSPEND host capability
In reply to Kamal DasuOn 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.
Add MMC_CAP2_NO_POWEROFF_SUSPEND, settable via the new "no-mmc-poweroff-suspend" DT property. In _mmc_suspend(), when this capability is set, skip the poweroff-notify/sleep/power-off sequence
entirely: just deselect the card and mark it suspended. At resume, _mmc_resume() already unconditionally calls mmc_power_up() and mmc_init_card(), so the card gets fully re-initialized regardless of what firmware may have done to it in the meantime.
---
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 | 15 +++++++++++++++ include/linux/mmc/host.h | 1 + 3 files changed, 18 insertions(+)
drivers/mmc/core/host.c +2 -0
@@ -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,"no-mmc-poweroff-suspend" )) + host->caps2 |=MMC_CAP2_NO_POWEROFF_SUSPEND ;/* Must be after "non-removable" check */ if (device_property_read_u32 (dev,"fixed-emmc-driver-type" , &drv_type) ==0 ) {
drivers/mmc/core/mmc.c +15 -0
@@ -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 ) { +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);
include/linux/mmc/host.h +1 -0
@@ -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_NO_POWEROFF_SUSPEND (1 << 29)/* Card must stay powered across suspend */ bool uhs2_sd_tran;/* UHS-II flag for SD_TRAN state */ bool uhs2_app_cmd;/* UHS-II flag for APP command */
Signature · 1 line
2.34.1
[email protected]
AI reviewRe: [PATCH v3 2/2] mmc: core: Add MMC_CAP2_NO_POWEROFF_SUSPEND host capability
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…
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 <[email protected]>
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().
[ ... ]
Quoted 6 lines
drivers/mmc/core/mmc.c +0
-0@@ -2157,6 +2157,21 @@ static int _mmc_suspend(struct mmc_host *host, enum mmc_poweroff_type pm_type) goto out;
drivers/mmc/core/mmc.c static int _mmc_suspend(struct mmc_host *host, enum mmc_poweroff_type pm_type) · line 2167
Quoted 10 lines
drivers/mmc/core/mmc.c +8
-0} +/* +* 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?
Quoted 10 lines
drivers/mmc/core/mmc.c +7
-0+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);
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
Re: [PATCH v3 2/2] mmc: core: Add MMC_CAP2_NO_POWEROFF_SUSPEND host capability
In reply to Kamal DasuQuoted 8 lines · Kamal Dasu
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.
A lot better description, thanks for the update!
Quoted 7 lines
Add
MMC_CAP2_NO_POWEROFF_SUSPEND, settable via the new "no-mmc-poweroff-suspend" DT property. In_mmc_suspend(), when this capability is set, skip the poweroff-notify/sleep/power-off sequence
entirely: just deselect the card and mark it suspended. At resume,_mmc_resume()already unconditionally callsmmc_power_up()andmmc_init_card(), so the card gets fully re-initialized regardless of what firmware may have done to it in the meantime.
That's not entirely correct as mmc_power_up() will in this case bail out early, as "host->ios.power_mode == MMC_POWER_ON".
That said, we probably need to call mmc_power_off() in _mmc_resume() before we start the re-initialization of the card, or what do you think?
Quoted 4 lines
Reported-by Florian Fainelli <[email protected]>Signed-off-by Kamal Dasu <[email protected]>
Kind regards
Uffe
Quoted 70 lines
---
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-levelMMC_QUIRK_BROKEN_SLEEPquirk with a host capability, per Ulf.
- Added Reported-by/Closes crediting Florian.drivers/mmc/core/host.c | 2 ++ drivers/mmc/core/mmc.c | 15 +++++++++++++++ include/linux/mmc/host.h | 1 + 3 files changed, 18 insertions(+)drivers/mmc/core/host.c +2
-0@@ -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,"no-mmc-poweroff-suspend" )) + host->caps2 |=MMC_CAP2_NO_POWEROFF_SUSPEND ;/* Must be after "non-removable" check */ if (device_property_read_u32 (dev,"fixed-emmc-driver-type" , &drv_type) ==0 ) {drivers/mmc/core/mmc.c +15
-0@@ -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 ) { +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);include/linux/mmc/host.h +1
-1@@ -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_NO_POWEROFF_SUSPEND (1 << 29)/* Card must stay powered across suspend */ bool uhs2_sd_tran;/* UHS-II flag for SD_TRAN state */ bool uhs2_app_cmd;/* UHS-II flag for APP command */ --2.34.1