| Message ID | 20200528110444.20456-2-armbru@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixes around device realization | expand |
On 5/28/20 1:04 PM, Markus Armbruster wrote: > stm32f405_soc_initfn() creates six such devices, but > stm32f405_soc_realize() realizes only one. Affects machine > netduinoplus2. > > In theory, a device becomes real only on realize. In practice, the > transition from unreal to real is a fuzzy one. The work to make a > device real can be spread between realize methods (fine), > instance_init methods (wrong), and board code wiring up the device > (fine as long as it effectively happens on realize). Depending on > what exactly is done where, a device can work even when we neglect > to realize it. > > The five unrealized devices appear to stay unreal: neither MMIO nor > IRQ get wired up. > > Fix stm32f405_soc_realize() to realize and wire up all six. Visible > in "info qtree": > > bus: main-system-bus > type System > dev: stm32f405-soc, id "" > cpu-type = "cortex-m4-arm-cpu" > dev: stm32f2xx-adc, id "" > gpio-out "sysbus-irq" 1 > - mmio ffffffffffffffff/00000000000000ff > + mmio 0000000040012000/00000000000000ff > dev: stm32f2xx-adc, id "" > gpio-out "sysbus-irq" 1 > - mmio ffffffffffffffff/00000000000000ff > + mmio 0000000040012100/00000000000000ff > dev: stm32f2xx-adc, id "" > gpio-out "sysbus-irq" 1 > - mmio ffffffffffffffff/00000000000000ff > + mmio 0000000040012200/00000000000000ff > dev: stm32f2xx-adc, id "" > gpio-out "sysbus-irq" 1 > - mmio ffffffffffffffff/00000000000000ff > + mmio 0000000040012300/00000000000000ff > dev: stm32f2xx-adc, id "" > gpio-out "sysbus-irq" 1 > - mmio 0000000040012000/00000000000000ff > + mmio 0000000040012400/00000000000000ff > dev: stm32f2xx-adc, id "" > gpio-out "sysbus-irq" 1 > - mmio ffffffffffffffff/00000000000000ff > + mmio 0000000040012500/00000000000000ff > dev: armv7m, id "" > > Fixes: 529fc5fd3e18ace8f739afd02dc0953354f39442 > Cc: Alistair Francis <alistair@alistair23.me> > Cc: Peter Maydell <peter.maydell@linaro.org> > Cc: qemu-arm@nongnu.org > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > hw/arm/stm32f405_soc.c | 23 +++++++++++++---------- > 1 file changed, 13 insertions(+), 10 deletions(-) > > diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c > index 4f10ce6176..c9a530eecf 100644 > --- a/hw/arm/stm32f405_soc.c > +++ b/hw/arm/stm32f405_soc.c > @@ -37,7 +37,8 @@ static const uint32_t usart_addr[] = { 0x40011000, 0x40004400, 0x40004800, > /* At the moment only Timer 2 to 5 are modelled */ > static const uint32_t timer_addr[] = { 0x40000000, 0x40000400, > 0x40000800, 0x40000C00 }; > -#define ADC_ADDR 0x40012000 > +static const uint32_t adc_addr[] = { 0x40012000, 0x40012100, 0x40012200, > + 0x40012300, 0x40012400, 0x40012500 }; > static const uint32_t spi_addr[] = { 0x40013000, 0x40003800, 0x40003C00, > 0x40013400, 0x40015000, 0x40015400 }; > #define EXTI_ADDR 0x40013C00 > @@ -185,16 +186,18 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp) > qdev_connect_gpio_out(DEVICE(&s->adc_irqs), 0, > qdev_get_gpio_in(armv7m, ADC_IRQ)); > > - dev = DEVICE(&(s->adc[i])); > - object_property_set_bool(OBJECT(&s->adc[i]), true, "realized", &err); > - if (err != NULL) { > - error_propagate(errp, err); > - return; > + for (i = 0; i < STM_NUM_ADCS; i++) { Correct fix. Problem will come back when we'll want to implement a STM SoC with 8 ADCs, modifying the definition... We'll need to remember to unref() again. Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > + dev = DEVICE(&(s->adc[i])); > + object_property_set_bool(OBJECT(&s->adc[i]), true, "realized", &err); > + if (err != NULL) { > + error_propagate(errp, err); > + return; > + } > + busdev = SYS_BUS_DEVICE(dev); > + sysbus_mmio_map(busdev, 0, adc_addr[i]); > + sysbus_connect_irq(busdev, 0, > + qdev_get_gpio_in(DEVICE(&s->adc_irqs), i)); > } > - busdev = SYS_BUS_DEVICE(dev); > - sysbus_mmio_map(busdev, 0, ADC_ADDR); > - sysbus_connect_irq(busdev, 0, > - qdev_get_gpio_in(DEVICE(&s->adc_irqs), i)); > > /* SPI devices */ > for (i = 0; i < STM_NUM_SPIS; i++) { >
diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c index 4f10ce6176..c9a530eecf 100644 --- a/hw/arm/stm32f405_soc.c +++ b/hw/arm/stm32f405_soc.c @@ -37,7 +37,8 @@ static const uint32_t usart_addr[] = { 0x40011000, 0x40004400, 0x40004800, /* At the moment only Timer 2 to 5 are modelled */ static const uint32_t timer_addr[] = { 0x40000000, 0x40000400, 0x40000800, 0x40000C00 }; -#define ADC_ADDR 0x40012000 +static const uint32_t adc_addr[] = { 0x40012000, 0x40012100, 0x40012200, + 0x40012300, 0x40012400, 0x40012500 }; static const uint32_t spi_addr[] = { 0x40013000, 0x40003800, 0x40003C00, 0x40013400, 0x40015000, 0x40015400 }; #define EXTI_ADDR 0x40013C00 @@ -185,16 +186,18 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp) qdev_connect_gpio_out(DEVICE(&s->adc_irqs), 0, qdev_get_gpio_in(armv7m, ADC_IRQ)); - dev = DEVICE(&(s->adc[i])); - object_property_set_bool(OBJECT(&s->adc[i]), true, "realized", &err); - if (err != NULL) { - error_propagate(errp, err); - return; + for (i = 0; i < STM_NUM_ADCS; i++) { + dev = DEVICE(&(s->adc[i])); + object_property_set_bool(OBJECT(&s->adc[i]), true, "realized", &err); + if (err != NULL) { + error_propagate(errp, err); + return; + } + busdev = SYS_BUS_DEVICE(dev); + sysbus_mmio_map(busdev, 0, adc_addr[i]); + sysbus_connect_irq(busdev, 0, + qdev_get_gpio_in(DEVICE(&s->adc_irqs), i)); } - busdev = SYS_BUS_DEVICE(dev); - sysbus_mmio_map(busdev, 0, ADC_ADDR); - sysbus_connect_irq(busdev, 0, - qdev_get_gpio_in(DEVICE(&s->adc_irqs), i)); /* SPI devices */ for (i = 0; i < STM_NUM_SPIS; i++) {
stm32f405_soc_initfn() creates six such devices, but stm32f405_soc_realize() realizes only one. Affects machine netduinoplus2. In theory, a device becomes real only on realize. In practice, the transition from unreal to real is a fuzzy one. The work to make a device real can be spread between realize methods (fine), instance_init methods (wrong), and board code wiring up the device (fine as long as it effectively happens on realize). Depending on what exactly is done where, a device can work even when we neglect to realize it. The five unrealized devices appear to stay unreal: neither MMIO nor IRQ get wired up. Fix stm32f405_soc_realize() to realize and wire up all six. Visible in "info qtree": bus: main-system-bus type System dev: stm32f405-soc, id "" cpu-type = "cortex-m4-arm-cpu" dev: stm32f2xx-adc, id "" gpio-out "sysbus-irq" 1 - mmio ffffffffffffffff/00000000000000ff + mmio 0000000040012000/00000000000000ff dev: stm32f2xx-adc, id "" gpio-out "sysbus-irq" 1 - mmio ffffffffffffffff/00000000000000ff + mmio 0000000040012100/00000000000000ff dev: stm32f2xx-adc, id "" gpio-out "sysbus-irq" 1 - mmio ffffffffffffffff/00000000000000ff + mmio 0000000040012200/00000000000000ff dev: stm32f2xx-adc, id "" gpio-out "sysbus-irq" 1 - mmio ffffffffffffffff/00000000000000ff + mmio 0000000040012300/00000000000000ff dev: stm32f2xx-adc, id "" gpio-out "sysbus-irq" 1 - mmio 0000000040012000/00000000000000ff + mmio 0000000040012400/00000000000000ff dev: stm32f2xx-adc, id "" gpio-out "sysbus-irq" 1 - mmio ffffffffffffffff/00000000000000ff + mmio 0000000040012500/00000000000000ff dev: armv7m, id "" Fixes: 529fc5fd3e18ace8f739afd02dc0953354f39442 Cc: Alistair Francis <alistair@alistair23.me> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: qemu-arm@nongnu.org Signed-off-by: Markus Armbruster <armbru@redhat.com> --- hw/arm/stm32f405_soc.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)