Hi Stefan, On 2 May 2017 at 05:45, Stefan Roese <s...@denx.de> wrote: > 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).
Possibly, but programmatically it becomes impossible to detect a failure. Say the USB fails to stop its DMA, we might want to reboot rather than continue to boot Linux and crash. > >> 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. I don't think the complexity is too great. It does need a new test. But I think we should hold the line on error checking even with remove(), by default. > > Thanks, > Stefan Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
