Patchwork [U-Boot,15/28] Blackfin: fix bd_t handling

login
register
mail settings
Submitter Mike Frysinger
Date Dec. 27, 2010, 4:48 a.m.
Message ID <1293425300-27644-16-git-send-email-vapier@gentoo.org>
Download mbox | patch
Permalink /patch/76745/
State Accepted
Commit 5a9a2c55d14260bc5b9fb6c36e2b32ee325a8aec
Headers show

Comments

Mike Frysinger - Dec. 27, 2010, 4:48 a.m.
The recent global data changes (making the size autogenerated) broke the
board info handling on Blackfin ports as we were lying and lumping the
bd_t size in with the gd_t size.  So use the new dedicated bd_t size to
setup its own address in memory.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 arch/blackfin/include/asm/config.h |    5 ++++-
 arch/blackfin/lib/board.c          |   10 ++--------
 2 files changed, 6 insertions(+), 9 deletions(-)
Sergei Shtylyov - Dec. 27, 2010, 11:41 a.m.
Hello.

On 27-12-2010 7:48, Mike Frysinger wrote:

> The recent global data changes (making the size autogenerated) broke the
> board info handling on Blackfin ports as we were lying and lumping the
> bd_t size in with the gd_t size.  So use the new dedicated bd_t size to
> setup its own address in memory.

> Signed-off-by: Mike Frysinger<vapier@gentoo.org>
[...]

> diff --git a/arch/blackfin/lib/board.c b/arch/blackfin/lib/board.c
> index 2b1f78c..47d487f 100644
> --- a/arch/blackfin/lib/board.c
> +++ b/arch/blackfin/lib/board.c
[...]
> @@ -244,14 +243,9 @@ void board_init_f(ulong bootflag)
>   	gd = (gd_t *) (CONFIG_SYS_GBL_DATA_ADDR);
>   	memset((void *)gd, 0, GENERATED_GBL_DATA_SIZE);
>
> -	/* Board data initialization */
> -	addr = (CONFIG_SYS_GBL_DATA_ADDR + sizeof(gd_t));
> -
> -	/* Align to 4 byte boundary */
> -	addr&= ~(4 - 1);
> -	bd = (bd_t *) addr;
> +	bd = (bd_t *) (CONFIG_SYS_BD_INFO_ADDR);

    Parens not needed around CONFIG_SYS_BD_INFO_ADDR.

WBR, Sergei
Mike Frysinger - Dec. 27, 2010, 4:42 p.m.
On Monday, December 27, 2010 06:41:47 Sergei Shtylyov wrote:
> On 27-12-2010 7:48, Mike Frysinger wrote:
> > +	bd = (bd_t *) (CONFIG_SYS_BD_INFO_ADDR);
> 
>     Parens not needed around CONFIG_SYS_BD_INFO_ADDR.

it isnt a problem to have the parens, and it keeps things sane if someone does 
something like:
#define CONFIG_SYS_BD_INFO_ADDR   SOME_DEFINE + 0x1000
-mike
Wolfgang Denk - Jan. 10, 2011, 10:28 p.m.
Dear Mike Frysinger,

In message <201012271142.17334.vapier@gentoo.org> you wrote:
>
> it isnt a problem to have the parens, and it keeps things sane if someone does 
> something like:
> #define CONFIG_SYS_BD_INFO_ADDR   SOME_DEFINE + 0x1000

This would be a violation of basic rules of defensive coding.

Please drop these parens.

Best regards,

Wolfgang Denk
Mike Frysinger - Jan. 11, 2011, 12:31 a.m.
On Monday, January 10, 2011 17:28:23 Wolfgang Denk wrote:
> Mike Frysinger wrote:
> > it isnt a problem to have the parens, and it keeps things sane if someone
> > does something like:
> > #define CONFIG_SYS_BD_INFO_ADDR   SOME_DEFINE + 0x1000
> 
> This would be a violation of basic rules of defensive coding.
> 
> Please drop these parens.

i dont see how these two statements are compatible.  defensive coding would 
mean i keep the parens.
-mike
Sergei Shtylyov - Jan. 12, 2011, 1:10 p.m.
On 11.01.2011 3:31, Mike Frysinger wrote:

>>> it isnt a problem to have the parens, and it keeps things sane if someone
>>> does something like:
>>> #define CONFIG_SYS_BD_INFO_ADDR   SOME_DEFINE + 0x1000

>> This would be a violation of basic rules of defensive coding.

>> Please drop these parens.

> i dont see how these two statements are compatible.  defensive coding would
> mean i keep the parens.

    I think Wolfgang meant that defining a macro without parens would be a 
violation -- which should be fixed, not worked around.

> -mike

WBR, Sergei

Patch

diff --git a/arch/blackfin/include/asm/config.h b/arch/blackfin/include/asm/config.h
index f0f3a39..89814cd 100644
--- a/arch/blackfin/include/asm/config.h
+++ b/arch/blackfin/include/asm/config.h
@@ -109,8 +109,11 @@ 
 #ifndef CONFIG_SYS_GBL_DATA_ADDR
 # define CONFIG_SYS_GBL_DATA_ADDR (CONFIG_SYS_MALLOC_BASE - GENERATED_GBL_DATA_SIZE)
 #endif
+#ifndef CONFIG_SYS_BD_INFO_ADDR
+# define CONFIG_SYS_BD_INFO_ADDR (CONFIG_SYS_GBL_DATA_ADDR - GENERATED_BD_INFO_SIZE)
+#endif
 #ifndef CONFIG_STACKBASE
-# define CONFIG_STACKBASE (CONFIG_SYS_GBL_DATA_ADDR - 4)
+# define CONFIG_STACKBASE (CONFIG_SYS_BD_INFO_ADDR - 4)
 #endif
 #ifndef CONFIG_SYS_MEMTEST_START
 # define CONFIG_SYS_MEMTEST_START 0
diff --git a/arch/blackfin/lib/board.c b/arch/blackfin/lib/board.c
index 2b1f78c..47d487f 100644
--- a/arch/blackfin/lib/board.c
+++ b/arch/blackfin/lib/board.c
@@ -207,7 +207,6 @@  extern int timer_init(void);
 
 void board_init_f(ulong bootflag)
 {
-	ulong addr;
 	bd_t *bd;
 	char buf[32];
 
@@ -244,14 +243,9 @@  void board_init_f(ulong bootflag)
 	gd = (gd_t *) (CONFIG_SYS_GBL_DATA_ADDR);
 	memset((void *)gd, 0, GENERATED_GBL_DATA_SIZE);
 
-	/* Board data initialization */
-	addr = (CONFIG_SYS_GBL_DATA_ADDR + sizeof(gd_t));
-
-	/* Align to 4 byte boundary */
-	addr &= ~(4 - 1);
-	bd = (bd_t *) addr;
+	bd = (bd_t *) (CONFIG_SYS_BD_INFO_ADDR);
 	gd->bd = bd;
-	memset((void *)bd, 0, sizeof(bd_t));
+	memset((void *)bd, 0, GENERATED_BD_INFO_SIZE);
 
 	bd->bi_r_version = version_string;
 	bd->bi_cpu = MK_STR(CONFIG_BFIN_CPU);