Add a generic function to read some blocks of data from the MMC, to be
used by drivers as part of their tuning.

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

diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
index 9cbdd240c3a7d..93fd502c1f5fc 100644
--- a/drivers/mmc/core/card.h
+++ b/drivers/mmc/core/card.h
@@ -11,6 +11,7 @@
 #define _MMC_CORE_CARD_H
 
 #include <linux/mmc/card.h>
+#include <linux/mmc/mmc.h>
 
 #define mmc_card_name(c)	((c)->cid.prod_name)
 #define mmc_card_id(c)		(dev_name(&(c)->dev))
@@ -300,4 +301,13 @@ static inline int mmc_card_no_uhs_ddr50_tuning(const struct mmc_card *c)
 	return c->quirks & MMC_QUIRK_NO_UHS_DDR50_TUNING;
 }
 
+static inline 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)) &&
+		!(card->quirks & MMC_QUIRK_BLK_NO_CMD23);
+}
+
 #endif
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 66283825513cb..848d8aa3ff2b5 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -1077,3 +1077,72 @@ int mmc_sanitize(struct mmc_card *card, unsigned int timeout_ms)
 	return err;
 }
 EXPORT_SYMBOL_GPL(mmc_sanitize);
+
+/**
+ * mmc_read_blocks() - 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
+ * @blk_addr: first block address
+ * @buf: output buffer
+ * @len: size of the buffer
+ *
+ * Read one or more blocks of data from 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.
+ *
+ * Return: 0 in case of success, otherwise -EIO
+ */
+int mmc_read_blocks(struct mmc_card *card, struct mmc_host *host,
+		    unsigned int blksz, unsigned int blocks,
+		    unsigned int blk_addr, void *buf, unsigned int len)
+{
+	struct mmc_request mrq = {};
+	struct mmc_command sbc = {};
+	struct mmc_command cmd = {};
+	struct mmc_command stop = {};
+	struct mmc_data data = {};
+	struct scatterlist sg;
+
+	if (blocks > 1) {
+		if (mmc_host_can_cmd23(host) &&
+		    (!card || mmc_card_can_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 = blk_addr;
+	data.sg = &sg;
+	data.sg_len = 1;
+	if (card)
+		mmc_set_data_timeout(&data, card);
+	else
+		data.timeout_ns = 1000000000;
+
+	sg_init_one(&sg, buf, len);
+
+	mmc_wait_for_req(host, &mrq);
+
+	if (sbc.error || cmd.error || data.error)
+		return -EIO;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mmc_read_blocks);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 68f09a955a902..72196817a6f0f 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -743,5 +743,8 @@ 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_blocks(struct mmc_card *card, struct mmc_host *host,
+		    unsigned int blksz, unsigned int blocks,
+		    unsigned int blk_addr, void *buf, unsigned int len);
 
 #endif /* LINUX_MMC_HOST_H */

On Mon, 7 Jul 2025 at 17:24, Benoît Monin <benoit.monin@bootlin.com> wrote:
>
> Add a generic function to read some blocks of data from the MMC, to be
> used by drivers as part of their tuning.
>
> Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
> ---
>  drivers/mmc/core/card.h    | 10 ++++++
>  drivers/mmc/core/mmc_ops.c | 69 ++++++++++++++++++++++++++++++++++++++
>  include/linux/mmc/host.h   |  3 ++
>  3 files changed, 82 insertions(+)
>
> diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
> index 9cbdd240c3a7d..93fd502c1f5fc 100644
> --- a/drivers/mmc/core/card.h
> +++ b/drivers/mmc/core/card.h
> @@ -11,6 +11,7 @@
>  #define _MMC_CORE_CARD_H
>
>  #include <linux/mmc/card.h>
> +#include <linux/mmc/mmc.h>
>
>  #define mmc_card_name(c)       ((c)->cid.prod_name)
>  #define mmc_card_id(c)         (dev_name(&(c)->dev))
> @@ -300,4 +301,13 @@ static inline int mmc_card_no_uhs_ddr50_tuning(const struct mmc_card *c)
>         return c->quirks & MMC_QUIRK_NO_UHS_DDR50_TUNING;
>  }
>
> +static inline 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)) &&
> +               !(card->quirks & MMC_QUIRK_BLK_NO_CMD23);

First, please make the above part a separate patch. It makes sense to
add a helper for this, as you show in patch3 and patch4. I also
recommend that these patches should also be re-ordered so they come
first in the series.

Second, I don't think we should mix mmc_card_can* functions with the
card-quirks. Better to have two separate helpers, especially since
CMD23 is used for other things too, like RPMB and reliable writes, for
example. Thus I suggest we add:

mmc_card_can_cmd23() - which looks at what the card supports, similar
to above without MMC_QUIRK_BLK_NO_CMD23. Put the definition in
drivers/mmc/core/core.h and export the symbols, similar to what we do
for mmc_card_can_erase() and friends.

mmc_card_broken_blk_cmd23() - which should only check
MMC_QUIRK_BLK_NO_CMD23. This belongs in drivers/mmc/core/card.h.

> +}
> +
>  #endif
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 66283825513cb..848d8aa3ff2b5 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -1077,3 +1077,72 @@ int mmc_sanitize(struct mmc_card *card, unsigned int timeout_ms)
>         return err;
>  }
>  EXPORT_SYMBOL_GPL(mmc_sanitize);
> +
> +/**
> + * mmc_read_blocks() - 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
> + * @blk_addr: first block address
> + * @buf: output buffer
> + * @len: size of the buffer
> + *
> + * Read one or more blocks of data from 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.
> + *
> + * Return: 0 in case of success, otherwise -EIO
> + */
> +int mmc_read_blocks(struct mmc_card *card, struct mmc_host *host,
> +                   unsigned int blksz, unsigned int blocks,
> +                   unsigned int blk_addr, void *buf, unsigned int len)
> +{
> +       struct mmc_request mrq = {};
> +       struct mmc_command sbc = {};
> +       struct mmc_command cmd = {};
> +       struct mmc_command stop = {};
> +       struct mmc_data data = {};
> +       struct scatterlist sg;
> +
> +       if (blocks > 1) {
> +               if (mmc_host_can_cmd23(host) &&
> +                   (!card || mmc_card_can_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 = blk_addr;
> +       data.sg = &sg;
> +       data.sg_len = 1;
> +       if (card)
> +               mmc_set_data_timeout(&data, card);
> +       else
> +               data.timeout_ns = 1000000000;
> +
> +       sg_init_one(&sg, buf, len);
> +
> +       mmc_wait_for_req(host, &mrq);
> +
> +       if (sbc.error || cmd.error || data.error)
> +               return -EIO;
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(mmc_read_blocks);
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 68f09a955a902..72196817a6f0f 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -743,5 +743,8 @@ 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_blocks(struct mmc_card *card, struct mmc_host *host,
> +                   unsigned int blksz, unsigned int blocks,
> +                   unsigned int blk_addr, void *buf, unsigned int len);

I really think we must avoid exporting such a generic function. This
becomes visible outside the mmc subsystem and I am worried that it
will be abused.

Can we perhaps make it harder to integrate with the tuning support on
the core, somehow? I haven't thought much about it, but maybe you can
propose something along those lines - otherwise I will try to think of
another way to do it.

>
>  #endif /* LINUX_MMC_HOST_H */

Kind regards
Uffe