Re: [PATCH 2/5] mtd: nand: Calculate better default ecc layout

[Troy Kisky <troy.kisky@boundarydevices.com>]

David Brownell wrote:
> On Wednesday 13 May 2009, Troy Kisky wrote:
>> This saves lines of code by having nand_base
>> calculate oob_free entries.
>>
>> Boards that don't specify a layout
> 
> Not specifying a layout ... that's fair, won't affect
> compatibility with anything now deployed.  A nice new
> feature, impacting nothing in-tree.
> 
> Another case would be anything using ECC_HW_SYNDROME,
> where the layout is fully specified by ecc.prepad,
> ecc.postpad, ecc.bytes, and the badblock marker
> location ... and if any other layout is specified,
> it will be ignored.  You could print warnings, and
> make the ecclayout reported through the ioctls be
> the actual layout derived from prepad/postpad/etc;
> that counts as a pure bugfix.
> 
> 
>> and don't 
>> use all ecc bytes in the current default layout
>> could have their ecc position changed.
> 
> That seems dangerous.  It will break all existing
> boards.  You *really* don't want that on your head,
> since it won't just break them but will probably
> end up trashing all the data stored on their NAND
> chips ...
> 

That was an "and" above. Not specifying a specific
layout and using a default "and" not using all bytes
in the current default layouts ecc bytes.

> 
>> Looking through the code, I can only see that
>> Davinci is effected this way. It has its ecc
>> bytes moved to the end of the oob data, which
>> is why I want this patch. Before, it was wasting
>> a lot of oob bytes.
> 
> Erm, it took the default of 6 bytes ECC every 512
> bytes of data ... which is more than the hardware
> needs, just 3 bytes ECC for that much data.  That's
> hardly a "lot" (3 extra ecc bytes per 512 data),
> especially now that new software like UBI isn't
> even using the oobdata areas.
> 
> Or the new 4-bit ECC code, *currently* supporting
> only small page chips ... needs 10 bytes ECC for
> each 512 bytes data.  There is no wasted space for
> those cases.
> 
> You should be coordinating changes to davinci_nand
> with Sneha, by the way.

Whoever is in charge of the EVM will need to specify
a layout in the platform data if they don't like the change.

> 
> 
>> Some boards will print warning messages. i.e.
>> mxc_nand which specifies ecc.bytes = 3, but
>> eccbytes = 5 (not a multiple of 3).
> 
> I think it's fair to print a warning if an ECC layout
> is provided and it's got nonfatal issues like that.
> 
> Or even print errors (and fail!) if something really
> wrong turns up ... like mxc_nand getting a large-page
> chip, so it needs eccbytes = 12 but gets an explicit
> and undersized layout as you describe above.  (You
> can see there's a pagesize_2k field that's never set.
> There are clear bugs in large page support for the
> mxc_nand driver, even if you ignore 4k pages.)
> 
> 
>> I may have missed other boards being affected
>> so it needs through testing.
>>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> 
> 
>