[U-Boot] [PATCH v6 4/7] jz4740 nand driver

Scott Wood
2011-03-07T23:44:36Z

On Mon, 28 Feb 2011 14:57:11 +0800
Xiangfu Liu <xiangfu@openmobilefree.net> wrote:

> +static int jz_nand_rs_correct_data(struct mtd_info *mtd, u_char *dat,
> + u_char *read_ecc, u_char *calc_ecc)
> +{
> + int k;
> + uint32_t errcnt, index, mask, status;
> + volatile u8 *paraddr = (volatile u8 *) &emc->nfpar[0];
> +
> + /* Set PAR values */
> + static uint8_t all_ff_ecc[] =
> + {0xcd, 0x9d, 0x90, 0x58, 0xf4, 0x8b, 0xff, 0xb7, 0x6f};

const?

> +
> + if (read_ecc[0] == 0xff && read_ecc[1] == 0xff &&
> + read_ecc[2] == 0xff && read_ecc[3] == 0xff &&
> + read_ecc[4] == 0xff && read_ecc[5] == 0xff &&
> + read_ecc[6] == 0xff && read_ecc[7] == 0xff &&
> + read_ecc[8] == 0xff) {
> + for (k = 0; k < 9; k++)
> + writeb(all_ff_ecc[k], (paraddr + k));
> + } else {
> + for (k = 0; k < 9; k++)
> + writeb(read_ecc[k], (paraddr + k));
> + }

Wouldn't it be simpler to do:

writeb(..., &emc->nfpar[k]);

And don't use volatile.

> + /* Set PRDY */
> + writel(readl(&emc->nfecr) | EMC_NFECR_PRDY, &emc->nfecr);
> +
> + /* Wait for completion */
> + do {
> + status = readl(&emc->nfints);
> + } while (!(status & EMC_NFINTS_DECF));
> +
> + /* disable ecc */
> + writel(readl(&emc->nfecr) & ~EMC_NFECR_ECCE, &emc->nfecr);
> +
> + /* Check decoding */
> + if (!(status & EMC_NFINTS_ERR))
> + return 0;
> +
> + if (status & EMC_NFINTS_UNCOR) {
> + printf("uncorrectable ecc\n");
> + return -1;
> + }
> +
> + errcnt = (status & EMC_NFINTS_ERRCNT_MASK) >> EMC_NFINTS_ERRCNT_BIT;
> +

> +#ifdef CONFIG_NAND_SPL
> + return 0;
> +#endif

Spaces should be a tab.

> + switch (errcnt) {
> + case 4:
> + index = (readl(&emc->nferr[3]) & EMC_NFERR_INDEX_MASK) >>
> + EMC_NFERR_INDEX_BIT;
> + mask = (readl(&emc->nferr[3]) & EMC_NFERR_MASK_MASK) >>
> + EMC_NFERR_MASK_BIT;
> + jz_rs_correct(dat, index, mask);
> + case 3:
> + index = (readl(&emc->nferr[2]) & EMC_NFERR_INDEX_MASK) >>
> + EMC_NFERR_INDEX_BIT;
> + mask = (readl(&emc->nferr[2]) & EMC_NFERR_MASK_MASK) >>
> + EMC_NFERR_MASK_BIT;
> + jz_rs_correct(dat, index, mask);
> + case 2:
> + index = (readl(&emc->nferr[1]) & EMC_NFERR_INDEX_MASK) >>
> + EMC_NFERR_INDEX_BIT;
> + mask = (readl(&emc->nferr[1]) & EMC_NFERR_MASK_MASK) >>
> + EMC_NFERR_MASK_BIT;
> + jz_rs_correct(dat, index, mask);
> + case 1:
> + index = (readl(&emc->nferr[0]) & EMC_NFERR_INDEX_MASK) >>
> + EMC_NFERR_INDEX_BIT;
> + mask = (readl(&emc->nferr[0]) & EMC_NFERR_MASK_MASK) >>
> + EMC_NFERR_MASK_BIT;
> + jz_rs_correct(dat, index, mask);
> + default:
> + break;
> + }
> +
> + return errcnt;

You don't do ECC correction in SPL? Can it not fit?

> +/*
> + * Main initialization routine
> + */
> +int board_nand_init(struct nand_chip *nand)
> +{
> +#ifdef CONFIG_NAND_SPL
> +extern void pll_init(void);
> +extern void sdram_init(void);
> +extern int serial_init(void);

Externs should go in header files.

> + __gpio_as_sdram_16bit_4720();
> + __gpio_as_uart0();
> +
> + pll_init();
> + serial_init();
> + sdram_init();

> +#if defined(CONFIG_QI_LB60)
> +#define KEY_U_OUT (32 * 2 + 16)
> +#define KEY_U_IN (32 * 3 + 19)
> + __gpio_as_input(KEY_U_IN);
> + __gpio_enable_pull(KEY_U_IN);
> + __gpio_as_output(KEY_U_OUT);
> + __gpio_clear_pin(KEY_U_OUT);
> +
> + if (__gpio_get_pin(KEY_U_IN) == 0)
> + usb_boot();
> +#endif
> +#endif

This stuff does not belong in the NAND driver; it belongs under your board
or cpu directory.

> + uint32_t reg;
> +
> + reg = readl(&emc->nfcsr);
> + reg |= EMC_NFCSR_NFE1; /* EMC setup, Set NFE bit */
> + writel(reg, &emc->nfcsr);
> +
> + writel(EMC_SMCR1_OPT_NAND, &emc->smcr[1]);
> +
> + nand->IO_ADDR_R = JZ_NAND_DATA_ADDR;
> + nand->IO_ADDR_W = JZ_NAND_DATA_ADDR;
> + nand->cmd_ctrl = jz_nand_cmd_ctrl;
> + nand->dev_ready = jz_nand_device_ready;
> +#ifdef CONFIG_NAND_SPL
> + nand->read_byte = nand_read_byte;
> + nand->read_buf = nand_read_buf;
> +#endif
> + nand->ecc.hwctl = jz_nand_hwctl;
> + nand->ecc.correct = jz_nand_rs_correct_data;
> + nand->ecc.calculate = jz_nand_rs_calculate_ecc;
> + nand->ecc.mode = NAND_ECC_HW_OOB_FIRST;
> + nand->ecc.size = CONFIG_SYS_NAND_ECCSIZE;
> + nand->ecc.bytes = CONFIG_SYS_NAND_ECCBYTES;
> + nand->ecc.layout = &qi_lb60_ecclayout_2gb;
> + nand->chip_delay = 50;

You don't set nand->options...

Don't you want a bad block table?

-Scott