mtd: Don't stop MTD partition creation when it fails on one device

MTD partition creation code is a bit tricky. It tries to figure out
when things have changed (either MTD dev list or mtdparts/mtdids vars)
and when that happens it first deletes all the partitions that had been
previously created and then creates the new ones based on the new
mtdparts/mtdids values.
But before deleting the old partitions, it ensures that none of the
currently registered parts are being used and bails out when that's
not the case. So, we end up in a situation where, if at least one MTD
dev has one of its partitions used by someone (UBI for instance), the
partitions update logic no longer works for other devs.

Rework the code to relax the logic and allow updates of MTD parts on
devices that are not being used (we still refuse to updates parts on
devices who have at least one of their partitions used by someone).

Fixes: 5db66b3aee6f ("cmd: mtd: add 'mtd' command")
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
Tested-by: Heiko Schocher <hs@denx.de>

diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index 6a36948b917b837f264215c3b0017c1dee2975bf..d638f700d041e32757f1e3dea3a005085cfd2a12 100644 (file)
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -149,6 +149,54 @@ static const char *get_mtdparts(void)
        return mtdparts;
 }
 
+static int mtd_del_parts(struct mtd_info *mtd, bool quiet)
+{
+       int ret;
+
+       if (!mtd_has_partitions(mtd))
+               return 0;
+
+       /* do not delete partitions if they are in use. */
+       if (mtd_partitions_used(mtd)) {
+               if (!quiet)
+                       printf("\"%s\" partitions still in use, can't delete them\n",
+                              mtd->name);
+               return -EACCES;
+       }
+
+       ret = del_mtd_partitions(mtd);
+       if (ret)
+               return ret;
+
+       return 1;
+}
+
+static bool mtd_del_all_parts_failed;
+
+static void mtd_del_all_parts(void)
+{
+       struct mtd_info *mtd;
+       int ret = 0;
+
+       mtd_del_all_parts_failed = false;
+
+       /*
+        * It is not safe to remove entries from the mtd_for_each_device loop
+        * as it uses idr indexes and the partitions removal is done in bulk
+        * (all partitions of one device at the same time), so break and
+        * iterate from start each time a new partition is found and deleted.
+        */
+       do {
+               mtd_for_each_device(mtd) {
+                       ret = mtd_del_parts(mtd, false);
+                       if (ret > 0)
+                               break;
+                       else if (ret < 0)
+                               mtd_del_all_parts_failed = true;
+               }
+       } while (ret > 0);
+}
+
 int mtd_probe_devices(void)
 {
        static char *old_mtdparts;
@@ -156,18 +204,19 @@ int mtd_probe_devices(void)
        const char *mtdparts = get_mtdparts();
        const char *mtdids = get_mtdids();
        const char *mtdparts_next = mtdparts;
-       bool remaining_partitions = true;
        struct mtd_info *mtd;
 
        mtd_probe_uclass_mtd_devs();
 
        /*
-        * Check if mtdparts/mtdids changed or if the MTD dev list was updated
-        * since last call, otherwise: exit
+        * Check if mtdparts/mtdids changed, if the MTD dev list was updated
+        * or if our previous attempt to delete existing partititions failed.
+        * In any of these cases we want to update the partitions, otherwise,
+        * everything is up-to-date and we can return 0 directly.
         */
        if ((!mtdparts && !old_mtdparts && !mtdids && !old_mtdids) ||
            (mtdparts && old_mtdparts && mtdids && old_mtdids &&
-            !mtd_dev_list_updated() &&
+            !mtd_dev_list_updated() && !mtd_del_all_parts_failed &&
             !strcmp(mtdparts, old_mtdparts) &&
             !strcmp(mtdids, old_mtdids)))
                return 0;
@@ -178,32 +227,12 @@ int mtd_probe_devices(void)
        old_mtdparts = strdup(mtdparts);
        old_mtdids = strdup(mtdids);
 
-       /* If at least one partition is still in use, do not delete anything */
-       mtd_for_each_device(mtd) {
-               if (mtd->usecount) {
-                       printf("Partition \"%s\" already in use, aborting\n",
-                              mtd->name);
-                       return -EACCES;
-               }
-       }
-
        /*
-        * Everything looks clear, remove all partitions. It is not safe to
-        * remove entries from the mtd_for_each_device loop as it uses idr
-        * indexes and the partitions removal is done in bulk (all partitions of
-        * one device at the same time), so break and iterate from start each
-        * time a new partition is found and deleted.
+        * Remove all old parts. Note that partition removal can fail in case
+        * one of the partition is still being used by an MTD user, so this
+        * does not guarantee that all old partitions are gone.
         */
-       while (remaining_partitions) {
-               remaining_partitions = false;
-               mtd_for_each_device(mtd) {
-                       if (!mtd_is_partition(mtd) && mtd_has_partitions(mtd)) {
-                               del_mtd_partitions(mtd);
-                               remaining_partitions = true;
-                               break;
-                       }
-               }
-       }
+       mtd_del_all_parts();
 
        /*
         * Call mtd_dev_list_updated() to clear updates generated by our own
@@ -278,6 +307,17 @@ int mtd_probe_devices(void)
                        }
                }
 
+               /*
+                * Call mtd_del_parts() again, even if it's already been called
+                * in mtd_del_all_parts(). We need to know if old partitions are
+                * still around (because they are still being used by someone),
+                * and if they are, we shouldn't create new partitions, so just
+                * skip this MTD device and try the next one.
+                */
+               ret = mtd_del_parts(mtd, true);
+               if (ret < 0)
+                       continue;
+
                /*
                 * Parse the MTD device partitions. It will update the mtdparts
                 * pointer, create an array of parts (that must be freed), and

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 4d2ac8107f01c564cf9165454f1e6203b99b06a3..fd8d8e5ea7295ac44c2bb25677d2e3df06f6d1cc 100644 (file)
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -63,6 +63,18 @@ char *kstrdup(const char *s, gfp_t gfp)
 #define MTD_SIZE_REMAINING             (~0LLU)
 #define MTD_OFFSET_NOT_SPECIFIED       (~0LLU)
 
+bool mtd_partitions_used(struct mtd_info *master)
+{
+       struct mtd_info *slave;
+
+       list_for_each_entry(slave, &master->partitions, node) {
+               if (slave->usecount)
+                       return true;
+       }
+
+       return false;
+}
+
 /**
  * mtd_parse_partition - Parse @mtdparts partition definition, fill @partition
  *                       with it and update the @mtdparts string pointer.

diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 4d0096d9f1d881025297a705897d56a04d4cb043..cd1f557a2f3151834f8a8211804c0623402cc78e 100644 (file)
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -366,6 +366,8 @@ static inline bool mtd_has_partitions(const struct mtd_info *mtd)
        return !list_empty(&mtd->partitions);
 }
 
+bool mtd_partitions_used(struct mtd_info *master);
+
 int mtd_ooblayout_ecc(struct mtd_info *mtd, int section,
                      struct mtd_oob_region *oobecc);
 int mtd_ooblayout_find_eccregion(struct mtd_info *mtd, int eccbyte,