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

[Philippe Mathieu-Daudé]

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