Re: [PATCH 7/7] hw: Simplify using sysbus_init_irqs() [manual]

[Markus Armbruster]

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

> Audit the sysbus_init_irq() calls and manually convert
> to sysbus_init_irqs() when a loop is involved.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/intc/loongarch_extioi.c | 3 +--
>  hw/intc/omap_intc.c        | 3 +--
>  hw/pci-host/gpex.c         | 2 +-
>  hw/timer/renesas_tmr.c     | 9 +++------
>  4 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/hw/intc/loongarch_extioi.c b/hw/intc/loongarch_extioi.c
> index db941de20e..c579636215 100644
> --- a/hw/intc/loongarch_extioi.c
> +++ b/hw/intc/loongarch_extioi.c
> @@ -275,8 +275,7 @@ static void loongarch_extioi_instance_init(Object *obj)
>      LoongArchExtIOI *s = LOONGARCH_EXTIOI(obj);
>      int cpu, pin;
>  
> -    sysbus_init_irqs(SYS_BUS_DEVICE(dev), s->irq, EXTIOI_IRQS);
> -
> +    sysbus_init_irqs(dev, s->irq, EXTIOI_IRQS);

Commit message claims "when a loop is involved".  No loop here.  That
work was actually done in the previous patch:

  diff --git a/hw/intc/loongarch_extioi.c b/hw/intc/loongarch_extioi.c
  index 0e7a3e32f3..db941de20e 100644
  --- a/hw/intc/loongarch_extioi.c
  +++ b/hw/intc/loongarch_extioi.c
  @@ -273,11 +273,9 @@ static void loongarch_extioi_instance_init(Object *obj)
   {
       SysBusDevice *dev = SYS_BUS_DEVICE(obj);
       LoongArchExtIOI *s = LOONGARCH_EXTIOI(obj);
  -    int i, cpu, pin;
  +    int cpu, pin;

  -    for (i = 0; i < EXTIOI_IRQS; i++) {
  -        sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[i]);
  -    }
  +    sysbus_init_irqs(SYS_BUS_DEVICE(dev), s->irq, EXTIOI_IRQS);

       qdev_init_gpio_in(DEVICE(obj), extioi_setirq, EXTIOI_IRQS);

In this patch, you merely delete a superfluous type conversion that is
present even before your series.

There are more of them in this function.  Please delete them all, and in
a separate patch.

Actually, there are more elsewhere.  Coccinelle script

    @@
    typedef SysBusDevice;
    SysBusDevice *dev;
    @@
    -    SYS_BUS_DEVICE(dev)
    +    dev

finds some in hw/arm/xlnx-versal.c and hw/rx/rx62n.c, too.

Would be nice to do this for every QOM type, but I don't know how
without duplicating the semantic patch for each of them.  There are
almost 150 uses os OBJECT_DECLARE_TYPE()...

You might want to address this in a separate series, to not delay this
one.

>      qdev_init_gpio_in(DEVICE(obj), extioi_setirq, EXTIOI_IRQS);
>  
>      for (cpu = 0; cpu < EXTIOI_CPUS; cpu++) {
> diff --git a/hw/intc/omap_intc.c b/hw/intc/omap_intc.c
> index 647bf324a8..f324b640e3 100644
> --- a/hw/intc/omap_intc.c
> +++ b/hw/intc/omap_intc.c
> @@ -627,8 +627,7 @@ static void omap2_intc_init(Object *obj)
>  
>      s->level_only = 1;
>      s->nbanks = 3;
> -    sysbus_init_irq(sbd, &s->parent_intr[0]);
> -    sysbus_init_irq(sbd, &s->parent_intr[1]);
> +    sysbus_init_irqs(sbd, s->parent_intr, ARRAY_SIZE(s->parent_intr));

Unrolled loop.  s->parent_intr[] indeed has 2 elements.  Okay.

>      qdev_init_gpio_in(dev, omap_set_intr_noedge, s->nbanks * 32);
>      memory_region_init_io(&s->mmio, obj, &omap2_inth_mem_ops, s,
>                            "omap2-intc", 0x1000);

[...]