[PATCH 1/3] mmc: core: activate pre-erased multiple write support for sd card

[PATCH 1/3] mmc: core: activate pre-erased multiple write support for sd card

[Shawn Lin]

Per SD specification version 4.10, section 4.3.4, it says "pre-erased
(ACMD23) will make a following Multiple Block Write operation faster
compared to the same operation without preceding ACMD23". ACMD23 is
mandatory for all sd cards and the spec also says "Command STOP_TRAN
(CMD12) shall be used to stop the transmission in Write Multiple Block
whether or not the preerase (ACMD23) feature is used."

However current code only check to see if CMD23 is available. If not, it
would fallback to open-ending mode. So this patch is trying to activate
ACMD23 support for SD cards when doing multiple write.

The policy is:
<1> If the sd card claims to support CMD23, the behaviour isn't
changed. So we still use CMD23 for both of multiple block read and
write as before.
<2> If the sd card *doesn't* claims to support CMD23, we force all
the multiple block write to use preceding ACMD23. The multiple read
behaviour is changed since ACMD23 is for write only, per spec.
<3> If the sd card *falsely* claims to support CMD23, and we add the
quirk, MMC_QUIRK_BLK_NO_CMD23. The behaviour is the same as <2>. And
one of the reports and test could be found at the bottom link.

Tested with the following cards:
Apacer High Speed SDHC 8GB
ADATA High Speed SDHC 4GB
ADATA Premier UHS-I SDHC 16GB
ez Share wifi-combo-SDHC 32GB
Kingston UHS-I micro SDHC 32GB
Toshiba High Speed micro SDHC 4GB
Toshiba High Speed micro SDHC 8GB
Toshiba Exceria UHS-I micro SDHC 32GB
PNY High Speed micro SDHC 32GB
Sandisk Ultra UHS-I micro SDHC 16GB
Sandisk Extreme Pro UHS-I micro SDXC 512GB
Samsung UHS-I micro SDHC 32GB
Sony SFUX UHS-I SDHC 32GB
Silicon Power High Speed SDHC 16GB
Transcend UHS-I SDXC 64GB
Topmore High Speed SDHC 8GB
Longsys UHS-I micro SDHC 32GB
Lexar Professional UHS-I micro SDHC 32GB
.....

More cards have been tested but not need to list here. Some of them
support CMD23 but some don't. All the test shows this patch work fine.

Link: https://patchwork.kernel.org/patch/9946125/
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/core/block.c | 50 ++++++++++++++++++++++++++++++++++++++++++------
 drivers/mmc/core/core.c  |  6 ++++++
 include/linux/mmc/core.h |  2 ++
 3 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index ab9c780..b253d6a 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -101,8 +101,9 @@ struct mmc_blk_data {
 	struct list_head rpmbs;
 
 	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 */
+#define MMC_BLK_CMD23	BIT(0)	/* Can do SET_BLOCK_COUNT for multiblock */
+#define MMC_BLK_REL_WR	BIT(1)	/* MMC Reliable write support */
+#define MMC_BLK_ACMD23	BIT(2)	/* Can do pre-erased before multiblock write */
 
 	unsigned int	usage;
 	unsigned int	read_only;
@@ -905,7 +906,8 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
 }
 
 static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
-		bool hw_busy_detect, struct request *req, bool *gen_err)
+		bool hw_busy_detect, struct request *req, bool *gen_err,
+		u32 *card_status)
 {
 	unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
 	int err = 0;
@@ -949,6 +951,9 @@ static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
 	} while (!(status & R1_READY_FOR_DATA) ||
 		 (R1_CURRENT_STATE(status) == R1_STATE_PRG));
 
+	if (card_status)
+		*card_status = status;
+
 	return err;
 }
 
@@ -994,7 +999,8 @@ static int send_stop(struct mmc_card *card, unsigned int timeout_ms,
 		*gen_err = true;
 	}
 
-	return card_busy_detect(card, timeout_ms, use_r1b_resp, req, gen_err);
+	return card_busy_detect(card, timeout_ms, use_r1b_resp, req,
+				gen_err, NULL);
 }
 
 #define ERR_NOMEDIUM	3
@@ -1520,6 +1526,7 @@ static enum mmc_blk_status mmc_blk_err_check(struct mmc_card *card,
 	 */
 	if (!mmc_host_is_spi(card->host) && rq_data_dir(req) != READ) {
 		int err;
+		u32 stop_status;
 
 		/* Check stop command response */
 		if (brq->stop.resp[0] & R1_ERROR) {
@@ -1530,9 +1537,23 @@ static enum mmc_blk_status mmc_blk_err_check(struct mmc_card *card,
 		}
 
 		err = card_busy_detect(card, MMC_BLK_TIMEOUT_MS, false, req,
-					&gen_err);
+					&gen_err, &stop_status);
 		if (err)
 			return MMC_BLK_CMD_ERR;
+
+		/*
+		 * Some sd cards still leave in recv state after sending CMD12
+		 * when preceding ACMD23 is used. But try again will fix that.
+		 */
+		if (brq->sbc.flags & MMC_CMD_SD_APP &&
+		    R1_CURRENT_STATE(stop_status) == R1_STATE_RCV) {
+			err = send_stop(card,
+					DIV_ROUND_UP(brq->data.timeout_ns,
+						     1000000),
+					req, &gen_err, &stop_status);
+			if (err)
+				return MMC_BLK_CMD_ERR;
+		}
 	}
 
 	/* if general error occurs, retry the write operation. */
@@ -1687,6 +1708,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
 	struct request *req = mmc_queue_req_to_req(mqrq);
 	struct mmc_blk_data *md = mq->blkdata;
 	bool do_rel_wr, do_data_tag;
+	bool need_acmd23;
 
 	mmc_blk_data_prep(mq, mqrq, disable_multi, &do_rel_wr, &do_data_tag);
 
@@ -1727,11 +1749,22 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
 	 * that for hosts that don't expose MMC_CAP_CMD23, no
 	 * change of behavior will be observed.
 	 *
+	 * Per sd spec, section 4.3.4, it suggests to use ACMD23
+	 * do achieve better write performance for those who can't
+	 * support CMD23. But also note that ACMD23 is for write only,
+	 * so we still need use CMD23 to do multiple block data transfer
+	 * if the card claims to support CMD23.
+	 *
 	 * 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) &&
+	need_acmd23 = !(md->flags & MMC_BLK_CMD23) &&
+		      brq->cmd.opcode == MMC_WRITE_MULTIPLE_BLOCK &&
+		      md->flags & MMC_BLK_ACMD23;
+
+	if (((md->flags & MMC_BLK_CMD23) || need_acmd23) &&
+	    mmc_op_multi(brq->cmd.opcode) &&
 	    (do_rel_wr || !(card->quirks & MMC_QUIRK_BLK_NO_CMD23) ||
 	     do_data_tag)) {
 		brq->sbc.opcode = MMC_SET_BLOCK_COUNT;
@@ -1739,6 +1772,8 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
 			(do_rel_wr ? (1 << 31) : 0) |
 			(do_data_tag ? (1 << 29) : 0);
 		brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
+		if (need_acmd23)
+			brq->sbc.flags |= MMC_CMD_SD_APP;
 		brq->mrq.sbc = &brq->sbc;
 	}
 
@@ -2158,6 +2193,9 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 		    (mmc_card_sd(card) &&
 		     card->scr.cmds & SD_SCR_CMD23_SUPPORT))
 			md->flags |= MMC_BLK_CMD23;
+
+		if (mmc_card_sd(card))
+			md->flags |= MMC_BLK_ACMD23;
 	}
 
 	if (mmc_card_mmc(card) &&
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 66c9cf4..bc96e6d 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -403,6 +403,12 @@ static int __mmc_start_data_req(struct mmc_host *host, struct mmc_request *mrq)
 
 	mmc_wait_ongoing_tfr_cmd(host);
 
+	if (mrq && mrq->sbc && mrq->sbc->flags & MMC_CMD_SD_APP) {
+		err = mmc_app_cmd(host, host->card);
+		if (err)
+			return err;
+	}
+
 	mrq->done = mmc_wait_data_done;
 	mrq->host = host;
 
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 9275193..dcb5783 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -46,6 +46,8 @@ struct mmc_command {
 #define MMC_CMD_BC	(2 << 5)
 #define MMC_CMD_BCR	(3 << 5)
 
+#define MMC_CMD_SD_APP	(1 << 6)		/* app cmd for SD cards */
+
 #define MMC_RSP_SPI_S1	(1 << 7)		/* one status byte */
 #define MMC_RSP_SPI_S2	(1 << 8)		/* second byte */
 #define MMC_RSP_SPI_B4	(1 << 9)		/* four data bytes */
-- 
1.9.1

[PATCH 2/3] mmc: test: prepare sbc for preceding ACMD23 multiple block write

[Shawn Lin]

Enable preceding ACMD23 for testing multiple block write for the
sd cards which don't claim to support CMD23.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/core/mmc_test.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mmc/core/mmc_test.c b/drivers/mmc/core/mmc_test.c
index 4788698..c941ba9 100644
--- a/drivers/mmc/core/mmc_test.c
+++ b/drivers/mmc/core/mmc_test.c
@@ -198,17 +198,26 @@ static void mmc_test_prepare_sbc(struct mmc_test_card *test,
 				 struct mmc_request *mrq, unsigned int blocks)
 {
 	struct mmc_card *card = test->card;
+	bool need_acmd23 = false;
 
 	if (!mrq->sbc || !mmc_host_cmd23(card->host) ||
 	    !mmc_test_card_cmd23(card) || !mmc_op_multi(mrq->cmd->opcode) ||
 	    (card->quirks & MMC_QUIRK_BLK_NO_CMD23)) {
+		need_acmd23 = mmc_card_sd(card) &&
+			      mrq->cmd->opcode == MMC_WRITE_MULTIPLE_BLOCK;
+		if (need_acmd23)
+			goto prepare_sbc;
+
 		mrq->sbc = NULL;
 		return;
 	}
 
+prepare_sbc:
 	mrq->sbc->opcode = MMC_SET_BLOCK_COUNT;
 	mrq->sbc->arg = blocks;
 	mrq->sbc->flags = MMC_RSP_R1 | MMC_CMD_AC;
+	if (need_acmd23)
+		mrq->sbc->flags |= MMC_CMD_SD_APP;
 }
 
 /*
-- 
1.9.1

[PATCH 3/3] mmc: cavium: Add ACMD23 support for dma's multiple mode

[Shawn Lin]

Still need prepare for dma's multiple mode if it's a preceding
ACMD23 since it indicates a multiple block write is coming.

Cc: "Steven J. Hill" <Steven.Hill@cavium.com>
Cc: Jan Glauber <jglauber@cavium.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
Just compile only, so need to do a solid test for sure
the ACMD23 support won't break cavium host. I'm also
suprised to see cavium relies on SD_SCR_CMD23_SUPPORT to
do some preparation for dma, but personly I think it's
wrong and should use a more legit way for that.

 drivers/mmc/host/cavium.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/cavium.c b/drivers/mmc/host/cavium.c
index fbd29f0..56bf0e0 100644
--- a/drivers/mmc/host/cavium.c
+++ b/drivers/mmc/host/cavium.c
@@ -637,7 +637,8 @@ static u64 prepare_ext_dma(struct mmc_host *mmc, struct mmc_request *mrq)
 	set_bus_id(&emm_dma, slot->bus_id);
 
 	if (mmc_card_mmc(mmc->card) || (mmc_card_sd(mmc->card) &&
-	    (mmc->card->scr.cmds & SD_SCR_CMD23_SUPPORT)))
+	    ((mmc->card->scr.cmds & SD_SCR_CMD23_SUPPORT) ||
+	     mrq->sbc->flags & MMC_CMD_SD_APP)))
 		emm_dma |= FIELD_PREP(MIO_EMM_DMA_MULTI, 1);
 
 	pr_debug("[%s] blocks: %u  multi: %d\n",
-- 
1.9.1

Re: [PATCH 1/3] mmc: core: activate pre-erased multiple write support for sd card

[Jan Glauber]

Hi Shawn,

I'm trying your patches on Cavium's ARM SOC. How do I get debug
messages from the MMC layer nowadays? Enabling CONFIG_MMC_DEBUG
does not show anything (4.14-rc2) but worked in the past.

thanks,
Jan

On Wed, Sep 27, 2017 at 11:40:36AM +0800, Shawn Lin wrote:
> Per SD specification version 4.10, section 4.3.4, it says "pre-erased
> (ACMD23) will make a following Multiple Block Write operation faster
> compared to the same operation without preceding ACMD23". ACMD23 is
> mandatory for all sd cards and the spec also says "Command STOP_TRAN
> (CMD12) shall be used to stop the transmission in Write Multiple Block
> whether or not the preerase (ACMD23) feature is used."
> 
> However current code only check to see if CMD23 is available. If not, it
> would fallback to open-ending mode. So this patch is trying to activate
> ACMD23 support for SD cards when doing multiple write.
> 
> The policy is:
> <1> If the sd card claims to support CMD23, the behaviour isn't
> changed. So we still use CMD23 for both of multiple block read and
> write as before.
> <2> If the sd card *doesn't* claims to support CMD23, we force all
> the multiple block write to use preceding ACMD23. The multiple read
> behaviour is changed since ACMD23 is for write only, per spec.
> <3> If the sd card *falsely* claims to support CMD23, and we add the
> quirk, MMC_QUIRK_BLK_NO_CMD23. The behaviour is the same as <2>. And
> one of the reports and test could be found at the bottom link.
> 
> Tested with the following cards:
> Apacer High Speed SDHC 8GB
> ADATA High Speed SDHC 4GB
> ADATA Premier UHS-I SDHC 16GB
> ez Share wifi-combo-SDHC 32GB
> Kingston UHS-I micro SDHC 32GB
> Toshiba High Speed micro SDHC 4GB
> Toshiba High Speed micro SDHC 8GB
> Toshiba Exceria UHS-I micro SDHC 32GB
> PNY High Speed micro SDHC 32GB
> Sandisk Ultra UHS-I micro SDHC 16GB
> Sandisk Extreme Pro UHS-I micro SDXC 512GB
> Samsung UHS-I micro SDHC 32GB
> Sony SFUX UHS-I SDHC 32GB
> Silicon Power High Speed SDHC 16GB
> Transcend UHS-I SDXC 64GB
> Topmore High Speed SDHC 8GB
> Longsys UHS-I micro SDHC 32GB
> Lexar Professional UHS-I micro SDHC 32GB
> .....
> 
> More cards have been tested but not need to list here. Some of them
> support CMD23 but some don't. All the test shows this patch work fine.
> 
> Link: https://patchwork.kernel.org/patch/9946125/
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> 
>  drivers/mmc/core/block.c | 50 ++++++++++++++++++++++++++++++++++++++++++------
>  drivers/mmc/core/core.c  |  6 ++++++
>  include/linux/mmc/core.h |  2 ++
>  3 files changed, 52 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index ab9c780..b253d6a 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -101,8 +101,9 @@ struct mmc_blk_data {
>  	struct list_head rpmbs;
>  
>  	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 */
> +#define MMC_BLK_CMD23	BIT(0)	/* Can do SET_BLOCK_COUNT for multiblock */
> +#define MMC_BLK_REL_WR	BIT(1)	/* MMC Reliable write support */
> +#define MMC_BLK_ACMD23	BIT(2)	/* Can do pre-erased before multiblock write */
>  
>  	unsigned int	usage;
>  	unsigned int	read_only;
> @@ -905,7 +906,8 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
>  }
>  
>  static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
> -		bool hw_busy_detect, struct request *req, bool *gen_err)
> +		bool hw_busy_detect, struct request *req, bool *gen_err,
> +		u32 *card_status)
>  {
>  	unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
>  	int err = 0;
> @@ -949,6 +951,9 @@ static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
>  	} while (!(status & R1_READY_FOR_DATA) ||
>  		 (R1_CURRENT_STATE(status) == R1_STATE_PRG));
>  
> +	if (card_status)
> +		*card_status = status;
> +
>  	return err;
>  }
>  
> @@ -994,7 +999,8 @@ static int send_stop(struct mmc_card *card, unsigned int timeout_ms,
>  		*gen_err = true;
>  	}
>  
> -	return card_busy_detect(card, timeout_ms, use_r1b_resp, req, gen_err);
> +	return card_busy_detect(card, timeout_ms, use_r1b_resp, req,
> +				gen_err, NULL);
>  }
>  
>  #define ERR_NOMEDIUM	3
> @@ -1520,6 +1526,7 @@ static enum mmc_blk_status mmc_blk_err_check(struct mmc_card *card,
>  	 */
>  	if (!mmc_host_is_spi(card->host) && rq_data_dir(req) != READ) {
>  		int err;
> +		u32 stop_status;
>  
>  		/* Check stop command response */
>  		if (brq->stop.resp[0] & R1_ERROR) {
> @@ -1530,9 +1537,23 @@ static enum mmc_blk_status mmc_blk_err_check(struct mmc_card *card,
>  		}
>  
>  		err = card_busy_detect(card, MMC_BLK_TIMEOUT_MS, false, req,
> -					&gen_err);
> +					&gen_err, &stop_status);
>  		if (err)
>  			return MMC_BLK_CMD_ERR;
> +
> +		/*
> +		 * Some sd cards still leave in recv state after sending CMD12
> +		 * when preceding ACMD23 is used. But try again will fix that.
> +		 */
> +		if (brq->sbc.flags & MMC_CMD_SD_APP &&
> +		    R1_CURRENT_STATE(stop_status) == R1_STATE_RCV) {
> +			err = send_stop(card,
> +					DIV_ROUND_UP(brq->data.timeout_ns,
> +						     1000000),
> +					req, &gen_err, &stop_status);
> +			if (err)
> +				return MMC_BLK_CMD_ERR;
> +		}
>  	}
>  
>  	/* if general error occurs, retry the write operation. */
> @@ -1687,6 +1708,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>  	struct request *req = mmc_queue_req_to_req(mqrq);
>  	struct mmc_blk_data *md = mq->blkdata;
>  	bool do_rel_wr, do_data_tag;
> +	bool need_acmd23;
>  
>  	mmc_blk_data_prep(mq, mqrq, disable_multi, &do_rel_wr, &do_data_tag);
>  
> @@ -1727,11 +1749,22 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>  	 * that for hosts that don't expose MMC_CAP_CMD23, no
>  	 * change of behavior will be observed.
>  	 *
> +	 * Per sd spec, section 4.3.4, it suggests to use ACMD23
> +	 * do achieve better write performance for those who can't
> +	 * support CMD23. But also note that ACMD23 is for write only,
> +	 * so we still need use CMD23 to do multiple block data transfer
> +	 * if the card claims to support CMD23.
> +	 *
>  	 * 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) &&
> +	need_acmd23 = !(md->flags & MMC_BLK_CMD23) &&
> +		      brq->cmd.opcode == MMC_WRITE_MULTIPLE_BLOCK &&
> +		      md->flags & MMC_BLK_ACMD23;
> +
> +	if (((md->flags & MMC_BLK_CMD23) || need_acmd23) &&
> +	    mmc_op_multi(brq->cmd.opcode) &&
>  	    (do_rel_wr || !(card->quirks & MMC_QUIRK_BLK_NO_CMD23) ||
>  	     do_data_tag)) {
>  		brq->sbc.opcode = MMC_SET_BLOCK_COUNT;
> @@ -1739,6 +1772,8 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>  			(do_rel_wr ? (1 << 31) : 0) |
>  			(do_data_tag ? (1 << 29) : 0);
>  		brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
> +		if (need_acmd23)
> +			brq->sbc.flags |= MMC_CMD_SD_APP;
>  		brq->mrq.sbc = &brq->sbc;
>  	}
>  
> @@ -2158,6 +2193,9 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>  		    (mmc_card_sd(card) &&
>  		     card->scr.cmds & SD_SCR_CMD23_SUPPORT))
>  			md->flags |= MMC_BLK_CMD23;
> +
> +		if (mmc_card_sd(card))
> +			md->flags |= MMC_BLK_ACMD23;
>  	}
>  
>  	if (mmc_card_mmc(card) &&
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 66c9cf4..bc96e6d 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -403,6 +403,12 @@ static int __mmc_start_data_req(struct mmc_host *host, struct mmc_request *mrq)
>  
>  	mmc_wait_ongoing_tfr_cmd(host);
>  
> +	if (mrq && mrq->sbc && mrq->sbc->flags & MMC_CMD_SD_APP) {
> +		err = mmc_app_cmd(host, host->card);
> +		if (err)
> +			return err;
> +	}
> +
>  	mrq->done = mmc_wait_data_done;
>  	mrq->host = host;
>  
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> index 9275193..dcb5783 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -46,6 +46,8 @@ struct mmc_command {
>  #define MMC_CMD_BC	(2 << 5)
>  #define MMC_CMD_BCR	(3 << 5)
>  
> +#define MMC_CMD_SD_APP	(1 << 6)		/* app cmd for SD cards */
> +
>  #define MMC_RSP_SPI_S1	(1 << 7)		/* one status byte */
>  #define MMC_RSP_SPI_S2	(1 << 8)		/* second byte */
>  #define MMC_RSP_SPI_B4	(1 << 9)		/* four data bytes */
> -- 
> 1.9.1
> 

Re: [PATCH 1/3] mmc: core: activate pre-erased multiple write support for sd card

[Shawn Lin]

Hi Jan,

On 2017/9/27 21:47, Jan Glauber wrote:
> Hi Shawn,
> 
> I'm trying your patches on Cavium's ARM SOC. How do I get debug
> messages from the MMC layer nowadays? Enabling CONFIG_MMC_DEBUG
> does not show anything (4.14-rc2) but worked in the past.
> 

Thanks for testing!

You could select CONFIG_DYNAMIC_DEBUG and enable/disable the log at any
time by manipulating <debugfs>/dynamic_debug/control

See: Documentation/dynamic-debug-howto.txt

> thanks,
> Jan
> 
> On Wed, Sep 27, 2017 at 11:40:36AM +0800, Shawn Lin wrote:
>> Per SD specification version 4.10, section 4.3.4, it says "pre-erased
>> (ACMD23) will make a following Multiple Block Write operation faster
>> compared to the same operation without preceding ACMD23". ACMD23 is
>> mandatory for all sd cards and the spec also says "Command STOP_TRAN
>> (CMD12) shall be used to stop the transmission in Write Multiple Block
>> whether or not the preerase (ACMD23) feature is used."
>>
>> However current code only check to see if CMD23 is available. If not, it
>> would fallback to open-ending mode. So this patch is trying to activate
>> ACMD23 support for SD cards when doing multiple write.
>>
>> The policy is:
>> <1> If the sd card claims to support CMD23, the behaviour isn't
>> changed. So we still use CMD23 for both of multiple block read and
>> write as before.
>> <2> If the sd card *doesn't* claims to support CMD23, we force all
>> the multiple block write to use preceding ACMD23. The multiple read
>> behaviour is changed since ACMD23 is for write only, per spec.
>> <3> If the sd card *falsely* claims to support CMD23, and we add the
>> quirk, MMC_QUIRK_BLK_NO_CMD23. The behaviour is the same as <2>. And
>> one of the reports and test could be found at the bottom link.
>>
>> Tested with the following cards:
>> Apacer High Speed SDHC 8GB
>> ADATA High Speed SDHC 4GB
>> ADATA Premier UHS-I SDHC 16GB
>> ez Share wifi-combo-SDHC 32GB
>> Kingston UHS-I micro SDHC 32GB
>> Toshiba High Speed micro SDHC 4GB
>> Toshiba High Speed micro SDHC 8GB
>> Toshiba Exceria UHS-I micro SDHC 32GB
>> PNY High Speed micro SDHC 32GB
>> Sandisk Ultra UHS-I micro SDHC 16GB
>> Sandisk Extreme Pro UHS-I micro SDXC 512GB
>> Samsung UHS-I micro SDHC 32GB
>> Sony SFUX UHS-I SDHC 32GB
>> Silicon Power High Speed SDHC 16GB
>> Transcend UHS-I SDXC 64GB
>> Topmore High Speed SDHC 8GB
>> Longsys UHS-I micro SDHC 32GB
>> Lexar Professional UHS-I micro SDHC 32GB
>> .....
>>
>> More cards have been tested but not need to list here. Some of them
>> support CMD23 but some don't. All the test shows this patch work fine.
>>
>> Link: https://patchwork.kernel.org/patch/9946125/
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>>   drivers/mmc/core/block.c | 50 ++++++++++++++++++++++++++++++++++++++++++------
>>   drivers/mmc/core/core.c  |  6 ++++++
>>   include/linux/mmc/core.h |  2 ++
>>   3 files changed, 52 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
>> index ab9c780..b253d6a 100644
>> --- a/drivers/mmc/core/block.c
>> +++ b/drivers/mmc/core/block.c
>> @@ -101,8 +101,9 @@ struct mmc_blk_data {
>>   	struct list_head rpmbs;
>>   
>>   	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 */
>> +#define MMC_BLK_CMD23	BIT(0)	/* Can do SET_BLOCK_COUNT for multiblock */
>> +#define MMC_BLK_REL_WR	BIT(1)	/* MMC Reliable write support */
>> +#define MMC_BLK_ACMD23	BIT(2)	/* Can do pre-erased before multiblock write */
>>   
>>   	unsigned int	usage;
>>   	unsigned int	read_only;
>> @@ -905,7 +906,8 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
>>   }
>>   
>>   static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
>> -		bool hw_busy_detect, struct request *req, bool *gen_err)
>> +		bool hw_busy_detect, struct request *req, bool *gen_err,
>> +		u32 *card_status)
>>   {
>>   	unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
>>   	int err = 0;
>> @@ -949,6 +951,9 @@ static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
>>   	} while (!(status & R1_READY_FOR_DATA) ||
>>   		 (R1_CURRENT_STATE(status) == R1_STATE_PRG));
>>   
>> +	if (card_status)
>> +		*card_status = status;
>> +
>>   	return err;
>>   }
>>   
>> @@ -994,7 +999,8 @@ static int send_stop(struct mmc_card *card, unsigned int timeout_ms,
>>   		*gen_err = true;
>>   	}
>>   
>> -	return card_busy_detect(card, timeout_ms, use_r1b_resp, req, gen_err);
>> +	return card_busy_detect(card, timeout_ms, use_r1b_resp, req,
>> +				gen_err, NULL);
>>   }
>>   
>>   #define ERR_NOMEDIUM	3
>> @@ -1520,6 +1526,7 @@ static enum mmc_blk_status mmc_blk_err_check(struct mmc_card *card,
>>   	 */
>>   	if (!mmc_host_is_spi(card->host) && rq_data_dir(req) != READ) {
>>   		int err;
>> +		u32 stop_status;
>>   
>>   		/* Check stop command response */
>>   		if (brq->stop.resp[0] & R1_ERROR) {
>> @@ -1530,9 +1537,23 @@ static enum mmc_blk_status mmc_blk_err_check(struct mmc_card *card,
>>   		}
>>   
>>   		err = card_busy_detect(card, MMC_BLK_TIMEOUT_MS, false, req,
>> -					&gen_err);
>> +					&gen_err, &stop_status);
>>   		if (err)
>>   			return MMC_BLK_CMD_ERR;
>> +
>> +		/*
>> +		 * Some sd cards still leave in recv state after sending CMD12
>> +		 * when preceding ACMD23 is used. But try again will fix that.
>> +		 */
>> +		if (brq->sbc.flags & MMC_CMD_SD_APP &&
>> +		    R1_CURRENT_STATE(stop_status) == R1_STATE_RCV) {
>> +			err = send_stop(card,
>> +					DIV_ROUND_UP(brq->data.timeout_ns,
>> +						     1000000),
>> +					req, &gen_err, &stop_status);
>> +			if (err)
>> +				return MMC_BLK_CMD_ERR;
>> +		}
>>   	}
>>   
>>   	/* if general error occurs, retry the write operation. */
>> @@ -1687,6 +1708,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>>   	struct request *req = mmc_queue_req_to_req(mqrq);
>>   	struct mmc_blk_data *md = mq->blkdata;
>>   	bool do_rel_wr, do_data_tag;
>> +	bool need_acmd23;
>>   
>>   	mmc_blk_data_prep(mq, mqrq, disable_multi, &do_rel_wr, &do_data_tag);
>>   
>> @@ -1727,11 +1749,22 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>>   	 * that for hosts that don't expose MMC_CAP_CMD23, no
>>   	 * change of behavior will be observed.
>>   	 *
>> +	 * Per sd spec, section 4.3.4, it suggests to use ACMD23
>> +	 * do achieve better write performance for those who can't
>> +	 * support CMD23. But also note that ACMD23 is for write only,
>> +	 * so we still need use CMD23 to do multiple block data transfer
>> +	 * if the card claims to support CMD23.
>> +	 *
>>   	 * 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) &&
>> +	need_acmd23 = !(md->flags & MMC_BLK_CMD23) &&
>> +		      brq->cmd.opcode == MMC_WRITE_MULTIPLE_BLOCK &&
>> +		      md->flags & MMC_BLK_ACMD23;
>> +
>> +	if (((md->flags & MMC_BLK_CMD23) || need_acmd23) &&
>> +	    mmc_op_multi(brq->cmd.opcode) &&
>>   	    (do_rel_wr || !(card->quirks & MMC_QUIRK_BLK_NO_CMD23) ||
>>   	     do_data_tag)) {
>>   		brq->sbc.opcode = MMC_SET_BLOCK_COUNT;
>> @@ -1739,6 +1772,8 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>>   			(do_rel_wr ? (1 << 31) : 0) |
>>   			(do_data_tag ? (1 << 29) : 0);
>>   		brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
>> +		if (need_acmd23)
>> +			brq->sbc.flags |= MMC_CMD_SD_APP;
>>   		brq->mrq.sbc = &brq->sbc;
>>   	}
>>   
>> @@ -2158,6 +2193,9 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>>   		    (mmc_card_sd(card) &&
>>   		     card->scr.cmds & SD_SCR_CMD23_SUPPORT))
>>   			md->flags |= MMC_BLK_CMD23;
>> +
>> +		if (mmc_card_sd(card))
>> +			md->flags |= MMC_BLK_ACMD23;
>>   	}
>>   
>>   	if (mmc_card_mmc(card) &&
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 66c9cf4..bc96e6d 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -403,6 +403,12 @@ static int __mmc_start_data_req(struct mmc_host *host, struct mmc_request *mrq)
>>   
>>   	mmc_wait_ongoing_tfr_cmd(host);
>>   
>> +	if (mrq && mrq->sbc && mrq->sbc->flags & MMC_CMD_SD_APP) {
>> +		err = mmc_app_cmd(host, host->card);
>> +		if (err)
>> +			return err;
>> +	}
>> +
>>   	mrq->done = mmc_wait_data_done;
>>   	mrq->host = host;
>>   
>> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
>> index 9275193..dcb5783 100644
>> --- a/include/linux/mmc/core.h
>> +++ b/include/linux/mmc/core.h
>> @@ -46,6 +46,8 @@ struct mmc_command {
>>   #define MMC_CMD_BC	(2 << 5)
>>   #define MMC_CMD_BCR	(3 << 5)
>>   
>> +#define MMC_CMD_SD_APP	(1 << 6)		/* app cmd for SD cards */
>> +
>>   #define MMC_RSP_SPI_S1	(1 << 7)		/* one status byte */
>>   #define MMC_RSP_SPI_S2	(1 << 8)		/* second byte */
>>   #define MMC_RSP_SPI_B4	(1 << 9)		/* four data bytes */
>> -- 
>> 1.9.1
>>
> 
> 
> 

Re: [PATCH 1/3] mmc: core: activate pre-erased multiple write support for sd card

[Jan Glauber]

On Thu, Sep 28, 2017 at 11:33:00AM +0800, Shawn Lin wrote:
> Hi Jan,
> 
> On 2017/9/27 21:47, Jan Glauber wrote:
> >Hi Shawn,
> >
> >I'm trying your patches on Cavium's ARM SOC. How do I get debug
> >messages from the MMC layer nowadays? Enabling CONFIG_MMC_DEBUG
> >does not show anything (4.14-rc2) but worked in the past.
> >
> 
> Thanks for testing!
> 
> You could select CONFIG_DYNAMIC_DEBUG and enable/disable the log at any
> time by manipulating <debugfs>/dynamic_debug/control

I have CONFIG_DYNAMIC_DEBUG enabled but I'm not a big fan of it as
I want it at boot time which means I need to touch the boot parameters
which is more work then switching on the config option.

Anyway, added "dyndbg=file drivers/mmc/* +p" but still nothing showed
up. Probably I need to spend more time studying the documentation...

--Jan

> See: Documentation/dynamic-debug-howto.txt
> 
> >thanks,
> >Jan
> >
> >On Wed, Sep 27, 2017 at 11:40:36AM +0800, Shawn Lin wrote:
> >>Per SD specification version 4.10, section 4.3.4, it says "pre-erased
> >>(ACMD23) will make a following Multiple Block Write operation faster
> >>compared to the same operation without preceding ACMD23". ACMD23 is
> >>mandatory for all sd cards and the spec also says "Command STOP_TRAN
> >>(CMD12) shall be used to stop the transmission in Write Multiple Block
> >>whether or not the preerase (ACMD23) feature is used."
> >>
> >>However current code only check to see if CMD23 is available. If not, it
> >>would fallback to open-ending mode. So this patch is trying to activate
> >>ACMD23 support for SD cards when doing multiple write.
> >>
> >>The policy is:
> >><1> If the sd card claims to support CMD23, the behaviour isn't
> >>changed. So we still use CMD23 for both of multiple block read and
> >>write as before.
> >><2> If the sd card *doesn't* claims to support CMD23, we force all
> >>the multiple block write to use preceding ACMD23. The multiple read
> >>behaviour is changed since ACMD23 is for write only, per spec.
> >><3> If the sd card *falsely* claims to support CMD23, and we add the
> >>quirk, MMC_QUIRK_BLK_NO_CMD23. The behaviour is the same as <2>. And
> >>one of the reports and test could be found at the bottom link.
> >>
> >>Tested with the following cards:
> >>Apacer High Speed SDHC 8GB
> >>ADATA High Speed SDHC 4GB
> >>ADATA Premier UHS-I SDHC 16GB
> >>ez Share wifi-combo-SDHC 32GB
> >>Kingston UHS-I micro SDHC 32GB
> >>Toshiba High Speed micro SDHC 4GB
> >>Toshiba High Speed micro SDHC 8GB
> >>Toshiba Exceria UHS-I micro SDHC 32GB
> >>PNY High Speed micro SDHC 32GB
> >>Sandisk Ultra UHS-I micro SDHC 16GB
> >>Sandisk Extreme Pro UHS-I micro SDXC 512GB
> >>Samsung UHS-I micro SDHC 32GB
> >>Sony SFUX UHS-I SDHC 32GB
> >>Silicon Power High Speed SDHC 16GB
> >>Transcend UHS-I SDXC 64GB
> >>Topmore High Speed SDHC 8GB
> >>Longsys UHS-I micro SDHC 32GB
> >>Lexar Professional UHS-I micro SDHC 32GB
> >>.....
> >>
> >>More cards have been tested but not need to list here. Some of them
> >>support CMD23 but some don't. All the test shows this patch work fine.
> >>
> >>Link: https://patchwork.kernel.org/patch/9946125/
> >>Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> >>---
> >>
> >>  drivers/mmc/core/block.c | 50 ++++++++++++++++++++++++++++++++++++++++++------
> >>  drivers/mmc/core/core.c  |  6 ++++++
> >>  include/linux/mmc/core.h |  2 ++
> >>  3 files changed, 52 insertions(+), 6 deletions(-)
> >>
> >>diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> >>index ab9c780..b253d6a 100644
> >>--- a/drivers/mmc/core/block.c
> >>+++ b/drivers/mmc/core/block.c
> >>@@ -101,8 +101,9 @@ struct mmc_blk_data {
> >>  	struct list_head rpmbs;
> >>  	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 */
> >>+#define MMC_BLK_CMD23	BIT(0)	/* Can do SET_BLOCK_COUNT for multiblock */
> >>+#define MMC_BLK_REL_WR	BIT(1)	/* MMC Reliable write support */
> >>+#define MMC_BLK_ACMD23	BIT(2)	/* Can do pre-erased before multiblock write */
> >>  	unsigned int	usage;
> >>  	unsigned int	read_only;
> >>@@ -905,7 +906,8 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
> >>  }
> >>  static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
> >>-		bool hw_busy_detect, struct request *req, bool *gen_err)
> >>+		bool hw_busy_detect, struct request *req, bool *gen_err,
> >>+		u32 *card_status)
> >>  {
> >>  	unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
> >>  	int err = 0;
> >>@@ -949,6 +951,9 @@ static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
> >>  	} while (!(status & R1_READY_FOR_DATA) ||
> >>  		 (R1_CURRENT_STATE(status) == R1_STATE_PRG));
> >>+	if (card_status)
> >>+		*card_status = status;
> >>+
> >>  	return err;
> >>  }
> >>@@ -994,7 +999,8 @@ static int send_stop(struct mmc_card *card, unsigned int timeout_ms,
> >>  		*gen_err = true;
> >>  	}
> >>-	return card_busy_detect(card, timeout_ms, use_r1b_resp, req, gen_err);
> >>+	return card_busy_detect(card, timeout_ms, use_r1b_resp, req,
> >>+				gen_err, NULL);
> >>  }
> >>  #define ERR_NOMEDIUM	3
> >>@@ -1520,6 +1526,7 @@ static enum mmc_blk_status mmc_blk_err_check(struct mmc_card *card,
> >>  	 */
> >>  	if (!mmc_host_is_spi(card->host) && rq_data_dir(req) != READ) {
> >>  		int err;
> >>+		u32 stop_status;
> >>  		/* Check stop command response */
> >>  		if (brq->stop.resp[0] & R1_ERROR) {
> >>@@ -1530,9 +1537,23 @@ static enum mmc_blk_status mmc_blk_err_check(struct mmc_card *card,
> >>  		}
> >>  		err = card_busy_detect(card, MMC_BLK_TIMEOUT_MS, false, req,
> >>-					&gen_err);
> >>+					&gen_err, &stop_status);
> >>  		if (err)
> >>  			return MMC_BLK_CMD_ERR;
> >>+
> >>+		/*
> >>+		 * Some sd cards still leave in recv state after sending CMD12
> >>+		 * when preceding ACMD23 is used. But try again will fix that.
> >>+		 */
> >>+		if (brq->sbc.flags & MMC_CMD_SD_APP &&
> >>+		    R1_CURRENT_STATE(stop_status) == R1_STATE_RCV) {
> >>+			err = send_stop(card,
> >>+					DIV_ROUND_UP(brq->data.timeout_ns,
> >>+						     1000000),
> >>+					req, &gen_err, &stop_status);
> >>+			if (err)
> >>+				return MMC_BLK_CMD_ERR;
> >>+		}
> >>  	}
> >>  	/* if general error occurs, retry the write operation. */
> >>@@ -1687,6 +1708,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> >>  	struct request *req = mmc_queue_req_to_req(mqrq);
> >>  	struct mmc_blk_data *md = mq->blkdata;
> >>  	bool do_rel_wr, do_data_tag;
> >>+	bool need_acmd23;
> >>  	mmc_blk_data_prep(mq, mqrq, disable_multi, &do_rel_wr, &do_data_tag);
> >>@@ -1727,11 +1749,22 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> >>  	 * that for hosts that don't expose MMC_CAP_CMD23, no
> >>  	 * change of behavior will be observed.
> >>  	 *
> >>+	 * Per sd spec, section 4.3.4, it suggests to use ACMD23
> >>+	 * do achieve better write performance for those who can't
> >>+	 * support CMD23. But also note that ACMD23 is for write only,
> >>+	 * so we still need use CMD23 to do multiple block data transfer
> >>+	 * if the card claims to support CMD23.
> >>+	 *
> >>  	 * 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) &&
> >>+	need_acmd23 = !(md->flags & MMC_BLK_CMD23) &&
> >>+		      brq->cmd.opcode == MMC_WRITE_MULTIPLE_BLOCK &&
> >>+		      md->flags & MMC_BLK_ACMD23;
> >>+
> >>+	if (((md->flags & MMC_BLK_CMD23) || need_acmd23) &&
> >>+	    mmc_op_multi(brq->cmd.opcode) &&
> >>  	    (do_rel_wr || !(card->quirks & MMC_QUIRK_BLK_NO_CMD23) ||
> >>  	     do_data_tag)) {
> >>  		brq->sbc.opcode = MMC_SET_BLOCK_COUNT;
> >>@@ -1739,6 +1772,8 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> >>  			(do_rel_wr ? (1 << 31) : 0) |
> >>  			(do_data_tag ? (1 << 29) : 0);
> >>  		brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
> >>+		if (need_acmd23)
> >>+			brq->sbc.flags |= MMC_CMD_SD_APP;
> >>  		brq->mrq.sbc = &brq->sbc;
> >>  	}
> >>@@ -2158,6 +2193,9 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
> >>  		    (mmc_card_sd(card) &&
> >>  		     card->scr.cmds & SD_SCR_CMD23_SUPPORT))
> >>  			md->flags |= MMC_BLK_CMD23;
> >>+
> >>+		if (mmc_card_sd(card))
> >>+			md->flags |= MMC_BLK_ACMD23;
> >>  	}
> >>  	if (mmc_card_mmc(card) &&
> >>diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> >>index 66c9cf4..bc96e6d 100644
> >>--- a/drivers/mmc/core/core.c
> >>+++ b/drivers/mmc/core/core.c
> >>@@ -403,6 +403,12 @@ static int __mmc_start_data_req(struct mmc_host *host, struct mmc_request *mrq)
> >>  	mmc_wait_ongoing_tfr_cmd(host);
> >>+	if (mrq && mrq->sbc && mrq->sbc->flags & MMC_CMD_SD_APP) {
> >>+		err = mmc_app_cmd(host, host->card);
> >>+		if (err)
> >>+			return err;
> >>+	}
> >>+
> >>  	mrq->done = mmc_wait_data_done;
> >>  	mrq->host = host;
> >>diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> >>index 9275193..dcb5783 100644
> >>--- a/include/linux/mmc/core.h
> >>+++ b/include/linux/mmc/core.h
> >>@@ -46,6 +46,8 @@ struct mmc_command {
> >>  #define MMC_CMD_BC	(2 << 5)
> >>  #define MMC_CMD_BCR	(3 << 5)
> >>+#define MMC_CMD_SD_APP	(1 << 6)		/* app cmd for SD cards */
> >>+
> >>  #define MMC_RSP_SPI_S1	(1 << 7)		/* one status byte */
> >>  #define MMC_RSP_SPI_S2	(1 << 8)		/* second byte */
> >>  #define MMC_RSP_SPI_B4	(1 << 9)		/* four data bytes */
> >>-- 
> >>1.9.1
> >>
> >
> >
> >

Re: [PATCH 1/3] mmc: core: activate pre-erased multiple write support for sd card

[Shawn Lin]

Hi, Ulf

On 2017/9/27 11:40, Shawn Lin wrote:
> Per SD specification version 4.10, section 4.3.4, it says "pre-erased
> (ACMD23) will make a following Multiple Block Write operation faster
> compared to the same operation without preceding ACMD23". ACMD23 is
> mandatory for all sd cards and the spec also says "Command STOP_TRAN
> (CMD12) shall be used to stop the transmission in Write Multiple Block
> whether or not the preerase (ACMD23) feature is used."

Ping... :)

Re: [PATCH 1/3] mmc: core: activate pre-erased multiple write support for sd card

[Ulf Hansson]

On 26 October 2017 at 03:19, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> Hi, Ulf
>
> On 2017/9/27 11:40, Shawn Lin wrote:
>>
>> Per SD specification version 4.10, section 4.3.4, it says "pre-erased
>> (ACMD23) will make a following Multiple Block Write operation faster
>> compared to the same operation without preceding ACMD23". ACMD23 is
>> mandatory for all sd cards and the spec also says "Command STOP_TRAN
>> (CMD12) shall be used to stop the transmission in Write Multiple Block
>> whether or not the preerase (ACMD23) feature is used."
>
>
> Ping... :)

