diff mbox

[U-Boot,2/2] mmc: Split device init to decouple OCR-polling delay

Message ID 1350789360-27032-2-git-send-email-sjg@chromium.org
State Superseded, archived
Delegated to: Andy Fleming
Headers show

Commit Message

Simon Glass Oct. 21, 2012, 3:16 a.m. UTC 
From: Che-Liang Chiou <clchiou@chromium.org>

Most of time that MMC driver spends on initializing a device is polling
OCR (operation conditions register).  To decouple this polling loop,
device init is split into two parts: The first part fires the OCR query
command, and the second part polls the result.  So the caller is now no
longer bound to the OCR-polling delay; he may fire the query, go
somewhere and then come back later for the result.

To use this, call mmc_set_preinit() on any device which needs this.

This can save significant amounts of time on boot (e.g. 200ms) by
hiding the MMC init time behind other init.

Signed-off-by: Che-Liang Chiou <clchiou@chromium.org>
Signed-off-by: Simon Glass <sjg@chromium.org>
---
 drivers/mmc/mmc.c |  135 ++++++++++++++++++++++++++++++++++++++++------------
 include/mmc.h     |   30 ++++++++++++
 2 files changed, 134 insertions(+), 31 deletions(-)

Comments

Mela Custodio Oct. 21, 2012, 8:12 a.m. UTC | #1 
Hi All,

On 2012.10/20, Simon Glass wrote:
> From: Che-Liang Chiou <clchiou@chromium.org>
> 
> Most of time that MMC driver spends on initializing a device is polling
> OCR (operation conditions register).  To decouple this polling loop,
> device init is split into two parts: The first part fires the OCR query
> command, and the second part polls the result.  So the caller is now no
> longer bound to the OCR-polling delay; he may fire the query, go
> somewhere and then come back later for the result.
> 
> To use this, call mmc_set_preinit() on any device which needs this.
> 
> This can save significant amounts of time on boot (e.g. 200ms) by
> hiding the MMC init time behind other init.
> 

Please note that this patch has a conflict with the patch from Kim Phillips'
[U-Boot,28/32] drivers/mmc/mmc.c: sparse fixes (191937 in patchworks) 

I had to apply this patch first before patching Kim's modifications which
succeeds with the hunk offsets adjusted. It builds OK with the eldk 5.2.1
for powerpc. Will test these on an ml507 when I have time.

Regards,
Rommel
Simon Glass Oct. 25, 2012, 7:27 p.m. UTC | #2 
Hi,

On Sun, Oct 21, 2012 at 1:12 AM, RgC <sessyargc@gmail.com> wrote:
> Hi All,
>
> On 2012.10/20, Simon Glass wrote:
>> From: Che-Liang Chiou <clchiou@chromium.org>
>>
>> Most of time that MMC driver spends on initializing a device is polling
>> OCR (operation conditions register).  To decouple this polling loop,
>> device init is split into two parts: The first part fires the OCR query
>> command, and the second part polls the result.  So the caller is now no
>> longer bound to the OCR-polling delay; he may fire the query, go
>> somewhere and then come back later for the result.
>>
>> To use this, call mmc_set_preinit() on any device which needs this.
>>
>> This can save significant amounts of time on boot (e.g. 200ms) by
>> hiding the MMC init time behind other init.
>>
>
> Please note that this patch has a conflict with the patch from Kim Phillips'
> [U-Boot,28/32] drivers/mmc/mmc.c: sparse fixes (191937 in patchworks)
>
> I had to apply this patch first before patching Kim's modifications which
> succeeds with the hunk offsets adjusted. It builds OK with the eldk 5.2.1
> for powerpc. Will test these on an ml507 when I have time.

Yes I found the same. In fact Kim's patches don't apply to master for me.

co upstream/master
Note: checking out 'upstream/master'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b new_branch_name

HEAD is now at 186fc4d... ColdFire: Add Freescale MCF54418TWR ColdFire
development board support
 ~/u> patch -p1 <~/Downloads/U-Boot-28-32-drivers-mmc-mmc.c-sparse-fixes.patch
patching file drivers/mmc/mmc.c
Hunk #1 succeeded at 47 with fuzz 2 (offset -87 lines).
Hunk #2 succeeded at 109 (offset -92 lines).
Hunk #3 succeeded at 153 (offset -92 lines).
Hunk #4 succeeded at 346 (offset -92 lines).
Hunk #5 succeeded at 417 (offset -92 lines).
Hunk #6 succeeded at 438 (offset -92 lines).
Hunk #7 succeeded at 503 (offset -92 lines).
Hunk #8 succeeded at 567 (offset -92 lines).
Hunk #9 succeeded at 589 (offset -92 lines).
Hunk #10 succeeded at 611 (offset -92 lines).
Hunk #11 succeeded at 681 (offset -92 lines).
Hunk #12 succeeded at 702 (offset -92 lines).
Hunk #13 succeeded at 841 (offset -92 lines).
Hunk #14 succeeded at 859 (offset -92 lines).
Hunk #15 succeeded at 1014 (offset -92 lines).
Hunk #16 succeeded at 1149 (offset -92 lines).

