Re: [U-Boot] [PATCH] nand booting support (SPL) for phyCORE-i.MX31

Hi

2008/11/29 user scn1874@yandex.ru:

nand booting support (SPL) for phyCORE-i.MX31

This patch allows Phytec phyCORE-i.MX31 based system to boot from embed NAND-flash memory.It not add support for NAND cmd and/or driver for U-BOOT, it is only SPL for U-BOOT. For NAND support in U-BOOT I give refer to Magnus Lilja's aug/2008 patch series for PDK/Litekit boards. ("[U-Boot] [PATCH v3 4/6] i.MX31: Add i.MX31 NAND Flash Controller driver."; MID 1219998982-21289-5-git-send-email-lilja.magnus@gmail.com)

Some note: by default, Phytec phyCORE-i.MX31 board (pcm-037 on pcm-970) is not suitable for nand-bootloading, because of SW5 switchers block haven't appropriate modes(8-bit, 2k page or 16-bit, 512B page) for BOOTPINS[4:0] and nand-flash(8-bit mode, 512B page) contemporary. There is require to make RESOLDER for really nand-booting either BOOTPIN state or JN1/2 and RN41/42. This nand-spl work with 8-bit, 512B page size nand-flash.

Patch is applicable on v2008.10 release tag master branch.

stgit couldn't apply the patch, but the standard 'patch' command could (it said 'applied with fuzz' on start.S).

Your patch seems to have some trailing whitespaces here and there.

diff --git a/include/asm-arm/arch-mx31/mx31-regs.h b/include/asm-arm/arch-mx31/mx31-regs.h index b04a718..7d6bd45 100644 --- a/include/asm-arm/arch-mx31/mx31-regs.h +++ b/include/asm-arm/arch-mx31/mx31-regs.h @@ -34,7 +34,7 @@ #define CCM_PDR1 (CCM_BASE + 0x08) #define CCM_RCSR (CCM_BASE + 0x0c) #define CCM_MPCTL (CCM_BASE + 0x10) -#define CCM_UPCTL (CCM_BASE + 0x10) +#define CCM_UPCTL (CCM_BASE + 0x14)

The above fix should really go into the arm-tree immediately since it's a separate bug fix (IMHO).

#define CCM_SPCTL (CCM_BASE + 0x18) #define CCM_COSR (CCM_BASE + 0x1C) #define CCM_CGR0 (CCM_BASE + 0x20) @@ -168,4 +168,100 @@ #define CS5_BASE 0xB6000000 #define PCMCIA_MEM_BASE 0xC0000000

+/*

• • NAND controller
• */

+#define NFC_BASE_ADDR 0xB8000000

+/*

• • Addresses for NFC registers
• */

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

+/*

• • Addresses for NFC RAM BUFFER Main area 0
• */

+#define MAIN_AREA0 (volatile u16 *)(NFC_BASE_ADDR + 0x000) +#define MAIN_AREA1 (volatile u16 *)(NFC_BASE_ADDR + 0x200) +#define MAIN_AREA2 (volatile u16 *)(NFC_BASE_ADDR + 0x400) +#define MAIN_AREA3 (volatile u16 *)(NFC_BASE_ADDR + 0x600)

+/*

• • Addresses for NFC SPARE BUFFER Spare area 0
• */

+#define SPARE_AREA0 (volatile u16 *)(NFC_BASE_ADDR + 0x800) +#define SPARE_AREA1 (volatile u16 *)(NFC_BASE_ADDR + 0x810) +#define SPARE_AREA2 (volatile u16 *)(NFC_BASE_ADDR + 0x820) +#define SPARE_AREA3 (volatile u16 *)(NFC_BASE_ADDR + 0x830)

+/*

• • Set INT to 0, FCMD to 1, rest to 0 in NFC_CONFIG2 Register for Command

Change "rest" to "reset" (for all #define's below as well).

• • operation
• */

+#define NFC_CMD 0x1

+/*

• • Set INT to 0, FADD to 1, rest to 0 in NFC_CONFIG2 Register for Address
• • operation
• */

+#define NFC_ADDR 0x2

+/*

• • Set INT to 0, FDI to 1, rest to 0 in NFC_CONFIG2 Register for Input
• • operation
• */

+#define NFC_INPUT 0x4

