Hi Michal,
On Mon, 2014-02-24 at 08:48 +0100, Michal Simek wrote:
> On 02/21/2014 09:51 PM, Chin Liang See wrote:
> > 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 <clsee at altera.com>
> > Cc: Artem Bityutskiy <artem.bityutskiy at linux.intel.com>
> > Cc: David Woodhouse <David.Woodhouse at intel.com>
> > Cc: Brian Norris <computersforpeace at gmail.com>
> > Cc: Scott Wood <scottwood at freescale.com>
> > ---
> > Changes for v2
> > - Enable this driver support for SOCFPGA
> > ---
> > drivers/mtd/nand/Makefile | 1 +
> > drivers/mtd/nand/denali_nand.c | 1166 ++++++++++++++++++++++++++++++++++++++++
> > drivers/mtd/nand/denali_nand.h | 501 +++++++++++++++++
> > 3 files changed, 1668 insertions(+)
> > create mode 100644 drivers/mtd/nand/denali_nand.c
> > create mode 100644 drivers/mtd/nand/denali_nand.h
> >
> > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> > index 02b149c..24e8218 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_nand.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_nand.c b/drivers/mtd/nand/denali_nand.c
> > new file mode 100644
> > index 0000000..55246c9
> > --- /dev/null
> > +++ b/drivers/mtd/nand/denali_nand.c
> > @@ -0,0 +1,1166 @@
> > +/*
> > + * Copyright (C) 2013 Altera Corporation <www.altera.com>
>
> What about 2014?
Good catch. The first revision was sent on 2013. Fixed
>
>
> > + * Copyright (C) 2009-2010, Intel Corporation and its suppliers.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0+
> > + */
> > +
> > +#include <common.h>
> > +#include <nand.h>
> > +#include <asm/errno.h>
> > +#include <asm/io.h>
> > +
> > +#include "denali_nand.h"
> > +
> > +/* We define a module parameter that allows the user to override
> > + * the hardware and decide what timing mode should be used.
> > + */
> > +#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_8BITECC 1
> > +
> > +/* This macro divides two integers and rounds fractional values up
> > + * to the nearest integer value. */
> > +#define CEIL_DIV(X, Y) (((X)%(Y)) ? ((X)/(Y)+1) : ((X)/(Y)))
> > +
> > +/* These constants are defined by the driver to enable common driver
> > + * configuration options. */
> > +#define SPARE_ACCESS 0x41
> > +#define MAIN_ACCESS 0x42
> > +#define MAIN_SPARE_ACCESS 0x43
> > +
> > +#define DENALI_UNLOCK_START 0x10
> > +#define DENALI_UNLOCK_END 0x11
> > +#define DENALI_LOCK 0x21
> > +#define DENALI_LOCK_TIGHT 0x31
> > +#define DENALI_BUFFER_LOAD 0x60
> > +#define DENALI_BUFFER_WRITE 0x62
> > +
> > +#define DENALI_READ 0
> > +#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);
> > + __raw_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 __raw_readl(denali.flash_reg + intr_status_reg);
> > +}
> > +
> > +static void clear_interrupts(void)
> > +{
> > + uint32_t status = 0x0;
>
> just 0
Fixed
>
> > + status = read_interrupt_status();
> > + clear_interrupt(status);
> > + denali.irq_status = 0x0;
>
> just 0
Fixed
>
> > +}
> > +
> > +static void denali_irq_enable(uint32_t int_mask)
> > +{
> > + int i;
> > + for (i = 0; i < denali.max_banks; ++i)
> > + __raw_writel(int_mask, denali.flash_reg + INTR_EN(i));
> > +}
> > +
> > +static uint32_t wait_for_irq(uint32_t irq_mask)
> > +{
> > + unsigned long comp_res = 1000;
> > + uint32_t intr_status = 0;
>
> do you need to init this?
Just some coding preference. Anyway its just easy fix.
>
> > +
> > + do {
> > + intr_status = read_interrupt_status() & DENALI_IRQ_ALL;
> > + if (intr_status & irq_mask) {
> > + denali.irq_status &= ~irq_mask;
> > + /* our interrupt was detected */
> > + break;
> > + }
> > + udelay(1);
> > + comp_res--;
> > + } while (comp_res != 0);
> > +
> > + if (comp_res == 0) {
> > + /* timeout */
> > + printf("Denali timeout with interrupt status %08x\n",
> > + read_interrupt_status());
> > + intr_status = 0;
> > + }
> > + return intr_status;
> > +}
> > +
> > +/* Certain operations for the denali NAND controller use
> > + * an indexed mode to read/write data. The operation is
> > + * performed by writing the address value of the command
> > + * to the device memory followed by the data. This function
> > + * abstracts this common operation.
> > +*/
>
> weird comment coding style check globally.
Fixed
>
> > +static void index_addr(uint32_t address, uint32_t data)
> > +{
> > + __raw_writel(address, denali.flash_mem);
> > + __raw_writel(data, denali.flash_mem + 0x10);
> > +}
> > +
> > +/* Perform an indexed read of the device */
> > +static void index_addr_read_data(uint32_t address, uint32_t *pdata)
> > +{
> > + __raw_writel(address, denali.flash_mem);
> > + *pdata = __raw_readl(denali.flash_mem + 0x10);
> > +}
> > +
> > +/* We need to buffer some data for some of the NAND core routines.
> > + * The operations manage buffering that data. */
> > +static void reset_buf(void)
> > +{
> > + denali.buf.head = denali.buf.tail = 0;
> > +}
> > +
> > +static void write_byte_to_buf(uint8_t byte)
> > +{
> > + BUG_ON(denali.buf.tail >= sizeof(denali.buf.buf));
> > + denali.buf.buf[denali.buf.tail++] = byte;
> > +}
> > +
> > +/* resets a specific device connected to the core */
> > +static void reset_bank(void)
> > +{
> > + uint32_t irq_status = 0;
>
> ditto.
>
> > + uint32_t irq_mask = INTR_STATUS__RST_COMP |
> > + INTR_STATUS__TIME_OUT;
> > +
> > + clear_interrupts();
> > +
> > + __raw_writel(1 << denali.flash_bank, denali.flash_reg + DEVICE_RESET);
> > +
> > + irq_status = wait_for_irq(irq_mask);
> > + if (irq_status & INTR_STATUS__TIME_OUT)
> > + debug(KERN_ERR "reset bank failed.\n");
> > +}
> > +
> > +/* Reset the flash controller */
> > +static uint16_t denali_nand_reset(void)
> > +{
> > + uint32_t i;
> > +
> > + for (i = 0 ; i < denali.max_banks; i++)
>
> fix all checkpatch warnings. This is also broken coding style.
>
> total: 0 errors, 15 warnings, 24 checks, 1674 lines checked
Fixed
>
>
>
> > + __raw_writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,
> > + denali.flash_reg + INTR_STATUS(i));
> > +
> > + for (i = 0 ; i < denali.max_banks; i++) {
> > + __raw_writel(1 << i, denali.flash_reg + DEVICE_RESET);
> > + while (!(__raw_readl(denali.flash_reg + INTR_STATUS(i)) &
> > + (INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT)))
> > + if (__raw_readl(denali.flash_reg + INTR_STATUS(i)) &
> > + INTR_STATUS__TIME_OUT)
> > + debug(KERN_DEBUG "NAND Reset operation "
> > + "timed out on bank %d\n", i);
> > + }
> > +
> > + for (i = 0; i < denali.max_banks; i++)
> > + __raw_writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,
> > + denali.flash_reg + INTR_STATUS(i));
> > +
> > + return PASS;
> > +}
> > +
> > +/* this routine calculates the ONFI timing values for a given mode and
> > + * programs the clocking register accordingly. The mode is determined by
> > + * the get_onfi_nand_para routine.
> > + */
> > +static void nand_onfi_timing_set(uint16_t mode)
> > +{
> > + uint16_t Trea[6] = {40, 30, 25, 20, 20, 16};
> > + uint16_t Trp[6] = {50, 25, 17, 15, 12, 10};
> > + uint16_t Treh[6] = {30, 15, 15, 10, 10, 7};
> > + uint16_t Trc[6] = {100, 50, 35, 30, 25, 20};
> > + uint16_t Trhoh[6] = {0, 15, 15, 15, 15, 15};
> > + uint16_t Trloh[6] = {0, 0, 0, 0, 5, 5};
> > + uint16_t Tcea[6] = {100, 45, 30, 25, 25, 25};
> > + uint16_t Tadl[6] = {200, 100, 100, 100, 70, 70};
> > + uint16_t Trhw[6] = {200, 100, 100, 100, 100, 100};
> > + uint16_t Trhz[6] = {200, 100, 100, 100, 100, 100};
> > + uint16_t Twhr[6] = {120, 80, 80, 60, 60, 60};
> > + uint16_t Tcs[6] = {70, 35, 25, 25, 20, 15};
> > +
> > + uint16_t TclsRising = 1;
> > + uint16_t data_invalid_rhoh, data_invalid_rloh, data_invalid;
> > + uint16_t dv_window = 0;
> > + uint16_t en_lo, en_hi;
> > + uint16_t acc_clks;
> > + uint16_t addr_2_data, re_2_we, re_2_re, we_2_re, cs_cnt;
> > +
> > + en_lo = CEIL_DIV(Trp[mode], CLK_X);
> > + en_hi = CEIL_DIV(Treh[mode], CLK_X);
> > +#if ONFI_BLOOM_TIME
> > + if ((en_hi * CLK_X) < (Treh[mode] + 2))
> > + en_hi++;
> > +#endif
> > +
> > + if ((en_lo + en_hi) * CLK_X < Trc[mode])
> > + en_lo += CEIL_DIV((Trc[mode] - (en_lo + en_hi) * CLK_X), CLK_X);
> > +
> > + if ((en_lo + en_hi) < CLK_MULTI)
> > + en_lo += CLK_MULTI - en_lo - en_hi;
> > +
> > + while (dv_window < 8) {
> > + data_invalid_rhoh = en_lo * CLK_X + Trhoh[mode];
> > +
> > + data_invalid_rloh = (en_lo + en_hi) * CLK_X + Trloh[mode];
> > +
> > + data_invalid =
> > + data_invalid_rhoh <
> > + data_invalid_rloh ? data_invalid_rhoh : data_invalid_rloh;
> > +
> > + dv_window = data_invalid - Trea[mode];
> > +
> > + if (dv_window < 8)
> > + en_lo++;
> > + }
> > +
> > + acc_clks = CEIL_DIV(Trea[mode], CLK_X);
> > +
> > + while (((acc_clks * CLK_X) - Trea[mode]) < 3)
> > + acc_clks++;
> > +
> > + if ((data_invalid - acc_clks * CLK_X) < 2)
> > + debug(KERN_WARNING "%s, Line %d: Warning!\n",
> > + __FILE__, __LINE__);
> > +
> > + addr_2_data = CEIL_DIV(Tadl[mode], CLK_X);
> > + re_2_we = CEIL_DIV(Trhw[mode], CLK_X);
> > + re_2_re = CEIL_DIV(Trhz[mode], CLK_X);
> > + we_2_re = CEIL_DIV(Twhr[mode], CLK_X);
> > + cs_cnt = CEIL_DIV((Tcs[mode] - Trp[mode]), CLK_X);
> > + if (!TclsRising)
> > + cs_cnt = CEIL_DIV(Tcs[mode], CLK_X);
> > + if (cs_cnt == 0)
> > + cs_cnt = 1;
> > +
> > + if (Tcea[mode]) {
> > + while (((cs_cnt * CLK_X) + Trea[mode]) < Tcea[mode])
> > + cs_cnt++;
> > + }
> > +
> > +#if MODE5_WORKAROUND
> > + if (mode == 5)
> > + acc_clks = 5;
> > +#endif
> > +
> > + /* Sighting 3462430: Temporary hack for MT29F128G08CJABAWP:B */
> > + if ((__raw_readl(denali.flash_reg + MANUFACTURER_ID) == 0) &&
> > + (__raw_readl(denali.flash_reg + DEVICE_ID) == 0x88))
> > + acc_clks = 6;
> > +
> > + __raw_writel(acc_clks, denali.flash_reg + ACC_CLKS);
> > + __raw_writel(re_2_we, denali.flash_reg + RE_2_WE);
> > + __raw_writel(re_2_re, denali.flash_reg + RE_2_RE);
> > + __raw_writel(we_2_re, denali.flash_reg + WE_2_RE);
> > + __raw_writel(addr_2_data, denali.flash_reg + ADDR_2_DATA);
> > + __raw_writel(en_lo, denali.flash_reg + RDWR_EN_LO_CNT);
> > + __raw_writel(en_hi, denali.flash_reg + RDWR_EN_HI_CNT);
> > + __raw_writel(cs_cnt, denali.flash_reg + CS_SETUP_CNT);
> > +}
> > +
> > +/* queries the NAND device to see what ONFI modes it supports. */
> > +static uint16_t get_onfi_nand_para(void)
> > +{
> > + int i;
> > + /* we needn't to do a reset here because driver has already
> > + * reset all the banks before
> > + * */
> > + if (!(__raw_readl(denali.flash_reg + ONFI_TIMING_MODE) &
> > + ONFI_TIMING_MODE__VALUE))
> > + return FAIL;
> > +
> > + for (i = 5; i > 0; i--) {
> > + if (__raw_readl(denali.flash_reg + ONFI_TIMING_MODE) &
> > + (0x01 << i))
> > + break;
> > + }
> > +
> > + nand_onfi_timing_set(i);
> > +
> > + /* By now, all the ONFI devices we know support the page cache */
> > + /* rw feature. So here we enable the pipeline_rw_ahead feature */
> > + /* __raw_writel(1, denali.flash_reg + CACHE_WRITE_ENABLE); */
> > + /* __raw_writel(1, denali.flash_reg + CACHE_READ_ENABLE); */
>
> Dead code?
Fixed.
>
> <snip>
>
> > +/* lld_nand.h */
> > +/*
> > + * NAND Flash Controller Device Driver
> > + * Copyright (c) 2009, Intel Corporation and its suppliers.
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along with
> > + * this program; if not, write to the Free Software Foundation, Inc.,
> > + * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
>
>
> Isn't this pretty weird if we are using SPDX?
I believe the Linux driver owner wish to maintain this header when
he/she copy into NAND driver. Anyhow, the SPDX license already stated at
top of the header file.
Thanks
Chin Liang
>
> Thanks,
> Michal
>