[PATCH 8/8] mtd: nand: use ECC, if present, when scanning OOB

[Brian Norris]

On Tue, Jun 26, 2012 at 7:09 AM, Shmulik Ladkani
<shmulik.ladkani at gmail.com> wrote:
> On Fri, 22 Jun 2012 16:35:45 -0700 Brian Norris <computersforpeace at gmail.com> wrote:
>> scan_read_raw_oob() is used in only in places where the MTD_OPS_PLACE_OOB
>> mode is preferable MTD_OPS_RAW mode, so use MTD_OPS_PLACE_OOB instead.
>
> Please consider rephrasing, text is bit confusing.

I realized I was missing the word "to". So the edit would read:

scan_read_raw_oob() is used in only in places where the
MTD_OPS_PLACE_OOB mode is preferable to MTD_OPS_RAW mode, so use
MTD_OPS_PLACE_OOB instead.

I can rephrase more, regarding what each mode does, but I'm wondering
if this clears up your understanding.

>> + * Scan read data from data+OOB. May traverse multiple pages, interleaving
>> + * page,OOB,page,OOB,... in buf.
>
> In the case 'len' is less than 'mtd->writesize' (possible from a
> 'scan_block_full' call), the read oob will be placed immediately after
> the partially read page, quoting scan_read_raw_oob:
>
>                 ops.len = min(len, (size_t)mtd->writesize);
>                 ops.oobbuf = buf + ops.len;
>
> Any idea if this is intentional or a bug?

Not really. I honestly don't fully understand many of the options in
NAND_BBT, but I'm trying to work through some of it. I think there's a
lot of half-baked stuff in here with not-very-good code structure.
Some of the code is written as if it were generic, but it's only meant
for some very specific circumstances.

With that said, here's a thought: it looks like your hypothetical "len
< mtd->writesize" would only occur with NAND_BBT_SCANEMPTY disabled
and NAND_BBT_SCANALLPAGES enabled. But I only see them used together:
NAND_BBT_SCANEMPTY | NAND_BBT_SCANALLPAGES (omap2.c and with
agand_flashbased in nand_bbt.c). So maybe it's a kind of "bug" for a
case that was never really thought out. And if they must be used
together, maybe we should document that.

Brian