From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ivan Khoronzhuk Date: Fri, 20 Jun 2014 17:59:33 +0300 Subject: [U-Boot] [U-Boot, U-boot, 2/2] mtd: nand: davinci: allow to change ecclayout by ecclayout command In-Reply-To: <20140620010750.GA6139@home.buserror.net> References: <1400264797-3593-3-git-send-email-ivan.khoronzhuk@ti.com> <20140620010750.GA6139@home.buserror.net> Message-ID: <53A44C55.3030008@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 06/20/2014 04:07 AM, Scott Wood wrote: > On Fri, May 16, 2014 at 09:26:37PM +0300, Khoronzhuk, Ivan wrote: >> From: WingMan Kwok >> >> This patch adds opportunity to change ecclayout of current nand >> device during runtime. So we can change the current nand device >> ecclayout using the "nand ecclayout set" command before writing >> the data to nand flash. >> >> Signed-off-by: Hao Zhang >> Signed-off-by: WingMan Kwok >> Signed-off-by: Ivan Khoronzhuk >> >> --- >> drivers/mtd/nand/davinci_nand.c | 101 ++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 101 insertions(+) >> >> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c >> index 75b03a7..b33de0d 100644 >> --- a/drivers/mtd/nand/davinci_nand.c >> +++ b/drivers/mtd/nand/davinci_nand.c >> @@ -306,6 +306,103 @@ static struct nand_ecclayout nand_davinci_4bit_layout_oobfirst = { >> #endif >> }; >> >> +#if defined(CONFIG_CMD_NAND_ECCLAYOUT) >> +#if defined(CONFIG_SYS_NAND_PAGE_2K) > Undocumented CONFIG symbol (yes, I know it's not new)... Also redundant > with CONFIG_SYS_NAND_PAGE_SIZE (though that's only documented in the > context of SPL). > >> +static struct nand_ecclayout nand_keystone_rbl_4bit_layout_oobfirst = { >> + .eccbytes = 40, >> + .eccpos = { >> + 6, 7, >> + 8, 9, 10, 11, 12, 13, 14, 15, >> + 22, 23, >> + 24, 25, 26, 27, 28, 29, 30, 31, >> + 38, 39, >> + 40, 41, 42, 43, 44, 45, 46, 47, >> + 54, 55, >> + 56, 57, 58, 59, 60, 61, 62, 63, >> + }, > Why the odd 2/8 pattern? ...I'll align. > >> + .oobfree = { >> + {.offset = 2, .length = 4, }, >> + {.offset = 16, .length = 6, }, >> + {.offset = 32, .length = 6, }, >> + {.offset = 48, .length = 6, }, >> + }, >> +}; >> +#elif defined(CONFIG_SYS_NAND_PAGE_4K) >> +static struct nand_ecclayout nand_keystone_rbl_4bit_layout_oobfirst = { >> + .eccbytes = 80, >> + .eccpos = { >> + 6, 7, >> + 8, 9, 10, 11, 12, 13, 14, 15, >> + 22, 23, >> + 24, 25, 26, 27, 28, 29, 30, 31, >> + 38, 39, >> + 40, 41, 42, 43, 44, 45, 46, 47, >> + 54, 55, >> + 56, 57, 58, 59, 60, 61, 62, 63, >> + 70, 71, >> + 72, 73, 74, 75, 76, 77, 78, 79, >> + 86, 87, >> + 88, 89, 90, 91, 92, 93, 94, 95, >> + 102, 103, >> + 104, 105, 106, 107, 108, 109, 110, 111, >> + 118, 119, >> + 120, 121, 122, 123, 124, 125, 126, 127, >> + }, >> + .oobfree = { >> + {.offset = 2, .length = 4, }, >> + {.offset = 16, .length = 6, }, >> + {.offset = 32, .length = 6, }, >> + {.offset = 48, .length = 6, }, >> + {.offset = 64, .length = 6, }, >> + {.offset = 80, .length = 6, }, >> + {.offset = 96, .length = 6, }, >> + {.offset = 112, .length = 6, }, >> + }, >> +}; >> +#endif >> + >> +#define NAND_ECCLAYOUT_NUM 2 >> + >> +struct nand_ecclayout *davinci_nand_ecclayouts[NAND_ECCLAYOUT_NUM] = { >> + &nand_davinci_4bit_layout_oobfirst, >> + &nand_keystone_rbl_4bit_layout_oobfirst, >> +}; >> + >> +int board_nand_ecclayout_get_idx(struct nand_chip *nand, >> + struct nand_ecclayout *p) >> +{ >> + int i; >> + >> + if (!p) >> + return -1; >> + >> + for (i = 0; i < NAND_ECCLAYOUT_NUM; i++) >> + if (davinci_nand_ecclayouts[i] == p) >> + return i; >> + >> + return -1; >> +} > Use ARRAY_SIZE(). Ok. > >> + >> +struct nand_ecclayout *board_nand_ecclayout_get_layout(struct nand_chip *nand, >> + int idx) >> +{ >> + if ((idx >= 0) && (idx < NAND_ECCLAYOUT_NUM)) >> + return davinci_nand_ecclayouts[idx]; >> + else >> + return NULL; >> +} >> + >> +int board_nand_ecclayout_set(struct nand_chip *nand, int idx) >> +{ >> + if (idx < 0 || idx >= NAND_ECCLAYOUT_NUM) >> + return -1; >> + >> + nand->ecc.layout = davinci_nand_ecclayouts[idx]; >> + >> + return 0; >> +} >> +#endif > Put a comment on this indicating what condition the #endif is ending. Ok. > >> static void nand_davinci_4bit_enable_hwecc(struct mtd_info *mtd, int mode) >> { >> u32 val; >> @@ -631,7 +728,11 @@ void davinci_nand_init(struct nand_chip *nand) >> nand->ecc.calculate = nand_davinci_4bit_calculate_ecc; >> nand->ecc.correct = nand_davinci_4bit_correct_data; >> nand->ecc.hwctl = nand_davinci_4bit_enable_hwecc; >> +#ifndef CONFIG_CMD_NAND_ECCLAYOUT >> nand->ecc.layout = &nand_davinci_4bit_layout_oobfirst; >> +#else >> + nand->ecc.layout = &nand_keystone_rbl_4bit_layout_oobfirst; >> +#endif > Use "ifdef/else" rather than "ifndef/else". > > Why does enabling layout selection change the default layout? Nothing in > the changelog suggests that. It's not correct. I will move it to board code. I'll better do smth like the following: In board config: #define CONFIG_SYS_NAND_SELF_INIT in board config: In nand davinci header: #define NAND_DAVINCI_4BIT_LAYOUT 0 #define NAND_KEYSTONE_RBL_4BIT_LAYOUT 1 In board file: int board_nand_init(struct nand_chip *chip) { davinci_nand_init(chip); chip->ecc.layout = board_nand_ecclayout_get_layout(NAND_KEYSTONE_RBL_4BIT_LAYOUT); return 0; } > > -Scott Thanks a lot Scott, I will send the new series soon. -- Regards, Ivan Khoronzhuk