Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1393162 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency Tero Kristo <t-kristo@ti.com> - 2016-05-03 10:20 +0200
    Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency Tony Lindgren <tony@atomide.com> - 2016-05-03 18:50 +0200
      Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency Tero Kristo <t-kristo@ti.com> - 2016-05-03 19:40 +0200
        Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency Tony Lindgren <tony@atomide.com> - 2016-05-03 20:10 +0200
        Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency Tero Kristo <t-kristo@ti.com> - 2016-05-03 20:20 +0200
          Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency Nishanth Menon <nm@ti.com> - 2016-05-04 01:20 +0200
        Re: [PATCH v2 2/3] ARM: DRA7x: dts: Update the OSC_32K_CLK frequency Matthijs van Duin <matthijsvanduin@gmail.com> - 2016-05-04 16:20 +0200

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

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

[toc] | [next] | [standalone]

#1393605

* 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

[toc] | [prev] | [next] | [standalone]

#1393643

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
>

[toc] | [prev] | [next] | [standalone]

#1393703

* 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

[toc] | [prev] | [next] | [standalone]

#1393719

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

[toc] | [prev] | [next] | [standalone]

#1393859

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

[toc] | [prev] | [next] | [standalone]

#1394331

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web