[RFC][PATCH 1/3] fdtdec: Deduplicate iterator function

[Casey Connolly]

On 25/03/2026 04:17, Marek Vasut wrote:
> Both fdtdec_setup_memory_banksize() and fdtdec_setup_mem_size_base_lowest()
> implement the exact same iterator over all memory banks, the only difference
> is the body that is executed for each bank. Deduplicate the functionality
> into iterator function fdtdec_setup_mem_for_each_bank(), which takes a
> function pointer to a function which implements the body as a parameter.
> No functional change.

This is a nice optimisation! It will be good to de-dup some thing in
mach-snapdragon too with this.

Reviewed-by: Casey Connolly <casey.connolly at linaro.org>

Kind regards,>
> Signed-off-by: Marek Vasut <marek.vasut+renesas at mailbox.org>
> ---
> Cc: Casey Connolly <casey.connolly at linaro.org>
> Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> Cc: Nobuhiro Iwamatsu <iwamatsu at nigauri.org>
> Cc: Raymond Mao <raymondmaoca at gmail.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Tom Rini <trini at konsulko.com>
> Cc: u-boot at lists.denx.de
> ---
>  lib/fdtdec.c | 77 ++++++++++++++++++++--------------------------------
>  1 file changed, 30 insertions(+), 47 deletions(-)
> 
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index c38738b48c7..d820f75b031 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -1092,7 +1092,7 @@ ofnode get_next_memory_node(ofnode mem)
>  	return mem;
>  }
>  
> -int fdtdec_setup_memory_banksize(void)
> +static int fdtdec_setup_mem_for_each_bank(void (*bankfn)(struct resource *res, int bank))
>  {
>  	int bank, ret, reg = 0;
>  	struct resource res;
> @@ -1120,63 +1120,46 @@ int fdtdec_setup_memory_banksize(void)
>  		if (ret != 0)
>  			return -EINVAL;
>  
> -		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);
> +		bankfn(&res, bank);
>  	}
>  
>  	return 0;
>  }
>  
> -int fdtdec_setup_mem_size_base_lowest(void)
> +static void fdtdec_setup_memory_banksize_bankfn(struct resource *res, int bank)
>  {
> -	int bank, ret, reg = 0;
> -	struct resource res;
> -	unsigned long base;
> -	phys_size_t size;
> -	ofnode mem = ofnode_null();
> +	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);
>  
> -	gd->ram_base = (unsigned long)~0;
> -
> -	mem = get_next_memory_node(mem);
> -	if (!ofnode_valid(mem)) {
> -		debug("%s: Missing /memory node\n", __func__);
> -		return -EINVAL;
> -	}
> -
> -	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;
> -
> -			ret = ofnode_read_resource(mem, reg++, &res);
> -			if (ret < 0)
> -				break;
> -		}
> -
> -		if (ret != 0)
> -			return -EINVAL;
> +	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);
> +}
>  
> -		base = (unsigned long)res.start;
> -		size = (phys_size_t)(res.end - res.start + 1);
> +int fdtdec_setup_memory_banksize(void)
> +{
> +	return fdtdec_setup_mem_for_each_bank(fdtdec_setup_memory_banksize_bankfn);
> +}
>  
> -		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);
> -		}
> +static void fdtdec_setup_mem_size_base_lowest_bankfn(struct resource *res, int bank)
> +{
> +	unsigned long base = (unsigned long)res->start;
> +	phys_size_t 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);
>  	}
> +}
>  
> -	return 0;
> +int fdtdec_setup_mem_size_base_lowest(void)
> +{
> +	gd->ram_base = (unsigned long)~0;
> +
> +	return fdtdec_setup_mem_for_each_bank(fdtdec_setup_mem_size_base_lowest_bankfn);
>  }
>  
>  static int uncompress_blob(const void *src, ulong sz_src, void **dstp)

-- 
// Casey (she/her)