| Message ID | 20230422150728.176512-2-shentey@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Clean up PCI IDE device models | expand |
On 22/04/2023 16:07, Bernhard Beschow wrote: > Exposing the legacy IDE interrupts as GPIOs allows them to be connected in the > parent device through qdev_connect_gpio_out(), i.e. without accessing private > data of TYPE_PCI_IDE. > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > --- > hw/ide/pci.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/hw/ide/pci.c b/hw/ide/pci.c > index fc9224bbc9..942e216b9b 100644 > --- a/hw/ide/pci.c > +++ b/hw/ide/pci.c > @@ -522,10 +522,18 @@ void bmdma_init(IDEBus *bus, BMDMAState *bm, PCIIDEState *d) > bm->pci_dev = d; > } > > +static void pci_ide_init(Object *obj) > +{ > + PCIIDEState *d = PCI_IDE(obj); > + > + qdev_init_gpio_out(DEVICE(d), d->isa_irq, ARRAY_SIZE(d->isa_irq)); Just one minor nit: can we make this qdev_init_gpio_out_named() and call it "isa-irq" to match? This is for 2 reasons: firstly these are PCI devices and so an unnamed IRQ/gpio could be considered to belong to PCI, and secondly it gives the gpio the same name as the struct field. From my previous email I think this should supercede Phil's patch at https://patchew.org/QEMU/20230302224058.43315-1-philmd@linaro.org/20230302224058.43315-2-philmd@linaro.org/. > +} > + > static const TypeInfo pci_ide_type_info = { > .name = TYPE_PCI_IDE, > .parent = TYPE_PCI_DEVICE, > .instance_size = sizeof(PCIIDEState), > + .instance_init = pci_ide_init, > .abstract = true, > .interfaces = (InterfaceInfo[]) { > { INTERFACE_CONVENTIONAL_PCI_DEVICE }, Otherwise: Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> ATB, Mark.
Am 26. April 2023 10:41:30 UTC schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>: >On 22/04/2023 16:07, Bernhard Beschow wrote: > >> Exposing the legacy IDE interrupts as GPIOs allows them to be connected in the >> parent device through qdev_connect_gpio_out(), i.e. without accessing private >> data of TYPE_PCI_IDE. >> >> Signed-off-by: Bernhard Beschow <shentey@gmail.com> >> --- >> hw/ide/pci.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/hw/ide/pci.c b/hw/ide/pci.c >> index fc9224bbc9..942e216b9b 100644 >> --- a/hw/ide/pci.c >> +++ b/hw/ide/pci.c >> @@ -522,10 +522,18 @@ void bmdma_init(IDEBus *bus, BMDMAState *bm, PCIIDEState *d) >> bm->pci_dev = d; >> } >> +static void pci_ide_init(Object *obj) >> +{ >> + PCIIDEState *d = PCI_IDE(obj); >> + >> + qdev_init_gpio_out(DEVICE(d), d->isa_irq, ARRAY_SIZE(d->isa_irq)); > >Just one minor nit: can we make this qdev_init_gpio_out_named() and call it "isa-irq" to match? This is for 2 reasons: firstly these are PCI devices and so an unnamed IRQ/gpio could be considered to belong to PCI, and secondly it gives the gpio the same name as the struct field. Yes, makes sense. > >From my previous email I think this should supercede Phil's patch at https://patchew.org/QEMU/20230302224058.43315-1-philmd@linaro.org/20230302224058.43315-2-philmd@linaro.org/. > >> +} >> + >> static const TypeInfo pci_ide_type_info = { >> .name = TYPE_PCI_IDE, >> .parent = TYPE_PCI_DEVICE, >> .instance_size = sizeof(PCIIDEState), >> + .instance_init = pci_ide_init, >> .abstract = true, >> .interfaces = (InterfaceInfo[]) { >> { INTERFACE_CONVENTIONAL_PCI_DEVICE }, > >Otherwise: > >Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > > >ATB, > >Mark.
diff --git a/hw/ide/pci.c b/hw/ide/pci.c index fc9224bbc9..942e216b9b 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -522,10 +522,18 @@ void bmdma_init(IDEBus *bus, BMDMAState *bm, PCIIDEState *d) bm->pci_dev = d; } +static void pci_ide_init(Object *obj) +{ + PCIIDEState *d = PCI_IDE(obj); + + qdev_init_gpio_out(DEVICE(d), d->isa_irq, ARRAY_SIZE(d->isa_irq)); +} + static const TypeInfo pci_ide_type_info = { .name = TYPE_PCI_IDE, .parent = TYPE_PCI_DEVICE, .instance_size = sizeof(PCIIDEState), + .instance_init = pci_ide_init, .abstract = true, .interfaces = (InterfaceInfo[]) { { INTERFACE_CONVENTIONAL_PCI_DEVICE },
Exposing the legacy IDE interrupts as GPIOs allows them to be connected in the parent device through qdev_connect_gpio_out(), i.e. without accessing private data of TYPE_PCI_IDE. Signed-off-by: Bernhard Beschow <shentey@gmail.com> --- hw/ide/pci.c | 8 ++++++++ 1 file changed, 8 insertions(+)