Re: [ImageBuilder][PATCH v2 3/4] uboot-script-gen: Enable appending extra commands to boot script

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 30 Jun 2022, Andrei Cherechesu (OSS) wrote:
> From: Andrei Cherechesu <andrei.cherechesu@xxxxxxx>
> 
> Added the "-a" parameter which stands for APPEND_EXTRA_CMDS option,
> which enables the user to specify the path to a text file that contains,
> on each line, u-boot commands that will be added to the generated script as
> "fixups", before the boot command.
> 
> The file specified via the "-a" parameter will be copied as-is in the
> generated script.
> 
> Signed-off-by: Andrei Cherechesu <andrei.cherechesu@xxxxxxx>
> ---
>  scripts/uboot-script-gen | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> index f8d2fb0..444c65a 100755
> --- a/scripts/uboot-script-gen
> +++ b/scripts/uboot-script-gen
> @@ -416,6 +416,10 @@ function check_file_type()
>      elif [ "$type" = "Device Tree Blob" ]
>      then
>          type="Device Tree Blob\|data"
> +
> +    elif [ "$type" = "text" ]
> +    then
> +        type="ASCII text"
>      fi
>  
>      file -L $filename | grep "$type" &> /dev/null
> @@ -973,7 +977,7 @@ function print_help
>  {
>      script=`basename "$0"`
>      echo "usage:"
> -    echo "   $script -c CONFIG_FILE -d DIRECTORY [-t LOAD_CMD] [-o FILE] [-k 
> KEY_DIR/HINT [-u U-BOOT_DTB]] [-e] [-f] [-p PREPEND_PATH] [-s]"
> +    echo "   $script -c CONFIG_FILE -d DIRECTORY [-t LOAD_CMD] [-o FILE] [-k 
> KEY_DIR/HINT [-u U-BOOT_DTB]] [-e] [-f] [-p PREPEND_PATH] [-s] [-a 
> APPEND_EXTRA_CMDS]"
>      echo "   $script -h"
>      echo "where:"
>      echo "   CONFIG_FILE - configuration file"
> @@ -991,6 +995,7 @@ function print_help
>      echo "   -f - enable generating a FIT image"
>      echo "   PREPEND_PATH - path to be appended before file names to match 
> deploy location within rootfs"
>      echo "   -s - enable dynamic loading of binaries by storing their 
> addresses and sizes u-boot env variables"
> +    echo "   APPEND_EXTRA_CMDS - absolute path to file containing extra 
> u-boot cmds (fixups) to be run before booting"
>      echo "   -h - prints out the help message and exits "
>      echo "Defaults:"
>      echo "   CONFIG_FILE=$cfg_file, UBOOT_TYPE=\"LOAD_CMD\" env var, 
> DIRECTORY=$uboot_dir"

Instead of adding it as a command line option, would it make sense to
just source it from the ImageBuilder config file? Like:

---
UBOOT_SOURCE="boot.source"
UBOOT_SCRIPT="boot.scr"
APPEND_EXTRA_CMDS="extra.txt"
---

In other words I think we can skip parsing "-a" and just check if
APPEND_EXTRA_CMDS is set


> @@ -998,7 +1003,7 @@ function print_help
>      echo "   $script -c ../config -d ./build42 -t \"scsi load 1:1\""
>  }
>  
> -while getopts ":c:t:d:ho:k:u:fp:s" opt; do
> +while getopts ":c:t:d:ho:k:u:fp:sa:" opt; do
>      case ${opt} in
>      t )
>          case $OPTARG in
> @@ -1043,6 +1048,9 @@ while getopts ":c:t:d:ho:k:u:fp:s" opt; do
>      s )
>          dynamic_loading_opt=y
>          ;;
> +    a )
> +        extra_cmds_file=$OPTARG
> +        ;;
>      h )
>          print_help
>          exit 0
> @@ -1235,6 +1243,13 @@ load_file $DEVICE_TREE "host_fdt"
>  bitstream_load_and_config  # bitstream is loaded last but used first
>  device_tree_editing $device_tree_addr
>  
> +# append extra u-boot commands (fixups) to script before boot command
> +if test "$extra_cmds_file"
> +then
> +    check_file_type "$extra_cmds_file" "text"
> +    cat $extra_cmds_file >> $UBOOT_SOURCE
> +fi

I think it should be below the setenv fdt_high command below. (Just
before the echo "$BOOT_CMD...

Other than that, it is fine, thank you!


>  # disable device tree reloation
>  echo "setenv fdt_high 0xffffffffffffffff" >> $UBOOT_SOURCE
>  
> -- 
> 2.35.1
>