[U-Boot] [PATCH] V2 NAND_SPL support for phycore imx31

[Scott Wood]

On Fri, Dec 05, 2008 at 02:13:51PM +0300, Maxim Artamonov wrote:
> +#ifdef CONFIG_NAND_SPL
> +/* somewhat macro to reduce bin size for CONFIG_NAND_SPL*/
> +.macro FILLREGS begreg, val, count, step

Why not use this macro always?

> @@ -32,6 +35,15 @@
>  #include <version.h>
>  .globl _start
>  _start: b	reset
> +#ifdef CONFIG_NAND_SPL
> +	nop
> +	nop
> +	nop
> +	nop
> +	nop
> +	nop
> +	nop
> +#else
>  #ifdef CONFIG_ONENAND_IPL

#elif

Do NAND and OneNAND really have different requirements here?

> @@ -156,9 +169,9 @@ relocate:				/* relocate U-Boot to RAM	    */
>  	adr	r0, _start		/* r0 <- current position of code   */
>  	ldr	r1, _TEXT_BASE		/* test if we run from flash or RAM */
>  	cmp	r0, r1			/* don't reloc during debug	    */
> -#ifndef CONFIG_ONENAND_IPL
> +#if !defined(CONFIG_ONENAND_IPL) && !defined(CONFIG_NAND_SPL)

Should define one symbol that is set if either of the above are set, to
simplify the ifdefs.

> +#define NFC_BUF_SIZE            (*((volatile u16 *)(NFC_BASE_ADDR + 0xE00)))
> +#define NFC_BUF_ADDR            (*((volatile u16 *)(NFC_BASE_ADDR + 0xE04)))
> +#define NFC_FLASH_ADDR          (*((volatile u16 *)(NFC_BASE_ADDR + 0xE06)))
> +#define NFC_FLASH_CMD           (*((volatile u16 *)(NFC_BASE_ADDR + 0xE08)))
> +#define NFC_CONFIG              (*((volatile u16 *)(NFC_BASE_ADDR + 0xE0A)))
> +#define NFC_ECC_STATUS_RESULT   (*((volatile u16 *)(NFC_BASE_ADDR + 0xE0C)))
> +#define NFC_RSLTMAIN_AREA       (*((volatile u16 *)(NFC_BASE_ADDR + 0xE0E)))
> +#define NFC_RSLTSPARE_AREA      (*((volatile u16 *)(NFC_BASE_ADDR + 0xE10)))
> +#define NFC_WRPROT              (*((volatile u16 *)(NFC_BASE_ADDR + 0xE12)))
> +#define NFC_UNLOCKSTART_BLKADDR (*((volatile u16 *)(NFC_BASE_ADDR + 0xE14)))
> +#define NFC_UNLOCKEND_BLKADDR   (*((volatile u16 *)(NFC_BASE_ADDR + 0xE16)))
> +#define NFC_NF_WRPRST           (*((volatile u16 *)(NFC_BASE_ADDR + 0xE18)))
> +#define NFC_CONFIG1             (*((volatile u16 *)(NFC_BASE_ADDR + 0xE1A)))
> +#define NFC_CONFIG2             (*((volatile u16 *)(NFC_BASE_ADDR + 0xE1C)))

Use I/O accessors, and move the NFC register layout to a
non-arch-specific header file (preferably as a struct), as it's shared
with other chips.

>  #define	CONFIG_EXTRA_ENV_SETTINGS											\
> -	"bootargs_base=setenv bootargs console=ttySMX0,115200\0"							\
> -	"bootargs_nfs=setenv bootargs $(bootargs) root=/dev/nfs ip=dhcp nfsroot=$(serverip):$(nfsrootfs),v3,tcp\0"	\
> -	"bootargs_flash=setenv bootargs $(bootargs) root=/dev/mtdblock2 rootfstype=jffs2"				\
> -	"bootargs_mtd=setenv bootargs $(bootargs) $(mtdparts)"								\
> +	"jffs2=root-pcm037.jffs2\0"											\
> +	"uimage=uImage-pcm037\0"											\
> +	"nfsrootfs=/nfsroot\0"												\
>  	"bootcmd=run bootcmd_net\0"											\
> +	"mtdids=nor0=phys_mapped_flash\0"										\
> +	"bootargs_base=setenv bootargs console=ttymxc0,115200\0"							\
> +	"bootargs_nfs=setenv bootargs $(bootargs) root=/dev/nfs ip=dhcp nfsroot=$(serverip):$(nfsrootfs),v3,tcp\0"	\
> +	"bootargs_mtd=setenv bootargs $(bootargs) $(mtdparts)\0"							\
> +	"bootargs_flash=setenv bootargs $(bootargs) root=/dev/mtdblock3 rootfstype=jffs2\0"				\

There are changes in here that don't seem to be related to nand boot.

> +static void mx31_wait_ready(void)
> +{
> +	while (1)
> +		if (NFC_CONFIG2 & NFC_INT){
> +				NFC_CONFIG2 = NFC_CONFIG2 & (~NFC_INT);/*int flag reset*/
> +				break;
> +		}
> +}

There's an extra indentation level in the if-body.

Braces around the while-body.  Space before and after braces (here and
elsewhere).

> +static void mx31_nand_init(void)
> +{
> +	/*unlocking RAM Buff*/
> +	NFC_CONFIG = 0x2;
> +
> +	/*hardware ECC checking and correct*/
> +	NFC_CONFIG1 = NFC_ECC_EN;

Space after /*, before */

> +	if (mx31_nand_check_ecc ())
> +		while (1){
> +			break;};	/*!!!!!*/

What is this?

> +	p1 = (u32 *)MAIN_AREA0;
> +	p2 = (u32 *)buf;
> +
> +	/*main copy loop from NAND-buffer to SDRAM memory*/
> +	for (i=0; i < (CFG_NAND_PAGE_SIZE / 4); i++){		
> +		*p2 = *p1;
> +		p1++;
> +		p2++;
> +	}
> +	
> +	p1 = (u32 *)SPARE_AREA0;
> +
> +	/*it is hardware specific code for 8-bit 512kB NAND-flash spare area*/
> +	p1++;
> +	a = *p1;
> +	a = (a & 0x0000ff00) >> 8;
> +
> +	if (a != 0xff) /*bad block marker verify*/
> +		return 1;/*potential bad block*/
> +	
> +	return 0;
> +}
> +
> +static int nand_load(unsigned int from, unsigned int size, unsigned char *buf)
> +{
> +	int i, bb;
> +	
> +	mx31_nand_init ();
> +	
> +	/*conver from to page number*/
> +	from = from / CFG_NAND_PAGE_SIZE;
> +
> +	i = 0;
> +
> +	while (i < (size/CFG_NAND_PAGE_SIZE)){
> +		
> +		if ((from*CFG_NAND_PAGE_SIZE) >= CFG_NAND_CHIP_SIZE)
> +			return 2;/*memory segment violation*/
> +
> +		bb = mx31_read_page (from, buf);
> +
> +		/*checking first page of each block*/
> +		/*if this page has bb marker, then skip whole block*/
> +		if ((!(from % CFG_NAND_PAGE_COUNT)) && bb){
> +			from = from + CFG_NAND_PAGE_COUNT;

CONFIG_NAND_PAGES_PER_BLOCK would probably be a better name.

> +		}
> +		else{

} else {

-Scott