[V2,1/9] arch/csky: Add VDSP and FLOAT_ABI compiler options.
diff mbox series

Message ID 1557305915-11247-1-git-send-email-guoren@kernel.org
State Changes Requested
Headers show
Series
  • [V2,1/9] arch/csky: Add VDSP and FLOAT_ABI compiler options.
Related show

Commit Message

Guo Ren May 8, 2019, 8:58 a.m. UTC
From: Guo Ren <ren_guo@c-sky.com>

We never use BR2_CSKY_DSP in buildroot and we use VDSP instead. For
float compiling, we need -mfloat-abi=xxx.

Signed-off-by: Guo Ren <ren_guo@c-sky.com>
---
 arch/Config.in.csky | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Guo Ren May 13, 2019, 5:46 a.m. UTC | #1
Ping, Are there any wrong with the patchsets ?

Best Regards
 Guo Ren

On Wed, May 08, 2019 at 04:58:27PM +0800, guoren@kernel.org wrote:
> From: Guo Ren <ren_guo@c-sky.com>
> 
> We never use BR2_CSKY_DSP in buildroot and we use VDSP instead. For
> float compiling, we need -mfloat-abi=xxx.
> 
> Signed-off-by: Guo Ren <ren_guo@c-sky.com>
> ---
>  arch/Config.in.csky | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/Config.in.csky b/arch/Config.in.csky
> index e88e4e2..4622eb9 100644
> --- a/arch/Config.in.csky
> +++ b/arch/Config.in.csky
> @@ -23,9 +23,13 @@ config BR2_CSKY_FPU
>  	  Floating-Point Coprocessor or if you don't need FPU support
>  	  for your user-space programs.
>  
> -config BR2_CSKY_DSP
> -	bool "Enable DSP enhanced instructions"
> -	depends on BR2_ck810 || BR2_ck807
> +config BR2_CSKY_VDSP
> +	bool "Enable VDSP 3.0 enhanced instructions Co-processor"
> +	depends on BR2_ck860
> +
> +config BR2_GCC_TARGET_FLOAT_ABI
> +	default "soft"		if !BR2_CSKY_FPU
> +	default "hard"		if BR2_CSKY_FPU
>  
>  config BR2_ARCH
>  	default "csky"
> -- 
> 2.7.4
>
Thomas Petazzoni May 26, 2019, 8:39 p.m. UTC | #2
Hello,

On Wed,  8 May 2019 16:58:27 +0800
guoren@kernel.org wrote:

> From: Guo Ren <ren_guo@c-sky.com>
> 
> We never use BR2_CSKY_DSP in buildroot

This is not true, BR2_CSKY_DSP is used:

arch/Config.in.csky:    default "ck610"         if (BR2_ck610 && !BR2_CSKY_FPU && !BR2_CSKY_DSP)
arch/Config.in.csky:    default "ck807"         if (BR2_ck807 && !BR2_CSKY_FPU && !BR2_CSKY_DSP)
arch/Config.in.csky:    default "ck807e"        if (BR2_ck807 && !BR2_CSKY_FPU &&  BR2_CSKY_DSP)
arch/Config.in.csky:    default "ck807f"        if (BR2_ck807 &&  BR2_CSKY_FPU && !BR2_CSKY_DSP)
arch/Config.in.csky:    default "ck807ef"       if (BR2_ck807 &&  BR2_CSKY_FPU &&  BR2_CSKY_DSP)
arch/Config.in.csky:    default "ck810"         if (BR2_ck810 && !BR2_CSKY_FPU && !BR2_CSKY_DSP)
arch/Config.in.csky:    default "ck810e"        if (BR2_ck810 && !BR2_CSKY_FPU &&  BR2_CSKY_DSP)
arch/Config.in.csky:    default "ck810f"        if (BR2_ck810 &&  BR2_CSKY_FPU && !BR2_CSKY_DSP)
arch/Config.in.csky:    default "ck810ef"       if (BR2_ck810 &&  BR2_CSKY_FPU &&  BR2_CSKY_DSP)

so as such, your patch is incorrect: you remove an option, but the
option is still being used.

If this option is not needed because in fact the DSP enhanced
instructions have never been used, then please have a patch that *ONLY*
removes the BR2_CSKY_DSP option (both its definition and where it is
used).

Then another patch that adds the BR2_CSKY_VDSP option.

And finally another that sets the BR2_GCC_TARGET_FLOAT_ABI value.

All of these topics are independent from each other, they should be in
separate patches.

Thanks,

Thomas
Thomas Petazzoni May 26, 2019, 8:40 p.m. UTC | #3
On Wed,  8 May 2019 16:58:27 +0800
guoren@kernel.org wrote:

> +config BR2_CSKY_VDSP
> +	bool "Enable VDSP 3.0 enhanced instructions Co-processor"
> +	depends on BR2_ck860

Another comment: you add an option that depends on BR2_ck860 in this
PATCH 1, but BR2_ck860 is only added in PATCH 2. This is not correct.

Best regards,

Thomas
Guo Ren May 27, 2019, 11:35 a.m. UTC | #4
On Mon, May 27, 2019 at 4:40 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Wed,  8 May 2019 16:58:27 +0800
> guoren@kernel.org wrote:
>
> > +config BR2_CSKY_VDSP
> > +     bool "Enable VDSP 3.0 enhanced instructions Co-processor"
> > +     depends on BR2_ck860
>
> Another comment: you add an option that depends on BR2_ck860 in this
> PATCH 1, but BR2_ck860 is only added in PATCH 2. This is not correct.
Yes, BR2_ck860 should be first in patch.

Best Regards
 Guo Ren

Patch
diff mbox series

diff --git a/arch/Config.in.csky b/arch/Config.in.csky
index e88e4e2..4622eb9 100644
--- a/arch/Config.in.csky
+++ b/arch/Config.in.csky
@@ -23,9 +23,13 @@  config BR2_CSKY_FPU
 	  Floating-Point Coprocessor or if you don't need FPU support
 	  for your user-space programs.
 
-config BR2_CSKY_DSP
-	bool "Enable DSP enhanced instructions"
-	depends on BR2_ck810 || BR2_ck807
+config BR2_CSKY_VDSP
+	bool "Enable VDSP 3.0 enhanced instructions Co-processor"
+	depends on BR2_ck860
+
+config BR2_GCC_TARGET_FLOAT_ABI
+	default "soft"		if !BR2_CSKY_FPU
+	default "hard"		if BR2_CSKY_FPU
 
 config BR2_ARCH
 	default "csky"