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

[Simon Glass]

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:

>                 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().

> diff --git a/drivers/core/dump.c b/drivers/core/dump.c
> index 8fbfd93fb5e4..95ba7dcb9193 100644
> --- a/drivers/core/dump.c
> +++ b/drivers/core/dump.c
> @@ -62,7 +62,7 @@ void dm_dump_all(void)
>   *
>   * @dev:       Device to display
>   */
> -static void dm_display_line(struct udevice *dev, int index)
> +void dm_display_line(struct udevice *dev, int index)
>  {
>         printf("%i %c %s @ %08lx", index,
>                dev->flags & DM_FLAG_ACTIVATED ? '*' : ' ',
> --
> 1.9.1
>

Regards,
Simon