linux-mmc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Seo, Kyungmin" <kyungmin.seo@intel.com>
To: Ulf Hansson <ulf.hansson@linaro.org>,
	"Hunter, Adrian" <adrian.hunter@intel.com>,
	Chaotian Jing <chaotian.jing@mediatek.com>
Cc: "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] mmc: mmc: Fix the timing for clock changing in mmc
Date: Tue, 10 Mar 2020 04:27:57 +0000	[thread overview]
Message-ID: <BYAPR11MB2696D160D6F5B7C98E0503E79CFF0@BYAPR11MB2696.namprd11.prod.outlook.com> (raw)
In-Reply-To: <CAPDyKFr=hE6diZmaVy-os3rFScHe+8OphBS+edkVGK+Z-J_=HA@mail.gmail.com>

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.
Of course, CMD6 with HS400 mode and 200MHz clock should not cause any problem because it's correct configuration.

Thanks

-----Original Message-----
From: Ulf Hansson <ulf.hansson@linaro.org> 
Sent: Friday, March 6, 2020 12:18 AM
To: Seo, Kyungmin <kyungmin.seo@intel.com>; Hunter, Adrian <adrian.hunter@intel.com>; Chaotian Jing <chaotian.jing@mediatek.com>
Cc: linux-mmc@vger.kernel.org; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mmc: mmc: Fix the timing for clock changing in mmc

+ Adrian, Chaotian

On Thu, 5 Mar 2020 at 09:57, Seo, Kyungmin <kyungmin.seo@intel.com> wrote:
>
> The mmc_hs400_to_hs200 function is called only in HS400 mode.
> I saw the clock change from 200MHz to 52MHz via oscilloscope on real platform.
>
> I think CMD6 is sent in HS400 mode with 200MHz clock, but it's not.
> First CMD6 in mmc_hs400_to_hs200 function is sent with 52MHz clock.

I had a vague memory that we have discussed a similar problem as your are pointing out on the mailing list already. And I was right.

Please read up on the below references, [1], [2] for the earlier discussions. I suggested a solution for Chaotian to try, but it seems like he never managed to give it a go, as I don't recall new patch being posted.

Perhaps you can pick up were Chaotian left and see if you can implement the suggested solution(s). My main concern is breaking other host drivers, as that seems quite likely to happen, if we aren't careful about this.

Kind regards
Uffe

[1]
https://lore.kernel.org/linux-mmc/1548921212-5219-1-git-send-email-chaotian.jing@mediatek.com/
[2]
https://lore.kernel.org/linux-mmc/CAPDyKFquyyXx1MqNLVXuFxcEDB9nKzN8LGGNUP2yxoVMQrWiUg@mail.gmail.com/



>
> Thanks
> KM
>
> -----Original Message-----
> From: Ulf Hansson <ulf.hansson@linaro.org>
> Sent: Wednesday, March 4, 2020 8:09 PM
> To: Seo, Kyungmin <kyungmin.seo@intel.com>
> Cc: linux-mmc@vger.kernel.org; Linux Kernel Mailing List 
> <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH] mmc: mmc: Fix the timing for clock changing in 
> mmc
>
> On Wed, 12 Feb 2020 at 03:40, Kyungmin Seo <kyungmin.seo@intel.com> wrote:
> >
> > The clock has to be changed after sending CMD6 for HS mode selection 
> > in
> > mmc_hs400_to_hs200() function.
> >
> > The JEDEC 5.0 and 5.1 said that "High-speed" mode selection has to 
> > enable the the high speed mode timing in the Device, before chaning 
> > the clock frequency to a frequency between 26MHz and 52MHz.
>
> I think that is based upon the assumption that you are using a lower frequency to start with.
>
> For example, assume that you are running with 400KHz during card initialization, then you want to send the CMD6 to switch to HS mode and that should be done, before updating the clock rate.
>
> mmc_hs400_to_hs200() goes the opposite direction, so I think the current code looks correct to me.
>
> Kind regards
> Uffe
>
> >
> > Signed-off-by: Kyungmin Seo <kyungmin.seo@intel.com>
> > ---
> >  drivers/mmc/core/mmc.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 
> > 3486bc7fbb64..98640b51c73e 100644
> > --- a/drivers/mmc/core/mmc.c
> > +++ b/drivers/mmc/core/mmc.c
> > @@ -1196,10 +1196,6 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
> >         int err;
> >         u8 val;
> >
> > -       /* Reduce frequency to HS */
> > -       max_dtr = card->ext_csd.hs_max_dtr;
> > -       mmc_set_clock(host, max_dtr);
> > -
> >         /* Switch HS400 to HS DDR */
> >         val = EXT_CSD_TIMING_HS;
> >         err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, 
> > EXT_CSD_HS_TIMING, @@ -1210,6 +1206,10 @@ int 
> > mmc_hs400_to_hs200(struct mmc_card *card)
> >
> >         mmc_set_timing(host, MMC_TIMING_MMC_DDR52);
> >
> > +       /* Reduce frequency to HS */
> > +       max_dtr = card->ext_csd.hs_max_dtr;
> > +       mmc_set_clock(host, max_dtr);
> > +
> >         err = mmc_switch_status(card);
> >         if (err)
> >                 goto out_err;
> > --
> > 2.17.1
> >

  reply	other threads:[~2020-03-10  4:28 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-12  2:42 [PATCH] mmc: mmc: Fix the timing for clock changing in mmc Kyungmin Seo
2020-03-04 11:08 ` Ulf Hansson
2020-03-05  8:57   ` Seo, Kyungmin
2020-03-05 15:17     ` Ulf Hansson
2020-03-10  4:27       ` Seo, Kyungmin [this message]
2020-03-10  9:05         ` Ulf Hansson
2020-03-10 10:44           ` Adrian Hunter
2020-03-10 15:41             ` Ulf Hansson
2020-03-11  2:13               ` Chaotian Jing
2020-03-11  8:57                 ` Ulf Hansson
2020-03-11  9:59                   ` Adrian Hunter
2020-03-11 14:06                     ` Ulf Hansson
2020-03-11 14:43                       ` Adrian Hunter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BYAPR11MB2696D160D6F5B7C98E0503E79CFF0@BYAPR11MB2696.namprd11.prod.outlook.com \
    --to=kyungmin.seo@intel.com \
    --cc=adrian.hunter@intel.com \
    --cc=chaotian.jing@mediatek.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).