[PATCH 0/3] mmc: core: hs400 fix probe errors

[PATCH 0/3] mmc: core: hs400 fix probe errors

[Chris Ruehl]

Fix the probe if hs400-1_8v / hs400-1_2v is used in the
dts and mmc-hs400-enhanced-strobe isn't set.

Kernel errors report when on boot and reboot and eMMC device
not detect randomly. With log output below.

[    1.802342] mmc1: SDHCI controller on fe330000.sdhci [fe330000.sdhci] using ADMA
[    2.007581] mmc1: mmc_select_hs200 failed, error -110
[    2.007589] mmc1: error -110 whilst initialising MMC card
[    2.413559] mmc1: mmc_select_hs200 failed, error -110
[    2.413562] mmc1: error -110 whilst initialising MMC card
[    3.183343] mmc1: Command Queue Engine enabled
[    3.183355] mmc1: new HS400 MMC card at address 0001
[    3.197163] mmcblk1: mmc1:0001 DG4008 7.28 GiB

Current implementation called the mmc_select_hs200 first, and after init
upgrade to hs400. Somehow the eMMC chip randomly crashed here.

Patch has been tested with customized Rockchip rk3399.

Patch 1/3 preparation
move mmc_select_hs400() in between hs400es and hs200, to be able to use
static functions defined previously below the mmc_select_hs400().

Patch 2/3 functionally change
mmc_select_timing()
 call mmc_select_hs400 if EXT_CSD_CARD_TYPE_HS400 is set the
 mmc_avail_type
mmc_select_hs400()
 fixup bus-width check
 fixup set power 1.8v or 1.2v; only one because of host-cap2
 is checked in mmc_select_card().
 Add drop-down to hs-mode.
 Call hs400 prepare tuning (if ops callback)
 and mmc_execute_tuning()

Patch 3/3 cleanup
Remove MMC_CAP2_HS200_1_8V_SDR / MMC_CAP2_HS200_1_2V_SDR from
host->caps2 when mmc-hs400-1_8v or mmc-hs400-1_2v is used in the dts.


Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
---

[PATCH 1/3] mmc: core: prepare hs400 update, code order

[Chris Ruehl]

This patch didn't change functionality just move
mmc_select_hs400() and mmc_hs200_to_hs400() between
mmc_select_hs400es() and mmc_select_hs200 and
fix checkpatch --script warings.

Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
---
 drivers/mmc/core/mmc.c | 184 ++++++++++++++++++++---------------------
 1 file changed, 92 insertions(+), 92 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index ff3063ce2acd..5477786aded8 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1142,98 +1142,6 @@ static int mmc_select_hs_ddr(struct mmc_card *card)
 	return err;
 }
 
