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

[Stefan Roese]

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.

Thanks,
Stefan