[U-Boot] [PATCH 10/10] ddr: altera: Stratix10: Add ECC memory scrubbing

[Marek Vasut]

On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> Scrub memory content if ECC is enabled and it is not
> from warm reset boot.
> 
> Enable icache and dcache before scrub memory
> and use "DC ZVA" instruction to clear memory
> to zeros. This instruction writes a cache line
> at a time and it can prevent false ECC error
> trigger if write cache line partially.
> 
> Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com>
> ---
>  .../arm/mach-socfpga/include/mach/sdram_s10.h |  9 +++
>  drivers/ddr/altera/sdram_s10.c                | 76 +++++++++++++++++++
>  2 files changed, 85 insertions(+)
> 
> diff --git a/arch/arm/mach-socfpga/include/mach/sdram_s10.h b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
> index 89e355010d..354f80bfce 100644
> --- a/arch/arm/mach-socfpga/include/mach/sdram_s10.h
> +++ b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
> @@ -23,6 +23,7 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
>  #define ECCCTRL1			0x100
>  #define ECCCTRL2			0x104
>  #define ERRINTEN			0x110
> +#define ERRINTENS			0x114
>  #define INTMODE				0x11c
>  #define INTSTAT				0x120
>  #define AUTOWB_CORRADDR			0x138
> @@ -53,6 +54,10 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
>  #define DDR_HMC_SEQ2CORE_INT_RESP_MASK		BIT(3)
>  #define DDR_HMC_HPSINTFCSEL_ENABLE_MASK		0x001f1f1f
>  
> +#define	DDR_HMC_ERRINTEN_INTMASK				\
> +		(DDR_HMC_ERRINTEN_SERRINTEN_EN_SET_MSK |	\
> +		 DDR_HMC_ERRINTEN_DERRINTEN_EN_SET_MSK)
> +
>  /* NOC DDR scheduler */
>  #define DDR_SCH_ID_COREID		0
>  #define DDR_SCH_ID_REVID		0x4
> @@ -181,4 +186,8 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
>  #define CALTIMING9_CFG_4_ACT_TO_ACT(x)			\
>  	(((x) >> 0) & 0xFF)
>  
> +/* Firewall DDR scheduler MPFE */
> +#define FW_HMC_ADAPTOR_REG_ADDR			0xf8020004
> +#define FW_HMC_ADAPTOR_MPU_MASK			BIT(0)
> +
>  #endif /* _SDRAM_S10_H_ */
> diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
> index ae4e5ea2fd..2c691d3bee 100644
> --- a/drivers/ddr/altera/sdram_s10.c
> +++ b/drivers/ddr/altera/sdram_s10.c
> @@ -22,6 +22,8 @@ static const struct socfpga_system_manager *sysmgr_regs =
>  
>  #define DDR_CONFIG(A, B, C, R)	(((A) << 24) | ((B) << 16) | ((C) << 8) | (R))
>  
> +#define PGTABLE_OFF	0x4000
> +
>  /* The followring are the supported configurations */
>  u32 ddr_config[] = {
>  	/* DDR_CONFIG(Address order,Bank,Column,Row) */
> @@ -135,6 +137,71 @@ static int poll_hmc_clock_status(void)
>  				 SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
>  }
>  
> +static void sdram_clear_mem(phys_addr_t addr, phys_size_t size)
> +{
> +	phys_size_t i;
> +
> +	if (addr % CONFIG_SYS_CACHELINE_SIZE) {
> +		printf("DDR: address 0x%lx not cacheline size aligned.\n",
> +		       (ulong)addr);

Is the cast needed ?

> +		hang();
> +	}
> +
> +	if (size % CONFIG_SYS_CACHELINE_SIZE) {
> +		printf("DDR: size 0x%lx not multiple of cacheline size\n",
> +		       (ulong)size);
> +		hang();
> +	}
> +
> +	/* Use DC ZVA instruction to clear memory to zeros by a cache line */
> +	for (i = 0; i < size; i = i + CONFIG_SYS_CACHELINE_SIZE) {
> +		asm("dc zva, %0"
> +		     :
> +		     : "r"(addr));

Should be asm volatile, so the compiler won't move this around.
Also, you want memory clobber here I think ?

> +		addr += CONFIG_SYS_CACHELINE_SIZE;
> +	}
> +}
> +
> +static void sdram_init_ecc_bits(phys_addr_t *bank_start, phys_size_t *bank_size)
> +{
> +	phys_size_t size, size_init;
> +	phys_addr_t start_addr;
> +	int bank;
> +	unsigned int start = get_timer(0);
> +
> +	icache_enable();
> +
> +	for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> +		start_addr = bank_start[bank];
> +		size = bank_size[bank];
> +
> +		if (bank == 0) {
> +			/* Initialize small block for page table */
> +			memset((void *)start_addr, 0,
> +			       PGTABLE_SIZE + PGTABLE_OFF);
> +			gd->arch.tlb_addr = start_addr + PGTABLE_OFF;
> +			gd->arch.tlb_size = PGTABLE_SIZE;
> +			start_addr += PGTABLE_SIZE + PGTABLE_OFF;
> +			size -= (PGTABLE_OFF + PGTABLE_SIZE);
> +			dcache_enable();

If it's only done for bank0, pull this code out of the loop ?

> +		}
> +
> +		while (size) {
> +			size_init = min((phys_addr_t)SZ_1G, (phys_addr_t)size);
> +			sdram_clear_mem(start_addr, size_init);
> +			size -= size_init;
> +			start_addr += size_init;
> +			WATCHDOG_RESET();
> +		}
> +	}
> +
> +	dcache_disable();
> +	icache_disable();
> +
> +	printf("SDRAM-ECC: Initialized success with %d ms\n",
> +	       (unsigned int)get_timer(start));
> +}
> +
>  static void sdram_size_check(phys_addr_t bank_start[], phys_size_t bank_size[])
>  {
>  	phys_size_t total_ram_check = 0;
> @@ -451,6 +518,15 @@ int sdram_mmr_init_full(unsigned int unused)
>  		setbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL2,
>  			     (DDR_HMC_ECCCTL2_RMW_EN_SET_MSK |
>  			      DDR_HMC_ECCCTL2_AWB_EN_SET_MSK));
> +		writel(DDR_HMC_ERRINTEN_INTMASK,
> +		       SOCFPGA_SDR_ADDRESS + ERRINTENS);
> +
> +		/* Enable non-secure writes to HMC Adapter for SDRAM ECC */
> +		writel(FW_HMC_ADAPTOR_MPU_MASK, FW_HMC_ADAPTOR_REG_ADDR);
> +
> +		/* Initialize memory content if not from warm reset */
> +		if (!cpu_has_been_warmreset())
> +			sdram_init_ecc_bits(bank_start, bank_size);
>  	} else {
>  		clrbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL1,
>  			     (DDR_HMC_ECCCTL_AWB_CNT_RST_SET_MSK |
> 


-- 
Best regards,
Marek Vasut