Hi Cédric

Thanks for the review and suggestion.

> Subject: Re: [PATCH v1 07/10] hw/i2c/aspeed_i2c: Introduce AST1040 I2C
> model
> 
> On 5/29/26 08:42, Jamin Lin wrote:
> > Introduce the AST1040 I2C controller model.
> >
> > The AST1040 I2C controller is compatible with the AST2700 I2C
> > controller, including DMA support and the 64-bit DMA address
> > registers. Set has_dma64 so firmware can access the high address
> > register and program it to zero, as the
> > CM4 CPU only supports 32-bit addressing.
> >
> > Since AST1040 has 16MB HyperRAM, limit the DMA low address mask to
> > 0x00ffffff.
> >
> > Signed-off-by: Jamin Lin <[email protected]>
> > ---
> >   include/hw/i2c/aspeed_i2c.h |  1 +
> >   hw/i2c/aspeed_i2c.c         | 29 +++++++++++++++++++++++++++++
> >   2 files changed, 30 insertions(+)
> >
> > diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
> > index b2e4d2fb9d..156998e7c1 100644
> > --- a/include/hw/i2c/aspeed_i2c.h
> > +++ b/include/hw/i2c/aspeed_i2c.h
> > @@ -30,6 +30,7 @@
> >   #define TYPE_ASPEED_2500_I2C TYPE_ASPEED_I2C "-ast2500"
> >   #define TYPE_ASPEED_2600_I2C TYPE_ASPEED_I2C "-ast2600"
> >   #define TYPE_ASPEED_1030_I2C TYPE_ASPEED_I2C "-ast1030"
> > +#define TYPE_ASPEED_1040_I2C TYPE_ASPEED_I2C "-ast1040"
> >   #define TYPE_ASPEED_2700_I2C TYPE_ASPEED_I2C "-ast2700"
> >   OBJECT_DECLARE_TYPE(AspeedI2CState, AspeedI2CClass, ASPEED_I2C)
> >
> > diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index
> > 5be6fefc4d..8418a0a686 100644
> > --- a/hw/i2c/aspeed_i2c.c
> > +++ b/hw/i2c/aspeed_i2c.c
> > @@ -1702,6 +1702,34 @@ static const TypeInfo aspeed_1030_i2c_info = {
> >       .class_init = aspeed_1030_i2c_class_init,
> >   };
> >
> > +static void aspeed_1040_i2c_class_init(ObjectClass *klass, const void
> > +*data) {
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +    AspeedI2CClass *aic = ASPEED_I2C_CLASS(klass);
> > +
> > +    dc->desc = "ASPEED 1040 I2C Controller";
> > +
> > +    aic->num_busses = 16;
> > +    aic->reg_size = 0xa0;
> > +    aic->reg_gap_size = 0x60;
> > +    aic->gap = -1; /* no gap */
> > +    aic->bus_get_irq = aspeed_2600_i2c_bus_get_irq;
> > +    aic->pool_size = 0x40;
> > +    aic->pool_gap_size = 0xc0;
> > +    aic->pool_base = 0x1c0;
> > +    aic->bus_pool_base = aspeed_2500_i2c_bus_pool_base;
> > +    aic->has_dma = true;
> > +    aic->mem_size = 0x2000;
> > +    aic->has_dma64 = true;
> > +    aic->dma_addr_lo_mask = 0x00ffffff; }
> 
> Above constants are similar to TYPE_ASPEED_2700_I2C. It's worth a comment.
> 
Will add.
> > +
> > +static const TypeInfo aspeed_1040_i2c_info = {
> > +    .name = TYPE_ASPEED_1040_I2C,
> > +    .parent = TYPE_ASPEED_I2C,
> > +    .class_init = aspeed_1040_i2c_class_init, };
> > +
> >   static void aspeed_2700_i2c_class_init(ObjectClass *klass, const void
> *data)
> >   {
> >       DeviceClass *dc = DEVICE_CLASS(klass); @@ -1739,6 +1767,7 @@
> > static void aspeed_i2c_register_types(void)
> >       type_register_static(&aspeed_2500_i2c_info);
> >       type_register_static(&aspeed_2600_i2c_info);
> >       type_register_static(&aspeed_1030_i2c_info);
> > +    type_register_static(&aspeed_1040_i2c_info);
> >       type_register_static(&aspeed_2700_i2c_info);
> >   }
> >
> 
> the type info will need an update bc of
> 
> 
> https://lore.kernel.org/qemu-devel/20260601024959.2347639-1-jamin_lin@as
> peedtech.com/
> 
Will do.

Thanks,
Jamin

> Anyhow,
> 
> Reviewed-by: Cédric Le Goater <[email protected]>
> 
> Thanks,
> 
> C.

Reply via email to