[U-Boot] [PATCH 1/3] lib/asm-offsets.c: Clean coding style

[U-Boot] [PATCH 1/3] lib/asm-offsets.c: Clean coding style

[Michal Simek]

Clear coding style issues.

Signed-off-by: Michal Simek <monstr@monstr.eu>
---
 lib/asm-offsets.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/asm-offsets.c b/lib/asm-offsets.c
index 2209561..f1af7e2 100644
--- a/lib/asm-offsets.c
+++ b/lib/asm-offsets.c
@@ -19,11 +19,11 @@
 
 #include <linux/kbuild.h>
 
-int main(void)
+int main (void)
 {
 	/* Round up to make sure size gives nice stack alignment */
-	DEFINE(GENERATED_GBL_DATA_SIZE,
-		(sizeof(struct global_data)+15) & ~15);
+	DEFINE (GENERATED_GBL_DATA_SIZE,
+		(sizeof (struct global_data) + 15) & ~15);
 
 	return 0;
 }
-- 
1.5.5.6

[U-Boot] [PATCH 2/3] Autogenerate GENERATED_BD_INFO_SIZE

[Michal Simek]

GENERATED_BD_INFO_SIZE represent sizeof bd_info structure
which is used across architectures.
This value can be used in assembler files and macros.

Signed-off-by: Michal Simek <monstr@monstr.eu>
---
 lib/asm-offsets.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/lib/asm-offsets.c b/lib/asm-offsets.c
index f1af7e2..8aba391 100644
--- a/lib/asm-offsets.c
+++ b/lib/asm-offsets.c
@@ -24,6 +24,7 @@ int main (void)
 	/* Round up to make sure size gives nice stack alignment */
 	DEFINE (GENERATED_GBL_DATA_SIZE,
 		(sizeof (struct global_data) + 15) & ~15);
+	DEFINE (GENERATED_BD_INFO_SIZE, (sizeof (struct bd_info) + 15) & ~15);
 
 	return 0;
 }
-- 
1.5.5.6

[U-Boot] [PATCH 3/3] microblaze: Fix bdiinfo pointer

[Michal Simek]

Patch "Replace CONFIG_SYS_GBL_DATA_SIZE by auto-generated value"
(sha1: 25ddd1fb0a2281b182529afbc8fda5de2dc16d96)
introduce GENERATED_GBL_DATA_SIZE which is sizeof aligned gd_t
(currently 0x40).
Microblaze configs used 0x40(128) because this place also contained
board info structure which lies on the top of ram.

U-Boot is placed to the top of the ram (for example 0xd7ffffff)
and bd structure was moved out of ram.

This patch is fixing this scheme with GENERATED_BD_INFO_SIZE
which swap global data and board info structures.

For example:
Current: gd 0xd7ffffc0, bd 0xd8000000
Fixed:   gd 0xd7ffffc0, bd 0xd7ffff90

Signed-off-by: Michal Simek <monstr@monstr.eu>
---
 arch/microblaze/lib/board.c          |    7 ++++---
 include/configs/microblaze-generic.h |    5 +++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c
index eeef579..8232cf0 100644
--- a/arch/microblaze/lib/board.c
+++ b/arch/microblaze/lib/board.c
@@ -91,15 +91,16 @@ void board_init (void)
 	bd_t *bd;
 	init_fnc_t **init_fnc_ptr;
 	gd = (gd_t *) CONFIG_SYS_GBL_DATA_OFFSET;
+	bd = (bd_t *) CONFIG_SYS_GBL_DATA_OFFSET - GENERATED_BD_INFO_SIZE;
 	char *s;
 #if defined(CONFIG_CMD_FLASH)
 	ulong flash_size = 0;
 #endif
 	asm ("nop");	/* FIXME gd is not initialize - wait */
-	memset ((void *)gd, 0, GENERATED_GBL_DATA_SIZE);
-	gd->bd = (bd_t *) (gd + 1);	/* At end of global data */
+	memset ((void *)bd, 0, GENERATED_GBL_DATA_SIZE
+						+ GENERATED_BD_INFO_SIZE);
+	gd->bd = bd;
 	gd->baudrate = CONFIG_BAUDRATE;
