From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alan Cooper Subject: Re: [PATCH] mmc: sdhci: Fix incorrect switch to HS mode Date: Thu, 19 Sep 2019 07:57:45 -0400 Message-ID: References: <20190903115114.33053-1-alcooperx@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Ulf Hansson Cc: Linux Kernel Mailing List , Adrian Hunter , "linux-mmc@vger.kernel.org" List-Id: linux-mmc@vger.kernel.org This does correct the sequence of switching to HS400 but it might be safest to just add this to the latest until it gets a little testing to make sure it doesn't expose some bug in existing controllers. Thanks Al On Tue, Sep 3, 2019 at 10:52 AM Ulf Hansson wrote: > > On Tue, 3 Sep 2019 at 13:51, Al Cooper wrote: > > > > When switching from any MMC speed mode that requires 1.8v > > (HS200, HS400 and HS400ES) to High Speed (HS) mode, the system > > ends up configured for SDR12 with a 50MHz clock which is an illegal > > mode. > > > > This happens because the SDHCI_CTRL_VDD_180 bit in the > > SDHCI_HOST_CONTROL2 register is left set and when this bit is > > set, the speed mode is controlled by the SDHCI_CTRL_UHS field > > in the SDHCI_HOST_CONTROL2 register. The SDHCI_CTRL_UHS field > > will end up being set to 0 (SDR12) by sdhci_set_uhs_signaling() > > because there is no UHS mode being set. > > > > The fix is to change sdhci_set_uhs_signaling() to set the > > SDHCI_CTRL_UHS field to SDR25 (which is the same as HS) for > > any switch to HS mode. > > > > This was found on a new eMMC controller that does strict checking > > of the speed mode and the corresponding clock rate. It caused the > > switch to HS400 mode to fail because part of the sequence to switch > > to HS400 requires a switch from HS200 to HS before going to HS400. > > > > This fix was suggested by Adrian Hunter > > > > Signed-off-by: Al Cooper > > Should this be applied for fixes and tagged for stable you think? > > Kind regards > Uffe > > > --- > > drivers/mmc/host/sdhci.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > > index 61d845fe0b97..068149640ecd 100644 > > --- a/drivers/mmc/host/sdhci.c > > +++ b/drivers/mmc/host/sdhci.c > > @@ -1858,7 +1858,9 @@ void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing) > > ctrl_2 |= SDHCI_CTRL_UHS_SDR104; > > else if (timing == MMC_TIMING_UHS_SDR12) > > ctrl_2 |= SDHCI_CTRL_UHS_SDR12; > > - else if (timing == MMC_TIMING_UHS_SDR25) > > + else if (timing == MMC_TIMING_SD_HS || > > + timing == MMC_TIMING_MMC_HS || > > + timing == MMC_TIMING_UHS_SDR25) > > ctrl_2 |= SDHCI_CTRL_UHS_SDR25; > > else if (timing == MMC_TIMING_UHS_SDR50) > > ctrl_2 |= SDHCI_CTRL_UHS_SDR50; > > -- > > 2.17.1 > >