[U-Boot] [PATCH 1/3] lib/asm-offsets.c: Clean coding style
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; }
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; }
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
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
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
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
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
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
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
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
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
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
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
Hello.
On 21-12-2010 21:12, Michal Simek wrote:
Clear coding style issues.
Signed-off-by: Michal Simekmonstr@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
participants (3)
-
Michal Simek -
Sergei Shtylyov -
Wolfgang Denk