Hi Shawn,

Actually I was waiting for confirmation from Jan about his tests.

However, overall the series looks reasonable to me.

Kind regards
Uffe

Re: [PATCH 1/3] mmc: core: activate pre-erased multiple write support for sd card

[Jan Glauber]

On Thu, Oct 26, 2017 at 12:46:02PM +0200, Ulf Hansson wrote:
> On 26 October 2017 at 03:19, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> > Hi, Ulf
> >
> > On 2017/9/27 11:40, Shawn Lin wrote:
> >>
> >> Per SD specification version 4.10, section 4.3.4, it says "pre-erased
> >> (ACMD23) will make a following Multiple Block Write operation faster
> >> compared to the same operation without preceding ACMD23". ACMD23 is
> >> mandatory for all sd cards and the spec also says "Command STOP_TRAN
> >> (CMD12) shall be used to stop the transmission in Write Multiple Block
> >> whether or not the preerase (ACMD23) feature is used."
> >
> >
> > Ping... :)
> 
> Hi Shawn,
> 
> Actually I was waiting for confirmation from Jan about his tests.

The patches worked for me but I'm not sure I've actually tested ACMD23.
Do I need to trigger ACMD23 somehow?

--Jan

> However, overall the series looks reasonable to me.
> 
> Kind regards
> Uffe

