[PATCH] mmc: core: Set HS clock speed before sending HS CMD13

[PATCH] mmc: core: Set HS clock speed before sending HS CMD13

[Brian Norris]

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.

Anyway, the aforementioned commit got that right for HS400ES, but the
refactoring in 53e60650f74e ("mmc: core: Allow CMD13 polling when
switching to HS mode for mmc") messed that back up again, by reordering
step 2 after step 3.

Let's fix that, and also apply the same logic to HS200/400, where this
eMMC has problems too.

This 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: 53e60650f74e ("mmc: core: Allow CMD13 polling when switching to HS mode for mmc")
Cc: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---

 drivers/mmc/core/mmc.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 13abfcd130a5..821b90caba2f 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1390,12 +1390,17 @@ static int mmc_select_hs400es(struct mmc_card *card)
 	}
 
 	mmc_set_timing(host, MMC_TIMING_MMC_HS);
+
+	/*
+	 * Bump to HS frequency. Some cards don't handle SEND_STATUS reliably
+	 * at the initial frequency.
+	 */
+	mmc_set_clock(host, card->ext_csd.hs_max_dtr);
+
 	err = mmc_switch_status(card, true);
 	if (err)
 		goto out_err;
 
-	mmc_set_clock(host, card->ext_csd.hs_max_dtr);
-
 	/* Switch card to DDR with strobe bit */
 	val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE;
 	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
@@ -1487,6 +1492,12 @@ static int mmc_select_hs200(struct mmc_card *card)
 		old_timing = host->ios.timing;
 		mmc_set_timing(host, MMC_TIMING_MMC_HS200);
 
+		/*
+		 * Bump to HS frequency. Some cards don't handle SEND_STATUS
+		 * reliably at the initial frequency.
+		 */
+		mmc_set_clock(host, card->ext_csd.hs_max_dtr);
+
 		/*
 		 * For HS200, CRC errors are not a reliable way to know the
 		 * switch failed. If there really is a problem, we would expect
-- 
2.35.1.723.g4982287a31-goog

Re: [PATCH] mmc: core: Set HS clock speed before sending HS CMD13

[Adrian Hunter]

On 10.3.2022 22.56, Brian Norris 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.
> 
> Anyway, the aforementioned commit got that right for HS400ES, but the
> refactoring in 53e60650f74e ("mmc: core: Allow CMD13 polling when
> switching to HS mode for mmc") messed that back up again, by reordering
> step 2 after step 3.

That description might not be accurate.

It looks like 4f25580fb84d did not have the intended effect because
CMD13 was already being sent by mmc_select_hs(), still before increasing
the frequency.  53e60650f74e just kept that behaviour.

> 
> Let's fix that, and also apply the same logic to HS200/400, where this
> eMMC has problems too.
> 
> This 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: 53e60650f74e ("mmc: core: Allow CMD13 polling when switching to HS mode for mmc")
> Cc: Shawn Lin <shawn.lin@rock-chips.com>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> 
>  drivers/mmc/core/mmc.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 13abfcd130a5..821b90caba2f 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1390,12 +1390,17 @@ static int mmc_select_hs400es(struct mmc_card *card)
>  	}
>  
>  	mmc_set_timing(host, MMC_TIMING_MMC_HS);
> +
> +	/*
> +	 * Bump to HS frequency. Some cards don't handle SEND_STATUS reliably
> +	 * at the initial frequency.
> +	 */
> +	mmc_set_clock(host, card->ext_csd.hs_max_dtr);
> +
>  	err = mmc_switch_status(card, true);
>  	if (err)
>  		goto out_err;
>  
> -	mmc_set_clock(host, card->ext_csd.hs_max_dtr);
> -
>  	/* Switch card to DDR with strobe bit */
>  	val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE;
>  	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> @@ -1487,6 +1492,12 @@ static int mmc_select_hs200(struct mmc_card *card)
>  		old_timing = host->ios.timing;
>  		mmc_set_timing(host, MMC_TIMING_MMC_HS200);
>  
> +		/*
> +		 * Bump to HS frequency. Some cards don't handle SEND_STATUS
> +		 * reliably at the initial frequency.
> +		 */
> +		mmc_set_clock(host, card->ext_csd.hs_max_dtr);

Is card->ext_csd.hs_max_dtr better than card->ext_csd.hs200_max_dtr here?

> +
>  		/*
>  		 * For HS200, CRC errors are not a reliable way to know the
>  		 * switch failed. If there really is a problem, we would expect

Re: [PATCH] mmc: core: Set HS clock speed before sending HS CMD13

[Brian Norris]

On Mon, Mar 14, 2022 at 6:13 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 10.3.2022 22.56, Brian Norris 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.
> >
> > Anyway, the aforementioned commit got that right for HS400ES, but the
> > refactoring in 53e60650f74e ("mmc: core: Allow CMD13 polling when
> > switching to HS mode for mmc") messed that back up again, by reordering
> > step 2 after step 3.
>
> That description might not be accurate.

I've been struggling to track where things were working, where things
were broken, and what/why Shawn's original fix was, precisely. So you
may be correct in many ways :) Thanks for looking.

> It looks like 4f25580fb84d did not have the intended effect because
> CMD13 was already being sent by mmc_select_hs(), still before increasing
> the frequency.  53e60650f74e just kept that behaviour.

You may be partially right, or fully right. But anyway, I think I have
some additional explanation, now that you've pointed that out: that
behavior changed a bit in this commit:

08573eaf1a70 mmc: mmc: do not use CMD13 to get status after speed mode switch

While that patch was merged in July 2016 and Shawn submitted his v1
fix in September, there's a very good chance that a lot of his work
was actually done via backports, and even if not, he may not have been
testing precisely the latest -next kernel when submitting. So his fix
may have worked out for _some_ near-upstream kernel he was testing in
2016, you may be correct that it didn't really work in the state it
was committed to git history.

This may also further explain why my attempts at bisection were rather
fruitless (notwithstanding the difficulties in getting RK3399 running
on that old of a kernel).

Anyway, I'll see if I can improve the messaging if/when a v2 comes around.

> > --- a/drivers/mmc/core/mmc.c
> > +++ b/drivers/mmc/core/mmc.c
...
> > @@ -1487,6 +1492,12 @@ static int mmc_select_hs200(struct mmc_card *card)
> >               old_timing = host->ios.timing;
> >               mmc_set_timing(host, MMC_TIMING_MMC_HS200);
> >
> > +             /*
> > +              * Bump to HS frequency. Some cards don't handle SEND_STATUS
> > +              * reliably at the initial frequency.
> > +              */
> > +             mmc_set_clock(host, card->ext_csd.hs_max_dtr);
>
> Is card->ext_csd.hs_max_dtr better than card->ext_csd.hs200_max_dtr here?

I believe either worked in practice. I ended up choosing hs_max_dtr
because it's lower and presumably safer. But frankly, I don't know
what the Right thing to do is here, since the spec just talks about
"<=", and yet f_init (which is also "<=") does not work. I think it
might be like Ulf was guessing way back in the first place [1], and
the key is that there is *some* increase (i.e., not using f_init).

So assuming either works, would you prefer hs200_max_dtr here, since
that does seem like the appropriate final rate?

Brian

[1] https://lore.kernel.org/all/CAPDyKFrNp=Y3BhVE_kxtggv7Qc6m=2kef2U8Dn2Bb3ANHPYV-Q@mail.gmail.com/
Re: [PATCH 3/5] mmc: core: changes frequency to hs_max_dtr when
selecting hs400es

Re: [PATCH] mmc: core: Set HS clock speed before sending HS CMD13

[Ulf Hansson]

+ Heiner

On Tue, 15 Mar 2022 at 00:11, Brian Norris <briannorris@chromium.org> wrote:
>
> On Mon, Mar 14, 2022 at 6:13 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
> >
> > On 10.3.2022 22.56, Brian Norris 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.
> > >
> > > Anyway, the aforementioned commit got that right for HS400ES, but the
> > > refactoring in 53e60650f74e ("mmc: core: Allow CMD13 polling when
> > > switching to HS mode for mmc") messed that back up again, by reordering
> > > step 2 after step 3.
> >
> > That description might not be accurate.
>
> I've been struggling to track where things were working, where things
> were broken, and what/why Shawn's original fix was, precisely. So you
> may be correct in many ways :) Thanks for looking.
>
> > It looks like 4f25580fb84d did not have the intended effect because
> > CMD13 was already being sent by mmc_select_hs(), still before increasing
> > the frequency.  53e60650f74e just kept that behaviour.
>
> You may be partially right, or fully right. But anyway, I think I have
> some additional explanation, now that you've pointed that out: that
> behavior changed a bit in this commit:
>
> 08573eaf1a70 mmc: mmc: do not use CMD13 to get status after speed mode switch
>
> While that patch was merged in July 2016 and Shawn submitted his v1
> fix in September, there's a very good chance that a lot of his work
> was actually done via backports, and even if not, he may not have been
> testing precisely the latest -next kernel when submitting. So his fix
> may have worked out for _some_ near-upstream kernel he was testing in
> 2016, you may be correct that it didn't really work in the state it
> was committed to git history.
>
> This may also further explain why my attempts at bisection were rather
> fruitless (notwithstanding the difficulties in getting RK3399 running
> on that old of a kernel).
>
> Anyway, I'll see if I can improve the messaging if/when a v2 comes around.
>
> > > --- a/drivers/mmc/core/mmc.c
> > > +++ b/drivers/mmc/core/mmc.c
> ...
> > > @@ -1487,6 +1492,12 @@ static int mmc_select_hs200(struct mmc_card *card)
> > >               old_timing = host->ios.timing;
> > >               mmc_set_timing(host, MMC_TIMING_MMC_HS200);
> > >
> > > +             /*
> > > +              * Bump to HS frequency. Some cards don't handle SEND_STATUS
> > > +              * reliably at the initial frequency.
> > > +              */
> > > +             mmc_set_clock(host, card->ext_csd.hs_max_dtr);
> >
> > Is card->ext_csd.hs_max_dtr better than card->ext_csd.hs200_max_dtr here?
>
> I believe either worked in practice. I ended up choosing hs_max_dtr
> because it's lower and presumably safer. But frankly, I don't know
> what the Right thing to do is here, since the spec just talks about
> "<=", and yet f_init (which is also "<=") does not work. I think it
> might be like Ulf was guessing way back in the first place [1], and
> the key is that there is *some* increase (i.e., not using f_init).
>
> So assuming either works, would you prefer hs200_max_dtr here, since
> that does seem like the appropriate final rate?

I think that makes most sense, as we are switching to that rate anyway
just a few cycles later in mmc_select_timing(), when it calls
mmc_set_bus_speed().

That said, I have recently queued a patch that improves the
speed-mode-selection-fallback, when switching to HS200 mode fails [2].
We need to make sure this part still works as expected. I have looped
in Heiner who has been in the loop around this change, hopefully he
can help with further testing or so. Maybe $subject patch (or a new
version of it) can even make HS200 to work on Heiner's platform!?

>
> Brian
>
> [1] https://lore.kernel.org/all/CAPDyKFrNp=Y3BhVE_kxtggv7Qc6m=2kef2U8Dn2Bb3ANHPYV-Q@mail.gmail.com/
> Re: [PATCH 3/5] mmc: core: changes frequency to hs_max_dtr when
> selecting hs400es

Kind regards
Uffe

[2]
https://patchwork.kernel.org/project/linux-mmc/patch/20220303164522.129583-1-ulf.hansson@linaro.org/

Re: [PATCH] mmc: core: Set HS clock speed before sending HS CMD13

[Heiner Kallweit]

