Re: [PATCH 01/13] hw/ide/pci: Expose legacy interrupts as GPIOs

[Mark Cave-Ayland]

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.