diff mbox

ARM64: dts: meson-gxbb-odroidc2: Disable SCPI DVFS

Message ID 1483628549-29486-1-git-send-email-narmstrong@baylibre.com (mailing list archive)
State Superseded
Headers show

Commit Message

Neil Armstrong Jan. 5, 2017, 3:02 p.m. UTC
The current hardware is not able to run with all cores enabled at a
cluster frequency superior at 1536MHz.
But the currently shipped u-boot for the platform still reports an OPP
table with possible DVFS frequency up to 2GHz, and will not change since
the off-tree linux tree supports limiting the OPPs with a kernel parameter.
A recent u-boot change reports the boot-time DVFS around 100MHz and
the default performance cpufreq governor sets the maximum frequency.
Previous version of u-boot reported to be already at the max OPP and
left the OPP as is.
Nevertheless, other governors like ondemand could setup the max frequency
and make the system crash.

This patch disables the DVFS clock and disables cpufreq.

Fixes: 70db166a2baa ("ARM64: dts: meson-gxbb: Add SCPI with cpufreq & sensors Nodes")
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Michał Zegan Jan. 5, 2017, 7:04 p.m. UTC | #1
Hello.

The patch causes cpufreq module (scpi-cpufreq) not to detect cpufreq, so
it actually works, but...
Loading the module causes few errors because of not found frequencies or
something, then it is all okay. However after loading scpi-cpufreq you
cannot actually power the cpu off and on. You will power it off
successfully, but when trying to power it on, the cpufreq driver will
error out, and then after it happens, the cpu that was trying to go
online will be offline again, and that is a little... unfortunate. The
question is, and I cannot really test that: will the module actually
autoload after this change?