Re: [PATCH 1/3] mmc: core: activate pre-erased multiple write support for sd card

[Shawn Lin]

Hi Jan,

On 2017/10/26 19:03, Jan Glauber wrote:
> On Thu, Oct 26, 2017 at 12:46:02PM +0200, Ulf Hansson wrote:
>> On 26 October 2017 at 03:19, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>>> Hi, Ulf
>>>
>>> On 2017/9/27 11:40, Shawn Lin wrote:
>>>>
>>>> Per SD specification version 4.10, section 4.3.4, it says "pre-erased
>>>> (ACMD23) will make a following Multiple Block Write operation faster
>>>> compared to the same operation without preceding ACMD23". ACMD23 is
>>>> mandatory for all sd cards and the spec also says "Command STOP_TRAN
>>>> (CMD12) shall be used to stop the transmission in Write Multiple Block
>>>> whether or not the preerase (ACMD23) feature is used."
>>>
>>>
>>> Ping... :)
>>
>> Hi Shawn,
>>
>> Actually I was waiting for confirmation from Jan about his tests.
> 
> The patches worked for me but I'm not sure I've actually tested ACMD23.
> Do I need to trigger ACMD23 somehow?

Thanks for testing!

You need a card which doesn't support CMD23 and try to write something
to the card to see if it works well. But given that it's not
easy to identify whether a card support CMD23, I suggest to hack the
code for force all SD cards to use ACMD23.


