diff mbox

[1/4] ARM: dts: augment Ux500 to use DT cpufreq

Message ID 20170810125221.25818-2-linus.walleij@linaro.org (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Linus Walleij Aug. 10, 2017, 12:52 p.m. UTC
This adds the operating points to the Ux500 device tree and
deletes the old special-purpose cpufreq node, as we can now
use the generic DT cpufreq driver.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Viresh et al: I will merge this into ARM SoC separately from
the rest of the patches once we agree on this approach.
---
 arch/arm/boot/dts/ste-dbx5x0.dtsi | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

Comments

Viresh Kumar Aug. 11, 2017, 5:15 a.m. UTC | #1
On 10-08-17, 14:52, Linus Walleij wrote:
> This adds the operating points to the Ux500 device tree and
> deletes the old special-purpose cpufreq node, as we can now
> use the generic DT cpufreq driver.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Viresh et al: I will merge this into ARM SoC separately from
> the rest of the patches once we agree on this approach.

Sure, no worries.

> ---
>  arch/arm/boot/dts/ste-dbx5x0.dtsi | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/ste-dbx5x0.dtsi b/arch/arm/boot/dts/ste-dbx5x0.dtsi
> index 6c5affe2d0f5..2310a4e97768 100644
> --- a/arch/arm/boot/dts/ste-dbx5x0.dtsi
> +++ b/arch/arm/boot/dts/ste-dbx5x0.dtsi
> @@ -37,6 +37,14 @@
>  			device_type = "cpu";
>  			compatible = "arm,cortex-a9";
>  			reg = <0x300>;
> +			/* cpufreq controls */
> +			operating-points = <998400 0
> +					    800000 0
> +					    400000 0
> +					    200000 0>;
> +			clocks = <&prcmu_clk PRCMU_ARMSS>;
> +			clock-names = "cpu";
> +			clock-latency = <20000>;
>  		};
>  		CPU1: cpu@301 {
>  			device_type = "cpu";
> @@ -494,13 +502,6 @@
>  				reg = <0x80157450 0xC>;
>  			};
>  
> -			cpufreq {
> -				compatible = "stericsson,cpufreq-ux500";
> -				clocks = <&prcmu_clk PRCMU_ARMSS>;
> -				clock-names = "armss";
> -				status = "disabled";
> -			};
> -

This is relieving :)

But this solution isn't the best really, though you may not face any
issues with your use case.

For example, below will fail cpufreq on your board. You can try that
to make sure I am not dreaming.

- Compile cpufreq-dt as a module.
- Boot the board.
- Offline CPU0 (i.e. Keep only CPU1 online).
- Boom, cpufreq failed.

This will happen because the CPU1 doesn't have the operating-points
and other properties. We should have them in all the CPUs.

In order to not repeat the OPP table for all CPUs, you can use the new
operating-points-v2 bindings and mark the OPP table shared.
Linus Walleij Aug. 13, 2017, 12:23 p.m. UTC | #2
On Fri, Aug 11, 2017 at 7:15 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:

> For example, below will fail cpufreq on your board. You can try that
> to make sure I am not dreaming.
>
> - Compile cpufreq-dt as a module.
> - Boot the board.
> - Offline CPU0 (i.e. Keep only CPU1 online).
> - Boom, cpufreq failed.
>
> This will happen because the CPU1 doesn't have the operating-points
> and other properties. We should have them in all the CPUs.

I don't think that will be a problem because CPU0 cannot be
taken offline on this system.

arch/arm/mach-ux500/platsmp.c surely only contain code to take
CPU1 online/offline.

At sleep, the BIOS/ROM handles taking down/up CPU0 so from
Linux' point of view that CPU is always online.

But I will look a bit closer and check.

This approach comes from the OMAP DT OPPs which also look like
this (and this system is similar).

Yours,
Linus Walleij
Viresh Kumar Aug. 16, 2017, 3:26 a.m. UTC | #3
On 13-08-17, 14:23, Linus Walleij wrote:
> On Fri, Aug 11, 2017 at 7:15 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> 
> > For example, below will fail cpufreq on your board. You can try that
> > to make sure I am not dreaming.
> >
> > - Compile cpufreq-dt as a module.
> > - Boot the board.
> > - Offline CPU0 (i.e. Keep only CPU1 online).
> > - Boom, cpufreq failed.
> >
> > This will happen because the CPU1 doesn't have the operating-points
> > and other properties. We should have them in all the CPUs.
> 
> I don't think that will be a problem because CPU0 cannot be
> taken offline on this system.

Should be fine then.
diff mbox

Patch

diff --git a/arch/arm/boot/dts/ste-dbx5x0.dtsi b/arch/arm/boot/dts/ste-dbx5x0.dtsi
index 6c5affe2d0f5..2310a4e97768 100644
--- a/arch/arm/boot/dts/ste-dbx5x0.dtsi
+++ b/arch/arm/boot/dts/ste-dbx5x0.dtsi
@@ -37,6 +37,14 @@ 
 			device_type = "cpu";
 			compatible = "arm,cortex-a9";
 			reg = <0x300>;
+			/* cpufreq controls */
+			operating-points = <998400 0
+					    800000 0
+					    400000 0
+					    200000 0>;
+			clocks = <&prcmu_clk PRCMU_ARMSS>;
+			clock-names = "cpu";
+			clock-latency = <20000>;
 		};
 		CPU1: cpu@301 {
 			device_type = "cpu";
@@ -494,13 +502,6 @@ 
 				reg = <0x80157450 0xC>;
 			};
 
-			cpufreq {
-				compatible = "stericsson,cpufreq-ux500";
-				clocks = <&prcmu_clk PRCMU_ARMSS>;
-				clock-names = "armss";
-				status = "disabled";
-			};
-
 			thermal@801573c0 {
 				compatible = "stericsson,db8500-thermal";
 				reg = <0x801573c0 0x40>;