[PATCH v3 06/18] hw/ide/piix: Ensure IDE output IRQs are wired at realization

Philippe Mathieu-Daudé posted 18 patches 2 years, 7 months ago
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, "Hervé Poussineau" <hpoussin@reactos.org>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, John Snow <jsnow@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Aurelien Jarno <aurelien@aurel32.net>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>
[PATCH v3 06/18] hw/ide/piix: Ensure IDE output IRQs are wired at realization
Posted by Philippe Mathieu-Daudé 2 years, 7 months ago
Ensure both IDE output IRQ lines are wired.

We can remove the last use of isa_get_irq(NULL).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/piix.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index cb527553e2..91424e5249 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -134,14 +134,17 @@ static bool pci_piix_init_bus(PCIIDEState *d, unsigned i, Error **errp)
     static const struct {
         int iobase;
         int iobase2;
-        int isairq;
     } port_info[] = {
-        {0x1f0, 0x3f6, 14},
-        {0x170, 0x376, 15},
+        {0x1f0, 0x3f6},
+        {0x170, 0x376},
     };
     int ret;
 
-    qemu_irq irq_out = d->isa_irq[i] ? : isa_get_irq(NULL, port_info[i].isairq);
+    if (!d->isa_irq[i]) {
+        error_setg(errp, "output IDE IRQ %u not connected", i);
+        return false;
+    }
+
     ide_bus_init(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
     ret = ide_bus_init_ioport_isa(&d->bus[i], NULL, port_info[i].iobase,
                                   port_info[i].iobase2);
@@ -150,7 +153,7 @@ static bool pci_piix_init_bus(PCIIDEState *d, unsigned i, Error **errp)
                          object_get_typename(OBJECT(d)), i);
         return false;
     }
-    ide_bus_init_output_irq(&d->bus[i], irq_out);
+    ide_bus_init_output_irq(&d->bus[i], d->isa_irq[i]);
 
     bmdma_init(&d->bus[i], &d->bmdma[i], d);
     d->bmdma[i].bus = &d->bus[i];
-- 
2.38.1


Re: [PATCH v3 06/18] hw/ide/piix: Ensure IDE output IRQs are wired at realization
Posted by Mark Cave-Ayland 2 years, 5 months ago
On 02/03/2023 22:40, Philippe Mathieu-Daudé wrote:

> Ensure both IDE output IRQ lines are wired.
> 
> We can remove the last use of isa_get_irq(NULL).
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/ide/piix.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index cb527553e2..91424e5249 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -134,14 +134,17 @@ static bool pci_piix_init_bus(PCIIDEState *d, unsigned i, Error **errp)
>       static const struct {
>           int iobase;
>           int iobase2;
> -        int isairq;
>       } port_info[] = {
> -        {0x1f0, 0x3f6, 14},
> -        {0x170, 0x376, 15},
> +        {0x1f0, 0x3f6},
> +        {0x170, 0x376},
>       };
>       int ret;
>   
> -    qemu_irq irq_out = d->isa_irq[i] ? : isa_get_irq(NULL, port_info[i].isairq);
> +    if (!d->isa_irq[i]) {
> +        error_setg(errp, "output IDE IRQ %u not connected", i);
> +        return false;
> +    }
> +
>       ide_bus_init(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
>       ret = ide_bus_init_ioport_isa(&d->bus[i], NULL, port_info[i].iobase,
>                                     port_info[i].iobase2);
> @@ -150,7 +153,7 @@ static bool pci_piix_init_bus(PCIIDEState *d, unsigned i, Error **errp)
>                            object_get_typename(OBJECT(d)), i);
>           return false;
>       }
> -    ide_bus_init_output_irq(&d->bus[i], irq_out);
> +    ide_bus_init_output_irq(&d->bus[i], d->isa_irq[i]);
>   
>       bmdma_init(&d->bus[i], &d->bmdma[i], d);
>       d->bmdma[i].bus = &d->bus[i];

I'm not sure I agree with this, since an unwired IRQ/gpio is normally a no-op rather 
than an error - do we really want to change this just for one case? Plus wiring with 
legacy_irqs you should already have eliminated isa_get_irq() in the device itself.


ATB,

Mark.