[U-Boot] [PATCH v3 1/2] distro_bootcmd: add spi flash boot command

[André Przywara]

On 01/08/2019 21:07, Oskari Lemmela wrote:

Hi,

> Add a boot command to distro boot to support load FIT image
> from SPI flash.

I think I mentioned this before, but I have my reservations about
introducing this to *distro_bootcmd*. IIUC, this feature is explicitly
about providing easy support for generic distributions to point U-Boot
to their kernel, without board-specific knowledge. Putting a FIT image
with a specific .dtb and initrd into a certain address in some SPI flash
does not fall into this category. Instead this sound like a valid, but
quite specific embedded use case.

If you want to pull this off for your board(s), why not just set bootcmd
to your specific instructions, then use "saveenv" to store this on the
SPI flash env?
This sounds easier and doesn't disturb the generic boot process.

If you are after easily creating an image file, I had some success
lately with adding a board specific environment into an extra
environment image file, which would save you from both tweaking the
default environment in source, also avoiding to do the modification live
on a board:

#!/bin/sh
addr=$(readelf -s u-boot | grep default_environment | tr -s ' ' | cut
-d\  -f3)
size=$(readelf -s u-boot | grep default_environment | tr -s ' ' | cut
-d\  -f4)
offset=$((0x$addr - 0x4a000000))
(dd if=u-boot.bin bs=1 skip=$offset count=$size status=none | tr '\0'
'\n' | grep -v '^$' | grep -v '^bootcmd='; cat myown_additions.env.txt)
| tools/mkenvimage -p 0 -s 65536 -o - -
(Opening the nerd competition to find an better combination of UNIX
tools to achieve the same purpose ;-)

The resulting file can be dumped as the external environment into
whatever place you configured U-Boot to look for that, next to
u-boot-sunxi-with-spl.bin, for instance into SPI flash.
So the distro_bootcmd stays clean in the source, yet you can automate
the boot on your specific board by flashing just one image file.

> Tested with Pine A64-LTS board to load kernel and dtb.
> 
> Signed-off-by: Oskari Lemmela <oskari at lemmela.net>
> ---
>  Kconfig                         | 32 ++++++++++++++++++++++++++++++++
>  include/config_distro_bootcmd.h | 16 ++++++++++++++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/Kconfig b/Kconfig
> index d2eb744e70..391ce58810 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -395,6 +395,38 @@ config FIT_IMAGE_POST_PROCESS
>  	  injected into the FIT creation (i.e. the blobs would have been pre-
>  	  processed before being added to the FIT image).
>  
> +config FIT_SPI_FLASH_BOOT
> +	bool "Distro bootcmd from SPI Flash"
> +	depends on DM_SPI_FLASH
> +	imply CMD_SF
> +	help
> +	  Enable SPI bootcmd to load FIT image from SPI flash. Command
> +	  probes for the SPI flash device. If device is found command
> +	  will try to load FIT image to memory and boot it.
> +
> +config FIT_FLASH_IMAGE_ADDR
> +	hex "FIT image memory address"
> +	depends on FIT_SPI_FLASH_BOOT
> +	help
> +	  Memory address where to load FIT image.
> +
> +config FIT_FLASH_IMAGE_OFFSET
> +	hex "FIT image flash offset"
> +	depends on FIT_SPI_FLASH_BOOT
> +	default 0x100000
> +	help
> +	  Flash offset of FIT image. Default 1MB offset leaves space for u-boot
> +	  itself.
> +
> +config FIT_FLASH_IMAGE_SIZE
> +	hex "FIT image size"
> +	depends on FIT_SPI_FLASH_BOOT
> +	default 0x700000
> +	help
> +	  Maximum size of the FIT image. Default value of 7MB is enough for
> +	  small compressed kernel, dtb and ramdisk. Reducing offset and image
> +	  size allows to use smaller than 8MB flash devices.
> +
>  if SPL
>  
>  config SPL_FIT
> diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
> index 3570a32dff..8c6376ccab 100644
> --- a/include/config_distro_bootcmd.h
> +++ b/include/config_distro_bootcmd.h
> @@ -280,6 +280,22 @@
>  	BOOT_TARGET_DEVICES_references_USB_without_CONFIG_CMD_USB
>  #endif
>  
> +#ifdef CONFIG_FIT_SPI_FLASH_BOOT
> +#define BOOT_TARGET_DEVICES_SPI(func) func(SPI, spi, 0)
> +#define BOOTENV_DEV_SPI(devtypeu, devtypel, instance) \
> +	"image_addr_r=" __stringify(CONFIG_FIT_FLASH_IMAGE_ADDR) "\0" \
> +	"image_addr=" __stringify(CONFIG_FIT_FLASH_IMAGE_OFFSET) "\0" \
> +	"image_size=" __stringify(CONFIG_FIT_FLASH_IMAGE_SIZE) "\0" \

I think this part gives it away: Here you set board specific flash
offset addresses in the generic config_distro_bootcmd.h file.
IMHO this contradicts the whole effort of distro_bootcmd.

Cheers,
Andre.

> +	"bootcmd_" #devtypel #instance "=" \
> +		"if sf probe " #instance "; then " \
> +			"sf read ${image_addr_r} ${image_addr} ${image_size}; " \
> +			"bootm ${image_addr_r}; " \
> +	"fi\0"
> +#define BOOTENV_DEV_NAME_SPI	BOOTENV_DEV_NAME_BLKDEV
> +#else
> +#define BOOT_TARGET_DEVICES_SPI(func)
> +#endif
> +
>  #ifdef CONFIG_CMD_VIRTIO
>  #define BOOTENV_RUN_VIRTIO_INIT "run virtio_init; "
>  #define BOOTENV_SET_VIRTIO_NEED_INIT "virtio_need_init=; "
>