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

[Simon Glass]

Hi Stefan,

On 8 May 2017 at 01:35, Stefan Roese <sr at denx.de> wrote:
> Hi Simon,
>
> On 04.05.2017 18:50, Simon Glass wrote:
>
> <snip>
>
>>>>> ... 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.
>
>
> Okay, I see your point.
>
>>>
>>>> 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.
>
>
> Understood. How about doing it this way: You drop this patch from
> the series for now (it still works for us with the force remove of
> the serial driver) and apply the remaining patches. I'll try to get
> back to this skip-failures flag implementation in a few days / weeks.
>
> Is this okay for you?

Yes that's fine.

>
> Thanks,
> Stefan

Regards,
Simon