Patchwork [U-Boot] tegra2: move tegra2 SoC code to arch/arm/cpu/tegra2-common

login
register
mail settings
Submitter Allen Martin
Date April 18, 2012, 10:46 p.m.
Message ID <1334789169-28311-1-git-send-email-amartin@nvidia.com>
Download mbox | patch
Permalink /patch/153618/
State Superseded
Headers show

Comments

Allen Martin - April 18, 2012, 10:46 p.m.
In preparation for splitting out the armv4t code from tegra2, move the
tegra2 SoC code to arch/arm/cpu/tegra2-common.  This code will be
compiled armv4t for the arm7tdmi and armv7 for the cortex A9.

Signed-off-by: Allen Martin <amartin@nvidia.com>
---
Resending this with "git format-patch -M" to make it more readable
and removing cover letter

This is part of an upcoming patch set to move all armv4t code out of
tegra2 u-boot and put it into an SPL, but I wanted to get some early
feedback on this patch.  This moves most of the tegra2 SoC code from
arch/arm/cpu/armv7 to a new directory arch/arm/cpu/tegra2-common.
This code will be shared between the armv7 and armv4t builds of
tegra2.

 Makefile                                           |    3 +
 arch/arm/cpu/armv7/tegra2/Makefile                 |   12 +----
 arch/arm/cpu/tegra2-common/Makefile                |   56 ++++++++++++++++++++
 .../arm/cpu/{armv7/tegra2 => tegra2-common}/ap20.c |    0
 .../arm/cpu/{armv7/tegra2 => tegra2-common}/ap20.h |    0
 .../cpu/{armv7/tegra2 => tegra2-common}/board.c    |    0
 .../cpu/{armv7/tegra2 => tegra2-common}/clock.c    |    0
 .../cpu/{armv7/tegra2 => tegra2-common}/funcmux.c  |    0
 .../tegra2 => tegra2-common}/lowlevel_init.S       |    0
 .../cpu/{armv7/tegra2 => tegra2-common}/pinmux.c   |    0
 .../cpu/{armv7/tegra2 => tegra2-common}/sys_info.c |    0
 .../cpu/{armv7/tegra2 => tegra2-common}/timer.c    |    0
 spl/Makefile                                       |    4 ++
 13 files changed, 65 insertions(+), 10 deletions(-)
 create mode 100644 arch/arm/cpu/tegra2-common/Makefile
 rename arch/arm/cpu/{armv7/tegra2 => tegra2-common}/ap20.c (100%)
 rename arch/arm/cpu/{armv7/tegra2 => tegra2-common}/ap20.h (100%)
 rename arch/arm/cpu/{armv7/tegra2 => tegra2-common}/board.c (100%)
 rename arch/arm/cpu/{armv7/tegra2 => tegra2-common}/clock.c (100%)
 rename arch/arm/cpu/{armv7/tegra2 => tegra2-common}/funcmux.c (100%)
 rename arch/arm/cpu/{armv7/tegra2 => tegra2-common}/lowlevel_init.S (100%)
 rename arch/arm/cpu/{armv7/tegra2 => tegra2-common}/pinmux.c (100%)
 rename arch/arm/cpu/{armv7/tegra2 => tegra2-common}/sys_info.c (100%)
 rename arch/arm/cpu/{armv7/tegra2 => tegra2-common}/timer.c (100%)
Simon Glass - April 19, 2012, 6:27 p.m.
+Albert

Hi Allen,

On Wed, Apr 18, 2012 at 3:46 PM, Allen Martin <amartin@nvidia.com> wrote:
> In preparation for splitting out the armv4t code from tegra2, move the
> tegra2 SoC code to arch/arm/cpu/tegra2-common.  This code will be
> compiled armv4t for the arm7tdmi and armv7 for the cortex A9.
>
> Signed-off-by: Allen Martin <amartin@nvidia.com>
> ---
> Resending this with "git format-patch -M" to make it more readable
> and removing cover letter
>
> This is part of an upcoming patch set to move all armv4t code out of
> tegra2 u-boot and put it into an SPL, but I wanted to get some early
> feedback on this patch.  This moves most of the tegra2 SoC code from
> arch/arm/cpu/armv7 to a new directory arch/arm/cpu/tegra2-common.
> This code will be shared between the armv7 and armv4t builds of
> tegra2.

I am not sure whether moving things up a level is OK. It makes some
sense to me but is different from what other boards do.

Secondly, you will at some point add Tegra 3 support I suppose. In
that case much of this code will become common anyway. Should you do
that split (tegra-common, tegra2-common) now or later?

Regards,
Simon
Allen Martin - April 19, 2012, 7:22 p.m.
On Thu, Apr 19, 2012 at 11:27:05AM -0700, Simon Glass wrote:
> >
> > This is part of an upcoming patch set to move all armv4t code out of
> > tegra2 u-boot and put it into an SPL, but I wanted to get some early
> > feedback on this patch.  This moves most of the tegra2 SoC code from
> > arch/arm/cpu/armv7 to a new directory arch/arm/cpu/tegra2-common.
> > This code will be shared between the armv7 and armv4t builds of
> > tegra2.
> 
> I am not sure whether moving things up a level is OK. It makes some
> sense to me but is different from what other boards do.

Agreed I couldn't find any other examples, but AFAIK no other board is
trying to do what we do (sharing code between 2 different CPU
models).  The other options I contemplated were:

-Have the armv4t build reach up and over into armv7
-Move the code out to board/nvidia

Both of these seemed worse


> Secondly, you will at some point add Tegra 3 support I suppose. In
> that case much of this code will become common anyway. Should you do
> that split (tegra-common, tegra2-common) now or later?

I'd like to keep the tegra3 changes separate since that will involve
refactoring of much of this code.  For the SPL build it's mostly
hidden in the config and Makefiles.  

I thought about doing exactly what you said to reduce the amount of
code I needed to pull into the SPL build, but what I found was that
because the SPL needs to initialize some regulators, clocks, and
pinmux to bring up the A9 and UART it ends up needing much of the
tegra2 SoC code, so there's not a lot of opportunity to minize the
code set for the SPL build beyond the CONFIG options to disable
drivers that area already there.

-Allen
nvpublic
Simon Glass - April 30, 2012, 6:31 p.m.
Hi Allen,

On Thu, Apr 19, 2012 at 12:22 PM, Allen Martin <amartin@nvidia.com> wrote:

> On Thu, Apr 19, 2012 at 11:27:05AM -0700, Simon Glass wrote:
> > >
> > > This is part of an upcoming patch set to move all armv4t code out of
> > > tegra2 u-boot and put it into an SPL, but I wanted to get some early
> > > feedback on this patch.  This moves most of the tegra2 SoC code from
> > > arch/arm/cpu/armv7 to a new directory arch/arm/cpu/tegra2-common.
> > > This code will be shared between the armv7 and armv4t builds of
> > > tegra2.
> >
> > I am not sure whether moving things up a level is OK. It makes some
> > sense to me but is different from what other boards do.
>
> Agreed I couldn't find any other examples, but AFAIK no other board is
> trying to do what we do (sharing code between 2 different CPU
> models).  The other options I contemplated were:
>
> -Have the armv4t build reach up and over into armv7
> -Move the code out to board/nvidia
>
> Both of these seemed worse
>

I've thought about this a bit more. To me you have a bit of a unique
problem in that you need to build the code as both ARMv4T and ARMv7. The
way it works at the moment is pretty simple - we just mark a few files that
must be build with ARMv4T and ARMv7 is happy with that also.

I believe the point of the 'cpu' directory is to separate out common code
between different architectures. It seems you want a chip which uses two
different architectures. That might be unique in the U-Boot world - I note
that other SOCs with different chips use ARMv7 for all of them.

But really, who cares what architecture you actually use? The code clearly
doesn't include ARMv7-isms otherwise it wouldn't build with current compile
options. It does work...

So my suggestion is that you just continue as now, and build the relevant
code for ARMv4T. I don't see a compelling reason to move it, least of all
into a no-man's land at the same level as 'cpu'.


>
> > Secondly, you will at some point add Tegra 3 support I suppose. In
> > that case much of this code will become common anyway. Should you do
> > that split (tegra-common, tegra2-common) now or later?
>
> I'd like to keep the tegra3 changes separate since that will involve
> refactoring of much of this code.  For the SPL build it's mostly
> hidden in the config and Makefiles.
>
> I thought about doing exactly what you said to reduce the amount of
> code I needed to pull into the SPL build, but what I found was that
> because the SPL needs to initialize some regulators, clocks, and
> pinmux to bring up the A9 and UART it ends up needing much of the
> tegra2 SoC code, so there's not a lot of opportunity to minize the
> code set for the SPL build beyond the CONFIG options to disable
> drivers that area already there.
>

OK. Please note that we do have a refactor in the Chromium tree to move
things into tegra-common and add T30 support. We hope to get this upstream
once the T20 stuff is there (NAND, keyboard and LCD are still pending).

Regards,
Simon


>
> -Allen
> nvpublic
>
Allen Martin - April 30, 2012, 7:52 p.m.
On Mon, Apr 30, 2012 at 11:31:44AM -0700, Simon Glass wrote:
> 
> -Have the armv4t build reach up and over into armv7
> -Move the code out to board/nvidia
> 
> Both of these seemed worse
> 
> I've thought about this a bit more. To me you have a bit of a unique problem in that you need to build the code as both ARMv4T and ARMv7. The way it works at the moment is pretty simple - we just mark a few files that must be build with ARMv4T and ARMv7 is happy with that also.
> 
> I believe the point of the 'cpu' directory is to separate out common code between different architectures. It seems you want a chip which uses two different architectures. That might be unique in the U-Boot world - I note that other SOCs with different chips use ARMv7 for all of them.
> 
> But really, who cares what architecture you actually use? The code clearly doesn't include ARMv7-isms otherwise it wouldn't build with current compile options. It does work...
> 
> So my suggestion is that you just continue as now, and build the relevant code for ARMv4T. I don't see a compelling reason to move it, least of all into a no-man's land at the same level as 'cpu'.
> 

The problem that pushed me over the edge on this was the patch series
to add thumb support for ARM.  The linker has to insert interworking
code for the cross ARM/thumb calls.  If it is linking armv4t and armv7
objects together it will promote them all to armv7 style interworking
which uses instructions that are illegal on armv4t.  The only fix for
this is to link armv4t objects separately or not support thumb, and we
would really like to support thumb.

Also it's not true that the code in arch/arm/cpu/armv7 doesn't include
any armv7'isms, we're just #ifdef around them for the tegra and call
them later in the boot sequence when we know we're running on the A9.
We also have to code the init sequence extremely carefully to make
sure we don't call any armv7 code on the armv4t.  It's extremely
fragile as any changes to start.S or the ordering of the init sequence
can (and have) broken us.

-Allen
--
nvpublic
Simon Glass - June 10, 2012, 3:16 a.m.
Hi Allen,

On Mon, Apr 30, 2012 at 12:52 PM, Allen Martin <amartin@nvidia.com> wrote:

> On Mon, Apr 30, 2012 at 11:31:44AM -0700, Simon Glass wrote:
> >
> > -Have the armv4t build reach up and over into armv7
> > -Move the code out to board/nvidia
> >
> > Both of these seemed worse
> >
> > I've thought about this a bit more. To me you have a bit of a unique
> problem in that you need to build the code as both ARMv4T and ARMv7. The
> way it works at the moment is pretty simple - we just mark a few files that
> must be build with ARMv4T and ARMv7 is happy with that also.
> >
> > I believe the point of the 'cpu' directory is to separate out common
> code between different architectures. It seems you want a chip which uses
> two different architectures. That might be unique in the U-Boot world - I
> note that other SOCs with different chips use ARMv7 for all of them.
> >
> > But really, who cares what architecture you actually use? The code
> clearly doesn't include ARMv7-isms otherwise it wouldn't build with current
> compile options. It does work...
> >
> > So my suggestion is that you just continue as now, and build the
> relevant code for ARMv4T. I don't see a compelling reason to move it, least
> of all into a no-man's land at the same level as 'cpu'.
> >
>
> The problem that pushed me over the edge on this was the patch series
> to add thumb support for ARM.  The linker has to insert interworking
> code for the cross ARM/thumb calls.  If it is linking armv4t and armv7
> objects together it will promote them all to armv7 style interworking
> which uses instructions that are illegal on armv4t.  The only fix for
> this is to link armv4t objects separately or not support thumb, and we
> would really like to support thumb.
>

