| Message ID | 20240704205854.18537-2-shentey@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Resolve vt82c686 and piix4 qemu_irq memory leaks | expand |
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); >
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
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>
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);
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(-)