On Mon, May 11, 2015 at 2:47 PM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote: > On 11 May 2015 at 13:07, Eliad Peller <eliad@xxxxxxxxxx> wrote: >> On Mon, May 11, 2015 at 11:48 AM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote: >>> On 5 May 2015 at 18:03, Eliad Peller <eliad@xxxxxxxxxx> wrote: >>>> mmc_power_restore_host() calls mmc_power_up(), which >>>> returns immediately if power is already on. >>>> >>>> However, it still calls host->bus_ops->power_restore, >>>> which might result in various errors if the bus_ops >>>> doesn't handle it well (e.g. failing to run init >>>> sequence twice) >>>> >>>> Simply bail out in this case, without further calling >>>> bus_ops->power_restore. >>>> >>>> Specifically, this solves issue with wl18xx sdio card, >>>> where the mmc core powers on the card on resume (while >>>> MMC_PM_KEEP_POWER is not set), and the wl18xx device >>>> driver calls mmc_power_restore_host() once more. >>> >>> Could you elaborate on why that driver calls mmc_power_restore_host() >>> after the system PM suspend sequence? I am trying to understand the >>> use case. >>> >> The driver assumes control over the mmc power, in order to save power >> when no interface is up. > > Makes sense! > >> It basically uses runtime_pm for it, but calls the power functions >> explicitly if pm_runtime returned non-zero (this is needed for some >> corner cases, e.g. runtime pm is disabled). > > I have some ideas about changing the way runtime PM shall be used for > SDIO func drivers. Instead of using it to control power to the SDIO > card, it should be used to deal with "idle operations". > > That would mean SDIO func drivers would use only > mmc_power_save|restore_host() APIs, to control the power to the SDIO > card. > > Do you see any issues with such an approach? > actually, the current driver code (to use runtime_pm, and then call the power functions explicitly in some cases) looks a bit weird. so your approach makes sense to me. >> >> On suspend (if wowlan is not configured), all the wlan interfaces are >> taken down, and the driver powers off the device. >> On resume, the interfaces are taken up again, and the driver powers on >> the device, by calling mmc_power_restore_host(). > > That raises the following question. > > When mmc_power_save_host() has been called for an SDIO card, should > really the mmc core restore power to that card during system PM > resume? Isn't it better to leave that to the SDIO func driver? > yes. but i didn't want to make a major change :) note that the suspend/resume flow used to work properly. iirc, we did some bisect when the issue first showed up, which showed it started by: 7459026 mmc: core: Push common suspend|resume code into each bus_ops (the reason might have been different, but it did used to work before this patch) Eliad. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
