[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b6ddabc6-dd9c-4230-b19c-b2b1f5192635@intel.com>
Date: Mon, 12 Jan 2026 09:39:26 +0200
From: Adrian Hunter <adrian.hunter@...el.com>
To: Tabby Kitten <nyanpasu256@...il.com>, Ulf Hansson
<ulf.hansson@...aro.org>, <ricky_wu@...ltek.com>
CC: <linux-mmc@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: rtsx_pci_sdmmc aborts suspend when /sys/power/wakeup_count is
enabled
On 09/01/2026 04:46, Tabby Kitten wrote:
> On 1/5/26 4:31 AM, Adrian Hunter wrote:
>> Seems reasonable, but isn't there also:
>> bus_ops->suspend() == mmc_sd_suspend()
>> _mmc_sd_suspend()
>> mmc_claim_host(host)
>>
>> In general, it looks difficult to avoid runtime resume on
>> the suspend path. PCI will usually runtime resume anyway
>> i.e. from pci_pm_suspend():
>>
>> /*
>> * PCI devices suspended at run time may need to be resumed at this
>> * point, because in general it may be necessary to reconfigure them for
>> * system suspend. Namely, if the device is expected to wake up the
>> * system from the sleep state, it may have to be reconfigured for this
>> * purpose, or if the device is not expected to wake up the system from
>> * the sleep state, it should be prevented from signaling wakeup events
>> * going forward.
>> *
>> * Also if the driver of the device does not indicate that its system
>> * suspend callbacks can cope with runtime-suspended devices, it is
>> * better to resume the device from runtime suspend here.
>> */
>> if (!dev_pm_smart_suspend(dev) || pci_dev_need_resume(pci_dev)) {
>> pm_runtime_resume(dev);
>>
>> So maybe alter rtsx_pci_runtime_resume() to avoid calling
>> pcr->slots[RTSX_SD_CARD].card_event() == rtsx_pci_sdmmc_card_event()
>> when suspending. Perhaps along the lines of the hack below:
>>
>> static int rtsx_pci_runtime_resume(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;
>>
>> dev_dbg(device, "--> %s\n", __func__);
>>
>> mutex_lock(&pcr->pcr_mutex);
>>
>> rtsx_pci_write_register(pcr, HOST_SLEEP_STATE, 0x03, 0x00);
>>
>> rtsx_pci_init_hw(pcr);
>>
>> if (pcr->slots[RTSX_SD_CARD].p_dev != NULL) {
>> +#if IS_ENABLED(CONFIG_SUSPEND)
>> + if (pm_suspend_target_state == PM_SUSPEND_ON)
>> +#endif
>> pcr->slots[RTSX_SD_CARD].card_event(
>> pcr->slots[RTSX_SD_CARD].p_dev);
>> }
>>
>> mutex_unlock(&pcr->pcr_mutex);
>> return 0;
>> }
>>
>>> WIP
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@...aro.org>
>>> ---
>>> drivers/mmc/core/core.c | 18 ++++++++++++------
>>> drivers/mmc/core/core.h | 11 ++++++++---
>>> drivers/mmc/core/queue.c | 4 ++--
>>> drivers/mmc/core/sdio_irq.c | 2 +-
>>> 4 files changed, 23 insertions(+), 12 deletions(-)
>>>
>>> ...
>
> Me earlier:
>
>> I'm attemping to manually replicate the changes on Fedora 43's
>> kernel-6.18.3 checkout
>> (https://docs.fedoraproject.org/en-US/quick-docs/kernel-build-custom/),
>> though I'm much less experienced building kernels here than on Arch
>> Linux (the Arch SSD is currently in another computer). I will be
>> replying back with results once I can build and test these patches.
>
> I've built a test kernel based on Fedora's 6.18.3 along with these two
> patches. Now `sudo badsleep.sh` successfully completes on the machine
> with the Realtek card reader.
>
> * Adrian's code would not compile until I edited
> drivers/misc/cardreader/rtsx_pcr.c and added #include <linux/suspend.h>.
> * It looks a bit janky that the inner line of code is tied to a
> different natural indentation level based on a compile-time flag.
> With suspend enabled, the function call is on the same indentation
> level as the if statement!
> o One possibility is to indent the inner code one more level
> (which is an extraneous indentation if the #if is inactive)
> o Another is to move the added condition into the surrounding `if
> (pcr->slots[RTSX_SD_CARD].p_dev != NULL)`, but this prevents us
> from adding code that /doesn't/ check pm_suspend_target_state.
Currently (since v6.5) pm_suspend_target_state gets defined even
if !CONFIG_SUSPEND so, allowing up to 100 cols, it can be
- if (pcr->slots[RTSX_SD_CARD].p_dev != NULL) {
- pcr->slots[RTSX_SD_CARD].card_event(
- pcr->slots[RTSX_SD_CARD].p_dev);
- }
+ if (pcr->slots[RTSX_SD_CARD].p_dev != NULL && pm_suspend_target_state == PM_SUSPEND_ON)
+ pcr->slots[RTSX_SD_CARD].card_event(pcr->slots[RTSX_SD_CARD].p_dev);
Could use a comment as well, noting that card_event() can call
mmc_detect_change() which prevents system sleep, so it must be
avoided if the system is suspending.
Note, this change should be sufficient to fix the issue by itself.
>
> I ran into a possible bug:
>
> * On my first boot attempt, I tried running badsleep.sh, waking the
> system, and inserted a microSD card. The card was not recognized in
> Dolphin or listed in lsblk. rtsx_pci_sdmmc was present in lsmod, and
> I saw no references to rtsx or mmc in the journal.
> * I could not reproduce this error on subsequent boots. I rebooted the
> computer, then tried badsleep.sh (with or without regular KDE sleep
> beforehand), then inserted the microSD card. At this point it was
> recognized properly. I also tried inserting the card /while/ the
> system was asleep, which worked too. I'm not sure why it failed the
> first time... dirty contacts? random bug?
Well, you need not only to reproduce it, but show that it doesn't
also happen with the old code, otherwise there is not enough information
to work with.
Powered by blists - more mailing lists