From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tero Kristo Subject: Re: [4.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers Date: Mon, 30 Nov 2015 15:32:18 +0200 Message-ID: <565C4FE2.3090403@ti.com> References: <1448653468-24908-1-git-send-email-grygorii.strashko@ti.com> <565C0814.3000001@ti.com> <565C38CE.1070907@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <565C38CE.1070907-l0cyMroinI0@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Grygorii Strashko , tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org Cc: nsekhar-l0cyMroinI0@public.gmane.org, linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Felipe Balbi List-Id: devicetree@vger.kernel.org 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 >>> Cc: Felipe Balbi >>> Cc: Tero Kristo >>> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and >>> SCU nodes") >>> Signed-off-by: Grygorii Strashko >>> --- >>> 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 = ; >>> interrupt-parent = <&gic>; >>> - clocks = <&dpll_mpu_m2_ck>; >>> + clocks = <&mpu_periphclk>; >>> }; >>> >>> local_timer: timer@48240600 { >>> @@ -82,7 +82,7 @@ >>> reg = <0x48240600 0x100>; >>> interrupts = ; >>> 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: t-kristo@ti.com (Tero Kristo) Date: Mon, 30 Nov 2015 15:32:18 +0200 Subject: [4.4-rc][PATCH] ARM: dts: am4372: fix clock source for arm twd and global timers In-Reply-To: <565C38CE.1070907@ti.com> References: <1448653468-24908-1-git-send-email-grygorii.strashko@ti.com> <565C0814.3000001@ti.com> <565C38CE.1070907@ti.com> Message-ID: <565C4FE2.3090403@ti.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 >>> Cc: Felipe Balbi >>> Cc: Tero Kristo >>> Fixes:commit 8cbd4c2f6a99 ("arm: boot: dts: am4372: add ARM timers and >>> SCU nodes") >>> Signed-off-by: Grygorii Strashko >>> --- >>> 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 = ; >>> interrupt-parent = <&gic>; >>> - clocks = <&dpll_mpu_m2_ck>; >>> + clocks = <&mpu_periphclk>; >>> }; >>> >>> local_timer: timer at 48240600 { >>> @@ -82,7 +82,7 @@ >>> reg = <0x48240600 0x100>; >>> interrupts = ; >>> 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"; >>> >> > >