NAND support for AM33XX

These series add support for NAND on AM33XX. AM33XX has the same GPMC controller as OMAP3 so the first part of the series just add required defines/initialization to enable the existing omap_gpmc driver to work on AM33XX. The rest of the series adds support for BCH8 error correction code. We use GPMC to generate codes/syndromes and ELM to find the errors.

Changes in v2:
-fix nand mux settings (profiles 2&3 don't have NAND)
- rebased on current master
- clean up mem.c (remove unused stuff that was copied from OMAP3)
- nand headers: remove unneeded stuff
- rebased onto master
- minor config style fix (wrt nand)
- fix wrong braces in Makefile

Ilya Yanok (6):
OMAP: include sys_proto.h from boot-common
am335x_evm: add nand pinmux definition
am33xx: NAND support
am335x_evm: enable NAND support
am33xx_spl_bch: simple SPL nand loader for AM33XX
am335x_evm: enable SPL NAND support

Mansoor Ahamed (2):
am33xx: add ELM support
omap_gpmc: BCH8 support (ELM based)

 arch/arm/cpu/armv7/am33xx/Makefile           |    2 +
 arch/arm/cpu/armv7/am33xx/board.c            |    1 +
 arch/arm/cpu/armv7/am33xx/clock.c            |   10 +
 arch/arm/cpu/armv7/am33xx/elm.c              |  212 ++++++++++++++
 arch/arm/cpu/armv7/am33xx/mem.c              |  101 +++++++
 arch/arm/cpu/armv7/omap-common/boot-common.c |    1 +
 arch/arm/include/asm/arch-am33xx/cpu.h       |   53 ++++
 arch/arm/include/asm/arch-am33xx/elm.h       |   93 ++++++
 arch/arm/include/asm/arch-am33xx/hardware.h  |    3 +
 arch/arm/include/asm/arch-am33xx/mem.h       |   83 ++++++
 arch/arm/include/asm/arch-am33xx/omap_gpmc.h |  120 ++++++++
 arch/arm/include/asm/arch-am33xx/sys_proto.h |    3 +
 board/ti/am335x/board.c                      |    2 +
 board/ti/am335x/mux.c                        |   22 ++
 drivers/mtd/nand/Makefile                    |    1 +
 drivers/mtd/nand/am335x_spl_bch.c            |  238 +++++++++++++++
 drivers/mtd/nand/omap_gpmc.c                 |  403 +++++++++++++++++++++++++-
 include/configs/am335x_evm.h                 |   46 +++
 18 files changed, 1393 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/cpu/armv7/am33xx/elm.c
 create mode 100644 arch/arm/cpu/armv7/am33xx/mem.c
 create mode 100644 arch/arm/include/asm/arch-am33xx/elm.h
 create mode 100644 arch/arm/include/asm/arch-am33xx/mem.h
 create mode 100644 arch/arm/include/asm/arch-am33xx/omap_gpmc.h
 create mode 100644 drivers/mtd/nand/am335x_spl_bch.c

1.7.10.2 (Apple Git-33)

Include asm/arch/sys_proto.h for gpmc_init prototype. Without this we get a warning while building for AM335x.

---

 arch/arm/cpu/armv7/omap-common/boot-common.c |    1 +
 1 file changed, 1 insertion(+)

index 0f19141..2b584e0 100644
@@ -21,6 +21,7 @@
 #include <asm/omap_common.h>
 #include <asm/arch/omap.h>
 #include <asm/arch/mmc_host_def.h>
+#include <asm/arch/sys_proto.h>
 
 /*
  * This is used to verify if the configuration header

1.7.10.2 (Apple Git-33)

Add NAND pins mux settings for AM335X devices. Enable NAND pins for AM335X EVM board.

---
Changes in v2:
-fix nand mux settings (profiles 2&3 don't have NAND)

 board/ti/am335x/mux.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

index 8437ef5..0283708 100644
@@ -171,6 +171,25 @@ static struct module_pin_mux mii1_pin_mux[] = {
 	{-1},
 };
 
+static struct module_pin_mux nand_pin_mux[] = {
+	{OFFSET(gpmc_ad0), (MODE(0) | PULLUP_EN | RXACTIVE)},	/* NAND AD0 */
+	{OFFSET(gpmc_ad1), (MODE(0) | PULLUP_EN | RXACTIVE)},	/* NAND AD1 */
+	{OFFSET(gpmc_ad2), (MODE(0) | PULLUP_EN | RXACTIVE)},	/* NAND AD2 */
+	{OFFSET(gpmc_ad3), (MODE(0) | PULLUP_EN | RXACTIVE)},	/* NAND AD3 */
+	{OFFSET(gpmc_ad4), (MODE(0) | PULLUP_EN | RXACTIVE)},	/* NAND AD4 */
+	{OFFSET(gpmc_ad5), (MODE(0) | PULLUP_EN | RXACTIVE)},	/* NAND AD5 */
+	{OFFSET(gpmc_ad6), (MODE(0) | PULLUP_EN | RXACTIVE)},	/* NAND AD6 */
+	{OFFSET(gpmc_ad7), (MODE(0) | PULLUP_EN | RXACTIVE)},	/* NAND AD7 */
+	{OFFSET(gpmc_wait0), (MODE(0) | RXACTIVE | PULLUP_EN)}, /* NAND WAIT */
+	{OFFSET(gpmc_wpn), (MODE(7) | PULLUP_EN | RXACTIVE)},	/* NAND_WPN */
+	{OFFSET(gpmc_csn0), (MODE(0) | PULLUDEN)},	/* NAND_CS0 */
+	{OFFSET(gpmc_advn_ale), (MODE(0) | PULLUDEN)}, /* NAND_ADV_ALE */
+	{OFFSET(gpmc_oen_ren), (MODE(0) | PULLUDEN)},	/* NAND_OE */
+	{OFFSET(gpmc_wen), (MODE(0) | PULLUDEN)},	/* NAND_WEN */
+	{OFFSET(gpmc_be0n_cle), (MODE(0) | PULLUDEN)},	/* NAND_BE_CLE */
+	{-1},
+};
+
 void enable_uart0_pin_mux(void)
 {
 	configure_module_pin_mux(uart0_pin_mux);

@@ -257,6 +276,9 @@ void enable_board_pin_mux(struct am335x_baseboard_id *header)
 		/* In profile #2 i2c1 and spi0 conflict. */
 		if (profile & ~PROFILE_2)
 			configure_module_pin_mux(i2c1_pin_mux);
+		/* Profiles 2 & 3 don't have NAND */
+		if (profile & ~(PROFILE_2 | PROFILE_3))
+			configure_module_pin_mux(nand_pin_mux);
 		else if (profile == PROFILE_2) {
 			configure_module_pin_mux(mmc1_pin_mux);
 			configure_module_pin_mux(spi0_pin_mux);

1.7.10.2 (Apple Git-33)

TI AM33XX has the same GPMC controller as OMAP3 so we could just use the existing omap_gpmc driver. This patch adds adds required definitions/intialization.

---
Changes in v2:
- rebased on current master
- clean up mem.c (remove unused stuff that was copied from OMAP3)
- nand headers: remove unneeded stuff

 arch/arm/cpu/armv7/am33xx/Makefile           |    1 +
 arch/arm/cpu/armv7/am33xx/board.c            |    1 +
 arch/arm/cpu/armv7/am33xx/clock.c            |    5 ++
 arch/arm/cpu/armv7/am33xx/mem.c              |  101 ++++++++++++++++++++++
 arch/arm/include/asm/arch-am33xx/cpu.h       |   53 ++++++++++++
 arch/arm/include/asm/arch-am33xx/hardware.h  |    3 +
 arch/arm/include/asm/arch-am33xx/mem.h       |   83 ++++++++++++++++++
 arch/arm/include/asm/arch-am33xx/omap_gpmc.h |  120 ++++++++++++++++++++++++++
 arch/arm/include/asm/arch-am33xx/sys_proto.h |    3 +
 9 files changed, 370 insertions(+)
 create mode 100644 arch/arm/cpu/armv7/am33xx/mem.c
 create mode 100644 arch/arm/include/asm/arch-am33xx/mem.h
 create mode 100644 arch/arm/include/asm/arch-am33xx/omap_gpmc.h

index 74875b3..f565357 100644
@@ -18,6 +18,7 @@ LIB	= $(obj)lib$(SOC).o
 
 COBJS	+= clock.o
 COBJS	+= sys_info.o
+COBJS	+= mem.o
 COBJS	+= ddr.o
 COBJS	+= emif4.o
 COBJS	+= board.o

index e4c123c..c756c09 100644
@@ -25,6 +25,7 @@
 #include <asm/arch/ddr_defs.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/gpio.h>
+#include <asm/arch/mem.h>
 #include <asm/arch/mmc_host_def.h>
 #include <asm/arch/sys_proto.h>
 #include <asm/io.h>

index bc2abb6..6eb7d9f 100644
@@ -150,6 +150,11 @@ static void enable_per_clocks(void)
 		;
 #endif /* CONFIG_SERIAL6 */
 
+	/* GPMC */
+	writel(PRCM_MOD_EN, &cmper->gpmcclkctrl);
+	while (readl(&cmper->gpmcclkctrl) != PRCM_MOD_EN)
+		;
+
 	/* MMC0*/
 	writel(PRCM_MOD_EN, &cmper->mmc0clkctrl);
 	while (readl(&cmper->mmc0clkctrl) != PRCM_MOD_EN)

index 5bd4bc8..b3922d9 100644
@@ -80,6 +80,9 @@
 #define DDRPHY_0_CONFIG_BASE		(CTRL_BASE + 0x1400)
 #define DDRPHY_CONFIG_BASE		DDRPHY_0_CONFIG_BASE
 
+/* GPMC Base address */
+#define GPMC_BASE			0x50000000
+
 /* CPSW Config space */
 #define AM335X_CPSW_BASE		0x4A100000
 #define AM335X_CPSW_MDIO_BASE		0x4A101000

index 9cf35e0..588d8de 100644
@@ -33,4 +33,7 @@ u32 get_device_type(void);
 void setup_clocks_for_console(void);
 void ddr_pll_config(unsigned int ddrpll_M);
 
+void sdelay(unsigned long);
+void gpmc_init(void);
+void omap_nand_switch_ecc(int);
 #endif

1.7.10.2 (Apple Git-33)

Ilya> TI AM33XX has the same GPMC controller as OMAP3 so we could just use the Ilya> existing omap_gpmc driver. This patch adds adds required Ilya> definitions/intialization.

Ilya> Signed-off-by: Ilya Yanok <[email protected]>

..

Ilya> +void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
Ilya> + u32 size)
Ilya> +{
Ilya> + writel(0, &cs->config7);
Ilya> + sdelay(1000);
Ilya> + /* Delay for settling */

That comment should go above the delay.

Ilya> + writel(gpmc_config[0], &cs->config1);
Ilya> + writel(gpmc_config[1], &cs->config2);
Ilya> + writel(gpmc_config[2], &cs->config3);
Ilya> + writel(gpmc_config[3], &cs->config4);
Ilya> + writel(gpmc_config[4], &cs->config5);
Ilya> + writel(gpmc_config[5], &cs->config6);
Ilya> + /* Enable the config */
Ilya> + writel((((size & 0xF) << 8) | ((base >> 24) & 0x3F) |
Ilya> + (1 << 6)), &cs->config7);
Ilya> + sdelay(2000);

Any reason you now wait double as long?

Ilya> +}
Ilya> +
Ilya> +/***************************************************** Ilya> + * gpmc_init(): init gpmc bus
Ilya> + * Init GPMC for x16, MuxMode (SDRAM in x32). Ilya> + * This code can only be executed from SRAM or SDRAM. Ilya> + *****************************************************/ Ilya> +void gpmc_init(void)
Ilya> +{
Ilya> + /* putting a blanket check on GPMC based on ZeBu for now */
Ilya> + gpmc_cfg = (struct gpmc *)GPMC_BASE;
Ilya> +
Ilya> +#ifdef CONFIG_CMD_NAND
Ilya> + const u32 *gpmc_config = NULL;
Ilya> + u32 base = 0;
Ilya> + u32 size = 0;
Ilya> +#endif
Ilya> + /* global settings */
Ilya> + writel(0x00000008, &gpmc_cfg->sysconfig);
Ilya> + writel(0x00000100, &gpmc_cfg->irqstatus);
Ilya> + writel(0x00000200, &gpmc_cfg->irqenable);
Ilya> + writel(0x00000012, &gpmc_cfg->config);
Ilya> + /*
Ilya> + * Disable the GPMC0 config set by ROM code
Ilya> + */
Ilya> + writel(0, &gpmc_cfg->cs[0].config7);
Ilya> + sdelay(1000);

Why? You already do this in enable_gpmc_cs_config().

Ilya> +
Ilya> +#ifdef CONFIG_CMD_NAND
Ilya> + gpmc_config = gpmc_m_nand;
Ilya> +
Ilya> + base = PISMO1_NAND_BASE;
Ilya> + size = PISMO1_NAND_SIZE;
Ilya> + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[0], base, size);
Ilya> +#endif
Ilya> +}

Ilya> +++ b/arch/arm/include/asm/arch-am33xx/mem.h
Ilya> @@ -0,0 +1,83 @@
Ilya> +/*
Ilya> + * (C) Copyright 2006-2008
Ilya> + * Texas Instruments, <www.ti.com>
Ilya> + *
Ilya> + * Author
Ilya> + * Mansoor Ahamed <[email protected]>
Ilya> + *
Ilya> + * Initial Code from:
Ilya> + * Richard Woodruff <[email protected]>
Ilya> + *
Ilya> + * See file CREDITS for list of people who contributed to this Ilya> + * project.
Ilya> + *
Ilya> + * This program is free software; you can redistribute it and/or Ilya> + * modify it under the terms of the GNU General Public License as Ilya> + * published by the Free Software Foundation; either version 2 of Ilya> + * the License, or (at your option) any later version.
Ilya> + *
Ilya> + * This program is distributed in the hope that it will be useful, Ilya> + * but WITHOUT ANY WARRANTY; without even the implied warranty of Ilya> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the Ilya> + * GNU General Public License for more details.
Ilya> + *
Ilya> + * You should have received a copy of the GNU General Public License Ilya> + * along with this program; if not, write to the Free Software Ilya> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
Ilya> + * MA 02111-1307 USA
Ilya> + */
Ilya> +
Ilya> +#ifndef _MEM_H_
Ilya> +#define _MEM_H_
Ilya> +
Ilya> +/*
Ilya> + * GPMC settings -
Ilya> + * Definitions is as per the following format
Ilya> + * #define <PART>_GPMC_CONFIG<x> <value>
Ilya> + * Where:
Ilya> + * PART is the part name e.g. STNOR - Intel Strata Flash
Ilya> + * x is GPMC config registers from 1 to 6 (there will be 6 macros) Ilya> + * Value is corresponding value
Ilya> + *
Ilya> + * For every valid PRCM configuration there should be only one definition of Ilya> + * the same. if values are independent of the board, this definition will be Ilya> + * present in this file if values are dependent on the board, then this should Ilya> + * go into corresponding mem-boardName.h file
Ilya> + *
Ilya> + * Currently valid part Names are (PART):
Ilya> + * M_NAND - Micron NAND
Ilya> + */
Ilya> +#define GPMC_SIZE_256M 0x0
Ilya> +#define GPMC_SIZE_128M 0x8
Ilya> +#define GPMC_SIZE_64M 0xC
Ilya> +#define GPMC_SIZE_32M 0xE
Ilya> +#define GPMC_SIZE_16M 0xF
Ilya> +
Ilya> +#define M_NAND_GPMC_CONFIG1 0x00000800
Ilya> +#define M_NAND_GPMC_CONFIG2 0x001e1e00
Ilya> +#define M_NAND_GPMC_CONFIG3 0x001e1e00
Ilya> +#define M_NAND_GPMC_CONFIG4 0x16051807
Ilya> +#define M_NAND_GPMC_CONFIG5 0x00151e1e
Ilya> +#define M_NAND_GPMC_CONFIG6 0x16000f80
Ilya> +#define M_NAND_GPMC_CONFIG7 0x00000008

