Re: [PATCH 1/2] mmc: core: when downgrading HS400, callback into drivers earlier

Wolfram Sang
2020-06-08T20:41:17Z

[-- Attachment #1: Type: text/plain, Size: 1227 bytes --]

Hi Shimoda-san,

thank you very much for the review!

> > The driver specific downgrade function makes more sense if we run it
> > before we switch anything, not after we already switched. Otherwise some
> > non-HS400 communication has already happened.
> >
> > No need to convert users. There is only one currenty which needs this
> > change in a later patchset.
>
> Perhaps, should we add Fixes tag like below?
>
> Fixes: ba6c7ac3a2f4 ("mmc: core: more fine-grained hooks for HS400 tuning")

I am not sure. While it is more correct to move the call to
hs400_downgrade upwards, it does not really fix a bug on its own.
Without patch 2/2 of this series, there is not really a huge difference
when we disable the SCC the old way. For the new way, this patch is a
prerequisite.

> > + /* Prepare host to downgrade to HS timing */
> > + if (host->ops->hs400_downgrade)
> > + host->ops->hs400_downgrade(host);
> > +
>
> IICU, we should call hs400_downgrade() between the __mmc_switch("EXT_CSD_TIMING_HS")
> and mmc_set_timing(card->host, MMC_TIMING_MMC_HS) because the switch command should
> be issued in HS400 mode.

Yes, of course, you are right. Will fix!

Kind regards,

Wolfram

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]