On 01/18/2013 06:00:28 AM, Sergey Lapin wrote:
diff --git a/drivers/mtd/nand/bfin_nand.c b/drivers/mtd/nand/bfin_nand.c
index c7ddbb2..7e755e8 100644
--- a/drivers/mtd/nand/bfin_nand.c
+++ b/drivers/mtd/nand/bfin_nand.c
@@ -374,9 +374,11 @@ int board_nand_init(struct nand_chip *chip)
                if (!NAND_IS_512()) {
                        chip->ecc.bytes = 3;
                        chip->ecc.size = 256;
+                       chip->ecc.strength = 1;
                } else {
                        chip->ecc.bytes = 6;
                        chip->ecc.size = 512;
+                       chip->ecc.strength = 2;
                }

Hmm, I'm not sure that ecc.strength should be set to 2 here, if it's really two separate 256-byte regions each of which can correct 1 bit. 2 bit correction can only be done under certain circumstances (i.e. the errors are not in the same half).

Linux does the same thing you've done, but that was also a "fix all the drivers" patch, so I'd like the input of someone familiar with the hardware. CCing Mike Frysinger. Maybe the driver should be reporting an ECC block size of 256 bytes? Or at least internally return the number of bitflips from the half that had more bitflips.

                chip->ecc.mode = NAND_ECC_HW;
                chip->ecc.calculate = bfin_nfc_calculate_ecc;
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 04b24f3..9777878 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -613,6 +613,7 @@ void davinci_nand_init(struct nand_chip *nand)
        nand->ecc.mode = NAND_ECC_HW;
        nand->ecc.size = 512;
        nand->ecc.bytes = 3;
+       nand->ecc.strength = 4;
        nand->ecc.calculate = nand_davinci_calculate_ecc;
        nand->ecc.correct  = nand_davinci_correct_data;
        nand->ecc.hwctl  = nand_davinci_enable_hwecc;

This is in the 1-bit ECC #ifdef, so ecc.strength should be 1, and later in CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST in should be set to 4.

diff --git a/drivers/mtd/nand/diskonchip.c b/drivers/mtd/nand/diskonchip.c
index 104d97f..4cd741e 100644
--- a/drivers/mtd/nand/diskonchip.c
+++ b/drivers/mtd/nand/diskonchip.c
@@ -1658,6 +1658,7 @@ static int __init doc_probe(unsigned long physadr)
        nand->ecc.mode               = NAND_ECC_HW_SYNDROME;
        nand->ecc.size               = 512;
        nand->ecc.bytes              = 6;
+       nand->ecc.strength   = 2;
        nand->bbt_options    = NAND_BBT_USE_FLASH;

        doc->physadr         = physadr;
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index 65edbf5..c3f350d 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -781,6 +781,12 @@ static int fsl_elbc_chip_init(int devnum, u8 *addr)
                nand->ecc.size = 512;
                nand->ecc.bytes = 3;
                nand->ecc.steps = 1;
+               chip->ecc.strength = 1;
+               /*
+ * FIXME: can hardware ecc correct 4 bitflips if page size is + * 2k? Then does hardware report number of corrections for this + * case? If so, ecc_stats reporting needs to be fixed as well.
+                */

It can correct 1 bitflip per 512 bytes. This is a similar situation to blackfin (as far as I can tell), except that we report the actual hardware ECC block size, so ecc.strength = 1 is clearly correct.

As for ecc_stats, read_page() is supposed to return the number of bitflips from the ECC block with the most bitflips (see the generic read_page implementations). This change was made to fsl_elbc_nand.c in Linux. Note that all of the conversions of read_page() functions to simply return zero seem to be incorrect, and should be returning max_bitflips.

diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
index 5d47b40..823a2eb 100644
--- a/drivers/mtd/nand/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/fsl_ifc_nand.c
@@ -871,6 +871,7 @@ int board_nand_init(struct nand_chip *nand)
        /* Hardware generates ECC per 512 Bytes */
        nand->ecc.size = 512;
        nand->ecc.bytes = 8;
+       nand->ecc.strength = 1;

        switch (csor & CSOR_NAND_PGS_MASK) {
        case CSOR_NAND_PGS_512:

IFC has ECC strength of 4 or 8 depending on ECC mode. It also needs max_bitflips handling in read_page().

diff --git a/drivers/mtd/nand/jz4740_nand.c b/drivers/mtd/nand/jz4740_nand.c
index 3ec34f3..3f64dc3 100644
--- a/drivers/mtd/nand/jz4740_nand.c
+++ b/drivers/mtd/nand/jz4740_nand.c
@@ -253,6 +253,11 @@ int board_nand_init(struct nand_chip *nand)
        nand->ecc.mode               = NAND_ECC_HW_OOB_FIRST;
        nand->ecc.size               = CONFIG_SYS_NAND_ECCSIZE;
        nand->ecc.bytes              = CONFIG_SYS_NAND_ECCBYTES;
+       nand->ecc.strength   = 2;
+       /*
+ * FIXME: ecc_strength value of 2 bits per 512 bytes of data is a
+        * conservative guess, given 9 ecc bytes and reed-solomon alg.
+        */
        nand->ecc.layout     = &qi_lb60_ecclayout_2gb;
        nand->chip_delay     = 50;
        nand->options                = NAND_USE_FLASH_BBT;

The Linux driver sets ecc.strength to 4, though that may have been after Linux 3.7.

diff --git a/drivers/mtd/nand/tegra_nand.c b/drivers/mtd/nand/tegra_nand.c
index 854fd21..6afbec6 100644
--- a/drivers/mtd/nand/tegra_nand.c
+++ b/drivers/mtd/nand/tegra_nand.c
@@ -1014,6 +1014,7 @@ int tegra_nand_init(struct nand_chip *nand, int devnum)
        nand->ecc.write_page_raw = nand_write_page_raw;
        nand->ecc.read_oob = nand_read_oob;
        nand->ecc.write_oob = nand_write_oob;
+       nand->ecc.strength = 1;
        nand->select_chip = nand_select_chip;
        nand->dev_ready  = nand_dev_ready;
        nand->priv = &nand_ctrl;

Jim, does ecc.strength = 1 look correct for Tegra? It has more ECC bytes than I'd expect for 1-bit correction (does "4 symbol correct ECC" mean "4-bit correctable ECC"?).

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

Reply via email to