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@xxxxxxxxxxxxxx> > --- > > 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
