Re: [PATCH 20/32] hw/sd: Add CMD21 tuning sequence

[Philippe Mathieu-Daudé]

On 13/6/24 00:15, Philippe Mathieu-Daudé wrote:

On 3/7/23 15:24, Cédric Le Goater wrote:

From: Sai Pavan Boddu <sai.pavan.bo...@xilinx.com>

MMC cards support different tuning sequence for entering HS200 mode.

Signed-off-by: Sai Pavan Boddu <sai.pavan.bo...@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
[ clg: - ported on QEMU 7.0 ]
Signed-off-by: Cédric Le Goater <c...@kaod.org>
---
  hw/sd/sd.c | 40 ++++++++++++++++++++++++++++++++++++++++
  1 file changed, 40 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 4b4a4cda2e68..7332f7a18435 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c

@@ -2017,6 +2017,30 @@ static const uint8_t sd_tuning_block_pattern[SD_TUNING_BLOCK_SIZE] = {

      0xbb, 0xff, 0xf7, 0xff,         0xf7, 0x7f, 0x7b, 0xde,
  };
+#define EXCSD_BUS_WIDTH_OFFSET 183
+#define BUS_WIDTH_8_MASK    0x4
+#define BUS_WIDTH_4_MASK    0x2
+#define MMC_TUNING_BLOCK_SIZE   128
+
+static const uint8_t mmc_tuning_block_pattern[128] = {
+       0xff, 0xff, 0x00, 0xff, 0xff, 0xff, 0x00, 0x00,
+       0xff, 0xff, 0xcc, 0xcc, 0xcc, 0x33, 0xcc, 0xcc,
+       0xcc, 0x33, 0x33, 0xcc, 0xcc, 0xcc, 0xff, 0xff,
+       0xff, 0xee, 0xff, 0xff, 0xff, 0xee, 0xee, 0xff,
+       0xff, 0xff, 0xdd, 0xff, 0xff, 0xff, 0xdd, 0xdd,
+       0xff, 0xff, 0xff, 0xbb, 0xff, 0xff, 0xff, 0xbb,
+       0xbb, 0xff, 0xff, 0xff, 0x77, 0xff, 0xff, 0xff,
+       0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee, 0xff,
+       0xff, 0xff, 0xff, 0x00, 0xff, 0xff, 0xff, 0x00,
+       0x00, 0xff, 0xff, 0xcc, 0xcc, 0xcc, 0x33, 0xcc,
+       0xcc, 0xcc, 0x33, 0x33, 0xcc, 0xcc, 0xcc, 0xff,
+       0xff, 0xff, 0xee, 0xff, 0xff, 0xff, 0xee, 0xee,
+       0xff, 0xff, 0xff, 0xdd, 0xff, 0xff, 0xff, 0xdd,
+       0xdd, 0xff, 0xff, 0xff, 0xbb, 0xff, 0xff, 0xff,
+       0xbb, 0xbb, 0xff, 0xff, 0xff, 0x77, 0xff, 0xff,
+       0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
+};
+
  uint8_t sd_read_byte(SDState *sd)
  {
      /* TODO: Append CRCs */
@@ -2103,6 +2127,22 @@ uint8_t sd_read_byte(SDState *sd)
          ret = sd_tuning_block_pattern[sd->data_offset++];
          break;
+    case 21:    /* CMD21: SEND_TUNING_BLOCK (MMC) */

This can be accessed in SPI/SD modes, should we check for eMMC then?

This could do:

-- >8 --
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 3c12ba2ad3..5bad19c766 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -160,12 +160,18 @@ static const struct SDProto *sd_proto(SDState *sd)
 }

 static const SDProto sd_proto_spi;
+static const SDProto sd_proto_emmc;

 static bool sd_is_spi(SDState *sd)
 {
     return sd_proto(sd) == &sd_proto_spi;
 }

+static bool sd_is_emmc(SDState *sd)
+{
+    return sd_proto(sd) == &sd_proto_emmc;
+}
+
 static const char *sd_version_str(enum SDPhySpecificationVersion version)
 {
     static const char *sdphy_version[] = {
@@ -2389,7 +2395,9 @@ uint8_t sd_read_byte(SDState *sd)
         break;

     case 21:    /* CMD21: SEND_TUNING_BLOCK (MMC) */
+        if (!sd_is_emmc(sd)) {
+            return 0x00;
+        }
         if (sd->data_offset >= MMC_TUNING_BLOCK_SIZE - 1) {
             sd->state = sd_transfer_state;
         }
---

Similarly, other cases previous eMMC introduction only expect SPI/SD
but don't check for it. I need to think a bit more on how to handle
that.

+        if (sd->data_offset >= MMC_TUNING_BLOCK_SIZE - 1) {
+            sd->state = sd_transfer_state;
+        }
+        if (sd->ext_csd[EXCSD_BUS_WIDTH_OFFSET] & BUS_WIDTH_8_MASK) {
+            ret = mmc_tuning_block_pattern[sd->data_offset++];
+        } else {
+            /*
+             * Return LSB Nibbles of two byte from the 8bit tuning
+             * block for 4bit mode
+             */
+            ret = mmc_tuning_block_pattern[sd->data_offset++] & 0x0F;

+            ret |= (mmc_tuning_block_pattern[sd->data_offset++] & 0x0F) << 4;

+        }
+        break;
+
      case 22:  /* ACMD22: SEND_NUM_WR_BLOCKS */
          ret = sd->data[sd->data_offset ++];