Hi Stefan, On 19 April 2017 at 03:27, Stefan Roese <s...@denx.de> wrote: > 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?
I think that sounds reasonable. I don't know this area very well though. > >>> >>> 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? Yes just a debug() will do. I see what you are saying, I'm just nervous of dropping error checking since devices really should remove cleanly. Hopefully as above you can fix the error? Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot