Hi Simon,
On Thu, Sep 10, 2015 at 2:07 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Friday, 4 September 2015, Bin Meng bmeng.cn@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@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