mmc: rtsx: card stuck busy fixes for Realtek PCI SD card readers

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 <[email protected]>

---
 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(+)

drivers/misc/cardreader/rtsx_pcr.c +9 -0

index f9952d76d6ed..f1f4d8ed544d 100644
@@ -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);

drivers/mmc/host/rtsx_pci_sdmmc.c +22 -0

index 792ebae46697..74ee8623ad4e 100644
@@ -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);

@@ -1524,6 +1534,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;

@@ -1556,6 +1576,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);
 

@@ -1587,6 +1608,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);

include/linux/rtsx_common.h +1 -1

index da9c8c6b5d50..f294f478f0c0 100644
@@ -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.52.0

Quoted 5 lines · Matthew Schwartz

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.

The mmc core should power-off the card, via the ->set_ios() callback before the rtsx_pci suspends. In other words, the mmc core should have set MMC_POWER_OFF.

That said, there seems to be something wrong in drivers/mmc/host/rtsx_pci_sdmmc.c's ->set_ios(), if that isn't tracked correctly. The parent device/driver, rtsx_pci should not need to inform that the power to the card is removed, as that should be known already by the rtsx_pci_sdmmc driver.

Quoted 127 lines

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 <[email protected]>

---
 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(+)

drivers/misc/cardreader/rtsx_pcr.c +9 -0

index f9952d76d6ed..f1f4d8ed544d 100644
@@ -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);

drivers/mmc/host/rtsx_pci_sdmmc.c +22 -0

index 792ebae46697..74ee8623ad4e 100644
@@ -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);

@@ -1524,6 +1534,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;

@@ -1556,6 +1576,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);
 

@@ -1587,6 +1608,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);

include/linux/rtsx_common.h +1 -1

index da9c8c6b5d50..f294f478f0c0 100644
@@ -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
--

Quoted 2 lines

2.52.0

Kind regards
Uffe

Quoted 12 lines · Ulf Hansson

Quoted 5 lines · Matthew Schwartz

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.

The mmc core should power-off the card, via the ->set_ios() callback before the rtsx_pci suspends. In other words, the mmc core should have set MMC_POWER_OFF.

Quoted 5 lines

That said, there seems to be something wrong in drivers/mmc/host/rtsx_pci_sdmmc.c's ->set_ios(), if that isn't tracked correctly. The parent device/driver, rtsx_pci should not need to inform that the power to the card is removed, as that should be known already by the rtsx_pci_sdmmc driver.

Ah, I see what you mean now, I think it should be fixed in rtsx_pci_sdmmc too.

These already seem to be in char-misc-next, so I think I need to wait until those hit upstream and then revert as a part of a v2 patch? If I revert from char-misc-next I'm worried the commit id won't match once it gets merged into master.

This series fixes some SD card timeout errors that occur after suspend and resume ops on systems with Realtek PCI card readers. These issues occur on my 2 RTS525A card readers and my RTS5261 card reader.

As a stress test, I ran the amd-s2idle script from amd-debug-tools to automate s2idle cycles and I launched a visual novel, WITCH ON THE HOLY NIGHT, from the SD card. Visual novels are basically just slideshows, and waiting for 10 seconds without advancing the text is an easy way to trigger rtsx_pci_runtime_suspend/rtsx_pci_runtime_resume. I set amd-s2idle to test with a 10 second long s2idle length with 13 seconds between runs.

The main symptom that always shows up is "mmc0: Card stuck being busy!" followed by timeouts and filesystem errors. This can occur during both runtime resume and s2idle resume:

[  260.638214] mmc0: error -110 doing runtime resume
[  261.054214] mmc0: card d555 removed
[  261.054382] Aborting journal on device mmcblk0p1-8.
[  261.054386] JBD2: I/O error when updating journal superblock for mmcblk0p1-8.
[  261.067729] EXT4-fs (mmcblk0p1): shut down requested (2)

Patch 1 adds a power_off callback so rtsx_pcr can notify the sdmmc driver during suspend, allowing it to reset prev_power_state. This ensures sd_power_on properly reinitializes the card on resume. It reduces the failure rate of "mmc0: Card stuck being busy!" after resume from 20% down to 4%.

