* [PATCH 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups
@ 2016-04-26 17:54 J.D. Schroeder
2016-04-26 17:54 ` [PATCH 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div J.D. Schroeder
` (3 more replies)
0 siblings, 4 replies; 29+ messages in thread
From: J.D. Schroeder @ 2016-04-26 17:54 UTC (permalink / raw)
To: linux-kernel, bcousson, tony, robh+dt, pawel.moll, mark.rutland,
ijc+devicetree, galak, linux, linux-omap, devicetree,
linux-arm-kernel
This series of patches fixes several discrepancies between the
AM57/DRA7 clock tree description and the actual hardware behavior and
frequencies. With these changes a more complete picture of the clock
tree is represented for a few of the clocks and their resulting
frequencies.
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div 2016-04-26 17:54 [PATCH 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups J.D. Schroeder @ 2016-04-26 17:54 ` J.D. Schroeder 2016-04-27 11:21 ` Tero Kristo 2016-04-26 17:54 ` [PATCH 2/3] ARM: DRA7x: dts: Fix the 32kHz clock calculation J.D. Schroeder ` (2 subsequent siblings) 3 siblings, 1 reply; 29+ messages in thread From: J.D. Schroeder @ 2016-04-26 17:54 UTC (permalink / raw) To: linux-kernel, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: J.D. Schroeder From: "J.D. Schroeder" <jay.schroeder@garmin.com> This commit fixes the clock data inside the DRA7xx clocks device tree structure for the gmac_gmii_ref_clk_div clock. This clock is actually the GMAC_MAIN_CLK and has nothing to do with the register at address 0x4a0093d0. If CLKSEL_REF bit 24 inside of CM_GMAC_GMAC_CLKCTRL, is set to 1 in order to use the GMAC_RMII_CLK instead of the GMAC_RMII_HS_CLK, the kernel generates a clock divider warning: WARNING: CPU: 0 PID: 0 at drivers/clk/clk-divider.c:129 clk_divider_recalc_rate+0xa8/0xe0() gmac_gmii_ref_clk_div: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set By properly configuring the gmac_gmii_ref_clk_div (GMAC_MAIN_CLK) to have the parent of dpll_gmac_m2_ck always divided by 2 the warning is resolved and the clock tree is fixed up. Additionally, a new clock called rmii_50mhz_clk_mux is defined that does utilize CM_GMAC_GMAC_CLKCTRL[24] CLKSEL_REF to configure the source clock for the RMII_50MHZ_CLK. Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com> Reviewed-by: Trenton Andres <trenton.andres@garmin.com> --- arch/arm/boot/dts/dra7xx-clocks.dtsi | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/arch/arm/boot/dts/dra7xx-clocks.dtsi b/arch/arm/boot/dts/dra7xx-clocks.dtsi index d0bae06..9d1a583 100644 --- a/arch/arm/boot/dts/dra7xx-clocks.dtsi +++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi @@ -1710,13 +1710,20 @@ reg = <0x0c00>; }; - gmac_gmii_ref_clk_div: gmac_gmii_ref_clk_div { + rmii_50mhz_clk_mux: rmii_50mhz_clk_mux { #clock-cells = <0>; - compatible = "ti,divider-clock"; - clocks = <&dpll_gmac_m2_ck>; + compatible = "ti,mux-clock"; + clocks = <&dpll_gmac_h11x2_ck>, <&rmii_clk_ck>; ti,bit-shift = <24>; reg = <0x13d0>; - ti,dividers = <2>; + }; + + gmac_gmii_ref_clk_div: gmac_gmii_ref_clk_div { + #clock-cells = <0>; + compatible = "fixed-factor-clock"; + clocks = <&dpll_gmac_m2_ck>; + clock-mult = <1>; + clock-div = <2>; }; gmac_rft_clk_mux: gmac_rft_clk_mux { -- 1.9.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div 2016-04-26 17:54 ` [PATCH 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div J.D. Schroeder @ 2016-04-27 11:21 ` Tero Kristo 2016-04-27 16:36 ` Tony Lindgren 0 siblings, 1 reply; 29+ messages in thread From: Tero Kristo @ 2016-04-27 11:21 UTC (permalink / raw) To: J.D. Schroeder, linux-kernel, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: J.D. Schroeder On 26/04/16 20:54, J.D. Schroeder wrote: > From: "J.D. Schroeder" <jay.schroeder@garmin.com> > > This commit fixes the clock data inside the DRA7xx clocks device tree > structure for the gmac_gmii_ref_clk_div clock. This clock is actually > the GMAC_MAIN_CLK and has nothing to do with the register at address > 0x4a0093d0. If CLKSEL_REF bit 24 inside of CM_GMAC_GMAC_CLKCTRL, is > set to 1 in order to use the GMAC_RMII_CLK instead of the > GMAC_RMII_HS_CLK, the kernel generates a clock divider warning: > WARNING: CPU: 0 PID: 0 at drivers/clk/clk-divider.c:129 clk_divider_recalc_rate+0xa8/0xe0() > gmac_gmii_ref_clk_div: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set > > By properly configuring the gmac_gmii_ref_clk_div (GMAC_MAIN_CLK) to > have the parent of dpll_gmac_m2_ck always divided by 2 the warning is > resolved and the clock tree is fixed up. > > Additionally, a new clock called rmii_50mhz_clk_mux is defined that > does utilize CM_GMAC_GMAC_CLKCTRL[24] CLKSEL_REF to configure the > source clock for the RMII_50MHZ_CLK. > > Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com> > Reviewed-by: Trenton Andres <trenton.andres@garmin.com> Looks like something weird happened with the clock data conversion tool with this specific clock. Seems to be the only buggy instance in our clock data across SoCs. Good catch. Acked-by: Tero Kristo <t-kristo@ti.com> > --- > arch/arm/boot/dts/dra7xx-clocks.dtsi | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/boot/dts/dra7xx-clocks.dtsi b/arch/arm/boot/dts/dra7xx-clocks.dtsi > index d0bae06..9d1a583 100644 > --- a/arch/arm/boot/dts/dra7xx-clocks.dtsi > +++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi > @@ -1710,13 +1710,20 @@ > reg = <0x0c00>; > }; > > - gmac_gmii_ref_clk_div: gmac_gmii_ref_clk_div { > + rmii_50mhz_clk_mux: rmii_50mhz_clk_mux { > #clock-cells = <0>; > - compatible = "ti,divider-clock"; > - clocks = <&dpll_gmac_m2_ck>; > + compatible = "ti,mux-clock"; > + clocks = <&dpll_gmac_h11x2_ck>, <&rmii_clk_ck>; > ti,bit-shift = <24>; > reg = <0x13d0>; > - ti,dividers = <2>; > + }; > + > + gmac_gmii_ref_clk_div: gmac_gmii_ref_clk_div { > + #clock-cells = <0>; > + compatible = "fixed-factor-clock"; > + clocks = <&dpll_gmac_m2_ck>; > + clock-mult = <1>; > + clock-div = <2>; > }; > > gmac_rft_clk_mux: gmac_rft_clk_mux { > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div 2016-04-27 11:21 ` Tero Kristo @ 2016-04-27 16:36 ` Tony Lindgren 2016-04-27 17:16 ` Tony Lindgren 0 siblings, 1 reply; 29+ messages in thread From: Tony Lindgren @ 2016-04-27 16:36 UTC (permalink / raw) To: Tero Kristo Cc: J.D. Schroeder, linux-kernel, bcousson, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel, J.D. Schroeder * Tero Kristo <t-kristo@ti.com> [160427 04:22]: > On 26/04/16 20:54, J.D. Schroeder wrote: > >From: "J.D. Schroeder" <jay.schroeder@garmin.com> > > > >This commit fixes the clock data inside the DRA7xx clocks device tree > >structure for the gmac_gmii_ref_clk_div clock. This clock is actually > >the GMAC_MAIN_CLK and has nothing to do with the register at address > >0x4a0093d0. If CLKSEL_REF bit 24 inside of CM_GMAC_GMAC_CLKCTRL, is > >set to 1 in order to use the GMAC_RMII_CLK instead of the > >GMAC_RMII_HS_CLK, the kernel generates a clock divider warning: > > WARNING: CPU: 0 PID: 0 at drivers/clk/clk-divider.c:129 clk_divider_recalc_rate+0xa8/0xe0() > > gmac_gmii_ref_clk_div: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set > > > >By properly configuring the gmac_gmii_ref_clk_div (GMAC_MAIN_CLK) to > >have the parent of dpll_gmac_m2_ck always divided by 2 the warning is > >resolved and the clock tree is fixed up. > > > >Additionally, a new clock called rmii_50mhz_clk_mux is defined that > >does utilize CM_GMAC_GMAC_CLKCTRL[24] CLKSEL_REF to configure the > >source clock for the RMII_50MHZ_CLK. > > > >Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com> > >Reviewed-by: Trenton Andres <trenton.andres@garmin.com> > > Looks like something weird happened with the clock data conversion tool with > this specific clock. Seems to be the only buggy instance in our clock data > across SoCs. Good catch. > > Acked-by: Tero Kristo <t-kristo@ti.com> Applying into omap-for-v4.6/fixes thanks. Tony ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div 2016-04-27 16:36 ` Tony Lindgren @ 2016-04-27 17:16 ` Tony Lindgren 2016-05-02 17:12 ` [PATCH v2 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups J.D. Schroeder 0 siblings, 1 reply; 29+ messages in thread From: Tony Lindgren @ 2016-04-27 17:16 UTC (permalink / raw) To: Tero Kristo Cc: J.D. Schroeder, linux-kernel, bcousson, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel, J.D. Schroeder * Tony Lindgren <tony@atomide.com> [160427 09:39]: > * Tero Kristo <t-kristo@ti.com> [160427 04:22]: > > On 26/04/16 20:54, J.D. Schroeder wrote: > > >From: "J.D. Schroeder" <jay.schroeder@garmin.com> > > > > > >This commit fixes the clock data inside the DRA7xx clocks device tree > > >structure for the gmac_gmii_ref_clk_div clock. This clock is actually > > >the GMAC_MAIN_CLK and has nothing to do with the register at address > > >0x4a0093d0. If CLKSEL_REF bit 24 inside of CM_GMAC_GMAC_CLKCTRL, is > > >set to 1 in order to use the GMAC_RMII_CLK instead of the > > >GMAC_RMII_HS_CLK, the kernel generates a clock divider warning: > > > WARNING: CPU: 0 PID: 0 at drivers/clk/clk-divider.c:129 clk_divider_recalc_rate+0xa8/0xe0() > > > gmac_gmii_ref_clk_div: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set > > > > > >By properly configuring the gmac_gmii_ref_clk_div (GMAC_MAIN_CLK) to > > >have the parent of dpll_gmac_m2_ck always divided by 2 the warning is > > >resolved and the clock tree is fixed up. > > > > > >Additionally, a new clock called rmii_50mhz_clk_mux is defined that > > >does utilize CM_GMAC_GMAC_CLKCTRL[24] CLKSEL_REF to configure the > > >source clock for the RMII_50MHZ_CLK. > > > > > >Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com> > > >Reviewed-by: Trenton Andres <trenton.andres@garmin.com> > > > > Looks like something weird happened with the clock data conversion tool with > > this specific clock. Seems to be the only buggy instance in our clock data > > across SoCs. Good catch. > > > > Acked-by: Tero Kristo <t-kristo@ti.com> > > Applying into omap-for-v4.6/fixes thanks. Actually then we end up creating self-inflicted merge conflict here with next. So let's wait a bit on this one as it's harmless. J.D. can you please rebase this against current Linux next? Note the recent unit name and unit address fixes for warnings with make W=1 dtbs. Regards, Tony ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups 2016-04-27 17:16 ` Tony Lindgren @ 2016-05-02 17:12 ` J.D. Schroeder 2016-05-02 17:12 ` [PATCH v2 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div J.D. Schroeder ` (3 more replies) 0 siblings, 4 replies; 29+ messages in thread From: J.D. Schroeder @ 2016-05-02 17:12 UTC (permalink / raw) To: linux-kernel, t-kristo, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: jay.schroeder This series of patches fixes several discrepancies between the AM57/DRA7 clock tree description and the actual hardware behavior and frequencies. With these changes a more complete picture of the clock tree is represented for a few of the clocks and their resulting frequencies. v2 Changes: * Rebased on linux-next as requested by Tony Lindgren * Eliminated previous patch 2 as another change fixing the same thing was merged in eea08802f586acd6aef377d1b4a541821013cc0b * Added to the commit message in patch 2 to clarify the source of the clock being internal to the processor * Added a new patch 3 to fix a new warning introduced by eea08802f586acd6aef377d1b4a541821013cc0b ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div 2016-05-02 17:12 ` [PATCH v2 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups J.D. Schroeder @ 2016-05-02 17:12 ` J.D. Schroeder 2016-05-02 17:12 ` [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency J.D. Schroeder ` (2 subsequent siblings) 3 siblings, 0 replies; 29+ messages in thread From: J.D. Schroeder @ 2016-05-02 17:12 UTC (permalink / raw) To: linux-kernel, t-kristo, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: jay.schroeder From: "J.D. Schroeder" <jay.schroeder@garmin.com> This commit fixes the clock data inside the DRA7xx clocks device tree structure for the gmac_gmii_ref_clk_div clock. This clock is actually the GMAC_MAIN_CLK and has nothing to do with the register at address 0x4a0093d0. If CLKSEL_REF bit 24 inside of CM_GMAC_GMAC_CLKCTRL, is set to 1 in order to use the GMAC_RMII_CLK instead of the GMAC_RMII_HS_CLK, the kernel generates a clock divider warning: WARNING: CPU: 0 PID: 0 at drivers/clk/clk-divider.c:129 clk_divider_recalc_rate+0xa8/0xe0() gmac_gmii_ref_clk_div: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set By properly configuring the gmac_gmii_ref_clk_div (GMAC_MAIN_CLK) to have the parent of dpll_gmac_m2_ck always divided by 2 the warning is resolved and the clock tree is fixed up. Additionally, a new clock called rmii_50mhz_clk_mux is defined that does utilize CM_GMAC_GMAC_CLKCTRL[24] CLKSEL_REF to configure the source clock for the RMII_50MHZ_CLK. Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com> Reviewed-by: Trenton Andres <trenton.andres@garmin.com> --- arch/arm/boot/dts/dra7xx-clocks.dtsi | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/arch/arm/boot/dts/dra7xx-clocks.dtsi b/arch/arm/boot/dts/dra7xx-clocks.dtsi index 8378b44..3f0c61d 100644 --- a/arch/arm/boot/dts/dra7xx-clocks.dtsi +++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi @@ -1718,13 +1718,20 @@ reg = <0x0c00>; }; - gmac_gmii_ref_clk_div: gmac_gmii_ref_clk_div@13d0 { + rmii_50mhz_clk_mux: rmii_50mhz_clk_mux@13d0 { #clock-cells = <0>; - compatible = "ti,divider-clock"; - clocks = <&dpll_gmac_m2_ck>; + compatible = "ti,mux-clock"; + clocks = <&dpll_gmac_h11x2_ck>, <&rmii_clk_ck>; ti,bit-shift = <24>; reg = <0x13d0>; - ti,dividers = <2>; + }; + + gmac_gmii_ref_clk_div: gmac_gmii_ref_clk_div { + #clock-cells = <0>; + compatible = "fixed-factor-clock"; + clocks = <&dpll_gmac_m2_ck>; + clock-mult = <1>; + clock-div = <2>; }; gmac_rft_clk_mux: gmac_rft_clk_mux@13d0 { -- 1.9.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency 2016-05-02 17:12 ` [PATCH v2 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups J.D. Schroeder 2016-05-02 17:12 ` [PATCH v2 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div J.D. Schroeder @ 2016-05-02 17:12 ` J.D. Schroeder 2016-05-03 8:16 ` Tero Kristo 2016-05-02 17:12 ` [PATCH v2 3/3] ARM: dts: dra7: fix clock node definition to avoid build warning J.D. Schroeder 2016-05-03 4:20 ` [PATCH v2 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups Lokesh Vutla 3 siblings, 1 reply; 29+ messages in thread From: J.D. Schroeder @ 2016-05-02 17:12 UTC (permalink / raw) To: linux-kernel, t-kristo, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: jay.schroeder From: "J.D. Schroeder" <jay.schroeder@garmin.com> This commit updates the OSC_32K_CLK (secure_32k_clk_src_ck) frequency from the precise 32kHz frequency (i.e., 32.768 kHz) to a more accurate frequency of ~34.6 kHz. Actual measured frequencies of the clock vary from processor to processor anywhere from 34.4 kHz up to 34.8 kHz. Note that the ~34 kHz frequency clock is generated internally by the processor, not an input to the processor. This change makes it more clear that the consumer of the secure_32k_clk_src_ck will not get a precise 32.768 kHz frequency output. Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com> Reviewed-by: Trenton Andres <trenton.andres@garmin.com> --- arch/arm/boot/dts/dra7xx-clocks.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/dra7xx-clocks.dtsi b/arch/arm/boot/dts/dra7xx-clocks.dtsi index 3f0c61d..f7ec976 100644 --- a/arch/arm/boot/dts/dra7xx-clocks.dtsi +++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi @@ -95,7 +95,7 @@ secure_32k_clk_src_ck: secure_32k_clk_src_ck { #clock-cells = <0>; compatible = "fixed-clock"; - clock-frequency = <32768>; + clock-frequency = <34600>; /* approximate frequency */ }; sys_clk32_crystal_ck: sys_clk32_crystal_ck { -- 1.9.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency 2016-05-02 17:12 ` [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency J.D. Schroeder @ 2016-05-03 8:16 ` Tero Kristo 2016-05-03 13:31 ` J.D. Schroeder 0 siblings, 1 reply; 29+ messages in thread From: Tero Kristo @ 2016-05-03 8:16 UTC (permalink / raw) To: J.D. Schroeder, linux-kernel, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: jay.schroeder On 02/05/16 20:12, J.D. Schroeder wrote: > From: "J.D. Schroeder" <jay.schroeder@garmin.com> > > This commit updates the OSC_32K_CLK (secure_32k_clk_src_ck) frequency > from the precise 32kHz frequency (i.e., 32.768 kHz) to a more > accurate frequency of ~34.6 kHz. Actual measured frequencies of the > clock vary from processor to processor anywhere from 34.4 kHz up to > 34.8 kHz. Note that the ~34 kHz frequency clock is generated > internally by the processor, not an input to the processor. This > change makes it more clear that the consumer of the > secure_32k_clk_src_ck will not get a precise 32.768 kHz frequency > output. > > Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com> > Reviewed-by: Trenton Andres <trenton.andres@garmin.com> > --- > arch/arm/boot/dts/dra7xx-clocks.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/dra7xx-clocks.dtsi b/arch/arm/boot/dts/dra7xx-clocks.dtsi > index 3f0c61d..f7ec976 100644 > --- a/arch/arm/boot/dts/dra7xx-clocks.dtsi > +++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi > @@ -95,7 +95,7 @@ > secure_32k_clk_src_ck: secure_32k_clk_src_ck { > #clock-cells = <0>; > compatible = "fixed-clock"; > - clock-frequency = <32768>; > + clock-frequency = <34600>; /* approximate frequency */ > }; > > sys_clk32_crystal_ck: sys_clk32_crystal_ck { > I still don't agree with this patch. The actual frequency can drift much more, you are just seeing this number at your setup. There is some internal discussion going on in TI about this, but I can't share any data on this yet. -Tero ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency 2016-05-03 8:16 ` Tero Kristo @ 2016-05-03 13:31 ` J.D. Schroeder 2016-05-03 16:43 ` Tony Lindgren 0 siblings, 1 reply; 29+ messages in thread From: J.D. Schroeder @ 2016-05-03 13:31 UTC (permalink / raw) To: Tero Kristo, linux-kernel, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: jay.schroeder On 05/03/2016 03:16 AM, Tero Kristo wrote: > On 02/05/16 20:12, J.D. Schroeder wrote: >> From: "J.D. Schroeder" <jay.schroeder@garmin.com> >> >> This commit updates the OSC_32K_CLK (secure_32k_clk_src_ck) frequency >> from the precise 32kHz frequency (i.e., 32.768 kHz) to a more >> accurate frequency of ~34.6 kHz. Actual measured frequencies of the >> clock vary from processor to processor anywhere from 34.4 kHz up to >> 34.8 kHz. Note that the ~34 kHz frequency clock is generated >> internally by the processor, not an input to the processor. This >> change makes it more clear that the consumer of the >> secure_32k_clk_src_ck will not get a precise 32.768 kHz frequency >> output. >> >> Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com> >> Reviewed-by: Trenton Andres <trenton.andres@garmin.com> >> --- >> arch/arm/boot/dts/dra7xx-clocks.dtsi | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm/boot/dts/dra7xx-clocks.dtsi >> b/arch/arm/boot/dts/dra7xx-clocks.dtsi >> index 3f0c61d..f7ec976 100644 >> --- a/arch/arm/boot/dts/dra7xx-clocks.dtsi >> +++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi >> @@ -95,7 +95,7 @@ >> secure_32k_clk_src_ck: secure_32k_clk_src_ck { >> #clock-cells = <0>; >> compatible = "fixed-clock"; >> - clock-frequency = <32768>; >> + clock-frequency = <34600>; /* approximate frequency */ >> }; >> >> sys_clk32_crystal_ck: sys_clk32_crystal_ck { >> > > I still don't agree with this patch. The actual frequency can drift much more, > you are just seeing this number at your setup. Yes, it can drift significantly from processor to processor. Do you agree that this frequency is closer to what can be expected than 32768 Hz? Like I said, I would have renamed the clock also but I opted to go the less obtrusive route while still helping others that might think they can reasonably use this clock in their design as a 32768 Hz clock source. Perhaps my comment and selection of the approximate frequency is not the best (I'm open for suggestions). However, I do think this change is an improvement and clarifying change to what is currently present in the clock description. -JD ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency 2016-05-03 13:31 ` J.D. Schroeder @ 2016-05-03 16:43 ` Tony Lindgren 2016-05-03 17:32 ` Tero Kristo 0 siblings, 1 reply; 29+ messages in thread From: Tony Lindgren @ 2016-05-03 16:43 UTC (permalink / raw) To: J.D. Schroeder Cc: Tero Kristo, linux-kernel, bcousson, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel, jay.schroeder, Matthijs van Duin * J.D. Schroeder <Linux.HWI@garmin.com> [160503 06:32]: > On 05/03/2016 03:16 AM, Tero Kristo wrote: > > On 02/05/16 20:12, J.D. Schroeder wrote: > >> From: "J.D. Schroeder" <jay.schroeder@garmin.com> > >> > >> This commit updates the OSC_32K_CLK (secure_32k_clk_src_ck) frequency > >> from the precise 32kHz frequency (i.e., 32.768 kHz) to a more > >> accurate frequency of ~34.6 kHz. Actual measured frequencies of the > >> clock vary from processor to processor anywhere from 34.4 kHz up to > >> 34.8 kHz. Note that the ~34 kHz frequency clock is generated > >> internally by the processor, not an input to the processor. This > >> change makes it more clear that the consumer of the > >> secure_32k_clk_src_ck will not get a precise 32.768 kHz frequency > >> output. > >> > >> Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com> > >> Reviewed-by: Trenton Andres <trenton.andres@garmin.com> > >> --- > >> arch/arm/boot/dts/dra7xx-clocks.dtsi | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/arch/arm/boot/dts/dra7xx-clocks.dtsi > >> b/arch/arm/boot/dts/dra7xx-clocks.dtsi > >> index 3f0c61d..f7ec976 100644 > >> --- a/arch/arm/boot/dts/dra7xx-clocks.dtsi > >> +++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi > >> @@ -95,7 +95,7 @@ > >> secure_32k_clk_src_ck: secure_32k_clk_src_ck { > >> #clock-cells = <0>; > >> compatible = "fixed-clock"; > >> - clock-frequency = <32768>; > >> + clock-frequency = <34600>; /* approximate frequency */ > >> }; > >> > >> sys_clk32_crystal_ck: sys_clk32_crystal_ck { > >> > > > > I still don't agree with this patch. The actual frequency can drift much more, > > you are just seeing this number at your setup. > > Yes, it can drift significantly from processor to processor. Do you agree that > this frequency is closer to what can be expected than 32768 Hz? > > Like I said, I would have renamed the clock also but I opted to go the less > obtrusive route while still helping others that might think they can > reasonably use this clock in their design as a 32768 Hz clock source. Perhaps > my comment and selection of the approximate frequency is not the best (I'm > open for suggestions). However, I do think this change is an improvement and > clarifying change to what is currently present in the clock description. Does a fixed divider calculation of input * (32768 / 0x27e6) make sense here too as pointed out earlier by Matthijs for the ti81xx? Regards, Tony ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency 2016-05-03 16:43 ` Tony Lindgren @ 2016-05-03 17:32 ` Tero Kristo 2016-05-03 17:49 ` J.D. Schroeder 2016-05-04 14:09 ` Matthijs van Duin 0 siblings, 2 replies; 29+ messages in thread From: Tero Kristo @ 2016-05-03 17:32 UTC (permalink / raw) To: Tony Lindgren, J.D. Schroeder Cc: linux-kernel, bcousson, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel, jay.schroeder, Matthijs van Duin On 03/05/16 19:43, Tony Lindgren wrote: > * J.D. Schroeder <Linux.HWI@garmin.com> [160503 06:32]: >> On 05/03/2016 03:16 AM, Tero Kristo wrote: >>> On 02/05/16 20:12, J.D. Schroeder wrote: >>>> From: "J.D. Schroeder" <jay.schroeder@garmin.com> >>>> >>>> This commit updates the OSC_32K_CLK (secure_32k_clk_src_ck) frequency >>>> from the precise 32kHz frequency (i.e., 32.768 kHz) to a more >>>> accurate frequency of ~34.6 kHz. Actual measured frequencies of the >>>> clock vary from processor to processor anywhere from 34.4 kHz up to >>>> 34.8 kHz. Note that the ~34 kHz frequency clock is generated >>>> internally by the processor, not an input to the processor. This >>>> change makes it more clear that the consumer of the >>>> secure_32k_clk_src_ck will not get a precise 32.768 kHz frequency >>>> output. >>>> >>>> Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com> >>>> Reviewed-by: Trenton Andres <trenton.andres@garmin.com> >>>> --- >>>> arch/arm/boot/dts/dra7xx-clocks.dtsi | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/arch/arm/boot/dts/dra7xx-clocks.dtsi >>>> b/arch/arm/boot/dts/dra7xx-clocks.dtsi >>>> index 3f0c61d..f7ec976 100644 >>>> --- a/arch/arm/boot/dts/dra7xx-clocks.dtsi >>>> +++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi >>>> @@ -95,7 +95,7 @@ >>>> secure_32k_clk_src_ck: secure_32k_clk_src_ck { >>>> #clock-cells = <0>; >>>> compatible = "fixed-clock"; >>>> - clock-frequency = <32768>; >>>> + clock-frequency = <34600>; /* approximate frequency */ >>>> }; >>>> >>>> sys_clk32_crystal_ck: sys_clk32_crystal_ck { >>>> >>> >>> I still don't agree with this patch. The actual frequency can drift much more, >>> you are just seeing this number at your setup. >> >> Yes, it can drift significantly from processor to processor. Do you agree that >> this frequency is closer to what can be expected than 32768 Hz? >> >> Like I said, I would have renamed the clock also but I opted to go the less >> obtrusive route while still helping others that might think they can >> reasonably use this clock in their design as a 32768 Hz clock source. Perhaps >> my comment and selection of the approximate frequency is not the best (I'm >> open for suggestions). However, I do think this change is an improvement and >> clarifying change to what is currently present in the clock description. > > Does a fixed divider calculation of input * (32768 / 0x27e6) make sense > here too as pointed out earlier by Matthijs for the ti81xx? Not really, the hardware setup for this clock is not like that. Also, as it is security related, this is kind of sensitive area to discuss publicly. Personally I would not recommend using this clock for any timing sensitive applications. May I ask why you are interested in the exact clock rate of this clock anyway? -Tero > > Regards, > > Tony > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency 2016-05-03 17:32 ` Tero Kristo @ 2016-05-03 17:49 ` J.D. Schroeder 2016-05-03 18:08 ` Tony Lindgren 2016-05-03 18:08 ` Tero Kristo 2016-05-04 14:09 ` Matthijs van Duin 1 sibling, 2 replies; 29+ messages in thread From: J.D. Schroeder @ 2016-05-03 17:49 UTC (permalink / raw) To: Tero Kristo, Tony Lindgren Cc: linux-kernel, bcousson, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel, jay.schroeder, Matthijs van Duin On 05/03/2016 12:32 PM, Tero Kristo wrote: > Personally I would not recommend using this clock for any timing sensitive > applications. May I ask why you are interested in the exact clock rate of this > clock anyway? I'm not interested in using this clock and I'm not sure how anyone would use this clock outside of the processor. See the inline comment that is part of the change and the commit message for the change. There is no hint in my change that this is an exact clock rate. It is a clarifying change to help others avoid using this clock as a 32 kHz clock (which the current clock name and frequency imply) and it more accurately represents the actual hardware behavior. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency 2016-05-03 17:49 ` J.D. Schroeder @ 2016-05-03 18:08 ` Tony Lindgren 2016-05-03 18:08 ` Tero Kristo 1 sibling, 0 replies; 29+ messages in thread From: Tony Lindgren @ 2016-05-03 18:08 UTC (permalink / raw) To: J.D. Schroeder Cc: Tero Kristo, linux-kernel, bcousson, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel, jay.schroeder, Matthijs van Duin * J.D. Schroeder <Linux.HWI@garmin.com> [160503 10:50]: > On 05/03/2016 12:32 PM, Tero Kristo wrote: > > Personally I would not recommend using this clock for any timing sensitive > > applications. May I ask why you are interested in the exact clock rate of this > > clock anyway? > > I'm not interested in using this clock and I'm not sure how anyone would use > this clock outside of the processor. See the inline comment that is part of > the change and the commit message for the change. There is no hint in my > change that this is an exact clock rate. It is a clarifying change to help > others avoid using this clock as a 32 kHz clock (which the current clock name > and frequency imply) and it more accurately represents the actual hardware > behavior. Well if it's inaccurate, how about we just add comments to the clock? Tony ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency 2016-05-03 17:49 ` J.D. Schroeder 2016-05-03 18:08 ` Tony Lindgren @ 2016-05-03 18:08 ` Tero Kristo 2016-05-03 23:17 ` Nishanth Menon 1 sibling, 1 reply; 29+ messages in thread From: Tero Kristo @ 2016-05-03 18:08 UTC (permalink / raw) To: J.D. Schroeder, Tony Lindgren Cc: linux-kernel, bcousson, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel, jay.schroeder, Matthijs van Duin On 03/05/16 20:49, J.D. Schroeder wrote: > On 05/03/2016 12:32 PM, Tero Kristo wrote: >> Personally I would not recommend using this clock for any timing sensitive >> applications. May I ask why you are interested in the exact clock rate of this >> clock anyway? > > I'm not interested in using this clock and I'm not sure how anyone would use > this clock outside of the processor. See the inline comment that is part of > the change and the commit message for the change. There is no hint in my > change that this is an exact clock rate. It is a clarifying change to help > others avoid using this clock as a 32 kHz clock (which the current clock name > and frequency imply) and it more accurately represents the actual hardware > behavior. > Imo, if you want to clarify things up, the whole secure_32k_ck should be removed from linux kernel. -Tero ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency 2016-05-03 18:08 ` Tero Kristo @ 2016-05-03 23:17 ` Nishanth Menon 0 siblings, 0 replies; 29+ messages in thread From: Nishanth Menon @ 2016-05-03 23:17 UTC (permalink / raw) To: Tero Kristo, J.D. Schroeder, Tony Lindgren Cc: linux-kernel, bcousson, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel, jay.schroeder, Matthijs van Duin On 05/03/2016 01:08 PM, Tero Kristo wrote: > On 03/05/16 20:49, J.D. Schroeder wrote: >> On 05/03/2016 12:32 PM, Tero Kristo wrote: >>> Personally I would not recommend using this clock for any timing sensitive >>> applications. May I ask why you are interested in the exact clock rate of this >>> clock anyway? >> >> I'm not interested in using this clock and I'm not sure how anyone would use >> this clock outside of the processor. See the inline comment that is part of >> the change and the commit message for the change. There is no hint in my >> change that this is an exact clock rate. It is a clarifying change to help >> others avoid using this clock as a 32 kHz clock (which the current clock name >> and frequency imply) and it more accurately represents the actual hardware >> behavior. >> > > Imo, if you want to clarify things up, the whole secure_32k_ck should be > removed from linux kernel. This is actually the RC oscillator clock[1] which also happens to source the secure_32k_clk. Jay is right that this is not an accurate 32k clock, however the actual range of this internal clock source is pretty wide (I am trying to get that information into public domain TRM - but that will take some time - since this patch just started the internal thread on the topic). since it is infact an accurate clock source from inside the SoC, how do we model that (a clock with a frequency range with nominal frequency expected to be around 32k - but not exactly 32k?). I think having a rename makes sense and modelling it as an in-accurate clock source is probably the need. [1] Search for "On-die 32K RC Osc" in http://www.ti.com/lit/pdf/spruhz6 -- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency 2016-05-03 17:32 ` Tero Kristo 2016-05-03 17:49 ` J.D. Schroeder @ 2016-05-04 14:09 ` Matthijs van Duin 1 sibling, 0 replies; 29+ messages in thread From: Matthijs van Duin @ 2016-05-04 14:09 UTC (permalink / raw) To: Tero Kristo Cc: Tony Lindgren, J.D. Schroeder, lkml, bcousson, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, Russell King - ARM Linux, linux-omap, devicetree, linux-arm-kernel, jay.schroeder On 3 May 2016 at 18:43, Tony Lindgren <tony@atomide.com> wrote: > Does a fixed divider calculation of input * (32768 / 27e6) make sense > here too as pointed out earlier by Matthijs for the ti81xx? That was an actual fractional divider, i.e. the output clock would be exactly that ratio of the input clock, which would therefore yield 32768 Hz if the default crystal is used for the main osc. It makes no sense to use it to describe a wobbly RC osc. On 3 May 2016 at 19:32, Tero Kristo <t-kristo@ti.com> wrote: > Also, as it is security related, this is kind of sensitive area to discuss publicly. Don't be silly. The only sense in which this clock is "security related" is because it's used for the secure watchdog, and the reason to use it despite its inaccuracy is completely obvious: an internal rc osc can't be easily manipulated by an external attacker. I see no reason to act all cloak-and-dagger about this. (This seems to be a general theme to conceal "security related" things from public documentation. If I were an HS customer I'd actually be concerned about such behaviour since it would seem to indicate a lack of confidence in one's security architecture.) Matthijs ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 3/3] ARM: dts: dra7: fix clock node definition to avoid build warning 2016-05-02 17:12 ` [PATCH v2 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups J.D. Schroeder 2016-05-02 17:12 ` [PATCH v2 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div J.D. Schroeder 2016-05-02 17:12 ` [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency J.D. Schroeder @ 2016-05-02 17:12 ` J.D. Schroeder 2016-05-03 4:20 ` [PATCH v2 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups Lokesh Vutla 3 siblings, 0 replies; 29+ messages in thread From: J.D. Schroeder @ 2016-05-02 17:12 UTC (permalink / raw) To: linux-kernel, t-kristo, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: jay.schroeder From: "J.D. Schroeder" <jay.schroeder@garmin.com> This patch fixes the following warning for the DRA7 clock node: Warning (unit_address_vs_reg): Node /ocp/l4@4a000000/scm@2000/scm_conf@0/clocks/sys_32k_ck has a reg or ranges property, but no unit name Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com> --- arch/arm/boot/dts/dra7xx-clocks.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/dra7xx-clocks.dtsi b/arch/arm/boot/dts/dra7xx-clocks.dtsi index f7ec976..34cda4b 100644 --- a/arch/arm/boot/dts/dra7xx-clocks.dtsi +++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi @@ -2186,7 +2186,7 @@ reg = <0x0558>; }; - sys_32k_ck: sys_32k_ck { + sys_32k_ck: sys_32k_ck@6c4 { #clock-cells = <0>; compatible = "ti,mux-clock"; clocks = <&sys_clk32_crystal_ck>, <&sys_clk32_pseudo_ck>, <&sys_clk32_pseudo_ck>, <&sys_clk32_pseudo_ck>; -- 1.9.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups 2016-05-02 17:12 ` [PATCH v2 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups J.D. Schroeder ` (2 preceding siblings ...) 2016-05-02 17:12 ` [PATCH v2 3/3] ARM: dts: dra7: fix clock node definition to avoid build warning J.D. Schroeder @ 2016-05-03 4:20 ` Lokesh Vutla 3 siblings, 0 replies; 29+ messages in thread From: Lokesh Vutla @ 2016-05-03 4:20 UTC (permalink / raw) To: J.D. Schroeder, linux-kernel, t-kristo, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: jay.schroeder On Monday 02 May 2016 10:42 PM, J.D. Schroeder wrote: > This series of patches fixes several discrepancies between the > AM57/DRA7 clock tree description and the actual hardware behavior and > frequencies. With these changes a more complete picture of the clock > tree is represented for a few of the clocks and their resulting > frequencies. Tested on my DRA74-evm. Tested-by: Lokesh Vutla <lokeshvutla@ti.com> Thanks and regards, Lokesh > > v2 Changes: > * Rebased on linux-next as requested by Tony Lindgren > * Eliminated previous patch 2 as another change fixing the same thing > was merged in eea08802f586acd6aef377d1b4a541821013cc0b > * Added to the commit message in patch 2 to clarify the source of the > clock being internal to the processor > * Added a new patch 3 to fix a new warning introduced by > eea08802f586acd6aef377d1b4a541821013cc0b > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 2/3] ARM: DRA7x: dts: Fix the 32kHz clock calculation 2016-04-26 17:54 [PATCH 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups J.D. Schroeder 2016-04-26 17:54 ` [PATCH 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div J.D. Schroeder @ 2016-04-26 17:54 ` J.D. Schroeder 2016-04-27 11:40 ` Tero Kristo 2016-04-26 17:54 ` [PATCH 3/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency J.D. Schroeder 2016-04-26 18:13 ` [PATCH 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups Tony Lindgren 3 siblings, 1 reply; 29+ messages in thread From: J.D. Schroeder @ 2016-04-26 17:54 UTC (permalink / raw) To: linux-kernel, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: J.D. Schroeder From: "J.D. Schroeder" <jay.schroeder@garmin.com> This commit fixes the 32kHz clock (sys_32k_ck) calculation to be correctly based on the SYS_CLK1 (sys_clkin1) frequency. Based on the TRM CTRL_CORE_BOOTSTRAP[9:8] SPEEDSELECT, set by the SYSBOOT[9:8] board jumpers according to the SYS_CLK1 frequency, the frequency of the 32kHz FUNC_32K_CLK is set to SYS_CLK1/610. The following sys_32k_ck frequencies get used for different SYS_CLK1 frequencies: 0b00: Unknown -> 32768 Hz crystal from CLKIN_32K pin 0b01: 20 MHz -> 32787 Hz clock (SYS_CLK1/610) 0b10: 27 MHz -> 44262 Hz clock (SYS_CLK1/610) 0b11: 19.2 MHz -> 31475 Hz clock (SYS_CLK1/610) Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com> Reviewed-by: Ben McCauley <ben.mccauley@garmin.com> --- arch/arm/boot/dts/dra7xx-clocks.dtsi | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/arch/arm/boot/dts/dra7xx-clocks.dtsi b/arch/arm/boot/dts/dra7xx-clocks.dtsi index 9d1a583..a514fc3 100644 --- a/arch/arm/boot/dts/dra7xx-clocks.dtsi +++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi @@ -98,12 +98,6 @@ clock-frequency = <32768>; }; - sys_32k_ck: sys_32k_ck { - #clock-cells = <0>; - compatible = "fixed-clock"; - clock-frequency = <32768>; - }; - virt_12000000_ck: virt_12000000_ck { #clock-cells = <0>; compatible = "fixed-clock"; @@ -2177,4 +2171,26 @@ ti,bit-shift = <22>; reg = <0x0558>; }; + + sys_32kin: sys_32kin { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <32768>; + }; + + sys_clkin1_32k_div: sys_clkin1_32k_div { + #clock-cells = <0>; + compatible = "fixed-factor-clock"; + clocks = <&sys_clkin1>; + clock-mult = <1>; + clock-div = <610>; + }; + + sys_32k_ck: sys_32k_ck { + #clock-cells = <0>; + compatible = "ti,mux-clock"; + clocks = <&sys_32kin>, <&sys_clkin1_32k_div>, <&sys_clkin1_32k_div>, <&sys_clkin1_32k_div>; + ti,bit-shift = <8>; + reg = <0x06c4>; + }; }; -- 1.9.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 2/3] ARM: DRA7x: dts: Fix the 32kHz clock calculation 2016-04-26 17:54 ` [PATCH 2/3] ARM: DRA7x: dts: Fix the 32kHz clock calculation J.D. Schroeder @ 2016-04-27 11:40 ` Tero Kristo 2016-04-27 14:06 ` J.D. Schroeder 0 siblings, 1 reply; 29+ messages in thread From: Tero Kristo @ 2016-04-27 11:40 UTC (permalink / raw) To: J.D. Schroeder, linux-kernel, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: J.D. Schroeder, J, Keerthy On 26/04/16 20:54, J.D. Schroeder wrote: > From: "J.D. Schroeder" <jay.schroeder@garmin.com> > > This commit fixes the 32kHz clock (sys_32k_ck) calculation to be > correctly based on the SYS_CLK1 (sys_clkin1) frequency. Based on the > TRM CTRL_CORE_BOOTSTRAP[9:8] SPEEDSELECT, set by the SYSBOOT[9:8] > board jumpers according to the SYS_CLK1 frequency, the frequency of > the 32kHz FUNC_32K_CLK is set to SYS_CLK1/610. The following > sys_32k_ck frequencies get used for different SYS_CLK1 frequencies: > 0b00: Unknown -> 32768 Hz crystal from CLKIN_32K pin > 0b01: 20 MHz -> 32787 Hz clock (SYS_CLK1/610) > 0b10: 27 MHz -> 44262 Hz clock (SYS_CLK1/610) > 0b11: 19.2 MHz -> 31475 Hz clock (SYS_CLK1/610) > > Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com> > Reviewed-by: Ben McCauley <ben.mccauley@garmin.com> A patch doing the same thing is already in mainline, see: commit eea08802f586acd6aef377d1b4a541821013cc0b Author: Keerthy <j-keerthy@ti.com> Date: Mon Apr 4 11:07:15 2016 +0530 ARM: dts: dra7: Correct clock tree for sys_32k_ck So, this one can be ignored. -Tero > --- > arch/arm/boot/dts/dra7xx-clocks.dtsi | 28 ++++++++++++++++++++++------ > 1 file changed, 22 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/boot/dts/dra7xx-clocks.dtsi b/arch/arm/boot/dts/dra7xx-clocks.dtsi > index 9d1a583..a514fc3 100644 > --- a/arch/arm/boot/dts/dra7xx-clocks.dtsi > +++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi > @@ -98,12 +98,6 @@ > clock-frequency = <32768>; > }; > > - sys_32k_ck: sys_32k_ck { > - #clock-cells = <0>; > - compatible = "fixed-clock"; > - clock-frequency = <32768>; > - }; > - > virt_12000000_ck: virt_12000000_ck { > #clock-cells = <0>; > compatible = "fixed-clock"; > @@ -2177,4 +2171,26 @@ > ti,bit-shift = <22>; > reg = <0x0558>; > }; > + > + sys_32kin: sys_32kin { > + #clock-cells = <0>; > + compatible = "fixed-clock"; > + clock-frequency = <32768>; > + }; > + > + sys_clkin1_32k_div: sys_clkin1_32k_div { > + #clock-cells = <0>; > + compatible = "fixed-factor-clock"; > + clocks = <&sys_clkin1>; > + clock-mult = <1>; > + clock-div = <610>; > + }; > + > + sys_32k_ck: sys_32k_ck { > + #clock-cells = <0>; > + compatible = "ti,mux-clock"; > + clocks = <&sys_32kin>, <&sys_clkin1_32k_div>, <&sys_clkin1_32k_div>, <&sys_clkin1_32k_div>; > + ti,bit-shift = <8>; > + reg = <0x06c4>; > + }; > }; > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/3] ARM: DRA7x: dts: Fix the 32kHz clock calculation 2016-04-27 11:40 ` Tero Kristo @ 2016-04-27 14:06 ` J.D. Schroeder 2016-04-27 19:47 ` Tero Kristo 0 siblings, 1 reply; 29+ messages in thread From: J.D. Schroeder @ 2016-04-27 14:06 UTC (permalink / raw) To: Tero Kristo, linux-kernel, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: J.D. Schroeder, J, Keerthy On 04/27/2016 06:40 AM, Tero Kristo wrote: > On 26/04/16 20:54, J.D. Schroeder wrote: >> This commit fixes the 32kHz clock (sys_32k_ck) calculation to be >> correctly based on the SYS_CLK1 (sys_clkin1) frequency. Based on the >> TRM CTRL_CORE_BOOTSTRAP[9:8] SPEEDSELECT, set by the SYSBOOT[9:8] >> board jumpers according to the SYS_CLK1 frequency, the frequency of >> the 32kHz FUNC_32K_CLK is set to SYS_CLK1/610. The following >> sys_32k_ck frequencies get used for different SYS_CLK1 frequencies: >> 0b00: Unknown -> 32768 Hz crystal from CLKIN_32K pin >> 0b01: 20 MHz -> 32787 Hz clock (SYS_CLK1/610) >> 0b10: 27 MHz -> 44262 Hz clock (SYS_CLK1/610) >> 0b11: 19.2 MHz -> 31475 Hz clock (SYS_CLK1/610) > > A patch doing the same thing is already in mainline, see: > > commit eea08802f586acd6aef377d1b4a541821013cc0b > Author: Keerthy <j-keerthy@ti.com> > Date: Mon Apr 4 11:07:15 2016 +0530 > > ARM: dts: dra7: Correct clock tree for sys_32k_ck > > So, this one can be ignored. My change had no issue when applying to the tip of master and I'm not seeing that SHA1 in mainline. Are you saying it is in another repo ready to be sent to mainline for the next release cycle? ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/3] ARM: DRA7x: dts: Fix the 32kHz clock calculation 2016-04-27 14:06 ` J.D. Schroeder @ 2016-04-27 19:47 ` Tero Kristo 2016-04-27 20:13 ` J.D. Schroeder 0 siblings, 1 reply; 29+ messages in thread From: Tero Kristo @ 2016-04-27 19:47 UTC (permalink / raw) To: J.D. Schroeder, linux-kernel, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: J.D. Schroeder, J, Keerthy On 27/04/16 17:06, J.D. Schroeder wrote: > On 04/27/2016 06:40 AM, Tero Kristo wrote: >> On 26/04/16 20:54, J.D. Schroeder wrote: >>> This commit fixes the 32kHz clock (sys_32k_ck) calculation to be >>> correctly based on the SYS_CLK1 (sys_clkin1) frequency. Based on the >>> TRM CTRL_CORE_BOOTSTRAP[9:8] SPEEDSELECT, set by the SYSBOOT[9:8] >>> board jumpers according to the SYS_CLK1 frequency, the frequency of >>> the 32kHz FUNC_32K_CLK is set to SYS_CLK1/610. The following >>> sys_32k_ck frequencies get used for different SYS_CLK1 frequencies: >>> 0b00: Unknown -> 32768 Hz crystal from CLKIN_32K pin >>> 0b01: 20 MHz -> 32787 Hz clock (SYS_CLK1/610) >>> 0b10: 27 MHz -> 44262 Hz clock (SYS_CLK1/610) >>> 0b11: 19.2 MHz -> 31475 Hz clock (SYS_CLK1/610) >> >> A patch doing the same thing is already in mainline, see: >> >> commit eea08802f586acd6aef377d1b4a541821013cc0b >> Author: Keerthy <j-keerthy@ti.com> >> Date: Mon Apr 4 11:07:15 2016 +0530 >> >> ARM: dts: dra7: Correct clock tree for sys_32k_ck >> >> So, this one can be ignored. > > My change had no issue when applying to the tip of master and I'm not seeing > that SHA1 in mainline. Are you saying it is in another repo ready to be sent > to mainline for the next release cycle? > The patch is merged already in 4.6-rc3. Which repo / version are you using as baseline? -Tero ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/3] ARM: DRA7x: dts: Fix the 32kHz clock calculation 2016-04-27 19:47 ` Tero Kristo @ 2016-04-27 20:13 ` J.D. Schroeder 0 siblings, 0 replies; 29+ messages in thread From: J.D. Schroeder @ 2016-04-27 20:13 UTC (permalink / raw) To: Tero Kristo, linux-kernel, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: J.D. Schroeder, J, Keerthy On 04/27/2016 02:47 PM, Tero Kristo wrote: > The patch is merged already in 4.6-rc3. > > Which repo / version are you using as baseline? I'm using git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git. I can see eea08802f586acd6aef377d1b4a541821013cc0b as of now. However, I don't think it was included in -rc3 or even -rc5 which is what I was basing my changes on. It looks like it got merged in late last night in Linus tree. No big deal. The change is nearly identical to my solution so no need to pursue mine. Please disregard this patch. Thanks, JD ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency 2016-04-26 17:54 [PATCH 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups J.D. Schroeder 2016-04-26 17:54 ` [PATCH 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div J.D. Schroeder 2016-04-26 17:54 ` [PATCH 2/3] ARM: DRA7x: dts: Fix the 32kHz clock calculation J.D. Schroeder @ 2016-04-26 17:54 ` J.D. Schroeder 2016-04-27 11:49 ` Tero Kristo 2016-04-26 18:13 ` [PATCH 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups Tony Lindgren 3 siblings, 1 reply; 29+ messages in thread From: J.D. Schroeder @ 2016-04-26 17:54 UTC (permalink / raw) To: linux-kernel, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: J.D. Schroeder From: "J.D. Schroeder" <jay.schroeder@garmin.com> This commit updates the OSC_32K_CLK (secure_32k_clk_src_ck) frequency from the precise 32kHz frequency (i.e., 32.768 kHz) to the more accurate frequency of ~34.6 kHz. Actual measured frequencies of the clock vary from board to board anywhere from 34.4 kHz up to 34.8 kHz. Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com> Reviewed-by: Trenton Andres <trenton.andres@garmin.com> --- arch/arm/boot/dts/dra7xx-clocks.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/dra7xx-clocks.dtsi b/arch/arm/boot/dts/dra7xx-clocks.dtsi index a514fc3..4501140 100644 --- a/arch/arm/boot/dts/dra7xx-clocks.dtsi +++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi @@ -95,7 +95,7 @@ secure_32k_clk_src_ck: secure_32k_clk_src_ck { #clock-cells = <0>; compatible = "fixed-clock"; - clock-frequency = <32768>; + clock-frequency = <34600>; /* approximate frequency */ }; virt_12000000_ck: virt_12000000_ck { -- 1.9.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency 2016-04-26 17:54 ` [PATCH 3/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency J.D. Schroeder @ 2016-04-27 11:49 ` Tero Kristo 2016-04-27 14:20 ` J.D. Schroeder 0 siblings, 1 reply; 29+ messages in thread From: Tero Kristo @ 2016-04-27 11:49 UTC (permalink / raw) To: J.D. Schroeder, linux-kernel, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: J.D. Schroeder On 26/04/16 20:54, J.D. Schroeder wrote: > From: "J.D. Schroeder" <jay.schroeder@garmin.com> > > This commit updates the OSC_32K_CLK (secure_32k_clk_src_ck) frequency > from the precise 32kHz frequency (i.e., 32.768 kHz) to the more > accurate frequency of ~34.6 kHz. Actual measured frequencies of the > clock vary from board to board anywhere from 34.4 kHz up to 34.8 kHz. Uhm, if you have a board specific, accurate value for this clock, you should update it in the board file itself. This definition is going to be used across all the DRA7 / AM57xx boards, which can very likely have different crystal accuracies. So, NAK. -Tero > > Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com> > Reviewed-by: Trenton Andres <trenton.andres@garmin.com> > --- > arch/arm/boot/dts/dra7xx-clocks.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/dra7xx-clocks.dtsi b/arch/arm/boot/dts/dra7xx-clocks.dtsi > index a514fc3..4501140 100644 > --- a/arch/arm/boot/dts/dra7xx-clocks.dtsi > +++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi > @@ -95,7 +95,7 @@ > secure_32k_clk_src_ck: secure_32k_clk_src_ck { > #clock-cells = <0>; > compatible = "fixed-clock"; > - clock-frequency = <32768>; > + clock-frequency = <34600>; /* approximate frequency */ > }; > > virt_12000000_ck: virt_12000000_ck { > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency 2016-04-27 11:49 ` Tero Kristo @ 2016-04-27 14:20 ` J.D. Schroeder 0 siblings, 0 replies; 29+ messages in thread From: J.D. Schroeder @ 2016-04-27 14:20 UTC (permalink / raw) To: Tero Kristo, linux-kernel, bcousson, tony, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Cc: J.D. Schroeder On 04/27/2016 06:49 AM, Tero Kristo wrote: > On 26/04/16 20:54, J.D. Schroeder wrote: >> From: "J.D. Schroeder" <jay.schroeder@garmin.com> >> >> This commit updates the OSC_32K_CLK (secure_32k_clk_src_ck) frequency >> from the precise 32kHz frequency (i.e., 32.768 kHz) to the more >> accurate frequency of ~34.6 kHz. Actual measured frequencies of the >> clock vary from board to board anywhere from 34.4 kHz up to 34.8 kHz. > > Uhm, if you have a board specific, accurate value for this clock, you should > update it in the board file itself. This definition is going to be used across > all the DRA7 / AM57xx boards, which can very likely have different crystal > accuracies. > > So, NAK. The source of this clock is internal to the processor and not specific to how the processor is configured or what clocks are coming in. The approximate frequency of 34.4-34.8 kHz is generated internal to the processor through some type of oscillator, not external. The problem is that the clock tree gives the impression that this is a 32.768 kHz clock source, when in fact it is *not* that. Both the name and the frequency are misleading. My change is an attempt to clarify the actual behavior of the clock and keep someone else from using the clock as a true 32.768 kHz clock when it is more than 5% off that particular frequency. I would even consider changing the name of the clock as that too is misleading, but opted not to since that would be more disruptive. If you are seeing 32.768 kHz come out of this clock source then I must have an issue with my silicon and we can discuss off line. However, if you configure this as one of the clock out sources and see something in the range of ~34 kHz, I still think the change is a valid change as it clarifies the true behavior of the hardware. Am I missing something? ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups 2016-04-26 17:54 [PATCH 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups J.D. Schroeder ` (2 preceding siblings ...) 2016-04-26 17:54 ` [PATCH 3/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency J.D. Schroeder @ 2016-04-26 18:13 ` Tony Lindgren 2016-04-26 19:18 ` J.D. Schroeder 3 siblings, 1 reply; 29+ messages in thread From: Tony Lindgren @ 2016-04-26 18:13 UTC (permalink / raw) To: J.D. Schroeder, Tero Kristo Cc: linux-kernel, bcousson, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel Hi, * J.D. Schroeder <Linux.HWI@garmin.com> [160426 10:57]: > This series of patches fixes several discrepancies between the > AM57/DRA7 clock tree description and the actual hardware behavior and > frequencies. With these changes a more complete picture of the clock > tree is represented for a few of the clocks and their resulting > frequencies. Are any of these needed for the v4.6-rc cycle? Adding Tero to Cc so he can review the changes. Regards, Tony ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups 2016-04-26 18:13 ` [PATCH 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups Tony Lindgren @ 2016-04-26 19:18 ` J.D. Schroeder 0 siblings, 0 replies; 29+ messages in thread From: J.D. Schroeder @ 2016-04-26 19:18 UTC (permalink / raw) To: Tony Lindgren, Tero Kristo Cc: linux-kernel, bcousson, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, linux, linux-omap, devicetree, linux-arm-kernel, J.D. Schroeder On 04/26/2016 01:13 PM, Tony Lindgren wrote: > Are any of these needed for the v4.6-rc cycle? I understand that these are arriving a little late especially if we don't get to rc7. However, it would be great if these could get in the v4.6 kernel release. They shouldn't be too risky to anyone, but I understand if the logistics prevent that from happening. Thanks, JD ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2016-05-04 14:10 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-04-26 17:54 [PATCH 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups J.D. Schroeder 2016-04-26 17:54 ` [PATCH 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div J.D. Schroeder 2016-04-27 11:21 ` Tero Kristo 2016-04-27 16:36 ` Tony Lindgren 2016-04-27 17:16 ` Tony Lindgren 2016-05-02 17:12 ` [PATCH v2 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups J.D. Schroeder 2016-05-02 17:12 ` [PATCH v2 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div J.D. Schroeder 2016-05-02 17:12 ` [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency J.D. Schroeder 2016-05-03 8:16 ` Tero Kristo 2016-05-03 13:31 ` J.D. Schroeder 2016-05-03 16:43 ` Tony Lindgren 2016-05-03 17:32 ` Tero Kristo 2016-05-03 17:49 ` J.D. Schroeder 2016-05-03 18:08 ` Tony Lindgren 2016-05-03 18:08 ` Tero Kristo 2016-05-03 23:17 ` Nishanth Menon 2016-05-04 14:09 ` Matthijs van Duin 2016-05-02 17:12 ` [PATCH v2 3/3] ARM: dts: dra7: fix clock node definition to avoid build warning J.D. Schroeder 2016-05-03 4:20 ` [PATCH v2 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups Lokesh Vutla 2016-04-26 17:54 ` [PATCH 2/3] ARM: DRA7x: dts: Fix the 32kHz clock calculation J.D. Schroeder 2016-04-27 11:40 ` Tero Kristo 2016-04-27 14:06 ` J.D. Schroeder 2016-04-27 19:47 ` Tero Kristo 2016-04-27 20:13 ` J.D. Schroeder 2016-04-26 17:54 ` [PATCH 3/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency J.D. Schroeder 2016-04-27 11:49 ` Tero Kristo 2016-04-27 14:20 ` J.D. Schroeder 2016-04-26 18:13 ` [PATCH 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups Tony Lindgren 2016-04-26 19:18 ` J.D. Schroeder
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).