[PATCH v1] mmc: rtsx_usb_sdmmc: add parameter to always poll for card presence

[PATCH v1] mmc: rtsx_usb_sdmmc: add parameter to always poll for card presence

[Gavin Li]

Some RTS5179 implementations have broken remote wake-up, preventing the
runtime_resume calback from being called and breaking card insertion.
With the current implementation, the card is detected if it is present
when the RTS5179 enumerates. However, card insertions after the initial
enumeration are not detected, and a rtsx_usb_sdmmc module reload is
necessary to detect the card again.

The change to only poll when card inserted was added in commit
4dad599b8b5d ("Re-work card detection/removal support") to save power
when the card is not present. Thus, this change adds a module parameter
to revert to the previous behavior of always polling for card presence.

Signed-off-by: Gavin Li <git@thegavinli.com>
---
 drivers/mmc/host/rtsx_usb_sdmmc.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c b/drivers/mmc/host/rtsx_usb_sdmmc.c
index d229c2b83ea9..246b0da1e908 100644
--- a/drivers/mmc/host/rtsx_usb_sdmmc.c
+++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
@@ -30,6 +30,10 @@
 #define RTSX_USB_USE_LEDS_CLASS
 #endif
 
+static bool always_poll = false;
+module_param(always_poll, bool, 0444);
+MODULE_PARM_DESC(always_poll, "always poll for card presence");
+
 struct rtsx_usb_sdmmc {
 	struct platform_device	*pdev;
 	struct rtsx_ucr	*ucr;
@@ -1306,6 +1310,14 @@ static void rtsx_usb_init_host(struct rtsx_usb_sdmmc *host)
 	mmc->caps2 = MMC_CAP2_NO_PRESCAN_POWERUP | MMC_CAP2_FULL_PWR_CYCLE |
 		MMC_CAP2_NO_SDIO;
 
+	/*
+	 * Some RTS5179 implementations have broken remote wake-up, preventing the
+	 * runtime_resume calback from being called and breaking card insertion.
+	 * In that case, we always need to poll.
+	 */
+	if (always_poll)
+		mmc->caps |= MMC_CAP_NEEDS_POLL;
+
 	mmc->max_current_330 = 400;
 	mmc->max_current_180 = 800;
 	mmc->ops = &rtsx_usb_sdmmc_ops;
@@ -1419,7 +1431,9 @@ static int rtsx_usb_sdmmc_runtime_suspend(struct device *dev)
 {
 	struct rtsx_usb_sdmmc *host = dev_get_drvdata(dev);
 
-	host->mmc->caps &= ~MMC_CAP_NEEDS_POLL;
+	if (!always_poll)
+		host->mmc->caps &= ~MMC_CAP_NEEDS_POLL;
+
 	return 0;
 }
 
@@ -1427,9 +1441,12 @@ static int rtsx_usb_sdmmc_runtime_resume(struct device *dev)
 {
 	struct rtsx_usb_sdmmc *host = dev_get_drvdata(dev);
 
-	host->mmc->caps |= MMC_CAP_NEEDS_POLL;
-	if (sdmmc_get_cd(host->mmc) == 1)
-		mmc_detect_change(host->mmc, 0);
+	if (!always_poll) {
+		host->mmc->caps |= MMC_CAP_NEEDS_POLL;
+		if (sdmmc_get_cd(host->mmc) == 1)
+			mmc_detect_change(host->mmc, 0);
+	}
+
 	return 0;
 }
 #endif
-- 
2.49.0

Re: [PATCH v1] mmc: rtsx_usb_sdmmc: add parameter to always poll for card presence

[Ulf Hansson]

+ Ricky,

On Sat, 10 May 2025 at 05:19, Gavin Li <gfl3162@gmail.com> wrote:
>
> Some RTS5179 implementations have broken remote wake-up, preventing the
> runtime_resume calback from being called and breaking card insertion.
> With the current implementation, the card is detected if it is present
> when the RTS5179 enumerates. However, card insertions after the initial
> enumeration are not detected, and a rtsx_usb_sdmmc module reload is
> necessary to detect the card again.
>
> The change to only poll when card inserted was added in commit
> 4dad599b8b5d ("Re-work card detection/removal support") to save power
> when the card is not present. Thus, this change adds a module parameter
> to revert to the previous behavior of always polling for card presence.

Moving this problem to userspace seems wrong to me. We should be able
to do the right thing in the kernel.

>
> Signed-off-by: Gavin Li <git@thegavinli.com>
> ---
>  drivers/mmc/host/rtsx_usb_sdmmc.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c b/drivers/mmc/host/rtsx_usb_sdmmc.c
> index d229c2b83ea9..246b0da1e908 100644
> --- a/drivers/mmc/host/rtsx_usb_sdmmc.c
> +++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
> @@ -30,6 +30,10 @@
>  #define RTSX_USB_USE_LEDS_CLASS
>  #endif
>
> +static bool always_poll = false;
> +module_param(always_poll, bool, 0444);
> +MODULE_PARM_DESC(always_poll, "always poll for card presence");
> +

Please drop this.

>  struct rtsx_usb_sdmmc {
>         struct platform_device  *pdev;
>         struct rtsx_ucr *ucr;
> @@ -1306,6 +1310,14 @@ static void rtsx_usb_init_host(struct rtsx_usb_sdmmc *host)
>         mmc->caps2 = MMC_CAP2_NO_PRESCAN_POWERUP | MMC_CAP2_FULL_PWR_CYCLE |
>                 MMC_CAP2_NO_SDIO;
>
> +       /*
> +        * Some RTS5179 implementations have broken remote wake-up, preventing the
> +        * runtime_resume calback from being called and breaking card insertion.
> +        * In that case, we always need to poll.
> +        */
> +       if (always_poll)
> +               mmc->caps |= MMC_CAP_NEEDS_POLL;
> +

We should be able to detect if we are running the broken HW and in
that case, set the flag based on that, right?

>         mmc->max_current_330 = 400;
>         mmc->max_current_180 = 800;
>         mmc->ops = &rtsx_usb_sdmmc_ops;
> @@ -1419,7 +1431,9 @@ static int rtsx_usb_sdmmc_runtime_suspend(struct device *dev)
>  {
>         struct rtsx_usb_sdmmc *host = dev_get_drvdata(dev);
>
> -       host->mmc->caps &= ~MMC_CAP_NEEDS_POLL;
> +       if (!always_poll)
> +               host->mmc->caps &= ~MMC_CAP_NEEDS_POLL;
> +
>         return 0;
>  }
>
> @@ -1427,9 +1441,12 @@ static int rtsx_usb_sdmmc_runtime_resume(struct device *dev)
>  {
>         struct rtsx_usb_sdmmc *host = dev_get_drvdata(dev);
>
> -       host->mmc->caps |= MMC_CAP_NEEDS_POLL;
> -       if (sdmmc_get_cd(host->mmc) == 1)
> -               mmc_detect_change(host->mmc, 0);
> +       if (!always_poll) {
> +               host->mmc->caps |= MMC_CAP_NEEDS_POLL;
> +               if (sdmmc_get_cd(host->mmc) == 1)
> +                       mmc_detect_change(host->mmc, 0);
> +       }
> +
>         return 0;
>  }
>  #endif
> --
> 2.49.0
>

Kind regards
Uffe

Re: [PATCH v1] mmc: rtsx_usb_sdmmc: add parameter to always poll for card presence

[Gavin Li]

On Mon, May 19, 2025 at 7:50 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:

> Moving this problem to userspace seems wrong to me. We should be able
> to do the right thing in the kernel.

Unfortunately, I don't have access to the datasheet for the RTS5179 or related
chips. This is what I could do to get my own hardware working, and it doesn't
make sense to revert to polling mode for all users if the interrupt
mode detection
works and reduces power consumption.

> We should be able to detect if we are running the broken HW and in
> that case, set the flag based on that, right?

I don't know of a way to do so, especially since I don't have non-broken HW
in my possession. On my hardware, once the device enters autosuspend,
inserting a card does not trigger a wakeup. I'm hoping that there's a way to
detect the broken HW via a hardware revision register or something similar.

Thanks,
Gavin

Re: [PATCH v1] mmc: rtsx_usb_sdmmc: add parameter to always poll for card presence

[Ulf Hansson]

On Sat, 24 May 2025 at 05:37, Gavin Li <gfl3162@gmail.com> wrote:
>
> On Mon, May 19, 2025 at 7:50 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> > Moving this problem to userspace seems wrong to me. We should be able
> > to do the right thing in the kernel.
>
> Unfortunately, I don't have access to the datasheet for the RTS5179 or related
> chips. This is what I could do to get my own hardware working, and it doesn't
> make sense to revert to polling mode for all users if the interrupt
> mode detection
> works and reduces power consumption.

Agree!

>
> > We should be able to detect if we are running the broken HW and in
> > that case, set the flag based on that, right?
>
> I don't know of a way to do so, especially since I don't have non-broken HW
> in my possession. On my hardware, once the device enters autosuspend,
> inserting a card does not trigger a wakeup. I'm hoping that there's a way to
> detect the broken HW via a hardware revision register or something similar.

Yes, something along those lines would make sense. Let's see if Ricky
can advise us on how to move forward.

Kind regards
Uffe

RE: [PATCH v1] mmc: rtsx_usb_sdmmc: add parameter to always poll for card presence

[Ricky WU]

> On Sat, 24 May 2025 at 05:37, Gavin Li <gfl3162@gmail.com> wrote:
> >
> > On Mon, May 19, 2025 at 7:50 AM Ulf Hansson <ulf.hansson@linaro.org>
> wrote:
> >
> > > Moving this problem to userspace seems wrong to me. We should be
> > > able to do the right thing in the kernel.
> >
> > Unfortunately, I don't have access to the datasheet for the RTS5179 or
> > related chips. This is what I could do to get my own hardware working,
> > and it doesn't make sense to revert to polling mode for all users if
> > the interrupt mode detection works and reduces power consumption.
> 
> Agree!
> 
> >
> > > We should be able to detect if we are running the broken HW and in
> > > that case, set the flag based on that, right?
> >
> > I don't know of a way to do so, especially since I don't have
> > non-broken HW in my possession. On my hardware, once the device enters
> > autosuspend, inserting a card does not trigger a wakeup. I'm hoping
> > that there's a way to detect the broken HW via a hardware revision register
> or something similar.
> 
> Yes, something along those lines would make sense. Let's see if Ricky can
> advise us on how to move forward.
> 

Hi Gavin,

I’m not entirely clear on what the actual issue is at this point. Initially, there was mention of “some RTS5179…” and later, 
“broken hardware…” was brought up.
Could you please clarify — is this problem happening only on certain platforms? Or is it something else?

HI Uif,

Is it generally true? That devices using the MMC_CAP_NEEDS_POLL flag may not fully support runtime_suspend,
Since they rely on polling rather than interrupts.
This can prevent the decices or host controller from reaching deeper power-saving states.


> Kind regards
> Uffe

Re: [PATCH v1] mmc: rtsx_usb_sdmmc: add parameter to always poll for card presence

[Gavin Li]

Hi Ricky,

This issue appears with the SD card reader on the IBM Lenovo
ThinkCentre M92z, which uses the RTS5179.

Assume that the rtsx_usb and rtsx_usb_sdmmc modules are loaded at boot time.

If the system boots with the SD card inserted, the SD card is detected
correctly.

if the system boots with the SD card _not_ inserted, then the SD card
is inserted after the modules are loaded, then the SD card is not
detected. If I then run `rmmod rtsx_usb_sdmmc && modprobe
rtsx_usb_sdmmc`, the SD card gets detected after the module reload.

Additionally, if I remove the SD card and insert it again, it is not
detected until I perform the module reload.

Setting always_poll=1 fixes all of the issues above.

Best,
Gavin

On Tue, May 27, 2025 at 11:10 PM Ricky WU <ricky_wu@realtek.com> wrote:
>
> > On Sat, 24 May 2025 at 05:37, Gavin Li <gfl3162@gmail.com> wrote:
> > >
> > > On Mon, May 19, 2025 at 7:50 AM Ulf Hansson <ulf.hansson@linaro.org>
> > wrote:
> > >
> > > > Moving this problem to userspace seems wrong to me. We should be
> > > > able to do the right thing in the kernel.
> > >
> > > Unfortunately, I don't have access to the datasheet for the RTS5179 or
> > > related chips. This is what I could do to get my own hardware working,
> > > and it doesn't make sense to revert to polling mode for all users if
> > > the interrupt mode detection works and reduces power consumption.
> >
> > Agree!
> >
> > >
> > > > We should be able to detect if we are running the broken HW and in
> > > > that case, set the flag based on that, right?
> > >
> > > I don't know of a way to do so, especially since I don't have
> > > non-broken HW in my possession. On my hardware, once the device enters
> > > autosuspend, inserting a card does not trigger a wakeup. I'm hoping
> > > that there's a way to detect the broken HW via a hardware revision register
> > or something similar.
> >
> > Yes, something along those lines would make sense. Let's see if Ricky can
> > advise us on how to move forward.
> >
>
> Hi Gavin,
>
> I’m not entirely clear on what the actual issue is at this point. Initially, there was mention of “some RTS5179…” and later,
> “broken hardware…” was brought up.
> Could you please clarify — is this problem happening only on certain platforms? Or is it something else?
>
> HI Uif,
>
> Is it generally true? That devices using the MMC_CAP_NEEDS_POLL flag may not fully support runtime_suspend,
> Since they rely on polling rather than interrupts.
> This can prevent the decices or host controller from reaching deeper power-saving states.
>
>
> > Kind regards
> > Uffe

RE: [PATCH v1] mmc: rtsx_usb_sdmmc: add parameter to always poll for card presence

[Ricky WU]

> Hi Ricky,
> 
> This issue appears with the SD card reader on the IBM Lenovo
> ThinkCentre M92z, which uses the RTS5179.
> 
> Assume that the rtsx_usb and rtsx_usb_sdmmc modules are loaded at boot
> time.
> 

Think you for your testing...
I think I need to found the platform(IBM Lenovo ThinkCentre M92z) first,
Because I can't reproduce this issue on my hand, I think this issue is not a generally issue  


> If the system boots with the SD card inserted, the SD card is detected
> correctly.
> 
> if the system boots with the SD card _not_ inserted, then the SD card
> is inserted after the modules are loaded, then the SD card is not
> detected. If I then run `rmmod rtsx_usb_sdmmc && modprobe
> rtsx_usb_sdmmc`, the SD card gets detected after the module reload.
> 
> Additionally, if I remove the SD card and insert it again, it is not
> detected until I perform the module reload.
> 
> Setting always_poll=1 fixes all of the issues above.
> 
> Best,
> Gavin
> 
> On Tue, May 27, 2025 at 11:10 PM Ricky WU <ricky_wu@realtek.com> wrote:
> >
> > > On Sat, 24 May 2025 at 05:37, Gavin Li <gfl3162@gmail.com> wrote:
> > > >
> > > > On Mon, May 19, 2025 at 7:50 AM Ulf Hansson
> <ulf.hansson@linaro.org>
> > > wrote:
> > > >
> > > > > Moving this problem to userspace seems wrong to me. We should be
> > > > > able to do the right thing in the kernel.
> > > >
> > > > Unfortunately, I don't have access to the datasheet for the RTS5179 or
> > > > related chips. This is what I could do to get my own hardware working,
> > > > and it doesn't make sense to revert to polling mode for all users if
> > > > the interrupt mode detection works and reduces power consumption.
> > >
> > > Agree!
> > >
> > > >
> > > > > We should be able to detect if we are running the broken HW and in
> > > > > that case, set the flag based on that, right?
> > > >
> > > > I don't know of a way to do so, especially since I don't have
> > > > non-broken HW in my possession. On my hardware, once the device
> enters
> > > > autosuspend, inserting a card does not trigger a wakeup. I'm hoping
> > > > that there's a way to detect the broken HW via a hardware revision
> register
> > > or something similar.
> > >
> > > Yes, something along those lines would make sense. Let's see if Ricky can
> > > advise us on how to move forward.
> > >
> >
> > Hi Gavin,
> >
> > I’m not entirely clear on what the actual issue is at this point. Initially, there
> was mention of “some RTS5179…” and later,
> > “broken hardware…” was brought up.
> > Could you please clarify — is this problem happening only on certain
> platforms? Or is it something else?
> >
> > HI Uif,
> >
> > Is it generally true? That devices using the MMC_CAP_NEEDS_POLL flag may
> not fully support runtime_suspend,
> > Since they rely on polling rather than interrupts.
> > This can prevent the decices or host controller from reaching deeper
> power-saving states.
> >
> >
> > > Kind regards
> > > Uffe