From patchwork Fri Mar 8 17:37:23 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Rini X-Patchwork-Id: 226174 X-Patchwork-Delegate: marek.vasut@gmail.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 46CF12C0392 for ; Sat, 9 Mar 2013 04:38:46 +1100 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 3FFBE4A3E8; Fri, 8 Mar 2013 18:38:25 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id uculFUtH9KXi; Fri, 8 Mar 2013 18:38:25 +0100 (CET) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 659F04A3F8; Fri, 8 Mar 2013 18:37:50 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 7DB874A3DC for ; Fri, 8 Mar 2013 18:37:44 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id o-wXr+bGpqO0 for ; Fri, 8 Mar 2013 18:37:43 +0100 (CET) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from mail-yh0-f53.google.com (mail-yh0-f53.google.com [209.85.213.53]) by theia.denx.de (Postfix) with ESMTPS id 1017A4A3D9 for ; Fri, 8 Mar 2013 18:37:40 +0100 (CET) Received: by mail-yh0-f53.google.com with SMTP id q3so296899yhf.26 for ; Fri, 08 Mar 2013 09:37:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:sender:from:to:cc:subject:date:message-id:x-mailer :in-reply-to:references; bh=hyugH7eYpx8liHoJ52GiP3gQo5OpHQpPG08WMKxDSpU=; b=B/tcJ+jM7jk5DjNwIV3oGa0iVfOS4gyc/kOl6hD5T2Pf0iVIFdEdVFetnEzO9D+K+d 3sJIfH1s+qTWc/EcuSc5VUNLqkA2XBAlOMyuxNBqiasFT5F0lztuIsqeSOkitzqD9Wyi OkD9+28IgVa8xxD65iWm9xFTveqOD6kNb6diPCPnNJZpsfqj7O80PDZm4GcR22SKB2I7 tLWxAVWM1BxCG9dNCh9/LfOFUfIYoclZCm3xRyzfpf72RXjICKP4Pwwoy3nD3InknS2L 5AkfZPhQS6bKoZfWF9IEfI+gERgjBqp74Oah5yXzyYtmaMKr79bUzUnX58QaM1RQlrza PX8Q== X-Received: by 10.236.166.34 with SMTP id f22mr2361573yhl.87.1362764259139; Fri, 08 Mar 2013 09:37:39 -0800 (PST) Received: from localhost.localdomain (cpe-065-184-250-089.ec.res.rr.com. [65.184.250.89]) by mx.google.com with ESMTPS id v43sm8924053yhm.11.2013.03.08.09.37.37 (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 08 Mar 2013 09:37:38 -0800 (PST) From: Tom Rini To: u-boot@lists.denx.de Date: Fri, 8 Mar 2013 12:37:23 -0500 Message-Id: <1362764249-15547-5-git-send-email-trini@ti.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1362764249-15547-1-git-send-email-trini@ti.com> References: <1362764249-15547-1-git-send-email-trini@ti.com> Cc: Scott Wood , Pantelis Antoniou Subject: [U-Boot] [PATCH v5 4/9] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.11 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@lists.denx.de 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 Signed-off-by: Tom Rini Acked-by: Scott Wood --- 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);