Re: [PATCH] mmc: mmc: Fix the timing for clock changing in mmc

[Chaotian Jing]

On Tue, 2020-03-10 at 16:41 +0100, Ulf Hansson wrote:
> On Tue, 10 Mar 2020 at 11:44, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
> >
> > On 10/03/20 11:05 am, Ulf Hansson wrote:
> > > On Tue, 10 Mar 2020 at 05:28, Seo, Kyungmin <kyungmin.seo@xxxxxxxxx> wrote:
> > >>
> > >> I read the link and patch of Chaotian Jing.
> > >> I also point out what Chaotian said.
> > >> Most host controllers have DLL tuning values for each mode. When host controller is set as HS400 mode with 50MHz clock, host controller uses DLL value which is tuned with 200MHz clock.
> > >>
> > >> If DLL value in HS400 mode doesn't have the pass range in HS mode, command transfer failing may fail.
> > >> In order to make robust sdhci driver, I think the patch needs to be considered.
> > >
> > > I have, but I am not picking it up in its current form.
> > >
> > >> Of course, CMD6 with HS400 mode and 200MHz clock should not cause any problem because it's correct configuration.
> > >
> > > Yes, but not for all cases, as I said in my reply in those email-threads.
> > >
> > > What I had in mind, is that I I think we should inform
> > > mmc_hs400_to_hs200() about under what situation it's getting called.
> > > Depending on that, we should either decrease the clock rate before or
> > > after we send the CMD6.
> > >
> > > Would that work for your case?
> >
> > Ulf, would you consider a new call back e.g.
> 
> That could work, but I am not sure what's best, honestly.
> 
> The problem may be generic or it could be specific to some host
> controller? I think we need to answer that question first.
> 
> What do you think?
> 
> Br
> Uffe
> 
When start to send CMD6 to switch to HS mode, both Host & eMMC device
are working on HS400 mode, so the timing used is MUST at HS400 mode and
the clock MUST keep at current clock(usually 200Mhz). after received the
response of CMD6, Never use CMD13 to polling card status for timing
switch. if host has ops->card_busy() or caps WAIT_WHILE_BUSY, then use
it, if not,just do mmc_delay() for specific time.

the next step is that call mmc_set_ios() to set current timing to HS
mode and clock to 50Mhz to let Host driver that eMMC device has been
switched to HS mode and Host can switch to HS mode at 50Mhz(may apply
parameters for this low speed).
> >
> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> > index c2abd417a84a..1bc18fe2632f 100644
> > --- a/drivers/mmc/core/mmc.c
> > +++ b/drivers/mmc/core/mmc.c
> > @@ -1237,7 +1237,10 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
> >
> >         /* Reduce frequency to HS */
> >         max_dtr = card->ext_csd.hs_max_dtr;
> > -       mmc_set_clock(host, max_dtr);
> > +       if (host->ops->hs400_to_hs200_prep)
> > +               host->ops->hs400_to_hs200_prep(host, max_dtr);
> > +       else
> > +               mmc_set_clock(host, max_dtr);
> >
> >         /* Switch HS400 to HS DDR */
> >         val = EXT_CSD_TIMING_HS;
> >
> >
> 
> [...]
> 
> Kind regards
> Uffe