From mboxrd@z Thu Jan  1 00:00:00 1970
From: Haiying Wang <Haiying.Wang@freescale.com>
Date: Fri, 28 Jan 2011 14:22:14 -0500
Subject: [U-Boot] [PATCH 5/8 v3] P1021: add P1021MDS board support
In-Reply-To: <AANLkTin2dxhLWWDX4CLPM=SP3SpjxecC==Zx4KowMrSx@mail.gmail.com>
References: <1296190690-21146-1-git-send-email-Haiying.Wang@freescale.com>
	<1296190690-21146-3-git-send-email-Haiying.Wang@freescale.com>
	<AANLkTin2dxhLWWDX4CLPM=SP3SpjxecC==Zx4KowMrSx@mail.gmail.com>
Message-ID: <1296242534.2918.132.camel@haiying-laptop>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Fri, 2011-01-28 at 13:06 -0600, Timur Tabi wrote:
> On Thu, Jan 27, 2011 at 10:58 PM,  <Haiying.Wang@freescale.com> wrote:
> 
> > +/* These are used when DDR doesn't use SPD.  */
> > +#define CONFIG_SYS_SDRAM_SIZE           512            /* DDR is 512MB */
> > +#define CONFIG_SYS_DDR_CS0_BNDS         0x0000001F
> > +#define CONFIG_SYS_DDR_CS0_CONFIG       0x80014202
> > +#define CONFIG_SYS_DDR_CS0_CONFIG_2    0x00000000
> > +#define CONFIG_SYS_DDR_SDRAM_CFG       0x47000000
> > +#define CONFIG_SYS_DDR_SDRAM_CFG_2     0x04401040
> > +#define CONFIG_SYS_DDR_ZQ_CNTL         0x89080600
> > +#define CONFIG_SYS_DDR_WRLVL_CNTL      0x86559608
> > +#define CONFIG_SYS_DDR_CDR_1           0x000eaa00
> > +#define CONFIG_SYS_DDR_CDR_2           0x00000000
> > +#define CONFIG_SYS_DDR_OCD_CTRL         0x00000000
> > +#define CONFIG_SYS_DDR_OCD_STATUS       0x00000000
> > +#define CONFIG_SYS_DDR_CONTROL          0x470c0000      /* Type = DDR3 */
> > +#define CONFIG_SYS_DDR_CONTROL_2       0x04401050
> > +#define CONFIG_SYS_DDR_DATA_INIT        0x1021babe
> > +#define CONFIG_SYS_DDR_TIMING_3                0x00010000
> > +#define CONFIG_SYS_DDR_TIMING_0                0x00330004
> > +#define CONFIG_SYS_DDR_TIMING_1                0x5d5bd746
> > +#define CONFIG_SYS_DDR_TIMING_2                0x0fa8c8cd
> > +#define CONFIG_SYS_DDR_SDRAM_MODE      0x40461320
> > +#define CONFIG_SYS_DDR_SDRAM_MODE_2    0x8000C000
> > +#define CONFIG_SYS_DDR_SDRAM_INTERVAL  0x0a280000
> > +#define CONFIG_SYS_DDR_SDRAM_CLK_CNTL  0x03000000
> > +#define CONFIG_SYS_DDR_TIMING_4                0x00220001
> > +#define CONFIG_SYS_DDR_TIMING_5                0x03402400
> 
> Aren't static DDR configurations now handled in a board-specific
> source file?  Look at board/freescale/corenet_ds/p4080ds_ddr.c
yes, static ddr configurations are handled in board c file, but it
doesn't matter to define those MACROs in header file. But anyway, I've
remove static ddr setting from board file as Scott suggested, so I
should remove above setting in header file. 

> 
> > +#define CONFIG_ID_EEPROM
> > +#ifdef CONFIG_ID_EEPROM
> > +#define CONFIG_SYS_I2C_EEPROM_NXID
> > +#endif
> 
> No need for the #ifdef here.  CONFIG_SYS_I2C_EEPROM_NXID is not used
> in any Makefile.
OK.

> 
> > +#define CONFIG_SYS_I2C_EEPROM_ADDR      0x52
> 
> Not 0x57?  That's where the NXID EEPROM almost always is.
It is board specific value, isn't it? P1021MDS does use 0x52 for board eeprom.

> > +void putc(char c)
> > +{
> > +       if (c == '\n')
> > +               NS16550_putc((NS16550_t)CONFIG_SYS_NS16550_COM1, '\r');
> > +
> > +       NS16550_putc((NS16550_t)CONFIG_SYS_NS16550_COM1, c);
> > +}
> > +
> > +void puts(const char *str)
> > +{
> > +       while (*str)
> > +               putc(*str++);
> > +}
> 
> These look like functions that shouldn't be in board-specific code.
> 
Scott replied you. I also think with partial linking, this can be
factored out. 

Haiying