[U-Boot] [PATCH 1/3] dm: core: Add flags parameter to device_remove()

[U-Boot] [PATCH 1/3] dm: core: Add flags parameter to device_remove()

[Stefan Roese]

This patch adds the flags parameter to device_remove() and changes all
calls to this function to provide the default value of DM_REMOVE_NORMAL
for "normal" device removal.

This is in preparation for the driver specific pre-OS (e.g. DMA
cancelling) remove support.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Simon Glass <sjg@chromium.org>
---
 arch/x86/cpu/queensbay/tnc.c   |  4 ++--
 cmd/cros_ec.c                  |  2 +-
 cmd/sf.c                       |  2 +-
 drivers/block/blk-uclass.c     |  2 +-
 drivers/block/sandbox.c        |  2 +-
 drivers/core/device-remove.c   |  9 +++++----
 drivers/core/device.c          |  2 +-
 drivers/core/root.c            |  2 +-
 drivers/core/uclass.c          |  2 +-
 drivers/mmc/mmc-uclass.c       |  2 +-
 drivers/mtd/spi/sandbox.c      |  2 +-
 drivers/mtd/spi/sf-uclass.c    |  2 +-
 drivers/spi/spi-uclass.c       |  4 ++--
 drivers/usb/emul/sandbox_hub.c |  2 +-
 drivers/usb/host/usb-uclass.c  |  4 ++--
 include/dm/device-internal.h   |  5 +++--
 include/dm/device.h            | 26 ++++++++++++++++++++++++++
 test/dm/bus.c                  |  8 ++++----
 test/dm/core.c                 | 16 ++++++++--------
 test/dm/eth.c                  |  2 +-
 test/dm/spi.c                  |  2 +-
 21 files changed, 65 insertions(+), 37 deletions(-)

diff --git a/arch/x86/cpu/queensbay/tnc.c b/arch/x86/cpu/queensbay/tnc.c
index f307c622c8..94668a4fda 100644
--- a/arch/x86/cpu/queensbay/tnc.c
+++ b/arch/x86/cpu/queensbay/tnc.c
@@ -76,13 +76,13 @@ static int __maybe_unused disable_igd(void)
 	 *
 	 * So the only option we have is to manually remove these two devices.
 	 */
-	ret = device_remove(igd);
+	ret = device_remove(igd, DM_REMOVE_NORMAL);
 	if (ret)
 		return ret;
 	ret = device_unbind(igd);
 	if (ret)
 		return ret;
-	ret = device_remove(sdvo);
+	ret = device_remove(sdvo, DM_REMOVE_NORMAL);
 	if (ret)
 		return ret;
 	ret = device_unbind(sdvo);
