[U-Boot] [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib common files, add NAND support

[U-Boot] [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib common files, add NAND support

[dirk.behme at googlemail.com]

Subject: [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib common files, add NAND support

From: Dirk Behme <dirk.behme@gmail.com>

Add memory and syslib common files, add NAND support

Signed-off-by: Dirk Behme <dirk.behme@gmail.com>

---
Changes in version v2:

- Move common ARM Cortex A8 code to cpu/arm_cortexa8/ and OMAP3 SoC specific common code to cpu/arm_cortexa8/omap3 as proposed by Wolfgang.

 common/cmd_nand.c               |   29 ++
 cpu/arm_cortexa8/omap3/Makefile |    2 
 cpu/arm_cortexa8/omap3/mem.c    |  301 ++++++++++++++++++++++++++++++
 cpu/arm_cortexa8/omap3/nand.c   |  399 ++++++++++++++++++++++++++++++++++++++++
 cpu/arm_cortexa8/omap3/syslib.c |   72 +++++++
 drivers/mtd/nand/nand_base.c    |  145 ++++++++++++++
 examples/Makefile               |    6 
 7 files changed, 951 insertions(+), 3 deletions(-)

Index: u-boot-arm/cpu/arm_cortexa8/omap3/mem.c
===================================================================
--- /dev/null
+++ u-boot-arm/cpu/arm_cortexa8/omap3/mem.c
@@ -0,0 +1,301 @@
+/*
+ * (C) Copyright 2008
+ * Texas Instruments, <www.ti.com>
+ *
+ * Author :
+ *     Manikandan Pillai <mani.pillai@ti.com>
+ *
+ * Initial Code from:
+ *     Richard Woodruff <r-woodruff2@ti.com>
+ *     Syed Mohammed Khasim <khasim@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/bits.h>
+#include <asm/arch/mem.h>
+#include <asm/arch/sys_proto.h>
+#include <command.h>
+
+/* Only One NAND allowed on board at a time.
+ * The GPMC CS Base for the same
+ */
+unsigned int boot_flash_base;
+unsigned int boot_flash_off;
+unsigned int boot_flash_sec;
+unsigned int boot_flash_type;
+volatile unsigned int boot_flash_env_addr;
+
+/* help common/env_flash.c */
+#ifdef ENV_IS_VARIABLE
+
+uchar(*boot_env_get_char_spec) (int index);
+int (*boot_env_init) (void);
+int (*boot_saveenv) (void);
+void (*boot_env_relocate_spec) (void);
+
+/* 16 bit NAND */
+uchar env_get_char_spec(int index);
+int env_init(void);
+int saveenv(void);
+void env_relocate_spec(void);
+extern char *env_name_spec;
+
+#if defined(CONFIG_CMD_NAND)
+u8 is_nand;
+#endif
+
+#if defined(CONFIG_CMD_ONENAND)
+u8 is_onenand;
+#endif
+
+#endif /* ENV_IS_VARIABLE */
+
+#if defined(CONFIG_CMD_NAND)
+static u32 gpmc_m_nand[GPMC_MAX_REG] = {
+	M_NAND_GPMC_CONFIG1,
+	M_NAND_GPMC_CONFIG2,
+	M_NAND_GPMC_CONFIG3,
+	M_NAND_GPMC_CONFIG4,
+	M_NAND_GPMC_CONFIG5,
+	M_NAND_GPMC_CONFIG6, 0
+};
+unsigned int nand_cs_base;
+#endif
+
+#if defined(CONFIG_CMD_ONENAND)
+static u32 gpmc_onenand[GPMC_MAX_REG] = {
+	ONENAND_GPMC_CONFIG1,
+	ONENAND_GPMC_CONFIG2,
+	ONENAND_GPMC_CONFIG3,
+	ONENAND_GPMC_CONFIG4,
+	ONENAND_GPMC_CONFIG5,
+	ONENAND_GPMC_CONFIG6, 0
+};
+unsigned int onenand_cs_base;
+
+#endif
+
+/**************************************************************************
+ * make_cs1_contiguous() - for es2 and above remap cs1 behind cs0 to allow
+ *  command line mem=xyz use all memory with out discontinuous support
+ *  compiled in.  Could do it at the ATAG, but there really is two banks...
+ * Called as part of 2nd phase DDR init.
+ **************************************************************************/
+void make_cs1_contiguous(void)
+{
+	u32 size, a_add_low, a_add_high;
+
+	size = get_sdr_cs_size(SDRC_CS0_OSET);
+	size /= SZ_32M;		        /* find size to offset CS1 */
+	a_add_high = (size & 3) << 8;	/* set up low field */
+	a_add_low = (size & 0x3C) >> 2;	/* set up high field */
+	__raw_writel((a_add_high | a_add_low), SDRC_CS_CFG);
+
+}
+
+/********************************************************
+ *  mem_ok() - test used to see if timings are correct
+ *             for a part. Helps in guessing which part
+ *             we are currently using.
+ *******************************************************/
+u32 mem_ok(void)
+{
+	u32 val1, val2, addr;
+	u32 pattern = 0x12345678;
+
+	addr = OMAP34XX_SDRC_CS0;
+
+	__raw_writel(0x0, addr + 0x400);  /* clear pos A */
+	__raw_writel(pattern, addr);	  /* pattern to pos B */
+	__raw_writel(0x0, addr + 4);	  /* remove pattern off the bus */
+	val1 = __raw_readl(addr + 0x400); /* get pos A value */
+	val2 = __raw_readl(addr);	  /* get val2 */
+
+	if ((val1 != 0) || (val2 != pattern))  /* see if pos A value changed */
+		return 0;
+	else
+		return 1;
+}
+
+/********************************************************
+ *  sdrc_init() - init the sdrc chip selects CS0 and CS1
+ *  - early init routines, called from flash or
+ *  SRAM.
+ *******************************************************/
+void sdrc_init(void)
+{
+	/* only init up first bank here */
+	do_sdrc_init(SDRC_CS0_OSET, EARLY_INIT);
+}
+
+/*************************************************************************
+ * do_sdrc_init(): initialize the SDRAM for use.
+ *  -code sets up SDRAM basic SDRC timings for CS0
+ *  -optimal settings can be placed here, or redone after i2c
+ *      inspection of board info
+ *
+ *  - code called ones in C-Stack only context for CS0 and a possible 2nd
+ *      time depending on memory configuration from stack+global context
+ **************************************************************************/
+
+void do_sdrc_init(u32 offset, u32 early)
+{
+
+	/* reset sdrc controller */
+	__raw_writel(SOFTRESET, SDRC_SYSCONFIG);
+	wait_on_value(BIT0, BIT0, SDRC_STATUS, 12000000);
+	__raw_writel(0, SDRC_SYSCONFIG);
+
+	/* setup sdrc to ball mux */
+	__raw_writel(SDP_SDRC_SHARING, SDRC_SHARING);
+
+	/* SDRC_MCFG0 register */
+	(*(unsigned int *) 0x6D000080) = 0x02584099; /* from Micron */
+
+	/* SDRC_RFR_CTRL0 register */
+	(*(unsigned int *) 0x6D0000a4) = 0x54601;    /* for 166M */
+
+	/* SDRC_ACTIM_CTRLA0 register */
+	(*(unsigned int *) 0x6D00009c) = 0xa29db4c6; /* for 166M */
+
+	/* SDRC_ACTIM_CTRLB0 register */
+	(*(unsigned int *) 0x6D0000a0) = 0x12214;    /* for 166M */
+
+	/* Disble Power Down of CKE cuz of 1 CKE on combo part */
+	(*(unsigned int *) 0x6D000070) = 0x00000081;
+
+	/* SDRC_Manual command register */
+	(*(unsigned int *) 0x6D0000a8) = 0x00000000; /* NOP command */
+	(*(unsigned int *) 0x6D0000a8) = 0x00000001; /* Precharge command */
+	(*(unsigned int *) 0x6D0000a8) = 0x00000002; /* Auto-refresh command */
+	(*(unsigned int *) 0x6D0000a8) = 0x00000002; /* Auto-refresh command */
+
+	/* SDRC MR0 register */
+	(*(int *) 0x6D000084) = 0x00000032;	/*  Burst length = 4 */
+	/* CAS latency = 3, Write Burst = Read Burst Serial Mode */
+
+	/* SDRC DLLA control register */
+	(*(unsigned int *) 0x6D000060) = 0x0000A;
+	sdelay(0x20000);
+}
+
+void enable_gpmc_config(u32 *gpmc_config, u32 gpmc_base, u32 base, u32 size)
+{
+	__raw_writel(0, GPMC_CONFIG7 + gpmc_base);
+	sdelay(1000);
+	/* Delay for settling */
+	__raw_writel(gpmc_config[0], GPMC_CONFIG1 + gpmc_base);
+	__raw_writel(gpmc_config[1], GPMC_CONFIG2 + gpmc_base);
+	__raw_writel(gpmc_config[2], GPMC_CONFIG3 + gpmc_base);
+	__raw_writel(gpmc_config[3], GPMC_CONFIG4 + gpmc_base);
+	__raw_writel(gpmc_config[4], GPMC_CONFIG5 + gpmc_base);
+	__raw_writel(gpmc_config[5], GPMC_CONFIG6 + gpmc_base);
+	/* Enable the config */
+	__raw_writel((((size & 0xF) << 8) | ((base >> 24) & 0x3F) |
+		      (1 << 6)), GPMC_CONFIG7 + gpmc_base);
+	sdelay(2000);
+}
+
+/*****************************************************
+ * gpmc_init(): init gpmc bus
+ * Init GPMC for x16, MuxMode (SDRAM in x32).
+ * This code can only be executed from SRAM or SDRAM.
+ *****************************************************/
+void gpmc_init(void)
+{
+	/* putting a blanket check on GPMC based on ZeBu for now */
+	u32 mux = 0, mwidth;
+	u32 *gpmc_config = NULL;
+	u32 gpmc_base = 0;
+	u32 base = 0;
+	u32 size = 0;
+	u32 f_off = CONFIG_SYS_MONITOR_LEN;
+	u32 f_sec = 0;
+	u32 config = 0;
+
+	mux = BIT9;
+	mwidth = get_gpmc0_width();
+
+	/* global settings */
+	__raw_writel(0x0, GPMC_IRQENABLE);	/* isr's sources masked */
+	__raw_writel(0, GPMC_TIMEOUT_CONTROL);	/* timeout disable */
+
+	config = __raw_readl(GPMC_CONFIG);
+	config &= (~0xf00);
+	__raw_writel(config, GPMC_CONFIG);
+
+	/* Disable the GPMC0 config set by ROM code
+	 * It conflicts with our MPDB (both@0x08000000)
+	 */
+	__raw_writel(0, GPMC_CONFIG7 + GPMC_CONFIG_CS0);
+	sdelay(1000);
+
+#if defined(CONFIG_CMD_NAND)    /* CS 0 */
+	gpmc_config = gpmc_m_nand;
+#if defined(CONFIG_ENV_IS_IN_NAND)
+	gpmc_base = GPMC_CONFIG_CS0 + (0 * GPMC_CONFIG_WIDTH);
+#else
+	gpmc_base = GPMC_CONFIG_CS0 + (1 * GPMC_CONFIG_WIDTH);
+#endif
+	base = PISMO1_NAND_BASE;
+	size = PISMO1_NAND_SIZE;
+	enable_gpmc_config(gpmc_config, gpmc_base, base, size);
+	is_nand = 1;
+	nand_cs_base = gpmc_base;
+#if defined(CONFIG_ENV_IS_IN_NAND)
+	f_off = SMNAND_ENV_OFFSET;
+	f_sec = SZ_128K;
+	/* env setup */
+	boot_flash_base = base;
+	boot_flash_off = f_off;
+	boot_flash_sec = f_sec;
+	boot_flash_env_addr = f_off;
+#endif
+#endif
+
+#if defined(CONFIG_CMD_ONENAND)
+	gpmc_config = gpmc_onenand;
+#if defined(CONFIG_ENV_IS_IN_ONENAND)
+	gpmc_base = GPMC_CONFIG_CS0 + (0 * GPMC_CONFIG_WIDTH);
+#else
+	gpmc_base = GPMC_CONFIG_CS0 + (1 * GPMC_CONFIG_WIDTH);
+#endif
+	base = PISMO1_ONEN_BASE;
+	size = PISMO1_ONEN_SIZE;
+	enable_gpmc_config(gpmc_config, gpmc_base, base, size);
+	is_onenand = 1;
+	onenand_cs_base = gpmc_base;
+#if defined(CONFIG_ENV_IS_IN_ONENAND)
+	f_off = ONENAND_ENV_OFFSET;
+	f_sec = SZ_128K;
+	/* env setup */
+	boot_flash_base = base;
+	boot_flash_off = f_off;
+	boot_flash_sec = f_sec;
+	boot_flash_env_addr = f_off;
+#endif
+#endif
+
+#ifdef ENV_IS_VARIABLE
+	boot_env_get_char_spec = env_get_char_spec;
+	boot_env_init = env_init;
+	boot_saveenv = saveenv;
+	boot_env_relocate_spec = env_relocate_spec;
+#endif
+}
Index: u-boot-arm/cpu/arm_cortexa8/omap3/syslib.c
===================================================================
--- /dev/null
+++ u-boot-arm/cpu/arm_cortexa8/omap3/syslib.c
@@ -0,0 +1,72 @@
+/*
+ * (C) Copyright 2008
+ * Texas Instruments, <www.ti.com>
+ *
+ * Richard Woodruff <r-woodruff2@ti.com>
+ * Syed Mohammed Khasim <khasim@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/bits.h>
+#include <asm/arch/mem.h>
+#include <asm/arch/clocks.h>
+#include <asm/arch/sys_proto.h>
+
+/************************************************************
+ * sdelay() - simple spin loop.  Will be constant time as
+ *  its generally used in bypass conditions only.  This
+ *  is necessary until timers are accessible.
+ *
+ *  not inline to increase chances its in cache when called
+ *************************************************************/
+void sdelay(unsigned long loops)
+{
+	__asm__ volatile ("1:\n" "subs %0, %1, #1\n"
+			  "bne 1b":"=r" (loops):"0"(loops));
+}
+
+/*****************************************************************
+ * sr32 - clear & set a value in a bit range for a 32 bit address
+ *****************************************************************/
+void sr32(u32 addr, u32 start_bit, u32 num_bits, u32 value)
+{
+	u32 tmp, msk = 0;
+	msk = 1 << num_bits;
+	--msk;
+	tmp = __raw_readl(addr) & ~(msk << start_bit);
+	tmp |= value << start_bit;
+	__raw_writel(tmp, addr);
+}
+
+/*********************************************************************
+ * wait_on_value() - common routine to allow waiting for changes in
+ *   volatile regs.
+ *********************************************************************/
+u32 wait_on_value(u32 read_bit_mask, u32 match_value, u32 read_addr, u32 bound)
+{
+	u32 i = 0, val;
+	do {
+		++i;
+		val = __raw_readl(read_addr) & read_bit_mask;
+		if (val == match_value)
+			return 1;
+		if (i == bound)
+			return 0;
+	} while (1);
+}
Index: u-boot-arm/cpu/arm_cortexa8/omap3/nand.c
===================================================================
--- /dev/null
+++ u-boot-arm/cpu/arm_cortexa8/omap3/nand.c
@@ -0,0 +1,399 @@
+/*
+ * (C) Copyright 2004-2008 Texas Instruments, <www.ti.com>
+ * Rohit Choraria <rohitkc@ti.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/mem.h>
+#include <linux/mtd/nand_ecc.h>
+
+#if defined(CONFIG_CMD_NAND)
+
+#include <nand.h>
+
+unsigned char cs;
+volatile unsigned long gpmc_cs_base_add;
+
+#define GPMC_BUF_EMPTY 0
+#define GPMC_BUF_FULL 1
+
+#define ECC_P1_128_E(val)    ((val)  & 0x000000FF)	/* Bit 0 to 7 */
+#define ECC_P512_2048_E(val) (((val) & 0x00000F00)>>8)	/* Bit 8 to 11 */
+#define ECC_P1_128_O(val)    (((val) & 0x00FF0000)>>16)	/* Bit 16 to Bit 23 */
+#define ECC_P512_2048_O(val) (((val) & 0x0F000000)>>24)	/* Bit 24 to Bit 27 */
+
+/*
+ * omap_nand_hwcontrol - Set the address pointers corretly for the
+ *			following address/data/command operation
+ */
+static void omap_nand_hwcontrol(struct mtd_info *mtd, int cmd,
+				unsigned int ctrl)
+{
+	register struct nand_chip *this = mtd->priv;
+
+	/* Point the IO_ADDR to DATA and ADDRESS registers instead
+	   of chip address */
+	switch (ctrl) {
+	case NAND_CTRL_CHANGE | NAND_CTRL_CLE:
+		this->IO_ADDR_W = (void *) gpmc_cs_base_add + GPMC_NAND_CMD;
+		this->IO_ADDR_R = (void *) gpmc_cs_base_add + GPMC_NAND_DAT;
+		break;
+	case NAND_CTRL_CHANGE | NAND_CTRL_ALE:
+		this->IO_ADDR_W = (void *) gpmc_cs_base_add + GPMC_NAND_ADR;
+		this->IO_ADDR_R = (void *) gpmc_cs_base_add + GPMC_NAND_DAT;
+		break;
+	case NAND_CTRL_CHANGE | NAND_NCE:
+		this->IO_ADDR_W = (void *) gpmc_cs_base_add + GPMC_NAND_DAT;
+		this->IO_ADDR_R = (void *) gpmc_cs_base_add + GPMC_NAND_DAT;
+		break;
+	}
+
+	if (cmd != NAND_CMD_NONE)
+		writeb(cmd, this->IO_ADDR_W);
+}
+
+/*
+ * omap_nand_wait - called primarily after a program/erase operation
+ *			so that we access NAND again only after the device
+ *			is ready again.
+ * @mtd:        MTD device structure
+ * @chip:	nand_chip structure
+ */
+static int omap_nand_wait(struct mtd_info *mtd, struct nand_chip *chip)
+{
+	register struct nand_chip *this = mtd->priv;
+	int status = 0;
+
+	this->IO_ADDR_W = (void *) gpmc_cs_base_add + GPMC_NAND_CMD;
+	this->IO_ADDR_R = (void *) gpmc_cs_base_add + GPMC_NAND_DAT;
+	/* Send the status command and loop until the device is free */
+	while (!(status & 0x40)) {
+		__raw_writeb(NAND_CMD_STATUS & 0xFF, this->IO_ADDR_W);
+		status = __raw_readb(this->IO_ADDR_R);
+	}
+	return status;
+}
+
+#ifdef CONFIG_SYS_NAND_WIDTH_16
+/*
+ * omap_nand_write_buf16 - [DEFAULT] write buffer to chip
+ * @mtd:	MTD device structure
+ * @buf:	data buffer
+ * @len:	number of bytes to write
+ *
+ * Default write function for 16bit buswith
+ */
+static void omap_nand_write_buf(struct mtd_info *mtd, const u_char *buf,
+				int len)
+{
+	int i;
+	struct nand_chip *this = mtd->priv;
+	u16 *p = (u16 *) buf;
+	len >>= 1;
+
+	for (i = 0; i < len; i++) {
+		writew(p[i], this->IO_ADDR_W);
+		while (GPMC_BUF_EMPTY == (readl(GPMC_STATUS) & GPMC_BUF_FULL)) ;
+	}
+}
+
+/*
+ * nand_read_buf16 - [DEFAULT] read chip data into buffer
+ * @mtd:	MTD device structure
+ * @buf:	buffer to store date
+ * @len:	number of bytes to read
+ *
+ * Default read function for 16bit buswith
+ */
+static void omap_nand_read_buf(struct mtd_info *mtd, u_char *buf, int len)
+{
+	int i;
+	struct nand_chip *this = mtd->priv;
+	u16 *p = (u16 *) buf;
+	len >>= 1;
+
+	for (i = 0; i < len; i++)
+		p[i] = readw(this->IO_ADDR_R);
+}
+
+#else
+/*
+ * omap_nand_write_buf -  write buffer to NAND controller
+ * @mtd:        MTD device structure
+ * @buf:        data buffer
+ * @len:        number of bytes to write
+ *
+ */
+static void omap_nand_write_buf(struct mtd_info *mtd, const uint8_t *buf,
+				int len)
+{
+	int i;
+	int j = 0;
+	struct nand_chip *chip = mtd->priv;
+
+	for (i = 0; i < len; i++) {
+		writeb(buf[i], chip->IO_ADDR_W);
+		for (j = 0; j < 10; j++) ;
+	}
+
+}
+
+/*
+ * omap_nand_read_buf - read data from NAND controller into buffer
+ * @mtd:        MTD device structure
+ * @buf:        buffer to store date
+ * @len:        number of bytes to read
+ *
+ */
+static void omap_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+	int i;
+	int j = 0;
+	struct nand_chip *chip = mtd->priv;
+
+	for (i = 0; i < len; i++) {
+		buf[i] = readb(chip->IO_ADDR_R);
+		while (GPMC_BUF_EMPTY == (readl(GPMC_STATUS) & GPMC_BUF_FULL));
+	}
+}
+#endif /* CONFIG_SYS_NAND_WIDTH_16 */
+
+/*
+ * omap_hwecc_init - Initialize the Hardware ECC for NAND flash in
+ *                   GPMC controller
+ * @mtd:        MTD device structure
+ *
+ */
+static void omap_hwecc_init(struct nand_chip *chip)
+{
+	unsigned long val = 0x0;
+
+	/* Init ECC Control Register */
+	/* Clear all ECC  | Enable Reg1 */
+	val = ((0x00000001 << 8) | 0x00000001);
+	__raw_writel(val, GPMC_BASE + GPMC_ECC_CONTROL);
+	__raw_writel(0x3fcff000, GPMC_BASE + GPMC_ECC_SIZE_CONFIG);
+}
+
+/*
+ * omap_correct_data - 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
+ * @calc_ecc: 		 ecc read from ECC registers
+ */
+static int omap_correct_data(struct mtd_info *mtd, u_char *dat,
+			     u_char *read_ecc, u_char *calc_ecc)
+{
+	return 0;
+}
+
+/*
+ *  omap_calculate_ecc - Generate non-inverted ECC bytes.
+ *
+ *  Using noninverted ECC can be considered ugly since writing a blank
+ *  page ie. padding will clear the ECC bytes. This is no problem as
+ *  long nobody is trying to write data on the seemingly unused page.
+ *  Reading an erased page will produce an ECC mismatch between
+ *  generated and read ECC bytes that has to be dealt with separately.
+ *  @mtd:	MTD structure
+ *  @dat:	unused
+ *  @ecc_code:	ecc_code buffer
+ */
+static int omap_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
+			      u_char *ecc_code)
+{
+	unsigned long val = 0x0;
+	unsigned long reg;
+
+	/* Start Reading from HW ECC1_Result = 0x200 */
+	reg = (unsigned long) (GPMC_BASE + GPMC_ECC1_RESULT);
+	val = __raw_readl(reg);
+
+	*ecc_code++ = ECC_P1_128_E(val);
+	*ecc_code++ = ECC_P1_128_O(val);
+	*ecc_code++ = ECC_P512_2048_E(val) | ECC_P512_2048_O(val) << 4;
+
+	return 0;
+}
+
+/*
+ * omap_enable_ecc - This function enables the hardware ecc functionality
+ * @mtd:        MTD device structure
+ * @mode:       Read/Write mode
+ */
+static void omap_enable_hwecc(struct mtd_info *mtd, int mode)
+{
+	struct nand_chip *chip = mtd->priv;
+	unsigned int val = __raw_readl(GPMC_BASE + GPMC_ECC_CONFIG);
+	unsigned int dev_width = (chip->options & NAND_BUSWIDTH_16) >> 1;
+
+	switch (mode) {
+	case NAND_ECC_READ:
+		__raw_writel(0x101, GPMC_BASE + GPMC_ECC_CONTROL);
+		/* ECC col width | CS | ECC Enable */
+		val = (dev_width << 7) | (cs << 1) | (0x1);
+		break;
+	case NAND_ECC_READSYN:
+		__raw_writel(0x100, GPMC_BASE + GPMC_ECC_CONTROL);
+		/* ECC col width | CS | ECC Enable */
+		val = (dev_width << 7) | (cs << 1) | (0x1);
+		break;
+	case NAND_ECC_WRITE:
+		__raw_writel(0x101, GPMC_BASE + GPMC_ECC_CONTROL);
+		/* ECC col width | CS | ECC Enable */
+		val = (dev_width << 7) | (cs << 1) | (0x1);
+		break;
+	default:
+		printf("Error: Unrecognized Mode[%d]!\n", mode);
+		break;
+	}
+
+	__raw_writel(val, GPMC_BASE + GPMC_ECC_CONFIG);
+}
+
+static struct nand_ecclayout hw_nand_oob_64 = {
+	.eccbytes = 12,
+	.eccpos = {
+		   2, 3, 4, 5,
+		   6, 7, 8, 9,
+		   10, 11, 12, 13},
+	.oobfree = { {20, 50} }	/* don't care */
+};
+
+static struct nand_ecclayout sw_nand_oob_64 = {
+	.eccbytes = 24,
+	.eccpos = {
+		   40, 41, 42, 43, 44, 45, 46, 47,
+		   48, 49, 50, 51, 52, 53, 54, 55,
+		   56, 57, 58, 59, 60, 61, 62, 63},
+	.oobfree = { {2, 38} }
+};
+
+void omap_nand_switch_ecc(struct mtd_info *mtd, int hardware)
+{
+	struct nand_chip *nand = mtd->priv;
+
+	if (!hardware) {
+		nand->ecc.mode = NAND_ECC_SOFT;
+		nand->ecc.layout = &sw_nand_oob_64;
+		nand->ecc.size = 256;	/* set default eccsize */
+		nand->ecc.bytes = 3;
+		nand->ecc.steps = 8;
+		nand->ecc.hwctl = 0;
+		nand->ecc.calculate = nand_calculate_ecc;
+		nand->ecc.correct = nand_correct_data;
+	} else {
+		nand->ecc.mode = NAND_ECC_HW;
+		nand->ecc.layout = &hw_nand_oob_64;
+		nand->ecc.size = 512;
+		nand->ecc.bytes = 3;
+		nand->ecc.steps = 4;
+		nand->ecc.hwctl = omap_enable_hwecc;
+		nand->ecc.correct = omap_correct_data;
+		nand->ecc.calculate = omap_calculate_ecc;
+		omap_hwecc_init(nand);
+	}
+	nand_switch_ecc(mtd);
+
+	if (nand->options & NAND_BUSWIDTH_16) {
+		mtd->oobavail = mtd->oobsize - (nand->ecc.layout->eccbytes + 2);
+		if (nand->ecc.layout->eccbytes & 0x01)
+			mtd->oobavail--;
+	} else
+		mtd->oobavail = mtd->oobsize - (nand->ecc.layout->eccbytes + 1);
+}
+
+/*
+ * Board-specific NAND initialization. The following members of the
+ * argument are board-specific (per include/linux/mtd/nand_new.h):
+ * - IO_ADDR_R?: address to read the 8 I/O lines of the flash device
+ * - IO_ADDR_W?: address to write the 8 I/O lines of the flash device
+ * - hwcontrol: hardwarespecific function for accesing control-lines
+ * - dev_ready: hardwarespecific function for  accesing device ready/busy line
+ * - enable_hwecc?: function to enable (reset)  hardware ecc generator. Must
+ *   only be provided if a hardware ECC is available
+ * - eccmode: mode of ecc, see defines
+ * - chip_delay: chip dependent delay for transfering data from array to
+ *   read regs (tR)
+ * - options: various chip options. They can partly be set to inform
+ *   nand_scan about special functionality. See the defines for further
+ *   explanation
+ * Members with a "?" were not set in the merged testing-NAND branch,
+ * so they are not set here either.
+ */
+int board_nand_init(struct nand_chip *nand)
+{
+	int gpmc_config = 0;
+	cs = 0;
+	while (cs <= GPMC_MAX_CS) {
+		/* Each GPMC set for a single CS is@offset 0x30 */
+		/* already remapped for us */
+		gpmc_cs_base_add = (GPMC_CONFIG_CS0 + (cs * 0x30));
+		/* xloader/Uboot would have written the NAND type for us
+		 * NOTE: This is a temporary measure and cannot handle ONENAND.
+		 * The proper way of doing this is to pass the setup of
+		 * u-boot up to kernel using kernel params - something on
+		 * the lines of machineID
+		 */
+		/* Check if NAND type is set */
+		if ((__raw_readl(gpmc_cs_base_add + GPMC_CONFIG1) & 0xC00) ==
+		    0x800) {
+			/* Found it!! */
+			break;
+		}
+		cs++;
+	}
+	if (cs > GPMC_MAX_CS) {
+		printf("NAND: Unable to find NAND settings in " \
+		       "GPMC Configuration - quitting\n");
+	}
+
+	gpmc_config = __raw_readl(GPMC_CONFIG);
+	/* Disable Write protect */
+	gpmc_config |= 0x10;
+	__raw_writel(gpmc_config, GPMC_CONFIG);
+
+	nand->IO_ADDR_R = (int *) gpmc_cs_base_add + GPMC_NAND_DAT;
+	nand->IO_ADDR_W = (int *) gpmc_cs_base_add + GPMC_NAND_CMD;
+
+	nand->cmd_ctrl = omap_nand_hwcontrol;
+	nand->options = NAND_NO_PADDING | NAND_CACHEPRG | NAND_NO_AUTOINCR |
+			NAND_BUSWIDTH_16 | NAND_NO_AUTOINCR;
+	nand->read_buf = omap_nand_read_buf;
+	nand->write_buf = omap_nand_write_buf;
+	nand->ecc.mode = NAND_ECC_SOFT;
+	/* if RDY/BSY line is connected to OMAP then use the omap ready
+	 * function and the generic nand_wait function which reads the
+	 * status register after monitoring the RDY/BSY line. Otherwise
+	 * use a standard chip delay which is slightly more than tR
+	 * (AC Timing) of the NAND device and read the status register
+	 * until you get a failure or success
+	 */
+	nand->waitfunc = omap_nand_wait;
+	nand->chip_delay = 50;
+
+	return 0;
+}
+#endif /* CONFIG_CMD_NAND */
Index: u-boot-arm/cpu/arm_cortexa8/omap3/Makefile
===================================================================
--- u-boot-arm.orig/cpu/arm_cortexa8/omap3/Makefile
+++ u-boot-arm/cpu/arm_cortexa8/omap3/Makefile
@@ -26,7 +26,7 @@ include $(TOPDIR)/config.mk
 LIB	= lib$(SOC).a
 
 SOBJS	:= lowlevel_init.o
-OBJS	:= sys_info.o board.o clock.o interrupts.o
+OBJS	:= sys_info.o board.o clock.o interrupts.o mem.o syslib.o nand.o
 
 all:	.depend $(LIB)
 
Index: u-boot-arm/examples/Makefile
===================================================================
--- u-boot-arm.orig/examples/Makefile
+++ u-boot-arm/examples/Makefile
@@ -30,10 +30,12 @@ LOAD_ADDR = 0x40000
 endif
 
 ifeq ($(ARCH),arm)
+LOAD_ADDR = 0xc100000
 ifeq ($(BOARD),omap2420h4)
 LOAD_ADDR = 0x80300000
-else
-LOAD_ADDR = 0xc100000
+endif
+ifeq ($(CPU),omap3)
+LOAD_ADDR = 0x80300000
 endif
 endif
 
Index: u-boot-arm/common/cmd_nand.c
===================================================================
--- u-boot-arm.orig/common/cmd_nand.c
+++ u-boot-arm/common/cmd_nand.c
@@ -38,6 +38,11 @@ int find_dev_and_part(const char *id, st
 		      u8 *part_num, struct part_info **part);
 #endif
 
+#if defined(CONFIG_OMAP) && (defined(CONFIG_OMAP3_BEAGLE)\
+	|| defined(CONFIG_OMAP3_EVM) || defined(CONFIG_OVERO))
+extern void omap_nand_switch_ecc(nand_info_t *nand, int hardware);
+#endif
+
 static int nand_dump(nand_info_t *nand, ulong off, int only_oob)
 {
 	int i;
@@ -234,6 +239,10 @@ int do_nand(cmd_tbl_t * cmdtp, int flag,
 	    strncmp(cmd, "read", 4) != 0 && strncmp(cmd, "write", 5) != 0 &&
 	    strcmp(cmd, "scrub") != 0 && strcmp(cmd, "markbad") != 0 &&
 	    strcmp(cmd, "biterr") != 0 &&
+#if defined(CONFIG_OMAP) && (defined(CONFIG_OMAP3_BEAGLE)\
+	|| defined(CONFIG_OMAP3_EVM) || defined(CONFIG_OVERO))
+	    strncmp(cmd, "ecc", 3) != 0 &&
+#endif
 	    strcmp(cmd, "lock") != 0 && strcmp(cmd, "unlock") != 0 )
 		goto usage;
 