OK I just remembered this thread...

The Thumb reason makes good sense to me. That said, you could arrange to
avoid interworked calls and have all the ARMv4 code built with the same
flags (and not allow the ARM7TDMI to enter board_init_f()).

So perhaps another way of putting it is that you are using SPL as a
convenient and existing way to allow you to built two parts of U-Boot with
separate flags.


>
> Also it's not true that the code in arch/arm/cpu/armv7 doesn't include
> any armv7'isms, we're just #ifdef around them for the tegra and call
> them later in the boot sequence when we know we're running on the A9.
> We also have to code the init sequence extremely carefully to make
> sure we don't call any armv7 code on the armv4t.  It's extremely
> fragile as any changes to start.S or the ordering of the init sequence
> can (and have) broken us.
>

See my other mail about this. I think you overstate things a little :-)
 But anyway, I think I understand the desirability for this now.


>
> -Allen
> --
> nvpublic
>

Regards,
Simon

Patch

diff --git a/Makefile b/Makefile
index 4ddf8d6..6639de0 100644
--- a/Makefile
+++ b/Makefile
@@ -319,6 +319,9 @@  endif
 ifeq ($(SOC),exynos)
 LIBS += $(CPUDIR)/s5p-common/libs5p-common.o
 endif
+ifeq ($(SOC),tegra2)
+LIBS += $(OBJTREE)/arch/$(ARCH)/cpu/$(SOC)-common/lib$(SOC)-common.o
+endif
 
 LIBS := $(addprefix $(obj),$(sort $(LIBS)))
 .PHONY : $(LIBS)
diff --git a/arch/arm/cpu/armv7/tegra2/Makefile b/arch/arm/cpu/armv7/tegra2/Makefile
index e9ac6c9..34452c4 100644
--- a/arch/arm/cpu/armv7/tegra2/Makefile
+++ b/arch/arm/cpu/armv7/tegra2/Makefile
@@ -22,23 +22,15 @@ 
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston,
 # MA 02111-1307 USA
 #
-
-# The AVP is ARMv4T architecture so we must use special compiler
-# flags for any startup files it might use.
-CFLAGS_arch/arm/cpu/armv7/tegra2/ap20.o += -march=armv4t
-CFLAGS_arch/arm/cpu/armv7/tegra2/clock.o += -march=armv4t
-
 include $(TOPDIR)/config.mk
 
 LIB	=  $(obj)lib$(SOC).o
 
-SOBJS	:= lowlevel_init.o
-COBJS-y	:= ap20.o board.o clock.o funcmux.o pinmux.o sys_info.o timer.o
 COBJS-$(CONFIG_USB_EHCI_TEGRA) += usb.o
 
 COBJS	:= $(COBJS-y)
-SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
-OBJS	:= $(addprefix $(obj),$(COBJS) $(SOBJS))
+SRCS	:= $(COBJS:.o=.c)
+OBJS	:= $(addprefix $(obj),$(COBJS))
 
 all:	 $(obj).depend $(LIB)
 
