nand booting support (SPL) for phyCORE-i.MX31
PATCH 1/1 nand booting support (SPL) for phyCORE-i.MX31
Hello, Magnus
29.11.08, 12:59, "Magnus Lilja" <[email protected]>:
Thank for time that you spend to review this patch.
Quoted 7 lines
Hi
2008/11/29 user <[email protected]>:Quoted 3 lines
nand booting support (SPL) for phyCORE-i.MX31
...
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).
May be it's my native utf-8 charset doesn't recode to latin-8859-1? Anything, I try this again.
Quoted 1 line
Your patch seems to have some trailing whitespaces here and there.
It is require to correct in according codestyle?
Quoted 5 lines
Review
Quoted 3 lines
#define
CCM_MPCTL(CCM_BASE+0x10)(quoted diff) +1
-1-#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).
What can I make with it?
Quoted 3 lines
Quoted 2 lines
+/*
+ * Set INT to 0, FCMD to 1, rest to 0 inNFC_CONFIG2Register for CommandChange "rest" to "reset" (for all #define's below as well).
More correctly to change "rest" to "the rest".
Quoted 7 lines
Review
Quoted 6 lines
(quoted diff) +6
-0+ /* 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.
You are right.
Quoted 7 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ mx31_read_page(from, buf); + from ++;Remove space before ++;
Review
Quoted 4 lines
(quoted diff) +4
-0+ buf = buf + CFG_NAND_PAGE_SIZE; + } + return 0; +}
Quoted 7 lines
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.
Yes, it's so. I wrote it for guaranteed first block chip. But for phycore board it's not so. I'll add bb verify feature.
Best wishes, Maxim
PATCH 1/1 nand booting support (SPL) for phyCORE-i.MX31
Hi
2008/12/1 Maxim Artamonov <[email protected]>:
Quoted 6 lines
Hello, Magnus
29.11.08, 12:59, "Magnus Lilja" <[email protected]>:
Quoted 1 line
Your patch seems to have some trailing whitespaces here and there.
It is require to correct in according codestyle?
Yes, I think so.
Quoted 6 lines
Quoted 5 lines
Review
Quoted 3 lines
#define
CCM_MPCTL(CCM_BASE+0x10)(quoted diff) +1
-1-#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).
What can I make with it?
You can move this part of the patch to a separate stand-alone patch and submit it to the mailing list.
Quoted 5 lines
Quoted 4 lines
Quoted 3 lines
+/*
+ * Set INT to 0, FCMD to 1, rest to 0 inNFC_CONFIG2Register for CommandChange "rest" to "reset" (for all #define's below as well).
More correctly to change "rest" to "the rest".
Ah, true.
Quoted 14 lines
Quoted 7 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ mx31_read_page(from, buf); + from ++;Remove space before ++;
Review
Quoted 4 lines
(quoted diff) +4
-0+ buf = buf + CFG_NAND_PAGE_SIZE; + } + return 0; +}Quoted 7 lines
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.
Quoted 2 lines
Yes, it's so. I wrote it for guaranteed first block chip. But for phycore board it's not so. I'll add bb verify feature.
Excellent!
Thanks, Magnus