Linux modified the MTD driver interface in commit edbc4540 (with the same name as this commit). The effect is that calls to mtd_read will not return -EUCLEAN if the number of ECC-corrected bit errors is below a certain threshold, which defaults to the strength of the ECC. This allows -EUCLEAN to stop indicating "some bits were corrected" and begin indicating "a large number of bits were corrected, the data held in this region of flash may be lost soon". UBI makes use of this and when -EUCLEAN is returned from mtd_read it will move data to another block of flash. Without adopting this interface change UBI on U-boot attempts to move data between blocks every time a single bit is corrected using the ECC, which is a very common occurance on some devices. For some devices it can be so common that UBI gets stuck constantly moving data around because each block it attempts to use has a single bit error. This patch adopts the interface change as in Linux commit edbc4540 in order to avoid such situations.

Given that none of the drivers under drivers/mtd return -EUCLEAN, this should only affect those using software ECC. I have tested that it works on a board which is currently out of tree, but which I hope to be able to begin upstreaming soon.

Signed-off-by: Paul Burton paul.burton@imgtec.com --- drivers/mtd/mtdcore.c | 14 +++++++++++++- drivers/mtd/mtdpart.c | 12 ++++++------ drivers/mtd/nand/nand_base.c | 18 ++++++++++++++---- drivers/mtd/onenand/onenand_base.c | 3 ++- include/linux/mtd/nand.h | 3 +++ 5 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index 49c0814..deda5f2 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -217,11 +217,23 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info *instr) int mtd_read(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen, u_char *buf) { + int ret_code; if (from < 0 || from > mtd->size || len > mtd->size - from) return -EINVAL; if (!len) return 0; - return mtd->_read(mtd, from, len, retlen, buf); + + /* + * In the absence of an error, drivers return a non-negative integer + * representing the maximum number of bitflips that were corrected on + * any one ecc region (if applicable; zero otherwise). + */ + ret_code = mtd->_read(mtd, from, len, retlen, buf); + if (unlikely(ret_code < 0)) + return ret_code; + if (mtd->ecc_strength == 0) + return 0; /* device lacks ecc */ + return ret_code >= mtd->bitflip_threshold ? -EUCLEAN : 0; }

int mtd_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen, diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c index 9dfe7bb..146ce11 100644 --- a/drivers/mtd/mtdpart.c +++ b/drivers/mtd/mtdpart.c @@ -53,12 +53,12 @@ static int part_read(struct mtd_info *mtd, loff_t from, size_t len,

stats = part->master->ecc_stats; res = mtd_read(part->master, from + part->offset, len, retlen, buf); - if (unlikely(res)) { - if (mtd_is_bitflip(res)) - mtd->ecc_stats.corrected += part->master->ecc_stats.corrected - stats.corrected; - if (mtd_is_eccerr(res)) - mtd->ecc_stats.failed += part->master->ecc_stats.failed - stats.failed; - } + if (unlikely(mtd_is_eccerr(res))) + mtd->ecc_stats.failed += + part->master->ecc_stats.failed - stats.failed; + else + mtd->ecc_stats.corrected += + part->master->ecc_stats.corrected - stats.corrected; return res; }

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 9e05cef..d4d586c 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -1238,6 +1238,7 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from, mtd->oobavail : mtd->oobsize;

uint8_t *bufpoi, *oob, *buf; + unsigned int max_bitflips = 0;

stats = mtd->ecc_stats;

@@ -1265,7 +1266,10 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,

chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);

- /* Now read the page into the buffer */ + /* + * Now read the page into the buffer. Absent an error, + * the read methods return max bitflips per ecc step. + */ if (unlikely(ops->mode == MTD_OPS_RAW)) ret = chip->ecc.read_page_raw(mtd, chip, bufpoi, oob_required, @@ -1284,15 +1288,19 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from, break; }

