Enhancements and Bug Fixes for F1Kx Port (#1169)

Fix FPU stack order issue and Improve FPU checking flow Fix Interrupt depth comparison logic Fix parameter mismatch in portmacro.h file Add comment to explain assembly code
FreeRTOS · Nov 5, 2024 · d0d55f3 · d0d55f3
1 parent f0d7945
commit d0d55f3
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 29 deletions.
diff --git a/portable/CCRH/F1Kx/README.md b/portable/CCRH/F1Kx/README.md
@@ -23,8 +23,8 @@ The test project can be found [here](https://github.com/FreeRTOS/FreeRTOS-Commun
 ## Note
    1. Configure IPIR Interrupt: Ensure that the bit specifying the destination for binding (requesting) an interrupt is enabled (e.g: IBDxxx register of F1KH-D8) (1)
    2. `Channel 0` and address `0xFFFEEC00` are used as default configuration for configIPIR_CHANNEL and configEXCLUSIVE_ADDRESS, in case of resource confliction other channel/address can be used. (2)
-   3. The minimal stack size (configMINIMAL_STACK_SIZE) must be included the reserved memory for nested interrupt. This formula can be referred: `(task_context_size) * (1 + configMAX_INT_NESTING) + Stack_depth_of_taskcode`
-   In which, `task_context_size` is calculated as `36*4bytes = 144bytes` (when FPU enabled) or `34*4bytes = 136` (when FPU disabled), configMAX_INT_NESTING is 02 as default.
+   3. The minimal stack size (configMINIMAL_STACK_SIZE) must be included the reserved memory for nested interrupt. This formula can be referred: `(task_context_size) * (2 + configMAX_INT_NESTING) + Stack_depth_of_taskcode`
+   In which, `task_context_size` is calculated as `36*4bytes = 144bytes` (when FPU enabled) or `34*4bytes = 136` (when FPU disabled), configMAX_INT_NESTING is `02` as default (Note that a value of `0` is not allowed).
    4. `configTIMER_PRESCALE`: This value is required in order to correctly configure clock for `CPUCLK_L`. Refer to Hardware Manual at `Table 44.22` for `option byte`: If the user sets the option byte `CKDIVMD to 1`, then `configTIMER_PRESCALE = 4`. Otherwise, if `CKDIVMD is set to 0`, then `configTIMER_PRESCALE = 2`.
 
 (1)  This is applicable for F1KH-D8 with SMP only.

diff --git a/portable/CCRH/F1Kx/port.c b/portable/CCRH/F1Kx/port.c
@@ -171,7 +171,7 @@
     #define configSETUP_TICK_INTERRUPT()    prvSetupTimerInterrupt()
 #endif /* configSETUP_TICK_INTERRUPT */
 
-#ifndef configMAX_INT_NESTING
+#if ( !defined( configMAX_INT_NESTING ) || ( configMAX_INT_NESTING == 0 ) )
 
 /* Set the default value for depth of nested interrupt. In theory, the
  * microcontroller have mechanism to limit number of nested level of interrupt
@@ -225,7 +225,7 @@ volatile BaseType_t xPortScheduleStatus[ configNUMBER_OF_CORES ] = { 0 };
  * It is necessary to control maximum stack depth.
  */
 volatile UBaseType_t uxInterruptNesting[ configNUMBER_OF_CORES ] = { 0 };
-volatile const UBaseType_t uxPortMaxInterruptDepth = configMAX_INT_NESTING - 1;
+volatile const UBaseType_t uxPortMaxInterruptDepth = configMAX_INT_NESTING;
 
 /* Count number of nested locks by same cores. The lock is completely released
  * only if this count is decreased to 0, the lock is separated for task

diff --git a/portable/CCRH/F1Kx/portasm.s b/portable/CCRH/F1Kx/portasm.s
@@ -84,6 +84,10 @@ portSAVE_CONTEXT .macro
     stsr    FPEPC, r19
     pushsp  r18, r19
 
+    ; Save EIPSW register to stack
+    ; Due to the syntax of the pushsp instruction, using r14 as dummy value
+    pushsp  r14, r15
+
     ; Get current TCB, the return value is stored in r10 (CCRH compiler)
     jarl    _pvPortGetCurrentTCB, lp
     st.w    sp, 0[r10]
@@ -101,14 +105,15 @@ portRESTORE_CONTEXT .macro
 
     ; Restore FPU registers if FPU is enabled
     mov     FPU_MSK, r19
-    stsr    PSW, r18
-    tst     r18, r19
-
-     ; Jump over next 3 instructions: stsr (4 bytes)*2 + popsp (4 bytes)
+    ; Restore EIPSW register to check FPU
+    ; Due to the syntax of the popsp instruction, using r14 as dummy value
+    popsp	r14, r15
+    tst     r15, r19
+    ; Jump over next 3 instructions: stsr (4 bytes)*2 + popsp (4 bytes)
     bz      12
     popsp   r18, r19
-    ldsr    r18, FPEPC
-    ldsr    r19, FPSR
+    ldsr    r19, FPEPC
+    ldsr    r18, FPSR
 
     ;Restore general-purpose registers and EIPSW, EIPC, EIIC, CTPSW, CTPC
     popsp   r15, r19
@@ -146,30 +151,30 @@ SAVE_REGISTER .macro
     mov     ep, r15
     stsr    CTPSW, r14
     stsr    CTPC, r13
-    pushsp  r13, r19
+    pushsp  r13, r18
 
     mov     FPU_MSK, r16
     tst     r16, r19
-    bz      12
-    stsr    FPSR, r18
-    stsr    FPEPC, r19
-    pushsp  r18, r19
+    bz      8
+    stsr    FPSR, r17
+    stsr    FPEPC, r18
+
+    pushsp  r17, r19
 
 .endm
 ;------------------------------------------------------------------------------
 ; Restore used registers
 ;------------------------------------------------------------------------------
 RESTORE_REGISTER .macro
 
-    mov     FPU_MSK, r16
-    stsr    PSW, r18
-    tst     r18, r19
-    bz      12
-    popsp   r18, r19
+	mov     FPU_MSK, r15
+	popsp   r17, r19
+    tst     r19, r15
+    bz      8
     ldsr    r18, FPEPC
-    ldsr    r19, FPSR
+    ldsr    r17, FPSR
 
-    popsp   r13, r19
+    popsp   r13, r18
     ldsr    r13, CTPC
     ldsr    r14, CTPSW
     mov     r15, ep
@@ -268,9 +273,10 @@ _vIrq_Handler:
 
     ; Do not enable interrupt for nesting. Stackover flow may occurs if the
     ; depth of nesting interrupt is exceeded.
-    mov     #_uxPortMaxInterruptDepth, r15
-    cmp     r16, r15
-    be      4                                 ; Jump over ei instruction
+    mov     #_uxPortMaxInterruptDepth, r19
+    ld.w    0[r19], r15
+    cmp     r15, r16
+	bge     4                            ; Jump over ei instruction
     ei
     jarl    _vCommonISRHandler, lp
     di

diff --git a/portable/CCRH/F1Kx/portmacro.h b/portable/CCRH/F1Kx/portmacro.h
@@ -111,11 +111,11 @@
 /* Scheduler utilities */
 
 /* Called at the end of an ISR that can cause a context switch */
-    extern void vPortSetSwitch( BaseType_t vPortSetSwitch );
+    extern void vPortSetSwitch( BaseType_t xSwitchRequired );
 
-    #define portEND_SWITCHING_ISR( xSwitchRequired )    vPortSetSwitch( vPortSetSwitch )
+    #define portEND_SWITCHING_ISR( x )    vPortSetSwitch( x )
 
-    #define portYIELD_FROM_ISR( x )                     portEND_SWITCHING_ISR( x )
+    #define portYIELD_FROM_ISR( x )       portEND_SWITCHING_ISR( x )
 
 /* Use to transfer control from one task to perform other tasks of
  * higher priority */
@@ -131,7 +131,7 @@
         #define coreid    xPortGET_CORE_ID()
 
 /* Request the core ID x to yield. */
-        extern void vPortYieldCore( unsigned int coreID );
+        extern void vPortYieldCore( uint32_t coreID );
 
         #define portYIELD_CORE( x )                vPortYieldCore( x )