Re: [PATCH 01/16] lib: fdtdec: Handle multiple memory nodes

On 22.05.24 17:34, Jiaxun Yang wrote:

Current code only tries to fetch the first memory node found in fdt tree and determine memory banks from multiple reg properties.

Linux do allow multiple memory nodes in devicetree, rework

%/do allow/allows/

It is not Linux that allows it but

Devicetree Specification, Release v0.4 https://github.com/devicetree-org/devicetree-specification/releases/download...

fdtdec_setup_mem_size_base_lowest and fdtdec_setup_memory_banksize to iterate over all memory nodes.

Signed-off-by: Jiaxun Yang jiaxun.yang@flygoat.com

lib/fdtdec.c | 137 ++++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 83 insertions(+), 54 deletions(-)

diff --git a/lib/fdtdec.c b/lib/fdtdec.c index b2c59ab3818b..403b363043d6 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1075,90 +1075,119 @@ ofnode get_next_memory_node(ofnode mem) return mem; }

+static void sort_memory_banks(int num) +{

• int i, j;
• phys_addr_t tmp_start;
• phys_size_t tmp_size;
• struct bd_info *bd = gd->bd;
• for (i = 0; i < num - 1; i++) {

• for (j = i + 1; j < num; j++) {
  

• 	if (bd->bi_dram[i].start > bd->bi_dram[j].start) {
  

• 		tmp_start = bd->bi_dram[i].start;
  

• 		tmp_size = bd->bi_dram[i].size;
  

• 		bd->bi_dram[i].start = bd->bi_dram[j].start;
  

• 		bd->bi_dram[i].size = bd->bi_dram[j].size;
  

• 		bd->bi_dram[j].start = tmp_start;
  

• 		bd->bi_dram[j].size = tmp_size;
  

• 	}
  

• }
  

• }

Please, use qsort() instead.

+}

• int fdtdec_setup_memory_banksize(void) {

• int bank, ret, reg = 0;
• struct resource res;

• int bank = 0; ofnode mem = ofnode_null();

• mem = get_next_memory_node(mem);
• if (!ofnode_valid(mem)) {

• debug("%s: Missing /memory node\n", __func__);
  

• return -EINVAL;
  

• }

• while (true) {

• struct resource res;
  

• int reg = 0;
  
  

• for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {

• ret = ofnode_read_resource(mem, reg++, &res);
  

• if (ret < 0) {
  

• 	reg = 0;
  

• 	mem = get_next_memory_node(mem);
  

• 	if (!ofnode_valid(mem))
  

• 		break;
  

• mem = get_next_memory_node(mem);
  

• if (!ofnode_valid(mem))
  

• 	break;
  
  

• 	ret = ofnode_read_resource(mem, reg++, &res);
  

• while (true) {
  

• 	int ret = ofnode_read_resource(mem, reg, &res);
  

Please, leave a blank line after declarations.

	if (ret < 0)
		break;

• }
  
  

• if (ret != 0)
  

• 	return -EINVAL;
  

• 	if (bank >= CONFIG_VAL(NR_DRAM_BANKS))
  

• 		goto too_may_memory_banks;
  
  

• gd->bd->bi_dram[bank].start = (phys_addr_t)res.start;
  

The conversion is superfluous.

• gd->bd->bi_dram[bank].size =
  

• 	(phys_size_t)(res.end - res.start + 1);
  

• 	gd->bd->bi_dram[bank].start = (phys_addr_t)res.start;
  

• 	gd->bd->bi_dram[bank].size =
  

• 		(phys_size_t)(res.end - res.start + 1);
  
  

• debug("%s: DRAM Bank #%d: start = 0x%llx, size = 0x%llx\n",
  

•       __func__, bank,
  

•       (unsigned long long)gd->bd->bi_dram[bank].start,
  

•       (unsigned long long)gd->bd->bi_dram[bank].size);
  

• 	log_debug("%s: DRAM Bank #%d %s.%d: start = 0x%llx, size = 0x%llx\n",
  

• 		  __func__, bank, ofnode_get_name(mem), reg,
  

• 		 (unsigned long long)gd->bd->bi_dram[bank].start,
  

• 		 (unsigned long long)gd->bd->bi_dram[bank].size);
  

• 	reg++;
  

• 	bank++;
  

• }
  

  }

• if (!bank) {

• log_warning("%s: Missing /memory node\n", __func__);
  

• return -EINVAL;
  

• }

• sort_memory_banks(bank);

• return 0;

+too_may_memory_banks:

• log_warning("%s: Too many memory banks\n", __func__);

• return -EINVAL; }

  int fdtdec_setup_mem_size_base_lowest(void) {

• int bank, ret, reg = 0;
• struct resource res;
• unsigned long base;
• phys_size_t size;

• int bank = 0; ofnode mem = ofnode_null();
• __maybe_unused const char *final_name;
• __maybe_unused int final_reg;

'__maybe_unused' is superfluous.

gd->ram_base = (unsigned long)~0;

gd->ram_base = ULONG_MAX;

• mem = get_next_memory_node(mem);
• if (!ofnode_valid(mem)) {

• debug("%s: Missing /memory node\n", __func__);
  

• return -EINVAL;
  

• }

• while (true) {

• struct resource res;
  

• phys_size_t base, size;
  

• int reg = 0;
  
  

• for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {

• ret = ofnode_read_resource(mem, reg++, &res);
  

• if (ret < 0) {
  

• 	reg = 0;
  

• 	mem = get_next_memory_node(mem);
  

• 	if (!ofnode_valid(mem))
  

• 		break;
  

• mem = get_next_memory_node(mem);
  

• if (!ofnode_valid(mem))
  

• 	break;
  
  

• 	ret = ofnode_read_resource(mem, reg++, &res);
  

• while (true) {
  

• 	int ret = ofnode_read_resource(mem, reg, &res);
  if (ret < 0)
  	break;
  

• 	base = res.start;
  

• 	size = res.end - res.start + 1;
  

• 	if (gd->ram_base > base && size) {
  

• 		gd->ram_base = base;
  

• 		gd->ram_size = size;
  

• 		final_name = ofnode_get_name(mem);
  

• 		final_reg = reg;
  

• 	}
  

• 	reg++;
  

• 	bank++;
  

  }
• }

• if (ret != 0)
  

• 	return -EINVAL;
  

• base = (unsigned long)res.start;
  

• size = (phys_size_t)(res.end - res.start + 1);
  

• if (gd->ram_base > base && size) {
  

• 	gd->ram_base = base;
  

• 	gd->ram_size = size;
  

• 	debug("%s: Initial DRAM base %lx size %lx\n",
  

• 	      __func__, base, (unsigned long)size);
  

• }
  

• if (!bank) {

• log_warning("%s: Missing /memory node\n", __func__);
  

• return -EINVAL;
  

  }

• log_debug("%s: Initial DRAM %s.%d: base %lx size %lx\n",

•   __func__, final_name, final_reg,
  

•   (ulong)gd->ram_base, (ulong)gd->ram_size);
  

ram_base is already defined as unsigned long.

include/asm-generic/global_data.h:154: unsigned long ram_base;

Best regards

Heinrich

• return 0; }