+ max_bitflips = max_t(unsigned int, max_bitflips, ret); + /* Transfer not aligned data */ if (!aligned) { if (!NAND_HAS_SUBPAGE_READ(chip) && !oob && !(mtd->ecc_stats.failed - stats.failed) && - (ops->mode != MTD_OPS_RAW)) + (ops->mode != MTD_OPS_RAW)) { chip->pagebuf = realpage; - else + chip->pagebuf_bitflips = ret; + } else { /* Invalidate page cache */ chip->pagebuf = -1; + } memcpy(buf, chip->buffers->databuf + col, bytes); }

@@ -1310,6 +1318,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from, } else { memcpy(buf, chip->buffers->databuf + col, bytes); buf += bytes; + max_bitflips = max_t(unsigned int, max_bitflips, + chip->pagebuf_bitflips); }

readlen -= bytes; @@ -1341,7 +1351,7 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from, if (mtd->ecc_stats.failed - stats.failed) return -EBADMSG;

- return mtd->ecc_stats.corrected - stats.corrected ? -EUCLEAN : 0; + return max_bitflips; }

/** diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c index ddfe7e7..067f8ef 100644 --- a/drivers/mtd/onenand/onenand_base.c +++ b/drivers/mtd/onenand/onenand_base.c @@ -969,7 +969,8 @@ static int onenand_read_ops_nolock(struct mtd_info *mtd, loff_t from, if (mtd->ecc_stats.failed - stats.failed) return -EBADMSG;

- return mtd->ecc_stats.corrected - stats.corrected ? -EUCLEAN : 0; + /* return max bitflips per ecc step; ONENANDs correct 1 bit only */ + return mtd->ecc_stats.corrected != stats.corrected ? 1 : 0; }

/** diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index 2055584..0546565 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -464,6 +464,8 @@ struct nand_buffers { * @pagemask: [INTERN] page number mask = number of (pages / chip) - 1 * @pagebuf: [INTERN] holds the pagenumber which is currently in * data_buf. + * @pagebuf_bitflips: [INTERN] holds the bitflip count for the page which is + * currently in data_buf. * @subpagesize: [INTERN] holds the subpagesize * @onfi_version: [INTERN] holds the chip ONFI version (BCD encoded), * non 0 if ONFI supported. @@ -531,6 +533,7 @@ struct nand_chip { uint64_t chipsize; int pagemask; int pagebuf; + unsigned int pagebuf_bitflips; int subpagesize; uint8_t cellinfo; int badblockpos;

This matches the 64 bit size in struct mtd_info and allows the mtdparts command to function correctly with a flash >= 4GiB. Format specifiers for size & offset are given the ll length, matching its use in drivers/mtd in absence of something like inttypes.h/PRIx64.

Signed-off-by: Paul Burton paul.burton@imgtec.com --- common/cmd_mtdparts.c | 54 ++++++++++++++++++++++++--------------------- include/jffs2/load_kernel.h | 6 ++--- 2 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/common/cmd_mtdparts.c b/common/cmd_mtdparts.c index 3023479..453ed57 100644 --- a/common/cmd_mtdparts.c +++ b/common/cmd_mtdparts.c @@ -93,13 +93,13 @@ DECLARE_GLOBAL_DATA_PTR;

/* special size referring to all the remaining space in a partition */ -#define SIZE_REMAINING 0xFFFFFFFF +#define SIZE_REMAINING (~0llu)

/* special offset value, it is used when not provided by user * * this value is used temporarily during parsing, later such offests * are recalculated */ -#define OFFSET_NOT_SPECIFIED 0xFFFFFFFF +#define OFFSET_NOT_SPECIFIED (~0llu)

/* minimum partition size */ #define MIN_PART_SIZE 4096 @@ -160,9 +160,9 @@ static int device_del(struct mtd_device *dev); * @param retptr output pointer to next char after parse completes (output) * @return resulting unsigned int */ -static unsigned long memsize_parse (const char *const ptr, const char **retptr) +static u64 memsize_parse (const char *const ptr, const char **retptr) { - unsigned long ret = simple_strtoul(ptr, (char **)retptr, 0); + u64 ret = simple_strtoull(ptr, (char **)retptr, 0);

