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

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

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

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 6bcdc5743d94..3359153304a3 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)
 
@@ -993,6 +994,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 */
@@ -1010,9 +1014,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);
@@ -1338,6 +1346,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 v3 07/14] mmc: sdhci-esdhc-imx: optimize the clock setting

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

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 3359153304a3..6ca01b766604 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>
@@ -312,6 +313,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);
@@ -525,6 +537,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);
@@ -753,12 +767,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) {
@@ -813,13 +829,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)
@@ -1003,6 +1024,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 v3 08/14] mmc: sdhci-esdhc-imx: optimize the strobe dll setting

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

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 6ca01b766604..1ec35cc45fc5 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1019,6 +1019,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) &
@@ -1044,15 +1045,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 v3 09/14] mmc: sdhci-esdhc-imx: add flag ESDHC_FLAG_BROKEN_AUTO_CMD23

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

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. So for
imx6qpdl/imx6sx/imx6sl/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 add a new flag ESDHC_FLAG_BROKEN_AUTO_CMD23, and add this flag to
imx6q/imx6sx/imx6sl/imx7d, and due to the imx6sll share the same compatible string
with imx6sx before, and imx6sll do not has this limitation, so add a new compatible
string for imx6sll.

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

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 1ec35cc45fc5..98b91b6ff2a9 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -166,6 +166,18 @@
 #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)
+/*
+ * The IP do not support the ACMD23 feature completely when use ADMA mode.
+ * In ADMA mode, it only use the 16 bit block count of the register 0x4
+ * (BLOCK_ATT) as the CMD23's argument for 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.
+ * imx6qpdl/imx6sx/imx6sl/imx7d has this limitation only for ADMA mode, SDMA
+ * do not has this limitation. so when these SoC use ADMA mode, it need to
+ * disable the ACMD23 feature.
+ */
+#define ESDHC_FLAG_BROKEN_AUTO_CMD23	BIT(16)
 
 struct esdhc_soc_data {
 	u32 flags;
@@ -188,21 +200,30 @@ static const struct esdhc_soc_data esdhc_imx53_data = {
 };
 
 static const struct esdhc_soc_data usdhc_imx6q_data = {
-	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING,
+	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
+			| ESDHC_FLAG_BROKEN_AUTO_CMD23,
 };
 
 static const struct esdhc_soc_data usdhc_imx6sl_data = {
 	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
 			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_ERR004536
-			| ESDHC_FLAG_HS200,
+			| ESDHC_FLAG_HS200
+			| ESDHC_FLAG_BROKEN_AUTO_CMD23,
 };
 
-static const struct esdhc_soc_data usdhc_imx6sx_data = {
+static const struct esdhc_soc_data usdhc_imx6sll_data = {
 	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
 			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
 			| ESDHC_FLAG_STATE_LOST_IN_LPMODE,
 };
 
+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_STATE_LOST_IN_LPMODE
+			| ESDHC_FLAG_BROKEN_AUTO_CMD23,
+};
+
 static const struct esdhc_soc_data usdhc_imx6ull_data = {
 	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
 			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
@@ -214,7 +235,8 @@ 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_STATE_LOST_IN_LPMODE,
+			| ESDHC_FLAG_STATE_LOST_IN_LPMODE
+			| ESDHC_FLAG_BROKEN_AUTO_CMD23,
 };
 
 static struct esdhc_soc_data usdhc_imx7ulp_data = {
@@ -276,6 +298,7 @@ static const struct of_device_id imx_esdhc_dt_ids[] = {
 	{ .compatible = "fsl,imx53-esdhc", .data = &esdhc_imx53_data, },
 	{ .compatible = "fsl,imx6sx-usdhc", .data = &usdhc_imx6sx_data, },
 	{ .compatible = "fsl,imx6sl-usdhc", .data = &usdhc_imx6sl_data, },
+	{ .compatible = "fsl,imx6sll-usdhc", .data = &usdhc_imx6sll_data, },
 	{ .compatible = "fsl,imx6q-usdhc", .data = &usdhc_imx6q_data, },
 	{ .compatible = "fsl,imx6ull-usdhc", .data = &usdhc_imx6ull_data, },
 	{ .compatible = "fsl,imx7d-usdhc", .data = &usdhc_imx7d_data, },
@@ -1560,6 +1583,9 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	if (imx_data->socdata->flags & ESDHC_FLAG_HS400)
 		host->quirks2 |= SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400;
 
+	if (imx_data->socdata->flags & ESDHC_FLAG_BROKEN_AUTO_CMD23)
+		host->quirks2 |= SDHCI_QUIRK2_ACMD23_BROKEN;
+
 	if (imx_data->socdata->flags & ESDHC_FLAG_HS400_ES) {
 		host->mmc->caps2 |= MMC_CAP2_HS400_ES;
 		host->mmc_host_ops.hs400_enhanced_strobe =
-- 
2.17.1

[PATCH v3 10/14] mmc: sdhci-esdhc-imx: Add an new esdhc_soc_data for i.MX8MM

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

Add new esdhc_soc_data, with compatible string "fsl,imx8mm-usdhc".

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

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 98b91b6ff2a9..100c081cfcae 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -255,6 +255,14 @@ static struct esdhc_soc_data usdhc_imx8qxp_data = {
 			| ESDHC_FLAG_CLK_RATE_LOST_IN_PM_RUNTIME,
 };
 
+static struct esdhc_soc_data usdhc_imx8mm_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_STATE_LOST_IN_LPMODE,
+};
+
 struct pltfm_imx_data {
 	u32 scratchpad;
 	struct pinctrl *pinctrl;
@@ -304,6 +312,7 @@ static const struct of_device_id imx_esdhc_dt_ids[] = {
 	{ .compatible = "fsl,imx7d-usdhc", .data = &usdhc_imx7d_data, },
 	{ .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, },
 	{ .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, },
+	{ .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, imx_esdhc_dt_ids);
-- 
2.17.1

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

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

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 100c081cfcae..106097cbd0d4 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1233,6 +1233,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)) {
@@ -1309,6 +1310,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 v3 12/14] mmc: sdhci-esdhc-imx: restore pin state when resume back

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

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 106097cbd0d4..dedc067cd0dd 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1717,7 +1717,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)
@@ -1725,6 +1731,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 v3 13/14] mmc: sdhci-esdhc-imx: clear DMA_SEL when disable DMA mode

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

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 dedc067cd0dd..44837e3014e8 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -639,10 +639,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 v3 14/14] mmc: queue: create dev->dma_parms before call dma_set_max_seg_size()

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

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 v3 09/14] mmc: sdhci-esdhc-imx: add flag ESDHC_FLAG_BROKEN_AUTO_CMD23

