| Message ID | 20231123143813.42632-8-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw: Simplify accesses to CPUState::'start-powered-off' property | expand |
On Thu, 23 Nov 2023 at 14:39, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > First run the code that can return errors, then on success > run what alters the instance state. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > hw/arm/bcm2836.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c > index 03e6eb2fb2..e56935f3e5 100644 > --- a/hw/arm/bcm2836.c > +++ b/hw/arm/bcm2836.c > @@ -119,13 +119,6 @@ static void bcm2836_realize(DeviceState *dev, Error **errp) > return; > } > > - sysbus_mmio_map(SYS_BUS_DEVICE(&s->control), 0, bc->ctrl_base); > - > - sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 0, > - qdev_get_gpio_in_named(DEVICE(&s->control), "gpu-irq", 0)); > - sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 1, > - qdev_get_gpio_in_named(DEVICE(&s->control), "gpu-fiq", 0)); > - > for (n = 0; n < BCM283X_NCPUS; n++) { > object_property_set_int(OBJECT(&s->cpu[n].core), "mp-affinity", > (bc->clusterid << 8) | n, &error_abort); > @@ -158,6 +151,13 @@ static void bcm2836_realize(DeviceState *dev, Error **errp) > qdev_connect_gpio_out(DEVICE(&s->cpu[n].core), GTIMER_SEC, > qdev_get_gpio_in_named(DEVICE(&s->control), "cntpsirq", n)); > } > + > + sysbus_mmio_map(SYS_BUS_DEVICE(&s->control), 0, bc->ctrl_base); > + > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 0, > + qdev_get_gpio_in_named(DEVICE(&s->control), "gpu-irq", 0)); > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 1, > + qdev_get_gpio_in_named(DEVICE(&s->control), "gpu-fiq", 0)); > } > There's no particular harm in moving the code, but given the loop we are already doing some IRQ-connection work before doing some things which might fail. We don't in general do a particularly consistent job with tidying up in realize methods that fail halfway through, but in general "connect an IRQ between two devices both of which are owned by this container device" and "map an MR of this device we own into a container region we also own" are not things which affect the overall simulation, and in theory should be cleanup-able later (maybe even automatically by refcount if we're really lucky). -- PMM
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c index 03e6eb2fb2..e56935f3e5 100644 --- a/hw/arm/bcm2836.c +++ b/hw/arm/bcm2836.c @@ -119,13 +119,6 @@ static void bcm2836_realize(DeviceState *dev, Error **errp) return; } - sysbus_mmio_map(SYS_BUS_DEVICE(&s->control), 0, bc->ctrl_base); - - sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 0, - qdev_get_gpio_in_named(DEVICE(&s->control), "gpu-irq", 0)); - sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 1, - qdev_get_gpio_in_named(DEVICE(&s->control), "gpu-fiq", 0)); - for (n = 0; n < BCM283X_NCPUS; n++) { object_property_set_int(OBJECT(&s->cpu[n].core), "mp-affinity", (bc->clusterid << 8) | n, &error_abort); @@ -158,6 +151,13 @@ static void bcm2836_realize(DeviceState *dev, Error **errp) qdev_connect_gpio_out(DEVICE(&s->cpu[n].core), GTIMER_SEC, qdev_get_gpio_in_named(DEVICE(&s->control), "cntpsirq", n)); } + + sysbus_mmio_map(SYS_BUS_DEVICE(&s->control), 0, bc->ctrl_base); + + sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 0, + qdev_get_gpio_in_named(DEVICE(&s->control), "gpu-irq", 0)); + sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 1, + qdev_get_gpio_in_named(DEVICE(&s->control), "gpu-fiq", 0)); } static void bcm283x_class_init(ObjectClass *oc, void *data)
First run the code that can return errors, then on success run what alters the instance state. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/arm/bcm2836.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)