RE: [RFC 23/47] mtd: nand: stm_nand_bch: read and write page (BCH)

Gupta, Pekon
2014-03-26T10:17:08Z

Hi Lee,

>From: Lee Jones [mailto:lee.jones@linaro.org]
>
>Use DMA to read and/or write a single page of data.
>
>Signed-off-by: Lee Jones <lee.jones@linaro.org>
>---
> drivers/mtd/nand/stm_nand_bch.c | 119 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 119 insertions(+)
>
>diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
>index 7874d85..6323590 100644
>--- a/drivers/mtd/nand/stm_nand_bch.c
>+++ b/drivers/mtd/nand/stm_nand_bch.c
>@@ -21,6 +21,7 @@
> #include <linux/interrupt.h>
> #include <linux/device.h>
> #include <linux/platform_device.h>
>+#include <linux/dma-mapping.h>
> #include <linux/completion.h>
> #include <linux/mtd/nand.h>
> #include <linux/mtd/stm_nand.h>
>@@ -345,6 +346,124 @@ static int check_erased_page(uint8_t *data, uint32_t page_size, int max_zeros)
> return zeros;
> }
>
>+/* Returns the number of ECC errors, or '-1' for uncorrectable error */
>+static int bch_read_page(struct nandi_controller *nandi,
>+ loff_t offs,
>+ uint8_t *buf)
>+{
>+ struct bch_prog *prog = &bch_prog_read_page;
>+ uint32_t page_size = nandi->info.mtd.writesize;
>+ unsigned long list_phys;
>+ unsigned long buf_phys;
>+ uint32_t ecc_err;
>+ int ret = 0;
>+
>+ dev_dbg(nandi->dev, "%s: offs = 0x%012llx\n", __func__, offs);
>+
>+ BUG_ON((unsigned long)buf & (NANDI_BCH_DMA_ALIGNMENT - 1));
>+ BUG_ON(offs & (NANDI_BCH_DMA_ALIGNMENT - 1));
>+
>+ emiss_nandi_select(STM_NANDI_BCH);
>+
>+ nandi_enable_interrupts(nandi, NANDBCH_INT_SEQNODESOVER);
>+ reinit_completion(&nandi->seq_completed);
>+
>+ /* Reset ECC stats */
>+ writel(CFG_RESET_ECC_ALL | CFG_ENABLE_AFM,
>+ nandi->base + NANDBCH_CONTROLLER_CFG);
>+ writel(CFG_ENABLE_AFM, nandi->base + NANDBCH_CONTROLLER_CFG);
>+
>+ prog->addr = (uint32_t)((offs >> (nandi->page_shift - 8)) & 0xffffff00);
>+
>+ buf_phys = dma_map_single(NULL, buf, page_size, DMA_FROM_DEVICE);
>+
>+ memset(nandi->buf_list, 0x00, NANDI_BCH_BUF_LIST_SIZE);
>+ nandi->buf_list[0] = buf_phys | (nandi->sectors_per_page - 1);
>+
>+ list_phys = dma_map_single(NULL, nandi->buf_list,
>+ NANDI_BCH_BUF_LIST_SIZE, DMA_TO_DEVICE);
>+
>+ writel(list_phys, nandi->base + NANDBCH_BUFFER_LIST_PTR);
>+
>+ bch_load_prog_cpu(nandi, prog);
>+
>+ bch_wait_seq(nandi);
>+
>+ nandi_disable_interrupts(nandi, NANDBCH_INT_SEQNODESOVER);
>+
>+ dma_unmap_single(NULL, list_phys, NANDI_BCH_BUF_LIST_SIZE,
>+ DMA_TO_DEVICE);
>+ dma_unmap_single(NULL, buf_phys, page_size, DMA_FROM_DEVICE);
>+
>+ /* Use the maximum per-sector ECC count! */

Firstly, this ecc checking and correction should not be part of bch_read_page().
This should be part of chip->ecc.correct().
But, I don't see your driver using nand_chip->ecc interfaces.
Why do you want to break away from generic driver flow ? any controller limitation ?

I think much of the code in below patch can be reused from nand_base.c
[RFC 43/47] mtd: nand: stm_nand_bch: read and write functions (BCH)

>+ ecc_err = readl(nandi->base + NANDBCH_ECC_SCORE_REG_A) & 0xff;
>+ if (ecc_err == 0xff) {
>+ /*
>+ * Downgrade uncorrectable ECC error for an erased page,
>+ * tolerating 'sectors_per_page' bits at zero.
>+ */
>+ ret = check_erased_page(buf, page_size,
>+ nandi->sectors_per_page);

This is also not correct. Here 'max_zeros' should be ecc.strength

>+ if (ret >= 0)
>+ dev_dbg(nandi->dev,
>+ "%s: erased page detected: \n"
>+ " downgrading uncorrectable ECC error.\n",
>+ __func__);
>+ } else {
>+ ret = (int)ecc_err;
>+ }
>+
>+ return ret;
>+}
>+

with regards, pekon