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