Skip to contentSkip to conversation

mmc: core: Set clock before switching to highspeed mode.

RFC In review Yousong Zhou 6 participants 13 messages Sep 5, 2015 – Sep 9, 2015 6 messages not in the archive

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

RFC 1/1 mmc: core: Set clock before switching to highspeed mode.

A SD card with sunxi-mmc can fail with the following error message (RCD for response CRC error) when trying to switch to highspeed mode. Setting the bus clock before the access mode switch fixed it.

    [    1.112060] mmc0: host does not support reading read-only switch, assuming write-enable
    [    1.120203] ehci-platform 1c1c000.usb: irq 31, io mem 0x01c1c000
    [    1.126527] sunxi-mmc 1c0f000.mmc: smc 0 err, cmd 6, RD RCE !!
    [    1.132388] sunxi-mmc 1c0f000.mmc: data error, sending stop command
    [    1.139451] sunxi-mmc 1c0f000.mmc: send stop command failed
    [    1.145056] mmc0: error -110 whilst initialising SD card
    [    1.150424] ehci-platform 1c1c000.usb: USB 2.0 started, EHCI 1.00
    [    1.156533] sunxi-mmc 1c0f000.mmc: smc 0 err, cmd 1, RTO !!
---
 drivers/mmc/core/sd.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/mmc/core/sd.c +5 -5
index 4e7366a..402a8db 100644
@@ -366,6 +366,11 @@ int mmc_sd_switch_hs(struct mmc_card *card)
 		return -ENOMEM;
 	}
 
+	/*
+	 * Set bus frequency to match highspeed mode.
+	 */
+	mmc_set_clock(card->host, mmc_sd_get_max_clock(card));
+
 	err = mmc_sd_switch(card, 1, 0, 1, status);
 	if (err)
 		goto out;
@@ -969,11 +974,6 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
 			goto free_card;
 
 		/*
-		 * Set bus speed.
-		 */
-		mmc_set_clock(host, mmc_sd_get_max_clock(card));
-
-		/*
 		 * Switch to wider bus (if supported).
 		 */
 		if ((host->caps & MMC_CAP_4_BIT_DATA) &&
Signature · 1 line
1.7.10.4

A message that is not in the archive (sent off-list or not yet received): 1441448358-13129-1-git-send-email-yszhou4tech-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

#2 lore raw
Quoted 3 lines · Yousong Zhou

A SD card with sunxi-mmc can fail with the following error message (RCD for response CRC error) when trying to switch to highspeed mode. Setting the bus clock before the access mode switch fixed it.

No, that's wrong!

Before this card is switched to highspeed, it works under identification mode(From spec: bus clock <= 400KHz). How could you raise bus clock to higher clk rate which I _guess_ is 52MHz before you notify sd cards to run into highspeed mode?

Althought it works for this card, this patch will not please the other cards that they might not reply CMDs any longer including the following CMD6.

Quoted 44 lines
     [    1.112060] mmc0: host does not support reading read-only switch, assuming write-enable
     [    1.120203] ehci-platform 1c1c000.usb: irq 31, io mem 0x01c1c000
     [    1.126527] sunxi-mmc 1c0f000.mmc: smc 0 err, cmd 6, RD RCE !!
     [    1.132388] sunxi-mmc 1c0f000.mmc: data error, sending stop command
     [    1.139451] sunxi-mmc 1c0f000.mmc: send stop command failed
     [    1.145056] mmc0: error -110 whilst initialising SD card
     [    1.150424] ehci-platform 1c1c000.usb: USB 2.0 started, EHCI 1.00
     [    1.156533] sunxi-mmc 1c0f000.mmc: smc 0 err, cmd 1, RTO !!
---
  drivers/mmc/core/sd.c |   10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)
drivers/mmc/core/sd.c +5 -5
index 4e7366a..402a8db 100644
@@ -366,6 +366,11 @@ int mmc_sd_switch_hs(struct mmc_card *card)
  		return -ENOMEM;
  	}
 
+	/*
+	 * Set bus frequency to match highspeed mode.
+	 */
+	mmc_set_clock(card->host, mmc_sd_get_max_clock(card));
+
  	err = mmc_sd_switch(card, 1, 0, 1, status);
  	if (err)
  		goto out;
@@ -969,11 +974,6 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
  			goto free_card;
 
  		/*
-		 * Set bus speed.
-		 */
-		mmc_set_clock(host, mmc_sd_get_max_clock(card));
-
-		/*
  		 * Switch to wider bus (if supported).
  		 */
  		if ((host->caps & MMC_CAP_4_BIT_DATA) &&
Signature · 2 lines
Best Regards
Shawn Lin
#3 lore raw

Hi Shawn Lin,

Quoted 11 lines · Shawn Lin
Quoted 3 lines · Yousong Zhou

A SD card with sunxi-mmc can fail with the following error message (RCD for response CRC error) when trying to switch to highspeed mode. Setting the bus clock before the access mode switch fixed it.

No, that's wrong!

Before this card is switched to highspeed, it works under identification mode(From spec: bus clock <= 400KHz). How could you raise bus clock to higher clk rate which I _guess_ is 52MHz before you notify sd cards to run into highspeed mode?

Althought it works for this card, this patch will not please the other cards that they might not reply CMDs any longer including the following CMD6.

Thanks for the feedback, this is exactly why I asked Yousong Zhou to take this to the mmc list.

So if this is not the proper fix for the problem that Yousong Zhou is seeing, then what might be the proper fix ?

Could it be that the sunxi-mmc is doing some things in the wrong order when changing the clock, or is this all under control of the mmc core ?

Regards,

Hans

Quoted 48 lines
Quoted 46 lines


     [    1.112060] mmc0: host does not support reading read-only switch, assuming write-enable
     [    1.120203] ehci-platform 1c1c000.usb: irq 31, io mem 0x01c1c000
     [    1.126527] sunxi-mmc 1c0f000.mmc: smc 0 err, cmd 6, RD RCE !!
     [    1.132388] sunxi-mmc 1c0f000.mmc: data error, sending stop command
     [    1.139451] sunxi-mmc 1c0f000.mmc: send stop command failed
     [    1.145056] mmc0: error -110 whilst initialising SD card
     [    1.150424] ehci-platform 1c1c000.usb: USB 2.0 started, EHCI 1.00
     [    1.156533] sunxi-mmc 1c0f000.mmc: smc 0 err, cmd 1, RTO !!
---
  drivers/mmc/core/sd.c |   10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)
drivers/mmc/core/sd.c +5 -5
index 4e7366a..402a8db 100644
@@ -366,6 +366,11 @@ int mmc_sd_switch_hs(struct mmc_card *card)
          return -ENOMEM;
      }
 
+    /*
+     * Set bus frequency to match highspeed mode.
+     */
+    mmc_set_clock(card->host, mmc_sd_get_max_clock(card));
+
      err = mmc_sd_switch(card, 1, 0, 1, status);
      if (err)
          goto out;
@@ -969,11 +974,6 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
              goto free_card;
 
          /*
-         * Set bus speed.
-         */
-        mmc_set_clock(host, mmc_sd_get_max_clock(card));
-
-        /*
           * Switch to wider bus (if supported).
           */
          if ((host->caps & MMC_CAP_4_BIT_DATA) &&

A message that is not in the archive (sent off-list or not yet received): [email protected]

#5 lore raw
Quoted 21 lines · Hans de Goede

Hi Shawn Lin,

Quoted 9 lines · Shawn Lin
Quoted 5 lines · Yousong Zhou

A SD card with sunxi-mmc can fail with the following error message (RCD for
response CRC error) when trying to switch to highspeed mode. Setting the bus
clock before the access mode switch fixed it.

No, that's wrong!

Quoted 8 lines

Before this card is switched to highspeed, it works under identification mode(From spec: bus clock <= 400KHz). How could you raise bus clock to higher clk rate which I _guess_ is 52MHz before you notify sd cards to run into highspeed mode?

Althought it works for this card, this patch will not please the other cards that they might not reply CMDs any longer including the following CMD6.

Quoted 8 lines

Thanks for the feedback, this is exactly why I asked Yousong Zhou to take this
to the mmc list.

So if this is not the proper fix for the problem that Yousong Zhou is seeing, then
what might be the proper fix ?

From my knowledge of mmc, there hadn't have a way to deal with this "broken" case. In another word, IMO,it's ANTI-SPEC. We can't be too spec sometimes, but at least we shouldn't violate it.

Quoted 3 lines

Could it be that the sunxi-mmc is doing some things in the wrong order when changing the clock, or is this all under control of the mmc core ?

all of this is under control of the mmc core.
So if Yongsong does want this card to work for any linux-based mmc stack, I guess something like that should be "better"?

if (switch to HS fail) {
set_bus_clk
goto retry switch to HS
}

BUT...I admit it seems strange as well.

Quoted 60 lines

Regards,

Hans

Quoted 52 lines
Quoted 50 lines


     [    1.112060] mmc0: host does not support reading read-only
switch, assuming write-enable
     [    1.120203] ehci-platform 1c1c000.usb: irq 31, io mem 0x01c1c000
     [    1.126527] sunxi-mmc 1c0f000.mmc: smc 0 err, cmd 6, RD RCE !!
     [    1.132388] sunxi-mmc 1c0f000.mmc: data error, sending stop
command
     [    1.139451] sunxi-mmc 1c0f000.mmc: send stop command failed
     [    1.145056] mmc0: error -110 whilst initialising SD card
     [    1.150424] ehci-platform 1c1c000.usb: USB 2.0 started, EHCI
1.00
     [    1.156533] sunxi-mmc 1c0f000.mmc: smc 0 err, cmd 1, RTO !!
---
  drivers/mmc/core/sd.c |   10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)
drivers/mmc/core/sd.c +5 -5
index 4e7366a..402a8db 100644
@@ -366,6 +366,11 @@ int mmc_sd_switch_hs(struct mmc_card *card)
          return -ENOMEM;
      }
 
+    /*
+     * Set bus frequency to match highspeed mode.
+     */
+    mmc_set_clock(card->host, mmc_sd_get_max_clock(card));
+
      err = mmc_sd_switch(card, 1, 0, 1, status);
      if (err)
          goto out;
@@ -969,11 +974,6 @@ static int mmc_sd_init_card(struct mmc_host
 *host, u32 ocr,
              goto free_card;
 
          /*
-         * Set bus speed.
-         */
-        mmc_set_clock(host, mmc_sd_get_max_clock(card));
-
-        /*
           * Switch to wider bus (if supported).
           */
          if ((host->caps & MMC_CAP_4_BIT_DATA) &&
Signature · 2 lines
Best Regards
Shawn Lin
#6 lore raw

Hi,

Quoted 31 lines · Shawn Lin
Quoted 30 lines · Hans de Goede

Hi Shawn Lin,

Quoted 12 lines · Shawn Lin
Quoted 6 lines · Yousong Zhou

A SD card with sunxi-mmc can fail with the following error message (RCD for
response CRC error) when trying to switch to highspeed mode. Setting the bus
clock before the access mode switch fixed it.

No, that's wrong!

Quoted 8 lines

Before this card is switched to highspeed, it works under identification mode(From spec: bus clock <= 400KHz). How could you raise bus clock to higher clk rate which I _guess_ is 52MHz before you notify sd cards to run into highspeed mode?

Althought it works for this card, this patch will not please the other cards that they might not reply CMDs any longer including the following CMD6.

Thanks for the feedback, this is exactly why I asked Yousong Zhou to take this
to the mmc list.

Quoted 9 lines
Quoted 4 lines

So if this is not the proper fix for the problem that Yousong Zhou is seeing, then
what might be the proper fix ?

From my knowledge of mmc, there hadn't have a way to deal with this "broken" case. In another word, IMO,it's ANTI-SPEC. We can't be too spec sometimes, but at least we shouldn't violate it.

Maybe the fix is anti-spec. But the fact is that the card works on many other platforms with the builtin card reader (not through an USB adapter), including Mac OS X, my old Nokia Symbian phone, and Windows.

Quoted 9 lines
Quoted 4 lines

Could it be that the sunxi-mmc is doing some things in the wrong order when
changing the clock, or is this all under control of the mmc core ?

all of this is under control of the mmc core.
So if Yongsong does want this card to work for any linux-based mmc stack, I guess something like that should be "better"?

Quoted 7 lines

if (switch to HS fail) {
set_bus_clk
goto retry switch to HS
}

BUT...I admit it seems strange as well.

The SD Specification (simplified version) says "If CRC error occurs on the status data, the host should issue a power cycle.", so I guess the above approach is anti-spec in some way :)

In case it may help debug this problem, I'd like to add that the card previously worked fine with U-Boot before commit [1]. This can also be confirmed by Debian Jessie installer image with which the old U-Boot there worked fine while the kernel (3.16) did not.

[1] sunxi: mmc: Properly setup mod-clk and clock sampling phases,
http://git.denx.de/?p=u-boot.git;a=commit;h=fc3a832576ce7bb597b1823935bfb7dcca235c3c

Cheers

yousong

A message that is not in the archive (sent off-list or not yet received): CAECwjAhe3g0T2icEExE_tBbZ2D1z+BpVNCd0dGZVNU=7zH-LHw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#8 lore raw
Quoted 62 lines · Yousong Zhou

Hi,

Quoted 31 lines · Shawn Lin
Quoted 30 lines · Hans de Goede

Hi Shawn Lin,

Quoted 12 lines · Shawn Lin
Quoted 6 lines · Yousong Zhou

A SD card with sunxi-mmc can fail with the following error message (RCD for
response CRC error) when trying to switch to highspeed mode. Setting the bus
clock before the access mode switch fixed it.

No, that's wrong!

Quoted 8 lines

Before this card is switched to highspeed, it works under identification mode(From spec: bus clock <= 400KHz). How could you raise bus clock to higher clk rate which I _guess_ is 52MHz before you notify sd cards to run into highspeed mode?

Althought it works for this card, this patch will not please the other cards that they might not reply CMDs any longer including the following CMD6.

Thanks for the feedback, this is exactly why I asked Yousong Zhou to take this
to the mmc list.

Quoted 9 lines
Quoted 4 lines

So if this is not the proper fix for the problem that Yousong Zhou is seeing, then
what might be the proper fix ?

From my knowledge of mmc, there hadn't have a way to deal with this "broken"
case. In another word, IMO,it's ANTI-SPEC. We can't be too spec sometimes, but at least we shouldn't violate it.

Maybe the fix is anti-spec. But the fact is that the card works on many other platforms with the builtin card reader (not through an USB adapter), including Mac OS X, my old Nokia Symbian phone, and Windows.

Quoted 14 lines
Quoted 4 lines

Could it be that the sunxi-mmc is doing some things in the wrong order when
changing the clock, or is this all under control of the mmc core ?

all of this is under control of the mmc core.
So if Yongsong does want this card to work for any linux-based mmc stack, I guess something like that should be "better"?

if (switch to HS fail) {
set_bus_clk
goto retry switch to HS
}

Quoted 7 lines
Quoted 2 lines

BUT...I admit it seems strange as well.

The SD Specification (simplified version) says "If CRC error occurs on the status data, the host should issue a power cycle.", so I guess the above approach is anti-spec in some way :)

Right,I was guessing that way from your intention.

Quoted 8 lines

In case it may help debug this problem, I'd like to add that the card previously worked fine with U-Boot before commit [1]. This can also be confirmed by Debian Jessie installer image with which the old U-Boot there worked fine while the kernel (3.16) did not.

[1] sunxi: mmc: Properly setup mod-clk and clock sampling phases,
http://git.denx.de/?p=u-boot.git;a=commit;h=fc3a832576ce7bb597b1823935bfb7dcca235c3c

Interesting... but that at least prove your patch is a workaround but not find the root cause.

Anyway, look into commit-sha [1], can you have a test by restoring mod-clk and clock sampling phases before jump to kernel.

I happended to fix some problems which seems *similar* to yours(but I'm not sure just from commit[1]'s msg):

https://patchwork.kernel.org/patch/7119661/

Quoted 10 lines

Cheers

yousong
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 7 lines
Best Regards
Shawn Lin

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected]
For more options, visit https://groups.google.com/d/optout.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#10 lore raw
Quoted 43 lines · Shawn Lin
Quoted 41 lines · Yousong Zhou

Hi,

Quoted 37 lines · Shawn Lin
Quoted 35 lines · Hans de Goede

Hi Shawn Lin,

Quoted 15 lines · Shawn Lin
Quoted 7 lines · Yousong Zhou

A SD card with sunxi-mmc can fail with the following error message (RCD for
response CRC error) when trying to switch to highspeed mode. Setting the bus
clock before the access mode switch fixed it.

No, that's wrong!

Quoted 8 lines

