Messages in this thread | | | Subject | Re: [PATCH 4/5] memstick: rtsx_usb_ms: Support runtime power management | From | Kai-Heng Feng <> | Date | Thu, 23 Aug 2018 16:12:42 +0800 |
| |
at 21:29, Ulf Hansson <ulf.hansson@linaro.org> 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
| |