Re: [PATCH v6 28/33] hw/isa/piix3: Merge hw/isa/piix4.c

[Philippe Mathieu-Daudé]

On 9/1/23 18:23, Bernhard Beschow wrote:
> Now that the PIIX3 and PIIX4 device models are sufficiently consolidated,
> their implementations can be merged into one file for further
> consolidation.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Message-Id: <20221022150508.26830-37-shentey@gmail.com>
> ---
>   hw/isa/{piix3.c => piix.c} | 158 ++++++++++++++++++++
>   hw/isa/piix4.c             | 285 -------------------------------------
>   MAINTAINERS                |   6 +-
>   hw/i386/Kconfig            |   2 +-
>   hw/isa/Kconfig             |  12 +-
>   hw/isa/meson.build         |   3 +-
>   hw/mips/Kconfig            |   2 +-
>   7 files changed, 165 insertions(+), 303 deletions(-)
>   rename hw/isa/{piix3.c => piix.c} (75%)
>   delete mode 100644 hw/isa/piix4.c

> +static void piix4_realize(PCIDevice *dev, Error **errp)
> +{
> +    PIIXState *s = PIIX_PCI_DEVICE(dev);
> +    PCIBus *pci_bus = pci_get_bus(dev);
> +    ISABus *isa_bus;
> +
> +    isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
> +                          pci_address_space_io(dev), errp);
> +    if (!isa_bus) {
> +        return;
> +    }
> +
> +    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &rcr_ops, s,
> +                          "reset-control", 1);
> +    memory_region_add_subregion_overlap(pci_address_space_io(dev),
> +                                        PIIX_RCR_IOPORT, &s->rcr_mem, 1);
> +
> +    /* initialize i8259 pic */
> +    if (!qdev_realize(DEVICE(&s->pic), NULL, errp)) {
> +        return;
> +    }
> +
> +    /* initialize ISA irqs */
> +    isa_bus_irqs(isa_bus, s->pic.in_irqs);
> +
> +    /* initialize pit */
> +    i8254_pit_init(isa_bus, 0x40, 0, NULL);
> +
> +    /* DMA */
> +    i8257_dma_init(isa_bus, 0);
> +
> +    /* RTC */
> +    qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
> +    if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
> +        return;
> +    }
> +    s->rtc.irq = qdev_get_gpio_in(DEVICE(&s->pic), s->rtc.isairq);

Pre-existing; it seems this difference with PIIX3 can be removed
because already taken care by calling isa_connect_gpio_out() in 
mc146818_rtc_init()?

ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq 
intercept_irq)
{
    DeviceState *dev;
    ISADevice *isadev;
    RTCState *s;

    isadev = isa_new(TYPE_MC146818_RTC);
    dev = DEVICE(isadev);
    s = MC146818_RTC(isadev);
    qdev_prop_set_int32(dev, "base_year", base_year);
    isa_realize_and_unref(isadev, bus, &error_fatal);
    if (intercept_irq) {
        qdev_connect_gpio_out(dev, 0, intercept_irq);
    } else {
        isa_connect_gpio_out(isadev, 0, s->isairq);

Having:

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);
}

What do you think?