[PATCH v4 0/8] Inline ECC Series
Hello,
This series is to:
Add support for Inline ECC in DDR for AM64X, AM62X, AM62AX, AM62PX, J721S2 and J784S4 devices.
(1/8) Enable ECC priming with BIST engine (2/8) Add a function to store base address and size of RAM's banks in a 64-bit device private data (3/8) Setup the ECC region start and range (4/8) Enable ECC 1-bit error, 2-bit error and multiple-bit error interrupts (5/8) Add CONFIG_K3_INLINE_ECC (6/8) Set NR_DRAM_BANKS to 2 (7/8) Pull Redundant DDR functions to a common location and Fixup DDR size when ECC is enabled (8/8) Add ss_cfg reg entry
Changes since v3: Pull redundant DDR functions to arch/arm/mach-k3/*
v3: https://lore.kernel.org/all/20240523050430.455201-1-s-k6@ti.com/
Changes since v2: Removed 'default n' in Kconfig as that is default setting
v2: https://lore.kernel.org/u-boot/20240510084707.1903133-1-s-k6@ti.com/
Changes since v1: Add support for J7* devices.
v1: https://lore.kernel.org/u-boot/20240131060213.1128024-1-s-k6@ti.com/
Test Results: https://gist.github.com/santhosh21/25937355aa2c134a64fa76480ac6a4a2
Thanks and Regards, Santhosh.
Georgi Vlaev (1): ram: k3-ddrss: Use the DDR controller BIST engine for ECC priming
Neha Malcom Francis (2): drivers: ram: Kconfig: Add CONFIG_K3_INLINE_ECC configs: j7*_evm_r5_defconfig: Set NR_DRAM_BANKS to 2
Santhosh Kumar K (5): ram: k3-ddrss: Add k3_ddrss_ddr_bank_base_size_calc() to solve 'calculations restricted to 32 bits' issue ram: k3-ddrss: Setup ECC region start and range ram: k3-ddrss: Enable ECC interrupts board: ti: Pull redundant DDR functions to a common location and Fixup DDR size when ECC is enabled arm: dts: k3-*-ddr: Add ss_cfg reg entry
arch/arm/dts/k3-am62a-ddr.dtsi | 7 +- arch/arm/dts/k3-j721s2-ddr.dtsi | 12 +- arch/arm/dts/k3-j784s4-ddr.dtsi | 24 ++- arch/arm/mach-k3/Makefile | 2 +- arch/arm/mach-k3/include/mach/k3-ddr.h | 15 ++ arch/arm/mach-k3/k3-ddr.c | 88 ++++++++++ board/ti/am62ax/evm.c | 17 +- board/ti/am62px/evm.c | 17 +- board/ti/am62x/evm.c | 62 +------ board/ti/am64x/evm.c | 73 +-------- board/ti/am65x/evm.c | 29 +--- board/ti/j721e/evm.c | 29 +--- board/ti/j721s2/evm.c | 35 ++-- board/ti/j784s4/evm.c | 17 +- configs/j7200_evm_r5_defconfig | 1 + configs/j721e_evm_r5_defconfig | 1 + configs/j721s2_evm_r5_defconfig | 1 + configs/j784s4_evm_r5_defconfig | 1 + drivers/ram/Kconfig | 10 ++ drivers/ram/k3-ddrss/k3-ddrss.c | 214 +++++++++++++++++++++---- 20 files changed, 387 insertions(+), 268 deletions(-) create mode 100644 arch/arm/mach-k3/include/mach/k3-ddr.h create mode 100644 arch/arm/mach-k3/k3-ddr.c
From: Georgi Vlaev g-vlaev@ti.com
The 1-bit inline ECC support in TI's DDRSS bridge requires the configured memory regions to be preloaded with a pattern before use. This is done by the k3-ddrss driver from the R5 SPL in a 'for' loop. It takes around 10 seconds to fill 2GB of memory, for example. Memset can cut the time in half and using DMA currently yields a similar result.
The BIST engine of DDR controller provides support for initializing any memory region with a pattern. This bypasses the DDRSS bridge, so the required inline ECC data is not computed and populated in the memory. For some values like zero, the computed ECC syndrome is also zero and we can use these values to preload the memory from the DDR controller, without the assistance of the bridge.
The registers involved in the process are described in the 'DDR controller registers' topic in [1] AM62 and [2] J721E reference manuals.
The patch replaces the 'for' loop memory fill function with the BIST memory initialization procedure. This cuts the time to preload the 2GB memory from 10 seconds down to 1 second. The bist preload function uses the lpddr4 APIs in the k3-ddrss, so this is compatible with devices with both 16-bit LPDDR4 and 32-bit LPDDR4 interfaces (e.g J721E).
[1] AM62x: https://www.ti.com/lit/pdf/spruiv7 [2] DRA829/TDA4VM: https://www.ti.com/lit/zip/spruil1
Signed-off-by: Georgi Vlaev g-vlaev@ti.com Signed-off-by: Santhosh Kumar K s-k6@ti.com --- drivers/ram/k3-ddrss/k3-ddrss.c | 122 +++++++++++++++++++++++++++++--- 1 file changed, 114 insertions(+), 8 deletions(-)
diff --git a/drivers/ram/k3-ddrss/k3-ddrss.c b/drivers/ram/k3-ddrss/k3-ddrss.c index 525b6d5b79fc..f2ab8cf2b49b 100644 --- a/drivers/ram/k3-ddrss/k3-ddrss.c +++ b/drivers/ram/k3-ddrss/k3-ddrss.c @@ -6,6 +6,7 @@ */
#include <config.h> +#include <time.h> #include <clk.h> #include <div64.h> #include <dm.h> @@ -553,14 +554,118 @@ static void k3_ddrss_set_ecc_range_r0(u32 base, u32 start_address, u32 size) writel((start_address + size - 1) >> 16, base + DDRSS_ECC_R0_END_ADDR_REG); }
-static void k3_ddrss_preload_ecc_mem_region(u32 *addr, u32 size, u32 word) +#define BIST_MODE_MEM_INIT 4 +#define BIST_MEM_INIT_TIMEOUT 10000 /* 1msec loops per block = 10s */ +static void k3_lpddr4_bist_init_mem_region(struct k3_ddrss_desc *ddrss, + u64 addr, u64 size, + u32 pattern) { - int i; + lpddr4_obj *driverdt = ddrss->driverdt; + lpddr4_privatedata *pd = &ddrss->pd; + u32 status, offset, regval; + bool int_status; + int i = 0; + + /* Set BIST_START_ADDR_0 [31:0] */ + regval = (u32)(addr & TH_FLD_MASK(LPDDR4__BIST_START_ADDRESS_0__FLD)); + TH_OFFSET_FROM_REG(LPDDR4__BIST_START_ADDRESS_0__REG, CTL_SHIFT, offset); + driverdt->writereg(pd, LPDDR4_CTL_REGS, offset, regval); + + /* Set BIST_START_ADDR_1 [32 or 34:32] */ + regval = (u32)(addr >> TH_FLD_WIDTH(LPDDR4__BIST_START_ADDRESS_0__FLD)); + regval &= TH_FLD_MASK(LPDDR4__BIST_START_ADDRESS_1__FLD); + TH_OFFSET_FROM_REG(LPDDR4__BIST_START_ADDRESS_1__REG, CTL_SHIFT, offset); + driverdt->writereg(pd, LPDDR4_CTL_REGS, offset, regval); + + /* Set ADDR_SPACE = log2(size) */ + regval = (u32)(ilog2(size) << TH_FLD_SHIFT(LPDDR4__ADDR_SPACE__FLD)); + TH_OFFSET_FROM_REG(LPDDR4__ADDR_SPACE__REG, CTL_SHIFT, offset); + driverdt->writereg(pd, LPDDR4_CTL_REGS, offset, regval); + + /* Enable the BIST data check. On 32bit lpddr4 (e.g J7) this shares a + * register with ADDR_SPACE and BIST_GO. + */ + TH_OFFSET_FROM_REG(LPDDR4__BIST_DATA_CHECK__REG, CTL_SHIFT, offset); + driverdt->readreg(pd, LPDDR4_CTL_REGS, offset, ®val); + regval |= TH_FLD_MASK(LPDDR4__BIST_DATA_CHECK__FLD); + driverdt->writereg(pd, LPDDR4_CTL_REGS, offset, regval); + /* Clear the address check bit */ + TH_OFFSET_FROM_REG(LPDDR4__BIST_ADDR_CHECK__REG, CTL_SHIFT, offset); + driverdt->readreg(pd, LPDDR4_CTL_REGS, offset, ®val); + regval &= ~TH_FLD_MASK(LPDDR4__BIST_ADDR_CHECK__FLD); + driverdt->writereg(pd, LPDDR4_CTL_REGS, offset, regval); + + /* Set BIST_TEST_MODE[2:0] to memory initialize (4) */ + regval = BIST_MODE_MEM_INIT; + TH_OFFSET_FROM_REG(LPDDR4__BIST_TEST_MODE__REG, CTL_SHIFT, offset); + driverdt->writereg(pd, LPDDR4_CTL_REGS, offset, regval); + + /* Set BIST_DATA_PATTERN[31:0] */ + TH_OFFSET_FROM_REG(LPDDR4__BIST_DATA_PATTERN_0__REG, CTL_SHIFT, offset); + driverdt->writereg(pd, LPDDR4_CTL_REGS, offset, pattern); + + /* Set BIST_DATA_PATTERN[63:32] */ + TH_OFFSET_FROM_REG(LPDDR4__BIST_DATA_PATTERN_1__REG, CTL_SHIFT, offset); + driverdt->writereg(pd, LPDDR4_CTL_REGS, offset, pattern); + + udelay(1000); + + /* Enable the programmed BIST operation - BIST_GO = 1 */ + TH_OFFSET_FROM_REG(LPDDR4__BIST_GO__REG, CTL_SHIFT, offset); + driverdt->readreg(pd, LPDDR4_CTL_REGS, offset, ®val); + regval |= TH_FLD_MASK(LPDDR4__BIST_GO__FLD); + driverdt->writereg(pd, LPDDR4_CTL_REGS, offset, regval); + + /* Wait for the BIST_DONE interrupt */ + while (i < BIST_MEM_INIT_TIMEOUT) { + status = driverdt->checkctlinterrupt(pd, LPDDR4_INTR_BIST_DONE, + &int_status); + if (!status & int_status) { + /* Clear LPDDR4_INTR_BIST_DONE */ + driverdt->ackctlinterrupt(pd, LPDDR4_INTR_BIST_DONE); + break; + } + udelay(1000); + i++; + }
- printf("ECC is enabled, priming DDR which will take several seconds.\n"); + /* Before continuing we have to stop BIST - BIST_GO = 0 */ + TH_OFFSET_FROM_REG(LPDDR4__BIST_GO__REG, CTL_SHIFT, offset); + driverdt->writereg(pd, LPDDR4_CTL_REGS, offset, 0); + + /* Timeout hit while priming the memory. We can't continue, + * since the memory is not fully initialized and we most + * likely get an uncorrectable error exception while booting. + */ + if (i == BIST_MEM_INIT_TIMEOUT) { + printf("ERROR: Timeout while priming the memory.\n"); + hang(); + } +}
- for (i = 0; i < (size / 4); i++) - addr[i] = word; +static void k3_ddrss_lpddr4_preload_full_mem(struct k3_ddrss_desc *ddrss, + u64 total_size, u32 pattern) +{ + u32 done, max_size2; + + /* Get the max size (log2) supported in this config (16/32 lpddr4) + * from the start_addess width - 16bit: 8G, 32bit: 32G + */ + max_size2 = TH_FLD_WIDTH(LPDDR4__BIST_START_ADDRESS_0__FLD) + + TH_FLD_WIDTH(LPDDR4__BIST_START_ADDRESS_1__FLD) + 1; + + /* ECC is enabled in dt but we can't preload the memory if + * the memory configuration is recognized and supported. + */ + if (!total_size || total_size > (1ull << max_size2) || + total_size & (total_size - 1)) { + printf("ECC: the memory configuration is not supported\n"); + hang(); + } + printf("ECC is enabled, priming DDR which will take several seconds.\n"); + done = get_timer(0); + k3_lpddr4_bist_init_mem_region(ddrss, 0, total_size, pattern); + printf("ECC: priming DDR completed in %lu msec\n", get_timer(done)); }
static void k3_ddrss_lpddr4_ecc_calc_reserved_mem(struct k3_ddrss_desc *ddrss) @@ -588,9 +693,10 @@ static void k3_ddrss_lpddr4_ecc_init(struct k3_ddrss_desc *ddrss) writel(DDRSS_ECC_CTRL_REG_ECC_EN | DDRSS_ECC_CTRL_REG_RMW_EN | DDRSS_ECC_CTRL_REG_WR_ALLOC, base + DDRSS_ECC_CTRL_REG);
- /* Preload ECC Mem region with 0's */ - k3_ddrss_preload_ecc_mem_region((u32 *)ecc_region_start, ecc_range, - 0x00000000); + /* Preload the full memory with 0's using the BIST engine of + * the LPDDR4 controller. + */ + k3_ddrss_lpddr4_preload_full_mem(ddrss, gd->ram_size, 0);
/* Clear Error Count Register */ writel(0x1, base + DDRSS_ECC_1B_ERR_CNT_REG);
As R5 is a 32 bit processor, the RAM banks' base and size calculation is restricted to 32 bits, which results in wrong values if bank's base is greater than 32 bits or bank's size is greater than or equal to 4GB.
So, add k3_ddrss_ddr_bank_base_size_calc() to get the base address and size of RAM's banks from the device tree memory node, and store in a 64 bit device private data which can be used for ECC reserved memory calculation, Setting ECC range and Fixing up bank size in device tree when ECC is enabled.
Signed-off-by: Santhosh Kumar K s-k6@ti.com --- drivers/ram/k3-ddrss/k3-ddrss.c | 72 ++++++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 15 deletions(-)
diff --git a/drivers/ram/k3-ddrss/k3-ddrss.c b/drivers/ram/k3-ddrss/k3-ddrss.c index f2ab8cf2b49b..bd6783ebc2cd 100644 --- a/drivers/ram/k3-ddrss/k3-ddrss.c +++ b/drivers/ram/k3-ddrss/k3-ddrss.c @@ -147,6 +147,9 @@ struct k3_ddrss_desc { struct k3_ddrss_ecc_region ecc_regions[K3_DDRSS_MAX_ECC_REGIONS]; u64 ecc_reserved_space; bool ti_ecc_enabled; + unsigned long long ddr_bank_base[CONFIG_NR_DRAM_BANKS]; + unsigned long long ddr_bank_size[CONFIG_NR_DRAM_BANKS]; + unsigned long long ddr_ram_size; };
struct reginitdata { @@ -668,11 +671,54 @@ static void k3_ddrss_lpddr4_preload_full_mem(struct k3_ddrss_desc *ddrss, printf("ECC: priming DDR completed in %lu msec\n", get_timer(done)); }
+static void k3_ddrss_ddr_bank_base_size_calc(struct k3_ddrss_desc *ddrss) +{ + int bank, na, ns, len, parent; + const fdt32_t *ptr, *end; + + for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) { + ddrss->ddr_bank_base[bank] = 0; + ddrss->ddr_bank_size[bank] = 0; + } + + ofnode mem = ofnode_null(); + + do { + mem = ofnode_by_prop_value(mem, "device_type", "memory", 7); + } while (!ofnode_is_enabled(mem)); + + const void *fdt = ofnode_to_fdt(mem); + int node = ofnode_to_offset(mem); + const char *property = "reg"; + + parent = fdt_parent_offset(fdt, node); + na = fdt_address_cells(fdt, parent); + ns = fdt_size_cells(fdt, parent); + ptr = fdt_getprop(fdt, node, property, &len); + end = ptr + len / sizeof(*ptr); + + for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) { + if (ptr + na + ns <= end) { + if (CONFIG_IS_ENABLED(OF_TRANSLATE)) + ddrss->ddr_bank_base[bank] = fdt_translate_address(fdt, node, ptr); + else + ddrss->ddr_bank_base[bank] = fdtdec_get_number(ptr, na); + + ddrss->ddr_bank_size[bank] = fdtdec_get_number(&ptr[na], ns); + } + + ptr += na + ns; + } + + for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) + ddrss->ddr_ram_size += ddrss->ddr_bank_size[bank]; +} + static void k3_ddrss_lpddr4_ecc_calc_reserved_mem(struct k3_ddrss_desc *ddrss) { fdtdec_setup_mem_size_base_lowest();
- ddrss->ecc_reserved_space = gd->ram_size; + ddrss->ecc_reserved_space = ddrss->ddr_ram_size; do_div(ddrss->ecc_reserved_space, 9);
/* Round to clean number */ @@ -696,7 +742,7 @@ static void k3_ddrss_lpddr4_ecc_init(struct k3_ddrss_desc *ddrss) /* Preload the full memory with 0's using the BIST engine of * the LPDDR4 controller. */ - k3_ddrss_lpddr4_preload_full_mem(ddrss, gd->ram_size, 0); + k3_ddrss_lpddr4_preload_full_mem(ddrss, ddrss->ddr_ram_size, 0);
/* Clear Error Count Register */ writel(0x1, base + DDRSS_ECC_1B_ERR_CNT_REG); @@ -741,6 +787,8 @@ static int k3_ddrss_probe(struct udevice *dev)
k3_lpddr4_start(ddrss);
+ k3_ddrss_ddr_bank_base_size_calc(ddrss); + if (ddrss->ti_ecc_enabled) { if (!ddrss->ddrss_ss_cfg) { printf("%s: ss_cfg is required if ecc is enabled but not provided.", @@ -761,30 +809,24 @@ static int k3_ddrss_probe(struct udevice *dev)
int k3_ddrss_ddr_fdt_fixup(struct udevice *dev, void *blob, struct bd_info *bd) { - struct k3_ddrss_desc *ddrss = dev_get_priv(dev); - u64 start[CONFIG_NR_DRAM_BANKS]; - u64 size[CONFIG_NR_DRAM_BANKS]; int bank; + struct k3_ddrss_desc *ddrss = dev_get_priv(dev);
if (ddrss->ecc_reserved_space == 0) return 0;
for (bank = CONFIG_NR_DRAM_BANKS - 1; bank >= 0; bank--) { - if (ddrss->ecc_reserved_space > bd->bi_dram[bank].size) { - ddrss->ecc_reserved_space -= bd->bi_dram[bank].size; - bd->bi_dram[bank].size = 0; + if (ddrss->ecc_reserved_space > ddrss->ddr_bank_size[bank]) { + ddrss->ecc_reserved_space -= ddrss->ddr_bank_size[bank]; + ddrss->ddr_bank_size[bank] = 0; } else { - bd->bi_dram[bank].size -= ddrss->ecc_reserved_space; + ddrss->ddr_bank_size[bank] -= ddrss->ecc_reserved_space; break; } }
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) { - start[bank] = bd->bi_dram[bank].start; - size[bank] = bd->bi_dram[bank].size; - } - - return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS); + return fdt_fixup_memory_banks(blob, ddrss->ddr_bank_base, + ddrss->ddr_bank_size, CONFIG_NR_DRAM_BANKS); }
static int k3_ddrss_get_info(struct udevice *dev, struct ram_info *info)
Hi Santhosh,
On 10:10-20241021, Santhosh Kumar K wrote:
As R5 is a 32 bit processor, the RAM banks' base and size calculation is restricted to 32 bits, which results in wrong values if bank's base is greater than 32 bits or bank's size is greater than or equal to 4GB.
So, add k3_ddrss_ddr_bank_base_size_calc() to get the base address and size of RAM's banks from the device tree memory node, and store in a 64 bit device private data which can be used for ECC reserved memory calculation, Setting ECC range and Fixing up bank size in device tree when ECC is enabled.
Signed-off-by: Santhosh Kumar K s-k6@ti.com
drivers/ram/k3-ddrss/k3-ddrss.c | 72 ++++++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 15 deletions(-)
diff --git a/drivers/ram/k3-ddrss/k3-ddrss.c b/drivers/ram/k3-ddrss/k3-ddrss.c index f2ab8cf2b49b..bd6783ebc2cd 100644 --- a/drivers/ram/k3-ddrss/k3-ddrss.c +++ b/drivers/ram/k3-ddrss/k3-ddrss.c @@ -147,6 +147,9 @@ struct k3_ddrss_desc { struct k3_ddrss_ecc_region ecc_regions[K3_DDRSS_MAX_ECC_REGIONS]; u64 ecc_reserved_space; bool ti_ecc_enabled;
- unsigned long long ddr_bank_base[CONFIG_NR_DRAM_BANKS];
- unsigned long long ddr_bank_size[CONFIG_NR_DRAM_BANKS];
- unsigned long long ddr_ram_size;
Would suggest using u64 ig if you are sending another revision, would be a bit more readable IMHO.
Regards, Manorit
};
struct reginitdata { @@ -668,11 +671,54 @@ static void k3_ddrss_lpddr4_preload_full_mem(struct k3_ddrss_desc *ddrss, printf("ECC: priming DDR completed in %lu msec\n", get_timer(done)); }
+static void k3_ddrss_ddr_bank_base_size_calc(struct k3_ddrss_desc *ddrss) +{
- int bank, na, ns, len, parent;
- const fdt32_t *ptr, *end;
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
ddrss->ddr_bank_base[bank] = 0;ddrss->ddr_bank_size[bank] = 0;- }
- ofnode mem = ofnode_null();
- do {
mem = ofnode_by_prop_value(mem, "device_type", "memory", 7);- } while (!ofnode_is_enabled(mem));
- const void *fdt = ofnode_to_fdt(mem);
- int node = ofnode_to_offset(mem);
- const char *property = "reg";
- parent = fdt_parent_offset(fdt, node);
- na = fdt_address_cells(fdt, parent);
- ns = fdt_size_cells(fdt, parent);
- ptr = fdt_getprop(fdt, node, property, &len);
- end = ptr + len / sizeof(*ptr);
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
if (ptr + na + ns <= end) {if (CONFIG_IS_ENABLED(OF_TRANSLATE))ddrss->ddr_bank_base[bank] = fdt_translate_address(fdt, node, ptr);elseddrss->ddr_bank_base[bank] = fdtdec_get_number(ptr, na);ddrss->ddr_bank_size[bank] = fdtdec_get_number(&ptr[na], ns);}ptr += na + ns;- }
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++)
ddrss->ddr_ram_size += ddrss->ddr_bank_size[bank];+}
static void k3_ddrss_lpddr4_ecc_calc_reserved_mem(struct k3_ddrss_desc *ddrss) { fdtdec_setup_mem_size_base_lowest();
- ddrss->ecc_reserved_space = gd->ram_size;
ddrss->ecc_reserved_space = ddrss->ddr_ram_size; do_div(ddrss->ecc_reserved_space, 9);
/* Round to clean number */
@@ -696,7 +742,7 @@ static void k3_ddrss_lpddr4_ecc_init(struct k3_ddrss_desc *ddrss) /* Preload the full memory with 0's using the BIST engine of * the LPDDR4 controller. */
- k3_ddrss_lpddr4_preload_full_mem(ddrss, gd->ram_size, 0);
k3_ddrss_lpddr4_preload_full_mem(ddrss, ddrss->ddr_ram_size, 0);
/* Clear Error Count Register */ writel(0x1, base + DDRSS_ECC_1B_ERR_CNT_REG);
@@ -741,6 +787,8 @@ static int k3_ddrss_probe(struct udevice *dev)
k3_lpddr4_start(ddrss);
- k3_ddrss_ddr_bank_base_size_calc(ddrss);
- if (ddrss->ti_ecc_enabled) { if (!ddrss->ddrss_ss_cfg) { printf("%s: ss_cfg is required if ecc is enabled but not provided.",
@@ -761,30 +809,24 @@ static int k3_ddrss_probe(struct udevice *dev)
int k3_ddrss_ddr_fdt_fixup(struct udevice *dev, void *blob, struct bd_info *bd) {
- struct k3_ddrss_desc *ddrss = dev_get_priv(dev);
- u64 start[CONFIG_NR_DRAM_BANKS];
- u64 size[CONFIG_NR_DRAM_BANKS]; int bank;
struct k3_ddrss_desc *ddrss = dev_get_priv(dev);
if (ddrss->ecc_reserved_space == 0) return 0;
for (bank = CONFIG_NR_DRAM_BANKS - 1; bank >= 0; bank--) {
if (ddrss->ecc_reserved_space > bd->bi_dram[bank].size) {ddrss->ecc_reserved_space -= bd->bi_dram[bank].size;bd->bi_dram[bank].size = 0;
if (ddrss->ecc_reserved_space > ddrss->ddr_bank_size[bank]) {ddrss->ecc_reserved_space -= ddrss->ddr_bank_size[bank];ddrss->ddr_bank_size[bank] = 0;
bd->bi_dram[bank].size -= ddrss->ecc_reserved_space;
ddrss->ddr_bank_size[bank] -= ddrss->ecc_reserved_space; break;
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
start[bank] = bd->bi_dram[bank].start;size[bank] = bd->bi_dram[bank].size;- }
- return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
- return fdt_fixup_memory_banks(blob, ddrss->ddr_bank_base,
ddrss->ddr_bank_size, CONFIG_NR_DRAM_BANKS);}
static int k3_ddrss_get_info(struct udevice *dev, struct ram_info *info)
2.34.1
Hi, Manorit,
On 23/10/24 10:14, Manorit Chawdhry wrote:
Hi Santhosh,
On 10:10-20241021, Santhosh Kumar K wrote:
As R5 is a 32 bit processor, the RAM banks' base and size calculation is restricted to 32 bits, which results in wrong values if bank's base is greater than 32 bits or bank's size is greater than or equal to 4GB.
So, add k3_ddrss_ddr_bank_base_size_calc() to get the base address and size of RAM's banks from the device tree memory node, and store in a 64 bit device private data which can be used for ECC reserved memory calculation, Setting ECC range and Fixing up bank size in device tree when ECC is enabled.
Signed-off-by: Santhosh Kumar K s-k6@ti.com
drivers/ram/k3-ddrss/k3-ddrss.c | 72 ++++++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 15 deletions(-)
diff --git a/drivers/ram/k3-ddrss/k3-ddrss.c b/drivers/ram/k3-ddrss/k3-ddrss.c index f2ab8cf2b49b..bd6783ebc2cd 100644 --- a/drivers/ram/k3-ddrss/k3-ddrss.c +++ b/drivers/ram/k3-ddrss/k3-ddrss.c @@ -147,6 +147,9 @@ struct k3_ddrss_desc { struct k3_ddrss_ecc_region ecc_regions[K3_DDRSS_MAX_ECC_REGIONS]; u64 ecc_reserved_space; bool ti_ecc_enabled;
- unsigned long long ddr_bank_base[CONFIG_NR_DRAM_BANKS];
- unsigned long long ddr_bank_size[CONFIG_NR_DRAM_BANKS];
- unsigned long long ddr_ram_size;
Would suggest using u64 ig if you are sending another revision, would be a bit more readable IMHO.
Yeah, sure!
Regards, Santhosh.
Regards, Manorit
};
struct reginitdata { @@ -668,11 +671,54 @@ static void k3_ddrss_lpddr4_preload_full_mem(struct k3_ddrss_desc *ddrss, printf("ECC: priming DDR completed in %lu msec\n", get_timer(done)); }
+static void k3_ddrss_ddr_bank_base_size_calc(struct k3_ddrss_desc *ddrss) +{
- int bank, na, ns, len, parent;
- const fdt32_t *ptr, *end;
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
ddrss->ddr_bank_base[bank] = 0;ddrss->ddr_bank_size[bank] = 0;- }
- ofnode mem = ofnode_null();
- do {
mem = ofnode_by_prop_value(mem, "device_type", "memory", 7);- } while (!ofnode_is_enabled(mem));
- const void *fdt = ofnode_to_fdt(mem);
- int node = ofnode_to_offset(mem);
- const char *property = "reg";
- parent = fdt_parent_offset(fdt, node);
- na = fdt_address_cells(fdt, parent);
- ns = fdt_size_cells(fdt, parent);
- ptr = fdt_getprop(fdt, node, property, &len);
- end = ptr + len / sizeof(*ptr);
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
if (ptr + na + ns <= end) {if (CONFIG_IS_ENABLED(OF_TRANSLATE))ddrss->ddr_bank_base[bank] = fdt_translate_address(fdt, node, ptr);elseddrss->ddr_bank_base[bank] = fdtdec_get_number(ptr, na);ddrss->ddr_bank_size[bank] = fdtdec_get_number(&ptr[na], ns);}ptr += na + ns;- }
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++)
ddrss->ddr_ram_size += ddrss->ddr_bank_size[bank];+}
- static void k3_ddrss_lpddr4_ecc_calc_reserved_mem(struct k3_ddrss_desc *ddrss) { fdtdec_setup_mem_size_base_lowest();
- ddrss->ecc_reserved_space = gd->ram_size;
ddrss->ecc_reserved_space = ddrss->ddr_ram_size; do_div(ddrss->ecc_reserved_space, 9);
/* Round to clean number */
@@ -696,7 +742,7 @@ static void k3_ddrss_lpddr4_ecc_init(struct k3_ddrss_desc *ddrss) /* Preload the full memory with 0's using the BIST engine of * the LPDDR4 controller. */
- k3_ddrss_lpddr4_preload_full_mem(ddrss, gd->ram_size, 0);
k3_ddrss_lpddr4_preload_full_mem(ddrss, ddrss->ddr_ram_size, 0);
/* Clear Error Count Register */ writel(0x1, base + DDRSS_ECC_1B_ERR_CNT_REG);
@@ -741,6 +787,8 @@ static int k3_ddrss_probe(struct udevice *dev)
k3_lpddr4_start(ddrss);
- k3_ddrss_ddr_bank_base_size_calc(ddrss);
- if (ddrss->ti_ecc_enabled) { if (!ddrss->ddrss_ss_cfg) { printf("%s: ss_cfg is required if ecc is enabled but not provided.",
@@ -761,30 +809,24 @@ static int k3_ddrss_probe(struct udevice *dev)
int k3_ddrss_ddr_fdt_fixup(struct udevice *dev, void *blob, struct bd_info *bd) {
- struct k3_ddrss_desc *ddrss = dev_get_priv(dev);
- u64 start[CONFIG_NR_DRAM_BANKS];
- u64 size[CONFIG_NR_DRAM_BANKS]; int bank;
struct k3_ddrss_desc *ddrss = dev_get_priv(dev);
if (ddrss->ecc_reserved_space == 0) return 0;
for (bank = CONFIG_NR_DRAM_BANKS - 1; bank >= 0; bank--) {
if (ddrss->ecc_reserved_space > bd->bi_dram[bank].size) {ddrss->ecc_reserved_space -= bd->bi_dram[bank].size;bd->bi_dram[bank].size = 0;
if (ddrss->ecc_reserved_space > ddrss->ddr_bank_size[bank]) {ddrss->ecc_reserved_space -= ddrss->ddr_bank_size[bank];ddrss->ddr_bank_size[bank] = 0;
bd->bi_dram[bank].size -= ddrss->ecc_reserved_space;
ddrss->ddr_bank_size[bank] -= ddrss->ecc_reserved_space; break;
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
start[bank] = bd->bi_dram[bank].start;size[bank] = bd->bi_dram[bank].size;- }
- return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
return fdt_fixup_memory_banks(blob, ddrss->ddr_bank_base,
ddrss->ddr_bank_size, CONFIG_NR_DRAM_BANKS);}
static int k3_ddrss_get_info(struct udevice *dev, struct ram_info *info)
-- 2.34.1
On October 21, 2024 thus sayeth Santhosh Kumar K:
As R5 is a 32 bit processor, the RAM banks' base and size calculation is restricted to 32 bits, which results in wrong values if bank's base is greater than 32 bits or bank's size is greater than or equal to 4GB.
So, add k3_ddrss_ddr_bank_base_size_calc() to get the base address and size of RAM's banks from the device tree memory node, and store in a 64 bit device private data which can be used for ECC reserved memory calculation, Setting ECC range and Fixing up bank size in device tree when ECC is enabled.
Signed-off-by: Santhosh Kumar K s-k6@ti.com
drivers/ram/k3-ddrss/k3-ddrss.c | 72 ++++++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 15 deletions(-)
diff --git a/drivers/ram/k3-ddrss/k3-ddrss.c b/drivers/ram/k3-ddrss/k3-ddrss.c index f2ab8cf2b49b..bd6783ebc2cd 100644 --- a/drivers/ram/k3-ddrss/k3-ddrss.c +++ b/drivers/ram/k3-ddrss/k3-ddrss.c @@ -147,6 +147,9 @@ struct k3_ddrss_desc { struct k3_ddrss_ecc_region ecc_regions[K3_DDRSS_MAX_ECC_REGIONS]; u64 ecc_reserved_space; bool ti_ecc_enabled;
- unsigned long long ddr_bank_base[CONFIG_NR_DRAM_BANKS];
- unsigned long long ddr_bank_size[CONFIG_NR_DRAM_BANKS];
- unsigned long long ddr_ram_size;
Should we use the u64 typedef here?
};
struct reginitdata { @@ -668,11 +671,54 @@ static void k3_ddrss_lpddr4_preload_full_mem(struct k3_ddrss_desc *ddrss, printf("ECC: priming DDR completed in %lu msec\n", get_timer(done)); }
+static void k3_ddrss_ddr_bank_base_size_calc(struct k3_ddrss_desc *ddrss) +{
- int bank, na, ns, len, parent;
- const fdt32_t *ptr, *end;
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
ddrss->ddr_bank_base[bank] = 0;ddrss->ddr_bank_size[bank] = 0;- }
- ofnode mem = ofnode_null();
- do {
mem = ofnode_by_prop_value(mem, "device_type", "memory", 7);- } while (!ofnode_is_enabled(mem));
- const void *fdt = ofnode_to_fdt(mem);
- int node = ofnode_to_offset(mem);
- const char *property = "reg";
- parent = fdt_parent_offset(fdt, node);
- na = fdt_address_cells(fdt, parent);
- ns = fdt_size_cells(fdt, parent);
- ptr = fdt_getprop(fdt, node, property, &len);
- end = ptr + len / sizeof(*ptr);
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
if (ptr + na + ns <= end) {if (CONFIG_IS_ENABLED(OF_TRANSLATE))ddrss->ddr_bank_base[bank] = fdt_translate_address(fdt, node, ptr);elseddrss->ddr_bank_base[bank] = fdtdec_get_number(ptr, na);ddrss->ddr_bank_size[bank] = fdtdec_get_number(&ptr[na], ns);}ptr += na + ns;- }
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++)
ddrss->ddr_ram_size += ddrss->ddr_bank_size[bank];+}
static void k3_ddrss_lpddr4_ecc_calc_reserved_mem(struct k3_ddrss_desc *ddrss) { fdtdec_setup_mem_size_base_lowest();
- ddrss->ecc_reserved_space = gd->ram_size;
ddrss->ecc_reserved_space = ddrss->ddr_ram_size; do_div(ddrss->ecc_reserved_space, 9);
/* Round to clean number */
@@ -696,7 +742,7 @@ static void k3_ddrss_lpddr4_ecc_init(struct k3_ddrss_desc *ddrss) /* Preload the full memory with 0's using the BIST engine of * the LPDDR4 controller. */
- k3_ddrss_lpddr4_preload_full_mem(ddrss, gd->ram_size, 0);
k3_ddrss_lpddr4_preload_full_mem(ddrss, ddrss->ddr_ram_size, 0);
/* Clear Error Count Register */ writel(0x1, base + DDRSS_ECC_1B_ERR_CNT_REG);
@@ -741,6 +787,8 @@ static int k3_ddrss_probe(struct udevice *dev)
k3_lpddr4_start(ddrss);
- k3_ddrss_ddr_bank_base_size_calc(ddrss);
- if (ddrss->ti_ecc_enabled) { if (!ddrss->ddrss_ss_cfg) { printf("%s: ss_cfg is required if ecc is enabled but not provided.",
@@ -761,30 +809,24 @@ static int k3_ddrss_probe(struct udevice *dev)
int k3_ddrss_ddr_fdt_fixup(struct udevice *dev, void *blob, struct bd_info *bd) {
- struct k3_ddrss_desc *ddrss = dev_get_priv(dev);
- u64 start[CONFIG_NR_DRAM_BANKS];
- u64 size[CONFIG_NR_DRAM_BANKS]; int bank;
struct k3_ddrss_desc *ddrss = dev_get_priv(dev);
if (ddrss->ecc_reserved_space == 0) return 0;
for (bank = CONFIG_NR_DRAM_BANKS - 1; bank >= 0; bank--) {
if (ddrss->ecc_reserved_space > bd->bi_dram[bank].size) {ddrss->ecc_reserved_space -= bd->bi_dram[bank].size;bd->bi_dram[bank].size = 0;
if (ddrss->ecc_reserved_space > ddrss->ddr_bank_size[bank]) {ddrss->ecc_reserved_space -= ddrss->ddr_bank_size[bank];ddrss->ddr_bank_size[bank] = 0;
bd->bi_dram[bank].size -= ddrss->ecc_reserved_space;
ddrss->ddr_bank_size[bank] -= ddrss->ecc_reserved_space; break;
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
start[bank] = bd->bi_dram[bank].start;size[bank] = bd->bi_dram[bank].size;- }
- return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
- return fdt_fixup_memory_banks(blob, ddrss->ddr_bank_base,
ddrss->ddr_bank_size, CONFIG_NR_DRAM_BANKS);}
Have we looked into passing this information via the global data pointer rather than fixing up the device tree on each boot phase?
https://patchwork.ozlabs.org/project/uboot/patch/20180926215520.87168-23-sjg...
I don't see a clear path to pass GD from SPL to SPL but there is a TPL to SPL which I think we should be able to copy.
~Bryan
Hi Bryan
On 23/10/24 20:09, Bryan Brattlof wrote:
On October 21, 2024 thus sayeth Santhosh Kumar K:
As R5 is a 32 bit processor, the RAM banks' base and size calculation is restricted to 32 bits, which results in wrong values if bank's base is greater than 32 bits or bank's size is greater than or equal to 4GB.
So, add k3_ddrss_ddr_bank_base_size_calc() to get the base address and size of RAM's banks from the device tree memory node, and store in a 64 bit device private data which can be used for ECC reserved memory calculation, Setting ECC range and Fixing up bank size in device tree when ECC is enabled.
Signed-off-by: Santhosh Kumar K s-k6@ti.com
drivers/ram/k3-ddrss/k3-ddrss.c | 72 ++++++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 15 deletions(-)
diff --git a/drivers/ram/k3-ddrss/k3-ddrss.c b/drivers/ram/k3-ddrss/k3-ddrss.c index f2ab8cf2b49b..bd6783ebc2cd 100644 --- a/drivers/ram/k3-ddrss/k3-ddrss.c +++ b/drivers/ram/k3-ddrss/k3-ddrss.c @@ -147,6 +147,9 @@ struct k3_ddrss_desc { struct k3_ddrss_ecc_region ecc_regions[K3_DDRSS_MAX_ECC_REGIONS]; u64 ecc_reserved_space; bool ti_ecc_enabled;
- unsigned long long ddr_bank_base[CONFIG_NR_DRAM_BANKS];
- unsigned long long ddr_bank_size[CONFIG_NR_DRAM_BANKS];
- unsigned long long ddr_ram_size;
Should we use the u64 typedef here?
};
struct reginitdata { @@ -668,11 +671,54 @@ static void k3_ddrss_lpddr4_preload_full_mem(struct k3_ddrss_desc *ddrss, printf("ECC: priming DDR completed in %lu msec\n", get_timer(done)); }
+static void k3_ddrss_ddr_bank_base_size_calc(struct k3_ddrss_desc *ddrss) +{
- int bank, na, ns, len, parent;
- const fdt32_t *ptr, *end;
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
ddrss->ddr_bank_base[bank] = 0;ddrss->ddr_bank_size[bank] = 0;- }
- ofnode mem = ofnode_null();
- do {
mem = ofnode_by_prop_value(mem, "device_type", "memory", 7);- } while (!ofnode_is_enabled(mem));
- const void *fdt = ofnode_to_fdt(mem);
- int node = ofnode_to_offset(mem);
- const char *property = "reg";
- parent = fdt_parent_offset(fdt, node);
- na = fdt_address_cells(fdt, parent);
- ns = fdt_size_cells(fdt, parent);
- ptr = fdt_getprop(fdt, node, property, &len);
- end = ptr + len / sizeof(*ptr);
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
if (ptr + na + ns <= end) {if (CONFIG_IS_ENABLED(OF_TRANSLATE))ddrss->ddr_bank_base[bank] = fdt_translate_address(fdt, node, ptr);elseddrss->ddr_bank_base[bank] = fdtdec_get_number(ptr, na);ddrss->ddr_bank_size[bank] = fdtdec_get_number(&ptr[na], ns);}ptr += na + ns;- }
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++)
ddrss->ddr_ram_size += ddrss->ddr_bank_size[bank];+}
- static void k3_ddrss_lpddr4_ecc_calc_reserved_mem(struct k3_ddrss_desc *ddrss) { fdtdec_setup_mem_size_base_lowest();
- ddrss->ecc_reserved_space = gd->ram_size;
ddrss->ecc_reserved_space = ddrss->ddr_ram_size; do_div(ddrss->ecc_reserved_space, 9);
/* Round to clean number */
@@ -696,7 +742,7 @@ static void k3_ddrss_lpddr4_ecc_init(struct k3_ddrss_desc *ddrss) /* Preload the full memory with 0's using the BIST engine of * the LPDDR4 controller. */
- k3_ddrss_lpddr4_preload_full_mem(ddrss, gd->ram_size, 0);
k3_ddrss_lpddr4_preload_full_mem(ddrss, ddrss->ddr_ram_size, 0);
/* Clear Error Count Register */ writel(0x1, base + DDRSS_ECC_1B_ERR_CNT_REG);
@@ -741,6 +787,8 @@ static int k3_ddrss_probe(struct udevice *dev)
k3_lpddr4_start(ddrss);
- k3_ddrss_ddr_bank_base_size_calc(ddrss);
- if (ddrss->ti_ecc_enabled) { if (!ddrss->ddrss_ss_cfg) { printf("%s: ss_cfg is required if ecc is enabled but not provided.",
@@ -761,30 +809,24 @@ static int k3_ddrss_probe(struct udevice *dev)
int k3_ddrss_ddr_fdt_fixup(struct udevice *dev, void *blob, struct bd_info *bd) {
- struct k3_ddrss_desc *ddrss = dev_get_priv(dev);
- u64 start[CONFIG_NR_DRAM_BANKS];
- u64 size[CONFIG_NR_DRAM_BANKS]; int bank;
struct k3_ddrss_desc *ddrss = dev_get_priv(dev);
if (ddrss->ecc_reserved_space == 0) return 0;
for (bank = CONFIG_NR_DRAM_BANKS - 1; bank >= 0; bank--) {
if (ddrss->ecc_reserved_space > bd->bi_dram[bank].size) {ddrss->ecc_reserved_space -= bd->bi_dram[bank].size;bd->bi_dram[bank].size = 0;
if (ddrss->ecc_reserved_space > ddrss->ddr_bank_size[bank]) {ddrss->ecc_reserved_space -= ddrss->ddr_bank_size[bank];ddrss->ddr_bank_size[bank] = 0;
bd->bi_dram[bank].size -= ddrss->ecc_reserved_space;
ddrss->ddr_bank_size[bank] -= ddrss->ecc_reserved_space; break;
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
start[bank] = bd->bi_dram[bank].start;size[bank] = bd->bi_dram[bank].size;- }
- return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
- return fdt_fixup_memory_banks(blob, ddrss->ddr_bank_base,
ddrss->ddr_bank_size, CONFIG_NR_DRAM_BANKS);Have we looked into passing this information via the global data pointer rather than fixing up the device tree on each boot phase?
https://patchwork.ozlabs.org/project/uboot/patch/20180926215520.87168-23-sjg@chromium.org/I don't see a clear path to pass GD from SPL to SPL but there is a TPL to SPL which I think we should be able to copy.
But we also make use of this to fix up the device tree that passes onto kernel as well, I need to look into this patch though. But from a first glance looks like we will be able to pass on the information from SPL->SPL (with modification as you said) and SPL->U-Boot but U-Boot->Kernel would still require this function to be present.
~Bryan
On October 24, 2024 thus sayeth Neha Malcom Francis:
Hi Bryan
On 23/10/24 20:09, Bryan Brattlof wrote:
On October 21, 2024 thus sayeth Santhosh Kumar K:
As R5 is a 32 bit processor, the RAM banks' base and size calculation is restricted to 32 bits, which results in wrong values if bank's base is greater than 32 bits or bank's size is greater than or equal to 4GB.
So, add k3_ddrss_ddr_bank_base_size_calc() to get the base address and size of RAM's banks from the device tree memory node, and store in a 64 bit device private data which can be used for ECC reserved memory calculation, Setting ECC range and Fixing up bank size in device tree when ECC is enabled.
Signed-off-by: Santhosh Kumar K s-k6@ti.com
drivers/ram/k3-ddrss/k3-ddrss.c | 72 ++++++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 15 deletions(-)
diff --git a/drivers/ram/k3-ddrss/k3-ddrss.c b/drivers/ram/k3-ddrss/k3-ddrss.c index f2ab8cf2b49b..bd6783ebc2cd 100644 --- a/drivers/ram/k3-ddrss/k3-ddrss.c +++ b/drivers/ram/k3-ddrss/k3-ddrss.c @@ -147,6 +147,9 @@ struct k3_ddrss_desc { struct k3_ddrss_ecc_region ecc_regions[K3_DDRSS_MAX_ECC_REGIONS]; u64 ecc_reserved_space; bool ti_ecc_enabled;
- unsigned long long ddr_bank_base[CONFIG_NR_DRAM_BANKS];
- unsigned long long ddr_bank_size[CONFIG_NR_DRAM_BANKS];
- unsigned long long ddr_ram_size;
Should we use the u64 typedef here?
}; struct reginitdata { @@ -668,11 +671,54 @@ static void k3_ddrss_lpddr4_preload_full_mem(struct k3_ddrss_desc *ddrss, printf("ECC: priming DDR completed in %lu msec\n", get_timer(done)); } +static void k3_ddrss_ddr_bank_base_size_calc(struct k3_ddrss_desc *ddrss) +{
- int bank, na, ns, len, parent;
- const fdt32_t *ptr, *end;
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
ddrss->ddr_bank_base[bank] = 0;ddrss->ddr_bank_size[bank] = 0;- }
- ofnode mem = ofnode_null();
- do {
mem = ofnode_by_prop_value(mem, "device_type", "memory", 7);- } while (!ofnode_is_enabled(mem));
- const void *fdt = ofnode_to_fdt(mem);
- int node = ofnode_to_offset(mem);
- const char *property = "reg";
- parent = fdt_parent_offset(fdt, node);
- na = fdt_address_cells(fdt, parent);
- ns = fdt_size_cells(fdt, parent);
- ptr = fdt_getprop(fdt, node, property, &len);
- end = ptr + len / sizeof(*ptr);
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
if (ptr + na + ns <= end) {if (CONFIG_IS_ENABLED(OF_TRANSLATE))ddrss->ddr_bank_base[bank] = fdt_translate_address(fdt, node, ptr);elseddrss->ddr_bank_base[bank] = fdtdec_get_number(ptr, na);ddrss->ddr_bank_size[bank] = fdtdec_get_number(&ptr[na], ns);}ptr += na + ns;- }
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++)
ddrss->ddr_ram_size += ddrss->ddr_bank_size[bank];+}
- static void k3_ddrss_lpddr4_ecc_calc_reserved_mem(struct k3_ddrss_desc *ddrss) { fdtdec_setup_mem_size_base_lowest();
- ddrss->ecc_reserved_space = gd->ram_size;
- ddrss->ecc_reserved_space = ddrss->ddr_ram_size; do_div(ddrss->ecc_reserved_space, 9); /* Round to clean number */
@@ -696,7 +742,7 @@ static void k3_ddrss_lpddr4_ecc_init(struct k3_ddrss_desc *ddrss) /* Preload the full memory with 0's using the BIST engine of * the LPDDR4 controller. */
- k3_ddrss_lpddr4_preload_full_mem(ddrss, gd->ram_size, 0);
- k3_ddrss_lpddr4_preload_full_mem(ddrss, ddrss->ddr_ram_size, 0); /* Clear Error Count Register */ writel(0x1, base + DDRSS_ECC_1B_ERR_CNT_REG);
@@ -741,6 +787,8 @@ static int k3_ddrss_probe(struct udevice *dev) k3_lpddr4_start(ddrss);
- k3_ddrss_ddr_bank_base_size_calc(ddrss);
- if (ddrss->ti_ecc_enabled) { if (!ddrss->ddrss_ss_cfg) { printf("%s: ss_cfg is required if ecc is enabled but not provided.",
@@ -761,30 +809,24 @@ static int k3_ddrss_probe(struct udevice *dev) int k3_ddrss_ddr_fdt_fixup(struct udevice *dev, void *blob, struct bd_info *bd) {
- struct k3_ddrss_desc *ddrss = dev_get_priv(dev);
- u64 start[CONFIG_NR_DRAM_BANKS];
- u64 size[CONFIG_NR_DRAM_BANKS]; int bank;
- struct k3_ddrss_desc *ddrss = dev_get_priv(dev); if (ddrss->ecc_reserved_space == 0) return 0; for (bank = CONFIG_NR_DRAM_BANKS - 1; bank >= 0; bank--) {
if (ddrss->ecc_reserved_space > bd->bi_dram[bank].size) {ddrss->ecc_reserved_space -= bd->bi_dram[bank].size;bd->bi_dram[bank].size = 0;
if (ddrss->ecc_reserved_space > ddrss->ddr_bank_size[bank]) {ddrss->ecc_reserved_space -= ddrss->ddr_bank_size[bank];ddrss->ddr_bank_size[bank] = 0;
bd->bi_dram[bank].size -= ddrss->ecc_reserved_space;
ddrss->ddr_bank_size[bank] -= ddrss->ecc_reserved_space; break;
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
start[bank] = bd->bi_dram[bank].start;size[bank] = bd->bi_dram[bank].size;- }
- return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
- return fdt_fixup_memory_banks(blob, ddrss->ddr_bank_base,
ddrss->ddr_bank_size, CONFIG_NR_DRAM_BANKS);Have we looked into passing this information via the global data pointer rather than fixing up the device tree on each boot phase?
https://patchwork.ozlabs.org/project/uboot/patch/20180926215520.87168-23-sjg@chromium.org/I don't see a clear path to pass GD from SPL to SPL but there is a TPL to SPL which I think we should be able to copy.
But we also make use of this to fix up the device tree that passes onto kernel as well, I need to look into this patch though. But from a first glance looks like we will be able to pass on the information from SPL->SPL (with modification as you said) and SPL->U-Boot but U-Boot->Kernel would still require this function to be present.
I agree. If this scheme works out we can place this kernel fixup function with the other OPN fixups we do though later on in the boot.
https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/mach-k3/common_f...
Am 24.10.24 um 18:19 schrieb Bryan Brattlof:
On October 24, 2024 thus sayeth Neha Malcom Francis:
Hi Bryan
On 23/10/24 20:09, Bryan Brattlof wrote:
On October 21, 2024 thus sayeth Santhosh Kumar K:
As R5 is a 32 bit processor, the RAM banks' base and size calculation is restricted to 32 bits, which results in wrong values if bank's base is greater than 32 bits or bank's size is greater than or equal to 4GB.
So, add k3_ddrss_ddr_bank_base_size_calc() to get the base address and size of RAM's banks from the device tree memory node, and store in a 64 bit device private data which can be used for ECC reserved memory calculation, Setting ECC range and Fixing up bank size in device tree when ECC is enabled.
Signed-off-by: Santhosh Kumar K s-k6@ti.com
drivers/ram/k3-ddrss/k3-ddrss.c | 72 ++++++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 15 deletions(-)
diff --git a/drivers/ram/k3-ddrss/k3-ddrss.c b/drivers/ram/k3-ddrss/k3-ddrss.c index f2ab8cf2b49b..bd6783ebc2cd 100644 --- a/drivers/ram/k3-ddrss/k3-ddrss.c +++ b/drivers/ram/k3-ddrss/k3-ddrss.c @@ -147,6 +147,9 @@ struct k3_ddrss_desc { struct k3_ddrss_ecc_region ecc_regions[K3_DDRSS_MAX_ECC_REGIONS]; u64 ecc_reserved_space; bool ti_ecc_enabled;
- unsigned long long ddr_bank_base[CONFIG_NR_DRAM_BANKS];
- unsigned long long ddr_bank_size[CONFIG_NR_DRAM_BANKS];
- unsigned long long ddr_ram_size;
Should we use the u64 typedef here?
}; struct reginitdata { @@ -668,11 +671,54 @@ static void k3_ddrss_lpddr4_preload_full_mem(struct k3_ddrss_desc *ddrss, printf("ECC: priming DDR completed in %lu msec\n", get_timer(done)); } +static void k3_ddrss_ddr_bank_base_size_calc(struct k3_ddrss_desc *ddrss) +{
- int bank, na, ns, len, parent;
- const fdt32_t *ptr, *end;
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
ddrss->ddr_bank_base[bank] = 0;ddrss->ddr_bank_size[bank] = 0;- }
- ofnode mem = ofnode_null();
- do {
mem = ofnode_by_prop_value(mem, "device_type", "memory", 7);- } while (!ofnode_is_enabled(mem));
- const void *fdt = ofnode_to_fdt(mem);
- int node = ofnode_to_offset(mem);
- const char *property = "reg";
- parent = fdt_parent_offset(fdt, node);
- na = fdt_address_cells(fdt, parent);
- ns = fdt_size_cells(fdt, parent);
- ptr = fdt_getprop(fdt, node, property, &len);
- end = ptr + len / sizeof(*ptr);
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
if (ptr + na + ns <= end) {if (CONFIG_IS_ENABLED(OF_TRANSLATE))ddrss->ddr_bank_base[bank] = fdt_translate_address(fdt, node, ptr);elseddrss->ddr_bank_base[bank] = fdtdec_get_number(ptr, na);ddrss->ddr_bank_size[bank] = fdtdec_get_number(&ptr[na], ns);}ptr += na + ns;- }
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++)
ddrss->ddr_ram_size += ddrss->ddr_bank_size[bank];+}
- static void k3_ddrss_lpddr4_ecc_calc_reserved_mem(struct k3_ddrss_desc *ddrss) { fdtdec_setup_mem_size_base_lowest();
- ddrss->ecc_reserved_space = gd->ram_size;
- ddrss->ecc_reserved_space = ddrss->ddr_ram_size; do_div(ddrss->ecc_reserved_space, 9); /* Round to clean number */
@@ -696,7 +742,7 @@ static void k3_ddrss_lpddr4_ecc_init(struct k3_ddrss_desc *ddrss) /* Preload the full memory with 0's using the BIST engine of * the LPDDR4 controller. */
- k3_ddrss_lpddr4_preload_full_mem(ddrss, gd->ram_size, 0);
- k3_ddrss_lpddr4_preload_full_mem(ddrss, ddrss->ddr_ram_size, 0); /* Clear Error Count Register */ writel(0x1, base + DDRSS_ECC_1B_ERR_CNT_REG);
@@ -741,6 +787,8 @@ static int k3_ddrss_probe(struct udevice *dev) k3_lpddr4_start(ddrss);
- k3_ddrss_ddr_bank_base_size_calc(ddrss);
- if (ddrss->ti_ecc_enabled) { if (!ddrss->ddrss_ss_cfg) { printf("%s: ss_cfg is required if ecc is enabled but not provided.",
@@ -761,30 +809,24 @@ static int k3_ddrss_probe(struct udevice *dev) int k3_ddrss_ddr_fdt_fixup(struct udevice *dev, void *blob, struct bd_info *bd) {
- struct k3_ddrss_desc *ddrss = dev_get_priv(dev);
- u64 start[CONFIG_NR_DRAM_BANKS];
- u64 size[CONFIG_NR_DRAM_BANKS]; int bank;
- struct k3_ddrss_desc *ddrss = dev_get_priv(dev); if (ddrss->ecc_reserved_space == 0) return 0; for (bank = CONFIG_NR_DRAM_BANKS - 1; bank >= 0; bank--) {
if (ddrss->ecc_reserved_space > bd->bi_dram[bank].size) {ddrss->ecc_reserved_space -= bd->bi_dram[bank].size;bd->bi_dram[bank].size = 0;
if (ddrss->ecc_reserved_space > ddrss->ddr_bank_size[bank]) {ddrss->ecc_reserved_space -= ddrss->ddr_bank_size[bank];ddrss->ddr_bank_size[bank] = 0; } else {
bd->bi_dram[bank].size -= ddrss->ecc_reserved_space;
ddrss->ddr_bank_size[bank] -= ddrss->ecc_reserved_space; break; }
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
start[bank] = bd->bi_dram[bank].start;size[bank] = bd->bi_dram[bank].size;- }
- return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
- return fdt_fixup_memory_banks(blob, ddrss->ddr_bank_base,
ddrss->ddr_bank_size, CONFIG_NR_DRAM_BANKS);Have we looked into passing this information via the global data pointer rather than fixing up the device tree on each boot phase?
https://patchwork.ozlabs.org/project/uboot/patch/20180926215520.87168-23-sjg@chromium.org/I don't see a clear path to pass GD from SPL to SPL but there is a TPL to SPL which I think we should be able to copy.
But we also make use of this to fix up the device tree that passes onto kernel as well, I need to look into this patch though. But from a first glance looks like we will be able to pass on the information from SPL->SPL (with modification as you said) and SPL->U-Boot but U-Boot->Kernel would still require this function to be present.
Just came across this series again and noticed that this will be a bit tricky with boards detecting RAM at runtime.
Updating the memory node with detected RAM size prio to DDR driver probing should be no problem. But the next stages will need to update u-boot's and Linux memory nodes too. Unfortunately, we do not have the data at this stage which tells us how much space was reserved for ECC.
It would be best to do the memory setup for GD or memory node once and have other stages reuse it.
Right now I have no idea how my board should be aware of the reserved ecc memory in u-boot. Any suggestions?
I agree. If this scheme works out we can place this kernel fixup function with the other OPN fixups we do though later on in the boot.
https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/mach-k3/common_f...
On December 6, 2024 thus sayeth Wadim Egorov:
Am 24.10.24 um 18:19 schrieb Bryan Brattlof:
On October 24, 2024 thus sayeth Neha Malcom Francis:
Hi Bryan
On 23/10/24 20:09, Bryan Brattlof wrote:
On October 21, 2024 thus sayeth Santhosh Kumar K:
As R5 is a 32 bit processor, the RAM banks' base and size calculation is restricted to 32 bits, which results in wrong values if bank's base is greater than 32 bits or bank's size is greater than or equal to 4GB.
So, add k3_ddrss_ddr_bank_base_size_calc() to get the base address and size of RAM's banks from the device tree memory node, and store in a 64 bit device private data which can be used for ECC reserved memory calculation, Setting ECC range and Fixing up bank size in device tree when ECC is enabled.
Signed-off-by: Santhosh Kumar K s-k6@ti.com
drivers/ram/k3-ddrss/k3-ddrss.c | 72 ++++++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 15 deletions(-)
diff --git a/drivers/ram/k3-ddrss/k3-ddrss.c b/drivers/ram/k3-ddrss/k3-ddrss.c index f2ab8cf2b49b..bd6783ebc2cd 100644 --- a/drivers/ram/k3-ddrss/k3-ddrss.c +++ b/drivers/ram/k3-ddrss/k3-ddrss.c @@ -147,6 +147,9 @@ struct k3_ddrss_desc { struct k3_ddrss_ecc_region ecc_regions[K3_DDRSS_MAX_ECC_REGIONS]; u64 ecc_reserved_space; bool ti_ecc_enabled;
- unsigned long long ddr_bank_base[CONFIG_NR_DRAM_BANKS];
- unsigned long long ddr_bank_size[CONFIG_NR_DRAM_BANKS];
- unsigned long long ddr_ram_size;
Should we use the u64 typedef here?
}; struct reginitdata { @@ -668,11 +671,54 @@ static void k3_ddrss_lpddr4_preload_full_mem(struct k3_ddrss_desc *ddrss, printf("ECC: priming DDR completed in %lu msec\n", get_timer(done)); } +static void k3_ddrss_ddr_bank_base_size_calc(struct k3_ddrss_desc *ddrss) +{
- int bank, na, ns, len, parent;
- const fdt32_t *ptr, *end;
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
ddrss->ddr_bank_base[bank] = 0;ddrss->ddr_bank_size[bank] = 0;- }
- ofnode mem = ofnode_null();
- do {
mem = ofnode_by_prop_value(mem, "device_type", "memory", 7);- } while (!ofnode_is_enabled(mem));
- const void *fdt = ofnode_to_fdt(mem);
- int node = ofnode_to_offset(mem);
- const char *property = "reg";
- parent = fdt_parent_offset(fdt, node);
- na = fdt_address_cells(fdt, parent);
- ns = fdt_size_cells(fdt, parent);
- ptr = fdt_getprop(fdt, node, property, &len);
- end = ptr + len / sizeof(*ptr);
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
if (ptr + na + ns <= end) {if (CONFIG_IS_ENABLED(OF_TRANSLATE))ddrss->ddr_bank_base[bank] = fdt_translate_address(fdt, node, ptr);elseddrss->ddr_bank_base[bank] = fdtdec_get_number(ptr, na);ddrss->ddr_bank_size[bank] = fdtdec_get_number(&ptr[na], ns);}ptr += na + ns;- }
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++)
ddrss->ddr_ram_size += ddrss->ddr_bank_size[bank];+}
- static void k3_ddrss_lpddr4_ecc_calc_reserved_mem(struct k3_ddrss_desc *ddrss) { fdtdec_setup_mem_size_base_lowest();
- ddrss->ecc_reserved_space = gd->ram_size;
- ddrss->ecc_reserved_space = ddrss->ddr_ram_size; do_div(ddrss->ecc_reserved_space, 9); /* Round to clean number */
@@ -696,7 +742,7 @@ static void k3_ddrss_lpddr4_ecc_init(struct k3_ddrss_desc *ddrss) /* Preload the full memory with 0's using the BIST engine of * the LPDDR4 controller. */
- k3_ddrss_lpddr4_preload_full_mem(ddrss, gd->ram_size, 0);
- k3_ddrss_lpddr4_preload_full_mem(ddrss, ddrss->ddr_ram_size, 0); /* Clear Error Count Register */ writel(0x1, base + DDRSS_ECC_1B_ERR_CNT_REG);
@@ -741,6 +787,8 @@ static int k3_ddrss_probe(struct udevice *dev) k3_lpddr4_start(ddrss);
- k3_ddrss_ddr_bank_base_size_calc(ddrss);
- if (ddrss->ti_ecc_enabled) { if (!ddrss->ddrss_ss_cfg) { printf("%s: ss_cfg is required if ecc is enabled but not provided.",
@@ -761,30 +809,24 @@ static int k3_ddrss_probe(struct udevice *dev) int k3_ddrss_ddr_fdt_fixup(struct udevice *dev, void *blob, struct bd_info *bd) {
- struct k3_ddrss_desc *ddrss = dev_get_priv(dev);
- u64 start[CONFIG_NR_DRAM_BANKS];
- u64 size[CONFIG_NR_DRAM_BANKS]; int bank;
- struct k3_ddrss_desc *ddrss = dev_get_priv(dev); if (ddrss->ecc_reserved_space == 0) return 0; for (bank = CONFIG_NR_DRAM_BANKS - 1; bank >= 0; bank--) {
if (ddrss->ecc_reserved_space > bd->bi_dram[bank].size) {ddrss->ecc_reserved_space -= bd->bi_dram[bank].size;bd->bi_dram[bank].size = 0;
if (ddrss->ecc_reserved_space > ddrss->ddr_bank_size[bank]) {ddrss->ecc_reserved_space -= ddrss->ddr_bank_size[bank];ddrss->ddr_bank_size[bank] = 0; } else {
bd->bi_dram[bank].size -= ddrss->ecc_reserved_space;
ddrss->ddr_bank_size[bank] -= ddrss->ecc_reserved_space; break; }
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
start[bank] = bd->bi_dram[bank].start;size[bank] = bd->bi_dram[bank].size;- }
- return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
- return fdt_fixup_memory_banks(blob, ddrss->ddr_bank_base,
ddrss->ddr_bank_size, CONFIG_NR_DRAM_BANKS);Have we looked into passing this information via the global data pointer rather than fixing up the device tree on each boot phase?
https://patchwork.ozlabs.org/project/uboot/patch/20180926215520.87168-23-sjg@chromium.org/I don't see a clear path to pass GD from SPL to SPL but there is a TPL to SPL which I think we should be able to copy.
But we also make use of this to fix up the device tree that passes onto kernel as well, I need to look into this patch though. But from a first glance looks like we will be able to pass on the information from SPL->SPL (with modification as you said) and SPL->U-Boot but U-Boot->Kernel would still require this function to be present.
Just came across this series again and noticed that this will be a bit tricky with boards detecting RAM at runtime.
Updating the memory node with detected RAM size prio to DDR driver probing should be no problem. But the next stages will need to update u-boot's and Linux memory nodes too. Unfortunately, we do not have the data at this stage which tells us how much space was reserved for ECC.
I agree. We'll need to prototype this out real quick but my goal is to set the gd->ram_size in the R5 SPL that we can then pass GD to each boot phase. What I haven't looked into is what other drivers will break :)
It would be best to do the memory setup for GD or memory node once and have other stages reuse it.
Right now I have no idea how my board should be aware of the reserved ecc memory in u-boot. Any suggestions?
I don't have a solution to this either. My only thought would be to look at the controller to see if it has ECC enabled and take 8/9s of the total capacity.
I'm not a huge fan of that as well as it's kinda opaque. It's much more clean to let the R5 SPL do this and pass the info along all the way to the kernel.
~Bryan
Hi, Bryan,
On 23/10/24 20:09, Bryan Brattlof wrote:
On October 21, 2024 thus sayeth Santhosh Kumar K:
As R5 is a 32 bit processor, the RAM banks' base and size calculation is restricted to 32 bits, which results in wrong values if bank's base is greater than 32 bits or bank's size is greater than or equal to 4GB.
So, add k3_ddrss_ddr_bank_base_size_calc() to get the base address and size of RAM's banks from the device tree memory node, and store in a 64 bit device private data which can be used for ECC reserved memory calculation, Setting ECC range and Fixing up bank size in device tree when ECC is enabled.
Signed-off-by: Santhosh Kumar K s-k6@ti.com
drivers/ram/k3-ddrss/k3-ddrss.c | 72 ++++++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 15 deletions(-)
diff --git a/drivers/ram/k3-ddrss/k3-ddrss.c b/drivers/ram/k3-ddrss/k3-ddrss.c index f2ab8cf2b49b..bd6783ebc2cd 100644 --- a/drivers/ram/k3-ddrss/k3-ddrss.c +++ b/drivers/ram/k3-ddrss/k3-ddrss.c @@ -147,6 +147,9 @@ struct k3_ddrss_desc { struct k3_ddrss_ecc_region ecc_regions[K3_DDRSS_MAX_ECC_REGIONS]; u64 ecc_reserved_space; bool ti_ecc_enabled;
- unsigned long long ddr_bank_base[CONFIG_NR_DRAM_BANKS];
- unsigned long long ddr_bank_size[CONFIG_NR_DRAM_BANKS];
- unsigned long long ddr_ram_size;
Should we use the u64 typedef here?
Yes, I'll replace.
};
struct reginitdata { @@ -668,11 +671,54 @@ static void k3_ddrss_lpddr4_preload_full_mem(struct k3_ddrss_desc *ddrss, printf("ECC: priming DDR completed in %lu msec\n", get_timer(done)); }
+static void k3_ddrss_ddr_bank_base_size_calc(struct k3_ddrss_desc *ddrss) +{
- int bank, na, ns, len, parent;
- const fdt32_t *ptr, *end;
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
ddrss->ddr_bank_base[bank] = 0;ddrss->ddr_bank_size[bank] = 0;- }
- ofnode mem = ofnode_null();
- do {
mem = ofnode_by_prop_value(mem, "device_type", "memory", 7);- } while (!ofnode_is_enabled(mem));
- const void *fdt = ofnode_to_fdt(mem);
- int node = ofnode_to_offset(mem);
- const char *property = "reg";
- parent = fdt_parent_offset(fdt, node);
- na = fdt_address_cells(fdt, parent);
- ns = fdt_size_cells(fdt, parent);
- ptr = fdt_getprop(fdt, node, property, &len);
- end = ptr + len / sizeof(*ptr);
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
if (ptr + na + ns <= end) {if (CONFIG_IS_ENABLED(OF_TRANSLATE))ddrss->ddr_bank_base[bank] = fdt_translate_address(fdt, node, ptr);elseddrss->ddr_bank_base[bank] = fdtdec_get_number(ptr, na);ddrss->ddr_bank_size[bank] = fdtdec_get_number(&ptr[na], ns);}ptr += na + ns;- }
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++)
ddrss->ddr_ram_size += ddrss->ddr_bank_size[bank];+}
- static void k3_ddrss_lpddr4_ecc_calc_reserved_mem(struct k3_ddrss_desc *ddrss) { fdtdec_setup_mem_size_base_lowest();
- ddrss->ecc_reserved_space = gd->ram_size;
ddrss->ecc_reserved_space = ddrss->ddr_ram_size; do_div(ddrss->ecc_reserved_space, 9);
/* Round to clean number */
@@ -696,7 +742,7 @@ static void k3_ddrss_lpddr4_ecc_init(struct k3_ddrss_desc *ddrss) /* Preload the full memory with 0's using the BIST engine of * the LPDDR4 controller. */
- k3_ddrss_lpddr4_preload_full_mem(ddrss, gd->ram_size, 0);
k3_ddrss_lpddr4_preload_full_mem(ddrss, ddrss->ddr_ram_size, 0);
/* Clear Error Count Register */ writel(0x1, base + DDRSS_ECC_1B_ERR_CNT_REG);
@@ -741,6 +787,8 @@ static int k3_ddrss_probe(struct udevice *dev)
k3_lpddr4_start(ddrss);
- k3_ddrss_ddr_bank_base_size_calc(ddrss);
- if (ddrss->ti_ecc_enabled) { if (!ddrss->ddrss_ss_cfg) { printf("%s: ss_cfg is required if ecc is enabled but not provided.",
@@ -761,30 +809,24 @@ static int k3_ddrss_probe(struct udevice *dev)
int k3_ddrss_ddr_fdt_fixup(struct udevice *dev, void *blob, struct bd_info *bd) {
- struct k3_ddrss_desc *ddrss = dev_get_priv(dev);
- u64 start[CONFIG_NR_DRAM_BANKS];
- u64 size[CONFIG_NR_DRAM_BANKS]; int bank;
struct k3_ddrss_desc *ddrss = dev_get_priv(dev);
if (ddrss->ecc_reserved_space == 0) return 0;
for (bank = CONFIG_NR_DRAM_BANKS - 1; bank >= 0; bank--) {
if (ddrss->ecc_reserved_space > bd->bi_dram[bank].size) {ddrss->ecc_reserved_space -= bd->bi_dram[bank].size;bd->bi_dram[bank].size = 0;
if (ddrss->ecc_reserved_space > ddrss->ddr_bank_size[bank]) {ddrss->ecc_reserved_space -= ddrss->ddr_bank_size[bank];ddrss->ddr_bank_size[bank] = 0;
bd->bi_dram[bank].size -= ddrss->ecc_reserved_space;
ddrss->ddr_bank_size[bank] -= ddrss->ecc_reserved_space; break;
- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
start[bank] = bd->bi_dram[bank].start;size[bank] = bd->bi_dram[bank].size;- }
- return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
- return fdt_fixup_memory_banks(blob, ddrss->ddr_bank_base,
ddrss->ddr_bank_size, CONFIG_NR_DRAM_BANKS);Have we looked into passing this information via the global data pointer rather than fixing up the device tree on each boot phase?
https://patchwork.ozlabs.org/project/uboot/patch/20180926215520.87168-23-sjg@chromium.org/I don't see a clear path to pass GD from SPL to SPL but there is a TPL to SPL which I think we should be able to copy.
At R5, the phys_addr_t is 32 bits - so, can't store more than 4GB in global data pointer.
Even in the above series you shared, handoff_save_dram() saves the RAM and banks' size from global data only.
Hence, the fix-up is necessary to pass information from R5 SPL to A53 SPL.
Regards, Santhosh.
~Bryan
Setup the ECC region's start and range using the device private data, ddrss->ddr_bank_base[0] and ddrss->ddr_ram_size. Also, move start and range of ECC regions from 32 bits to 64 bits to accommodate for DDR greater than or equal to 4GB.
Signed-off-by: Santhosh Kumar K s-k6@ti.com --- drivers/ram/k3-ddrss/k3-ddrss.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/ram/k3-ddrss/k3-ddrss.c b/drivers/ram/k3-ddrss/k3-ddrss.c index bd6783ebc2cd..66b7f31f4f64 100644 --- a/drivers/ram/k3-ddrss/k3-ddrss.c +++ b/drivers/ram/k3-ddrss/k3-ddrss.c @@ -121,8 +121,8 @@ struct k3_msmc { #define K3_DDRSS_MAX_ECC_REGIONS 3
struct k3_ddrss_ecc_region { - u32 start; - u32 range; + u64 start; + u64 range; };
struct k3_ddrss_desc { @@ -551,7 +551,7 @@ void k3_lpddr4_start(struct k3_ddrss_desc *ddrss) } }
-static void k3_ddrss_set_ecc_range_r0(u32 base, u32 start_address, u32 size) +static void k3_ddrss_set_ecc_range_r0(u32 base, u64 start_address, u64 size) { writel((start_address) >> 16, base + DDRSS_ECC_R0_STR_ADDR_REG); writel((start_address + size - 1) >> 16, base + DDRSS_ECC_R0_END_ADDR_REG); @@ -727,13 +727,13 @@ static void k3_ddrss_lpddr4_ecc_calc_reserved_mem(struct k3_ddrss_desc *ddrss)
static void k3_ddrss_lpddr4_ecc_init(struct k3_ddrss_desc *ddrss) { - u32 ecc_region_start = ddrss->ecc_regions[0].start; - u32 ecc_range = ddrss->ecc_regions[0].range; + u64 ecc_region_start = ddrss->ecc_regions[0].start; + u64 ecc_range = ddrss->ecc_regions[0].range; u32 base = (u32)ddrss->ddrss_ss_cfg; u32 val;
/* Only Program region 0 which covers full ddr space */ - k3_ddrss_set_ecc_range_r0(base, ecc_region_start - gd->ram_base, ecc_range); + k3_ddrss_set_ecc_range_r0(base, ecc_region_start - ddrss->ddr_bank_base[0], ecc_range);
/* Enable ECC, RMW, WR_ALLOC */ writel(DDRSS_ECC_CTRL_REG_ECC_EN | DDRSS_ECC_CTRL_REG_RMW_EN | @@ -799,8 +799,8 @@ static int k3_ddrss_probe(struct udevice *dev) k3_ddrss_lpddr4_ecc_calc_reserved_mem(ddrss);
/* Always configure one region that covers full DDR space */ - ddrss->ecc_regions[0].start = gd->ram_base; - ddrss->ecc_regions[0].range = gd->ram_size - ddrss->ecc_reserved_space; + ddrss->ecc_regions[0].start = ddrss->ddr_bank_base[0]; + ddrss->ecc_regions[0].range = ddrss->ddr_ram_size - ddrss->ecc_reserved_space; k3_ddrss_lpddr4_ecc_init(ddrss); }
Enable ECC 1-bit error, 2-bit error, multiple 1-bit error interrupts by setting the respective bits in the DDRSS_V2A_INT_SET_REG register.
Signed-off-by: Santhosh Kumar K s-k6@ti.com --- drivers/ram/k3-ddrss/k3-ddrss.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/ram/k3-ddrss/k3-ddrss.c b/drivers/ram/k3-ddrss/k3-ddrss.c index 66b7f31f4f64..9f1a3530c704 100644 --- a/drivers/ram/k3-ddrss/k3-ddrss.c +++ b/drivers/ram/k3-ddrss/k3-ddrss.c @@ -45,6 +45,11 @@ #define DDRSS_ECC_R2_STR_ADDR_REG 0x0140 #define DDRSS_ECC_R2_END_ADDR_REG 0x0144 #define DDRSS_ECC_1B_ERR_CNT_REG 0x0150 +#define DDRSS_V2A_INT_SET_REG 0x00a8 + +#define DDRSS_V2A_INT_SET_REG_ECC1BERR_EN BIT(3) +#define DDRSS_V2A_INT_SET_REG_ECC2BERR_EN BIT(4) +#define DDRSS_V2A_INT_SET_REG_ECCM1BERR_EN BIT(5)
#define SINGLE_DDR_SUBSYSTEM 0x1 #define MULTI_DDR_SUBSYSTEM 0x2 @@ -747,6 +752,9 @@ static void k3_ddrss_lpddr4_ecc_init(struct k3_ddrss_desc *ddrss) /* Clear Error Count Register */ writel(0x1, base + DDRSS_ECC_1B_ERR_CNT_REG);
+ writel(DDRSS_V2A_INT_SET_REG_ECC1BERR_EN | DDRSS_V2A_INT_SET_REG_ECC2BERR_EN | + DDRSS_V2A_INT_SET_REG_ECCM1BERR_EN, base + DDRSS_V2A_INT_SET_REG); + /* Enable ECC Check */ val = readl(base + DDRSS_ECC_CTRL_REG); val |= DDRSS_ECC_CTRL_REG_ECC_CK;
From: Neha Malcom Francis n-francis@ti.com
Add CONFIG_K3_INLINE_ECC so that ECC functions can be compiled into R5 SPL only when the config has been enabled.
Signed-off-by: Neha Malcom Francis n-francis@ti.com --- drivers/ram/Kconfig | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/ram/Kconfig b/drivers/ram/Kconfig index f7e357f24da7..533d0c629a64 100644 --- a/drivers/ram/Kconfig +++ b/drivers/ram/Kconfig @@ -116,6 +116,16 @@ config IMXRT_SDRAM to support external memories like sdram, psram & nand. This driver is for the sdram memory interface with the SEMC.
+config K3_INLINE_ECC + bool "Enable TI Inline ECC support" + depends on K3_DDRSS + help + Enable Inline ECC support on K3 platforms. 1/9th of the SDRAM space + is used for ECC storage and the rest 8/9th is available for system + use. Enabling ECC increases boot time as the ECC protected regions + need to be primed with a predefined value prior to enabling ECC + check. + source "drivers/ram/aspeed/Kconfig" source "drivers/ram/cadence/Kconfig" source "drivers/ram/octeon/Kconfig"
On October 21, 2024 thus sayeth Santhosh Kumar K:
From: Neha Malcom Francis n-francis@ti.com
Add CONFIG_K3_INLINE_ECC so that ECC functions can be compiled into R5 SPL only when the config has been enabled.
Signed-off-by: Neha Malcom Francis n-francis@ti.com
drivers/ram/Kconfig | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/ram/Kconfig b/drivers/ram/Kconfig index f7e357f24da7..533d0c629a64 100644 --- a/drivers/ram/Kconfig +++ b/drivers/ram/Kconfig @@ -116,6 +116,16 @@ config IMXRT_SDRAM to support external memories like sdram, psram & nand. This driver is for the sdram memory interface with the SEMC.
+config K3_INLINE_ECC
- bool "Enable TI Inline ECC support"
- depends on K3_DDRSS
- help
Enable Inline ECC support on K3 platforms. 1/9th of the SDRAM spaceis used for ECC storage and the rest 8/9th is available for systemuse. Enabling ECC increases boot time as the ECC protected regionsneed to be primed with a predefined value prior to enabling ECCcheck.
I agree size will always be a complaint at such an early stage of boot but we're already adding the ECC configuration in the memory{} node.
We should do one or the other but not both. It would be hard to debug problems if someone added the protected{} node but not enable this config
Hi Bryan
On 23/10/24 20:15, Bryan Brattlof wrote:
On October 21, 2024 thus sayeth Santhosh Kumar K:
From: Neha Malcom Francis n-francis@ti.com
Add CONFIG_K3_INLINE_ECC so that ECC functions can be compiled into R5 SPL only when the config has been enabled.
Signed-off-by: Neha Malcom Francis n-francis@ti.com
drivers/ram/Kconfig | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/ram/Kconfig b/drivers/ram/Kconfig index f7e357f24da7..533d0c629a64 100644 --- a/drivers/ram/Kconfig +++ b/drivers/ram/Kconfig @@ -116,6 +116,16 @@ config IMXRT_SDRAM to support external memories like sdram, psram & nand. This driver is for the sdram memory interface with the SEMC.
+config K3_INLINE_ECC
- bool "Enable TI Inline ECC support"
- depends on K3_DDRSS
- help
Enable Inline ECC support on K3 platforms. 1/9th of the SDRAM spaceis used for ECC storage and the rest 8/9th is available for systemuse. Enabling ECC increases boot time as the ECC protected regionsneed to be primed with a predefined value prior to enabling ECCcheck.I agree size will always be a complaint at such an early stage of boot but we're already adding the ECC configuration in the memory{} node.
We should do one or the other but not both. It would be hard to debug problems if someone added the protected{} node but not enable this config
Actually I do agree with this, maybe we can get rid of the flag ti,ecc-enable which is redundant. In multi-DDR systems, it was still a possibility that we could use it to select the DDR we want ECC enabled on, but that hasn't been tested nor does it make any sense since they'll be interleaved anyways.
Hi, Bryan and Neha,
On 24/10/24 09:33, Neha Malcom Francis wrote:
Hi Bryan
On 23/10/24 20:15, Bryan Brattlof wrote:
On October 21, 2024 thus sayeth Santhosh Kumar K:
From: Neha Malcom Francis n-francis@ti.com
Add CONFIG_K3_INLINE_ECC so that ECC functions can be compiled into R5 SPL only when the config has been enabled.
Signed-off-by: Neha Malcom Francis n-francis@ti.com
drivers/ram/Kconfig | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/ram/Kconfig b/drivers/ram/Kconfig index f7e357f24da7..533d0c629a64 100644 --- a/drivers/ram/Kconfig +++ b/drivers/ram/Kconfig @@ -116,6 +116,16 @@ config IMXRT_SDRAM to support external memories like sdram, psram & nand. This driver is for the sdram memory interface with the SEMC. +config K3_INLINE_ECC + bool "Enable TI Inline ECC support" + depends on K3_DDRSS + help + Enable Inline ECC support on K3 platforms. 1/9th of the SDRAM space + is used for ECC storage and the rest 8/9th is available for system + use. Enabling ECC increases boot time as the ECC protected regions + need to be primed with a predefined value prior to enabling ECC + check.
I agree size will always be a complaint at such an early stage of boot but we're already adding the ECC configuration in the memory{} node.
We should do one or the other but not both. It would be hard to debug problems if someone added the protected{} node but not enable this config
Actually I do agree with this, maybe we can get rid of the flag ti,ecc-enable which is redundant. In multi-DDR systems, it was still a possibility that we could use it to select the DDR we want ECC enabled on, but that hasn't been tested nor does it make any sense since they'll be interleaved anyways.
Yes, we can get rid of 'ti,ecc-enable' flag and use only K3_INLINE_ECC config for enabling ECC.
Regards, Santhosh.
From: Neha Malcom Francis n-francis@ti.com
Set CONFIG_NR_DRAM_BANKS to 2 as we have two banks described in the memory/ node for lower and higher addressible DDR regions.
This allows use of FDT functions from fdt_support.c to set up and fixup the memory/ node correctly.
Signed-off-by: Neha Malcom Francis n-francis@ti.com --- configs/j7200_evm_r5_defconfig | 1 + configs/j721e_evm_r5_defconfig | 1 + configs/j721s2_evm_r5_defconfig | 1 + configs/j784s4_evm_r5_defconfig | 1 + 4 files changed, 4 insertions(+)
diff --git a/configs/j7200_evm_r5_defconfig b/configs/j7200_evm_r5_defconfig index 774a9eff439d..78dbd44f845f 100644 --- a/configs/j7200_evm_r5_defconfig +++ b/configs/j7200_evm_r5_defconfig @@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x70000 CONFIG_SPL_GPIO=y CONFIG_SPL_LIBCOMMON_SUPPORT=y CONFIG_SPL_LIBGENERIC_SUPPORT=y +CONFIG_NR_DRAM_BANKS=2 CONFIG_SOC_K3_J721E=y CONFIG_K3_EARLY_CONS=y CONFIG_TARGET_J7200_R5_EVM=y diff --git a/configs/j721e_evm_r5_defconfig b/configs/j721e_evm_r5_defconfig index 3b4a7c3c0eeb..dc391f14b193 100644 --- a/configs/j721e_evm_r5_defconfig +++ b/configs/j721e_evm_r5_defconfig @@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x70000 CONFIG_SPL_GPIO=y CONFIG_SPL_LIBCOMMON_SUPPORT=y CONFIG_SPL_LIBGENERIC_SUPPORT=y +CONFIG_NR_DRAM_BANKS=2 CONFIG_SOC_K3_J721E=y CONFIG_K3_EARLY_CONS=y CONFIG_K3_QOS=y diff --git a/configs/j721s2_evm_r5_defconfig b/configs/j721s2_evm_r5_defconfig index b6adb6a77d7e..374bc283be2d 100644 --- a/configs/j721s2_evm_r5_defconfig +++ b/configs/j721s2_evm_r5_defconfig @@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x10000 CONFIG_SPL_GPIO=y CONFIG_SPL_LIBCOMMON_SUPPORT=y CONFIG_SPL_LIBGENERIC_SUPPORT=y +CONFIG_NR_DRAM_BANKS=2 CONFIG_SOC_K3_J721S2=y CONFIG_K3_EARLY_CONS=y CONFIG_K3_QOS=y diff --git a/configs/j784s4_evm_r5_defconfig b/configs/j784s4_evm_r5_defconfig index a11688194977..6fe36ab38f02 100644 --- a/configs/j784s4_evm_r5_defconfig +++ b/configs/j784s4_evm_r5_defconfig @@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x10000 CONFIG_SPL_GPIO=y CONFIG_SPL_LIBCOMMON_SUPPORT=y CONFIG_SPL_LIBGENERIC_SUPPORT=y +CONFIG_NR_DRAM_BANKS=2 CONFIG_SOC_K3_J784S4=y CONFIG_K3_EARLY_CONS=y CONFIG_K3_QOS=y
On October 21, 2024 thus sayeth Santhosh Kumar K:
From: Neha Malcom Francis n-francis@ti.com
Set CONFIG_NR_DRAM_BANKS to 2 as we have two banks described in the memory/ node for lower and higher addressible DDR regions.
This allows use of FDT functions from fdt_support.c to set up and fixup the memory/ node correctly.
Signed-off-by: Neha Malcom Francis n-francis@ti.com
configs/j7200_evm_r5_defconfig | 1 + configs/j721e_evm_r5_defconfig | 1 + configs/j721s2_evm_r5_defconfig | 1 + configs/j784s4_evm_r5_defconfig | 1 + 4 files changed, 4 insertions(+)
diff --git a/configs/j7200_evm_r5_defconfig b/configs/j7200_evm_r5_defconfig index 774a9eff439d..78dbd44f845f 100644 --- a/configs/j7200_evm_r5_defconfig +++ b/configs/j7200_evm_r5_defconfig @@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x70000 CONFIG_SPL_GPIO=y CONFIG_SPL_LIBCOMMON_SUPPORT=y CONFIG_SPL_LIBGENERIC_SUPPORT=y +CONFIG_NR_DRAM_BANKS=2
Should these go into Kconfig somewhere? These boards will always have 2 DRAAM banks unless someone is artificially limiting DDR sizes.
CONFIG_SOC_K3_J721E=y CONFIG_K3_EARLY_CONS=y CONFIG_TARGET_J7200_R5_EVM=y diff --git a/configs/j721e_evm_r5_defconfig b/configs/j721e_evm_r5_defconfig index 3b4a7c3c0eeb..dc391f14b193 100644 --- a/configs/j721e_evm_r5_defconfig +++ b/configs/j721e_evm_r5_defconfig @@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x70000 CONFIG_SPL_GPIO=y CONFIG_SPL_LIBCOMMON_SUPPORT=y CONFIG_SPL_LIBGENERIC_SUPPORT=y +CONFIG_NR_DRAM_BANKS=2 CONFIG_SOC_K3_J721E=y CONFIG_K3_EARLY_CONS=y CONFIG_K3_QOS=y diff --git a/configs/j721s2_evm_r5_defconfig b/configs/j721s2_evm_r5_defconfig index b6adb6a77d7e..374bc283be2d 100644 --- a/configs/j721s2_evm_r5_defconfig +++ b/configs/j721s2_evm_r5_defconfig @@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x10000 CONFIG_SPL_GPIO=y CONFIG_SPL_LIBCOMMON_SUPPORT=y CONFIG_SPL_LIBGENERIC_SUPPORT=y +CONFIG_NR_DRAM_BANKS=2 CONFIG_SOC_K3_J721S2=y CONFIG_K3_EARLY_CONS=y CONFIG_K3_QOS=y diff --git a/configs/j784s4_evm_r5_defconfig b/configs/j784s4_evm_r5_defconfig index a11688194977..6fe36ab38f02 100644 --- a/configs/j784s4_evm_r5_defconfig +++ b/configs/j784s4_evm_r5_defconfig @@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x10000 CONFIG_SPL_GPIO=y CONFIG_SPL_LIBCOMMON_SUPPORT=y CONFIG_SPL_LIBGENERIC_SUPPORT=y +CONFIG_NR_DRAM_BANKS=2 CONFIG_SOC_K3_J784S4=y CONFIG_K3_EARLY_CONS=y CONFIG_K3_QOS=y -- 2.34.1
Hi, Bryan,
On 23/10/24 20:18, Bryan Brattlof wrote:
On October 21, 2024 thus sayeth Santhosh Kumar K:
From: Neha Malcom Francis n-francis@ti.com
Set CONFIG_NR_DRAM_BANKS to 2 as we have two banks described in the memory/ node for lower and higher addressible DDR regions.
This allows use of FDT functions from fdt_support.c to set up and fixup the memory/ node correctly.
Signed-off-by: Neha Malcom Francis n-francis@ti.com
configs/j7200_evm_r5_defconfig | 1 + configs/j721e_evm_r5_defconfig | 1 + configs/j721s2_evm_r5_defconfig | 1 + configs/j784s4_evm_r5_defconfig | 1 + 4 files changed, 4 insertions(+)
diff --git a/configs/j7200_evm_r5_defconfig b/configs/j7200_evm_r5_defconfig index 774a9eff439d..78dbd44f845f 100644 --- a/configs/j7200_evm_r5_defconfig +++ b/configs/j7200_evm_r5_defconfig @@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x70000 CONFIG_SPL_GPIO=y CONFIG_SPL_LIBCOMMON_SUPPORT=y CONFIG_SPL_LIBGENERIC_SUPPORT=y +CONFIG_NR_DRAM_BANKS=2
Should these go into Kconfig somewhere? These boards will always have 2 DRAAM banks unless someone is artificially limiting DDR sizes.
Thanks for the suggestion. We can make it default at Kconfig.
Regards, Santhosh.
CONFIG_SOC_K3_J721E=y CONFIG_K3_EARLY_CONS=y CONFIG_TARGET_J7200_R5_EVM=y diff --git a/configs/j721e_evm_r5_defconfig b/configs/j721e_evm_r5_defconfig index 3b4a7c3c0eeb..dc391f14b193 100644 --- a/configs/j721e_evm_r5_defconfig +++ b/configs/j721e_evm_r5_defconfig @@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x70000 CONFIG_SPL_GPIO=y CONFIG_SPL_LIBCOMMON_SUPPORT=y CONFIG_SPL_LIBGENERIC_SUPPORT=y +CONFIG_NR_DRAM_BANKS=2 CONFIG_SOC_K3_J721E=y CONFIG_K3_EARLY_CONS=y CONFIG_K3_QOS=y diff --git a/configs/j721s2_evm_r5_defconfig b/configs/j721s2_evm_r5_defconfig index b6adb6a77d7e..374bc283be2d 100644 --- a/configs/j721s2_evm_r5_defconfig +++ b/configs/j721s2_evm_r5_defconfig @@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x10000 CONFIG_SPL_GPIO=y CONFIG_SPL_LIBCOMMON_SUPPORT=y CONFIG_SPL_LIBGENERIC_SUPPORT=y +CONFIG_NR_DRAM_BANKS=2 CONFIG_SOC_K3_J721S2=y CONFIG_K3_EARLY_CONS=y CONFIG_K3_QOS=y diff --git a/configs/j784s4_evm_r5_defconfig b/configs/j784s4_evm_r5_defconfig index a11688194977..6fe36ab38f02 100644 --- a/configs/j784s4_evm_r5_defconfig +++ b/configs/j784s4_evm_r5_defconfig @@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x10000 CONFIG_SPL_GPIO=y CONFIG_SPL_LIBCOMMON_SUPPORT=y CONFIG_SPL_LIBGENERIC_SUPPORT=y +CONFIG_NR_DRAM_BANKS=2 CONFIG_SOC_K3_J784S4=y CONFIG_K3_EARLY_CONS=y CONFIG_K3_QOS=y -- 2.34.1
As there are few redundant functions in board/ti/*/evm.c files, pull them to a common location of access to reuse and include the common file to access the functions.
Call k3-ddrss driver through fixup_ddr_driver_for_ecc() to fixup the device tree and resize the available amount of DDR, if ECC is enabled. Otherwise, fixup the device tree using the regular fdt_fixup_memory_banks().
Also call dram_init_banksize() after every call to fixup_ddr_driver_for_ecc() is made so that gd->bd is populated correctly.
Ensure that fixup_ddr_driver_for_ecc() is agnostic to the number of DDR controllers present.
Signed-off-by: Santhosh Kumar K s-k6@ti.com Signed-off-by: Neha Malcom Francis n-francis@ti.com --- arch/arm/mach-k3/Makefile | 2 +- arch/arm/mach-k3/include/mach/k3-ddr.h | 15 +++++ arch/arm/mach-k3/k3-ddr.c | 88 ++++++++++++++++++++++++++ board/ti/am62ax/evm.c | 17 +++-- board/ti/am62px/evm.c | 17 +++-- board/ti/am62x/evm.c | 62 ++---------------- board/ti/am64x/evm.c | 73 ++------------------- board/ti/am65x/evm.c | 29 +-------- board/ti/j721e/evm.c | 29 +-------- board/ti/j721s2/evm.c | 35 +++------- board/ti/j784s4/evm.c | 17 +++-- 11 files changed, 160 insertions(+), 224 deletions(-) create mode 100644 arch/arm/mach-k3/include/mach/k3-ddr.h create mode 100644 arch/arm/mach-k3/k3-ddr.c
diff --git a/arch/arm/mach-k3/Makefile b/arch/arm/mach-k3/Makefile index 8c4f6786a5b5..5ce7fc62d80c 100644 --- a/arch/arm/mach-k3/Makefile +++ b/arch/arm/mach-k3/Makefile @@ -6,7 +6,7 @@ obj-$(CONFIG_ARM64) += arm64/ obj-$(CONFIG_CPU_V7R) += r5/ obj-$(CONFIG_OF_LIBFDT) += common_fdt.o -obj-y += common.o security.o +obj-y += common.o security.o k3-ddr.o obj-$(CONFIG_SOC_K3_AM62A7) += am62ax/ obj-$(CONFIG_SOC_K3_AM62P5) += am62px/ obj-$(CONFIG_SOC_K3_AM625) += am62x/ diff --git a/arch/arm/mach-k3/include/mach/k3-ddr.h b/arch/arm/mach-k3/include/mach/k3-ddr.h new file mode 100644 index 000000000000..754e9ba29dac --- /dev/null +++ b/arch/arm/mach-k3/include/mach/k3-ddr.h @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2024, Texas Instruments Incorporated - https://www.ti.com/ + */ + +#ifndef _K3_DDR_H_ +#define _K3_DDR_H_ + +int dram_init(void); +int dram_init_banksize(void); + +void fixup_ddr_driver_for_ecc(struct spl_image_info *spl_image); +void fixup_memory_node(struct spl_image_info *spl_image); + +#endif /* _K3_DDR_H_ */ diff --git a/arch/arm/mach-k3/k3-ddr.c b/arch/arm/mach-k3/k3-ddr.c new file mode 100644 index 000000000000..2383d061e93e --- /dev/null +++ b/arch/arm/mach-k3/k3-ddr.c @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2024, Texas Instruments Incorporated - https://www.ti.com/ + */ + +#include <fdt_support.h> +#include <dm/uclass.h> +#include <k3-ddrss.h> +#include <spl.h> + +#include <asm/arch/k3-ddr.h> + +int dram_init(void) +{ + s32 ret; + + ret = fdtdec_setup_mem_size_base_lowest(); + if (ret) + printf("Error setting up mem size and base. %d\n", ret); + + return ret; +} + +int dram_init_banksize(void) +{ + s32 ret; + + ret = fdtdec_setup_memory_banksize(); + if (ret) + printf("Error setting up memory banksize. %d\n", ret); + + return ret; +} + +#if defined(CONFIG_SPL_BUILD) + +void fixup_ddr_driver_for_ecc(struct spl_image_info *spl_image) +{ + struct udevice *dev; + int ret, ctr = 1; + + dram_init_banksize(); + + ret = uclass_get_device(UCLASS_RAM, 0, &dev); + if (ret) + panic("Cannnot get RAM device for ddr size fixup: %d\n", ret); + + ret = k3_ddrss_ddr_fdt_fixup(dev, spl_image->fdt_addr, gd->bd); + if (ret) + printf("Error fixing up ddr node for ECC use! %d\n", ret); + + ret = uclass_next_device_err(&dev); + + while (ret && ret != -ENODEV) + { + ret = k3_ddrss_ddr_fdt_fixup(dev, spl_image->fdt_addr, gd->bd); + if (ret) + printf("Error fixing up ddr node %d for ECC use! %d\n", ctr, ret); + + ret = uclass_next_device_err(&dev); + ctr++; + } +} + +void fixup_memory_node(struct spl_image_info *spl_image) +{ + u64 start[CONFIG_NR_DRAM_BANKS]; + u64 size[CONFIG_NR_DRAM_BANKS]; + int bank; + int ret; + + dram_init(); + dram_init_banksize(); + + for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) + { + start[bank] = gd->bd->bi_dram[bank].start; + size[bank] = gd->bd->bi_dram[bank].size; + } + + ret = fdt_fixup_memory_banks(spl_image->fdt_addr, start, size, + CONFIG_NR_DRAM_BANKS); + + if (ret) + printf("Error fixing up memory node! %d\n", ret); +} + +#endif diff --git a/board/ti/am62ax/evm.c b/board/ti/am62ax/evm.c index 62d3664936e7..ce359be6eaa2 100644 --- a/board/ti/am62ax/evm.c +++ b/board/ti/am62ax/evm.c @@ -12,6 +12,7 @@ #include <env.h> #include <fdt_support.h> #include <spl.h> +#include <asm/arch/k3-ddr.h>
#include "../common/fdt_ops.h"
@@ -20,15 +21,17 @@ int board_init(void) return 0; }
-int dram_init(void) +#if defined(CONFIG_SPL_BUILD) +void spl_perform_fixups(struct spl_image_info *spl_image) { - return fdtdec_setup_mem_size_base(); -} - -int dram_init_banksize(void) -{ - return fdtdec_setup_memory_banksize(); + if (IS_ENABLED(CONFIG_K3_DDRSS)) { + if (IS_ENABLED(CONFIG_K3_INLINE_ECC)) + fixup_ddr_driver_for_ecc(spl_image); + } else { + fixup_memory_node(spl_image); + } } +#endif
#ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void) diff --git a/board/ti/am62px/evm.c b/board/ti/am62px/evm.c index 7362fa4520a3..e85bac40a910 100644 --- a/board/ti/am62px/evm.c +++ b/board/ti/am62px/evm.c @@ -13,6 +13,7 @@ #include <env.h> #include <fdt_support.h> #include <spl.h> +#include <asm/arch/k3-ddr.h> #include "../common/fdt_ops.h"
struct efi_fw_image fw_images[] = { @@ -53,15 +54,17 @@ int board_init(void) return 0; }
-int dram_init(void) +#if defined(CONFIG_SPL_BUILD) +void spl_perform_fixups(struct spl_image_info *spl_image) { - return fdtdec_setup_mem_size_base(); -} - -int dram_init_banksize(void) -{ - return fdtdec_setup_memory_banksize(); + if (IS_ENABLED(CONFIG_K3_DDRSS)) { + if (IS_ENABLED(CONFIG_K3_INLINE_ECC)) + fixup_ddr_driver_for_ecc(spl_image); + } else { + fixup_memory_node(spl_image); + } } +#endif
#if IS_ENABLED(CONFIG_BOARD_LATE_INIT) int board_late_init(void) diff --git a/board/ti/am62x/evm.c b/board/ti/am62x/evm.c index 9bdd0223cdb6..3dd8d1722a1a 100644 --- a/board/ti/am62x/evm.c +++ b/board/ti/am62x/evm.c @@ -19,6 +19,7 @@ #include <asm/io.h> #include <asm/arch/hardware.h> #include <dm/uclass.h> +#include <asm/arch/k3-ddr.h>
#include "../common/fdt_ops.h"
@@ -85,11 +86,6 @@ int board_init(void) return 0; }
-int dram_init(void) -{ - return fdtdec_setup_mem_size_base(); -} - #ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void) { @@ -98,11 +94,6 @@ int board_late_init(void) } #endif
-int dram_init_banksize(void) -{ - return fdtdec_setup_memory_banksize(); -} - #if defined(CONFIG_SPL_BUILD)
void spl_board_init(void) @@ -113,52 +104,13 @@ void spl_board_init(void)
}
-#if defined(CONFIG_K3_AM64_DDRSS) -static void fixup_ddr_driver_for_ecc(struct spl_image_info *spl_image) -{ - struct udevice *dev; - int ret; - - dram_init_banksize(); - - ret = uclass_get_device(UCLASS_RAM, 0, &dev); - if (ret) - panic("Cannot get RAM device for ddr size fixup: %d\n", ret); - - ret = k3_ddrss_ddr_fdt_fixup(dev, spl_image->fdt_addr, gd->bd); - if (ret) - printf("Error fixing up ddr node for ECC use! %d\n", ret); -} -#else -static void fixup_memory_node(struct spl_image_info *spl_image) -{ - u64 start[CONFIG_NR_DRAM_BANKS]; - u64 size[CONFIG_NR_DRAM_BANKS]; - int bank; - int ret; - - dram_init(); - dram_init_banksize(); - - for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) { - start[bank] = gd->bd->bi_dram[bank].start; - size[bank] = gd->bd->bi_dram[bank].size; - } - - /* dram_init functions use SPL fdt, and we must fixup u-boot fdt */ - ret = fdt_fixup_memory_banks(spl_image->fdt_addr, start, size, - CONFIG_NR_DRAM_BANKS); - if (ret) - printf("Error fixing up memory node! %d\n", ret); -} -#endif - void spl_perform_fixups(struct spl_image_info *spl_image) { -#if defined(CONFIG_K3_AM64_DDRSS) - fixup_ddr_driver_for_ecc(spl_image); -#else - fixup_memory_node(spl_image); -#endif + if (IS_ENABLED(CONFIG_K3_DDRSS)) { + if (IS_ENABLED(CONFIG_K3_INLINE_ECC)) + fixup_ddr_driver_for_ecc(spl_image); + } else { + fixup_memory_node(spl_image); + } } #endif diff --git a/board/ti/am64x/evm.c b/board/ti/am64x/evm.c index 609e5cf6d51f..61ab1714e5dc 100644 --- a/board/ti/am64x/evm.c +++ b/board/ti/am64x/evm.c @@ -15,6 +15,7 @@ #include <fdt_support.h> #include <asm/arch/hardware.h> #include <env.h> +#include <asm/arch/k3-ddr.h>
#include "../common/board_detect.h" #include "../common/fdt_ops.h" @@ -66,28 +67,6 @@ int board_init(void) return 0; }
-int dram_init(void) -{ - s32 ret; - - ret = fdtdec_setup_mem_size_base(); - if (ret) - printf("Error setting up mem size and base. %d\n", ret); - - return ret; -} - -int dram_init_banksize(void) -{ - s32 ret; - - ret = fdtdec_setup_memory_banksize(); - if (ret) - printf("Error setting up memory banksize. %d\n", ret); - - return ret; -} - #if defined(CONFIG_SPL_LOAD_FIT) int board_fit_config_name_match(const char *name) { @@ -132,52 +111,14 @@ static int fixup_usb_boot(const void *fdt_blob) } #endif
-#if defined(CONFIG_K3_AM64_DDRSS) -static void fixup_ddr_driver_for_ecc(struct spl_image_info *spl_image) -{ - struct udevice *dev; - int ret; - - dram_init_banksize(); - - ret = uclass_get_device(UCLASS_RAM, 0, &dev); - if (ret) - panic("Cannot get RAM device for ddr size fixup: %d\n", ret); - - ret = k3_ddrss_ddr_fdt_fixup(dev, spl_image->fdt_addr, gd->bd); - if (ret) - printf("Error fixing up ddr node for ECC use! %d\n", ret); -} -#else -static void fixup_memory_node(struct spl_image_info *spl_image) -{ - u64 start[CONFIG_NR_DRAM_BANKS]; - u64 size[CONFIG_NR_DRAM_BANKS]; - int bank; - int ret; - - dram_init(); - dram_init_banksize(); - - for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) { - start[bank] = gd->bd->bi_dram[bank].start; - size[bank] = gd->bd->bi_dram[bank].size; - } - - /* dram_init functions use SPL fdt, and we must fixup u-boot fdt */ - ret = fdt_fixup_memory_banks(spl_image->fdt_addr, start, size, CONFIG_NR_DRAM_BANKS); - if (ret) - printf("Error fixing up memory node! %d\n", ret); -} -#endif - void spl_perform_fixups(struct spl_image_info *spl_image) { -#if defined(CONFIG_K3_AM64_DDRSS) - fixup_ddr_driver_for_ecc(spl_image); -#else - fixup_memory_node(spl_image); -#endif + if (IS_ENABLED(CONFIG_K3_DDRSS)) { + if (IS_ENABLED(CONFIG_K3_INLINE_ECC)) + fixup_ddr_driver_for_ecc(spl_image); + } else { + fixup_memory_node(spl_image); + }
#if CONFIG_IS_ENABLED(USB_STORAGE) fixup_usb_boot(spl_image->fdt_addr); diff --git a/board/ti/am65x/evm.c b/board/ti/am65x/evm.c index 07073a5940b1..6658794a1375 100644 --- a/board/ti/am65x/evm.c +++ b/board/ti/am65x/evm.c @@ -20,6 +20,7 @@ #include <env.h> #include <spl.h> #include <linux/printk.h> +#include <asm/arch/k3-ddr.h>
#include "../common/board_detect.h" #include "../common/fdt_ops.h" @@ -49,17 +50,6 @@ int board_init(void) return 0; }
-int dram_init(void) -{ -#ifdef CONFIG_PHYS_64BIT - gd->ram_size = 0x100000000; -#else - gd->ram_size = 0x80000000; -#endif - - return 0; -} - phys_addr_t board_get_usable_ram_top(phys_size_t total_size) { #ifdef CONFIG_PHYS_64BIT @@ -71,23 +61,6 @@ phys_addr_t board_get_usable_ram_top(phys_size_t total_size) return gd->ram_top; }
-int dram_init_banksize(void) -{ - /* Bank 0 declares the memory available in the DDR low region */ - gd->bd->bi_dram[0].start = 0x80000000; - gd->bd->bi_dram[0].size = 0x80000000; - gd->ram_size = 0x80000000; - -#ifdef CONFIG_PHYS_64BIT - /* Bank 1 declares the memory available in the DDR high region */ - gd->bd->bi_dram[1].start = 0x880000000; - gd->bd->bi_dram[1].size = 0x80000000; - gd->ram_size = 0x100000000; -#endif - - return 0; -} - #ifdef CONFIG_SPL_LOAD_FIT int board_fit_config_name_match(const char *name) { diff --git a/board/ti/j721e/evm.c b/board/ti/j721e/evm.c index f3452ff0a8fb..d0385aa25973 100644 --- a/board/ti/j721e/evm.c +++ b/board/ti/j721e/evm.c @@ -15,6 +15,7 @@ #include <asm/gpio.h> #include <spl.h> #include <dm.h> +#include <asm/arch/k3-ddr.h>
#include "../common/board_detect.h" #include "../common/fdt_ops.h" @@ -77,17 +78,6 @@ int board_init(void) return 0; }
-int dram_init(void) -{ -#ifdef CONFIG_PHYS_64BIT - gd->ram_size = 0x100000000; -#else - gd->ram_size = 0x80000000; -#endif - - return 0; -} - phys_addr_t board_get_usable_ram_top(phys_size_t total_size) { #ifdef CONFIG_PHYS_64BIT @@ -99,23 +89,6 @@ phys_addr_t board_get_usable_ram_top(phys_size_t total_size) return gd->ram_top; }
-int dram_init_banksize(void) -{ - /* Bank 0 declares the memory available in the DDR low region */ - gd->bd->bi_dram[0].start = 0x80000000; - gd->bd->bi_dram[0].size = 0x80000000; - gd->ram_size = 0x80000000; - -#ifdef CONFIG_PHYS_64BIT - /* Bank 1 declares the memory available in the DDR high region */ - gd->bd->bi_dram[1].start = 0x880000000; - gd->bd->bi_dram[1].size = 0x80000000; - gd->ram_size = 0x100000000; -#endif - - return 0; -} - #ifdef CONFIG_SPL_LOAD_FIT int board_fit_config_name_match(const char *name) { diff --git a/board/ti/j721s2/evm.c b/board/ti/j721s2/evm.c index 5a0281d6b483..be21b0d95da5 100644 --- a/board/ti/j721s2/evm.c +++ b/board/ti/j721s2/evm.c @@ -21,6 +21,7 @@ #include <dm.h> #include <dm/uclass-internal.h> #include <dm/root.h> +#include <asm/arch/k3-ddr.h>
#include "../common/board_detect.h" #include "../common/fdt_ops.h" @@ -32,17 +33,6 @@ int board_init(void) return 0; }
-int dram_init(void) -{ -#ifdef CONFIG_PHYS_64BIT - gd->ram_size = 0x100000000; -#else - gd->ram_size = 0x80000000; -#endif - - return 0; -} - phys_addr_t board_get_usable_ram_top(phys_size_t total_size) { #ifdef CONFIG_PHYS_64BIT @@ -54,22 +44,17 @@ phys_addr_t board_get_usable_ram_top(phys_size_t total_size) return gd->ram_top; }
-int dram_init_banksize(void) +#if defined(CONFIG_SPL_BUILD) +void spl_perform_fixups(struct spl_image_info *spl_image) { - /* Bank 0 declares the memory available in the DDR low region */ - gd->bd->bi_dram[0].start = 0x80000000; - gd->bd->bi_dram[0].size = 0x7fffffff; - gd->ram_size = 0x80000000; - -#ifdef CONFIG_PHYS_64BIT - /* Bank 1 declares the memory available in the DDR high region */ - gd->bd->bi_dram[1].start = 0x880000000; - gd->bd->bi_dram[1].size = 0x37fffffff; - gd->ram_size = 0x400000000; -#endif - - return 0; + if (IS_ENABLED(CONFIG_K3_DDRSS)) { + if (IS_ENABLED(CONFIG_K3_INLINE_ECC)) + fixup_ddr_driver_for_ecc(spl_image); + } else { + fixup_memory_node(spl_image); + } } +#endif
#ifdef CONFIG_TI_I2C_BOARD_DETECT /* diff --git a/board/ti/j784s4/evm.c b/board/ti/j784s4/evm.c index 548dbd5925df..c843c9501588 100644 --- a/board/ti/j784s4/evm.c +++ b/board/ti/j784s4/evm.c @@ -10,6 +10,7 @@ #include <efi_loader.h> #include <init.h> #include <spl.h> +#include <asm/arch/k3-ddr.h> #include "../common/fdt_ops.h"
DECLARE_GLOBAL_DATA_PTR; @@ -52,15 +53,17 @@ int board_init(void) return 0; }
-int dram_init(void) +#if defined(CONFIG_SPL_BUILD) +void spl_perform_fixups(struct spl_image_info *spl_image) { - return fdtdec_setup_mem_size_base(); -} - -int dram_init_banksize(void) -{ - return fdtdec_setup_memory_banksize(); + if (IS_ENABLED(CONFIG_K3_DDRSS)) { + if (IS_ENABLED(CONFIG_K3_INLINE_ECC)) + fixup_ddr_driver_for_ecc(spl_image); + } else { + fixup_memory_node(spl_image); + } } +#endif
#ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void)
Hi Santhosh,
Am 21.10.24 um 06:40 schrieb Santhosh Kumar K:
As there are few redundant functions in board/ti/*/evm.c files, pull them to a common location of access to reuse and include the common file to access the functions.
Call k3-ddrss driver through fixup_ddr_driver_for_ecc() to fixup the device tree and resize the available amount of DDR, if ECC is enabled. Otherwise, fixup the device tree using the regular fdt_fixup_memory_banks().
Also call dram_init_banksize() after every call to fixup_ddr_driver_for_ecc() is made so that gd->bd is populated correctly.
Ensure that fixup_ddr_driver_for_ecc() is agnostic to the number of DDR controllers present.
Signed-off-by: Santhosh Kumar K s-k6@ti.com Signed-off-by: Neha Malcom Francis n-francis@ti.com
<snip>
+obj-y += common.o security.o k3-ddr.o obj-$(CONFIG_SOC_K3_AM62A7) += am62ax/ obj-$(CONFIG_SOC_K3_AM62P5) += am62px/ obj-$(CONFIG_SOC_K3_AM625) += am62x/ diff --git a/arch/arm/mach-k3/include/mach/k3-ddr.h b/arch/arm/mach-k3/include/mach/k3-ddr.h new file mode 100644 index 000000000000..754e9ba29dac --- /dev/null
<snip>
+#include <fdt_support.h> +#include <dm/uclass.h> +#include <k3-ddrss.h> +#include <spl.h>
+#include <asm/arch/k3-ddr.h>
+int dram_init(void) +{
s32 ret;ret = fdtdec_setup_mem_size_base_lowest();if (ret)printf("Error setting up mem size and base. %d\n", ret);return ret;+}
+int dram_init_banksize(void) +{
s32 ret;ret = fdtdec_setup_memory_banksize();if (ret)printf("Error setting up memory banksize. %d\n", ret);return ret;+} +Moving dram_init() and dram_init_banksize() into a generic part forces
every K3 based board to use your implementation.
This also breaks the builds for phycore_am62x, phycore_am64x and also the verdin-am62,
phycore-am62x.c:49: multiple definition of `dram_init' phycore-am62x.c:97: multiple definition of `dram_init_banksize
Regards, Wadim
Hi, Wadim,
On 22/10/24 15:34, Wadim Egorov wrote:
Hi Santhosh,
Am 21.10.24 um 06:40 schrieb Santhosh Kumar K:
As there are few redundant functions in board/ti/*/evm.c files, pull them to a common location of access to reuse and include the common file to access the functions.
Call k3-ddrss driver through fixup_ddr_driver_for_ecc() to fixup the device tree and resize the available amount of DDR, if ECC is enabled. Otherwise, fixup the device tree using the regular fdt_fixup_memory_banks().
Also call dram_init_banksize() after every call to fixup_ddr_driver_for_ecc() is made so that gd->bd is populated correctly.
Ensure that fixup_ddr_driver_for_ecc() is agnostic to the number of DDR controllers present.
Signed-off-by: Santhosh Kumar K s-k6@ti.com Signed-off-by: Neha Malcom Francis n-francis@ti.com
<snip>
+obj-y += common.o security.o k3-ddr.o obj-$(CONFIG_SOC_K3_AM62A7) += am62ax/ obj-$(CONFIG_SOC_K3_AM62P5) += am62px/ obj-$(CONFIG_SOC_K3_AM625) += am62x/ diff --git a/arch/arm/mach-k3/include/mach/k3-ddr.h b/arch/arm/mach-k3/include/mach/k3-ddr.h new file mode 100644 index 000000000000..754e9ba29dac --- /dev/null
<snip>
+#include <fdt_support.h> +#include <dm/uclass.h> +#include <k3-ddrss.h> +#include <spl.h>
+#include <asm/arch/k3-ddr.h>
+int dram_init(void) +{ + s32 ret;
+ ret = fdtdec_setup_mem_size_base_lowest(); + if (ret) + printf("Error setting up mem size and base. %d\n", ret);
+ return ret; +}
+int dram_init_banksize(void) +{ + s32 ret;
+ ret = fdtdec_setup_memory_banksize(); + if (ret) + printf("Error setting up memory banksize. %d\n", ret);
+ return ret; +} +Moving dram_init() and dram_init_banksize() into a generic part forces
every K3 based board to use your implementation.
This also breaks the builds for phycore_am62x, phycore_am64x and also the verdin-am62,
phycore-am62x.c:49: multiple definition of `dram_init' phycore-am62x.c:97: multiple definition of `dram_init_banksize
Oops, that's bad. I'll fix this in the next version.
Regards, Santhosh.
Regards, Wadim
Add ss_cfg memory region which maps the DDRSS configuration region for the memory controller node.
Signed-off-by: Santhosh Kumar K s-k6@ti.com Signed-off-by: Neha Malcom Francis n-francis@ti.com --- arch/arm/dts/k3-am62a-ddr.dtsi | 7 ++++--- arch/arm/dts/k3-j721s2-ddr.dtsi | 12 ++++++++---- arch/arm/dts/k3-j784s4-ddr.dtsi | 24 ++++++++++++++++-------- 3 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/arch/arm/dts/k3-am62a-ddr.dtsi b/arch/arm/dts/k3-am62a-ddr.dtsi index 8629ea45b847..42e41f78505a 100644 --- a/arch/arm/dts/k3-am62a-ddr.dtsi +++ b/arch/arm/dts/k3-am62a-ddr.dtsi @@ -4,11 +4,12 @@ */
/ { - memorycontroller: memory-controller@f308000 { + memorycontroller: memory-controller@f300000 { compatible = "ti,am62a-ddrss"; reg = <0x00 0x0f308000 0x00 0x4000>, - <0x00 0x43014000 0x00 0x100>; - reg-names = "cfg", "ctrl_mmr_lp4"; + <0x00 0x43014000 0x00 0x100>, + <0x00 0x0f300000 0x00 0x200>; + reg-names = "cfg", "ctrl_mmr_lp4", "ss_cfg"; ti,ddr-freq1 = <DDRSS_PLL_FREQUENCY_1>; ti,ddr-freq2 = <DDRSS_PLL_FREQUENCY_2>; ti,ddr-fhs-cnt = <DDRSS_PLL_FHS_CNT>; diff --git a/arch/arm/dts/k3-j721s2-ddr.dtsi b/arch/arm/dts/k3-j721s2-ddr.dtsi index 345e2b84f9e8..9764085163c4 100644 --- a/arch/arm/dts/k3-j721s2-ddr.dtsi +++ b/arch/arm/dts/k3-j721s2-ddr.dtsi @@ -5,6 +5,8 @@
&main_navss { ranges = <0x00 0x00114000 0x00 0x00114000 0x00 0x00000100>, // ctrl_mmr_lpr + <0x00 0x02980000 0x00 0x02980000 0x00 0x00000200>, // ss cfg 0 + <0x00 0x029a0000 0x00 0x029a0000 0x00 0x00000200>, // ss cfg 1 <0x00 0x02990000 0x00 0x02990000 0x00 0x00004000>, // ddr0 cfg <0x00 0x029b0000 0x00 0x029b0000 0x00 0x00004000>, // ddr1 cfg <0x00 0x30000000 0x00 0x30000000 0x00 0x0c400000>; @@ -24,8 +26,9 @@ memorycontroller0: memorycontroller@2990000 { compatible = "ti,j721s2-ddrss"; reg = <0x0 0x02990000 0x0 0x4000>, - <0x0 0x0114000 0x0 0x100>; - reg-names = "cfg", "ctrl_mmr_lp4"; + <0x0 0x0114000 0x0 0x100>, + <0x0 0x02980000 0x0 0x200>; + reg-names = "cfg", "ctrl_mmr_lp4", "ss_cfg"; power-domains = <&k3_pds 138 TI_SCI_PD_SHARED>, <&k3_pds 96 TI_SCI_PD_SHARED>; clocks = <&k3_clks 138 0>, <&k3_clks 43 2>; @@ -2232,8 +2235,9 @@ memorycontroller1: memorycontroller@29b0000 { compatible = "ti,j721s2-ddrss"; reg = <0x0 0x029b0000 0x0 0x4000>, - <0x0 0x0114000 0x0 0x100>; - reg-names = "cfg", "ctrl_mmr_lp4"; + <0x0 0x0114000 0x0 0x100>, + <0x0 0x029a0000 0x0 0x200>; + reg-names = "cfg", "ctrl_mmr_lp4", "ss_cfg"; power-domains = <&k3_pds 139 TI_SCI_PD_SHARED>, <&k3_pds 97 TI_SCI_PD_SHARED>; clocks = <&k3_clks 139 0>, <&k3_clks 43 2>; diff --git a/arch/arm/dts/k3-j784s4-ddr.dtsi b/arch/arm/dts/k3-j784s4-ddr.dtsi index 1c3242b0870c..fc74c539331e 100644 --- a/arch/arm/dts/k3-j784s4-ddr.dtsi +++ b/arch/arm/dts/k3-j784s4-ddr.dtsi @@ -9,6 +9,10 @@ <0x00 0x029b0000 0x00 0x029b0000 0x00 0x00004000>, // ddr1 cfg <0x00 0x029d0000 0x00 0x029d0000 0x00 0x00004000>, // ddr2 cfg <0x00 0x029f0000 0x00 0x029f0000 0x00 0x00004000>, // ddr3 cfg + <0x00 0x02980000 0x00 0x02980000 0x00 0x00000200>, // ss cfg 0 + <0x00 0x029a0000 0x00 0x029a0000 0x00 0x00000200>, // ss cfg 1 + <0x00 0x029c0000 0x00 0x029c0000 0x00 0x00000200>, // ss cfg 2 + <0x00 0x029e0000 0x00 0x029e0000 0x00 0x00000200>, // ss cfg 3 <0x00 0x30000000 0x00 0x30000000 0x00 0x0c400000>;
msmc0: msmc { @@ -26,8 +30,9 @@ memorycontroller0: memorycontroller@2990000 { compatible = "ti,j721s2-ddrss"; reg = <0x0 0x02990000 0x0 0x4000>, - <0x0 0x0114000 0x0 0x100>; - reg-names = "cfg", "ctrl_mmr_lp4"; + <0x0 0x0114000 0x0 0x100>, + <0x0 0x02980000 0x0 0x200>; + reg-names = "cfg", "ctrl_mmr_lp4", "ss_cfg"; power-domains = <&k3_pds 191 TI_SCI_PD_SHARED>, <&k3_pds 131 TI_SCI_PD_SHARED>; clocks = <&k3_clks 191 1>, <&k3_clks 78 2>; @@ -2234,8 +2239,9 @@ memorycontroller1: memorycontroller@29b0000 { compatible = "ti,j721s2-ddrss"; reg = <0x0 0x029b0000 0x0 0x4000>, - <0x0 0x0114000 0x0 0x100>; - reg-names = "cfg", "ctrl_mmr_lp4"; + <0x0 0x0114000 0x0 0x100>, + <0x0 0x029a0000 0x0 0x200>; + reg-names = "cfg", "ctrl_mmr_lp4", "ss_cfg"; power-domains = <&k3_pds 192 TI_SCI_PD_SHARED>, <&k3_pds 132 TI_SCI_PD_SHARED>; clocks = <&k3_clks 192 1>, <&k3_clks 78 2>; @@ -4442,8 +4448,9 @@ memorycontroller2: memorycontroller@29d0000 { compatible = "ti,j721s2-ddrss"; reg = <0x0 0x029d0000 0x0 0x4000>, - <0x0 0x0114000 0x0 0x100>; - reg-names = "cfg", "ctrl_mmr_lp4"; + <0x0 0x0114000 0x0 0x100>, + <0x0 0x029c0000 0x0 0x200>; + reg-names = "cfg", "ctrl_mmr_lp4", "ss_cfg"; power-domains = <&k3_pds 193 TI_SCI_PD_SHARED>, <&k3_pds 133 TI_SCI_PD_SHARED>; clocks = <&k3_clks 193 1>, <&k3_clks 78 2>; @@ -6650,8 +6657,9 @@ memorycontroller3: memorycontroller@29f0000 { compatible = "ti,j721s2-ddrss"; reg = <0x0 0x029f0000 0x0 0x4000>, - <0x0 0x0114000 0x0 0x100>; - reg-names = "cfg", "ctrl_mmr_lp4"; + <0x0 0x0114000 0x0 0x100>, + <0x0 0x29e0000 0x0 0x200>; + reg-names = "cfg", "ctrl_mmr_lp4", "ss_cfg"; power-domains = <&k3_pds 194 TI_SCI_PD_SHARED>, <&k3_pds 139 TI_SCI_PD_SHARED>; clocks = <&k3_clks 194 1>, <&k3_clks 78 2>;
Hi Santhosh
On 21/10/24 10:10, Santhosh Kumar K wrote:
Add ss_cfg memory region which maps the DDRSS configuration region for the memory controller node.
Signed-off-by: Santhosh Kumar K s-k6@ti.com Signed-off-by: Neha Malcom Francis n-francis@ti.com
arch/arm/dts/k3-am62a-ddr.dtsi | 7 ++++--- arch/arm/dts/k3-j721s2-ddr.dtsi | 12 ++++++++---- arch/arm/dts/k3-j784s4-ddr.dtsi | 24 ++++++++++++++++-------- 3 files changed, 28 insertions(+), 15 deletions(-)
Thank you for this series. Maybe I missed something, but why is this DT patch at the very end of the series? Doesn't it make sense to include it before the patches triggering inline ECC i.e. patch 7/8?
Sanity tested inline ECC on j784s4 and j721s2 and all looks good [1].
Tested-by: Neha Malcom Francis n-francis@ti.com
[1] https://gist.github.com/nehamalcom/66f1a2ef5288a94eebabe97708b5a277
Hi, Neha,
On 22/10/24 14:52, Neha Malcom Francis wrote:
Hi Santhosh
On 21/10/24 10:10, Santhosh Kumar K wrote:
Add ss_cfg memory region which maps the DDRSS configuration region for the memory controller node.
Signed-off-by: Santhosh Kumar K s-k6@ti.com Signed-off-by: Neha Malcom Francis n-francis@ti.com
arch/arm/dts/k3-am62a-ddr.dtsi | 7 ++++--- arch/arm/dts/k3-j721s2-ddr.dtsi | 12 ++++++++---- arch/arm/dts/k3-j784s4-ddr.dtsi | 24 ++++++++++++++++-------- 3 files changed, 28 insertions(+), 15 deletions(-)
Thank you for this series. Maybe I missed something, but why is this DT patch at the very end of the series? Doesn't it make sense to include it before the patches triggering inline ECC i.e. patch 7/8?
True, will change the order in next version.
Regards, Santhosh.
Sanity tested inline ECC on j784s4 and j721s2 and all looks good [1].
Tested-by: Neha Malcom Francis n-francis@ti.com
[1] https://gist.github.com/nehamalcom/66f1a2ef5288a94eebabe97708b5a277
Hi, everyone,
On 21/10/24 10:10, Santhosh Kumar K wrote:
Hello,
This series is to:
Add support for Inline ECC in DDR for AM64X, AM62X, AM62AX, AM62PX, J721S2 and J784S4 devices.
(1/8) Enable ECC priming with BIST engine (2/8) Add a function to store base address and size of RAM's banks in a 64-bit device private data (3/8) Setup the ECC region start and range (4/8) Enable ECC 1-bit error, 2-bit error and multiple-bit error interrupts (5/8) Add CONFIG_K3_INLINE_ECC (6/8) Set NR_DRAM_BANKS to 2 (7/8) Pull Redundant DDR functions to a common location and Fixup DDR size when ECC is enabled (8/8) Add ss_cfg reg entry
Changes since v3: Pull redundant DDR functions to arch/arm/mach-k3/*
v3: https://lore.kernel.org/all/20240523050430.455201-1-s-k6@ti.com/
Changes since v2: Removed 'default n' in Kconfig as that is default setting
v2: https://lore.kernel.org/u-boot/20240510084707.1903133-1-s-k6@ti.com/
Changes since v1: Add support for J7* devices.
v1: https://lore.kernel.org/u-boot/20240131060213.1128024-1-s-k6@ti.com/
Test Results: https://gist.github.com/santhosh21/25937355aa2c134a64fa76480ac6a4a2
I apologize for the delayed response!
Regards, Santhosh.
Thanks and Regards, Santhosh.
Georgi Vlaev (1): ram: k3-ddrss: Use the DDR controller BIST engine for ECC priming
Neha Malcom Francis (2): drivers: ram: Kconfig: Add CONFIG_K3_INLINE_ECC configs: j7*_evm_r5_defconfig: Set NR_DRAM_BANKS to 2
Santhosh Kumar K (5): ram: k3-ddrss: Add k3_ddrss_ddr_bank_base_size_calc() to solve 'calculations restricted to 32 bits' issue ram: k3-ddrss: Setup ECC region start and range ram: k3-ddrss: Enable ECC interrupts board: ti: Pull redundant DDR functions to a common location and Fixup DDR size when ECC is enabled arm: dts: k3-*-ddr: Add ss_cfg reg entry
arch/arm/dts/k3-am62a-ddr.dtsi | 7 +- arch/arm/dts/k3-j721s2-ddr.dtsi | 12 +- arch/arm/dts/k3-j784s4-ddr.dtsi | 24 ++- arch/arm/mach-k3/Makefile | 2 +- arch/arm/mach-k3/include/mach/k3-ddr.h | 15 ++ arch/arm/mach-k3/k3-ddr.c | 88 ++++++++++ board/ti/am62ax/evm.c | 17 +- board/ti/am62px/evm.c | 17 +- board/ti/am62x/evm.c | 62 +------ board/ti/am64x/evm.c | 73 +-------- board/ti/am65x/evm.c | 29 +--- board/ti/j721e/evm.c | 29 +--- board/ti/j721s2/evm.c | 35 ++-- board/ti/j784s4/evm.c | 17 +- configs/j7200_evm_r5_defconfig | 1 + configs/j721e_evm_r5_defconfig | 1 + configs/j721s2_evm_r5_defconfig | 1 + configs/j784s4_evm_r5_defconfig | 1 + drivers/ram/Kconfig | 10 ++ drivers/ram/k3-ddrss/k3-ddrss.c | 214 +++++++++++++++++++++---- 20 files changed, 387 insertions(+), 268 deletions(-) create mode 100644 arch/arm/mach-k3/include/mach/k3-ddr.h create mode 100644 arch/arm/mach-k3/k3-ddr.c
participants (5)
-
Bryan Brattlof -
Manorit Chawdhry -
Neha Malcom Francis -
Santhosh Kumar K -
Wadim Egorov