[PATCH v3 0/6] mmc: introduce multi-block read gap tuning

[PATCH v3 0/6] mmc: introduce multi-block read gap tuning

[Benoît Monin]

This patchset implements the multi-block read gap tuning for the SDHCI
cadence driver.

The first two patches introduce helpers to check for CMD23 support by
MMC card: mmc_card_can_cmd23 for support proper and mmc_card_blk_no_cmd23
for the NO_CMD23 quirk.

The next two patches use the new helpers in mmc/core/mmc_test.c and
mmc/core/block.c.

The next patch add mmc_read_tuning to read blocks from MMC as part of
the tuning. This function does not return the data read to the caller,
only the status of the read operation. It also takes an optional card
argument, not used when called from execute_tuning() but present in
execute_hs400_tuning() and prepare_sd_hs_tuning().

Finally the last patch uses mmc_read_tuning to implement the multi-block
read gap tuning in the cadence host driver, by doing a multi-block read
and increasing the gap delay until the read succeeds.

v2 -> v3:
Move the changes related to CMD23 support by MMC card to separate patches
at the beginning of the series.
Change the mmc read function to be less appealing to reuse/abuse
outside of tuning context.

Link to v2:
https://lore.kernel.org/linux-mmc/cover.1751898225.git.benoit.monin@bootlin.com/

v1 -> v2:
Split the code between the core and the driver by adding a generic 
function to read blocks from the MMC.

Link to v1:
https://lore.kernel.org/linux-mmc/2a43386ffef4012530ca2421ad81ad21c36c8a25.1750943549.git.benoit.monin@bootlin.com/

Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
Benoît Monin (6):
      mmc: core: add mmc_card_can_cmd23
      mmc: card: add mmc_card_blk_no_cmd23
      mmc: mmc_test: use mmc_card cmd23 helpers
      mmc: block: use mmc_card cmd23 helpers
      mmc: core: add mmc_read_tuning
      mmc: sdhci-cadence: implement multi-block read gap tuning

 drivers/mmc/core/block.c         | 12 ++----
 drivers/mmc/core/card.h          |  9 ++++-
 drivers/mmc/core/core.c          |  9 +++++
 drivers/mmc/core/core.h          |  1 +
 drivers/mmc/core/mmc_ops.c       | 79 ++++++++++++++++++++++++++++++++++++++++
 drivers/mmc/core/mmc_test.c      | 10 +----
 drivers/mmc/host/sdhci-cadence.c | 69 ++++++++++++++++++++++++++++++++++-
 include/linux/mmc/host.h         |  2 +
 8 files changed, 171 insertions(+), 20 deletions(-)
---
base-commit: 4ad9e44c76b301e786eb4cdab890eac8c7eebd42
change-id: 20250716-mobileye-emmc-for-upstream-4-40ef552c3b59

Best regards,
-- 
Benoît Monin <benoit.monin@bootlin.com>

[PATCH v3 1/6] mmc: core: add mmc_card_can_cmd23

[Benoît Monin]

Add a dedicated helper to check for CMD23 support for MMC card, similar
to mmc_host_can_cmd23 for the host, as it is easy to get the check
wrong.

Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
 drivers/mmc/core/core.c | 9 +++++++++
 drivers/mmc/core/core.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 874c6fe92855e3535ec0ebc0254146f4b96bccc3..88fd231fee1d150b22baebddb7ad47472c24fb32 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1875,6 +1875,15 @@ bool mmc_card_can_secure_erase_trim(struct mmc_card *card)
 }
 EXPORT_SYMBOL(mmc_card_can_secure_erase_trim);
 
