Re: [PATCH v1 04/11] hw: arm: allwinner-r40: Add 5 TWI controllers

[Strahinja Jankovic <strahinjapjankovic@gmail.com>]

Hi,

On Tue, Mar 21, 2023 at 11:25 AM <qianfanguijin@163.com> wrote:
>
> From: qianfan Zhao <qianfanguijin@163.com>
>
> TWI(i2c) is designed to be used as an interface between CPU host and the
> serial 2-Wire bus. It can support all standard 2-Wire transfer, can be
> operated in standard mode(100kbit/s) or fast-mode, supporting data rate
> up to 400kbit/s.
>
> Signed-off-by: qianfan Zhao <qianfanguijin@163.com>
> ---
>  hw/arm/allwinner-r40.c         | 47 ++++++++++++++++++++++++++++++----
>  include/hw/arm/allwinner-r40.h | 11 ++++++++
>  2 files changed, 53 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/allwinner-r40.c b/hw/arm/allwinner-r40.c
> index fde01783b1..9fa23e1f33 100644
> --- a/hw/arm/allwinner-r40.c
> +++ b/hw/arm/allwinner-r40.c
> @@ -52,6 +52,11 @@ const hwaddr allwinner_r40_memmap[] = {
>      [AW_R40_DEV_UART5]      = 0x01c29400,
>      [AW_R40_DEV_UART6]      = 0x01c29800,
>      [AW_R40_DEV_UART7]      = 0x01c29c00,
> +    [AW_R40_DEV_TWI0]       = 0x01c2ac00,
> +    [AW_R40_DEV_TWI1]       = 0x01c2b000,
> +    [AW_R40_DEV_TWI2]       = 0x01c2b400,
> +    [AW_R40_DEV_TWI3]       = 0x01c2b800,
> +    [AW_R40_DEV_TWI4]       = 0x01c2c000,
>      [AW_R40_DEV_GIC_DIST]   = 0x01c81000,
>      [AW_R40_DEV_GIC_CPU]    = 0x01c82000,
>      [AW_R40_DEV_GIC_HYP]    = 0x01c84000,
> @@ -115,11 +120,6 @@ static struct AwR40Unimplemented r40_unimplemented[] = {
>      { "uart7",      0x01c29c00, 1 * KiB },
>      { "ps20",       0x01c2a000, 1 * KiB },
>      { "ps21",       0x01c2a400, 1 * KiB },
> -    { "twi0",       0x01c2ac00, 1 * KiB },
> -    { "twi1",       0x01c2b000, 1 * KiB },
> -    { "twi2",       0x01c2b400, 1 * KiB },
> -    { "twi3",       0x01c2b800, 1 * KiB },
> -    { "twi4",       0x01c2c000, 1 * KiB },
>      { "scr",        0x01c2c400, 1 * KiB },
>      { "tvd-top",    0x01c30000, 4 * KiB },
>      { "tvd0",       0x01c31000, 4 * KiB },
> @@ -167,6 +167,9 @@ enum {
>      AW_R40_GIC_SPI_UART1     =  2,
>      AW_R40_GIC_SPI_UART2     =  3,
>      AW_R40_GIC_SPI_UART3     =  4,
> +    AW_R40_GIC_SPI_TWI0      =  7,
> +    AW_R40_GIC_SPI_TWI1      =  8,
> +    AW_R40_GIC_SPI_TWI2      =  9,
>      AW_R40_GIC_SPI_UART4     = 17,
>      AW_R40_GIC_SPI_UART5     = 18,
>      AW_R40_GIC_SPI_UART6     = 19,
> @@ -177,6 +180,8 @@ enum {
>      AW_R40_GIC_SPI_MMC1      = 33,
>      AW_R40_GIC_SPI_MMC2      = 34,
>      AW_R40_GIC_SPI_MMC3      = 35,
> +    AW_R40_GIC_SPI_TWI3      = 88,
> +    AW_R40_GIC_SPI_TWI4      = 89,
>  };
>
>  /* Allwinner R40 general constants */
> @@ -262,6 +267,12 @@ static void allwinner_r40_init(Object *obj)
>      object_initialize_child(obj, "mmc1", &s->mmc1, TYPE_AW_SDHOST_SUN5I);
>      object_initialize_child(obj, "mmc2", &s->mmc2, TYPE_AW_SDHOST_SUN5I);
>      object_initialize_child(obj, "mmc3", &s->mmc3, TYPE_AW_SDHOST_SUN5I);
> +
> +    object_initialize_child(obj, "twi0", &s->i2c0, TYPE_AW_I2C_SUN6I);
> +    object_initialize_child(obj, "twi1", &s->i2c1, TYPE_AW_I2C_SUN6I);
> +    object_initialize_child(obj, "twi2", &s->i2c2, TYPE_AW_I2C_SUN6I);
> +    object_initialize_child(obj, "twi3", &s->i2c3, TYPE_AW_I2C_SUN6I);
> +    object_initialize_child(obj, "twi4", &s->i2c4, TYPE_AW_I2C_SUN6I);
>  }
>
>  static void allwinner_r40_realize(DeviceState *dev, Error **errp)
> @@ -429,6 +440,32 @@ static void allwinner_r40_realize(DeviceState *dev, Error **errp)
>                     qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_UART7),
>                     115200, serial_hd(7), DEVICE_NATIVE_ENDIAN);
>
> +    /* I2C */
> +    sysbus_realize(SYS_BUS_DEVICE(&s->i2c0), &error_fatal);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c0), 0, s->memmap[AW_R40_DEV_TWI0]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c0), 0,
> +                       qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_TWI0));
> +
> +    sysbus_realize(SYS_BUS_DEVICE(&s->i2c1), &error_fatal);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c1), 0, s->memmap[AW_R40_DEV_TWI1]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c1), 0,
> +                       qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_TWI1));
> +
> +    sysbus_realize(SYS_BUS_DEVICE(&s->i2c2), &error_fatal);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c2), 0, s->memmap[AW_R40_DEV_TWI2]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c2), 0,
> +                       qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_TWI2));
> +
> +    sysbus_realize(SYS_BUS_DEVICE(&s->i2c3), &error_fatal);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c3), 0, s->memmap[AW_R40_DEV_TWI3]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c3), 0,
> +                       qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_TWI3));
> +
> +    sysbus_realize(SYS_BUS_DEVICE(&s->i2c4), &error_fatal);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c4), 0, s->memmap[AW_R40_DEV_TWI4]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c4), 0,
> +                       qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_TWI4));
> +
>      /* Unimplemented devices */
>      for (i = 0; i < ARRAY_SIZE(r40_unimplemented); i++) {
>          create_unimplemented_device(r40_unimplemented[i].device_name,
> diff --git a/include/hw/arm/allwinner-r40.h b/include/hw/arm/allwinner-r40.h
> index dfb5eb609c..6a7e5c1e31 100644
> --- a/include/hw/arm/allwinner-r40.h
> +++ b/include/hw/arm/allwinner-r40.h
> @@ -26,6 +26,7 @@
>  #include "hw/intc/arm_gic.h"
>  #include "hw/sd/allwinner-sdhost.h"
>  #include "hw/misc/allwinner-r40-ccu.h"
> +#include "hw/i2c/allwinner-i2c.h"
>  #include "target/arm/cpu.h"
>  #include "sysemu/block-backend.h"
>
> @@ -48,6 +49,11 @@ enum {
>      AW_R40_DEV_UART5,
>      AW_R40_DEV_UART6,
>      AW_R40_DEV_UART7,
> +    AW_R40_DEV_TWI0,
> +    AW_R40_DEV_TWI1,
> +    AW_R40_DEV_TWI2,
> +    AW_R40_DEV_TWI3,
> +    AW_R40_DEV_TWI4,
>      AW_R40_DEV_GIC_DIST,
>      AW_R40_DEV_GIC_CPU,
>      AW_R40_DEV_GIC_HYP,
> @@ -89,6 +95,11 @@ struct AwR40State {
>      AwSdHostState mmc2;
>      AwSdHostState mmc3;
>      AwR40ClockCtlState ccu;
> +    AWI2CState i2c0;
> +    AWI2CState i2c1;
> +    AWI2CState i2c2;
> +    AWI2CState i2c3;
> +    AWI2CState i2c4;

I am not sure what is the benefit of having all 5 I2C controllers
emulated? Unlike UARTs, where a user can decide at runtime how many
will be used, for I2C the decision is made at compile time.
Thus, I would suggest only creating those that have peripherals that
are emulated attached to them.

Best regards,
Strahinja


>      GICState gic;
>      MemoryRegion sram_a1;
>      MemoryRegion sram_a2;
> --
> 2.25.1
>