Patchwork [U-Boot,RFC] mpc85xx: move generic corenet to cpu/mpc85xxx

login
register
mail settings
Submitter Valentin Longchamp
Date May 3, 2013, 2:02 p.m.
Message ID <1367589737-5904-1-git-send-email-valentin.longchamp@keymile.com>
Download mbox | patch
Permalink /patch/241321/
State Rejected
Delegated to: Andy Fleming
Headers show

Comments

Valentin Longchamp - May 3, 2013, 2:02 p.m.
This allows to use this code on non Freescale QorIQ boards. If I am
right, there are currently only Freescale boards with a QorIQ Soc that
is supported by u-boot, that's why this code was located there.

Some other code parts that currently are located in the board/freescale
directory could be moved but this is a first patch to discuss trigger
the discussion.

It was tested on P2041rdb_SPIFLASH.

Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
---
 arch/powerpc/cpu/mpc85xx/Makefile                    | 10 ++++++++--
 .../powerpc/cpu/mpc85xx}/p_corenet/Makefile          |  0
 .../powerpc/cpu/mpc85xx}/p_corenet/law.c             |  0
 .../powerpc/cpu/mpc85xx}/p_corenet/pci.c             |  0
 .../powerpc/cpu/mpc85xx}/p_corenet/tlb.c             | 20 ++++++++++----------
 board/freescale/common/Makefile                      | 15 ++-------------
 6 files changed, 20 insertions(+), 25 deletions(-)
 rename {board/freescale/common => arch/powerpc/cpu/mpc85xx}/p_corenet/Makefile (100%)
 rename {board/freescale/common => arch/powerpc/cpu/mpc85xx}/p_corenet/law.c (100%)
 rename {board/freescale/common => arch/powerpc/cpu/mpc85xx}/p_corenet/pci.c (100%)
 rename {board/freescale/common => arch/powerpc/cpu/mpc85xx}/p_corenet/tlb.c (92%)
Andy Fleming - May 3, 2013, 10:55 p.m.
Valentin, sorry for the dup (forgot to copy the mailing list


On Fri, May 3, 2013 at 9:02 AM, Valentin Longchamp <
valentin.longchamp@keymile.com> wrote:

> This allows to use this code on non Freescale QorIQ boards. If I am
> right, there are currently only Freescale boards with a QorIQ Soc that
> is supported by u-boot, that's why this code was located there.
>
>
Well, the problem is that it is not necessarily the case that a
non-Freescale QorIQ board will want the same LAW, TLB, and pci setup as the
Freescale boards. This code is shared on our boards because our boards
share a common infrastructure. We use the same FPGA for board management
across these boards, and the SoC architectures are similar enough that this
code can be shared. It is unlikely that non-Freescale boards need:

#ifdef PIXIS_BASE
        SET_TLB_ENTRY(0, PIXIS_BASE, PIXIS_BASE_PHYS,
                      MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
                      0, 0, BOOKE_PAGESZ_4K, 0),
#endif

There are many decisions that need to be made when making a board port
regarding the allocation of LAWs and TLBs, and those decisions are highly
influenced by the design of the board. This is an odd case where, even if
your board is nearly identical to ours, it would be better to copy the
code. We may, at some point release another board which is close enough
that we can share the code, at which point we would (carefully) modify
these files to accommodate the new board. If there are unknown other boards
consuming these settings, it's far more complicated for both parties.

TLB and LAW settings are highly specific to the board. While some of the
information is similar across boards, most of the information in those
files is how to lay out a memory map for the devices on the chip and the
board. You don't want to blindly copy ours -- you want to tailor yours to
the board you have. This is an odd case where, even if you *did* want to
copy ours, you should actually make a separate copy. We made this a shared
file because the QorIQ boards share a similar architecture. If you look
through board/freescale/ for tlb.c, you'll see that every other board in
there has its own, despite the fact that many of the SoCs are quite similar.
Valentin Longchamp - May 6, 2013, 1:58 p.m.
Hi Andy,

On 05/04/2013 12:55 AM, Andy Fleming wrote:
> On Fri, May 3, 2013 at 9:02 AM, Valentin Longchamp
> <valentin.longchamp@keymile.com <mailto:valentin.longchamp@keymile.com>> wrote:
> 
>     This allows to use this code on non Freescale QorIQ boards. If I am
>     right, there are currently only Freescale boards with a QorIQ Soc that
>     is supported by u-boot, that's why this code was located there.
> 
> 
> Well, the problem is that it is not necessarily the case that a non-Freescale
> QorIQ board will want the same LAW, TLB, and pci setup as the Freescale boards.
> This code is shared on our boards because our boards share a common
> infrastructure. We use the same FPGA for board management across these boards,
> and the SoC architectures are similar enough that this code can be shared. It is
> unlikely that non-Freescale boards need:
> 
> #ifdef PIXIS_BASE
>         SET_TLB_ENTRY(0, PIXIS_BASE, PIXIS_BASE_PHYS,
>                       MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
>                       0, 0, BOOKE_PAGESZ_4K, 0),
> #endif 

My main intention with this patch was to avoid as much as possible code
duplication. This is our first QorIQ board and we are quite conservative with
many things, that's why mcuh of the initialization would still be valid for us.

You are right about the management FPGA, we have our own. My idea was simply to
not #define PIXIS_BASE in our config file.

> 
> There are many decisions that need to be made when making a board port regarding
> the allocation of LAWs and TLBs, and those decisions are highly influenced by
> the design of the board. This is an odd case where, even if your board is nearly
> identical to ours, it would be better to copy the code. We may, at some point
> release another board which is close enough that we can share the code, at which
> point we would (carefully) modify these files to accommodate the new board. If
> there are unknown other boards consuming these settings, it's far more
> complicated for both parties.

Ok thank you for the precision. As I said, since our current design is nearly
identical to the Freescale dev boards, I wanted to avoid copying the code. Since
your recommendation is actually to copy it I will do this.

> 
> TLB and LAW settings are highly specific to the board. While some of the
> information is similar across boards, most of the information in those files is
> how to lay out a memory map for the devices on the chip and the board. You don't
> want to blindly copy ours -- you want to tailor yours to the board you have.
> This is an odd case where, even if you *did* want to copy ours, you should
> actually make a separate copy. We made this a shared file because the QorIQ
> boards share a similar architecture. If you look through board/freescale/ for
> tlb.c, you'll see that every other board in there has its own, despite the fact
> that many of the SoCs are quite similar.

OK. Since a big part of the layout is related to devices on the chip and not on
the board, that's why I thought it made sense to share this code. It will maybe
come at a later point.

Valentin

Patch

diff --git a/arch/powerpc/cpu/mpc85xx/Makefile b/arch/powerpc/cpu/mpc85xx/Makefile
index 6776c85..0b76ff5 100644
--- a/arch/powerpc/cpu/mpc85xx/Makefile
+++ b/arch/powerpc/cpu/mpc85xx/Makefile
@@ -141,6 +141,8 @@  COBJS-$(CONFIG_PPC_B4420) += b4860_serdes.o
 COBJS-$(CONFIG_PPC_B4860) += b4860_serdes.o
 COBJS-$(CONFIG_BSC9132) += bsc9132_serdes.o
 
+SUBLIB-$(CONFIG_FSL_CORENET) += p_corenet/libp_corenet.o
+
 COBJS-y	+= cpu.o
 COBJS-y	+= cpu_init.o
 COBJS-y	+= cpu_init_early.o
@@ -158,12 +160,16 @@  COBJS	= $(COBJS-y)
 
 SRCS	:= $(START:.o=.S) $(SOBJS:.o=.S) $(COBJS:.o=.c)
 OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS))
+SUBLIB	:= $(addprefix $(obj),$(SUBLIB-y))
 START	:= $(addprefix $(obj),$(START))
 
 all:	$(obj).depend $(START) $(LIB)
 
-$(LIB):	$(OBJS)
-	$(call cmd_link_o_target, $(OBJS))
+$(LIB):	$(obj).depend $(OBJS) $(SUBLIB)
+	$(call cmd_link_o_target, $(OBJS) $(SUBLIB))
+
+$(SUBLIB): $(obj).depend
+	$(MAKE) -C $(dir $(subst $(obj),,$@))
 
 #########################################################################
 
