[PATCH] imx: bootaux: respect ELF support configuration

[Max Krummenacher]

Hi Cody

Am Samstag, den 26.06.2021, 22:01 -0700 schrieb Cody Gray:
> When ELF support is disabled in the configuration (CONFIG_CMD_ELF),
> the arch_auxiliary_core_up() function should not check whether the
> specified address contains a valid ELF image. Attempting to do so
> by calling the valid_elf_image() function results in a linker error,
> since the definition for this function will not be available.
> 
> This changeset conditionally includes the ELF-related code in the
> i.MX auxiliary core booter logic ONLY if ELF support is enabled
> in the configuration (i.e., if CONFIG_CMD_ELF is defined).
> 
> If ELF support is not defined, then the logic is the same as if the
> image at the address was not a valid ELF image: it assumes a binary
> file with a vector table at the beginning.
> 
> Signed-off-by: Cody Gray <cody at codygray.com>
> ---
> 
> This is my first patch submission, so I appreciate your patience if
> I've made any mistakes. Regarding code style, there are multiple ways
> that the "if/else" blocks and opening braces could be formatted with
> respect to the preprocessor conditional statements. I made the
> deliberate choice to keep the number of changed lines as few as
> possible, and not to repeat any code, but that does mean that the
> indentation level is wrong if the conditionally-included blocks are
> not considered. This is not addressed specifically in the U-Boot code
> style document, and I didn't see any similar instances in the existing
> code files. If the maintainers would prefer a different formatting,
> please let me know.
> 
> ---
> 
>  arch/arm/mach-imx/imx_bootaux.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/imx_bootaux.c b/arch/arm/mach-imx/imx_bootaux.c
> index 30fb45d48c..a895ace9cb 100644
> --- a/arch/arm/mach-imx/imx_bootaux.c
> +++ b/arch/arm/mach-imx/imx_bootaux.c
> @@ -90,6 +90,7 @@ int arch_auxiliary_core_up(u32 core_id, ulong addr)
>         stack = *(u32 *)addr;
>         pc = *(u32 *)(addr + 4);
>  #else
> +#ifdef CONFIG_CMD_ELF

Wouldn't it make the code cleaner, if the existing `#ifdef CONFIG_IMX8M`
which also excludes the elf
path would be reused, e.g. with a change to:

#if !defined(CONFIG_CMD_ELF) || defined(CONFIG_IMX8M)

(or to remove the `#ifdef CONFIG_IMX8M` pattern and integrate its effects into
your pre-processor pattern.)

Max

>         /*
>          * handling ELF64 binaries
>          * isn't supported yet.
> @@ -101,6 +102,7 @@ int arch_auxiliary_core_up(u32 core_id, ulong addr)
>                         return CMD_RET_FAILURE;
> 
>         } else {
> +#endif
>                 /*
>                  * Assume binary file with vector table at the beginning.
>                  * Cortex-M4 vector tables start with the stack pointer (SP)
> @@ -108,7 +110,9 @@ int arch_auxiliary_core_up(u32 core_id, ulong addr)
>                  */
>                 stack = *(u32 *)addr;
>                 pc = *(u32 *)(addr + 4);
> +#ifdef CONFIG_CMD_ELF
>         }
> +#endif
>  #endif
>         printf("## Starting auxiliary core stack = 0x%08lX, pc = 0x%08lX...\n",
>                stack, pc);