Regards,
Simon

>
> Regards,
> Rommel
>
Andy Fleming Nov. 27, 2012, 11:33 p.m. UTC | #3 
On Sat, Oct 20, 2012 at 10:16 PM, Simon Glass <sjg@chromium.org> wrote:

> From: Che-Liang Chiou <clchiou@chromium.org>
>
> Most of time that MMC driver spends on initializing a device is polling
> OCR (operation conditions register).  To decouple this polling loop,
> device init is split into two parts: The first part fires the OCR query
> command, and the second part polls the result.  So the caller is now no
> longer bound to the OCR-polling delay; he may fire the query, go
> somewhere and then come back later for the result.
>
> To use this, call mmc_set_preinit() on any device which needs this.
>
> This can save significant amounts of time on boot (e.g. 200ms) by
> hiding the MMC init time behind other init.
>
> Signed-off-by: Che-Liang Chiou <clchiou@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
>
>

Could you rebase this on my latest tree? It failed to apply for reasons
which weren't immediately clear to me, and it's a non-trivial chunk that
failed.

Thanks,
Andy
diff mbox

Patch

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 18a037d..d397326 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -594,48 +594,70 @@  sd_send_op_cond(struct mmc *mmc)
 	return 0;
 }
 
+/* We pass in the cmd since otherwise the init seems to fail */
+static int mmc_send_op_cond_iter(struct mmc *mmc, struct mmc_cmd *cmd,
+		int use_arg)
+{
+	int err;
+
+	cmd->cmdidx = MMC_CMD_SEND_OP_COND;
+	cmd->resp_type = MMC_RSP_R3;
+	cmd->cmdarg = 0;
+	if (use_arg && !mmc_host_is_spi(mmc)) {
+		cmd->cmdarg =
+			(mmc->voltages &
+			(mmc->op_cond_response & OCR_VOLTAGE_MASK)) |
+			(mmc->op_cond_response & OCR_ACCESS_MODE);
+
+		if (mmc->host_caps & MMC_MODE_HC)
+			cmd->cmdarg |= OCR_HCS;
+	}
+	err = mmc_send_cmd(mmc, cmd, NULL);
+	if (err)
+		return err;
+	mmc->op_cond_response = cmd->response[0];
+	return 0;
+}
+
 int mmc_send_op_cond(struct mmc *mmc)
 {
-	int timeout = 10000;
 	struct mmc_cmd cmd;
-	int err;
+	int err, i;
 
 	/* Some cards seem to need this */
 	mmc_go_idle(mmc);
 
  	/* Asking to the card its capabilities */
- 	cmd.cmdidx = MMC_CMD_SEND_OP_COND;
- 	cmd.resp_type = MMC_RSP_R3;
- 	cmd.cmdarg = 0;
-
- 	err = mmc_send_cmd(mmc, &cmd, NULL);
+	mmc->op_cond_pending = 1;
+	for (i = 0; i < 2; i++) {
+		err = mmc_send_op_cond_iter(mmc, &cmd, i != 0);
+		if (err)
+			return err;
 
- 	if (err)
- 		return err;
+		/* exit if not busy (flag seems to be inverted) */
+		if (mmc->op_cond_response & OCR_BUSY)
+			return 0;
+	}
+	return IN_PROGRESS;
+}
 
- 	udelay(1000);
+int mmc_complete_op_cond(struct mmc *mmc)
+{
+	struct mmc_cmd cmd;
+	int timeout = 1000;
+	uint start;
+	int err;
 
+	mmc->op_cond_pending = 0;
+	start = get_timer(0);
 	do {
-		cmd.cmdidx = MMC_CMD_SEND_OP_COND;
-		cmd.resp_type = MMC_RSP_R3;
-		cmd.cmdarg = (mmc_host_is_spi(mmc) ? 0 :
-				(mmc->voltages &
-				(cmd.response[0] & OCR_VOLTAGE_MASK)) |
-				(cmd.response[0] & OCR_ACCESS_MODE));
-
-		if (mmc->host_caps & MMC_MODE_HC)
-			cmd.cmdarg |= OCR_HCS;
-
-		err = mmc_send_cmd(mmc, &cmd, NULL);
-
+		err = mmc_send_op_cond_iter(mmc, &cmd, 1);
 		if (err)
 			return err;
-
-		udelay(1000);
-	} while (!(cmd.response[0] & OCR_BUSY) && timeout--);
-
-	if (timeout <= 0)
-		return UNUSABLE_ERR;
+		if (get_timer(start) > timeout)
+			return UNUSABLE_ERR;
+		udelay(100);
+	} while (!(mmc->op_cond_response & OCR_BUSY));
 
 	if (mmc_host_is_spi(mmc)) { /* read OCR for spi */
 		cmd.cmdidx = MMC_CMD_SPI_READ_OCR;
@@ -1295,7 +1317,7 @@  block_dev_desc_t *mmc_get_dev(int dev)
 }
 #endif
 
