[U-Boot] [PATCH 1/5 v2] dm: device_remove: Don't return in device_chld_remove() upon error

[Simon Glass]

Hi Stefan,

On 1 May 2017 at 00:14, Stefan Roese <sr at denx.de> wrote:
> Hi Simon,
>
>
> On 29.04.2017 02:26, Simon Glass wrote:
>>
>> On 24 April 2017 at 01:48, Stefan Roese <sr at 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 <sr at denx.de>
>>> Cc: Simon Glass <sjg at chromium.org>
>>> Cc: Bin Meng <bmeng.cn at 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.

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

Regards,
Simon