[U-Boot] [v2, 1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY for CMD with busy response

[U-Boot] [v2, 1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY for CMD with busy response

[Yangbo Lu]

For CMD with busy response, the eSDHC driver would poll DAT0 until
CMD completion rather than polling IRQSTAT. So, don't set
XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC) in IRQSTAT.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v2:
	- None
---
 drivers/mmc/fsl_esdhc.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index a865c7b..b23845d 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -136,8 +136,16 @@ static uint esdhc_xfertyp(struct mmc_cmd *cmd, struct mmc_data *data)
 		xfertyp |= XFERTYP_CICEN;
 	if (cmd->resp_type & MMC_RSP_136)
 		xfertyp |= XFERTYP_RSPTYP_136;
-	else if (cmd->resp_type & MMC_RSP_BUSY)
-		xfertyp |= XFERTYP_RSPTYP_48_BUSY;
+	/*
+	 * For CMD with busy response, the eSDHC driver would poll DAT0
+	 * until CMD completion rather than polling IRQSTAT. So, don't
+	 * set XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC)
+	 * in IRQSTAT.
+	 *
+	 * Remove:
+	 * else if (cmd->resp_type & MMC_RSP_BUSY)
+	 *      xfertyp |= XFERTYP_RSPTYP_48_BUSY;
+	 */
 	else if (cmd->resp_type & MMC_RSP_PRESENT)
 		xfertyp |= XFERTYP_RSPTYP_48;
 
-- 
2.1.0.27.g96db324

[U-Boot] [v2, 2/5] mmc: send STOP command when the READ/WRITE commands fail

[Yangbo Lu]

The STOP command should be sent to stop data transfer when the
READ/WRITE commands fail. Otherwise, any subsequent command will
fail to be sent.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v2:
	- None
---
 drivers/mmc/mmc.c         | 28 +++++++++++++++++++---------
 drivers/mmc/mmc_private.h |  1 +
 drivers/mmc/mmc_write.c   |  8 ++------
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index f8e5f7a..85d1e18 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -188,6 +188,21 @@ int mmc_set_blocklen(struct mmc *mmc, int len)
 	return mmc_send_cmd(mmc, &cmd, NULL);
 }
 
+int mmc_send_stop(struct mmc *mmc)
+{
+	struct mmc_cmd cmd;
+	int err;
+
+	cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
+	cmd.cmdarg = 0;
+	cmd.resp_type = MMC_RSP_R1b;
+
+	err = mmc_send_cmd(mmc, &cmd, NULL);
+	if (err)
+		printf("mmc fail to send stop cmd\n");
+	return err;
+}
+
 static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
 			   lbaint_t blkcnt)
 {
@@ -211,19 +226,14 @@ static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
 	data.blocksize = mmc->read_bl_len;
 	data.flags = MMC_DATA_READ;
 
-	if (mmc_send_cmd(mmc, &cmd, &data))
+	if (mmc_send_cmd(mmc, &cmd, &data)) {
+		mmc_send_stop(mmc);
 		return 0;
+	}
 
 	if (blkcnt > 1) {
-		cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
-		cmd.cmdarg = 0;
-		cmd.resp_type = MMC_RSP_R1b;
-		if (mmc_send_cmd(mmc, &cmd, NULL)) {
-#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
-			printf("mmc fail to send stop cmd\n");
-#endif
+		if (mmc_send_stop(mmc))
 			return 0;
-		}
 	}
 
 	return blkcnt;
diff --git a/drivers/mmc/mmc_private.h b/drivers/mmc/mmc_private.h
index 49ec022..2791125 100644
--- a/drivers/mmc/mmc_private.h
+++ b/drivers/mmc/mmc_private.h
@@ -16,6 +16,7 @@ extern int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 			struct mmc_data *data);
 extern int mmc_send_status(struct mmc *mmc, int timeout);
 extern int mmc_set_blocklen(struct mmc *mmc, int len);
+int mmc_send_stop(struct mmc *mmc);
 #ifdef CONFIG_FSL_ESDHC_ADAPTER_IDENT
 void mmc_adapter_card_type_ident(void);
 #endif
diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
index 0f8b5c7..fb8488c 100644
--- a/drivers/mmc/mmc_write.c
+++ b/drivers/mmc/mmc_write.c
@@ -150,6 +150,7 @@ static ulong mmc_write_blocks(struct mmc *mmc, lbaint_t start,
 
 	if (mmc_send_cmd(mmc, &cmd, &data)) {
 		printf("mmc write failed\n");
+		mmc_send_stop(mmc);
 		return 0;
 	}
 
@@ -157,13 +158,8 @@ static ulong mmc_write_blocks(struct mmc *mmc, lbaint_t start,
 	 * token, not a STOP_TRANSMISSION request.
 	 */
 	if (!mmc_host_is_spi(mmc) && blkcnt > 1) {
-		cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
-		cmd.cmdarg = 0;
-		cmd.resp_type = MMC_RSP_R1b;
-		if (mmc_send_cmd(mmc, &cmd, NULL)) {
-			printf("mmc fail to send stop cmd\n");
+		if (mmc_send_stop(mmc))
 			return 0;
-		}
 	}
 
 	/* Waiting for the ready status */
-- 
2.1.0.27.g96db324

[U-Boot] [v2, 3/5] mmc: fsl_esdhc: add error recovery for data transfer with Auto CMD12

[Yangbo Lu]

For data transfer with Auto CMD12, the host will not send an Auto
CMD12 to stop when the transfer fails. So this patch adds a flag
to indicate the READ/WRITE command error, and makes the driver
continue to send a CMD12 manually.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v2:
	- None
---
 drivers/mmc/fsl_esdhc.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index b23845d..80bc177 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -106,6 +106,9 @@ struct fsl_esdhc_priv {
 	int wp_enable;
 	struct gpio_desc cd_gpio;
 	struct gpio_desc wp_gpio;
+#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111
+	int rw_err;
+#endif
 };
 
 /* Return the XFERTYP flags for a given command and data packet */
@@ -362,8 +365,12 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 	struct fsl_esdhc *regs = priv->esdhc_regs;
 
 #ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111
-	if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION)
-		return 0;
+	if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION) {
+		if (priv->rw_err)
+			priv->rw_err = 0;
+		else
+			return 0;
+	}
 #endif
 
 	esdhc_write32(&regs->irqstat, -1);
@@ -518,6 +525,13 @@ out:
 		/* If this was CMD11, then notify that power cycle is needed */
 		if (cmd->cmdidx == SD_CMD_SWITCH_UHS18V)
 			printf("CMD11 to switch to 1.8V mode failed, card requires power cycle.\n");
+#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111
+		if (cmd->cmdidx == MMC_CMD_READ_SINGLE_BLOCK ||
+		    cmd->cmdidx == MMC_CMD_READ_MULTIPLE_BLOCK ||
+		    cmd->cmdidx == MMC_CMD_WRITE_SINGLE_BLOCK ||
+		    cmd->cmdidx == MMC_CMD_WRITE_MULTIPLE_BLOCK)
+			priv->rw_err = 1;
+#endif
 	}
 
 	esdhc_write32(&regs->irqstat, -1);
