-----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