[U-Boot] [PATCH v2 1/3] MX31: Add NAND SPL for i.MX31.

[Scott Wood]

On Sun, May 03, 2009 at 09:56:57PM +0200, Magnus Lilja wrote:
> This patch adds the NAND SPL framework needed to boot i.MX31 boards
> from NAND.

Sorry for the delay...

> diff --git a/include/asm-arm/arch-mx31/mx31-regs.h b/include/asm-arm/arch-mx31/mx31-regs.h
> index a8a05c8..3d811d7 100644
> --- a/include/asm-arm/arch-mx31/mx31-regs.h
> +++ b/include/asm-arm/arch-mx31/mx31-regs.h
> @@ -194,4 +194,94 @@
>  #define CS5_BASE	0xB6000000
>  #define PCMCIA_MEM_BASE	0xC0000000
>  
> +/*
> + * NAND controller
> + */
> +#define NFC_BASE_ADDR	0xB8000000
> +
> +/*
> + * Addresses for NFC registers
> + */
> +#define NFC_BUF_SIZE		(NFC_BASE_ADDR + 0xE00)
> +#define NFC_BUF_ADDR		(NFC_BASE_ADDR + 0xE04)
> +#define NFC_FLASH_ADDR		(NFC_BASE_ADDR + 0xE06)
> +#define NFC_FLASH_CMD		(NFC_BASE_ADDR + 0xE08)

The NFC register info should go in its own header, as it exists on more
than just MX31 (e.g. mpc5xxx).  Should probably use register structs
rather than #defines.

> +/*
> + * Addresses for NFC RAM BUFFER Main area 0
> + */
> +#define MAIN_AREA0	(NFC_BASE_ADDR + 0x000)
> +#define MAIN_AREA1	(NFC_BASE_ADDR + 0x200)
> +#define MAIN_AREA2	(NFC_BASE_ADDR + 0x400)
> +#define MAIN_AREA3	(NFC_BASE_ADDR + 0x600)
> +
> +/*
> + * Addresses for NFC SPARE BUFFER Spare area 0
> + */
> +#define SPARE_AREA0	(NFC_BASE_ADDR + 0x800)
> +#define SPARE_AREA1	(NFC_BASE_ADDR + 0x810)
> +#define SPARE_AREA2	(NFC_BASE_ADDR + 0x820)
> +#define SPARE_AREA3	(NFC_BASE_ADDR + 0x830)

NFC_MAIN_AREA0, etc.

> diff --git a/nand_spl/nand_boot_mx31.c b/nand_spl/nand_boot_mx31.c
> new file mode 100644
> index 0000000..d698d2a
> --- /dev/null
> +++ b/nand_spl/nand_boot_mx31.c

What in this file is MX31-specific?  Even if you don't have other chips
to test on, please try to provide a framework for generic NFC support so
the next chip only needs to tweak the details.  That may be as simple as
s/mx31/nfc/.

> +static void mx31_wait_ready(void)
> +{
> +	while (1) {
> +		if (readw(NFC_CONFIG2) & NFC_INT) {
> +			uint32_t tmp;
> +			/* Reset interrupt flag */
> +			tmp = readw(NFC_CONFIG2);
> +			tmp &= ~NFC_INT;
> +			writew(tmp, NFC_CONFIG2);
> +			break;
> +		}
> +	}
> +}

while (!(readw(NFC_CONFIG2) & NFC_INT))
	;

tmp = readw(NFC_CONFIG2);
...

> +static void mx31_nand_page_address(unsigned int page_address)
> +{
> +	unsigned int page_count;
> +
> +	writew(0x00, NFC_FLASH_ADDR);
> +	writew(NFC_ADDR, NFC_CONFIG2);
> +	mx31_wait_ready();
> +
> +	/* code only for 2kb flash */
> +	if (CFG_NAND_PAGE_SIZE == 0x800) {
> +		writew(0x00, NFC_FLASH_ADDR);
> +		writew(NFC_ADDR, NFC_CONFIG2);
> +		mx31_wait_ready();
> +	}
> +
> +	page_count = CFG_NAND_CHIP_SIZE / CFG_NAND_PAGE_SIZE;
> +
> +	if (page_address <= page_count) {
> +		page_count--; /* transform 0x01000000 to 0x00ffffff */
> +		do {
> +			writew(page_address & 0xff, NFC_FLASH_ADDR);
> +			writew(NFC_ADDR, NFC_CONFIG2);
> +			mx31_wait_ready();
> +			page_address = page_address >> 8;
> +			page_count = page_count >> 8;
> +		} while (page_count);
> +	}

Does the number of address bytes really need to depend on the size of the
flash chip, or can you base it on the number of non-zero bytes in
page_address (the chip will know when the address phase is over because
ALE drops)?

> +static int mx31_nand_check_ecc(void)
> +{
> +	unsigned short ecc_status_register;
> +
> +	ecc_status_register = readw(NFC_ECC_STATUS_RESULT);
> +
> +	if (ecc_status_register != 0)
> +		return 1; /* error */
> +	return 0;
> +}

How about just "return readw(NFC_ECC_STATUS_RESULT);"?

> +static int mx31_read_page(unsigned int page_address, unsigned char *buf)
> +{
> +	int i;
> +	volatile u32 *p1;
> +	volatile u32 *p2;
> +	u32 a;

s/p1/src/
s/p2/dest/

s/a/something (just a little) more descriptive/ -- my first guess was
"address", but that doesn't seem right...

No volatile; use I/O accessors.

> +	writew(0, NFC_BUF_ADDR); /* read in first 0 buffer */
> +	mx31_nand_command(NAND_CMD_READ0);
> +	mx31_nand_page_address(page_address);
> +
> +	if (CFG_NAND_CHIP_SIZE >= 0x08000000)
> +		mx31_nand_command(NAND_CMD_READSTART);

Is it guaranteed that all NAND chips above that size will be large page
and all chips below that size will be small page?

> +	/* it is hardware specific code for 8-bit 512B 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;

The bad block location is typically different (offset 0 rather than 5)
with large page flash.

> +		/* checking first page of each block */

I believe some NAND chips may mark a block bad in its second page...

-Scott