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