@@ -828,6 +842,10 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv)
 
 	priv->cfg.b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
 
+#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111
+	priv->rw_err = 0;
+#endif
+
 	mmc = mmc_create(&priv->cfg, priv);
 	if (mmc == NULL)
 		return -1;
-- 
2.1.0.27.g96db324

[U-Boot] [v2, 4/5] mmc: add workaround for eSDHC erratum A009620

[Yangbo Lu]

Erratum Title:
Data timeout error not getting set in case of command with busy
response (R1b) as well as for busy period after last write block
transfer.

Description:
In the event that a busy timeout occurs for a command with a busy
response (e.g. R1b response) as well as busy period after the last
write block, the eSDHC does not set the IRQSTAT[DTOE] bit or the
IRQSTAT[TC]. Therefore, the current command transfer is never completed.

Workaround:
Workaround for CMD with busy:
Don't set the XFRTYP[RSP]=2'b11 for commands with busy response and
poll the busy status of the card from the PRSSTAT[DLSL]

Workaround for busy period after last write block:
1. After the command completion interrupt (IRQSTAT[CC]), wait for
   de-assertion of PRSTAT[WTA].
2. Once PRSTAT[WTA] is de-asserted, start the software timer and poll
   the busy signal (DAT0) using PRSTAT[DLSL[0]].
3. Wait for DAT0 signal to go high (which indicate transfer complete)
   or software timer expiry (which indicate data timeout error).
4. Issue soft reset for data (SYSCTL[RSTD]).
5. In case of data timeout error (detected in step 3) perform the
   error recovery.

The workaround for CMD with busy has already been applied in eSDHC
driver. This patch is to add workaround for the 2nd issue.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v2:
	- Split original patch into config part and mmc part
---
 drivers/mmc/fsl_esdhc.c | 26 ++++++++++++++++++++++++++
 include/fsl_esdhc.h     |  1 +
 2 files changed, 27 insertions(+)

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index 80bc177..99cadae 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -482,6 +482,32 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 #ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
 		esdhc_pio_read_write(mmc, data);
 #else
+#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
+		int timeout = 5000;
+		if (data->flags & MMC_DATA_WRITE) {
+			while (esdhc_read32(&regs->prsstat) & PRSSTAT_WTA)
+				;
+
+			/* Poll on DATA0 line for 500 ms */
+			while (!(esdhc_read32(&regs->prsstat) & PRSSTAT_DAT0)) {
+				udelay(100);
+				timeout--;
+				if (timeout <= 0) {
+					err = TIMEOUT;
+					break;
+				}
+			}
+			if (!err) {
+				esdhc_write32(&regs->sysctl,
+					      esdhc_read32(&regs->sysctl) |
+					      SYSCTL_RSTD);
+				while ((esdhc_read32(&regs->sysctl) &
+					SYSCTL_RSTD))
+					;
+			}
+			goto out;
+		}
+#endif
 		do {
 			irqstat = esdhc_read32(&regs->irqstat);
 
diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc.h
index c6f4666..3f146f7 100644
--- a/include/fsl_esdhc.h
+++ b/include/fsl_esdhc.h
@@ -97,6 +97,7 @@
 #define PRSSTAT_CINS		(0x00010000)
 #define PRSSTAT_BREN		(0x00000800)
 #define PRSSTAT_BWEN		(0x00000400)
+#define PRSSTAT_WTA		(0x00000100)
 #define PRSSTAT_SDSTB		(0X00000008)
 #define PRSSTAT_DLA		(0x00000004)
 #define PRSSTAT_CICHB		(0x00000002)
-- 
2.1.0.27.g96db324

[U-Boot] [v2, 5/5] arch/arm, arch/powerpc: enable workaround for eSDHC erratum A009620

[Yangbo Lu]

This patch is to enable workaround for eSDHC erratum A009620. All the
affected platforms include PowerPC(P1010/P2020/P5020/P5040/T1024/T1040/
T2080/T4240) and ARM(LS1021A/LS1043A/LS2080A).

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v2:
	- Added this patch
	- Moved definition out of board files
---
 arch/arm/include/asm/arch-fsl-layerscape/config.h | 2 ++
 arch/arm/include/asm/arch-ls102xa/config.h        | 1 +
 arch/powerpc/include/asm/config_mpc85xx.h         | 8 ++++++++
 3 files changed, 11 insertions(+)

diff --git a/arch/arm/include/asm/arch-fsl-layerscape/config.h b/arch/arm/include/asm/arch-fsl-layerscape/config.h
index b0ad4b4..7f31fcd 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/config.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/config.h
@@ -32,6 +32,7 @@
 #define CONFIG_NUM_DDR_CONTROLLERS		3
 #define CONFIG_SYS_FSL_HAS_DP_DDR		/* Runtime check to confirm */
 #define CONFIG_SYS_FSL_CLUSTER_CLOCKS		{ 1, 1, 4, 4 }
+#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
 #define	SRDS_MAX_LANES	8
 #define CONFIG_SYS_FSL_SRDS_1
 #define CONFIG_SYS_FSL_SRDS_2
@@ -175,6 +176,7 @@
 #define CONFIG_SYS_NUM_FM1_DTSEC		7
 #define CONFIG_SYS_NUM_FM1_10GEC		1
 #define CONFIG_SYS_FSL_IFC_BANK_COUNT		4
+#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
 #define CONFIG_SYS_FSL_DDR_BE
 #define CONFIG_SYS_DDR_BLOCK1_SIZE		((phys_size_t)2 << 30)
 #define CONFIG_MAX_MEM_MAPPED			CONFIG_SYS_DDR_BLOCK1_SIZE
diff --git a/arch/arm/include/asm/arch-ls102xa/config.h b/arch/arm/include/asm/arch-ls102xa/config.h
index d408fe4..054f05d 100644
--- a/arch/arm/include/asm/arch-ls102xa/config.h
+++ b/arch/arm/include/asm/arch-ls102xa/config.h
@@ -131,6 +131,7 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT		1
 #define CONFIG_SYS_FSL_ERRATUM_A008378
 #define CONFIG_SYS_FSL_ERRATUM_A009663
+#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
 #define CONFIG_SYS_FSL_MAX_NUM_OF_SEC		1
 #else
 #error SoC not defined
diff --git a/arch/powerpc/include/asm/config_mpc85xx.h b/arch/powerpc/include/asm/config_mpc85xx.h
index 505d355..b3d8fe8 100644
--- a/arch/powerpc/include/asm/config_mpc85xx.h
+++ b/arch/powerpc/include/asm/config_mpc85xx.h
@@ -148,6 +148,7 @@
 #define CONFIG_TSECV2
 #define CONFIG_SYS_FSL_SEC_COMPAT	4
 #define CONFIG_SYS_FSL_ERRATUM_ESDHC111
+#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
 #define CONFIG_NUM_DDR_CONTROLLERS	1
 #define CONFIG_USB_MAX_CONTROLLER_COUNT	1
 #define CONFIG_SYS_FSL_IFC_BANK_COUNT	4
@@ -369,6 +370,7 @@
 #define CONFIG_SYS_CCSRBAR_DEFAULT	0xff700000
 #define CONFIG_SYS_FSL_ERRATUM_ESDHC111
 #define CONFIG_SYS_FSL_ERRATUM_ESDHC_A001
+#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
 #define CONFIG_SYS_FSL_SRIO_MAX_PORTS	2
 #define CONFIG_SYS_FSL_SRIO_OB_WIN_NUM	9
 #define CONFIG_SYS_FSL_SRIO_IB_WIN_NUM	5
@@ -530,6 +532,7 @@
 #define CONFIG_SYS_FSL_USB2_PHY_ENABLE
 #define CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY
 #define CONFIG_SYS_FSL_ERRATUM_ESDHC111
+#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
 #define CONFIG_SYS_FSL_ERRATUM_USB14
 #define CONFIG_SYS_FSL_ERRATUM_DDR_A003
 #define CONFIG_SYS_FSL_ERRATUM_DDR_A003474
@@ -568,6 +571,7 @@
 #define CONFIG_SYS_FSL_USB2_PHY_ENABLE
 #define CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY
 #define CONFIG_SYS_FSL_ERRATUM_ESDHC111
+#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
 #define CONFIG_SYS_FSL_ERRATUM_USB14
 #define CONFIG_SYS_FSL_ERRATUM_DDR_A003
 #define CONFIG_SYS_FSL_ERRATUM_DDR_A003474
@@ -686,6 +690,7 @@
 #define CONFIG_SYS_FSL_ERRATUM_A007186
 #define CONFIG_SYS_FSL_ERRATUM_A006593
 #define CONFIG_SYS_FSL_ERRATUM_A007798
+#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
 #define CONFIG_SYS_CCSRBAR_DEFAULT	0xfe000000
 #define CONFIG_SYS_FSL_SFP_VER_3_0
 #define CONFIG_SYS_FSL_PCI_VER_3_X
@@ -802,6 +807,7 @@ defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022)
 #define CONFIG_SYS_FSL_ERRATUM_A006261
 #define CONFIG_SYS_CCSRBAR_DEFAULT	0xfe000000
 #define CONFIG_SYS_FSL_ERRATUM_ESDHC111
