Re: [PATCH 07/11] hw/m68k/next-cube: Make next_irq GPIO inputs to NEXT_PC device

From: Thomas Huth <huth@tuxfamily.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-devel@nongnu.org, Laurent Vivier <laurent@vivier.eu>
Subject: Re: [PATCH 07/11] hw/m68k/next-cube: Make next_irq GPIO inputs to NEXT_PC device
Date: Sat, 16 Jan 2021 11:24:48 +0100	[thread overview]
Message-ID: <20210116112448.15c2b8dd@tuxfamily.org> (raw)
In-Reply-To: <20210115201206.17347-8-peter.maydell@linaro.org>

Am Fri, 15 Jan 2021 20:12:02 +0000
schrieb Peter Maydell <peter.maydell@linaro.org>:

> Make the next_irq function be GPIO inputs to the NEXT_PC
> device, rather than a freestanding set of qemu_irq lines.
> 
> This fixes a minor Coverity issue where it correctly points
> out the trivial memory leak of the memory allocated in the
> call to qemu_allocate_irqs().
> 
> Fixes: CID 1421962
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/m68k/next-cube.h |  3 ++-
>  hw/m68k/next-cube.c         | 21 ++++-----------------
>  2 files changed, 6 insertions(+), 18 deletions(-)
> 
> diff --git a/include/hw/m68k/next-cube.h b/include/hw/m68k/next-cube.h
> index 5a56c354b8e..d38c52d540d 100644
> --- a/include/hw/m68k/next-cube.h
> +++ b/include/hw/m68k/next-cube.h
> @@ -39,7 +39,8 @@ enum next_irqs {
>      NEXT_ENRX_DMA_I,
>      NEXT_SCSI_DMA_I,
>      NEXT_SCC_DMA_I,
> -    NEXT_SND_I
> +    NEXT_SND_I,
> +    NEXT_NUM_IRQS
>  };
>  
>  #endif /* NEXT_CUBE_H */
> diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
> index 6b4bcfd4b9b..5a8fc24ed35 100644
> --- a/hw/m68k/next-cube.c
> +++ b/hw/m68k/next-cube.c
> @@ -734,10 +734,6 @@ static const MemoryRegionOps dma_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
> -/*
> - * TODO: set the shift numbers as values in the enum, so the first
> switch
> - * will not be needed
> - */
>  static void next_irq(void *opaque, int number, int level)
>  {
>      NeXTPC *s = NEXT_PC(opaque);
> @@ -838,19 +834,8 @@ static void next_irq(void *opaque, int number,
> int level) }
>  }
>  
> -static void next_serial_irq(void *opaque, int n, int level)
> -{
> -    /* DPRINTF("SCC IRQ NUM %i\n",n); */
> -    if (n) {
> -        next_irq(opaque, NEXT_SCC_DMA_I, level);
> -    } else {
> -        next_irq(opaque, NEXT_SCC_I, level);
> -    }
> -}
> -
>  static void next_escc_init(DeviceState *pcdev)
>  {
> -    qemu_irq *ser_irq = qemu_allocate_irqs(next_serial_irq, pcdev,
> 2); DeviceState *dev;
>      SysBusDevice *s;
>  
> @@ -866,8 +851,8 @@ static void next_escc_init(DeviceState *pcdev)
>  
>      s = SYS_BUS_DEVICE(dev);
>      sysbus_realize_and_unref(s, &error_fatal);
> -    sysbus_connect_irq(s, 0, ser_irq[0]);
> -    sysbus_connect_irq(s, 1,  ser_irq[1]);
> +    sysbus_connect_irq(s, 0, qdev_get_gpio_in(pcdev, NEXT_SCC_I));
> +    sysbus_connect_irq(s, 1, qdev_get_gpio_in(pcdev,
> NEXT_SCC_DMA_I)); sysbus_mmio_map(s, 0, 0x2118000);
>  }
>  
> @@ -886,6 +871,8 @@ static void next_pc_realize(DeviceState *dev,
> Error **errp) NeXTPC *s = NEXT_PC(dev);
>      SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>  
> +    qdev_init_gpio_in(dev, next_irq, NEXT_NUM_IRQS);
> +
>      memory_region_init_io(&s->mmiomem, OBJECT(s), &mmio_ops, s,
>                            "next.mmio", 0xD0000);
>      memory_region_init_io(&s->scrmem, OBJECT(s), &scr_ops, s,

Acked-by: Thomas Huth <huth@tuxfamily.org>