On Thu, 2015-03-05 at 07:46 +0100, Albert ARIBAUD (3ADEV) wrote:
> diff --git a/drivers/mtd/nand/lpc32xx_nand_mlc.c 
> b/drivers/mtd/nand/lpc32xx_nand_mlc.c
> new file mode 100644
> index 0000000..cb23972
> --- /dev/null
> +++ b/drivers/mtd/nand/lpc32xx_nand_mlc.c
> @@ -0,0 +1,589 @@
> +/*
> + * LPC32xx MLC NAND flash controller driver
> + *
> + * (C) Copyright 2014 3ADEV <http://3adev.com>
> + * Written by Albert ARIBAUD <albert.arib...@3adev.fr>
> + *
> + * SPDX-License-Identifier:  GPL-2.0+
> + *
> + * NOTE:
> + *
> + * The MLC NAND flash controller provides hardware Reed-Solomon ECC
> + * covering in- and out-of-band data together. Therefore, in- and out-
> + * of-band data must be written together in order to have a valid ECC.
> + *
> + * Consequently, pages with meaningful in-band data are written with
> + * blank (all-ones) out-of-band data and a valid ECC, and any later
> + * out-of-band data write will void the ECC.
> + *
> + * Therefore, code which reads such late-written out-of-band data
> + * should not rely on the ECC validity.
> + */

When are you expecting later out-of-band writes?

> +static struct lpc32xx_nand_mlc_registers *lpc32xx_nand_mlc_registers
> +     = (struct lpc32xx_nand_mlc_registers *)MLC_NAND_BASE;

__iomem

> +                                unsigned int ctrl)
> +{
> +     if (cmd == NAND_CMD_NONE)
> +             return;
> +
> +     if (ctrl & NAND_CLE)
> +             writeb(cmd & 0Xff, &lpc32xx_nand_mlc_registers->cmd);
> +     else if (ctrl & NAND_ALE)
> +             writeb(cmd & 0Xff, &lpc32xx_nand_mlc_registers->addr);
> +     else
> +             return;
> +}

That last return is superfluous.

> +/**
> + * ECC layout -- this is needed whatever ECC mode we are using.
> + * In a 2KB (4*512B) page, R/S codes occupy 40 (4*10) bytes.
> + * To make U-Boot's life easier, we pack 'useable' OOB at the
> + * front and R/S ECC at the back.
> + */
> +
> +static struct nand_ecclayout lpc32xx_largepage_ecclayout = {
> +     .eccbytes = 40,
> +     .eccpos = {24, 25, 26, 27, 28, 29, 30, 31, 32, 33,
> +                34, 35, 36, 37, 38, 39, 40, 41, 42, 43,
> +                44, 45, 46, 47, 48, 48, 50, 51, 52, 53,
> +                54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
> +                },
> +     .oobfree = {
> +             {
> +                     .offset = 0,
> +                     .length = 24
> +             },
> +     }
> +};

Is offset zero really free?  Where does the bad block marker go?

> +     for (i = 0; i < 4; i++) {
> +             /* start data input */
> +             oob[6*i] = ((0xfe << i) & 0xff);
> +             oob[6*i+5] = ((0x7f >> i) & 0xff);
> +             chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x200+0x210*i, page);
> +             /* copy 6 non-ECC out-of-band bytes directly into NAND */
> +             memcpy(lpc32xx_nand_mlc_registers->data, oob+6*i, 6);

Lots of magic numbers...

> +             /* wait for NAND to return to ready state */
> +             while (!(readl(&lpc32xx_nand_mlc_registers->isr)
> +                     & ISR_NAND_READY))
> +                     ;

Timeout?

> +     }
> +     return 0;
> +}
> +
> +/**
> + * lpc32xx_waitfunc - wait until a command is done
> + * @mtd: MTD device structure
> + * @chip: NAND chip structure
> + *
> + * Wait for controller and FLASH to both be ready.
> + */
> +
> +static int lpc32xx_waitfunc(struct mtd_info *mtd, struct nand_chip *chip)
> +{
> +     /* FIXME: put a time-out here */
> +     int status;
> +     /* wait until both controller and NAND are ready */
> +     do {
> +             status = readl(&lpc32xx_nand_mlc_registers->isr);
> +     } while ((status & (ISR_CONTROLLER_READY || ISR_NAND_READY))
> +              != (ISR_CONTROLLER_READY || ISR_NAND_READY));

Timeout?  Etc.

> +/*
> + * Initialize the controller
> + */
> +
> +int board_nand_init(struct nand_chip *nand)
> +{

It'd be nice if new NAND drivers used CONFIG_SYS_NAND_SELF_INIT.

> +
> +     /* BBT options: read from last two pages, don't write */
> +     nand->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_LASTBLOCK
> +             | NAND_BBT_SCANLASTPAGE | NAND_BBT_SCAN2NDPAGE
> +             | NAND_BBT_WRITE;

Don't write?

> +#define LARGE_PAGE_SIZE 2048
> +
> +int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
> +{
> +     unsigned int page = offs / LARGE_PAGE_SIZE;
> +     unsigned int left = (size + LARGE_PAGE_SIZE - 1) / LARGE_PAGE_SIZE;

DIV_ROUND_UP()

> +     while (left) {
> +             if (read_single_page(dst, page) >= 0) {
> +                     dst += LARGE_PAGE_SIZE;
> +                     page++;
> +                     left--;
> +             }
> +     }

No bad block skipping?

-Scott


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to