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]
>
>>
>> There is currently only one prototype board that is based on this design
>> and this patch also introduces it. The board is called kmlion1.
>>
>> Signed-off-by: Stefan Bigler <stefan.big...@keymile.com>
>> Signed-off-by: Valentin Longchamp <valentin.longch...@keymile.com>
>> ---
>> MAINTAINERS | 1 +
>> board/keymile/common/common.c | 4 +-
>> board/keymile/kmp204x/Makefile | 48 ++++
>> board/keymile/kmp204x/ddr.c | 84 +++++++
>> board/keymile/kmp204x/eth.c | 87 +++++++
>> board/keymile/kmp204x/kmp204x.c | 307 ++++++++++++++++++++++
>> board/keymile/kmp204x/kmp204x.h | 31 +++
>> board/keymile/kmp204x/law.c | 51 ++++
>> board/keymile/kmp204x/pbi.cfg | 51 ++++
>> board/keymile/kmp204x/pci.c | 49 ++++
>> board/keymile/kmp204x/rcw_kmp204x.cfg | 11 +
>> board/keymile/kmp204x/tlb.c | 122 +++++++++
>> boards.cfg | 1 +
>> include/configs/km/kmp204x-common.h | 462
>> ++++++++++++++++++++++++++++++++++
>> include/configs/kmp204x.h | 84 +++++++
>> 15 files changed, 1392 insertions(+), 1 deletion(-)
>> create mode 100644 board/keymile/kmp204x/Makefile
>> create mode 100644 board/keymile/kmp204x/ddr.c
>> create mode 100644 board/keymile/kmp204x/eth.c
>> create mode 100644 board/keymile/kmp204x/kmp204x.c
>> create mode 100644 board/keymile/kmp204x/kmp204x.h
>> create mode 100644 board/keymile/kmp204x/law.c
>> create mode 100644 board/keymile/kmp204x/pbi.cfg
>> create mode 100644 board/keymile/kmp204x/pci.c
>> create mode 100644 board/keymile/kmp204x/rcw_kmp204x.cfg
>> create mode 100644 board/keymile/kmp204x/tlb.c
>> create mode 100644 include/configs/km/kmp204x-common.h
>> create mode 100644 include/configs/kmp204x.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 081cf96..fff77f0 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -778,6 +778,7 @@ Valentin Longchamp <valentin.longch...@keymile.com>
>> mgcoge3un ARM926EJS (Kirkwood SoC)
>> kmcoge5un ARM926EJS (Kirkwood SoC)
>> portl2 ARM926EJS (Kirkwood SoC)
>> + kmcoge4 MPC85xx (P2041 SoC)
>>
>> Nishanth Menon <n...@ti.com>
>>
>> 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.
>
>> static void i2c_write_start_seq(void)
>> {
>> set_sda(1);
>> @@ -183,6 +183,7 @@ void i2c_init_board(void)
>> }
>> #endif
>>
>> +#ifndef CONFIG_KMP204X
>> int board_eth_init(bd_t *bis)
>
> Likewise.
Same as above.
>
>> +/* TODO: implement the I2C functions */
>> +void i2c_write_start_seq(void){
>> + return;
>> +}
>
> Opening brace goes on its own line for function definitions.
OK
>
>> + if (en)
>> + prst &= ~(1<<bit);
>> + else
>> + prst |= (1<<bit);
>
> Spaces around binary operators such as <<
OK
>
>> +#ifdef CONFIG_SYS_NAND_BASE_PHYS
>> + SET_LAW(CONFIG_SYS_NAND_BASE_PHYS, LAW_SIZE_1M, LAW_TRGT_IF_LBC),
>> +#endif
>
> The NAND window is 8K. There's no reason for the LAW to be more than
> the minimum 32K.
OK, I agree and will test this.
>
>> diff --git a/board/keymile/kmp204x/tlb.c b/board/keymile/kmp204x/tlb.c
>> new file mode 100644
>> index 0000000..5bbefd5
>> --- /dev/null
>> +++ b/board/keymile/kmp204x/tlb.c
>> @@ -0,0 +1,122 @@
>> +/*
>> + * (C) Copyright 2013 Keymile AG
>> + * Valentin Longchamp <valentin.longch...@keymile.com>
>> + *
>> + * 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., 51 Franklin Street, Fifth Floor, Boston,
>> + * MA 02110-1301 USA
>> + */
>> +
>> +#include <common.h>
>> +#include <asm/mmu.h>
>> +
>> +struct fsl_e_tlb_entry tlb_table[] = {
>> + /* TLB 0 - for temp stack in cache */
>> + SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR,
>> + CONFIG_SYS_INIT_RAM_ADDR_PHYS,
>> + MAS3_SW|MAS3_SR, 0,
>> + 0, 0, BOOKE_PAGESZ_4K, 0),
>> + SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 4 * 1024,
>> + CONFIG_SYS_INIT_RAM_ADDR_PHYS + 4 * 1024,
>> + MAS3_SW|MAS3_SR, 0,
>> + 0, 0, BOOKE_PAGESZ_4K, 0),
>> + SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 8 * 1024,
>> + CONFIG_SYS_INIT_RAM_ADDR_PHYS + 8 * 1024,
>> + MAS3_SW|MAS3_SR, 0,
>> + 0, 0, BOOKE_PAGESZ_4K, 0),
>> + SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 12 * 1024,
>> + CONFIG_SYS_INIT_RAM_ADDR_PHYS + 12 * 1024,
>> + MAS3_SW|MAS3_SR, 0,
>> + 0, 0, BOOKE_PAGESZ_4K, 0),
>> + /* 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.
>> +#ifdef CONFIG_SYS_NAND_BASE
>> + /*
>> + * *I*G - NAND
>> + * entry 14 and 15 has been used hard coded, they will be disabled
>> + * 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),
>> +#endif
>
> Again, NAND is an 8K window. Don't set MAS3_SX.
OK
>
>> +};
>> +
>> +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.
>
>> +#define CONFIG_SYS_NAND_BASE_LIST {CONFIG_SYS_NAND_BASE}
>> +#define CONFIG_SYS_MAX_NAND_DEVICE 1
>> +#define CONFIG_MTD_NAND_VERIFY_WRITE
>> +#define CONFIG_CMD_NAND
>> +#define CONFIG_SYS_NAND_BLOCK_SIZE (128 * 1024)
>> +
>> +#define CONFIG_NAND_ECC_BCH
>> +#define CONFIG_BCH
>>
>> +/* NAND flash config */
>> +#define CONFIG_SYS_NAND_BR_PRELIM (BR_PHYS_ADDR(CONFIG_SYS_NAND_BASE_PHYS)
>> \
>> + | BR_PS_8 /* Port Size = 8 bit */ \
>> + | BR_MS_FCM /* MSEL = FCM */ \
>> + | BR_V) /* valid */
>> +
>> +#define CONFIG_SYS_NAND_OR_PRELIM (OR_AM_256KB /* length 256K */
>> \
>> + | OR_FCM_BCTLD /* LBCTL not ass */ \
>> + | OR_FCM_SCY_1 /* 1 clk wait cycle */ \
>> + | OR_FCM_RST /* 1 clk read setup */ \
>> + | OR_FCM_PGS /* Large Page 128k */ \
>> + | OR_FCM_CST) /* 0.25 command setup */
>
> What does "large page 128k" mean? 128k is the block size, not the page
> size.
>
OK
Valentin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
