From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Date: Tue, 29 Jan 2013 13:54:34 -0600 Subject: [U-Boot] [PATCH] am335x: NAND: add BCH16 and 4k page size support In-Reply-To: <1359380140-9842-1-git-send-email-jordyvanwolferen@gmail.com> (from jordyvanwolferen@gmail.com on Mon Jan 28 07:35:40 2013) Message-ID: <1359489274.32453.9@snotra> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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