-static int mmc_select_hs400(struct mmc_card *card)
-{
-	struct mmc_host *host = card->host;
-	unsigned int max_dtr;
-	int err = 0;
-	u8 val;
-
-	/*
-	 * HS400 mode requires 8-bit bus width
-	 */
-	if (!(card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400 &&
-	      host->ios.bus_width == MMC_BUS_WIDTH_8))
-		return 0;
-
-	/* Switch card to HS mode */
-	val = EXT_CSD_TIMING_HS;
-	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
-			   EXT_CSD_HS_TIMING, val,
-			   card->ext_csd.generic_cmd6_time, 0,
-			   false, true);
-	if (err) {
-		pr_err("%s: switch to high-speed from hs200 failed, err:%d\n",
-			mmc_hostname(host), err);
-		return err;
-	}
-
-	/* Prepare host to downgrade to HS timing */
-	if (host->ops->hs400_downgrade)
-		host->ops->hs400_downgrade(host);
-
-	/* Set host controller to HS timing */
-	mmc_set_timing(host, MMC_TIMING_MMC_HS);
-
-	/* Reduce frequency to HS frequency */
-	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;
-
-	if (host->ops->hs400_prepare_ddr)
-		host->ops->hs400_prepare_ddr(host);
-
-	/* Switch card to DDR */
-	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
-			 EXT_CSD_BUS_WIDTH,
-			 EXT_CSD_DDR_BUS_WIDTH_8,
-			 card->ext_csd.generic_cmd6_time);
-	if (err) {
-		pr_err("%s: switch to bus width for hs400 failed, err:%d\n",
-			mmc_hostname(host), err);
-		return err;
-	}
-
-	/* Switch card to HS400 */
-	val = EXT_CSD_TIMING_HS400 |
-	      card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
-	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
-			   EXT_CSD_HS_TIMING, val,
-			   card->ext_csd.generic_cmd6_time, 0,
-			   false, true);
-	if (err) {
-		pr_err("%s: switch to hs400 failed, err:%d\n",
-			 mmc_hostname(host), err);
-		return err;
-	}
-
-	/* Set host controller to HS400 timing and frequency */
-	mmc_set_timing(host, MMC_TIMING_MMC_HS400);
-	mmc_set_bus_speed(card);
-
-	if (host->ops->hs400_complete)
-		host->ops->hs400_complete(host);
-
-	err = mmc_switch_status(card, true);
-	if (err)
-		goto out_err;
-
-	return 0;
-
-out_err:
-	pr_err("%s: %s failed, error %d\n", mmc_hostname(card->host),
-	       __func__, err);
-	return err;
-}
-
-int mmc_hs200_to_hs400(struct mmc_card *card)
-{
-	return mmc_select_hs400(card);
-}
-
 int mmc_hs400_to_hs200(struct mmc_card *card)
 {
 	struct mmc_host *host = card->host;
@@ -1425,6 +1333,98 @@ static int mmc_select_hs400es(struct mmc_card *card)
 	return err;
 }
 
