* [PATCH] dt-bindings: arm: sunxi: update H2+/H3 cpu clocks
@ 2020-09-03 10:07 Wilken Gottwalt
2020-09-09 12:08 ` Maxime Ripard
0 siblings, 1 reply; 6+ messages in thread
From: Wilken Gottwalt @ 2020-09-03 10:07 UTC (permalink / raw)
To: linux-kernel; +Cc: Rob Herring, Maxime Ripard, Chen-Yu Tsai
Change H2+/H3 clocks to 8 steps from 528 MHz up to 1200 MHz to support a
more fine-grained powersave setup. The SoCs are made for 1296 MHz, so
these clocks are still in a safe range. Tested on a NanoPi Duo and
OrangePi Zero.
Signed-off-by: Wilken Gottwalt <wilken.gottwalt@mailbox.org>
---
arch/arm/boot/dts/sun8i-h3.dtsi | 34 +++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 4e89701df91f..5517fcc02b7d 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -48,23 +48,53 @@ cpu0_opp_table: opp_table0 {
compatible = "operating-points-v2";
opp-shared;
- opp-648000000 {
- opp-hz = /bits/ 64 <648000000>;
+ opp-528000000 {
+ opp-hz = /bits/ 64 <528000000>;
+ opp-microvolt = <1020000 1020000 1300000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
+
+ opp-624000000 {
+ opp-hz = /bits/ 64 <624000000>;
opp-microvolt = <1040000 1040000 1300000>;
clock-latency-ns = <244144>; /* 8 32k periods */
};
+ opp-720000000 {
+ opp-hz = /bits/ 64 <720000000>;
+ opp-microvolt = <1060000 1060000 1300000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
+
opp-816000000 {
opp-hz = /bits/ 64 <816000000>;
opp-microvolt = <1100000 1100000 1300000>;
clock-latency-ns = <244144>; /* 8 32k periods */
};
+ opp-912000000 {
+ opp-hz = /bits/ 64 <912000000>;
+ opp-microvolt = <1140000 1140000 1300000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
+
opp-1008000000 {
opp-hz = /bits/ 64 <1008000000>;
opp-microvolt = <1200000 1200000 1300000>;
clock-latency-ns = <244144>; /* 8 32k periods */
};
+
+ opp-1104000000 {
+ opp-hz = /bits/ 64 <1104000000>;
+ opp-microvolt = <1240000 1240000 1300000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
+
+ opp-1200000000 {
+ opp-hz = /bits/ 64 <1200000000>;
+ opp-microvolt = <1300000 1300000 1300000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
};
cpus {
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] dt-bindings: arm: sunxi: update H2+/H3 cpu clocks
2020-09-03 10:07 [PATCH] dt-bindings: arm: sunxi: update H2+/H3 cpu clocks Wilken Gottwalt
@ 2020-09-09 12:08 ` Maxime Ripard
2020-09-09 13:54 ` Wilken Gottwalt
0 siblings, 1 reply; 6+ messages in thread
From: Maxime Ripard @ 2020-09-09 12:08 UTC (permalink / raw)
To: Wilken Gottwalt; +Cc: linux-kernel, Rob Herring, Chen-Yu Tsai
[-- Attachment #1: Type: text/plain, Size: 3491 bytes --]
Hi!
Thanks for contributing
The prefix isn't right though.
dt-bindings is used when you're modifying the binding itself, ie the
description of what the node is supposed to look like, not when you
actually use that node in a DT.
In that case, that would be ARM: dts: sunxi:
(we're on the ARM architecture, modifying dts's, for the sunxi platform)
On Thu, Sep 03, 2020 at 12:07:08PM +0200, Wilken Gottwalt wrote:
> Change H2+/H3 clocks to 8 steps from 528 MHz up to 1200 MHz to support a
> more fine-grained powersave setup. The SoCs are made for 1296 MHz, so
> these clocks are still in a safe range. Tested on a NanoPi Duo and
> OrangePi Zero.
How was this tested?
cpufreq OPP misconfiguration on Allwinner SoCs has been known to create
some errors that are fairly hard to spot and be quite easy to go
unnoticed (like caches corruptions).
The only reliable test we have is:
https://github.com/ssvb/cpuburn-arm/blob/master/cpufreq-ljt-stress-test
> Signed-off-by: Wilken Gottwalt <wilken.gottwalt@mailbox.org>
> ---
> arch/arm/boot/dts/sun8i-h3.dtsi | 34 +++++++++++++++++++++++++++++++--
> 1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index 4e89701df91f..5517fcc02b7d 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -48,23 +48,53 @@ cpu0_opp_table: opp_table0 {
> compatible = "operating-points-v2";
> opp-shared;
>
> - opp-648000000 {
> - opp-hz = /bits/ 64 <648000000>;
> + opp-528000000 {
> + opp-hz = /bits/ 64 <528000000>;
> + opp-microvolt = <1020000 1020000 1300000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> +
> + opp-624000000 {
> + opp-hz = /bits/ 64 <624000000>;
> opp-microvolt = <1040000 1040000 1300000>;
> clock-latency-ns = <244144>; /* 8 32k periods */
> };
>
> + opp-720000000 {
> + opp-hz = /bits/ 64 <720000000>;
> + opp-microvolt = <1060000 1060000 1300000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> +
> opp-816000000 {
> opp-hz = /bits/ 64 <816000000>;
> opp-microvolt = <1100000 1100000 1300000>;
> clock-latency-ns = <244144>; /* 8 32k periods */
> };
>
> + opp-912000000 {
> + opp-hz = /bits/ 64 <912000000>;
> + opp-microvolt = <1140000 1140000 1300000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> +
> opp-1008000000 {
> opp-hz = /bits/ 64 <1008000000>;
> opp-microvolt = <1200000 1200000 1300000>;
> clock-latency-ns = <244144>; /* 8 32k periods */
> };
> +
> + opp-1104000000 {
> + opp-hz = /bits/ 64 <1104000000>;
> + opp-microvolt = <1240000 1240000 1300000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> +
> + opp-1200000000 {
> + opp-hz = /bits/ 64 <1200000000>;
> + opp-microvolt = <1300000 1300000 1300000>;
> + clock-latency-ns = <244144>; /* 8 32k periods */
> + };
> };
IIRC U-Boot will start the CPU at 1008 MHz on the H3, so in the
situation where a board doesn't list the regulators attached to the CPU,
the kernel will happily use the 1104 and 1200 MHz frequencies, even
though it won't adjust the voltage accordingly, causing errors.
The way we worked around that is to provide the extra OOPs in a separate
DTSI so that the boards with regulator support can opt-in.
Maxime
>
> cpus {
> --
> 2.28.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dt-bindings: arm: sunxi: update H2+/H3 cpu clocks
2020-09-09 12:08 ` Maxime Ripard
@ 2020-09-09 13:54 ` Wilken Gottwalt
2020-09-09 15:53 ` Maxime Ripard
0 siblings, 1 reply; 6+ messages in thread
From: Wilken Gottwalt @ 2020-09-09 13:54 UTC (permalink / raw)
To: Maxime Ripard; +Cc: linux-kernel, Rob Herring, Chen-Yu Tsai
On Wed, 9 Sep 2020 14:08:59 +0200
Maxime Ripard <maxime@cerno.tech> wrote:
> Hi!
>
> Thanks for contributing
>
> The prefix isn't right though.
>
> dt-bindings is used when you're modifying the binding itself, ie the
> description of what the node is supposed to look like, not when you
> actually use that node in a DT.
>
> In that case, that would be ARM: dts: sunxi:
>
> (we're on the ARM architecture, modifying dts's, for the sunxi platform)
Ah, I see, it was my first attempt to contribute and wasn't 100% sure, just
took the line from similar patches on the LKML. Thanks for the correction.
> On Thu, Sep 03, 2020 at 12:07:08PM +0200, Wilken Gottwalt wrote:
> > Change H2+/H3 clocks to 8 steps from 528 MHz up to 1200 MHz to support a
> > more fine-grained powersave setup. The SoCs are made for 1296 MHz, so
> > these clocks are still in a safe range. Tested on a NanoPi Duo and
> > OrangePi Zero.
>
> How was this tested?
This is a longer story. It actually runs on hardware which is in production
for about 2-3 years, some use H2+ with full voltage regulators and some are
similar to the NanoPi DUO, where the voltage regulator can only switch
between 1.1 and 1.3 volts. It runs in two ways: A fully dynamic setup where
the ondemand scheduler is used and the second way where it is switched to
fixed values (based on load and temperature) using the cpufrequtils. The
devices running a 4.14.x kernel and are tested against 4.19.x kernels.
These devices are routers running a custom tcp/ip stack and have a high I/O
load. I also prepared devices based on a custom H3 design, which ran stable
at 1.392 GHz (but had passive coolers attached). Do these explanations
help?
> cpufreq OPP misconfiguration on Allwinner SoCs has been known to create
> some errors that are fairly hard to spot and be quite easy to go
> unnoticed (like caches corruptions).
Yeah, I noticed that in the beginning where I prepared the first kernels
for these devices. But after switching to multiples of 48MHz and bigger
steps these issues disappeared. I'm aware that this does not proof that
these issues do not appear, but thougth I share the values which I
consider stable.
> The only reliable test we have is:
> https://github.com/ssvb/cpuburn-arm/blob/master/cpufreq-ljt-stress-test
>
> > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@mailbox.org>
> > ---
> > arch/arm/boot/dts/sun8i-h3.dtsi | 34 +++++++++++++++++++++++++++++++--
> > 1 file changed, 32 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> > index 4e89701df91f..5517fcc02b7d 100644
> > --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> > @@ -48,23 +48,53 @@ cpu0_opp_table: opp_table0 {
> > compatible = "operating-points-v2";
> > opp-shared;
> >
> > - opp-648000000 {
> > - opp-hz = /bits/ 64 <648000000>;
> > + opp-528000000 {
> > + opp-hz = /bits/ 64 <528000000>;
> > + opp-microvolt = <1020000 1020000 1300000>;
> > + clock-latency-ns = <244144>; /* 8 32k periods */
> > + };
> > +
> > + opp-624000000 {
> > + opp-hz = /bits/ 64 <624000000>;
> > opp-microvolt = <1040000 1040000 1300000>;
> > clock-latency-ns = <244144>; /* 8 32k periods */
> > };
> >
> > + opp-720000000 {
> > + opp-hz = /bits/ 64 <720000000>;
> > + opp-microvolt = <1060000 1060000 1300000>;
> > + clock-latency-ns = <244144>; /* 8 32k periods */
> > + };
> > +
> > opp-816000000 {
> > opp-hz = /bits/ 64 <816000000>;
> > opp-microvolt = <1100000 1100000 1300000>;
> > clock-latency-ns = <244144>; /* 8 32k periods */
> > };
> >
> > + opp-912000000 {
> > + opp-hz = /bits/ 64 <912000000>;
> > + opp-microvolt = <1140000 1140000 1300000>;
> > + clock-latency-ns = <244144>; /* 8 32k periods */
> > + };
> > +
> > opp-1008000000 {
> > opp-hz = /bits/ 64 <1008000000>;
> > opp-microvolt = <1200000 1200000 1300000>;
> > clock-latency-ns = <244144>; /* 8 32k periods */
> > };
> > +
> > + opp-1104000000 {
> > + opp-hz = /bits/ 64 <1104000000>;
> > + opp-microvolt = <1240000 1240000 1300000>;
> > + clock-latency-ns = <244144>; /* 8 32k periods */
> > + };
> > +
> > + opp-1200000000 {
> > + opp-hz = /bits/ 64 <1200000000>;
> > + opp-microvolt = <1300000 1300000 1300000>;
> > + clock-latency-ns = <244144>; /* 8 32k periods */
> > + };
> > };
>
> IIRC U-Boot will start the CPU at 1008 MHz on the H3, so in the
> situation where a board doesn't list the regulators attached to the CPU,
> the kernel will happily use the 1104 and 1200 MHz frequencies, even
> though it won't adjust the voltage accordingly, causing errors.
Yeah, I know that. The u-boot running on the mentioned devices also have
these modifications, including a higher clocked DRAM (actually running at
624 MHz).
> The way we worked around that is to provide the extra OOPs in a separate
> DTSI so that the boards with regulator support can opt-in.
Oh, did I overlooked something? I was working on 4.14 up to 4.19 kernels
and may be a bit to eager to post the modifications.
greetings,
Will
> Maxime
>
> >
> > cpus {
> > --
> > 2.28.0
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dt-bindings: arm: sunxi: update H2+/H3 cpu clocks
2020-09-09 13:54 ` Wilken Gottwalt
@ 2020-09-09 15:53 ` Maxime Ripard
2020-09-14 4:00 ` Wilken Gottwalt
0 siblings, 1 reply; 6+ messages in thread
From: Maxime Ripard @ 2020-09-09 15:53 UTC (permalink / raw)
To: Wilken Gottwalt; +Cc: linux-kernel, Rob Herring, Chen-Yu Tsai
[-- Attachment #1: Type: text/plain, Size: 6117 bytes --]
On Wed, Sep 09, 2020 at 03:54:46PM +0200, Wilken Gottwalt wrote:
> On Wed, 9 Sep 2020 14:08:59 +0200
> Maxime Ripard <maxime@cerno.tech> wrote:
> > Hi!
> >
> > Thanks for contributing
> >
> > The prefix isn't right though.
> >
> > dt-bindings is used when you're modifying the binding itself, ie the
> > description of what the node is supposed to look like, not when you
> > actually use that node in a DT.
> >
> > In that case, that would be ARM: dts: sunxi:
> >
> > (we're on the ARM architecture, modifying dts's, for the sunxi platform)
>
> Ah, I see, it was my first attempt to contribute and wasn't 100% sure, just
> took the line from similar patches on the LKML. Thanks for the correction.
>
> > On Thu, Sep 03, 2020 at 12:07:08PM +0200, Wilken Gottwalt wrote:
> > > Change H2+/H3 clocks to 8 steps from 528 MHz up to 1200 MHz to support a
> > > more fine-grained powersave setup. The SoCs are made for 1296 MHz, so
> > > these clocks are still in a safe range. Tested on a NanoPi Duo and
> > > OrangePi Zero.
> >
> > How was this tested?
>
> This is a longer story. It actually runs on hardware which is in production
> for about 2-3 years, some use H2+ with full voltage regulators and some are
> similar to the NanoPi DUO, where the voltage regulator can only switch
> between 1.1 and 1.3 volts. It runs in two ways: A fully dynamic setup where
> the ondemand scheduler is used and the second way where it is switched to
> fixed values (based on load and temperature) using the cpufrequtils. The
> devices running a 4.14.x kernel and are tested against 4.19.x kernels.
> These devices are routers running a custom tcp/ip stack and have a high I/O
> load. I also prepared devices based on a custom H3 design, which ran stable
> at 1.392 GHz (but had passive coolers attached). Do these explanations
> help?
To some extent, but not entirely. Depending on the governor / workload,
some OPPs might never be used at all.
> > cpufreq OPP misconfiguration on Allwinner SoCs has been known to create
> > some errors that are fairly hard to spot and be quite easy to go
> > unnoticed (like caches corruptions).
>
> Yeah, I noticed that in the beginning where I prepared the first kernels
> for these devices. But after switching to multiples of 48MHz and bigger
> steps these issues disappeared. I'm aware that this does not proof that
> these issues do not appear, but thougth I share the values which I
> consider stable.
The only really reliable test we've had so far is the one I pointed out,
so please run it on one board at least
> > The only reliable test we have is:
> > https://github.com/ssvb/cpuburn-arm/blob/master/cpufreq-ljt-stress-test
> >
> > > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@mailbox.org>
> > > ---
> > > arch/arm/boot/dts/sun8i-h3.dtsi | 34 +++++++++++++++++++++++++++++++--
> > > 1 file changed, 32 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > index 4e89701df91f..5517fcc02b7d 100644
> > > --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> > > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > @@ -48,23 +48,53 @@ cpu0_opp_table: opp_table0 {
> > > compatible = "operating-points-v2";
> > > opp-shared;
> > >
> > > - opp-648000000 {
> > > - opp-hz = /bits/ 64 <648000000>;
> > > + opp-528000000 {
> > > + opp-hz = /bits/ 64 <528000000>;
> > > + opp-microvolt = <1020000 1020000 1300000>;
> > > + clock-latency-ns = <244144>; /* 8 32k periods */
> > > + };
> > > +
> > > + opp-624000000 {
> > > + opp-hz = /bits/ 64 <624000000>;
> > > opp-microvolt = <1040000 1040000 1300000>;
> > > clock-latency-ns = <244144>; /* 8 32k periods */
> > > };
> > >
> > > + opp-720000000 {
> > > + opp-hz = /bits/ 64 <720000000>;
> > > + opp-microvolt = <1060000 1060000 1300000>;
> > > + clock-latency-ns = <244144>; /* 8 32k periods */
> > > + };
> > > +
> > > opp-816000000 {
> > > opp-hz = /bits/ 64 <816000000>;
> > > opp-microvolt = <1100000 1100000 1300000>;
> > > clock-latency-ns = <244144>; /* 8 32k periods */
> > > };
> > >
> > > + opp-912000000 {
> > > + opp-hz = /bits/ 64 <912000000>;
> > > + opp-microvolt = <1140000 1140000 1300000>;
> > > + clock-latency-ns = <244144>; /* 8 32k periods */
> > > + };
> > > +
> > > opp-1008000000 {
> > > opp-hz = /bits/ 64 <1008000000>;
> > > opp-microvolt = <1200000 1200000 1300000>;
> > > clock-latency-ns = <244144>; /* 8 32k periods */
> > > };
> > > +
> > > + opp-1104000000 {
> > > + opp-hz = /bits/ 64 <1104000000>;
> > > + opp-microvolt = <1240000 1240000 1300000>;
> > > + clock-latency-ns = <244144>; /* 8 32k periods */
> > > + };
> > > +
> > > + opp-1200000000 {
> > > + opp-hz = /bits/ 64 <1200000000>;
> > > + opp-microvolt = <1300000 1300000 1300000>;
> > > + clock-latency-ns = <244144>; /* 8 32k periods */
> > > + };
> > > };
> >
> > IIRC U-Boot will start the CPU at 1008 MHz on the H3, so in the
> > situation where a board doesn't list the regulators attached to the CPU,
> > the kernel will happily use the 1104 and 1200 MHz frequencies, even
> > though it won't adjust the voltage accordingly, causing errors.
>
> Yeah, I know that. The u-boot running on the mentioned devices also have
> these modifications, including a higher clocked DRAM (actually running at
> 624 MHz).
>
> > The way we worked around that is to provide the extra OOPs in a separate
> > DTSI so that the boards with regulator support can opt-in.
>
> Oh, did I overlooked something? I was working on 4.14 up to 4.19 kernels
> and may be a bit to eager to post the modifications.
It's not really my point :)
My point is that since it's in the DTSI, every board using an H2/H3 will
now get to use those OPPs, even if they don't have the regulator support
and the voltage set for the highest frequency, so you can end up on
those boards with the CPU running at 1.2GHz and the voltage associated
to 1GHz
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dt-bindings: arm: sunxi: update H2+/H3 cpu clocks
2020-09-09 15:53 ` Maxime Ripard
@ 2020-09-14 4:00 ` Wilken Gottwalt
2020-09-21 12:10 ` Maxime Ripard
0 siblings, 1 reply; 6+ messages in thread
From: Wilken Gottwalt @ 2020-09-14 4:00 UTC (permalink / raw)
To: Maxime Ripard; +Cc: linux-kernel, Rob Herring, Chen-Yu Tsai
On Wed, 9 Sep 2020 17:53:07 +0200
Maxime Ripard <maxime@cerno.tech> wrote:
> On Wed, Sep 09, 2020 at 03:54:46PM +0200, Wilken Gottwalt wrote:
> > On Wed, 9 Sep 2020 14:08:59 +0200
> > Maxime Ripard <maxime@cerno.tech> wrote:
> > > Hi!
> > >
> > > Thanks for contributing
> > >
> > > The prefix isn't right though.
> > >
> > > dt-bindings is used when you're modifying the binding itself, ie the
> > > description of what the node is supposed to look like, not when you
> > > actually use that node in a DT.
> > >
> > > In that case, that would be ARM: dts: sunxi:
> > >
> > > (we're on the ARM architecture, modifying dts's, for the sunxi platform)
> >
> > Ah, I see, it was my first attempt to contribute and wasn't 100% sure, just
> > took the line from similar patches on the LKML. Thanks for the correction.
> >
> > > On Thu, Sep 03, 2020 at 12:07:08PM +0200, Wilken Gottwalt wrote:
> > > > Change H2+/H3 clocks to 8 steps from 528 MHz up to 1200 MHz to support a
> > > > more fine-grained powersave setup. The SoCs are made for 1296 MHz, so
> > > > these clocks are still in a safe range. Tested on a NanoPi Duo and
> > > > OrangePi Zero.
> > >
> > > How was this tested?
> >
> > This is a longer story. It actually runs on hardware which is in production
> > for about 2-3 years, some use H2+ with full voltage regulators and some are
> > similar to the NanoPi DUO, where the voltage regulator can only switch
> > between 1.1 and 1.3 volts. It runs in two ways: A fully dynamic setup where
> > the ondemand scheduler is used and the second way where it is switched to
> > fixed values (based on load and temperature) using the cpufrequtils. The
> > devices running a 4.14.x kernel and are tested against 4.19.x kernels.
> > These devices are routers running a custom tcp/ip stack and have a high I/O
> > load. I also prepared devices based on a custom H3 design, which ran stable
> > at 1.392 GHz (but had passive coolers attached). Do these explanations
> > help?
>
> To some extent, but not entirely. Depending on the governor / workload,
> some OPPs might never be used at all.
I am aware of this and the devices had 24 hours burnin tests with a selfwritten
tool very similar to your posted cpuburn + scripts. I will try to run your
pointed out tool, but I may need some time for doing so. Getting a ruby
installation into this embedded Linux is not easy and a whole compiler won't
be possible at all. If you are interested I could put our test tool to github.
> > > cpufreq OPP misconfiguration on Allwinner SoCs has been known to create
> > > some errors that are fairly hard to spot and be quite easy to go
> > > unnoticed (like caches corruptions).
> >
> > Yeah, I noticed that in the beginning where I prepared the first kernels
> > for these devices. But after switching to multiples of 48MHz and bigger
> > steps these issues disappeared. I'm aware that this does not proof that
> > these issues do not appear, but thougth I share the values which I
> > consider stable.
>
> The only really reliable test we've had so far is the one I pointed out,
> so please run it on one board at least
>
> > > The only reliable test we have is:
> > > https://github.com/ssvb/cpuburn-arm/blob/master/cpufreq-ljt-stress-test
> > >
> > > > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@mailbox.org>
> > > > ---
> > > > arch/arm/boot/dts/sun8i-h3.dtsi | 34 +++++++++++++++++++++++++++++++--
> > > > 1 file changed, 32 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > index 4e89701df91f..5517fcc02b7d 100644
> > > > --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > @@ -48,23 +48,53 @@ cpu0_opp_table: opp_table0 {
> > > > compatible = "operating-points-v2";
> > > > opp-shared;
> > > >
> > > > - opp-648000000 {
> > > > - opp-hz = /bits/ 64 <648000000>;
> > > > + opp-528000000 {
> > > > + opp-hz = /bits/ 64 <528000000>;
> > > > + opp-microvolt = <1020000 1020000 1300000>;
> > > > + clock-latency-ns = <244144>; /* 8 32k periods */
> > > > + };
> > > > +
> > > > + opp-624000000 {
> > > > + opp-hz = /bits/ 64 <624000000>;
> > > > opp-microvolt = <1040000 1040000 1300000>;
> > > > clock-latency-ns = <244144>; /* 8 32k periods */
> > > > };
> > > >
> > > > + opp-720000000 {
> > > > + opp-hz = /bits/ 64 <720000000>;
> > > > + opp-microvolt = <1060000 1060000 1300000>;
> > > > + clock-latency-ns = <244144>; /* 8 32k periods */
> > > > + };
> > > > +
> > > > opp-816000000 {
> > > > opp-hz = /bits/ 64 <816000000>;
> > > > opp-microvolt = <1100000 1100000 1300000>;
> > > > clock-latency-ns = <244144>; /* 8 32k periods */
> > > > };
> > > >
> > > > + opp-912000000 {
> > > > + opp-hz = /bits/ 64 <912000000>;
> > > > + opp-microvolt = <1140000 1140000 1300000>;
> > > > + clock-latency-ns = <244144>; /* 8 32k periods */
> > > > + };
> > > > +
> > > > opp-1008000000 {
> > > > opp-hz = /bits/ 64 <1008000000>;
> > > > opp-microvolt = <1200000 1200000 1300000>;
> > > > clock-latency-ns = <244144>; /* 8 32k periods */
> > > > };
> > > > +
> > > > + opp-1104000000 {
> > > > + opp-hz = /bits/ 64 <1104000000>;
> > > > + opp-microvolt = <1240000 1240000 1300000>;
> > > > + clock-latency-ns = <244144>; /* 8 32k periods */
> > > > + };
> > > > +
> > > > + opp-1200000000 {
> > > > + opp-hz = /bits/ 64 <1200000000>;
> > > > + opp-microvolt = <1300000 1300000 1300000>;
> > > > + clock-latency-ns = <244144>; /* 8 32k periods */
> > > > + };
> > > > };
> > >
> > > IIRC U-Boot will start the CPU at 1008 MHz on the H3, so in the
> > > situation where a board doesn't list the regulators attached to the CPU,
> > > the kernel will happily use the 1104 and 1200 MHz frequencies, even
> > > though it won't adjust the voltage accordingly, causing errors.
> >
> > Yeah, I know that. The u-boot running on the mentioned devices also have
> > these modifications, including a higher clocked DRAM (actually running at
> > 624 MHz).
> >
> > > The way we worked around that is to provide the extra OOPs in a separate
> > > DTSI so that the boards with regulator support can opt-in.
> >
> > Oh, did I overlooked something? I was working on 4.14 up to 4.19 kernels
> > and may be a bit to eager to post the modifications.
>
> It's not really my point :)
>
> My point is that since it's in the DTSI, every board using an H2/H3 will
> now get to use those OPPs, even if they don't have the regulator support
> and the voltage set for the highest frequency, so you can end up on
> those boards with the CPU running at 1.2GHz and the voltage associated
> to 1GHz
Yeah, I understand. So the solution would be to drop the two highest OPPs
or make these changes only for the tested devices. What would you prefer?
Will
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dt-bindings: arm: sunxi: update H2+/H3 cpu clocks
2020-09-14 4:00 ` Wilken Gottwalt
@ 2020-09-21 12:10 ` Maxime Ripard
0 siblings, 0 replies; 6+ messages in thread
From: Maxime Ripard @ 2020-09-21 12:10 UTC (permalink / raw)
To: Wilken Gottwalt; +Cc: linux-kernel, Rob Herring, Chen-Yu Tsai
Hi,
On Mon, Sep 14, 2020 at 06:00:56AM +0200, Wilken Gottwalt wrote:
> > > > cpufreq OPP misconfiguration on Allwinner SoCs has been known to create
> > > > some errors that are fairly hard to spot and be quite easy to go
> > > > unnoticed (like caches corruptions).
> > >
> > > Yeah, I noticed that in the beginning where I prepared the first kernels
> > > for these devices. But after switching to multiples of 48MHz and bigger
> > > steps these issues disappeared. I'm aware that this does not proof that
> > > these issues do not appear, but thougth I share the values which I
> > > consider stable.
> >
> > The only really reliable test we've had so far is the one I pointed out,
> > so please run it on one board at least
> >
> > > > The only reliable test we have is:
> > > > https://github.com/ssvb/cpuburn-arm/blob/master/cpufreq-ljt-stress-test
> > > >
> > > > > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@mailbox.org>
> > > > > ---
> > > > > arch/arm/boot/dts/sun8i-h3.dtsi | 34 +++++++++++++++++++++++++++++++--
> > > > > 1 file changed, 32 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > index 4e89701df91f..5517fcc02b7d 100644
> > > > > --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > @@ -48,23 +48,53 @@ cpu0_opp_table: opp_table0 {
> > > > > compatible = "operating-points-v2";
> > > > > opp-shared;
> > > > >
> > > > > - opp-648000000 {
> > > > > - opp-hz = /bits/ 64 <648000000>;
> > > > > + opp-528000000 {
> > > > > + opp-hz = /bits/ 64 <528000000>;
> > > > > + opp-microvolt = <1020000 1020000 1300000>;
> > > > > + clock-latency-ns = <244144>; /* 8 32k periods */
> > > > > + };
> > > > > +
> > > > > + opp-624000000 {
> > > > > + opp-hz = /bits/ 64 <624000000>;
> > > > > opp-microvolt = <1040000 1040000 1300000>;
> > > > > clock-latency-ns = <244144>; /* 8 32k periods */
> > > > > };
> > > > >
> > > > > + opp-720000000 {
> > > > > + opp-hz = /bits/ 64 <720000000>;
> > > > > + opp-microvolt = <1060000 1060000 1300000>;
> > > > > + clock-latency-ns = <244144>; /* 8 32k periods */
> > > > > + };
> > > > > +
> > > > > opp-816000000 {
> > > > > opp-hz = /bits/ 64 <816000000>;
> > > > > opp-microvolt = <1100000 1100000 1300000>;
> > > > > clock-latency-ns = <244144>; /* 8 32k periods */
> > > > > };
> > > > >
> > > > > + opp-912000000 {
> > > > > + opp-hz = /bits/ 64 <912000000>;
> > > > > + opp-microvolt = <1140000 1140000 1300000>;
> > > > > + clock-latency-ns = <244144>; /* 8 32k periods */
> > > > > + };
> > > > > +
> > > > > opp-1008000000 {
> > > > > opp-hz = /bits/ 64 <1008000000>;
> > > > > opp-microvolt = <1200000 1200000 1300000>;
> > > > > clock-latency-ns = <244144>; /* 8 32k periods */
> > > > > };
> > > > > +
> > > > > + opp-1104000000 {
> > > > > + opp-hz = /bits/ 64 <1104000000>;
> > > > > + opp-microvolt = <1240000 1240000 1300000>;
> > > > > + clock-latency-ns = <244144>; /* 8 32k periods */
> > > > > + };
> > > > > +
> > > > > + opp-1200000000 {
> > > > > + opp-hz = /bits/ 64 <1200000000>;
> > > > > + opp-microvolt = <1300000 1300000 1300000>;
> > > > > + clock-latency-ns = <244144>; /* 8 32k periods */
> > > > > + };
> > > > > };
> > > >
> > > > IIRC U-Boot will start the CPU at 1008 MHz on the H3, so in the
> > > > situation where a board doesn't list the regulators attached to the CPU,
> > > > the kernel will happily use the 1104 and 1200 MHz frequencies, even
> > > > though it won't adjust the voltage accordingly, causing errors.
> > >
> > > Yeah, I know that. The u-boot running on the mentioned devices also have
> > > these modifications, including a higher clocked DRAM (actually running at
> > > 624 MHz).
> > >
> > > > The way we worked around that is to provide the extra OOPs in a separate
> > > > DTSI so that the boards with regulator support can opt-in.
> > >
> > > Oh, did I overlooked something? I was working on 4.14 up to 4.19 kernels
> > > and may be a bit to eager to post the modifications.
> >
> > It's not really my point :)
> >
> > My point is that since it's in the DTSI, every board using an H2/H3 will
> > now get to use those OPPs, even if they don't have the regulator support
> > and the voltage set for the highest frequency, so you can end up on
> > those boards with the CPU running at 1.2GHz and the voltage associated
> > to 1GHz
>
> Yeah, I understand. So the solution would be to drop the two highest OPPs
> or make these changes only for the tested devices. What would you prefer?
Like I said earlier, the easiest would be to have another DTSI with the
OPPs that would be included by the boards with regulator support
Maxime
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-21 12:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-03 10:07 [PATCH] dt-bindings: arm: sunxi: update H2+/H3 cpu clocks Wilken Gottwalt
2020-09-09 12:08 ` Maxime Ripard
2020-09-09 13:54 ` Wilken Gottwalt
2020-09-09 15:53 ` Maxime Ripard
2020-09-14 4:00 ` Wilken Gottwalt
2020-09-21 12:10 ` Maxime Ripard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).