Re: SET_BLOCK_COUNT-bounded multiblock transfers

Re: SET_BLOCK_COUNT-bounded multiblock transfers

[Andrei Warkentin]

On Wed, Apr 13, 2011 at 7:38 PM, Andrei Warkentin <andreiw@motorola.com> wrote:
> This is a request for comments for a patch set that enables
> predefined multiblock transfers if these are supported.
>
> Before this patch set, all multiblock transfers look like (for example)
>
> CMD25 -> [block] [block] [block] [block] -> CMD12 (stop)
>
> ...or for controllers with Auto-CMD12
>
> CMD25 -> [block] [block] [block] [block] (host sends CMD12 itself).
>
> We want to enable -
>
> CMD23(4 blocks) CMD25 [data] [data] [data] [data]
>
> ...and for controllers with Auto-CMD23 -
>
> (Host sends CMD23(4 blocks)) CMD25 [data] [data] [data] [data]
>
> The interrupt overhead is the same, but for cards that optimize for predefined transfers
> we can see better transfer rates. I've tested this on a Sandisk eMMC where I saw as good as
> a 50% improvement on writes, and on a Toshiba eMMC where I saw no improvement at all. Also,
> reliable writes use CMD23, so this cleans up that code path as well. Note, that if a transfer
> fails, CMD12 must still be sent, so it is not sufficient to just not fill the mrq->stop field
> while doing a CMD23-enabled transfer. Due to this error handling and off-loads of Auto-CMD23
> (and interaction with Auto-CMD12) handling of SET_BLOCK_COUNT, just like STOP, is left to the host driver.
> Host driver now exposes MMC_CAP_CMD23 if it has been changed to handle the new "sbc" field in struct mmc_request.
> If a host doesn't expose this capability, open-ended transfers are used, and all functionality
> relying on CMD23 (such as reliable writes) is disabled.
>
> The third patch has been only tested on SD cards that don't expose CMD23. Still need to get an SDXC
> card and test it out, but I wanted to get eyes on this patch set anyway :-).
>
> I had a fourth patch enabling Auto-CMD23 for SDHCI, but I'll hold off until I can verify it.
>
> Thoughts?
>
> Table of Contents:
> [RFC 1/3] MMC: use CMD23 for multiblock transfers when we can.
> [RFC 2/3] MMC: Implement MMC_CAP_CMD23 for SDHCI.
> [RFC 3/3] MMC: Block CMD23 support for UHS104/SDXC cards.
>
> A
>

+ Yunpeng

SET_BLOCK_COUNT-bounded multiblock transfers

[Andrei Warkentin]

This is a request for comments for a patch set that enables
predefined multiblock transfers if these are supported.

Before this patch set, all multiblock transfers look like (for example)

CMD25 -> [block] [block] [block] [block] -> CMD12 (stop)

...or for controllers with Auto-CMD12

CMD25 -> [block] [block] [block] [block] (host sends CMD12 itself).

We want to enable - 

CMD23(4 blocks) CMD25 [data] [data] [data] [data]

...and for controllers with Auto-CMD23 -

(Host sends CMD23(4 blocks)) CMD25 [data] [data] [data] [data]

The interrupt overhead is the same, but for cards that optimize for predefined transfers
we can see better transfer rates. I've tested this on a Sandisk eMMC where I saw as good as
a 50% improvement on writes, and on a Toshiba eMMC where I saw no improvement at all. Also,
reliable writes use CMD23, so this cleans up that code path as well. Note, that if a transfer
fails, CMD12 must still be sent, so it is not sufficient to just not fill the mrq->stop field
while doing a CMD23-enabled transfer. Due to this error handling and off-loads of Auto-CMD23
(and interaction with Auto-CMD12) handling of SET_BLOCK_COUNT, just like STOP, is left to the host driver.
Host driver now exposes MMC_CAP_CMD23 if it has been changed to handle the new "sbc" field in struct mmc_request.
If a host doesn't expose this capability, open-ended transfers are used, and all functionality
relying on CMD23 (such as reliable writes) is disabled.

The third patch has been only tested on SD cards that don't expose CMD23. Still need to get an SDXC
card and test it out, but I wanted to get eyes on this patch set anyway :-).

I had a fourth patch enabling Auto-CMD23 for SDHCI, but I'll hold off until I can verify it.

Thoughts?

Table of Contents:
[RFC 1/3] MMC: use CMD23 for multiblock transfers when we can.
[RFC 2/3] MMC: Implement MMC_CAP_CMD23 for SDHCI.
[RFC 3/3] MMC: Block CMD23 support for UHS104/SDXC cards.

A

[RFC 1/3] MMC: use CMD23 for multiblock transfers when we can.

[Andrei Warkentin]

CMD23-prefixed instead of open-ended multiblock transfers
have a performance advantage on some MMC cards.

Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/card/block.c |   89 ++++++++++++++++++++++++++++++---------------
 include/linux/mmc/core.h |    1 +
 include/linux/mmc/host.h |    6 +++
 include/linux/mmc/mmc.h  |    6 +++
 4 files changed, 72 insertions(+), 30 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index c209953..282e6ef 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -55,10 +55,6 @@ MODULE_ALIAS("mmc:block");
 #define INAND_CMD38_ARG_SECTRIM1 0x81
 #define INAND_CMD38_ARG_SECTRIM2 0x88
 
-#define REL_WRITES_SUPPORTED(card) (mmc_card_mmc((card)) &&	\
-    (((card)->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||	\
-     ((card)->ext_csd.rel_sectors)))
-
 static DEFINE_MUTEX(block_mutex);
 
 /*
@@ -85,6 +81,10 @@ struct mmc_blk_data {
 	struct mmc_queue queue;
 	struct list_head part;
 
+	unsigned int	flags;
+#define MMC_BLK_CMD23	(1 << 0)	/* Can do SET_BLOCK_COUNT for multiblock */
+#define MMC_BLK_REL_WR	(1 << 1)	/* MMC Reliable write support */
+
 	unsigned int	usage;
 	unsigned int	read_only;
 	unsigned int	part_type;
