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