[PATCH v46 0/5] hw/sd/sdcard: Cleanups before adding eMMC support

[PATCH v46 0/5] hw/sd/sdcard: Cleanups before adding eMMC support

[Philippe Mathieu-Daudé]

(patches from v42 already reviewed not reposted)

Since v45:
- RAZ/WI on GEN_CMD (Luc & Manos)
- Rename sd_cmd_SEND_OP_COND
- Introduce TYPE_SDMMC_COMMON

Philippe Mathieu-Daudé (5):
  hw/sd/sdcard: Use spec v3.01 by default
  hw/sd/sdcard: Add sd_cmd_GEN_CMD handler (CMD56)
  hw/sd/sdcard: Rename sd_cmd_SEND_OP_COND handler
  hw/sd/sdcard: Introduce set_csd/set_cid handlers
  hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD

 hw/sd/sdmmc-internal.h |   3 ++
 include/hw/sd/sd.h     |   2 +
 hw/core/machine.c      |   1 +
 hw/sd/core.c           |  29 ++++++-----
 hw/sd/sd.c             | 113 ++++++++++++++++++++++-------------------
 5 files changed, 81 insertions(+), 67 deletions(-)

-- 
2.41.0

[PATCH v46 1/5] hw/sd/sdcard: Use spec v3.01 by default

[Philippe Mathieu-Daudé]

Recent SDHCI expect cards to support the v3.01 spec
to negociate lower I/O voltage. Select it by default.

Versioned machine types with a version of 9.0 or
earlier retain the old default (spec v2.00).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
v43: update versioned machines (danpb)

Cc: Daniel P . Berrangé <berrange@redhat.com>
---
 hw/core/machine.c | 1 +
 hw/sd/sd.c        | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 655d75c21f..4377f943d5 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -38,6 +38,7 @@ GlobalProperty hw_compat_9_0[] = {
     {"arm-cpu", "backcompat-cntfrq", "true" },
     {"scsi-disk-base", "migrate-emulated-scsi-request", "false" },
     {"vfio-pci", "skip-vsc-check", "false" },
+    {"sd-card", "spec_version", "2" },
 };
 const size_t hw_compat_9_0_len = G_N_ELEMENTS(hw_compat_9_0);
 
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 53767beaf8..a08a452d81 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2471,7 +2471,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
 
 static Property sd_properties[] = {
     DEFINE_PROP_UINT8("spec_version", SDState,
-                      spec_version, SD_PHY_SPECv2_00_VERS),
+                      spec_version, SD_PHY_SPECv3_01_VERS),
     DEFINE_PROP_DRIVE("drive", SDState, blk),
     DEFINE_PROP_END_OF_LIST()
 };
-- 
2.41.0

[PATCH v46 2/5] hw/sd/sdcard: Add sd_cmd_GEN_CMD handler (CMD56)

[Philippe Mathieu-Daudé]

"General command" (GEN_CMD, CMD56) is described as:

  GEN_CMD is the same as the single block read or write
  commands (CMD24 or CMD17). The difference is that [...]
  the data block is not a memory payload data but has a
  vendor specific format and meaning.

Thus this block must not be stored overwriting data block
on underlying storage drive. Handle as RAZ/WI.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sd/sd.c | 54 ++++++++++++++++++++----------------------------------
 1 file changed, 20 insertions(+), 34 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a08a452d81..04e8fdb262 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -243,7 +243,6 @@ static const char *sd_cmd_name(SDState *sd, uint8_t cmd)
                                             [25]    = "WRITE_MULTIPLE_BLOCK",
         [26]    = "MANUF_RSVD",
         [40]    = "DPS_spec",
-        [56]    = "GEN_CMD",
         [60]    = "MANUF_RSVD",             [61]    = "MANUF_RSVD",
         [62]    = "MANUF_RSVD",             [63]    = "MANUF_RSVD",
     };
@@ -902,9 +901,6 @@ static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
     }
 }
 
-#define APP_READ_BLOCK(a, len)  memset(sd->data, 0xec, len)
-#define APP_WRITE_BLOCK(a, len)
-
 static void sd_erase(SDState *sd)
 {
     uint64_t erase_start = sd->erase_start;
@@ -1641,6 +1637,22 @@ static sd_rsp_type_t sd_cmd_APP_CMD(SDState *sd, SDRequest req)
     return sd_r1;
 }
 
+/* CMD56 */
+static sd_rsp_type_t sd_cmd_GEN_CMD(SDState *sd, SDRequest req)
+{
+    if (sd->state != sd_transfer_state) {
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+
+    /* Vendor specific command: our model is RAZ/WI */
+    if (req.arg & 1) {
+        memset(sd->data, 0, sizeof(sd->data));
+        return sd_cmd_to_sendingdata(sd, req, 0, NULL, 0);
+    } else {
+        return sd_cmd_to_receivingdata(sd, req, 0, 0);
+    }
+}
+
 /* CMD58 */
 static sd_rsp_type_t spi_cmd_READ_OCR(SDState *sd, SDRequest req)
 {
@@ -1836,22 +1848,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
     case 26:  /* CMD26:  PROGRAM_CID */
         return sd_cmd_to_receivingdata(sd, req, 0, sizeof(sd->cid));
 
-    /* Application specific commands (Class 8) */
-    case 56:  /* CMD56:  GEN_CMD */
-        switch (sd->state) {
-        case sd_transfer_state:
-            sd->data_offset = 0;
-            if (req.arg & 1)
-                sd->state = sd_sendingdata_state;
-            else
-                sd->state = sd_receivingdata_state;
-            return sd_r1;
-
-        default:
-            break;
-        }
-        break;
-
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "SD: Unknown CMD%i\n", req.cmd);
         return sd_illegal;
@@ -2187,11 +2183,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
         break;
 
     case 56:  /* CMD56:  GEN_CMD */
-        sd->data[sd->data_offset ++] = value;
-        if (sd->data_offset >= sd->blk_len) {
-            APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
-            sd->state = sd_transfer_state;
-        }
+        sd_generic_write_byte(sd, value);
         break;
 
     default:
@@ -2233,6 +2225,7 @@ uint8_t sd_read_byte(SDState *sd)
     case 22: /* ACMD22: SEND_NUM_WR_BLOCKS */
     case 30: /* CMD30:  SEND_WRITE_PROT */
     case 51: /* ACMD51: SEND_SCR */
+    case 56: /* CMD56:  GEN_CMD */
         sd_generic_read_byte(sd, &ret);
         break;
 
@@ -2260,15 +2253,6 @@ uint8_t sd_read_byte(SDState *sd)
         }
         break;
 
-    case 56:  /* CMD56:  GEN_CMD */
-        if (sd->data_offset == 0)
-            APP_READ_BLOCK(sd->data_start, sd->blk_len);
-        ret = sd->data[sd->data_offset ++];
-
-        if (sd->data_offset >= sd->blk_len)
-            sd->state = sd_transfer_state;
-        break;
-
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "%s: unknown command\n", __func__);
         return 0x00;
@@ -2323,6 +2307,7 @@ static const SDProto sd_proto_spi = {
         [52] = {9,  sd_spi, "IO_RW_DIRECT", sd_cmd_optional},
         [53] = {9,  sd_spi, "IO_RW_EXTENDED", sd_cmd_optional},
         [55] = {8,  sd_spi, "APP_CMD", sd_cmd_APP_CMD},
+        [56] = {8,  sd_spi, "GEN_CMD", sd_cmd_GEN_CMD},
         [57] = {10, sd_spi, "DIRECT_SECURE_WRITE", sd_cmd_optional},
         [58] = {0,  sd_spi, "READ_OCR", spi_cmd_READ_OCR},
         [59] = {0,  sd_spi, "CRC_ON_OFF", spi_cmd_CRC_ON_OFF},
@@ -2383,6 +2368,7 @@ static const SDProto sd_proto_sd = {
         [52] = {9,  sd_bc,   "IO_RW_DIRECT", sd_cmd_optional},
         [53] = {9,  sd_bc,   "IO_RW_EXTENDED", sd_cmd_optional},
         [55] = {8,  sd_ac,   "APP_CMD", sd_cmd_APP_CMD},
+        [56] = {8,  sd_adtc, "GEN_CMD", sd_cmd_GEN_CMD},
         [57] = {10, sd_adtc, "DIRECT_SECURE_WRITE", sd_cmd_optional},
         [58] = {11, sd_adtc, "READ_EXTR_MULTI", sd_cmd_optional},
         [59] = {11, sd_adtc, "WRITE_EXTR_MULTI", sd_cmd_optional},
-- 
2.41.0

[PATCH v46 3/5] hw/sd/sdcard: Rename sd_cmd_SEND_OP_COND handler

[Philippe Mathieu-Daudé]

The correct command name is 'SD SEND_OP_COND',
rename accordingly.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.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 04e8fdb262..10f2764a53 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1701,7 +1701,7 @@ static sd_rsp_type_t sd_acmd_SET_WR_BLK_ERASE_COUNT(SDState *sd, SDRequest req)
 }
 
 /* ACMD41 */
-static sd_rsp_type_t sd_acmd_SD_APP_OP_COND(SDState *sd, SDRequest req)
+static sd_rsp_type_t sd_cmd_SEND_OP_COND(SDState *sd, SDRequest req)
 {
     if (sd->state != sd_idle_state) {
         return sd_invalid_state_for_cmd(sd, req);
@@ -2378,7 +2378,7 @@ static const SDProto sd_proto_sd = {
         [13] = {8,  sd_adtc, "SD_STATUS", sd_acmd_SD_STATUS},
         [22] = {8,  sd_adtc, "SEND_NUM_WR_BLOCKS", sd_acmd_SEND_NUM_WR_BLOCKS},
         [23] = {8,  sd_ac,   "SET_WR_BLK_ERASE_COUNT", sd_acmd_SET_WR_BLK_ERASE_COUNT},
-        [41] = {8,  sd_bcr,  "SD_APP_OP_COND", sd_acmd_SD_APP_OP_COND},
+        [41] = {8,  sd_bcr,  "SEND_OP_COND", sd_cmd_SEND_OP_COND},
         [42] = {8,  sd_ac,   "SET_CLR_CARD_DETECT", sd_acmd_SET_CLR_CARD_DETECT},
         [51] = {8,  sd_adtc, "SEND_SCR", sd_acmd_SEND_SCR},
     },
-- 
2.41.0

Re: [PATCH v46 3/5] hw/sd/sdcard: Rename sd_cmd_SEND_OP_COND handler

[Cédric Le Goater]

On 7/3/24 3:43 PM, Philippe Mathieu-Daudé wrote:
> The correct command name is 'SD SEND_OP_COND',
> rename accordingly.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   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 04e8fdb262..10f2764a53 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1701,7 +1701,7 @@ static sd_rsp_type_t sd_acmd_SET_WR_BLK_ERASE_COUNT(SDState *sd, SDRequest req)
>   }
>   
>   /* ACMD41 */
> -static sd_rsp_type_t sd_acmd_SD_APP_OP_COND(SDState *sd, SDRequest req)
> +static sd_rsp_type_t sd_cmd_SEND_OP_COND(SDState *sd, SDRequest req)
>   {
>       if (sd->state != sd_idle_state) {
>           return sd_invalid_state_for_cmd(sd, req);
> @@ -2378,7 +2378,7 @@ static const SDProto sd_proto_sd = {
>           [13] = {8,  sd_adtc, "SD_STATUS", sd_acmd_SD_STATUS},
>           [22] = {8,  sd_adtc, "SEND_NUM_WR_BLOCKS", sd_acmd_SEND_NUM_WR_BLOCKS},
>           [23] = {8,  sd_ac,   "SET_WR_BLK_ERASE_COUNT", sd_acmd_SET_WR_BLK_ERASE_COUNT},
> -        [41] = {8,  sd_bcr,  "SD_APP_OP_COND", sd_acmd_SD_APP_OP_COND},
> +        [41] = {8,  sd_bcr,  "SEND_OP_COND", sd_cmd_SEND_OP_COND},
>           [42] = {8,  sd_ac,   "SET_CLR_CARD_DETECT", sd_acmd_SET_CLR_CARD_DETECT},
>           [51] = {8,  sd_adtc, "SEND_SCR", sd_acmd_SEND_SCR},
>       },

[PATCH v46 4/5] hw/sd/sdcard: Introduce set_csd/set_cid handlers

[Philippe Mathieu-Daudé]

In preparation of introducing eMMC support which have
different CSD/CID structures, introduce a pair of handlers
in SDCardClass.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/sd/sd.h | 2 ++
 hw/sd/sd.c         | 7 +++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 29c76935a0..bfbc83c110 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -128,6 +128,8 @@ struct SDCardClass {
     void (*enable)(SDState *sd, bool enable);
     bool (*get_inserted)(SDState *sd);
     bool (*get_readonly)(SDState *sd);
+    void (*set_cid)(SDState *sd);
+    void (*set_csd)(SDState *sd, uint64_t size);
 
     const struct SDProto *proto;
 };
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 10f2764a53..d46be50760 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -689,6 +689,7 @@ static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
 static void sd_reset(DeviceState *dev)
 {
     SDState *sd = SD_CARD(dev);
+    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
     uint64_t size;
     uint64_t sect;
 
@@ -709,8 +710,8 @@ static void sd_reset(DeviceState *dev)
     sd->size = size;
     sd_set_ocr(sd);
     sd_set_scr(sd);
-    sd_set_cid(sd);
-    sd_set_csd(sd, size);
+    sc->set_cid(sd);
+    sc->set_csd(sd, size);
     sd_set_cardstatus(sd);
     sd_set_sdstatus(sd);
 
@@ -2485,6 +2486,8 @@ 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->set_cid = sd_set_cid;
+    sc->set_csd = sd_set_csd;
     sc->proto = &sd_proto_sd;
 }
 
-- 
2.41.0

Re: [PATCH v46 4/5] hw/sd/sdcard: Introduce set_csd/set_cid handlers

[Cédric Le Goater]

On 7/3/24 3:43 PM, Philippe Mathieu-Daudé wrote:
> In preparation of introducing eMMC support which have
> different CSD/CID structures, introduce a pair of handlers
> in SDCardClass.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   include/hw/sd/sd.h | 2 ++
>   hw/sd/sd.c         | 7 +++++--
>   2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
> index 29c76935a0..bfbc83c110 100644
> --- a/include/hw/sd/sd.h
> +++ b/include/hw/sd/sd.h
> @@ -128,6 +128,8 @@ struct SDCardClass {
>       void (*enable)(SDState *sd, bool enable);
>       bool (*get_inserted)(SDState *sd);
>       bool (*get_readonly)(SDState *sd);
> +    void (*set_cid)(SDState *sd);
> +    void (*set_csd)(SDState *sd, uint64_t size);
>   
>       const struct SDProto *proto;
>   };
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 10f2764a53..d46be50760 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -689,6 +689,7 @@ static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
>   static void sd_reset(DeviceState *dev)
>   {
>       SDState *sd = SD_CARD(dev);
> +    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
>       uint64_t size;
>       uint64_t sect;
>   
> @@ -709,8 +710,8 @@ static void sd_reset(DeviceState *dev)
>       sd->size = size;
>       sd_set_ocr(sd);
>       sd_set_scr(sd);
> -    sd_set_cid(sd);
> -    sd_set_csd(sd, size);
> +    sc->set_cid(sd);
> +    sc->set_csd(sd, size);
>       sd_set_cardstatus(sd);
>       sd_set_sdstatus(sd);
>   
> @@ -2485,6 +2486,8 @@ 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->set_cid = sd_set_cid;
> +    sc->set_csd = sd_set_csd;
>       sc->proto = &sd_proto_sd;
>   }
>   

[PATCH v46 5/5] hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD

[Philippe Mathieu-Daudé]

In order to keep eMMC model simpler to maintain,
extract common properties and the common code from
class_init to the (internal) TYPE_SDMMC_COMMON.

Update the corresponding QOM cast macros.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sd/sdmmc-internal.h |  3 +++
 hw/sd/core.c           | 29 ++++++++++++------------
 hw/sd/sd.c             | 50 ++++++++++++++++++++++++++++--------------
 3 files changed, 52 insertions(+), 30 deletions(-)

diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h
index cc0b69e834..91eb5b6b2f 100644
--- a/hw/sd/sdmmc-internal.h
+++ b/hw/sd/sdmmc-internal.h
@@ -11,6 +11,9 @@
 #ifndef SDMMC_INTERNAL_H
 #define SDMMC_INTERNAL_H
 
+#define TYPE_SDMMC_COMMON "sdmmc-common"
+DECLARE_OBJ_CHECKERS(SDState, SDCardClass, SDMMC_COMMON, TYPE_SDMMC_COMMON)
+
 /*
  * EXT_CSD Modes segment
  *
diff --git a/hw/sd/core.c b/hw/sd/core.c
index 52d5d90045..4b30218b52 100644
--- a/hw/sd/core.c
+++ b/hw/sd/core.c
@@ -24,6 +24,7 @@
 #include "hw/sd/sd.h"
 #include "qemu/module.h"
 #include "qapi/error.h"
+#include "sdmmc-internal.h"
 #include "trace.h"
 
 static inline const char *sdbus_name(SDBus *sdbus)
@@ -39,7 +40,7 @@ static SDState *get_card(SDBus *sdbus)
     if (!kid) {
         return NULL;
     }
-    return SD_CARD(kid->child);
+    return SDMMC_COMMON(kid->child);
 }
 
 uint8_t sdbus_get_dat_lines(SDBus *sdbus)
@@ -48,7 +49,7 @@ uint8_t sdbus_get_dat_lines(SDBus *sdbus)
     uint8_t dat_lines = 0b1111; /* 4 bit bus width */
 
     if (slave) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(slave);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(slave);
 
         if (sc->get_dat_lines) {
             dat_lines = sc->get_dat_lines(slave);
@@ -65,7 +66,7 @@ bool sdbus_get_cmd_line(SDBus *sdbus)
     bool cmd_line = true;
 
     if (slave) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(slave);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(slave);
 
         if (sc->get_cmd_line) {
             cmd_line = sc->get_cmd_line(slave);
@@ -82,7 +83,7 @@ void sdbus_set_voltage(SDBus *sdbus, uint16_t millivolts)
 
     trace_sdbus_set_voltage(sdbus_name(sdbus), millivolts);
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         assert(sc->set_voltage);
         sc->set_voltage(card, millivolts);
@@ -95,7 +96,7 @@ int sdbus_do_command(SDBus *sdbus, SDRequest *req, uint8_t *response)
 
     trace_sdbus_command(sdbus_name(sdbus), req->cmd, req->arg);
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         return sc->do_command(card, req, response);
     }
@@ -109,7 +110,7 @@ void sdbus_write_byte(SDBus *sdbus, uint8_t value)
 
     trace_sdbus_write(sdbus_name(sdbus), value);
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         sc->write_byte(card, value);
     }
@@ -121,7 +122,7 @@ void sdbus_write_data(SDBus *sdbus, const void *buf, size_t length)
     const uint8_t *data = buf;
 
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         for (size_t i = 0; i < length; i++) {
             trace_sdbus_write(sdbus_name(sdbus), data[i]);
@@ -136,7 +137,7 @@ uint8_t sdbus_read_byte(SDBus *sdbus)
     uint8_t value = 0;
 
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         value = sc->read_byte(card);
     }
@@ -151,7 +152,7 @@ void sdbus_read_data(SDBus *sdbus, void *buf, size_t length)
     uint8_t *data = buf;
 
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         for (size_t i = 0; i < length; i++) {
             data[i] = sc->read_byte(card);
@@ -165,7 +166,7 @@ bool sdbus_receive_ready(SDBus *sdbus)
     SDState *card = get_card(sdbus);
 
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         return sc->receive_ready(card);
     }
@@ -178,7 +179,7 @@ bool sdbus_data_ready(SDBus *sdbus)
     SDState *card = get_card(sdbus);
 
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         return sc->data_ready(card);
     }
@@ -191,7 +192,7 @@ bool sdbus_get_inserted(SDBus *sdbus)
     SDState *card = get_card(sdbus);
 
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         return sc->get_inserted(card);
     }
@@ -204,7 +205,7 @@ bool sdbus_get_readonly(SDBus *sdbus)
     SDState *card = get_card(sdbus);
 
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         return sc->get_readonly(card);
     }
@@ -250,7 +251,7 @@ void sdbus_reparent_card(SDBus *from, SDBus *to)
         return;
     }
 
-    sc = SD_CARD_GET_CLASS(card);
+    sc = SDMMC_COMMON_GET_CLASS(card);
     readonly = sc->get_readonly(card);
 
     sdbus_set_inserted(from, false);
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index d46be50760..9fcb2b9e89 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -688,8 +688,8 @@ static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
 
 static void sd_reset(DeviceState *dev)
 {
-    SDState *sd = SD_CARD(dev);
-    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
+    SDState *sd = SDMMC_COMMON(dev);
+    SDCardClass *sc = SDMMC_COMMON_GET_CLASS(sd);
     uint64_t size;
     uint64_t sect;
 
@@ -2387,8 +2387,8 @@ static const SDProto sd_proto_sd = {
 
 static void sd_instance_init(Object *obj)
 {
-    SDState *sd = SD_CARD(obj);
-    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
+    SDState *sd = SDMMC_COMMON(obj);
+    SDCardClass *sc = SDMMC_COMMON_GET_CLASS(sd);
 
     sd->proto = sc->proto;
     sd->last_cmd_name = "UNSET";
@@ -2398,14 +2398,14 @@ static void sd_instance_init(Object *obj)
 
 static void sd_instance_finalize(Object *obj)
 {
-    SDState *sd = SD_CARD(obj);
+    SDState *sd = SDMMC_COMMON(obj);
 
     timer_free(sd->ocr_power_timer);
 }
 
 static void sd_realize(DeviceState *dev, Error **errp)
 {
-    SDState *sd = SD_CARD(dev);
+    SDState *sd = SDMMC_COMMON(dev);
     int ret;
 
     switch (sd->spec_version) {
@@ -2456,20 +2456,23 @@ static void sd_realize(DeviceState *dev, Error **errp)
     }
 }
 
-static Property sd_properties[] = {
-    DEFINE_PROP_UINT8("spec_version", SDState,
-                      spec_version, SD_PHY_SPECv3_01_VERS),
+static Property sdmmc_common_properties[] = {
     DEFINE_PROP_DRIVE("drive", SDState, blk),
     DEFINE_PROP_END_OF_LIST()
 };
 
-static void sd_class_init(ObjectClass *klass, void *data)
+static Property sd_properties[] = {
+    DEFINE_PROP_UINT8("spec_version", SDState,
+                      spec_version, SD_PHY_SPECv3_01_VERS),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void sdmmc_common_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SDCardClass *sc = SD_CARD_CLASS(klass);
+    SDCardClass *sc = SDMMC_COMMON_CLASS(klass);
 
-    dc->realize = sd_realize;
-    device_class_set_props(dc, sd_properties);
+    device_class_set_props(dc, sdmmc_common_properties);
     dc->vmsd = &sd_vmstate;
     dc->reset = sd_reset;
     dc->bus_type = TYPE_SD_BUS;
@@ -2486,6 +2489,16 @@ 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;
+}
+
+static void sd_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SDCardClass *sc = SDMMC_COMMON_CLASS(klass);
+
+    dc->realize = sd_realize;
+    device_class_set_props(dc, sd_properties);
+
     sc->set_cid = sd_set_cid;
     sc->set_csd = sd_set_csd;
     sc->proto = &sd_proto_sd;
@@ -2500,7 +2513,7 @@ static void sd_class_init(ObjectClass *klass, void *data)
 static void sd_spi_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SDCardClass *sc = SD_CARD_CLASS(klass);
+    SDCardClass *sc = SDMMC_COMMON_CLASS(klass);
 
     dc->desc = "SD SPI";
     sc->proto = &sd_proto_spi;
@@ -2508,14 +2521,19 @@ static void sd_spi_class_init(ObjectClass *klass, void *data)
 
 static const TypeInfo sd_types[] = {
     {
-        .name           = TYPE_SD_CARD,
+        .name           = TYPE_SDMMC_COMMON,
         .parent         = TYPE_DEVICE,
         .instance_size  = sizeof(SDState),
         .class_size     = sizeof(SDCardClass),
-        .class_init     = sd_class_init,
+        .class_init     = sdmmc_common_class_init,
         .instance_init  = sd_instance_init,
         .instance_finalize = sd_instance_finalize,
     },
+    {
+        .name           = TYPE_SD_CARD,
+        .parent         = TYPE_SDMMC_COMMON,
+        .class_init     = sd_class_init,
+    },
     {
         .name           = TYPE_SD_CARD_SPI,
         .parent         = TYPE_SD_CARD,
-- 
2.41.0

Re: [PATCH v46 5/5] hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD

[Cédric Le Goater]

On 7/3/24 3:43 PM, Philippe Mathieu-Daudé wrote:
> In order to keep eMMC model simpler to maintain,
> extract common properties and the common code from
> class_init to the (internal) TYPE_SDMMC_COMMON.
> 
> Update the corresponding QOM cast macros.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/sd/sdmmc-internal.h |  3 +++
>   hw/sd/core.c           | 29 ++++++++++++------------
>   hw/sd/sd.c             | 50 ++++++++++++++++++++++++++++--------------
>   3 files changed, 52 insertions(+), 30 deletions(-)
> 
> diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h
> index cc0b69e834..91eb5b6b2f 100644
> --- a/hw/sd/sdmmc-internal.h
> +++ b/hw/sd/sdmmc-internal.h
> @@ -11,6 +11,9 @@
>   #ifndef SDMMC_INTERNAL_H
>   #define SDMMC_INTERNAL_H
>   
> +#define TYPE_SDMMC_COMMON "sdmmc-common"
> +DECLARE_OBJ_CHECKERS(SDState, SDCardClass, SDMMC_COMMON, TYPE_SDMMC_COMMON)
> +
>   /*
>    * EXT_CSD Modes segment
>    *
> diff --git a/hw/sd/core.c b/hw/sd/core.c
> index 52d5d90045..4b30218b52 100644
> --- a/hw/sd/core.c
> +++ b/hw/sd/core.c
> @@ -24,6 +24,7 @@
>   #include "hw/sd/sd.h"
>   #include "qemu/module.h"
>   #include "qapi/error.h"
> +#include "sdmmc-internal.h"
>   #include "trace.h"
>   
>   static inline const char *sdbus_name(SDBus *sdbus)
> @@ -39,7 +40,7 @@ static SDState *get_card(SDBus *sdbus)
>       if (!kid) {
>           return NULL;
>       }
> -    return SD_CARD(kid->child);
> +    return SDMMC_COMMON(kid->child);
>   }
>   
>   uint8_t sdbus_get_dat_lines(SDBus *sdbus)
> @@ -48,7 +49,7 @@ uint8_t sdbus_get_dat_lines(SDBus *sdbus)
>       uint8_t dat_lines = 0b1111; /* 4 bit bus width */
>   
>       if (slave) {
> -        SDCardClass *sc = SD_CARD_GET_CLASS(slave);
> +        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(slave);
>   
>           if (sc->get_dat_lines) {
>               dat_lines = sc->get_dat_lines(slave);
> @@ -65,7 +66,7 @@ bool sdbus_get_cmd_line(SDBus *sdbus)
>       bool cmd_line = true;
>   
>       if (slave) {
> -        SDCardClass *sc = SD_CARD_GET_CLASS(slave);
> +        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(slave);
>   
>           if (sc->get_cmd_line) {
>               cmd_line = sc->get_cmd_line(slave);
> @@ -82,7 +83,7 @@ void sdbus_set_voltage(SDBus *sdbus, uint16_t millivolts)
>   
>       trace_sdbus_set_voltage(sdbus_name(sdbus), millivolts);
>       if (card) {
> -        SDCardClass *sc = SD_CARD_GET_CLASS(card);
> +        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
>   
>           assert(sc->set_voltage);
>           sc->set_voltage(card, millivolts);
> @@ -95,7 +96,7 @@ int sdbus_do_command(SDBus *sdbus, SDRequest *req, uint8_t *response)
>   
>       trace_sdbus_command(sdbus_name(sdbus), req->cmd, req->arg);
>       if (card) {
> -        SDCardClass *sc = SD_CARD_GET_CLASS(card);
> +        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
>   
>           return sc->do_command(card, req, response);
>       }
> @@ -109,7 +110,7 @@ void sdbus_write_byte(SDBus *sdbus, uint8_t value)
>   
>       trace_sdbus_write(sdbus_name(sdbus), value);
>       if (card) {
> -        SDCardClass *sc = SD_CARD_GET_CLASS(card);
> +        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
>   
>           sc->write_byte(card, value);
>       }
> @@ -121,7 +122,7 @@ void sdbus_write_data(SDBus *sdbus, const void *buf, size_t length)
>       const uint8_t *data = buf;
>   
>       if (card) {
> -        SDCardClass *sc = SD_CARD_GET_CLASS(card);
> +        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
>   
>           for (size_t i = 0; i < length; i++) {
>               trace_sdbus_write(sdbus_name(sdbus), data[i]);
> @@ -136,7 +137,7 @@ uint8_t sdbus_read_byte(SDBus *sdbus)
>       uint8_t value = 0;
>   
>       if (card) {
> -        SDCardClass *sc = SD_CARD_GET_CLASS(card);
> +        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
>   
>           value = sc->read_byte(card);
>       }
> @@ -151,7 +152,7 @@ void sdbus_read_data(SDBus *sdbus, void *buf, size_t length)
>       uint8_t *data = buf;
>   
>       if (card) {
> -        SDCardClass *sc = SD_CARD_GET_CLASS(card);
> +        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
>   
>           for (size_t i = 0; i < length; i++) {
>               data[i] = sc->read_byte(card);
> @@ -165,7 +166,7 @@ bool sdbus_receive_ready(SDBus *sdbus)
>       SDState *card = get_card(sdbus);
>   
>       if (card) {
> -        SDCardClass *sc = SD_CARD_GET_CLASS(card);
> +        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
>   
>           return sc->receive_ready(card);
>       }
> @@ -178,7 +179,7 @@ bool sdbus_data_ready(SDBus *sdbus)
>       SDState *card = get_card(sdbus);
>   
>       if (card) {
> -        SDCardClass *sc = SD_CARD_GET_CLASS(card);
> +        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
>   
>           return sc->data_ready(card);
>       }
> @@ -191,7 +192,7 @@ bool sdbus_get_inserted(SDBus *sdbus)
>       SDState *card = get_card(sdbus);
>   
>       if (card) {
> -        SDCardClass *sc = SD_CARD_GET_CLASS(card);
> +        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
>   
>           return sc->get_inserted(card);
>       }
> @@ -204,7 +205,7 @@ bool sdbus_get_readonly(SDBus *sdbus)
>       SDState *card = get_card(sdbus);
>   
>       if (card) {
> -        SDCardClass *sc = SD_CARD_GET_CLASS(card);
> +        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
>   
>           return sc->get_readonly(card);
>       }
> @@ -250,7 +251,7 @@ void sdbus_reparent_card(SDBus *from, SDBus *to)
>           return;
>       }
>   
> -    sc = SD_CARD_GET_CLASS(card);
> +    sc = SDMMC_COMMON_GET_CLASS(card);
>       readonly = sc->get_readonly(card);
>   
>       sdbus_set_inserted(from, false);
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index d46be50760..9fcb2b9e89 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -688,8 +688,8 @@ static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
>   
>   static void sd_reset(DeviceState *dev)
>   {
> -    SDState *sd = SD_CARD(dev);
> -    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
> +    SDState *sd = SDMMC_COMMON(dev);
> +    SDCardClass *sc = SDMMC_COMMON_GET_CLASS(sd);
>       uint64_t size;
>       uint64_t sect;
>   
> @@ -2387,8 +2387,8 @@ static const SDProto sd_proto_sd = {
>   
>   static void sd_instance_init(Object *obj)
>   {
> -    SDState *sd = SD_CARD(obj);
> -    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
> +    SDState *sd = SDMMC_COMMON(obj);
> +    SDCardClass *sc = SDMMC_COMMON_GET_CLASS(sd);
>   
>       sd->proto = sc->proto;
>       sd->last_cmd_name = "UNSET";
> @@ -2398,14 +2398,14 @@ static void sd_instance_init(Object *obj)
>   
>   static void sd_instance_finalize(Object *obj)
>   {
> -    SDState *sd = SD_CARD(obj);
> +    SDState *sd = SDMMC_COMMON(obj);
>   
>       timer_free(sd->ocr_power_timer);
>   }
>   
>   static void sd_realize(DeviceState *dev, Error **errp)
>   {
> -    SDState *sd = SD_CARD(dev);
> +    SDState *sd = SDMMC_COMMON(dev);
>       int ret;
>   
>       switch (sd->spec_version) {
> @@ -2456,20 +2456,23 @@ static void sd_realize(DeviceState *dev, Error **errp)
>       }
>   }
>   
> -static Property sd_properties[] = {
> -    DEFINE_PROP_UINT8("spec_version", SDState,
> -                      spec_version, SD_PHY_SPECv3_01_VERS),
> +static Property sdmmc_common_properties[] = {
>       DEFINE_PROP_DRIVE("drive", SDState, blk),
>       DEFINE_PROP_END_OF_LIST()
>   };
>   
> -static void sd_class_init(ObjectClass *klass, void *data)
> +static Property sd_properties[] = {
> +    DEFINE_PROP_UINT8("spec_version", SDState,
> +                      spec_version, SD_PHY_SPECv3_01_VERS),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
> +static void sdmmc_common_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
> -    SDCardClass *sc = SD_CARD_CLASS(klass);
> +    SDCardClass *sc = SDMMC_COMMON_CLASS(klass);
>   
> -    dc->realize = sd_realize;
> -    device_class_set_props(dc, sd_properties);
> +    device_class_set_props(dc, sdmmc_common_properties);
>       dc->vmsd = &sd_vmstate;
>       dc->reset = sd_reset;
>       dc->bus_type = TYPE_SD_BUS;
> @@ -2486,6 +2489,16 @@ 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;
> +}
> +
> +static void sd_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    SDCardClass *sc = SDMMC_COMMON_CLASS(klass);
> +
> +    dc->realize = sd_realize;
> +    device_class_set_props(dc, sd_properties);
> +
>       sc->set_cid = sd_set_cid;
>       sc->set_csd = sd_set_csd;
>       sc->proto = &sd_proto_sd;
> @@ -2500,7 +2513,7 @@ static void sd_class_init(ObjectClass *klass, void *data)
>   static void sd_spi_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
> -    SDCardClass *sc = SD_CARD_CLASS(klass);
> +    SDCardClass *sc = SDMMC_COMMON_CLASS(klass);
>   
>       dc->desc = "SD SPI";
>       sc->proto = &sd_proto_spi;
> @@ -2508,14 +2521,19 @@ static void sd_spi_class_init(ObjectClass *klass, void *data)
>   
>   static const TypeInfo sd_types[] = {
>       {
> -        .name           = TYPE_SD_CARD,
> +        .name           = TYPE_SDMMC_COMMON,
>           .parent         = TYPE_DEVICE,
>           .instance_size  = sizeof(SDState),
>           .class_size     = sizeof(SDCardClass),
> -        .class_init     = sd_class_init,
> +        .class_init     = sdmmc_common_class_init,
>           .instance_init  = sd_instance_init,
>           .instance_finalize = sd_instance_finalize,
>       },

Shouldn't it be an abstract class ?


Thanks,

C.


> +    {
> +        .name           = TYPE_SD_CARD,
> +        .parent         = TYPE_SDMMC_COMMON,
> +        .class_init     = sd_class_init,
> +    },
>       {
>           .name           = TYPE_SD_CARD_SPI,
>           .parent         = TYPE_SD_CARD,

Re: [PATCH v46 5/5] hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD

[Philippe Mathieu-Daudé]

On 3/7/24 16:02, Cédric Le Goater wrote:
> On 7/3/24 3:43 PM, Philippe Mathieu-Daudé wrote:
>> In order to keep eMMC model simpler to maintain,
>> extract common properties and the common code from
>> class_init to the (internal) TYPE_SDMMC_COMMON.
>>
>> Update the corresponding QOM cast macros.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/sd/sdmmc-internal.h |  3 +++
>>   hw/sd/core.c           | 29 ++++++++++++------------
>>   hw/sd/sd.c             | 50 ++++++++++++++++++++++++++++--------------
>>   3 files changed, 52 insertions(+), 30 deletions(-)


>> @@ -2508,14 +2521,19 @@ static void sd_spi_class_init(ObjectClass 
>> *klass, void *data)
>>   static const TypeInfo sd_types[] = {
>>       {
>> -        .name           = TYPE_SD_CARD,
>> +        .name           = TYPE_SDMMC_COMMON,
>>           .parent         = TYPE_DEVICE,
>>           .instance_size  = sizeof(SDState),
>>           .class_size     = sizeof(SDCardClass),
>> -        .class_init     = sd_class_init,
>> +        .class_init     = sdmmc_common_class_init,
>>           .instance_init  = sd_instance_init,
>>           .instance_finalize = sd_instance_finalize,
>>       },
> 
> Shouldn't it be an abstract class ?

Ah yes, safer. Squashing:

-- >8 --
@@ -2513,6 +2513,7 @@ static const TypeInfo sd_types[] = {
      {
          .name           = TYPE_SDMMC_COMMON,
          .parent         = TYPE_DEVICE,
+        .abstract       = true,
          .instance_size  = sizeof(SDState),
          .class_size     = sizeof(SDCardClass),
          .class_init     = sdmmc_common_class_init,
---

> 
> 
> Thanks,
> 
> C.
> 
> 
>> +    {
>> +        .name           = TYPE_SD_CARD,
>> +        .parent         = TYPE_SDMMC_COMMON,
>> +        .class_init     = sd_class_init,
>> +    },
>>       {
>>           .name           = TYPE_SD_CARD_SPI,
>>           .parent         = TYPE_SD_CARD,
> 

Re: [PATCH v46 5/5] hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD

[Cédric Le Goater]

On 7/3/24 4:07 PM, Philippe Mathieu-Daudé wrote:
> On 3/7/24 16:02, Cédric Le Goater wrote:
>> On 7/3/24 3:43 PM, Philippe Mathieu-Daudé wrote:
>>> In order to keep eMMC model simpler to maintain,
>>> extract common properties and the common code from
>>> class_init to the (internal) TYPE_SDMMC_COMMON.
>>>
>>> Update the corresponding QOM cast macros.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   hw/sd/sdmmc-internal.h |  3 +++
>>>   hw/sd/core.c           | 29 ++++++++++++------------
>>>   hw/sd/sd.c             | 50 ++++++++++++++++++++++++++++--------------
>>>   3 files changed, 52 insertions(+), 30 deletions(-)
> 
> 
>>> @@ -2508,14 +2521,19 @@ static void sd_spi_class_init(ObjectClass *klass, void *data)
>>>   static const TypeInfo sd_types[] = {
>>>       {
>>> -        .name           = TYPE_SD_CARD,
>>> +        .name           = TYPE_SDMMC_COMMON,
>>>           .parent         = TYPE_DEVICE,
>>>           .instance_size  = sizeof(SDState),
>>>           .class_size     = sizeof(SDCardClass),
>>> -        .class_init     = sd_class_init,
>>> +        .class_init     = sdmmc_common_class_init,
>>>           .instance_init  = sd_instance_init,
>>>           .instance_finalize = sd_instance_finalize,
>>>       },
>>
>> Shouldn't it be an abstract class ?
> 
> Ah yes, safer. Squashing:
> 
> -- >8 --
> @@ -2513,6 +2513,7 @@ static const TypeInfo sd_types[] = {
>       {
>           .name           = TYPE_SDMMC_COMMON,
>           .parent         = TYPE_DEVICE,
> +        .abstract       = true,
>           .instance_size  = sizeof(SDState),
>           .class_size     = sizeof(SDCardClass),
>           .class_init     = sdmmc_common_class_init,


with that,

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.

Re: [PATCH v46 0/5] hw/sd/sdcard: Cleanups before adding eMMC support

[Philippe Mathieu-Daudé]

On 3/7/24 15:43, Philippe Mathieu-Daudé wrote:
> (patches from v42 already reviewed not reposted)
> 
> Since v45:
> - RAZ/WI on GEN_CMD (Luc & Manos)
> - Rename sd_cmd_SEND_OP_COND
> - Introduce TYPE_SDMMC_COMMON
> 
> Philippe Mathieu-Daudé (5):
>    hw/sd/sdcard: Use spec v3.01 by default
>    hw/sd/sdcard: Add sd_cmd_GEN_CMD handler (CMD56)
>    hw/sd/sdcard: Rename sd_cmd_SEND_OP_COND handler
>    hw/sd/sdcard: Introduce set_csd/set_cid handlers
>    hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD

Per 
https://lore.kernel.org/qemu-devel/c434c7e88153a134bc6ba2dc202595211702398c.camel@codeconstruct.com.au/
Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>

:)

Re: [PATCH v46 0/5] hw/sd/sdcard: Cleanups before adding eMMC support

[Cédric Le Goater]

On 7/3/24 3:43 PM, Philippe Mathieu-Daudé wrote:
> (patches from v42 already reviewed not reposted)
> 
> Since v45:
> - RAZ/WI on GEN_CMD (Luc & Manos)
> - Rename sd_cmd_SEND_OP_COND
> - Introduce TYPE_SDMMC_COMMON
> 
> Philippe Mathieu-Daudé (5):
>    hw/sd/sdcard: Use spec v3.01 by default
>    hw/sd/sdcard: Add sd_cmd_GEN_CMD handler (CMD56)
>    hw/sd/sdcard: Rename sd_cmd_SEND_OP_COND handler
>    hw/sd/sdcard: Introduce set_csd/set_cid handlers
>    hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD
> 
>   hw/sd/sdmmc-internal.h |   3 ++
>   include/hw/sd/sd.h     |   2 +
>   hw/core/machine.c      |   1 +
>   hw/sd/core.c           |  29 ++++++-----
>   hw/sd/sd.c             | 113 ++++++++++++++++++++++-------------------
>   5 files changed, 81 insertions(+), 67 deletions(-)
> 


Tested-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.

Re: [PATCH v46 0/5] hw/sd/sdcard: Cleanups before adding eMMC support

[Philippe Mathieu-Daudé]

On 3/7/24 15:43, Philippe Mathieu-Daudé wrote:

> Philippe Mathieu-Daudé (5):
>    hw/sd/sdcard: Use spec v3.01 by default
>    hw/sd/sdcard: Add sd_cmd_GEN_CMD handler (CMD56)
>    hw/sd/sdcard: Rename sd_cmd_SEND_OP_COND handler
>    hw/sd/sdcard: Introduce set_csd/set_cid handlers
>    hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD

Series queued, thanks.