@@ -318,6 +327,22 @@ int do_nand(cmd_tbl_t * cmdtp, int flag,
 
 	}
 
+#if defined(CONFIG_OMAP) && (defined(CONFIG_OMAP3_BEAGLE) \
+	|| defined(CONFIG_OMAP3_EVM) || defined(CONFIG_OVERO))
+	if (strncmp(cmd, "ecc", 3) == 0) {
+		if (argc < 2)
+			goto usage;
+		if (strncmp(argv[2], "hw", 2) == 0)
+			omap_nand_switch_ecc(nand, 1);
+		else if (strncmp(argv[2], "sw", 2) == 0)
+			omap_nand_switch_ecc(nand, 0);
+		else
+			goto usage;
+
+		return 0;
+	}
+#endif
+
 	if (strncmp(cmd, "read", 4) == 0 || strncmp(cmd, "write", 5) == 0) {
 		int read;
 
@@ -483,6 +508,10 @@ U_BOOT_CMD(nand, 5, 1, do_nand,
 	   "nand scrub - really clean NAND erasing bad blocks (UNSAFE)\n"
 	   "nand markbad off - mark bad block at offset (UNSAFE)\n"
 	   "nand biterr off - make a bit error@offset (UNSAFE)\n"
+#if defined(CONFIG_OMAP) && (defined(CONFIG_OMAP3_BEAGLE) \
+	|| defined(CONFIG_OMAP3_EVM) || defined(CONFIG_OVERO))
+	   "nand ecc [hw/sw] - switch the ecc calculation algorithm \n"
+#endif
 	   "nand lock [tight] [status]\n"
 	   "    bring nand to lock state or display locked pages\n"
 	   "nand unlock [offset] [size] - unlock section\n");
Index: u-boot-arm/drivers/mtd/nand/nand_base.c
===================================================================
--- u-boot-arm.orig/drivers/mtd/nand/nand_base.c
+++ u-boot-arm/drivers/mtd/nand/nand_base.c
@@ -2730,6 +2730,151 @@ int nand_scan_tail(struct mtd_info *mtd)
 	return 0;
 }
 
+#if defined(CONFIG_OMAP) && (defined(CONFIG_OMAP3_BEAGLE) \
+	|| defined(CONFIG_OMAP3_EVM) || defined(CONFIG_OVERO))
+void nand_switch_ecc(struct mtd_info *mtd)
+{
+	int i;
+	struct nand_chip *chip = mtd->priv;
+
+	if (!chip->buffers)
+		return;
+
+	switch (chip->ecc.mode) {
+	case NAND_ECC_HW:
+		/* Use standard hwecc read page function ? */
+		chip->ecc.read_page = nand_read_page_hwecc;
+		chip->ecc.write_page = nand_write_page_hwecc;
+		chip->ecc.read_oob = nand_read_oob_std;
+		chip->ecc.write_oob = nand_write_oob_std;
+		break;
+	case NAND_ECC_HW_SYNDROME:
+		if ((!chip->ecc.calculate || !chip->ecc.correct ||
+			!chip->ecc.hwctl) &&
+			(!chip->ecc.read_page ||
+			chip->ecc.read_page == nand_read_page_hwecc ||
+			!chip->ecc.write_page ||
+			chip->ecc.write_page == nand_write_page_hwecc)) {
+			printk(KERN_WARNING "No ECC functions supplied, "
+				"Hardware ECC not possible\n");
+			BUG();
+		}
+		/* Use standard syndrome read/write page function ? */
+		chip->ecc.read_page = nand_read_page_syndrome;
+		chip->ecc.write_page = nand_write_page_syndrome;
+		chip->ecc.read_oob = nand_read_oob_syndrome;
+		chip->ecc.write_oob = nand_write_oob_syndrome;
+
+		if (mtd->writesize >= chip->ecc.size)
+			break;
+		printk(KERN_WARNING "%d byte HW ECC not possible on "
+			"%d byte page size, fallback to SW ECC\n",
+			chip->ecc.size, mtd->writesize);
+		chip->ecc.mode = NAND_ECC_SOFT;
+		break;
+	case NAND_ECC_SOFT:
+		chip->ecc.calculate = nand_calculate_ecc;
+		chip->ecc.correct = nand_correct_data;
+		chip->ecc.read_page = nand_read_page_swecc;
+		chip->ecc.write_page = nand_write_page_swecc;
+		chip->ecc.read_oob = nand_read_oob_std;
+		chip->ecc.write_oob = nand_write_oob_std;
+		chip->ecc.size = 256;
+		chip->ecc.bytes = 3;
+		break;
+
+	case NAND_ECC_NONE:
+		printk(KERN_WARNING "NAND_ECC_NONE selected by board driver. "
+			"This is not recommended !!\n");
+		chip->ecc.read_page = nand_read_page_raw;
+		chip->ecc.write_page = nand_write_page_raw;
+		chip->ecc.read_oob = nand_read_oob_std;
+		chip->ecc.write_oob = nand_write_oob_std;
+		chip->ecc.size = mtd->writesize;
+		chip->ecc.bytes = 0;
+		break;
+
+	default:
+		printk(KERN_WARNING "Invalid NAND_ECC_MODE %d\n",
+			chip->ecc.mode);
+		BUG();
+	}
+	/*
+	* The number of bytes available for a client to place data into
+	* the out of band area
+	*/
+	chip->ecc.layout->oobavail = 0;
+	for (i = 0; chip->ecc.layout->oobfree[i].length; i++)
+		chip->ecc.layout->oobavail +=
+			chip->ecc.layout->oobfree[i].length;
+	mtd->oobavail = chip->ecc.layout->oobavail;
+
+	/*
+	* Set the number of read / write steps for one page depending on ECC
+	* mode
+	*/
+	chip->ecc.steps = mtd->writesize / chip->ecc.size;
+	if (chip->ecc.steps * chip->ecc.size != mtd->writesize) {
+		printk(KERN_WARNING "Invalid ecc parameters\n");
+		BUG();
+	}
+	chip->ecc.total = chip->ecc.steps * chip->ecc.bytes;
+
+	/*
+	* Allow subpage writes up to ecc.steps. Not possible for MLC
+	* FLASH.
+	*/
+	if (!(chip->options & NAND_NO_SUBPAGE_WRITE) &&
+		!(chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
+		switch (chip->ecc.steps) {
+		case 2:
+			mtd->subpage_sft = 1;
+			break;
+		case 4:
+		case 8:
+			mtd->subpage_sft = 2;
+			break;
+		}
+	}
+	chip->subpagesize = mtd->writesize >> mtd->subpage_sft;
+
+	/* Initialize state */
+	chip->state = FL_READY;
+
+	/* De-select the device */
+	chip->select_chip(mtd, -1);
+
+	/* Invalidate the pagebuffer reference */
+	chip->pagebuf = -1;
+
+	/* Fill in remaining MTD driver data */
+	mtd->type = MTD_NANDFLASH;
+	mtd->flags = MTD_CAP_NANDFLASH;
+	mtd->erase = nand_erase;
+	mtd->point = NULL;
+	mtd->unpoint = NULL;
+	mtd->read = nand_read;
+	mtd->write = nand_write;
+	mtd->read_oob = nand_read_oob;
+	mtd->write_oob = nand_write_oob;
+	mtd->sync = nand_sync;
+	mtd->lock = NULL;
+	mtd->unlock = NULL;
+	mtd->suspend = nand_suspend;
+	mtd->resume = nand_resume;
+	mtd->block_isbad = nand_block_isbad;
+	mtd->block_markbad = nand_block_markbad;
+
+	/* propagate ecc.layout to mtd_info */
+	mtd->ecclayout = chip->ecc.layout;
+
+	/* Check, if we should skip the bad block table scan */
+	if (chip->options & NAND_SKIP_BBTSCAN)
+		chip->options |= NAND_BBT_SCANNED;
+
+}
+#endif
+
 /* module_text_address() isn't exported, and it's mostly a pointless
    test if this is a module _anyway_ -- they'd have to try _really_ hard
    to call us from in-kernel code if the core NAND support is modular. */

[U-Boot] [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib common files, add NAND support

[Scott Wood]

On Fri, Oct 03, 2008 at 12:40:25PM +0200, dirk.behme at googlemail.com wrote:
> +#include <common.h>
> +#include <asm/io.h>
> +#include <asm/arch/mem.h>
> +#include <linux/mtd/nand_ecc.h>
> +
> +#if defined(CONFIG_CMD_NAND)
> +
> +#include <nand.h>

Move the #ifdef to the Makefile.

> +/*
> + * nand_read_buf16 - [DEFAULT] read chip data into buffer
> + * @mtd:	MTD device structure
> + * @buf:	buffer to store date
> + * @len:	number of bytes to read
> + *
> + * Default read function for 16bit buswith
> + */
> +static void omap_nand_read_buf(struct mtd_info *mtd, u_char *buf, int len)
> +{
> +	int i;
> +	struct nand_chip *this = mtd->priv;
> +	u16 *p = (u16 *) buf;
> +	len >>= 1;
> +
> +	for (i = 0; i < len; i++)
> +		p[i] = readw(this->IO_ADDR_R);
> +}

How does this differ from the default implementation?

> +static void omap_nand_write_buf(struct mtd_info *mtd, const uint8_t *buf,
> +				int len)
> +{
> +	int i;
> +	int j = 0;
> +	struct nand_chip *chip = mtd->priv;
> +
> +	for (i = 0; i < len; i++) {
> +		writeb(buf[i], chip->IO_ADDR_W);
> +		for (j = 0; j < 10; j++) ;
> +	}
> +
> +}
> +
> +/*
> + * omap_nand_read_buf - read data from NAND controller into buffer
> + * @mtd:        MTD device structure
> + * @buf:        buffer to store date
> + * @len:        number of bytes to read
> + *
> + */
> +static void omap_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
> +{
> +	int i;
> +	int j = 0;
> +	struct nand_chip *chip = mtd->priv;
> +
> +	for (i = 0; i < len; i++) {
> +		buf[i] = readb(chip->IO_ADDR_R);
> +		while (GPMC_BUF_EMPTY == (readl(GPMC_STATUS) & GPMC_BUF_FULL));
> +	}
> +}

I'm a bit confused... with 16-bit NAND, you check GMPC_STATUS[BUF_FULL]
when writing, but with 8-bit NAND, you check it when reading?  And 8-bit
writes have an arbitrary, cpu-speed-dependent delay, and 16-bit reads
have no delay at all?

> +static void omap_hwecc_init(struct nand_chip *chip)
> +{
> +	unsigned long val = 0x0;
> +
> +	/* Init ECC Control Register */
> +	/* Clear all ECC  | Enable Reg1 */
> +	val = ((0x00000001 << 8) | 0x00000001);
> +	__raw_writel(val, GPMC_BASE + GPMC_ECC_CONTROL);
> +	__raw_writel(0x3fcff000, GPMC_BASE + GPMC_ECC_SIZE_CONFIG);
> +}

Why raw?

> +/*
> + * omap_correct_data - 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
> + * @calc_ecc: 		 ecc read from ECC registers
> + */
> +static int omap_correct_data(struct mtd_info *mtd, u_char *dat,
> +			     u_char *read_ecc, u_char *calc_ecc)
> +{
> +	return 0;
> +}

This obviously is not correcting anything.  If the errors are corrected
automatically by the controller, say so in the comments.

> +/*
> + *  omap_calculate_ecc - Generate non-inverted ECC bytes.
> + *
> + *  Using noninverted ECC can be considered ugly since writing a blank
> + *  page ie. padding will clear the ECC bytes. This is no problem as
> + *  long nobody is trying to write data on the seemingly unused page.
> + *  Reading an erased page will produce an ECC mismatch between
> + *  generated and read ECC bytes that has to be dealt with separately.

Is this a hardware limitation?  If so, say so in the comment.  If not,
why do it this way?

> + *  @mtd:	MTD structure
> + *  @dat:	unused
> + *  @ecc_code:	ecc_code buffer
> + */
> +static int omap_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
> +			      u_char *ecc_code)
> +{
> +	unsigned long val = 0x0;
> +	unsigned long reg;
> +
> +	/* Start Reading from HW ECC1_Result = 0x200 */
> +	reg = (unsigned long) (GPMC_BASE + GPMC_ECC1_RESULT);
> +	val = __raw_readl(reg);
> +
> +	*ecc_code++ = ECC_P1_128_E(val);
> +	*ecc_code++ = ECC_P1_128_O(val);
> +	*ecc_code++ = ECC_P512_2048_E(val) | ECC_P512_2048_O(val) << 4;

These macros are used nowhere else; why obscure what it's doing by moving
it to the top of the file?

> +static void omap_enable_hwecc(struct mtd_info *mtd, int mode)
> +{
> +	struct nand_chip *chip = mtd->priv;
> +	unsigned int val = __raw_readl(GPMC_BASE + GPMC_ECC_CONFIG);
> +	unsigned int dev_width = (chip->options & NAND_BUSWIDTH_16) >> 1;
> +
> +	switch (mode) {
> +	case NAND_ECC_READ:
> +		__raw_writel(0x101, GPMC_BASE + GPMC_ECC_CONTROL);
> +		/* ECC col width | CS | ECC Enable */
> +		val = (dev_width << 7) | (cs << 1) | (0x1);
> +		break;
> +	case NAND_ECC_READSYN:
> +		__raw_writel(0x100, GPMC_BASE + GPMC_ECC_CONTROL);
> +		/* ECC col width | CS | ECC Enable */
> +		val = (dev_width << 7) | (cs << 1) | (0x1);
> +		break;
> +	case NAND_ECC_WRITE:
> +		__raw_writel(0x101, GPMC_BASE + GPMC_ECC_CONTROL);
> +		/* ECC col width | CS | ECC Enable */
> +		val = (dev_width << 7) | (cs << 1) | (0x1);
> +		break;
> +	default:
> +		printf("Error: Unrecognized Mode[%d]!\n", mode);
> +		break;
> +	}
> +
> +	__raw_writel(val, GPMC_BASE + GPMC_ECC_CONFIG);
> +}

Is it OK if config gets written before control, or if this whole thing
gets done out of order with respect to other raw writes?

> +static struct nand_ecclayout hw_nand_oob_64 = {
> +	.eccbytes = 12,
> +	.eccpos = {
> +		   2, 3, 4, 5,
> +		   6, 7, 8, 9,
> +		   10, 11, 12, 13},
> +	.oobfree = { {20, 50} }	/* don't care */

Bytes 64-69 of a 64-byte OOB are free?
What don't we care about?

> +	if (nand->options & NAND_BUSWIDTH_16) {
> +		mtd->oobavail = mtd->oobsize - (nand->ecc.layout->eccbytes + 2);
> +		if (nand->ecc.layout->eccbytes & 0x01)
> +			mtd->oobavail--;
> +	} else
> +		mtd->oobavail = mtd->oobsize - (nand->ecc.layout->eccbytes + 1);
> +}

oobavail is calculated by the generic NAND code.  You don't need to do it
here.

> +/*
> + * Board-specific NAND initialization. The following members of the
> + * argument are board-specific (per include/linux/mtd/nand_new.h):
> + * - IO_ADDR_R?: address to read the 8 I/O lines of the flash device
> + * - IO_ADDR_W?: address to write the 8 I/O lines of the flash device
> + * - hwcontrol: hardwarespecific function for accesing control-lines
> + * - dev_ready: hardwarespecific function for  accesing device ready/busy line
> + * - enable_hwecc?: function to enable (reset)  hardware ecc generator. Must
> + *   only be provided if a hardware ECC is available
> + * - eccmode: mode of ecc, see defines
> + * - chip_delay: chip dependent delay for transfering data from array to
> + *   read regs (tR)
> + * - options: various chip options. They can partly be set to inform
> + *   nand_scan about special functionality. See the defines for further
> + *   explanation
> + * Members with a "?" were not set in the merged testing-NAND branch,
> + * so they are not set here either.

IO_ADDR_R and IO_ADDR_W have a "?" but are set here.  If you have a
question about the API, ask it on the list, rather than encoding it in
the source.

> ===================================================================
> --- u-boot-arm.orig/drivers/mtd/nand/nand_base.c
> +++ u-boot-arm/drivers/mtd/nand/nand_base.c
> @@ -2730,6 +2730,151 @@ int nand_scan_tail(struct mtd_info *mtd)
>  	return 0;
>  }
>  
> +#if defined(CONFIG_OMAP) && (defined(CONFIG_OMAP3_BEAGLE) \
> +	|| defined(CONFIG_OMAP3_EVM) || defined(CONFIG_OVERO))
> +void nand_switch_ecc(struct mtd_info *mtd)

NACK.  First, explain why you need to be able to dynamically switch ECC
modes.

Then, if it is justified, implement it in a separate patch, without all
the duplication of code, and without platform-specific #ifdefs.

-Scott

[U-Boot] [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib common files, add NAND support

[Dirk Behme]

Scott,

many thanks for the review!

As this code is directly taken from some TI code, it seems I have to 
find somebody who can answer your questions and rework the code now. 
Will do so now. Unfortunately, I don't know a lot about NAND.

Thanks

Dirk

Scott Wood wrote:
> On Fri, Oct 03, 2008 at 12:40:25PM +0200, dirk.behme at googlemail.com wrote:
> 
>>+#include <common.h>
>>+#include <asm/io.h>
>>+#include <asm/arch/mem.h>
>>+#include <linux/mtd/nand_ecc.h>
>>+
>>+#if defined(CONFIG_CMD_NAND)
>>+
>>+#include <nand.h>
> 
> 
> Move the #ifdef to the Makefile.
> 
> 
>>+/*
>>+ * nand_read_buf16 - [DEFAULT] read chip data into buffer
>>+ * @mtd:	MTD device structure
>>+ * @buf:	buffer to store date
>>+ * @len:	number of bytes to read
>>+ *
>>+ * Default read function for 16bit buswith
>>+ */
>>+static void omap_nand_read_buf(struct mtd_info *mtd, u_char *buf, int len)
>>+{
>>+	int i;
>>+	struct nand_chip *this = mtd->priv;
>>+	u16 *p = (u16 *) buf;
>>+	len >>= 1;
>>+
>>+	for (i = 0; i < len; i++)
>>+		p[i] = readw(this->IO_ADDR_R);
>>+}
> 
> 
> How does this differ from the default implementation?
> 
> 
>>+static void omap_nand_write_buf(struct mtd_info *mtd, const uint8_t *buf,
>>+				int len)
>>+{
>>+	int i;
>>+	int j = 0;
>>+	struct nand_chip *chip = mtd->priv;
>>+
>>+	for (i = 0; i < len; i++) {
>>+		writeb(buf[i], chip->IO_ADDR_W);
>>+		for (j = 0; j < 10; j++) ;
>>+	}
>>+
>>+}
>>+
>>+/*
>>+ * omap_nand_read_buf - read data from NAND controller into buffer
>>+ * @mtd:        MTD device structure
>>+ * @buf:        buffer to store date
>>+ * @len:        number of bytes to read
>>+ *
>>+ */
>>+static void omap_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
>>+{
>>+	int i;
>>+	int j = 0;
>>+	struct nand_chip *chip = mtd->priv;
>>+
>>+	for (i = 0; i < len; i++) {
>>+		buf[i] = readb(chip->IO_ADDR_R);
>>+		while (GPMC_BUF_EMPTY == (readl(GPMC_STATUS) & GPMC_BUF_FULL));
>>+	}
>>+}
> 
> 
> I'm a bit confused... with 16-bit NAND, you check GMPC_STATUS[BUF_FULL]
> when writing, but with 8-bit NAND, you check it when reading?  And 8-bit
> writes have an arbitrary, cpu-speed-dependent delay, and 16-bit reads
> have no delay at all?
> 
> 
>>+static void omap_hwecc_init(struct nand_chip *chip)
>>+{
>>+	unsigned long val = 0x0;
>>+
>>+	/* Init ECC Control Register */
>>+	/* Clear all ECC  | Enable Reg1 */
>>+	val = ((0x00000001 << 8) | 0x00000001);
>>+	__raw_writel(val, GPMC_BASE + GPMC_ECC_CONTROL);
>>+	__raw_writel(0x3fcff000, GPMC_BASE + GPMC_ECC_SIZE_CONFIG);
>>+}
> 
> 
> Why raw?
> 
> 
>>+/*
>>+ * omap_correct_data - 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
>>+ * @calc_ecc: 		 ecc read from ECC registers
>>+ */
>>+static int omap_correct_data(struct mtd_info *mtd, u_char *dat,
>>+			     u_char *read_ecc, u_char *calc_ecc)
>>+{
>>+	return 0;
>>+}
> 
> 
> This obviously is not correcting anything.  If the errors are corrected
> automatically by the controller, say so in the comments.
> 
> 
>>+/*
>>+ *  omap_calculate_ecc - Generate non-inverted ECC bytes.
>>+ *
>>+ *  Using noninverted ECC can be considered ugly since writing a blank
>>+ *  page ie. padding will clear the ECC bytes. This is no problem as
>>+ *  long nobody is trying to write data on the seemingly unused page.
>>+ *  Reading an erased page will produce an ECC mismatch between
>>+ *  generated and read ECC bytes that has to be dealt with separately.
> 
> 
> Is this a hardware limitation?  If so, say so in the comment.  If not,
> why do it this way?
> 
> 
>>+ *  @mtd:	MTD structure
>>+ *  @dat:	unused
>>+ *  @ecc_code:	ecc_code buffer
>>+ */
>>+static int omap_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
>>+			      u_char *ecc_code)
>>+{
>>+	unsigned long val = 0x0;
>>+	unsigned long reg;
>>+
>>+	/* Start Reading from HW ECC1_Result = 0x200 */
>>+	reg = (unsigned long) (GPMC_BASE + GPMC_ECC1_RESULT);
>>+	val = __raw_readl(reg);
>>+
>>+	*ecc_code++ = ECC_P1_128_E(val);
>>+	*ecc_code++ = ECC_P1_128_O(val);
>>+	*ecc_code++ = ECC_P512_2048_E(val) | ECC_P512_2048_O(val) << 4;
> 
> 
> These macros are used nowhere else; why obscure what it's doing by moving
> it to the top of the file?
> 
> 
>>+static void omap_enable_hwecc(struct mtd_info *mtd, int mode)
>>+{
>>+	struct nand_chip *chip = mtd->priv;
>>+	unsigned int val = __raw_readl(GPMC_BASE + GPMC_ECC_CONFIG);
>>+	unsigned int dev_width = (chip->options & NAND_BUSWIDTH_16) >> 1;
>>+
>>+	switch (mode) {
>>+	case NAND_ECC_READ:
>>+		__raw_writel(0x101, GPMC_BASE + GPMC_ECC_CONTROL);
>>+		/* ECC col width | CS | ECC Enable */
>>+		val = (dev_width << 7) | (cs << 1) | (0x1);
>>+		break;
>>+	case NAND_ECC_READSYN:
>>+		__raw_writel(0x100, GPMC_BASE + GPMC_ECC_CONTROL);
>>+		/* ECC col width | CS | ECC Enable */
>>+		val = (dev_width << 7) | (cs << 1) | (0x1);
>>+		break;
>>+	case NAND_ECC_WRITE:
>>+		__raw_writel(0x101, GPMC_BASE + GPMC_ECC_CONTROL);
>>+		/* ECC col width | CS | ECC Enable */
>>+		val = (dev_width << 7) | (cs << 1) | (0x1);
>>+		break;
>>+	default:
>>+		printf("Error: Unrecognized Mode[%d]!\n", mode);
>>+		break;
>>+	}
>>+
>>+	__raw_writel(val, GPMC_BASE + GPMC_ECC_CONFIG);
>>+}
> 
> 
> Is it OK if config gets written before control, or if this whole thing
> gets done out of order with respect to other raw writes?
> 
> 
>>+static struct nand_ecclayout hw_nand_oob_64 = {
>>+	.eccbytes = 12,
>>+	.eccpos = {
>>+		   2, 3, 4, 5,
>>+		   6, 7, 8, 9,
>>+		   10, 11, 12, 13},
>>+	.oobfree = { {20, 50} }	/* don't care */
> 
> 
> Bytes 64-69 of a 64-byte OOB are free?
> What don't we care about?
> 
> 
>>+	if (nand->options & NAND_BUSWIDTH_16) {
>>+		mtd->oobavail = mtd->oobsize - (nand->ecc.layout->eccbytes + 2);
>>+		if (nand->ecc.layout->eccbytes & 0x01)
>>+			mtd->oobavail--;
>>+	} else
>>+		mtd->oobavail = mtd->oobsize - (nand->ecc.layout->eccbytes + 1);
>>+}
> 
> 
> oobavail is calculated by the generic NAND code.  You don't need to do it
> here.
> 
> 
>>+/*
>>+ * Board-specific NAND initialization. The following members of the
>>+ * argument are board-specific (per include/linux/mtd/nand_new.h):
>>+ * - IO_ADDR_R?: address to read the 8 I/O lines of the flash device
>>+ * - IO_ADDR_W?: address to write the 8 I/O lines of the flash device
>>+ * - hwcontrol: hardwarespecific function for accesing control-lines
>>+ * - dev_ready: hardwarespecific function for  accesing device ready/busy line
>>+ * - enable_hwecc?: function to enable (reset)  hardware ecc generator. Must
>>+ *   only be provided if a hardware ECC is available
>>+ * - eccmode: mode of ecc, see defines
>>+ * - chip_delay: chip dependent delay for transfering data from array to
>>+ *   read regs (tR)
>>+ * - options: various chip options. They can partly be set to inform
>>+ *   nand_scan about special functionality. See the defines for further
>>+ *   explanation
>>+ * Members with a "?" were not set in the merged testing-NAND branch,
>>+ * so they are not set here either.
> 
> 
> IO_ADDR_R and IO_ADDR_W have a "?" but are set here.  If you have a
> question about the API, ask it on the list, rather than encoding it in
> the source.
> 
> 
>>===================================================================
>>--- u-boot-arm.orig/drivers/mtd/nand/nand_base.c
>>+++ u-boot-arm/drivers/mtd/nand/nand_base.c
>>@@ -2730,6 +2730,151 @@ int nand_scan_tail(struct mtd_info *mtd)
>> 	return 0;
>> }
>> 
>>+#if defined(CONFIG_OMAP) && (defined(CONFIG_OMAP3_BEAGLE) \
>>+	|| defined(CONFIG_OMAP3_EVM) || defined(CONFIG_OVERO))
>>+void nand_switch_ecc(struct mtd_info *mtd)
> 
> 
> NACK.  First, explain why you need to be able to dynamically switch ECC
> modes.
> 
> Then, if it is justified, implement it in a separate patch, without all
> the duplication of code, and without platform-specific #ifdefs.
> 
> -Scott
> 

[U-Boot] [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib common files, add NAND support

[Dirk Behme]

Unfortunately, except Nishanth's comments, I didn't get any further 
help (e.g. from TI) for this yet. So I started to look at this myself. 
Please forgive everything I missed as I'm no NAND expert. Maybe you 
like to explain some additional details regarding what I missed ;)

First version of updated NAND patch in attachment. This is RFC only. 
I'd like to fix anything still open. When this is done and when 
everybody is fine with this, I will send an updated v3 of whole OMAP3 
patch set including the then final NAND patch (and the fixes for all 
other comments).

Scott Wood wrote:
> On Fri, Oct 03, 2008 at 12:40:25PM +0200, dirk.behme at googlemail.com wrote:
> 
>>+#include <common.h>
>>+#include <asm/io.h>
>>+#include <asm/arch/mem.h>
>>+#include <linux/mtd/nand_ecc.h>
>>+
>>+#if defined(CONFIG_CMD_NAND)
>>+
>>+#include <nand.h>
> 
> 
> Move the #ifdef to the Makefile.

Did this. Additionally, I moved OMAP3 NAND driver to 
drivers/mtd/nand/omap3.c.

>>+/*
>>+ * nand_read_buf16 - [DEFAULT] read chip data into buffer
>>+ * @mtd:	MTD device structure
>>+ * @buf:	buffer to store date
>>+ * @len:	number of bytes to read
>>+ *
>>+ * Default read function for 16bit buswith
>>+ */
>>+static void omap_nand_read_buf(struct mtd_info *mtd, u_char *buf, int len)
>>+{
>>+	int i;
>>+	struct nand_chip *this = mtd->priv;
>>+	u16 *p = (u16 *) buf;
>>+	len >>= 1;
>>+
>>+	for (i = 0; i < len; i++)
>>+		p[i] = readw(this->IO_ADDR_R);
>>+}
> 
> 
> How does this differ from the default implementation?

It doesn't differ ;)

So I removed this and tried to use default nand_read_buf16() instead:

nand->read_buf = nand_read_buf16;

in board_nand_init(). But this doesn't compile as nand_read_buf16() is 
static in nand_base.c. How do I use it in OMAP3 NAND driver? Marked it 
as FIXME in patch.

>>+static void omap_nand_write_buf(struct mtd_info *mtd, const uint8_t *buf,
>>+				int len)
>>+{
>>+	int i;
>>+	int j = 0;
>>+	struct nand_chip *chip = mtd->priv;
>>+
>>+	for (i = 0; i < len; i++) {
>>+		writeb(buf[i], chip->IO_ADDR_W);
>>+		for (j = 0; j < 10; j++) ;
>>+	}
>>+
>>+}
>>+
>>+/*
>>+ * omap_nand_read_buf - read data from NAND controller into buffer
>>+ * @mtd:        MTD device structure
>>+ * @buf:        buffer to store date
>>+ * @len:        number of bytes to read
>>+ *
>>+ */
>>+static void omap_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
>>+{
>>+	int i;
>>+	int j = 0;
>>+	struct nand_chip *chip = mtd->priv;
>>+
>>+	for (i = 0; i < len; i++) {
>>+		buf[i] = readb(chip->IO_ADDR_R);
>>+		while (GPMC_BUF_EMPTY == (readl(GPMC_STATUS) & GPMC_BUF_FULL));
>>+	}
>>+}
> 
> 
> I'm a bit confused... with 16-bit NAND, you check GMPC_STATUS[BUF_FULL]
> when writing, but with 8-bit NAND, you check it when reading?  And 8-bit
> writes have an arbitrary, cpu-speed-dependent delay, and 16-bit reads
> have no delay at all?

Removed 8-bit stuff completely as it isn't used.

>>+static void omap_hwecc_init(struct nand_chip *chip)
>>+{
>>+	unsigned long val = 0x0;
>>+
>>+	/* Init ECC Control Register */
>>+	/* Clear all ECC  | Enable Reg1 */
>>+	val = ((0x00000001 << 8) | 0x00000001);
>>+	__raw_writel(val, GPMC_BASE + GPMC_ECC_CONTROL);
>>+	__raw_writel(0x3fcff000, GPMC_BASE + GPMC_ECC_SIZE_CONFIG);
>>+}
> 
> 
> Why raw?

Removed all _raw_ . At ARM, it seems that __raw_writex()/readx() are 
the same as  writex()/readx().

>>+/*
>>+ * omap_correct_data - 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
>>+ * @calc_ecc: 		 ecc read from ECC registers
>>+ */
>>+static int omap_correct_data(struct mtd_info *mtd, u_char *dat,
>>+			     u_char *read_ecc, u_char *calc_ecc)
>>+{
>>+	return 0;
>>+}
> 
> 
> This obviously is not correcting anything.  If the errors are corrected
> automatically by the controller, say so in the comments.

I replaced this with the version from Nishanth's U-Boot v2:

http://git.denx.de/?p=u-boot/u-boot-v2.git;a=blob;f=drivers/nand/nand_omap_gpmc.c;h=afd676a74194c63a1e3cf581d210235a9d4c78ba;hb=HEAD

>>+/*
>>+ *  omap_calculate_ecc - Generate non-inverted ECC bytes.
>>+ *
>>+ *  Using noninverted ECC can be considered ugly since writing a blank
>>+ *  page ie. padding will clear the ECC bytes. This is no problem as
>>+ *  long nobody is trying to write data on the seemingly unused page.
>>+ *  Reading an erased page will produce an ECC mismatch between
>>+ *  generated and read ECC bytes that has to be dealt with separately.
> 
> 
> Is this a hardware limitation?  If so, say so in the comment.  If not,
> why do it this way?

Don't know.

All: Any help?

>>+ *  @mtd:	MTD structure
>>+ *  @dat:	unused
>>+ *  @ecc_code:	ecc_code buffer
>>+ */
>>+static int omap_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
>>+			      u_char *ecc_code)
>>+{
>>+	unsigned long val = 0x0;
>>+	unsigned long reg;
>>+
>>+	/* Start Reading from HW ECC1_Result = 0x200 */
>>+	reg = (unsigned long) (GPMC_BASE + GPMC_ECC1_RESULT);
>>+	val = __raw_readl(reg);
>>+
>>+	*ecc_code++ = ECC_P1_128_E(val);
>>+	*ecc_code++ = ECC_P1_128_O(val);
>>+	*ecc_code++ = ECC_P512_2048_E(val) | ECC_P512_2048_O(val) << 4;
> 
> 
> These macros are used nowhere else; why obscure what it's doing by moving
> it to the top of the file?

I replaced this with the version from Nishanth's U-Boot v2 (see link 
above)

>>+static void omap_enable_hwecc(struct mtd_info *mtd, int mode)
>>+{
>>+	struct nand_chip *chip = mtd->priv;
>>+	unsigned int val = __raw_readl(GPMC_BASE + GPMC_ECC_CONFIG);
>>+	unsigned int dev_width = (chip->options & NAND_BUSWIDTH_16) >> 1;
>>+
>>+	switch (mode) {
>>+	case NAND_ECC_READ:
>>+		__raw_writel(0x101, GPMC_BASE + GPMC_ECC_CONTROL);
>>+		/* ECC col width | CS | ECC Enable */
>>+		val = (dev_width << 7) | (cs << 1) | (0x1);
>>+		break;
>>+	case NAND_ECC_READSYN:
>>+		__raw_writel(0x100, GPMC_BASE + GPMC_ECC_CONTROL);
>>+		/* ECC col width | CS | ECC Enable */
>>+		val = (dev_width << 7) | (cs << 1) | (0x1);
>>+		break;
>>+	case NAND_ECC_WRITE:
>>+		__raw_writel(0x101, GPMC_BASE + GPMC_ECC_CONTROL);
>>+		/* ECC col width | CS | ECC Enable */
>>+		val = (dev_width << 7) | (cs << 1) | (0x1);
>>+		break;
>>+	default:
>>+		printf("Error: Unrecognized Mode[%d]!\n", mode);
>>+		break;
>>+	}
>>+
>>+	__raw_writel(val, GPMC_BASE + GPMC_ECC_CONFIG);
>>+}
> 
> 
> Is it OK if config gets written before control, or if this whole thing
> gets done out of order with respect to other raw writes?

Hmm. I replaced this with the version from Nishanth's U-Boot v2 (see 
link above). If this isn't ok, can you explain a little more?

>>+static struct nand_ecclayout hw_nand_oob_64 = {
>>+	.eccbytes = 12,
>>+	.eccpos = {
>>+		   2, 3, 4, 5,
>>+		   6, 7, 8, 9,
>>+		   10, 11, 12, 13},
>>+	.oobfree = { {20, 50} }	/* don't care */
> 
> 
> Bytes 64-69 of a 64-byte OOB are free?
> What don't we care about?

Don't know (or understand?).

All: Any help?

>>+	if (nand->options & NAND_BUSWIDTH_16) {
>>+		mtd->oobavail = mtd->oobsize - (nand->ecc.layout->eccbytes + 2);
>>+		if (nand->ecc.layout->eccbytes & 0x01)
>>+			mtd->oobavail--;
>>+	} else
>>+		mtd->oobavail = mtd->oobsize - (nand->ecc.layout->eccbytes + 1);
>>+}
> 
> 
> oobavail is calculated by the generic NAND code.  You don't need to do it
> here.

Removed it.

>>+/*
>>+ * Board-specific NAND initialization. The following members of the
>>+ * argument are board-specific (per include/linux/mtd/nand_new.h):
>>+ * - IO_ADDR_R?: address to read the 8 I/O lines of the flash device
>>+ * - IO_ADDR_W?: address to write the 8 I/O lines of the flash device
>>+ * - hwcontrol: hardwarespecific function for accesing control-lines
>>+ * - dev_ready: hardwarespecific function for  accesing device ready/busy line
>>+ * - enable_hwecc?: function to enable (reset)  hardware ecc generator. Must
>>+ *   only be provided if a hardware ECC is available
>>+ * - eccmode: mode of ecc, see defines
>>+ * - chip_delay: chip dependent delay for transfering data from array to
>>+ *   read regs (tR)
>>+ * - options: various chip options. They can partly be set to inform
>>+ *   nand_scan about special functionality. See the defines for further
>>+ *   explanation
>>+ * Members with a "?" were not set in the merged testing-NAND branch,
>>+ * so they are not set here either.
> 
> 
> IO_ADDR_R and IO_ADDR_W have a "?" but are set here.  If you have a
> question about the API, ask it on the list, rather than encoding it in
> the source.

Seems that this was only a comment style issue (?). Cleaned up the 
comment. Better?

>>===================================================================
>>--- u-boot-arm.orig/drivers/mtd/nand/nand_base.c
>>+++ u-boot-arm/drivers/mtd/nand/nand_base.c
>>@@ -2730,6 +2730,151 @@ int nand_scan_tail(struct mtd_info *mtd)
>> 	return 0;
>> }
>> 
>>+#if defined(CONFIG_OMAP) && (defined(CONFIG_OMAP3_BEAGLE) \
>>+	|| defined(CONFIG_OMAP3_EVM) || defined(CONFIG_OVERO))
>>+void nand_switch_ecc(struct mtd_info *mtd)
> 
> 
> NACK.  First, explain why you need to be able to dynamically switch ECC
> modes.

Two topics here, changes in cmd_nand.c and nand_base.c.

cmd_nand.c:

We need to be able to switch ECC at runtime cause some images have to 
be written to NAND with HW ECC and some with SW ECC. This depends on 
what user (reader) of these parts expect.

OMAP3 has a boot ROM which is able to read a second level loader 
(called x-loader) from NAND and start/execute it. This 2nd level 
loader has to be written by U-Boot using HW ECC as ROM code does HW 
ECC to read the image. All other parts, e.g. Linux kernel, use SW ECC 
as default. For this we have to use SW ECC to write images, then.

Therefore we add an additional user command in cmd_nand.c to switch 
ECC depending on what user wants to write.

> Then, if it is justified, implement it in a separate patch, without all
> the duplication of code, and without platform-specific #ifdefs.

nand_base.c:

The large nand_switch_ecc() in nand_base.c seems to be a merge error. 
Thanks for finding this. I *removed* it completely.

I.e. with patch in attachment we don't touch nand_base.c any more, but 
still have an #ifdeffed user command in cmd_nand.c. If you don't like 
this, please give a hint how to better do this.

Many thanks for your review and help

Dirk

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 08_omap3_nand.txt
Url: http://lists.denx.de/pipermail/u-boot/attachments/20081007/a9ab7378/attachment-0001.txt 

[U-Boot] [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib common files, add NAND support

[Nishanth Menon]

Dirk Behme said the following on 10/07/2008 04:42 AM:
>
> Scott Wood wrote:
>> On Fri, Oct 03, 2008 at 12:40:25PM +0200, dirk.behme at googlemail.com
>> wrote:
>>
>>> +#include <common.h>
>>> +#include <asm/io.h>
>>> +#include <asm/arch/mem.h>
>>> +#include <linux/mtd/nand_ecc.h>
>>> +
>>> +#if defined(CONFIG_CMD_NAND)
>>> +
>>> +#include <nand.h>
>>
>>
>> Move the #ifdef to the Makefile.
>
> Did this. Additionally, I moved OMAP3 NAND driver to
> drivers/mtd/nand/omap3.c.
I would recommend having a nand_omap_gpmc.c if the driver can be made
generic enough.
>
>>> +/*
>>> + * nand_read_buf16 - [DEFAULT] read chip data into buffer
>>> + * @mtd:    MTD device structure
>>> + * @buf:    buffer to store date
>>> + * @len:    number of bytes to read
>>> + *
>>> + * Default read function for 16bit buswith
>>> + */
>>> +static void omap_nand_read_buf(struct mtd_info *mtd, u_char *buf,
>>> int len)
>>> +{
>>> +    int i;
>>> +    struct nand_chip *this = mtd->priv;
>>> +    u16 *p = (u16 *) buf;
>>> +    len >>= 1;
>>> +
>>> +    for (i = 0; i < len; i++)
>>> +        p[i] = readw(this->IO_ADDR_R);
>>> +}
>>
>>
>> How does this differ from the default implementation?
>
> It doesn't differ ;)
>
> So I removed this and tried to use default nand_read_buf16() instead:
>
> nand->read_buf = nand_read_buf16;
>
> in board_nand_init(). But this doesn't compile as nand_read_buf16() is
> static in nand_base.c. How do I use it in OMAP3 NAND driver? Marked it
> as FIXME in patch.
Probably does not need an explicit initialization, mtd nand_scan should
populate that. in fact IMHO, the read/write buf ops might be duplicating
mtd's implementation..  :(

>
>
>>> +/*
>>> + *  omap_calculate_ecc - Generate non-inverted ECC bytes.
>>> + *
>>> + *  Using noninverted ECC can be considered ugly since writing a blank
>>> + *  page ie. padding will clear the ECC bytes. This is no problem as
>>> + *  long nobody is trying to write data on the seemingly unused page.
>>> + *  Reading an erased page will produce an ECC mismatch between
>>> + *  generated and read ECC bytes that has to be dealt with separately.
>>
>>
>> Is this a hardware limitation?  If so, say so in the comment.  If not,
>> why do it this way?
>
> Don't know.
>
> All: Any help?
The issue is simple: assume we read a page of 0xFF's(fresh erased), IF
using H/w ECC engine within GPMC, the result of read will be 0x0 while
the ECC offsets of the spare area will be 0xFF which will result in an
ECC mismatch.
>>> +    .eccbytes = 12,
>>> +    .eccpos = {
>>> +           2, 3, 4, 5,
>>> +           6, 7, 8, 9,
>>> +           10, 11, 12, 13},
>>> +    .oobfree = { {20, 50} }    /* don't care */
>>
>>
>> Bytes 64-69 of a 64-byte OOB are free?
>> What don't we care about?
> +static struct nand_ecclayout hw_nand_oob_64 = {
>
> Don't know (or understand?).
>
> All: Any help?
I do not get it either.. ECCPOS is in offset bytes, and oobfree should
be {.offset=20,.length=44} /*I always hated struct initialization done
as above..*/, but then,
>
>>> ===================================================================
>>> --- u-boot-arm.orig/drivers/mtd/nand/nand_base.c
>>> +++ u-boot-arm/drivers/mtd/nand/nand_base.c
>>> @@ -2730,6 +2730,151 @@ int nand_scan_tail(struct mtd_info *mtd)
>>>     return 0;
>>> }
>>>
>>> +#if defined(CONFIG_OMAP) && (defined(CONFIG_OMAP3_BEAGLE) \
>>> +    || defined(CONFIG_OMAP3_EVM) || defined(CONFIG_OVERO))
>>> +void nand_switch_ecc(struct mtd_info *mtd)
>>
>>
>> NACK.  First, explain why you need to be able to dynamically switch ECC
>> modes.
>
> Two topics here, changes in cmd_nand.c and nand_base.c.
>
> cmd_nand.c:
>
> We need to be able to switch ECC at runtime cause some images have to
> be written to NAND with HW ECC and some with SW ECC. This depends on
> what user (reader) of these parts expect.
>
> OMAP3 has a boot ROM which is able to read a second level loader
> (called x-loader) from NAND and start/execute it. This 2nd level
> loader has to be written by U-Boot using HW ECC as ROM code does HW
> ECC to read the image. All other parts, e.g. Linux kernel, use SW ECC
> as default. For this we have to use SW ECC to write images, then.
>
> Therefore we add an additional user command in cmd_nand.c to switch
> ECC depending on what user wants to write.
why not use h/w ecc which rom code understands in kernel and u-boot. H/w
ecc will be faster in comparison to doing s/w ecc and there is good
support in MTD to do it, then there would be no reason for s/w ecc IMHO..

Regards,
Nishanth Menon

[U-Boot] [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib common files, add NAND support

[Scott Wood]

On Tue, Oct 07, 2008 at 11:42:38AM +0200, Dirk Behme wrote:
>> Is it OK if config gets written before control, or if this whole thing
>> gets done out of order with respect to other raw writes?
>
> Hmm. I replaced this with the version from Nishanth's U-Boot v2 (see  
> link above). If this isn't ok, can you explain a little more?

I was referring to the raw accessors, which you removed, so it's OK.

> I.e. with patch in attachment we don't touch nand_base.c any more, but  
> still have an #ifdeffed user command in cmd_nand.c. If you don't like  
> this, please give a hint how to better do this.

If at all possible, fix Linux to accept hardware ECC.

Otherwise, either add something to the MTD API that the nand command can
call, or have your own board-specific command defined in the board file. 
Don't put platform-specific ifdefs in generic files.

-Scott

[U-Boot] [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib common files, add NAND support

[Scott Wood]

On Tue, Oct 07, 2008 at 06:25:11AM -0500, Nishanth Menon wrote:
> Dirk Behme said the following on 10/07/2008 04:42 AM:
> > It doesn't differ ;)
> >
> > So I removed this and tried to use default nand_read_buf16() instead:
> >
> > nand->read_buf = nand_read_buf16;
> >
> > in board_nand_init(). But this doesn't compile as nand_read_buf16() is
> > static in nand_base.c. How do I use it in OMAP3 NAND driver? Marked it
> > as FIXME in patch.
> Probably does not need an explicit initialization, mtd nand_scan should
> populate that.

Correct, NULL methods will be filled in with defaults if applicable.

> >>> +/*
> >>> + *  omap_calculate_ecc - Generate non-inverted ECC bytes.
> >>> + *
> >>> + *  Using noninverted ECC can be considered ugly since writing a blank
> >>> + *  page ie. padding will clear the ECC bytes. This is no problem as
> >>> + *  long nobody is trying to write data on the seemingly unused page.
> >>> + *  Reading an erased page will produce an ECC mismatch between
> >>> + *  generated and read ECC bytes that has to be dealt with separately.
> >>
> >>
> >> Is this a hardware limitation?  If so, say so in the comment.  If not,
> >> why do it this way?
> >
> > Don't know.
> >
> > All: Any help?
> The issue is simple: assume we read a page of 0xFF's(fresh erased), IF
> using H/w ECC engine within GPMC, the result of read will be 0x0 while
> the ECC offsets of the spare area will be 0xFF which will result in an
> ECC mismatch.

Right, I'd just like to see an explicit statement that this is the only way
to do HW ECC that the hardware supports (following verification of that
fact, of course), along with a pointer to where in the code the ECC error
when reading an empty page is dealt with.

> >>> +    .eccbytes = 12,
> >>> +    .eccpos = {
> >>> +           2, 3, 4, 5,
> >>> +           6, 7, 8, 9,
> >>> +           10, 11, 12, 13},
> >>> +    .oobfree = { {20, 50} }    /* don't care */
> >>
> >>
> >> Bytes 64-69 of a 64-byte OOB are free?
> >> What don't we care about?
> > +static struct nand_ecclayout hw_nand_oob_64 = {
> >
> > Don't know (or understand?).
> >
> > All: Any help?
> I do not get it either.. ECCPOS is in offset bytes, and oobfree should
> be {.offset=20,.length=44} /*I always hated struct initialization done
> as above..*/, but then,

Why not offset 14, length 50?

> > We need to be able to switch ECC at runtime cause some images have to
> > be written to NAND with HW ECC and some with SW ECC. This depends on
> > what user (reader) of these parts expect.
> >
> > OMAP3 has a boot ROM which is able to read a second level loader
> > (called x-loader) from NAND and start/execute it. This 2nd level
> > loader has to be written by U-Boot using HW ECC as ROM code does HW
> > ECC to read the image. All other parts, e.g. Linux kernel, use SW ECC
> > as default. For this we have to use SW ECC to write images, then.
> >
> > Therefore we add an additional user command in cmd_nand.c to switch
> > ECC depending on what user wants to write.
> why not use h/w ecc which rom code understands in kernel and u-boot. H/w
> ecc will be faster in comparison to doing s/w ecc and there is good
> support in MTD to do it, then there would be no reason for s/w ecc IMHO..

Agreed.

-Scott

[U-Boot] [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib common files, add NAND support

[Menon, Nishanth]

> -----Original Message-----
> From: Scott Wood [mailto:scottwood at freescale.com]
> Sent: Tuesday, October 07, 2008 12:30 PM
> To: Nishanth Menon
> Cc: Dirk Behme; u-boot at lists.denx.de; Kamat, Nishant; Menon, Nishanth
> Subject: Re: [U-Boot] [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib
> common files, add NAND support
> > >>> +    .eccbytes = 12,
> > >>> +    .eccpos = {
> > >>> +           2, 3, 4, 5,
> > >>> +           6, 7, 8, 9,
> > >>> +           10, 11, 12, 13},
> > >>> +    .oobfree = { {20, 50} }    /* don't care */
> > >>
> > >>
> > >> Bytes 64-69 of a 64-byte OOB are free?
> > >> What don't we care about?
> > > +static struct nand_ecclayout hw_nand_oob_64 = {
> > >
> > > Don't know (or understand?).
> > >
> > > All: Any help?
> > I do not get it either.. ECCPOS is in offset bytes, and oobfree should
> > be {.offset=20,.length=44} /*I always hated struct initialization done
> > as above..*/, but then,
> 
> Why not offset 14, length 50?
How about this part being used by ubi/jffs2 or some fs.. I cant think off-hand if there an implication, but yep, I suspect there is no reason why not..

Regards,
Nishanth Menon

[U-Boot] [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib common files, add NAND support

[Scott Wood]

Menon, Nishanth wrote:
>>> I do not get it either.. ECCPOS is in offset bytes, and oobfree should
>>> be {.offset=20,.length=44} /*I always hated struct initialization done
>>> as above..*/, but then,
>> Why not offset 14, length 50?
> How about this part being used by ubi/jffs2 or some fs.. I cant think off-hand if there an implication, but yep, I suspect there is no reason why not..

The higher levels allocate their OOB usage from the regions that the 
low-level driver claims are free.

-Scott

[U-Boot] [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib common files, add NAND support

[Dirk Behme]

Scott Wood wrote:
> On Tue, Oct 07, 2008 at 11:42:38AM +0200, Dirk Behme wrote:
> 
>>>Is it OK if config gets written before control, or if this whole thing
>>>gets done out of order with respect to other raw writes?
>>
>>Hmm. I replaced this with the version from Nishanth's U-Boot v2 (see  
>>link above). If this isn't ok, can you explain a little more?
> 
> 
> I was referring to the raw accessors, which you removed, so it's OK.

:)

>>I.e. with patch in attachment we don't touch nand_base.c any more, but  
>>still have an #ifdeffed user command in cmd_nand.c. If you don't like  
>>this, please give a hint how to better do this.
> 
> 
> If at all possible, fix Linux to accept hardware ECC.

Yes, as mentioned by Nishanth, too, this will be possible. But I don't 
think in the short term, and not with breaking compatibility with 
kernels already out there. So for the moment I like to have both 
options, HW & SW ECC.

> Otherwise, either add something to the MTD API that the nand command can
> call, or have your own board-specific command defined in the board file. 
> Don't put platform-specific ifdefs in generic files.

Do you have any example how to extend MTD API and/or can link to 
example where board-specific command is already used?

Thanks

Dirk

[U-Boot] [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib common files, add NAND support

[Dirk Behme]

Scott Wood wrote:
> On Tue, Oct 07, 2008 at 06:25:11AM -0500, Nishanth Menon wrote:
> 
>>Dirk Behme said the following on 10/07/2008 04:42 AM:
>>
>>>It doesn't differ ;)
>>>
>>>So I removed this and tried to use default nand_read_buf16() instead:
>>>
>>>nand->read_buf = nand_read_buf16;
>>>
>>>in board_nand_init(). But this doesn't compile as nand_read_buf16() is
>>>static in nand_base.c. How do I use it in OMAP3 NAND driver? Marked it
>>>as FIXME in patch.
>>
>>Probably does not need an explicit initialization, mtd nand_scan should
>>populate that.
> 
> 
> Correct, NULL methods will be filled in with defaults if applicable.

ok, will do so, this is easy :)

>>>>>+/*
>>>>>+ *  omap_calculate_ecc - Generate non-inverted ECC bytes.
>>>>>+ *
>>>>>+ *  Using noninverted ECC can be considered ugly since writing a blank
>>>>>+ *  page ie. padding will clear the ECC bytes. This is no problem as
>>>>>+ *  long nobody is trying to write data on the seemingly unused page.
>>>>>+ *  Reading an erased page will produce an ECC mismatch between
>>>>>+ *  generated and read ECC bytes that has to be dealt with separately.
>>>>
>>>>
>>>>Is this a hardware limitation?  If so, say so in the comment.  If not,
>>>>why do it this way?
>>>
>>>Don't know.
>>>
>>>All: Any help?
>>
>>The issue is simple: assume we read a page of 0xFF's(fresh erased), IF
>>using H/w ECC engine within GPMC, the result of read will be 0x0 while
>>the ECC offsets of the spare area will be 0xFF which will result in an
>>ECC mismatch.
> 
> Right, I'd just like to see an explicit statement that this is the only way
> to do HW ECC that the hardware supports (following verification of that
> fact, of course), along with a pointer to where in the code the ECC error
> when reading an empty page is dealt with.

Will add Nishanth's explanation to comment and check code for this.

>>>>>+    .eccbytes = 12,
>>>>>+    .eccpos = {
>>>>>+           2, 3, 4, 5,
>>>>>+           6, 7, 8, 9,
>>>>>+           10, 11, 12, 13},
>>>>>+    .oobfree = { {20, 50} }    /* don't care */
>>>>
>>>>
>>>>Bytes 64-69 of a 64-byte OOB are free?
>>>>What don't we care about?
>>>
>>>+static struct nand_ecclayout hw_nand_oob_64 = {
>>>
>>>Don't know (or understand?).
>>>
>>>All: Any help?
>>
>>I do not get it either.. ECCPOS is in offset bytes, and oobfree should
>>be {.offset=20,.length=44} /*I always hated struct initialization done
>>as above..*/, but then,
> 
> 
> Why not offset 14, length 50?

Seems I need a closer look what we are talking about here ;)

>>>We need to be able to switch ECC at runtime cause some images have to
>>>be written to NAND with HW ECC and some with SW ECC. This depends on
>>>what user (reader) of these parts expect.
>>>
>>>OMAP3 has a boot ROM which is able to read a second level loader
>>>(called x-loader) from NAND and start/execute it. This 2nd level
>>>loader has to be written by U-Boot using HW ECC as ROM code does HW
>>>ECC to read the image. All other parts, e.g. Linux kernel, use SW ECC
>>>as default. For this we have to use SW ECC to write images, then.
>>>
>>>Therefore we add an additional user command in cmd_nand.c to switch
>>>ECC depending on what user wants to write.
>>
>>why not use h/w ecc which rom code understands in kernel and u-boot. H/w
>>ecc will be faster in comparison to doing s/w ecc and there is good
>>support in MTD to do it, then there would be no reason for s/w ecc IMHO..
> 
> 
> Agreed.

As already mentioned in previous post, I think for the the moment we 
have to go with both ways.

To summarize the open points I will look at:

a) Remove unnecessary nand->read_buf init
b) Add comment and check code for HW ecc issue with erased page
c) Fix offset 14, length 50 issue
d) Extend MTD API with a call to switch HW/SW ecc, remove 
platform-specific ifdefs in generic files

Nishanth M.: Would be nice to get some help for these changes. E.g. 
can you say if this HW ecc issue with erased page is handled correctly 
in exisiting code?

Thanks for the help

Dirk

[U-Boot] [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib common files, add NAND support

[Scott Wood]

Dirk Behme wrote:
>> Otherwise, either add something to the MTD API that the nand command can
>> call, or have your own board-specific command defined in the board file. 
>> Don't put platform-specific ifdefs in generic files.
> 
> Do you have any example how to extend MTD API

You'd need to add callbacks in the device structures; since this is a 
workaround for legacy support, and not something that is desireable in 
most cases, I'd go with the latter option of a board-specific command.

> and/or can link to 
> example where board-specific command is already used?

grep -rI U_BOOT_CMD board/

-Scott

[U-Boot] [PATCH 07/12 v2] ARM: OMAP3: Add memory and syslib common files, add NAND support

[Dirk Behme]

Dirk Behme wrote:
> Scott Wood wrote:
> 
>> On Tue, Oct 07, 2008 at 06:25:11AM -0500, Nishanth Menon wrote:
>>
>>> Dirk Behme said the following on 10/07/2008 04:42 AM:
>>>
>>>> It doesn't differ ;)
>>>>
>>>> So I removed this and tried to use default nand_read_buf16() instead:
>>>>
>>>> nand->read_buf = nand_read_buf16;
>>>>
>>>> in board_nand_init(). But this doesn't compile as nand_read_buf16() is
>>>> static in nand_base.c. How do I use it in OMAP3 NAND driver? Marked it
>>>> as FIXME in patch.
>>>
>>>
>>> Probably does not need an explicit initialization, mtd nand_scan should
>>> populate that.
>>
>>
>>
>> Correct, NULL methods will be filled in with defaults if applicable.
> 
> 
> ok, will do so, this is easy :)
> 
>>>>>> +/*
>>>>>> + *  omap_calculate_ecc - Generate non-inverted ECC bytes.
>>>>>> + *
>>>>>> + *  Using noninverted ECC can be considered ugly since writing a 
>>>>>> blank
>>>>>> + *  page ie. padding will clear the ECC bytes. This is no problem as
>>>>>> + *  long nobody is trying to write data on the seemingly unused 
>>>>>> page.
>>>>>> + *  Reading an erased page will produce an ECC mismatch between
>>>>>> + *  generated and read ECC bytes that has to be dealt with 
>>>>>> separately.
>>>>>
>>>>>
>>>>>
>>>>> Is this a hardware limitation?  If so, say so in the comment.  If not,
>>>>> why do it this way?
>>>>
>>>>
>>>> Don't know.
>>>>
>>>> All: Any help?
>>>
>>>
>>> The issue is simple: assume we read a page of 0xFF's(fresh erased), IF
>>> using H/w ECC engine within GPMC, the result of read will be 0x0 while
>>> the ECC offsets of the spare area will be 0xFF which will result in an
>>> ECC mismatch.
>>
>>
>> Right, I'd just like to see an explicit statement that this is the 
>> only way
>> to do HW ECC that the hardware supports (following verification of that
>> fact, of course), along with a pointer to where in the code the ECC error
>> when reading an empty page is dealt with.
> 
> 
> Will add Nishanth's explanation to comment and check code for this.
> 
>>>>>> +    .eccbytes = 12,
>>>>>> +    .eccpos = {
>>>>>> +           2, 3, 4, 5,
>>>>>> +           6, 7, 8, 9,
>>>>>> +           10, 11, 12, 13},
>>>>>> +    .oobfree = { {20, 50} }    /* don't care */
>>>>>
>>>>>
>>>>>
>>>>> Bytes 64-69 of a 64-byte OOB are free?
>>>>> What don't we care about?
>>>>
>>>>
>>>> +static struct nand_ecclayout hw_nand_oob_64 = {
>>>>
>>>> Don't know (or understand?).
>>>>
>>>> All: Any help?
>>>
>>>
>>> I do not get it either.. ECCPOS is in offset bytes, and oobfree should
>>> be {.offset=20,.length=44} /*I always hated struct initialization done
>>> as above..*/, but then,
>>
>>
>>
>> Why not offset 14, length 50?
> 
> 
> Seems I need a closer look what we are talking about here ;)
> 
>>>> We need to be able to switch ECC at runtime cause some images have to
>>>> be written to NAND with HW ECC and some with SW ECC. This depends on
>>>> what user (reader) of these parts expect.
>>>>
>>>> OMAP3 has a boot ROM which is able to read a second level loader
>>>> (called x-loader) from NAND and start/execute it. This 2nd level
>>>> loader has to be written by U-Boot using HW ECC as ROM code does HW
>>>> ECC to read the image. All other parts, e.g. Linux kernel, use SW ECC
>>>> as default. For this we have to use SW ECC to write images, then.
>>>>
>>>> Therefore we add an additional user command in cmd_nand.c to switch
>>>> ECC depending on what user wants to write.
>>>
>>>
>>> why not use h/w ecc which rom code understands in kernel and u-boot. H/w
>>> ecc will be faster in comparison to doing s/w ecc and there is good
>>> support in MTD to do it, then there would be no reason for s/w ecc 
>>> IMHO..
>>
>>
>>
>> Agreed.
> 
> 
> As already mentioned in previous post, I think for the the moment we 
> have to go with both ways.
> 
> To summarize the open points I will look at:
> 
> a) Remove unnecessary nand->read_buf init

Removed in attachment

> b) Add comment and check code for HW ecc issue with erased page

Added comment. Not sure if we have to do anything in code, though.

> c) Fix offset 14, length 50 issue

Changed in attachment.

> d) Extend MTD API with a call to switch HW/SW ecc, remove 
> platform-specific ifdefs in generic files

Removed nand ecc command from cmd_nand.c and added "nandecc" command 
to board file (Scott: Thanks for the hint!):

--- /dev/null
+++ u-boot-arm/cpu/arm_cortexa8/omap3/board.c
...
+#ifdef CONFIG_NAND_OMAP3
+/******************************************************************************
+ * OMAP3 specific command to switch between NAND HW and SW ecc
+ 
*****************************************************************************/
+static int do_switch_ecc(cmd_tbl_t * cmdtp, int flag, int argc, char 
*argv[])
+{
+	if (argc != 2)
+		goto usage;
+	if (strncmp(argv[1], "hw", 2) == 0)
+		omap_nand_switch_ecc(1);
+	else if (strncmp(argv[1], "sw", 2) == 0)
+		omap_nand_switch_ecc(0);
+	else
+		goto usage;
+
+	return 0;
+
+usage:
+	printf ("Usage: nandecc %s\n", cmdtp->help);
+	return 1;
+}
+
+U_BOOT_CMD(
+	nandecc, 2, 1,	do_switch_ecc,
+	"nandecc - switch OMAP3 NAND ECC calculation algorithm\n",
+	"[hw/sw] - Switch between NAND hardware (hw) or software (sw) ecc 
algorithm\n"
+	);
+
+#endif /* CONFIG_NAND_OMAP3 */

See attachment for latest version of NAND patch. Please let us know if 
anything else has to be changed. If not, I will prepare OMAP3 v3 patch 
set.

Thanks

Dirk

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 08_omap3_nand.txt
Url: http://lists.denx.de/pipermail/u-boot/attachments/20081007/88d2c45d/attachment-0001.txt