All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.