[PATCH 0/4] arm64: dts: allwinner: h6: Enable CPU DVFS(cpufreq)

[PATCH 0/4] arm64: dts: allwinner: h6: Enable CPU DVFS(cpufreq)

[Yangtao Li]

Add the cpufreq support of h6, source of information is as follows.

h6 cpu opp info:
https://github.com/Allwinner-Homlet/H6-BSP4.9-linux/blob/master/arch/arm64/boot/dts/sunxi/sun50iw6p1.dtsi

axp805 spec:
http://linux-sunxi.org/images/b/bc/AXP805_Datasheet_V1.0_en.pdf

---
The patchset is based on the sunxi source code and has not been tested on an
actual board.
---

Yangtao Li (4):
  arm64: dts: allwinner: h6: orangepi: Add CPU supply regulator
  arm64: dts: allwinner: h6: pine: Add CPU supply regulator
  arm64: dts: allwinner: h6: Add clock to CPU cores
  arm64: dts: allwinner: h6: Add CPU Operating Performance Points table

 .../dts/allwinner/sun50i-h6-orangepi.dtsi     |  8 ++-
 .../boot/dts/allwinner/sun50i-h6-pine-h64.dts |  8 ++-
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  | 69 +++++++++++++++++++
 3 files changed, 81 insertions(+), 4 deletions(-)

-- 
2.17.0

[PATCH 1/4] arm64: dts: allwinner: h6: orangepi: Add CPU supply regulator

[Yangtao Li]

The original orangepi use the dcdca to supply the CPU cores. According
to the axp805 spec, the range of dcdca is 0.6 to 1.1v, 1.12 to 1.52v.
In order to support more CPU frequency, slightly increase the voltage
maximum and minimum.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
index b2526dac2fcf..e7aebaf91ede 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
@@ -159,8 +159,8 @@
 
 			reg_dcdca: dcdca {
 				regulator-always-on;
-				regulator-min-microvolt = <810000>;
-				regulator-max-microvolt = <1080000>;
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <1160000>;
 				regulator-name = "vdd-cpu";
 			};
 
@@ -191,6 +191,10 @@
 	};
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdca>;
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_ph_pins>;
-- 
2.17.0

Re: [PATCH 1/4] arm64: dts: allwinner: h6: orangepi: Add CPU supply regulator

[Maxime Ripard]

Hi!

Thanks for your patches

On Thu, Feb 14, 2019 at 08:09:07AM -0500, Yangtao Li wrote:
> The original orangepi use the dcdca to supply the CPU cores. According
> to the axp805 spec, the range of dcdca is 0.6 to 1.1v, 1.12 to 1.52v.
> In order to support more CPU frequency, slightly increase the voltage
> maximum and minimum.
> 
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
> index b2526dac2fcf..e7aebaf91ede 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
> @@ -159,8 +159,8 @@
>  
>  			reg_dcdca: dcdca {
>  				regulator-always-on;
> -				regulator-min-microvolt = <810000>;
> -				regulator-max-microvolt = <1080000>;
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <1160000>;

This violates the minimum and maximum recommended voltages in the H6
datasheet. Could you clarify why you are doing so (ideally in the
commit log)?

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 1/4] arm64: dts: allwinner: h6: orangepi: Add CPU supply regulator

[Frank Lee]