diff --git a/arch/arm/cpu/tegra2-common/Makefile b/arch/arm/cpu/tegra2-common/Makefile
new file mode 100644
index 0000000..8de59cf
--- /dev/null
+++ b/arch/arm/cpu/tegra2-common/Makefile
@@ -0,0 +1,56 @@ 
+#
+# (C) Copyright 2010,2011 Nvidia Corporation.
+#
+# (C) Copyright 2000-2008
+# Wolfgang Denk, DENX Software Engineering, wd@denx.de.
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+include $(TOPDIR)/config.mk
+
+# The AVP is ARMv4T architecture so we must use special compiler
+# flags for any startup files it might use.
+CFLAGS_arch/arm/cpu/tegra2-common/ap20.o += -march=armv4t
+CFLAGS_arch/arm/cpu/tegra2-common/clock.o += -march=armv4t
+
+LIB	= $(obj)lib$(SOC)-common.o
+
+SOBJS += lowlevel_init.o
+COBJS-y	+= ap20.o board.o clock.o funcmux.o pinmux.o sys_info.o timer.o
+
+SRCS	:= $(SOBJS:.o=.S) $(COBJS-y:.o=.c)
+OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS-y))
+
+all:	$(obj).depend $(LIB)
+
+$(LIB):	$(OBJS)
+	$(call cmd_link_o_target, $(OBJS))
+
+$(obj).depend:
+	echo wtf
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
diff --git a/arch/arm/cpu/armv7/tegra2/ap20.c b/arch/arm/cpu/tegra2-common/ap20.c
similarity index 100%
rename from arch/arm/cpu/armv7/tegra2/ap20.c
rename to arch/arm/cpu/tegra2-common/ap20.c
diff --git a/arch/arm/cpu/armv7/tegra2/ap20.h b/arch/arm/cpu/tegra2-common/ap20.h
similarity index 100%
rename from arch/arm/cpu/armv7/tegra2/ap20.h
rename to arch/arm/cpu/tegra2-common/ap20.h
diff --git a/arch/arm/cpu/armv7/tegra2/board.c b/arch/arm/cpu/tegra2-common/board.c
similarity index 100%
rename from arch/arm/cpu/armv7/tegra2/board.c
rename to arch/arm/cpu/tegra2-common/board.c
diff --git a/arch/arm/cpu/armv7/tegra2/clock.c b/arch/arm/cpu/tegra2-common/clock.c
similarity index 100%
rename from arch/arm/cpu/armv7/tegra2/clock.c
rename to arch/arm/cpu/tegra2-common/clock.c
diff --git a/arch/arm/cpu/armv7/tegra2/funcmux.c b/arch/arm/cpu/tegra2-common/funcmux.c
similarity index 100%
rename from arch/arm/cpu/armv7/tegra2/funcmux.c
rename to arch/arm/cpu/tegra2-common/funcmux.c
diff --git a/arch/arm/cpu/armv7/tegra2/lowlevel_init.S b/arch/arm/cpu/tegra2-common/lowlevel_init.S
similarity index 100%
rename from arch/arm/cpu/armv7/tegra2/lowlevel_init.S
rename to arch/arm/cpu/tegra2-common/lowlevel_init.S
diff --git a/arch/arm/cpu/armv7/tegra2/pinmux.c b/arch/arm/cpu/tegra2-common/pinmux.c
similarity index 100%
rename from arch/arm/cpu/armv7/tegra2/pinmux.c
rename to arch/arm/cpu/tegra2-common/pinmux.c
diff --git a/arch/arm/cpu/armv7/tegra2/sys_info.c b/arch/arm/cpu/tegra2-common/sys_info.c
similarity index 100%
rename from arch/arm/cpu/armv7/tegra2/sys_info.c
rename to arch/arm/cpu/tegra2-common/sys_info.c
diff --git a/arch/arm/cpu/armv7/tegra2/timer.c b/arch/arm/cpu/tegra2-common/timer.c
similarity index 100%
rename from arch/arm/cpu/armv7/tegra2/timer.c
rename to arch/arm/cpu/tegra2-common/timer.c
diff --git a/spl/Makefile b/spl/Makefile
index ea7d475..6d3241f 100644
--- a/spl/Makefile
+++ b/spl/Makefile
@@ -62,6 +62,10 @@  ifneq ($(CONFIG_AM33XX)$(CONFIG_OMAP34XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX),)
 LIBS-y += $(CPUDIR)/omap-common/libomap-common.o
 endif
 
+ifneq ($(CONFIG_TEGRA2),)
+LIBS-y += arch/$(ARCH)/cpu/$(SOC)-common/lib$(SOC)-common.o
+endif
+
 START := $(addprefix $(SPLTREE)/,$(START))
 LIBS := $(addprefix $(SPLTREE)/,$(sort $(LIBS-y)))