@@ -225,6 +225,7 @@ static const struct block_device_operations mmc_bdops = {
 
 struct mmc_blk_request {
 	struct mmc_request	mrq;
+	struct mmc_command	sbc;
 	struct mmc_command	cmd;
 	struct mmc_command	stop;
 	struct mmc_data		data;
@@ -455,13 +456,10 @@ static int mmc_blk_issue_flush(struct mmc_queue *mq, struct request *req)
  * reliable write can handle, thus finish the request in
  * partial completions.
  */
-static inline int mmc_apply_rel_rw(struct mmc_blk_request *brq,
-				   struct mmc_card *card,
-				   struct request *req)
+static inline void mmc_apply_rel_rw(struct mmc_blk_request *brq,
+				    struct mmc_card *card,
+				    struct request *req)
 {
-	int err;
-	struct mmc_command set_count;
-
 	if (!(card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN)) {
 		/* Legacy mode imposes restrictions on transfers. */
 		if (!IS_ALIGNED(brq->cmd.arg, card->ext_csd.rel_sectors))
@@ -472,16 +470,6 @@ static inline int mmc_apply_rel_rw(struct mmc_blk_request *brq,
 		else if (brq->data.blocks < card->ext_csd.rel_sectors)
 			brq->data.blocks = 1;
 	}
-
-	memset(&set_count, 0, sizeof(struct mmc_command));
-	set_count.opcode = MMC_SET_BLOCK_COUNT;
-	set_count.arg = brq->data.blocks | (1 << 31);
-	set_count.flags = MMC_RSP_R1 | MMC_CMD_AC;
-	err = mmc_wait_for_cmd(card->host, &set_count, 0);
-	if (err)
-		printk(KERN_ERR "%s: error %d SET_BLOCK_COUNT\n",
-		       req->rq_disk->disk_name, err);
-	return err;
 }
 
 static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
@@ -498,7 +486,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 	bool do_rel_wr = ((req->cmd_flags & REQ_FUA) ||
 			  (req->cmd_flags & REQ_META)) &&
 		(rq_data_dir(req) == WRITE) &&
-		REL_WRITES_SUPPORTED(card);
+		(md->flags & MMC_BLK_REL_WR);
 
 	do {
 		struct mmc_command cmd;
@@ -536,11 +524,9 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 
 		if (brq.data.blocks > 1 || do_rel_wr) {
 			/* SPI multiblock writes terminate using a special
-			 * token, not a STOP_TRANSMISSION request. Reliable
-			 * writes use SET_BLOCK_COUNT and do not use a
-			 * STOP_TRANSMISSION request either.
+			 * token, not a STOP_TRANSMISSION request.
 			 */
-			if ((!mmc_host_is_spi(card->host) && !do_rel_wr) ||
+			if (!mmc_host_is_spi(card->host) ||
 			    rq_data_dir(req) == READ)
 				brq.mrq.stop = &brq.stop;
 			readcmd = MMC_READ_MULTIPLE_BLOCK;
@@ -558,8 +544,32 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 			brq.data.flags |= MMC_DATA_WRITE;
 		}
 
-		if (do_rel_wr && mmc_apply_rel_rw(&brq, card, req))
-			goto cmd_err;
+		if (do_rel_wr)
+			mmc_apply_rel_rw(&brq, card, req);
+
+		/*
+		 * Pre-defined multi-block transfers are preferable to
+		 * open ended-ones (and necessary for reliable writes).
+		 * However, it is not sufficient to just send CMD23,
+		 * and avoid the final CMD12, as on an error condition
+		 * CMD12 (stop) needs to be sent anyway. This, coupled
+		 * with Auto-CMD23 enhancements provided by some
+		 * hosts, means that the complexity of dealing
+		 * with this is best left to the host. If CMD23 is
+		 * supported by card and host, we'll fill sbc in and let
+		 * the host deal with handling it correctly. This means
+		 * that for hosts that don't expose MMC_CAP_CMD23, no
+		 * change of behavior will be observed.
+		 */
+
+		if ((md->flags & MMC_BLK_CMD23) &&
+		    mmc_op_multi(brq.cmd.opcode)) {
+			brq.sbc.opcode = MMC_SET_BLOCK_COUNT;
+			brq.sbc.arg = brq.data.blocks |
+				(do_rel_wr ? (1 << 31) : 0);
+			brq.sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
+			brq.mrq.sbc = &brq.sbc;
+		}
 
 		mmc_set_data_timeout(&brq.data, card);
 
@@ -596,7 +606,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 		 * until later as we need to wait for the card to leave
 		 * programming mode even when things go wrong.
 		 */
-		if (brq.cmd.error || brq.data.error || brq.stop.error) {
+		if (brq.sbc.error || brq.cmd.error ||
+		    brq.data.error || brq.stop.error) {
 			if (brq.data.blocks > 1 && rq_data_dir(req) == READ) {
 				/* Redo read one sector at a time */
 				printk(KERN_WARNING "%s: retrying using single "
@@ -607,6 +618,13 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 			status = get_card_status(card, req);
 		}
 
+		if (brq.sbc.error) {
+			printk(KERN_ERR "%s: error %d sending SET_BLOCK_COUNT "
+			       "command, response %#x, card status %#x\n",
+			       req->rq_disk->disk_name, brq.sbc.error,
+			       brq.sbc.resp[0], status);
+		}
+
 		if (brq.cmd.error) {
 			printk(KERN_ERR "%s: error %d sending read/write "
 			       "command, response %#x, card status %#x\n",
@@ -804,8 +822,6 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	md->disk->queue = md->queue.queue;
 	md->disk->driverfs_dev = parent;
 	set_disk_ro(md->disk, md->read_only || default_ro);
-	if (REL_WRITES_SUPPORTED(card))
-		blk_queue_flush(md->queue.queue, REQ_FLUSH | REQ_FUA);
 
 	/*
 	 * As discussed on lkml, GENHD_FL_REMOVABLE should:
@@ -829,6 +845,19 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 
 	blk_queue_logical_block_size(md->queue.queue, 512);
 	set_capacity(md->disk, size);
+
+	if (mmc_host_cmd23(card->host) &&
+	    mmc_card_mmc(card))
+		md->flags |= MMC_BLK_CMD23;
+
+	if (mmc_card_mmc(card) &&
+	    md->flags & MMC_BLK_CMD23 &&
+	    ((card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||
+	     card->ext_csd.rel_sectors)) {
+		md->flags |= MMC_BLK_REL_WR;
+		blk_queue_flush(md->queue.queue, REQ_FLUSH | REQ_FUA);
+	}
+
 	return md;
 
  err_putdisk:
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index f8e4bcb..55d7fde 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -120,6 +120,7 @@ struct mmc_data {
 };
 
 struct mmc_request {
+	struct mmc_command	*sbc;		/* SET_BLOCK_COUNT for multiblock */
 	struct mmc_command	*cmd;
 	struct mmc_data		*data;
 	struct mmc_command	*stop;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 4f705eb..ba34fc5 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -173,6 +173,7 @@ struct mmc_host {
 						/* DDR mode at 1.2V */
 #define MMC_CAP_POWER_OFF_CARD	(1 << 13)	/* Can power off after boot */
 #define MMC_CAP_BUS_WIDTH_TEST	(1 << 14)	/* CMD14/CMD19 bus width ok */
+#define MMC_CAP_CMD23		(1 << 15)	/* CMD23 supported */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 
@@ -330,5 +331,10 @@ static inline int mmc_card_wake_sdio_irq(struct mmc_host *host)
 {
 	return host->pm_flags & MMC_PM_WAKE_SDIO_IRQ;
 }
+
+static inline int mmc_host_cmd23(struct mmc_host *host)
+{
+	return host->caps & MMC_CAP_CMD23;
+}
 #endif
 
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 373b2bf..ea74168 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -82,6 +82,12 @@
 #define MMC_APP_CMD              55   /* ac   [31:16] RCA        R1  */
 #define MMC_GEN_CMD              56   /* adtc [0] RD/WR          R1  */
 
+static inline bool mmc_op_multi(u32 opcode)
+{
+	return opcode == MMC_WRITE_MULTIPLE_BLOCK ||
+		opcode == MMC_READ_MULTIPLE_BLOCK;
+}
+
 /*
  * MMC_SWITCH argument format:
  *
-- 
1.7.0.4

[RFC 2/3] MMC: Implement MMC_CAP_CMD23 for SDHCI.

[Andrei Warkentin]

Implements support for multiblock transfers bounded
by SET_BLOCK_COUNT (CMD23).

Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/host/sdhci.c |   60 +++++++++++++++++++++++++++++++++------------
 1 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 94793f2..0b71ce8 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -822,9 +822,10 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 }
 
 static void sdhci_set_transfer_mode(struct sdhci_host *host,
-	struct mmc_data *data)
+	struct mmc_command *cmd)
 {
 	u16 mode;
+	struct mmc_data *data = cmd->data;
 
 	if (data == NULL)
 		return;
@@ -832,11 +833,17 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
 	WARN_ON(!host->data);
 
 	mode = SDHCI_TRNS_BLK_CNT_EN;
-	if (data->blocks > 1) {
-		if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
-			mode |= SDHCI_TRNS_MULTI | SDHCI_TRNS_ACMD12;
-		else
-			mode |= SDHCI_TRNS_MULTI;
+	if (mmc_op_multi(cmd->opcode) ||
+	    data->blocks > 1) {
+		mode |= SDHCI_TRNS_MULTI;
+
+		/*
+		 * If we are sending CMD23, CMD12 never gets sent
+		 * on successful completion.
+		 */
+		if (!host->mrq->sbc &&
+		    host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
+			mode |= SDHCI_TRNS_ACMD12;
 	}
 	if (data->flags & MMC_DATA_READ)
 		mode |= SDHCI_TRNS_READ;
@@ -877,7 +884,15 @@ static void sdhci_finish_data(struct sdhci_host *host)
 	else
 		data->bytes_xfered = data->blksz * data->blocks;
 
-	if (data->stop) {
+	/*
+	 * Need to send CMD12 if -
+	 * a) open-ended multiblock transfer (no CMD23)
+	 * b) error in multiblock transfer
+	 */
+	if (data->stop &&
+	    (data->error ||
+	     !host->mrq->sbc)) {
+
 		/*
 		 * The controller needs a reset of internal state machines
 		 * upon error conditions.
@@ -933,7 +948,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 
 	sdhci_writel(host, cmd->arg, SDHCI_ARGUMENT);
 
-	sdhci_set_transfer_mode(host, cmd->data);
+	sdhci_set_transfer_mode(host, cmd);
 
 	if ((cmd->flags & MMC_RSP_136) && (cmd->flags & MMC_RSP_BUSY)) {
 		printk(KERN_ERR "%s: Unsupported response type!\n",
@@ -986,13 +1001,21 @@ static void sdhci_finish_command(struct sdhci_host *host)
 
 	host->cmd->error = 0;
 
-	if (host->data && host->data_early)
-		sdhci_finish_data(host);
+	/* Finished CMD23, now send actual command. */
+	if (host->cmd == host->mrq->sbc) {
+		host->cmd = NULL;
+		sdhci_send_command(host, host->mrq->cmd);
+	} else {
 
-	if (!host->cmd->data)
-		tasklet_schedule(&host->finish_tasklet);
+		/* Processed actual command. */
+		if (host->data && host->data_early)
+			sdhci_finish_data(host);
 
-	host->cmd = NULL;
+		if (!host->cmd->data)
+			tasklet_schedule(&host->finish_tasklet);
+
+		host->cmd = NULL;
+	}
 }
 
 static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
@@ -1159,8 +1182,13 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	if (!present || host->flags & SDHCI_DEVICE_DEAD) {
 		host->mrq->cmd->error = -ENOMEDIUM;
 		tasklet_schedule(&host->finish_tasklet);
-	} else
-		sdhci_send_command(host, mrq->cmd);
+	} else {
+
+		if (mrq->sbc)
+			sdhci_send_command(host, mrq->sbc);
+		else
+			sdhci_send_command(host, mrq->cmd);
+	}
 
 	mmiowb();
 	spin_unlock_irqrestore(&host->lock, flags);
@@ -1909,7 +1937,7 @@ int sdhci_add_host(struct sdhci_host *host)
 		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;
 
 	mmc->f_max = host->max_clk;
-	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE;
+	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;
 
 	/*
 	 * A controller may support 8-bit width, but the board itself
-- 
1.7.0.4

[RFC 3/3] MMC: Block CMD23 support for UHS104/SDXC cards.

[Andrei Warkentin]

SD cards operating at UHS104 or better support SET_BLOCK_COUNT.

Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/card/block.c |    9 ++++++---
 drivers/mmc/core/sd.c    |    1 +
 include/linux/mmc/card.h |    1 +
 include/linux/mmc/sd.h   |    1 +
 4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 282e6ef..6573e36 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -846,9 +846,12 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	blk_queue_logical_block_size(md->queue.queue, 512);
 	set_capacity(md->disk, size);
 
-	if (mmc_host_cmd23(card->host) &&
-	    mmc_card_mmc(card))
-		md->flags |= MMC_BLK_CMD23;
+	if (mmc_host_cmd23(card->host)) {
+		if (mmc_card_mmc(card) ||
+		    (mmc_card_sd(card) &&
+		     card->scr.cmds & SCR_CMDS_CMD23))
+			md->flags |= MMC_BLK_CMD23;
+	}
 
 	if (mmc_card_mmc(card) &&
 	    md->flags & MMC_BLK_CMD23 &&
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 6dac89f..16a1f10 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -195,6 +195,7 @@ static int mmc_decode_scr(struct mmc_card *card)
 	else
 		card->erased_byte = 0x0;
 
+	scr->cmds = UNSTUFF_BITS(resp, 32, 2);
 	return 0;
 }
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 72a9868..337a851 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -70,6 +70,7 @@ struct sd_scr {
 	unsigned char		bus_widths;
 #define SD_SCR_BUS_WIDTH_1	(1<<0)
 #define SD_SCR_BUS_WIDTH_4	(1<<2)
+	unsigned char		cmds;
 };
 
 struct sd_ssr {
diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h
index 3fd85e0..81ba37b 100644
--- a/include/linux/mmc/sd.h
+++ b/include/linux/mmc/sd.h
@@ -60,6 +60,7 @@
 #define SCR_SPEC_VER_0		0	/* Implements system specification 1.0 - 1.01 */
 #define SCR_SPEC_VER_1		1	/* Implements system specification 1.10 */
 #define SCR_SPEC_VER_2		2	/* Implements system specification 2.00 */
+#define SCR_CMDS_CMD23		(1 << 1) /* Supports CMD23 */
 
 /*
  * SD bus widths
-- 
1.7.0.4

RE: SET_BLOCK_COUNT-bounded multiblock transfers

[Gao, Yunpeng]

>-----Original Message-----
>From: linux-mmc-owner@vger.kernel.org
>[mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Andrei Warkentin
>Sent: Thursday, April 14, 2011 8:04 AM
>To: linux-mmc@vger.kernel.org
>Cc: arnd@arndb.de; cjb@laptop.org; Gao, Yunpeng
>Subject: Re: SET_BLOCK_COUNT-bounded multiblock transfers
>
>On Wed, Apr 13, 2011 at 7:38 PM, Andrei Warkentin
><andreiw@motorola.com> wrote:
>> This is a request for comments for a patch set that enables
>> predefined multiblock transfers if these are supported.
>>
>> Before this patch set, all multiblock transfers look like (for example)
>>
>> CMD25 -> [block] [block] [block] [block] -> CMD12 (stop)
>>
>> ...or for controllers with Auto-CMD12
>>
>> CMD25 -> [block] [block] [block] [block] (host sends CMD12 itself).
>>
>> We want to enable -
>>
>> CMD23(4 blocks) CMD25 [data] [data] [data] [data]
>>
>> ...and for controllers with Auto-CMD23 -
>>
>> (Host sends CMD23(4 blocks)) CMD25 [data] [data] [data] [data]
>>
>> The interrupt overhead is the same, but for cards that optimize for
>predefined transfers
>> we can see better transfer rates. I've tested this on a Sandisk eMMC where I
>saw as good as
>> a 50% improvement on writes, and on a Toshiba eMMC where I saw no
>improvement at all. Also,
>> reliable writes use CMD23, so this cleans up that code path as well. Note,
>that if a transfer
>> fails, CMD12 must still be sent, so it is not sufficient to just not fill the
>mrq->stop field
>> while doing a CMD23-enabled transfer. Due to this error handling and
>off-loads of Auto-CMD23
>> (and interaction with Auto-CMD12) handling of SET_BLOCK_COUNT, just like
>STOP, is left to the host driver.
>> Host driver now exposes MMC_CAP_CMD23 if it has been changed to
>handle the new "sbc" field in struct mmc_request.
>> If a host doesn't expose this capability, open-ended transfers are used, and
>all functionality
>> relying on CMD23 (such as reliable writes) is disabled.
>>
>> The third patch has been only tested on SD cards that don't expose CMD23.
>Still need to get an SDXC
>> card and test it out, but I wanted to get eyes on this patch set anyway :-).
>>
>> I had a fourth patch enabling Auto-CMD23 for SDHCI, but I'll hold off until I
>can verify it.
>>
>> Thoughts?
>>
>> Table of Contents:
>> [RFC 1/3] MMC: use CMD23 for multiblock transfers when we can.
>> [RFC 2/3] MMC: Implement MMC_CAP_CMD23 for SDHCI.
>> [RFC 3/3] MMC: Block CMD23 support for UHS104/SDXC cards.
>>
>> A
>>
>
>+ Yunpeng

Thanks for add me in the loop. I'm fine with the 3 patches.

And just think that maybe it's not necessary to enable the Auto-CMD23 for SDHCI,
because I have an impression that some Silicon bugs related to Auto-CMD12/Auto-CMD23
existed in some vendor's SDHCI host controller IP, and I'm not sure they have been fixed or not by now.

Regards,
Yunpeng

Re: SET_BLOCK_COUNT-bounded multiblock transfers

[Andrei Warkentin]

On Thu, Apr 14, 2011 at 4:05 AM, Gao, Yunpeng <yunpeng.gao@intel.com> wrote:
>
> Thanks for add me in the loop. I'm fine with the 3 patches.
>
> And just think that maybe it's not necessary to enable the Auto-CMD23 for SDHCI,
> because I have an impression that some Silicon bugs related to Auto-CMD12/Auto-CMD23
> existed in some vendor's SDHCI host controller IP, and I'm not sure they have been fixed or not by now.
>

That's why I didn't send a patch for that :-). When I get some SDHCI
silicon with working Auto-CMD12/Auto-CMD23 I'll come back to that
code path.

I actually thought about this some more and I decided to decouple
CMD23 support and CMD23 use for multiblock writes. Will add a new
md->flags bit.
This will allow cards where for whatever reason CMD23-bounded
transfers are slower to use CMD12 instead, while retaining CMD23 use
for things like reliable writes and all those other features you've
mentioned that are in the still not public eMMC 4.5.

The question to Arnd and Chris - should by default use of CMD23 for
multiblock writes be disabled, and enabled per known-good card using
add_flag quirk support? Or should CMD23 for multiblock writes be
enabled by default, and then disabled for known-bad cards?

First approach is safer from the point of view of regressions. People
who upgrade to 2.6.40 won't suddenly see worse I/O performance if they
have some silicon bugs or if use of CMD23 nullifies some internal
optimizations for sequential writes. On the other hand, at some point
hopefully everybody fixes their silicon such that CMD23 always means
better performance.

Thoughts?
A

Re: SET_BLOCK_COUNT-bounded multiblock transfers

[Chris Ball]

Hi,

On Thu, Apr 14 2011, Andrei Warkentin wrote:
> The question to Arnd and Chris - should by default use of CMD23 for
> multiblock writes be disabled, and enabled per known-good card using
> add_flag quirk support? Or should CMD23 for multiblock writes be
> enabled by default, and then disabled for known-bad cards?

I don't have a standard answer for this -- it just depends on the numbers.

If we've seen a card that freaks out and loses significant bandwidth, that
would be a good reason to start with a whitelist.  If all the data we have
suggests that it's performance-wise either a win or a no-op, that's a fine
reason to push it to mmc-next with a blacklist approach and change our
minds later if our knowledge gets updated.  Does that help?

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: SET_BLOCK_COUNT-bounded multiblock transfers

[Andrei Warkentin]

On Thu, Apr 14, 2011 at 6:18 PM, Chris Ball <cjb@laptop.org> wrote:
>
> If we've seen a card that freaks out and loses significant bandwidth, that
> would be a good reason to start with a whitelist.  If all the data we have
> suggests that it's performance-wise either a win or a no-op, that's a fine
> reason to push it to mmc-next with a blacklist approach and change our
> minds later if our knowledge gets updated.  Does that help?
>

Yes, it does, thanks! Basically so far -

(a) amazing boost for Sandisk eMMCs
(b) synthetic tests showed no-op for Toshiba (a derivative of Arnd's
flashbench, I'll put it up on GitHub as soon as my hands get to it),
but more realistic test (via SQLite transactions on top of fs) showed
slight deterioration. I'm bringing it up with the vendor to get a
clear picture why and if they are doing anything to address it.
(c) I am going to try to dig up some discrete MMCs (hard beasts to
come by these days) and try with that.
(d) I am going to dig up a few SDXCs and try them out.

My current theory is that the CMD23 for multiblock should have no
effect on the really braindead cards, and could have deterioration on
cards that somehow optimize the CMD12 path (for example, the T eMMC
optimize consecutive multiblock writes).

At any rate, I'll decouple CMD23 support from multiblock CMD23 use...

Yunpeng, I'm curious, what vendors did you have in mind when you
suggested in your original email to use the CMD23 multiblock stuff?

Thanks again,
A

Re: SET_BLOCK_COUNT-bounded multiblock transfers

[Andrei Warkentin]

+Arindam (This is the patch set I mentioned)

On Fri, Apr 15, 2011 at 2:10 AM, Andrei Warkentin <andreiw@motorola.com> wrote:
> On Thu, Apr 14, 2011 at 6:18 PM, Chris Ball <cjb@laptop.org> wrote:
>>
>> If we've seen a card that freaks out and loses significant bandwidth, that
>> would be a good reason to start with a whitelist.  If all the data we have
>> suggests that it's performance-wise either a win or a no-op, that's a fine
>> reason to push it to mmc-next with a blacklist approach and change our
>> minds later if our knowledge gets updated.  Does that help?
>>
>
> Yes, it does, thanks! Basically so far -
>
> (a) amazing boost for Sandisk eMMCs
> (b) synthetic tests showed no-op for Toshiba (a derivative of Arnd's
> flashbench, I'll put it up on GitHub as soon as my hands get to it),
> but more realistic test (via SQLite transactions on top of fs) showed
> slight deterioration. I'm bringing it up with the vendor to get a
> clear picture why and if they are doing anything to address it.
> (c) I am going to try to dig up some discrete MMCs (hard beasts to
> come by these days) and try with that.
> (d) I am going to dig up a few SDXCs and try them out.
>
> My current theory is that the CMD23 for multiblock should have no
> effect on the really braindead cards, and could have deterioration on
> cards that somehow optimize the CMD12 path (for example, the T eMMC
> optimize consecutive multiblock writes).
>
> At any rate, I'll decouple CMD23 support from multiblock CMD23 use...
>
> Yunpeng, I'm curious, what vendors did you have in mind when you
> suggested in your original email to use the CMD23 multiblock stuff?
>
> Thanks again,
> A
>

[patchv2 1/3] MMC: Use CMD23 for multiblock transfers when we can.

[Andrei Warkentin]

CMD23-prefixed instead of open-ended multiblock transfers
have a performance advantage on some MMC cards.

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/card/block.c |  112 +++++++++++++++++++++++++++++++++------------
 include/linux/mmc/card.h |    1 +
 include/linux/mmc/core.h |    1 +
 include/linux/mmc/host.h |    6 +++
 include/linux/mmc/mmc.h  |    6 +++
 5 files changed, 96 insertions(+), 30 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index c209953..bba0888 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -55,10 +55,6 @@ MODULE_ALIAS("mmc:block");
 #define INAND_CMD38_ARG_SECTRIM1 0x81
 #define INAND_CMD38_ARG_SECTRIM2 0x88
 
-#define REL_WRITES_SUPPORTED(card) (mmc_card_mmc((card)) &&	\
-    (((card)->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||	\
-     ((card)->ext_csd.rel_sectors)))
-
 static DEFINE_MUTEX(block_mutex);
 
 /*
@@ -85,6 +81,10 @@ struct mmc_blk_data {
 	struct mmc_queue queue;
 	struct list_head part;
 
+	unsigned int	flags;
+#define MMC_BLK_CMD23	(1 << 0)	/* Can do SET_BLOCK_COUNT for multiblock */
+#define MMC_BLK_REL_WR	(1 << 1)	/* MMC Reliable write support */
+
 	unsigned int	usage;
 	unsigned int	read_only;
 	unsigned int	part_type;
@@ -225,6 +225,7 @@ static const struct block_device_operations mmc_bdops = {
 
 struct mmc_blk_request {
 	struct mmc_request	mrq;
+	struct mmc_command	sbc;
 	struct mmc_command	cmd;
 	struct mmc_command	stop;
 	struct mmc_data		data;
@@ -455,13 +456,10 @@ static int mmc_blk_issue_flush(struct mmc_queue *mq, struct request *req)
  * reliable write can handle, thus finish the request in
  * partial completions.
  */
-static inline int mmc_apply_rel_rw(struct mmc_blk_request *brq,
-				   struct mmc_card *card,
-				   struct request *req)
+static inline void mmc_apply_rel_rw(struct mmc_blk_request *brq,
+				    struct mmc_card *card,
+				    struct request *req)
 {
-	int err;
-	struct mmc_command set_count;
-
 	if (!(card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN)) {
 		/* Legacy mode imposes restrictions on transfers. */
 		if (!IS_ALIGNED(brq->cmd.arg, card->ext_csd.rel_sectors))
@@ -472,16 +470,6 @@ static inline int mmc_apply_rel_rw(struct mmc_blk_request *brq,
 		else if (brq->data.blocks < card->ext_csd.rel_sectors)
 			brq->data.blocks = 1;
 	}
-
-	memset(&set_count, 0, sizeof(struct mmc_command));
-	set_count.opcode = MMC_SET_BLOCK_COUNT;
-	set_count.arg = brq->data.blocks | (1 << 31);
-	set_count.flags = MMC_RSP_R1 | MMC_CMD_AC;
-	err = mmc_wait_for_cmd(card->host, &set_count, 0);
-	if (err)
-		printk(KERN_ERR "%s: error %d SET_BLOCK_COUNT\n",
-		       req->rq_disk->disk_name, err);
-	return err;
 }
 
 static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
@@ -498,7 +486,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 	bool do_rel_wr = ((req->cmd_flags & REQ_FUA) ||
 			  (req->cmd_flags & REQ_META)) &&
 		(rq_data_dir(req) == WRITE) &&
-		REL_WRITES_SUPPORTED(card);
+		(md->flags & MMC_BLK_REL_WR);
 
 	do {
 		struct mmc_command cmd;
@@ -536,11 +524,9 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 
 		if (brq.data.blocks > 1 || do_rel_wr) {
 			/* SPI multiblock writes terminate using a special
-			 * token, not a STOP_TRANSMISSION request. Reliable
-			 * writes use SET_BLOCK_COUNT and do not use a
-			 * STOP_TRANSMISSION request either.
+			 * token, not a STOP_TRANSMISSION request.
 			 */
-			if ((!mmc_host_is_spi(card->host) && !do_rel_wr) ||
+			if (!mmc_host_is_spi(card->host) ||
 			    rq_data_dir(req) == READ)
 				brq.mrq.stop = &brq.stop;
 			readcmd = MMC_READ_MULTIPLE_BLOCK;
@@ -558,8 +544,37 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 			brq.data.flags |= MMC_DATA_WRITE;
 		}
 
-		if (do_rel_wr && mmc_apply_rel_rw(&brq, card, req))
-			goto cmd_err;
+		if (do_rel_wr)
+			mmc_apply_rel_rw(&brq, card, req);
+
+		/*
+		 * Pre-defined multi-block transfers are preferable to
+		 * open ended-ones (and necessary for reliable writes).
+		 * However, it is not sufficient to just send CMD23,
+		 * and avoid the final CMD12, as on an error condition
+		 * CMD12 (stop) needs to be sent anyway. This, coupled
+		 * with Auto-CMD23 enhancements provided by some
+		 * hosts, means that the complexity of dealing
+		 * with this is best left to the host. If CMD23 is
+		 * supported by card and host, we'll fill sbc in and let
+		 * the host deal with handling it correctly. This means
+		 * that for hosts that don't expose MMC_CAP_CMD23, no
+		 * change of behavior will be observed.
+		 *
+		 * N.B: Some MMC cards experience perf degradation.
+		 * We'll avoid using CMD23-bounded multiblock writes for
+		 * these, while retaining features like reliable writes.
+		 */
+
+		if ((md->flags & MMC_BLK_CMD23) &&
+		    mmc_op_multi(brq.cmd.opcode) &&
+		    (do_rel_wr || !(card->quirks & MMC_QUIRK_BLK_NO_CMD23))) {
+			brq.sbc.opcode = MMC_SET_BLOCK_COUNT;
+			brq.sbc.arg = brq.data.blocks |
+				(do_rel_wr ? (1 << 31) : 0);
+			brq.sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
+			brq.mrq.sbc = &brq.sbc;
+		}
 
 		mmc_set_data_timeout(&brq.data, card);
 
@@ -596,7 +611,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 		 * until later as we need to wait for the card to leave
 		 * programming mode even when things go wrong.
 		 */
-		if (brq.cmd.error || brq.data.error || brq.stop.error) {
+		if (brq.sbc.error || brq.cmd.error ||
+		    brq.data.error || brq.stop.error) {
 			if (brq.data.blocks > 1 && rq_data_dir(req) == READ) {
 				/* Redo read one sector at a time */
 				printk(KERN_WARNING "%s: retrying using single "
@@ -607,6 +623,13 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 			status = get_card_status(card, req);
 		}
 
+		if (brq.sbc.error) {
+			printk(KERN_ERR "%s: error %d sending SET_BLOCK_COUNT "
+			       "command, response %#x, card status %#x\n",
+			       req->rq_disk->disk_name, brq.sbc.error,
+			       brq.sbc.resp[0], status);
+		}
+
 		if (brq.cmd.error) {
 			printk(KERN_ERR "%s: error %d sending read/write "
 			       "command, response %#x, card status %#x\n",
@@ -804,8 +827,6 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	md->disk->queue = md->queue.queue;
 	md->disk->driverfs_dev = parent;
 	set_disk_ro(md->disk, md->read_only || default_ro);
-	if (REL_WRITES_SUPPORTED(card))
-		blk_queue_flush(md->queue.queue, REQ_FLUSH | REQ_FUA);
 
 	/*
 	 * As discussed on lkml, GENHD_FL_REMOVABLE should:
@@ -829,6 +850,19 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 
 	blk_queue_logical_block_size(md->queue.queue, 512);
 	set_capacity(md->disk, size);
+
+	if (mmc_host_cmd23(card->host) &&
+	    mmc_card_mmc(card))
+		md->flags |= MMC_BLK_CMD23;
+
+	if (mmc_card_mmc(card) &&
+	    md->flags & MMC_BLK_CMD23 &&
+	    ((card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||
+	     card->ext_csd.rel_sectors)) {
+		md->flags |= MMC_BLK_REL_WR;
+		blk_queue_flush(md->queue.queue, REQ_FLUSH | REQ_FUA);
+	}
+
 	return md;
 
  err_putdisk:
@@ -982,6 +1016,24 @@ static const struct mmc_fixup blk_fixups[] =
 	MMC_FIXUP("SEM08G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
 	MMC_FIXUP("SEM16G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
 	MMC_FIXUP("SEM32G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
+
+	/*
+	 * Some MMC cards experience performance degradation with CMD23
+	 * instead of CMD12-bounded multiblock transfers. For now we'll
+	 * white list what's good.
+	 */
+	MMC_FIXUP(CID_NAME_ANY, CID_MANFID_ANY, CID_OEMID_ANY, add_quirk,
+		  MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("SEM02G", 0x2, 0x100, remove_quirk, MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("SEM04G", 0x2, 0x100, remove_quirk, MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("SEM08G", 0x2, 0x100, remove_quirk, MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("SEM16G", 0x2, 0x100, remove_quirk, MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("SEM32G", 0x2, 0x100, remove_quirk, MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("SEM02G", 0x45, 0x0, remove_quirk, MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("SEM04G", 0x45, 0x0, remove_quirk, MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("SEM08G", 0x45, 0x0, remove_quirk, MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("SEM16G", 0x45, 0x0, remove_quirk, MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("SEM32G", 0x45, 0x0, remove_quirk, MMC_QUIRK_BLK_NO_CMD23),
 	END_FIXUP
 };
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 72a9868..1e6bd6c 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -134,6 +134,7 @@ struct mmc_card {
 #define MMC_QUIRK_NONSTD_FUNC_IF (1<<4)		/* SDIO card has nonstd function interfaces */
 #define MMC_QUIRK_DISABLE_CD	(1<<5)		/* disconnect CD/DAT[3] resistor */
 #define MMC_QUIRK_INAND_CMD38	(1<<6)		/* iNAND devices have broken CMD38 */
+#define MMC_QUIRK_BLK_NO_CMD23	(1<<7)		/* Avoid CMD23 for regular multiblock */
 
 	unsigned int		erase_size;	/* erase size in sectors */
  	unsigned int		erase_shift;	/* if erase unit is power 2 */
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index f8e4bcb..55d7fde 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -120,6 +120,7 @@ struct mmc_data {
 };
 
 struct mmc_request {
+	struct mmc_command	*sbc;		/* SET_BLOCK_COUNT for multiblock */
 	struct mmc_command	*cmd;
 	struct mmc_data		*data;
 	struct mmc_command	*stop;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 4f705eb..ba34fc5 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -173,6 +173,7 @@ struct mmc_host {
 						/* DDR mode at 1.2V */
 #define MMC_CAP_POWER_OFF_CARD	(1 << 13)	/* Can power off after boot */
 #define MMC_CAP_BUS_WIDTH_TEST	(1 << 14)	/* CMD14/CMD19 bus width ok */
+#define MMC_CAP_CMD23		(1 << 15)	/* CMD23 supported */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 
@@ -330,5 +331,10 @@ static inline int mmc_card_wake_sdio_irq(struct mmc_host *host)
 {
 	return host->pm_flags & MMC_PM_WAKE_SDIO_IRQ;
 }
+
+static inline int mmc_host_cmd23(struct mmc_host *host)
+{
+	return host->caps & MMC_CAP_CMD23;
+}
 #endif
 
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 373b2bf..ea74168 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -82,6 +82,12 @@
 #define MMC_APP_CMD              55   /* ac   [31:16] RCA        R1  */
 #define MMC_GEN_CMD              56   /* adtc [0] RD/WR          R1  */
 
+static inline bool mmc_op_multi(u32 opcode)
+{
+	return opcode == MMC_WRITE_MULTIPLE_BLOCK ||
+		opcode == MMC_READ_MULTIPLE_BLOCK;
+}
+
 /*
  * MMC_SWITCH argument format:
  *
-- 
1.7.0.4

[patchv2 2/3] MMC: Implement MMC_CAP_CMD23 for SDHCI.

[Andrei Warkentin]

Implements support for multiblock transfers bounded
by SET_BLOCK_COUNT (CMD23).

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/host/sdhci.c |   60 +++++++++++++++++++++++++++++++++------------
 1 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 94793f2..0b71ce8 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -822,9 +822,10 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 }
 
 static void sdhci_set_transfer_mode(struct sdhci_host *host,
-	struct mmc_data *data)
+	struct mmc_command *cmd)
 {
 	u16 mode;
+	struct mmc_data *data = cmd->data;
 
 	if (data == NULL)
 		return;
@@ -832,11 +833,17 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
 	WARN_ON(!host->data);
 
 	mode = SDHCI_TRNS_BLK_CNT_EN;
-	if (data->blocks > 1) {
-		if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
-			mode |= SDHCI_TRNS_MULTI | SDHCI_TRNS_ACMD12;
-		else
-			mode |= SDHCI_TRNS_MULTI;
+	if (mmc_op_multi(cmd->opcode) ||
+	    data->blocks > 1) {
+		mode |= SDHCI_TRNS_MULTI;
+
+		/*
+		 * If we are sending CMD23, CMD12 never gets sent
+		 * on successful completion.
+		 */
+		if (!host->mrq->sbc &&
+		    host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
+			mode |= SDHCI_TRNS_ACMD12;
 	}
 	if (data->flags & MMC_DATA_READ)
 		mode |= SDHCI_TRNS_READ;
@@ -877,7 +884,15 @@ static void sdhci_finish_data(struct sdhci_host *host)
 	else
 		data->bytes_xfered = data->blksz * data->blocks;
 
-	if (data->stop) {
+	/*
+	 * Need to send CMD12 if -
+	 * a) open-ended multiblock transfer (no CMD23)
+	 * b) error in multiblock transfer
+	 */
+	if (data->stop &&
+	    (data->error ||
+	     !host->mrq->sbc)) {
+
 		/*
 		 * The controller needs a reset of internal state machines
 		 * upon error conditions.
@@ -933,7 +948,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 
 	sdhci_writel(host, cmd->arg, SDHCI_ARGUMENT);
 
-	sdhci_set_transfer_mode(host, cmd->data);
+	sdhci_set_transfer_mode(host, cmd);
 
 	if ((cmd->flags & MMC_RSP_136) && (cmd->flags & MMC_RSP_BUSY)) {
 		printk(KERN_ERR "%s: Unsupported response type!\n",
@@ -986,13 +1001,21 @@ static void sdhci_finish_command(struct sdhci_host *host)
 
 	host->cmd->error = 0;
 
-	if (host->data && host->data_early)
-		sdhci_finish_data(host);
+	/* Finished CMD23, now send actual command. */
+	if (host->cmd == host->mrq->sbc) {
+		host->cmd = NULL;
+		sdhci_send_command(host, host->mrq->cmd);
+	} else {
 
-	if (!host->cmd->data)
-		tasklet_schedule(&host->finish_tasklet);
+		/* Processed actual command. */
+		if (host->data && host->data_early)
+			sdhci_finish_data(host);
 
-	host->cmd = NULL;
+		if (!host->cmd->data)
+			tasklet_schedule(&host->finish_tasklet);
+
+		host->cmd = NULL;
+	}
 }
 
 static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
@@ -1159,8 +1182,13 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	if (!present || host->flags & SDHCI_DEVICE_DEAD) {
 		host->mrq->cmd->error = -ENOMEDIUM;
 		tasklet_schedule(&host->finish_tasklet);
-	} else
-		sdhci_send_command(host, mrq->cmd);
+	} else {
+
+		if (mrq->sbc)
+			sdhci_send_command(host, mrq->sbc);
+		else
+			sdhci_send_command(host, mrq->cmd);
+	}
 
 	mmiowb();
 	spin_unlock_irqrestore(&host->lock, flags);
@@ -1909,7 +1937,7 @@ int sdhci_add_host(struct sdhci_host *host)
 		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;
 
 	mmc->f_max = host->max_clk;
-	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE;
+	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;
 
 	/*
 	 * A controller may support 8-bit width, but the board itself
-- 
1.7.0.4

[patchv2 3/3] MMC: Block CMD23 support for UHS104/SDXC cards.

[Andrei Warkentin]

SD cards operating at UHS104 or better support SET_BLOCK_COUNT.

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/card/block.c |    9 ++++++---
 drivers/mmc/core/sd.c    |    1 +
 include/linux/mmc/card.h |    1 +
 include/linux/mmc/sd.h   |    1 +
 4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index bba0888..69b70ce 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -851,9 +851,12 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	blk_queue_logical_block_size(md->queue.queue, 512);
 	set_capacity(md->disk, size);
 
-	if (mmc_host_cmd23(card->host) &&
-	    mmc_card_mmc(card))
-		md->flags |= MMC_BLK_CMD23;
+	if (mmc_host_cmd23(card->host)) {
+		if (mmc_card_mmc(card) ||
+		    (mmc_card_sd(card) &&
+		     card->scr.cmds & SCR_CMDS_CMD23))
+			md->flags |= MMC_BLK_CMD23;
+	}
 
 	if (mmc_card_mmc(card) &&
 	    md->flags & MMC_BLK_CMD23 &&
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 6dac89f..16a1f10 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -195,6 +195,7 @@ static int mmc_decode_scr(struct mmc_card *card)
 	else
 		card->erased_byte = 0x0;
 
+	scr->cmds = UNSTUFF_BITS(resp, 32, 2);
 	return 0;
 }
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 1e6bd6c..c036a78 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -70,6 +70,7 @@ struct sd_scr {
 	unsigned char		bus_widths;
 #define SD_SCR_BUS_WIDTH_1	(1<<0)
 #define SD_SCR_BUS_WIDTH_4	(1<<2)
+	unsigned char		cmds;
 };
 
 struct sd_ssr {
diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h
index 3fd85e0..81ba37b 100644
--- a/include/linux/mmc/sd.h
+++ b/include/linux/mmc/sd.h
@@ -60,6 +60,7 @@
 #define SCR_SPEC_VER_0		0	/* Implements system specification 1.0 - 1.01 */
 #define SCR_SPEC_VER_1		1	/* Implements system specification 1.10 */
 #define SCR_SPEC_VER_2		2	/* Implements system specification 2.00 */
+#define SCR_CMDS_CMD23		(1 << 1) /* Supports CMD23 */
 
 /*
  * SD bus widths
-- 
1.7.0.4

CMD23 plumbing and support patchset.

[Andrei Warkentin]

Dear Group,

This is the third version of the CMD23 plumbing and host driver support
patch set.

Changes:
1) CMD23 support (used for features such as reliable writes) is decoupled
   from general multiblock trans through use of a quirk for affected cards.
2) Newer Sandisk MMC products are whitelisted along with some known good ones. All other
   MMC products do not use CMD23 for general transfers (seems like safest choice for now).
   SD products unaffected.
3) Small fix for interaction with Auto-CMD12 (and resulting cleanup of Auto-CMD12 SDHCI capability)
4) Completely experimental (and untested) Auto-CMD23 support, for Arindam to look over (Hi!).

Table of Contents:
[patchv3 1/5] MMC: Add/remove quirks conditional support.
[patchv3 2/5] MMC: Use CMD23 for multiblock transfers when we can.
[patchv3 3/5] MMC: Implement MMC_CAP_CMD23 for SDHCI.
[patchv3 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.
[patchv3 5/5] MMC: SDHCI AutoCMD23 support.

Thanks,
A

[patchv3 1/5] MMC: Add/remove quirks conditional support.

[Andrei Warkentin]

Conditional add/remove quirks for MMC and SD.

Cc: cjb@laptop.org
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 include/linux/mmc/card.h |   40 ++++++++++++++++++++++++++++++++++++----
 1 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 72a9868..6a4ed2a 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -233,16 +233,14 @@ struct mmc_fixup {
 		    card->cid.month)
 
 /*
- * This hook just adds a quirk unconditionally.
+ * Unconditionally quirk add/remove.
  */
+
 static inline void __maybe_unused add_quirk(struct mmc_card *card, int data)
 {
 	card->quirks |= data;
 }
 
-/*
- * This hook just removes a quirk unconditionally.
- */
 static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
 {
 	card->quirks &= ~data;
@@ -264,6 +262,40 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
 #define mmc_card_set_blockaddr(c) ((c)->state |= MMC_STATE_BLOCKADDR)
 #define mmc_card_set_ddr_mode(c) ((c)->state |= MMC_STATE_HIGHSPEED_DDR)
 
+/*
+ * Quirk add/remove for MMC products.
+ */
+
+static inline void __maybe_unused add_quirk_mmc(struct mmc_card *card, int data)
+{
+	if (mmc_card_mmc(card))
+		card->quirks |= data;
+}
+
+static inline void __maybe_unused remove_quirk_mmc(struct mmc_card *card,
+						   int data)
+{
+	if (mmc_card_mmc(card))
+		card->quirks &= ~data;
+}
+
+/*
+ * Quirk add/remove for SD products.
+ */
+
+static inline void __maybe_unused add_quirk_sd(struct mmc_card *card, int data)
+{
+	if (mmc_card_sd(card))
+		card->quirks |= data;
+}
+
+static inline void __maybe_unused remove_quirk_sd(struct mmc_card *card,
+						   int data)
+{
+	if (mmc_card_sd(card))
+		card->quirks &= ~data;
+}
+
 static inline int mmc_card_lenient_fn0(const struct mmc_card *c)
 {
 	return c->quirks & MMC_QUIRK_LENIENT_FN0;
-- 
1.7.0.4

[patchv3 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Andrei Warkentin]

CMD23-prefixed instead of open-ended multiblock transfers
have a performance advantage on some MMC cards.

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/card/block.c |  114 ++++++++++++++++++++++++++++++++++------------
 include/linux/mmc/card.h |    1 +
 include/linux/mmc/core.h |    1 +
 include/linux/mmc/host.h |    6 ++
 include/linux/mmc/mmc.h  |    6 ++
 5 files changed, 98 insertions(+), 30 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index c209953..4b1a58e 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -55,10 +55,6 @@ MODULE_ALIAS("mmc:block");
 #define INAND_CMD38_ARG_SECTRIM1 0x81
 #define INAND_CMD38_ARG_SECTRIM2 0x88
 
-#define REL_WRITES_SUPPORTED(card) (mmc_card_mmc((card)) &&	\
-    (((card)->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||	\
-     ((card)->ext_csd.rel_sectors)))
-
 static DEFINE_MUTEX(block_mutex);
 
 /*
@@ -85,6 +81,10 @@ struct mmc_blk_data {
 	struct mmc_queue queue;
 	struct list_head part;
 
+	unsigned int	flags;
+#define MMC_BLK_CMD23	(1 << 0)	/* Can do SET_BLOCK_COUNT for multiblock */
+#define MMC_BLK_REL_WR	(1 << 1)	/* MMC Reliable write support */
+
 	unsigned int	usage;
 	unsigned int	read_only;
 	unsigned int	part_type;
@@ -225,6 +225,7 @@ static const struct block_device_operations mmc_bdops = {
 
 struct mmc_blk_request {
 	struct mmc_request	mrq;
+	struct mmc_command	sbc;
 	struct mmc_command	cmd;
 	struct mmc_command	stop;
 	struct mmc_data		data;
@@ -455,13 +456,10 @@ static int mmc_blk_issue_flush(struct mmc_queue *mq, struct request *req)
  * reliable write can handle, thus finish the request in
  * partial completions.
  */
-static inline int mmc_apply_rel_rw(struct mmc_blk_request *brq,
-				   struct mmc_card *card,
-				   struct request *req)
+static inline void mmc_apply_rel_rw(struct mmc_blk_request *brq,
+				    struct mmc_card *card,
+				    struct request *req)
 {
-	int err;
-	struct mmc_command set_count;
-
 	if (!(card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN)) {
 		/* Legacy mode imposes restrictions on transfers. */
 		if (!IS_ALIGNED(brq->cmd.arg, card->ext_csd.rel_sectors))
@@ -472,16 +470,6 @@ static inline int mmc_apply_rel_rw(struct mmc_blk_request *brq,
 		else if (brq->data.blocks < card->ext_csd.rel_sectors)
 			brq->data.blocks = 1;
 	}
-
-	memset(&set_count, 0, sizeof(struct mmc_command));
-	set_count.opcode = MMC_SET_BLOCK_COUNT;
-	set_count.arg = brq->data.blocks | (1 << 31);
-	set_count.flags = MMC_RSP_R1 | MMC_CMD_AC;
-	err = mmc_wait_for_cmd(card->host, &set_count, 0);
-	if (err)
-		printk(KERN_ERR "%s: error %d SET_BLOCK_COUNT\n",
-		       req->rq_disk->disk_name, err);
-	return err;
 }
 
 static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
@@ -498,7 +486,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 	bool do_rel_wr = ((req->cmd_flags & REQ_FUA) ||
 			  (req->cmd_flags & REQ_META)) &&
 		(rq_data_dir(req) == WRITE) &&
-		REL_WRITES_SUPPORTED(card);
+		(md->flags & MMC_BLK_REL_WR);
 
 	do {
 		struct mmc_command cmd;
@@ -536,11 +524,9 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 
 		if (brq.data.blocks > 1 || do_rel_wr) {
 			/* SPI multiblock writes terminate using a special
-			 * token, not a STOP_TRANSMISSION request. Reliable
-			 * writes use SET_BLOCK_COUNT and do not use a
-			 * STOP_TRANSMISSION request either.
+			 * token, not a STOP_TRANSMISSION request.
 			 */
-			if ((!mmc_host_is_spi(card->host) && !do_rel_wr) ||
+			if (!mmc_host_is_spi(card->host) ||
 			    rq_data_dir(req) == READ)
 				brq.mrq.stop = &brq.stop;
 			readcmd = MMC_READ_MULTIPLE_BLOCK;
@@ -558,8 +544,37 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 			brq.data.flags |= MMC_DATA_WRITE;
 		}
 
-		if (do_rel_wr && mmc_apply_rel_rw(&brq, card, req))
-			goto cmd_err;
+		if (do_rel_wr)
+			mmc_apply_rel_rw(&brq, card, req);
+
+		/*
+		 * Pre-defined multi-block transfers are preferable to
+		 * open ended-ones (and necessary for reliable writes).
+		 * However, it is not sufficient to just send CMD23,
+		 * and avoid the final CMD12, as on an error condition
+		 * CMD12 (stop) needs to be sent anyway. This, coupled
+		 * with Auto-CMD23 enhancements provided by some
+		 * hosts, means that the complexity of dealing
+		 * with this is best left to the host. If CMD23 is
+		 * supported by card and host, we'll fill sbc in and let
+		 * the host deal with handling it correctly. This means
+		 * that for hosts that don't expose MMC_CAP_CMD23, no
+		 * change of behavior will be observed.
+		 *
+		 * N.B: Some MMC cards experience perf degradation.
+		 * We'll avoid using CMD23-bounded multiblock writes for
+		 * these, while retaining features like reliable writes.
+		 */
+
+		if ((md->flags & MMC_BLK_CMD23) &&
+		    mmc_op_multi(brq.cmd.opcode) &&
+		    (do_rel_wr || !(card->quirks & MMC_QUIRK_BLK_NO_CMD23))) {
+			brq.sbc.opcode = MMC_SET_BLOCK_COUNT;
+			brq.sbc.arg = brq.data.blocks |
+				(do_rel_wr ? (1 << 31) : 0);
+			brq.sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
+			brq.mrq.sbc = &brq.sbc;
+		}
 
 		mmc_set_data_timeout(&brq.data, card);
 
@@ -596,7 +611,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 		 * until later as we need to wait for the card to leave
 		 * programming mode even when things go wrong.
 		 */
-		if (brq.cmd.error || brq.data.error || brq.stop.error) {
+		if (brq.sbc.error || brq.cmd.error ||
+		    brq.data.error || brq.stop.error) {
 			if (brq.data.blocks > 1 && rq_data_dir(req) == READ) {
 				/* Redo read one sector at a time */
 				printk(KERN_WARNING "%s: retrying using single "
@@ -607,6 +623,13 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 			status = get_card_status(card, req);
 		}
 
+		if (brq.sbc.error) {
+			printk(KERN_ERR "%s: error %d sending SET_BLOCK_COUNT "
+			       "command, response %#x, card status %#x\n",
+			       req->rq_disk->disk_name, brq.sbc.error,
+			       brq.sbc.resp[0], status);
+		}
+
 		if (brq.cmd.error) {
 			printk(KERN_ERR "%s: error %d sending read/write "
 			       "command, response %#x, card status %#x\n",
@@ -804,8 +827,6 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	md->disk->queue = md->queue.queue;
 	md->disk->driverfs_dev = parent;
 	set_disk_ro(md->disk, md->read_only || default_ro);
-	if (REL_WRITES_SUPPORTED(card))
-		blk_queue_flush(md->queue.queue, REQ_FLUSH | REQ_FUA);
 
 	/*
 	 * As discussed on lkml, GENHD_FL_REMOVABLE should:
@@ -829,6 +850,19 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 
 	blk_queue_logical_block_size(md->queue.queue, 512);
 	set_capacity(md->disk, size);
+
+	if (mmc_host_cmd23(card->host) &&
+	    mmc_card_mmc(card))
+		md->flags |= MMC_BLK_CMD23;
+
+	if (mmc_card_mmc(card) &&
+	    md->flags & MMC_BLK_CMD23 &&
+	    ((card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||
+	     card->ext_csd.rel_sectors)) {
+		md->flags |= MMC_BLK_REL_WR;
+		blk_queue_flush(md->queue.queue, REQ_FLUSH | REQ_FUA);
+	}
+
 	return md;
 
  err_putdisk:
@@ -982,6 +1016,26 @@ static const struct mmc_fixup blk_fixups[] =
 	MMC_FIXUP("SEM08G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
 	MMC_FIXUP("SEM16G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
 	MMC_FIXUP("SEM32G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
+
+	/*
+	 * Some MMC cards experience performance degradation with CMD23
+	 * instead of CMD12-bounded multiblock transfers. For now we'll
+	 * white list what's good:
+	 * 1) Certain SanDisk eMMCs with the old MMCA manfid.
+	 * 2) All new SanDisk products.
+	 *
+	 * N.B. This doesn't affect SD cards.
+	 */
+	MMC_FIXUP(CID_NAME_ANY, CID_MANFID_ANY, CID_OEMID_ANY, add_quirk_mmc,
+		  MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("SEM04G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("SEM08G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("SEM16G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("SEM32G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("SEM02G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP(CID_NAME_ANY, 0x45, CID_OEMID_ANY,
+		  remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
+
 	END_FIXUP
 };
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 6a4ed2a..c758181 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -134,6 +134,7 @@ struct mmc_card {
 #define MMC_QUIRK_NONSTD_FUNC_IF (1<<4)		/* SDIO card has nonstd function interfaces */
 #define MMC_QUIRK_DISABLE_CD	(1<<5)		/* disconnect CD/DAT[3] resistor */
 #define MMC_QUIRK_INAND_CMD38	(1<<6)		/* iNAND devices have broken CMD38 */
+#define MMC_QUIRK_BLK_NO_CMD23	(1<<7)		/* Avoid CMD23 for regular multiblock */
 
 	unsigned int		erase_size;	/* erase size in sectors */
  	unsigned int		erase_shift;	/* if erase unit is power 2 */
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index f8e4bcb..55d7fde 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -120,6 +120,7 @@ struct mmc_data {
 };
 
 struct mmc_request {
+	struct mmc_command	*sbc;		/* SET_BLOCK_COUNT for multiblock */
 	struct mmc_command	*cmd;
 	struct mmc_data		*data;
 	struct mmc_command	*stop;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 4f705eb..ba34fc5 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -173,6 +173,7 @@ struct mmc_host {
 						/* DDR mode at 1.2V */
 #define MMC_CAP_POWER_OFF_CARD	(1 << 13)	/* Can power off after boot */
 #define MMC_CAP_BUS_WIDTH_TEST	(1 << 14)	/* CMD14/CMD19 bus width ok */
+#define MMC_CAP_CMD23		(1 << 15)	/* CMD23 supported */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 
@@ -330,5 +331,10 @@ static inline int mmc_card_wake_sdio_irq(struct mmc_host *host)
 {
 	return host->pm_flags & MMC_PM_WAKE_SDIO_IRQ;
 }
+
+static inline int mmc_host_cmd23(struct mmc_host *host)
+{
+	return host->caps & MMC_CAP_CMD23;
+}
 #endif
 
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 373b2bf..ea74168 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -82,6 +82,12 @@
 #define MMC_APP_CMD              55   /* ac   [31:16] RCA        R1  */
 #define MMC_GEN_CMD              56   /* adtc [0] RD/WR          R1  */
 
+static inline bool mmc_op_multi(u32 opcode)
+{
+	return opcode == MMC_WRITE_MULTIPLE_BLOCK ||
+		opcode == MMC_READ_MULTIPLE_BLOCK;
+}
+
 /*
  * MMC_SWITCH argument format:
  *
-- 
1.7.0.4

[patchv3 3/5] MMC: Implement MMC_CAP_CMD23 for SDHCI.

[Andrei Warkentin]

Implements support for multiblock transfers bounded
by SET_BLOCK_COUNT (CMD23).

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/host/sdhci.c  |   70 ++++++++++++++++++++++++++++++++++-----------
 include/linux/mmc/sdhci.h |    1 +
 2 files changed, 54 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 94793f2..41c987d 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -822,9 +822,10 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 }
 
 static void sdhci_set_transfer_mode(struct sdhci_host *host,
-	struct mmc_data *data)
+	struct mmc_command *cmd)
 {
 	u16 mode;
+	struct mmc_data *data = cmd->data;
 
 	if (data == NULL)
 		return;
@@ -832,11 +833,17 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
 	WARN_ON(!host->data);
 
 	mode = SDHCI_TRNS_BLK_CNT_EN;
-	if (data->blocks > 1) {
-		if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
-			mode |= SDHCI_TRNS_MULTI | SDHCI_TRNS_ACMD12;
-		else
-			mode |= SDHCI_TRNS_MULTI;
+	if (mmc_op_multi(cmd->opcode) ||
+	    data->blocks > 1) {
+		mode |= SDHCI_TRNS_MULTI;
+
+		/*
+		 * If we are sending CMD23, CMD12 never gets sent
+		 * on successful completion (so no Auto-CMD12).
+		 */
+		if (!host->mrq->sbc &&
+		    host->flags & SDHCI_ACMD12)
+			mode |= SDHCI_TRNS_ACMD12;
 	}
 	if (data->flags & MMC_DATA_READ)
 		mode |= SDHCI_TRNS_READ;
@@ -877,7 +884,15 @@ static void sdhci_finish_data(struct sdhci_host *host)
 	else
 		data->bytes_xfered = data->blksz * data->blocks;
 
-	if (data->stop) {
+	/*
+	 * Need to send CMD12 if -
+	 * a) open-ended multiblock transfer (no CMD23)
+	 * b) error in multiblock transfer
+	 */
+	if (data->stop &&
+	    (data->error ||
+	     !host->mrq->sbc)) {
+
 		/*
 		 * The controller needs a reset of internal state machines
 		 * upon error conditions.
@@ -933,7 +948,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 
 	sdhci_writel(host, cmd->arg, SDHCI_ARGUMENT);
 
-	sdhci_set_transfer_mode(host, cmd->data);
+	sdhci_set_transfer_mode(host, cmd);
 
 	if ((cmd->flags & MMC_RSP_136) && (cmd->flags & MMC_RSP_BUSY)) {
 		printk(KERN_ERR "%s: Unsupported response type!\n",
@@ -986,13 +1001,21 @@ static void sdhci_finish_command(struct sdhci_host *host)
 
 	host->cmd->error = 0;
 
-	if (host->data && host->data_early)
-		sdhci_finish_data(host);
+	/* Finished CMD23, now send actual command. */
+	if (host->cmd == host->mrq->sbc) {
+		host->cmd = NULL;
+		sdhci_send_command(host, host->mrq->cmd);
+	} else {
 
-	if (!host->cmd->data)
-		tasklet_schedule(&host->finish_tasklet);
+		/* Processed actual command. */
+		if (host->data && host->data_early)
+			sdhci_finish_data(host);
 
-	host->cmd = NULL;
+		if (!host->cmd->data)
+			tasklet_schedule(&host->finish_tasklet);
+
+		host->cmd = NULL;
+	}
 }
 
 static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
@@ -1140,7 +1163,12 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 #ifndef SDHCI_USE_LEDS_CLASS
 	sdhci_activate_led(host);
 #endif
-	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12) {
+
+	/*
+	 * Ensure we don't send the STOP for non-SET_BLOCK_COUNTED
+	 * requests if Auto-CMD12 is enabled.
+	 */
+	if (!mrq->sbc && (host->flags & SDHCI_ACMD12)) {
 		if (mrq->stop) {
 			mrq->data->stop = NULL;
 			mrq->stop = NULL;
@@ -1159,8 +1187,13 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	if (!present || host->flags & SDHCI_DEVICE_DEAD) {
 		host->mrq->cmd->error = -ENOMEDIUM;
 		tasklet_schedule(&host->finish_tasklet);
-	} else
-		sdhci_send_command(host, mrq->cmd);
+	} else {
+
+		if (mrq->sbc)
+			sdhci_send_command(host, mrq->sbc);
+		else
+			sdhci_send_command(host, mrq->cmd);
+	}
 
 	mmiowb();
 	spin_unlock_irqrestore(&host->lock, flags);
@@ -1909,7 +1942,10 @@ int sdhci_add_host(struct sdhci_host *host)
 		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;
 
 	mmc->f_max = host->max_clk;
-	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE;
+	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;
+
+	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
+		host->flags |= SDHCI_ACMD12;
 
 	/*
 	 * A controller may support 8-bit width, but the board itself
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 83bd9f7..c54ac14 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -109,6 +109,7 @@ struct sdhci_host {
 #define SDHCI_USE_ADMA		(1<<1)	/* Host is ADMA capable */
 #define SDHCI_REQ_USE_DMA	(1<<2)	/* Use DMA for this req. */
 #define SDHCI_DEVICE_DEAD	(1<<3)	/* Device unresponsive */
+#define SDHCI_ACMD12		(1<<4)	/* Auto CMD12 support */
 
 	unsigned int version;	/* SDHCI spec. version */
 
-- 
1.7.0.4

[patchv3 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.

[Andrei Warkentin]

SD cards operating at UHS104 or better support SET_BLOCK_COUNT.

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/card/block.c |    9 ++++++---
 drivers/mmc/core/sd.c    |    1 +
 include/linux/mmc/card.h |    1 +
 include/linux/mmc/sd.h   |    1 +
 4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 4b1a58e..23da6e6 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -851,9 +851,12 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	blk_queue_logical_block_size(md->queue.queue, 512);
 	set_capacity(md->disk, size);
 
-	if (mmc_host_cmd23(card->host) &&
-	    mmc_card_mmc(card))
-		md->flags |= MMC_BLK_CMD23;
+	if (mmc_host_cmd23(card->host)) {
+		if (mmc_card_mmc(card) ||
+		    (mmc_card_sd(card) &&
+		     card->scr.cmds & SCR_CMDS_CMD23))
+			md->flags |= MMC_BLK_CMD23;
+	}
 
 	if (mmc_card_mmc(card) &&
 	    md->flags & MMC_BLK_CMD23 &&
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 6dac89f..16a1f10 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -195,6 +195,7 @@ static int mmc_decode_scr(struct mmc_card *card)
 	else
 		card->erased_byte = 0x0;
 
+	scr->cmds = UNSTUFF_BITS(resp, 32, 2);
 	return 0;
 }
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index c758181..b33186a 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -70,6 +70,7 @@ struct sd_scr {
 	unsigned char		bus_widths;
 #define SD_SCR_BUS_WIDTH_1	(1<<0)
 #define SD_SCR_BUS_WIDTH_4	(1<<2)
+	unsigned char		cmds;
 };
 
 struct sd_ssr {
diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h
index 3fd85e0..81ba37b 100644
--- a/include/linux/mmc/sd.h
+++ b/include/linux/mmc/sd.h
@@ -60,6 +60,7 @@
 #define SCR_SPEC_VER_0		0	/* Implements system specification 1.0 - 1.01 */
 #define SCR_SPEC_VER_1		1	/* Implements system specification 1.10 */
 #define SCR_SPEC_VER_2		2	/* Implements system specification 2.00 */
+#define SCR_CMDS_CMD23		(1 << 1) /* Supports CMD23 */
 
 /*
  * SD bus widths
-- 
1.7.0.4

[patchv3 5/5] MMC: SDHCI AutoCMD23 support.

[Andrei Warkentin]

Enables Auto-CMD23 support where available (SDHCI 3.0 controllers)

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Cc: subhashj@codeaurora.org
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/host/Kconfig  |    9 +++++++++
 drivers/mmc/host/sdhci.c  |   16 +++++++++++++++-
 drivers/mmc/host/sdhci.h  |    2 ++
 include/linux/mmc/sdhci.h |    1 +
 4 files changed, 27 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index b981715..d4e5658 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -68,6 +68,15 @@ config MMC_SDHCI_PCI
 
 	  If unsure, say N.
 
+config MMC_SDHCI_AUTO_CMD23
+       tristate "SDHCI 3.0 Auto-CMD23 support (EXPERIMENTAL)"
+       depends on MMC_SDHCI && EXPERIMENTAL
+       help
+         This enables the Auto-CMD23 optimization for multiblock
+         transfers where a CMD23 would be sent
+
+         If unsure, say N.
+
 config MMC_RICOH_MMC
 	bool "Ricoh MMC Controller Disabler  (EXPERIMENTAL)"
 	depends on MMC_SDHCI_PCI
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 41c987d..e7a38b6 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -844,7 +844,13 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
 		if (!host->mrq->sbc &&
 		    host->flags & SDHCI_ACMD12)
 			mode |= SDHCI_TRNS_ACMD12;
+		else if (host->mrq->sbc &&
+			 host->flags & SDHCI_ACMD23) {
+			mode |= SDHCI_TRNS_ACMD23;
+			sdhci_writel(host, data->blocks, SDHCI_ARGUMENT2);
+		}
 	}
+
 	if (data->flags & MMC_DATA_READ)
 		mode |= SDHCI_TRNS_READ;
 	if (host->flags & SDHCI_REQ_USE_DMA)
@@ -1189,7 +1195,7 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 		tasklet_schedule(&host->finish_tasklet);
 	} else {
 
-		if (mrq->sbc)
+		if (mrq->sbc && !(host->flags & SDHCI_ACMD23))
 			sdhci_send_command(host, mrq->sbc);
 		else
 			sdhci_send_command(host, mrq->cmd);
@@ -1947,6 +1953,14 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
 		host->flags |= SDHCI_ACMD12;
 
+#ifdef CONFIG_MMC_SDHCI_CMD23
+	/* Auto-CMD23 stuff only works in ADMA or PIO. */
+	if ((host->version == SDHCI_SPEC_300) &&
+	    ((host->flags & SDHCI_USE_ADMA) ||
+	     !(host->flags & SDHCI_REQ_USE_DMA)))
+		host->flags |= SDHCI_ACMD23;
+#endif
+
 	/*
 	 * A controller may support 8-bit width, but the board itself
 	 * might not have the pins brought out.  Boards that support
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 85750a9..2b3fb76 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -25,6 +25,7 @@
  */
 
 #define SDHCI_DMA_ADDRESS	0x00
+#define SDHCI_ARGUMENT2		0x00
 
 #define SDHCI_BLOCK_SIZE	0x04
 #define  SDHCI_MAKE_BLKSZ(dma, blksz) (((dma & 0x7) << 12) | (blksz & 0xFFF))
@@ -37,6 +38,7 @@
 #define  SDHCI_TRNS_DMA		0x01
 #define  SDHCI_TRNS_BLK_CNT_EN	0x02
 #define  SDHCI_TRNS_ACMD12	0x04
+#define  SDHCI_TRNS_ACMD23	0x08
 #define  SDHCI_TRNS_READ	0x10
 #define  SDHCI_TRNS_MULTI	0x20
 
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index c54ac14..d91585a 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -110,6 +110,7 @@ struct sdhci_host {
 #define SDHCI_REQ_USE_DMA	(1<<2)	/* Use DMA for this req. */
 #define SDHCI_DEVICE_DEAD	(1<<3)	/* Device unresponsive */
 #define SDHCI_ACMD12		(1<<4)	/* Auto CMD12 support */
+#define SDHCI_ACMD23		(1<<5)	/* Auto CMD23 support */
 
 	unsigned int version;	/* SDHCI spec. version */
 
-- 
1.7.0.4

RE: [patchv3 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.

[Nath, Arindam]

Hi Andrei,


> -----Original Message-----
> From: Andrei Warkentin [mailto:andreiw@motorola.com]
> Sent: Saturday, April 16, 2011 4:11 PM
> To: linux-mmc@vger.kernel.org
> Cc: Nath, Arindam; Andrei Warkentin; cjb@laptop.org; arnd@arndb.de
> Subject: [patchv3 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.
> 
> SD cards operating at UHS104 or better support SET_BLOCK_COUNT.
> 
> Cc: arindam.nath@amd.com
> Cc: cjb@laptop.org
> Cc: arnd@arndb.de
> Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
> ---
>  drivers/mmc/card/block.c |    9 ++++++---
>  drivers/mmc/core/sd.c    |    1 +
>  include/linux/mmc/card.h |    1 +
>  include/linux/mmc/sd.h   |    1 +
>  4 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 4b1a58e..23da6e6 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -851,9 +851,12 @@ static struct mmc_blk_data
> *mmc_blk_alloc_req(struct mmc_card *card,
>  	blk_queue_logical_block_size(md->queue.queue, 512);
>  	set_capacity(md->disk, size);
> 
> -	if (mmc_host_cmd23(card->host) &&
> -	    mmc_card_mmc(card))
> -		md->flags |= MMC_BLK_CMD23;
> +	if (mmc_host_cmd23(card->host)) {
> +		if (mmc_card_mmc(card) ||
> +		    (mmc_card_sd(card) &&
> +		     card->scr.cmds & SCR_CMDS_CMD23))
> +			md->flags |= MMC_BLK_CMD23;
> +	}
> 
>  	if (mmc_card_mmc(card) &&
>  	    md->flags & MMC_BLK_CMD23 &&
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 6dac89f..16a1f10 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -195,6 +195,7 @@ static int mmc_decode_scr(struct mmc_card *card)
>  	else
>  		card->erased_byte = 0x0;
> 
> +	scr->cmds = UNSTUFF_BITS(resp, 32, 2);

Since support for CMD20 and CMD23 have been added in Physical Layer Spec v3.01, IMO it is better to set scr->cmds based on the spec version, rather than doing it unconditionally. Something like,

+       if (scr->sda_vsn == SCR_SPEC_VER_2) {
+               /* Check if Physical Layer Spec v3.0 is supported*/
+               scr->sda_spec3 = UNSTUFF_BITS(resp, 47, 1);
+               if (scr->sda_spec3)
+                       scr->cmds = UNSTUFF_BITS(resp, 32, 2);
+       }

>  	return 0;
>  }
> 
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index c758181..b33186a 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -70,6 +70,7 @@ struct sd_scr {
>  	unsigned char		bus_widths;
>  #define SD_SCR_BUS_WIDTH_1	(1<<0)
>  #define SD_SCR_BUS_WIDTH_4	(1<<2)
> +	unsigned char		cmds;
>  };

How about...

struct sd_scr {
       unsigned char           sda_vsn;
+       unsigned char           sda_spec3;
       unsigned char           bus_widths;
 #define SD_SCR_BUS_WIDTH_1     (1<<0)
 #define SD_SCR_BUS_WIDTH_4     (1<<2)
+       unsigned char           cmds;
+#define SD_SCR_CMD20_SUPPORT   (1<<0)
+#define SD_SCR_CMD23_SUPPORT   (1<<1)
 };

This way we can let go of...

> diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h
> index 3fd85e0..81ba37b 100644
> --- a/include/linux/mmc/sd.h
> +++ b/include/linux/mmc/sd.h
> +#define SCR_CMDS_CMD23		(1 << 1) /* Supports CMD23 */

Thanks,
Arindam
 

Re: [patchv3 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.

[Andrei Warkentin]

On Sat, Apr 16, 2011 at 11:19 AM, Nath, Arindam <Arindam.Nath@amd.com> wrote:
> Hi Andrei,
> Since support for CMD20 and CMD23 have been added in Physical Layer Spec v3.01, IMO it is better to set scr->cmds based on the spec version, rather than doing it unconditionally. Something like,
>
> +       if (scr->sda_vsn == SCR_SPEC_VER_2) {
> +               /* Check if Physical Layer Spec v3.0 is supported*/
> +               scr->sda_spec3 = UNSTUFF_BITS(resp, 47, 1);
> +               if (scr->sda_spec3)
> +                       scr->cmds = UNSTUFF_BITS(resp, 32, 2);
> +       }
>

Sounds good.

A

[PATCH] MMC: Block CMD23 support for UHS104/SDXC cards.

[Andrei Warkentin]

SD cards operating at UHS104 or better support SET_BLOCK_COUNT.

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/card/block.c |    9 ++++++---
 drivers/mmc/core/sd.c    |    7 +++++++
 include/linux/mmc/card.h |    4 ++++
 include/linux/mmc/sd.h   |    2 +-
 4 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 4b1a58e..520529a 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -851,9 +851,12 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	blk_queue_logical_block_size(md->queue.queue, 512);
 	set_capacity(md->disk, size);
 
-	if (mmc_host_cmd23(card->host) &&
-	    mmc_card_mmc(card))
-		md->flags |= MMC_BLK_CMD23;
+	if (mmc_host_cmd23(card->host)) {
+		if (mmc_card_mmc(card) ||
+		    (mmc_card_sd(card) &&
+		     card->scr.cmds & SD_SCR_CMD23_SUPPORT))
+			md->flags |= MMC_BLK_CMD23;
+	}
 
 	if (mmc_card_mmc(card) &&
 	    md->flags & MMC_BLK_CMD23 &&
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 6dac89f..a3d771a 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -195,6 +195,13 @@ static int mmc_decode_scr(struct mmc_card *card)
 	else
 		card->erased_byte = 0x0;
 
+	if (scr->sda_vsn == SCR_SPEC_VER_2) {
+
+		/* Check if Physical Layer Spec v3.0X is supported. */
+		scr->sda_spec3 = UNSTUFF_BITS(resp, 47, 1);
+		if (scr->sda_spec3)
+			scr->cmds = UNSTUFF_BITS(resp, 32, 2);
+	}
 	return 0;
 }
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index c758181..e6b2e6f 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -67,9 +67,13 @@ struct mmc_ext_csd {
 
 struct sd_scr {
 	unsigned char		sda_vsn;
+	unsigned char		sda_spec3;
 	unsigned char		bus_widths;
 #define SD_SCR_BUS_WIDTH_1	(1<<0)
 #define SD_SCR_BUS_WIDTH_4	(1<<2)
+	unsigned char		cmds;
+#define SD_SCR_CMD20_SUPPORT   (1<<0)
+#define SD_SCR_CMD23_SUPPORT   (1<<1)
 };
 
 struct sd_ssr {
diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h
index 3fd85e0..9fe0689 100644
--- a/include/linux/mmc/sd.h
+++ b/include/linux/mmc/sd.h
@@ -59,7 +59,7 @@
 
 #define SCR_SPEC_VER_0		0	/* Implements system specification 1.0 - 1.01 */
 #define SCR_SPEC_VER_1		1	/* Implements system specification 1.10 */
-#define SCR_SPEC_VER_2		2	/* Implements system specification 2.00 */
+#define SCR_SPEC_VER_2		2	/* Implements system specification 2.00-3.0X */
 
 /*
  * SD bus widths
-- 
1.7.0.4

Re: [patchv3 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Arnd Bergmann]

On Saturday 16 April 2011, Andrei Warkentin wrote:
> @@ -982,6 +1016,26 @@ static const struct mmc_fixup blk_fixups[] =
>         MMC_FIXUP("SEM08G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
>         MMC_FIXUP("SEM16G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
>         MMC_FIXUP("SEM32G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
> +
> +       /*
> +        * Some MMC cards experience performance degradation with CMD23
> +        * instead of CMD12-bounded multiblock transfers. For now we'll
> +        * white list what's good:
> +        * 1) Certain SanDisk eMMCs with the old MMCA manfid.
> +        * 2) All new SanDisk products.
> +        *
> +        * N.B. This doesn't affect SD cards.
> +        */
> +       MMC_FIXUP(CID_NAME_ANY, CID_MANFID_ANY, CID_OEMID_ANY, add_quirk_mmc,
> +                 MMC_QUIRK_BLK_NO_CMD23),
> +       MMC_FIXUP("SEM04G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
> +       MMC_FIXUP("SEM08G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
> +       MMC_FIXUP("SEM16G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
> +       MMC_FIXUP("SEM32G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
> +       MMC_FIXUP("SEM02G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
> +       MMC_FIXUP(CID_NAME_ANY, 0x45, CID_OEMID_ANY,
> +                 remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
> +
>         END_FIXUP
>  };

Shouldn't this better be a blacklist for known bad cards?

As far as I can tell, we should always use CMD23 where possible.

	Arnd

Re: [patchv3 5/5] MMC: SDHCI AutoCMD23 support.

[Arnd Bergmann]

On Saturday 16 April 2011, Andrei Warkentin wrote:
> @@ -68,6 +68,15 @@ config MMC_SDHCI_PCI
>  
>           If unsure, say N.
>  
> +config MMC_SDHCI_AUTO_CMD23
> +       tristate "SDHCI 3.0 Auto-CMD23 support (EXPERIMENTAL)"
> +       depends on MMC_SDHCI && EXPERIMENTAL
> +       help
> +         This enables the Auto-CMD23 optimization for multiblock
> +         transfers where a CMD23 would be sent
> +
> +         If unsure, say N.
> +
>  config MMC_RICOH_MMC
>         bool "Ricoh MMC Controller Disabler  (EXPERIMENTAL)"
>         depends on MMC_SDHCI_PCI

Does this need to be a Konfig option? We are already asking too many
questions here IMHO.

	Arnd

Re: [patchv3 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Andrei Warkentin]

Hi Arnd,

On Sun, Apr 17, 2011 at 12:23 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Saturday 16 April 2011, Andrei Warkentin wrote:
>> @@ -982,6 +1016,26 @@ static const struct mmc_fixup blk_fixups[] =
>>         MMC_FIXUP("SEM08G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
>>         MMC_FIXUP("SEM16G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
>>         MMC_FIXUP("SEM32G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
>> +
>> +       /*
>> +        * Some MMC cards experience performance degradation with CMD23
>> +        * instead of CMD12-bounded multiblock transfers. For now we'll
>> +        * white list what's good:
>> +        * 1) Certain SanDisk eMMCs with the old MMCA manfid.
>> +        * 2) All new SanDisk products.
>> +        *
>> +        * N.B. This doesn't affect SD cards.
>> +        */
>> +       MMC_FIXUP(CID_NAME_ANY, CID_MANFID_ANY, CID_OEMID_ANY, add_quirk_mmc,
>> +                 MMC_QUIRK_BLK_NO_CMD23),
>> +       MMC_FIXUP("SEM04G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
>> +       MMC_FIXUP("SEM08G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
>> +       MMC_FIXUP("SEM16G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
>> +       MMC_FIXUP("SEM32G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
>> +       MMC_FIXUP("SEM02G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
>> +       MMC_FIXUP(CID_NAME_ANY, 0x45, CID_OEMID_ANY,
>> +                 remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
>> +
>>         END_FIXUP
>>  };
>
> Shouldn't this better be a blacklist for known bad cards?
>
> As far as I can tell, we should always use CMD23 where possible.
>
>        Arnd
>

Unfortunately it's unknown yet for how many MMC cards this could be a
regression. So far it's a regression for Toshiba MMC32G/MMC16G/MMC08G
cards because they don't do anything with the advanced knowledge of
transfer size, and not using CMD12 seems somehow to interfere with
some internal optimization, resulting in a real-life degradation of
about 8-10%.  According to Sandisk, all their newer products (MMC spec
4.3+) should be good (with new manfid), and I am stilll waiting for
specifics for older cards. I (well, and another person at MMI) are
collecting some more data for more newer eMMC devices to get a clearer
picture, and I am trying to get word on whether newer Toshiba devices
should work better with CMD23.

A

Re: [patchv3 5/5] MMC: SDHCI AutoCMD23 support.

[Andrei Warkentin]

Hi Arnd,

On Sun, Apr 17, 2011 at 12:25 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Saturday 16 April 2011, Andrei Warkentin wrote:
>> @@ -68,6 +68,15 @@ config MMC_SDHCI_PCI
>>
>>           If unsure, say N.
>>
>> +config MMC_SDHCI_AUTO_CMD23
>> +       tristate "SDHCI 3.0 Auto-CMD23 support (EXPERIMENTAL)"
>> +       depends on MMC_SDHCI && EXPERIMENTAL
>> +       help
>> +         This enables the Auto-CMD23 optimization for multiblock
>> +         transfers where a CMD23 would be sent
>> +
>> +         If unsure, say N.
>> +
>>  config MMC_RICOH_MMC
>>         bool "Ricoh MMC Controller Disabler  (EXPERIMENTAL)"
>>         depends on MMC_SDHCI_PCI
>
> Does this need to be a Konfig option? We are already asking too many
> questions here IMHO.

Sorry, I should have clarified - I made this an experimental option
because I couldn't vouch for the correct functionality (no SDHCI 3.0
hardware around me) and I posted it only because Arindam wanted to try
it out and see if it worked alright. As soon as we're clear it works
fine I'll take the Kconfig out.

A

Re: [patchv3 5/5] MMC: SDHCI AutoCMD23 support.

[Andrei Warkentin]

Arindam,

On Sun, Apr 17, 2011 at 2:31 PM, Andrei Warkentin <andreiw@motorola.com> wrote:
> Hi Arnd,
>
> On Sun, Apr 17, 2011 at 12:25 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Saturday 16 April 2011, Andrei Warkentin wrote:
>>> @@ -68,6 +68,15 @@ config MMC_SDHCI_PCI
>>>
>>>           If unsure, say N.
>>>
>>> +config MMC_SDHCI_AUTO_CMD23
>>> +       tristate "SDHCI 3.0 Auto-CMD23 support (EXPERIMENTAL)"
>>> +       depends on MMC_SDHCI && EXPERIMENTAL
>>> +       help
>>> +         This enables the Auto-CMD23 optimization for multiblock
>>> +         transfers where a CMD23 would be sent
>>> +
>>> +         If unsure, say N.
>>> +
>>>  config MMC_RICOH_MMC
>>>         bool "Ricoh MMC Controller Disabler  (EXPERIMENTAL)"
>>>         depends on MMC_SDHCI_PCI
>>
>> Does this need to be a Konfig option? We are already asking too many
>> questions here IMHO.
>
> Sorry, I should have clarified - I made this an experimental option
> because I couldn't vouch for the correct functionality (no SDHCI 3.0
> hardware around me) and I posted it only because Arindam wanted to try
> it out and see if it worked alright. As soon as we're clear it works
> fine I'll take the Kconfig out.
>
> A
>

Any update? Have you tried this out?

Thanks,
A

RE: [patchv3 5/5] MMC: SDHCI AutoCMD23 support.

[Nath, Arindam]

Hi Andrei,


> -----Original Message-----
> From: Andrei Warkentin [mailto:andreiw@motorola.com]
> Sent: Tuesday, April 19, 2011 8:36 AM
> To: Nath, Arindam
> Cc: linux-mmc@vger.kernel.org; cjb@laptop.org; subhashj@codeaurora.org;
> Arnd Bergmann
> Subject: Re: [patchv3 5/5] MMC: SDHCI AutoCMD23 support.
> 
> Arindam,
> 
> On Sun, Apr 17, 2011 at 2:31 PM, Andrei Warkentin
> <andreiw@motorola.com> wrote:
> > Hi Arnd,
> >
> > On Sun, Apr 17, 2011 at 12:25 PM, Arnd Bergmann <arnd@arndb.de>
> wrote:
> >> On Saturday 16 April 2011, Andrei Warkentin wrote:
> >>> @@ -68,6 +68,15 @@ config MMC_SDHCI_PCI
> >>>
> >>>           If unsure, say N.
> >>>
> >>> +config MMC_SDHCI_AUTO_CMD23
> >>> +       tristate "SDHCI 3.0 Auto-CMD23 support (EXPERIMENTAL)"
> >>> +       depends on MMC_SDHCI && EXPERIMENTAL
> >>> +       help
> >>> +         This enables the Auto-CMD23 optimization for multiblock
> >>> +         transfers where a CMD23 would be sent
> >>> +
> >>> +         If unsure, say N.
> >>> +
> >>>  config MMC_RICOH_MMC
> >>>         bool "Ricoh MMC Controller Disabler  (EXPERIMENTAL)"
> >>>         depends on MMC_SDHCI_PCI
> >>
> >> Does this need to be a Konfig option? We are already asking too many
> >> questions here IMHO.
> >
> > Sorry, I should have clarified - I made this an experimental option
> > because I couldn't vouch for the correct functionality (no SDHCI 3.0
> > hardware around me) and I posted it only because Arindam wanted to
> try
> > it out and see if it worked alright. As soon as we're clear it works
> > fine I'll take the Kconfig out.
> >
> > A
> >
> 
> Any update? Have you tried this out?

I am planning to re-submit V3 patches sometime today or tomorrow. Only after that I will apply your patches on top of mine, and test it out. Please give me some time, I will try to get back to you ASAP.

Thanks,
Arindam

> 
> Thanks,
> A

Re: [patchv3 5/5] MMC: SDHCI AutoCMD23 support.

[Andrei Warkentin]

On Mon, Apr 18, 2011 at 10:19 PM, Nath, Arindam <Arindam.Nath@amd.com> wrote:
> Hi Andrei,
>
>
>> -----Original Message-----
>> From: Andrei Warkentin [mailto:andreiw@motorola.com]
>> Sent: Tuesday, April 19, 2011 8:36 AM
>> To: Nath, Arindam
>> Cc: linux-mmc@vger.kernel.org; cjb@laptop.org; subhashj@codeaurora.org;
>> Arnd Bergmann
>> Subject: Re: [patchv3 5/5] MMC: SDHCI AutoCMD23 support.
>>
>> Arindam,
>>
>> On Sun, Apr 17, 2011 at 2:31 PM, Andrei Warkentin
>> <andreiw@motorola.com> wrote:
>> > Hi Arnd,
>> >
>> > On Sun, Apr 17, 2011 at 12:25 PM, Arnd Bergmann <arnd@arndb.de>
>> wrote:
>> >> On Saturday 16 April 2011, Andrei Warkentin wrote:
>> >>> @@ -68,6 +68,15 @@ config MMC_SDHCI_PCI
>> >>>
>> >>>           If unsure, say N.
>> >>>
>> >>> +config MMC_SDHCI_AUTO_CMD23
>> >>> +       tristate "SDHCI 3.0 Auto-CMD23 support (EXPERIMENTAL)"
>> >>> +       depends on MMC_SDHCI && EXPERIMENTAL
>> >>> +       help
>> >>> +         This enables the Auto-CMD23 optimization for multiblock
>> >>> +         transfers where a CMD23 would be sent
>> >>> +
>> >>> +         If unsure, say N.
>> >>> +
>> >>>  config MMC_RICOH_MMC
>> >>>         bool "Ricoh MMC Controller Disabler  (EXPERIMENTAL)"
>> >>>         depends on MMC_SDHCI_PCI
>> >>
>> >> Does this need to be a Konfig option? We are already asking too many
>> >> questions here IMHO.
>> >
>> > Sorry, I should have clarified - I made this an experimental option
>> > because I couldn't vouch for the correct functionality (no SDHCI 3.0
>> > hardware around me) and I posted it only because Arindam wanted to
>> try
>> > it out and see if it worked alright. As soon as we're clear it works
>> > fine I'll take the Kconfig out.
>> >
>> > A
>> >
>>
>> Any update? Have you tried this out?
>
> I am planning to re-submit V3 patches sometime today or tomorrow. Only after that I will apply your patches on top of mine, and test it out. Please give me some time, I will try to get back to you ASAP.

Alright, thanks, looking forward to your results.

Thanks,
A

Re: [patchv3 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Andrei Warkentin]

On Sun, Apr 17, 2011 at 12:23 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Saturday 16 April 2011, Andrei Warkentin wrote:
>> @@ -982,6 +1016,26 @@ static const struct mmc_fixup blk_fixups[] =
>>         MMC_FIXUP("SEM08G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
>>         MMC_FIXUP("SEM16G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
>>         MMC_FIXUP("SEM32G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
>> +
>> +       /*
>> +        * Some MMC cards experience performance degradation with CMD23
>> +        * instead of CMD12-bounded multiblock transfers. For now we'll
>> +        * white list what's good:
>> +        * 1) Certain SanDisk eMMCs with the old MMCA manfid.
>> +        * 2) All new SanDisk products.
>> +        *
>> +        * N.B. This doesn't affect SD cards.
>> +        */
>> +       MMC_FIXUP(CID_NAME_ANY, CID_MANFID_ANY, CID_OEMID_ANY, add_quirk_mmc,
>> +                 MMC_QUIRK_BLK_NO_CMD23),
>> +       MMC_FIXUP("SEM04G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
>> +       MMC_FIXUP("SEM08G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
>> +       MMC_FIXUP("SEM16G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
>> +       MMC_FIXUP("SEM32G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
>> +       MMC_FIXUP("SEM02G", 0x2, 0x100, remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
>> +       MMC_FIXUP(CID_NAME_ANY, 0x45, CID_OEMID_ANY,
>> +                 remove_quirk_mmc, MMC_QUIRK_BLK_NO_CMD23),
>> +
>>         END_FIXUP
>>  };
>
> Shouldn't this better be a blacklist for known bad cards?
>
> As far as I can tell, we should always use CMD23 where possible.
>

I thought about this some more. In this space (eMMCs), it's still
going to be up to the system integrator to evaluate the system and
enable/disable certain properties (such as CMD23 use) as they make
sense. What you would want to avoid, ideally, is someone compiling a
kernel for their pre-2.6.40 embedded system, not knowing anything
amount eMMCs and CMD23, and winding up with an I/O performance
regression.

So maybe this should be a blacklist for known bad cards. And the
entire support should be a "default-N" compile option for MMCs (not
SDs). That way someone who just does an "make oldconfig" will see
"CONFIG_MMC_BLK_CMD23 - I/O performance improvement for newer eMMC
cards, may cause degradation on older cards". What do you think?

A

Re: [patchv3 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Arnd Bergmann]

On Tuesday 19 April 2011, Andrei Warkentin wrote:

> So maybe this should be a blacklist for known bad cards. And the
> entire support should be a "default-N" compile option for MMCs (not
> SDs). That way someone who just does an "make oldconfig" will see
> "CONFIG_MMC_BLK_CMD23 - I/O performance improvement for newer eMMC
> cards, may cause degradation on older cards". What do you think?

I'm not sure if I understand the distinction between MMC and SD
here. Do you suggest we always enable it for SD but make it compile-time
selected for MMC?

I generally argue against compile time options. A distribution
integrator needs to choose a reasonable default, and giving them
an option makes it possible to get it wrong.

I believe the best way would be trying to warn people against
regressions while going forward with this enabled unconditionally,
unless we hear back from people that actually got regressions.

We could perhaps key enabling the feature by the production date
on the card, so it only gets turned on for new cards.

	Arnd

Re: [patchv3 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Andrei Warkentin]

On Tue, Apr 19, 2011 at 2:39 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 19 April 2011, Andrei Warkentin wrote:
>
>> So maybe this should be a blacklist for known bad cards. And the
>> entire support should be a "default-N" compile option for MMCs (not
>> SDs). That way someone who just does an "make oldconfig" will see
>> "CONFIG_MMC_BLK_CMD23 - I/O performance improvement for newer eMMC
>> cards, may cause degradation on older cards". What do you think?
>
> I'm not sure if I understand the distinction between MMC and SD
> here. Do you suggest we always enable it for SD but make it compile-time
> selected for MMC?

I did, because CMD23 support on SDs is a new feature mandatory on
UHS104 cards. However, I guess no matter
what you do, it's going to break something.  Right now I'm only aware
of Toshiba perf regressions.

>
> I generally argue against compile time options. A distribution
> integrator needs to choose a reasonable default, and giving them
> an option makes it possible to get it wrong.
>
> I believe the best way would be trying to warn people against
> regressions while going forward with this enabled unconditionally,
> unless we hear back from people that actually got regressions.

Alright. Then I'll just blacklist the known-affected Toshiba cards for
now, and we'll keep it enabled by default.

>
> We could perhaps key enabling the feature by the production date
> on the card, so it only gets turned on for new cards.
>

Unfortunately, date by itself is meaningless too (current Toshiba eMMCs)

A

Re: CMD23 plumbing and support patchset.

[Chris Ball]

Hi,

On Sat, Apr 16 2011, Andrei Warkentin wrote:
> This is the third version of the CMD23 plumbing and host driver support
> patch set.
>
> Changes:
> 1) CMD23 support (used for features such as reliable writes) is decoupled
>    from general multiblock trans through use of a quirk for affected cards.
> 2) Newer Sandisk MMC products are whitelisted along with some known good ones. All other
>    MMC products do not use CMD23 for general transfers (seems like safest choice for now).
>    SD products unaffected.

Just gave this a try on SEM04G, which is in the whitelist, but didn't
see a performance increase (or decrease).  What are you using for a
testcase?  I'm a little wary of running mmc_test on it.  :)

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: CMD23 plumbing and support patchset.

[Andrei Warkentin]

Hi Chris,

On Wed, Apr 20, 2011 at 8:44 PM, Chris Ball <cjb@laptop.org> wrote:
> Hi,
>
> On Sat, Apr 16 2011, Andrei Warkentin wrote:
>> This is the third version of the CMD23 plumbing and host driver support
>> patch set.
>>
>> Changes:
>> 1) CMD23 support (used for features such as reliable writes) is decoupled
>>    from general multiblock trans through use of a quirk for affected cards.
>> 2) Newer Sandisk MMC products are whitelisted along with some known good ones. All other
>>    MMC products do not use CMD23 for general transfers (seems like safest choice for now).
>>    SD products unaffected.
>
> Just gave this a try on SEM04G, which is in the whitelist, but didn't
> see a performance increase (or decrease).  What are you using for a
> testcase?  I'm a little wary of running mmc_test on it.  :)
>

Can you give me the manufacturing date? This is pretty interesting.

You can see an improvement even if you do something like time block
writes (O_DIRECT | O_SYNC, of course). But after all synthetic tests I
measured the time it took to do 40000 SQLite insertions and saw
something like 30% improvement.

A

Re: CMD23 plumbing and support patchset.

[Andrei Warkentin]

On Thu, Apr 21, 2011 at 1:29 AM, Andrei Warkentin <andreiw@motorola.com> wrote:
> Hi Chris,
>
> On Wed, Apr 20, 2011 at 8:44 PM, Chris Ball <cjb@laptop.org> wrote:
>> Hi,
>>
>> On Sat, Apr 16 2011, Andrei Warkentin wrote:
>>> This is the third version of the CMD23 plumbing and host driver support
>>> patch set.
>>>
>>> Changes:
>>> 1) CMD23 support (used for features such as reliable writes) is decoupled
>>>    from general multiblock trans through use of a quirk for affected cards.
>>> 2) Newer Sandisk MMC products are whitelisted along with some known good ones. All other
>>>    MMC products do not use CMD23 for general transfers (seems like safest choice for now).
>>>    SD products unaffected.
>>
>> Just gave this a try on SEM04G, which is in the whitelist, but didn't
>> see a performance increase (or decrease).  What are you using for a
>> testcase?  I'm a little wary of running mmc_test on it.  :)
>>
>
> Can you give me the manufacturing date? This is pretty interesting.
>
> You can see an improvement even if you do something like time block
> writes (O_DIRECT | O_SYNC, of course). But after all synthetic tests I
> measured the time it took to do 40000 SQLite insertions and saw
> something like 30% improvement.
>

And you're trying this on SDHCI I presume, right?

A

Re: CMD23 plumbing and support patchset.

[Andrei Warkentin]

On Thu, Apr 21, 2011 at 1:30 AM, Andrei Warkentin <andreiw@motorola.com> wrote:
> On Thu, Apr 21, 2011 at 1:29 AM, Andrei Warkentin <andreiw@motorola.com> wrote:
>> Hi Chris,
>>
>> On Wed, Apr 20, 2011 at 8:44 PM, Chris Ball <cjb@laptop.org> wrote:
>>> Hi,
>>>
>>> On Sat, Apr 16 2011, Andrei Warkentin wrote:
>>>> This is the third version of the CMD23 plumbing and host driver support
>>>> patch set.
>>>>
>>>> Changes:
>>>> 1) CMD23 support (used for features such as reliable writes) is decoupled
>>>>    from general multiblock trans through use of a quirk for affected cards.
>>>> 2) Newer Sandisk MMC products are whitelisted along with some known good ones. All other
>>>>    MMC products do not use CMD23 for general transfers (seems like safest choice for now).
>>>>    SD products unaffected.
>>>
>>> Just gave this a try on SEM04G, which is in the whitelist, but didn't
>>> see a performance increase (or decrease).  What are you using for a
>>> testcase?  I'm a little wary of running mmc_test on it.  :)
>>>
>>
>> Can you give me the manufacturing date? This is pretty interesting.
>>
>> You can see an improvement even if you do something like time block
>> writes (O_DIRECT | O_SYNC, of course). But after all synthetic tests I
>> measured the time it took to do 40000 SQLite insertions and saw
>> something like 30% improvement.
>>
>
> And you're trying this on SDHCI I presume, right?

Is this on the same board as the other log you sent? Controller is
SDHCI-PXA? If you give me  /sys/block/mmcblk0/device/date,
I'll investigate why this improvement isn't helping you out.

A

Re: CMD23 plumbing and support patchset.

[Chris Ball]

Hi,

On Thu, Apr 21 2011, Andrei Warkentin wrote:
>>> Can you give me the manufacturing date? This is pretty interesting.
>>>
>>> You can see an improvement even if you do something like time block
>>> writes (O_DIRECT | O_SYNC, of course). But after all synthetic tests I
>>> measured the time it took to do 40000 SQLite insertions and saw
>>> something like 30% improvement.
>>>
>>
>> And you're trying this on SDHCI I presume, right?
>
> Is this on the same board as the other log you sent? Controller is
> SDHCI-PXA? If you give me  /sys/block/mmcblk0/device/date,
> I'll investigate why this improvement isn't helping you out.

Yeah, sdhci-pxa:

[root@localhost olpc]# cat /sys/block/mmcblk0/device/date
10/2010

Want to give me a benchmark script so that we can compare speeds
directly?

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: CMD23 plumbing and support patchset.

[Andrei Warkentin]

On Fri, Apr 22, 2011 at 9:51 PM, Chris Ball <cjb@laptop.org> wrote:
> Hi,
>
> On Thu, Apr 21 2011, Andrei Warkentin wrote:
>>>> Can you give me the manufacturing date? This is pretty interesting.
>>>>
>>>> You can see an improvement even if you do something like time block
>>>> writes (O_DIRECT | O_SYNC, of course). But after all synthetic tests I
>>>> measured the time it took to do 40000 SQLite insertions and saw
>>>> something like 30% improvement.
>>>>
>>>
>>> And you're trying this on SDHCI I presume, right?
>>
>> Is this on the same board as the other log you sent? Controller is
>> SDHCI-PXA? If you give me  /sys/block/mmcblk0/device/date,
>> I'll investigate why this improvement isn't helping you out.
>
> Yeah, sdhci-pxa:
>
> [root@localhost olpc]# cat /sys/block/mmcblk0/device/date
> 10/2010
>
> Want to give me a benchmark script so that we can compare speeds
> directly?
>

One more thing to check - is this an eMMC 4.3+ card? (you're going to
have to plumb something in mmc.c to print it). Sandisk says
improvement going to be seen only on 4.3+ cards. Sandisk releases
different generation cards concurrently, and there is no way to tell
from manfid/oemid/date/hwref/fwrev. Ugh.

I'll send another email (in a bit) with test script and a patch to
enable/disable cmd23 use at runtime so you can get better tests than
across
reboot.

A

CMD23 plumbing patchset.

[Andrei Warkentin]

This is the fourth version of the CMD23 plumbing and host driver support.

Changes:
1) Changes MMC white list to a black list.
2) Fixes two stupid bugs in tentative AutoCMD23 patch (sorry Arindam)

ToC:
[[v4] 1/5] MMC: Add/remove quirks conditional support.
[[v4] 2/5] MMC: Use CMD23 for multiblock transfers when we can.
[[v4] 3/5] MMC: Implement MMC_CAP_CMD23 for SDHCI.
[[v4] 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.
[[v4] 5/5] MMC: SDHCI AutoCMD23 support.

A

[[v4] 1/5] MMC: Add/remove quirks conditional support.

[Andrei Warkentin]

Conditional add/remove quirks for MMC and SD.

Cc: cjb@laptop.org
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 include/linux/mmc/card.h |   40 ++++++++++++++++++++++++++++++++++++----
 1 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 72a9868..6a4ed2a 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -233,16 +233,14 @@ struct mmc_fixup {
 		    card->cid.month)
 
 /*
- * This hook just adds a quirk unconditionally.
+ * Unconditionally quirk add/remove.
  */
+
 static inline void __maybe_unused add_quirk(struct mmc_card *card, int data)
 {
 	card->quirks |= data;
 }
 
-/*
- * This hook just removes a quirk unconditionally.
- */
 static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
 {
 	card->quirks &= ~data;
@@ -264,6 +262,40 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
 #define mmc_card_set_blockaddr(c) ((c)->state |= MMC_STATE_BLOCKADDR)
 #define mmc_card_set_ddr_mode(c) ((c)->state |= MMC_STATE_HIGHSPEED_DDR)
 
+/*
+ * Quirk add/remove for MMC products.
+ */
+
+static inline void __maybe_unused add_quirk_mmc(struct mmc_card *card, int data)
+{
+	if (mmc_card_mmc(card))
+		card->quirks |= data;
+}
+
+static inline void __maybe_unused remove_quirk_mmc(struct mmc_card *card,
+						   int data)
+{
+	if (mmc_card_mmc(card))
+		card->quirks &= ~data;
+}
+
+/*
+ * Quirk add/remove for SD products.
+ */
+
+static inline void __maybe_unused add_quirk_sd(struct mmc_card *card, int data)
+{
+	if (mmc_card_sd(card))
+		card->quirks |= data;
+}
+
+static inline void __maybe_unused remove_quirk_sd(struct mmc_card *card,
+						   int data)
+{
+	if (mmc_card_sd(card))
+		card->quirks &= ~data;
+}
+
 static inline int mmc_card_lenient_fn0(const struct mmc_card *c)
 {
 	return c->quirks & MMC_QUIRK_LENIENT_FN0;
-- 
1.7.0.4

[[v4] 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Andrei Warkentin]

CMD23-prefixed instead of open-ended multiblock transfers
have a performance advantage on some MMC cards.

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/card/block.c |  109 +++++++++++++++++++++++++++++++++-------------
 include/linux/mmc/card.h |    1 +
 include/linux/mmc/core.h |    1 +
 include/linux/mmc/host.h |    6 +++
 include/linux/mmc/mmc.h  |    6 +++
 5 files changed, 93 insertions(+), 30 deletions(-)
 mode change 100644 => 100755 drivers/mmc/card/block.c

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
old mode 100644
new mode 100755
index 92e4a00..b91bec2
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -55,10 +55,6 @@ MODULE_ALIAS("mmc:block");
 #define INAND_CMD38_ARG_SECTRIM1 0x81
 #define INAND_CMD38_ARG_SECTRIM2 0x88
 
-#define REL_WRITES_SUPPORTED(card) (mmc_card_mmc((card)) &&	\
-    (((card)->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||	\
-     ((card)->ext_csd.rel_sectors)))
-
 static DEFINE_MUTEX(block_mutex);
 
 /*
@@ -86,6 +82,10 @@ struct mmc_blk_data {
 	struct mmc_queue queue;
 	struct list_head part;
 
+	unsigned int	flags;
+#define MMC_BLK_CMD23	(1 << 0)	/* Can do SET_BLOCK_COUNT for multiblock */
+#define MMC_BLK_REL_WR	(1 << 1)	/* MMC Reliable write support */
+
 	unsigned int	usage;
 	unsigned int	read_only;
 	unsigned int	part_type;
@@ -227,6 +227,7 @@ static const struct block_device_operations mmc_bdops = {
 
 struct mmc_blk_request {
 	struct mmc_request	mrq;
+	struct mmc_command	sbc;
 	struct mmc_command	cmd;
 	struct mmc_command	stop;
 	struct mmc_data		data;
@@ -457,13 +458,10 @@ static int mmc_blk_issue_flush(struct mmc_queue *mq, struct request *req)
  * reliable write can handle, thus finish the request in
  * partial completions.
  */
-static inline int mmc_apply_rel_rw(struct mmc_blk_request *brq,
-				   struct mmc_card *card,
-				   struct request *req)
+static inline void mmc_apply_rel_rw(struct mmc_blk_request *brq,
+				    struct mmc_card *card,
+				    struct request *req)
 {
-	int err;
-	struct mmc_command set_count;
-
 	if (!(card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN)) {
 		/* Legacy mode imposes restrictions on transfers. */
 		if (!IS_ALIGNED(brq->cmd.arg, card->ext_csd.rel_sectors))
@@ -474,16 +472,6 @@ static inline int mmc_apply_rel_rw(struct mmc_blk_request *brq,
 		else if (brq->data.blocks < card->ext_csd.rel_sectors)
 			brq->data.blocks = 1;
 	}
-
-	memset(&set_count, 0, sizeof(struct mmc_command));
-	set_count.opcode = MMC_SET_BLOCK_COUNT;
-	set_count.arg = brq->data.blocks | (1 << 31);
-	set_count.flags = MMC_RSP_R1 | MMC_CMD_AC;
-	err = mmc_wait_for_cmd(card->host, &set_count, 0);
-	if (err)
-		printk(KERN_ERR "%s: error %d SET_BLOCK_COUNT\n",
-		       req->rq_disk->disk_name, err);
-	return err;
 }
 
 static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
@@ -500,7 +488,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 	bool do_rel_wr = ((req->cmd_flags & REQ_FUA) ||
 			  (req->cmd_flags & REQ_META)) &&
 		(rq_data_dir(req) == WRITE) &&
-		REL_WRITES_SUPPORTED(card);
+		(md->flags & MMC_BLK_REL_WR);
 
 	do {
 		struct mmc_command cmd;
@@ -539,11 +527,9 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 
 		if (brq.data.blocks > 1 || do_rel_wr) {
 			/* SPI multiblock writes terminate using a special
-			 * token, not a STOP_TRANSMISSION request. Reliable
-			 * writes use SET_BLOCK_COUNT and do not use a
-			 * STOP_TRANSMISSION request either.
+			 * token, not a STOP_TRANSMISSION request.
 			 */
-			if ((!mmc_host_is_spi(card->host) && !do_rel_wr) ||
+			if (!mmc_host_is_spi(card->host) ||
 			    rq_data_dir(req) == READ)
 				brq.mrq.stop = &brq.stop;
 			readcmd = MMC_READ_MULTIPLE_BLOCK;
@@ -561,8 +547,37 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 			brq.data.flags |= MMC_DATA_WRITE;
 		}
 
-		if (do_rel_wr && mmc_apply_rel_rw(&brq, card, req))
-			goto cmd_err;
+		if (do_rel_wr)
+			mmc_apply_rel_rw(&brq, card, req);
+
+		/*
+		 * Pre-defined multi-block transfers are preferable to
+		 * open ended-ones (and necessary for reliable writes).
+		 * However, it is not sufficient to just send CMD23,
+		 * and avoid the final CMD12, as on an error condition
+		 * CMD12 (stop) needs to be sent anyway. This, coupled
+		 * with Auto-CMD23 enhancements provided by some
+		 * hosts, means that the complexity of dealing
+		 * with this is best left to the host. If CMD23 is
+		 * supported by card and host, we'll fill sbc in and let
+		 * the host deal with handling it correctly. This means
+		 * that for hosts that don't expose MMC_CAP_CMD23, no
+		 * change of behavior will be observed.
+		 *
+		 * N.B: Some MMC cards experience perf degradation.
+		 * We'll avoid using CMD23-bounded multiblock writes for
+		 * these, while retaining features like reliable writes.
+		 */
+
+		if ((md->flags & MMC_BLK_CMD23) &&
+		    mmc_op_multi(brq.cmd.opcode) &&
+		    (do_rel_wr || !(card->quirks & MMC_QUIRK_BLK_NO_CMD23))) {
+			brq.sbc.opcode = MMC_SET_BLOCK_COUNT;
+			brq.sbc.arg = brq.data.blocks |
+				(do_rel_wr ? (1 << 31) : 0);
+			brq.sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
+			brq.mrq.sbc = &brq.sbc;
+		}
 
 		mmc_set_data_timeout(&brq.data, card);
 
@@ -599,7 +614,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 		 * until later as we need to wait for the card to leave
 		 * programming mode even when things go wrong.
 		 */
-		if (brq.cmd.error || brq.data.error || brq.stop.error) {
+		if (brq.sbc.error || brq.cmd.error ||
+		    brq.data.error || brq.stop.error) {
 			if (brq.data.blocks > 1 && rq_data_dir(req) == READ) {
 				/* Redo read one sector at a time */
 				printk(KERN_WARNING "%s: retrying using single "
@@ -610,6 +626,13 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 			status = get_card_status(card, req);
 		}
 
+		if (brq.sbc.error) {
+			printk(KERN_ERR "%s: error %d sending SET_BLOCK_COUNT "
+			       "command, response %#x, card status %#x\n",
+			       req->rq_disk->disk_name, brq.sbc.error,
+			       brq.sbc.resp[0], status);
+		}
+
 		if (brq.cmd.error) {
 			printk(KERN_ERR "%s: error %d sending read/write "
 			       "command, response %#x, card status %#x\n",
@@ -821,8 +844,6 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	md->disk->queue = md->queue.queue;
 	md->disk->driverfs_dev = parent;
 	set_disk_ro(md->disk, md->read_only || default_ro);
-	if (REL_WRITES_SUPPORTED(card))
-		blk_queue_flush(md->queue.queue, REQ_FLUSH | REQ_FUA);
 
 	/*
 	 * As discussed on lkml, GENHD_FL_REMOVABLE should:
@@ -841,6 +862,19 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 
 	blk_queue_logical_block_size(md->queue.queue, 512);
 	set_capacity(md->disk, size);
+
+	if (mmc_host_cmd23(card->host) &&
+	    mmc_card_mmc(card))
+		md->flags |= MMC_BLK_CMD23;
+
+	if (mmc_card_mmc(card) &&
+	    md->flags & MMC_BLK_CMD23 &&
+	    ((card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||
+	     card->ext_csd.rel_sectors)) {
+		md->flags |= MMC_BLK_REL_WR;
+		blk_queue_flush(md->queue.queue, REQ_FLUSH | REQ_FUA);
+	}
+
 	return md;
 
  err_putdisk:
@@ -995,6 +1029,21 @@ static const struct mmc_fixup blk_fixups[] =
 	MMC_FIXUP("SEM08G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
 	MMC_FIXUP("SEM16G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
 	MMC_FIXUP("SEM32G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
+
+	/*
+	 * Some MMC cards experience performance degradation with CMD23
+	 * instead of CMD12-bounded multiblock transfers. For now we'll
+	 * black list what's bad...
+	 * - Certain Toshiba cards.
+	 *
+	 * N.B. This doesn't affect SD cards.
+	 */
+	MMC_FIXUP("MMC08G", 0x11, CID_OEMID_ANY, add_quirk_mmc,
+		  MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("MMC16G", 0x11, CID_OEMID_ANY, add_quirk_mmc,
+		  MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("MMC32G", 0x11, CID_OEMID_ANY, add_quirk_mmc,
+		  MMC_QUIRK_BLK_NO_CMD23),
 	END_FIXUP
 };
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 6a4ed2a..c758181 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -134,6 +134,7 @@ struct mmc_card {
 #define MMC_QUIRK_NONSTD_FUNC_IF (1<<4)		/* SDIO card has nonstd function interfaces */
 #define MMC_QUIRK_DISABLE_CD	(1<<5)		/* disconnect CD/DAT[3] resistor */
 #define MMC_QUIRK_INAND_CMD38	(1<<6)		/* iNAND devices have broken CMD38 */
+#define MMC_QUIRK_BLK_NO_CMD23	(1<<7)		/* Avoid CMD23 for regular multiblock */
 
 	unsigned int		erase_size;	/* erase size in sectors */
  	unsigned int		erase_shift;	/* if erase unit is power 2 */
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index f8e4bcb..55d7fde 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -120,6 +120,7 @@ struct mmc_data {
 };
 
 struct mmc_request {
+	struct mmc_command	*sbc;		/* SET_BLOCK_COUNT for multiblock */
 	struct mmc_command	*cmd;
 	struct mmc_data		*data;
 	struct mmc_command	*stop;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 4f705eb..ba34fc5 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -173,6 +173,7 @@ struct mmc_host {
 						/* DDR mode at 1.2V */
 #define MMC_CAP_POWER_OFF_CARD	(1 << 13)	/* Can power off after boot */
 #define MMC_CAP_BUS_WIDTH_TEST	(1 << 14)	/* CMD14/CMD19 bus width ok */
+#define MMC_CAP_CMD23		(1 << 15)	/* CMD23 supported */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 
@@ -330,5 +331,10 @@ static inline int mmc_card_wake_sdio_irq(struct mmc_host *host)
 {
 	return host->pm_flags & MMC_PM_WAKE_SDIO_IRQ;
 }
+
+static inline int mmc_host_cmd23(struct mmc_host *host)
+{
+	return host->caps & MMC_CAP_CMD23;
+}
 #endif
 
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 373b2bf..ea74168 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -82,6 +82,12 @@
 #define MMC_APP_CMD              55   /* ac   [31:16] RCA        R1  */
 #define MMC_GEN_CMD              56   /* adtc [0] RD/WR          R1  */
 
+static inline bool mmc_op_multi(u32 opcode)
+{
+	return opcode == MMC_WRITE_MULTIPLE_BLOCK ||
+		opcode == MMC_READ_MULTIPLE_BLOCK;
+}
+
 /*
  * MMC_SWITCH argument format:
  *
-- 
1.7.0.4

[[v4] 3/5] MMC: Implement MMC_CAP_CMD23 for SDHCI.

[Andrei Warkentin]

Implements support for multiblock transfers bounded
by SET_BLOCK_COUNT (CMD23).

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/host/sdhci.c  |   70 ++++++++++++++++++++++++++++++++++-----------
 include/linux/mmc/sdhci.h |    1 +
 2 files changed, 54 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 94793f2..41c987d 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -822,9 +822,10 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 }
 
 static void sdhci_set_transfer_mode(struct sdhci_host *host,
-	struct mmc_data *data)
+	struct mmc_command *cmd)
 {
 	u16 mode;
+	struct mmc_data *data = cmd->data;
 
 	if (data == NULL)
 		return;
@@ -832,11 +833,17 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
 	WARN_ON(!host->data);
 
 	mode = SDHCI_TRNS_BLK_CNT_EN;
-	if (data->blocks > 1) {
-		if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
-			mode |= SDHCI_TRNS_MULTI | SDHCI_TRNS_ACMD12;
-		else
-			mode |= SDHCI_TRNS_MULTI;
+	if (mmc_op_multi(cmd->opcode) ||
+	    data->blocks > 1) {
+		mode |= SDHCI_TRNS_MULTI;
+
+		/*
+		 * If we are sending CMD23, CMD12 never gets sent
+		 * on successful completion (so no Auto-CMD12).
+		 */
+		if (!host->mrq->sbc &&
+		    host->flags & SDHCI_ACMD12)
+			mode |= SDHCI_TRNS_ACMD12;
 	}
 	if (data->flags & MMC_DATA_READ)
 		mode |= SDHCI_TRNS_READ;
@@ -877,7 +884,15 @@ static void sdhci_finish_data(struct sdhci_host *host)
 	else
 		data->bytes_xfered = data->blksz * data->blocks;
 
-	if (data->stop) {
+	/*
+	 * Need to send CMD12 if -
+	 * a) open-ended multiblock transfer (no CMD23)
+	 * b) error in multiblock transfer
+	 */
+	if (data->stop &&
+	    (data->error ||
+	     !host->mrq->sbc)) {
+
 		/*
 		 * The controller needs a reset of internal state machines
 		 * upon error conditions.
@@ -933,7 +948,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 
 	sdhci_writel(host, cmd->arg, SDHCI_ARGUMENT);
 
-	sdhci_set_transfer_mode(host, cmd->data);
+	sdhci_set_transfer_mode(host, cmd);
 
 	if ((cmd->flags & MMC_RSP_136) && (cmd->flags & MMC_RSP_BUSY)) {
 		printk(KERN_ERR "%s: Unsupported response type!\n",
@@ -986,13 +1001,21 @@ static void sdhci_finish_command(struct sdhci_host *host)
 
 	host->cmd->error = 0;
 
-	if (host->data && host->data_early)
-		sdhci_finish_data(host);
+	/* Finished CMD23, now send actual command. */
+	if (host->cmd == host->mrq->sbc) {
+		host->cmd = NULL;
+		sdhci_send_command(host, host->mrq->cmd);
+	} else {
 
-	if (!host->cmd->data)
-		tasklet_schedule(&host->finish_tasklet);
+		/* Processed actual command. */
+		if (host->data && host->data_early)
+			sdhci_finish_data(host);
 
-	host->cmd = NULL;
+		if (!host->cmd->data)
+			tasklet_schedule(&host->finish_tasklet);
+
+		host->cmd = NULL;
+	}
 }
 
 static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
@@ -1140,7 +1163,12 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 #ifndef SDHCI_USE_LEDS_CLASS
 	sdhci_activate_led(host);
 #endif
-	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12) {
+
+	/*
+	 * Ensure we don't send the STOP for non-SET_BLOCK_COUNTED
+	 * requests if Auto-CMD12 is enabled.
+	 */
+	if (!mrq->sbc && (host->flags & SDHCI_ACMD12)) {
 		if (mrq->stop) {
 			mrq->data->stop = NULL;
 			mrq->stop = NULL;
@@ -1159,8 +1187,13 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	if (!present || host->flags & SDHCI_DEVICE_DEAD) {
 		host->mrq->cmd->error = -ENOMEDIUM;
 		tasklet_schedule(&host->finish_tasklet);
-	} else
-		sdhci_send_command(host, mrq->cmd);
+	} else {
+
+		if (mrq->sbc)
+			sdhci_send_command(host, mrq->sbc);
+		else
+			sdhci_send_command(host, mrq->cmd);
+	}
 
 	mmiowb();
 	spin_unlock_irqrestore(&host->lock, flags);
@@ -1909,7 +1942,10 @@ int sdhci_add_host(struct sdhci_host *host)
 		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;
 
 	mmc->f_max = host->max_clk;
-	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE;
+	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;
+
+	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
+		host->flags |= SDHCI_ACMD12;
 
 	/*
 	 * A controller may support 8-bit width, but the board itself
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 83bd9f7..c54ac14 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -109,6 +109,7 @@ struct sdhci_host {
 #define SDHCI_USE_ADMA		(1<<1)	/* Host is ADMA capable */
 #define SDHCI_REQ_USE_DMA	(1<<2)	/* Use DMA for this req. */
 #define SDHCI_DEVICE_DEAD	(1<<3)	/* Device unresponsive */
+#define SDHCI_ACMD12		(1<<4)	/* Auto CMD12 support */
 
 	unsigned int version;	/* SDHCI spec. version */
 
-- 
1.7.0.4

[[v4] 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.

[Andrei Warkentin]

SD cards operating at UHS104 or better support SET_BLOCK_COUNT.

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/card/block.c |    9 ++++++---
 drivers/mmc/core/sd.c    |    7 +++++++
 include/linux/mmc/card.h |    4 ++++
 include/linux/mmc/sd.h   |    2 +-
 4 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index b91bec2..96250cf 100755
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -863,9 +863,12 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	blk_queue_logical_block_size(md->queue.queue, 512);
 	set_capacity(md->disk, size);
 
-	if (mmc_host_cmd23(card->host) &&
-	    mmc_card_mmc(card))
-		md->flags |= MMC_BLK_CMD23;
+	if (mmc_host_cmd23(card->host)) {
+		if (mmc_card_mmc(card) ||
+		    (mmc_card_sd(card) &&
+		     card->scr.cmds & SD_SCR_CMD23_SUPPORT))
+			md->flags |= MMC_BLK_CMD23;
+	}
 
 	if (mmc_card_mmc(card) &&
 	    md->flags & MMC_BLK_CMD23 &&
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 6dac89f..a3d771a 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -195,6 +195,13 @@ static int mmc_decode_scr(struct mmc_card *card)
 	else
 		card->erased_byte = 0x0;
 
+	if (scr->sda_vsn == SCR_SPEC_VER_2) {
+
+		/* Check if Physical Layer Spec v3.0X is supported. */
+		scr->sda_spec3 = UNSTUFF_BITS(resp, 47, 1);
+		if (scr->sda_spec3)
+			scr->cmds = UNSTUFF_BITS(resp, 32, 2);
+	}
 	return 0;
 }
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index c758181..e6b2e6f 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -67,9 +67,13 @@ struct mmc_ext_csd {
 
 struct sd_scr {
 	unsigned char		sda_vsn;
+	unsigned char		sda_spec3;
 	unsigned char		bus_widths;
 #define SD_SCR_BUS_WIDTH_1	(1<<0)
 #define SD_SCR_BUS_WIDTH_4	(1<<2)
+	unsigned char		cmds;
+#define SD_SCR_CMD20_SUPPORT   (1<<0)
+#define SD_SCR_CMD23_SUPPORT   (1<<1)
 };
 
 struct sd_ssr {
diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h
index 3fd85e0..9fe0689 100644
--- a/include/linux/mmc/sd.h
+++ b/include/linux/mmc/sd.h
@@ -59,7 +59,7 @@
 
 #define SCR_SPEC_VER_0		0	/* Implements system specification 1.0 - 1.01 */
 #define SCR_SPEC_VER_1		1	/* Implements system specification 1.10 */
-#define SCR_SPEC_VER_2		2	/* Implements system specification 2.00 */
+#define SCR_SPEC_VER_2		2	/* Implements system specification 2.00-3.0X */
 
 /*
  * SD bus widths
-- 
1.7.0.4

[[v4] 5/5] MMC: SDHCI AutoCMD23 support.

[Andrei Warkentin]

Enables Auto-CMD23 support where available (SDHCI 3.0 controllers)

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Cc: subhashj@codeaurora.org
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/host/sdhci.c  |   17 ++++++++++++++++-
 drivers/mmc/host/sdhci.h  |    2 ++
 include/linux/mmc/sdhci.h |    1 +
 3 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 41c987d..bf9aedb 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -844,7 +844,13 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
 		if (!host->mrq->sbc &&
 		    host->flags & SDHCI_ACMD12)
 			mode |= SDHCI_TRNS_ACMD12;
+		else if (host->mrq->sbc &&
+			 host->flags & SDHCI_ACMD23) {
+			mode |= SDHCI_TRNS_ACMD23;
+			sdhci_writel(host, host->mrq->sbc->arg, SDHCI_ARGUMENT2);
+		}
 	}
+
 	if (data->flags & MMC_DATA_READ)
 		mode |= SDHCI_TRNS_READ;
 	if (host->flags & SDHCI_REQ_USE_DMA)
@@ -1189,7 +1195,7 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 		tasklet_schedule(&host->finish_tasklet);
 	} else {
 
-		if (mrq->sbc)
+		if (mrq->sbc && !(host->flags & SDHCI_ACMD23))
 			sdhci_send_command(host, mrq->sbc);
 		else
 			sdhci_send_command(host, mrq->cmd);
@@ -1947,6 +1953,15 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
 		host->flags |= SDHCI_ACMD12;
 
+	/* Auto-CMD23 stuff only works in ADMA or PIO. */
+	if ((host->version == SDHCI_SPEC_300) &&
+	    ((host->flags & SDHCI_USE_ADMA) ||
+	     !(host->flags & SDHCI_REQ_USE_DMA))) {
+		host->flags |= SDHCI_ACMD23;
+		printk(KERN_INFO "%s: Auto-CMD23 available\n", mmc_hostname(mmc));
+	} else
+		printk(KERN_INFO "%s: Auto-CMD23 unavailable\n", mmc_hostname(mmc));
+
 	/*
 	 * A controller may support 8-bit width, but the board itself
 	 * might not have the pins brought out.  Boards that support
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 85750a9..2b3fb76 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -25,6 +25,7 @@
  */
 
 #define SDHCI_DMA_ADDRESS	0x00
+#define SDHCI_ARGUMENT2		0x00
 
 #define SDHCI_BLOCK_SIZE	0x04
 #define  SDHCI_MAKE_BLKSZ(dma, blksz) (((dma & 0x7) << 12) | (blksz & 0xFFF))
@@ -37,6 +38,7 @@
 #define  SDHCI_TRNS_DMA		0x01
 #define  SDHCI_TRNS_BLK_CNT_EN	0x02
 #define  SDHCI_TRNS_ACMD12	0x04
+#define  SDHCI_TRNS_ACMD23	0x08
 #define  SDHCI_TRNS_READ	0x10
 #define  SDHCI_TRNS_MULTI	0x20
 
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index c54ac14..d91585a 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -110,6 +110,7 @@ struct sdhci_host {
 #define SDHCI_REQ_USE_DMA	(1<<2)	/* Use DMA for this req. */
 #define SDHCI_DEVICE_DEAD	(1<<3)	/* Device unresponsive */
 #define SDHCI_ACMD12		(1<<4)	/* Auto CMD12 support */
+#define SDHCI_ACMD23		(1<<5)	/* Auto CMD23 support */
 
 	unsigned int version;	/* SDHCI spec. version */
 
-- 
1.7.0.4

RE: [[v4] 5/5] MMC: SDHCI AutoCMD23 support.

[Nath, Arindam]

Hi Andrei,

Is there any more change to your V4 than the ones we discussed yesterday? I did the mod we discussed, but seems like the driver only sends CMD23 now, not Auto CMD23.

Thanks,
Arindam

> -----Original Message-----
> From: Andrei Warkentin [mailto:andreiw@motorola.com]
> Sent: Wednesday, April 27, 2011 7:41 AM
> To: linux-mmc@vger.kernel.org
> Cc: Andrei Warkentin; Nath, Arindam; cjb@laptop.org; arnd@arndb.de;
> subhashj@codeaurora.org
> Subject: [[v4] 5/5] MMC: SDHCI AutoCMD23 support.
> 
> Enables Auto-CMD23 support where available (SDHCI 3.0 controllers)
> 
> Cc: arindam.nath@amd.com
> Cc: cjb@laptop.org
> Cc: arnd@arndb.de
> Cc: subhashj@codeaurora.org
> Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
> ---
>  drivers/mmc/host/sdhci.c  |   17 ++++++++++++++++-
>  drivers/mmc/host/sdhci.h  |    2 ++
>  include/linux/mmc/sdhci.h |    1 +
>  3 files changed, 19 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 41c987d..bf9aedb 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -844,7 +844,13 @@ static void sdhci_set_transfer_mode(struct
> sdhci_host *host,
>  		if (!host->mrq->sbc &&
>  		    host->flags & SDHCI_ACMD12)
>  			mode |= SDHCI_TRNS_ACMD12;
> +		else if (host->mrq->sbc &&
> +			 host->flags & SDHCI_ACMD23) {
> +			mode |= SDHCI_TRNS_ACMD23;
> +			sdhci_writel(host, host->mrq->sbc->arg,
> SDHCI_ARGUMENT2);
> +		}
>  	}
> +
>  	if (data->flags & MMC_DATA_READ)
>  		mode |= SDHCI_TRNS_READ;
>  	if (host->flags & SDHCI_REQ_USE_DMA)
> @@ -1189,7 +1195,7 @@ static void sdhci_request(struct mmc_host *mmc,
> struct mmc_request *mrq)
>  		tasklet_schedule(&host->finish_tasklet);
>  	} else {
> 
> -		if (mrq->sbc)
> +		if (mrq->sbc && !(host->flags & SDHCI_ACMD23))
>  			sdhci_send_command(host, mrq->sbc);
>  		else
>  			sdhci_send_command(host, mrq->cmd);
> @@ -1947,6 +1953,15 @@ int sdhci_add_host(struct sdhci_host *host)
>  	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
>  		host->flags |= SDHCI_ACMD12;
> 
> +	/* Auto-CMD23 stuff only works in ADMA or PIO. */
> +	if ((host->version == SDHCI_SPEC_300) &&
> +	    ((host->flags & SDHCI_USE_ADMA) ||
> +	     !(host->flags & SDHCI_REQ_USE_DMA))) {
> +		host->flags |= SDHCI_ACMD23;
> +		printk(KERN_INFO "%s: Auto-CMD23 available\n",
> mmc_hostname(mmc));
> +	} else
> +		printk(KERN_INFO "%s: Auto-CMD23 unavailable\n",
> mmc_hostname(mmc));
> +
>  	/*
>  	 * A controller may support 8-bit width, but the board itself
>  	 * might not have the pins brought out.  Boards that support
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 85750a9..2b3fb76 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -25,6 +25,7 @@
>   */
> 
>  #define SDHCI_DMA_ADDRESS	0x00
> +#define SDHCI_ARGUMENT2		0x00
> 
>  #define SDHCI_BLOCK_SIZE	0x04
>  #define  SDHCI_MAKE_BLKSZ(dma, blksz) (((dma & 0x7) << 12) | (blksz &
> 0xFFF))
> @@ -37,6 +38,7 @@
>  #define  SDHCI_TRNS_DMA		0x01
>  #define  SDHCI_TRNS_BLK_CNT_EN	0x02
>  #define  SDHCI_TRNS_ACMD12	0x04
> +#define  SDHCI_TRNS_ACMD23	0x08
>  #define  SDHCI_TRNS_READ	0x10
>  #define  SDHCI_TRNS_MULTI	0x20
> 
> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
> index c54ac14..d91585a 100644
> --- a/include/linux/mmc/sdhci.h
> +++ b/include/linux/mmc/sdhci.h
> @@ -110,6 +110,7 @@ struct sdhci_host {
>  #define SDHCI_REQ_USE_DMA	(1<<2)	/* Use DMA for this req. */
>  #define SDHCI_DEVICE_DEAD	(1<<3)	/* Device unresponsive */
>  #define SDHCI_ACMD12		(1<<4)	/* Auto CMD12 support */
> +#define SDHCI_ACMD23		(1<<5)	/* Auto CMD23 support */
> 
>  	unsigned int version;	/* SDHCI spec. version */
> 
> --
> 1.7.0.4
> 

Re: [[v4] 5/5] MMC: SDHCI AutoCMD23 support.

[Andrei Warkentin]

Hi Arindam,

On Wed, Apr 27, 2011 at 12:49 AM, Nath, Arindam <Arindam.Nath@amd.com> wrote:
> Hi Andrei,
>
> Is there any more change to your V4 than the ones we discussed yesterday? I did the mod we discussed, but seems like the driver only sends CMD23 now, not Auto CMD23.
>

Two things V4 does, both of which we discussed yesterday -
1) Make sure the code that set SDHCI_ACMD23 is always run (got rid of
the Kconfig conditional)
2) Make sure value written into ARGUMENT2 comes from sbc struct.

Only (1) should have affected you. (2) would have broken rel. writes.
I put in an info print telling you if you have auto-cmd23 available or
not. If you see CMD23 getting sent, then AutoCMD23 was not detected as
a feature. Weird. SDHCI_ACMD23 is set if
a) Host controller is version 3
b) You're using ADMA or no DMA.

A

RE: [[v4] 5/5] MMC: SDHCI AutoCMD23 support.

[Nath, Arindam]

Guess I will have to hand modify my code based on your V4 patches again, I don't think your patches are based on top of mine.

Thanks,
Arindam

> -----Original Message-----
> From: Andrei Warkentin [mailto:andreiw@motorola.com]
> Sent: Wednesday, April 27, 2011 11:30 AM
> To: Nath, Arindam
> Cc: linux-mmc@vger.kernel.org; cjb@laptop.org; arnd@arndb.de;
> subhashj@codeaurora.org
> Subject: Re: [[v4] 5/5] MMC: SDHCI AutoCMD23 support.
> 
> Hi Arindam,
> 
> On Wed, Apr 27, 2011 at 12:49 AM, Nath, Arindam <Arindam.Nath@amd.com>
> wrote:
> > Hi Andrei,
> >
> > Is there any more change to your V4 than the ones we discussed
> yesterday? I did the mod we discussed, but seems like the driver only
> sends CMD23 now, not Auto CMD23.
> >
> 
> Two things V4 does, both of which we discussed yesterday -
> 1) Make sure the code that set SDHCI_ACMD23 is always run (got rid of
> the Kconfig conditional)
> 2) Make sure value written into ARGUMENT2 comes from sbc struct.
> 
> Only (1) should have affected you. (2) would have broken rel. writes.
> I put in an info print telling you if you have auto-cmd23 available or
> not. If you see CMD23 getting sent, then AutoCMD23 was not detected as
> a feature. Weird. SDHCI_ACMD23 is set if
> a) Host controller is version 3
> b) You're using ADMA or no DMA.
> 
> A

Re: [[v4] 5/5] MMC: SDHCI AutoCMD23 support.

[Andrei Warkentin]

On Wed, Apr 27, 2011 at 1:02 AM, Nath, Arindam <Arindam.Nath@amd.com> wrote:
> Guess I will have to hand modify my code based on your V4 patches again, I don't think your patches are based on top of mine.
>
> Thanks,
> Arindam
>

Just verify these blobs look similar -

+               else if (host->mrq->sbc &&
+                        host->flags & SDHCI_ACMD23) {
+                       mode |= SDHCI_TRNS_ACMD23;
+                       sdhci_writel(host, host->mrq->sbc->arg,
SDHCI_ARGUMENT2);
+               }

and

               host->flags |= SDHCI_ACMD12;

+       /* Auto-CMD23 stuff only works in ADMA or PIO. */
+       if ((host->version == SDHCI_SPEC_300) &&
+           ((host->flags & SDHCI_USE_ADMA) ||
+            !(host->flags & SDHCI_REQ_USE_DMA))) {
+               host->flags |= SDHCI_ACMD23;
+               printk(KERN_INFO "%s: Auto-CMD23 available\n",
mmc_hostname(mmc));
+       } else
+               printk(KERN_INFO "%s: Auto-CMD23 unavailable\n",
mmc_hostname(mmc));
+
       /*
        * A controller may support 8-bit width, but the board itself
        * might not have the pins brought out.  Boards that support

I apologize for the fuss.

A

RE: [[v4] 5/5] MMC: SDHCI AutoCMD23 support.

[Nath, Arindam]

Hi Andrei,

The controller is able to send Auto CMD23 with your patch.

Thanks,
Arindam

> -----Original Message-----
> From: Andrei Warkentin [mailto:andreiw@motorola.com]
> Sent: Wednesday, April 27, 2011 11:36 AM
> To: Nath, Arindam
> Cc: linux-mmc@vger.kernel.org; cjb@laptop.org; arnd@arndb.de;
> subhashj@codeaurora.org
> Subject: Re: [[v4] 5/5] MMC: SDHCI AutoCMD23 support.
> 
> On Wed, Apr 27, 2011 at 1:02 AM, Nath, Arindam <Arindam.Nath@amd.com>
> wrote:
> > Guess I will have to hand modify my code based on your V4 patches
> again, I don't think your patches are based on top of mine.
> >
> > Thanks,
> > Arindam
> >
> 
> Just verify these blobs look similar -
> 
> +               else if (host->mrq->sbc &&
> +                        host->flags & SDHCI_ACMD23) {
> +                       mode |= SDHCI_TRNS_ACMD23;
> +                       sdhci_writel(host, host->mrq->sbc->arg,
> SDHCI_ARGUMENT2);
> +               }
> 
> and
> 
>                host->flags |= SDHCI_ACMD12;
> 
> +       /* Auto-CMD23 stuff only works in ADMA or PIO. */
> +       if ((host->version == SDHCI_SPEC_300) &&
> +           ((host->flags & SDHCI_USE_ADMA) ||
> +            !(host->flags & SDHCI_REQ_USE_DMA))) {
> +               host->flags |= SDHCI_ACMD23;
> +               printk(KERN_INFO "%s: Auto-CMD23 available\n",
> mmc_hostname(mmc));
> +       } else
> +               printk(KERN_INFO "%s: Auto-CMD23 unavailable\n",
> mmc_hostname(mmc));
> +
>        /*
>         * A controller may support 8-bit width, but the board itself
>         * might not have the pins brought out.  Boards that support
> 
> I apologize for the fuss.
> 
> A

Re: [[v4] 5/5] MMC: SDHCI AutoCMD23 support.

[Andrei Warkentin]

On Thu, Apr 28, 2011 at 3:34 AM, Nath, Arindam <Arindam.Nath@amd.com> wrote:
> Hi Andrei,
>
> The controller is able to send Auto CMD23 with your patch.
>
> Thanks,
> Arindam
>

Whoo hoo :-). This just about made my day... Thanks a lot Arindam, I
really appreciate you taking your time to test this out!

So is that an Ack?

A

RE: [[v4] 5/5] MMC: SDHCI AutoCMD23 support.

[Nath, Arindam]

Hi Andrei,


> -----Original Message-----
> From: Andrei Warkentin [mailto:andreiw@motorola.com]
> Sent: Friday, April 29, 2011 12:40 AM
> To: Nath, Arindam
> Cc: linux-mmc@vger.kernel.org; cjb@laptop.org; arnd@arndb.de;
> subhashj@codeaurora.org; Xu, Suki
> Subject: Re: [[v4] 5/5] MMC: SDHCI AutoCMD23 support.
> 
> On Thu, Apr 28, 2011 at 3:34 AM, Nath, Arindam <Arindam.Nath@amd.com>
> wrote:
> > Hi Andrei,
> >
> > The controller is able to send Auto CMD23 with your patch.
> >
> > Thanks,
> > Arindam
> >
> 
> Whoo hoo :-). This just about made my day... Thanks a lot Arindam, I
> really appreciate you taking your time to test this out!
> 
> So is that an Ack?

I am not sure whether I qualify for an Acked-by, but I can give you Tested-by.

Tested-by: Arindam Nath <arindam.nath@amd.com>

Thanks,
Arindam

Re: CMD23 plumbing patchset.

[Andrei Warkentin]

On Tue, Apr 26, 2011 at 9:10 PM, Andrei Warkentin <andreiw@motorola.com> wrote:
> This is the fourth version of the CMD23 plumbing and host driver support.
>
> Changes:
> 1) Changes MMC white list to a black list.
> 2) Fixes two stupid bugs in tentative AutoCMD23 patch (sorry Arindam)
>
> ToC:
> [[v4] 1/5] MMC: Add/remove quirks conditional support.
> [[v4] 2/5] MMC: Use CMD23 for multiblock transfers when we can.
> [[v4] 3/5] MMC: Implement MMC_CAP_CMD23 for SDHCI.
> [[v4] 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.
> [[v4] 5/5] MMC: SDHCI AutoCMD23 support.
>

We've a tested-by Arindam Nath... Does anyone else have feedback?

A

Re: [[v4] 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Jaehoon Chung]

Hi Andrei

Andrei Warkentin wrote:
> CMD23-prefixed instead of open-ended multiblock transfers
> have a performance advantage on some MMC cards.
> 
you mentioned about "some MMC cards". 
Conversely, that means the some card didn't have a performance advantage?

Did you find the performance advantage? 
if you found the advantage and you can tell me, 
i want to know what do you have the some MMC cards..

Regards,
Jaehoon Chung

Re: [[v4] 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Andrei Warkentin]

On Wed, May 18, 2011 at 9:37 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi Andrei
>
> Andrei Warkentin wrote:
>> CMD23-prefixed instead of open-ended multiblock transfers
>> have a performance advantage on some MMC cards.
>>
> you mentioned about "some MMC cards".
> Conversely, that means the some card didn't have a performance advantage?
>
> Did you find the performance advantage?
> if you found the advantage and you can tell me,
> i want to know what do you have the some MMC cards..
>

I've tested this on a Sandisk eMMC where I saw as good as
a 50% improvement on writes (30% real-life use cases). This was a
SEM32G 4.3+ part.

A

CMD23 plumbing patchset

[Andrei Warkentin]

This is the fifth version of the CMD23 plumbing and host driver support.

Changes:
- rebased on top of latest mmc-next
- have a tested-by by Arindam

ToC:
[v5 1/5] MMC: Add/remove quirks conditional support.
[v5 2/5] MMC: Use CMD23 for multiblock transfers when we can.
[v5 3/5] MMC: Implement MMC_CAP_CMD23 for SDHCI.
[v5 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.
[v5 5/5] MMC: SDHCI AutoCMD23 support.

Thanks,
A

[v5 1/5] MMC: Add/remove quirks conditional support.

[Andrei Warkentin]

Conditional add/remove quirks for MMC and SD.

Cc: cjb@laptop.org
Cc: malchev@google.com
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 include/linux/mmc/card.h |   40 ++++++++++++++++++++++++++++++++++++----
 1 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 4910dec..7190aa2 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -273,16 +273,14 @@ struct mmc_fixup {
 		    card->cid.month)
 
 /*
- * This hook just adds a quirk unconditionally.
+ * Unconditionally quirk add/remove.
  */
+
 static inline void __maybe_unused add_quirk(struct mmc_card *card, int data)
 {
 	card->quirks |= data;
 }
 
-/*
- * This hook just removes a quirk unconditionally.
- */
 static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
 {
 	card->quirks &= ~data;
@@ -308,6 +306,40 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
 #define mmc_sd_card_set_uhs(c) ((c)->state |= MMC_STATE_ULTRAHIGHSPEED)
 #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC)
 
+/*
+ * Quirk add/remove for MMC products.
+ */
+
+static inline void __maybe_unused add_quirk_mmc(struct mmc_card *card, int data)
+{
+	if (mmc_card_mmc(card))
+		card->quirks |= data;
+}
+
+static inline void __maybe_unused remove_quirk_mmc(struct mmc_card *card,
+						   int data)
+{
+	if (mmc_card_mmc(card))
+		card->quirks &= ~data;
+}
+
+/*
+ * Quirk add/remove for SD products.
+ */
+
+static inline void __maybe_unused add_quirk_sd(struct mmc_card *card, int data)
+{
+	if (mmc_card_sd(card))
+		card->quirks |= data;
+}
+
+static inline void __maybe_unused remove_quirk_sd(struct mmc_card *card,
+						   int data)
+{
+	if (mmc_card_sd(card))
+		card->quirks &= ~data;
+}
+
 static inline int mmc_card_lenient_fn0(const struct mmc_card *c)
 {
 	return c->quirks & MMC_QUIRK_LENIENT_FN0;
-- 
1.7.0.4

[v5 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Andrei Warkentin]

CMD23-prefixed instead of open-ended multiblock transfers
have a performance advantage on some MMC cards.

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Cc: malchev@google.com
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/card/block.c |  108 +++++++++++++++++++++++++++++++++------------
 include/linux/mmc/card.h |    1 +
 include/linux/mmc/core.h |    1 +
 include/linux/mmc/host.h |    6 +++
 include/linux/mmc/mmc.h  |    6 +++
 5 files changed, 93 insertions(+), 29 deletions(-)
 mode change 100644 => 100755 drivers/mmc/card/block.c

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
old mode 100644
new mode 100755
index 126c7f4..a380acc
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -59,10 +59,6 @@ MODULE_ALIAS("mmc:block");
 #define INAND_CMD38_ARG_SECTRIM1 0x81
 #define INAND_CMD38_ARG_SECTRIM2 0x88
 
-#define REL_WRITES_SUPPORTED(card) (mmc_card_mmc((card)) &&	\
-    (((card)->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||	\
-     ((card)->ext_csd.rel_sectors)))
-
 static DEFINE_MUTEX(block_mutex);
 
 /*
@@ -90,6 +86,10 @@ struct mmc_blk_data {
 	struct mmc_queue queue;
 	struct list_head part;
 
+	unsigned int	flags;
+#define MMC_BLK_CMD23	(1 << 0)	/* Can do SET_BLOCK_COUNT for multiblock */
+#define MMC_BLK_REL_WR	(1 << 1)	/* MMC Reliable write support */
+
 	unsigned int	usage;
 	unsigned int	read_only;
 	unsigned int	part_type;
@@ -429,6 +429,7 @@ static const struct block_device_operations mmc_bdops = {
 
 struct mmc_blk_request {
 	struct mmc_request	mrq;
+	struct mmc_command	sbc;
 	struct mmc_command	cmd;
 	struct mmc_command	stop;
 	struct mmc_data		data;
@@ -652,13 +653,10 @@ static int mmc_blk_issue_flush(struct mmc_queue *mq, struct request *req)
  * reliable write can handle, thus finish the request in
  * partial completions.
  */
-static inline int mmc_apply_rel_rw(struct mmc_blk_request *brq,
-				   struct mmc_card *card,
-				   struct request *req)
+static inline void mmc_apply_rel_rw(struct mmc_blk_request *brq,
+				    struct mmc_card *card,
+				    struct request *req)
 {
-	int err;
-	struct mmc_command set_count = {0};
-
 	if (!(card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN)) {
 		/* Legacy mode imposes restrictions on transfers. */
 		if (!IS_ALIGNED(brq->cmd.arg, card->ext_csd.rel_sectors))
@@ -669,15 +667,6 @@ static inline int mmc_apply_rel_rw(struct mmc_blk_request *brq,
 		else if (brq->data.blocks < card->ext_csd.rel_sectors)
 			brq->data.blocks = 1;
 	}
-
-	set_count.opcode = MMC_SET_BLOCK_COUNT;
-	set_count.arg = brq->data.blocks | (1 << 31);
-	set_count.flags = MMC_RSP_R1 | MMC_CMD_AC;
-	err = mmc_wait_for_cmd(card->host, &set_count, 0);
-	if (err)
-		printk(KERN_ERR "%s: error %d SET_BLOCK_COUNT\n",
-		       req->rq_disk->disk_name, err);
-	return err;
 }
 
 static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
@@ -694,7 +683,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 	bool do_rel_wr = ((req->cmd_flags & REQ_FUA) ||
 			  (req->cmd_flags & REQ_META)) &&
 		(rq_data_dir(req) == WRITE) &&
-		REL_WRITES_SUPPORTED(card);
+		(md->flags & MMC_BLK_REL_WR);
 
 	do {
 		struct mmc_command cmd = {0};
@@ -732,11 +721,9 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 
 		if (brq.data.blocks > 1 || do_rel_wr) {
 			/* SPI multiblock writes terminate using a special
-			 * token, not a STOP_TRANSMISSION request. Reliable
-			 * writes use SET_BLOCK_COUNT and do not use a
-			 * STOP_TRANSMISSION request either.
+			 * token, not a STOP_TRANSMISSION request.
 			 */
-			if ((!mmc_host_is_spi(card->host) && !do_rel_wr) ||
+			if (!mmc_host_is_spi(card->host) ||
 			    rq_data_dir(req) == READ)
 				brq.mrq.stop = &brq.stop;
 			readcmd = MMC_READ_MULTIPLE_BLOCK;
@@ -754,8 +741,37 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 			brq.data.flags |= MMC_DATA_WRITE;
 		}
 
-		if (do_rel_wr && mmc_apply_rel_rw(&brq, card, req))
-			goto cmd_err;
+		if (do_rel_wr)
+			mmc_apply_rel_rw(&brq, card, req);
+
+		/*
+		 * Pre-defined multi-block transfers are preferable to
+		 * open ended-ones (and necessary for reliable writes).
+		 * However, it is not sufficient to just send CMD23,
+		 * and avoid the final CMD12, as on an error condition
+		 * CMD12 (stop) needs to be sent anyway. This, coupled
+		 * with Auto-CMD23 enhancements provided by some
+		 * hosts, means that the complexity of dealing
+		 * with this is best left to the host. If CMD23 is
+		 * supported by card and host, we'll fill sbc in and let
+		 * the host deal with handling it correctly. This means
+		 * that for hosts that don't expose MMC_CAP_CMD23, no
+		 * change of behavior will be observed.
+		 *
+		 * N.B: Some MMC cards experience perf degradation.
+		 * We'll avoid using CMD23-bounded multiblock writes for
+		 * these, while retaining features like reliable writes.
+		 */
+
+		if ((md->flags & MMC_BLK_CMD23) &&
+		    mmc_op_multi(brq.cmd.opcode) &&
+		    (do_rel_wr || !(card->quirks & MMC_QUIRK_BLK_NO_CMD23))) {
+			brq.sbc.opcode = MMC_SET_BLOCK_COUNT;
+			brq.sbc.arg = brq.data.blocks |
+				(do_rel_wr ? (1 << 31) : 0);
+			brq.sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
+			brq.mrq.sbc = &brq.sbc;
+		}
 
 		mmc_set_data_timeout(&brq.data, card);
 
@@ -792,7 +808,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 		 * until later as we need to wait for the card to leave
 		 * programming mode even when things go wrong.
 		 */
-		if (brq.cmd.error || brq.data.error || brq.stop.error) {
+		if (brq.sbc.error || brq.cmd.error ||
+		    brq.data.error || brq.stop.error) {
 			if (brq.data.blocks > 1 && rq_data_dir(req) == READ) {
 				/* Redo read one sector at a time */
 				printk(KERN_WARNING "%s: retrying using single "
@@ -803,6 +820,13 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 			status = get_card_status(card, req);
 		}
 
+		if (brq.sbc.error) {
+			printk(KERN_ERR "%s: error %d sending SET_BLOCK_COUNT "
+			       "command, response %#x, card status %#x\n",
+			       req->rq_disk->disk_name, brq.sbc.error,
+			       brq.sbc.resp[0], status);
+		}
+
 		if (brq.cmd.error) {
 			printk(KERN_ERR "%s: error %d sending read/write "
 			       "command, response %#x, card status %#x\n",
@@ -1014,8 +1038,6 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	md->disk->queue = md->queue.queue;
 	md->disk->driverfs_dev = parent;
 	set_disk_ro(md->disk, md->read_only || default_ro);
-	if (REL_WRITES_SUPPORTED(card))
-		blk_queue_flush(md->queue.queue, REQ_FLUSH | REQ_FUA);
 
 	/*
 	 * As discussed on lkml, GENHD_FL_REMOVABLE should:
@@ -1034,6 +1056,19 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 
 	blk_queue_logical_block_size(md->queue.queue, 512);
 	set_capacity(md->disk, size);
+
+	if (mmc_host_cmd23(card->host) &&
+	    mmc_card_mmc(card))
+		md->flags |= MMC_BLK_CMD23;
+
+	if (mmc_card_mmc(card) &&
+	    md->flags & MMC_BLK_CMD23 &&
+	    ((card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||
+	     card->ext_csd.rel_sectors)) {
+		md->flags |= MMC_BLK_REL_WR;
+		blk_queue_flush(md->queue.queue, REQ_FLUSH | REQ_FUA);
+	}
+
 	return md;
 
  err_putdisk:
@@ -1189,6 +1224,21 @@ static const struct mmc_fixup blk_fixups[] =
 	MMC_FIXUP("SEM08G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
 	MMC_FIXUP("SEM16G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
 	MMC_FIXUP("SEM32G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
+
+	/*
+	 * Some MMC cards experience performance degradation with CMD23
+	 * instead of CMD12-bounded multiblock transfers. For now we'll
+	 * black list what's bad...
+	 * - Certain Toshiba cards.
+	 *
+	 * N.B. This doesn't affect SD cards.
+	 */
+	MMC_FIXUP("MMC08G", 0x11, CID_OEMID_ANY, add_quirk_mmc,
+		  MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("MMC16G", 0x11, CID_OEMID_ANY, add_quirk_mmc,
+		  MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("MMC32G", 0x11, CID_OEMID_ANY, add_quirk_mmc,
+		  MMC_QUIRK_BLK_NO_CMD23),
 	END_FIXUP
 };
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 7190aa2..4a0e27b 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -171,6 +171,7 @@ struct mmc_card {
 #define MMC_QUIRK_NONSTD_FUNC_IF (1<<4)		/* SDIO card has nonstd function interfaces */
 #define MMC_QUIRK_DISABLE_CD	(1<<5)		/* disconnect CD/DAT[3] resistor */
 #define MMC_QUIRK_INAND_CMD38	(1<<6)		/* iNAND devices have broken CMD38 */
+#define MMC_QUIRK_BLK_NO_CMD23	(1<<7)		/* Avoid CMD23 for regular multiblock */
 
 	unsigned int		erase_size;	/* erase size in sectors */
  	unsigned int		erase_shift;	/* if erase unit is power 2 */
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index cbe8d55..b6718e5 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -120,6 +120,7 @@ struct mmc_data {
 };
 
 struct mmc_request {
+	struct mmc_command	*sbc;		/* SET_BLOCK_COUNT for multiblock */
 	struct mmc_command	*cmd;
 	struct mmc_data		*data;
 	struct mmc_command	*stop;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index de32e6a..e946bd1 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -210,6 +210,7 @@ struct mmc_host {
 #define MMC_CAP_MAX_CURRENT_400	(1 << 27)	/* Host max current limit is 400mA */
 #define MMC_CAP_MAX_CURRENT_600	(1 << 28)	/* Host max current limit is 600mA */
 #define MMC_CAP_MAX_CURRENT_800	(1 << 29)	/* Host max current limit is 800mA */
+#define MMC_CAP_CMD23		(1 << 30)	/* CMD23 supported. */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 
@@ -366,5 +367,10 @@ static inline int mmc_card_wake_sdio_irq(struct mmc_host *host)
 {
 	return host->pm_flags & MMC_PM_WAKE_SDIO_IRQ;
 }
+
+static inline int mmc_host_cmd23(struct mmc_host *host)
+{
+	return host->caps & MMC_CAP_CMD23;
+}
 #endif
 
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 9fa5a73..848aab2 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -83,6 +83,12 @@
 #define MMC_APP_CMD              55   /* ac   [31:16] RCA        R1  */
 #define MMC_GEN_CMD              56   /* adtc [0] RD/WR          R1  */
 
+static inline bool mmc_op_multi(u32 opcode)
+{
+	return opcode == MMC_WRITE_MULTIPLE_BLOCK ||
+		opcode == MMC_READ_MULTIPLE_BLOCK;
+}
+
 /*
  * MMC_SWITCH argument format:
  *
-- 
1.7.0.4

[v5 3/5] MMC: Implement MMC_CAP_CMD23 for SDHCI.

[Andrei Warkentin]

Implements support for multiblock transfers bounded
by SET_BLOCK_COUNT (CMD23).

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Cc: malchev@google.com
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/host/sdhci.c  |   66 ++++++++++++++++++++++++++++++++++-----------
 include/linux/mmc/sdhci.h |    1 +
 2 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index cc63f5e..1138b55 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -838,9 +838,10 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 }
 
 static void sdhci_set_transfer_mode(struct sdhci_host *host,
-	struct mmc_data *data)
+	struct mmc_command *cmd)
 {
 	u16 mode;
+	struct mmc_data *data = cmd->data;
 
 	if (data == NULL)
 		return;
@@ -848,11 +849,17 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
 	WARN_ON(!host->data);
 
 	mode = SDHCI_TRNS_BLK_CNT_EN;
-	if (data->blocks > 1) {
-		if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
-			mode |= SDHCI_TRNS_MULTI | SDHCI_TRNS_ACMD12;
-		else
-			mode |= SDHCI_TRNS_MULTI;
+	if (mmc_op_multi(cmd->opcode) ||
+	    data->blocks > 1) {
+		mode |= SDHCI_TRNS_MULTI;
+
+		/*
+		 * If we are sending CMD23, CMD12 never gets sent
+		 * on successful completion (so no Auto-CMD12).
+		 */
+		if (!host->mrq->sbc &&
+		    host->flags & SDHCI_ACMD12)
+			mode |= SDHCI_TRNS_ACMD12;
 	}
 	if (data->flags & MMC_DATA_READ)
 		mode |= SDHCI_TRNS_READ;
@@ -893,7 +900,15 @@ static void sdhci_finish_data(struct sdhci_host *host)
 	else
 		data->bytes_xfered = data->blksz * data->blocks;
 
-	if (data->stop) {
+	/*
+	 * Need to send CMD12 if -
+	 * a) open-ended multiblock transfer (no CMD23)
+	 * b) error in multiblock transfer
+	 */
+	if (data->stop &&
+	    (data->error ||
+	     !host->mrq->sbc)) {
+
 		/*
 		 * The controller needs a reset of internal state machines
 		 * upon error conditions.
@@ -949,7 +964,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 
 	sdhci_writel(host, cmd->arg, SDHCI_ARGUMENT);
 
-	sdhci_set_transfer_mode(host, cmd->data);
+	sdhci_set_transfer_mode(host, cmd);
 
 	if ((cmd->flags & MMC_RSP_136) && (cmd->flags & MMC_RSP_BUSY)) {
 		printk(KERN_ERR "%s: Unsupported response type!\n",
@@ -1004,13 +1019,21 @@ static void sdhci_finish_command(struct sdhci_host *host)
 
 	host->cmd->error = 0;
 
-	if (host->data && host->data_early)
-		sdhci_finish_data(host);
+	/* Finished CMD23, now send actual command. */
+	if (host->cmd == host->mrq->sbc) {
+		host->cmd = NULL;
+		sdhci_send_command(host, host->mrq->cmd);
+	} else {
 
-	if (!host->cmd->data)
-		tasklet_schedule(&host->finish_tasklet);
+		/* Processed actual command. */
+		if (host->data && host->data_early)
+			sdhci_finish_data(host);
 
-	host->cmd = NULL;
+		if (!host->cmd->data)
+			tasklet_schedule(&host->finish_tasklet);
+
+		host->cmd = NULL;
+	}
 }
 
 static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
@@ -1189,7 +1212,12 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 #ifndef SDHCI_USE_LEDS_CLASS
 	sdhci_activate_led(host);
 #endif
-	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12) {
+
+	/*
+	 * Ensure we don't send the STOP for non-SET_BLOCK_COUNTED
+	 * requests if Auto-CMD12 is enabled.
+	 */
+	if (!mrq->sbc && (host->flags & SDHCI_ACMD12)) {
 		if (mrq->stop) {
 			mrq->data->stop = NULL;
 			mrq->stop = NULL;
@@ -1227,7 +1255,10 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 			host->mrq = mrq;
 		}
 
-		sdhci_send_command(host, mrq->cmd);
+		if (mrq->sbc)
+			sdhci_send_command(host, mrq->sbc);
+		else
+			sdhci_send_command(host, mrq->cmd);
 	}
 
 	mmiowb();
@@ -2455,7 +2486,10 @@ int sdhci_add_host(struct sdhci_host *host)
 	} else
 		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;
 
-	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE;
+	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;
+
+	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
+		host->flags |= SDHCI_ACMD12;
 
 	/*
 	 * A controller may support 8-bit width, but the board itself
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index e902618..d70de9d 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -113,6 +113,7 @@ struct sdhci_host {
 #define SDHCI_DEVICE_DEAD	(1<<3)	/* Device unresponsive */
 #define SDHCI_SDR50_NEEDS_TUNING (1<<4)	/* SDR50 needs tuning */
 #define SDHCI_NEEDS_RETUNING	(1<<5)	/* Host needs retuning */
+#define SDHCI_ACMD12		(1<<6)	/* Auto CMD12 support */
 
 	unsigned int version;	/* SDHCI spec. version */
 
-- 
1.7.0.4

[v5 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.

[Andrei Warkentin]

SD cards operating at UHS104 or better support SET_BLOCK_COUNT.

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Cc: malchev@google.com
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/card/block.c |    9 ++++++---
 drivers/mmc/core/sd.c    |    7 +++++++
 include/linux/mmc/card.h |    3 +++
 include/linux/mmc/sd.h   |    2 +-
 4 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index a380acc..71da564 100755
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1057,9 +1057,12 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	blk_queue_logical_block_size(md->queue.queue, 512);
 	set_capacity(md->disk, size);
 
-	if (mmc_host_cmd23(card->host) &&
-	    mmc_card_mmc(card))
-		md->flags |= MMC_BLK_CMD23;
+	if (mmc_host_cmd23(card->host)) {
+		if (mmc_card_mmc(card) ||
+		    (mmc_card_sd(card) &&
+		     card->scr.cmds & SD_SCR_CMD23_SUPPORT))
+			md->flags |= MMC_BLK_CMD23;
+	}
 
 	if (mmc_card_mmc(card) &&
 	    md->flags & MMC_BLK_CMD23 &&
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 596d0b9..efff41d 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -203,6 +203,13 @@ static int mmc_decode_scr(struct mmc_card *card)
 	else
 		card->erased_byte = 0x0;
 
+	if (scr->sda_vsn == SCR_SPEC_VER_2) {
+
+		/* Check if Physical Layer Spec v3.0X is supported. */
+		scr->sda_spec3 = UNSTUFF_BITS(resp, 47, 1);
+		if (scr->sda_spec3)
+			scr->cmds = UNSTUFF_BITS(resp, 32, 2);
+	}
 	return 0;
 }
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 4a0e27b..c6927a4 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -72,6 +72,9 @@ struct sd_scr {
 	unsigned char		bus_widths;
 #define SD_SCR_BUS_WIDTH_1	(1<<0)
 #define SD_SCR_BUS_WIDTH_4	(1<<2)
+	unsigned char		cmds;
+#define SD_SCR_CMD20_SUPPORT   (1<<0)
+#define SD_SCR_CMD23_SUPPORT   (1<<1)
 };
 
 struct sd_ssr {
diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h
index c648878..7d35d52 100644
--- a/include/linux/mmc/sd.h
+++ b/include/linux/mmc/sd.h
@@ -66,7 +66,7 @@
 
 #define SCR_SPEC_VER_0		0	/* Implements system specification 1.0 - 1.01 */
 #define SCR_SPEC_VER_1		1	/* Implements system specification 1.10 */
-#define SCR_SPEC_VER_2		2	/* Implements system specification 2.00 */
+#define SCR_SPEC_VER_2		2	/* Implements system specification 2.00-3.0X */
 
 /*
  * SD bus widths
-- 
1.7.0.4

[v5 5/5] MMC: SDHCI AutoCMD23 support.

[Andrei Warkentin]

Enables Auto-CMD23 support where available (SDHCI 3.0 controllers)

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Cc: subhashj@codeaurora.org
Cc: malchev@google.com
Tested-by: Arindam Nath <arindam.nath@amd.com>
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/host/sdhci.c  |   17 ++++++++++++++++-
 drivers/mmc/host/sdhci.h  |    2 ++
 include/linux/mmc/sdhci.h |    1 +
 3 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 1138b55..d79b2d0 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -860,7 +860,13 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
 		if (!host->mrq->sbc &&
 		    host->flags & SDHCI_ACMD12)
 			mode |= SDHCI_TRNS_ACMD12;
+		else if (host->mrq->sbc &&
+			 host->flags & SDHCI_ACMD23) {
+			mode |= SDHCI_TRNS_ACMD23;
+			sdhci_writel(host, host->mrq->sbc->arg, SDHCI_ARGUMENT2);
+		}
 	}
+
 	if (data->flags & MMC_DATA_READ)
 		mode |= SDHCI_TRNS_READ;
 	if (host->flags & SDHCI_REQ_USE_DMA)
@@ -1255,7 +1261,7 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 			host->mrq = mrq;
 		}
 
-		if (mrq->sbc)
+		if (mrq->sbc && !(host->flags & SDHCI_ACMD23))
 			sdhci_send_command(host, mrq->sbc);
 		else
 			sdhci_send_command(host, mrq->cmd);
@@ -2491,6 +2497,15 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
 		host->flags |= SDHCI_ACMD12;
 
+	/* Auto-CMD23 stuff only works in ADMA or PIO. */
+	if ((host->version == SDHCI_SPEC_300) &&
+	    ((host->flags & SDHCI_USE_ADMA) ||
+	     !(host->flags & SDHCI_REQ_USE_DMA))) {
+		host->flags |= SDHCI_ACMD23;
+		printk(KERN_INFO "%s: Auto-CMD23 available\n", mmc_hostname(mmc));
+	} else
+		printk(KERN_INFO "%s: Auto-CMD23 unavailable\n", mmc_hostname(mmc));
+
 	/*
 	 * A controller may support 8-bit width, but the board itself
 	 * might not have the pins brought out.  Boards that support
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 7e28eec..713caf0 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -25,6 +25,7 @@
  */
 
 #define SDHCI_DMA_ADDRESS	0x00
+#define SDHCI_ARGUMENT2		0x00
 
 #define SDHCI_BLOCK_SIZE	0x04
 #define  SDHCI_MAKE_BLKSZ(dma, blksz) (((dma & 0x7) << 12) | (blksz & 0xFFF))
@@ -37,6 +38,7 @@
 #define  SDHCI_TRNS_DMA		0x01
 #define  SDHCI_TRNS_BLK_CNT_EN	0x02
 #define  SDHCI_TRNS_ACMD12	0x04
+#define  SDHCI_TRNS_ACMD23	0x08
 #define  SDHCI_TRNS_READ	0x10
 #define  SDHCI_TRNS_MULTI	0x20
 
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index d70de9d..d0db8ae 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -114,6 +114,7 @@ struct sdhci_host {
 #define SDHCI_SDR50_NEEDS_TUNING (1<<4)	/* SDR50 needs tuning */
 #define SDHCI_NEEDS_RETUNING	(1<<5)	/* Host needs retuning */
 #define SDHCI_ACMD12		(1<<6)	/* Auto CMD12 support */
+#define SDHCI_ACMD23		(1<<7)	/* Auto CMD23 support */
 
 	unsigned int version;	/* SDHCI spec. version */
 
-- 
1.7.0.4

Re: [[v4] 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Jaehoon Chung]

Hi Andrei,

Andrei Warkentin wrote:
> On Wed, May 18, 2011 at 9:37 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Hi Andrei
>>
>> Andrei Warkentin wrote:
>>> CMD23-prefixed instead of open-ended multiblock transfers
>>> have a performance advantage on some MMC cards.
>>>
>> you mentioned about "some MMC cards".
>> Conversely, that means the some card didn't have a performance advantage?
>>
>> Did you find the performance advantage?
>> if you found the advantage and you can tell me,
>> i want to know what do you have the some MMC cards..
>>
> 
> I've tested this on a Sandisk eMMC where I saw as good as
> a 50% improvement on writes (30% real-life use cases). This was a
> SEM32G 4.3+ part.

Can you tell me your environment? buswidth, AP information, benchmark etc..
And if you have the performance result's data, can you share them?

Regards,
Jaehoon Chung

Re: [[v4] 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Andrei Warkentin]

On Fri, May 20, 2011 at 1:49 AM, Andrei Warkentin <andreiw@motorola.com> wrote:
> On Thu, May 19, 2011 at 11:38 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Hi Andrei,
>>
>> Andrei Warkentin wrote:
>>> On Wed, May 18, 2011 at 9:37 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>> Hi Andrei
>>>>
>>>> Andrei Warkentin wrote:
>>>>> CMD23-prefixed instead of open-ended multiblock transfers
>>>>> have a performance advantage on some MMC cards.
>>>>>
>>>> you mentioned about "some MMC cards".
>>>> Conversely, that means the some card didn't have a performance advantage?
>>>>
>>>> Did you find the performance advantage?
>>>> if you found the advantage and you can tell me,
>>>> i want to know what do you have the some MMC cards..
>>>>
>>>
>>> I've tested this on a Sandisk eMMC where I saw as good as
>>> a 50% improvement on writes (30% real-life use cases). This was a
>>> SEM32G 4.3+ part.
>>
>> Can you tell me your environment? buswidth, AP information, benchmark etc..
>> And if you have the performance result's data, can you share them?
>>
>
> This was on an SDHCI controller (hence the patch...) on a Tegra
> 2-based system. I was measuring
> throughput on reads and writes (obviously without block cache,
> filesystem, etc) to an eMMC card, 8 bits.
>
> Tested both with my tool (https://github.com/andreiw/superalign) and
> an sqllite-based test.
>
> I'm attaching the data I have.
>
> A
>

Additionally, CMD23 use is a requirement for SDXC cards (Arindam can
comment on that), as well as for MMC reliable writes and eMMC 4.5-spec
features (Yunpeng Gao can comment on that).

These patches allow CMD23 use. They do involve a some changes to host
controller because of interaction with CMD12, as well as Auto-CMD12
and Auto-CMD23 features. I can definitely consult you if you need help
implementing CMD23 support for whatever controller you develop for.

A

Re: [[v4] 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Jaehoon Chung]

Andrei Warkentin wrote:
> On Fri, May 20, 2011 at 1:49 AM, Andrei Warkentin <andreiw@motorola.com> wrote:
>> On Thu, May 19, 2011 at 11:38 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> Hi Andrei,
>>>
>>> Andrei Warkentin wrote:
>>>> On Wed, May 18, 2011 at 9:37 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>>> Hi Andrei
>>>>>
>>>>> Andrei Warkentin wrote:
>>>>>> CMD23-prefixed instead of open-ended multiblock transfers
>>>>>> have a performance advantage on some MMC cards.
>>>>>>
>>>>> you mentioned about "some MMC cards".
>>>>> Conversely, that means the some card didn't have a performance advantage?
>>>>>
>>>>> Did you find the performance advantage?
>>>>> if you found the advantage and you can tell me,
>>>>> i want to know what do you have the some MMC cards..
>>>>>
>>>> I've tested this on a Sandisk eMMC where I saw as good as
>>>> a 50% improvement on writes (30% real-life use cases). This was a
>>>> SEM32G 4.3+ part.
>>> Can you tell me your environment? buswidth, AP information, benchmark etc..
>>> And if you have the performance result's data, can you share them?
>>>
>> This was on an SDHCI controller (hence the patch...) on a Tegra
>> 2-based system. I was measuring
>> throughput on reads and writes (obviously without block cache,
>> filesystem, etc) to an eMMC card, 8 bits.
>>
>> Tested both with my tool (https://github.com/andreiw/superalign) and
>> an sqllite-based test.
>>
>> I'm attaching the data I have.
>>
>> A
>>
> 
> Additionally, CMD23 use is a requirement for SDXC cards (Arindam can
> comment on that), as well as for MMC reliable writes and eMMC 4.5-spec
> features (Yunpeng Gao can comment on that).
> 
> These patches allow CMD23 use. They do involve a some changes to host
> controller because of interaction with CMD12, as well as Auto-CMD12
> and Auto-CMD23 features. I can definitely consult you if you need help
> implementing CMD23 support for whatever controller you develop for.
> 

I used two host-controller (sdhci and dw_mmc). you implemented them in sdhci.
I applied your patch then tested CMD23...but i didn't applied  auto-CMD23.
I known that auto-CMD23 supported at SD3.0..right?
My controller is supported SD2.0..

If you can consult me, too much helpful to me..

Regards,
Jaehoon Chung

Re: [v5 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Jaehoon Chung]

Hi Andrei.

I have some question..
In mmc_blk_issue_rw_rq() of block.c, there is the below code.

static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
{
	struct mmc_blk_data *md = mq->data;
	struct mmc_card *card = md->queue.card;
	struct mmc_blk_request brq;
	int ret = 1, disable_multi = 0;

	/*
	 * Reliable writes are used to implement Forced Unit Access and
	 * REQ_META accesses, and are supported only on MMCs.
	 */
	bool do_rel_wr = ((req->cmd_flags & REQ_FUA) ||
			  (req->cmd_flags & REQ_META)) &&
		(rq_data_dir(req) == WRITE) &&
		REL_WRITES_SUPPORTED(card);

	do {
		struct mmc_command cmd = {0};
		u32 readcmd, writecmd, status = 0;

		memset(&brq, 0, sizeof(struct mmc_blk_request));
		brq.mrq.cmd = &brq.cmd;
		brq.mrq.data = &brq.data;

		brq.cmd.arg = blk_rq_pos(req);
		if (!mmc_card_blockaddr(card))
			brq.cmd.arg <<= 9;
		brq.cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
		brq.data.blksz = 512;
		brq.stop.opcode = MMC_STOP_TRANSMISSION;
		brq.stop.arg = 0;
		brq.stop.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
		brq.data.blocks = blk_rq_sectors(req);

There is "brq.stop.opcode = MMC_STOP_TRANSMISSION".
If we use CMD23, this line didn't need? Actually i don't know, so i ask to you.

Regards,
Jaehoon Chung

Andrei Warkentin wrote:
> CMD23-prefixed instead of open-ended multiblock transfers
> have a performance advantage on some MMC cards.
> 
> Cc: arindam.nath@amd.com
> Cc: cjb@laptop.org
> Cc: arnd@arndb.de
> Cc: malchev@google.com
> Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
> ---
>  drivers/mmc/card/block.c |  108 +++++++++++++++++++++++++++++++++------------
>  include/linux/mmc/card.h |    1 +
>  include/linux/mmc/core.h |    1 +
>  include/linux/mmc/host.h |    6 +++
>  include/linux/mmc/mmc.h  |    6 +++
>  5 files changed, 93 insertions(+), 29 deletions(-)
>  mode change 100644 => 100755 drivers/mmc/card/block.c
> 
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> old mode 100644
> new mode 100755
> index 126c7f4..a380acc
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -59,10 +59,6 @@ MODULE_ALIAS("mmc:block");
>  #define INAND_CMD38_ARG_SECTRIM1 0x81
>  #define INAND_CMD38_ARG_SECTRIM2 0x88
>  
> -#define REL_WRITES_SUPPORTED(card) (mmc_card_mmc((card)) &&	\
> -    (((card)->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||	\
> -     ((card)->ext_csd.rel_sectors)))
> -
>  static DEFINE_MUTEX(block_mutex);
>  
>  /*
> @@ -90,6 +86,10 @@ struct mmc_blk_data {
>  	struct mmc_queue queue;
>  	struct list_head part;
>  
> +	unsigned int	flags;
> +#define MMC_BLK_CMD23	(1 << 0)	/* Can do SET_BLOCK_COUNT for multiblock */
> +#define MMC_BLK_REL_WR	(1 << 1)	/* MMC Reliable write support */
> +
>  	unsigned int	usage;
>  	unsigned int	read_only;
>  	unsigned int	part_type;
> @@ -429,6 +429,7 @@ static const struct block_device_operations mmc_bdops = {
>  
>  struct mmc_blk_request {
>  	struct mmc_request	mrq;
> +	struct mmc_command	sbc;
>  	struct mmc_command	cmd;
>  	struct mmc_command	stop;
>  	struct mmc_data		data;
> @@ -652,13 +653,10 @@ static int mmc_blk_issue_flush(struct mmc_queue *mq, struct request *req)
>   * reliable write can handle, thus finish the request in
>   * partial completions.
>   */
> -static inline int mmc_apply_rel_rw(struct mmc_blk_request *brq,
> -				   struct mmc_card *card,
> -				   struct request *req)
> +static inline void mmc_apply_rel_rw(struct mmc_blk_request *brq,
> +				    struct mmc_card *card,
> +				    struct request *req)
>  {
> -	int err;
> -	struct mmc_command set_count = {0};
> -
>  	if (!(card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN)) {
>  		/* Legacy mode imposes restrictions on transfers. */
>  		if (!IS_ALIGNED(brq->cmd.arg, card->ext_csd.rel_sectors))
> @@ -669,15 +667,6 @@ static inline int mmc_apply_rel_rw(struct mmc_blk_request *brq,
>  		else if (brq->data.blocks < card->ext_csd.rel_sectors)
>  			brq->data.blocks = 1;
>  	}
> -
> -	set_count.opcode = MMC_SET_BLOCK_COUNT;
> -	set_count.arg = brq->data.blocks | (1 << 31);
> -	set_count.flags = MMC_RSP_R1 | MMC_CMD_AC;
> -	err = mmc_wait_for_cmd(card->host, &set_count, 0);
> -	if (err)
> -		printk(KERN_ERR "%s: error %d SET_BLOCK_COUNT\n",
> -		       req->rq_disk->disk_name, err);
> -	return err;
>  }
>  
>  static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
> @@ -694,7 +683,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
>  	bool do_rel_wr = ((req->cmd_flags & REQ_FUA) ||
>  			  (req->cmd_flags & REQ_META)) &&
>  		(rq_data_dir(req) == WRITE) &&
> -		REL_WRITES_SUPPORTED(card);
> +		(md->flags & MMC_BLK_REL_WR);
>  
>  	do {
>  		struct mmc_command cmd = {0};
> @@ -732,11 +721,9 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
>  
>  		if (brq.data.blocks > 1 || do_rel_wr) {
>  			/* SPI multiblock writes terminate using a special
> -			 * token, not a STOP_TRANSMISSION request. Reliable
> -			 * writes use SET_BLOCK_COUNT and do not use a
> -			 * STOP_TRANSMISSION request either.
> +			 * token, not a STOP_TRANSMISSION request.
>  			 */
> -			if ((!mmc_host_is_spi(card->host) && !do_rel_wr) ||
> +			if (!mmc_host_is_spi(card->host) ||
>  			    rq_data_dir(req) == READ)
>  				brq.mrq.stop = &brq.stop;
>  			readcmd = MMC_READ_MULTIPLE_BLOCK;
> @@ -754,8 +741,37 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
>  			brq.data.flags |= MMC_DATA_WRITE;
>  		}
>  
> -		if (do_rel_wr && mmc_apply_rel_rw(&brq, card, req))
> -			goto cmd_err;
> +		if (do_rel_wr)
> +			mmc_apply_rel_rw(&brq, card, req);
> +
> +		/*
> +		 * Pre-defined multi-block transfers are preferable to
> +		 * open ended-ones (and necessary for reliable writes).
> +		 * However, it is not sufficient to just send CMD23,
> +		 * and avoid the final CMD12, as on an error condition
> +		 * CMD12 (stop) needs to be sent anyway. This, coupled
> +		 * with Auto-CMD23 enhancements provided by some
> +		 * hosts, means that the complexity of dealing
> +		 * with this is best left to the host. If CMD23 is
> +		 * supported by card and host, we'll fill sbc in and let
> +		 * the host deal with handling it correctly. This means
> +		 * that for hosts that don't expose MMC_CAP_CMD23, no
> +		 * change of behavior will be observed.
> +		 *
> +		 * N.B: Some MMC cards experience perf degradation.
> +		 * We'll avoid using CMD23-bounded multiblock writes for
> +		 * these, while retaining features like reliable writes.
> +		 */
> +
> +		if ((md->flags & MMC_BLK_CMD23) &&
> +		    mmc_op_multi(brq.cmd.opcode) &&
> +		    (do_rel_wr || !(card->quirks & MMC_QUIRK_BLK_NO_CMD23))) {
> +			brq.sbc.opcode = MMC_SET_BLOCK_COUNT;
> +			brq.sbc.arg = brq.data.blocks |
> +				(do_rel_wr ? (1 << 31) : 0);
> +			brq.sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
> +			brq.mrq.sbc = &brq.sbc;
> +		}
>  
>  		mmc_set_data_timeout(&brq.data, card);
>  
> @@ -792,7 +808,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
>  		 * until later as we need to wait for the card to leave
>  		 * programming mode even when things go wrong.
>  		 */
> -		if (brq.cmd.error || brq.data.error || brq.stop.error) {
> +		if (brq.sbc.error || brq.cmd.error ||
> +		    brq.data.error || brq.stop.error) {
>  			if (brq.data.blocks > 1 && rq_data_dir(req) == READ) {
>  				/* Redo read one sector at a time */
>  				printk(KERN_WARNING "%s: retrying using single "
> @@ -803,6 +820,13 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
>  			status = get_card_status(card, req);
>  		}
>  
> +		if (brq.sbc.error) {
> +			printk(KERN_ERR "%s: error %d sending SET_BLOCK_COUNT "
> +			       "command, response %#x, card status %#x\n",
> +			       req->rq_disk->disk_name, brq.sbc.error,
> +			       brq.sbc.resp[0], status);
> +		}
> +
>  		if (brq.cmd.error) {
>  			printk(KERN_ERR "%s: error %d sending read/write "
>  			       "command, response %#x, card status %#x\n",
> @@ -1014,8 +1038,6 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>  	md->disk->queue = md->queue.queue;
>  	md->disk->driverfs_dev = parent;
>  	set_disk_ro(md->disk, md->read_only || default_ro);
> -	if (REL_WRITES_SUPPORTED(card))
> -		blk_queue_flush(md->queue.queue, REQ_FLUSH | REQ_FUA);
>  
>  	/*
>  	 * As discussed on lkml, GENHD_FL_REMOVABLE should:
> @@ -1034,6 +1056,19 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>  
>  	blk_queue_logical_block_size(md->queue.queue, 512);
>  	set_capacity(md->disk, size);
> +
> +	if (mmc_host_cmd23(card->host) &&
> +	    mmc_card_mmc(card))
> +		md->flags |= MMC_BLK_CMD23;
> +
> +	if (mmc_card_mmc(card) &&
> +	    md->flags & MMC_BLK_CMD23 &&
> +	    ((card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||
> +	     card->ext_csd.rel_sectors)) {
> +		md->flags |= MMC_BLK_REL_WR;
> +		blk_queue_flush(md->queue.queue, REQ_FLUSH | REQ_FUA);
> +	}
> +
>  	return md;
>  
>   err_putdisk:
> @@ -1189,6 +1224,21 @@ static const struct mmc_fixup blk_fixups[] =
>  	MMC_FIXUP("SEM08G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
>  	MMC_FIXUP("SEM16G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
>  	MMC_FIXUP("SEM32G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
> +
> +	/*
> +	 * Some MMC cards experience performance degradation with CMD23
> +	 * instead of CMD12-bounded multiblock transfers. For now we'll
> +	 * black list what's bad...
> +	 * - Certain Toshiba cards.
> +	 *
> +	 * N.B. This doesn't affect SD cards.
> +	 */
> +	MMC_FIXUP("MMC08G", 0x11, CID_OEMID_ANY, add_quirk_mmc,
> +		  MMC_QUIRK_BLK_NO_CMD23),
> +	MMC_FIXUP("MMC16G", 0x11, CID_OEMID_ANY, add_quirk_mmc,
> +		  MMC_QUIRK_BLK_NO_CMD23),
> +	MMC_FIXUP("MMC32G", 0x11, CID_OEMID_ANY, add_quirk_mmc,
> +		  MMC_QUIRK_BLK_NO_CMD23),
>  	END_FIXUP
>  };
>  
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 7190aa2..4a0e27b 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -171,6 +171,7 @@ struct mmc_card {
>  #define MMC_QUIRK_NONSTD_FUNC_IF (1<<4)		/* SDIO card has nonstd function interfaces */
>  #define MMC_QUIRK_DISABLE_CD	(1<<5)		/* disconnect CD/DAT[3] resistor */
>  #define MMC_QUIRK_INAND_CMD38	(1<<6)		/* iNAND devices have broken CMD38 */
> +#define MMC_QUIRK_BLK_NO_CMD23	(1<<7)		/* Avoid CMD23 for regular multiblock */
>  
>  	unsigned int		erase_size;	/* erase size in sectors */
>   	unsigned int		erase_shift;	/* if erase unit is power 2 */
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> index cbe8d55..b6718e5 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -120,6 +120,7 @@ struct mmc_data {
>  };
>  
>  struct mmc_request {
> +	struct mmc_command	*sbc;		/* SET_BLOCK_COUNT for multiblock */
>  	struct mmc_command	*cmd;
>  	struct mmc_data		*data;
>  	struct mmc_command	*stop;
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index de32e6a..e946bd1 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -210,6 +210,7 @@ struct mmc_host {
>  #define MMC_CAP_MAX_CURRENT_400	(1 << 27)	/* Host max current limit is 400mA */
>  #define MMC_CAP_MAX_CURRENT_600	(1 << 28)	/* Host max current limit is 600mA */
>  #define MMC_CAP_MAX_CURRENT_800	(1 << 29)	/* Host max current limit is 800mA */
> +#define MMC_CAP_CMD23		(1 << 30)	/* CMD23 supported. */
>  
>  	mmc_pm_flag_t		pm_caps;	/* supported pm features */
>  
> @@ -366,5 +367,10 @@ static inline int mmc_card_wake_sdio_irq(struct mmc_host *host)
>  {
>  	return host->pm_flags & MMC_PM_WAKE_SDIO_IRQ;
>  }
> +
> +static inline int mmc_host_cmd23(struct mmc_host *host)
> +{
> +	return host->caps & MMC_CAP_CMD23;
> +}
>  #endif
>  
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index 9fa5a73..848aab2 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -83,6 +83,12 @@
>  #define MMC_APP_CMD              55   /* ac   [31:16] RCA        R1  */
>  #define MMC_GEN_CMD              56   /* adtc [0] RD/WR          R1  */
>  
> +static inline bool mmc_op_multi(u32 opcode)
> +{
> +	return opcode == MMC_WRITE_MULTIPLE_BLOCK ||
> +		opcode == MMC_READ_MULTIPLE_BLOCK;
> +}
> +
>  /*
>   * MMC_SWITCH argument format:
>   *

RE: [v5 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.

[Nath, Arindam]

Hi Andrei,


[...]
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 596d0b9..efff41d 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -203,6 +203,13 @@ static int mmc_decode_scr(struct mmc_card *card)
>  	else
>  		card->erased_byte = 0x0;
> 
> +	if (scr->sda_vsn == SCR_SPEC_VER_2) {
> +
> +		/* Check if Physical Layer Spec v3.0X is supported. */
> +		scr->sda_spec3 = UNSTUFF_BITS(resp, 47, 1);
> +		if (scr->sda_spec3)
> +			scr->cmds = UNSTUFF_BITS(resp, 32, 2);
> +	}

There is already this condition checking (== SCR_SPEC_VER_2) in mmc_decode_scr(). So can you please make your changes appropriately?

Thanks,
Arindam

RE: [v5 5/5] MMC: SDHCI AutoCMD23 support.

[Nath, Arindam]

Hi Andrei,

[...]
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 7e28eec..713caf0 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -25,6 +25,7 @@
>   */
> 
>  #define SDHCI_DMA_ADDRESS	0x00
> +#define SDHCI_ARGUMENT2		0x00

Since SDHCI_DMA_ADDRESS and SDHCI_ARGUMENT2 in actually the same register, probably ...

#define SDHCI_ARGUMENT2		SDHCI_DMA_ADDRESS

is a good idea?

> 
>  #define SDHCI_BLOCK_SIZE	0x04
>  #define  SDHCI_MAKE_BLKSZ(dma, blksz) (((dma & 0x7) << 12) | (blksz &
> 0xFFF))
> @@ -37,6 +38,7 @@
>  #define  SDHCI_TRNS_DMA		0x01
>  #define  SDHCI_TRNS_BLK_CNT_EN	0x02
>  #define  SDHCI_TRNS_ACMD12	0x04
> +#define  SDHCI_TRNS_ACMD23	0x08

How about changing these names explicitly to _AUTO_CMD12 and _AUTO_CMD23? That way we are less likely to get confused with App commands with same names.

>  #define  SDHCI_TRNS_READ	0x10
>  #define  SDHCI_TRNS_MULTI	0x20
> 
> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
> index d70de9d..d0db8ae 100644
> --- a/include/linux/mmc/sdhci.h
> +++ b/include/linux/mmc/sdhci.h
> @@ -114,6 +114,7 @@ struct sdhci_host {
>  #define SDHCI_SDR50_NEEDS_TUNING (1<<4)	/* SDR50 needs tuning */
>  #define SDHCI_NEEDS_RETUNING	(1<<5)	/* Host needs retuning */
>  #define SDHCI_ACMD12		(1<<6)	/* Auto CMD12 support */
> +#define SDHCI_ACMD23		(1<<7)	/* Auto CMD23 support */

Same comment as above.

Thanks,
Arindam

Re: [v5 5/5] MMC: SDHCI AutoCMD23 support.

[Andrei Warkentin]

Hi Arindam,

On Fri, May 20, 2011 at 5:46 AM, Nath, Arindam <Arindam.Nath@amd.com> wrote:
> Hi Andrei,
>
> [...]
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index 7e28eec..713caf0 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -25,6 +25,7 @@
>>   */
>>
>>  #define SDHCI_DMA_ADDRESS    0x00
>> +#define SDHCI_ARGUMENT2              0x00
>
> Since SDHCI_DMA_ADDRESS and SDHCI_ARGUMENT2 in actually the same register, probably ...
>
> #define SDHCI_ARGUMENT2         SDHCI_DMA_ADDRESS

Sure.

>>  #define  SDHCI_TRNS_ACMD12   0x04
>> +#define  SDHCI_TRNS_ACMD23   0x08
>
> How about changing these names explicitly to _AUTO_CMD12 and _AUTO_CMD23? That way we are less likely to get confused with App commands with same names.

Sounds good.

>>  #define SDHCI_ACMD12         (1<<6)  /* Auto CMD12 support */
>> +#define SDHCI_ACMD23         (1<<7)  /* Auto CMD23 support */
>

Ok, will clean it up.

A

Re: [v5 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.

[Andrei Warkentin]

On Fri, May 20, 2011 at 5:38 AM, Nath, Arindam <Arindam.Nath@amd.com> wrote:
> Hi Andrei,
>
>
> [...]
>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>> index 596d0b9..efff41d 100644
>> --- a/drivers/mmc/core/sd.c
>> +++ b/drivers/mmc/core/sd.c
>> @@ -203,6 +203,13 @@ static int mmc_decode_scr(struct mmc_card *card)
>>       else
>>               card->erased_byte = 0x0;
>>
>> +     if (scr->sda_vsn == SCR_SPEC_VER_2) {
>> +
>> +             /* Check if Physical Layer Spec v3.0X is supported. */
>> +             scr->sda_spec3 = UNSTUFF_BITS(resp, 47, 1);
>> +             if (scr->sda_spec3)
>> +                     scr->cmds = UNSTUFF_BITS(resp, 32, 2);
>> +     }
>
> There is already this condition checking (== SCR_SPEC_VER_2) in mmc_decode_scr(). So can you please make your changes appropriately?

Ok, will clean it up.

A

Re: [v5 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Andrei Warkentin]

On Fri, May 20, 2011 at 5:29 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi Andrei.
>
> I have some question..
> In mmc_blk_issue_rw_rq() of block.c, there is the below code.
>
> static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
> {
>        struct mmc_blk_data *md = mq->data;
>        struct mmc_card *card = md->queue.card;
>        struct mmc_blk_request brq;
>        int ret = 1, disable_multi = 0;
>
>        /*
>         * Reliable writes are used to implement Forced Unit Access and
>         * REQ_META accesses, and are supported only on MMCs.
>         */
>        bool do_rel_wr = ((req->cmd_flags & REQ_FUA) ||
>                          (req->cmd_flags & REQ_META)) &&
>                (rq_data_dir(req) == WRITE) &&
>                REL_WRITES_SUPPORTED(card);
>
>        do {
>                struct mmc_command cmd = {0};
>                u32 readcmd, writecmd, status = 0;
>
>                memset(&brq, 0, sizeof(struct mmc_blk_request));
>                brq.mrq.cmd = &brq.cmd;
>                brq.mrq.data = &brq.data;
>
>                brq.cmd.arg = blk_rq_pos(req);
>                if (!mmc_card_blockaddr(card))
>                        brq.cmd.arg <<= 9;
>                brq.cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
>                brq.data.blksz = 512;
>                brq.stop.opcode = MMC_STOP_TRANSMISSION;
>                brq.stop.arg = 0;
>                brq.stop.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
>                brq.data.blocks = blk_rq_sectors(req);
>
> There is "brq.stop.opcode = MMC_STOP_TRANSMISSION".
> If we use CMD23, this line didn't need? Actually i don't know, so i ask to you.
>

Nope. Needed. CMD23 support might not be implemented by host driver
(like it isn't right now for anything non-SDHCI). And even it if were,
you still need to send stop command on any error conditions. I suggest
you read the relevant sections in the publically available eMMC spec.

A

Re: [[v4] 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Jaehoon Chung]

Hi A..

I tested your patch..(using CMD23)
my environment is the below.
eMMC card : Sandisk SEM8G (eMMC 4.3+)
buswidth : 4bit (SDR)
AP : C110
benchmark : IOzone

I want to know how do you think about this result?
(i can't see your results)

* open-ended 
	      KB  reclen   write rewrite    read    reread
           10240       4    9128    9859    18072    18131
           10240       8    9510   10025    18107    18031
           10240      16    9445   10104    18143    18084
           10240      32    9583   10076    17912    18097
           10240      64    2957    3025    12320    12317
           10240     128    9500    9850    18082    18085
           10240     256    9506    8712    17952    17905
           10240     512    9445    9905    17981    17851
           10240    1024    8894    9724    18079    18196
           10240    2048    9401    9810    18181    18040
           10240    4096    8657    9358    18172    17980
           10240    8192    8779    7730    18067    17943

* pre-defined (CMD23)

              KB  reclen   write rewrite    read    reread
           10240       4    8799    8061    18069    18212
           10240       8    8850    8465    17938    18084
           10240      16    8077    9171    18131    18249
           10240      32    8810    8209    18019    17999
           10240      64    8744    8214    18096    18204
           10240     128    7940   10041    18036    18043
           10240     256    8466    7557    18101    18186
           10240     512    7372    8486    18272    18010
           10240    1024    5565    9922    18155    18196
           10240    2048    9383    9686    18116    18049
           10240    4096    9282    8792    18073    18154
           10240    8192    3770   10136    18083    18155

Regards,
Jaehoon Chung


Andrei Warkentin wrote:
> On Fri, May 20, 2011 at 1:49 AM, Andrei Warkentin <andreiw@motorola.com> wrote:
>> On Thu, May 19, 2011 at 11:38 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> Hi Andrei,
>>>
>>> Andrei Warkentin wrote:
>>>> On Wed, May 18, 2011 at 9:37 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>>> Hi Andrei
>>>>>
>>>>> Andrei Warkentin wrote:
>>>>>> CMD23-prefixed instead of open-ended multiblock transfers
>>>>>> have a performance advantage on some MMC cards.
>>>>>>
>>>>> you mentioned about "some MMC cards".
>>>>> Conversely, that means the some card didn't have a performance advantage?
>>>>>
>>>>> Did you find the performance advantage?
>>>>> if you found the advantage and you can tell me,
>>>>> i want to know what do you have the some MMC cards..
>>>>>
>>>> I've tested this on a Sandisk eMMC where I saw as good as
>>>> a 50% improvement on writes (30% real-life use cases). This was a
>>>> SEM32G 4.3+ part.
>>> Can you tell me your environment? buswidth, AP information, benchmark etc..
>>> And if you have the performance result's data, can you share them?
>>>
>> This was on an SDHCI controller (hence the patch...) on a Tegra
>> 2-based system. I was measuring
>> throughput on reads and writes (obviously without block cache,
>> filesystem, etc) to an eMMC card, 8 bits.
>>
>> Tested both with my tool (https://github.com/andreiw/superalign) and
>> an sqllite-based test.
>>
>> I'm attaching the data I have.
>>
>> A
>>
> 
> Additionally, CMD23 use is a requirement for SDXC cards (Arindam can
> comment on that), as well as for MMC reliable writes and eMMC 4.5-spec
> features (Yunpeng Gao can comment on that).
> 
> These patches allow CMD23 use. They do involve a some changes to host
> controller because of interaction with CMD12, as well as Auto-CMD12
> and Auto-CMD23 features. I can definitely consult you if you need help
> implementing CMD23 support for whatever controller you develop for.
> 
> A
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [[v4] 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Andrei Warkentin]

On Mon, May 23, 2011 at 7:40 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi A..
>
> I tested your patch..(using CMD23)
> my environment is the below.
> eMMC card : Sandisk SEM8G (eMMC 4.3+)
> buswidth : 4bit (SDR)
> AP : C110
> benchmark : IOzone
>
> I want to know how do you think about this result?
> (i can't see your results)
>

I think that you should use my tool to measure I/O performance.
Because I want to see the mins, maxes, average and
std dev. Iozone adds way too much noise to the data. You can run it 5
times in a row and get completely different numbers. Please use
https://github.com/andreiw/superalign.

For a more realistic test you can try performing 20000 sqlite inserts
or something of the sort and timing that.

A

Re: [[v4] 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Andrei Warkentin]

On Mon, May 23, 2011 at 2:25 PM, Andrei Warkentin <andreiw@motorola.com> wrote:
> On Mon, May 23, 2011 at 7:40 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Hi A..
>>
>> I tested your patch..(using CMD23)
>> my environment is the below.
>> eMMC card : Sandisk SEM8G (eMMC 4.3+)
>> buswidth : 4bit (SDR)
>> AP : C110
>> benchmark : IOzone
>>
>> I want to know how do you think about this result?
>> (i can't see your results)
>>
>
> I think that you should use my tool to measure I/O performance.
> Because I want to see the mins, maxes, average and
> std dev. Iozone adds way too much noise to the data. You can run it 5
> times in a row and get completely different numbers. Please use
> https://github.com/andreiw/superalign.
>
> For a more realistic test you can try performing 20000 sqlite inserts
> or something of the sort and timing that.
>
> A
>

Additionally, be careful how you do your testing. I don't want to
sound obvious, but, to ensure you can actually compare the collected
data against each other -
1) Disable all power/frequency scaling/management/gating, suspend/resume, etc.
2) Make sure nothing else uses the eMMC. No root mounted fs, nothing.
3) Make sure you are avoiding block cache and file system. You want
direct block I/O.
4) For extra extra extra reliable results - Make sure you are not
rebooting across testing. You will need to add a flag so you can
disable CMD23 on the fly via debugfs.

For sqlite testing some other helpful hints -
1) Unmount partition containing files on which the SQLite test operates.
2) Perform BLKDISCARD over the partition.
3) Format with desired file system.
4) Mount.
6) Sync && echo 3 > /proc/sys/vm/drop_caches
7) Perform test.
8) Umount.
9) Repeat from (1)

A

Re: [[v4] 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Andrei Warkentin]

On Mon, May 23, 2011 at 7:40 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi A..
>
> I tested your patch..(using CMD23)
> my environment is the below.
> eMMC card : Sandisk SEM8G (eMMC 4.3+)
> buswidth : 4bit (SDR)
> AP : C110
> benchmark : IOzone
>

Knowing what controller you are on is helpful too. Unless you are on
SDHCI, you will see no effect.

A

CMD23 plumbing patchset.

[Andrei Warkentin]

This is the 6th version of the patchset. Made sure that Auto-CMD(12|23)
is called *_AUTO_CMD* instead of *_ACMD*.

Also removed an unnecessary check for v3 spec, since one is already in place
after Arindam's patches.

ToC:
[v6 1/5] MMC: Add/remove quirks conditional support.
[v6 2/5] MMC: Use CMD23 for multiblock transfers when we can.
[v6 3/5] MMC: Implement MMC_CAP_CMD23 for SDHCI.
[v6 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.
[v6 5/5] MMC: SDHCI AutoCMD23 support.

A

[v6 1/5] MMC: Add/remove quirks conditional support.

[Andrei Warkentin]

Conditional add/remove quirks for MMC and SD.

Cc: cjb@laptop.org
Cc: malchev@google.com
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 include/linux/mmc/card.h |   40 ++++++++++++++++++++++++++++++++++++----
 1 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 4910dec..7190aa2 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -273,16 +273,14 @@ struct mmc_fixup {
 		    card->cid.month)
 
 /*
- * This hook just adds a quirk unconditionally.
+ * Unconditionally quirk add/remove.
  */
+
 static inline void __maybe_unused add_quirk(struct mmc_card *card, int data)
 {
 	card->quirks |= data;
 }
 
-/*
- * This hook just removes a quirk unconditionally.
- */
 static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
 {
 	card->quirks &= ~data;
@@ -308,6 +306,40 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
 #define mmc_sd_card_set_uhs(c) ((c)->state |= MMC_STATE_ULTRAHIGHSPEED)
 #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC)
 
+/*
+ * Quirk add/remove for MMC products.
+ */
+
+static inline void __maybe_unused add_quirk_mmc(struct mmc_card *card, int data)
+{
+	if (mmc_card_mmc(card))
+		card->quirks |= data;
+}
+
+static inline void __maybe_unused remove_quirk_mmc(struct mmc_card *card,
+						   int data)
+{
+	if (mmc_card_mmc(card))
+		card->quirks &= ~data;
+}
+
+/*
+ * Quirk add/remove for SD products.
+ */
+
+static inline void __maybe_unused add_quirk_sd(struct mmc_card *card, int data)
+{
+	if (mmc_card_sd(card))
+		card->quirks |= data;
+}
+
+static inline void __maybe_unused remove_quirk_sd(struct mmc_card *card,
+						   int data)
+{
+	if (mmc_card_sd(card))
+		card->quirks &= ~data;
+}
+
 static inline int mmc_card_lenient_fn0(const struct mmc_card *c)
 {
 	return c->quirks & MMC_QUIRK_LENIENT_FN0;
-- 
1.7.0.4

[v6 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Andrei Warkentin]

CMD23-prefixed instead of open-ended multiblock transfers
have a performance advantage on some MMC cards.

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Cc: malchev@google.com
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/card/block.c |  108 +++++++++++++++++++++++++++++++++------------
 include/linux/mmc/card.h |    1 +
 include/linux/mmc/core.h |    1 +
 include/linux/mmc/host.h |    6 +++
 include/linux/mmc/mmc.h  |    6 +++
 5 files changed, 93 insertions(+), 29 deletions(-)
 mode change 100644 => 100755 drivers/mmc/card/block.c

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
old mode 100644
new mode 100755
index 126c7f4..a380acc
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -59,10 +59,6 @@ MODULE_ALIAS("mmc:block");
 #define INAND_CMD38_ARG_SECTRIM1 0x81
 #define INAND_CMD38_ARG_SECTRIM2 0x88
 
-#define REL_WRITES_SUPPORTED(card) (mmc_card_mmc((card)) &&	\
-    (((card)->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||	\
-     ((card)->ext_csd.rel_sectors)))
-
 static DEFINE_MUTEX(block_mutex);
 
 /*
@@ -90,6 +86,10 @@ struct mmc_blk_data {
 	struct mmc_queue queue;
 	struct list_head part;
 
+	unsigned int	flags;
+#define MMC_BLK_CMD23	(1 << 0)	/* Can do SET_BLOCK_COUNT for multiblock */
+#define MMC_BLK_REL_WR	(1 << 1)	/* MMC Reliable write support */
+
 	unsigned int	usage;
 	unsigned int	read_only;
 	unsigned int	part_type;
@@ -429,6 +429,7 @@ static const struct block_device_operations mmc_bdops = {
 
 struct mmc_blk_request {
 	struct mmc_request	mrq;
+	struct mmc_command	sbc;
 	struct mmc_command	cmd;
 	struct mmc_command	stop;
 	struct mmc_data		data;
@@ -652,13 +653,10 @@ static int mmc_blk_issue_flush(struct mmc_queue *mq, struct request *req)
  * reliable write can handle, thus finish the request in
  * partial completions.
  */
-static inline int mmc_apply_rel_rw(struct mmc_blk_request *brq,
-				   struct mmc_card *card,
-				   struct request *req)
+static inline void mmc_apply_rel_rw(struct mmc_blk_request *brq,
+				    struct mmc_card *card,
+				    struct request *req)
 {
-	int err;
-	struct mmc_command set_count = {0};
-
 	if (!(card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN)) {
 		/* Legacy mode imposes restrictions on transfers. */
 		if (!IS_ALIGNED(brq->cmd.arg, card->ext_csd.rel_sectors))
@@ -669,15 +667,6 @@ static inline int mmc_apply_rel_rw(struct mmc_blk_request *brq,
 		else if (brq->data.blocks < card->ext_csd.rel_sectors)
 			brq->data.blocks = 1;
 	}
-
-	set_count.opcode = MMC_SET_BLOCK_COUNT;
-	set_count.arg = brq->data.blocks | (1 << 31);
-	set_count.flags = MMC_RSP_R1 | MMC_CMD_AC;
-	err = mmc_wait_for_cmd(card->host, &set_count, 0);
-	if (err)
-		printk(KERN_ERR "%s: error %d SET_BLOCK_COUNT\n",
-		       req->rq_disk->disk_name, err);
-	return err;
 }
 
 static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
@@ -694,7 +683,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 	bool do_rel_wr = ((req->cmd_flags & REQ_FUA) ||
 			  (req->cmd_flags & REQ_META)) &&
 		(rq_data_dir(req) == WRITE) &&
-		REL_WRITES_SUPPORTED(card);
+		(md->flags & MMC_BLK_REL_WR);
 
 	do {
 		struct mmc_command cmd = {0};
@@ -732,11 +721,9 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 
 		if (brq.data.blocks > 1 || do_rel_wr) {
 			/* SPI multiblock writes terminate using a special
-			 * token, not a STOP_TRANSMISSION request. Reliable
-			 * writes use SET_BLOCK_COUNT and do not use a
-			 * STOP_TRANSMISSION request either.
+			 * token, not a STOP_TRANSMISSION request.
 			 */
-			if ((!mmc_host_is_spi(card->host) && !do_rel_wr) ||
+			if (!mmc_host_is_spi(card->host) ||
 			    rq_data_dir(req) == READ)
 				brq.mrq.stop = &brq.stop;
 			readcmd = MMC_READ_MULTIPLE_BLOCK;
@@ -754,8 +741,37 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 			brq.data.flags |= MMC_DATA_WRITE;
 		}
 
-		if (do_rel_wr && mmc_apply_rel_rw(&brq, card, req))
-			goto cmd_err;
+		if (do_rel_wr)
+			mmc_apply_rel_rw(&brq, card, req);
+
+		/*
+		 * Pre-defined multi-block transfers are preferable to
+		 * open ended-ones (and necessary for reliable writes).
+		 * However, it is not sufficient to just send CMD23,
+		 * and avoid the final CMD12, as on an error condition
+		 * CMD12 (stop) needs to be sent anyway. This, coupled
+		 * with Auto-CMD23 enhancements provided by some
+		 * hosts, means that the complexity of dealing
+		 * with this is best left to the host. If CMD23 is
+		 * supported by card and host, we'll fill sbc in and let
+		 * the host deal with handling it correctly. This means
+		 * that for hosts that don't expose MMC_CAP_CMD23, no
+		 * change of behavior will be observed.
+		 *
+		 * N.B: Some MMC cards experience perf degradation.
+		 * We'll avoid using CMD23-bounded multiblock writes for
+		 * these, while retaining features like reliable writes.
+		 */
+
+		if ((md->flags & MMC_BLK_CMD23) &&
+		    mmc_op_multi(brq.cmd.opcode) &&
+		    (do_rel_wr || !(card->quirks & MMC_QUIRK_BLK_NO_CMD23))) {
+			brq.sbc.opcode = MMC_SET_BLOCK_COUNT;
+			brq.sbc.arg = brq.data.blocks |
+				(do_rel_wr ? (1 << 31) : 0);
+			brq.sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
+			brq.mrq.sbc = &brq.sbc;
+		}
 
 		mmc_set_data_timeout(&brq.data, card);
 
@@ -792,7 +808,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 		 * until later as we need to wait for the card to leave
 		 * programming mode even when things go wrong.
 		 */
-		if (brq.cmd.error || brq.data.error || brq.stop.error) {
+		if (brq.sbc.error || brq.cmd.error ||
+		    brq.data.error || brq.stop.error) {
 			if (brq.data.blocks > 1 && rq_data_dir(req) == READ) {
 				/* Redo read one sector at a time */
 				printk(KERN_WARNING "%s: retrying using single "
@@ -803,6 +820,13 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 			status = get_card_status(card, req);
 		}
 
+		if (brq.sbc.error) {
+			printk(KERN_ERR "%s: error %d sending SET_BLOCK_COUNT "
+			       "command, response %#x, card status %#x\n",
+			       req->rq_disk->disk_name, brq.sbc.error,
+			       brq.sbc.resp[0], status);
+		}
+
 		if (brq.cmd.error) {
 			printk(KERN_ERR "%s: error %d sending read/write "
 			       "command, response %#x, card status %#x\n",
@@ -1014,8 +1038,6 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	md->disk->queue = md->queue.queue;
 	md->disk->driverfs_dev = parent;
 	set_disk_ro(md->disk, md->read_only || default_ro);
-	if (REL_WRITES_SUPPORTED(card))
-		blk_queue_flush(md->queue.queue, REQ_FLUSH | REQ_FUA);
 
 	/*
 	 * As discussed on lkml, GENHD_FL_REMOVABLE should:
@@ -1034,6 +1056,19 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 
 	blk_queue_logical_block_size(md->queue.queue, 512);
 	set_capacity(md->disk, size);
+
+	if (mmc_host_cmd23(card->host) &&
+	    mmc_card_mmc(card))
+		md->flags |= MMC_BLK_CMD23;
+
+	if (mmc_card_mmc(card) &&
+	    md->flags & MMC_BLK_CMD23 &&
+	    ((card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||
+	     card->ext_csd.rel_sectors)) {
+		md->flags |= MMC_BLK_REL_WR;
+		blk_queue_flush(md->queue.queue, REQ_FLUSH | REQ_FUA);
+	}
+
 	return md;
 
  err_putdisk:
@@ -1189,6 +1224,21 @@ static const struct mmc_fixup blk_fixups[] =
 	MMC_FIXUP("SEM08G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
 	MMC_FIXUP("SEM16G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
 	MMC_FIXUP("SEM32G", 0x2, 0x100, add_quirk, MMC_QUIRK_INAND_CMD38),
+
+	/*
+	 * Some MMC cards experience performance degradation with CMD23
+	 * instead of CMD12-bounded multiblock transfers. For now we'll
+	 * black list what's bad...
+	 * - Certain Toshiba cards.
+	 *
+	 * N.B. This doesn't affect SD cards.
+	 */
+	MMC_FIXUP("MMC08G", 0x11, CID_OEMID_ANY, add_quirk_mmc,
+		  MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("MMC16G", 0x11, CID_OEMID_ANY, add_quirk_mmc,
+		  MMC_QUIRK_BLK_NO_CMD23),
+	MMC_FIXUP("MMC32G", 0x11, CID_OEMID_ANY, add_quirk_mmc,
+		  MMC_QUIRK_BLK_NO_CMD23),
 	END_FIXUP
 };
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 7190aa2..4a0e27b 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -171,6 +171,7 @@ struct mmc_card {
 #define MMC_QUIRK_NONSTD_FUNC_IF (1<<4)		/* SDIO card has nonstd function interfaces */
 #define MMC_QUIRK_DISABLE_CD	(1<<5)		/* disconnect CD/DAT[3] resistor */
 #define MMC_QUIRK_INAND_CMD38	(1<<6)		/* iNAND devices have broken CMD38 */
+#define MMC_QUIRK_BLK_NO_CMD23	(1<<7)		/* Avoid CMD23 for regular multiblock */
 
 	unsigned int		erase_size;	/* erase size in sectors */
  	unsigned int		erase_shift;	/* if erase unit is power 2 */
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index cbe8d55..b6718e5 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -120,6 +120,7 @@ struct mmc_data {
 };
 
 struct mmc_request {
+	struct mmc_command	*sbc;		/* SET_BLOCK_COUNT for multiblock */
 	struct mmc_command	*cmd;
 	struct mmc_data		*data;
 	struct mmc_command	*stop;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index de32e6a..e946bd1 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -210,6 +210,7 @@ struct mmc_host {
 #define MMC_CAP_MAX_CURRENT_400	(1 << 27)	/* Host max current limit is 400mA */
 #define MMC_CAP_MAX_CURRENT_600	(1 << 28)	/* Host max current limit is 600mA */
 #define MMC_CAP_MAX_CURRENT_800	(1 << 29)	/* Host max current limit is 800mA */
+#define MMC_CAP_CMD23		(1 << 30)	/* CMD23 supported. */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 
@@ -366,5 +367,10 @@ static inline int mmc_card_wake_sdio_irq(struct mmc_host *host)
 {
 	return host->pm_flags & MMC_PM_WAKE_SDIO_IRQ;
 }
+
+static inline int mmc_host_cmd23(struct mmc_host *host)
+{
+	return host->caps & MMC_CAP_CMD23;
+}
 #endif
 
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 9fa5a73..848aab2 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -83,6 +83,12 @@
 #define MMC_APP_CMD              55   /* ac   [31:16] RCA        R1  */
 #define MMC_GEN_CMD              56   /* adtc [0] RD/WR          R1  */
 
+static inline bool mmc_op_multi(u32 opcode)
+{
+	return opcode == MMC_WRITE_MULTIPLE_BLOCK ||
+		opcode == MMC_READ_MULTIPLE_BLOCK;
+}
+
 /*
  * MMC_SWITCH argument format:
  *
-- 
1.7.0.4

[v6 3/5] MMC: Implement MMC_CAP_CMD23 for SDHCI.

[Andrei Warkentin]

Implements support for multiblock transfers bounded
by SET_BLOCK_COUNT (CMD23).

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Cc: malchev@google.com
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/host/sdhci.c  |   66 ++++++++++++++++++++++++++++++++++-----------
 drivers/mmc/host/sdhci.h  |    2 +-
 include/linux/mmc/sdhci.h |    1 +
 3 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index cc63f5e..3a53512 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -838,9 +838,10 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 }
 
 static void sdhci_set_transfer_mode(struct sdhci_host *host,
-	struct mmc_data *data)
+	struct mmc_command *cmd)
 {
 	u16 mode;
+	struct mmc_data *data = cmd->data;
 
 	if (data == NULL)
 		return;
@@ -848,11 +849,17 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
 	WARN_ON(!host->data);
 
 	mode = SDHCI_TRNS_BLK_CNT_EN;
-	if (data->blocks > 1) {
-		if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
-			mode |= SDHCI_TRNS_MULTI | SDHCI_TRNS_ACMD12;
-		else
-			mode |= SDHCI_TRNS_MULTI;
+	if (mmc_op_multi(cmd->opcode) ||
+	    data->blocks > 1) {
+		mode |= SDHCI_TRNS_MULTI;
+
+		/*
+		 * If we are sending CMD23, CMD12 never gets sent
+		 * on successful completion (so no Auto-CMD12).
+		 */
+		if (!host->mrq->sbc &&
+		    host->flags & SDHCI_AUTO_CMD12)
+			mode |= SDHCI_TRNS_AUTO_CMD12;
 	}
 	if (data->flags & MMC_DATA_READ)
 		mode |= SDHCI_TRNS_READ;
@@ -893,7 +900,15 @@ static void sdhci_finish_data(struct sdhci_host *host)
 	else
 		data->bytes_xfered = data->blksz * data->blocks;
 
-	if (data->stop) {
+	/*
+	 * Need to send CMD12 if -
+	 * a) open-ended multiblock transfer (no CMD23)
+	 * b) error in multiblock transfer
+	 */
+	if (data->stop &&
+	    (data->error ||
+	     !host->mrq->sbc)) {
+
 		/*
 		 * The controller needs a reset of internal state machines
 		 * upon error conditions.
@@ -949,7 +964,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 
 	sdhci_writel(host, cmd->arg, SDHCI_ARGUMENT);
 
-	sdhci_set_transfer_mode(host, cmd->data);
+	sdhci_set_transfer_mode(host, cmd);
 
 	if ((cmd->flags & MMC_RSP_136) && (cmd->flags & MMC_RSP_BUSY)) {
 		printk(KERN_ERR "%s: Unsupported response type!\n",
@@ -1004,13 +1019,21 @@ static void sdhci_finish_command(struct sdhci_host *host)
 
 	host->cmd->error = 0;
 
-	if (host->data && host->data_early)
-		sdhci_finish_data(host);
+	/* Finished CMD23, now send actual command. */
+	if (host->cmd == host->mrq->sbc) {
+		host->cmd = NULL;
+		sdhci_send_command(host, host->mrq->cmd);
+	} else {
 
-	if (!host->cmd->data)
-		tasklet_schedule(&host->finish_tasklet);
+		/* Processed actual command. */
+		if (host->data && host->data_early)
+			sdhci_finish_data(host);
 
-	host->cmd = NULL;
+		if (!host->cmd->data)
+			tasklet_schedule(&host->finish_tasklet);
+
+		host->cmd = NULL;
+	}
 }
 
 static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
@@ -1189,7 +1212,12 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 #ifndef SDHCI_USE_LEDS_CLASS
 	sdhci_activate_led(host);
 #endif
-	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12) {
+
+	/*
+	 * Ensure we don't send the STOP for non-SET_BLOCK_COUNTED
+	 * requests if Auto-CMD12 is enabled.
+	 */
+	if (!mrq->sbc && (host->flags & SDHCI_AUTO_CMD12)) {
 		if (mrq->stop) {
 			mrq->data->stop = NULL;
 			mrq->stop = NULL;
@@ -1227,7 +1255,10 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 			host->mrq = mrq;
 		}
 
-		sdhci_send_command(host, mrq->cmd);
+		if (mrq->sbc)
+			sdhci_send_command(host, mrq->sbc);
+		else
+			sdhci_send_command(host, mrq->cmd);
 	}
 
 	mmiowb();
@@ -2455,7 +2486,10 @@ int sdhci_add_host(struct sdhci_host *host)
 	} else
 		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;
 
-	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE;
+	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;
+
+	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
+		host->flags |= SDHCI_AUTO_CMD12;
 
 	/*
 	 * A controller may support 8-bit width, but the board itself
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 7e28eec..2c3fbc5 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -36,7 +36,7 @@
 #define SDHCI_TRANSFER_MODE	0x0C
 #define  SDHCI_TRNS_DMA		0x01
 #define  SDHCI_TRNS_BLK_CNT_EN	0x02
-#define  SDHCI_TRNS_ACMD12	0x04
+#define  SDHCI_TRNS_AUTO_CMD12	0x04
 #define  SDHCI_TRNS_READ	0x10
 #define  SDHCI_TRNS_MULTI	0x20
 
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index e902618..73e27ba 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -113,6 +113,7 @@ struct sdhci_host {
 #define SDHCI_DEVICE_DEAD	(1<<3)	/* Device unresponsive */
 #define SDHCI_SDR50_NEEDS_TUNING (1<<4)	/* SDR50 needs tuning */
 #define SDHCI_NEEDS_RETUNING	(1<<5)	/* Host needs retuning */
+#define SDHCI_AUTO_CMD12	(1<<6)	/* Auto CMD12 support */
 
 	unsigned int version;	/* SDHCI spec. version */
 
-- 
1.7.0.4

[v6 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.

[Andrei Warkentin]

SD cards operating at UHS104 or better support SET_BLOCK_COUNT.

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Cc: malchev@google.com
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/card/block.c |    9 ++++++---
 drivers/mmc/core/sd.c    |    2 ++
 include/linux/mmc/card.h |    3 +++
 include/linux/mmc/sd.h   |    2 +-
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index a380acc..71da564 100755
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1057,9 +1057,12 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	blk_queue_logical_block_size(md->queue.queue, 512);
 	set_capacity(md->disk, size);
 
-	if (mmc_host_cmd23(card->host) &&
-	    mmc_card_mmc(card))
-		md->flags |= MMC_BLK_CMD23;
+	if (mmc_host_cmd23(card->host)) {
+		if (mmc_card_mmc(card) ||
+		    (mmc_card_sd(card) &&
+		     card->scr.cmds & SD_SCR_CMD23_SUPPORT))
+			md->flags |= MMC_BLK_CMD23;
+	}
 
 	if (mmc_card_mmc(card) &&
 	    md->flags & MMC_BLK_CMD23 &&
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 596d0b9..ff27741 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -203,6 +203,8 @@ static int mmc_decode_scr(struct mmc_card *card)
 	else
 		card->erased_byte = 0x0;
 
+	if (scr->sda_spec3)
+		scr->cmds = UNSTUFF_BITS(resp, 32, 2);
 	return 0;
 }
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 4a0e27b..c6927a4 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -72,6 +72,9 @@ struct sd_scr {
 	unsigned char		bus_widths;
 #define SD_SCR_BUS_WIDTH_1	(1<<0)
 #define SD_SCR_BUS_WIDTH_4	(1<<2)
+	unsigned char		cmds;
+#define SD_SCR_CMD20_SUPPORT   (1<<0)
+#define SD_SCR_CMD23_SUPPORT   (1<<1)
 };
 
 struct sd_ssr {
diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h
index c648878..7d35d52 100644
--- a/include/linux/mmc/sd.h
+++ b/include/linux/mmc/sd.h
@@ -66,7 +66,7 @@
 
 #define SCR_SPEC_VER_0		0	/* Implements system specification 1.0 - 1.01 */
 #define SCR_SPEC_VER_1		1	/* Implements system specification 1.10 */
-#define SCR_SPEC_VER_2		2	/* Implements system specification 2.00 */
+#define SCR_SPEC_VER_2		2	/* Implements system specification 2.00-3.0X */
 
 /*
  * SD bus widths
-- 
1.7.0.4

[v6 5/5] MMC: SDHCI AutoCMD23 support.

[Andrei Warkentin]

Enables Auto-CMD23 support where available (SDHCI 3.0 controllers)

Cc: arindam.nath@amd.com
Cc: cjb@laptop.org
Cc: arnd@arndb.de
Cc: subhashj@codeaurora.org
Cc: malchev@google.com
Tested-by: Arindam Nath <arindam.nath@amd.com>
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 drivers/mmc/host/sdhci.c  |   17 ++++++++++++++++-
 drivers/mmc/host/sdhci.h  |    2 ++
 include/linux/mmc/sdhci.h |    1 +
 3 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 3a53512..fbb1842 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -860,7 +860,13 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
 		if (!host->mrq->sbc &&
 		    host->flags & SDHCI_AUTO_CMD12)
 			mode |= SDHCI_TRNS_AUTO_CMD12;
+		else if (host->mrq->sbc &&
+			 host->flags & SDHCI_AUTO_CMD23) {
+			mode |= SDHCI_TRNS_AUTO_CMD23;
+			sdhci_writel(host, host->mrq->sbc->arg, SDHCI_ARGUMENT2);
+		}
 	}
+
 	if (data->flags & MMC_DATA_READ)
 		mode |= SDHCI_TRNS_READ;
 	if (host->flags & SDHCI_REQ_USE_DMA)
@@ -1255,7 +1261,7 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 			host->mrq = mrq;
 		}
 
-		if (mrq->sbc)
+		if (mrq->sbc && !(host->flags & SDHCI_AUTO_CMD23))
 			sdhci_send_command(host, mrq->sbc);
 		else
 			sdhci_send_command(host, mrq->cmd);
@@ -2491,6 +2497,15 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
 		host->flags |= SDHCI_AUTO_CMD12;
 
+	/* Auto-CMD23 stuff only works in ADMA or PIO. */
+	if ((host->version == SDHCI_SPEC_300) &&
+	    ((host->flags & SDHCI_USE_ADMA) ||
+	     !(host->flags & SDHCI_REQ_USE_DMA))) {
+		host->flags |= SDHCI_AUTO_CMD23;
+		printk(KERN_INFO "%s: Auto-CMD23 available\n", mmc_hostname(mmc));
+	} else
+		printk(KERN_INFO "%s: Auto-CMD23 unavailable\n", mmc_hostname(mmc));
+
 	/*
 	 * A controller may support 8-bit width, but the board itself
 	 * might not have the pins brought out.  Boards that support
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 2c3fbc5..745c42f 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -25,6 +25,7 @@
  */
 
 #define SDHCI_DMA_ADDRESS	0x00
+#define SDHCI_ARGUMENT2		SDHCI_DMA_ADDRESS
 
 #define SDHCI_BLOCK_SIZE	0x04
 #define  SDHCI_MAKE_BLKSZ(dma, blksz) (((dma & 0x7) << 12) | (blksz & 0xFFF))
@@ -37,6 +38,7 @@
 #define  SDHCI_TRNS_DMA		0x01
 #define  SDHCI_TRNS_BLK_CNT_EN	0x02
 #define  SDHCI_TRNS_AUTO_CMD12	0x04
+#define  SDHCI_TRNS_AUTO_CMD23	0x08
 #define  SDHCI_TRNS_READ	0x10
 #define  SDHCI_TRNS_MULTI	0x20
 
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 73e27ba..6a68c4e 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -114,6 +114,7 @@ struct sdhci_host {
 #define SDHCI_SDR50_NEEDS_TUNING (1<<4)	/* SDR50 needs tuning */
 #define SDHCI_NEEDS_RETUNING	(1<<5)	/* Host needs retuning */
 #define SDHCI_AUTO_CMD12	(1<<6)	/* Auto CMD12 support */
+#define SDHCI_AUTO_CMD23	(1<<7)	/* Auto CMD23 support */
 
 	unsigned int version;	/* SDHCI spec. version */
 
-- 
1.7.0.4

Re: [[v4] 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Andrei Warkentin]

On Mon, May 23, 2011 at 2:34 PM, Andrei Warkentin <andreiw@motorola.com> wrote:
> On Mon, May 23, 2011 at 7:40 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Hi A..
>>
>> I tested your patch..(using CMD23)
>> my environment is the below.
>> eMMC card : Sandisk SEM8G (eMMC 4.3+)
>> buswidth : 4bit (SDR)
>> AP : C110
>> benchmark : IOzone
>>
>
> Knowing what controller you are on is helpful too. Unless you are on
> SDHCI, you will see no effect.
>
> A
>

If after all you still see no improvement, you should talk to your
Sandisk representative if your particular batch actually is new enough
to support CMD23 improvements. Sandisk ships different versions of
hardware at the same time, so it's pretty impossible to tell it apart
other than some vendor specific EXT_CSD fields...

A

Re: [[v4] 2/5] MMC: Use CMD23 for multiblock transfers when we can.

[Jaehoon Chung]

Andrei Warkentin wrote:
> On Mon, May 23, 2011 at 7:40 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Hi A..
>>
>> I tested your patch..(using CMD23)
>> my environment is the below.
>> eMMC card : Sandisk SEM8G (eMMC 4.3+)
>> buswidth : 4bit (SDR)
>> AP : C110
>> benchmark : IOzone
>>
> 
> Knowing what controller you are on is helpful too. Unless you are on
> SDHCI, you will see no effect.
> 
I'm using SDHCI controller and tested with them.
I'll try to test with your comment and share the results.

Regards,
Jaehoon Chung

RE: [v6 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.

[Nath, Arindam]

Reviewed-by: Arindam Nath <arindam.nath@amd.com>

Thanks,
Arindam

> -----Original Message-----
> From: Andrei Warkentin [mailto:andreiw@motorola.com]
> Sent: Tuesday, May 24, 2011 1:37 AM
> To: linux-mmc@vger.kernel.org
> Cc: cjb@laptop.org; Andrei Warkentin; Nath, Arindam; arnd@arndb.de;
> malchev@google.com
> Subject: [v6 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.
> 
> SD cards operating at UHS104 or better support SET_BLOCK_COUNT.
> 
> Cc: arindam.nath@amd.com
> Cc: cjb@laptop.org
> Cc: arnd@arndb.de
> Cc: malchev@google.com
> Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
> ---
>  drivers/mmc/card/block.c |    9 ++++++---
>  drivers/mmc/core/sd.c    |    2 ++
>  include/linux/mmc/card.h |    3 +++
>  include/linux/mmc/sd.h   |    2 +-
>  4 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index a380acc..71da564 100755
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -1057,9 +1057,12 @@ static struct mmc_blk_data
> *mmc_blk_alloc_req(struct mmc_card *card,
>  	blk_queue_logical_block_size(md->queue.queue, 512);
>  	set_capacity(md->disk, size);
> 
> -	if (mmc_host_cmd23(card->host) &&
> -	    mmc_card_mmc(card))
> -		md->flags |= MMC_BLK_CMD23;
> +	if (mmc_host_cmd23(card->host)) {
> +		if (mmc_card_mmc(card) ||
> +		    (mmc_card_sd(card) &&
> +		     card->scr.cmds & SD_SCR_CMD23_SUPPORT))
> +			md->flags |= MMC_BLK_CMD23;
> +	}
> 
>  	if (mmc_card_mmc(card) &&
>  	    md->flags & MMC_BLK_CMD23 &&
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 596d0b9..ff27741 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -203,6 +203,8 @@ static int mmc_decode_scr(struct mmc_card *card)
>  	else
>  		card->erased_byte = 0x0;
> 
> +	if (scr->sda_spec3)
> +		scr->cmds = UNSTUFF_BITS(resp, 32, 2);
>  	return 0;
>  }
> 
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 4a0e27b..c6927a4 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -72,6 +72,9 @@ struct sd_scr {
>  	unsigned char		bus_widths;
>  #define SD_SCR_BUS_WIDTH_1	(1<<0)
>  #define SD_SCR_BUS_WIDTH_4	(1<<2)
> +	unsigned char		cmds;
> +#define SD_SCR_CMD20_SUPPORT   (1<<0)
> +#define SD_SCR_CMD23_SUPPORT   (1<<1)
>  };
> 
>  struct sd_ssr {
> diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h
> index c648878..7d35d52 100644
> --- a/include/linux/mmc/sd.h
> +++ b/include/linux/mmc/sd.h
> @@ -66,7 +66,7 @@
> 
>  #define SCR_SPEC_VER_0		0	/* Implements system specification
> 1.0 - 1.01 */
>  #define SCR_SPEC_VER_1		1	/* Implements system specification
> 1.10 */
> -#define SCR_SPEC_VER_2		2	/* Implements system specification
> 2.00 */
> +#define SCR_SPEC_VER_2		2	/* Implements system specification
> 2.00-3.0X */
> 
>  /*
>   * SD bus widths
> --
> 1.7.0.4
> 

Re: [v6 5/5] MMC: SDHCI AutoCMD23 support.

[Chris Ball]

Hi Andrei,

On Mon, May 23 2011, Andrei Warkentin wrote:
> Enables Auto-CMD23 support where available (SDHCI 3.0 controllers)
>
> Cc: arindam.nath@amd.com
> Cc: cjb@laptop.org
> Cc: arnd@arndb.de
> Cc: subhashj@codeaurora.org
> Cc: malchev@google.com
> Tested-by: Arindam Nath <arindam.nath@amd.com>
> Signed-off-by: Andrei Warkentin <andreiw@motorola.com>

Looks like this patch is breaking boot on my XO-1.75/Marvell MMP2 A2
with SD 3.0 controller.  :/

[    1.217156] sdhci: Secure Digital Host Controller Interface driver
[    1.223361] sdhci: Copyright(c) Pierre Ossman
[    1.227793] mmc0: Auto-CMD23 available
[    1.227793] mmc0: no vmmc regulator found
[    1.242557] mmc0: SDHCI controller on MMC [sdhci-pxa.0] using DMA
[    1.248748] mmc1: Auto-CMD23 available
[    1.252479] mmc1: no vmmc regulator found
[    1.256599] mmc1: SDHCI controller on MMC [sdhci-pxa.1] using DMA
[    1.262694] mmc2: Auto-CMD23 available
[    1.266571] mmc2: no vmmc regulator found
[    1.270655] mmc2: SDHCI controller on MMC [sdhci-pxa.2] using DMA
[    1.656280] mmc1: new SDIO card at address 0001
[    2.423849] libertas: 20:7c:8f:4a:79:c5, fw 9.70.7p0, cap 0x000003a3
[    2.432951] libertas: wlan0: Marvell WLAN 802.11 adapter
[    2.438262] Waiting for root device /dev/mmcblk0p2...
[    2.528607] mmc2: new high speed DDR MMC card at address 0001
[    2.534597] mmcblk0: mmc2:0001 SEM04G 3.68 GiB
[    2.539198] mmcblk0boot0: mmc2:0001 SEM04G partition 1 1.00 MiB
[    2.545166] mmcblk0boot1: mmc2:0001 SEM04G partition 2 1.00 MiB
[    2.555985]  mmcblk0: unknown partition table
^^^^^
[    2.562610]  mmcblk0boot1: unknown partition table
[    2.569715]  mmcblk0boot0: unknown partition table
[    2.663264] VFS: Cannot open root device "mmcblk0p2" or unknown-block(179,2)

With the patch reverted, mmcblk0 comes up with partitions 1 and 2 and
mmcblk0p2 is mounted.  In this log, we fail to find a partition table
and panic.

Any ideas/suggestions?  I'm surprised that we enabled Auto-CMD23 on this
hardware; it is a v3 controller, but it has SDHCI_QUIRK_BROKEN_ADMA set..

Philip/Zhangfei, were you able to test this patch on MMP2?

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [v6 5/5] MMC: SDHCI AutoCMD23 support.

[Andrei Warkentin]

Hi Chris,

On Tue, May 24, 2011 at 6:27 PM, Chris Ball <cjb@laptop.org> wrote:
> Hi Andrei,
>
> On Mon, May 23 2011, Andrei Warkentin wrote:
>> Enables Auto-CMD23 support where available (SDHCI 3.0 controllers)
>>
>> Cc: arindam.nath@amd.com
>> Cc: cjb@laptop.org
>> Cc: arnd@arndb.de
>> Cc: subhashj@codeaurora.org
>> Cc: malchev@google.com
>> Tested-by: Arindam Nath <arindam.nath@amd.com>
>> Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
>
> Looks like this patch is breaking boot on my XO-1.75/Marvell MMP2 A2
> with SD 3.0 controller.  :/
>
> [    1.217156] sdhci: Secure Digital Host Controller Interface driver
> [    1.223361] sdhci: Copyright(c) Pierre Ossman
> [    1.227793] mmc0: Auto-CMD23 available
> [    1.227793] mmc0: no vmmc regulator found
> [    1.242557] mmc0: SDHCI controller on MMC [sdhci-pxa.0] using DMA
> [    1.248748] mmc1: Auto-CMD23 available
> [    1.252479] mmc1: no vmmc regulator found
> [    1.256599] mmc1: SDHCI controller on MMC [sdhci-pxa.1] using DMA
> [    1.262694] mmc2: Auto-CMD23 available
> [    1.266571] mmc2: no vmmc regulator found
> [    1.270655] mmc2: SDHCI controller on MMC [sdhci-pxa.2] using DMA
> [    1.656280] mmc1: new SDIO card at address 0001
> [    2.423849] libertas: 20:7c:8f:4a:79:c5, fw 9.70.7p0, cap 0x000003a3
> [    2.432951] libertas: wlan0: Marvell WLAN 802.11 adapter
> [    2.438262] Waiting for root device /dev/mmcblk0p2...
> [    2.528607] mmc2: new high speed DDR MMC card at address 0001
> [    2.534597] mmcblk0: mmc2:0001 SEM04G 3.68 GiB
> [    2.539198] mmcblk0boot0: mmc2:0001 SEM04G partition 1 1.00 MiB
> [    2.545166] mmcblk0boot1: mmc2:0001 SEM04G partition 2 1.00 MiB
> [    2.555985]  mmcblk0: unknown partition table
> ^^^^^
> [    2.562610]  mmcblk0boot1: unknown partition table
> [    2.569715]  mmcblk0boot0: unknown partition table
> [    2.663264] VFS: Cannot open root device "mmcblk0p2" or unknown-block(179,2)
>
> With the patch reverted, mmcblk0 comes up with partitions 1 and 2 and
> mmcblk0p2 is mounted.  In this log, we fail to find a partition table
> and panic.
>
> Any ideas/suggestions?  I'm surprised that we enabled Auto-CMD23 on this
> hardware; it is a v3 controller, but it has SDHCI_QUIRK_BROKEN_ADMA set..
>

Looks like I am not honoring the broken ADMA flag :(. I was under the
impression that it would have resulted in clearing the ADMA flag
earlier, but looks like I was wrong.

Will resend tomorrow.

A

Re: CMD23 plumbing patchset.

[Chris Ball]

Hi Andrei,

On Mon, May 23 2011, Andrei Warkentin wrote:
> This is the 6th version of the patchset. Made sure that Auto-CMD(12|23)
> is called *_AUTO_CMD* instead of *_ACMD*.
>
> Also removed an unnecessary check for v3 spec, since one is already in place
> after Arindam's patches.
>
> ToC:
> [v6 1/5] MMC: Add/remove quirks conditional support.
> [v6 2/5] MMC: Use CMD23 for multiblock transfers when we can.
> [v6 3/5] MMC: Implement MMC_CAP_CMD23 for SDHCI.
> [v6 4/5] MMC: Block CMD23 support for UHS104/SDXC cards.
> [v6 5/5] MMC: SDHCI AutoCMD23 support.

Thanks, I'll merge this for .40 with the following printk level change --
I don't think we should bother KERN_INFO with CMD23 availability:

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c29e067..62d1bba 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2500,9 +2500,9 @@ int sdhci_add_host(struct sdhci_host *host)
            ((host->flags & SDHCI_USE_ADMA) ||
             !(host->flags & SDHCI_REQ_USE_DMA))) {
                host->flags |= SDHCI_AUTO_CMD23;
-               printk(KERN_INFO "%s: Auto-CMD23 available\n", mmc_hostname(mmc));
+               DBG("%s: Auto-CMD23 available\n", mmc_hostname(mmc));
        } else
-               printk(KERN_INFO "%s: Auto-CMD23 unavailable\n", mmc_hostname(mmc);
+               DBG("%s: Auto-CMD23 unavailable\n", mmc_hostname(mmc));
 
        /*
         * A controller may support 8-bit width, but the board itself

-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child