[PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Viresh Kumar]

V1: https://lkml.org/lkml/2014/6/25/152

Stephen Boyd sent few patches some time back around a new cpufreq driver for
Qualcomm's Krait SoC: https://lkml.org/lkml/2014/6/24/918.

Krait couldn't use existing cpufreq-cpu0 driver as it doesn't have support for
SoC's with multiple clusters or SoC's which don't share clock line across all
CPUs.

This patchset is all about extending support beyond CPU0. It can be used for
platforms like Krait or ARM big LITTLE architecture now.

First two patches add helper routine for of and clk layer, which would be used
by later patches.

Third one adds space for driver specific data in 'struct cpufreq_policy' and
later ones migrate cpufreq-cpu0 to cpufreq-generic, i.e. can be used for SoCs
which don't share clock lines across all CPUs.

@Stephen: I haven't added your Tested-by as there were few modifications since
the time you tested it last.

Pushed here:
Rebased over v3.16-rc3:
git://git.linaro.org/people/viresh.kumar/linux.git cpufreq/cpu0-krait-v2

For guys looking to test on exynos, rebased over linux-next + some patches from
Thomas Abraham to use cpufreq-cpu0 for exynos:
git://git.linaro.org/people/viresh.kumar/linux.git cpufreq/cpu0-exynos-v2

Cc: devicetree@vger.kernel.org
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Mike Turquette <mturquette@linaro.org>
Cc: Rob Herring <rob.herring@linaro.org>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Simon Horman <horms@verge.net.au>

Viresh Kumar (14):
  of: Create of_property_match()
  clk: Create of_clk_shared_by_cpus()
  cpufreq: Add support for per-policy driver data
  cpufreq: cpu0: Add Module Author
  cpufreq: cpu0: don't validate clock on clk_put()
  cpufreq: cpu0: defer probe if clock isn't registered yet
  cpufreq: cpu0: OPPs can be populated at runtime
  cpufreq: cpu0: use dev_{err|warn|dbg} instead of pr_{err|warn|debug}
  cpufreq: cpu0: Move per-cluster initialization code to ->init()
  cpufreq: cpu0: try regulators with name "cpu-supply"
  cpufreq: cpu0: Make allocate_resources() work for any CPU
  cpufreq: cpu0: Extend support beyond CPU0
  cpufreq: cpu0: rename driver and internals to 'cpufreq_generic'
  cpufreq: generic: set platform_{driver|device} '.name' to
    'cpufreq-generic'

 .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   |  62 ----
 .../bindings/cpufreq/cpufreq-generic.txt           | 126 +++++++
 arch/arm/mach-imx/imx27-dt.c                       |   2 +-
 arch/arm/mach-imx/imx51-dt.c                       |   2 +-
 arch/arm/mach-omap2/pm.c                           |   2 +-
 arch/arm/mach-shmobile/board-ape6evm-reference.c   |   2 +-
 arch/arm/mach-shmobile/setup-sh73a0.c              |   4 +-
 arch/arm/mach-zynq/common.c                        |   2 +-
 drivers/clk/clk.c                                  |  56 +++
 drivers/cpufreq/Kconfig                            |  10 +-
 drivers/cpufreq/Kconfig.arm                        |   2 +-
 drivers/cpufreq/Makefile                           |   2 +-
 drivers/cpufreq/cpufreq-cpu0.c                     | 251 -------------
 drivers/cpufreq/cpufreq-generic.c                  | 394 +++++++++++++++++++++
 drivers/cpufreq/exynos4x12-cpufreq.c               |   2 +-
 drivers/cpufreq/highbank-cpufreq.c                 |   6 +-
 drivers/of/base.c                                  |  29 ++
 include/linux/clk.h                                |   6 +
 include/linux/cpufreq.h                            |   3 +
 include/linux/of.h                                 |  10 +
 20 files changed, 642 insertions(+), 331 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-generic.txt
 delete mode 100644 drivers/cpufreq/cpufreq-cpu0.c
 create mode 100644 drivers/cpufreq/cpufreq-generic.c

-- 
2.0.0.rc2

[PATCH 01/14] of: Create of_property_match()

[Viresh Kumar]

Create a new routine of_property_match() that will match a property between two
nodes. If they are exactly same, it returns 1 else 0. If the property isn't
available in any of the nodes or the API isn't implemented, proper error codes
would be returned.

The first user of this routine would be cpufreq-cpu0 driver, which requires to
match "clock" property between CPU nodes to identify which CPUs share clock
line. And hence this routine.

Cc: Rob Herring <rob.herring@linaro.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/of/base.c  | 29 +++++++++++++++++++++++++++++
 include/linux/of.h | 10 ++++++++++
 2 files changed, 39 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index b986480..a036c91 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1490,6 +1490,35 @@ int of_property_count_strings(struct device_node *np, const char *propname)
 }
 EXPORT_SYMBOL_GPL(of_property_count_strings);
 
+/**
+ * of_property_match - Match 'prop' property between two nodes
+ * @np1, np2: Nodes to match for property
+ * @prop_name: property to match
+ *
+ * Returns 1 on match, 0 on no match, and error for missing property.
+ */
+int of_property_match(const struct device_node *np1,
+		      const struct device_node *np2, const char *prop_name)
+{
+	const __be32 *prop1, *prop2;
+	int size1, size2;
+
+	/* Retrieve property from both nodes */
+	prop1 = of_get_property(np1, prop_name, &size1);
+	if (!prop1)
+		return -ENOENT;
+
+	prop2 = of_get_property(np2, prop_name, &size2);
+	if (!prop2)
+		return -ENOENT;
+
+	if (size1 != size2)
+		return 0;
+
+	return !memcmp(prop1, prop2, size1);
+}
+EXPORT_SYMBOL_GPL(of_property_match);
+
 void of_print_phandle_args(const char *msg, const struct of_phandle_args *args)
 {
 	int i;
diff --git a/include/linux/of.h b/include/linux/of.h
index 196b34c..4e9cf5a 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -275,6 +275,9 @@ extern int of_property_match_string(struct device_node *np,
 				    const char *string);
 extern int of_property_count_strings(struct device_node *np,
 				     const char *propname);
+extern int of_property_match(const struct device_node *np1,
+			     const struct device_node *np2,
+			     const char *prop_name);
 extern int of_device_is_compatible(const struct device_node *device,
 				   const char *);
 extern int of_device_is_available(const struct device_node *device);
@@ -498,6 +501,13 @@ static inline int of_property_count_strings(struct device_node *np,
 	return -ENOSYS;
 }
 
+static inline int of_property_match(const struct device_node *np1,
+				    const struct device_node *np2,
+				    const char *prop_name)
+{
+	return -ENOSYS;
+}
+
 static inline const void *of_get_property(const struct device_node *node,
 				const char *name,
 				int *lenp)
-- 
2.0.0.rc2

[PATCH 12/14] cpufreq: cpu0: Extend support beyond CPU0

[Viresh Kumar]

Most of the infrastructure is in place now, with only little left. How to find
siblings ?

Stephen Boyd suggested to compare "clocks" properties from CPU's DT node and
siblings should match. This patch adds another routine find_siblings() which
calls of_clk_shared_by_cpus() to find if CPUs share clock line or not.

If of_clk_shared_by_cpus() returns error, we fallback to all CPUs sharing clock
line assumption as existing platforms don't have "clocks" property in all CPU
nodes and would fail from of_clk_shared_by_cpus().

Cc: devicetree@vger.kernel.org
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   | 72 ++++++++++++++++++++--
 drivers/cpufreq/cpufreq-cpu0.c                     | 35 ++++++++++-
 2 files changed, 101 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
index f055515..4b83c1a 100644
--- a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
@@ -1,11 +1,11 @@
-Generic CPU0 cpufreq driver
+Generic cpufreq driver
 
-It is a generic cpufreq driver for CPU0 frequency management.  It
+It is a generic cpufreq driver for frequency management.  It
 supports both uniprocessor (UP) and symmetric multiprocessor (SMP)
-systems which share clock and voltage across all CPUs.
+systems which may or maynot share clock and voltage across all CPUs.
 
 Both required and optional properties listed below must be defined
-under node /cpus/cpu@0.
+under node /cpus/cpu@x. Where x is the first cpu inside a cluster.
 
 Required properties:
 - operating-points: Refer to Documentation/devicetree/bindings/power/opp.txt
@@ -19,9 +19,16 @@ Optional properties:
 - cooling-min-level:
 - cooling-max-level:
      Please refer to Documentation/devicetree/bindings/thermal/thermal.txt.
+- clocks: If CPU clock is populated from DT, "clocks" property must be copied to
+  every cpu node sharing clock with cpu@x. Generic cpufreq driver compares
+  "clocks" to find siblings, i.e. to see which CPUs share clock/voltages. If
+  only cpu@0 contains "clocks" property it is assumed that all CPUs share clock
+  line.
 
 Examples:
 
+1. All CPUs share clock/voltages
+
 cpus {
 	#address-cells = <1>;
 	#size-cells = <0>;
@@ -36,6 +43,8 @@ cpus {
 			396000  950000
 			198000  850000
 		>;
+		clocks = <&clock CLK_ARM_CLK>;
+		clock-names = "cpu";
 		clock-latency = <61036>; /* two CLK32 periods */
 		#cooling-cells = <2>;
 		cooling-min-level = <0>;
@@ -46,17 +55,72 @@ cpus {
 		compatible = "arm,cortex-a9";
 		reg = <1>;
 		next-level-cache = <&L2>;
+		clocks = <&clock CLK_ARM_CLK>;
 	};
 
 	cpu@2 {
 		compatible = "arm,cortex-a9";
 		reg = <2>;
 		next-level-cache = <&L2>;
+		clocks = <&clock CLK_ARM_CLK>;
 	};
 
 	cpu@3 {
 		compatible = "arm,cortex-a9";
 		reg = <3>;
 		next-level-cache = <&L2>;
+		clocks = <&clock CLK_ARM_CLK>;
+	};
+};
+
+
+2. All CPUs inside a cluster share clock/voltages, there are multiple clusters.
+
+cpus {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	cpu@0 {
+		compatible = "arm,cortex-a15";
+		reg = <0>;
+		next-level-cache = <&L2>;
+		operating-points = <
+			/* kHz    uV */
+			792000  1100000
+			396000  950000
+			198000  850000
+		>;
+		clocks = <&clock CLK_ARM1_CLK>;
+		clock-names = "cpu";
+		clock-latency = <61036>; /* two CLK32 periods */
+	};
+
+	cpu@1 {
+		compatible = "arm,cortex-a15";
+		reg = <1>;
+		next-level-cache = <&L2>;
+		clocks = <&clock CLK_ARM1_CLK>;
+	};
+
+	cpu@100 {
+		compatible = "arm,cortex-a7";
+		reg = <100>;
+		next-level-cache = <&L2>;
+		operating-points = <
+			/* kHz    uV */
+			792000  950000
+			396000  750000
+			198000  450000
+		>;
+		clocks = <&clock CLK_ARM2_CLK>;
+		clock-names = "cpu";
+		clock-latency = <61036>; /* two CLK32 periods */
+	};
+
+	cpu@101 {
+		compatible = "arm,cortex-a7";
+		reg = <101>;
+		next-level-cache = <&L2>;
+		clocks = <&clock CLK_ARM2_CLK>;
 	};
 };
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index 44633f6..b3f2bf0 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -177,6 +177,30 @@ try_again:
 	return ret;
 }
 
+/*
+ * Sets all CPUs as sibling if any cpu doesn't have a "clocks" property,
+ * Otherwise it matches "clocks" property to find siblings.
+ */
+static void find_siblings(struct cpufreq_policy *policy)
+{
+	int cpu, ret;
+
+	for_each_possible_cpu(cpu) {
+		if (cpu == policy->cpu)
+			continue;
+
+		ret = of_clk_shared_by_cpus(policy->cpu, cpu);
+
+		/* Error while parsing nodes, fallback to set-all */
+		if (ret < 0) {
+			cpumask_setall(policy->cpus);
+			return;
+		} else if (ret == 1) {
+			cpumask_set_cpu(cpu, policy->cpus);
+		}
+	}
+}
+
 static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
 {
 	struct cpufreq_frequency_table *freq_table;
@@ -266,9 +290,16 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
 	policy->driver_data = priv;
 
 	policy->clk = cpu_clk;
-	ret = cpufreq_generic_init(policy, freq_table, transition_latency);
-	if (ret)
+
+	find_siblings(policy);
+	ret = cpufreq_table_validate_and_show(policy, freq_table);
+	if (ret) {
+		dev_err(cpu_dev, "%s: invalid frequency table: %d\n", __func__,
+			ret);
 		goto out_cooling_unregister;
+	}
+
+	policy->cpuinfo.transition_latency = transition_latency;
 
 	return 0;
 
-- 
2.0.0.rc2

[PATCH V2 Resend 12/14] cpufreq: cpu0: Extend support beyond CPU0

[Viresh Kumar]

Most of the infrastructure is in place now, with only little left. How to find
siblings ?

Stephen Boyd suggested to compare "clocks" properties from CPU's DT node and
siblings should match. This patch adds another routine find_siblings() which
calls of_property_match() to find if CPUs share clock line or not.

If of_property_match() returns error, we fallback to all CPUs sharing clock line
assumption as existing platforms don't have "clocks" property in all CPU nodes
and would fail from of_property_match().

Cc: devicetree@vger.kernel.org
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V2 Resend: Use of_property_match() directly instead of of_clk_shared_by_cpus()
which would be dropped now.

 .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   | 72 ++++++++++++++++++++--
 drivers/cpufreq/cpufreq-cpu0.c                     | 62 ++++++++++++++++++-
 2 files changed, 128 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
index 366690c..9d65799 100644
--- a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
@@ -1,11 +1,11 @@
-Generic CPU0 cpufreq driver
+Generic cpufreq driver
 
-It is a generic cpufreq driver for CPU0 frequency management.  It
+It is a generic cpufreq driver for frequency management.  It
 supports both uniprocessor (UP) and symmetric multiprocessor (SMP)
-systems which share clock and voltage across all CPUs.
+systems which may or maynot share clock and voltage across all CPUs.
 
 Both required and optional properties listed below must be defined
-under node /cpus/cpu@0.
+under node /cpus/cpu@x. Where x is the first cpu inside a cluster.
 
 Required properties:
 - None
@@ -21,9 +21,16 @@ Optional properties:
 - cooling-min-level:
 - cooling-max-level:
      Please refer to Documentation/devicetree/bindings/thermal/thermal.txt.
+- clocks: If CPU clock is populated from DT, "clocks" property must be copied to
+  every cpu node sharing clock with cpu@x. Generic cpufreq driver compares
+  "clocks" to find siblings, i.e. to see which CPUs share clock/voltages. If
+  only cpu@0 contains "clocks" property it is assumed that all CPUs share clock
+  line.
 
 Examples:
 
+1. All CPUs share clock/voltages
+
 cpus {
 	#address-cells = <1>;
 	#size-cells = <0>;
@@ -38,6 +45,8 @@ cpus {
 			396000  950000
 			198000  850000
 		>;
+		clocks = <&clock CLK_ARM_CLK>;
+		clock-names = "cpu";
 		clock-latency = <61036>; /* two CLK32 periods */
 		#cooling-cells = <2>;
 		cooling-min-level = <0>;
@@ -48,17 +57,72 @@ cpus {
 		compatible = "arm,cortex-a9";
 		reg = <1>;
 		next-level-cache = <&L2>;
+		clocks = <&clock CLK_ARM_CLK>;
 	};
 
 	cpu@2 {
 		compatible = "arm,cortex-a9";
 		reg = <2>;
 		next-level-cache = <&L2>;
+		clocks = <&clock CLK_ARM_CLK>;
 	};
 
 	cpu@3 {
 		compatible = "arm,cortex-a9";
 		reg = <3>;
 		next-level-cache = <&L2>;
+		clocks = <&clock CLK_ARM_CLK>;
+	};
+};
+
+
+2. All CPUs inside a cluster share clock/voltages, there are multiple clusters.
+
+cpus {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	cpu@0 {
+		compatible = "arm,cortex-a15";
+		reg = <0>;
+		next-level-cache = <&L2>;
+		operating-points = <
+			/* kHz    uV */
+			792000  1100000
+			396000  950000
+			198000  850000
+		>;
+		clocks = <&clock CLK_ARM1_CLK>;
+		clock-names = "cpu";
+		clock-latency = <61036>; /* two CLK32 periods */
+	};
+
+	cpu@1 {
+		compatible = "arm,cortex-a15";
+		reg = <1>;
+		next-level-cache = <&L2>;
+		clocks = <&clock CLK_ARM1_CLK>;
+	};
+
+	cpu@100 {
+		compatible = "arm,cortex-a7";
+		reg = <100>;
+		next-level-cache = <&L2>;
+		operating-points = <
+			/* kHz    uV */
+			792000  950000
+			396000  750000
+			198000  450000
+		>;
+		clocks = <&clock CLK_ARM2_CLK>;
+		clock-names = "cpu";
+		clock-latency = <61036>; /* two CLK32 periods */
+	};
+
+	cpu@101 {
+		compatible = "arm,cortex-a7";
+		reg = <101>;
+		next-level-cache = <&L2>;
+		clocks = <&clock CLK_ARM2_CLK>;
 	};
 };
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index 44633f6..0f2fe76 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -177,6 +177,57 @@ try_again:
 	return ret;
 }
 
+/*
+ * Sets all CPUs as sibling if any cpu doesn't have a "clocks" property,
+ * Otherwise it matches "clocks" property to find siblings.
+ */
+static void find_siblings(struct cpufreq_policy *policy)
+{
+	struct private_data *priv = policy->driver_data;
+	struct device *cpu1_dev = priv->cpu_dev, *cpu2_dev;
+	struct device_node *np1, *np2;
+	int cpu, ret, set_all = 1;
+
+	np1 = of_node_get(cpu1_dev->of_node);
+
+	for_each_possible_cpu(cpu) {
+		if (cpu == policy->cpu)
+			continue;
+
+		cpu2_dev = get_cpu_device(cpu);
+		if (!cpu2_dev) {
+			dev_err(cpu1_dev, "%s: failed to cpu_dev for cpu%d\n",
+				__func__, cpu);
+			goto out_set_all;
+		}
+
+		np2 = of_node_get(cpu2_dev->of_node);
+		if (!np2) {
+			dev_err(cpu1_dev, "failed to find cpu%d node\n", cpu);
+			goto out_set_all;
+		}
+
+		ret = of_property_match(np1, np2, "clocks");
+		of_node_put(np2);
+
+		/* Error while parsing nodes, fallback to set-all */
+		if (ret < 0)
+			goto out_set_all;
+		else if (ret == 1)
+			cpumask_set_cpu(cpu, policy->cpus);
+	}
+
+	/* All processors don't share clock and voltage */
+	set_all = 0;
+
+out_set_all:
+	/* All processors share clock and voltage */
+	if (set_all)
+		cpumask_setall(policy->cpus);
+
+	of_node_put(np1);
+}
+
 static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
 {
 	struct cpufreq_frequency_table *freq_table;
@@ -266,9 +317,16 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
 	policy->driver_data = priv;
 
 	policy->clk = cpu_clk;
-	ret = cpufreq_generic_init(policy, freq_table, transition_latency);
-	if (ret)
+
+	find_siblings(policy);
+	ret = cpufreq_table_validate_and_show(policy, freq_table);
+	if (ret) {
+		dev_err(cpu_dev, "%s: invalid frequency table: %d\n", __func__,
+			ret);
 		goto out_cooling_unregister;
+	}
+
+	policy->cpuinfo.transition_latency = transition_latency;
 
 	return 0;
 
-- 
2.0.0.rc2

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Viresh Kumar]

On 1 July 2014 22:02, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> V1: https://lkml.org/lkml/2014/6/25/152
>
> Stephen Boyd sent few patches some time back around a new cpufreq driver for
> Qualcomm's Krait SoC: https://lkml.org/lkml/2014/6/24/918.
>
> Krait couldn't use existing cpufreq-cpu0 driver as it doesn't have support for
> SoC's with multiple clusters or SoC's which don't share clock line across all
> CPUs.
>
> This patchset is all about extending support beyond CPU0. It can be used for
> platforms like Krait or ARM big LITTLE architecture now.
>
> First two patches add helper routine for of and clk layer, which would be used
> by later patches.
>
> Third one adds space for driver specific data in 'struct cpufreq_policy' and
> later ones migrate cpufreq-cpu0 to cpufreq-generic, i.e. can be used for SoCs
> which don't share clock lines across all CPUs.
>
> @Stephen: I haven't added your Tested-by as there were few modifications since
> the time you tested it last.
>
> Pushed here:
> Rebased over v3.16-rc3:
> git://git.linaro.org/people/viresh.kumar/linux.git cpufreq/cpu0-krait-v2

Hi Stephen,

As suggested by you I have updated patch

7/14: cpufreq: cpu0: OPPs can be populated at runtime
12/14: cpufreq: cpu0: Extend support beyond CPU0

and dropped
2/14: clk: Create of_clk_shared_by_cpus()

Please see if they look fine and work for you.

git://git.linaro.org/people/viresh.kumar/linux.git cpufreq/cpu0-krait-v3

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Stephen Boyd]

On 07/01/14 21:12, Viresh Kumar wrote:
> On 1 July 2014 22:02, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> V1: https://lkml.org/lkml/2014/6/25/152
>>
>> Stephen Boyd sent few patches some time back around a new cpufreq driver for
>> Qualcomm's Krait SoC: https://lkml.org/lkml/2014/6/24/918.
>>
>> Krait couldn't use existing cpufreq-cpu0 driver as it doesn't have support for
>> SoC's with multiple clusters or SoC's which don't share clock line across all
>> CPUs.
>>
>> This patchset is all about extending support beyond CPU0. It can be used for
>> platforms like Krait or ARM big LITTLE architecture now.
>>
>> First two patches add helper routine for of and clk layer, which would be used
>> by later patches.
>>
>> Third one adds space for driver specific data in 'struct cpufreq_policy' and
>> later ones migrate cpufreq-cpu0 to cpufreq-generic, i.e. can be used for SoCs
>> which don't share clock lines across all CPUs.
>>
>> @Stephen: I haven't added your Tested-by as there were few modifications since
>> the time you tested it last.
>>
>> Pushed here:
>> Rebased over v3.16-rc3:
>> git://git.linaro.org/people/viresh.kumar/linux.git cpufreq/cpu0-krait-v2
> Hi Stephen,
>
> As suggested by you I have updated patch
>
> 7/14: cpufreq: cpu0: OPPs can be populated at runtime
> 12/14: cpufreq: cpu0: Extend support beyond CPU0
>
> and dropped
> 2/14: clk: Create of_clk_shared_by_cpus()
>
> Please see if they look fine and work for you.
>
> git://git.linaro.org/people/viresh.kumar/linux.git cpufreq/cpu0-krait-v3

I gave it a spin. It works so you can have my

Tested-by: Stephen Boyd <sboyd@codeaurora.org>

I'm still concerned about the patch where we figure out if the clocks
are shared. I worry about a configuration where there are different
clocks for on/off (i.e. gates) that are per-cpu but they all source from
the same divider or something that is per-cluster. In DT this may be
described as different clock provider outputs for the gates and in the
cpu node we would have different clock specifiers but in reality all the
CPUs in that cluster are affected by the same frequency scaling.

In this case we'll need to get help from the clock framework to
determine that those gates clocks don't have any .set_rate() callback so
they can't actually change rate independently, and then walk up the tree
to their parents to see if they have a common ancestor that does change
rates. That's where it becomes useful to have a clock framework API for
this, like clk_shares_rate(struct clk *clk, struct clk *peer) or
something that can hide all this from cpufreq. Here's what I think it
would look like (totally untested/uncompiled):

static struct clk *find_rate_changer(struct clk *clk)
{	

	if (!clk)
		return NULL; 

	do {
		/* Rate could change by changing parents */
		if (clk->num_parents > 1)
			return clk;

		/* Rate could change by calling clk_set_rate() */
		if (clk->ops->set_rate)
			return clk;

		/* 
		 * This is odd, clk_set_rate() doesn't propagate 
		 * and this clock can't change rate or parents
		 * so we must be at the root and the clock we
		 * started at can't change rates. Just return the
		 * root so that we can see if the other clock shares
		 * the same root although CPUfreq should never care.
		 */
		if (!(clk->flags & CLK_SET_RATE_PARENT))
			return clk;
	} while ((clk = clk->parent))

	return NULL;
}

bool clk_shares_rate(struct clk *clk, struct clk *peer)
{
	struct clk *p1, *p2;

	p1 = find_rate_changer(clk);
	p2 = find_rate_changer(peer)

	return p1 == p2;
}


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Viresh Kumar]

