Re: [PATCH v1 03/15] hw/i2c/aspeed: support to set the different memory size

[Cédric Le Goater]

On 7/19/24 03:11, Jamin Lin wrote:
> Hi Cedric,
>
> > Subject: Re: [PATCH v1 03/15] hw/i2c/aspeed: support to set the different
> > memory size
> >
> > On 7/18/24 11:42, Jamin Lin wrote:
> > > Hi Cedric,
> > >
> > > > Subject: Re: [PATCH v1 03/15] hw/i2c/aspeed: support to set the
> > > > different memory size
> > > >
> > > > On 7/18/24 08:49, Jamin Lin wrote:
> > > > > According to the datasheet of ASPEED SOCs, an I2C controller owns
> > > > > 8KB of register space for AST2700, owns 4KB of register space for
> > > > > AST2600,
> > > > > AST2500 and AST2400, and owns 64KB of register space for AST1030.
> > > > >
> > > > > It set the memory region size 4KB by default and it does not
> > > > > compatible register space for AST2700.
> > > > >
> > > > > Introduce a new class attribute to set the I2C controller memory
> > > > > size for different ASPEED SOCs.
> > > > >
> > > > > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > > > > ---
> > > > >     hw/i2c/aspeed_i2c.c         | 6 +++++-
> > > > >     include/hw/i2c/aspeed_i2c.h | 2 +-
> > > > >     2 files changed, 6 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index
> > > > > b43afd250d..7d5a53c4c0 100644
> > > > > --- a/hw/i2c/aspeed_i2c.c
> > > > > +++ b/hw/i2c/aspeed_i2c.c
> > > > > @@ -1014,7 +1014,7 @@ static void aspeed_i2c_realize(DeviceState
> > > > > *dev, Error **errp)
> > > > >
> > > > >         sysbus_init_irq(sbd, &s->irq);
> > > > >         memory_region_init_io(&s->iomem, OBJECT(s),
> > > > &aspeed_i2c_ctrl_ops, s,
> > > > > -                          "aspeed.i2c", 0x1000);
> > > > > +                          "aspeed.i2c", aic->mem_size);
> > > > >         sysbus_init_mmio(sbd, &s->iomem);
> > > > >
> > > > >         for (i = 0; i < aic->num_busses; i++) { @@ -1286,6 +1286,7 @@
> > > > > static void aspeed_2400_i2c_class_init(ObjectClass *klass, void *data)
> > > > >         aic->pool_size = 0x800;
> > > > >         aic->pool_base = 0x800;
> > > > >         aic->bus_pool_base = aspeed_2400_i2c_bus_pool_base;
> > > > > +    aic->mem_size = 0x1000;
> > > > >     }
> > > > >
> > > > >     static const TypeInfo aspeed_2400_i2c_info = { @@ -1320,6 +1321,7
> > > > > @@ static void aspeed_2500_i2c_class_init(ObjectClass *klass, void
> > *data)
> > > > >         aic->bus_pool_base = aspeed_2500_i2c_bus_pool_base;
> > > > >         aic->check_sram = true;
> > > > >         aic->has_dma = true;
> > > > > +    aic->mem_size = 0x1000;
> > > > >     }
> > > > >
> > > > >     static const TypeInfo aspeed_2500_i2c_info = { @@ -1353,6 +1355,7
> > > > > @@ static void aspeed_2600_i2c_class_init(ObjectClass *klass, void
> > *data)
> > > > >         aic->pool_base = 0xC00;
> > > > >         aic->bus_pool_base = aspeed_2600_i2c_bus_pool_base;
> > > > >         aic->has_dma = true;
> > > > > +    aic->mem_size = 0x1000;
> > > > >     }
> > > > >
> > > > >     static const TypeInfo aspeed_2600_i2c_info = { @@ -1376,6 +1379,7
> > > > > @@ static void aspeed_1030_i2c_class_init(ObjectClass *klass, void
> > *data)
> > > > >         aic->pool_base = 0xC00;
> > > > >         aic->bus_pool_base = aspeed_2600_i2c_bus_pool_base;
> > > > >         aic->has_dma = true;
> > > > > +    aic->mem_size = 0x10000;
> > > Thanks for review.
> > >
> > > According to the datasheet of AST1030 in chapter 7 (Memory Space
> > > Allocation Table) , the register address space of I2C Controller range
> > > is start from 7E7B0000 to 7E7BFFFF and its register address space is
> > 64KB(0x10000).
> >
> > OK.
> >
> > > The firmware only use 4KB address space. We can change mem_size either
> > 4KB or 64KB.
> > > Could you tell me which size you prefer?
> >
> > I would keep the larger value for the model and let FW decide to resize or not.
> Thanks for suggestion.
> Got it.

and so,


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> > Thanks,
> >
> > C.
>
> ************* Email Confidentiality Notice ********************
> 免責聲明:
> 本信件(或其附件)可能包含機密資訊，並受法律保護。如 台端非指定之收件者，請以電子郵件通知本電子郵件之發送者, 
> 並請立即刪除本電子郵件及其附件和銷毀所有複印件。謝謝您的合作!
>
> DISCLAIMER:
> This message (and any attachments) may contain legally privileged and/or other 
> confidential information. If you have received it in error, please notify the 
> sender by reply e-mail and immediately delete the e-mail and any attachments 
> without copying or disclosing the contents. Thank you.