Re: [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support

On Thu, 2014-02-27 at 15:02 -0600, Chin Liang See wrote:

Hi Masahiro,

On Thu, 2014-02-27 at 23:35 +0900, Masahiro Yamada wrote:

...

Hello Chin,

...

• nand->ecc.mode = NAND_ECC_HW;
• nand->ecc.size = CONFIG_NAND_DENALI_ECC_SIZE;
• nand->ecc.read_oob = denali_read_oob;
• nand->ecc.write_oob = denali_write_oob;
• nand->ecc.read_page = denali_read_page;
• nand->ecc.read_page_raw = denali_read_page_raw;
• nand->ecc.write_page = denali_write_page;
• nand->ecc.write_page_raw = denali_write_page_raw;

+#ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST

• /* 15bit ECC */
• nand->ecc.bytes = 26;
• nand->ecc.layout = &nand_15bit_oob;

+#else /* 8bit ECC */

• nand->ecc.bytes = 14;
• nand->ecc.layout = &nand_8bit_oob;

+#endif

• nand->ecc.calculate = denali_ecc_calculate;
• nand->ecc.correct = denali_ecc_correct;
• nand->ecc.hwctl = denali_ecc_hwctl;

You set nand->ecc.mode = NAND_ECC_HW, but it looks like you don't set nand->ecc.strength.

So, I think initialization will fail in nand_scan_tail() function.

Here,

            if (mtd->writesize >= chip->ecc.size) {
                    if (!chip->ecc.strength) {
                            pr_warn("Driver must set ecc.strength when using hardware ECC\n");
                            BUG();
                    }
                    break;

Where do you set nand->ecc.strength?

I believe this is only applicable for NAND_ECC_HW_SYNDROME mode. We are using the NAND_ECC_HW (without the syndrome). Wonder you hit error during run?

No, it must always be set for hardware ECC. Note the lack of a break; before case NAND_ECC_HW_SYNDROME.

-Scott