Commit 061d6782756f modified load_file() to take load command as
argument but did not change all the invocations (e.g. loading standalone
Linux, bitstream, etc.) which broke the output script (load command
empty). Fix it by defaulting to LOAD_CMD if not specified.

Fixes: 061d6782756f ("Add config option to use separate load commands for Xen, DOM0 and DOMU binaries")
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
 scripts/uboot-script-gen | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
index 849b8f939e81..4f9261035d73 100755
--- a/scripts/uboot-script-gen
+++ b/scripts/uboot-script-gen
@@ -736,6 +736,12 @@ function load_file()
     local base="$(realpath $PWD)"/
     local relative_path=${absolute_path#"$base"}
 
+    # Default to LOAD_CMD if not specified
+    if test -z "${load_cmd}"
+    then
+        load_cmd="${LOAD_CMD}"
+    fi
+
     if test "$FIT"
     then
         echo "imxtract \$fit_addr $fit_scr_name $memaddr" >> $UBOOT_SOURCE
-- 
2.43.0

On Tue, Sep 09, 2025 at 09:41:41AM +0200, Michal Orzel wrote:
> Commit 061d6782756f modified load_file() to take load command as
> argument but did not change all the invocations (e.g. loading standalone
> Linux, bitstream, etc.) which broke the output script (load command
> empty). Fix it by defaulting to LOAD_CMD if not specified.
> 
> Fixes: 061d6782756f ("Add config option to use separate load commands for Xen, DOM0 and DOMU binaries")
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Reviewed-by: Denis Mukhin <dmukhin@ford.com> 

> ---
>  scripts/uboot-script-gen | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> index 849b8f939e81..4f9261035d73 100755
> --- a/scripts/uboot-script-gen
> +++ b/scripts/uboot-script-gen
> @@ -736,6 +736,12 @@ function load_file()
>      local base="$(realpath $PWD)"/
>      local relative_path=${absolute_path#"$base"}
>  
> +    # Default to LOAD_CMD if not specified
> +    if test -z "${load_cmd}"
> +    then
> +        load_cmd="${LOAD_CMD}"
> +    fi
> +
>      if test "$FIT"
>      then
>          echo "imxtract \$fit_addr $fit_scr_name $memaddr" >> $UBOOT_SOURCE
> -- 
> 2.43.0
> 
>

On Tue, 9 Sep 2025, dmukhin@xen.org wrote:
> On Tue, Sep 09, 2025 at 09:41:41AM +0200, Michal Orzel wrote:
> > Commit 061d6782756f modified load_file() to take load command as
> > argument but did not change all the invocations (e.g. loading standalone
> > Linux, bitstream, etc.) which broke the output script (load command
> > empty). Fix it by defaulting to LOAD_CMD if not specified.
> > 
> > Fixes: 061d6782756f ("Add config option to use separate load commands for Xen, DOM0 and DOMU binaries")
> > Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> 
> Reviewed-by: Denis Mukhin <dmukhin@ford.com> 

Acked-by: Stefano Stabellini <sstabellini@kernel.org>

Hi Michal,

Thank you for the patch and the detailed explanation.

On Tue, Sep 9, 2025 at 10:42 AM Michal Orzel <michal.orzel@amd.com> wrote:
>
> Commit 061d6782756f modified load_file() to take load command as
> argument but did not change all the invocations (e.g. loading standalone
> Linux, bitstream, etc.) which broke the output script (load command
> empty). Fix it by defaulting to LOAD_CMD if not specified.
>
> Fixes: 061d6782756f ("Add config option to use separate load commands for Xen, DOM0 and DOMU binaries")
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> ---
>  scripts/uboot-script-gen | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> index 849b8f939e81..4f9261035d73 100755
> --- a/scripts/uboot-script-gen
> +++ b/scripts/uboot-script-gen
> @@ -736,6 +736,12 @@ function load_file()
>      local base="$(realpath $PWD)"/
>      local relative_path=${absolute_path#"$base"}
>
> +    # Default to LOAD_CMD if not specified
> +    if test -z "${load_cmd}"
> +    then
> +        load_cmd="${LOAD_CMD}"
> +    fi
> +

I was wondering if we could use a slightly more concise notation here, like:
: "${load_cmd:=$LOAD_CMD}"

It does the same thing but is a bit more idiomatic for Bash scripts.

>      if test "$FIT"
>      then
>          echo "imxtract \$fit_addr $fit_scr_name $memaddr" >> $UBOOT_SOURCE
> --
> 2.43.0
>
>

Thanks again for your work on this!

Best regards,
Mykola

On 09/09/2025 11:22, Mykola Kvach wrote:
> Hi Michal,
> 
> Thank you for the patch and the detailed explanation.
> 
> On Tue, Sep 9, 2025 at 10:42 AM Michal Orzel <michal.orzel@amd.com> wrote:
>>
>> Commit 061d6782756f modified load_file() to take load command as
>> argument but did not change all the invocations (e.g. loading standalone
>> Linux, bitstream, etc.) which broke the output script (load command
>> empty). Fix it by defaulting to LOAD_CMD if not specified.
>>
>> Fixes: 061d6782756f ("Add config option to use separate load commands for Xen, DOM0 and DOMU binaries")
>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>> ---
>>  scripts/uboot-script-gen | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
>> index 849b8f939e81..4f9261035d73 100755
>> --- a/scripts/uboot-script-gen
>> +++ b/scripts/uboot-script-gen
>> @@ -736,6 +736,12 @@ function load_file()
>>      local base="$(realpath $PWD)"/
>>      local relative_path=${absolute_path#"$base"}
>>
>> +    # Default to LOAD_CMD if not specified
>> +    if test -z "${load_cmd}"
>> +    then
>> +        load_cmd="${LOAD_CMD}"
>> +    fi
>> +
> 
> I was wondering if we could use a slightly more concise notation here, like:
> : "${load_cmd:=$LOAD_CMD}"
> 
> It does the same thing but is a bit more idiomatic for Bash scripts.
Some time ago, Stefano requested me to use a simpler notation in ImageBuilder,
so that it's immediately clear what the script does. Therefore I followed this
suggestion here as well. I will let him choose what suits the project best.

~Michal

On Tue, 9 Sep 2025, Orzel, Michal wrote:
> On 09/09/2025 11:22, Mykola Kvach wrote:
> > Hi Michal,
> > 
> > Thank you for the patch and the detailed explanation.
> > 
> > On Tue, Sep 9, 2025 at 10:42 AM Michal Orzel <michal.orzel@amd.com> wrote:
> >>
> >> Commit 061d6782756f modified load_file() to take load command as
> >> argument but did not change all the invocations (e.g. loading standalone
> >> Linux, bitstream, etc.) which broke the output script (load command
> >> empty). Fix it by defaulting to LOAD_CMD if not specified.
> >>
> >> Fixes: 061d6782756f ("Add config option to use separate load commands for Xen, DOM0 and DOMU binaries")
> >> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> >> ---
> >>  scripts/uboot-script-gen | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> >> index 849b8f939e81..4f9261035d73 100755
> >> --- a/scripts/uboot-script-gen
> >> +++ b/scripts/uboot-script-gen
> >> @@ -736,6 +736,12 @@ function load_file()
> >>      local base="$(realpath $PWD)"/
> >>      local relative_path=${absolute_path#"$base"}
> >>
> >> +    # Default to LOAD_CMD if not specified
> >> +    if test -z "${load_cmd}"
> >> +    then
> >> +        load_cmd="${LOAD_CMD}"
> >> +    fi
> >> +
> > 
> > I was wondering if we could use a slightly more concise notation here, like:
> > : "${load_cmd:=$LOAD_CMD}"
> > 
> > It does the same thing but is a bit more idiomatic for Bash scripts.
> Some time ago, Stefano requested me to use a simpler notation in ImageBuilder,
> so that it's immediately clear what the script does. Therefore I followed this
> suggestion here as well. I will let him choose what suits the project best.

I prefer the way Michal wrote it.

This is one of those cases where there is no right or wrong answer.
Mykola's suggestion is a more modern and concise version. The code style
I used was an attempt to be more verbose but also clearer. As always,
when it comes to clarity, each individual finds different approaches
more understandable.