Hi Sylvain, On 05.08.2015 21:31, slemieux.t...@gmail.com wrote: > From: Sylvain Lemieux <slemi...@tycoint.com> > > Incorporate NAND SLC hardware ECC support from legacy > LPCLinux NXP BSP. > The code taken from the legacy patch is: > - lpc32xx SLC NAND driver (hardware ECC support) > - lpc3250 header file missing SLC NAND registers definition > > The legacy driver code was updated to integrate with the existing NAND SLC > driver. > > Signed-off-by: Sylvain Lemieux <slemi...@tycoint.com> > --- > Changes from v4 to v5: > * Addressed Marek's comments on LPC32xx DMA driver: > - Move macro use by the DMA client to write the next > DMA linked list item address to NAND SLC driver. > * Updated multiline comments style. > * Use "uxx" instead of "uintxx_t" (new code), > note: keep uintxx_t for API; to match initial patch. > > Changes from v3 to v4: > * No changes. > > Changes from v2 to v3: > * As per feedback on mailing list from Vladimir, > - add warning when DMA and SPL build option are selected. > - add another DMA specific read and write buffer functions. > * Addressed Marek's comments on LPC32xx NAND driver > removed typecast and update variable type, when possible. > * As per discussion on mailing list with Vladimir & Marek, > updated conditional compile option (DMA only) to build > the original NAND SLC code using software ECC. > * Re-organized the conditional compile code inside > "board_nand_init()" - DMA vs. non-DMA initialization. > * Added original source code credit (copyright & author). > > Changes from v1 to v2: > * Moved the NAND SLC patch as the second patch of the series. > * As per discussion on mailing list with Vladimir, > incorporate NAND SLC hardware ECC support into the following > NAND SLC patch: https://patchwork.ozlabs.org/patch/497308/ > * As per discussion on mailing list with Vladimir & Albert, > add conditional compile option to build the original > NAND SLC code using software ECC for SPL build. > * Removed ECC layout for small page NAND from this patch. > > Update to the legacy code to integrate with the NAND SLC patch: > 1) Fixed checkpatch script output in legacy code. > 2) Use u-boot API for register access to remove the volatile > in register definition taken from "lpc3250.h" header file. > 3) Use register definition from the NAND SLC patch. > > The legacy BSP patch (u-boot-2009.03_lpc32x0-v1.07.patch.tar.bz2) > was downloaded from the LPCLinux Web site. > > drivers/mtd/nand/lpc32xx_nand_slc.c | 369 > +++++++++++++++++++++++++++++++++++- > 1 file changed, 364 insertions(+), 5 deletions(-) > > diff --git a/drivers/mtd/nand/lpc32xx_nand_slc.c > b/drivers/mtd/nand/lpc32xx_nand_slc.c > index 719a74d..715152d 100644 > --- a/drivers/mtd/nand/lpc32xx_nand_slc.c > +++ b/drivers/mtd/nand/lpc32xx_nand_slc.c > @@ -3,15 +3,38 @@ > * > * (C) Copyright 2015 Vladimir Zapolskiy <v...@mleia.com> > * > + * Hardware ECC support original source code > + * Copyright (C) 2008 by NXP Semiconductors > + * Author: Kevin Wells > + * > * SPDX-License-Identifier: GPL-2.0+ > */ > > #include <common.h> > #include <nand.h> > +#include <linux/mtd/nand_ecc.h> > #include <asm/errno.h> > #include <asm/io.h> > #include <asm/arch/clk.h> > #include <asm/arch/sys_proto.h> > +#include <asm/arch/dma.h> > +#include <asm/arch/cpu.h> > + > +#if defined(CONFIG_DMA_LPC32XX) && defined(CONFIG_SPL_BUILD) > +#warning "DMA support in SPL image is not tested" > +#endif > + > +/* Provide default for ECC size / bytes / OOB size for large page) > + * if target did not. */ > +#if !defined(CONFIG_SYS_NAND_ECCSIZE) > +#define CONFIG_SYS_NAND_ECCSIZE 2048 > +#endif
The definition above looks wrong... s/CONFIG_SYS_NAND_ECCSIZE/CONFIG_SYS_NAND_PAGE_SIZE/g CONFIG_SYS_NAND_ECCSIZE is hardcoded to 0x100 for LPC32xx NAND SLC. > +#if !defined(CONFIG_SYS_NAND_ECCBYTES) > +#define CONFIG_SYS_NAND_ECCBYTES 24 > +#endif > +#if !defined(CONFIG_SYS_NAND_OOBSIZE) > +#define CONFIG_SYS_NAND_OOBSIZE 64 > +#endif > > struct lpc32xx_nand_slc_regs { > u32 data; > @@ -33,11 +56,18 @@ struct lpc32xx_nand_slc_regs { > > /* CFG register */ > #define CFG_CE_LOW (1 << 5) > +#define SLCCFG_DMA_ECC (1 << 4) /* Enable DMA ECC bit */ > +#define SLCCFG_ECC_EN (1 << 3) /* ECC enable bit */ > +#define SLCCFG_DMA_BURST (1 << 2) /* DMA burst bit */ > +#define SLCCFG_DMA_DIR (1 << 1) /* DMA write(0)/read(1) bit */ > > /* CTRL register */ > #define CTRL_SW_RESET (1 << 2) > +#define SLCCTRL_ECC_CLEAR (1 << 1) /* Reset ECC bit */ > +#define SLCCTRL_DMA_START (1 << 0) /* Start DMA channel bit */ > > /* STAT register */ > +#define SLCSTAT_DMA_FIFO (1 << 2) /* DMA FIFO has data bit */ > #define STAT_NAND_READY (1 << 0) Do you see any chance of confusion, if "SLC" prefix is dropped from new bit field definition? IMO it is more confusing, if there are two styles of them at once. > /* INT_STAT register */ > @@ -54,6 +84,59 @@ struct lpc32xx_nand_slc_regs { > #define TAC_R_HOLD(n) (max_t(uint32_t, (n), 0xF) << 4) > #define TAC_R_SETUP(n) (max_t(uint32_t, (n), 0xF) << 0) > > +/* control register definitions */ > +#define DMAC_CHAN_INT_TC_EN (1 << 31) /* channel terminal count interrupt */ > +#define DMAC_CHAN_DEST_AUTOINC (1 << 27) /* automatic destination > increment */ > +#define DMAC_CHAN_SRC_AUTOINC (1 << 26) /* automatic source increment > */ > +#define DMAC_CHAN_DEST_AHB1 (1 << 25) /* AHB1 master for dest. transfer */ > +#define DMAC_CHAN_DEST_WIDTH_32 (1 << 22) /* Destination data width > selection */ > +#define DMAC_CHAN_SRC_WIDTH_32 (1 << 19) /* Source data width > selection */ > +#define DMAC_CHAN_DEST_BURST_1 0 > +#define DMAC_CHAN_DEST_BURST_4 (1 << 15) /* Destination data burst > size */ > +#define DMAC_CHAN_SRC_BURST_1 0 > +#define DMAC_CHAN_SRC_BURST_4 (1 << 12) /* Source data burst size */ > + > +/* > + * config_ch register definitions > + * DMAC_CHAN_FLOW_D_xxx: flow control with DMA as the controller > + * DMAC_DEST_PERIP: Macro for loading destination peripheral > + * DMAC_SRC_PERIP: Macro for loading source peripheral > + */ > +#define DMAC_CHAN_FLOW_D_M2P (0x1 << 11) > +#define DMAC_CHAN_FLOW_D_P2M (0x2 << 11) > +#define DMAC_DEST_PERIP(n) (((n) & 0x1F) << 6) > +#define DMAC_SRC_PERIP(n) (((n) & 0x1F) << 1) > + > +/* > + * config_ch register definitions > + * (source and destination peripheral ID numbers). > + * These can be used with the DMAC_DEST_PERIP and DMAC_SRC_PERIP macros. > + */ > +#define DMA_PERID_NAND1 1 > + > +/* Channel enable bit */ > +#define DMAC_CHAN_ENABLE (1 << 0) All these DMA specific macro definitions above should go to arch/dma.h added in 1/5, this is not the right place for any of them. > +#define NAND_LARGE_BLOCK_PAGE_SIZE 2048 NB! See NB below. This macro is used only in (CONFIG_SYS_NAND_ECCSIZE == NAND_LARGE_BLOCK_PAGE_SIZE ? 8 : 2) cases, and actually the resulting value is always (CONFIG_SYS_NAND_ECCSIZE / 0x100) Here I would propose * to add a sanity check in board_nand_init() for (CONFIG_SYS_NAND_ECCSIZE == NAND_LARGE_BLOCK_PAGE_SIZE || CONFIG_SYS_NAND_ECCSIZE == NAND_SMALL_BLOCK_PAGE_SIZE), * to replace the tristate operator case above with (CONFIG_SYS_NAND_ECCSIZE / 0x100). NB! Here CONFIG_SYS_NAND_ECCSIZE is misused, it must be replaced by CONFIG_SYS_NAND_PAGE_SIZE, and 0x100 must be replaced by CONFIG_SYS_NAND_ECCSIZE. In comments I keep the old notation to reduce confusion, but please change the names. > +#define NAND_SMALL_BLOCK_PAGE_SIZE 512 This macro is not used at the moment, but see the comment above. > +#if defined(CONFIG_DMA_LPC32XX) > +/* > + * DMA Descriptors > + * For Large Block: 17 descriptors = ((16 Data and ECC Read) + 1 Spare Area) > + * For Small Block: 5 descriptors = ((4 Data and ECC Read) + 1 Spare Area) > + */ > +static struct lpc32xx_dmac_ll dmalist[(CONFIG_SYS_NAND_ECCSIZE/256) * 2 + 1]; Aha, I see CONFIG_SYS_NAND_ECCSIZE / 0x100 again, it deserves its own macro. > +static u32 ecc_buffer[8]; /* MAX ECC size */ And this 8 again stands for CONFIG_SYS_NAND_ECCSIZE / 0x100. Well, you remember it must be (CONFIG_SYS_NAND_PAGE_SIZE / CONFIG_SYS_NAND_ECCSIZE) in the corrected version, e.g. for small page devices it is 2. > +static int dmachan = -1; > + > +/* > + * Helper macro for the DMA client (i.e. NAND SLC) to write the next > + * DMA linked list item address (see arch/include/asm/arch-lpc32xx/dma.h). > + */ > +#define lpc32xx_dmac_next_lli(x) ((u32)x) > +#endif > + > static struct lpc32xx_nand_slc_regs __iomem *lpc32xx_nand_slc_regs > = (struct lpc32xx_nand_slc_regs __iomem *)SLC_NAND_BASE; > > @@ -108,15 +191,266 @@ static int lpc32xx_nand_dev_ready(struct mtd_info *mtd) > return readl(&lpc32xx_nand_slc_regs->stat) & STAT_NAND_READY; > } > > -static void lpc32xx_read_buf(struct mtd_info *mtd, uint8_t *buf, int len) > +#if defined(CONFIG_DMA_LPC32XX) > +/* > + * Prepares DMA descriptors for NAND RD/WR operations > + * If the size is < 256 Bytes then it is assumed to be > + * an OOB transfer > + */ > +static void lpc32xx_nand_dma_configure(struct nand_chip *chip, > + const u8 *buffer, int size, > + int read) > { > - while (len-- > 0) > - *buf++ = readl(&lpc32xx_nand_slc_regs->data); > + u32 i, dmasrc, ctrl, ecc_ctrl, oob_ctrl, dmadst; > + u32 base = (u32)chip->IO_ADDR_R; This is SLC_NAND_BASE, but the variable is not needed, see below. > + struct lpc32xx_dmac_ll *dmalist_cur; > + struct lpc32xx_dmac_ll *dmalist_cur_ecc; > + > + /* > + * CTRL descriptor entry for reading ECC > + * Copy Multiple times to sync DMA with Flash Controller > + */ > + ecc_ctrl = 0x5 | > + DMAC_CHAN_SRC_BURST_1 | > + DMAC_CHAN_DEST_BURST_1 | > + DMAC_CHAN_SRC_WIDTH_32 | > + DMAC_CHAN_DEST_WIDTH_32 | > + DMAC_CHAN_DEST_AHB1; > + > + /* CTRL descriptor entry for reading/writing Data */ > + ctrl = 64 | /* 256/4 */ > + DMAC_CHAN_SRC_BURST_4 | > + DMAC_CHAN_DEST_BURST_4 | > + DMAC_CHAN_SRC_WIDTH_32 | > + DMAC_CHAN_DEST_WIDTH_32 | > + DMAC_CHAN_DEST_AHB1; > + > + /* CTRL descriptor entry for reading/writing Spare Area */ > + oob_ctrl = (CONFIG_SYS_NAND_OOBSIZE / 4) | > + DMAC_CHAN_SRC_BURST_4 | > + DMAC_CHAN_DEST_BURST_4 | > + DMAC_CHAN_SRC_WIDTH_32 | > + DMAC_CHAN_DEST_WIDTH_32 | > + DMAC_CHAN_DEST_AHB1; > + > + if (read) { > + dmasrc = base + offsetof(struct lpc32xx_nand_slc_regs, > + dma_data); This is &lpc32xx_nand_slc_regs->dma_data. > + dmadst = (u32)buffer; > + ctrl |= DMAC_CHAN_DEST_AUTOINC; > + } else { > + dmadst = base + offsetof(struct lpc32xx_nand_slc_regs, > + dma_data); This is &lpc32xx_nand_slc_regs->dma_data. > + dmasrc = (u32)buffer; > + ctrl |= DMAC_CHAN_SRC_AUTOINC; > + } > + > + /* > + * Write Operation Sequence for Small Block NAND > + * ---------------------------------------------------------- > + * 1. X'fer 256 bytes of data from Memory to Flash. > + * 2. Copy generated ECC data from Register to Spare Area > + * 3. X'fer next 256 bytes of data from Memory to Flash. > + * 4. Copy generated ECC data from Register to Spare Area. > + * 5. X'fer 16 byets of Spare area from Memory to Flash. > + * Read Operation Sequence for Small Block NAND > + * ---------------------------------------------------------- > + * 1. X'fer 256 bytes of data from Flash to Memory. > + * 2. Copy generated ECC data from Register to ECC calc Buffer. > + * 3. X'fer next 256 bytes of data from Flash to Memory. > + * 4. Copy generated ECC data from Register to ECC calc Buffer. > + * 5. X'fer 16 bytes of Spare area from Flash to Memory. > + * Write Operation Sequence for Large Block NAND > + * ---------------------------------------------------------- > + * 1. Steps(1-4) of Write Operations repeate for four times > + * which generates 16 DMA descriptors to X'fer 2048 bytes of > + * data & 32 bytes of ECC data. > + * 2. X'fer 64 bytes of Spare area from Memory to Flash. > + * Read Operation Sequence for Large Block NAND > + * ---------------------------------------------------------- > + * 1. Steps(1-4) of Read Operations repeate for four times > + * which generates 16 DMA descriptors to X'fer 2048 bytes of > + * data & 32 bytes of ECC data. > + * 2. X'fer 64 bytes of Spare area from Flash to Memory. > + */ > + > + for (i = 0; i < size/256; i++) { > + dmalist_cur = &dmalist[i * 2]; > + dmalist_cur_ecc = &dmalist[(i * 2) + 1]; > + > + dmalist_cur->dma_src = (read ? (dmasrc) : (dmasrc + (i*256))); > + dmalist_cur->dma_dest = (read ? (dmadst + (i*256)) : dmadst); > + dmalist_cur->next_lli = lpc32xx_dmac_next_lli(dmalist_cur_ecc); > + dmalist_cur->next_ctrl = ctrl; > + > + dmalist_cur_ecc->dma_src = > + base + offsetof(struct lpc32xx_nand_slc_regs, ecc); > + dmalist_cur_ecc->dma_dest = (u32)&ecc_buffer[i]; > + dmalist_cur_ecc->next_lli = > + lpc32xx_dmac_next_lli(&dmalist[(i * 2) + 2]); > + dmalist_cur_ecc->next_ctrl = ecc_ctrl; > + } > + > + if (i) { /* Data only transfer */ > + dmalist_cur_ecc = &dmalist[(i * 2) - 1]; > + dmalist_cur_ecc->next_lli = 0; > + dmalist_cur_ecc->next_ctrl |= DMAC_CHAN_INT_TC_EN; > + return; > + } > + > + /* OOB only transfer */ > + if (read) { > + dmasrc = base + offsetof(struct lpc32xx_nand_slc_regs, > + dma_data); This is &lpc32xx_nand_slc_regs->dma_data. > + dmadst = (u32)buffer; > + oob_ctrl |= DMAC_CHAN_DEST_AUTOINC; > + } else { > + dmadst = base + offsetof(struct lpc32xx_nand_slc_regs, > + dma_data); This is &lpc32xx_nand_slc_regs->dma_data. No "base" variable please. > + dmasrc = (u32)buffer; > + oob_ctrl |= DMAC_CHAN_SRC_AUTOINC; > + } > + > + /* Read/ Write Spare Area Data To/From Flash */ > + dmalist_cur = &dmalist[i * 2]; > + dmalist_cur->dma_src = dmasrc; > + dmalist_cur->dma_dest = dmadst; > + dmalist_cur->next_lli = 0; > + dmalist_cur->next_ctrl = (oob_ctrl | DMAC_CHAN_INT_TC_EN); > } > > -static uint8_t lpc32xx_read_byte(struct mtd_info *mtd) > +static void lpc32xx_nand_xfer(struct mtd_info *mtd, const u8 *buf, > + int len, int read) > { > - return readl(&lpc32xx_nand_slc_regs->data); > + struct nand_chip *chip = mtd->priv; > + u32 config; > + > + /* DMA Channel Configuration */ > + config = (read ? DMAC_CHAN_FLOW_D_P2M : DMAC_CHAN_FLOW_D_M2P) | > + (read ? DMAC_DEST_PERIP(0) : DMAC_DEST_PERIP(DMA_PERID_NAND1)) | > + (read ? DMAC_SRC_PERIP(DMA_PERID_NAND1) : DMAC_SRC_PERIP(0)) | > + DMAC_CHAN_ENABLE; > + > + /* Prepare DMA descriptors */ > + lpc32xx_nand_dma_configure(chip, buf, len, read); > + > + /* Setup SLC controller and start transfer */ > + if (read) > + setbits_le32(&lpc32xx_nand_slc_regs->cfg, SLCCFG_DMA_DIR); > + else /* NAND_ECC_WRITE */ > + clrbits_le32(&lpc32xx_nand_slc_regs->cfg, SLCCFG_DMA_DIR); > + setbits_le32(&lpc32xx_nand_slc_regs->cfg, SLCCFG_DMA_BURST); > + > + /* Write length for new transfers */ > + if (!((readl(&lpc32xx_nand_slc_regs->stat) & SLCSTAT_DMA_FIFO) | > + readl(&lpc32xx_nand_slc_regs->tc))) { > + int tmp = (len != mtd->oobsize) ? mtd->oobsize : 0; > + writel(len + tmp, &lpc32xx_nand_slc_regs->tc); > + } > + > + setbits_le32(&lpc32xx_nand_slc_regs->ctrl, SLCCTRL_DMA_START); > + > + /* Start DMA transfers */ > + lpc32xx_dma_start_xfer(dmachan, dmalist, config); > + > + /* Wait for NAND to be ready */ > + while (!lpc32xx_nand_dev_ready(mtd)) > + ; > + > + /* Wait till DMA transfer is DONE */ > + if (lpc32xx_dma_wait_status(dmachan)) > + pr_err("NAND DMA transfer error!\r\n"); > + > + /* Stop DMA & HW ECC */ > + clrbits_le32(&lpc32xx_nand_slc_regs->ctrl, SLCCTRL_DMA_START); > + clrbits_le32(&lpc32xx_nand_slc_regs->cfg, > + SLCCFG_DMA_DIR | SLCCFG_DMA_BURST | > + SLCCFG_ECC_EN | SLCCFG_DMA_ECC); > +} > + > +static u32 slc_ecc_copy_to_buffer(u8 *spare, const u32 *ecc, int count) > +{ > + int i; > + for (i = 0; i < (count * 3); i += 3) { > + u32 ce = ecc[i/3]; > + ce = ~(ce << 2) & 0xFFFFFF; > + spare[i+2] = (u8)(ce & 0xFF); ce >>= 8; > + spare[i+1] = (u8)(ce & 0xFF); ce >>= 8; > + spare[i] = (u8)(ce & 0xFF); > + } > + return 0; > +} > + > +static int lpc32xx_ecc_calculate(struct mtd_info *mtd, const uint8_t *dat, > + uint8_t *ecc_code) > +{ > + return slc_ecc_copy_to_buffer(ecc_code, ecc_buffer, > + CONFIG_SYS_NAND_ECCSIZE > + == NAND_LARGE_BLOCK_PAGE_SIZE ? 8 : 2); > +} > + > +/* > + * Enables and prepares SLC NAND controller > + * for doing data transfers with H/W ECC enabled. > + */ > +static void lpc32xx_hwecc_enable(struct mtd_info *mtd, int mode) > +{ > + /* Clear ECC */ > + writel(SLCCTRL_ECC_CLEAR, &lpc32xx_nand_slc_regs->ctrl); > + > + /* Setup SLC controller for H/W ECC operations */ > + setbits_le32(&lpc32xx_nand_slc_regs->cfg, > + SLCCFG_ECC_EN | SLCCFG_DMA_ECC); > +} > + > +/* > + * lpc32xx_correct_data - [NAND Interface] Detect and correct bit error(s) > + * mtd: MTD block structure > + * dat: raw data read from the chip > + * read_ecc: ECC from the chip > + * calc_ecc: the ECC calculated from raw data > + * > + * Detect and correct a 1 bit error for 256 byte block > + */ > +int lpc32xx_correct_data(struct mtd_info *mtd, u_char *dat, > + u_char *read_ecc, u_char *calc_ecc) > +{ > + u8 i, nb_ecc256; > + int ret1, ret2 = 0; > + u_char *r = read_ecc; > + u_char *c = calc_ecc; > + u16 data_offset = 0; > + > + nb_ecc256 = (CONFIG_SYS_NAND_ECCSIZE == NAND_LARGE_BLOCK_PAGE_SIZE > + ? 8 : 2); > + > + for (i = 0 ; i < nb_ecc256 ; i++ , r += 3, c += 3, data_offset += 256) { > + ret1 = nand_correct_data(mtd, dat + data_offset, r, c); > + > + if (ret1 < 0) > + return -EBADMSG; > + else > + ret2 += ret1; > + } > + > + return ret2; > +} > + > +static void lpc32xx_dma_read_buf(struct mtd_info *mtd, uint8_t *buf, int len) > +{ > + lpc32xx_nand_xfer(mtd, buf, len, 1); > +} > + > +static void lpc32xx_dma_write_buf(struct mtd_info *mtd, const uint8_t *buf, > + int len) > +{ > + lpc32xx_nand_xfer(mtd, buf, len, 0); > +} > +#else > +static void lpc32xx_read_buf(struct mtd_info *mtd, uint8_t *buf, int len) > +{ > + while (len-- > 0) > + *buf++ = readl(&lpc32xx_nand_slc_regs->data); > } Please preserve the original order: lpc32xx_read_buf() lpc32xx_read_byte() lpc32xx_write_buf() lpc32xx_write_byte() I'd like to see $OP_byte() follows $OP_buf() as a motivator to measure performance degradation one day, if $OP_byte() body is replaced by $OP_buf(..., 1). > static void lpc32xx_write_buf(struct mtd_info *mtd, const uint8_t *buf, int > len) > @@ -124,6 +458,12 @@ static void lpc32xx_write_buf(struct mtd_info *mtd, > const uint8_t *buf, int len) > while (len-- > 0) > writel(*buf++, &lpc32xx_nand_slc_regs->data); > } > +#endif > + > +static uint8_t lpc32xx_read_byte(struct mtd_info *mtd) > +{ > + return readl(&lpc32xx_nand_slc_regs->data); > +} > > static void lpc32xx_write_byte(struct mtd_info *mtd, uint8_t byte) > { > @@ -137,9 +477,45 @@ static void lpc32xx_write_byte(struct mtd_info *mtd, > uint8_t byte) > */ > int board_nand_init(struct nand_chip *lpc32xx_chip) > { > +#if defined(CONFIG_DMA_LPC32XX) > + /* Acquire a channel for our use */ > + dmachan = lpc32xx_dma_get_channel(); > + if (unlikely(dmachan < 0)) { > + pr_info("Unable to get free DMA channel for NAND transfers\n"); > + return -1; > + } > +#endif > + > lpc32xx_chip->cmd_ctrl = lpc32xx_nand_cmd_ctrl; > lpc32xx_chip->dev_ready = lpc32xx_nand_dev_ready; > > +#if defined(CONFIG_DMA_LPC32XX) > + /* > + * Hardware ECC calculation is supported when > + * DMA driver is selected. > + */ > + lpc32xx_chip->ecc.mode = NAND_ECC_HW; > + > + /* > + * The implementation of these functions is quite common, but > + * they MUST be defined, because access to data register > + * is strictly 32-bit aligned. > + */ > + lpc32xx_chip->read_byte = lpc32xx_read_byte; > + lpc32xx_chip->write_byte = lpc32xx_write_byte; .read_byte() / .write_byte() are shared, these two lines should be outside of #if defined(CONFIG_DMA_LPC32XX) > + lpc32xx_chip->read_buf = lpc32xx_dma_read_buf; > + lpc32xx_chip->write_buf = lpc32xx_dma_write_buf; > + > + lpc32xx_chip->ecc.calculate = lpc32xx_ecc_calculate; > + lpc32xx_chip->ecc.correct = lpc32xx_correct_data; > + lpc32xx_chip->ecc.hwctl = lpc32xx_hwecc_enable; > + lpc32xx_chip->chip_delay = 2000; > + > + lpc32xx_chip->ecc.bytes = CONFIG_SYS_NAND_ECCBYTES; > + lpc32xx_chip->ecc.size = CONFIG_SYS_NAND_ECCSIZE; > + lpc32xx_chip->ecc.strength = 1; Please double check, because this is wrong, it should be the same as below for PIO: lpc32xx_chip->ecc.size = CONFIG_SYS_NAND_ECCSIZE; lpc32xx_chip->ecc.bytes = CONFIG_SYS_NAND_ECCBYTES / (CONFIG_SYS_NAND_PAGE_SIZE / CONFIG_SYS_NAND_ECCSIZE); lpc32xx_chip->ecc.strength = 1; and therefore this chunk should be placed outside of #if defined(CONFIG_DMA_LPC32XX)/ #else / #endif block. > +#else > /* > * Hardware ECC calculation is not supported by the driver, > * because it requires DMA support, see LPC32x0 User Manual, > @@ -164,6 +540,7 @@ int board_nand_init(struct nand_chip *lpc32xx_chip) > lpc32xx_chip->ecc.size = 256; > lpc32xx_chip->ecc.bytes = 3; > lpc32xx_chip->ecc.strength = 1; ^^^ It is the same as for DMA branch. > +#endif > > #if defined(CONFIG_SYS_NAND_USE_FLASH_BBT) > lpc32xx_chip->bbt_options |= NAND_BBT_USE_FLASH; > -- With best wishes, Vladimir _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot