Re: [RFC PATCH 1/2] rtsx_usb: hold runtime PM during transfers

[Ulf Hansson]

On Mon, 13 Apr 2026 at 13:42, Sean Rhodes <sean@starlabs.systems> wrote:
>
> Hold a runtime-PM reference across bulk transfers, and mark the device
> busy afterwards.

Why isn't it sufficient to let the mmc host device (child device) and
its corresponding driver to manage runtime PM reference counting?

As soon as the mmc host driver is requested to power-on the card via
its ->ios() callback (set to sdmmc_set_ios() in
drivers/mmc/host/rtsx_usb_sdmmc.c, it bumps the runtime PM usage
count. Therefore, the usb device should be prevented from being
runtime suspended as long as a card is inserted. Isn't that
sufficient?

>
> When runtime PM is already in progress (e.g. from rtsx_usb_suspend()),
> avoid forcing a runtime resume from within the PM path by using
> usb_autopm_get_interface_no_resume() unless the interface is already
> runtime-suspended.

Why is this needed? What problem does it solve?

>
> Signed-off-by: Sean Rhodes <sean@starlabs.systems>

Kind regards
Uffe

> ---
>  drivers/misc/cardreader/rtsx_usb.c | 38 ++++++++++++++++++++++++------
>  1 file changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/misc/cardreader/rtsx_usb.c b/drivers/misc/cardreader/rtsx_usb.c
> index f65acf4d1164..c8badd0db87b 100644
> --- a/drivers/misc/cardreader/rtsx_usb.c
> +++ b/drivers/misc/cardreader/rtsx_usb.c
> @@ -12,6 +12,7 @@
>  #include <linux/usb.h>
>  #include <linux/platform_device.h>
>  #include <linux/mfd/core.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/rtsx_usb.h>
>
>  static int polling_pipe = 1;
> @@ -65,19 +66,42 @@ static int rtsx_usb_bulk_transfer_sglist(struct rtsx_ucr *ucr,
>  }
>
>  int rtsx_usb_transfer_data(struct rtsx_ucr *ucr, unsigned int pipe,
> -                             void *buf, unsigned int len, int num_sg,
> -                             unsigned int *act_len, int timeout)
> +                                     void *buf, unsigned int len, int num_sg,
> +                                     unsigned int *act_len, int timeout)
>  {
> +       int ret;
> +       struct device *dev = &ucr->pusb_intf->dev;
> +
>         if (timeout < 600)
>                 timeout = 600;
>
> +       /*
> +        * During runtime suspend/resume callbacks, avoid forcing a runtime resume
> +        * from within the PM path. The device is still active when
> +        * rtsx_usb_suspend() runs, but usb_autopm_get_interface() can block when
> +        * runtime PM is already in progress.
> +        */
> +       if (pm_runtime_status_suspended(dev)) {
> +               ret = usb_autopm_get_interface(ucr->pusb_intf);
> +       } else {
> +               usb_autopm_get_interface_no_resume(ucr->pusb_intf);
> +               ret = 0;
> +       }
> +       if (ret)
> +               return ret;
> +
>         if (num_sg)
> -               return rtsx_usb_bulk_transfer_sglist(ucr, pipe,
> -                               (struct scatterlist *)buf, num_sg, len, act_len,
> -                               timeout);
> +               ret = rtsx_usb_bulk_transfer_sglist(ucr, pipe,
> +                                                   (struct scatterlist *)buf,
> +                                                   num_sg, len, act_len,
> +                                                   timeout);
>         else
> -               return usb_bulk_msg(ucr->pusb_dev, pipe, buf, len, act_len,
> -                               timeout);
> +               ret = usb_bulk_msg(ucr->pusb_dev, pipe, buf, len, act_len,
> +                                  timeout);
> +
> +       usb_mark_last_busy(ucr->pusb_dev);
> +       usb_autopm_put_interface(ucr->pusb_intf);
> +       return ret;
>  }
>  EXPORT_SYMBOL_GPL(rtsx_usb_transfer_data);
>