* [U-Boot] [U-Boot-DM] [PATCH 1/1] early_malloc() introduced to ARM architecture
[not found] <CAEB7QLAf3ha5wp50+2R7_vhMh7bEU8EtHx-QvoRz3fXy+AsJtQ@mail.gmail.com>
@ 2012-07-28 20:36 ` Marek Vasut
2012-07-29 6:15 ` Tomas Hlavacek
0 siblings, 1 reply; 3+ messages in thread
From: Marek Vasut @ 2012-07-28 20:36 UTC (permalink / raw)
To: u-boot
Dear Tomas Hlavacek,
I think we should still mark early patches as RFC.
> early_malloc() introduced to ARM architecture (it is a proof of concept).
> GD datastructure extended for holding early-heap.
> mem_malloc_init() in board_init_r() has been put up by few lines.
I take it was sent by the gmail webIF ? Like WD said, not a good idea, git send-
email is really the way. it's actually simple, we can discuss that tomorrow.
> Signed-off-by: Tomas Hlavacek <tmshlvck@gmail.com>
> ---
> arch/arm/include/asm/global_data.h | 4 +-
> arch/arm/lib/board.c | 32 +++++++++--
> common/Makefile | 1 +
> common/earlymalloc.c | 102
> ++++++++++++++++++++++++++++++++++++
> include/common.h | 1 +
> include/earlymalloc.h | 84 +++++++++++++++++++++++++++++
> lib/asm-offsets.c | 2 +-
> 7 files changed, 220 insertions(+), 6 deletions(-)
> create mode 100644 common/earlymalloc.c
> create mode 100644 include/earlymalloc.h
>
> diff --git a/arch/arm/include/asm/global_data.h
> b/arch/arm/include/asm/global_data.h
> index c3ff789..215212a 100644
> --- a/arch/arm/include/asm/global_data.h
> +++ b/arch/arm/include/asm/global_data.h
> @@ -30,7 +30,8 @@
> * global variables during system initialization (until we have set
> * up the memory controller so that we can use RAM).
> *
> - * Keep it *SMALL* and remember to set GENERATED_GBL_DATA_SIZE >
> sizeof(gd_t)
> + * Keep it *SMALL* and remember to set GENERATED_GBL_DATA_SIZE >
> + * sizeof(gd_t) + EARLY_HEAP_SIZE
> */
>
> typedef struct global_data {
> @@ -86,6 +87,7 @@ typedef struct global_data {
> #endif
> } gd_t;
>
> +
Ok well ... you know, this will need cleanup. But for RFC, this is OK.
> /*
> * Global Data Flags
> */
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 500e216..81ee27b 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -52,6 +52,7 @@
> #include <fdtdec.h>
> #include <post.h>
> #include <logbuff.h>
> +#include <earlymalloc.h>
Do we need early_malloc.h at all? malloc.h won't cut it?
> #ifdef CONFIG_BITBANGMII
> #include <miiphy.h>
> @@ -273,6 +274,9 @@ void board_init_f(ulong bootflag)
>
> memset((void *)gd, 0, sizeof(gd_t));
>
> + early_malloc_init();
So this basically flips a bit, do it the other way and you don't need it.
> + debug("Early malloc initialized.\n");
> +
> gd->mon_len = _bss_end_ofs;
> #ifdef CONFIG_OF_EMBED
> /* Get a pointer to the FDT */
> @@ -452,12 +469,23 @@ void board_init_r(gd_t *id, ulong dest_addr)
> ulong flash_size;
> #endif
>
> + gd_t *old_gd = gd;
> +
> gd = id;
>
> gd->flags |= GD_FLG_RELOC; /* tell others: relocation done */
>
> monitor_flash_len = _end_ofs;
>
> + /* The Malloc area is immediately below the monitor copy in DRAM */
> + malloc_start = dest_addr - TOTAL_MALLOC_LEN;
> + mem_malloc_init (malloc_start, TOTAL_MALLOC_LEN);
> +
> + // TODO: DM Cores relocation goes here.
> +
> + /* Disable early mallocator (effectively switch calls to the real
> malloc). */
> + early_malloc_disab((gde_t *)old_gd);
> +
> /* Enable caches */
> enable_caches();
>
> @@ -485,10 +513,6 @@ void board_init_r(gd_t *id, ulong dest_addr)
> post_output_backlog();
> #endif
>
> - /* The Malloc area is immediately below the monitor copy in DRAM */
> - malloc_start = dest_addr - TOTAL_MALLOC_LEN;
> - mem_malloc_init (malloc_start, TOTAL_MALLOC_LEN);
> -
Can mem_malloc_init() not flip on the early_malloc bit for you here?
> #if !defined(CONFIG_SYS_NO_FLASH)
> puts("Flash: ");
>
[...]
> +
> +#include <asm/global_data.h> /* for gd_t and gd */
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static inline size_t early_malloc_align_up(phys_addr_t addr)
> +{
> + if(!EARLY_MALLOC_IS_ALIGNED(addr)) {
> + addr = EARLY_MALLOC_ALIGN_DOWN(addr);
> + addr += EARLY_MALLOC_ALIGN_MULTIPLE;
> + }
> +
> + return addr;
> +}
> +
> +void early_malloc_init(void)
> +{
> + ((gde_t *)gd)->em.flags = EARLY_MALLOC_INIT_FLAGS;
> + ((gde_t *)gd)->em.size = 0;
> +}
> +
> +
> +int early_malloc_isactive(void)
> +{
> + /* The early_malloc() is inactive after relocation or when the
> + flag says so. */
> + return ((EARLY_MALLOC_IS_ACTIVE(((gde_t *)gd)->em.flags)) ||
> + (gd->flags & GD_FLG_RELOC));
So ... gd->flags will be enough for the early malloc, you don't need any further
flags
> +}
> +
> +int early_free_isactive(void)
> +{
> + /* The early free is inactive when the flag says so. */
> + return (gd->flags & GD_FLG_RELOC);
> +}
> +
> +
> +void *early_malloc(size_t size)
No, we want this wrapped into malloc() call, so the drivers can be inited
indifferent of time.
> +{
> + em_spc_t *em = &((gde_t *)gd)->em;
> +
> + /* Check flag active. */
> + if(!EARLY_MALLOC_IS_ACTIVE(em->flags))
> + return NULL;
Indent with tab please ... tools/checkpatch.pl will help here.
> + /* Choose block beginning address. */
> + phys_addr_t addr =
> early_malloc_align_up(((phys_addr_t)&em->heap)+em->size);
> +
> + /* Check free space. */
> + if(addr + size >= ((phys_addr_t)(&(em->heap))) + EARLY_HEAP_SIZE)
> + return NULL;
> +
> + /* Calculate next free space. */
> + em->size+=early_malloc_align_up(size);
> +
> + return (void *)addr;
> +}
> +
> +
> +int early_malloc_isaddress(void *addr)
> +{
> + if((((phys_addr_t)addr) >= (phys_addr_t)(&(((gde_t *)gd)->em.heap)))&&
> + (((phys_addr_t)addr) < ((phys_addr_t)(&(((gde_t
> *)gd)->em.heap)))+EARLY_HEAP_SIZE))
See container_of() ... and you can really split the checks.
> + return 1;
> + else
> + return 0;
> +}
> +
> +void early_free(void *addr)
> +{
> + /* NOOP - Early malloc does not support free(). */
This goes to free() call ...
> +}
> +
> +void early_malloc_disab(gde_t *gd)
> +{
> + EARLY_MALLOC_DISABLE(gd->em.flags);
> +}
> diff --git a/include/common.h b/include/common.h
> index a2c6b27..486925f 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -171,6 +171,7 @@ typedef void (interrupt_handler_t)(void *);
>
> #include <asm/u-boot.h> /* boot information for Linux kernel */
> #include <asm/global_data.h> /* global data used for startup functions
> */
> +#include <earlymalloc.h> /* Early mallocator data structure that extends
> global data. */
Drop this altogether.
[...]
> +
> +typedef struct early_malloc_spc {
> + char flags;
> + short size;
> + char heap[EARLY_HEAP_SIZE];
> +} em_spc_t;
Drop the typedef please.
> +/*
> + * Platform data extended for the early mallocator.
> + * This structure holds the original global data (and it starts at
> + * the very same address), but it is longer because it holds
> + * early malloc space in the end.
> + * This is a trick with data structures: This datastructure is
> + * pre-allocated by lib/asm-offsets.c but this structure is not to
> + * be relocated as whole because struct global_data is used during
> + * normal operations as well as during relocation phase.
> + */
> +typedef struct global_data_extended {
I'm not convinced about the typedef here either ... others?
> + gd_t gd; /* Original Global Data
> structure */
> + em_spc_t em; /* Early-malloc space.
> */
> +} gde_t;
> +
> +/* Initial flags block. Active flag set. */
> +#define EARLY_MALLOC_INIT_FLAGS 0x80
> +
> +/* Alignment size - number of bytes to align to it's mupliple. */
> +#define EARLY_MALLOC_ALIGN_MULTIPLE (sizeof(phys_addr_t))
> +#define EARLY_MALLOC_ALIGN_MASK (EARLY_MALLOC_ALIGN_MULTIPLE-1)
> +
> +
> +#define EARLY_MALLOC_ALIGN_DOWN(addr) ((phys_addr_t)addr &
> (~EARLY_MALLOC_ALIGN_MASK))
> +#define EARLY_MALLOC_IS_ALIGNED(addr) (!(((phys_addr_t)addr) &
> EARLY_MALLOC_ALIGN_MASK))
> +#define EARLY_MALLOC_FLAG_ACTIVE 0x80
> +#define EARLY_MALLOC_IS_ACTIVE(flags) (flags & EARLY_MALLOC_FLAG_ACTIVE)
> +#define EARLY_MALLOC_DISABLE(flags)
> (flags=(flags&(~EARLY_MALLOC_FLAG_ACTIVE)))
You won't need this whole file I think ...
> +
> +
> +void early_malloc_init(void);
> +int early_malloc_isactive(void);
> +int early_free_isactive(void);
> +void *early_malloc(size_t size);
> +void early_free(void *addr);
> +void early_malloc_disab(gde_t *gd);
> +int early_malloc_isaddress(void *);
> +
> +#endif
> diff --git a/lib/asm-offsets.c b/lib/asm-offsets.c
> index c88f5d4..6e39826 100644
> --- a/lib/asm-offsets.c
> +++ b/lib/asm-offsets.c
> @@ -23,7 +23,7 @@ int main(void)
> {
> /* Round up to make sure size gives nice stack alignment */
> DEFINE(GENERATED_GBL_DATA_SIZE,
> - (sizeof(struct global_data) + 15) & ~15);
> + (sizeof(struct global_data_extended) + 15) & ~15);
And if you adjusted the global data to be the "container of both, you won't need
this here either.
>
> DEFINE(GENERATED_BD_INFO_SIZE,
> (sizeof(struct bd_info) + 15) & ~15);
^ permalink raw reply [flat|nested] 3+ messages in thread