On 1 September 2016 at 10:37, Simon Horman <horms@xxxxxxxxxxxx> wrote: > On Thu, Sep 01, 2016 at 08:46:44AM +0200, Simon Horman wrote: >> On Wed, Aug 31, 2016 at 09:38:40AM +0200, Ulf Hansson wrote: >> > On 30 August 2016 at 22:51, Simon Horman <horms@xxxxxxxxxxxx> wrote: >> > > On Mon, Aug 29, 2016 at 04:05:55PM +0200, Ulf Hansson wrote: >> > >> On 29 August 2016 at 14:05, Simon Horman <horms@xxxxxxxxxxxx> wrote: >> > >> > On Fri, Aug 26, 2016 at 10:01:35AM +0200, Ulf Hansson wrote: >> > >> >> On 25 August 2016 at 14:04, Simon Horman <horms@xxxxxxxxxxxx> wrote: >> > >> >> > On Tue, Aug 23, 2016 at 05:02:56PM +0200, Ulf Hansson wrote: >> > >> > >> > >> > ... >> > > >> > > ... >> > > >> > >> > @@ -1271,6 +1278,7 @@ int tmio_mmc_host_runtime_suspend(struct device *dev) >> > >> > >> > >> > mmc_retune_timer_stop(host->mmc); >> > >> > mmc_retune_needed(host->mmc); >> > >> > + host->use_saved_taps = true; >> > >> >> > >> I don't think you should trigger a re-tune here at all. Moreover you >> > >> don't need to keep track of whether you have valid tap values by using >> > >> the ->use_saved_taps variable, the mmc core deals with this for you. >> > >> >> > >> Instead, you should restore the tap values by invoking >> > >> host->select_tuning() from the ->runtime_resume() callback, although >> > >> only when host->mmc->can_retune == 1. We should probably add a helper >> > >> function in the mmc core for this check, instead of accessing >> > >> "can_retune" directly. >> > > >> > > Thanks, I was wondering what the best way to handle this is. >> > > >> > > I tried your suggestion above but it seems that host->mmc->can_retune is >> > > zero when ->runtime_resume() is called because of the following call paths: >> > > >> > > a) _mmc_sd_suspend() >> > > -> mmc_power_off() >> > > -> mmc_set_initial_state() >> > > -> mmc_retune_disable >> > > and >> > > b) mmc_sd_runtime_resume() >> > > -> mmc_power_up >> > > -> mmc_set_initial_state() >> > > -> mmc_retune_disable >> > > >> > >> > tmio_mmc_disable_mmc_irqs(host, TMIO_MASK_ALL); >> > >> > This is exactly what shall happen :-) >> > >> > If the mmc core suspends the card, it means it will also re-initialize >> > the card when resuming it. In that way the regular tuning sequence >> > will take place as a part of re-initialization of the card, so you >> > definitely must not restore tap values in these case. That is why you >> > should be using the can_retune to know when to restore the values. >> >> Understood. In that case things are working as expected. > > I do have one question. It seems to me that host->mmc->can_retune == 1 on > resume. If so, when would the saved tap values be used? I feel that I am > missing something here. There are different "resumes" here, so this gets a bit tricky to discuss around. Let me give it another try. First, let's assume we have a card inserted, initialized and tuned. This also means we have a cache of tuning (tap) values in the host. When the *host* runtime PM suspends, which isn't related to when the card suspends, typically the card remains powered/initialized. Then, when the host runtime PM resumes the tuning values can be restored in the host controller, as those should still be working because the card has remained initialized. When the card system PM suspends, the card will be put to sleep/power off (can_retune == 0). Sooner or later the host also becomes runtime PM suspended in this sequence. When the card system PM resumes, the card will be woken up from sleep, power on and the re-initializations starts. Before the re-initializations starts, the host will be runtime PM resumed, but since can_retune == 0, no tuning values will be restored (correct). When the re-initialization sequence completes of the card, can_retune == 1 as a new tuning sequence has also been completed. This also means refreshed cached tunings values in the host, which it can use the next time it will be runtime PM resumed. Hope this made more sense now. [...] Kind regards Uffe -- 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