switch (**retptr) { case 'G': @@ -193,20 +193,20 @@ static unsigned long memsize_parse (const char *const ptr, const char **retptr) * @param buf output buffer * @param size size to be converted to string */ -static void memsize_format(char *buf, u32 size) +static void memsize_format(char *buf, u64 size) { #define SIZE_GB ((u32)1024*1024*1024) #define SIZE_MB ((u32)1024*1024) #define SIZE_KB ((u32)1024)

if ((size % SIZE_GB) == 0) - sprintf(buf, "%ug", size/SIZE_GB); + sprintf(buf, "%llug", size/SIZE_GB); else if ((size % SIZE_MB) == 0) - sprintf(buf, "%um", size/SIZE_MB); + sprintf(buf, "%llum", size/SIZE_MB); else if (size % SIZE_KB == 0) - sprintf(buf, "%uk", size/SIZE_KB); + sprintf(buf, "%lluk", size/SIZE_KB); else - sprintf(buf, "%u", size); + sprintf(buf, "%llu", size); }

/** @@ -310,6 +310,7 @@ static int part_validate_eraseblock(struct mtdids *id, struct part_info *part) struct mtd_info *mtd = NULL; int i, j; ulong start; + u64 offset, size;

if (get_mtd_info(id->type, id->num, &mtd)) return 1; @@ -321,14 +322,16 @@ static int part_validate_eraseblock(struct mtdids *id, struct part_info *part) * Only one eraseregion (NAND, OneNAND or uniform NOR), * checking for alignment is easy here */ - if ((unsigned long)part->offset % mtd->erasesize) { + offset = part->offset; + if (do_div(offset, mtd->erasesize)) { printf("%s%d: partition (%s) start offset" "alignment incorrect\n", MTD_DEV_TYPE(id->type), id->num, part->name); return 1; }

- if (part->size % mtd->erasesize) { + size = part->size; + if (do_div(size, mtd->erasesize)) { printf("%s%d: partition (%s) size alignment incorrect\n", MTD_DEV_TYPE(id->type), id->num, part->name); return 1; @@ -396,7 +399,7 @@ static int part_validate(struct mtdids *id, struct part_info *part) part->size = id->size - part->offset;

if (part->offset > id->size) { - printf("%s: offset %08x beyond flash size %08x\n", + printf("%s: offset %08llx beyond flash size %08llx\n", id->mtd_id, part->offset, id->size); return 1; } @@ -579,8 +582,8 @@ static int part_add(struct mtd_device *dev, struct part_info *part) static int part_parse(const char *const partdef, const char **ret, struct part_info **retpart) { struct part_info *part; - unsigned long size; - unsigned long offset; + u64 size; + u64 offset; const char *name; int name_len; unsigned int mask_flags; @@ -599,7 +602,7 @@ static int part_parse(const char *const partdef, const char **ret, struct part_i } else { size = memsize_parse(p, &p); if (size < MIN_PART_SIZE) { - printf("partition size too small (%lx)\n", size); + printf("partition size too small (%llx)\n", size); return 1; } } @@ -671,14 +674,14 @@ static int part_parse(const char *const partdef, const char **ret, struct part_i part->auto_name = 0; } else { /* auto generated name in form of size@offset */ - sprintf(part->name, "0x%08lx@0x%08lx", size, offset); + sprintf(part->name, "0x%08llx@0x%08llx", size, offset); part->auto_name = 1; }

part->name[name_len - 1] = '\0'; INIT_LIST_HEAD(&part->link);

- debug("+ partition: name %-22s size 0x%08x offset 0x%08x mask flags %d\n", + debug("+ partition: name %-22s size 0x%08llx offset 0x%08llx mask flags %d\n", part->name, part->size, part->offset, part->mask_flags);

@@ -694,7 +697,7 @@ static int part_parse(const char *const partdef, const char **ret, struct part_i * @param size a pointer to the size of the mtd device (output) * @return 0 if device is valid, 1 otherwise */ -static int mtd_device_validate(u8 type, u8 num, u32 *size) +static int mtd_device_validate(u8 type, u8 num, u64 *size) { struct mtd_info *mtd = NULL;

@@ -827,7 +830,7 @@ static int device_parse(const char *const mtd_dev, const char **ret, struct mtd_ LIST_HEAD(tmp_list); struct list_head *entry, *n; u16 num_parts; - u32 offset; + u64 offset; int err = 1;

debug("===device_parse===\n"); @@ -1072,7 +1075,8 @@ static int generate_mtdparts(char *buf, u32 buflen) struct part_info *part, *prev_part; char *p = buf; char tmpbuf[32]; - u32 size, offset, len, part_cnt; + u64 size, offset; + u32 len, part_cnt; u32 maxlen = buflen - 1;

debug("--- generate_mtdparts ---\n"); @@ -1271,7 +1275,7 @@ static void print_partition_table(void)

list_for_each(pentry, &dev->parts) { part = list_entry(pentry, struct part_info, link); - printf("%2d: %-20s0x%08x\t0x%08x\t%d\n", + printf("%2d: %-20s0x%08llx\t0x%08llx\t%d\n", part_num, part->name, part->size, part->offset, part->mask_flags); #endif /* defined(CONFIG_CMD_MTDPARTS_SHOW_NET_SIZES) */ @@ -1298,7 +1302,7 @@ static void list_partitions(void) if (current_mtd_dev) { part = mtd_part_info(current_mtd_dev, current_mtd_partnum); if (part) { - printf("\nactive partition: %s%d,%d - (%s) 0x%08x @ 0x%08x\n", + printf("\nactive partition: %s%d,%d - (%s) 0x%08llx @ 0x%08llx\n", MTD_DEV_TYPE(current_mtd_dev->id->type), current_mtd_dev->id->num, current_mtd_partnum, part->name, part->size, part->offset); @@ -1398,7 +1402,7 @@ static int delete_partition(const char *id)

if (find_dev_and_part(id, &dev, &pnum, &part) == 0) {

- debug("delete_partition: device = %s%d, partition %d = (%s) 0x%08x@0x%08x\n", + debug("delete_partition: device = %s%d, partition %d = (%s) 0x%08llx@0x%08llx\n", MTD_DEV_TYPE(dev->id->type), dev->id->num, pnum, part->name, part->size, part->offset);

@@ -1590,7 +1594,7 @@ static int parse_mtdids(const char *const ids) struct list_head *entry, *n; struct mtdids *id_tmp; u8 type, num; - u32 size; + u64 size; int ret = 1;

debug("\n---parse_mtdids---\nmtdids = %s\n\n", ids); @@ -1664,7 +1668,7 @@ static int parse_mtdids(const char *const ids) id->mtd_id[mtd_id_len - 1] = '\0'; INIT_LIST_HEAD(&id->link);

- debug("+ id %s%d\t%16d bytes\t%s\n", + debug("+ id %s%d\t%16lld bytes\t%s\n", MTD_DEV_TYPE(id->type), id->num, id->size, id->mtd_id);

diff --git a/include/jffs2/load_kernel.h b/include/jffs2/load_kernel.h index e1943e5..dd0d23f 100644 --- a/include/jffs2/load_kernel.h +++ b/include/jffs2/load_kernel.h @@ -32,8 +32,8 @@ struct part_info { struct list_head link; char *name; /* partition name */ u8 auto_name; /* set to 1 for generated name */ - u32 size; /* total size of the partition */ - u32 offset; /* offset within device */ + u64 size; /* total size of the partition */ + u64 offset; /* offset within device */ void *jffs2_priv; /* used internaly by jffs2 */ u32 mask_flags; /* kernel MTD mask flags */ u32 sector_size; /* size of sector */ @@ -44,7 +44,7 @@ struct mtdids { struct list_head link; u8 type; /* device type */ u8 num; /* device number */ - u32 size; /* device size */ + u64 size; /* device size */ char *mtd_id; /* linux kernel device id */ };