[U-Boot] [RFC PATCH] dm: device: Do not probe parents which are probed already

[Michal Simek]

On 31. 01. 19 11:04, Simon Glass wrote:
> Hi Michal,
> 
> On Fri, 18 Jan 2019 at 02:41, Michal Simek <michal.simek at xilinx.com> wrote:
>>
>> From the first look there is no reason to probe parent nodes if they are
>> active already.
>>
>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>> ---
>>
>> I have created this just for showing status of parent device.
>> Maybe there is any strong reason to do this but I just wanted to check
>> this because it looks like just wasting of time.
>>
>> Just revert this condition when you want to see outputs.
>> if (dev->parent && !(dev->parent->flags & DM_FLAG_ACTIVATED)) {
>>
>> Without this line
>>
>> 99   amba @ 7df04d20
>> 100   amba @ 7df04d20
>> 99 * root_driver @ 7df04960, seq 0, (req -1)
>> 100 * root_driver @ 7df04960, seq 0, (req -1)
>> 99 * root_driver @ 7df04960, seq 0, (req -1)
>> 100 * root_driver @ 7df04960, seq 0, (req -1)
>> 99 * root_driver @ 7df04960, seq 0, (req -1)
>> 100 * root_driver @ 7df04960, seq 0, (req -1)
>> 99 * root_driver @ 7df04960, seq 0, (req -1)
>> 100 * root_driver @ 7df04960, seq 0, (req -1)
>> 99 * root_driver @ 7df04960, seq 0, (req -1)
>> 100 * root_driver @ 7df04960, seq 0, (req -1)
>> 99 * root_driver @ 7df04960, seq 0, (req -1)
>> 100 * root_driver @ 7df04960, seq 0, (req -1)
>> 99 * root_driver @ 7df04960, seq 0, (req -1)
>> 100 * root_driver @ 7df04960, seq 0, (req -1)
>> MMC:   99 * amba @ 7df04d20, seq 0, (req -1)
>> 100 * amba @ 7df04d20, seq 0, (req -1)
>>
>> ZynqMP> i2c dev 0
>> Setting bus to 0
>> 99 * amba @ 7df04d20, seq 0, (req -1)
>> 100 * amba @ 7df04d20, seq 0, (req -1)
>>
>> with this line added
>>
>> 99   amba @ 7df04d20
>> 100   amba @ 7df04d20
>> 99 * root_driver @ 7df04960, seq 0, (req -1)
>> 99 * root_driver @ 7df04960, seq 0, (req -1)
>> 99 * root_driver @ 7df04960, seq 0, (req -1)
>> 99 * root_driver @ 7df04960, seq 0, (req -1)
>> 99 * root_driver @ 7df04960, seq 0, (req -1)
>> 99 * root_driver @ 7df04960, seq 0, (req -1)
>> 99 * root_driver @ 7df04960, seq 0, (req -1)
>> MMC:   99 * amba @ 7df04d20, seq 0, (req -1)
>>
>> ZynqMP> i2c dev 0
>> Setting bus to 0
>> 99 * amba @ 7df04d20, seq 0, (req -1)
>>
>> ---
>>  drivers/core/device.c | 7 ++++++-
>>  drivers/core/dump.c   | 2 +-
>>  2 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/core/device.c b/drivers/core/device.c
>> index 0d15e5062b66..114888a8f7cf 100644
>> --- a/drivers/core/device.c
>> +++ b/drivers/core/device.c
>> @@ -341,8 +341,13 @@ int device_probe(struct udevice *dev)
>>                 }
>>         }
>>
>> +       if (dev->parent)
>> +               dm_display_line(dev->parent, 99);
>> +
>>         /* Ensure all parents are probed */
>> -       if (dev->parent) {
>> +       if (dev->parent && !(dev->parent->flags & DM_FLAG_ACTIVATED)) {
>> +               dm_display_line(dev->parent, 100);
>> +
> 
> Yes this looks like a good change in principle.
> 
> But we still need to execute the code below even if the parent is
> probed, so that we allocate the child's parent data:

ok.


>>                 size = dev->parent->driver->per_child_auto_alloc_size;
>>                 if (!size) {
>>                         size = dev->parent->uclass->uc_drv->
> 
> ...
> 
> So can you please rework this to allow for that?
> 
> Overall I think your change saves a function call. As you can see the
> flag is checked right at the top of device_probe().

I am not quite sure how that rework should look like.

If just this.
if (!(dev->parent->flags & DM_FLAG_ACTIVATED))
	ret = device_probe(dev->parent);
	if (ret)
		goto fail;


Then improvement will be very minimal.

Thanks,
Michal