diff mbox

[U-Boot,1/8] POST/arm: adaptations needed for POST on ARM to work

Message ID 1312375027-27693-2-git-send-email-valentin.longchamp@keymile.com
State Superseded
Headers show

Commit Message

Valentin Longchamp Aug. 3, 2011, 12:37 p.m. UTC 
For post to run on ARM, 3 things are needed:
- post_log_word to be defined in gd
- a post.h include in arch/arm/lib/board.c
- most ARM boards will set POST_WORD in RAM, so we need gd->ram, and
  thus DECLARE_GLOBAL_DATA_PTR is needed in post.h

Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
Signed-off-by: Holger Brunck <holger.brunck@keymile.com>
---
 arch/arm/include/asm/global_data.h |    4 ++++
 arch/arm/lib/board.c               |    2 ++
 include/post.h                     |    6 ++++++
 3 files changed, 12 insertions(+), 0 deletions(-)

Comments

Mike Frysinger Aug. 14, 2011, 7:07 p.m. UTC | #1 
On Wednesday, August 03, 2011 08:37:00 Valentin Longchamp wrote:
> --- a/include/post.h
> +++ b/include/post.h
> 
> +/*
> + * some ARM implementations have to use gd->ram_size, since POST_WORD is
> + * defined in RAM
> + */
> +DECLARE_GLOBAL_DATA_PTR;

i'm not sure about this.  no other header has been allowed to do this in the 
past, and i dont think we should start now.
-mike
Valentin Longchamp Aug. 15, 2011, 9:09 a.m. UTC | #2 
On 08/14/2011 09:07 PM, Mike Frysinger wrote:
> On Wednesday, August 03, 2011 08:37:00 Valentin Longchamp wrote:
>> --- a/include/post.h
>> +++ b/include/post.h
>>
>> +/*
>> + * some ARM implementations have to use gd->ram_size, since POST_WORD is
>> + * defined in RAM
>> + */
>> +DECLARE_GLOBAL_DATA_PTR;
> 
> i'm not sure about this.  no other header has been allowed to do this in the 
> past, and i dont think we should start now.
> -mike

OK. Then we should move the post_word_load and post_word_store function to
post/post.c. Would this be accepted ?
Mike Frysinger Aug. 16, 2011, 6:03 p.m. UTC | #3 
On Monday, August 15, 2011 05:09:42 Valentin Longchamp wrote:
> On 08/14/2011 09:07 PM, Mike Frysinger wrote:
> > On Wednesday, August 03, 2011 08:37:00 Valentin Longchamp wrote:
> >> --- a/include/post.h
> >> +++ b/include/post.h
> >> 
> >> +/*
> >> + * some ARM implementations have to use gd->ram_size, since POST_WORD
> >> is + * defined in RAM
> >> + */
> >> +DECLARE_GLOBAL_DATA_PTR;
> > 
> > i'm not sure about this.  no other header has been allowed to do this in
> > the past, and i dont think we should start now.
> 
> OK. Then we should move the post_word_load and post_word_store function to
> post/post.c. Would this be accepted ?

that would add overhead that most people dont need.  i guess the only other 
option would be to add a CONFIG_POST_EXTERNAL_WORD_FUNCS and then post.h would 
just define the two funcs as externs.  it'd be up to the board porters to 
define them however they want.
-mike
Marek Vasut Aug. 18, 2011, 10:07 a.m. UTC | #4 
On Tuesday, August 16, 2011 08:03:54 PM Mike Frysinger wrote:
> On Monday, August 15, 2011 05:09:42 Valentin Longchamp wrote:
> > On 08/14/2011 09:07 PM, Mike Frysinger wrote:
> > > On Wednesday, August 03, 2011 08:37:00 Valentin Longchamp wrote:
> > >> --- a/include/post.h
> > >> +++ b/include/post.h
> > >> 
> > >> +/*
> > >> + * some ARM implementations have to use gd->ram_size, since POST_WORD
> > >> is + * defined in RAM
> > >> + */
> > >> +DECLARE_GLOBAL_DATA_PTR;
> > > 
> > > i'm not sure about this.  no other header has been allowed to do this
> > > in the past, and i dont think we should start now.
> > 
> > OK. Then we should move the post_word_load and post_word_store function
> > to post/post.c. Would this be accepted ?
> 
> that would add overhead that most people dont need.  i guess the only other
> option would be to add a CONFIG_POST_EXTERNAL_WORD_FUNCS and then post.h
> would just define the two funcs as externs.  it'd be up to the board
> porters to define them however they want.
> -mike

We don't want externs. Why would moving it into post.c introduce any overhead ?
Mike Frysinger Aug. 18, 2011, 2:31 p.m. UTC | #5 
On Thursday, August 18, 2011 06:07:06 Marek Vasut wrote:
> On Tuesday, August 16, 2011 08:03:54 PM Mike Frysinger wrote:
> > On Monday, August 15, 2011 05:09:42 Valentin Longchamp wrote:
> > > On 08/14/2011 09:07 PM, Mike Frysinger wrote:
> > > > On Wednesday, August 03, 2011 08:37:00 Valentin Longchamp wrote:
> > > >> --- a/include/post.h
> > > >> +++ b/include/post.h
> > > >> 
> > > >> +/*
> > > >> + * some ARM implementations have to use gd->ram_size, since
> > > >> POST_WORD is + * defined in RAM
> > > >> + */
> > > >> +DECLARE_GLOBAL_DATA_PTR;
> > > > 
> > > > i'm not sure about this.  no other header has been allowed to do this
> > > > in the past, and i dont think we should start now.
> > > 
> > > OK. Then we should move the post_word_load and post_word_store function
> > > to post/post.c. Would this be accepted ?
> > 
> > that would add overhead that most people dont need.  i guess the only
> > other option would be to add a CONFIG_POST_EXTERNAL_WORD_FUNCS and then
> > post.h would just define the two funcs as externs.  it'd be up to the
> > board porters to define them however they want.
> 
> We don't want externs. Why would moving it into post.c introduce any
> overhead ?

because the current code expands into a single memory read/write for many 
arches.  moving it into post.c already means making it into an extern and now 
people have to call an external function instead of inlining the memory 
access.
-mike
Marek Vasut Aug. 18, 2011, 5:01 p.m. UTC | #6 
On Thursday, August 18, 2011 04:31:09 PM Mike Frysinger wrote:
> On Thursday, August 18, 2011 06:07:06 Marek Vasut wrote:
> > On Tuesday, August 16, 2011 08:03:54 PM Mike Frysinger wrote:
> > > On Monday, August 15, 2011 05:09:42 Valentin Longchamp wrote:
> > > > On 08/14/2011 09:07 PM, Mike Frysinger wrote:
> > > > > On Wednesday, August 03, 2011 08:37:00 Valentin Longchamp wrote:
> > > > >> --- a/include/post.h
> > > > >> +++ b/include/post.h
> > > > >> 
> > > > >> +/*
> > > > >> + * some ARM implementations have to use gd->ram_size, since
> > > > >> POST_WORD is + * defined in RAM
> > > > >> + */
> > > > >> +DECLARE_GLOBAL_DATA_PTR;
> > > > > 
> > > > > i'm not sure about this.  no other header has been allowed to do
> > > > > this in the past, and i dont think we should start now.
> > > > 
> > > > OK. Then we should move the post_word_load and post_word_store
> > > > function to post/post.c. Would this be accepted ?
> > > 
> > > that would add overhead that most people dont need.  i guess the only
> > > other option would be to add a CONFIG_POST_EXTERNAL_WORD_FUNCS and then
> > > post.h would just define the two funcs as externs.  it'd be up to the
> > > board porters to define them however they want.
> > 
> > We don't want externs. Why would moving it into post.c introduce any
> > overhead ?
> 
> because the current code expands into a single memory read/write for many
> arches.  moving it into post.c already means making it into an extern and
> now people have to call an external function instead of inlining the
> memory access.

I don't think I follow you here ... why won't you be able to inline that stuff if 
it's in post.c ?

> -mike
Mike Frysinger Aug. 18, 2011, 5:42 p.m. UTC | #7 
On Thursday, August 18, 2011 13:01:56 Marek Vasut wrote:
> On Thursday, August 18, 2011 04:31:09 PM Mike Frysinger wrote:
> > On Thursday, August 18, 2011 06:07:06 Marek Vasut wrote:
> > > On Tuesday, August 16, 2011 08:03:54 PM Mike Frysinger wrote:
> > > > On Monday, August 15, 2011 05:09:42 Valentin Longchamp wrote:
> > > > > On 08/14/2011 09:07 PM, Mike Frysinger wrote:
> > > > > > On Wednesday, August 03, 2011 08:37:00 Valentin Longchamp wrote:
> > > > > >> --- a/include/post.h
> > > > > >> +++ b/include/post.h
> > > > > >> 
> > > > > >> +/*
> > > > > >> + * some ARM implementations have to use gd->ram_size, since
> > > > > >> POST_WORD is + * defined in RAM
> > > > > >> + */
> > > > > >> +DECLARE_GLOBAL_DATA_PTR;
> > > > > > 
> > > > > > i'm not sure about this.  no other header has been allowed to do
> > > > > > this in the past, and i dont think we should start now.
> > > > > 
> > > > > OK. Then we should move the post_word_load and post_word_store
> > > > > function to post/post.c. Would this be accepted ?
> > > > 
> > > > that would add overhead that most people dont need.  i guess the only
> > > > other option would be to add a CONFIG_POST_EXTERNAL_WORD_FUNCS and
> > > > then post.h would just define the two funcs as externs.  it'd be up
> > > > to the board porters to define them however they want.
> > > 
> > > We don't want externs. Why would moving it into post.c introduce any
> > > overhead ?
> > 
> > because the current code expands into a single memory read/write for many
> > arches.  moving it into post.c already means making it into an extern and
> > now people have to call an external function instead of inlining the
> > memory access.
> 
> I don't think I follow you here ... why won't you be able to inline that
> stuff if it's in post.c ?

grep the tree.  post.c isnt the only consumer of these funcs.
-mike
diff mbox

Patch

diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
index 4fc51fd..b2c336a 100644
--- a/arch/arm/include/asm/global_data.h
+++ b/arch/arm/include/asm/global_data.h
@@ -75,6 +75,10 @@  typedef	struct	global_data {
 #endif
 	void		**jt;		/* jump table */
 	char		env_buf[32];	/* buffer for getenv() before reloc. */
+#if defined(CONFIG_POST) || defined(CONFIG_LOGBUFFER)
+	unsigned long	post_log_word; /* Record POST activities */
+	unsigned long	post_init_f_time; /* When post_init_f started */
+#endif
 } gd_t;
 
 /*
diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 90709d0..db8abac 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -48,6 +48,8 @@ 
 #include <nand.h>
 #include <onenand_uboot.h>
 #include <mmc.h>
+#include <post.h>
+#include <logbuff.h>
 
 #ifdef CONFIG_BITBANGMII
 #include <miiphy.h>
diff --git a/include/post.h b/include/post.h
index 3f259b7..0801956 100644
--- a/include/post.h
+++ b/include/post.h
@@ -76,6 +76,12 @@ 
 #endif
 #endif /* CONFIG_SYS_POST_WORD_ADDR */
 
+/*
+ * some ARM implementations have to use gd->ram_size, since POST_WORD is
+ * defined in RAM
+ */
+DECLARE_GLOBAL_DATA_PTR;
+
 static inline ulong post_word_load (void)
 {
 	return in_le32((volatile void *)(_POST_WORD_ADDR));