Re: [Buildroot] [PATCH v2 1/5] arch/Config.in.xtensa: adjust endianness logic to avoid bogus configurations

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Thomas, All,

On 2022-09-25 11:09 +0200, Thomas Petazzoni spake thusly:
> The Xtensa architecture supports both LE and BE configurations. When
> BR2_XTENSA_CUSTOM is chosen, a choice in menuconfig to choose between
> LE and BE.
> 
> However, if using the internal toolchain backend, when the
> BR2_XTENSA_OVERLAY_FILE is empty, in practice, the configuration will
> always be big endian. But the choice being present, random
> configuration testing ends up testing configurations where
> BR2_XTENSA_OVERLAY_FILE is empty, but the endianness selected is LE,
> which is incorrect.
> 
> This commit fixes this by preventing the selection of
> BR2_XTENSA_BIG_ENDIAN when an internal toolchain is used with an empty
> overlay file.
> 
> This commit will allow reverting
> 4cbf7336914f25478aea943456ba7dc3c892c21a ("arch/xtensa: custom
> configuration requires an overlay") which broke the build for external
> toolchains that use a custom Xtensa core.
> 
> Fixes (even though currently hidden by commit
> 4cbf7336914f25478aea943456ba7dc3c892c21a):
> 
>   http://autobuild.buildroot.net/results/702e2886156f291466375dfcf412c20f1aa5857d/
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  arch/Config.in.xtensa | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/Config.in.xtensa b/arch/Config.in.xtensa
> index f9d5adb4c9..4bb58e1964 100644
> --- a/arch/Config.in.xtensa
> +++ b/arch/Config.in.xtensa
> @@ -37,6 +37,10 @@ config BR2_XTENSA_LITTLE_ENDIAN
>  
>  config BR2_XTENSA_BIG_ENDIAN
>  	bool "Big endian"
> +	# Internal toolchains without an overlay file are always
> +	# little-endian, so we prevent from selecting big endian in
> +	# this case.
> +	depends on BR2_XTENSA_OVERLAY_FILE != "" || BR2_TOOLCHAIN_EXTERNAL

I think that dependency should go to the whole choice instead:

    choice
        prompt "Target Architecture Endianness"
        default BR2_XTENSA_LITTLE_ENDIAN
        depends on BR2_XTENSA_CUSTOM || BR2_TOOLCHAIN_EXTERNAL

This should cover the external toolchain case, and the internal
toolchain for a custom core (assuming we keep the no-empty overlay for
custom cores).

I.e. semantically, choosing between big or little is not about having or
not having an overlay, but about being a custom core or not (or using a
custom toolchain).

That way, even for gdb and the kernel, with an external toolchain, we
still allow building the; with an empty overlay.

Regards,
Yann E. MORIN.

>  endchoice
>  
> -- 
> 2.37.3
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot