From: "Heiko Stübner" <heiko@sntech.de>
To: Quentin Schulz <quentin.schulz@cherry.de>
Cc: jonas@kwiboo.se, dsimic@manjaro.org,
linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arm64: dts: rockchip: add regulator-enable-ramp-delay to RK860-2/-3 regulators
Date: Thu, 05 Jun 2025 11:09:40 +0200 [thread overview]
Message-ID: <49977521.MN2xkq1pzW@diego> (raw)
In-Reply-To: <e4a1bf0a-0fc5-4c15-96dc-7e9c7ae3ccd5@cherry.de>
Am Donnerstag, 5. Juni 2025, 10:57:13 Mitteleuropäische Sommerzeit schrieb Quentin Schulz:
> Hi Heiko,
>
> On 6/4/25 10:24 PM, Heiko Stuebner wrote:
> > The RK860-2/-3 regulators are used on rk3588 boards to supply components
> > like the big cpu-clusters and npu individually.
> >
> > Most of these things will be, and in fact most regulator nodes right now are,
> > always-on - probably nobody has tried completely turning of the big clusters
> > for example.
> >
>
> This is a bit of a confusing wording here. If most things will be (and
> are) always-on, then the ramp-up typically shouldn't be an issue I
> assume? I'm not too familiar with the regulator subsystem but I guess
> for the first initial enable this could be an issue?
The regulators normally are all boot-on, so we normally start with all
of them running. But I'll try to improve the wording overall :-) .
> > This changed with the new NPU driver, which does combined runtime-suspends
> > with the regulator being tied to the power-domain (it supplies the pd).
> >
> > If the NPU runtime-suspends while running a load and then starts again
> > hangs can be observed with messages like
> >
> > rockchip-pm-domain fd8d8000.power-management:power-controller: failed to set domain 'nputop' on, val=0
> >
> > Removing the regulator from the domain and instead setting it to always-on
> > "fixes" the issue, which suggests that the domain is trying to get current
> > from the regulator before it is ready when it gets turned back on.
> >
>
> If I'm not mistaken, this is also misleading as nothing currently uses
> that (NPU support not merged yet and most if not all NPU regulators
> currently are always-on so typically not impacted by this issue).
>
> I assume this will be an issue the moment we add support for suspending
> the NPU regulator, which would anyway require modification of the
> various DT. Is that correct?
Only when the regulator is not set to always-on, so gets disabled by
runtime-pm. But yes, the npu driver was triggering this quite easily.
> > And in fact the datasheet for the regulator defines an "Internal soft-start
> > time". For a target output voltage of 1.0V the _typical_ time to reach at
> > least 92% of the output is given as 260uS.
> >
>
> Indeed. Now looking at the existing Device Trees, it seems some set the
> ramp-up delay already, but to 2300 and not 500 like suggested here.
> Maybe it'd be safer to go for 2300 by default then?
enable-ramp-delay is a totally different beast than the ramp-delay.
ramp-delay is needed when changing the running voltage and uses a unit
of "uV/us", so microvolt per microsecond ... where the enable-ramp-delay
is the one for enabling the regulator and uses uS.
> > So that value is dependent on the target voltage (up to 1.5V for the type)
> > and also the rest of the board design.
> >
> > So add a regulator-enable-ramp-delay to all rk860-2/-3 nodes we have in
>
> Maybe mention that there's currently no rk8601 node upstream, and the
> only rk8600 (arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi)
> already has a ramp-up delay configured. Otherwise reading this I'm
> wondering why the rk860-0 and rk860-1 while being handled by the same
> datasheet are implicitly excluded from this change.
in turn with the above paragraph, I should probably check again for the
Radxa, because that variant has the same enable-ramp-delay.
> > mainline right now. I've chosen 500uS to be on the safe side, as
> > 260uS is the "typical" value for 1.0V and sadly no max value is given
> > in the datasheet.
> >
>
> Reading the rk808 regulator driver... maybe we should also set the
> typical delay as default in the fan53555.c driver? See rk805_reg which
> sets the enable_time for some (typically the LDO with 400 and the DCDC
> at 0). I assume those can be overridden from the DT anyway, but at least
> we would have some decently safe defaults?
>
> If we do not do this, then we should probably force the presence of
> regulator-ramp-delay property for the rk860x DT binding so we don't
> forget for future Device Trees?
that is scope-creep (rk808 != rk860) ... but I find the idea of trying to
set the enable-ramp-delay as required for the rk860-x interesting :-) .
Heiko
WARNING: multiple messages have this Message-ID (diff)
From: "Heiko Stübner" <heiko@sntech.de>
To: Quentin Schulz <quentin.schulz@cherry.de>
Cc: jonas@kwiboo.se, dsimic@manjaro.org,
linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arm64: dts: rockchip: add regulator-enable-ramp-delay to RK860-2/-3 regulators
Date: Thu, 05 Jun 2025 11:09:40 +0200 [thread overview]
Message-ID: <49977521.MN2xkq1pzW@diego> (raw)
In-Reply-To: <e4a1bf0a-0fc5-4c15-96dc-7e9c7ae3ccd5@cherry.de>
Am Donnerstag, 5. Juni 2025, 10:57:13 Mitteleuropäische Sommerzeit schrieb Quentin Schulz:
> Hi Heiko,
>
> On 6/4/25 10:24 PM, Heiko Stuebner wrote:
> > The RK860-2/-3 regulators are used on rk3588 boards to supply components
> > like the big cpu-clusters and npu individually.
> >
> > Most of these things will be, and in fact most regulator nodes right now are,
> > always-on - probably nobody has tried completely turning of the big clusters
> > for example.
> >
>
> This is a bit of a confusing wording here. If most things will be (and
> are) always-on, then the ramp-up typically shouldn't be an issue I
> assume? I'm not too familiar with the regulator subsystem but I guess
> for the first initial enable this could be an issue?
The regulators normally are all boot-on, so we normally start with all
of them running. But I'll try to improve the wording overall :-) .
> > This changed with the new NPU driver, which does combined runtime-suspends
> > with the regulator being tied to the power-domain (it supplies the pd).
> >
> > If the NPU runtime-suspends while running a load and then starts again
> > hangs can be observed with messages like
> >
> > rockchip-pm-domain fd8d8000.power-management:power-controller: failed to set domain 'nputop' on, val=0
> >
> > Removing the regulator from the domain and instead setting it to always-on
> > "fixes" the issue, which suggests that the domain is trying to get current
> > from the regulator before it is ready when it gets turned back on.
> >
>
> If I'm not mistaken, this is also misleading as nothing currently uses
> that (NPU support not merged yet and most if not all NPU regulators
> currently are always-on so typically not impacted by this issue).
>
> I assume this will be an issue the moment we add support for suspending
> the NPU regulator, which would anyway require modification of the
> various DT. Is that correct?
Only when the regulator is not set to always-on, so gets disabled by
runtime-pm. But yes, the npu driver was triggering this quite easily.
> > And in fact the datasheet for the regulator defines an "Internal soft-start
> > time". For a target output voltage of 1.0V the _typical_ time to reach at
> > least 92% of the output is given as 260uS.
> >
>
> Indeed. Now looking at the existing Device Trees, it seems some set the
> ramp-up delay already, but to 2300 and not 500 like suggested here.
> Maybe it'd be safer to go for 2300 by default then?
enable-ramp-delay is a totally different beast than the ramp-delay.
ramp-delay is needed when changing the running voltage and uses a unit
of "uV/us", so microvolt per microsecond ... where the enable-ramp-delay
is the one for enabling the regulator and uses uS.
> > So that value is dependent on the target voltage (up to 1.5V for the type)
> > and also the rest of the board design.
> >
> > So add a regulator-enable-ramp-delay to all rk860-2/-3 nodes we have in
>
> Maybe mention that there's currently no rk8601 node upstream, and the
> only rk8600 (arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi)
> already has a ramp-up delay configured. Otherwise reading this I'm
> wondering why the rk860-0 and rk860-1 while being handled by the same
> datasheet are implicitly excluded from this change.
in turn with the above paragraph, I should probably check again for the
Radxa, because that variant has the same enable-ramp-delay.
> > mainline right now. I've chosen 500uS to be on the safe side, as
> > 260uS is the "typical" value for 1.0V and sadly no max value is given
> > in the datasheet.
> >
>
> Reading the rk808 regulator driver... maybe we should also set the
> typical delay as default in the fan53555.c driver? See rk805_reg which
> sets the enable_time for some (typically the LDO with 400 and the DCDC
> at 0). I assume those can be overridden from the DT anyway, but at least
> we would have some decently safe defaults?
>
> If we do not do this, then we should probably force the presence of
> regulator-ramp-delay property for the rk860x DT binding so we don't
> forget for future Device Trees?
that is scope-creep (rk808 != rk860) ... but I find the idea of trying to
set the enable-ramp-delay as required for the rk860-x interesting :-) .
Heiko
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
next prev parent reply other threads:[~2025-06-05 9:09 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-04 20:24 [PATCH] arm64: dts: rockchip: add regulator-enable-ramp-delay to RK860-2/-3 regulators Heiko Stuebner
2025-06-04 20:24 ` Heiko Stuebner
2025-06-05 8:57 ` Quentin Schulz
2025-06-05 8:57 ` Quentin Schulz
2025-06-05 9:05 ` Quentin Schulz
2025-06-05 9:05 ` Quentin Schulz
2025-06-05 9:09 ` Heiko Stübner [this message]
2025-06-05 9:09 ` Heiko Stübner
2025-06-05 9:22 ` Quentin Schulz
2025-06-05 9:22 ` Quentin Schulz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=49977521.MN2xkq1pzW@diego \
--to=heiko@sntech.de \
--cc=dsimic@manjaro.org \
--cc=jonas@kwiboo.se \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=quentin.schulz@cherry.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.