From d0d55f30314c72f323254bda2bf8506d9510b582 Mon Sep 17 00:00:00 2001 From: Trong Nguyen <147012384+TrongNguyenR@users.noreply.github.com> Date: Tue, 5 Nov 2024 15:09:50 +0700 Subject: [PATCH] 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 --- portable/CCRH/F1Kx/README.md | 4 +-- portable/CCRH/F1Kx/port.c | 4 +-- portable/CCRH/F1Kx/portasm.s | 48 +++++++++++++++++++--------------- portable/CCRH/F1Kx/portmacro.h | 8 +++--- 4 files changed, 35 insertions(+), 29 deletions(-) diff --git a/portable/CCRH/F1Kx/README.md b/portable/CCRH/F1Kx/README.md index 7583c67aff2..4cb9f07b610 100644 --- 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 index e3d71929d8e..0e6116527df 100644 --- 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 index 4e56f449390..ff8e7ee31f5 100644 --- 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,14 +151,15 @@ 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 ;------------------------------------------------------------------------------ @@ -161,15 +167,14 @@ SAVE_REGISTER .macro ;------------------------------------------------------------------------------ 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 index e2b41f2640b..09f9f461450 100644 --- 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 )