* [PATCH v2 0/2] ARM: topology: Allow to set the frequency through a clock @ 2014-06-30 10:12 ` Maxime Ripard 0 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-06-30 10:12 UTC (permalink / raw) To: Russell King, Arnd Bergmann Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Nicolas Pitre, Maxime Ripard Hi, This patch series add the possibility for the topology code to get the CPU frequency through a DT clock handle instead of needing a clock-frequency property. Indeed, this information can be quite redundant if the clock tree defined in the DT is already describing the CPU parent clock. Maxime Changes from v1: - Rebased on top of 3.16 - Reworked a bit the code in the first patch as requested by Rob Herring Maxime Ripard (2): ARM: topology: Use a clock if possible to get the CPU frequency ARM: sunxi: Add clocks node to the CPU nodes arch/arm/boot/dts/sun6i-a31.dtsi | 4 ++++ arch/arm/boot/dts/sun7i-a20.dtsi | 2 ++ arch/arm/kernel/topology.c | 25 ++++++++++++++++++------- 3 files changed, 24 insertions(+), 7 deletions(-) -- 2.0.0 -- 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 0/2] ARM: topology: Allow to set the frequency through a clock @ 2014-06-30 10:12 ` Maxime Ripard 0 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-06-30 10:12 UTC (permalink / raw) To: linux-arm-kernel Hi, This patch series add the possibility for the topology code to get the CPU frequency through a DT clock handle instead of needing a clock-frequency property. Indeed, this information can be quite redundant if the clock tree defined in the DT is already describing the CPU parent clock. Maxime Changes from v1: - Rebased on top of 3.16 - Reworked a bit the code in the first patch as requested by Rob Herring Maxime Ripard (2): ARM: topology: Use a clock if possible to get the CPU frequency ARM: sunxi: Add clocks node to the CPU nodes arch/arm/boot/dts/sun6i-a31.dtsi | 4 ++++ arch/arm/boot/dts/sun7i-a20.dtsi | 2 ++ arch/arm/kernel/topology.c | 25 ++++++++++++++++++------- 3 files changed, 24 insertions(+), 7 deletions(-) -- 2.0.0 ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <1404123143-13041-1-git-send-email-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>]
* [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency 2014-06-30 10:12 ` Maxime Ripard @ 2014-06-30 10:12 ` Maxime Ripard -1 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-06-30 10:12 UTC (permalink / raw) To: Russell King, Arnd Bergmann Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Nicolas Pitre, Maxime Ripard The Cortex-A7 and Cortex-A15 based SoCs need a clock-frequency property in the topology code. Allow to use a clock to provide the same information. Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> --- arch/arm/kernel/topology.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c index 9d853189028b..268443d88094 100644 --- a/arch/arm/kernel/topology.c +++ b/arch/arm/kernel/topology.c @@ -11,6 +11,7 @@ * for more details. */ +#include <linux/clk.h> #include <linux/cpu.h> #include <linux/cpumask.h> #include <linux/export.h> @@ -100,8 +101,8 @@ static void __init parse_dt_topology(void) GFP_NOWAIT); for_each_possible_cpu(cpu) { - const u32 *rate; - int len; + struct clk *clk; + u32 rate = 0; /* too early to use cpu->of_node */ cn = of_get_cpu_node(cpu, NULL); @@ -117,14 +118,24 @@ static void __init parse_dt_topology(void) if (cpu_eff->compatible == NULL) continue; - 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); + else + of_property_read_u32(cn, "clock-frequency", &rate); + + if (!rate) { + pr_err("%s missing clocks or clock-frequency properties\n", + cn->full_name); + continue; + } + + if (!rate) { + pr_err("%s invalid CPU frequency", cn->full_name); continue; } - capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency; + capacity = ((rate) >> 20) * cpu_eff->efficiency; /* Save min capacity of the system */ if (capacity < min_capacity) -- 2.0.0 -- 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency @ 2014-06-30 10:12 ` Maxime Ripard 0 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-06-30 10:12 UTC (permalink / raw) To: linux-arm-kernel The Cortex-A7 and Cortex-A15 based SoCs need a clock-frequency property in the topology code. Allow to use a clock to provide the same information. Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> --- arch/arm/kernel/topology.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c index 9d853189028b..268443d88094 100644 --- a/arch/arm/kernel/topology.c +++ b/arch/arm/kernel/topology.c @@ -11,6 +11,7 @@ * for more details. */ +#include <linux/clk.h> #include <linux/cpu.h> #include <linux/cpumask.h> #include <linux/export.h> @@ -100,8 +101,8 @@ static void __init parse_dt_topology(void) GFP_NOWAIT); for_each_possible_cpu(cpu) { - const u32 *rate; - int len; + struct clk *clk; + u32 rate = 0; /* too early to use cpu->of_node */ cn = of_get_cpu_node(cpu, NULL); @@ -117,14 +118,24 @@ static void __init parse_dt_topology(void) if (cpu_eff->compatible == NULL) continue; - 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); + else + of_property_read_u32(cn, "clock-frequency", &rate); + + if (!rate) { + pr_err("%s missing clocks or clock-frequency properties\n", + cn->full_name); + continue; + } + + if (!rate) { + pr_err("%s invalid CPU frequency", cn->full_name); continue; } - capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency; + capacity = ((rate) >> 20) * cpu_eff->efficiency; /* Save min capacity of the system */ if (capacity < min_capacity) -- 2.0.0 ^ permalink raw reply related [flat|nested] 32+ messages in thread
[parent not found: <1404123143-13041-2-git-send-email-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>]
* Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency 2014-06-30 10:12 ` Maxime Ripard @ 2014-06-30 10:29 ` Russell King - ARM Linux -1 siblings, 0 replies; 32+ messages in thread From: Russell King - ARM Linux @ 2014-06-30 10:29 UTC (permalink / raw) To: Maxime Ripard, Vincent Guittot Cc: Arnd Bergmann, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Nicolas Pitre On Mon, Jun 30, 2014 at 12:12:22PM +0200, Maxime Ripard wrote: > The Cortex-A7 and Cortex-A15 based SoCs need a clock-frequency property in the > topology code. > > Allow to use a clock to provide the same information. > > Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> This looks fine to me, but I don't know this code. Maybe Vincent Guittot should be on the To: list (added)? > --- > arch/arm/kernel/topology.c | 25 ++++++++++++++++++------- > 1 file changed, 18 insertions(+), 7 deletions(-) > > diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c > index 9d853189028b..268443d88094 100644 > --- a/arch/arm/kernel/topology.c > +++ b/arch/arm/kernel/topology.c > @@ -11,6 +11,7 @@ > * for more details. > */ > > +#include <linux/clk.h> > #include <linux/cpu.h> > #include <linux/cpumask.h> > #include <linux/export.h> > @@ -100,8 +101,8 @@ static void __init parse_dt_topology(void) > GFP_NOWAIT); > > for_each_possible_cpu(cpu) { > - const u32 *rate; > - int len; > + struct clk *clk; > + u32 rate = 0; > > /* too early to use cpu->of_node */ > cn = of_get_cpu_node(cpu, NULL); > @@ -117,14 +118,24 @@ static void __init parse_dt_topology(void) > if (cpu_eff->compatible == NULL) > continue; > > - 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); > + else > + of_property_read_u32(cn, "clock-frequency", &rate); > + > + if (!rate) { > + pr_err("%s missing clocks or clock-frequency properties\n", > + cn->full_name); > + continue; > + } > + > + if (!rate) { > + pr_err("%s invalid CPU frequency", cn->full_name); > continue; > } > > - capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency; > + capacity = ((rate) >> 20) * cpu_eff->efficiency; > > /* Save min capacity of the system */ > if (capacity < min_capacity) > -- > 2.0.0 > -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. -- 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency @ 2014-06-30 10:29 ` Russell King - ARM Linux 0 siblings, 0 replies; 32+ messages in thread From: Russell King - ARM Linux @ 2014-06-30 10:29 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jun 30, 2014 at 12:12:22PM +0200, Maxime Ripard wrote: > The Cortex-A7 and Cortex-A15 based SoCs need a clock-frequency property in the > topology code. > > Allow to use a clock to provide the same information. > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> This looks fine to me, but I don't know this code. Maybe Vincent Guittot should be on the To: list (added)? > --- > arch/arm/kernel/topology.c | 25 ++++++++++++++++++------- > 1 file changed, 18 insertions(+), 7 deletions(-) > > diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c > index 9d853189028b..268443d88094 100644 > --- a/arch/arm/kernel/topology.c > +++ b/arch/arm/kernel/topology.c > @@ -11,6 +11,7 @@ > * for more details. > */ > > +#include <linux/clk.h> > #include <linux/cpu.h> > #include <linux/cpumask.h> > #include <linux/export.h> > @@ -100,8 +101,8 @@ static void __init parse_dt_topology(void) > GFP_NOWAIT); > > for_each_possible_cpu(cpu) { > - const u32 *rate; > - int len; > + struct clk *clk; > + u32 rate = 0; > > /* too early to use cpu->of_node */ > cn = of_get_cpu_node(cpu, NULL); > @@ -117,14 +118,24 @@ static void __init parse_dt_topology(void) > if (cpu_eff->compatible == NULL) > continue; > > - 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); > + else > + of_property_read_u32(cn, "clock-frequency", &rate); > + > + if (!rate) { > + pr_err("%s missing clocks or clock-frequency properties\n", > + cn->full_name); > + continue; > + } > + > + if (!rate) { > + pr_err("%s invalid CPU frequency", cn->full_name); > continue; > } > > - capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency; > + capacity = ((rate) >> 20) * cpu_eff->efficiency; > > /* Save min capacity of the system */ > if (capacity < min_capacity) > -- > 2.0.0 > -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <20140630102934.GV32514-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>]
* Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency 2014-06-30 10:29 ` Russell King - ARM Linux @ 2014-06-30 10:39 ` Vincent Guittot -1 siblings, 0 replies; 32+ messages in thread From: Vincent Guittot @ 2014-06-30 10:39 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Maxime Ripard, Arnd Bergmann, LAK, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Nicolas Pitre On 30 June 2014 12:29, Russell King - ARM Linux <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> wrote: > On Mon, Jun 30, 2014 at 12:12:22PM +0200, Maxime Ripard wrote: >> The Cortex-A7 and Cortex-A15 based SoCs need a clock-frequency property in the >> topology code. >> >> Allow to use a clock to provide the same information. >> >> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> > > This looks fine to me, but I don't know this code. Maybe Vincent Guittot > should be on the To: list (added)? Thanks > >> --- >> arch/arm/kernel/topology.c | 25 ++++++++++++++++++------- >> 1 file changed, 18 insertions(+), 7 deletions(-) >> >> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c >> index 9d853189028b..268443d88094 100644 >> --- a/arch/arm/kernel/topology.c >> +++ b/arch/arm/kernel/topology.c >> @@ -11,6 +11,7 @@ >> * for more details. >> */ >> >> +#include <linux/clk.h> >> #include <linux/cpu.h> >> #include <linux/cpumask.h> >> #include <linux/export.h> >> @@ -100,8 +101,8 @@ static void __init parse_dt_topology(void) >> GFP_NOWAIT); >> >> for_each_possible_cpu(cpu) { >> - const u32 *rate; >> - int len; >> + struct clk *clk; >> + u32 rate = 0; >> >> /* too early to use cpu->of_node */ >> cn = of_get_cpu_node(cpu, NULL); >> @@ -117,14 +118,24 @@ static void __init parse_dt_topology(void) >> if (cpu_eff->compatible == NULL) >> continue; >> >> - 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 ? Vincent >> + else >> + of_property_read_u32(cn, "clock-frequency", &rate); >> + >> + if (!rate) { >> + pr_err("%s missing clocks or clock-frequency properties\n", >> + cn->full_name); >> + continue; >> + } >> + >> + if (!rate) { >> + pr_err("%s invalid CPU frequency", cn->full_name); >> continue; >> } >> >> - capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency; >> + capacity = ((rate) >> 20) * cpu_eff->efficiency; >> >> /* Save min capacity of the system */ >> if (capacity < min_capacity) >> -- >> 2.0.0 >> > > -- > FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly > improving, and getting towards what was expected from it. -- 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency @ 2014-06-30 10:39 ` Vincent Guittot 0 siblings, 0 replies; 32+ messages in thread From: Vincent Guittot @ 2014-06-30 10:39 UTC (permalink / raw) To: linux-arm-kernel On 30 June 2014 12:29, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Mon, Jun 30, 2014 at 12:12:22PM +0200, Maxime Ripard wrote: >> The Cortex-A7 and Cortex-A15 based SoCs need a clock-frequency property in the >> topology code. >> >> Allow to use a clock to provide the same information. >> >> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > This looks fine to me, but I don't know this code. Maybe Vincent Guittot > should be on the To: list (added)? Thanks > >> --- >> arch/arm/kernel/topology.c | 25 ++++++++++++++++++------- >> 1 file changed, 18 insertions(+), 7 deletions(-) >> >> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c >> index 9d853189028b..268443d88094 100644 >> --- a/arch/arm/kernel/topology.c >> +++ b/arch/arm/kernel/topology.c >> @@ -11,6 +11,7 @@ >> * for more details. >> */ >> >> +#include <linux/clk.h> >> #include <linux/cpu.h> >> #include <linux/cpumask.h> >> #include <linux/export.h> >> @@ -100,8 +101,8 @@ static void __init parse_dt_topology(void) >> GFP_NOWAIT); >> >> for_each_possible_cpu(cpu) { >> - const u32 *rate; >> - int len; >> + struct clk *clk; >> + u32 rate = 0; >> >> /* too early to use cpu->of_node */ >> cn = of_get_cpu_node(cpu, NULL); >> @@ -117,14 +118,24 @@ static void __init parse_dt_topology(void) >> if (cpu_eff->compatible == NULL) >> continue; >> >> - 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 ? Vincent >> + else >> + of_property_read_u32(cn, "clock-frequency", &rate); >> + >> + if (!rate) { >> + pr_err("%s missing clocks or clock-frequency properties\n", >> + cn->full_name); >> + continue; >> + } >> + >> + if (!rate) { >> + pr_err("%s invalid CPU frequency", cn->full_name); >> continue; >> } >> >> - capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency; >> + capacity = ((rate) >> 20) * cpu_eff->efficiency; >> >> /* Save min capacity of the system */ >> if (capacity < min_capacity) >> -- >> 2.0.0 >> > > -- > FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly > improving, and getting towards what was expected from it. ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <CAKfTPtC+XcH122AytyZ=x1JVx9THmxCasJLA3Z1e17X_OmH1xA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency 2014-06-30 10:39 ` Vincent Guittot @ 2014-06-30 12:49 ` Maxime Ripard -1 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-06-30 12:49 UTC (permalink / raw) To: Vincent Guittot Cc: Russell King - ARM Linux, Arnd Bergmann, LAK, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Nicolas Pitre [-- Attachment #1: Type: text/plain, Size: 2585 bytes --] On Mon, Jun 30, 2014 at 12:39:26PM +0200, Vincent Guittot wrote: > On 30 June 2014 12:29, Russell King - ARM Linux <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> wrote: > > On Mon, Jun 30, 2014 at 12:12:22PM +0200, Maxime Ripard wrote: > >> The Cortex-A7 and Cortex-A15 based SoCs need a clock-frequency property in the > >> topology code. > >> > >> Allow to use a clock to provide the same information. > >> > >> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> > > > > This looks fine to me, but I don't know this code. Maybe Vincent Guittot > > should be on the To: list (added)? > > Thanks Ah, sorry for that. > > > > >> --- > >> arch/arm/kernel/topology.c | 25 ++++++++++++++++++------- > >> 1 file changed, 18 insertions(+), 7 deletions(-) > >> > >> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c > >> index 9d853189028b..268443d88094 100644 > >> --- a/arch/arm/kernel/topology.c > >> +++ b/arch/arm/kernel/topology.c > >> @@ -11,6 +11,7 @@ > >> * for more details. > >> */ > >> > >> +#include <linux/clk.h> > >> #include <linux/cpu.h> > >> #include <linux/cpumask.h> > >> #include <linux/export.h> > >> @@ -100,8 +101,8 @@ static void __init parse_dt_topology(void) > >> GFP_NOWAIT); > >> > >> for_each_possible_cpu(cpu) { > >> - const u32 *rate; > >> - int len; > >> + struct clk *clk; > >> + u32 rate = 0; > >> > >> /* too early to use cpu->of_node */ > >> cn = of_get_cpu_node(cpu, NULL); > >> @@ -117,14 +118,24 @@ static void __init parse_dt_topology(void) > >> if (cpu_eff->compatible == NULL) > >> continue; > >> > >> - 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. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency @ 2014-06-30 12:49 ` Maxime Ripard 0 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-06-30 12:49 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jun 30, 2014 at 12:39:26PM +0200, Vincent Guittot wrote: > On 30 June 2014 12:29, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > On Mon, Jun 30, 2014 at 12:12:22PM +0200, Maxime Ripard wrote: > >> The Cortex-A7 and Cortex-A15 based SoCs need a clock-frequency property in the > >> topology code. > >> > >> Allow to use a clock to provide the same information. > >> > >> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > > > This looks fine to me, but I don't know this code. Maybe Vincent Guittot > > should be on the To: list (added)? > > Thanks Ah, sorry for that. > > > > >> --- > >> arch/arm/kernel/topology.c | 25 ++++++++++++++++++------- > >> 1 file changed, 18 insertions(+), 7 deletions(-) > >> > >> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c > >> index 9d853189028b..268443d88094 100644 > >> --- a/arch/arm/kernel/topology.c > >> +++ b/arch/arm/kernel/topology.c > >> @@ -11,6 +11,7 @@ > >> * for more details. > >> */ > >> > >> +#include <linux/clk.h> > >> #include <linux/cpu.h> > >> #include <linux/cpumask.h> > >> #include <linux/export.h> > >> @@ -100,8 +101,8 @@ static void __init parse_dt_topology(void) > >> GFP_NOWAIT); > >> > >> for_each_possible_cpu(cpu) { > >> - const u32 *rate; > >> - int len; > >> + struct clk *clk; > >> + u32 rate = 0; > >> > >> /* too early to use cpu->of_node */ > >> cn = of_get_cpu_node(cpu, NULL); > >> @@ -117,14 +118,24 @@ static void __init parse_dt_topology(void) > >> if (cpu_eff->compatible == NULL) > >> continue; > >> > >> - 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. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140630/72cfdc8d/attachment-0001.sig> ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency 2014-06-30 12:49 ` Maxime Ripard @ 2014-06-30 13:27 ` Vincent Guittot -1 siblings, 0 replies; 32+ messages in thread From: Vincent Guittot @ 2014-06-30 13:27 UTC (permalink / raw) To: Maxime Ripard Cc: Russell King - ARM Linux, Arnd Bergmann, LAK, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Nicolas Pitre On 30 June 2014 14:49, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > On Mon, Jun 30, 2014 at 12:39:26PM +0200, Vincent Guittot wrote: >> On 30 June 2014 12:29, Russell King - ARM Linux <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> wrote: >> > On Mon, Jun 30, 2014 at 12:12:22PM +0200, Maxime Ripard wrote: >> >> The Cortex-A7 and Cortex-A15 based SoCs need a clock-frequency property in the >> >> topology code. >> >> >> >> Allow to use a clock to provide the same information. >> >> >> >> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> >> > >> > This looks fine to me, but I don't know this code. Maybe Vincent Guittot >> > should be on the To: list (added)? >> >> Thanks > > Ah, sorry for that. > >> >> > >> >> --- >> >> arch/arm/kernel/topology.c | 25 ++++++++++++++++++------- >> >> 1 file changed, 18 insertions(+), 7 deletions(-) >> >> >> >> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c >> >> index 9d853189028b..268443d88094 100644 >> >> --- a/arch/arm/kernel/topology.c >> >> +++ b/arch/arm/kernel/topology.c >> >> @@ -11,6 +11,7 @@ >> >> * for more details. >> >> */ >> >> >> >> +#include <linux/clk.h> >> >> #include <linux/cpu.h> >> >> #include <linux/cpumask.h> >> >> #include <linux/export.h> >> >> @@ -100,8 +101,8 @@ static void __init parse_dt_topology(void) >> >> GFP_NOWAIT); >> >> >> >> for_each_possible_cpu(cpu) { >> >> - const u32 *rate; >> >> - int len; >> >> + struct clk *clk; >> >> + u32 rate = 0; >> >> >> >> /* too early to use cpu->of_node */ >> >> cn = of_get_cpu_node(cpu, NULL); >> >> @@ -117,14 +118,24 @@ static void __init parse_dt_topology(void) >> >> if (cpu_eff->compatible == NULL) >> >> continue; >> >> >> >> - 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 ? 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency @ 2014-06-30 13:27 ` Vincent Guittot 0 siblings, 0 replies; 32+ messages in thread From: Vincent Guittot @ 2014-06-30 13:27 UTC (permalink / raw) To: linux-arm-kernel On 30 June 2014 14:49, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Mon, Jun 30, 2014 at 12:39:26PM +0200, Vincent Guittot wrote: >> On 30 June 2014 12:29, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: >> > On Mon, Jun 30, 2014 at 12:12:22PM +0200, Maxime Ripard wrote: >> >> The Cortex-A7 and Cortex-A15 based SoCs need a clock-frequency property in the >> >> topology code. >> >> >> >> Allow to use a clock to provide the same information. >> >> >> >> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> >> > >> > This looks fine to me, but I don't know this code. Maybe Vincent Guittot >> > should be on the To: list (added)? >> >> Thanks > > Ah, sorry for that. > >> >> > >> >> --- >> >> arch/arm/kernel/topology.c | 25 ++++++++++++++++++------- >> >> 1 file changed, 18 insertions(+), 7 deletions(-) >> >> >> >> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c >> >> index 9d853189028b..268443d88094 100644 >> >> --- a/arch/arm/kernel/topology.c >> >> +++ b/arch/arm/kernel/topology.c >> >> @@ -11,6 +11,7 @@ >> >> * for more details. >> >> */ >> >> >> >> +#include <linux/clk.h> >> >> #include <linux/cpu.h> >> >> #include <linux/cpumask.h> >> >> #include <linux/export.h> >> >> @@ -100,8 +101,8 @@ static void __init parse_dt_topology(void) >> >> GFP_NOWAIT); >> >> >> >> for_each_possible_cpu(cpu) { >> >> - const u32 *rate; >> >> - int len; >> >> + struct clk *clk; >> >> + u32 rate = 0; >> >> >> >> /* too early to use cpu->of_node */ >> >> cn = of_get_cpu_node(cpu, NULL); >> >> @@ -117,14 +118,24 @@ static void __init parse_dt_topology(void) >> >> if (cpu_eff->compatible == NULL) >> >> continue; >> >> >> >> - 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 ? Vincent > > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <CAKfTPtCgpc1J888hbqxoaF5UfcUoRwcpokme32WHPpsPAAawUQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency 2014-06-30 13:27 ` Vincent Guittot @ 2014-06-30 14:01 ` Maxime Ripard -1 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-06-30 14:01 UTC (permalink / raw) To: Vincent Guittot Cc: Russell King - ARM Linux, Arnd Bergmann, LAK, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Nicolas Pitre [-- Attachment #1: Type: text/plain, Size: 1315 bytes --] On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote: > >> >> - 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 [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency @ 2014-06-30 14:01 ` Maxime Ripard 0 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-06-30 14:01 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote: > >> >> - 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 -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140630/3955e656/attachment.sig> ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency 2014-06-30 14:01 ` Maxime Ripard @ 2014-06-30 14:48 ` Vincent Guittot -1 siblings, 0 replies; 32+ messages in thread From: Vincent Guittot @ 2014-06-30 14:48 UTC (permalink / raw) To: Maxime Ripard Cc: Russell King - ARM Linux, Arnd Bergmann, LAK, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Nicolas Pitre On 30 June 2014 16:01, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote: >> >> >> - 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency @ 2014-06-30 14:48 ` Vincent Guittot 0 siblings, 0 replies; 32+ messages in thread From: Vincent Guittot @ 2014-06-30 14:48 UTC (permalink / raw) To: linux-arm-kernel On 30 June 2014 16:01, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote: >> >> >> - 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency 2014-06-30 14:48 ` Vincent Guittot @ 2014-06-30 14:58 ` Maxime Ripard -1 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-06-30 14:58 UTC (permalink / raw) To: Vincent Guittot Cc: Russell King - ARM Linux, Arnd Bergmann, LAK, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Nicolas Pitre [-- Attachment #1: Type: text/plain, Size: 1938 bytes --] On Mon, Jun 30, 2014 at 04:48:35PM +0200, Vincent Guittot wrote: > On 30 June 2014 16:01, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > > On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote: > >> >> >> - 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 [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency @ 2014-06-30 14:58 ` Maxime Ripard 0 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-06-30 14:58 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jun 30, 2014 at 04:48:35PM +0200, Vincent Guittot wrote: > On 30 June 2014 16:01, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > > On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote: > >> >> >> - 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 -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140630/6b84a52e/attachment.sig> ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency 2014-06-30 14:58 ` Maxime Ripard @ 2014-06-30 15:06 ` Vincent Guittot -1 siblings, 0 replies; 32+ messages in thread From: Vincent Guittot @ 2014-06-30 15:06 UTC (permalink / raw) To: Maxime Ripard Cc: Russell King - ARM Linux, Arnd Bergmann, LAK, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Nicolas Pitre On 30 June 2014 16:58, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > On Mon, Jun 30, 2014 at 04:48:35PM +0200, Vincent Guittot wrote: >> On 30 June 2014 16:01, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: >> > On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote: >> >> >> >> - 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 > >> 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency @ 2014-06-30 15:06 ` Vincent Guittot 0 siblings, 0 replies; 32+ messages in thread From: Vincent Guittot @ 2014-06-30 15:06 UTC (permalink / raw) To: linux-arm-kernel On 30 June 2014 16:58, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Mon, Jun 30, 2014 at 04:48:35PM +0200, Vincent Guittot wrote: >> On 30 June 2014 16:01, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: >> > On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote: >> >> >> >> - 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 > >> 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <CAKfTPtDvQJ5i=t8FN0gLQex0yAC4NNgewTnGKdiCeGk7nQwZYg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency 2014-06-30 15:06 ` Vincent Guittot @ 2014-07-03 7:10 ` Maxime Ripard -1 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-07-03 7:10 UTC (permalink / raw) To: Vincent Guittot Cc: Russell King - ARM Linux, Arnd Bergmann, LAK, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Nicolas Pitre [-- Attachment #1: Type: text/plain, Size: 2479 bytes --] On Mon, Jun 30, 2014 at 05:06:16PM +0200, Vincent Guittot wrote: > On 30 June 2014 16:58, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > > On Mon, Jun 30, 2014 at 04:48:35PM +0200, Vincent Guittot wrote: > >> On 30 June 2014 16:01, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > >> > On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote: > >> >> >> >> - 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 > > > > >> 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 Is this an Acked-by ? Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency @ 2014-07-03 7:10 ` Maxime Ripard 0 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-07-03 7:10 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jun 30, 2014 at 05:06:16PM +0200, Vincent Guittot wrote: > On 30 June 2014 16:58, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > > On Mon, Jun 30, 2014 at 04:48:35PM +0200, Vincent Guittot wrote: > >> On 30 June 2014 16:01, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > >> > On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote: > >> >> >> >> - 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 > > > > >> 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 Is this an Acked-by ? Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140703/491ce4a6/attachment.sig> ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency 2014-07-03 7:10 ` Maxime Ripard @ 2014-07-03 7:51 ` Vincent Guittot -1 siblings, 0 replies; 32+ messages in thread From: Vincent Guittot @ 2014-07-03 7:51 UTC (permalink / raw) To: Maxime Ripard Cc: Russell King - ARM Linux, Arnd Bergmann, LAK, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Nicolas Pitre On 3 July 2014 09:10, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > On Mon, Jun 30, 2014 at 05:06:16PM +0200, Vincent Guittot wrote: >> On 30 June 2014 16:58, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: >> > On Mon, Jun 30, 2014 at 04:48:35PM +0200, Vincent Guittot wrote: >> >> On 30 June 2014 16:01, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: >> >> > On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote: >> >> >> >> >> - 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 >> >> > >> >> 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 > > Is this an Acked-by ? I still think that using the current rate with clk_get_rate is not a good solution. Could you give us more details about why you nee to use current clock ? AFAICT, your patch only makes sense for HMP system which don't have DVFS, is it your case ? 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency @ 2014-07-03 7:51 ` Vincent Guittot 0 siblings, 0 replies; 32+ messages in thread From: Vincent Guittot @ 2014-07-03 7:51 UTC (permalink / raw) To: linux-arm-kernel On 3 July 2014 09:10, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Mon, Jun 30, 2014 at 05:06:16PM +0200, Vincent Guittot wrote: >> On 30 June 2014 16:58, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: >> > On Mon, Jun 30, 2014 at 04:48:35PM +0200, Vincent Guittot wrote: >> >> On 30 June 2014 16:01, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: >> >> > On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote: >> >> >> >> >> - 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 >> >> > >> >> 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 > > Is this an Acked-by ? I still think that using the current rate with clk_get_rate is not a good solution. Could you give us more details about why you nee to use current clock ? AFAICT, your patch only makes sense for HMP system which don't have DVFS, is it your case ? Vincent > > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <CAKfTPtBonwH0PoEkKazUS9bPaR631Vu7p7qZ0AF1whmXq_53wg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency 2014-07-03 7:51 ` Vincent Guittot @ 2014-07-04 8:02 ` Maxime Ripard -1 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-07-04 8:02 UTC (permalink / raw) To: Vincent Guittot Cc: Russell King - ARM Linux, Arnd Bergmann, LAK, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Nicolas Pitre [-- Attachment #1: Type: text/plain, Size: 3420 bytes --] On Thu, Jul 03, 2014 at 09:51:27AM +0200, Vincent Guittot wrote: > On 3 July 2014 09:10, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > > On Mon, Jun 30, 2014 at 05:06:16PM +0200, Vincent Guittot wrote: > >> On 30 June 2014 16:58, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > >> > On Mon, Jun 30, 2014 at 04:48:35PM +0200, Vincent Guittot wrote: > >> >> On 30 June 2014 16:01, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > >> >> > On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote: > >> >> >> >> >> - 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 > >> > >> > > >> >> 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 > > > > Is this an Acked-by ? > > I still think that using the current rate with clk_get_rate is not a > good solution. Well, it's just as good as using clock-frequency, really. If you want the CPU max frequency, it's not the right property (plus, since the changed behaviour is not documented in Linux anywhere, the only reference we have is the ePAPR on this). > Could you give us more details about why you nee to use current clock ? Honestly, I just want to remove that big warning at boot on the A7-based SoCs we have :) > AFAICT, your patch only makes sense for HMP system which don't have > DVFS, is it your case ? Nope, it's just plain simple SMP. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency @ 2014-07-04 8:02 ` Maxime Ripard 0 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-07-04 8:02 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 03, 2014 at 09:51:27AM +0200, Vincent Guittot wrote: > On 3 July 2014 09:10, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > > On Mon, Jun 30, 2014 at 05:06:16PM +0200, Vincent Guittot wrote: > >> On 30 June 2014 16:58, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > >> > On Mon, Jun 30, 2014 at 04:48:35PM +0200, Vincent Guittot wrote: > >> >> On 30 June 2014 16:01, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > >> >> > On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote: > >> >> >> >> >> - 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 > >> > >> > > >> >> 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 > > > > Is this an Acked-by ? > > I still think that using the current rate with clk_get_rate is not a > good solution. Well, it's just as good as using clock-frequency, really. If you want the CPU max frequency, it's not the right property (plus, since the changed behaviour is not documented in Linux anywhere, the only reference we have is the ePAPR on this). > Could you give us more details about why you nee to use current clock ? Honestly, I just want to remove that big warning at boot on the A7-based SoCs we have :) > AFAICT, your patch only makes sense for HMP system which don't have > DVFS, is it your case ? Nope, it's just plain simple SMP. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140704/4224dc53/attachment.sig> ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency 2014-07-04 8:02 ` Maxime Ripard @ 2014-07-04 9:22 ` Vincent Guittot -1 siblings, 0 replies; 32+ messages in thread From: Vincent Guittot @ 2014-07-04 9:22 UTC (permalink / raw) To: Maxime Ripard Cc: Russell King - ARM Linux, Arnd Bergmann, LAK, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Nicolas Pitre On 4 July 2014 10:02, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > On Thu, Jul 03, 2014 at 09:51:27AM +0200, Vincent Guittot wrote: >> On 3 July 2014 09:10, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: >> > On Mon, Jun 30, 2014 at 05:06:16PM +0200, Vincent Guittot wrote: >> >> On 30 June 2014 16:58, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: >> >> > On Mon, Jun 30, 2014 at 04:48:35PM +0200, Vincent Guittot wrote: >> >> >> On 30 June 2014 16:01, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: >> >> >> > On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote: >> >> >> >> >> >> - 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 >> >> >> >> > >> >> >> 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 >> > >> > Is this an Acked-by ? >> >> I still think that using the current rate with clk_get_rate is not a >> good solution. > > Well, it's just as good as using clock-frequency, really. If you want > the CPU max frequency, it's not the right property (plus, since the > changed behaviour is not documented in Linux anywhere, the only > reference we have is the ePAPR on this). > >> Could you give us more details about why you nee to use current clock ? > > Honestly, I just want to remove that big warning at boot on the > A7-based SoCs we have :) so Why not changing the log message ? It's clearly not an error to not have clock-frequency > >> AFAICT, your patch only makes sense for HMP system which don't have >> DVFS, is it your case ? > > Nope, it's just plain simple SMP. > > 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency @ 2014-07-04 9:22 ` Vincent Guittot 0 siblings, 0 replies; 32+ messages in thread From: Vincent Guittot @ 2014-07-04 9:22 UTC (permalink / raw) To: linux-arm-kernel On 4 July 2014 10:02, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Thu, Jul 03, 2014 at 09:51:27AM +0200, Vincent Guittot wrote: >> On 3 July 2014 09:10, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: >> > On Mon, Jun 30, 2014 at 05:06:16PM +0200, Vincent Guittot wrote: >> >> On 30 June 2014 16:58, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: >> >> > On Mon, Jun 30, 2014 at 04:48:35PM +0200, Vincent Guittot wrote: >> >> >> On 30 June 2014 16:01, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: >> >> >> > On Mon, Jun 30, 2014 at 03:27:21PM +0200, Vincent Guittot wrote: >> >> >> >> >> >> - 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 >> >> >> >> > >> >> >> 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 >> > >> > Is this an Acked-by ? >> >> I still think that using the current rate with clk_get_rate is not a >> good solution. > > Well, it's just as good as using clock-frequency, really. If you want > the CPU max frequency, it's not the right property (plus, since the > changed behaviour is not documented in Linux anywhere, the only > reference we have is the ePAPR on this). > >> Could you give us more details about why you nee to use current clock ? > > Honestly, I just want to remove that big warning at boot on the > A7-based SoCs we have :) so Why not changing the log message ? It's clearly not an error to not have clock-frequency > >> AFAICT, your patch only makes sense for HMP system which don't have >> DVFS, is it your case ? > > Nope, it's just plain simple SMP. > > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <CAKfTPtBPR9QA9h4bF3EkCZufpF+X84N7JzjU88CptR1RtiNrdg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency 2014-07-04 9:22 ` Vincent Guittot @ 2014-07-08 7:39 ` Maxime Ripard -1 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-07-08 7:39 UTC (permalink / raw) To: Vincent Guittot Cc: Russell King - ARM Linux, Arnd Bergmann, LAK, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Nicolas Pitre [-- Attachment #1: Type: text/plain, Size: 1085 bytes --] On Fri, Jul 04, 2014 at 11:22:33AM +0200, Vincent Guittot wrote: > >> I still think that using the current rate with clk_get_rate is not a > >> good solution. > > > > Well, it's just as good as using clock-frequency, really. If you want > > the CPU max frequency, it's not the right property (plus, since the > > changed behaviour is not documented in Linux anywhere, the only > > reference we have is the ePAPR on this). > > > >> Could you give us more details about why you nee to use current clock ? > > > > Honestly, I just want to remove that big warning at boot on the > > A7-based SoCs we have :) > > so Why not changing the log message ? It's clearly not an error to not > have clock-frequency Because the next SoCs we're going to support will be HMP, so we're only delaying the issue by a few month if we're doing so. It doesn't fix the underlying issue where you're using a standard property for a (slightly) different usage. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency @ 2014-07-08 7:39 ` Maxime Ripard 0 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-07-08 7:39 UTC (permalink / raw) To: linux-arm-kernel On Fri, Jul 04, 2014 at 11:22:33AM +0200, Vincent Guittot wrote: > >> I still think that using the current rate with clk_get_rate is not a > >> good solution. > > > > Well, it's just as good as using clock-frequency, really. If you want > > the CPU max frequency, it's not the right property (plus, since the > > changed behaviour is not documented in Linux anywhere, the only > > reference we have is the ePAPR on this). > > > >> Could you give us more details about why you nee to use current clock ? > > > > Honestly, I just want to remove that big warning at boot on the > > A7-based SoCs we have :) > > so Why not changing the log message ? It's clearly not an error to not > have clock-frequency Because the next SoCs we're going to support will be HMP, so we're only delaying the issue by a few month if we're doing so. It doesn't fix the underlying issue where you're using a standard property for a (slightly) different usage. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140708/32eb41b8/attachment.sig> ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 2/2] ARM: sunxi: Add clocks node to the CPU nodes 2014-06-30 10:12 ` Maxime Ripard @ 2014-06-30 10:12 ` Maxime Ripard -1 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-06-30 10:12 UTC (permalink / raw) To: Russell King, Arnd Bergmann Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Nicolas Pitre, Maxime Ripard Setting the clock will make the topology code work, and will remove the following error at boot [ 0.097194] /cpus/cpu@0 missing clock-frequency property [ 0.103657] /cpus/cpu@1 missing clock-frequency property [ 0.110698] /cpus/cpu@2 missing clock-frequency property [ 0.117132] /cpus/cpu@3 missing clock-frequency property Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> --- arch/arm/boot/dts/sun6i-a31.dtsi | 4 ++++ arch/arm/boot/dts/sun7i-a20.dtsi | 2 ++ 2 files changed, 6 insertions(+) diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi index a9dfa12eb735..905b84add002 100644 --- a/arch/arm/boot/dts/sun6i-a31.dtsi +++ b/arch/arm/boot/dts/sun6i-a31.dtsi @@ -34,24 +34,28 @@ cpu@0 { compatible = "arm,cortex-a7"; device_type = "cpu"; + clocks = <&cpu>; reg = <0>; }; cpu@1 { compatible = "arm,cortex-a7"; device_type = "cpu"; + clocks = <&cpu>; reg = <1>; }; cpu@2 { compatible = "arm,cortex-a7"; device_type = "cpu"; + clocks = <&cpu>; reg = <2>; }; cpu@3 { compatible = "arm,cortex-a7"; device_type = "cpu"; + clocks = <&cpu>; reg = <3>; }; }; diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index 17823759bdc9..1d263b7f02dc 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -35,12 +35,14 @@ cpu@0 { compatible = "arm,cortex-a7"; device_type = "cpu"; + clocks = <&cpu>; reg = <0>; }; cpu@1 { compatible = "arm,cortex-a7"; device_type = "cpu"; + clocks = <&cpu>; reg = <1>; }; }; -- 2.0.0 -- 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 2/2] ARM: sunxi: Add clocks node to the CPU nodes @ 2014-06-30 10:12 ` Maxime Ripard 0 siblings, 0 replies; 32+ messages in thread From: Maxime Ripard @ 2014-06-30 10:12 UTC (permalink / raw) To: linux-arm-kernel Setting the clock will make the topology code work, and will remove the following error at boot [ 0.097194] /cpus/cpu at 0 missing clock-frequency property [ 0.103657] /cpus/cpu at 1 missing clock-frequency property [ 0.110698] /cpus/cpu at 2 missing clock-frequency property [ 0.117132] /cpus/cpu at 3 missing clock-frequency property Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> --- arch/arm/boot/dts/sun6i-a31.dtsi | 4 ++++ arch/arm/boot/dts/sun7i-a20.dtsi | 2 ++ 2 files changed, 6 insertions(+) diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi index a9dfa12eb735..905b84add002 100644 --- a/arch/arm/boot/dts/sun6i-a31.dtsi +++ b/arch/arm/boot/dts/sun6i-a31.dtsi @@ -34,24 +34,28 @@ cpu at 0 { compatible = "arm,cortex-a7"; device_type = "cpu"; + clocks = <&cpu>; reg = <0>; }; cpu at 1 { compatible = "arm,cortex-a7"; device_type = "cpu"; + clocks = <&cpu>; reg = <1>; }; cpu at 2 { compatible = "arm,cortex-a7"; device_type = "cpu"; + clocks = <&cpu>; reg = <2>; }; cpu at 3 { compatible = "arm,cortex-a7"; device_type = "cpu"; + clocks = <&cpu>; reg = <3>; }; }; diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index 17823759bdc9..1d263b7f02dc 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -35,12 +35,14 @@ cpu at 0 { compatible = "arm,cortex-a7"; device_type = "cpu"; + clocks = <&cpu>; reg = <0>; }; cpu at 1 { compatible = "arm,cortex-a7"; device_type = "cpu"; + clocks = <&cpu>; reg = <1>; }; }; -- 2.0.0 ^ permalink raw reply related [flat|nested] 32+ messages in thread
end of thread, other threads:[~2014-07-08 7:39 UTC | newest] Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-06-30 10:12 [PATCH v2 0/2] ARM: topology: Allow to set the frequency through a clock Maxime Ripard 2014-06-30 10:12 ` Maxime Ripard [not found] ` <1404123143-13041-1-git-send-email-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> 2014-06-30 10:12 ` [PATCH v2 1/2] ARM: topology: Use a clock if possible to get the CPU frequency Maxime Ripard 2014-06-30 10:12 ` Maxime Ripard [not found] ` <1404123143-13041-2-git-send-email-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> 2014-06-30 10:29 ` Russell King - ARM Linux 2014-06-30 10:29 ` Russell King - ARM Linux [not found] ` <20140630102934.GV32514-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org> 2014-06-30 10:39 ` Vincent Guittot 2014-06-30 10:39 ` Vincent Guittot [not found] ` <CAKfTPtC+XcH122AytyZ=x1JVx9THmxCasJLA3Z1e17X_OmH1xA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2014-06-30 12:49 ` Maxime Ripard 2014-06-30 12:49 ` Maxime Ripard 2014-06-30 13:27 ` Vincent Guittot 2014-06-30 13:27 ` Vincent Guittot [not found] ` <CAKfTPtCgpc1J888hbqxoaF5UfcUoRwcpokme32WHPpsPAAawUQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2014-06-30 14:01 ` Maxime Ripard 2014-06-30 14:01 ` Maxime Ripard 2014-06-30 14:48 ` Vincent Guittot 2014-06-30 14:48 ` Vincent Guittot 2014-06-30 14:58 ` Maxime Ripard 2014-06-30 14:58 ` Maxime Ripard 2014-06-30 15:06 ` Vincent Guittot 2014-06-30 15:06 ` Vincent Guittot [not found] ` <CAKfTPtDvQJ5i=t8FN0gLQex0yAC4NNgewTnGKdiCeGk7nQwZYg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2014-07-03 7:10 ` Maxime Ripard 2014-07-03 7:10 ` Maxime Ripard 2014-07-03 7:51 ` Vincent Guittot 2014-07-03 7:51 ` Vincent Guittot [not found] ` <CAKfTPtBonwH0PoEkKazUS9bPaR631Vu7p7qZ0AF1whmXq_53wg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2014-07-04 8:02 ` Maxime Ripard 2014-07-04 8:02 ` Maxime Ripard 2014-07-04 9:22 ` Vincent Guittot 2014-07-04 9:22 ` Vincent Guittot [not found] ` <CAKfTPtBPR9QA9h4bF3EkCZufpF+X84N7JzjU88CptR1RtiNrdg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2014-07-08 7:39 ` Maxime Ripard 2014-07-08 7:39 ` Maxime Ripard 2014-06-30 10:12 ` [PATCH v2 2/2] ARM: sunxi: Add clocks node to the CPU nodes Maxime Ripard 2014-06-30 10:12 ` Maxime Ripard
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.