Re: [PATCH v3 2/3] hw/arm: Connect BSC to BCM2835 board as I2C0, I2C1 and I2C2

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

On 20/2/24 14:41, Rayhan Faizel wrote:
> BCM2835 has three I2C controllers. All of them share the same interrupt line.
> 
> Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com>
> ---
>   hw/arm/Kconfig                       |  1 +
>   hw/arm/bcm2835_peripherals.c         | 32 +++++++++++++++++++++++++---
>   include/hw/arm/bcm2835_peripherals.h |  3 ++-
>   3 files changed, 32 insertions(+), 4 deletions(-)


> diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
> index d5573fd954..ca692ed9a5 100644
> --- a/hw/arm/bcm2835_peripherals.c
> +++ b/hw/arm/bcm2835_peripherals.c
> @@ -148,6 +148,14 @@ static void bcm2835_peripherals_init(Object *obj)
>       /* SPI */
>       object_initialize_child(obj, "bcm2835-spi0", &s->spi[0],
>                               TYPE_BCM2835_SPI);
> +
> +    /* I2C */
> +    object_initialize_child(obj, "bcm2835-i2c0", &s->i2c[0],
> +                            TYPE_BCM2835_I2C);
> +    object_initialize_child(obj, "bcm2835-i2c1", &s->i2c[1],
> +                            TYPE_BCM2835_I2C);
> +    object_initialize_child(obj, "bcm2835-i2c2", &s->i2c[2],
> +                            TYPE_BCM2835_I2C);
>   }
>   
>   static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
> @@ -418,14 +426,32 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
>                                                 BCM2835_IC_GPU_IRQ,
>                                                 INTERRUPT_SPI));
>   
> +    /* I2C */
> +    for (n = 0; n < 3; n++) {
> +        if (!sysbus_realize(SYS_BUS_DEVICE(&s->i2c[n]), errp)) {
> +            return;
> +        }
> +    }
> +
> +    memory_region_add_subregion(&s->peri_mr, BSC0_OFFSET,
> +            sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->i2c[0]), 0));
> +    memory_region_add_subregion(&s->peri_mr, BSC1_OFFSET,
> +            sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->i2c[1]), 0));
> +    memory_region_add_subregion(&s->peri_mr, BSC2_OFFSET,
> +            sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->i2c[2]), 0));
> +
> +    for (n = 0; n < 3; n++) {
> +        sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c[n]), 0,
> +                           qdev_get_gpio_in_named(DEVICE(&s->ic),
> +                                                  BCM2835_IC_GPU_IRQ,

Due to how QEMU IRQs are implemented, we can not wire multiple IRQs
to the same output without using an intermediate "OR gate". We model
it as TYPE_OR_IRQ. See the comment in "hw/qdev-core.h" added in
commit cd07d7f9f5 ("qdev: Document GPIO related functions"):

   * It is not valid to try to connect one outbound GPIO to multiple
   * qemu_irqs at once, or to connect multiple outbound GPIOs to the
   * same qemu_irq. (Warning: there is no assertion or other guard to
   * catch this error: the model will just not do the right thing.)
   * Instead, for fan-out you can use the TYPE_SPLIT_IRQ device: connect
   * a device's outbound GPIO to the splitter's input, and connect each
   * of the splitter's outputs to a different device.  For fan-in you
   * can use the TYPE_OR_IRQ device, which is a model of a logical OR
   * gate with multiple inputs and one output.

> +                                                  INTERRUPT_I2C));
> +    }
> +
>       create_unimp(s, &s->txp, "bcm2835-txp", TXP_OFFSET, 0x1000);
>       create_unimp(s, &s->armtmr, "bcm2835-sp804", ARMCTRL_TIMER0_1_OFFSET, 0x40);
>       create_unimp(s, &s->i2s, "bcm2835-i2s", I2S_OFFSET, 0x100);
>       create_unimp(s, &s->smi, "bcm2835-smi", SMI_OFFSET, 0x100);
>       create_unimp(s, &s->bscsl, "bcm2835-spis", BSC_SL_OFFSET, 0x100);
> -    create_unimp(s, &s->i2c[0], "bcm2835-i2c0", BSC0_OFFSET, 0x20);
> -    create_unimp(s, &s->i2c[1], "bcm2835-i2c1", BSC1_OFFSET, 0x20);
> -    create_unimp(s, &s->i2c[2], "bcm2835-i2c2", BSC2_OFFSET, 0x20);
>       create_unimp(s, &s->otp, "bcm2835-otp", OTP_OFFSET, 0x80);
>       create_unimp(s, &s->dbus, "bcm2835-dbus", DBUS_OFFSET, 0x8000);
>       create_unimp(s, &s->ave0, "bcm2835-ave0", AVE0_OFFSET, 0x8000);