From 40a71a87fa8e0cb3ec0fca4d152263734b203eb2 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sun, 28 Jun 2009 19:16:55 +0300 Subject: UBI: add empty eraseblocks verification This patch adds code which makes sure eraseblocks contain all 0xFF bytes before starting using them. The verification is done only when debugging checks are enabled. Signed-off-by: Artem Bityutskiy --- drivers/mtd/ubi/debug.h | 6 ++++++ drivers/mtd/ubi/io.c | 17 +++++++---------- drivers/mtd/ubi/wl.c | 8 ++++++++ 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/drivers/mtd/ubi/debug.h b/drivers/mtd/ubi/debug.h index 13777e5..6fc7fda 100644 --- a/drivers/mtd/ubi/debug.h +++ b/drivers/mtd/ubi/debug.h @@ -93,6 +93,12 @@ void ubi_dbg_dump_mkvol_req(const struct ubi_mkvol_req *req); #define UBI_IO_DEBUG 0 #endif +#ifdef CONFIG_MTD_UBI_DEBUG_PARANOID +int ubi_dbg_check_all_ff(struct ubi_device *ubi, int pnum, int offset, int len); +#else +#define ubi_dbg_check_all_ff(ubi, pnum, offset, len) 0 +#endif + #ifdef CONFIG_MTD_UBI_DEBUG_DISABLE_BGT #define DBG_DISABLE_BGT 1 #else diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index effaff2..6c5441e 100644 --- a/drivers/mtd/ubi/io.c +++ b/drivers/mtd/ubi/io.c @@ -98,8 +98,6 @@ static int paranoid_check_ec_hdr(const struct ubi_device *ubi, int pnum, static int paranoid_check_peb_vid_hdr(const struct ubi_device *ubi, int pnum); static int paranoid_check_vid_hdr(const struct ubi_device *ubi, int pnum, const struct ubi_vid_hdr *vid_hdr); -static int paranoid_check_all_ff(struct ubi_device *ubi, int pnum, int offset, - int len); static int paranoid_check_empty(struct ubi_device *ubi, int pnum); #else #define paranoid_check_not_bad(ubi, pnum) 0 @@ -107,7 +105,6 @@ static int paranoid_check_empty(struct ubi_device *ubi, int pnum); #define paranoid_check_ec_hdr(ubi, pnum, ec_hdr) 0 #define paranoid_check_peb_vid_hdr(ubi, pnum) 0 #define paranoid_check_vid_hdr(ubi, pnum, vid_hdr) 0 -#define paranoid_check_all_ff(ubi, pnum, offset, len) 0 #define paranoid_check_empty(ubi, pnum) 0 #endif @@ -244,7 +241,7 @@ int ubi_io_write(struct ubi_device *ubi, const void *buf, int pnum, int offset, return err > 0 ? -EINVAL : err; /* The area we are writing to has to contain all 0xFF bytes */ - err = paranoid_check_all_ff(ubi, pnum, offset, len); + err = ubi_dbg_check_all_ff(ubi, pnum, offset, len); if (err) return err > 0 ? -EINVAL : err; @@ -350,7 +347,7 @@ retry: return -EIO; } - err = paranoid_check_all_ff(ubi, pnum, 0, ubi->peb_size); + err = ubi_dbg_check_all_ff(ubi, pnum, 0, ubi->peb_size); if (err) return err > 0 ? -EINVAL : err; @@ -672,8 +669,7 @@ int ubi_io_read_ec_hdr(struct ubi_device *ubi, int pnum, if (read_err != -EBADMSG && check_pattern(ec_hdr, 0xFF, UBI_EC_HDR_SIZE)) { /* The physical eraseblock is supposedly empty */ - err = paranoid_check_all_ff(ubi, pnum, 0, - ubi->peb_size); + err = ubi_dbg_check_all_ff(ubi, pnum, 0, ubi->peb_size); if (err) return err > 0 ? UBI_IO_BAD_EC_HDR : err; @@ -1229,7 +1225,7 @@ exit: } /** - * paranoid_check_all_ff - check that a region of flash is empty. + * ubi_dbg_check_all_ff - check that a region of flash is empty. * @ubi: UBI device description object * @pnum: the physical eraseblock number to check * @offset: the starting offset within the physical eraseblock to check @@ -1239,13 +1235,14 @@ exit: * @offset of the physical eraseblock @pnum, %1 if not, and a negative error * code if an error occurred. */ -static int paranoid_check_all_ff(struct ubi_device *ubi, int pnum, int offset, - int len) +int ubi_dbg_check_all_ff(struct ubi_device *ubi, int pnum, int offset, int len) { size_t read; int err; loff_t addr = (loff_t)pnum * ubi->peb_size + offset; + ubi_assert(!mutex_is_locked(&ubi->dbg_buf_mutex)); + mutex_lock(&ubi->dbg_buf_mutex); err = ubi->mtd->read(ubi->mtd, addr, len, &read, ubi->dbg_peb_buf); if (err && err != -EUCLEAN) { diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c index 2b24723..e4be446 100644 --- a/drivers/mtd/ubi/wl.c +++ b/drivers/mtd/ubi/wl.c @@ -459,6 +459,14 @@ retry: dbg_wl("PEB %d EC %d", e->pnum, e->ec); prot_queue_add(ubi, e); spin_unlock(&ubi->wl_lock); + + err = ubi_dbg_check_all_ff(ubi, e->pnum, ubi->vid_hdr_aloffset, + ubi->peb_size - ubi->vid_hdr_aloffset); + if (err) { + dbg_err("new PEB does not contain all 0xFF bytes"); + return err > 0 ? -EINVAL : err; + } + return e->pnum; } -- cgit v1.1 From 1398788fe7b730db10e97dcb9f838603e41922d5 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Mon, 29 Jun 2009 15:58:36 +0300 Subject: UBI: remove bogus debugging checks The 'paranoid_check_empty()' is bogus because, which is easilly seen on NOR flash, which has long erase cycles, and which may easilly end-up with half-erased eraseblocks. In this case the paranoid check fails. I is just wrong to assume that PEBs which do not have EC headers always contain all 0xFF. Such assumption should not be made on the I/O level, which is quite low. Thus, just kill the check. Signed-off-by: Artem Bityutskiy --- drivers/mtd/ubi/debug.h | 1 + drivers/mtd/ubi/io.c | 87 ------------------------------------------------- drivers/mtd/ubi/wl.c | 2 +- 3 files changed, 2 insertions(+), 88 deletions(-) diff --git a/drivers/mtd/ubi/debug.h b/drivers/mtd/ubi/debug.h index 6fc7fda..a4da7a0 100644 --- a/drivers/mtd/ubi/debug.h +++ b/drivers/mtd/ubi/debug.h @@ -173,6 +173,7 @@ static inline int ubi_dbg_is_erase_failure(void) #define ubi_dbg_is_bitflip() 0 #define ubi_dbg_is_write_failure() 0 #define ubi_dbg_is_erase_failure() 0 +#define ubi_dbg_check_all_ff(ubi, pnum, offset, len) 0 #endif /* !CONFIG_MTD_UBI_DEBUG */ #endif /* !__UBI_DEBUG_H__ */ diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index 6c5441e..c8edbfd 100644 --- a/drivers/mtd/ubi/io.c +++ b/drivers/mtd/ubi/io.c @@ -98,14 +98,12 @@ static int paranoid_check_ec_hdr(const struct ubi_device *ubi, int pnum, static int paranoid_check_peb_vid_hdr(const struct ubi_device *ubi, int pnum); static int paranoid_check_vid_hdr(const struct ubi_device *ubi, int pnum, const struct ubi_vid_hdr *vid_hdr); -static int paranoid_check_empty(struct ubi_device *ubi, int pnum); #else #define paranoid_check_not_bad(ubi, pnum) 0 #define paranoid_check_peb_ec_hdr(ubi, pnum) 0 #define paranoid_check_ec_hdr(ubi, pnum, ec_hdr) 0 #define paranoid_check_peb_vid_hdr(ubi, pnum) 0 #define paranoid_check_vid_hdr(ubi, pnum, vid_hdr) 0 -#define paranoid_check_empty(ubi, pnum) 0 #endif /** @@ -669,10 +667,6 @@ int ubi_io_read_ec_hdr(struct ubi_device *ubi, int pnum, if (read_err != -EBADMSG && check_pattern(ec_hdr, 0xFF, UBI_EC_HDR_SIZE)) { /* The physical eraseblock is supposedly empty */ - err = ubi_dbg_check_all_ff(ubi, pnum, 0, ubi->peb_size); - if (err) - return err > 0 ? UBI_IO_BAD_EC_HDR : err; - if (verbose) ubi_warn("no EC header found at PEB %d, " "only 0xFF bytes", pnum); @@ -943,15 +937,6 @@ int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum, if (read_err != -EBADMSG && check_pattern(vid_hdr, 0xFF, UBI_VID_HDR_SIZE)) { /* The physical eraseblock is supposedly free */ - - /* - * The below is just a paranoid check, it has to be - * compiled out if paranoid checks are disabled. - */ - err = paranoid_check_empty(ubi, pnum); - if (err) - return err > 0 ? UBI_IO_BAD_VID_HDR : err; - if (verbose) ubi_warn("no VID header found at PEB %d, " "only 0xFF bytes", pnum); @@ -1241,8 +1226,6 @@ int ubi_dbg_check_all_ff(struct ubi_device *ubi, int pnum, int offset, int len) int err; loff_t addr = (loff_t)pnum * ubi->peb_size + offset; - ubi_assert(!mutex_is_locked(&ubi->dbg_buf_mutex)); - mutex_lock(&ubi->dbg_buf_mutex); err = ubi->mtd->read(ubi->mtd, addr, len, &read, ubi->dbg_peb_buf); if (err && err != -EUCLEAN) { @@ -1273,74 +1256,4 @@ error: return err; } -/** - * paranoid_check_empty - whether a PEB is empty. - * @ubi: UBI device description object - * @pnum: the physical eraseblock number to check - * - * This function makes sure PEB @pnum is empty, which means it contains only - * %0xFF data bytes. Returns zero if the PEB is empty, %1 if not, and a - * negative error code in case of failure. - * - * Empty PEBs have the EC header, and do not have the VID header. The caller of - * this function should have already made sure the PEB does not have the VID - * header. However, this function re-checks that, because it is possible that - * the header and data has already been written to the PEB. - * - * Let's consider a possible scenario. Suppose there are 2 tasks - A and B. - * Task A is in 'wear_leveling_worker()'. It is reading VID header of PEB X to - * find which LEB it corresponds to. PEB X is currently unmapped, and has no - * VID header. Task B is trying to write to PEB X. - * - * Task A: in 'ubi_io_read_vid_hdr()': reads the VID header from PEB X. The - * read data contain all 0xFF bytes; - * Task B: writes VID header and some data to PEB X; - * Task A: assumes PEB X is empty, calls 'paranoid_check_empty()'. And if we - * do not re-read the VID header, and do not cancel the checking if it - * is there, we fail. - */ -static int paranoid_check_empty(struct ubi_device *ubi, int pnum) -{ - int err, offs = ubi->vid_hdr_aloffset, len = ubi->vid_hdr_alsize; - size_t read; - uint32_t magic; - const struct ubi_vid_hdr *vid_hdr; - - mutex_lock(&ubi->dbg_buf_mutex); - err = ubi->mtd->read(ubi->mtd, offs, len, &read, ubi->dbg_peb_buf); - if (err && err != -EUCLEAN) { - ubi_err("error %d while reading %d bytes from PEB %d:%d, " - "read %zd bytes", err, len, pnum, offs, read); - goto error; - } - - vid_hdr = ubi->dbg_peb_buf; - magic = be32_to_cpu(vid_hdr->magic); - if (magic == UBI_VID_HDR_MAGIC) - /* The PEB contains VID header, so it is not empty */ - goto out; - - err = check_pattern(ubi->dbg_peb_buf, 0xFF, len); - if (err == 0) { - ubi_err("flash region at PEB %d:%d, length %d does not " - "contain all 0xFF bytes", pnum, offs, len); - goto fail; - } - -out: - mutex_unlock(&ubi->dbg_buf_mutex); - return 0; - -fail: - ubi_err("paranoid check failed for PEB %d", pnum); - ubi_msg("hex dump of the %d-%d region", offs, offs + len); - print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 32, 1, - ubi->dbg_peb_buf, len, 1); - err = 1; -error: - ubi_dbg_dump_stack(); - mutex_unlock(&ubi->dbg_buf_mutex); - return err; -} - #endif /* CONFIG_MTD_UBI_DEBUG_PARANOID */ diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c index e4be446..600c722 100644 --- a/drivers/mtd/ubi/wl.c +++ b/drivers/mtd/ubi/wl.c @@ -463,7 +463,7 @@ retry: err = ubi_dbg_check_all_ff(ubi, e->pnum, ubi->vid_hdr_aloffset, ubi->peb_size - ubi->vid_hdr_aloffset); if (err) { - dbg_err("new PEB does not contain all 0xFF bytes"); + ubi_err("new PEB %d does not contain all 0xFF bytes", e->pnum); return err > 0 ? -EINVAL : err; } -- cgit v1.1 From 0c6c7fa1313fcb69cae35e34168d2e83b8da854a Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Fri, 26 Jun 2009 14:58:01 +0300 Subject: UBI: add image sequence number to EC header An image sequence number is added to the UBI erase-counter header to be able determine if the root file system contains a mixture of old and new images (because the flashing failed to complete). A change to nolo is also needed for this to take effect. Signed-off-by: Adrian Hunter Signed-off-by: Artem Bityutskiy --- drivers/mtd/ubi/build.c | 1 + drivers/mtd/ubi/debug.c | 2 ++ drivers/mtd/ubi/io.c | 15 +++++++++++++-- drivers/mtd/ubi/scan.c | 2 ++ drivers/mtd/ubi/ubi-media.h | 12 +++++++++++- drivers/mtd/ubi/ubi.h | 4 ++++ 6 files changed, 33 insertions(+), 3 deletions(-) diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c index 286ed59..db0b9cb 100644 --- a/drivers/mtd/ubi/build.c +++ b/drivers/mtd/ubi/build.c @@ -996,6 +996,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset) ubi_msg("number of PEBs reserved for bad PEB handling: %d", ubi->beb_rsvd_pebs); ubi_msg("max/mean erase counter: %d/%d", ubi->max_ec, ubi->mean_ec); + ubi_msg("image sequence number: %d", ubi->image_seq); /* * The below lock makes sure we do not race with 'ubi_thread()' which diff --git a/drivers/mtd/ubi/debug.c b/drivers/mtd/ubi/debug.c index c0ed60e..54b0186 100644 --- a/drivers/mtd/ubi/debug.c +++ b/drivers/mtd/ubi/debug.c @@ -44,6 +44,8 @@ void ubi_dbg_dump_ec_hdr(const struct ubi_ec_hdr *ec_hdr) be32_to_cpu(ec_hdr->vid_hdr_offset)); printk(KERN_DEBUG "\tdata_offset %d\n", be32_to_cpu(ec_hdr->data_offset)); + printk(KERN_DEBUG "\timage_seq %d\n", + be32_to_cpu(ec_hdr->image_seq)); printk(KERN_DEBUG "\thdr_crc %#08x\n", be32_to_cpu(ec_hdr->hdr_crc)); printk(KERN_DEBUG "erase counter header hexdump:\n"); diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index c8edbfd..b587140 100644 --- a/drivers/mtd/ubi/io.c +++ b/drivers/mtd/ubi/io.c @@ -563,15 +563,16 @@ int ubi_io_mark_bad(const struct ubi_device *ubi, int pnum) * This function returns zero if the erase counter header is OK, and %1 if * not. */ -static int validate_ec_hdr(const struct ubi_device *ubi, +static int validate_ec_hdr(struct ubi_device *ubi, const struct ubi_ec_hdr *ec_hdr) { long long ec; - int vid_hdr_offset, leb_start; + int vid_hdr_offset, leb_start, image_seq; ec = be64_to_cpu(ec_hdr->ec); vid_hdr_offset = be32_to_cpu(ec_hdr->vid_hdr_offset); leb_start = be32_to_cpu(ec_hdr->data_offset); + image_seq = be32_to_cpu(ec_hdr->image_seq); if (ec_hdr->version != UBI_VERSION) { ubi_err("node with incompatible UBI version found: " @@ -597,6 +598,15 @@ static int validate_ec_hdr(const struct ubi_device *ubi, goto bad; } + if (!ubi->image_seq_set) { + ubi->image_seq = image_seq; + ubi->image_seq_set = 1; + } else if (ubi->image_seq != image_seq) { + ubi_err("bad image sequence number %d, expected %d", + image_seq, ubi->image_seq); + goto bad; + } + return 0; bad: @@ -742,6 +752,7 @@ int ubi_io_write_ec_hdr(struct ubi_device *ubi, int pnum, ec_hdr->version = UBI_VERSION; ec_hdr->vid_hdr_offset = cpu_to_be32(ubi->vid_hdr_offset); ec_hdr->data_offset = cpu_to_be32(ubi->leb_start); + ec_hdr->image_seq = cpu_to_be32(ubi->image_seq); crc = crc32(UBI_CRC32_INIT, ec_hdr, UBI_EC_HDR_SIZE_CRC); ec_hdr->hdr_crc = cpu_to_be32(crc); diff --git a/drivers/mtd/ubi/scan.c b/drivers/mtd/ubi/scan.c index c3d653b..72570ed 100644 --- a/drivers/mtd/ubi/scan.c +++ b/drivers/mtd/ubi/scan.c @@ -910,6 +910,8 @@ struct ubi_scan_info *ubi_scan(struct ubi_device *ubi) if (si->is_empty) ubi_msg("empty MTD device detected"); + ubi->image_seq_set = 1; + /* * In case of unknown erase counter we use the mean erase counter * value. diff --git a/drivers/mtd/ubi/ubi-media.h b/drivers/mtd/ubi/ubi-media.h index 8419fdc..503ea9b 100644 --- a/drivers/mtd/ubi/ubi-media.h +++ b/drivers/mtd/ubi/ubi-media.h @@ -129,6 +129,7 @@ enum { * @ec: the erase counter * @vid_hdr_offset: where the VID header starts * @data_offset: where the user data start + * @image_seq: image sequence number * @padding2: reserved for future, zeroes * @hdr_crc: erase counter header CRC checksum * @@ -144,6 +145,14 @@ enum { * volume identifier header and user data, relative to the beginning of the * physical eraseblock. These values have to be the same for all physical * eraseblocks. + * + * The @image_seq field is used to validate a UBI image that has been prepared + * for a UBI device. The @image_seq value can be any value, but it must be the + * same on all eraseblocks. UBI will ensure that all new erase counter headers + * also contain this value, and will check the value when scanning at start-up. + * One way to make use of @image_seq is to increase its value by one every time + * an image is flashed over an existing image, then, if the flashing does not + * complete, UBI will detect the error when scanning. */ struct ubi_ec_hdr { __be32 magic; @@ -152,7 +161,8 @@ struct ubi_ec_hdr { __be64 ec; /* Warning: the current limit is 31-bit anyway! */ __be32 vid_hdr_offset; __be32 data_offset; - __u8 padding2[36]; + __be32 image_seq; + __u8 padding2[32]; __be32 hdr_crc; } __attribute__ ((packed)); diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h index 28acd13..bb372c4 100644 --- a/drivers/mtd/ubi/ubi.h +++ b/drivers/mtd/ubi/ubi.h @@ -301,6 +301,8 @@ struct ubi_wl_entry; * @vol->readers, @vol->writers, @vol->exclusive, * @vol->ref_count, @vol->mapping and @vol->eba_tbl. * @ref_count: count of references on the UBI device + * @image_seq: image sequence number recorded on EC headers + * @image_seq_set: indicates @image_seq is known * * @rsvd_pebs: count of reserved physical eraseblocks * @avail_pebs: count of available physical eraseblocks @@ -390,6 +392,8 @@ struct ubi_device { struct ubi_volume *volumes[UBI_MAX_VOLUMES+UBI_INT_VOL_COUNT]; spinlock_t volumes_lock; int ref_count; + int image_seq; + int image_seq_set; int rsvd_pebs; int avail_pebs; -- cgit v1.1 From fe96efc1a3c049f0a1bcd9b65e0faeb751ce5ec6 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Tue, 30 Jun 2009 16:11:59 +0300 Subject: UBI: nicify image sequence number handling Move the image seq. number handling from I/O level to the scanning lever, where it really belongs to. Move the @image_seq_set variable to the @struct ubi_scan_info structure, which exists only during scanning. Signed-off-by: Artem Bityutskiy --- drivers/mtd/ubi/io.c | 14 ++------------ drivers/mtd/ubi/scan.c | 16 ++++++++++++++-- drivers/mtd/ubi/scan.h | 2 ++ drivers/mtd/ubi/ubi.h | 2 -- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index b587140..1ea1421 100644 --- a/drivers/mtd/ubi/io.c +++ b/drivers/mtd/ubi/io.c @@ -563,16 +563,15 @@ int ubi_io_mark_bad(const struct ubi_device *ubi, int pnum) * This function returns zero if the erase counter header is OK, and %1 if * not. */ -static int validate_ec_hdr(struct ubi_device *ubi, +static int validate_ec_hdr(const struct ubi_device *ubi, const struct ubi_ec_hdr *ec_hdr) { long long ec; - int vid_hdr_offset, leb_start, image_seq; + int vid_hdr_offset, leb_start; ec = be64_to_cpu(ec_hdr->ec); vid_hdr_offset = be32_to_cpu(ec_hdr->vid_hdr_offset); leb_start = be32_to_cpu(ec_hdr->data_offset); - image_seq = be32_to_cpu(ec_hdr->image_seq); if (ec_hdr->version != UBI_VERSION) { ubi_err("node with incompatible UBI version found: " @@ -598,15 +597,6 @@ static int validate_ec_hdr(struct ubi_device *ubi, goto bad; } - if (!ubi->image_seq_set) { - ubi->image_seq = image_seq; - ubi->image_seq_set = 1; - } else if (ubi->image_seq != image_seq) { - ubi_err("bad image sequence number %d, expected %d", - image_seq, ubi->image_seq); - goto bad; - } - return 0; bad: diff --git a/drivers/mtd/ubi/scan.c b/drivers/mtd/ubi/scan.c index 72570ed..f60895e 100644 --- a/drivers/mtd/ubi/scan.c +++ b/drivers/mtd/ubi/scan.c @@ -757,6 +757,8 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si, si->is_empty = 0; if (!ec_corr) { + int image_seq; + /* Make sure UBI version is OK */ if (ech->version != UBI_VERSION) { ubi_err("this UBI version is %d, image version is %d", @@ -778,6 +780,18 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si, ubi_dbg_dump_ec_hdr(ech); return -EINVAL; } + + image_seq = be32_to_cpu(ech->ec); + if (!si->image_seq_set) { + ubi->image_seq = image_seq; + si->image_seq_set = 1; + } else if (ubi->image_seq != image_seq) { + ubi_err("bad image sequence number %d in PEB %d, " + "expected %d", image_seq, pnum, ubi->image_seq); + ubi_dbg_dump_ec_hdr(ech); + return -EINVAL; + } + } /* OK, we've done with the EC header, let's look at the VID header */ @@ -910,8 +924,6 @@ struct ubi_scan_info *ubi_scan(struct ubi_device *ubi) if (si->is_empty) ubi_msg("empty MTD device detected"); - ubi->image_seq_set = 1; - /* * In case of unknown erase counter we use the mean erase counter * value. diff --git a/drivers/mtd/ubi/scan.h b/drivers/mtd/ubi/scan.h index 61df208..1017cf1 100644 --- a/drivers/mtd/ubi/scan.h +++ b/drivers/mtd/ubi/scan.h @@ -102,6 +102,7 @@ struct ubi_scan_volume { * @mean_ec: mean erase counter value * @ec_sum: a temporary variable used when calculating @mean_ec * @ec_count: a temporary variable used when calculating @mean_ec + * @image_seq_set: indicates @ubi->image_seq is known * * This data structure contains the result of scanning and may be used by other * UBI sub-systems to build final UBI data structures, further error-recovery @@ -124,6 +125,7 @@ struct ubi_scan_info { int mean_ec; uint64_t ec_sum; int ec_count; + int image_seq_set; }; struct ubi_device; diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h index bb372c4..64604e8 100644 --- a/drivers/mtd/ubi/ubi.h +++ b/drivers/mtd/ubi/ubi.h @@ -302,7 +302,6 @@ struct ubi_wl_entry; * @vol->ref_count, @vol->mapping and @vol->eba_tbl. * @ref_count: count of references on the UBI device * @image_seq: image sequence number recorded on EC headers - * @image_seq_set: indicates @image_seq is known * * @rsvd_pebs: count of reserved physical eraseblocks * @avail_pebs: count of available physical eraseblocks @@ -393,7 +392,6 @@ struct ubi_device { spinlock_t volumes_lock; int ref_count; int image_seq; - int image_seq_set; int rsvd_pebs; int avail_pebs; -- cgit v1.1 From ebf53f421308c2f59c9bcbad4c5c297a0d00199a Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Mon, 6 Jul 2009 08:57:53 +0300 Subject: UBI: fix NOR flash recovery This commit fixes NOR flash recovery issues observed with Spansion S29GL512N NOR. When NOR erases, it first fills PEBs with zeroes, then sets all bytes to 0xFF. Filling with zeroes starts from the end of the PEB. And when power is cut, this results in PEBs containing correct EC and VID headers but corrupted with zeros at the end. This confuses UBI and it mistakinly accepts these PEBs and associate them with LEBs. Fis this issue by zeroing EC and VID magics before erasing PEBs, to make UBI later refuse zem. Signed-off-by: Artem Bityutskiy --- drivers/mtd/ubi/build.c | 5 +++++ drivers/mtd/ubi/io.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++-- drivers/mtd/ubi/ubi.h | 4 +++- 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c index db0b9cb..e1f7d0a 100644 --- a/drivers/mtd/ubi/build.c +++ b/drivers/mtd/ubi/build.c @@ -657,6 +657,11 @@ static int io_init(struct ubi_device *ubi) if (ubi->mtd->block_isbad && ubi->mtd->block_markbad) ubi->bad_allowed = 1; + if (ubi->mtd->type == MTD_NORFLASH) { + ubi_assert(ubi->mtd->writesize == 1); + ubi->nor_flash = 1; + } + ubi->min_io_size = ubi->mtd->writesize; ubi->hdrs_min_io_size = ubi->mtd->writesize >> ubi->mtd->subpage_sft; diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index 1ea1421..c3bd2e3 100644 --- a/drivers/mtd/ubi/io.c +++ b/drivers/mtd/ubi/io.c @@ -266,8 +266,8 @@ int ubi_io_write(struct ubi_device *ubi, const void *buf, int pnum, int offset, addr = (loff_t)pnum * ubi->peb_size + offset; err = ubi->mtd->write(ubi->mtd, addr, len, &written, buf); if (err) { - ubi_err("error %d while writing %d bytes to PEB %d:%d, written" - " %zd bytes", err, len, pnum, offset, written); + ubi_err("error %d while writing %d bytes to PEB %d:%d, written " + "%zd bytes", err, len, pnum, offset, written); ubi_dbg_dump_stack(); } else ubi_assert(written == len); @@ -454,6 +454,54 @@ out: } /** + * nor_erase_prepare - prepare a NOR flash PEB for erasure. + * @ubi: UBI device description object + * @pnum: physical eraseblock number to prepare + * + * NOR flash, or at least some of them, have peculiar embedded PEB erasure + * algorithm: the PEB is first filled with zeroes, then it is erased. And + * filling with zeroes starts from the end of the PEB. This was observed with + * Spansion S29GL512N NOR flash. + * + * This means that in case of a power cut we may end up with intact data at the + * beginning of the PEB, and all zeroes at the end of PEB. In other words, the + * EC and VID headers are OK, but a large chunk of data at the end of PEB is + * zeroed. This makes UBI mistakenly treat this PEB as used and associate it + * with an LEB, which leads to subsequent failures (e.g., UBIFS fails). + * + * This function is called before erasing NOR PEBs and it zeroes out EC and VID + * magic numbers in order to invalidate them and prevent the failures. Returns + * zero in case of success and a negative error code in case of failure. + */ +static int nor_erase_prepare(struct ubi_device *ubi, int pnum) +{ + int err; + size_t written; + loff_t addr; + uint32_t data = 0; + + addr = (loff_t)pnum * ubi->peb_size; + err = ubi->mtd->write(ubi->mtd, addr, 4, &written, &data); + if (err) { + ubi_err("error %d while writing 4 bytes to PEB %d:0, written " + "%zd bytes", err, pnum, 0, written); + ubi_dbg_dump_stack(); + return err; + } + + addr += ubi->vid_hdr_aloffset; + err = ubi->mtd->write(ubi->mtd, addr, 4, &written, &data); + if (err) { + ubi_err("error %d while writing 4 bytes to PEB %d:%d, written " + "%zd bytes", err, pnum, ubi->vid_hdr_aloffset, written); + ubi_dbg_dump_stack(); + return err; + } + + return 0; +} + +/** * ubi_io_sync_erase - synchronously erase a physical eraseblock. * @ubi: UBI device description object * @pnum: physical eraseblock number to erase @@ -484,6 +532,12 @@ int ubi_io_sync_erase(struct ubi_device *ubi, int pnum, int torture) return -EROFS; } + if (ubi->nor_flash) { + err = nor_erase_prepare(ubi, pnum); + if (err) + return err; + } + if (torture) { ret = torture_peb(ubi, pnum); if (ret < 0) diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h index 64604e8..6a5fe96 100644 --- a/drivers/mtd/ubi/ubi.h +++ b/drivers/mtd/ubi/ubi.h @@ -373,6 +373,7 @@ struct ubi_wl_entry; * @vid_hdr_shift: contains @vid_hdr_offset - @vid_hdr_aloffset * @bad_allowed: whether the MTD device admits of bad physical eraseblocks or * not + * @nor_flash: non-zero if working on top of NOR flash * @mtd: MTD device descriptor * * @peb_buf1: a buffer of PEB size used for different purposes @@ -454,7 +455,8 @@ struct ubi_device { int vid_hdr_offset; int vid_hdr_aloffset; int vid_hdr_shift; - int bad_allowed; + unsigned int bad_allowed:1; + unsigned int nor_flash:1; struct mtd_info *mtd; void *peb_buf1; -- cgit v1.1 From 83c2099f5e1f0a4621ed4f20fc539069c636a24b Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Wed, 8 Jul 2009 10:15:41 +0300 Subject: UBI: fix compilation warnings The recent "UBI: fix NOR flash recovery" introduced compilation warnings which were immediately spotted by our linux-next keeper. This patch fixes them. Reported-by: Stephen Rothwell Signed-off-by: Artem Bityutskiy --- drivers/mtd/ubi/io.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index c3bd2e3..4cb6992 100644 --- a/drivers/mtd/ubi/io.c +++ b/drivers/mtd/ubi/io.c @@ -481,16 +481,16 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum) uint32_t data = 0; addr = (loff_t)pnum * ubi->peb_size; - err = ubi->mtd->write(ubi->mtd, addr, 4, &written, &data); + err = ubi->mtd->write(ubi->mtd, addr, 4, &written, (void *)&data); if (err) { - ubi_err("error %d while writing 4 bytes to PEB %d:0, written " + ubi_err("error %d while writing 4 bytes to PEB %d:%d, written " "%zd bytes", err, pnum, 0, written); ubi_dbg_dump_stack(); return err; } addr += ubi->vid_hdr_aloffset; - err = ubi->mtd->write(ubi->mtd, addr, 4, &written, &data); + err = ubi->mtd->write(ubi->mtd, addr, 4, &written, (void *)&data); if (err) { ubi_err("error %d while writing 4 bytes to PEB %d:%d, written " "%zd bytes", err, pnum, ubi->vid_hdr_aloffset, written); -- cgit v1.1