On Tue, 2012-05-15 at 19:11 +0200, Richard Weinberger wrote:
> The fastmap mechanism needs to read the next sequence nummber
> directly.
>
> Signed-off-by: Richard Weinberger <richard at nod.at>
I started looking at the code, and made some notes at the same time and
few minor changes - here is the diff which you can apply on top of your
patches.
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 2399511..2f5c362 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -869,15 +869,22 @@ static int attach_by_fastmap(struct ubi_device *ubi)
fm_start = ubi_find_fastmap(ubi);
if (fm_start < 0)
+ /* TODO: instead, return 1 which means that fall-back to
+ * scanning is needed */
return -ENOENT;
+ /* TODO: we need to re-name scan.h to attach.h, scan_info to
+ * attach_info, si to ai, seb to aeb, and so on - because we share the
+ * same data structures between scanning and fastmap, so the word
+ * "scan" is not very sensible to have in the name any more.
+ */
si = ubi_read_fastmap(ubi, fm_start);
if (IS_ERR(si))
return PTR_ERR(si);
- ubi->bad_peb_count = 0;
+ /* TODO: the rest looks very much like attach_by_scanning() - we
+ * need to re-structure the code and avoid duplication */
ubi->good_peb_count = ubi->peb_count;
- ubi->corr_peb_count = 0;
ubi->max_ec = si->max_ec;
ubi->mean_ec = si->mean_ec;
ubi_msg("max. sequence number: %llu", si->max_sqnum);
@@ -888,6 +895,7 @@ static int attach_by_fastmap(struct ubi_device *ubi)
goto out_si;
}
+ /* TODO: the _scan suffix makes no sens anymore - kill it */
err = ubi_wl_init_scan(ubi, si);
if (err) {
ubi_err("ubi_wl_init_scan failed!");
@@ -1022,7 +1030,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset)
err = attach_by_fastmap(ubi);
if (err) {
if (err != -ENOENT)
- ubi_msg("falling back to attach by scanning mode!");
+ ubi_msg("falling back to attach by scanning");
ubi->attached_by_scanning = true;
err = attach_by_scanning(ubi);
diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
index 2ea1682..7c5f1f7 100644
--- a/drivers/mtd/ubi/fastmap.c
+++ b/drivers/mtd/ubi/fastmap.c
@@ -582,7 +582,12 @@ struct ubi_scan_info *ubi_read_fastmap(struct ubi_device *ubi,
goto out;
}
+ /* TODO: Before checking version - check the CRC.
+ * I think you need to add a separate helper function in io.c, similar
+ * to ubi_io_read_vid_hdr(). */
if (fmsb->version != UBI_FM_FMT_VERSION) {
+ /* TODO: we can fall back to scanning instead of saying good
+ * bye! */
ubi_err("Unknown fastmap format version!");
si = ERR_PTR(-EINVAL);
kfree(fmsb);
@@ -606,6 +611,7 @@ struct ubi_scan_info *ubi_read_fastmap(struct ubi_device *ubi,
if (!fm_raw) {
si = ERR_PTR(-ENOMEM);
kfree(fmsb);
+ /* TODO: goto? */
}
vh = ubi_zalloc_vid_hdr(ubi, GFP_KERNEL);
@@ -617,6 +623,9 @@ struct ubi_scan_info *ubi_read_fastmap(struct ubi_device *ubi,
}
for (i = 0; i < nblocks; i++) {
+ /* TODO: you basically perform the scanning here - you should
+ * share the same code as we use in scan.c: use process_peb().
+ */
ret = ubi_io_read_vid_hdr(ubi, be32_to_cpu(fmsb->block_loc[i]),
vh, 0);
if (ret) {
@@ -653,6 +662,7 @@ struct ubi_scan_info *ubi_read_fastmap(struct ubi_device *ubi,
i, be32_to_cpu(fmsb->block_loc[i]));
si = ERR_PTR(ret);
+ /* TODO: fsmb is not freed? */
goto free_vhdr;
}
}
--
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20120516/5d66ad08/attachment.sig>