[RFC PATCH] mmc: meson-mx-sdhc: add DMA bounce buffer support

[RFC PATCH] mmc: meson-mx-sdhc: add DMA bounce buffer support

[ZHENPENG YUAN]

The Meson MX SDHC controller uses a single DMA address for each
transfer. Since the driver leaves max_segs at the MMC core default
of 1, large I/O requests are split into single-segment requests,
typically 4 KiB each.

On an Amlogic S805, this limits eMMC sequential read throughput
to approximately 15 MiB/s despite using HS200 timing.

Add a 128 KiB DMA bounce buffer for multi-segment requests and
advertise up to 64 scatterlist segments. Single-segment requests
continue to use direct DMA. If buffer allocation fails, fall back
to the original single-segment limit.

Testing on a Xunlei OneCloud (Meson8b, Samsung 8GME4R eMMC)
running Linux 7.3.1:

  Block size       Before       After
  4 KiB            14.47        14.69 MiB/s
  64 KiB           15.06        63.21 MiB/s
  128 KiB          15.08        75.13 MiB/s
  1 MiB            15.07        75.93 MiB/s

For a 128 MiB sequential read, block requests decreased from
32768 to 1024.

Direct I/O SHA256 readback verification and fio CRC32C write
verification passed. No MMC errors were observed.

Assisted-by: LLM
Signed-off-by: ZHENPENG YUAN <yuanzhenpeng210@gmail.com>
---
 drivers/mmc/host/meson-mx-sdhc-mmc.c | 165 ++++++++++++++++++++++++---
 1 file changed, 149 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c
index fb49ea712..c62583a80 100644
--- a/drivers/mmc/host/meson-mx-sdhc-mmc.c
+++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c
@@ -16,6 +16,7 @@
 #include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
+#include <linux/scatterlist.h>
 #include <linux/types.h>
 
 #include <linux/mmc/host.h>
@@ -29,6 +30,12 @@
 #define MESON_SDHC_MAX_BLK_SIZE					512
 #define MESON_SDHC_NUM_TUNING_TRIES				10
 
+/*
+ * A 128 KiB request spans at most 33 pages when it is not page aligned.
+ * The extra entries give the block layer room to merge a full request.
+ */
+#define MESON_SDHC_MAX_SEGS					64
+
 #define MESON_SDHC_WAIT_CMD_READY_SLEEP_US			1
 #define MESON_SDHC_WAIT_CMD_READY_TIMEOUT_US			100000
 #define MESON_SDHC_WAIT_BEFORE_SEND_SLEEP_US			1
