[PATCH v2 00/14] few fix for sdhci-esdhc-imx

[PATCH v2 00/14] few fix for sdhci-esdhc-imx

[BOUGH CHEN]

---
Changes for v2:
Add Adrian's ack for patch 1~8
Patch 9~10, change to use 'sdhci,auto-cmd23-broken' instead of 'fsl,auto-cmd23-broken'
Patch 11, clear cqhci irq status and halt cqhci, move this change to sdhci_esdhc_imx_hwinit()
and add some comment to explain the reason.
Patch 12, add error return check for pinctrl_pm_select_sleep_state()
Patch 13, clear the DMA_SEL in sdhci-esdhc-imx.c instead of sdhci.c
Patch 14, add the dev->dma_parms in core/queue.c, instead of host/sdhci.c.

Haibo Chen (14):
  mmc: sdhci: do not enable card detect interrupt for gpio cd type
  mmc: sdhci-esdhc-imx: no fail when no pinctrl available
  mmc: sdhci-esdhci-imx: retune needed for Mega/Mix enabled SoCs
  mmc: sdhci-esdhc-imx: restore the per_clk rate in PM_RUNTIME
  doc: dt: fsl-imx-esdhc: add strobe-dll-delay-target binding
  mmc: sdhci-esdhc-imx: add strobe-dll-delay-target support
  mmc: sdhci-esdhc-imx: optimize the clock setting
  mmc: sdhci-esdhc-imx: optimize the strobe dll setting
  doc: dt: fsl-imx-esdhc: add auto-cmd23-broken binding
  mmc: sdhci-esdhc-imx: handle 'sdhci,auto-cmd23-broken' from devicetree
  mmc: sdhci-esdhc-imx: clear pending interrupt and halt cqhci
  mmc: sdhci-esdhc-imx: restore pin state when resume back
  mmc: sdhci-esdhc-imx: clear DMA_SEL when disable DMA mode
  mmc: queue: create dev->dma_parms before call dma_set_max_seg_size()

 .../devicetree/bindings/mmc/fsl-imx-esdhc.txt |  13 ++
 drivers/mmc/core/queue.c                      |   9 +-
 drivers/mmc/host/sdhci-esdhc-imx.c            | 146 ++++++++++++++----
 drivers/mmc/host/sdhci-esdhc.h                |   1 +
 drivers/mmc/host/sdhci-pltfm.c                |   3 +
 drivers/mmc/host/sdhci.c                      |   2 +-
 include/linux/platform_data/mmc-esdhc-imx.h   |   1 +
 7 files changed, 144 insertions(+), 31 deletions(-)

-- 
2.17.1

[PATCH v2 01/14] mmc: sdhci: do not enable card detect interrupt for gpio cd type

[BOUGH CHEN]

Except SDHCI_QUIRK_BROKEN_CARD_DETECTION and MMC_CAP_NONREMOVABLE,
we also do not need to handle controller native card detect interrupt
for gpio cd type.
If we wrong enabled the card detect interrupt for gpio case, it will
cause a lot of unexpected card detect interrupts during data transfer
which should not happen.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 3140fe2e5dba..6dc88b1aee6e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -152,7 +152,7 @@ static void sdhci_set_card_detection(struct sdhci_host *host, bool enable)
 	u32 present;
 
 	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) ||
