[PATCH 1/3] hw/isa/vt82c686: Turn "intr" irq into a named gpio

Bernhard Beschow posted 3 patches 4 months, 3 weeks ago
Maintainers: "Philippe Mathieu-Daudé" <philmd@linaro.org>, Aurelien Jarno <aurelien@aurel32.net>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Hervé Poussineau" <hpoussin@reactos.org>, BALATON Zoltan <balaton@eik.bme.hu>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Huacai Chen <chenhuacai@kernel.org>
[PATCH 1/3] hw/isa/vt82c686: Turn "intr" irq into a named gpio
Posted by Bernhard Beschow 4 months, 3 weeks ago
Makes the code more comprehensible, matches the datasheet and the piix4 device
model.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/vt82c686.c   | 2 +-
 hw/mips/fuloong2e.c | 2 +-
 hw/ppc/amigaone.c   | 4 ++--
 hw/ppc/pegasos2.c   | 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 8582ac0322..505b44c4e6 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -719,7 +719,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
     ISABus *isa_bus;
     int i;
 
-    qdev_init_gpio_out(dev, &s->cpu_intr, 1);
+    qdev_init_gpio_out_named(dev, &s->cpu_intr, "intr", 1);
     qdev_init_gpio_in_named(dev, via_isa_pirq, "pirq", PCI_NUM_PINS);
     isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
     isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index a45aac368c..6e4303ba47 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -299,7 +299,7 @@ static void mips_fuloong2e_init(MachineState *machine)
                               object_resolve_path_component(OBJECT(pci_dev),
                                                             "rtc"),
                               "date");
-    qdev_connect_gpio_out(DEVICE(pci_dev), 0, env->irq[5]);
+    qdev_connect_gpio_out_named(DEVICE(pci_dev), "intr", 0, env->irq[5]);
 
     dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide"));
     pci_ide_create_devs(PCI_DEVICE(dev));
diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
index ddfa09457a..9dcc486c1a 100644
--- a/hw/ppc/amigaone.c
+++ b/hw/ppc/amigaone.c
@@ -153,8 +153,8 @@ static void amigaone_init(MachineState *machine)
     object_property_add_alias(OBJECT(machine), "rtc-time",
                               object_resolve_path_component(via, "rtc"),
                               "date");
-    qdev_connect_gpio_out(DEVICE(via), 0,
-                          qdev_get_gpio_in(DEVICE(cpu), PPC6xx_INPUT_INT));
+    qdev_connect_gpio_out_named(DEVICE(via), "intr", 0,
+                                qdev_get_gpio_in(DEVICE(cpu), PPC6xx_INPUT_INT));
     for (i = 0; i < PCI_NUM_PINS; i++) {
         qdev_connect_gpio_out(dev, i, qdev_get_gpio_in_named(DEVICE(via),
                                                              "pirq", i));
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index c1bd8dfa21..9b0a6b70ab 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -195,8 +195,8 @@ static void pegasos2_init(MachineState *machine)
     object_property_add_alias(OBJECT(machine), "rtc-time",
                               object_resolve_path_component(via, "rtc"),
                               "date");
-    qdev_connect_gpio_out(DEVICE(via), 0,
-                          qdev_get_gpio_in_named(pm->mv, "gpp", 31));
+    qdev_connect_gpio_out_named(DEVICE(via), "intr", 0,
+                                qdev_get_gpio_in_named(pm->mv, "gpp", 31));
 
     dev = PCI_DEVICE(object_resolve_path_component(via, "ide"));
     pci_ide_create_devs(dev);
-- 
2.45.2
Re: [PATCH 1/3] hw/isa/vt82c686: Turn "intr" irq into a named gpio
Posted by Philippe Mathieu-Daudé 4 months, 1 week ago
On 4/7/24 22:58, Bernhard Beschow wrote:
> Makes the code more comprehensible, matches the datasheet and the piix4 device
> model.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/isa/vt82c686.c   | 2 +-
>   hw/mips/fuloong2e.c | 2 +-
>   hw/ppc/amigaone.c   | 4 ++--
>   hw/ppc/pegasos2.c   | 4 ++--
>   4 files changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>


Re: [PATCH 1/3] hw/isa/vt82c686: Turn "intr" irq into a named gpio
Posted by BALATON Zoltan 4 months, 3 weeks ago
On Thu, 4 Jul 2024, Bernhard Beschow wrote:
> Makes the code more comprehensible, matches the datasheet and the piix4 device
> model.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> hw/isa/vt82c686.c   | 2 +-
> hw/mips/fuloong2e.c | 2 +-
> hw/ppc/amigaone.c   | 4 ++--
> hw/ppc/pegasos2.c   | 4 ++--
> 4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 8582ac0322..505b44c4e6 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -719,7 +719,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>     ISABus *isa_bus;
>     int i;
>
> -    qdev_init_gpio_out(dev, &s->cpu_intr, 1);
> +    qdev_init_gpio_out_named(dev, &s->cpu_intr, "intr", 1);
>     qdev_init_gpio_in_named(dev, via_isa_pirq, "pirq", PCI_NUM_PINS);
>     isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
>     isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> index a45aac368c..6e4303ba47 100644
> --- a/hw/mips/fuloong2e.c
> +++ b/hw/mips/fuloong2e.c
> @@ -299,7 +299,7 @@ static void mips_fuloong2e_init(MachineState *machine)
>                               object_resolve_path_component(OBJECT(pci_dev),
>                                                             "rtc"),
>                               "date");
> -    qdev_connect_gpio_out(DEVICE(pci_dev), 0, env->irq[5]);
> +    qdev_connect_gpio_out_named(DEVICE(pci_dev), "intr", 0, env->irq[5]);

