Message-ID: <CAMuHMdXGS+efbbQ_Pn1iYhQ1aWc_DuJ-CBN=jxfjwOWxTRx+9Q@mail.gmail.com>
Date: Mon, 23 Jun 2025 17:06:42 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Ulf Hansson <ulf.hansson@...aro.org>
Cc: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>, Saravana Kannan <saravanak@...gle.com>, 
	Stephen Boyd <sboyd@...nel.org>, linux-pm@...r.kernel.org, 
	"Rafael J . Wysocki" <rafael@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Michael Grzeschik <m.grzeschik@...gutronix.de>, Bjorn Andersson <andersson@...nel.org>, 
	Abel Vesa <abel.vesa@...aro.org>, Peng Fan <peng.fan@....nxp.com>, 
	Johan Hovold <johan@...nel.org>, Maulik Shah <maulik.shah@....qualcomm.com>, 
	Michal Simek <michal.simek@....com>, Konrad Dybcio <konradybcio@...nel.org>, 
	Thierry Reding <thierry.reding@...il.com>, Jonathan Hunter <jonathanh@...dia.com>, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	Linux-Renesas <linux-renesas-soc@...r.kernel.org>
Subject: Re: [PATCH v2 00/21] pmdomain: Add generic ->sync_state() support to genpd

Hi Ulf,

On Mon, 23 Jun 2025 at 16:21, Ulf Hansson <ulf.hansson@...aro.org> wrote:
> On Thu, 19 Jun 2025 at 13:40, Ulf Hansson <ulf.hansson@...aro.org> wrote:
> > On Fri, 13 Jun 2025 at 12:33, Tomi Valkeinen
> > <tomi.valkeinen@...asonboard.com> wrote:
> > > On 23/05/2025 16:39, Ulf Hansson wrote:
> > > > Changes in v2:
> > > >       - Well, quite a lot as I discovered various problems when doing
> > > >       additional testing of corner-case. I suggest re-review from scratch,
> > > >       even if I decided to keep some reviewed-by tags.
> > > >       - Added patches to allow some drivers that needs to align or opt-out
> > > >       from the new common behaviour in genpd.
> > > >
> > > > If a PM domain (genpd) is powered-on during boot, there is probably a good
> > > > reason for it. Therefore it's known to be a bad idea to allow such genpd to be
> > > > powered-off before all of its consumer devices have been probed. This series
> > > > intends to fix this problem.
> > > >
> > > > We have been discussing these issues at LKML and at various Linux-conferences
> > > > in the past. I have therefore tried to include the people I can recall being
> > > > involved, but I may have forgotten some (my apologies), feel free to loop them
> > > > in.
> > > >
> > > > I have tested this with QEMU with a bunch of local test-drivers and DT nodes.
> > > > Let me know if you want me to share this code too.
> > > >
> > > > Please help review and test!
> > >
> > > I tested this Renesas white-hawk board, and it hangs at boot. With
> > > earlycon, I captured with/without boot logs, attached.
> > >
> > > The hang case doesn't look very healthy with all these: "kobject:
> > > '(null)' ((____ptrval____)): is not initialized, yet kobject_get() is
> > > being called."
> >
> > Tomi, thanks a lot for helping out with testing!
> >
> > rcar_gen4_sysc_pd_init() calls pm_genpd_init() and
> > of_genpd_add_provider_onecell().
> >
> > rcar_gen4_sysc_pd_init() is an early_initcall, which I guess is the
> > reason for these problems, as the genpd_provider_bus has not been
> > registered that early (it's done at core_initcall)
> >
> > Do you think it would be possible to move rcar_gen4_sysc_pd_init() to
> > a postcore/arch_initcall?
>
> I did some investigation around this and found that both
> drivers/pmdomain/renesas/rcar-gen4-sysc.c and
> drivers/pmdomain/renesas/rcar-sysc.c are registering their genpd
> providers at the early_initcall() level.
>
> I was trying to find (by browsing renesas DTSes and looking into
> drivers) if there is any consumers that actually relies on this, but
> so far the earliest consumer I have found is the
> drivers/irqchip/irq-renesas-irqc.c, but that's at postcore_initcall().
> Of course, it's difficult to say if my analysis is complete as there
> are a lot of platform variants and I didn't check them all.
>
> Maybe we should just give it a try and move both two drivers above to
> postcore_initcall and see if it works (assuming the irq-renesas-irqc
> supports -EPROBE_DEFER correctly too).
>
> If this doesn't work, I think we need to find a way to allow deferring
> the call to device_add() in of_genpd_provider_add*() for genpd
> provider's devices.

Commit dcc09fd143bb97c2 ("soc: renesas: rcar-sysc: Add DT support for
SYSC PM domains") explains:

   "Initialization is done from an early_initcall(), to make sure the PM
    Domains are initialized before secondary CPU bringup."

but that matters only for arm32 systems (R-Car Gen1 and Gen2).
Arm64 systems (R-Car Gen3 and Gen4) use PSCI for CPU PM Domain control.

While changing rcar-sysc.c to use a postcore_initcall indeed moves PM
Domain initialization after secondary CPU bringup, the second CPU core
on R-Car M2-W is still brought up fine.

For R-Car H1, there is a regression:

    smp: Bringing up secondary CPUs ...
    CPU1: failed to boot: -19
    CPU2: failed to boot: -19
    CPU3: failed to boot: -19
    smp: Brought up 1 node, 1 CPU
    SMP: Total of 1 processors activated (500.00 BogoMIPS).

CPU bringup/teardown in userspace using
/sys/devices/system/cpu/cpu*/online still works.
R-Car H1 was never converted to use "enable-method" in DT, and relies
on calling into the rcar-sysc driver directly (see [1]).  However,
that does not use any actual calls into the genpd core, so probably it
can be made to work by splitting rcar_sysc_pd_init() in two parts: an
early_initcall() that allocates all domain structures and populates the
internal hierarchy, and a postcore_initcall() that registers everything
with the genpd core.

As expected, there is no impact on R-Car H3 ES2.0.
I will test on R-Car V4M tomorrow, but expect no issues.

FTR, drivers/pmdomain/renesas/rmobile-sysc.c uses core_initcall().
Changing that to postcore_initcall does not seem to make a difference
(on R-Mobile A1).

[1] https://elixir.bootlin.com/linux/v6.15.3/source/drivers/pmdomain/renesas/rcar-sysc.c#L439

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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

Powered by blists - more mailing lists