On Wed, Feb 07, 2018 at 11:20:12PM +0100, Wolfram Sang wrote:
> 
> Hi Simon,
> 
> > +   void (*disable_scc)(struct mmc_host *mmc);
> 
> Do we really need this callback? I'd think it can be folded into
> reset_hs400_mode() because it is called only once?
> 
> > +   void (*prepare_hs400_tuning)(struct mmc_host *mmc, struct mmc_ios *ios);
> 
> Can't we use the host->ops->prepare_hs400_tuning() callback invoked by
> the core?

Empirically that does not seem to work.

> > +   void (*reset_hs400_mode)(struct mmc_host *mmc);
> 
> Maybe we can get rid of this, too? See later...
> 
> > +   if (host->disable_scc)
> > +           host->disable_scc(mmc);
> 
> (Here, this can be folded into the next callback)

Yes, agreed. I've folded the callbacks as you suggest.

> > +
> > +   /* reset HS400 mode */
> > +   if (ios->timing != MMC_TIMING_MMC_HS400 && host->reset_hs400_mode)
> > +           host->reset_hs400_mode(mmc);
> 
> I wonder: If for any ios which is != MMC_TIMING_MMC_HS400, the
> hs400_mode needs to be reset. Couldn't we as well then disable the mode
> always after the MMC_TIMING_MMC_HS400 tuning was selected? Just
> brainstorming here...

Perhaps but I'm unsure where we would hook in this change, any ideas?

Reply via email to