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

[Bernhard Beschow]

Am 11. Mai 2022 10:24:58 UTC schrieb Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk>:
>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,

Hi Mark,

>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 :)

Yes, that'd be great!

As expressed earlier [1] I'd like to remove the isabus global in
isa-bus.c. Meanwhile, I implemented a POC [2]. While the result seems
to work for the PC and Malta boards, some of the changes required
don't seem quite clean. So yeah, I'd be interested in the bigger
picture regarding ISA improvements and would like to help out as time
permits, too.

Best regards,
Bernhard

[1] https://lists.nongnu.org/archive/html/qemu-devel/2022-03/msg05955.html
[2] https://github.com/shentok/qemu/commits/isa

>
>ATB,
>
>Mark.