[PATCH v2] Revert "mmc: dw_mmc: Extract FIFO data transfer into a separate routine"

Jonas Karlman jonas at kwiboo.se
Tue Oct 8 21:18:31 CEST 2024


The commit 0252924ac6d4 ("mmc: dw_mmc: Extract FIFO data transfer into a
separate routine") unintentionally changed behavior of the FIFO data
transfer routine.

When data is read and size reaches 0 the original loop would wait on
DWMCI_INTMSK_DTO or timeout. The remaining size to read and buf position
is no longer tracked across dwmci_data_transfer_fifo() calls and because
of this an extra call to fifo() and dwmci_fifo_ready() may now trigger a
FIFO underflow timeout error and slows down FIFO reading.

  Buswidth = 4, clock: 50000000
  Sending CMD16
  Sending CMD17
  dwmci_fifo_ready: FIFO underflow timeout
  Sending CMD16
  Sending CMD18
  dwmci_fifo_ready: FIFO underflow timeout
  Sending CMD12
  ## Checking hash(es) for config config-1 ... OK

This reverts commit 0252924ac6d4af69061bb9589d16b30c5bdb7178 to restore
the old working behavior.

Fixes: 0252924ac6d4 ("mmc: dw_mmc: Extract FIFO data transfer into a separate routine")
Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
---
Changes in v2:
- Change to revert the offending commit, both size and buf where reset
  between fifo() calls, easier to do a full revert.
- R-b tag not collected because of this being a different patch.

Reading FIT from SD-card take ~20-30 seconds on a Rockchip RK3328 board
without this reverted, and goes back to sub second once reverted.
---
 drivers/mmc/dw_mmc.c | 103 ++++++++++++++++++++++---------------------
 1 file changed, 52 insertions(+), 51 deletions(-)

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 8551eac70185..e1110cace897 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -214,66 +214,24 @@ static unsigned int dwmci_get_timeout(struct mmc *mmc, const unsigned int size)
 	return timeout;
 }
 
-static int dwmci_data_transfer_fifo(struct dwmci_host *host,
-				    struct mmc_data *data, u32 mask)
+static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data)
 {
-	const u32 int_rx = mask & (DWMCI_INTMSK_RXDR | DWMCI_INTMSK_DTO);
-	const u32 int_tx = mask & DWMCI_INTMSK_TXDR;
+	struct mmc *mmc = host->mmc;
 	int ret = 0;
-	u32 len = 0, size, i;
-	u32 *buf;
-
-	size = (data->blocksize * data->blocks) / 4;
-	if (!host->fifo_mode || !size)
-		return 0;
+	u32 timeout, mask, size, i, len = 0;
+	u32 *buf = NULL;
+	ulong start = get_timer(0);
 
+	size = data->blocksize * data->blocks;
 	if (data->flags == MMC_DATA_READ)
 		buf = (unsigned int *)data->dest;
 	else
 		buf = (unsigned int *)data->src;
 
-	if (data->flags == MMC_DATA_READ && int_rx) {
-		dwmci_writel(host, DWMCI_RINTSTS, int_rx);
-		while (size) {
-			ret = dwmci_fifo_ready(host, DWMCI_FIFO_EMPTY, &len);
-			if (ret < 0)
-				break;
-
-			len = (len >> DWMCI_FIFO_SHIFT) & DWMCI_FIFO_MASK;
-			len = min(size, len);
-			for (i = 0; i < len; i++)
-				*buf++ = dwmci_readl(host, DWMCI_DATA);
-			size = size > len ? (size - len) : 0;
-		}
-	} else if (data->flags == MMC_DATA_WRITE && int_tx) {
-		while (size) {
-			ret = dwmci_fifo_ready(host, DWMCI_FIFO_FULL, &len);
-			if (ret < 0)
-				break;
-
-			len = host->fifo_depth - ((len >> DWMCI_FIFO_SHIFT) &
-						  DWMCI_FIFO_MASK);
-			len = min(size, len);
-			for (i = 0; i < len; i++)
-				dwmci_writel(host, DWMCI_DATA, *buf++);
-			size = size > len ? (size - len) : 0;
-		}
-		dwmci_writel(host, DWMCI_RINTSTS, DWMCI_INTMSK_TXDR);
-	}
-
-	return ret;
-}
-
-static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data)
-{
-	struct mmc *mmc = host->mmc;
-	int ret = 0;
-	u32 timeout, mask, size;
-	ulong start = get_timer(0);
-
-	size = data->blocksize * data->blocks;
 	timeout = dwmci_get_timeout(mmc, size);
 
+	size /= 4;
+
 	for (;;) {
 		mask = dwmci_readl(host, DWMCI_RINTSTS);
 		/* Error during data transfer */
@@ -283,7 +241,50 @@ static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data)
 			break;
 		}
 
-		ret = dwmci_data_transfer_fifo(host, data, mask);
+		if (host->fifo_mode && size) {
+			len = 0;
+			if (data->flags == MMC_DATA_READ &&
+			    (mask & (DWMCI_INTMSK_RXDR | DWMCI_INTMSK_DTO))) {
+				dwmci_writel(host, DWMCI_RINTSTS,
+					     mask & (DWMCI_INTMSK_RXDR |
+						     DWMCI_INTMSK_DTO));
+				while (size) {
+					ret = dwmci_fifo_ready(host,
+							DWMCI_FIFO_EMPTY,
+							&len);
+					if (ret < 0)
+						break;
+
+					len = (len >> DWMCI_FIFO_SHIFT) &
+						    DWMCI_FIFO_MASK;
+					len = min(size, len);
+					for (i = 0; i < len; i++)
+						*buf++ =
+						dwmci_readl(host, DWMCI_DATA);
+					size = size > len ? (size - len) : 0;
+				}
+			} else if (data->flags == MMC_DATA_WRITE &&
+				   (mask & DWMCI_INTMSK_TXDR)) {
+				while (size) {
+					ret = dwmci_fifo_ready(host,
+							DWMCI_FIFO_FULL,
+							&len);
+					if (ret < 0)
+						break;
+
+					len = host->fifo_depth - ((len >>
+						   DWMCI_FIFO_SHIFT) &
+						   DWMCI_FIFO_MASK);
+					len = min(size, len);
+					for (i = 0; i < len; i++)
+						dwmci_writel(host, DWMCI_DATA,
+							     *buf++);
+					size = size > len ? (size - len) : 0;
+				}
+				dwmci_writel(host, DWMCI_RINTSTS,
+					     DWMCI_INTMSK_TXDR);
+			}
+		}
 
 		/* Data arrived correctly */
 		if (mask & DWMCI_INTMSK_DTO) {
-- 
2.46.2



More information about the U-Boot mailing list