[PATCH] dm: core: Check flags before removing devices

Calling device_chld_remove() before flags_remove() means all devices get removed no matter whether they should be removed late or not. This breaks teardown of eMMC when booting and other critical boot paths.
Fixes: c51d2e704a1 ("dm: core: Avoid partially removing devices") Signed-off-by: Marek Vasut marex@denx.de Cc: Simon Glass sjg@chromium.org --- drivers/core/device-remove.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c index e6ec6ff4212..0454f55c330 100644 --- a/drivers/core/device-remove.c +++ b/drivers/core/device-remove.c @@ -207,14 +207,6 @@ 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 @@ -228,6 +220,14 @@ int device_remove(struct udevice *dev, uint flags) return ret; }
+ /* + * 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; + ret = uclass_pre_remove_device(dev); if (ret) return ret;

Hi Marek,
On Thu, 27 Jan 2022 at 20:41, Marek Vasut marex@denx.de wrote:
Calling device_chld_remove() before flags_remove() means all devices get removed no matter whether they should be removed late or not. This breaks teardown of eMMC when booting and other critical boot paths.
Fixes: c51d2e704a1 ("dm: core: Avoid partially removing devices") Signed-off-by: Marek Vasut marex@denx.de Cc: Simon Glass sjg@chromium.org
drivers/core/device-remove.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
This means that the children do not get the remove signal if -EPROBE_DEFER or -EKEYREJECTED are returned by the 'dev' device.
Also it fails several tests ('make qcheck').
diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c index e6ec6ff4212..0454f55c330 100644 --- a/drivers/core/device-remove.c +++ b/drivers/core/device-remove.c @@ -207,14 +207,6 @@ 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
@@ -228,6 +220,14 @@ int device_remove(struct udevice *dev, uint flags) return ret; }
/*
* 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;
ret = uclass_pre_remove_device(dev); if (ret) return ret;
-- 2.34.1
Regards, Simon

On 2/11/22 16:05, Simon Glass wrote:
Hi Marek,
Hi,
Calling device_chld_remove() before flags_remove() means all devices get removed no matter whether they should be removed late or not. This breaks teardown of eMMC when booting and other critical boot paths.
Fixes: c51d2e704a1 ("dm: core: Avoid partially removing devices") Signed-off-by: Marek Vasut marex@denx.de Cc: Simon Glass sjg@chromium.org
drivers/core/device-remove.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
This means that the children do not get the remove signal if -EPROBE_DEFER or -EKEYREJECTED are returned by the 'dev' device.
Also it fails several tests ('make qcheck').
Do you have an idea for a better fix, one which doesn't break booting Linux from U-Boot ? I think that's a rather important use-case .

Hi Marek,
On Fri, 11 Feb 2022 at 08:24, Marek Vasut marex@denx.de wrote:
On 2/11/22 16:05, Simon Glass wrote:
Hi Marek,
Hi,
Calling device_chld_remove() before flags_remove() means all devices get removed no matter whether they should be removed late or not. This breaks teardown of eMMC when booting and other critical boot paths.
Fixes: c51d2e704a1 ("dm: core: Avoid partially removing devices") Signed-off-by: Marek Vasut marex@denx.de Cc: Simon Glass sjg@chromium.org
drivers/core/device-remove.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
This means that the children do not get the remove signal if -EPROBE_DEFER or -EKEYREJECTED are returned by the 'dev' device.
Also it fails several tests ('make qcheck').
Do you have an idea for a better fix, one which doesn't break booting Linux from U-Boot ? I think that's a rather important use-case .
Well the problem is that I don't understand the problem.
Can you explain it in more detail? The commit message does not help much and you have not added a test for the case you are trying to enable.
We must remove children before their parents, since children may rely on their parents to be around until they are removed. This is part of the device lifecycle as documented.
So what specific devices are children here? Perhaps the output of 'dm tree' would help.
Regards, Simon
participants (2)
-
Marek Vasut
-
Simon Glass