Message-ID: <1953725.CQOukoFCf9@workhorse>
Date: Mon, 08 Sep 2025 15:14:28 +0200
From: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
To: Ulf Hansson <ulf.hansson@...aro.org>
Cc: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
 kernel@...labora.com, linux-rockchip@...ts.infradead.org,
 Cristian Ciocaltea <cristian.ciocaltea@...labora.com>,
 Sebastian Reichel <sebastian.reichel@...labora.com>,
 Heiko Stuebner <heiko@...ech.de>
Subject: Re: [PATCH] pmdomian: core: don't unset stay_on during sync_state

On Friday, 5 September 2025 16:27:27 Central European Summer Time Ulf Hansson wrote:
> [...]
> 
> >
> > Okay so I believe I have found the root cause of the regression. UFS is
> > innocent, disabling UFS just happens to avoid it due to how the timing of
> > things works out.
> >
> > The real issue is that the NPU power domains on the RK3576, which are
> > currently unused, have an undeclared dependency on vdd_npu_s0.
> >
> > Declaring this dependency with a `domain-supply` and adding the
> > necessary flag in the rockchip PD controller to use it does not solve
> > he problem. This is because the rockchip PD controller cannot acquire
> > those supplies during probe, as they're not available yet and their
> > availability depends on the PD controller finishing probe.
> >
> > That's why it acquires them in the PD enable callback, but the NPU
> > PDs are never enabled because they're unused.
> >
> > This worked fine when unused PDs were still turned off quite early, as
> > this meant they were turned off before regulators. Now the unused
> > regulators are turned off before turning off the unused PDs happens.
> 
> I see, thanks for sharing these details. What a mess.
> 

Agreed :(

> >
> > I don't really see an easy way to fix this with a patch that's fit for
> > an rc cycle. We can't request the regulator early or even just add a
> > device link, as the regulator is not around yet.
> 
> Right, I will work on a patch or two that allows rockchip
> power-domains to opt-out from genpds new behavior and to keep using
> the old one.
> 
> I think we prefer to do it like this (should be quite a limited amount
> of code and okay for an rc), rather than reverting for everyone.

That sounds good to me.

> 
> >
> > Marking vdd_npu_s0 as always-on would be abusing DT to work around a
> > Linux kernel shortcoming, which is a no-no.
> >
> > What we need is either a way to register with pmdomain core that
> > certain PDs need a late init for additional supplies, which is then
> > called before any of the unused regulator power off functionality is
> > invoked by the regulator core.
> >
> > Any ideas?
> 
> Yes :-)
> 
> I would suggest implementing an auxiliary driver, along with the
> rockchip_pm_domain_driver. The main job for the auxiliary driver would
> be to get the regulator in its ->probe() - and if it fails because the
> regulator isn't available yet, it should keep trying by returning
> -EPROBE_DEFER. See more about the auxiliary bus/device/driver in
> include/linux/auxiliary_bus.h and module_auxiliary_driver().
> 
> Moreover, when the rockchip_pm_domain_driver probes, it becomes
> responsible for pre-parsing the OF nodes for the domain-supply DT
> property, for each of the specified power-domains. If it finds a
> domain-supply, it should register an auxiliary device that corresponds
> to that particular power-domain. This can be done by using
> platform-data that is shared with the auxiliary device/driver. See
> devm_auxiliary_device_create().
> 
> Furthermore we would need some kind of synchronization mechanism
> between the rockchip_pm_domain_driver and the auxiliary driver, to
> manage the regulator get/enable/disable. I think that should be rather
> easy to work out.
> 
> Do you think this can work?

This sounds similar to something Heiko suggested to me, and I agree
it could work. It does seem like a pretty painful solution though,
in terms of needed additional code and complexity to basically just
tell Linux "hey you can't get this regulator yet but please try
again later without our involvement".

To that end, I've tried working out a regulator-based solution to
it, where the rockchip_pm_domain driver registers a "proxy"
regulator for each power domain that wants a regulator, with its
supply set to the name of the real `domain-supply` regulator.

The logic behind this was that the regulator core runs a check
for whether every supply is resolved before it turns off unused
regulators. The hope was that if we register a proxy regulator
and enable it immediately in the pm_domain probe, then regulator
core will handle the actual dependency at precisely the right
time, namely before it checks to see whether any are unused and
can be turned off.

As I discovered though, this can't really work. The regulator
core will in this case just set the supply regulator to a dummy
regulator when it's enabled in our probe function, and later
supply resolving passes are then content that this supply is
resolved even though it is a dummy supply. There does not appear
to be any way to opt out of getting this dummy supply.

So knowing that this doesn't work, I have another idea, but I
feel like both the regulator subsystem and the pm-domains
subsystem will hate this: explicitly create an order between
the pmdomain idle check and the regulator idle check to make
sure that the pmdomains idle check runs first.

This would set in stone how the kernel worked previously for
kernel users that relied on this, but is a conceptually
unpleasant cross-subsystem dependency.

> 
> Kind regards
> Uffe
> 

Kind regards,
Nicolas Frattaroli

Powered by blists - more mailing lists