Re: [PATCH] hw/misc: Add an iBT device model

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

On 9/24/21 01:48, Titus Rwantare wrote:
> Hello all,
> 
> I'd like some clarification on how the following code transfers irqs
> back and forth:
> 
>> b/hw/arm/aspeed_soc.c
>> +    /* iBT */
>> +    if (!sysbus_realize(SYS_BUS_DEVICE(&s->ibt), errp)) {
>> +        return;
>> +    }
>> +    memory_region_add_subregion(&s->lpc.iomem,
>> +                   sc->memmap[ASPEED_DEV_IBT] - sc->memmap[ASPEED_DEV_LPC],
>> +                   &s->ibt.iomem);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_ibt,

Code smell indeed, likely:

         sysbus_connect_irq(SYS_BUS_DEVICE(&s->ibt), 1 + aspeed_lpc_ibt,

>> +                       qdev_get_gpio_in(DEVICE(&s->lpc), aspeed_lpc_ibt));
>> }
> 
> and
> 
>> hw/misc/aspeed_ibt.c
>> +static void aspeed_ibt_realize(DeviceState *dev, Error **errp)
>> +{
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> +    AspeedIBTState *ibt = ASPEED_IBT(dev);
> ...
>> +
>> +    sysbus_init_irq(sbd, &ibt->irq);
> 
> I ask because the code in aspeed_soc.c seems to connect to the
> lpc->subdevice_irqs[aspeed_lpc_ibt], initialised on
> hw/misc/aspeed_lpc.c:408.
> I noticed that bmc firmware running in qemu was checking the BT_CTRL
> register less frequently than I'd like while editing this patch to use
> the IPMIInterface.
> 
> Thanks,
> Titus
>