-	    !mmc_card_is_removable(host->mmc))
+	    !mmc_card_is_removable(host->mmc) || mmc_can_gpio_cd(host->mmc))
 		return;
 
 	if (enable) {
-- 
2.17.1

[PATCH v2 02/14] mmc: sdhci-esdhc-imx: no fail when no pinctrl available

[BOUGH CHEN]

When using jailhouse to support two Linux on i.MX8MQ EVK, we use the
1st Linux to configure pinctrl for the 2nd Linux. Then the 2nd Linux
could use the mmc without pinctrl driver.

So give a warning message when no pinctrl available, but no fail probe.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 1c988d6a2433..3830f4dc8dfc 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1489,13 +1489,14 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	imx_data->pinctrl = devm_pinctrl_get(&pdev->dev);
 	if (IS_ERR(imx_data->pinctrl)) {
 		err = PTR_ERR(imx_data->pinctrl);
-		goto disable_ahb_clk;
-	}
-
-	imx_data->pins_default = pinctrl_lookup_state(imx_data->pinctrl,
+		dev_warn(mmc_dev(host->mmc), "could not get pinctrl\n");
+		imx_data->pins_default = ERR_PTR(-EINVAL);
+	} else {
+		imx_data->pins_default = pinctrl_lookup_state(imx_data->pinctrl,
 						PINCTRL_STATE_DEFAULT);
-	if (IS_ERR(imx_data->pins_default))
-		dev_warn(mmc_dev(host->mmc), "could not get default state\n");
+		if (IS_ERR(imx_data->pins_default))
+			dev_warn(mmc_dev(host->mmc), "could not get default state\n");
+	}
 
 	if (esdhc_is_usdhc(imx_data)) {
 		host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
-- 
2.17.1

[PATCH v2 03/14] mmc: sdhci-esdhci-imx: retune needed for Mega/Mix enabled SoCs

[BOUGH CHEN]

For Mega/Mix enabled SoCs like MX7D and MX6SX, uSDHC will lost power in
LP mode no matter whether the MMC_KEEP_POWER flag is set or not.
This may cause state misalign between kernel and HW, especially for
SDIO3.0 WiFi cards.
e.g. SDIO WiFi driver usually will keep power during system suspend.
And after resume, no card re-enumeration called.
But the tuning state is lost due to Mega/Mix.
Then CRC error may happen during next data transfer.

So we should always fire a mmc_retune_needed() for such type SoC
to tell MMC core retuning is needed for next data transfer.
mmc: sdhci-esdhci-imx: retune needed for Mega/Mix enabled SoCs

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 3830f4dc8dfc..0f281821a269 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -160,6 +160,8 @@
 #define ESDHC_FLAG_CQHCI		BIT(12)
 /* need request pmqos during low power */
 #define ESDHC_FLAG_PMQOS		BIT(13)
+/* The IP state got lost in low power mode */
+#define ESDHC_FLAG_STATE_LOST_IN_LPMODE		BIT(14)
 
 struct esdhc_soc_data {
 	u32 flags;
@@ -193,32 +195,37 @@ static const struct esdhc_soc_data usdhc_imx6sl_data = {
 
 static const struct esdhc_soc_data usdhc_imx6sx_data = {
 	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
-			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200,
+			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
+			| ESDHC_FLAG_STATE_LOST_IN_LPMODE,
 };
 
 static const struct esdhc_soc_data usdhc_imx6ull_data = {
 	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
 			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
-			| ESDHC_FLAG_ERR010450,
+			| ESDHC_FLAG_ERR010450
+			| ESDHC_FLAG_STATE_LOST_IN_LPMODE,
 };
 
 static const struct esdhc_soc_data usdhc_imx7d_data = {
 	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
 			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
-			| ESDHC_FLAG_HS400,
+			| ESDHC_FLAG_HS400
+			| ESDHC_FLAG_STATE_LOST_IN_LPMODE,
 };
 
 static struct esdhc_soc_data usdhc_imx7ulp_data = {
 	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
 			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
-			| ESDHC_FLAG_PMQOS | ESDHC_FLAG_HS400,
+			| ESDHC_FLAG_PMQOS | ESDHC_FLAG_HS400
+			| ESDHC_FLAG_STATE_LOST_IN_LPMODE,
 };
 
 static struct esdhc_soc_data usdhc_imx8qxp_data = {
 	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
 			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
 			| ESDHC_FLAG_HS400 | ESDHC_FLAG_HS400_ES
-			| ESDHC_FLAG_CQHCI,
+			| ESDHC_FLAG_CQHCI
+			| ESDHC_FLAG_STATE_LOST_IN_LPMODE,
 };
 
 struct pltfm_imx_data {
@@ -1613,6 +1620,8 @@ static int sdhci_esdhc_imx_remove(struct platform_device *pdev)
 static int sdhci_esdhc_suspend(struct device *dev)
 {
 	struct sdhci_host *host = dev_get_drvdata(dev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
 	int ret;
 
 	if (host->mmc->caps2 & MMC_CAP2_CQE) {
@@ -1621,6 +1630,12 @@ static int sdhci_esdhc_suspend(struct device *dev)
 			return ret;
 	}
 
+	if ((imx_data->socdata->flags & ESDHC_FLAG_STATE_LOST_IN_LPMODE) &&
+		(host->tuning_mode != SDHCI_TUNING_MODE_1)) {
+		mmc_retune_timer_stop(host->mmc);
+		mmc_retune_needed(host->mmc);
+	}
+
 	if (host->tuning_mode != SDHCI_TUNING_MODE_3)
 		mmc_retune_needed(host->mmc);
 
-- 
2.17.1

[PATCH v2 04/14] mmc: sdhci-esdhc-imx: restore the per_clk rate in PM_RUNTIME

[BOUGH CHEN]

When pm_runtime_suspend is run, a call to SCFW power off the SS (SS is a
power domain, usdhc belong to this SS power domain) in which the resource
resides is made. The SCFW can power off the SS if no other resource in
active in that SS. If so, all state associated with all the resources within
the SS that is powered off is lost, this includes the clock rates, clock state
etc. When pm_runtime_resume is called, the SS associated with that resource
is powered up. But the clocks are left in the default state.

This patch restore clock rate in pm_runtime_resume, make sure the
clock is right rather than depending on the default state setting
by SCFW.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 0f281821a269..0cce4d97f0a5 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -162,6 +162,8 @@
 #define ESDHC_FLAG_PMQOS		BIT(13)
 /* The IP state got lost in low power mode */
 #define ESDHC_FLAG_STATE_LOST_IN_LPMODE		BIT(14)
+/* The IP lost clock rate in PM_RUNTIME */
+#define ESDHC_FLAG_CLK_RATE_LOST_IN_PM_RUNTIME	BIT(15)
 
 struct esdhc_soc_data {
 	u32 flags;
@@ -225,7 +227,8 @@ static struct esdhc_soc_data usdhc_imx8qxp_data = {
 			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
 			| ESDHC_FLAG_HS400 | ESDHC_FLAG_HS400_ES
 			| ESDHC_FLAG_CQHCI
-			| ESDHC_FLAG_STATE_LOST_IN_LPMODE,
+			| ESDHC_FLAG_STATE_LOST_IN_LPMODE
+			| ESDHC_FLAG_CLK_RATE_LOST_IN_PM_RUNTIME,
 };
 
 struct pltfm_imx_data {
@@ -1705,6 +1708,9 @@ static int sdhci_esdhc_runtime_resume(struct device *dev)
 		pm_qos_add_request(&imx_data->pm_qos_req,
 			PM_QOS_CPU_DMA_LATENCY, 0);
 
+	if (imx_data->socdata->flags & ESDHC_FLAG_CLK_RATE_LOST_IN_PM_RUNTIME)
+		clk_set_rate(imx_data->clk_per, pltfm_host->clock);
+
 	err = clk_prepare_enable(imx_data->clk_ahb);
 	if (err)
 		goto remove_pm_qos_request;
-- 
2.17.1

[PATCH v2 05/14] doc: dt: fsl-imx-esdhc: add strobe-dll-delay-target binding

[BOUGH CHEN]

Add fsl,strobe-dll-delay-target binding.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
index 2fb466ca2a9d..89b986a2f9d5 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
@@ -41,6 +41,11 @@ Optional properties:
   This property allows user to change the tuning step to more than one delay
   cells which is useful for some special boards or cards when the default
   tuning step can't find the proper delay window within limited tuning retries.
+- fsl,strobe-dll-delay-target: Specify the strobe dll control slave delay target.
+  This delay target programming host controller loopback read clock, and this
+  property allows user to change the delay target for the strobe input read clock.
+  If not use this property, driver default set the delay target to value 7.
+  Only eMMC HS400 mode need to take care of this property.
 
 Examples:
 
-- 
2.17.1

[PATCH v2 06/14] mmc: sdhci-esdhc-imx: add strobe-dll-delay-target support

[BOUGH CHEN]

strobe-dll-delay-target is the delay cell add on the strobe line.
Strobe line the the uSDHC loopback read clock which is use in HS400
mode. Different strobe-dll-delay-target may need to set for different
board/SoC. If this delay cell is not set to an appropriate value,
we may see some read operation meet CRC error after HS400 mode select
which already pass the tuning.

This patch add the strobe-dll-delay-target setting in driver, so that
user can easily config this delay cell in dts file.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c          | 12 +++++++++++-
 include/linux/platform_data/mmc-esdhc-imx.h |  1 +
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 0cce4d97f0a5..4b19ac0039f6 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -73,6 +73,7 @@
 #define ESDHC_STROBE_DLL_CTRL		0x70
 #define ESDHC_STROBE_DLL_CTRL_ENABLE	(1 << 0)
 #define ESDHC_STROBE_DLL_CTRL_RESET	(1 << 1)
+#define ESDHC_STROBE_DLL_CTRL_SLV_DLY_TARGET_DEFAULT	0x7
 #define ESDHC_STROBE_DLL_CTRL_SLV_DLY_TARGET_SHIFT	3
 #define ESDHC_STROBE_DLL_CTRL_SLV_UPDATE_INT_DEFAULT	(4 << 20)
 
@@ -995,6 +996,9 @@ static int esdhc_change_pinstate(struct sdhci_host *host,
  */
 static void esdhc_set_strobe_dll(struct sdhci_host *host)
 {
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
+	u32 strobe_delay;
 	u32 v;
 
 	/* disable clock before enabling strobe dll */
@@ -1012,9 +1016,13 @@ static void esdhc_set_strobe_dll(struct sdhci_host *host)
 	 * enable strobe dll ctrl and adjust the delay target
 	 * for the uSDHC loopback read clock
 	 */
+	if (imx_data->boarddata.strobe_dll_delay_target)
+		strobe_delay = imx_data->boarddata.strobe_dll_delay_target;
+	else
+		strobe_delay = ESDHC_STROBE_DLL_CTRL_SLV_DLY_TARGET_DEFAULT;
 	v = ESDHC_STROBE_DLL_CTRL_ENABLE |
 		ESDHC_STROBE_DLL_CTRL_SLV_UPDATE_INT_DEFAULT |
-		(7 << ESDHC_STROBE_DLL_CTRL_SLV_DLY_TARGET_SHIFT);
+		(strobe_delay << ESDHC_STROBE_DLL_CTRL_SLV_DLY_TARGET_SHIFT);
 	writel(v, host->ioaddr + ESDHC_STROBE_DLL_CTRL);
 	/* wait 5us to make sure strobe dll status register stable */
 	udelay(5);
@@ -1340,6 +1348,8 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
 	of_property_read_u32(np, "fsl,tuning-start-tap",
 			     &boarddata->tuning_start_tap);
 
+	of_property_read_u32(np, "fsl,strobe-dll-delay-target",
+				&boarddata->strobe_dll_delay_target);
 	if (of_find_property(np, "no-1-8-v", NULL))
 		host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
 
diff --git a/include/linux/platform_data/mmc-esdhc-imx.h b/include/linux/platform_data/mmc-esdhc-imx.h
index 6c006078c8a1..0434f68eda86 100644
--- a/include/linux/platform_data/mmc-esdhc-imx.h
+++ b/include/linux/platform_data/mmc-esdhc-imx.h
@@ -37,5 +37,6 @@ struct esdhc_platform_data {
 	unsigned int delay_line;
 	unsigned int tuning_step;       /* The delay cell steps in tuning procedure */
 	unsigned int tuning_start_tap;	/* The start delay cell point in tuning procedure */
+	unsigned int strobe_dll_delay_target;	/* The delay cell for strobe pad (read clock) */
 };
 #endif /* __ASM_ARCH_IMX_ESDHC_H */
-- 
2.17.1

[PATCH v2 07/14] mmc: sdhci-esdhc-imx: optimize the clock setting

[BOUGH CHEN]

When force clock off, check the SDOFF of register PRSSTAT to make sure
the clock is gate off. Before force clock on, check the SDSTB of register
PRSSTAT to make sure the clock is stable, this will eliminate the clock
glitch.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 24 +++++++++++++++++++++++-
 drivers/mmc/host/sdhci-esdhc.h     |  1 +
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 4b19ac0039f6..9b03656c7abc 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/clk.h>
@@ -313,6 +314,17 @@ static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask, u32 val, i
 	writel(((readl(base) & ~(mask << shift)) | (val << shift)), base);
 }
 
+static inline void esdhc_wait_for_card_clock_gate_off(struct sdhci_host *host)
+{
+	u32 present_state;
+	int ret;
+
+	ret = readl_poll_timeout(host->ioaddr + ESDHC_PRSSTAT, present_state,
+				(present_state & ESDHC_CLOCK_GATE_OFF), 2, 100);
+	if (ret == -ETIMEDOUT)
+		dev_warn(mmc_dev(host->mmc), "%s: card clock still not gate off in 100us!.\n", __func__);
+}
+
 static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -526,6 +538,8 @@ static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
 		else
 			new_val &= ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON;
 		writel(new_val, host->ioaddr + ESDHC_VENDOR_SPEC);
+		if (!(new_val & ESDHC_VENDOR_SPEC_FRC_SDCLK_ON))
+			esdhc_wait_for_card_clock_gate_off(host);
 		return;
 	case SDHCI_HOST_CONTROL2:
 		new_val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
@@ -754,12 +768,14 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
 	int ddr_pre_div = imx_data->is_ddr ? 2 : 1;
 	int pre_div = 1;
 	int div = 1;
+	int ret;
 	u32 temp, val;
 
 	if (esdhc_is_usdhc(imx_data)) {
 		val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
 		writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
 			host->ioaddr + ESDHC_VENDOR_SPEC);
+		esdhc_wait_for_card_clock_gate_off(host);
 	}
 
 	if (clock == 0) {
@@ -814,13 +830,18 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
 		| (pre_div << ESDHC_PREDIV_SHIFT));
 	sdhci_writel(host, temp, ESDHC_SYSTEM_CONTROL);
 
+	/* need to wait the bit 3 of the PRSSTAT to be set, make sure card clock is stable */
+	ret = readl_poll_timeout(host->ioaddr + ESDHC_PRSSTAT, temp,
+				(temp & ESDHC_CLOCK_STABLE), 2, 100);
+	if (ret == -ETIMEDOUT)
+		dev_warn(mmc_dev(host->mmc), "card clock still not stable in 100us!.\n");
+
 	if (esdhc_is_usdhc(imx_data)) {
 		val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
 		writel(val | ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
 			host->ioaddr + ESDHC_VENDOR_SPEC);
 	}
 
-	mdelay(1);
 }
 
 static unsigned int esdhc_pltfm_get_ro(struct sdhci_host *host)
@@ -1005,6 +1026,7 @@ static void esdhc_set_strobe_dll(struct sdhci_host *host)
 	writel(readl(host->ioaddr + ESDHC_VENDOR_SPEC) &
 		~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
 		host->ioaddr + ESDHC_VENDOR_SPEC);
+	esdhc_wait_for_card_clock_gate_off(host);
 
 	/* force a reset on strobe dll */
 	writel(ESDHC_STROBE_DLL_CTRL_RESET,
diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
index 9289bb4d633e..947212f16bc6 100644
--- a/drivers/mmc/host/sdhci-esdhc.h
+++ b/drivers/mmc/host/sdhci-esdhc.h
@@ -31,6 +31,7 @@
 
 /* Present State Register */
 #define ESDHC_PRSSTAT			0x24
+#define ESDHC_CLOCK_GATE_OFF		0x00000080
 #define ESDHC_CLOCK_STABLE		0x00000008
 
 /* Protocol Control Register */
-- 
2.17.1

[PATCH v2 08/14] mmc: sdhci-esdhc-imx: optimize the strobe dll setting

[BOUGH CHEN]

After set the STROBE SLV delay target value, it need to wait some
time to let the usdhc lock the REF and SLV clock. In normal case,
1~2us is enough for imx8/imx6 and imx7d, and 4~5us is enough for
imx7ulp, but when do reboot stress test or do the bind/unbind stress
test, sometimes need to wait about 10us to get the status lock.

This patch optimize delay handle method, only print the warning
message if the status is still not lock after 1ms delay.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 9b03656c7abc..54ffb04ab1d1 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1021,6 +1021,7 @@ static void esdhc_set_strobe_dll(struct sdhci_host *host)
 	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
 	u32 strobe_delay;
 	u32 v;
+	int ret;
 
 	/* disable clock before enabling strobe dll */
 	writel(readl(host->ioaddr + ESDHC_VENDOR_SPEC) &
@@ -1046,15 +1047,13 @@ static void esdhc_set_strobe_dll(struct sdhci_host *host)
 		ESDHC_STROBE_DLL_CTRL_SLV_UPDATE_INT_DEFAULT |
 		(strobe_delay << ESDHC_STROBE_DLL_CTRL_SLV_DLY_TARGET_SHIFT);
 	writel(v, host->ioaddr + ESDHC_STROBE_DLL_CTRL);
-	/* wait 5us to make sure strobe dll status register stable */
-	udelay(5);
-	v = readl(host->ioaddr + ESDHC_STROBE_DLL_STATUS);
-	if (!(v & ESDHC_STROBE_DLL_STS_REF_LOCK))
-		dev_warn(mmc_dev(host->mmc),
-		"warning! HS400 strobe DLL status REF not lock!\n");
-	if (!(v & ESDHC_STROBE_DLL_STS_SLV_LOCK))
+
+	/* wait max 50us to get the REF/SLV lock */
+	ret = readl_poll_timeout(host->ioaddr + ESDHC_STROBE_DLL_STATUS, v,
+		((v & ESDHC_STROBE_DLL_STS_REF_LOCK) && (v & ESDHC_STROBE_DLL_STS_SLV_LOCK)), 1, 50);
+	if (ret == -ETIMEDOUT)
 		dev_warn(mmc_dev(host->mmc),
-		"warning! HS400 strobe DLL status SLV not lock!\n");
+		"warning! HS400 strobe DLL status REF/SLV not lock in 50us, STROBE DLL status is %x!\n", v);
 }
 
 static void esdhc_reset_tuning(struct sdhci_host *host)
-- 
2.17.1

[PATCH v2 09/14] doc: dt: fsl-imx-esdhc: add auto-cmd23-broken binding

[BOUGH CHEN]

Add sdhci,auto-cmd23-broken binding.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
index 89b986a2f9d5..7ea8e0f14e5f 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
@@ -46,6 +46,14 @@ Optional properties:
   property allows user to change the delay target for the strobe input read clock.
   If not use this property, driver default set the delay target to value 7.
   Only eMMC HS400 mode need to take care of this property.
+- sdhci,auto-cmd23-broken: disable the ACMD23 function of USDHC.
+  This is required for eMMC on imx6qpdl/imx6sx/imx7d when it use ADMA mode. Because
+  for these SoC, it do not support the ACMD23 completely, only take the 16 bit block
+  count from the 0x4 register (BLK_ATT) as argument for the ACMD23, the upper 16 bit
+  of the CMD23's argument is ignored. This will impact the reliable write operation
+  and the RPMB block write operation, because these operations need to set the bit 31
+  of the CMD23's argument. SDMA mode will default disable the ACMD23 mode. SD card do
+  not has this limitation on these SoCs.
 
 Examples:
 
-- 
2.17.1

[PATCH v2 10/14] mmc: sdhci-esdhc-imx: handle 'sdhci,auto-cmd23-broken' from devicetree

[BOUGH CHEN]

Since L4.15, community involve the commit 105819c8a545 ("mmc: core: use mrq->sbc
when sending CMD23 for RPMB"), let the usdhc to decide whether to use ACMD23 for
RPMB. This CMD23 for RPMB need to set the bit 31 to its argument, if not, the
RPMB write operation will return general fail.

According to the sdhci logic, SDMA mode will disable the ACMD23, and only in
ADMA mode, it will chose to use ACMD23 if the host support. But according to
debug, and confirm with IC, the imx6qpdl/imx6sx/imx6sl/imx7d do not support
the ACMD23 feature completely. These SoCs only use the 16 bit block count of
the register 0x4 (BLOCK_ATT) as the CMD23's argument in ACMD23 mode, which
means it will ignore the upper 16 bit of the CMD23's argument. This will block
the reliable write operation in RPMB, because RPMB reliable write need to set
the bit31 of the CMD23's argument. This is the hardware limitation. Due to
imx6sl use SDMA, so for imx6qpdl/imx6sx/imx7d, it need to broke the ACMD23 for
eMMC, SD card do not has this limitation, because SD card do not support reliable
write.

For imx6ul/imx6ull/imx6sll/imx7ulp/imx8, it support the ACMD23 completely, it
change to use the 0x0 register (DS_ADDR) to put the CMD23's argument in ADMA mode.

This patch handle 'sdhci,auto-cmd23-broken' from devicetree.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 5 +++--
 drivers/mmc/host/sdhci-pltfm.c     | 3 +++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 54ffb04ab1d1..0a8a7b748341 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1371,8 +1371,6 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
 
 	of_property_read_u32(np, "fsl,strobe-dll-delay-target",
 				&boarddata->strobe_dll_delay_target);
-	if (of_find_property(np, "no-1-8-v", NULL))
-		host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
 
 	if (of_property_read_u32(np, "fsl,delay-line", &boarddata->delay_line))
 		boarddata->delay_line = 0;
@@ -1386,6 +1384,9 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
 						ESDHC_PINCTRL_STATE_200MHZ);
 	}
 
+	/* call to generic sdhci_get_property to support additional capabilities */
+	sdhci_get_property(pdev);
+
 	/* call to generic mmc_of_parse to support additional capabilities */
 	ret = mmc_of_parse(host->mmc);
 	if (ret)
diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 328b132bbe57..862d2bb9c2a8 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -86,6 +86,9 @@ void sdhci_get_property(struct platform_device *pdev)
 	if (device_property_present(dev, "sdhci,auto-cmd12"))
 		host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
 
+	if (device_property_present(dev, "sdhci,auto-cmd23-broken"))
+		host->quirks2 |= SDHCI_QUIRK2_ACMD23_BROKEN;
+
 	if (device_property_present(dev, "sdhci,1-bit-only") ||
 	    (device_property_read_u32(dev, "bus-width", &bus_width) == 0 &&
 	    bus_width == 1))
-- 
2.17.1

[PATCH v2 11/14] mmc: sdhci-esdhc-imx: clear pending interrupt and halt cqhci

[BOUGH CHEN]

On i.MX8MM, we are running Dual Linux OS, with 1st Linux using SD Card
as rootfs storage, 2nd Linux using eMMC as rootfs storage. We let the
the 1st linux configure power/clock for the 2nd Linux.

When the 2nd Linux is booting into rootfs stage, we let the 1st Linux
to destroy the 2nd linux, then restart the 2nd linux, we met SDHCI dump
as following, after we clear the pending interrupt and halt CQCTL, issue
gone.

[ 1.334594] mmc2: Got command interrupt 0x00000001 even though no command operation was in progress.
[ 1.334595] mmc2: sdhci: ============ SDHCI REGISTER DUMP ===========
[ 1.334599] mmc2: sdhci: Sys addr: 0xa05dcc00 | Version: 0x00000002
[ 1.345538] mmc2: sdhci: Blk size: 0x00000200 | Blk cnt: 0x00000000
[ 1.345541] mmc2: sdhci: Argument: 0x00018000 | Trn mode: 0x00000033
[ 1.345543] mmc2: sdhci: Present: 0x01f88008 | Host ctl: 0x00000031
[ 1.345547] mmc2: sdhci: Power: 0x00000002 | Blk gap: 0x00000080
[ 1.357903] mmc2: sdhci: Wake-up: 0x00000008 | Clock: 0x0000003f
[ 1.357905] mmc2: sdhci: Timeout: 0x0000008f | Int stat: 0x00000000
[ 1.357908] mmc2: sdhci: Int enab: 0x107f100b | Sig enab: 0x107f100b
[ 1.357911] mmc2: sdhci: AC12 err: 0x00000000 | Slot int: 0x00000502
[ 1.370268] mmc2: sdhci: Caps: 0x07eb0000 | Caps_1: 0x0000b400
[ 1.370270] mmc2: sdhci: Cmd: 0x00000d1a | Max curr: 0x00ffffff
[ 1.370273] mmc2: sdhci: Resp[0]: 0x00000b00 | Resp[1]: 0xffffffff
[ 1.370276] mmc2: sdhci: Resp[2]: 0x328f5903 | Resp[3]: 0x00d00f00
[ 1.382132] mmc2: sdhci: Host ctl2: 0x00000000
[ 1.382135] mmc2: sdhci: ADMA Err: 0x00000000 | ADMA Ptr: 0xa2040208

[ 2.060932] mmc2: Unexpected interrupt 0x00004000.
[ 2.065538] mmc2: sdhci: ============ SDHCI REGISTER DUMP ===========
[ 2.071720] mmc2: sdhci: Sys addr: 0x00000000 | Version: 0x00000002
[ 2.077902] mmc2: sdhci: Blk size: 0x00000200 | Blk cnt: 0x00000001
[ 2.084083] mmc2: sdhci: Argument: 0x00000000 | Trn mode: 0x00000000
[ 2.090264] mmc2: sdhci: Present: 0x01f88009 | Host ctl: 0x00000011
[ 2.096446] mmc2: sdhci: Power: 0x00000002 | Blk gap: 0x00000080
[ 2.102627] mmc2: sdhci: Wake-up: 0x00000008 | Clock: 0x000010ff
[ 2.108809] mmc2: sdhci: Timeout: 0x0000008f | Int stat: 0x00004000
[ 2.114990] mmc2: sdhci: Int enab: 0x007f1003 | Sig enab: 0x007f1003
[ 2.121171] mmc2: sdhci: AC12 err: 0x00000000 | Slot int: 0x00000502
[ 2.127353] mmc2: sdhci: Caps: 0x07eb0000 | Caps_1: 0x0000b400
[ 2.133534] mmc2: sdhci: Cmd: 0x0000371a | Max curr: 0x00ffffff
[ 2.139715] mmc2: sdhci: Resp[0]: 0x00000900 | Resp[1]: 0xffffffff
[ 2.145896] mmc2: sdhci: Resp[2]: 0x328f5903 | Resp[3]: 0x00d00f00
[ 2.152077] mmc2: sdhci: Host ctl2: 0x00000000
[ 2.156342] mmc2: sdhci: ADMA Err: 0x00000000 | ADMA Ptr: 0x00000000

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 0a8a7b748341..ae1538e30b23 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1203,6 +1203,7 @@ static void sdhci_esdhc_imx_hwinit(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
+	struct cqhci_host *cq_host = host->mmc->cqe_private;
 	int tmp;
 
 	if (esdhc_is_usdhc(imx_data)) {
@@ -1279,6 +1280,21 @@ static void sdhci_esdhc_imx_hwinit(struct sdhci_host *host)
 			tmp &= ~ESDHC_STD_TUNING_EN;
 			writel(tmp, host->ioaddr + ESDHC_TUNING_CTRL);
 		}
+
+		/*
+		 * On i.MX8MM, we are running Dual Linux OS, with 1st Linux using SD Card
+		 * as rootfs storage, 2nd Linux using eMMC as rootfs storage. We let the
+		 * the 1st linux configure power/clock for the 2nd Linux.
+		 *
+		 * When the 2nd Linux is booting into rootfs stage, we let the 1st Linux
+		 * to destroy the 2nd linux, then restart the 2nd linux, we met SDHCI dump.
+		 * After we clear the pending interrupt and halt CQCTL, issue gone.
+		 */
+		if (cq_host) {
+			tmp = cqhci_readl(cq_host, CQHCI_IS);
+			cqhci_writel(cq_host, tmp, CQHCI_IS);
+			cqhci_writel(cq_host, CQHCI_HALT, CQHCI_CTL);
+		}
 	}
 }
 
-- 
2.17.1

[PATCH v2 12/14] mmc: sdhci-esdhc-imx: restore pin state when resume back

[BOUGH CHEN]

In some low power mode, SoC will lose the pin state, so need to restore
the pin state when resume back.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index ae1538e30b23..246b4eb52378 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1690,7 +1690,13 @@ static int sdhci_esdhc_suspend(struct device *dev)
 	if (host->tuning_mode != SDHCI_TUNING_MODE_3)
 		mmc_retune_needed(host->mmc);
 
-	return sdhci_suspend_host(host);
+	ret = sdhci_suspend_host(host);
+	if (!ret)
+		if (pinctrl_pm_select_sleep_state(dev))
+			dev_warn(mmc_dev(host->mmc),
+			 "%s, failed to select sleep pin state!\n", __func__);
+
+	return ret;
 }
 
 static int sdhci_esdhc_resume(struct device *dev)
@@ -1698,6 +1704,10 @@ static int sdhci_esdhc_resume(struct device *dev)
 	struct sdhci_host *host = dev_get_drvdata(dev);
 	int ret;
 
+	if (pinctrl_pm_select_default_state(dev))
+		dev_warn(mmc_dev(host->mmc),
+		 "%s, failed to select default pin state!\n", __func__);
+
 	/* re-initialize hw state in case it's lost in low power mode */
 	sdhci_esdhc_imx_hwinit(host);
 
-- 
2.17.1

[PATCH v2 13/14] mmc: sdhci-esdhc-imx: clear DMA_SEL when disable DMA mode

[BOUGH CHEN]

Currently, when use standard tuning, driver default disable DMA just before
send tuning command. But on i.MX8 usdhc, this is not enough. Need also clear
DMA_SEL. If not, once the DMA_SEL select AMDA2 before, even dma already disabled,
when send tuning command, usdhc will still prefetch the ADMA script from wrong
DMA address, then we will see IOMMU report some error which show lack of TLB
mapping.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 246b4eb52378..7430708f72e1 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -608,10 +608,24 @@ static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
 			 * For DMA access restore the levels to default value.
 			 */
 			m = readl(host->ioaddr + ESDHC_WTMK_LVL);
-			if (val & SDHCI_TRNS_DMA)
+			if (val & SDHCI_TRNS_DMA) {
 				wml = ESDHC_WTMK_LVL_WML_VAL_DEF;
-			else
+			} else {
+				u8 ctrl;
 				wml = ESDHC_WTMK_LVL_WML_VAL_MAX;
+
+				/*
+				 * Since already disable DMA mode, so also need
+				 * to clear the DMASEL. Otherwise, for standard
+				 * tuning, when send tuning command, usdhc will
+				 * still prefetch the ADMA script from wrong
+				 * DMA address, then we will see IOMMU report
+				 * some error which show lack of TLB mapping.
+				 */
+				ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+				ctrl &= ~SDHCI_CTRL_DMA_MASK;
+				sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+			}
 			m &= ~(ESDHC_WTMK_LVL_RD_WML_MASK |
 			       ESDHC_WTMK_LVL_WR_WML_MASK);
 			m |= (wml << ESDHC_WTMK_LVL_RD_WML_SHIFT) |
-- 
2.17.1

[PATCH v2 14/14] mmc: queue: create dev->dma_parms before call dma_set_max_seg_size()

[BOUGH CHEN]

To make dma_set_max_seg_size() work, need to create dev->dma_parms.

Find this issue on i.MX8QM mek board, this platform config the
max_segment_size to 65535, but this dma_set_max_seg_size do not
actuall work, find sometimes the segment size is 65536, exceed
the hardware max segment limitation, trigger ADMA error.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/core/queue.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index 9edc08685e86..f74c28c58482 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -359,6 +359,7 @@ static const struct blk_mq_ops mmc_mq_ops = {
 static void mmc_setup_queue(struct mmc_queue *mq, struct mmc_card *card)
 {
 	struct mmc_host *host = card->host;
+	struct device *dev = mmc_dev(host);
 	unsigned block_size = 512;
 
 	blk_queue_flag_set(QUEUE_FLAG_NONROT, mq->queue);
@@ -366,13 +367,12 @@ static void mmc_setup_queue(struct mmc_queue *mq, struct mmc_card *card)
 	if (mmc_can_erase(card))
 		mmc_queue_setup_discard(mq->queue, card);
 
-	if (!mmc_dev(host)->dma_mask || !*mmc_dev(host)->dma_mask)
+	if (!dev->dma_mask || !*dev->dma_mask)
 		blk_queue_bounce_limit(mq->queue, BLK_BOUNCE_HIGH);
 	blk_queue_max_hw_sectors(mq->queue,
 		min(host->max_blk_count, host->max_req_size / 512));
 	if (host->can_dma_map_merge)
-		WARN(!blk_queue_can_use_dma_map_merging(mq->queue,
-							mmc_dev(host)),
+		WARN(!blk_queue_can_use_dma_map_merging(mq->queue, dev),
 		     "merging was advertised but not possible");
 	blk_queue_max_segments(mq->queue, mmc_get_max_segments(host));
 
@@ -389,7 +389,8 @@ static void mmc_setup_queue(struct mmc_queue *mq, struct mmc_card *card)
 		blk_queue_max_segment_size(mq->queue,
 			round_down(host->max_seg_size, block_size));
 
-	dma_set_max_seg_size(mmc_dev(host), queue_max_segment_size(mq->queue));
+	dev->dma_parms = devm_kzalloc(dev, sizeof(*dev->dma_parms), GFP_KERNEL);
+	dma_set_max_seg_size(dev, queue_max_segment_size(mq->queue));
 
 	INIT_WORK(&mq->recovery_work, mmc_mq_recovery_handler);
 	INIT_WORK(&mq->complete_work, mmc_blk_mq_complete_work);
-- 
2.17.1

Re: [PATCH v2 02/14] mmc: sdhci-esdhc-imx: no fail when no pinctrl available

[Linus Walleij]

On Tue, Dec 3, 2019 at 1:54 PM BOUGH CHEN <haibo.chen@nxp.com> wrote:

> When using jailhouse to support two Linux on i.MX8MQ EVK, we use the
> 1st Linux to configure pinctrl for the 2nd Linux. Then the 2nd Linux
> could use the mmc without pinctrl driver.
>
> So give a warning message when no pinctrl available, but no fail probe.
>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>

Wow what an exotic thing.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v2 09/14] doc: dt: fsl-imx-esdhc: add auto-cmd23-broken binding

[Linus Walleij]

On Tue, Dec 3, 2019 at 1:54 PM BOUGH CHEN <haibo.chen@nxp.com> wrote:

> +- sdhci,auto-cmd23-broken: disable the ACMD23 function of USDHC.
> +  This is required for eMMC on imx6qpdl/imx6sx/imx7d when it use ADMA mode. Because
> +  for these SoC, it do not support the ACMD23 completely, only take the 16 bit block
> +  count from the 0x4 register (BLK_ATT) as argument for the ACMD23, the upper 16 bit
> +  of the CMD23's argument is ignored. This will impact the reliable write operation
> +  and the RPMB block write operation, because these operations need to set the bit 31
> +  of the CMD23's argument. SDMA mode will default disable the ACMD23 mode. SD card do
> +  not has this limitation on these SoCs.

This looks weird.

Is the bug in the *host controller* or in *the card*?

It looks like the card.

And then it should just be a quirk for this eMMC card in
drivers/mmc/core/quirks.h plus corresponding code in
the core.

Otherwise everyone using this eMMC card has to add this
property in their device tree and that is wrong if we can already
detect it from the card ID and apply it for that card, anywhere.

This is not the first time I see card quirks as DT properties,
it seems like a bit over-reliance on DT.

Yours,
Linus Walleij

Re: [PATCH v2 10/14] mmc: sdhci-esdhc-imx: handle 'sdhci,auto-cmd23-broken' from devicetree

[Linus Walleij]

On Tue, Dec 3, 2019 at 1:54 PM BOUGH CHEN <haibo.chen@nxp.com> wrote:

> Since L4.15, community involve the commit 105819c8a545 ("mmc: core: use mrq->sbc
> when sending CMD23 for RPMB"), let the usdhc to decide whether to use ACMD23 for
> RPMB. This CMD23 for RPMB need to set the bit 31 to its argument, if not, the
> RPMB write operation will return general fail.
>
> According to the sdhci logic, SDMA mode will disable the ACMD23, and only in
> ADMA mode, it will chose to use ACMD23 if the host support. But according to
> debug, and confirm with IC, the imx6qpdl/imx6sx/imx6sl/imx7d do not support
> the ACMD23 feature completely. These SoCs only use the 16 bit block count of
> the register 0x4 (BLOCK_ATT) as the CMD23's argument in ACMD23 mode, which
> means it will ignore the upper 16 bit of the CMD23's argument. This will block
> the reliable write operation in RPMB, because RPMB reliable write need to set
> the bit31 of the CMD23's argument. This is the hardware limitation. Due to
> imx6sl use SDMA, so for imx6qpdl/imx6sx/imx7d, it need to broke the ACMD23 for
> eMMC, SD card do not has this limitation, because SD card do not support reliable
> write.
>
> For imx6ul/imx6ull/imx6sll/imx7ulp/imx8, it support the ACMD23 completely, it
> change to use the 0x0 register (DS_ADDR) to put the CMD23's argument in ADMA mode.
>
> This patch handle 'sdhci,auto-cmd23-broken' from devicetree.
>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>

So it does seem like this is a problem with the host controller, not
with the card?

> -       if (of_find_property(np, "no-1-8-v", NULL))
> -               host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;

Why are you deleting this? Seems unrelated.

> +       /* call to generic sdhci_get_property to support additional capabilities */
> +       sdhci_get_property(pdev);

Or does it get set by this call?
(Then explain that change in the commit message.)

> +       if (device_property_present(dev, "sdhci,auto-cmd23-broken"))
> +               host->quirks2 |= SDHCI_QUIRK2_ACMD23_BROKEN;

Even if this is a problem with the host controller I am not convinced.

Can't you just use the compatible-string of the host controller to
switch this flag?

They should be different compatibles because if this bug is present
in one version of the host controller and not another version then
by definition they are not compatible, right?

The idea with very specific compatible strings is to handle
exactly things like this.

Yours,
Linus Walleij

Re: [PATCH v2 09/14] doc: dt: fsl-imx-esdhc: add auto-cmd23-broken binding

[Linus Walleij]

On Wed, Dec 11, 2019 at 12:32 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Dec 3, 2019 at 1:54 PM BOUGH CHEN <haibo.chen@nxp.com> wrote:
>
> > +- sdhci,auto-cmd23-broken: disable the ACMD23 function of USDHC.
> > +  This is required for eMMC on imx6qpdl/imx6sx/imx7d when it use ADMA mode. Because
> > +  for these SoC, it do not support the ACMD23 completely, only take the 16 bit block
> > +  count from the 0x4 register (BLK_ATT) as argument for the ACMD23, the upper 16 bit
> > +  of the CMD23's argument is ignored. This will impact the reliable write operation
> > +  and the RPMB block write operation, because these operations need to set the bit 31
> > +  of the CMD23's argument. SDMA mode will default disable the ACMD23 mode. SD card do
> > +  not has this limitation on these SoCs.
>
> This looks weird.
>
> Is the bug in the *host controller* or in *the card*?
>
> It looks like the card.

After looking at the next patch it looks like the host controller.

In that case the compatible-string should indicate what version of
the IP you are using and if it has this bug.

No special flags needed for that.

Yours,
Linus Walleij

RE: [PATCH v2 09/14] doc: dt: fsl-imx-esdhc: add auto-cmd23-broken binding

[BOUGH CHEN]

> -----Original Message-----
> From: Linus Walleij <linus.walleij@linaro.org>
> Sent: 2019年12月11日 7:40
> To: BOUGH CHEN <haibo.chen@nxp.com>
> Cc: ulf.hansson@linaro.org; adrian.hunter@intel.com;
> shawnguo@kernel.org; kernel@pengutronix.de; dl-linux-imx
> <linux-imx@nxp.com>; linux-mmc@vger.kernel.org
> Subject: Re: [PATCH v2 09/14] doc: dt: fsl-imx-esdhc: add
> auto-cmd23-broken binding
> 
> On Wed, Dec 11, 2019 at 12:32 AM Linus Walleij <linus.walleij@linaro.org>
> wrote:
> > On Tue, Dec 3, 2019 at 1:54 PM BOUGH CHEN <haibo.chen@nxp.com>
> wrote:
> >
> > > +- sdhci,auto-cmd23-broken: disable the ACMD23 function of USDHC.
> > > +  This is required for eMMC on imx6qpdl/imx6sx/imx7d when it use
> > > +ADMA mode. Because
> > > +  for these SoC, it do not support the ACMD23 completely, only take
> > > +the 16 bit block
> > > +  count from the 0x4 register (BLK_ATT) as argument for the ACMD23,
> > > +the upper 16 bit
> > > +  of the CMD23's argument is ignored. This will impact the reliable
> > > +write operation
> > > +  and the RPMB block write operation, because these operations need
> > > +to set the bit 31
> > > +  of the CMD23's argument. SDMA mode will default disable the
> > > +ACMD23 mode. SD card do
> > > +  not has this limitation on these SoCs.
> >
> > This looks weird.
> >
> > Is the bug in the *host controller* or in *the card*?
> >
> > It looks like the card.
> 
> After looking at the next patch it looks like the host controller.
> 
> In that case the compatible-string should indicate what version of the IP you
> are using and if it has this bug.
> 
> No special flags needed for that.

Hi Linus,

Yes, this is host IP limitation. I did consider the method as you suggested, use compatible-string to distinguish. But then I notice that this host limitation only impact the eMMC device for RPMB reliable write, for SD card, it do not support this mode, so this hardware limitation do not impact for sd card.
This is why I use "sdhci,auto-cmd23-broken" in devicetree, only the emmc device need to contain this in dts file.

I double check this issue, since this auto-cmd23 will not impact the sd performance, and it is the host IP limitation, I will chose to accept your suggestion, will send a new patch.

Thanks,

Bough Chen


> 
> Yours,
> Linus Walleij

Re: [PATCH v2 09/14] doc: dt: fsl-imx-esdhc: add auto-cmd23-broken binding

[Linus Walleij]

On Mon, Feb 10, 2020 at 4:08 AM BOUGH CHEN <haibo.chen@nxp.com> wrote:
> > On Wed, Dec 11, 2019 at 12:32 AM Linus Walleij <linus.walleij@linaro.org>
> > wrote:
> > > On Tue, Dec 3, 2019 at 1:54 PM BOUGH CHEN <haibo.chen@nxp.com>
> > wrote:
> > >
> > > > +- sdhci,auto-cmd23-broken: disable the ACMD23 function of USDHC.
> > > > +  This is required for eMMC on imx6qpdl/imx6sx/imx7d when it use
> > > > +ADMA mode. Because
> > > > +  for these SoC, it do not support the ACMD23 completely, only take
> > > > +the 16 bit block
> > > > +  count from the 0x4 register (BLK_ATT) as argument for the ACMD23,
> > > > +the upper 16 bit
> > > > +  of the CMD23's argument is ignored. This will impact the reliable
> > > > +write operation
> > > > +  and the RPMB block write operation, because these operations need
> > > > +to set the bit 31
> > > > +  of the CMD23's argument. SDMA mode will default disable the
> > > > +ACMD23 mode. SD card do
> > > > +  not has this limitation on these SoCs.
> > >
> > > This looks weird.
> > >
> > > Is the bug in the *host controller* or in *the card*?
> > >
> > > It looks like the card.
> >
> > After looking at the next patch it looks like the host controller.
> >
> > In that case the compatible-string should indicate what version of the IP you
> > are using and if it has this bug.
> >
> > No special flags needed for that.
>
> Hi Linus,
>
> Yes, this is host IP limitation. I did consider the method as you suggested,
>  use compatible-string to distinguish. But then I notice that this host limitation
> only impact the eMMC device for RPMB reliable write, for SD card, it do not
> support this mode, so this hardware limitation do not impact for sd card.
> This is why I use "sdhci,auto-cmd23-broken" in devicetree, only the emmc
> device need to contain this in dts file.
>
> I double check this issue, since this auto-cmd23 will not impact the sd
> performance, and it is the host IP limitation, I will chose to accept your
> suggestion, will send a new patch.

Should be fine, the code in the MMC core knows if what you are using
is an eMMC or an SD card so you can just handle the quirk only in the
eMMC case.

Yours,
Linus Walleij

[PATCH v2 09/14] doc: dt: fsl-imx-esdhc: add auto-cmd23-broken binding

[BOUGH CHEN]

Add sdhci,auto-cmd23-broken binding.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
index 89b986a2f9d5..7ea8e0f14e5f 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
@@ -46,6 +46,14 @@ Optional properties:
   property allows user to change the delay target for the strobe input read clock.
   If not use this property, driver default set the delay target to value 7.
   Only eMMC HS400 mode need to take care of this property.
+- sdhci,auto-cmd23-broken: disable the ACMD23 function of USDHC.
+  This is required for eMMC on imx6qpdl/imx6sx/imx7d when it use ADMA mode. Because
+  for these SoC, it do not support the ACMD23 completely, only take the 16 bit block
+  count from the 0x4 register (BLK_ATT) as argument for the ACMD23, the upper 16 bit
+  of the CMD23's argument is ignored. This will impact the reliable write operation
+  and the RPMB block write operation, because these operations need to set the bit 31
+  of the CMD23's argument. SDMA mode will default disable the ACMD23 mode. SD card do
+  not has this limitation on these SoCs.
 
 Examples:
 
-- 
2.17.1