From 01962f8d7ced3d8b9748d3b902f7f9b68587c426 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Tue, 2 Feb 2021 10:32:48 +0800 Subject: mmc: mmc_spi: Print verbose debug output when crc16 check fails Add some verbose debug output when crc16 check fails. Signed-off-by: Bin Meng Reviewed-by: Jaehoon Chung --- drivers/mmc/mmc_spi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers/mmc/mmc_spi.c') diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c index 46800bbed2..23b907328c 100644 --- a/drivers/mmc/mmc_spi.c +++ b/drivers/mmc/mmc_spi.c @@ -181,8 +181,10 @@ static int mmc_spi_readdata(struct udevice *dev, if (ret) return ret; #ifdef CONFIG_MMC_SPI_CRC_ON - if (be16_to_cpu(crc16_ccitt(0, buf, bsize)) != crc) { - debug("%s: data crc error\n", __func__); + u16 crc_ok = be16_to_cpu(crc16_ccitt(0, buf, bsize)); + if (crc_ok != crc) { + debug("%s: data crc error, expected %04x got %04x\n", + __func__, crc_ok, crc); r1 = R1_SPI_COM_CRC; break; } -- cgit v1.2.3 From 781aad0de93aedf94a17cc66b2f211bf156b718e Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Tue, 2 Feb 2021 10:48:46 +0800 Subject: mmc: mmc_spi: Move argument check to the beginning of mmc_spi_sendcmd() The argument check should happen before any transfer on the SPI lines. Signed-off-by: Bin Meng Reviewed-by: Jaehoon Chung --- drivers/mmc/mmc_spi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/mmc/mmc_spi.c') diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c index 23b907328c..a06862aa48 100644 --- a/drivers/mmc/mmc_spi.c +++ b/drivers/mmc/mmc_spi.c @@ -78,6 +78,9 @@ static int mmc_spi_sendcmd(struct udevice *dev, int i, rpos = 0, ret = 0; u8 cmdo[7], r; + if (!resp || !resp_size) + return 0; + debug("%s: cmd%d cmdarg=0x%x resp_type=0x%x " "resp_size=%d resp_match=%d resp_match_value=0x%x\n", __func__, cmdidx, cmdarg, resp_type, @@ -98,9 +101,6 @@ static int mmc_spi_sendcmd(struct udevice *dev, if (ret) return ret; - if (!resp || !resp_size) - return 0; - debug("%s: cmd%d", __func__, cmdidx); if (resp_match) { -- cgit v1.2.3 From 2f22cb40e57f28b694c05c0b631cbb0c09eb03a4 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Tue, 2 Feb 2021 10:48:47 +0800 Subject: 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 Reviewed-by: Jaehoon Chung --- drivers/mmc/mmc_spi.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) (limited to 'drivers/mmc/mmc_spi.c') 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; -- cgit v1.2.3 From 46938abd2c4c2b605c5af9483fff20ef6772eafc Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Tue, 2 Feb 2021 10:48:48 +0800 Subject: mmc: mmc_spi: Document the 3 local functions mmc_spi_sendcmd(), mmc_spi_readdata() and mmc_spi_writedata() are currently undocumented. Add comment blocks to explain the arguments and the return value. Signed-off-by: Bin Meng Reviewed-by: Jaehoon Chung --- drivers/mmc/mmc_spi.c | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) (limited to 'drivers/mmc/mmc_spi.c') diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c index fbdbcf73ff..e2d78794f2 100644 --- a/drivers/mmc/mmc_spi.c +++ b/drivers/mmc/mmc_spi.c @@ -37,7 +37,8 @@ #define SPI_RESPONSE_CRC_ERR ((5 << 1)|1) #define SPI_RESPONSE_WRITE_ERR ((6 << 1)|1) -/* Read and write blocks start with these tokens and end with crc; +/* + * Read and write blocks start with these tokens and end with crc; * on error, read tokens act like a subset of R2_SPI_* values. */ /* single block write multiblock read */ @@ -70,6 +71,20 @@ struct mmc_spi_priv { struct spi_slave *spi; }; +/** + * mmc_spi_sendcmd() - send a command to the SD card + * + * @dev: mmc_spi device + * @cmdidx: command index + * @cmdarg: command argument + * @resp_type: card response type + * @resp: buffer to store the card response + * @resp_size: size of the card response + * @resp_match: if true, compare each of received bytes with @resp_match_value + * @resp_match_value: a value to be compared with each of received bytes + * @r1b: if true, receive additional bytes for busy signal token + * @return 0 if OK, -ETIMEDOUT if no card response is received, -ve on error + */ static int mmc_spi_sendcmd(struct udevice *dev, ushort cmdidx, u32 cmdarg, u32 resp_type, u8 *resp, u32 resp_size, @@ -159,6 +174,15 @@ static int mmc_spi_sendcmd(struct udevice *dev, return 0; } +/** + * mmc_spi_readdata() - read data block(s) from the SD card + * + * @dev: mmc_spi device + * @xbuf: buffer of the actual data (excluding token and crc) to read + * @bcnt: number of data blocks to transfer + * @bsize: size of the actual data (excluding token and crc) in bytes + * @return 0 if OK, -ECOMM if crc error, -ETIMEDOUT on other errors + */ static int mmc_spi_readdata(struct udevice *dev, void *xbuf, u32 bcnt, u32 bsize) { @@ -207,6 +231,16 @@ static int mmc_spi_readdata(struct udevice *dev, return ret; } +/** + * mmc_spi_writedata() - write data block(s) to the SD card + * + * @dev: mmc_spi device + * @xbuf: buffer of the actual data (excluding token and crc) to write + * @bcnt: number of data blocks to transfer + * @bsize: size of actual data (excluding token and crc) in bytes + * @multi: indicate a transfer by multiple block write command (CMD25) + * @return 0 if OK, -ECOMM if crc error, -ETIMEDOUT on other errors + */ static int mmc_spi_writedata(struct udevice *dev, const void *xbuf, u32 bcnt, u32 bsize, int multi) { -- cgit v1.2.3