I was wondering why we still have 0 when we have a name so checked the doc 
commant in include/hw/qdev-core.h and found that the docs in 
qdev_connect_gpio_out_named is mostly just a copy&paste of the 
qdev_connect_gpio_out and it also talks about output GPIO array but then 
says input GPIOs in that array. I've stopped reading at that point as this 
text makes little sense. Somebody who knows how this actually works might 
want to update that doc comment.

But that's unrelated to this patch so this is

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

>
>     dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide"));
>     pci_ide_create_devs(PCI_DEVICE(dev));
> diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
> index ddfa09457a..9dcc486c1a 100644
> --- a/hw/ppc/amigaone.c
> +++ b/hw/ppc/amigaone.c
> @@ -153,8 +153,8 @@ static void amigaone_init(MachineState *machine)
>     object_property_add_alias(OBJECT(machine), "rtc-time",
>                               object_resolve_path_component(via, "rtc"),
>                               "date");
> -    qdev_connect_gpio_out(DEVICE(via), 0,
> -                          qdev_get_gpio_in(DEVICE(cpu), PPC6xx_INPUT_INT));
> +    qdev_connect_gpio_out_named(DEVICE(via), "intr", 0,
> +                                qdev_get_gpio_in(DEVICE(cpu), PPC6xx_INPUT_INT));
>     for (i = 0; i < PCI_NUM_PINS; i++) {
>         qdev_connect_gpio_out(dev, i, qdev_get_gpio_in_named(DEVICE(via),
>                                                              "pirq", i));
> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
> index c1bd8dfa21..9b0a6b70ab 100644
> --- a/hw/ppc/pegasos2.c
> +++ b/hw/ppc/pegasos2.c
> @@ -195,8 +195,8 @@ static void pegasos2_init(MachineState *machine)
>     object_property_add_alias(OBJECT(machine), "rtc-time",
>                               object_resolve_path_component(via, "rtc"),
>                               "date");
> -    qdev_connect_gpio_out(DEVICE(via), 0,
> -                          qdev_get_gpio_in_named(pm->mv, "gpp", 31));
> +    qdev_connect_gpio_out_named(DEVICE(via), "intr", 0,
> +                                qdev_get_gpio_in_named(pm->mv, "gpp", 31));
>
>     dev = PCI_DEVICE(object_resolve_path_component(via, "ide"));
>     pci_ide_create_devs(dev);
>
Re: [PATCH 1/3] hw/isa/vt82c686: Turn "intr" irq into a named gpio
Posted by Peter Maydell 4 months, 2 weeks ago
On Fri, 5 Jul 2024 at 01:32, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Thu, 4 Jul 2024, Bernhard Beschow wrote:
> > Makes the code more comprehensible, matches the datasheet and the piix4 device
> > model.
> >
> > Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> > ---
> > hw/isa/vt82c686.c   | 2 +-
> > hw/mips/fuloong2e.c | 2 +-
> > hw/ppc/amigaone.c   | 4 ++--
> > hw/ppc/pegasos2.c   | 4 ++--
> > 4 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> > index 8582ac0322..505b44c4e6 100644
> > --- a/hw/isa/vt82c686.c
> > +++ b/hw/isa/vt82c686.c
> > @@ -719,7 +719,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
> >     ISABus *isa_bus;
> >     int i;
> >
> > -    qdev_init_gpio_out(dev, &s->cpu_intr, 1);
> > +    qdev_init_gpio_out_named(dev, &s->cpu_intr, "intr", 1);
> >     qdev_init_gpio_in_named(dev, via_isa_pirq, "pirq", PCI_NUM_PINS);
> >     isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
> >     isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
> > diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> > index a45aac368c..6e4303ba47 100644
> > --- a/hw/mips/fuloong2e.c
> > +++ b/hw/mips/fuloong2e.c
> > @@ -299,7 +299,7 @@ static void mips_fuloong2e_init(MachineState *machine)
> >                               object_resolve_path_component(OBJECT(pci_dev),
> >                                                             "rtc"),
> >                               "date");
> > -    qdev_connect_gpio_out(DEVICE(pci_dev), 0, env->irq[5]);
> > +    qdev_connect_gpio_out_named(DEVICE(pci_dev), "intr", 0, env->irq[5]);
>
> I was wondering why we still have 0 when we have a name so checked the doc
> commant in include/hw/qdev-core.h and found that the docs in
> qdev_connect_gpio_out_named is mostly just a copy&paste of the
> qdev_connect_gpio_out and it also talks about output GPIO array but then
> says input GPIOs in that array. I've stopped reading at that point as this
> text makes little sense. Somebody who knows how this actually works might
> want to update that doc comment.

Yeah, there's some copy-n-paste errors there. I'll send a patch.

The answer to "why is there both a name and a number 0" is
that named GPIOs (both input and output) are always created as
arrays of GPIOs, not single GPIOs. So you can create a named GPIO
output array like this:
   qdev_init_gpio_out_named(dev, s->fiq, "fiq", BCM2836_NCORES);
and then connect to fiq 0, fiq 1, fiq 2, and so on.
A single named output GPIO is a special case of the array-output
with only one element, so when you connect it up you still have
to say "I want element 0 of this length-1 array".

(The unnamed (anonymous) GPIOs are implemented under the hood
as "a named GPIO array where the name of the GPIO array is NULL".
So their semantics and also the documentation for the functions
is very similar to that for named GPIO arrays. But there are
also some places where I made cut-n-paste errors...)

thanks
-- PMM