+#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
 #define ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE
 #define QE_MURAM_SIZE			0x6000UL
 #define MAX_QE_RISC			1
@@ -823,6 +829,7 @@ defined(CONFIG_PPC_T1014) || defined(CONFIG_PPC_T1013)
 #endif
 #if defined(CONFIG_PPC_T1024) || defined(CONFIG_PPC_T1023)
 #define CONFIG_MAX_CPUS			2
+#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
 #elif defined(CONFIG_PPC_T1014) || defined(CONFIG_PPC_T1013)
 #define CONFIG_MAX_CPUS			1
 #endif
@@ -882,6 +889,7 @@ defined(CONFIG_PPC_T1014) || defined(CONFIG_PPC_T1013)
 #define CONFIG_SYS_FSL_SRIO_MAX_PORTS	2
 #define CONFIG_SYS_FSL_SRIO_OB_WIN_NUM	9
 #define CONFIG_SYS_FSL_SRIO_IB_WIN_NUM	5
+#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
 #elif defined(CONFIG_PPC_T2081)
 #define CONFIG_SYS_NUM_FM1_DTSEC	6
 #define CONFIG_SYS_NUM_FM1_10GEC	2
-- 
2.1.0.27.g96db324

[U-Boot] [v2, 1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY for CMD with busy response

[york sun]

Panto,

On 08/02/2016 02:32 AM, Yangbo Lu wrote:
> For CMD with busy response, the eSDHC driver would poll DAT0 until
> CMD completion rather than polling IRQSTAT. So, don't set
> XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC) in IRQSTAT.
>
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---
> Changes for v2:
> 	- None
> ---
>  drivers/mmc/fsl_esdhc.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>

I noticed this set wasn't CC'ing you. Can you check and comment/ack this 
set?

York

