Re: [PATCH v10 0/6] mmc: core: Keep the card powered across suspend when firmware needs it live

[Ulf Hansson]

On Thu, Oct 29, 2026 at 8:14 PM Kamal Dasu <kamal.dasu@xxxxxxxxxxxx> wrote:
>
> On Wed, Oct 28, 2026 at 5:35 AM Ulf Hansson
> <ulf.hansson@xxxxxxxxxxxxxxxx> wrote:
> >
> > On Tue, Oct 27, 2026 at 8:53 PM Kamal Dasu <kamal.dasu@xxxxxxxxxxxx> wrote:
> > >
> > > On Tue, Oct 27, 2026 at 10:41 AM Ulf Hansson
> > > <ulf.hansson@xxxxxxxxxxxxxxxx> wrote:
> > > >
> > > > 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 v10:
> > > >         - To help out with the implementation I offered my help to try to move
> > > >         this forward.
> > > >         - The DT patches are good to go so I included them from the previous
> > > >         versions.
> > > >         - Patch 3 to 6 are new, but based on the work done by Kamal Dasu.
> > > >         - This is completely untested from my side, please help to test it.
> > > >
> > > > Kind regards
> > > > Ulf Hansson
> > > >
> > > >
> > > > Kamal Dasu (2):
> > > >   dt-bindings: mmc: Extend keep-power-in-suspend beyond SDIO
> > > >   dt-bindings: mmc: Add reset-card-at-resume property
> > > >
> > > > Ulf Hansson (4):
> > > >   mmc: core: Add option to keep an eMMC powered on at system suspend
> > > >   mmc: core: Split sdio_set_host_pm_flags()
> > > >   mmc: core: Move reset functions for eMMC
> > > >   mmc: core: Add support for resetting an eMMC at system resume
> > > >
> > > >  .../bindings/mmc/mmc-controller-common.yaml   | 10 +-
> > > >  drivers/mmc/core/core.c                       |  9 ++
> > > >  drivers/mmc/core/core.h                       |  1 +
> > > >  drivers/mmc/core/host.c                       | 10 ++
> > > >  drivers/mmc/core/mmc.c                        | 95 ++++++++++++-------
> > > >  drivers/mmc/core/sdio_io.c                    |  7 +-
> > > >  include/linux/mmc/host.h                      |  1 +
> > > >  7 files changed, 92 insertions(+), 41 deletions(-)
> > > >
> > > > --
> > > > 2.43.0
> > > >
> > >
> > > Hi Ulf,
> > >
> > > Test results for this series on a Broadcom STB board with the Kioxia
> > > 016G01 eMMC,
> > > which reproduces the problem during S3 suspend/resume with two DT variables:
> > >
> > >   keep-power-in-suspend  reset-card-at-resume  Result
> > >                             no                     no
> > >   enters suspend, board resets
> > >
> > >       at resume
> > >                             yes                    no
> > >   enters suspend, board resets
> > >
> > >        at resume
> > >                             no                     yes
> > >   suspend aborts, -EINVAL
> > >                             yes                    yes
> > >   s3 suspend/resume works
> > >
> > > Changes applied to Kernel: v7.3 + 7.4 merge window (linus/master
> > > d135ab453a74) with this
> > > series, plus the one-line fix I'm sending in reply to patch 6/6
> > > (details in my reply to that patch):
> > > mmc_resume() calls _mmc_hw_reset() without claiming the host, which
> > > triggers WARN_ON(!host->claimed) for every command sent during the
> > > reset. Claiming the host around the reset removes the warnings..
> > >
> > > The -EINVAL in the third row comes from mmc_host_class_prepare(), since
> > > MMC_PM_KEEP_POWER is not in pm_caps without keep-power-in-suspend.
> >
> > Yes, that was actually intentional as we can't allow the system to
> > suspend in this configuration, as we may not wake up correctly, right?
> >
> > Although, perhaps we should also log a warning earlier during the
> > probe of the host, when we are parsing the DT properties for it. To
> > upfront say something isn't correct.
> >
> > >
> > > In the working case I also checked the md5sum of the first 5 MiB of
> > > /dev/mmcblk1 before and after suspend, and it is unchanged.
> > >
> > > With the fix applied:
> > > Tested-by: Kamal Dasu <kamal.dasu@xxxxxxxxxxxx>
> >
> > Thanks and thanks for the fix on top!
> >
>
> Hi Uffe,
>
> While testing I noticed an error message printed on every resume when
> both properties are set:
>
>   mmc1: cache flush error -110
>
> It comes from the _mmc_flush_cache() call at the start of
> _mmc_hw_reset(). That call was added by Adrian in 437db4c6e798 ("mmc:
> mmc: Attempt to flush cache before reset") because a reset may
> invalidate the cache, and it is now also reached from mmc_resume()
> through patch 6. At that point the card has just been accessed by
> firmware, so the flush times out. Resume and data are fine, it is only
> the message.
>
> The cache has already been flushed in _mmc_suspend() before the card is
> kept powered, and the queues are quiesced by then, so the flush at
> resume looks redundant. But I don't want to drop a safeguard without
> knowing whether it was meant to cover this path. Do you think it is
> fine to skip the flush when resetting at system resume?

Yes, the cache flush at resume seems redundant.

>
> I tried that, by moving the flush from _mmc_hw_reset() into the
> ->hw_reset() bus op and calling _mmc_hw_reset() directly from
> mmc_resume(). That removes the message. I tested it on the same board,
> booted from the eMMC.
>
> Would you like this as a patch in the series, or should I leave it as
> it is?

Please make the change as part of the series.

I suggest we extend the preparation patch (patch5), from "mmc: core:
Move reset functions for eMMC" to "mmc: core: Move and split reset
functions for eMMC"

Kind regards
Uffe