[PATCH 0/6] mci: pxamci fixes

[PATCH 0/6] mci: pxamci fixes

[Robert Jarzmik]

This serie was triggered by several tests to copy files from
one directory to another on 2 SD cards :
 - one slow 256MB card
 - one fast 8G Transcend 8X card

The fixes improve :
 - writes which do no longer fail (mostly because of R1b and
 CMD12 handling)
 - initial mounting of SD card from env (poweron ramp delay)

Happy review.

Robert Jarzmik (6):
  mci: pxamci define timeouts
  mci: pxamci change clocks handling
  mci: pxamci fix response type
  mci: pxamci fix CMD12 handling
  mci: pxamci fix R1b responses
  mci: pxamci poweron ramp delay

 drivers/mci/pxamci.c |   49 +++++++++++++++++++++++++++++++------------------
 drivers/mci/pxamci.h |    1 +
 2 files changed, 32 insertions(+), 18 deletions(-)

-- 
1.7.5.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/6] mci: pxamci define timeouts

[Robert Jarzmik]

Instead of using hard encoded values in the code, use defines to setup
the timeouts of reads/writes/commands.
Fix the read timeout as defined in the PXA Developer Manual.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mci/pxamci.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/mci/pxamci.c b/drivers/mci/pxamci.c
index 239b46a..ee2f08c 100644
--- a/drivers/mci/pxamci.c
+++ b/drivers/mci/pxamci.c
@@ -25,6 +25,10 @@
 
 #define DRIVER_NAME	"pxa-mmc"
 
