All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
@ 2014-07-01 16:32 Viresh Kumar
  2014-07-01 16:32 ` [PATCH 01/14] of: Create of_property_match() Viresh Kumar
                   ` (14 more replies)
  0 siblings, 15 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-01 16:32 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar,
	devicetree, Kukjin Kim, Michal Simek, Mike Turquette,
	Rob Herring, Santosh Shilimkar, Simon Horman

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* [PATCH 01/14] of: Create of_property_match()
  2014-07-01 16:32 [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
@ 2014-07-01 16:32 ` Viresh Kumar
  2014-07-01 16:32 ` [PATCH 02/14] clk: Create of_clk_shared_by_cpus() Viresh Kumar
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-01 16:32 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar,
	Rob Herring, devicetree

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

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* [PATCH 02/14] clk: Create of_clk_shared_by_cpus()
  2014-07-01 16:32 [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
  2014-07-01 16:32 ` [PATCH 01/14] of: Create of_property_match() Viresh Kumar
@ 2014-07-01 16:32 ` Viresh Kumar
  2014-07-01 18:00   ` Stephen Boyd
                     ` (2 more replies)
  2014-07-01 16:32 ` [PATCH 03/14] cpufreq: Add support for per-policy driver data Viresh Kumar
                   ` (12 subsequent siblings)
  14 siblings, 3 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-01 16:32 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar,
	Mike Turquette

Create a new routine of_clk_shared_by_cpus() that finds if clock lines are
shared between two CPUs. This is verified by comparing "clocks" property from
CPU's DT node.

Returns 1 if clock line is shared between them, 0 if clock isn't shared and
return appropriate errors in case nodes/properties are missing.

Cc: Mike Turquette <mturquette@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/clk/clk.c   | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/clk.h |  6 ++++++
 2 files changed, 62 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8b73ede..497735c 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -10,6 +10,7 @@
  */
 
 #include <linux/clk-private.h>
+#include <linux/cpu.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/spinlock.h>
@@ -2528,6 +2529,61 @@ const char *of_clk_get_parent_name(struct device_node *np, int index)
 }
 EXPORT_SYMBOL_GPL(of_clk_get_parent_name);
 
+/**
+ * of_clk_shared_by_cpus - Finds if clock line is shared between CPUs
+ * @cpu1, cpu2: CPU numbers
+ *
+ * Finds if clock lines are shared by two CPUs. This is verified by comparing
+ * "clocks" property from CPU's DT node.
+ *
+ * Returns 1 if clock line is shared between them, 0 if clock isn't shared.
+ * Return appropriate errors in case some requirements aren't met.
+ */
+int of_clk_shared_by_cpus(int cpu1, int cpu2)
+{
+	struct device *cpu1_dev, *cpu2_dev;
+	struct device_node *np1, *np2;
+	int ret;
+
+	cpu1_dev = get_cpu_device(cpu1);
+	if (!cpu1_dev) {
+		pr_err("%s: failed to get cpu_dev for cpu%d\n", __func__, cpu1);
+		return -ENODEV;
+	}
+
+	cpu2_dev = get_cpu_device(cpu2);
+	if (!cpu2_dev) {
+		pr_err("%s: failed to get cpu_dev for cpu%d\n", __func__, cpu2);
+		return -ENODEV;
+	}
+
+	np1 = of_node_get(cpu1_dev->of_node);
+	if (!np1) {
+		pr_err("%s: failed to find of_node for cpu%d\n", __func__,
+		       cpu1);
+		return -ENODEV;
+	}
+
+	np2 = of_node_get(cpu2_dev->of_node);
+	if (!np2) {
+		pr_err("%s: failed to find of_node for cpu%d\n", __func__,
+		       cpu2);
+		ret = -ENODEV;
+		goto put_np1;
+	}
+
+	/* Match "clocks" property */
+	ret = of_property_match(np1, np2, "clocks");
+
+	of_node_put(np2);
+
+put_np1:
+	of_node_put(np1);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(of_clk_shared_by_cpus);
+
 struct clock_provider {
 	of_clk_init_cb_t clk_init_cb;
 	struct device_node *np;
diff --git a/include/linux/clk.h b/include/linux/clk.h
index fb5e097..58e281a 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -399,6 +399,7 @@ struct of_phandle_args;
 struct clk *of_clk_get(struct device_node *np, int index);
 struct clk *of_clk_get_by_name(struct device_node *np, const char *name);
 struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec);
+int of_clk_shared_by_cpus(int cpu1, int cpu2);
 #else
 static inline struct clk *of_clk_get(struct device_node *np, int index)
 {
@@ -409,6 +410,11 @@ static inline struct clk *of_clk_get_by_name(struct device_node *np,
 {
 	return ERR_PTR(-ENOENT);
 }
+
+static inline int of_clk_shared_by_cpus(int cpu1, int cpu2)
+{
+	return -ENOSYS;
+}
 #endif
 
 #endif
-- 
2.0.0.rc2

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* [PATCH 03/14] cpufreq: Add support for per-policy driver data
  2014-07-01 16:32 [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
  2014-07-01 16:32 ` [PATCH 01/14] of: Create of_property_match() Viresh Kumar
  2014-07-01 16:32 ` [PATCH 02/14] clk: Create of_clk_shared_by_cpus() Viresh Kumar
@ 2014-07-01 16:32 ` Viresh Kumar
  2014-07-09 14:33     ` Santosh Shilimkar
  2014-07-01 16:32 ` [PATCH 04/14] cpufreq: cpu0: Add Module Author Viresh Kumar
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-01 16:32 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar

Drivers supporting multiple clusters or multiple 'struct cpufreq_policy'
instances may need to keep per-policy data. If the core doesn't support them,
they might do it in the most unoptimized way: 'per-cpu' data.

This patch adds another field in 'struct cpufreq_policy': 'driver_data'. It
isn't accessed by core and is for driver's internal use.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/cpufreq.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index ec4112d..d4b1108 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -112,6 +112,9 @@ struct cpufreq_policy {
 	spinlock_t		transition_lock;
 	wait_queue_head_t	transition_wait;
 	struct task_struct	*transition_task; /* Task which is doing the transition */
+
+	/* For cpufreq driver's internal use */
+	void			*driver_data;
 };
 
 /* Only for ACPI */
-- 
2.0.0.rc2

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* [PATCH 04/14] cpufreq: cpu0: Add Module Author
  2014-07-01 16:32 [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
                   ` (2 preceding siblings ...)
  2014-07-01 16:32 ` [PATCH 03/14] cpufreq: Add support for per-policy driver data Viresh Kumar
@ 2014-07-01 16:32 ` Viresh Kumar
  2014-07-09 14:42     ` Santosh Shilimkar
  2014-07-01 16:32 ` [PATCH 05/14] cpufreq: cpu0: don't validate clock on clk_put() Viresh Kumar
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-01 16:32 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar

Two people are maintaining it now, Viresh and Shawn. Add Viresh's details in
MODULE_AUTHOR() and copyright section.

Suggested-by: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq-cpu0.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index ee1ae30..7e191db 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -1,6 +1,9 @@
 /*
  * Copyright (C) 2012 Freescale Semiconductor, Inc.
  *
+ * Copyright (C) 2014 Linaro.
+ * Viresh Kumar <viresh.kumar@linaro.org>
+ *
  * The OPP code in function cpu0_set_target() is reused from
  * drivers/cpufreq/omap-cpufreq.c
  *
@@ -246,6 +249,7 @@ static struct platform_driver cpu0_cpufreq_platdrv = {
 };
 module_platform_driver(cpu0_cpufreq_platdrv);
 
+MODULE_AUTHOR("Viresh Kumar <viresh.kumar@linaro.org>");
 MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");
 MODULE_DESCRIPTION("Generic CPU0 cpufreq driver");
 MODULE_LICENSE("GPL");
-- 
2.0.0.rc2

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* [PATCH 05/14] cpufreq: cpu0: don't validate clock on clk_put()
  2014-07-01 16:32 [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
                   ` (3 preceding siblings ...)
  2014-07-01 16:32 ` [PATCH 04/14] cpufreq: cpu0: Add Module Author Viresh Kumar
@ 2014-07-01 16:32 ` Viresh Kumar
  2014-07-09 14:42     ` Santosh Shilimkar
  2014-07-01 16:32 ` [PATCH 06/14] cpufreq: cpu0: defer probe if clock isn't registered yet Viresh Kumar
                   ` (9 subsequent siblings)
  14 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-01 16:32 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar

CPU clk is not optional for this driver and probe would fail if it couldn't find
a suitable clock.

And so, while calling clk_put() we don't need to validate clocks.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq-cpu0.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index 7e191db..4273a5f 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -220,8 +220,7 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 out_free_table:
 	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table);
 out_put_clk:
-	if (!IS_ERR(cpu_clk))
-		clk_put(cpu_clk);
+	clk_put(cpu_clk);
 out_put_reg:
 	if (!IS_ERR(cpu_reg))
 		regulator_put(cpu_reg);
-- 
2.0.0.rc2

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* [PATCH 06/14] cpufreq: cpu0: defer probe if clock isn't registered yet
  2014-07-01 16:32 [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
                   ` (4 preceding siblings ...)
  2014-07-01 16:32 ` [PATCH 05/14] cpufreq: cpu0: don't validate clock on clk_put() Viresh Kumar
@ 2014-07-01 16:32 ` Viresh Kumar
  2014-07-02  5:53     ` Shawn Guo
  2014-07-01 16:32 ` [PATCH 07/14] cpufreq: cpu0: OPPs can be populated at runtime Viresh Kumar
                   ` (8 subsequent siblings)
  14 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-01 16:32 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar

Currently, we defer probe if regulator_get() returned -EPROBE_DEFER, i.e.
regulator isn't registered yet.

The same is true for clock as well, so lets defer in that case as well.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq-cpu0.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index 4273a5f..b5b8e1c 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -150,8 +150,17 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 
 	cpu_clk = clk_get(cpu_dev, NULL);
 	if (IS_ERR(cpu_clk)) {
-		ret = PTR_ERR(cpu_clk);
-		pr_err("failed to get cpu0 clock: %d\n", ret);
+		/*
+		 * If cpu's clk node is present, but clock is not yet
+		 * registered, we should try defering probe.
+		 */
+		if (PTR_ERR(cpu_clk) == -EPROBE_DEFER) {
+			dev_err(cpu_dev, "cpu0 clock not ready, retry\n");
+			ret = -EPROBE_DEFER;
+		} else {
+			ret = PTR_ERR(cpu_clk);
+			dev_err(cpu_dev, "failed to get cpu0 clock: %d\n", ret);
+		}
 		goto out_put_reg;
 	}
 
-- 
2.0.0.rc2

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* [PATCH 07/14] cpufreq: cpu0: OPPs can be populated at runtime
  2014-07-01 16:32 [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
                   ` (5 preceding siblings ...)
  2014-07-01 16:32 ` [PATCH 06/14] cpufreq: cpu0: defer probe if clock isn't registered yet Viresh Kumar
@ 2014-07-01 16:32 ` Viresh Kumar
  2014-07-01 18:02   ` Stephen Boyd
                     ` (2 more replies)
  2014-07-01 16:32 ` [PATCH 08/14] cpufreq: cpu0: use dev_{err|warn|dbg} instead of pr_{err|warn|debug} Viresh Kumar
                   ` (7 subsequent siblings)
  14 siblings, 3 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-01 16:32 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar

OPPs can be populated statically, via DT, or added at run time with
dev_pm_opp_add().

While this driver handles the first case correctly, it would fail to populate
OPPs added at runtime. Because call to of_init_opp_table() would fail as there
are no OPPs in DT and probe will return early.

To fix this, remove error checking and call dev_pm_opp_init_cpufreq_table()
unconditionally.

Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq-cpu0.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index b5b8e1c..f47f703 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -164,11 +164,8 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 		goto out_put_reg;
 	}
 
-	ret = of_init_opp_table(cpu_dev);
-	if (ret) {
-		pr_err("failed to init OPP table: %d\n", ret);
-		goto out_put_clk;
-	}
+	/* OPPs might be populated at runtime, don't check for error here */
+	of_init_opp_table(cpu_dev);
 
 	ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table);
 	if (ret) {
-- 
2.0.0.rc2


^ permalink raw reply related	[flat|nested] 83+ messages in thread

* [PATCH 08/14] cpufreq: cpu0: use dev_{err|warn|dbg} instead of pr_{err|warn|debug}
  2014-07-01 16:32 [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
                   ` (6 preceding siblings ...)
  2014-07-01 16:32 ` [PATCH 07/14] cpufreq: cpu0: OPPs can be populated at runtime Viresh Kumar
@ 2014-07-01 16:32 ` Viresh Kumar
  2014-07-09 14:45     ` Santosh Shilimkar
  2014-07-01 16:32 ` [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init() Viresh Kumar
                   ` (6 subsequent siblings)
  14 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-01 16:32 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar

We already have cpu_dev and is used at multiple places for printing errors using
dev_*(). But some prints are still using pr_*(). Lets make it consistent and
replace those pr_*() macros with dev_*() macros.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq-cpu0.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index f47f703..2b7b0ea 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -58,7 +58,8 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
 		opp = dev_pm_opp_find_freq_ceil(cpu_dev, &freq_Hz);
 		if (IS_ERR(opp)) {
 			rcu_read_unlock();
-			pr_err("failed to find OPP for %ld\n", freq_Hz);
+			dev_err(cpu_dev, "failed to find OPP for %ld\n",
+				freq_Hz);
 			return PTR_ERR(opp);
 		}
 		volt = dev_pm_opp_get_voltage(opp);
@@ -67,22 +68,23 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
 		volt_old = regulator_get_voltage(cpu_reg);
 	}
 
-	pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
-		 old_freq / 1000, volt_old ? volt_old / 1000 : -1,
-		 new_freq / 1000, volt ? volt / 1000 : -1);
+	dev_dbg(cpu_dev, "%u MHz, %ld mV --> %u MHz, %ld mV\n",
+		old_freq / 1000, volt_old ? volt_old / 1000 : -1,
+		new_freq / 1000, volt ? volt / 1000 : -1);
 
 	/* scaling up?  scale voltage before frequency */
 	if (!IS_ERR(cpu_reg) && new_freq > old_freq) {
 		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
 		if (ret) {
-			pr_err("failed to scale voltage up: %d\n", ret);
+			dev_err(cpu_dev, "failed to scale voltage up: %d\n",
+				ret);
 			return ret;
 		}
 	}
 
 	ret = clk_set_rate(cpu_clk, freq_exact);
 	if (ret) {
-		pr_err("failed to set clock rate: %d\n", ret);
+		dev_err(cpu_dev, "failed to set clock rate: %d\n", ret);
 		if (!IS_ERR(cpu_reg))
 			regulator_set_voltage_tol(cpu_reg, volt_old, tol);
 		return ret;
@@ -92,7 +94,8 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
 	if (!IS_ERR(cpu_reg) && new_freq < old_freq) {
 		ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
 		if (ret) {
-			pr_err("failed to scale voltage down: %d\n", ret);
+			dev_err(cpu_dev, "failed to scale voltage down: %d\n",
+				ret);
 			clk_set_rate(cpu_clk, old_freq * 1000);
 		}
 	}
@@ -129,7 +132,7 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 
 	np = of_node_get(cpu_dev->of_node);
 	if (!np) {
-		pr_err("failed to find cpu0 node\n");
+		dev_err(cpu_dev, "failed to find cpu0 node\n");
 		return -ENOENT;
 	}
 
@@ -144,8 +147,8 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 			ret = -EPROBE_DEFER;
 			goto out_put_node;
 		}
-		pr_warn("failed to get cpu0 regulator: %ld\n",
-			PTR_ERR(cpu_reg));
+		dev_warn(cpu_dev, "failed to get cpu0 regulator: %ld\n",
+			 PTR_ERR(cpu_reg));
 	}
 
 	cpu_clk = clk_get(cpu_dev, NULL);
@@ -169,7 +172,7 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 
 	ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table);
 	if (ret) {
-		pr_err("failed to init cpufreq table: %d\n", ret);
+		dev_err(cpu_dev, "failed to init cpufreq table: %d\n", ret);
 		goto out_put_clk;
 	}
 
@@ -205,7 +208,7 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 
 	ret = cpufreq_register_driver(&cpu0_cpufreq_driver);
 	if (ret) {
-		pr_err("failed register driver: %d\n", ret);
+		dev_err(cpu_dev, "failed to register driver: %d\n", ret);
 		goto out_free_table;
 	}
 
@@ -216,8 +219,9 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 	if (of_find_property(np, "#cooling-cells", NULL)) {
 		cdev = of_cpufreq_cooling_register(np, cpu_present_mask);
 		if (IS_ERR(cdev))
-			pr_err("running cpufreq without cooling device: %ld\n",
-			       PTR_ERR(cdev));
+			dev_err(cpu_dev,
+				"running cpufreq without cooling device: %ld\n",
+				PTR_ERR(cdev));
 	}
 
 	of_node_put(np);
-- 
2.0.0.rc2

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
  2014-07-01 16:32 [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
                   ` (7 preceding siblings ...)
  2014-07-01 16:32 ` [PATCH 08/14] cpufreq: cpu0: use dev_{err|warn|dbg} instead of pr_{err|warn|debug} Viresh Kumar
@ 2014-07-01 16:32 ` Viresh Kumar
  2014-07-03  0:43   ` Stephen Boyd
  2014-07-09 14:53     ` Santosh Shilimkar
  2014-07-01 16:32 ` [PATCH 10/14] cpufreq: cpu0: try regulators with name "cpu-supply" Viresh Kumar
                   ` (5 subsequent siblings)
  14 siblings, 2 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-01 16:32 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar

Currently this driver only support platforms on which all CPUs share clock &
voltage lines and there is requirement to support platforms which have separate
clock & voltage lines for CPUs, like Qualcomm's Krait and ARM's big LITTLE.

Each group of CPUs sharing clock/voltage lines are represented by 'struct
cpufreq_policy' in cpufreq framework. And core calls ->init() once for each
policy.

Currently we do all initialization/allocation from probe() which wouldn't work
for above scenario. To make it work for these platforms, the first step is to
move all initialization/allocation to ->init() and add ->exit() to do the
reverse of it.

Also, remove all global variables and allocate space for them at runtime.

This patch creates 'struct private_data' for keeping all such information and
a pointer to that would be stored in policy->driver_data.

The changed probe() routine now tries to see if regulator/clocks are available
or we need to defer probe. In case they are available, it registers cpufreq
driver. Otherwise, returns with -EPROBE_DEFER.

We still *don't* support platforms with separate clock/voltage lines for CPUs.
This would be done in a separate patch.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq-cpu0.c | 189 +++++++++++++++++++++++++++++------------
 1 file changed, 136 insertions(+), 53 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index 2b7b0ea..15b8e7a 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -28,18 +28,21 @@
 #include <linux/slab.h>
 #include <linux/thermal.h>
 
-static unsigned int transition_latency;
-static unsigned int voltage_tolerance; /* in percentage */
-
-static struct device *cpu_dev;
-static struct clk *cpu_clk;
-static struct regulator *cpu_reg;
-static struct cpufreq_frequency_table *freq_table;
-static struct thermal_cooling_device *cdev;
+struct private_data {
+	struct device *cpu_dev;
+	struct regulator *cpu_reg;
+	struct thermal_cooling_device *cdev;
+	unsigned int voltage_tolerance; /* in percentage */
+};
 
 static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
 {
 	struct dev_pm_opp *opp;
+	struct cpufreq_frequency_table *freq_table = policy->freq_table;
+	struct clk *cpu_clk = policy->clk;
+	struct private_data *priv = policy->driver_data;
+	struct device *cpu_dev = priv->cpu_dev;
+	struct regulator *cpu_reg = priv->cpu_reg;
 	unsigned long volt = 0, volt_old = 0, tol = 0;
 	unsigned int old_freq, new_freq;
 	long freq_Hz, freq_exact;
@@ -64,7 +67,7 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
 		}
 		volt = dev_pm_opp_get_voltage(opp);
 		rcu_read_unlock();
-		tol = volt * voltage_tolerance / 100;
+		tol = volt * priv->voltage_tolerance / 100;
 		volt_old = regulator_get_voltage(cpu_reg);
 	}
 
@@ -103,26 +106,13 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
 	return ret;
 }
 
-static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
-{
-	policy->clk = cpu_clk;
-	return cpufreq_generic_init(policy, freq_table, transition_latency);
-}
-
-static struct cpufreq_driver cpu0_cpufreq_driver = {
-	.flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK,
-	.verify = cpufreq_generic_frequency_table_verify,
-	.target_index = cpu0_set_target,
-	.get = cpufreq_generic_get,
-	.init = cpu0_cpufreq_init,
-	.name = "generic_cpu0",
-	.attr = cpufreq_generic_attr,
-};
-
-static int cpu0_cpufreq_probe(struct platform_device *pdev)
+static int allocate_resources(struct device **cdev,
+			      struct regulator **creg, struct clk **cclk)
 {
-	struct device_node *np;
-	int ret;
+	struct device *cpu_dev;
+	struct regulator *cpu_reg;
+	struct clk *cpu_clk;
+	int ret = 0;
 
 	cpu_dev = get_cpu_device(0);
 	if (!cpu_dev) {
@@ -130,12 +120,6 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	np = of_node_get(cpu_dev->of_node);
-	if (!np) {
-		dev_err(cpu_dev, "failed to find cpu0 node\n");
-		return -ENOENT;
-	}
-
 	cpu_reg = regulator_get_optional(cpu_dev, "cpu0");
 	if (IS_ERR(cpu_reg)) {
 		/*
@@ -144,8 +128,7 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 		 */
 		if (PTR_ERR(cpu_reg) == -EPROBE_DEFER) {
 			dev_err(cpu_dev, "cpu0 regulator not ready, retry\n");
-			ret = -EPROBE_DEFER;
-			goto out_put_node;
+			return -EPROBE_DEFER;
 		}
 		dev_warn(cpu_dev, "failed to get cpu0 regulator: %ld\n",
 			 PTR_ERR(cpu_reg));
@@ -153,6 +136,10 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 
 	cpu_clk = clk_get(cpu_dev, NULL);
 	if (IS_ERR(cpu_clk)) {
+		/* put regulator */
+		if (!IS_ERR(cpu_reg))
+			regulator_put(cpu_reg);
+
 		/*
 		 * If cpu's clk node is present, but clock is not yet
 		 * registered, we should try defering probe.
@@ -164,7 +151,39 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 			ret = PTR_ERR(cpu_clk);
 			dev_err(cpu_dev, "failed to get cpu0 clock: %d\n", ret);
 		}
-		goto out_put_reg;
+	} else {
+		*cdev = cpu_dev;
+		*creg = cpu_reg;
+		*cclk = cpu_clk;
+	}
+
+	return ret;
+}
+
+static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
+{
+	struct cpufreq_frequency_table *freq_table;
+	struct thermal_cooling_device *cdev;
+	struct device_node *np;
+	struct private_data *priv;
+	struct device *cpu_dev;
+	struct regulator *cpu_reg;
+	struct clk *cpu_clk;
+	unsigned int transition_latency;
+	int ret;
+
+	/* We only support cpu0 currently */
+	ret = allocate_resources(&cpu_dev, &cpu_reg, &cpu_clk);
+	if (ret) {
+		pr_err("%s: Failed to allocate resources\n: %d", __func__, ret);
+		return ret;
+	}
+
+	np = of_node_get(cpu_dev->of_node);
+	if (!np) {
+		dev_err(cpu_dev, "failed to find cpu%d node\n", policy->cpu);
+		ret = -ENOENT;
+		goto out_put_reg_clk;
 	}
 
 	/* OPPs might be populated at runtime, don't check for error here */
@@ -173,10 +192,16 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 	ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table);
 	if (ret) {
 		dev_err(cpu_dev, "failed to init cpufreq table: %d\n", ret);
-		goto out_put_clk;
+		goto out_put_node;
+	}
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto out_free_table;
 	}
 
-	of_property_read_u32(np, "voltage-tolerance", &voltage_tolerance);
+	of_property_read_u32(np, "voltage-tolerance", &priv->voltage_tolerance);
 
 	if (of_property_read_u32(np, "clock-latency", &transition_latency))
 		transition_latency = CPUFREQ_ETERNAL;
@@ -206,12 +231,6 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 			transition_latency += ret * 1000;
 	}
 
-	ret = cpufreq_register_driver(&cpu0_cpufreq_driver);
-	if (ret) {
-		dev_err(cpu_dev, "failed to register driver: %d\n", ret);
-		goto out_free_table;
-	}
-
 	/*
 	 * For now, just loading the cooling device;
 	 * thermal DT code takes care of matching them.
@@ -223,28 +242,92 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 				"running cpufreq without cooling device: %ld\n",
 				PTR_ERR(cdev));
 	}
-
 	of_node_put(np);
+
+	priv->cdev = cdev;
+	priv->cpu_dev = cpu_dev;
+	priv->cpu_reg = cpu_reg;
+	policy->driver_data = priv;
+
+	policy->clk = cpu_clk;
+	ret = cpufreq_generic_init(policy, freq_table, transition_latency);
+	if (ret)
+		goto out_cooling_unregister;
+
 	return 0;
 
+out_cooling_unregister:
+	cpufreq_cooling_unregister(priv->cdev);
+	kfree(priv);
 out_free_table:
 	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table);
-out_put_clk:
+out_put_node:
+	of_node_put(np);
+out_put_reg_clk:
 	clk_put(cpu_clk);
-out_put_reg:
 	if (!IS_ERR(cpu_reg))
 		regulator_put(cpu_reg);
-out_put_node:
-	of_node_put(np);
+
+	return ret;
+}
+
+static int cpu0_cpufreq_exit(struct cpufreq_policy *policy)
+{
+	struct private_data *priv = policy->driver_data;
+
+	cpufreq_cooling_unregister(priv->cdev);
+	dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table);
+	clk_put(policy->clk);
+	if (!IS_ERR(priv->cpu_reg))
+		regulator_put(priv->cpu_reg);
+	kfree(priv);
+
+	return 0;
+}
+
+static struct cpufreq_driver cpu0_cpufreq_driver = {
+	.flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK,
+	.verify = cpufreq_generic_frequency_table_verify,
+	.target_index = cpu0_set_target,
+	.get = cpufreq_generic_get,
+	.init = cpu0_cpufreq_init,
+	.exit = cpu0_cpufreq_exit,
+	.name = "generic_cpu0",
+	.attr = cpufreq_generic_attr,
+};
+
+static int cpu0_cpufreq_probe(struct platform_device *pdev)
+{
+	struct device *cpu_dev;
+	struct regulator *cpu_reg;
+	struct clk *cpu_clk;
+	int ret;
+
+	/*
+	 * All per-cluster (CPUs sharing clock/voltages) initialization is done
+	 * from ->init(). In probe(), we just need to make sure that clk and
+	 * regulators are available. Else defer probe and retry.
+	 *
+	 * FIXME: Is checking this only for CPU0 sufficient ?
+	 */
+	ret = allocate_resources(&cpu_dev, &cpu_reg, &cpu_clk);
+	if (ret)
+		return ret;
+
+	clk_put(cpu_clk);
+	if (!IS_ERR(cpu_reg))
+		regulator_put(cpu_reg);
+
+	ret = cpufreq_register_driver(&cpu0_cpufreq_driver);
+	if (ret)
+		dev_err(cpu_dev, "failed register driver: %d\n", ret);
+
 	return ret;
 }
 
 static int cpu0_cpufreq_remove(struct platform_device *pdev)
 {
-	cpufreq_cooling_unregister(cdev);
 	cpufreq_unregister_driver(&cpu0_cpufreq_driver);
-	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table);
-
 	return 0;
 }
 
-- 
2.0.0.rc2

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* [PATCH 10/14] cpufreq: cpu0: try regulators with name "cpu-supply"
  2014-07-01 16:32 [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
                   ` (8 preceding siblings ...)
  2014-07-01 16:32 ` [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init() Viresh Kumar
@ 2014-07-01 16:32 ` Viresh Kumar
  2014-07-01 16:32 ` [PATCH 11/14] cpufreq: cpu0: Make allocate_resources() work for any CPU Viresh Kumar
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-01 16:32 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar

Currently, we expect regulator name to be "cpu0", but as we are going to support
multiple cpu-blocks (all CPUs in a block share clock/voltage) later, we need to
pass some generic string instead of that.

For backwards compatibility try for "cpu0" first and if it fails, then try for
"cpu".

Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq-cpu0.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index 15b8e7a..17001a8 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -113,6 +113,7 @@ static int allocate_resources(struct device **cdev,
 	struct regulator *cpu_reg;
 	struct clk *cpu_clk;
 	int ret = 0;
+	char *reg_cpu0 = "cpu0", *reg_cpu = "cpu", *reg;
 
 	cpu_dev = get_cpu_device(0);
 	if (!cpu_dev) {
@@ -120,7 +121,11 @@ static int allocate_resources(struct device **cdev,
 		return -ENODEV;
 	}
 
-	cpu_reg = regulator_get_optional(cpu_dev, "cpu0");
+	/* Try "cpu0" for older DTs */
+	reg = reg_cpu0;
+
+try_again:
+	cpu_reg = regulator_get_optional(cpu_dev, reg);
 	if (IS_ERR(cpu_reg)) {
 		/*
 		 * If cpu0 regulator supply node is present, but regulator is
@@ -130,6 +135,13 @@ static int allocate_resources(struct device **cdev,
 			dev_err(cpu_dev, "cpu0 regulator not ready, retry\n");
 			return -EPROBE_DEFER;
 		}
+
+		/* Try with "cpu-supply" */
+		if (reg == reg_cpu0) {
+			reg = reg_cpu;
+			goto try_again;
+		}
+
 		dev_warn(cpu_dev, "failed to get cpu0 regulator: %ld\n",
 			 PTR_ERR(cpu_reg));
 	}
-- 
2.0.0.rc2

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* [PATCH 11/14] cpufreq: cpu0: Make allocate_resources() work for any CPU
  2014-07-01 16:32 [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
                   ` (9 preceding siblings ...)
  2014-07-01 16:32 ` [PATCH 10/14] cpufreq: cpu0: try regulators with name "cpu-supply" Viresh Kumar
@ 2014-07-01 16:32 ` Viresh Kumar
  2014-07-01 16:32 ` [PATCH 12/14] cpufreq: cpu0: Extend support beyond CPU0 Viresh Kumar
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-01 16:32 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar

Currently allocate_resources() supports only CPU0 and it would need to allocate
resources for any CPU going forward.

Add another argument to it, i.e. cpu, and update code accordingly.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq-cpu0.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index 17001a8..44633f6 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -106,7 +106,7 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
 	return ret;
 }
 
-static int allocate_resources(struct device **cdev,
+static int allocate_resources(int cpu, struct device **cdev,
 			      struct regulator **creg, struct clk **cclk)
 {
 	struct device *cpu_dev;
@@ -115,24 +115,28 @@ static int allocate_resources(struct device **cdev,
 	int ret = 0;
 	char *reg_cpu0 = "cpu0", *reg_cpu = "cpu", *reg;
 
-	cpu_dev = get_cpu_device(0);
+	cpu_dev = get_cpu_device(cpu);
 	if (!cpu_dev) {
-		pr_err("failed to get cpu0 device\n");
+		pr_err("failed to get cpu%d device\n", cpu);
 		return -ENODEV;
 	}
 
 	/* Try "cpu0" for older DTs */
-	reg = reg_cpu0;
+	if (!cpu)
+		reg = reg_cpu0;
+	else
+		reg = reg_cpu;
 
 try_again:
 	cpu_reg = regulator_get_optional(cpu_dev, reg);
 	if (IS_ERR(cpu_reg)) {
 		/*
-		 * If cpu0 regulator supply node is present, but regulator is
+		 * If cpu's regulator supply node is present, but regulator is
 		 * not yet registered, we should try defering probe.
 		 */
 		if (PTR_ERR(cpu_reg) == -EPROBE_DEFER) {
-			dev_err(cpu_dev, "cpu0 regulator not ready, retry\n");
+			dev_err(cpu_dev, "cpu%d regulator not ready, retry\n",
+				cpu);
 			return -EPROBE_DEFER;
 		}
 
@@ -142,8 +146,8 @@ try_again:
 			goto try_again;
 		}
 
-		dev_warn(cpu_dev, "failed to get cpu0 regulator: %ld\n",
-			 PTR_ERR(cpu_reg));
+		dev_warn(cpu_dev, "failed to get cpu%d regulator: %ld\n",
+			 cpu, PTR_ERR(cpu_reg));
 	}
 
 	cpu_clk = clk_get(cpu_dev, NULL);
@@ -157,11 +161,12 @@ try_again:
 		 * registered, we should try defering probe.
 		 */
 		if (PTR_ERR(cpu_clk) == -EPROBE_DEFER) {
-			dev_err(cpu_dev, "cpu0 clock not ready, retry\n");
+			dev_err(cpu_dev, "cpu%d clock not ready, retry\n", cpu);
 			ret = -EPROBE_DEFER;
 		} else {
 			ret = PTR_ERR(cpu_clk);
-			dev_err(cpu_dev, "failed to get cpu0 clock: %d\n", ret);
+			dev_err(cpu_dev, "failed to get cpu%d clock: %d\n", ret,
+				cpu);
 		}
 	} else {
 		*cdev = cpu_dev;
@@ -184,8 +189,7 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
 	unsigned int transition_latency;
 	int ret;
 
-	/* We only support cpu0 currently */
-	ret = allocate_resources(&cpu_dev, &cpu_reg, &cpu_clk);
+	ret = allocate_resources(policy->cpu, &cpu_dev, &cpu_reg, &cpu_clk);
 	if (ret) {
 		pr_err("%s: Failed to allocate resources\n: %d", __func__, ret);
 		return ret;
@@ -322,7 +326,7 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 	 *
 	 * FIXME: Is checking this only for CPU0 sufficient ?
 	 */
-	ret = allocate_resources(&cpu_dev, &cpu_reg, &cpu_clk);
+	ret = allocate_resources(0, &cpu_dev, &cpu_reg, &cpu_clk);
 	if (ret)
 		return ret;
 
-- 
2.0.0.rc2

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* [PATCH 12/14] cpufreq: cpu0: Extend support beyond CPU0
  2014-07-01 16:32 [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
                   ` (10 preceding siblings ...)
  2014-07-01 16:32 ` [PATCH 11/14] cpufreq: cpu0: Make allocate_resources() work for any CPU Viresh Kumar
@ 2014-07-01 16:32 ` Viresh Kumar
  2014-07-02  4:03   ` [PATCH V2 Resend " Viresh Kumar
  2014-07-01 16:32 ` [PATCH 13/14] cpufreq: cpu0: rename driver and internals to 'cpufreq_generic' Viresh Kumar
                   ` (2 subsequent siblings)
  14 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-01 16:32 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar,
	devicetree

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

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* [PATCH 13/14] cpufreq: cpu0: rename driver and internals to 'cpufreq_generic'
  2014-07-01 16:32 [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
                   ` (11 preceding siblings ...)
  2014-07-01 16:32 ` [PATCH 12/14] cpufreq: cpu0: Extend support beyond CPU0 Viresh Kumar
@ 2014-07-01 16:32 ` Viresh Kumar
  2014-07-01 16:32 ` [PATCH 14/14] cpufreq: generic: set platform_{driver|device} '.name' to 'cpufreq-generic' Viresh Kumar
  2014-07-02  4:12 ` [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
  14 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-01 16:32 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar

This is not a cpu0 driver anymore as it supports any number of clusters with
separate clock/voltage lines.

Lets rename it to 'cpufreq_generic' from 'cpufreq_cpu0'.

'.name' field of platform driver/devices isn't renamed yet, would be done
separately.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .../{cpufreq-cpu0.txt => cpufreq-generic.txt}      |  0
 drivers/cpufreq/Kconfig                            | 10 +++---
 drivers/cpufreq/Kconfig.arm                        |  2 +-
 drivers/cpufreq/Makefile                           |  2 +-
 .../cpufreq/{cpufreq-cpu0.c => cpufreq-generic.c}  | 36 +++++++++++-----------
 5 files changed, 25 insertions(+), 25 deletions(-)
 rename Documentation/devicetree/bindings/cpufreq/{cpufreq-cpu0.txt => cpufreq-generic.txt} (100%)
 rename drivers/cpufreq/{cpufreq-cpu0.c => cpufreq-generic.c} (91%)

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-generic.txt
similarity index 100%
rename from Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
rename to Documentation/devicetree/bindings/cpufreq/cpufreq-generic.txt
diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index ffe350f..22b42d5 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -183,16 +183,16 @@ config CPU_FREQ_GOV_CONSERVATIVE
 
 	  If in doubt, say N.
 
-config GENERIC_CPUFREQ_CPU0
-	tristate "Generic CPU0 cpufreq driver"
+config CPUFREQ_GENERIC
+	tristate "Generic cpufreq driver"
 	depends on HAVE_CLK && OF
-	# if CPU_THERMAL is on and THERMAL=m, CPU0 cannot be =y:
+	# if CPU_THERMAL is on and THERMAL=m, GENERIC cannot be =y:
 	depends on !CPU_THERMAL || THERMAL
 	select PM_OPP
 	help
-	  This adds a generic cpufreq driver for CPU0 frequency management.
+	  This adds 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.
 
 	  If in doubt, say N.
 
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index ebac671..16bdd31 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -92,7 +92,7 @@ config ARM_EXYNOS_CPU_FREQ_BOOST_SW
 
 config ARM_HIGHBANK_CPUFREQ
 	tristate "Calxeda Highbank-based"
-	depends on ARCH_HIGHBANK && GENERIC_CPUFREQ_CPU0 && REGULATOR
+	depends on ARCH_HIGHBANK && CPUFREQ_GENERIC && REGULATOR
 	default m
 	help
 	  This adds the CPUFreq driver for Calxeda Highbank SoC
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 738c8b7..0d0cddc 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -13,7 +13,7 @@ obj-$(CONFIG_CPU_FREQ_GOV_ONDEMAND)	+= cpufreq_ondemand.o
 obj-$(CONFIG_CPU_FREQ_GOV_CONSERVATIVE)	+= cpufreq_conservative.o
 obj-$(CONFIG_CPU_FREQ_GOV_COMMON)		+= cpufreq_governor.o
 
-obj-$(CONFIG_GENERIC_CPUFREQ_CPU0)	+= cpufreq-cpu0.o
+obj-$(CONFIG_CPUFREQ_GENERIC)		+= cpufreq-generic.o
 
 ##################################################################################
 # x86 drivers.
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-generic.c
similarity index 91%
rename from drivers/cpufreq/cpufreq-cpu0.c
rename to drivers/cpufreq/cpufreq-generic.c
index b3f2bf0..ac8fd9f 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-generic.c
@@ -4,7 +4,7 @@
  * Copyright (C) 2014 Linaro.
  * Viresh Kumar <viresh.kumar@linaro.org>
  *
- * The OPP code in function cpu0_set_target() is reused from
+ * The OPP code in function set_target() is reused from
  * drivers/cpufreq/omap-cpufreq.c
  *
  * This program is free software; you can redistribute it and/or modify
@@ -35,7 +35,7 @@ struct private_data {
 	unsigned int voltage_tolerance; /* in percentage */
 };
 
-static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
+static int set_target(struct cpufreq_policy *policy, unsigned int index)
 {
 	struct dev_pm_opp *opp;
 	struct cpufreq_frequency_table *freq_table = policy->freq_table;
@@ -201,7 +201,7 @@ static void find_siblings(struct cpufreq_policy *policy)
 	}
 }
 
-static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
+static int cpufreq_init(struct cpufreq_policy *policy)
 {
 	struct cpufreq_frequency_table *freq_table;
 	struct thermal_cooling_device *cdev;
@@ -318,7 +318,7 @@ out_put_reg_clk:
 	return ret;
 }
 
-static int cpu0_cpufreq_exit(struct cpufreq_policy *policy)
+static int cpufreq_exit(struct cpufreq_policy *policy)
 {
 	struct private_data *priv = policy->driver_data;
 
@@ -332,18 +332,18 @@ static int cpu0_cpufreq_exit(struct cpufreq_policy *policy)
 	return 0;
 }
 
-static struct cpufreq_driver cpu0_cpufreq_driver = {
+static struct cpufreq_driver generic_cpufreq_driver = {
 	.flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK,
 	.verify = cpufreq_generic_frequency_table_verify,
-	.target_index = cpu0_set_target,
+	.target_index = set_target,
 	.get = cpufreq_generic_get,
-	.init = cpu0_cpufreq_init,
-	.exit = cpu0_cpufreq_exit,
-	.name = "generic_cpu0",
+	.init = cpufreq_init,
+	.exit = cpufreq_exit,
+	.name = "cpufreq-generic",
 	.attr = cpufreq_generic_attr,
 };
 
-static int cpu0_cpufreq_probe(struct platform_device *pdev)
+static int generic_cpufreq_probe(struct platform_device *pdev)
 {
 	struct device *cpu_dev;
 	struct regulator *cpu_reg;
@@ -365,30 +365,30 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 	if (!IS_ERR(cpu_reg))
 		regulator_put(cpu_reg);
 
-	ret = cpufreq_register_driver(&cpu0_cpufreq_driver);
+	ret = cpufreq_register_driver(&generic_cpufreq_driver);
 	if (ret)
 		dev_err(cpu_dev, "failed register driver: %d\n", ret);
 
 	return ret;
 }
 
-static int cpu0_cpufreq_remove(struct platform_device *pdev)
+static int generic_cpufreq_remove(struct platform_device *pdev)
 {
-	cpufreq_unregister_driver(&cpu0_cpufreq_driver);
+	cpufreq_unregister_driver(&generic_cpufreq_driver);
 	return 0;
 }
 
-static struct platform_driver cpu0_cpufreq_platdrv = {
+static struct platform_driver generic_cpufreq_platdrv = {
 	.driver = {
 		.name	= "cpufreq-cpu0",
 		.owner	= THIS_MODULE,
 	},
-	.probe		= cpu0_cpufreq_probe,
-	.remove		= cpu0_cpufreq_remove,
+	.probe		= generic_cpufreq_probe,
+	.remove		= generic_cpufreq_remove,
 };
-module_platform_driver(cpu0_cpufreq_platdrv);
+module_platform_driver(generic_cpufreq_platdrv);
 
 MODULE_AUTHOR("Viresh Kumar <viresh.kumar@linaro.org>");
 MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");
-MODULE_DESCRIPTION("Generic CPU0 cpufreq driver");
+MODULE_DESCRIPTION("Generic cpufreq driver");
 MODULE_LICENSE("GPL");
-- 
2.0.0.rc2


^ permalink raw reply related	[flat|nested] 83+ messages in thread

* [PATCH 14/14] cpufreq: generic: set platform_{driver|device} '.name' to 'cpufreq-generic'
  2014-07-01 16:32 [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
                   ` (12 preceding siblings ...)
  2014-07-01 16:32 ` [PATCH 13/14] cpufreq: cpu0: rename driver and internals to 'cpufreq_generic' Viresh Kumar
@ 2014-07-01 16:32 ` Viresh Kumar
  2014-07-02  4:12 ` [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
  14 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-01 16:32 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar,
	Shawn Guo, Santosh Shilimkar, Simon Horman, Michal Simek,
	Kukjin Kim, Rob Herring

We have already renamed cpufreq-cpu0 to cpufreq-generic at most of the places,
the only one left is in the '.name' field of platform driver and devices.

Lets do it now for all users.

Updates cpufreq-cpu0 from comments as well.

Cc: Shawn Guo <shawn.guo@freescale.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Simon Horman <horms@verge.net.au>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Rob Herring <rob.herring@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
@Santosh: Probably you can drop omap's cpufreq driver now?

 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/cpufreq/cpufreq-generic.c                | 2 +-
 drivers/cpufreq/exynos4x12-cpufreq.c             | 2 +-
 drivers/cpufreq/highbank-cpufreq.c               | 6 +++---
 9 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-imx/imx27-dt.c b/arch/arm/mach-imx/imx27-dt.c
index 17bd405..02376e1 100644
--- a/arch/arm/mach-imx/imx27-dt.c
+++ b/arch/arm/mach-imx/imx27-dt.c
@@ -20,7 +20,7 @@
 
 static void __init imx27_dt_init(void)
 {
-	struct platform_device_info devinfo = { .name = "cpufreq-cpu0", };
+	struct platform_device_info devinfo = { .name = "cpufreq-generic", };
 
 	mxc_arch_reset_init_dt();
 
diff --git a/arch/arm/mach-imx/imx51-dt.c b/arch/arm/mach-imx/imx51-dt.c
index b8cd968..66d415a 100644
--- a/arch/arm/mach-imx/imx51-dt.c
+++ b/arch/arm/mach-imx/imx51-dt.c
@@ -21,7 +21,7 @@
 
 static void __init imx51_dt_init(void)
 {
-	struct platform_device_info devinfo = { .name = "cpufreq-cpu0", };
+	struct platform_device_info devinfo = { .name = "cpufreq-generic", };
 
 	mxc_arch_reset_init_dt();
 
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 828aee9..7ec2fbb 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -282,7 +282,7 @@ static inline void omap_init_cpufreq(void)
 	if (!of_have_populated_dt())
 		devinfo.name = "omap-cpufreq";
 	else
-		devinfo.name = "cpufreq-cpu0";
+		devinfo.name = "cpufreq-generic";
 	platform_device_register_full(&devinfo);
 }
 
diff --git a/arch/arm/mach-shmobile/board-ape6evm-reference.c b/arch/arm/mach-shmobile/board-ape6evm-reference.c
index 3276afc..94eb59d 100644
--- a/arch/arm/mach-shmobile/board-ape6evm-reference.c
+++ b/arch/arm/mach-shmobile/board-ape6evm-reference.c
@@ -48,7 +48,7 @@ static void __init ape6evm_add_standard_devices(void)
 
 	r8a73a4_add_dt_devices();
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
-	platform_device_register_simple("cpufreq-cpu0", -1, NULL, 0);
+	platform_device_register_simple("cpufreq-generic", -1, NULL, 0);
 }
 
 static const char *ape6evm_boards_compat_dt[] __initdata = {
diff --git a/arch/arm/mach-shmobile/setup-sh73a0.c b/arch/arm/mach-shmobile/setup-sh73a0.c
index ad00724..360eace 100644
--- a/arch/arm/mach-shmobile/setup-sh73a0.c
+++ b/arch/arm/mach-shmobile/setup-sh73a0.c
@@ -774,7 +774,7 @@ void __init sh73a0_add_early_devices(void)
 
 void __init sh73a0_add_standard_devices_dt(void)
 {
-	struct platform_device_info devinfo = { .name = "cpufreq-cpu0", .id = -1, };
+	struct platform_device_info devinfo = { .name = "cpufreq-generic", .id = -1, };
 
 	/* clocks are setup late during boot in the case of DT */
 	sh73a0_clock_init();
@@ -783,7 +783,7 @@ void __init sh73a0_add_standard_devices_dt(void)
 			     ARRAY_SIZE(sh73a0_devices_dt));
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 
-	/* Instantiate cpufreq-cpu0 */
+	/* Instantiate cpufreq-generic */
 	platform_device_register_full(&devinfo);
 }
 
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index 31a6fa4..79f3c61 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -104,7 +104,7 @@ static int __init zynq_get_revision(void)
  */
 static void __init zynq_init_machine(void)
 {
-	struct platform_device_info devinfo = { .name = "cpufreq-cpu0", };
+	struct platform_device_info devinfo = { .name = "cpufreq-generic", };
 	struct soc_device_attribute *soc_dev_attr;
 	struct soc_device *soc_dev;
 	struct device *parent = NULL;
diff --git a/drivers/cpufreq/cpufreq-generic.c b/drivers/cpufreq/cpufreq-generic.c
index ac8fd9f..d47b4a3 100644
--- a/drivers/cpufreq/cpufreq-generic.c
+++ b/drivers/cpufreq/cpufreq-generic.c
@@ -380,7 +380,7 @@ static int generic_cpufreq_remove(struct platform_device *pdev)
 
 static struct platform_driver generic_cpufreq_platdrv = {
 	.driver = {
-		.name	= "cpufreq-cpu0",
+		.name	= "cpufreq-generic",
 		.owner	= THIS_MODULE,
 	},
 	.probe		= generic_cpufreq_probe,
diff --git a/drivers/cpufreq/exynos4x12-cpufreq.c b/drivers/cpufreq/exynos4x12-cpufreq.c
index 351a207..7bbe5ef 100644
--- a/drivers/cpufreq/exynos4x12-cpufreq.c
+++ b/drivers/cpufreq/exynos4x12-cpufreq.c
@@ -174,7 +174,7 @@ int exynos4x12_cpufreq_init(struct exynos_dvfs_info *info)
 	 * dependencies on platform headers. It is necessary to enable
 	 * Exynos multi-platform support and will be removed together with
 	 * this whole driver as soon as Exynos gets migrated to use
-	 * cpufreq-cpu0 driver.
+	 * cpufreq-generic driver.
 	 */
 	np = of_find_compatible_node(NULL, NULL, "samsung,exynos4412-clock");
 	if (!np) {
diff --git a/drivers/cpufreq/highbank-cpufreq.c b/drivers/cpufreq/highbank-cpufreq.c
index bf8902a..dc7a9ab 100644
--- a/drivers/cpufreq/highbank-cpufreq.c
+++ b/drivers/cpufreq/highbank-cpufreq.c
@@ -6,7 +6,7 @@
  * published by the Free Software Foundation.
  *
  * This driver provides the clk notifier callbacks that are used when
- * the cpufreq-cpu0 driver changes to frequency to alert the highbank
+ * the cpufreq-generic driver changes to frequency to alert the highbank
  * EnergyCore Management Engine (ECME) about the need to change
  * voltage. The ECME interfaces with the actual voltage regulators.
  */
@@ -60,7 +60,7 @@ static struct notifier_block hb_cpufreq_clk_nb = {
 
 static int hb_cpufreq_driver_init(void)
 {
-	struct platform_device_info devinfo = { .name = "cpufreq-cpu0", };
+	struct platform_device_info devinfo = { .name = "cpufreq-generic", };
 	struct device *cpu_dev;
 	struct clk *cpu_clk;
 	struct device_node *np;
@@ -95,7 +95,7 @@ static int hb_cpufreq_driver_init(void)
 		goto out_put_node;
 	}
 
-	/* Instantiate cpufreq-cpu0 */
+	/* Instantiate cpufreq-generic */
 	platform_device_register_full(&devinfo);
 
 out_put_node:
-- 
2.0.0.rc2

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* Re: [PATCH 02/14] clk: Create of_clk_shared_by_cpus()
  2014-07-01 16:32 ` [PATCH 02/14] clk: Create of_clk_shared_by_cpus() Viresh Kumar
@ 2014-07-01 18:00   ` Stephen Boyd
  2014-07-02  1:57     ` Viresh Kumar
  2014-07-09 14:38     ` Santosh Shilimkar
       [not found]   ` <5f7164d789e87c62d722b575980c92dfd0504334.1404231535.git.viresh.kumar@linar o.org>
  2 siblings, 1 reply; 83+ messages in thread
From: Stephen Boyd @ 2014-07-01 18:00 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rjw, shawn.guo, linaro-kernel, linux-pm, linux-kernel,
	arvind.chauhan, linux-arm-msm, spk.linux, thomas.ab, nm, t.figa,
	Mike Turquette

On 07/01/14 09:32, Viresh Kumar wrote:
> Create a new routine of_clk_shared_by_cpus() that finds if clock lines are
> shared between two CPUs. This is verified by comparing "clocks" property from
> CPU's DT node.
>
> Returns 1 if clock line is shared between them, 0 if clock isn't shared and
> return appropriate errors in case nodes/properties are missing.
>
> Cc: Mike Turquette <mturquette@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/clk/clk.c   | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/clk.h |  6 ++++++

This doesn't make much sense to me. This function doesn't deal with
struct clk pointers or any of the internals of the common clock
framework so why put it in clk.c? It looks more like an internal
function that the cpufreq-generic driver should have.

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


^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 07/14] cpufreq: cpu0: OPPs can be populated at runtime
  2014-07-01 16:32 ` [PATCH 07/14] cpufreq: cpu0: OPPs can be populated at runtime Viresh Kumar
@ 2014-07-01 18:02   ` Stephen Boyd
  2014-07-02  2:03     ` Viresh Kumar
  2014-07-02  4:03   ` [PATCH V2 Resend " Viresh Kumar
  2014-07-09 14:44     ` Santosh Shilimkar
  2 siblings, 1 reply; 83+ messages in thread
From: Stephen Boyd @ 2014-07-01 18:02 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rjw, shawn.guo, linaro-kernel, linux-pm, linux-kernel,
	arvind.chauhan, linux-arm-msm, spk.linux, thomas.ab, nm, t.figa

On 07/01/14 09:32, Viresh Kumar wrote:
> OPPs can be populated statically, via DT, or added at run time with
> dev_pm_opp_add().
>
> While this driver handles the first case correctly, it would fail to populate
> OPPs added at runtime. Because call to of_init_opp_table() would fail as there
> are no OPPs in DT and probe will return early.
>
> To fix this, remove error checking and call dev_pm_opp_init_cpufreq_table()
> unconditionally.
>
> Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq-cpu0.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)

Please update the binding as well to indicate that this property is now
optional.

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 02/14] clk: Create of_clk_shared_by_cpus()
  2014-07-01 18:00   ` Stephen Boyd
@ 2014-07-02  1:57     ` Viresh Kumar
  0 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-02  1:57 UTC (permalink / raw)
  To: Stephen Boyd, Rob Herring
  Cc: Rafael J. Wysocki, Shawn Guo, Lists linaro-kernel, linux-pm,
	Linux Kernel Mailing List, Arvind Chauhan, linux-arm-msm,
	Sachin Kamat, Thomas P Abraham, Nishanth Menon, Tomasz Figa,
	Mike Turquette

On 1 July 2014 23:30, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 07/01/14 09:32, Viresh Kumar wrote:
>> Create a new routine of_clk_shared_by_cpus() that finds if clock lines are
>> shared between two CPUs. This is verified by comparing "clocks" property from
>> CPU's DT node.
>>
>> Returns 1 if clock line is shared between them, 0 if clock isn't shared and
>> return appropriate errors in case nodes/properties are missing.
>>
>> Cc: Mike Turquette <mturquette@linaro.org>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>>  drivers/clk/clk.c   | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/clk.h |  6 ++++++
>
> This doesn't make much sense to me. This function doesn't deal with
> struct clk pointers or any of the internals of the common clock
> framework so why put it in clk.c? It looks more like an internal
> function that the cpufreq-generic driver should have.

I thought this is what Rob suggested when he said:
"I think a clock api function would be better."

I had it in cpufreq-cpu0 driver earlier and moved it to a separate API
yesterday only.

Sorry if I misunderstood his comment.

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 07/14] cpufreq: cpu0: OPPs can be populated at runtime
  2014-07-01 18:02   ` Stephen Boyd
@ 2014-07-02  2:03     ` Viresh Kumar
  0 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-02  2:03 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Rafael J. Wysocki, Shawn Guo, Lists linaro-kernel, linux-pm,
	Linux Kernel Mailing List, Arvind Chauhan, linux-arm-msm,
	Sachin Kamat, Thomas P Abraham, Nishanth Menon, Tomasz Figa

On 1 July 2014 23:32, Stephen Boyd <sboyd@codeaurora.org> wrote:
> Please update the binding as well to indicate that this property is now
> optional.

Does this look fine..

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
index f055515..366690c 100644
--- a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
@@ -8,10 +8,12 @@ Both required and optional properties listed below
must be defined
 under node /cpus/cpu@0.

 Required properties:
-- operating-points: Refer to Documentation/devicetree/bindings/power/opp.txt
-  for details
+- None

 Optional properties:
+- operating-points: Refer to
Documentation/devicetree/bindings/power/opp.txt for
+  details. OPPs *must* be supplied either via DT, i.e. this property, or
+  populated at runtime.
 - clock-latency: Specify the possible maximum transition latency for clock,
   in unit of nanoseconds.
 - voltage-tolerance: Specify the CPU voltage tolerance in percentage.

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* [PATCH V2 Resend 07/14] cpufreq: cpu0: OPPs can be populated at runtime
  2014-07-01 16:32 ` [PATCH 07/14] cpufreq: cpu0: OPPs can be populated at runtime Viresh Kumar
  2014-07-01 18:02   ` Stephen Boyd
@ 2014-07-02  4:03   ` Viresh Kumar
  2014-07-09 14:44     ` Santosh Shilimkar
  2 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-02  4:03 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar

OPPs can be populated statically, via DT, or added at run time with
dev_pm_opp_add().

While this driver handles the first case correctly, it would fail to populate
OPPs added at runtime. Because call to of_init_opp_table() would fail as there
are no OPPs in DT and probe will return early.

To fix this, remove error checking and call dev_pm_opp_init_cpufreq_table()
unconditionally.

Update bindings as well.

Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V2 Resend: Update bindings as well

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

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
index f055515..366690c 100644
--- a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
@@ -8,10 +8,12 @@ Both required and optional properties listed below must be defined
 under node /cpus/cpu@0.
 
 Required properties:
-- operating-points: Refer to Documentation/devicetree/bindings/power/opp.txt
-  for details
+- None
 
 Optional properties:
+- operating-points: Refer to Documentation/devicetree/bindings/power/opp.txt for
+  details. OPPs *must* be supplied either via DT, i.e. this property, or
+  populated at runtime.
 - clock-latency: Specify the possible maximum transition latency for clock,
   in unit of nanoseconds.
 - voltage-tolerance: Specify the CPU voltage tolerance in percentage.
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index b5b8e1c..f47f703 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -164,11 +164,8 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 		goto out_put_reg;
 	}
 
-	ret = of_init_opp_table(cpu_dev);
-	if (ret) {
-		pr_err("failed to init OPP table: %d\n", ret);
-		goto out_put_clk;
-	}
+	/* OPPs might be populated at runtime, don't check for error here */
+	of_init_opp_table(cpu_dev);
 
 	ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table);
 	if (ret) {
-- 
2.0.0.rc2

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* [PATCH V2 Resend 12/14] cpufreq: cpu0: Extend support beyond CPU0
  2014-07-01 16:32 ` [PATCH 12/14] cpufreq: cpu0: Extend support beyond CPU0 Viresh Kumar
@ 2014-07-02  4:03   ` Viresh Kumar
  0 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-02  4:03 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: linaro-kernel, linux-pm, linux-kernel, arvind.chauhan, sboyd,
	linux-arm-msm, spk.linux, thomas.ab, nm, t.figa, Viresh Kumar,
	devicetree

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

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-01 16:32 [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
                   ` (13 preceding siblings ...)
  2014-07-01 16:32 ` [PATCH 14/14] cpufreq: generic: set platform_{driver|device} '.name' to 'cpufreq-generic' Viresh Kumar
@ 2014-07-02  4:12 ` Viresh Kumar
  2014-07-03  1:24   ` Stephen Boyd
  14 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-02  4:12 UTC (permalink / raw)
  To: Rafael J. Wysocki, Stephen Boyd
  Cc: Lists linaro-kernel, linux-pm, Arvind Chauhan, linux-arm-msm,
	Sachin Kamat, Thomas P Abraham, Shawn Guo,
	Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
	Viresh Kumar, devicetree, Kukjin Kim, Michal Simek,
	Mike Turquette, Rob Herring, Santosh Shilimkar, Simon Horman

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 06/14] cpufreq: cpu0: defer probe if clock isn't registered yet
  2014-07-01 16:32 ` [PATCH 06/14] cpufreq: cpu0: defer probe if clock isn't registered yet Viresh Kumar
@ 2014-07-02  5:53     ` Shawn Guo
  0 siblings, 0 replies; 83+ messages in thread
From: Shawn Guo @ 2014-07-02  5:53 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rjw, linaro-kernel, linux-pm, linux-kernel, arvind.chauhan,
	sboyd, linux-arm-msm, spk.linux, thomas.ab, nm, t.figa

On Tue, Jul 01, 2014 at 10:02:35PM +0530, Viresh Kumar wrote:
> Currently, we defer probe if regulator_get() returned -EPROBE_DEFER, i.e.
> regulator isn't registered yet.
> 
> The same is true for clock as well, so lets defer in that case as well.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq-cpu0.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
> index 4273a5f..b5b8e1c 100644
> --- a/drivers/cpufreq/cpufreq-cpu0.c
> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> @@ -150,8 +150,17 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
>  
>  	cpu_clk = clk_get(cpu_dev, NULL);
>  	if (IS_ERR(cpu_clk)) {
> -		ret = PTR_ERR(cpu_clk);

If you keep this ...

> -		pr_err("failed to get cpu0 clock: %d\n", ret);
> +		/*
> +		 * If cpu's clk node is present, but clock is not yet
> +		 * registered, we should try defering probe.
> +		 */
> +		if (PTR_ERR(cpu_clk) == -EPROBE_DEFER) {

... you can use 'ret' here ...

> +			dev_err(cpu_dev, "cpu0 clock not ready, retry\n");
> +			ret = -EPROBE_DEFER;

... this can be saved ...

> +		} else {
> +			ret = PTR_ERR(cpu_clk);

... and this as well.

Shawn

> +			dev_err(cpu_dev, "failed to get cpu0 clock: %d\n", ret);
> +		}
>  		goto out_put_reg;
>  	}
>  
> -- 
> 2.0.0.rc2
> 

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 06/14] cpufreq: cpu0: defer probe if clock isn't registered yet
@ 2014-07-02  5:53     ` Shawn Guo
  0 siblings, 0 replies; 83+ messages in thread
From: Shawn Guo @ 2014-07-02  5:53 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rjw, linaro-kernel, linux-pm, linux-kernel, arvind.chauhan,
	sboyd, linux-arm-msm, spk.linux, thomas.ab, nm, t.figa

On Tue, Jul 01, 2014 at 10:02:35PM +0530, Viresh Kumar wrote:
> Currently, we defer probe if regulator_get() returned -EPROBE_DEFER, i.e.
> regulator isn't registered yet.
> 
> The same is true for clock as well, so lets defer in that case as well.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq-cpu0.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
> index 4273a5f..b5b8e1c 100644
> --- a/drivers/cpufreq/cpufreq-cpu0.c
> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> @@ -150,8 +150,17 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
>  
>  	cpu_clk = clk_get(cpu_dev, NULL);
>  	if (IS_ERR(cpu_clk)) {
> -		ret = PTR_ERR(cpu_clk);

If you keep this ...

> -		pr_err("failed to get cpu0 clock: %d\n", ret);
> +		/*
> +		 * If cpu's clk node is present, but clock is not yet
> +		 * registered, we should try defering probe.
> +		 */
> +		if (PTR_ERR(cpu_clk) == -EPROBE_DEFER) {

... you can use 'ret' here ...

> +			dev_err(cpu_dev, "cpu0 clock not ready, retry\n");
> +			ret = -EPROBE_DEFER;

... this can be saved ...

> +		} else {
> +			ret = PTR_ERR(cpu_clk);

... and this as well.

Shawn

> +			dev_err(cpu_dev, "failed to get cpu0 clock: %d\n", ret);
> +		}
>  		goto out_put_reg;
>  	}
>  
> -- 
> 2.0.0.rc2
> 

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 06/14] cpufreq: cpu0: defer probe if clock isn't registered yet
  2014-07-02  5:53     ` Shawn Guo
  (?)
@ 2014-07-02  5:55     ` Viresh Kumar
  2014-07-02 11:32       ` Viresh Kumar
  -1 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-02  5:55 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rafael J. Wysocki, Lists linaro-kernel, linux-pm,
	Linux Kernel Mailing List, Arvind Chauhan, Stephen Boyd,
	linux-arm-msm, Sachin Kamat, Thomas P Abraham, Nishanth Menon,
	Tomasz Figa

On 2 July 2014 11:23, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Tue, Jul 01, 2014 at 10:02:35PM +0530, Viresh Kumar wrote:

>> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
>> index 4273a5f..b5b8e1c 100644
>> --- a/drivers/cpufreq/cpufreq-cpu0.c
>> +++ b/drivers/cpufreq/cpufreq-cpu0.c
>> @@ -150,8 +150,17 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
>>
>>       cpu_clk = clk_get(cpu_dev, NULL);
>>       if (IS_ERR(cpu_clk)) {
>> -             ret = PTR_ERR(cpu_clk);
>
> If you keep this ...
>
>> -             pr_err("failed to get cpu0 clock: %d\n", ret);
>> +             /*
>> +              * If cpu's clk node is present, but clock is not yet
>> +              * registered, we should try defering probe.
>> +              */
>> +             if (PTR_ERR(cpu_clk) == -EPROBE_DEFER) {
>
> ... you can use 'ret' here ...
>
>> +                     dev_err(cpu_dev, "cpu0 clock not ready, retry\n");
>> +                     ret = -EPROBE_DEFER;
>
> ... this can be saved ...
>
>> +             } else {
>> +                     ret = PTR_ERR(cpu_clk);
>
> ... and this as well.

All accepted. Thanks.

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 06/14] cpufreq: cpu0: defer probe if clock isn't registered yet
  2014-07-02  5:55     ` Viresh Kumar
@ 2014-07-02 11:32       ` Viresh Kumar
  2014-07-03  0:38         ` Stephen Boyd
  0 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-02 11:32 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rafael J. Wysocki, Lists linaro-kernel, linux-pm,
	Linux Kernel Mailing List, Arvind Chauhan, Stephen Boyd,
	linux-arm-msm, Sachin Kamat, Thomas P Abraham, Nishanth Menon,
	Tomasz Figa

On 2 July 2014 11:25, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 2 July 2014 11:23, Shawn Guo <shawn.guo@linaro.org> wrote:
>> On Tue, Jul 01, 2014 at 10:02:35PM +0530, Viresh Kumar wrote:
>
>>> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
>>> index 4273a5f..b5b8e1c 100644
>>> --- a/drivers/cpufreq/cpufreq-cpu0.c
>>> +++ b/drivers/cpufreq/cpufreq-cpu0.c
>>> @@ -150,8 +150,17 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
>>>
>>>       cpu_clk = clk_get(cpu_dev, NULL);
>>>       if (IS_ERR(cpu_clk)) {
>>> -             ret = PTR_ERR(cpu_clk);
>>
>> If you keep this ...
>>
>>> -             pr_err("failed to get cpu0 clock: %d\n", ret);
>>> +             /*
>>> +              * If cpu's clk node is present, but clock is not yet
>>> +              * registered, we should try defering probe.
>>> +              */
>>> +             if (PTR_ERR(cpu_clk) == -EPROBE_DEFER) {
>>
>> ... you can use 'ret' here ...
>>
>>> +                     dev_err(cpu_dev, "cpu0 clock not ready, retry\n");
>>> +                     ret = -EPROBE_DEFER;
>>
>> ... this can be saved ...
>>
>>> +             } else {
>>> +                     ret = PTR_ERR(cpu_clk);
>>
>> ... and this as well.
>
> All accepted. Thanks.

The motive of this patch is changed completely after what you suggested
and so logs are updated as well:

    cpufreq: cpu0: print relevant error when we defer probe

    Currently, we defer probe if regulator_get() returned -EPROBE_DEFER, i.e.
    regulator isn't registered yet.

    The same is true for clock as well and should be done there.

    Current code already does it, but it wasn't intentional probably.
Its just that
    we are returning the right error with wrong print message.

    Fix print message to convey right error.

    Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq-cpu0.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index 4273a5f..0c16b2f 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -151,7 +151,16 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
        cpu_clk = clk_get(cpu_dev, NULL);
        if (IS_ERR(cpu_clk)) {
                ret = PTR_ERR(cpu_clk);
-               pr_err("failed to get cpu0 clock: %d\n", ret);
+
+               /*
+                * If cpu's clk node is present, but clock is not yet
+                * registered, we should try defering probe.
+                */
+               if (ret == -EPROBE_DEFER)
+                       dev_err(cpu_dev, "cpu0 clock not ready, retry\n");
+               else
+                       dev_err(cpu_dev, "failed to get cpu0 clock: %d\n", ret);
+
                goto out_put_reg;
        }

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* Re: [PATCH 06/14] cpufreq: cpu0: defer probe if clock isn't registered yet
  2014-07-02 11:32       ` Viresh Kumar
@ 2014-07-03  0:38         ` Stephen Boyd
  2014-07-03  2:19           ` Viresh Kumar
  0 siblings, 1 reply; 83+ messages in thread
From: Stephen Boyd @ 2014-07-03  0:38 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Shawn Guo, Rafael J. Wysocki, Lists linaro-kernel, linux-pm,
	Linux Kernel Mailing List, Arvind Chauhan, linux-arm-msm,
	Sachin Kamat, Thomas P Abraham, Nishanth Menon, Tomasz Figa

On 07/02/14 04:32, Viresh Kumar wrote:
>
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
> index 4273a5f..0c16b2f 100644
> --- a/drivers/cpufreq/cpufreq-cpu0.c
> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> @@ -151,7 +151,16 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
>         cpu_clk = clk_get(cpu_dev, NULL);
>         if (IS_ERR(cpu_clk)) {
>                 ret = PTR_ERR(cpu_clk);
> -               pr_err("failed to get cpu0 clock: %d\n", ret);
> +
> +               /*
> +                * If cpu's clk node is present, but clock is not yet
> +                * registered, we should try defering probe.
> +                */
> +               if (ret == -EPROBE_DEFER)
> +                       dev_err(cpu_dev, "cpu0 clock not ready, retry\n");

Please make this a dev_dbg() or just remove it entirely. Sending a
message to the log on probe defer just duplicates what the driver core
is already doing.

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


^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
  2014-07-01 16:32 ` [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init() Viresh Kumar
@ 2014-07-03  0:43   ` Stephen Boyd
  2014-07-03  2:11     ` Viresh Kumar
  2014-07-09 14:53     ` Santosh Shilimkar
  1 sibling, 1 reply; 83+ messages in thread
From: Stephen Boyd @ 2014-07-03  0:43 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rjw, shawn.guo, linaro-kernel, linux-pm, linux-kernel,
	arvind.chauhan, linux-arm-msm, spk.linux, thomas.ab, nm, t.figa

On 07/01/14 09:32, Viresh Kumar wrote:
> +static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
> +{
> +	struct cpufreq_frequency_table *freq_table;
> +	struct thermal_cooling_device *cdev;

This..

> +	struct device_node *np;
> +	struct private_data *priv;
> +	
[...]
> ing them.
> @@ -223,28 +242,92 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
>  				"running cpufreq without cooling device: %ld\n",
>  				PTR_ERR(cdev));
>  	}
> -
>  	of_node_put(np);
> +
> +	priv->cdev = cdev;

Causes a build warning:

drivers/cpufreq/cpufreq-generic.c:313:13: warning: 'cdev' may be used
uninitialized in this function [-Wmaybe-uninitialized]

So I guess we should initialize it to NULL?

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-02  4:12 ` [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
@ 2014-07-03  1:24   ` Stephen Boyd
  2014-07-03  2:44     ` Viresh Kumar
  0 siblings, 1 reply; 83+ messages in thread
From: Stephen Boyd @ 2014-07-03  1:24 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael J. Wysocki, Lists linaro-kernel, linux-pm, Arvind Chauhan,
	linux-arm-msm, Sachin Kamat, Thomas P Abraham, Shawn Guo,
	Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
	devicetree, Kukjin Kim, Michal Simek, Mike Turquette,
	Rob Herring, Santosh Shilimkar, Simon Horman

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


^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
  2014-07-03  0:43   ` Stephen Boyd
@ 2014-07-03  2:11     ` Viresh Kumar
  0 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-03  2:11 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Rafael J. Wysocki, Shawn Guo, Lists linaro-kernel, linux-pm,
	Linux Kernel Mailing List, Arvind Chauhan, linux-arm-msm,
	Sachin Kamat, Thomas P Abraham, Nishanth Menon, Tomasz Figa

On 3 July 2014 06:13, Stephen Boyd <sboyd@codeaurora.org> wrote:
> drivers/cpufreq/cpufreq-generic.c:313:13: warning: 'cdev' may be used
> uninitialized in this function [-Wmaybe-uninitialized]
>
> So I guess we should initialize it to NULL?

I somehow didn't got this, I checked again. I have fixed it this way:

diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index e6dc8ea..05a18bd 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -240,10 +240,11 @@ static int cpu0_cpufreq_init(struct
cpufreq_policy *policy)
                        dev_err(cpu_dev,
                                "running cpufreq without cooling device: %ld\n",
                                PTR_ERR(cdev));
+               else
+                       priv->cdev = cdev;
        }
        of_node_put(np);

-       priv->cdev = cdev;
        priv->cpu_dev = cpu_dev;
        priv->cpu_reg = cpu_reg;
        policy->driver_data = priv;

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* Re: [PATCH 06/14] cpufreq: cpu0: defer probe if clock isn't registered yet
  2014-07-03  0:38         ` Stephen Boyd
@ 2014-07-03  2:19           ` Viresh Kumar
  2014-07-09 14:43               ` Santosh Shilimkar
  0 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-03  2:19 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Shawn Guo, Rafael J. Wysocki, Lists linaro-kernel, linux-pm,
	Linux Kernel Mailing List, Arvind Chauhan, linux-arm-msm,
	Sachin Kamat, Thomas P Abraham, Nishanth Menon, Tomasz Figa

On 3 July 2014 06:08, Stephen Boyd <sboyd@codeaurora.org> wrote:
> Please make this a dev_dbg() or just remove it entirely. Sending a
> message to the log on probe defer just duplicates what the driver core
> is already doing.

Updated as:

Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Thu Jun 26 10:40:21 2014 +0530

    cpufreq: cpu0: print relevant error when we defer probe

    Currently, we defer probe if regulator_get() returned -EPROBE_DEFER, i.e.
    regulator isn't registered yet. We do a dev_err() in this case. Sending a
    message to the log on probe defer just duplicates what the driver core is
    already doing. Convert it to dev_dbg() instead.

    We should defer in case of clk_get() as well.

    Current code already does it, but it wasn't intentional probably.
Its just that
    we are returning the right error with wrong print message.

    Fix print message to convey right error.

    Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq-cpu0.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index 4273a5f..8a1166c 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -140,7 +140,7 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
                 * not yet registered, we should try defering probe.
                 */
                if (PTR_ERR(cpu_reg) == -EPROBE_DEFER) {
-                       dev_err(cpu_dev, "cpu0 regulator not ready, retry\n");
+                       dev_dbg(cpu_dev, "cpu0 regulator not ready, retry\n");
                        ret = -EPROBE_DEFER;
                        goto out_put_node;
                }
@@ -151,7 +151,16 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
        cpu_clk = clk_get(cpu_dev, NULL);
        if (IS_ERR(cpu_clk)) {
                ret = PTR_ERR(cpu_clk);
-               pr_err("failed to get cpu0 clock: %d\n", ret);
+
+               /*
+                * If cpu's clk node is present, but clock is not yet
+                * registered, we should try defering probe.
+                */
+               if (ret == -EPROBE_DEFER)
+                       dev_dbg(cpu_dev, "cpu0 clock not ready, retry\n");
+               else
+                       dev_err(cpu_dev, "failed to get cpu0 clock: %d\n", ret);
+
                goto out_put_reg;
        }

^ permalink raw reply related	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-03  1:24   ` Stephen Boyd
@ 2014-07-03  2:44     ` Viresh Kumar
  2014-07-03 22:16       ` Mike Turquette
  0 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-03  2:44 UTC (permalink / raw)
  To: Stephen Boyd, Mike Turquette
  Cc: Rafael J. Wysocki, Lists linaro-kernel, linux-pm, Arvind Chauhan,
	linux-arm-msm, Sachin Kamat, Thomas P Abraham, Shawn Guo,
	Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
	devicetree, Kukjin Kim, Michal Simek, Rob Herring,
	Santosh Shilimkar, Simon Horman

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 ? :)

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-03  2:44     ` Viresh Kumar
@ 2014-07-03 22:16       ` Mike Turquette
  2014-07-04  4:21         ` Viresh Kumar
  0 siblings, 1 reply; 83+ messages in thread
From: Mike Turquette @ 2014-07-03 22:16 UTC (permalink / raw)
  To: Viresh Kumar, Stephen Boyd
  Cc: Rafael J. Wysocki, Lists linaro-kernel, linux-pm, Arvind Chauhan,
	linux-arm-msm, Sachin Kamat, Thomas P Abraham, Shawn Guo,
	Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
	devicetree, Kukjin Kim, Michal Simek, Rob Herring,
	Santosh Shilimkar, Simon Horman

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 ? :)

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-03 22:16       ` Mike Turquette
@ 2014-07-04  4:21         ` Viresh Kumar
  2014-07-08  4:50           ` Viresh Kumar
  0 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-04  4:21 UTC (permalink / raw)
  To: Mike Turquette
  Cc: Stephen Boyd, Rafael J. Wysocki, Lists linaro-kernel, linux-pm,
	Arvind Chauhan, linux-arm-msm, Sachin Kamat, Thomas P Abraham,
	Shawn Guo, Linux Kernel Mailing List, Nishanth Menon,
	Tomasz Figa, devicetree, Kukjin Kim, Michal Simek, Rob Herring,
	Santosh Shilimkar, Simon Horman

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-04  4:21         ` Viresh Kumar
@ 2014-07-08  4:50           ` Viresh Kumar
  2014-07-09 17:41             ` Stephen Boyd
  2014-07-16 16:01             ` Viresh Kumar
  0 siblings, 2 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-08  4:50 UTC (permalink / raw)
  To: Mike Turquette
  Cc: Stephen Boyd, Rafael J. Wysocki, Lists linaro-kernel, linux-pm,
	Arvind Chauhan, linux-arm-msm, Sachin Kamat, Thomas P Abraham,
	Shawn Guo, Linux Kernel Mailing List, Nishanth Menon,
	Tomasz Figa, devicetree, Kukjin Kim, Michal Simek, Rob Herring,
	Santosh Shilimkar, Simon Horman

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 03/14] cpufreq: Add support for per-policy driver data
  2014-07-01 16:32 ` [PATCH 03/14] cpufreq: Add support for per-policy driver data Viresh Kumar
@ 2014-07-09 14:33     ` Santosh Shilimkar
  0 siblings, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 14:33 UTC (permalink / raw)
  To: Viresh Kumar, rjw, shawn.guo
  Cc: nm, linaro-kernel, linux-pm, linux-arm-msm, t.figa, sboyd,
	linux-kernel, thomas.ab, arvind.chauhan, spk.linux

On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote:
> Drivers supporting multiple clusters or multiple 'struct cpufreq_policy'
> instances may need to keep per-policy data. If the core doesn't support them,
> they might do it in the most unoptimized way: 'per-cpu' data.
> 
> This patch adds another field in 'struct cpufreq_policy': 'driver_data'. It
> isn't accessed by core and is for driver's internal use.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  include/linux/cpufreq.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index ec4112d..d4b1108 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -112,6 +112,9 @@ struct cpufreq_policy {
>  	spinlock_t		transition_lock;
>  	wait_queue_head_t	transition_wait;
>  	struct task_struct	*transition_task; /* Task which is doing the transition */
> +
> +	/* For cpufreq driver's internal use */
> +	void			*driver_data;
>  };
>  
Minor comment for consistency either maintain same commenting style
for the above structure (description after the variable) or may
be clean up the comments in another patch.

Regards,
Santosh

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 03/14] cpufreq: Add support for per-policy driver data
@ 2014-07-09 14:33     ` Santosh Shilimkar
  0 siblings, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 14:33 UTC (permalink / raw)
  To: Viresh Kumar, rjw, shawn.guo
  Cc: nm, linaro-kernel, linux-pm, linux-arm-msm, t.figa, sboyd,
	linux-kernel, thomas.ab, arvind.chauhan, spk.linux

On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote:
> Drivers supporting multiple clusters or multiple 'struct cpufreq_policy'
> instances may need to keep per-policy data. If the core doesn't support them,
> they might do it in the most unoptimized way: 'per-cpu' data.
> 
> This patch adds another field in 'struct cpufreq_policy': 'driver_data'. It
> isn't accessed by core and is for driver's internal use.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  include/linux/cpufreq.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index ec4112d..d4b1108 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -112,6 +112,9 @@ struct cpufreq_policy {
>  	spinlock_t		transition_lock;
>  	wait_queue_head_t	transition_wait;
>  	struct task_struct	*transition_task; /* Task which is doing the transition */
> +
> +	/* For cpufreq driver's internal use */
> +	void			*driver_data;
>  };
>  
Minor comment for consistency either maintain same commenting style
for the above structure (description after the variable) or may
be clean up the comments in another patch.

Regards,
Santosh


^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 02/14] clk: Create of_clk_shared_by_cpus()
  2014-07-01 16:32 ` [PATCH 02/14] clk: Create of_clk_shared_by_cpus() Viresh Kumar
@ 2014-07-09 14:38     ` Santosh Shilimkar
  2014-07-09 14:38     ` Santosh Shilimkar
       [not found]   ` <5f7164d789e87c62d722b575980c92dfd0504334.1404231535.git.viresh.kumar@linar o.org>
  2 siblings, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 14:38 UTC (permalink / raw)
  To: Viresh Kumar, rjw, shawn.guo
  Cc: nm, linaro-kernel, linux-pm, linux-arm-msm, t.figa, sboyd,
	linux-kernel, thomas.ab, arvind.chauhan, spk.linux,
	Lorenzo Pieralisi

+ Lorenzo

On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote:
> Create a new routine of_clk_shared_by_cpus() that finds if clock lines are
> shared between two CPUs. This is verified by comparing "clocks" property from
> CPU's DT node.
> 
> Returns 1 if clock line is shared between them, 0 if clock isn't shared and
> return appropriate errors in case nodes/properties are missing.
> 
> Cc: Mike Turquette <mturquette@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/clk/clk.c   | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/clk.h |  6 ++++++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 8b73ede..497735c 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -10,6 +10,7 @@
>   */
>  
>  #include <linux/clk-private.h>
> +#include <linux/cpu.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/spinlock.h>
> @@ -2528,6 +2529,61 @@ const char *of_clk_get_parent_name(struct device_node *np, int index)
>  }
>  EXPORT_SYMBOL_GPL(of_clk_get_parent_name);
>  
> +/**
> + * of_clk_shared_by_cpus - Finds if clock line is shared between CPUs
> + * @cpu1, cpu2: CPU numbers
> + *
> + * Finds if clock lines are shared by two CPUs. This is verified by comparing
> + * "clocks" property from CPU's DT node.
> + *
> + * Returns 1 if clock line is shared between them, 0 if clock isn't shared.
> + * Return appropriate errors in case some requirements aren't met.
> + */
> +int of_clk_shared_by_cpus(int cpu1, int cpu2)

I might be wrong but this API seems to bit short cited. We should probably
handle it bit better since there are more cases instead of just checking CPU
tuple. More than two CPUs may share the clock so discovering that in one iteration
is better. I also think this is closely related to CPU topology.

- CPUs part of 'a cluster' shares same clock.
- Multiple clusters may share same clock
- Every CPU might be clocked from separate PLL.

What you say ?

Regards,
Santosh

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 02/14] clk: Create of_clk_shared_by_cpus()
@ 2014-07-09 14:38     ` Santosh Shilimkar
  0 siblings, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 14:38 UTC (permalink / raw)
  To: Viresh Kumar, rjw, shawn.guo
  Cc: nm, linaro-kernel, linux-pm, linux-arm-msm, t.figa, sboyd,
	linux-kernel, thomas.ab, arvind.chauhan, spk.linux,
	Lorenzo Pieralisi

+ Lorenzo

On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote:
> Create a new routine of_clk_shared_by_cpus() that finds if clock lines are
> shared between two CPUs. This is verified by comparing "clocks" property from
> CPU's DT node.
> 
> Returns 1 if clock line is shared between them, 0 if clock isn't shared and
> return appropriate errors in case nodes/properties are missing.
> 
> Cc: Mike Turquette <mturquette@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/clk/clk.c   | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/clk.h |  6 ++++++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 8b73ede..497735c 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -10,6 +10,7 @@
>   */
>  
>  #include <linux/clk-private.h>
> +#include <linux/cpu.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/spinlock.h>
> @@ -2528,6 +2529,61 @@ const char *of_clk_get_parent_name(struct device_node *np, int index)
>  }
>  EXPORT_SYMBOL_GPL(of_clk_get_parent_name);
>  
> +/**
> + * of_clk_shared_by_cpus - Finds if clock line is shared between CPUs
> + * @cpu1, cpu2: CPU numbers
> + *
> + * Finds if clock lines are shared by two CPUs. This is verified by comparing
> + * "clocks" property from CPU's DT node.
> + *
> + * Returns 1 if clock line is shared between them, 0 if clock isn't shared.
> + * Return appropriate errors in case some requirements aren't met.
> + */
> +int of_clk_shared_by_cpus(int cpu1, int cpu2)

I might be wrong but this API seems to bit short cited. We should probably
handle it bit better since there are more cases instead of just checking CPU
tuple. More than two CPUs may share the clock so discovering that in one iteration
is better. I also think this is closely related to CPU topology.

- CPUs part of 'a cluster' shares same clock.
- Multiple clusters may share same clock
- Every CPU might be clocked from separate PLL.

What you say ?

Regards,
Santosh





^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 04/14] cpufreq: cpu0: Add Module Author
  2014-07-01 16:32 ` [PATCH 04/14] cpufreq: cpu0: Add Module Author Viresh Kumar
@ 2014-07-09 14:42     ` Santosh Shilimkar
  0 siblings, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 14:42 UTC (permalink / raw)
  To: Viresh Kumar, rjw, shawn.guo
  Cc: nm, linaro-kernel, linux-pm, linux-arm-msm, t.figa, sboyd,
	linux-kernel, thomas.ab, arvind.chauhan, spk.linux

On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote:
> Two people are maintaining it now, Viresh and Shawn. Add Viresh's details in
> MODULE_AUTHOR() and copyright section.
> 
> Suggested-by: Shawn Guo <shawn.guo@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq-cpu0.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
> index ee1ae30..7e191db 100644
> --- a/drivers/cpufreq/cpufreq-cpu0.c
> +++ b/drivers/cpufreq/cpufreq-cpu0.c

Not related to this patch but I think its time to change
the name of this driver. I never liked this 'cpufreq-cpu0.c'
and already mentioned that during the reviews of this
driver.

Not sure if there are still sentiments about it but
considering the additional functionality coming in,
the name will definitely miss-leading.

Regards,
Santosh

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 04/14] cpufreq: cpu0: Add Module Author
@ 2014-07-09 14:42     ` Santosh Shilimkar
  0 siblings, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 14:42 UTC (permalink / raw)
  To: Viresh Kumar, rjw, shawn.guo
  Cc: nm, linaro-kernel, linux-pm, linux-arm-msm, t.figa, sboyd,
	linux-kernel, thomas.ab, arvind.chauhan, spk.linux

On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote:
> Two people are maintaining it now, Viresh and Shawn. Add Viresh's details in
> MODULE_AUTHOR() and copyright section.
> 
> Suggested-by: Shawn Guo <shawn.guo@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq-cpu0.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
> index ee1ae30..7e191db 100644
> --- a/drivers/cpufreq/cpufreq-cpu0.c
> +++ b/drivers/cpufreq/cpufreq-cpu0.c

Not related to this patch but I think its time to change
the name of this driver. I never liked this 'cpufreq-cpu0.c'
and already mentioned that during the reviews of this
driver.

Not sure if there are still sentiments about it but
considering the additional functionality coming in,
the name will definitely miss-leading.

Regards,
Santosh


^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 05/14] cpufreq: cpu0: don't validate clock on clk_put()
  2014-07-01 16:32 ` [PATCH 05/14] cpufreq: cpu0: don't validate clock on clk_put() Viresh Kumar
@ 2014-07-09 14:42     ` Santosh Shilimkar
  0 siblings, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 14:42 UTC (permalink / raw)
  To: Viresh Kumar, rjw, shawn.guo
  Cc: nm, linaro-kernel, linux-pm, linux-arm-msm, t.figa, sboyd,
	linux-kernel, thomas.ab, arvind.chauhan, spk.linux

On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote:
> CPU clk is not optional for this driver and probe would fail if it couldn't find
> a suitable clock.
> 
> And so, while calling clk_put() we don't need to validate clocks.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 05/14] cpufreq: cpu0: don't validate clock on clk_put()
@ 2014-07-09 14:42     ` Santosh Shilimkar
  0 siblings, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 14:42 UTC (permalink / raw)
  To: Viresh Kumar, rjw, shawn.guo
  Cc: nm, linaro-kernel, linux-pm, linux-arm-msm, t.figa, sboyd,
	linux-kernel, thomas.ab, arvind.chauhan, spk.linux

On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote:
> CPU clk is not optional for this driver and probe would fail if it couldn't find
> a suitable clock.
> 
> And so, while calling clk_put() we don't need to validate clocks.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>



^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 06/14] cpufreq: cpu0: defer probe if clock isn't registered yet
  2014-07-03  2:19           ` Viresh Kumar
@ 2014-07-09 14:43               ` Santosh Shilimkar
  0 siblings, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 14:43 UTC (permalink / raw)
  To: Viresh Kumar, Stephen Boyd
  Cc: Nishanth Menon, linux-pm, linux-arm-msm, Tomasz Figa,
	Rafael J. Wysocki, Linux Kernel Mailing List,
	Lists linaro-kernel, Thomas P Abraham, Arvind Chauhan,
	Sachin Kamat

On Wednesday 02 July 2014 10:19 PM, Viresh Kumar wrote:
> On 3 July 2014 06:08, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> Please make this a dev_dbg() or just remove it entirely. Sending a
>> message to the log on probe defer just duplicates what the driver core
>> is already doing.
> 
> Updated as:
> 
> Author: Viresh Kumar <viresh.kumar@linaro.org>
> Date:   Thu Jun 26 10:40:21 2014 +0530
> 
>     cpufreq: cpu0: print relevant error when we defer probe
> 
>     Currently, we defer probe if regulator_get() returned -EPROBE_DEFER, i.e.
>     regulator isn't registered yet. We do a dev_err() in this case. Sending a
>     message to the log on probe defer just duplicates what the driver core is
>     already doing. Convert it to dev_dbg() instead.
> 
>     We should defer in case of clk_get() as well.
> 
>     Current code already does it, but it wasn't intentional probably.
> Its just that
>     we are returning the right error with wrong print message.
> 
>     Fix print message to convey right error.
> 
>     Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq-cpu0.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
Looks good to me.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>


^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 06/14] cpufreq: cpu0: defer probe if clock isn't registered yet
@ 2014-07-09 14:43               ` Santosh Shilimkar
  0 siblings, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 14:43 UTC (permalink / raw)
  To: Viresh Kumar, Stephen Boyd
  Cc: Nishanth Menon, linux-pm, linux-arm-msm, Tomasz Figa,
	Rafael J. Wysocki, Linux Kernel Mailing List,
	Lists linaro-kernel, Thomas P Abraham, Arvind Chauhan,
	Sachin Kamat

On Wednesday 02 July 2014 10:19 PM, Viresh Kumar wrote:
> On 3 July 2014 06:08, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> Please make this a dev_dbg() or just remove it entirely. Sending a
>> message to the log on probe defer just duplicates what the driver core
>> is already doing.
> 
> Updated as:
> 
> Author: Viresh Kumar <viresh.kumar@linaro.org>
> Date:   Thu Jun 26 10:40:21 2014 +0530
> 
>     cpufreq: cpu0: print relevant error when we defer probe
> 
>     Currently, we defer probe if regulator_get() returned -EPROBE_DEFER, i.e.
>     regulator isn't registered yet. We do a dev_err() in this case. Sending a
>     message to the log on probe defer just duplicates what the driver core is
>     already doing. Convert it to dev_dbg() instead.
> 
>     We should defer in case of clk_get() as well.
> 
>     Current code already does it, but it wasn't intentional probably.
> Its just that
>     we are returning the right error with wrong print message.
> 
>     Fix print message to convey right error.
> 
>     Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq-cpu0.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
Looks good to me.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>


^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 07/14] cpufreq: cpu0: OPPs can be populated at runtime
  2014-07-01 16:32 ` [PATCH 07/14] cpufreq: cpu0: OPPs can be populated at runtime Viresh Kumar
@ 2014-07-09 14:44     ` Santosh Shilimkar
  2014-07-02  4:03   ` [PATCH V2 Resend " Viresh Kumar
  2014-07-09 14:44     ` Santosh Shilimkar
  2 siblings, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 14:44 UTC (permalink / raw)
  To: Viresh Kumar, rjw, shawn.guo
  Cc: nm, linaro-kernel, linux-pm, linux-arm-msm, t.figa, sboyd,
	linux-kernel, thomas.ab, arvind.chauhan, spk.linux

On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote:
> OPPs can be populated statically, via DT, or added at run time with
> dev_pm_opp_add().
> 
> While this driver handles the first case correctly, it would fail to populate
> OPPs added at runtime. Because call to of_init_opp_table() would fail as there
> are no OPPs in DT and probe will return early.
> 
> To fix this, remove error checking and call dev_pm_opp_init_cpufreq_table()
> unconditionally.
> 
> Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
Assuming you are updating bidnings as suggested by Stephen,
patch looks good to me.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 07/14] cpufreq: cpu0: OPPs can be populated at runtime
@ 2014-07-09 14:44     ` Santosh Shilimkar
  0 siblings, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 14:44 UTC (permalink / raw)
  To: Viresh Kumar, rjw, shawn.guo
  Cc: nm, linaro-kernel, linux-pm, linux-arm-msm, t.figa, sboyd,
	linux-kernel, thomas.ab, arvind.chauhan, spk.linux

On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote:
> OPPs can be populated statically, via DT, or added at run time with
> dev_pm_opp_add().
> 
> While this driver handles the first case correctly, it would fail to populate
> OPPs added at runtime. Because call to of_init_opp_table() would fail as there
> are no OPPs in DT and probe will return early.
> 
> To fix this, remove error checking and call dev_pm_opp_init_cpufreq_table()
> unconditionally.
> 
> Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
Assuming you are updating bidnings as suggested by Stephen,
patch looks good to me.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>



^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 08/14] cpufreq: cpu0: use dev_{err|warn|dbg} instead of pr_{err|warn|debug}
  2014-07-01 16:32 ` [PATCH 08/14] cpufreq: cpu0: use dev_{err|warn|dbg} instead of pr_{err|warn|debug} Viresh Kumar
@ 2014-07-09 14:45     ` Santosh Shilimkar
  0 siblings, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 14:45 UTC (permalink / raw)
  To: Viresh Kumar, rjw, shawn.guo
  Cc: nm, linaro-kernel, linux-pm, linux-arm-msm, t.figa, sboyd,
	linux-kernel, thomas.ab, arvind.chauhan, spk.linux

On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote:
> We already have cpu_dev and is used at multiple places for printing errors using
> dev_*(). But some prints are still using pr_*(). Lets make it consistent and
> replace those pr_*() macros with dev_*() macros.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq-cpu0.c | 32 ++++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 14 deletions(-)
> 
Looks good to me.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 08/14] cpufreq: cpu0: use dev_{err|warn|dbg} instead of pr_{err|warn|debug}
@ 2014-07-09 14:45     ` Santosh Shilimkar
  0 siblings, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 14:45 UTC (permalink / raw)
  To: Viresh Kumar, rjw, shawn.guo
  Cc: nm, linaro-kernel, linux-pm, linux-arm-msm, t.figa, sboyd,
	linux-kernel, thomas.ab, arvind.chauhan, spk.linux

On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote:
> We already have cpu_dev and is used at multiple places for printing errors using
> dev_*(). But some prints are still using pr_*(). Lets make it consistent and
> replace those pr_*() macros with dev_*() macros.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq-cpu0.c | 32 ++++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 14 deletions(-)
> 
Looks good to me.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>


^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
  2014-07-01 16:32 ` [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init() Viresh Kumar
@ 2014-07-09 14:53     ` Santosh Shilimkar
  2014-07-09 14:53     ` Santosh Shilimkar
  1 sibling, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 14:53 UTC (permalink / raw)
  To: Viresh Kumar, rjw, shawn.guo
  Cc: nm, linaro-kernel, linux-pm, linux-arm-msm, t.figa, sboyd,
	linux-kernel, thomas.ab, arvind.chauhan, spk.linux,
	Lorenzo Pieralisi


On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote:
> Currently this driver only support platforms on which all CPUs share clock &
> voltage lines and there is requirement to support platforms which have separate
> clock & voltage lines for CPUs, like Qualcomm's Krait and ARM's big LITTLE.
> 
> Each group of CPUs sharing clock/voltage lines are represented by 'struct
> cpufreq_policy' in cpufreq framework. And core calls ->init() once for each
> policy.
> 
> Currently we do all initialization/allocation from probe() which wouldn't work
> for above scenario. To make it work for these platforms, the first step is to
> move all initialization/allocation to ->init() and add ->exit() to do the
> reverse of it.
> 
> Also, remove all global variables and allocate space for them at runtime.
> 
> This patch creates 'struct private_data' for keeping all such information and
> a pointer to that would be stored in policy->driver_data.
> 
> The changed probe() routine now tries to see if regulator/clocks are available
> or we need to defer probe. In case they are available, it registers cpufreq
> driver. Otherwise, returns with -EPROBE_DEFER.
> 
> We still *don't* support platforms with separate clock/voltage lines for CPUs.
> This would be done in a separate patch.
> 
I scanned this patch and subsequent patches from the series. Since you are
modifying the interfaces and bindings, I just think its better if we can
address the cases where separate clock lines will be used by CPUs.

Surely don't want to increase your work neither want hold the progress
of the series but if you look at the changes to the interfaces, they
give you a feeling of incompleteness.

> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
Would you able to give some idea about what will it take to address that one
remainder case as well as part of this series.

Regards,
Santosh

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
@ 2014-07-09 14:53     ` Santosh Shilimkar
  0 siblings, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 14:53 UTC (permalink / raw)
  To: Viresh Kumar, rjw, shawn.guo
  Cc: nm, linaro-kernel, linux-pm, linux-arm-msm, t.figa, sboyd,
	linux-kernel, thomas.ab, arvind.chauhan, spk.linux,
	Lorenzo Pieralisi


On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote:
> Currently this driver only support platforms on which all CPUs share clock &
> voltage lines and there is requirement to support platforms which have separate
> clock & voltage lines for CPUs, like Qualcomm's Krait and ARM's big LITTLE.
> 
> Each group of CPUs sharing clock/voltage lines are represented by 'struct
> cpufreq_policy' in cpufreq framework. And core calls ->init() once for each
> policy.
> 
> Currently we do all initialization/allocation from probe() which wouldn't work
> for above scenario. To make it work for these platforms, the first step is to
> move all initialization/allocation to ->init() and add ->exit() to do the
> reverse of it.
> 
> Also, remove all global variables and allocate space for them at runtime.
> 
> This patch creates 'struct private_data' for keeping all such information and
> a pointer to that would be stored in policy->driver_data.
> 
> The changed probe() routine now tries to see if regulator/clocks are available
> or we need to defer probe. In case they are available, it registers cpufreq
> driver. Otherwise, returns with -EPROBE_DEFER.
> 
> We still *don't* support platforms with separate clock/voltage lines for CPUs.
> This would be done in a separate patch.
> 
I scanned this patch and subsequent patches from the series. Since you are
modifying the interfaces and bindings, I just think its better if we can
address the cases where separate clock lines will be used by CPUs.

Surely don't want to increase your work neither want hold the progress
of the series but if you look at the changes to the interfaces, they
give you a feeling of incompleteness.

> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
Would you able to give some idea about what will it take to address that one
remainder case as well as part of this series.

Regards,
Santosh

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 03/14] cpufreq: Add support for per-policy driver data
  2014-07-09 14:33     ` Santosh Shilimkar
  (?)
@ 2014-07-09 15:07     ` Viresh Kumar
  -1 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-09 15:07 UTC (permalink / raw)
  To: Santosh Shilimkar
  Cc: Rafael J. Wysocki, Shawn Guo, Nishanth Menon,
	Lists linaro-kernel, linux-pm, linux-arm-msm, Tomasz Figa,
	Stephen Boyd, Linux Kernel Mailing List, Thomas P Abraham,
	Arvind Chauhan, Sachin Kamat

On 9 July 2014 20:03, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
>> index ec4112d..d4b1108 100644
>> --- a/include/linux/cpufreq.h
>> +++ b/include/linux/cpufreq.h
>> @@ -112,6 +112,9 @@ struct cpufreq_policy {
>>       spinlock_t              transition_lock;
>>       wait_queue_head_t       transition_wait;
>>       struct task_struct      *transition_task; /* Task which is doing the transition */
>> +
>> +     /* For cpufreq driver's internal use */
>> +     void                    *driver_data;
>>  };
>>
> Minor comment for consistency either maintain same commenting style
> for the above structure (description after the variable) or may
> be clean up the comments in another patch.

Adding these to the end of variable makes it take multiple lines as we need
to break after 80 columns. And both the styles are already mixed
in existing file. So, would fix it separately in case I go for it :)

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 04/14] cpufreq: cpu0: Add Module Author
  2014-07-09 14:42     ` Santosh Shilimkar
  (?)
@ 2014-07-09 15:08     ` Viresh Kumar
  2014-07-09 15:24       ` Santosh Shilimkar
  -1 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-09 15:08 UTC (permalink / raw)
  To: Santosh Shilimkar
  Cc: Rafael J. Wysocki, Shawn Guo, Nishanth Menon,
	Lists linaro-kernel, linux-pm, linux-arm-msm, Tomasz Figa,
	Stephen Boyd, Linux Kernel Mailing List, Thomas P Abraham,
	Arvind Chauhan, Sachin Kamat

On 9 July 2014 20:12, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> Not related to this patch but I think its time to change
> the name of this driver. I never liked this 'cpufreq-cpu0.c'
> and already mentioned that during the reviews of this
> driver.
>
> Not sure if there are still sentiments about it but
> considering the additional functionality coming in,
> the name will definitely miss-leading.

Check last patch, its called cpufreq-generic now.

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 07/14] cpufreq: cpu0: OPPs can be populated at runtime
  2014-07-09 14:44     ` Santosh Shilimkar
  (?)
@ 2014-07-09 15:09     ` Viresh Kumar
  -1 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-09 15:09 UTC (permalink / raw)
  To: Santosh Shilimkar
  Cc: Rafael J. Wysocki, Shawn Guo, Nishanth Menon,
	Lists linaro-kernel, linux-pm, linux-arm-msm, Tomasz Figa,
	Stephen Boyd, Linux Kernel Mailing List, Thomas P Abraham,
	Arvind Chauhan, Sachin Kamat

On 9 July 2014 20:14, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> Assuming you are updating bidnings as suggested by Stephen,
> patch looks good to me.

I have already sent a patch in reply to this one earlier with updated
bindings.

> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

Thanks.

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
  2014-07-09 14:53     ` Santosh Shilimkar
  (?)
@ 2014-07-09 15:17     ` Viresh Kumar
  2014-07-09 15:26       ` Santosh Shilimkar
  -1 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-09 15:17 UTC (permalink / raw)
  To: Santosh Shilimkar
  Cc: Rafael J. Wysocki, Shawn Guo, Nishanth Menon,
	Lists linaro-kernel, linux-pm, linux-arm-msm, Tomasz Figa,
	Stephen Boyd, Linux Kernel Mailing List, Thomas P Abraham,
	Arvind Chauhan, Sachin Kamat, Lorenzo Pieralisi

On 9 July 2014 20:23, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> I scanned this patch and subsequent patches from the series. Since you are
> modifying the interfaces and bindings, I just think its better if we can
> address the cases where separate clock lines will be used by CPUs.
>
> Surely don't want to increase your work neither want hold the progress
> of the series but if you look at the changes to the interfaces, they
> give you a feeling of incompleteness.

Lets talk in the other thread you raised, 2/14 ..

>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
> Would you able to give some idea about what will it take to address that one
> remainder case as well as part of this series.

Which one? This:

> We still *don't* support platforms with separate clock/voltage lines for CPUs.
> This would be done in a separate patch.

Its already fixed as part of this series.

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 02/14] clk: Create of_clk_shared_by_cpus()
  2014-07-09 14:38     ` Santosh Shilimkar
  (?)
@ 2014-07-09 15:20     ` Viresh Kumar
  -1 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-09 15:20 UTC (permalink / raw)
  To: Santosh Shilimkar
  Cc: Rafael J. Wysocki, Shawn Guo, Nishanth Menon,
	Lists linaro-kernel, linux-pm, linux-arm-msm, Tomasz Figa,
	Stephen Boyd, Linux Kernel Mailing List, Thomas P Abraham,
	Arvind Chauhan, Sachin Kamat, Lorenzo Pieralisi

On 9 July 2014 20:08, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> I might be wrong but this API seems to bit short cited. We should probably
> handle it bit better since there are more cases instead of just checking CPU
> tuple. More than two CPUs may share the clock so discovering that in one iteration
> is better. I also think this is closely related to CPU topology.
>
> - CPUs part of 'a cluster' shares same clock.
> - Multiple clusters may share same clock
> - Every CPU might be clocked from separate PLL.

All these configurations are currently supported by this series.

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 04/14] cpufreq: cpu0: Add Module Author
  2014-07-09 15:08     ` Viresh Kumar
@ 2014-07-09 15:24       ` Santosh Shilimkar
  0 siblings, 0 replies; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 15:24 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael J. Wysocki, Shawn Guo, Nishanth Menon,
	Lists linaro-kernel, linux-pm, linux-arm-msm, Tomasz Figa,
	Stephen Boyd, Linux Kernel Mailing List, Thomas P Abraham,
	Arvind Chauhan, Sachin Kamat

On Wednesday 09 July 2014 11:08 AM, Viresh Kumar wrote:
> On 9 July 2014 20:12, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>> Not related to this patch but I think its time to change
>> the name of this driver. I never liked this 'cpufreq-cpu0.c'
>> and already mentioned that during the reviews of this
>> driver.
>>
>> Not sure if there are still sentiments about it but
>> considering the additional functionality coming in,
>> the name will definitely miss-leading.
> 
> Check last patch, its called cpufreq-generic now.
> 
Sounds good :)

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
  2014-07-09 15:17     ` Viresh Kumar
@ 2014-07-09 15:26       ` Santosh Shilimkar
  2014-07-09 15:27         ` Viresh Kumar
  0 siblings, 1 reply; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 15:26 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael J. Wysocki, Shawn Guo, Nishanth Menon,
	Lists linaro-kernel, linux-pm, linux-arm-msm, Tomasz Figa,
	Stephen Boyd, Linux Kernel Mailing List, Thomas P Abraham,
	Arvind Chauhan, Sachin Kamat, Lorenzo Pieralisi

On Wednesday 09 July 2014 11:17 AM, Viresh Kumar wrote:
> On 9 July 2014 20:23, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>> I scanned this patch and subsequent patches from the series. Since you are
>> modifying the interfaces and bindings, I just think its better if we can
>> address the cases where separate clock lines will be used by CPUs.
>>
>> Surely don't want to increase your work neither want hold the progress
>> of the series but if you look at the changes to the interfaces, they
>> give you a feeling of incompleteness.
> 
> Lets talk in the other thread you raised, 2/14 ..
> 
>>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>>> ---
>> Would you able to give some idea about what will it take to address that one
>> remainder case as well as part of this series.
> 
> Which one? This:
> 
>> We still *don't* support platforms with separate clock/voltage lines for CPUs.
>> This would be done in a separate patch.
> 
> Its already fixed as part of this series.
> 
I suggest you word the commit in that case. Saying subsequent patch
adds support for the remainder case. Let me scan the remainder
patches again to see how its done.

Regards,
Santosh

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
  2014-07-09 15:26       ` Santosh Shilimkar
@ 2014-07-09 15:27         ` Viresh Kumar
  2014-07-09 15:29           ` Santosh Shilimkar
  0 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-09 15:27 UTC (permalink / raw)
  To: Santosh Shilimkar
  Cc: Rafael J. Wysocki, Shawn Guo, Nishanth Menon,
	Lists linaro-kernel, linux-pm, linux-arm-msm, Tomasz Figa,
	Stephen Boyd, Linux Kernel Mailing List, Thomas P Abraham,
	Arvind Chauhan, Sachin Kamat, Lorenzo Pieralisi

On 9 July 2014 20:56, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>>> We still *don't* support platforms with separate clock/voltage lines for CPUs.
>>> This would be done in a separate patch.

Probably s/separate/later would be enough to make you happy ?

>> Its already fixed as part of this series.
>>
> I suggest you word the commit in that case. Saying subsequent patch
> adds support for the remainder case. Let me scan the remainder
> patches again to see how its done.
>
> Regards,
> Santosh

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
  2014-07-09 15:27         ` Viresh Kumar
@ 2014-07-09 15:29           ` Santosh Shilimkar
  2014-07-09 15:33             ` Viresh Kumar
  0 siblings, 1 reply; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-09 15:29 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael J. Wysocki, Shawn Guo, Nishanth Menon,
	Lists linaro-kernel, linux-pm, linux-arm-msm, Tomasz Figa,
	Stephen Boyd, Linux Kernel Mailing List, Thomas P Abraham,
	Arvind Chauhan, Sachin Kamat, Lorenzo Pieralisi

On Wednesday 09 July 2014 11:27 AM, Viresh Kumar wrote:
> On 9 July 2014 20:56, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>>>> We still *don't* support platforms with separate clock/voltage lines for CPUs.
>>>> This would be done in a separate patch.
> 
> Probably s/separate/later would be enough to make you happy ?
> 
For reviewer, its easy if this later is "in the current series" or
really a "later" series.
Anyway I understood it now, so its upto you.

regards,
Santosh

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
  2014-07-09 15:29           ` Santosh Shilimkar
@ 2014-07-09 15:33             ` Viresh Kumar
  0 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-09 15:33 UTC (permalink / raw)
  To: Santosh Shilimkar
  Cc: Rafael J. Wysocki, Shawn Guo, Nishanth Menon,
	Lists linaro-kernel, linux-pm, linux-arm-msm, Tomasz Figa,
	Stephen Boyd, Linux Kernel Mailing List, Thomas P Abraham,
	Arvind Chauhan, Sachin Kamat, Lorenzo Pieralisi

On 9 July 2014 20:59, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> On Wednesday 09 July 2014 11:27 AM, Viresh Kumar wrote:
>> On 9 July 2014 20:56, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>>>>> We still *don't* support platforms with separate clock/voltage lines for CPUs.
>>>>> This would be done in a separate patch.
>>
>> Probably s/separate/later would be enough to make you happy ?
>>
> For reviewer, its easy if this later is "in the current series" or
> really a "later" series.

As you say  BOSS :)

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-08  4:50           ` Viresh Kumar
@ 2014-07-09 17:41             ` Stephen Boyd
  2014-07-16 16:01             ` Viresh Kumar
  1 sibling, 0 replies; 83+ messages in thread
From: Stephen Boyd @ 2014-07-09 17:41 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Mike Turquette, Rafael J. Wysocki, Lists linaro-kernel, linux-pm,
	Arvind Chauhan, linux-arm-msm, Sachin Kamat, Thomas P Abraham,
	Shawn Guo, Linux Kernel Mailing List, Nishanth Menon,
	Tomasz Figa, devicetree, Kukjin Kim, Michal Simek, Rob Herring,
	Santosh Shilimkar, Simon Horman

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 07/14] cpufreq: cpu0: OPPs can be populated at runtime
  2014-07-09 14:44     ` Santosh Shilimkar
  (?)
  (?)
@ 2014-07-10 11:19     ` Viresh Kumar
  2014-07-10 12:39       ` Nishanth Menon
  -1 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-10 11:19 UTC (permalink / raw)
  To: Santosh Shilimkar
  Cc: Rafael J. Wysocki, Shawn Guo, Nishanth Menon,
	Lists linaro-kernel, linux-pm, linux-arm-msm, Tomasz Figa,
	Stephen Boyd, Linux Kernel Mailing List, Thomas P Abraham,
	Arvind Chauhan, Sachin Kamat

On 9 July 2014 20:14, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> Assuming you are updating bidnings as suggested by Stephen,
> patch looks good to me.
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

Why do you still have a separate cpufreq driver for omap?
Would this patch help getting that out?

I see this for omap:

static inline void omap_init_cpufreq(void)
{
        struct platform_device_info devinfo = { };

        if (!of_have_populated_dt())
                devinfo.name = "omap-cpufreq";
        else
                devinfo.name = "cpufreq-generic";
        platform_device_register_full(&devinfo);
}

and it makes me believe that you were just waiting for this patch?

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 07/14] cpufreq: cpu0: OPPs can be populated at runtime
  2014-07-10 11:19     ` Viresh Kumar
@ 2014-07-10 12:39       ` Nishanth Menon
  2014-07-10 13:31         ` Santosh Shilimkar
  0 siblings, 1 reply; 83+ messages in thread
From: Nishanth Menon @ 2014-07-10 12:39 UTC (permalink / raw)
  To: Viresh Kumar, Tony Lindgren
  Cc: Santosh Shilimkar, Rafael J. Wysocki, Shawn Guo,
	Lists linaro-kernel, linux-pm, linux-arm-msm, Tomasz Figa,
	Stephen Boyd, Linux Kernel Mailing List, Thomas P Abraham,
	Arvind Chauhan, Sachin Kamat, Dave Gerlach, linux-omap

On Thu, Jul 10, 2014 at 6:19 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 9 July 2014 20:14, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>> Assuming you are updating bidnings as suggested by Stephen,
>> patch looks good to me.
>> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>
> Why do you still have a separate cpufreq driver for omap?
> Would this patch help getting that out?
>
> I see this for omap:
>
> static inline void omap_init_cpufreq(void)
> {
>         struct platform_device_info devinfo = { };
>
>         if (!of_have_populated_dt())
>                 devinfo.name = "omap-cpufreq";
>         else
>                 devinfo.name = "cpufreq-generic";
>         platform_device_register_full(&devinfo);
> }
>
> and it makes me believe that you were just waiting for this patch?

Sorry, am away on vacation and slow on emails. The plan was to kill
omap cpufreq once all platforms convert to device tree only boot. Only
platform left is OMAP3 based platforms - though the date for removing
non-dt support has changed a couple of kernel revisions - but we
should be able to remove that entire file with this change.

We will need this support to go with the solution recommended for opp
modifier series[1] - where platform code will populate or add OPPs
based on "speed grade" sample detection.

[1]http://comments.gmane.org/gmane.linux.ports.arm.kernel/309466

---
Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 07/14] cpufreq: cpu0: OPPs can be populated at runtime
  2014-07-10 12:39       ` Nishanth Menon
@ 2014-07-10 13:31         ` Santosh Shilimkar
  2014-07-10 13:36           ` Viresh Kumar
  0 siblings, 1 reply; 83+ messages in thread
From: Santosh Shilimkar @ 2014-07-10 13:31 UTC (permalink / raw)
  To: Nishanth Menon, Viresh Kumar, Tony Lindgren
  Cc: Rafael J. Wysocki, Shawn Guo, Lists linaro-kernel, linux-pm,
	linux-arm-msm, Tomasz Figa, Stephen Boyd,
	Linux Kernel Mailing List, Thomas P Abraham, Arvind Chauhan,
	Sachin Kamat, Dave Gerlach, linux-omap

On Thursday 10 July 2014 08:39 AM, Nishanth Menon wrote:
> On Thu, Jul 10, 2014 at 6:19 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> On 9 July 2014 20:14, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>>> Assuming you are updating bidnings as suggested by Stephen,
>>> patch looks good to me.
>>> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>
>> Why do you still have a separate cpufreq driver for omap?
>> Would this patch help getting that out?
>>
>> I see this for omap:
>>
>> static inline void omap_init_cpufreq(void)
>> {
>>         struct platform_device_info devinfo = { };
>>
>>         if (!of_have_populated_dt())
>>                 devinfo.name = "omap-cpufreq";
>>         else
>>                 devinfo.name = "cpufreq-generic";
>>         platform_device_register_full(&devinfo);
>> }
>>
>> and it makes me believe that you were just waiting for this patch?
> 
> Sorry, am away on vacation and slow on emails. The plan was to kill
> omap cpufreq once all platforms convert to device tree only boot. Only
> platform left is OMAP3 based platforms - though the date for removing
> non-dt support has changed a couple of kernel revisions - but we
> should be able to remove that entire file with this change.
> 
> We will need this support to go with the solution recommended for opp
> modifier series[1] - where platform code will populate or add OPPs
> based on "speed grade" sample detection.
>
Yep. Last time I blocked the series because all the DT conversions
were not done. Considering now the cpufreq-generic can work on non
DT platforms, I am ok to remove the omap-cpufreq.

Regards,
Santosh

 

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 07/14] cpufreq: cpu0: OPPs can be populated at runtime
  2014-07-10 13:31         ` Santosh Shilimkar
@ 2014-07-10 13:36           ` Viresh Kumar
  0 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-10 13:36 UTC (permalink / raw)
  To: Santosh Shilimkar
  Cc: Nishanth Menon, Tony Lindgren, Rafael J. Wysocki, Shawn Guo,
	Lists linaro-kernel, linux-pm, linux-arm-msm, Tomasz Figa,
	Stephen Boyd, Linux Kernel Mailing List, Thomas P Abraham,
	Arvind Chauhan, Sachin Kamat, Dave Gerlach, linux-omap

On 10 July 2014 19:01, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> Yep. Last time I blocked the series because all the DT conversions
> were not done. Considering now the cpufreq-generic can work on non
> DT platforms, I am ok to remove the omap-cpufreq.

Great.

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-08  4:50           ` Viresh Kumar
  2014-07-09 17:41             ` Stephen Boyd
@ 2014-07-16 16:01             ` Viresh Kumar
  2014-07-16 17:28               ` Thomas Petazzoni
  2014-07-16 21:18               ` Rafael J. Wysocki
  1 sibling, 2 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-16 16:01 UTC (permalink / raw)
  To: Mike Turquette, Rafael J. Wysocki, Thomas Petazzoni
  Cc: Stephen Boyd, Lists linaro-kernel, linux-pm, Arvind Chauhan,
	linux-arm-msm, Sachin Kamat, Thomas P Abraham, Shawn Guo,
	Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
	devicetree, Kukjin Kim, Michal Simek, Rob Herring,
	Santosh Shilimkar, Simon Horman

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-16 16:01             ` Viresh Kumar
@ 2014-07-16 17:28               ` Thomas Petazzoni
  2014-07-16 21:17                 ` Rafael J. Wysocki
  2014-07-16 21:18               ` Rafael J. Wysocki
  1 sibling, 1 reply; 83+ messages in thread
From: Thomas Petazzoni @ 2014-07-16 17:28 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Nishanth Menon, Andrew Lunn, Rafael J. Wysocki, Tomasz Figa,
	Arvind Chauhan, Rob Herring, Kukjin Kim, Michal Simek,
	Thomas P Abraham, Sebastian Hesselbarth, devicetree,
	Lists linaro-kernel, Jason Cooper, linux-pm, linux-arm-msm,
	Simon Horman, Gregory Clément, Sachin Kamat, Stephen Boyd,
	Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-16 17:28               ` Thomas Petazzoni
@ 2014-07-16 21:17                 ` Rafael J. Wysocki
  0 siblings, 0 replies; 83+ messages in thread
From: Rafael J. Wysocki @ 2014-07-16 21:17 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Nishanth Menon, Andrew Lunn, Viresh Kumar, Tomasz Figa,
	Arvind Chauhan, Rob Herring, Kukjin Kim, Michal Simek,
	Thomas P Abraham, Sebastian Hesselbarth, devicetree,
	Lists linaro-kernel, Jason Cooper, linux-pm, linux-arm-msm,
	Simon Horman, Gregory Clément, Sachin Kamat, Stephen Boyd,
	Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-16 16:01             ` Viresh Kumar
  2014-07-16 17:28               ` Thomas Petazzoni
@ 2014-07-16 21:18               ` Rafael J. Wysocki
  2014-07-17  0:28                 ` Viresh Kumar
  1 sibling, 1 reply; 83+ messages in thread
From: Rafael J. Wysocki @ 2014-07-16 21:18 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Mike Turquette, Thomas Petazzoni, Stephen Boyd,
	Lists linaro-kernel, linux-pm, Arvind Chauhan, linux-arm-msm,
	Sachin Kamat, Thomas P Abraham, Shawn Guo,
	Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
	devicetree, Kukjin Kim, Michal Simek, Rob Herring,
	Santosh Shilimkar, Simon Horman

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-16 21:18               ` Rafael J. Wysocki
@ 2014-07-17  0:28                 ` Viresh Kumar
  2014-07-17  7:35                   ` Thomas Petazzoni
  0 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-17  0:28 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Mike Turquette, Thomas Petazzoni, Stephen Boyd,
	Lists linaro-kernel, linux-pm, Arvind Chauhan, linux-arm-msm,
	Sachin Kamat, Thomas P Abraham, Shawn Guo,
	Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
	devicetree, Kukjin Kim, Michal Simek, Rob Herring,
	Santosh Shilimkar, Simon Horman

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-17  0:28                 ` Viresh Kumar
@ 2014-07-17  7:35                   ` Thomas Petazzoni
  2014-07-17  7:41                     ` Viresh Kumar
  0 siblings, 1 reply; 83+ messages in thread
From: Thomas Petazzoni @ 2014-07-17  7:35 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael J. Wysocki, Mike Turquette, Stephen Boyd,
	Lists linaro-kernel, linux-pm, Arvind Chauhan, linux-arm-msm,
	Sachin Kamat, Thomas P Abraham, Shawn Guo,
	Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
	devicetree, Kukjin Kim, Michal Simek, Rob Herring,
	Santosh Shilimkar, Simon Horman

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-17  7:35                   ` Thomas Petazzoni
@ 2014-07-17  7:41                     ` Viresh Kumar
  2014-07-18  1:02                       ` Rafael J. Wysocki
  0 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-17  7:41 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Nishanth Menon, devicetree, Lists linaro-kernel, Simon Horman,
	linux-pm, Stephen Boyd, Tomasz Figa, Rafael J. Wysocki,
	Linux Kernel Mailing List, Thomas P Abraham, Rob Herring,
	linux-arm-msm, Kukjin Kim, Arvind Chauhan, Sachin Kamat,
	Michal Simek

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..

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-17  7:41                     ` Viresh Kumar
@ 2014-07-18  1:02                       ` Rafael J. Wysocki
  2014-07-18  4:17                         ` Viresh Kumar
  0 siblings, 1 reply; 83+ messages in thread
From: Rafael J. Wysocki @ 2014-07-18  1:02 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Thomas Petazzoni, Nishanth Menon, devicetree,
	Lists linaro-kernel, Simon Horman, linux-pm, Stephen Boyd,
	Tomasz Figa, Linux Kernel Mailing List, Thomas P Abraham,
	Rob Herring, linux-arm-msm, Kukjin Kim, Arvind Chauhan,
	Sachin Kamat, Michal Simek

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-18  1:02                       ` Rafael J. Wysocki
@ 2014-07-18  4:17                         ` Viresh Kumar
       [not found]                           ` <CAKohpomKzK8pMJs1gv+uXxhd17HtCQyfjSnVYw9KpGz6FwbgDA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 83+ messages in thread
From: Viresh Kumar @ 2014-07-18  4:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Stephen Boyd, Rob Herring, Mike Turquette
  Cc: Thomas Petazzoni, Nishanth Menon, devicetree,
	Lists linaro-kernel, Simon Horman, linux-pm, Tomasz Figa,
	Linux Kernel Mailing List, Thomas P Abraham, linux-arm-msm,
	Kukjin Kim, Arvind Chauhan, Sachin Kamat, Michal Simek

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-18  4:17                         ` Viresh Kumar
@ 2014-07-24 10:48                               ` Viresh Kumar
  0 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-24 10:48 UTC (permalink / raw)
  To: Rafael J. Wysocki, Stephen Boyd, Rob Herring, Mike Turquette
  Cc: Thomas Petazzoni, Nishanth Menon,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Lists linaro-kernel,
	Simon Horman, linux-pm-u79uwXL29TY76Z2rM5mHXA, Tomasz Figa,
	Linux Kernel Mailing List, Thomas P Abraham,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Kukjin Kim, Arvind Chauhan,
	Sachin Kamat, Michal Simek

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
@ 2014-07-24 10:48                               ` Viresh Kumar
  0 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-24 10:48 UTC (permalink / raw)
  To: Rafael J. Wysocki, Stephen Boyd, Rob Herring, Mike Turquette
  Cc: Thomas Petazzoni, Nishanth Menon, devicetree,
	Lists linaro-kernel, Simon Horman, linux-pm, Tomasz Figa,
	Linux Kernel Mailing List, Thomas P Abraham, linux-arm-msm,
	Kukjin Kim, Arvind Chauhan, Sachin Kamat, Michal Simek

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 ?

--
viresh

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-24 10:48                               ` Viresh Kumar
  (?)
@ 2014-07-25 14:29                               ` Rob Herring
       [not found]                                 ` <CAL_JsqKqCeU0zs+rS1vxsOeh=Kuw_-gaVHtGU76Lb6TchCTytw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2014-07-25 15:41                                 ` Thomas Petazzoni
  -1 siblings, 2 replies; 83+ messages in thread
From: Rob Herring @ 2014-07-25 14:29 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael J. Wysocki, Stephen Boyd, Rob Herring, Mike Turquette,
	Thomas Petazzoni, Nishanth Menon, Lists linaro-kernel, linux-pm,
	linux-arm-msm, Tomasz Figa, Linux Kernel Mailing List,
	Michal Simek, devicetree, Simon Horman, Thomas P Abraham,
	Kukjin Kim, Arvind Chauhan, Sachin Kamat

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-25 14:29                               ` Rob Herring
@ 2014-07-25 14:34                                     ` Viresh Kumar
  2014-07-25 15:41                                 ` Thomas Petazzoni
  1 sibling, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-25 14:34 UTC (permalink / raw)
  To: Rob Herring
  Cc: Rafael J. Wysocki, Stephen Boyd, Rob Herring, Mike Turquette,
	Thomas Petazzoni, Nishanth Menon, Lists linaro-kernel,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Tomasz Figa,
	Linux Kernel Mailing List, Michal Simek,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Simon Horman,
	Thomas P Abraham, Kukjin Kim, Arvind Chauhan, Sachin Kamat

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
@ 2014-07-25 14:34                                     ` Viresh Kumar
  0 siblings, 0 replies; 83+ messages in thread
From: Viresh Kumar @ 2014-07-25 14:34 UTC (permalink / raw)
  To: Rob Herring
  Cc: Rafael J. Wysocki, Stephen Boyd, Rob Herring, Mike Turquette,
	Thomas Petazzoni, Nishanth Menon, Lists linaro-kernel, linux-pm,
	linux-arm-msm, Tomasz Figa, Linux Kernel Mailing List,
	Michal Simek, devicetree, Simon Horman, Thomas P Abraham,
	Kukjin Kim, Arvind Chauhan, Sachin Kamat

On 25 July 2014 19:59, Rob Herring <robherring2@gmail.com> 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.

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2
  2014-07-25 14:29                               ` Rob Herring
       [not found]                                 ` <CAL_JsqKqCeU0zs+rS1vxsOeh=Kuw_-gaVHtGU76Lb6TchCTytw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2014-07-25 15:41                                 ` Thomas Petazzoni
  1 sibling, 0 replies; 83+ messages in thread
From: Thomas Petazzoni @ 2014-07-25 15:41 UTC (permalink / raw)
  To: Rob Herring
  Cc: Viresh Kumar, Rafael J. Wysocki, Stephen Boyd, Rob Herring,
	Mike Turquette, Nishanth Menon, Lists linaro-kernel, linux-pm,
	linux-arm-msm, Tomasz Figa, Linux Kernel Mailing List,
	Michal Simek, devicetree, Simon Horman, Thomas P Abraham,
	Kukjin Kim, Arvind Chauhan, Sachin Kamat

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

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 02/14] clk: Create of_clk_shared_by_cpus()
       [not found]   ` <5f7164d789e87c62d722b575980c92dfd0504334.1404231535.git.viresh.kumar@linar o.org>
@ 2014-07-28 14:01       ` Grant Likely
  0 siblings, 0 replies; 83+ messages in thread
From: Grant Likely @ 2014-07-28 14:01 UTC (permalink / raw)
  To: rjw, shawn.guo
  Cc: nm, linaro-kernel, linux-pm, linux-arm-msm, t.figa, sboyd,
	linux-kernel, thomas.ab, Viresh Kumar, arvind.chauhan, spk.linux

On Tue,  1 Jul 2014 22:02:31 +0530, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Create a new routine of_clk_shared_by_cpus() that finds if clock lines are
> shared between two CPUs. This is verified by comparing "clocks" property from
> CPU's DT node.
> 
> Returns 1 if clock line is shared between them, 0 if clock isn't shared and
> return appropriate errors in case nodes/properties are missing.
> 
> Cc: Mike Turquette <mturquette@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/clk/clk.c   | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/clk.h |  6 ++++++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 8b73ede..497735c 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -10,6 +10,7 @@
>   */
>  
>  #include <linux/clk-private.h>
> +#include <linux/cpu.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/spinlock.h>
> @@ -2528,6 +2529,61 @@ const char *of_clk_get_parent_name(struct device_node *np, int index)
>  }
>  EXPORT_SYMBOL_GPL(of_clk_get_parent_name);
>  
> +/**
> + * of_clk_shared_by_cpus - Finds if clock line is shared between CPUs
> + * @cpu1, cpu2: CPU numbers
> + *
> + * Finds if clock lines are shared by two CPUs. This is verified by comparing
> + * "clocks" property from CPU's DT node.
> + *
> + * Returns 1 if clock line is shared between them, 0 if clock isn't shared.
> + * Return appropriate errors in case some requirements aren't met.
> + */
> +int of_clk_shared_by_cpus(int cpu1, int cpu2)
> +{
> +	struct device *cpu1_dev, *cpu2_dev;
> +	struct device_node *np1, *np2;
> +	int ret;
> +
> +	cpu1_dev = get_cpu_device(cpu1);
> +	if (!cpu1_dev) {
> +		pr_err("%s: failed to get cpu_dev for cpu%d\n", __func__, cpu1);
> +		return -ENODEV;
> +	}
> +
> +	cpu2_dev = get_cpu_device(cpu2);
> +	if (!cpu2_dev) {
> +		pr_err("%s: failed to get cpu_dev for cpu%d\n", __func__, cpu2);
> +		return -ENODEV;
> +	}
> +
> +	np1 = of_node_get(cpu1_dev->of_node);
> +	if (!np1) {
> +		pr_err("%s: failed to find of_node for cpu%d\n", __func__,
> +		       cpu1);
> +		return -ENODEV;
> +	}
> +
> +	np2 = of_node_get(cpu2_dev->of_node);
> +	if (!np2) {
> +		pr_err("%s: failed to find of_node for cpu%d\n", __func__,
> +		       cpu2);
> +		ret = -ENODEV;
> +		goto put_np1;
> +	}
> +
> +	/* Match "clocks" property */
> +	ret = of_property_match(np1, np2, "clocks");

This looks naïve. It is entirely possible for different clock specifiers
to resolve to the same clock line, or for there to be multple clocks in
the clocks property. This looks like a buggy way to do it. The only
reliable way to determine if two clocks resolve to the same thing is to
resolve the references with the clock controller.

g.

^ permalink raw reply	[flat|nested] 83+ messages in thread

* Re: [PATCH 02/14] clk: Create of_clk_shared_by_cpus()
@ 2014-07-28 14:01       ` Grant Likely
  0 siblings, 0 replies; 83+ messages in thread
From: Grant Likely @ 2014-07-28 14:01 UTC (permalink / raw)
  To: Viresh Kumar, rjw, shawn.guo
  Cc: nm, linaro-kernel, linux-pm, linux-arm-msm, t.figa, sboyd,
	linux-kernel, thomas.ab, Viresh Kumar, arvind.chauhan, spk.linux

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2790 bytes --]

On Tue,  1 Jul 2014 22:02:31 +0530, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Create a new routine of_clk_shared_by_cpus() that finds if clock lines are
> shared between two CPUs. This is verified by comparing "clocks" property from
> CPU's DT node.
> 
> Returns 1 if clock line is shared between them, 0 if clock isn't shared and
> return appropriate errors in case nodes/properties are missing.
> 
> Cc: Mike Turquette <mturquette@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/clk/clk.c   | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/clk.h |  6 ++++++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 8b73ede..497735c 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -10,6 +10,7 @@
>   */
>  
>  #include <linux/clk-private.h>
> +#include <linux/cpu.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/spinlock.h>
> @@ -2528,6 +2529,61 @@ const char *of_clk_get_parent_name(struct device_node *np, int index)
>  }
>  EXPORT_SYMBOL_GPL(of_clk_get_parent_name);
>  
> +/**
> + * of_clk_shared_by_cpus - Finds if clock line is shared between CPUs
> + * @cpu1, cpu2: CPU numbers
> + *
> + * Finds if clock lines are shared by two CPUs. This is verified by comparing
> + * "clocks" property from CPU's DT node.
> + *
> + * Returns 1 if clock line is shared between them, 0 if clock isn't shared.
> + * Return appropriate errors in case some requirements aren't met.
> + */
> +int of_clk_shared_by_cpus(int cpu1, int cpu2)
> +{
> +	struct device *cpu1_dev, *cpu2_dev;
> +	struct device_node *np1, *np2;
> +	int ret;
> +
> +	cpu1_dev = get_cpu_device(cpu1);
> +	if (!cpu1_dev) {
> +		pr_err("%s: failed to get cpu_dev for cpu%d\n", __func__, cpu1);
> +		return -ENODEV;
> +	}
> +
> +	cpu2_dev = get_cpu_device(cpu2);
> +	if (!cpu2_dev) {
> +		pr_err("%s: failed to get cpu_dev for cpu%d\n", __func__, cpu2);
> +		return -ENODEV;
> +	}
> +
> +	np1 = of_node_get(cpu1_dev->of_node);
> +	if (!np1) {
> +		pr_err("%s: failed to find of_node for cpu%d\n", __func__,
> +		       cpu1);
> +		return -ENODEV;
> +	}
> +
> +	np2 = of_node_get(cpu2_dev->of_node);
> +	if (!np2) {
> +		pr_err("%s: failed to find of_node for cpu%d\n", __func__,
> +		       cpu2);
> +		ret = -ENODEV;
> +		goto put_np1;
> +	}
> +
> +	/* Match "clocks" property */
> +	ret = of_property_match(np1, np2, "clocks");

This looks naïve. It is entirely possible for different clock specifiers
to resolve to the same clock line, or for there to be multple clocks in
the clocks property. This looks like a buggy way to do it. The only
reliable way to determine if two clocks resolve to the same thing is to
resolve the references with the clock controller.

g.

^ permalink raw reply	[flat|nested] 83+ messages in thread

end of thread, other threads:[~2014-07-28 14:01 UTC | newest]

Thread overview: 83+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-01 16:32 [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
2014-07-01 16:32 ` [PATCH 01/14] of: Create of_property_match() Viresh Kumar
2014-07-01 16:32 ` [PATCH 02/14] clk: Create of_clk_shared_by_cpus() Viresh Kumar
2014-07-01 18:00   ` Stephen Boyd
2014-07-02  1:57     ` Viresh Kumar
2014-07-09 14:38   ` Santosh Shilimkar
2014-07-09 14:38     ` Santosh Shilimkar
2014-07-09 15:20     ` Viresh Kumar
     [not found]   ` <5f7164d789e87c62d722b575980c92dfd0504334.1404231535.git.viresh.kumar@linar o.org>
2014-07-28 14:01     ` Grant Likely
2014-07-28 14:01       ` Grant Likely
2014-07-01 16:32 ` [PATCH 03/14] cpufreq: Add support for per-policy driver data Viresh Kumar
2014-07-09 14:33   ` Santosh Shilimkar
2014-07-09 14:33     ` Santosh Shilimkar
2014-07-09 15:07     ` Viresh Kumar
2014-07-01 16:32 ` [PATCH 04/14] cpufreq: cpu0: Add Module Author Viresh Kumar
2014-07-09 14:42   ` Santosh Shilimkar
2014-07-09 14:42     ` Santosh Shilimkar
2014-07-09 15:08     ` Viresh Kumar
2014-07-09 15:24       ` Santosh Shilimkar
2014-07-01 16:32 ` [PATCH 05/14] cpufreq: cpu0: don't validate clock on clk_put() Viresh Kumar
2014-07-09 14:42   ` Santosh Shilimkar
2014-07-09 14:42     ` Santosh Shilimkar
2014-07-01 16:32 ` [PATCH 06/14] cpufreq: cpu0: defer probe if clock isn't registered yet Viresh Kumar
2014-07-02  5:53   ` Shawn Guo
2014-07-02  5:53     ` Shawn Guo
2014-07-02  5:55     ` Viresh Kumar
2014-07-02 11:32       ` Viresh Kumar
2014-07-03  0:38         ` Stephen Boyd
2014-07-03  2:19           ` Viresh Kumar
2014-07-09 14:43             ` Santosh Shilimkar
2014-07-09 14:43               ` Santosh Shilimkar
2014-07-01 16:32 ` [PATCH 07/14] cpufreq: cpu0: OPPs can be populated at runtime Viresh Kumar
2014-07-01 18:02   ` Stephen Boyd
2014-07-02  2:03     ` Viresh Kumar
2014-07-02  4:03   ` [PATCH V2 Resend " Viresh Kumar
2014-07-09 14:44   ` [PATCH " Santosh Shilimkar
2014-07-09 14:44     ` Santosh Shilimkar
2014-07-09 15:09     ` Viresh Kumar
2014-07-10 11:19     ` Viresh Kumar
2014-07-10 12:39       ` Nishanth Menon
2014-07-10 13:31         ` Santosh Shilimkar
2014-07-10 13:36           ` Viresh Kumar
2014-07-01 16:32 ` [PATCH 08/14] cpufreq: cpu0: use dev_{err|warn|dbg} instead of pr_{err|warn|debug} Viresh Kumar
2014-07-09 14:45   ` Santosh Shilimkar
2014-07-09 14:45     ` Santosh Shilimkar
2014-07-01 16:32 ` [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init() Viresh Kumar
2014-07-03  0:43   ` Stephen Boyd
2014-07-03  2:11     ` Viresh Kumar
2014-07-09 14:53   ` Santosh Shilimkar
2014-07-09 14:53     ` Santosh Shilimkar
2014-07-09 15:17     ` Viresh Kumar
2014-07-09 15:26       ` Santosh Shilimkar
2014-07-09 15:27         ` Viresh Kumar
2014-07-09 15:29           ` Santosh Shilimkar
2014-07-09 15:33             ` Viresh Kumar
2014-07-01 16:32 ` [PATCH 10/14] cpufreq: cpu0: try regulators with name "cpu-supply" Viresh Kumar
2014-07-01 16:32 ` [PATCH 11/14] cpufreq: cpu0: Make allocate_resources() work for any CPU Viresh Kumar
2014-07-01 16:32 ` [PATCH 12/14] cpufreq: cpu0: Extend support beyond CPU0 Viresh Kumar
2014-07-02  4:03   ` [PATCH V2 Resend " Viresh Kumar
2014-07-01 16:32 ` [PATCH 13/14] cpufreq: cpu0: rename driver and internals to 'cpufreq_generic' Viresh Kumar
2014-07-01 16:32 ` [PATCH 14/14] cpufreq: generic: set platform_{driver|device} '.name' to 'cpufreq-generic' Viresh Kumar
2014-07-02  4:12 ` [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
2014-07-03  1:24   ` Stephen Boyd
2014-07-03  2:44     ` Viresh Kumar
2014-07-03 22:16       ` Mike Turquette
2014-07-04  4:21         ` Viresh Kumar
2014-07-08  4:50           ` Viresh Kumar
2014-07-09 17:41             ` Stephen Boyd
2014-07-16 16:01             ` Viresh Kumar
2014-07-16 17:28               ` Thomas Petazzoni
2014-07-16 21:17                 ` Rafael J. Wysocki
2014-07-16 21:18               ` Rafael J. Wysocki
2014-07-17  0:28                 ` Viresh Kumar
2014-07-17  7:35                   ` Thomas Petazzoni
2014-07-17  7:41                     ` Viresh Kumar
2014-07-18  1:02                       ` Rafael J. Wysocki
2014-07-18  4:17                         ` Viresh Kumar
     [not found]                           ` <CAKohpomKzK8pMJs1gv+uXxhd17HtCQyfjSnVYw9KpGz6FwbgDA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-07-24 10:48                             ` Viresh Kumar
2014-07-24 10:48                               ` Viresh Kumar
2014-07-25 14:29                               ` Rob Herring
     [not found]                                 ` <CAL_JsqKqCeU0zs+rS1vxsOeh=Kuw_-gaVHtGU76Lb6TchCTytw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-07-25 14:34                                   ` Viresh Kumar
2014-07-25 14:34                                     ` Viresh Kumar
2014-07-25 15:41                                 ` Thomas Petazzoni

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.