Could you kindly try this on top of this series? And do some write test
for the SD cards.

@@ -2210,9 +2210,9 @@ static struct mmc_blk_data 
*mmc_blk_alloc_req(struct mmc_card *card,

         if (mmc_host_cmd23(card->host)) {
                 if ((mmc_card_mmc(card) &&
-                    card->csd.mmca_vsn >= CSD_SPEC_VER_3) ||
-                   (mmc_card_sd(card) &&
-                    card->scr.cmds & SD_SCR_CMD23_SUPPORT))
+                    card->csd.mmca_vsn >= CSD_SPEC_VER_3))
+                   //(mmc_card_sd(card) &&
+                    //card->scr.cmds & SD_SCR_CMD23_SUPPORT))
                         md->flags |= MMC_BLK_CMD23;


Thanks.


> 
> --Jan
> 
>> However, overall the series looks reasonable to me.
>>
>> Kind regards
>> Uffe
> 
> 
> 

Re: [PATCH 1/3] mmc: core: activate pre-erased multiple write support for sd card

[Jan Glauber]

On Fri, Oct 27, 2017 at 09:46:32AM +0800, Shawn Lin wrote:
> Could you kindly try this on top of this series? And do some write test
> for the SD cards.
> 
> @@ -2210,9 +2210,9 @@ static struct mmc_blk_data
> *mmc_blk_alloc_req(struct mmc_card *card,
> 
>         if (mmc_host_cmd23(card->host)) {
>                 if ((mmc_card_mmc(card) &&
> -                    card->csd.mmca_vsn >= CSD_SPEC_VER_3) ||
> -                   (mmc_card_sd(card) &&
> -                    card->scr.cmds & SD_SCR_CMD23_SUPPORT))
> +                    card->csd.mmca_vsn >= CSD_SPEC_VER_3))
> +                   //(mmc_card_sd(card) &&
> +                    //card->scr.cmds & SD_SCR_CMD23_SUPPORT))
>                         md->flags |= MMC_BLK_CMD23;

