On Thu, 19 Feb 2015 16:56:05 -0700 Simon Glass sjg@chromium.org wrote:

Hi Masahiro,

On 19 February 2015 at 16:46, Masahiro YAMADA yamada.m@jp.panasonic.com wrote:

...

Simon,

2015-02-19 23:08 GMT+09:00 Simon Glass sjg@chromium.org:

...

This should be a bool, not hex.

Signed-off-by: Simon Glass sjg@chromium.org Reported-by: Alexey Brodkin abrodkin@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@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@oscar:~/workspace/u-boot$ make beaver_defconfig # # configuration written to .config # # # configuration written to spl/.config # masahiro@oscar:~/workspace/u-boot$ make menuconfig

Go to "General setup" and disable "Enable malloc() pool before relocation" and save.

masahiro@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 no idea. Adding default values in Kconfig is not probably what Linux folks expect.

BTW, 1/2 introduces more warnings to microblaze-generic.

yamada@beagle:~/workspace/u-boot$ tools/buildman/buildman -se -b simon_kconfig microblaze boards.cfg is up to date. Nothing to do. Summary of 3 commits for 1 boards (1 thread, 8 jobs per thread) 01: Merge branch 'master' of git://git.denx.de/u-boot-fdt microblaze: + microblaze-generic + sect_first, sect_last, (info-flash_info)+1); + ^ + (info-flash_info)+1); w+../common/cmd_flash.c: In function 'do_flerase': w+../common/cmd_flash.c:339:4: warning: format '%zu' expects argument of type 'size_t', but argument 4 has type 'long int' [-Wformat=] w+../common/cmd_flash.c: In function 'do_protect': w+../common/cmd_flash.c:525:4: warning: format '%zu' expects argument of type 'size_t', but argument 5 has type 'long int' [-Wformat=] 02: dm: Correct default value for SYS_MALLOC_F +In file included from include/config.h:4:0, + from ../include/common.h:18, + from ../lib/asm-offsets.c:15: + # define CONFIG_SYS_MALLOC_F_LEN 1024 + ^ +In file included from ./../include/linux/kconfig.h:4:0, + from <command-line>:0: +include/generated/autoconf.h:31:0: note: this is the location of the previous definition + #define CONFIG_SYS_MALLOC_F_LEN 0x400 + from ../arch/microblaze/cpu/irq.S:9: + from ../arch/microblaze/lib/bootm.c:11: + from ../board/xilinx/microblaze-generic/microblaze-generic.c:12: + from ../disk/part.c:8: + from ../disk/part_dos.c:17: + from ../arch/microblaze/cpu/interrupts.c:11: + from ../drivers/block/systemace.c:28: + from ../arch/microblaze/cpu/cache.c:9: + from ../drivers/crypto/fsl/sec.c:7: + from ../arch/microblaze/cpu/exception.c:9: + from ../arch/microblaze/cpu/timer.c:9: + from ../drivers/gpio/xilinx_gpio.c:7: + from ../drivers/input/input.c:10: + from ../arch/microblaze/cpu/start.S:12: + from ../drivers/input/key_matrix.c:10: + from ../common/main.c:10: + from ../drivers/pcmcia/tqm8xx_pcmcia.c:5: + from ../common/exports.c:1: + from ../include/ubi_uboot.h:17, + from ../drivers/mtd/mtdcore.c:32: + from ../common/hash.c:14: + from ../common/cli_hush.c:81: + from ../drivers/rtc/date.c:12: + from ../drivers/mtd/mtdpart.c:21: + from ../common/autoboot.c:8: + from ../drivers/mtd/cfi_flash.c:20: + from ../common/board_f.c:13: + from ../drivers/watchdog/xilinx_tb_wdt.c:7: + from ../drivers/mtd/cfi_mtd.c:9: + from ../drivers/mtd/ubi/attach.c:85: + from ../drivers/mtd/ubi/build.c:36: + from ../drivers/net/netconsole.c:8: + from ../drivers/net/xilinx_emaclite.c:10: + from ../common/board_r.c:13: + from ../common/cmd_boot.c:11: + from ../drivers/net/xilinx_ll_temac.c:22: + from ../drivers/net/xilinx_ll_temac_mdio.c:24: + from ../common/cmd_bootm.c:11: + from ../common/bootm.c:9: + from ../drivers/mtd/ubi/vtbl.c:52: + from ../drivers/net/xilinx_ll_temac_fifo.c:25: + from ../drivers/net/xilinx_ll_temac_sdma.c:25: + from ../drivers/mtd/ubi/vmt.c:20: + from ../common/cmd_help.c:8: + from ../common/bootm_os.c:8: + from ../common/cmd_version.c:8: + from ../common/env_attr.c:13: + from ../common/env_callback.c:8: + from ../drivers/mtd/ubi/upd.c:33: + from ../common/env_flags.c:19: + from ../drivers/net/phy/phy.c:12: + from ../common/env_flash.c:13: + from ../drivers/mtd/ubi/kapi.c:18: + from ../common/cmd_bdinfo.c:11: + from ../common/cmd_source.c:18: + from ../common/cmd_console.c:11: + from ../common/cmd_echo.c:8: + from ../drivers/mtd/ubi/eba.c:36: + from ../drivers/mtd/ubi/io.c:81: + from ../drivers/mtd/ubi/wl.c:95: + from ../common/cmd_exit.c:8: + from ../include/fs.h:19, + from ../common/cmd_ext2.c:23: + from ../common/cmd_fat.c:11: + from ../common/cmd_fdt.c:11: + from ../common/fdt_support.c:10: + from ../common/cmd_flash.c:11: + from ../drivers/net/phy/broadcom.c:9: + from ../drivers/mtd/ubi/misc.c:11: + from ../common/cmd_gpio.c:9: + from ../drivers/net/phy/marvell.c:9: + from ../drivers/net/phy/micrel.c:10: + from ../common/cmd_irq.c:7: + from ../drivers/mtd/ubi/debug.c:9: + from ../common/cmd_itest.c:15: + from ../drivers/net/phy/realtek.c:9: + from ../include/miiphy.h:18, + from ../drivers/net/phy/vitesse.c:9: + from ../common/cmd_jffs2.c:74: + from ../common/cmd_load.c:11: + from ../common/cmd_mem.c:14: + from ../common/cmd_mfsl.c:13: + from ../common/miiphyutil.c:13: + from ../common/cmd_mii.c:12: + from ../common/cmd_mdio.c:12: + from ../drivers/serial/serial.c:8: + from ../common/cmd_misc.c:11: + from ../drivers/spi/spi.c:7: + from ../common/cmd_mtdparts.c:74: + from ../drivers/serial/serial_xuartlite.c:11: + from ../common/cmd_pcmcia.c:40: + from ../common/cmd_net.c:11: + from ../common/cmd_test.c:17: + from ../common/cmd_ubi.c:14: + from ../common/cmd_ximg.c:15: + from ../common/cmd_spl.c:8: + from ../common/flash.c:10: + from ../common/splash.c:23: + from ../common/cmd_nvedit.c:27: + from ../common/env_common.c:11: + from ../common/console.c:8: + from ../common/dlmalloc.c:1: + from ../common/malloc_simple.c:9: + from ../common/image.c:11: + from ../fs/fs.c:17: + from ../common/image-fdt.c:12: + from ../common/image-fit.c:17: + from ../common/memsize.c:8: + from ../lib/zlib/zlib.c:15: + from ../fs/ext4/ext4_common.c:22: + from ../fs/ext4/ext4fs.c:24: + from ../common/stdio.c:12: + from ../common/cli_simple.c:12: + from ../common/cli.c:12: + from ../common/cli_readline.c:12: + from ../common/command.c:12: + from ../common/s_record.c:8: + from ../common/xyzModem.c:25: + from ../fs/ext4/dev.c:26: + from ../common/cmd_disk.c:7: + from ../common/env_embedded.c:12: + from ../net/checksum.c:11: + from ../fs/fat/fat.c:12: + from ../net/arp.c:12: + from ../fs/jffs2/compr_rtime.c:48: + from ../net/bootp.c:11: + from ../net/eth.c:8: + from ../fs/jffs2/compr_rubin.c:41: + from ../net/net.c:84: + # define CONFIG_SYS_MALLOC_F_LEN 0x150 +spl/include/generated/autoconf.h:20:0: note: this is the location of the previous definition + from ../fs/jffs2/compr_zlib.c:38: + from ../fs/jffs2/jffs2_1pass.c:114: + from ../net/nfs.c:25: + from ../net/ping.h:15, + from ../net/ping.c:12: + from ../net/tftp.c:9: + from ../arch/microblaze/cpu/spl.c:9: + from ../lib/fdtdec_common.c:12: + from ../lib/gunzip.c:8: + from ../lib/fdtdec.c:7: + from ../lib/initcall.c:7: + from ../lib/lmb.c:10: + from ../fs/jffs2/mini_inflate.c:12: + from ../fs/fat/file.c:12: + from ../lib/md5.c:31: + from ../common/spl/spl.c:9: + from ../lib/net_utils.c:13: + from ../lib/sha1.c:33: + from ../lib/qsort.c:19: + from ../common/spl/spl_nor.c:7: + from ../lib/sha256.c:10: + from ../lib/strmhz.c:7: +In file included from ././include/config.h:4:0, + from ../lib/hashtable.c:32: + from ../lib/display_options.c:8: + from ../lib/crc32.c:14: + from ../lib/rbtree.c:16: + from ../lib/list_sort.c:7: + from ../lib/hang.c:11: + from ../lib/linux_compat.c:2: + from ../lib/time.c:8: + from ../lib/vsprintf.c:20: + from ../examples/standalone/stubs.c:1: + from ../examples/standalone/hello_world.c:8: w+../include/configs/microblaze-generic.h:117:0: warning: "CONFIG_SYS_MALLOC_F_LEN" redefined [enabled by default] w+../include/configs/microblaze-generic.h:120:0: warning: "CONFIG_SYS_MALLOC_F_LEN" redefined [enabled by default] 03: kconfig: Adjust ordering so that defaults work as expected

Best Regards Masahiro Yamada

Hi Masahiro,

On 19 February 2015 at 22:43, Masahiro Yamada yamada.m@jp.panasonic.com wrote:

Hi Simon,

On Thu, 19 Feb 2015 16:56:05 -0700 Simon Glass sjg@chromium.org wrote:

...

Hi Masahiro,

On 19 February 2015 at 16:46, Masahiro YAMADA yamada.m@jp.panasonic.com wrote:

...

Simon,

2015-02-19 23:08 GMT+09:00 Simon Glass sjg@chromium.org:

...

This should be a bool, not hex.

Signed-off-by: Simon Glass sjg@chromium.org Reported-by: Alexey Brodkin abrodkin@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@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@oscar:~/workspace/u-boot$ make beaver_defconfig # # configuration written to .config # # # configuration written to spl/.config # masahiro@oscar:~/workspace/u-boot$ make menuconfig

Go to "General setup" and disable "Enable malloc() pool before relocation" and save.

masahiro@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