From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Sat, 22 Sep 2012 15:19:30 +0200
Subject: [U-Boot] [PATCH v5] [RFC] early_malloc for DM added.
In-Reply-To: <CAEB7QLD-_XQLHy8zNENEpytQLX8V5r_V4uhjr0f8OoXV80Swnw@mail.gmail.com>
References: <1346069529-27397-1-git-send-email-tmshlvck@gmail.com>
	<201209220228.23341.marex@denx.de>
	<CAEB7QLD-_XQLHy8zNENEpytQLX8V5r_V4uhjr0f8OoXV80Swnw@mail.gmail.com>
Message-ID: <201209221519.30436.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Dear Tomas Hlavacek,

> Hello Marek,
> 
> On Sat, Sep 22, 2012 at 2:28 AM, Marek Vasut <marex@denx.de> wrote:
> > Dear Tomas Hlavacek,
> > 
> >> early_malloc for DM with support for more heaps and lightweight
> >> first heap in the same memory as an early stack.
> >> 
> >> Adaptation layer for seamless calling of early_malloc or dlmalloc from
> >> DM based on init stage added (dmmalloc() and related functions).
> >> 
> >> Signed-off-by: Tomas Hlavacek <tmshlvck@gmail.com>
> > 
> > [...]
> > 
> >>  31 files changed, 363 insertions(+)
> >>  create mode 100644 common/dmmalloc.c
> >>  create mode 100644 include/dmmalloc.h
> > 
> > What exactly changed in this version? Changelog is missing.
> 
> Preprocessor config-dependent (ifdef ... endif) blocks are different.
> early_brk call from early_malloc when the heaps are full.
> default early_brk checks existing heap for before setting.
> 
> > [...]
> > 
> >> +static int early_malloc_active(void)
> >> +{
> >> +     if ((gd->flags & GD_FLG_RELOC) == GD_FLG_RELOC)
> >> +             return 0;
> > 
> > Did you completely ignore the comments?
> 
> Yes, this is ugly and I did not notice the point of (your?) comment
> while reworking this tiny part again and again... Sorry.

I think it was GR.

> >> +     return 1;
> >> +}
> >> +#endif /* CONFIG_SYS_EARLY_MALLOC */
> >> +
> >> +#ifdef CONFIG_SYS_EARLY_MALLOC
> >> +void *dmmalloc(size_t size)
> >> +{
> >> +     if (early_malloc_active())
> >> +             return early_malloc(size);
> >> +     return malloc(size);
> >> +}
> >> +#else /* CONFIG_SYS_EARLY_MALLOC */
> >> +#define dmmalloc malloc
> > 
> > How is this actually supposed to work?
> 
> Of course, this have to be in the header file dmmalloc.h. Just ignore
> the #else ... #endif part.

That won't work well either ... I'd say make the wrapper static inline for 
typechecking, no ?

> >> +#endif /* CONFIG_SYS_EARLY_MALLOC */
> >> +
> >> +#ifdef CONFIG_SYS_EARLY_MALLOC
> >> +void dmfree(void *ptr)
> >> +{
> >> +     if (early_malloc_active())
> >> +             return;
> >> +     free(ptr);
> >> +}
> >> +#else /* CONFIG_SYS_EARLY_MALLOC */
> >> +#define dmfree free
> >> +#endif /* CONFIG_SYS_EARLY_MALLOC */
> >> +
> >> +#ifdef CONFIG_SYS_EARLY_MALLOC
> >> +void *dmcalloc(size_t n, size_t elem_size)
> >> +{
> >> +     if (early_malloc_active())
> >> +             return NULL;
> >> +     return calloc(n, elem_size);
> >> +}
> >> +#else /* CONFIG_SYS_EARLY_MALLOC */
> >> +#define dmcalloc calloc
> >> +#endif /* CONFIG_SYS_EARLY_MALLOC */
> >> +
> >> +#ifdef CONFIG_SYS_EARLY_MALLOC
> >> +void *dmrealloc(void *oldmem, size_t bytes)
> >> +{
> >> +     if (early_malloc_active())
> >> +             return NULL;
> >> +     return dmrealloc(oldmem, bytes);
> >> +}
> >> +#else /* CONFIG_SYS_EARLY_MALLOC */
> >> +#define dmrealloc realloc
> >> +#endif /* CONFIG_SYS_EARLY_MALLOC */
> >> +
> >> diff --git a/include/dmmalloc.h b/include/dmmalloc.h
> >> new file mode 100644
> >> index 0000000..726c6c9
> >> --- /dev/null
> >> +++ b/include/dmmalloc.h
> >> @@ -0,0 +1,56 @@
> >> +/*
> >> + * (C) Copyright 2012
> >> + * Tomas Hlavacek (tmshlvck at gmail.com)
> >> + *
> >> + * See file CREDITS for list of people who contributed to this
> >> + * project.
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public License as
> >> + * published by the Free Software Foundation; either version 2 of
> >> + * the License, or (at your option) any later version.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> + * along with this program; if not, write to the Free Software
> >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> >> + * MA 02111-1307 USA
> >> + */
> >> +
> >> +#ifndef __INCLUDE_DMMALLOC_H
> >> +#define __INCLUDE_DMMALLOC_H
> >> +
> >> +#include <config.h>
> >> +#include <linux/stddef.h> /* for size_t */
> >> +
> >> +#if (!defined(CONFIG_SYS_EARLY_HEAP_ADDR)) || \
> >> +     (!defined(CONFIG_SYS_EARLY_HEAP_SIZE))
> >> +#undef CONFIG_SYS_EARLY_MALLOC
> >> +#endif /* CONFIG_SYS_EARLY_HEAP_ADDR */
> >> +
> >> +#ifdef CONFIG_SYS_EARLY_MALLOC
> >> +struct early_heap_header {
> >> +     void *free_space_pointer;
> >> +     size_t free_bytes;
> >> +     void *next_early_heap;
> >> +};
> >> +
> >> +struct early_heap_header *early_brk(size_t size);
> >> +void *early_malloc(size_t size);
> >> +
> >> +#endif /* CONFIG_SYS_EARLY_MALLOC */
> >> +
> >> +#ifdef CONFIG_SYS_DM
> > 
> > Isn't it CONFIG_DM ?
> 
> I will change that. (Is it consistent with the naming convention? I
> mean: What the "_SYS_" exactly means?)
> 
> Tomas

Best regards,
Marek Vasut