[U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support
Chin Liang See
clsee at altera.com
Wed Mar 5 18:34:13 CET 2014
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 at altera.com>
> > Cc: Artem Bityutskiy <artem.bityutskiy at linux.intel.com>
> > Cc: David Woodhouse <David.Woodhouse at intel.com>
> > Cc: Brian Norris <computersforpeace at gmail.com>
> > Cc: Scott Wood <scottwood at freescale.com>
> > ---
> > Changes for v2
> > - Enable this driver support for SOCFPGA
> > ---
> > drivers/mtd/nand/Makefile | 1 +
> > drivers/mtd/nand/denali_nand.c | 1166 ++++++++++++++++++++++++++++++++++++++++
> > drivers/mtd/nand/denali_nand.h | 501 +++++++++++++++++
> > 3 files changed, 1668 insertions(+)
> > create mode 100644 drivers/mtd/nand/denali_nand.c
> > create mode 100644 drivers/mtd/nand/denali_nand.h
> >
> > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> > index 02b149c..24e8218 100644
> > --- a/drivers/mtd/nand/Makefile
> > +++ b/drivers/mtd/nand/Makefile
> > @@ -39,6 +39,7 @@ obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o
> > obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o
> > obj-$(CONFIG_DRIVER_NAND_BFIN) += bfin_nand.o
> > obj-$(CONFIG_NAND_DAVINCI) += davinci_nand.o
> > +obj-$(CONFIG_NAND_DENALI) += denali_nand.o
> > obj-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_nand.o
> > obj-$(CONFIG_NAND_FSL_IFC) += fsl_ifc_nand.o
> > obj-$(CONFIG_NAND_FSL_UPM) += fsl_upm.o
> > diff --git a/drivers/mtd/nand/denali_nand.c b/drivers/mtd/nand/denali_nand.c
> > new file mode 100644
> > index 0000000..55246c9
> > --- /dev/null
> > +++ b/drivers/mtd/nand/denali_nand.c
>
> 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
>
>
>
More information about the U-Boot
mailing list