From mboxrd@z Thu Jan  1 00:00:00 1970
From: Tomas Hlavacek <tmshlvck@gmail.com>
Date: Sat, 22 Sep 2012 09:52:19 +0200
Subject: [U-Boot] [PATCH v5] [RFC] early_malloc for DM added.
In-Reply-To: <201209220228.23341.marex@denx.de>
References: <1346069529-27397-1-git-send-email-tmshlvck@gmail.com>
	<1348273502-23171-1-git-send-email-tmshlvck@gmail.com>
	<201209220228.23341.marex@denx.de>
Message-ID: <CAEB7QLD-_XQLHy8zNENEpytQLX8V5r_V4uhjr0f8OoXV80Swnw@mail.gmail.com>
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

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.

>
>> +     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.

>
>> +#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