For what Micron part is this exactly? How about using the actual part number in the define like we recently did for the DDR configuration?

Ilya> +++ b/arch/arm/include/asm/arch-am33xx/omap_gpmc.h
Ilya> @@ -0,0 +1,120 @@
Ilya> +/*
Ilya> + * (C) Copyright 2004-2008 Texas Instruments, <www.ti.com> Ilya> + * Rohit Choraria <[email protected]>
Ilya> + *
Ilya> + * See file CREDITS for list of people who contributed to this Ilya> + * project.
Ilya> + *
Ilya> + * This program is free software; you can redistribute it and/or Ilya> + * modify it under the terms of the GNU General Public License as Ilya> + * published by the Free Software Foundation; either version 2 of Ilya> + * the License, or (at your option) any later version. Ilya> + *
Ilya> + * This program is distributed in the hope that it will be useful, Ilya> + * but WITHOUT ANY WARRANTY; without even the implied warranty of Ilya> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the Ilya> + * GNU General Public License for more details.
Ilya> + *
Ilya> + * You should have received a copy of the GNU General Public License Ilya> + * along with this program; if not, write to the Free Software Ilya> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, Ilya> + * MA 02111-1307 USA
Ilya> + */
Ilya> +#ifndef __ASM_ARCH_OMAP_GPMC_H
Ilya> +#define __ASM_ARCH_OMAP_GPMC_H
Ilya> +
Ilya> +#define GPMC_BUF_EMPTY 0
Ilya> +#define GPMC_BUF_FULL 1
Ilya> +
Ilya> +#define ECCCLEAR (0x1 << 8)
Ilya> +#define ECCRESULTREG1 (0x1 << 0)
Ilya> +#define ECCSIZE512BYTE 0xFF
Ilya> +#define ECCSIZE1 (ECCSIZE512BYTE << 22)
Ilya> +#define ECCSIZE0 (ECCSIZE512BYTE << 12)
Ilya> +#define ECCSIZE0SEL (0x000 << 0)
Ilya> +
Ilya> +/* Generic ECC Layouts */
Ilya> +/* Large Page x8 NAND device Layout */
Ilya> +#ifdef GPMC_NAND_ECC_LP_x8_LAYOUT
Ilya> +#define GPMC_NAND_HW_ECC_LAYOUT {\
Ilya> + .eccbytes = 12,\
Ilya> + .eccpos = {1, 2, 3, 4, 5, 6, 7, 8,\
Ilya> + 9, 10, 11, 12},\
Ilya> + .oobfree = {\
Ilya> + {.offset = 13,\
Ilya> + .length = 51 } } \
Ilya> +}
Ilya> +#endif
Ilya> +
Ilya> +/* Large Page x16 NAND device Layout */
Ilya> +#ifdef GPMC_NAND_ECC_LP_x16_LAYOUT
Ilya> +#define GPMC_NAND_HW_ECC_LAYOUT {\
Ilya> + .eccbytes = 12,\
Ilya> + .eccpos = {2, 3, 4, 5, 6, 7, 8, 9,\
Ilya> + 10, 11, 12, 13},\
Ilya> + .oobfree = {\
Ilya> + {.offset = 14,\
Ilya> + .length = 50 } } \
Ilya> +}
Ilya> +#endif
Ilya> +
Ilya> +/* Small Page x8 NAND device Layout */
Ilya> +#ifdef GPMC_NAND_ECC_SP_x8_LAYOUT
Ilya> +#define GPMC_NAND_HW_ECC_LAYOUT {\
Ilya> + .eccbytes = 3,\
Ilya> + .eccpos = {1, 2, 3},\
Ilya> + .oobfree = {\
Ilya> + {.offset = 4,\
Ilya> + .length = 12 } } \
Ilya> +}
Ilya> +#endif
Ilya> +
Ilya> +/* Small Page x16 NAND device Layout */
Ilya> +#ifdef GPMC_NAND_ECC_SP_x16_LAYOUT
Ilya> +#define GPMC_NAND_HW_ECC_LAYOUT {\
Ilya> + .eccbytes = 3,\
Ilya> + .eccpos = {2, 3, 4},\
Ilya> + .oobfree = {\
Ilya> + {.offset = 5,\
Ilya> + .length = 11 } } \
Ilya> +}
Ilya> +#endif
Ilya> +
Ilya> +#define GPMC_NAND_HW_BCH4_ECC_LAYOUT {\
Ilya> + .eccbytes = 32,\
Ilya> + .eccpos = {2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,\
Ilya> + 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27,\
Ilya> + 28, 29, 30, 31, 32, 33},\
Ilya> + .oobfree = {\
Ilya> + {.offset = 34,\
Ilya> + .length = 30 } } \
Ilya> +}
Ilya> +
Ilya> +#define GPMC_NAND_HW_BCH8_ECC_LAYOUT {\
Ilya> + .eccbytes = 56,\
Ilya> + .eccpos = {2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,\
Ilya> + 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27,\
Ilya> + 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39,\
Ilya> + 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51,\
Ilya> + 52, 53, 54, 55, 56, 57},\
Ilya> + .oobfree = {\
Ilya> + {.offset = 58,\
Ilya> + .length = 6 } } \
Ilya> +}
Ilya> +
Ilya> +#define GPMC_NAND_HW_BCH16_ECC_LAYOUT {\
Ilya> + .eccbytes = 104,\
Ilya> + .eccpos = {2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,\
Ilya> + 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27,\
Ilya> + 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39,\
Ilya> + 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51,\
Ilya> + 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,\
Ilya> + 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75,\
Ilya> + 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87,\
Ilya> + 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99,\
Ilya> + 100, 101, 102, 103, 104, 105},\
Ilya> + .oobfree = {\
Ilya> + {.offset = 106,\
Ilya> + .length = 8 } } \
Ilya> +}
Ilya> +#endif /* __ASM_ARCH_OMAP_GPMC_H */

Do the non-BCH layouts make sense for am33xx? I've noticed that the TRM shows the BCH8 format as 13 bytes/256 tightly packed instead of 14, but that's wrong. I'll report it to the doc people.

Bye, Peter Korsgaard

Hi Peter,

Ok, will fix.

No idea ;) To tell the truth I've taken all this code from omap3 as is.

Hm... Again, I'm not the one who written this code, I just stole it ;) but probably the idea was to disable the config even in case of CONFIG_CMD_NAND undefined...

Ok, I'll try to figure that out from schematics.

Do the non-BCH layouts make sense for am33xx? I've noticed that the TRM

Hm. Non-BCH layouts was here because I initially planned to support nandecc command to switch between supported ECC schemas... But as Tom requested to remove it I guess we don't need these layouts any more...
Well, my idea was to make separate patches (one to just support existing omap_gpmc driver on AM33xx and one to add BCH8 support) and this kinda needs non-BCH layouts too... but I guess I should just squash the patches into one.

Thanks for the review.

Regards, Ilya.

Hi,

Peter> Why? You already do this in enable_gpmc_cs_config().

Ilya> Hm... Again, I'm not the one who written this code, I just stole Ilya> it ;) but probably the idea was to disable the config even in Ilya> case of CONFIG_CMD_NAND undefined...

Why would you want to do that? I don't see a reason for touching the GPMC if you're not going to use it.

 Ilya>     ?Ilya> +#define M_NAND_GPMC_CONFIG1 ? ? 0x00000800
 Ilya>     ?Ilya> +#define M_NAND_GPMC_CONFIG2 ? ? 0x001e1e00
 Ilya>     ?Ilya> +#define M_NAND_GPMC_CONFIG3 ? ? 0x001e1e00
 Ilya>     ?Ilya> +#define M_NAND_GPMC_CONFIG4 ? ? 0x16051807
 Ilya>     ?Ilya> +#define M_NAND_GPMC_CONFIG5 ? ? 0x00151e1e
 Ilya>     ?Ilya> +#define M_NAND_GPMC_CONFIG6 ? ? 0x16000f80
 Ilya>     ?Ilya> +#define M_NAND_GPMC_CONFIG7 ? ? 0x00000008

Peter> For what Micron part is this exactly? How about using the actual part
Peter> number in the define like we recently did for the DDR configuration?

Ilya> Ok, I'll try to figure that out from schematics.

Great, thanks. The choice of GPMC configuration values should also be moved to board code, similar to how I did it for DDR.

 Peter>   Do the non-BCH layouts make sense for am33xx? I've noticed
 Peter>   that the TRM shows the BCH8 format as 13 bytes/256 tightly
 Peter>   packed instead of 14, but that's wrong. I'll report it to the
 Peter>   doc people.

Ilya> Hm. Non-BCH layouts was here because I initially planned to Ilya> support nandecc command to switch between supported ECC Ilya> schemas... But as Tom requested to remove it I guess we don't Ilya> need these layouts any more... Well, my idea was to make Ilya> separate patches (one to just support existing omap_gpmc driver Ilya> on AM33xx and one to add BCH8 support) and this kinda needs Ilya> non-BCH layouts too... but I guess I should just squash the Ilya> patches into one.

I don't think non-BCH makes much sense on am33xx considering the ROM uses/requires BCH8.

Ilya> Thanks for the review.

You're welcome.

Bye, Peter Korsgaard

Long term (not today tho) we need to support BCH16 as well since the ROM requires BCH16 on some parts and BCH8 on others. We can cross that bridge when we come to it, however.

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121121/981a5e28/attachment.pgp>

Enable NAND support for AM335X boards.

---
Changes in v2:
- rebased onto master
- minor config style fix (wrt nand)

 board/ti/am335x/board.c      |    2 ++
 include/configs/am335x_evm.h |   12 ++++++++++++
 2 files changed, 14 insertions(+)

index b56a801..6908378 100644
@@ -318,6 +318,8 @@ int board_init(void)
 
 	gd->bd->bi_boot_params = PHYS_DRAM_1 + 0x100;
 
+	gpmc_init();
+
 	return 0;
 }
 

index b6e48f8..ded1cab 100644
@@ -274,4 +274,16 @@
 #define CONFIG_PHYLIB
 #define CONFIG_PHY_SMSC
 
+#define CONFIG_NAND
+/* NAND support */
+#ifdef CONFIG_NAND
+#define CONFIG_CMD_NAND
+#define CONFIG_NAND_OMAP_GPMC
+#define GPMC_NAND_ECC_LP_x16_LAYOUT	1
+#define CONFIG_SYS_NAND_BASE		(0x08000000)	/* physical address */
+							/* to access nand at */
+							/* CS0 */
+#define CONFIG_SYS_MAX_NAND_DEVICE	1		/* Max number of NAND */
+#endif							/* devices */
+
 #endif	/* ! __CONFIG_AM335X_EVM_H */

1.7.10.2 (Apple Git-33)

From: Mansoor Ahamed <[email protected]>

AM33XX has Error Location Module (ELM) that can be used in conjuction with GPMC controller to implement BCH codes fully in hardware. This code is mostly taken from arago tree.

---
Changes in v2:
- fix wrong braces in Makefile

 arch/arm/cpu/armv7/am33xx/Makefile     |    1 +
 arch/arm/cpu/armv7/am33xx/clock.c      |    5 +
 arch/arm/cpu/armv7/am33xx/elm.c        |  212 ++++++++++++++++++++++++++++++++
 arch/arm/include/asm/arch-am33xx/elm.h |   93 ++++++++++++++
 4 files changed, 311 insertions(+)
 create mode 100644 arch/arm/cpu/armv7/am33xx/elm.c
 create mode 100644 arch/arm/include/asm/arch-am33xx/elm.h

index f565357..70c443e 100644
@@ -23,6 +23,7 @@ COBJS	+= ddr.o
 COBJS	+= emif4.o
 COBJS	+= board.o
 COBJS	+= mux.o
+COBJS-$(CONFIG_NAND_OMAP_GPMC)	+= elm.o
 
 SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
 OBJS	:= $(addprefix $(obj),$(COBJS) $(COBJS-y) $(SOBJS))

index 6eb7d9f..2b7c910 100644
@@ -155,6 +155,11 @@ static void enable_per_clocks(void)
 	while (readl(&cmper->gpmcclkctrl) != PRCM_MOD_EN)
 		;
 
+	/* ELM */
+	writel(PRCM_MOD_EN, &cmper->elmclkctrl);
+	while (readl(&cmper->elmclkctrl) != PRCM_MOD_EN)
+		;
+
 	/* MMC0*/
 	writel(PRCM_MOD_EN, &cmper->mmc0clkctrl);
 	while (readl(&cmper->mmc0clkctrl) != PRCM_MOD_EN)

1.7.10.2 (Apple Git-33)

From: Mansoor Ahamed <[email protected]>

This patch adds support for BCH8 error correction code to omap_gpmc driver. We use GPMC to generate codes/syndromes but we need ELM to find error locations from given syndrome.

---

 drivers/mtd/nand/omap_gpmc.c |  403 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 402 insertions(+), 1 deletion(-)

index f1469d1..cee394e 100644
@@ -29,6 +29,9 @@
 #include <linux/mtd/nand_ecc.h>
 #include <linux/compiler.h>
 #include <nand.h>
+#ifdef CONFIG_AM33XX
+#include <asm/arch/elm.h>
+#endif
 
 static uint8_t cs;
 static __maybe_unused struct nand_ecclayout hw_nand_oob =

@@ -234,6 +237,370 @@ static void __maybe_unused omap_enable_hwecc(struct mtd_info *mtd, int32_t mode)
 	}
 }
 
