[U-Boot-Users] [PATCH] ColdFire: Assign version_string as data section in start.S

The compiler does not place the .ascii in start.S to data section, instead it put in under text section. This is an issue where it never gets notice and causes error until an update for tools/setlocalversion has been applied. A label of .data before .globl version_string will force to put under data section.
Signed-off-by: TsiChung Liew Tsi-Chung.Liew@freescale.com --- cpu/mcf5227x/start.S | 4 ++-- cpu/mcf523x/start.S | 4 ++-- cpu/mcf52x2/start.S | 4 ++-- cpu/mcf5445x/start.S | 4 ++-- cpu/mcf547x_8x/start.S | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/cpu/mcf5227x/start.S b/cpu/mcf5227x/start.S index 0e2db12..454251a 100644 --- a/cpu/mcf5227x/start.S +++ b/cpu/mcf5227x/start.S @@ -22,7 +22,7 @@ */
#include <config.h> -#include "version.h" +#include <version.h>
#ifndef CONFIG_IDENT_STRING #define CONFIG_IDENT_STRING "" @@ -348,7 +348,7 @@ dcache_status: rts
/*------------------------------------------------------------------------------*/ - + .data .globl version_string version_string: .ascii U_BOOT_VERSION diff --git a/cpu/mcf523x/start.S b/cpu/mcf523x/start.S index 2bd603d..30c1b50 100644 --- a/cpu/mcf523x/start.S +++ b/cpu/mcf523x/start.S @@ -22,7 +22,7 @@ */
#include <config.h> -#include "version.h" +#include <version.h>
#ifndef CONFIG_IDENT_STRING #define CONFIG_IDENT_STRING "" @@ -332,7 +332,7 @@ dcache_status: rts
/*------------------------------------------------------------------------------*/ - + .data .globl version_string version_string: .ascii U_BOOT_VERSION diff --git a/cpu/mcf52x2/start.S b/cpu/mcf52x2/start.S index 9e496a4..c11d0cb 100644 --- a/cpu/mcf52x2/start.S +++ b/cpu/mcf52x2/start.S @@ -22,7 +22,7 @@ */
#include <config.h> -#include "version.h" +#include <version.h>
#ifndef CONFIG_IDENT_STRING #define CONFIG_IDENT_STRING "" @@ -470,7 +470,7 @@ dcache_status: rts
/*------------------------------------------------------------------------------*/ - + .data .globl version_string version_string: .ascii U_BOOT_VERSION diff --git a/cpu/mcf5445x/start.S b/cpu/mcf5445x/start.S index d64c5af..7532490 100644 --- a/cpu/mcf5445x/start.S +++ b/cpu/mcf5445x/start.S @@ -22,7 +22,7 @@ */
#include <config.h> -#include "version.h" +#include <version.h>
#ifndef CONFIG_IDENT_STRING #define CONFIG_IDENT_STRING "" @@ -373,7 +373,7 @@ dcache_status: rts
/*------------------------------------------------------------------------------*/ - + .data .globl version_string version_string: .ascii U_BOOT_VERSION diff --git a/cpu/mcf547x_8x/start.S b/cpu/mcf547x_8x/start.S index 442665f..c516341 100644 --- a/cpu/mcf547x_8x/start.S +++ b/cpu/mcf547x_8x/start.S @@ -22,7 +22,7 @@ */
#include <config.h> -#include "version.h" +#include <version.h>
#ifndef CONFIG_IDENT_STRING #define CONFIG_IDENT_STRING "" @@ -353,7 +353,7 @@ dcache_status: rts
/*------------------------------------------------------------------------------*/ - + .data .globl version_string version_string: .ascii U_BOOT_VERSION