[Adrian Hunter]

On 10/02/20 10:49 am, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> 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. So for
> imx6qpdl/imx6sx/imx6sl/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 add a new flag ESDHC_FLAG_BROKEN_AUTO_CMD23, and add this flag to
> imx6q/imx6sx/imx6sl/imx7d, and due to the imx6sll share the same compatible string
> with imx6sx before, and imx6sll do not has this limitation, so add a new compatible
> string for imx6sll.
> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 34 ++++++++++++++++++++++++++----
>  1 file changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 1ec35cc45fc5..98b91b6ff2a9 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -166,6 +166,18 @@
>  #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)
> +/*
> + * The IP do not support the ACMD23 feature completely when use ADMA mode.
> + * In ADMA mode, it only use the 16 bit block count of the register 0x4
> + * (BLOCK_ATT) as the CMD23's argument for 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.
> + * imx6qpdl/imx6sx/imx6sl/imx7d has this limitation only for ADMA mode, SDMA
> + * do not has this limitation. so when these SoC use ADMA mode, it need to
> + * disable the ACMD23 feature.
> + */
> +#define ESDHC_FLAG_BROKEN_AUTO_CMD23	BIT(16)
>  
>  struct esdhc_soc_data {
>  	u32 flags;
> @@ -188,21 +200,30 @@ static const struct esdhc_soc_data esdhc_imx53_data = {
>  };
>  
>  static const struct esdhc_soc_data usdhc_imx6q_data = {
> -	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING,
> +	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
> +			| ESDHC_FLAG_BROKEN_AUTO_CMD23,
>  };
>  
>  static const struct esdhc_soc_data usdhc_imx6sl_data = {
>  	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
>  			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_ERR004536
> -			| ESDHC_FLAG_HS200,
> +			| ESDHC_FLAG_HS200
> +			| ESDHC_FLAG_BROKEN_AUTO_CMD23,
>  };
>  
> -static const struct esdhc_soc_data usdhc_imx6sx_data = {
> +static const struct esdhc_soc_data usdhc_imx6sll_data = {
>  	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
>  			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
>  			| ESDHC_FLAG_STATE_LOST_IN_LPMODE,
>  };
>  
> +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_STATE_LOST_IN_LPMODE
> +			| ESDHC_FLAG_BROKEN_AUTO_CMD23,
> +};
> +
>  static const struct esdhc_soc_data usdhc_imx6ull_data = {
>  	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
>  			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> @@ -214,7 +235,8 @@ 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_STATE_LOST_IN_LPMODE,
> +			| ESDHC_FLAG_STATE_LOST_IN_LPMODE
> +			| ESDHC_FLAG_BROKEN_AUTO_CMD23,
>  };
>  
>  static struct esdhc_soc_data usdhc_imx7ulp_data = {
> @@ -276,6 +298,7 @@ static const struct of_device_id imx_esdhc_dt_ids[] = {
>  	{ .compatible = "fsl,imx53-esdhc", .data = &esdhc_imx53_data, },
>  	{ .compatible = "fsl,imx6sx-usdhc", .data = &usdhc_imx6sx_data, },
>  	{ .compatible = "fsl,imx6sl-usdhc", .data = &usdhc_imx6sl_data, },
> +	{ .compatible = "fsl,imx6sll-usdhc", .data = &usdhc_imx6sll_data, },
>  	{ .compatible = "fsl,imx6q-usdhc", .data = &usdhc_imx6q_data, },
>  	{ .compatible = "fsl,imx6ull-usdhc", .data = &usdhc_imx6ull_data, },
>  	{ .compatible = "fsl,imx7d-usdhc", .data = &usdhc_imx7d_data, },
> @@ -1560,6 +1583,9 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
>  	if (imx_data->socdata->flags & ESDHC_FLAG_HS400)
>  		host->quirks2 |= SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400;
>  
> +	if (imx_data->socdata->flags & ESDHC_FLAG_BROKEN_AUTO_CMD23)
> +		host->quirks2 |= SDHCI_QUIRK2_ACMD23_BROKEN;
> +
>  	if (imx_data->socdata->flags & ESDHC_FLAG_HS400_ES) {
>  		host->mmc->caps2 |= MMC_CAP2_HS400_ES;
>  		host->mmc_host_ops.hs400_enhanced_strobe =
> 

Re: [PATCH v3 10/14] mmc: sdhci-esdhc-imx: Add an new esdhc_soc_data for i.MX8MM

[Adrian Hunter]

On 10/02/20 10:49 am, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> Add new esdhc_soc_data, with compatible string "fsl,imx8mm-usdhc".
> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 98b91b6ff2a9..100c081cfcae 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -255,6 +255,14 @@ static struct esdhc_soc_data usdhc_imx8qxp_data = {
>  			| ESDHC_FLAG_CLK_RATE_LOST_IN_PM_RUNTIME,
>  };
>  
> +static struct esdhc_soc_data usdhc_imx8mm_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_STATE_LOST_IN_LPMODE,
> +};
> +
>  struct pltfm_imx_data {
>  	u32 scratchpad;
>  	struct pinctrl *pinctrl;
> @@ -304,6 +312,7 @@ static const struct of_device_id imx_esdhc_dt_ids[] = {
>  	{ .compatible = "fsl,imx7d-usdhc", .data = &usdhc_imx7d_data, },
>  	{ .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, },
>  	{ .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, },
> +	{ .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, imx_esdhc_dt_ids);
> 

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

[Adrian Hunter]

On 10/02/20 10:49 am, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> 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>

Acked-by: Adrian Hunter <adrian.hunter@intel.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 100c081cfcae..106097cbd0d4 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -1233,6 +1233,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)) {
> @@ -1309,6 +1310,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);
> +		}
>  	}
>  }
>  
> 

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

[Adrian Hunter]

On 10/02/20 10:49 am, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> 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 106097cbd0d4..dedc067cd0dd 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -1717,7 +1717,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__);

It looks to me like pinctrl_pm_select_sleep_state() prints an error anyway
if it fails, so the warning here is redundant.

Also a comment about why it is OK to ignore an error could be added.

> +
> +	return ret;
>  }
>  
>  static int sdhci_esdhc_resume(struct device *dev)
> @@ -1725,6 +1731,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__);

Same as above

> +
>  	/* re-initialize hw state in case it's lost in low power mode */
>  	sdhci_esdhc_imx_hwinit(host);
>  
> 

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

[Adrian Hunter]

On 10/02/20 10:49 am, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> 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>

Acked-by: Adrian Hunter <adrian.hunter@intel.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 dedc067cd0dd..44837e3014e8 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -639,10 +639,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) |
> 

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

[Adrian Hunter]

On 10/02/20 10:49 am, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> 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);

Wouldn't it be more logical to keep existing dma_parms? i.e.

	if (!dev->dma_parms)
		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);
> 

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

[BOUGH CHEN]

> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org
> <linux-mmc-owner@vger.kernel.org> On Behalf Of Adrian Hunter
> Sent: 2020年2月18日 15:58
> To: BOUGH CHEN <haibo.chen@nxp.com>; ulf.hansson@linaro.org;
> linux-mmc@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>; linus.walleij@linaro.org
> Subject: Re: [PATCH v3 12/14] mmc: sdhci-esdhc-imx: restore pin state when
> resume back
> 
> On 10/02/20 10:49 am, haibo.chen@nxp.com wrote:
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > 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 106097cbd0d4..dedc067cd0dd 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -1717,7 +1717,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__);
> 
> It looks to me like pinctrl_pm_select_sleep_state() prints an error anyway if
> it fails, so the warning here is redundant.

