All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Simek <monstr@monstr.eu>
To: Jorge Ramirez-Ortiz <jorge@foundries.io>
Cc: ricardo@foundries.io, mike@foundries.io, u-boot@lists.denx.de
Subject: Re: [PATCHv2] zynqmp: spl: support DRAM ECC initialization
Date: Mon, 14 Jun 2021 09:55:32 +0200	[thread overview]
Message-ID: <8e02e94b-eae8-617b-2448-5c69f3b1dfeb@monstr.eu> (raw)
In-Reply-To: <20210613185553.717-1-jorge@foundries.io>



On 6/13/21 8:55 PM, Jorge Ramirez-Ortiz wrote:
> Use the ZDMA channel 0 to initialize the DRAM banks. This avoid
> spurious ECC errors that can occur when accessing unitialized memory.
> 
> The feature is enabled by setting the option
> CONFIG_SPL_ZYNQMP_DRAM_ECC_INIT and providing the following data:
> 
>  SPL_ZYNQMP_DRAM_BANK1_BASE: start of memory to initialize
>  SPL_ZYNQMP_DRAM_BANK1_LEN : len of memory to initialize (hex)
>  SPL_ZYNQMP_DRAM_BANK2_BASE: start of memory to initialize
>  SPL_ZYNQMP_DRAM_BANK2_LEN : len of memory to initialize (hex)
> 
> Setting SPL_ZYNQMP_DRAM_BANK_LEN to 0 takes no action.
> 
> Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> ---
>  v2: extend commit message
>      _BASE suffix for DRAM_BANK address
>      DRAM_BANK2_BASE to 0x8.00.00.00.00
>      remove unnecessary includes from ecc_spl_init.c
>      add ADMA_CH0_BASEADDR to hardware.h
>      remove cache management
>      timeout on loops
>      retry once on DMA status error
>      move DMA restore to reset values to its own function(*)
>      removed extern clause from splc and added header file instead
>      fixed duplicated call to board_early_init_r()
> 
>      (*) IP reset: there is no register to toggle (checked docs and linux kernel)
>                    opted for leaving the restore function as per FSBL
> 
> 
>  arch/arm/mach-zynqmp/Kconfig                  |  35 ++++
>  arch/arm/mach-zynqmp/Makefile                 |   1 +
>  arch/arm/mach-zynqmp/ecc_spl_init.c           | 163 ++++++++++++++++++
>  .../mach-zynqmp/include/mach/ecc_spl_init.h   |  13 ++
>  arch/arm/mach-zynqmp/include/mach/hardware.h  |   2 +
>  arch/arm/mach-zynqmp/spl.c                    |   4 +
>  6 files changed, 218 insertions(+)
>  create mode 100644 arch/arm/mach-zynqmp/ecc_spl_init.c
>  create mode 100644 arch/arm/mach-zynqmp/include/mach/ecc_spl_init.h
> 
> diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
> index f1301f6661..39144d654e 100644
> --- a/arch/arm/mach-zynqmp/Kconfig
> +++ b/arch/arm/mach-zynqmp/Kconfig
> @@ -92,6 +92,41 @@ config ZYNQMP_NO_DDR
>  	  This option configures MMU with no DDR to avoid speculative
>  	  access to DDR memory where DDR is not present.
>  
> +config SPL_ZYNQMP_DRAM_ECC_INIT
> +	bool "Initialize DRAM ECC"
> +	depends on SPL
> +	help
> +	  This option initializes all memory to 0xdeadbeef. Must be set if your
> +	  memory is of ECC type.
> +
> +config SPL_ZYNQMP_DRAM_BANK1_BASE
> +	depends on SPL_ZYNQMP_DRAM_ECC_INIT
> +	hex "DRAM Bank1 address"
> +       default 0x00000000
> +       help
> +         Start address of DRAM ECC bank1
> +
> +config SPL_ZYNQMP_DRAM_BANK1_LEN
> +	depends on SPL_ZYNQMP_DRAM_ECC_INIT
> +	hex "DRAM Bank1 size"
> +       default 0x80000000
> +       help
> +         Size in bytes of the DRAM ECC bank1
> +
> +config SPL_ZYNQMP_DRAM_BANK2_BASE
> +	depends on SPL_ZYNQMP_DRAM_ECC_INIT
> +	hex "DRAM Bank2 address"
> +       default 0x800000000
> +       help
> +         Start address of DRAM ECC bank2
> +
> +config SPL_ZYNQMP_DRAM_BANK2_LEN
> +	depends on SPL_ZYNQMP_DRAM_ECC_INIT
> +	hex "DRAM Bank2 size"
> +       default 0x0
> +       help
> +         Size in bytes of the DRAM ECC bank2. A null size takes no action.
> +
>  config SYS_MALLOC_F_LEN
>  	default 0x600
>  
> diff --git a/arch/arm/mach-zynqmp/Makefile b/arch/arm/mach-zynqmp/Makefile
> index 8a3b074724..eb6c5112b3 100644
> --- a/arch/arm/mach-zynqmp/Makefile
> +++ b/arch/arm/mach-zynqmp/Makefile
> @@ -7,4 +7,5 @@ obj-y	+= clk.o
>  obj-y	+= cpu.o
>  obj-$(CONFIG_MP)	+= mp.o
>  obj-$(CONFIG_SPL_BUILD) += spl.o handoff.o
> +obj-$(CONFIG_SPL_ZYNQMP_DRAM_ECC_INIT) += ecc_spl_init.o
>  obj-$(CONFIG_ZYNQMP_PSU_INIT_ENABLED)	+= psu_spl_init.o
> diff --git a/arch/arm/mach-zynqmp/ecc_spl_init.c b/arch/arm/mach-zynqmp/ecc_spl_init.c
> new file mode 100644
> index 0000000000..f547d8e3a5
> --- /dev/null
> +++ b/arch/arm/mach-zynqmp/ecc_spl_init.c
> @@ -0,0 +1,163 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + *  Copyright(c) 2015 - 2020 Xilinx, Inc.
> + *
> + *  Jorge Ramirez-Ortiz <jorge@foundries.io>
> + */
> +
> +#include <common.h>
> +#include <cpu_func.h>
> +#include <asm/arch/hardware.h>
> +#include <asm/arch/ecc_spl_init.h>
> +#include <asm/io.h>
> +#include <linux/delay.h>
> +
> +#define ZDMA_TRANSFER_MAX_LEN		(0x3FFFFFFFU - 7U)
> +#define ZDMA_CH_STATUS			((ADMA_CH0_BASEADDR) + 0x0000011CU)
> +#define ZDMA_CH_STATUS_STATE_MASK	0x00000003U
> +#define ZDMA_CH_STATUS_STATE_DONE	0x00000000U
> +#define ZDMA_CH_STATUS_STATE_ERR	0x00000003U
> +#define ZDMA_CH_CTRL0			((ADMA_CH0_BASEADDR) + 0x00000110U)
> +#define ZDMA_CH_CTRL0_POINT_TYPE_MASK	(u32)0x00000040U
> +#define ZDMA_CH_CTRL0_POINT_TYPE_NORMAL	(u32)0x00000000U
> +#define ZDMA_CH_CTRL0_MODE_MASK		(u32)0x00000030U
> +#define ZDMA_CH_CTRL0_MODE_WR_ONLY	(u32)0x00000010U
> +#define ZDMA_CH_CTRL0_TOTAL_BYTE_COUNT	((ADMA_CH0_BASEADDR) + 0x00000188U)
> +#define ZDMA_CH_WR_ONLY_WORD0		((ADMA_CH0_BASEADDR) + 0x00000148U)
> +#define ZDMA_CH_WR_ONLY_WORD1		((ADMA_CH0_BASEADDR) + 0x0000014CU)
> +#define ZDMA_CH_WR_ONLY_WORD2		((ADMA_CH0_BASEADDR) + 0x00000150U)
> +#define ZDMA_CH_WR_ONLY_WORD3		((ADMA_CH0_BASEADDR) + 0x00000154U)
> +#define ZDMA_CH_DST_DSCR_WORD0		((ADMA_CH0_BASEADDR) + 0x00000138U)
> +#define ZDMA_CH_DST_DSCR_WORD0_LSB_MASK	0xFFFFFFFFU
> +#define ZDMA_CH_DST_DSCR_WORD1		((ADMA_CH0_BASEADDR) + 0x0000013CU)
> +#define ZDMA_CH_DST_DSCR_WORD1_MSB_MASK	0x0001FFFFU
> +#define ZDMA_CH_SRC_DSCR_WORD2		((ADMA_CH0_BASEADDR) + 0x00000130U)
> +#define ZDMA_CH_DST_DSCR_WORD2		((ADMA_CH0_BASEADDR) + 0x00000140U)
> +#define ZDMA_CH_CTRL2			((ADMA_CH0_BASEADDR) + 0x00000200U)
> +#define ZDMA_CH_CTRL2_EN_MASK		0x00000001U
> +#define ZDMA_CH_ISR			((ADMA_CH0_BASEADDR) + 0x00000100U)
> +#define ZDMA_CH_ISR_DMA_DONE_MASK	0x00000400U
> +#define ECC_INIT_VAL_WORD		0xDEADBEEFU
> +
> +#define ZDMA_IDLE_TIMEOUT_USEC		1000000
> +#define ZDMA_DONE_TIMEOUT_USEC		5000000
> +
> +static void ecc_zdma_restore(void)
> +{
> +	/* Restore reset values for the DMA registers used */
> +	writel(ZDMA_CH_CTRL0, 0x00000080U);
> +	writel(ZDMA_CH_WR_ONLY_WORD0, 0x00000000U);
> +	writel(ZDMA_CH_WR_ONLY_WORD1, 0x00000000U);
> +	writel(ZDMA_CH_WR_ONLY_WORD2, 0x00000000U);
> +	writel(ZDMA_CH_WR_ONLY_WORD3, 0x00000000U);
> +	writel(ZDMA_CH_DST_DSCR_WORD0, 0x00000000U);
> +	writel(ZDMA_CH_DST_DSCR_WORD1, 0x00000000U);
> +	writel(ZDMA_CH_SRC_DSCR_WORD2, 0x00000000U);
> +	writel(ZDMA_CH_DST_DSCR_WORD2, 0x00000000U);
> +	writel(ZDMA_CH_CTRL0_TOTAL_BYTE_COUNT, 0x00000000U);
> +}
> +
> +static void ecc_dram_bank_init(u64 addr, u64 len)
> +{
> +	bool retry = true;
> +	u32 timeout;
> +	u64 bytes;
> +	u32 size;
> +	u64 src;
> +	u32 reg;
> +
> +	if (!len)
> +		return;
> +retry:
> +	bytes = len;
> +	src = addr;
> +	ecc_zdma_restore();
> +	while (bytes > 0) {
> +		size = bytes > ZDMA_TRANSFER_MAX_LEN ?
> +			ZDMA_TRANSFER_MAX_LEN : (u32)bytes;
> +
> +		/* Wait until the DMA is in idle state */
> +		timeout = ZDMA_IDLE_TIMEOUT_USEC;
> +		do {
> +			udelay(1);
> +			reg = readl(ZDMA_CH_STATUS);
> +			reg &= ZDMA_CH_STATUS_STATE_MASK;
> +			if (!timeout--) {
> +				puts("error, ECC DMA failed to idle\n");
> +				goto done;
> +			}
> +
> +		} while ((reg != ZDMA_CH_STATUS_STATE_DONE) &&
> +			(reg != ZDMA_CH_STATUS_STATE_ERR));
> +
> +		/* Enable Simple (Write Only) Mode */
> +		reg = readl(ZDMA_CH_CTRL0);
> +		reg &= (ZDMA_CH_CTRL0_POINT_TYPE_MASK |
> +			ZDMA_CH_CTRL0_MODE_MASK);
> +		reg |= (ZDMA_CH_CTRL0_POINT_TYPE_NORMAL |
> +			ZDMA_CH_CTRL0_MODE_WR_ONLY);
> +		writel(reg, ZDMA_CH_CTRL0);
> +
> +		/* Fill in the data to be written */
> +		writel(ECC_INIT_VAL_WORD, ZDMA_CH_WR_ONLY_WORD0);
> +		writel(ECC_INIT_VAL_WORD, ZDMA_CH_WR_ONLY_WORD1);
> +		writel(ECC_INIT_VAL_WORD, ZDMA_CH_WR_ONLY_WORD2);
> +		writel(ECC_INIT_VAL_WORD, ZDMA_CH_WR_ONLY_WORD3);
> +
> +		/* Write Destination Address */
> +		writel((u32)(src & ZDMA_CH_DST_DSCR_WORD0_LSB_MASK),
> +		       ZDMA_CH_DST_DSCR_WORD0);
> +		writel((u32)((src >> 32) & ZDMA_CH_DST_DSCR_WORD1_MSB_MASK),
> +		       ZDMA_CH_DST_DSCR_WORD1);
> +
> +		/* Size to be Transferred. Recommended to set both src and dest sizes */
> +		writel(size, ZDMA_CH_SRC_DSCR_WORD2);
> +		writel(size, ZDMA_CH_DST_DSCR_WORD2);
> +
> +		/* DMA Enable */
> +		reg = readl(ZDMA_CH_CTRL2);
> +		reg |= ZDMA_CH_CTRL2_EN_MASK;
> +		writel(reg, ZDMA_CH_CTRL2);
> +
> +		/* Check the status of the transfer by polling on DMA Done */
> +		timeout = ZDMA_DONE_TIMEOUT_USEC;
> +		do {
> +			udelay(1);
> +			reg = readl(ZDMA_CH_ISR);
> +			reg &= ZDMA_CH_ISR_DMA_DONE_MASK;
> +			if (!timeout--) {
> +				puts("error, ECC DMA timeout\n");
> +				goto done;
> +			}
> +		} while (reg != ZDMA_CH_ISR_DMA_DONE_MASK);
> +
> +		/* Clear DMA status */
> +		reg = readl(ZDMA_CH_ISR);
> +		reg |= ZDMA_CH_ISR_DMA_DONE_MASK;
> +		writel(ZDMA_CH_ISR_DMA_DONE_MASK, ZDMA_CH_ISR);
> +
> +		/* Read the channel status for errors */
> +		reg = readl(ZDMA_CH_STATUS);
> +		if (reg == ZDMA_CH_STATUS_STATE_ERR) {
> +			if (retry) {
> +				retry = false;
> +				goto retry;
> +			}
> +			puts("error, ECC DMA error\n");
> +			break;
> +		}
> +
> +		bytes -= size;
> +		src += size;
> +	}
> +done:
> +	ecc_zdma_restore();
> +}
> +
> +void zynqmp_ecc_init(void)
> +{
> +	ecc_dram_bank_init(CONFIG_SPL_ZYNQMP_DRAM_BANK1_BASE,
> +			   CONFIG_SPL_ZYNQMP_DRAM_BANK1_LEN);
> +	ecc_dram_bank_init(CONFIG_SPL_ZYNQMP_DRAM_BANK2_BASE,
> +			   CONFIG_SPL_ZYNQMP_DRAM_BANK2_LEN);
> +}
> diff --git a/arch/arm/mach-zynqmp/include/mach/ecc_spl_init.h b/arch/arm/mach-zynqmp/include/mach/ecc_spl_init.h
> new file mode 100644
> index 0000000000..b4b6fcf53b
> --- /dev/null
> +++ b/arch/arm/mach-zynqmp/include/mach/ecc_spl_init.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + *  Copyright(c) 2015 - 2020 Xilinx, Inc.
> + *
> + *  Jorge Ramirez-Ortiz <jorge@foundries.io>
> + */
> +
> +#ifndef __ARCH_ZYNQMP_ECC_INIT_H
> +#define __ARCH_ZYNQMP_ECC_INIT_H
> +
> +void zynqmp_ecc_init(void);
> +
> +#endif
> diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h b/arch/arm/mach-zynqmp/include/mach/hardware.h
> index 3d3c48e247..fa612887a4 100644
> --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> @@ -19,6 +19,8 @@
>  #define ZYNQMP_CRL_APB_BOOT_PIN_CTRL_OUT_EN_SHIFT	0
>  #define ZYNQMP_CRL_APB_BOOT_PIN_CTRL_OUT_VAL_SHIFT	8
>  
> +#define ADMA_CH0_BASEADDR	0xFFA80000
> +
>  #define PS_MODE0	BIT(0)
>  #define PS_MODE1	BIT(1)
>  #define PS_MODE2	BIT(2)
> diff --git a/arch/arm/mach-zynqmp/spl.c b/arch/arm/mach-zynqmp/spl.c
> index 88386b23e5..8fcae2c6a6 100644
> --- a/arch/arm/mach-zynqmp/spl.c
> +++ b/arch/arm/mach-zynqmp/spl.c
> @@ -15,6 +15,7 @@
>  #include <asm/io.h>
>  #include <asm/spl.h>
>  #include <asm/arch/hardware.h>
> +#include <asm/arch/ecc_spl_init.h>
>  #include <asm/arch/psu_init_gpl.h>
>  #include <asm/arch/sys_proto.h>
>  
> @@ -22,6 +23,9 @@ void board_init_f(ulong dummy)
>  {
>  	board_early_init_f();
>  	board_early_init_r();
> +#ifdef CONFIG_SPL_ZYNQMP_DRAM_ECC_INIT
> +	zynqmp_ecc_init();
> +#endif
>  }
>  
>  static void ps_mode_reset(ulong mode)
> 

Applied.
M

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs




      reply	other threads:[~2021-06-14  7:55 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-13 18:55 [PATCHv2] zynqmp: spl: support DRAM ECC initialization Jorge Ramirez-Ortiz
2021-06-14  7:55 ` Michal Simek [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8e02e94b-eae8-617b-2448-5c69f3b1dfeb@monstr.eu \
    --to=monstr@monstr.eu \
    --cc=jorge@foundries.io \
    --cc=mike@foundries.io \
    --cc=ricardo@foundries.io \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.