@@ -48,6 +55,17 @@ struct meson_mx_sdhc_host {
 	struct mmc_command		*cmd;
 	int				error;
 
+	/*
+	 * The controller has a single DMA address register, so requests that
+	 * are not physically contiguous have to be bounced through this
+	 * buffer.
+	 */
+	void				*bounce_buf;
+	size_t				bounce_buf_size;
+	dma_addr_t			dma_addr;
+	unsigned int			dma_len;
+	bool				dma_bounce;
+
 	struct regmap			*regmap;
 
 	struct clk			*pclk;
@@ -220,8 +238,7 @@ static void meson_mx_sdhc_start_cmd(struct mmc_host *mmc,
 			   FIELD_PREP(MESON_SDHC_CTRL_PACK_LEN, pack_len));
 
 	if (cmd->data)
-		regmap_write(host->regmap, MESON_SDHC_ADDR,
-			     sg_dma_address(cmd->data->sg));
+		regmap_write(host->regmap, MESON_SDHC_ADDR, host->dma_addr);
 
 	meson_mx_sdhc_wait_cmd_ready(mmc);
 
@@ -362,32 +379,97 @@ static void meson_mx_sdhc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 static int meson_mx_sdhc_map_dma(struct mmc_host *mmc, struct mmc_request *mrq)
 {
+	struct meson_mx_sdhc_host *host = mmc_priv(mmc);
 	struct mmc_data *data = mrq->data;
-	unsigned int dma_len;
+	enum dma_data_direction dir;
+	struct scatterlist *sg;
+	unsigned int len, total_len;
+	int i, nents;
 
 	if (!data)
 		return 0;
 
-	dma_len = dma_map_sg(mmc_dev(mmc), data->sg, data->sg_len,
-			     mmc_get_dma_dir(data));
-	if (!dma_len) {
-		dev_err(mmc_dev(mmc), "dma_map_sg failed\n");
+	dir = mmc_get_dma_dir(data);
+	len = data->blocks * data->blksz;
+	host->dma_len = len;
+	host->dma_bounce = false;
+
+	/*
+	 * A request that consists of a single scatterlist entry is already
+	 * contiguous from the controller's point of view and can be
+	 * programmed directly.  This is also the only case where transfers
+	 * larger than the advertised max_req_size may occur (SDIO).
+	 */
+	if (data->sg_len == 1 && data->sg->length >= len) {
+		nents = dma_map_sg(mmc_dev(mmc), data->sg, data->sg_len, dir);
+		if (!nents) {
+			dev_err(mmc_dev(mmc), "dma_map_sg failed\n");
+			return -ENOMEM;
+		}
+
+		host->dma_addr = sg_dma_address(data->sg);
+		return 0;
+	}
+
+	total_len = 0;
+	for_each_sg(data->sg, sg, data->sg_len, i)
+		total_len += sg->length;
+
+	if (!host->bounce_buf || len > host->bounce_buf_size ||
+	    total_len < len) {
+		dev_err(mmc_dev(mmc),
+			"cannot DMA %u bytes in %u segments\n",
+			len, data->sg_len);
+		return -EINVAL;
+	}
+
+	if (dir == DMA_TO_DEVICE) {
+		if (sg_copy_to_buffer(data->sg, data->sg_len, host->bounce_buf,
+				      len) != len) {
+			dev_err(mmc_dev(mmc),
+				"failed to copy data into the bounce buffer\n");
+			return -EIO;
+		}
+	}
+
+	host->dma_addr = dma_map_single(mmc_dev(mmc), host->bounce_buf, len,
+					dir);
+	if (dma_mapping_error(mmc_dev(mmc), host->dma_addr)) {
+		dev_err(mmc_dev(mmc), "dma_map_single failed\n");
 		return -ENOMEM;
 	}
 
+	host->dma_bounce = true;
+
 	return 0;
 }
 
+static void meson_mx_sdhc_unmap_dma(struct meson_mx_sdhc_host *host,
+				    struct mmc_data *data)
+{
+	struct device *dev = mmc_dev(host->mmc);
+
+	if (host->dma_bounce) {
+		dma_unmap_single(dev, host->dma_addr, host->dma_len,
+				 mmc_get_dma_dir(data));
+		host->dma_bounce = false;
+	} else {
+		dma_unmap_sg(dev, data->sg, data->sg_len,
+			     mmc_get_dma_dir(data));
+	}
+}
+
 static void meson_mx_sdhc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
 	struct meson_mx_sdhc_host *host = mmc_priv(mmc);
 	struct mmc_command *cmd = mrq->cmd;
+	int ret = 0;
 
 	if (!host->error)
-		host->error = meson_mx_sdhc_map_dma(mmc, mrq);
+		ret = meson_mx_sdhc_map_dma(mmc, mrq);
 
-	if (host->error) {
-		cmd->error = host->error;
+	if (host->error || ret) {
+		cmd->error = host->error ? host->error : ret;
 		mmc_request_done(mmc, mrq);
 		return;
 	}
@@ -596,11 +678,6 @@ static irqreturn_t meson_mx_sdhc_irq_thread(int irq, void *irq_data)
 			regmap_update_bits(host->regmap, MESON_SDHC_PDMA, val,
 					   val);
 		}
-
-		dma_unmap_sg(mmc_dev(host->mmc), cmd->data->sg,
-			     cmd->data->sg_len, mmc_get_dma_dir(cmd->data));
-
-		cmd->data->bytes_xfered = cmd->data->blksz * cmd->data->blocks;
 	}
 
 	meson_mx_sdhc_wait_cmd_ready(host->mmc);
@@ -625,6 +702,40 @@ static irqreturn_t meson_mx_sdhc_irq_thread(int irq, void *irq_data)
 		 */
 		meson_mx_sdhc_clear_fifo(host->mmc);
 
+	if (cmd->data) {
+		struct mmc_data *data = cmd->data;
+		size_t len = data->blksz * data->blocks;
+		bool bounced = host->dma_bounce;
+
+		/*
+		 * Unmap the data after the controller has been quiesced so
+		 * that neither the reset nor the FIFO flush can race with an
+		 * ongoing DMA transfer.  dma_unmap_single() (for bounced
+		 * reads) also invalidates the CPU caches, so the copy below
+		 * sees the data written by the controller.
+		 */
+		meson_mx_sdhc_unmap_dma(host, data);
+
+		if (!data->error) {
+			if (bounced && data->flags & MMC_DATA_READ) {
+				size_t copied;
+
+				copied = sg_copy_from_buffer(data->sg,
+							     data->sg_len,
+							     host->bounce_buf,
+							     len);
+				if (copied != len) {
+					dev_err(mmc_dev(host->mmc),
+						"failed to copy data from the bounce buffer\n");
+					data->error = -EIO;
+				}
+			}
+
+			if (!data->error)
+				data->bytes_xfered = len;
+		}
+	}
+
 	meson_mx_sdhc_request_done(host);
 
 	return IRQ_HANDLED;
@@ -813,8 +924,17 @@ static int meson_mx_sdhc_probe(struct platform_device *pdev)
 
 	mmc->max_req_size = SZ_128K;
 	mmc->max_seg_size = mmc->max_req_size;
-	mmc->max_blk_count = FIELD_GET(MESON_SDHC_SEND_TOTAL_PACK, ~0);
 	mmc->max_blk_size = MESON_SDHC_MAX_BLK_SIZE;
+	/*
+	 * The controller has a single DMA address register.  Allowing the
+	 * block layer to merge more segments is only safe because requests
+	 * consisting of multiple segments are bounced through a contiguous
+	 * buffer.  Keep every request within max_req_size so that it always
+	 * fits into that buffer; SDIO uses max_blk_count to split transfers
+	 * as well.
+	 */
+	mmc->max_blk_count = mmc->max_req_size / mmc->max_blk_size;
+	mmc->max_segs = MESON_SDHC_MAX_SEGS;
 	mmc->max_busy_timeout = 30 * MSEC_PER_SEC;
 	mmc->f_min = clk_round_rate(host->sd_clk, 1);
 	mmc->f_max = clk_round_rate(host->sd_clk, ULONG_MAX);
@@ -823,6 +943,19 @@ static int meson_mx_sdhc_probe(struct platform_device *pdev)
 	mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_HW_RESET;
 	mmc->ops = &meson_mx_sdhc_ops;
 
+	host->bounce_buf = devm_kmalloc(dev, mmc->max_req_size, GFP_KERNEL);
+	if (host->bounce_buf) {
+		host->bounce_buf_size = mmc->max_req_size;
+	} else {
+		/*
+		 * Without the bounce buffer only physically contiguous
+		 * requests can be handled, so fall back to the old limit.
+		 */
+		mmc->max_segs = 1;
+		dev_warn(dev,
+			 "failed to allocate the DMA bounce buffer, using single-segment transfers\n");
+	}
+
 	ret = mmc_of_parse(mmc);
 	if (ret)
 		goto err_disable_pclk;
-- 
2.56.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [RFC PATCH] mmc: meson-mx-sdhc: add DMA bounce buffer support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] mmc: meson-mx-sdhc: missing cmd->error check in meson_mx_sdhc_irq_thread()
--

