Re: [PATCH 1/2] mmc: rtsx: reset power state on suspend

[Matthew Schwartz <matthew.schwartz@linux.dev>]

On 1/20/26 4:41 AM, Ulf Hansson wrote:
> On Mon, 5 Jan 2026 at 07:02, Matthew Schwartz
> <matthew.schwartz@linux.dev> wrote:
>>
>> 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.

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.

Thanks,
Matt

> 
>>
>> 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@linux.dev>
>> ---
>>  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 f9952d76d6ed..f1f4d8ed544d 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 792ebae46697..74ee8623ad4e 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);
>> @@ -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);
>> diff --git a/include/linux/rtsx_common.h b/include/linux/rtsx_common.h
>> index da9c8c6b5d50..f294f478f0c0 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.52.0
>>
> 
> Kind regards
> Uffe