Message-ID: <CAPDyKFofhy5wiNsHUgdtzFwGtO3QPqhVuu1KsPLBWHF08JzqyA@mail.gmail.com>
Date: Mon, 8 Sep 2025 15:40:24 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
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 Mon, 8 Sept 2025 at 15:14, Nicolas Frattaroli
<nicolas.frattaroli@...labora.com> wrote:
>
> 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".

Well, I would give this a go and see what you end up with. The nice
thing with this approach, I think, is that we get a driver and can use
the -EPROBE_DEFER mechanism.

Another option would be to explore using fw_devlink/device_links, to
somehow get a notification as soon as the regulator gets registered.

>
> 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.

I think those kinds of dependencies are better solved by using
fw_devlink/device_links.

>
> 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.

Then there are other resources cropping up that we need support,
extending to more subsystems. So, no, this does not seem like a
scalable solution to me. :-)

Kind regards
Uffe

Powered by blists - more mailing lists