[U-Boot] [U-BOOT] [PATCH 2/6] mv: seperate kirkwood and mmp from common setting

Wed Jan 5 16:29:05 CET 2011

On Wed, Jan 5, 2011 at 5:40 PM, Prafulla Wadaskar <prafulla at marvell.com> wrote:
>
>
>> -----Original Message-----
>> From: Lei Wen [mailto:leiwen at marvell.com]
>> Sent: Tuesday, January 04, 2011 8:21 PM
>> To: u-boot at lists.denx.de; Prafulla Wadaskar; Yu Tang; Ashish Karkare;
>> Prabhanjan Sarnaik; adrian.wenl at gmail.com
>> Subject: [U-BOOT] [PATCH 2/6] mv: seperate kirkwood and mmp from common
>> setting
>>
>> Since there are lots of difference between kirkwood and mmp series,
>
> What is mmp?
mmp is what we call it in Linux kernel, as you already use armada, we
also could use armada series here.

>
>> it is better to seperate them but still keep the most common file
>> shared by all marvell platform in the mv-common configure file.
>>
>> This patch move the kirkwood only driver definitoin in mv-common to
>> the kirkwood.h and it is tested with compilation for armada100 and
>
> I think you need to specify config.h instead of Kirkwood.h??

Yes, it should be config.h here.
>
>> guruplug.
>>
>> Signed-off-by: Lei Wen <leiwen at marvell.com>
>> ---
>>  arch/arm/include/asm/arch-armada100/config.h |   50 ++++++++
>>  arch/arm/include/asm/arch-kirkwood/config.h  |  170
>> ++++++++++++++++++++++++++
>>  include/configs/aspenite.h                   |    1 +
>>  include/configs/mv-common.h                  |  154 +------------------
>> -----
>>  4 files changed, 223 insertions(+), 152 deletions(-)
>>  create mode 100644 arch/arm/include/asm/arch-armada100/config.h
>>  create mode 100644 arch/arm/include/asm/arch-kirkwood/config.h
>>
>> diff --git a/arch/arm/include/asm/arch-armada100/config.h
>> b/arch/arm/include/asm/arch-armada100/config.h
>> new file mode 100644
>> index 0000000..9c18b1c
>> --- /dev/null
>> +++ b/arch/arm/include/asm/arch-armada100/config.h
>> @@ -0,0 +1,50 @@
>> +/*
>> + * (C) Copyright 2010
>> + * Marvell Semiconductor <www.marvell.com>
>> + * Written-by: Lei Wen <leiwen at marvell.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
>> + */
>> +
>> +/*
>> + * This file should be included in board config header file.
>
> Should be included to mv-common.h

Apply this for the next patch submit...

>
>> + *
>> + * It supports common definations for MMP platform
>
> Spell check s/definition/definition/g
> What is MMP? Replace it with ARMADA100

Apply this for the next patch submit...

>
>> + */
>> +
>> +#ifndef _ARMADA100_H
>> +#define _ARMADA100_H
>
> Should be _ARMD1_CONFIG_H

Ok..

