On Fri, 14 Oct 2005, Charles Manning wrote:

> On Friday 14 October 2005 11:14, Martin Fouts wrote:
> > Thank you for the patch.  I've added it to my patch queue, and will
> test
> > it as soon as I have time available.
> >
> > Marty
> >
> > > -----Original Message-----
> > > From: yaffs-bounces@stoneboat.aleph1.co.uk
> > > [mailto:yaffs-bounces@stoneboat.aleph1.co.uk] On Behalf Of
> > > Sergey Kubushyn
> > > Sent: Wednesday, October 12, 2005 10:12 AM
> > > To: yaffs-list
> > > Subject: [Yaffs] Make YAFFS2 work, patch1
> > >
> > > This is a quick hack probably good for foreseable future. I
> > > don't think a revolution for large page devices is on the
> > > horizon, we will be using that
> > > nand_oob_64 autoplacement scheme for quite a long time.
> > >
> > > The proper way is to consult mtd_info->nand_oobinfo->oobavail
> > > and handle YAFFS tags placement ourselves, in that same
> > > mtdif2.c. Autoplacement is not rocket science, there is
> > > nothing divine in it. As a matter of fact it's as simple as a
> > > shovel and proper handling of oobavail part of the entire oob
> > > area is trivial and easier to implement than to write a "Hello,
> world"
> > > program.
>
> My understanding from a IRC discussion where this was discussed with
> Thomas G,
> and I might be wrong in this, is that the whole idea behind AUTOPLACE
> is for
> the mtd decides on the actual physical location of the bytes.

Yes, if you do NOT use nand_read/write_oob.

> The reason for using this, rather than any fixed placement is that it
> allows
> the mtd user (YAFFS in this case) to ask the mtd to store/retrieve data
> on
> its behalf without concern as to the actual physical address.
>
> The patch shifts the data 2 bytes to move it past the bad block marker,
> since
> the default position for a bad block marker is position 0 in the OOB
> area.,
> and then does a raw read.
>
> This makes some assumptions that:
> * the default bad block marker is being used.
> * the rest of the oob area has sufficient contigous area to store the
> packed
> tags.
>
> These assumptions are not valid for many situations. eg some that use
> HWECC or
> some that use some different layout.

As I said it's just a quick hack that works for the only MTD autoplacement
available for large page NAND devices with software ECC at this time. I
don't think it's going to change really soon but it's better to do this in a
proper way.

The proper way is to get the nand_oobinfo structure (available as a member
of mtd_info structure), get the oobavail field of that structure and put/get
all the tags into those available areas byte-by-byte. This way it will work
for ANY oob layout, SOFT, HW, you-name-it.

No MTD fixes required, it'll work with existing MTD.

Implementing this is an hour time job. I can do it myself but I don't wanna
take a slice of bread from you so I'm leaving it to you. Please let me know
if you want me to do it and I'll send a patch.

> The whole point of AUTOPLACE is that the user can say: "Here is a
> buffer of
> data. Store it as you see fit. I don't want to know about it". At
> present we
> can only use AUTOPLACE semantics for a full page read/wr and not an OOB
> read/wr. We still need to know the inside trickery for any raw reads.

There is no trickery. MTD just puts all the oob data you gave it for a full
page write into oobavail areas, byte-by-byte in order they are in your
buffer when you're doing full page writes. Read is exactly the same.

> I expect the patch will fix the situation for many, perhaps even most,
> users.
> It is definitely an improvement and is worth applying, but it is not
> the end
> of the road. We still should be using AUTOPLACE to do this properly.

You don't have to. The entire autoplace is as simple as it's described
above. If you read/write your tags from nand_oobinfo->oobavail byte-by-byte
you're fine. That will match full page reads/writes exactly. And this
behavior didn't change for a very long time...

> > > BTW, it is NOT a very good idea to write mtd->oobsize (i.e.
> > > 64) bytes of data out of yaffs_PackedTags2 structure...
>
> That is a good observation. It should really use the spareBuffer like
> the read
> path does.

That's an alignment issue. Using a structure address as a pointer to a char
buffer is plain wrong. That might work for some processors but it'll give
you totally unexpected results for others. Structures are aligned and padded
so you'll be reading pads instead of real values. And there is no guarantee
that char size members will be put at the same place as int ones at all,
i.e. they might be put in totally different memory area.

---
******************************************************************
*  KSI@home    KOI8 Net  < >  The impossible we do immediately.  *
*  Las Vegas   NV, USA   < >  Miracles require 24-hour notice.   *
******************************************************************