Hi Simon,

On 02.05.2017 13:27, Simon Glass wrote:
On 1 May 2017 at 00:14, Stefan Roese <s...@denx.de> wrote:
Hi Simon,


On 29.04.2017 02:26, Simon Glass wrote:

On 24 April 2017 at 01:48, 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.

Signed-off-by: Stefan Roese <s...@denx.de>
Cc: Simon Glass <s...@chromium.org>
Cc: Bin Meng <bmeng...@gmail.com>
---
v2:
- Add debug() output in error case

 drivers/core/device-remove.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)


I thought that your change to force removal of the stdio dev would
make this change unnecessary?


Yes, the force removal made this change unnecessary in this specific
case. But...

I really would rather fix the root cause if we can.


... the current implementation to exit the loop over all children
upon error and not remove the remaining children is wrong IMO. All
devices should at least be tried to get removed, even if one fails
to get removed. This is what this patch makes sure of.

Yes I see that, but not being able to remove is actually an error. In
the normal course of events, a device that will not remove itself is
likely buggy.

Isn't it enough then to just print an error message in this case
in this loop - change debug() to printf() in this current patch
version? Then "users" of this code will be aware of such remove
failures and can take appropriate actions (fix bug etc in their
setup).

What do you think about adding a new remove flag to indicate that
failures should be skipped?

I'm a bit afraid that this makes the code overly complex. But if
you prefer to have it this way, then I can come up with such a
version as well. Just let me know.

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

Reply via email to