commit dab15b5e51f3b15121298d5095440b6a71ad6e0f
Author: ZHENPENG YUAN <yuanzhenpeng210@gmail.com>

mmc: meson-mx-sdhc: add DMA bounce buffer support

This commit adds a 128 KiB DMA bounce buffer for multi-segment scatterlist
requests in the Meson MX SDHC driver and increases max_segs to 64. Single
segment requests continue to use direct DMA, while multi-segment transfers
are copied through the bounce buffer.

> diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c
> index fb49ea71289e6..c62583a804f49 100644
> --- a/drivers/mmc/host/meson-mx-sdhc-mmc.c
> +++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c

[ ... ]

> @@ -625,6 +702,40 @@ static irqreturn_t meson_mx_sdhc_irq_thread(int irq, void *irq_data)
>  		 */
>  		meson_mx_sdhc_clear_fifo(host->mmc);
>  
> +	if (cmd->data) {
> +		struct mmc_data *data = cmd->data;
> +		size_t len = data->blksz * data->blocks;
> +		bool bounced = host->dma_bounce;
> +
> +		/*
> +		 * Unmap the data after the controller has been quiesced so
> +		 * that neither the reset nor the FIFO flush can race with an
> +		 * ongoing DMA transfer.  dma_unmap_single() (for bounced
> +		 * reads) also invalidates the CPU caches, so the copy below
> +		 * sees the data written by the controller.
> +		 */
> +		meson_mx_sdhc_unmap_dma(host, data);
> +
> +		if (!data->error) {
> +			if (bounced && data->flags & MMC_DATA_READ) {
> +				size_t copied;
> +
> +				copied = sg_copy_from_buffer(data->sg,
> +							     data->sg_len,
> +							     host->bounce_buf,
> +							     len);
> +				if (copied != len) {
> +					dev_err(mmc_dev(host->mmc),
> +						"failed to copy data from the bounce buffer\n");
> +					data->error = -EIO;
> +				}
> +			}
> +
> +			if (!data->error)
> +				data->bytes_xfered = len;
> +		}
> +	}

