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