Hi Cedric,
> -----Original Message-----
> From: Cédric Le Goater <c...@kaod.org>
> Sent: Monday, March 1, 2021 4:32 PM
> To: Sai Pavan Boddu <saip...@xilinx.com>; Markus Armbruster
> <arm...@redhat.com>; Kevin Wolf <kw...@redhat.com>; Max Reitz
> <mre...@redhat.com>; Vladimir Sementsov-Ogievskiy
> <vsement...@virtuozzo.com>; Eric Blake <ebl...@redhat.com>; Joel Stanley
> <j...@jms.id.au>; Vincent Palatin <vpala...@chromium.org>; Dr. David Alan
> Gilbert <dgilb...@redhat.com>; Thomas Huth <th...@redhat.com>; Stefan
> Hajnoczi <stefa...@redhat.com>; Peter Maydell <peter.mayd...@linaro.org>;
> Alistair Francis <alistair.fran...@wdc.com>; Edgar Iglesias
> <edg...@xilinx.com>;
> Luc Michel <luc.mic...@greensocs.com>; Paolo Bonzini
> <pbonz...@redhat.com>
> Cc: qemu-bl...@nongnu.org; qemu-devel@nongnu.org; Sai Pavan Boddu
> <saip...@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.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,
> +};
> +
[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)
