From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ivan Khoronzhuk Date: Fri, 20 Jun 2014 19:57:13 +0300 Subject: [U-Boot] [U-Boot, U-boot, 2/2] mtd: nand: davinci: allow to change ecclayout by ecclayout command In-Reply-To: <1403281359.12851.188.camel@snotra.buserror.net> References: <1400264797-3593-3-git-send-email-ivan.khoronzhuk@ti.com> <20140620010750.GA6139@home.buserror.net> <53A44C55.3030008@ti.com> <1403281359.12851.188.camel@snotra.buserror.net> Message-ID: <53A467E9.6060308@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 07:22 PM, Scott Wood wrote: > On Fri, 2014-06-20 at 17:59 +0300, Ivan Khoronzhuk wrote: >> 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; >> } > What does CONFIG_SYS_NAND_SELF_INIT have to do with this? Note that > with that set, board_nand_init does not take an argument. Yes. Just seeing this while implementing...It's not needed. > > "board_nand_ecclayout_get_layout" is a bit long -- how about just > "board_nand_get_ecclayout"? I will rename to board_nand_get_ecclayout -- Regards, Ivan Khoronzhuk