On 15.03.2022 13:27, Ulf Hansson wrote:
> + Heiner
> 
> On Tue, 15 Mar 2022 at 00:11, Brian Norris <briannorris@chromium.org> wrote:
>>
>> On Mon, Mar 14, 2022 at 6:13 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>
>>> On 10.3.2022 22.56, Brian Norris 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.
>>>>
>>>> Anyway, the aforementioned commit got that right for HS400ES, but the
>>>> refactoring in 53e60650f74e ("mmc: core: Allow CMD13 polling when
>>>> switching to HS mode for mmc") messed that back up again, by reordering
>>>> step 2 after step 3.
>>>
>>> That description might not be accurate.
>>
>> I've been struggling to track where things were working, where things
>> were broken, and what/why Shawn's original fix was, precisely. So you
>> may be correct in many ways :) Thanks for looking.
>>
>>> It looks like 4f25580fb84d did not have the intended effect because
>>> CMD13 was already being sent by mmc_select_hs(), still before increasing
>>> the frequency.  53e60650f74e just kept that behaviour.
>>
>> You may be partially right, or fully right. But anyway, I think I have
>> some additional explanation, now that you've pointed that out: that
>> behavior changed a bit in this commit:
>>
>> 08573eaf1a70 mmc: mmc: do not use CMD13 to get status after speed mode switch
>>
>> While that patch was merged in July 2016 and Shawn submitted his v1
>> fix in September, there's a very good chance that a lot of his work
>> was actually done via backports, and even if not, he may not have been
>> testing precisely the latest -next kernel when submitting. So his fix
>> may have worked out for _some_ near-upstream kernel he was testing in
>> 2016, you may be correct that it didn't really work in the state it
>> was committed to git history.
>>
>> This may also further explain why my attempts at bisection were rather
>> fruitless (notwithstanding the difficulties in getting RK3399 running
>> on that old of a kernel).
>>
>> Anyway, I'll see if I can improve the messaging if/when a v2 comes around.
>>
>>>> --- a/drivers/mmc/core/mmc.c
>>>> +++ b/drivers/mmc/core/mmc.c
>> ...
>>>> @@ -1487,6 +1492,12 @@ static int mmc_select_hs200(struct mmc_card *card)
>>>>               old_timing = host->ios.timing;
>>>>               mmc_set_timing(host, MMC_TIMING_MMC_HS200);
>>>>
>>>> +             /*
>>>> +              * Bump to HS frequency. Some cards don't handle SEND_STATUS
>>>> +              * reliably at the initial frequency.
>>>> +              */
>>>> +             mmc_set_clock(host, card->ext_csd.hs_max_dtr);
>>>
>>> Is card->ext_csd.hs_max_dtr better than card->ext_csd.hs200_max_dtr here?
>>
>> I believe either worked in practice. I ended up choosing hs_max_dtr
>> because it's lower and presumably safer. But frankly, I don't know
>> what the Right thing to do is here, since the spec just talks about
>> "<=", and yet f_init (which is also "<=") does not work. I think it
>> might be like Ulf was guessing way back in the first place [1], and
>> the key is that there is *some* increase (i.e., not using f_init).
>>
>> So assuming either works, would you prefer hs200_max_dtr here, since
>> that does seem like the appropriate final rate?
> 
> I think that makes most sense, as we are switching to that rate anyway
> just a few cycles later in mmc_select_timing(), when it calls
> mmc_set_bus_speed().
> 
> That said, I have recently queued a patch that improves the
> speed-mode-selection-fallback, when switching to HS200 mode fails [2].
> We need to make sure this part still works as expected. I have looped
> in Heiner who has been in the loop around this change, hopefully he
> can help with further testing or so. Maybe $subject patch (or a new
> version of it) can even make HS200 to work on Heiner's platform!?
> 
>>
>> Brian
>>
>> [1] https://lore.kernel.org/all/CAPDyKFrNp=Y3BhVE_kxtggv7Qc6m=2kef2U8Dn2Bb3ANHPYV-Q@mail.gmail.com/
>> Re: [PATCH 3/5] mmc: core: changes frequency to hs_max_dtr when
>> selecting hs400es
> 
> Kind regards
> Uffe
> 
> [2]
> https://patchwork.kernel.org/project/linux-mmc/patch/20220303164522.129583-1-ulf.hansson@linaro.org/

In my specific case this patch makes no difference. My test system is a
dirt-cheap Amlogic SoC based Android TV box. My best guess is that maybe due
to chip shortage the vendor omitted some regulator, making the eMMC card
refuse the switch to HS200.
Therefore my test result doesn't speak against the proposed patch.

Re: [PATCH] mmc: core: Set HS clock speed before sending HS CMD13

[Ulf Hansson]

