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 <s...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot