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

[Magnus Lilja]

2009/5/29 Scott Wood <scottwood at freescale.com>:
> 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...

Well, better now than the next time I post these patches.

>> 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.

Yes, will do that. I'm converting the #define's to struct {} and all
accesses will be in the readl/writel(&nfc->flash_cmd) format.

>> 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/.

I'll look into that.

>> +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);
> ...
>

Ok.

>> +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)?

Ok, will try to look into it.

>> +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);"?

ok

>> +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...

Ok

> No volatile; use I/O accessors.

Yes, already on todo-list.

>> +     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?

No, I'll change to use the CONFIG_SYS_NAND_PAGE_SIZE to do different
stuff depending on the page size.

>> +     /* 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.

I think that's because of the non-standard imlementation of large page
support in the i.MX31 NFC.

>> +             /* checking first page of each block */
>
> I believe some NAND chips may mark a block bad in its second page...

I'll see what I can do.

/Magnus