Before this card is switched to highspeed, it works under identification mode(From spec: bus clock <= 400KHz). How could you raise bus clock to higher clk rate which I _guess_ is 52MHz before you notify sd cards to run into highspeed mode?

Althought it works for this card, this patch will not please the other cards that they might not reply CMDs any longer including the following CMD6.

Thanks for the feedback, this is exactly why I asked Yousong Zhou to take this
to the mmc list.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

So if this is not the proper fix for the problem that Yousong Zhou is seeing, then
what might be the proper fix ?

From my knowledge of mmc, there hadn't have a way to deal with this

"broken"

Quoted 1 line
Quoted 1 line
Quoted 1 line

case. In another word, IMO,it's ANTI-SPEC. We can't be too spec

sometimes,

Quoted 7 lines
Quoted 7 lines
Quoted 2 lines

but at least we shouldn't violate it.

Maybe the fix is anti-spec. But the fact is that the card works on many other platforms with the builtin card reader (not through an USB adapter), including Mac OS X, my old Nokia Symbian phone, and Windows.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines

Could it be that the sunxi-mmc is doing some things in the wrong order when
changing the clock, or is this all under control of the mmc core ?

all of this is under control of the mmc core.
So if Yongsong does want this card to work for any linux-based mmc

stack, I

Quoted 10 lines
Quoted 10 lines
Quoted 9 lines

guess something like that should be "better"?

if (switch to HS fail) {
set_bus_clk
goto retry switch to HS
}

BUT...I admit it seems strange as well.

Quoted 15 lines
Quoted 4 lines

The SD Specification (simplified version) says "If CRC error occurs on the status data, the host should issue a power cycle.", so I guess the above approach is anti-spec in some way :)

Right,I was guessing that way from your intention.

Quoted 7 lines

In case it may help debug this problem, I'd like to add that the card previously worked fine with U-Boot before commit [1]. This can also be confirmed by Debian Jessie installer image with which the old U-Boot there worked fine while the kernel (3.16) did not.

[1] sunxi: mmc: Properly setup mod-clk and clock sampling phases,

http://git.denx.de/?p=u-boot.git;a=commit;h=fc3a832576ce7bb597b1823935bfb7dcca235c3c

Quoted 3 lines
Quoted 1 line

Interesting... but that at least prove your patch is a workaround but not

find the root cause.

Quoted 2 lines

Anyway, look into commit-sha [1], can you have a test by restoring

mod-clk and clock sampling phases before jump to kernel.

Quoted 1 line

Maybe my statement about U-Boot commit [1] above was a little ambiguous, sorry. I meant to say that with that commit applied, U-Boot failed initialising the card the same way as the kernel, i.e. response crc error.

The Debian Jessie installer image's U-Boot worked fine and booted the kernel because it was before commit [1] happened. However after that the 3.16 kernel failed initialising the card.

So, with commit [1], U-Boot failed right away without any chance at all jumping to kernel.

OpenWrt ticket 20387 has more info about the U-Boot failure.

https://dev.openwrt.org/ticket/20387

Anyway, I have no idea what's the effect of those magic numbers on "sampling phases". Never played with such things before :)

Quoted 1 line

I happended to fix some problems which seems *similar* to yours(but I'm

not sure just from commit[1]'s msg):

Quoted 15 lines

https://patchwork.kernel.org/patch/7119661/

Quoted 10 lines

Cheers

yousong
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 6 lines

--
Best Regards
Shawn Lin

--
You received this message because you are subscribed to the Google Groups

"linux-sunxi" group.

Quoted 1 line

To unsubscribe from this group and stop receiving emails from it, send an

email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected]

Quoted 1 line

For more options, visit https://groups.google.com/d/optout.

Signature · 3 lines
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected]
For more options, visit https://groups.google.com/d/optout.

A message that is not in the archive (sent off-list or not yet received): CAECwjAgLuskgyHv50GOVN8fGOmQz+jo9PLCAr-POpYH6AszWSg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#12 lore raw

Hi,

Quoted 86 lines · Yousong Zhou
Quoted 43 lines · Shawn Lin
Quoted 41 lines · Yousong Zhou

Hi,

Quoted 37 lines · Shawn Lin
Quoted 35 lines · Hans de Goede

Hi Shawn Lin,

Quoted 15 lines · Shawn Lin
Quoted 7 lines · Yousong Zhou

A SD card with sunxi-mmc can fail with the following error message (RCD for
response CRC error) when trying to switch to highspeed mode. Setting the bus
clock before the access mode switch fixed it.

No, that's wrong!

Quoted 8 lines

Before this card is switched to highspeed, it works under identification mode(From spec: bus clock <= 400KHz). How could you raise bus clock to higher clk rate which I _guess_ is 52MHz before you notify sd cards to run into highspeed mode?

Althought it works for this card, this patch will not please the other cards that they might not reply CMDs any longer including the following CMD6.

Thanks for the feedback, this is exactly why I asked Yousong Zhou to take this
to the mmc list.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

So if this is not the proper fix for the problem that Yousong Zhou is seeing, then
what might be the proper fix ?

From my knowledge of mmc, there hadn't have a way to deal with this

"broken"

Quoted 1 line
Quoted 1 line
Quoted 1 line

case. In another word, IMO,it's ANTI-SPEC. We can't be too spec

sometimes,

Quoted 7 lines
Quoted 7 lines
Quoted 2 lines

but at least we shouldn't violate it.

Maybe the fix is anti-spec. But the fact is that the card works on many other platforms with the builtin card reader (not through an USB adapter), including Mac OS X, my old Nokia Symbian phone, and Windows.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines

Could it be that the sunxi-mmc is doing some things in the wrong order when
changing the clock, or is this all under control of the mmc core ?

all of this is under control of the mmc core.
So if Yongsong does want this card to work for any linux-based mmc

stack, I

Quoted 18 lines
Quoted 7 lines
Quoted 7 lines

guess something like that should be "better"?

if (switch to HS fail) {
set_bus_clk
goto retry switch to HS
}

Quoted 7 lines
Quoted 2 lines

BUT...I admit it seems strange as well.

The SD Specification (simplified version) says "If CRC error occurs on the status data, the host should issue a power cycle.", so I guess the above approach is anti-spec in some way :)

Right,I was guessing that way from your intention.

Quoted 8 lines
Quoted 7 lines
Quoted 7 lines

In case it may help debug this problem, I'd like to add that the card previously worked fine with U-Boot before commit [1]. This can also be confirmed by Debian Jessie installer image with which the old U-Boot there worked fine while the kernel (3.16) did not.

[1] sunxi: mmc: Properly setup mod-clk and clock sampling phases,

http://git.denx.de/?p=u-boot.git;a=commit;h=fc3a832576ce7bb597b1823935bfb7dcca235c3c

Interesting, one thing that patch does is introduce a switch from parent pll when switching from 400KHz to higher clocks. Can you try the attached patch ?

If reverting u-boot commit fc3a832576ce7bb597b1823935bfb7dcca235c3c fixes things, then it is probably best to focus on fixing u-boot first, and then we can likely apply the same fix to the kernel.

With which SoC(s) are you seeing this problem ? I believe that there may be some differences between the mmc controller used in the A10/13 vs later SoCs so this may be a SoC specific issue.

Regards,

Hans

Quoted 35 lines

OpenWrt ticket 20387 has more info about the U-Boot failure.

https://dev.openwrt.org/ticket/20387

Anyway, I have no idea what's the effect of those magic numbers on "sampling phases". Never played with such things before :)

Quoted 1 line

I happended to fix some problems which seems *similar* to yours(but I'm

not sure just from commit[1]'s msg):

Quoted 15 lines

https://patchwork.kernel.org/patch/7119661/

Quoted 10 lines

Cheers

yousong
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 6 lines

--
Best Regards
Shawn Lin

--
You received this message because you are subscribed to the Google Groups

"linux-sunxi" group.

Quoted 1 line

To unsubscribe from this group and stop receiving emails from it, send an

email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected]

Quoted 1 line

For more options, visit https://groups.google.com/d/optout.

Signature · 3 lines
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected]
For more options, visit https://groups.google.com/d/optout.
Quoted 1 line

From 1523e6fbd27b5da22b0c3cb4e8eda50bce7562cf Mon Sep 17 00:00:00 2001

From: Hans de Goede <[email protected]>
Date: Mon, 7 Sep 2015 08:56:52 +0200
Subject: [PATCH] sunxi: mmc: Add a delay after changing the PLL to allow clock
to stabilize

---
 drivers/mmc/sunxi_mmc.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/mmc/sunxi_mmc.c +2 -0
index 25f18ad..db3c489 100644
@@ -204,6 +204,8 @@ static int mmc_config_clock(struct mmc *mmc)
 	if (mmc_set_mod_clk(mmchost, mmc->clock))
 		return -1;
 
+	mdelay(100);
+
 	/* Clear internal divider */
 	rval &= ~SUNXI_MMC_CLK_DIVIDER_MASK;
 	writel(rval, &mmchost->reg->clkcr);
#13 lore raw

Hi

Quoted 117 lines · Hans de Goede

Hi,

Quoted 113 lines · Yousong Zhou
Quoted 55 lines · Shawn Lin
Quoted 52 lines · Yousong Zhou

Hi,

Quoted 46 lines · Shawn Lin
Quoted 43 lines · Hans de Goede

Hi Shawn Lin,

Quoted 25 lines · Shawn Lin
Quoted 9 lines · Yousong Zhou

A SD card with sunxi-mmc can fail with the following error message (RCD for
response CRC error) when trying to switch to highspeed mode. Setting
the bus
clock before the access mode switch fixed it.

No, that's wrong!

Before this card is switched to highspeed, it works under identification mode(From spec: bus clock <= 400KHz). How could you raise bus clock to higher clk rate which I _guess_ is 52MHz before you
notify sd cards to run into highspeed mode?

Quoted 4 lines

Althought it works for this card, this patch will not please the other
cards that they might not reply CMDs any longer including the following CMD6.

Thanks for the feedback, this is exactly why I asked Yousong Zhou to take this
to the mmc list.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

So if this is not the proper fix for the problem that Yousong Zhou is seeing, then
what might be the proper fix ?

From my knowledge of mmc, there hadn't have a way to deal with this

"broken"

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

case. In another word, IMO,it's ANTI-SPEC. We can't be too spec

sometimes,

Quoted 8 lines
Quoted 8 lines
Quoted 3 lines

but at least we shouldn't violate it.

Maybe the fix is anti-spec. But the fact is that the card works on many other platforms with the builtin card reader (not through an USB adapter), including Mac OS X, my old Nokia Symbian phone, and Windows.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines

Could it be that the sunxi-mmc is doing some things in the wrong order when
changing the clock, or is this all under control of the mmc core ?

all of this is under control of the mmc core.
So if Yongsong does want this card to work for any linux-based mmc

stack, I

Quoted 26 lines
Quoted 8 lines
Quoted 8 lines

guess something like that should be "better"?

if (switch to HS fail) {
set_bus_clk
goto retry switch to HS
}

Quoted 7 lines
Quoted 2 lines

BUT...I admit it seems strange as well.

The SD Specification (simplified version) says "If CRC error occurs on the status data, the host should issue a power cycle.", so I guess the above approach is anti-spec in some way :)

