Re: rtsx_pci_sdmmc aborts suspend when /sys/power/wakeup_count is enabled

[Tabby Kitten <nyanpasu256@gmail.com>]

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@linaro.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.

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?