[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPDyKFo-2TcU6MPZpeEU7kPx4tOLG4ekEFyZnzy1qeH09MTNfQ@mail.gmail.com>
Date: Tue, 20 Jan 2026 13:41:55 +0100
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Matthew Schwartz <matthew.schwartz@...ux.dev>
Cc: Arnd Bergmann <arnd@...db.de>, Ricky Wu <ricky_wu@...ltek.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>, linux-mmc@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] mmc: rtsx: reset power state on suspend
On Mon, 5 Jan 2026 at 07:02, Matthew Schwartz
<matthew.schwartz@...ux.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.
>
> 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@...ux.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
Powered by blists - more mailing lists