On 3 July 2014 06:54, Stephen Boyd <sboyd@codeaurora.org> wrote:
> I gave it a spin. It works so you can have my
>
> Tested-by: Stephen Boyd <sboyd@codeaurora.org>

Thanks, all suggested improvements are made and pushed again with
your Tested-by..

> I'm still concerned about the patch where we figure out if the clocks
> are shared. I worry about a configuration where there are different
> clocks for on/off (i.e. gates) that are per-cpu but they all source from
> the same divider or something that is per-cluster. In DT this may be
> described as different clock provider outputs for the gates and in the
> cpu node we would have different clock specifiers but in reality all the
> CPUs in that cluster are affected by the same frequency scaling.

Yeah, this is probably what matches with Rob's doubt. These can
actually be different. Good point.

> In this case we'll need to get help from the clock framework to
> determine that those gates clocks don't have any .set_rate() callback so
> they can't actually change rate independently, and then walk up the tree
> to their parents to see if they have a common ancestor that does change
> rates. That's where it becomes useful to have a clock framework API for
> this, like clk_shares_rate(struct clk *clk, struct clk *peer) or
> something that can hide all this from cpufreq. Here's what I think it
> would look like (totally untested/uncompiled):
>
> static struct clk *find_rate_changer(struct clk *clk)
> {
>
>         if (!clk)
>                 return NULL;
>
>         do {
>                 /* Rate could change by changing parents */
>                 if (clk->num_parents > 1)
>                         return clk;
>
>                 /* Rate could change by calling clk_set_rate() */
>                 if (clk->ops->set_rate)
>                         return clk;
>
>                 /*
>                  * This is odd, clk_set_rate() doesn't propagate
>                  * and this clock can't change rate or parents
>                  * so we must be at the root and the clock we
>                  * started at can't change rates. Just return the
>                  * root so that we can see if the other clock shares
>                  * the same root although CPUfreq should never care.
>                  */
>                 if (!(clk->flags & CLK_SET_RATE_PARENT))
>                         return clk;
>         } while ((clk = clk->parent))
>
>         return NULL;
> }
>
> bool clk_shares_rate(struct clk *clk, struct clk *peer)
> {
>         struct clk *p1, *p2;
>
>         p1 = find_rate_changer(clk);
>         p2 = find_rate_changer(peer)
>
>         return p1 == p2;
> }

