On 2/24/21 8:13 PM, Sai Pavan Boddu wrote: > 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!
It could be a SDCardClass attribute instead since it's a constant for the emmc device. C. > > I would get back with v3, your review on those patches would be great. > > Thanks & Regards, > Sai Pavan >> >> C.