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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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.boddu@xilinx.com>
>>
>> MMC cards support different tuning sequence for entering HS200 mode.
>>
>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>> [ clg: - ported on QEMU 7.0 ]
>> Signed-off-by: Cédric Le Goater <clg@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 ++];
>