[PATCH 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups

[PATCH 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups

[J.D. 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.

[PATCH 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div

[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

Re: [PATCH 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div

[Tero Kristo]

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 {
>

Re: [PATCH 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div

[Tony Lindgren]

* 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

Re: [PATCH 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div

[Tony Lindgren]

* Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [160427 09:39]:
> * Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org> [160427 04:22]:
> > On 26/04/16 20:54, J.D. Schroeder wrote:
> > >From: "J.D. Schroeder" <jay.schroeder-UF6BFNFdnjXQT0dZR+AlfA@public.gmane.org>
> > >
> > >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-UF6BFNFdnjXQT0dZR+AlfA@public.gmane.org>
> > >Reviewed-by: Trenton Andres <trenton.andres-UF6BFNFdnjXQT0dZR+AlfA@public.gmane.org>
> > 
> > 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-l0cyMroinI0@public.gmane.org>
> 
> 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
--
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

[PATCH v2 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups

[J.D. 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

[PATCH v2 1/3] DRA7: Fix clock data for gmac_gmii_ref_clk_div

[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 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

[PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency

[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 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

Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency

[Tero Kristo]

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

Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency

[J.D. 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

Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency

[Tony Lindgren]

* 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

Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency

[Tero Kristo]

On 03/05/16 19:43, Tony Lindgren wrote:
> * J.D. Schroeder <Linux.HWI-UF6BFNFdnjXQT0dZR+AlfA@public.gmane.org> [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-UF6BFNFdnjXQT0dZR+AlfA@public.gmane.org>
>>>>
>>>> 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-UF6BFNFdnjXQT0dZR+AlfA@public.gmane.org>
>>>> Reviewed-by: Trenton Andres <trenton.andres-UF6BFNFdnjXQT0dZR+AlfA@public.gmane.org>
>>>> ---
>>>>    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
>

--
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

Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency

[J.D. Schroeder]

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.

--
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

Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency

[Tony Lindgren]

* J.D. Schroeder <Linux.HWI-UF6BFNFdnjXQT0dZR+AlfA@public.gmane.org> [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
--
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

Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency

[Tero Kristo]

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

Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency

[Nishanth Menon]

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

Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency

[Matthijs van Duin]

On 3 May 2016 at 18:43, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 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-l0cyMroinI0@public.gmane.org> 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
--
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

[PATCH v2 3/3] ARM: dts: dra7: fix clock node definition to avoid build warning

[J.D. Schroeder]

From: "J.D. Schroeder" <jay.schroeder-UF6BFNFdnjXQT0dZR+AlfA@public.gmane.org>

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-UF6BFNFdnjXQT0dZR+AlfA@public.gmane.org>
---
 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

--
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

Re: [PATCH v2 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups

[Lokesh Vutla]

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
> 

[PATCH 2/3] ARM: DRA7x: dts: Fix the 32kHz clock calculation

[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

Re: [PATCH 2/3] ARM: DRA7x: dts: Fix the 32kHz clock calculation

[Tero Kristo]

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>;
> +	};
>   };
>

Re: [PATCH 2/3] ARM: DRA7x: dts: Fix the 32kHz clock calculation

[J.D. Schroeder]

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-l0cyMroinI0@public.gmane.org>
> 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?
--
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

Re: [PATCH 2/3] ARM: DRA7x: dts: Fix the 32kHz clock calculation

[Tero Kristo]

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-l0cyMroinI0@public.gmane.org>
>> 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
--
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

Re: [PATCH 2/3] ARM: DRA7x: dts: Fix the 32kHz clock calculation

[J.D. Schroeder]

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

[PATCH 3/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency

[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

Re: [PATCH 3/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency

[Tero Kristo]

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 {
>

Re: [PATCH 3/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency

[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-UF6BFNFdnjXQT0dZR+AlfA@public.gmane.org>
>>
>> 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?


--
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

Re: [PATCH 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups

[Tony Lindgren]

Hi,

* J.D. Schroeder <Linux.HWI-UF6BFNFdnjXQT0dZR+AlfA@public.gmane.org> [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
--
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

Re: [PATCH 0/3] AM57/DRA7 Clock Tree DTSI Fix-ups

[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