[U-Boot] [PATCH v3 1/4] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters
Scott Wood
scottwood at freescale.com
Fri Mar 1 02:37:51 CET 2013
On 02/28/2013 01:09:05 PM, Tom Rini wrote:
> We make these two functions take a size_t pointer to how much space
> was used on NAND to read or write the buffer (when reads/writes
> happen)
> so that bad blocks can be accounted for. We also make them take an
> loff_t limit on how much data can be read or written. This means that
> we can now catch the case of when writing to a partition would exceed
> the partition size due to bad blocks. To do this we also need to make
> check_skip_len count not just complete blocks used but partial ones as
> well. All callers of nand_(read|write)_skip_bad are adjusted to call
> these with the most sensible limits available.
>
> The changes were started by Pantelis and finished by Tom.
>
> Signed-off-by: Pantelis Antoniou <panto at antoniou-consulting.com>
> Signed-off-by: Tom Rini <trini at ti.com>
> ---
> Changes in v3:
> - Reworked skip_check_len changes to just add accounting for *used to
> the logic.
> - Allow for actual to be NULL in nand_(read|write)_skip_bad, only DFU
> calls this with a non-NULL parameter. Make sure the comments for
> both
> functions explain the parameters and their behavior.
> - Other style changes requested by Scott.
> - As nand_(write|read)_skip_bad passes back just a used length now.
>
> Changes in v2:
> - NAND skip_check_len changes reworked to allow
> nand_(read|write)_skip_bad to return this information to the caller.
>
> common/cmd_nand.c | 56
> +++++++++++++++++++----------------
> common/env_nand.c | 3 +-
> drivers/mtd/nand/nand_util.c | 67
> +++++++++++++++++++++++++++++++++++++-----
> include/nand.h | 4 +--
> 4 files changed, 93 insertions(+), 37 deletions(-)
Looks mostly good, just some minor issues:
> - if (*size > maxsize) {
> - puts("Size exceeds partition or device limit\n");
> - return -1;
> - }
> -
I assume you're removing this because you rely on the read/write
functions printing the error... what about other users of this such as
erase, lock, etc?
> @@ -476,20 +483,30 @@ 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. Due to bad blocks we may not be able to
> + * perform the requested write. In the case where the write would
> + * extend beyond the end of the NAND device, both length and actual
> (if
> + * not NULL) are set to 0. In the case where the write would extend
> + * beyond the limit we are passed, length is set to 0 and actual is
> set
> + * to the required length.
> *
> * @param nand NAND device
> * @param offset offset in flash
> * @param length buffer length
> + * @param actual set to size required to write length worth of
> + * buffer or 0, if not NULL
s/or 0/or 0 on error/
or
s/or 0/in case of success/
The read function doesn't have the "or 0" comment...
> + * @param lim maximum size that length may be in
> order to not
> + * exceed the buffer
s/that length may be/that actual may be/
> * @param buffer buffer to read from
> * @param flags flags modifying the behaviour of the
> write to NAND
> * @return 0 in case of success
> */
> int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t
> *length,
> - u_char *buffer, int flags)
> + size_t *actual, loff_t lim, u_char *buffer, int flags)
> {
> int rval = 0, blocksize;
> size_t left_to_write = *length;
> + size_t used_for_write = 0;
> u_char *p_buffer = buffer;
> int need_skip;
>
> @@ -526,16 +543,28 @@ int nand_write_skip_bad(nand_info_t *nand,
> loff_t offset, size_t *length,
> if ((offset & (nand->writesize - 1)) != 0) {
> printf("Attempt to write non page-aligned data\n");
> *length = 0;
> + if (actual)
> + *actual = 0;
> return -EINVAL;
> }
Again, what about the returns in the WITH_YAFFS_OOB section? Or if we
document that "actual" is undefined for error returns we can not worry
about this.
-Scott
More information about the U-Boot
mailing list