Patch 2 increases the post-power-on settling delay from 1ms to 5ms. The shorter delay seems insufficient for reliable reinitialization after the hardware has been powered off during suspend. Increasing this reduces the failure rate from 4% down to 0%, tested with over 300 s2idle cycles.

Matthew Schwartz (2):
mmc: rtsx: reset power state on suspend
mmc: rtsx_pci_sdmmc: increase power-on settling delay to 5ms

 drivers/misc/cardreader/rtsx_pcr.c |  9 +++++++++
 drivers/mmc/host/rtsx_pci_sdmmc.c  | 24 +++++++++++++++++++++++-
 include/linux/rtsx_common.h        |  1 +
 3 files changed, 33 insertions(+), 1 deletion(-)

--
2.52.0

Quoted 42 lines · Matthew Schwartz

This series fixes some SD card timeout errors that occur after suspend and resume ops on systems with Realtek PCI card readers. These issues occur on my 2 RTS525A card readers and my RTS5261 card reader.

As a stress test, I ran the amd-s2idle script from amd-debug-tools to automate s2idle cycles and I launched a visual novel, WITCH ON THE HOLY NIGHT, from the SD card. Visual novels are basically just slideshows, and waiting for 10 seconds without advancing the text is an easy way to trigger rtsx_pci_runtime_suspend/rtsx_pci_runtime_resume. I set amd-s2idle to test with a 10 second long s2idle length with 13 seconds between runs.

The main symptom that always shows up is "mmc0: Card stuck being busy!" followed by timeouts and filesystem errors. This can occur during both runtime resume and s2idle resume:

[  260.638214] mmc0: error -110 doing runtime resume
[  261.054214] mmc0: card d555 removed
[  261.054382] Aborting journal on device mmcblk0p1-8.
[  261.054386] JBD2: I/O error when updating journal superblock for mmcblk0p1-8.
[  261.067729] EXT4-fs (mmcblk0p1): shut down requested (2)

Patch 1 adds a power_off callback so rtsx_pcr can notify the sdmmc driver during suspend, allowing it to reset prev_power_state. This ensures sd_power_on properly reinitializes the card on resume. It reduces the failure rate of "mmc0: Card stuck being busy!" after resume from 20% down to 4%.

Patch 2 increases the post-power-on settling delay from 1ms to 5ms. The shorter delay seems insufficient for reliable reinitialization after the hardware has been powered off during suspend. Increasing this reduces the failure rate from 4% down to 0%, tested with over 300 s2idle cycles.

Matthew Schwartz (2):
mmc: rtsx: reset power state on suspend
mmc: rtsx_pci_sdmmc: increase power-on settling delay to 5ms

 drivers/misc/cardreader/rtsx_pcr.c |  9 +++++++++
 drivers/mmc/host/rtsx_pci_sdmmc.c  | 24 +++++++++++++++++++++++-
 include/linux/rtsx_common.h        |  1 +
 3 files changed, 33 insertions(+), 1 deletion(-)

Quoted 3 lines

--
2.52.0

Greg, looks like you have queued up these patches via your misc tree, can you please drop them? We need a different approach to the problem.

Quoted 48 lines · Ulf Hansson

Quoted 45 lines · Matthew Schwartz

This series fixes some SD card timeout errors that occur after suspend and resume ops on systems with Realtek PCI card readers. These issues occur on my 2 RTS525A card readers and my RTS5261 card reader.

As a stress test, I ran the amd-s2idle script from amd-debug-tools to automate s2idle cycles and I launched a visual novel, WITCH ON THE HOLY NIGHT, from the SD card. Visual novels are basically just slideshows, and waiting for 10 seconds without advancing the text is an easy way to trigger rtsx_pci_runtime_suspend/rtsx_pci_runtime_resume. I set amd-s2idle to test with a 10 second long s2idle length with 13 seconds between runs.

The main symptom that always shows up is "mmc0: Card stuck being busy!" followed by timeouts and filesystem errors. This can occur during both runtime resume and s2idle resume:

