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

[McClintock Matthew-B29882]

On Sat, Jun 4, 2011 at 7:32 AM, Wolfgang Denk <wd at denx.de> wrote:
>> +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.

Perhaps having a CONFIG_TARGET_IMAGE available and having just one
generic TARGET available?

ifdef CONFIG_TARGET_IMAGE
ALL += $(CONFIG_TARGET_IMAGE)
endif

TARGET_IMAGE_OBJS-y += various.bin
TARGET_IMAGE_OBJS-y += required.bin
TARGET_IMAGE_OBJS-y += blobs.bin

$(obj)$(CONFIG_TARGET_IMAGE):   $(TARGET_IMAGE_OBJS-y)
                cat $(TARGET_IMAGE_OBJS-y) > $(obj)$(CONFIG_TARGET_IMAGE)

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

We choose these targets by adding a config line in boards.cfg. We
don't build them unless we do a make BOARD_{NAND,SDCARD,SPIFLASH}. We
could optionally just require the user to type out the target manually
or do what I mentioned above however, I strongly believe these images
should always be built as we don't want end user confusion about which
image to program.

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

This u-boot-spl-16k.bin reference has been here before this patch. It
is typically just the u-boot-nand image to be used and not actually
required to be 16k. It could probably be updated to just be
u-boot-spl.bin except that for some boards this is not the right
image. Usually the 16k one points at the right image.

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

I'm not sure there is a great way around this. I played around with
trying to include them in the config.mk or a board/cpu/arch Makefile
(I'll admit my lack of Makefile mastery here) but what seems to work
best is something like this to add "other" targets that don't need to
reside in the top level Makefile. This is best because targets depend
on things in the toplevel Makefile.

sinclude $(obj)arch/$(ARCH)/Makefile.targets
sinclude $(obj)arch/$(ARCH)/cpu/$(CPU)/Makefile.targets

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

How does this break existing code? Because a specific board might also
defined this? I can drop it and add it to the board config file
instead, but it seemed easier to programmatically do this so one
target can build another target. This combined with the previous patch
lets one build invoke another build (then we can add an additional
option via EXTRA_OPTS from the previous patch)

-M