>
>> +
>> +#define CONFIG_ARM926EJS     1       /* Basic Architecture */
>> +
>> +#define CONFIG_SYS_TCLK              (14745600)      /* NS16550 clk config */
>> +#define CONFIG_SYS_HZ_CLOCK  (3250000)       /* Timer Freq. 3.25MHZ */
>> +#define CONFIG_MARVELL_MFP                   /* Enable mvmfp driver */
>> +#define MV_MFPR_BASE         ARMD1_MFPR_BASE
>> +#define MV_UART_CONSOLE_BASE ARMD1_UART1_BASE
>> +#define CONFIG_SYS_NS16550_IER       (1 << 6)        /* Bit 6 in UART_IER register
>> +                                             represents UART Unit Enable */
>> +/*
>> + * There is no internal RAM in ARMADA100, using DRAM
>> + * TBD: dcache to be used for this
>> + */
>> +#define CONFIG_SYS_INIT_SP_ADDR              (CONFIG_SYS_TEXT_BASE -
>> 0x00200000)
>> +#define CONFIG_NR_DRAM_BANKS_MAX     2
>> +
>> +#endif /* _ARMADA100_H */
>> diff --git a/arch/arm/include/asm/arch-kirkwood/config.h
>> b/arch/arm/include/asm/arch-kirkwood/config.h
>> new file mode 100644
>> index 0000000..0a15e6b
>> --- /dev/null
>> +++ b/arch/arm/include/asm/arch-kirkwood/config.h
>> @@ -0,0 +1,170 @@
>> +/*
>> + * (C) Copyright 2010
>> + * Marvell Semiconductor <www.marvell.com>
>> + * Written-by: Lei Wen <leiwen at marvell.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
>> + */
>> +
>> +/*
>> + * This file should be included in board config header file.
>
> Ditto
>
>> + *
>> + * It supports common definations for Kirkwood platform
>
> Definitions
>
>> + * TBD: support for Orion5X platforms
>
>
> This line stands invalid here.
>
>> + */
>> +
>> +#ifndef _MV_KIRKWOOD_H
>> +#define _MV_KIRKWOOD_H
>
> Should be _KW_CONFIG_H
Ok..
>
>> +
>> +#if defined (CONFIG_KW88F6281)
>> +#include <asm/arch/kw88f6281.h>
>> +#elif defined (CONFIG_KW88F6192)
>> +#include <asm/arch/kw88f6192.h>
>> +#else
>> +#error "SOC Name not defined"
>> +#endif /* CONFIG_KW88F6281 */
>> +
>> +#define CONFIG_ARM926EJS     1       /* Basic Architecture */
>> +
>> +#define CONFIG_MD5   /* get_random_hex on krikwood needs MD5 support */
>> +#define CONFIG_KIRKWOOD_EGIGA_INIT   /* Enable GbePort0/1 for kernel */
>> +#define CONFIG_KIRKWOOD_RGMII_PAD_1V8        /* Set RGMII Pad voltage to 1.8V
>> */
>> +#define CONFIG_KIRKWOOD_PCIE_INIT       /* Enable PCIE Port0 for kernel
>> */
>> +
>> +/*
>> + * By default kwbimage.cfg from board specific folder is used
>> + * If for some board, different configuration file need to be used,
>> + * CONFIG_SYS_KWD_CONFIG should be defined in board specific header
>> file
>> + */
>> +#ifndef CONFIG_SYS_KWD_CONFIG
>> +#define      CONFIG_SYS_KWD_CONFIG   $(SRCTREE)/$(CONFIG_BOARDDIR)/kwbimage.cfg
>> +#endif /* CONFIG_SYS_KWD_CONFIG */
>> +
>> +/* Kirkwood has 2k of Security SRAM, use it for SP */
>> +#define CONFIG_SYS_INIT_SP_ADDR              0xC8012000
>> +#define CONFIG_NR_DRAM_BANKS_MAX     2
>> +
>> +#define CONFIG_I2C_MVTWSI_BASE       KW_TWSI_BASE
>> +#define MV_UART_CONSOLE_BASE KW_UART0_BASE
>> +#define MV_SATA_BASE         KW_SATA_BASE
>> +#define MV_SATA_PORT0_OFFSET KW_SATA_PORT0_OFFSET
>> +#define MV_SATA_PORT1_OFFSET KW_SATA_PORT1_OFFSET
>> +
>> +/*
>> + * NAND configuration
>> + */
>> +#ifdef CONFIG_CMD_NAND
>> +#define CONFIG_NAND_KIRKWOOD
>> +#define CONFIG_SYS_MAX_NAND_DEVICE   1
>> +#define NAND_MAX_CHIPS                       1
>> +#define CONFIG_SYS_NAND_BASE         0xD8000000      /* MV_DEFADR_NANDF */
>> +#define NAND_ALLOW_ERASE_ALL         1
>> +#define CONFIG_SYS_64BIT_VSPRINTF    /* needed for nand_util.c */
>> +#endif
>> +
>
> I think split below makes more sense- overall objective is to avoid code duplication here.
>
> For mv-common.h
> +/*
> + * NAND configuration
> + */
> +#ifdef CONFIG_CMD_NAND
> +#define CONFIG_SYS_MAX_NAND_DEVICE     1
> +#define NAND_MAX_CHIPS                 1
> +#define CONFIG_SYS_64BIT_VSPRINTF      /* needed for nand_util.c */
> +#endif
>
> For arch-kirkwood/config.h
> +/*
> + * NAND configuration
> + */
> +#ifdef CONFIG_CMD_NAND
> +#define CONFIG_NAND_KIRKWOOD
> +#define CONFIG_SYS_NAND_BASE           0xD8000000      /* MV_DEFADR_NANDF */
> +#define NAND_ALLOW_ERASE_ALL           1
> +#endif
>
>> +/*
>> + * SPI Flash configuration
>> + */
>> +#ifdef CONFIG_CMD_SF
>> +#define CONFIG_SPI_FLASH             1
>> +#define CONFIG_HARD_SPI                      1
>> +#define CONFIG_KIRKWOOD_SPI          1
>> +#define CONFIG_SPI_FLASH_MACRONIX    1
>> +#define CONFIG_ENV_SPI_BUS           0
>> +#define CONFIG_ENV_SPI_CS            0
>> +#define CONFIG_ENV_SPI_MAX_HZ                50000000        /*50Mhz */
>> +#endif
>> +
>
> Same applies for rest of the definitions.
> mv-common.h should represent common definitions for Kirkwood,armada100, and future SoCs based boards so that
>
Yes, agree...
Since current armada100 havn't added nand and spi support, how about
move those nand and spi definition to kirkwood and
then after armada100 add the nand, we reconsider to move back the common back?

Especially for those code below, I really believe this should not be
put into the common code...
#ifndef CONFIG_ARMADA100       /* will be removed latter */-#define
CONFIG_CMD_EXT2
#define CONFIG_CMD_JFFS2
#define CONFIG_CMD_FAT
#define CONFIG_CMD_UBI
#define CONFIG_CMD_UBIFS
#define CONFIG_RBTREE
#define CONFIG_MTD_DEVICE               /* needed for mtdparts commands */
#define CONFIG_MTD_PARTITIONS
#define CONFIG_CMD_MTDPARTS
#define CONFIG_LZO
#endif

Best regards,
Lei