Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency

4 messages, 2 authors, 2014-06-30 · open the first message on its own page

Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency

From: Maxime Ripard <hidden>
Date: 2014-06-30 14:01:46 

On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote:
quoted
quoted
quoted
quoted
-             rate = of_get_property(cn, "clock-frequency", &len);
-             if (!rate || len != 4) {
-                     pr_err("%s missing clock-frequency property\n",
-                             cn->full_name);
+             clk = of_clk_get(cn, 0);
+             if (!IS_ERR(clk))
+                     rate = clk_get_rate(clk);
We need the max frequency as it will be used to weight the different
CPUs capacity. How do you ensure that the current clock rate is the
max one ?
Hmm, the clock-frequency attribute in the ePAPR is defined at the
current CPU frequency, not the max one.
What means current frequency in device tree when DVFS is involved ?
The ePAPR states that clock-frequency is supposed to be "the current
clock speed of the CPU in Hertz". It's exactly what my patch add.

Now, you're right, DVFS would be an issue here with clock-frequency,
but this patch actually makes it easier to deal with, since you only
get a reference to a clock, and you can get its rate at any given
time.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency

From: Vincent Guittot <hidden>
Date: 2014-06-30 14:48:35 

On 30 June 2014 16:01, Maxime Ripard [off-list ref] wrote:
On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote:
quoted
quoted
quoted
quoted
quoted
-             rate = of_get_property(cn, "clock-frequency", &len);
-             if (!rate || len != 4) {
-                     pr_err("%s missing clock-frequency property\n",
-                             cn->full_name);
+             clk = of_clk_get(cn, 0);
+             if (!IS_ERR(clk))
+                     rate = clk_get_rate(clk);
We need the max frequency as it will be used to weight the different
CPUs capacity. How do you ensure that the current clock rate is the
max one ?
Hmm, the clock-frequency attribute in the ePAPR is defined at the
current CPU frequency, not the max one.
What means current frequency in device tree when DVFS is involved ?
The ePAPR states that clock-frequency is supposed to be "the current
clock speed of the CPU in Hertz". It's exactly what my patch add.

Now, you're right, DVFS would be an issue here with clock-frequency,
but this patch actually makes it easier to deal with, since you only
get a reference to a clock, and you can get its rate at any given
time.
and what about using clk_round_rate(clk, ULONG_MAX) ?

We will not be dependent of when we parse DT

Vincent


Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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 1/2] ARM: topology: Use a clock if possible to get the CPU frequency

From: Maxime Ripard <hidden>
Date: 2014-06-30 14:58:47 

On Mon, Jun 30, 2014 at 04:48:35PM +0200, Vincent Guittot wrote:
On 30 June 2014 16:01, Maxime Ripard [off-list ref] wrote:
quoted
On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote:
quoted
quoted
quoted
quoted
quoted
-             rate = of_get_property(cn, "clock-frequency", &len);
-             if (!rate || len != 4) {
-                     pr_err("%s missing clock-frequency property\n",
-                             cn->full_name);
+             clk = of_clk_get(cn, 0);
+             if (!IS_ERR(clk))
+                     rate = clk_get_rate(clk);
We need the max frequency as it will be used to weight the different
CPUs capacity. How do you ensure that the current clock rate is the
max one ?
Hmm, the clock-frequency attribute in the ePAPR is defined at the
current CPU frequency, not the max one.
What means current frequency in device tree when DVFS is involved ?
The ePAPR states that clock-frequency is supposed to be "the current
clock speed of the CPU in Hertz". It's exactly what my patch add.

Now, you're right, DVFS would be an issue here with clock-frequency,
but this patch actually makes it easier to deal with, since you only
get a reference to a clock, and you can get its rate at any given
time.
and what about using clk_round_rate(clk, ULONG_MAX) ?
Well, the clock itself might generate a frequency higher that what the
CPU can run at, so I'm not sure it's a valid assumption.

We will not be dependent of when we parse DT
You lost me there. clk_round_rate and clk_get_rate are available at
the same time in the boot process, aren't they?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency

From: Vincent Guittot <hidden>
Date: 2014-06-30 15:06:16 

On 30 June 2014 16:58, Maxime Ripard [off-list ref] wrote:
On Mon, Jun 30, 2014 at 04:48:35PM +0200, Vincent Guittot wrote:
quoted
On 30 June 2014 16:01, Maxime Ripard [off-list ref] wrote:
quoted
On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote:
quoted
quoted
quoted
quoted
quoted
-             rate = of_get_property(cn, "clock-frequency", &len);
-             if (!rate || len != 4) {
-                     pr_err("%s missing clock-frequency property\n",
-                             cn->full_name);
+             clk = of_clk_get(cn, 0);
+             if (!IS_ERR(clk))
+                     rate = clk_get_rate(clk);
We need the max frequency as it will be used to weight the different
CPUs capacity. How do you ensure that the current clock rate is the
max one ?
Hmm, the clock-frequency attribute in the ePAPR is defined at the
current CPU frequency, not the max one.
What means current frequency in device tree when DVFS is involved ?
The ePAPR states that clock-frequency is supposed to be "the current
clock speed of the CPU in Hertz". It's exactly what my patch add.

Now, you're right, DVFS would be an issue here with clock-frequency,
but this patch actually makes it easier to deal with, since you only
get a reference to a clock, and you can get its rate at any given
time.
and what about using clk_round_rate(clk, ULONG_MAX) ?
Well, the clock itself might generate a frequency higher that what the
CPU can run at, so I'm not sure it's a valid assumption.
yes, you're right

quoted
We will not be dependent of when we parse DT
You lost me there. clk_round_rate and clk_get_rate are available at
the same time in the boot process, aren't they?
yes, but  clk_round_rate(clk, ULONG_MAX) will return the max frequency
and not the current one. But as you mentioned, it doesn't ensure that
it's a possible frequency for the core

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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




    
  

  
    

      

        Keyboard shortcuts
      

      

        
          
hback out one level

          
jnext message in thread

          
kprevious message in thread

          
ldrill in

          
Escclose help / fold thread tree

          
?toggle this help