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

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(-)

+ 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

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

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

> 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