Re: [PATCH AUTOSEL 6.19-6.1] mmc: rtsx: reset power state on suspend

[Ulf Hansson]

On Thu, 19 Feb 2026 at 03:04, Sasha Levin <sashal@xxxxxxxxxx> wrote:
>
> From: Matthew Schwartz <matthew.schwartz@xxxxxxxxx>
>
> [ Upstream commit eac85fbd0867c25ac517f58fae401d65c627edff ]
>
> When rtsx_pci suspends, the card reader hardware powers off but the sdmmc
> driver's prev_power_state remains as MMC_POWER_ON. This causes sd_power_on
> to skip reinitialization on the next I/O request, leading to DMA transfer
> timeouts and errors on resume 20% of the time.
>
> Add a power_off slot callback so the PCR can notify the sdmmc driver
> during suspend. The sdmmc driver resets prev_power_state, and sd_request
> checks this to reinitialize the card before the next I/O.
>
> Signed-off-by: Matthew Schwartz <matthew.schwartz@xxxxxxxxx>
> Link: https://patch.msgid.link/20260105060236.400366-2-matthew.schwartz@xxxxxxxxx
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

NAK.

This patch is reverted in mainline, as it's not the proper fix.

See commit c23f0550c05d40762b141808709667759291c938

Kind regards
Uffe

