sdmmc: don’t flip word order in MMC_RSP_BITS

MMC_RSP_BITS helper function had a hack that it flipped word order in
the response, assuming that response size is 4 words. This hack does not
work for responses which are not 4 words long (such as the SWITCH_FUNC
response, which is 64 words long).

This change removes the hack and the matching word order reversal code
in sdmmc driver.

diff --git a/components/driver/include/driver/sdmmc_defs.h b/components/driver/include/driver/sdmmc_defs.h
index 455d1c7aff6ffeb320ef6c8fe63c6d7ea1fb3e9f..30df7e2fc051a3776829f8237c839d9846074a29 100644 (file)
--- a/components/driver/include/driver/sdmmc_defs.h
+++ b/components/driver/include/driver/sdmmc_defs.h
@@ -304,13 +304,38 @@
 #define SD_ACCESS_MODE_SDR104   3
 #define SD_ACCESS_MODE_DDR50    4
 
+/**
+ * @brief Extract up to 32 sequential bits from an array of 32-bit words
+ *
+ * Bits within the word are numbered in the increasing order from LSB to MSB.
+ *
+ * As an example, consider 2 32-bit words:
+ *
+ * 0x01234567 0x89abcdef
+ *
+ * On a little-endian system, the bytes are stored in memory as follows:
+ *
+ * 67 45 23 01 ef cd ab 89
+ *
+ * MMC_RSP_BITS will extact bits as follows:
+ *
+ * start=0  len=4   -> result=0x00000007
+ * start=0  len=12  -> result=0x00000567
+ * start=28 len=8   -> result=0x000000f0
+ * start=59 len=5   -> result=0x00000011
+ *
+ * @param src array of words to extract bits from
+ * @param start index of the first bit to extract
+ * @param len number of bits to extract, 1 to 32
+ * @return 32-bit word where requested bits start from LSB
+ */
 static inline uint32_t MMC_RSP_BITS(uint32_t *src, int start, int len)
 {
     uint32_t mask = (len % 32 == 0) ? UINT_MAX : UINT_MAX >> (32 - (len % 32));
-    size_t word = 3 - start / 32;
+    size_t word = start / 32;
     size_t shift = start % 32;
     uint32_t right = src[word] >> shift;
-    uint32_t left = (len + shift <= 32) ? 0 : src[word - 1] << ((32 - shift) % 32);
+    uint32_t left = (len + shift <= 32) ? 0 : src[word + 1] << ((32 - shift) % 32);
     return (left | right) & mask;
 }
 

diff --git a/components/driver/sdmmc_transaction.c b/components/driver/sdmmc_transaction.c
index c3a5827544f29223f9a2d895f5ff1a82dbf63a0d..3cc5ee5dae26b35ae24e422c7a72f4b9a2afcde4 100644 (file)
--- a/components/driver/sdmmc_transaction.c
+++ b/components/driver/sdmmc_transaction.c
@@ -261,11 +261,8 @@ static void process_command_response(uint32_t status, sdmmc_command_t* cmd)
 {
     if (cmd->flags & SCF_RSP_PRESENT) {
         if (cmd->flags & SCF_RSP_136) {
-            cmd->response[3] = SDMMC.resp[0];
-            cmd->response[2] = SDMMC.resp[1];
-            cmd->response[1] = SDMMC.resp[2];
-            cmd->response[0] = SDMMC.resp[3];
-
+            /* Destination is 4-byte aligned, can memcopy from peripheral registers */
+            memcpy(cmd->response, (uint32_t*) SDMMC.resp, 4 * sizeof(uint32_t));
         } else {
             cmd->response[0] = SDMMC.resp[0];
             cmd->response[1] = 0;

diff --git a/components/sdmmc/sdmmc_cmd.c b/components/sdmmc/sdmmc_cmd.c
index 0784a279a159d8967bd4eda33904f4e2f1476cde..65363d7d7e09891a484b4cdd6b1e7b3815c26bb5 100644 (file)
--- a/components/sdmmc/sdmmc_cmd.c
+++ b/components/sdmmc/sdmmc_cmd.c
@@ -51,7 +51,7 @@ static esp_err_t sdmmc_send_cmd_stop_transmission(sdmmc_card_t* card, uint32_t*
 static esp_err_t sdmmc_send_cmd_send_status(sdmmc_card_t* card, uint32_t* out_status);
 static esp_err_t sdmmc_send_cmd_crc_on_off(sdmmc_card_t* card, bool crc_enable);
 static uint32_t  get_host_ocr(float voltage);
-static void response_ntoh(sdmmc_response_t response);
+static void flip_byte_order(uint32_t* response, size_t size);
 static esp_err_t sdmmc_write_sectors_dma(sdmmc_card_t* card, const void* src,
         size_t start_block, size_t block_count);
 static esp_err_t sdmmc_read_sectors_dma(sdmmc_card_t* card, void* dst,
@@ -419,7 +419,7 @@ static esp_err_t sdmmc_send_cmd_send_cid(sdmmc_card_t *card, sdmmc_cid_t *out_ci
     if (err != ESP_OK) {
         return err;
     }
-    response_ntoh(buf);
+    flip_byte_order(buf, sizeof(buf));
     return sdmmc_decode_cid(buf, out_cid);
 }
 
@@ -501,10 +501,12 @@ static esp_err_t sdmmc_send_cmd_send_csd(sdmmc_card_t* card, sdmmc_csd_t* out_cs
     if (err != ESP_OK) {
         return err;
     }
+    uint32_t* ptr = cmd.response;
     if (is_spi) {
-        response_ntoh(spi_buf);
+        flip_byte_order(spi_buf,  sizeof(spi_buf));
+        ptr = spi_buf;
     }
-    return sdmmc_decode_csd(is_spi ? spi_buf : cmd.response, out_csd);
+    return sdmmc_decode_csd(ptr, out_csd);
 }
 
 static esp_err_t sdmmc_send_cmd_select_card(sdmmc_card_t* card)
@@ -520,8 +522,8 @@ static esp_err_t sdmmc_send_cmd_select_card(sdmmc_card_t* card)
 static esp_err_t sdmmc_decode_scr(uint32_t *raw_scr, sdmmc_scr_t* out_scr)
 {
     sdmmc_response_t resp = {0xabababab, 0xabababab, 0x12345678, 0x09abcdef};
-    resp[2] = __builtin_bswap32(raw_scr[0]);
-    resp[3] = __builtin_bswap32(raw_scr[1]);
+    resp[1] = __builtin_bswap32(raw_scr[0]);
+    resp[0] = __builtin_bswap32(raw_scr[1]);
     int ver = SCR_STRUCTURE(resp);
     if (ver != 0) {
         return ESP_ERR_NOT_SUPPORTED;
@@ -597,10 +599,15 @@ static uint32_t get_host_ocr(float voltage)
     return SD_OCR_VOL_MASK;
 }
 
-static void response_ntoh(sdmmc_response_t response)
+static void flip_byte_order(uint32_t* response, size_t size)
 {
-    for (int i = 0; i < 4; ++i) {
-        response[i] = __builtin_bswap32(response[i]);
+    assert(size % (2 * sizeof(uint32_t)) == 0);
+    const size_t n_words = size / sizeof(uint32_t);
+    for (int i = 0; i < n_words / 2; ++i) {
+        uint32_t left = __builtin_bswap32(response[i]);
+        uint32_t right = __builtin_bswap32(response[n_words - i - 1]);
+        response[i] = right;
+        response[n_words - i - 1] = left;
     }
 }
 

diff --git a/components/sdmmc/test/test_sd.c b/components/sdmmc/test/test_sd.c
index 86ff501b855a87b1dc29a6de84c950c324d48246..5602bd608449d045e3ddbcb65eb8be8baab2fdde 100644 (file)
--- a/components/sdmmc/test/test_sd.c
+++ b/components/sdmmc/test/test_sd.c
@@ -26,6 +26,15 @@
 #include <time.h>
 #include <sys/time.h>
 
+TEST_CASE("MMC_RSP_BITS", "[sd]")
+{
+    uint32_t data[2] = { 0x01234567, 0x89abcdef };
+    TEST_ASSERT_EQUAL_HEX32(0x7,   MMC_RSP_BITS(data, 0, 4));
+    TEST_ASSERT_EQUAL_HEX32(0x567, MMC_RSP_BITS(data, 0, 12));
+    TEST_ASSERT_EQUAL_HEX32(0xf0,  MMC_RSP_BITS(data, 28, 8));
+    TEST_ASSERT_EQUAL_HEX32(0x3,   MMC_RSP_BITS(data, 1, 3));
+    TEST_ASSERT_EQUAL_HEX32(0x11,  MMC_RSP_BITS(data, 59, 5));
+}
 
 TEST_CASE("can probe SD", "[sd][ignore]")
 {