[PATCH] imx: bootaux: respect ELF support configuration

[PATCH] imx: bootaux: respect ELF support configuration

[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@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
        /*
         * 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);
-- 
2.17.1

Re: [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@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);