From: Bin Meng <bin.meng@windriver.com>
Date: Tue, 2 Feb 2021 02:48:47 +0000 (+0800)
Subject: mmc: mmc_spi: Fix potential spec violation in receiving card response
X-Git-Url: http://git.dujemihanovic.xyz/img/static/gitweb.css?a=commitdiff_plain;h=2f22cb40e57f28b694c05c0b631cbb0c09eb03a4;p=u-boot.git

mmc: mmc_spi: Fix potential spec violation in receiving card response

After command is sent and before card response shows up on the line,
there is a variable number of clock cycles in between called Ncr.
The spec [1] says the minimum is 1 byte and the maximum is 8 bytes.

Current logic in mmc_spi_sendcmd() has a flaw that it could only work
with certain SD cards with their Ncr being just 1 byte.

When resp_match is false, the codes try to receive only 1 byte from
the SD card. On the other hand when resp_match is true, the logic
happens to be no problem as it loops until timeout to receive as many
bytes as possible to see a match of the expected resp_match_value.
However not every call to mmc_spi_sendcmd() is made with resp_match
being true hence this exposes a potential issue with SD cards that
have a larger Ncr value.

Given no issue was reported as of today, we can reasonably conclude
that all cards being used on the supported boards happen to have a 1
byte Ncr timing requirement. But a broken case can be triggered by
utilizing QEMU to emulate a larger value of Ncr (by default 1 byte
Ncr is used on QEMU). This commit fixes such potential spec violation
to improve the card compatibility.

[1] "Physical Layer Specification Version 8.00"
     chapter 7.5.1: Command / Response
     chapter 7.5.4: Timing Values

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
---

diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c
index a06862aa48..fbdbcf73ff 100644
--- a/drivers/mmc/mmc_spi.c
+++ b/drivers/mmc/mmc_spi.c
@@ -103,29 +103,31 @@ static int mmc_spi_sendcmd(struct udevice *dev,
 
 	debug("%s: cmd%d", __func__, cmdidx);
 
-	if (resp_match) {
+	if (resp_match)
 		r = ~resp_match_value;
-		i = CMD_TIMEOUT;
-		while (i) {
-			ret = dm_spi_xfer(dev, 1 * 8, NULL, &r, 0);
-			if (ret)
-				return ret;
-			debug(" resp%d=0x%x", rpos, r);
-			rpos++;
-			i--;
+	i = CMD_TIMEOUT;
+	while (i) {
+		ret = dm_spi_xfer(dev, 1 * 8, NULL, &r, 0);
+		if (ret)
+			return ret;
+		debug(" resp%d=0x%x", rpos, r);
+		rpos++;
+		i--;
 
+		if (resp_match) {
 			if (r == resp_match_value)
 				break;
+		} else {
+			if (!(r & 0x80))
+				break;
 		}
-		if (!i && (r != resp_match_value))
+
+		if (!i)
 			return -ETIMEDOUT;
 	}
 
-	for (i = 0; i < resp_size; i++) {
-		if (i == 0 && resp_match) {
-			resp[i] = resp_match_value;
-			continue;
-		}
+	resp[0] = r;
+	for (i = 1; i < resp_size; i++) {
 		ret = dm_spi_xfer(dev, 1 * 8, NULL, &r, 0);
 		if (ret)
 			return ret;