diff mbox series

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

Message ID 1614540807-30686-3-git-send-email-sai.pavan.boddu@xilinx.com (mailing list archive)
State New, archived
Headers show
Series eMMC support | expand

Commit Message

Sai Pavan Boddu Feb. 28, 2021, 7:33 p.m. UTC 
Add eMMC device built on top of SD card.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 include/hw/sd/sd.h |  2 ++
 hw/sd/sd.c         | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+)

Comments

Cédric Le Goater March 1, 2021, 11:02 a.m. UTC | #1 
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.boddu@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)
Sai Pavan Boddu March 3, 2021, 3:57 a.m. UTC | #2 
Hi Cedric,
> -----Original Message-----
> From: Cédric Le Goater <clg@kaod.org>
> Sent: Monday, March 1, 2021 4:32 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>; Markus Armbruster
> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
> <joel@jms.id.au>; Vincent Palatin <vpalatin@chromium.org>; Dr. David Alan
> Gilbert <dgilbert@redhat.com>; Thomas Huth <thuth@redhat.com>; Stefan
> Hajnoczi <stefanha@redhat.com>; Peter Maydell <peter.maydell@linaro.org>;
> Alistair Francis <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>;
> Luc Michel <luc.michel@greensocs.com>; Paolo Bonzini
> <pbonzini@redhat.com>
> Cc: qemu-block@nongnu.org; qemu-devel@nongnu.org; Sai Pavan Boddu
> <saipava@xilinx.com>
> Subject: Re: [PATCH v3 02/21] sd: emmc: Add support for eMMC cards
> 
> 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.boddu@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,
> +};
> +
[Sai Pavan Boddu] Yes, I see your point. Let me try, I was preferring a simpler approach just to not disturb the code much but lets see how this workout.

Thanks,
Sai Pavan
>  static void sd_register_types(void)
>  {
>      type_register_static(&sd_info);
> +    type_register_static(&emmc_info);
>  }
> 
>  type_init(sd_register_types)
diff mbox series

Patch

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;
+}
+
 static void sd_instance_finalize(Object *obj)
 {
     SDState *sd = SD_CARD(obj);
@@ -2213,9 +2225,17 @@  static const TypeInfo sd_info = {
     .instance_finalize = sd_instance_finalize,
 };
 
+static const TypeInfo emmc_info = {
+    .name = TYPE_EMMC,
+    .parent = TYPE_SD_CARD,
+    .instance_size = sizeof(EMMCState),
+    .instance_init = emmc_instance_init,
+};
+
 static void sd_register_types(void)
 {
     type_register_static(&sd_info);
+    type_register_static(&emmc_info);
 }
 
 type_init(sd_register_types)