Hi Mashiro,
On Wed, 2014-03-19 at 20:26 +0900, Masahiro Yamada wrote: > Hi Chin, > > > > --- /dev/null > > +++ b/drivers/mtd/nand/denali.c > > @@ -0,0 +1,1132 @@ > > +/* > > + * Copyright (C) 2013-2014 Altera Corporation <www.altera.com> > > + * Copyright (C) 2009-2010, Intel Corporation and its suppliers. > > + * > > + * SPDX-License-Identifier: GPL-2.0+ > > + */ > > If you don't mind, is it OK to add the creadit of Panasonic? > Going forward, Altera and Panasonic will share this driver > and I am contributing on code improvement and run test. > (I will post Panasonic boards support code after > this driver is merged.) > Sure, no problem. Its open source and code belongs to everyone :) > > > +/* setups the HW to perform the data DMA */ > > +static void denali_setup_dma(int op) > > +{ > > I sent a question to Cadence again and > finally received an answer I had wanted. > They said DMA command sequence depends on the bus width. > > Panasonic bought 64bit bus version. > I guess Altera bought 32bit bus version. > > So I'd like to suggest to use #ifdef CONFIG_NAND_DENALI_64BIT > here. > Yup, finally we know why. Added in next patch. > > > > > +/* > > + * Although controller spec said SLC ECC is forceb to be 4bit, but denali > > + * controller in MRST only support 15bit and 8bit ECC correction > > + */ > > +#ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST > > +#define ECC_15BITS 26 > > +static struct nand_ecclayout nand_15bit_oob = { > > + .eccbytes = ECC_15BITS, > > +}; > > +#else > > +#define ECC_8BITS 14 > > +static struct nand_ecclayout nand_8bit_oob = { > > + .eccbytes = ECC_8BITS, > > +}; > > I think supporting only 15bit, 8bit is odd. > The number of ECC bits depends on the hardware. > (You can choose ECC bits you like when you buy the IP > from Cadence.) > > I'd like to suggest to re-write this part in a generic way > by using the formula given in Denali's document. > > if (ecc.size == 512) > ecc.bytes = Ceiling_to_next_word(ecc.strength * 13) > > if (ecc.size == 1024) > ecc.bytes = Ceiling_to_next_word(ecc.strength * 14) > Sure, we can enhance this. The old code which mentioned MRST seems not applicable any more. At least, this is true for both of us. > > > > > + nand->ecc.mode = NAND_ECC_HW; > > + nand->ecc.size = CONFIG_NAND_DENALI_ECC_SIZE; > > + nand->ecc.read_oob = denali_read_oob; > > + nand->ecc.write_oob = denali_write_oob; > > + nand->ecc.read_page = denali_read_page; > > + nand->ecc.read_page_raw = denali_read_page_raw; > > + nand->ecc.write_page = denali_write_page; > > + nand->ecc.write_page_raw = denali_write_page_raw; > > +#ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST > > + /* 15bit ECC */ > > + nand->ecc.bytes = 26; > > + nand->ecc.layout = &nand_15bit_oob; > > +#else /* 8bit ECC */ > > + nand->ecc.bytes = 14; > > + nand->ecc.layout = &nand_8bit_oob; > > +#endif > > + nand->ecc.calculate = denali_ecc_calculate; > > + nand->ecc.correct = denali_ecc_correct; > > + nand->ecc.hwctl = denali_ecc_hwctl; > > denali_ecc_calculate(), > denali_ecc_correct(), > denali_ecc_hwctl() > are never called. Nor do we need to set stub functions. > > Besides, ecc.strength must be set. > > I guess ECC_CORRECTION register is already set correctly. > (In the case of SOCFPGA, it would be set to 8.) > > So, ecc.strength should be set to the value of ECC_CORRECTION. > Yup, these can be removed. It was added for SPL version but later I modified the SPL driver to use the HW ECC correction. > > > > > + > > +typedef int irqreturn_t; > > + > > +#define IRQ_HANDLED 1 > > +#define IRQ_NONE 0 > > These typedef and macros are not used. > denali.h in Linux Kernel does not have them either. > Please delete. > > > > +#define SUPPORT_15BITECC 1 > > +#define SUPPORT_8BITECC 1 > > These are no longer necessary. > > Noted. They are removed. > > > +#define DENALI_BUF_SIZE (NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE) > > NAND_MAX_PAGESIZE and NAND_MAX_OOBSIZE are defined in > include/linux/mtd/nand.h > > So, denali.h must include it. > > Added in next patch. > > > Code diff is as follows: > > > > diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c > index fce1b62..8ba8f04 100644 > --- a/drivers/mtd/nand/denali.c > +++ b/drivers/mtd/nand/denali.c > @@ -1,5 +1,6 @@ > /* > - * Copyright (C) 2013-2014 Altera Corporation <www.altera.com> > + * Copyright (C) 2014 Panasonic Corporation > + * Copyright (C) 2013-2014 Altera Corporation <www.altera.com> > * Copyright (C) 2009-2010, Intel Corporation and its suppliers. > * > * SPDX-License-Identifier: GPL-2.0+ > @@ -660,6 +661,21 @@ static void denali_setup_dma(int op) > > flush_dcache_range(addr, addr + sizeof(denali.buf.dma_buf)); > > +#ifdef CONFIG_NAND_DENALI_64BIT > + mode = MODE_10 | BANK(denali.flash_bank) | denali.page; > + > + /* DMA is a three step process */ > + > + /* 1. setup transfer type, interrupt when complete, > + burst len = 64 bytes, the number of pages */ > + index_addr(mode, 0x01002000 | (64 << 16) | op | page_count); > + > + /* 2. set memory low address bits 31:0 */ > + index_addr(mode, addr); > + > + /* 3. set memory high address bits 64:32 */ > + index_addr(mode, 0); > +#else > mode = MODE_10 | BANK(denali.flash_bank); > > /* DMA is a four step process */ > @@ -675,6 +691,7 @@ static void denali_setup_dma(int op) > > /* 4. interrupt when complete, burst len = 64 bytes*/ > index_addr(mode | 0x14000, 0x2400); > +#endif > } > > /* Common DMA function */ > @@ -1017,26 +1034,6 @@ static void denali_cmdfunc(struct mtd_info *mtd, > unsigned int cmd, int col, > break; > } > } > - > -/* stubs for ECC functions not used by the NAND core */ > -static int denali_ecc_calculate(struct mtd_info *mtd, const uint8_t *data, > - uint8_t *ecc_code) > -{ > - BUG(); > - return -EIO; > -} > - > -static int denali_ecc_correct(struct mtd_info *mtd, uint8_t *data, > - uint8_t *read_ecc, uint8_t *calc_ecc) > -{ > - BUG(); > - return -EIO; > -} > - > -static void denali_ecc_hwctl(struct mtd_info *mtd, int mode) > -{ > - BUG(); > -} > /* end NAND core entry points */ > > /* Initialization code to bring the device up to a known good state */ > @@ -1062,23 +1059,9 @@ static void denali_hw_init(void) > denali_irq_init(); > } > > -/* > - * Although controller spec said SLC ECC is forceb to be 4bit, but denali > - * controller in MRST only support 15bit and 8bit ECC correction > - */ > -#ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST > -#define ECC_15BITS 26 > -static struct nand_ecclayout nand_15bit_oob = { > - .eccbytes = ECC_15BITS, > -}; > -#else > -#define ECC_8BITS 14 > -static struct nand_ecclayout nand_8bit_oob = { > - .eccbytes = ECC_8BITS, > -}; > -#endif /* CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST */ > +static struct nand_ecclayout nand_oob; > > -static void denali_nand_init(struct nand_chip *nand) > +static int denali_nand_init(struct nand_chip *nand) > { > denali.flash_reg = (void __iomem *)CONFIG_SYS_NAND_REGS_BASE; > denali.flash_mem = (void __iomem *)CONFIG_SYS_NAND_DATA_BASE; > @@ -1104,17 +1087,24 @@ static void denali_nand_init(struct nand_chip *nand) > nand->ecc.read_page_raw = denali_read_page_raw; > nand->ecc.write_page = denali_write_page; > nand->ecc.write_page_raw = denali_write_page_raw; > -#ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST > - /* 15bit ECC */ > - nand->ecc.bytes = 26; > - nand->ecc.layout = &nand_15bit_oob; > -#else /* 8bit ECC */ > - nand->ecc.bytes = 14; > - nand->ecc.layout = &nand_8bit_oob; > -#endif > - nand->ecc.calculate = denali_ecc_calculate; > - nand->ecc.correct = denali_ecc_correct; > - nand->ecc.hwctl = denali_ecc_hwctl; > + /* > + * Tell driver the ecc strength. This register may be already set > + * correctly. So we read this value out. > + */ > + nand->ecc.strength = readl(denali.flash_reg + ECC_CORRECTION); > + switch (nand->ecc.size) { > + case 512: > + nand->ecc.bytes = (nand->ecc.strength * 13 + 15) / 16 * 2; > + break; > + case 1024: > + nand->ecc.bytes = (nand->ecc.strength * 14 + 15) / 16 * 2; > + break; > + default: > + pr_err("Unsupported ECC size\n"); > + return -EINVAL; > + } > + nand_oob.eccbytes = nand->ecc.bytes; > + nand->ecc.layout = &nand_oob; > > /* Set address of hardware control function */ > nand->cmdfunc = denali_cmdfunc; > @@ -1123,10 +1113,10 @@ static void denali_nand_init(struct nand_chip *nand) > nand->select_chip = denali_select_chip; > nand->waitfunc = denali_waitfunc; > denali_hw_init(); > + return 0; > } > > int board_nand_init(struct nand_chip *chip) > { > - denali_nand_init(chip); > - return 0; > + return denali_nand_init(chip); > } > diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h > index c668d8c..50a109d 100644 > --- a/drivers/mtd/nand/denali.h > +++ b/drivers/mtd/nand/denali.h > @@ -5,10 +5,7 @@ > * SPDX-License-Identifier: GPL-2.0+ > */ > > -typedef int irqreturn_t; > - > -#define IRQ_HANDLED 1 > -#define IRQ_NONE 0 > +#include <linux/mtd/nand.h> > > #define DEVICE_RESET 0x0 > #define DEVICE_RESET__BANK0 0x0001 > @@ -382,9 +379,6 @@ typedef int irqreturn_t; > > #define GLOB_HWCTL_DEFAULT_BLKS 2048 > > -#define SUPPORT_15BITECC 1 > -#define SUPPORT_8BITECC 1 > - > #define CUSTOM_CONF_PARAMS 0 > > #define ONFI_BLOOM_TIME 1 > > > Applied at v7 patch. Thanks Chin Liang > > > Best Regards > Masahiro Yamada > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot