[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