On 02/27/2013 08:20:19 AM, Tom Rini wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 02/26/2013 09:08 PM, Scott Wood wrote:
The name "maxsize" suggests that it's a size, not a position.
OK, I'll call it maxoff (because it's the max offset within the NAND for a given partition, or end of the NAND).
Wouldn't it be less intrusive to just make it actually be the size instead of an offset?
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 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.
-Scott