> ---
>
> LLM Generated explanations, may be completely bogus:
>
> Good - `sd_power_on` has been part of this file since initial creation.
> The forward declaration added in the patch is just to allow the function
> (defined later in the file) to be called from `sd_request` (defined
> earlier).
>
> ### Analysis Summary
>
> **What problem the commit solves:**
>
> This commit fixes a suspend/resume regression in Realtek PCI-E SD card
> readers where, after suspend, 20% of resume attempts fail with DMA
> transfer timeouts because:
>
> 1. During suspend, `rtsx_pci_power_off()` powers down the card reader
>    hardware
> 2. But the sdmmc driver's `prev_power_state` remains `MMC_POWER_ON`
> 3. On resume, when `sd_power_on()` is called, it sees `prev_power_state
>    == MMC_POWER_ON` at line 912 and returns immediately without
>    reinitializing the hardware
> 4. The first I/O attempt then fails with DMA timeout because the
>    hardware was never re-powered
>
> **The fix has three parts:**
> 1. Adds a `power_off` callback to `struct rtsx_slot` (1 line in header)
> 2. In the PCR suspend paths (both system and runtime), calls
>    `slot->power_off()` to notify the sdmmc driver before powering off
>    hardware
> 3. In the sdmmc driver: implements `rtsx_pci_sdmmc_power_off()` to reset
>    `prev_power_state = MMC_POWER_OFF`, and adds a check in
>    `sd_request()` to call `sd_power_on()` if `prev_power_state ==
>    MMC_POWER_OFF`
>
> **Stable kernel criteria assessment:**
> - **Fixes a real bug:** Yes - 20% failure rate on resume with SD card
>   operations
> - **Obviously correct:** Yes - the logic is straightforward: notify the
>   driver that hardware was powered off so it reinitializes properly
> - **Small and contained:** Yes - 32 lines of insertion across 3 files,
>   all within the rtsx subsystem
> - **No new features:** Correct - this adds only internal plumbing to fix
>   power state tracking
> - **Tested:** Author tested 300+ s2idle cycles
> - **Impact:** Users with Realtek card readers experience I/O errors,
>   filesystem journal failures, and card detection loss after
>   suspend/resume
>
> **Risk assessment:**
> - Low risk - the changes are confined to the rtsx card reader subsystem
> - The `power_off` callback is properly guarded with NULL checks (`if
>   (slot->p_dev && slot->power_off)`)
> - The `sd_request` power-on check has proper error handling
> - The drv_remove function properly clears the callback
> - No impact on other subsystems
>
> **Dependencies:**
> - Self-contained - no dependencies on other commits
> - The companion delay patch (aced969e9bf37) has already been selected
>   for stable and is independent
> - The code being modified (`rtsx_slot`, `rtsx_pci_suspend`,
>   `sd_request`) exists in all recent stable trees
>
> ### Verification
>
> - **Verified** that `sd_power_on()` at line 912 returns early when
>   `prev_power_state == MMC_POWER_ON`, confirming the bug mechanism
> - **Verified** via `git show aced969e9bf37` that the companion patch
>   (delay increase) has a Fixes: tag and was independently picked for
>   stable
> - **Verified** via `git show ed0d7f8559dbd` (stable branch version) that
>   the delay patch was already applied to stable with `[Upstream commit
>   ...]` marker
> - **Verified** via lore.kernel.org that this is a 2-patch series (0/2
>   cover letter), both fixing suspend/resume failures, with testing
>   showing patch 1 reduces failures from 20% to 4%
> - **Verified** that `struct rtsx_slot` currently only has `p_dev` and
>   `card_event` fields by reading `include/linux/rtsx_common.h`
> - **Verified** that the suspend paths properly hold `pcr->pcr_mutex`
>   when calling the callback
> - **Verified** the NULL check guards (`slot->p_dev && slot->power_off`)
>   in both suspend functions
> - **Verified** that `sd_power_on()` has existed since the driver was
>   created (`ff984e57d36e8`) - the forward declaration is just for
>   compilation order
> - **Verified** that `rtsx_pci_sdmmc_drv_remove` properly clears
>   `power_off = NULL` to prevent stale callbacks
>
> This is a clear, well-tested bug fix for a user-impactful suspend/resume
> issue. It's small, contained, properly guarded, and fixes a 20% failure
> rate. It meets all stable kernel criteria.
>
> **YES**
>
>  drivers/misc/cardreader/rtsx_pcr.c |  9 +++++++++
>  drivers/mmc/host/rtsx_pci_sdmmc.c  | 22 ++++++++++++++++++++++
>  include/linux/rtsx_common.h        |  1 +
>  3 files changed, 32 insertions(+)
>
> diff --git a/drivers/misc/cardreader/rtsx_pcr.c b/drivers/misc/cardreader/rtsx_pcr.c
> index f9952d76d6ed7..f1f4d8ed544d6 100644
> --- a/drivers/misc/cardreader/rtsx_pcr.c
> +++ b/drivers/misc/cardreader/rtsx_pcr.c
> @@ -1654,6 +1654,7 @@ static int __maybe_unused rtsx_pci_suspend(struct device *dev_d)
>         struct pci_dev *pcidev = to_pci_dev(dev_d);
>         struct pcr_handle *handle = pci_get_drvdata(pcidev);
>         struct rtsx_pcr *pcr = handle->pcr;
> +       struct rtsx_slot *slot = &pcr->slots[RTSX_SD_CARD];
>
>         dev_dbg(&(pcidev->dev), "--> %s\n", __func__);
>
> @@ -1661,6 +1662,9 @@ static int __maybe_unused rtsx_pci_suspend(struct device *dev_d)
>
>         mutex_lock(&pcr->pcr_mutex);
>
> +       if (slot->p_dev && slot->power_off)
> +               slot->power_off(slot->p_dev);
> +
>         rtsx_pci_power_off(pcr, HOST_ENTER_S3, false);
>
>         mutex_unlock(&pcr->pcr_mutex);
> @@ -1772,12 +1776,17 @@ static int rtsx_pci_runtime_suspend(struct device *device)
>         struct pci_dev *pcidev = to_pci_dev(device);
>         struct pcr_handle *handle = pci_get_drvdata(pcidev);
>         struct rtsx_pcr *pcr = handle->pcr;
> +       struct rtsx_slot *slot = &pcr->slots[RTSX_SD_CARD];
>
>         dev_dbg(device, "--> %s\n", __func__);
>
>         cancel_delayed_work_sync(&pcr->carddet_work);
>
>         mutex_lock(&pcr->pcr_mutex);
> +
> +       if (slot->p_dev && slot->power_off)
> +               slot->power_off(slot->p_dev);
> +
>         rtsx_pci_power_off(pcr, HOST_ENTER_S3, true);
>
>         mutex_unlock(&pcr->pcr_mutex);
> diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
> index 8df60000b5b41..34343b5d5823d 100644
> --- a/drivers/mmc/host/rtsx_pci_sdmmc.c
> +++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
> @@ -47,6 +47,7 @@ struct realtek_pci_sdmmc {
>  };
>
>  static int sdmmc_init_sd_express(struct mmc_host *mmc, struct mmc_ios *ios);
> +static int sd_power_on(struct realtek_pci_sdmmc *host, unsigned char power_mode);
>
>  static inline struct device *sdmmc_dev(struct realtek_pci_sdmmc *host)
>  {
> @@ -821,6 +822,15 @@ static void sd_request(struct work_struct *work)
>
>         rtsx_pci_start_run(pcr);
>
> +       if (host->prev_power_state == MMC_POWER_OFF) {
> +               err = sd_power_on(host, MMC_POWER_ON);
> +               if (err) {
> +                       cmd->error = err;
> +                       mutex_unlock(&pcr->pcr_mutex);
> +                       goto finish;
> +               }
> +       }
> +
>         rtsx_pci_switch_clock(pcr, host->clock, host->ssc_depth,
>                         host->initial_mode, host->double_clk, host->vpclk);
>         rtsx_pci_write_register(pcr, CARD_SELECT, 0x07, SD_MOD_SEL);
> @@ -1522,6 +1532,16 @@ static void rtsx_pci_sdmmc_card_event(struct platform_device *pdev)
>         mmc_detect_change(host->mmc, 0);
>  }
>
> +static void rtsx_pci_sdmmc_power_off(struct platform_device *pdev)
> +{
> +       struct realtek_pci_sdmmc *host = platform_get_drvdata(pdev);
> +
> +       if (!host)
> +               return;
> +
> +       host->prev_power_state = MMC_POWER_OFF;
> +}
> +
>  static int rtsx_pci_sdmmc_drv_probe(struct platform_device *pdev)
>  {
>         struct mmc_host *mmc;
> @@ -1554,6 +1574,7 @@ static int rtsx_pci_sdmmc_drv_probe(struct platform_device *pdev)
>         platform_set_drvdata(pdev, host);
>         pcr->slots[RTSX_SD_CARD].p_dev = pdev;
>         pcr->slots[RTSX_SD_CARD].card_event = rtsx_pci_sdmmc_card_event;
> +       pcr->slots[RTSX_SD_CARD].power_off = rtsx_pci_sdmmc_power_off;
>
>         mutex_init(&host->host_mutex);
>
> @@ -1585,6 +1606,7 @@ static void rtsx_pci_sdmmc_drv_remove(struct platform_device *pdev)
>         pcr = host->pcr;
>         pcr->slots[RTSX_SD_CARD].p_dev = NULL;
>         pcr->slots[RTSX_SD_CARD].card_event = NULL;
> +       pcr->slots[RTSX_SD_CARD].power_off = NULL;
>         mmc = host->mmc;
>
>         cancel_work_sync(&host->work);
> diff --git a/include/linux/rtsx_common.h b/include/linux/rtsx_common.h
> index da9c8c6b5d50f..f294f478f0c0e 100644
> --- a/include/linux/rtsx_common.h
> +++ b/include/linux/rtsx_common.h
> @@ -32,6 +32,7 @@ struct platform_device;
>  struct rtsx_slot {
>         struct platform_device  *p_dev;
>         void                    (*card_event)(struct platform_device *p_dev);
> +       void                    (*power_off)(struct platform_device *p_dev);
>  };
>
>  #endif
> --
> 2.51.0
>