[U-Boot] [PATCH 1/2] dm: Correct default value for SYS_MALLOC_F

[Simon Glass]

Hi Masahiro,

On 19 February 2015 at 22:43, Masahiro Yamada <yamada.m at jp.panasonic.com> wrote:
> Hi Simon,
>
>
> On Thu, 19 Feb 2015 16:56:05 -0700
> Simon Glass <sjg at chromium.org> wrote:
>
>> Hi Masahiro,
>>
>> On 19 February 2015 at 16:46, Masahiro YAMADA <yamada.m at jp.panasonic.com> wrote:
>> > Simon,
>> >
>> >
>> >
>> > 2015-02-19 23:08 GMT+09:00 Simon Glass <sjg at chromium.org>:
>> >> This should be a bool, not hex.
>> >>
>> >> Signed-off-by: Simon Glass <sjg at chromium.org>
>> >> Reported-by: Alexey Brodkin <abrodkin at synopsys.com>
>> >> ---
>> >>
>> >>  Kconfig | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/Kconfig b/Kconfig
>> >> index 75bab7f..aa35ac9 100644
>> >> --- a/Kconfig
>> >> +++ b/Kconfig
>> >> @@ -58,7 +58,7 @@ config CC_OPTIMIZE_FOR_SIZE
>> >>
>> >>  config SYS_MALLOC_F
>> >>         bool "Enable malloc() pool before relocation"
>> >> -       default 0x400
>> >> +       default y
>> >>         help
>> >>           Before relocation memory is very limited on many platforms. Still,
>> >>           we can provide a small malloc() pool if needed. Driver model in
>> >> --
>> >> 2.2.0.rc0.207.ga3a616c
>> >>
>> >
>> >
>> > It is OK to enable this option by default.
>> > For now, it is meaningless for non-DriverModel boards,
>> > but Driver Model will be the default in a long run.
>> >
>> >
>> >
>> >
>> > BTW, this series still could generate a broken .config file.
>> >
>> >
>> > I applied this series onto u-boot/master.
>> >
>> > masahiro at oscar:~/workspace/u-boot$ git log -3 --oneline
>> > f02712d kconfig: Adjust ordering so that defaults work as expected
>> > 755ad86 dm: Correct default value for SYS_MALLOC_F
>> > 1320112 Merge branch 'master' of git://git.denx.de/u-boot-fdt
>> >
>> >
>> > masahiro at oscar:~/workspace/u-boot$ make beaver_defconfig
>> > #
>> > # configuration written to .config
>> > #
>> > #
>> > # configuration written to spl/.config
>> > #
>> > masahiro at oscar:~/workspace/u-boot$ make menuconfig
>> >
>> >   Go to "General setup" and disable "Enable malloc() pool before relocation"
>> >   and save.
>> >
>> >
>> > masahiro at oscar:~/workspace/u-boot$ less .config
>> > [ snip ]
>> > CONFIG_DM_SPI_FLASH=y
>> > # CONFIG_SYS_MALLOC_F is not set
>> > CONFIG_SYS_MALLOC_F_LEN=0x1800
>> > # CONFIG_DM_CROS_EC is not set
>> > # CONFIG_TEGRA20 is not set
>> >
>> >
>> >
>> > This is why I mentioned that
>> > "default 0x1800" must be followed by "if SYS_MALLOC_F".
>>
>> Right I understand now. But does that matter? Adding the same
>> conditional to every default sounds like a pain to me. Perhaps this
>> should be a bugfix for kconfig?
>>
>
>
>
> I have thought about this for a while.
>
> I think we should always generate the sane .config in any cases.
> I hesitate to omit "if SYS_MALLOC_F", but I understand your pain.
>
> So, I have proposed another solution:
> http://patchwork.ozlabs.org/patch/441835/
> Do you like it?

It's the worst approach, apart from all the other alternatives that we
have tried :-)

So yes I think we shold use this.

> If you like, this idea can be applicable to other CONFIGs.
>
>
> As mentioned, your patch breaks microblaze-generic board.
> We cannot enable SYS_MALLOC_F globally unless we do something with
> include/configs/microblaze-generic.h
>
> So, for now I suggest to enable it if and only if CONFIG_DM is on.
> http://patchwork.ozlabs.org/patch/441836/
> This patch should not disturb MicroBlaze as it has not introduced DM yet.

OK.

Regards,
Simon