+bool mmc_card_can_cmd23(struct mmc_card *card)
+{
+	return ((mmc_card_mmc(card) &&
+		 card->csd.mmca_vsn >= CSD_SPEC_VER_3) ||
+		(mmc_card_sd(card) && !mmc_card_ult_capacity(card) &&
+		 card->scr.cmds & SD_SCR_CMD23_SUPPORT));
+}
+EXPORT_SYMBOL(mmc_card_can_cmd23);
+
 int mmc_erase_group_aligned(struct mmc_card *card, sector_t from,
 			    unsigned int nr)
 {
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 622085cd766f91334d5b9362cd7b96245af8884d..73f5d3d8c77d5da53795fe09beb384f134d6a886 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -123,6 +123,7 @@ bool mmc_card_can_trim(struct mmc_card *card);
 bool mmc_card_can_discard(struct mmc_card *card);
 bool mmc_card_can_sanitize(struct mmc_card *card);
 bool mmc_card_can_secure_erase_trim(struct mmc_card *card);
+bool mmc_card_can_cmd23(struct mmc_card *card);
 int mmc_erase_group_aligned(struct mmc_card *card, sector_t from, unsigned int nr);
 unsigned int mmc_calc_max_discard(struct mmc_card *card);
 

-- 
2.50.1

[PATCH v3 2/6] mmc: card: add mmc_card_blk_no_cmd23

[Benoît Monin]

Add a helper to check for the missing CMD23 quirk, similar to other
quirk helpers. Also reorder the helpers to match the order of the quirk
bits defined in include/linux/mmc/card.h.

Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
 drivers/mmc/core/card.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
index 9cbdd240c3a7d42ecbb45cbc52c5562e4664ee35..1200951bab08c2e18ce96becf56693e696b9f30b 100644
--- a/drivers/mmc/core/card.h
+++ b/drivers/mmc/core/card.h
@@ -245,14 +245,19 @@ static inline int mmc_blksz_for_byte_mode(const struct mmc_card *c)
 	return c->quirks & MMC_QUIRK_BLKSZ_FOR_BYTE_MODE;
 }
 
+static inline int mmc_card_nonstd_func_interface(const struct mmc_card *c)
+{
+	return c->quirks & MMC_QUIRK_NONSTD_FUNC_IF;
+}
+
 static inline int mmc_card_disable_cd(const struct mmc_card *c)
 {
 	return c->quirks & MMC_QUIRK_DISABLE_CD;
 }
 
-static inline int mmc_card_nonstd_func_interface(const struct mmc_card *c)
+static inline int mmc_card_blk_no_cmd23(const struct mmc_card *c)
 {
-	return c->quirks & MMC_QUIRK_NONSTD_FUNC_IF;
+	return c->quirks & MMC_QUIRK_BLK_NO_CMD23;
 }
 
 static inline int mmc_card_broken_byte_mode_512(const struct mmc_card *c)

-- 
2.50.1

[PATCH v3 3/6] mmc: mmc_test: use mmc_card cmd23 helpers

[Benoît Monin]

Use mmc_card_can_cmd23 instead of using a local and partial
implementation, and check for the CMD23 quirk with
mmc_card_blk_no_cmd23.

Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
 drivers/mmc/core/mmc_test.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/core/mmc_test.c b/drivers/mmc/core/mmc_test.c
index 80e5d87a5e50bea772c06443bca8f48f042d6ad5..67d4a301895c905bf52776f970bfc7a8f86a65aa 100644
--- a/drivers/mmc/core/mmc_test.c
+++ b/drivers/mmc/core/mmc_test.c
@@ -180,20 +180,14 @@ static int mmc_test_set_blksize(struct mmc_test_card *test, unsigned size)
 	return mmc_set_blocklen(test->card, size);
 }
 
-static bool mmc_test_card_cmd23(struct mmc_card *card)
-{
-	return mmc_card_mmc(card) ||
-	       (mmc_card_sd(card) && card->scr.cmds & SD_SCR_CMD23_SUPPORT);
-}
-
 static void mmc_test_prepare_sbc(struct mmc_test_card *test,
 				 struct mmc_request *mrq, unsigned int blocks)
 {
 	struct mmc_card *card = test->card;
 
 	if (!mrq->sbc || !mmc_host_can_cmd23(card->host) ||
-	    !mmc_test_card_cmd23(card) || !mmc_op_multi(mrq->cmd->opcode) ||
-	    (card->quirks & MMC_QUIRK_BLK_NO_CMD23)) {
+	    !mmc_card_can_cmd23(card) || !mmc_op_multi(mrq->cmd->opcode) ||
+	    mmc_card_blk_no_cmd23(card)) {
 		mrq->sbc = NULL;
 		return;
 	}

-- 
2.50.1

[PATCH v3 4/6] mmc: block: use mmc_card cmd23 helpers

[Benoît Monin]

Use the dedicated helpers for CMD23 card support.

Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
 drivers/mmc/core/block.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 9cc47bf94804b64a9cc60c7a5d95a0082d546ea9..8fd9891462054da8c00bdbb93e3414614441644d 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1768,8 +1768,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
 	 * 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) ||
