new file mode 100644
@@ -0,0 +1,12 @@
+This is a TODO list for the fastmap feature which should be done before
+it is merged.
+
+For all or most of the items there we need to write a testcase. We can put it
+to the ubi-utils.git repository, to a separate branch at the beginning
+
+1. Test with a R/O MTD device: !(mtd->flags & MTD_WRITEABLE)
+2. Implement power-cut emulation infrastructure similar to what is in UBIFS and
+ test UBI + fastmap with it.
+3. Test the autoresize feature
+4. Test 'ubi_flush()'
+5. Test
@@ -1224,12 +1224,26 @@ int ubi_attach(struct ubi_device *ubi)
int err;
struct ubi_attach_info *ai = NULL;
+ /* TODO: Allocate ai in this fuction. And destroy it here as well */
+
err = ubi_scan_fastmap(ubi, &ai);
if (err > 0) {
+ /* TODO: in UBIFS we have a convention: every function prints
+ * its own error messages. This makes things cleaner and easier
+ * - the caller should not care about printing anything.
+ * Please, move this error message to 'ubi_scan_fastmap()'. And
+ * keep this in mind, and do similar thing globally for entire
+ * fastmap code. */
if (err == UBI_BAD_FASTMAP)
- ubi_err("Attach by fastmap failed! " \
+ ubi_err("Attach by fastmap failed! "
"Falling back to attach by scanning.");
+ /* TODO: please, remove this variable altogether, it is not
+ * needed and it is a hack which you use to tell 'scan_peb()'
+ * to handle fastmap volumes specially. Make this is a clean
+ * way instead: after the scanning, go through the fastmap
+ * volumes (if any was found) and delete them or do whatever
+ * you need. Do not inject hacks to the scanning code. */
ubi->attached_by_scanning = 1;
ai = scan_all(ubi);
if (IS_ERR(ai))
@@ -1237,6 +1251,12 @@ int ubi_attach(struct ubi_device *ubi)
} else if (err < 0)
return err;
+ /* TODO: When you create an image with ubinize - you do not know the
+ * amount of PEBs. So you need to initialize this field with '-1' at
+ * ubinize time. And here you need to check for -1 and initialize it if
+ * needed. Then store it at fastmap. This special value has to be also
+ * documented at ubi-media.h. You also have to amend 'nused' etc.
+ * Probably this can be done later. */
ubi->bad_peb_count = ai->bad_peb_count;
ubi->good_peb_count = ubi->peb_count - ubi->bad_peb_count;
ubi->corr_peb_count = ai->corr_peb_count;
@@ -1244,6 +1264,10 @@ int ubi_attach(struct ubi_device *ubi)
ubi->mean_ec = ai->mean_ec;
ubi_msg("max. sequence number: %llu", ai->max_sqnum);
+ /* TODO: If you support fastmap but it was not found, you need to check
+ * here that ai does not contain fastmap volumes. If it was corrupted,
+ * you need to delete fastmap volumes or possible leftovers of them.
+ * And then you have to create _new_ fastmap */
err = ubi_read_volume_table(ubi, ai);
if (err)
goto out_ai;
@@ -1257,6 +1281,14 @@ int ubi_attach(struct ubi_device *ubi)
goto out_wl;
ubi_destroy_ai(ai);
+
+ /* TODO: UBI auto formats the flash if it is empty (see ubi->is_empty).
+ * It is currently done so that every sub-system writes initializes its
+ * own stuff. Well, now it is only the vtbl sub-system - it creates
+ * empty volume table. And this is why we have "early" function for
+ * getting free PEBs. Fastmap should do the same - so I guess it is
+ * good to do it somewhere here. Also, we need to re-create the fastmap
+ * on-flash data-structures if they were corrupted. */
return 0;
out_wl:
@@ -595,6 +595,8 @@ fail:
* ubi_find_fastmap - searches the first UBI_FM_MAX_START PEBs for the
* fastmap super block.
* @ubi: UBI device object
+ *
+ * TODO: clearly document return codes
*/
static int ubi_find_fastmap(struct ubi_device *ubi, int *fm_start)
{
@@ -614,6 +616,11 @@ static int ubi_find_fastmap(struct ubi_device *ubi, int *fm_start)
if (be32_to_cpu(vhdr->vol_id) == UBI_FM_SB_VOLUME_ID) {
*fm_start = i;
+ /* TODO: fix globally: all UBI prints:
+ * a) do not need the '\n' at the end
+ * b) start with a small letter, because the macros
+ * add several prefixes.
+ */
dbg_bld("Found fastmap super block at PEB %i\n", i);
ret = 0;
@@ -623,6 +630,23 @@ static int ubi_find_fastmap(struct ubi_device *ubi, int *fm_start)
ubi_free_vid_hdr(ubi, vhdr);
+ /* TODO: we can return:
+ * < 0 - error
+ * 0 - fastmap found
+ * 0 - also if we did not find fastmap and the last
+ * 'ubi_io_read_vid_hdr()' call returned 0. This was not
+ * tested with volumes which do not contain fastmap? We need to
+ * test this - we need to have a testcase for this in mtd-utils. I
+ * can create a branch there and we should agree on which tests are
+ * run before anything goes into the git repo.
+ * > 0 - ignore?
+ *
+ * Please, make it instead:
+ * return 0 - no errors
+ * return < 0 - error
+ * If FM not found, fm_start = -1
+ * If FM found, fm_start is set properly
+ */
return ret;
}
@@ -650,6 +674,13 @@ int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info **ai)
goto out;
}
+ /* TODO: then this will be:
+ * if (ret)
+ * return ret;
+ * if (sb_pnum == -1)
+ * return UBI_NO_FASTMAP;
+ *
+ * Much better, I think */
fmsb = kmalloc(sizeof(*fmsb), GFP_KERNEL);
if (!fmsb) {
@@ -661,6 +692,11 @@ int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info **ai)
ret = ubi_io_read(ubi, fmsb, sb_pnum, ubi->leb_start, sizeof(*fmsb));
if (ret) {
ubi_err("Unable to read fastmap super block");
+ /* TODO: please, read what 'ubi_io_read()' returns.
+ * This code is incorrect. All return codes are carefully
+ * documented there. And do it globally, I see you ignore the
+ * bitflips error code and treat it as error everywhere.
+ * Please, start testing with UBI bit-flips emulation enabled */
if (ret > 0)
ret = UBI_BAD_FASTMAP;
@@ -389,6 +389,7 @@ struct ubi_vtbl_record {
#define UBI_FM_POOL_MAGIC 0x67AF4D08
#define UBI_FM_EBA_MAGIC 0xf0c040a8
+/* TODO: would be great to have short coments for the constants */
#define UBI_FM_MAX_START 64
#define UBI_FM_MAX_BLOCKS 32
#define UBI_FM_MIN_POOL_SIZE 8
@@ -426,6 +427,9 @@ struct ubi_fm_sb {
*/
struct ubi_fm_hdr {
__be32 magic;
+ /* TODO: would you please name these fields using the same names UBI
+ * uses in the in-RAM data structures (bad_peb_count, good_peb_count,
+ * etc.) See struct ubi_device. */
__be32 nfree;
__be32 nused;
__be32 nvol;