[U-Boot] [PATCH v2 1/4] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/27/2013 12:09 PM, Scott Wood wrote:
> On 02/27/2013 10:49:55 AM, Tom Rini wrote:
>> On Tue, Feb 26, 2013 at 10:56:08AM -0500, Tom Rini wrote:
>> 
>>> We make these two functions take a size_t pointer to how much
>>> space was used on NAND to read or write the buffer (when
>>> reads/writes happen) so that bad blocks can be accounted for.
>>> We also make them take an loff_t limit on how much data can be
>>> read or written.  This means that we can now catch the case of
>>> when writing to a partition would exceed the partition size due
>>> to bad blocks.  To do this we also need to make
>> [snip]
>>> int nand_read_skip_bad(nand_info_t *nand, loff_t offset,
>>> size_t
>> *length,
>>> -               u_char *buffer) +               size_t *actual,
>>> loff_t lim, u_char *buffer)
>> [snip]
>>> +    if (*actual > lim) { +        puts("Size of read exceeds
>>> partition or device limit\n"); +        *length = 0; +
>>> return -EFBIG; +    }
>> 
>> The more I look at this and try testing things, I think I
>> shouldn't be introducing a change here.  Before you could do: 
>> nand read ${address} partname-with-badblock
>> 
>> And it would suceed but bleed into the next partition if it
>> wasn't the last one.  So your production system could do "nand
>> read ${address} kernel" and be OK.  But with this change, it
>> would fail because reading the whole partition is now too large
>> with a bad block (you would need partition+(blocksize*number bad
>> blocks).
> 
> You wouldn't be quite so OK if it were a write instead.

Correct.  But the check is now inside of nand_(read|write)_skip_bad.
So the write case will fail (without trying to write), but the read
case should not.

>> So I'm going to put this back to a check simply against requested
>> size being greater than lim rather than required size greater
>> than lim (since required size exceeds device is still caught).
> 
> No, please retain the check.  The other issue is a separate
> (though related) bug, and there's a patch from Harvey Chapman to
> deal with it.

I could be missing something, but I'm not sure how to otherwise adjust
things here.  With all of the checking moved to nand_util.c and
check_skip_len not knowing if we're a read or write it can only say
"fits using X" or "exceeds device", then it's up to the caller to
decide the next action.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRLj+TAAoJENk4IS6UOR1WJAAP/RLxsGHN4HqoEC0zfe2eIDZe
ZJU+sevT/murom6fNOfKFuC8pwPavy64FGdXNspwdpJCRTcXIs503k4x4ZMy9USC
HszV9NKF42HyscQi3RmBqlIUb9WQ6UikMZEUpteO0ZGQajRKho3nL3M1qjQsASG9
745qrmJijQEjEu3MsnfEZYEzyliVCpHHiIylhBALizwpjEwzKwlebe/ZrmbTPYO6
QXyfHLd+/5iNUJEjXOaXP/z7gXU+85m2uwxRhyLB1PmULGkmh7A+cvByTPt8TcxK
HTWnL3at4S+OvWqlnYTdTYbsCBUQ3jp/wnLF39vq9E5VIPv7UshwPponeNbFEGzt
GOZP1fIGMWcSFJHd3usR7wkhA5tJZu9329Av283ckwIbSlwz/tOP/efwFF/Zsrc5
oLn4ezRrA+RSKCtCujYJPhnAyEJEkncvpA4Imx6R9v9LZlXu3z8w5AQKcuAlsrxm
FDcjtcAA6y804I2jGPvL9laoCy9li1SVMHbd71zVtRwRJBMYUMfw7aI6uUwBslLJ
4GlnVKKsfEAkEb7APZ5v5YJrwD+mgSJ4TR1fwtoirbQKpG3OrMnXNhzZSBD5UFv6
DUxhMRL+EOawFJIl5AUPHHB7LdvV4qtDwee1bPg31ZXetnimQGLHn8qzppioMIvC
suvFme3/03ZGTDFwUUjp
=My8j
-----END PGP SIGNATURE-----