diff mbox series

ARM: tegra: Populate OPP table for Tegra20 Ventana

Message ID 20201111103847.152721-1-jonathanh@nvidia.com
State Accepted
Headers show
Series ARM: tegra: Populate OPP table for Tegra20 Ventana | expand

Commit Message

Jon Hunter Nov. 11, 2020, 10:38 a.m. UTC
Commit 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver
(Tegra30 supported now)") update the Tegra20 CPUFREQ driver to use the
generic CPUFREQ device-tree driver. Since this change CPUFREQ support
on the Tegra20 Ventana platform has been broken because the necessary
device-tree nodes with the operating point information are not populated
for this platform. Fix this by updating device-tree for Venata to
include the operating point informration for Tegra20.

Fixes: 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver (Tegra30 supported now)")
Cc: stable@vger.kernel.org

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 arch/arm/boot/dts/tegra20-ventana.dts | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Dmitry Osipenko Nov. 11, 2020, 1:47 p.m. UTC | #1
11.11.2020 13:38, Jon Hunter пишет:
> Commit 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver
> (Tegra30 supported now)") update the Tegra20 CPUFREQ driver to use the
> generic CPUFREQ device-tree driver. Since this change CPUFREQ support
> on the Tegra20 Ventana platform has been broken because the necessary
> device-tree nodes with the operating point information are not populated
> for this platform. Fix this by updating device-tree for Venata to
> include the operating point informration for Tegra20.
> 
> Fixes: 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver (Tegra30 supported now)")
> Cc: stable@vger.kernel.org
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  arch/arm/boot/dts/tegra20-ventana.dts | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
> index b158771ac0b7..055334ae3d28 100644
> --- a/arch/arm/boot/dts/tegra20-ventana.dts
> +++ b/arch/arm/boot/dts/tegra20-ventana.dts
> @@ -3,6 +3,7 @@
>  
>  #include <dt-bindings/input/input.h>
>  #include "tegra20.dtsi"
> +#include "tegra20-cpu-opp.dtsi"
>  
>  / {
>  	model = "NVIDIA Tegra20 Ventana evaluation board";
> @@ -592,6 +593,16 @@ clk32k_in: clock@0 {
>  		#clock-cells = <0>;
>  	};
>  
> +	cpus {
> +		cpu0: cpu@0 {
> +			operating-points-v2 = <&cpu0_opp_table>;
> +		};
> +
> +		cpu@1 {
> +			operating-points-v2 = <&cpu0_opp_table>;
> +		};
> +	};
> +
>  	gpio-keys {
>  		compatible = "gpio-keys";
>  
> 

This could be wrong to do because CPU voltage is fixed to 1000mV in
Ventana's DT, are you sure that higher clock rates don't require higher
voltages? What is the CPU process ID and SoC speedo ID on Ventana?

You could easily hook up CPU voltage scaling, please see acer-500 DT and
patch [1] for examples of how to set up regulators in DT. But then it
shouldn't be a stable patch.

[1]
https://patchwork.ozlabs.org/project/linux-tegra/patch/20201104234427.26477-27-digetx@gmail.com/
Jon Hunter Nov. 11, 2020, 2:42 p.m. UTC | #2
On 11/11/2020 13:47, Dmitry Osipenko wrote:
> 11.11.2020 13:38, Jon Hunter пишет:
>> Commit 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver
>> (Tegra30 supported now)") update the Tegra20 CPUFREQ driver to use the
>> generic CPUFREQ device-tree driver. Since this change CPUFREQ support
>> on the Tegra20 Ventana platform has been broken because the necessary
>> device-tree nodes with the operating point information are not populated
>> for this platform. Fix this by updating device-tree for Venata to
>> include the operating point informration for Tegra20.
>>
>> Fixes: 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver (Tegra30 supported now)")
>> Cc: stable@vger.kernel.org
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  arch/arm/boot/dts/tegra20-ventana.dts | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
>> index b158771ac0b7..055334ae3d28 100644
>> --- a/arch/arm/boot/dts/tegra20-ventana.dts
>> +++ b/arch/arm/boot/dts/tegra20-ventana.dts
>> @@ -3,6 +3,7 @@
>>  
>>  #include <dt-bindings/input/input.h>
>>  #include "tegra20.dtsi"
>> +#include "tegra20-cpu-opp.dtsi"
>>  
>>  / {
>>  	model = "NVIDIA Tegra20 Ventana evaluation board";
>> @@ -592,6 +593,16 @@ clk32k_in: clock@0 {
>>  		#clock-cells = <0>;
>>  	};
>>  
>> +	cpus {
>> +		cpu0: cpu@0 {
>> +			operating-points-v2 = <&cpu0_opp_table>;
>> +		};
>> +
>> +		cpu@1 {
>> +			operating-points-v2 = <&cpu0_opp_table>;
>> +		};
>> +	};
>> +
>>  	gpio-keys {
>>  		compatible = "gpio-keys";
>>  
>>
> 
> This could be wrong to do because CPU voltage is fixed to 1000mV in
> Ventana's DT, are you sure that higher clock rates don't require higher
> voltages? What is the CPU process ID and SoC speedo ID on Ventana?

I looked at that and I did not see any voltage scaling being done with
the previous version of the CPUFREQ driver on Ventana. I will double
check but if it should be then I guess that was broken before.

> You could easily hook up CPU voltage scaling, please see acer-500 DT and
> patch [1] for examples of how to set up regulators in DT. But then it
> shouldn't be a stable patch.

That assumes that you are in the same country as the board you are
testing on :-)

I went back and verified that CPUFREQ scaling is working on stable
kernels 4.4, 4.9, 4.14, 4.19 and 5.4 on Ventana. It is only after your
change that it no longer works and yes it should be in stable.

Jon
Jon Hunter Nov. 11, 2020, 8:31 p.m. UTC | #3
On 11/11/2020 13:47, Dmitry Osipenko wrote:
> 11.11.2020 13:38, Jon Hunter пишет:
>> Commit 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver
>> (Tegra30 supported now)") update the Tegra20 CPUFREQ driver to use the
>> generic CPUFREQ device-tree driver. Since this change CPUFREQ support
>> on the Tegra20 Ventana platform has been broken because the necessary
>> device-tree nodes with the operating point information are not populated
>> for this platform. Fix this by updating device-tree for Venata to
>> include the operating point informration for Tegra20.
>>
>> Fixes: 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver (Tegra30 supported now)")
>> Cc: stable@vger.kernel.org
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  arch/arm/boot/dts/tegra20-ventana.dts | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
>> index b158771ac0b7..055334ae3d28 100644
>> --- a/arch/arm/boot/dts/tegra20-ventana.dts
>> +++ b/arch/arm/boot/dts/tegra20-ventana.dts
>> @@ -3,6 +3,7 @@
>>  
>>  #include <dt-bindings/input/input.h>
>>  #include "tegra20.dtsi"
>> +#include "tegra20-cpu-opp.dtsi"
>>  
>>  / {
>>  	model = "NVIDIA Tegra20 Ventana evaluation board";
>> @@ -592,6 +593,16 @@ clk32k_in: clock@0 {
>>  		#clock-cells = <0>;
>>  	};
>>  
>> +	cpus {
>> +		cpu0: cpu@0 {
>> +			operating-points-v2 = <&cpu0_opp_table>;
>> +		};
>> +
>> +		cpu@1 {
>> +			operating-points-v2 = <&cpu0_opp_table>;
>> +		};
>> +	};
>> +
>>  	gpio-keys {
>>  		compatible = "gpio-keys";
>>  
>>
> 
> This could be wrong to do because CPU voltage is fixed to 1000mV in
> Ventana's DT, are you sure that higher clock rates don't require higher
> voltages? What is the CPU process ID and SoC speedo ID on Ventana?

I see this in the bootlog ...

[    2.797684] tegra20-cpufreq tegra20-cpufreq: hardware version 0x2 0x2

> You could easily hook up CPU voltage scaling, please see acer-500 DT and
> patch [1] for examples of how to set up regulators in DT. But then it
> shouldn't be a stable patch.

According to the Ventana design guide the CPU voltage range is 0.8-1.0V
and so it appears to be set to the max. The CPUFREQ test is reporting
the following ...

cpu: cpufreq: - CPU#0:
cpu: cpufreq:   - supported governors:
cpu: cpufreq:     - ondemand *
cpu: cpufreq:     - performance
cpu: cpufreq:     - schedutil
cpu: cpufreq:   - supported rates:
cpu: cpufreq:     -  216000
cpu: cpufreq:     -  312000
cpu: cpufreq:     -  456000
cpu: cpufreq:     -  608000
cpu: cpufreq:     -  760000
cpu: cpufreq:     -  816000
cpu: cpufreq:     -  912000
cpu: cpufreq:     - 1000000 *
cpu: cpufreq: - CPU#1:
cpu: cpufreq:   - supported governors:
cpu: cpufreq:     - ondemand *
cpu: cpufreq:     - performance
cpu: cpufreq:     - schedutil
cpu: cpufreq:   - supported rates:
cpu: cpufreq:     -  216000
cpu: cpufreq:     -  312000
cpu: cpufreq:     -  456000
cpu: cpufreq:     -  608000
cpu: cpufreq:     -  760000
cpu: cpufreq:     -  816000
cpu: cpufreq:     -  912000
cpu: cpufreq:     - 1000000 *

Cheers
Jon
Dmitry Osipenko Nov. 12, 2020, 10:51 a.m. UTC | #4
11.11.2020 23:31, Jon Hunter пишет:
> 
> On 11/11/2020 13:47, Dmitry Osipenko wrote:
>> 11.11.2020 13:38, Jon Hunter пишет:
>>> Commit 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver
>>> (Tegra30 supported now)") update the Tegra20 CPUFREQ driver to use the
>>> generic CPUFREQ device-tree driver. Since this change CPUFREQ support
>>> on the Tegra20 Ventana platform has been broken because the necessary
>>> device-tree nodes with the operating point information are not populated
>>> for this platform. Fix this by updating device-tree for Venata to
>>> include the operating point informration for Tegra20.
>>>
>>> Fixes: 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver (Tegra30 supported now)")
>>> Cc: stable@vger.kernel.org
>>>
>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>> ---
>>>  arch/arm/boot/dts/tegra20-ventana.dts | 11 +++++++++++
>>>  1 file changed, 11 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
>>> index b158771ac0b7..055334ae3d28 100644
>>> --- a/arch/arm/boot/dts/tegra20-ventana.dts
>>> +++ b/arch/arm/boot/dts/tegra20-ventana.dts
>>> @@ -3,6 +3,7 @@
>>>  
>>>  #include <dt-bindings/input/input.h>
>>>  #include "tegra20.dtsi"
>>> +#include "tegra20-cpu-opp.dtsi"
>>>  
>>>  / {
>>>  	model = "NVIDIA Tegra20 Ventana evaluation board";
>>> @@ -592,6 +593,16 @@ clk32k_in: clock@0 {
>>>  		#clock-cells = <0>;
>>>  	};
>>>  
>>> +	cpus {
>>> +		cpu0: cpu@0 {
>>> +			operating-points-v2 = <&cpu0_opp_table>;
>>> +		};
>>> +
>>> +		cpu@1 {
>>> +			operating-points-v2 = <&cpu0_opp_table>;
>>> +		};
>>> +	};
>>> +
>>>  	gpio-keys {
>>>  		compatible = "gpio-keys";
>>>  
>>>
>>
>> This could be wrong to do because CPU voltage is fixed to 1000mV in
>> Ventana's DT, are you sure that higher clock rates don't require higher
>> voltages? What is the CPU process ID and SoC speedo ID on Ventana?
> 
> I see this in the bootlog ...
> 
> [    2.797684] tegra20-cpufreq tegra20-cpufreq: hardware version 0x2 0x2
> 
>> You could easily hook up CPU voltage scaling, please see acer-500 DT and
>> patch [1] for examples of how to set up regulators in DT. But then it
>> shouldn't be a stable patch.
> 
> According to the Ventana design guide the CPU voltage range is 0.8-1.0V
> and so it appears to be set to the max. The CPUFREQ test is reporting
> the following ...
> 
> cpu: cpufreq: - CPU#0:
> cpu: cpufreq:   - supported governors:
> cpu: cpufreq:     - ondemand *
> cpu: cpufreq:     - performance
> cpu: cpufreq:     - schedutil
> cpu: cpufreq:   - supported rates:
> cpu: cpufreq:     -  216000
> cpu: cpufreq:     -  312000
> cpu: cpufreq:     -  456000
> cpu: cpufreq:     -  608000
> cpu: cpufreq:     -  760000
> cpu: cpufreq:     -  816000
> cpu: cpufreq:     -  912000
> cpu: cpufreq:     - 1000000 *
> cpu: cpufreq: - CPU#1:
> cpu: cpufreq:   - supported governors:
> cpu: cpufreq:     - ondemand *
> cpu: cpufreq:     - performance
> cpu: cpufreq:     - schedutil
> cpu: cpufreq:   - supported rates:
> cpu: cpufreq:     -  216000
> cpu: cpufreq:     -  312000
> cpu: cpufreq:     -  456000
> cpu: cpufreq:     -  608000
> cpu: cpufreq:     -  760000
> cpu: cpufreq:     -  816000
> cpu: cpufreq:     -  912000
> cpu: cpufreq:     - 1000000 *

If you don't see a message in KMSG saying "bringing vdd_cpu to
1000000uV", then should be good.
Jon Hunter Nov. 12, 2020, 11:34 a.m. UTC | #5
On 12/11/2020 10:51, Dmitry Osipenko wrote:

...

> If you don't see a message in KMSG saying "bringing vdd_cpu to
> 1000000uV", then should be good.

The bootlog shows ...

[    2.271768] tps6586x 3-0034: Found TPS658621C/D, VERSIONCRC is 2c

[    2.280320] vdd_sys: supplied by vdd_5v0

[    2.285153] vdd_sm0,vdd_core: supplied by vdd_sys

[    2.294231] vdd_sm1,vdd_cpu: supplied by vdd_sys

[    2.299984] vdd_sm2,vin_ldo*: supplied by vdd_sys

[    2.305285] REG-LDO_0: supplied by vdd_sm2,vin_ldo*

[    2.311492] vdd_ldo1,avdd_pll*: supplied by vdd_sm2,vin_ldo*

[    2.318053] vdd_ldo2,vdd_rtc: supplied by vdd_sm2,vin_ldo*

[    2.324786] vdd_ldo3,avdd_usb*: supplied by vdd_sm2,vin_ldo*

[    2.331848] vdd_ldo4,avdd_osc,vddio_sys: supplied by vdd_sm2,vin_ldo*

[    2.339104] vdd_ldo5,vcore_mmc: supplied by vdd_sys

[    2.344447] vdd_ldo6,avdd_vdac: Bringing 2850000uV into 1800000-1800000uV

[    2.352226] vdd_ldo6,avdd_vdac: supplied by vdd_sm2,vin_ldo*

[    2.358814] vdd_ldo7,avdd_hdmi,vdd_fuse: supplied by vdd_sm2,vin_ldo*

[    2.366176] vdd_ldo8,avdd_hdmi_pll: supplied by vdd_sm2,vin_ldo*

[    2.372959] vdd_ldo9,avdd_2v85,vdd_ddr_rx: supplied by vdd_sm2,vin_ldo*

[    2.380373] vdd_rtc_out,vdd_cell: supplied by vdd_sys


Jon
Dmitry Osipenko Nov. 12, 2020, 12:11 p.m. UTC | #6
11.11.2020 13:38, Jon Hunter пишет:
> Commit 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver
> (Tegra30 supported now)") update the Tegra20 CPUFREQ driver to use the
> generic CPUFREQ device-tree driver. Since this change CPUFREQ support
> on the Tegra20 Ventana platform has been broken because the necessary
> device-tree nodes with the operating point information are not populated
> for this platform. Fix this by updating device-tree for Venata to
> include the operating point informration for Tegra20.
> 
> Fixes: 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver (Tegra30 supported now)")
> Cc: stable@vger.kernel.org
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  arch/arm/boot/dts/tegra20-ventana.dts | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
> index b158771ac0b7..055334ae3d28 100644
> --- a/arch/arm/boot/dts/tegra20-ventana.dts
> +++ b/arch/arm/boot/dts/tegra20-ventana.dts
> @@ -3,6 +3,7 @@
>  
>  #include <dt-bindings/input/input.h>
>  #include "tegra20.dtsi"
> +#include "tegra20-cpu-opp.dtsi"
>  
>  / {
>  	model = "NVIDIA Tegra20 Ventana evaluation board";
> @@ -592,6 +593,16 @@ clk32k_in: clock@0 {
>  		#clock-cells = <0>;
>  	};
>  
> +	cpus {
> +		cpu0: cpu@0 {

I assume you're going to use this cpu0 handle later on.

> +			operating-points-v2 = <&cpu0_opp_table>;
> +		};
> +
> +		cpu@1 {
> +			operating-points-v2 = <&cpu0_opp_table>;
> +		};
> +	};
> +
>  	gpio-keys {
>  		compatible = "gpio-keys";
>  
> 

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
Jon Hunter Nov. 12, 2020, 12:52 p.m. UTC | #7
On 12/11/2020 12:11, Dmitry Osipenko wrote:
> 11.11.2020 13:38, Jon Hunter пишет:
>> Commit 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver
>> (Tegra30 supported now)") update the Tegra20 CPUFREQ driver to use the
>> generic CPUFREQ device-tree driver. Since this change CPUFREQ support
>> on the Tegra20 Ventana platform has been broken because the necessary
>> device-tree nodes with the operating point information are not populated
>> for this platform. Fix this by updating device-tree for Venata to
>> include the operating point informration for Tegra20.
>>
>> Fixes: 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver (Tegra30 supported now)")
>> Cc: stable@vger.kernel.org
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  arch/arm/boot/dts/tegra20-ventana.dts | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
>> index b158771ac0b7..055334ae3d28 100644
>> --- a/arch/arm/boot/dts/tegra20-ventana.dts
>> +++ b/arch/arm/boot/dts/tegra20-ventana.dts
>> @@ -3,6 +3,7 @@
>>  
>>  #include <dt-bindings/input/input.h>
>>  #include "tegra20.dtsi"
>> +#include "tegra20-cpu-opp.dtsi"
>>  
>>  / {
>>  	model = "NVIDIA Tegra20 Ventana evaluation board";
>> @@ -592,6 +593,16 @@ clk32k_in: clock@0 {
>>  		#clock-cells = <0>;
>>  	};
>>  
>> +	cpus {
>> +		cpu0: cpu@0 {
> 
> I assume you're going to use this cpu0 handle later on.

Opps, I will remove that.

> 
>> +			operating-points-v2 = <&cpu0_opp_table>;
>> +		};
>> +
>> +		cpu@1 {
>> +			operating-points-v2 = <&cpu0_opp_table>;
>> +		};
>> +	};
>> +
>>  	gpio-keys {
>>  		compatible = "gpio-keys";
>>  
>>
> 
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
> 

Thanks!
Jon
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
index b158771ac0b7..055334ae3d28 100644
--- a/arch/arm/boot/dts/tegra20-ventana.dts
+++ b/arch/arm/boot/dts/tegra20-ventana.dts
@@ -3,6 +3,7 @@ 
 
 #include <dt-bindings/input/input.h>
 #include "tegra20.dtsi"
+#include "tegra20-cpu-opp.dtsi"
 
 / {
 	model = "NVIDIA Tegra20 Ventana evaluation board";
@@ -592,6 +593,16 @@  clk32k_in: clock@0 {
 		#clock-cells = <0>;
 	};
 
+	cpus {
+		cpu0: cpu@0 {
+			operating-points-v2 = <&cpu0_opp_table>;
+		};
+
+		cpu@1 {
+			operating-points-v2 = <&cpu0_opp_table>;
+		};
+	};
+
 	gpio-keys {
 		compatible = "gpio-keys";