[PATCH] make the code simpler

[PATCH] make the code simpler

[u74147]

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

Re: [PATCH] make the code simpler

[Antony Pavlov]

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

[PATCH] MIPS: start.S: remove duplicate ADR macro definition

[Antony Pavlov]

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 arch/mips/boot/start.S | 22 +---------------------
 1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/arch/mips/boot/start.S b/arch/mips/boot/start.S
index 7e2ae5e..d180157 100644
--- a/arch/mips/boot/start.S
+++ b/arch/mips/boot/start.S
@@ -2,7 +2,6 @@
  * Startup Code for MIPS CPU
  *
  * Copyright (C) 2011 Antony Pavlov <antonynpavlov@gmail.com>
- * Used code copyrighted (C) 2009 by Shinya Kuribayashi <skuribay@pobox.com>
  *
  * This file is part of barebox.
  * See file CREDITS for list of people who contributed to this project.
@@ -24,26 +23,7 @@
 #include <asm-generic/memory_layout.h>
 #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
+#include <asm/pbl_macros.h>
 
 	.set noreorder
 	.text
-- 
1.8.4.4


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

RE: 答复: [PATCH] MIPS: start.S: remove duplicate ADR macro definition

[Kevin Du Huanpeng]

Hi!
I am using this model:
WR720N   http://wiki.openwrt.org/toh/tp-link/tl-wr720n
note that, there is a very different model with the same name.
(*NOT* this one) http://www.tp-link.com/en/products/details/?model=TL-WR720N
but I suggest you to buy this model: wr703n
http://wiki.openwrt.org/ru/toh/tp-link/tl-wr703n
http://wiki.openwrt.org/toh/tp-link/tl-wr703n
it's even smaller. But only have one RJ45 port. The wr720n has two.
wr703n is cute.

I don't know if these two model available outside of china.
But there are some model very similar to these two.
and ar9331 based tp-link router:
https://github.com/pepe2k/u-boot_mod#supported-devices
http://wiki.openwrt.org/toh/tp-link/tl-mr10u

in china, we can buy it from
WR703N 99CNY http://item.yixun.com/item-216308.html?YTAG=3.21012001
WR720N 125CNY http://item.yixun.com/item-350056.html?YTAG=3.21012000
Or taobao.com, a site like Yahoo or ebay, C2C.

125CNY=21USD,99CNY=17USD,
but I don’t know why it is 64.99USD in amazon USA.
http://www.amazon.com/TP-Link-TL-WR720N-150Mbps-Wireless-N-Portable/dp/B00EYD1VKC/ref=sr_1_3?ie=UTF8&qid=1386231687&sr=8-3&keywords=wr720N

if you can't find it in your location, I can buy you one.
- - -
The AR9331 datasheet is available there:
http://see.sl088.com/w/images/6/69/AR9331.pdf

serial console access, hardware and many things are there:
http://wiki.openwrt.org/toh/tp-link/tl-wr703n

u-boot is here:
https://github.com/zhang3/u-boot_mod
forked from
https://github.com/pepe2k/u-boot_mod
thanks to pepe2k(https://github.com/pepe2k)

I have create a repository on github, now it's empty.
https://github.com/zhang3/barebox-wr720n
I will upload my source code and write document later.


...
DuHuanpeng

-----Original Message-----
From: Antony Pavlov [mailto:antonynpavlov@gmail.com] 
Sent: 2013年12月5日 12:35
To: Kevin Du Huanpeng
Subject: Re: 答复: [PATCH] MIPS: start.S: remove duplicate ADR macro definition

On Thu, 5 Dec 2013 01:25:04 +0800
"Kevin Du Huanpeng" <u74147@gmail.com> wrote:

> I am trying to port barebox on my TP-LINK WR720N router
> (SoC: AR9331 24KC, DRAM: 32MiB,  FLASH: 4MiB)

http://market.yandex.ru/model.xml?modelid=8444230

If it's your model then I can easely buy it in Moscow :)

> In the start.S, this ADR macro make me confuse...
> But finally, I find it 
> 	.macro  ADR rd label temp
> just used to get the label's VMA. (correct me if I am wrong) .
> in the start.S, it wants to get the _start 's VMA.
> Assuming the _start is the first instruction of barebox.
> Why not save the _start's VMA to ra when running the first instruction?
> By 
> 	bal __start
> 	nop
> 	...
> 	__start:
> I think the different between above and ADR is:
> My code overwrite the ra register.
> If any other bootloader or something call's barebox, It can't return.
> >Personally I have no objections on removing ADR macro.
> Just remove it from start.S
> 
> By the way, I really need help...
> Do you interested in porting barebox to ar9331?
> The router is quite cute and small, and very cheap!
> Just 125CNY, (20USD).
> You can find quit a lot information about it on OpenWRT.
> And there is a working u-boot for this router.
> https://github.com/pepe2k/u-boot_mod
> I have modify this u-boot and make it run In the kernel's position.
> https://github.com/zhang3/u-boot_mod
> So that the original u-boot can boot my u-boot.
> By done this,
> I modify the link address of barebox the same as the u-boot(second 
> one).
> 
>   
> 
> -----邮件原件-----
> 发件人: Antony Pavlov [mailto:antonynpavlov@gmail.com]
> 发送时间: 2013年12月5日 0:34
> 收件人: Kevin Du Huanpeng
> 主题: Re: [PATCH] MIPS: start.S: remove duplicate ADR macro definition
> 
> On Wed, 4 Dec 2013 20:47:47 +0800
> Kevin Du Huanpeng <u74147@gmail.com> wrote:
> 
> > hi, mips has a b/bal, why does mips have to use the macro: ADR in 
> > the start.S
> > 
> 
> Personally I have no objections on removing ADR macro. It has some disadvantages (e.g. it can't be used twice in one program because it uses global label).
> 
> If you want understand the situation with ADR macro you shoul track the MIPS barebox history.
> 
> My original MIPS barebox has no ADR macro. See head.S file and 
> especialy 'compute_offset' label in this patch 
> http://lists.infradead.org/pipermail/barebox/2011-June/003695.html
> 
> But after my publication Shinya Kuribayashi has appeared:
> http://lists.infradead.org/pipermail/barebox/2011-June/003707.html
> 
> Kuribayashi-san has his own (but not working) mips barebox implementation and Jean-Christophe enforced me to use Kuribayashi's start.S realisation (with ADR macro):
> http://lists.infradead.org/pipermail/barebox/2011-June/003715.html
> 
> As a result now we have start.S with ADR macro.
> 
> You want drop ADR macro from start.S, but IMHO we can drop relocation code from start.S completely!
> Today we have smart PBL (pre-boot loader) in barebox. This PBL can comress and relocate main barebox image. So we can easely drop relocation code from start.S:
> if you need relocation then just simply use PBL!
> 
> Can you please describe your interest in barebox for MIPS?
> That type of board do you use?
> 
> If you need any help on barebox for MIPS you are welcome!
> 
> > _start: bal __start
> > nop
> > __start:
> > addiu a0, ra, -8
> > a0, now is the _start's vma.
> > 
> > 
> > 2013/12/4 Antony Pavlov <antonynpavlov@gmail.com>:
> > > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > > ---
> > >  arch/mips/boot/start.S | 22 +---------------------
> > >  1 file changed, 1 insertion(+), 21 deletions(-)
> > >
> > > diff --git a/arch/mips/boot/start.S b/arch/mips/boot/start.S index
> > > 7e2ae5e..d180157 100644
> > > --- a/arch/mips/boot/start.S
> > > +++ b/arch/mips/boot/start.S
> > > @@ -2,7 +2,6 @@
> > >   * Startup Code for MIPS CPU
> > >   *
> > >   * Copyright (C) 2011 Antony Pavlov <antonynpavlov@gmail.com>
> > > - * Used code copyrighted (C) 2009 by Shinya Kuribayashi <skuribay@pobox.com>
> > >   *
> > >   * This file is part of barebox.
> > >   * See file CREDITS for list of people who contributed to this project.
> > > @@ -24,26 +23,7 @@
> > >  #include <asm-generic/memory_layout.h>  #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
> > > +#include <asm/pbl_macros.h>
> > >
> > >         .set noreorder
> > >         .text
> > > --
> > > 1.8.4.4
> > >
> 
> 
> --
> --
> Best regards,
>   Antony Pavlov
> 


--
--
Best regards,
  Antony Pavlov


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