Hi Ian,
Thanks for the comments.
> Hi Rick,
>
> On Thursday 06 September 2007 14:30, Rick Bronson wrote:
> >
> > This is my first patch for yaffs2 so bear with me... sorry if I missed
> > something.
> >
> > [PATCH] [YAFFS] YAFFS2: Add comments for maintaining backward compatibility.
> >
> > [From: Rick Bronson <rick@efn.org>]
> > [Acked-by: Realname <realmail>]
> > Signed-off-by: Realname <realmail>
> >
> >
> > diff -ruN yaffs2.orig/yaffs_mtdif1.c yaffs2/yaffs_mtdif1.c
> > --- yaffs2.orig/yaffs_mtdif1.c 2007-07-23 12:14:04.000000000 -0700
> > +++ yaffs2/yaffs_mtdif1.c 2007-09-06 11:26:13.000000000 -0700
> > @@ -45,7 +45,8 @@
> > #endif
> >
> > #if 0
> > -/* Use the following nand_ecclayout with MTD when using
> > +/* Use the following nand ecclayout (replace the one in
> > + * nand_base.c with this one) with MTD when using
>
> Adding more comment/help describing how to use this nand_ecclayout is
> good. I would rather encourage folks to add this to their platform
> "NAND probe routine" rather than the stock mtd/nand/nand_base.c
> module.
>
This is true, thanks for pointing it out.
> > * CONFIG_YAFFS_9BYTE_TAGS and the older on-NAND tags layout.
> > * If you have existing Yaffs images and the byte order differs from this,
> > * adjust 'oobfree' to match your existing Yaffs data.
> > @@ -58,12 +59,24 @@
> > * We have/need PackedTags1 plus pageStatus: T0,T1,T2,T3,T4,T5,T6,T7,P
> > * where Tn are the tag bytes, En are MTD's ECC bytes, P is the pageStatus
> > * byte and B is the small-page bad-block indicator byte.
> > + *
> > + * If you need to maintain different OOB layouts on jffs2 vs yaffs on the
> > + * same NAND part then you will need to set chip->ecc.layout on entry to
> > + * the following routines: nand_read, nand_read_oob, nand_write,
> > + * nand_write_oob, nand_erase_nand, nand_block_markbad. You also need
> > + * to set parts->ecclayout for the separate partitions, probably in your
> > + * NAND probe routine.
>
> Using different layouts for different partitions of a device is
> not something that comes easy to MTD. The layout being part of
> the nand_chip object (single instance) rather that the mtd_partition
> object. So I'm not sure I follow what you mean when you say "need to set
> chip->ecc.layout on entry" -- do you mean make changes to nand_base.c
> to support multiple (runtime) layouts? If so, there's quite a bit
> more to this, isn't there?
>
We needed to maintain backward compatibilty with 2.4 which,
unfortunately, used jffs2 on one partition and old yaffs on 4 other
partitions. jffs2 and the old yaffs had different OOB layouts. As
far as my limited testing goes it seems to work fine with the changes
outlined above. If you know of a better way, I'd love to hear it.
> > */
> > static struct nand_ecclayout nand_oob_16 = {
> > .eccbytes = 6,
> > .eccpos = { 8, 9, 10, 13, 14, 15 },
> > +#ifdef CONFIG_YAFFS_9BYTE_TAGS
> > .oobavail = 9,
> > .oobfree = { { 0, 4 }, { 6, 2 }, { 11, 2 }, { 4, 1 } }
> > +#else
> > + .oobavail = 8,
> > + .oobfree = { { 0, 4 }, { 6, 2 }, { 11, 2 } }
> > +#endif
> > };
> > #endif
>
> This ifdef only works if Yaffs config is part of the kernel build
> because the file containing this data definition would normally be
> in the (static) kernel or platform module -- not in Yaffs code.
In my case it's part of the kernel build so it works fine. I would
still keep the #ifdef as above because it lets people know that there
are two distinct layouts here. It wasn't totally obvious to me when I
first encountered this comment.
> If the layout were passed to MTD at initialization in the probe
> routine, simply ifdef'ing the assignment to chip->ecc.layout in
> the probe would be as easy.
I don't think so because chip->ecc.layout is a "per chip" entity,
not a "per partition" entity. I have differnt chip->ecc.layout's for
the different partitions on the same NAND chip. They are being
changed "on the fly" so to speak.
> Also, what harm is there in offering
> an additional byte of oob/spare if it's available?
You might be right here,
Rick
(text/plain)