> >>
> > Hi Cedrice,
> >
> > Thanks for review and sorry reply you late.
> >
> >> On 3/4/24 10:29, Jamin Lin wrote:
> >>> The SDRAM memory controller(DRAMC) controls the access to external
> >>> DDR4 and DDR5 SDRAM and power up to DDR4 and DDR5 PHY.
> >>>
> >>> The DRAM memory controller of AST2700 is not backward compatible to
> >>> previous chips such AST2600, AST2500 and AST2400.
> >>>
> >>> Max memory is now 8GiB on the AST2700. Introduce new
> >> aspeed_2700_sdmc
> >>> and class with read/write operation and reset handlers.
> >>>
> >>> Define DRAMC necessary protected registers and unprotected registers
> >>> for AST2700 and increase the register set to 0x1000.
> >>>
> >>> Signed-off-by: Troy Lee <troy_...@aspeedtech.com>
> >>> Signed-off-by: Jamin Lin <jamin_...@aspeedtech.com>
> >>> ---
> >>> hw/misc/aspeed_sdmc.c | 215
> >> ++++++++++++++++++++++++++++++----
> >>> include/hw/misc/aspeed_sdmc.h | 4 +-
> >>> 2 files changed, 198 insertions(+), 21 deletions(-)
> >>>
> >>> diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c index
> >>> 64cd1a81dc..63fb7936c4 100644
> >>> --- a/hw/misc/aspeed_sdmc.c
> >>> +++ b/hw/misc/aspeed_sdmc.c
> >>> @@ -27,6 +27,7 @@
> >>> #define PROT_SOFTLOCKED 0x00
> >>>
> >>> #define PROT_KEY_UNLOCK 0xFC600309
> >>> +#define PROT_2700_KEY_UNLOCK 0x1688A8A8
> >>> #define PROT_KEY_HARDLOCK 0xDEADDEAD /* AST2600 */
> >>>
> >>> /* Configuration Register */
> >>> @@ -54,6 +55,46 @@
> >>> #define R_DRAM_TIME (0x8c / 4)
> >>> #define R_ECC_ERR_INJECT (0xb4 / 4)
> >>>
> >>> +/* AST2700 Register */
> >>> +#define R_2700_PROT (0x00 / 4)
> >>> +#define R_INT_STATUS (0x04 / 4)
> >>> +#define R_INT_CLEAR (0x08 / 4)
> >>> +#define R_INT_MASK (0x0c / 4)
> >>> +#define R_MAIN_CONF (0x10 / 4)
> >>> +#define R_MAIN_CONTROL (0x14 / 4)
> >>> +#define R_MAIN_STATUS (0x18 / 4)
> >>> +#define R_ERR_STATUS (0x1c / 4)
> >>> +#define R_ECC_FAIL_STATUS (0x78 / 4)
> >>> +#define R_ECC_FAIL_ADDR (0x7c / 4)
> >>> +#define R_ECC_TESTING_CONTROL (0x80 / 4)
> >>> +#define R_PROT_REGION_LOCK_STATUS (0x94 / 4)
> >>> +#define R_TEST_FAIL_ADDR (0xd4 / 4)
> >>> +#define R_TEST_FAIL_D0 (0xd8 / 4)
> >>> +#define R_TEST_FAIL_D1 (0xdc / 4)
> >>> +#define R_TEST_FAIL_D2 (0xe0 / 4)
> >>> +#define R_TEST_FAIL_D3 (0xe4 / 4)
> >>> +#define R_DBG_STATUS (0xf4 / 4)
> >>> +#define R_PHY_INTERFACE_STATUS (0xf8 / 4)
> >>> +#define R_GRAPHIC_MEM_BASE_ADDR (0x10c / 4)
> >>> +#define R_PORT0_INTERFACE_MONITOR0 (0x240 / 4) #define
> >>> +R_PORT0_INTERFACE_MONITOR1 (0x244 / 4) #define
> >>> +R_PORT0_INTERFACE_MONITOR2 (0x248 / 4) #define
> >>> +R_PORT1_INTERFACE_MONITOR0 (0x2c0 / 4) #define
> >>> +R_PORT1_INTERFACE_MONITOR1 (0x2c4 / 4) #define
> >>> +R_PORT1_INTERFACE_MONITOR2 (0x2c8 / 4) #define
> >>> +R_PORT2_INTERFACE_MONITOR0 (0x340 / 4) #define
> >>> +R_PORT2_INTERFACE_MONITOR1 (0x344 / 4) #define
> >>> +R_PORT2_INTERFACE_MONITOR2 (0x348 / 4) #define
> >>> +R_PORT3_INTERFACE_MONITOR0 (0x3c0 / 4) #define
> >>> +R_PORT3_INTERFACE_MONITOR1 (0x3c4 / 4) #define
> >>> +R_PORT3_INTERFACE_MONITOR2 (0x3c8 / 4) #define
> >>> +R_PORT4_INTERFACE_MONITOR0 (0x440 / 4) #define
> >>> +R_PORT4_INTERFACE_MONITOR1 (0x444 / 4) #define
> >>> +R_PORT4_INTERFACE_MONITOR2 (0x448 / 4) #define
> >>> +R_PORT5_INTERFACE_MONITOR0 (0x4c0 / 4) #define
> >>> +R_PORT5_INTERFACE_MONITOR1 (0x4c4 / 4) #define
> >>> +R_PORT5_INTERFACE_MONITOR2 (0x4c8 / 4)
> >>> +
> >>> /*
> >>> * Configuration register Ox4 (for Aspeed AST2400 SOC)
> >>> *
> >>> @@ -76,10 +117,6 @@
> >>> #define ASPEED_SDMC_VGA_32MB 0x2
> >>> #define ASPEED_SDMC_VGA_64MB 0x3
> >>> #define ASPEED_SDMC_DRAM_SIZE(x) (x & 0x3)
> >>> -#define ASPEED_SDMC_DRAM_64MB 0x0
> >>> -#define ASPEED_SDMC_DRAM_128MB 0x1
> >>> -#define ASPEED_SDMC_DRAM_256MB 0x2
> >>> -#define ASPEED_SDMC_DRAM_512MB 0x3
> >>>
> >>> #define ASPEED_SDMC_READONLY_MASK
> \
> >>> (ASPEED_SDMC_RESERVED | ASPEED_SDMC_VGA_COMPAT |
> \
> >>> @@ -100,22 +137,24 @@
> >>> #define ASPEED_SDMC_CACHE_ENABLE (1 << 10) /* differs
> >> from AST2400 */
> >>> #define ASPEED_SDMC_DRAM_TYPE (1 << 4) /* differs
> >> from AST2400 */
> >>>
> >>> -/* DRAM size definitions differs */
> >>> -#define ASPEED_SDMC_AST2500_128MB 0x0
> >>> -#define ASPEED_SDMC_AST2500_256MB 0x1
> >>> -#define ASPEED_SDMC_AST2500_512MB 0x2
> >>> -#define ASPEED_SDMC_AST2500_1024MB 0x3
> >>> -
> >>> -#define ASPEED_SDMC_AST2600_256MB 0x0
> >>> -#define ASPEED_SDMC_AST2600_512MB 0x1
> >>> -#define ASPEED_SDMC_AST2600_1024MB 0x2
> >>> -#define ASPEED_SDMC_AST2600_2048MB 0x3
> >>> -
> >> Please move the removal above in a separate patch.
> > Will fix
> >>
> >>> #define ASPEED_SDMC_AST2500_READONLY_MASK
> >> \
> >>> (ASPEED_SDMC_HW_VERSION(0xf) |
> >> ASPEED_SDMC_CACHE_INITIAL_DONE | \
> >>> ASPEED_SDMC_AST2500_RESERVED |
> >> ASPEED_SDMC_VGA_COMPAT | \
> >>> ASPEED_SDMC_VGA_APERTURE(ASPEED_SDMC_VGA_64MB))
> >>>
> >>> +/*
> >>> + * Main Configuration register Ox10 (for Aspeed AST2700 SOC and
> >>> +higher)
> >>> + *
> >>> + */
> >>> +#define ASPEED_SDMC_AST2700_RESERVED 0xFFFF2082 /*
> 31:16,
> >> 13, 7, 1 */
> >>> +#define ASPEED_SDMC_AST2700_DATA_SCRAMBLE (1 << 8)
> >>> +#define ASPEED_SDMC_AST2700_ECC_ENABLE (1 << 6)
> >>> +#define ASPEED_SDMC_AST2700_PAGE_MATCHING_ENABLE (1 <<
> 5)
> >>> +#define ASPEED_SDMC_AST2700_DRAM_SIZE(x) ((x & 0x7)
> >> << 2)
> >>> +
> >>> +#define ASPEED_SDMC_AST2700_READONLY_MASK \
> >>> + (ASPEED_SDMC_AST2700_RESERVED)
> >>> +
> >>> static uint64_t aspeed_sdmc_read(void *opaque, hwaddr addr,
> >>> unsigned
> >> size)
> >>> {
> >>> AspeedSDMCState *s = ASPEED_SDMC(opaque); @@ -231,7
> +270,10
> >> @@
> >>> static void aspeed_sdmc_realize(DeviceState *dev, Error **errp)
> >>> AspeedSDMCState *s = ASPEED_SDMC(dev);
> >>> AspeedSDMCClass *asc = ASPEED_SDMC_GET_CLASS(s);
> >>>
> >>> - assert(asc->max_ram_size < 4 * GiB); /* 32-bit address bus */
> >>> + if (!asc->is_bus64bit) {
> >>> + assert(asc->max_ram_size < 4 * GiB); /* 32-bit address bus */
> >>> + }
> >>
> >> May be :
> >>
> >> assert(asc->max_ram_size < 4 * GiB || asc->is_bus64bit);
> >>
> >> ?
> > Will fix
> >>
> >>> +
> >>> s->max_ram_size = asc->max_ram_size;
> >>>
> >>> memory_region_init_io(&s->iomem, OBJECT(s),
> &aspeed_sdmc_ops,
> >> s,
> >>> @@ -311,7 +353,8 @@ static void
> >> aspeed_2400_sdmc_write(AspeedSDMCState *s, uint32_t reg,
> >>> uint32_t data)
> >>> {
> >>> if (reg == R_PROT) {
> >>> - s->regs[reg] = (data == PROT_KEY_UNLOCK) ?
> PROT_UNLOCKED :
> >> PROT_SOFTLOCKED;
> >>> + s->regs[reg] =
> >>> + (data == PROT_KEY_UNLOCK) ? PROT_UNLOCKED :
> >>> + PROT_SOFTLOCKED;
> >>> return;
> >>> }
> >>>
> >>> @@ -369,7 +412,8 @@ static void
> >> aspeed_2500_sdmc_write(AspeedSDMCState *s, uint32_t reg,
> >>> uint32_t data)
> >>> {
> >>> if (reg == R_PROT) {
> >>> - s->regs[reg] = (data == PROT_KEY_UNLOCK) ?
> PROT_UNLOCKED :
> >> PROT_SOFTLOCKED;
> >>> + s->regs[reg] =
> >>> + (data == PROT_KEY_UNLOCK) ? PROT_UNLOCKED :
> >>> + PROT_SOFTLOCKED;
> >>> return;
> >>> }
> >>>
> >>> @@ -449,8 +493,9 @@ static void
> >> aspeed_2600_sdmc_write(AspeedSDMCState *s, uint32_t reg,
> >>> }
> >>>
> >>> if (s->regs[R_PROT] == PROT_HARDLOCKED) {
> >>> - qemu_log_mask(LOG_GUEST_ERROR, "%s: SDMC is locked until
> >> system reset!\n",
> >>> - __func__);
> >>> + qemu_log_mask(LOG_GUEST_ERROR,
> >>> + "%s: SDMC is locked until system reset!\n",
> >>> + __func__);
> >>> return;
> >>> }
> >>
> >> The checkpatch changes could be in a separate patch. This is minor.
> > Will fix
> >>
> >>>
> >>> @@ -512,12 +557,142 @@ static const TypeInfo aspeed_2600_sdmc_info
> = {
> >>> .class_init = aspeed_2600_sdmc_class_init,
> >>> };
> >>>
> >>> +static void aspeed_2700_sdmc_reset(DeviceState *dev) {
> >>> + AspeedSDMCState *s = ASPEED_SDMC(dev);
> >>> + AspeedSDMCClass *asc = ASPEED_SDMC_GET_CLASS(s);
> >>> +
> >>> + memset(s->regs, 0, sizeof(s->regs));
> >>> +
> >>> + /* Set ram size bit and defaults values */
> >>> + s->regs[R_MAIN_CONF] = asc->compute_conf(s, 0);
> >>> + s->regs[R_2700_PROT] = PROT_UNLOCKED;
> >>
> >> The default reset value is unlocked ?
> >>
> > The sdmc controller is unlocked at the SPL stage for AST2700.
> > However, currently QEMU only support to emulate that AST2700 booting
> start at the u-boot stage.
> > That was why I set the default value was unlocked, so firmware(u-boot)
> > able to write the sdmc registers.
> >
> https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-v2023.10/b
> > oard/aspeed/ibex_ast2700/sdram_ast2700.c#L716
> >
> https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-v2023.10/b
> > oard/aspeed/ibex_ast2700/sdram_ast2700.c#L277
>
> Since the model is diverging from HW, a comment explaining the reason why
> would be useful.
>
> As an extension preparing SPL support, you could add a new "unlocked"
> property, set to "false" by default. The AST2700 SoC realize handler would set
> it to "true" and the reset handler would test the value to initialize the
> R_PROT
> register.
>
Hi Cedric,
Thanks for your suggestion. Will add
Jamin
> Thanks,
>
> C.
>
>
>
> >
> > Thanks-Jamin
> >
> >>
> >> Thanks,
> >>
> >> C.
> >>
> >>
> >>
> >>> +}
> >>> +
> >>> +static uint32_t aspeed_2700_sdmc_compute_conf(AspeedSDMCState *s,
> >>> +uint32_t data) {
> >>> + uint32_t fixed_conf =
> >> ASPEED_SDMC_AST2700_PAGE_MATCHING_ENABLE |
> >>> +
> >> ASPEED_SDMC_AST2700_DRAM_SIZE(aspeed_sdmc_get_ram_bits(s));
> >>> +
> >>> + /* Make sure readonly bits are kept */
> >>> + data &= ~ASPEED_SDMC_AST2700_READONLY_MASK;
> >>> +
> >>> + return data | fixed_conf;
> >>> +}
> >>> +
> >>> +static void aspeed_2700_sdmc_write(AspeedSDMCState *s, uint32_t reg,
> >>> + uint32_t data) {
> >>> + /* Unprotected registers */
> >>> + switch (reg) {
> >>> + case R_INT_STATUS:
> >>> + case R_INT_CLEAR:
> >>> + case R_INT_MASK:
> >>> + case R_MAIN_STATUS:
> >>> + case R_ERR_STATUS:
> >>> + case R_ECC_FAIL_STATUS:
> >>> + case R_ECC_FAIL_ADDR:
> >>> + case R_PROT_REGION_LOCK_STATUS:
> >>> + case R_TEST_FAIL_ADDR:
> >>> + case R_TEST_FAIL_D0:
> >>> + case R_TEST_FAIL_D1:
> >>> + case R_TEST_FAIL_D2:
> >>> + case R_TEST_FAIL_D3:
> >>> + case R_DBG_STATUS:
> >>> + case R_PHY_INTERFACE_STATUS:
> >>> + case R_GRAPHIC_MEM_BASE_ADDR:
> >>> + case R_PORT0_INTERFACE_MONITOR0:
> >>> + case R_PORT0_INTERFACE_MONITOR1:
> >>> + case R_PORT0_INTERFACE_MONITOR2:
> >>> + case R_PORT1_INTERFACE_MONITOR0:
> >>> + case R_PORT1_INTERFACE_MONITOR1:
> >>> + case R_PORT1_INTERFACE_MONITOR2:
> >>> + case R_PORT2_INTERFACE_MONITOR0:
> >>> + case R_PORT2_INTERFACE_MONITOR1:
> >>> + case R_PORT2_INTERFACE_MONITOR2:
> >>> + case R_PORT3_INTERFACE_MONITOR0:
> >>> + case R_PORT3_INTERFACE_MONITOR1:
> >>> + case R_PORT3_INTERFACE_MONITOR2:
> >>> + case R_PORT4_INTERFACE_MONITOR0:
> >>> + case R_PORT4_INTERFACE_MONITOR1:
> >>> + case R_PORT4_INTERFACE_MONITOR2:
> >>> + case R_PORT5_INTERFACE_MONITOR0:
> >>> + case R_PORT5_INTERFACE_MONITOR1:
> >>> + case R_PORT5_INTERFACE_MONITOR2:
> >>> + s->regs[reg] = data;
> >>> + return;
> >>> + }
> >>> +
> >>> + if (s->regs[R_2700_PROT] == PROT_HARDLOCKED) {
> >>> + qemu_log_mask(LOG_GUEST_ERROR,
> >>> + "%s: SDMC is locked until system reset!\n",
> >>> + __func__);
> >>> + return;
> >>> + }
> >>> +
> >>> + if (reg != R_2700_PROT && s->regs[R_2700_PROT] ==
> >> PROT_SOFTLOCKED) {
> >>> + qemu_log_mask(LOG_GUEST_ERROR,
> >>> + "%s: SDMC is locked! (write to MCR%02x
> >> blocked)\n",
> >>> + __func__, reg * 4);
> >>> + return;
> >>> + }
> >>> +
> >>> + switch (reg) {
> >>> + case R_2700_PROT:
> >>> + if (data == PROT_2700_KEY_UNLOCK) {
> >>> + data = PROT_UNLOCKED;
> >>> + } else if (data == PROT_KEY_HARDLOCK) {
> >>> + data = PROT_HARDLOCKED;
> >>> + } else {
> >>> + data = PROT_SOFTLOCKED;
> >>> + }
> >>> + break;
> >>> + case R_MAIN_CONF:
> >>> + data = aspeed_2700_sdmc_compute_conf(s, data);
> >>> + break;
> >>> + case R_MAIN_STATUS:
> >>> + /* Will never return 'busy'. */
> >>> + data &= ~PHY_BUSY_STATE;
> >>> + break;
> >>> + default:
> >>> + break;
> >>> + }
> >>> +
> >>> + s->regs[reg] = data;
> >>> +}
> >>> +
> >>> +static const uint64_t
> >>> + aspeed_2700_ram_sizes[] = { 256 * MiB, 512 * MiB, 1024 * MiB,
> >>> + 2048 * MiB, 4096 * MiB, 8192 *
> >> MiB,
> >>> +0};
> >>> +
> >>> +static void aspeed_2700_sdmc_class_init(ObjectClass *klass, void
> >>> +*data) {
> >>> + DeviceClass *dc = DEVICE_CLASS(klass);
> >>> + AspeedSDMCClass *asc = ASPEED_SDMC_CLASS(klass);
> >>> +
> >>> + dc->desc = "ASPEED 2700 SDRAM Memory Controller";
> >>> + dc->reset = aspeed_2700_sdmc_reset;
> >>> +
> >>> + asc->is_bus64bit = true;
> >>> + asc->max_ram_size = 8 * GiB;
> >>> + asc->compute_conf = aspeed_2700_sdmc_compute_conf;
> >>> + asc->write = aspeed_2700_sdmc_write;
> >>> + asc->valid_ram_sizes = aspeed_2700_ram_sizes; }
> >>> +
> >>> +static const TypeInfo aspeed_2700_sdmc_info = {
> >>> + .name = TYPE_ASPEED_2700_SDMC,
> >>> + .parent = TYPE_ASPEED_SDMC,
> >>> + .class_init = aspeed_2700_sdmc_class_init, };
> >>> +
> >>> static void aspeed_sdmc_register_types(void)
> >>> {
> >>> type_register_static(&aspeed_sdmc_info);
> >>> type_register_static(&aspeed_2400_sdmc_info);
> >>> type_register_static(&aspeed_2500_sdmc_info);
> >>> type_register_static(&aspeed_2600_sdmc_info);
> >>> + type_register_static(&aspeed_2700_sdmc_info);
> >>> }
> >>>
> >>> type_init(aspeed_sdmc_register_types);
> >>> diff --git a/include/hw/misc/aspeed_sdmc.h
> >>> b/include/hw/misc/aspeed_sdmc.h index ec2d59a14f..6df2f0a3b7 100644
> >>> --- a/include/hw/misc/aspeed_sdmc.h
> >>> +++ b/include/hw/misc/aspeed_sdmc.h
> >>> @@ -17,6 +17,7 @@ OBJECT_DECLARE_TYPE(AspeedSDMCState,
> >> AspeedSDMCClass, ASPEED_SDMC)
> >>> #define TYPE_ASPEED_2400_SDMC TYPE_ASPEED_SDMC "-ast2400"
> >>> #define TYPE_ASPEED_2500_SDMC TYPE_ASPEED_SDMC "-ast2500"
> >>> #define TYPE_ASPEED_2600_SDMC TYPE_ASPEED_SDMC "-ast2600"
> >>> +#define TYPE_ASPEED_2700_SDMC TYPE_ASPEED_SDMC "-ast2700"
> >>>
> >>> /*
> >>> * SDMC has 174 documented registers. In addition the u-boot
> >>> device tree @@ -29,7 +30,7 @@
> OBJECT_DECLARE_TYPE(AspeedSDMCState,
> >> AspeedSDMCClass, ASPEED_SDMC)
> >>> * time, and the other is in the DDR-PHY IP which is used during
> DDR-PHY
> >>> * training.
> >>> */
> >>> -#define ASPEED_SDMC_NR_REGS (0x500 >> 2)
> >>> +#define ASPEED_SDMC_NR_REGS (0x1000 >> 2)
> >>>
> >>> struct AspeedSDMCState {
> >>> /*< private >*/
> >>> @@ -51,6 +52,7 @@ struct AspeedSDMCClass {
> >>> const uint64_t *valid_ram_sizes;
> >>> uint32_t (*compute_conf)(AspeedSDMCState *s, uint32_t data);
> >>> void (*write)(AspeedSDMCState *s, uint32_t reg, uint32_t
> >>> data);
> >>> + bool is_bus64bit;
> >>> };
> >>>
> >>> #endif /* ASPEED_SDMC_H */
> >
