Re: [PATCH 1/2] pbl: cleanup early malloc

["Sascha Hauer" <s.hauer@pengutronix.de>]

On 2026-05-18 18:14, Ahmad Fatoum wrote:
> On 5/18/26 5:32 PM, Sascha Hauer wrote:
> > All architectures using PBL define the same variables free_mem_ptr and
> > free_mem_end_ptr and initialize them. create a helper for that in
> > pbl/malloc.c and rename simple_*() to pbl_*().
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> 
> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> with issues below addressed
> 
> > ---
> >  arch/arm/cpu/uncompress.c              |  6 +---
> >  arch/mips/boot/uncompress.c            |  6 +---
> >  arch/riscv/boot/uncompress.c           |  6 +---
> >  arch/riscv/include/asm/barebox-riscv.h |  6 ----
> >  include/linux/decompress/mm.h          | 44 ++++----------------------
> >  include/mach/bcm283x/debug_ll.h        | 10 ------
> >  include/pbl.h                          |  7 ++--
> >  pbl/Makefile                           |  1 +
> >  pbl/malloc.c                           | 43 +++++++++++++++++++++++++
> >  9 files changed, 57 insertions(+), 72 deletions(-)
> >  create mode 100644 pbl/malloc.c
> > 
> > diff --git a/arch/arm/cpu/uncompress.c b/arch/arm/cpu/uncompress.c
> > index 38f7dbc113..c7802226e6 100644
> > --- a/arch/arm/cpu/uncompress.c
> > +++ b/arch/arm/cpu/uncompress.c
> > @@ -27,9 +27,6 @@
> >  
> >  #include "entry.h"
> >  
> > -unsigned long free_mem_ptr;
> > -unsigned long free_mem_end_ptr;
> > -
> >  extern unsigned char input_data[];
> >  extern unsigned char input_data_end[];
> >  
> > @@ -78,8 +75,7 @@ void __noreturn barebox_pbl_start(unsigned long membase, unsigned long memsize,
> >  
> >  	handoff_data = (void *)barebox_base + uncompressed_len + MAX_BSS_SIZE;
> >  
> > -	free_mem_ptr = barebox_base - ARM_MEM_EARLY_MALLOC_SIZE;
> > -	free_mem_end_ptr = barebox_base;
> > +	pbl_malloc_init(barebox_base - ARM_MEM_EARLY_MALLOC_SIZE, ARM_MEM_EARLY_MALLOC_SIZE);
> >  
> >  #ifdef DEBUG
> >  	print_pbl_mem_layout(membase, endmem, barebox_base);
> > diff --git a/arch/mips/boot/uncompress.c b/arch/mips/boot/uncompress.c
> > index 0630d03c6b..b58b23ea6b 100644
> > --- a/arch/mips/boot/uncompress.c
> > +++ b/arch/mips/boot/uncompress.c
> > @@ -16,9 +16,6 @@
> >  extern void *input_data;
> >  extern void *input_data_end;
> >  
> > -unsigned long free_mem_ptr;
> > -unsigned long free_mem_end_ptr;
> > -
> >  void barebox_pbl_start(void *fdt, void *fdt_end, unsigned long ram_size);
> >  
> >  void __section(.text_entry) barebox_pbl_start(void *fdt, void *fdt_end,
> > @@ -34,8 +31,7 @@ void __section(.text_entry) barebox_pbl_start(void *fdt, void *fdt_end,
> >  	memset(__bss_start, 0, __bss_stop - __bss_start);
> >  
> >  	/* set 128 KiB at the end of the MALLOC_BASE for early malloc */
> > -	free_mem_ptr = TEXT_BASE - SZ_128K;
> > -	free_mem_end_ptr = free_mem_ptr + SZ_128K;
> > +	pbl_malloc_init(barebox_base - TEXT_BASE - SZ_128K, SZ_128K);
> >  
> >  	piggy_len = (unsigned long)&input_data_end - (unsigned long)&input_data;
> >  
> > diff --git a/arch/riscv/boot/uncompress.c b/arch/riscv/boot/uncompress.c
> > index e51f1b0121..df60b14ed8 100644
> > --- a/arch/riscv/boot/uncompress.c
> > +++ b/arch/riscv/boot/uncompress.c
> > @@ -23,9 +23,6 @@
> >  
> >  #include "entry.h"
> >  
> > -unsigned long free_mem_ptr;
> > -unsigned long free_mem_end_ptr;
> > -
> >  void __noreturn barebox_pbl_start(unsigned long membase, unsigned long memsize,
> >  				  void *fdt)
> >  {
> > @@ -63,8 +60,7 @@ void __noreturn barebox_pbl_start(unsigned long membase, unsigned long memsize,
> >  
> >  	pr_debug("memory at 0x%08lx, size 0x%08lx\n", membase, memsize);
> >  
> > -	free_mem_ptr = riscv_mem_early_malloc(membase, endmem);
> > -	free_mem_end_ptr = riscv_mem_early_malloc_end(membase, endmem);
> > +	pbl_malloc_init(riscv_mem_early_malloc(), SZ_128K);
> 
> This is a bit dangerous as it now hardcodes SZ_128K at two places and
> one could be missed when trying to extend the region.
> 
> Could you #define RISCV_MEM_EARLY_MALLOC_SIZE SZ_128K in the same patch?

All architectures use 128KiB and if we have to change it chances are
good that we have to change it for all architectures, so I'll add a
common define for this instead.

> 
> 
> > diff --git a/include/mach/bcm283x/debug_ll.h b/include/mach/bcm283x/debug_ll.h
> > index 8bbff4fd9a..68542fd6a3 100644
> > --- a/include/mach/bcm283x/debug_ll.h
> > +++ b/include/mach/bcm283x/debug_ll.h
> > @@ -42,16 +42,6 @@ static inline void debug_ll_init(void)
> >  
> >  #elif defined CONFIG_DEBUG_RPI3_MINI_UART
> >  
> > -static inline uint8_t debug_ll_read_reg(void __iomem *base, int reg)
> > -{
> > -	return readb(base + (reg << 2));
> > -}
> > -
> > -static inline void debug_ll_write_reg(void __iomem *base, int reg, uint8_t val)
> > -{
> > -	writeb(val, base + (reg << 2));
> > -}
> > -
> 
> Unrelated change?

Yes, dropped.

Sascha

--
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |