Re: [PATCH v2 01/24] arm/stm32f405: Fix realization of "stm32f2xx-adc" devices

2020-05-28 Thread Philippe Mathieu-Daudé
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 /00ff
> +mmio 40012000/00ff
>dev: stm32f2xx-adc, id ""
>  gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012100/00ff
>dev: stm32f2xx-adc, id ""
>  gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012200/00ff
>dev: stm32f2xx-adc, id ""
>  gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012300/00ff
>dev: stm32f2xx-adc, id ""
>  gpio-out "sysbus-irq" 1
> -mmio 40012000/00ff
> +mmio 40012400/00ff
>dev: stm32f2xx-adc, id ""
>  gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012500/00ff
>dev: armv7m, id ""
> 
> Fixes: 529fc5fd3e18ace8f739afd02dc0953354f39442
> Cc: Alistair Francis 
> Cc: Peter Maydell 
> Cc: qemu-...@nongnu.org
> Signed-off-by: Markus Armbruster 
> ---
>  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[] = { 0x4000, 0x4400,
> 0x4800, 0x4C00 };
> -#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(>adc_irqs), 0,
>qdev_get_gpio_in(armv7m, ADC_IRQ));
>  
> -dev = DEVICE(&(s->adc[i]));
> -object_property_set_bool(OBJECT(>adc[i]), true, "realized", );
> -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é 

> +dev = DEVICE(&(s->adc[i]));
> +object_property_set_bool(OBJECT(>adc[i]), true, "realized", );
> +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(>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(>adc_irqs), i));
>  
>  /* SPI devices */
>  for (i = 0; i < STM_NUM_SPIS; i++) {
> 




[PATCH v2 01/24] arm/stm32f405: Fix realization of "stm32f2xx-adc" devices

2020-05-28 Thread Markus Armbruster
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 /00ff
+mmio 40012000/00ff
   dev: stm32f2xx-adc, id ""
 gpio-out "sysbus-irq" 1
-mmio /00ff
+mmio 40012100/00ff
   dev: stm32f2xx-adc, id ""
 gpio-out "sysbus-irq" 1
-mmio /00ff
+mmio 40012200/00ff
   dev: stm32f2xx-adc, id ""
 gpio-out "sysbus-irq" 1
-mmio /00ff
+mmio 40012300/00ff
   dev: stm32f2xx-adc, id ""
 gpio-out "sysbus-irq" 1
-mmio 40012000/00ff
+mmio 40012400/00ff
   dev: stm32f2xx-adc, id ""
 gpio-out "sysbus-irq" 1
-mmio /00ff
+mmio 40012500/00ff
   dev: armv7m, id ""

Fixes: 529fc5fd3e18ace8f739afd02dc0953354f39442
Cc: Alistair Francis 
Cc: Peter Maydell 
Cc: qemu-...@nongnu.org
Signed-off-by: Markus Armbruster 
---
 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[] = { 0x4000, 0x4400,
0x4800, 0x4C00 };
-#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(>adc_irqs), 0,
   qdev_get_gpio_in(armv7m, ADC_IRQ));
 
-dev = DEVICE(&(s->adc[i]));
-object_property_set_bool(OBJECT(>adc[i]), true, "realized", );
-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(>adc[i]), true, "realized", );
+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(>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(>adc_irqs), i));
 
 /* SPI devices */
 for (i = 0; i < STM_NUM_SPIS; i++) {
-- 
2.21.3