[U-Boot] [PATCH 3/4] nand_spl/spiboot/sdboot: Add ability to have smaller middle loader

[Wolfgang Denk]

Dear y at right.am.freescale.net,

In message <1307042852-10385-3-git-send-email-y> you wrote:
> 
> This patch allows some boards do define CONFIG_BOOTSTRAP to let them
> build a MPL or middle program loader to setup the board before booting
> to the full u-boot build. The advantage is that we are using the same
> build system and linker scripts that would be used in a normal u-boot
> build.

The intention is OK, but I disagree with the implementation.

> +ifndef CONFIG_IN_BOOTSTRAP
> +ifeq ($(CONFIG_SPIFLASH), y)
> +ALL += $(obj)u-boot-spi.bin
> +endif
> +
> +ifeq ($(CONFIG_SDCARD), y)
> +ALL += $(obj)u-boot-sd.bin
> +endif
> +endif

I really dislike to have this in the top level Makefile. We want to
make it simpler, less board specific. and this is the opposite.

I doubt you always need all these images.  If you need u-boot-sd.bin,
then just type "make u-boot-sd.bin".

>  nand_spl:	$(TIMESTAMP_FILE) $(VERSION_FILE) depend
>  		$(MAKE) -C nand_spl/board/$(BOARDDIR) all
>  
> +NAND_SPL_OBJS-y += $(obj)nand_spl/u-boot-spl-16k.bin
> +NAND_SPL_OBJS-$(CONFIG_BOOTSTRAP) += $(obj)u-boot-bootstrap.bin
> +NAND_SPL_OBJS-y += $(obj)u-boot.bin

This looks pretty much board specific to me.  I doubt all boards can
use u-boot-spl-16k.bin here.

> +ifeq ($(CONFIG_BOOTSTRAP),y)
> +$(obj)u-boot-nand.bin:	nand_spl $(obj)u-boot-bootstrap.bin $(obj)u-boot.bin
> +else
...
> +$(obj)u-boot-bootstrap.bin:
> +		rm -rf $(BUILD_DIR)bootstrap
> +		$(MAKE) $(CONFIG_BOARD_NAME) O=bootstrap/ EXTRA_OPTS=IN_BOOTSTRAP
> +		cp $(BUILD_DIR)bootstrap/u-boot.bin u-boot-bootstrap.bin

Argh.. how many different images are you going to add?  This is
becoming a mess.

Do we really need all this in the top level Makefile?

> diff --git a/mkconfig b/mkconfig
> index e72ec3d..74294ec 100755
> --- a/mkconfig
> +++ b/mkconfig
> @@ -159,6 +159,7 @@ for i in ${TARGETS} ; do
>  done
>  
>  cat << EOF >> config.h
> +#define CONFIG_BOARD_NAME $BOARD_NAME
>  #define CONFIG_BOARDDIR board/$BOARDDIR
>  #include <config_cmd_defaults.h>
>  #include <config_defaults.h>

NAK. This breaks existing code, also it should not be needed.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I see that Microsoft's campaign  to  destroy  all  knowledge  of  any
operating   environment   but  its  own  environment-of-the-year  has
succeeded in creating a generation of users who don't understand  the
concept of a shell...
            -- L. Peter Deutsch in <m0x5jNX-000R2UC at lamp.aladdin.com>