* [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 12/14] cpufreq: cpu0: Extend support beyond CPU0 Viresh Kumar
2014-07-02 4:12 ` [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
2 siblings, 0 replies; 24+ 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] 24+ 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
2014-07-01 16:32 ` [PATCH 01/14] of: Create of_property_match() Viresh Kumar
@ 2014-07-01 16:32 ` Viresh Kumar
2014-07-02 4:03 ` [PATCH V2 Resend " Viresh Kumar
2014-07-02 4:12 ` [PATCH 00/14] cpufreq: cpu0: Extend support beyond CPU0, V2 Viresh Kumar
2 siblings, 1 reply; 24+ 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] 24+ 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; 24+ 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] 24+ 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
2014-07-01 16:32 ` [PATCH 01/14] of: Create of_property_match() Viresh Kumar
2014-07-01 16:32 ` [PATCH 12/14] cpufreq: cpu0: Extend support beyond CPU0 Viresh Kumar
@ 2014-07-02 4:12 ` Viresh Kumar
2014-07-03 1:24 ` Stephen Boyd
2 siblings, 1 reply; 24+ 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@vger.kernel.org, Arvind Chauhan,
linux-arm-msm, Sachin Kamat, Thomas P Abraham, Shawn Guo,
Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
Viresh Kumar, devicetree@vger.kernel.org, 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] 24+ 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; 24+ 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@vger.kernel.org,
Arvind Chauhan, linux-arm-msm, Sachin Kamat, Thomas P Abraham,
Shawn Guo, Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
devicetree@vger.kernel.org, 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] 24+ 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; 24+ 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@vger.kernel.org,
Arvind Chauhan, linux-arm-msm, Sachin Kamat, Thomas P Abraham,
Shawn Guo, Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
devicetree@vger.kernel.org, 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] 24+ 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; 24+ 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@vger.kernel.org,
Arvind Chauhan, linux-arm-msm, Sachin Kamat, Thomas P Abraham,
Shawn Guo, Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
devicetree@vger.kernel.org, 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] 24+ 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; 24+ 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@vger.kernel.org, Arvind Chauhan, linux-arm-msm,
Sachin Kamat, Thomas P Abraham, Shawn Guo,
Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
devicetree@vger.kernel.org, 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] 24+ 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; 24+ 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@vger.kernel.org, Arvind Chauhan,
linux-arm-msm@vger.kernel.org, Sachin Kamat, Thomas P Abraham,
Shawn Guo, Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
devicetree@vger.kernel.org, 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] 24+ 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; 24+ 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@vger.kernel.org, Arvind Chauhan,
linux-arm-msm@vger.kernel.org, Sachin Kamat, Thomas P Abraham,
Shawn Guo, Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
devicetree@vger.kernel.org, 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] 24+ 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; 24+ 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@vger.kernel.org,
Arvind Chauhan, linux-arm-msm@vger.kernel.org, Sachin Kamat,
Thomas P Abraham, Shawn Guo, Linux Kernel Mailing List,
Nishanth Menon, Tomasz Figa, devicetree@vger.kernel.org,
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] 24+ 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; 24+ 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@vger.kernel.org, Lists linaro-kernel, Jason Cooper,
linux-pm@vger.kernel.org, linux-arm-msm@vger.kernel.org,
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] 24+ 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; 24+ 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@vger.kernel.org, Lists linaro-kernel, Jason Cooper,
linux-pm@vger.kernel.org, linux-arm-msm@vger.kernel.org,
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] 24+ 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; 24+ 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@vger.kernel.org, Arvind Chauhan,
linux-arm-msm@vger.kernel.org, Sachin Kamat, Thomas P Abraham,
Shawn Guo, Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
devicetree@vger.kernel.org, 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] 24+ 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; 24+ 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@vger.kernel.org, Arvind Chauhan,
linux-arm-msm@vger.kernel.org, Sachin Kamat, Thomas P Abraham,
Shawn Guo, Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
devicetree@vger.kernel.org, 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] 24+ 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; 24+ 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@vger.kernel.org, Arvind Chauhan,
linux-arm-msm@vger.kernel.org, Sachin Kamat, Thomas P Abraham,
Shawn Guo, Linux Kernel Mailing List, Nishanth Menon, Tomasz Figa,
devicetree@vger.kernel.org, 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] 24+ 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; 24+ messages in thread
From: Viresh Kumar @ 2014-07-17 7:41 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Nishanth Menon, devicetree@vger.kernel.org, Lists linaro-kernel,
Simon Horman, linux-pm@vger.kernel.org, Stephen Boyd, Tomasz Figa,
Rafael J. Wysocki, Linux Kernel Mailing List, Thomas P Abraham,
Rob Herring, linux-arm-msm@vger.kernel.org, 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] 24+ 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; 24+ messages in thread
From: Rafael J. Wysocki @ 2014-07-18 1:02 UTC (permalink / raw)
To: Viresh Kumar
Cc: Thomas Petazzoni, Nishanth Menon, devicetree@vger.kernel.org,
Lists linaro-kernel, Simon Horman, linux-pm@vger.kernel.org,
Stephen Boyd, Tomasz Figa, Linux Kernel Mailing List,
Thomas P Abraham, Rob Herring, linux-arm-msm@vger.kernel.org,
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] 24+ 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; 24+ 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@vger.kernel.org,
Lists linaro-kernel, Simon Horman, linux-pm@vger.kernel.org,
Tomasz Figa, Linux Kernel Mailing List, Thomas P Abraham,
linux-arm-msm@vger.kernel.org, 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] 24+ messages in thread