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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to