Okay, thanks for point out that!

> 
> Also a comment about why it is OK to ignore an error could be added.

Yes, I will add a comment like below, should it be okay?

/*
 * here ignore the error return, because current API sdhci_suspend_host always return zero,
 * When the API sdhci_suspend_host change the return policy, need to change at here too.
 */

Best Regards
Bough chen
> 
> > +
> > +	return ret;
> >  }
> >
> >  static int sdhci_esdhc_resume(struct device *dev) @@ -1725,6 +1731,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__);
> 
> Same as above
> 
> > +
> >  	/* re-initialize hw state in case it's lost in low power mode */
> >  	sdhci_esdhc_imx_hwinit(host);
> >
> >

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

[BOUGH CHEN]

> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org
> <linux-mmc-owner@vger.kernel.org> On Behalf Of Adrian Hunter
> Sent: 2020年2月18日 16:16
> To: BOUGH CHEN <haibo.chen@nxp.com>; ulf.hansson@linaro.org;
> linux-mmc@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>; linus.walleij@linaro.org
> Subject: Re: [PATCH v3 14/14] mmc: queue: create dev->dma_parms before
> call dma_set_max_seg_size()
> 
> On 10/02/20 10:49 am, haibo.chen@nxp.com wrote:
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > 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);
> 
> Wouldn't it be more logical to keep existing dma_parms? i.e.
> 
> 	if (!dev->dma_parms)
> 		dev->dma_parms = devm_kzalloc(dev, sizeof(*dev->dma_parms),
> GFP_KERNEL);

Yes! I will do that.

Best Regards
Bough Chen
> 
> > +	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);
> >

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

[Hunter, Adrian]

> -----Original Message-----
> From: BOUGH CHEN <haibo.chen@nxp.com>
> Sent: Wednesday, February 19, 2020 9:16 AM
> To: Hunter, Adrian <adrian.hunter@intel.com>; ulf.hansson@linaro.org;
> linux-mmc@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>; linus.walleij@linaro.org
> Subject: RE: [PATCH v3 12/14] mmc: sdhci-esdhc-imx: restore pin state when
> resume back
> 
> > -----Original Message-----
> > From: linux-mmc-owner@vger.kernel.org
> > <linux-mmc-owner@vger.kernel.org> On Behalf Of Adrian Hunter
> > Sent: 2020年2月18日 15:58
> > To: BOUGH CHEN <haibo.chen@nxp.com>; ulf.hansson@linaro.org;
> > linux-mmc@vger.kernel.org
> > Cc: dl-linux-imx <linux-imx@nxp.com>; linus.walleij@linaro.org
> > Subject: Re: [PATCH v3 12/14] mmc: sdhci-esdhc-imx: restore pin state
> > when resume back
> >
> > On 10/02/20 10:49 am, haibo.chen@nxp.com wrote:
> > > From: Haibo Chen <haibo.chen@nxp.com>
> > >
> > > 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 106097cbd0d4..dedc067cd0dd 100644
> > > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > > @@ -1717,7 +1717,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__);
> >
> > It looks to me like pinctrl_pm_select_sleep_state() prints an error
> > anyway if it fails, so the warning here is redundant.
> 
> Okay, thanks for point out that!
> 
> >
> > Also a comment about why it is OK to ignore an error could be added.
> 
> Yes, I will add a comment like below, should it be okay?

I meant why it is ok to ignore the pinctrl error

> 
> /*
>  * here ignore the error return, because current API sdhci_suspend_host
> always return zero,
>  * When the API sdhci_suspend_host change the return policy, need to
> change at here too.
>  */
> 
> Best Regards
> Bough chen
> >
> > > +
> > > +	return ret;
> > >  }
> > >
> > >  static int sdhci_esdhc_resume(struct device *dev) @@ -1725,6
> > > +1731,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__);
> >
> > Same as above
> >
> > > +
> > >  	/* re-initialize hw state in case it's lost in low power mode */
> > >  	sdhci_esdhc_imx_hwinit(host);
> > >
> > >

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

[BOUGH CHEN]

> -----Original Message-----
> From: Hunter, Adrian <adrian.hunter@intel.com>
> Sent: 2020年2月19日 15:29
> To: BOUGH CHEN <haibo.chen@nxp.com>; ulf.hansson@linaro.org;
> linux-mmc@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>; linus.walleij@linaro.org
> Subject: RE: [PATCH v3 12/14] mmc: sdhci-esdhc-imx: restore pin state when
> resume back
> 
> 
> 
> > -----Original Message-----
> > From: BOUGH CHEN <haibo.chen@nxp.com>
> > Sent: Wednesday, February 19, 2020 9:16 AM
> > To: Hunter, Adrian <adrian.hunter@intel.com>; ulf.hansson@linaro.org;
> > linux-mmc@vger.kernel.org
> > Cc: dl-linux-imx <linux-imx@nxp.com>; linus.walleij@linaro.org
> > Subject: RE: [PATCH v3 12/14] mmc: sdhci-esdhc-imx: restore pin state
> > when resume back
> >
> > > -----Original Message-----
> > > From: linux-mmc-owner@vger.kernel.org
> > > <linux-mmc-owner@vger.kernel.org> On Behalf Of Adrian Hunter
> > > Sent: 2020年2月18日 15:58
> > > To: BOUGH CHEN <haibo.chen@nxp.com>; ulf.hansson@linaro.org;
> > > linux-mmc@vger.kernel.org
> > > Cc: dl-linux-imx <linux-imx@nxp.com>; linus.walleij@linaro.org
> > > Subject: Re: [PATCH v3 12/14] mmc: sdhci-esdhc-imx: restore pin
> > > state when resume back
> > >
> > > On 10/02/20 10:49 am, haibo.chen@nxp.com wrote:
> > > > From: Haibo Chen <haibo.chen@nxp.com>
> > > >
> > > > 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 106097cbd0d4..dedc067cd0dd 100644
> > > > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > > > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > > > @@ -1717,7 +1717,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__);
> > >
> > > It looks to me like pinctrl_pm_select_sleep_state() prints an error
> > > anyway if it fails, so the warning here is redundant.
> >
> > Okay, thanks for point out that!
> >
> > >
> > > Also a comment about why it is OK to ignore an error could be added.
> >
> > Yes, I will add a comment like below, should it be okay?
> 
> I meant why it is ok to ignore the pinctrl error

Okay, I know your point. 
So I think this patch can change like below:

index 786305309eb0..b38b9d7f0a0d 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1731,7 +1731,11 @@ 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)
+               return pinctrl_pm_select_sleep_state(dev);
+
+       return ret;
 }

 static int sdhci_esdhc_resume(struct device *dev)
@@ -1739,6 +1743,10 @@ static int sdhci_esdhc_resume(struct device *dev)
        struct sdhci_host *host = dev_get_drvdata(dev);
        int ret;

+       ret = pinctrl_pm_select_default_state(dev);
+       if (ret)
+               return ret;
+
        /* re-initialize hw state in case it's lost in low power mode */
        sdhci_esdhc_imx_hwinit(host);

> 
> >
> > /*
> >  * here ignore the error return, because current API
> > sdhci_suspend_host always return zero,
> >  * When the API sdhci_suspend_host change the return policy, need to
> > change at here too.
> >  */
> >
> > Best Regards
> > Bough chen
> > >
> > > > +
> > > > +	return ret;
> > > >  }
> > > >
> > > >  static int sdhci_esdhc_resume(struct device *dev) @@ -1725,6
> > > > +1731,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__);
> > >
> > > Same as above
> > >
> > > > +
> > > >  	/* re-initialize hw state in case it's lost in low power mode */
> > > >  	sdhci_esdhc_imx_hwinit(host);
> > > >
> > > >