Hi Simon,

sorry for the late replay - just back from vacation....

On 09.04.2017 21:28, Simon Glass wrote:
Hi Stefan,

On 6 April 2017 at 07:29, Stefan Roese <s...@denx.de> wrote:
On my x86 platform I've noticed, that calling dm_uninit() or the new
function dm_remove_devices_flags() does not remove the desired device at
all. Debugging showed, that the serial uclass returns -EPERM in
serial_pre_remove() and this leads to a complete stop of the device
removal pretty early, as the serial device is one of the first ones in
the DM. Here the dm tree output:

=> dm tree
 Class       Probed   Name
----------------------------------------
 root        [ + ]    root_driver
 rsa_mod_exp [   ]    |-- mod_exp_sw
 serial      [ + ]    |-- serial
 rtc         [   ]    |-- rtc
 timer       [ + ]    |-- tsc-timer
 syscon      [ + ]    |-- pch_pinctrl
 ...

In this example, device_remove(root) will stop directly after trying to
remove the "serial" device.

To solve this problem, this patch removes the return upon error check in
the device_remove() call in device_chld_remove(). This leads to
device_chld_remove() continuing with the device_remove() call to the
following child devices.

I think the right solution is to find out why stdio_deregister_dev()
fails. It is probably because the device is in use within the stdio
variables.

This is most likely the case, yes.

Perhaps you need to remove it first?

Not sure if this should / could be done in this general case of
removal of all devices (or all devices matching a remove-flag)?
Please think of dm_uninit() being called. This functions should
have no internal knowledge of usage of devices. One thing I could
do, probably best in a separate patch, is to use the force flag
of stdio_deregister_dev() in serial_pre_remove() to force the
serial device(s) to be removed in this case. What do you think?


Signed-off-by: Stefan Roese <s...@denx.de>
Cc: Simon Glass <s...@chromium.org>
Cc: Bin Meng <bmeng...@gmail.com>
---
 drivers/core/device-remove.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index cc0043b990..8b46f3343e 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -52,15 +52,11 @@ static int device_chld_unbind(struct udevice *dev)
 static int device_chld_remove(struct udevice *dev, uint flags)
 {
        struct udevice *pos, *n;
-       int ret;

        assert(dev);

-       list_for_each_entry_safe(pos, n, &dev->child_head, sibling_node) {
-               ret = device_remove(pos, flags);
-               if (ret)
-                       return ret;
-       }
+       list_for_each_entry_safe(pos, n, &dev->child_head, sibling_node)
+               device_remove(pos, flags);

I think we should keep the error checking here.

The main fix with this patch is, that removing of child devices does
not stop once an error is encountered. Even if an error occurs with
one child-device of a parent, all other child-devices of this
parent should still be removed - or at least tried to.

So what does this error code buy us here? Should I print a log message
here in this error case?

Thanks,
Stefan
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to