From: Grant Likely <grant.likely@arm.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Saravana Kannan <saravanak@google.com>
Cc: a.hajda@samsung.com, artem.bityutskiy@linux.intel.com,
balbi@kernel.org, broonie@kernel.org, fntoth@gmail.com,
gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
linux-pm@vger.kernel.org, peter.ujfalusi@ti.com,
rafael@kernel.org, kernel-team@android.com, nd <nd@arm.com>
Subject: Re: [PATCH v3] driver core: Break infinite loop when deferred probe can't be satisfied
Date: Thu, 26 Mar 2020 15:20:27 +0000 [thread overview]
Message-ID: <2a5cb0a2-6236-d265-ae88-e0d0db427d38@arm.com> (raw)
In-Reply-To: <295d25de-f01e-26de-02d6-1ac0c149d828@arm.com>
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@linux.intel.com> 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.
next prev parent reply other threads:[~2020-03-26 15:21 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-24 17:57 [PATCH v3] driver core: Break infinite loop when deferred probe can't be satisfied Andy Shevchenko
2020-03-25 3:29 ` Saravana Kannan
2020-03-25 12:51 ` Andy Shevchenko
2020-03-25 22:08 ` Saravana Kannan
2020-03-26 8:39 ` Rafael J. Wysocki
2020-03-26 9:45 ` Peter Ujfalusi
2020-03-26 12:03 ` Andy Shevchenko
2020-03-26 13:45 ` Grant Likely
2020-03-26 14:23 ` Andy Shevchenko
2020-03-26 11:57 ` Andy Shevchenko
2020-03-26 13:48 ` Grant Likely
2020-03-26 18:45 ` Saravana Kannan
2020-03-26 11:54 ` Andy Shevchenko
2020-03-26 14:46 ` Grant Likely
2020-03-26 19:55 ` Saravana Kannan
2020-03-26 15:01 ` Grant Likely
2020-03-26 15:20 ` Grant Likely [this message]
2020-03-26 16:31 ` Andy Shevchenko
2020-03-26 16:39 ` Greg KH
2020-03-26 18:06 ` Grant Likely
2020-03-27 8:03 ` Greg KH
2020-03-27 12:37 ` Grant Likely
2020-03-27 12:51 ` Greg KH
2020-06-08 9:17 ` Marco Felsch
2020-06-08 11:11 ` Andrzej Hajda
2020-06-09 6:45 ` Marco Felsch
2020-06-09 7:30 ` Saravana Kannan
2020-06-09 9:27 ` Andrzej Hajda
2020-06-09 12:10 ` Marco Felsch
2020-06-09 13:02 ` Andrzej Hajda
2020-06-09 13:16 ` Mark Brown
2020-06-08 11:13 ` Andy Shevchenko
2020-06-08 11:59 ` Marco Felsch
2020-06-08 12:11 ` Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2a5cb0a2-6236-d265-ae88-e0d0db427d38@arm.com \
--to=grant.likely@arm.com \
--cc=a.hajda@samsung.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=artem.bityutskiy@linux.intel.com \
--cc=balbi@kernel.org \
--cc=broonie@kernel.org \
--cc=fntoth@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=kernel-team@android.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=nd@arm.com \
--cc=peter.ujfalusi@ti.com \
--cc=rafael@kernel.org \
--cc=saravanak@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).