Message ID | 1336163380-29679-1-git-send-email-amartin@nvidia.com |
---|---|
State | Superseded |
Headers | show |
Dear Allen Martin, In message <1336163380-29679-1-git-send-email-amartin@nvidia.com> you wrote: > Override -march setting for tegra to -march=armv4t for cmd_nvedit.c > > The recent change to use -march=armv7-a for armv7 caused a regression > on tegra because tegra starts boot on a arm7tdmi processor before > transferring control to the cortex-a9. While still executing on the > arm7tdmi there is a call to getenv_ulong() that causes an illegal > instruction exception if cmd_nvedit is compiled for armv7. > > Signed-off-by: Allen Martin <amartin@nvidia.com> > --- > common/Makefile | 6 ++++++ > 1 file changed, 6 insertions(+) We definitely do not want to introduce board specific compile options in common code, and especially not on a per-file base. Best regards, Wolfgang Denk
On Sat, May 05, 2012 at 01:01:22PM -0700, Wolfgang Denk wrote: > Dear Allen Martin, > > In message <1336163380-29679-1-git-send-email-amartin@nvidia.com> you wrote: > > Override -march setting for tegra to -march=armv4t for cmd_nvedit.c > > > > The recent change to use -march=armv7-a for armv7 caused a regression > > on tegra because tegra starts boot on a arm7tdmi processor before > > transferring control to the cortex-a9. While still executing on the > > arm7tdmi there is a call to getenv_ulong() that causes an illegal > > instruction exception if cmd_nvedit is compiled for armv7. > > > > Signed-off-by: Allen Martin <amartin@nvidia.com> > > --- > > common/Makefile | 6 ++++++ > > 1 file changed, 6 insertions(+) > > We definitely do not want to introduce board specific compile > options in common code, and especially not on a per-file base. > The other option is to compile all of tegra armv4t by modifying arch/arm/cpu/armv7/config.mk, the resulting binary is about 300 bytes larger though, which is a shame since it's only this one file that really needs to be armv4t for the getenv_ulong() call. Does this seem like a more acceptable change though? I see either solution as a temporary band-aid to get tegra booting again until I finish the patch set to split out the armv4t code to a SPL. I hope to have a complete patch set ready to post for review in a few days. If you get a chance could you look over the following patches that I posted recently: tegra2: move tegra2 SoC code to arch/arm/cpu/tegra2-common mkconfig: add support for SPL CPU These are definately the most controversial patches in the patch set, and I wanted to get some feedback if I'm headed in the right direction. Thanks, -Allen ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. -----------------------------------------------------------------------------------
On Monday 07 May 2012 12:36:00 Allen Martin wrote: > On Sat, May 05, 2012 at 01:01:22PM -0700, Wolfgang Denk wrote: > > Allen Martin wrote: > > > Override -march setting for tegra to -march=armv4t for cmd_nvedit.c > > > > > > The recent change to use -march=armv7-a for armv7 caused a regression > > > on tegra because tegra starts boot on a arm7tdmi processor before > > > transferring control to the cortex-a9. While still executing on the > > > arm7tdmi there is a call to getenv_ulong() that causes an illegal > > > instruction exception if cmd_nvedit is compiled for armv7. > > > > > > Signed-off-by: Allen Martin <amartin@nvidia.com> > > > --- > > > > > > common/Makefile | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > We definitely do not want to introduce board specific compile > > options in common code, and especially not on a per-file base. > > The other option is to compile all of tegra armv4t by modifying > arch/arm/cpu/armv7/config.mk no it's not. arch and board config.mk get sourced, so you can put this board/arch specific cruft there. we do it for blackfin boards all the time ... see board/bf537-stamp/config.mk for example. -mike
On Sun, May 13, 2012 at 10:24:19PM -0700, Mike Frysinger wrote: > On Monday 07 May 2012 12:36:00 Allen Martin wrote: > > The other option is to compile all of tegra armv4t by modifying > > arch/arm/cpu/armv7/config.mk > > no it's not. arch and board config.mk get sourced, so you can put this > board/arch specific cruft there. we do it for blackfin boards all the time ... > see board/bf537-stamp/config.mk for example. > -mike > Thanks Mike, that definately sounds preferable, I'll take a look. I recently posted a patch series that makes this kind of hack unnecessary for tegra: "split tegra arm7 code into separate SPL". If you get a chance it could definately use a few more eyeballs :^) Especially the following two changes, which are probably the most controversial: tegra2: move tegra2 SoC code to arch/arm/cpu/tegra2-common mkconfig: add support for SPL CPU -Allen
diff --git a/common/Makefile b/common/Makefile index d9f10f3..e1b5307 100644 --- a/common/Makefile +++ b/common/Makefile @@ -43,6 +43,12 @@ COBJS-y += cmd_help.o COBJS-y += cmd_nvedit.o COBJS-y += cmd_version.o +# tegra calls getenv_ulong() from the context of the arm7tdmi processor +# so this needs to be compiled armv4t on tegra +ifneq ($(CONFIG_MACH_TEGRA_GENERIC),) +CFLAGS_common/cmd_nvedit.o += -march=armv4t +endif + # environment COBJS-y += env_common.o COBJS-$(CONFIG_ENV_IS_IN_DATAFLASH) += env_dataflash.o
Override -march setting for tegra to -march=armv4t for cmd_nvedit.c The recent change to use -march=armv7-a for armv7 caused a regression on tegra because tegra starts boot on a arm7tdmi processor before transferring control to the cortex-a9. While still executing on the arm7tdmi there is a call to getenv_ulong() that causes an illegal instruction exception if cmd_nvedit is compiled for armv7. Signed-off-by: Allen Martin <amartin@nvidia.com> --- common/Makefile | 6 ++++++ 1 file changed, 6 insertions(+)