All of lore.kernel.org
 help / color / mirror / Atom feed
* [4.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers
@ 2015-11-27 19:44 ` Grygorii Strashko
  0 siblings, 0 replies; 12+ messages in thread
From: Grygorii Strashko @ 2015-11-27 19:44 UTC (permalink / raw)
  To: tony-4v6yS6AI5VpBDgjK7y7TUQ, t-kristo-l0cyMroinI0
  Cc: nsekhar-l0cyMroinI0, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Grygorii Strashko,
	Felipe Balbi

ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2.
But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result.
Timekeeping core misbehaves. For example, execution of command
"sleep 5" will take 10 sec instead of 5.

Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use
it for clocking ARM TWD and Global timer (same way as on OMAP4).

Cc: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Cc: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
Cc: Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org>
Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and SCU nodes")
Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/am4372.dtsi        | 4 ++--
 arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index d83ff9c..de8791a 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -74,7 +74,7 @@
 		reg = <0x48240200 0x100>;
 		interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
 		interrupt-parent = <&gic>;
-		clocks = <&dpll_mpu_m2_ck>;
+		clocks = <&mpu_periphclk>;
 	};
 
 	local_timer: timer@48240600 {
@@ -82,7 +82,7 @@
 		reg = <0x48240600 0x100>;
 		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
 		interrupt-parent = <&gic>;
-		clocks = <&dpll_mpu_m2_ck>;
+		clocks = <&mpu_periphclk>;
 	};
 
 	l2-cache-controller@48242000 {
diff --git a/arch/arm/boot/dts/am43xx-clocks.dtsi b/arch/arm/boot/dts/am43xx-clocks.dtsi
index cc88728..2ff58b1 100644
--- a/arch/arm/boot/dts/am43xx-clocks.dtsi
+++ b/arch/arm/boot/dts/am43xx-clocks.dtsi
@@ -259,6 +259,14 @@
 		ti,invert-autoidle-bit;
 	};
 
+	mpu_periphclk: mpu_periphclk {
+		#clock-cells = <0>;
+		compatible = "fixed-factor-clock";
+		clocks = <&dpll_mpu_ck>;
+		clock-mult = <1>;
+		clock-div = <2>;
+	};
+
 	dpll_ddr_ck: dpll_ddr_ck {
 		#clock-cells = <0>;
 		compatible = "ti,am3-dpll-clock";
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [4.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers
@ 2015-11-27 19:44 ` Grygorii Strashko
  0 siblings, 0 replies; 12+ messages in thread
From: Grygorii Strashko @ 2015-11-27 19:44 UTC (permalink / raw)
  To: linux-arm-kernel

ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2.
But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result.
Timekeeping core misbehaves. For example, execution of command
"sleep 5" will take 10 sec instead of 5.

Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use
it for clocking ARM TWD and Global timer (same way as on OMAP4).

Cc: Tony Lindgren <tony@atomide.com>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Tero Kristo <t-kristo@ti.com>
Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and SCU nodes")
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/boot/dts/am4372.dtsi        | 4 ++--
 arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index d83ff9c..de8791a 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -74,7 +74,7 @@
 		reg = <0x48240200 0x100>;
 		interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
 		interrupt-parent = <&gic>;
-		clocks = <&dpll_mpu_m2_ck>;
+		clocks = <&mpu_periphclk>;
 	};
 
 	local_timer: timer at 48240600 {
@@ -82,7 +82,7 @@
 		reg = <0x48240600 0x100>;
 		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
 		interrupt-parent = <&gic>;
-		clocks = <&dpll_mpu_m2_ck>;
+		clocks = <&mpu_periphclk>;
 	};
 
 	l2-cache-controller at 48242000 {
diff --git a/arch/arm/boot/dts/am43xx-clocks.dtsi b/arch/arm/boot/dts/am43xx-clocks.dtsi
index cc88728..2ff58b1 100644
--- a/arch/arm/boot/dts/am43xx-clocks.dtsi
+++ b/arch/arm/boot/dts/am43xx-clocks.dtsi
@@ -259,6 +259,14 @@
 		ti,invert-autoidle-bit;
 	};
 