+static int mmc_select_hs400(struct mmc_card *card)
+{
+	struct mmc_host *host = card->host;
+	unsigned int max_dtr;
+	int err = 0;
+	u8 val;
+
+	/*
+	 * HS400 mode requires 8-bit bus width
+	 */
+	if (!(card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400 &&
+	      host->ios.bus_width == MMC_BUS_WIDTH_8))
+		return 0;
+
+	/* Switch card to HS mode */
+	val = EXT_CSD_TIMING_HS;
+	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+			   EXT_CSD_HS_TIMING, val,
+			   card->ext_csd.generic_cmd6_time, 0,
+			   false, true);
+	if (err) {
+		pr_err("%s: switch to high-speed from hs200 failed, err:%d\n",
+		       mmc_hostname(host), err);
+		return err;
+	}
+
+	/* Prepare host to downgrade to HS timing */
+	if (host->ops->hs400_downgrade)
+		host->ops->hs400_downgrade(host);
+
+	/* Set host controller to HS timing */
+	mmc_set_timing(host, MMC_TIMING_MMC_HS);
+
+	/* Reduce frequency to HS frequency */
+	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;
+
+	if (host->ops->hs400_prepare_ddr)
+		host->ops->hs400_prepare_ddr(host);
+
+	/* Switch card to DDR */
+	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+			 EXT_CSD_BUS_WIDTH,
+			 EXT_CSD_DDR_BUS_WIDTH_8,
+			 card->ext_csd.generic_cmd6_time);
+	if (err) {
+		pr_err("%s: switch to bus width for hs400 failed, err:%d\n",
+		       mmc_hostname(host), err);
+		return err;
+	}
+
+	/* Switch card to HS400 */
+	val = EXT_CSD_TIMING_HS400 |
+	      card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
+	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+			   EXT_CSD_HS_TIMING, val,
+			   card->ext_csd.generic_cmd6_time, 0,
+			   false, true);
+	if (err) {
+		pr_err("%s: switch to hs400 failed, err:%d\n",
+		       mmc_hostname(host), err);
+		return err;
+	}
+
+	/* Set host controller to HS400 timing and frequency */
+	mmc_set_timing(host, MMC_TIMING_MMC_HS400);
+	mmc_set_bus_speed(card);
+
+	if (host->ops->hs400_complete)
+		host->ops->hs400_complete(host);
+
+	err = mmc_switch_status(card, true);
+	if (err)
+		goto out_err;
+
+	return 0;
+
+out_err:
+	pr_err("%s: %s failed, error %d\n", mmc_hostname(card->host),
+	       __func__, err);
+	return err;
+}
+
+int mmc_hs200_to_hs400(struct mmc_card *card)
+{
+	return mmc_select_hs400(card);
+}
+
 /*
  * For device supporting HS200 mode, the following sequence
  * should be done before executing the tuning process.
-- 
2.20.1

[PATCH 2/3] mmc: core: make hs400 independent from hs200 init

[Chris Ruehl]

Move mmc_select_hs400() out from hs200 init path and make hs400
independent.

The patch makes quite some changes and needs to be reviewed carefully.

In function mmc_select_timing() call for mmc_select_hs400().
HS400 requires a host bus with of 8bit, if not supported we
return with -ENOTSUPP, there is no retry.
If the host controller can't switch to 8bit (dts: bus-width = <4>)
it can't recover on next power-up init failed.
Have the controller set to HS mode, make the hs400 tuning prepare
if any and run mmc tuning before switching to HS400.

This patch resolve the problem if hs400-1_8v is set but extended
strobe is not.

&sdhci { // eMMC
        bus-width = <8>;
        mmc-hs400-1_8v;
        // mmc-hs400-enhanced-strobe;
        non-removable;
        status = "okay";
};

[    1.775721] mmc1: CQHCI version 5.10
[    1.802342] mmc1: SDHCI controller on fe330000.sdhci [fe330000.sdhci] using ADMA
[    2.007581] mmc1: mmc_select_hs200 failed, error -110
[    2.007589] mmc1: error -110 whilst initialising MMC card
[    2.413559] mmc1: mmc_select_hs200 failed, error -110
[    2.413562] mmc1: error -110 whilst initialising MMC card
[    3.183343] mmc1: Command Queue Engine enabled
[    3.183355] mmc1: new HS400 MMC card at address 0001
[    3.197163] mmcblk1: mmc1:0001 DG4008 7.28 GiB
[    3.197256] mmcblk1boot0: mmc1:0001 DG4008 partition 1 4.00 MiB
[    3.197360] mmcblk1boot1: mmc1:0001 DG4008 partition 2 4.00 MiB
[    3.197360] mmcblk1boot1: mmc1:0001 DG4008 partition 2 4.00 MiB
[    3.197479] mmcblk1rpmb: mmc1:0001 DG4008 partition 3 4.00 MiB, chardev (242:0)

[    1.743386] mmc1: CQHCI version 5.10
[    1.769952] mmc1: SDHCI controller on fe330000.sdhci [fe330000.sdhci] using ADMA
[    1.846223] mmc1: Command Queue Engine enabled
[    1.846230] mmc1: new HS400 MMC card at address 0001
[    1.846557] mmcblk1: mmc1:0001 DG4008 7.28 GiB
[    1.846650] mmcblk1boot0: mmc1:0001 DG4008 partition 1 4.00 MiB
[    1.846742] mmcblk1boot1: mmc1:0001 DG4008 partition 2 4.00 MiB
[    1.846825] mmcblk1rpmb: mmc1:0001 DG4008 partition 3 4.00 MiB, chardev (242:0)

Tested with rk3399 customized board.

Note:
We had 20% failure rate in the current production "not boot" or
"reboot issues" this patch solved it. Some boards had been
cooled down to -20C have boot failures, after apply the patch
they pass the QC.

Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
---
 drivers/mmc/core/mmc.c | 59 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 5477786aded8..45db890dee80 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1337,15 +1337,46 @@ static int mmc_select_hs400(struct mmc_card *card)
 {
 	struct mmc_host *host = card->host;
 	unsigned int max_dtr;
-	int err = 0;
+	int err = -EINVAL;
 	u8 val;
 
 	/*
-	 * HS400 mode requires 8-bit bus width
+	 * HS400 mode requires host 8-bit bus width
 	 */
-	if (!(card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400 &&
-	      host->ios.bus_width == MMC_BUS_WIDTH_8))
-		return 0;
+	if (!(host->caps & MMC_CAP_8_BIT_DATA)) {
+		pr_err("%s: host not support hs400 8bit\n",
+		       mmc_hostname(host));
+		return -EOPNOTSUPP;
+	}
+
+	/*
+	 * Below is a change from previous logic.
+	 * mmc_avail_type vccq voltage set in dts, it can be 1_2v or 1_8v
+	 * but not both.
+	 */
+	if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400_1_2V) {
+		err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_120);
+	} else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400_1_8V) {
+		err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180);
+	} else {
+		pr_err("%s: hs400 unsupported vccq voltage, err:%d\n",
+		       mmc_hostname(host), -EOPNOTSUPP);
+		return -EOPNOTSUPP;
+	}
+
+	/* Failed set 1_2v or 1_8v try again during next card power cycle */
+	if (err)
+		return err;
+
+	mmc_select_driver_type(card);
+
+	err = mmc_select_bus_width(card);
+	if (err != MMC_BUS_WIDTH_8) {
+		pr_err("%s: switch to 8bit bus width failed, err:%d\n",
+		       mmc_hostname(host), err);
+		err = err < 0 ? err : -EOPNOTSUPP;
+		goto out_err;
+	}
 
 	/* Switch card to HS mode */
 	val = EXT_CSD_TIMING_HS;
@@ -1388,6 +1419,18 @@ static int mmc_select_hs400(struct mmc_card *card)
 		return err;
 	}
 
+	/* HS400 prepare and execute tuning  */
+	host->doing_init_tune = 1;
+	if (host->ops->prepare_hs400_tuning)
+		host->ops->prepare_hs400_tuning(host, &host->ios);
+	err = mmc_execute_tuning(card);
+	host->doing_init_tune = 0;
+	if (err) {
+		pr_err("%s: hs400 tuning failed, err:%d\n",
+		       mmc_hostname(host), err);
+		return err;
+	}
+
 	/* Switch card to HS400 */
 	val = EXT_CSD_TIMING_HS400 |
 	      card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
@@ -1496,7 +1539,7 @@ static int mmc_select_hs200(struct mmc_card *card)
 }
 
 /*
- * Activate High Speed, HS200 or HS400ES mode if supported.
+ * Activate High Speed, HS200 ,HS400 or HS400ES mode if supported.
  */
 static int mmc_select_timing(struct mmc_card *card)
 {
@@ -1507,6 +1550,8 @@ static int mmc_select_timing(struct mmc_card *card)
 
 	if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400ES)
 		err = mmc_select_hs400es(card);
+	else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400)
+		err = mmc_select_hs400(card);
 	else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
 		err = mmc_select_hs200(card);
 	else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS)
@@ -1766,8 +1811,6 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 		host->doing_init_tune = 1;
 
 		err = mmc_hs200_tuning(card);
-		if (!err)
-			err = mmc_select_hs400(card);
 
 		host->doing_init_tune = 0;
 
-- 
2.20.1

[PATCH 3/3] mmc: core: MMC_CAP2_HS200_1_8V_SDR with mmc-hs400-1_8v

[Chris Ruehl]

With the introduction of an independent hs400 timing select,
PATCH 2/3 the availability of the hs200 caps will not checked.
Therefore cleanup and remove MMC_CAP2_HS200_1_8V_SDR /
MMC_CAP2_HS200_1_2V_SDR from host->caps2 when mmc-hs400-1_8v
or mmc-hs400-1_2v is used in the dts.

Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
---
 drivers/mmc/core/host.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 96b2ca1f1b06..46fde60a2372 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -295,9 +295,9 @@ int mmc_of_parse(struct mmc_host *host)
 	if (device_property_read_bool(dev, "mmc-hs200-1_2v"))
 		host->caps2 |= MMC_CAP2_HS200_1_2V_SDR;
 	if (device_property_read_bool(dev, "mmc-hs400-1_8v"))
