[U-Boot] [PATCH v2 6/6] arm: mvf600: Add basic support for Vybrid MVF600TWR board

[Wang Huan-B18965]

Hi, Stefano,

> 
> On 14/05/2013 11:51, Alison Wang wrote:
> > MVF600TWR is a board based on Vybrid MVF600 SoC.
> >
> > This patch adds basic support for Vybrid MVF600TWR board.
> >
> > Signed-off-by: Alison Wang <b18965 at freescale.com>
> > Signed-off-by: Jason Jin <Jason.jin at freescale.com>
> > Signed-off-by: TsiChung Liew <tsicliew at gmail.com>
> > ---
> 
> Hi Alison,
> 
> > diff --git a/board/freescale/mvf600twr/imximage.cfg
> > b/board/freescale/mvf600twr/imximage.cfg
> > new file mode 100644
> > index 0000000..33ead0f
> > --- /dev/null
> > +++ b/board/freescale/mvf600twr/imximage.cfg
> > @@ -0,0 +1,35 @@
> > +/*
> > + * Copyright 2013 Freescale Semiconductor, Inc.
> > + *
> > + * 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
> > + *
> > + * Refer docs/README.imxmage for more details about how-to configure
> > + * and create imximage boot image
> > + *
> > + * The syntax is taken as close as possible with the kwbimage  */
> > +
> > +/* image version */
> > +IMAGE_VERSION 2
> > +
> > +/*
> > + * Boot Device : one of
> > + * spi, sd (the board has no nand neither onenand)  */
> > +BOOT_FROM	sd
> 
> Ok, verstanden (I hope). This SOC has plenty of internal IRAM. It still
> uses the imximage mechanism, but you decided to start from internal RAM
> instead of DDR and to set up the RAM controller in the board
> initialization function.
> 
> We recently discussed about the wrongness of BOOT_FROM command. It makes
> no sense, and it was replaced by BOOT_OFFSET. If the SOC uses the main
> common offset (0x400), you can put
> 
> 	BOOT_OFFSET	FLASH_OFFSET_STANDARD
> 
[Alison Wang] I will use BOOT_OFFSET. Thanks.
> 
> > +#include <netdev.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +#define UART_PAD_CTRL	(PAD_CTL_PUS_100K_UP | PAD_CTL_SPEED_MED |
> \
> > +			PAD_CTL_DSE_25ohm | PAD_CTL_OBE_IBE_ENABLE)
> > +
> > +#define ESDHC_PAD_CTRL	(PAD_CTL_PKE | PAD_CTL_PUE |
> PAD_CTL_PUS_100K_UP | \
> > +			PAD_CTL_SPEED_HIGH | PAD_CTL_DSE_20ohm | \
> > +			PAD_CTL_OBE_IBE_ENABLE)
> 
> Please see commit 7e2173cf82d0bc235b695460c56d46927febdf36 and adjust
> this SOC consequently. PUE requires that PKE is enabled, that means that
> setting PKE alone does nothing. Then it is better to define PAD_CTL_PUE
> so that PKE is enabled, so in your 2/6 (that I have too fast acked):
> 
> PAD_CTL_PUE		(1 << 2 |  PAD_CTL_PKE)
> 
> and drop PAD_CTL_PKE here. This must fixed globally.
[Alison Wang] Agree, I will change as above. Thanks.
> 
> > +#define ENET_PAD_CTRL	(PAD_CTL_PUS_47K_UP | PAD_CTL_SPEED_HIGH |
> \
> > +			PAD_CTL_DSE_50ohm | PAD_CTL_OBE_IBE_ENABLE)
> > +
> > +#define DDR_PAD_CTRL	PAD_CTL_DSE_25ohm
> > +
> > +#define PHY_DQ_TIMING		0x00002613
> > +#define PHY_DQS_TIMING		0x00002615
> > +#define PHY_CTRL		0x01210080
> > +#define PHY_MASTER_CTRL		0x0001012a
> > +#define PHY_SLAVE_CTRL		0x00012020
> > +
> > +iomux_v3_cfg_t const ddr_pads[] = {
> > +	MVF600_PAD_DDR_A15__DDR_A_15 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_A14__DDR_A_14 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_A13__DDR_A_13 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_A12__DDR_A_12 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_A11__DDR_A_11 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_A10__DDR_A_10 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_A9__DDR_A_9 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_A8__DDR_A_8 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_A7__DDR_A_7 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_A6__DDR_A_6 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_A5__DDR_A_5 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_A4__DDR_A_4 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_A3__DDR_A_3 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_A2__DDR_A_2 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_A1__DDR_A_1 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_BA2__DDR_BA_2 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_BA1__DDR_BA_1 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_BA0__DDR_BA_0 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_CAS__DDR_CAS_B | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_CKE__DDR_CKE_0 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_CLK__DDR_CLK_0 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_CS__DDR_CS_B_0 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_D15__DDR_D_15 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_D14__DDR_D_14 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_D13__DDR_D_13 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_D12__DDR_D_12 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_D11__DDR_D_11 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_D10__DDR_D_10 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_D9__DDR_D_9 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_D8__DDR_D_8 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_D7__DDR_D_7 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_D6__DDR_D_6 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_D5__DDR_D_5 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_D4__DDR_D_4 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_D3__DDR_D_3 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_D2__DDR_D_2 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_D1__DDR_D_1 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_D0__DDR_D_0 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_DQM1__DDR_DQM_1 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_DQM0__DDR_DQM_0 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_DQS1__DDR_DQS_1 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_DQS0__DDR_DQS_0 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_RAS__DDR_RAS_B | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_WE__DDR_WE_B | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_ODT1__DDR_ODT_0 | MUX_PAD_CTRL(DDR_PAD_CTRL),
> > +	MVF600_PAD_DDR_ODT0__DDR_ODT_1 | MUX_PAD_CTRL(DDR_PAD_CTRL), };
> > +
> 
> As this is the first board with this SOC, it can be ok. But is this setup
> valid only for this board ? Maybe the pads for RAM should be the same.
> Then this part should be factorize elsewhere. 
> 
> > +void setup_iomux_ddr(void)
> > +{
> > +	imx_iomux_v3_setup_multiple_pads(ddr_pads, ARRAY_SIZE(ddr_pads)); }
> > +
> > +void ddr_phy_init(void)
> > +{
> > +	struct ddrmr_regs *ddrmr = (struct ddrmr_regs *)DDR_BASE_ADDR;
> > +
> > +	__raw_writel(PHY_DQ_TIMING, &ddrmr->phy[0]);
> > +	__raw_writel(PHY_DQ_TIMING, &ddrmr->phy[16]);
> > +	__raw_writel(PHY_DQ_TIMING, &ddrmr->phy[32]);
> > +	__raw_writel(PHY_DQ_TIMING, &ddrmr->phy[48]);
> > +
> > +	__raw_writel(PHY_DQS_TIMING, &ddrmr->phy[1]);
> > +	__raw_writel(PHY_DQS_TIMING, &ddrmr->phy[17]);
> > +	__raw_writel(PHY_DQS_TIMING, &ddrmr->phy[33]);
> > +	__raw_writel(PHY_DQS_TIMING, &ddrmr->phy[49]);
> > +
> > +	__raw_writel(PHY_CTRL, &ddrmr->phy[2]);
> > +	__raw_writel(PHY_CTRL, &ddrmr->phy[18]);
> > +	__raw_writel(PHY_CTRL, &ddrmr->phy[34]);
> > +	__raw_writel(PHY_CTRL, &ddrmr->phy[50]);
> > +
> > +	__raw_writel(PHY_MASTER_CTRL, &ddrmr->phy[3]);
> > +	__raw_writel(PHY_MASTER_CTRL, &ddrmr->phy[19]);
> > +	__raw_writel(PHY_MASTER_CTRL, &ddrmr->phy[35]);
> > +	__raw_writel(PHY_MASTER_CTRL, &ddrmr->phy[51]);
> > +
> > +	__raw_writel(PHY_SLAVE_CTRL, &ddrmr->phy[4]);
> > +	__raw_writel(PHY_SLAVE_CTRL, &ddrmr->phy[20]);
> > +	__raw_writel(PHY_SLAVE_CTRL, &ddrmr->phy[36]);
> > +	__raw_writel(PHY_SLAVE_CTRL, &ddrmr->phy[52]);
> > +
> 
> Without reference manual, it is difficult to judge. But it is surely
> difficult to read. What does hide under the magic index of the ddrmr
> stucture ?
[Alison Wang] In the reference manual, the registers are named as phy00, phy01, phy02.... cr00, cr01, cr02....
I think there may be some confusion if I rename the registers.
> 
> 
> > +	__raw_writel(0x00001105, &ddrmr->phy[50]); }
> 
> And can you add useful define instead of raw number ?
[Alison Wang] Yes, I will add useful define.
> 
> > +
> > +void ddr_ctrl_init(void)
> > +{
> > +	struct ddrmr_regs *ddrmr = (struct ddrmr_regs *)DDR_BASE_ADDR;
> > +
> > +	__raw_writel(0x00000600, &ddrmr->cr[0]);	/* LPDDR2 or DDR3 */
> > +	__raw_writel(0x00000020, &ddrmr->cr[2]);	/* TINIT */
> > +	__raw_writel(0x0000007c, &ddrmr->cr[10]);
> > +
> > +	__raw_writel(0x00013880, &ddrmr->cr[11]);
> > +	__raw_writel(0x0000050c, &ddrmr->cr[12]);
> > +	__raw_writel(0x15040404, &ddrmr->cr[13]);
> > +	__raw_writel(0x1406040F, &ddrmr->cr[14]);
> > +	__raw_writel(0x04040000, &ddrmr->cr[16]);
> > +	__raw_writel(0x006DB00C, &ddrmr->cr[17]);
> > +	__raw_writel(0x00000403, &ddrmr->cr[18]);
> > +
> > +	__raw_writel(0x01000000, &ddrmr->cr[20]);
> > +	__raw_writel(0x06060101, &ddrmr->cr[21]);
> > +
> > +	__raw_writel(0x000B0000, &ddrmr->cr[22]);
> > +	__raw_writel(0x03000200, &ddrmr->cr[23]);
> > +	__raw_writel(0x00000006, &ddrmr->cr[24]);
> > +
> > +	__raw_writel(0x00010000, &ddrmr->cr[25]);
> > +	__raw_writel(0x0C28002C, &ddrmr->cr[26]);
> > +	__raw_writel(0x00000005, &ddrmr->cr[28]);
> > +	__raw_writel(0x00000003, &ddrmr->cr[29]);
> > +
> > +	__raw_writel(0x0000000A, &ddrmr->cr[30]);
> > +	__raw_writel(0x00440200, &ddrmr->cr[31]);
> > +	__raw_writel(0x00010000, &ddrmr->cr[33]);
> > +	__raw_writel(0x00050500, &ddrmr->cr[34]);
> > +
> > +	/* Frequency change */
> > +	__raw_writel(0x00000100, &ddrmr->cr[38]);
> > +	__raw_writel(0x04001002, &ddrmr->cr[39]);
> > +
> > +	__raw_writel(0x00000001, &ddrmr->cr[41]);
> > +	__raw_writel(0x00000000, &ddrmr->cr[45]);
> > +	__raw_writel(0x00000000, &ddrmr->cr[46]);
> > +	__raw_writel(0x00000000, &ddrmr->cr[47]);
> > +
> > +	/* DRAM device Mode registers */
> > +	__raw_writel(0x00460420, &ddrmr->cr[48]);
> > +	__raw_writel(0x00000000, &ddrmr->cr[49]);
> > +	__raw_writel(0x00000000, &ddrmr->cr[51]);
> > +	__raw_writel(0x00000000, &ddrmr->cr[57]);
> > +
> > +	/* ZQ stuff */
> > +	__raw_writel(0x01000200, &ddrmr->cr[66]);
> > +	__raw_writel(0x02000040, &ddrmr->cr[67]);
> > +	__raw_writel(0x00000200, &ddrmr->cr[69]);
> > +
> > +	__raw_writel(0x00000040, &ddrmr->cr[70]);
> > +	__raw_writel(0x00000000, &ddrmr->cr[71]);
> > +	__raw_writel(0x01000000, &ddrmr->cr[72]);
> > +
> > +	/* DRAM controller misc */
> > +	__raw_writel(0x0a010300, &ddrmr->cr[73]);
> > +	__raw_writel(0x0101ffff, &ddrmr->cr[74]);
> > +	__raw_writel(0x01010101, &ddrmr->cr[75]);
> > +	__raw_writel(0x03030101, &ddrmr->cr[76]);
> > +	__raw_writel(0x01000101, &ddrmr->cr[77]);
> > +	__raw_writel(0x0000000C, &ddrmr->cr[78]);
> > +	__raw_writel(0x01000000, &ddrmr->cr[79]);
> > +
> > +	/* Disable interrupts */
> > +	__raw_writel(0x1FFFFFFF, &ddrmr->cr[82]);
> > +
> > +	/* ODT */
> > +	__raw_writel(0x01010000, &ddrmr->cr[87]);
> > +	__raw_writel(0x00040000, &ddrmr->cr[88]);
> > +	__raw_writel(0x00000002, &ddrmr->cr[89]);
> > +
> > +	__raw_writel(0x00020000, &ddrmr->cr[91]);
> > +	__raw_writel(0x00000000, &ddrmr->cr[92]);
> > +
> > +	__raw_writel(0x00002819, &ddrmr->cr[96]);
> > +
> > +	/* AXI ports */
> > +	__raw_writel(0x00202000, &ddrmr->cr[105]);
> > +	__raw_writel(0x20200000, &ddrmr->cr[106]);
> > +	__raw_writel(0x00002020, &ddrmr->cr[110]);
> > +	__raw_writel(0x00202000, &ddrmr->cr[114]);
> > +	__raw_writel(0x20200000, &ddrmr->cr[115]);
> > +
> > +	__raw_writel(0x00000101, &ddrmr->cr[117]);
> > +	__raw_writel(0x01010000, &ddrmr->cr[118]);
> > +	__raw_writel(0x00000000, &ddrmr->cr[119]);
> > +
> > +	__raw_writel(0x02020000, &ddrmr->cr[120]);
> > +	__raw_writel(0x00000202, &ddrmr->cr[121]);
> > +	__raw_writel(0x01010064, &ddrmr->cr[122]);
> > +	__raw_writel(0x00000101, &ddrmr->cr[123]);
> > +	__raw_writel(0x00000064, &ddrmr->cr[124]);
> > +
> > +	/* TDFI */
> > +	__raw_writel(0x00000000, &ddrmr->cr[125]);
> > +	__raw_writel(0x00000B00, &ddrmr->cr[126]);
> > +	__raw_writel(0x00000000, &ddrmr->cr[127]);
> > +
> > +	__raw_writel(0x00000000, &ddrmr->cr[131]);
> > +	__raw_writel(0x00000506, &ddrmr->cr[132]);
> > +	__raw_writel(0x02000000, &ddrmr->cr[137]);
> > +	__raw_writel(0x04070303, &ddrmr->cr[139]);
> > +
> > +	__raw_writel(0x00000000, &ddrmr->cr[136]);
> > +
> > +	__raw_writel(0x68200000, &ddrmr->cr[154]);
> > +	__raw_writel(0x00000202, &ddrmr->cr[155]);
> > +	__raw_writel(0x00000006, &ddrmr->cr[158]);
> > +	__raw_writel(0x00000006, &ddrmr->cr[159]);
> > +
> > +	ddr_phy_init();
> > +
> > +	__raw_writel(0x00000601, &ddrmr->cr[0]);
> > +
> > +	udelay(200);
> > +}
> 
> 
> Ditto. There are a lot of magical numbers here associated to magic
> indexes.
> 
> > +
> > +static void clock_init(void)
> > +{
> > +	struct ccm_reg *ccm = (struct ccm_reg *)CCM_BASE_ADDR;
> > +	struct anadig_reg *anadig = (struct anadig_reg *)ANADIG_BASE_ADDR;
> > +
> > +	__raw_writel(0x00000078, &ccm->clpcr);
> > +	__raw_writel(0x000fc000, &ccm->ccgr0);
> > +	__raw_writel(0xf00fc0c0, &ccm->ccgr1);
> > +	__raw_writel(0x0fff0303, &ccm->ccgr2);
> > +	__raw_writel(0x00000033, &ccm->ccgr3);
> > +	__raw_writel(0x33f0f003, &ccm->ccgr4);
> > +	__raw_writel(0x3003cc00, &ccm->ccgr6);
> > +	__raw_writel(0x0000033c, &ccm->ccgr7);
> > +	__raw_writel(0x0000000f, &ccm->ccgr9);
> > +
> > +	__raw_writel(0x00002001, &anadig->pll2_ctrl);
> > +	__raw_writel(0x00011001, &anadig->pll5_ctrl);
> > +	__raw_writel(0x00002001, &anadig->pll1_ctrl);
> > +
> > +	__raw_writel(0x00010005, &ccm->ccr);
> > +	__raw_writel(0x0003ff64, &ccm->ccsr);
> > +	__raw_writel(0x00000810, &ccm->cacrr);
> > +	__raw_writel(0x03cf0000, &ccm->cscmr1);
> > +	__raw_writel(0x01000000, &ccm->cscdr1);
> > +	__raw_writel(0x30004240, &ccm->cscdr2);
> > +	__raw_writel(0x00003f1f, &ccm->cscdr3);
> > +	__raw_writel(0, &ccm->cscmr2);
> > +	__raw_writel(0, &ccm->cscdr4);
> > +}
> 
> The same here. Note that in most boards we use clrsetbits_le32() to set
> the single bits and all bits are defined in the crm_regs.h.
[Alison Wang] Agree, I will change them. Thanks.
> 
> > +int board_eth_init(bd_t *bis)
> > +{
> > +	int ret;
> > +
> > +	setup_iomux_enet();
> > +
> > +	ret = cpu_eth_init(bis);
> > +	if (ret)
> > +		printf("FEC MXC: %s:failed\n", __func__);
> > +
> > +	return 0;
> > +}
> 
> Maybe not useful ? I mean, if you move setup_iomux_enet() into
> board_early_init_f(), you can drop this function, because it simply calls
> cpu_eth_init()
[Alison Wang] Agree, I will drop this funtion, and move setup_iomux_enet() into board_early_init_f(). Thanks.
> 
> > diff --git a/include/configs/mvf600twr.h b/include/configs/mvf600twr.h
> > new file mode 100644 index 0000000..bb1f3ef
> > --- /dev/null
> > +++ b/include/configs/mvf600twr.h
> > @@ -0,0 +1,147 @@
> > +/*
> > + * Copyright 2013 Freescale Semiconductor, Inc.
> > + *
> > + * Configuration settings for the Freescale Vybrid mvf600twr board.
> > + *
> > + * 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., 59 Temple Place, Suite 330, Boston,
> > + * MA 02111-1307 USA
> > + */
> > +
> > +#ifndef __CONFIG_H
> > +#define __CONFIG_H
> > +
> > +#include <asm/arch/imx-regs.h>
> > +#include <config_cmd_default.h>
> > +
> > +#define CONFIG_MVF600
> > +
> > +#define CONFIG_DISPLAY_CPUINFO
> > +#define CONFIG_DISPLAY_BOARDINFO
> > +
> > +#define MACH_TYPE_VYBRID_VF6XX		4146
> > +#define CONFIG_MACH_TYPE		MACH_TYPE_VYBRID_VF6XX
> 
> Your choice, but maybe you could drop MACH_TYPE_VYBRID_VF6XX, because it
> is not used in any file.
[Alison Wang] Agree. I will drop it.
> 
> > +
> > +#define CONFIG_SKIP_LOWLEVEL_INIT
> > +
> > +#define CONFIG_SYS_ICACHE_OFF
> > +#define CONFIG_SYS_CACHELINE_SIZE	64
> 
> On the website, cache is marked "optional" in the figure. Does it mean
> that some SOC have instruction cache and other ones not ?
[Alison Wang] There is instruction cache on MVF600.
> 
> Is this setup related to the fact that you use start.S inside armv7 and
> something is not compatible ?
[Alison Wang] No, I will drop it.
> 
> > +
> > +/* Enable passing of ATAGs */
> > +#define CONFIG_CMDLINE_TAG
> > +
> > +/* Size of malloc() pool */
> > +#define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + 2 * 1024 *
> 1024)
> > +
> > +#define CONFIG_BOARD_EARLY_INIT_F
> > +
> > +#define CONFIG_FSL_LPUART
> > +
> > +/* Allow to overwrite serial and ethaddr */ #define
> > +CONFIG_ENV_OVERWRITE
> > +#define CONFIG_SYS_UART_PORT		(1)
> > +#define CONFIG_BAUDRATE			115200
> > +#define CONFIG_SYS_BAUDRATE_TABLE	{9600, 19200, 38400, 57600,
> 115200}
> > +
> > +#define CONFIG_CMD_BDI		/* bdinfo */
> > +#define CONFIG_CMD_BOOTD
> > +#define CONFIG_CMD_CONSOLE	/* coninfo */
> > +#define CONFIG_CMD_ELF
> > +#define CONFIG_CMD_MEMORY	/* md mm nm mw cp cmp crc base loop mtest
> */
> > +#define CONFIG_CMD_MISC
> > +#undef CONFIG_CMD_IMLS
> 
> You do not include cmd_default.h. Is it wanted ?
[Alison Wang] As I include config_cmd_default.h at the start of this file, I will drop these redundant options.
> 
> > +
> > +/* MUX mode and PAD ctrl are in one register */ #define
> > +CONFIG_IOMUX_SHARE_CONF_REG
> 
> NAK. This is not a board configuration, it is related to the SOC. This
> setup should flow into the related imx-regs.h for this SOC. When you set
> CONFIG_MVF600, this value should be set automatically.
[Alison Wang] Agree, I will move it to imx-regs.h. Thanks.


BTW, what's your suggestions about the other two patches, [PATCH v2 4/6] and [PATCH v2 5/6]?
Thanks.


Best regards,
Alison Wang