All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] ftsdc010: improve performance and capability
@ 2011-11-03  9:35 Macpaul Lin
  2011-11-11  7:50 ` Macpaul Lin
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Macpaul Lin @ 2011-11-03  9:35 UTC (permalink / raw)
  To: u-boot

This patch improve the performance by spliting flag examination code
in ftsdc010_send_cmd() into 3 functions.
This patch also reordered the function which made better capability to
some high performance cards against to the next version of ftsdc010
hardware.

Signed-off-by: Macpaul Lin <macpaul@andestech.com>
---
 drivers/mmc/ftsdc010_esdhc.c |  172 +++++++++++++++++++++++-------------------
 1 files changed, 93 insertions(+), 79 deletions(-)

diff --git a/drivers/mmc/ftsdc010_esdhc.c b/drivers/mmc/ftsdc010_esdhc.c
index e38dd87..538800b 100644
--- a/drivers/mmc/ftsdc010_esdhc.c
+++ b/drivers/mmc/ftsdc010_esdhc.c
@@ -90,8 +90,13 @@ static void ftsdc010_pio_read(struct mmc_host *host, char *buf, unsigned int siz
 
 	while (size) {
 		status = readl(&host->reg->status);
+		debug("%s: size: %08x\n", __func__, size);
 
 		if (status & FTSDC010_STATUS_FIFO_ORUN) {
+
+			debug("%s: FIFO OVERRUN: sta: %08x\n",
+					__func__, status);
+
 			fifo = host->fifo_len > size ?
 				size : host->fifo_len;
 
@@ -146,7 +151,7 @@ static void ftsdc010_pio_write(struct mmc_host *host, const char *buf,
 	while (size) {
 		status = readl(&host->reg->status);
 
-		if (status & FTSDC010_STATUS_FIFO_ORUN) {
+		if (status & FTSDC010_STATUS_FIFO_URUN) {
 			fifo = host->fifo_len > size ?
 				size : host->fifo_len;
 
@@ -158,7 +163,6 @@ static void ftsdc010_pio_write(struct mmc_host *host, const char *buf,
 				writel(*ptr, &host->reg->dwr);
 				ptr++;
 			}
-
 		} else {
 			udelay(1);
 			if (++retry >= FTSDC010_PIO_RETRY) {
@@ -169,56 +173,19 @@ static void ftsdc010_pio_write(struct mmc_host *host, const char *buf,
 	}
 }
 
-static int ftsdc010_pio_check_status(struct mmc *mmc, struct mmc_cmd *cmd,
+static int ftsdc010_check_rsp(struct mmc *mmc, struct mmc_cmd *cmd,
 			struct mmc_data *data)
 {
 	struct mmc_host *host = mmc->priv;
-
 	unsigned int sta, clear;
-	unsigned int i;
-
-	/* check response and hardware status */
-	clear = 0;
-
-	/* chech CMD_SEND */
-	for (i = 0; i < FTSDC010_CMD_RETRY; i++) {
-		sta = readl(&host->reg->status);
-		/* Command Complete */
-		if (sta & FTSDC010_STATUS_CMD_SEND) {
-			if (!data)
-				clear |= FTSDC010_CLR_CMD_SEND;
-			break;
-		}
-	}
-
-	if (i > FTSDC010_CMD_RETRY) {
-		printf("%s: send command timeout\n", __func__);
-		return TIMEOUT;
-	}
 
-	/* debug: print status register and command index*/
-	debug("sta: %08x cmd %d\n", sta, cmd->cmdidx);
-
-	/* handle data FIFO */
-	if ((sta & FTSDC010_STATUS_FIFO_ORUN) ||
-		(sta & FTSDC010_STATUS_FIFO_URUN)) {
-
-		/* Wrong DATA FIFO Flag */
-		if (data == NULL)
-			printf("%s, data fifo wrong: sta: %08x cmd %d\n",
-				__func__, sta, cmd->cmdidx);
-
-		if (sta & FTSDC010_STATUS_FIFO_ORUN)
-			clear |= FTSDC010_STATUS_FIFO_ORUN;
-		if (sta & FTSDC010_STATUS_FIFO_URUN)
-			clear |= FTSDC010_STATUS_FIFO_URUN;
-	}
+	sta = readl(&host->reg->status);
+	debug("%s: sta: %08x cmd %d\n", __func__, sta, cmd->cmdidx);
 
 	/* check RSP TIMEOUT or FAIL */
 	if (sta & FTSDC010_STATUS_RSP_TIMEOUT) {
 		/* RSP TIMEOUT */
-		debug("%s: RSP timeout: sta: %08x cmd %d\n",
-				__func__, sta, cmd->cmdidx);
+		debug("%s: RSP timeout: sta: %08x\n", __func__, sta);
 
 		clear |= FTSDC010_CLR_RSP_TIMEOUT;
 		writel(clear, &host->reg->clr);
@@ -226,47 +193,62 @@ static int ftsdc010_pio_check_status(struct mmc *mmc, struct mmc_cmd *cmd,
 		return TIMEOUT;
 	} else if (sta & FTSDC010_STATUS_RSP_CRC_FAIL) {
 		/* clear response fail bit */
-		debug("%s: RSP CRC FAIL: sta: %08x cmd %d\n",
-				__func__, sta, cmd->cmdidx);
+		debug("%s: RSP CRC FAIL: sta: %08x\n", __func__, sta);
 
 		clear |= FTSDC010_CLR_RSP_CRC_FAIL;
 		writel(clear, &host->reg->clr);
 
-		return 0;
+		return COMM_ERR;
 	} else if (sta & FTSDC010_STATUS_RSP_CRC_OK) {
 
 		/* clear response CRC OK bit */
 		clear |= FTSDC010_CLR_RSP_CRC_OK;
 	}
 
+	writel(clear, &host->reg->clr);
+	return 0;
+}
+
+static int ftsdc010_check_data(struct mmc *mmc, struct mmc_cmd *cmd,
+			struct mmc_data *data)
+{
+	struct mmc_host *host = mmc->priv;
+	unsigned int sta, clear;
+
+	sta = readl(&host->reg->status);
+	debug("%s: sta: %08x cmd %d\n", __func__, sta, cmd->cmdidx);
+
 	/* check DATA TIMEOUT or FAIL */
 	if (data) {
+		if (sta & FTSDC010_STATUS_DATA_END) {
+			/* Transfer Complete */
+			clear |= FTSDC010_STATUS_DATA_END;
+		}
+
+		if (sta & FTSDC010_STATUS_DATA_CRC_OK) {
+			/* Data CRC_OK */
+			clear |= FTSDC010_STATUS_DATA_CRC_OK;
+		}
+
 		if (sta & FTSDC010_STATUS_DATA_TIMEOUT) {
 			/* DATA TIMEOUT */
-			debug("%s: DATA TIMEOUT: sta: %08x\n",
-					__func__, sta);
+			debug("%s: DATA TIMEOUT: sta: %08x\n", __func__, sta);
 
 			clear |= FTSDC010_STATUS_DATA_TIMEOUT;
 			writel(sta, &host->reg->clr);
+
 			return TIMEOUT;
 		} else if (sta & FTSDC010_STATUS_DATA_CRC_FAIL) {
 			/* Error Interrupt */
-			debug("%s: DATA CRC FAIL: sta: %08x\n",
-					__func__, sta);
+			debug("%s: DATA CRC FAIL: sta: %08x\n", __func__, sta);
 
 			clear |= FTSDC010_STATUS_DATA_CRC_FAIL;
 			writel(clear, &host->reg->clr);
 
-			return 0;
-		} else if (sta & FTSDC010_STATUS_DATA_END) {
-			/* Transfer Complete */
-			clear |= FTSDC010_STATUS_DATA_END;
+			return COMM_ERR;
 		}
+		writel(clear, &host->reg->clr);
 	}
-
-	/* transaction is success and clear status register */
-	writel(clear, &host->reg->clr);
-
 	return 0;
 }
 
@@ -281,6 +263,9 @@ static int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 	unsigned int ccon;
 	unsigned int mask, tmpmask;
 	unsigned int ret;
+	unsigned int sta, clear, i;
+
+	ret = 0;
 
 	if (data)
 		mask = FTSDC010_INT_MASK_RSP_TIMEOUT;
@@ -290,13 +275,8 @@ static int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 		mask = FTSDC010_INT_MASK_CMD_SEND;
 
 	/* write argu reg */
-	debug("%s: cmd->arg: %08x\n", __func__, cmd->cmdarg);
 	writel(cmd->cmdarg, &host->reg->argu);
 
-	/* setup cmd reg */
-	debug("cmd: %d\n", cmd->cmdidx);
-	debug("resp: %08x\n", cmd->resp_type);
-
 	/* setup commnad */
 	ccon = FTSDC010_CMD_IDX(cmd->cmdidx);
 
@@ -342,6 +322,45 @@ static int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 	writel(ccon, &host->reg->cmd);
 	udelay(4*FTSDC010_DELAY_UNIT);
 
+	/* check CMD_SEND */
+	/*
+	 * Note:
+	 *	Do not clear FTSDC010_CLR_CMD_SEND flag here,
+	 *	(write FTSDC010_CLR_CMD_SEND bit to clear register)
+	 *	it will made the driver becomes very slow.
+	 *	If the operation hasn't been finished, hardware will
+	 *	clear this bit automatically.
+	 */
+	for (i = 0; i < FTSDC010_CMD_RETRY; i++) {
+		sta = readl(&host->reg->status);
+		/* Command Complete */
+		if (sta & FTSDC010_STATUS_CMD_SEND) {
+			if (!data)
+				clear |= FTSDC010_CLR_CMD_SEND;
+			break;
+		}
+	}
+
+	if (i > FTSDC010_CMD_RETRY) {
+		printf("%s: send command timeout\n", __func__);
+		return TIMEOUT;
+	}
+
+	/* check rsp status */
+	ret = ftsdc010_check_rsp(mmc, cmd, data);
+	if (ret)
+		return ret;
+
+	/* read response if we have RSP_OK */
+	if (ccon & FTSDC010_CMD_LONG_RSP) {
+		cmd->response[0] = readl(&host->reg->rsp3);
+		cmd->response[1] = readl(&host->reg->rsp2);
+		cmd->response[2] = readl(&host->reg->rsp1);
+		cmd->response[3] = readl(&host->reg->rsp0);
+	} else {
+		cmd->response[0] = readl(&host->reg->rsp0);
+	}
+
 	/* read/write data */
 	if (data && (data->flags & MMC_DATA_READ)) {
 		ftsdc010_pio_read(host, data->dest,
@@ -351,19 +370,11 @@ static int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 				data->blocksize * data->blocks);
 	}
 
-	/* pio check response status */
-	ret = ftsdc010_pio_check_status(mmc, cmd, data);
-	if (!ret) {
-		/* if it is long response */
-		if (ccon & FTSDC010_CMD_LONG_RSP) {
-			cmd->response[0] = readl(&host->reg->rsp3);
-			cmd->response[1] = readl(&host->reg->rsp2);
-			cmd->response[2] = readl(&host->reg->rsp1);
-			cmd->response[3] = readl(&host->reg->rsp0);
-
-		} else {
-			cmd->response[0] = readl(&host->reg->rsp0);
-		}
+	/* check data status */
+	if (data) {
+		ret = ftsdc010_check_data(mmc, cmd, data);
+		if (ret)
+			return ret;
 	}
 
 	udelay(FTSDC010_DELAY_UNIT);
@@ -431,8 +442,6 @@ static int ftsdc010_setup_data(struct mmc *mmc, struct mmc_data *data)
 	/* always reset fifo since last transfer may fail */
 	dcon |= FTSDC010_DCR_FIFO_RST;
 
-	/* handle sdio */
-	dcon = data->blocksize | data->blocks << 15;
 	if (data->blocks > 1)
 		dcon |= FTSDC010_SDIO_CTRL1_SDIO_BLK_MODE;
 #endif
@@ -518,7 +527,7 @@ static void ftsdc010_set_clk(struct mmc *mmc)
 				break;
 		}
 
-		debug("%s: computed real_rete: %x, clk_div: %x\n",
+		debug("%s: computed real_rate: %x, clk_div: %x\n",
 			 __func__, real_rate, clk_div);
 
 		if (clk_div > 127)
@@ -579,6 +588,7 @@ static void ftsdc010_set_ios(struct mmc *mmc)
 static void ftsdc010_reset(struct mmc_host *host)
 {
 	unsigned int timeout;
+	unsigned int sta;
 
 	/* Do SDC_RST: Software reset for all register */
 	writel(FTSDC010_CMD_SDC_RST, &host->reg->cmd);
@@ -598,6 +608,10 @@ static void ftsdc010_reset(struct mmc_host *host)
 		timeout--;
 		udelay(10*FTSDC010_DELAY_UNIT);
 	}
+
+	sta = readl(&host->reg->status);
+	if (sta & FTSDC010_STATUS_CARD_CHANGE)
+		writel(FTSDC010_CLR_CARD_CHANGE, &host->reg->clr);
 }
 
 static int ftsdc010_core_init(struct mmc *mmc)
-- 
1.7.3.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH] ftsdc010: improve performance and capability
  2011-11-03  9:35 [U-Boot] [PATCH] ftsdc010: improve performance and capability Macpaul Lin
@ 2011-11-11  7:50 ` Macpaul Lin
  2011-11-14 11:28 ` [U-Boot] [PATCH v2] " Macpaul Lin
  2011-11-17  9:34 ` [U-Boot] [PATCH v3] " Macpaul Lin
  2 siblings, 0 replies; 10+ messages in thread
From: Macpaul Lin @ 2011-11-11  7:50 UTC (permalink / raw)
  To: u-boot

Hi Andy,

2011/11/3 Macpaul Lin <macpaul@andestech.com>:
> This patch improve the performance by spliting flag examination code
> in ftsdc010_send_cmd() into 3 functions.
> This patch also reordered the function which made better capability to
> some high performance cards against to the next version of ftsdc010
> hardware.
>
> Signed-off-by: Macpaul Lin <macpaul@andestech.com>

If you're reviewing this patch, please stop.
I think I still need to check this drive if it will be affect by the
work I'm doing.
While I'm checking the difference of dealing with the high speed
capability between Linux and u-boot,
I'm not sure if there will affect the controller's driver in bottom.

Thanks.

If there is any modification, I will send a new patch.

-- 
Best regards,
Macpaul Lin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2] ftsdc010: improve performance and capability
  2011-11-03  9:35 [U-Boot] [PATCH] ftsdc010: improve performance and capability Macpaul Lin
  2011-11-11  7:50 ` Macpaul Lin
@ 2011-11-14 11:28 ` Macpaul Lin
  2011-11-17  9:34 ` [U-Boot] [PATCH v3] " Macpaul Lin
  2 siblings, 0 replies; 10+ messages in thread
From: Macpaul Lin @ 2011-11-14 11:28 UTC (permalink / raw)
  To: u-boot

This patch improve the performance by spliting flag examination code
in ftsdc010_send_cmd() into 3 functions.
This patch also reordered the function which made better capability to
some high performance cards against to the next version of ftsdc010
hardware.

Signed-off-by: Macpaul Lin <macpaul@andestech.com>
---
Changes for v2:
  - Fix the problem if we read status register too fast in FTSDC010_CMD_RETRY
    loop. If we read status register here too fast, the hardware will report
    RSP_TIMEOUT incorrectly.

 drivers/mmc/ftsdc010_esdhc.c |  181 +++++++++++++++++++++++-------------------
 1 files changed, 100 insertions(+), 81 deletions(-)

diff --git a/drivers/mmc/ftsdc010_esdhc.c b/drivers/mmc/ftsdc010_esdhc.c
index 0a27e3b..0ab1089 100644
--- a/drivers/mmc/ftsdc010_esdhc.c
+++ b/drivers/mmc/ftsdc010_esdhc.c
@@ -94,8 +94,13 @@ static void ftsdc010_pio_read(struct mmc_host *host, char *buf, unsigned int siz
 
 	while (size) {
 		status = readl(&host->reg->status);
+		debug("%s: size: %08x\n", __func__, size);
 
 		if (status & FTSDC010_STATUS_FIFO_ORUN) {
+
+			debug("%s: FIFO OVERRUN: sta: %08x\n",
+					__func__, status);
+
 			fifo = host->fifo_len > size ?
 				size : host->fifo_len;
 
@@ -150,7 +155,7 @@ static void ftsdc010_pio_write(struct mmc_host *host, const char *buf,
 	while (size) {
 		status = readl(&host->reg->status);
 
-		if (status & FTSDC010_STATUS_FIFO_ORUN) {
+		if (status & FTSDC010_STATUS_FIFO_URUN) {
 			fifo = host->fifo_len > size ?
 				size : host->fifo_len;
 
@@ -162,7 +167,6 @@ static void ftsdc010_pio_write(struct mmc_host *host, const char *buf,
 				writel(*ptr, &host->reg->dwr);
 				ptr++;
 			}
-
 		} else {
 			udelay(1);
 			if (++retry >= FTSDC010_PIO_RETRY) {
@@ -173,56 +177,19 @@ static void ftsdc010_pio_write(struct mmc_host *host, const char *buf,
 	}
 }
 
-static int ftsdc010_pio_check_status(struct mmc *mmc, struct mmc_cmd *cmd,
+static int ftsdc010_check_rsp(struct mmc *mmc, struct mmc_cmd *cmd,
 			struct mmc_data *data)
 {
 	struct mmc_host *host = mmc->priv;
-
 	unsigned int sta, clear;
-	unsigned int i;
-
-	/* check response and hardware status */
-	clear = 0;
-
-	/* chech CMD_SEND */
-	for (i = 0; i < FTSDC010_CMD_RETRY; i++) {
-		sta = readl(&host->reg->status);
-		/* Command Complete */
-		if (sta & FTSDC010_STATUS_CMD_SEND) {
-			if (!data)
-				clear |= FTSDC010_CLR_CMD_SEND;
-			break;
-		}
-	}
-
-	if (i > FTSDC010_CMD_RETRY) {
-		printf("%s: send command timeout\n", __func__);
-		return TIMEOUT;
-	}
-
-	/* debug: print status register and command index*/
-	debug("sta: %08x cmd %d\n", sta, cmd->cmdidx);
 
-	/* handle data FIFO */
-	if ((sta & FTSDC010_STATUS_FIFO_ORUN) ||
-		(sta & FTSDC010_STATUS_FIFO_URUN)) {
-
-		/* Wrong DATA FIFO Flag */
-		if (data == NULL)
-			printf("%s, data fifo wrong: sta: %08x cmd %d\n",
-				__func__, sta, cmd->cmdidx);
-
-		if (sta & FTSDC010_STATUS_FIFO_ORUN)
-			clear |= FTSDC010_STATUS_FIFO_ORUN;
-		if (sta & FTSDC010_STATUS_FIFO_URUN)
-			clear |= FTSDC010_STATUS_FIFO_URUN;
-	}
+	sta = readl(&host->reg->status);
+	debug("%s: sta: %08x cmd %d\n", __func__, sta, cmd->cmdidx);
 
 	/* check RSP TIMEOUT or FAIL */
 	if (sta & FTSDC010_STATUS_RSP_TIMEOUT) {
 		/* RSP TIMEOUT */
-		debug("%s: RSP timeout: sta: %08x cmd %d\n",
-				__func__, sta, cmd->cmdidx);
+		debug("%s: RSP timeout: sta: %08x\n", __func__, sta);
 
 		clear |= FTSDC010_CLR_RSP_TIMEOUT;
 		writel(clear, &host->reg->clr);
@@ -230,47 +197,62 @@ static int ftsdc010_pio_check_status(struct mmc *mmc, struct mmc_cmd *cmd,
 		return TIMEOUT;
 	} else if (sta & FTSDC010_STATUS_RSP_CRC_FAIL) {
 		/* clear response fail bit */
-		debug("%s: RSP CRC FAIL: sta: %08x cmd %d\n",
-				__func__, sta, cmd->cmdidx);
+		debug("%s: RSP CRC FAIL: sta: %08x\n", __func__, sta);
 
 		clear |= FTSDC010_CLR_RSP_CRC_FAIL;
 		writel(clear, &host->reg->clr);
 
-		return 0;
+		return COMM_ERR;
 	} else if (sta & FTSDC010_STATUS_RSP_CRC_OK) {
 
 		/* clear response CRC OK bit */
 		clear |= FTSDC010_CLR_RSP_CRC_OK;
 	}
 
+	writel(clear, &host->reg->clr);
+	return 0;
+}
+
+static int ftsdc010_check_data(struct mmc *mmc, struct mmc_cmd *cmd,
+			struct mmc_data *data)
+{
+	struct mmc_host *host = mmc->priv;
+	unsigned int sta, clear;
+
+	sta = readl(&host->reg->status);
+	debug("%s: sta: %08x cmd %d\n", __func__, sta, cmd->cmdidx);
+
 	/* check DATA TIMEOUT or FAIL */
 	if (data) {
+		if (sta & FTSDC010_STATUS_DATA_END) {
+			/* Transfer Complete */
+			clear |= FTSDC010_STATUS_DATA_END;
+		}
+
+		if (sta & FTSDC010_STATUS_DATA_CRC_OK) {
+			/* Data CRC_OK */
+			clear |= FTSDC010_STATUS_DATA_CRC_OK;
+		}
+
 		if (sta & FTSDC010_STATUS_DATA_TIMEOUT) {
 			/* DATA TIMEOUT */
-			debug("%s: DATA TIMEOUT: sta: %08x\n",
-					__func__, sta);
+			debug("%s: DATA TIMEOUT: sta: %08x\n", __func__, sta);
 
 			clear |= FTSDC010_STATUS_DATA_TIMEOUT;
 			writel(sta, &host->reg->clr);
+
 			return TIMEOUT;
 		} else if (sta & FTSDC010_STATUS_DATA_CRC_FAIL) {
 			/* Error Interrupt */
-			debug("%s: DATA CRC FAIL: sta: %08x\n",
-					__func__, sta);
+			debug("%s: DATA CRC FAIL: sta: %08x\n", __func__, sta);
 
 			clear |= FTSDC010_STATUS_DATA_CRC_FAIL;
 			writel(clear, &host->reg->clr);
 
-			return 0;
-		} else if (sta & FTSDC010_STATUS_DATA_END) {
-			/* Transfer Complete */
-			clear |= FTSDC010_STATUS_DATA_END;
+			return COMM_ERR;
 		}
+		writel(clear, &host->reg->clr);
 	}
-
-	/* transaction is success and clear status register */
-	writel(clear, &host->reg->clr);
-
 	return 0;
 }
 
@@ -285,6 +267,9 @@ static int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 	unsigned int ccon;
 	unsigned int mask, tmpmask;
 	unsigned int ret;
+	unsigned int sta, clear, i;
+
+	ret = 0;
 
 	if (data)
 		mask = FTSDC010_INT_MASK_RSP_TIMEOUT;
@@ -294,13 +279,8 @@ static int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 		mask = FTSDC010_INT_MASK_CMD_SEND;
 
 	/* write argu reg */
-	debug("%s: cmd->arg: %08x\n", __func__, cmd->cmdarg);
 	writel(cmd->cmdarg, &host->reg->argu);
 
-	/* setup cmd reg */
-	debug("cmd: %d\n", cmd->cmdidx);
-	debug("resp: %08x\n", cmd->resp_type);
-
 	/* setup commnad */
 	ccon = FTSDC010_CMD_IDX(cmd->cmdidx);
 
@@ -344,7 +324,51 @@ static int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 	/* write cmd reg */
 	debug("%s: ccon: %08x\n", __func__, ccon);
 	writel(ccon, &host->reg->cmd);
-	udelay(4*FTSDC010_DELAY_UNIT);
+
+	/* check CMD_SEND */
+	for (i = 0; i < FTSDC010_CMD_RETRY; i++) {
+		/*
+		 * If we read status register too fast
+		 * will lead hardware error and the RSP_TIMEOUT
+		 * flag will be raised incorrectly.
+		 */
+		udelay(16*FTSDC010_DELAY_UNIT);
+		sta = readl(&host->reg->status);
+
+		/* Command Complete */
+		/*
+		 * Note:
+		 *	Do not clear FTSDC010_CLR_CMD_SEND flag.
+		 *	(by writing FTSDC010_CLR_CMD_SEND bit to clear register)
+		 *	It will make the driver becomes very slow.
+		 *	If the operation hasn't been finished, hardware will
+		 *	clear this bit automatically.
+		 *	In origin, the driver will clear this flag if there is
+		 *	no data need to be read.
+		 */
+		if (sta & FTSDC010_STATUS_CMD_SEND)
+			break;
+	}
+
+	if (i > FTSDC010_CMD_RETRY) {
+		printf("%s: send command timeout\n", __func__);
+		return TIMEOUT;
+	}
+
+	/* check rsp status */
+	ret = ftsdc010_check_rsp(mmc, cmd, data);
+	if (ret)
+		return ret;
+
+	/* read response if we have RSP_OK */
+	if (ccon & FTSDC010_CMD_LONG_RSP) {
+		cmd->response[0] = readl(&host->reg->rsp3);
+		cmd->response[1] = readl(&host->reg->rsp2);
+		cmd->response[2] = readl(&host->reg->rsp1);
+		cmd->response[3] = readl(&host->reg->rsp0);
+	} else {
+		cmd->response[0] = readl(&host->reg->rsp0);
+	}
 
 	/* read/write data */
 	if (data && (data->flags & MMC_DATA_READ)) {
@@ -355,19 +379,11 @@ static int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 				data->blocksize * data->blocks);
 	}
 
-	/* pio check response status */
-	ret = ftsdc010_pio_check_status(mmc, cmd, data);
-	if (!ret) {
-		/* if it is long response */
-		if (ccon & FTSDC010_CMD_LONG_RSP) {
-			cmd->response[0] = readl(&host->reg->rsp3);
-			cmd->response[1] = readl(&host->reg->rsp2);
-			cmd->response[2] = readl(&host->reg->rsp1);
-			cmd->response[3] = readl(&host->reg->rsp0);
-
-		} else {
-			cmd->response[0] = readl(&host->reg->rsp0);
-		}
+	/* check data status */
+	if (data) {
+		ret = ftsdc010_check_data(mmc, cmd, data);
+		if (ret)
+			return ret;
 	}
 
 	udelay(FTSDC010_DELAY_UNIT);
@@ -435,8 +451,6 @@ static int ftsdc010_setup_data(struct mmc *mmc, struct mmc_data *data)
 	/* always reset fifo since last transfer may fail */
 	dcon |= FTSDC010_DCR_FIFO_RST;
 
-	/* handle sdio */
-	dcon = data->blocksize | data->blocks << 15;
 	if (data->blocks > 1)
 		dcon |= FTSDC010_SDIO_CTRL1_SDIO_BLK_MODE;
 #endif
@@ -501,7 +515,7 @@ static void ftsdc010_set_clk(struct mmc *mmc)
 {
 	struct mmc_host *host = mmc->priv;
 	unsigned char clk_div;
-	unsigned char real_rate;
+	unsigned int real_rate;
 	unsigned int clock;
 
 	debug("%s: mmc_set_clock: %x\n", __func__, mmc->clock);
@@ -522,7 +536,7 @@ static void ftsdc010_set_clk(struct mmc *mmc)
 				break;
 		}
 
-		debug("%s: computed real_rete: %x, clk_div: %x\n",
+		debug("%s: computed real_rate: %x, clk_div: %x\n",
 			 __func__, real_rate, clk_div);
 
 		if (clk_div > 127)
@@ -583,6 +597,7 @@ static void ftsdc010_set_ios(struct mmc *mmc)
 static void ftsdc010_reset(struct mmc_host *host)
 {
 	unsigned int timeout;
+	unsigned int sta;
 
 	/* Do SDC_RST: Software reset for all register */
 	writel(FTSDC010_CMD_SDC_RST, &host->reg->cmd);
@@ -602,6 +617,10 @@ static void ftsdc010_reset(struct mmc_host *host)
 		timeout--;
 		udelay(10*FTSDC010_DELAY_UNIT);
 	}
+
+	sta = readl(&host->reg->status);
+	if (sta & FTSDC010_STATUS_CARD_CHANGE)
+		writel(FTSDC010_CLR_CARD_CHANGE, &host->reg->clr);
 }
 
 static int ftsdc010_core_init(struct mmc *mmc)
-- 
1.7.3.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v3] ftsdc010: improve performance and capability
  2011-11-03  9:35 [U-Boot] [PATCH] ftsdc010: improve performance and capability Macpaul Lin
  2011-11-11  7:50 ` Macpaul Lin
  2011-11-14 11:28 ` [U-Boot] [PATCH v2] " Macpaul Lin
@ 2011-11-17  9:34 ` Macpaul Lin
  2011-11-26  0:07   ` Andy Fleming
  2 siblings, 1 reply; 10+ messages in thread
From: Macpaul Lin @ 2011-11-17  9:34 UTC (permalink / raw)
  To: u-boot

This patch improve the performance by spliting flag examination code
in ftsdc010_send_cmd() into 3 functions.
This patch also reordered the function which made better capability to
some high performance cards against to the next version of ftsdc010
hardware.

Signed-off-by: Macpaul Lin <macpaul@andestech.com>
---
Changes for v2:
  - Fix the problem if we read status register too fast in FTSDC010_CMD_RETRY
    loop. If we read status register here too fast, the hardware will report
    RSP_TIMEOUT incorrectly.
Changes for v3:
  - Remove host high speed capability due to hardware limitation.
  - Remove unused variables.

 drivers/mmc/ftsdc010_esdhc.c |  184 +++++++++++++++++++++++-------------------
 1 files changed, 101 insertions(+), 83 deletions(-)

diff --git a/drivers/mmc/ftsdc010_esdhc.c b/drivers/mmc/ftsdc010_esdhc.c
index e38dd87..23d21ad 100644
--- a/drivers/mmc/ftsdc010_esdhc.c
+++ b/drivers/mmc/ftsdc010_esdhc.c
@@ -90,8 +90,13 @@ static void ftsdc010_pio_read(struct mmc_host *host, char *buf, unsigned int siz
 
 	while (size) {
 		status = readl(&host->reg->status);
+		debug("%s: size: %08x\n", __func__, size);
 
 		if (status & FTSDC010_STATUS_FIFO_ORUN) {
+
+			debug("%s: FIFO OVERRUN: sta: %08x\n",
+					__func__, status);
+
 			fifo = host->fifo_len > size ?
 				size : host->fifo_len;
 
@@ -146,7 +151,7 @@ static void ftsdc010_pio_write(struct mmc_host *host, const char *buf,
 	while (size) {
 		status = readl(&host->reg->status);
 
-		if (status & FTSDC010_STATUS_FIFO_ORUN) {
+		if (status & FTSDC010_STATUS_FIFO_URUN) {
 			fifo = host->fifo_len > size ?
 				size : host->fifo_len;
 
@@ -158,7 +163,6 @@ static void ftsdc010_pio_write(struct mmc_host *host, const char *buf,
 				writel(*ptr, &host->reg->dwr);
 				ptr++;
 			}
-
 		} else {
 			udelay(1);
 			if (++retry >= FTSDC010_PIO_RETRY) {
@@ -169,56 +173,19 @@ static void ftsdc010_pio_write(struct mmc_host *host, const char *buf,
 	}
 }
 
-static int ftsdc010_pio_check_status(struct mmc *mmc, struct mmc_cmd *cmd,
+static int ftsdc010_check_rsp(struct mmc *mmc, struct mmc_cmd *cmd,
 			struct mmc_data *data)
 {
 	struct mmc_host *host = mmc->priv;
-
 	unsigned int sta, clear;
-	unsigned int i;
-
-	/* check response and hardware status */
-	clear = 0;
-
-	/* chech CMD_SEND */
-	for (i = 0; i < FTSDC010_CMD_RETRY; i++) {
-		sta = readl(&host->reg->status);
-		/* Command Complete */
-		if (sta & FTSDC010_STATUS_CMD_SEND) {
-			if (!data)
-				clear |= FTSDC010_CLR_CMD_SEND;
-			break;
-		}
-	}
-
-	if (i > FTSDC010_CMD_RETRY) {
-		printf("%s: send command timeout\n", __func__);
-		return TIMEOUT;
-	}
-
-	/* debug: print status register and command index*/
-	debug("sta: %08x cmd %d\n", sta, cmd->cmdidx);
 
-	/* handle data FIFO */
-	if ((sta & FTSDC010_STATUS_FIFO_ORUN) ||
-		(sta & FTSDC010_STATUS_FIFO_URUN)) {
-
-		/* Wrong DATA FIFO Flag */
-		if (data == NULL)
-			printf("%s, data fifo wrong: sta: %08x cmd %d\n",
-				__func__, sta, cmd->cmdidx);
-
-		if (sta & FTSDC010_STATUS_FIFO_ORUN)
-			clear |= FTSDC010_STATUS_FIFO_ORUN;
-		if (sta & FTSDC010_STATUS_FIFO_URUN)
-			clear |= FTSDC010_STATUS_FIFO_URUN;
-	}
+	sta = readl(&host->reg->status);
+	debug("%s: sta: %08x cmd %d\n", __func__, sta, cmd->cmdidx);
 
 	/* check RSP TIMEOUT or FAIL */
 	if (sta & FTSDC010_STATUS_RSP_TIMEOUT) {
 		/* RSP TIMEOUT */
-		debug("%s: RSP timeout: sta: %08x cmd %d\n",
-				__func__, sta, cmd->cmdidx);
+		debug("%s: RSP timeout: sta: %08x\n", __func__, sta);
 
 		clear |= FTSDC010_CLR_RSP_TIMEOUT;
 		writel(clear, &host->reg->clr);
@@ -226,47 +193,62 @@ static int ftsdc010_pio_check_status(struct mmc *mmc, struct mmc_cmd *cmd,
 		return TIMEOUT;
 	} else if (sta & FTSDC010_STATUS_RSP_CRC_FAIL) {
 		/* clear response fail bit */
-		debug("%s: RSP CRC FAIL: sta: %08x cmd %d\n",
-				__func__, sta, cmd->cmdidx);
+		debug("%s: RSP CRC FAIL: sta: %08x\n", __func__, sta);
 
 		clear |= FTSDC010_CLR_RSP_CRC_FAIL;
 		writel(clear, &host->reg->clr);
 
-		return 0;
+		return COMM_ERR;
 	} else if (sta & FTSDC010_STATUS_RSP_CRC_OK) {
 
 		/* clear response CRC OK bit */
 		clear |= FTSDC010_CLR_RSP_CRC_OK;
 	}
 
+	writel(clear, &host->reg->clr);
+	return 0;
+}
+
+static int ftsdc010_check_data(struct mmc *mmc, struct mmc_cmd *cmd,
+			struct mmc_data *data)
+{
+	struct mmc_host *host = mmc->priv;
+	unsigned int sta, clear;
+
+	sta = readl(&host->reg->status);
+	debug("%s: sta: %08x cmd %d\n", __func__, sta, cmd->cmdidx);
+
 	/* check DATA TIMEOUT or FAIL */
 	if (data) {
+		if (sta & FTSDC010_STATUS_DATA_END) {
+			/* Transfer Complete */
+			clear |= FTSDC010_STATUS_DATA_END;
+		}
+
+		if (sta & FTSDC010_STATUS_DATA_CRC_OK) {
+			/* Data CRC_OK */
+			clear |= FTSDC010_STATUS_DATA_CRC_OK;
+		}
+
 		if (sta & FTSDC010_STATUS_DATA_TIMEOUT) {
 			/* DATA TIMEOUT */
-			debug("%s: DATA TIMEOUT: sta: %08x\n",
-					__func__, sta);
+			debug("%s: DATA TIMEOUT: sta: %08x\n", __func__, sta);
 
 			clear |= FTSDC010_STATUS_DATA_TIMEOUT;
 			writel(sta, &host->reg->clr);
+
 			return TIMEOUT;
 		} else if (sta & FTSDC010_STATUS_DATA_CRC_FAIL) {
 			/* Error Interrupt */
-			debug("%s: DATA CRC FAIL: sta: %08x\n",
-					__func__, sta);
+			debug("%s: DATA CRC FAIL: sta: %08x\n", __func__, sta);
 
 			clear |= FTSDC010_STATUS_DATA_CRC_FAIL;
 			writel(clear, &host->reg->clr);
 
-			return 0;
-		} else if (sta & FTSDC010_STATUS_DATA_END) {
-			/* Transfer Complete */
-			clear |= FTSDC010_STATUS_DATA_END;
+			return COMM_ERR;
 		}
+		writel(clear, &host->reg->clr);
 	}
-
-	/* transaction is success and clear status register */
-	writel(clear, &host->reg->clr);
-
 	return 0;
 }
 
@@ -281,6 +263,9 @@ static int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 	unsigned int ccon;
 	unsigned int mask, tmpmask;
 	unsigned int ret;
+	unsigned int sta, i;
+
+	ret = 0;
 
 	if (data)
 		mask = FTSDC010_INT_MASK_RSP_TIMEOUT;
@@ -290,13 +275,9 @@ static int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 		mask = FTSDC010_INT_MASK_CMD_SEND;
 
 	/* write argu reg */
-	debug("%s: cmd->arg: %08x\n", __func__, cmd->cmdarg);
+	debug("%s: argu: %08x\n", __func__, host->reg->argu);
 	writel(cmd->cmdarg, &host->reg->argu);
 
-	/* setup cmd reg */
-	debug("cmd: %d\n", cmd->cmdidx);
-	debug("resp: %08x\n", cmd->resp_type);
-
 	/* setup commnad */
 	ccon = FTSDC010_CMD_IDX(cmd->cmdidx);
 
@@ -340,7 +321,51 @@ static int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 	/* write cmd reg */
 	debug("%s: ccon: %08x\n", __func__, ccon);
 	writel(ccon, &host->reg->cmd);
-	udelay(4*FTSDC010_DELAY_UNIT);
+
+	/* check CMD_SEND */
+	for (i = 0; i < FTSDC010_CMD_RETRY; i++) {
+		/*
+		 * If we read status register too fast
+		 * will lead hardware error and the RSP_TIMEOUT
+		 * flag will be raised incorrectly.
+		 */
+		udelay(16*FTSDC010_DELAY_UNIT);
+		sta = readl(&host->reg->status);
+
+		/* Command Complete */
+		/*
+		 * Note:
+		 *	Do not clear FTSDC010_CLR_CMD_SEND flag.
+		 *	(by writing FTSDC010_CLR_CMD_SEND bit to clear register)
+		 *	It will make the driver becomes very slow.
+		 *	If the operation hasn't been finished, hardware will
+		 *	clear this bit automatically.
+		 *	In origin, the driver will clear this flag if there is
+		 *	no data need to be read.
+		 */
+		if (sta & FTSDC010_STATUS_CMD_SEND)
+			break;
+	}
+
+	if (i > FTSDC010_CMD_RETRY) {
+		printf("%s: send command timeout\n", __func__);
+		return TIMEOUT;
+	}
+
+	/* check rsp status */
+	ret = ftsdc010_check_rsp(mmc, cmd, data);
+	if (ret)
+		return ret;
+
+	/* read response if we have RSP_OK */
+	if (ccon & FTSDC010_CMD_LONG_RSP) {
+		cmd->response[0] = readl(&host->reg->rsp3);
+		cmd->response[1] = readl(&host->reg->rsp2);
+		cmd->response[2] = readl(&host->reg->rsp1);
+		cmd->response[3] = readl(&host->reg->rsp0);
+	} else {
+		cmd->response[0] = readl(&host->reg->rsp0);
+	}
 
 	/* read/write data */
 	if (data && (data->flags & MMC_DATA_READ)) {
@@ -351,19 +376,11 @@ static int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 				data->blocksize * data->blocks);
 	}
 
-	/* pio check response status */
-	ret = ftsdc010_pio_check_status(mmc, cmd, data);
-	if (!ret) {
-		/* if it is long response */
-		if (ccon & FTSDC010_CMD_LONG_RSP) {
-			cmd->response[0] = readl(&host->reg->rsp3);
-			cmd->response[1] = readl(&host->reg->rsp2);
-			cmd->response[2] = readl(&host->reg->rsp1);
-			cmd->response[3] = readl(&host->reg->rsp0);
-
-		} else {
-			cmd->response[0] = readl(&host->reg->rsp0);
-		}
+	/* check data status */
+	if (data) {
+		ret = ftsdc010_check_data(mmc, cmd, data);
+		if (ret)
+			return ret;
 	}
 
 	udelay(FTSDC010_DELAY_UNIT);
@@ -431,8 +448,6 @@ static int ftsdc010_setup_data(struct mmc *mmc, struct mmc_data *data)
 	/* always reset fifo since last transfer may fail */
 	dcon |= FTSDC010_DCR_FIFO_RST;
 
-	/* handle sdio */
-	dcon = data->blocksize | data->blocks << 15;
 	if (data->blocks > 1)
 		dcon |= FTSDC010_SDIO_CTRL1_SDIO_BLK_MODE;
 #endif
@@ -497,7 +512,7 @@ static void ftsdc010_set_clk(struct mmc *mmc)
 {
 	struct mmc_host *host = mmc->priv;
 	unsigned char clk_div;
-	unsigned char real_rate;
+	unsigned int real_rate;
 	unsigned int clock;
 
 	debug("%s: mmc_set_clock: %x\n", __func__, mmc->clock);
@@ -518,7 +533,7 @@ static void ftsdc010_set_clk(struct mmc *mmc)
 				break;
 		}
 
-		debug("%s: computed real_rete: %x, clk_div: %x\n",
+		debug("%s: computed real_rate: %x, clk_div: %x\n",
 			 __func__, real_rate, clk_div);
 
 		if (clk_div > 127)
@@ -579,6 +594,7 @@ static void ftsdc010_set_ios(struct mmc *mmc)
 static void ftsdc010_reset(struct mmc_host *host)
 {
 	unsigned int timeout;
+	unsigned int sta;
 
 	/* Do SDC_RST: Software reset for all register */
 	writel(FTSDC010_CMD_SDC_RST, &host->reg->cmd);
@@ -598,6 +614,10 @@ static void ftsdc010_reset(struct mmc_host *host)
 		timeout--;
 		udelay(10*FTSDC010_DELAY_UNIT);
 	}
+
+	sta = readl(&host->reg->status);
+	if (sta & FTSDC010_STATUS_CARD_CHANGE)
+		writel(FTSDC010_CLR_CARD_CHANGE, &host->reg->clr);
 }
 
 static int ftsdc010_core_init(struct mmc *mmc)
@@ -650,8 +670,6 @@ int ftsdc010_mmc_init(int dev_index)
 
 	mmc->host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT;
 
-	mmc->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
-
 	mmc->f_min = CONFIG_SYS_CLK_FREQ / 2 / (2*128);
 	mmc->f_max = CONFIG_SYS_CLK_FREQ / 2 / 2;
 
-- 
1.7.3.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v3] ftsdc010: improve performance and capability
  2011-11-17  9:34 ` [U-Boot] [PATCH v3] " Macpaul Lin
@ 2011-11-26  0:07   ` Andy Fleming
  2011-11-28  8:07     ` Macpaul Lin
  0 siblings, 1 reply; 10+ messages in thread
From: Andy Fleming @ 2011-11-26  0:07 UTC (permalink / raw)
  To: u-boot

On Thu, Nov 17, 2011 at 3:34 AM, Macpaul Lin <macpaul@andestech.com> wrote:
> This patch improve the performance by spliting flag examination code
> in ftsdc010_send_cmd() into 3 functions.
> This patch also reordered the function which made better capability to
> some high performance cards against to the next version of ftsdc010
> hardware.
>
> Signed-off-by: Macpaul Lin <macpaul@andestech.com>
> ---
> Changes for v2:
> ?- Fix the problem if we read status register too fast in FTSDC010_CMD_RETRY
> ? ?loop. If we read status register here too fast, the hardware will report
> ? ?RSP_TIMEOUT incorrectly.
> Changes for v3:
> ?- Remove host high speed capability due to hardware limitation.
> ?- Remove unused variables.
>
> ?drivers/mmc/ftsdc010_esdhc.c | ?184 +++++++++++++++++++++++-------------------
> ?1 files changed, 101 insertions(+), 83 deletions(-)

> @@ -146,7 +151,7 @@ static void ftsdc010_pio_write(struct mmc_host *host, const char *buf,
> ? ? ? ?while (size) {
> ? ? ? ? ? ? ? ?status = readl(&host->reg->status);
>
> - ? ? ? ? ? ? ? if (status & FTSDC010_STATUS_FIFO_ORUN) {
> + ? ? ? ? ? ? ? if (status & FTSDC010_STATUS_FIFO_URUN) {


Was this a bug before? If so, it should be mentioned in the changelog
that you fixed it.


> -static int ftsdc010_pio_check_status(struct mmc *mmc, struct mmc_cmd *cmd,
> +static int ftsdc010_check_rsp(struct mmc *mmc, struct mmc_cmd *cmd,
> ? ? ? ? ? ? ? ? ? ? ? ?struct mmc_data *data)
> ?{
> ? ? ? ?struct mmc_host *host = mmc->priv;
> -
> ? ? ? ?unsigned int sta, clear;
> - ? ? ? unsigned int i;
> -
> - ? ? ? /* check response and hardware status */
> - ? ? ? clear = 0;
> -
> - ? ? ? /* chech CMD_SEND */
> - ? ? ? for (i = 0; i < FTSDC010_CMD_RETRY; i++) {
> - ? ? ? ? ? ? ? sta = readl(&host->reg->status);
> - ? ? ? ? ? ? ? /* Command Complete */
> - ? ? ? ? ? ? ? if (sta & FTSDC010_STATUS_CMD_SEND) {
> - ? ? ? ? ? ? ? ? ? ? ? if (!data)
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? clear |= FTSDC010_CLR_CMD_SEND;
> - ? ? ? ? ? ? ? ? ? ? ? break;
> - ? ? ? ? ? ? ? }
> - ? ? ? }
> -
> - ? ? ? if (i > FTSDC010_CMD_RETRY) {
> - ? ? ? ? ? ? ? printf("%s: send command timeout\n", __func__);
> - ? ? ? ? ? ? ? return TIMEOUT;
> - ? ? ? }
> -
> - ? ? ? /* debug: print status register and command index*/
> - ? ? ? debug("sta: %08x cmd %d\n", sta, cmd->cmdidx);
>
> - ? ? ? /* handle data FIFO */
> - ? ? ? if ((sta & FTSDC010_STATUS_FIFO_ORUN) ||
> - ? ? ? ? ? ? ? (sta & FTSDC010_STATUS_FIFO_URUN)) {
> -
> - ? ? ? ? ? ? ? /* Wrong DATA FIFO Flag */
> - ? ? ? ? ? ? ? if (data == NULL)
> - ? ? ? ? ? ? ? ? ? ? ? printf("%s, data fifo wrong: sta: %08x cmd %d\n",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __func__, sta, cmd->cmdidx);
> -
> - ? ? ? ? ? ? ? if (sta & FTSDC010_STATUS_FIFO_ORUN)
> - ? ? ? ? ? ? ? ? ? ? ? clear |= FTSDC010_STATUS_FIFO_ORUN;
> - ? ? ? ? ? ? ? if (sta & FTSDC010_STATUS_FIFO_URUN)
> - ? ? ? ? ? ? ? ? ? ? ? clear |= FTSDC010_STATUS_FIFO_URUN;
> - ? ? ? }
> + ? ? ? sta = readl(&host->reg->status);
> + ? ? ? debug("%s: sta: %08x cmd %d\n", __func__, sta, cmd->cmdidx);
>
> ? ? ? ?/* check RSP TIMEOUT or FAIL */
> ? ? ? ?if (sta & FTSDC010_STATUS_RSP_TIMEOUT) {
> ? ? ? ? ? ? ? ?/* RSP TIMEOUT */
> - ? ? ? ? ? ? ? debug("%s: RSP timeout: sta: %08x cmd %d\n",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __func__, sta, cmd->cmdidx);
> + ? ? ? ? ? ? ? debug("%s: RSP timeout: sta: %08x\n", __func__, sta);
>
> ? ? ? ? ? ? ? ?clear |= FTSDC010_CLR_RSP_TIMEOUT;
> ? ? ? ? ? ? ? ?writel(clear, &host->reg->clr);
> @@ -226,47 +193,62 @@ static int ftsdc010_pio_check_status(struct mmc *mmc, struct mmc_cmd *cmd,
> ? ? ? ? ? ? ? ?return TIMEOUT;
> ? ? ? ?} else if (sta & FTSDC010_STATUS_RSP_CRC_FAIL) {
> ? ? ? ? ? ? ? ?/* clear response fail bit */
> - ? ? ? ? ? ? ? debug("%s: RSP CRC FAIL: sta: %08x cmd %d\n",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __func__, sta, cmd->cmdidx);
> + ? ? ? ? ? ? ? debug("%s: RSP CRC FAIL: sta: %08x\n", __func__, sta);
>
> ? ? ? ? ? ? ? ?clear |= FTSDC010_CLR_RSP_CRC_FAIL;
> ? ? ? ? ? ? ? ?writel(clear, &host->reg->clr);
>
> - ? ? ? ? ? ? ? return 0;
> + ? ? ? ? ? ? ? return COMM_ERR;
> ? ? ? ?} else if (sta & FTSDC010_STATUS_RSP_CRC_OK) {
>
> ? ? ? ? ? ? ? ?/* clear response CRC OK bit */
> ? ? ? ? ? ? ? ?clear |= FTSDC010_CLR_RSP_CRC_OK;
> ? ? ? ?}
>
> + ? ? ? writel(clear, &host->reg->clr);
> + ? ? ? return 0;
> +}
> +
> +static int ftsdc010_check_data(struct mmc *mmc, struct mmc_cmd *cmd,
> + ? ? ? ? ? ? ? ? ? ? ? struct mmc_data *data)
> +{
> + ? ? ? struct mmc_host *host = mmc->priv;
> + ? ? ? unsigned int sta, clear;
> +
> + ? ? ? sta = readl(&host->reg->status);
> + ? ? ? debug("%s: sta: %08x cmd %d\n", __func__, sta, cmd->cmdidx);
> +
> ? ? ? ?/* check DATA TIMEOUT or FAIL */
> ? ? ? ?if (data) {
> + ? ? ? ? ? ? ? if (sta & FTSDC010_STATUS_DATA_END) {
> + ? ? ? ? ? ? ? ? ? ? ? /* Transfer Complete */
> + ? ? ? ? ? ? ? ? ? ? ? clear |= FTSDC010_STATUS_DATA_END;
> + ? ? ? ? ? ? ? }
> +
> + ? ? ? ? ? ? ? if (sta & FTSDC010_STATUS_DATA_CRC_OK) {
> + ? ? ? ? ? ? ? ? ? ? ? /* Data CRC_OK */
> + ? ? ? ? ? ? ? ? ? ? ? clear |= FTSDC010_STATUS_DATA_CRC_OK;
> + ? ? ? ? ? ? ? }


Instead of:

if (foo) {
  /* comment */
  bar;
}

It's better, I think to do:

/* comment */
if (foo)
  bar;

However, aside from that, the interrupt clearing confuses me. Usually,
you read the event register, and then write it back to clear
it. If there is more than one error, some of the status bits will be
left uncleared. If you only want to clear the bits being dealt with in
a particular section, I think it would be clearer and safer to set
"clear" up-front based on a MASK of bits that are being dealt with in
that section.

> +
> ? ? ? ? ? ? ? ?if (sta & FTSDC010_STATUS_DATA_TIMEOUT) {
> ? ? ? ? ? ? ? ? ? ? ? ?/* DATA TIMEOUT */
> - ? ? ? ? ? ? ? ? ? ? ? debug("%s: DATA TIMEOUT: sta: %08x\n",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __func__, sta);
> + ? ? ? ? ? ? ? ? ? ? ? debug("%s: DATA TIMEOUT: sta: %08x\n", __func__, sta);
>
> ? ? ? ? ? ? ? ? ? ? ? ?clear |= FTSDC010_STATUS_DATA_TIMEOUT;

Why set clear? This code returns before clear is written.
> ? ? ? ? ? ? ? ? ? ? ? ?writel(sta, &host->reg->clr);
> +
> ? ? ? ? ? ? ? ? ? ? ? ?return TIMEOUT;
> ? ? ? ? ? ? ? ?} else if (sta & FTSDC010_STATUS_DATA_CRC_FAIL) {
> ? ? ? ? ? ? ? ? ? ? ? ?/* Error Interrupt */
> - ? ? ? ? ? ? ? ? ? ? ? debug("%s: DATA CRC FAIL: sta: %08x\n",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __func__, sta);
> + ? ? ? ? ? ? ? ? ? ? ? debug("%s: DATA CRC FAIL: sta: %08x\n", __func__, sta);
>
> ? ? ? ? ? ? ? ? ? ? ? ?clear |= FTSDC010_STATUS_DATA_CRC_FAIL;
> ? ? ? ? ? ? ? ? ? ? ? ?writel(clear, &host->reg->clr);

Ok, here "clear" is actually written to the register, but doesn't it
leave open the possibility that DATA_END is still there? Maybe it
doesn't matter for this, but it seems very fragile.

Andy

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v3] ftsdc010: improve performance and capability
  2011-11-26  0:07   ` Andy Fleming
@ 2011-11-28  8:07     ` Macpaul Lin
  2011-11-28 16:02       ` Andy Fleming
  0 siblings, 1 reply; 10+ messages in thread
From: Macpaul Lin @ 2011-11-28  8:07 UTC (permalink / raw)
  To: u-boot

Hi Andy,

> Changes for v2:
> >  - Fix the problem if we read status register too fast in
> FTSDC010_CMD_RETRY
> >    loop. If we read status register here too fast, the hardware will
> report
> >    RSP_TIMEOUT incorrectly.
> > Changes for v3:
> >  - Remove host high speed capability due to hardware limitation.
> >  - Remove unused variables.
> > -               if (status & FTSDC010_STATUS_FIFO_ORUN) {
> > +               if (status & FTSDC010_STATUS_FIFO_URUN) {
>
>
> Was this a bug before? If so, it should be mentioned in the changelog
> that you fixed it.
>
>
Thanks. I missed this modification to be marked in the change log.


>
> >        /* check DATA TIMEOUT or FAIL */
> >        if (data) {
> > +               if (sta & FTSDC010_STATUS_DATA_END) {
> > +                       /* Transfer Complete */
> > +                       clear |= FTSDC010_STATUS_DATA_END;
> > +               }
> > +
> > +               if (sta & FTSDC010_STATUS_DATA_CRC_OK) {
> > +                       /* Data CRC_OK */
> > +                       clear |= FTSDC010_STATUS_DATA_CRC_OK;
> > +               }
>
> Instead of:
>
> if (foo) {
>  /* comment */
>  bar;
> }
>
> It's better, I think to do:
>
> /* comment */
> if (foo)
>  bar;
>

 Okay, I'll fix it in patch v4.


> However, aside from that, the interrupt clearing confuses me. Usually,
> you read the event register, and then write it back to clear
> it. If there is more than one error, some of the status bits will be
> left uncleared. If you only want to clear the bits being dealt with in
> a particular section, I think it would be clearer and safer to set
> "clear" up-front based on a MASK of bits that are being dealt with in
> that section.
>

The MASK bits doesn't really work at all. :-(
If I've disabled some of the interrupt flags by mask, these disabled flag
will still raise
(I think is a design flaw in hardware) if the error or success has happened.

The event (status) register is different from the clear register.
They are 2 different register with slightly different definition of their
bit fields,
these 2 registers are only partially identical of the meaning of the flags.

The flags indeed can be separate to different stage during one command
transaction.
 Actually, not all the error status bit will raise at the same time.
Some flags will only be raised exclusively.
For example, there will be only one flag raised on time for the following 3
flags,
FTSDC010_STATUS_RSP_TIMEOUT, FTSDC010_STATUS_RSP_CRC_FAIL, and
FTSDC010_STATUS_RSP_CRC_OK.
If one of the flag didn't be cleared right now, say, RSP_TIMEOUT,  the
hardware will clear it if RSP_CRC_FAIL must be
raised in the next time.


>
> > +
> >                if (sta & FTSDC010_STATUS_DATA_TIMEOUT) {
> >                        /* DATA TIMEOUT */
> > -                       debug("%s: DATA TIMEOUT: sta: %08x\n",
> > -                                       __func__, sta);
> > +                       debug("%s: DATA TIMEOUT: sta: %08x\n", __func__,
> sta);
> >
> >                        clear |= FTSDC010_STATUS_DATA_TIMEOUT;
>
> Why set clear? This code returns before clear is written.
> >                        writel(sta, &host->reg->clr);
> > +
> >                        return TIMEOUT;
>

Why did you say the code "returns before clear is written"?
FTSDC010_STATUS_DATA_TIMEOUT and FTSDC010_STATUS_DATA_CRC_FAIL are
exclusive just like
RSP_TIMEOUT and RSP_CRC_FAIL managed by hardware.
The driver will indeed clear DATA_TIMEOUT when the flag of clear has been
set and then will be wrote into
clear register on the next line of the code "writel(sta, &host->reg->clr);",
Finally we return TIMEOUT since the DATA_TIMEOUT has been detected.

We have a COMM_ERR returned after the DATA_CRC_FAIL has been detected and
cleared also.


>  >                } else if (sta & FTSDC010_STATUS_DATA_CRC_FAIL) {
> >                        /* Error Interrupt */
> > -                       debug("%s: DATA CRC FAIL: sta: %08x\n",
> > -                                       __func__, sta);
> > +                       debug("%s: DATA CRC FAIL: sta: %08x\n",
> __func__, sta);
> >
> >                        clear |= FTSDC010_STATUS_DATA_CRC_FAIL;
> >                        writel(clear, &host->reg->clr);
>
> Ok, here "clear" is actually written to the register, but doesn't it
> leave open the possibility that DATA_END is still there? Maybe it
> doesn't matter for this, but it seems very fragile.
>
>
The clear here is used to clear FTSDC010_STATUS_DATA_END and
FTSDC010_STATUS_DATA_CRC_OK.
Only these 2 flags are independent to other DATA related flags.
The last writel() is used to clear up these 2 DATA_END and DATA_CRC_OK if
TIMEOUT and CRC_ERR didn't happened.


-- 
Best regards,
Macpaul Lin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v3] ftsdc010: improve performance and capability
  2011-11-28  8:07     ` Macpaul Lin
@ 2011-11-28 16:02       ` Andy Fleming
  2011-11-29  2:52         ` Macpaul Lin
  2011-11-29  3:30         ` [U-Boot] [PATCH v4] " Macpaul Lin
  0 siblings, 2 replies; 10+ messages in thread
From: Andy Fleming @ 2011-11-28 16:02 UTC (permalink / raw)
  To: u-boot

On Mon, Nov 28, 2011 at 2:07 AM, Macpaul Lin <macpaul@gmail.com> wrote:
>> However, aside from that, the interrupt clearing confuses me. Usually,
>> you read the event register, and then write it back to clear
>> it. If there is more than one error, some of the status bits will be
>> left uncleared. If you only want to clear the bits being dealt with in
>> a particular section, I think it would be clearer and safer to set
>> "clear" up-front based on a MASK of bits that are being dealt with in
>> that section.
>
> The MASK bits doesn't really work at all. :-(
> If I've disabled some of the interrupt flags by mask, these disabled flag
> will still raise
> (I think is a design flaw in hardware) if the error or success has happened.
>
> The event (status) register is different from the clear register.
> They are 2 different register with slightly different definition of their
> bit fields,
> these 2 registers are only partially identical of the meaning of the flags.
>
> The flags indeed can be separate to different stage during one command
> transaction.
> ?Actually, not all the error status bit will raise at the same time.
> Some flags will only be raised exclusively.
> For example, there will be only one flag raised on time for the following 3
> flags,
> FTSDC010_STATUS_RSP_TIMEOUT, FTSDC010_STATUS_RSP_CRC_FAIL, and
> FTSDC010_STATUS_RSP_CRC_OK.
> If one of the flag didn't be cleared right now, say, RSP_TIMEOUT,? the
> hardware will clear it if RSP_CRC_FAIL must be
> raised in the next time.

Alright, if you say the bits aren't all the same, and they will be
cleared by the hardware before the next interrupt, then I'll withdraw
that issue.

>
>>
>> > +
>> > ? ? ? ? ? ? ? ?if (sta & FTSDC010_STATUS_DATA_TIMEOUT) {
>> > ? ? ? ? ? ? ? ? ? ? ? ?/* DATA TIMEOUT */
>> > - ? ? ? ? ? ? ? ? ? ? ? debug("%s: DATA TIMEOUT: sta: %08x\n",
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __func__, sta);
>> > + ? ? ? ? ? ? ? ? ? ? ? debug("%s: DATA TIMEOUT: sta: %08x\n", __func__,
>> > sta);
>> >
>> > ? ? ? ? ? ? ? ? ? ? ? ?clear |= FTSDC010_STATUS_DATA_TIMEOUT;
>>
>> Why set clear? This code returns before clear is written.
>> > ? ? ? ? ? ? ? ? ? ? ? ?writel(sta, &host->reg->clr);
>> > +
>> > ? ? ? ? ? ? ? ? ? ? ? ?return TIMEOUT;
>
> Why did you say the code "returns before clear is written"?

I'm saying this sets "clear" to FTSDC010_STATUS_DATA_TIMEOUT, but then
it writes "sta" to the clr register, and returns.

"clear" is never used after being set in this case.


> FTSDC010_STATUS_DATA_TIMEOUT and FTSDC010_STATUS_DATA_CRC_FAIL are exclusive
> just like
> RSP_TIMEOUT and RSP_CRC_FAIL managed by hardware.
> The driver will indeed clear DATA_TIMEOUT when the flag of clear has been
> set and then will be wrote into
> clear register on the next line of the code "writel(sta, &host->reg->clr);",
> Finally we return TIMEOUT since the DATA_TIMEOUT has been detected.
>
> We have a COMM_ERR returned after the DATA_CRC_FAIL has been detected and
> cleared also.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v3] ftsdc010: improve performance and capability
  2011-11-28 16:02       ` Andy Fleming
@ 2011-11-29  2:52         ` Macpaul Lin
  2011-11-29  3:30         ` [U-Boot] [PATCH v4] " Macpaul Lin
  1 sibling, 0 replies; 10+ messages in thread
From: Macpaul Lin @ 2011-11-29  2:52 UTC (permalink / raw)
  To: u-boot

Hi Andy

2011/11/29 Andy Fleming <afleming@gmail.com>

>  >> >
> >> >                        clear |= FTSDC010_STATUS_DATA_TIMEOUT;
> >>
> >> Why set clear? This code returns before clear is written.
> >> >                        writel(sta, &host->reg->clr);
> >> > +
> >> >                        return TIMEOUT;
> >
> > Why did you say the code "returns before clear is written"?
>
> I'm saying this sets "clear" to FTSDC010_STATUS_DATA_TIMEOUT, but then
> it writes "sta" to the clr register, and returns.
>
> "clear" is never used after being set in this case.


You're really caught a bug. Sorry I didn't aware what you meant before.
I thought I've fix this bug at the 1st version of the patch. This is weird.
Anyway, I'll fix this in patch v4 also.
Thanks!

-- 
Best regards,
Macpaul Lin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v4] ftsdc010: improve performance and capability
  2011-11-28 16:02       ` Andy Fleming
  2011-11-29  2:52         ` Macpaul Lin
@ 2011-11-29  3:30         ` Macpaul Lin
  2011-12-30  6:12           ` Andy Fleming
  1 sibling, 1 reply; 10+ messages in thread
From: Macpaul Lin @ 2011-11-29  3:30 UTC (permalink / raw)
  To: u-boot

This patch improve the performance by spliting flag examination code
in ftsdc010_send_cmd() into 3 functions.
This patch also reordered the function which made better capability to
some high performance cards against to the next version of ftsdc010
hardware.

Signed-off-by: Macpaul Lin <macpaul@andestech.com>
---
Changes for v2:
  - Fix the problem if we read status register too fast in FTSDC010_CMD_RETRY
    loop. If we read status register here too fast, the hardware will report
    RSP_TIMEOUT incorrectly.
Changes for v3:
  - Remove host high speed capability due to hardware limitation.
  - Remove unused variables.
Changes for v4:
  - Add missing change log of replacing FTSDC010_STATUS_FIFO_ORUN by
    FTSDC010_STATUS_FIFO_URUN in ftsdc010_pio_write()
  - Replace comment of processing flag of DATA_TIMEOUT by simple format.
  - Fix incorrect register updating on processing DATA_TIMEOUT flag.

 drivers/mmc/ftsdc010_esdhc.c |  188 +++++++++++++++++++++++-------------------
 1 files changed, 103 insertions(+), 85 deletions(-)

diff --git a/drivers/mmc/ftsdc010_esdhc.c b/drivers/mmc/ftsdc010_esdhc.c
index e38dd87..33cb5d6 100644
--- a/drivers/mmc/ftsdc010_esdhc.c
+++ b/drivers/mmc/ftsdc010_esdhc.c
@@ -90,8 +90,13 @@ static void ftsdc010_pio_read(struct mmc_host *host, char *buf, unsigned int siz
 
 	while (size) {
 		status = readl(&host->reg->status);
+		debug("%s: size: %08x\n", __func__, size);
 
 		if (status & FTSDC010_STATUS_FIFO_ORUN) {
+
+			debug("%s: FIFO OVERRUN: sta: %08x\n",
+					__func__, status);
+
 			fifo = host->fifo_len > size ?
 				size : host->fifo_len;
 
@@ -146,7 +151,7 @@ static void ftsdc010_pio_write(struct mmc_host *host, const char *buf,
 	while (size) {
 		status = readl(&host->reg->status);
 
-		if (status & FTSDC010_STATUS_FIFO_ORUN) {
+		if (status & FTSDC010_STATUS_FIFO_URUN) {
 			fifo = host->fifo_len > size ?
 				size : host->fifo_len;
 
@@ -158,7 +163,6 @@ static void ftsdc010_pio_write(struct mmc_host *host, const char *buf,
 				writel(*ptr, &host->reg->dwr);
 				ptr++;
 			}
-
 		} else {
 			udelay(1);
 			if (++retry >= FTSDC010_PIO_RETRY) {
@@ -169,56 +173,19 @@ static void ftsdc010_pio_write(struct mmc_host *host, const char *buf,
 	}
 }
 
-static int ftsdc010_pio_check_status(struct mmc *mmc, struct mmc_cmd *cmd,
+static int ftsdc010_check_rsp(struct mmc *mmc, struct mmc_cmd *cmd,
 			struct mmc_data *data)
 {
 	struct mmc_host *host = mmc->priv;
-
 	unsigned int sta, clear;
-	unsigned int i;
-
-	/* check response and hardware status */
-	clear = 0;
-
-	/* chech CMD_SEND */
-	for (i = 0; i < FTSDC010_CMD_RETRY; i++) {
-		sta = readl(&host->reg->status);
-		/* Command Complete */
-		if (sta & FTSDC010_STATUS_CMD_SEND) {
-			if (!data)
-				clear |= FTSDC010_CLR_CMD_SEND;
-			break;
-		}
-	}
-
-	if (i > FTSDC010_CMD_RETRY) {
-		printf("%s: send command timeout\n", __func__);
-		return TIMEOUT;
-	}
-
-	/* debug: print status register and command index*/
-	debug("sta: %08x cmd %d\n", sta, cmd->cmdidx);
 
-	/* handle data FIFO */
-	if ((sta & FTSDC010_STATUS_FIFO_ORUN) ||
-		(sta & FTSDC010_STATUS_FIFO_URUN)) {
-
-		/* Wrong DATA FIFO Flag */
-		if (data == NULL)
-			printf("%s, data fifo wrong: sta: %08x cmd %d\n",
-				__func__, sta, cmd->cmdidx);
-
-		if (sta & FTSDC010_STATUS_FIFO_ORUN)
-			clear |= FTSDC010_STATUS_FIFO_ORUN;
-		if (sta & FTSDC010_STATUS_FIFO_URUN)
-			clear |= FTSDC010_STATUS_FIFO_URUN;
-	}
+	sta = readl(&host->reg->status);
+	debug("%s: sta: %08x cmd %d\n", __func__, sta, cmd->cmdidx);
 
 	/* check RSP TIMEOUT or FAIL */
 	if (sta & FTSDC010_STATUS_RSP_TIMEOUT) {
 		/* RSP TIMEOUT */
-		debug("%s: RSP timeout: sta: %08x cmd %d\n",
-				__func__, sta, cmd->cmdidx);
+		debug("%s: RSP timeout: sta: %08x\n", __func__, sta);
 
 		clear |= FTSDC010_CLR_RSP_TIMEOUT;
 		writel(clear, &host->reg->clr);
@@ -226,47 +193,62 @@ static int ftsdc010_pio_check_status(struct mmc *mmc, struct mmc_cmd *cmd,
 		return TIMEOUT;
 	} else if (sta & FTSDC010_STATUS_RSP_CRC_FAIL) {
 		/* clear response fail bit */
-		debug("%s: RSP CRC FAIL: sta: %08x cmd %d\n",
-				__func__, sta, cmd->cmdidx);
+		debug("%s: RSP CRC FAIL: sta: %08x\n", __func__, sta);
 
 		clear |= FTSDC010_CLR_RSP_CRC_FAIL;
 		writel(clear, &host->reg->clr);
 
-		return 0;
+		return COMM_ERR;
 	} else if (sta & FTSDC010_STATUS_RSP_CRC_OK) {
 
 		/* clear response CRC OK bit */
 		clear |= FTSDC010_CLR_RSP_CRC_OK;
 	}
 
+	writel(clear, &host->reg->clr);
+	return 0;
+}
+
+static int ftsdc010_check_data(struct mmc *mmc, struct mmc_cmd *cmd,
+			struct mmc_data *data)
+{
+	struct mmc_host *host = mmc->priv;
+	unsigned int sta, clear;
+
+	sta = readl(&host->reg->status);
+	debug("%s: sta: %08x cmd %d\n", __func__, sta, cmd->cmdidx);
+
 	/* check DATA TIMEOUT or FAIL */
 	if (data) {
+
+		/* Transfer Complete */
+		if (sta & FTSDC010_STATUS_DATA_END)
+			clear |= FTSDC010_STATUS_DATA_END;
+
+		/* Data CRC_OK */
+		if (sta & FTSDC010_STATUS_DATA_CRC_OK)
+			clear |= FTSDC010_STATUS_DATA_CRC_OK;
+
+		/* DATA TIMEOUT or DATA CRC FAIL */
 		if (sta & FTSDC010_STATUS_DATA_TIMEOUT) {
 			/* DATA TIMEOUT */
-			debug("%s: DATA TIMEOUT: sta: %08x\n",
-					__func__, sta);
+			debug("%s: DATA TIMEOUT: sta: %08x\n", __func__, sta);
 
 			clear |= FTSDC010_STATUS_DATA_TIMEOUT;
-			writel(sta, &host->reg->clr);
+			writel(clear, &host->reg->clr);
+
 			return TIMEOUT;
 		} else if (sta & FTSDC010_STATUS_DATA_CRC_FAIL) {
-			/* Error Interrupt */
-			debug("%s: DATA CRC FAIL: sta: %08x\n",
-					__func__, sta);
+			/* DATA CRC FAIL */
+			debug("%s: DATA CRC FAIL: sta: %08x\n", __func__, sta);
 
 			clear |= FTSDC010_STATUS_DATA_CRC_FAIL;
 			writel(clear, &host->reg->clr);
 
-			return 0;
-		} else if (sta & FTSDC010_STATUS_DATA_END) {
-			/* Transfer Complete */
-			clear |= FTSDC010_STATUS_DATA_END;
+			return COMM_ERR;
 		}
+		writel(clear, &host->reg->clr);
 	}
-
-	/* transaction is success and clear status register */
-	writel(clear, &host->reg->clr);
-
 	return 0;
 }
 
@@ -281,6 +263,9 @@ static int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 	unsigned int ccon;
 	unsigned int mask, tmpmask;
 	unsigned int ret;
+	unsigned int sta, i;
+
+	ret = 0;
 
 	if (data)
 		mask = FTSDC010_INT_MASK_RSP_TIMEOUT;
@@ -290,13 +275,9 @@ static int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 		mask = FTSDC010_INT_MASK_CMD_SEND;
 
 	/* write argu reg */
-	debug("%s: cmd->arg: %08x\n", __func__, cmd->cmdarg);
+	debug("%s: argu: %08x\n", __func__, host->reg->argu);
 	writel(cmd->cmdarg, &host->reg->argu);
 
-	/* setup cmd reg */
-	debug("cmd: %d\n", cmd->cmdidx);
-	debug("resp: %08x\n", cmd->resp_type);
-
 	/* setup commnad */
 	ccon = FTSDC010_CMD_IDX(cmd->cmdidx);
 
@@ -340,7 +321,51 @@ static int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 	/* write cmd reg */
 	debug("%s: ccon: %08x\n", __func__, ccon);
 	writel(ccon, &host->reg->cmd);
-	udelay(4*FTSDC010_DELAY_UNIT);
+
+	/* check CMD_SEND */
+	for (i = 0; i < FTSDC010_CMD_RETRY; i++) {
+		/*
+		 * If we read status register too fast
+		 * will lead hardware error and the RSP_TIMEOUT
+		 * flag will be raised incorrectly.
+		 */
+		udelay(16*FTSDC010_DELAY_UNIT);
+		sta = readl(&host->reg->status);
+
+		/* Command Complete */
+		/*
+		 * Note:
+		 *	Do not clear FTSDC010_CLR_CMD_SEND flag.
+		 *	(by writing FTSDC010_CLR_CMD_SEND bit to clear register)
+		 *	It will make the driver becomes very slow.
+		 *	If the operation hasn't been finished, hardware will
+		 *	clear this bit automatically.
+		 *	In origin, the driver will clear this flag if there is
+		 *	no data need to be read.
+		 */
+		if (sta & FTSDC010_STATUS_CMD_SEND)
+			break;
+	}
+
+	if (i > FTSDC010_CMD_RETRY) {
+		printf("%s: send command timeout\n", __func__);
+		return TIMEOUT;
+	}
+
+	/* check rsp status */
+	ret = ftsdc010_check_rsp(mmc, cmd, data);
+	if (ret)
+		return ret;
+
+	/* read response if we have RSP_OK */
+	if (ccon & FTSDC010_CMD_LONG_RSP) {
+		cmd->response[0] = readl(&host->reg->rsp3);
+		cmd->response[1] = readl(&host->reg->rsp2);
+		cmd->response[2] = readl(&host->reg->rsp1);
+		cmd->response[3] = readl(&host->reg->rsp0);
+	} else {
+		cmd->response[0] = readl(&host->reg->rsp0);
+	}
 
 	/* read/write data */
 	if (data && (data->flags & MMC_DATA_READ)) {
@@ -351,19 +376,11 @@ static int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 				data->blocksize * data->blocks);
 	}
 
-	/* pio check response status */
-	ret = ftsdc010_pio_check_status(mmc, cmd, data);
-	if (!ret) {
-		/* if it is long response */
-		if (ccon & FTSDC010_CMD_LONG_RSP) {
-			cmd->response[0] = readl(&host->reg->rsp3);
-			cmd->response[1] = readl(&host->reg->rsp2);
-			cmd->response[2] = readl(&host->reg->rsp1);
-			cmd->response[3] = readl(&host->reg->rsp0);
-
-		} else {
-			cmd->response[0] = readl(&host->reg->rsp0);
-		}
+	/* check data status */
+	if (data) {
+		ret = ftsdc010_check_data(mmc, cmd, data);
+		if (ret)
+			return ret;
 	}
 
 	udelay(FTSDC010_DELAY_UNIT);
@@ -431,8 +448,6 @@ static int ftsdc010_setup_data(struct mmc *mmc, struct mmc_data *data)
 	/* always reset fifo since last transfer may fail */
 	dcon |= FTSDC010_DCR_FIFO_RST;
 
-	/* handle sdio */
-	dcon = data->blocksize | data->blocks << 15;
 	if (data->blocks > 1)
 		dcon |= FTSDC010_SDIO_CTRL1_SDIO_BLK_MODE;
 #endif
@@ -497,7 +512,7 @@ static void ftsdc010_set_clk(struct mmc *mmc)
 {
 	struct mmc_host *host = mmc->priv;
 	unsigned char clk_div;
-	unsigned char real_rate;
+	unsigned int real_rate;
 	unsigned int clock;
 
 	debug("%s: mmc_set_clock: %x\n", __func__, mmc->clock);
@@ -518,7 +533,7 @@ static void ftsdc010_set_clk(struct mmc *mmc)
 				break;
 		}
 
-		debug("%s: computed real_rete: %x, clk_div: %x\n",
+		debug("%s: computed real_rate: %x, clk_div: %x\n",
 			 __func__, real_rate, clk_div);
 
 		if (clk_div > 127)
@@ -579,6 +594,7 @@ static void ftsdc010_set_ios(struct mmc *mmc)
 static void ftsdc010_reset(struct mmc_host *host)
 {
 	unsigned int timeout;
+	unsigned int sta;
 
 	/* Do SDC_RST: Software reset for all register */
 	writel(FTSDC010_CMD_SDC_RST, &host->reg->cmd);
@@ -598,6 +614,10 @@ static void ftsdc010_reset(struct mmc_host *host)
 		timeout--;
 		udelay(10*FTSDC010_DELAY_UNIT);
 	}
+
+	sta = readl(&host->reg->status);
+	if (sta & FTSDC010_STATUS_CARD_CHANGE)
+		writel(FTSDC010_CLR_CARD_CHANGE, &host->reg->clr);
 }
 
 static int ftsdc010_core_init(struct mmc *mmc)
@@ -650,8 +670,6 @@ int ftsdc010_mmc_init(int dev_index)
 
 	mmc->host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT;
 
-	mmc->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
-
 	mmc->f_min = CONFIG_SYS_CLK_FREQ / 2 / (2*128);
 	mmc->f_max = CONFIG_SYS_CLK_FREQ / 2 / 2;
 
-- 
1.7.3.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v4] ftsdc010: improve performance and capability
  2011-11-29  3:30         ` [U-Boot] [PATCH v4] " Macpaul Lin
@ 2011-12-30  6:12           ` Andy Fleming
  0 siblings, 0 replies; 10+ messages in thread
From: Andy Fleming @ 2011-12-30  6:12 UTC (permalink / raw)
  To: u-boot

On Mon, Nov 28, 2011 at 9:30 PM, Macpaul Lin <macpaul@andestech.com> wrote:
> This patch improve the performance by spliting flag examination code
> in ftsdc010_send_cmd() into 3 functions.
> This patch also reordered the function which made better capability to
> some high performance cards against to the next version of ftsdc010
> hardware.
>
> Signed-off-by: Macpaul Lin <macpaul@andestech.com>


Applied, thanks!

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-12-30  6:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-03  9:35 [U-Boot] [PATCH] ftsdc010: improve performance and capability Macpaul Lin
2011-11-11  7:50 ` Macpaul Lin
2011-11-14 11:28 ` [U-Boot] [PATCH v2] " Macpaul Lin
2011-11-17  9:34 ` [U-Boot] [PATCH v3] " Macpaul Lin
2011-11-26  0:07   ` Andy Fleming
2011-11-28  8:07     ` Macpaul Lin
2011-11-28 16:02       ` Andy Fleming
2011-11-29  2:52         ` Macpaul Lin
2011-11-29  3:30         ` [U-Boot] [PATCH v4] " Macpaul Lin
2011-12-30  6:12           ` Andy Fleming

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.