dm: core: Add late driver remove option

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>

diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index bc99ef0032f6c476067d42727ef2c2308a67c885..616dcf0785921bf11496729aaec331eb18e72bad 100644 (file)
--- 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 2bfa75b4725018508d83a01962fb7c339b7ba2b6..7ef2ec2da27fe030bbcb232c74e5d33e150df02a 100644 (file)
--- 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 b513b6861a676b25eca0e302fde856d01126ee88..39406c3f352587631778cd5a3858460b5cb7980b 100644 (file)
--- 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 204441fd23457695c456c3ce866e9e15ec3500f3..28533ce0b6dff8c8dd1091a2b592dd969ca69d36 100644 (file)
--- 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 1f5ca570dc7a9f511680c466ae0055d55970188e..bfd6565d9527220b8288dc2767e8a47c11a72aa4 100644 (file)
--- 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 a67f5d3f982ac2ab6298549b94cb4084729a8d83..ca7626a0668b60efed88555dc2e18cf2041a3892 100644 (file)
--- 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,
+};