| Message ID | 20200225050828.56458-1-john.stultz@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | driver core: Improve and cleanup driver_deferred_probe_check_state() | expand |
On Tue, Feb 25, 2020 at 05:08:22AM +0000, John Stultz wrote: > This series goal is to improve and cleanup the > driver_deferred_probe_check_state() code in the driver core. > > This series is useful for being able to support modules > dependencies which may be loaded by userland, far after > late_initcall is done. For instance, this series allows us to > successfully use various clk drivers as modules on the db845c > board. And without it, those drivers have to be statically built > in to work. > > Since I first sent out this patch, Saravana suggested an > alternative approach which also works for our needs, and is a > bit simpler: > https://lore.kernel.org/lkml/20200220055250.196456-1-saravanak@google.com/T/#u > > However, while that patch provides the functionality we need, > I still suspect the driver_deferred_probe_check_state() code > could benefit from the cleanup in this patch, as the existing > logic is somewhat muddy. This looks much better, thanks for sticking with it, all now queued up. greg k-h
Hi John, Cc: linux-renesas-soc On Tue, Feb 25, 2020 at 05:08:22AM +0000, John Stultz wrote: > This series goal is to improve and cleanup the > driver_deferred_probe_check_state() code in the driver core. > > This series is useful for being able to support modules > dependencies which may be loaded by userland, far after > late_initcall is done. For instance, this series allows us to > successfully use various clk drivers as modules on the db845c > board. And without it, those drivers have to be statically built > in to work. > > Since I first sent out this patch, Saravana suggested an > alternative approach which also works for our needs, and is a > bit simpler: > https://lore.kernel.org/lkml/20200220055250.196456-1-saravanak@google.com/T/#u > > However, while that patch provides the functionality we need, > I still suspect the driver_deferred_probe_check_state() code > could benefit from the cleanup in this patch, as the existing > logic is somewhat muddy. > > New in v5: > * Reworked the driver_deferred_probe_check_state() logic as > suggested by Saravana to tie the initcall_done checking with > modules being enabled. > * Cleanup some comment wording as suggested by Rafael > * Try to slightly simplify the regulator logic as suggested by > Bjorn > > Thanks so much to Bjorn, Saravana and Rafael for their reviews > and suggestions! Additional review and feedback is always greatly > appreciated! Building a recent [0] kernel using vanilla arm64 defconfig and booting it on H3ULCB, I get buried into backtraces [1]. After reverting this series, up to and including its first commit, booting goes back to normal [2]. Any chance to get a fix or at least some hints where to dig into? [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=18bf34080c4c3b ("Merge branch 'akpm' (patches from Andrew)") [1] https://gist.github.com/erosca/ac779c348dd272c448e162c406c48f4a [2] https://gist.github.com/erosca/5eea2bc5e82be651d405ba038d0ad036
On Tue, Apr 21, 2020 at 4:59 PM Eugeniu Rosca <erosca@de.adit-jv.com> wrote: > > Hi John, > Cc: linux-renesas-soc > > On Tue, Feb 25, 2020 at 05:08:22AM +0000, John Stultz wrote: > > This series goal is to improve and cleanup the > > driver_deferred_probe_check_state() code in the driver core. > > > > This series is useful for being able to support modules > > dependencies which may be loaded by userland, far after > > late_initcall is done. For instance, this series allows us to > > successfully use various clk drivers as modules on the db845c > > board. And without it, those drivers have to be statically built > > in to work. > > > > Since I first sent out this patch, Saravana suggested an > > alternative approach which also works for our needs, and is a > > bit simpler: > > https://lore.kernel.org/lkml/20200220055250.196456-1-saravanak@google.com/T/#u > > > > However, while that patch provides the functionality we need, > > I still suspect the driver_deferred_probe_check_state() code > > could benefit from the cleanup in this patch, as the existing > > logic is somewhat muddy. > > > > New in v5: > > * Reworked the driver_deferred_probe_check_state() logic as > > suggested by Saravana to tie the initcall_done checking with > > modules being enabled. > > * Cleanup some comment wording as suggested by Rafael > > * Try to slightly simplify the regulator logic as suggested by > > Bjorn > > > > Thanks so much to Bjorn, Saravana and Rafael for their reviews > > and suggestions! Additional review and feedback is always greatly > > appreciated! > > Building a recent [0] kernel using vanilla arm64 defconfig > and booting it on H3ULCB, I get buried into backtraces [1]. > > After reverting this series, up to and including its first commit, > booting goes back to normal [2]. > > Any chance to get a fix or at least some hints where to dig into? Yea. There's two patch sets I have for this. The first quiets down the warnings(we don't need stack dumps for these): https://lore.kernel.org/lkml/20200330202715.86609-1-john.stultz@linaro.org/ The second reverts the default timeout back to 0: https://lore.kernel.org/lkml/20200413204253.84991-1-john.stultz@linaro.org/ Let me know if those work for you, or if you're still having trouble afterwards. I need to resubmit the set as I'm guessing they've been overlooked. thanks -john
Hi John, On Tue, Apr 21, 2020 at 06:16:31PM -0700, John Stultz wrote: > On Tue, Apr 21, 2020 at 4:59 PM Eugeniu Rosca <erosca@de.adit-jv.com> wrote: > > > > Building a recent [0] kernel using vanilla arm64 defconfig > > and booting it on H3ULCB, I get buried into backtraces [1]. > > > > After reverting this series, up to and including its first commit, > > booting goes back to normal [2]. > > > > Any chance to get a fix or at least some hints where to dig into? > > Yea. There's two patch sets I have for this. The first quiets down the > warnings(we don't need stack dumps for these): > https://lore.kernel.org/lkml/20200330202715.86609-1-john.stultz@linaro.org/ > > The second reverts the default timeout back to 0: > https://lore.kernel.org/lkml/20200413204253.84991-1-john.stultz@linaro.org/ > > > Let me know if those work for you, or if you're still having trouble > afterwards. I need to resubmit the set as I'm guessing they've been > overlooked. The patches fix the issue on my end. Thanks! One note is that they carry a slight mutual conflict, but that's up to the maintainer to complain about. Many thanks again! Hope the patches will be picked up soon!
On Tue, Apr 21, 2020 at 06:16:31PM -0700, John Stultz wrote: > The second reverts the default timeout back to 0: > https://lore.kernel.org/lkml/20200413204253.84991-1-john.stultz@linaro.org/ If you're reverting the timeout we should revert the regulator change too I think.
On Wed, Apr 22, 2020 at 12:54 AM Mark Brown <broonie@kernel.org> wrote: > On Tue, Apr 21, 2020 at 06:16:31PM -0700, John Stultz wrote: > > > The second reverts the default timeout back to 0: > > https://lore.kernel.org/lkml/20200413204253.84991-1-john.stultz@linaro.org/ > > If you're reverting the timeout we should revert the regulator change > too I think. Maybe? The main issue for me was my change was clearly breaking users with dts with missing dependencies where their setup was working before. I sort of feel like having a dtb with missing dependencies is less valid than wanting to load module dependencies from userland, but they were working first, so we have to keep them happy :) And at least now the latter can add the timeout boot argument to make it work. For your case, I'm not sure if the timeout would run afoul on the nfs root mounting case this one tripped over. thanks -john
Hi John, On Wed, Apr 22, 2020 at 10:46 PM John Stultz <john.stultz@linaro.org> wrote: > On Wed, Apr 22, 2020 at 12:54 AM Mark Brown <broonie@kernel.org> wrote: > > On Tue, Apr 21, 2020 at 06:16:31PM -0700, John Stultz wrote: > > > The second reverts the default timeout back to 0: > > > https://lore.kernel.org/lkml/20200413204253.84991-1-john.stultz@linaro.org/ > > > > If you're reverting the timeout we should revert the regulator change > > too I think. > > Maybe? The main issue for me was my change was clearly breaking users > with dts with missing dependencies where their setup was working > before. I sort of feel like having a dtb with missing dependencies is > less valid than wanting to load module dependencies from userland, but > they were working first, so we have to keep them happy :) And at least > now the latter can add the timeout boot argument to make it work. IOMMU support is optional. Gr{oetje,eeting}s, Geert
On Wed, Apr 22, 2020 at 01:45:49PM -0700, John Stultz wrote: > On Wed, Apr 22, 2020 at 12:54 AM Mark Brown <broonie@kernel.org> wrote: > > If you're reverting the timeout we should revert the regulator change > > too I think. > Maybe? The main issue for me was my change was clearly breaking users > with dts with missing dependencies where their setup was working > before. I sort of feel like having a dtb with missing dependencies is > less valid than wanting to load module dependencies from userland, but > they were working first, so we have to keep them happy :) And at least > now the latter can add the timeout boot argument to make it work. > For your case, I'm not sure if the timeout would run afoul on the nfs > root mounting case this one tripped over. Given that it's basically entirely about glitching displays rather than an unrecoverable break I suspect that anyone using NFS root is some combination of unaffected or doesn't care if they see the timeout kick in.
This series goal is to improve and cleanup the driver_deferred_probe_check_state() code in the driver core. This series is useful for being able to support modules dependencies which may be loaded by userland, far after late_initcall is done. For instance, this series allows us to successfully use various clk drivers as modules on the db845c board. And without it, those drivers have to be statically built in to work. Since I first sent out this patch, Saravana suggested an alternative approach which also works for our needs, and is a bit simpler: https://lore.kernel.org/lkml/20200220055250.196456-1-saravanak@google.com/T/#u However, while that patch provides the functionality we need, I still suspect the driver_deferred_probe_check_state() code could benefit from the cleanup in this patch, as the existing logic is somewhat muddy. New in v5: * Reworked the driver_deferred_probe_check_state() logic as suggested by Saravana to tie the initcall_done checking with modules being enabled. * Cleanup some comment wording as suggested by Rafael * Try to slightly simplify the regulator logic as suggested by Bjorn Thanks so much to Bjorn, Saravana and Rafael for their reviews and suggestions! Additional review and feedback is always greatly appreciated! thanks -john Cc: Rob Herring <robh@kernel.org> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> Cc: Kevin Hilman <khilman@kernel.org> Cc: Ulf Hansson <ulf.hansson@linaro.org> Cc: Pavel Machek <pavel@ucw.cz> Cc: Len Brown <len.brown@intel.com> Cc: Todd Kjos <tkjos@google.com> Cc: Saravana Kannan <saravanak@google.com> Cc: Bjorn Andersson <bjorn.andersson@linaro.org> Cc: Liam Girdwood <lgirdwood@gmail.com> Cc: Mark Brown <broonie@kernel.org> Cc: Thierry Reding <treding@nvidia.com> Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: linux-pm@vger.kernel.org John Stultz (6): driver core: Fix driver_deferred_probe_check_state() logic driver core: Set deferred_probe_timeout to a longer default if CONFIG_MODULES is set pinctrl: Remove use of driver_deferred_probe_check_state_continue() driver core: Remove driver_deferred_probe_check_state_continue() driver core: Rename deferred_probe_timeout and make it global regulator: Use driver_deferred_probe_timeout for regulator_init_complete_work drivers/base/dd.c | 82 +++++++++++++---------------------- drivers/pinctrl/devicetree.c | 9 ++-- drivers/regulator/core.c | 25 ++++++----- include/linux/device/driver.h | 2 +- 4 files changed, 49 insertions(+), 69 deletions(-)