2022-04-26 12:44:03

by Srinivasarao Pathipati

[permalink] [raw]
Subject: [PATCH V1] mmc: core: Select HS mode in device first and then in the host

From: Sayali Lokhande <[email protected]>

While switching from hs400 to hs200 mode, high speed mode
timing should be selected in the device before changing the
clock frequency in the host. But current implementation,
(mmc_hs400_to_hs200) first updates the frequency in the host
and then updates mode in the device. This is a spec violation.
Hence update the sequence to comply with the spec.

Signed-off-by: Veerabhadrarao Badiganti <[email protected]>
Signed-off-by: Sayali Lokhande <[email protected]>
Signed-off-by: Kishor Krishna Bhat <[email protected]>
Signed-off-by: kamasali <[email protected]>
Signed-off-by: Srinivasarao Pathipati <[email protected]>
---
drivers/mmc/core/mmc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 8691c00..b4bcebc 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1259,10 +1259,6 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
int err;
u8 val;

- /* Reduce frequency to HS */
- max_dtr = card->ext_csd.hs_max_dtr;
- mmc_set_clock(host, max_dtr);
-
/* Switch HS400 to HS DDR */
val = EXT_CSD_TIMING_HS;
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING,
@@ -1276,6 +1272,10 @@ int mmc_hs400_to_hs200(struct mmc_card *card)

mmc_set_timing(host, MMC_TIMING_MMC_DDR52);

+ /* Reduce frequency to HS */
+ max_dtr = card->ext_csd.hs_max_dtr;
+ mmc_set_clock(host, max_dtr);
+
err = mmc_switch_status(card, true);
if (err)
goto out_err;
--
2.7.4


2022-04-27 09:18:56

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH V1] mmc: core: Select HS mode in device first and then in the host

> Hi,
>
> On Tue, Apr 26, 2022 at 2:16 AM Srinivasarao Pathipati
> <[email protected]> wrote:
> >
> > From: Sayali Lokhande <[email protected]>
> >
> > While switching from hs400 to hs200 mode, high speed mode
> > timing should be selected in the device before changing the
> > clock frequency in the host. But current implementation,
> > (mmc_hs400_to_hs200) first updates the frequency in the host
> > and then updates mode in the device. This is a spec violation.
> > Hence update the sequence to comply with the spec.
>
> I'm a bit new to interpreting eMMC specs, but are you sure this is a
> violation? In JESD84-B51, I see:
>
> "The bus frequency can be changed at any time (under the restrictions
> of maximum data transfer frequency, defined by the Device, and the
> identification frequency defined by the standard document)."
>
> I think that suggests we can lower the host clock first, and then
> lower the device timing. And (according to my limited knowledge) that
> makes sense too: the device timing is a "maximum" (to some extent) and
> we're free to run the host bus somewhat slower.
>
> And on the flip side: it sounds like you may be _introducing_ a spec
> violation (that we'll be running the host faster than the device
> timing, briefly)?
Ack on that.

Also, specifically, please refer to Figure 28 — HS200 Selection flow diagram.
You can see that the flow goes though:
BUS_WIDTH [183] -> from 0x06 (DDR 8bit) to 0x02 (SDR 8bit)
HS_TIMING [185] -> from 0x03(HS400) to 0x02 (HS200)
Host may changes the frequency , but it’s no needed as HS400 and HS200 use same CLK

Thanks,
Avri
>
> Apologies if I'm off base. But you did CC me ;)
>
> Regards,
> Brian

2022-04-27 11:20:18

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH V1] mmc: core: Select HS mode in device first and then in the host

Hi,

On Tue, Apr 26, 2022 at 2:16 AM Srinivasarao Pathipati
<[email protected]> wrote:
>
> From: Sayali Lokhande <[email protected]>
>
> While switching from hs400 to hs200 mode, high speed mode
> timing should be selected in the device before changing the
> clock frequency in the host. But current implementation,
> (mmc_hs400_to_hs200) first updates the frequency in the host
> and then updates mode in the device. This is a spec violation.
> Hence update the sequence to comply with the spec.

I'm a bit new to interpreting eMMC specs, but are you sure this is a
violation? In JESD84-B51, I see:

"The bus frequency can be changed at any time (under the restrictions
of maximum data transfer frequency, defined by the Device, and the
identification frequency defined by the standard document)."

I think that suggests we can lower the host clock first, and then
lower the device timing. And (according to my limited knowledge) that
makes sense too: the device timing is a "maximum" (to some extent) and
we're free to run the host bus somewhat slower.

And on the flip side: it sounds like you may be _introducing_ a spec
violation (that we'll be running the host faster than the device
timing, briefly)?

Apologies if I'm off base. But you did CC me ;)

Regards,
Brian