On 01/28/2013 07:35:40 AM, Jordy van Wolferen wrote:
This is tested with a custom AM3359 (rev 2.0) board.
NAND chip: MT29F16G08ABABAWP

This code allows me to boot from ROM code.
The ROM code forces BCH16 on NAND chips with a 4k page size.

BCH16 is not enabled by default.


---

Missing Signed-off-by (please read the "Sign your work" section of Documentation/SubmittingPatches in Linux and be sure that you meet the conditions of the Developer's Certificate of Origin before adding your sign off).

Could you explain the patch in a bit more detail? You say it is "not enabled by default" -- what would be required to enable it?

 arch/arm/include/asm/arch-am33xx/cpu.h       |   8 +-
 arch/arm/include/asm/arch-am33xx/omap_gpmc.h |  43 ++++++++
drivers/mtd/nand/omap_gpmc.c | 150 +++++++++++++++++----------
 include/linux/mtd/mtd-abi.h                  |   2 +-
 4 files changed, 148 insertions(+), 55 deletions(-)

diff --git a/arch/arm/include/asm/arch-am33xx/cpu.h b/arch/arm/include/asm/arch-am33xx/cpu.h
index 16e8a80..0a1f1ff 100644
--- a/arch/arm/include/asm/arch-am33xx/cpu.h
+++ b/arch/arm/include/asm/arch-am33xx/cpu.h
@@ -78,6 +78,10 @@ struct bch_res_0_3 {
        u32 bch_result_x[4];
 };

+struct bch_res_4_6 {
+       u32 bch_result_x[3];
+};
+
 struct gpmc {
        u8 res1[0x10];
        u32 sysconfig;          /* 0x10 */
@@ -107,7 +111,9 @@ struct gpmc {
        u8 res7[12];            /* 0x224 */
        u32 testmomde_ctrl;     /* 0x230 */
        u8 res8[12];            /* 0x234 */
-       struct bch_res_0_3 bch_result_0_3[2];   /* 0x240 */
+       struct bch_res_0_3 bch_result_0_3;      /* 0x240 */
+       u32 dummy[44];          /* not used */
+       struct bch_res_4_6 bch_result_4_6;      /* 300 */
 };

 /* Used for board specific gpmc initialization */
diff --git a/arch/arm/include/asm/arch-am33xx/omap_gpmc.h b/arch/arm/include/asm/arch-am33xx/omap_gpmc.h
index 572f9d0..534fa6e 100644
--- a/arch/arm/include/asm/arch-am33xx/omap_gpmc.h
+++ b/arch/arm/include/asm/arch-am33xx/omap_gpmc.h
@@ -117,4 +117,47 @@
                {.offset = 106,\
                 .length = 8 } } \
 }
+
+#define GPMC_NAND_4K_HW_BCH8_ECC_LAYOUT {\
+       .eccbytes = 112,\
+       .eccpos = {2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,\
+ 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27,\ + 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39,\ + 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51,\ + 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,\ + 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75,\ + 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87,\ + 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99,\ + 100, 101, 102, 103, 104, 105, 106, 107, 108, 109,\
+                               110, 111, 112, 113},\
+       .oobfree = {\
+               {.offset = 114,\
+                .length = 110 } } \
+}
+
+#define GPMC_NAND_4K_HW_BCH16_ECC_LAYOUT {\
+       .eccbytes = 208,\
+       .eccpos = { 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,\
+ 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27,\ + 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39,\ + 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51,\ + 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,\ + 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75,\ + 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87,\ + 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99,\ + 100, 101, 102, 103, 104, 105, 106, 107, 108, 109,\ + 110, 111, 112, 113, 114, 115, 116, 117, 118, 119,\ + 120, 121, 122, 123, 124, 125, 126, 127, 128, 129,\ + 130, 131, 132, 133, 134, 135, 136, 137, 138, 139,\ + 140, 141, 142, 143, 144, 145, 146, 147, 148, 149,\ + 150, 151, 152, 153, 154, 155, 156, 157, 158, 159,\ + 160, 161, 162, 163, 164, 165, 166, 167, 168, 169,\ + 170, 171, 172, 173, 174, 175, 176, 177, 178, 179,\ + 180, 181, 182, 183, 184, 185, 186, 187, 188, 189,\ + 190, 191, 192, 193, 194, 195, 196, 197, 198, 199,\ + 200, 201, 202, 203, 204, 205, 206, 207, 208, 209},\

You have too many tabs here -- be sure your editor is set for 8-character tabs.

+       .oobfree = {\
+               {.offset = 210,\
+                .length = 14 } } \
+}
 #endif /* __ASM_ARCH_OMAP_GPMC_H */
diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
index cee394e..3c42a54 100644
--- a/drivers/mtd/nand/omap_gpmc.c
+++ b/drivers/mtd/nand/omap_gpmc.c
@@ -76,8 +76,8 @@ int omap_spl_dev_ready(struct mtd_info *mtd)

 /*
  * omap_hwecc_init - Initialize the Hardware ECC for NAND flash in
- *                   GPMC controller
- * @mtd:        MTD device structure
+ *                                      GPMC controller
+ * @mtd:               MTD device structure
  *
  */
 static void __maybe_unused omap_hwecc_init(struct nand_chip *chip)

No unrelated whitespace changes, please.

@@ -258,7 +258,7 @@ struct nand_bch_priv {
 #define ECC_BCH8_NIBBLES       26
 #define ECC_BCH16_NIBBLES      52

-static struct nand_ecclayout hw_bch8_nand_oob = GPMC_NAND_HW_BCH8_ECC_LAYOUT; +static struct nand_ecclayout nand_ecclayout = GPMC_NAND_HW_BCH8_ECC_LAYOUT;

Why the name change?  It's still the bch8 layout.

If your intent is for this to be the configuration knob, U-Boot is not configured by making edits to random source files. It needs to be a proper config symbol (and documented).

 static struct nand_bch_priv bch_priv = {
        .mode = NAND_ECC_HW_BCH,
@@ -280,21 +280,21 @@ static void omap_read_bch8_result(struct mtd_info *mtd, uint8_t big_endian,
        int8_t i = 0, j;

        if (big_endian) {
-               ptr = &gpmc_cfg->bch_result_0_3[0].bch_result_x[3];
+               ptr = &gpmc_cfg->bch_result_0_3.bch_result_x[3];
                ecc_code[i++] = readl(ptr) & 0xFF;
                ptr--;
                for (j = 0; j < 3; j++) {
                        ecc_code[i++] = (readl(ptr) >> 24) & 0xFF;
                        ecc_code[i++] = (readl(ptr) >> 16) & 0xFF;
-                       ecc_code[i++] = (readl(ptr) >>  8) & 0xFF;
+                       ecc_code[i++] = (readl(ptr) >>    8) & 0xFF;
                        ecc_code[i++] = readl(ptr) & 0xFF;
                        ptr--;
                }
        } else {
-               ptr = &gpmc_cfg->bch_result_0_3[0].bch_result_x[0];
+               ptr = &gpmc_cfg->bch_result_0_3.bch_result_x[0];
                for (j = 0; j < 3; j++) {
                        ecc_code[i++] = readl(ptr) & 0xFF;
-                       ecc_code[i++] = (readl(ptr) >>  8) & 0xFF;
+                       ecc_code[i++] = (readl(ptr) >>    8) & 0xFF;
                        ecc_code[i++] = (readl(ptr) >> 16) & 0xFF;
                        ecc_code[i++] = (readl(ptr) >> 24) & 0xFF;
                        ptr++;
@@ -304,6 +304,53 @@ static void omap_read_bch8_result(struct mtd_info *mtd, uint8_t big_endian,
        }
 }

+static void omap_read_bch16_result(struct mtd_info *mtd, uint8_t big_endian,
+                               uint8_t *ecc_code)
+{
+       uint32_t *ptr;
+       int8_t i = 0, j;
+       uint32_t data;
+
+       if(big_endian) {
+               ptr = &gpmc_cfg->bch_result_4_6.bch_result_x[2];
+
+               for (j = 0; j < 7; j++) {
+                       if(j == 3) {
+ ptr = &gpmc_cfg->bch_result_0_3.bch_result_x[3];
+                       }

Please put a space after "if".  Don't use braces around a single line.

+
+                       data = readl(ptr);
+                       ptr--;
+
+                       if(i > 0) {
+                               ecc_code[i++] = (data >> 24) & 0xFF;
+                               ecc_code[i++] = (data >> 16) & 0xFF;
+                       }
+                       ecc_code[i++] = (data >> 8) & 0xFF;
+                       ecc_code[i++] = data & 0xFF;
+               }
+               ecc_code[i++] = 0;
+               ecc_code[i++] = 0;
+       }
+       else {

} else {

+               ptr = &gpmc_cfg->bch_result_0_3.bch_result_x[0];
+
+               for (j = 0; j < 7; j++) {
+                       if(j == 4) {
+ ptr = &gpmc_cfg->bch_result_4_6.bch_result_x[0];
+                       }
+
+                       data = readl(ptr);
+                       ptr++;
+
+                       ecc_code[i++] = data & 0xFF;
+                       ecc_code[i++] = (data >> 8) & 0xFF;
+                       ecc_code[i++] = (data >> 16) & 0xFF;
+                       ecc_code[i++] = (data >> 24) & 0xFF;
+               }
+       }
+}
+
 /*
  * omap_ecc_disable - Disable H/W ECC calculation
  *
@@ -330,7 +377,7 @@ static void omap_rotate_ecc_bch(struct mtd_info *mtd, uint8_t *calc_ecc,
        struct nand_chip *chip = mtd->priv;
        struct nand_bch_priv *bch = chip->priv;
        uint8_t n_bytes = 0;
-       int8_t i, j;
+       int8_t i;

        switch (bch->type) {
        case ECC_BCH4:
@@ -338,7 +385,12 @@ static void omap_rotate_ecc_bch(struct mtd_info *mtd, uint8_t *calc_ecc,
                break;

        case ECC_BCH16:
-               n_bytes = 28;
+               n_bytes = 26;
+
+               /* Last 2 register of ELM need to be zero */

s/register/registers/

@@ -347,16 +399,17 @@ static void omap_rotate_ecc_bch(struct mtd_info *mtd, uint8_t *calc_ecc,
                break;
        }

-       for (i = 0, j = (n_bytes-1); i < n_bytes; i++, j--)
-               syndrome[i] =  calc_ecc[j];
+       for (i = 0; i < n_bytes; i++) {
+               syndrome[i] = calc_ecc[(n_bytes-1)-i];
+       }

Please put spaces around binary operators such as '-' (and again, no braces around single lines).

 void omap_nand_switch_ecc(int32_t hardware)
 {
+#ifndef CONFIG_AM33XX
        struct nand_chip *nand;
        struct mtd_info *mtd;

        if (nand_curr_device < 0 ||
-           nand_curr_device >= CONFIG_SYS_MAX_NAND_DEVICE ||
-           !nand_info[nand_curr_device].name) {
+               nand_curr_device >= CONFIG_SYS_MAX_NAND_DEVICE ||
+               !nand_info[nand_curr_device].name) {
printf("Error: Can't switch ecc, no devices available\n");
                return;
        }

Please never have the continuation line of a conditional lined up with the body of the construct.

@@ -646,19 +702,6 @@ void omap_nand_switch_ecc(int32_t hardware)
                nand->ecc.calculate = omap_calculate_ecc;
                omap_hwecc_init(nand);
                printf("HW ECC selected\n");
-#ifdef CONFIG_AM33XX
-       } else if (hardware == 2) {
-               nand->ecc.mode = NAND_ECC_HW;
-               nand->ecc.layout = &hw_bch8_nand_oob;
-               nand->ecc.size = 512;
-               nand->ecc.bytes = 14;
-               nand->ecc.read_page = omap_read_page_bch;
-               nand->ecc.hwctl = omap_enable_ecc_bch;
-               nand->ecc.correct = omap_correct_data_bch;
-               nand->ecc.calculate = omap_calculate_ecc_bch;
-               omap_hwecc_init_bch(nand, NAND_ECC_READ);
-               printf("HW BCH8 selected\n");
-#endif
        } else {

Explain.

diff --git a/include/linux/mtd/mtd-abi.h b/include/linux/mtd/mtd-abi.h
index 8bdd231..6979a2a 100644
--- a/include/linux/mtd/mtd-abi.h
+++ b/include/linux/mtd/mtd-abi.h
@@ -125,7 +125,7 @@ struct nand_oobfree {
  */
 struct nand_ecclayout {
        uint32_t eccbytes;
-       uint32_t eccpos[128];
+       uint32_t eccpos[208];
        uint32_t oobavail;
        struct nand_oobfree oobfree[MTD_MAX_OOBFREE_ENTRIES];
 };

Changes to generic code should ideally be separate patches.

-Scott
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to