Hi Bin,
On 10 September 2015 at 00:07, Bin Meng <bmeng...@gmail.com> wrote:
> Hi Simon,
>
> On Thu, Sep 10, 2015 at 2:07 AM, Simon Glass <s...@chromium.org> wrote:
>> Hi Bin,
>>
>> On Friday, 4 September 2015, Bin Meng <bmeng...@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...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
