Re: [PATCH] make the code simpler

[Antony Pavlov <antonynpavlov@gmail.com>]

On Mon,  2 Dec 2013 09:17:46 -0800
u74147@gmail.com wrote:

There are some issues on your patch:

1. the "make the code simpler" comment is not informative.

Could you add the words "MIPS", "start.S", "ADR macro" etc?

2. if you drop the ADR macro then you should drop Shinya Kuribayashi's
copyright too as it's him who invented this macro.

3. your patch contains some formatting errors.

TIP: use ./scripts/checkpatch.pl

4. as we have duplicated ADR macro definition in pbl 
you can make start.S' code simpler just by
* including arch/mips/include/asm/pbl_macros.h into start.S;
* dropping ADR macro definition in start.S.

> From: Du Huanpeng <u74147@gmail.com>
> 
> 
> Signed-off-by: Du Huanpeng <u74147@gmail.com>
> ---
>  arch/mips/boot/start.S |   26 +++-----------------------
>  1 file changed, 3 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/mips/boot/start.S b/arch/mips/boot/start.S
> index 7e2ae5e..474407d 100644
> --- a/arch/mips/boot/start.S
> +++ b/arch/mips/boot/start.S
> @@ -25,26 +25,6 @@
>  #include <generated/compile.h>
>  #include <generated/utsrelease.h>
>  
> -	/*
> -	 * ADR macro instruction (inspired by ARM)
> -	 *
> -	 * ARM architecture doesn't have PC-relative jump instruction
> -	 * like MIPS' B/BAL insns.  When ARM makes PC-relative jumps,
> -	 * it uses ADR insn.  ADR is used to get a destination address
> -	 * of 'label' against current PC.  With this, ARM can safely
> -	 * make PC-relative jumps.
> -	 */
> -	.macro	ADR rd label temp
> -	.set	push
> -	.set	noreorder
> -	move	\temp, ra			# preserve ra beforehand
> -	bal	_pc
> -	 nop
> -_pc:	addiu	\rd, ra, \label - _pc		# label is assumed to be
> -	move	ra, \temp			# within pc +/- 32KB
> -	.set	pop
> -	.endm
> -
>  	.set noreorder
>  	.text
>  	.section ".text_bare_init"
> @@ -52,8 +32,9 @@ _pc:	addiu	\rd, ra, \label - _pc		# label is assumed to be
>  
>  EXPORT(_start)
>  
> -	b	__start
> +	bal	__start
>  	 nop
> +_ra: 
>  
>  	.org	0x10
>  	.ascii	"barebox " UTS_RELEASE " " UTS_VERSION
> @@ -72,8 +53,7 @@ __start:
>  	mtc0	k0, CP0_STATUS
>  
>  	/* copy barebox to link location */
> -	ADR	a0, _start, t1	/* a0 <- pc-relative position of _start */
> -
> +	addiu	a0, ra, (_start - _ra)	/*a0 <- pc-relative position of _start  */
>  	la	a1, _start	/* link (RAM) _start address */
>  
>  	beq	a0, a1, stack_setup
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox


-- 
-- 
Best regards,
  Antony Pavlov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox