* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
@ 2014-02-21 20:51 Chin Liang See
2014-02-24 7:48 ` Michal Simek
` (3 more replies)
0 siblings, 4 replies; 27+ messages in thread
From: Chin Liang See @ 2014-02-21 20:51 UTC (permalink / raw)
To: u-boot
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@altera.com>
Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Cc: David Woodhouse <David.Woodhouse@intel.com>
Cc: Brian Norris <computersforpeace@gmail.com>
Cc: Scott Wood <scottwood@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>
+ * 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;
+ status = read_interrupt_status();
+ clear_interrupt(status);
+ denali.irq_status = 0x0;
+}
+
+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 {
+ 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.
+*/
+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;
+ 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++)
+ __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); */
+
+ return PASS;
+}
+
+static void get_samsung_nand_para(uint8_t device_id)
+{
+ if (device_id == 0xd3) { /* Samsung K9WAG08U1A */
+ /* Set timing register values according to datasheet */
+ __raw_writel(5, denali.flash_reg + ACC_CLKS);
+ __raw_writel(20, denali.flash_reg + RE_2_WE);
+ __raw_writel(12, denali.flash_reg + WE_2_RE);
+ __raw_writel(14, denali.flash_reg + ADDR_2_DATA);
+ __raw_writel(3, denali.flash_reg + RDWR_EN_LO_CNT);
+ __raw_writel(2, denali.flash_reg + RDWR_EN_HI_CNT);
+ __raw_writel(2, denali.flash_reg + CS_SETUP_CNT);
+ }
+}
+
+static void get_toshiba_nand_para(void)
+{
+ uint32_t tmp;
+
+ /* Workaround to fix a controller bug which reports a wrong */
+ /* spare area size for some kind of Toshiba NAND device */
+ if ((__raw_readl(denali.flash_reg + DEVICE_MAIN_AREA_SIZE) == 4096) &&
+ (__raw_readl(denali.flash_reg + DEVICE_SPARE_AREA_SIZE)
+ == 64)){
+ __raw_writel(216, denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
+ tmp = __raw_readl(denali.flash_reg + DEVICES_CONNECTED) *
+ __raw_readl(denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
+ __raw_writel(tmp,
+ denali.flash_reg + LOGICAL_PAGE_SPARE_SIZE);
+#if SUPPORT_15BITECC
+ __raw_writel(15, denali.flash_reg + ECC_CORRECTION);
+#elif SUPPORT_8BITECC
+ __raw_writel(8, denali.flash_reg + ECC_CORRECTION);
+#endif
+ }
+}
+
+static void get_hynix_nand_para(uint8_t device_id)
+{
+ uint32_t main_size, spare_size;
+
+ switch (device_id) {
+ case 0xD5: /* Hynix H27UAG8T2A, H27UBG8U5A or H27UCG8VFA */
+ case 0xD7: /* Hynix H27UDG8VEM, H27UCG8UDM or H27UCG8V5A */
+ __raw_writel(128, denali.flash_reg + PAGES_PER_BLOCK);
+ __raw_writel(4096, denali.flash_reg + DEVICE_MAIN_AREA_SIZE);
+ __raw_writel(224, denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
+ main_size = 4096 *
+ __raw_readl(denali.flash_reg + DEVICES_CONNECTED);
+ spare_size = 224 *
+ __raw_readl(denali.flash_reg + DEVICES_CONNECTED);
+ __raw_writel(main_size,
+ denali.flash_reg + LOGICAL_PAGE_DATA_SIZE);
+ __raw_writel(spare_size,
+ denali.flash_reg + LOGICAL_PAGE_SPARE_SIZE);
+ __raw_writel(0, denali.flash_reg + DEVICE_WIDTH);
+#if SUPPORT_15BITECC
+ __raw_writel(15, denali.flash_reg + ECC_CORRECTION);
+#elif SUPPORT_8BITECC
+ __raw_writel(8, denali.flash_reg + ECC_CORRECTION);
+#endif
+ break;
+ default:
+ debug(KERN_WARNING
+ "Spectra: Unknown Hynix NAND (Device ID: 0x%x)."
+ "Will use default parameter values instead.\n",
+ device_id);
+ }
+}
+
+/* determines how many NAND chips are connected to the controller. Note for
+ * Intel CE4100 devices we don't support more than one device.
+ */
+static void find_valid_banks(void)
+{
+ uint32_t id[denali.max_banks];
+ int i;
+
+ denali.total_used_banks = 1;
+ for (i = 0; i < denali.max_banks; i++) {
+ index_addr((uint32_t)(MODE_11 | (i << 24) | 0), 0x90);
+ index_addr((uint32_t)(MODE_11 | (i << 24) | 1), 0);
+ index_addr_read_data((uint32_t)(MODE_11 | (i << 24) | 2),
+ &id[i]);
+
+ if (i == 0) {
+ if (!(id[i] & 0x0ff))
+ break; /* WTF? */
+ } else {
+ if ((id[i] & 0x0ff) == (id[0] & 0x0ff))
+ denali.total_used_banks++;
+ else
+ break;
+ }
+ }
+}
+
+/*
+ * Use the configuration feature register to determine the maximum number of
+ * banks that the hardware supports.
+ */
+static void detect_max_banks(void)
+{
+ uint32_t features = __raw_readl(denali.flash_reg + FEATURES);
+ denali.max_banks = 2 << (features & FEATURES__N_BANKS);
+}
+
+static void detect_partition_feature(void)
+{
+ /* For MRST platform, denali.fwblks represent the
+ * number of blocks firmware is taken,
+ * FW is in protect partition and MTD driver has no
+ * permission to access it. So let driver know how many
+ * blocks it can't touch.
+ * */
+ if (__raw_readl(denali.flash_reg + FEATURES) & FEATURES__PARTITION) {
+ if ((__raw_readl(denali.flash_reg + PERM_SRC_ID(1)) &
+ PERM_SRC_ID__SRCID) == SPECTRA_PARTITION_ID) {
+ denali.fwblks =
+ ((__raw_readl(denali.flash_reg + MIN_MAX_BANK(1)) &
+ MIN_MAX_BANK__MIN_VALUE) *
+ denali.blksperchip)
+ +
+ (__raw_readl(denali.flash_reg + MIN_BLK_ADDR(1)) &
+ MIN_BLK_ADDR__VALUE);
+ } else
+ denali.fwblks = SPECTRA_START_BLOCK;
+ } else
+ denali.fwblks = SPECTRA_START_BLOCK;
+}
+
+static uint16_t denali_nand_timing_set(void)
+{
+ uint16_t status = PASS;
+ uint32_t id_bytes[5], addr;
+ uint8_t i, maf_id, device_id;
+
+ /* Use read id method to get device ID and other
+ * params. For some NAND chips, controller can't
+ * report the correct device ID by reading from
+ * DEVICE_ID register
+ * */
+ addr = (uint32_t)MODE_11 | BANK(denali.flash_bank);
+ index_addr((uint32_t)addr | 0, 0x90);
+ index_addr((uint32_t)addr | 1, 0);
+ for (i = 0; i < 5; i++)
+ index_addr_read_data(addr | 2, &id_bytes[i]);
+ maf_id = id_bytes[0];
+ device_id = id_bytes[1];
+
+ if (__raw_readl(denali.flash_reg + ONFI_DEVICE_NO_OF_LUNS) &
+ ONFI_DEVICE_NO_OF_LUNS__ONFI_DEVICE) { /* ONFI 1.0 NAND */
+ if (FAIL == get_onfi_nand_para())
+ return FAIL;
+ } else if (maf_id == 0xEC) { /* Samsung NAND */
+ get_samsung_nand_para(device_id);
+ } else if (maf_id == 0x98) { /* Toshiba NAND */
+ get_toshiba_nand_para();
+ } else if (maf_id == 0xAD) { /* Hynix NAND */
+ get_hynix_nand_para(device_id);
+ }
+
+ find_valid_banks();
+
+ detect_partition_feature();
+
+ /* If the user specified to override the default timings
+ * with a specific ONFI mode, we apply those changes here.
+ */
+ if (onfi_timing_mode != NAND_DEFAULT_TIMINGS)
+ nand_onfi_timing_set(onfi_timing_mode);
+
+ return status;
+}
+
+static void denali_set_intr_modes(uint16_t INT_ENABLE)
+{
+ if (INT_ENABLE)
+ __raw_writel(1, denali.flash_reg + GLOBAL_INT_ENABLE);
+ else
+ __raw_writel(0, denali.flash_reg + GLOBAL_INT_ENABLE);
+}
+
+/* validation function to verify that the controlling software is making
+ * a valid request
+ */
+static inline bool is_flash_bank_valid(int flash_bank)
+{
+ return (flash_bank >= 0 && flash_bank < 4);
+}
+
+static void denali_irq_init(void)
+{
+ uint32_t int_mask = 0;
+ int i;
+
+ /* Disable global interrupts */
+ denali_set_intr_modes(false);
+
+ int_mask = DENALI_IRQ_ALL;
+
+ /* Clear all status bits */
+ for (i = 0; i < denali.max_banks; ++i)
+ __raw_writel(0xFFFF, denali.flash_reg + INTR_STATUS(i));
+
+ denali_irq_enable(int_mask);
+}
+
+/* This helper function setups the registers for ECC and whether or not
+ * the spare area will be transferred. */
+static void setup_ecc_for_xfer(bool ecc_en, bool transfer_spare)
+{
+ int ecc_en_flag = 0, transfer_spare_flag = 0;
+
+ /* set ECC, transfer spare bits if needed */
+ ecc_en_flag = ecc_en ? ECC_ENABLE__FLAG : 0;
+ transfer_spare_flag = transfer_spare ? TRANSFER_SPARE_REG__FLAG : 0;
+
+ /* Enable spare area/ECC per user's request. */
+ __raw_writel(ecc_en_flag, denali.flash_reg + ECC_ENABLE);
+ /* applicable for MAP01 only */
+ __raw_writel(transfer_spare_flag,
+ denali.flash_reg + TRANSFER_SPARE_REG);
+}
+
+/* sends a pipeline command operation to the controller. See the Denali NAND
+ * controller's user guide for more information (section 4.2.3.6).
+ */
+static int denali_send_pipeline_cmd(bool ecc_en, bool transfer_spare,
+ int access_type, int op)
+{
+ uint32_t addr = 0x0, cmd = 0x0, irq_status = 0, irq_mask = 0;
+ uint32_t page_count = 1; /* always read a page */
+
+ if (op == DENALI_READ)
+ irq_mask = INTR_STATUS__LOAD_COMP;
+ else if (op == DENALI_WRITE)
+ irq_mask = INTR_STATUS__PROGRAM_COMP |
+ INTR_STATUS__PROGRAM_FAIL;
+ else
+ BUG();
+
+ /* clear interrupts */
+ clear_interrupts();
+
+ /* setup ECC and transfer spare reg */
+ setup_ecc_for_xfer(ecc_en, transfer_spare);
+
+ addr = BANK(denali.flash_bank) | denali.page;
+
+ /* setup the acccess type */
+ cmd = MODE_10 | addr;
+ index_addr((uint32_t)cmd, access_type);
+
+ /* setup the pipeline command */
+ if (access_type == SPARE_ACCESS && op == DENALI_WRITE)
+ index_addr((uint32_t)cmd, DENALI_BUFFER_WRITE);
+ else if (access_type == SPARE_ACCESS && op == DENALI_READ)
+ index_addr((uint32_t)cmd, DENALI_BUFFER_LOAD);
+ else
+ index_addr((uint32_t)cmd, 0x2000 | op | page_count);
+
+ /* wait for command to be accepted */
+ irq_status = wait_for_irq(irq_mask);
+ if ((irq_status & irq_mask) != irq_mask)
+ return FAIL;
+
+ if (access_type != SPARE_ACCESS) {
+ cmd = MODE_01 | addr;
+ __raw_writel(cmd, denali.flash_mem);
+ }
+ return PASS;
+}
+
+/* helper function that simply writes a buffer to the flash */
+static int write_data_to_flash_mem(const uint8_t *buf,
+ int len)
+{
+ uint32_t i = 0, *buf32;
+
+ /* verify that the len is a multiple of 4. see comment in
+ * read_data_from_flash_mem() */
+ BUG_ON((len % 4) != 0);
+
+ /* write the data to the flash memory */
+ buf32 = (uint32_t *)buf;
+ for (i = 0; i < len / 4; i++)
+ __raw_writel(*buf32++, denali.flash_mem + 0x10);
+ return i*4; /* intent is to return the number of bytes read */
+}
+
+static void denali_mode_main_access(void)
+{
+ uint32_t addr, cmd;
+ addr = BANK(denali.flash_bank) | denali.page;
+ cmd = MODE_10 | addr;
+ index_addr((uint32_t)cmd, MAIN_ACCESS);
+}
+
+static void denali_mode_main_spare_access(void)
+{
+ uint32_t addr, cmd;
+ addr = BANK(denali.flash_bank) | denali.page;
+ cmd = MODE_10 | addr;
+ index_addr((uint32_t)cmd, MAIN_SPARE_ACCESS);
+}
+
+/* Writes OOB data to the device.
+ * This code unused under normal U-Boot console as normally page write raw
+ * to be used for write oob data with main data.
+ */
+static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
+{
+ uint32_t cmd;
+
+ denali.page = page;
+ debug("* write_oob_data *\n");
+
+ /* We need to write to buffer first through MAP00 command */
+ cmd = MODE_00 | BANK(denali.flash_bank);
+ __raw_writel(cmd, denali.flash_mem);
+
+ /* send the data into flash buffer */
+ write_data_to_flash_mem(buf, mtd->oobsize);
+
+ /* activate the write through MAP10 commands */
+ if (denali_send_pipeline_cmd(false, false,
+ SPARE_ACCESS, DENALI_WRITE) != PASS)
+ return -EIO;
+
+ return 0;
+}
+
+/* this function examines buffers to see if they contain data that
+ * indicate that the buffer is part of an erased region of flash.
+ */
+bool is_erased(uint8_t *buf, int len)
+{
+ int i = 0;
+ for (i = 0; i < len; i++)
+ if (buf[i] != 0xFF)
+ return false;
+ return true;
+}
+
+
+/* programs the controller to either enable/disable DMA transfers */
+static void denali_enable_dma(bool en)
+{
+ uint32_t reg_val = 0x0;
+
+ if (en)
+ reg_val = DMA_ENABLE__FLAG;
+
+ __raw_writel(reg_val, denali.flash_reg + DMA_ENABLE);
+ __raw_readl(denali.flash_reg + DMA_ENABLE);
+}
+
+/* setups the HW to perform the data DMA */
+static void denali_setup_dma_sequence(int op)
+{
+ const int page_count = 1;
+ uint32_t mode;
+ uint32_t addr = (uint32_t)denali.buf.dma_buf;
+
+ mode = MODE_10 | BANK(denali.flash_bank);
+
+ /* DMA is a four step process */
+
+ /* 1. setup transfer type and # of pages */
+ index_addr(mode | denali.page, 0x2000 | op | page_count);
+
+ /* 2. set memory high address bits 23:8 */
+ index_addr(mode | ((uint16_t)(addr >> 16) << 8), 0x2200);
+
+ /* 3. set memory low address bits 23:8 */
+ index_addr(mode | ((uint16_t)addr << 8), 0x2300);
+
+ /* 4. interrupt when complete, burst len = 64 bytes*/
+ index_addr(mode | 0x14000, 0x2400);
+}
+
+/* Common DMA function */
+static uint32_t denali_dma_configuration(uint32_t ops, bool raw_xfer,
+ uint32_t irq_mask, int oob_required)
+{
+ uint32_t irq_status = 0;
+ /* setup_ecc_for_xfer(bool ecc_en, bool transfer_spare) */
+ setup_ecc_for_xfer(!raw_xfer, oob_required);
+
+ /* clear any previous interrupt flags */
+ clear_interrupts();
+
+ /* enable the DMA */
+ denali_enable_dma(true);
+
+ /* setup the DMA */
+ denali_setup_dma_sequence(ops);
+
+ /* wait for operation to complete */
+ irq_status = wait_for_irq(irq_mask);
+
+ /* if ECC fault happen, seems we need delay before turning off DMA.
+ * If not, the controller will go into non responsive condition */
+ if (irq_status & INTR_STATUS__ECC_UNCOR_ERR)
+ udelay(100);
+
+ /* disable the DMA */
+ denali_enable_dma(false);
+
+ return irq_status;
+}
+
+static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
+ const uint8_t *buf, bool raw_xfer, int oob_required)
+{
+ uint32_t irq_status = 0;
+ uint32_t irq_mask = INTR_STATUS__DMA_CMD_COMP;
+
+ denali.status = PASS;
+
+ /* copy buffer into DMA buffer */
+ memcpy((void *)denali.buf.dma_buf, buf, mtd->writesize);
+
+ /* need extra memcpoy for raw transfer */
+ if (raw_xfer)
+ memcpy((void *)denali.buf.dma_buf + mtd->writesize,
+ chip->oob_poi, mtd->oobsize);
+
+ /* setting up DMA */
+ irq_status = denali_dma_configuration(DENALI_WRITE, raw_xfer, irq_mask,
+ oob_required);
+
+ /* if timeout happen, error out */
+ if (!(irq_status & INTR_STATUS__DMA_CMD_COMP)) {
+ debug("DMA timeout for denali write_page\n");
+ denali.status = NAND_STATUS_FAIL;
+ return -EIO;
+ }
+
+ if (irq_status & INTR_STATUS__LOCKED_BLK) {
+ debug("Failed as write to locked block\n");
+ denali.status = NAND_STATUS_FAIL;
+ return -EIO;
+ }
+ return 0;
+}
+
+/* NAND core entry points */
+
+/*
+ * this is the callback that the NAND core calls to write a page. Since
+ * writing a page with ECC or without is similar, all the work is done
+ * by write_page above.
+ */
+static int denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
+ const uint8_t *buf, int oob_required)
+{
+ /*
+ * for regular page writes, we let HW handle all the ECC
+ * data written to the device.
+ */
+ debug("denali_write_page at page %08x\n", denali.page);
+
+ if (oob_required)
+ /* switch to main + spare access */
+ denali_mode_main_spare_access();
+ else
+ /* switch to main access only */
+ denali_mode_main_access();
+
+ return write_page(mtd, chip, buf, false, oob_required);
+}
+
+/*
+ * This is the callback that the NAND core calls to write a page without ECC.
+ * raw access is similar to ECC page writes, so all the work is done in the
+ * write_page() function above.
+ */
+static int denali_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
+ const uint8_t *buf, int oob_required)
+{
+ /*
+ * for raw page writes, we want to disable ECC and simply write
+ * whatever data is in the buffer.
+ */
+ debug("denali_write_page_raw at page %08x\n", denali.page);
+
+ if (oob_required)
+ /* switch to main + spare access */
+ denali_mode_main_spare_access();
+ else
+ /* switch to main access only */
+ denali_mode_main_access();
+
+ return write_page(mtd, chip, buf, true, oob_required);
+}
+
+static int denali_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
+ int page)
+{
+ return write_oob_data(mtd, chip->oob_poi, page);
+}
+
+/* raw include ECC value and all the spare area */
+static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
+ uint8_t *buf, int oob_required, int page)
+{
+ uint32_t irq_status, irq_mask = INTR_STATUS__DMA_CMD_COMP;
+
+ debug("denali_read_page_raw at page %08x\n", page);
+ if (denali.page != page) {
+ debug("Missing NAND_CMD_READ0 command\n");
+ return -EIO;
+ }
+
+ if (oob_required)
+ /* switch to main + spare access */
+ denali_mode_main_spare_access();
+ else
+ /* switch to main access only */
+ denali_mode_main_access();
+
+ /* setting up the DMA where ecc_enable is false */
+ irq_status = denali_dma_configuration(DENALI_READ, true, irq_mask,
+ oob_required);
+
+ /* if timeout happen, error out */
+ if (!(irq_status & INTR_STATUS__DMA_CMD_COMP)) {
+ debug("DMA timeout for denali_read_page_raw\n");
+ return -EIO;
+ }
+
+ /* splitting the content to destination buffer holder */
+ memcpy(chip->oob_poi, (const void *)(denali.buf.dma_buf +
+ mtd->writesize), mtd->oobsize);
+ memcpy(buf, (const void *)denali.buf.dma_buf, mtd->writesize);
+ debug("buf %02x %02x\n", buf[0], buf[1]);
+ debug("chip->oob_poi %02x %02x\n", chip->oob_poi[0], chip->oob_poi[1]);
+ return 0;
+}
+
+static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
+ uint8_t *buf, int oob_required, int page)
+{
+ uint32_t irq_status, irq_mask = INTR_STATUS__DMA_CMD_COMP;
+
+ debug("denali_read_page at page %08x\n", page);
+ if (denali.page != page) {
+ debug("Missing NAND_CMD_READ0 command\n");
+ return -EIO;
+ }
+
+ if (oob_required)
+ /* switch to main + spare access */
+ denali_mode_main_spare_access();
+ else
+ /* switch to main access only */
+ denali_mode_main_access();
+
+ /* setting up the DMA where ecc_enable is true */
+ irq_status = denali_dma_configuration(DENALI_READ, false, irq_mask,
+ oob_required);
+
+ memcpy(buf, (const void *)denali.buf.dma_buf, mtd->writesize);
+ debug("buf %02x %02x\n", buf[0], buf[1]);
+
+ /* check whether any ECC error */
+ if (irq_status & INTR_STATUS__ECC_UNCOR_ERR) {
+
+ /* is the ECC cause by erase page, check using read_page_raw */
+ debug(" Uncorrected ECC detected\n");
+ denali_read_page_raw(mtd, chip, buf, oob_required, denali.page);
+
+ if (is_erased(buf, mtd->writesize) == true &&
+ is_erased(chip->oob_poi, mtd->oobsize) == true) {
+ debug(" ECC error cause by erased block\n");
+ /* false alarm, return the 0xFF */
+ } else
+ return -EIO;
+ }
+ memcpy(buf, (const void *)denali.buf.dma_buf, mtd->writesize);
+ return 0;
+}
+
+static uint8_t denali_read_byte(struct mtd_info *mtd)
+{
+ uint32_t addr, result;
+ addr = (uint32_t)MODE_11 | BANK(denali.flash_bank);
+ index_addr_read_data((uint32_t)addr | 2, &result);
+ return (uint8_t)result & 0xFF;
+}
+
+static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
+ int page)
+{
+ debug("denali_read_oob at page %08x\n", page);
+ denali.page = page;
+ return denali_read_page_raw(mtd, chip, denali.buf.buf, 1, page);
+}
+
+static void denali_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+ uint32_t i, addr, result;
+
+ /* delay for tR (data transfer from Flash array to data register) */
+ udelay(25);
+
+ /* ensure device completed else additional delay and polling */
+ wait_for_irq(INTR_STATUS__INT_ACT);
+
+ addr = (uint32_t)MODE_11 | BANK(denali.flash_bank);
+ for (i = 0; i < len; i++) {
+ index_addr_read_data((uint32_t)addr | 2, &result);
+ write_byte_to_buf(result);
+ }
+ memcpy(buf, denali.buf.buf, len);
+}
+
+static void denali_select_chip(struct mtd_info *mtd, int chip)
+{
+ denali.flash_bank = chip;
+}
+
+static int denali_waitfunc(struct mtd_info *mtd, struct nand_chip *chip)
+{
+ int status = denali.status;
+ denali.status = 0;
+
+ return status;
+}
+
+static void denali_erase(struct mtd_info *mtd, int page)
+{
+ uint32_t cmd = 0x0, irq_status = 0;
+
+ debug("denali_erase@page %08x\n", page);
+
+ /* clear interrupts */
+ clear_interrupts();
+
+ /* setup page read request for access type */
+ cmd = MODE_10 | BANK(denali.flash_bank) | page;
+ index_addr((uint32_t)cmd, 0x1);
+
+ /* wait for erase to complete or failure to occur */
+ irq_status = wait_for_irq(INTR_STATUS__ERASE_COMP |
+ INTR_STATUS__ERASE_FAIL);
+
+ if (irq_status & INTR_STATUS__ERASE_FAIL ||
+ irq_status & INTR_STATUS__LOCKED_BLK)
+ denali.status = NAND_STATUS_FAIL;
+ else
+ denali.status = PASS;
+}
+
+static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
+ int page)
+{
+ uint32_t addr;
+
+ switch (cmd) {
+ case NAND_CMD_PAGEPROG:
+ break;
+ case NAND_CMD_STATUS:
+ addr = (uint32_t)MODE_11 | BANK(denali.flash_bank);
+ index_addr((uint32_t)addr | 0, cmd);
+ break;
+ case NAND_CMD_PARAM:
+ clear_interrupts();
+ case NAND_CMD_READID:
+ reset_buf();
+ /* sometimes ManufactureId read from register is not right
+ * e.g. some of Micron MT29F32G08QAA MLC NAND chips
+ * So here we send READID cmd to NAND insteand
+ * */
+ addr = (uint32_t)MODE_11 | BANK(denali.flash_bank);
+ index_addr((uint32_t)addr | 0, cmd);
+ index_addr((uint32_t)addr | 1, col & 0xFF);
+ break;
+ case NAND_CMD_READ0:
+ case NAND_CMD_SEQIN:
+ denali.page = page;
+ break;
+ case NAND_CMD_RESET:
+ reset_bank();
+ break;
+ case NAND_CMD_READOOB:
+ /* TODO: Read OOB data */
+ break;
+ case NAND_CMD_ERASE1:
+ /*
+ * supporting block erase only, not multiblock erase as
+ * it will cross plane and software need complex calculation
+ * to identify the block count for the cross plane
+ */
+ denali_erase(mtd, page);
+ break;
+ case NAND_CMD_ERASE2:
+ /* nothing to do here as it was done during NAND_CMD_ERASE1 */
+ break;
+ case NAND_CMD_UNLOCK1:
+ addr = (uint32_t)MODE_10 | BANK(denali.flash_bank) | page;
+ index_addr((uint32_t)addr | 0, DENALI_UNLOCK_START);
+ break;
+ case NAND_CMD_UNLOCK2:
+ addr = (uint32_t)MODE_10 | BANK(denali.flash_bank) | page;
+ index_addr((uint32_t)addr | 0, DENALI_UNLOCK_END);
+ break;
+ case NAND_CMD_LOCK:
+ addr = (uint32_t)MODE_10 | BANK(denali.flash_bank);
+ index_addr((uint32_t)addr | 0, DENALI_LOCK);
+ break;
+ case NAND_CMD_LOCK_TIGHT:
+ addr = (uint32_t)MODE_10 | BANK(denali.flash_bank);
+ index_addr((uint32_t)addr | 0, DENALI_LOCK_TIGHT);
+ break;
+ default:
+ printf(": unsupported command received 0x%x\n", cmd);
+ 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)
+{
+ debug("Should not be called as ECC handled by hardware\n");
+ BUG();
+ return -EIO;
+}
+
+static int denali_ecc_correct(struct mtd_info *mtd, uint8_t *data,
+ uint8_t *read_ecc, uint8_t *calc_ecc)
+{
+ debug("Should not be called as ECC handled by hardware\n");
+ BUG();
+ return -EIO;
+}
+
+static void denali_ecc_hwctl(struct mtd_info *mtd, int mode)
+{
+ debug("Should not be called as ECC handled by hardware\n");
+ BUG();
+}
+/* end NAND core entry points */
+
+/* Initialization code to bring the device up to a known good state */
+static void denali_hw_init(void)
+{
+ /*
+ * tell driver how many bit controller will skip before writing
+ * ECC code in OOB. This is normally used for bad block marker
+ */
+ __raw_writel(CONFIG_NAND_DENALI_SPARE_AREA_SKIP_BYTES,
+ denali.flash_reg + SPARE_AREA_SKIP_BYTES);
+ detect_max_banks();
+ denali_nand_reset();
+ __raw_writel(0x0F, denali.flash_reg + RB_PIN_ENABLED);
+ __raw_writel(CHIP_EN_DONT_CARE__FLAG,
+ denali.flash_reg + CHIP_ENABLE_DONT_CARE);
+ __raw_writel(0xffff, denali.flash_reg + SPARE_AREA_MARKER);
+
+ /* Should set value for these registers when init */
+ __raw_writel(0, denali.flash_reg + TWO_ROW_ADDR_CYCLES);
+ __raw_writel(1, denali.flash_reg + ECC_ENABLE);
+ denali_nand_timing_set();
+ 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 */
+
+void 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;
+
+ nand->chip_delay = 0;
+#ifdef CONFIG_SYS_NAND_USE_FLASH_BBT
+ /* check whether flash got BBT table (located at end of flash). As we
+ * use NAND_BBT_NO_OOB, the BBT page will start with
+ * bbt_pattern. We will have mirror pattern too */
+ nand->options |= NAND_BBT_USE_FLASH;
+ /*
+ * We are using main + spare with ECC support. As BBT need ECC support,
+ * we need to ensure BBT code don't write to OOB for the BBT pattern.
+ * All BBT info will be stored into data area with ECC support.
+ */
+ nand->options |= NAND_BBT_NO_OOB;
+#endif
+
+ 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;
+
+ /* Set address of hardware control function */
+ nand->cmdfunc = denali_cmdfunc;
+ nand->read_byte = denali_read_byte;
+ nand->read_buf = denali_read_buf;
+ nand->select_chip = denali_select_chip;
+ nand->waitfunc = denali_waitfunc;
+ denali_hw_init();
+}
+
+int board_nand_init(struct nand_chip *chip)
+{
+ puts("NAND: Denali NAND controller\n");
+ denali_nand_init(chip);
+ return 0;
+}
diff --git a/drivers/mtd/nand/denali_nand.h b/drivers/mtd/nand/denali_nand.h
new file mode 100644
index 0000000..fd91c64
--- /dev/null
+++ b/drivers/mtd/nand/denali_nand.h
@@ -0,0 +1,501 @@
+/*
+ * Copyright (C) 2013 Altera Corporation <www.altera.com>
+ * Copyright (C) 2009-2010, Intel Corporation and its suppliers.
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+typedef int irqreturn_t;
+
+#define IRQ_HANDLED 1
+#define IRQ_NONE 0
+
+#define DEVICE_RESET 0x0
+#define DEVICE_RESET__BANK0 0x0001
+#define DEVICE_RESET__BANK1 0x0002
+#define DEVICE_RESET__BANK2 0x0004
+#define DEVICE_RESET__BANK3 0x0008
+
+#define TRANSFER_SPARE_REG 0x10
+#define TRANSFER_SPARE_REG__FLAG 0x0001
+
+#define LOAD_WAIT_CNT 0x20
+#define LOAD_WAIT_CNT__VALUE 0xffff
+
+#define PROGRAM_WAIT_CNT 0x30
+#define PROGRAM_WAIT_CNT__VALUE 0xffff
+
+#define ERASE_WAIT_CNT 0x40
+#define ERASE_WAIT_CNT__VALUE 0xffff
+
+#define INT_MON_CYCCNT 0x50
+#define INT_MON_CYCCNT__VALUE 0xffff
+
+#define RB_PIN_ENABLED 0x60
+#define RB_PIN_ENABLED__BANK0 0x0001
+#define RB_PIN_ENABLED__BANK1 0x0002
+#define RB_PIN_ENABLED__BANK2 0x0004
+#define RB_PIN_ENABLED__BANK3 0x0008
+
+#define MULTIPLANE_OPERATION 0x70
+#define MULTIPLANE_OPERATION__FLAG 0x0001
+
+#define MULTIPLANE_READ_ENABLE 0x80
+#define MULTIPLANE_READ_ENABLE__FLAG 0x0001
+
+#define COPYBACK_DISABLE 0x90
+#define COPYBACK_DISABLE__FLAG 0x0001
+
+#define CACHE_WRITE_ENABLE 0xa0
+#define CACHE_WRITE_ENABLE__FLAG 0x0001
+
+#define CACHE_READ_ENABLE 0xb0
+#define CACHE_READ_ENABLE__FLAG 0x0001
+
+#define PREFETCH_MODE 0xc0
+#define PREFETCH_MODE__PREFETCH_EN 0x0001
+#define PREFETCH_MODE__PREFETCH_BURST_LENGTH 0xfff0
+
+#define CHIP_ENABLE_DONT_CARE 0xd0
+#define CHIP_EN_DONT_CARE__FLAG 0x01
+
+#define ECC_ENABLE 0xe0
+#define ECC_ENABLE__FLAG 0x0001
+
+#define GLOBAL_INT_ENABLE 0xf0
+#define GLOBAL_INT_EN_FLAG 0x01
+
+#define WE_2_RE 0x100
+#define WE_2_RE__VALUE 0x003f
+
+#define ADDR_2_DATA 0x110
+#define ADDR_2_DATA__VALUE 0x003f
+
+#define RE_2_WE 0x120
+#define RE_2_WE__VALUE 0x003f
+
+#define ACC_CLKS 0x130
+#define ACC_CLKS__VALUE 0x000f
+
+#define NUMBER_OF_PLANES 0x140
+#define NUMBER_OF_PLANES__VALUE 0x0007
+
+#define PAGES_PER_BLOCK 0x150
+#define PAGES_PER_BLOCK__VALUE 0xffff
+
+#define DEVICE_WIDTH 0x160
+#define DEVICE_WIDTH__VALUE 0x0003
+
+#define DEVICE_MAIN_AREA_SIZE 0x170
+#define DEVICE_MAIN_AREA_SIZE__VALUE 0xffff
+
+#define DEVICE_SPARE_AREA_SIZE 0x180
+#define DEVICE_SPARE_AREA_SIZE__VALUE 0xffff
+
+#define TWO_ROW_ADDR_CYCLES 0x190
+#define TWO_ROW_ADDR_CYCLES__FLAG 0x0001
+
+#define MULTIPLANE_ADDR_RESTRICT 0x1a0
+#define MULTIPLANE_ADDR_RESTRICT__FLAG 0x0001
+
+#define ECC_CORRECTION 0x1b0
+#define ECC_CORRECTION__VALUE 0x001f
+
+#define READ_MODE 0x1c0
+#define READ_MODE__VALUE 0x000f
+
+#define WRITE_MODE 0x1d0
+#define WRITE_MODE__VALUE 0x000f
+
+#define COPYBACK_MODE 0x1e0
+#define COPYBACK_MODE__VALUE 0x000f
+
+#define RDWR_EN_LO_CNT 0x1f0
+#define RDWR_EN_LO_CNT__VALUE 0x001f
+
+#define RDWR_EN_HI_CNT 0x200
+#define RDWR_EN_HI_CNT__VALUE 0x001f
+
+#define MAX_RD_DELAY 0x210
+#define MAX_RD_DELAY__VALUE 0x000f
+
+#define CS_SETUP_CNT 0x220
+#define CS_SETUP_CNT__VALUE 0x001f
+
+#define SPARE_AREA_SKIP_BYTES 0x230
+#define SPARE_AREA_SKIP_BYTES__VALUE 0x003f
+
+#define SPARE_AREA_MARKER 0x240
+#define SPARE_AREA_MARKER__VALUE 0xffff
+
+#define DEVICES_CONNECTED 0x250
+#define DEVICES_CONNECTED__VALUE 0x0007
+
+#define DIE_MASK 0x260
+#define DIE_MASK__VALUE 0x00ff
+
+#define FIRST_BLOCK_OF_NEXT_PLANE 0x270
+#define FIRST_BLOCK_OF_NEXT_PLANE__VALUE 0xffff
+
+#define WRITE_PROTECT 0x280
+#define WRITE_PROTECT__FLAG 0x0001
+
+#define RE_2_RE 0x290
+#define RE_2_RE__VALUE 0x003f
+
+#define MANUFACTURER_ID 0x300
+#define MANUFACTURER_ID__VALUE 0x00ff
+
+#define DEVICE_ID 0x310
+#define DEVICE_ID__VALUE 0x00ff
+
+#define DEVICE_PARAM_0 0x320
+#define DEVICE_PARAM_0__VALUE 0x00ff
+
+#define DEVICE_PARAM_1 0x330
+#define DEVICE_PARAM_1__VALUE 0x00ff
+
+#define DEVICE_PARAM_2 0x340
+#define DEVICE_PARAM_2__VALUE 0x00ff
+
+#define LOGICAL_PAGE_DATA_SIZE 0x350
+#define LOGICAL_PAGE_DATA_SIZE__VALUE 0xffff
+
+#define LOGICAL_PAGE_SPARE_SIZE 0x360
+#define LOGICAL_PAGE_SPARE_SIZE__VALUE 0xffff
+
+#define REVISION 0x370
+#define REVISION__VALUE 0xffff
+
+#define ONFI_DEVICE_FEATURES 0x380
+#define ONFI_DEVICE_FEATURES__VALUE 0x003f
+
+#define ONFI_OPTIONAL_COMMANDS 0x390
+#define ONFI_OPTIONAL_COMMANDS__VALUE 0x003f
+
+#define ONFI_TIMING_MODE 0x3a0
+#define ONFI_TIMING_MODE__VALUE 0x003f
+
+#define ONFI_PGM_CACHE_TIMING_MODE 0x3b0
+#define ONFI_PGM_CACHE_TIMING_MODE__VALUE 0x003f
+
+#define ONFI_DEVICE_NO_OF_LUNS 0x3c0
+#define ONFI_DEVICE_NO_OF_LUNS__NO_OF_LUNS 0x00ff
+#define ONFI_DEVICE_NO_OF_LUNS__ONFI_DEVICE 0x0100
+
+#define ONFI_DEVICE_NO_OF_BLOCKS_PER_LUN_L 0x3d0
+#define ONFI_DEVICE_NO_OF_BLOCKS_PER_LUN_L__VALUE 0xffff
+
+#define ONFI_DEVICE_NO_OF_BLOCKS_PER_LUN_U 0x3e0
+#define ONFI_DEVICE_NO_OF_BLOCKS_PER_LUN_U__VALUE 0xffff
+
+#define FEATURES 0x3f0
+#define FEATURES__N_BANKS 0x0003
+#define FEATURES__ECC_MAX_ERR 0x003c
+#define FEATURES__DMA 0x0040
+#define FEATURES__CMD_DMA 0x0080
+#define FEATURES__PARTITION 0x0100
+#define FEATURES__XDMA_SIDEBAND 0x0200
+#define FEATURES__GPREG 0x0400
+#define FEATURES__INDEX_ADDR 0x0800
+
+#define TRANSFER_MODE 0x400
+#define TRANSFER_MODE__VALUE 0x0003
+
+#define INTR_STATUS(__bank) (0x410 + ((__bank) * 0x50))
+#define INTR_EN(__bank) (0x420 + ((__bank) * 0x50))
+
+/*
+ * Some versions of the IP have the ECC fixup handled in hardware. In this
+ * configuration we only get interrupted when the error is uncorrectable.
+ * Unfortunately this bit replaces INTR_STATUS__ECC_TRANSACTION_DONE from the
+ * old IP.
+ */
+#define INTR_STATUS__ECC_UNCOR_ERR 0x0001
+#define INTR_STATUS__ECC_TRANSACTION_DONE 0x0001
+#define INTR_STATUS__ECC_ERR 0x0002
+#define INTR_STATUS__DMA_CMD_COMP 0x0004
+#define INTR_STATUS__TIME_OUT 0x0008
+#define INTR_STATUS__PROGRAM_FAIL 0x0010
+#define INTR_STATUS__ERASE_FAIL 0x0020
+#define INTR_STATUS__LOAD_COMP 0x0040
+#define INTR_STATUS__PROGRAM_COMP 0x0080
+#define INTR_STATUS__ERASE_COMP 0x0100
+#define INTR_STATUS__PIPE_CPYBCK_CMD_COMP 0x0200
+#define INTR_STATUS__LOCKED_BLK 0x0400
+#define INTR_STATUS__UNSUP_CMD 0x0800
+#define INTR_STATUS__INT_ACT 0x1000
+#define INTR_STATUS__RST_COMP 0x2000
+#define INTR_STATUS__PIPE_CMD_ERR 0x4000
+#define INTR_STATUS__PAGE_XFER_INC 0x8000
+
+#define INTR_EN__ECC_TRANSACTION_DONE 0x0001
+#define INTR_EN__ECC_ERR 0x0002
+#define INTR_EN__DMA_CMD_COMP 0x0004
+#define INTR_EN__TIME_OUT 0x0008
+#define INTR_EN__PROGRAM_FAIL 0x0010
+#define INTR_EN__ERASE_FAIL 0x0020
+#define INTR_EN__LOAD_COMP 0x0040
+#define INTR_EN__PROGRAM_COMP 0x0080
+#define INTR_EN__ERASE_COMP 0x0100
+#define INTR_EN__PIPE_CPYBCK_CMD_COMP 0x0200
+#define INTR_EN__LOCKED_BLK 0x0400
+#define INTR_EN__UNSUP_CMD 0x0800
+#define INTR_EN__INT_ACT 0x1000
+#define INTR_EN__RST_COMP 0x2000
+#define INTR_EN__PIPE_CMD_ERR 0x4000
+#define INTR_EN__PAGE_XFER_INC 0x8000
+
+#define PAGE_CNT(__bank) (0x430 + ((__bank) * 0x50))
+#define ERR_PAGE_ADDR(__bank) (0x440 + ((__bank) * 0x50))
+#define ERR_BLOCK_ADDR(__bank) (0x450 + ((__bank) * 0x50))
+
+#define DATA_INTR 0x550
+#define DATA_INTR__WRITE_SPACE_AV 0x0001
+#define DATA_INTR__READ_DATA_AV 0x0002
+
+#define DATA_INTR_EN 0x560
+#define DATA_INTR_EN__WRITE_SPACE_AV 0x0001
+#define DATA_INTR_EN__READ_DATA_AV 0x0002
+
+#define GPREG_0 0x570
+#define GPREG_0__VALUE 0xffff
+
+#define GPREG_1 0x580
+#define GPREG_1__VALUE 0xffff
+
+#define GPREG_2 0x590
+#define GPREG_2__VALUE 0xffff
+
+#define GPREG_3 0x5a0
+#define GPREG_3__VALUE 0xffff
+
+#define ECC_THRESHOLD 0x600
+#define ECC_THRESHOLD__VALUE 0x03ff
+
+#define ECC_ERROR_BLOCK_ADDRESS 0x610
+#define ECC_ERROR_BLOCK_ADDRESS__VALUE 0xffff
+
+#define ECC_ERROR_PAGE_ADDRESS 0x620
+#define ECC_ERROR_PAGE_ADDRESS__VALUE 0x0fff
+#define ECC_ERROR_PAGE_ADDRESS__BANK 0xf000
+
+#define ECC_ERROR_ADDRESS 0x630
+#define ECC_ERROR_ADDRESS__OFFSET 0x0fff
+#define ECC_ERROR_ADDRESS__SECTOR_NR 0xf000
+
+#define ERR_CORRECTION_INFO 0x640
+#define ERR_CORRECTION_INFO__BYTEMASK 0x00ff
+#define ERR_CORRECTION_INFO__DEVICE_NR 0x0f00
+#define ERR_CORRECTION_INFO__ERROR_TYPE 0x4000
+#define ERR_CORRECTION_INFO__LAST_ERR_INFO 0x8000
+
+#define DMA_ENABLE 0x700
+#define DMA_ENABLE__FLAG 0x0001
+
+#define IGNORE_ECC_DONE 0x710
+#define IGNORE_ECC_DONE__FLAG 0x0001
+
+#define DMA_INTR 0x720
+#define DMA_INTR__TARGET_ERROR 0x0001
+#define DMA_INTR__DESC_COMP_CHANNEL0 0x0002
+#define DMA_INTR__DESC_COMP_CHANNEL1 0x0004
+#define DMA_INTR__DESC_COMP_CHANNEL2 0x0008
+#define DMA_INTR__DESC_COMP_CHANNEL3 0x0010
+#define DMA_INTR__MEMCOPY_DESC_COMP 0x0020
+
+#define DMA_INTR_EN 0x730
+#define DMA_INTR_EN__TARGET_ERROR 0x0001
+#define DMA_INTR_EN__DESC_COMP_CHANNEL0 0x0002
+#define DMA_INTR_EN__DESC_COMP_CHANNEL1 0x0004
+#define DMA_INTR_EN__DESC_COMP_CHANNEL2 0x0008
+#define DMA_INTR_EN__DESC_COMP_CHANNEL3 0x0010
+#define DMA_INTR_EN__MEMCOPY_DESC_COMP 0x0020
+
+#define TARGET_ERR_ADDR_LO 0x740
+#define TARGET_ERR_ADDR_LO__VALUE 0xffff
+
+#define TARGET_ERR_ADDR_HI 0x750
+#define TARGET_ERR_ADDR_HI__VALUE 0xffff
+
+#define CHNL_ACTIVE 0x760
+#define CHNL_ACTIVE__CHANNEL0 0x0001
+#define CHNL_ACTIVE__CHANNEL1 0x0002
+#define CHNL_ACTIVE__CHANNEL2 0x0004
+#define CHNL_ACTIVE__CHANNEL3 0x0008
+
+#define ACTIVE_SRC_ID 0x800
+#define ACTIVE_SRC_ID__VALUE 0x00ff
+
+#define PTN_INTR 0x810
+#define PTN_INTR__CONFIG_ERROR 0x0001
+#define PTN_INTR__ACCESS_ERROR_BANK0 0x0002
+#define PTN_INTR__ACCESS_ERROR_BANK1 0x0004
+#define PTN_INTR__ACCESS_ERROR_BANK2 0x0008
+#define PTN_INTR__ACCESS_ERROR_BANK3 0x0010
+#define PTN_INTR__REG_ACCESS_ERROR 0x0020
+
+#define PTN_INTR_EN 0x820
+#define PTN_INTR_EN__CONFIG_ERROR 0x0001
+#define PTN_INTR_EN__ACCESS_ERROR_BANK0 0x0002
+#define PTN_INTR_EN__ACCESS_ERROR_BANK1 0x0004
+#define PTN_INTR_EN__ACCESS_ERROR_BANK2 0x0008
+#define PTN_INTR_EN__ACCESS_ERROR_BANK3 0x0010
+#define PTN_INTR_EN__REG_ACCESS_ERROR 0x0020
+
+#define PERM_SRC_ID(__bank) (0x830 + ((__bank) * 0x40))
+#define PERM_SRC_ID__SRCID 0x00ff
+#define PERM_SRC_ID__DIRECT_ACCESS_ACTIVE 0x0800
+#define PERM_SRC_ID__WRITE_ACTIVE 0x2000
+#define PERM_SRC_ID__READ_ACTIVE 0x4000
+#define PERM_SRC_ID__PARTITION_VALID 0x8000
+
+#define MIN_BLK_ADDR(__bank) (0x840 + ((__bank) * 0x40))
+#define MIN_BLK_ADDR__VALUE 0xffff
+
+#define MAX_BLK_ADDR(__bank) (0x850 + ((__bank) * 0x40))
+#define MAX_BLK_ADDR__VALUE 0xffff
+
+#define MIN_MAX_BANK(__bank) (0x860 + ((__bank) * 0x40))
+#define MIN_MAX_BANK__MIN_VALUE 0x0003
+#define MIN_MAX_BANK__MAX_VALUE 0x000c
+
+
+/* ffsdefs.h */
+#define CLEAR 0 /*use this to clear a field instead of "fail"*/
+#define SET 1 /*use this to set a field instead of "pass"*/
+#define FAIL 1 /*failed flag*/
+#define PASS 0 /*success flag*/
+#define ERR -1 /*error flag*/
+
+/* lld.h */
+#define GOOD_BLOCK 0
+#define DEFECTIVE_BLOCK 1
+#define READ_ERROR 2
+
+#define CLK_X 5
+#define CLK_MULTI 4
+
+/* spectraswconfig.h */
+#define CMD_DMA 0
+
+#define SPECTRA_PARTITION_ID 0
+/**** Block Table and Reserved Block Parameters *****/
+#define SPECTRA_START_BLOCK 3
+#define NUM_FREE_BLOCKS_GATE 30
+
+/* KBV - Updated to LNW scratch register address */
+#define SCRATCH_REG_ADDR CONFIG_MTD_NAND_DENALI_SCRATCH_REG_ADDR
+#define SCRATCH_REG_SIZE 64
+
+#define GLOB_HWCTL_DEFAULT_BLKS 2048
+
+#define SUPPORT_15BITECC 1
+#define SUPPORT_8BITECC 1
+
+#define CUSTOM_CONF_PARAMS 0
+
+#define ONFI_BLOOM_TIME 1
+#define MODE5_WORKAROUND 0
+
+/* 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.
+ *
+ */
+
+#ifndef _LLD_NAND_
+#define _LLD_NAND_
+
+#define MODE_00 0x00000000
+#define MODE_01 0x04000000
+#define MODE_10 0x08000000
+#define MODE_11 0x0C000000
+
+
+#define DATA_TRANSFER_MODE 0
+#define PROTECTION_PER_BLOCK 1
+#define LOAD_WAIT_COUNT 2
+#define PROGRAM_WAIT_COUNT 3
+#define ERASE_WAIT_COUNT 4
+#define INT_MONITOR_CYCLE_COUNT 5
+#define READ_BUSY_PIN_ENABLED 6
+#define MULTIPLANE_OPERATION_SUPPORT 7
+#define PRE_FETCH_MODE 8
+#define CE_DONT_CARE_SUPPORT 9
+#define COPYBACK_SUPPORT 10
+#define CACHE_WRITE_SUPPORT 11
+#define CACHE_READ_SUPPORT 12
+#define NUM_PAGES_IN_BLOCK 13
+#define ECC_ENABLE_SELECT 14
+#define WRITE_ENABLE_2_READ_ENABLE 15
+#define ADDRESS_2_DATA 16
+#define READ_ENABLE_2_WRITE_ENABLE 17
+#define TWO_ROW_ADDRESS_CYCLES 18
+#define MULTIPLANE_ADDRESS_RESTRICT 19
+#define ACC_CLOCKS 20
+#define READ_WRITE_ENABLE_LOW_COUNT 21
+#define READ_WRITE_ENABLE_HIGH_COUNT 22
+
+#define ECC_SECTOR_SIZE 512
+
+#define DENALI_BUF_SIZE (NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE)
+
+struct nand_buf {
+ int head;
+ int tail;
+ /* seprating dma_buf as buf can be used for status read purpose */
+ uint8_t dma_buf[DENALI_BUF_SIZE] __aligned(64);
+ uint8_t buf[DENALI_BUF_SIZE];
+};
+
+#define INTEL_CE4100 1
+#define INTEL_MRST 2
+#define DT 3
+
+struct denali_nand_info {
+ struct mtd_info mtd;
+ struct nand_chip *nand;
+
+ int flash_bank; /* currently selected chip */
+ int status;
+ int platform;
+ struct nand_buf buf;
+ struct device *dev;
+ int total_used_banks;
+ uint32_t block; /* stored for future use */
+ uint32_t page;
+ void __iomem *flash_reg; /* Mapped io reg base address */
+ void __iomem *flash_mem; /* Mapped io reg base address */
+
+ /* elements used by ISR */
+ /*struct completion complete;*/
+
+ uint32_t irq_status;
+ int irq_debug_array[32];
+ int idx;
+ int irq;
+
+ uint32_t devnum; /* represent how many nands connected */
+ uint32_t fwblks; /* represent how many blocks FW used */
+ uint32_t totalblks;
+ uint32_t blksperchip;
+ uint32_t bbtskipbytes;
+ uint32_t max_banks;
+};
+
+#endif /*_LLD_NAND_*/
--
1.7.9.5
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-02-21 20:51 [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support Chin Liang See
@ 2014-02-24 7:48 ` Michal Simek
2014-02-24 8:06 ` Masahiro Yamada
2014-02-27 17:04 ` Chin Liang See
2014-02-27 14:35 ` Masahiro Yamada
` (2 subsequent siblings)
3 siblings, 2 replies; 27+ messages in thread
From: Michal Simek @ 2014-02-24 7:48 UTC (permalink / raw)
To: u-boot
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@altera.com>
> Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> Cc: David Woodhouse <David.Woodhouse@intel.com>
> Cc: Brian Norris <computersforpeace@gmail.com>
> Cc: Scott Wood <scottwood@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?
> + * 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
> + status = read_interrupt_status();
> + clear_interrupt(status);
> + denali.irq_status = 0x0;
just 0
> +}
> +
> +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?
> +
> + 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.
> +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
> + __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?
<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?
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140224/997697f0/attachment.pgp>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-02-24 7:48 ` Michal Simek
@ 2014-02-24 8:06 ` Masahiro Yamada
2014-02-24 8:16 ` Michal Simek
2014-02-27 17:04 ` Chin Liang See
1 sibling, 1 reply; 27+ messages in thread
From: Masahiro Yamada @ 2014-02-24 8:06 UTC (permalink / raw)
To: u-boot
Hello Michal,
These files were imported from Linux Kernel.
(drivers/mtd/nand/denali.[ch])
I guess Chin does not want to change the code
unless it is really necessary.
(And I like this way.
We can easily find which parts were adjusted by diffing.)
But, good catch!
I think your feedback is highly appreciated for Linux folks.
Can you post your feedback to Linux Kernel?
> > --- /dev/null
> > +++ b/drivers/mtd/nand/denali_nand.c
> > @@ -0,0 +1,1166 @@
> > +/*
> > + * Copyright (C) 2013 Altera Corporation <www.altera.com>
>
> What about 2014?
I agree that this part should be fixed at the next version.
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-02-24 8:06 ` Masahiro Yamada
@ 2014-02-24 8:16 ` Michal Simek
0 siblings, 0 replies; 27+ messages in thread
From: Michal Simek @ 2014-02-24 8:16 UTC (permalink / raw)
To: u-boot
Hi Masahiro.
On 02/24/2014 09:06 AM, Masahiro Yamada wrote:
> Hello Michal,
>
> These files were imported from Linux Kernel.
> (drivers/mtd/nand/denali.[ch])
then they should be fixed too. Or better fix kernel
driver first and then add these changes to u-boot.
Checkpatch in the u-boot is just the same as is in the kernel.
> I guess Chin does not want to change the code
> unless it is really necessary.
> (And I like this way.
> We can easily find which parts were adjusted by diffing.)
I have no problem that you want to keep that code synchronized
for easier diffing but adding incorrect code is just really bad.
And you shouldn't just copy what's wrong.
> But, good catch!
> I think your feedback is highly appreciated for Linux folks.
> Can you post your feedback to Linux Kernel?
The driver is in mainline from 2010 that's why go and fix it.
Make no sense for me to send this to linux kernel because
the reaction will be that I should fix it and I have no interest
to fix it.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140224/8e9c90f9/attachment.pgp>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-02-21 20:51 [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support Chin Liang See
2014-02-24 7:48 ` Michal Simek
@ 2014-02-27 14:35 ` Masahiro Yamada
2014-02-27 21:02 ` Chin Liang See
2014-03-04 0:03 ` Scott Wood
2014-05-30 10:50 ` Rik Smith
3 siblings, 1 reply; 27+ messages in thread
From: Masahiro Yamada @ 2014-02-27 14:35 UTC (permalink / raw)
To: u-boot
Hello Chin,
> +
> + 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;
You set nand->ecc.mode = NAND_ECC_HW,
but it looks like you don't set nand->ecc.strength.
So, I think initialization will fail in nand_scan_tail() function.
Here,
if (mtd->writesize >= chip->ecc.size) {
if (!chip->ecc.strength) {
pr_warn("Driver must set ecc.strength when using hardware ECC\n");
BUG();
}
break;
Where do you set nand->ecc.strength?
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-02-24 7:48 ` Michal Simek
2014-02-24 8:06 ` Masahiro Yamada
@ 2014-02-27 17:04 ` Chin Liang See
2014-02-28 10:37 ` Michal Simek
1 sibling, 1 reply; 27+ messages in thread
From: Chin Liang See @ 2014-02-27 17:04 UTC (permalink / raw)
To: u-boot
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@altera.com>
> > Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> > Cc: David Woodhouse <David.Woodhouse@intel.com>
> > Cc: Brian Norris <computersforpeace@gmail.com>
> > Cc: Scott Wood <scottwood@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
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-02-27 14:35 ` Masahiro Yamada
@ 2014-02-27 21:02 ` Chin Liang See
2014-02-27 22:32 ` Scott Wood
0 siblings, 1 reply; 27+ messages in thread
From: Chin Liang See @ 2014-02-27 21:02 UTC (permalink / raw)
To: u-boot
Hi Masahiro,
On Thu, 2014-02-27 at 23:35 +0900, Masahiro Yamada wrote:
> Hello Chin,
>
>
> > +
> > + 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;
>
> You set nand->ecc.mode = NAND_ECC_HW,
> but it looks like you don't set nand->ecc.strength.
>
> So, I think initialization will fail in nand_scan_tail() function.
>
> Here,
>
> if (mtd->writesize >= chip->ecc.size) {
> if (!chip->ecc.strength) {
> pr_warn("Driver must set ecc.strength when using hardware ECC\n");
> BUG();
> }
> break;
>
>
>
> Where do you set nand->ecc.strength?
I believe this is only applicable for NAND_ECC_HW_SYNDROME mode. We are
using the NAND_ECC_HW (without the syndrome). Wonder you hit error
during run?
Chin Liang
>
>
>
>
> Best Regards
> Masahiro Yamada
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-02-27 21:02 ` Chin Liang See
@ 2014-02-27 22:32 ` Scott Wood
2014-02-27 23:03 ` Chin Liang See
0 siblings, 1 reply; 27+ messages in thread
From: Scott Wood @ 2014-02-27 22:32 UTC (permalink / raw)
To: u-boot
On Thu, 2014-02-27 at 15:02 -0600, Chin Liang See wrote:
> Hi Masahiro,
>
> On Thu, 2014-02-27 at 23:35 +0900, Masahiro Yamada wrote:
> > Hello Chin,
> >
> >
> > > +
> > > + 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;
> >
> > You set nand->ecc.mode = NAND_ECC_HW,
> > but it looks like you don't set nand->ecc.strength.
> >
> > So, I think initialization will fail in nand_scan_tail() function.
> >
> > Here,
> >
> > if (mtd->writesize >= chip->ecc.size) {
> > if (!chip->ecc.strength) {
> > pr_warn("Driver must set ecc.strength when using hardware ECC\n");
> > BUG();
> > }
> > break;
> >
> >
> >
> > Where do you set nand->ecc.strength?
>
> I believe this is only applicable for NAND_ECC_HW_SYNDROME mode. We are
> using the NAND_ECC_HW (without the syndrome). Wonder you hit error
> during run?
No, it must always be set for hardware ECC. Note the lack of a break;
before case NAND_ECC_HW_SYNDROME.
-Scott
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-02-27 22:32 ` Scott Wood
@ 2014-02-27 23:03 ` Chin Liang See
2014-02-28 12:57 ` Masahiro Yamada
0 siblings, 1 reply; 27+ messages in thread
From: Chin Liang See @ 2014-02-27 23:03 UTC (permalink / raw)
To: u-boot
Hi Scott,
On Thu, 2014-02-27 at 16:32 -0600, Scott Wood wrote:
> On Thu, 2014-02-27 at 15:02 -0600, Chin Liang See wrote:
> > Hi Masahiro,
> >
> > On Thu, 2014-02-27 at 23:35 +0900, Masahiro Yamada wrote:
> > > Hello Chin,
> > >
> > >
> > > > +
> > > > + 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;
> > >
> > > You set nand->ecc.mode = NAND_ECC_HW,
> > > but it looks like you don't set nand->ecc.strength.
> > >
> > > So, I think initialization will fail in nand_scan_tail() function.
> > >
> > > Here,
> > >
> > > if (mtd->writesize >= chip->ecc.size) {
> > > if (!chip->ecc.strength) {
> > > pr_warn("Driver must set ecc.strength when using hardware ECC\n");
> > > BUG();
> > > }
> > > break;
> > >
> > >
> > >
> > > Where do you set nand->ecc.strength?
> >
> > I believe this is only applicable for NAND_ECC_HW_SYNDROME mode. We are
> > using the NAND_ECC_HW (without the syndrome). Wonder you hit error
> > during run?
>
> No, it must always be set for hardware ECC. Note the lack of a break;
> before case NAND_ECC_HW_SYNDROME.
Good catch, thanks Scott!
Hi Masahiro,
I rechecked my documentation and the value is 8.
The data sector size is 512 bytes while ECC sector size is 14 bytes.
With that, the controller able to auto correct up to 8 bits.
This is how a page will look like
512 bytes data | 14 bytes ECC | 512 bytes data | 14 bytes ECC | 512
bytes data | 14 bytes ECC | 470 bytes data | 2 byte for bad block marker
| 42 bytes data | 14 bytes ECC | unused
FYI, my documentation is located at
http://rocketboards.org/gitweb/?p=u-boot-socfpga.git;a=blob_plain;f=doc/README.SOCFPGA;hb=refs/heads/socfpga_v2013.01.01
Chin Liang
>
> -Scott
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-02-27 17:04 ` Chin Liang See
@ 2014-02-28 10:37 ` Michal Simek
2014-03-04 23:57 ` Chin Liang See
0 siblings, 1 reply; 27+ messages in thread
From: Michal Simek @ 2014-02-28 10:37 UTC (permalink / raw)
To: u-boot
>>> +/* 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.
1. Using address in license is just wrong and it should be removed
because they can move to different location.
2. u-boot adopted SPDX and all these fragments were removed.
Wolfgang/Tom: IMHO this should be also changed/fixed.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140228/f6bf0e35/attachment.pgp>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-02-27 23:03 ` Chin Liang See
@ 2014-02-28 12:57 ` Masahiro Yamada
2014-03-05 0:24 ` Chin Liang See
0 siblings, 1 reply; 27+ messages in thread
From: Masahiro Yamada @ 2014-02-28 12:57 UTC (permalink / raw)
To: u-boot
Hello Chin,
> > > > Where do you set nand->ecc.strength?
> > >
> > > I believe this is only applicable for NAND_ECC_HW_SYNDROME mode. We are
> > > using the NAND_ECC_HW (without the syndrome). Wonder you hit error
> > > during run?
> >
> > No, it must always be set for hardware ECC. Note the lack of a break;
> > before case NAND_ECC_HW_SYNDROME.
>
> Good catch, thanks Scott!
ecc.strengh must be set for NAND_ECC_HW.
Otherwise, error message will be displayed and reboot.
NAND: NAND: Denali NAND controller
BUG: failure at drivers/mtd/nand/nand_base.c:3224/nand_scan_tail()!
BUG!
resetting ...
You said this driver was tested against 3 different devices.
In that case, I can't understand how your board passed
through nand_scan_tail().
Anyway, I modifed denali_nand_init() locally
to set nand->ecc.strength.
> Hi Masahiro,
>
> I rechecked my documentation and the value is 8.
> The data sector size is 512 bytes while ECC sector size is 14 bytes.
> With that, the controller able to auto correct up to 8 bits.
> This is how a page will look like
>
> 512 bytes data | 14 bytes ECC | 512 bytes data | 14 bytes ECC | 512
> bytes data | 14 bytes ECC | 470 bytes data | 2 byte for bad block marker
> | 42 bytes data | 14 bytes ECC | unused
>
> FYI, my documentation is located at
> http://rocketboards.org/gitweb/?p=u-boot-socfpga.git;a=blob_plain;f=doc/README.SOCFPGA;hb=refs/heads/socfpga_v2013.01.01
For SOCFPAG, Denali controller IP is configured with
512 byte ECC sector size. OK.
I refer to "Denali NAND Flash Memory Controller User's Guide".
Accoding to it, Denali's IP has 2 choice for ECC sector size:
512 byte or 1024 byte.
Panasonic's UniPhier SoCs adopt 1024 byte ECC sector size.
(CONFIG_NAND_DENALI_ECC_SIZE = nand->ecc.size = 1024 for us.)
ECC strength (nand->ecc.strength) is selectable from 8bit/16bit/24bit.
(They correspond to nand->ecc.bytes = 14, 28, 42, respectively)
So, In our SoC s
1024 byte data | {14 or 28 or 42 byte ECC} | 1024 byte data |
{14 or 28 or 42 byte ECC} ...
> #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 */
I'm afraid this part works only for 512 byte ECC sector.
Denali's document says
For 512B ECC sector size,
ecc.bytes = Ceiling to next word (13 * ecc.strength)
For 1024B ECC sector size,
ecc.bytes = Ceiling to next word (14 * ecc.strength)
And denali_setup_dma_sequence() function
(Why did you rename this function?)
did not work either.
I needed to fix it locally.
So bad news is this version itself does not work for me.
Good news is I could adjust it locally and confirmed some features
worked. (But I think I need more test.)
So, how will this situation work?
It turned out there are some differences between
two Denali hardwares and this driver works only for yours.
You merge it first, and (if you don't mind) shall I modify it
in a more generic way to run on both hardwares?
> If you want to run under SPL, there are some patches for that. Let me
> know if you need that. While for U-Boot, they are working fine. Probably
Thanks for your offering help.
But I am not sure if SOCFPGA and UniPhier can share a SPL nand driver.
(Actually I have locally our own Denali driver for SPL.
And I have Denali driver for main U-Boot, which is adjusted for
our SoCs, too.
But I don't mind to switch onto your driver if it works for me.)
> +#define CONFIG_SYS_NAND_USE_FLASH_BBT
> +#define CONFIG_SYS_NAND_REGS_BASE SOCFPGA_NAND_REGS_ADDRESS
> +#define CONFIG_SYS_NAND_DATA_BASE SOCFPGA_NAND_DATA_ADDRESS
> +#define CONFIG_SYS_NAND_BASE CONFIG_SYS_NAND_REGS_BASE
Maybe
#define CONFIG_SYS_NAND_BASE (SOCFPGA_NAND_DATA_ADDRESS + 0x10)
?
BTW, you changed all denali->foo to denali.foo.
It looks unnecessay to me.
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-02-21 20:51 [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support Chin Liang See
2014-02-24 7:48 ` Michal Simek
2014-02-27 14:35 ` Masahiro Yamada
@ 2014-03-04 0:03 ` Scott Wood
2014-03-04 10:31 ` Masahiro Yamada
2014-03-05 17:34 ` Chin Liang See
2014-05-30 10:50 ` Rik Smith
3 siblings, 2 replies; 27+ messages in thread
From: Scott Wood @ 2014-03-04 0:03 UTC (permalink / raw)
To: u-boot
On Fri, 2014-02-21 at 14:51 -0600, 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@altera.com>
> Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> Cc: David Woodhouse <David.Woodhouse@intel.com>
> Cc: Brian Norris <computersforpeace@gmail.com>
> Cc: Scott Wood <scottwood@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
It's "denali.c" in Linux -- why "denali_nand.c" here?
> @@ -0,0 +1,1166 @@
> +/*
> + * Copyright (C) 2013 Altera Corporation <www.altera.com>
> + * 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
A module parameter? In U-Boot?
Sharing code with Linux is fine, but try to edit out the stuff that's
irrelevant in U-Boot.
> +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);
> +}
Why are you using raw I/O accessors? The Linux driver doesn't do this.
> +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;
> + status = read_interrupt_status();
> + clear_interrupt(status);
> + denali.irq_status = 0x0;
> +}
> +
> +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 {
> + 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);
This looks like a much shorter timeout than Linux uses (1000us versus
1000ms). Though FWIW the Linux timeout code looks buggy.
Also, comp_res is a very odd name for a timeout variable.
> +/* 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.
> +*/
> +static void index_addr(uint32_t address, uint32_t data)
> +{
> + __raw_writel(address, denali.flash_mem);
> + __raw_writel(data, denali.flash_mem + 0x10);
> +}
What is 0x10? No magic numbers, please.
> +
> +/* 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;
> + 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++)
> + __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;
Don't align continuation lines with an if/for body -- it makes it harder
to see where one ends and the other begins.
Why PASS/FAIL rather than normal "0 on success, negative error code on
error"? Why uint16_t?
> + 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); */
Don't add commented-out-code.
> +/* determines how many NAND chips are connected to the controller. Note for
> + * Intel CE4100 devices we don't support more than one device.
> + */
> +static void find_valid_banks(void)
> +{
> + uint32_t id[denali.max_banks];
> + int i;
> +
> + denali.total_used_banks = 1;
> + for (i = 0; i < denali.max_banks; i++) {
> + index_addr((uint32_t)(MODE_11 | (i << 24) | 0), 0x90);
> + index_addr((uint32_t)(MODE_11 | (i << 24) | 1), 0);
> + index_addr_read_data((uint32_t)(MODE_11 | (i << 24) | 2),
> + &id[i]);
> +
> + if (i == 0) {
> + if (!(id[i] & 0x0ff))
> + break; /* WTF? */
I'm not sure that this is the most helpful comment that could go here.
> +static void detect_partition_feature(void)
> +{
> + /* For MRST platform, denali.fwblks represent the
> + * number of blocks firmware is taken,
> + * FW is in protect partition and MTD driver has no
> + * permission to access it. So let driver know how many
> + * blocks it can't touch.
> + * */
> + if (__raw_readl(denali.flash_reg + FEATURES) & FEATURES__PARTITION) {
> + if ((__raw_readl(denali.flash_reg + PERM_SRC_ID(1)) &
> + PERM_SRC_ID__SRCID) == SPECTRA_PARTITION_ID) {
> + denali.fwblks =
> + ((__raw_readl(denali.flash_reg + MIN_MAX_BANK(1)) &
> + MIN_MAX_BANK__MIN_VALUE) *
> + denali.blksperchip)
> + +
> + (__raw_readl(denali.flash_reg + MIN_BLK_ADDR(1)) &
> + MIN_BLK_ADDR__VALUE);
> + } else
> + denali.fwblks = SPECTRA_START_BLOCK;
> + } else
> + denali.fwblks = SPECTRA_START_BLOCK;
If braces are needed on one side of if/else, use on both sides.
> +static void denali_set_intr_modes(uint16_t INT_ENABLE)
> +{
> + if (INT_ENABLE)
> + __raw_writel(1, denali.flash_reg + GLOBAL_INT_ENABLE);
> + else
> + __raw_writel(0, denali.flash_reg + GLOBAL_INT_ENABLE);
> +}
CAPS are for macros, not function parameters.
Why not just require the caller to pass in 0/1 rather than 0/non-0 and
write that directly to the register?
> +/* helper function that simply writes a buffer to the flash */
> +static int write_data_to_flash_mem(const uint8_t *buf,
> + int len)
There's no point in a continuation line if you indent so far that you're
beyond where the previous line ended. If the above doesn't violate 80
columns, then you could just as well do it on one line.
> +{
> + uint32_t i = 0, *buf32;
> +
> + /* verify that the len is a multiple of 4. see comment in
> + * read_data_from_flash_mem() */
> + BUG_ON((len % 4) != 0);
> +
> + /* write the data to the flash memory */
> + buf32 = (uint32_t *)buf;
> + for (i = 0; i < len / 4; i++)
> + __raw_writel(*buf32++, denali.flash_mem + 0x10);
This violates C99 aliasing rules.
> + return i*4; /* intent is to return the number of bytes read */
> +}
> +
> +static void denali_mode_main_access(void)
> +{
> + uint32_t addr, cmd;
> + addr = BANK(denali.flash_bank) | denali.page;
> + cmd = MODE_10 | addr;
> + index_addr((uint32_t)cmd, MAIN_ACCESS);
Unnecessary cast.
> +/* raw include ECC value and all the spare area */
> +static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> + uint8_t *buf, int oob_required, int page)
> +{
> + uint32_t irq_status, irq_mask = INTR_STATUS__DMA_CMD_COMP;
> +
> + debug("denali_read_page_raw at page %08x\n", page);
> + if (denali.page != page) {
> + debug("Missing NAND_CMD_READ0 command\n");
> + return -EIO;
> + }
> +
> + if (oob_required)
> + /* switch to main + spare access */
> + denali_mode_main_spare_access();
> + else
> + /* switch to main access only */
> + denali_mode_main_access();
> +
> + /* setting up the DMA where ecc_enable is false */
> + irq_status = denali_dma_configuration(DENALI_READ, true, irq_mask,
> + oob_required);
> +
> + /* if timeout happen, error out */
> + if (!(irq_status & INTR_STATUS__DMA_CMD_COMP)) {
> + debug("DMA timeout for denali_read_page_raw\n");
> + return -EIO;
> + }
> +
> + /* splitting the content to destination buffer holder */
> + memcpy(chip->oob_poi, (const void *)(denali.buf.dma_buf +
> + mtd->writesize), mtd->oobsize);
> + memcpy(buf, (const void *)denali.buf.dma_buf, mtd->writesize);
Unnecessary casts.
> +static void denali_erase(struct mtd_info *mtd, int page)
> +{
> + uint32_t cmd = 0x0, irq_status = 0;
> +
> + debug("denali_erase at page %08x\n", page);
> +
> + /* clear interrupts */
> + clear_interrupts();
> +
> + /* setup page read request for access type */
> + cmd = MODE_10 | BANK(denali.flash_bank) | page;
> + index_addr((uint32_t)cmd, 0x1);
> +
> + /* wait for erase to complete or failure to occur */
> + irq_status = wait_for_irq(INTR_STATUS__ERASE_COMP |
> + INTR_STATUS__ERASE_FAIL);
> +
> + if (irq_status & INTR_STATUS__ERASE_FAIL ||
> + irq_status & INTR_STATUS__LOCKED_BLK)
> + denali.status = NAND_STATUS_FAIL;
> + else
> + denali.status = PASS;
> +}
So PASS/FAIL are supposed to be in the same numberspace as
NAND_STATUS_foo? Why use a separate FAIL elsewhere, then?
> +static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
> + int page)
> +{
> + uint32_t addr;
> +
> + switch (cmd) {
> + case NAND_CMD_PAGEPROG:
> + break;
> + case NAND_CMD_STATUS:
> + addr = (uint32_t)MODE_11 | BANK(denali.flash_bank);
> + index_addr((uint32_t)addr | 0, cmd);
More unnecessary casts...
> +/* 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)
> +{
> + debug("Should not be called as ECC handled by hardware\n");
> + BUG();
> + return -EIO;
> +}
> +
> +static int denali_ecc_correct(struct mtd_info *mtd, uint8_t *data,
> + uint8_t *read_ecc, uint8_t *calc_ecc)
> +{
> + debug("Should not be called as ECC handled by hardware\n");
> + BUG();
> + return -EIO;
> +}
> +
> +static void denali_ecc_hwctl(struct mtd_info *mtd, int mode)
> +{
> + debug("Should not be called as ECC handled by hardware\n");
> + BUG();
> +}
These debug() messages seem superfluous given the BUG() will identify
the line in the code. debug() is not for commenting code.
> +/* ffsdefs.h */
> +#define CLEAR 0 /*use this to clear a field instead of "fail"*/
> +#define SET 1 /*use this to set a field instead of "pass"*/
> +#define FAIL 1 /*failed flag*/
> +#define PASS 0 /*success flag*/
> +#define ERR -1 /*error flag*/
What's the difference between FAIL and ERR?
-Scott
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-03-04 0:03 ` Scott Wood
@ 2014-03-04 10:31 ` Masahiro Yamada
2014-03-04 18:44 ` Scott Wood
2014-03-05 17:40 ` Chin Liang See
2014-03-05 17:34 ` Chin Liang See
1 sibling, 2 replies; 27+ messages in thread
From: Masahiro Yamada @ 2014-03-04 10:31 UTC (permalink / raw)
To: u-boot
Hello Scott, Chin,
> > +/* 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);
> > +}
>
> Why are you using raw I/O accessors? The Linux driver doesn't do this.
Add ioread32/iowrite32 to arch/arm/include/asm/io.h
and use them?
> > +
> > +static uint32_t wait_for_irq(uint32_t irq_mask)
> > +{
> > + unsigned long comp_res = 1000;
> > + uint32_t intr_status = 0;
> > +
> > + 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);
>
> This looks like a much shorter timeout than Linux uses (1000us versus
> 1000ms). Though FWIW the Linux timeout code looks buggy.
>
> Also, comp_res is a very odd name for a timeout variable.
Right. I think wait time is too short.
When I tested this code on my board, timeout error always
occurred on page write command.
I had to fix it to run write command.
s/udelay(1)/udelay(1000)/
There is another problem.
I think there is a cache coherency problem in this driver code.
DMA is used in this driver but D-cache is never flushed.
When D-cache is on (CONFIG_SYS_DCACHE_OFF is not defined),
ARM processor writes to/reads from the buffer through D-cache.
On the other hand, Denali NAND controller always wites to/reads from
the buffer on physical memory.
So, this driver writes/reads wrong data.
I had to add flush_dcache_range() function call
in denali_setup_dma_sequence().
@@ -689,6 +689,8 @@ static void denali_setup_dma_sequence(int op)
uint32_t mode;
uint32_t addr = (uint32_t)denali.buf.dma_buf;
+ flush_dcache_range(addr, addr + sizeof(denali.buf.dma_buf));
+
mode = MODE_10 | BANK(denali.flash_bank);
/* DMA is a four step process */
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-03-04 10:31 ` Masahiro Yamada
@ 2014-03-04 18:44 ` Scott Wood
2014-03-05 17:40 ` Chin Liang See
1 sibling, 0 replies; 27+ messages in thread
From: Scott Wood @ 2014-03-04 18:44 UTC (permalink / raw)
To: u-boot
On Tue, 2014-03-04 at 19:31 +0900, Masahiro Yamada wrote:
> Hello Scott, Chin,
>
>
> > > +/* 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);
> > > +}
> >
> > Why are you using raw I/O accessors? The Linux driver doesn't do this.
>
> Add ioread32/iowrite32 to arch/arm/include/asm/io.h
> and use them?
We probably should add them given they're the standard arch and bus
independent accessors in Linux, but you could also use readl()/writel().
Why did you choose the raw version?
> There is another problem.
> I think there is a cache coherency problem in this driver code.
> DMA is used in this driver but D-cache is never flushed.
>
> When D-cache is on (CONFIG_SYS_DCACHE_OFF is not defined),
> ARM processor writes to/reads from the buffer through D-cache.
> On the other hand, Denali NAND controller always wites to/reads from
> the buffer on physical memory.
> So, this driver writes/reads wrong data.
>
> I had to add flush_dcache_range() function call
> in denali_setup_dma_sequence().
Yes, in Linux this would have been handled through the DMA API.
-Scott
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-02-28 10:37 ` Michal Simek
@ 2014-03-04 23:57 ` Chin Liang See
2014-03-05 6:22 ` Michal Simek
0 siblings, 1 reply; 27+ messages in thread
From: Chin Liang See @ 2014-03-04 23:57 UTC (permalink / raw)
To: u-boot
On Fri, 2014-02-28 at 11:37 +0100, Michal Simek wrote:
> >>> +/* 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.
>
> 1. Using address in license is just wrong and it should be removed
> because they can move to different location.
>
> 2. u-boot adopted SPDX and all these fragments were removed.
>
> Wolfgang/Tom: IMHO this should be also changed/fixed.
Just check the latest denali.h in Linux and this license header still
there. But removing the license header put by original author might not
a good thing to do. Unless we can get some approval from Wolfgang or
Jason (who is submitter of this file to Linux).
Thanks
Chin Liang
>
> Thanks,
> Michal
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-02-28 12:57 ` Masahiro Yamada
@ 2014-03-05 0:24 ` Chin Liang See
0 siblings, 0 replies; 27+ messages in thread
From: Chin Liang See @ 2014-03-05 0:24 UTC (permalink / raw)
To: u-boot
Hi Masahiro,
On Fri, 2014-02-28 at 21:57 +0900, Masahiro Yamada wrote:
> Hello Chin,
>
>
> > > > > Where do you set nand->ecc.strength?
> > > >
> > > > I believe this is only applicable for NAND_ECC_HW_SYNDROME mode. We are
> > > > using the NAND_ECC_HW (without the syndrome). Wonder you hit error
> > > > during run?
> > >
> > > No, it must always be set for hardware ECC. Note the lack of a break;
> > > before case NAND_ECC_HW_SYNDROME.
> >
> > Good catch, thanks Scott!
>
> ecc.strengh must be set for NAND_ECC_HW.
>
> Otherwise, error message will be displayed and reboot.
>
> NAND: NAND: Denali NAND controller
> BUG: failure at drivers/mtd/nand/nand_base.c:3224/nand_scan_tail()!
> BUG!
> resetting ...
>
> You said this driver was tested against 3 different devices.
>
> In that case, I can't understand how your board passed
> through nand_scan_tail().
>
> Anyway, I modifed denali_nand_init() locally
> to set nand->ecc.strength.
>
Actually this first revision of this patch was last year. It was tested
again old code base too that time. As I don't have the board with me
now, might need your help for new changes testing. I believe we can work
together to upstream this driver.
>
>
> > Hi Masahiro,
> >
> > I rechecked my documentation and the value is 8.
> > The data sector size is 512 bytes while ECC sector size is 14 bytes.
> > With that, the controller able to auto correct up to 8 bits.
> > This is how a page will look like
> >
> > 512 bytes data | 14 bytes ECC | 512 bytes data | 14 bytes ECC | 512
> > bytes data | 14 bytes ECC | 470 bytes data | 2 byte for bad block marker
> > | 42 bytes data | 14 bytes ECC | unused
> >
> > FYI, my documentation is located at
> > http://rocketboards.org/gitweb/?p=u-boot-socfpga.git;a=blob_plain;f=doc/README.SOCFPGA;hb=refs/heads/socfpga_v2013.01.01
>
> For SOCFPAG, Denali controller IP is configured with
> 512 byte ECC sector size. OK.
>
> I refer to "Denali NAND Flash Memory Controller User's Guide".
>
> Accoding to it, Denali's IP has 2 choice for ECC sector size:
> 512 byte or 1024 byte.
>
> Panasonic's UniPhier SoCs adopt 1024 byte ECC sector size.
> (CONFIG_NAND_DENALI_ECC_SIZE = nand->ecc.size = 1024 for us.)
> ECC strength (nand->ecc.strength) is selectable from 8bit/16bit/24bit.
> (They correspond to nand->ecc.bytes = 14, 28, 42, respectively)
>
> So, In our SoC s
> 1024 byte data | {14 or 28 or 42 byte ECC} | 1024 byte data |
> {14 or 28 or 42 byte ECC} ...
Hmmm from Denali user guide dated Jul 20 2009, 1024 can use correct bit
of 26 or 30 bits. 26 will yield 46 bytes of ECC sector size while 30
will yield 54 bytes sector size.
>
>
> > #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 */
>
> I'm afraid this part works only for 512 byte ECC sector.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-03-04 23:57 ` Chin Liang See
@ 2014-03-05 6:22 ` Michal Simek
0 siblings, 0 replies; 27+ messages in thread
From: Michal Simek @ 2014-03-05 6:22 UTC (permalink / raw)
To: u-boot
On 03/05/2014 12:57 AM, Chin Liang See wrote:
> On Fri, 2014-02-28 at 11:37 +0100, Michal Simek wrote:
>>>>> +/* 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.
>>
>> 1. Using address in license is just wrong and it should be removed
>> because they can move to different location.
>>
>> 2. u-boot adopted SPDX and all these fragments were removed.
>>
>> Wolfgang/Tom: IMHO this should be also changed/fixed.
>
>
> Just check the latest denali.h in Linux and this license header still
> there. But removing the license header put by original author might not
> a good thing to do. Unless we can get some approval from Wolfgang or
> Jason (who is submitter of this file to Linux).
ok then what have you done with license on the top of drivers/mtd/nand/denali.h?
You have changed it to SPDX license. Have you asked them to change it too?
That file already has license it it and the second license is just compatible
with the first one that's why IMHO you can just remove it.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140305/85aad933/attachment.pgp>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-03-04 0:03 ` Scott Wood
2014-03-04 10:31 ` Masahiro Yamada
@ 2014-03-05 17:34 ` Chin Liang See
2014-03-05 18:23 ` Scott Wood
1 sibling, 1 reply; 27+ messages in thread
From: Chin Liang See @ 2014-03-05 17:34 UTC (permalink / raw)
To: u-boot
On Mon, 2014-03-03 at 18:03 -0600, Scott Wood wrote:
> On Fri, 2014-02-21 at 14:51 -0600, 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@altera.com>
> > Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> > Cc: David Woodhouse <David.Woodhouse@intel.com>
> > Cc: Brian Norris <computersforpeace@gmail.com>
> > Cc: Scott Wood <scottwood@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
>
> It's "denali.c" in Linux -- why "denali_nand.c" here?
It seems all the existing U-Boot nand driver is using this naming
standard where <platform>_nand.
>
> > @@ -0,0 +1,1166 @@
> > +/*
> > + * Copyright (C) 2013 Altera Corporation <www.altera.com>
> > + * 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
>
> A module parameter? In U-Boot?
Removed
>
> Sharing code with Linux is fine, but try to edit out the stuff that's
> irrelevant in U-Boot.
>
> > +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);
> > +}
>
> Why are you using raw I/O accessors? The Linux driver doesn't do this.
Changed all to writel and readl
>
> > +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;
> > + status = read_interrupt_status();
> > + clear_interrupt(status);
> > + denali.irq_status = 0x0;
> > +}
> > +
> > +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 {
> > + 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);
>
> This looks like a much shorter timeout than Linux uses (1000us versus
> 1000ms). Though FWIW the Linux timeout code looks buggy.
>
Changed to 1s delay (although it didn't take that long)
> Also, comp_res is a very odd name for a timeout variable.
Changed to better name, timeout :)
>
> > +/* 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.
> > +*/
> > +static void index_addr(uint32_t address, uint32_t data)
> > +{
> > + __raw_writel(address, denali.flash_mem);
> > + __raw_writel(data, denali.flash_mem + 0x10);
> > +}
>
> What is 0x10? No magic numbers, please.
>
Changed to use macro.
> > +
> > +/* 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;
> > + 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++)
> > + __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;
>
> Don't align continuation lines with an if/for body -- it makes it harder
> to see where one ends and the other begins.
>
Fixed
> Why PASS/FAIL rather than normal "0 on success, negative error code on
> error"? Why uint16_t?
>
Fixed by returning 0 when pass. Also changed uint16_t to uint32_t
> > + 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); */
>
> Don't add commented-out-code.
Removed
>
> > +/* determines how many NAND chips are connected to the controller. Note for
> > + * Intel CE4100 devices we don't support more than one device.
> > + */
> > +static void find_valid_banks(void)
> > +{
> > + uint32_t id[denali.max_banks];
> > + int i;
> > +
> > + denali.total_used_banks = 1;
> > + for (i = 0; i < denali.max_banks; i++) {
> > + index_addr((uint32_t)(MODE_11 | (i << 24) | 0), 0x90);
> > + index_addr((uint32_t)(MODE_11 | (i << 24) | 1), 0);
> > + index_addr_read_data((uint32_t)(MODE_11 | (i << 24) | 2),
> > + &id[i]);
> > +
> > + if (i == 0) {
> > + if (!(id[i] & 0x0ff))
> > + break; /* WTF? */
>
> I'm not sure that this is the most helpful comment that could go here.
>
Oops..really sorry about this. Forget to remove this out when porting
> > +static void detect_partition_feature(void)
> > +{
> > + /* For MRST platform, denali.fwblks represent the
> > + * number of blocks firmware is taken,
> > + * FW is in protect partition and MTD driver has no
> > + * permission to access it. So let driver know how many
> > + * blocks it can't touch.
> > + * */
> > + if (__raw_readl(denali.flash_reg + FEATURES) & FEATURES__PARTITION) {
> > + if ((__raw_readl(denali.flash_reg + PERM_SRC_ID(1)) &
> > + PERM_SRC_ID__SRCID) == SPECTRA_PARTITION_ID) {
> > + denali.fwblks =
> > + ((__raw_readl(denali.flash_reg + MIN_MAX_BANK(1)) &
> > + MIN_MAX_BANK__MIN_VALUE) *
> > + denali.blksperchip)
> > + +
> > + (__raw_readl(denali.flash_reg + MIN_BLK_ADDR(1)) &
> > + MIN_BLK_ADDR__VALUE);
> > + } else
> > + denali.fwblks = SPECTRA_START_BLOCK;
> > + } else
> > + denali.fwblks = SPECTRA_START_BLOCK;
>
> If braces are needed on one side of if/else, use on both sides.
Fixed
>
> > +static void denali_set_intr_modes(uint16_t INT_ENABLE)
> > +{
> > + if (INT_ENABLE)
> > + __raw_writel(1, denali.flash_reg + GLOBAL_INT_ENABLE);
> > + else
> > + __raw_writel(0, denali.flash_reg + GLOBAL_INT_ENABLE);
> > +}
>
> CAPS are for macros, not function parameters.
>
> Why not just require the caller to pass in 0/1 rather than 0/non-0 and
> write that directly to the register?
Both fixed
>
> > +/* helper function that simply writes a buffer to the flash */
> > +static int write_data_to_flash_mem(const uint8_t *buf,
> > + int len)
>
> There's no point in a continuation line if you indent so far that you're
> beyond where the previous line ended. If the above doesn't violate 80
> columns, then you could just as well do it on one line.
>
> > +{
> > + uint32_t i = 0, *buf32;
> > +
> > + /* verify that the len is a multiple of 4. see comment in
> > + * read_data_from_flash_mem() */
> > + BUG_ON((len % 4) != 0);
> > +
> > + /* write the data to the flash memory */
> > + buf32 = (uint32_t *)buf;
> > + for (i = 0; i < len / 4; i++)
> > + __raw_writel(*buf32++, denali.flash_mem + 0x10);
>
> This violates C99 aliasing rules.
Fixed
>
> > + return i*4; /* intent is to return the number of bytes read */
> > +}
> > +
> > +static void denali_mode_main_access(void)
> > +{
> > + uint32_t addr, cmd;
> > + addr = BANK(denali.flash_bank) | denali.page;
> > + cmd = MODE_10 | addr;
> > + index_addr((uint32_t)cmd, MAIN_ACCESS);
>
> Unnecessary cast.
Removed
>
> > +/* raw include ECC value and all the spare area */
> > +static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> > + uint8_t *buf, int oob_required, int page)
> > +{
> > + uint32_t irq_status, irq_mask = INTR_STATUS__DMA_CMD_COMP;
> > +
> > + debug("denali_read_page_raw at page %08x\n", page);
> > + if (denali.page != page) {
> > + debug("Missing NAND_CMD_READ0 command\n");
> > + return -EIO;
> > + }
> > +
> > + if (oob_required)
> > + /* switch to main + spare access */
> > + denali_mode_main_spare_access();
> > + else
> > + /* switch to main access only */
> > + denali_mode_main_access();
> > +
> > + /* setting up the DMA where ecc_enable is false */
> > + irq_status = denali_dma_configuration(DENALI_READ, true, irq_mask,
> > + oob_required);
> > +
> > + /* if timeout happen, error out */
> > + if (!(irq_status & INTR_STATUS__DMA_CMD_COMP)) {
> > + debug("DMA timeout for denali_read_page_raw\n");
> > + return -EIO;
> > + }
> > +
> > + /* splitting the content to destination buffer holder */
> > + memcpy(chip->oob_poi, (const void *)(denali.buf.dma_buf +
> > + mtd->writesize), mtd->oobsize);
> > + memcpy(buf, (const void *)denali.buf.dma_buf, mtd->writesize);
>
> Unnecessary casts.
Removed
>
> > +static void denali_erase(struct mtd_info *mtd, int page)
> > +{
> > + uint32_t cmd = 0x0, irq_status = 0;
> > +
> > + debug("denali_erase at page %08x\n", page);
> > +
> > + /* clear interrupts */
> > + clear_interrupts();
> > +
> > + /* setup page read request for access type */
> > + cmd = MODE_10 | BANK(denali.flash_bank) | page;
> > + index_addr((uint32_t)cmd, 0x1);
> > +
> > + /* wait for erase to complete or failure to occur */
> > + irq_status = wait_for_irq(INTR_STATUS__ERASE_COMP |
> > + INTR_STATUS__ERASE_FAIL);
> > +
> > + if (irq_status & INTR_STATUS__ERASE_FAIL ||
> > + irq_status & INTR_STATUS__LOCKED_BLK)
> > + denali.status = NAND_STATUS_FAIL;
> > + else
> > + denali.status = PASS;
> > +}
>
> So PASS/FAIL are supposed to be in the same numberspace as
> NAND_STATUS_foo? Why use a separate FAIL elsewhere, then?
Using the standard where 0 as pass
>
> > +static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
> > + int page)
> > +{
> > + uint32_t addr;
> > +
> > + switch (cmd) {
> > + case NAND_CMD_PAGEPROG:
> > + break;
> > + case NAND_CMD_STATUS:
> > + addr = (uint32_t)MODE_11 | BANK(denali.flash_bank);
> > + index_addr((uint32_t)addr | 0, cmd);
>
> More unnecessary casts...
Removed
>
> > +/* 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)
> > +{
> > + debug("Should not be called as ECC handled by hardware\n");
> > + BUG();
> > + return -EIO;
> > +}
> > +
> > +static int denali_ecc_correct(struct mtd_info *mtd, uint8_t *data,
> > + uint8_t *read_ecc, uint8_t *calc_ecc)
> > +{
> > + debug("Should not be called as ECC handled by hardware\n");
> > + BUG();
> > + return -EIO;
> > +}
> > +
> > +static void denali_ecc_hwctl(struct mtd_info *mtd, int mode)
> > +{
> > + debug("Should not be called as ECC handled by hardware\n");
> > + BUG();
> > +}
>
> These debug() messages seem superfluous given the BUG() will identify
> the line in the code. debug() is not for commenting code.
Removed
>
> > +/* ffsdefs.h */
> > +#define CLEAR 0 /*use this to clear a field instead of "fail"*/
> > +#define SET 1 /*use this to set a field instead of "pass"*/
> > +#define FAIL 1 /*failed flag*/
> > +#define PASS 0 /*success flag*/
> > +#define ERR -1 /*error flag*/
>
> What's the difference between FAIL and ERR?
>
All these are no longer used. Removed
Thanks
Chin Liang
> -Scott
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-03-04 10:31 ` Masahiro Yamada
2014-03-04 18:44 ` Scott Wood
@ 2014-03-05 17:40 ` Chin Liang See
1 sibling, 0 replies; 27+ messages in thread
From: Chin Liang See @ 2014-03-05 17:40 UTC (permalink / raw)
To: u-boot
Hi Masahiro,
On Tue, 2014-03-04 at 19:31 +0900, Masahiro Yamada wrote:
> Hello Scott, Chin,
>
>
> > > +/* 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);
> > > +}
> >
> > Why are you using raw I/O accessors? The Linux driver doesn't do this.
>
> Add ioread32/iowrite32 to arch/arm/include/asm/io.h
> and use them?
>
Changed all to use standard writel and readl.
>
>
> > > +
> > > +static uint32_t wait_for_irq(uint32_t irq_mask)
> > > +{
> > > + unsigned long comp_res = 1000;
> > > + uint32_t intr_status = 0;
> > > +
> > > + 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);
> >
> > This looks like a much shorter timeout than Linux uses (1000us versus
> > 1000ms). Though FWIW the Linux timeout code looks buggy.
> >
> > Also, comp_res is a very odd name for a timeout variable.
>
> Right. I think wait time is too short.
> When I tested this code on my board, timeout error always
> occurred on page write command.
>
> I had to fix it to run write command.
> s/udelay(1)/udelay(1000)/
>
>
Hmmm that is interesting.
Probably I speed up the NAND controller to max.
Anyway, the delay now is 1s.
>
>
> There is another problem.
> I think there is a cache coherency problem in this driver code.
> DMA is used in this driver but D-cache is never flushed.
>
> When D-cache is on (CONFIG_SYS_DCACHE_OFF is not defined),
> ARM processor writes to/reads from the buffer through D-cache.
> On the other hand, Denali NAND controller always wites to/reads from
> the buffer on physical memory.
> So, this driver writes/reads wrong data.
>
> I had to add flush_dcache_range() function call
> in denali_setup_dma_sequence().
>
>
> @@ -689,6 +689,8 @@ static void denali_setup_dma_sequence(int op)
> uint32_t mode;
> uint32_t addr = (uint32_t)denali.buf.dma_buf;
>
> + flush_dcache_range(addr, addr + sizeof(denali.buf.dma_buf));
> +
> mode = MODE_10 | BANK(denali.flash_bank);
>
> /* DMA is a four step process */
>
>
I miss out this as I didn't enable the cache.
Thanks and I will put this into new patches
Chin Liang
>
>
> Best Regards
> Masahiro Yamada
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-03-05 17:34 ` Chin Liang See
@ 2014-03-05 18:23 ` Scott Wood
2014-03-05 23:01 ` Chin Liang See
0 siblings, 1 reply; 27+ messages in thread
From: Scott Wood @ 2014-03-05 18:23 UTC (permalink / raw)
To: u-boot
On Wed, 2014-03-05 at 11:34 -0600, Chin Liang See wrote:
> On Mon, 2014-03-03 at 18:03 -0600, Scott Wood wrote:
> > On Fri, 2014-02-21 at 14:51 -0600, 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@altera.com>
> > > Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> > > Cc: David Woodhouse <David.Woodhouse@intel.com>
> > > Cc: Brian Norris <computersforpeace@gmail.com>
> > > Cc: Scott Wood <scottwood@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
> >
> > It's "denali.c" in Linux -- why "denali_nand.c" here?
>
>
>
> It seems all the existing U-Boot nand driver is using this naming
> standard where <platform>_nand.
Not all -- there's omap_gpmc.c, omap_elm.c, nomadik.c, ndfc.c, etc.
A lot of them have the _nand.c suffix in Linux, too. Personally, I
think it's redundant.
> > > Why PASS/FAIL rather than normal "0 on success, negative error code on
> > error"? Why uint16_t?
> >
>
>
> Fixed by returning 0 when pass. Also changed uint16_t to uint32_t
Why uint32_t and not int? Is that return value somewhere used in a
context that expects a NAND hardware status?
-Scott
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-03-05 18:23 ` Scott Wood
@ 2014-03-05 23:01 ` Chin Liang See
2014-03-05 23:04 ` Scott Wood
0 siblings, 1 reply; 27+ messages in thread
From: Chin Liang See @ 2014-03-05 23:01 UTC (permalink / raw)
To: u-boot
On Wed, 2014-03-05 at 12:23 -0600, Scott Wood wrote:
> On Wed, 2014-03-05 at 11:34 -0600, Chin Liang See wrote:
> > On Mon, 2014-03-03 at 18:03 -0600, Scott Wood wrote:
> > > On Fri, 2014-02-21 at 14:51 -0600, 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@altera.com>
> > > > Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> > > > Cc: David Woodhouse <David.Woodhouse@intel.com>
> > > > Cc: Brian Norris <computersforpeace@gmail.com>
> > > > Cc: Scott Wood <scottwood@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
> > >
> > > It's "denali.c" in Linux -- why "denali_nand.c" here?
> >
> >
> >
> > It seems all the existing U-Boot nand driver is using this naming
> > standard where <platform>_nand.
>
> Not all -- there's omap_gpmc.c, omap_elm.c, nomadik.c, ndfc.c, etc.
>
> A lot of them have the _nand.c suffix in Linux, too. Personally, I
> think it's redundant.
Sure, I can change to denali.c
>
> > > > Why PASS/FAIL rather than normal "0 on success, negative error code on
> > > error"? Why uint16_t?
> > >
> >
> >
> > Fixed by returning 0 when pass. Also changed uint16_t to uint32_t
>
> Why uint32_t and not int? Is that return value somewhere used in a
> context that expects a NAND hardware status?
>
Nope, the return value is not used to compare against > 0 or < 0
Thanks
Chin Liang
> -Scott
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-03-05 23:01 ` Chin Liang See
@ 2014-03-05 23:04 ` Scott Wood
2014-03-05 23:09 ` Chin Liang See
0 siblings, 1 reply; 27+ messages in thread
From: Scott Wood @ 2014-03-05 23:04 UTC (permalink / raw)
To: u-boot
On Wed, 2014-03-05 at 17:01 -0600, Chin Liang See wrote:
> On Wed, 2014-03-05 at 12:23 -0600, Scott Wood wrote:
> > On Wed, 2014-03-05 at 11:34 -0600, Chin Liang See wrote:
> > > On Mon, 2014-03-03 at 18:03 -0600, Scott Wood wrote:
> > > > On Fri, 2014-02-21 at 14:51 -0600, Chin Liang See wrote:
> > > > > Why PASS/FAIL rather than normal "0 on success, negative error code on
> > > > error"? Why uint16_t?
> > > >
> > >
> > >
> > > Fixed by returning 0 when pass. Also changed uint16_t to uint32_t
> >
> > Why uint32_t and not int? Is that return value somewhere used in a
> > context that expects a NAND hardware status?
> >
>
>
> Nope, the return value is not used to compare against > 0 or < 0
Why not?
-Scott
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-03-05 23:04 ` Scott Wood
@ 2014-03-05 23:09 ` Chin Liang See
2014-03-05 23:11 ` Scott Wood
0 siblings, 1 reply; 27+ messages in thread
From: Chin Liang See @ 2014-03-05 23:09 UTC (permalink / raw)
To: u-boot
On Wed, 2014-03-05 at 17:04 -0600, Scott Wood wrote:
> On Wed, 2014-03-05 at 17:01 -0600, Chin Liang See wrote:
> > On Wed, 2014-03-05 at 12:23 -0600, Scott Wood wrote:
> > > On Wed, 2014-03-05 at 11:34 -0600, Chin Liang See wrote:
> > > > On Mon, 2014-03-03 at 18:03 -0600, Scott Wood wrote:
> > > > > On Fri, 2014-02-21 at 14:51 -0600, Chin Liang See wrote:
> > > > > > Why PASS/FAIL rather than normal "0 on success, negative error code on
> > > > > error"? Why uint16_t?
> > > > >
> > > >
> > > >
> > > > Fixed by returning 0 when pass. Also changed uint16_t to uint32_t
> > >
> > > Why uint32_t and not int? Is that return value somewhere used in a
> > > context that expects a NAND hardware status?
> > >
> >
> >
> > Nope, the return value is not used to compare against > 0 or < 0
>
> Why not?
>
We just check whether 0 or not as success will return 0.
Thanks
Chin Liang
> -Scott
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-03-05 23:09 ` Chin Liang See
@ 2014-03-05 23:11 ` Scott Wood
2014-03-05 23:21 ` Chin Liang See
0 siblings, 1 reply; 27+ messages in thread
From: Scott Wood @ 2014-03-05 23:11 UTC (permalink / raw)
To: u-boot
On Wed, 2014-03-05 at 17:09 -0600, Chin Liang See wrote:
> On Wed, 2014-03-05 at 17:04 -0600, Scott Wood wrote:
> > On Wed, 2014-03-05 at 17:01 -0600, Chin Liang See wrote:
> > > On Wed, 2014-03-05 at 12:23 -0600, Scott Wood wrote:
> > > > On Wed, 2014-03-05 at 11:34 -0600, Chin Liang See wrote:
> > > > > On Mon, 2014-03-03 at 18:03 -0600, Scott Wood wrote:
> > > > > > On Fri, 2014-02-21 at 14:51 -0600, Chin Liang See wrote:
> > > > > > > Why PASS/FAIL rather than normal "0 on success, negative error code on
> > > > > > error"? Why uint16_t?
> > > > > >
> > > > >
> > > > >
> > > > > Fixed by returning 0 when pass. Also changed uint16_t to uint32_t
> > > >
> > > > Why uint32_t and not int? Is that return value somewhere used in a
> > > > context that expects a NAND hardware status?
> > > >
> > >
> > >
> > > Nope, the return value is not used to compare against > 0 or < 0
> >
> > Why not?
> >
>
> We just check whether 0 or not as success will return 0.
The standard error idiom in Linux and U-Boot is negative values for
errors. That's why I asked if there was a reason for this, such as
passing the value to something that expects status values as would be
returned by hardware.
-Scott
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-03-05 23:11 ` Scott Wood
@ 2014-03-05 23:21 ` Chin Liang See
0 siblings, 0 replies; 27+ messages in thread
From: Chin Liang See @ 2014-03-05 23:21 UTC (permalink / raw)
To: u-boot
On Wed, 2014-03-05 at 17:11 -0600, Scott Wood wrote:
> On Wed, 2014-03-05 at 17:09 -0600, Chin Liang See wrote:
> > On Wed, 2014-03-05 at 17:04 -0600, Scott Wood wrote:
> > > On Wed, 2014-03-05 at 17:01 -0600, Chin Liang See wrote:
> > > > On Wed, 2014-03-05 at 12:23 -0600, Scott Wood wrote:
> > > > > On Wed, 2014-03-05 at 11:34 -0600, Chin Liang See wrote:
> > > > > > On Mon, 2014-03-03 at 18:03 -0600, Scott Wood wrote:
> > > > > > > On Fri, 2014-02-21 at 14:51 -0600, Chin Liang See wrote:
> > > > > > > > Why PASS/FAIL rather than normal "0 on success, negative error code on
> > > > > > > error"? Why uint16_t?
> > > > > > >
> > > > > >
> > > > > >
> > > > > > Fixed by returning 0 when pass. Also changed uint16_t to uint32_t
> > > > >
> > > > > Why uint32_t and not int? Is that return value somewhere used in a
> > > > > context that expects a NAND hardware status?
> > > > >
> > > >
> > > >
> > > > Nope, the return value is not used to compare against > 0 or < 0
> > >
> > > Why not?
> > >
> >
> > We just check whether 0 or not as success will return 0.
>
> The standard error idiom in Linux and U-Boot is negative values for
> errors. That's why I asked if there was a reason for this, such as
> passing the value to something that expects status values as would be
> returned by hardware.
>
Yup, there are function which return the register value. From there,
there will be a check for whether certain bit is set or not.
Thanks
Chin Liang
> -Scott
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-02-21 20:51 [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support Chin Liang See
` (2 preceding siblings ...)
2014-03-04 0:03 ` Scott Wood
@ 2014-05-30 10:50 ` Rik Smith
2014-05-30 11:36 ` Masahiro Yamada
3 siblings, 1 reply; 27+ messages in thread
From: Rik Smith @ 2014-05-30 10:50 UTC (permalink / raw)
To: u-boot
Chin Liang See <clsee <at> altera.com> writes:
>
> 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
>
Hi,
I was wondering which kernel this patch went into or is going into?
I am having problems with accessing a hynix NAND from an altera cyclone V
using kernel 3.11. It is coming up with a kernel driver error on boot of:
"Your NAND chip OOB is not large enough to contain 8bit ECC correction codes"
Looking for help on this issue.
Cheers
Rik
kernel bootlog extract:
at24 0-0051: 4096 byte 24c32 EEPROM, writable, 32 bytes/write
denali-nand-dt ff900000.flash: Dump timing register values:acc_clks: 0,
re_2_we: 50, re_2_re: 50
we_2_re: 5170, addr_2_data: 5170, rdwr_en_lo_cnt: 18
rdwr_en_hi_cnt: 12, cs_setup_cnt: 3
NAND device: Manufacturer ID: 0xad, Chip ID: 0xd5 (Hynix NAND 2GiB 3,3V
8-bit), 2048MiB, page size: 4096, OOB size: 64
Your NAND chip OOB is not large enough to
contain 8bit ECC correction codes
cadence-qspi ff705000.flash: DMA NOT enabled
cadence-qspi ff705000.flash: master is unqueued, this is deprecated
m25p80 spi2.0: found n25q00, expected n25q512a
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
2014-05-30 10:50 ` Rik Smith
@ 2014-05-30 11:36 ` Masahiro Yamada
0 siblings, 0 replies; 27+ messages in thread
From: Masahiro Yamada @ 2014-05-30 11:36 UTC (permalink / raw)
To: u-boot
Hi Rik,
On Fri, 30 May 2014 10:50:39 +0000 (UTC)
Rik Smith <rsmith@optos.com> wrote:
> Chin Liang See <clsee <at> altera.com> writes:
>
> >
> > 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
> >
>
> Hi,
> I was wondering which kernel this patch went into or is going into?
>
> I am having problems with accessing a hynix NAND from an altera cyclone V
> using kernel 3.11. It is coming up with a kernel driver error on boot of:
> "Your NAND chip OOB is not large enough to contain 8bit ECC correction codes"
This is not a driver for Linux Kernel,
but for U-Boot (boot loader).
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2014-05-30 11:36 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-21 20:51 [U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support Chin Liang See
2014-02-24 7:48 ` Michal Simek
2014-02-24 8:06 ` Masahiro Yamada
2014-02-24 8:16 ` Michal Simek
2014-02-27 17:04 ` Chin Liang See
2014-02-28 10:37 ` Michal Simek
2014-03-04 23:57 ` Chin Liang See
2014-03-05 6:22 ` Michal Simek
2014-02-27 14:35 ` Masahiro Yamada
2014-02-27 21:02 ` Chin Liang See
2014-02-27 22:32 ` Scott Wood
2014-02-27 23:03 ` Chin Liang See
2014-02-28 12:57 ` Masahiro Yamada
2014-03-05 0:24 ` Chin Liang See
2014-03-04 0:03 ` Scott Wood
2014-03-04 10:31 ` Masahiro Yamada
2014-03-04 18:44 ` Scott Wood
2014-03-05 17:40 ` Chin Liang See
2014-03-05 17:34 ` Chin Liang See
2014-03-05 18:23 ` Scott Wood
2014-03-05 23:01 ` Chin Liang See
2014-03-05 23:04 ` Scott Wood
2014-03-05 23:09 ` Chin Liang See
2014-03-05 23:11 ` Scott Wood
2014-03-05 23:21 ` Chin Liang See
2014-05-30 10:50 ` Rik Smith
2014-05-30 11:36 ` Masahiro Yamada
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.