On Thu, Feb 14, 2019 at 9:46 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> Hi!
>
> Thanks for your patches
>
> On Thu, Feb 14, 2019 at 08:09:07AM -0500, Yangtao Li wrote:
> > The original orangepi use the dcdca to supply the CPU cores. According
> > to the axp805 spec, the range of dcdca is 0.6 to 1.1v, 1.12 to 1.52v.
> > In order to support more CPU frequency, slightly increase the voltage
> > maximum and minimum.
> >
> > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> > ---
> >  arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
> > index b2526dac2fcf..e7aebaf91ede 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
> > @@ -159,8 +159,8 @@
> >
> >                       reg_dcdca: dcdca {
> >                               regulator-always-on;
> > -                             regulator-min-microvolt = <810000>;
> > -                             regulator-max-microvolt = <1080000>;
> > +                             regulator-min-microvolt = <800000>;
> > +                             regulator-max-microvolt = <1160000>;
>
> This violates the minimum and maximum recommended voltages in the H6
> datasheet. Could you clarify why you are doing so (ideally in the
> commit log)?
In fact, in sunxi's sdk, the actual minimum and maximum voltage of the
cpu is smaller or larger than the datasheet.

For some better quality ic, the minimum voltage can be smaller.
For some poor quality ic, the maximum voltage needs to be increased a little.

MBR,
Yangtao
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH 1/4] arm64: dts: allwinner: h6: orangepi: Add CPU supply regulator

[Maxime Ripard]

Hi,

On Thu, Feb 14, 2019 at 10:07:02PM +0800, Frank Lee wrote:
> On Thu, Feb 14, 2019 at 9:46 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > Thanks for your patches
> >
> > On Thu, Feb 14, 2019 at 08:09:07AM -0500, Yangtao Li wrote:
> > > The original orangepi use the dcdca to supply the CPU cores. According
> > > to the axp805 spec, the range of dcdca is 0.6 to 1.1v, 1.12 to 1.52v.
> > > In order to support more CPU frequency, slightly increase the voltage
> > > maximum and minimum.
> > >
> > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> > > ---
> > >  arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi | 8 ++++++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
> > > index b2526dac2fcf..e7aebaf91ede 100644
> > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
> > > @@ -159,8 +159,8 @@
> > >
> > >                       reg_dcdca: dcdca {
> > >                               regulator-always-on;
> > > -                             regulator-min-microvolt = <810000>;
> > > -                             regulator-max-microvolt = <1080000>;
> > > +                             regulator-min-microvolt = <800000>;
> > > +                             regulator-max-microvolt = <1160000>;
> >
> > This violates the minimum and maximum recommended voltages in the H6
> > datasheet. Could you clarify why you are doing so (ideally in the
> > commit log)?
>
> In fact, in sunxi's sdk, the actual minimum and maximum voltage of the
> cpu is smaller or larger than the datasheet.
> 
> For some better quality ic, the minimum voltage can be smaller.
> For some poor quality ic, the maximum voltage needs to be increased a little.

Ok, that should definitely be part of the commit log then :)

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 2/4] arm64: dts: allwinner: h6: pine: Add CPU supply regulator

[Yangtao Li]

The original pine use the dcdca to supply the CPU cores. According
to the axp805 spec, the range of dcdca is 0.6 to 1.1v, 1.12 to 1.52v.
In order to support more CPU frequency, slightly increase the voltage
maximum and minimum.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
index bdb8470fc8dc..95c81250c2b3 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
@@ -213,8 +213,8 @@
 
 			reg_dcdca: dcdca {
 				regulator-always-on;
-				regulator-min-microvolt = <810000>;
-				regulator-max-microvolt = <1080000>;
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <1160000>;
 				regulator-name = "vdd-cpu";
 			};
 
@@ -251,6 +251,10 @@
 	};
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdca>;
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_ph_pins>;
-- 
2.17.0

[PATCH 3/4] arm64: dts: allwinner: h6: Add clock to CPU cores

[Yangtao Li]

The ARM CPU cores are fed by the CPU clock from the CCU. Add a
reference to the clock for each CPU core, along with the clock
transition latency.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index d93a7add67e7..57a1390ecdc2 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -26,6 +26,8 @@
 			device_type = "cpu";
 			reg = <0>;
 			enable-method = "psci";
+			clocks = <&ccu CLK_CPUX>; 
+			clock-latency-ns = <244144>; /* 8 32k periods */
 		};
 
 		cpu1: cpu@1 {
@@ -33,6 +35,8 @@
 			device_type = "cpu";
 			reg = <1>;
 			enable-method = "psci";
+			clocks = <&ccu CLK_CPUX>; 
+			clock-latency-ns = <244144>; /* 8 32k periods */
 		};
 
 		cpu2: cpu@2 {
@@ -40,6 +44,8 @@
 			device_type = "cpu";
 			reg = <2>;
 			enable-method = "psci";
+			clocks = <&ccu CLK_CPUX>; 
+			clock-latency-ns = <244144>; /* 8 32k periods */
 		};
 
 		cpu3: cpu@3 {
@@ -47,6 +53,8 @@
 			device_type = "cpu";
 			reg = <3>;
 			enable-method = "psci";
+			clocks = <&ccu CLK_CPUX>; 
+			clock-latency-ns = <244144>; /* 8 32k periods */
 		};
 	};
 
-- 
2.17.0

[PATCH 4/4] arm64: dts: allwinner: h6: Add CPU Operating Performance Points table

[Yangtao Li]

Add an OPP (Operating Performance Points) table for the CPU cores to
enable DVFS (Dynamic Voltage & Frequency Scaling) on the H6. This
information comes from github.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 61 ++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 57a1390ecdc2..46a4a69eb38f 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -28,6 +28,8 @@
 			enable-method = "psci";
 			clocks = <&ccu CLK_CPUX>; 
 			clock-latency-ns = <244144>; /* 8 32k periods */
