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

[Jamin Lin]

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.

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