| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <79349439-97ea-4147-961e-63cd6a5bfd68@linux.dev>
Date: Tue, 20 Jan 2026 10:38:59 -0800
From: Matthew Schwartz <matthew.schwartz@...ux.dev>
To: Ulf Hansson <ulf.hansson@...aro.org>
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 1/20/26 4:41 AM, Ulf Hansson wrote:
> 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.
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@...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