W dniu 05.01.2017 o 16:02, Neil Armstrong pisze:
> The current hardware is not able to run with all cores enabled at a
> cluster frequency superior at 1536MHz.
> But the currently shipped u-boot for the platform still reports an OPP
> table with possible DVFS frequency up to 2GHz, and will not change since
> the off-tree linux tree supports limiting the OPPs with a kernel parameter.
> A recent u-boot change reports the boot-time DVFS around 100MHz and
> the default performance cpufreq governor sets the maximum frequency.
> Previous version of u-boot reported to be already at the max OPP and
> left the OPP as is.
> Nevertheless, other governors like ondemand could setup the max frequency
> and make the system crash.
> 
> This patch disables the DVFS clock and disables cpufreq.
> 
> Fixes: 70db166a2baa ("ARM64: dts: meson-gxbb: Add SCPI with cpufreq & sensors Nodes")
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> index 238fbea..5e63e3b 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> @@ -137,6 +137,10 @@
>  	};
>  };
>  
> +&scpi_dvfs {
> +	status = "disabled";
> +};
> +
>  &uart_AO {
>  	status = "okay";
>  	pinctrl-0 = <&uart_ao_a_pins>;
>
Neil Armstrong Jan. 6, 2017, 8:04 a.m. UTC | #2
On 01/05/2017 08:04 PM, Michał Zegan wrote:
> Hello.
> 
> The patch causes cpufreq module (scpi-cpufreq) not to detect cpufreq, so
> it actually works, but...
> Loading the module causes few errors because of not found frequencies or
> something, then it is all okay. However after loading scpi-cpufreq you
> cannot actually power the cpu off and on. You will power it off
> successfully, but when trying to power it on, the cpufreq driver will
> error out, and then after it happens, the cpu that was trying to go
> online will be offline again, and that is a little... unfortunate. The
> question is, and I cannot really test that: will the module actually
> autoload after this change?

Hi Michal,

You are right, it breaks cpufreq and the cpu hotplug feature, I will send a v2 completely disabling cpufreq instead.

For the module autoloading, the arm_scpi.ko must be loaded before the other scpi modules.

Please ask Sudeep Holla <sudeep.holla@arm.com> if module autoloading for scpi is meant to work.

Neil


> 
> W dniu 05.01.2017 o 16:02, Neil Armstrong pisze:
>> The current hardware is not able to run with all cores enabled at a
>> cluster frequency superior at 1536MHz.
>> But the currently shipped u-boot for the platform still reports an OPP
>> table with possible DVFS frequency up to 2GHz, and will not change since
>> the off-tree linux tree supports limiting the OPPs with a kernel parameter.
>> A recent u-boot change reports the boot-time DVFS around 100MHz and
>> the default performance cpufreq governor sets the maximum frequency.
>> Previous version of u-boot reported to be already at the max OPP and
>> left the OPP as is.
>> Nevertheless, other governors like ondemand could setup the max frequency
>> and make the system crash.
>>
>> This patch disables the DVFS clock and disables cpufreq.
>>
>> Fixes: 70db166a2baa ("ARM64: dts: meson-gxbb: Add SCPI with cpufreq & sensors Nodes")
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
>> index 238fbea..5e63e3b 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
>> @@ -137,6 +137,10 @@
>>  	};
>>  };
>>  
>> +&scpi_dvfs {
>> +	status = "disabled";
>> +};
>> +
>>  &uart_AO {
>>  	status = "okay";
>>  	pinctrl-0 = <&uart_ao_a_pins>;
>>
>
Sudeep Holla Jan. 6, 2017, 11:54 a.m. UTC | #3
Hi Michał,

On 05/01/17 19:04, Michał Zegan wrote:
> Hello.
> 
> The patch causes cpufreq module (scpi-cpufreq) not to detect cpufreq, so
> it actually works, but...
> Loading the module causes few errors because of not found frequencies or
> something, then it is all okay. However after loading scpi-cpufreq you
> cannot actually power the cpu off and on. You will power it off
> successfully, but when trying to power it on, the cpufreq driver will
> error out, 

Yes I had noticed this in past, this needs to be fixed. I had a patch
and seems like it slipped through the cracks. I will fins and post it.

> and then after it happens, the cpu that was trying to go
> online will be offline again, and that is a little... unfortunate. The

IIUC, you mean the cpufreq drive spits error on every hotplug event ?
If so yes, otherwise I think I didn't understand you concern above.

> question is, and I cannot really test that: will the module actually
> autoload after this change?
>

It should work, I had tested this in past.
Michał Zegan Jan. 6, 2017, 1:12 p.m. UTC | #4
Yes, I meant what you think I meant. :) thanks

W dniu 06.01.2017 o 12:54, Sudeep Holla pisze:
> Hi Michał,
> 
> On 05/01/17 19:04, Michał Zegan wrote:
>> Hello.
>>
>> The patch causes cpufreq module (scpi-cpufreq) not to detect cpufreq, so
>> it actually works, but...
>> Loading the module causes few errors because of not found frequencies or
>> something, then it is all okay. However after loading scpi-cpufreq you
>> cannot actually power the cpu off and on. You will power it off
>> successfully, but when trying to power it on, the cpufreq driver will
>> error out, 
> 
> Yes I had noticed this in past, this needs to be fixed. I had a patch
> and seems like it slipped through the cracks. I will fins and post it.
> 
>> and then after it happens, the cpu that was trying to go
>> online will be offline again, and that is a little... unfortunate. The
> 
> IIUC, you mean the cpufreq drive spits error on every hotplug event ?
> If so yes, otherwise I think I didn't understand you concern above.
> 
>> question is, and I cannot really test that: will the module actually
>> autoload after this change?
>>
> 
> It should work, I had tested this in past.
>
diff mbox

Patch

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index 238fbea..5e63e3b 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -137,6 +137,10 @@ 
 	};
 };
 
+&scpi_dvfs {
+	status = "disabled";
+};
+
 &uart_AO {
 	status = "okay";
 	pinctrl-0 = <&uart_ao_a_pins>;