On Mon, 2 Dec 2013 09:17:46 -0800 [email protected] 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 <[email protected]> > > > Signed-off-by: Du Huanpeng <[email protected]> > --- > 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 > [email protected] > http://lists.infradead.org/mailman/listinfo/barebox -- -- Best regards, Antony Pavlov _______________________________________________ barebox mailing list [email protected] http://lists.infradead.org/mailman/listinfo/barebox
