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