Re: [Buildroot] [PATCH 3/9] arch/powerpc: Enable powerpc64le only on CPUs that support it

[Arnout Vandecappelle <arnout@mind.be>]

On 29/07/2022 02:08, Joel Stanley wrote:
> Invalid configurations lead to build failures, such as trying to enable
> ppc64le for the ppc970:
> 
>    http://autobuild.buildroot.net/results/fda886768fce25ccd9b52b635ff5b13da7ba2d0c/
> 
> In order to run a ppc64le userspace a kernel that runs in this mode is
> required. The only CPU supported in buildroot that can boot a ppc64le
> kernel is Power8, so mark all of the other 64-bit capable CPUs as not
> supporting ppc64le.
> 
> This drops the comment about libc, which is true but doesn't tell the
> whole story.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>

  Applied to master with some changes, see below.

> ---
>   arch/Config.in.powerpc | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/Config.in.powerpc b/arch/Config.in.powerpc
> index 8d392bfde814..7c6ae84348e9 100644
> --- a/arch/Config.in.powerpc
> +++ b/arch/Config.in.powerpc
> @@ -71,11 +71,9 @@ config BR2_powerpc_604e
>   	depends on !BR2_ARCH_IS_64
>   config BR2_powerpc_620
>   	bool "620"
> -	# No C library supports this variant on ppc64le
>   	depends on !BR2_powerpc64le
>   config BR2_powerpc_630
>   	bool "630"
> -	# No C library supports this variant on ppc64le
>   	depends on !BR2_powerpc64le
>   config BR2_powerpc_740
>   	bool "740"
> @@ -102,6 +100,7 @@ config BR2_powerpc_860
>   	depends on !BR2_ARCH_IS_64
>   config BR2_powerpc_970
>   	bool "970"
> +	depends on !BR2_powerpc64le

  As suggested by Thomas, I changed this to positive logic. I'll also push a 
follow-up patch that changes the rest of the file to positive logic.

  Could you (or any other powerpc "expert") review the conditions to be sure the 
architecture selection options are correct now?

>   	select BR2_POWERPC_CPU_HAS_ALTIVEC
>   config BR2_powerpc_8540
>   	bool "8540 / e500v1"
> @@ -129,21 +128,25 @@ config BR2_powerpc_e6500
>   	select BR2_POWERPC_CPU_HAS_ALTIVEC
>   config BR2_powerpc_power4
>   	bool "power4"
> -	# No C library supports this variant on ppc64le
>   	depends on !BR2_powerpc64le
>   config BR2_powerpc_power5
>   	bool "power5"
> -	# No C library supports this variant on ppc64le
>   	depends on !BR2_powerpc64le
> +	depends on BR2_ARCH_IS_64

  AFAIU, all POWER CPUs are able to run in 32-bit mode and I believe it's even 
possible to run a 64-bit kernel with 32-bit userspace like on ARM. Is this not true?

  If POWER5+ really can't run in 32-bit mode (or this is not supported by the 
kernel), are you sure that this *is* possible on POWER4?

  In either case, if the current conditions are not correct, please send a 
follow-up patch to fix them even more.


>   config BR2_powerpc_power6
>   	bool "power6"
> +	depends on !BR2_powerpc64le
> +	depends on BR2_ARCH_IS_64

  So here I wrote the condition as

	depends on BR2_powerpc || BR2_powerpc64

because I do believe it supports 32-bit.

  Regards,
  Arnout

>   	select BR2_POWERPC_CPU_HAS_ALTIVEC
>   config BR2_powerpc_power7
>   	bool "power7"
> +	depends on !BR2_powerpc64le
> +	depends on BR2_ARCH_IS_64
>   	select BR2_POWERPC_CPU_HAS_ALTIVEC
>   	select BR2_POWERPC_CPU_HAS_VSX
>   config BR2_powerpc_power8
>   	bool "power8"
> +	depends on BR2_ARCH_IS_64
>   	select BR2_POWERPC_CPU_HAS_ALTIVEC
>   	select BR2_POWERPC_CPU_HAS_VSX
>   endchoice
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot