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

2014-03-12 Thread Chin Liang See
To add the Denali NAND driver support into U-Boot. It required
information such as register base address from configuration
header file  within include/configs folder.

Signed-off-by: Chin Liang See 
Cc: Artem Bityutskiy 
Cc: David Woodhouse 
Cc: Brian Norris 
Cc: Scott Wood 
Cc: Masahiro Yamada 
---
Changes for v6
- Remove chip_delay as its unused
- Remove ECC bit assignment in nand_para functions
Changes for v5
- Rename denali_nand to denali only
- Rename the macro for ctrl and data address
Changes for v4
- Added cache flush to handle dcache enabled
- Used standard return where 0 for pass
- Removed unnecessary casting
- Used standard readl and writel
Changes for v3
- Fixed coding style
Changes for v2
- Enable this driver support for SOCFPGA
---
 drivers/mtd/nand/Makefile |1 +
 drivers/mtd/nand/denali.c | 1132 +
 drivers/mtd/nand/denali.h |  496 
 3 files changed, 1629 insertions(+)
 create mode 100644 drivers/mtd/nand/denali.c
 create mode 100644 drivers/mtd/nand/denali.h

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 02b149c..76ae105 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o
 obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o
 obj-$(CONFIG_DRIVER_NAND_BFIN) += bfin_nand.o
 obj-$(CONFIG_NAND_DAVINCI) += davinci_nand.o
+obj-$(CONFIG_NAND_DENALI) += denali.o
 obj-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_nand.o
 obj-$(CONFIG_NAND_FSL_IFC) += fsl_ifc_nand.o
 obj-$(CONFIG_NAND_FSL_UPM) += fsl_upm.o
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
new file mode 100644
index 000..fce1b62
--- /dev/null
+++ b/drivers/mtd/nand/denali.c
@@ -0,0 +1,1132 @@
+/*
+ * Copyright (C) 2013-2014 Altera Corporation 
+ * Copyright (C) 2009-2010, Intel Corporation and its suppliers.
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "denali.h"
+
+#define NAND_DEFAULT_TIMINGS   -1
+
+static struct denali_nand_info denali;
+static int onfi_timing_mode = NAND_DEFAULT_TIMINGS;
+
+/* We define a macro here that combines all interrupts this driver uses into
+ * a single constant value, for convenience. */
+#define DENALI_IRQ_ALL (INTR_STATUS__DMA_CMD_COMP | \
+   INTR_STATUS__ECC_TRANSACTION_DONE | \
+   INTR_STATUS__ECC_ERR | \
+   INTR_STATUS__PROGRAM_FAIL | \
+   INTR_STATUS__LOAD_COMP | \
+   INTR_STATUS__PROGRAM_COMP | \
+   INTR_STATUS__TIME_OUT | \
+   INTR_STATUS__ERASE_FAIL | \
+   INTR_STATUS__RST_COMP | \
+   INTR_STATUS__ERASE_COMP | \
+   INTR_STATUS__ECC_UNCOR_ERR | \
+   INTR_STATUS__INT_ACT | \
+   INTR_STATUS__LOCKED_BLK)
+
+/* indicates whether or not the internal value for the flash bank is
+ * valid or not */
+#define CHIP_SELECT_INVALID-1
+
+#define SUPPORT_8BITECC1
+
+/* These constants are defined by the driver to enable common driver
+ * configuration options. */
+#define SPARE_ACCESS   0x41
+#define MAIN_ACCESS0x42
+#define MAIN_SPARE_ACCESS  0x43
+
+#define DENALI_UNLOCK_START0x10
+#define DENALI_UNLOCK_END  0x11
+#define DENALI_LOCK0x21
+#define DENALI_LOCK_TIGHT  0x31
+#define DENALI_BUFFER_LOAD 0x60
+#define DENALI_BUFFER_WRITE0x62
+
+#define DENALI_READ0
+#define DENALI_WRITE   0x100
+
+/* types of device accesses. We can issue commands and get status */
+#define COMMAND_CYCLE  0
+#define ADDR_CYCLE 1
+#define STATUS_CYCLE   2
+
+/* this is a helper macro that allows us to
+ * format the bank into the proper bits for the controller */
+#define BANK(x) ((x) << 24)
+
+/* Interrupts are cleared by writing a 1 to the appropriate status bit */
+static inline void clear_interrupt(uint32_t irq_mask)
+{
+   uint32_t intr_status_reg = 0;
+   intr_status_reg = INTR_STATUS(denali.flash_bank);
+   writel(irq_mask, denali.flash_reg + intr_status_reg);
+}
+
+static uint32_t read_interrupt_status(void)
+{
+   uint32_t intr_status_reg = 0;
+   intr_status_reg = INTR_STATUS(denali.flash_bank);
+   return readl(denali.flash_reg + intr_status_reg);
+}
+
+static void clear_interrupts(void)
+{
+   uint32_t status = 0;
+   status = read_interrupt_status();
+   clear_interrupt(status);
+   denali.irq_status = 0;
+}
+
+static void denali_irq_enable(uint32_t int_mask)
+{
+   int i;
+   for (i = 0; i < denali.max_banks; ++i)
+   writel(int_mask, denali.flash_reg + INTR_EN(i));
+}
+
+static uint32_t wait_for_irq(uint32_t irq_mask)
+{
+   unsigned long timeout = 100;
+   uint32_t intr_status;
+
+   do {
+   intr_status = read_interrupt_status() & DENALI_IRQ_ALL;
+   if (in

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

2014-03-19 Thread Masahiro Yamada
Hi Chin,


> --- /dev/null
> +++ b/drivers/mtd/nand/denali.c
> @@ -0,0 +1,1132 @@
> +/*
> + * Copyright (C) 2013-2014 Altera Corporation 
> + * 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_8BITS14
> +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_15BITECC1
> +#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 
+ * Copyright (C) 2014   Panasonic Corporation
+ * Copyright (C) 2013-2014  Altera Corporation 
  * 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;
}
 }

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

2014-03-24 Thread Chin Liang See
Hi Mashiro,


On Wed, 2014-03-19 at 20:26 +0900, Masahiro Yamada wrote:
> Hi Chin,
> 
> 
> > --- /dev/null
> > +++ b/drivers/mtd/nand/denali.c
> > @@ -0,0 +1,1132 @@
> > +/*
> > + * Copyright (C) 2013-2014 Altera Corporation 
> > + * 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.)
> 

Sure, no problem.
Its open source and code belongs to everyone :)


> 
> > +/* 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.
> 

Yup, finally we know why.
Added in next patch.


> 
> 
> 
> > +/*
> > + * 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)
> 

Sure, we can enhance this. The old code which mentioned MRST seems not
applicable any more. At least, this is true for both of us.


> 
> 
> 
> > +   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.
> 

Yup, these can be removed.
It was added for SPL version but later I modified the SPL driver to use
the HW ECC correction.

> 
> 
> 
> > +
> > +typedef int irqreturn_t;
> > +
> > +#define IRQ_HANDLED1
> > +#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_15BITECC1
> > +#define SUPPORT_8BITECC 1
> 
> These are no longer necessary.
> 
> 

Noted. They are removed.

> 
> > +#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.
> 
> 

Added in next patch.

> 
> 
> 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 
> + * Copyright (C) 2014   Panasonic Corporation
> + * Copyright (C) 2013-2014  Altera Corporation 
>   * 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. s