[U-Boot] [PATCH v5 4/9] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters

Tom Rini
2013-03-08T17:37:23Z

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@antoniou-consulting.com>
Signed-off-by: Tom Rini <trini@ti.com>
---
Changes in v5: None
Changes in v4:
- Further reword nand_util.c comments, from Scott
- In nand_write_skip_bad make sure *actual is 0 for YAFFS2 errors too,
reminded by Scott.
- In cmd_nand.c don't drop the size is less than maxsize check in
arg_off_size as other nand functions need this still (Scott).

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 | 53 ++++++++++++++++++--------------
common/env_nand.c | 3 +-
drivers/mtd/nand/nand_util.c | 68 +++++++++++++++++++++++++++++++++++++-----
include/nand.h | 4 +--
4 files changed, 95 insertions(+), 33 deletions(-)

diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index 32348f3..76f4d3f 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -137,7 +137,8 @@ static inline int str2long(const char *p, ulong *num)
return *p != '\0' && *endptr == '\0';
}

-static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size)
+static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size,
+ loff_t *maxsize)
{
#ifdef CONFIG_CMD_MTDPARTS
struct mtd_device *dev;
@@ -160,6 +161,7 @@ static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size)

*off = part->offset;
*size = part->size;
+ *maxsize = part->size;
*idx = dev->id->num;

ret = set_dev(*idx);
@@ -173,10 +175,11 @@ static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size)
#endif
}

-static int arg_off(const char *arg, int *idx, loff_t *off, loff_t *maxsize)
+static int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,
+ loff_t *maxsize)
{
if (!str2off(arg, off))
- return get_part(arg, idx, off, maxsize);
+ return get_part(arg, idx, off, size, maxsize);

if (*off >= nand_info[*idx].size) {
puts("Offset exceeds device limit\n");
@@ -184,36 +187,35 @@ static int arg_off(const char *arg, int *idx, loff_t *off, loff_t *maxsize)
}

*maxsize = nand_info[*idx].size - *off;
+ *size = *maxsize;
return 0;
}

static int arg_off_size(int argc, char *const argv[], int *idx,
- loff_t *off, loff_t *size)
+ loff_t *off, loff_t *size, loff_t *maxsize)
{
int ret;
- loff_t maxsize = 0;

if (argc == 0) {
*off = 0;
*size = nand_info[*idx].size;
+ *maxsize = *size;
goto print;
}

- ret = arg_off(argv[0], idx, off, &maxsize);
+ ret = arg_off(argv[0], idx, off, size, maxsize);
if (ret)
return ret;

- if (argc == 1) {
- *size = maxsize;
+ if (argc == 1)
goto print;
- }

if (!str2off(argv[1], size)) {
printf("'%s' is not a number\n", argv[1]);
return -1;
}

- if (*size > maxsize) {
+ if (*size > *maxsize) {
puts("Size exceeds partition or device limit\n");
return -1;
}
@@ -307,7 +309,8 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
if (argc < 3)
goto usage;

- if (arg_off(argv[2], &idx, &addr, &maxsize)) {
+ /* We don't care about size, or maxsize. */
+ if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize)) {
puts("Offset or partition name expected\n");
return 1;
}
@@ -426,7 +429,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
{
int i, ret = 0;
ulong addr;
- loff_t off, size;
+ loff_t off, size, maxsize;
char *cmd, *s;
nand_info_t *nand;
#ifdef CONFIG_SYS_NAND_QUIET
@@ -551,7 +554,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])

printf("\nNAND %s: ", cmd);
/* skip first two or three arguments, look for offset and size */
- if (arg_off_size(argc - o, argv + o, &dev, &off, &size) != 0)
+ if (arg_off_size(argc - o, argv + o, &dev, &off, &size,
+ &maxsize) != 0)
return 1;

nand = &nand_info[dev];
@@ -619,7 +623,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
if (s && !strcmp(s, ".raw")) {
raw = 1;

- if (arg_off(argv[3], &dev, &off, &size))
+ if (arg_off(argv[3], &dev, &off, &size, &maxsize))
return 1;

if (argc > 4 && !str2long(argv[4], &pagecount)) {
@@ -635,7 +639,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
rwsize = pagecount * (nand->writesize + nand->oobsize);
} else {
if (arg_off_size(argc - 3, argv + 3, &dev,
- &off, &size) != 0)
+ &off, &size, &maxsize) != 0)
return 1;

