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?

> 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,
Bin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to