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

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

[Thomas Petazzoni via buildroot]

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:

 (1) Showing the overlay file option only when building an internal
     toolchain. For external toolchain configurations, it does not
     make sense to show it as it will have no effect.

 (2) Showing the endianness choice either for internal toolchain
     configurations with non-empty BR2_XTENSA_OVERLAY_FILE, or for
     external toolchain configurations.

 (2) Defaulting to BE when BR2_XTENSA_OVERLAY_FILE is empty.

Fixes:

  http://autobuild.buildroot.net/results/702e2886156f291466375dfcf412c20f1aa5857d/

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 arch/Config.in.xtensa | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/Config.in.xtensa b/arch/Config.in.xtensa
index f9d5adb4c9..e9263f888e 100644
--- a/arch/Config.in.xtensa
+++ b/arch/Config.in.xtensa
@@ -14,7 +14,7 @@ endchoice
 
 config BR2_XTENSA_OVERLAY_FILE
 	string "Overlay file for custom configuration"
-	depends on BR2_XTENSA_CUSTOM
+	depends on BR2_XTENSA_CUSTOM && BR2_TOOLCHAIN_BUILDROOT
 	help
 	  Enter the path to the overlay tarball for a custom processor
 	  configuration.
@@ -31,6 +31,7 @@ choice
 	prompt "Target Architecture Endianness"
 	default BR2_XTENSA_LITTLE_ENDIAN
 	depends on BR2_XTENSA_CUSTOM
+	depends on BR2_XTENSA_OVERLAY_FILE != "" || BR2_TOOLCHAIN_EXTERNAL
 
 config BR2_XTENSA_LITTLE_ENDIAN
 	bool "Little endian"
@@ -51,7 +52,9 @@ config BR2_XTENSA_USE_MMU
 
 config BR2_ENDIAN
 	default "LITTLE"	if BR2_XTENSA_LITTLE_ENDIAN
-	default "BIG"		if BR2_xtensa_fsf || BR2_XTENSA_BIG_ENDIAN
+	default "BIG"		if BR2_XTENSA_BIG_ENDIAN
+	default "BIG"		if BR2_xtensa_fsf
+	default "BIG"		if BR2_XTENSA_CUSTOM && BR2_XTENSA_OVERLAY_FILE = ""
 
 config BR2_ARCH
 	default "xtensa"	if BR2_xtensa
-- 
2.37.3

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 2/4] Revert "arch/xtensa: custom configuration requires an overlay"

[Thomas Petazzoni]

This reverts commit 4cbf7336914f25478aea943456ba7dc3c892c21a.

This commit breaks the build of BR2_XTENSA_CUSTOM=y configurations
that use an external toolchain. In such configurations, having an
empty overlay is perfectly fine. For example, this commit broke the
following two runtime tests:

  tests.toolchain.test_external_bootlin.TestExternalToolchainBootlinXtensalx60UclibcBleedingEdge
  tests.toolchain.test_external_bootlin.TestExternalToolchainBootlinXtensalx60UclibcStable

And in fact, having an empty overlay is even correct when building an
internal toolchain. The endianness issue that
4cbf7336914f25478aea943456ba7dc3c892c21a was attempting to fix has
been fixed in a better way by the previous commit.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 arch/arch.mk.xtensa | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/arch.mk.xtensa b/arch/arch.mk.xtensa
index 7b6c59cecd..fd410f6bfa 100644
--- a/arch/arch.mk.xtensa
+++ b/arch/arch.mk.xtensa
@@ -1,9 +1,3 @@
-BR_ARCH_XTENSA_OVERLAY_FILE = $(call qstrip,$(BR2_XTENSA_OVERLAY_FILE))
-
-ifeq ($(BR_BUILDING)$(BR2_XTENSA_CUSTOM):$(BR_ARCH_XTENSA_OVERLAY_FILE),yy:)
-$(error No xtensa overlay file provided. Check your BR2_XTENSA_OVERLAY_FILE setting)
-endif
-
 ################################################################################
 # This variable can be used by packages that need to extract the overlay.
 #
@@ -15,6 +9,7 @@ endif
 #       tar xf $(ARCH_XTENSA_OVERLAY_FILE) -C $(@D) --strip-components=1 gcc
 #   endif
 ################################################################################
+BR_ARCH_XTENSA_OVERLAY_FILE = $(call qstrip,$(BR2_XTENSA_OVERLAY_FILE))
 ifneq ($(filter http://% https://% ftp://% scp://%,$(BR_ARCH_XTENSA_OVERLAY_FILE)),)
 ARCH_XTENSA_OVERLAY_URL = $(BR_ARCH_XTENSA_OVERLAY_FILE)
 ARCH_XTENSA_OVERLAY_FILE = $($(PKG)_DL_DIR)/$(notdir $(BR_ARCH_XTENSA_OVERLAY_FILE))
-- 
2.37.3

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 3/4] arch/{Config.in, arch.mk}.xtensa: add explicit support for DC233C core

[Thomas Petazzoni]

The DC233C Xtensa core is the one used by our Qemu Xtensa
defconfigs, and it is a little endian core. It makes sense to promote
it as a supported Xtensa core in arch/Config.in.xtensa, so that our
autobuilder testing covers build testing of little endian Xtensa
configurations. Indeed currently, both the FSF core and the custom
core are big endian (the custom core cannot be little endian without
an overlay).

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 arch/Config.in.xtensa | 6 +++++-
 arch/arch.mk.xtensa   | 6 ++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/Config.in.xtensa b/arch/Config.in.xtensa
index e9263f888e..57b7a5a12a 100644
--- a/arch/Config.in.xtensa
+++ b/arch/Config.in.xtensa
@@ -10,6 +10,9 @@ config BR2_xtensa_fsf
 	bool "fsf - Default configuration"
 	select BR2_USE_MMU
 
+config BR2_xtensa_dc233c
+	bool "dc233c"
+
 endchoice
 
 config BR2_XTENSA_OVERLAY_FILE
@@ -44,7 +47,7 @@ endchoice
 config BR2_XTENSA_USE_MMU
 	bool "MMU support"
 	default y
-	depends on BR2_XTENSA_CUSTOM
+	depends on BR2_XTENSA_CUSTOM || BR2_xtensa_dc233c
 	select BR2_USE_MMU
 	help
 	  Enable this option if your Xtensa core has a MMU (Memory
@@ -52,6 +55,7 @@ config BR2_XTENSA_USE_MMU
 
 config BR2_ENDIAN
 	default "LITTLE"	if BR2_XTENSA_LITTLE_ENDIAN
+	default "LITTLE"	if BR2_xtensa_dc233c
 	default "BIG"		if BR2_XTENSA_BIG_ENDIAN
 	default "BIG"		if BR2_xtensa_fsf
 	default "BIG"		if BR2_XTENSA_CUSTOM && BR2_XTENSA_OVERLAY_FILE = ""
diff --git a/arch/arch.mk.xtensa b/arch/arch.mk.xtensa
index fd410f6bfa..92499f396f 100644
--- a/arch/arch.mk.xtensa
+++ b/arch/arch.mk.xtensa
@@ -9,7 +9,13 @@
 #       tar xf $(ARCH_XTENSA_OVERLAY_FILE) -C $(@D) --strip-components=1 gcc
 #   endif
 ################################################################################
+
+ifeq ($(BR2_xtensa_dc233c),y)
+BR_ARCH_XTENSA_OVERLAY_FILE = https://github.com/jcmvbkbc/xtensa-toolchain-build/raw/95291b7c39e6f790d0b2f062c945a630290f2c81/overlays/xtensa_dc233c.tar.gz
+else ifeq ($(BR2_XTENSA_CUSTOM),y)
 BR_ARCH_XTENSA_OVERLAY_FILE = $(call qstrip,$(BR2_XTENSA_OVERLAY_FILE))
+endif
+
 ifneq ($(filter http://% https://% ftp://% scp://%,$(BR_ARCH_XTENSA_OVERLAY_FILE)),)
 ARCH_XTENSA_OVERLAY_URL = $(BR_ARCH_XTENSA_OVERLAY_FILE)
 ARCH_XTENSA_OVERLAY_FILE = $($(PKG)_DL_DIR)/$(notdir $(BR_ARCH_XTENSA_OVERLAY_FILE))
-- 
2.37.3

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 4/4] configs/qemu_xtensa_lx60*: use new BR2_xtensa_dc233c option

[Thomas Petazzoni]

Now that Xtensa DC233C is directly supported as an Xtensa core in
menuconfig, let's use the new option.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 configs/qemu_xtensa_lx60_defconfig       | 3 +--
 configs/qemu_xtensa_lx60_nommu_defconfig | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/configs/qemu_xtensa_lx60_defconfig b/configs/qemu_xtensa_lx60_defconfig
index cb78106091..845d466caf 100644
--- a/configs/qemu_xtensa_lx60_defconfig
+++ b/configs/qemu_xtensa_lx60_defconfig
@@ -1,7 +1,6 @@
 # Architecture
 BR2_xtensa=y
-BR2_XTENSA_CUSTOM=y
-BR2_XTENSA_OVERLAY_FILE="https://github.com/jcmvbkbc/xtensa-toolchain-build/raw/95291b7c39e6f790d0b2f062c945a630290f2c81/overlays/xtensa_dc233c.tar.gz"
+BR2_xtensa_dc233c=y
 
 # System
 BR2_SYSTEM_DHCP="eth0"
diff --git a/configs/qemu_xtensa_lx60_nommu_defconfig b/configs/qemu_xtensa_lx60_nommu_defconfig
index 4be1b66edc..f94019540f 100644
--- a/configs/qemu_xtensa_lx60_nommu_defconfig
+++ b/configs/qemu_xtensa_lx60_nommu_defconfig
@@ -1,7 +1,6 @@
 # Architecture
 BR2_xtensa=y
-BR2_XTENSA_CUSTOM=y
-BR2_XTENSA_OVERLAY_FILE="https://github.com/jcmvbkbc/xtensa-toolchain-build/raw/95291b7c39e6f790d0b2f062c945a630290f2c81/overlays/xtensa_dc233c.tar.gz"
+BR2_xtensa_dc233c=y
 # BR2_XTENSA_USE_MMU is not set
 
 # Use minimal busybox with hush and networking tools
-- 
2.37.3

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Yann E. MORIN]

Thomas, All,

On 2022-09-24 22:53 +0200, Thomas Petazzoni via buildroot 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:
> 
>  (1) Showing the overlay file option only when building an internal
>      toolchain. For external toolchain configurations, it does not
>      make sense to show it as it will have no effect.

This should be a separate patch.

>  (2) Showing the endianness choice either for internal toolchain
>      configurations with non-empty BR2_XTENSA_OVERLAY_FILE, or for
>      external toolchain configurations.

As an empty BR2_XTENSA_OVERLAY_FILE is not accepted anymore (see below),
this can be simplified to "sharing the endianness chice for internal and
external toolchains".

>  (2) Defaulting to BE when BR2_XTENSA_OVERLAY_FILE is empty.

An empty BR2_XTENSA_OVERLAY_FILE is now no longer accepted, after commit
4cbf7336914f (arch/xtensa: custom configuration requires an overlay).

> Fixes:
>   http://autobuild.buildroot.net/results/702e2886156f291466375dfcf412c20f1aa5857d/

This commit if from August the 15th; I could not find any such failure
since 4cbf7336914f was applied; the last occured 2022-09-08, on commit
d2141f65e449, which is the parent of 4cbf7336914f (there are build
failures on older branches, of course, but not on master).

Regards,
Yann E. MORIN.

> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  arch/Config.in.xtensa | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/Config.in.xtensa b/arch/Config.in.xtensa
> index f9d5adb4c9..e9263f888e 100644
> --- a/arch/Config.in.xtensa
> +++ b/arch/Config.in.xtensa
> @@ -14,7 +14,7 @@ endchoice
>  
>  config BR2_XTENSA_OVERLAY_FILE
>  	string "Overlay file for custom configuration"
> -	depends on BR2_XTENSA_CUSTOM
> +	depends on BR2_XTENSA_CUSTOM && BR2_TOOLCHAIN_BUILDROOT
>  	help
>  	  Enter the path to the overlay tarball for a custom processor
>  	  configuration.
> @@ -31,6 +31,7 @@ choice
>  	prompt "Target Architecture Endianness"
>  	default BR2_XTENSA_LITTLE_ENDIAN
>  	depends on BR2_XTENSA_CUSTOM
> +	depends on BR2_XTENSA_OVERLAY_FILE != "" || BR2_TOOLCHAIN_EXTERNAL
>  
>  config BR2_XTENSA_LITTLE_ENDIAN
>  	bool "Little endian"
> @@ -51,7 +52,9 @@ config BR2_XTENSA_USE_MMU
>  
>  config BR2_ENDIAN
>  	default "LITTLE"	if BR2_XTENSA_LITTLE_ENDIAN
> -	default "BIG"		if BR2_xtensa_fsf || BR2_XTENSA_BIG_ENDIAN
> +	default "BIG"		if BR2_XTENSA_BIG_ENDIAN
> +	default "BIG"		if BR2_xtensa_fsf
> +	default "BIG"		if BR2_XTENSA_CUSTOM && BR2_XTENSA_OVERLAY_FILE = ""
>  
>  config BR2_ARCH
>  	default "xtensa"	if BR2_xtensa
> -- 
> 2.37.3
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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

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

[Thomas Petazzoni]

Hello,

On Sun, 25 Sep 2022 09:25:02 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> >  (1) Showing the overlay file option only when building an internal
> >      toolchain. For external toolchain configurations, it does not
> >      make sense to show it as it will have no effect.  
> 
> This should be a separate patch.

ACK.

> >  (2) Showing the endianness choice either for internal toolchain
> >      configurations with non-empty BR2_XTENSA_OVERLAY_FILE, or for
> >      external toolchain configurations.  
> 
> As an empty BR2_XTENSA_OVERLAY_FILE is not accepted anymore (see below),

See my patch 2/4. Not accepting an empty BR2_XTENSA_OVERLAY_FILE is
totally wrong, as it breaks the build with external toolchains.
External toolchains can target a custom Xtensa core, but specifying a
overlay file with an external toolchain does not make any sense.

My patch 2/4 therefore reverts 4cbf7336914f, which is wrong.

> this can be simplified to "sharing the endianness chice for internal and
> external toolchains".
> 
> >  (2) Defaulting to BE when BR2_XTENSA_OVERLAY_FILE is empty.  
> 
> An empty BR2_XTENSA_OVERLAY_FILE is now no longer accepted, after commit
> 4cbf7336914f (arch/xtensa: custom configuration requires an overlay).

Right, but this commit is incorrect, and needs to be reverted.

> > Fixes:
> >   http://autobuild.buildroot.net/results/702e2886156f291466375dfcf412c20f1aa5857d/  
> 
> This commit if from August the 15th; I could not find any such failure
> since 4cbf7336914f was applied; the last occured 2022-09-08, on commit
> d2141f65e449, which is the parent of 4cbf7336914f (there are build
> failures on older branches, of course, but not on master).

See above: 4cbf7336914f is incorrect. It breaks for example the test
case for the Bootlin Xtensa LX60 external toolchain. An empty overlay
file is perfectly valid for a custom Xtensa core, when using an
external toolchain.

And I am precisely doing the revert *after* this patch 1/4 so that the
series is bisectable. I.e introduce the correct fix, and then remove
the incorrect fix.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Thomas Petazzoni]

On Sun, 25 Sep 2022 10:30:19 +0200
Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:

> On Sun, 25 Sep 2022 09:25:02 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> 
> > >  (1) Showing the overlay file option only when building an internal
> > >      toolchain. For external toolchain configurations, it does not
> > >      make sense to show it as it will have no effect.    
> > 
> > This should be a separate patch.  
> 
> ACK.

Except in fact the overlay thing needs to be allowed even with an
external toolchain: it can patch Linux and U-Boot as well as the
toolchain components.

I'll rework the patch series with this in mind.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot