[U-Boot] [PATCH v5 2/5] nand: lpc32xx: add hardware ECC support
Vladimir Zapolskiy
vz at mleia.com
Wed Aug 5 23:43:16 CEST 2015
Hi Sylvain,
On 05.08.2015 21:31, slemieux.tyco at gmail.com wrote:
> From: Sylvain Lemieux <slemieux at 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 <slemieux at 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 <vz at 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
More information about the U-Boot
mailing list