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

[Cédric Le Goater]

On 6/7/22 01:18, Joel Stanley wrote:
> 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?

I will take care of it.

Thanks,

C.


> > +        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