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

[Simon Glass]

Hi Caleb,

On Wed, 20 Nov 2024 at 08:51, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>
> Hi Simon, Marex,
>
> On 20/11/2024 16:36, 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.
> >
> > 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.
> >
> > For boards with SPL, the autoprobe is still done there as well.
> >
> > Note that with this change, autoprobe happens after the
> > EVT_DM_POST_INIT_R/F events are sent, rather than before.
> >
> > Link: https://lore.kernel.org/u-boot/20240626235717.272219-1-marex@denx.de/
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
>
> [...]
>
> > +For some platforms, certain devices must be probed to get the platform into
> > +a working state. To help with this, drivers marked with DM_FLAG_PROBE_AFTER_BIND
>
> I've found the documentation around this flag lacking, and the
> implementation confusing. Here you say /drivers/ with this flag (which
> would imply I can add it to the U_BOOT_DRIVER() definition, but further
> below the function doc says /device/ with this flag.
>
> In practise, it seems like the flag has to be added to the device in the
> bind() function, and adding it to the driver definition doesn't work
> (this left me scrataching my head for a while). The commit message
> adding it explains this and it seems like the idea was to have a way to
> trigger probing a device from the .bind() callback.
>
> I don't think I'm the only one confused by this, since grepping for it
> reveals two watchdog drivers which set this on their U_BOOT_DRIVER()
> definition.
>
> I can only see one driver that ever enables this flag conditionally,
> that's the ARM PSCI driver. Every other user sets it unconditionally in
> the .bind() callback.
>
> Would it make sense to rework this into a driver flag?
>
> Otherwise, could you document this behaviour more explicitly in this series?

I agree with you and I very-much want to document this better.

The original patch[1] came with no documentation nor tests. It was
applied within 5 days, after Sean raised exactly the same objections
that I have with this feature. Those objections were overruled and the
patch was applied anyway.

So here I am trying to clean this up.

As to the driver thing, my main concern is that it is too easy for
people to add the flag, causing a large slowdown in U-Boot's
pre-relocation startup speed. Perhaps the solution to that could be to
enable bootstage everywhere and report the time in the banner?

Regards,
Simon

[1] https://patchwork.ozlabs.org/project/uboot/patch/20220422131555.123598-1-marex@denx.de/