Re: [U-Boot] [U-Boot, U-boot, 2/2] mtd: nand: davinci: allow to change ecclayout by ecclayout command

[Ivan Khoronzhuk]

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 <w-kw...@ti.com>

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 <hzh...@ti.com>
Signed-off-by: WingMan Kwok <w-kw...@ti.com>
Signed-off-by: Ivan Khoronzhuk <ivan.khoronz...@ti.com>

---
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

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot