Hi Scott,
Thanks for the review.
On Mon, Jun 6, 2011 at 4:58 PM, Scott Wood <scottw...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
