Re: [PATCH v3 06/11] hw/misc/macio: Realize IDE controller before accessing it

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 8/2/24 19:33, BALATON Zoltan wrote:
> On Thu, 8 Feb 2024, Philippe Mathieu-Daudé wrote:
>> We should not wire IRQs on unrealized device.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/misc/macio/macio.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
>> index c9f22f8515..db662a2065 100644
>> --- a/hw/misc/macio/macio.c
>> +++ b/hw/misc/macio/macio.c
>> @@ -122,15 +122,17 @@ static bool macio_realize_ide(MacIOState *s, 
>> MACIOIDEState *ide,
>>                               Error **errp)
>> {
>>     SysBusDevice *sbd = SYS_BUS_DEVICE(ide);
>> +    bool success;
>>
>> -    sysbus_connect_irq(sbd, 0, irq0);
>> -    sysbus_connect_irq(sbd, 1, irq1);
>>     qdev_prop_set_uint32(DEVICE(ide), "channel", dmaid);
>>     object_property_set_link(OBJECT(ide), "dbdma", OBJECT(&s->dbdma),
>>                              &error_abort);
>>     macio_ide_register_dma(ide);
>> +    success = qdev_realize(DEVICE(ide), BUS(&s->macio_bus), errp);
> 
> If realize is unsuccessful can you connect irqs if device may be 
> unrealized? So maybe either the next two lines should be in an if block 
> or drop the success flag and return false here if (!qdev_realize) and 
> true at end of func?

Doh you are right, thanks!

>> +    sysbus_connect_irq(sbd, 0, irq0);
>> +    sysbus_connect_irq(sbd, 1, irq1);
>>
>> -    return qdev_realize(DEVICE(ide), BUS(&s->macio_bus), errp);
>> +    return success;
>> }
>>
>> static void macio_oldworld_realize(PCIDevice *d, Error **errp)
>>