+	mpu_periphclk: mpu_periphclk {
+		#clock-cells = <0>;
+		compatible = "fixed-factor-clock";
+		clocks = <&dpll_mpu_ck>;
+		clock-mult = <1>;
+		clock-div = <2>;
+	};
+
 	dpll_ddr_ck: dpll_ddr_ck {
 		#clock-cells = <0>;
 		compatible = "ti,am3-dpll-clock";
-- 
2.6.3

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [4.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers
  2015-11-27 19:44 ` Grygorii Strashko
@ 2015-11-30  8:25     ` Tero Kristo
  -1 siblings, 0 replies; 12+ messages in thread
From: Tero Kristo @ 2015-11-30  8:25 UTC (permalink / raw)
  To: Grygorii Strashko, tony-4v6yS6AI5VpBDgjK7y7TUQ
  Cc: nsekhar-l0cyMroinI0, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Felipe Balbi

On 11/27/2015 09:44 PM, Grygorii Strashko wrote:
> ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2.
> But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result.
> Timekeeping core misbehaves. For example, execution of command
> "sleep 5" will take 10 sec instead of 5.
>
> Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use
> it for clocking ARM TWD and Global timer (same way as on OMAP4).
>
> Cc: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> Cc: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
> Cc: Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org>
> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and SCU nodes")
> Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
> ---
>   arch/arm/boot/dts/am4372.dtsi        | 4 ++--
>   arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++
>   2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
> index d83ff9c..de8791a 100644
> --- a/arch/arm/boot/dts/am4372.dtsi
> +++ b/arch/arm/boot/dts/am4372.dtsi
> @@ -74,7 +74,7 @@
>   		reg = <0x48240200 0x100>;
>   		interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
>   		interrupt-parent = <&gic>;
> -		clocks = <&dpll_mpu_m2_ck>;
> +		clocks = <&mpu_periphclk>;
>   	};
>
>   	local_timer: timer@48240600 {
> @@ -82,7 +82,7 @@
>   		reg = <0x48240600 0x100>;
>   		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
>   		interrupt-parent = <&gic>;
> -		clocks = <&dpll_mpu_m2_ck>;
> +		clocks = <&mpu_periphclk>;
>   	};
>
>   	l2-cache-controller@48242000 {
> diff --git a/arch/arm/boot/dts/am43xx-clocks.dtsi b/arch/arm/boot/dts/am43xx-clocks.dtsi
> index cc88728..2ff58b1 100644
> --- a/arch/arm/boot/dts/am43xx-clocks.dtsi
> +++ b/arch/arm/boot/dts/am43xx-clocks.dtsi
> @@ -259,6 +259,14 @@
>   		ti,invert-autoidle-bit;
>   	};
>
> +	mpu_periphclk: mpu_periphclk {
> +		#clock-cells = <0>;
> +		compatible = "fixed-factor-clock";
> +		clocks = <&dpll_mpu_ck>;

I don't think this is correct, ARM core is fed dpll_mpu_m2_ck, where the 
divisor value can potentially differ from 1. If you feed this clock 
directly from dpll_mpu_ck, you bypass this divisor.

Did you check what is the impact of cpufreq on the ARM TWD/timers?

-Tero

> +		clock-mult = <1>;
> +		clock-div = <2>;
> +	};
> +
>   	dpll_ddr_ck: dpll_ddr_ck {
>   		#clock-cells = <0>;
>   		compatible = "ti,am3-dpll-clock";
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [4.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers
@ 2015-11-30  8:25     ` Tero Kristo
  0 siblings, 0 replies; 12+ messages in thread
From: Tero Kristo @ 2015-11-30  8:25 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/27/2015 09:44 PM, Grygorii Strashko wrote:
> ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2.
> But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result.
> Timekeeping core misbehaves. For example, execution of command
> "sleep 5" will take 10 sec instead of 5.
>
> Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use
> it for clocking ARM TWD and Global timer (same way as on OMAP4).
>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Felipe Balbi <balbi@ti.com>
> Cc: Tero Kristo <t-kristo@ti.com>
> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and SCU nodes")
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>   arch/arm/boot/dts/am4372.dtsi        | 4 ++--
>   arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++
>   2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
> index d83ff9c..de8791a 100644
> --- a/arch/arm/boot/dts/am4372.dtsi
> +++ b/arch/arm/boot/dts/am4372.dtsi
> @@ -74,7 +74,7 @@
>   		reg = <0x48240200 0x100>;
>   		interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
>   		interrupt-parent = <&gic>;
> -		clocks = <&dpll_mpu_m2_ck>;
> +		clocks = <&mpu_periphclk>;
>   	};
>
>   	local_timer: timer at 48240600 {
> @@ -82,7 +82,7 @@
>   		reg = <0x48240600 0x100>;
>   		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
>   		interrupt-parent = <&gic>;
> -		clocks = <&dpll_mpu_m2_ck>;
> +		clocks = <&mpu_periphclk>;
>   	};
>
>   	l2-cache-controller at 48242000 {
> diff --git a/arch/arm/boot/dts/am43xx-clocks.dtsi b/arch/arm/boot/dts/am43xx-clocks.dtsi
> index cc88728..2ff58b1 100644
> --- a/arch/arm/boot/dts/am43xx-clocks.dtsi
> +++ b/arch/arm/boot/dts/am43xx-clocks.dtsi
> @@ -259,6 +259,14 @@
>   		ti,invert-autoidle-bit;
>   	};
>
> +	mpu_periphclk: mpu_periphclk {
> +		#clock-cells = <0>;
> +		compatible = "fixed-factor-clock";
> +		clocks = <&dpll_mpu_ck>;

I don't think this is correct, ARM core is fed dpll_mpu_m2_ck, where the 
divisor value can potentially differ from 1. If you feed this clock 
directly from dpll_mpu_ck, you bypass this divisor.

Did you check what is the impact of cpufreq on the ARM TWD/timers?

-Tero

> +		clock-mult = <1>;
> +		clock-div = <2>;
> +	};
> +
>   	dpll_ddr_ck: dpll_ddr_ck {
>   		#clock-cells = <0>;
>   		compatible = "ti,am3-dpll-clock";
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [4.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers
  2015-11-30  8:25     ` Tero Kristo
@ 2015-11-30 11:53         ` Grygorii Strashko
  -1 siblings, 0 replies; 12+ messages in thread
From: Grygorii Strashko @ 2015-11-30 11:53 UTC (permalink / raw)
  To: Tero Kristo, tony-4v6yS6AI5VpBDgjK7y7TUQ
  Cc: nsekhar-l0cyMroinI0, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Felipe Balbi

On 11/30/2015 10:25 AM, Tero Kristo wrote:
> On 11/27/2015 09:44 PM, Grygorii Strashko wrote:
>> ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2.
>> But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result.
>> Timekeeping core misbehaves. For example, execution of command
>> "sleep 5" will take 10 sec instead of 5.
>>
>> Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use
>> it for clocking ARM TWD and Global timer (same way as on OMAP4).
>>
>> Cc: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
>> Cc: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
>> Cc: Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org>
>> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and
>> SCU nodes")
>> Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
>> ---
>>   arch/arm/boot/dts/am4372.dtsi        | 4 ++--
>>   arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++
>>   2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/am4372.dtsi
>> b/arch/arm/boot/dts/am4372.dtsi
>> index d83ff9c..de8791a 100644
>> --- a/arch/arm/boot/dts/am4372.dtsi
>> +++ b/arch/arm/boot/dts/am4372.dtsi
>> @@ -74,7 +74,7 @@
>>           reg = <0x48240200 0x100>;
>>           interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
>>           interrupt-parent = <&gic>;
>> -        clocks = <&dpll_mpu_m2_ck>;
>> +        clocks = <&mpu_periphclk>;
>>       };
>>
>>       local_timer: timer@48240600 {
>> @@ -82,7 +82,7 @@
>>           reg = <0x48240600 0x100>;
>>           interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
>>           interrupt-parent = <&gic>;
>> -        clocks = <&dpll_mpu_m2_ck>;
>> +        clocks = <&mpu_periphclk>;
>>       };
>>
>>       l2-cache-controller@48242000 {
>> diff --git a/arch/arm/boot/dts/am43xx-clocks.dtsi
>> b/arch/arm/boot/dts/am43xx-clocks.dtsi
>> index cc88728..2ff58b1 100644
>> --- a/arch/arm/boot/dts/am43xx-clocks.dtsi
>> +++ b/arch/arm/boot/dts/am43xx-clocks.dtsi
>> @@ -259,6 +259,14 @@
>>           ti,invert-autoidle-bit;
>>       };
>>
>> +    mpu_periphclk: mpu_periphclk {
>> +        #clock-cells = <0>;
>> +        compatible = "fixed-factor-clock";
>> +        clocks = <&dpll_mpu_ck>;
>
> I don't think this is correct, ARM core is fed dpll_mpu_m2_ck, where the
> divisor value can potentially differ from 1. If you feed this clock
> directly from dpll_mpu_ck, you bypass this divisor.

Sry. My mistake. I'll update it to use dpll_mpu_m2_ck.

>
> Did you check what is the impact of cpufreq on the ARM TWD/timers?

TWD is cpufreq friendly, ARM GT is not.


>> +        clock-mult = <1>;
>> +        clock-div = <2>;
>> +    };
>> +
>>       dpll_ddr_ck: dpll_ddr_ck {
>>           #clock-cells = <0>;
>>           compatible = "ti,am3-dpll-clock";
>>
>


-- 
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [4.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers
@ 2015-11-30 11:53         ` Grygorii Strashko
  0 siblings, 0 replies; 12+ messages in thread
From: Grygorii Strashko @ 2015-11-30 11:53 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/30/2015 10:25 AM, Tero Kristo wrote:
> On 11/27/2015 09:44 PM, Grygorii Strashko wrote:
>> ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2.
>> But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result.
>> Timekeeping core misbehaves. For example, execution of command
>> "sleep 5" will take 10 sec instead of 5.
>>
>> Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use
>> it for clocking ARM TWD and Global timer (same way as on OMAP4).
>>
>> Cc: Tony Lindgren <tony@atomide.com>
>> Cc: Felipe Balbi <balbi@ti.com>
>> Cc: Tero Kristo <t-kristo@ti.com>
>> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and
>> SCU nodes")
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   arch/arm/boot/dts/am4372.dtsi        | 4 ++--
>>   arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++
>>   2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/am4372.dtsi
>> b/arch/arm/boot/dts/am4372.dtsi
>> index d83ff9c..de8791a 100644
>> --- a/arch/arm/boot/dts/am4372.dtsi
>> +++ b/arch/arm/boot/dts/am4372.dtsi
>> @@ -74,7 +74,7 @@
>>           reg = <0x48240200 0x100>;
>>           interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
>>           interrupt-parent = <&gic>;
>> -        clocks = <&dpll_mpu_m2_ck>;
>> +        clocks = <&mpu_periphclk>;
>>       };
>>
>>       local_timer: timer at 48240600 {
>> @@ -82,7 +82,7 @@
>>           reg = <0x48240600 0x100>;
>>           interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
>>           interrupt-parent = <&gic>;
>> -        clocks = <&dpll_mpu_m2_ck>;
>> +        clocks = <&mpu_periphclk>;
>>       };
>>
>>       l2-cache-controller at 48242000 {
>> diff --git a/arch/arm/boot/dts/am43xx-clocks.dtsi
>> b/arch/arm/boot/dts/am43xx-clocks.dtsi
>> index cc88728..2ff58b1 100644
>> --- a/arch/arm/boot/dts/am43xx-clocks.dtsi
>> +++ b/arch/arm/boot/dts/am43xx-clocks.dtsi
>> @@ -259,6 +259,14 @@
>>           ti,invert-autoidle-bit;
>>       };
>>
>> +    mpu_periphclk: mpu_periphclk {
>> +        #clock-cells = <0>;
>> +        compatible = "fixed-factor-clock";
>> +        clocks = <&dpll_mpu_ck>;
>
> I don't think this is correct, ARM core is fed dpll_mpu_m2_ck, where the
> divisor value can potentially differ from 1. If you feed this clock
> directly from dpll_mpu_ck, you bypass this divisor.

Sry. My mistake. I'll update it to use dpll_mpu_m2_ck.

>
> Did you check what is the impact of cpufreq on the ARM TWD/timers?

TWD is cpufreq friendly, ARM GT is not.


>> +        clock-mult = <1>;
>> +        clock-div = <2>;
>> +    };
>> +
>>       dpll_ddr_ck: dpll_ddr_ck {
>>           #clock-cells = <0>;
>>           compatible = "ti,am3-dpll-clock";
>>
>


-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [4.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers
  2015-11-30 11:53         ` Grygorii Strashko
@ 2015-11-30 13:32             ` Tero Kristo
  -1 siblings, 0 replies; 12+ messages in thread
From: Tero Kristo @ 2015-11-30 13:32 UTC (permalink / raw)
  To: Grygorii Strashko, tony-4v6yS6AI5VpBDgjK7y7TUQ
  Cc: nsekhar-l0cyMroinI0, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Felipe Balbi

On 11/30/2015 01:53 PM, Grygorii Strashko wrote:
> On 11/30/2015 10:25 AM, Tero Kristo wrote:
>> On 11/27/2015 09:44 PM, Grygorii Strashko wrote:
>>> ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2.
>>> But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result.
>>> Timekeeping core misbehaves. For example, execution of command
>>> "sleep 5" will take 10 sec instead of 5.
>>>
>>> Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use
>>> it for clocking ARM TWD and Global timer (same way as on OMAP4).
>>>
>>> Cc: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
>>> Cc: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
>>> Cc: Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org>
>>> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and
>>> SCU nodes")
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
>>> ---
>>>   arch/arm/boot/dts/am4372.dtsi        | 4 ++--
>>>   arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++
>>>   2 files changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/am4372.dtsi
>>> b/arch/arm/boot/dts/am4372.dtsi
>>> index d83ff9c..de8791a 100644
>>> --- a/arch/arm/boot/dts/am4372.dtsi
>>> +++ b/arch/arm/boot/dts/am4372.dtsi
>>> @@ -74,7 +74,7 @@
>>>           reg = <0x48240200 0x100>;
>>>           interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
>>>           interrupt-parent = <&gic>;
>>> -        clocks = <&dpll_mpu_m2_ck>;
>>> +        clocks = <&mpu_periphclk>;
>>>       };
>>>
>>>       local_timer: timer@48240600 {
>>> @@ -82,7 +82,7 @@
>>>           reg = <0x48240600 0x100>;
>>>           interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
>>>           interrupt-parent = <&gic>;
>>> -        clocks = <&dpll_mpu_m2_ck>;
>>> +        clocks = <&mpu_periphclk>;
>>>       };
>>>
>>>       l2-cache-controller@48242000 {
>>> diff --git a/arch/arm/boot/dts/am43xx-clocks.dtsi
>>> b/arch/arm/boot/dts/am43xx-clocks.dtsi
>>> index cc88728..2ff58b1 100644
>>> --- a/arch/arm/boot/dts/am43xx-clocks.dtsi
>>> +++ b/arch/arm/boot/dts/am43xx-clocks.dtsi
>>> @@ -259,6 +259,14 @@
>>>           ti,invert-autoidle-bit;
>>>       };
>>>
>>> +    mpu_periphclk: mpu_periphclk {
>>> +        #clock-cells = <0>;
>>> +        compatible = "fixed-factor-clock";
>>> +        clocks = <&dpll_mpu_ck>;
>>
>> I don't think this is correct, ARM core is fed dpll_mpu_m2_ck, where the
>> divisor value can potentially differ from 1. If you feed this clock
>> directly from dpll_mpu_ck, you bypass this divisor.
>
> Sry. My mistake. I'll update it to use dpll_mpu_m2_ck.
>
>>
>> Did you check what is the impact of cpufreq on the ARM TWD/timers?
>
> TWD is cpufreq friendly, ARM GT is not.

I think the TWD kick period changes with cpufreq also right?

How are the clocks handled with cpufreq? The user just needs to 
understand that the timers will be screwed if he uses ARM GT? Should we 
add some sort of dependency to disable the ARM GT if cpufreq is enabled?

-Tero

>
>
>>> +        clock-mult = <1>;
>>> +        clock-div = <2>;
>>> +    };
>>> +
>>>       dpll_ddr_ck: dpll_ddr_ck {
>>>           #clock-cells = <0>;
>>>           compatible = "ti,am3-dpll-clock";
>>>
>>
>
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [4.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers
@ 2015-11-30 13:32             ` Tero Kristo
  0 siblings, 0 replies; 12+ messages in thread
From: Tero Kristo @ 2015-11-30 13:32 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/30/2015 01:53 PM, Grygorii Strashko wrote:
> On 11/30/2015 10:25 AM, Tero Kristo wrote:
>> On 11/27/2015 09:44 PM, Grygorii Strashko wrote:
>>> ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2.
>>> But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result.
>>> Timekeeping core misbehaves. For example, execution of command
>>> "sleep 5" will take 10 sec instead of 5.
>>>
>>> Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use
>>> it for clocking ARM TWD and Global timer (same way as on OMAP4).
>>>
>>> Cc: Tony Lindgren <tony@atomide.com>
>>> Cc: Felipe Balbi <balbi@ti.com>
>>> Cc: Tero Kristo <t-kristo@ti.com>
>>> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and
>>> SCU nodes")
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>> ---
>>>   arch/arm/boot/dts/am4372.dtsi        | 4 ++--
>>>   arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++
>>>   2 files changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/am4372.dtsi
>>> b/arch/arm/boot/dts/am4372.dtsi
>>> index d83ff9c..de8791a 100644
>>> --- a/arch/arm/boot/dts/am4372.dtsi
>>> +++ b/arch/arm/boot/dts/am4372.dtsi
>>> @@ -74,7 +74,7 @@
>>>           reg = <0x48240200 0x100>;
>>>           interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
>>>           interrupt-parent = <&gic>;
>>> -        clocks = <&dpll_mpu_m2_ck>;
>>> +        clocks = <&mpu_periphclk>;
>>>       };
>>>
>>>       local_timer: timer at 48240600 {
>>> @@ -82,7 +82,7 @@
>>>           reg = <0x48240600 0x100>;
>>>           interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
>>>           interrupt-parent = <&gic>;
>>> -        clocks = <&dpll_mpu_m2_ck>;
>>> +        clocks = <&mpu_periphclk>;
>>>       };
>>>
>>>       l2-cache-controller at 48242000 {
>>> diff --git a/arch/arm/boot/dts/am43xx-clocks.dtsi
>>> b/arch/arm/boot/dts/am43xx-clocks.dtsi
>>> index cc88728..2ff58b1 100644
>>> --- a/arch/arm/boot/dts/am43xx-clocks.dtsi
>>> +++ b/arch/arm/boot/dts/am43xx-clocks.dtsi
>>> @@ -259,6 +259,14 @@
>>>           ti,invert-autoidle-bit;
>>>       };
>>>
>>> +    mpu_periphclk: mpu_periphclk {
>>> +        #clock-cells = <0>;
>>> +        compatible = "fixed-factor-clock";
>>> +        clocks = <&dpll_mpu_ck>;
>>
>> I don't think this is correct, ARM core is fed dpll_mpu_m2_ck, where the
>> divisor value can potentially differ from 1. If you feed this clock
>> directly from dpll_mpu_ck, you bypass this divisor.
>
> Sry. My mistake. I'll update it to use dpll_mpu_m2_ck.
>
>>
>> Did you check what is the impact of cpufreq on the ARM TWD/timers?
>
> TWD is cpufreq friendly, ARM GT is not.

I think the TWD kick period changes with cpufreq also right?

How are the clocks handled with cpufreq? The user just needs to 
understand that the timers will be screwed if he uses ARM GT? Should we 
add some sort of dependency to disable the ARM GT if cpufreq is enabled?

-Tero

>
>
>>> +        clock-mult = <1>;
>>> +        clock-div = <2>;
>>> +    };
>>> +
>>>       dpll_ddr_ck: dpll_ddr_ck {
>>>           #clock-cells = <0>;
>>>           compatible = "ti,am3-dpll-clock";
>>>
>>
>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [4.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers
  2015-11-30 13:32             ` Tero Kristo
@ 2015-11-30 13:49                 ` Grygorii Strashko
  -1 siblings, 0 replies; 12+ messages in thread
From: Grygorii Strashko @ 2015-11-30 13:49 UTC (permalink / raw)
  To: Tero Kristo, tony-4v6yS6AI5VpBDgjK7y7TUQ
  Cc: nsekhar-l0cyMroinI0, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Felipe Balbi

On 11/30/2015 03:32 PM, Tero Kristo wrote:
> On 11/30/2015 01:53 PM, Grygorii Strashko wrote:
>> On 11/30/2015 10:25 AM, Tero Kristo wrote:
>>> On 11/27/2015 09:44 PM, Grygorii Strashko wrote:
>>>> ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2.
>>>> But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result.
>>>> Timekeeping core misbehaves. For example, execution of command
>>>> "sleep 5" will take 10 sec instead of 5.
>>>>
>>>> Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use
>>>> it for clocking ARM TWD and Global timer (same way as on OMAP4).
>>>>
>>>> Cc: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
>>>> Cc: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
>>>> Cc: Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org>
>>>> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and
>>>> SCU nodes")
>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
>>>> ---
>>>>   arch/arm/boot/dts/am4372.dtsi        | 4 ++--
>>>>   arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++
>>>>   2 files changed, 10 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/am4372.dtsi
>>>> b/arch/arm/boot/dts/am4372.dtsi
>>>> index d83ff9c..de8791a 100644
>>>> --- a/arch/arm/boot/dts/am4372.dtsi
>>>> +++ b/arch/arm/boot/dts/am4372.dtsi
>>>> @@ -74,7 +74,7 @@
>>>>           reg = <0x48240200 0x100>;
>>>>           interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
>>>>           interrupt-parent = <&gic>;
>>>> -        clocks = <&dpll_mpu_m2_ck>;
>>>> +        clocks = <&mpu_periphclk>;
>>>>       };
>>>>
>>>>       local_timer: timer@48240600 {
>>>> @@ -82,7 +82,7 @@
>>>>           reg = <0x48240600 0x100>;
>>>>           interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
>>>>           interrupt-parent = <&gic>;
>>>> -        clocks = <&dpll_mpu_m2_ck>;
>>>> +        clocks = <&mpu_periphclk>;
>>>>       };
>>>>
>>>>       l2-cache-controller@48242000 {
>>>> diff --git a/arch/arm/boot/dts/am43xx-clocks.dtsi
>>>> b/arch/arm/boot/dts/am43xx-clocks.dtsi
>>>> index cc88728..2ff58b1 100644
>>>> --- a/arch/arm/boot/dts/am43xx-clocks.dtsi
>>>> +++ b/arch/arm/boot/dts/am43xx-clocks.dtsi
>>>> @@ -259,6 +259,14 @@
>>>>           ti,invert-autoidle-bit;
>>>>       };
>>>>
>>>> +    mpu_periphclk: mpu_periphclk {
>>>> +        #clock-cells = <0>;
>>>> +        compatible = "fixed-factor-clock";
>>>> +        clocks = <&dpll_mpu_ck>;
>>>
>>> I don't think this is correct, ARM core is fed dpll_mpu_m2_ck, where the
>>> divisor value can potentially differ from 1. If you feed this clock
>>> directly from dpll_mpu_ck, you bypass this divisor.
>>
>> Sry. My mistake. I'll update it to use dpll_mpu_m2_ck.
>>
>>>
>>> Did you check what is the impact of cpufreq on the ARM TWD/timers?
>>
>> TWD is cpufreq friendly, ARM GT is not.
> 
> I think the TWD kick period changes with cpufreq also right?

linux/arch/arm/kernel/smp_twd.c has code to handle cpufreq.

> 
> How are the clocks handled with cpufreq? The user just needs to 
> understand that the timers will be screwed if he uses ARM GT? Should we 
> add some sort of dependency to disable the ARM GT if cpufreq is enabled?

Yep. May be, but very good question is how to do that in case of
OMAP multiplatform build which enables most of all config options at once.

There two threads related to this:
[1] http://www.spinics.net/lists/arm-kernel/msg459649.html
[2] http://www.spinics.net/lists/arm-kernel/msg461141.html

Personally, I've do not see better way than [2] right now.


>>>> +        clock-mult = <1>;
>>>> +        clock-div = <2>;
>>>> +    };
>>>> +
>>>>       dpll_ddr_ck: dpll_ddr_ck {
>>>>           #clock-cells = <0>;
>>>>           compatible = "ti,am3-dpll-clock";
>>>>

By the way, does this patch is still correct taking into account dependency
from cpufreq?
Does it make sense update it to use dpll_mpu_ck and resend?

-- 
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [4.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers
@ 2015-11-30 13:49                 ` Grygorii Strashko
  0 siblings, 0 replies; 12+ messages in thread
From: Grygorii Strashko @ 2015-11-30 13:49 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/30/2015 03:32 PM, Tero Kristo wrote:
> On 11/30/2015 01:53 PM, Grygorii Strashko wrote:
>> On 11/30/2015 10:25 AM, Tero Kristo wrote:
>>> On 11/27/2015 09:44 PM, Grygorii Strashko wrote:
>>>> ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2.
>>>> But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result.
>>>> Timekeeping core misbehaves. For example, execution of command
>>>> "sleep 5" will take 10 sec instead of 5.
>>>>
>>>> Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use
>>>> it for clocking ARM TWD and Global timer (same way as on OMAP4).
>>>>
>>>> Cc: Tony Lindgren <tony@atomide.com>
>>>> Cc: Felipe Balbi <balbi@ti.com>
>>>> Cc: Tero Kristo <t-kristo@ti.com>
>>>> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and
>>>> SCU nodes")
>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>> ---
>>>>   arch/arm/boot/dts/am4372.dtsi        | 4 ++--
>>>>   arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++
>>>>   2 files changed, 10 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/am4372.dtsi
>>>> b/arch/arm/boot/dts/am4372.dtsi
>>>> index d83ff9c..de8791a 100644
>>>> --- a/arch/arm/boot/dts/am4372.dtsi
>>>> +++ b/arch/arm/boot/dts/am4372.dtsi
>>>> @@ -74,7 +74,7 @@
>>>>           reg = <0x48240200 0x100>;
>>>>           interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
>>>>           interrupt-parent = <&gic>;
>>>> -        clocks = <&dpll_mpu_m2_ck>;
>>>> +        clocks = <&mpu_periphclk>;
>>>>       };
>>>>
>>>>       local_timer: timer at 48240600 {
>>>> @@ -82,7 +82,7 @@
>>>>           reg = <0x48240600 0x100>;
>>>>           interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
>>>>           interrupt-parent = <&gic>;
>>>> -        clocks = <&dpll_mpu_m2_ck>;
>>>> +        clocks = <&mpu_periphclk>;
>>>>       };
>>>>
>>>>       l2-cache-controller at 48242000 {
>>>> diff --git a/arch/arm/boot/dts/am43xx-clocks.dtsi
>>>> b/arch/arm/boot/dts/am43xx-clocks.dtsi
>>>> index cc88728..2ff58b1 100644
>>>> --- a/arch/arm/boot/dts/am43xx-clocks.dtsi
>>>> +++ b/arch/arm/boot/dts/am43xx-clocks.dtsi
>>>> @@ -259,6 +259,14 @@
>>>>           ti,invert-autoidle-bit;
>>>>       };
>>>>
>>>> +    mpu_periphclk: mpu_periphclk {
>>>> +        #clock-cells = <0>;
>>>> +        compatible = "fixed-factor-clock";
>>>> +        clocks = <&dpll_mpu_ck>;
>>>
>>> I don't think this is correct, ARM core is fed dpll_mpu_m2_ck, where the
>>> divisor value can potentially differ from 1. If you feed this clock
>>> directly from dpll_mpu_ck, you bypass this divisor.
>>
>> Sry. My mistake. I'll update it to use dpll_mpu_m2_ck.
>>
>>>
>>> Did you check what is the impact of cpufreq on the ARM TWD/timers?
>>
>> TWD is cpufreq friendly, ARM GT is not.
> 
> I think the TWD kick period changes with cpufreq also right?

linux/arch/arm/kernel/smp_twd.c has code to handle cpufreq.

> 
> How are the clocks handled with cpufreq? The user just needs to 
> understand that the timers will be screwed if he uses ARM GT? Should we 
> add some sort of dependency to disable the ARM GT if cpufreq is enabled?

Yep. May be, but very good question is how to do that in case of
OMAP multiplatform build which enables most of all config options at once.

There two threads related to this:
[1] http://www.spinics.net/lists/arm-kernel/msg459649.html
[2] http://www.spinics.net/lists/arm-kernel/msg461141.html

Personally, I've do not see better way than [2] right now.


>>>> +        clock-mult = <1>;
>>>> +        clock-div = <2>;
>>>> +    };
>>>> +
>>>>       dpll_ddr_ck: dpll_ddr_ck {
>>>>           #clock-cells = <0>;
>>>>           compatible = "ti,am3-dpll-clock";
>>>>

By the way, does this patch is still correct taking into account dependency
from cpufreq?
Does it make sense update it to use dpll_mpu_ck and resend?

-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [4.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers
  2015-11-30 13:49                 ` Grygorii Strashko
@ 2015-11-30 14:02                     ` Tero Kristo
  -1 siblings, 0 replies; 12+ messages in thread
From: Tero Kristo @ 2015-11-30 14:02 UTC (permalink / raw)
  To: Grygorii Strashko, tony-4v6yS6AI5VpBDgjK7y7TUQ
  Cc: nsekhar-l0cyMroinI0, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Felipe Balbi

On 11/30/2015 03:49 PM, Grygorii Strashko wrote:
> On 11/30/2015 03:32 PM, Tero Kristo wrote:
>> On 11/30/2015 01:53 PM, Grygorii Strashko wrote:
>>> On 11/30/2015 10:25 AM, Tero Kristo wrote:
>>>> On 11/27/2015 09:44 PM, Grygorii Strashko wrote:
>>>>> ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2.
>>>>> But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result.
>>>>> Timekeeping core misbehaves. For example, execution of command
>>>>> "sleep 5" will take 10 sec instead of 5.
>>>>>
>>>>> Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use
>>>>> it for clocking ARM TWD and Global timer (same way as on OMAP4).
>>>>>
>>>>> Cc: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
>>>>> Cc: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
>>>>> Cc: Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org>
>>>>> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and
>>>>> SCU nodes")
>>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
>>>>> ---
>>>>>    arch/arm/boot/dts/am4372.dtsi        | 4 ++--
>>>>>    arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++
>>>>>    2 files changed, 10 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/am4372.dtsi
>>>>> b/arch/arm/boot/dts/am4372.dtsi
>>>>> index d83ff9c..de8791a 100644
>>>>> --- a/arch/arm/boot/dts/am4372.dtsi
>>>>> +++ b/arch/arm/boot/dts/am4372.dtsi
>>>>> @@ -74,7 +74,7 @@
>>>>>            reg = <0x48240200 0x100>;
>>>>>            interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
>>>>>            interrupt-parent = <&gic>;
>>>>> -        clocks = <&dpll_mpu_m2_ck>;
>>>>> +        clocks = <&mpu_periphclk>;
>>>>>        };
>>>>>
>>>>>        local_timer: timer@48240600 {
>>>>> @@ -82,7 +82,7 @@
>>>>>            reg = <0x48240600 0x100>;
>>>>>            interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
>>>>>            interrupt-parent = <&gic>;
>>>>> -        clocks = <&dpll_mpu_m2_ck>;
>>>>> +        clocks = <&mpu_periphclk>;
>>>>>        };
>>>>>
>>>>>        l2-cache-controller@48242000 {
>>>>> diff --git a/arch/arm/boot/dts/am43xx-clocks.dtsi
>>>>> b/arch/arm/boot/dts/am43xx-clocks.dtsi
>>>>> index cc88728..2ff58b1 100644
>>>>> --- a/arch/arm/boot/dts/am43xx-clocks.dtsi
>>>>> +++ b/arch/arm/boot/dts/am43xx-clocks.dtsi
>>>>> @@ -259,6 +259,14 @@
>>>>>            ti,invert-autoidle-bit;
>>>>>        };
>>>>>
>>>>> +    mpu_periphclk: mpu_periphclk {
>>>>> +        #clock-cells = <0>;
>>>>> +        compatible = "fixed-factor-clock";
>>>>> +        clocks = <&dpll_mpu_ck>;
>>>>
>>>> I don't think this is correct, ARM core is fed dpll_mpu_m2_ck, where the
>>>> divisor value can potentially differ from 1. If you feed this clock
>>>> directly from dpll_mpu_ck, you bypass this divisor.
>>>
>>> Sry. My mistake. I'll update it to use dpll_mpu_m2_ck.
>>>
>>>>
>>>> Did you check what is the impact of cpufreq on the ARM TWD/timers?
>>>
>>> TWD is cpufreq friendly, ARM GT is not.
>>
>> I think the TWD kick period changes with cpufreq also right?
>
> linux/arch/arm/kernel/smp_twd.c has code to handle cpufreq.
>
>>
>> How are the clocks handled with cpufreq? The user just needs to
>> understand that the timers will be screwed if he uses ARM GT? Should we
>> add some sort of dependency to disable the ARM GT if cpufreq is enabled?
>
> Yep. May be, but very good question is how to do that in case of
> OMAP multiplatform build which enables most of all config options at once.
>
> There two threads related to this:
> [1] http://www.spinics.net/lists/arm-kernel/msg459649.html
> [2] http://www.spinics.net/lists/arm-kernel/msg461141.html
>
> Personally, I've do not see better way than [2] right now.

Yeah, [2] seems the way to go.

>
>
>>>>> +        clock-mult = <1>;
>>>>> +        clock-div = <2>;
>>>>> +    };
>>>>> +
>>>>>        dpll_ddr_ck: dpll_ddr_ck {
>>>>>            #clock-cells = <0>;
>>>>>            compatible = "ti,am3-dpll-clock";
>>>>>
>
> By the way, does this patch is still correct taking into account dependency
> from cpufreq?
> Does it make sense update it to use dpll_mpu_ck and resend?
>

Well, this patch is still valid, as the selection of the clocksource 
should be done elsewhere, and this patch should not care about that.

-Tero

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [4.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers
@ 2015-11-30 14:02                     ` Tero Kristo
  0 siblings, 0 replies; 12+ messages in thread
From: Tero Kristo @ 2015-11-30 14:02 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/30/2015 03:49 PM, Grygorii Strashko wrote:
> On 11/30/2015 03:32 PM, Tero Kristo wrote:
>> On 11/30/2015 01:53 PM, Grygorii Strashko wrote:
>>> On 11/30/2015 10:25 AM, Tero Kristo wrote:
>>>> On 11/27/2015 09:44 PM, Grygorii Strashko wrote:
>>>>> ARM TWD and Global timer are clocked by PERIPHCLK which is MPU_CLK/2.
>>>>> But now they are clocked by dpll_mpu_m2_ck == MPU_CLK and, as result.
>>>>> Timekeeping core misbehaves. For example, execution of command
>>>>> "sleep 5" will take 10 sec instead of 5.
>>>>>
>>>>> Hence, fix it by adding mpu_periphclk ("fixed-factor-clock") and use
>>>>> it for clocking ARM TWD and Global timer (same way as on OMAP4).
>>>>>
>>>>> Cc: Tony Lindgren <tony@atomide.com>
>>>>> Cc: Felipe Balbi <balbi@ti.com>
>>>>> Cc: Tero Kristo <t-kristo@ti.com>
>>>>> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and
>>>>> SCU nodes")
>>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>>> ---
>>>>>    arch/arm/boot/dts/am4372.dtsi        | 4 ++--
>>>>>    arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++
>>>>>    2 files changed, 10 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/am4372.dtsi
>>>>> b/arch/arm/boot/dts/am4372.dtsi
>>>>> index d83ff9c..de8791a 100644
>>>>> --- a/arch/arm/boot/dts/am4372.dtsi
>>>>> +++ b/arch/arm/boot/dts/am4372.dtsi
>>>>> @@ -74,7 +74,7 @@
>>>>>            reg = <0x48240200 0x100>;
>>>>>            interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
>>>>>            interrupt-parent = <&gic>;
>>>>> -        clocks = <&dpll_mpu_m2_ck>;
>>>>> +        clocks = <&mpu_periphclk>;
>>>>>        };
>>>>>
>>>>>        local_timer: timer at 48240600 {
>>>>> @@ -82,7 +82,7 @@
>>>>>            reg = <0x48240600 0x100>;
>>>>>            interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
>>>>>            interrupt-parent = <&gic>;
>>>>> -        clocks = <&dpll_mpu_m2_ck>;
>>>>> +        clocks = <&mpu_periphclk>;
>>>>>        };
>>>>>
>>>>>        l2-cache-controller at 48242000 {
>>>>> diff --git a/arch/arm/boot/dts/am43xx-clocks.dtsi
>>>>> b/arch/arm/boot/dts/am43xx-clocks.dtsi
>>>>> index cc88728..2ff58b1 100644
>>>>> --- a/arch/arm/boot/dts/am43xx-clocks.dtsi
>>>>> +++ b/arch/arm/boot/dts/am43xx-clocks.dtsi
>>>>> @@ -259,6 +259,14 @@
>>>>>            ti,invert-autoidle-bit;
>>>>>        };
>>>>>
>>>>> +    mpu_periphclk: mpu_periphclk {
>>>>> +        #clock-cells = <0>;
>>>>> +        compatible = "fixed-factor-clock";
>>>>> +        clocks = <&dpll_mpu_ck>;
>>>>
>>>> I don't think this is correct, ARM core is fed dpll_mpu_m2_ck, where the
>>>> divisor value can potentially differ from 1. If you feed this clock
>>>> directly from dpll_mpu_ck, you bypass this divisor.
>>>
>>> Sry. My mistake. I'll update it to use dpll_mpu_m2_ck.
>>>
>>>>
>>>> Did you check what is the impact of cpufreq on the ARM TWD/timers?
>>>
>>> TWD is cpufreq friendly, ARM GT is not.
>>
>> I think the TWD kick period changes with cpufreq also right?
>
> linux/arch/arm/kernel/smp_twd.c has code to handle cpufreq.
>
>>
>> How are the clocks handled with cpufreq? The user just needs to
>> understand that the timers will be screwed if he uses ARM GT? Should we
>> add some sort of dependency to disable the ARM GT if cpufreq is enabled?
>
> Yep. May be, but very good question is how to do that in case of
> OMAP multiplatform build which enables most of all config options at once.
>
> There two threads related to this:
> [1] http://www.spinics.net/lists/arm-kernel/msg459649.html
> [2] http://www.spinics.net/lists/arm-kernel/msg461141.html
>
> Personally, I've do not see better way than [2] right now.

Yeah, [2] seems the way to go.

>
>
>>>>> +        clock-mult = <1>;
>>>>> +        clock-div = <2>;
>>>>> +    };
>>>>> +
>>>>>        dpll_ddr_ck: dpll_ddr_ck {
>>>>>            #clock-cells = <0>;
>>>>>            compatible = "ti,am3-dpll-clock";
>>>>>
>
> By the way, does this patch is still correct taking into account dependency
> from cpufreq?
> Does it make sense update it to use dpll_mpu_ck and resend?
>

Well, this patch is still valid, as the selection of the clocksource 
should be done elsewhere, and this patch should not care about that.

-Tero

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-11-30 14:02 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-27 19:44 [4.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers Grygorii Strashko
2015-11-27 19:44 ` Grygorii Strashko
     [not found] ` <1448653468-24908-1-git-send-email-grygorii.strashko-l0cyMroinI0@public.gmane.org>
2015-11-30  8:25   ` Tero Kristo
2015-11-30  8:25     ` Tero Kristo
     [not found]     ` <565C0814.3000001-l0cyMroinI0@public.gmane.org>
2015-11-30 11:53       ` Grygorii Strashko
2015-11-30 11:53         ` Grygorii Strashko
     [not found]         ` <565C38CE.1070907-l0cyMroinI0@public.gmane.org>
2015-11-30 13:32           ` Tero Kristo
2015-11-30 13:32             ` Tero Kristo
     [not found]             ` <565C4FE2.3090403-l0cyMroinI0@public.gmane.org>
2015-11-30 13:49               ` Grygorii Strashko
2015-11-30 13:49                 ` Grygorii Strashko
     [not found]                 ` <565C53D0.6090305-l0cyMroinI0@public.gmane.org>
2015-11-30 14:02                   ` Tero Kristo
2015-11-30 14:02                     ` Tero Kristo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.