I find it much better then doing what I did initially, simply matching clk_get()
outputs. Lets see what Mike has to say..

@Mike: Is this less ugly ? :)

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Mike Turquette]

Quoting Viresh Kumar (2014-07-02 19:44:04)
> On 3 July 2014 06:54, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > I gave it a spin. It works so you can have my
> >
> > Tested-by: Stephen Boyd <sboyd@codeaurora.org>
> 
> Thanks, all suggested improvements are made and pushed again with
> your Tested-by..
> 
> > I'm still concerned about the patch where we figure out if the clocks
> > are shared. I worry about a configuration where there are different
> > clocks for on/off (i.e. gates) that are per-cpu but they all source from
> > the same divider or something that is per-cluster. In DT this may be
> > described as different clock provider outputs for the gates and in the
> > cpu node we would have different clock specifiers but in reality all the
> > CPUs in that cluster are affected by the same frequency scaling.
> 
> Yeah, this is probably what matches with Rob's doubt. These can
> actually be different. Good point.
> 
> > In this case we'll need to get help from the clock framework to
> > determine that those gates clocks don't have any .set_rate() callback so
> > they can't actually change rate independently, and then walk up the tree
> > to their parents to see if they have a common ancestor that does change
> > rates. That's where it becomes useful to have a clock framework API for
> > this, like clk_shares_rate(struct clk *clk, struct clk *peer) or
> > something that can hide all this from cpufreq. Here's what I think it
> > would look like (totally untested/uncompiled):
> >
> > static struct clk *find_rate_changer(struct clk *clk)
> > {
> >
> >         if (!clk)
> >                 return NULL;
> >
> >         do {
> >                 /* Rate could change by changing parents */
> >                 if (clk->num_parents > 1)
> >                         return clk;
> >
> >                 /* Rate could change by calling clk_set_rate() */
> >                 if (clk->ops->set_rate)
> >                         return clk;
> >
> >                 /*
> >                  * This is odd, clk_set_rate() doesn't propagate
> >                  * and this clock can't change rate or parents
> >                  * so we must be at the root and the clock we
> >                  * started at can't change rates. Just return the
> >                  * root so that we can see if the other clock shares
> >                  * the same root although CPUfreq should never care.
> >                  */
> >                 if (!(clk->flags & CLK_SET_RATE_PARENT))
> >                         return clk;
> >         } while ((clk = clk->parent))
> >
> >         return NULL;
> > }
> >
> > bool clk_shares_rate(struct clk *clk, struct clk *peer)
> > {
> >         struct clk *p1, *p2;
> >
> >         p1 = find_rate_changer(clk);
> >         p2 = find_rate_changer(peer)
> >
> >         return p1 == p2;
> > }
> 
> I find it much better then doing what I did initially, simply matching clk_get()
> outputs. Lets see what Mike has to say..

Sorry for being dense, but I still do not get why trying to dynamically
discover a shared rate-changeable clock is a better approach than simply
describing the hardware in DT?

Is adding a property to the CPU binding that describes how the CPUs in a
cluster expect to use a clock somehow a non-starter? It is certainly a
win for readability when staring at DT and trying to understand how DVFS
on that CPU is meant to work (as opposed to hiding that knowledge behind
a tree walk).

Regards,
Mike

> 
> @Mike: Is this less ugly ? :)

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Viresh Kumar]

On 4 July 2014 03:46, Mike Turquette <mturquette@linaro.org> wrote:
> Sorry for being dense, but I still do not get why trying to dynamically
> discover a shared rate-changeable clock is a better approach than simply
> describing the hardware in DT?
>
> Is adding a property to the CPU binding that describes how the CPUs in a
> cluster expect to use a clock somehow a non-starter? It is certainly a
> win for readability when staring at DT and trying to understand how DVFS
> on that CPU is meant to work (as opposed to hiding that knowledge behind
> a tree walk).

Yeah, having something like what you suggested from DT is the perfect
solution to get over this. The only reason why I am not touching that here
is to not delay other patches just because of that.

There are separate threads going on for that and probably somebody
else was trying to push for that.

That's it, nothing more. I would definitely like to use those bindings instead
 of the crazy routines we are trying here, once that is finalized :)

