Hi Michal,
On Thu, 31 Jan 2019 at 03:28, Michal Simek <michal.simek at xilinx.com> wrote:
>
> 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.
Yes indeed, it is just saving a function call.
Regards,
Simon