Hi Ulf,
On Wed, 30 Jul 2025 at 12:29, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> On Wed, 30 Jul 2025 at 11:56, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> > On Wed, 9 Jul 2025 at 13:31, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> > > On Tue, 1 Jul 2025 at 13:47, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> > > > Changes in v3:
> > > > - Added a couple of patches to adress problems on some Renesas
> > > > platforms. Thanks Geert and Tomi for helping out!
> > > > - Adressed a few comments from Saravanna and Konrad.
> > > > - Added some tested-by tags.
> > >
> > > I decided it was time to give this a try, so I have queued this up for
> > > v6.17 via the next branch at my pmdomain tree.
> > >
> > > If you encounter any issues, please let me know so I can help to fix them.
> >
> > Thanks for your series! Due to holidays, I only managed to test
> > this very recently.
> >
> > Unfortunately I have an issue with unused PM Domains no longer being
> > disabled on R-Car:
> > - On R-Car Gen1/2/3, using rcar-sysc.c, unused PM Domains are never
> > disabled.
> > - On R-Car Gen4, using rcar-gen4-sysc.c, unused PM Domains are
> > sometimes not disabled.
> > At first, I noticed the IOMMU driver was not enabled in my config,
> > and enabling it did fix the issue. However, after that I still
> > encountered the issue in a different config that does have the
> > IOMMU driver enabled...
> >
> > FTR, unused PM Domains are still disabled correctly on R/SH-Mobile
> > (using rmobile-sysc.c) and on BeagleBone Black. Note that these use
> > of_genpd_add_provider_simple(), while all R-Car drivers use
> > of_genpd_add_provider_onecell(). Perhaps there is an issue with
> > the latter? If you don't have a clue, I plan to do some more
> > investigation later...
of_genpd_add_provider_onecell() has:
if (!dev)
sync_state = true;
else
dev_set_drv_sync_state(dev, genpd_sync_state);
for (i = 0; i < data->num_domains; i++) {
...
if (sync_state && !genpd_is_no_sync_state(genpd)) {
genpd->sync_state = GENPD_SYNC_STATE_ONECELL;
device_set_node(&genpd->dev, fwnode);
sync_state = false;
^^^^^^^^^^^^^^^^^^^
}
...
}
As the R-Car SYSC drivers are not platform drivers, dev is NULL, and
genpd->sync_state is set to GENPD_SYNC_STATE_ONECELL for the first PM
Domain only. All other domains have the default value of sync_state
(0 = GENPD_SYNC_STATE_OFF). Hence when genpd_provider_sync_state()
is called later, it ignores all but the first domain.
Apparently this is intentional, as of_genpd_sync_state() tries to
power off all domains handled by the same controller anyway (see below)?
> > BTW, the "pending due to"-messages look weird to me.
> > On R-Car M2-W (r8a7791.dtsi) I see e.g.:
> >
> > genpd_provider ca15-cpu0: sync_state() pending due to e6020000.watchdog
> > renesas-cpg-mssr e6150000.clock-controller: sync_state() pending
> > due to e6020000.watchdog
> >
> > ca15-cpu0 is the PM Domain holding the first CPU core, while
> > the watchdog resides in the always-on Clock Domain, and uses the
> > clock-controller for PM_CLK handling.
Unfortunately the first PM Domain is "ca15-cpu0", which is blocked on
these bogus pending states, and no PM Domain is powered off.
If I remove the "sync_state = false" above, genpd_provider_sync_state()
considers all domains, and does power down all unused domains (even
multiple times, as expected).
Upon closer look, all "pending due to" messages I see claim that the
first (index 0) PM Domain is pending on some devices, while all of
these devices are part of a different domain (usually the always-on
domain, which is always the last (32 or 64) on R-Car).
So I think there are two issues:
1. Devices are not attributed to the correct PM Domain using
fw_devlink sync_state,
2. One PM Domain of a multi-domain controller being blocked should
not block all other domains handled by the same controller.
Does that make sense?
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