-int mmc_init(struct mmc *mmc)
+int mmc_start_init(struct mmc *mmc)
 {
 	int err;
 
@@ -1335,17 +1357,48 @@  int mmc_init(struct mmc *mmc)
 	if (err == TIMEOUT) {
 		err = mmc_send_op_cond(mmc);
 
-		if (err) {
+		if (err && err != IN_PROGRESS) {
 			printf("Card did not respond to voltage select!\n");
 			return UNUSABLE_ERR;
 		}
 	}
 
-	err = mmc_startup(mmc);
+	if (err == IN_PROGRESS)
+		mmc->init_in_progress = 1;
+
+	return err;
+}
+
+static int mmc_complete_init(struct mmc *mmc)
+{
+	int err = 0;
+
+	if (mmc->op_cond_pending)
+		err = mmc_complete_op_cond(mmc);
+
+	if (!err)
+		err = mmc_startup(mmc);
 	if (err)
 		mmc->has_init = 0;
 	else
 		mmc->has_init = 1;
+	mmc->init_in_progress = 0;
+	return err;
+}
+
+int mmc_init(struct mmc *mmc)
+{
+	int err = IN_PROGRESS;
+	unsigned start = get_timer(0);
+
+	if (mmc->has_init)
+		return 0;
+	if (!mmc->init_in_progress)
+		err = mmc_start_init(mmc);
+
+	if (!err || err == IN_PROGRESS)
+		err = mmc_complete_init(mmc);
+	debug("%s: %d, time %lu\n", __func__, err, get_timer(start));
 	return err;
 }
 
@@ -1383,6 +1436,25 @@  int get_mmc_num(void)
 	return cur_dev_num;
 }
 
+void mmc_set_preinit(struct mmc *mmc, int preinit)
+{
+	mmc->preinit = preinit;
+}
+
+static void do_preinit(void)
+{
+	struct mmc *m;
+	struct list_head *entry;
+
+	list_for_each(entry, &mmc_devices) {
+		m = list_entry(entry, struct mmc, link);
+
+		if (m->preinit)
+			mmc_start_init(m);
+	}
+}
+
+
 int mmc_initialize(bd_t *bis)
 {
 	INIT_LIST_HEAD (&mmc_devices);
@@ -1393,5 +1465,6 @@  int mmc_initialize(bd_t *bis)
 
 	print_mmc_devices(',');
 
+	do_preinit();
 	return 0;
 }
diff --git a/include/mmc.h b/include/mmc.h
index a13e2bd..445d714 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -62,6 +62,7 @@ 
 #define UNUSABLE_ERR		-17 /* Unusable Card */
 #define COMM_ERR		-18 /* Communications Error */
 #define TIMEOUT			-19
+#define IN_PROGRESS		-20 /* operation is in progress */
 
 #define MMC_CMD_GO_IDLE_STATE		0
 #define MMC_CMD_SEND_OP_COND		1
@@ -260,6 +261,10 @@  struct mmc {
 	int (*init)(struct mmc *mmc);
 	int (*getcd)(struct mmc *mmc);
 	uint b_max;
+	char op_cond_pending;	/* 1 if we are waiting on an op_cond command */
+	char init_in_progress;	/* 1 if we have done mmc_start_init() */
+	char preinit;		/* start init as early as possible */
+	uint op_cond_response;	/* the response byte from the last op_cond */
 };
 
 int mmc_register(struct mmc *mmc);
@@ -276,6 +281,31 @@  int mmc_switch_part(int dev_num, unsigned int part_num);
 int mmc_getcd(struct mmc *mmc);
 void spl_mmc_load(void) __noreturn;
 
+/**
+ * Start device initialization and return immediately; it does not block on
+ * polling OCR (operation condition register) status.  Then you should call
+ * mmc_init, which would block on polling OCR status and complete the device
+ * initializatin.
+ *
+ * @param mmc	Pointer to a MMC device struct
+ * @return 0 on success, IN_PROGRESS on waiting for OCR status, <0 on error.
+ */
+int mmc_start_init(struct mmc *mmc);
+
+/**
+ * Set preinit flag of mmc device.
+ *
+ * This will cause the device to be pre-inited during mmc_initialize(),
+ * which may save boot time if the device is not accessed until later.
+ * Some eMMC devices take 200-300ms to init, but unfortunately they
+ * must be sent a series of commands to even get them to start preparing
+ * for operation.
+ *
+ * @param mmc		Pointer to a MMC device struct
+ * @param preinit	preinit flag value
+ */
+void mmc_set_preinit(struct mmc *mmc, int preinit);
+
 #ifdef CONFIG_GENERIC_MMC
 #define mmc_host_is_spi(mmc)	((mmc)->host_caps & MMC_MODE_SPI)
 struct mmc *mmc_spi_init(uint bus, uint cs, uint speed, uint mode);