[U-Boot] [PATCH/RFC] gen_atmel_mci: add mci_set_data_timeout()

[U-Boot] [PATCH/RFC] gen_atmel_mci: add mci_set_data_timeout()

[Andreas Bießmann]

Before the DTOR register is set to a fixed value and resulted in some cards
not working. Setting the fixed value to a hihger value is not appropriate
cause we could wait way to long for slow clock rates.

This patch moves the mci_set_data_timeout() from old atmel_mci driver to
gen_atmel_mci driver and adopts to the parameters. In contrast to the origin
this version of mci_set_data_timeout() relies on some fixed input values for
timeout_ns and timeout_clks. Before these values where taken from the card's
CSD.

Signed-off-by: Andreas Bie?mann <biessmann@corscience.de>
CC: Sven Schnelle <svens@stackframe.org>
CC: Reinhard Meyer <u-boot@emk-elektronik.de>
CC: Andy Fleming <afleming@gmail.com>
---
RESENT TO LIST ...

This is an RFC. The most questionary thing is whether we use fixed values for
timeout_ns/timeout_clks or take the values from CSD as before.

I wonder if we should add the taac and nsac values to the mmc struct or if we
should handle the mmc->csd[] inside the driver if we requiore the card data as
input for the timeout equtation.

Please read also http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/112056/focus=112057

total: 0 errors, 0 warnings, 74 lines checked

0001-gen_atmel_mci-add-mci_set_data_timeout.patch has no obvious style problems and is ready for submission.


 drivers/mmc/gen_atmel_mci.c |   56 +++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c
index f346b24..9f9ea38 100644
--- a/drivers/mmc/gen_atmel_mci.c
+++ b/drivers/mmc/gen_atmel_mci.c
@@ -62,6 +62,58 @@ static void dump_cmd(u32 cmdr, u32 arg, u32 status, const char* msg)
 		cmdr, cmdr&0x3F, arg, status, msg);
 }
 
+static void mci_set_data_timeout(struct mmc *mmc)
+{
+	atmel_mci_t *mci = (atmel_mci_t *)mmc->priv;
+
+	static const unsigned int dtomul_to_shift[] = {
+		0, 4, 7, 8, 10, 12, 16, 20,
+	};
+
+	unsigned int timeout_ns, timeout_clks;
+	unsigned int dtocyc, dtomul;
+	unsigned int shift;
+	u32 dtor;
+
+	/* we assume 1.5 ms data-read-access-time-1 (taac -> see CSD spec)
+	 * and 0 clock data-read-access-time-2 (nsac -> see CSD spec)
+	 */
+	timeout_ns = 1500000;
+	timeout_clks = 0;
+
+	printf("gen_atmel_mci: timeout_ns = %u\n", timeout_ns);
+
+	timeout_clks += (((timeout_ns + 9) / 10)
+			 * ((mmc->clock + 99999) / 100000) + 9999) / 10000;
+	if (!IS_SD(mmc))
+		timeout_clks *= 10;
+	else
+		timeout_clks *= 100;
+	printf("gen_atmel_mci: timeout_clks = %u\n", timeout_clks);
+
+	dtocyc = timeout_clks;
+	dtomul = 0;
+	shift = 0;
+	while (dtocyc > 15 && dtomul < 8) {
+		dtomul++;
+		shift = dtomul_to_shift[dtomul];
+		dtocyc = (timeout_clks + (1 << shift) - 1) >> shift;
+	}
+
+	if (dtomul >= 8) {
+		dtomul = 7;
+		dtocyc = 15;
+		puts("Warning: Using maximum data timeout\n");
+	}
+
+	dtor = (MMCI_BF(DTOMUL, dtomul)
+		| MMCI_BF(DTOCYC, dtocyc));
+	writel(dtor, &mci->dtor);
+
+	printf("mci: Using %u cycles data timeout (DTOR=0x%x)\n",
+	       dtocyc << shift, dtor);
+}
+
 /* Setup for MCI Clock and Block Size */
 static void mci_set_mode(struct mmc *mmc, u32 hz, u32 blklen)
 {
@@ -81,6 +133,8 @@ static void mci_set_mode(struct mmc *mmc, u32 hz, u32 blklen)
 	printf("mci: setting clock %u Hz, block size %u\n",
 		(bus_hz / (clkdiv+1)) / 2, blklen);
 
+	mci_set_data_timeout(mmc);
+
 	blklen &= 0xfffc;
 	/* On some platforms RDPROOF and WRPROOF are ignored */
 	writel((MMCI_BF(CLKDIV, clkdiv)
@@ -310,8 +364,6 @@ static int mci_init(struct mmc *mmc)
 	writel(MMCI_BIT(MCIEN), &mci->cr);	/* enable mci */
 	writel(MMCI_BF(SCDSEL, MCI_BUS), &mci->sdcr);	/* select port */
 
-	/* Initial Time-outs */
-	writel(0x5f, &mci->dtor);
 	/* Disable Interrupts */
 	writel(~0UL, &mci->idr);
 
-- 
1.7.6.3

[U-Boot] [PATCH/RFC] gen_atmel_mci: add mci_set_data_timeout()

[Andreas Bießmann]

Dear Andy Fleming,

Am 14.10.2011 14:36, schrieb Andreas Bie?mann:
> Before the DTOR register is set to a fixed value and resulted in some cards
> not working. Setting the fixed value to a hihger value is not appropriate
> cause we could wait way to long for slow clock rates.
> 
> This patch moves the mci_set_data_timeout() from old atmel_mci driver to
> gen_atmel_mci driver and adopts to the parameters. In contrast to the origin
> this version of mci_set_data_timeout() relies on some fixed input values for
> timeout_ns and timeout_clks. Before these values where taken from the card's
> CSD.
> 
> Signed-off-by: Andreas Bie?mann <biessmann@corscience.de>
> CC: Sven Schnelle <svens@stackframe.org>
> CC: Reinhard Meyer <u-boot@emk-elektronik.de>
> CC: Andy Fleming <afleming@gmail.com>
> ---
> RESENT TO LIST ...
> 
> This is an RFC. The most questionary thing is whether we use fixed values for
> timeout_ns/timeout_clks or take the values from CSD as before.
> 
> I wonder if we should add the taac and nsac values to the mmc struct or if we
> should handle the mmc->csd[] inside the driver if we requiore the card data as
> input for the timeout equtation.
> 
> Please read also http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/112056/focus=112057

This is the patch in question (see discussion @
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/119611/focus=119626)

Is it OK for you to use the fixed values for timeout_ns/timeout_clks
here or should we
 a) use the mmc->csd[] values in the respective driver?
or
 b) introduce some generic handling for mmc->csd[]?

best regards

Andreas Bie?mann