[PATCH v3 7/8] board: ti: Pull redundant DDR functions to a common location and Fixup DDR size when ECC is enabled
Santhosh Kumar K
s-k6 at ti.com
Fri Jun 7 07:19:13 CEST 2024
Hi, Wadim,
Thanks for the review.
On 30/05/24 17:48, Wadim Egorov wrote:
> Hi Santhosh,
>
> thanks for this series!
>
> Am 23.05.24 um 07:04 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 at ti.com>
>> Signed-off-by: Neha Malcom Francis <n-francis at ti.com>
>> ---
> [...]
>> +++ b/board/ti/common/k3-ddr-init.c
>> @@ -0,0 +1,89 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2023, Texas Instruments Incorporated -
>> https://www.ti.com/
>> + */
>> +
>> +#include <fdt_support.h>
>> +#include <dm/uclass.h>
>> +#include <k3-ddrss.h>
>> +#include <spl.h>
>> +
>> +#include "k3-ddr-init.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;
>> +}
>
> I'm wondering if we can generalize more.
>
> What do you say if we keep dram_init() & dram_init_banksize() in the
> board code and move fixup_ddr_driver_for_ecc() & fixup_memory_node() to
> mach-k3 and make them available for all K3 based boards?
>
> It looks like I will reuse the code for our boards and I assume other
> vendors too.
>
> Regards,
> Wadim
>
Yeah, makes sense, will work on it and post v4.
Regards,
Santhosh.
>> +
>> +#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);
>> +
>> + dram_init_banksize();
>> +
>> + ret = uclass_next_device_err(&dev);
>> +
>> + while (!ret) {
>> + 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);
>> +
>> + dram_init_banksize();
>> + 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/common/k3-ddr-init.h
>> b/board/ti/common/k3-ddr-init.h
>> new file mode 100644
>> index 000000000000..9d1826815dfd
>> --- /dev/null
>> +++ b/board/ti/common/k3-ddr-init.h
>> @@ -0,0 +1,15 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * Copyright (C) 2023, Texas Instruments Incorporated -
>> https://www.ti.com/
>> + */
>> +
>> +#ifndef __K3_DDR_INIT_H
>> +#define __K3_DDR_INIT_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_INIT_H */
>> diff --git a/board/ti/j721e/evm.c b/board/ti/j721e/evm.c
>> index 539eaf47186a..e0cd8529bc2b 100644
>> --- a/board/ti/j721e/evm.c
>> +++ b/board/ti/j721e/evm.c
>> @@ -17,6 +17,7 @@
>> #include "../common/board_detect.h"
>> #include "../common/fdt_ops.h"
>> +#include "../common/k3-ddr-init.h"
>> #define board_is_j721e_som() (board_ti_k3_is("J721EX-PM1-SOM") || \
>> board_ti_k3_is("J721EX-PM2-SOM"))
>> @@ -37,17 +38,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
>> @@ -59,23 +49,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..fca30107cddb 100644
>> --- a/board/ti/j721s2/evm.c
>> +++ b/board/ti/j721s2/evm.c
>> @@ -24,6 +24,7 @@
>> #include "../common/board_detect.h"
>> #include "../common/fdt_ops.h"
>> +#include "../common/k3-ddr-init.h"
>> DECLARE_GLOBAL_DATA_PTR;
>> @@ -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 aed0ea5b9495..73056a16dd5f 100644
>> --- a/board/ti/j784s4/evm.c
>> +++ b/board/ti/j784s4/evm.c
>> @@ -10,6 +10,7 @@
>> #include <init.h>
>> #include <spl.h>
>> #include "../common/fdt_ops.h"
>> +#include "../common/k3-ddr-init.h"
>> DECLARE_GLOBAL_DATA_PTR;
>> @@ -18,15 +19,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)
More information about the U-Boot
mailing list