[U-Boot] [v2, 1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY for CMD with busy response

[Y.B. Lu]

Hi York,

I found the MMC maintainer had been changed to Jeahoon.
It seems Panto haven't commented on upstream for a very long time.

MMC
M:      Jaehoon Chung <jh80.chung@samsung.com>
S:      Maintained
T:      git git://git.denx.de/u-boot-mmc.git
F:      drivers/mmc/

Thanks.


Best regards,
Yangbo Lu


> -----Original Message-----
> From: york sun
> Sent: Tuesday, September 13, 2016 3:24 AM
> To: Y.B. Lu; u-boot at lists.denx.de; Pantelis Antoniou
> Cc: Jaehoon Chung
> Subject: Re: [v2, 1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY
> for CMD with busy response
> 
> Panto,
> 
> On 08/02/2016 02:32 AM, Yangbo Lu wrote:
> > For CMD with busy response, the eSDHC driver would poll DAT0 until CMD
> > completion rather than polling IRQSTAT. So, don't set
> > XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC) in IRQSTAT.
> >
> > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > ---
> > Changes for v2:
> > 	- None
> > ---
> >  drivers/mmc/fsl_esdhc.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> 
> I noticed this set wasn't CC'ing you. Can you check and comment/ack this
> set?
> 
> York

[U-Boot] [v2, 1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY for CMD with busy response

[Jaehoon Chung]

Hi Yangbo,

On 08/02/2016 06:20 PM, Yangbo Lu wrote:
> For CMD with busy response, the eSDHC driver would poll DAT0 until
> CMD completion rather than polling IRQSTAT. So, don't set
> XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC) in IRQSTAT.

Sorry for late.. I missed your patchset.

> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---
> Changes for v2:
> 	- None
> ---
>  drivers/mmc/fsl_esdhc.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index a865c7b..b23845d 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -136,8 +136,16 @@ static uint esdhc_xfertyp(struct mmc_cmd *cmd, struct mmc_data *data)
>  		xfertyp |= XFERTYP_CICEN;
>  	if (cmd->resp_type & MMC_RSP_136)
>  		xfertyp |= XFERTYP_RSPTYP_136;
> -	else if (cmd->resp_type & MMC_RSP_BUSY)
> -		xfertyp |= XFERTYP_RSPTYP_48_BUSY;
> +	/*
> +	 * For CMD with busy response, the eSDHC driver would poll DAT0
> +	 * until CMD completion rather than polling IRQSTAT. So, don't
> +	 * set XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC)
> +	 * in IRQSTAT.
> +	 *
> +	 * Remove:
> +	 * else if (cmd->resp_type & MMC_RSP_BUSY)
> +	 *      xfertyp |= XFERTYP_RSPTYP_48_BUSY;
> +	 */

I don't have the board that is using the fsl_esdhc driver.
I wonder that it doesn't need to set XFERTYP_RSPTYP_48_BUSY in future.
If so be, is it possible to remove this comments?

Why add this comment?

Best Regards,
Jaehoon Chung

>  	else if (cmd->resp_type & MMC_RSP_PRESENT)
>  		xfertyp |= XFERTYP_RSPTYP_48;
>  
> 

[U-Boot] [v2, 2/5] mmc: send STOP command when the READ/WRITE commands fail

[Jaehoon Chung]

Hi Yangbo,

On 08/02/2016 06:20 PM, Yangbo Lu wrote:
> The STOP command should be sent to stop data transfer when the
> READ/WRITE commands fail. Otherwise, any subsequent command will
> fail to be sent.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---
> Changes for v2:
> 	- None
> ---
>  drivers/mmc/mmc.c         | 28 +++++++++++++++++++---------
>  drivers/mmc/mmc_private.h |  1 +
>  drivers/mmc/mmc_write.c   |  8 ++------
>  3 files changed, 22 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index f8e5f7a..85d1e18 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -188,6 +188,21 @@ int mmc_set_blocklen(struct mmc *mmc, int len)
>  	return mmc_send_cmd(mmc, &cmd, NULL);
>  }
>  
> +int mmc_send_stop(struct mmc *mmc)
> +{
> +	struct mmc_cmd cmd;
> +	int err;
> +
> +	cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
> +	cmd.cmdarg = 0;
> +	cmd.resp_type = MMC_RSP_R1b;
> +
> +	err = mmc_send_cmd(mmc, &cmd, NULL);
> +	if (err)
> +		printf("mmc fail to send stop cmd\n");
> +	return err;
> +}
> +
>  static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
>  			   lbaint_t blkcnt)
>  {
> @@ -211,19 +226,14 @@ static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
>  	data.blocksize = mmc->read_bl_len;
>  	data.flags = MMC_DATA_READ;
>  
> -	if (mmc_send_cmd(mmc, &cmd, &data))
> +	if (mmc_send_cmd(mmc, &cmd, &data)) {
> +		mmc_send_stop(mmc);

If mmc_send_stop() is also failed, is it doesn't need to return error number?

>  		return 0;
> +	}
>  
>  	if (blkcnt > 1) {
> -		cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
> -		cmd.cmdarg = 0;
> -		cmd.resp_type = MMC_RSP_R1b;
> -		if (mmc_send_cmd(mmc, &cmd, NULL)) {
> -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> -			printf("mmc fail to send stop cmd\n");
> -#endif
> +		if (mmc_send_stop(mmc))
>  			return 0;
> -		}
>  	}
>  
>  	return blkcnt;
> diff --git a/drivers/mmc/mmc_private.h b/drivers/mmc/mmc_private.h
> index 49ec022..2791125 100644
> --- a/drivers/mmc/mmc_private.h
> +++ b/drivers/mmc/mmc_private.h
> @@ -16,6 +16,7 @@ extern int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
>  			struct mmc_data *data);
>  extern int mmc_send_status(struct mmc *mmc, int timeout);
>  extern int mmc_set_blocklen(struct mmc *mmc, int len);
> +int mmc_send_stop(struct mmc *mmc);
>  #ifdef CONFIG_FSL_ESDHC_ADAPTER_IDENT
>  void mmc_adapter_card_type_ident(void);
>  #endif
> diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
> index 0f8b5c7..fb8488c 100644
> --- a/drivers/mmc/mmc_write.c
> +++ b/drivers/mmc/mmc_write.c
> @@ -150,6 +150,7 @@ static ulong mmc_write_blocks(struct mmc *mmc, lbaint_t start,
>  
>  	if (mmc_send_cmd(mmc, &cmd, &data)) {
>  		printf("mmc write failed\n");
> +		mmc_send_stop(mmc);

ditto.

Best Regards,
Jaehoon Chung

>  		return 0;
>  	}
>  
> @@ -157,13 +158,8 @@ static ulong mmc_write_blocks(struct mmc *mmc, lbaint_t start,
>  	 * token, not a STOP_TRANSMISSION request.
>  	 */
>  	if (!mmc_host_is_spi(mmc) && blkcnt > 1) {
> -		cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
> -		cmd.cmdarg = 0;
> -		cmd.resp_type = MMC_RSP_R1b;
> -		if (mmc_send_cmd(mmc, &cmd, NULL)) {
> -			printf("mmc fail to send stop cmd\n");
> +		if (mmc_send_stop(mmc))
>  			return 0;
> -		}
>  	}
>  
>  	/* Waiting for the ready status */
> 

[U-Boot] [v2, 4/5] mmc: add workaround for eSDHC erratum A009620

[Jaehoon Chung]

On 08/02/2016 06:20 PM, Yangbo Lu wrote:
> Erratum Title:
> Data timeout error not getting set in case of command with busy
> response (R1b) as well as for busy period after last write block
> transfer.
> 
> Description:
> In the event that a busy timeout occurs for a command with a busy
> response (e.g. R1b response) as well as busy period after the last
> write block, the eSDHC does not set the IRQSTAT[DTOE] bit or the
> IRQSTAT[TC]. Therefore, the current command transfer is never completed.
> 
> Workaround:
> Workaround for CMD with busy:
> Don't set the XFRTYP[RSP]=2'b11 for commands with busy response and
> poll the busy status of the card from the PRSSTAT[DLSL]
> 
> Workaround for busy period after last write block:
> 1. After the command completion interrupt (IRQSTAT[CC]), wait for
>    de-assertion of PRSTAT[WTA].
> 2. Once PRSTAT[WTA] is de-asserted, start the software timer and poll
>    the busy signal (DAT0) using PRSTAT[DLSL[0]].
> 3. Wait for DAT0 signal to go high (which indicate transfer complete)
>    or software timer expiry (which indicate data timeout error).
> 4. Issue soft reset for data (SYSCTL[RSTD]).
> 5. In case of data timeout error (detected in step 3) perform the
>    error recovery.
> 
> The workaround for CMD with busy has already been applied in eSDHC
> driver. This patch is to add workaround for the 2nd issue.

This workaround is used the CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620?..
Then if this config is disabled..how is it handling?

> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---
> Changes for v2:
> 	- Split original patch into config part and mmc part
> ---
>  drivers/mmc/fsl_esdhc.c | 26 ++++++++++++++++++++++++++
>  include/fsl_esdhc.h     |  1 +
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index 80bc177..99cadae 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -482,6 +482,32 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>  #ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
>  		esdhc_pio_read_write(mmc, data);
>  #else
> +#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
> +		int timeout = 5000;
> +		if (data->flags & MMC_DATA_WRITE) {
> +			while (esdhc_read32(&regs->prsstat) & PRSSTAT_WTA)
> +				;

I don't want to have the potential infinite loop.

Best Regards,
Jaehoon Chung

> +
> +			/* Poll on DATA0 line for 500 ms */
> +			while (!(esdhc_read32(&regs->prsstat) & PRSSTAT_DAT0)) {
> +				udelay(100);
> +				timeout--;
> +				if (timeout <= 0) {
> +					err = TIMEOUT;
> +					break;
> +				}
> +			}
> +			if (!err) {
> +				esdhc_write32(&regs->sysctl,
> +					      esdhc_read32(&regs->sysctl) |
> +					      SYSCTL_RSTD);
> +				while ((esdhc_read32(&regs->sysctl) &
> +					SYSCTL_RSTD))
> +					;
> +			}
> +			goto out;
> +		}
> +#endif
>  		do {
>  			irqstat = esdhc_read32(&regs->irqstat);
>  
> diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc.h
> index c6f4666..3f146f7 100644
> --- a/include/fsl_esdhc.h
> +++ b/include/fsl_esdhc.h
> @@ -97,6 +97,7 @@
>  #define PRSSTAT_CINS		(0x00010000)
>  #define PRSSTAT_BREN		(0x00000800)
>  #define PRSSTAT_BWEN		(0x00000400)
> +#define PRSSTAT_WTA		(0x00000100)
>  #define PRSSTAT_SDSTB		(0X00000008)
>  #define PRSSTAT_DLA		(0x00000004)
>  #define PRSSTAT_CICHB		(0x00000002)
> 

[U-Boot] [v2, 1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY for CMD with busy response

[Jaehoon Chung]

On 09/13/2016 04:30 PM, Y.B. Lu wrote:
> Hi York,
> 
> I found the MMC maintainer had been changed to Jeahoon.
> It seems Panto haven't commented on upstream for a very long time.
> 
> MMC
> M:      Jaehoon Chung <jh80.chung@samsung.com>
> S:      Maintained
> T:      git git://git.denx.de/u-boot-mmc.git
> F:      drivers/mmc/
> 
> Thanks.

Thanks for reminding. I missed these patchset.
Sorry.

Best Regards,
Jaehoon Chung

> 
> 
> Best regards,
> Yangbo Lu
> 
> 
>> -----Original Message-----
>> From: york sun
>> Sent: Tuesday, September 13, 2016 3:24 AM
>> To: Y.B. Lu; u-boot at lists.denx.de; Pantelis Antoniou
>> Cc: Jaehoon Chung
>> Subject: Re: [v2, 1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY
>> for CMD with busy response
>>
>> Panto,
>>
>> On 08/02/2016 02:32 AM, Yangbo Lu wrote:
>>> For CMD with busy response, the eSDHC driver would poll DAT0 until CMD
>>> completion rather than polling IRQSTAT. So, don't set
>>> XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC) in IRQSTAT.
>>>
>>> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
>>> ---
>>> Changes for v2:
>>> 	- None
>>> ---
>>>  drivers/mmc/fsl_esdhc.c | 12 ++++++++++--
>>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>>
>>
>> I noticed this set wasn't CC'ing you. Can you check and comment/ack this
>> set?
>>
>> York
> 
> 
> 
> 

[U-Boot] [v2, 1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY for CMD with busy response

[Y.B. Lu]

> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung at samsung.com]
> Sent: Monday, September 19, 2016 8:07 AM
> To: Y.B. Lu; u-boot at lists.denx.de
> Cc: york sun
> Subject: Re: [v2, 1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY
> for CMD with busy response
> 
> Hi Yangbo,
> 
> On 08/02/2016 06:20 PM, Yangbo Lu wrote:
> > For CMD with busy response, the eSDHC driver would poll DAT0 until CMD
> > completion rather than polling IRQSTAT. So, don't set
> > XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC) in IRQSTAT.
> 
> Sorry for late.. I missed your patchset.
> 
> >
> > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > ---
> > Changes for v2:
> > 	- None
> > ---
> >  drivers/mmc/fsl_esdhc.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index
> > a865c7b..b23845d 100644
> > --- a/drivers/mmc/fsl_esdhc.c
> > +++ b/drivers/mmc/fsl_esdhc.c
> > @@ -136,8 +136,16 @@ static uint esdhc_xfertyp(struct mmc_cmd *cmd,
> struct mmc_data *data)
> >  		xfertyp |= XFERTYP_CICEN;
> >  	if (cmd->resp_type & MMC_RSP_136)
> >  		xfertyp |= XFERTYP_RSPTYP_136;
> > -	else if (cmd->resp_type & MMC_RSP_BUSY)
> > -		xfertyp |= XFERTYP_RSPTYP_48_BUSY;
> > +	/*
> > +	 * For CMD with busy response, the eSDHC driver would poll DAT0
> > +	 * until CMD completion rather than polling IRQSTAT. So, don't
> > +	 * set XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC)
> > +	 * in IRQSTAT.
> > +	 *
> > +	 * Remove:
> > +	 * else if (cmd->resp_type & MMC_RSP_BUSY)
> > +	 *      xfertyp |= XFERTYP_RSPTYP_48_BUSY;
> > +	 */
> 
> I don't have the board that is using the fsl_esdhc driver.
> I wonder that it doesn't need to set XFERTYP_RSPTYP_48_BUSY in future.
> If so be, is it possible to remove this comments?
> 
> Why add this comment?

[Lu Yangbo-B47093] I added this comment to explain why there isn't XFERTYP_RSPTYP_48_BUSY setting in esdhc_xfertyp().
Because usually the xfertyp should be XFERTYP_RSPTYP_48_BUSY for cmd with busy response.

Although we don't need to set XFERTYP_RSPTYP_48_BUSY in the future, I'd like to keep an explain comment here if possible :)

> 
> Best Regards,
> Jaehoon Chung
> 
> >  	else if (cmd->resp_type & MMC_RSP_PRESENT)
> >  		xfertyp |= XFERTYP_RSPTYP_48;
> >
> >

[U-Boot] [v2, 2/5] mmc: send STOP command when the READ/WRITE commands fail

[Y.B. Lu]

> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung at samsung.com]
> Sent: Monday, September 19, 2016 8:09 AM
> To: Y.B. Lu; u-boot at lists.denx.de
> Cc: york sun
> Subject: Re: [v2, 2/5] mmc: send STOP command when the READ/WRITE
> commands fail
> 
> Hi Yangbo,
> 
> On 08/02/2016 06:20 PM, Yangbo Lu wrote:
> > The STOP command should be sent to stop data transfer when the
> > READ/WRITE commands fail. Otherwise, any subsequent command will fail
> > to be sent.
> >
> > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > ---
> > Changes for v2:
> > 	- None
> > ---
> >  drivers/mmc/mmc.c         | 28 +++++++++++++++++++---------
> >  drivers/mmc/mmc_private.h |  1 +
> >  drivers/mmc/mmc_write.c   |  8 ++------
> >  3 files changed, 22 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
> > f8e5f7a..85d1e18 100644
> > --- a/drivers/mmc/mmc.c
> > +++ b/drivers/mmc/mmc.c
> > @@ -188,6 +188,21 @@ int mmc_set_blocklen(struct mmc *mmc, int len)
> >  	return mmc_send_cmd(mmc, &cmd, NULL);  }
> >
> > +int mmc_send_stop(struct mmc *mmc)
> > +{
> > +	struct mmc_cmd cmd;
> > +	int err;
> > +
> > +	cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
> > +	cmd.cmdarg = 0;
> > +	cmd.resp_type = MMC_RSP_R1b;
> > +
> > +	err = mmc_send_cmd(mmc, &cmd, NULL);
> > +	if (err)
> > +		printf("mmc fail to send stop cmd\n");
> > +	return err;
> > +}
> > +
> >  static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
> >  			   lbaint_t blkcnt)
> >  {
> > @@ -211,19 +226,14 @@ static int mmc_read_blocks(struct mmc *mmc, void
> *dst, lbaint_t start,
> >  	data.blocksize = mmc->read_bl_len;
> >  	data.flags = MMC_DATA_READ;
> >
> > -	if (mmc_send_cmd(mmc, &cmd, &data))
> > +	if (mmc_send_cmd(mmc, &cmd, &data)) {
> > +		mmc_send_stop(mmc);
> 
> If mmc_send_stop() is also failed, is it doesn't need to return error
> number?
> 

[Lu Yangbo-B47093] You're right. I need to check the return. Thanks :)

> >  		return 0;
> > +	}
> >
> >  	if (blkcnt > 1) {
> > -		cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
> > -		cmd.cmdarg = 0;
> > -		cmd.resp_type = MMC_RSP_R1b;
> > -		if (mmc_send_cmd(mmc, &cmd, NULL)) {
> > -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> > -			printf("mmc fail to send stop cmd\n");
> > -#endif
> > +		if (mmc_send_stop(mmc))
> >  			return 0;
> > -		}
> >  	}
> >
> >  	return blkcnt;
> > diff --git a/drivers/mmc/mmc_private.h b/drivers/mmc/mmc_private.h
> > index 49ec022..2791125 100644
> > --- a/drivers/mmc/mmc_private.h
> > +++ b/drivers/mmc/mmc_private.h
> > @@ -16,6 +16,7 @@ extern int mmc_send_cmd(struct mmc *mmc, struct
> mmc_cmd *cmd,
> >  			struct mmc_data *data);
> >  extern int mmc_send_status(struct mmc *mmc, int timeout);  extern int
> > mmc_set_blocklen(struct mmc *mmc, int len);
> > +int mmc_send_stop(struct mmc *mmc);
> >  #ifdef CONFIG_FSL_ESDHC_ADAPTER_IDENT  void
> > mmc_adapter_card_type_ident(void);
> >  #endif
> > diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c index
> > 0f8b5c7..fb8488c 100644
> > --- a/drivers/mmc/mmc_write.c
> > +++ b/drivers/mmc/mmc_write.c
> > @@ -150,6 +150,7 @@ static ulong mmc_write_blocks(struct mmc *mmc,
> > lbaint_t start,
> >
> >  	if (mmc_send_cmd(mmc, &cmd, &data)) {
> >  		printf("mmc write failed\n");
> > +		mmc_send_stop(mmc);
> 
> ditto.

[Lu Yangbo-B47093] Ok, I will check the return. Thanks :)
> 
> Best Regards,
> Jaehoon Chung
> 
> >  		return 0;
> >  	}
> >
> > @@ -157,13 +158,8 @@ static ulong mmc_write_blocks(struct mmc *mmc,
> lbaint_t start,
> >  	 * token, not a STOP_TRANSMISSION request.
> >  	 */
> >  	if (!mmc_host_is_spi(mmc) && blkcnt > 1) {
> > -		cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
> > -		cmd.cmdarg = 0;
> > -		cmd.resp_type = MMC_RSP_R1b;
> > -		if (mmc_send_cmd(mmc, &cmd, NULL)) {
> > -			printf("mmc fail to send stop cmd\n");
> > +		if (mmc_send_stop(mmc))
> >  			return 0;
> > -		}
> >  	}
> >
> >  	/* Waiting for the ready status */
> >

[U-Boot] [v2, 4/5] mmc: add workaround for eSDHC erratum A009620

[Y.B. Lu]

> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung at samsung.com]
> Sent: Monday, September 19, 2016 8:16 AM
> To: Y.B. Lu; u-boot at lists.denx.de
> Cc: york sun
> Subject: Re: [v2, 4/5] mmc: add workaround for eSDHC erratum A009620
> 
> On 08/02/2016 06:20 PM, Yangbo Lu wrote:
> > Erratum Title:
> > Data timeout error not getting set in case of command with busy
> > response (R1b) as well as for busy period after last write block
> > transfer.
> >
> > Description:
> > In the event that a busy timeout occurs for a command with a busy
> > response (e.g. R1b response) as well as busy period after the last
> > write block, the eSDHC does not set the IRQSTAT[DTOE] bit or the
> > IRQSTAT[TC]. Therefore, the current command transfer is never completed.
> >
> > Workaround:
> > Workaround for CMD with busy:
> > Don't set the XFRTYP[RSP]=2'b11 for commands with busy response and
> > poll the busy status of the card from the PRSSTAT[DLSL]
> >
> > Workaround for busy period after last write block:
> > 1. After the command completion interrupt (IRQSTAT[CC]), wait for
> >    de-assertion of PRSTAT[WTA].
> > 2. Once PRSTAT[WTA] is de-asserted, start the software timer and poll
> >    the busy signal (DAT0) using PRSTAT[DLSL[0]].
> > 3. Wait for DAT0 signal to go high (which indicate transfer complete)
> >    or software timer expiry (which indicate data timeout error).
> > 4. Issue soft reset for data (SYSCTL[RSTD]).
> > 5. In case of data timeout error (detected in step 3) perform the
> >    error recovery.
> >
> > The workaround for CMD with busy has already been applied in eSDHC
> > driver. This patch is to add workaround for the 2nd issue.
> 
> This workaround is used the CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620?..
> Then if this config is disabled..how is it handling?
> 

[Lu Yangbo-B47093] Yes, it uses CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620.
The erratum is hard to be reproduced normally because it occurs only when data has timeout.
If data timeout occurs, there will be not any data related interrupt. I think driver will hang at below while().

                do {
                        irqstat = esdhc_read32(&regs->irqstat);

                        if (irqstat & IRQSTAT_DTOE) {
                                err = -ETIMEDOUT;
                                goto out;
                        }

                        if (irqstat & DATA_ERR) {
                                err = -ECOMM;
                                goto out;
                        }
                } while ((irqstat & DATA_COMPLETE) != DATA_COMPLETE);

> >
> > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > ---
> > Changes for v2:
> > 	- Split original patch into config part and mmc part
> > ---
> >  drivers/mmc/fsl_esdhc.c | 26 ++++++++++++++++++++++++++
> >  include/fsl_esdhc.h     |  1 +
> >  2 files changed, 27 insertions(+)
> >
> > diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index
> > 80bc177..99cadae 100644
> > --- a/drivers/mmc/fsl_esdhc.c
> > +++ b/drivers/mmc/fsl_esdhc.c
> > @@ -482,6 +482,32 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd
> > *cmd, struct mmc_data *data)  #ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
> >  		esdhc_pio_read_write(mmc, data);
> >  #else
> > +#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
> > +		int timeout = 5000;
> > +		if (data->flags & MMC_DATA_WRITE) {
> > +			while (esdhc_read32(&regs->prsstat) & PRSSTAT_WTA)
> > +				;
> 
> I don't want to have the potential infinite loop.
> 

[Lu Yangbo-B47093] Sometimes we have to wait for some register status bit,
while the silicon RM believes it couldn't be unexpectable and it will be completed.

I'm not sure how to make it not infinite loop properly. 
Add timeout checking? What's the timeout value should we use ?

> Best Regards,
> Jaehoon Chung
> 
> > +
> > +			/* Poll on DATA0 line for 500 ms */
> > +			while (!(esdhc_read32(&regs->prsstat) & PRSSTAT_DAT0))
> {
> > +				udelay(100);
> > +				timeout--;
> > +				if (timeout <= 0) {
> > +					err = TIMEOUT;
> > +					break;
> > +				}
> > +			}
> > +			if (!err) {
> > +				esdhc_write32(&regs->sysctl,
> > +					      esdhc_read32(&regs->sysctl) |
> > +					      SYSCTL_RSTD);
> > +				while ((esdhc_read32(&regs->sysctl) &
> > +					SYSCTL_RSTD))
> > +					;
> > +			}
> > +			goto out;
> > +		}
> > +#endif
> >  		do {
> >  			irqstat = esdhc_read32(&regs->irqstat);
> >
> > diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc.h index
> > c6f4666..3f146f7 100644
> > --- a/include/fsl_esdhc.h
> > +++ b/include/fsl_esdhc.h
> > @@ -97,6 +97,7 @@
> >  #define PRSSTAT_CINS		(0x00010000)
> >  #define PRSSTAT_BREN		(0x00000800)
> >  #define PRSSTAT_BWEN		(0x00000400)
> > +#define PRSSTAT_WTA		(0x00000100)
> >  #define PRSSTAT_SDSTB		(0X00000008)
> >  #define PRSSTAT_DLA		(0x00000004)
> >  #define PRSSTAT_CICHB		(0x00000002)
> >

[U-Boot] [v2, 1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY for CMD with busy response

[Y.B. Lu]

> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung at samsung.com]
> Sent: Monday, September 19, 2016 8:17 AM
> To: Y.B. Lu; york sun; u-boot at lists.denx.de; Pantelis Antoniou
> Subject: Re: [v2, 1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY
> for CMD with busy response
> 
> On 09/13/2016 04:30 PM, Y.B. Lu wrote:
> > Hi York,
> >
> > I found the MMC maintainer had been changed to Jeahoon.
> > It seems Panto haven't commented on upstream for a very long time.
> >
> > MMC
> > M:      Jaehoon Chung <jh80.chung@samsung.com>
> > S:      Maintained
> > T:      git git://git.denx.de/u-boot-mmc.git
> > F:      drivers/mmc/
> >
> > Thanks.
> 
> Thanks for reminding. I missed these patchset.
> Sorry.
> 

[Lu Yangbo-B47093] That's ok. Thank you for your comments :)

> Best Regards,
> Jaehoon Chung
> 
> >
> >
> > Best regards,
> > Yangbo Lu
> >
> >
> >> -----Original Message-----
> >> From: york sun
> >> Sent: Tuesday, September 13, 2016 3:24 AM
> >> To: Y.B. Lu; u-boot at lists.denx.de; Pantelis Antoniou
> >> Cc: Jaehoon Chung
> >> Subject: Re: [v2, 1/5] mmc: fsl_esdhc: don't set
> >> XFERTYP_RSPTYP_48_BUSY for CMD with busy response
> >>
> >> Panto,
> >>
> >> On 08/02/2016 02:32 AM, Yangbo Lu wrote:
> >>> For CMD with busy response, the eSDHC driver would poll DAT0 until
> >>> CMD completion rather than polling IRQSTAT. So, don't set
> >>> XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC) in IRQSTAT.
> >>>
> >>> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> >>> ---
> >>> Changes for v2:
> >>> 	- None
> >>> ---
> >>>  drivers/mmc/fsl_esdhc.c | 12 ++++++++++--
> >>>  1 file changed, 10 insertions(+), 2 deletions(-)
> >>>
> >>
> >> I noticed this set wasn't CC'ing you. Can you check and comment/ack
> >> this set?
> >>
> >> York
> >
> >
> >
> >

[U-Boot] [v2, 5/5] arch/arm, arch/powerpc: enable workaround for eSDHC erratum A009620

[Y.B. Lu]

Hi York,

I have a question about CONFIG option. Are we going to completely using Kconfig in the future like kernel?
I just found below compiling error.

=============Error log===================
./scripts/check-config.sh u-boot.cfg \
		./scripts/config_whitelist.txt . 1>&2
Error: You must add new CONFIG options using Kconfig
The following new ad-hoc CONFIG options were detected:
CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620

Please add these via Kconfig instead. Find a suitable Kconfig
file and add a 'config' or 'menuconfig' option.
make: *** [all] Error 1
===========================================

As you know, all CONFIG options of the QorIQ eSDHC driver were defined in <board>.h.
Should we convert to use Kconfig for eSDHC driver?

Thanks.


Best regards,
Yangbo Lu

> -----Original Message-----
> From: Yangbo Lu [mailto:yangbo.lu at nxp.com]
> Sent: Tuesday, August 02, 2016 5:21 PM
> To: u-boot at lists.denx.de
> Cc: york sun; Jaehoon Chung; Yangbo Lu
> Subject: [v2, 5/5] arch/arm, arch/powerpc: enable workaround for eSDHC
> erratum A009620
> 
> This patch is to enable workaround for eSDHC erratum A009620. All the
> affected platforms include PowerPC(P1010/P2020/P5020/P5040/T1024/T1040/
> T2080/T4240) and ARM(LS1021A/LS1043A/LS2080A).
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---
> Changes for v2:
> 	- Added this patch
> 	- Moved definition out of board files
> ---
>  arch/arm/include/asm/arch-fsl-layerscape/config.h | 2 ++
>  arch/arm/include/asm/arch-ls102xa/config.h        | 1 +
>  arch/powerpc/include/asm/config_mpc85xx.h         | 8 ++++++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/config.h
> b/arch/arm/include/asm/arch-fsl-layerscape/config.h
> index b0ad4b4..7f31fcd 100644
> --- a/arch/arm/include/asm/arch-fsl-layerscape/config.h
> +++ b/arch/arm/include/asm/arch-fsl-layerscape/config.h
> @@ -32,6 +32,7 @@
>  #define CONFIG_NUM_DDR_CONTROLLERS		3
>  #define CONFIG_SYS_FSL_HAS_DP_DDR		/* Runtime check to confirm */
>  #define CONFIG_SYS_FSL_CLUSTER_CLOCKS		{ 1, 1, 4, 4 }
> +#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
>  #define	SRDS_MAX_LANES	8
>  #define CONFIG_SYS_FSL_SRDS_1
>  #define CONFIG_SYS_FSL_SRDS_2
> @@ -175,6 +176,7 @@
>  #define CONFIG_SYS_NUM_FM1_DTSEC		7
>  #define CONFIG_SYS_NUM_FM1_10GEC		1
>  #define CONFIG_SYS_FSL_IFC_BANK_COUNT		4
> +#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
>  #define CONFIG_SYS_FSL_DDR_BE
>  #define CONFIG_SYS_DDR_BLOCK1_SIZE		((phys_size_t)2 << 30)
>  #define CONFIG_MAX_MEM_MAPPED
> 	CONFIG_SYS_DDR_BLOCK1_SIZE
> diff --git a/arch/arm/include/asm/arch-ls102xa/config.h
> b/arch/arm/include/asm/arch-ls102xa/config.h
> index d408fe4..054f05d 100644
> --- a/arch/arm/include/asm/arch-ls102xa/config.h
> +++ b/arch/arm/include/asm/arch-ls102xa/config.h
> @@ -131,6 +131,7 @@
>  #define CONFIG_USB_MAX_CONTROLLER_COUNT		1
>  #define CONFIG_SYS_FSL_ERRATUM_A008378
>  #define CONFIG_SYS_FSL_ERRATUM_A009663
> +#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
>  #define CONFIG_SYS_FSL_MAX_NUM_OF_SEC		1
>  #else
>  #error SoC not defined
> diff --git a/arch/powerpc/include/asm/config_mpc85xx.h
> b/arch/powerpc/include/asm/config_mpc85xx.h
> index 505d355..b3d8fe8 100644
> --- a/arch/powerpc/include/asm/config_mpc85xx.h
> +++ b/arch/powerpc/include/asm/config_mpc85xx.h
> @@ -148,6 +148,7 @@
>  #define CONFIG_TSECV2
>  #define CONFIG_SYS_FSL_SEC_COMPAT	4
>  #define CONFIG_SYS_FSL_ERRATUM_ESDHC111
> +#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
>  #define CONFIG_NUM_DDR_CONTROLLERS	1
>  #define CONFIG_USB_MAX_CONTROLLER_COUNT	1
>  #define CONFIG_SYS_FSL_IFC_BANK_COUNT	4
> @@ -369,6 +370,7 @@
>  #define CONFIG_SYS_CCSRBAR_DEFAULT	0xff700000
>  #define CONFIG_SYS_FSL_ERRATUM_ESDHC111  #define
> CONFIG_SYS_FSL_ERRATUM_ESDHC_A001
> +#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
>  #define CONFIG_SYS_FSL_SRIO_MAX_PORTS	2
>  #define CONFIG_SYS_FSL_SRIO_OB_WIN_NUM	9
>  #define CONFIG_SYS_FSL_SRIO_IB_WIN_NUM	5
> @@ -530,6 +532,7 @@
>  #define CONFIG_SYS_FSL_USB2_PHY_ENABLE
>  #define CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY
>  #define CONFIG_SYS_FSL_ERRATUM_ESDHC111
> +#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
>  #define CONFIG_SYS_FSL_ERRATUM_USB14
>  #define CONFIG_SYS_FSL_ERRATUM_DDR_A003  #define
> CONFIG_SYS_FSL_ERRATUM_DDR_A003474
> @@ -568,6 +571,7 @@
>  #define CONFIG_SYS_FSL_USB2_PHY_ENABLE
>  #define CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY
>  #define CONFIG_SYS_FSL_ERRATUM_ESDHC111
> +#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
>  #define CONFIG_SYS_FSL_ERRATUM_USB14
>  #define CONFIG_SYS_FSL_ERRATUM_DDR_A003  #define
> CONFIG_SYS_FSL_ERRATUM_DDR_A003474
> @@ -686,6 +690,7 @@
>  #define CONFIG_SYS_FSL_ERRATUM_A007186
>  #define CONFIG_SYS_FSL_ERRATUM_A006593
>  #define CONFIG_SYS_FSL_ERRATUM_A007798
> +#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
>  #define CONFIG_SYS_CCSRBAR_DEFAULT	0xfe000000
>  #define CONFIG_SYS_FSL_SFP_VER_3_0
>  #define CONFIG_SYS_FSL_PCI_VER_3_X
> @@ -802,6 +807,7 @@ defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022)
> #define CONFIG_SYS_FSL_ERRATUM_A006261
>  #define CONFIG_SYS_CCSRBAR_DEFAULT	0xfe000000
>  #define CONFIG_SYS_FSL_ERRATUM_ESDHC111
> +#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
>  #define ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE
>  #define QE_MURAM_SIZE			0x6000UL
>  #define MAX_QE_RISC			1
> @@ -823,6 +829,7 @@ defined(CONFIG_PPC_T1014) || defined(CONFIG_PPC_T1013)
> #endif  #if defined(CONFIG_PPC_T1024) || defined(CONFIG_PPC_T1023)
>  #define CONFIG_MAX_CPUS			2
> +#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
>  #elif defined(CONFIG_PPC_T1014) || defined(CONFIG_PPC_T1013)
>  #define CONFIG_MAX_CPUS			1
>  #endif
> @@ -882,6 +889,7 @@ defined(CONFIG_PPC_T1014) || defined(CONFIG_PPC_T1013)
>  #define CONFIG_SYS_FSL_SRIO_MAX_PORTS	2
>  #define CONFIG_SYS_FSL_SRIO_OB_WIN_NUM	9
>  #define CONFIG_SYS_FSL_SRIO_IB_WIN_NUM	5
> +#define CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
>  #elif defined(CONFIG_PPC_T2081)
>  #define CONFIG_SYS_NUM_FM1_DTSEC	6
>  #define CONFIG_SYS_NUM_FM1_10GEC	2
> --
> 2.1.0.27.g96db324

[U-Boot] [v2, 5/5] arch/arm, arch/powerpc: enable workaround for eSDHC erratum A009620

[york sun]

On 10/18/2016 02:39 AM, Y.B. Lu wrote:
> Hi York,
>
> I have a question about CONFIG option. Are we going to completely using Kconfig in the future like kernel?
> I just found below compiling error.
>
> =============Error log===================
> ./scripts/check-config.sh u-boot.cfg \
> 		./scripts/config_whitelist.txt . 1>&2
> Error: You must add new CONFIG options using Kconfig
> The following new ad-hoc CONFIG options were detected:
> CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620
>
> Please add these via Kconfig instead. Find a suitable Kconfig
> file and add a 'config' or 'menuconfig' option.
> make: *** [all] Error 1
> ===========================================
>
> As you know, all CONFIG options of the QorIQ eSDHC driver were defined in <board>.h.
> Should we convert to use Kconfig for eSDHC driver?
>


That is the direction U-Boot is taking. Please check my recent commits 
on upstream U-Boot for examples to move config macros to Kconfig. I 
converted some DDR options, and some errata macros.

York

[U-Boot] [v2, 2/5] mmc: send STOP command when the READ/WRITE commands fail

[york sun]

On 09/23/2016 12:38 AM, Y.B. Lu wrote:
>> ditto.
>
> [Lu Yangbo-B47093] Ok, I will check the return. Thanks :)
>>

Yangbo,

Do you have an update?

York