diff mbox series

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

Message ID 1552379474-12867-11-git-send-email-ley.foon.tan@intel.com
State Superseded
Delegated to: Marek Vasut
Headers show
Series Update Stratix 10 SDRAM driver | expand

Commit Message

Ley Foon Tan March 12, 2019, 8:31 a.m. UTC
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@intel.com>
---
 .../arm/mach-socfpga/include/mach/sdram_s10.h |  9 +++
 drivers/ddr/altera/sdram_s10.c                | 76 +++++++++++++++++++
 2 files changed, 85 insertions(+)

Comments

Marek Vasut March 12, 2019, 10:49 a.m. UTC | #1
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@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 |
>
Ley Foon Tan March 19, 2019, 3:14 a.m. UTC | #2
On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex@denx.de> wrote:
>
> 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@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 ?
Yes, SPL doesn't support %llx, we need cast to ulong %lx.
>
> > +             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.
Okay.
> Also, you want memory clobber here I think ?
Yes, will add "memory" clobber here.
>
> > +             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 ?
Okay.
>
> > +             }
> > +
> > +             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
Marek Vasut March 19, 2019, 8:57 a.m. UTC | #3
On 3/19/19 4:14 AM, Ley Foon Tan wrote:
> On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex@denx.de> wrote:
>>
>> 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@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 ?
> Yes, SPL doesn't support %llx, we need cast to ulong %lx.

But that doesn't work for 64bit addresses ?
Isn't that limitation of tiny printf implementation instead of SPL ?

[...]
Ley Foon Tan March 19, 2019, 9:49 a.m. UTC | #4
On Tue, Mar 19, 2019 at 5:39 PM Marek Vasut <marex@denx.de> wrote:
>
> On 3/19/19 4:14 AM, Ley Foon Tan wrote:
> > On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> 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@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 ?
> > Yes, SPL doesn't support %llx, we need cast to ulong %lx.
>
> But that doesn't work for 64bit addresses ?
> Isn't that limitation of tiny printf implementation instead of SPL ?
>
%lx still work for 64 bit address.

Yes, I think it is limitation of  tiny printf in SPL.

Regards
Ley Foon
Marek Vasut March 19, 2019, 12:20 p.m. UTC | #5
On 3/19/19 10:49 AM, Ley Foon Tan wrote:
> On Tue, Mar 19, 2019 at 5:39 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 3/19/19 4:14 AM, Ley Foon Tan wrote:
>>> On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> 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@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 ?
>>> Yes, SPL doesn't support %llx, we need cast to ulong %lx.
>>
>> But that doesn't work for 64bit addresses ?
>> Isn't that limitation of tiny printf implementation instead of SPL ?
>>
> %lx still work for 64 bit address.
> 
> Yes, I think it is limitation of  tiny printf in SPL.

Can the tiny printf be fixed ? I think it should be easy.
diff mbox series

Patch

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);
+		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));
+		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();
+		}
+
+		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 |