Re: [Buildroot] [PATCH 1/3] arch: add support for mcf54418 Colfdire

From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Romain Naour <romain.naour@gmail.com>,
	Giulio Benetti <giulio.benetti@benettiengineering.com>,
	Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>,
	buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 1/3] arch: add support for mcf54418 Colfdire
Date: Mon, 3 Jul 2023 20:55:32 +0200	[thread overview]
Message-ID: <20230703185532.GC2510@scaer> (raw)
In-Reply-To: <20230629053130.3976-2-jeanmichel.hautbois@yoseli.org>

Jean-Michel, All,

On 2023-06-29 07:31 +0200, Jean-Michel Hautbois spake thusly:
> The m68k family ony has one Coldfire variant, namely the 5208. Add the
> support for the MCF54418 CPU in the configuration file.

As your cover-letter said, eda11417beaf (m68k: remove
BR2_GCC_TARGET_ARCH), setting it breaks use of external toolchains.

Granted, the commit log was not very expansive in why it did break them
which required dropping setting m68k/cf.

However, it did state that the m68k/cf defconfigs where working
perfectly well without it.

So, your commit log should explain how/why it is safe to re-introduce
BR2_GCC_TARGET_ARCH. I see you only set ig for this new CPU variant, so
maybe that's enough of a reason.

Still, it'd be nice to have it explained in plain letters.

Also, it'd be nice if you could try with an external toolchain, and
explain that in the commit log. If you do not have an external toolchain
available, you can generate one with Buildroot, and re-use it as an
external toolchain.

Regards,
Yann E. MORIN.

> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> ---
>  arch/Config.in.m68k | 9 +++++++++
>  toolchain/Config.in | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/arch/Config.in.m68k b/arch/Config.in.m68k
> index 9fd22aaf1e..6aa409b272 100644
> --- a/arch/Config.in.m68k
> +++ b/arch/Config.in.m68k
> @@ -33,11 +33,20 @@ config BR2_m68k_cf5208
>  	select BR2_m68k_cf
>  	select BR2_SOFT_FLOAT
>  
> +config BR2_m68k_cf54418
> +	bool "54418"
> +	select BR2_m68k_cf
> +	select BR2_USE_MMU
> +	select BR2_SOFT_FLOAT
>  endchoice
>  
>  config BR2_GCC_TARGET_CPU
>  	default "68040"		if BR2_m68k_68040
>  	default "5208"		if BR2_m68k_cf5208
> +	default "54455"		if BR2_m68k_cf54418
> +
> +config BR2_GCC_TARGET_ARCH
> +	default "cf"            if BR2_m68k_cf54418
>  
>  config BR2_READELF_ARCH_NAME
>  	default "MC68000"
> diff --git a/toolchain/Config.in b/toolchain/Config.in
> index ff0eb93017..ec2185cb39 100644
> --- a/toolchain/Config.in
> +++ b/toolchain/Config.in
> @@ -95,6 +95,7 @@ config BR2_TOOLCHAIN_SUPPORTS_ALWAYS_LOCKFREE_ATOMIC_INTS
>  	depends on !BR2_ARM_CPU_ARMV5
>  	depends on !BR2_sparc_v8
>  	depends on !BR2_m68k_cf5208
> +	depends on !BR2_m68k_cf54418
>  
>  # GCC uses thunk functions to adjust the 'this' pointer when calling
>  # C++ member functions in classes derived with multiple inheritance.
> -- 
> 2.39.2
> 
> _______________________________________________
> 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