[U-Boot] [PATCH v6] nand/denali: Adding Denali NAND driver support

Masahiro Yamada yamada.m at jp.panasonic.com
Wed Mar 19 12:26:01 CET 2014


Hi Chin,


> --- /dev/null
> +++ b/drivers/mtd/nand/denali.c
> @@ -0,0 +1,1132 @@
> +/*
> + * Copyright (C) 2013-2014 Altera Corporation <www.altera.com>
> + * Copyright (C) 2009-2010, Intel Corporation and its suppliers.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */

If you don't mind, is it OK to add the creadit of Panasonic?
Going forward, Altera and Panasonic will share this driver
and I am contributing on code improvement and  run test.
(I will post Panasonic boards support code after
this driver is merged.)


> +/* setups the HW to perform the data DMA */
> +static void denali_setup_dma(int op)
> +{

I sent a question to Cadence again and
finally received an answer I had wanted.
They said DMA command sequence depends on the bus width.

Panasonic bought 64bit bus version.
I guess Altera bought 32bit bus version.

So I'd like to suggest to use #ifdef CONFIG_NAND_DENALI_64BIT
here.




> +/*
> + * Although controller spec said SLC ECC is forceb to be 4bit, but denali
> + * controller in MRST only support 15bit and 8bit ECC correction
> + */
> +#ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST
> +#define ECC_15BITS	26
> +static struct nand_ecclayout nand_15bit_oob = {
> +	.eccbytes = ECC_15BITS,
> +};
> +#else
> +#define ECC_8BITS	14
> +static struct nand_ecclayout nand_8bit_oob = {
> +	.eccbytes = ECC_8BITS,
> +};

I think supporting only 15bit, 8bit is odd.
The number of ECC bits depends on the hardware.
(You can choose ECC bits you like when you buy the IP
from Cadence.)

I'd like to suggest to re-write this part in a generic way
by using the formula given in Denali's document.

 if (ecc.size == 512)
      ecc.bytes = Ceiling_to_next_word(ecc.strength * 13)

if (ecc.size == 1024)
     ecc.bytes = Ceiling_to_next_word(ecc.strength * 14)




> +	nand->ecc.mode = NAND_ECC_HW;
> +	nand->ecc.size = CONFIG_NAND_DENALI_ECC_SIZE;
> +	nand->ecc.read_oob = denali_read_oob;
> +	nand->ecc.write_oob = denali_write_oob;
> +	nand->ecc.read_page = denali_read_page;
> +	nand->ecc.read_page_raw = denali_read_page_raw;
> +	nand->ecc.write_page = denali_write_page;
> +	nand->ecc.write_page_raw = denali_write_page_raw;
> +#ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST
> +	/* 15bit ECC */
> +	nand->ecc.bytes = 26;
> +	nand->ecc.layout = &nand_15bit_oob;
> +#else	/* 8bit ECC */
> +	nand->ecc.bytes = 14;
> +	nand->ecc.layout = &nand_8bit_oob;
> +#endif
> +	nand->ecc.calculate = denali_ecc_calculate;
> +	nand->ecc.correct  = denali_ecc_correct;
> +	nand->ecc.hwctl  = denali_ecc_hwctl;

denali_ecc_calculate(),
denali_ecc_correct(),
denali_ecc_hwctl()
are never called. Nor do we need to set stub functions.

Besides, ecc.strength must be set.

I guess ECC_CORRECTION register is already set correctly.
(In the case of SOCFPGA, it would be set to 8.)

So, ecc.strength should be set to the value of ECC_CORRECTION.




> +
> +typedef int irqreturn_t;
> +
> +#define IRQ_HANDLED				1
> +#define IRQ_NONE				0

These typedef and macros are not used.
denali.h in Linux Kernel does not have them either.
Please delete.


> +#define SUPPORT_15BITECC        1
> +#define SUPPORT_8BITECC         1

These are no longer necessary.



> +#define DENALI_BUF_SIZE		(NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE)

NAND_MAX_PAGESIZE and NAND_MAX_OOBSIZE are defined in 
include/linux/mtd/nand.h

So, denali.h must include it.




Code diff is as follows:



diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index fce1b62..8ba8f04 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1,5 +1,6 @@
 /*
- * Copyright (C) 2013-2014 Altera Corporation <www.altera.com>
+ * Copyright (C) 2014       Panasonic Corporation
+ * Copyright (C) 2013-2014  Altera Corporation <www.altera.com>
  * Copyright (C) 2009-2010, Intel Corporation and its suppliers.
  *
  * SPDX-License-Identifier:    GPL-2.0+
@@ -660,6 +661,21 @@ static void denali_setup_dma(int op)
 
        flush_dcache_range(addr, addr + sizeof(denali.buf.dma_buf));
 
+#ifdef CONFIG_NAND_DENALI_64BIT
+       mode = MODE_10 | BANK(denali.flash_bank) | denali.page;
+
+       /* DMA is a three step process */
+
+       /* 1. setup transfer type, interrupt when complete,
+             burst len = 64 bytes, the number of pages */
+       index_addr(mode, 0x01002000 | (64 << 16) | op | page_count);
+
+       /* 2. set memory low address bits 31:0 */
+       index_addr(mode, addr);
+
+       /* 3. set memory high address bits 64:32 */
+       index_addr(mode, 0);
+#else
        mode = MODE_10 | BANK(denali.flash_bank);
 
        /* DMA is a four step process */
@@ -675,6 +691,7 @@ static void denali_setup_dma(int op)
 
        /* 4.  interrupt when complete, burst len = 64 bytes*/
        index_addr(mode | 0x14000, 0x2400);
+#endif
 }
 
 /* Common DMA function */
@@ -1017,26 +1034,6 @@ static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
                break;
        }
 }
-
-/* stubs for ECC functions not used by the NAND core */
-static int denali_ecc_calculate(struct mtd_info *mtd, const uint8_t *data,
-                               uint8_t *ecc_code)
-{
-       BUG();
-       return -EIO;
-}
-
-static int denali_ecc_correct(struct mtd_info *mtd, uint8_t *data,
-                               uint8_t *read_ecc, uint8_t *calc_ecc)
-{
-       BUG();
-       return -EIO;
-}
-
-static void denali_ecc_hwctl(struct mtd_info *mtd, int mode)
-{
-       BUG();
-}
 /* end NAND core entry points */
 
 /* Initialization code to bring the device up to a known good state */
@@ -1062,23 +1059,9 @@ static void denali_hw_init(void)
        denali_irq_init();
 }
 
-/*
- * Although controller spec said SLC ECC is forceb to be 4bit, but denali
- * controller in MRST only support 15bit and 8bit ECC correction
- */
-#ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST
-#define ECC_15BITS     26
-static struct nand_ecclayout nand_15bit_oob = {
-       .eccbytes = ECC_15BITS,
-};
-#else
-#define ECC_8BITS      14
-static struct nand_ecclayout nand_8bit_oob = {
-       .eccbytes = ECC_8BITS,
-};
-#endif  /* CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST */
+static struct nand_ecclayout nand_oob;
 
-static void denali_nand_init(struct nand_chip *nand)
+static int denali_nand_init(struct nand_chip *nand)
 {
        denali.flash_reg = (void  __iomem *)CONFIG_SYS_NAND_REGS_BASE;
        denali.flash_mem = (void  __iomem *)CONFIG_SYS_NAND_DATA_BASE;
@@ -1104,17 +1087,24 @@ static void denali_nand_init(struct nand_chip *nand)
        nand->ecc.read_page_raw = denali_read_page_raw;
        nand->ecc.write_page = denali_write_page;
        nand->ecc.write_page_raw = denali_write_page_raw;
-#ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST
-       /* 15bit ECC */
-       nand->ecc.bytes = 26;
-       nand->ecc.layout = &nand_15bit_oob;
-#else  /* 8bit ECC */
-       nand->ecc.bytes = 14;
-       nand->ecc.layout = &nand_8bit_oob;
-#endif
-       nand->ecc.calculate = denali_ecc_calculate;
-       nand->ecc.correct  = denali_ecc_correct;
-       nand->ecc.hwctl  = denali_ecc_hwctl;
+       /*
+        * Tell driver the ecc strength. This register may be already set
+        * correctly. So we read this value out.
+        */
+       nand->ecc.strength = readl(denali.flash_reg + ECC_CORRECTION);
+       switch (nand->ecc.size) {
+       case 512:
+               nand->ecc.bytes = (nand->ecc.strength * 13 + 15) / 16 * 2;
+               break;
+       case 1024:
+               nand->ecc.bytes = (nand->ecc.strength * 14 + 15) / 16 * 2;
+               break;
+       default:
+               pr_err("Unsupported ECC size\n");
+               return -EINVAL;
+       }
+       nand_oob.eccbytes = nand->ecc.bytes;
+       nand->ecc.layout = &nand_oob;
 
        /* Set address of hardware control function */
        nand->cmdfunc = denali_cmdfunc;
@@ -1123,10 +1113,10 @@ static void denali_nand_init(struct nand_chip *nand)
        nand->select_chip = denali_select_chip;
        nand->waitfunc = denali_waitfunc;
        denali_hw_init();
+       return 0;
 }
 
 int board_nand_init(struct nand_chip *chip)
 {
-       denali_nand_init(chip);
-       return 0;
+       return denali_nand_init(chip);
 }
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index c668d8c..50a109d 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -5,10 +5,7 @@
  * SPDX-License-Identifier:    GPL-2.0+
  */
 
-typedef int irqreturn_t;
-
-#define IRQ_HANDLED                            1
-#define IRQ_NONE                               0
+#include <linux/mtd/nand.h>
 
 #define DEVICE_RESET                           0x0
 #define     DEVICE_RESET__BANK0                                0x0001
@@ -382,9 +379,6 @@ typedef int irqreturn_t;
 
 #define GLOB_HWCTL_DEFAULT_BLKS    2048
 
-#define SUPPORT_15BITECC        1
-#define SUPPORT_8BITECC         1
-
 #define CUSTOM_CONF_PARAMS      0
 
 #define ONFI_BLOOM_TIME         1





Best Regards
Masahiro Yamada




More information about the U-Boot mailing list