+/*

• • Set INT to 0, FDO to 001, rest to 0 in NFC_CONFIG2 Register for Data
• • Output operation
• */

+#define NFC_OUTPUT 0x8

+/*

• • Set INT to 0, FD0 to 010, rest to 0 in NFC_CONFIG2 Register for Read ID
• • operation
• */

+#define NFC_ID 0x10

+/*

• • Set INT to 0, FDO to 100, rest to 0 in NFC_CONFIG2 Register for Read
• • Status operation
• */

+#define NFC_STATUS 0x20

+/*

• • Set INT to 1, rest to 0 in NFC_CONFIG2 Register for Read Status
• • operation
• */

+#define NFC_INT 0x8000

+#define NFC_SP_EN (1 << 2) +#define NFC_ECC_EN (1 << 3) +#define NFC_INT_MSK (1 << 4) +#define NFC_BIG (1 << 5) +#define NFC_RST (1 << 6) +#define NFC_CE (1 << 7) +#define NFC_ONE_CYCLE (1 << 8)

+/*

• • NFMS bit in RCSR register for pagesize of nandflash
• */

+#define NFMS (*((volatile u32 *)CCM_RCSR)) +#define NFMS_BIT 30

#endif /* __ASM_ARCH_MX31_REGS_H */

diff --git a/nand_spl/nand_boot_mx31.c b/nand_spl/nand_boot_mx31.c new file mode 100644 index 0000000..0003d7d --- /dev/null +++ b/nand_spl/nand_boot_mx31.c @@ -0,0 +1,160 @@ +/*

• • (C) Copyright 2008
• • Maxim Artamonov, <scn1874 at yandex.ru>
• • (C) Copyright 2006-2008
• • Stefan Roese, DENX Software Engineering, sr@denx.de.
• • This program is free software; you can redistribute it and/or
• • modify it under the terms of the GNU General Public License as
• • published by the Free Software Foundation; either version 2 of
• • the License, or (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • You should have received a copy of the GNU General Public License
• • along with this program; if not, write to the Free Software
• • Foundation, Inc., 59 Temple Place, Suite 330, Boston,
• • MA 02111-1307 USA
• */

+#include <common.h> +#include <nand.h> +#include <asm-arm/arch/mx31-regs.h>

+static void mx31_wait_ready(void) +{

•   while(1)
  

Insert space before '('.

•           if (NFC_CONFIG2 & NFC_INT)
  

•           {
  

Move { to the same line as if.

•                           NFC_CONFIG2 = NFC_CONFIG2 & (~NFC_INT);/*int flag reset*/
  

•                           break;
  

•           }
  

+}

+static void mx31_nand_init(void) +{

•   /*unlocking RAM Buff*/
  

•   NFC_CONFIG = 0x2;
  

•   /* Block of NAND flash to be unlocked */
  

•   /*NFC_UNLOCKSTART_BLKADDR = 0x0;*/
  

•   /*NFC_UNLOCKEND_BLKADDR = 0x4000;*/
  

•   /* Unlock NAND Flash Block Command for given address range */
  

•   /*NFC_WRPROT = 0x4;*/
  

If the above is not needed, remove it.

•   NFC_CONFIG1 = NFC_ECC_EN;
  

+}

+static void mx31_nand_command(unsigned short command) +{

•   NFC_FLASH_CMD = command;
  

•   NFC_CONFIG2 = NFC_CMD;
  

•   mx31_wait_ready();
  

+}

+static void mx31_nand_page_address(unsigned int page_address) +{

•   unsigned int page_count;
  

•   NFC_FLASH_ADDR = 0x00;
  

•   NFC_CONFIG2 = NFC_ADDR;
  

•   mx31_wait_ready();
  

•   if(CFG_NAND_PAGE_SIZE==0x800)
  

Add space before '('. Add space around '=='.

•   {
  

Move { to the if-line.

•           NFC_FLASH_ADDR = 0x00;
  

•           NFC_CONFIG2 = NFC_ADDR;
  

•           mx31_wait_ready();
  

•   };
  

•   page_count = CFG_NAND_CHIP_SIZE/CFG_NAND_PAGE_SIZE;
  

Add space around /.

•   if (page_address <= (page_count))
  

No need to have parenthesis around page_count in the if statement.

•   {
  

Move { to the if-line.

•           page_count--;// transform 0x01000000 to 0x00ffffff
  

Use C style comments /* .. */ instead of C++.

•           do
  

•           {
  

Move { to the do-line.

•                   NFC_FLASH_ADDR = (unsigned short)(page_address & 0xff);
  

•                   NFC_CONFIG2 = NFC_ADDR;
  

•                   mx31_wait_ready();
  

•                   page_address = page_address >> 8;
  

•                   page_count = page_count >> 8;
  

•           }
  

•           while(page_count);
  

Add space before parenthesis. The while statement should probably be moved to the previous line (after }) but I'm not sure what the coding style says aout this.

•   }
  

+}

+static void mx31_nand_data_output(void) +{

•   NFC_CONFIG1 = NFC_ECC_EN;
  

•   NFC_BUF_ADDR = 0; /* read in first 0 buffer*/
  

•   NFC_CONFIG2 = NFC_OUTPUT;
  

•   mx31_wait_ready();
  

+}

+static int mx31_nand_check_ecc(void) +{

•   unsigned short ecc_status_registr;
  

Add empty line after variable declarations.

•   ecc_status_registr = NFC_ECC_STATUS_RESULT;
  

•   if(ecc_status_registr!=0)
  

Add space before parenthesis.Add space around '!='.

•           return 1;/*error*/
  

•   return 0;
  

+}

+static int mx31_read_page(unsigned int page_address, unsigned char *buf) +{

•   int i;
  

•   u32 *p1,*p2;
  

Insert empty line after variable declarations. Add space after comma.

•   mx31_nand_init();
  

•   NFC_BUF_ADDR = 0; /* read in first 0 buffer*/
  

•   mx31_nand_command(NAND_CMD_READ0);
  

•   mx31_nand_page_address(page_address);
  

•   if(CFG_NAND_CHIP_SIZE >= 0x08000000)
  

Add space before parenthesis.

•           mx31_nand_command(NAND_CMD_READSTART);
  

•   mx31_nand_data_output();/*fill the main buffer 0*/
  

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

Add space before parenthesis. And split this line into several lines.

•   p1=NFC_BASE_ADDR; p2=buf;
  

Add spaces around =. Consider placing p2=buf; on a separate line.

•   for(i=0;i<(CFG_NAND_PAGE_SIZE/4);i++)
  

Space before parenthesis, space after ;

•   {
  

Move { to the for-line.

•           *p2= *p1;
  

Add space before =.

•           p1++;p2++;
  

Split in two lines.

•   }
  

•   return 0;
  

+}

+static int nand_load(unsigned int from, unsigned int size, unsigned char *buf) +{

•   int i;
  

Add empty line after variable declarations.

•   if((from+size) > CFG_NAND_CHIP_SIZE)
  

Add space before (.

•           return 1;
  

•   mx31_nand_init();
  

•   from = from/CFG_NAND_PAGE_SIZE;/*from must be set in full pages*/
  

•   for(i=0;i<(size/CFG_NAND_PAGE_SIZE);i++)
  

Add space before (. Add sape after ;. Add space around < and /.

•   {
  

•           mx31_read_page(from, buf);
  

•           from ++;
  

Remove space before ++;

•           buf = buf + CFG_NAND_PAGE_SIZE;
  

•   }
  

•   return 0;
  

+}

Insert empty line.

Shouldn't the code check if the current block is marked as bad before copying all the pages from that block into SDRAM? The first block is typically guaranteed to be good (if I understand correctly) but any subsequent block may be marked bad already from the factory and should be skipped. The entire U-boot won't fit in the first block, atleast for small page NAND devices.

+/*

• • The main entry for NAND booting. It's necessary that SDRAM is already
• • configured and available since this code loads the main U-Boot image
• • from NAND into SDRAM and starts it from there.
• */

+void nand_boot(void) +{

•   __attribute__((noreturn)) void (*uboot)(void);
  

•   nand_load(CFG_NAND_U_BOOT_OFFS, CFG_NAND_U_BOOT_SIZE, (uchar *)CFG_NAND_U_BOOT_DST);
  

Split this line into two in order to keep it less than 80 chars long.

•   uboot = (void *)CFG_NAND_U_BOOT_START;
  

•   uboot();
  

+}

1.5.2.4

Regards, Magnus