On Wed, Oct 29, 2008 at 05:37:52PM -0600, John Rigby wrote:
Scott, thanks for your feedback. I can easily fix most of the issues.
The one question I have is if this can go in only supporting 5121 rev2. If I need to add rev1 support it will take more time than I have right now.
If rev1 is as dead as Wolfgang says, then I'm fine with leaving it out -- but that fact should be reflected in a comment, rather than the name of the driver. After all, this driver should support rev3 if there is one, right? :-)
+/*
- Copyright 2004-2008 Freescale Semiconductor, Inc. All Rights Reserved.
- Based on drivers/mtd/nand/mpc5121_nand.c
- which was forked from drivers/mtd/nand/mxc_nd.c
Forking's bad, mmkay?
Yes, I'm guilty. The original was pretty ugly.
If it's anything like the i.MX driver that was posted earlier, I agree entirely. :-)
I was just hoping that we could come up with one new, cleaned-up driver that supports both.
+static struct nand_ecclayout nand_hw_eccoob_4k = {
- .eccbytes = 64, /* actually 72 but only room for 64 */
Let's fix that, then.
This is defined in generic mtd code that has exposure to userland.
include/mtd/mtd-abi.h: /*
- ECC layout control structure. Exported to userspace for
- diagnosis and to allow creation of raw images
*/ struct nand_ecclayout { uint32_t eccbytes; uint32_t eccpos[64]; uint32_t oobavail; struct nand_oobfree oobfree[MTD_MAX_OOBFREE_ENTRIES]; };
Grr... I guess it's OK for now, as the main purpose of eccpos with hardware ECC is to exclude segments from use by other layers.
Very short-sighted API design, though.
read_byte should never be called with 16-bit NAND.
This is the replacement for nand_read_byte16 in nand_base.c.
Right, I should have verified my assumption before asserting it. :-P
Not sure why the generic nand_read_byte16 wasn't implemented in terms of the read_word method.
+/*!
- This function writes data of length \b len from buffer \b buf to the
NAND
- internal RAM buffer's MAIN area 0.
- @mtd MTD structure for the NAND Flash
- @buf data to be written to NAND Flash
- @len number of bytes to be written
- */
+static void mpc5121_nand_write_buf(struct mtd_info *mtd,
const u_char *buf, int len)+{
- printf("re-work may be needed?\n");
Answer this before we apply the patch. :-)
Perhaps I can just get rid of this whole routine?
I don't think so...
Why override these rather than let them go through the command function?
Not sure, I think it is so we can call the copy to/from spare routine.
You can do that in write_buf/read_buf/etc.
I'd rather scan_bbt not be abused for this -- we need to fix the NAND probe code so that drivers can do things between nand_scan() and nand_scan_tail().
And until then?
It's OK for now -- that was more of a note to myself to hurry up and fix the NAND probing mechanism. :-)
-Scott