diff --git a/board/freescale/common/p_corenet/Makefile b/arch/powerpc/cpu/mpc85xx/p_corenet/Makefile
similarity index 100%
rename from board/freescale/common/p_corenet/Makefile
rename to arch/powerpc/cpu/mpc85xx/p_corenet/Makefile
diff --git a/board/freescale/common/p_corenet/law.c b/arch/powerpc/cpu/mpc85xx/p_corenet/law.c
similarity index 100%
rename from board/freescale/common/p_corenet/law.c
rename to arch/powerpc/cpu/mpc85xx/p_corenet/law.c
diff --git a/board/freescale/common/p_corenet/pci.c b/arch/powerpc/cpu/mpc85xx/p_corenet/pci.c
similarity index 100%
rename from board/freescale/common/p_corenet/pci.c
rename to arch/powerpc/cpu/mpc85xx/p_corenet/pci.c
diff --git a/board/freescale/common/p_corenet/tlb.c b/arch/powerpc/cpu/mpc85xx/p_corenet/tlb.c
similarity index 92%
rename from board/freescale/common/p_corenet/tlb.c
rename to arch/powerpc/cpu/mpc85xx/p_corenet/tlb.c
index e5cf208..e6204e1 100644
--- a/board/freescale/common/p_corenet/tlb.c
+++ b/arch/powerpc/cpu/mpc85xx/p_corenet/tlb.c
@@ -64,17 +64,17 @@  struct fsl_e_tlb_entry tlb_table[] = {
 	 * SRAM is at 0xfff00000, it covered the 0xfffff000.
 	 */
 	SET_TLB_ENTRY(1, CONFIG_SYS_INIT_L3_ADDR, CONFIG_SYS_INIT_L3_ADDR,
-			MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
-			0, 0, BOOKE_PAGESZ_1M, 1),
+		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+		      0, 0, BOOKE_PAGESZ_1M, 1),
 #elif defined(CONFIG_SRIO_PCIE_BOOT_SLAVE)
 	/*
 	 * SRIO_PCIE_BOOT-SLAVE. When slave boot, the address of the
 	 * space is at 0xfff00000, it covered the 0xfffff000.
 	 */
 	SET_TLB_ENTRY(1, CONFIG_SYS_SRIO_PCIE_BOOT_SLAVE_ADDR,
-			CONFIG_SYS_SRIO_PCIE_BOOT_SLAVE_ADDR_PHYS,
-			MAS3_SX|MAS3_SW|MAS3_SR, MAS2_W|MAS2_G,
-			0, 0, BOOKE_PAGESZ_1M, 1),
+		      CONFIG_SYS_SRIO_PCIE_BOOT_SLAVE_ADDR_PHYS,
+		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_W|MAS2_G,
+		      0, 0, BOOKE_PAGESZ_1M, 1),
 #else
 	SET_TLB_ENTRY(1, 0xfffff000, 0xfffff000,
 		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
@@ -144,8 +144,8 @@  struct fsl_e_tlb_entry tlb_table[] = {
 	 * in cpu_init_f, so we use entry 16 for nand.
 	 */
 	SET_TLB_ENTRY(1, CONFIG_SYS_NAND_BASE, CONFIG_SYS_NAND_BASE_PHYS,
-			MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
-			0, 16, BOOKE_PAGESZ_1M, 1),
+		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+		      0, 16, BOOKE_PAGESZ_1M, 1),
 #endif
 #ifdef CONFIG_SRIO_PCIE_BOOT_SLAVE
 	/*
@@ -153,9 +153,9 @@  struct fsl_e_tlb_entry tlb_table[] = {
 	 * fetching ucode and ENV from master
 	 */
 	SET_TLB_ENTRY(1, CONFIG_SYS_SRIO_PCIE_BOOT_UCODE_ENV_ADDR,
-		CONFIG_SYS_SRIO_PCIE_BOOT_UCODE_ENV_ADDR_PHYS,
-		MAS3_SX|MAS3_SW|MAS3_SR, MAS2_G,
-		0, 17, BOOKE_PAGESZ_1M, 1),
+		      CONFIG_SYS_SRIO_PCIE_BOOT_UCODE_ENV_ADDR_PHYS,
+		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_G,
+		      0, 17, BOOKE_PAGESZ_1M, 1),
 #endif
 };
 
diff --git a/board/freescale/common/Makefile b/board/freescale/common/Makefile
index 75725b4..41c6a14 100644
--- a/board/freescale/common/Makefile
+++ b/board/freescale/common/Makefile
@@ -56,23 +56,12 @@  COBJS-$(CONFIG_P5020DS)		+= ics307_clk.o
 COBJS-$(CONFIG_P5040DS)		+= ics307_clk.o
 COBJS-$(CONFIG_VSC_CROSSBAR)    += vsc3316_3308.o
 
-# deal with common files for P-series corenet based devices
-SUBLIB-$(CONFIG_P2041RDB)	+= p_corenet/libp_corenet.o
-SUBLIB-$(CONFIG_P3041DS)	+= p_corenet/libp_corenet.o
-SUBLIB-$(CONFIG_P4080DS)	+= p_corenet/libp_corenet.o
-SUBLIB-$(CONFIG_P5020DS)	+= p_corenet/libp_corenet.o
-SUBLIB-$(CONFIG_P5040DS)	+= p_corenet/libp_corenet.o
-
 SRCS	:= $(SOBJS:.o=.S) $(COBJS-y:.o=.c)
 OBJS	:= $(addprefix $(obj),$(COBJS-y))
 SOBJS	:= $(addprefix $(obj),$(SOBJS))
-SUBLIB	:= $(addprefix $(obj),$(SUBLIB-y))
-
-$(LIB):	$(obj).depend $(OBJS) $(SUBLIB)
-	$(call cmd_link_o_target, $(OBJS) $(SUBLIB))
 
-$(SUBLIB): $(obj).depend
-	$(MAKE) -C $(dir $(subst $(obj),,$@))
+$(LIB):	$(obj).depend $(OBJS)
+	$(call cmd_link_o_target, $(OBJS))
 
 #########################################################################