[U-Boot] [PATCH v6] nand/denali: Adding Denali NAND driver support
Chin Liang See
clsee at altera.com
Mon Mar 24 16:16:58 CET 2014
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
>
>
More information about the U-Boot
mailing list