+/*
+ * BCH8 support (needs ELM and thus AM33xx-only)
+ */
+#ifdef CONFIG_AM33XX
+struct nand_bch_priv {
+	uint8_t mode;
+	uint8_t type;
+	uint8_t nibbles;
+};
+
+/* bch types */
+#define ECC_BCH4	0
+#define ECC_BCH8	1
+#define ECC_BCH16	2
+
+/* BCH nibbles for diff bch levels */
+#define NAND_ECC_HW_BCH ((uint8_t)(NAND_ECC_HW_OOB_FIRST) + 1)
+#define ECC_BCH4_NIBBLES	13
+#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_bch_priv bch_priv = {
+	.mode = NAND_ECC_HW_BCH,
+	.type = ECC_BCH8,
+	.nibbles = ECC_BCH8_NIBBLES
+};
+
+/*
+ * omap_read_bch8_result - Read BCH result for BCH8 level
+ *
+ * @mtd:	MTD device structure
+ * @big_endian:	When set read register 3 first
+ * @ecc_code:	Read syndrome from BCH result registers
+ */
+static void omap_read_bch8_result(struct mtd_info *mtd, uint8_t big_endian,
+				uint8_t *ecc_code)
+{
+	uint32_t *ptr;
+	int8_t i = 0, j;
+
+	if (big_endian) {
+		ptr = &gpmc_cfg->bch_result_0_3[0].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) & 0xFF;
+			ptr--;
+		}
+	} else {
+		ptr = &gpmc_cfg->bch_result_0_3[0].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) >> 16) & 0xFF;
+			ecc_code[i++] = (readl(ptr) >> 24) & 0xFF;
+			ptr++;
+		}
+		ecc_code[i++] = readl(ptr) & 0xFF;
+		ecc_code[i++] = 0;	/* 14th byte is always zero */
+	}
+}
+
+/*
+ * omap_ecc_disable - Disable H/W ECC calculation
+ *
+ * @mtd:	MTD device structure
+ *
+ */
+static void omap_ecc_disable(struct mtd_info *mtd)
+{
+	writel((readl(&gpmc_cfg->ecc_config) & ~0x1),
+		&gpmc_cfg->ecc_config);
+}
+
+/*
+ * omap_rotate_ecc_bch - Rotate the syndrome bytes
+ *
+ * @mtd:	MTD device structure
+ * @calc_ecc:	ECC read from ECC registers
+ * @syndrome:	Rotated syndrome will be retuned in this array
+ *
+ */
+static void omap_rotate_ecc_bch(struct mtd_info *mtd, uint8_t *calc_ecc,
+		uint8_t *syndrome)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct nand_bch_priv *bch = chip->priv;
+	uint8_t n_bytes = 0;
+	int8_t i, j;
+
+	switch (bch->type) {
+	case ECC_BCH4:
+		n_bytes = 8;
+		break;
+
+	case ECC_BCH16:
+		n_bytes = 28;
+		break;
+
+	case ECC_BCH8:
+	default:
+		n_bytes = 13;
+		break;
+	}
+
+	for (i = 0, j = (n_bytes-1); i < n_bytes; i++, j--)
+		syndrome[i] =  calc_ecc[j];
+}
+
+/*
+ *  omap_calculate_ecc_bch - Read BCH ECC result
+ *
+ *  @mtd:	MTD structure
+ *  @dat:	unused
+ *  @ecc_code:	ecc_code buffer
+ */
+static int omap_calculate_ecc_bch(struct mtd_info *mtd, const uint8_t *dat,
+				uint8_t *ecc_code)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct nand_bch_priv *bch = chip->priv;
+	uint8_t big_endian = 1;
+	int8_t ret = 0;
+
+	if (bch->type == ECC_BCH8)
+		omap_read_bch8_result(mtd, big_endian, ecc_code);
+	else /* BCH4 and BCH16 currently not supported */
+		ret = -1;
+
+	/*
+	 * Stop reading anymore ECC vals and clear old results
+	 * enable will be called if more reads are required
+	 */
+	omap_ecc_disable(mtd);
+
+	return ret;
+}
+
+/*
+ * omap_fix_errors_bch - Correct bch error in the data
+ *
+ * @mtd:	MTD device structure
+ * @data:	Data read from flash
+ * @error_count:Number of errors in data
+ * @error_loc:	Locations of errors in the data
+ *
+ */
+static void omap_fix_errors_bch(struct mtd_info *mtd, uint8_t *data,
+		uint32_t error_count, uint32_t *error_loc)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct nand_bch_priv *bch = chip->priv;
+	uint8_t count = 0;
+	uint32_t error_byte_pos;
+	uint32_t error_bit_mask;
+	uint32_t last_bit = (bch->nibbles * 4) - 1;
+
+	/* Flip all bits as specified by the error location array. */
+	/* FOR( each found error location flip the bit ) */
+	for (count = 0; count < error_count; count++) {
+		if (error_loc[count] > last_bit) {
+			/* Remove the ECC spare bits from correction. */
+			error_loc[count] -= (last_bit + 1);
+			/* Offset bit in data region */
+			error_byte_pos = ((512 * 8) -
+					(error_loc[count]) - 1) / 8;
+			/* Error Bit mask */
+			error_bit_mask = 0x1 << (error_loc[count] % 8);
+			/* Toggle the error bit to make the correction. */
+			data[error_byte_pos] ^= error_bit_mask;
+		}
+	}
+}
+
+/*
+ * omap_correct_data_bch - Compares the ecc read from nand spare area
+ * with ECC registers values and corrects one bit error if it has occured
+ *
+ * @mtd:	MTD device structure
+ * @dat:	page data
+ * @read_ecc:	ecc read from nand flash (ignored)
+ * @calc_ecc:	ecc read from ECC registers
+ *
+ * @return 0 if data is OK or corrected, else returns -1
+ */
+static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
+				uint8_t *read_ecc, uint8_t *calc_ecc)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct nand_bch_priv *bch = chip->priv;
+	uint8_t syndrome[28];
+	uint32_t error_count = 0;
+	uint32_t error_loc[8];
+	uint32_t i, ecc_flag;
+
+	ecc_flag = 0;
+	for (i = 0; i < chip->ecc.bytes; i++)
+		if (read_ecc[i] != 0xff)
+			ecc_flag = 1;
+
+	if (!ecc_flag)
+		return 0;
+
+	elm_reset();
+	elm_config((enum bch_level)(bch->type));
+
+	/*
+	 * while reading ECC result we read it in big endian.
+	 * Hence while loading to ELM we have rotate to get the right endian.
+	 */
+	omap_rotate_ecc_bch(mtd, calc_ecc, syndrome);
+
+	/* use elm module to check for errors */
+	if (elm_check_error(syndrome, bch->nibbles, &error_count,
+				error_loc) != 0) {
+		printf("ECC: uncorrectable.\n");
+		return -1;
+	}
+
+	/* correct bch error */
+	if (error_count > 0)
+		omap_fix_errors_bch(mtd, dat, error_count, error_loc);
+
+	return 0;
+}
+/*
+ * omap_hwecc_init_bch - Initialize the BCH Hardware ECC for NAND flash in
+ *				GPMC controller
+ * @mtd:       MTD device structure
+ * @mode:	Read/Write mode
+ */
+static void omap_hwecc_init_bch(struct nand_chip *chip, int32_t mode)
+{
+	uint32_t val, dev_width = (chip->options & NAND_BUSWIDTH_16) >> 1;
+	uint32_t unused_length = 0;
+	struct nand_bch_priv *bch = chip->priv;
+
+	switch (bch->nibbles) {
+	case ECC_BCH4_NIBBLES:
+		unused_length = 3;
+		break;
+	case ECC_BCH8_NIBBLES:
+		unused_length = 2;
+		break;
+	case ECC_BCH16_NIBBLES:
+		unused_length = 0;
+		break;
+	}
+
+	/* Clear the ecc result registers, select ecc reg as 1 */
+	writel(ECCCLEAR | ECCRESULTREG1, &gpmc_cfg->ecc_control);
+
+	switch (mode) {
+	case NAND_ECC_WRITE:
+		/* eccsize1 config */
+		val = ((unused_length + bch->nibbles) << 22);
+		break;
+
+	case NAND_ECC_READ:
+	default:
+		/* by default eccsize0 selected for ecc1resultsize */
+		/* eccsize0 config */
+		val  = (bch->nibbles << 12);
+		/* eccsize1 config */
+		val |= (unused_length << 22);
+		break;
+	}
+	/* ecc size configuration */
+	writel(val, &gpmc_cfg->ecc_size_config);
+	/* by default 512bytes sector page is selected */
+	/* set bch mode */
+	val  = (1 << 16);
+	/* bch4 / bch8 / bch16 */
+	val |= (bch->type << 12);
+	/* set wrap mode to 1 */
+	val |= (1 << 8);
+	val |= (dev_width << 7);
+	val |= (cs << 1);
+	writel(val, &gpmc_cfg->ecc_config);
+}
+
+/*
+ * omap_enable_ecc_bch- This function enables the bch h/w ecc functionality
+ * @mtd:        MTD device structure
+ * @mode:       Read/Write mode
+ *
+ */
+static void omap_enable_ecc_bch(struct mtd_info *mtd, int32_t mode)
+{
+	struct nand_chip *chip = mtd->priv;
+
+	omap_hwecc_init_bch(chip, mode);
+	/* enable ecc */
+	writel((readl(&gpmc_cfg->ecc_config) | 0x1), &gpmc_cfg->ecc_config);
+}
+
+/**
+ * omap_read_page_bch - hardware ecc based page read function
+ * @mtd:	mtd info structure
+ * @chip:	nand chip info structure
+ * @buf:	buffer to store read data
+ * @page:	page number to read
+ *
+ */
+static int omap_read_page_bch(struct mtd_info *mtd, struct nand_chip *chip,
+				uint8_t *buf, int page)
+{
+	int i, eccsize = chip->ecc.size;
+	int eccbytes = chip->ecc.bytes;
+	int eccsteps = chip->ecc.steps;
+	uint8_t *p = buf;
+	uint8_t *ecc_calc = chip->buffers->ecccalc;
+	uint8_t *ecc_code = chip->buffers->ecccode;
+	uint32_t *eccpos = chip->ecc.layout->eccpos;
+	uint8_t *oob = chip->oob_poi;
+	uint32_t data_pos;
+	uint32_t oob_pos;
+
+	data_pos = 0;
+	/* oob area start */
+	oob_pos = (eccsize * eccsteps) + chip->ecc.layout->eccpos[0];
+	oob += chip->ecc.layout->eccpos[0];
+
+	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize,
+				oob += eccbytes) {
+		chip->ecc.hwctl(mtd, NAND_ECC_READ);
+		/* read data */
+		chip->cmdfunc(mtd, NAND_CMD_RNDOUT, data_pos, page);
+		chip->read_buf(mtd, p, eccsize);
+
+		/* read respective ecc from oob area */
+		chip->cmdfunc(mtd, NAND_CMD_RNDOUT, oob_pos, page);
+		chip->read_buf(mtd, oob, eccbytes);
+		/* read syndrome */
+		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
+
+		data_pos += eccsize;
+		oob_pos += eccbytes;
+	}
+
+	for (i = 0; i < chip->ecc.total; i++)
+		ecc_code[i] = chip->oob_poi[eccpos[i]];
+
+	eccsteps = chip->ecc.steps;
+	p = buf;
+
+	for (i = 0 ; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
+		int stat;
+
+		stat = chip->ecc.correct(mtd, p, &ecc_code[i], &ecc_calc[i]);
+		if (stat < 0)
+			mtd->ecc_stats.failed++;
+		else
+			mtd->ecc_stats.corrected += stat;
+	}
+	return 0;
+}
+#endif /* CONFIG_AM33XX */
+
 #ifndef CONFIG_SPL_BUILD
 /*
  * omap_nand_switch_ecc - switch the ECC operation b/w h/w ecc and s/w ecc.

@@ -269,7 +636,7 @@ void omap_nand_switch_ecc(int32_t hardware)
 	nand->ecc.calculate = NULL;
 
 	/* Setup the ecc configurations again */
-	if (hardware) {
+	if (hardware == 1) {
 		nand->ecc.mode = NAND_ECC_HW;
 		nand->ecc.layout = &hw_nand_oob;
 		nand->ecc.size = 512;

@@ -279,6 +646,19 @@ 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 {
 		nand->ecc.mode = NAND_ECC_SOFT;
 		/* Use mtd default settings */

@@ -350,7 +730,27 @@ int board_nand_init(struct nand_chip *nand)
 		nand->options |= NAND_BUSWIDTH_16;
 
 	nand->chip_delay = 100;
+
+#ifdef CONFIG_AM33XX
+	/* required in case of BCH */
+	elm_init();
+
+	/* BCH info that will be correct for SPL or overridden otherwise. */
+	nand->priv = &bch_priv;
+#endif
+
 	/* Default ECC mode */
+#ifdef CONFIG_AM33XX
+	nand->ecc.mode = NAND_ECC_HW;
+	nand->ecc.layout = &hw_bch8_nand_oob;
+	nand->ecc.size = CONFIG_SYS_NAND_ECCSIZE;
+	nand->ecc.bytes = CONFIG_SYS_NAND_ECCBYTES;
+	nand->ecc.hwctl = omap_enable_ecc_bch;
+	nand->ecc.correct = omap_correct_data_bch;
+	nand->ecc.calculate = omap_calculate_ecc_bch;
+	nand->ecc.read_page = omap_read_page_bch;
+	omap_hwecc_init_bch(nand, NAND_ECC_READ);
+#else
 #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_NAND_SOFTECC)
 	nand->ecc.mode = NAND_ECC_SOFT;
 #else

@@ -363,6 +763,7 @@ int board_nand_init(struct nand_chip *nand)
 	nand->ecc.calculate = omap_calculate_ecc;
 	omap_hwecc_init(nand);
 #endif
+#endif
 
 #ifdef CONFIG_SPL_BUILD
 	if (nand->options & NAND_BUSWIDTH_16)

1.7.10.2 (Apple Git-33)

Dear Ilya Yanok,

first of all, I wonder why this is so different than the kernel implementation for BCH. I mean the API (and content) of this and commit 8d602cf50d3bba864bc1438f486b626df69c87b3 mainline linux seems to differ. The main question coming to mind is: Is the resulting OOB layout compatible then?

Best regards

Andreas Bie?mann

Dear Ilya Yanok,

You are right, ELM is not available in OMAP3 devices. It seems the ROM loader of these devices only support the 1-Bit Hamming, but is also different to the OOB layout used for the SW 1-bit hamming provided by the Kernel.
So we get here a lot of different OOB layouts ... I wonder if we can stick to e.g. the generic SW BCH layout (of linux kernel) for all but the ROM partition (where the SPL is placed). So the SPL need to know just one mechanism but software modifying that place needs to know about the 'special' ROM layout.

Sorry, what does RBL mean in that context?

Ok, that would be required to write the SPL to flash.

Well, but you have defined that for example it is written in big endian.

I'm currently working on a omap3 enabled device that requires 4-bit ECC for all but the first block. And I'm searching for a clean solution that would be accepted mainline.
I think it would be best to have the same OOB layout for the whole device but the SPL space (cause that needs to be read by ROM). The layout should be chosen at compile time of the SPL.
What do you think about?

Best regards

Andreas Bie?mann

Hi,

Andreas> You are right, ELM is not available in OMAP3 devices. It seems Andreas> the ROM loader of these devices only support the 1-Bit Andreas> Hamming, but is also different to the OOB layout used for the Andreas> SW 1-bit hamming provided by the Kernel. So we get here a lot Andreas> of different OOB layouts ... I wonder if we can stick to Andreas> e.g. the generic SW BCH layout (of linux kernel) for all but Andreas> the ROM partition (where the SPL is placed). So the SPL need Andreas> to know just one mechanism but software modifying that place Andreas> needs to know about the 'special' ROM layout.

No, please not. Having more than 1 OOB layout on the same NAND device leads to all kind of complications. There has also been kernel patches posted for the ELM, so IMHO the only sane option for am33xx is BCH8 everywhere (with the ROM layout).

Andreas> Sorry, what does RBL mean in that context?

The ROM boot loader, E.G. the part loading the spl.

Andreas> Well, but you have defined that for example it is written in Andreas> big endian.

Andreas> I'm currently working on a omap3 enabled device that requires Andreas> 4-bit ECC for all but the first block. And I'm searching for a Andreas> clean solution that would be accepted mainline. I think it Andreas> would be best to have the same OOB layout for the whole device Andreas> but the SPL space (cause that needs to be read by ROM). The Andreas> layout should be chosen at compile time of the SPL. What do Andreas> you think about?

So the only reason to not have the same OOB layout everywhere is because of ROM restrictions and that 1bit ECC isn't good enough anymore? E.G. you actually would have prefered to use the ROM layout if it would have used something better like the am33xx ROM does.

Bye, Peter Korsgaard

Dear Peter Korsgaard,

Ok, I understand your point here. But there are device combinations out there that do not match!
We have an AM37xx with some micron NAND that requires 1bit ECC for the first page (if less than 1000 erase cycle). But 4bit ECC for the rest of the device. With the 1bit for first page it does fit to the ROM requirements of that SoC but unfortunately we can not use the same OOB layout on the whole device, cause the ROM can only do the 1bit hamming ECC. I think there are other boards out there facing the same problem. I have to recheck for example the devkit8000 which is a mainlined development device some users based their products on (as we do).

That's my point.

You are right.

Best regards

Andreas Bie?mann

Hi,

Andreas> Ok, I understand your point here. But there are device Andreas> combinations out there that do not match! We have an AM37xx Andreas> with some micron NAND that requires 1bit ECC for the first Andreas> page (if less than 1000 erase cycle). But 4bit ECC for the Andreas> rest of the device. With the 1bit for first page it does fit Andreas> to the ROM requirements of that SoC but unfortunately we can Andreas> not use the same OOB layout on the whole device, cause the ROM Andreas> can only do the 1bit hamming ECC. I think there are other Andreas> boards out there facing the same problem. I have to recheck Andreas> for example the devkit8000 which is a mainlined development Andreas> device some users based their products on (as we do).

Sure, reality tends to be more complicated than that. The different SoCs, Nand devices and projects have different requirements. Notice that this patch is specifically about the Error Location Module present on E.G. am33xx and omap4, where the ROM also uses BCH encoding.

Bye, Peter Korsgaard

I think this has been mostly addressed now, but for clarity:
- We do NOT want to have > 1 layout used per NAND chip unless we must (historically we did because we had ROM that couldn't use >1bit ECC).
- We DO want to utilize the HW as this is the only easy way to get a match with the BCH constants the ROM uses.
- There are corresponding kernel patches already posted and working their way along.

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121121/2929d2b4/attachment.pgp>

I'm fine with all these three points. My question came up when I looked into this deeply the very first time cause BCH4/8 support was missing for OMAP35xx/AM37xx devices.
I do have now working support for these (hw assisted BCH but sw correction) like the kernel does. It needs some final polishing however I will send it these days as RFC, would be great to get some feedback.

Best regards

Andreas Bie?mann

AM33XX with BCH8 can't work with nand_spl_simple correctly because custom read_page implementation is required for proper syndrome generation.

This simple driver mostly duplicates nand_spl_simple but has nand_read_page changed to suit our needs.

---

 drivers/mtd/nand/Makefile         |    1 +
 drivers/mtd/nand/am335x_spl_bch.c |  238 +++++++++++++++++++++++++++++++++++++
 2 files changed, 239 insertions(+)
 create mode 100644 drivers/mtd/nand/am335x_spl_bch.c

index beb99ca..5322f3a 100644
@@ -30,6 +30,7 @@ ifdef CONFIG_SPL_BUILD
 ifdef CONFIG_SPL_NAND_SIMPLE
 COBJS-y += nand_spl_simple.o
 endif
+COBJS-$(CONFIG_SPL_NAND_AM33XX_BCH) += am335x_spl_bch.o
 ifdef CONFIG_SPL_NAND_LOAD
 COBJS-y	+= nand_spl_load.o
 endif

1.7.10.2 (Apple Git-33)

Enable booting from NAND support from AM335x boards as well as environment in NAND.

---

 include/configs/am335x_evm.h |   38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

index ded1cab..6abe544 100644
@@ -240,6 +240,35 @@
 #define CONFIG_SYS_SPI_U_BOOT_SIZE	0x40000
 #define CONFIG_SPL_LDSCRIPT		"$(CPUDIR)/omap-common/u-boot-spl.lds"
 
+#define CONFIG_SPL_BOARD_INIT
+#define CONFIG_SPL_NAND_AM33XX_BCH
+#define CONFIG_SPL_NAND_SUPPORT
+#define CONFIG_SYS_NAND_5_ADDR_CYCLE
+#define CONFIG_SYS_NAND_PAGE_COUNT	(CONFIG_SYS_NAND_BLOCK_SIZE / \
+					 CONFIG_SYS_NAND_PAGE_SIZE)
+#define CONFIG_SYS_NAND_PAGE_SIZE	2048
+#define CONFIG_SYS_NAND_OOBSIZE		64
+#define CONFIG_SYS_NAND_BLOCK_SIZE	(128*1024)
+#define CONFIG_SYS_NAND_BAD_BLOCK_POS	NAND_LARGE_BADBLOCK_POS
+#define CONFIG_SYS_NAND_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, }
+
+#define CONFIG_SYS_NAND_ECCSIZE		512
+#define CONFIG_SYS_NAND_ECCBYTES	14
+
+#define CONFIG_SYS_NAND_ECCSTEPS	4
+#define	CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES * \
+						CONFIG_SYS_NAND_ECCSTEPS)
+
+#define	CONFIG_SYS_NAND_U_BOOT_START	CONFIG_SYS_TEXT_BASE
+
+#define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
+
 /*
  * 1MB into the SDRAM to allow for SPL's bss at the beginning of SDRAM
  * 64 bytes before this address should be set aside for u-boot.img's

@@ -283,7 +312,12 @@
 #define CONFIG_SYS_NAND_BASE		(0x08000000)	/* physical address */
 							/* to access nand at */
 							/* CS0 */
-#define CONFIG_SYS_MAX_NAND_DEVICE	1		/* Max number of NAND */
-#endif							/* devices */
+#define CONFIG_SYS_MAX_NAND_DEVICE	1		/* Max number of NAND
+							   devices */
+#undef CONFIG_ENV_IS_NOWHERE
+#define CONFIG_ENV_IS_IN_NAND
+#define CONFIG_ENV_OFFSET		0x260000 /* environment starts here */
+#define CONFIG_SYS_ENV_SECT_SIZE	(128 << 10)	/* 128 KiB */
+#endif
 
 #endif	/* ! __CONFIG_AM335X_EVM_H */

1.7.10.2 (Apple Git-33)

For the series, applied to u-boot-ti/master, thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121210/45f4f960/attachment.pgp>