Re: [PATCH for-5.2 16/19] aspeed/sdmc: Perform memory training

[Philippe Mathieu-Daudé]

On 8/6/20 3:21 PM, Cédric Le Goater wrote:
> From: Joel Stanley <joel@jms.id.au>
> 
> This allows qemu to run the "normal" power on reset boot path through
> u-boot, where the DDR is trained.
> 
> An enhancement would be to have the SCU bit stick across qemu reboots,
> but be unset on initial boot.
> 
> Proper modelling would be to discard all writes to the phy setting regs
> at offset 0x100 - 0x400 and to model the phy status regs at offset
> 0x400.
> 
> The status regs model would only need to account for offets 0x00,
> 0x50, 0x68 and 0x7c.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> [ clg: checkpatch fixes ]
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  include/hw/misc/aspeed_sdmc.h | 13 ++++++++++++-
>  hw/misc/aspeed_scu.c          |  2 +-
>  hw/misc/aspeed_sdmc.c         | 19 +++++++++++++++++--
>  3 files changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/include/hw/misc/aspeed_sdmc.h b/include/hw/misc/aspeed_sdmc.h
> index cea1e67fe365..c6226957dd3d 100644
> --- a/include/hw/misc/aspeed_sdmc.h
> +++ b/include/hw/misc/aspeed_sdmc.h
> @@ -17,7 +17,18 @@
>  #define TYPE_ASPEED_2500_SDMC TYPE_ASPEED_SDMC "-ast2500"
>  #define TYPE_ASPEED_2600_SDMC TYPE_ASPEED_SDMC "-ast2600"
>  
> -#define ASPEED_SDMC_NR_REGS (0x174 >> 2)
> +/*
> + * SDMC has 174 documented registers. In addition the u-boot device tree
> + * describes the following regions:
> + *  - PHY status regs at offset 0x400, length 0x200
> + *  - PHY setting regs at offset 0x100, length 0x300
> + *
> + * There are two sets of MRS (Mode Registers) configuration in ast2600 memory
> + * system: one is in the SDRAM MC (memory controller) which is used in run
> + * time, and the other is in the DDR-PHY IP which is used during DDR-PHY
> + * training.
> + */
> +#define ASPEED_SDMC_NR_REGS (0x500 >> 2)
>  
>  typedef struct AspeedSDMCState {
>      /*< private >*/
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index 764222404bef..dc6dd87c22f4 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -656,7 +656,7 @@ static const uint32_t 
> ast2600_a1_resets[ASPEED_AST2600_SCU_NR_REGS] = {
>      [AST2600_SYS_RST_CTRL2]     = 0xFFFFFFFC,
>      [AST2600_CLK_STOP_CTRL]     = 0xFFFF7F8A,
>      [AST2600_CLK_STOP_CTRL2]    = 0xFFF0FFF0,
> -    [AST2600_SDRAM_HANDSHAKE]   = 0x00000040,  /* SoC completed DRAM init */
> +    [AST2600_SDRAM_HANDSHAKE]   = 0x00000000,
>      [AST2600_HPLL_PARAM]        = 0x1000405F,
>      [AST2600_CHIP_ID0]          = 0x1234ABCD,
>      [AST2600_CHIP_ID1]          = 0x88884444,
> diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
> index 855848b7d23a..ff2809a09965 100644
> --- a/hw/misc/aspeed_sdmc.c
> +++ b/hw/misc/aspeed_sdmc.c
> @@ -113,7 +113,7 @@ static uint64_t aspeed_sdmc_read(void *opaque, hwaddr 
> addr, unsigned size)
>      if (addr >= ARRAY_SIZE(s->regs)) {
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "%s: Out-of-bounds read at offset 0x%" HWADDR_PRIx 
> "\n",
> -                      __func__, addr);
> +                      __func__, addr * 4);
>          return 0;
>      }
>  
> @@ -206,6 +206,19 @@ static void aspeed_sdmc_reset(DeviceState *dev)
>  
>      /* Set ram size bit and defaults values */
>      s->regs[R_CONF] = asc->compute_conf(s, 0);
> +
> +    /*
> +     * PHY status:
> +     *  - set phy status ok (set bit 1)
> +     *  - initial PVT calibration ok (clear bit 3)
> +     *  - runtime calibration ok (clear bit 5)
> +     */
> +    s->regs[0x100] = BIT(1);

This is usually implemented with a one-shot timer, see
sd_ocr_powerup() in hw/sd/sd.c (migration is handled).

> +
> +    /* PHY eye window: set all as passing */
> +    s->regs[0x100 | (0x68 / 4)] = 0xff;
> +    s->regs[0x100 | (0x7c / 4)] = 0xff;
> +    s->regs[0x100 | (0x50 / 4)] = 0xfffffff;
>  }
>  
>  static void aspeed_sdmc_get_ram_size(Object *obj, Visitor *v, const char 
> *name,
> @@ -443,7 +456,9 @@ static void aspeed_2600_sdmc_write(AspeedSDMCState *s, 
> uint32_t reg,
>      }
>  
>      if (reg != R_PROT && s->regs[R_PROT] == PROT_SOFTLOCKED) {
> -        qemu_log_mask(LOG_GUEST_ERROR, "%s: SDMC is locked!\n", __func__);
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: SDMC is locked! (write to MCR%02x blocked)\n",
> +                      __func__, reg * 4);
>          return;
>      }
>  
>