ARM: dts: sun8i: a83t: Add CPU regulator supplies for A83T boards
diff mbox

Message ID 20180627022709.27434-1-wens@csie.org
State New, archived
Headers show

Commit Message

Chen-Yu Tsai June 27, 2018, 2:27 a.m. UTC
The OPPs for the A83T CPU cores were added in v4.17 in commit 2db639d8c166
("ARM: dts: sun8i: a83t: add stable OPP tables and CPUfreq"), but board
level regulator supplies for the CPU clusters were only added for the
TBS-A711 tablet. This means the other A83T boards do not benefit from
voltage scaling, or worse, if the implementation does not scale the
frequency when the voltage is fixed, no benefit at all.

Add board level CPU cluster power supplies to all the A83T development
boards, so they can have proper dynamic CPU voltage and frequency scaling.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts | 8 ++++++++
 arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts           | 8 ++++++++
 arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts       | 8 ++++++++
 3 files changed, 24 insertions(+)

Comments

Maxime Ripard June 27, 2018, 5:04 p.m. UTC | #1
On Wed, Jun 27, 2018 at 10:27:09AM +0800, Chen-Yu Tsai wrote:
> The OPPs for the A83T CPU cores were added in v4.17 in commit 2db639d8c166
> ("ARM: dts: sun8i: a83t: add stable OPP tables and CPUfreq"), but board
> level regulator supplies for the CPU clusters were only added for the
> TBS-A711 tablet. This means the other A83T boards do not benefit from
> voltage scaling, or worse, if the implementation does not scale the
> frequency when the voltage is fixed, no benefit at all.
> 
> Add board level CPU cluster power supplies to all the A83T development
> boards, so they can have proper dynamic CPU voltage and frequency scaling.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, thanks!
Maxime
Ondřej Jirman June 27, 2018, 6:45 p.m. UTC | #2
Hi,

On Wed, Jun 27, 2018 at 10:27:09AM +0800, Chen-Yu Tsai wrote:
> The OPPs for the A83T CPU cores were added in v4.17 in commit 2db639d8c166
> ("ARM: dts: sun8i: a83t: add stable OPP tables and CPUfreq"), but board
> level regulator supplies for the CPU clusters were only added for the
> TBS-A711 tablet. This means the other A83T boards do not benefit from
> voltage scaling, or worse, if the implementation does not scale the
> frequency when the voltage is fixed, no benefit at all.
> 
> Add board level CPU cluster power supplies to all the A83T development
> boards, so they can have proper dynamic CPU voltage and frequency scaling.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts | 8 ++++++++
>  arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts           | 8 ++++++++
>  arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts       | 8 ++++++++
>  3 files changed, 24 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts b/arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts
> index 36ecebaff3c0..1c012a4def16 100644
> --- a/arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts
> +++ b/arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts
> @@ -79,6 +79,14 @@
>  	};
>  };
>  
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
> +&cpu100 {
> +	cpu-supply = <&reg_dcdc3>;
> +};
> +

Shouldn't this be done for all CPU cores? Say you start the kernel with
maxcpus=1 and then enable CPU5 later on, will the regulator get enabled,
if it is not enabled by always-on setting?

regards,
  o.

>  &ehci0 {
>  	status = "okay";
>  };
> diff --git a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
> index 3b579d7567c8..c7ce4158d6c8 100644
> --- a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
> +++ b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
> @@ -107,6 +107,14 @@
>  	};
>  };
>  
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
> +&cpu100 {
> +	cpu-supply = <&reg_dcdc3>;
> +};
> +
>  &de {
>  	status = "okay";
>  };
> diff --git a/arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts b/arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts
> index 88decb0747ac..e5f0645e53a7 100644
> --- a/arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts
> +++ b/arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts
> @@ -145,6 +145,14 @@
>  	};
>  };
>  
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
> +&cpu100 {
> +	cpu-supply = <&reg_dcdc3>;
> +};
> +
>  &ehci0 {
>  	/* GL830 USB-to-SATA bridge here */
>  	status = "okay";
> -- 
> 2.18.0
> 
> -- 
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
Chen-Yu Tsai June 28, 2018, 3:10 a.m. UTC | #3
On Thu, Jun 28, 2018 at 2:45 AM, Ondřej Jirman
<doudahwezomiechahtah@xff.cz> wrote:
> Hi,
>
> On Wed, Jun 27, 2018 at 10:27:09AM +0800, Chen-Yu Tsai wrote:
>> The OPPs for the A83T CPU cores were added in v4.17 in commit 2db639d8c166
>> ("ARM: dts: sun8i: a83t: add stable OPP tables and CPUfreq"), but board
>> level regulator supplies for the CPU clusters were only added for the
>> TBS-A711 tablet. This means the other A83T boards do not benefit from
>> voltage scaling, or worse, if the implementation does not scale the
>> frequency when the voltage is fixed, no benefit at all.
>>
>> Add board level CPU cluster power supplies to all the A83T development
>> boards, so they can have proper dynamic CPU voltage and frequency scaling.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>>  arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts | 8 ++++++++
>>  arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts           | 8 ++++++++
>>  arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts       | 8 ++++++++
>>  3 files changed, 24 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts b/arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts
>> index 36ecebaff3c0..1c012a4def16 100644
>> --- a/arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts
>> +++ b/arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts
>> @@ -79,6 +79,14 @@
>>       };
>>  };
>>
>> +&cpu0 {
>> +     cpu-supply = <&reg_dcdc2>;
>> +};
>> +
>> +&cpu100 {
>> +     cpu-supply = <&reg_dcdc3>;
>> +};
>> +
>
> Shouldn't this be done for all CPU cores? Say you start the kernel with
> maxcpus=1 and then enable CPU5 later on, will the regulator get enabled,
> if it is not enabled by always-on setting?

As it stands, the cpufreq framework doesn't enable the regulator anyway.
There's nothing in the bindings that say how this should be set either.

ChenYu

>
> regards,
>   o.
>
>>  &ehci0 {
>>       status = "okay";
>>  };
>> diff --git a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
>> index 3b579d7567c8..c7ce4158d6c8 100644
>> --- a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
>> +++ b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
>> @@ -107,6 +107,14 @@
>>       };
>>  };
>>
>> +&cpu0 {
>> +     cpu-supply = <&reg_dcdc2>;
>> +};
>> +
>> +&cpu100 {
>> +     cpu-supply = <&reg_dcdc3>;
>> +};
>> +
>>  &de {
>>       status = "okay";
>>  };
>> diff --git a/arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts b/arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts
>> index 88decb0747ac..e5f0645e53a7 100644
>> --- a/arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts
>> +++ b/arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts
>> @@ -145,6 +145,14 @@
>>       };
>>  };
>>
>> +&cpu0 {
>> +     cpu-supply = <&reg_dcdc2>;
>> +};
>> +
>> +&cpu100 {
>> +     cpu-supply = <&reg_dcdc3>;
>> +};
>> +
>>  &ehci0 {
>>       /* GL830 USB-to-SATA bridge here */
>>       status = "okay";
>> --
>> 2.18.0
>>
>> --
>> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.

Patch
diff mbox

diff --git a/arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts b/arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts
index 36ecebaff3c0..1c012a4def16 100644
--- a/arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts
@@ -79,6 +79,14 @@ 
 	};
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
+&cpu100 {
+	cpu-supply = <&reg_dcdc3>;
+};
+
 &ehci0 {
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
index 3b579d7567c8..c7ce4158d6c8 100644
--- a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
@@ -107,6 +107,14 @@ 
 	};
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
+&cpu100 {
+	cpu-supply = <&reg_dcdc3>;
+};
+
 &de {
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts b/arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts
index 88decb0747ac..e5f0645e53a7 100644
--- a/arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts
@@ -145,6 +145,14 @@ 
 	};
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
+&cpu100 {
+	cpu-supply = <&reg_dcdc3>;
+};
+
 &ehci0 {
 	/* GL830 USB-to-SATA bridge here */
 	status = "okay";