pkarashchenko commented on a change in pull request #5662:
URL: https://github.com/apache/incubator-nuttx/pull/5662#discussion_r818432057



##########
File path: arch/arm/src/armv7-a/Toolchain.defs
##########
@@ -56,21 +56,61 @@ endif
 #
 
 ifeq ($(CONFIG_ARM_THUMB),y)
-ARCHCPUFLAGS += -mthumb
-endif
-
-ifeq ($(CONFIG_ARCH_FPU),y)
-ARCHCPUFLAGS += -mfloat-abi=hard
+  ARCHCPUFLAGS += -mthumb
 endif
 
 ifeq ($(CONFIG_ARCH_CORTEXA5),y)
-ARCHCPUFLAGS += -mcpu=cortex-a5
+  ARCHCPUFLAGS += -mcpu=cortex-a5
 else ifeq ($(CONFIG_ARCH_CORTEXA7),y)
-ARCHCPUFLAGS += -mcpu=cortex-a7
+  ARCHCPUFLAGS += -mcpu=cortex-a7
 else ifeq ($(CONFIG_ARCH_CORTEXA8),y)
-ARCHCPUFLAGS += -mcpu=cortex-a8
+  ARCHCPUFLAGS += -mcpu=cortex-a8
 else ifeq ($(CONFIG_ARCH_CORTEXA9),y)
-ARCHCPUFLAGS += -mcpu=cortex-a9
+  ARCHCPUFLAGS += -mcpu=cortex-a9
+endif
+
+ifneq ($(CONFIG_ARM_HAVE_FPU_D32),y)
+  ARCHFPUD32FLAGS = -d16
+endif
+
+ifeq ($(CONFIG_ARM_NEON),y)
+  ARCHNEONFLAGS = neon-
+  ifneq ($(CONFIG_ARM_HAVE_FPU_D32),y)
+    $(error "Number of FPU is alwasys 32 with NEON enabled, please check 
Kconfig of your SoC.")
+  endif
+endif
+
+ifeq ($(CONFIG_ARCH_FPU),y)
+  ifeq ($(CONFIG_ARM_FPU_ABI_SOFT),y)
+    ARCHCPUFLAGS += -mfloat-abi=softfp
+  else
+    ARCHCPUFLAGS += -mfloat-abi=hard
+  endif
+
+  ifeq ($(CONFIG_ARCH_CORTEXA7),y)
+    ifneq ($(CONFIG_ARM_NEON),y)

Review comment:
       I think that `vfpv4` should be specified unconditionally. This is 
according to the table that you provided

##########
File path: arch/arm/src/armv7-a/Toolchain.defs
##########
@@ -56,21 +56,51 @@ endif
 #
 
 ifeq ($(CONFIG_ARM_THUMB),y)
-ARCHCPUFLAGS += -mthumb
-endif
-
-ifeq ($(CONFIG_ARCH_FPU),y)
-ARCHCPUFLAGS += -mfloat-abi=hard
+  ARCHCPUFLAGS += -mthumb
 endif
 
 ifeq ($(CONFIG_ARCH_CORTEXA5),y)
-ARCHCPUFLAGS += -mcpu=cortex-a5
+  ARCHCPUFLAGS += -mcpu=cortex-a5
 else ifeq ($(CONFIG_ARCH_CORTEXA7),y)
-ARCHCPUFLAGS += -mcpu=cortex-a7
+  ARCHCPUFLAGS += -mcpu=cortex-a7
 else ifeq ($(CONFIG_ARCH_CORTEXA8),y)
-ARCHCPUFLAGS += -mcpu=cortex-a8
+  ARCHCPUFLAGS += -mcpu=cortex-a8
 else ifeq ($(CONFIG_ARCH_CORTEXA9),y)
-ARCHCPUFLAGS += -mcpu=cortex-a9
+  ARCHCPUFLAGS += -mcpu=cortex-a9
+endif
+
+ifneq ($(CONFIG_ARM_HAVE_FPU_D32),y)
+  ARCHFPUD32FLAGS = -d16
+endif
+
+ifeq ($(CONFIG_ARM_FP_NEON),y)
+  ARCHNEONFLAGS = neon-
+  ifneq ($(CONFIG_ARM_HAVE_FPU_D32),y)
+    $(error "Number of FPU is alwasys 32 with NEON enabled, please check 
Kconfig of your SoC.")
+  endif
+endif
+
+ifeq ($(CONFIG_ARCH_FPU),y)
+  ifeq ($(CONFIG_ARM_FPU_ABI_SOFT),y)
+    ARCHCPUFLAGS += -mfloat-abi=softfp
+  else
+    ARCHCPUFLAGS += -mfloat-abi=hard
+  endif
+  ifeq ($(CONFIG_ARCH_CORTEXA7),y)
+    ifneq ($(CONFIG_ARM_FP_NEON),y)
+      ARCHFPUVERSION = vfpv4
+    endif
+  else
+    ARCHFPUVERSION = vfpv3

Review comment:
       Looking into the table that you provided I think the implementation does 
not match.
   We should have something like:
   remove `ARCHNEONFLAGS` completely together with
   ```
   ifeq ($(CONFIG_ARM_NEON),y)
     ARCHNEONFLAGS = neon-
     ifneq ($(CONFIG_ARM_HAVE_FPU_D32),y)
       $(error "Number of FPU is alwasys 32 with NEON enabled, please check 
Kconfig of your SoC.")
     endif
   endif
   ```
   change
   ```
   ifeq ($(CONFIG_ARCH_CORTEXA7),y)
     ifeq ($(CONFIG_ARM_NEON),y)
       ARCHFPUVERSION = neon-vfpv4
     else
       ARCHFPUVERSION = vfpv4
     endif
   else
     ifeq ($(CONFIG_ARM_NEON),y)
       ARCHFPUVERSION = neon
     else
       ARCHFPUVERSION = vfpv3
     endif
   endif
   
   ARCHCPUFLAGS += -mfpu=$(ARCHFPUVERSION)$(ARCHFPUD32FLAGS)
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to