[  260.638214] mmc0: error -110 doing runtime resume
[  261.054214] mmc0: card d555 removed
[  261.054382] Aborting journal on device mmcblk0p1-8.
[  261.054386] JBD2: I/O error when updating journal superblock for mmcblk0p1-8.
[  261.067729] EXT4-fs (mmcblk0p1): shut down requested (2)

Patch 1 adds a power_off callback so rtsx_pcr can notify the sdmmc driver during suspend, allowing it to reset prev_power_state. This ensures sd_power_on properly reinitializes the card on resume. It reduces the failure rate of "mmc0: Card stuck being busy!" after resume from 20% down to 4%.

Patch 2 increases the post-power-on settling delay from 1ms to 5ms. The shorter delay seems insufficient for reliable reinitialization after the hardware has been powered off during suspend. Increasing this reduces the failure rate from 4% down to 0%, tested with over 300 s2idle cycles.

Matthew Schwartz (2):
mmc: rtsx: reset power state on suspend
mmc: rtsx_pci_sdmmc: increase power-on settling delay to 5ms

 drivers/misc/cardreader/rtsx_pcr.c |  9 +++++++++
 drivers/mmc/host/rtsx_pci_sdmmc.c  | 24 +++++++++++++++++++++++-
 include/linux/rtsx_common.h        |  1 +
 3 files changed, 33 insertions(+), 1 deletion(-)

--
2.52.0

Quoted 2 lines

Greg, looks like you have queued up these patches via your misc tree, can you please drop them? We need a different approach to the problem.

They need to be reverted (my tree can not be rebased.) Should I do that, or does someone want to send me the reverts?

Quoted 53 lines · Greg Kroah-Hartman

Quoted 50 lines · Ulf Hansson

Quoted 37 lines · Matthew Schwartz

This series fixes some SD card timeout errors that occur after suspend and resume ops on systems with Realtek PCI card readers. These issues occur on my 2 RTS525A card readers and my RTS5261 card reader.

As a stress test, I ran the amd-s2idle script from amd-debug-tools to automate s2idle cycles and I launched a visual novel, WITCH ON THE HOLY NIGHT, from the SD card. Visual novels are basically just slideshows, and waiting for 10 seconds without advancing the text is an easy way to trigger rtsx_pci_runtime_suspend/rtsx_pci_runtime_resume. I set amd-s2idle to test with a 10 second long s2idle length with 13 seconds between runs.

The main symptom that always shows up is "mmc0: Card stuck being busy!" followed by timeouts and filesystem errors. This can occur during both runtime resume and s2idle resume:

[  260.638214] mmc0: error -110 doing runtime resume
[  261.054214] mmc0: card d555 removed
[  261.054382] Aborting journal on device mmcblk0p1-8.
[  261.054386] JBD2: I/O error when updating journal superblock for mmcblk0p1-8.
[  261.067729] EXT4-fs (mmcblk0p1): shut down requested (2)

Patch 1 adds a power_off callback so rtsx_pcr can notify the sdmmc driver during suspend, allowing it to reset prev_power_state. This ensures sd_power_on properly reinitializes the card on resume. It reduces the failure rate of "mmc0: Card stuck being busy!" after resume from 20% down to 4%.

Patch 2 increases the post-power-on settling delay from 1ms to 5ms. The shorter delay seems insufficient for reliable reinitialization after the hardware has been powered off during suspend. Increasing this reduces the failure rate from 4% down to 0%, tested with over 300 s2idle cycles.

Matthew Schwartz (2):
mmc: rtsx: reset power state on suspend
mmc: rtsx_pci_sdmmc: increase power-on settling delay to 5ms

Quoted 8 lines

 drivers/misc/cardreader/rtsx_pcr.c |  9 +++++++++
 drivers/mmc/host/rtsx_pci_sdmmc.c  | 24 +++++++++++++++++++++++-
 include/linux/rtsx_common.h        |  1 +
 3 files changed, 33 insertions(+), 1 deletion(-)

--
2.52.0

Greg, looks like you have queued up these patches via your misc tree, can you please drop them? We need a different approach to the problem.

Quoted 2 lines

They need to be reverted (my tree can not be rebased.) Should I do that, or does someone want to send me the reverts?

If you can revert them, that would be great!

Then I will continue to work with Matthew to fix the problem in a more proper way.