Re: [PATCH v2 03/42] esp: QOMify the internal ESP device state

[Mark Cave-Ayland]

On 12/02/2021 18:51, Philippe Mathieu-Daudé wrote:

> On 2/9/21 8:29 PM, Mark Cave-Ayland wrote:
> > Make this new QOM device state a child device of both the sysbus-esp and esp-pci
> > implementations.
> >
> > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > ---
> >   hw/scsi/esp-pci.c     | 48 +++++++++++++++++++++++++++++++------------
> >   hw/scsi/esp.c         | 45 +++++++++++++++++++++++++++++++++-------
> >   include/hw/scsi/esp.h |  5 +++++
> >   3 files changed, 78 insertions(+), 20 deletions(-)
>
> Please setup scripts/git.orderfile ;)

I will have to take a look at this at some point - it has been on my TODO list for a 
while :)

> > @@ -354,9 +365,11 @@ static void esp_pci_scsi_realize(PCIDevice *dev, Error 
> > **errp)
> >   {
> >       PCIESPState *pci = PCI_ESP(dev);
> >       DeviceState *d = DEVICE(dev);
> > -    ESPState *s = &pci->esp;
> > +    ESPState *s = ESP(&pci->esp);
> >       uint8_t *pci_conf;
> >   
> > +    qdev_realize(DEVICE(s), NULL, errp);
>
>         if (!qdev_realize(DEVICE(s), NULL, errp)) {
>             return;
>         }
>
> >       pci_conf = dev->config;
> >   
> >       /* Interrupt pin A */
> > @@ -375,11 +388,19 @@ static void esp_pci_scsi_realize(PCIDevice *dev, Error 
> > **errp)
> >       scsi_bus_new(&s->bus, sizeof(s->bus), d, &esp_pci_scsi_info, NULL);
> >   }
> ...
>
> > @@ -956,7 +958,9 @@ static void sysbus_esp_realize(DeviceState *dev, Error 
> > **errp)
> >   {
> >       SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> >       SysBusESPState *sysbus = SYSBUS_ESP(dev);
> > -    ESPState *s = &sysbus->esp;
> > +    ESPState *s = ESP(&sysbus->esp);
> > +
> > +    qdev_realize(DEVICE(s), NULL, errp);
>
>         if (!qdev_realize(DEVICE(s), NULL, errp)) {
>             return;
>         }
>
> With both if():
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Great! I've added the if() statements and added your R-B to the patch.


ATB,

Mark.