[PATCH RESEND 0/2] ARM: topology: Allow to set the frequency through a clock

[PATCH RESEND 0/2] ARM: topology: Allow to set the frequency through a clock

[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

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       | 24 +++++++++++++++++-------
 3 files changed, 23 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

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

[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 | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index 9d853189028b..0bd044cbbcb2 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,23 @@ 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 {
+			if (of_property_read_u32(cn, "clock-frequency", &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

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

[Rob Herring]

On Tue, Jun 17, 2014 at 2:56 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> 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>
> ---
>  arch/arm/kernel/topology.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
> index 9d853189028b..0bd044cbbcb2 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,23 @@ 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 {
> +                       if (of_property_read_u32(cn, "clock-frequency", &rate)) {
> +                               pr_err("%s missing clocks or clock-frequency properties\n",
> +                                      cn->full_name);

This error check and message is redundant with the next error message.
You can remove this one and just call of_property_read_u32. rate will
remain untouched on error.

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

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2850 bytes --]

Hi Rob,

On Tue, Jun 17, 2014 at 04:13:26PM -0500, Rob Herring wrote:
> On Tue, Jun 17, 2014 at 2:56 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> 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>
> > ---
> >  arch/arm/kernel/topology.c | 24 +++++++++++++++++-------
> >  1 file changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
> > index 9d853189028b..0bd044cbbcb2 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,23 @@ 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 {
> > +                       if (of_property_read_u32(cn, "clock-frequency", &rate)) {
> > +                               pr_err("%s missing clocks or clock-frequency properties\n",
> > +                                      cn->full_name);
> 
> This error check and message is redundant with the next error message.
> You can remove this one and just call of_property_read_u32. rate will
> remain untouched on error.

I'm not sure what you mean here. There's no next error message in the
code as far as I'm aware.

If you mean that I'd rather have something like

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

Then, yes, it makes sense. I'll resend a version

Maxime

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

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RESEND 2/2] ARM: sunxi: Add clocks node to the CPU nodes

[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 01e94664232a..12a4940be551 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