[PATCH v5 0/9] dm: core: Add support for vital devices

[PATCH v5 0/9] dm: core: Add support for vital devices

[Simon Glass]

This is an attempt to come up with a 'late removal' feature, as
implemented by Marek, but using different terminology and trying to fit in
with the existing removal flags.

I have taken the opportunity to clean up a few related things.

The series is available at u-boot-dm/late-working

The previous patch was here:

   http://patchwork.ozlabs.org/project/uboot/patch/20201108140818.55172-1-marek.vasut+renesas at gmail.com/

Changes in v5:
- Change logic to allow flags to be combined
- Add patch to remove vital devices last

Changes in v4:
- Use 'vital' rather than 'late' as the description
- Invert the removal flag to DM_REMOVE_NON_VITAL, to avoid the need for
        two-pass processing in the bowels of driver model
- Simplify the test to only test 'vital'
- Drop the change to uclass_destroy()
- Revised and updated based on discussion

Marek Vasut (1):
  dm: core: Add late driver remove option

Simon Glass (8):
  smem: Don't use -EPROBE_DEFER
  nand: brcmnand: Don't use -EPROBE_DEFER
  dm: Rename DM_FLAG_REMOVE_WITH_PD_ON
  dm: pci: Correct use of wrong flag name
  dm: core: Remove children before advising uclass
  dm: core: Avoid partially removing devices
  arm: Remove vital devices last
  dm: core: Add documentation about device removal

 arch/arm/lib/bootm.c                     |  3 +
 doc/driver-model/design.rst              | 20 +++++
 drivers/core/device-remove.c             | 96 +++++++++++++++++-------
 drivers/core/root.c                      |  2 +
 drivers/mtd/nand/raw/brcmnand/brcmnand.c |  5 +-
 drivers/pci/pcie_iproc.c                 |  2 +-
 drivers/smem/msm_smem.c                  |  6 +-
 drivers/video/meson/meson_vpu.c          |  2 +-
 drivers/watchdog/rti_wdt.c               |  2 +-
 include/dm/device-internal.h             | 15 +++-
 include/dm/device.h                      | 15 +++-
 test/dm/core.c                           | 94 +++++++++++++++++++++++
 test/dm/test-driver.c                    | 22 ++++++
 test/dm/virtio.c                         |  4 +-
 14 files changed, 244 insertions(+), 44 deletions(-)

-- 
2.30.0.280.ga3ce27912f-goog

[PATCH v5 1/9] smem: Don't use -EPROBE_DEFER

[Simon Glass]

This has no useful meaning in U-Boot. Use -ENOMEM since that appears to
be what has gone wrong in this case. We want to reserve this flag for
internal driver model use.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 drivers/smem/msm_smem.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/smem/msm_smem.c b/drivers/smem/msm_smem.c
index 597d425d11f..26462151b3a 100644
--- a/drivers/smem/msm_smem.c
+++ b/drivers/smem/msm_smem.c
@@ -437,7 +437,7 @@ static int qcom_smem_alloc(unsigned int host, unsigned int item, size_t size)
 	int ret;
 
 	if (!__smem)
-		return -EPROBE_DEFER;
+		return -ENOMEM;
 
 	if (item < SMEM_ITEM_LAST_FIXED) {
 		dev_err(__smem->dev,
@@ -559,7 +559,7 @@ static void *qcom_smem_get(unsigned int host, unsigned int item, size_t *size)
 {
 	struct smem_partition_header *phdr;
 	size_t cacheln;
-	void *ptr = ERR_PTR(-EPROBE_DEFER);
+	void *ptr = ERR_PTR(-ENOMEM);
 
 	if (!__smem)
 		return ptr;
@@ -597,7 +597,7 @@ static int qcom_smem_get_free_space(unsigned int host)
 	unsigned int ret;
 
 	if (!__smem)
-		return -EPROBE_DEFER;
+		return -ENOMEM;
 
 	if (host < SMEM_HOST_COUNT && __smem->partitions[host]) {
 		phdr = __smem->partitions[host];
-- 
2.30.0.280.ga3ce27912f-goog

[PATCH v5 2/9] nand: brcmnand: Don't use -EPROBE_DEFER

[Simon Glass]

This has no useful meaning in U-Boot and will never be returned. We want
to reserve this flag for internal driver model use.

Drop the code.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 7349a9bc99e..99a1c2e6e2e 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -2526,10 +2526,7 @@ int brcmnand_probe(struct udevice *dev, struct brcmnand_soc *soc)
 		if (ret)
 			return ret;
 	} else {
-		ret = PTR_ERR(ctrl->clk);
-		if (ret == -EPROBE_DEFER)
-			return ret;
-
+		/* Ignore PTR_ERR(ctrl->clk) */
 		ctrl->clk = NULL;
 	}
 
-- 
2.30.0.280.ga3ce27912f-goog

[PATCH v5 3/9] dm: Rename DM_FLAG_REMOVE_WITH_PD_ON

[Simon Glass]

This flag has the word 'REMOVE' in it which means it conflicts with
the DM_REMOVE flags. Rename it to DM_FLAG_LEAVE_PD_ON which seems to
indicate its purpose well enough.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 drivers/core/device-remove.c    | 2 +-
 drivers/video/meson/meson_vpu.c | 2 +-
 drivers/watchdog/rti_wdt.c      | 2 +-
 include/dm/device.h             | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index 7e8f3afb2d6..7874d53c843 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -200,7 +200,7 @@ int device_remove(struct udevice *dev, uint flags)
 
 	if (!(flags & DM_REMOVE_NO_PD) &&
 	    !(drv->flags &
-	      (DM_FLAG_DEFAULT_PD_CTRL_OFF | DM_FLAG_REMOVE_WITH_PD_ON)) &&
+	      (DM_FLAG_DEFAULT_PD_CTRL_OFF | DM_FLAG_LEAVE_PD_ON)) &&
 	    dev != gd->cur_serial_dev)
 		dev_power_domain_off(dev);
 
diff --git a/drivers/video/meson/meson_vpu.c b/drivers/video/meson/meson_vpu.c
index ca6933a6c54..558f9ba766c 100644
--- a/drivers/video/meson/meson_vpu.c
+++ b/drivers/video/meson/meson_vpu.c
@@ -212,5 +212,5 @@ U_BOOT_DRIVER(meson_vpu) = {
 	.probe = meson_vpu_probe,
 	.bind = meson_vpu_bind,
 	.priv_auto	= sizeof(struct meson_vpu_priv),
-	.flags  = DM_FLAG_PRE_RELOC | DM_FLAG_REMOVE_WITH_PD_ON,
+	.flags  = DM_FLAG_PRE_RELOC | DM_FLAG_LEAVE_PD_ON,
 };
diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index f64a39f6579..8335b20ae84 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -119,5 +119,5 @@ U_BOOT_DRIVER(rti_wdt) = {
 	.ops = &rti_wdt_ops,
 	.probe = rti_wdt_probe,
 	.priv_auto	= sizeof(struct rti_wdt_priv),
-	.flags = DM_FLAG_REMOVE_WITH_PD_ON,
+	.flags = DM_FLAG_LEAVE_PD_ON,
 };
diff --git a/include/dm/device.h b/include/dm/device.h
index f5b4cd6876e..a52bbdc3a63 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -71,7 +71,7 @@ struct driver_info;
  * Device is removed without switching off its power domain. This might
  * be required, i. e. for serial console (debug) output when booting OS.
  */
-#define DM_FLAG_REMOVE_WITH_PD_ON	(1 << 13)
+#define DM_FLAG_LEAVE_PD_ON		(1 << 13)
 
 /*
  * One or multiple of these flags are passed to device_remove() so that
-- 
2.30.0.280.ga3ce27912f-goog

[PATCH v5 4/9] dm: pci: Correct use of wrong flag name

[Simon Glass]

Update a driver that uses the incorrect flag. Add a comment to hopefully
prevent furture mistakes.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 drivers/pci/pcie_iproc.c | 2 +-
 include/dm/device.h      | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pcie_iproc.c b/drivers/pci/pcie_iproc.c
index 6725ff64372..12ce9d525ca 100644
--- a/drivers/pci/pcie_iproc.c
+++ b/drivers/pci/pcie_iproc.c
@@ -1283,5 +1283,5 @@ U_BOOT_DRIVER(pci_iproc) = {
 	.probe = iproc_pcie_probe,
 	.remove = iproc_pcie_remove,
 	.priv_auto	= sizeof(struct iproc_pcie),
-	.flags = DM_REMOVE_OS_PREPARE,
+	.flags = DM_FLAG_OS_PREPARE,
 };
diff --git a/include/dm/device.h b/include/dm/device.h
index a52bbdc3a63..2554f679277 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -77,6 +77,9 @@ struct driver_info;
  * One or multiple of these flags are passed to device_remove() so that
  * a selective device removal as specified by the remove-stage and the
  * driver flags can be done.
+ *
+ * DO NOT use these flags in your driver's @flags value...
+ *	use the above DM_FLAG_... values instead
  */
 enum {
 	/* Normal remove, remove all devices */
-- 
2.30.0.280.ga3ce27912f-goog

[PATCH v5 5/9] dm: core: Remove children before advising uclass

[Simon Glass]

At present the uclass pre-remove method is called before the children are
removed. But the children may refused to be removed, in whch case the
uclass is in a tricky situation. At present we handle this by calling
the uclass' post_probe() method. But it seems better to avoid doing
anything with the uclass in this case.

Switch the ordering so that we make sure the children can be removed
before advising the uclass.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 drivers/core/device-remove.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index 7874d53c843..35b625e7b1f 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -172,13 +172,13 @@ int device_remove(struct udevice *dev, uint flags)
 	drv = dev->driver;
 	assert(drv);
 
-	ret = uclass_pre_remove_device(dev);
+	ret = device_chld_remove(dev, NULL, flags);
 	if (ret)
 		return ret;
 
-	ret = device_chld_remove(dev, NULL, flags);
+	ret = uclass_pre_remove_device(dev);
 	if (ret)
-		goto err;
+		return ret;
 
 	/*
 	 * Remove the device if called with the "normal" remove flag set,
@@ -216,12 +216,6 @@ err_remove:
 	/* We can't put the children back */
 	dm_warn("%s: Device '%s' failed to remove, but children are gone\n",
 		__func__, dev->name);
-err:
-	ret = uclass_post_probe_device(dev);
-	if (ret) {
-		dm_warn("%s: Device '%s' failed to post_probe on error path\n",
-			__func__, dev->name);
-	}
 
 	return ret;
 }
-- 
2.30.0.280.ga3ce27912f-goog

[PATCH v5 6/9] dm: core: Avoid partially removing devices

[Simon Glass]

At present if device_remove() decides that the device should not actually
be removed, it still calls the uclass pre_remove() method and powers the
device down.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 drivers/core/device-remove.c | 59 ++++++++++++++++++++++++------------
 include/dm/device-internal.h |  9 +++++-
 test/dm/virtio.c             |  4 ++-
 3 files changed, 51 insertions(+), 21 deletions(-)

diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index 35b625e7b1f..bc99ef0032f 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -8,6 +8,8 @@
  * Pavel Herrmann <morpheus.ibis@gmail.com>
  */
 
+#define LOG_CATEGORY	LOGC_DM
+
 #include <common.h>
 #include <errno.h>
 #include <log.h>
@@ -54,7 +56,7 @@ int device_chld_remove(struct udevice *dev, struct driver *drv,
 			continue;
 
 		ret = device_remove(pos, flags);
-		if (ret)
+		if (ret && ret != -EKEYREJECTED)
 			return ret;
 	}
 
@@ -149,13 +151,24 @@ void device_free(struct udevice *dev)
 	devres_release_probe(dev);
 }
 
-static bool flags_remove(uint flags, uint drv_flags)
+/**
+ * flags_remove() - Figure out whether to remove a device
+ *
+ * @flags: Flags passed to device_remove()
+ * @drv_flags: Driver flags
+ * @return 0 if the device should be removed,
+ * -EKEYREJECTED if @flags includes a flag in DM_REMOVE_ACTIVE_ALL but
+ *	@drv_flags does not (indicates that this device has nothing to do for
+ *	DMA shutdown or OS prepare)
+ */
+static int flags_remove(uint flags, uint drv_flags)
 {
-	if ((flags & DM_REMOVE_NORMAL) ||
-	    (flags && (drv_flags & (DM_FLAG_ACTIVE_DMA | DM_FLAG_OS_PREPARE))))
-		return true;
+	if (flags & DM_REMOVE_NORMAL)
+		return 0;
+	if (flags && (drv_flags & DM_REMOVE_ACTIVE_ALL))
+		return 0;
 
-	return false;
+	return -EKEYREJECTED;
 }
 
 int device_remove(struct udevice *dev, uint flags)
@@ -169,22 +182,32 @@ int device_remove(struct udevice *dev, uint flags)
 	if (!(dev_get_flags(dev) & DM_FLAG_ACTIVATED))
 		return 0;
 
+	/*
+	 * If the child returns EKEYREJECTED, continue. It just means that it
+	 * didn't match the flags.
+	 */
+	ret = device_chld_remove(dev, NULL, flags);
+	if (ret && ret != -EKEYREJECTED)
+		return ret;
+
+	/*
+	 * Remove the device if called with the "normal" remove flag set,
+	 * or if the remove flag matches any of the drivers remove flags
+	 */
 	drv = dev->driver;
 	assert(drv);
-
-	ret = device_chld_remove(dev, NULL, flags);
-	if (ret)
+	ret = flags_remove(flags, drv->flags);
+	if (ret) {
+		log_debug("%s: When removing: flags=%x, drv->flags=%x, err=%d\n",
+			  dev->name, flags, drv->flags, ret);
 		return ret;
+	}
 
 	ret = uclass_pre_remove_device(dev);
 	if (ret)
 		return ret;
 
-	/*
-	 * Remove the device if called with the "normal" remove flag set,
-	 * or if the remove flag matches any of the drivers remove flags
-	 */
-	if (drv->remove && flags_remove(flags, drv->flags)) {
+	if (drv->remove) {
 		ret = drv->remove(dev);
 		if (ret)
 			goto err_remove;
@@ -204,13 +227,11 @@ int device_remove(struct udevice *dev, uint flags)
 	    dev != gd->cur_serial_dev)
 		dev_power_domain_off(dev);
 
-	if (flags_remove(flags, drv->flags)) {
-		device_free(dev);
+	device_free(dev);
 
-		dev_bic_flags(dev, DM_FLAG_ACTIVATED);
-	}
+	dev_bic_flags(dev, DM_FLAG_ACTIVATED);
 
-	return ret;
+	return 0;
 
 err_remove:
 	/* We can't put the children back */
diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
index 639bbd293d9..b513b6861a6 100644
--- a/include/dm/device-internal.h
+++ b/include/dm/device-internal.h
@@ -123,7 +123,8 @@ int device_probe(struct udevice *dev);
  *
  * @dev: Pointer to device to remove
  * @flags: Flags for selective device removal (DM_REMOVE_...)
- * @return 0 if OK, -ve on error (an error here is normally a very bad thing)
+ * @return 0 if OK, -EKEYREJECTED if not removed due to flags, other -ve on
+ *	error (such an error here is normally a very bad thing)
  */
 #if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE)
 int device_remove(struct udevice *dev, uint flags);
@@ -173,6 +174,12 @@ static inline int device_chld_unbind(struct udevice *dev, struct driver *drv)
 
 /**
  * device_chld_remove() - Stop all device's children
+ *
+ * This continues through all children recursively stopping part-way through if
+ * an error occurs. Return values of -EKEYREJECTED are ignored and processing
+ * continues, since they just indicate that the child did not elect to be
+ * removed based on the value of @flags.
+ *
  * @dev:	The device whose children are to be removed
  * @drv:	The targeted driver
  * @flags:	Flag, if this functions is called in the pre-OS stage
diff --git a/test/dm/virtio.c b/test/dm/virtio.c
index ad355981cf4..9a7e658cceb 100644
--- a/test/dm/virtio.c
+++ b/test/dm/virtio.c
@@ -123,7 +123,9 @@ static int dm_test_virtio_remove(struct unit_test_state *uts)
 
 	/* check the device can be successfully removed */
 	dev_or_flags(dev, DM_FLAG_ACTIVATED);
-	ut_assertok(device_remove(bus, DM_REMOVE_ACTIVE_ALL));
+	ut_asserteq(-EKEYREJECTED, device_remove(bus, DM_REMOVE_ACTIVE_ALL));
+
+	ut_asserteq(false, device_active(dev));
 
 	return 0;
 }
-- 
2.30.0.280.ga3ce27912f-goog

[PATCH v5 7/9] dm: core: Add late driver remove option

[Simon Glass]

From: Marek Vasut <marek.vasut@gmail.com>

Add another flag to the DM core which could be assigned to drivers and
which makes those drivers call their remove callbacks last, just before
booting OS and after all the other drivers finished with their remove
callbacks. This is necessary for things like clock drivers, where the
other drivers might depend on the clock driver in their remove callbacks.
Prime example is the mmc subsystem, which can reconfigure a card from HS
mode to slower modes in the remove callback and for that it needs to
reconfigure the controller clock.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5:
- Change logic to allow flags to be combined

Changes in v4:
- Use 'vital' rather than 'late' as the description
- Invert the removal flag to DM_REMOVE_NON_VITAL, to avoid the need for
        two-pass processing in the bowels of driver model
- Simplify the test to only test 'vital'
- Drop the change to uclass_destroy()

 drivers/core/device-remove.c | 39 ++++++++++++---
 drivers/core/root.c          |  2 +
 include/dm/device-internal.h | 10 ++--
 include/dm/device.h          | 10 +++-
 test/dm/core.c               | 94 ++++++++++++++++++++++++++++++++++++
 test/dm/test-driver.c        | 22 +++++++++
 6 files changed, 165 insertions(+), 12 deletions(-)

diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index bc99ef0032f..616dcf07859 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -47,20 +47,24 @@ int device_chld_remove(struct udevice *dev, struct driver *drv,
 		       uint flags)
 {
 	struct udevice *pos, *n;
-	int ret;
+	int result = 0;
 
 	assert(dev);
 
 	list_for_each_entry_safe(pos, n, &dev->child_head, sibling_node) {
+		int ret;
+
 		if (drv && (pos->driver != drv))
 			continue;
 
 		ret = device_remove(pos, flags);
-		if (ret && ret != -EKEYREJECTED)
+		if (ret == -EPROBE_DEFER)
+			result = ret;
+		else if (ret && ret != -EKEYREJECTED)
 			return ret;
 	}
 
-	return 0;
+	return result;
 }
 
 int device_unbind(struct udevice *dev)
@@ -154,21 +158,40 @@ void device_free(struct udevice *dev)
 /**
  * flags_remove() - Figure out whether to remove a device
  *
+ * If this is called with @flags == DM_REMOVE_NON_VITAL | DM_REMOVE_ACTIVE_DMA,
+ * then it returns 0 (=go head and remove) if the device is not matked vital
+ * but is marked DM_REMOVE_ACTIVE_DMA.
+ *
+ * If this is called with @flags == DM_REMOVE_ACTIVE_DMA,
+ * then it returns 0 (=go head and remove) if the device is marked
+ * DM_REMOVE_ACTIVE_DMA, regardless of whether it is marked vital.
+ *
  * @flags: Flags passed to device_remove()
  * @drv_flags: Driver flags
  * @return 0 if the device should be removed,
  * -EKEYREJECTED if @flags includes a flag in DM_REMOVE_ACTIVE_ALL but
  *	@drv_flags does not (indicates that this device has nothing to do for
  *	DMA shutdown or OS prepare)
+ * -EPROBE_DEFER if @flags is DM_REMOVE_NON_VITAL but @drv_flags contains
+ *	DM_FLAG_VITAL (indicates the device is vital and should not be removed)
  */
 static int flags_remove(uint flags, uint drv_flags)
 {
-	if (flags & DM_REMOVE_NORMAL)
-		return 0;
-	if (flags && (drv_flags & DM_REMOVE_ACTIVE_ALL))
-		return 0;
+	if (!(flags & DM_REMOVE_NORMAL)) {
+		bool vital_match;
+		bool active_match;
+
+		active_match = !(flags & DM_REMOVE_ACTIVE_ALL) ||
+			(drv_flags & flags);
+		vital_match = !(flags & DM_REMOVE_NON_VITAL) ||
+			!(drv_flags & DM_FLAG_VITAL);
+		if (!vital_match)
+			return -EPROBE_DEFER;
+		if (!active_match)
+			return -EKEYREJECTED;
+	}
 
-	return -EKEYREJECTED;
+	return 0;
 }
 
 int device_remove(struct udevice *dev, uint flags)
diff --git a/drivers/core/root.c b/drivers/core/root.c
index 2bfa75b4725..7ef2ec2da27 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -162,6 +162,8 @@ int dm_init(bool of_live)
 
 int dm_uninit(void)
 {
+	/* Remove non-vital devices first */
+	device_remove(dm_root(), DM_REMOVE_NON_VITAL);
 	device_remove(dm_root(), DM_REMOVE_NORMAL);
 	device_unbind(dm_root());
 	gd->dm_root = NULL;
diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
index b513b6861a6..39406c3f352 100644
--- a/include/dm/device-internal.h
+++ b/include/dm/device-internal.h
@@ -123,7 +123,8 @@ int device_probe(struct udevice *dev);
  *
  * @dev: Pointer to device to remove
  * @flags: Flags for selective device removal (DM_REMOVE_...)
- * @return 0 if OK, -EKEYREJECTED if not removed due to flags, other -ve on
+ * @return 0 if OK, -EKEYREJECTED if not removed due to flags, -EPROBE_DEFER if
+ *	this is a vital device and flags is DM_REMOVE_NON_VITAL, other -ve on
  *	error (such an error here is normally a very bad thing)
  */
 #if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE)
@@ -178,12 +179,15 @@ static inline int device_chld_unbind(struct udevice *dev, struct driver *drv)
  * This continues through all children recursively stopping part-way through if
  * an error occurs. Return values of -EKEYREJECTED are ignored and processing
  * continues, since they just indicate that the child did not elect to be
- * removed based on the value of @flags.
+ * removed based on the value of @flags. Return values of -EPROBE_DEFER cause
+ * processing of other children to continue, but the function will return
+ * -EPROBE_DEFER.
  *
  * @dev:	The device whose children are to be removed
  * @drv:	The targeted driver
  * @flags:	Flag, if this functions is called in the pre-OS stage
- * @return 0 on success, -ve on error
+ * @return 0 on success, -EPROBE_DEFER if any child failed to remove, other
+ *	-ve on error
  */
 #if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE)
 int device_chld_remove(struct udevice *dev, struct driver *drv,
diff --git a/include/dm/device.h b/include/dm/device.h
index 2554f679277..5f5df770ce1 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -73,6 +73,13 @@ struct driver_info;
  */
 #define DM_FLAG_LEAVE_PD_ON		(1 << 13)
 
+/*
+ * Device is vital to the operation of other devices. It is possible to remove
+ * removed this device after all regular devices are removed. This is useful
+ * e.g. for clock, which need to be active during the device-removal phase.
+ */
+#define DM_FLAG_VITAL			(1 << 14)
+
 /*
  * One or multiple of these flags are passed to device_remove() so that
  * a selective device removal as specified by the remove-stage and the
@@ -91,7 +98,8 @@ enum {
 	/* Remove devices which need some final OS preparation steps */
 	DM_REMOVE_OS_PREPARE	= DM_FLAG_OS_PREPARE,
 
-	/* Add more use cases here */
+	/* Remove only devices that are not marked vital */
+	DM_REMOVE_NON_VITAL	= DM_FLAG_VITAL,
 
 	/* Remove devices with any active flag */
 	DM_REMOVE_ACTIVE_ALL	= DM_REMOVE_ACTIVE_DMA | DM_REMOVE_OS_PREPARE,
diff --git a/test/dm/core.c b/test/dm/core.c
index 1f5ca570dc7..bfd6565d952 100644
--- a/test/dm/core.c
+++ b/test/dm/core.c
@@ -72,6 +72,14 @@ static struct driver_info driver_info_act_dma = {
 	.name = "test_act_dma_drv",
 };
 
+static struct driver_info driver_info_vital_clk = {
+	.name = "test_vital_clk_drv",
+};
+
+static struct driver_info driver_info_act_dma_vital_clk = {
+	.name = "test_act_dma_vital_clk_drv",
+};
+
 void dm_leak_check_start(struct unit_test_state *uts)
 {
 	uts->start = mallinfo();
@@ -883,6 +891,92 @@ static int dm_test_remove_active_dma(struct unit_test_state *uts)
 }
 DM_TEST(dm_test_remove_active_dma, 0);
 
+/* Test removal of 'vital' devices */
+static int dm_test_remove_vital(struct unit_test_state *uts)
+{
+	struct dm_test_state *dms = uts->priv;
+	struct udevice *normal, *dma, *vital, *dma_vital;
+
+	/* Skip the behaviour in test_post_probe() */
+	dms->skip_post_probe = 1;
+
+	ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual,
+					&normal));
+	ut_assertnonnull(normal);
+
+	ut_assertok(device_bind_by_name(dms->root, false, &driver_info_act_dma,
+					&dma));
+	ut_assertnonnull(dma);
+
+	ut_assertok(device_bind_by_name(dms->root, false,
+					&driver_info_vital_clk, &vital));
+	ut_assertnonnull(vital);
+
+	ut_assertok(device_bind_by_name(dms->root, false,
+					&driver_info_act_dma_vital_clk,
+					&dma_vital));
+	ut_assertnonnull(dma_vital);
+
+	/* Probe the devices */
+	ut_assertok(device_probe(normal));
+	ut_assertok(device_probe(dma));
+	ut_assertok(device_probe(vital));
+	ut_assertok(device_probe(dma_vital));
+
+	/* Check that devices are active right now */
+	ut_asserteq(true, device_active(normal));
+	ut_asserteq(true, device_active(dma));
+	ut_asserteq(true, device_active(vital));
+	ut_asserteq(true, device_active(dma_vital));
+
+	/* Remove active devices via selective remove flag */
+	dm_remove_devices_flags(DM_REMOVE_NON_VITAL | DM_REMOVE_ACTIVE_ALL);
+
+	/*
+	 * Check that this only has an effect on the dma device, since two
+	 * devices are vital and the third does not have active DMA
+	 */
+	ut_asserteq(true, device_active(normal));
+	ut_asserteq(false, device_active(dma));
+	ut_asserteq(true, device_active(vital));
+	ut_asserteq(true, device_active(dma_vital));
+
+	/* Remove active devices via selective remove flag */
+	ut_assertok(device_probe(dma));
+	dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL);
+
+	/* This should have affected both active-dma devices */
+	ut_asserteq(true, device_active(normal));
+	ut_asserteq(false, device_active(dma));
+	ut_asserteq(true, device_active(vital));
+	ut_asserteq(false, device_active(dma_vital));
+
+	/* Remove non-vital devices */
+	ut_assertok(device_probe(dma));
+	ut_assertok(device_probe(dma_vital));
+	dm_remove_devices_flags(DM_REMOVE_NON_VITAL);
+
+	/* This should have affected only non-vital devices */
+	ut_asserteq(false, device_active(normal));
+	ut_asserteq(false, device_active(dma));
+	ut_asserteq(true, device_active(vital));
+	ut_asserteq(true, device_active(dma_vital));
+
+	/* Remove vital devices via normal remove flag */
+	ut_assertok(device_probe(normal));
+	ut_assertok(device_probe(dma));
+	dm_remove_devices_flags(DM_REMOVE_NORMAL);
+
+	/* Check that all devices are inactive right now */
+	ut_asserteq(false, device_active(normal));
+	ut_asserteq(false, device_active(dma));
+	ut_asserteq(false, device_active(vital));
+	ut_asserteq(false, device_active(dma_vital));
+
+	return 0;
+}
+DM_TEST(dm_test_remove_vital, 0);
+
 static int dm_test_uclass_before_ready(struct unit_test_state *uts)
 {
 	struct uclass *uc;
diff --git a/test/dm/test-driver.c b/test/dm/test-driver.c
index a67f5d3f982..ca7626a0668 100644
--- a/test/dm/test-driver.c
+++ b/test/dm/test-driver.c
@@ -170,3 +170,25 @@ U_BOOT_DRIVER(test_act_dma_drv) = {
 	.unbind	= test_manual_unbind,
 	.flags	= DM_FLAG_ACTIVE_DMA,
 };
+
+U_BOOT_DRIVER(test_vital_clk_drv) = {
+	.name	= "test_vital_clk_drv",
+	.id	= UCLASS_TEST,
+	.ops	= &test_manual_ops,
+	.bind	= test_manual_bind,
+	.probe	= test_manual_probe,
+	.remove	= test_manual_remove,
+	.unbind	= test_manual_unbind,
+	.flags	= DM_FLAG_VITAL,
+};
+
+U_BOOT_DRIVER(test_act_dma_vital_clk_drv) = {
+	.name	= "test_act_dma_vital_clk_drv",
+	.id	= UCLASS_TEST,
+	.ops	= &test_manual_ops,
+	.bind	= test_manual_bind,
+	.probe	= test_manual_probe,
+	.remove	= test_manual_remove,
+	.unbind	= test_manual_unbind,
+	.flags	= DM_FLAG_VITAL | DM_FLAG_ACTIVE_DMA,
+};
-- 
2.30.0.280.ga3ce27912f-goog

[PATCH v5 8/9] arm: Remove vital devices last

[Simon Glass]

Update announce_and_cleanup() to remove all devices, with the vital ones
being removed last.

This is an extra patch on top of the recent RFC:

   http://patchwork.ozlabs.org/project/uboot/list/?series=223280

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5:
- Add patch to remove vital devices last

 arch/arm/lib/bootm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 1206e306db6..f46d51d6553 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -119,6 +119,9 @@ static void announce_and_cleanup(int fake)
 	 * This may be useful for last-stage operations, like cancelling
 	 * of DMA operation or releasing device internal buffers.
 	 */
+	dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL | DM_REMOVE_NON_VITAL);
+
+	/* Remove all active vital devices next */
 	dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL);
 
 	cleanup_before_linux();
-- 
2.30.0.280.ga3ce27912f-goog

[PATCH v5 9/9] dm: core: Add documentation about device removal

[Simon Glass]

Make mention of this feature in the core documentation so people can
discover it without looking at a header file.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v4)

Changes in v4:
- Revised and updated based on discussion

 doc/driver-model/design.rst | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/doc/driver-model/design.rst b/doc/driver-model/design.rst
index ffed7d5f79a..2417976ab74 100644
--- a/doc/driver-model/design.rst
+++ b/doc/driver-model/design.rst
@@ -880,6 +880,26 @@ If a parent has children these will be destroyed first. After this point
 the device does not exist and its memory has be deallocated.
 
 
+Special cases for removal
+-------------------------
+
+Some devices need to do clean-up before the OS is called. For example, a USB
+driver may want to stop the bus. This can be done in the remove() method.
+Some special flags are used to determine whether to remove the device:
+
+   DM_FLAG_OS_PREPARE - indicates that the device needs to get ready for OS
+          boot. The device will be removed just before the OS is booted
+   DM_REMOVE_ACTIVE_DMA - indicates that the device uses DMA. This is
+          effectively the same as DM_FLAG_OS_PREPARE, so the device is removed
+          before the OS is booted
+   DM_FLAG_VITAL - indicates that the device is 'vital' to the operation of
+          other devices. It is possible to remove this device after all regular
+          devices are removed. This is useful e.g. for a clock, which need to
+          be active during the device-removal phase.
+
+The dm_remove_devices_flags() function can be used to remove devices based on
+their driver flags.
+
 Data Structures
 ---------------
 
-- 
2.30.0.280.ga3ce27912f-goog

[PATCH v5 9/9] dm: core: Add documentation about device removal

[Simon Glass]

Make mention of this feature in the core documentation so people can
discover it without looking at a header file.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v4)

Changes in v4:
- Revised and updated based on discussion

 doc/driver-model/design.rst | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Applied to u-boot-dm, thanks!

[PATCH v5 8/9] arm: Remove vital devices last

[Simon Glass]

Update announce_and_cleanup() to remove all devices, with the vital ones
being removed last.

This is an extra patch on top of the recent RFC:

   http://patchwork.ozlabs.org/project/uboot/list/?series=223280

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5:
- Add patch to remove vital devices last

 arch/arm/lib/bootm.c | 3 +++
 1 file changed, 3 insertions(+)

Applied to u-boot-dm, thanks!

[PATCH v5 7/9] dm: core: Add late driver remove option

[Simon Glass]

From: Marek Vasut <marek.vasut@gmail.com>

Add another flag to the DM core which could be assigned to drivers and
which makes those drivers call their remove callbacks last, just before
booting OS and after all the other drivers finished with their remove
callbacks. This is necessary for things like clock drivers, where the
other drivers might depend on the clock driver in their remove callbacks.
Prime example is the mmc subsystem, which can reconfigure a card from HS
mode to slower modes in the remove callback and for that it needs to
reconfigure the controller clock.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5:
- Change logic to allow flags to be combined

Changes in v4:
- Use 'vital' rather than 'late' as the description
- Invert the removal flag to DM_REMOVE_NON_VITAL, to avoid the need for
        two-pass processing in the bowels of driver model
- Simplify the test to only test 'vital'
- Drop the change to uclass_destroy()

 drivers/core/device-remove.c | 39 ++++++++++++---
 drivers/core/root.c          |  2 +
 include/dm/device-internal.h | 10 ++--
 include/dm/device.h          | 10 +++-
 test/dm/core.c               | 94 ++++++++++++++++++++++++++++++++++++
 test/dm/test-driver.c        | 22 +++++++++
 6 files changed, 165 insertions(+), 12 deletions(-)

Applied to u-boot-dm, thanks!

[PATCH v5 6/9] dm: core: Avoid partially removing devices

[Simon Glass]

At present if device_remove() decides that the device should not actually
be removed, it still calls the uclass pre_remove() method and powers the
device down.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 drivers/core/device-remove.c | 59 ++++++++++++++++++++++++------------
 include/dm/device-internal.h |  9 +++++-
 test/dm/virtio.c             |  4 ++-
 3 files changed, 51 insertions(+), 21 deletions(-)

Applied to u-boot-dm, thanks!

[PATCH v5 5/9] dm: core: Remove children before advising uclass

[Simon Glass]

At present the uclass pre-remove method is called before the children are
removed. But the children may refused to be removed, in whch case the
uclass is in a tricky situation. At present we handle this by calling
the uclass' post_probe() method. But it seems better to avoid doing
anything with the uclass in this case.

Switch the ordering so that we make sure the children can be removed
before advising the uclass.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 drivers/core/device-remove.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

Applied to u-boot-dm, thanks!

[PATCH v5 4/9] dm: pci: Correct use of wrong flag name

[Simon Glass]

Update a driver that uses the incorrect flag. Add a comment to hopefully
prevent furture mistakes.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 drivers/pci/pcie_iproc.c | 2 +-
 include/dm/device.h      | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

Applied to u-boot-dm, thanks!

[PATCH v5 3/9] dm: Rename DM_FLAG_REMOVE_WITH_PD_ON

[Simon Glass]

This flag has the word 'REMOVE' in it which means it conflicts with
the DM_REMOVE flags. Rename it to DM_FLAG_LEAVE_PD_ON which seems to
indicate its purpose well enough.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 drivers/core/device-remove.c    | 2 +-
 drivers/video/meson/meson_vpu.c | 2 +-
 drivers/watchdog/rti_wdt.c      | 2 +-
 include/dm/device.h             | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

Applied to u-boot-dm, thanks!

[PATCH v5 2/9] nand: brcmnand: Don't use -EPROBE_DEFER

[Simon Glass]

This has no useful meaning in U-Boot and will never be returned. We want
to reserve this flag for internal driver model use.

Drop the code.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Applied to u-boot-dm, thanks!

[PATCH v5 1/9] smem: Don't use -EPROBE_DEFER

[Simon Glass]

This has no useful meaning in U-Boot. Use -ENOMEM since that appears to
be what has gone wrong in this case. We want to reserve this flag for
internal driver model use.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 drivers/smem/msm_smem.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Applied to u-boot-dm, thanks!