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

Ley Foon Tan lftan.linux at gmail.com
Tue Mar 19 03:14:23 UTC 2019


On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex at 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 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 ?
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


More information about the U-Boot mailing list