Re: [PATCH v5 3/3] hw/arm: Connect STM32L4x5 EXTI to STM32L4x5 SoC

[Philippe Mathieu-Daudé]

On 28/12/23 17:19, Inès Varhol wrote:
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
>   hw/arm/Kconfig                 |  1 +
>   hw/arm/stm32l4x5_soc.c         | 56 ++++++++++++++++++++++++++++++++--
>   include/hw/arm/stm32l4x5_soc.h |  3 ++
>   3 files changed, 58 insertions(+), 2 deletions(-)


>   static void stm32l4x5_soc_initfn(Object *obj)
>   {
>       Stm32l4x5SocState *s = STM32L4X5_SOC(obj);
>   
> +    object_initialize_child(obj, "exti", &s->exti, TYPE_STM32L4X5_EXTI);
> +
>       s->sysclk = qdev_init_clock_in(DEVICE(s), "sysclk", NULL, NULL, 0);
>       s->refclk = qdev_init_clock_in(DEVICE(s), "refclk", NULL, NULL, 0);
>   }
> @@ -50,7 +91,9 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error 
> **errp)
>       Stm32l4x5SocState *s = STM32L4X5_SOC(dev_soc);
>       const Stm32l4x5SocClass *sc = STM32L4X5_SOC_GET_CLASS(dev_soc);
>       MemoryRegion *system_memory = get_system_memory();
> -    DeviceState *armv7m;
> +    DeviceState *dev, *armv7m;
> +    SysBusDevice *busdev;
> +    int i;
>   
>       /*
>        * We use s->refclk internally and only define it with 
> qdev_init_clock_in()
> @@ -115,6 +158,16 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, 
> Error **errp)
>           return;
>       }
>   
> +    dev = DEVICE(&s->exti);

'dev' isn't used, you can drop it and directly use:

       busdev = SYS_BUS_DEVICE(&s->exti);

> +    if (!sysbus_realize(SYS_BUS_DEVICE(&s->exti), errp)) {

Here you can use 'busdev' directly.

> +        return;
> +    }
> +    busdev = SYS_BUS_DEVICE(dev);
> +    sysbus_mmio_map(busdev, 0, EXTI_ADDR);
> +    for (i = 0; i < NUM_EXTI_IRQ; i++) {

Preferably reduce 'i' scope:

       for (unsigned i = 0; ...) {

> +        sysbus_connect_irq(busdev, i, qdev_get_gpio_in(armv7m, exti_irq[i]));
> +    }

Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

With the comments addressed:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>