On Tue, Jun 14, 2016 at 3:01 PM, macro.wave.z@gmail.com wrote:
From: Hongbo Zhang hongbo.zhang@nxp.com
The input parameter CPU ID needs to be validated before furher oprations such as CPU_ON, this patch introduces the function to do this.
Signed-off-by: Wang Dongsheng dongsheng.wang@nxp.com Signed-off-by: Hongbo Zhang hongbo.zhang@nxp.com
arch/arm/cpu/armv7/ls102xa/psci.S | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv7/ls102xa/psci.S b/arch/arm/cpu/armv7/ls102xa/psci.S index 548c507..a4482e4 100644 --- a/arch/arm/cpu/armv7/ls102xa/psci.S +++ b/arch/arm/cpu/armv7/ls102xa/psci.S @@ -25,6 +25,34 @@ #define ONE_MS (GENERIC_TIMER_CLK / 1000) #define RESET_WAIT (30 * ONE_MS)
A note describing the arguments, the return value, and any other affected registers would be nice. For example r1 is clobbered.
+.globl psci_check_target_cpu_id +psci_check_target_cpu_id:
You probably don't need .globl.
Also you can use ENTRY() or LENTRY() here. These and ENDPROC() below are defined in linux/linkage.h
@ Get the real CPU numberand r0, r1, #0xff@ Verify bit[31:24], bits must be zero.tst r1, #0xff000000bne out_psci_invalid_target_cpu_id@ Verify Affinity level 2: Cluster, only one cluster in LS1021xa SoC.tst r1, #0xff0000bne out_psci_invalid_target_cpu_id@ Verify Affinity level 1: Processors, should be in 0xf00 format.lsr r1, r1, #8teq r1, #0xfbne out_psci_invalid_target_cpu_id@ Verify Affinity level 0: CPU, only 0, 1 are valid values.cmp r0, #2bge out_psci_invalid_target_cpu_idbx lr+out_psci_invalid_target_cpu_id:
mov r0, #ARM_PSCI_RET_INVALbx lr
And you could have an ENDPROC() here.
About the whole function, you could use an extra scratch register, store ARM_PSCI_RET_INVAL in r0 at the beginning, and directly return at errors. Kind of like:
if (bit[31:24] != 0) return ARM_PSCI_RET_INVAL; if (cluster != 0) return ARM_PSCI_RET_INVAL; if (processor != 0xf) return ARM_PSCI_RET_INVAL; if (cpu >= 2) return ARM_PSCI_RET_INVAL; return cpu;
It's just a different style though. Feel free to keep the current structure.
The code itself looks good.
Regards ChenYu
@ r1 = target CPU @ r2 = target PC.globl psci_cpu_on @@ -33,7 +61,10 @@ psci_cpu_on:
@ Clear and Get the correct CPU number @ r1 = 0xf01
and r1, r1, #0xff
bl psci_check_target_cpu_idcmp r0, #ARM_PSCI_RET_INVALbeq out_psci_cpu_onmov r1, r0 bl psci_cpu_on_common@@ -98,6 +129,7 @@ holdoff_release: @ Return mov r0, #ARM_PSCI_RET_SUCCESS
+out_psci_cpu_on: pop {lr} bx lr
-- 2.1.4