[U-Boot] [PATCH 4/5] dm: core: Remove the call to device_free()

[Simon Glass]

Hi Bin,

On 10 September 2015 at 00:07, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Thu, Sep 10, 2015 at 2:07 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Bin,
>>
>> On Friday, 4 September 2015, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>
>>> When something goes wrong during device_probe(), we just need do
>>> device_remove() which calls device_free() internally.
>>>
>>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>> ---
>>>
>>>  drivers/core/device.c | 9 ++-------
>>>  1 file changed, 2 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/core/device.c b/drivers/core/device.c
>>> index a6cd936..061a7ef 100644
>>> --- a/drivers/core/device.c
>>> +++ b/drivers/core/device.c
>>> @@ -316,19 +316,14 @@ int device_probe_child(struct udevice *dev, void *parent_priv)
>>>
>>>         ret = uclass_post_probe_device(dev);
>>>         if (ret)
>>> -               goto fail_uclass;
>>> +               goto fail;
>>>
>>>         return 0;
>>> -fail_uclass:
>>> +fail:
>>>         if (device_remove(dev)) {
>>>                 dm_warn("%s: Device '%s' failed to remove on error path\n",
>>>                         __func__, dev->name);
>>>         }
>>> -fail:
>>> -       dev->flags &= ~DM_FLAG_ACTIVATED;
>>> -
>>> -       dev->seq = -1;
>>> -       device_free(dev);
>>>
>>>         return ret;
>>>  }
>>
>>
>> The problem is that in this case you end up calling functions that
>> should not be called. For example uclass_pre_remove_device() will be
>> called even if uclass_post_probe_device() was not.
>>
>
> Yes, but doing uclass_pre_remove_device() should not bring any harm,
> given we already want to remove this device, right?

That function is entitled to assume that the device was set up at
least to the post_remove uclass state. Here that would not be try, so
we are creating an entirely new test case.

>
>> There is definitely room for improvement here - we could/should call
>> only those 'remove' functions that mirror the 'probe' ones we called.
>> But that is quite a lot of code for little benefit.

Regards,
Simon