[PATCH 00/32] hw/sd: eMMC support

[PATCH 00/32] hw/sd: eMMC support

[Cédric Le Goater]

Hello,

  
This series adds an extension for a new eMMC device using the
framework Philippe put in place to support various SD implementations.
Previous discussion on the same topic:

  http://patchwork.ozlabs.org/project/qemu-devel/list/?series=250563
  https://lore.kernel.org/qemu-devel/20220318132824.1134400-1-clg@kaod.org/

patch 1-12
  - introduce SDProto structure
  - could be merged. They have been reviewed.
  
patch 13
  - adds a SPI variant model
  
patch 14-27
  - adds eMMC support
  - need better commit logs
  
patch 28-32 (for later)

  - aspeed wiring

Please comment the core part, we can leave out the aspeed part for
now. I won't have much time to fix the issues unless it's about
compile and style issues. If someone is interested and could take
over the series, that would be nice.

Thanks,

C.

Cédric Le Goater (11):
  hw/sd: Introduce a "sd-card" SPI variant model
  hw/sd: Add emmc_cmd_SEND_OP_CMD() handler
  hw/sd: Add emmc_cmd_ALL_SEND_CID() handler
  hw/sd: Add emmc_cmd_SEND_RELATIVE_ADDR() handler
  hw/sd: Add emmc_cmd_APP_CMD() handler
  hw/sd: add emmc_cmd_SEND_TUNING_BLOCK() handler
  hw/sd: Add emmc_cmd_SEND_EXT_CSD() handler
  hw/sd: Fix SET_BLOCK_COUNT command argument
  hw/arm/aspeed: Add eMMC device
  hw/arm/aspeed: Load eMMC first boot area as a boot rom
  aspeed: Introduce a 'boot-emmc' property for AST2600 based machines

Joel Stanley (6):
  hw/sd: Add sd_cmd_SEND_TUNING_BLOCK() handler
  hw/sd: Support boot area in emmc image
  hw/sd: Subtract bootarea size from blk
  hw/sd: Add boot config support
  hw/arm/aspeed: Set boot device to emmc
  aspeed: Set bootconfig

Philippe Mathieu-Daudé (12):
  hw/sd: When card is in wrong state, log which state it is
  hw/sd: When card is in wrong state, log which spec version is used
  hw/sd: Move proto_name to SDProto structure
  hw/sd: Introduce sd_cmd_handler type
  hw/sd: Add sd_cmd_illegal() handler
  hw/sd: Add sd_cmd_unimplemented() handler
  hw/sd: Add sd_cmd_GO_IDLE_STATE() handler
  hw/sd: Add sd_cmd_SEND_OP_CMD() handler
  hw/sd: Add sd_cmd_ALL_SEND_CID() handler
  hw/sd: Add sd_cmd_SEND_RELATIVE_ADDR() handler
  hw/sd: Add sd_cmd_SET_BLOCK_COUNT() handler
  hw/sd: Basis for eMMC support

Sai Pavan Boddu (3):
  hw/sd: Add CMD21 tuning sequence
  hw/sd: Add mmc switch function support
  hw/sd: Update CMD1 definition for MMC

 docs/system/arm/aspeed.rst   |   2 +
 hw/sd/sdmmc-internal.h       |  97 +++++
 include/hw/arm/aspeed_soc.h  |   1 +
 include/hw/misc/aspeed_scu.h |   7 +
 include/hw/sd/sd.h           |  10 +
 hw/arm/aspeed.c              |  68 +++-
 hw/arm/aspeed_ast2600.c      |   1 +
 hw/arm/stellaris.c           |   3 +-
 hw/riscv/sifive_u.c          |   3 +-
 hw/sd/sd.c                   | 702 ++++++++++++++++++++++++++++-------
 hw/sd/sdmmc-internal.c       |   2 +-
 11 files changed, 748 insertions(+), 148 deletions(-)

-- 
2.41.0

[PATCH 01/32] hw/sd: When card is in wrong state, log which state it is

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

We report the card is in an inconsistent state, but don't precise
in which state it is. Add this information, as it is useful when
debugging problems.

Since we will reuse this code, extract as sd_invalid_state_for_cmd()
helper.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210624142209.1193073-2-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 0be9ee965ac9..4412559c05be 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -966,6 +966,14 @@ static bool address_in_range(SDState *sd, const char *desc,
     return true;
 }
 
+static sd_rsp_type_t sd_invalid_state_for_cmd(SDState *sd, SDRequest req)
+{
+    qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state: %s\n",
+                  req.cmd, sd_state_name(sd->state));
+
+    return sd_illegal;
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1534,9 +1542,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         return sd_illegal;
     }
 
-    qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state: %s\n",
-                  req.cmd, sd_state_name(sd->state));
-    return sd_illegal;
+    return sd_invalid_state_for_cmd(sd, req);
 }
 
 static sd_rsp_type_t sd_app_command(SDState *sd,
-- 
2.41.0

[PATCH 02/32] hw/sd: When card is in wrong state, log which spec version is used

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Add the sd_version_str() helper.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 4412559c05be..20e62aff70b6 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -145,6 +145,19 @@ struct SDState {
 
 static void sd_realize(DeviceState *dev, Error **errp);
 
+static const char *sd_version_str(enum SDPhySpecificationVersion version)
+{
+    static const char *sdphy_version[] = {
+        [SD_PHY_SPECv1_10_VERS]     = "v1.10",
+        [SD_PHY_SPECv2_00_VERS]     = "v2.00",
+        [SD_PHY_SPECv3_01_VERS]     = "v3.01",
+    };
+    if (version >= ARRAY_SIZE(sdphy_version)) {
+        return "unsupported version";
+    }
+    return sdphy_version[version];
+}
+
 static const char *sd_state_name(enum SDCardStates state)
 {
     static const char *state_name[] = {
@@ -968,8 +981,9 @@ static bool address_in_range(SDState *sd, const char *desc,
 
 static sd_rsp_type_t sd_invalid_state_for_cmd(SDState *sd, SDRequest req)
 {
-    qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state: %s\n",
-                  req.cmd, sd_state_name(sd->state));
+    qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state: %s (spec %s)\n",
+                  req.cmd, sd_state_name(sd->state),
+                  sd_version_str(sd->spec_version));
 
     return sd_illegal;
 }
-- 
2.41.0

[PATCH 03/32] hw/sd: Move proto_name to SDProto structure

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Introduce a new structure to hold the bus protocol specific
fields: SDProto. The first field is the protocol name.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210624142209.1193073-4-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/sd/sd.h |  2 ++
 hw/sd/sd.c         | 35 +++++++++++++++++++++++++++--------
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 3047adb2fc86..b322d8f19b17 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -124,6 +124,8 @@ struct SDCardClass {
     void (*enable)(SDState *sd, bool enable);
     bool (*get_inserted)(SDState *sd);
     bool (*get_readonly)(SDState *sd);
+
+    const struct SDProto *proto;
 };
 
 #define TYPE_SD_BUS "sd-bus"
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 20e62aff70b6..f6aa3b0a80bf 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -87,6 +87,10 @@ enum SDCardStates {
     sd_disconnect_state,
 };
 
+typedef struct SDProto {
+    const char *name;
+} SDProto;
+
 struct SDState {
     DeviceState parent_obj;
 
@@ -137,7 +141,6 @@ struct SDState {
     qemu_irq readonly_cb;
     qemu_irq inserted_cb;
     QEMUTimer *ocr_power_timer;
-    const char *proto_name;
     bool enable;
     uint8_t dat_lines;
     bool cmd_line;
@@ -145,6 +148,13 @@ struct SDState {
 
 static void sd_realize(DeviceState *dev, Error **errp);
 
+static const struct SDProto *sd_proto(SDState *sd)
+{
+    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
+
+    return sc->proto;
+}
+
 static const char *sd_version_str(enum SDPhySpecificationVersion version)
 {
     static const char *sdphy_version[] = {
@@ -981,8 +991,8 @@ static bool address_in_range(SDState *sd, const char *desc,
 
 static sd_rsp_type_t sd_invalid_state_for_cmd(SDState *sd, SDRequest req)
 {
-    qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state: %s (spec %s)\n",
-                  req.cmd, sd_state_name(sd->state),
+    qemu_log_mask(LOG_GUEST_ERROR, "%s: CMD%i in a wrong state: %s (spec %s)\n",
+                  sd_proto(sd)->name, req.cmd, sd_state_name(sd->state),
                   sd_version_str(sd->spec_version));
 
     return sd_illegal;
@@ -997,7 +1007,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
      * However there is no ACMD55, so we want to trace this particular case.
      */
     if (req.cmd != 55 || sd->expecting_acmd) {
-        trace_sdcard_normal_command(sd->proto_name,
+        trace_sdcard_normal_command(sd_proto(sd)->name,
                                     sd_cmd_name(req.cmd), req.cmd,
                                     req.arg, sd_state_name(sd->state));
     }
@@ -1562,7 +1572,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 static sd_rsp_type_t sd_app_command(SDState *sd,
                                     SDRequest req)
 {
-    trace_sdcard_app_command(sd->proto_name, sd_acmd_name(req.cmd),
+    trace_sdcard_app_command(sd_proto(sd)->name, sd_acmd_name(req.cmd),
                              req.cmd, req.arg, sd_state_name(sd->state));
     sd->card_status |= APP_CMD;
     switch (req.cmd) {
@@ -1856,7 +1866,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
     if (sd->card_status & (ADDRESS_ERROR | WP_VIOLATION))
         return;
 
-    trace_sdcard_write_data(sd->proto_name,
+    trace_sdcard_write_data(sd_proto(sd)->name,
                             sd_acmd_name(sd->current_cmd),
                             sd->current_cmd, value);
     switch (sd->current_cmd) {
@@ -2012,7 +2022,7 @@ uint8_t sd_read_byte(SDState *sd)
 
     io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;
 
-    trace_sdcard_read_data(sd->proto_name,
+    trace_sdcard_read_data(sd_proto(sd)->name,
                            sd_acmd_name(sd->current_cmd),
                            sd->current_cmd, io_len);
     switch (sd->current_cmd) {
@@ -2131,6 +2141,14 @@ void sd_enable(SDState *sd, bool enable)
     sd->enable = enable;
 }
 
+static const SDProto sd_proto_spi = {
+    .name = "SPI",
+};
+
+static const SDProto sd_proto_sd = {
+    .name = "SD",
+};
+
 static void sd_instance_init(Object *obj)
 {
     SDState *sd = SD_CARD(obj);
@@ -2149,9 +2167,10 @@ static void sd_instance_finalize(Object *obj)
 static void sd_realize(DeviceState *dev, Error **errp)
 {
     SDState *sd = SD_CARD(dev);
+    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
     int ret;
 
-    sd->proto_name = sd->spi ? "SPI" : "SD";
+    sc->proto = sd->spi ? &sd_proto_spi : &sd_proto_sd;
 
     switch (sd->spec_version) {
     case SD_PHY_SPECv1_10_VERS
-- 
2.41.0

[PATCH 04/32] hw/sd: Introduce sd_cmd_handler type

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Add 2 command handler arrays in SDProto, for CMD and ACMD.
Have sd_normal_command() / sd_app_command() use these arrays:
if an command handler is registered, call it, otherwise fall
back to current code base.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210624142209.1193073-5-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index f6aa3b0a80bf..889c44a5c07c 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -87,8 +87,12 @@ enum SDCardStates {
     sd_disconnect_state,
 };
 
+typedef sd_rsp_type_t (*sd_cmd_handler)(SDState *sd, SDRequest req);
+
 typedef struct SDProto {
     const char *name;
+    sd_cmd_handler cmd[SDMMC_CMD_MAX];
+    sd_cmd_handler acmd[SDMMC_CMD_MAX];
 } SDProto;
 
 struct SDState {
@@ -1031,6 +1035,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         return sd_illegal;
     }
 
+    if (sd_proto(sd)->cmd[req.cmd]) {
+        return sd_proto(sd)->cmd[req.cmd](sd, req);
+    }
+
     switch (req.cmd) {
     /* Basic commands (Class 0 and Class 1) */
     case 0:  /* CMD0:   GO_IDLE_STATE */
@@ -1575,6 +1583,11 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
     trace_sdcard_app_command(sd_proto(sd)->name, sd_acmd_name(req.cmd),
                              req.cmd, req.arg, sd_state_name(sd->state));
     sd->card_status |= APP_CMD;
+
+    if (sd_proto(sd)->acmd[req.cmd]) {
+        return sd_proto(sd)->acmd[req.cmd](sd, req);
+    }
+
     switch (req.cmd) {
     case 6:  /* ACMD6:  SET_BUS_WIDTH */
         if (sd->spi) {
-- 
2.41.0

[PATCH 05/32] hw/sd: Add sd_cmd_illegal() handler

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Log illegal commands as GUEST_ERROR.

Note: we are logging back the SDIO commands (CMD5, CMD52-54).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210624142209.1193073-6-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 62 +++++++++++++++++++++++-------------------------------
 1 file changed, 26 insertions(+), 36 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 889c44a5c07c..83a0d6825147 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1002,6 +1002,15 @@ static sd_rsp_type_t sd_invalid_state_for_cmd(SDState *sd, SDRequest req)
     return sd_illegal;
 }
 
+static sd_rsp_type_t sd_cmd_illegal(SDState *sd, SDRequest req)
+{
+    qemu_log_mask(LOG_GUEST_ERROR, "%s: Unknown CMD%i for spec %s\n",
+                  sd_proto(sd)->name, req.cmd,
+                  sd_version_str(sd->spec_version));
+
+    return sd_illegal;
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1054,15 +1063,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 1:  /* CMD1:   SEND_OP_CMD */
-        if (!sd->spi)
-            goto bad_cmd;
-
         sd->state = sd_transfer_state;
         return sd_r1;
 
     case 2:  /* CMD2:   ALL_SEND_CID */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->state) {
         case sd_ready_state:
             sd->state = sd_identification_state;
@@ -1074,8 +1078,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 3:  /* CMD3:   SEND_RELATIVE_ADDR */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->state) {
         case sd_identification_state:
         case sd_standby_state:
@@ -1089,8 +1091,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 4:  /* CMD4:   SEND_DSR */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->state) {
         case sd_standby_state:
             break;
@@ -1100,9 +1100,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 5: /* CMD5: reserved for SDIO cards */
-        return sd_illegal;
-
     case 6:  /* CMD6:   SWITCH_FUNCTION */
         switch (sd->mode) {
         case sd_data_transfer_mode:
@@ -1118,8 +1115,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 7:  /* CMD7:   SELECT/DESELECT_CARD */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->state) {
         case sd_standby_state:
             if (sd->rca != rca)
@@ -1249,8 +1244,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 15:  /* CMD15:  GO_INACTIVE_STATE */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->mode) {
         case sd_data_transfer_mode:
             if (sd->rca != rca)
@@ -1303,7 +1296,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     case 19:    /* CMD19: SEND_TUNING_BLOCK (SD) */
         if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
-            goto bad_cmd;
+            return sd_invalid_state_for_cmd(sd, req);
         }
         if (sd->state == sd_transfer_state) {
             sd->state = sd_sendingdata_state;
@@ -1314,7 +1307,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     case 23:    /* CMD23: SET_BLOCK_COUNT */
         if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
-            goto bad_cmd;
+            return sd_invalid_state_for_cmd(sd, req);
         }
         switch (sd->state) {
         case sd_transfer_state:
@@ -1357,8 +1350,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 26:  /* CMD26:  PROGRAM_CID */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->state) {
         case sd_transfer_state:
             sd->state = sd_receivingdata_state;
@@ -1508,15 +1499,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 52 ... 54:
-        /* CMD52, CMD53, CMD54: reserved for SDIO cards
-         * (see the SDIO Simplified Specification V2.0)
-         * Handle as illegal command but do not complain
-         * on stderr, as some OSes may use these in their
-         * probing for presence of an SDIO card.
-         */
-        return sd_illegal;
-
     /* Application specific commands (Class 8) */
     case 55:  /* CMD55:  APP_CMD */
         switch (sd->state) {
@@ -1557,19 +1539,12 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 58:    /* CMD58:   READ_OCR (SPI) */
-        if (!sd->spi) {
-            goto bad_cmd;
-        }
         return sd_r3;
 
     case 59:    /* CMD59:   CRC_ON_OFF (SPI) */
-        if (!sd->spi) {
-            goto bad_cmd;
-        }
         return sd_r1;
 
     default:
-    bad_cmd:
         qemu_log_mask(LOG_GUEST_ERROR, "SD: Unknown CMD%i\n", req.cmd);
         return sd_illegal;
     }
@@ -2156,10 +2131,25 @@ void sd_enable(SDState *sd, bool enable)
 
 static const SDProto sd_proto_spi = {
     .name = "SPI",
+    .cmd = {
+        [2 ... 4]   = sd_cmd_illegal,
+        [5]         = sd_cmd_illegal,
+        [7]         = sd_cmd_illegal,
+        [15]        = sd_cmd_illegal,
+        [26]        = sd_cmd_illegal,
+        [52 ... 54] = sd_cmd_illegal,
+    },
 };
 
 static const SDProto sd_proto_sd = {
     .name = "SD",
+    .cmd = {
+        [1]         = sd_cmd_illegal,
+        [5]         = sd_cmd_illegal,
+        [52 ... 54] = sd_cmd_illegal,
+        [58]        = sd_cmd_illegal,
+        [59]        = sd_cmd_illegal,
+    },
 };
 
 static void sd_instance_init(Object *obj)
-- 
2.41.0

[PATCH 06/32] hw/sd: Add sd_cmd_unimplemented() handler

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
[ clg: Fix redundant assignment of .cmd ]
Message-Id: <20210624142209.1193073-7-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 83a0d6825147..e88bfcb8c802 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1011,6 +1011,15 @@ static sd_rsp_type_t sd_cmd_illegal(SDState *sd, SDRequest req)
     return sd_illegal;
 }
 
+/* Commands that are recognised but not yet implemented. */
+static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd, SDRequest req)
+{
+    qemu_log_mask(LOG_UNIMP, "%s: CMD%i not implemented\n",
+                  sd_proto(sd)->name, req.cmd);
+
+    return sd_illegal;
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1565,9 +1574,6 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
 
     switch (req.cmd) {
     case 6:  /* ACMD6:  SET_BUS_WIDTH */
-        if (sd->spi) {
-            goto unimplemented_spi_cmd;
-        }
         switch (sd->state) {
         case sd_transfer_state:
             sd->sd_status[0] &= 0x3f;
@@ -1698,12 +1704,6 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
     default:
         /* Fall back to standard commands.  */
         return sd_normal_command(sd, req);
-
-    unimplemented_spi_cmd:
-        /* Commands that are recognised but not yet implemented in SPI mode.  */
-        qemu_log_mask(LOG_UNIMP, "SD: CMD%i not implemented in SPI mode\n",
-                      req.cmd);
-        return sd_illegal;
     }
 
     qemu_log_mask(LOG_GUEST_ERROR, "SD: ACMD%i in a wrong state\n", req.cmd);
@@ -2139,6 +2139,9 @@ static const SDProto sd_proto_spi = {
         [26]        = sd_cmd_illegal,
         [52 ... 54] = sd_cmd_illegal,
     },
+    .acmd = {
+        [6]         = sd_cmd_unimplemented,
+    },
 };
 
 static const SDProto sd_proto_sd = {
-- 
2.41.0

[PATCH 07/32] hw/sd: Add sd_cmd_GO_IDLE_STATE() handler

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210624142209.1193073-8-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index e88bfcb8c802..535b72ff5c2a 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1020,6 +1020,16 @@ static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd, SDRequest req)
     return sd_illegal;
 }
 
+static sd_rsp_type_t sd_cmd_GO_IDLE_STATE(SDState *sd, SDRequest req)
+{
+    if (sd->state != sd_inactive_state) {
+        sd->state = sd_idle_state;
+        sd_reset(DEVICE(sd));
+    }
+
+    return sd->spi ? sd_r1 : sd_r0;
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1059,18 +1069,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     switch (req.cmd) {
     /* Basic commands (Class 0 and Class 1) */
-    case 0:  /* CMD0:   GO_IDLE_STATE */
-        switch (sd->state) {
-        case sd_inactive_state:
-            return sd->spi ? sd_r1 : sd_r0;
-
-        default:
-            sd->state = sd_idle_state;
-            sd_reset(DEVICE(sd));
-            return sd->spi ? sd_r1 : sd_r0;
-        }
-        break;
-
     case 1:  /* CMD1:   SEND_OP_CMD */
         sd->state = sd_transfer_state;
         return sd_r1;
@@ -2132,6 +2130,7 @@ void sd_enable(SDState *sd, bool enable)
 static const SDProto sd_proto_spi = {
     .name = "SPI",
     .cmd = {
+        [0]         = sd_cmd_GO_IDLE_STATE,
         [2 ... 4]   = sd_cmd_illegal,
         [5]         = sd_cmd_illegal,
         [7]         = sd_cmd_illegal,
@@ -2147,6 +2146,7 @@ static const SDProto sd_proto_spi = {
 static const SDProto sd_proto_sd = {
     .name = "SD",
     .cmd = {
+        [0]         = sd_cmd_GO_IDLE_STATE,
         [1]         = sd_cmd_illegal,
         [5]         = sd_cmd_illegal,
         [52 ... 54] = sd_cmd_illegal,
-- 
2.41.0

[PATCH 08/32] hw/sd: Add sd_cmd_SEND_OP_CMD() handler

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
[ clg: Update cmd_abbrev ]
Message-Id: <20210624142209.1193073-9-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c             | 18 +++++++++---------
 hw/sd/sdmmc-internal.c |  2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 535b72ff5c2a..bd67c50894fe 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1030,6 +1030,13 @@ static sd_rsp_type_t sd_cmd_GO_IDLE_STATE(SDState *sd, SDRequest req)
     return sd->spi ? sd_r1 : sd_r0;
 }
 
+static sd_rsp_type_t sd_cmd_SEND_OP_CMD(SDState *sd, SDRequest req)
+{
+    sd->state = sd_transfer_state;
+
+    return sd_r1;
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1069,10 +1076,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     switch (req.cmd) {
     /* Basic commands (Class 0 and Class 1) */
-    case 1:  /* CMD1:   SEND_OP_CMD */
-        sd->state = sd_transfer_state;
-        return sd_r1;
-
     case 2:  /* CMD2:   ALL_SEND_CID */
         switch (sd->state) {
         case sd_ready_state:
@@ -1622,11 +1625,6 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         break;
 
     case 41:  /* ACMD41: SD_APP_OP_COND */
-        if (sd->spi) {
-            /* SEND_OP_CMD */
-            sd->state = sd_transfer_state;
-            return sd_r1;
-        }
         if (sd->state != sd_idle_state) {
             break;
         }
@@ -2131,6 +2129,7 @@ static const SDProto sd_proto_spi = {
     .name = "SPI",
     .cmd = {
         [0]         = sd_cmd_GO_IDLE_STATE,
+        [1]         = sd_cmd_SEND_OP_CMD,
         [2 ... 4]   = sd_cmd_illegal,
         [5]         = sd_cmd_illegal,
         [7]         = sd_cmd_illegal,
@@ -2140,6 +2139,7 @@ static const SDProto sd_proto_spi = {
     },
     .acmd = {
         [6]         = sd_cmd_unimplemented,
+        [41]        = sd_cmd_SEND_OP_CMD,
     },
 };
 
diff --git a/hw/sd/sdmmc-internal.c b/hw/sd/sdmmc-internal.c
index 2053def3f10b..8648a7808dcc 100644
--- a/hw/sd/sdmmc-internal.c
+++ b/hw/sd/sdmmc-internal.c
@@ -14,7 +14,7 @@
 const char *sd_cmd_name(uint8_t cmd)
 {
     static const char *cmd_abbrev[SDMMC_CMD_MAX] = {
-         [0]    = "GO_IDLE_STATE",
+         [0]    = "GO_IDLE_STATE",           [1]    = "SEND_OP_CMD",
          [2]    = "ALL_SEND_CID",            [3]    = "SEND_RELATIVE_ADDR",
          [4]    = "SET_DSR",                 [5]    = "IO_SEND_OP_COND",
          [6]    = "SWITCH_FUNC",             [7]    = "SELECT/DESELECT_CARD",
-- 
2.41.0

[PATCH 09/32] hw/sd: Add sd_cmd_ALL_SEND_CID() handler

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210624142209.1193073-10-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index bd67c50894fe..33ecff496ade 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1037,6 +1037,17 @@ static sd_rsp_type_t sd_cmd_SEND_OP_CMD(SDState *sd, SDRequest req)
     return sd_r1;
 }
 
+static sd_rsp_type_t sd_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
+{
+    if (sd->state != sd_ready_state) {
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+
+    sd->state = sd_identification_state;
+
+    return sd_r2_i;
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1076,17 +1087,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     switch (req.cmd) {
     /* Basic commands (Class 0 and Class 1) */
-    case 2:  /* CMD2:   ALL_SEND_CID */
-        switch (sd->state) {
-        case sd_ready_state:
-            sd->state = sd_identification_state;
-            return sd_r2_i;
-
-        default:
-            break;
-        }
-        break;
-
     case 3:  /* CMD3:   SEND_RELATIVE_ADDR */
         switch (sd->state) {
         case sd_identification_state:
@@ -2148,6 +2148,7 @@ static const SDProto sd_proto_sd = {
     .cmd = {
         [0]         = sd_cmd_GO_IDLE_STATE,
         [1]         = sd_cmd_illegal,
+        [2]         = sd_cmd_ALL_SEND_CID,
         [5]         = sd_cmd_illegal,
         [52 ... 54] = sd_cmd_illegal,
         [58]        = sd_cmd_illegal,
-- 
2.41.0

[PATCH 10/32] hw/sd: Add sd_cmd_SEND_RELATIVE_ADDR() handler

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210624142209.1193073-11-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 33ecff496ade..b46072424108 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1048,6 +1048,20 @@ static sd_rsp_type_t sd_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
     return sd_r2_i;
 }
 
+static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
+{
+    switch (sd->state) {
+    case sd_identification_state:
+    case sd_standby_state:
+        sd->state = sd_standby_state;
+        sd_set_rca(sd);
+        return sd_r6;
+
+    default:
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1087,19 +1101,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     switch (req.cmd) {
     /* Basic commands (Class 0 and Class 1) */
-    case 3:  /* CMD3:   SEND_RELATIVE_ADDR */
-        switch (sd->state) {
-        case sd_identification_state:
-        case sd_standby_state:
-            sd->state = sd_standby_state;
-            sd_set_rca(sd);
-            return sd_r6;
-
-        default:
-            break;
-        }
-        break;
-
     case 4:  /* CMD4:   SEND_DSR */
         switch (sd->state) {
         case sd_standby_state:
@@ -2149,6 +2150,7 @@ static const SDProto sd_proto_sd = {
         [0]         = sd_cmd_GO_IDLE_STATE,
         [1]         = sd_cmd_illegal,
         [2]         = sd_cmd_ALL_SEND_CID,
+        [3]         = sd_cmd_SEND_RELATIVE_ADDR,
         [5]         = sd_cmd_illegal,
         [52 ... 54] = sd_cmd_illegal,
         [58]        = sd_cmd_illegal,
-- 
2.41.0

[PATCH 11/32] hw/sd: Add sd_cmd_SEND_TUNING_BLOCK() handler

[Cédric Le Goater]

From: Joel Stanley <joel@jms.id.au>

Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
[PMD: Rebased]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b46072424108..00a59450b726 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1062,6 +1062,22 @@ static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
     }
 }
 
+static sd_rsp_type_t sd_cmd_SEND_TUNING_BLOCK(SDState *sd, SDRequest req)
+{
+        if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
+            return sd_cmd_illegal(sd, req);
+        }
+
+        if (sd->state != sd_transfer_state) {
+            return sd_invalid_state_for_cmd(sd, req);
+        }
+
+        sd->state = sd_sendingdata_state;
+        sd->data_offset = 0;
+
+        return sd_r1;
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1305,17 +1321,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 19:    /* CMD19: SEND_TUNING_BLOCK (SD) */
-        if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
-            return sd_invalid_state_for_cmd(sd, req);
-        }
-        if (sd->state == sd_transfer_state) {
-            sd->state = sd_sendingdata_state;
-            sd->data_offset = 0;
-            return sd_r1;
-        }
-        break;
-
     case 23:    /* CMD23: SET_BLOCK_COUNT */
         if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
             return sd_invalid_state_for_cmd(sd, req);
@@ -2152,6 +2157,7 @@ static const SDProto sd_proto_sd = {
         [2]         = sd_cmd_ALL_SEND_CID,
         [3]         = sd_cmd_SEND_RELATIVE_ADDR,
         [5]         = sd_cmd_illegal,
+        [19]        = sd_cmd_SEND_TUNING_BLOCK,
         [52 ... 54] = sd_cmd_illegal,
         [58]        = sd_cmd_illegal,
         [59]        = sd_cmd_illegal,
-- 
2.41.0

[PATCH 12/32] hw/sd: Add sd_cmd_SET_BLOCK_COUNT() handler

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 00a59450b726..d1c0b132c227 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1078,6 +1078,21 @@ static sd_rsp_type_t sd_cmd_SEND_TUNING_BLOCK(SDState *sd, SDRequest req)
         return sd_r1;
 }
 
+static sd_rsp_type_t sd_cmd_SET_BLOCK_COUNT(SDState *sd, SDRequest req)
+{
+        if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
+            return sd_cmd_illegal(sd, req);
+        }
+
+        if (sd->state != sd_transfer_state) {
+            return sd_invalid_state_for_cmd(sd, req);
+        }
+
+        sd->multi_blk_cnt = req.arg;
+
+        return sd_r1;
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1321,20 +1336,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 23:    /* CMD23: SET_BLOCK_COUNT */
-        if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
-            return sd_invalid_state_for_cmd(sd, req);
-        }
-        switch (sd->state) {
-        case sd_transfer_state:
-            sd->multi_blk_cnt = req.arg;
-            return sd_r1;
-
-        default:
-            break;
-        }
-        break;
-
     /* Block write commands (Class 4) */
     case 24:  /* CMD24:  WRITE_SINGLE_BLOCK */
     case 25:  /* CMD25:  WRITE_MULTIPLE_BLOCK */
@@ -2158,6 +2159,7 @@ static const SDProto sd_proto_sd = {
         [3]         = sd_cmd_SEND_RELATIVE_ADDR,
         [5]         = sd_cmd_illegal,
         [19]        = sd_cmd_SEND_TUNING_BLOCK,
+        [23]        = sd_cmd_SET_BLOCK_COUNT,
         [52 ... 54] = sd_cmd_illegal,
         [58]        = sd_cmd_illegal,
         [59]        = sd_cmd_illegal,
-- 
2.41.0

[PATCH 13/32] hw/sd: Introduce a "sd-card" SPI variant model

[Cédric Le Goater]

and replace the SDState::spi attribute with a test checking the
SDProto array of commands.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/sd/sd.h  |  3 +++
 hw/arm/stellaris.c  |  3 +--
 hw/riscv/sifive_u.c |  3 +--
 hw/sd/sd.c          | 52 +++++++++++++++++++++++++++++++++------------
 4 files changed, 43 insertions(+), 18 deletions(-)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index b322d8f19b17..2c8748fb9b97 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -93,6 +93,9 @@ typedef struct {
 #define TYPE_SD_CARD "sd-card"
 OBJECT_DECLARE_TYPE(SDState, SDCardClass, SD_CARD)
 
+#define TYPE_SD_CARD_SPI "sd-card-spi"
+DECLARE_INSTANCE_CHECKER(SDState, SD_CARD_SPI, TYPE_SD_CARD_SPI)
+
 struct SDCardClass {
     /*< private >*/
     DeviceClass parent_class;
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index 5a3106e00939..aa5b0ddfaa5a 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -1235,9 +1235,8 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
 
             dinfo = drive_get(IF_SD, 0, 0);
             blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
-            carddev = qdev_new(TYPE_SD_CARD);
+            carddev = qdev_new(TYPE_SD_CARD_SPI);
             qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal);
-            qdev_prop_set_bit(carddev, "spi", true);
             qdev_realize_and_unref(carddev,
                                    qdev_get_child_bus(sddev, "sd-bus"),
                                    &error_fatal);
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 35a335b8d0ba..ec76dce6c952 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -674,9 +674,8 @@ static void sifive_u_machine_init(MachineState *machine)
 
     dinfo = drive_get(IF_SD, 0, 0);
     blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
-    card_dev = qdev_new(TYPE_SD_CARD);
+    card_dev = qdev_new(TYPE_SD_CARD_SPI);
     qdev_prop_set_drive_err(card_dev, "drive", blk, &error_fatal);
-    qdev_prop_set_bit(card_dev, "spi", true);
     qdev_realize_and_unref(card_dev,
                            qdev_get_child_bus(sd_dev, "sd-bus"),
                            &error_fatal);
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index d1c0b132c227..03fdb3addc38 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -115,7 +115,6 @@ struct SDState {
 
     uint8_t spec_version;
     BlockBackend *blk;
-    bool spi;
 
     /* Runtime changeables */
 
@@ -159,6 +158,13 @@ static const struct SDProto *sd_proto(SDState *sd)
     return sc->proto;
 }
 
+static const SDProto sd_proto_spi;
+
+static bool sd_is_spi(SDState *sd)
+{
+    return sd_proto(sd) == &sd_proto_spi;
+}
+
 static const char *sd_version_str(enum SDPhySpecificationVersion version)
 {
     static const char *sdphy_version[] = {
@@ -336,7 +342,7 @@ static void sd_set_ocr(SDState *sd)
     /* All voltages OK */
     sd->ocr = R_OCR_VDD_VOLTAGE_WIN_HI_MASK;
 
-    if (sd->spi) {
+    if (sd_is_spi(sd)) {
         /*
          * We don't need to emulate power up sequence in SPI-mode.
          * Thus, the card's power up status bit should be set to 1 when reset.
@@ -741,13 +747,12 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
     SDState *sd;
     Error *err = NULL;
 
-    obj = object_new(TYPE_SD_CARD);
+    obj = object_new(is_spi ? TYPE_SD_CARD_SPI : TYPE_SD_CARD);
     dev = DEVICE(obj);
     if (!qdev_prop_set_drive_err(dev, "drive", blk, &err)) {
         error_reportf_err(err, "sd_init failed: ");
         return NULL;
     }
-    qdev_prop_set_bit(dev, "spi", is_spi);
 
     /*
      * Realizing the device properly would put it into the QOM
@@ -1027,7 +1032,7 @@ static sd_rsp_type_t sd_cmd_GO_IDLE_STATE(SDState *sd, SDRequest req)
         sd_reset(DEVICE(sd));
     }
 
-    return sd->spi ? sd_r1 : sd_r0;
+    return sd_is_spi(sd) ? sd_r1 : sd_r0;
 }
 
 static sd_rsp_type_t sd_cmd_SEND_OP_CMD(SDState *sd, SDRequest req)
@@ -1203,7 +1208,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
         /* No response if not exactly one VHS bit is set.  */
         if (!(req.arg >> 8) || (req.arg >> (ctz32(req.arg & ~0xff) + 1))) {
-            return sd->spi ? sd_r7 : sd_r0;
+            return sd_is_spi(sd) ? sd_r7 : sd_r0;
         }
 
         /* Accept.  */
@@ -1219,7 +1224,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             return sd_r2_s;
 
         case sd_transfer_state:
-            if (!sd->spi)
+            if (!sd_is_spi(sd))
                 break;
             sd->state = sd_sendingdata_state;
             memcpy(sd->data, sd->csd, 16);
@@ -1241,7 +1246,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             return sd_r2_i;
 
         case sd_transfer_state:
-            if (!sd->spi)
+            if (!sd_is_spi(sd))
                 break;
             sd->state = sd_sendingdata_state;
             memcpy(sd->data, sd->cid, 16);
@@ -1274,7 +1279,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
     case 13:  /* CMD13:  SEND_STATUS */
         switch (sd->mode) {
         case sd_data_transfer_mode:
-            if (!sd->spi && sd->rca != rca) {
+            if (!sd_is_spi(sd) && sd->rca != rca) {
                 return sd_r0;
             }
 
@@ -1531,7 +1536,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         default:
             break;
         }
-        if (!sd->spi) {
+        if (!sd_is_spi(sd)) {
             if (sd->rca != rca) {
                 return sd_r0;
             }
@@ -2184,11 +2189,8 @@ static void sd_instance_finalize(Object *obj)
 static void sd_realize(DeviceState *dev, Error **errp)
 {
     SDState *sd = SD_CARD(dev);
-    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
     int ret;
 
-    sc->proto = sd->spi ? &sd_proto_spi : &sd_proto_sd;
-
     switch (sd->spec_version) {
     case SD_PHY_SPECv1_10_VERS
      ... SD_PHY_SPECv3_01_VERS:
@@ -2245,7 +2247,6 @@ static Property sd_properties[] = {
      * whether card should be in SSI or MMC/SD mode.  It is also up to the
      * board to ensure that ssi transfers only occur when the chip select
      * is asserted.  */
-    DEFINE_PROP_BOOL("spi", SDState, spi, false),
     DEFINE_PROP_END_OF_LIST()
 };
 
@@ -2272,6 +2273,7 @@ static void sd_class_init(ObjectClass *klass, void *data)
     sc->enable = sd_enable;
     sc->get_inserted = sd_get_inserted;
     sc->get_readonly = sd_get_readonly;
+    sc->proto = &sd_proto_sd;
 }
 
 static const TypeInfo sd_info = {
@@ -2284,9 +2286,31 @@ static const TypeInfo sd_info = {
     .instance_finalize = sd_instance_finalize,
 };
 
+/*
+ * We do not model the chip select pin, so allow the board to select
+ * whether card should be in SSI or MMC/SD mode.  It is also up to the
+ * board to ensure that ssi transfers only occur when the chip select
+ * is asserted.
+ */
+static void sd_spi_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SDCardClass *sc = SD_CARD_CLASS(klass);
+
+    dc->desc = "SD SPI";
+    sc->proto = &sd_proto_spi;
+}
+
+static const TypeInfo sd_spi_info = {
+    .name = TYPE_SD_CARD_SPI,
+    .parent = TYPE_SD_CARD,
+    .class_init = sd_spi_class_init,
+};
+
 static void sd_register_types(void)
 {
     type_register_static(&sd_info);
+    type_register_static(&sd_spi_info);
 }
 
 type_init(sd_register_types)
-- 
2.41.0

Re: [PATCH 13/32] hw/sd: Introduce a "sd-card" SPI variant model

[Philippe Mathieu-Daudé]

On 3/7/23 15:24, Cédric Le Goater wrote:
> and replace the SDState::spi attribute with a test checking the
> SDProto array of commands.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   include/hw/sd/sd.h  |  3 +++
>   hw/arm/stellaris.c  |  3 +--
>   hw/riscv/sifive_u.c |  3 +--
>   hw/sd/sd.c          | 52 +++++++++++++++++++++++++++++++++------------
>   4 files changed, 43 insertions(+), 18 deletions(-)

I'd rather TYPE_SD_CARD_SPI have a SSI parent bus, having
the realize() method create an internal TYPE_SSI_SD
SD <-> SPI bus bridge, and attaching the SD card on it.
Then we could even make TYPE_SSI_SD private.

Anyway, "can be done later" and "good enough for now"
I suppose, so:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks!

[PATCH 14/32] hw/sd: Basis for eMMC support

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

The initial eMMC support from Vincent Palatin was largely reworked to
match the current SD framework.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/sd/sd.h |  3 +++
 hw/sd/sd.c         | 44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 2c8748fb9b97..da97400469a0 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -96,6 +96,9 @@ OBJECT_DECLARE_TYPE(SDState, SDCardClass, SD_CARD)
 #define TYPE_SD_CARD_SPI "sd-card-spi"
 DECLARE_INSTANCE_CHECKER(SDState, SD_CARD_SPI, TYPE_SD_CARD_SPI)
 
+#define TYPE_EMMC "emmc"
+DECLARE_INSTANCE_CHECKER(SDState, EMMC, TYPE_EMMC)
+
 struct SDCardClass {
     /*< private >*/
     DeviceClass parent_class;
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 03fdb3addc38..409fbbcbd8d7 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2171,6 +2171,20 @@ static const SDProto sd_proto_sd = {
     },
 };
 
+static const SDProto sd_proto_emmc = {
+    .name = "eMMC",
+    .cmd = {
+        [0]         = sd_cmd_GO_IDLE_STATE,
+        [5]         = sd_cmd_illegal,
+        [19]        = sd_cmd_SEND_TUNING_BLOCK,
+        [23]        = sd_cmd_SET_BLOCK_COUNT,
+        [41]        = sd_cmd_illegal,
+        [52 ... 54] = sd_cmd_illegal,
+        [58]        = sd_cmd_illegal,
+        [59]        = sd_cmd_illegal,
+    },
+};
+
 static void sd_instance_init(Object *obj)
 {
     SDState *sd = SD_CARD(obj);
@@ -2307,10 +2321,40 @@ static const TypeInfo sd_spi_info = {
     .class_init = sd_spi_class_init,
 };
 
+static void emmc_realize(DeviceState *dev, Error **errp)
+{
+    SDState *sd = SD_CARD(dev);
+
+    if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
+            error_setg(errp, "Minimum spec for eMMC is v3.01");
+            return;
+    }
+
+    sd_realize(dev, errp);
+}
+
+static void emmc_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SDCardClass *sc = SD_CARD_CLASS(klass);
+
+    dc->desc = "eMMC";
+    dc->realize = emmc_realize;
+    sc->proto = &sd_proto_emmc;
+}
+
+static const TypeInfo emmc_info = {
+    .name = TYPE_EMMC,
+    .parent = TYPE_SD_CARD,
+    .class_init = emmc_class_init,
+ };
+
+
 static void sd_register_types(void)
 {
     type_register_static(&sd_info);
     type_register_static(&sd_spi_info);
+    type_register_static(&emmc_info);
 }
 
 type_init(sd_register_types)
-- 
2.41.0

[PATCH 15/32] hw/sd: Add emmc_cmd_SEND_OP_CMD() handler

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 409fbbcbd8d7..f846440b737a 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2171,10 +2171,17 @@ static const SDProto sd_proto_sd = {
     },
 };
 
+static sd_rsp_type_t emmc_cmd_SEND_OP_CMD(SDState *sd, SDRequest req)
+{
+    sd->state = sd_ready_state;
+    return sd_r3;
+}
+
 static const SDProto sd_proto_emmc = {
     .name = "eMMC",
     .cmd = {
         [0]         = sd_cmd_GO_IDLE_STATE,
+        [1]         = emmc_cmd_SEND_OP_CMD,
         [5]         = sd_cmd_illegal,
         [19]        = sd_cmd_SEND_TUNING_BLOCK,
         [23]        = sd_cmd_SET_BLOCK_COUNT,
-- 
2.41.0

Re: [PATCH 15/32] hw/sd: Add emmc_cmd_SEND_OP_CMD() handler

[Philippe Mathieu-Daudé]

On 3/7/23 15:24, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sd.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 409fbbcbd8d7..f846440b737a 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -2171,10 +2171,17 @@ static const SDProto sd_proto_sd = {
>       },
>   };
>   
> +static sd_rsp_type_t emmc_cmd_SEND_OP_CMD(SDState *sd, SDRequest req)

"emmc_cmd_SEND_OP_COND" I suppose?

> +{
> +    sd->state = sd_ready_state;
> +    return sd_r3;
> +}
> +
>   static const SDProto sd_proto_emmc = {
>       .name = "eMMC",
>       .cmd = {
>           [0]         = sd_cmd_GO_IDLE_STATE,
> +        [1]         = emmc_cmd_SEND_OP_CMD,
>           [5]         = sd_cmd_illegal,
>           [19]        = sd_cmd_SEND_TUNING_BLOCK,
>           [23]        = sd_cmd_SET_BLOCK_COUNT,

Re: [PATCH 15/32] hw/sd: Add emmc_cmd_SEND_OP_CMD() handler

[Philippe Mathieu-Daudé]

On 3/6/24 14:25, Philippe Mathieu-Daudé wrote:
> On 3/7/23 15:24, Cédric Le Goater wrote:
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   hw/sd/sd.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 409fbbcbd8d7..f846440b737a 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -2171,10 +2171,17 @@ static const SDProto sd_proto_sd = {
>>       },
>>   };
>> +static sd_rsp_type_t emmc_cmd_SEND_OP_CMD(SDState *sd, SDRequest req)
> 
> "emmc_cmd_SEND_OP_COND" I suppose?

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>> +{
>> +    sd->state = sd_ready_state;
>> +    return sd_r3;
>> +}
>> +
>>   static const SDProto sd_proto_emmc = {
>>       .name = "eMMC",
>>       .cmd = {
>>           [0]         = sd_cmd_GO_IDLE_STATE,
>> +        [1]         = emmc_cmd_SEND_OP_CMD,
>>           [5]         = sd_cmd_illegal,
>>           [19]        = sd_cmd_SEND_TUNING_BLOCK,
>>           [23]        = sd_cmd_SET_BLOCK_COUNT,
> 

[PATCH 16/32] hw/sd: Add emmc_cmd_ALL_SEND_CID() handler

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index f846440b737a..dd60a16f8c0a 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2177,11 +2177,23 @@ static sd_rsp_type_t emmc_cmd_SEND_OP_CMD(SDState *sd, SDRequest req)
     return sd_r3;
 }
 
+static sd_rsp_type_t emmc_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
+{
+    if (sd->state != sd_ready_state && sd->state != sd_idle_state) {
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+
+    sd->state = sd_identification_state;
+
+    return sd_r2_i;
+}
+
 static const SDProto sd_proto_emmc = {
     .name = "eMMC",
     .cmd = {
         [0]         = sd_cmd_GO_IDLE_STATE,
         [1]         = emmc_cmd_SEND_OP_CMD,
+        [2]         = emmc_cmd_ALL_SEND_CID,
         [5]         = sd_cmd_illegal,
         [19]        = sd_cmd_SEND_TUNING_BLOCK,
         [23]        = sd_cmd_SET_BLOCK_COUNT,
-- 
2.41.0

Re: [PATCH 16/32] hw/sd: Add emmc_cmd_ALL_SEND_CID() handler

[Philippe Mathieu-Daudé]

On 3/7/23 15:24, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sd.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 17/32] hw/sd: Add emmc_cmd_SEND_RELATIVE_ADDR() handler

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index dd60a16f8c0a..5ff132139ea9 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1053,6 +1053,25 @@ static sd_rsp_type_t sd_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
     return sd_r2_i;
 }
 
+static void sd_emmc_set_rca(SDState *sd, uint16_t value)
+{
+    sd->rca = value;
+}
+
+static sd_rsp_type_t emmc_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
+{
+    switch (sd->state) {
+    case sd_identification_state:
+    case sd_standby_state:
+        sd->state = sd_standby_state;
+        sd_emmc_set_rca(sd, req.arg >> 16);
+        return sd_r1;
+
+    default:
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+}
+
 static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
 {
     switch (sd->state) {
@@ -2194,6 +2213,7 @@ static const SDProto sd_proto_emmc = {
         [0]         = sd_cmd_GO_IDLE_STATE,
         [1]         = emmc_cmd_SEND_OP_CMD,
         [2]         = emmc_cmd_ALL_SEND_CID,
+        [3]         = emmc_cmd_SEND_RELATIVE_ADDR,
         [5]         = sd_cmd_illegal,
         [19]        = sd_cmd_SEND_TUNING_BLOCK,
         [23]        = sd_cmd_SET_BLOCK_COUNT,
-- 
2.41.0

Re: [PATCH 17/32] hw/sd: Add emmc_cmd_SEND_RELATIVE_ADDR() handler

[Philippe Mathieu-Daudé]

On 3/7/23 15:24, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sd.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index dd60a16f8c0a..5ff132139ea9 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1053,6 +1053,25 @@ static sd_rsp_type_t sd_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
>       return sd_r2_i;
>   }
>   
> +static void sd_emmc_set_rca(SDState *sd, uint16_t value)
> +{
> +    sd->rca = value;
> +}
> +
> +static sd_rsp_type_t emmc_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)

"emmc_cmd_SET_RELATIVE_ADDR".

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +{
> +    switch (sd->state) {
> +    case sd_identification_state:
> +    case sd_standby_state:
> +        sd->state = sd_standby_state;
> +        sd_emmc_set_rca(sd, req.arg >> 16);
> +        return sd_r1;
> +
> +    default:
> +        return sd_invalid_state_for_cmd(sd, req);
> +    }
> +}
> +
>   static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
>   {
>       switch (sd->state) {
> @@ -2194,6 +2213,7 @@ static const SDProto sd_proto_emmc = {
>           [0]         = sd_cmd_GO_IDLE_STATE,
>           [1]         = emmc_cmd_SEND_OP_CMD,
>           [2]         = emmc_cmd_ALL_SEND_CID,
> +        [3]         = emmc_cmd_SEND_RELATIVE_ADDR,
>           [5]         = sd_cmd_illegal,
>           [19]        = sd_cmd_SEND_TUNING_BLOCK,
>           [23]        = sd_cmd_SET_BLOCK_COUNT,

Re: [PATCH 17/32] hw/sd: Add emmc_cmd_SEND_RELATIVE_ADDR() handler

[Cédric Le Goater]

On 6/3/24 14:26, Philippe Mathieu-Daudé wrote:
> On 3/7/23 15:24, Cédric Le Goater wrote:
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   hw/sd/sd.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index dd60a16f8c0a..5ff132139ea9 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -1053,6 +1053,25 @@ static sd_rsp_type_t sd_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
>>       return sd_r2_i;
>>   }
>> +static void sd_emmc_set_rca(SDState *sd, uint16_t value)
>> +{
>> +    sd->rca = value;
>> +}
>> +
>> +static sd_rsp_type_t emmc_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
> 
> "emmc_cmd_SET_RELATIVE_ADDR".

Fixed.


Thanks,

C.


> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
>> +{
>> +    switch (sd->state) {
>> +    case sd_identification_state:
>> +    case sd_standby_state:
>> +        sd->state = sd_standby_state;
>> +        sd_emmc_set_rca(sd, req.arg >> 16);
>> +        return sd_r1;
>> +
>> +    default:
>> +        return sd_invalid_state_for_cmd(sd, req);
>> +    }
>> +}
>> +
>>   static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
>>   {
>>       switch (sd->state) {
>> @@ -2194,6 +2213,7 @@ static const SDProto sd_proto_emmc = {
>>           [0]         = sd_cmd_GO_IDLE_STATE,
>>           [1]         = emmc_cmd_SEND_OP_CMD,
>>           [2]         = emmc_cmd_ALL_SEND_CID,
>> +        [3]         = emmc_cmd_SEND_RELATIVE_ADDR,
>>           [5]         = sd_cmd_illegal,
>>           [19]        = sd_cmd_SEND_TUNING_BLOCK,
>>           [23]        = sd_cmd_SET_BLOCK_COUNT,
> 

[PATCH 18/32] hw/sd: Add emmc_cmd_APP_CMD() handler

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 5ff132139ea9..95cb46b87519 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2207,6 +2207,11 @@ static sd_rsp_type_t emmc_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
     return sd_r2_i;
 }
 
+static sd_rsp_type_t emmc_cmd_APP_CMD(SDState *sd, SDRequest req)
+{
+    return sd_r0;
+}
+
 static const SDProto sd_proto_emmc = {
     .name = "eMMC",
     .cmd = {
@@ -2219,6 +2224,7 @@ static const SDProto sd_proto_emmc = {
         [23]        = sd_cmd_SET_BLOCK_COUNT,
         [41]        = sd_cmd_illegal,
         [52 ... 54] = sd_cmd_illegal,
+        [55]        = emmc_cmd_APP_CMD,
         [58]        = sd_cmd_illegal,
         [59]        = sd_cmd_illegal,
     },
-- 
2.41.0

Re: [PATCH 18/32] hw/sd: Add emmc_cmd_APP_CMD() handler

[Philippe Mathieu-Daudé]

Hi Cédric,

On 3/7/23 15:24, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sd.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 5ff132139ea9..95cb46b87519 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -2207,6 +2207,11 @@ static sd_rsp_type_t emmc_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
>       return sd_r2_i;
>   }
>   
> +static sd_rsp_type_t emmc_cmd_APP_CMD(SDState *sd, SDRequest req)
> +{
> +    return sd_r0;

Why are you returning R0? This is invalid, only R1 can be
returned by APP_CMD.

> +}
> +
>   static const SDProto sd_proto_emmc = {
>       .name = "eMMC",
>       .cmd = {
> @@ -2219,6 +2224,7 @@ static const SDProto sd_proto_emmc = {
>           [23]        = sd_cmd_SET_BLOCK_COUNT,
>           [41]        = sd_cmd_illegal,
>           [52 ... 54] = sd_cmd_illegal,
> +        [55]        = emmc_cmd_APP_CMD,
>           [58]        = sd_cmd_illegal,
>           [59]        = sd_cmd_illegal,
>       },

Re: [PATCH 18/32] hw/sd: Add emmc_cmd_APP_CMD() handler

[Cédric Le Goater]

On 6/25/24 5:04 PM, Philippe Mathieu-Daudé wrote:
> Hi Cédric,
> 
> On 3/7/23 15:24, Cédric Le Goater wrote:
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   hw/sd/sd.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 5ff132139ea9..95cb46b87519 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -2207,6 +2207,11 @@ static sd_rsp_type_t emmc_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
>>       return sd_r2_i;
>>   }
>> +static sd_rsp_type_t emmc_cmd_APP_CMD(SDState *sd, SDRequest req)
>> +{
>> +    return sd_r0;
> 
> Why are you returning R0? This is invalid, only R1 can be
> returned by APP_CMD.

Probably a typo. This is old ... 4/5 years at least.

Thanks,

C.




> 
>> +}
>> +
>>   static const SDProto sd_proto_emmc = {
>>       .name = "eMMC",
>>       .cmd = {
>> @@ -2219,6 +2224,7 @@ static const SDProto sd_proto_emmc = {
>>           [23]        = sd_cmd_SET_BLOCK_COUNT,
>>           [41]        = sd_cmd_illegal,
>>           [52 ... 54] = sd_cmd_illegal,
>> +        [55]        = emmc_cmd_APP_CMD,
>>           [58]        = sd_cmd_illegal,
>>           [59]        = sd_cmd_illegal,
>>       },
> 

Re: [PATCH 18/32] hw/sd: Add emmc_cmd_APP_CMD() handler

[Philippe Mathieu-Daudé]

On 25/6/24 17:13, Cédric Le Goater wrote:
> On 6/25/24 5:04 PM, Philippe Mathieu-Daudé wrote:
>> Hi Cédric,
>>
>> On 3/7/23 15:24, Cédric Le Goater wrote:
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>   hw/sd/sd.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>>> index 5ff132139ea9..95cb46b87519 100644
>>> --- a/hw/sd/sd.c
>>> +++ b/hw/sd/sd.c
>>> @@ -2207,6 +2207,11 @@ static sd_rsp_type_t 
>>> emmc_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
>>>       return sd_r2_i;
>>>   }
>>> +static sd_rsp_type_t emmc_cmd_APP_CMD(SDState *sd, SDRequest req)
>>> +{
>>> +    return sd_r0;
>>
>> Why are you returning R0? This is invalid, only R1 can be
>> returned by APP_CMD.
> 
> Probably a typo. This is old ... 4/5 years at least.

Well, a smart typo, because it hides unimplemented features
(and probably some bugs). Maybe someone clever used R0 on
purpose :)

> Thanks,
> 
> C.

Re: [PATCH 18/32] hw/sd: Add emmc_cmd_APP_CMD() handler

[Cédric Le Goater]

On 6/25/24 5:32 PM, Philippe Mathieu-Daudé wrote:
> On 25/6/24 17:13, Cédric Le Goater wrote:
>> On 6/25/24 5:04 PM, Philippe Mathieu-Daudé wrote:
>>> Hi Cédric,
>>>
>>> On 3/7/23 15:24, Cédric Le Goater wrote:
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> ---
>>>>   hw/sd/sd.c | 6 ++++++
>>>>   1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>>>> index 5ff132139ea9..95cb46b87519 100644
>>>> --- a/hw/sd/sd.c
>>>> +++ b/hw/sd/sd.c
>>>> @@ -2207,6 +2207,11 @@ static sd_rsp_type_t emmc_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
>>>>       return sd_r2_i;
>>>>   }
>>>> +static sd_rsp_type_t emmc_cmd_APP_CMD(SDState *sd, SDRequest req)
>>>> +{
>>>> +    return sd_r0;
>>>
>>> Why are you returning R0? This is invalid, only R1 can be
>>> returned by APP_CMD.
>>
>> Probably a typo. This is old ... 4/5 years at least.
> 
> Well, a smart typo, because it hides unimplemented features
> (and probably some bugs). Maybe someone clever used R0 on
> purpose :)

I can't tell. The initial patch [*] had :

@@ -1115,6 +1219,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
  
      /* Application specific commands (Class 8) */
      case 55:	/* CMD55:  APP_CMD */
+        /* Not supported by MMC */
+        if (sd->emmc) {
+            return sd_r0;
+        }
+

but it's 13 years old now.

Thanks,

C.


[*] https://lore.kernel.org/qemu-devel/1311635951-11047-5-git-send-email-vpalatin@chromium.org/

[PATCH 19/32] hw/sd: add emmc_cmd_SEND_TUNING_BLOCK() handler

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 95cb46b87519..4b4a4cda2e68 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2212,6 +2212,17 @@ static sd_rsp_type_t emmc_cmd_APP_CMD(SDState *sd, SDRequest req)
     return sd_r0;
 }
 
+static sd_rsp_type_t emmc_cmd_SEND_TUNING_BLOCK(SDState *sd, SDRequest req)
+{
+    if (sd->state != sd_transfer_state) {
+        sd_invalid_state_for_cmd(sd, req);
+    }
+
+    sd->state = sd_sendingdata_state;
+    sd->data_offset = 0;
+    return sd_r1;
+}
+
 static const SDProto sd_proto_emmc = {
     .name = "eMMC",
     .cmd = {
@@ -2222,6 +2233,7 @@ static const SDProto sd_proto_emmc = {
         [5]         = sd_cmd_illegal,
         [19]        = sd_cmd_SEND_TUNING_BLOCK,
         [23]        = sd_cmd_SET_BLOCK_COUNT,
+        [21]        = emmc_cmd_SEND_TUNING_BLOCK,
         [41]        = sd_cmd_illegal,
         [52 ... 54] = sd_cmd_illegal,
         [55]        = emmc_cmd_APP_CMD,
-- 
2.41.0

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

[Cédric Le Goater]

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) */
+        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 ++];
 
-- 
2.41.0

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

[Philippe Mathieu-Daudé]

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?

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 ++];
>   

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.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 ++];
> 

[PATCH 21/32] hw/sd: Add mmc switch function support

[Cédric Le Goater]

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

switch operation in mmc cards, updated the ext_csd register to
request changes in card operations. Here we implement similar
sequence but requests are mostly dummy and make no change.

Implement SWITCH_ERROR if the write operation offset goes beyond length
of ext_csd.

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

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 7332f7a18435..51e2254728a6 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -482,6 +482,7 @@ static void sd_set_rca(SDState *sd)
 FIELD(CSR, AKE_SEQ_ERROR,               3,  1)
 FIELD(CSR, APP_CMD,                     5,  1)
 FIELD(CSR, FX_EVENT,                    6,  1)
+FIELD(CSR, SWITCH_ERROR,                7,  1)
 FIELD(CSR, READY_FOR_DATA,              8,  1)
 FIELD(CSR, CURRENT_STATE,               9,  4)
 FIELD(CSR, ERASE_RESET,                13,  1)
@@ -878,6 +879,43 @@ static uint32_t sd_wpbits(SDState *sd, uint64_t addr)
     return ret;
 }
 
+enum {
+    MMC_CMD6_ACCESS_COMMAND_SET = 0,
+    MMC_CMD6_ACCESS_SET_BITS,
+    MMC_CMD6_ACCESS_CLEAR_BITS,
+    MMC_CMD6_ACCESS_WRITE_BYTE,
+};
+
+static void mmc_function_switch(SDState *sd, uint32_t arg)
+{
+    uint32_t access = extract32(arg, 24, 2);
+    uint32_t index = extract32(arg, 16, 8);
+    uint32_t value = extract32(arg, 8, 8);
+    uint8_t b = sd->ext_csd[index];
+
+    switch (access) {
+    case MMC_CMD6_ACCESS_COMMAND_SET:
+        qemu_log_mask(LOG_UNIMP, "MMC Command set switching not supported\n");
+        return;
+    case MMC_CMD6_ACCESS_SET_BITS:
+        b |= value;
+        break;
+    case MMC_CMD6_ACCESS_CLEAR_BITS:
+        b &= ~value;
+        break;
+    case MMC_CMD6_ACCESS_WRITE_BYTE:
+        b = value;
+        break;
+    }
+
+    if (index >= 192) {
+        sd->card_status |= R_CSR_SWITCH_ERROR_MASK;
+        return;
+    }
+
+    sd->ext_csd[index] = b;
+}
+
 static void sd_function_switch(SDState *sd, uint32_t arg)
 {
     int i, mode, new_func;
@@ -2263,6 +2301,19 @@ static sd_rsp_type_t emmc_cmd_SEND_TUNING_BLOCK(SDState *sd, SDRequest req)
     return sd_r1;
 }
 
+static sd_rsp_type_t emmc_cmd_SWITCH_FUNCTION(SDState *sd, SDRequest req)
+{
+    switch (sd->state) {
+    case sd_transfer_state:
+        sd->state = sd_programming_state;
+        mmc_function_switch(sd, req.arg);
+        sd->state = sd_transfer_state;
+        return sd_r1b;
+    default:
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+}
+
 static const SDProto sd_proto_emmc = {
     .name = "eMMC",
     .cmd = {
@@ -2271,6 +2322,7 @@ static const SDProto sd_proto_emmc = {
         [2]         = emmc_cmd_ALL_SEND_CID,
         [3]         = emmc_cmd_SEND_RELATIVE_ADDR,
         [5]         = sd_cmd_illegal,
+        [6]         = emmc_cmd_SWITCH_FUNCTION,
         [19]        = sd_cmd_SEND_TUNING_BLOCK,
         [23]        = sd_cmd_SET_BLOCK_COUNT,
         [21]        = emmc_cmd_SEND_TUNING_BLOCK,
-- 
2.41.0

Re: [PATCH 21/32] hw/sd: Add mmc switch function support

[Philippe Mathieu-Daudé]

On 3/7/23 15:24, Cédric Le Goater wrote:
> From: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> 
> switch operation in mmc cards, updated the ext_csd register to
> request changes in card operations. Here we implement similar
> sequence but requests are mostly dummy and make no change.
> 
> Implement SWITCH_ERROR if the write operation offset goes beyond length
> of ext_csd.
> 
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> [ clg: - ported on SDProto framework ]
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sd.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 52 insertions(+)


> +static void mmc_function_switch(SDState *sd, uint32_t arg)
> +{
> +    uint32_t access = extract32(arg, 24, 2);
> +    uint32_t index = extract32(arg, 16, 8);
> +    uint32_t value = extract32(arg, 8, 8);
> +    uint8_t b = sd->ext_csd[index];

This field is added in the next patch :)

../../hw/sd/sd.c:927:21: error: no member named 'ext_csd' in 'struct 
SDState'
     uint8_t b = sd->ext_csd[index];
                 ~~  ^
../../hw/sd/sd.c:949:9: error: no member named 'ext_csd' in 'struct SDState'
     sd->ext_csd[index] = b;
     ~~  ^

No need to respin, as I'm integrating your work.

> +    switch (access) {
> +    case MMC_CMD6_ACCESS_COMMAND_SET:
> +        qemu_log_mask(LOG_UNIMP, "MMC Command set switching not supported\n");
> +        return;
> +    case MMC_CMD6_ACCESS_SET_BITS:
> +        b |= value;
> +        break;
> +    case MMC_CMD6_ACCESS_CLEAR_BITS:
> +        b &= ~value;
> +        break;
> +    case MMC_CMD6_ACCESS_WRITE_BYTE:
> +        b = value;
> +        break;
> +    }
> +
> +    if (index >= 192) {
> +        sd->card_status |= R_CSR_SWITCH_ERROR_MASK;
> +        return;
> +    }
> +
> +    sd->ext_csd[index] = b;
> +}

Re: [PATCH 21/32] hw/sd: Add mmc switch function support

[Cédric Le Goater]

On 6/13/24 12:49 AM, Philippe Mathieu-Daudé wrote:
> On 3/7/23 15:24, Cédric Le Goater wrote:
>> From: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>>
>> switch operation in mmc cards, updated the ext_csd register to
>> request changes in card operations. Here we implement similar
>> sequence but requests are mostly dummy and make no change.
>>
>> Implement SWITCH_ERROR if the write operation offset goes beyond length
>> of ext_csd.
>>
>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>> [ clg: - ported on SDProto framework ]
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   hw/sd/sd.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 52 insertions(+)
> 
> 
>> +static void mmc_function_switch(SDState *sd, uint32_t arg)
>> +{
>> +    uint32_t access = extract32(arg, 24, 2);
>> +    uint32_t index = extract32(arg, 16, 8);
>> +    uint32_t value = extract32(arg, 8, 8);
>> +    uint8_t b = sd->ext_csd[index];
> 
> This field is added in the next patch :)
> 
> ../../hw/sd/sd.c:927:21: error: no member named 'ext_csd' in 'struct SDState'
>      uint8_t b = sd->ext_csd[index];
>                  ~~  ^
> ../../hw/sd/sd.c:949:9: error: no member named 'ext_csd' in 'struct SDState'
>      sd->ext_csd[index] = b;
>      ~~  ^
> 
> No need to respin, as I'm integrating your work.


Ah good !

There are 3 main parts :

   * Base eMMC support:
     hw/sd: Basis for eMMC support
     hw/sd: Add emmc_cmd_SEND_OP_CMD() handler
     hw/sd: Add emmc_cmd_ALL_SEND_CID() handler
     hw/sd: Add emmc_cmd_SET_RELATIVE_ADDR() handler
     hw/sd: Add emmc_cmd_APP_CMD() handler
     hw/sd: add emmc_cmd_SEND_TUNING_BLOCK() handler
     hw/sd: Add CMD21 tuning sequence
     hw/sd: Add mmc switch function support
     hw/sd: Add emmc_cmd_SEND_EXT_CSD() handler

   * Boot area support
     hw/sd: Support boot area in emmc image
     hw/sd: Subtract bootarea size from blk
     hw/sd: Add boot config support
     hw/sd: Fix SET_BLOCK_COUNT command argument
     hw/sd: Update CMD1 definition for MMC

   * Aspeed eMMC support :
     hw/arm/aspeed: Add eMMC device
     hw/arm/aspeed: Load eMMC first boot area as a boot rom
     hw/arm/aspeed: Set boot device to emmc
     aspeed: Set bootconfig
     aspeed: Introduce a 'boot-emmc' property for AST2600 based machines

and I can rework the aspeed part if needed.

Here is an image you can try boot on :

   https://www.kaod.org/qemu/aspeed/rainier/mmc-p10bmc.qcow2

Run with :

   qemu-system-arm -M rainier-bmc -net nic,netdev=net0 -netdev user,id=net0 -drive file=./mmc-p10bmc.qcow2,format=qcow2,if=sd,id=sd2,index=2 -nographic -serial mon:stdio

Thanks,

C.
  

>> +    switch (access) {
>> +    case MMC_CMD6_ACCESS_COMMAND_SET:
>> +        qemu_log_mask(LOG_UNIMP, "MMC Command set switching not supported\n");
>> +        return;
>> +    case MMC_CMD6_ACCESS_SET_BITS:
>> +        b |= value;
>> +        break;
>> +    case MMC_CMD6_ACCESS_CLEAR_BITS:
>> +        b &= ~value;
>> +        break;
>> +    case MMC_CMD6_ACCESS_WRITE_BYTE:
>> +        b = value;
>> +        break;
>> +    }
>> +
>> +    if (index >= 192) {
>> +        sd->card_status |= R_CSR_SWITCH_ERROR_MASK;
>> +        return;
>> +    }
>> +
>> +    sd->ext_csd[index] = b;
>> +}
> 

Re: [PATCH 21/32] hw/sd: Add mmc switch function support

[Philippe Mathieu-Daudé]

On 13/6/24 09:44, Cédric Le Goater wrote:
> On 6/13/24 12:49 AM, Philippe Mathieu-Daudé wrote:
>> On 3/7/23 15:24, Cédric Le Goater wrote:
>>> From: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>>>
>>> switch operation in mmc cards, updated the ext_csd register to
>>> request changes in card operations. Here we implement similar
>>> sequence but requests are mostly dummy and make no change.
>>>
>>> Implement SWITCH_ERROR if the write operation offset goes beyond length
>>> of ext_csd.
>>>
>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>>> [ clg: - ported on SDProto framework ]
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>   hw/sd/sd.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 52 insertions(+)
>>
>>
>>> +static void mmc_function_switch(SDState *sd, uint32_t arg)
>>> +{
>>> +    uint32_t access = extract32(arg, 24, 2);
>>> +    uint32_t index = extract32(arg, 16, 8);
>>> +    uint32_t value = extract32(arg, 8, 8);
>>> +    uint8_t b = sd->ext_csd[index];
>>
>> This field is added in the next patch :)
>>
>> ../../hw/sd/sd.c:927:21: error: no member named 'ext_csd' in 'struct 
>> SDState'
>>      uint8_t b = sd->ext_csd[index];
>>                  ~~  ^
>> ../../hw/sd/sd.c:949:9: error: no member named 'ext_csd' in 'struct 
>> SDState'
>>      sd->ext_csd[index] = b;
>>      ~~  ^
>>
>> No need to respin, as I'm integrating your work.
> 
> 
> Ah good !
> 
> There are 3 main parts :
> 
>    * Base eMMC support:
>      hw/sd: Basis for eMMC support
>      hw/sd: Add emmc_cmd_SEND_OP_CMD() handler
>      hw/sd: Add emmc_cmd_ALL_SEND_CID() handler
>      hw/sd: Add emmc_cmd_SET_RELATIVE_ADDR() handler
>      hw/sd: Add emmc_cmd_APP_CMD() handler
>      hw/sd: add emmc_cmd_SEND_TUNING_BLOCK() handler
>      hw/sd: Add CMD21 tuning sequence
>      hw/sd: Add mmc switch function support
>      hw/sd: Add emmc_cmd_SEND_EXT_CSD() handler
> 
 >      hw/sd: Fix SET_BLOCK_COUNT command argument
 >      hw/sd: Update CMD1 definition for MMC

I am focusing on these for now and should include them
in the next SD pull request, but I'll repost them with
more patches to be reviewed first.

>    * Boot area support
>      hw/sd: Support boot area in emmc image
>      hw/sd: Subtract bootarea size from blk
>      hw/sd: Add boot config support

I haven't studied them yet and plan to look at them
after mentioned repost. I'll try to also include them
in the PR.

>    * Aspeed eMMC support :
>      hw/arm/aspeed: Add eMMC device
>      hw/arm/aspeed: Load eMMC first boot area as a boot rom
>      hw/arm/aspeed: Set boot device to emmc
>      aspeed: Set bootconfig
>      aspeed: Introduce a 'boot-emmc' property for AST2600 based machines

Once my PR posted I'll review them and let you merge them.
Then I'll invite you a beer at the next KVM forum :)

> and I can rework the aspeed part if needed.
> 
> Here is an image you can try boot on :
> 
>    https://www.kaod.org/qemu/aspeed/rainier/mmc-p10bmc.qcow2
> 
> Run with :
> 
>    qemu-system-arm -M rainier-bmc -net nic,netdev=net0 -netdev 
> user,id=net0 -drive 
> file=./mmc-p10bmc.qcow2,format=qcow2,if=sd,id=sd2,index=2 -nographic 
> -serial mon:stdio
> 
> Thanks,
> 
> C.

[PATCH 22/32] hw/sd: Add emmc_cmd_SEND_EXT_CSD() handler

[Cédric Le Goater]

The parameters mimick a real 4GB eMMC, but it can be set to various
sizes. Initially from Vincent Palatin <vpalatin@chromium.org>

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sdmmc-internal.h |  97 ++++++++++++++++++++++++++++++++++++
 include/hw/sd/sd.h     |   1 +
 hw/sd/sd.c             | 109 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 206 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h
index d8bf17d204fc..2b98f117cd8f 100644
--- a/hw/sd/sdmmc-internal.h
+++ b/hw/sd/sdmmc-internal.h
@@ -37,4 +37,101 @@ const char *sd_cmd_name(uint8_t cmd);
  */
 const char *sd_acmd_name(uint8_t cmd);
 
+/*
+ * EXT_CSD fields
+ */
+
+#define EXT_CSD_CMDQ_MODE_EN            15      /* R/W */
+#define EXT_CSD_FLUSH_CACHE             32      /* W */
+#define EXT_CSD_CACHE_CTRL              33      /* R/W */
+#define EXT_CSD_POWER_OFF_NOTIFICATION  34      /* R/W */
+#define EXT_CSD_PACKED_FAILURE_INDEX    35      /* RO */
+#define EXT_CSD_PACKED_CMD_STATUS       36      /* RO */
+#define EXT_CSD_EXP_EVENTS_STATUS       54      /* RO, 2 bytes */
+#define EXT_CSD_EXP_EVENTS_CTRL         56      /* R/W, 2 bytes */
+#define EXT_CSD_DATA_SECTOR_SIZE        61      /* R */
+#define EXT_CSD_GP_SIZE_MULT            143     /* R/W */
+#define EXT_CSD_PARTITION_SETTING_COMPLETED 155 /* R/W */
+#define EXT_CSD_PARTITION_ATTRIBUTE     156     /* R/W */
+#define EXT_CSD_PARTITION_SUPPORT       160     /* RO */
+#define EXT_CSD_HPI_MGMT                161     /* R/W */
+#define EXT_CSD_RST_N_FUNCTION          162     /* R/W */
+#define EXT_CSD_BKOPS_EN                163     /* R/W */
+#define EXT_CSD_BKOPS_START             164     /* W */
+#define EXT_CSD_SANITIZE_START          165     /* W */
+#define EXT_CSD_WR_REL_PARAM            166     /* RO */
+#define EXT_CSD_RPMB_MULT               168     /* RO */
+#define EXT_CSD_FW_CONFIG               169     /* R/W */
+#define EXT_CSD_BOOT_WP                 173     /* R/W */
+#define EXT_CSD_ERASE_GROUP_DEF         175     /* R/W */
+#define EXT_CSD_PART_CONFIG             179     /* R/W */
+#define EXT_CSD_ERASED_MEM_CONT         181     /* RO */
+#define EXT_CSD_BUS_WIDTH               183     /* R/W */
+#define EXT_CSD_STROBE_SUPPORT          184     /* RO */
+#define EXT_CSD_HS_TIMING               185     /* R/W */
+#define EXT_CSD_POWER_CLASS             187     /* R/W */
+#define EXT_CSD_REV                     192     /* RO */
+#define EXT_CSD_STRUCTURE               194     /* RO */
+#define EXT_CSD_CARD_TYPE               196     /* RO */
+#define EXT_CSD_DRIVER_STRENGTH         197     /* RO */
+#define EXT_CSD_OUT_OF_INTERRUPT_TIME   198     /* RO */
+#define EXT_CSD_PART_SWITCH_TIME        199     /* RO */
+#define EXT_CSD_PWR_CL_52_195           200     /* RO */
+#define EXT_CSD_PWR_CL_26_195           201     /* RO */
+#define EXT_CSD_PWR_CL_52_360           202     /* RO */
+#define EXT_CSD_PWR_CL_26_360           203     /* RO */
+#define EXT_CSD_SEC_CNT                 212     /* RO, 4 bytes */
+#define EXT_CSD_S_A_TIMEOUT             217     /* RO */
+#define EXT_CSD_S_C_VCCQ                219     /* RO */
+#define EXT_CSD_S_C_VCC                 220     /* RO */
+#define EXT_CSD_REL_WR_SEC_C            222     /* RO */
+#define EXT_CSD_HC_WP_GRP_SIZE          221     /* RO */
+#define EXT_CSD_ERASE_TIMEOUT_MULT      223     /* RO */
+#define EXT_CSD_HC_ERASE_GRP_SIZE       224     /* RO */
+#define EXT_CSD_ACC_SIZE                225     /* RO */
+#define EXT_CSD_BOOT_MULT               226     /* RO */
+#define EXT_CSD_BOOT_INFO               228     /* RO */
+#define EXT_CSD_SEC_TRIM_MULT           229     /* RO */
+#define EXT_CSD_SEC_ERASE_MULT          230     /* RO */
+#define EXT_CSD_SEC_FEATURE_SUPPORT     231     /* RO */
+#define EXT_CSD_TRIM_MULT               232     /* RO */
+#define EXT_CSD_PWR_CL_200_195          236     /* RO */
+#define EXT_CSD_PWR_CL_200_360          237     /* RO */
+#define EXT_CSD_PWR_CL_DDR_52_195       238     /* RO */
+#define EXT_CSD_PWR_CL_DDR_52_360       239     /* RO */
+#define EXT_CSD_BKOPS_STATUS            246     /* RO */
+#define EXT_CSD_POWER_OFF_LONG_TIME     247     /* RO */
+#define EXT_CSD_GENERIC_CMD6_TIME       248     /* RO */
+#define EXT_CSD_CACHE_SIZE              249     /* RO, 4 bytes */
+#define EXT_CSD_PWR_CL_DDR_200_360      253     /* RO */
+#define EXT_CSD_FIRMWARE_VERSION        254     /* RO, 8 bytes */
+#define EXT_CSD_PRE_EOL_INFO            267     /* RO */
+#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A      268     /* RO */
+#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B      269     /* RO */
+#define EXT_CSD_CMDQ_DEPTH              307     /* RO */
+#define EXT_CSD_CMDQ_SUPPORT            308     /* RO */
+#define EXT_CSD_SUPPORTED_MODE          493     /* RO */
+#define EXT_CSD_TAG_UNIT_SIZE           498     /* RO */
+#define EXT_CSD_DATA_TAG_SUPPORT        499     /* RO */
+#define EXT_CSD_MAX_PACKED_WRITES       500     /* RO */
+#define EXT_CSD_MAX_PACKED_READS        501     /* RO */
+#define EXT_CSD_BKOPS_SUPPORT           502     /* RO */
+#define EXT_CSD_HPI_FEATURES            503     /* RO */
+#define EXT_CSD_S_CMD_SET               504     /* RO */
+
+/*
+ * EXT_CSD field definitions
+ */
+
+#define EXT_CSD_WR_REL_PARAM_EN         (1 << 2)
+#define EXT_CSD_WR_REL_PARAM_EN_RPMB_REL_WR     (1 << 4)
+
+#define EXT_CSD_PART_CONFIG_ACC_MASK    (0x7)
+#define EXT_CSD_PART_CONFIG_ACC_DEFAULT (0x0)
+#define EXT_CSD_PART_CONFIG_ACC_BOOT0   (0x1)
+
+#define EXT_CSD_PART_CONFIG_EN_MASK     (0x7 << 3)
+#define EXT_CSD_PART_CONFIG_EN_BOOT0    (0x1 << 3)
+#define EXT_CSD_PART_CONFIG_EN_USER     (0x7 << 3)
+
 #endif
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index da97400469a0..1565cd2b353c 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -132,6 +132,7 @@ struct SDCardClass {
     bool (*get_readonly)(SDState *sd);
 
     const struct SDProto *proto;
+    void (*set_csd)(SDState *sd, uint64_t size);
 };
 
 #define TYPE_SD_BUS "sd-bus"
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 51e2254728a6..212658050441 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -141,6 +141,7 @@ struct SDState {
     uint64_t data_start;
     uint32_t data_offset;
     uint8_t data[512];
+    uint8_t ext_csd[512];
     qemu_irq readonly_cb;
     qemu_irq inserted_cb;
     QEMUTimer *ocr_power_timer;
@@ -414,8 +415,85 @@ static const uint8_t sd_csd_rw_mask[16] = {
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xfc, 0xfe,
 };
 
+static void mmc_set_ext_csd(SDState *sd, uint64_t size)
+{
+    uint32_t sectcount = size >> HWBLOCK_SHIFT;
+
+    memset(sd->ext_csd, 0, sizeof(sd->ext_csd));
+
+    sd->ext_csd[EXT_CSD_S_CMD_SET] = 0x1; /* supported command sets */
+    sd->ext_csd[EXT_CSD_HPI_FEATURES] = 0x3; /* HPI features  */
+    sd->ext_csd[EXT_CSD_BKOPS_SUPPORT] = 0x1; /* Background operations */
+    sd->ext_csd[241] = 0xA; /* 1st initialization time after partitioning */
+    sd->ext_csd[EXT_CSD_TRIM_MULT] = 0x1; /* Trim multiplier */
+    sd->ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] = 0x15; /* Secure feature */
+    sd->ext_csd[EXT_CSD_SEC_ERASE_MULT] = 0x96; /* Secure erase support */
+    sd->ext_csd[EXT_CSD_SEC_TRIM_MULT] = 0x96; /* Secure TRIM multiplier */
+    sd->ext_csd[EXT_CSD_BOOT_INFO] = 0x7; /* Boot information */
+    sd->ext_csd[EXT_CSD_BOOT_MULT] = 0x8; /* Boot partition size. 128KB unit */
+    sd->ext_csd[EXT_CSD_ACC_SIZE] = 0x6; /* Access size */
+    sd->ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] = 0x4; /* HC Erase unit size */
+    sd->ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] = 0x1; /* HC erase timeout */
+    sd->ext_csd[EXT_CSD_REL_WR_SEC_C] = 0x1; /* Reliable write sector count */
+    sd->ext_csd[EXT_CSD_HC_WP_GRP_SIZE] = 0x4; /* HC write protect group size */
+    sd->ext_csd[EXT_CSD_S_C_VCC] = 0x8; /* Sleep current VCC  */
+    sd->ext_csd[EXT_CSD_S_C_VCCQ] = 0x7; /* Sleep current VCCQ */
+    sd->ext_csd[EXT_CSD_S_A_TIMEOUT] = 0x11; /* Sleep/Awake timeout */
+    sd->ext_csd[215] = (sectcount >> 24) & 0xff; /* Sector count */
+    sd->ext_csd[214] = (sectcount >> 16) & 0xff; /* ... */
+    sd->ext_csd[213] = (sectcount >> 8) & 0xff;  /* ... */
+    sd->ext_csd[EXT_CSD_SEC_CNT] = (sectcount & 0xff);       /* ... */
+    sd->ext_csd[210] = 0xa; /* Min write perf for 8bit@52Mhz */
+    sd->ext_csd[209] = 0xa; /* Min read perf for 8bit@52Mhz  */
+    sd->ext_csd[208] = 0xa; /* Min write perf for 4bit@52Mhz */
+    sd->ext_csd[207] = 0xa; /* Min read perf for 4bit@52Mhz */
+    sd->ext_csd[206] = 0xa; /* Min write perf for 4bit@26Mhz */
+    sd->ext_csd[205] = 0xa; /* Min read perf for 4bit@26Mhz */
+    sd->ext_csd[EXT_CSD_PART_SWITCH_TIME] = 0x1;
+    sd->ext_csd[EXT_CSD_OUT_OF_INTERRUPT_TIME] = 0x1;
+    sd->ext_csd[EXT_CSD_CARD_TYPE] = 0x7;
+    sd->ext_csd[EXT_CSD_STRUCTURE] = 0x2;
+    sd->ext_csd[EXT_CSD_REV] = 0x5;
+    sd->ext_csd[EXT_CSD_RPMB_MULT] = 0x1; /* RPMB size */
+    sd->ext_csd[EXT_CSD_PARTITION_SUPPORT] = 0x3;
+    sd->ext_csd[159] = 0x00; /* Max enhanced area size */
+    sd->ext_csd[158] = 0x00; /* ... */
+    sd->ext_csd[157] = 0xEC; /* ... */
+}
+
+static void sd_emmc_set_csd(SDState *sd, uint64_t size)
+{
+    sd->csd[0] = 0xd0;
+    sd->csd[1] = 0x0f;
+    sd->csd[2] = 0x00;
+    sd->csd[3] = 0x32;
+    sd->csd[4] = 0x0f;
+    if (size <= 2 * GiB) {
+        /* use 1k blocks */
+        uint32_t csize1k = (size >> (CMULT_SHIFT + 10)) - 1;
+        sd->csd[5] = 0x5a;
+        sd->csd[6] = 0x80 | ((csize1k >> 10) & 0xf);
+        sd->csd[7] = (csize1k >> 2) & 0xff;
+    } else { /* >= 2GB : size stored in ext CSD, block addressing */
+        sd->csd[5] = 0x59;
+        sd->csd[6] = 0x8f;
+        sd->csd[7] = 0xff;
+        sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_CAPACITY, 1);
+    }
+    sd->csd[8] = 0xff;
+    sd->csd[9] = 0xff;
+    sd->csd[10] = 0xf7;
+    sd->csd[11] = 0xfe;
+    sd->csd[12] = 0x49;
+    sd->csd[13] = 0x10;
+    sd->csd[14] = 0x00;
+    sd->csd[15] = (sd_crc7(sd->csd, 15) << 1) | 1;
+    mmc_set_ext_csd(sd, size);
+}
+
 static void sd_set_csd(SDState *sd, uint64_t size)
 {
+    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
     int hwblock_shift = HWBLOCK_SHIFT;
     uint32_t csize;
     uint32_t sectsize = (1 << (SECTOR_SHIFT + 1)) - 1;
@@ -427,7 +505,9 @@ static void sd_set_csd(SDState *sd, uint64_t size)
     }
     csize = (size >> (CMULT_SHIFT + hwblock_shift)) - 1;
 
-    if (size <= SDSC_MAX_CAPACITY) { /* Standard Capacity SD */
+    if (sc->set_csd) {
+        sc->set_csd(sd, size);
+    } else if (size <= SDSC_MAX_CAPACITY) { /* Standard Capacity SD */
         sd->csd[0] = 0x00;      /* CSD structure */
         sd->csd[1] = 0x26;      /* Data read access-time-1 */
         sd->csd[2] = 0x00;      /* Data read access-time-2 */
@@ -2110,6 +2190,14 @@ uint8_t sd_read_byte(SDState *sd)
             sd->state = sd_transfer_state;
         break;
 
+    case 8:     /* CMD8: SEND_EXT_CSD on MMC */
+        ret = sd->data[sd->data_offset++];
+
+        if (sd->data_offset >= sizeof(sd->ext_csd)) {
+            sd->state = sd_transfer_state;
+        }
+        break;
+
     case 9:  /* CMD9:   SEND_CSD */
     case 10:  /* CMD10:  SEND_CID */
         ret = sd->data[sd->data_offset ++];
@@ -2285,6 +2373,23 @@ static sd_rsp_type_t emmc_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
     return sd_r2_i;
 }
 
+static sd_rsp_type_t emmc_cmd_SEND_EXT_CSD(SDState *sd, SDRequest req)
+{
+    uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : req.arg;
+
+    switch (sd->state) {
+    case sd_transfer_state:
+        /* MMC : Sends the EXT_CSD register as a Block of data */
+        sd->state = sd_sendingdata_state;
+        memcpy(sd->data, sd->ext_csd, sizeof(sd->ext_csd));
+        sd->data_start = addr;
+        sd->data_offset = 0;
+        return sd_r1;
+    default:
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+}
+
 static sd_rsp_type_t emmc_cmd_APP_CMD(SDState *sd, SDRequest req)
 {
     return sd_r0;
@@ -2323,6 +2428,7 @@ static const SDProto sd_proto_emmc = {
         [3]         = emmc_cmd_SEND_RELATIVE_ADDR,
         [5]         = sd_cmd_illegal,
         [6]         = emmc_cmd_SWITCH_FUNCTION,
+        [8]         = emmc_cmd_SEND_EXT_CSD,
         [19]        = sd_cmd_SEND_TUNING_BLOCK,
         [23]        = sd_cmd_SET_BLOCK_COUNT,
         [21]        = emmc_cmd_SEND_TUNING_BLOCK,
@@ -2490,6 +2596,7 @@ static void emmc_class_init(ObjectClass *klass, void *data)
     dc->desc = "eMMC";
     dc->realize = emmc_realize;
     sc->proto = &sd_proto_emmc;
+    sc->set_csd = sd_emmc_set_csd;
 }
 
 static const TypeInfo emmc_info = {
-- 
2.41.0

Re: [PATCH 22/32] hw/sd: Add emmc_cmd_SEND_EXT_CSD() handler

[Philippe Mathieu-Daudé]

Hi,

On 3/7/23 15:24, Cédric Le Goater wrote:
> The parameters mimick a real 4GB eMMC, but it can be set to various
> sizes. Initially from Vincent Palatin <vpalatin@chromium.org>
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sdmmc-internal.h |  97 ++++++++++++++++++++++++++++++++++++
>   include/hw/sd/sd.h     |   1 +
>   hw/sd/sd.c             | 109 ++++++++++++++++++++++++++++++++++++++++-
>   3 files changed, 206 insertions(+), 1 deletion(-)

First pass review, this will take time...

> +static void mmc_set_ext_csd(SDState *sd, uint64_t size)
> +{
> +    uint32_t sectcount = size >> HWBLOCK_SHIFT;
> +
> +    memset(sd->ext_csd, 0, sizeof(sd->ext_csd));
> +
> +    sd->ext_csd[EXT_CSD_S_CMD_SET] = 0x1; /* supported command sets */
> +    sd->ext_csd[EXT_CSD_HPI_FEATURES] = 0x3; /* HPI features  */
> +    sd->ext_csd[EXT_CSD_BKOPS_SUPPORT] = 0x1; /* Background operations */
> +    sd->ext_csd[241] = 0xA; /* 1st initialization time after partitioning */
> +    sd->ext_csd[EXT_CSD_TRIM_MULT] = 0x1; /* Trim multiplier */
> +    sd->ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] = 0x15; /* Secure feature */

We do not support (and are not interested in) that. I'll use 0x0 for
"do not support".

> +    sd->ext_csd[EXT_CSD_SEC_ERASE_MULT] = 0x96; /* Secure erase support */

This value is obsolete, so I'd use 0x0 to avoid confusions.

> +    sd->ext_csd[EXT_CSD_SEC_TRIM_MULT] = 0x96; /* Secure TRIM multiplier */

Again, 0x0 for "not defined".

> +    sd->ext_csd[EXT_CSD_BOOT_INFO] = 0x7; /* Boot information */
> +    sd->ext_csd[EXT_CSD_BOOT_MULT] = 0x8; /* Boot partition size. 128KB unit */
> +    sd->ext_csd[EXT_CSD_ACC_SIZE] = 0x6; /* Access size */

16KB of super_page_size hmm. Simpler could be the underlying block
retrieved with bdrv_nb_sectors() or simply BDRV_SECTOR_SIZE (0x1).

> +    sd->ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] = 0x4; /* HC Erase unit size */

2MB of erase size hmmm why not.

> +    sd->ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] = 0x1; /* HC erase timeout */

We don't implement timeout, can we use 0?

> +    sd->ext_csd[EXT_CSD_REL_WR_SEC_C] = 0x1; /* Reliable write sector count */
> +    sd->ext_csd[EXT_CSD_HC_WP_GRP_SIZE] = 0x4; /* HC write protect group size */
> +    sd->ext_csd[EXT_CSD_S_C_VCC] = 0x8; /* Sleep current VCC  */
> +    sd->ext_csd[EXT_CSD_S_C_VCCQ] = 0x7; /* Sleep current VCCQ */
> +    sd->ext_csd[EXT_CSD_S_A_TIMEOUT] = 0x11; /* Sleep/Awake timeout */
> +    sd->ext_csd[215] = (sectcount >> 24) & 0xff; /* Sector count */
> +    sd->ext_csd[214] = (sectcount >> 16) & 0xff; /* ... */
> +    sd->ext_csd[213] = (sectcount >> 8) & 0xff;  /* ... */
> +    sd->ext_csd[EXT_CSD_SEC_CNT] = (sectcount & 0xff);       /* ... */
> +    sd->ext_csd[210] = 0xa; /* Min write perf for 8bit@52Mhz */
> +    sd->ext_csd[209] = 0xa; /* Min read perf for 8bit@52Mhz  */
> +    sd->ext_csd[208] = 0xa; /* Min write perf for 4bit@52Mhz */
> +    sd->ext_csd[207] = 0xa; /* Min read perf for 4bit@52Mhz */
> +    sd->ext_csd[206] = 0xa; /* Min write perf for 4bit@26Mhz */
> +    sd->ext_csd[205] = 0xa; /* Min read perf for 4bit@26Mhz */

Class B at 3MB/s. I suppose announcing up to J at 21MB/s is safe (0x46).

> +    sd->ext_csd[EXT_CSD_PART_SWITCH_TIME] = 0x1;

SWITCH command isn't implemented so far. We could use 0x0 for "not
defined".

> +    sd->ext_csd[EXT_CSD_OUT_OF_INTERRUPT_TIME] = 0x1;

Similarly, 0x0 for "undefined" is legal.

> +    sd->ext_csd[EXT_CSD_CARD_TYPE] = 0x7;

You anounce dual data rate. Could we just use High-Speed mode (0x3)
to ease modelling?

> +    sd->ext_csd[EXT_CSD_STRUCTURE] = 0x2;
> +    sd->ext_csd[EXT_CSD_REV] = 0x5;

This is Revision 1.5 (for MMC v4.41)... The first QEMU implementation
was based on Revision 1.3 (for MMC v4.3) and I'm seeing some features
from Revision 1.6 (for MMC v4.5)...

Do we want to implement all of them? Since we are adding from
scratch, I suggest we directly start with v4.5 (0x6).

Note, EXT_CSD_BUS_WIDTH is not set (0x0) meaning 1-bit data bus.
I'd set it to 0x2 (8-bit):

        sd->ext_csd[EXT_CSD_BUS_WIDTH] = EXT_CSD_BUS_WIDTH_8_MASK;

> +    sd->ext_csd[EXT_CSD_RPMB_MULT] = 0x1; /* RPMB size */
> +    sd->ext_csd[EXT_CSD_PARTITION_SUPPORT] = 0x3;
> +    sd->ext_csd[159] = 0x00; /* Max enhanced area size */
> +    sd->ext_csd[158] = 0x00; /* ... */
> +    sd->ext_csd[157] = 0xEC; /* ... */
> +}

Re: [PATCH 22/32] hw/sd: Add emmc_cmd_SEND_EXT_CSD() handler

[Cédric Le Goater]

Hello

On 6/19/24 7:40 PM, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> On 3/7/23 15:24, Cédric Le Goater wrote:
>> The parameters mimick a real 4GB eMMC, but it can be set to various
>> sizes. Initially from Vincent Palatin <vpalatin@chromium.org>
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   hw/sd/sdmmc-internal.h |  97 ++++++++++++++++++++++++++++++++++++
>>   include/hw/sd/sd.h     |   1 +
>>   hw/sd/sd.c             | 109 ++++++++++++++++++++++++++++++++++++++++-
>>   3 files changed, 206 insertions(+), 1 deletion(-)
> 
> First pass review, this will take time...
> 
>> +static void mmc_set_ext_csd(SDState *sd, uint64_t size)
>> +{
>> +    uint32_t sectcount = size >> HWBLOCK_SHIFT;
>> +
>> +    memset(sd->ext_csd, 0, sizeof(sd->ext_csd));
>> +
>> +    sd->ext_csd[EXT_CSD_S_CMD_SET] = 0x1; /* supported command sets */
>> +    sd->ext_csd[EXT_CSD_HPI_FEATURES] = 0x3; /* HPI features  */
>> +    sd->ext_csd[EXT_CSD_BKOPS_SUPPORT] = 0x1; /* Background operations */
>> +    sd->ext_csd[241] = 0xA; /* 1st initialization time after partitioning */
>> +    sd->ext_csd[EXT_CSD_TRIM_MULT] = 0x1; /* Trim multiplier */
>> +    sd->ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] = 0x15; /* Secure feature */
> 
> We do not support (and are not interested in) that. I'll use 0x0 for
> "do not support".
> 
>> +    sd->ext_csd[EXT_CSD_SEC_ERASE_MULT] = 0x96; /* Secure erase support */
> 
> This value is obsolete, so I'd use 0x0 to avoid confusions.
> 
>> +    sd->ext_csd[EXT_CSD_SEC_TRIM_MULT] = 0x96; /* Secure TRIM multiplier */
> 
> Again, 0x0 for "not defined".
> 
>> +    sd->ext_csd[EXT_CSD_BOOT_INFO] = 0x7; /* Boot information */
>> +    sd->ext_csd[EXT_CSD_BOOT_MULT] = 0x8; /* Boot partition size. 128KB unit */
>> +    sd->ext_csd[EXT_CSD_ACC_SIZE] = 0x6; /* Access size */
> 
> 16KB of super_page_size hmm. Simpler could be the underlying block
> retrieved with bdrv_nb_sectors() or simply BDRV_SECTOR_SIZE (0x1).
> 
>> +    sd->ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] = 0x4; /* HC Erase unit size */
> 
> 2MB of erase size hmmm why not.
> 
>> +    sd->ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] = 0x1; /* HC erase timeout */
> 
> We don't implement timeout, can we use 0?
> 
>> +    sd->ext_csd[EXT_CSD_REL_WR_SEC_C] = 0x1; /* Reliable write sector count */
>> +    sd->ext_csd[EXT_CSD_HC_WP_GRP_SIZE] = 0x4; /* HC write protect group size */
>> +    sd->ext_csd[EXT_CSD_S_C_VCC] = 0x8; /* Sleep current VCC  */
>> +    sd->ext_csd[EXT_CSD_S_C_VCCQ] = 0x7; /* Sleep current VCCQ */
>> +    sd->ext_csd[EXT_CSD_S_A_TIMEOUT] = 0x11; /* Sleep/Awake timeout */
>> +    sd->ext_csd[215] = (sectcount >> 24) & 0xff; /* Sector count */
>> +    sd->ext_csd[214] = (sectcount >> 16) & 0xff; /* ... */
>> +    sd->ext_csd[213] = (sectcount >> 8) & 0xff;  /* ... */
>> +    sd->ext_csd[EXT_CSD_SEC_CNT] = (sectcount & 0xff);       /* ... */
>> +    sd->ext_csd[210] = 0xa; /* Min write perf for 8bit@52Mhz */
>> +    sd->ext_csd[209] = 0xa; /* Min read perf for 8bit@52Mhz  */
>> +    sd->ext_csd[208] = 0xa; /* Min write perf for 4bit@52Mhz */
>> +    sd->ext_csd[207] = 0xa; /* Min read perf for 4bit@52Mhz */
>> +    sd->ext_csd[206] = 0xa; /* Min write perf for 4bit@26Mhz */
>> +    sd->ext_csd[205] = 0xa; /* Min read perf for 4bit@26Mhz */
> 
> Class B at 3MB/s. I suppose announcing up to J at 21MB/s is safe (0x46).
> 
>> +    sd->ext_csd[EXT_CSD_PART_SWITCH_TIME] = 0x1;
> 
> SWITCH command isn't implemented so far. We could use 0x0 for "not
> defined".
> 
>> +    sd->ext_csd[EXT_CSD_OUT_OF_INTERRUPT_TIME] = 0x1;
> 
> Similarly, 0x0 for "undefined" is legal.
> 
>> +    sd->ext_csd[EXT_CSD_CARD_TYPE] = 0x7;
> 
> You anounce dual data rate. Could we just use High-Speed mode (0x3)
> to ease modelling?
> 
>> +    sd->ext_csd[EXT_CSD_STRUCTURE] = 0x2;
>> +    sd->ext_csd[EXT_CSD_REV] = 0x5;
> 
> This is Revision 1.5 (for MMC v4.41)... The first QEMU implementation
> was based on Revision 1.3 (for MMC v4.3) and I'm seeing some features
> from Revision 1.6 (for MMC v4.5)...
> 
> Do we want to implement all of them? Since we are adding from
> scratch, I suggest we directly start with v4.5 (0x6).
> 
> Note, EXT_CSD_BUS_WIDTH is not set (0x0) meaning 1-bit data bus.
> I'd set it to 0x2 (8-bit):
> 
>         sd->ext_csd[EXT_CSD_BUS_WIDTH] = EXT_CSD_BUS_WIDTH_8_MASK;


I applied the proposed changes from above and the rainier-bmc boots fine.
Here are the mmc related logs :


   U-Boot SPL 2019.04 (Jun 17 2024 - 07:49:13 +0000)
   Trying to boot from MMC1
   
   
   U-Boot 2019.04 (Jun 17 2024 - 07:49:13 +0000)
   
   SOC: AST2600-A3
   eMMC 2nd Boot (ABR): Enable, boot partition: 1
   LPC Mode: SIO:Disable
   Eth: MAC0: RMII/NCSI, MAC1: RMII/NCSI, MAC2: RMII/NCSI, MAC3: RMII/NCSI
   Model: IBM P10 BMC
   DRAM:  already initialized, 896 MiB (capacity:1024 MiB, VGA:64 MiB, ECC:on, ECC size:896 MiB)
   MMC:   emmc_slot0@100: 0
   Loading Environment from MMC... OK
   In:    serial@1e784000
   Out:   serial@1e784000
   Err:   serial@1e784000
   Model: IBM P10 BMC
   Net:   No MDIO found.
   ftgmac100_probe - NCSI detected
   
   ...
   
   [    0.640650] mmc0: SDHCI controller on 1e750100.sdhci [1e750100.sdhci] using ADMA
   [    0.658402] mmc0: unspecified timeout for CMD6 - use generic
   [    0.659014] mmc0: unspecified timeout for CMD6 - use generic
   [    0.659314] mmc0: unspecified timeout for CMD6 - use generic
   [    0.659722] mmc0: unspecified timeout for CMD6 - use generic
   [    0.660740] mmc0: unspecified timeout for CMD6 - use generic
   [    0.661139] mmc0: new high speed MMC card at address 0001
   [    0.662825] mmcblk0: mmc0:0001 QEMU! 16.0 GiB
   [    0.688329]  mmcblk0: p1 p2 p3 p4 p5 p6 p7
   [    0.692837] mmcblk0boot0: mmc0:0001 QEMU! 1.00 MiB
   [    0.694416] mmcblk0boot1: mmc0:0001 QEMU! 1.00 MiB
   [    0.695166] mmcblk0rpmb: mmc0:0001 QEMU! 128 KiB, chardev (243:0)
   [    2.455427]  mmcblk0: p1 p2 p3 p4 p5 p6 p7
   [    7.624272] EXT4-fs (mmcblk0p4): orphan cleanup on readonly fs
   [    7.624837] EXT4-fs (mmcblk0p4): mounted filesystem 6f526507-e73b-4094-8f08-f310b5da5b3a ro with ordered data mode. Quota mode: disabled.
   [    8.024897] EXT4-fs (mmcblk0p6): mounted filesystem 6dc9b0da-2b0f-4822-9eac-df4dd782ddfc r/w with ordered data mode. Quota mode: disabled.
   [   15.991016] EXT4-fs (mmcblk0p4): re-mounted 6f526507-e73b-4094-8f08-f310b5da5b3a ro. Quota mode: disabled.
   

I think these initial values are fine to start with.

Thanks,

C.



>> +    sd->ext_csd[EXT_CSD_RPMB_MULT] = 0x1; /* RPMB size */
>> +    sd->ext_csd[EXT_CSD_PARTITION_SUPPORT] = 0x3;
>> +    sd->ext_csd[159] = 0x00; /* Max enhanced area size */
>> +    sd->ext_csd[158] = 0x00; /* ... */
>> +    sd->ext_csd[157] = 0xEC; /* ... */
>> +}
> 

Re: [PATCH 22/32] hw/sd: Add emmc_cmd_SEND_EXT_CSD() handler

[Philippe Mathieu-Daudé]

On 20/6/24 09:23, Cédric Le Goater wrote:
> Hello
> 
> On 6/19/24 7:40 PM, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> On 3/7/23 15:24, Cédric Le Goater wrote:
>>> The parameters mimick a real 4GB eMMC, but it can be set to various
>>> sizes. Initially from Vincent Palatin <vpalatin@chromium.org>
>>>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>   hw/sd/sdmmc-internal.h |  97 ++++++++++++++++++++++++++++++++++++
>>>   include/hw/sd/sd.h     |   1 +
>>>   hw/sd/sd.c             | 109 ++++++++++++++++++++++++++++++++++++++++-
>>>   3 files changed, 206 insertions(+), 1 deletion(-)
>>
>> First pass review, this will take time...
>>
>>> +static void mmc_set_ext_csd(SDState *sd, uint64_t size)
>>> +{
>>> +    uint32_t sectcount = size >> HWBLOCK_SHIFT;
>>> +
>>> +    memset(sd->ext_csd, 0, sizeof(sd->ext_csd));
>>> +
>>> +    sd->ext_csd[EXT_CSD_S_CMD_SET] = 0x1; /* supported command sets */
>>> +    sd->ext_csd[EXT_CSD_HPI_FEATURES] = 0x3; /* HPI features  */
>>> +    sd->ext_csd[EXT_CSD_BKOPS_SUPPORT] = 0x1; /* Background 
>>> operations */
>>> +    sd->ext_csd[241] = 0xA; /* 1st initialization time after 
>>> partitioning */
>>> +    sd->ext_csd[EXT_CSD_TRIM_MULT] = 0x1; /* Trim multiplier */
>>> +    sd->ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] = 0x15; /* Secure 
>>> feature */
>>
>> We do not support (and are not interested in) that. I'll use 0x0 for
>> "do not support".
>>
>>> +    sd->ext_csd[EXT_CSD_SEC_ERASE_MULT] = 0x96; /* Secure erase 
>>> support */
>>
>> This value is obsolete, so I'd use 0x0 to avoid confusions.
>>
>>> +    sd->ext_csd[EXT_CSD_SEC_TRIM_MULT] = 0x96; /* Secure TRIM 
>>> multiplier */
>>
>> Again, 0x0 for "not defined".
>>
>>> +    sd->ext_csd[EXT_CSD_BOOT_INFO] = 0x7; /* Boot information */
>>> +    sd->ext_csd[EXT_CSD_BOOT_MULT] = 0x8; /* Boot partition size. 
>>> 128KB unit */
>>> +    sd->ext_csd[EXT_CSD_ACC_SIZE] = 0x6; /* Access size */
>>
>> 16KB of super_page_size hmm. Simpler could be the underlying block
>> retrieved with bdrv_nb_sectors() or simply BDRV_SECTOR_SIZE (0x1).
>>
>>> +    sd->ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] = 0x4; /* HC Erase unit 
>>> size */
>>
>> 2MB of erase size hmmm why not.
>>
>>> +    sd->ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] = 0x1; /* HC erase 
>>> timeout */
>>
>> We don't implement timeout, can we use 0?
>>
>>> +    sd->ext_csd[EXT_CSD_REL_WR_SEC_C] = 0x1; /* Reliable write 
>>> sector count */
>>> +    sd->ext_csd[EXT_CSD_HC_WP_GRP_SIZE] = 0x4; /* HC write protect 
>>> group size */
>>> +    sd->ext_csd[EXT_CSD_S_C_VCC] = 0x8; /* Sleep current VCC  */
>>> +    sd->ext_csd[EXT_CSD_S_C_VCCQ] = 0x7; /* Sleep current VCCQ */
>>> +    sd->ext_csd[EXT_CSD_S_A_TIMEOUT] = 0x11; /* Sleep/Awake timeout */
>>> +    sd->ext_csd[215] = (sectcount >> 24) & 0xff; /* Sector count */
>>> +    sd->ext_csd[214] = (sectcount >> 16) & 0xff; /* ... */
>>> +    sd->ext_csd[213] = (sectcount >> 8) & 0xff;  /* ... */
>>> +    sd->ext_csd[EXT_CSD_SEC_CNT] = (sectcount & 0xff);       /* ... */
>>> +    sd->ext_csd[210] = 0xa; /* Min write perf for 8bit@52Mhz */
>>> +    sd->ext_csd[209] = 0xa; /* Min read perf for 8bit@52Mhz  */
>>> +    sd->ext_csd[208] = 0xa; /* Min write perf for 4bit@52Mhz */
>>> +    sd->ext_csd[207] = 0xa; /* Min read perf for 4bit@52Mhz */
>>> +    sd->ext_csd[206] = 0xa; /* Min write perf for 4bit@26Mhz */
>>> +    sd->ext_csd[205] = 0xa; /* Min read perf for 4bit@26Mhz */
>>
>> Class B at 3MB/s. I suppose announcing up to J at 21MB/s is safe (0x46).
>>
>>> +    sd->ext_csd[EXT_CSD_PART_SWITCH_TIME] = 0x1;
>>
>> SWITCH command isn't implemented so far. We could use 0x0 for "not
>> defined".
>>
>>> +    sd->ext_csd[EXT_CSD_OUT_OF_INTERRUPT_TIME] = 0x1;
>>
>> Similarly, 0x0 for "undefined" is legal.
>>
>>> +    sd->ext_csd[EXT_CSD_CARD_TYPE] = 0x7;
>>
>> You anounce dual data rate. Could we just use High-Speed mode (0x3)
>> to ease modelling?
>>
>>> +    sd->ext_csd[EXT_CSD_STRUCTURE] = 0x2;
>>> +    sd->ext_csd[EXT_CSD_REV] = 0x5;
>>
>> This is Revision 1.5 (for MMC v4.41)... The first QEMU implementation
>> was based on Revision 1.3 (for MMC v4.3) and I'm seeing some features
>> from Revision 1.6 (for MMC v4.5)...
>>
>> Do we want to implement all of them? Since we are adding from
>> scratch, I suggest we directly start with v4.5 (0x6).
>>
>> Note, EXT_CSD_BUS_WIDTH is not set (0x0) meaning 1-bit data bus.
>> I'd set it to 0x2 (8-bit):
>>
>>         sd->ext_csd[EXT_CSD_BUS_WIDTH] = EXT_CSD_BUS_WIDTH_8_MASK;
> 
> 
> I applied the proposed changes from above and the rainier-bmc boots fine.
> Here are the mmc related logs :
> 
> 
>    U-Boot SPL 2019.04 (Jun 17 2024 - 07:49:13 +0000)
>    Trying to boot from MMC1
>    U-Boot 2019.04 (Jun 17 2024 - 07:49:13 +0000)
>    SOC: AST2600-A3
>    eMMC 2nd Boot (ABR): Enable, boot partition: 1
>    LPC Mode: SIO:Disable
>    Eth: MAC0: RMII/NCSI, MAC1: RMII/NCSI, MAC2: RMII/NCSI, MAC3: RMII/NCSI
>    Model: IBM P10 BMC
>    DRAM:  already initialized, 896 MiB (capacity:1024 MiB, VGA:64 MiB, 
> ECC:on, ECC size:896 MiB)
>    MMC:   emmc_slot0@100: 0
>    Loading Environment from MMC... OK
>    In:    serial@1e784000
>    Out:   serial@1e784000
>    Err:   serial@1e784000
>    Model: IBM P10 BMC
>    Net:   No MDIO found.
>    ftgmac100_probe - NCSI detected
>    ...
>    [    0.640650] mmc0: SDHCI controller on 1e750100.sdhci 
> [1e750100.sdhci] using ADMA
>    [    0.658402] mmc0: unspecified timeout for CMD6 - use generic
>    [    0.659014] mmc0: unspecified timeout for CMD6 - use generic
>    [    0.659314] mmc0: unspecified timeout for CMD6 - use generic
>    [    0.659722] mmc0: unspecified timeout for CMD6 - use generic
>    [    0.660740] mmc0: unspecified timeout for CMD6 - use generic
>    [    0.661139] mmc0: new high speed MMC card at address 0001
>    [    0.662825] mmcblk0: mmc0:0001 QEMU! 16.0 GiB
>    [    0.688329]  mmcblk0: p1 p2 p3 p4 p5 p6 p7
>    [    0.692837] mmcblk0boot0: mmc0:0001 QEMU! 1.00 MiB
>    [    0.694416] mmcblk0boot1: mmc0:0001 QEMU! 1.00 MiB
>    [    0.695166] mmcblk0rpmb: mmc0:0001 QEMU! 128 KiB, chardev (243:0)
>    [    2.455427]  mmcblk0: p1 p2 p3 p4 p5 p6 p7
>    [    7.624272] EXT4-fs (mmcblk0p4): orphan cleanup on readonly fs
>    [    7.624837] EXT4-fs (mmcblk0p4): mounted filesystem 
> 6f526507-e73b-4094-8f08-f310b5da5b3a ro with ordered data mode. Quota 
> mode: disabled.
>    [    8.024897] EXT4-fs (mmcblk0p6): mounted filesystem 
> 6dc9b0da-2b0f-4822-9eac-df4dd782ddfc r/w with ordered data mode. Quota 
> mode: disabled.
>    [   15.991016] EXT4-fs (mmcblk0p4): re-mounted 
> 6f526507-e73b-4094-8f08-f310b5da5b3a ro. Quota mode: disabled.
> 
> I think these initial values are fine to start with.

Great! Thank you for testing them :)

Re: [PATCH 22/32] hw/sd: Add emmc_cmd_SEND_EXT_CSD() handler

[Philippe Mathieu-Daudé]

On 3/7/23 15:24, Cédric Le Goater wrote:
> The parameters mimick a real 4GB eMMC, but it can be set to various
> sizes. Initially from Vincent Palatin <vpalatin@chromium.org>
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sdmmc-internal.h |  97 ++++++++++++++++++++++++++++++++++++
>   include/hw/sd/sd.h     |   1 +
>   hw/sd/sd.c             | 109 ++++++++++++++++++++++++++++++++++++++++-
>   3 files changed, 206 insertions(+), 1 deletion(-)


> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 51e2254728a6..212658050441 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -141,6 +141,7 @@ struct SDState {
>       uint64_t data_start;
>       uint32_t data_offset;
>       uint8_t data[512];
> +    uint8_t ext_csd[512];

Since the SWITCH command writes to EXT_CSD, this array must be
migrated.

>       qemu_irq readonly_cb;
>       qemu_irq inserted_cb;
>       QEMUTimer *ocr_power_timer;
> @@ -414,8 +415,85 @@ static const uint8_t sd_csd_rw_mask[16] = {
>       0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xfc, 0xfe,
>   };
>   
> +static void mmc_set_ext_csd(SDState *sd, uint64_t size)
> +{
> +    uint32_t sectcount = size >> HWBLOCK_SHIFT;
> +
> +    memset(sd->ext_csd, 0, sizeof(sd->ext_csd));
> +
> +    sd->ext_csd[EXT_CSD_S_CMD_SET] = 0x1; /* supported command sets */
> +    sd->ext_csd[EXT_CSD_HPI_FEATURES] = 0x3; /* HPI features  */
> +    sd->ext_csd[EXT_CSD_BKOPS_SUPPORT] = 0x1; /* Background operations */
> +    sd->ext_csd[241] = 0xA; /* 1st initialization time after partitioning */
> +    sd->ext_csd[EXT_CSD_TRIM_MULT] = 0x1; /* Trim multiplier */
> +    sd->ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] = 0x15; /* Secure feature */
> +    sd->ext_csd[EXT_CSD_SEC_ERASE_MULT] = 0x96; /* Secure erase support */
> +    sd->ext_csd[EXT_CSD_SEC_TRIM_MULT] = 0x96; /* Secure TRIM multiplier */
> +    sd->ext_csd[EXT_CSD_BOOT_INFO] = 0x7; /* Boot information */
> +    sd->ext_csd[EXT_CSD_BOOT_MULT] = 0x8; /* Boot partition size. 128KB unit */
> +    sd->ext_csd[EXT_CSD_ACC_SIZE] = 0x6; /* Access size */
> +    sd->ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] = 0x4; /* HC Erase unit size */
> +    sd->ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] = 0x1; /* HC erase timeout */
> +    sd->ext_csd[EXT_CSD_REL_WR_SEC_C] = 0x1; /* Reliable write sector count */
> +    sd->ext_csd[EXT_CSD_HC_WP_GRP_SIZE] = 0x4; /* HC write protect group size */
> +    sd->ext_csd[EXT_CSD_S_C_VCC] = 0x8; /* Sleep current VCC  */
> +    sd->ext_csd[EXT_CSD_S_C_VCCQ] = 0x7; /* Sleep current VCCQ */
> +    sd->ext_csd[EXT_CSD_S_A_TIMEOUT] = 0x11; /* Sleep/Awake timeout */
> +    sd->ext_csd[215] = (sectcount >> 24) & 0xff; /* Sector count */
> +    sd->ext_csd[214] = (sectcount >> 16) & 0xff; /* ... */
> +    sd->ext_csd[213] = (sectcount >> 8) & 0xff;  /* ... */
> +    sd->ext_csd[EXT_CSD_SEC_CNT] = (sectcount & 0xff);       /* ... */
> +    sd->ext_csd[210] = 0xa; /* Min write perf for 8bit@52Mhz */
> +    sd->ext_csd[209] = 0xa; /* Min read perf for 8bit@52Mhz  */
> +    sd->ext_csd[208] = 0xa; /* Min write perf for 4bit@52Mhz */
> +    sd->ext_csd[207] = 0xa; /* Min read perf for 4bit@52Mhz */
> +    sd->ext_csd[206] = 0xa; /* Min write perf for 4bit@26Mhz */
> +    sd->ext_csd[205] = 0xa; /* Min read perf for 4bit@26Mhz */
> +    sd->ext_csd[EXT_CSD_PART_SWITCH_TIME] = 0x1;
> +    sd->ext_csd[EXT_CSD_OUT_OF_INTERRUPT_TIME] = 0x1;
> +    sd->ext_csd[EXT_CSD_CARD_TYPE] = 0x7;
> +    sd->ext_csd[EXT_CSD_STRUCTURE] = 0x2;
> +    sd->ext_csd[EXT_CSD_REV] = 0x5;
> +    sd->ext_csd[EXT_CSD_RPMB_MULT] = 0x1; /* RPMB size */
> +    sd->ext_csd[EXT_CSD_PARTITION_SUPPORT] = 0x3;
> +    sd->ext_csd[159] = 0x00; /* Max enhanced area size */
> +    sd->ext_csd[158] = 0x00; /* ... */
> +    sd->ext_csd[157] = 0xEC; /* ... */
> +}

[PATCH 23/32] hw/sd: Support boot area in emmc image

[Cédric Le Goater]

From: Joel Stanley <joel@jms.id.au>

This assumes a specially constructued image:

  dd if=/dev/zero of=mmc-bootarea.img count=2 bs=1M
  dd if=u-boot-spl.bin of=mmc-bootarea.img conv=notrunc
  dd if=u-boot.bin of=mmc-bootarea.img conv=notrunc count=64 bs=1K
  cat mmc-bootarea.img obmc-phosphor-image.wic > mmc.img
  truncate --size 16GB mmc.img
  truncate --size 128MB mmc-bootarea.img

For now this still requires a mtd image to load the SPL:

  qemu-system-arm -M tacoma-bmc -nographic \
   -global driver=sd-card,property=emmc,value=true \
   -drive file=mmc.img,if=sd,index=2 \
   -drive file=mmc-bootarea.img,if=mtd,format=raw

Signed-off-by: Joel Stanley <joel@jms.id.au>
[clg: - definition renames
      - Introduced bootpart_offset
      - Introduced sd_boot_capacity_bytes() helper (Philippe) ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/sd/sd.h |  1 +
 hw/sd/sd.c         | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 1565cd2b353c..619f1852384c 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -133,6 +133,7 @@ struct SDCardClass {
 
     const struct SDProto *proto;
     void (*set_csd)(SDState *sd, uint64_t size);
+    uint32_t (*bootpart_offset)(SDState *sd);
 };
 
 #define TYPE_SD_BUS "sd-bus"
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 212658050441..6da17a8d0972 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -665,6 +665,12 @@ static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
     return addr >> (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT);
 }
 
+
+static unsigned sd_boot_capacity_bytes(SDState *sd)
+{
+    return sd->ext_csd[EXT_CSD_BOOT_MULT] << 17;
+}
+
 static void sd_reset(DeviceState *dev)
 {
     SDState *sd = SD_CARD(dev);
@@ -862,9 +868,40 @@ void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
     qemu_set_irq(insert, sd->blk ? blk_is_inserted(sd->blk) : 0);
 }
 
+/*
+ * This requires a disk image that has two boot partitions inserted at the
+ * beginning of it. The size of the boot partitions are configured in the
+ * ext_csd structure, which is hardcoded in qemu. They are currently set to
+ * 1MB each.
+ */
+static uint32_t sd_emmc_bootpart_offset(SDState *sd)
+{
+    unsigned int access = sd->ext_csd[EXT_CSD_PART_CONFIG] &
+        EXT_CSD_PART_CONFIG_ACC_MASK;
+    unsigned int boot_capacity = sd_boot_capacity_bytes(sd);
+
+    switch (access) {
+    case EXT_CSD_PART_CONFIG_ACC_DEFAULT:
+        return boot_capacity * 2;
+    case EXT_CSD_PART_CONFIG_ACC_BOOT0:
+        return 0;
+    case EXT_CSD_PART_CONFIG_ACC_BOOT0 + 1:
+        return boot_capacity * 1;
+    default:
+         g_assert_not_reached();
+    }
+}
+
+static uint32_t sd_bootpart_offset(SDState *sd)
+{
+    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
+    return sc->bootpart_offset ? sc->bootpart_offset(sd) : 0;
+}
+
 static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len)
 {
     trace_sdcard_read_block(addr, len);
+    addr += sd_bootpart_offset(sd);
     if (!sd->blk || blk_pread(sd->blk, addr, len, sd->data, 0) < 0) {
         fprintf(stderr, "sd_blk_read: read error on host side\n");
     }
@@ -873,6 +910,7 @@ static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len)
 static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
 {
     trace_sdcard_write_block(addr, len);
+    addr += sd_bootpart_offset(sd);
     if (!sd->blk || blk_pwrite(sd->blk, addr, len, sd->data, 0) < 0) {
         fprintf(stderr, "sd_blk_write: write error on host side\n");
     }
@@ -2597,6 +2635,7 @@ static void emmc_class_init(ObjectClass *klass, void *data)
     dc->realize = emmc_realize;
     sc->proto = &sd_proto_emmc;
     sc->set_csd = sd_emmc_set_csd;
+    sc->bootpart_offset = sd_emmc_bootpart_offset;
 }
 
 static const TypeInfo emmc_info = {
-- 
2.41.0

[PATCH 24/32] hw/sd: Subtract bootarea size from blk

[Cédric Le Goater]

From: Joel Stanley <joel@jms.id.au>

The userdata size is derived from the file the user passes on the
command line, but we must take into account the boot areas.

Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 6da17a8d0972..1df7c7ac9dae 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -674,6 +674,7 @@ static unsigned sd_boot_capacity_bytes(SDState *sd)
 static void sd_reset(DeviceState *dev)
 {
     SDState *sd = SD_CARD(dev);
+    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
     uint64_t size;
     uint64_t sect;
 
@@ -685,6 +686,10 @@ static void sd_reset(DeviceState *dev)
     }
     size = sect << 9;
 
+    if (sc->bootpart_offset) {
+        size -= sd_boot_capacity_bytes(sd) * 2;
+    }
+
     sect = sd_addr_to_wpnum(size) + 1;
 
     sd->state = sd_idle_state;
-- 
2.41.0

Re: [PATCH 24/32] hw/sd: Subtract bootarea size from blk

[Philippe Mathieu-Daudé]

On 3/7/23 15:25, Cédric Le Goater wrote:
> From: Joel Stanley <joel@jms.id.au>
> 
> The userdata size is derived from the file the user passes on the
> command line, but we must take into account the boot areas.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sd.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 6da17a8d0972..1df7c7ac9dae 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -674,6 +674,7 @@ static unsigned sd_boot_capacity_bytes(SDState *sd)
>   static void sd_reset(DeviceState *dev)
>   {
>       SDState *sd = SD_CARD(dev);
> +    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
>       uint64_t size;
>       uint64_t sect;
>   
> @@ -685,6 +686,10 @@ static void sd_reset(DeviceState *dev)
>       }
>       size = sect << 9;
>   
> +    if (sc->bootpart_offset) {
> +        size -= sd_boot_capacity_bytes(sd) * 2;

IMO this patch and sd_boot_capacity_bytes() definition
from previous patch should be squashed in patch 22 where
you add emmc_cmd_SEND_EXT_CSD.

> +    }
> +
>       sect = sd_addr_to_wpnum(size) + 1;
>   
>       sd->state = sd_idle_state;

Re: [PATCH 24/32] hw/sd: Subtract bootarea size from blk

[Cédric Le Goater]

On 6/3/24 14:31, Philippe Mathieu-Daudé wrote:
> On 3/7/23 15:25, Cédric Le Goater wrote:
>> From: Joel Stanley <joel@jms.id.au>
>>
>> The userdata size is derived from the file the user passes on the
>> command line, but we must take into account the boot areas.
>>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   hw/sd/sd.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 6da17a8d0972..1df7c7ac9dae 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -674,6 +674,7 @@ static unsigned sd_boot_capacity_bytes(SDState *sd)
>>   static void sd_reset(DeviceState *dev)
>>   {
>>       SDState *sd = SD_CARD(dev);
>> +    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
>>       uint64_t size;
>>       uint64_t sect;
>> @@ -685,6 +686,10 @@ static void sd_reset(DeviceState *dev)
>>       }
>>       size = sect << 9;
>> +    if (sc->bootpart_offset) {
>> +        size -= sd_boot_capacity_bytes(sd) * 2;
> 
> IMO this patch and sd_boot_capacity_bytes() definition
> from previous patch should be squashed in patch 22 where
> you add emmc_cmd_SEND_EXT_CSD.

OK. I will check.

Have you looked at the other patches ?

Thanks,

C.


> 
>> +    }
>> +
>>       sect = sd_addr_to_wpnum(size) + 1;
>>       sd->state = sd_idle_state;
> 

[PATCH 25/32] hw/sd: Add boot config support

[Cédric Le Goater]

From: Joel Stanley <joel@jms.id.au>

Introduced "boot-config" property to set CSD 179, the boot config
register.

With this correctly set we can use the enable bit to detect if partition
support is enabled.

Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 1df7c7ac9dae..c4c9e9ee7999 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -115,6 +115,7 @@ struct SDState {
 
     uint8_t spec_version;
     BlockBackend *blk;
+    uint8_t boot_config;
 
     /* Runtime changeables */
 
@@ -459,6 +460,8 @@ static void mmc_set_ext_csd(SDState *sd, uint64_t size)
     sd->ext_csd[159] = 0x00; /* Max enhanced area size */
     sd->ext_csd[158] = 0x00; /* ... */
     sd->ext_csd[157] = 0xEC; /* ... */
+
+    sd->ext_csd[EXT_CSD_PART_CONFIG] = sd->boot_config;
 }
 
 static void sd_emmc_set_csd(SDState *sd, uint64_t size)
@@ -883,8 +886,14 @@ static uint32_t sd_emmc_bootpart_offset(SDState *sd)
 {
     unsigned int access = sd->ext_csd[EXT_CSD_PART_CONFIG] &
         EXT_CSD_PART_CONFIG_ACC_MASK;
+    unsigned int enable = sd->ext_csd[EXT_CSD_PART_CONFIG] &
+         EXT_CSD_PART_CONFIG_EN_MASK;
     unsigned int boot_capacity = sd_boot_capacity_bytes(sd);
 
+    if (!enable) {
+        return 0;
+    }
+
     switch (access) {
     case EXT_CSD_PART_CONFIG_ACC_DEFAULT:
         return boot_capacity * 2;
@@ -2559,6 +2568,7 @@ static Property sd_properties[] = {
      * whether card should be in SSI or MMC/SD mode.  It is also up to the
      * board to ensure that ssi transfers only occur when the chip select
      * is asserted.  */
+    DEFINE_PROP_UINT8("boot-config", SDState, boot_config, 0x0),
     DEFINE_PROP_END_OF_LIST()
 };
 
-- 
2.41.0

[PATCH 26/32] hw/sd: Fix SET_BLOCK_COUNT command argument

[Cédric Le Goater]

The number of blocks is defined in the lower bits [15:0].

TODO: This needs to be more precise on the spec version.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index c4c9e9ee7999..7f07d0e99d15 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1282,7 +1282,7 @@ static sd_rsp_type_t sd_cmd_SET_BLOCK_COUNT(SDState *sd, SDRequest req)
             return sd_invalid_state_for_cmd(sd, req);
         }
 
-        sd->multi_blk_cnt = req.arg;
+        sd->multi_blk_cnt = req.arg & 0xFFFF;
 
         return sd_r1;
 }
-- 
2.41.0

Re: [PATCH 26/32] hw/sd: Fix SET_BLOCK_COUNT command argument

[Philippe Mathieu-Daudé]

On 3/7/23 15:25, Cédric Le Goater wrote:
> The number of blocks is defined in the lower bits [15:0].
> 
> TODO: This needs to be more precise on the spec version.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sd.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index c4c9e9ee7999..7f07d0e99d15 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1282,7 +1282,7 @@ static sd_rsp_type_t sd_cmd_SET_BLOCK_COUNT(SDState *sd, SDRequest req)
>               return sd_invalid_state_for_cmd(sd, req);
>           }
>   
> -        sd->multi_blk_cnt = req.arg;
> +        sd->multi_blk_cnt = req.arg & 0xFFFF;

On the SD Physical Layer spec v9.10 this field is still 32-bit
(see table 4-24, p. 104).

Should we use a sd_is_emmc() helper similar to sd_is_spi()?

>   
>           return sd_r1;
>   }

[PATCH 27/32] hw/sd: Update CMD1 definition for MMC

[Cédric Le Goater]

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

Add support to Power up the card and send response r3 in case of MMC.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
[ clg: - ported on SDProto framework ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 7f07d0e99d15..be9d07126fd9 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2410,8 +2410,8 @@ static const SDProto sd_proto_sd = {
 
 static sd_rsp_type_t emmc_cmd_SEND_OP_CMD(SDState *sd, SDRequest req)
 {
-    sd->state = sd_ready_state;
-    return sd_r3;
+    sd_ocr_powerup(sd);
+    return sd->state == sd_idle_state ? sd_r3 : sd_r0;
 }
 
 static sd_rsp_type_t emmc_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
-- 
2.41.0

[PATCH 28/32] hw/arm/aspeed: Add eMMC device

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/aspeed.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 505cfd3b5fc4..4f028b704751 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -319,14 +319,17 @@ void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
     }
 }
 
-static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo)
+static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc)
 {
         DeviceState *card;
 
         if (!dinfo) {
             return;
         }
-        card = qdev_new(TYPE_SD_CARD);
+        card = qdev_new(emmc ? TYPE_EMMC : TYPE_SD_CARD);
+        if (emmc) {
+            qdev_prop_set_uint8(card, "spec_version", SD_PHY_SPECv3_01_VERS);
+        }
         qdev_prop_set_drive_err(card, "drive", blk_by_legacy_dinfo(dinfo),
                                 &error_fatal);
         qdev_realize_and_unref(card,
@@ -428,12 +431,13 @@ static void aspeed_machine_init(MachineState *machine)
 
     for (i = 0; i < bmc->soc.sdhci.num_slots; i++) {
         sdhci_attach_drive(&bmc->soc.sdhci.slots[i],
-                           drive_get(IF_SD, 0, i));
+                           drive_get(IF_SD, 0, i), false);
     }
 
     if (bmc->soc.emmc.num_slots) {
         sdhci_attach_drive(&bmc->soc.emmc.slots[0],
-                           drive_get(IF_SD, 0, bmc->soc.sdhci.num_slots));
+                           drive_get(IF_SD, 0, bmc->soc.sdhci.num_slots),
+                           true);
     }
 
     if (!bmc->mmio_exec) {
-- 
2.41.0

[PATCH 29/32] hw/arm/aspeed: Load eMMC first boot area as a boot rom

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/aspeed.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 4f028b704751..2cb51ffd8d49 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -360,6 +360,7 @@ static void aspeed_machine_init(MachineState *machine)
     AspeedSoCClass *sc;
     int i;
     NICInfo *nd = &nd_table[0];
+    DriveInfo *emmc0 = NULL;
 
     object_initialize_child(OBJECT(machine), "soc", &bmc->soc, amc->soc_name);
 
@@ -435,9 +436,8 @@ static void aspeed_machine_init(MachineState *machine)
     }
 
     if (bmc->soc.emmc.num_slots) {
-        sdhci_attach_drive(&bmc->soc.emmc.slots[0],
-                           drive_get(IF_SD, 0, bmc->soc.sdhci.num_slots),
-                           true);
+        emmc0 = drive_get(IF_SD, 0, bmc->soc.sdhci.num_slots);
+        sdhci_attach_drive(&bmc->soc.emmc.slots[0], emmc0, true);
     }
 
     if (!bmc->mmio_exec) {
@@ -447,6 +447,8 @@ static void aspeed_machine_init(MachineState *machine)
         if (fmc0) {
             uint64_t rom_size = memory_region_size(&bmc->soc.spi_boot);
             aspeed_install_boot_rom(bmc, fmc0, rom_size);
+        } else if (emmc0) {
+            aspeed_install_boot_rom(bmc, blk_by_legacy_dinfo(emmc0), 64 * KiB);
         }
     }
 
-- 
2.41.0

[PATCH 30/32] hw/arm/aspeed: Set boot device to emmc

[Cédric Le Goater]

From: Joel Stanley <joel@jms.id.au>

This must be configurable by the user, as it forces a the u-boot SPL to
use the eMMC device to boot.

Signed-off-by: Joel Stanley <joel@jms.id.au>
[ clg: activated support on rainier ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/misc/aspeed_scu.h | 7 +++++++
 hw/arm/aspeed.c              | 4 ++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/hw/misc/aspeed_scu.h b/include/hw/misc/aspeed_scu.h
index 5c7c04eedfa7..d48ecf48b8fa 100644
--- a/include/hw/misc/aspeed_scu.h
+++ b/include/hw/misc/aspeed_scu.h
@@ -361,4 +361,11 @@ uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s);
  */
 #define SCU_AST1030_CLK_GET_PCLK_DIV(x)                    (((x) >> 8) & 0xf)
 
+/* STRAP1 SCU500 */
+#define AST26500_HW_STRAP_BOOT_SRC_EMMC            (0x1 << 2)
+#define AST26500_HW_STRAP_BOOT_SRC_SPI             (0x0 << 2)
+
+/* STRAP2 SCU510 */
+#define AST26500_HW_STRAP_BOOT_SRC_UART            (0x1 << 8)
+
 #endif /* ASPEED_SCU_H */
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 2cb51ffd8d49..00fd3c2e4e8a 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -179,11 +179,11 @@ struct AspeedMachineState {
 #define AST2600_EVB_HW_STRAP2 0x00000003
 
 /* Tacoma hardware value */
-#define TACOMA_BMC_HW_STRAP1  0x00000000
+#define TACOMA_BMC_HW_STRAP1  (0x00000000 | AST26500_HW_STRAP_BOOT_SRC_EMMC)
 #define TACOMA_BMC_HW_STRAP2  0x00000040
 
 /* Rainier hardware value: (QEMU prototype) */
-#define RAINIER_BMC_HW_STRAP1 0x00422016
+#define RAINIER_BMC_HW_STRAP1 (0x00422016 | AST26500_HW_STRAP_BOOT_SRC_EMMC)
 #define RAINIER_BMC_HW_STRAP2 0x80000848
 
 /* Fuji hardware value */
-- 
2.41.0

[PATCH 31/32] aspeed: Set bootconfig

[Cédric Le Goater]

From: Joel Stanley <joel@jms.id.au>

This value is taken from a running Rainier machine. It sets bit 3
(select boot0) and bit 6 (enable).

Signed-off-by: Joel Stanley <joel@jms.id.au>
[ clg: Check HW strapping to choose the boot device ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/arm/aspeed_soc.h |  1 +
 hw/arm/aspeed.c             | 14 ++++++++++----
 hw/arm/aspeed_ast2600.c     |  1 +
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 8adff7007286..17ded3e1fd18 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -118,6 +118,7 @@ struct AspeedSoCClass {
     const hwaddr *memmap;
     uint32_t num_cpus;
     qemu_irq (*get_irq)(AspeedSoCState *s, int dev);
+    bool boot_emmc;
 };
 
 
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 00fd3c2e4e8a..6631552358d1 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -319,7 +319,8 @@ void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
     }
 }
 
-static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc)
+static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc,
+                               bool boot_emmc)
 {
         DeviceState *card;
 
@@ -329,6 +330,7 @@ static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc)
         card = qdev_new(emmc ? TYPE_EMMC : TYPE_SD_CARD);
         if (emmc) {
             qdev_prop_set_uint8(card, "spec_version", SD_PHY_SPECv3_01_VERS);
+            qdev_prop_set_uint8(card, "boot-config", boot_emmc ? 0x48 : 0x0);
         }
         qdev_prop_set_drive_err(card, "drive", blk_by_legacy_dinfo(dinfo),
                                 &error_fatal);
@@ -361,11 +363,15 @@ static void aspeed_machine_init(MachineState *machine)
     int i;
     NICInfo *nd = &nd_table[0];
     DriveInfo *emmc0 = NULL;
+    bool boot_emmc;
 
     object_initialize_child(OBJECT(machine), "soc", &bmc->soc, amc->soc_name);
 
     sc = ASPEED_SOC_GET_CLASS(&bmc->soc);
 
+    boot_emmc = sc->boot_emmc &&
+        !!(amc->hw_strap1 & AST26500_HW_STRAP_BOOT_SRC_EMMC);
+
     /*
      * This will error out if the RAM size is not supported by the
      * memory controller of the SoC.
@@ -432,19 +438,19 @@ static void aspeed_machine_init(MachineState *machine)
 
     for (i = 0; i < bmc->soc.sdhci.num_slots; i++) {
         sdhci_attach_drive(&bmc->soc.sdhci.slots[i],
-                           drive_get(IF_SD, 0, i), false);
+                           drive_get(IF_SD, 0, i), false, false);
     }
 
     if (bmc->soc.emmc.num_slots) {
         emmc0 = drive_get(IF_SD, 0, bmc->soc.sdhci.num_slots);
-        sdhci_attach_drive(&bmc->soc.emmc.slots[0], emmc0, true);
+        sdhci_attach_drive(&bmc->soc.emmc.slots[0], emmc0, true, boot_emmc);
     }
 
     if (!bmc->mmio_exec) {
         DeviceState *dev = ssi_get_cs(bmc->soc.fmc.spi, 0);
         BlockBackend *fmc0 = dev ? m25p80_get_blk(dev) : NULL;
 
-        if (fmc0) {
+        if (fmc0 && !boot_emmc) {
             uint64_t rom_size = memory_region_size(&bmc->soc.spi_boot);
             aspeed_install_boot_rom(bmc, fmc0, rom_size);
         } else if (emmc0) {
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index a8b3a8065a11..9d7551b109df 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -644,6 +644,7 @@ static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
     sc->memmap       = aspeed_soc_ast2600_memmap;
     sc->num_cpus     = 2;
     sc->get_irq      = aspeed_soc_ast2600_get_irq;
+    sc->boot_emmc    = true;
 }
 
 static const TypeInfo aspeed_soc_ast2600_type_info = {
-- 
2.41.0

[PATCH 32/32] aspeed: Introduce a 'boot-emmc' property for AST2600 based machines

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 docs/system/arm/aspeed.rst |  2 ++
 hw/arm/aspeed.c            | 42 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
index 80538422a1a4..157bfcc8f396 100644
--- a/docs/system/arm/aspeed.rst
+++ b/docs/system/arm/aspeed.rst
@@ -113,6 +113,8 @@ The image should be attached as an MTD drive. Run :
 
 Options specific to Aspeed machines are :
 
+ * ``boot-emmc`` to set or unset boot from eMMC (AST2600 only).
+
  * ``execute-in-place`` which emulates the boot from the CE0 flash
    device by using the FMC controller to load the instructions, and
    not simply from RAM. This takes a little longer.
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 6631552358d1..dde8dc62a87e 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -46,6 +46,7 @@ struct AspeedMachineState {
     uint32_t uart_chosen;
     char *fmc_model;
     char *spi_model;
+    uint32_t hw_strap1;
 };
 
 /* On 32-bit hosts, lower RAM to 1G because of the 2047 MB limit */
@@ -370,7 +371,7 @@ static void aspeed_machine_init(MachineState *machine)
     sc = ASPEED_SOC_GET_CLASS(&bmc->soc);
 
     boot_emmc = sc->boot_emmc &&
-        !!(amc->hw_strap1 & AST26500_HW_STRAP_BOOT_SRC_EMMC);
+        !!(bmc->hw_strap1 & AST26500_HW_STRAP_BOOT_SRC_EMMC);
 
     /*
      * This will error out if the RAM size is not supported by the
@@ -387,7 +388,7 @@ static void aspeed_machine_init(MachineState *machine)
         }
     }
 
-    object_property_set_int(OBJECT(&bmc->soc), "hw-strap1", amc->hw_strap1,
+    object_property_set_int(OBJECT(&bmc->soc), "hw-strap1", bmc->hw_strap1,
                             &error_abort);
     object_property_set_int(OBJECT(&bmc->soc), "hw-strap2", amc->hw_strap2,
                             &error_abort);
@@ -1069,7 +1070,10 @@ static void aspeed_set_mmio_exec(Object *obj, bool value, Error **errp)
 
 static void aspeed_machine_instance_init(Object *obj)
 {
+    AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(obj);
+
     ASPEED_MACHINE(obj)->mmio_exec = false;
+    ASPEED_MACHINE(obj)->hw_strap1 = amc->hw_strap1;
 }
 
 static char *aspeed_get_fmc_model(Object *obj, Error **errp)
@@ -1349,6 +1353,32 @@ static void aspeed_machine_witherspoon_class_init(ObjectClass *oc, void *data)
         aspeed_soc_num_cpus(amc->soc_name);
 };
 
+static bool aspeed_get_boot_emmc(Object *obj, Error **errp)
+{
+    AspeedMachineState *bmc = ASPEED_MACHINE(obj);
+
+    return !!(bmc->hw_strap1 & AST26500_HW_STRAP_BOOT_SRC_EMMC);
+}
+
+static void aspeed_set_boot_emmc(Object *obj, bool value, Error **errp)
+{
+    AspeedMachineState *bmc = ASPEED_MACHINE(obj);
+
+    if (value) {
+        bmc->hw_strap1 |= AST26500_HW_STRAP_BOOT_SRC_EMMC;
+    } else {
+        bmc->hw_strap1 &= ~AST26500_HW_STRAP_BOOT_SRC_EMMC;
+    }
+}
+
+static void aspeed_machine_ast2600_class_init(ObjectClass *oc, void *data)
+{
+    object_class_property_add_bool(oc, "boot-emmc", aspeed_get_boot_emmc,
+                                  aspeed_set_boot_emmc);
+    object_class_property_set_description(oc, "boot-emmc",
+                                          "Set or unset boot from EMMC");
+}
+
 static void aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -1367,6 +1397,8 @@ static void aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data)
     mc->default_ram_size = 1 * GiB;
     mc->default_cpus = mc->min_cpus = mc->max_cpus =
         aspeed_soc_num_cpus(amc->soc_name);
+
+    aspeed_machine_ast2600_class_init(oc, data);
 };
 
 static void aspeed_machine_tacoma_class_init(ObjectClass *oc, void *data)
@@ -1386,6 +1418,8 @@ static void aspeed_machine_tacoma_class_init(ObjectClass *oc, void *data)
     mc->default_ram_size = 1 * GiB;
     mc->default_cpus = mc->min_cpus = mc->max_cpus =
         aspeed_soc_num_cpus(amc->soc_name);
+
+    aspeed_machine_ast2600_class_init(oc, data);
 };
 
 static void aspeed_machine_g220a_class_init(ObjectClass *oc, void *data)
@@ -1441,6 +1475,8 @@ static void aspeed_machine_rainier_class_init(ObjectClass *oc, void *data)
     mc->default_ram_size = 1 * GiB;
     mc->default_cpus = mc->min_cpus = mc->max_cpus =
         aspeed_soc_num_cpus(amc->soc_name);
+
+    aspeed_machine_ast2600_class_init(oc, data);
 };
 
 #define FUJI_BMC_RAM_SIZE ASPEED_RAM_SIZE(2 * GiB)
@@ -1463,6 +1499,8 @@ static void aspeed_machine_fuji_class_init(ObjectClass *oc, void *data)
     mc->default_ram_size = FUJI_BMC_RAM_SIZE;
     mc->default_cpus = mc->min_cpus = mc->max_cpus =
         aspeed_soc_num_cpus(amc->soc_name);
+
+    aspeed_machine_ast2600_class_init(oc, data);
 };
 
 #define BLETCHLEY_BMC_RAM_SIZE ASPEED_RAM_SIZE(2 * GiB)
-- 
2.41.0

Re: [PATCH 00/32] hw/sd: eMMC support

[Cédric Le Goater]

Hello,

On 7/3/23 15:24, Cédric Le Goater wrote:
> Hello,
> 
>    
> This series adds an extension for a new eMMC device using the
> framework Philippe put in place to support various SD implementations.
> Previous discussion on the same topic:
> 
>    http://patchwork.ozlabs.org/project/qemu-devel/list/?series=250563
>    https://lore.kernel.org/qemu-devel/20220318132824.1134400-1-clg@kaod.org/
> 
> patch 1-12
>    - introduce SDProto structure
>    - could be merged. They have been reviewed.
>    
> patch 13
>    - adds a SPI variant model

I plan to include 1-13 in the next aspeed PR.

Thanks,

C.

>    
> patch 14-27
>    - adds eMMC support
>    - need better commit logs
>    
> patch 28-32 (for later)
> 
>    - aspeed wiring
> 
> Please comment the core part, we can leave out the aspeed part for
> now. I won't have much time to fix the issues unless it's about
> compile and style issues. If someone is interested and could take
> over the series, that would be nice.
> 
> Thanks,
> 
> C.
> 
> Cédric Le Goater (11):
>    hw/sd: Introduce a "sd-card" SPI variant model
>    hw/sd: Add emmc_cmd_SEND_OP_CMD() handler
>    hw/sd: Add emmc_cmd_ALL_SEND_CID() handler
>    hw/sd: Add emmc_cmd_SEND_RELATIVE_ADDR() handler
>    hw/sd: Add emmc_cmd_APP_CMD() handler
>    hw/sd: add emmc_cmd_SEND_TUNING_BLOCK() handler
>    hw/sd: Add emmc_cmd_SEND_EXT_CSD() handler
>    hw/sd: Fix SET_BLOCK_COUNT command argument
>    hw/arm/aspeed: Add eMMC device
>    hw/arm/aspeed: Load eMMC first boot area as a boot rom
>    aspeed: Introduce a 'boot-emmc' property for AST2600 based machines
> 
> Joel Stanley (6):
>    hw/sd: Add sd_cmd_SEND_TUNING_BLOCK() handler
>    hw/sd: Support boot area in emmc image
>    hw/sd: Subtract bootarea size from blk
>    hw/sd: Add boot config support
>    hw/arm/aspeed: Set boot device to emmc
>    aspeed: Set bootconfig
> 
> Philippe Mathieu-Daudé (12):
>    hw/sd: When card is in wrong state, log which state it is
>    hw/sd: When card is in wrong state, log which spec version is used
>    hw/sd: Move proto_name to SDProto structure
>    hw/sd: Introduce sd_cmd_handler type
>    hw/sd: Add sd_cmd_illegal() handler
>    hw/sd: Add sd_cmd_unimplemented() handler
>    hw/sd: Add sd_cmd_GO_IDLE_STATE() handler
>    hw/sd: Add sd_cmd_SEND_OP_CMD() handler
>    hw/sd: Add sd_cmd_ALL_SEND_CID() handler
>    hw/sd: Add sd_cmd_SEND_RELATIVE_ADDR() handler
>    hw/sd: Add sd_cmd_SET_BLOCK_COUNT() handler
>    hw/sd: Basis for eMMC support
> 
> Sai Pavan Boddu (3):
>    hw/sd: Add CMD21 tuning sequence
>    hw/sd: Add mmc switch function support
>    hw/sd: Update CMD1 definition for MMC
> 
>   docs/system/arm/aspeed.rst   |   2 +
>   hw/sd/sdmmc-internal.h       |  97 +++++
>   include/hw/arm/aspeed_soc.h  |   1 +
>   include/hw/misc/aspeed_scu.h |   7 +
>   include/hw/sd/sd.h           |  10 +
>   hw/arm/aspeed.c              |  68 +++-
>   hw/arm/aspeed_ast2600.c      |   1 +
>   hw/arm/stellaris.c           |   3 +-
>   hw/riscv/sifive_u.c          |   3 +-
>   hw/sd/sd.c                   | 702 ++++++++++++++++++++++++++++-------
>   hw/sd/sdmmc-internal.c       |   2 +-
>   11 files changed, 748 insertions(+), 148 deletions(-)
> 

Re: [PATCH 00/32] hw/sd: eMMC support

[Cédric Le Goater]

On 8/28/23 18:27, Cédric Le Goater wrote:
> Hello,
> 
> On 7/3/23 15:24, Cédric Le Goater wrote:
>> Hello,
>>
>> This series adds an extension for a new eMMC device using the
>> framework Philippe put in place to support various SD implementations.
>> Previous discussion on the same topic:
>>
>>    http://patchwork.ozlabs.org/project/qemu-devel/list/?series=250563
>>    https://lore.kernel.org/qemu-devel/20220318132824.1134400-1-clg@kaod.org/
>>
>> patch 1-12
>>    - introduce SDProto structure
>>    - could be merged. They have been reviewed.
>> patch 13
>>    - adds a SPI variant model
> 
> I plan to include 1-13 in the next aspeed PR.

I plan to include the rest of this series in the next aspeed PR,
for QEMU 9.1


Thanks,

C.



> 
> Thanks,
> 
> C.
> 
>> patch 14-27
>>    - adds eMMC support
>>    - need better commit logs
>> patch 28-32 (for later)
>>
>>    - aspeed wiring
>>
>> Please comment the core part, we can leave out the aspeed part for
>> now. I won't have much time to fix the issues unless it's about
>> compile and style issues. If someone is interested and could take
>> over the series, that would be nice.
>>
>> Thanks,
>>
>> C.
>>
>> Cédric Le Goater (11):
>>    hw/sd: Introduce a "sd-card" SPI variant model
>>    hw/sd: Add emmc_cmd_SEND_OP_CMD() handler
>>    hw/sd: Add emmc_cmd_ALL_SEND_CID() handler
>>    hw/sd: Add emmc_cmd_SEND_RELATIVE_ADDR() handler
>>    hw/sd: Add emmc_cmd_APP_CMD() handler
>>    hw/sd: add emmc_cmd_SEND_TUNING_BLOCK() handler
>>    hw/sd: Add emmc_cmd_SEND_EXT_CSD() handler
>>    hw/sd: Fix SET_BLOCK_COUNT command argument
>>    hw/arm/aspeed: Add eMMC device
>>    hw/arm/aspeed: Load eMMC first boot area as a boot rom
>>    aspeed: Introduce a 'boot-emmc' property for AST2600 based machines
>>
>> Joel Stanley (6):
>>    hw/sd: Add sd_cmd_SEND_TUNING_BLOCK() handler
>>    hw/sd: Support boot area in emmc image
>>    hw/sd: Subtract bootarea size from blk
>>    hw/sd: Add boot config support
>>    hw/arm/aspeed: Set boot device to emmc
>>    aspeed: Set bootconfig
>>
>> Philippe Mathieu-Daudé (12):
>>    hw/sd: When card is in wrong state, log which state it is
>>    hw/sd: When card is in wrong state, log which spec version is used
>>    hw/sd: Move proto_name to SDProto structure
>>    hw/sd: Introduce sd_cmd_handler type
>>    hw/sd: Add sd_cmd_illegal() handler
>>    hw/sd: Add sd_cmd_unimplemented() handler
>>    hw/sd: Add sd_cmd_GO_IDLE_STATE() handler
>>    hw/sd: Add sd_cmd_SEND_OP_CMD() handler
>>    hw/sd: Add sd_cmd_ALL_SEND_CID() handler
>>    hw/sd: Add sd_cmd_SEND_RELATIVE_ADDR() handler
>>    hw/sd: Add sd_cmd_SET_BLOCK_COUNT() handler
>>    hw/sd: Basis for eMMC support
>>
>> Sai Pavan Boddu (3):
>>    hw/sd: Add CMD21 tuning sequence
>>    hw/sd: Add mmc switch function support
>>    hw/sd: Update CMD1 definition for MMC
>>
>>   docs/system/arm/aspeed.rst   |   2 +
>>   hw/sd/sdmmc-internal.h       |  97 +++++
>>   include/hw/arm/aspeed_soc.h  |   1 +
>>   include/hw/misc/aspeed_scu.h |   7 +
>>   include/hw/sd/sd.h           |  10 +
>>   hw/arm/aspeed.c              |  68 +++-
>>   hw/arm/aspeed_ast2600.c      |   1 +
>>   hw/arm/stellaris.c           |   3 +-
>>   hw/riscv/sifive_u.c          |   3 +-
>>   hw/sd/sd.c                   | 702 ++++++++++++++++++++++++++++-------
>>   hw/sd/sdmmc-internal.c       |   2 +-
>>   11 files changed, 748 insertions(+), 148 deletions(-)
>>
> 
>