Skip to contentSkip to conversation

dm: core: Add flags parameter to device_remove()

PATCH Superseded Stefan Roese 2 participants 10 messages Mar 10, 2017 – Mar 13, 2017

Subsystems (8)
Arm (32-bit) x86 USB Block layer +4 more subsystems

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/3 dm: core: Add flags parameter to device_remove()

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.

---
 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(-)
arch/x86/cpu/queensbay/tnc.c +2 -2
index f307c622c8..94668a4fda 100644
@@ -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);
cmd/cros_ec.c +1 -1
index 9d42f870dc..af0b4eee78 100644
@@ -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);
cmd/sf.c +1 -1
index 65b117feee..f971eec781 100644
@@ -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);
drivers/block/blk-uclass.c +1 -1
index 38cb9388da..af3c35f6d0 100644
@@ -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);
drivers/block/sandbox.c +1 -1
index 36c2ff3007..34d1c638bc 100644
@@ -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);
drivers/core/device-remove.c +5 -4
index a7f77b4a21..b80bf52320 100644
@@ -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;
 
drivers/core/device.c +1 -1
index 70fcfc23e0..e1b0ebffc5 100644
@@ -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);
 	}
drivers/core/root.c +1 -1
index 175fd3fb25..6dafc6ecfc 100644
@@ -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;
drivers/core/uclass.c +1 -1
index 7de370644d..d94d43a98d 100644
@@ -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);
drivers/mmc/mmc-uclass.c +1 -1
index 5bb446bcc2..9c07871d3a 100644
@@ -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);
 	}
 
drivers/mtd/spi/sandbox.c +1 -1
index 36a50fe3a1..a53f4ebc68 100644
@@ -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;
 }
drivers/mtd/spi/sf-uclass.c +1 -1
index 19de964e61..83876485fe 100644
@@ -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,
drivers/spi/spi-uclass.c +2 -2
index ac17da0777..c061c05443 100644
@@ -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;
 }
 
drivers/usb/emul/sandbox_hub.c +1 -1
index c3a8e73389..f0939b19f4 100644
@@ -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;
 			}
 		}
drivers/usb/host/usb-uclass.c +2 -2
index 5cf1e9a36c..6eded4abad 100644
@@ -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;
 