diff --git a/cmd/cros_ec.c b/cmd/cros_ec.c
index 9d42f870dc..af0b4eee78 100644
--- a/cmd/cros_ec.c
+++ b/cmd/cros_ec.c
@@ -110,7 +110,7 @@ static int do_cros_ec(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		/* Remove any existing device */
 		ret = uclass_find_device(UCLASS_CROS_EC, 0, &udev);
 		if (!ret)
-			device_remove(udev);
+			device_remove(udev, DM_REMOVE_NORMAL);
 		ret = uclass_get_device(UCLASS_CROS_EC, 0, &udev);
 		if (ret) {
 			printf("Could not init cros_ec device (err %d)\n", ret);
diff --git a/cmd/sf.c b/cmd/sf.c
index 65b117feee..f971eec781 100644
--- a/cmd/sf.c
+++ b/cmd/sf.c
@@ -124,7 +124,7 @@ static int do_spi_flash_probe(int argc, char * const argv[])
 	/* Remove the old device, otherwise probe will just be a nop */
 	ret = spi_find_bus_and_cs(bus, cs, &bus_dev, &new);
 	if (!ret) {
-		device_remove(new);
+		device_remove(new, DM_REMOVE_NORMAL);
 	}
 	flash = NULL;
 	ret = spi_flash_probe_bus_cs(bus, cs, speed, mode, &new);
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 38cb9388da..af3c35f6d0 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -530,7 +530,7 @@ int blk_unbind_all(int if_type)
 		struct blk_desc *desc = dev_get_uclass_platdata(dev);
 
 		if (desc->if_type == if_type) {
-			ret = device_remove(dev);
+			ret = device_remove(dev, DM_REMOVE_NORMAL);
 			if (ret)
 				return ret;
 			ret = device_unbind(dev);
diff --git a/drivers/block/sandbox.c b/drivers/block/sandbox.c
index 36c2ff3007..34d1c638bc 100644
--- a/drivers/block/sandbox.c
+++ b/drivers/block/sandbox.c
@@ -98,7 +98,7 @@ int host_dev_bind(int devnum, char *filename)
 	/* Remove and unbind the old device, if any */
 	ret = blk_get_device(IF_TYPE_HOST, devnum, &dev);
 	if (ret == 0) {
-		ret = device_remove(dev);
+		ret = device_remove(dev, DM_REMOVE_NORMAL);
 		if (ret)
 			return ret;
 		ret = device_unbind(dev);
diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index a7f77b4a21..b80bf52320 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -46,9 +46,10 @@ static int device_chld_unbind(struct udevice *dev)
 /**
  * device_chld_remove() - Stop all device's children
  * @dev:	The device whose children are to be removed
+ * @pre_os_remove: Flag, if this functions is called in the pre-OS stage
  * @return 0 on success, -ve on error
  */
-static int device_chld_remove(struct udevice *dev)
+static int device_chld_remove(struct udevice *dev, uint flags)
 {
 	struct udevice *pos, *n;
 	int ret;
@@ -56,7 +57,7 @@ static int device_chld_remove(struct udevice *dev)
 	assert(dev);
 
 	list_for_each_entry_safe(pos, n, &dev->child_head, sibling_node) {
-		ret = device_remove(pos);
+		ret = device_remove(pos, flags);
 		if (ret)
 			return ret;
 	}
@@ -151,7 +152,7 @@ void device_free(struct udevice *dev)
 	devres_release_probe(dev);
 }
 
-int device_remove(struct udevice *dev)
+int device_remove(struct udevice *dev, uint flags)
 {
 	const struct driver *drv;
 	int ret;
@@ -169,7 +170,7 @@ int device_remove(struct udevice *dev)
 	if (ret)
 		return ret;
 
-	ret = device_chld_remove(dev);
+	ret = device_chld_remove(dev, flags);
 	if (ret)
 		goto err;
 
diff --git a/drivers/core/device.c b/drivers/core/device.c
index 70fcfc23e0..e1b0ebffc5 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -378,7 +378,7 @@ int device_probe(struct udevice *dev)
 
 	return 0;
 fail_uclass:
-	if (device_remove(dev)) {
+	if (device_remove(dev, DM_REMOVE_NORMAL)) {
 		dm_warn("%s: Device '%s' failed to remove on error path\n",
 			__func__, dev->name);
 	}
diff --git a/drivers/core/root.c b/drivers/core/root.c
index 175fd3fb25..6dafc6ecfc 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -176,7 +176,7 @@ int dm_init(void)
 
 int dm_uninit(void)
 {
-	device_remove(dm_root());
+	device_remove(dm_root(), DM_REMOVE_NORMAL);
 	device_unbind(dm_root());
 
 	return 0;
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index 7de370644d..d94d43a98d 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -116,7 +116,7 @@ int uclass_destroy(struct uclass *uc)
 	while (!list_empty(&uc->dev_head)) {
 		dev = list_first_entry(&uc->dev_head, struct udevice,
 				       uclass_node);
-		ret = device_remove(dev);
+		ret = device_remove(dev, DM_REMOVE_NORMAL);
 		if (ret)
 			return ret;
 		ret = device_unbind(dev);
diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index 5bb446bcc2..9c07871d3a 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -232,7 +232,7 @@ int mmc_unbind(struct udevice *dev)
 
 	device_find_first_child(dev, &bdev);
 	if (bdev) {
-		device_remove(bdev);
+		device_remove(bdev, DM_REMOVE_NORMAL);
 		device_unbind(bdev);
 	}
 
diff --git a/drivers/mtd/spi/sandbox.c b/drivers/mtd/spi/sandbox.c
index 36a50fe3a1..a53f4ebc68 100644
--- a/drivers/mtd/spi/sandbox.c
+++ b/drivers/mtd/spi/sandbox.c
@@ -595,7 +595,7 @@ void sandbox_sf_unbind_emul(struct sandbox_state *state, int busnum, int cs)
 	struct udevice *dev;
 
 	dev = state->spi[busnum][cs].emul;
-	device_remove(dev);
+	device_remove(dev, DM_REMOVE_NORMAL);
 	device_unbind(dev);
 	state->spi[busnum][cs].emul = NULL;
 }
diff --git a/drivers/mtd/spi/sf-uclass.c b/drivers/mtd/spi/sf-uclass.c
index 19de964e61..83876485fe 100644
--- a/drivers/mtd/spi/sf-uclass.c
+++ b/drivers/mtd/spi/sf-uclass.c
@@ -46,7 +46,7 @@ struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs,
 
 void spi_flash_free(struct spi_flash *flash)
 {
-	device_remove(flash->spi->dev);
+	device_remove(flash->spi->dev, DM_REMOVE_NORMAL);
 }
 
 int spi_flash_probe_bus_cs(unsigned int busnum, unsigned int cs,
diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
index ac17da0777..c061c05443 100644
--- a/drivers/spi/spi-uclass.c
+++ b/drivers/spi/spi-uclass.c
@@ -343,7 +343,7 @@ err:
 	debug("%s: Error path, created=%d, device '%s'\n", __func__,
 	      created, dev->name);
 	if (created) {
-		device_remove(dev);
+		device_remove(dev, DM_REMOVE_NORMAL);
 		device_unbind(dev);
 	}
 
@@ -384,7 +384,7 @@ struct spi_slave *spi_setup_slave(unsigned int busnum, unsigned int cs,
 
 void spi_free_slave(struct spi_slave *slave)
 {
-	device_remove(slave->dev);
+	device_remove(slave->dev, DM_REMOVE_NORMAL);
 	slave->dev = NULL;
 }
 
diff --git a/drivers/usb/emul/sandbox_hub.c b/drivers/usb/emul/sandbox_hub.c
index c3a8e73389..f0939b19f4 100644
--- a/drivers/usb/emul/sandbox_hub.c
+++ b/drivers/usb/emul/sandbox_hub.c
@@ -156,7 +156,7 @@ static int clrset_post_state(struct udevice *hub, int port, int clear, int set)
 			} else if (clear & USB_PORT_STAT_POWER) {
 				debug("%s: %s: power off, removed, ret=%d\n",
 				      __func__, dev->name, ret);
-				ret = device_remove(dev);
+				ret = device_remove(dev, DM_REMOVE_NORMAL);
 				clear |= USB_PORT_STAT_CONNECTION;
 			}
 		}
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 5cf1e9a36c..6eded4abad 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -154,7 +154,7 @@ int usb_stop(void)
 	uc_priv = uc->priv;
 
 	uclass_foreach_dev(bus, uc) {
-		ret = device_remove(bus);
+		ret = device_remove(bus, DM_REMOVE_NORMAL);
 		if (ret && !err)
 			err = ret;
 	}
@@ -358,7 +358,7 @@ int usb_setup_ehci_gadget(struct ehci_ctrl **ctlrp)
 	ret = uclass_find_device_by_seq(UCLASS_USB, 0, true, &dev);
 	if (ret)
 		return ret;
-	ret = device_remove(dev);
+	ret = device_remove(dev, DM_REMOVE_NORMAL);
 	if (ret)
 		return ret;
 
diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
index 0bf8707493..2cabc87338 100644
--- a/include/dm/device-internal.h
+++ b/include/dm/device-internal.h
@@ -96,12 +96,13 @@ int device_probe(struct udevice *dev);
  * children are deactivated first.
  *
  * @dev: Pointer to device to remove
+ * @flags: Flags for selective device removal
  * @return 0 if OK, -ve on error (an error here is normally a very bad thing)
  */
 #if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE)
-int device_remove(struct udevice *dev);
+int device_remove(struct udevice *dev, uint flags);
 #else
-static inline int device_remove(struct udevice *dev) { return 0; }
+static inline int device_remove(struct udevice *dev, uint flags) { return 0; }
 #endif
 
 /**
diff --git a/include/dm/device.h b/include/dm/device.h
index 4e95fb7773..88d11e912e 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -46,6 +46,32 @@ struct driver_info;
 
 #define DM_FLAG_OF_PLATDATA		(1 << 8)
 
+/*
+ * Call driver remove function to stop currently active DMA transfers or
+ * give DMA buffers back to the HW / controller. This may be needed for
+ * some drivers to do some final stage cleanup before the OS is called
+ * (U-Boot exit)
+ */
+#define DM_FLAG_ACTIVE_DMA_REMOVE	(1 << 9)
+
+/*
+ * 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.
+ */
+enum {
+	/* Normal remove, remove all devices */
+	DM_REMOVE_NORMAL     = 1 << 0,
+
+	/* Remove only devices with active DMA */
+	DM_REMOVE_ACTIVE_DMA = DM_FLAG_ACTIVE_DMA_REMOVE,
+
+	/* Add more use cases here */
+
+	/* Remove devices with any active flag */
+	DM_REMOVE_ACTIVE_ALL = DM_REMOVE_ACTIVE_DMA,
+};
+
 /**
  * struct udevice - An instance of a driver
  *
diff --git a/test/dm/bus.c b/test/dm/bus.c
index d94dcf7a60..6a2773565e 100644
--- a/test/dm/bus.c
+++ b/test/dm/bus.c
@@ -222,7 +222,7 @@ static int test_bus_parent_data(struct unit_test_state *uts)
 	/* Check that it starts@0 and goes away when device is removed */
 	parent_data->sum += 5;
 	ut_asserteq(5, parent_data->sum);
-	device_remove(dev);
+	device_remove(dev, DM_REMOVE_NORMAL);
 	ut_asserteq_ptr(NULL, dev_get_parent_priv(dev));
 
 	/* Check that we can do this twice */
@@ -323,7 +323,7 @@ static int dm_test_bus_parent_ops(struct unit_test_state *uts)
 			continue;
 		parent_data = dev_get_parent_priv(dev);
 		ut_asserteq(FLAG_CHILD_PROBED, parent_data->flag);
-		ut_assertok(device_remove(dev));
+		ut_assertok(device_remove(dev, DM_REMOVE_NORMAL));
 		ut_asserteq_ptr(NULL, dev_get_parent_priv(dev));
 		ut_asserteq_ptr(dms->removed, dev);
 	}
@@ -360,7 +360,7 @@ static int test_bus_parent_platdata(struct unit_test_state *uts)
 		plat->count++;
 		ut_asserteq(1, plat->count);
 		device_probe(dev);
-		device_remove(dev);
+		device_remove(dev, DM_REMOVE_NORMAL);
 
 		ut_asserteq_ptr(plat, dev_get_parent_platdata(dev));
 		ut_asserteq(1, plat->count);
@@ -370,7 +370,7 @@ static int test_bus_parent_platdata(struct unit_test_state *uts)
 	ut_asserteq(3, child_count);
 
 	/* Removing the bus should also have no effect (it is still bound) */
-	device_remove(bus);
+	device_remove(bus, DM_REMOVE_NORMAL);
 	for (device_find_first_child(bus, &dev), child_count = 0;
 	     dev;
 	     device_find_next_child(&dev)) {
diff --git a/test/dm/core.c b/test/dm/core.c
index 70bf4d0605..07b2419ea4 100644
--- a/test/dm/core.c
+++ b/test/dm/core.c
@@ -319,7 +319,7 @@ static int dm_test_lifecycle(struct unit_test_state *uts)
 
 	/* Now remove device 3 */
 	ut_asserteq(0, dm_testdrv_op_count[DM_TEST_OP_PRE_REMOVE]);
-	ut_assertok(device_remove(dev));
+	ut_assertok(device_remove(dev, DM_REMOVE_NORMAL));
 	ut_asserteq(1, dm_testdrv_op_count[DM_TEST_OP_PRE_REMOVE]);
 
 	ut_asserteq(0, dm_testdrv_op_count[DM_TEST_OP_UNBIND]);
@@ -352,7 +352,7 @@ static int dm_test_ordering(struct unit_test_state *uts)
 	ut_assert(dev_last);
 
 	/* Now remove device 3 */
-	ut_assertok(device_remove(dev));
+	ut_assertok(device_remove(dev, DM_REMOVE_NORMAL));
 	ut_assertok(device_unbind(dev));
 
 	/* The device numbering should have shifted down one */
@@ -371,9 +371,9 @@ static int dm_test_ordering(struct unit_test_state *uts)
 	ut_assert(pingret == 102);
 
 	/* Remove 3 and 4 */
-	ut_assertok(device_remove(dev_penultimate));
+	ut_assertok(device_remove(dev_penultimate, DM_REMOVE_NORMAL));
 	ut_assertok(device_unbind(dev_penultimate));
-	ut_assertok(device_remove(dev_last));
+	ut_assertok(device_remove(dev_last, DM_REMOVE_NORMAL));
 	ut_assertok(device_unbind(dev_last));
 
 	/* Our device should now be in position 3 */
@@ -381,7 +381,7 @@ static int dm_test_ordering(struct unit_test_state *uts)
 	ut_assert(dev == test_dev);
 
 	/* Now remove device 3 */
-	ut_assertok(device_remove(dev));
+	ut_assertok(device_remove(dev, DM_REMOVE_NORMAL));
 	ut_assertok(device_unbind(dev));
 
 	return 0;
@@ -457,7 +457,7 @@ static int dm_test_remove(struct unit_test_state *uts)
 		ut_assert(dev);
 		ut_assertf(dev->flags & DM_FLAG_ACTIVATED,
 			   "Driver %d/%s not activated", i, dev->name);
-		ut_assertok(device_remove(dev));
+		ut_assertok(device_remove(dev, DM_REMOVE_NORMAL));
 		ut_assertf(!(dev->flags & DM_FLAG_ACTIVATED),
 			   "Driver %d/%s should have deactivated", i,
 			   dev->name);
@@ -611,14 +611,14 @@ static int dm_test_children(struct unit_test_state *uts)
 	ut_asserteq(total, dm_testdrv_op_count[DM_TEST_OP_PROBE]);
 
 	/* Remove a top-level child and check that the children are removed */
-	ut_assertok(device_remove(top[2]));
+	ut_assertok(device_remove(top[2], DM_REMOVE_NORMAL));
 	ut_asserteq(NODE_COUNT + 1, dm_testdrv_op_count[DM_TEST_OP_REMOVE]);
 	dm_testdrv_op_count[DM_TEST_OP_REMOVE] = 0;
 
 	/* Try one with grandchildren */
 	ut_assertok(uclass_get_device(UCLASS_TEST, 5, &dev));
 	ut_asserteq_ptr(dev, top[5]);
-	ut_assertok(device_remove(dev));
+	ut_assertok(device_remove(dev, DM_REMOVE_NORMAL));
 	ut_asserteq(1 + NODE_COUNT * (1 + NODE_COUNT),
 		    dm_testdrv_op_count[DM_TEST_OP_REMOVE]);
 
diff --git a/test/dm/eth.c b/test/dm/eth.c
index 6288ae24ab..564ad36916 100644
--- a/test/dm/eth.c
+++ b/test/dm/eth.c
@@ -116,7 +116,7 @@ static int dm_test_eth_act(struct unit_test_state *uts)
 	for (i = 0; i < DM_TEST_ETH_NUM; i++) {
 		ut_assertok(uclass_find_device_by_name(UCLASS_ETH,
 						       ethname[i], &dev[i]));
-		ut_assertok(device_remove(dev[i]));
+		ut_assertok(device_remove(dev[i], DM_REMOVE_NORMAL));
 
 		/* Invalidate MAC address */
 		strcpy(ethaddr[i], getenv(addrname[i]));
diff --git a/test/dm/spi.c b/test/dm/spi.c
index f52cb733c7..24fa2a48ae 100644
--- a/test/dm/spi.c
+++ b/test/dm/spi.c
@@ -36,7 +36,7 @@ static int dm_test_spi_find(struct unit_test_state *uts)
 	ut_asserteq(0, uclass_get_device_by_seq(UCLASS_SPI, busnum, &bus));
 	ut_assertok(spi_cs_info(bus, cs, &info));
 	of_offset = dev_of_offset(info.dev);
-	device_remove(info.dev);
+	device_remove(info.dev, DM_REMOVE_NORMAL);
 	device_unbind(info.dev);
 
 	/*
-- 
2.12.0

[U-Boot] [PATCH 2/3] dm: core: Add dm_remove_devices_flags() and hook it into device_remove()

[Stefan Roese]

The new function dm_remove_devices_flags() is intented for driver specific
last-stage cleanup operations before the OS is started. This patch adds
this functionality and hooks it into the common device_remove()
function.

Drivers wanting to use this feature for some last-stage removal calls,
need to add one of the DM_REMOVE_xx flags to their driver .flags.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Simon Glass <sjg@chromium.org>
---
 drivers/core/device-remove.c | 15 +++++++++++----
 drivers/core/root.c          |  7 +++++++
 include/dm/root.h            | 12 ++++++++++++
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index b80bf52320..ca4680f7c2 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -174,7 +174,12 @@ int device_remove(struct udevice *dev, uint flags)
 	if (ret)
 		goto err;
 
-	if (drv->remove) {
+	/*
+	 * Remove the device if called with the "normal" remove flag set,
+	 * or if the remove flag matches the driver flags
+	 */
+	if (drv->remove &&
+	    ((flags & DM_REMOVE_NORMAL) || (flags & drv->flags))) {
 		ret = drv->remove(dev);
 		if (ret)
 			goto err_remove;
@@ -188,10 +193,12 @@ int device_remove(struct udevice *dev, uint flags)
 		}
 	}
 
-	device_free(dev);
+	if ((flags & DM_REMOVE_NORMAL) || (flags & drv->flags)) {
+		device_free(dev);
 
-	dev->seq = -1;
-	dev->flags &= ~DM_FLAG_ACTIVATED;
+		dev->seq = -1;
+		dev->flags &= ~DM_FLAG_ACTIVATED;
+	}
 
 	return ret;
 
diff --git a/drivers/core/root.c b/drivers/core/root.c
index 6dafc6ecfc..4c4642a8a4 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -182,6 +182,13 @@ int dm_uninit(void)
 	return 0;
 }
 
+int dm_remove_devices_flags(uint flags)
+{
+	device_remove(dm_root(), flags);
+
+	return 0;
+}
+
 int dm_scan_platdata(bool pre_reloc_only)
 {
 	int ret;
diff --git a/include/dm/root.h b/include/dm/root.h
index 3cf730dcee..b13c005c17 100644
--- a/include/dm/root.h
+++ b/include/dm/root.h
@@ -115,4 +115,16 @@ int dm_init(void);
  */
 int dm_uninit(void);
 
+/**
+ * dm_remove_devices_flags - Call remove function of all drivers with
+ *                           specific removal flags set to selectively
+ *                           remove drivers
+ *
+ * All devices with the matching flags set will be removed
+ *
+ * @flags: Flags for selective device removal
+ * @return 0 if OK, -ve on error
+ */
+int dm_remove_devices_flags(uint flags);
+
 #endif
-- 
2.12.0

[U-Boot] [PATCH 3/3] arm: bootm: Add dm_remove_devices_flags() call to announce_and_cleanup()

[Stefan Roese]

This patch adds a call to dm_remove_devices_flags() to
announce_and_cleanup() so that drivers that have one of the removal flags
set (e.g. DM_FLAG_ACTIVE_DMA_REMOVE) in their driver struct, may do some
last-stage cleanup before the OS is started.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Simon Glass <sjg@chromium.org>
---
 arch/arm/lib/bootm.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 8125cf023f..426bee6da5 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -14,6 +14,8 @@
 
 #include <common.h>
 #include <command.h>
+#include <dm/device.h>
+#include <dm/root.h>
 #include <image.h>
 #include <u-boot/zlib.h>
 #include <asm/byteorder.h>
@@ -91,6 +93,13 @@ static void announce_and_cleanup(int fake)
 
 	board_quiesce_devices();
 
+	/*
+	 * Call remove function of all devices with a removal flag set.
+	 * 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);
+
 	cleanup_before_linux();
 }
 
-- 
2.12.0

[U-Boot] [PATCH 1/3] dm: core: Add flags parameter to device_remove()

[Simon Glass]

Hi Stefan,

On 10 March 2017 at 06:50, Stefan Roese <sr@denx.de> wrote:
> This patch adds the flags parameter to device_remove() and changes all
> calls to this function to provide the default value of DM_REMOVE_NORMAL
> for "normal" device removal.
>
> This is in preparation for the driver specific pre-OS (e.g. DMA
> cancelling) remove support.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>  arch/x86/cpu/queensbay/tnc.c   |  4 ++--
>  cmd/cros_ec.c                  |  2 +-
>  cmd/sf.c                       |  2 +-
>  drivers/block/blk-uclass.c     |  2 +-
>  drivers/block/sandbox.c        |  2 +-
>  drivers/core/device-remove.c   |  9 +++++----
>  drivers/core/device.c          |  2 +-
>  drivers/core/root.c            |  2 +-
>  drivers/core/uclass.c          |  2 +-
>  drivers/mmc/mmc-uclass.c       |  2 +-
>  drivers/mtd/spi/sandbox.c      |  2 +-
>  drivers/mtd/spi/sf-uclass.c    |  2 +-
>  drivers/spi/spi-uclass.c       |  4 ++--
>  drivers/usb/emul/sandbox_hub.c |  2 +-
>  drivers/usb/host/usb-uclass.c  |  4 ++--
>  include/dm/device-internal.h   |  5 +++--
>  include/dm/device.h            | 26 ++++++++++++++++++++++++++
>  test/dm/bus.c                  |  8 ++++----
>  test/dm/core.c                 | 16 ++++++++--------
>  test/dm/eth.c                  |  2 +-
>  test/dm/spi.c                  |  2 +-
>  21 files changed, 65 insertions(+), 37 deletions(-)
>

[...]

> diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
> index 0bf8707493..2cabc87338 100644
> --- a/include/dm/device-internal.h
> +++ b/include/dm/device-internal.h
> @@ -96,12 +96,13 @@ int device_probe(struct udevice *dev);
>   * children are deactivated first.
>   *
>   * @dev: Pointer to device to remove
> + * @flags: Flags for selective device removal
>   * @return 0 if OK, -ve on error (an error here is normally a very bad thing)
>   */
>  #if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE)
> -int device_remove(struct udevice *dev);
> +int device_remove(struct udevice *dev, uint flags);
>  #else
> -static inline int device_remove(struct udevice *dev) { return 0; }
> +static inline int device_remove(struct udevice *dev, uint flags) { return 0; }
>  #endif
>
>  /**
> diff --git a/include/dm/device.h b/include/dm/device.h
> index 4e95fb7773..88d11e912e 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -46,6 +46,32 @@ struct driver_info;
>
>  #define DM_FLAG_OF_PLATDATA            (1 << 8)
>
> +/*
> + * Call driver remove function to stop currently active DMA transfers or
> + * give DMA buffers back to the HW / controller. This may be needed for
> + * some drivers to do some final stage cleanup before the OS is called
> + * (U-Boot exit)
> + */
> +#define DM_FLAG_ACTIVE_DMA_REMOVE      (1 << 9)

Do we need _REMOVE on this end of this? It seems to me that we could
just flag that this driver has active DMA.

> +
> +/*
> + * 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.
> + */
> +enum {
> +       /* Normal remove, remove all devices */
> +       DM_REMOVE_NORMAL     = 1 << 0,
> +
> +       /* Remove only devices with active DMA */

Can we remove the word 'only'? I think it gets confusing if one flag
negates another.

> +       DM_REMOVE_ACTIVE_DMA = DM_FLAG_ACTIVE_DMA_REMOVE,
> +
> +       /* Add more use cases here */
> +
> +       /* Remove devices with any active flag */
> +       DM_REMOVE_ACTIVE_ALL = DM_REMOVE_ACTIVE_DMA,
> +};
> +
>  /**
>   * struct udevice - An instance of a driver
>   *

[..]

Regards,
Simon

[U-Boot] [PATCH 2/3] dm: core: Add dm_remove_devices_flags() and hook it into device_remove()

[Simon Glass]

On 10 March 2017 at 06:50, Stefan Roese <sr@denx.de> wrote:
> The new function dm_remove_devices_flags() is intented for driver specific
> last-stage cleanup operations before the OS is started. This patch adds
> this functionality and hooks it into the common device_remove()
> function.
>
> Drivers wanting to use this feature for some last-stage removal calls,
> need to add one of the DM_REMOVE_xx flags to their driver .flags.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>  drivers/core/device-remove.c | 15 +++++++++++----
>  drivers/core/root.c          |  7 +++++++
>  include/dm/root.h            | 12 ++++++++++++
>  3 files changed, 30 insertions(+), 4 deletions(-)

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

[U-Boot] [PATCH 3/3] arm: bootm: Add dm_remove_devices_flags() call to announce_and_cleanup()

[Simon Glass]

On 10 March 2017 at 06:50, Stefan Roese <sr@denx.de> wrote:
> This patch adds a call to dm_remove_devices_flags() to
> announce_and_cleanup() so that drivers that have one of the removal flags
> set (e.g. DM_FLAG_ACTIVE_DMA_REMOVE) in their driver struct, may do some
> last-stage cleanup before the OS is started.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>  arch/arm/lib/bootm.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

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

One more patch with a test?

[U-Boot] [PATCH 1/3] dm: core: Add flags parameter to device_remove()

[Stefan Roese]

Hi Simon,

On 13.03.2017 13:33, Simon Glass wrote:
> Hi Stefan,
>
> On 10 March 2017 at 06:50, Stefan Roese <sr@denx.de> wrote:
>> This patch adds the flags parameter to device_remove() and changes all
>> calls to this function to provide the default value of DM_REMOVE_NORMAL
>> for "normal" device removal.
>>
>> This is in preparation for the driver specific pre-OS (e.g. DMA
>> cancelling) remove support.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Simon Glass <sjg@chromium.org>
>> ---
>>  arch/x86/cpu/queensbay/tnc.c   |  4 ++--
>>  cmd/cros_ec.c                  |  2 +-
>>  cmd/sf.c                       |  2 +-
>>  drivers/block/blk-uclass.c     |  2 +-
>>  drivers/block/sandbox.c        |  2 +-
>>  drivers/core/device-remove.c   |  9 +++++----
>>  drivers/core/device.c          |  2 +-
>>  drivers/core/root.c            |  2 +-
>>  drivers/core/uclass.c          |  2 +-
>>  drivers/mmc/mmc-uclass.c       |  2 +-
>>  drivers/mtd/spi/sandbox.c      |  2 +-
>>  drivers/mtd/spi/sf-uclass.c    |  2 +-
>>  drivers/spi/spi-uclass.c       |  4 ++--
>>  drivers/usb/emul/sandbox_hub.c |  2 +-
>>  drivers/usb/host/usb-uclass.c  |  4 ++--
>>  include/dm/device-internal.h   |  5 +++--
>>  include/dm/device.h            | 26 ++++++++++++++++++++++++++
>>  test/dm/bus.c                  |  8 ++++----
>>  test/dm/core.c                 | 16 ++++++++--------
>>  test/dm/eth.c                  |  2 +-
>>  test/dm/spi.c                  |  2 +-
>>  21 files changed, 65 insertions(+), 37 deletions(-)
>>
>
> [...]
>
>> diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
>> index 0bf8707493..2cabc87338 100644
>> --- a/include/dm/device-internal.h
>> +++ b/include/dm/device-internal.h
>> @@ -96,12 +96,13 @@ int device_probe(struct udevice *dev);
>>   * children are deactivated first.
>>   *
>>   * @dev: Pointer to device to remove
>> + * @flags: Flags for selective device removal
>>   * @return 0 if OK, -ve on error (an error here is normally a very bad thing)
>>   */
>>  #if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE)
>> -int device_remove(struct udevice *dev);
>> +int device_remove(struct udevice *dev, uint flags);
>>  #else
>> -static inline int device_remove(struct udevice *dev) { return 0; }
>> +static inline int device_remove(struct udevice *dev, uint flags) { return 0; }
>>  #endif
>>
>>  /**
>> diff --git a/include/dm/device.h b/include/dm/device.h
>> index 4e95fb7773..88d11e912e 100644
>> --- a/include/dm/device.h
>> +++ b/include/dm/device.h
>> @@ -46,6 +46,32 @@ struct driver_info;
>>
>>  #define DM_FLAG_OF_PLATDATA            (1 << 8)
>>
>> +/*
>> + * Call driver remove function to stop currently active DMA transfers or
>> + * give DMA buffers back to the HW / controller. This may be needed for
>> + * some drivers to do some final stage cleanup before the OS is called
>> + * (U-Boot exit)
>> + */
>> +#define DM_FLAG_ACTIVE_DMA_REMOVE      (1 << 9)
>
> Do we need _REMOVE on this end of this? It seems to me that we could
> just flag that this driver has active DMA.

Sounds better, yes.

>> +
>> +/*
>> + * 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.
>> + */
>> +enum {
>> +       /* Normal remove, remove all devices */
>> +       DM_REMOVE_NORMAL     = 1 << 0,
>> +
>> +       /* Remove only devices with active DMA */
>
> Can we remove the word 'only'? I think it gets confusing if one flag
> negates another.

Sure. I'll change this in v2.

Thanks,
Stefan

[U-Boot] [PATCH 3/3] arm: bootm: Add dm_remove_devices_flags() call to announce_and_cleanup()

[Stefan Roese]

Hi Simon,

On 13.03.2017 13:34, Simon Glass wrote:
> On 10 March 2017 at 06:50, Stefan Roese <sr@denx.de> wrote:
>> This patch adds a call to dm_remove_devices_flags() to
>> announce_and_cleanup() so that drivers that have one of the removal flags
>> set (e.g. DM_FLAG_ACTIVE_DMA_REMOVE) in their driver struct, may do some
>> last-stage cleanup before the OS is started.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Simon Glass <sjg@chromium.org>
>> ---
>>  arch/arm/lib/bootm.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> One more patch with a test?

Right, already on the list. Lets see, if I can get this finished today.
Otherwise it will have to wait for a few days...

Thanks,
Stefan

[U-Boot] [PATCH 3/3] arm: bootm: Add dm_remove_devices_flags() call to announce_and_cleanup()

[Stefan Roese]

Hi Simon,

On 13.03.2017 13:50, Stefan Roese wrote:
> On 13.03.2017 13:34, Simon Glass wrote:
>> On 10 March 2017 at 06:50, Stefan Roese <sr@denx.de> wrote:
>>> This patch adds a call to dm_remove_devices_flags() to
>>> announce_and_cleanup() so that drivers that have one of the removal
>>> flags
>>> set (e.g. DM_FLAG_ACTIVE_DMA_REMOVE) in their driver struct, may do some
>>> last-stage cleanup before the OS is started.
>>>
>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> ---
>>>  arch/arm/lib/bootm.c | 9 +++++++++
>>>  1 file changed, 9 insertions(+)
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> One more patch with a test?
>
> Right, already on the list. Lets see, if I can get this finished today.
> Otherwise it will have to wait for a few days...

Do you have something like the reset test case:

4581b717 [reset: implement a reset test]

in mind for this selective device remove test? Or something
completely different?

Thanks,
Stefan

[U-Boot] [PATCH 3/3] arm: bootm: Add dm_remove_devices_flags() call to announce_and_cleanup()

[Simon Glass]

Hi Stefan,

On 13 March 2017 at 07:20, Stefan Roese <sr@denx.de> wrote:
> Hi Simon,
>
> On 13.03.2017 13:50, Stefan Roese wrote:
>>
>> On 13.03.2017 13:34, Simon Glass wrote:
>>>
>>> On 10 March 2017 at 06:50, Stefan Roese <sr@denx.de> wrote:
>>>>
>>>> This patch adds a call to dm_remove_devices_flags() to
>>>> announce_and_cleanup() so that drivers that have one of the removal
>>>> flags
>>>> set (e.g. DM_FLAG_ACTIVE_DMA_REMOVE) in their driver struct, may do some
>>>> last-stage cleanup before the OS is started.
>>>>
>>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>>> Cc: Simon Glass <sjg@chromium.org>
>>>> ---
>>>>  arch/arm/lib/bootm.c | 9 +++++++++
>>>>  1 file changed, 9 insertions(+)
>>>
>>>
>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>
>>> One more patch with a test?
>>
>>
>> Right, already on the list. Lets see, if I can get this finished today.
>> Otherwise it will have to wait for a few days...
>
>
> Do you have something like the reset test case:
>
> 4581b717 [reset: implement a reset test]
>
> in mind for this selective device remove test? Or something
> completely different?

I think you could create a device with the DMA flag set and see that
it (and only it) is removed. Then another test case which removes
everything. Basically we want to test that each of the flag values
works as expected and at present you only have two).

One way to check that is via device_active().

Regards,
Simon