Hi Cedric,
> -----Original Message----- > From: Cédric Le Goater <c...@kaod.org> > Sent: Wednesday, February 24, 2021 7:25 PM > To: Stefan Hajnoczi <stefa...@redhat.com>; Sai Pavan Boddu > <saip...@xilinx.com> > Cc: Philippe Mathieu-Daudé <phi...@redhat.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>; 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>; qemu- > de...@nongnu.org; qemu-block@nongnu.org > Subject: Re: [PATCH v2 01/22] block: add eMMC block device type > > On 2/24/21 12:40 PM, Stefan Hajnoczi wrote: > > On Tue, Feb 23, 2021 at 05:35:20PM +0000, Sai Pavan Boddu wrote: > >> Hi Philippe, > >> > >>> -----Original Message----- > >>> From: Philippe Mathieu-Daudé <phi...@redhat.com> > >>> Sent: Monday, February 22, 2021 5:34 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>; Cédric Le Goater <c...@kaod.org>; 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: Sai Pavan Boddu <saip...@xilinx.com>; qemu-de...@nongnu.org; > >>> qemu- bl...@nongnu.org > >>> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type > >>> > >>> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote: > >>>> From: Vincent Palatin <vpala...@chromium.org> > >>>> > >>>> Add new block device type. > >>>> > >>>> Signed-off-by: Vincent Palatin <vpala...@chromium.org> > >>>> [SPB: Rebased over 5.1 version] > >>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.bo...@xilinx.com> > >>>> Signed-off-by: Joel Stanley <j...@jms.id.au> > >>>> Signed-off-by: Cédric Le Goater <c...@kaod.org> > >>>> Reviewed-by: Alistair Francis <alistair.fran...@wdc.com> > >>>> --- > >>>> include/sysemu/blockdev.h | 1 + > >>>> blockdev.c | 1 + > >>>> 2 files changed, 2 insertions(+) > >>>> > >>>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h > >>>> index 3b5fcda..eefae9f 100644 > >>>> --- a/include/sysemu/blockdev.h > >>>> +++ b/include/sysemu/blockdev.h > >>>> @@ -24,6 +24,7 @@ typedef enum { > >>>> */ > >>>> IF_NONE = 0, > >>>> IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, > >>>> IF_VIRTIO, IF_XEN, > >>>> + IF_EMMC, > >>>> IF_COUNT > >>>> } BlockInterfaceType; > >>>> > >>>> diff --git a/blockdev.c b/blockdev.c index cd438e6..390d43c 100644 > >>>> --- a/blockdev.c > >>>> +++ b/blockdev.c > >>>> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = { > >>>> [IF_SD] = "sd", > >>>> [IF_VIRTIO] = "virtio", > >>>> [IF_XEN] = "xen", > >>>> + [IF_EMMC] = "emmc", > >>>> }; > >>> > >>> We don't need to introduce support for the legacy -drive magic. > >>> > >>> -device should be enough for this device, right? > >> [Sai Pavan Boddu] I was seeing to use -device for emmc. But I see we > anyway need blockdev support for this, which would require us the use -drive. > >> > >> Can you give some pointers, how to approach this ? > > > > It is probably not necessary to add a new IF_ constant. Would this work: > > > > -drive if=none,id=emmc0,file=test.img,format=raw > > -device emmc,...,drive=emmc0 > > > > Or the more modern: > > > > -blockdev node-name=emmc0,driver=file,filename=test.img > > -device emmc,...,drive=emmc0 > > > > ? > > > > (The syntax might need small tweaks but is shows the general idea.) > > Yes. This is better. > > We could have an "emmc" device inheriting from "sd-card". The "emmc" > property would not be necessary anymore and may be, we could cleanup up > some parts doing : > > if (sd->emmc) { /* eMMC */ > ... > } else { > > } > > with SDCardClass handlers. the SWITCH_FUNCTION command is a good > candidate, CMD8 also. [Sai Pavan Boddu] Nice, this approach looks clean. But we still may be depending on emmc property. Not sure! I would get back with v3, your review on those patches would be great. Thanks & Regards, Sai Pavan > > C.