[Severity: Critical]
Should this block in meson_mx_sdhc_irq_thread() check !cmd->error in addition
to !data->error before copying from host->bounce_buf and setting
data->bytes_xfered?

When a command with cmd->data encounters a FIFO overrun/underrun or a command
response CRC or timeout error, meson_mx_sdhc_irq() sets cmd->error while
leaving cmd->data->error as 0:

drivers/mmc/host/meson-mx-sdhc-mmc.c:meson_mx_sdhc_irq() {
	if (ista & MESON_SDHC_ISTA_RXFIFO_FULL ||
	    ista & MESON_SDHC_ISTA_TXFIFO_EMPTY)
		cmd->error = -EIO;
	else if (ista & MESON_SDHC_ISTA_RESP_ERR_CRC)
		cmd->error = -EILSEQ;
	else if (ista & MESON_SDHC_ISTA_RESP_TIMEOUT)
		cmd->error = -ETIMEDOUT;

	if (cmd->data) {
		if (ista & MESON_SDHC_ISTA_DATA_ERR_CRC)
			cmd->data->error = -EILSEQ;
		else if (ista & MESON_SDHC_ISTA_DATA_TIMEOUT)
			cmd->data->error = -ETIMEDOUT;
	}
}

In meson_mx_sdhc_irq_thread(), if cmd->error is -EIO or -ETIMEDOUT,
meson_mx_sdhc_reset() resets the controller. However, because the post-unmap
completion check only tests !data->error:

1. On a bounced multi-segment read, sg_copy_from_buffer() still copies len
   bytes from host->bounce_buf into data->sg even though the transfer failed
   or was aborted, overwriting the caller's scatterlist with uninitialized
   devm_kmalloc() heap memory or stale data from an earlier transfer.

2. data->bytes_xfered is set to len despite cmd->error being non-zero. In
   mmc_blk_mq_poll_completion() -> mmc_blk_mq_rw_recovery(), if CMD13 status
   polling finds the card in the transfer state without card-side error bits,
   recovery sees a non-zero brq->data.bytes_xfered and returns early:

drivers/mmc/core/block.c:mmc_blk_mq_rw_recovery() {
	...
	if (brq->data.bytes_xfered)
		return;
}

Could this cause mmc_blk_mq_complete_rq() to complete a failed read or write
request with BLK_STS_OK, resulting in silent data corruption or data loss?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031200925.3364-1-yuanzhenpeng210@gmail.com?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [RFC PATCH] mmc: meson-mx-sdhc: add DMA bounce buffer support

[Martin Blumenstingl]

Hi,

Thank you for this patch!

On Sat, Oct 31, 2026 at 9:09 PM ZHENPENG YUAN <yuanzhenpeng210@gmail.com> wrote:
>
> The Meson MX SDHC controller uses a single DMA address for each
> transfer. Since the driver leaves max_segs at the MMC core default
> of 1, large I/O requests are split into single-segment requests,
> typically 4 KiB each.
>
> On an Amlogic S805, this limits eMMC sequential read throughput
> to approximately 15 MiB/s despite using HS200 timing.
>
> Add a 128 KiB DMA bounce buffer for multi-segment requests and
FWIW, while reviewing your patch I found that Amlogic actually uses a
512KiB bounce buffer in their downstream driver: [0]

> advertise up to 64 scatterlist segments. Single-segment requests
> continue to use direct DMA. If buffer allocation fails, fall back
> to the original single-segment limit.
>
> Testing on a Xunlei OneCloud (Meson8b, Samsung 8GME4R eMMC)
> running Linux 7.3.1:
>
>   Block size       Before       After
>   4 KiB            14.47        14.69 MiB/s
>   64 KiB           15.06        63.21 MiB/s
>   128 KiB          15.08        75.13 MiB/s
>   1 MiB            15.07        75.93 MiB/s
>
> For a 128 MiB sequential read, block requests decreased from
> 32768 to 1024.
That's a nice performance improvement!

> Direct I/O SHA256 readback verification and fio CRC32C write
> verification passed. No MMC errors were observed.
>
> Assisted-by: LLM
> Signed-off-by: ZHENPENG YUAN <yuanzhenpeng210@gmail.com>
Could you please compare your patch to simply setting mmc->caps2 |=
MMC_CAP2_MERGE_CAPABLE; ?
From what I understand this will allow physically contiguous SG
entries to be merged by mmc core.

Ulf: the code here looks similar to the sdhci.c implementation. I'm
wondering if there were any plans to somehow move this to MMC core.


Best regards,
Martin


[0] https://github.com/endlessm/linux-meson/blob/d6e13c220931110fe676ede6da69fc61a7cb04b6/arch/arm/mach-meson8/include/mach/sd.h#L627

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic