Re: [PATCH v3 02/21] sd: emmc: Add support for eMMC cards

[Cédric Le Goater]

On 2/28/21 8:33 PM, Sai Pavan Boddu wrote:
> Add eMMC device built on top of SD card.
> 
> Signed-off-by: Sai Pavan Boddu <sai.pavan.bo...@xilinx.com>
> ---
>  include/hw/sd/sd.h |  2 ++
>  hw/sd/sd.c         | 20 ++++++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
> index 05ef9b7..b402dad 100644
> --- a/include/hw/sd/sd.h
> +++ b/include/hw/sd/sd.h
> @@ -90,6 +90,8 @@ typedef struct {
>  } SDRequest;
>  
>  
> +#define TYPE_EMMC "emmc"
> +OBJECT_DECLARE_SIMPLE_TYPE(EMMCState, EMMC)
>  #define TYPE_SD_CARD "sd-card"
>  OBJECT_DECLARE_TYPE(SDState, SDCardClass, SD_CARD)
>  
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 74b9162..a23af6d 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -108,6 +108,7 @@ struct SDState {
>      uint8_t spec_version;
>      BlockBackend *blk;
>      bool spi;
> +    bool emmc;
>  
>      /* Runtime changeables */
>  
> @@ -143,6 +144,10 @@ struct SDState {
>      bool cmd_line;
>  };
>  
> +struct EMMCState {
> +    SDState parent;
> +};
> +
>  static void sd_realize(DeviceState *dev, Error **errp);
>  
>  static const char *sd_state_name(enum SDCardStates state)
> @@ -2105,6 +2110,13 @@ static void sd_instance_init(Object *obj)
>      sd->ocr_power_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sd_ocr_powerup, 
> sd);
>  }
>  
> +static void emmc_instance_init(Object *obj)
> +{
> +    SDState *sd = SD_CARD(obj);
> +
> +    sd->emmc = true;
> +}
I think field 'emmc' would fit better in SDCardClass since it is a device 
constant. You should not need 'struct EMMCState'. So something like below.
Then you can add handlers for specific emmc commands.

Thanks,

C.


diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 47360ba4ee98..80e7cd526a57 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -93,6 +93,8 @@ typedef struct {
 #define TYPE_SD_CARD "sd-card"
 OBJECT_DECLARE_TYPE(SDState, SDCardClass, SD_CARD)
 
+#define TYPE_EMMC "emmc"
+
 struct SDCardClass {
     /*< private >*/
     DeviceClass parent_class;
@@ -124,6 +126,8 @@ struct SDCardClass {
     void (*enable)(SDState *sd, bool enable);
     bool (*get_inserted)(SDState *sd);
     bool (*get_readonly)(SDState *sd);
+
+    bool emmc;
 };
 
 #define TYPE_SD_BUS "sd-bus"
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 660026f2a667..95608f11b36e 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2447,9 +2447,24 @@ static const TypeInfo sd_info = {
     .instance_finalize = sd_instance_finalize,
 };
 
+static void emmc_class_init(ObjectClass *klass, void *data)
+{
+    SDCardClass *sc = SD_CARD_CLASS(klass);
+
+    sc->emmc = true;
+}
+
+static const TypeInfo emmc_info = {
+    .name = TYPE_EMMC,
+    .parent = TYPE_SD_CARD,
+    .instance_size = sizeof(SDState),
+    .class_init = emmc_class_init,
+};
+
 static void sd_register_types(void)
 {
     type_register_static(&sd_info);
+    type_register_static(&emmc_info);
 }
 
 type_init(sd_register_types)