Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Romain Naour <romain.naour@gmail.com>,
	Buildroot List <buildroot@buildroot.org>
Subject: Re: [Buildroot] [PATCH v2 2/5] Revert "arch/xtensa: custom configuration requires an overlay"
Date: Sun, 25 Sep 2022 12:03:55 +0200	[thread overview]
Message-ID: <20220925100355.GG1127102@scaer> (raw)
In-Reply-To: <20220925090920.1585682-3-thomas.petazzoni@bootlin.com>

Thomas, All,

On 2022-09-25 11:09 +0200, Thomas Petazzoni spake thusly:
> 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.

As 4cbf7336914f explained, an empty overlay is equivalent to using the
fsf variant, so in that case, there would be no point in asking for a
custom core to begin with, and Max Filipov seemed to agree:
    https://lore.kernel.org/buildroot/CAMo8BfKHKuVC86uOwP0z8CzE425q7u0B_q6jdeU0Rw9upaiY=Q@mail.gmail.com/

So, I think we are maybe taking the issue from the wrong side here. We
should forbid an empty overlay only when using our internal toolchain
for a custom xtensa core. I.e.:

    diff --git a/arch/arch.mk.xtensa b/arch/arch.mk.xtensa
    index 7b6c59cecd..1799528a93 100644
    --- a/arch/arch.mk.xtensa
    +++ b/arch/arch.mk.xtensa
    @@ -1,8 +1,10 @@
     BR_ARCH_XTENSA_OVERLAY_FILE = $(call qstrip,$(BR2_XTENSA_OVERLAY_FILE))
     
    -ifeq ($(BR_BUILDING)$(BR2_XTENSA_CUSTOM):$(BR_ARCH_XTENSA_OVERLAY_FILE),yy:)
    +ifeq ($(BR_BUILDING)$(BR2_TOOLCHAIN_BUILDROOT),yy)
    +ifeq ($(BR_ARCH_XTENSA_OVERLAY_FILE),)
     $(error No xtensa overlay file provided. Check your BR2_XTENSA_OVERLAY_FILE setting)
     endif
    +endif
     
     ################################################################################
     # This variable can be used by packages that need to extract the
     # overlay.

Regards,
Yann E. MORIN.

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

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

  reply	other threads:[~2022-09-25 10:04 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-25  9:09 [Buildroot] [PATCH v2 0/5] Fixes/improvements in Xtensa handling Thomas Petazzoni
2022-09-25  9:09 ` [Buildroot] [PATCH v2 1/5] arch/Config.in.xtensa: adjust endianness logic to avoid bogus configurations Thomas Petazzoni
2022-09-25 10:10   ` Yann E. MORIN
2022-10-31 21:02     ` Thomas Petazzoni via buildroot
2022-09-25  9:09 ` [Buildroot] [PATCH v2 2/5] Revert "arch/xtensa: custom configuration requires an overlay" Thomas Petazzoni
2022-09-25 10:03   ` Yann E. MORIN [this message]
2022-09-25 10:13     ` Thomas Petazzoni
2022-10-31 21:05       ` Thomas Petazzoni via buildroot
2022-09-25  9:09 ` [Buildroot] [PATCH v2 3/5] arch/Config.in.xtensa: use one line per case in BR2_ENDIAN definition Thomas Petazzoni
2022-09-25  9:09 ` [Buildroot] [PATCH v2 4/5] arch/{Config.in, arch.mk}.xtensa: add explicit support for DC233C core Thomas Petazzoni
2022-09-25  9:09 ` [Buildroot] [PATCH v2 5/5] configs/qemu_xtensa_lx60*: use new BR2_xtensa_dc233c option Thomas Petazzoni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220925100355.GG1127102@scaer \
    --to=yann.morin.1998@free.fr \
    --cc=buildroot@buildroot.org \
    --cc=romain.naour@gmail.com \
    --cc=thomas.petazzoni@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox