Patchwork [U-Boot] tegra: override compiler flags for cmd_nvedit

login
register
mail settings
Submitter Allen Martin
Date May 4, 2012, 8:29 p.m.
Message ID <1336163380-29679-1-git-send-email-amartin@nvidia.com>
Download mbox | patch
Permalink /patch/156985/
State Superseded
Headers show

Comments

Allen Martin - May 4, 2012, 8:29 p.m.
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(+)
Wolfgang Denk - May 5, 2012, 8:01 p.m.
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
Allen Martin - May 7, 2012, 4:36 p.m.
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.
-----------------------------------------------------------------------------------
Mike Frysinger - May 14, 2012, 5:24 a.m.
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
Allen Martin - May 14, 2012, 7:39 p.m.
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

Patch

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