[U-Boot] [PATCH v2 3/7] ARM: add assembly routine to switch to non-secure state
Masahiro Yamada
yamada.m at jp.panasonic.com
Fri Jun 28 05:09:58 CEST 2013
Hi, Andre
> +.globl _nonsec_init
> +_nonsec_init:
These two lines can be written:
ENTRY(_nonsec_init)
> + mrc p15, 0, r0, c0, c0, 0 @ read MIDR
> + bfc r0, #20, #4 @ mask out variant
> + bfc r0, #0, #4 @ and revision
Why do you hard code bit positions
even though MIDR_PRIMARY_PART_MASK is available?
> + movw r1, #lo(MIDR_CORTEX_A7_R0P0 & MIDR_PRIMARY_PART_MASK)
> + movt r1, #hi(MIDR_CORTEX_A7_R0P0 & MIDR_PRIMARY_PART_MASK)
Why don't you use "ldr r*, =..." statement here?
> + orr r1, #(MIDR_CORTEX_A15_R0P0 & 0xf0)
This code relies on the value of MIDR_CORTEX_A15_R0P0.
So, I think you can rewrite this part more simply as follows:
mrc p15, 0, r0, c0, c0, 0 @ read MIDR
ldr r1, =MIDR_PRIMARY_PART_MASK
and r0, r0, r1 @ mask out variant and revision
ldr r1, =MIDR_CORTEX_A7_R0P0
cmp r0, r1 @ check for Cortex-A7
ldr r1, =MIDR_CORTEX_A15_R0P0
cmpne r0, r1 @ check for Cortex-A15
> + bx lr
Add ENDPROC(_nonsec_init) when beginning with ENTRY(_nonsec_init).
> +/* defined in cpu/armv7/nonsec_virt.S */
> +void _nonsec_init(void);
I think this comment is not necessary and
mantainancability is not excellent in case you renaming
nonsec_virt.S.
BTW, tag generation tool, GNU Global can help us
for searching definition.
If you begin routines in assembly with ENTRY(...),
GNU Global picks up these symbols for tag jump.
Masahiro Yamada
More information about the U-Boot
mailing list