In message 1206531845-30730-1-git-send-email-Tsi-Chung.Liew@freescale.com you wrote:
The compiler does not place the .ascii in start.S to data section, instead it put in under text section. This is an issue where it
This is intentional on most architectures. What exactly is your problem?
never gets notice and causes error until an update for tools/setlocalversion has been applied. A label of .data before .globl version_string will force to put under data section.
Can you please explain which exact bug this is supposed to fix?
Best regards,
Wolfgang Denk

Wolfgang,
In message 1206531845-30730-1-git-send-email-Tsi-Chung.Liew@freescale.com you wrote:
The compiler does not place the .ascii in start.S to data section, instead it put in under text section. This is an issue where it
This is intentional on most architectures. What exactly is your problem?
Apparently, it is not the case in ColdFire compilers. The ColdFire compilers always put the version_string in text section if .data is not declared and shows build error in start.S - "unaligned opcodes detected in executable segment". The PowerPC compiler, however, will put the version_string in data section either with or without .data declared.
Unfortunately, I did not catch the error in the previous release (1.3.0 - 1.3.2), and probably based on luck that the u-boot version, date and time information was always 4-byte align when I retrieved and worked on the source. Until now, the version, date and time information was not 4-byte align. I agreed with you on previous email after I submitted the patch "Add brackets to if condition in tools/setlocalversion", there is nothing wrong with the Linux script.
never gets notice and causes error until an update for tools/setlocalversion has been applied. A label of .data before
.globl
version_string will force to put under data section.
Can you please explain which exact bug this is supposed to fix?
I am going to re-state the patch to "Fix build error generates by CF compilers for all CF CPUs" or "Fix version_string not in data section generates by CF compilers". Will either one of these works for you?
Regards, TsiChung

In message 4791E710007FEB4BBF83775D787F462F06EAF585@az33exm22.fsl.freescale.net you wrote:
This is intentional on most architectures. What exactly is your problem?
Apparently, it is not the case in ColdFire compilers. The ColdFire compilers always put the version_string in text section if .data is not declared and shows build error in start.S - "unaligned opcodes detected in executable segment". The PowerPC compiler, however, will put the version_string in data section either with or without .data declared.
No, that's not correct. See for example here:
"cpu/mpc8xx/start.S":
85 .text 86 .long 0x27051956 /* U-Boot Magic Number */ 87 .globl version_string 88 version_string: 89 .ascii U_BOOT_VERSION 90 .ascii " (", __DATE__, " - ", __TIME__, ")" 91 .ascii CONFIG_IDENT_STRING, "\0" 92 93 . = EXC_OFF_SYS_RESET 94 .globl _start 95 _start: 96 lis r3, CFG_IMMR@h /* position IMMR */ ...
Here we explicitely (and intentionally) place the version string in the text segment. It will NOT be put in the data segment.
Maybe just adding some ".align" directive helps?
I am going to re-state the patch to "Fix build error generates by CF compilers for all CF CPUs" or "Fix version_string not in data section generates by CF compilers". Will either one of these works for you?
I would like to understand the problem first. I feel you might just be missing some alignment... It has been working so far, hasn't it?
Best regards,
Wolfgang Denk

Wolfgang,
Maybe just adding some ".align" directive helps?
The example provided for 8xx or others, there is an offset alignment after CONFIG_IDENT_STRING - ". = EXC_OFF_SYS_RESET". In fact, it works after adding ".align 4" after CONFIG_IDENT_STRING.
Thanks.
Regards, TsiChung

In message 4791E710007FEB4BBF83775D787F462F06EAF645@az33exm22.fsl.freescale.net you wrote:
Maybe just adding some ".align" directive helps?
The example provided for 8xx or others, there is an offset alignment after CONFIG_IDENT_STRING - ". =3D EXC_OFF_SYS_RESET". In fact, it works after adding ".align 4" after CONFIG_IDENT_STRING.
Ah, great! Now *that* patch will be merged really fast - promised!
Best regards,
Wolfgang Denk
participants (3)
-
Liew Tsi Chung
-
Tsi-Chung Liew
-
Wolfgang Denk