John Rigby wrote:
My only concern is that the u-boot and linux nand drivers need to have the same approach regarding ecc. The linux driver recently submitted only supports sw ecc because using hw ecc means the spare area is not writeable. The u-boot driver that I submitted supported hw_ecc only and was compatible with the linux driver in ltib. Unusable spare is an issue for JFFS2 but not UBIFS. If I were to make the decision I would just say not to JFFS2 on NAND. UBIFS is a far better solution for NAND anyway.
How much of a performance hit is the sw ecc? We should at least support it as an option, and probably by default if that's what Linux does.
I have seen at least one other controller where the controller included the spare in the ECC so I don't know if this is a trend or not.
I hope not. :-(
On Thu, Jun 4, 2009 at 7:18 AM, Stefan Roese sr@denx.de wrote:
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):
Thanks!
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?
Remove the "*priv" at the end and use mtd->priv instead.
+#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.
But there are two definitions of fsl_nfc_select_chip in that case... or am I missing something?
- 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.
Still, better to have code that might work than code that will definitely break. :-)
Alternatively, make it obvious that the driver does not support small-page.
+/*
- 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?
Change the offset. :-)
Perhaps with different offsets for small and large page.
-Scott