linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Luca Weiss <luca@z3ntu.xyz>
To: Brian Norris <briannorris@chromium.org>,
	Ulf Hansson <ulf.hansson@linaro.org>
Cc: 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: Re: [PATCH v3] mmc: core: Set HS clock speed before sending HS CMD13
Date: Thu, 21 Apr 2022 20:46:42 +0200	[thread overview]
Message-ID: <11962455.O9o76ZdvQC@g550jk> (raw)
In-Reply-To: <CAPDyKFpQGR3ughi+6rCLUiK07Jxd5y20oK9HBjYiO-+TE8-o=Q@mail.gmail.com>

Hi Brian and Ulf,

On Mittwoch, 6. April 2022 16:55:40 CEST Ulf Hansson wrote:
> On Wed, 30 Mar 2022 at 22:30, Brian Norris <briannorris@chromium.org> wrote:
> > Way back in commit 4f25580fb84d ("mmc: core: changes frequency to
> > hs_max_dtr when selecting hs400es"), Rockchip engineers noticed that
> > some eMMC don't respond to SEND_STATUS commands very reliably if they're
> > still running at a low initial frequency. As mentioned in that commit,
> > JESD84-B51 P49 suggests a sequence in which the host:
> > 1. sets HS_TIMING
> > 2. bumps the clock ("<= 52 MHz")
> > 3. sends further commands
> > 
> > It doesn't exactly require that we don't use a lower-than-52MHz
> > frequency, but in practice, these eMMC don't like it.
> > 
> > The aforementioned commit tried to get that right for HS400ES, although
> > it's unclear whether this ever truly worked as committed into mainline,
> > as other changes/refactoring adjusted the sequence in conflicting ways:
> > 
> > 08573eaf1a70 ("mmc: mmc: do not use CMD13 to get status after speed mode
> > switch")
> > 
> > 53e60650f74e ("mmc: core: Allow CMD13 polling when switching to HS mode
> > for mmc")
> > 
> > In any case, today we do step 3 before step 2. Let's fix that, and also
> > apply the same logic to HS200/400, where this eMMC has problems too.
> > 
> > Resolves errors like this seen when booting some RK3399 Gru/Scarlet
> > systems:
> > 
> > [    2.058881] mmc1: CQHCI version 5.10
> > [    2.097545] mmc1: SDHCI controller on fe330000.mmc [fe330000.mmc] using
> > ADMA [    2.209804] mmc1: mmc_select_hs400es failed, error -84
> > [    2.215597] mmc1: error -84 whilst initialising MMC card
> > [    2.417514] mmc1: mmc_select_hs400es failed, error -110
> > [    2.423373] mmc1: error -110 whilst initialising MMC card
> > [    2.605052] mmc1: mmc_select_hs400es failed, error -110
> > [    2.617944] mmc1: error -110 whilst initialising MMC card
> > [    2.835884] mmc1: mmc_select_hs400es failed, error -110
> > [    2.841751] mmc1: error -110 whilst initialising MMC card
> > 
> > Fixes: 08573eaf1a70 ("mmc: mmc: do not use CMD13 to get status after speed
> > mode switch") Fixes: 53e60650f74e ("mmc: core: Allow CMD13 polling when
> > switching to HS mode for mmc") Fixes: 4f25580fb84d ("mmc: core: changes
> > frequency to hs_max_dtr when selecting hs400es") Cc: Shawn Lin
> > <shawn.lin@rock-chips.com>
> > Signed-off-by: Brian Norris <briannorris@chromium.org>
> 
> To get this thoroughly tested, I have applied it to my next branch, for now.
> 
> If it turns out that there are no regressions being reported, I think
> we should move the patch to the fixes branch (to get it included for
> v5.18) and then also tag it for stable. So, I will get back to this in
> a couple of weeks.

Unfortunately this patch breaks internal storage on qcom-msm8974-fairphone-fp2

With this patch (included in linux-next-20220421) it fails to initialize:

[    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

After reverting this patch, it works fine again.

[    1.908835] mmc0: SDHCI controller on f9824900.sdhci [f9824900.sdhci] using 
ADMA 64-bit
[    1.964700] mmc0: new HS200 MMC card at address 0001
[    1.965388] mmcblk0: mmc0:0001 BWBC3R 29.1 GiB 
[    1.975106]  mmcblk0: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10 p11 p12 p13 p14 p15 
p16 p17 p18 p19 p20
[    1.982545] mmcblk0boot0: mmc0:0001 BWBC3R 4.00 MiB 
[    1.988247] mmcblk0boot1: mmc0:0001 BWBC3R 4.00 MiB 
[    1.993287] mmcblk0rpmb: mmc0:0001 BWBC3R 4.00 MiB, chardev (242:0)


Regards
Luca




  reply	other threads:[~2022-04-21 18:46 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-30 20:29 [PATCH v3] mmc: core: Set HS clock speed before sending HS CMD13 Brian Norris
2022-04-06 14:55 ` Ulf Hansson
2022-04-21 18:46   ` Luca Weiss [this message]
2022-04-21 20:25     ` Brian Norris
2022-04-21 22:04       ` Luca Weiss
2022-04-22  0:18         ` [PATCH] mmc: core: Don't set HS200 clock rate prematurely Brian Norris

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=11962455.O9o76ZdvQC@g550jk \
    --to=luca@z3ntu.xyz \
    --cc=adrian.hunter@intel.com \
    --cc=alexeymin@postmarketos.org \
    --cc=briannorris@chromium.org \
    --cc=dianders@chromium.org \
    --cc=hkallweit1@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=shawn.lin@rock-chips.com \
    --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).