Hi Scott,
Thanks for the review.
On Mon, Jun 6, 2011 at 4:58 PM, Scott Wood scottwood@freescale.com wrote:
On Tue, May 24, 2011 at 10:18:36AM -0400, Ben Gardiner wrote:
+#ifdef CONFIG_CMD_NAND_TRIMFFS +static size_t drop_ffs(const nand_info_t *nand, const u_char *buf,
- const size_t *len)
+{
- size_t i, l = *len;
- for (i = l - 1; i >= 0; i--)
- if (((const uint8_t *)buf)[i] != 0xFF)
- break;
This cast looks unnecessary.
You're absolutely right. It will be gone in v4.
- /* The resulting length must be aligned to the minimum flash I/O size */
- l = i + 1;
- l = (l + nand->writesize - 1) / nand->writesize;
- l *= nand->writesize;
- return l;
We allow unaligned lengths (the rest of the page gets padded with 0xff, see nand_do_page_write-ops). The input length might be unaligned -- this adjustment could cause you to read beyond the end of the supplied buffer.
Right. Sorry I missed that. In v4 I will drop also any trailling 0xff which do not make-up a full page since they would be padded out to trailing 0xff.
+} +#endif
/** * nand_write_skip_bad: * @@ -499,7 +520,7 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length, return -EINVAL; }
- if (!need_skip) {
- if (!need_skip && !(flags & WITH_DROP_FFS)) {
rval = nand_write (nand, offset, length, buffer); if (rval == 0) return 0;
Why not call drop_ffs before this point?
To achieve the desired effect, drop_ffs must be called on each eraseblock sized chunk being written; so it seemed the simplest way was to force a block-by-block pass with the !WITH_DROP_FFS to enter
while (left_to_write > 0) {
I'll leave this as-is in v4.
@@ -512,7 +533,7 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
while (left_to_write > 0) { size_t block_offset = offset & (nand->erasesize - 1);
- size_t write_size;
- size_t write_size, truncated_write_size;
WATCHDOG_RESET ();
@@ -558,7 +579,15 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length, else #endif {
- rval = nand_write (nand, offset, &write_size, p_buffer);
- truncated_write_size = write_size;
+#ifdef CONFIG_CMD_NAND_TRIMFFS
- if (flags & WITH_DROP_FFS)
- truncated_write_size = drop_ffs(nand, p_buffer,
- &write_size);
+#endif
What if both WITH_DROP_FFS and WITH_YAFFS_OOB are specified?
I didn't plan for that or intend for it to be supported.
Previous to my introduction of WITH_DROP_FFS; using the YAFFS oob mode was mutually exclusive with the 'usual' way of writing. The introduction of WITH_DROP_FFs respects this precedent.
If both flags were set 1) cmd_nand.c would need to be changed ( :) ) and 2) the WITH_YAFFS_OOB behaviour would override.
In v4 I will add a -EINVAL if WITH_YAFFS_OOB flag is used with any other flag.
Best Regards, Ben Gardiner
--- Nanometrics Inc. http://www.nanometrics.ca