Hi Ulf,
On Thu, Nov 21, 2019 at 11:30 AM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> On Thu, 21 Nov 2019 at 10:35, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> > On Thu, Nov 21, 2019 at 9:57 AM Wolfram Sang <wsa@xxxxxxxxxxxxx> wrote:
> > > > So some of my local code on top must have impacted the behavior.
> > >
> > > Any change in temperature? Niklas and I wonder if it is thermal related.
> >
> > Nope. I tried an old "known" good kernel again yesterday, and it worked.
> > That was BTW the one which had the additional debug prints:
> >
> > --- a/drivers/base/power/domain.c
> > +++ b/drivers/base/power/domain.c
> > @@ -199,6 +199,7 @@ static struct generic_pm_domain
> > *dev_to_genpd(struct device *dev)
> > static int genpd_stop_dev(const struct generic_pm_domain *genpd,
> > struct device *dev)
> > {
> > +pr_info("==== %s/%s: stop\n", genpd->name, dev_name(dev));
> > WARN(device_may_wakeup(dev),
> > "Domain %s must be active_wakeup for wakeup source %s\n",
> > genpd->name, dev_name(dev));
> > @@ -208,6 +209,7 @@ static int genpd_stop_dev(const struct
> > generic_pm_domain *genpd,
> > static int genpd_start_dev(const struct generic_pm_domain *genpd,
> > struct device *dev)
> > {
> > +pr_info("==== %s/%s: start\n", genpd->name, dev_name(dev));
> > return GENPD_DEV_CALLBACK(genpd, int, start, dev);
> > }
> >
> > Removing those prints made the old kernel fail, too, so this is why I think
> > it is a race with Runtime PM.
> >
> > With a tree based on latest renesas-drivers, it happens regardless of those
> > debug prints.
> >
> > > > > I am working on an issue where the SCC hangs, but this has to do with
> > > > > always providing the SCC clock (SDnH). I don't really see the connection
> > > > > of that to RuntimePM yet, though :/
> A few comments around your runtime PM concerns, not sure if this
> matters to you issues.
>
> So, only by looking at the code for probing of the tmio variant
> drivers, it seems like there are accesses (both reads and writes) for
> the device being done, without first making sure that the clock
> managed by the PM domain has been enabled. Is that really okay? Note,
> this isn't a new thing, but it has been there as long as can remember.
No, that is not OK.
On R-Car Gen2, my debug code that disables all non-critical module
clocks during early boot should have caught them.
On R-Car Gen3, it's different, unfortunately, as apparently not all
module clocks can be disabled (protected by secure code?).
So my debug code has limited use on those platforms.
Note that the SCC seems to be clocked by a normal clock (SDnH), not by a
module clock, so it's not controlled by Runtime PM.
In fact, without "[PATCH] WIP: clk: renesas: rcar-gen3: enable SDnH clk
for HS modes", Linux doesn't enable it at all.
BTW, perhaps U-Boot leaves the SCC in a different state on R-Car E3
than on M3-N?
> For example, in renesas_sdhi_probe() there are calls made to
> sd_ctrl_write16|read16() before calling tmio_mmc_host_probe().
>
> Additionally in tmio_mmc_host_probe(), there are calls made to
> sd_ctrl_write16|read16(), before calling pm_runtime_get_sync().
Ugh.
> If you haven't noticed, we have also managed to replace the call with
> pm_runtime_get_sync() with a call to dev_pm_domain_start(), to
> simplify the code. The point is, these changes are queued via Rafael's
> pm-tree for v5.5.
Thanks, I hadn't noticed that.
I do have pm/linux-next in renesas-drivers, so that code has been exercised.
> So, perhaps at this point we should simply drop the offending commit
> and revisit this once v5.5-rc1 is out?
Yes, that looks like the best short-term solution.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