+			operating-points-v2 = <&cpu_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		cpu1: cpu@1 {
@@ -37,6 +39,8 @@
 			enable-method = "psci";
 			clocks = <&ccu CLK_CPUX>; 
 			clock-latency-ns = <244144>; /* 8 32k periods */
+			operating-points-v2 = <&cpu_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		cpu2: cpu@2 {
@@ -46,6 +50,8 @@
 			enable-method = "psci";
 			clocks = <&ccu CLK_CPUX>; 
 			clock-latency-ns = <244144>; /* 8 32k periods */
+			operating-points-v2 = <&cpu_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		cpu3: cpu@3 {
@@ -55,6 +61,61 @@
 			enable-method = "psci";
 			clocks = <&ccu CLK_CPUX>; 
 			clock-latency-ns = <244144>; /* 8 32k periods */
+			operating-points-v2 = <&cpu_opp_table>;
+			#cooling-cells = <2>;
+		};
+	};
+
+	cpu_opp_table: opp_table {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp@480000000 {
+			opp-hz = /bits/ 64 <480000000>;
+			opp-microvolt = <800000 800000 880000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+
+		opp@720000000 {
+			opp-hz = /bits/ 64 <720000000>;
+			opp-microvolt = <800000 800000 880000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+
+		opp@816000000 {
+			opp-hz = /bits/ 64 <816000000>;
+			opp-microvolt = <800000 800000 880000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+
+		opp@888000000 {
+			opp-hz = /bits/ 64 <888000000>;
+			opp-microvolt = <800000 800000 940000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+
+		opp@1080000000 {
+			opp-hz = /bits/ 64 <1080000000>;
+			opp-microvolt = <840000 840000 1060000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+
+		opp@1320000000 {
+			opp-hz = /bits/ 64 <1320000000>;
+			opp-microvolt = <900000 900000 1160000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+
+		opp@1488000000 {
+			opp-hz = /bits/ 64 <1488000000>;
+			opp-microvolt = <960000 960000 1160000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+
+		opp@1800000000 {
+			opp-hz = /bits/ 64 <1800000000>;
+			opp-microvolt = <1060000 1060000 1160000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
 		};
 	};
 
-- 
2.17.0

Re: [PATCH 4/4] arm64: dts: allwinner: h6: Add CPU Operating Performance Points table

[Maxime Ripard]

Hi,

On Thu, Feb 14, 2019 at 08:09:10AM -0500, Yangtao Li wrote:
> Add an OPP (Operating Performance Points) table for the CPU cores to
> enable DVFS (Dynamic Voltage & Frequency Scaling) on the H6. This
> information comes from github.
> 
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 61 ++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> index 57a1390ecdc2..46a4a69eb38f 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> @@ -28,6 +28,8 @@
>  			enable-method = "psci";
>  			clocks = <&ccu CLK_CPUX>; 
>  			clock-latency-ns = <244144>; /* 8 32k periods */
> +			operating-points-v2 = <&cpu_opp_table>;
> +			#cooling-cells = <2>;
>  		};
>  
>  		cpu1: cpu@1 {
> @@ -37,6 +39,8 @@
>  			enable-method = "psci";
>  			clocks = <&ccu CLK_CPUX>; 
>  			clock-latency-ns = <244144>; /* 8 32k periods */
> +			operating-points-v2 = <&cpu_opp_table>;
> +			#cooling-cells = <2>;
>  		};
>  
>  		cpu2: cpu@2 {
> @@ -46,6 +50,8 @@
>  			enable-method = "psci";
>  			clocks = <&ccu CLK_CPUX>; 
>  			clock-latency-ns = <244144>; /* 8 32k periods */
> +			operating-points-v2 = <&cpu_opp_table>;
> +			#cooling-cells = <2>;
>  		};
>  
>  		cpu3: cpu@3 {
> @@ -55,6 +61,61 @@
>  			enable-method = "psci";
>  			clocks = <&ccu CLK_CPUX>; 
>  			clock-latency-ns = <244144>; /* 8 32k periods */
> +			operating-points-v2 = <&cpu_opp_table>;
> +			#cooling-cells = <2>;
> +		};
> +	};
> +
> +	cpu_opp_table: opp_table {
> +		compatible = "operating-points-v2";
> +		opp-shared;
> +
> +		opp@480000000 {
> +			opp-hz = /bits/ 64 <480000000>;
> +			opp-microvolt = <800000 800000 880000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +
> +		opp@720000000 {
> +			opp-hz = /bits/ 64 <720000000>;
> +			opp-microvolt = <800000 800000 880000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +
> +		opp@816000000 {
> +			opp-hz = /bits/ 64 <816000000>;
> +			opp-microvolt = <800000 800000 880000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +
> +		opp@888000000 {
> +			opp-hz = /bits/ 64 <888000000>;
> +			opp-microvolt = <800000 800000 940000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +
> +		opp@1080000000 {
> +			opp-hz = /bits/ 64 <1080000000>;
> +			opp-microvolt = <840000 840000 1060000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +
> +		opp@1320000000 {
> +			opp-hz = /bits/ 64 <1320000000>;
> +			opp-microvolt = <900000 900000 1160000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +
> +		opp@1488000000 {
> +			opp-hz = /bits/ 64 <1488000000>;
> +			opp-microvolt = <960000 960000 1160000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +
> +		opp@1800000000 {
> +			opp-hz = /bits/ 64 <1800000000>;
> +			opp-microvolt = <1060000 1060000 1160000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */

So we definitely want to have that tested, especially since cpufreq
can lead to all kind of hard to debug errors (brown-outs, CPU lockups,
cache corruption, etc.). I good way to test that would be to use
cpufreq-ljt-stress-test here:
https://github.com/ssvb/cpuburn-arm/blob/master/cpufreq-ljt-stress-test

I'm especially worried about the higher frequencies that will probably
make the SoC heat too much

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 4/4] arm64: dts: allwinner: h6: Add CPU Operating Performance Points table

[Frank Lee]

On Thu, Feb 14, 2019 at 10:38 PM Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
>
> Hi,
>
> On Thu, Feb 14, 2019 at 08:09:10AM -0500, Yangtao Li wrote:
> > Add an OPP (Operating Performance Points) table for the CPU cores to
> > enable DVFS (Dynamic Voltage & Frequency Scaling) on the H6. This
> > information comes from github.
> >
> > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> > ---
> >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 61 ++++++++++++++++++++
> >  1 file changed, 61 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > index 57a1390ecdc2..46a4a69eb38f 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > @@ -28,6 +28,8 @@
> >                       enable-method = "psci";
> >                       clocks = <&ccu CLK_CPUX>;
> >                       clock-latency-ns = <244144>; /* 8 32k periods */
> > +                     operating-points-v2 = <&cpu_opp_table>;
> > +                     #cooling-cells = <2>;
> >               };
> >
> >               cpu1: cpu@1 {
> > @@ -37,6 +39,8 @@
> >                       enable-method = "psci";
> >                       clocks = <&ccu CLK_CPUX>;
> >                       clock-latency-ns = <244144>; /* 8 32k periods */
> > +                     operating-points-v2 = <&cpu_opp_table>;
> > +                     #cooling-cells = <2>;
> >               };
> >
> >               cpu2: cpu@2 {
> > @@ -46,6 +50,8 @@
> >                       enable-method = "psci";
> >                       clocks = <&ccu CLK_CPUX>;
> >                       clock-latency-ns = <244144>; /* 8 32k periods */
> > +                     operating-points-v2 = <&cpu_opp_table>;
> > +                     #cooling-cells = <2>;
> >               };
> >
> >               cpu3: cpu@3 {
> > @@ -55,6 +61,61 @@
> >                       enable-method = "psci";
> >                       clocks = <&ccu CLK_CPUX>;
> >                       clock-latency-ns = <244144>; /* 8 32k periods */
> > +                     operating-points-v2 = <&cpu_opp_table>;
> > +                     #cooling-cells = <2>;
> > +             };
> > +     };
> > +
> > +     cpu_opp_table: opp_table {
> > +             compatible = "operating-points-v2";
> > +             opp-shared;
> > +
> > +             opp@480000000 {
> > +                     opp-hz = /bits/ 64 <480000000>;
> > +                     opp-microvolt = <800000 800000 880000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
> > +
> > +             opp@720000000 {
> > +                     opp-hz = /bits/ 64 <720000000>;
> > +                     opp-microvolt = <800000 800000 880000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
> > +
> > +             opp@816000000 {
> > +                     opp-hz = /bits/ 64 <816000000>;
> > +                     opp-microvolt = <800000 800000 880000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
> > +
> > +             opp@888000000 {
> > +                     opp-hz = /bits/ 64 <888000000>;
> > +                     opp-microvolt = <800000 800000 940000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
> > +
> > +             opp@1080000000 {
> > +                     opp-hz = /bits/ 64 <1080000000>;
> > +                     opp-microvolt = <840000 840000 1060000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
> > +
> > +             opp@1320000000 {
> > +                     opp-hz = /bits/ 64 <1320000000>;
> > +                     opp-microvolt = <900000 900000 1160000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
> > +
> > +             opp@1488000000 {
> > +                     opp-hz = /bits/ 64 <1488000000>;
> > +                     opp-microvolt = <960000 960000 1160000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
> > +
> > +             opp@1800000000 {
> > +                     opp-hz = /bits/ 64 <1800000000>;
> > +                     opp-microvolt = <1060000 1060000 1160000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
>
> So we definitely want to have that tested, especially since cpufreq
> can lead to all kind of hard to debug errors (brown-outs, CPU lockups,
> cache corruption, etc.). I good way to test that would be to use
> cpufreq-ljt-stress-test here:
> https://github.com/ssvb/cpuburn-arm/blob/master/cpufreq-ljt-stress-test
>
> I'm especially worried about the higher frequencies that will probably
> make the SoC heat too much
Indeed, in order to avoid this situation, it is best to have cpu cooling
support(But now it does not support thermal driver? ).


In this case, perhaps we should remove the frequency beyond a certain
range to avoid the CPU being too hot?

Yangtao
>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH 4/4] arm64: dts: allwinner: h6: Add CPU Operating Performance Points table

[Frank Lee]

HI Maxime,

How about to implement a thermal driver that is not integrated with gpadc ？

GPADC on soc is rarely used now.

MBR,
Yangtao

Re: [PATCH 4/4] arm64: dts: allwinner: h6: Add CPU Operating Performance Points table

[Maxime Ripard]

Hi

On Fri, Feb 15, 2019 at 12:56:03AM +0800, Frank Lee wrote:
> How about to implement a thermal driver that is not integrated with
> gpadc ？
> 
> GPADC on soc is rarely used now.

You mean GPADC in general, or the gpadc driver in particular?
Otherwise, yes, having a driver for the thermal sensor in the H6
sounds like a good plan for thermal throttling, but I don't see it as
a dependency for that whole series.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 4/4] arm64: dts: allwinner: h6: Add CPU Operating Performance Points table

[Frank Lee]

On Fri, Feb 15, 2019 at 9:56 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> Hi
>
> On Fri, Feb 15, 2019 at 12:56:03AM +0800, Frank Lee wrote:
> > How about to implement a thermal driver that is not integrated with
> > gpadc ？
> >
> > GPADC on soc is rarely used now.
>
> You mean GPADC in general, or the gpadc driver in particular?
> Otherwise, yes, having a driver for the thermal sensor in the H6
> sounds like a good plan for thermal throttling, but I don't see it as
> a dependency for that whole series.
At present, sunxi's thermal driver is integrated with the gpadc driver.
I have a idea to implement the thermal driver alone. I haven't started yet,
and it may take a while to do it.

How about the plan to implement a thermal drive alone and put i
under drivers/thermal?

In addition to the cpu dvfs of H6, patch v2 has been sent.

Thanks,
Yangtao
>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH 4/4] arm64: dts: allwinner: h6: Add CPU Operating Performance Points table

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]

Hi,

On Fri, Feb 15, 2019 at 10:09:51PM +0800, Frank Lee wrote:
> On Fri, Feb 15, 2019 at 9:56 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > Hi
> >
> > On Fri, Feb 15, 2019 at 12:56:03AM +0800, Frank Lee wrote:
> > > How about to implement a thermal driver that is not integrated with
> > > gpadc ？
> > >
> > > GPADC on soc is rarely used now.
> >
> > You mean GPADC in general, or the gpadc driver in particular?
> > Otherwise, yes, having a driver for the thermal sensor in the H6
> > sounds like a good plan for thermal throttling, but I don't see it as
> > a dependency for that whole series.
>
> At present, sunxi's thermal driver is integrated with the gpadc driver.
> I have a idea to implement the thermal driver alone. I haven't started yet,
> and it may take a while to do it.
> 
> How about the plan to implement a thermal drive alone and put i
> under drivers/thermal?

The Thermal Sensor is basically a stripped down GPADC, which is why it
would make sense to have a single driver.

There was a pending series doing this that could be a good idea to
improve and merge:
https://lkml.org/lkml/2018/8/30/729

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/4] arm64: dts: allwinner: h6: Add CPU Operating Performance Points table

[Maxime Ripard]

On Thu, Feb 14, 2019 at 10:52:16PM +0800, Frank Lee wrote:
> On Thu, Feb 14, 2019 at 10:38 PM Maxime Ripard
> <maxime.ripard@bootlin.com> wrote:
> >
> > Hi,
> >
> > On Thu, Feb 14, 2019 at 08:09:10AM -0500, Yangtao Li wrote:
> > > Add an OPP (Operating Performance Points) table for the CPU cores to
> > > enable DVFS (Dynamic Voltage & Frequency Scaling) on the H6. This
> > > information comes from github.
> > >
> > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> > > ---
> > >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 61 ++++++++++++++++++++
> > >  1 file changed, 61 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > index 57a1390ecdc2..46a4a69eb38f 100644
> > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > @@ -28,6 +28,8 @@
> > >                       enable-method = "psci";
> > >                       clocks = <&ccu CLK_CPUX>;
> > >                       clock-latency-ns = <244144>; /* 8 32k periods */
> > > +                     operating-points-v2 = <&cpu_opp_table>;
> > > +                     #cooling-cells = <2>;
> > >               };
> > >
> > >               cpu1: cpu@1 {
> > > @@ -37,6 +39,8 @@
> > >                       enable-method = "psci";
> > >                       clocks = <&ccu CLK_CPUX>;
> > >                       clock-latency-ns = <244144>; /* 8 32k periods */
> > > +                     operating-points-v2 = <&cpu_opp_table>;
> > > +                     #cooling-cells = <2>;
> > >               };
> > >
> > >               cpu2: cpu@2 {
> > > @@ -46,6 +50,8 @@
> > >                       enable-method = "psci";
> > >                       clocks = <&ccu CLK_CPUX>;
> > >                       clock-latency-ns = <244144>; /* 8 32k periods */
> > > +                     operating-points-v2 = <&cpu_opp_table>;
> > > +                     #cooling-cells = <2>;
> > >               };
> > >
> > >               cpu3: cpu@3 {
> > > @@ -55,6 +61,61 @@
> > >                       enable-method = "psci";
> > >                       clocks = <&ccu CLK_CPUX>;
> > >                       clock-latency-ns = <244144>; /* 8 32k periods */
> > > +                     operating-points-v2 = <&cpu_opp_table>;
> > > +                     #cooling-cells = <2>;
> > > +             };
> > > +     };
> > > +
> > > +     cpu_opp_table: opp_table {
> > > +             compatible = "operating-points-v2";
> > > +             opp-shared;
> > > +
> > > +             opp@480000000 {
> > > +                     opp-hz = /bits/ 64 <480000000>;
> > > +                     opp-microvolt = <800000 800000 880000>;
> > > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > > +             };
> > > +
> > > +             opp@720000000 {
> > > +                     opp-hz = /bits/ 64 <720000000>;
> > > +                     opp-microvolt = <800000 800000 880000>;
> > > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > > +             };
> > > +
> > > +             opp@816000000 {
> > > +                     opp-hz = /bits/ 64 <816000000>;
> > > +                     opp-microvolt = <800000 800000 880000>;
> > > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > > +             };
> > > +
> > > +             opp@888000000 {
> > > +                     opp-hz = /bits/ 64 <888000000>;
> > > +                     opp-microvolt = <800000 800000 940000>;
> > > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > > +             };
> > > +
> > > +             opp@1080000000 {
> > > +                     opp-hz = /bits/ 64 <1080000000>;
> > > +                     opp-microvolt = <840000 840000 1060000>;
> > > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > > +             };
> > > +
> > > +             opp@1320000000 {
> > > +                     opp-hz = /bits/ 64 <1320000000>;
> > > +                     opp-microvolt = <900000 900000 1160000>;
> > > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > > +             };
> > > +
> > > +             opp@1488000000 {
> > > +                     opp-hz = /bits/ 64 <1488000000>;
> > > +                     opp-microvolt = <960000 960000 1160000>;
> > > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > > +             };
> > > +
> > > +             opp@1800000000 {
> > > +                     opp-hz = /bits/ 64 <1800000000>;
> > > +                     opp-microvolt = <1060000 1060000 1160000>;
> > > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> >
> > So we definitely want to have that tested, especially since cpufreq
> > can lead to all kind of hard to debug errors (brown-outs, CPU lockups,
> > cache corruption, etc.). I good way to test that would be to use
> > cpufreq-ljt-stress-test here:
> > https://github.com/ssvb/cpuburn-arm/blob/master/cpufreq-ljt-stress-test
> >
> > I'm especially worried about the higher frequencies that will probably
> > make the SoC heat too much
>
> Indeed, in order to avoid this situation, it is best to have cpu cooling
> support(But now it does not support thermal driver? ).
> 
> In this case, perhaps we should remove the frequency beyond a certain
> range to avoid the CPU being too hot?

Yeah, that seems like a nice solution until we have the thermal sensor
running.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com