[U-Boot] [PATCH v3 2/4] dm: core: Test dev->flags after device_probe() in device_probe_child()

[Simon Glass]

Hi Bin,

On 23 August 2015 at 02:32, Bin Meng <bmeng.cn at gmail.com> wrote:
> The device might have already been probed during the call to
> device_probe() on its parent device (e.g. PCI bridge devices).
> Test the flags again so that we don't mess up the device.
>
> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> ---
>
> Changes in v3: None
>
> drivers/core/device.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index e23a872..7a62812 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -266,6 +266,14 @@ int device_probe_child(struct udevice *dev, void
*parent_priv)
> goto fail;
> }
>
> + /*
> + * The device might have already been probed during the call to
> + * device_probe() on its parent device (e.g. PCI bridge devices).
> + * Test the flags again so that we don't mess up the device.
> + */
> + if (dev->flags & DM_FLAG_ACTIVATED)
> + return 0;
> +
> seq = uclass_resolve_seq(dev);
> if (seq < 0) {
> ret = seq;
> --
> 1.8.2.1
>

This seems problematic since presumably we reenter device_probe_child() in
this case. We then re-allocate the memory and so there is a memory leak.

Firstly I think this code should go inside the if (dev->parent) part.

Secondly I think we need a way to detect thie re-entry and do the right
thing. But it is pretty ugly - we essentially need to skip the memory
allocation. Maybe another flag? Ick.

Can you give me your thoughts on this and maybe we can work out a solution
that isn't too ugly. The expectation is that probing a parent should not
probe its children, so we need to deal with it as a special case. We should
probably also put an assert in to detect this problem.

Regards,
Simon