-	     do_data_tag)) {
+	    (do_rel_wr || !mmc_card_blk_no_cmd23(card) || do_data_tag)) {
 		brq->sbc.opcode = MMC_SET_BLOCK_COUNT;
 		brq->sbc.arg = brq->data.blocks |
 			(do_rel_wr ? (1 << 31) : 0) |
@@ -2618,13 +2617,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	 */
 	md->read_only = mmc_blk_readonly(card);
 
-	if (mmc_host_can_cmd23(card->host)) {
-		if ((mmc_card_mmc(card) &&
-		     card->csd.mmca_vsn >= CSD_SPEC_VER_3) ||
-		    (mmc_card_sd(card) && !mmc_card_ult_capacity(card) &&
-		     card->scr.cmds & SD_SCR_CMD23_SUPPORT))
-			md->flags |= MMC_BLK_CMD23;
-	}
+	if (mmc_host_can_cmd23(card->host) && mmc_card_can_cmd23(card))
+		md->flags |= MMC_BLK_CMD23;
 
 	if (md->flags & MMC_BLK_CMD23 &&
 	    ((card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||

-- 
2.50.1

[PATCH v3 5/6] mmc: core: add mmc_read_tuning

[Benoît Monin]

Provide a function to the MMC hosts to read some blocks of data as part
of their tuning. The card parameter is optional since it is not
available from the execute_tuning() operation, but present in
execute_hs400_tuning() and prepare_sd_hs_tuning().

This function only returns the status of the read operation, not the
data read.

Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
 drivers/mmc/core/mmc_ops.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/host.h   |  2 ++
 2 files changed, 81 insertions(+)

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 66283825513cb4ff993a1b2ec1f0b0cac4e74487..d29e5daf3e326ab37e61c99456421b1f66bcb0de 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -1077,3 +1077,82 @@ int mmc_sanitize(struct mmc_card *card, unsigned int timeout_ms)
 	return err;
 }
 EXPORT_SYMBOL_GPL(mmc_sanitize);
+
+/**
+ * mmc_read_tuning() - read data blocks from the mmc
+ * @card: mmc card to read from, can be NULL
+ * @host: mmc host doing the read
+ * @blksz: data block size
+ * @blocks: number of blocks to read
+ *
+ * Read one or more blocks of data from the beginning of the mmc. This is a
+ * low-level helper for tuning operation. If card is NULL, it is assumed that
+ * CMD23 can be used for multi-block read.
+ *
+ * Note: Allocate and free a temporary buffer to store the data read. The data
+ * is not available outside of the function, only the status of the read
+ * operation.
+ *
+ * Return: 0 in case of success, otherwise -EIO / -ENOMEM / -E2BIG
+ */
+int mmc_read_tuning(struct mmc_card *card, struct mmc_host *host,
+		    unsigned int blksz, unsigned int blocks)
+{
+	struct mmc_request mrq = {};
+	struct mmc_command sbc = {};
+	struct mmc_command cmd = {};
+	struct mmc_command stop = {};
+	struct mmc_data data = {};
+	struct scatterlist sg;
+	void *buf;
+	unsigned int len;
+
+	if (blocks > 1) {
+		if (mmc_host_can_cmd23(host) &&
+		    (!card || (mmc_card_can_cmd23(card) &&
+		     !mmc_card_blk_no_cmd23(card)))) {
+			mrq.sbc = &sbc;
+			sbc.opcode = MMC_SET_BLOCK_COUNT;
+			sbc.arg = blocks;
+			sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
+		}
+		cmd.opcode = MMC_READ_MULTIPLE_BLOCK;
+		mrq.stop = &stop;
+		stop.opcode = MMC_STOP_TRANSMISSION;
+		stop.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
+	} else {
+		cmd.opcode = MMC_READ_SINGLE_BLOCK;
+	}
+
+	mrq.cmd = &cmd;
+	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
+
+	mrq.data = &data;
+	data.flags = MMC_DATA_READ;
+	data.blksz = blksz;
+	data.blocks = blocks;
+	data.blk_addr = 0;
+	data.sg = &sg;
+	data.sg_len = 1;
+	if (card)
+		mmc_set_data_timeout(&data, card);
+	else
+		data.timeout_ns = 1000000000;
+
+	if (check_mul_overflow(blksz, blocks, &len))
+		return -E2BIG;
+	buf = kmalloc(len, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	sg_init_one(&sg, buf, len);
+
+	mmc_wait_for_req(host, &mrq);
+	kfree(buf);
+
+	if (sbc.error || cmd.error || data.error)
+		return -EIO;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mmc_read_tuning);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 68f09a955a902047ac517441b6820fa6e4166a13..5a6471a6219222b199a16afd9e6bd5ab74b05c86 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -743,5 +743,7 @@ int mmc_send_status(struct mmc_card *card, u32 *status);
 int mmc_send_tuning(struct mmc_host *host, u32 opcode, int *cmd_error);
 int mmc_send_abort_tuning(struct mmc_host *host, u32 opcode);
 int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd);
+int mmc_read_tuning(struct mmc_card *card, struct mmc_host *host,
+		    unsigned int blksz, unsigned int blocks);
 
 #endif /* LINUX_MMC_HOST_H */

-- 
2.50.1

[PATCH v3 6/6] mmc: sdhci-cadence: implement multi-block read gap tuning

[Benoît Monin]

The controller suspends the clock between blocks when reading from the
MMC as part of its flow-control, called read block gap. At higher clock
speed and with IO delay between the controller and the MMC, this clock
pause can happen too late, during the read of the next block and
trigger a read error.

To prevent this, the delay can be programmed for each mode via the pair
of registers HRS37/38. This delay is obtained during tuning, by trying
a multi-block read and increasing the delay until the read succeeds.

For now, the tuning is only done in HS200, as the read error has only
been observed at that speed.

Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
 drivers/mmc/host/sdhci-cadence.c | 69 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
index 2d823e158c59844dc7916db6a1d6e3d8b02ea5a0..0a9a90f9791d343b5d64ed602066f6291efa75b5 100644
--- a/drivers/mmc/host/sdhci-cadence.c
+++ b/drivers/mmc/host/sdhci-cadence.c
@@ -36,6 +36,24 @@
 #define   SDHCI_CDNS_HRS06_MODE_MMC_HS400	0x5
 #define   SDHCI_CDNS_HRS06_MODE_MMC_HS400ES	0x6
 
+/* Read block gap */
+#define SDHCI_CDNS_HRS37		0x94	/* interface mode select */
+#define   SDHCI_CDNS_HRS37_MODE_DS		0x0
+#define   SDHCI_CDNS_HRS37_MODE_HS		0x1
+#define   SDHCI_CDNS_HRS37_MODE_UDS_SDR12	0x8
+#define   SDHCI_CDNS_HRS37_MODE_UDS_SDR25	0x9
+#define   SDHCI_CDNS_HRS37_MODE_UDS_SDR50	0xa
+#define   SDHCI_CDNS_HRS37_MODE_UDS_SDR104	0xb
+#define   SDHCI_CDNS_HRS37_MODE_UDS_DDR50	0xc
+#define   SDHCI_CDNS_HRS37_MODE_MMC_LEGACY	0x20
+#define   SDHCI_CDNS_HRS37_MODE_MMC_SDR		0x21
+#define   SDHCI_CDNS_HRS37_MODE_MMC_DDR		0x22
+#define   SDHCI_CDNS_HRS37_MODE_MMC_HS200	0x23
+#define   SDHCI_CDNS_HRS37_MODE_MMC_HS400	0x24
+#define   SDHCI_CDNS_HRS37_MODE_MMC_HS400ES	0x25
+#define SDHCI_CDNS_HRS38		0x98	/* Read block gap coefficient */
+#define   SDHCI_CDNS_HRS38_BLKGAP_MAX		0xf
+
 /* SRS - Slot Register Set (SDHCI-compatible) */
 #define SDHCI_CDNS_SRS_BASE		0x200
 
@@ -251,6 +269,49 @@ static int sdhci_cdns_set_tune_val(struct sdhci_host *host, unsigned int val)
 	return 0;
 }
 
+/**
+ * sdhci_cdns_tune_blkgap() - tune multi-block read gap
+ * @mmc: MMC host
+ *
+ * Tune delay used in multi block read. To do so,
+ * try sending multi-block read command with incremented gap, unless
+ * it succeeds.
+ *
+ * Return: error code
+ */
+static int sdhci_cdns_tune_blkgap(struct mmc_host *mmc)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_cdns_priv *priv = sdhci_pltfm_priv(pltfm_host);
+	void __iomem *hrs37_reg = priv->hrs_addr + SDHCI_CDNS_HRS37;
+	void __iomem *hrs38_reg = priv->hrs_addr + SDHCI_CDNS_HRS38;
+	int ret;
+	u32 gap;
+	u32 hrs37_mode;
+
+	switch (host->timing) {
+	case MMC_TIMING_MMC_HS200:
+		hrs37_mode = SDHCI_CDNS_HRS37_MODE_MMC_HS200;
+		break;
+	default:
+		return 0; /* no tuning in this mode */
+	}
+
+	writel(hrs37_mode, hrs37_reg);
+
+	for (gap = 0; gap <= SDHCI_CDNS_HRS38_BLKGAP_MAX; gap++) {
+		writel(gap, hrs38_reg);
+		ret = mmc_read_tuning(NULL, mmc, 512, 32);
+		if (ret == 0)
+			break;
+	}
+
+	dev_dbg(mmc_dev(mmc), "read block gap tune %s, gap %d\n",
+		ret == 0 ? "OK" : "failed", gap);
+	return ret;
+}
+
 /*
  * In SD mode, software must not use the hardware tuning and instead perform
  * an almost identical procedure to eMMC.
@@ -261,6 +322,7 @@ static int sdhci_cdns_execute_tuning(struct sdhci_host *host, u32 opcode)
 	int max_streak = 0;
 	int end_of_streak = 0;
 	int i;
+	int ret;
 
 	/*
 	 * Do not execute tuning for UHS_SDR50 or UHS_DDR50.
@@ -288,7 +350,12 @@ static int sdhci_cdns_execute_tuning(struct sdhci_host *host, u32 opcode)
 		return -EIO;
 	}
 
-	return sdhci_cdns_set_tune_val(host, end_of_streak - max_streak / 2);
+	ret = sdhci_cdns_set_tune_val(host, end_of_streak - max_streak / 2);
+
+	if (!ret)
+		ret = sdhci_cdns_tune_blkgap(host->mmc);
+
+	return ret;
 }
 
 static void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host,

-- 
2.50.1

Re: [PATCH v3 0/6] mmc: introduce multi-block read gap tuning

[Ulf Hansson]

On Wed, 16 Jul 2025 at 17:47, Benoît Monin <benoit.monin@bootlin.com> wrote:
>
> This patchset implements the multi-block read gap tuning for the SDHCI
> cadence driver.
>
> The first two patches introduce helpers to check for CMD23 support by
> MMC card: mmc_card_can_cmd23 for support proper and mmc_card_blk_no_cmd23
> for the NO_CMD23 quirk.
>
> The next two patches use the new helpers in mmc/core/mmc_test.c and
> mmc/core/block.c.
>
> The next patch add mmc_read_tuning to read blocks from MMC as part of
> the tuning. This function does not return the data read to the caller,
> only the status of the read operation. It also takes an optional card
> argument, not used when called from execute_tuning() but present in
> execute_hs400_tuning() and prepare_sd_hs_tuning().
>
> Finally the last patch uses mmc_read_tuning to implement the multi-block
> read gap tuning in the cadence host driver, by doing a multi-block read
> and increasing the gap delay until the read succeeds.
>
> v2 -> v3:
> Move the changes related to CMD23 support by MMC card to separate patches
> at the beginning of the series.
> Change the mmc read function to be less appealing to reuse/abuse
> outside of tuning context.

A quick review made me feel a lot better about this, thanks!

Although I will need a few more days for review and possibly it's
getting too late for v6.17 for me, but let's see, I will do my best.

Kind regards
Uffe

>
> Link to v2:
> https://lore.kernel.org/linux-mmc/cover.1751898225.git.benoit.monin@bootlin.com/
>
> v1 -> v2:
> Split the code between the core and the driver by adding a generic
> function to read blocks from the MMC.
>
> Link to v1:
> https://lore.kernel.org/linux-mmc/2a43386ffef4012530ca2421ad81ad21c36c8a25.1750943549.git.benoit.monin@bootlin.com/
>
> Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
> ---
> Benoît Monin (6):
>       mmc: core: add mmc_card_can_cmd23
>       mmc: card: add mmc_card_blk_no_cmd23
>       mmc: mmc_test: use mmc_card cmd23 helpers
>       mmc: block: use mmc_card cmd23 helpers
>       mmc: core: add mmc_read_tuning
>       mmc: sdhci-cadence: implement multi-block read gap tuning
>
>  drivers/mmc/core/block.c         | 12 ++----
>  drivers/mmc/core/card.h          |  9 ++++-
>  drivers/mmc/core/core.c          |  9 +++++
>  drivers/mmc/core/core.h          |  1 +
>  drivers/mmc/core/mmc_ops.c       | 79 ++++++++++++++++++++++++++++++++++++++++
>  drivers/mmc/core/mmc_test.c      | 10 +----
>  drivers/mmc/host/sdhci-cadence.c | 69 ++++++++++++++++++++++++++++++++++-
>  include/linux/mmc/host.h         |  2 +
>  8 files changed, 171 insertions(+), 20 deletions(-)
> ---
> base-commit: 4ad9e44c76b301e786eb4cdab890eac8c7eebd42
> change-id: 20250716-mobileye-emmc-for-upstream-4-40ef552c3b59
>
> Best regards,
> --
> Benoît Monin <benoit.monin@bootlin.com>
>

Re: [PATCH v3 6/6] mmc: sdhci-cadence: implement multi-block read gap tuning

[Adrian Hunter]

On 16/07/2025 18:47, Benoît Monin wrote:
> The controller suspends the clock between blocks when reading from the
> MMC as part of its flow-control, called read block gap. At higher clock
> speed and with IO delay between the controller and the MMC, this clock
> pause can happen too late, during the read of the next block and
> trigger a read error.
> 
> To prevent this, the delay can be programmed for each mode via the pair
> of registers HRS37/38. This delay is obtained during tuning, by trying
> a multi-block read and increasing the delay until the read succeeds.
> 
> For now, the tuning is only done in HS200, as the read error has only
> been observed at that speed.
> 
> Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>

Some minor cosmetic comments below, otherwise:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-cadence.c | 69 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> index 2d823e158c59844dc7916db6a1d6e3d8b02ea5a0..0a9a90f9791d343b5d64ed602066f6291efa75b5 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -36,6 +36,24 @@
>  #define   SDHCI_CDNS_HRS06_MODE_MMC_HS400	0x5
>  #define   SDHCI_CDNS_HRS06_MODE_MMC_HS400ES	0x6
>  
> +/* Read block gap */
> +#define SDHCI_CDNS_HRS37		0x94	/* interface mode select */
> +#define   SDHCI_CDNS_HRS37_MODE_DS		0x0
> +#define   SDHCI_CDNS_HRS37_MODE_HS		0x1
> +#define   SDHCI_CDNS_HRS37_MODE_UDS_SDR12	0x8
> +#define   SDHCI_CDNS_HRS37_MODE_UDS_SDR25	0x9
> +#define   SDHCI_CDNS_HRS37_MODE_UDS_SDR50	0xa
> +#define   SDHCI_CDNS_HRS37_MODE_UDS_SDR104	0xb
> +#define   SDHCI_CDNS_HRS37_MODE_UDS_DDR50	0xc
> +#define   SDHCI_CDNS_HRS37_MODE_MMC_LEGACY	0x20
> +#define   SDHCI_CDNS_HRS37_MODE_MMC_SDR		0x21
> +#define   SDHCI_CDNS_HRS37_MODE_MMC_DDR		0x22
> +#define   SDHCI_CDNS_HRS37_MODE_MMC_HS200	0x23
> +#define   SDHCI_CDNS_HRS37_MODE_MMC_HS400	0x24
> +#define   SDHCI_CDNS_HRS37_MODE_MMC_HS400ES	0x25
> +#define SDHCI_CDNS_HRS38		0x98	/* Read block gap coefficient */
> +#define   SDHCI_CDNS_HRS38_BLKGAP_MAX		0xf
> +
>  /* SRS - Slot Register Set (SDHCI-compatible) */
>  #define SDHCI_CDNS_SRS_BASE		0x200
>  
> @@ -251,6 +269,49 @@ static int sdhci_cdns_set_tune_val(struct sdhci_host *host, unsigned int val)
>  	return 0;
>  }
>  
> +/**
> + * sdhci_cdns_tune_blkgap() - tune multi-block read gap
> + * @mmc: MMC host
> + *
> + * Tune delay used in multi block read. To do so,
> + * try sending multi-block read command with incremented gap, unless
> + * it succeeds.
> + *
> + * Return: error code
> + */
> +static int sdhci_cdns_tune_blkgap(struct mmc_host *mmc)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_cdns_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +	void __iomem *hrs37_reg = priv->hrs_addr + SDHCI_CDNS_HRS37;
> +	void __iomem *hrs38_reg = priv->hrs_addr + SDHCI_CDNS_HRS38;
> +	int ret;
> +	u32 gap;
> +	u32 hrs37_mode;
> +
> +	switch (host->timing) {
> +	case MMC_TIMING_MMC_HS200:
> +		hrs37_mode = SDHCI_CDNS_HRS37_MODE_MMC_HS200;
> +		break;
> +	default:
> +		return 0; /* no tuning in this mode */
> +	}

Could refrain from the switch statement until it is needed.
So for now, just:

	/* Currently only needed in HS200 mode */
	if (host->timing != MMC_TIMING_MMC_HS200)
		return 0;

	writel(SDHCI_CDNS_HRS37_MODE_MMC_HS200, hrs37_reg);

> +
> +	writel(hrs37_mode, hrs37_reg);
> +
> +	for (gap = 0; gap <= SDHCI_CDNS_HRS38_BLKGAP_MAX; gap++) {
> +		writel(gap, hrs38_reg);
> +		ret = mmc_read_tuning(NULL, mmc, 512, 32);
> +		if (ret == 0)

Kernel style is:

		if (!ret)

> +			break;
> +	}
> +
> +	dev_dbg(mmc_dev(mmc), "read block gap tune %s, gap %d\n",
> +		ret == 0 ? "OK" : "failed", gap);

Kernel style is:

		ret ? "failed" : "OK"

Also up to 100 columns is ok, so could be all one line if you like.

> +	return ret;
> +}
> +
>  /*
>   * In SD mode, software must not use the hardware tuning and instead perform
>   * an almost identical procedure to eMMC.
> @@ -261,6 +322,7 @@ static int sdhci_cdns_execute_tuning(struct sdhci_host *host, u32 opcode)
>  	int max_streak = 0;
>  	int end_of_streak = 0;
>  	int i;
> +	int ret;
>  
>  	/*
>  	 * Do not execute tuning for UHS_SDR50 or UHS_DDR50.
> @@ -288,7 +350,12 @@ static int sdhci_cdns_execute_tuning(struct sdhci_host *host, u32 opcode)
>  		return -EIO;
>  	}
>  
> -	return sdhci_cdns_set_tune_val(host, end_of_streak - max_streak / 2);
> +	ret = sdhci_cdns_set_tune_val(host, end_of_streak - max_streak / 2);
> +
> +	if (!ret)
> +		ret = sdhci_cdns_tune_blkgap(host->mmc);
> +
> +	return ret;

Or tidier:

	ret = sdhci_cdns_set_tune_val(host, end_of_streak - max_streak / 2);
	if (ret)
		return ret;

	return sdhci_cdns_tune_blkgap(host->mmc);

>  }
>  
>  static void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host,
> 

Re: [PATCH v3 0/6] mmc: introduce multi-block read gap tuning

[Benoît Monin]

Hello Ulf,

On Friday, 18 July 2025 at 15:04:18 CEST, Ulf Hansson wrote:
> A quick review made me feel a lot better about this, thanks!
> 
> Although I will need a few more days for review and possibly it's
> getting too late for v6.17 for me, but let's see, I will do my best.
> 
Any update on the review?

I have a new version with the small changes suggested by Adrian. I can send
it but I'd rather wait if you also have some comments to avoid doing two
separate versions of the patchset.

Best regards,
-- 
Benoît Monin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v3 5/6] mmc: core: add mmc_read_tuning

[Ulf Hansson]

On Wed, 16 Jul 2025 at 17:47, Benoît Monin <benoit.monin@bootlin.com> wrote:
>
> Provide a function to the MMC hosts to read some blocks of data as part
> of their tuning. The card parameter is optional since it is not
> available from the execute_tuning() operation, but present in
> execute_hs400_tuning() and prepare_sd_hs_tuning().
>
> This function only returns the status of the read operation, not the
> data read.
>
> Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
> ---
>  drivers/mmc/core/mmc_ops.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mmc/host.h   |  2 ++
>  2 files changed, 81 insertions(+)
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 66283825513cb4ff993a1b2ec1f0b0cac4e74487..d29e5daf3e326ab37e61c99456421b1f66bcb0de 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -1077,3 +1077,82 @@ int mmc_sanitize(struct mmc_card *card, unsigned int timeout_ms)
>         return err;
>  }
>  EXPORT_SYMBOL_GPL(mmc_sanitize);
> +
> +/**
> + * mmc_read_tuning() - read data blocks from the mmc
> + * @card: mmc card to read from, can be NULL
> + * @host: mmc host doing the read
> + * @blksz: data block size
> + * @blocks: number of blocks to read
> + *
> + * Read one or more blocks of data from the beginning of the mmc. This is a
> + * low-level helper for tuning operation. If card is NULL, it is assumed that
> + * CMD23 can be used for multi-block read.

It makes sense to have a comment for this, but I would just just drop
the "struct mmc_card *card" as an in-parameter. If it turns out to be
needed later on, we would need additional changes to enable the caller
of mmc_read_tuning() to pass the "card" along.

> + *
> + * Note: Allocate and free a temporary buffer to store the data read. The data
> + * is not available outside of the function, only the status of the read
> + * operation.
> + *
> + * Return: 0 in case of success, otherwise -EIO / -ENOMEM / -E2BIG
> + */
> +int mmc_read_tuning(struct mmc_card *card, struct mmc_host *host,
> +                   unsigned int blksz, unsigned int blocks)
> +{
> +       struct mmc_request mrq = {};
> +       struct mmc_command sbc = {};
> +       struct mmc_command cmd = {};
> +       struct mmc_command stop = {};
> +       struct mmc_data data = {};
> +       struct scatterlist sg;
> +       void *buf;
> +       unsigned int len;
> +
> +       if (blocks > 1) {
> +               if (mmc_host_can_cmd23(host) &&
> +                   (!card || (mmc_card_can_cmd23(card) &&
> +                    !mmc_card_blk_no_cmd23(card)))) {
> +                       mrq.sbc = &sbc;
> +                       sbc.opcode = MMC_SET_BLOCK_COUNT;
> +                       sbc.arg = blocks;
> +                       sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
> +               }
> +               cmd.opcode = MMC_READ_MULTIPLE_BLOCK;
> +               mrq.stop = &stop;
> +               stop.opcode = MMC_STOP_TRANSMISSION;
> +               stop.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
> +       } else {
> +               cmd.opcode = MMC_READ_SINGLE_BLOCK;
> +       }
> +
> +       mrq.cmd = &cmd;
> +       cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
> +
> +       mrq.data = &data;
> +       data.flags = MMC_DATA_READ;
> +       data.blksz = blksz;
> +       data.blocks = blocks;
> +       data.blk_addr = 0;
> +       data.sg = &sg;
> +       data.sg_len = 1;
> +       if (card)
> +               mmc_set_data_timeout(&data, card);
> +       else
> +               data.timeout_ns = 1000000000;
> +
> +       if (check_mul_overflow(blksz, blocks, &len))
> +               return -E2BIG;
> +       buf = kmalloc(len, GFP_KERNEL);
> +       if (!buf)
> +               return -ENOMEM;
> +
> +       sg_init_one(&sg, buf, len);
> +
> +       mmc_wait_for_req(host, &mrq);
> +       kfree(buf);
> +
> +       if (sbc.error || cmd.error || data.error)
> +               return -EIO;
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(mmc_read_tuning);
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 68f09a955a902047ac517441b6820fa6e4166a13..5a6471a6219222b199a16afd9e6bd5ab74b05c86 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -743,5 +743,7 @@ int mmc_send_status(struct mmc_card *card, u32 *status);
>  int mmc_send_tuning(struct mmc_host *host, u32 opcode, int *cmd_error);
>  int mmc_send_abort_tuning(struct mmc_host *host, u32 opcode);
>  int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd);
> +int mmc_read_tuning(struct mmc_card *card, struct mmc_host *host,
> +                   unsigned int blksz, unsigned int blocks);
>
>  #endif /* LINUX_MMC_HOST_H */
>
> --
> 2.50.1
>

Otherwise this looks good to me!

Kind regards
Uffe

Re: [PATCH v3 0/6] mmc: introduce multi-block read gap tuning

[Ulf Hansson]

On Thu, 14 Aug 2025 at 16:07, Benoît Monin <benoit.monin@bootlin.com> wrote:
>
> Hello Ulf,
>
> On Friday, 18 July 2025 at 15:04:18 CEST, Ulf Hansson wrote:
> > A quick review made me feel a lot better about this, thanks!
> >
> > Although I will need a few more days for review and possibly it's
> > getting too late for v6.17 for me, but let's see, I will do my best.
> >
> Any update on the review?

Sorry for the delay. Just a minor comment on patch5, but the rest
looks good to me!

>
> I have a new version with the small changes suggested by Adrian. I can send
> it but I'd rather wait if you also have some comments to avoid doing two
> separate versions of the patchset.

Thanks for pinging me! I will try to be quicker next time.

Kind regards
Uffe