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