[PATCH 3/3] common: Move autoprobe out to board init

[Simon Glass]

Hi Jonas,

On Mon, 21 Oct 2024 at 18:10, Jonas Karlman <jonas at kwiboo.se> wrote:
>
> Hi Simon,
>
> On 2024-10-21 17:51, Simon Glass wrote:
> > Rather than doing autoprobe within the driver model code, move it out to
> > the board-init code. This makes it clear that it is a separate step from
> > binding devices.
>
> It could probably be good to mention that after this patch the events
> EVT_DM_POST_INIT_R/F will happen before autoprobe instead of after.

Yes, good point.

>
> >
> > For now this is always done twice, before and after relocation, but we
> > should discuss whether it might be possible to drop the post-relocation
> > probe.
>
> If you have plans to drop post-relocation probe, I suggest you first
> look into having OF_LIVE working at pre-reloaction to speed up boot, the
> use of fdtdec is super slow in getting parent node/offset.

Indeed. I keep hoping someone else will look at this.

>
> >
> > This patch also drops autoprobe from SPL. If that is needed, we could
> > add a call in SPL.
>
> This patch will break a feature used by the Pine64 PineTab2 because of
> this change, please restore autoprobe in SPL.
>
> The PineTab2 have a feature to power itself off when power-on happen
> because of power cable was plugged in, for this to work it currently
> depends on DM probe after bind works in SPL.

OK. We can always make it optional later. At least with this series it
is explicit.

Regards,
Simon

>
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> >  common/board_f.c    |  4 ++++
> >  common/board_r.c    |  4 ++++
> >  drivers/core/root.c | 17 ++++++++++++++---
> >  include/dm/root.h   | 10 ++++++++++
> >  4 files changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/common/board_f.c b/common/board_f.c
> > index 8684ed71284..6564b2ac2f7 100644
> > --- a/common/board_f.c
> > +++ b/common/board_f.c
> > @@ -822,6 +822,10 @@ static int initf_dm(void)
> >       if (ret)
> >               return ret;
> >
> > +     ret = dm_autoprobe();
> > +     if (ret)
> > +             return ret;
> > +
> >       if (IS_ENABLED(CONFIG_TIMER_EARLY)) {
> >               ret = dm_timer_init();
> >               if (ret)
> > diff --git a/common/board_r.c b/common/board_r.c
> > index e5f33f40643..a2948b7b55c 100644
> > --- a/common/board_r.c
> > +++ b/common/board_r.c
> > @@ -243,6 +243,10 @@ static int initr_dm(void)
> >       if (ret)
> >               return ret;
> >
> > +     ret = dm_autoprobe();
> > +     if (ret)
> > +             return ret;
> > +
> >       return 0;
> >  }
> >  #endif
> > diff --git a/drivers/core/root.c b/drivers/core/root.c
> > index 2d4f078f97f..f6bda84b2b9 100644
> > --- a/drivers/core/root.c
> > +++ b/drivers/core/root.c
> > @@ -281,7 +281,7 @@ void *dm_priv_to_rw(void *priv)
> >  }
> >  #endif
> >
> > -static int dm_probe_devices(struct udevice *dev)
> > +static int dm_probe_devices_(struct udevice *dev)
>
> Why is this function renamed?

It's not needed, since I decided on a different name for the function.
I'll change it back.

Thank you for the review on this.

I am hoping Marek will chime in too!

[..]

Regards,
Simon