Re: [PATCH 4/5] memstick: rtsx_usb_ms: Support runtime power management

From: Kai-Heng Feng
Date: Thu Aug 23 2018 - 04:12:54 EST


at 21:29, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:

[...]

-#ifdef CONFIG_PM_SLEEP
-static int rtsx_usb_ms_suspend(struct device *dev)
+#ifdef CONFIG_PM
+static int rtsx_usb_ms_runtime_suspend(struct device *dev)
{
struct rtsx_usb_ms *host = dev_get_drvdata(dev);
struct memstick_host *msh = host->msh;

- dev_dbg(ms_dev(host), "--> %s\n", __func__);
-
+ host->suspend = true;
memstick_suspend_host(msh);

I missed this one. Does this really work? To me, this looks like doing
things upside-down.

To suspend the host, you first need to runtime resume it, because
mmc_suspend_host() calls into one of the host ops and my touch the
device, right?

If you want to suspend the host (actually the naming is wrong, as it's
about suspending/power-iff the memstick card), that should be done via
when the memstick core finds that the card is removed or during system
wide suspend.

Do you mean the logic was wrong even before my modification?

Kai-Heng


+
return 0;
}

-static int rtsx_usb_ms_resume(struct device *dev)
+static int rtsx_usb_ms_runtime_resume(struct device *dev)
{
struct rtsx_usb_ms *host = dev_get_drvdata(dev);
struct memstick_host *msh = host->msh;

- dev_dbg(ms_dev(host), "--> %s\n", __func__);
-
memstick_resume_host(msh);

According to the above, this seems not correct to me.

+ host->suspend = false;
+ schedule_delayed_work(&host->poll_card, 100);
+
return 0;
}
-#endif /* CONFIG_PM_SLEEP */

[...]

Kind regards
Uffe