-		host->caps2 |= MMC_CAP2_HS400_1_8V | MMC_CAP2_HS200_1_8V_SDR;
+		host->caps2 |= MMC_CAP2_HS400_1_8V;
 	if (device_property_read_bool(dev, "mmc-hs400-1_2v"))
-		host->caps2 |= MMC_CAP2_HS400_1_2V | MMC_CAP2_HS200_1_2V_SDR;
+		host->caps2 |= MMC_CAP2_HS400_1_2V;
 	if (device_property_read_bool(dev, "mmc-hs400-enhanced-strobe"))
 		host->caps2 |= MMC_CAP2_HS400_ES;
 	if (device_property_read_bool(dev, "no-sdio"))
-- 
2.20.1

Re: [PATCH 2/3] mmc: core: make hs400 independent from hs200 init

[Chris Ruehl]

On 3/12/2020 5:33 pm, Chris Ruehl wrote:
> Move mmc_select_hs400() out from hs200 init path and make hs400
> independent.
> 
> The patch makes quite some changes and needs to be reviewed carefully.
> 
> In function mmc_select_timing() call for mmc_select_hs400().
> HS400 requires a host bus with of 8bit, if not supported we
> return with -ENOTSUPP, there is no retry.
> If the host controller can't switch to 8bit (dts: bus-width = <4>)
> it can't recover on next power-up init failed.
> Have the controller set to HS mode, make the hs400 tuning prepare
> if any and run mmc tuning before switching to HS400.
> 
> This patch resolve the problem if hs400-1_8v is set but extended
> strobe is not.
> 
> &sdhci { // eMMC
>          bus-width = <8>;
>          mmc-hs400-1_8v;
>          // mmc-hs400-enhanced-strobe;
>          non-removable;
>          status = "okay";
> };
> 
> [    1.775721] mmc1: CQHCI version 5.10
> [    1.802342] mmc1: SDHCI controller on fe330000.sdhci [fe330000.sdhci] using ADMA
> [    2.007581] mmc1: mmc_select_hs200 failed, error -110
> [    2.007589] mmc1: error -110 whilst initialising MMC card
> [    2.413559] mmc1: mmc_select_hs200 failed, error -110
> [    2.413562] mmc1: error -110 whilst initialising MMC card
> [    3.183343] mmc1: Command Queue Engine enabled
> [    3.183355] mmc1: new HS400 MMC card at address 0001
> [    3.197163] mmcblk1: mmc1:0001 DG4008 7.28 GiB
> [    3.197256] mmcblk1boot0: mmc1:0001 DG4008 partition 1 4.00 MiB
> [    3.197360] mmcblk1boot1: mmc1:0001 DG4008 partition 2 4.00 MiB
> [    3.197360] mmcblk1boot1: mmc1:0001 DG4008 partition 2 4.00 MiB
> [    3.197479] mmcblk1rpmb: mmc1:0001 DG4008 partition 3 4.00 MiB, chardev (242:0)
> 
> [    1.743386] mmc1: CQHCI version 5.10
> [    1.769952] mmc1: SDHCI controller on fe330000.sdhci [fe330000.sdhci] using ADMA
> [    1.846223] mmc1: Command Queue Engine enabled
> [    1.846230] mmc1: new HS400 MMC card at address 0001
> [    1.846557] mmcblk1: mmc1:0001 DG4008 7.28 GiB
> [    1.846650] mmcblk1boot0: mmc1:0001 DG4008 partition 1 4.00 MiB
> [    1.846742] mmcblk1boot1: mmc1:0001 DG4008 partition 2 4.00 MiB
> [    1.846825] mmcblk1rpmb: mmc1:0001 DG4008 partition 3 4.00 MiB, chardev (242:0)
> 
> Tested with rk3399 customized board.
> 

Find that patch missed something
-       } else if (!mmc_card_hs400es(card)) {
+       } else if (!mmc_card_hs400es(card) && !mmc_card_hs400(card)) {

will update my patches and resend v2

Chris