-	bd = gd->bd;
 	bd->bi_baudrate = CONFIG_BAUDRATE;
 	bd->bi_memstart = CONFIG_SYS_SDRAM_BASE;
 	bd->bi_memsize = CONFIG_SYS_SDRAM_SIZE;
diff --git a/include/configs/microblaze-generic.h b/include/configs/microblaze-generic.h
index 75e4e07..fdfc0d8 100644
--- a/include/configs/microblaze-generic.h
+++ b/include/configs/microblaze-generic.h
@@ -142,9 +142,10 @@
 
 /* monitor code */
 #define	SIZE				0x40000
-#define	CONFIG_SYS_MONITOR_LEN		(SIZE - GENERATED_GBL_DATA_SIZE)
+#define	CONFIG_SYS_MONITOR_LEN		SIZE
 #define	CONFIG_SYS_MONITOR_BASE	\
-			(CONFIG_SYS_GBL_DATA_OFFSET - CONFIG_SYS_MONITOR_LEN)
+		(CONFIG_SYS_GBL_DATA_OFFSET - CONFIG_SYS_MONITOR_LEN \
+						- GENERATED_BD_INFO_SIZE)
 #define	CONFIG_SYS_MONITOR_END \
 			(CONFIG_SYS_MONITOR_BASE + CONFIG_SYS_MONITOR_LEN)
 #define	CONFIG_SYS_MALLOC_LEN		SIZE
-- 
1.5.5.6

[U-Boot] [PATCH 1/3] lib/asm-offsets.c: Clean coding style

[Wolfgang Denk]

Dear Michal Simek,

In message <1292955178-13018-1-git-send-email-monstr@monstr.eu> you wrote:
> Clear coding style issues.
> 
> Signed-off-by: Michal Simek <monstr@monstr.eu>
> ---
>  lib/asm-offsets.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/asm-offsets.c b/lib/asm-offsets.c
> index 2209561..f1af7e2 100644
> --- a/lib/asm-offsets.c
> +++ b/lib/asm-offsets.c
> @@ -19,11 +19,11 @@
>  
>  #include <linux/kbuild.h>
>  
> -int main(void)
> +int main (void)
>  {
>  	/* Round up to make sure size gives nice stack alignment */
> -	DEFINE(GENERATED_GBL_DATA_SIZE,
> -		(sizeof(struct global_data)+15) & ~15);
> +	DEFINE (GENERATED_GBL_DATA_SIZE,
> +		(sizeof (struct global_data) + 15) & ~15);

These changes are to the worse.  All.  Why do you think this would be
better?


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Weekends were made for programming.                 - Karl Lehenbauer

[U-Boot] [PATCH 2/3] Autogenerate GENERATED_BD_INFO_SIZE

[Wolfgang Denk]

Dear Michal Simek,

In message <1292955178-13018-2-git-send-email-monstr@monstr.eu> you wrote:
> GENERATED_BD_INFO_SIZE represent sizeof bd_info structure
> which is used across architectures.
> This value can be used in assembler files and macros.
> 
> Signed-off-by: Michal Simek <monstr@monstr.eu>
> ---
>  lib/asm-offsets.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/lib/asm-offsets.c b/lib/asm-offsets.c
> index f1af7e2..8aba391 100644
> --- a/lib/asm-offsets.c
> +++ b/lib/asm-offsets.c
> @@ -24,6 +24,7 @@ int main (void)
>  	/* Round up to make sure size gives nice stack alignment */
>  	DEFINE (GENERATED_GBL_DATA_SIZE,
>  		(sizeof (struct global_data) + 15) & ~15);
> +	DEFINE (GENERATED_BD_INFO_SIZE, (sizeof (struct bd_info) + 15) & ~15);

Please remove the space between macro name and argument list.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Anything free is worth what you pay for it.

[U-Boot] [PATCH 3/3] microblaze: Fix bdiinfo pointer

[Wolfgang Denk]

Dear Michal Simek,

In message <1292955178-13018-3-git-send-email-monstr@monstr.eu> you wrote:
> Patch "Replace CONFIG_SYS_GBL_DATA_SIZE by auto-generated value"
> (sha1: 25ddd1fb0a2281b182529afbc8fda5de2dc16d96)
> introduce GENERATED_GBL_DATA_SIZE which is sizeof aligned gd_t
> (currently 0x40).
> Microblaze configs used 0x40(128) because this place also contained
> board info structure which lies on the top of ram.

In the Subject: s/bdiinfo/bd_info/

> index eeef579..8232cf0 100644
> --- a/arch/microblaze/lib/board.c
> +++ b/arch/microblaze/lib/board.c
> @@ -91,15 +91,16 @@ void board_init (void)
>  	bd_t *bd;
>  	init_fnc_t **init_fnc_ptr;
>  	gd = (gd_t *) CONFIG_SYS_GBL_DATA_OFFSET;
> +	bd = (bd_t *) CONFIG_SYS_GBL_DATA_OFFSET - GENERATED_BD_INFO_SIZE;

This is actually wrong.

You are using CONFIG_SYS_GBL_DATA_OFFSET as if it were
CONFIG_SYS_GBL_DATA_ADDR, but it ain't so:  it is an _offset_, and NOT
and address.

> -	memset ((void *)gd, 0, GENERATED_GBL_DATA_SIZE);
> -	gd->bd = (bd_t *) (gd + 1);	/* At end of global data */
> +	memset ((void *)bd, 0, GENERATED_GBL_DATA_SIZE
> +						+ GENERATED_BD_INFO_SIZE);

Don't do this. Instead, use two separate memset() calls, one for gd
and another one for bd. The stucts may be in a contiguous area now,
but you would probably run into nasty bugs if this gets changed one
day.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I usually tell my classes "if you are using @ and [] together in this
class, you will almost certainly NOT get what you want. That's  going
down  the wrong tunnel. There's no cheese at the end of that tunnel."
         -- Randal L. Schwartz in <8czptuomey.fsf@gadget.cscaper.com>

[U-Boot] [PATCH 1/3] lib/asm-offsets.c: Clean coding style

[Michal Simek]

Wolfgang Denk wrote:
> Dear Michal Simek,
> 
> In message <1292955178-13018-1-git-send-email-monstr@monstr.eu> you wrote:
>> Clear coding style issues.
>>
>> Signed-off-by: Michal Simek <monstr@monstr.eu>
>> ---
>>  lib/asm-offsets.c |    6 +++---
>>  1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/asm-offsets.c b/lib/asm-offsets.c
>> index 2209561..f1af7e2 100644
>> --- a/lib/asm-offsets.c
>> +++ b/lib/asm-offsets.c
>> @@ -19,11 +19,11 @@
>>  
>>  #include <linux/kbuild.h>
>>  
>> -int main(void)
>> +int main (void)
>>  {
>>  	/* Round up to make sure size gives nice stack alignment */
>> -	DEFINE(GENERATED_GBL_DATA_SIZE,
>> -		(sizeof(struct global_data)+15) & ~15);
>> +	DEFINE (GENERATED_GBL_DATA_SIZE,
>> +		(sizeof (struct global_data) + 15) & ~15);
> 
> These changes are to the worse.  All.  Why do you think this would be
> better?

It is what intend suggest to do.
intend -npro -kr -i8 -ts8 -sob -l80 -ss -ncs -cp1 -pcs

I am OK to remove space between macro name and argument as you suggested 
in your other email.

 >> +	DEFINE (GENERATED_GBL_DATA_SIZE,
 >> +		(sizeof (struct global_data) + 15) & ~15);
                                             ^ ^   ^ ^

I believe that marked space are OK, or not?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

[U-Boot] [PATCH 3/3] microblaze: Fix bdiinfo pointer

[Michal Simek]

Wolfgang Denk wrote:
> Dear Michal Simek,
> 
> In message <1292955178-13018-3-git-send-email-monstr@monstr.eu> you wrote:
>> Patch "Replace CONFIG_SYS_GBL_DATA_SIZE by auto-generated value"
>> (sha1: 25ddd1fb0a2281b182529afbc8fda5de2dc16d96)
>> introduce GENERATED_GBL_DATA_SIZE which is sizeof aligned gd_t
>> (currently 0x40).
>> Microblaze configs used 0x40(128) because this place also contained
>> board info structure which lies on the top of ram.
> 
> In the Subject: s/bdiinfo/bd_info/
> 
>> index eeef579..8232cf0 100644
>> --- a/arch/microblaze/lib/board.c
>> +++ b/arch/microblaze/lib/board.c
>> @@ -91,15 +91,16 @@ void board_init (void)
>>  	bd_t *bd;
>>  	init_fnc_t **init_fnc_ptr;
>>  	gd = (gd_t *) CONFIG_SYS_GBL_DATA_OFFSET;
>> +	bd = (bd_t *) CONFIG_SYS_GBL_DATA_OFFSET - GENERATED_BD_INFO_SIZE;
> 
> This is actually wrong.
> 
> You are using CONFIG_SYS_GBL_DATA_OFFSET as if it were
> CONFIG_SYS_GBL_DATA_ADDR, but it ain't so:  it is an _offset_, and NOT
> and address.

I agree. BTW: Maybe nios2 and sparc use it too.

> 
>> -	memset ((void *)gd, 0, GENERATED_GBL_DATA_SIZE);
>> -	gd->bd = (bd_t *) (gd + 1);	/* At end of global data */
>> +	memset ((void *)bd, 0, GENERATED_GBL_DATA_SIZE
>> +						+ GENERATED_BD_INFO_SIZE);
> 
> Don't do this. Instead, use two separate memset() calls, one for gd
> and another one for bd. The stucts may be in a contiguous area now,
> but you would probably run into nasty bugs if this gets changed one
> day.

I just wanted to save some instructions and no problem to separate it.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

[U-Boot] [PATCH 1/3] lib/asm-offsets.c: Clean coding style

[Wolfgang Denk]

Dear Michal Simek,

In message <4D1107D2.7070607@monstr.eu> you wrote:
>
> > These changes are to the worse.  All.  Why do you think this would be
> > better?
> 
> It is what intend suggest to do.
> intend -npro -kr -i8 -ts8 -sob -l80 -ss -ncs -cp1 -pcs

Please omit the "-pcs" part.  It has always been my personal
preference, but I've been overruled, and we use plain Lindent these
days.

> I am OK to remove space between macro name and argument as you suggested 
> in your other email.
> 
>  >> +	DEFINE (GENERATED_GBL_DATA_SIZE,
>  >> +		(sizeof (struct global_data) + 15) & ~15);
>                                              ^ ^   ^ ^
> 
> I believe that marked space are OK, or not?

Yes.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"If God had wanted us to use the metric system, Jesus would have  had
10 apostles."

[U-Boot] [PATCH 3/3] microblaze: Fix bdiinfo pointer

[Wolfgang Denk]

Dear Michal Simek,

In message <4D1111CF.5090002@monstr.eu> you wrote:
>
> >> @@ -91,15 +91,16 @@ void board_init (void)
> >>  	bd_t *bd;
> >>  	init_fnc_t **init_fnc_ptr;
> >>  	gd = (gd_t *) CONFIG_SYS_GBL_DATA_OFFSET;
> >> +	bd = (bd_t *) CONFIG_SYS_GBL_DATA_OFFSET - GENERATED_BD_INFO_SIZE;
> > 
> > This is actually wrong.
> > 
> > You are using CONFIG_SYS_GBL_DATA_OFFSET as if it were
> > CONFIG_SYS_GBL_DATA_ADDR, but it ain't so:  it is an _offset_, and NOT
> > and address.
> 
> I agree. BTW: Maybe nios2 and sparc use it too.

I see - I put the custodians on Cc:.

> >> -	memset ((void *)gd, 0, GENERATED_GBL_DATA_SIZE);
> >> -	gd->bd = (bd_t *) (gd + 1);	/* At end of global data */
> >> +	memset ((void *)bd, 0, GENERATED_GBL_DATA_SIZE
> >> +						+ GENERATED_BD_INFO_SIZE);
> > 
> > Don't do this. Instead, use two separate memset() calls, one for gd
> > and another one for bd. The stucts may be in a contiguous area now,
> > but you would probably run into nasty bugs if this gets changed one
> > day.
> 
> I just wanted to save some instructions and no problem to separate it.

Yes, I understand this, but it's a dangerous thing to so, and
robust and maintainable code is more important than a few bytes.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Never call a man a fool.  Borrow from him.

[U-Boot] [PATCH 1/3] lib/asm-offsets.c: Clean coding style

[Michal Simek]

Wolfgang Denk wrote:
> Dear Michal Simek,
> 
> In message <4D1107D2.7070607@monstr.eu> you wrote:
>>> These changes are to the worse.  All.  Why do you think this would be
>>> better?
>> It is what intend suggest to do.
>> intend -npro -kr -i8 -ts8 -sob -l80 -ss -ncs -cp1 -pcs
> 
> Please omit the "-pcs" part.  It has always been my personal
> preference, but I've been overruled, and we use plain Lindent these
> days.

Ok. Would it be possible to remove it from coding style page.
http://www.denx.de/wiki/U-Boot/CodingStyle

I just used what there is.

Best regards,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

[U-Boot] [PATCH 1/3] lib/asm-offsets.c: Clean coding style

[Wolfgang Denk]

Dear Michal Simek,

In message <4D119D06.2010709@monstr.eu> you wrote:
>
> > Please omit the "-pcs" part.  It has always been my personal
> > preference, but I've been overruled, and we use plain Lindent these
> > days.
> 
> Ok. Would it be possible to remove it from coding style page.
> http://www.denx.de/wiki/U-Boot/CodingStyle

Sure, done.

Note: you could have fixed this yourself.  The DULG is a wiki, and
everybody can contribute to correct and extend the documentation.

> I just used what there is.

I understand, sorry for the confusion.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Beware of programmers carrying screwdrivers."      - Chip Salzenberg

[U-Boot] [PATCH 1/3] lib/asm-offsets.c: Clean coding style

[Michal Simek]

Wolfgang Denk wrote:
> Dear Michal Simek,
> 
> In message <4D119D06.2010709@monstr.eu> you wrote:
>>> Please omit the "-pcs" part.  It has always been my personal
>>> preference, but I've been overruled, and we use plain Lindent these
>>> days.
>> Ok. Would it be possible to remove it from coding style page.
>> http://www.denx.de/wiki/U-Boot/CodingStyle
> 
> Sure, done.
> 
> Note: you could have fixed this yourself.  The DULG is a wiki, and
> everybody can contribute to correct and extend the documentation.
> 
>> I just used what there is.
> 
> I understand, sorry for the confusion.

No problem,

Thanks,
Michal



-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

[U-Boot] [PATCH 1/3] lib/asm-offsets.c: Clean coding style

[Sergei Shtylyov]

Hello.

On 21-12-2010 21:12, Michal Simek wrote:

> Clear coding style issues.

> Signed-off-by: Michal Simek<monstr@monstr.eu>
[...]

> diff --git a/lib/asm-offsets.c b/lib/asm-offsets.c
> index 2209561..f1af7e2 100644
> --- a/lib/asm-offsets.c
> +++ b/lib/asm-offsets.c
> @@ -19,11 +19,11 @@
>
>   #include<linux/kbuild.h>
>
> -int main(void)
> +int main (void)

    Why add space before paren? checkpatch.pl wouldn't like it.

>   {
>   	/* Round up to make sure size gives nice stack alignment */
> -	DEFINE(GENERATED_GBL_DATA_SIZE,
> -		(sizeof(struct global_data)+15) & ~15);
> +	DEFINE (GENERATED_GBL_DATA_SIZE,

    Same here...

WBR, Sergei