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);
I plan to repost this patchset without the tap restoration code.
This is because I have a number of changes locally, in particular
to address other parts of your review, and I would like
them to see the light of day.
I will mark the tap restoration as a TODO item which we can address
before merging the code in question.
