From: Scott Wood <scottwood@freescale.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] am335x: NAND: add BCH16 and 4k page size support
Date: Tue, 29 Jan 2013 13:54:34 -0600 [thread overview]
Message-ID: <1359489274.32453.9@snotra> (raw)
In-Reply-To: <1359380140-9842-1-git-send-email-jordyvanwolferen@gmail.com> (from jordyvanwolferen@gmail.com on Mon Jan 28 07:35:40 2013)
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
next prev parent reply other threads:[~2013-01-29 19:54 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-28 13:35 [U-Boot] [PATCH] am335x: NAND: add BCH16 and 4k page size support Jordy van Wolferen
2013-01-29 19:54 ` Scott Wood [this message]
2013-01-29 20:01 ` Tom Rini
[not found] <1359376558-14035-1-git-send-email-jordyvanwolferen@gmail.com>
2013-01-28 12:55 ` [U-Boot] [PATCH] am335x: NAND, " Jordy van Wolferen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1359489274.32453.9@snotra \
--to=scottwood@freescale.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.