[U-Boot] [PATCH] ddr: altera: Add ECC DRAM scrubbing support for Stratix 10

[Chee, Tien Fong <tien.fong.chee@intel.com>]

On Tue, 2018-07-24 at 12:52 +0200, Marek Vasut wrote:
> On 07/24/2018 10:10 AM, Chee, Tien Fong wrote:
> > 
> > On Mon, 2018-07-23 at 11:46 +0200, Marek Vasut wrote:
> > > 
> > > On 07/23/2018 10:20 AM, tien.fong.chee at intel.com wrote:
> > > > 
> > > > 
> > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > 
> > > > The SDRAM must first be rewritten by zeroes if ECC is used to
> > > > initialize
> > > > the ECC metadata. Make the CPU overwrite the DRAM with zeroes
> > > > in
> > > > such a
> > > > case. This scrubbing implementation turns the caches on
> > > > temporarily, then
> > > > overwrites the whole RAM with zeroes, flushes the caches and
> > > > turns
> > > > them
> > > > off again. This provides satisfactory performance.
> > > > 
> > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > ---
> > > >  drivers/ddr/altera/sdram_s10.c |   44
> > > > ++++++++++++++++++++++++++++++++++++++++
> > > >  1 files changed, 44 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/drivers/ddr/altera/sdram_s10.c
> > > > b/drivers/ddr/altera/sdram_s10.c
> > > > index 48f4f47..cce261f 100644
> > > > --- a/drivers/ddr/altera/sdram_s10.c
> > > > +++ b/drivers/ddr/altera/sdram_s10.c
> > > > @@ -8,6 +8,7 @@
> > > >  #include <errno.h>
> > > >  #include <div64.h>
> > > >  #include <asm/io.h>
> > > > +#include <linux/sizes.h>
> > > >  #include <wait_bit.h>
> > > >  #include <asm/arch/firewall_s10.h>
> > > >  #include <asm/arch/sdram_s10.h>
> > > > @@ -134,6 +135,47 @@ static int poll_hmc_clock_status(void)
> > > >  				 SYSMGR_HMC_CLK_STATUS_MSK,
> > > > true,
> > > > 1000, false);
> > > >  }
> > > >  
> > > > +/* Initialize SDRAM ECC bits to avoid false DBE */
> > > > +static void sdram_init_ecc_bits(unsigned long long size)
> > > > +{
> > > > +	/* 1GB per chunk */
> > > > +	unsigned long long size_byte = SZ_1G;
> > > > +	unsigned long long remaining_size;
> > > > +	unsigned long long dst_addr = 0x8000;
> > > > +	unsigned int start = get_timer(0);
> > > > +
> > > > +	icache_enable();
> > > > +
> > > > +	memset(0, 0, dst_addr);
> > > > +	gd->arch.tlb_addr = 0x4000;
> > > > +	gd->arch.tlb_size = PGTABLE_SIZE;
> > > Are you sure this is valid on arm64 ? It looks like something
> > > copies
> > > from arria10.
> > The cache on/off is copied from your implementation on Arria 10.
> > Yes, i
> > have tested it, it is working on Stratix 10 board.
> Right, except S10 is arm64, A10 is arm32, which is why I wonder
> whether
> careless copying is enough.
The PGTABLE_SIZE is getting from the function get_page_table_size()
which is specific for armv8 architecture. I'm also testing it on board
to ensure sufficient RAM for the page table.
> 
> > 
> > > 
> > > > 
> > > > 
> > > > +	dcache_enable();
> > > > +
> > > > +	remaining_size = size - dst_addr;
> > > > +	printf("DDRCAL: Scrubbing ECC RAM (%d MiB).\n",
> > > > (u32)(size
> > > > > 
> > > > > > 
> > > > > > 20));
> > > > +
> > > > +	while (remaining_size) {
> > > > +		if (remaining_size <= size_byte) {
> > > > +			memset((void *)dst_addr, 0,
> > > > remaining_size);
> > > > +			break;
> > > > +		} else {
> > > > +			memset((void *)dst_addr, 0,
> > > > size_byte);
> > > > +			dst_addr += size_byte;
> > > > +		}
> > > > +
> > > > +		WATCHDOG_RESET();
> > > > +		remaining_size -= size_byte;
> > > > +	}
> > > How long does this take ?
> > 1359ms for 2GB.
> So why do you need this watchdog reset hack ?
Yeah, for size scalable. When size growing up, it would take longer to
scrub. Without watchdog reset on time, it might be causing system to
reset.
> 
> > 
> > But I have no idea why Arria 10 board can't achieve the
> > same result. Could you try again on your Arria 10 ES board?
> There's nothing to try, the scrubbing works fine on A10.
Then, that really weird it took so long to finish scrubbing.
> 
> > 
> > > 
> > > > 
> > > > 
> > > > +	flush_dcache_all();
> > > > +	printf("DDRCAL: Scrubbing ECC RAM done.\n");
> > > > +	dcache_disable();
> > > > +
> > > > +	printf("SDRAM-ECC: Initialized success with %d ms\n",
> > > > +	(unsigned)get_timer(start));
> > > > +}
> > > > +
> > > >  /**
> > > >   * sdram_mmr_init_full() - Function to initialize SDRAM MMR
> > > >   *
> > > > @@ -351,6 +393,8 @@ 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))
> > > > ;
> > > > +
> > > > +		sdram_init_ecc_bits(gd->ram_size);
> > > >  	} else {
> > > >  		clrbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL1,
> > > >  			     (DDR_HMC_ECCCTL_AWB_CNT_RST_SET_M
> > > > SK |
> > > > 
>