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

[Tom Rini <trini@ti.com>]

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

On 02/27/2013 05:04 PM, Scott Wood wrote:
> On 02/27/2013 08:20:19 AM, Tom Rini wrote:
[snip]
>>>> -        offset += block_len; +        *offset += block_len;
>>>> }
>>>> 
>>>> return ret; @@ -459,22 +463,26 @@ static size_t
>>>> drop_ffs(const nand_info_t *nand, const u_char *buf, * Write
>>>> image to NAND flash. * Blocks that are marked bad are skipped
>>>> and the is written to the next * block instead as long as the
>>>> image is short enough to fit even after - * skipping the bad
>>>> blocks. + * skipping the bad blocks.  Note that the actual
>>>> size needed may exceed + * both the length and available NAND
>>>> due to bad blocks.
>>> 
>>> If that happens, then the function returns failure.  Are the 
>>> contents of "actual" well-defined when the function returns 
>>> failure?
>> 
>> They are as well defined as what happens with length.  If we say
>> we can't write, we set both to 0 and return an error.  I'll take
>> this as a request to expand the comment and do so.
> 
> The comments could use expanding (it doesn't even explain what
> happens to length in the non-error case), but also it looks like
> there are some error paths where actual doesn't get cleared, in
> the CONFIG_CMD_NAND_YAFFS section.

I've dropped actual for everything except for DFU where it's really
needed.  And I've expanded the big comment block (not the @param)
lines with what happens to length and actual.

> I was also wondering about the case where check_skip_bad() says it 
> doesn't fit.  It doesn't return the actual in that case -- it
> returns offset as of when it stopped.  So a caller of
> nand_read|write_skip_bad() would see the same "actual" as if it
> just barely fit.  I'm not sure that this function ever would return
> an actual size that exceeds the available NAND.

No?  check_skip_bad only bails out totally on end of NAND (and doesn't
care about limit).  I've re-worked things so check_skip_bad treats
actual as a size not an offset, but yes, in the case of actual exceeds
NAND, actual is only as far as we calculated.  If we're going to start
fiddling around in here with these cases for non-human users (IOW, the
"You've exceeded the device size" print wouldn't be seen) then we need
to start using more than just -EINVAL so we can differentiate "No
space" and "Too Big".

At least with v3 (which I was about to send and saw your email) should
address everything.  I jsut want to re-read your comments above with
the code and a fresh mind in the morning before re-posting.

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

iQIcBAEBAgAGBQJRLpiJAAoJENk4IS6UOR1Wj4MP/08nqOakLldtjLUWBhFhTQA5
8kjaDZj0pP/RFkVUgprDuOM+JTWSY08KVVQ25pooXUN7C5OxivLgZPPcsExKeXWd
TiO8ADzVqns24Cg/RmS16ZZzpNiwjhzR/oWoaI+Zi6t1S4pSx7UBMQVQhH81sXOn
VQzWzGithY74bsFzUwQQkIIeWL+WqgCZuySop09JIB1mXcleiV4FB9/5aXmWEzOg
YhtE10cR6RCHVPBJR50qMbelvtS+h2DzFSrvz66YlicBWXdoD8gkLJ2WdM0zojuP
cetdpFSsbgadpjp8MO4SQLpim7ytdTm3IWpqGUFFTFLoaEiAxcqts82I+NcPIxF7
c9q3iS2P5h+NT9ZUqW+j+BcEz8fvJv1lWwB6t3CTsLE4EynK9iZaCNfAa8eZ3457
a3G4eVmCCFH5ZaLgmnDQR1ol6Pc2iuoZH6XX6I5JWMdSRRs5arHMaF8LCydSuPcE
gII59PK3O+RbpJYGE8nvFwByamGPtaNhTzBqBSkdGp3+lKhkEWM31dvUR0s6GGRS
5rTdEIXkHqLpu0+4YMDVFp2PAYuITnkuZCynOWyjJSojufEYTcwJiX4GRFfL+o+x
SCMsWdzqAnzrVQAP9Re+MDU+6qiPw4GRe36SOu5uq/QNwyAyFM1xC6dXogcfHYSH
rgmSTadLscWZBaR3guc4
=hPgQ
-----END PGP SIGNATURE-----