On Tue, Feb 13, 2018 at 12:33:53PM +0100, Simon Horman wrote:
> 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.

:( Pity.

> > > + 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.

Cool, thanks!

> > > +
> > > + /* 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?

I think this becomes moot if we can't get the prepare-callback of the
core to work. I'd think it makes sense to have both callbacks from here
moved to the core or none to keep at least one kind of symmetry.

Attachment: signature.asc
Description: PGP signature

Reply via email to