From c51d2e704a1c89d504b379b133bd552c3387fa6c Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 24 Jan 2021 14:32:45 -0700 Subject: [PATCH] dm: core: Avoid partially removing devices 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 --- 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 35b625e7b1..bc99ef0032 100644 --- a/drivers/core/device-remove.c +++ b/drivers/core/device-remove.c @@ -8,6 +8,8 @@ * Pavel Herrmann */ +#define LOG_CATEGORY LOGC_DM + #include #include #include @@ -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 639bbd293d..b513b6861a 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 ad355981cf..9a7e658cce 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.39.5