ARM: dts: am4372: fix clock source for arm twd and global timers
PATCH 1/1 ARM: dts: am4372: fix clock source for arm twd and global timers
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).
--- arch/arm/boot/dts/am4372.dtsi | 4 ++-- arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-)
arch/arm/boot/dts/am4372.dtsi +2 -2
@@ -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 {
arch/arm/boot/dts/am43xx-clocks.dtsi +8 -0
@@ -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";
Signature · 1 line
2.6.3
PATCH 1/1 ARM: dts: am4372: fix clock source for arm twd and global timers
Quoted 45 lines · Grygorii Strashko
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_CLKand, 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 <[email protected]>Cc Felipe Balbi <[email protected]>Cc Tero Kristo <[email protected]>Fixes commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and SCU nodes")Signed-off-by Grygorii Strashko <[email protected]>--- arch/arm/boot/dts/am4372.dtsi | 4 ++-- arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-)arch/arm/boot/dts/am4372.dtsi +2
-2@@ -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 {arch/arm/boot/dts/am43xx-clocks.dtsi +0
-0
arch/arm/boot/dts/am43xx-clocks.dtsi line 265
Quoted 7 lines
arch/arm/boot/dts/am43xx-clocks.dtsi +4
-0@@ -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
Quoted 8 lines
arch/arm/boot/dts/am43xx-clocks.dtsi +4
-0+ clock-mult = <1>; + clock-div = <2>; + }; + dpll_ddr_ck: dpll_ddr_ck { #clock-cells = <0>; compatible = "ti,am3-dpll-clock";
PATCH 1/1 ARM: dts: am4372: fix clock source for arm twd and global timers
Quoted 52 lines · Tero Kristo
Quoted 51 lines · Grygorii Strashko
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_CLKand, 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 <[email protected]>Cc Felipe Balbi <[email protected]>Cc Tero Kristo <[email protected]>Fixes commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers andSCU nodes")
Signed-off-by Grygorii Strashko <[email protected]>--- arch/arm/boot/dts/am4372.dtsi | 4 ++-- arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-)arch/arm/boot/dts/am4372.dtsi +2
-2@@ -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 {arch/arm/boot/dts/am43xx-clocks.dtsi +0
-0@@ -259,6 +259,14 @@ ti,invert-autoidle-bit; };
Quoted 8 lines
arch/arm/boot/dts/am43xx-clocks.dtsi line 265
Quoted 4 lines
arch/arm/boot/dts/am43xx-clocks.dtsi +4
-0+ 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 fromdpll_mpu_ck, you bypass this divisor.
Sry. My mistake. I'll update it to use dpll_mpu_m2_ck.
Quoted 2 lines
Did you check what is the impact of cpufreq on the ARM TWD/timers?
TWD is cpufreq friendly, ARM GT is not.
Quoted 9 lines
Quoted 8 lines
arch/arm/boot/dts/am43xx-clocks.dtsi +4
-0+ clock-mult = <1>; + clock-div = <2>; + }; + dpll_ddr_ck: dpll_ddr_ck { #clock-cells = <0>; compatible = "ti,am3-dpll-clock";
Signature · 2 lines
regards, -grygorii
PATCH 1/1 ARM: dts: am4372: fix clock source for arm twd and global timers
Quoted 62 lines · Grygorii Strashko
Quoted 52 lines · Tero Kristo
Quoted 51 lines · Grygorii Strashko
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_CLKand, 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 <[email protected]>Cc Felipe Balbi <[email protected]>Cc Tero Kristo <[email protected]>Fixes commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers andSCU nodes")
Signed-off-by Grygorii Strashko <[email protected]>--- arch/arm/boot/dts/am4372.dtsi | 4 ++-- arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-)arch/arm/boot/dts/am4372.dtsi +2
-2@@ -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 {arch/arm/boot/dts/am43xx-clocks.dtsi +0
-0@@ -259,6 +259,14 @@ ti,invert-autoidle-bit; };Quoted 8 lines
arch/arm/boot/dts/am43xx-clocks.dtsi line 265
Quoted 4 lines
arch/arm/boot/dts/am43xx-clocks.dtsi +4
-0+ 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 fromdpll_mpu_ck, you bypass this divisor.
Quoted 6 lines
Sry. My mistake. I'll update it to use
dpll_mpu_m2_ck.Quoted 2 lines
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
Quoted 13 lines
Quoted 11 lines
Quoted 10 lines
arch/arm/boot/dts/am43xx-clocks.dtsi +4
-0+ clock-mult = <1>; + clock-div = <2>; + }; + dpll_ddr_ck: dpll_ddr_ck { #clock-cells = <0>; compatible = "ti,am3-dpll-clock";
PATCH 1/1 ARM: dts: am4372: fix clock source for arm twd and global timers
Quoted 63 lines · Tero Kristo
Quoted 62 lines · Grygorii Strashko
Quoted 60 lines · Tero Kristo
Quoted 48 lines · Grygorii Strashko
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_CLKand, 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 <[email protected]>Cc Felipe Balbi <[email protected]>Cc Tero Kristo <[email protected]>Fixes commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers andSCU nodes")
Signed-off-by Grygorii Strashko <[email protected]>--- arch/arm/boot/dts/am4372.dtsi | 4 ++-- arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-)arch/arm/boot/dts/am4372.dtsi +2
-2@@ -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 {arch/arm/boot/dts/am43xx-clocks.dtsi +0
-0arch/arm/boot/dts/am43xx-clocks.dtsi line 265
Quoted 7 lines
arch/arm/boot/dts/am43xx-clocks.dtsi +4
-0@@ -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 fromdpll_mpu_ck, you bypass this divisor.
Quoted 8 lines
Quoted 6 lines
Sry. My mistake. I'll update it to use
dpll_mpu_m2_ck.Quoted 2 lines
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.
Quoted 4 lines
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.
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
arch/arm/boot/dts/am43xx-clocks.dtsi line 272
Quoted 8 lines
arch/arm/boot/dts/am43xx-clocks.dtsi +4
-0+ 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?
Signature · 2 lines
regards, -grygorii
PATCH 1/1 ARM: dts: am4372: fix clock source for arm twd and global timers
Quoted 80 lines · Grygorii Strashko
Quoted 63 lines · Tero Kristo
Quoted 62 lines · Grygorii Strashko
Quoted 60 lines · Tero Kristo
Quoted 48 lines · Grygorii Strashko
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_CLKand, 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 <[email protected]>Cc Felipe Balbi <[email protected]>Cc Tero Kristo <[email protected]>Fixes commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers andSCU nodes")
Signed-off-by Grygorii Strashko <[email protected]>--- arch/arm/boot/dts/am4372.dtsi | 4 ++-- arch/arm/boot/dts/am43xx-clocks.dtsi | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-)arch/arm/boot/dts/am4372.dtsi +2
-2@@ -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 {arch/arm/boot/dts/am43xx-clocks.dtsi +0
-0arch/arm/boot/dts/am43xx-clocks.dtsi line 265
Quoted 7 lines
arch/arm/boot/dts/am43xx-clocks.dtsi +4
-0@@ -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 fromdpll_mpu_ck, you bypass this divisor.Quoted 8 lines
Quoted 6 lines
Sry. My mistake. I'll update it to use
dpll_mpu_m2_ck.Quoted 2 lines
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.chas code to handle cpufreq.Quoted 4 lines
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?
Quoted 8 lines
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.htmlPersonally, I've do not see better way than [2] right now.
Yeah, [2] seems the way to go.
Quoted 11 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
arch/arm/boot/dts/am43xx-clocks.dtsi line 272
Quoted 10 lines
arch/arm/boot/dts/am43xx-clocks.dtsi +4
-0+ clock-mult = <1>; + clock-div = <2>; + }; + dpll_ddr_ck: dpll_ddr_ck { #clock-cells = <0>; compatible = "ti,am3-dpll-clock";
Quoted 4 lines
By the way, does this patch is still correct taking into account dependency from cpufreq?
Does it make sense update it to usedpll_mpu_ckand 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