Did that and tested read/write to SD card, works fine on the Cavium
tn81xx arm64 board.

Feel free to add:
Tested-by: Jan Glauber <jglauber@cavium.com>

--Jan

Re: [PATCH 1/3] mmc: core: activate pre-erased multiple write support for sd card

[Adrian Hunter]

On 26/10/17 13:46, Ulf Hansson wrote:
> On 26 October 2017 at 03:19, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>> Hi, Ulf
>>
>> On 2017/9/27 11:40, Shawn Lin wrote:
>>>
>>> Per SD specification version 4.10, section 4.3.4, it says "pre-erased
>>> (ACMD23) will make a following Multiple Block Write operation faster
>>> compared to the same operation without preceding ACMD23". ACMD23 is
>>> mandatory for all sd cards and the spec also says "Command STOP_TRAN
>>> (CMD12) shall be used to stop the transmission in Write Multiple Block
>>> whether or not the preerase (ACMD23) feature is used."
>>
>>
>> Ping... :)
> 
> Hi Shawn,
> 
> Actually I was waiting for confirmation from Jan about his tests.
> 
> However, overall the series looks reasonable to me.

Obviously the blk-mq patches do not have support for this.  Please apply the
blk-mq patches first.

What were the performance numbers?

Re: [PATCH 1/3] mmc: core: activate pre-erased multiple write support for sd card