Right,I was guessing that way from your intention.

Quoted 7 lines

In case it may help debug this problem, I'd like to add that the card previously worked fine with U-Boot before commit [1]. This can also be confirmed by Debian Jessie installer image with which the old U-Boot there worked fine while the kernel (3.16) did not.

[1] sunxi: mmc: Properly setup mod-clk and clock sampling phases,

Quoted 7 lines
Quoted 1 line

http://git.denx.de/?p=u-boot.git;a=commit;h=fc3a832576ce7bb597b1823935bfb7dcca235c3c

Interesting, one thing that patch does is introduce a switch from parent pll when switching from 400KHz to higher clocks. Can you try the attached patch ?

Previously I tried inserting udelay(1000) immediately after many writel calls: the result were all the same. I just tried that patch adding mdelay(100) after mmc_set_mod_clk(), still no luck, the same error message just as before.

Quoted 7 lines

If reverting u-boot commit fc3a832576ce7bb597b1823935bfb7dcca235c3c fixes things, then it is probably best to focus on fixing u-boot first, and then we can likely apply the same fix to the kernel.

With which SoC(s) are you seeing this problem ? I believe that there may be some differences between the mmc controller used in the A10/13 vs later SoCs so this may be a SoC specific issue.

It's a Cubieboard2 with Allwinner A20.

Regards,

yousong

Quoted 50 lines

Regards,

Hans

Quoted 42 lines

OpenWrt ticket 20387 has more info about the U-Boot failure.

https://dev.openwrt.org/ticket/20387

Anyway, I have no idea what's the effect of those magic numbers on "sampling phases". Never played with such things before :)

Quoted 1 line

I happended to fix some problems which seems *similar* to yours(but I'm

not sure just from commit[1]'s msg):

Quoted 16 lines

https://patchwork.kernel.org/patch/7119661/

Quoted 10 lines

Cheers

yousong
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 6 lines

--
Best Regards
Shawn Lin

--
You received this message because you are subscribed to the Google Groups

"linux-sunxi" group.

Quoted 2 lines

To unsubscribe from this group and stop receiving emails from it, send an

email to [email protected].

Quoted 2 lines

For more options, visit https://groups.google.com/d/optout.

#14 lore raw
In reply to Hans de Goede
Quoted 120 lines · Hans de Goede

Hi,

Quoted 110 lines · Yousong Zhou
Quoted 55 lines · Shawn Lin
Quoted 52 lines · Yousong Zhou

Hi,

Quoted 46 lines · Shawn Lin
Quoted 43 lines · Hans de Goede

Hi Shawn Lin,

Quoted 25 lines · Shawn Lin
Quoted 9 lines · Yousong Zhou

A SD card with sunxi-mmc can fail with the following error message (RCD for
response CRC error) when trying to switch to highspeed mode. Setting
the bus
clock before the access mode switch fixed it.

No, that's wrong!

Before this card is switched to highspeed, it works under identification mode(From spec: bus clock <= 400KHz). How could you raise bus clock to higher clk rate which I _guess_ is 52MHz before you
notify sd cards to run into highspeed mode?

Quoted 4 lines

Althought it works for this card, this patch will not please the other
cards that they might not reply CMDs any longer including the following CMD6.

Thanks for the feedback, this is exactly why I asked Yousong Zhou to take this
to the mmc list.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

So if this is not the proper fix for the problem that Yousong Zhou is seeing, then
what might be the proper fix ?

From my knowledge of mmc, there hadn't have a way to deal with this

"broken"

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

case. In another word, IMO,it's ANTI-SPEC. We can't be too spec

sometimes,

Quoted 8 lines
Quoted 8 lines
Quoted 3 lines

but at least we shouldn't violate it.

Maybe the fix is anti-spec. But the fact is that the card works on many other platforms with the builtin card reader (not through an USB adapter), including Mac OS X, my old Nokia Symbian phone, and Windows.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines

Could it be that the sunxi-mmc is doing some things in the wrong order when
changing the clock, or is this all under control of the mmc core ?

all of this is under control of the mmc core.
So if Yongsong does want this card to work for any linux-based mmc

stack, I

Quoted 24 lines
Quoted 8 lines
Quoted 8 lines

guess something like that should be "better"?

if (switch to HS fail) {
set_bus_clk
goto retry switch to HS
}

Quoted 7 lines
Quoted 2 lines

BUT...I admit it seems strange as well.

The SD Specification (simplified version) says "If CRC error occurs on the status data, the host should issue a power cycle.", so I guess the above approach is anti-spec in some way :)

Right,I was guessing that way from your intention.

Quoted 5 lines

In case it may help debug this problem, I'd like to add that the card previously worked fine with U-Boot before commit [1]. This can also be confirmed by Debian Jessie installer image with which the old U-Boot there worked fine while the kernel (3.16) did not.

Quoted 4 lines
Quoted 2 lines
Quoted 2 lines

[1] sunxi: mmc: Properly setup mod-clk and clock sampling phases,

http://git.denx.de/?p=u-boot.git;a=commit;h=fc3a832576ce7bb597b1823935bfb7dcca235c3c

Quoted 8 lines

Interesting, one thing that patch does is introduce a switch from parent pll when switching from 400KHz to higher clocks. Can you try the attached patch ?

If reverting u-boot commit fc3a832576ce7bb597b1823935bfb7dcca235c3c fixes things, then it is probably best to focus on fixing u-boot first, and then we can likely apply the same fix to the kernel.

I just tried a method from allwinner's u-boot sources: retry the mode switch in case of an error [1]. It was accompanied with a comment quoted below:

//retry for Toshiba emmc;for the first time Toshiba emmc change to HS //it will return response crc err,so retry