rwsize = size;
@@ -645,9 +649,11 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
!strcmp(s, ".e") || !strcmp(s, ".i")) {
if (read)
ret = nand_read_skip_bad(nand, off, &rwsize,
+ NULL, maxsize,
(u_char *)addr);
else
ret = nand_write_skip_bad(nand, off, &rwsize,
+ NULL, maxsize,
(u_char *)addr, 0);
#ifdef CONFIG_CMD_NAND_TRIMFFS
} else if (!strcmp(s, ".trimffs")) {
@@ -655,8 +661,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
printf("Unknown nand command suffix '%s'\n", s);
return 1;
}
- ret = nand_write_skip_bad(nand, off, &rwsize,
- (u_char *)addr,
+ ret = nand_write_skip_bad(nand, off, &rwsize, NULL,
+ maxsize, (u_char *)addr,
WITH_DROP_FFS);
#endif
#ifdef CONFIG_CMD_NAND_YAFFS
@@ -665,8 +671,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
printf("Unknown nand command suffix '%s'.\n", s);
return 1;
}
- ret = nand_write_skip_bad(nand, off, &rwsize,
- (u_char *)addr,
+ ret = nand_write_skip_bad(nand, off, &rwsize, NULL,
+ maxsize, (u_char *)addr,
WITH_INLINE_OOB);
#endif
} else if (!strcmp(s, ".oob")) {
@@ -775,7 +781,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
if (s && !strcmp(s, ".allexcept"))
allexcept = 1;

- if (arg_off_size(argc - 2, argv + 2, &dev, &off, &size) < 0)
+ if (arg_off_size(argc - 2, argv + 2, &dev, &off, &size,
+ &maxsize) < 0)
return 1;

if (!nand_unlock(&nand_info[dev], off, size, allexcept)) {
@@ -873,7 +880,8 @@ static int nand_load_image(cmd_tbl_t *cmdtp, nand_info_t *nand,
printf("\nLoading from %s, offset 0x%lx\n", nand->name, offset);

cnt = nand->writesize;
- r = nand_read_skip_bad(nand, offset, &cnt, (u_char *) addr);
+ r = nand_read_skip_bad(nand, offset, &cnt, NULL, nand->size,
+ (u_char *) addr);
if (r) {
puts("** Read error\n");
bootstage_error(BOOTSTAGE_ID_NAND_HDR_READ);
@@ -905,7 +913,8 @@ static int nand_load_image(cmd_tbl_t *cmdtp, nand_info_t *nand,
}
bootstage_mark(BOOTSTAGE_ID_NAND_TYPE);

- r = nand_read_skip_bad(nand, offset, &cnt, (u_char *) addr);
+ r = nand_read_skip_bad(nand, offset, &cnt, NULL, nand->size,
+ (u_char *) addr);
if (r) {
puts("** Read error\n");
bootstage_error(BOOTSTAGE_ID_NAND_READ);
diff --git a/common/env_nand.c b/common/env_nand.c
index 5b69889..b745822 100644
--- a/common/env_nand.c
+++ b/common/env_nand.c
@@ -281,7 +281,8 @@ int readenv(size_t offset, u_char *buf)
} else {
char_ptr = &buf[amount_loaded];
if (nand_read_skip_bad(&nand_info[0], offset,
- &len, char_ptr))
+ &len, NULL,
+ nand_info[0].size, char_ptr))
return 1;

offset += blocksize;
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c
index ff2d348..4727f9c 100644
--- a/drivers/mtd/nand/nand_util.c
+++ b/drivers/mtd/nand/nand_util.c
@@ -416,11 +416,13 @@ int nand_unlock(struct mtd_info *mtd, loff_t start, size_t length,
* @param nand NAND device
* @param offset offset in flash
* @param length image length
+ * @param used length of flash needed for the requested length
* @return 0 if the image fits and there are no bad blocks
* 1 if the image fits, but there are bad blocks
* -1 if the image does not fit
*/
-static int check_skip_len(nand_info_t *nand, loff_t offset, size_t length)
+static int check_skip_len(nand_info_t *nand, loff_t offset, size_t length,
+ size_t *used)
{
size_t len_excl_bad = 0;
int ret = 0;
@@ -442,8 +444,13 @@ static int check_skip_len(nand_info_t *nand, loff_t offset, size_t length)
ret = 1;

offset += block_len;
+ *used += block_len;
}

+ /* If the length is not a multiple of block_len, adjust. */
+ if (len_excl_bad > length)
+ *used -= (len_excl_bad - length);
+
return ret;
}

@@ -476,23 +483,36 @@ 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 on error, if not NULL
+ * @param lim maximum size that actual may be in order to not
+ * exceed the buffer
* @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;

+ if (actual)
+ *actual = 0;
+
#ifdef CONFIG_CMD_NAND_YAFFS
if (flags & WITH_YAFFS_OOB) {
if (flags & ~WITH_YAFFS_OOB)
@@ -529,13 +549,23 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
return -EINVAL;
}

- need_skip = check_skip_len(nand, offset, *length);
+ need_skip = check_skip_len(nand, offset, *length, &used_for_write);
+
+ if (actual)
+ *actual = used_for_write;
+
if (need_skip < 0) {
printf("Attempt to write outside the flash area\n");
*length = 0;
return -EINVAL;
}

+ if (used_for_write > lim) {
+ puts("Size of write exceeds partition or device limit\n");
+ *length = 0;
+ return -EFBIG;
+ }
+
if (!need_skip && !(flags & WITH_DROP_FFS)) {
rval = nand_write(nand, offset, length, buffer);
if (rval == 0)
@@ -626,36 +656,58 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
*
* Read image from NAND flash.
* Blocks that are marked bad are skipped and the next block is read
- * instead as long as the image is short enough to fit even after skipping the
- * bad blocks.
+ * instead as long as the image is short enough to fit even after
+ * skipping the bad blocks. Due to bad blocks we may not be able to
+ * perform the requested read. In the case where the read 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 read 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, on return holds number of read bytes
+ * @param actual set to size required to read length worth of buffer or 0
+ * on error, if not NULL
+ * @param lim maximum size that actual may be in order to not exceed the
+ * buffer
* @param buffer buffer to write to
* @return 0 in case of success
*/
int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
- u_char *buffer)
+ size_t *actual, loff_t lim, u_char *buffer)
{
int rval;
size_t left_to_read = *length;
+ size_t used_for_read = 0;
u_char *p_buffer = buffer;
int need_skip;

if ((offset & (nand->writesize - 1)) != 0) {
printf("Attempt to read non page-aligned data\n");
*length = 0;
+ if (actual)
+ *actual = 0;
return -EINVAL;
}

- need_skip = check_skip_len(nand, offset, *length);
+ need_skip = check_skip_len(nand, offset, *length, &used_for_read);
+
+ if (actual)
+ *actual = used_for_read;
+
if (need_skip < 0) {
printf("Attempt to read outside the flash area\n");
*length = 0;
return -EINVAL;
}

+ if (used_for_read > lim) {
+ puts("Size of read exceeds partition or device limit\n");
+ *length = 0;
+ return -EFBIG;
+ }
+
if (!need_skip) {
rval = nand_read(nand, offset, length, buffer);
if (!rval || rval == -EUCLEAN)
diff --git a/include/nand.h b/include/nand.h
index dded4e2..f0f3bf9 100644
--- a/include/nand.h
+++ b/include/nand.h
@@ -129,7 +129,7 @@ struct nand_erase_options {
typedef struct nand_erase_options nand_erase_options_t;

int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
- u_char *buffer);
+ size_t *actual, loff_t lim, u_char *buffer);

#define WITH_YAFFS_OOB (1 << 0) /* whether write with yaffs format. This flag
* is a 'mode' meaning it cannot be mixed with
@@ -137,7 +137,7 @@ int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
#define WITH_DROP_FFS (1 << 1) /* drop trailing all-0xff pages */

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 nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts);
int nand_torture(nand_info_t *nand, loff_t offset);

--
1.7.9.5