Re: [PATCH v3] driver core: Break infinite loop when deferred probe can't be satisfied
From: Grant Likely
Date: Thu Mar 26 2020 - 11:21:01 EST
On 26/03/2020 15:01, Grant Likely wrote:
On 25/03/2020 12:51, Andy Shevchenko wrote:
On Tue, Mar 24, 2020 at 08:29:01PM -0700, Saravana Kannan wrote:
On Tue, Mar 24, 2020 at 5:38 AM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
[...]
...but extcon driver is still missing...
[ÂÂ 22.283174] platform dwc3.0.auto: Added to deferred list
[ÂÂ 22.288513] platform dwc3.0.auto:
driver_deferred_probe_add_trigger local counter: 1 new counter 2
I'm not fully aware of all the USB implications, but if extcon is
needed, why can't that check be done before we add and probe the ulpi
device? That'll avoid this whole "fake" probing and avoid the counter
increase. And avoid the need for this patch that's touching the code
code that's already a bit delicate.
Also, with my limited experience with all the possible drivers in the
kernel, it's weird that the ulpi device is added and probed before we
make sure the parent device (dwc3.0.auto) can actually probe
successfully.
As I said above the deferred probe trigger has flaw on its own.
Even if we fix for USB case, there is (and probably will be) others.
Right here is the driver design bug. A driver's probe() hook should
*not* return -EPROBE_DEFER after already creating child devices which
may have already been probed.
It can be solved by refactoring the driver probe routine. If a resource
is required to be present, then check that it is available early; before
registering child devices.
If it is difficult to determine whether extcon is available before
creating the child devices, then there is a way to solve it that still
leverages the driver core. You could refactor dwc3_core_init_mode() into
a set of separate probe() routines, one for each mode. dwc3_probe()
could register the matching child device just before it exits. Then the
driver core will take care of calling it again at a later point in time
without tearing down the ulpi device probe.
Cheers,
g.