--
viresh

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Viresh Kumar]

On 4 July 2014 09:51, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Yeah, having something like what you suggested from DT is the perfect
> solution to get over this. The only reason why I am not touching that here
> is to not delay other patches just because of that.
>
> There are separate threads going on for that and probably somebody
> else was trying to push for that.
>
> That's it, nothing more. I would definitely like to use those bindings instead
>  of the crazy routines we are trying here, once that is finalized :)

Do we have some kind of agreement for this temporary solution? Anyways
I will kick the other thread again to resolve the bindings soon.

@Stephen: Do you still want find_rate_changer() stuff in this series only
and or this can go into 3.17 without anymore changes and lets finalize
the bindings Mike suggested and then update this code?

--
viresh

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Stephen Boyd]

On 07/07/14 21:50, Viresh Kumar wrote:
> On 4 July 2014 09:51, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> Yeah, having something like what you suggested from DT is the perfect
>> solution to get over this. The only reason why I am not touching that here
>> is to not delay other patches just because of that.
>>
>> There are separate threads going on for that and probably somebody
>> else was trying to push for that.
>>
>> That's it, nothing more. I would definitely like to use those bindings instead
>>  of the crazy routines we are trying here, once that is finalized :)
> Do we have some kind of agreement for this temporary solution? Anyways
> I will kick the other thread again to resolve the bindings soon.
>
> @Stephen: Do you still want find_rate_changer() stuff in this series only
> and or this can go into 3.17 without anymore changes and lets finalize
> the bindings Mike suggested and then update this code?
>
>

Finding the rate change is not necessary for me. I don't imagine my code
will be getting into 3.17 anyway though so I'm no in a rush to merge
anything immediately.

I still prefer the common clock framework API. If we go ahead with the
find_rate_changer() stuff we've pretty well insulated this driver from
any DTisms, which is nice. The only thing left would be
transition-latency and voltage-tolerance, but those are already optional
so you really don't need to even run on a DT enabled platform to use
this code.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Viresh Kumar]

Cc'ing Thomas,

On 8 July 2014 10:20, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 4 July 2014 09:51, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> Yeah, having something like what you suggested from DT is the perfect
>> solution to get over this. The only reason why I am not touching that here
>> is to not delay other patches just because of that.
>>
>> There are separate threads going on for that and probably somebody
>> else was trying to push for that.
>>
>> That's it, nothing more. I would definitely like to use those bindings instead
>>  of the crazy routines we are trying here, once that is finalized :)
>
> Do we have some kind of agreement for this temporary solution? Anyways
> I will kick the other thread again to resolve the bindings soon.

Hi Mike/Rafael,

Thomas has a dependency on the patch adding "find_siblings()":
http://www.spinics.net/lists/arm-kernel/msg348080.html

Would it be fine to get this temporary solution (Only within cpufreq-cpu0
file, so that it doesn't become an API) for now and updating it later once
bindings are closed?

I have tried pinging on the other thread as well, but no one replied.
And it looks those bindings are going to take some time to settle.

--
viresh

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Thomas Petazzoni]

Dear Viresh Kumar,

On Wed, 16 Jul 2014 21:31:54 +0530, Viresh Kumar wrote:
> Cc'ing Thomas,

Thanks. Also adding Jason Cooper, Andrew Lunn, Grégory Clement and
Sebastian Hesselbart (the mvebu platform maintainers, which include
Marvell Armada XP).

> On 8 July 2014 10:20, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > On 4 July 2014 09:51, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >> Yeah, having something like what you suggested from DT is the perfect
> >> solution to get over this. The only reason why I am not touching that here
> >> is to not delay other patches just because of that.
> >>
> >> There are separate threads going on for that and probably somebody
> >> else was trying to push for that.
> >>
> >> That's it, nothing more. I would definitely like to use those bindings instead
> >>  of the crazy routines we are trying here, once that is finalized :)
> >
> > Do we have some kind of agreement for this temporary solution? Anyways
> > I will kick the other thread again to resolve the bindings soon.
> 
> Hi Mike/Rafael,
> 
> Thomas has a dependency on the patch adding "find_siblings()":
> http://www.spinics.net/lists/arm-kernel/msg348080.html
> 
> Would it be fine to get this temporary solution (Only within cpufreq-cpu0
> file, so that it doesn't become an API) for now and updating it later once
> bindings are closed?
> 
> I have tried pinging on the other thread as well, but no one replied.
> And it looks those bindings are going to take some time to settle.

Alternatively, I could respin my Armada XP specific cpufreq driver,
until a proper cpufreq-generic driver is sorted out, if that's needed.
Since I was told by Viresh that the cpufreq-generic driver supporting
independent per-CPU clocks would be merged for 3.17, I based my cpufreq
development on that. Please let us know quickly if that's not going to
be the case, so that we can enable cpufreq on Armada XP in 3.17, even
if it's done with a separate cpufreq driver, until cpufreq-generic
issues get sorted out.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

_______________________________________________
linaro-kernel mailing list
linaro-kernel@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-kernel

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Rafael J. Wysocki]

On Wednesday, July 16, 2014 07:28:15 PM Thomas Petazzoni wrote:
> Dear Viresh Kumar,
> 
> On Wed, 16 Jul 2014 21:31:54 +0530, Viresh Kumar wrote:
> > Cc'ing Thomas,
> 
> Thanks. Also adding Jason Cooper, Andrew Lunn, Grégory Clement and
> Sebastian Hesselbart (the mvebu platform maintainers, which include
> Marvell Armada XP).
> 
> > On 8 July 2014 10:20, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > On 4 July 2014 09:51, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > >> Yeah, having something like what you suggested from DT is the perfect
> > >> solution to get over this. The only reason why I am not touching that here
> > >> is to not delay other patches just because of that.
> > >>
> > >> There are separate threads going on for that and probably somebody
> > >> else was trying to push for that.
> > >>
> > >> That's it, nothing more. I would definitely like to use those bindings instead
> > >>  of the crazy routines we are trying here, once that is finalized :)
> > >
> > > Do we have some kind of agreement for this temporary solution? Anyways
> > > I will kick the other thread again to resolve the bindings soon.
> > 
> > Hi Mike/Rafael,
> > 
> > Thomas has a dependency on the patch adding "find_siblings()":
> > http://www.spinics.net/lists/arm-kernel/msg348080.html
> > 
> > Would it be fine to get this temporary solution (Only within cpufreq-cpu0
> > file, so that it doesn't become an API) for now and updating it later once
> > bindings are closed?
> > 
> > I have tried pinging on the other thread as well, but no one replied.
> > And it looks those bindings are going to take some time to settle.
> 
> Alternatively, I could respin my Armada XP specific cpufreq driver,
> until a proper cpufreq-generic driver is sorted out, if that's needed.
> Since I was told by Viresh that the cpufreq-generic driver supporting
> independent per-CPU clocks would be merged for 3.17, I based my cpufreq
> development on that. Please let us know quickly if that's not going to
> be the case,

First off, I'm sorry I may not be able to do that quickly.  I'll let you
know when I get to that material.

> so that we can enable cpufreq on Armada XP in 3.17, even
> if it's done with a separate cpufreq driver, until cpufreq-generic
> issues get sorted out.

If you target the generic one, I'd strongly recommend against trying to do
a separate driver even if the generic thing is not ready for the merge
window.

Kind regards,
Rafael


_______________________________________________
linaro-kernel mailing list
linaro-kernel@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-kernel

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Rafael J. Wysocki]

On Wednesday, July 16, 2014 09:31:54 PM Viresh Kumar wrote:
> Cc'ing Thomas,
> 
> On 8 July 2014 10:20, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > On 4 July 2014 09:51, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >> Yeah, having something like what you suggested from DT is the perfect
> >> solution to get over this. The only reason why I am not touching that here
> >> is to not delay other patches just because of that.
> >>
> >> There are separate threads going on for that and probably somebody
> >> else was trying to push for that.
> >>
> >> That's it, nothing more. I would definitely like to use those bindings instead
> >>  of the crazy routines we are trying here, once that is finalized :)
> >
> > Do we have some kind of agreement for this temporary solution? Anyways
> > I will kick the other thread again to resolve the bindings soon.
> 
> Hi Mike/Rafael,
> 
> Thomas has a dependency on the patch adding "find_siblings()":
> http://www.spinics.net/lists/arm-kernel/msg348080.html
> 
> Would it be fine to get this temporary solution (Only within cpufreq-cpu0
> file, so that it doesn't become an API) for now and updating it later once
> bindings are closed?

I don't like that idea, but I wonder what other people think.

Rafael

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Viresh Kumar]

On 17 July 2014 02:48, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> I don't like that idea, but I wonder what other people think.

Hmm, the other thread around looking at the bindings is really slow.

One common thing around the platforms which want to use
cpufreq-cpu0 is they have different clocks for ALL CPUs.

I was wondering if instead of a clock-matching routine, we can provide
some temporary relief to them via some other means.

I meant we can allow cpufreq-cpu0/generic to either set policy->cpus
to ALL CPUs or just 1. So that existing and these new platforms can
atleast get going..

But don't know how should we do that. Not a binding ofcourse, a
Kconfig option could work but multiplatform stuff would break. What
else?

Maybe platform data as we are handling cpufreq-cpu0 with a platform
device?

--
viresh

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Thomas Petazzoni]

Dear Viresh Kumar,

On Thu, 17 Jul 2014 05:58:22 +0530, Viresh Kumar wrote:
> On 17 July 2014 02:48, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > I don't like that idea, but I wonder what other people think.
> 
> Hmm, the other thread around looking at the bindings is really slow.

Could you summarize what is the issue with the binding?

At least for the case where we have one clock per CPU, the DT binding
is really dead simple: each CPU node can carry a "clocks" property, and
a "clock-latency" property. I really don't see why a long discussion is
needed to agree on such a binding.

Now, if the DT binding problem is related to those cases where you have
siblings, i.e one clock controlling *some* of the CPUs, but not all
CPUs or just one CPU, then maybe we could leave this aside for now,
only support the following cases:

 * One clock for all CPUs
 * One clock for each CPU

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Viresh Kumar]

On 17 July 2014 13:05, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Could you summarize what is the issue with the binding?
>
> At least for the case where we have one clock per CPU, the DT binding
> is really dead simple: each CPU node can carry a "clocks" property, and
> a "clock-latency" property. I really don't see why a long discussion is
> needed to agree on such a binding.
>
> Now, if the DT binding problem is related to those cases where you have
> siblings, i.e one clock controlling *some* of the CPUs, but not all
> CPUs or just one CPU, then maybe we could leave this aside for now,

Yeah, we are stuck on that for now.

> only support the following cases:
>
>  * One clock for all CPUs
>  * One clock for each CPU

Yeah, so I also proposed this yesterday that we stick to only these
two implementations for now. And was looking at how would the
cpufreq-generic driver come to know about this.

So, one way out now is to see if "clocks" property is defined in
multiple cpu nodes, if yes don't compare them and consider separate
clocks for each cpu. We don't have to try matching that to any other
node, as that's a very bad idea. Mike was already very upset with that :)

@Stephen/Rafael: Does that sound any better? Ofcourse the final thing
is to get bindings to figure out relations between CPUs..

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Rafael J. Wysocki]

On Thursday, July 17, 2014 01:11:45 PM Viresh Kumar wrote:
> On 17 July 2014 13:05, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
> > Could you summarize what is the issue with the binding?
> >
> > At least for the case where we have one clock per CPU, the DT binding
> > is really dead simple: each CPU node can carry a "clocks" property, and
> > a "clock-latency" property. I really don't see why a long discussion is
> > needed to agree on such a binding.
> >
> > Now, if the DT binding problem is related to those cases where you have
> > siblings, i.e one clock controlling *some* of the CPUs, but not all
> > CPUs or just one CPU, then maybe we could leave this aside for now,
> 
> Yeah, we are stuck on that for now.
> 
> > only support the following cases:
> >
> >  * One clock for all CPUs
> >  * One clock for each CPU
> 
> Yeah, so I also proposed this yesterday that we stick to only these
> two implementations for now. And was looking at how would the
> cpufreq-generic driver come to know about this.
> 
> So, one way out now is to see if "clocks" property is defined in
> multiple cpu nodes, if yes don't compare them and consider separate
> clocks for each cpu. We don't have to try matching that to any other
> node, as that's a very bad idea. Mike was already very upset with that :)
> 
> @Stephen/Rafael: Does that sound any better? Ofcourse the final thing
> is to get bindings to figure out relations between CPUs..

Before I apply anything in this area, I need a clear statement from the ARM
people as a group on what the approach is going to be.

Rafael

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Viresh Kumar]

On 18 July 2014 06:32, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> > only support the following cases:
>> >
>> >  * One clock for all CPUs
>> >  * One clock for each CPU
>>
>> Yeah, so I also proposed this yesterday that we stick to only these
>> two implementations for now. And was looking at how would the
>> cpufreq-generic driver come to know about this.
>>
>> So, one way out now is to see if "clocks" property is defined in
>> multiple cpu nodes, if yes don't compare them and consider separate
>> clocks for each cpu. We don't have to try matching that to any other
>> node, as that's a very bad idea. Mike was already very upset with that :)
>>
>> @Stephen/Rafael: Does that sound any better? Ofcourse the final thing
>> is to get bindings to figure out relations between CPUs..
>
> Before I apply anything in this area, I need a clear statement from the ARM
> people as a group on what the approach is going to be.

Thanks for your response Rafael.

Mike/Rob/Stephen: I believe Atleast three of you should express your views
now :)

So, this is what I propose:

- I will start another thread with a new DT binding, something like:

"clocks-ganged" = <&cpu0>

and then we can decide on naming/etc ..

- I will drop the patch which matches clock nodes from DT and introduce
another one that will just check if "clocks" is mentioned in more than one
CPU. If yes, then we behave as if all CPUs have separate clock lines.

That will work for Krait/mvebu and all existing users.

Does that sound good?

--
viresh

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Viresh Kumar]

On 18 July 2014 09:47, Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> Before I apply anything in this area, I need a clear statement from the ARM
>> people as a group on what the approach is going to be.

@Rafael: The only patch which has blocked this set is:

cpufreq: cpu0: Extend support beyond CPU0

This is about finding which CPUs share clock line..

Other patches are sort of independent of this one and cpufreq-cpu0 would
continue to work for existing platforms if we apply these patches.

I was wondering if you would like to apply other patches leaving this one?
I will then rebase stuff again and send non-controversial patches for 3.17.
So, that we get something in 3.17 and the last change can be merged during
rc's as well once we finalize on bindings..

> Mike/Rob/Stephen: I believe Atleast three of you should express your views
> now :)

Any idea on how can we get some temporary solution for 3.17 as we didn't
conclude anything yet on bindings ?

--
viresh
--
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 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Rob Herring]

On Thu, Jul 24, 2014 at 5:48 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 18 July 2014 09:47, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>> Before I apply anything in this area, I need a clear statement from the ARM
>>> people as a group on what the approach is going to be.
>
> @Rafael: The only patch which has blocked this set is:
>
> cpufreq: cpu0: Extend support beyond CPU0
>
> This is about finding which CPUs share clock line..
>
> Other patches are sort of independent of this one and cpufreq-cpu0 would
> continue to work for existing platforms if we apply these patches.
>
> I was wondering if you would like to apply other patches leaving this one?
> I will then rebase stuff again and send non-controversial patches for 3.17.
> So, that we get something in 3.17 and the last change can be merged during
> rc's as well once we finalize on bindings..
>
>> Mike/Rob/Stephen: I believe Atleast three of you should express your views
>> now :)
>
> Any idea on how can we get some temporary solution for 3.17 as we didn't
> conclude anything yet on bindings ?

A temporary solution would have to be NOT in DT because once you add
something into DT you are stuck with it for some time. You could
simply support independent clocks by looking at the platform type, but
that is still risky since you may want to define the OPP in DT
differently for the 2 cases.

The other problem with temporary solutions is once they are accepted,
people loose motivation to create the permanent solution. "Can you
take this now and I'll fix the issues later" is a red flag to
maintainers.

Rob

Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Viresh Kumar]

On 25 July 2014 19:59, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> A temporary solution would have to be NOT in DT because once you add
> something into DT you are stuck with it for some time. You could

I agree..

> simply support independent clocks by looking at the platform type, but
> that is still risky since you may want to define the OPP in DT
> differently for the 2 cases.

Or because we are going to create platform devices for now, lets have
some platform data for cpufreq-cpu0 ?

> The other problem with temporary solutions is once they are accepted,
> people loose motivation to create the permanent solution. "Can you
> take this now and I'll fix the issues later" is a red flag to
> maintainers.

I completely agree, but there are few points here which might make the red
flag yellow if not green :)
- I co-maintain this framework and so I do care about it :)
- Even with the temporary stuff the solution wouldn't be complete for platforms
with multiple clusters having separate clock lines.
--
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 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2

[Thomas Petazzoni]

Hello,

On Fri, 25 Jul 2014 09:29:09 -0500, Rob Herring wrote:

> > Any idea on how can we get some temporary solution for 3.17 as we didn't
> > conclude anything yet on bindings ?
> 
> A temporary solution would have to be NOT in DT because once you add
> something into DT you are stuck with it for some time. You could
> simply support independent clocks by looking at the platform type, but
> that is still risky since you may want to define the OPP in DT
> differently for the 2 cases.
> 
> The other problem with temporary solutions is once they are accepted,
> people loose motivation to create the permanent solution. "Can you
> take this now and I'll fix the issues later" is a red flag to
> maintainers.

On the Marvell Armada XP side of things, the OPPs are registered
dynamically because they change from one board to the other. The only
thing that is in the DT for cpufreq-generic is the clock-latency
parameter.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com