Re: [PATCH v3 1/2] driver core: Rework logic in __driver_deferred_probe_check_state to allow EPROBE_DEFER to be returned for longer

[Saravana Kannan]

On Thu, Feb 20, 2020 at 2:09 AM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
>
> On Thu, Feb 20, 2020 at 7:29 AM Saravana Kannan <saravanak@xxxxxxxxxx> wrote:
> > On Tue, Feb 18, 2020 at 4:07 PM John Stultz wrote:
> > > Due to commit e01afc3250255 ("PM / Domains: Stop deferring probe
> > > at the end of initcall"), along with commit 25b4e70dcce9
> > > ("driver core: allow stopping deferred probe after init") after
> > > late_initcall, drivers will stop getting EPROBE_DEFER, and
> > > instead see an error causing the driver to fail to load.
> >
> > Both of those patches were the best solution at that point in time. But
> > the kernel has changed a lot since then. Power domain and IOMMU drivers
> > can work as modules now. We have of_devlink and sync_state().
> >
> > So, while a delay might have been the ideal solution back then, I think
> > we need to work towards removing arbitrary timeouts instead of making
> > the timeout mechanism more elaborate.
> >
> > I think driver_deferred_probe_check_state() logic should boiled down
> > to something like:
>
> ...
>
> > Once we add of_devlink support for power-domains, you won't even hit the
> > genpd error path if you have of_devlink enabled. I believe in the case
> > you are testing DB 845c, of_devlink is enabled?
> >
> > If of_devlink is enabled, the devices depending on the unprobed power
> > domains would be deferred without even calling the driver's probe()
> > function.
> >
> > Adding power-domain support to of_devlink is a 2 line change. I'll send
> > it out soon.
>
> ...
>
> Pardon me for not knowing the OF and devlink stuff in particular, but
> have you had a chance to test your changes on some (rather complex)
> ACPI enabled platforms?
> Would it have any complications there?

Hi Andy,

I'm not sure which changes you are referring to. So I'll try to answer
all possibilities :)

If you are referring to the pseudo code proposal, it's mostly
narrowing down the conditions under which the timeout will matter.
1. It's ignoring the timeout stuff if modules are not supported and
all initcall levels are done.
2. If modules are enabled and of_devlink is enabled, it's making sure
the timeout doesn't apply.

(1) seems like a straightforward assumption. (2) is functionally a NOP
for ACPI based platforms. So I don't think we'd have ACPI specific
complications here.

If you were referring to the "of_devlink support for power-domains"
that also won't have any impact on ACPI platforms because right not it
only runs for OF based systems. So again a NOP for ACPI.

Hope that answers your questions.

Thanks,
Saravana