Re: [PATCH 01/11] hw/rtc/mc146818rtc: Inline isa_connect_gpio_out() and remove it

[Mark Cave-Ayland]

On 05/05/2022 17:17, Bernhard Beschow wrote:

> Commit 250263033c5343012b2cd6f01210ffb5b908a159 'isa: introduce wrapper
> isa_connect_gpio_out' introduced it in 2016. Since then, its only user
> remained mc146818rtc. Remove this one-off solution.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/isa/isa-bus.c     | 6 ------
>   hw/rtc/mc146818rtc.c | 3 ++-
>   include/hw/isa/isa.h | 1 -
>   3 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
> index 0ad1c5fd65..59eb1af9e3 100644
> --- a/hw/isa/isa-bus.c
> +++ b/hw/isa/isa-bus.c
> @@ -85,12 +85,6 @@ qemu_irq isa_get_irq(ISADevice *dev, unsigned isairq)
>       return isabus->irqs[isairq];
>   }
>   
> -void isa_connect_gpio_out(ISADevice *isadev, int gpioirq, unsigned isairq)
> -{
> -    qemu_irq irq = isa_get_irq(isadev, isairq);
> -    qdev_connect_gpio_out(DEVICE(isadev), gpioirq, irq);
> -}
> -
>   void isa_bus_dma(ISABus *bus, IsaDma *dma8, IsaDma *dma16)
>   {
>       assert(bus && dma8 && dma16);
> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
> index f235c2ddbe..79c43391cb 100644
> --- a/hw/rtc/mc146818rtc.c
> +++ b/hw/rtc/mc146818rtc.c
> @@ -972,7 +972,8 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, 
> qemu_irq intercept_irq)
>       if (intercept_irq) {
>           qdev_connect_gpio_out(dev, 0, intercept_irq);
>       } else {
> -        isa_connect_gpio_out(isadev, 0, s->isairq);
> +        qemu_irq irq = isa_get_irq(isadev, s->isairq);
> +        qdev_connect_gpio_out(dev, 0, irq);
>       }
>   
>       object_property_add_alias(qdev_get_machine(), "rtc-time", OBJECT(isadev),
> diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
> index 034d706ba1..1b758ae1ab 100644
> --- a/include/hw/isa/isa.h
> +++ b/include/hw/isa/isa.h
> @@ -90,7 +90,6 @@ ISABus *isa_bus_new(DeviceState *dev, MemoryRegion 
> *address_space,
>                       MemoryRegion *address_space_io, Error **errp);
>   void isa_bus_irqs(ISABus *bus, qemu_irq *irqs);
>   qemu_irq isa_get_irq(ISADevice *dev, unsigned isairq);
> -void isa_connect_gpio_out(ISADevice *isadev, int gpioirq, unsigned isairq);
>   void isa_bus_dma(ISABus *bus, IsaDma *dma8, IsaDma *dma16);
>   IsaDma *isa_get_dma(ISABus *bus, int nchan);
>   MemoryRegion *isa_address_space(ISADevice *dev);

Hi Bernhard,

I've been tinkering again with trying to improve QEMU's ISA implementation to use 
gpios, and actually I believe as per the comment in isa-bus.c that 
isa_connect_gpio_out() will be the preferred way to wire up ISA devices. So really we 
should be trying to use this function more rather than removing it.

BTW if you are interested in helping to work on ISA bus improvements, I can certainly 
come up with a TODO list of useful tasks :)


ATB,

Mark.