Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Thu, 21 Apr 2022 17:18:53 -0700
From:   Brian Norris <briannorris@chromium.org>
To:     Luca Weiss <luca@z3ntu.xyz>
Cc:     Ulf Hansson <ulf.hansson@linaro.org>,
        Adrian Hunter <adrian.hunter@intel.com>,
        Douglas Anderson <dianders@chromium.org>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Shawn Lin <shawn.lin@rock-chips.com>,
        linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org,
        alexeymin@postmarketos.org
Subject: [PATCH] mmc: core: Don't set HS200 clock rate prematurely
Message-ID: <YmH0bU5VEnaVds5H@google.com>
References: <20220330132946.v3.1.I484f4ee35609f78b932bd50feed639c29e64997e@changeid>
 <11962455.O9o76ZdvQC@g550jk>
 <YmG9sdJ8RoKH4gUS@google.com>
 <4731277.31r3eYUQgx@g550jk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4731277.31r3eYUQgx@g550jk>
Precedence: bulk

Commit 1c7ec586fe55 ("mmc: core: Set HS clock speed before sending HS
CMD13") fixes problems with certain eMMC, but introduced new ones with
others:

  qcom-msm8974-fairphone-fp2:

  [    1.868608] mmc0: SDHCI controller on f9824900.sdhci [f9824900.sdhci] using ADMA 64-bit
  [    1.925220] mmc0: mmc_select_hs200 failed, error -110
  [    1.925285] mmc0: error -110 whilst initialising MMC card

It appears we've overshot the acceptable clock rate here; while JESD84
suggests that we can bump to 52 MHz before switching (CMD6) to HS400, it
does *not* say we can switch to 200 MHz before switching to HS200 (see
page 45 / table 28). Use the HS bounds (typically 52 MHz) instead of the
HS200 bounds (which are only applicable after we successfully switch).

Link: https://lore.kernel.org/lkml/11962455.O9o76ZdvQC@g550jk/
Fixes: 1c7ec586fe55 ("mmc: core: Set HS clock speed before sending HS CMD13")
Reported-by: Luca Weiss <luca@z3ntu.xyz>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---

Hi Luca,

On Fri, Apr 22, 2022 at 12:04:01AM +0200, Luca Weiss wrote:
> It looks like with the original patch in, plus your attached patch on top it 
> seems to work as well. Thanks!

Awesome! I've written up a formal patch (surrounding this), with my best
understanding of an explanation. Thanks again.

Ulf, please let me know whether you're happier with this, or whether we
should (partially?) revert.

Brian
 
 drivers/mmc/core/mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 9ab915b5737a..f0bbac364682 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1491,7 +1491,7 @@ static int mmc_select_hs200(struct mmc_card *card)
 		old_timing = host->ios.timing;
 		old_clock = host->ios.clock;
 		mmc_set_timing(host, MMC_TIMING_MMC_HS200);
-		mmc_set_bus_speed(card);
+		mmc_set_clock(card->host, card->ext_csd.hs_max_dtr);
 
 		/*
 		 * For HS200, CRC errors are not a reliable way to know the
-- 
2.36.0.rc2.479.g8af0fa9b8e-goog