| Message ID | 20220729000904.1295295-2-joel@jms.id.au |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc: Fix ppc64le configurations | expand |
On 7/29/22 02:08, Joel Stanley wrote: > The target ABI option for PowerPC is for selecting the SPE ABI, not the > ELF ABI that users may expect. > > Hide it when there is no option available to make it clearer when > browsing menuconfig. > > Signed-off-by: Joel Stanley <joel@jms.id.au> Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > --- > arch/Config.in.powerpc | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/Config.in.powerpc b/arch/Config.in.powerpc > index c48edd3bb40c..ef0e85fec680 100644 > --- a/arch/Config.in.powerpc > +++ b/arch/Config.in.powerpc > @@ -143,6 +143,7 @@ config BR2_powerpc_power8 > select BR2_POWERPC_CPU_HAS_VSX > endchoice > > +if BR2_POWERPC_CPU_HAS_SPE > choice > prompt "Target ABI" > default BR2_powerpc_SPE if BR2_POWERPC_CPU_HAS_SPE > @@ -157,6 +158,7 @@ config BR2_powerpc_SPE > bool "SPE" > depends on BR2_POWERPC_CPU_HAS_SPE > endchoice > +endif > > config BR2_POWERPC_SOFT_FLOAT > bool "Use soft-float"
Joel, All, On 2022-07-29 09:38 +0930, Joel Stanley spake thusly: > The target ABI option for PowerPC is for selecting the SPE ABI, not the > ELF ABI that users may expect. > > Hide it when there is no option available to make it clearer when > browsing menuconfig. > > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > arch/Config.in.powerpc | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/Config.in.powerpc b/arch/Config.in.powerpc > index c48edd3bb40c..ef0e85fec680 100644 > --- a/arch/Config.in.powerpc > +++ b/arch/Config.in.powerpc > @@ -143,6 +143,7 @@ config BR2_powerpc_power8 > select BR2_POWERPC_CPU_HAS_VSX > endchoice > > +if BR2_POWERPC_CPU_HAS_SPE > choice > prompt "Target ABI" > default BR2_powerpc_SPE if BR2_POWERPC_CPU_HAS_SPE > @@ -157,6 +158,7 @@ config BR2_powerpc_SPE > bool "SPE" > depends on BR2_POWERPC_CPU_HAS_SPE > endchoice > +endif I don't understand this change. Currently we have: 1. If BR2_POWERPC_CPU_HAS_SPE=y: BR2_powerpc_CLASSIC is not selectable BR2_powerpc_SPE is the only choice 2. if BR2_POWERPC_CPU_HAS_SPE is not set: BR2_powerpc_CLASSIC is the only choice BR2_powerpc_SPE is not selectable This means that the choice was really nt a choice, because the value of HAS_SPE implies which option is settable or not. With your change, the BR2_powerpc_CLASSIC will never be selectable at all, and BR2_powerpc_SPE will be forcibly set when BR2_POWERPC_CPU_HAS_SPE=y So we should just drop the choice altogether, and use the SPE ABI when the CPu has SPE. Is that what your patch was aiming at? Regards, Yann E. MORIN. > config BR2_POWERPC_SOFT_FLOAT > bool "Use soft-float" > -- > 2.35.1 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
Hello, On Fri, 29 Jul 2022 23:17:55 +0200 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > I don't understand this change. Currently we have: > > 1. If BR2_POWERPC_CPU_HAS_SPE=y: > BR2_powerpc_CLASSIC is not selectable > BR2_powerpc_SPE is the only choice > > 2. if BR2_POWERPC_CPU_HAS_SPE is not set: > BR2_powerpc_CLASSIC is the only choice > BR2_powerpc_SPE is not selectable > > This means that the choice was really nt a choice, because the value of > HAS_SPE implies which option is settable or not. > > With your change, the BR2_powerpc_CLASSIC will never be selectable at > all, and BR2_powerpc_SPE will be forcibly set when > BR2_POWERPC_CPU_HAS_SPE=y > > So we should just drop the choice altogether, and use the SPE ABI when > the CPu has SPE. I agree with your analysis here. I would like to go one step further: SPE is only used by two PowerPC cores: config BR2_powerpc_8540 bool "8540 / e500v1" depends on !BR2_ARCH_IS_64 select BR2_POWERPC_CPU_HAS_SPE config BR2_powerpc_8548 bool "8548 / e500v2" depends on !BR2_ARCH_IS_64 select BR2_POWERPC_CPU_HAS_SPE and SPE is no longer supported by upstream GCC. For now, we have kept GCC 8.x around just to support this SPE ABI, but it is clearly not a future proof solution. Can we drop support for SPE entirely? Best regards, Thomas
On Wed, 3 Aug 2022 at 21:22, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Hello, > > On Fri, 29 Jul 2022 23:17:55 +0200 > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > > I don't understand this change. Currently we have: > > > > 1. If BR2_POWERPC_CPU_HAS_SPE=y: > > BR2_powerpc_CLASSIC is not selectable > > BR2_powerpc_SPE is the only choice > > > > 2. if BR2_POWERPC_CPU_HAS_SPE is not set: > > BR2_powerpc_CLASSIC is the only choice > > BR2_powerpc_SPE is not selectable > > > > This means that the choice was really nt a choice, because the value of > > HAS_SPE implies which option is settable or not. > > > > With your change, the BR2_powerpc_CLASSIC will never be selectable at > > all, and BR2_powerpc_SPE will be forcibly set when > > BR2_POWERPC_CPU_HAS_SPE=y > > > > So we should just drop the choice altogether, and use the SPE ABI when > > the CPu has SPE. > > I agree with your analysis here. I would like to go one step further: > SPE is only used by two PowerPC cores: > > config BR2_powerpc_8540 > bool "8540 / e500v1" > depends on !BR2_ARCH_IS_64 > select BR2_POWERPC_CPU_HAS_SPE > config BR2_powerpc_8548 > bool "8548 / e500v2" > depends on !BR2_ARCH_IS_64 > select BR2_POWERPC_CPU_HAS_SPE > > and SPE is no longer supported by upstream GCC. For now, we have kept > GCC 8.x around just to support this SPE ABI, but it is clearly not a > future proof solution. > > Can we drop support for SPE entirely? I vote for dropping support. Given the lack of GCC support it's only a matter of time before our hand would be forced. I'll drop this patch from my series for now, as it isn't related to the ppc64le fixes.
Joel, All, On 2022-08-05 06:05 +0000, Joel Stanley spake thusly: > On Wed, 3 Aug 2022 at 21:22, Thomas Petazzoni > <thomas.petazzoni@bootlin.com> wrote: > > On Fri, 29 Jul 2022 23:17:55 +0200 > > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: [--SNIP--] > > > So we should just drop the choice altogether, and use the SPE ABI when > > > the CPu has SPE. [--SNIP--] > > and SPE is no longer supported by upstream GCC. For now, we have kept > > GCC 8.x around just to support this SPE ABI, but it is clearly not a > > future proof solution. > > Can we drop support for SPE entirely? > I vote for dropping support. Given the lack of GCC support it's only a > matter of time before our hand would be forced. That's totally fine by me, too, with a commit log that summarises this discussion of course! ;-) Regards, Yann E. MORIN.
diff --git a/arch/Config.in.powerpc b/arch/Config.in.powerpc index c48edd3bb40c..ef0e85fec680 100644 --- a/arch/Config.in.powerpc +++ b/arch/Config.in.powerpc @@ -143,6 +143,7 @@ config BR2_powerpc_power8 select BR2_POWERPC_CPU_HAS_VSX endchoice +if BR2_POWERPC_CPU_HAS_SPE choice prompt "Target ABI" default BR2_powerpc_SPE if BR2_POWERPC_CPU_HAS_SPE @@ -157,6 +158,7 @@ config BR2_powerpc_SPE bool "SPE" depends on BR2_POWERPC_CPU_HAS_SPE endchoice +endif config BR2_POWERPC_SOFT_FLOAT bool "Use soft-float"
The target ABI option for PowerPC is for selecting the SPE ABI, not the ELF ABI that users may expect. Hide it when there is no option available to make it clearer when browsing menuconfig. Signed-off-by: Joel Stanley <joel@jms.id.au> --- arch/Config.in.powerpc | 2 ++ 1 file changed, 2 insertions(+)