On Tue, Aug 23, 2016 at 05:02:56PM +0200, Ulf Hansson wrote: > [...] > > >> > +static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode) > >> > +{ > >> > + struct tmio_mmc_host *host = mmc_priv(mmc); > >> > + unsigned int num; > >> > + int i, ret = 0; > >> > + bool *tap; > >> > + > >> > + if (!host->init_tuning || !host->select_tuning) > >> > >> When defining these callbacks, it would be nice to know which ones are > >> optional and which ones are required. > > > > Would some comments in struct tmio_mmc_host be an appropriate way > > to achieve that? > > Yes, that would be nice! > > [...] > > >> > static int tmio_mmc_init_ocr(struct tmio_mmc_host *host) > >> > @@ -1202,6 +1276,9 @@ int tmio_mmc_host_runtime_suspend(struct device *dev) > >> > struct mmc_host *mmc = dev_get_drvdata(dev); > >> > struct tmio_mmc_host *host = mmc_priv(mmc); > >> > > >> > + mmc_retune_timer_stop(host->mmc); > >> > + mmc_retune_needed(host->mmc); > >> > >> I am wondering whether it would it be possible to keep a cache of the > >> currently used tuning values and then restore these values at > >> runtime_resume? > >> > >> In that way you wouldn't need to force new re-tuning cycle here. > > > > I will check with the hardware people to see if it is practical from > > that POV. > > Okay! The feedback I received is something like this: * The tap values calculated during retuning depend on the temperature of the SoC and card. * So after resume the values may be invalid. * It may be possible to re-use the TAP values and re-tune if a transfer fails but the people I spoke with were unsure of the merit of that approach Personally my feeling is that we should initiate a retune on resume for now as that seems to be the safest option. > > From a software POV that ought to be simple enough: a small bitmap ought > > to be sufficient to save the values for the hardware covered by this > > series. > > Yes, indeed! > > Kind regards > Uffe >