[Shawn Lin]

Hi Adrian,

On 2017/10/26 19:32, Adrian Hunter wrote:
> On 26/10/17 13:46, Ulf Hansson wrote:
>> On 26 October 2017 at 03:19, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>>> Hi, Ulf
>>>
>>> On 2017/9/27 11:40, Shawn Lin wrote:
>>>>
>>>> Per SD specification version 4.10, section 4.3.4, it says "pre-erased
>>>> (ACMD23) will make a following Multiple Block Write operation faster
>>>> compared to the same operation without preceding ACMD23". ACMD23 is
>>>> mandatory for all sd cards and the spec also says "Command STOP_TRAN
>>>> (CMD12) shall be used to stop the transmission in Write Multiple Block
>>>> whether or not the preerase (ACMD23) feature is used."
>>>
>>>
>>> Ping... :)
>>
>> Hi Shawn,
>>
>> Actually I was waiting for confirmation from Jan about his tests.
>>
>> However, overall the series looks reasonable to me.
> 
> Obviously the blk-mq patches do not have support for this.  Please apply the
> blk-mq patches first.

Ok, I will rebase this on the top of your V12 of cqe+mq patchset.

> 
> What were the performance numbers?

I did have some test result about performance, but actually there were
not any gain or regression from the result, so I didn't provide the 
numbers as it doesn't seem to improve te performance but just change
the behaviour for how the sd cards do multiple write w/o CMD23
support.


> --
> 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
> 
> 
>