Well, that 1GB card of mine is labeled as "pqi" made in Korea. This workaround really worked and it succeeded on the second try, though I am not sure how anti-spec this can be (the spec says a power reset SHOULD be done in the case of crc error in the status data).

The above change alone can make the card work. I also noticed that allwinner's uboot will disable the clock first before setting it and re-enable it thereafter [2]. I can only guess that this is possibly some bugfix made by engineers of allwinner. So I mimicked this approach and attached a patch here.

[1] https://github.com/allwinner-zh/bootloader/blob/master/u-boot-2011.09/drivers/mmc/mmc.c#L2220
[2] https://github.com/allwinner-zh/bootloader/blob/master/u-boot-2011.09/drivers/mmc/sunxi_mmc.c#L822

Regards.

yousong

Quoted 4 lines

With which SoC(s) are you seeing this problem ? I believe that there may be some differences between the mmc controller used in the A10/13 vs later SoCs so this may be a SoC specific issue.

Quoted 49 lines

Regards,

Hans

Quoted 42 lines

OpenWrt ticket 20387 has more info about the U-Boot failure.

https://dev.openwrt.org/ticket/20387

Anyway, I have no idea what's the effect of those magic numbers on "sampling phases". Never played with such things before :)

Quoted 1 line

I happended to fix some problems which seems *similar* to yours(but I'm

not sure just from commit[1]'s msg):

Quoted 16 lines

https://patchwork.kernel.org/patch/7119661/

Quoted 10 lines

Cheers

yousong
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 6 lines

--
Best Regards
Shawn Lin

--
You received this message because you are subscribed to the Google Groups

"linux-sunxi" group.

Quoted 2 lines

To unsubscribe from this group and stop receiving emails from it, send an

email to [email protected].

Quoted 2 lines

For more options, visit https://groups.google.com/d/optout.

From 926a68de91619f95399e4621d6b2d57bd0cf91e0 Mon Sep 17 00:00:00 2001
From: Yousong Zhou <[email protected]>
Date: Tue, 8 Sep 2015 14:24:58 +0800
Subject: [PATCH 1/2] mmc: sunxi: split setting mod_clk into 3 steps

---
 drivers/mmc/sunxi_mmc.c |   18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)
drivers/mmc/sunxi_mmc.c +14 -4
index 25f18ad..e92b4a5 100644
@@ -133,14 +133,24 @@ static int mmc_set_mod_clk(struct sunxi_mmc_host *mmchost, unsigned int hz)
 		sclk_dly = 4;
 	}
 
-	writel(CCM_MMC_CTRL_ENABLE | pll | CCM_MMC_CTRL_SCLK_DLY(sclk_dly) |
-	       CCM_MMC_CTRL_N(n) | CCM_MMC_CTRL_OCLK_DLY(oclk_dly) |
-	       CCM_MMC_CTRL_M(div), mmchost->mclkreg);
-
 	debug("mmc %u set mod-clk req %u parent %u n %u m %u rate %u\n",
 	      mmchost->mmc_no, hz, pll_hz, 1u << n, div,
 	      pll_hz / (1u << n) / div);
 
+	/* Disable mclk first */
+	writel(0, mmchost->mclkreg);
+	udelay(500);
+
+	writel(pll | CCM_MMC_CTRL_SCLK_DLY(sclk_dly) | CCM_MMC_CTRL_N(n) |
+	             CCM_MMC_CTRL_OCLK_DLY(oclk_dly) | CCM_MMC_CTRL_M(div),
+	       mmchost->mclkreg);
+	udelay(500);
+
+	/* Re-enable mclk */
+	writel(CCM_MMC_CTRL_ENABLE | readl(mmchost->mclkreg),
+	       mmchost->mclkreg);
+	udelay(500);
+
 	return 0;
 }
 
Signature · 1 line
1.7.10.4

From 4bc6e5f555b317b2d5c3801fe7fb8904183c6157 Mon Sep 17 00:00:00 2001
From: Yousong Zhou <[email protected]>
Date: Sun, 30 Aug 2015 15:21:49 +0800
Subject: [PATCH 2/2] mmc: retry in the case of failing to switch to HS mode

---
 drivers/mmc/mmc.c |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
drivers/mmc/mmc.c +11 -3
index 371c1ec..7eb9374 100644
@@ -948,10 +948,18 @@ retry_scr:
 		(mmc->cfg->host_caps & MMC_MODE_HS)))
 		return 0;
 
-	err = sd_switch(mmc, SD_SWITCH_SWITCH, 0, 1, (u8 *)switch_status);
+	timeout = 4;
+	while (true) {
+		err = sd_switch(mmc, SD_SWITCH_SWITCH, 0, 1, (u8 *)switch_status);
+		if (!err)
+			break;
 
-	if (err)
-		return err;
+		timeout -= 1;
+		if (!timeout) {
+			debug("switching to highspeed mode failed: %d\n", err);
+			return err;
+		}
+	}
 
 	if ((__be32_to_cpu(switch_status[4]) & 0x0f000000) == 0x01000000)
 		mmc->card_caps |= MMC_MODE_HS;
Signature · 1 line
1.7.10.4

A message that is not in the archive (sent off-list or not yet received): CAECwjAjsi28gQ-iMZbEgFdoDoTcf0khnS0KRrzO1wnL1j0o0vQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#16 lore raw

Hi,

Quoted 11 lines · Yousong Zhou

I just tried a method from allwinner's u-boot sources: retry the mode switch in case of an error [1]. It was accompanied with a comment quoted below:

//retry for Toshiba emmc;for the first time Toshiba emmc change to HS //it will return response crc err,so retry

Well, that 1GB card of mine is labeled as "pqi" made in Korea. This workaround really worked and it succeeded on the second try, though I am not sure how anti-spec this can be (the spec says a power reset SHOULD be done in the case of crc error in the status data).

Quoted 5 lines

The above change alone can make the card work. I also noticed that allwinner's uboot will disable the clock first before setting it and re-enable it thereafter [2]. I can only guess that this is possibly some bugfix made by engineers of allwinner. So I mimicked this approach and attached a patch here.

Thanks for this. So if I understand you correctly then just the:

"mmc: retry in the case of failing to switch to HS mode"

Patch is enough to make things work, right ?

That makes sense because the clock dance you are subscribing is something which allwinner does in many places and it really is not necessary.

Can you do a similar patch for the kernel and test that too ?

And once you've a working kernel patch submit that to Ulf, with the mmc list in the Cc?

If Ulf is willing to take this fix on the kernel side (I'm the sunxi-mmc maintainer in the kernel, and the fix will get my ack), then I'm willing to take the u-boot version of this fix.

Regards,

Hans

#17 lore raw
In reply to Shawn Lin
Quoted 27 lines
Quoted 10 lines
Quoted 10 lines

all of this is under control of the mmc core.
So if Yongsong does want this card to work for any linux-based mmc stack, I
guess something like that should be "better"?

if (switch to HS fail) {
set_bus_clk
goto retry switch to HS
}

Quoted 7 lines
Quoted 2 lines

BUT...I admit it seems strange as well.

The SD Specification (simplified version) says "If CRC error occurs on the status data, the host should issue a power cycle.", so I guess the above approach is anti-spec in some way :)

Right,I was guessing that way from your intention.

Quoted 7 lines

In case it may help debug this problem, I'd like to add that the card previously worked fine with U-Boot before commit [1]. This can also be confirmed by Debian Jessie installer image with which the old U-Boot there worked fine while the kernel (3.16) did not.

[1] sunxi: mmc: Properly setup mod-clk and clock sampling phases,

Quoted 9 lines
Quoted 2 lines

http://git.denx.de/?p=u-boot.git;a=commit;h=fc3a832576ce7bb597b1823935bfb7dcca235c3c

Interesting... but that at least prove your patch is a workaround but not find the root cause.

Anyway, look into commit-sha [1], can you have a test by restoring mod-clk and clock sampling phases before jump to kernel.

Maybe my statement about U-Boot commit [1] above was a little ambiguous, sorry. I meant to say that with that commit applied, U-Boot failed initialising the card the same way as the kernel, i.e. response crc error.

The Debian Jessie installer image's U-Boot worked fine and booted the kernel because it was before commit [1] happened. However after that the 3.16 kernel failed initialising the card.

So, with commit [1], U-Boot failed right away without any chance at all jumping to kernel.

OpenWrt ticket 20387 has more info about the U-Boot failure.

https://dev.openwrt.org/ticket/20387

Anyway, I have no idea what's the effect of those magic numbers on "sampling phases". Never played with such things before :)

yousong

Quoted 4 lines

I happended to fix some problems which seems *similar* to yours(but I'm not sure just from commit[1]'s msg):

https://patchwork.kernel.org/patch/7119661/

#18 lore raw
Quoted 37 lines · Yousong Zhou
Quoted 27 lines
Quoted 10 lines
Quoted 10 lines

all of this is under control of the mmc core.
So if Yongsong does want this card to work for any linux-based mmc stack, I
guess something like that should be "better"?

if (switch to HS fail) {
set_bus_clk
goto retry switch to HS
}

Quoted 7 lines
Quoted 2 lines

BUT...I admit it seems strange as well.

The SD Specification (simplified version) says "If CRC error occurs on the status data, the host should issue a power cycle.", so I guess the above approach is anti-spec in some way :)

Right,I was guessing that way from your intention.

Quoted 7 lines

In case it may help debug this problem, I'd like to add that the card previously worked fine with U-Boot before commit [1]. This can also be confirmed by Debian Jessie installer image with which the old U-Boot there worked fine while the kernel (3.16) did not.

[1] sunxi: mmc: Properly setup mod-clk and clock sampling phases,

Quoted 9 lines
Quoted 2 lines

http://git.denx.de/?p=u-boot.git;a=commit;h=fc3a832576ce7bb597b1823935bfb7dcca235c3c

Interesting... but that at least prove your patch is a workaround but not find the root cause.

Anyway, look into commit-sha [1], can you have a test by restoring mod-clk and clock sampling phases before jump to kernel.

Quoted 9 lines

Maybe my statement about U-Boot commit [1] above was a little ambiguous, sorry. I meant to say that with that commit applied, U-Boot failed initialising the card the same way as the kernel, i.e. response crc error.

The Debian Jessie installer image's U-Boot worked fine and booted the kernel because it was before commit [1] happened. However after that the 3.16 kernel failed initialising the card.

So I undertand your point: Uboot w/o commit[1] + 3.16 kernel failed that way just as Uboot w/ commit[1] + pre-3.16 kernel, right?

If that is it, could you check sunxi-platform's patches merged into v3.16 for sure whether there is any patch do the same things just like what commit[1] did for uboot or not?

Quoted 19 lines

So, with commit [1], U-Boot failed right away without any chance at all jumping to kernel.

OpenWrt ticket 20387 has more info about the U-Boot failure.

https://dev.openwrt.org/ticket/20387

Anyway, I have no idea what's the effect of those magic numbers on "sampling phases". Never played with such things before :)

yousong

Quoted 4 lines

I happended to fix some problems which seems *similar* to yours(but I'm not sure just from commit[1]'s msg):

https://patchwork.kernel.org/patch/7119661/

Signature · 2 lines
Best Regards
Shawn Lin