Hi Caleb, On Wed, 20 Nov 2024 at 08:51, Caleb Connolly <caleb.conno...@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-ma...@denx.de/ > > > > Signed-off-by: Simon Glass <s...@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-ma...@denx.de/
