[U-Boot] [PATCH v1 8/8] mpc85xx: introduce the kmp204x reference design support

Scott Wood
2013-08-20T00:48:09Z

On Mon, 2013-08-19 at 17:50 +0200, Valentin Longchamp wrote:
> On 08/13/2013 11:38 PM, Scott Wood wrote:
> > On Fri, 2013-07-26 at 12:02 +0200, Valentin Longchamp wrote:
> >> This patch introduces the support for Keymile's kmp204x reference
> >> design. This design is based on Freescale's P2040/P2041 SoC.
> >>
> >> The peripherals used by this design are:
> >> - DDR3 RAM with SPD support
> >> - SPI NOR Flash as boot medium
> >> - NAND Flash
> >> - 2 PCIe busses (hosts 1 and 3)
> >> - 3 FMAN Ethernet devices (FMAN1 DTSEC1/2/5)
> >> - 3 Local Bus windows, with one dedicated to the QRIO reset/power mgmt
> >> FPGA
> >> - 2 HW I2C busses
> >> - last but not least, the mandatory serial port
> >>
> >> The board/keymile/kmp204x code is mostly based on Freescale's P2041rdb
> >> support and was changed according to our design (that means essentially
> >> removing what is not present on the designs and a few adaptations).
> >
> > A lot of the copied files have had Freescale copyrights removed... Also
> > please try to factor shared code out rather than duplicate, where
> > practical.
>
> Well, I had tried to come a first proposal that factored shared code but I was
> advised to copy the code: [1]. For the copyrights I will fix it.
>
> [1]
>

Did you mean to cite something here?

> >> diff --git a/board/keymile/common/common.c b/board/keymile/common/common.c
> >> index ef93ed3..ca833db 100644
> >> --- a/board/keymile/common/common.c
> >> +++ b/board/keymile/common/common.c
> >> @@ -94,7 +94,7 @@ int set_km_env(void)
> >> }
> >>
> >> #if defined(CONFIG_SYS_I2C_INIT_BOARD)
> >> -#if !defined(CONFIG_MPC83xx)
> >> +#if !defined(CONFIG_MPC83xx) && !defined(CONFIG_PPC_P2041)
> >
> > Perhaps you should check for when you do want to run this code, rather
> > than when you don't.
>
> No, there are more cases where this is needed than not needed.

Doesn't matter; it's still less readable and a maintenance pain.
Introduce a symbol that describes the situation where you want to run
this code, so you don't have to repeat the list each time.

> >> + /* TLB 1 */
> >> + /* *I*** - Covers boot page */
> >> + /* *I*G - L3SRAM. When L3 is used as 1M SRAM, the address of the
> >> + * 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),
> >
> > What does that "covers boot page" comment refer to?
> >
> > Why is L3SRAM I+G?
> >
>
> I have taken this from the corenet SYS_RAMBOOT boot scenario since it's also the
> way our board boots.

York, can you answer this?

I suspect the "covers boot page" comment is left over from before the
recent spin table changes.

> >> +int num_tlb_entries = ARRAY_SIZE(tlb_table);
> >> diff --git a/boards.cfg b/boards.cfg
> >> index 6a368de..b818f1e 100644
> >> --- a/boards.cfg
> >> +++ b/boards.cfg
> >> @@ -744,6 +744,7 @@ tuge1 powerpc mpc83xx km83xx keymile
> >> tuxx1 powerpc mpc83xx km83xx keymile - tuxx1:TUXX1
> >> kmopti2 powerpc mpc83xx km83xx keymile - tuxx1:KMOPTI2
> >> kmsupx5 powerpc mpc83xx km83xx keymile - tuxx1:KMSUPX5
> >> +kmlion1 powerpc mpc85xx kmp204x keymile - kmp204x:KMLION1
> >
> > Is it kmlion1 or km204x? The target name should match what the board
> > prints out on boot.
>
> It is the kmlion1 board that is based on the kmp204x "generic" design.

Then U-Boot should print out "kmlion1" as its board name.

-Scott