include/dm/device-internal.h +3 -2
index 0bf8707493..2cabc87338 100644
@@ -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
 
 /**
include/dm/device.h +26 -0
index 4e95fb7773..88d11e912e 100644
@@ -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
  *
test/dm/bus.c +4 -4
index d94dcf7a60..6a2773565e 100644
@@ -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)) {
test/dm/core.c +8 -8
index 70bf4d0605..07b2419ea4 100644
@@ -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]);
 
test/dm/eth.c +1 -1
index 6288ae24ab..564ad36916 100644
@@ -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]));
test/dm/spi.c +1 -1
index f52cb733c7..24fa2a48ae 100644
@@ -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);
 
 	/*
Signature · 1 line
2.12.0
#1 lore raw

PATCH 2/3 dm: core: Add dm_remove_devices_flags() and hook it into device_remove()

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.

---
 drivers/core/device-remove.c | 15 +++++++++++----
 drivers/core/root.c          |  7 +++++++
 include/dm/root.h            | 12 ++++++++++++
 3 files changed, 30 insertions(+), 4 deletions(-)
drivers/core/device-remove.c +11 -4
index b80bf52320..ca4680f7c2 100644
@@ -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;
 
drivers/core/root.c +7 -0
index 6dafc6ecfc..4c4642a8a4 100644
@@ -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;
include/dm/root.h +12 -0
index 3cf730dcee..b13c005c17 100644
@@ -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
Signature · 1 line
2.12.0
#2 lore raw

PATCH 2/3 dm: core: Add dm_remove_devices_flags() and hook it into device_remove()

Quoted 8 lines · 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.

Quoted 7 lines
---
 drivers/core/device-remove.c | 15 +++++++++++----
 drivers/core/root.c          |  7 +++++++
 include/dm/root.h            | 12 ++++++++++++
 3 files changed, 30 insertions(+), 4 deletions(-)
#3 lore raw

PATCH 3/3 arm: bootm: Add dm_remove_devices_flags() call to announce_and_cleanup()

In reply to 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.

---
 arch/arm/lib/bootm.c | 9 +++++++++
 1 file changed, 9 insertions(+)
arch/arm/lib/bootm.c +9 -0
index 8125cf023f..426bee6da5 100644
@@ -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();
 }
 
Signature · 1 line
2.12.0
#4 lore raw

PATCH 3/3 arm: bootm: Add dm_remove_devices_flags() call to announce_and_cleanup()

Quoted 10 lines · 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.

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

One more patch with a test?

#5 lore raw

PATCH 3/3 arm: bootm: Add dm_remove_devices_flags() call to announce_and_cleanup()

Hi Simon,

Quoted 12 lines · Simon Glass
Quoted 10 lines · 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.

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

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

#6 lore raw

PATCH 3/3 arm: bootm: Add dm_remove_devices_flags() call to announce_and_cleanup()

Hi Simon,

Quoted 14 lines · Stefan Roese
Quoted 13 lines · Simon Glass
Quoted 11 lines · 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.

---
 arch/arm/lib/bootm.c | 9 +++++++++
 1 file changed, 9 insertions(+)
Quoted 6 lines
Quoted 3 lines

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

#7 lore raw

PATCH 3/3 arm: bootm: Add dm_remove_devices_flags() call to announce_and_cleanup()

Hi Stefan,

Quoted 30 lines · Stefan Roese

Hi Simon,

Quoted 18 lines · Stefan Roese
Quoted 16 lines · Simon Glass
Quoted 12 lines · 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.

---
 arch/arm/lib/bootm.c | 9 +++++++++
 1 file changed, 9 insertions(+)
Quoted 7 lines
Quoted 3 lines

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...

Quoted 6 lines

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

#8 lore raw

PATCH 1/3 dm: core: Add flags parameter to device_remove()

In reply to Stefan Roese

Hi Stefan,

Quoted 33 lines · 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.

---
 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(-)

[...]

Quoted 25 lines
include/dm/device-internal.h +3 -2
index 0bf8707493..2cabc87338 100644
@@ -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
 
 /**
include/dm/device.h +0 -0
index 4e95fb7773..88d11e912e 100644

include/dm/device.h struct driver_info; · line 55

Quoted 10 lines
include/dm/device.h +7 -0
@@ -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.

include/dm/device.h struct driver_info; · line 66

Quoted 11 lines
include/dm/device.h +11 -0
+
+/*
+ * 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.

include/dm/device.h struct driver_info; · line 77

Quoted 11 lines
include/dm/device.h +8 -0
+       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

#9 lore raw

PATCH 1/3 dm: core: Add flags parameter to device_remove()

Hi Simon,

Quoted 67 lines · Simon Glass

Hi Stefan,

Quoted 33 lines · 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.

---
 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(-)

[...]

Quoted 28 lines
include/dm/device-internal.h +3 -2
index 0bf8707493..2cabc87338 100644
@@ -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
 
 /**
include/dm/device.h +0 -0
index 4e95fb7773..88d11e912e 100644
@@ -46,6 +46,32 @@ struct driver_info;
 
 #define DM_FLAG_OF_PLATDATA            (1 << 8)
 
Quoted 10 lines

include/dm/device.h struct driver_info; · line 55

Quoted 7 lines
include/dm/device.h +7 -0
+/*
+ * 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.

Quoted 6 lines
Quoted 6 lines
include/dm/device.h +6 -0
+
+/*
+ * 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.
+ */
Quoted 8 lines

include/dm/device.h struct driver_info; · line 66

Quoted 5 lines
include/dm/device.h +5 -0
+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