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

[Bernhard Beschow]

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.