+#define RX_TIMEOUT (100 * MSECOND)
+#define TX_TIMEOUT (250 * MSECOND)
+#define CMD_TIMEOUT (100 * MSECOND)
+
 static void clk_enable(void)
 {
 	CKEN |= CKEN_MMC;
@@ -54,7 +58,7 @@ static void pxamci_stop_clock(struct pxamci_host *host)
 	stat = mmc_readl(MMC_STAT);
 	if (stat & STAT_CLK_EN)
 		writel(STOP_CLOCK, host->base + MMC_STRPCL);
-	while (!is_timeout(start, 10 * MSECOND) && stat & STAT_CLK_EN)
+	while (!is_timeout(start, CMD_TIMEOUT) && stat & STAT_CLK_EN)
 		stat = mmc_readl(MMC_STAT);
 
 	if (stat & STAT_CLK_EN)
@@ -63,12 +67,12 @@ static void pxamci_stop_clock(struct pxamci_host *host)
 
 static void pxamci_setup_data(struct pxamci_host *host, struct mci_data *data)
 {
-	static const unsigned int timeout = 100000000; /* 10ms */
+	static const unsigned int timeout_ns = 1000 * MSECOND; /* 1000 ms */
 
 	mci_dbg("nbblocks=%d, blocksize=%d\n", data->blocks, data->blocksize);
 	mmc_writel(data->blocks, MMC_NOB);
 	mmc_writel(data->blocksize, MMC_BLKLEN);
-	mmc_writel((timeout + 255) / 256, MMC_RDTO);
+	mmc_writel(DIV_ROUND_UP(timeout_ns, 13128), MMC_RDTO);
 }
 
 static int pxamci_read_data(struct pxamci_host *host, unsigned char *dst,
@@ -83,9 +87,10 @@ static int pxamci_read_data(struct pxamci_host *host, unsigned char *dst,
 		trf_len = min_t(int, len, MMC_FIFO_LENGTH);
 
 		for (start = get_time_ns(), ret = -ETIMEDOUT;
-		     ret && !is_timeout(start, 10 * MSECOND);)
+		     ret && !is_timeout(start, RX_TIMEOUT);)
 			if (mmc_readl(MMC_I_REG) & RXFIFO_RD_REQ)
 				ret = 0;
+
 		trf_len1 = trf_len % 4;
 		trf_len4 = trf_len / 4;
 		for (dst4 = (u32 *)dst; !ret && trf_len4 > 0; trf_len4--)
@@ -97,7 +102,7 @@ static int pxamci_read_data(struct pxamci_host *host, unsigned char *dst,
 
 	if (!ret)
 		for (start = get_time_ns(), ret = -ETIMEDOUT;
-		     ret && !is_timeout(start, 10 * MSECOND);)
+		     ret && !is_timeout(start, RX_TIMEOUT);)
 			if (mmc_readl(MMC_STAT) & STAT_DATA_TRAN_DONE)
 				ret = 0;
 	mci_dbg("ret=%d, remain=%d, stat=%x, mmc_i_reg=%x\n",
@@ -118,7 +123,7 @@ static int pxamci_write_data(struct pxamci_host *host, const unsigned char *src,
 		partial = trf_len < MMC_FIFO_LENGTH;
 
 		for (start = get_time_ns(), ret = -ETIMEDOUT;
-		     ret && !is_timeout(start, 10 * MSECOND);)
+		     ret && !is_timeout(start, TX_TIMEOUT);)
 			if (mmc_readl(MMC_I_REG) & TXFIFO_WR_REQ)
 				ret = 0;
 		for (; !ret && trf_len > 0; trf_len--, len--)
@@ -129,7 +134,7 @@ static int pxamci_write_data(struct pxamci_host *host, const unsigned char *src,
 
 	if (!ret)
 		for (start = get_time_ns(), ret = -ETIMEDOUT;
-		     ret && !is_timeout(start, 100 * MSECOND);)  {
+		     ret && !is_timeout(start, TX_TIMEOUT);)  {
 			stat = mmc_readl(MMC_STAT);
 			stat &= STAT_DATA_TRAN_DONE | STAT_PRG_DONE;
 			if (stat == (STAT_DATA_TRAN_DONE | STAT_PRG_DONE))
@@ -236,7 +241,7 @@ static int pxamci_mmccmd(struct pxamci_host *host, struct mci_cmd *cmd,
 
 	pxamci_start_cmd(host, cmd, cmddat);
 	for (start = get_time_ns(), ret = -ETIMEDOUT;
-	     ret && !is_timeout(start, 10 * MSECOND);)
+	     ret && !is_timeout(start, CMD_TIMEOUT);)
 		if (mmc_readl(MMC_STAT) & STAT_END_CMD_RES)
 			ret = 0;
 
-- 
1.7.5.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/6] mci: pxamci change clocks handling

[Robert Jarzmik]

Fix clock handling accordingly to PXA manual :
 - enable the MMC controller clock once and for all
 - only disable the MMC bus clock when changing the MMCLK by
 adjusting the clock ratio, else let the controller and SD
 card shut down the clock as the see fit.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mci/pxamci.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/mci/pxamci.c b/drivers/mci/pxamci.c
index ee2f08c..2d49187 100644
--- a/drivers/mci/pxamci.c
+++ b/drivers/mci/pxamci.c
@@ -190,7 +190,6 @@ static void pxamci_start_cmd(struct pxamci_host *host, struct mci_cmd *cmd,
 	mmc_writel(cmd->cmdidx, MMC_CMD);
 	mmc_writel(cmd->cmdarg >> 16, MMC_ARGH);
 	mmc_writel(cmd->cmdarg & 0xffff, MMC_ARGL);
-	mmc_writel(host->clkrt, MMC_CLKRT);
 	pxamci_start_clock(host);
 	mmc_writel(cmdat, MMC_CMDAT);
 }
@@ -260,8 +259,6 @@ static int pxamci_request(struct mci_host *mci, struct mci_cmd *cmd,
 	unsigned int cmdat;
 	int ret;
 
-	pxamci_stop_clock(host);
-
 	cmdat = host->cmdat;
 	host->cmdat &= ~CMDAT_INIT;
 
@@ -311,8 +308,9 @@ static void pxamci_set_ios(struct mci_host *mci, struct mci_ios *ios)
 
 	host->cmdat |= CMDAT_INIT;
 
-	clk_enable();
 	pxamci_set_power(host, 1);
+	pxamci_stop_clock(host);
+	mmc_writel(host->clkrt, MMC_CLKRT);
 }
 
 static int pxamci_init(struct mci_host *mci, struct device_d *dev)
@@ -329,6 +327,7 @@ static int pxamci_probe(struct device_d *dev)
 	struct pxamci_host *host;
 	int gpio_power = -1;
 
+	clk_enable();
 	host = xzalloc(sizeof(*host));
 	host->base = dev_request_mem_region(dev, 0);
 
-- 
1.7.5.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/6] mci: pxamci fix response type

[Robert Jarzmik]

When preparing a command, apply a mask so that only the command part
will be used for the switch case. This will be more robust
for future command response types.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mci/pxamci.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mci/pxamci.c b/drivers/mci/pxamci.c
index 2d49187..f251d75 100644
--- a/drivers/mci/pxamci.c
+++ b/drivers/mci/pxamci.c
@@ -163,18 +163,19 @@ static int pxamci_transfer_data(struct pxamci_host *host,
 	return ret;
 }
 
+#define MMC_RSP_MASK (MMC_RSP_PRESENT | MMC_RSP_136 | MMC_RSP_CRC | \
+		      MMC_RSP_BUSY | MMC_RSP_OPCODE)
 static void pxamci_start_cmd(struct pxamci_host *host, struct mci_cmd *cmd,
 			     unsigned int cmdat)
 {
 	mci_dbg("cmd=(idx=%d,type=%d,clkrt=%d)\n", cmd->cmdidx, cmd->resp_type,
 		host->clkrt);
-	if (cmd->resp_type & MMC_RSP_BUSY)
-		cmdat |= CMDAT_BUSY;
 
-	switch (cmd->resp_type) {
+	switch (cmd->resp_type & MMC_RSP_MASK) {
 	/* r1, r1b, r6, r7 */
-	case MMC_RSP_R1:
 	case MMC_RSP_R1b:
+		cmdat |= CMDAT_BUSY;
+	case MMC_RSP_R1:
 		cmdat |= CMDAT_RESP_SHORT;
 		break;
 	case MMC_RSP_R2:
-- 
1.7.5.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/6] mci: pxamci fix CMD12 handling

[Robert Jarzmik]

The pxamci requires a bit to be set in the command control
register when a CMD12 is sent. Set it, as required in the
PXA Developer Manuel, chapter "Stop Data Transmission
Command (CMD12 or IO/Abort with CMD52)".

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mci/pxamci.c |    3 +++
 drivers/mci/pxamci.h |    1 +
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/mci/pxamci.c b/drivers/mci/pxamci.c
index f251d75..c51fb77 100644
--- a/drivers/mci/pxamci.c
+++ b/drivers/mci/pxamci.c
@@ -188,6 +188,9 @@ static void pxamci_start_cmd(struct pxamci_host *host, struct mci_cmd *cmd,
 		break;
 	}
 
+	if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION)
+		cmdat |= CMDAT_STOP_TRAN;
+
 	mmc_writel(cmd->cmdidx, MMC_CMD);
 	mmc_writel(cmd->cmdarg >> 16, MMC_ARGH);
 	mmc_writel(cmd->cmdarg & 0xffff, MMC_ARGL);
diff --git a/drivers/mci/pxamci.h b/drivers/mci/pxamci.h
index 18d12a3..07dea45 100644
--- a/drivers/mci/pxamci.h
+++ b/drivers/mci/pxamci.h
@@ -40,6 +40,7 @@
 
 #define MMC_CMDAT			0x0010
 #define CMDAT_SDIO_INT_EN		(1 << 11)
+#define CMDAT_STOP_TRAN			(1 << 10)
 #define CMDAT_SD_4DAT			(1 << 8)
 #define CMDAT_DMAEN			(1 << 7)
 #define CMDAT_INIT			(1 << 6)
-- 
1.7.5.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 5/6] mci: pxamci fix R1b responses

[Robert Jarzmik]

The pxamci driver was not waiting for the BUSY line to be
deasserted. This was specifically breaking the CMD12 at
the end of block multiple writes, when the SD card had not
time enough to commit the last write.

Fix it by waiting for PRG_DONE bit (which is actually the
busy signal end condition).

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mci/pxamci.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mci/pxamci.c b/drivers/mci/pxamci.c
index c51fb77..9665196 100644
--- a/drivers/mci/pxamci.c
+++ b/drivers/mci/pxamci.c
@@ -239,13 +239,17 @@ static int pxamci_cmd_response(struct pxamci_host *host, struct mci_cmd *cmd)
 static int pxamci_mmccmd(struct pxamci_host *host, struct mci_cmd *cmd,
 			 struct mci_data *data, unsigned int cmddat)
 {
-	int ret = 0;
+	int ret = 0, stat_mask;
 	uint64_t start;
 
 	pxamci_start_cmd(host, cmd, cmddat);
+
+	stat_mask = STAT_END_CMD_RES;
+	if (cmd->resp_type & MMC_RSP_BUSY)
+		stat_mask |= STAT_PRG_DONE;
 	for (start = get_time_ns(), ret = -ETIMEDOUT;
 	     ret && !is_timeout(start, CMD_TIMEOUT);)
-		if (mmc_readl(MMC_STAT) & STAT_END_CMD_RES)
+		if ((mmc_readl(MMC_STAT) & stat_mask) == stat_mask)
 			ret = 0;
 
 	if (!ret && data)
-- 
1.7.5.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 6/6] mci: pxamci poweron ramp delay

[Robert Jarzmik]

As per MMC spec, once power has been applied to an SD card, the card
can take as much as 250ms to complete its power-up cycle, and become
responsive to CMD0.

When this delay was not in place, activating the SD card in the env
init failed sometimes. With it, no more failure are observed.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mci/pxamci.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/mci/pxamci.c b/drivers/mci/pxamci.c
index 9665196..027fa7b 100644
--- a/drivers/mci/pxamci.c
+++ b/drivers/mci/pxamci.c
@@ -42,6 +42,7 @@ static int pxamci_set_power(struct pxamci_host *host, int on)
 			       !!on ^ host->pdata->gpio_power_invert);
 	else if (host->pdata && host->pdata->setpower)
 		host->pdata->setpower(&host->mci, on);
+	mdelay(250);
 	return 0;
 }
 
-- 
1.7.5.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/6] mci: pxamci fixes

[Sascha Hauer]

Hi Robert,

On Mon, Apr 16, 2012 at 09:47:12PM +0200, Robert Jarzmik wrote:
> This serie was triggered by several tests to copy files from
> one directory to another on 2 SD cards :
>  - one slow 256MB card
>  - one fast 8G Transcend 8X card
> 
> The fixes improve :
>  - writes which do no longer fail (mostly because of R1b and
>  CMD12 handling)
>  - initial mounting of SD card from env (poweron ramp delay)
> 
> Happy review.
> 

The patches look good. I applied them all.

Thanks
 Sascha

> Robert Jarzmik (6):
>   mci: pxamci define timeouts
>   mci: pxamci change clocks handling
>   mci: pxamci fix response type
>   mci: pxamci fix CMD12 handling
>   mci: pxamci fix R1b responses
>   mci: pxamci poweron ramp delay
> 
>  drivers/mci/pxamci.c |   49 +++++++++++++++++++++++++++++++------------------
>  drivers/mci/pxamci.h |    1 +
>  2 files changed, 32 insertions(+), 18 deletions(-)
> 
> -- 
> 1.7.5.4
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox