[PATCH v3 7/8] board: ti: Pull redundant DDR functions to a common location and Fixup DDR size when ECC is enabled

Wadim Egorov w.egorov at phytec.de
Thu May 30 14:18:31 CEST 2024


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

> +
> +#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