Hi Bin,
On 10 September 2015 at 00:07, Bin Meng bmeng.cn@gmail.com wrote:
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?
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