Hi Simon,
On Mon, Aug 24, 2015 at 9:04 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 23 August 2015 at 02:32, Bin Meng bmeng.cn@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@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.
Ah, yes! 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.
I think we can just test these to see if they are NULL? Like this:
/* Allocate private data if requested */ if (drv->priv_auto_alloc_size && !dev->priv) { dev->priv = alloc_priv(drv->priv_auto_alloc_size, drv->flags); if (!dev->priv) { ret = -ENOMEM; goto fail; } }
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.
If probing a parent should not probe its children, I am afraid the pci-uclass driver needs rewritten.
Regards, Bin