On Wed, 16 Mar 2022 at 22:57, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On 15.03.2022 13:27, Ulf Hansson wrote:
> > + Heiner
> >
> > On Tue, 15 Mar 2022 at 00:11, Brian Norris <briannorris@chromium.org> wrote:
> >>
> >> On Mon, Mar 14, 2022 at 6:13 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
> >>>
> >>> On 10.3.2022 22.56, Brian Norris 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.
> >>>>
> >>>> Anyway, the aforementioned commit got that right for HS400ES, but the
> >>>> refactoring in 53e60650f74e ("mmc: core: Allow CMD13 polling when
> >>>> switching to HS mode for mmc") messed that back up again, by reordering
> >>>> step 2 after step 3.
> >>>
> >>> That description might not be accurate.
> >>
> >> I've been struggling to track where things were working, where things
> >> were broken, and what/why Shawn's original fix was, precisely. So you
> >> may be correct in many ways :) Thanks for looking.
> >>
> >>> It looks like 4f25580fb84d did not have the intended effect because
> >>> CMD13 was already being sent by mmc_select_hs(), still before increasing
> >>> the frequency.  53e60650f74e just kept that behaviour.
> >>
> >> You may be partially right, or fully right. But anyway, I think I have
> >> some additional explanation, now that you've pointed that out: that
> >> behavior changed a bit in this commit:
> >>
> >> 08573eaf1a70 mmc: mmc: do not use CMD13 to get status after speed mode switch
> >>
> >> While that patch was merged in July 2016 and Shawn submitted his v1
> >> fix in September, there's a very good chance that a lot of his work
> >> was actually done via backports, and even if not, he may not have been
> >> testing precisely the latest -next kernel when submitting. So his fix
> >> may have worked out for _some_ near-upstream kernel he was testing in
> >> 2016, you may be correct that it didn't really work in the state it
> >> was committed to git history.
> >>
> >> This may also further explain why my attempts at bisection were rather
> >> fruitless (notwithstanding the difficulties in getting RK3399 running
> >> on that old of a kernel).
> >>
> >> Anyway, I'll see if I can improve the messaging if/when a v2 comes around.
> >>
> >>>> --- a/drivers/mmc/core/mmc.c
> >>>> +++ b/drivers/mmc/core/mmc.c
> >> ...
> >>>> @@ -1487,6 +1492,12 @@ static int mmc_select_hs200(struct mmc_card *card)
> >>>>               old_timing = host->ios.timing;
> >>>>               mmc_set_timing(host, MMC_TIMING_MMC_HS200);
> >>>>
> >>>> +             /*
> >>>> +              * Bump to HS frequency. Some cards don't handle SEND_STATUS
> >>>> +              * reliably at the initial frequency.
> >>>> +              */
> >>>> +             mmc_set_clock(host, card->ext_csd.hs_max_dtr);
> >>>
> >>> Is card->ext_csd.hs_max_dtr better than card->ext_csd.hs200_max_dtr here?
> >>
> >> I believe either worked in practice. I ended up choosing hs_max_dtr
> >> because it's lower and presumably safer. But frankly, I don't know
> >> what the Right thing to do is here, since the spec just talks about
> >> "<=", and yet f_init (which is also "<=") does not work. I think it
> >> might be like Ulf was guessing way back in the first place [1], and
> >> the key is that there is *some* increase (i.e., not using f_init).
> >>
> >> So assuming either works, would you prefer hs200_max_dtr here, since
> >> that does seem like the appropriate final rate?
> >
> > I think that makes most sense, as we are switching to that rate anyway
> > just a few cycles later in mmc_select_timing(), when it calls
> > mmc_set_bus_speed().
> >
> > That said, I have recently queued a patch that improves the
> > speed-mode-selection-fallback, when switching to HS200 mode fails [2].
> > We need to make sure this part still works as expected. I have looped
> > in Heiner who has been in the loop around this change, hopefully he
> > can help with further testing or so. Maybe $subject patch (or a new
> > version of it) can even make HS200 to work on Heiner's platform!?
> >
> >>
> >> Brian
> >>
> >> [1] https://lore.kernel.org/all/CAPDyKFrNp=Y3BhVE_kxtggv7Qc6m=2kef2U8Dn2Bb3ANHPYV-Q@mail.gmail.com/
> >> Re: [PATCH 3/5] mmc: core: changes frequency to hs_max_dtr when
> >> selecting hs400es
> >
> > Kind regards
> > Uffe
> >
> > [2]
> > https://patchwork.kernel.org/project/linux-mmc/patch/20220303164522.129583-1-ulf.hansson@linaro.org/
>
> In my specific case this patch makes no difference. My test system is a
> dirt-cheap Amlogic SoC based Android TV box. My best guess is that maybe due
> to chip shortage the vendor omitted some regulator, making the eMMC card
> refuse the switch to HS200.
> Therefore my test result doesn't speak against the proposed patch.

Thanks Heiner for confirming!

Brian, I expect you to send an updated/rebased patch that we can test
and review.

Kind regards
Uffe