Hi Scott,
I'll try to continue with this patch so that we can integrate it hopefully
soon. I already addressed some of your comments (the easy ones ;)). Please
find some further questions below (I'm still new to the FSL NFC):
On Thursday 06 November 2008 00:06:48 Scott Wood wrote:
> > +static struct fsl_nfc_private {
> > + struct mtd_info mtd;
> > + char spare_only;
> > + char status_req;
> > + u16 col_addr;
> > + int writesize;
> > + int sparesize;
> > + int width;
> > + int chipsel;
> > +} *priv;
>
> Is it plausable that there will ever be a chip with more than one of
> these controllers?
I have no idea. What do you suggest I should change here?
> > +/*
> > + * OOB placement block for use with hardware ecc generation
> > + */
> > +static struct nand_ecclayout nand_hw_eccoob_512 = {
> > + .eccbytes = 9,
> > + .eccpos = {
> > + 7, 8, 9, 10, 11, 12, 13, 14, 15,
> > + },
> > + .oobfree = {
> > + {0, 5} /* byte 5 is factory bad block marker */
> > + },
> > +};
>
> Is byte 6 free?
Looks this way. I'll add it to the free area. John, please shout if you think
this is not correct.
> > +static struct nand_ecclayout nand_hw_eccoob_4k_218_spare = {
> > + .eccbytes = 64, /* actually 144 but only room for 64 */
> > + .eccpos = {
> > + /* 18 bytes of ecc for each 512 bytes of data */
> > + 7, 8, 9, 10, 11, 12, 13, 14, 15,
> > + 16, 17, 18, 19, 20, 21, 22, 23, 24,
> > + 33, 34, 35, 36, 37, 38, 39, 40, 41,
> > + 42, 43, 44, 45, 46, 47, 48, 49, 50,
> > + 59, 60, 61, 62, 63, 64, 65, 66, 67,
> > + 68, 69, 70, 71, 72, 73, 74, 75, 76,
> > + 85, 86, 87, 88, 89, 90, 91, 92, 93,
> > + 94, /* 95, 96, 97, 98, 99, 100, 101, 102,
> > + 111, 112, 113, 114, 115, 116, 117, 118, 119,
> > + 120, 121, 122, 123, 124, 125, 126, 127, 128,
> > + 137, 138, 139, 140, 141, 142, 143, 144, 145,
> > + 146, 147, 148, 149, 150, 151, 152, 153, 154,
> > + 163, 164, 165, 166, 167, 168, 169, 170, 171,
> > + 172, 173, 174, 175, 176, 177, 178, 179, 180,
> > + 189, 190, 191, 192, 193, 194, 195, 196, 197,
> > + 198, 199, 200, 201, 202, 203, 204, 205, 206, */
> > + },
> > + .oobfree = {
> > + {2, 5}, /* bytes 0 and 1 are factory bad block markers */
> > + {25, 8},
> > + {51, 8},
> > + {77, 8},
> > + {103, 8},
> > + {129, 8},
> > + {155, 8},
> > + {181, 8},
> > + },
> > +};
>
> Need to change NAND_MAX_OOBSIZE.
I'll send a patch for this shortly.
> > +#ifndef CONFIG_FSL_NFC_BOARD_CS_FUNC
> > +static void fsl_nfc_select_chip(u8 cs)
> > +{
> > + u32 val = NFC_READW(NFC_BUF_ADDR);
> > +
> > + val &= ~0x60;
> > + val |= cs << 5;
> > + NFC_WRITEW(NFC_BUF_ADDR, val);
> > +}
> > +#define CONFIG_FSL_NFC_BOARD_CS_FUNC fsl_nfc_select_chip
> > +#endif
> > +
> > +
> > +/*!
> > + * This function is used by upper layer for select and deselect of the
> > NAND + * chip
> > + *
> > + * @mtd MTD structure for the NAND Flash
> > + * @chip val indicating select or deselect
> > + */
> > +static void fsl_nfc_select_chip(struct mtd_info *mtd, int chip)
>
> This looks like a compilation error if CONFIG_FSL_NFC_BOARD_CS_FUNC is
> not defined.
Works fine here on a board without CONFIG_FSL_NFC_BOARD_CS_FUNC defined. So
I'll leave it as is.
> > + case NAND_CMD_READOOB:
> > + priv->col_addr = column;
> > + priv->spare_only = 1;
> > + command = NAND_CMD_READ0; /* only READ0 is valid */
> > + break;
>
> What about small-page flash that takes an actual READOOB command?
I don't have access to a board with small-page NAND. So I can't test anything
here.
> > +static int fsl_nfc_read_oob(struct mtd_info *mtd, struct nand_chip
> > *chip, + int page, int sndcmd)
> > +{
> > + if (sndcmd) {
> > + read_full_page(mtd, page);
> > + sndcmd = 0;
> > + }
> > +
> > + copy_from_spare(mtd, chip->oob_poi, mtd->oobsize);
> > + return sndcmd;
> > +}
> > +
> > +static int fsl_nfc_write_oob(struct mtd_info *mtd, struct nand_chip
> > *chip, + int page)
> > +{
> > + int status = 0;
> > + int read_oob_col = 0;
> > +
> > + send_cmd(NAND_CMD_READ0);
> > + send_cmd(NAND_CMD_SEQIN);
> > + fsl_nfc_do_addr_cycle(mtd, read_oob_col, page);
> > +
> > + /* copy the oob data */
> > + copy_to_spare(mtd, chip->oob_poi, mtd->oobsize);
> > +
> > + send_prog_page(0);
> > +
> > + send_cmd(NAND_CMD_PAGEPROG);
> > +
> > + status = fsl_nfc_wait(mtd, chip);
> > + if (status & NAND_STATUS_FAIL)
> > + return -1;
> > + return 0;
> > +}
>
> Again, I'm fairly sure you don't need these if the other functions are
> defined properly.
OK, I removed these functions and tested a bit on my MPC5121 board. Everything
seems to be working fine without it.
Again, John please let me know if you think these functions are really
necessary.
> > +/*
> > + * These are identical to the generic versions except
> > + * for the offsets.
> > + */
> > +static struct nand_bbt_descr bbt_main_descr = {
> > + .options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
> > + | NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP,
> > + .offs = 0,
> > + .len = 4,
> > + .veroffs = 4,
> > + .maxblocks = 4,
> > + .pattern = bbt_pattern
> > +};
> > +
> > +static struct nand_bbt_descr bbt_mirror_descr = {
> > + .options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
> > + | NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP,
> > + .offs = 0,
> > + .len = 4,
> > + .veroffs = 4,
> > + .maxblocks = 4,
> > + .pattern = mirror_pattern
> > +};
>
> This will overlap the bad block marker on large-page flash.
Good catch. Do you have any idea how can this be solved?
Thanks.
Best regards,
Stefan
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de
=====================================================================
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
