Re: [PATCH v5 0/6] driver core: Improve and cleanup driver_deferred_probe_check_state()

Re: [PATCH v5 0/6] driver core: Improve and cleanup driver_deferred_probe_check_state()

[Eugeniu Rosca]

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

-- 
Best regards,
Eugeniu Rosca

Re: [PATCH v5 0/6] driver core: Improve and cleanup driver_deferred_probe_check_state()

[John Stultz]

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

Re: [PATCH v5 0/6] driver core: Improve and cleanup driver_deferred_probe_check_state()

[Eugeniu Rosca]

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!

-- 
Best regards,
Eugeniu Rosca

Re: [PATCH v5 0/6] driver core: Improve and cleanup driver_deferred_probe_check_state()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 278 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v5 0/6] driver core: Improve and cleanup driver_deferred_probe_check_state()

[John Stultz]

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

Re: [PATCH v5 0/6] driver core: Improve and cleanup driver_deferred_probe_check_state()

[Geert Uytterhoeven]

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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v5 0/6] driver core: Improve and cleanup driver_deferred_probe_check_state()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 986 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]