[PATCH 03/11] cmd: ubi: unbind the block device when detaching UBI

[Daniel Golle]

The ubi_blk device is parented to the MTD device UBI was attached to,
but ubi_detach() leaves it bound. After 'ubi part A; ubi detach; ubi
part B' the device still hangs off A's MTD while UBI sits on B, so the
driver-model tree describes a parent that is no longer in use and A's
MTD can no longer be removed. Reads happen to keep working only because
get_ubi_device() always returns ubi_devices[0].

Return the new device from ubi_bind(), remember it, and unbind it in
ubi_detach() so the next attach binds a fresh one under the right
parent. Tracking the device also replaces the scan of every UCLASS_BLK
device that was used to find it.

Fixes: dec405d1653a ("cmd: ubi: create a ubi_blk device when attaching UBI")
Signed-off-by: Daniel Golle <daniel at makrotopia.org>
---
 cmd/ubi.c               | 73 +++++++++++++++++++++++++----------------
 drivers/mtd/ubi/block.c |  4 ++-
 include/ubi_uboot.h     |  4 +--
 3 files changed, 49 insertions(+), 32 deletions(-)

diff --git a/cmd/ubi.c b/cmd/ubi.c
index 7cbb3ee8668..ca8184aa736 100644
--- a/cmd/ubi.c
+++ b/cmd/ubi.c
@@ -21,6 +21,7 @@
 #include <nand.h>
 #include <onenand_uboot.h>
 #include <dm/device.h>
+#include <dm/device-internal.h>
 #include <dm/devres.h>
 #include <dm/root.h>
 #include <dm/uclass-internal.h>
@@ -650,6 +651,47 @@ static int ubi_set_skip_check(const char *volume, bool skip_check)
 	return ubi_change_vtbl_record(ubi, vol->vol_id, &vtbl_rec);
 }
 
+#if CONFIG_IS_ENABLED(UBI_BLOCK)
+static struct udevice *ubi_blk_dev;
+
+static void ubi_blk_bind_once(void)
+{
+	struct udevice *parent;
+	int ret;
+
+	/*
+	 * A single ubi_blk device serves all volumes of the attached UBI
+	 * device, the volume being selected through the block descriptor's
+	 * hwpart. It is parented to the MTD device UBI sits on, so it must
+	 * not outlive the attach that created it.
+	 */
+	if (ubi_blk_dev)
+		return;
+
+	parent = ubi && ubi->mtd && ubi->mtd->dev ? ubi->mtd->dev : dm_root();
+
+	ret = ubi_bind(parent, &ubi_blk_dev);
+	if (ret)
+		printf("Cannot create UBI block device: %d\n", ret);
+}
+
+static void ubi_blk_unbind(void)
+{
+	if (!ubi_blk_dev)
+		return;
+
+	device_remove(ubi_blk_dev, DM_REMOVE_NORMAL);
+	device_unbind(ubi_blk_dev);
+	ubi_blk_dev = NULL;
+}
+#else
+static inline void ubi_blk_bind_once(void) { }
+
+static inline void ubi_blk_unbind(void)
+{
+}
+#endif
+
 int ubi_detach(void)
 {
 #ifdef CONFIG_CMD_UBIFS
@@ -662,6 +704,8 @@ int ubi_detach(void)
 		cmd_ubifs_umount();
 #endif
 
+	ubi_blk_unbind();
+
 	/*
 	 * Call ubi_exit() before re-initializing the UBI subsystem
 	 */
@@ -673,35 +717,6 @@ int ubi_detach(void)
 	return 0;
 }
 
-#if CONFIG_IS_ENABLED(UBI_BLOCK)
-static void ubi_blk_bind_once(void)
-{
-	struct udevice *parent;
-	struct udevice *dev;
-	int ret;
-
-	/*
-	 * A single ubi_blk device serves all volumes of the attached UBI
-	 * device, the volume being selected through the block descriptor's
-	 * hwpart. Bind one when UBI is attached -- unless a previous attach
-	 * already did -- parented to the MTD device UBI sits on.
-	 */
-	for (uclass_find_first_device(UCLASS_BLK, &dev); dev;
-	     uclass_find_next_device(&dev)) {
-		if (dev->driver == DM_DRIVER_GET(ubi_blk))
-			return;
-	}
-
-	parent = ubi && ubi->mtd && ubi->mtd->dev ? ubi->mtd->dev : dm_root();
-
-	ret = ubi_bind(parent);
-	if (ret)
-		printf("Cannot create UBI block device: %d\n", ret);
-}
-#else
-static inline void ubi_blk_bind_once(void) { }
-#endif
-
 int ubi_part(const char *part_name, const char *vid_header_offset)
 {
 	struct mtd_info *mtd;
diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index 99d55282cd7..f5a79e4f475 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -11,7 +11,7 @@
 #include <dm/device.h>
 #include <dm/device-internal.h>
 
-int ubi_bind(struct udevice *dev)
+int ubi_bind(struct udevice *dev, struct udevice **bdevp)
 {
 	struct blk_desc *bdesc;
 	struct udevice *bdev;
@@ -29,6 +29,8 @@ int ubi_bind(struct udevice *dev)
 	bdesc->bdev = bdev;
 	bdesc->part_type = PART_TYPE_UBI;
 
+	*bdevp = bdev;
+
 	return 0;
 }
 
diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
index bdf5645de3d..9faecc3870c 100644
--- a/include/ubi_uboot.h
+++ b/include/ubi_uboot.h
@@ -148,9 +148,9 @@ int cmd_ubifs_mount(const char *vol_name);
 int cmd_ubifs_umount(void);
 
 #if IS_ENABLED(CONFIG_UBI_BLOCK)
-int ubi_bind(struct udevice *dev);
+int ubi_bind(struct udevice *dev, struct udevice **bdevp);
 #else
-static inline int ubi_bind(struct udevice *dev)
+static inline int ubi_bind(struct udevice *dev, struct udevice **bdevp)
 {
 	return -EOPNOTSUPP;
 }
-- 
2.55.0