Re: [PATCH 13/21] aspeed: Add I2C buses to AST1030 model

[Joel Stanley]

On Mon, 6 Jun 2022 at 15:09, Cédric Le Goater <clg@kaod.org> wrote:
>
> From: Troy Lee <troy_lee@aspeedtech.com>
>
> Instantiate the I2C buses in AST1030 model and create two slave device
> for ast1030-evb.
>
> Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
> [ clg : - adapted to current ast1030 upstream models
>         - fixed typo in commit log ]
> Message-Id: <20220324100439.478317-3-troy_lee@aspeedtech.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Joel Stanley <joel@jms.id.au>

one question about a comment below.

> ---
>  hw/arm/aspeed.c         | 13 +++++++++++++
>  hw/arm/aspeed_ast10x0.c | 18 ++++++++++++++++++
>  2 files changed, 31 insertions(+)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 98dc185acd9a..5c3802308e80 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -1401,6 +1401,18 @@ static void aspeed_minibmc_machine_init(MachineState 
> *machine)
>                         AST1030_INTERNAL_FLASH_SIZE);
>  }
>
> +static void ast1030_evb_i2c_init(AspeedMachineState *bmc)
> +{
> +    AspeedSoCState *soc = &bmc->soc;
> +
> +    /* U10 24C08 connects to SDA/SCL Groupt 1 by default */
> +    uint8_t *eeprom_buf = g_malloc0(32 * 1024);
> +    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 0), 0x50, 
> eeprom_buf);
> +
> +    /* U11 LM75 connects to SDA/SCL Group 2 by default */
> +    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 1), "tmp105", 
> 0x4d);
> +}
> +
>  static void aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc,
>                                                            void *data)
>  {
> @@ -1412,6 +1424,7 @@ static void 
> aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc,
>      amc->hw_strap1 = 0;
>      amc->hw_strap2 = 0;
>      mc->init = aspeed_minibmc_machine_init;
> +    amc->i2c_init = ast1030_evb_i2c_init;
>      mc->default_ram_size = 0;
>      mc->default_cpus = mc->min_cpus = mc->max_cpus = 1;
>      amc->fmc_model = "sst25vf032b";
> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> index d53454168403..a2ed275712fb 100644
> --- a/hw/arm/aspeed_ast10x0.c
> +++ b/hw/arm/aspeed_ast10x0.c
> @@ -114,6 +114,9 @@ static void aspeed_soc_ast1030_init(Object *obj)
>      object_property_add_alias(obj, "hw-strap1", OBJECT(&s->scu), 
> "hw-strap1");
>      object_property_add_alias(obj, "hw-strap2", OBJECT(&s->scu), 
> "hw-strap2");
>
> +    snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname);
> +    object_initialize_child(obj, "i2c", &s->i2c, typename);
> +
>      snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname);
>      object_initialize_child(obj, "timerctrl", &s->timerctrl, typename);
>
> @@ -188,6 +191,21 @@ static void aspeed_soc_ast1030_realize(DeviceState 
> *dev_soc, Error **errp)
>      }
>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->scu), 0, sc->memmap[ASPEED_DEV_SCU]);
>
> +    /* I2C */
> +
> +    object_property_set_link(OBJECT(&s->i2c), "dram", OBJECT(&s->sram),
> +                             &error_abort);
> +    if (!sysbus_realize(SYS_BUS_DEVICE(&s->i2c), errp)) {
> +        return;
> +    }
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, sc->memmap[ASPEED_DEV_I2C]);
> +    for (i = 0; i < ASPEED_I2C_GET_CLASS(&s->i2c)->num_busses; i++) {
> +        qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->armv7m),
> +                                        sc->irqmap[ASPEED_DEV_I2C] + i);
> +        /* The AST2600 I2C controller has one IRQ per bus. */

I know it's the same hardware, but is the "AST2600" part of the comment correct?

> +        sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c.busses[i]), 0, irq);
> +    }
> +
>      /* LPC */
>      if (!sysbus_realize(SYS_BUS_DEVICE(&s->lpc), errp)) {
>          return;
> --
> 2.35.3
>