Re: [PATCH v2 02/15] hw/ide: Convert macio ide_irq into GPIO line

[Mark Cave-Ayland]

On 27/06/2024 14:37, Akihiko Odaki wrote:

> macio ide_irq is connected to the IDE bus. This fixes the leak of
> ide_irq.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   hw/ide/macio.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
> index aca90d04f0e8..9c96a857a7c1 100644
> --- a/hw/ide/macio.c
> +++ b/hw/ide/macio.c
> @@ -427,7 +427,7 @@ static void macio_ide_realizefn(DeviceState *dev, Error 
> **errp)
>       s->bus.dma = &s->dma;
>   }
>   
> -static void pmac_ide_irq(void *opaque, int n, int level)
> +static void pmac_irq(void *opaque, int n, int level)
>   {
>       MACIOIDEState *s = opaque;
>       uint32_t mask = 0x80000000u >> n;
> @@ -446,6 +446,11 @@ static void pmac_ide_irq(void *opaque, int n, int level)
>       }
>   }
>   
> +static void pmac_ide_irq(void *opaque, int n, int level)
> +{
> +    pmac_irq(opaque, 1, level);
> +}
> +
>   static void macio_ide_initfn(Object *obj)
>   {
>       SysBusDevice *d = SYS_BUS_DEVICE(obj);
> @@ -456,8 +461,8 @@ static void macio_ide_initfn(Object *obj)
>       sysbus_init_mmio(d, &s->mem);
>       sysbus_init_irq(d, &s->real_ide_irq);
>       sysbus_init_irq(d, &s->real_dma_irq);
> -    s->dma_irq = qemu_allocate_irq(pmac_ide_irq, s, 0);
> -    s->ide_irq = qemu_allocate_irq(pmac_ide_irq, s, 1);
> +    s->dma_irq = qemu_allocate_irq(pmac_irq, s, 0);
> +    qdev_init_gpio_in_named_with_opaque(DEVICE(obj), pmac_ide_irq, s, NULL, 1);
>   
>       object_property_add_link(obj, "dbdma", TYPE_MAC_DBDMA,
>                                (Object **) &s->dbdma,

This doesn't feel quite right: generally I consider the use of 
qdev_init_gpio_in_named_with_opaque() to indicate that the underlying modelling is 
incorrect. Let me have a look and see if I can figure out what's supposed to be 
happening.

I guess I should probably be marked as maintainer of hw/ide/macio.c as it is part of 
the macio device, but it looks as if this is missing from MAINTAINERS.


ATB,

Mark.