[RFC] cpufreq: Add bindings for CPU clock sharing topology

[RFC] cpufreq: Add bindings for CPU clock sharing topology

[Viresh Kumar]

Clock lines may or may not be shared among different CPUs on a platform. When
clock lines are shared between CPUs, they change DVFS state together.

Possible configurations:

1.) All CPUs share a single clock line.
2.) All CPUs have independent clock lines.
3.) CPUs within a group/cluster share clock line but each group/cluster have a
    separate line for itself.

There is no generic way available today to detect which CPUs share clock lines
and so this is an attempt towards that.

Much of the information is present in the commit and so no point duplicating it
here.

These are obviously not finalized yet and this is an attempt to initiate a
discussion around this.

Please share your valuable feedback.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
I wasn't sure about the path/name of this file, so please don't blast me on that
:)

 .../devicetree/bindings/cpufreq/cpu_clocks.txt     | 159 +++++++++++++++++++++
 1 file changed, 159 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt

diff --git a/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt b/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
new file mode 100644
index 0000000..30ce9ad
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
@@ -0,0 +1,159 @@
+* Generic CPUFreq clock bindings
+
+Clock lines may or may not be shared among different CPUs on a platform.
+
+Possible configurations:
+1.) All CPUs share a single clock line
+2.) All CPUs have independent clock lines
+3.) CPUs within a group/cluster share clock line but each group/cluster have a
+    separate line for itself
+
+Optional Properties:
+- clock-master: This declares cpu as clock master. Other CPUs can either define
+  "clock-ganged" or "clock-master" property, but shouldn't be missing both.
+
+- clock-ganged: Should have phandle of a cpu declared as "clock-master".
+
+If any cpu node, doesn't have both "clock-master" and "clock-ganged" properties
+defined, it would be assumed that all CPUs on that platform share a single clock
+line. This will help supporting already upstreamed platforms.
+
+
+Examples:
+1.) All CPUs share a single clock line
+
+cpus {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	cpu0: cpu@0 {
+		compatible = "arm,cortex-a15";
+		reg = <0>;
+		next-level-cache = <&L2>;
+		clock-master;
+		operating-points = <
+			/* kHz    uV */
+			792000  1100000
+			396000  950000
+			198000  850000
+		>;
+		clock-latency = <61036>; /* two CLK32 periods */
+	};
+
+	cpu1: cpu@1 {
+		compatible = "arm,cortex-a15";
+		reg = <1>;
+		next-level-cache = <&L2>;
+		clock-ganged = <&cpu0>;
+	};
+};
+
+OR (clock-master/ganged aren't defined)
+
+cpus {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	cpu0: cpu@0 {
+		compatible = "arm,cortex-a15";
+		reg = <0>;
+		next-level-cache = <&L2>;
+		operating-points = <
+			/* kHz    uV */
+			792000  1100000
+			396000  950000
+			198000  850000
+		>;
+		clock-latency = <61036>; /* two CLK32 periods */
+	};
+
+	cpu1: cpu@1 {
+		compatible = "arm,cortex-a15";
+		reg = <1>;
+		next-level-cache = <&L2>;
+	};
+};
+
+2.) All CPUs have independent clock lines
+cpus {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	cpu0: cpu@0 {
+		compatible = "arm,cortex-a15";
+		reg = <0>;
+		next-level-cache = <&L2>;
+		clock-master;
+		operating-points = <
+			/* kHz    uV */
+			792000  1100000
+			396000  950000
+			198000  850000
+		>;
+		clock-latency = <61036>; /* two CLK32 periods */
+	};
+
+	cpu1: cpu@1 {
+		compatible = "arm,cortex-a15";
+		reg = <1>;
+		next-level-cache = <&L2>;
+		clock-master;
+		operating-points = <
+			/* kHz    uV */
+			792000  1100000
+			396000  950000
+			198000  850000
+		>;
+		clock-latency = <61036>; /* two CLK32 periods */
+	};
+};
+
+3.) CPUs within a group/cluster share single clock line but each group/cluster
+have a separate line for itself
+
+cpus {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	cpu0: cpu@0 {
+		compatible = "arm,cortex-a15";
+		reg = <0>;
+		next-level-cache = <&L2>;
+		clock-master;
+		operating-points = <
+			/* kHz    uV */
+			792000  1100000
+			396000  950000
+			198000  850000
+		>;
+		clock-latency = <61036>; /* two CLK32 periods */
+	};
+
+	cpu1: cpu@1 {
+		compatible = "arm,cortex-a15";
+		reg = <1>;
+		next-level-cache = <&L2>;
+		clock-ganged = <&cpu0>;
+	};
+
+	cpu2: cpu@100 {
+		compatible = "arm,cortex-a7";
+		reg = <100>;
+		next-level-cache = <&L2>;
+		clock-master;
+		operating-points = <
+			/* kHz    uV */
+			792000  950000
+			396000  750000
+			198000  450000
+		>;
+		clock-latency = <61036>; /* two CLK32 periods */
+	};
+
+	cpu3: cpu@101 {
+		compatible = "arm,cortex-a7";
+		reg = <101>;
+		next-level-cache = <&L2>;
+		clock-ganged = <&cpu2>;
+	};
+};
-- 
2.0.0.rc2

[RFC] cpufreq: Add bindings for CPU clock sharing topology

[Viresh Kumar]

Clock lines may or may not be shared among different CPUs on a platform. When
clock lines are shared between CPUs, they change DVFS state together.

Possible configurations:

1.) All CPUs share a single clock line.
2.) All CPUs have independent clock lines.
3.) CPUs within a group/cluster share clock line but each group/cluster have a
    separate line for itself.

There is no generic way available today to detect which CPUs share clock lines
and so this is an attempt towards that.

Much of the information is present in the commit and so no point duplicating it
here.

These are obviously not finalized yet and this is an attempt to initiate a
discussion around this.

Please share your valuable feedback.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
I wasn't sure about the path/name of this file, so please don't blast me on that
:)

 .../devicetree/bindings/cpufreq/cpu_clocks.txt     | 159 +++++++++++++++++++++
 1 file changed, 159 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt

diff --git a/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt b/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
new file mode 100644
index 0000000..30ce9ad
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
@@ -0,0 +1,159 @@
+* Generic CPUFreq clock bindings
+
+Clock lines may or may not be shared among different CPUs on a platform.
+
+Possible configurations:
+1.) All CPUs share a single clock line
+2.) All CPUs have independent clock lines
+3.) CPUs within a group/cluster share clock line but each group/cluster have a
+    separate line for itself
+
+Optional Properties:
+- clock-master: This declares cpu as clock master. Other CPUs can either define
+  "clock-ganged" or "clock-master" property, but shouldn't be missing both.
+
+- clock-ganged: Should have phandle of a cpu declared as "clock-master".
+
+If any cpu node, doesn't have both "clock-master" and "clock-ganged" properties
+defined, it would be assumed that all CPUs on that platform share a single clock
+line. This will help supporting already upstreamed platforms.
+
+
+Examples:
+1.) All CPUs share a single clock line
+
+cpus {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	cpu0: cpu at 0 {
+		compatible = "arm,cortex-a15";
+		reg = <0>;
+		next-level-cache = <&L2>;
+		clock-master;
+		operating-points = <
+			/* kHz    uV */
+			792000  1100000
+			396000  950000
+			198000  850000
+		>;
+		clock-latency = <61036>; /* two CLK32 periods */
+	};
+
+	cpu1: cpu at 1 {
+		compatible = "arm,cortex-a15";
+		reg = <1>;
+		next-level-cache = <&L2>;
+		clock-ganged = <&cpu0>;
+	};
+};
+
+OR (clock-master/ganged aren't defined)
+
+cpus {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	cpu0: cpu at 0 {
+		compatible = "arm,cortex-a15";
+		reg = <0>;
+		next-level-cache = <&L2>;
+		operating-points = <
+			/* kHz    uV */
+			792000  1100000
+			396000  950000
+			198000  850000
+		>;
+		clock-latency = <61036>; /* two CLK32 periods */
+	};
+
+	cpu1: cpu at 1 {
+		compatible = "arm,cortex-a15";
+		reg = <1>;
+		next-level-cache = <&L2>;
+	};
+};
+
+2.) All CPUs have independent clock lines
+cpus {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	cpu0: cpu at 0 {
+		compatible = "arm,cortex-a15";
+		reg = <0>;
+		next-level-cache = <&L2>;
+		clock-master;
+		operating-points = <
+			/* kHz    uV */
+			792000  1100000
+			396000  950000
+			198000  850000
+		>;
+		clock-latency = <61036>; /* two CLK32 periods */
+	};
+
+	cpu1: cpu at 1 {
+		compatible = "arm,cortex-a15";
+		reg = <1>;
+		next-level-cache = <&L2>;
+		clock-master;
+		operating-points = <
+			/* kHz    uV */
+			792000  1100000
+			396000  950000
+			198000  850000
+		>;
+		clock-latency = <61036>; /* two CLK32 periods */
+	};
+};
+
+3.) CPUs within a group/cluster share single clock line but each group/cluster
+have a separate line for itself
+
+cpus {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	cpu0: cpu at 0 {
+		compatible = "arm,cortex-a15";
+		reg = <0>;
+		next-level-cache = <&L2>;
+		clock-master;
+		operating-points = <
+			/* kHz    uV */
+			792000  1100000
+			396000  950000
+			198000  850000
+		>;
+		clock-latency = <61036>; /* two CLK32 periods */
+	};
+
+	cpu1: cpu at 1 {
+		compatible = "arm,cortex-a15";
+		reg = <1>;
+		next-level-cache = <&L2>;
+		clock-ganged = <&cpu0>;
+	};
+
+	cpu2: cpu at 100 {
+		compatible = "arm,cortex-a7";
+		reg = <100>;
+		next-level-cache = <&L2>;
+		clock-master;
+		operating-points = <
+			/* kHz    uV */
+			792000  950000
+			396000  750000
+			198000  450000
+		>;
+		clock-latency = <61036>; /* two CLK32 periods */
+	};
+
+	cpu3: cpu at 101 {
+		compatible = "arm,cortex-a7";
+		reg = <101>;
+		next-level-cache = <&L2>;
+		clock-ganged = <&cpu2>;
+	};
+};
-- 
2.0.0.rc2

Re: [RFC] cpufreq: Add bindings for CPU clock sharing topology

[Olof Johansson]

On Thu, Jul 17, 2014 at 10:35 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Clock lines may or may not be shared among different CPUs on a platform. When
> clock lines are shared between CPUs, they change DVFS state together.
>
> Possible configurations:
>
> 1.) All CPUs share a single clock line.
> 2.) All CPUs have independent clock lines.
> 3.) CPUs within a group/cluster share clock line but each group/cluster have a
>     separate line for itself.
>
> There is no generic way available today to detect which CPUs share clock lines
> and so this is an attempt towards that.
>
> Much of the information is present in the commit and so no point duplicating it
> here.
>
> These are obviously not finalized yet and this is an attempt to initiate a
> discussion around this.
>
> Please share your valuable feedback.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> I wasn't sure about the path/name of this file, so please don't blast me on that
> :)
>
>  .../devicetree/bindings/cpufreq/cpu_clocks.txt     | 159 +++++++++++++++++++++
>  1 file changed, 159 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
>
> diff --git a/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt b/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
> new file mode 100644
> index 0000000..30ce9ad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
> @@ -0,0 +1,159 @@
> +* Generic CPUFreq clock bindings
> +
> +Clock lines may or may not be shared among different CPUs on a platform.
> +
> +Possible configurations:
> +1.) All CPUs share a single clock line
> +2.) All CPUs have independent clock lines
> +3.) CPUs within a group/cluster share clock line but each group/cluster have a
> +    separate line for itself
> +
> +Optional Properties:
> +- clock-master: This declares cpu as clock master. Other CPUs can either define
> +  "clock-ganged" or "clock-master" property, but shouldn't be missing both.
> +
> +- clock-ganged: Should have phandle of a cpu declared as "clock-master".

Why complicate it by using two properties?

If there is no property, then the CPUs are assumed to be controlled
independently.

if there is a clock-master = <phandle> property, then that points at
the cpu that is the main one controlling clock for the group.

There's really no need to label the master -- it will be the only one
with incoming links but nothing outgoing. And a master without slaves
is an independently controlled cpu so you should be fine in that
aspect too.


-Olof

[RFC] cpufreq: Add bindings for CPU clock sharing topology

[Olof Johansson]

On Thu, Jul 17, 2014 at 10:35 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Clock lines may or may not be shared among different CPUs on a platform. When
> clock lines are shared between CPUs, they change DVFS state together.
>
> Possible configurations:
>
> 1.) All CPUs share a single clock line.
> 2.) All CPUs have independent clock lines.
> 3.) CPUs within a group/cluster share clock line but each group/cluster have a
>     separate line for itself.
>
> There is no generic way available today to detect which CPUs share clock lines
> and so this is an attempt towards that.
>
> Much of the information is present in the commit and so no point duplicating it
> here.
>
> These are obviously not finalized yet and this is an attempt to initiate a
> discussion around this.
>
> Please share your valuable feedback.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> I wasn't sure about the path/name of this file, so please don't blast me on that
> :)
>
>  .../devicetree/bindings/cpufreq/cpu_clocks.txt     | 159 +++++++++++++++++++++
>  1 file changed, 159 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
>
> diff --git a/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt b/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
> new file mode 100644
> index 0000000..30ce9ad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
> @@ -0,0 +1,159 @@
> +* Generic CPUFreq clock bindings
> +
> +Clock lines may or may not be shared among different CPUs on a platform.
> +
> +Possible configurations:
> +1.) All CPUs share a single clock line
> +2.) All CPUs have independent clock lines
> +3.) CPUs within a group/cluster share clock line but each group/cluster have a
> +    separate line for itself
> +
> +Optional Properties:
> +- clock-master: This declares cpu as clock master. Other CPUs can either define
> +  "clock-ganged" or "clock-master" property, but shouldn't be missing both.
> +
> +- clock-ganged: Should have phandle of a cpu declared as "clock-master".

Why complicate it by using two properties?

If there is no property, then the CPUs are assumed to be controlled
independently.

if there is a clock-master = <phandle> property, then that points at
the cpu that is the main one controlling clock for the group.

There's really no need to label the master -- it will be the only one
with incoming links but nothing outgoing. And a master without slaves
is an independently controlled cpu so you should be fine in that
aspect too.


-Olof

Re: [RFC] cpufreq: Add bindings for CPU clock sharing topology

[Viresh Kumar]

On 18 July 2014 11:47, Olof Johansson <olof@lixom.net> wrote:
> Why complicate it by using two properties?
>
> If there is no property, then the CPUs are assumed to be controlled
> independently.
>
> if there is a clock-master = <phandle> property, then that points at
> the cpu that is the main one controlling clock for the group.
>
> There's really no need to label the master -- it will be the only one
> with incoming links but nothing outgoing. And a master without slaves
> is an independently controlled cpu so you should be fine in that
> aspect too.

I thought so earlier, but then I remembered something I read long back.
Don't remember which thread now, but I *might* be wrong..

"Bindings are like APIs and new bindings shouldn't break existing stuff.."

And:

> If there is no property, then the CPUs are assumed to be controlled
> independently.

seems to break the existing API.

But if that isn't the case, the bindings are very simple & clear to handle.
Diff for new bindings:

diff --git a/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
b/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
index 30ce9ad..d127bba 100644
--- a/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
+++ b/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
@@ -9,15 +9,14 @@ Clock lines may or may not be shared among different
CPUs on a platform.
     separate line for itself

 Optional Properties:
-- clock-master: This declares cpu as clock master. Other CPUs can either define
-  "clock-ganged" or "clock-master" property, but shouldn't be missing both.
+- clock-master: Contains phandle of the master cpu controlling clocks.

-- clock-ganged: Should have phandle of a cpu declared as "clock-master".
-
-If any cpu node, doesn't have both "clock-master" and "clock-ganged" properties
-defined, it would be assumed that all CPUs on that platform share a
single clock
-line. This will help supporting already upstreamed platforms.
+  Ideally there is nothing like a "master" CPU as any CPU can play with DVFS
+  settings. But we have to choose one cpu out of a group, so that others can
+  point to it.

+  If there is no "clock-master" property for a cpu node, it is considered as
+  master. It may or may not have other slave CPUs pointing towards it.

 Examples:
 1.) All CPUs share a single clock line
@@ -30,34 +29,6 @@ cpus {
                compatible = "arm,cortex-a15";
                reg = <0>;
                next-level-cache = <&L2>;
-               clock-master;
-               operating-points = <
-                       /* kHz    uV */
-                       792000  1100000
-                       396000  950000
-                       198000  850000
-               >;
-               clock-latency = <61036>; /* two CLK32 periods */
-       };
-
-       cpu1: cpu@1 {
-               compatible = "arm,cortex-a15";
-               reg = <1>;
-               next-level-cache = <&L2>;
-               clock-ganged = <&cpu0>;
-       };
-};
-
-OR (clock-master/ganged aren't defined)
-
-cpus {
-       #address-cells = <1>;
-       #size-cells = <0>;
-
-       cpu0: cpu@0 {
-               compatible = "arm,cortex-a15";
-               reg = <0>;
-               next-level-cache = <&L2>;
                operating-points = <
                        /* kHz    uV */
                        792000  1100000
@@ -71,6 +42,7 @@ cpus {
                compatible = "arm,cortex-a15";
                reg = <1>;
                next-level-cache = <&L2>;
+               clock-master = <&cpu0>;
        };
 };

@@ -83,7 +55,6 @@ cpus {
                compatible = "arm,cortex-a15";
                reg = <0>;
                next-level-cache = <&L2>;
-               clock-master;
                operating-points = <
                        /* kHz    uV */
                        792000  1100000
@@ -97,7 +68,6 @@ cpus {
                compatible = "arm,cortex-a15";
                reg = <1>;
                next-level-cache = <&L2>;
-               clock-master;
                operating-points = <
                        /* kHz    uV */
                        792000  1100000
@@ -119,7 +89,6 @@ cpus {
                compatible = "arm,cortex-a15";
                reg = <0>;
                next-level-cache = <&L2>;
-               clock-master;
                operating-points = <
                        /* kHz    uV */
                        792000  1100000
@@ -133,14 +102,13 @@ cpus {
                compatible = "arm,cortex-a15";
                reg = <1>;
                next-level-cache = <&L2>;
-               clock-ganged = <&cpu0>;
+               clock-master = <&cpu0>;
        };

        cpu2: cpu@100 {
                compatible = "arm,cortex-a7";
                reg = <100>;
                next-level-cache = <&L2>;
-               clock-master;
                operating-points = <
                        /* kHz    uV */
                        792000  950000
@@ -154,6 +122,6 @@ cpus {
                compatible = "arm,cortex-a7";
                reg = <101>;
                next-level-cache = <&L2>;
-               clock-ganged = <&cpu2>;
+               clock-master = <&cpu2>;
        };
 };

[RFC] cpufreq: Add bindings for CPU clock sharing topology

[Viresh Kumar]

On 18 July 2014 11:47, Olof Johansson <olof@lixom.net> wrote:
> Why complicate it by using two properties?
>
> If there is no property, then the CPUs are assumed to be controlled
> independently.
>
> if there is a clock-master = <phandle> property, then that points at
> the cpu that is the main one controlling clock for the group.
>
> There's really no need to label the master -- it will be the only one
> with incoming links but nothing outgoing. And a master without slaves
> is an independently controlled cpu so you should be fine in that
> aspect too.

I thought so earlier, but then I remembered something I read long back.
Don't remember which thread now, but I *might* be wrong..

"Bindings are like APIs and new bindings shouldn't break existing stuff.."

And:

> If there is no property, then the CPUs are assumed to be controlled
> independently.

seems to break the existing API.

But if that isn't the case, the bindings are very simple & clear to handle.
Diff for new bindings:

diff --git a/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
b/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
index 30ce9ad..d127bba 100644
--- a/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
+++ b/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
@@ -9,15 +9,14 @@ Clock lines may or may not be shared among different
CPUs on a platform.
     separate line for itself

 Optional Properties:
-- clock-master: This declares cpu as clock master. Other CPUs can either define
-  "clock-ganged" or "clock-master" property, but shouldn't be missing both.
+- clock-master: Contains phandle of the master cpu controlling clocks.

-- clock-ganged: Should have phandle of a cpu declared as "clock-master".
-
-If any cpu node, doesn't have both "clock-master" and "clock-ganged" properties
-defined, it would be assumed that all CPUs on that platform share a
single clock
-line. This will help supporting already upstreamed platforms.
+  Ideally there is nothing like a "master" CPU as any CPU can play with DVFS
+  settings. But we have to choose one cpu out of a group, so that others can
+  point to it.

+  If there is no "clock-master" property for a cpu node, it is considered as
+  master. It may or may not have other slave CPUs pointing towards it.

 Examples:
 1.) All CPUs share a single clock line
@@ -30,34 +29,6 @@ cpus {
                compatible = "arm,cortex-a15";
                reg = <0>;
                next-level-cache = <&L2>;
-               clock-master;
-               operating-points = <
-                       /* kHz    uV */
-                       792000  1100000
-                       396000  950000
-                       198000  850000
-               >;
-               clock-latency = <61036>; /* two CLK32 periods */
-       };
-
-       cpu1: cpu at 1 {
-               compatible = "arm,cortex-a15";
-               reg = <1>;
-               next-level-cache = <&L2>;
-               clock-ganged = <&cpu0>;
-       };
-};
-
-OR (clock-master/ganged aren't defined)
-
-cpus {
-       #address-cells = <1>;
-       #size-cells = <0>;
-
-       cpu0: cpu at 0 {
-               compatible = "arm,cortex-a15";
-               reg = <0>;
-               next-level-cache = <&L2>;
                operating-points = <
                        /* kHz    uV */
                        792000  1100000
@@ -71,6 +42,7 @@ cpus {
                compatible = "arm,cortex-a15";
                reg = <1>;
                next-level-cache = <&L2>;
+               clock-master = <&cpu0>;
        };
 };

@@ -83,7 +55,6 @@ cpus {
                compatible = "arm,cortex-a15";
                reg = <0>;
                next-level-cache = <&L2>;
-               clock-master;
                operating-points = <
                        /* kHz    uV */
                        792000  1100000
@@ -97,7 +68,6 @@ cpus {
                compatible = "arm,cortex-a15";
                reg = <1>;
                next-level-cache = <&L2>;
-               clock-master;
                operating-points = <
                        /* kHz    uV */
                        792000  1100000
@@ -119,7 +89,6 @@ cpus {
                compatible = "arm,cortex-a15";
                reg = <0>;
                next-level-cache = <&L2>;
-               clock-master;
                operating-points = <
                        /* kHz    uV */
                        792000  1100000
@@ -133,14 +102,13 @@ cpus {
                compatible = "arm,cortex-a15";
                reg = <1>;
                next-level-cache = <&L2>;
-               clock-ganged = <&cpu0>;
+               clock-master = <&cpu0>;
        };

        cpu2: cpu at 100 {
                compatible = "arm,cortex-a7";
                reg = <100>;
                next-level-cache = <&L2>;
-               clock-master;
                operating-points = <
                        /* kHz    uV */
                        792000  950000
@@ -154,6 +122,6 @@ cpus {
                compatible = "arm,cortex-a7";
                reg = <101>;
                next-level-cache = <&L2>;
-               clock-ganged = <&cpu2>;
+               clock-master = <&cpu2>;
        };
 };

Re: [RFC] cpufreq: Add bindings for CPU clock sharing topology

[Olof Johansson]

On Thu, Jul 17, 2014 at 11:40 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 18 July 2014 11:47, Olof Johansson <olof@lixom.net> wrote:
>> Why complicate it by using two properties?
>>
>> If there is no property, then the CPUs are assumed to be controlled
>> independently.
>>
>> if there is a clock-master = <phandle> property, then that points at
>> the cpu that is the main one controlling clock for the group.
>>
>> There's really no need to label the master -- it will be the only one
>> with incoming links but nothing outgoing. And a master without slaves
>> is an independently controlled cpu so you should be fine in that
>> aspect too.
>
> I thought so earlier, but then I remembered something I read long back.
> Don't remember which thread now, but I *might* be wrong..
>
> "Bindings are like APIs and new bindings shouldn't break existing stuff.."
>
> And:
>
>> If there is no property, then the CPUs are assumed to be controlled
>> independently.
>
> seems to break the existing API.

What is the current API that is being broken, in your opinion?

> But if that isn't the case, the bindings are very simple & clear to handle.
> Diff for new bindings:

It's somewhat confusing to see a diff to the patch instead of a new
version. It seems to remove the cpu 0 entry now?


-Olof

[RFC] cpufreq: Add bindings for CPU clock sharing topology

[Olof Johansson]

On Thu, Jul 17, 2014 at 11:40 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 18 July 2014 11:47, Olof Johansson <olof@lixom.net> wrote:
>> Why complicate it by using two properties?
>>
>> If there is no property, then the CPUs are assumed to be controlled
>> independently.
>>
>> if there is a clock-master = <phandle> property, then that points at
>> the cpu that is the main one controlling clock for the group.
>>
>> There's really no need to label the master -- it will be the only one
>> with incoming links but nothing outgoing. And a master without slaves
>> is an independently controlled cpu so you should be fine in that
>> aspect too.
>
> I thought so earlier, but then I remembered something I read long back.
> Don't remember which thread now, but I *might* be wrong..
>
> "Bindings are like APIs and new bindings shouldn't break existing stuff.."
>
> And:
>
>> If there is no property, then the CPUs are assumed to be controlled
>> independently.
>
> seems to break the existing API.

What is the current API that is being broken, in your opinion?

> But if that isn't the case, the bindings are very simple & clear to handle.
> Diff for new bindings:

It's somewhat confusing to see a diff to the patch instead of a new
version. It seems to remove the cpu 0 entry now?


-Olof

Re: [RFC] cpufreq: Add bindings for CPU clock sharing topology

[Viresh Kumar]

On 19 July 2014 03:22, Olof Johansson <olof@lixom.net> wrote:
> What is the current API that is being broken, in your opinion?

So, currently the nodes doesn't have any such property. And drivers
consider all of them as sharing clocks, for eg: cpufreq-cpu0.

Now, if we use those older DT's after the new changes, drivers would
consider CPUs as having separate clocks. And that would be opposite
of what currently happens.

Not sure if this counts as broken.

>> But if that isn't the case, the bindings are very simple & clear to handle.
>> Diff for new bindings:
>
> It's somewhat confusing to see a diff to the patch instead of a new
> version. It seems to remove the cpu 0 entry now?

Not really, I removed an unwanted example. This is how it looks:



* Generic CPUFreq clock bindings

Clock lines may or may not be shared among different CPUs on a platform.

Possible configurations:
1.) All CPUs share a single clock line
2.) All CPUs have independent clock lines
3.) CPUs within a group/cluster share clock line but each group/cluster have a
    separate line for itself

Optional Properties:
- clock-master: Contains phandle of the master cpu controlling clocks.

  Ideally there is nothing like a "master" CPU as any CPU can play with DVFS
  settings. But we have to choose one cpu out of a group, so that others can
  point to it.

  If there is no "clock-master" property for a cpu node, it is considered as
  master. It may or may not have other slave CPUs pointing towards it.

Examples:
1.) All CPUs share a single clock line

cpus {
        #address-cells = <1>;
        #size-cells = <0>;

        cpu0: cpu@0 {
                compatible = "arm,cortex-a15";
                reg = <0>;
                next-level-cache = <&L2>;
                operating-points = <
                        /* kHz    uV */
                        792000  1100000
                        396000  950000
                        198000  850000
                >;
                clock-latency = <61036>; /* two CLK32 periods */
        };

        cpu1: cpu@1 {
                compatible = "arm,cortex-a15";
                reg = <1>;
                next-level-cache = <&L2>;
                clock-master = <&cpu0>;
        };
};

2.) All CPUs have independent clock lines
cpus {
        #address-cells = <1>;
        #size-cells = <0>;

        cpu0: cpu@0 {
                compatible = "arm,cortex-a15";
                reg = <0>;
                next-level-cache = <&L2>;
                operating-points = <
                        /* kHz    uV */
                        792000  1100000
                        396000  950000
                        198000  850000
                >;
                clock-latency = <61036>; /* two CLK32 periods */
        };

        cpu1: cpu@1 {
                compatible = "arm,cortex-a15";
                reg = <1>;
                next-level-cache = <&L2>;
                operating-points = <
                        /* kHz    uV */
                        792000  1100000
                        396000  950000
                        198000  850000
                >;
                clock-latency = <61036>; /* two CLK32 periods */
        };
};

3.) CPUs within a group/cluster share single clock line but each group/cluster
have a separate line for itself

cpus {
        #address-cells = <1>;
        #size-cells = <0>;

        cpu0: cpu@0 {
                compatible = "arm,cortex-a15";
                reg = <0>;
                next-level-cache = <&L2>;
                operating-points = <
                        /* kHz    uV */
                        792000  1100000
                        396000  950000
                        198000  850000
                >;
                clock-latency = <61036>; /* two CLK32 periods */
        };

        cpu1: cpu@1 {
                compatible = "arm,cortex-a15";
                reg = <1>;
                next-level-cache = <&L2>;
                clock-master = <&cpu0>;
        };

        cpu2: cpu@100 {
                compatible = "arm,cortex-a7";
                reg = <100>;
                next-level-cache = <&L2>;
                operating-points = <
                        /* kHz    uV */
                        792000  950000
                        396000  750000
                        198000  450000
                >;
                clock-latency = <61036>; /* two CLK32 periods */
        };

        cpu3: cpu@101 {
                compatible = "arm,cortex-a7";
                reg = <101>;
                next-level-cache = <&L2>;
                clock-master = <&cpu2>;
        };
};

[RFC] cpufreq: Add bindings for CPU clock sharing topology

[Viresh Kumar]

On 19 July 2014 03:22, Olof Johansson <olof@lixom.net> wrote:
> What is the current API that is being broken, in your opinion?

So, currently the nodes doesn't have any such property. And drivers
consider all of them as sharing clocks, for eg: cpufreq-cpu0.

Now, if we use those older DT's after the new changes, drivers would
consider CPUs as having separate clocks. And that would be opposite
of what currently happens.

Not sure if this counts as broken.

>> But if that isn't the case, the bindings are very simple & clear to handle.
>> Diff for new bindings:
>
> It's somewhat confusing to see a diff to the patch instead of a new
> version. It seems to remove the cpu 0 entry now?

Not really, I removed an unwanted example. This is how it looks:



* Generic CPUFreq clock bindings

Clock lines may or may not be shared among different CPUs on a platform.

Possible configurations:
1.) All CPUs share a single clock line
2.) All CPUs have independent clock lines
3.) CPUs within a group/cluster share clock line but each group/cluster have a
    separate line for itself

Optional Properties:
- clock-master: Contains phandle of the master cpu controlling clocks.

  Ideally there is nothing like a "master" CPU as any CPU can play with DVFS
  settings. But we have to choose one cpu out of a group, so that others can
  point to it.

  If there is no "clock-master" property for a cpu node, it is considered as
  master. It may or may not have other slave CPUs pointing towards it.

Examples:
1.) All CPUs share a single clock line

cpus {
        #address-cells = <1>;
        #size-cells = <0>;

        cpu0: cpu at 0 {
                compatible = "arm,cortex-a15";
                reg = <0>;
                next-level-cache = <&L2>;
                operating-points = <
                        /* kHz    uV */
                        792000  1100000
                        396000  950000
                        198000  850000
                >;
                clock-latency = <61036>; /* two CLK32 periods */
        };

        cpu1: cpu at 1 {
                compatible = "arm,cortex-a15";
                reg = <1>;
                next-level-cache = <&L2>;
                clock-master = <&cpu0>;
        };
};

2.) All CPUs have independent clock lines
cpus {
        #address-cells = <1>;
        #size-cells = <0>;

        cpu0: cpu at 0 {
                compatible = "arm,cortex-a15";
                reg = <0>;
                next-level-cache = <&L2>;
                operating-points = <
                        /* kHz    uV */
                        792000  1100000
                        396000  950000
                        198000  850000
                >;
                clock-latency = <61036>; /* two CLK32 periods */
        };

        cpu1: cpu at 1 {
                compatible = "arm,cortex-a15";
                reg = <1>;
                next-level-cache = <&L2>;
                operating-points = <
                        /* kHz    uV */
                        792000  1100000
                        396000  950000
                        198000  850000
                >;
                clock-latency = <61036>; /* two CLK32 periods */
        };
};

3.) CPUs within a group/cluster share single clock line but each group/cluster
have a separate line for itself

cpus {
        #address-cells = <1>;
        #size-cells = <0>;

        cpu0: cpu at 0 {
                compatible = "arm,cortex-a15";
                reg = <0>;
                next-level-cache = <&L2>;
                operating-points = <
                        /* kHz    uV */
                        792000  1100000
                        396000  950000
                        198000  850000
                >;
                clock-latency = <61036>; /* two CLK32 periods */
        };

        cpu1: cpu at 1 {
                compatible = "arm,cortex-a15";
                reg = <1>;
                next-level-cache = <&L2>;
                clock-master = <&cpu0>;
        };

        cpu2: cpu at 100 {
                compatible = "arm,cortex-a7";
                reg = <100>;
                next-level-cache = <&L2>;
                operating-points = <
                        /* kHz    uV */
                        792000  950000
                        396000  750000
                        198000  450000
                >;
                clock-latency = <61036>; /* two CLK32 periods */
        };

        cpu3: cpu at 101 {
                compatible = "arm,cortex-a7";
                reg = <101>;
                next-level-cache = <&L2>;
                clock-master = <&cpu2>;
        };
};

Re: [RFC] cpufreq: Add bindings for CPU clock sharing topology

[Santosh Shilimkar]

Viresh,

On Saturday 19 July 2014 10:46 AM, Viresh Kumar wrote:
> On 19 July 2014 03:22, Olof Johansson <olof@lixom.net> wrote:
>> What is the current API that is being broken, in your opinion?
> 
> So, currently the nodes doesn't have any such property. And drivers
> consider all of them as sharing clocks, for eg: cpufreq-cpu0.
> 
> Now, if we use those older DT's after the new changes, drivers would
> consider CPUs as having separate clocks. And that would be opposite
> of what currently happens.
> 
> Not sure if this counts as broken.
> 
>>> But if that isn't the case, the bindings are very simple & clear to handle.
>>> Diff for new bindings:
>>
>> It's somewhat confusing to see a diff to the patch instead of a new
>> version. It seems to remove the cpu 0 entry now?
> 
> Not really, I removed an unwanted example. This is how it looks:
> 
> 
> 
> * Generic CPUFreq clock bindings
> 
> Clock lines may or may not be shared among different CPUs on a platform.
> 
> Possible configurations:
> 1.) All CPUs share a single clock line
> 2.) All CPUs have independent clock lines
> 3.) CPUs within a group/cluster share clock line but each group/cluster have a
>     separate line for itself
> 
> Optional Properties:
> - clock-master: Contains phandle of the master cpu controlling clocks.
> 
>   Ideally there is nothing like a "master" CPU as any CPU can play with DVFS
>   settings. But we have to choose one cpu out of a group, so that others can
>   point to it.
> 
>   If there is no "clock-master" property for a cpu node, it is considered as
>   master. It may or may not have other slave CPUs pointing towards it.
> 

Sorry for jumping late, but one of the point I was raising as part of your
other series was to extend the CPU topology bindings to cover the voltage
domain information which is probably what is really needed to let the
CPUfreq extract the information. Not sure if it was already discussed.

After all the CPU clocks, cluster, clock-gating, power domains are pretty much
related. So instead of having new binding for CPUFreq, I was wondering whether
we can extend the CPU topology binding information to include missing information.
Scheduler work anyway needs that information.

Ref: Documentation/devicetree/bindings/arm/topology.txt

Does that make sense ?

> Examples:
> 1.) All CPUs share a single clock line
> 
> cpus {
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         cpu0: cpu@0 {
>                 compatible = "arm,cortex-a15";
>                 reg = <0>;
>                 next-level-cache = <&L2>;
>                 operating-points = <
>                         /* kHz    uV */
>                         792000  1100000
>                         396000  950000
>                         198000  850000
>                 >;
>                 clock-latency = <61036>; /* two CLK32 periods */
>         };
> 
>         cpu1: cpu@1 {
>                 compatible = "arm,cortex-a15";
>                 reg = <1>;
>                 next-level-cache = <&L2>;
>                 clock-master = <&cpu0>;
>         };
> };
> 
> 2.) All CPUs have independent clock lines
> cpus {
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         cpu0: cpu@0 {
>                 compatible = "arm,cortex-a15";
>                 reg = <0>;
>                 next-level-cache = <&L2>;
>                 operating-points = <
>                         /* kHz    uV */
>                         792000  1100000
>                         396000  950000
>                         198000  850000
>                 >;
>                 clock-latency = <61036>; /* two CLK32 periods */
>         };
> 
>         cpu1: cpu@1 {
>                 compatible = "arm,cortex-a15";
>                 reg = <1>;
>                 next-level-cache = <&L2>;
>                 operating-points = <
>                         /* kHz    uV */
>                         792000  1100000
>                         396000  950000
>                         198000  850000
>                 >;
>                 clock-latency = <61036>; /* two CLK32 periods */
>         };
> };
> 
> 3.) CPUs within a group/cluster share single clock line but each group/cluster
> have a separate line for itself
> 
> cpus {
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         cpu0: cpu@0 {
>                 compatible = "arm,cortex-a15";
>                 reg = <0>;
>                 next-level-cache = <&L2>;
>                 operating-points = <
>                         /* kHz    uV */
>                         792000  1100000
>                         396000  950000
>                         198000  850000
>                 >;
>                 clock-latency = <61036>; /* two CLK32 periods */
>         };
> 
>         cpu1: cpu@1 {
>                 compatible = "arm,cortex-a15";
>                 reg = <1>;
>                 next-level-cache = <&L2>;
>                 clock-master = <&cpu0>;
>         };
> 
>         cpu2: cpu@100 {
>                 compatible = "arm,cortex-a7";
>                 reg = <100>;
>                 next-level-cache = <&L2>;
>                 operating-points = <
>                         /* kHz    uV */
>                         792000  950000
>                         396000  750000
>                         198000  450000
>                 >;
>                 clock-latency = <61036>; /* two CLK32 periods */
>         };
> 
>         cpu3: cpu@101 {
>                 compatible = "arm,cortex-a7";
>                 reg = <101>;
>                 next-level-cache = <&L2>;
>                 clock-master = <&cpu2>;
>         };
> };
> 

[RFC] cpufreq: Add bindings for CPU clock sharing topology

[Santosh Shilimkar]

Viresh,

On Saturday 19 July 2014 10:46 AM, Viresh Kumar wrote:
> On 19 July 2014 03:22, Olof Johansson <olof@lixom.net> wrote:
>> What is the current API that is being broken, in your opinion?
> 
> So, currently the nodes doesn't have any such property. And drivers
> consider all of them as sharing clocks, for eg: cpufreq-cpu0.
> 
> Now, if we use those older DT's after the new changes, drivers would
> consider CPUs as having separate clocks. And that would be opposite
> of what currently happens.
> 
> Not sure if this counts as broken.
> 
>>> But if that isn't the case, the bindings are very simple & clear to handle.
>>> Diff for new bindings:
>>
>> It's somewhat confusing to see a diff to the patch instead of a new
>> version. It seems to remove the cpu 0 entry now?
> 
> Not really, I removed an unwanted example. This is how it looks:
> 
> 
> 
> * Generic CPUFreq clock bindings
> 
> Clock lines may or may not be shared among different CPUs on a platform.
> 
> Possible configurations:
> 1.) All CPUs share a single clock line
> 2.) All CPUs have independent clock lines
> 3.) CPUs within a group/cluster share clock line but each group/cluster have a
>     separate line for itself
> 
> Optional Properties:
> - clock-master: Contains phandle of the master cpu controlling clocks.
> 
>   Ideally there is nothing like a "master" CPU as any CPU can play with DVFS
>   settings. But we have to choose one cpu out of a group, so that others can
>   point to it.
> 
>   If there is no "clock-master" property for a cpu node, it is considered as
>   master. It may or may not have other slave CPUs pointing towards it.
> 

Sorry for jumping late, but one of the point I was raising as part of your
other series was to extend the CPU topology bindings to cover the voltage
domain information which is probably what is really needed to let the
CPUfreq extract the information. Not sure if it was already discussed.

After all the CPU clocks, cluster, clock-gating, power domains are pretty much
related. So instead of having new binding for CPUFreq, I was wondering whether
we can extend the CPU topology binding information to include missing information.
Scheduler work anyway needs that information.

Ref: Documentation/devicetree/bindings/arm/topology.txt

Does that make sense ?

> Examples:
> 1.) All CPUs share a single clock line
> 
> cpus {
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         cpu0: cpu at 0 {
>                 compatible = "arm,cortex-a15";
>                 reg = <0>;
>                 next-level-cache = <&L2>;
>                 operating-points = <
>                         /* kHz    uV */
>                         792000  1100000
>                         396000  950000
>                         198000  850000
>                 >;
>                 clock-latency = <61036>; /* two CLK32 periods */
>         };
> 
>         cpu1: cpu at 1 {
>                 compatible = "arm,cortex-a15";
>                 reg = <1>;
>                 next-level-cache = <&L2>;
>                 clock-master = <&cpu0>;
>         };
> };
> 
> 2.) All CPUs have independent clock lines
> cpus {
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         cpu0: cpu at 0 {
>                 compatible = "arm,cortex-a15";
>                 reg = <0>;
>                 next-level-cache = <&L2>;
>                 operating-points = <
>                         /* kHz    uV */
>                         792000  1100000
>                         396000  950000
>                         198000  850000
>                 >;
>                 clock-latency = <61036>; /* two CLK32 periods */
>         };
> 
>         cpu1: cpu at 1 {
>                 compatible = "arm,cortex-a15";
>                 reg = <1>;
>                 next-level-cache = <&L2>;
>                 operating-points = <
>                         /* kHz    uV */
>                         792000  1100000
>                         396000  950000
>                         198000  850000
>                 >;
>                 clock-latency = <61036>; /* two CLK32 periods */
>         };
> };
> 
> 3.) CPUs within a group/cluster share single clock line but each group/cluster
> have a separate line for itself
> 
> cpus {
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         cpu0: cpu at 0 {
>                 compatible = "arm,cortex-a15";
>                 reg = <0>;
>                 next-level-cache = <&L2>;
>                 operating-points = <
>                         /* kHz    uV */
>                         792000  1100000
>                         396000  950000
>                         198000  850000
>                 >;
>                 clock-latency = <61036>; /* two CLK32 periods */
>         };
> 
>         cpu1: cpu at 1 {
>                 compatible = "arm,cortex-a15";
>                 reg = <1>;
>                 next-level-cache = <&L2>;
>                 clock-master = <&cpu0>;
>         };
> 
>         cpu2: cpu at 100 {
>                 compatible = "arm,cortex-a7";
>                 reg = <100>;
>                 next-level-cache = <&L2>;
>                 operating-points = <
>                         /* kHz    uV */
>                         792000  950000
>                         396000  750000
>                         198000  450000
>                 >;
>                 clock-latency = <61036>; /* two CLK32 periods */
>         };
> 
>         cpu3: cpu at 101 {
>                 compatible = "arm,cortex-a7";
>                 reg = <101>;
>                 next-level-cache = <&L2>;
>                 clock-master = <&cpu2>;
>         };
> };
> 

Re: [RFC] cpufreq: Add bindings for CPU clock sharing topology

[Viresh Kumar]

On 19 July 2014 20:54, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> Sorry for jumping late

No, you aren't late. Its just 2 days old thread :)

> but one of the point I was raising as part of your
> other series was to extend the CPU topology bindings to cover the voltage
> domain information which is probably what is really needed to let the
> CPUfreq extract the information. Not sure if it was already discussed.

Not it wasn't.

> After all the CPU clocks, cluster, clock-gating, power domains are pretty much
> related. So instead of having new binding for CPUFreq, I was wondering whether
> we can extend the CPU topology binding information to include missing information.
> Scheduler work anyway needs that information.
>
> Ref: Documentation/devicetree/bindings/arm/topology.txt
>
> Does that make sense ?

Yeah it does, but I am not sure what exactly the bindings should look then.
So, the most basic step could be moving the new bindings to topology.txt
and name clock-master to dvfs-master.

What else?

If its going to be much controversial then we *can* go for just dvfs bindings
for now and then update them later.

Doesn't make sense? :)

[RFC] cpufreq: Add bindings for CPU clock sharing topology

[Viresh Kumar]

On 19 July 2014 20:54, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> Sorry for jumping late

No, you aren't late. Its just 2 days old thread :)

> but one of the point I was raising as part of your
> other series was to extend the CPU topology bindings to cover the voltage
> domain information which is probably what is really needed to let the
> CPUfreq extract the information. Not sure if it was already discussed.

Not it wasn't.

> After all the CPU clocks, cluster, clock-gating, power domains are pretty much
> related. So instead of having new binding for CPUFreq, I was wondering whether
> we can extend the CPU topology binding information to include missing information.
> Scheduler work anyway needs that information.
>
> Ref: Documentation/devicetree/bindings/arm/topology.txt
>
> Does that make sense ?

Yeah it does, but I am not sure what exactly the bindings should look then.
So, the most basic step could be moving the new bindings to topology.txt
and name clock-master to dvfs-master.

What else?

If its going to be much controversial then we *can* go for just dvfs bindings
for now and then update them later.

Doesn't make sense? :)

Re: [RFC] cpufreq: Add bindings for CPU clock sharing topology

[Santosh Shilimkar]

On Sunday 20 July 2014 08:07 AM, Viresh Kumar wrote:
> On 19 July 2014 20:54, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>> Sorry for jumping late
> 
> No, you aren't late. Its just 2 days old thread :)
> 
>> but one of the point I was raising as part of your
>> other series was to extend the CPU topology bindings to cover the voltage
>> domain information which is probably what is really needed to let the
>> CPUfreq extract the information. Not sure if it was already discussed.
> 
> Not it wasn't.
> 
>> After all the CPU clocks, cluster, clock-gating, power domains are pretty much
>> related. So instead of having new binding for CPUFreq, I was wondering whether
>> we can extend the CPU topology binding information to include missing information.
>> Scheduler work anyway needs that information.
>>
>> Ref: Documentation/devicetree/bindings/arm/topology.txt
>>
>> Does that make sense ?
> 
> Yeah it does, but I am not sure what exactly the bindings should look then.
> So, the most basic step could be moving the new bindings to topology.txt
> and name clock-master to dvfs-master.
> 
> What else?
> 
> If its going to be much controversial then we *can* go for just dvfs bindings
> for now and then update them later.
> 
Would be good to get others opinion. As you said if it is controversial then
it will stall the development.

Regards,
Santosh

[RFC] cpufreq: Add bindings for CPU clock sharing topology

[Santosh Shilimkar]

On Sunday 20 July 2014 08:07 AM, Viresh Kumar wrote:
> On 19 July 2014 20:54, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>> Sorry for jumping late
> 
> No, you aren't late. Its just 2 days old thread :)
> 
>> but one of the point I was raising as part of your
>> other series was to extend the CPU topology bindings to cover the voltage
>> domain information which is probably what is really needed to let the
>> CPUfreq extract the information. Not sure if it was already discussed.
> 
> Not it wasn't.
> 
>> After all the CPU clocks, cluster, clock-gating, power domains are pretty much
>> related. So instead of having new binding for CPUFreq, I was wondering whether
>> we can extend the CPU topology binding information to include missing information.
>> Scheduler work anyway needs that information.
>>
>> Ref: Documentation/devicetree/bindings/arm/topology.txt
>>
>> Does that make sense ?
> 
> Yeah it does, but I am not sure what exactly the bindings should look then.
> So, the most basic step could be moving the new bindings to topology.txt
> and name clock-master to dvfs-master.
> 
> What else?
> 
> If its going to be much controversial then we *can* go for just dvfs bindings
> for now and then update them later.
> 
Would be good to get others opinion. As you said if it is controversial then
it will stall the development.

Regards,
Santosh

Re: [RFC] cpufreq: Add bindings for CPU clock sharing topology

[Rob Herring]

On Sat, Jul 19, 2014 at 10:24 AM, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
> Viresh,
>
> On Saturday 19 July 2014 10:46 AM, Viresh Kumar wrote:
>> On 19 July 2014 03:22, Olof Johansson <olof@lixom.net> wrote:
>>> What is the current API that is being broken, in your opinion?
>>
>> So, currently the nodes doesn't have any such property. And drivers
>> consider all of them as sharing clocks, for eg: cpufreq-cpu0.
>>
>> Now, if we use those older DT's after the new changes, drivers would
>> consider CPUs as having separate clocks. And that would be opposite
>> of what currently happens.
>>
>> Not sure if this counts as broken.
>>
>>>> But if that isn't the case, the bindings are very simple & clear to handle.
>>>> Diff for new bindings:
>>>
>>> It's somewhat confusing to see a diff to the patch instead of a new
>>> version. It seems to remove the cpu 0 entry now?
>>
>> Not really, I removed an unwanted example. This is how it looks:
>>
>>
>>
>> * Generic CPUFreq clock bindings
>>
>> Clock lines may or may not be shared among different CPUs on a platform.
>>
>> Possible configurations:
>> 1.) All CPUs share a single clock line
>> 2.) All CPUs have independent clock lines
>> 3.) CPUs within a group/cluster share clock line but each group/cluster have a
>>     separate line for itself
>>
>> Optional Properties:
>> - clock-master: Contains phandle of the master cpu controlling clocks.
>>
>>   Ideally there is nothing like a "master" CPU as any CPU can play with DVFS
>>   settings. But we have to choose one cpu out of a group, so that others can
>>   point to it.
>>
>>   If there is no "clock-master" property for a cpu node, it is considered as
>>   master. It may or may not have other slave CPUs pointing towards it.
>>
>
> Sorry for jumping late, but one of the point I was raising as part of your
> other series was to extend the CPU topology bindings to cover the voltage
> domain information which is probably what is really needed to let the
> CPUfreq extract the information. Not sure if it was already discussed.
>
> After all the CPU clocks, cluster, clock-gating, power domains are pretty much
> related. So instead of having new binding for CPUFreq, I was wondering whether
> we can extend the CPU topology binding information to include missing information.
> Scheduler work anyway needs that information.
>
> Ref: Documentation/devicetree/bindings/arm/topology.txt
>
> Does that make sense ?

To me, but every time I suggest adding things to the topology the ARM
folks object... I really think we should have built the topology into
the /cpus hierarchy. Then we could add properties at the correct place
in the hierarchy where they are common.

I don't really like the proposal here. It just doesn't look like a
clean description of the h/w.

Ignoring compatibility, I would like to see something like
operating-points and/or the clock properties be moved up to /cpus if
they are shared and be per cpu node when they are not. This of course
does not work if you have independent OPPs for each cluster with a
shared clock within cluster.

The operating-points binding has obvious shortcomings and this is
another example. Someone needs to step up with a new binding that
addresses this and all other issues (e.g. turbo modes, extra per OPP
data, etc.). I don't really want to see halfway fixes to the binding
that ignore the other issues (unless there is a really simple
solution).

Rob

[RFC] cpufreq: Add bindings for CPU clock sharing topology

[Rob Herring]

On Sat, Jul 19, 2014 at 10:24 AM, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
> Viresh,
>
> On Saturday 19 July 2014 10:46 AM, Viresh Kumar wrote:
>> On 19 July 2014 03:22, Olof Johansson <olof@lixom.net> wrote:
>>> What is the current API that is being broken, in your opinion?
>>
>> So, currently the nodes doesn't have any such property. And drivers
>> consider all of them as sharing clocks, for eg: cpufreq-cpu0.
>>
>> Now, if we use those older DT's after the new changes, drivers would
>> consider CPUs as having separate clocks. And that would be opposite
>> of what currently happens.
>>
>> Not sure if this counts as broken.
>>
>>>> But if that isn't the case, the bindings are very simple & clear to handle.
>>>> Diff for new bindings:
>>>
>>> It's somewhat confusing to see a diff to the patch instead of a new
>>> version. It seems to remove the cpu 0 entry now?
>>
>> Not really, I removed an unwanted example. This is how it looks:
>>
>>
>>
>> * Generic CPUFreq clock bindings
>>
>> Clock lines may or may not be shared among different CPUs on a platform.
>>
>> Possible configurations:
>> 1.) All CPUs share a single clock line
>> 2.) All CPUs have independent clock lines
>> 3.) CPUs within a group/cluster share clock line but each group/cluster have a
>>     separate line for itself
>>
>> Optional Properties:
>> - clock-master: Contains phandle of the master cpu controlling clocks.
>>
>>   Ideally there is nothing like a "master" CPU as any CPU can play with DVFS
>>   settings. But we have to choose one cpu out of a group, so that others can
>>   point to it.
>>
>>   If there is no "clock-master" property for a cpu node, it is considered as
>>   master. It may or may not have other slave CPUs pointing towards it.
>>
>
> Sorry for jumping late, but one of the point I was raising as part of your
> other series was to extend the CPU topology bindings to cover the voltage
> domain information which is probably what is really needed to let the
> CPUfreq extract the information. Not sure if it was already discussed.
>
> After all the CPU clocks, cluster, clock-gating, power domains are pretty much
> related. So instead of having new binding for CPUFreq, I was wondering whether
> we can extend the CPU topology binding information to include missing information.
> Scheduler work anyway needs that information.
>
> Ref: Documentation/devicetree/bindings/arm/topology.txt
>
> Does that make sense ?

To me, but every time I suggest adding things to the topology the ARM
folks object... I really think we should have built the topology into
the /cpus hierarchy. Then we could add properties at the correct place
in the hierarchy where they are common.

I don't really like the proposal here. It just doesn't look like a
clean description of the h/w.

Ignoring compatibility, I would like to see something like
operating-points and/or the clock properties be moved up to /cpus if
they are shared and be per cpu node when they are not. This of course
does not work if you have independent OPPs for each cluster with a
shared clock within cluster.

The operating-points binding has obvious shortcomings and this is
another example. Someone needs to step up with a new binding that
addresses this and all other issues (e.g. turbo modes, extra per OPP
data, etc.). I don't really want to see halfway fixes to the binding
that ignore the other issues (unless there is a really simple
solution).

Rob

Re: [RFC] cpufreq: Add bindings for CPU clock sharing topology

[Viresh Kumar]

On 21 July 2014 22:30, Rob Herring <rob.herring@linaro.org> wrote:

> To me, but every time I suggest adding things to the topology the ARM
> folks object... I really think we should have built the topology into
> the /cpus hierarchy. Then we could add properties at the correct place
> in the hierarchy where they are common.
>
> I don't really like the proposal here. It just doesn't look like a
> clean description of the h/w.

I knew it wouldn't be easy to get these bindings correctly in the first
attempt atleast, but I still went for it so that this thread can progress:

https://lkml.org/lkml/2014/7/18/3

There are changes waiting to get some kind of intermediate solution
in, so that other platforms can reuse cpufreq-cpu0 driver. Also, so that
cpufreq-cpu0 can be converted to cpufreq-generic, so that it can support
almost any platform.

Can you please reply to that thread to suggest some intermediate
solution that we can get fixed in 3.17? Mvebu and Krait are waiting for
these patches to get in..

The solution can be simple enough as right now we have only two kinds
of platforms:
- All CPUs share clock
- All CPUs have separate clocks

The most simple solution of that could have been a Kconfig entry, but that
would be a problem for multi-arch builds if these platforms do want to get
build with multi-arch config. Otherwise we have to get some binding in
place for 3.17..

Please see if you can get that thread going :)

> Ignoring compatibility, I would like to see something like
> operating-points and/or the clock properties be moved up to /cpus if
> they are shared and be per cpu node when they are not. This of course
> does not work if you have independent OPPs for each cluster with a
> shared clock within cluster.

Yeah, that had always been the issue. People probably tried to get a
cluster {} block as well to simplify that but there were some objections
to it as well, though not sure what happened to that stuff..

[RFC] cpufreq: Add bindings for CPU clock sharing topology

[Viresh Kumar]

On 21 July 2014 22:30, Rob Herring <rob.herring@linaro.org> wrote:

> To me, but every time I suggest adding things to the topology the ARM
> folks object... I really think we should have built the topology into
> the /cpus hierarchy. Then we could add properties at the correct place
> in the hierarchy where they are common.
>
> I don't really like the proposal here. It just doesn't look like a
> clean description of the h/w.

I knew it wouldn't be easy to get these bindings correctly in the first
attempt atleast, but I still went for it so that this thread can progress:

https://lkml.org/lkml/2014/7/18/3

There are changes waiting to get some kind of intermediate solution
in, so that other platforms can reuse cpufreq-cpu0 driver. Also, so that
cpufreq-cpu0 can be converted to cpufreq-generic, so that it can support
almost any platform.

Can you please reply to that thread to suggest some intermediate
solution that we can get fixed in 3.17? Mvebu and Krait are waiting for
these patches to get in..

The solution can be simple enough as right now we have only two kinds
of platforms:
- All CPUs share clock
- All CPUs have separate clocks

The most simple solution of that could have been a Kconfig entry, but that
would be a problem for multi-arch builds if these platforms do want to get
build with multi-arch config. Otherwise we have to get some binding in
place for 3.17..

Please see if you can get that thread going :)

> Ignoring compatibility, I would like to see something like
> operating-points and/or the clock properties be moved up to /cpus if
> they are shared and be per cpu node when they are not. This of course
> does not work if you have independent OPPs for each cluster with a
> shared clock within cluster.

Yeah, that had always been the issue. People probably tried to get a
cluster {} block as well to simplify that but there were some objections
to it as well, though not sure what happened to that stuff..

Re: [RFC] cpufreq: Add bindings for CPU clock sharing topology

[Mike Turquette]

Quoting Viresh Kumar (2014-07-20 05:07:32)
> On 19 July 2014 20:54, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> > Sorry for jumping late
> 
> No, you aren't late. Its just 2 days old thread :)
> 
> > but one of the point I was raising as part of your
> > other series was to extend the CPU topology bindings to cover the voltage
> > domain information which is probably what is really needed to let the
> > CPUfreq extract the information. Not sure if it was already discussed.
> 
> Not it wasn't.
> 
> > After all the CPU clocks, cluster, clock-gating, power domains are pretty much
> > related. So instead of having new binding for CPUFreq, I was wondering whether
> > we can extend the CPU topology binding information to include missing information.
> > Scheduler work anyway needs that information.
> >
> > Ref: Documentation/devicetree/bindings/arm/topology.txt
> >
> > Does that make sense ?
> 
> Yeah it does, but I am not sure what exactly the bindings should look then.
> So, the most basic step could be moving the new bindings to topology.txt
> and name clock-master to dvfs-master.
> 
> What else?

If we're going to model the hardware then the binding should not use the
CPU phandles in "clock-master" or "dvfs-master". The correct thing to
model for a given CPU is which clock consumes. It's not accurate to say
that one CPU is the "master", at least not in this context.

A previous approach tried to compare struct clk pointers, which is a bad
idea since those are just cookies and should not be deref'd by drivers.
However a similar approach would be to compare the phandle, right?

Regards,
Mike

> 
> If its going to be much controversial then we *can* go for just dvfs bindings
> for now and then update them later.
> 
> Doesn't make sense? :)

[RFC] cpufreq: Add bindings for CPU clock sharing topology

[Mike Turquette]

Quoting Viresh Kumar (2014-07-20 05:07:32)
> On 19 July 2014 20:54, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> > Sorry for jumping late
> 
> No, you aren't late. Its just 2 days old thread :)
> 
> > but one of the point I was raising as part of your
> > other series was to extend the CPU topology bindings to cover the voltage
> > domain information which is probably what is really needed to let the
> > CPUfreq extract the information. Not sure if it was already discussed.
> 
> Not it wasn't.
> 
> > After all the CPU clocks, cluster, clock-gating, power domains are pretty much
> > related. So instead of having new binding for CPUFreq, I was wondering whether
> > we can extend the CPU topology binding information to include missing information.
> > Scheduler work anyway needs that information.
> >
> > Ref: Documentation/devicetree/bindings/arm/topology.txt
> >
> > Does that make sense ?
> 
> Yeah it does, but I am not sure what exactly the bindings should look then.
> So, the most basic step could be moving the new bindings to topology.txt
> and name clock-master to dvfs-master.
> 
> What else?

If we're going to model the hardware then the binding should not use the
CPU phandles in "clock-master" or "dvfs-master". The correct thing to
model for a given CPU is which clock consumes. It's not accurate to say
that one CPU is the "master", at least not in this context.

A previous approach tried to compare struct clk pointers, which is a bad
idea since those are just cookies and should not be deref'd by drivers.
However a similar approach would be to compare the phandle, right?

Regards,
Mike

> 
> If its going to be much controversial then we *can* go for just dvfs bindings
> for now and then update them later.
> 
> Doesn't make sense? :)

Re: [RFC] cpufreq: Add bindings for CPU clock sharing topology

[Rob Herring]

On Wed, Jul 23, 2014 at 7:33 PM, Mike Turquette
<mike.turquette@linaro.org> wrote:
> Quoting Viresh Kumar (2014-07-20 05:07:32)
>> On 19 July 2014 20:54, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>> > Sorry for jumping late
>>
>> No, you aren't late. Its just 2 days old thread :)
>>
>> > but one of the point I was raising as part of your
>> > other series was to extend the CPU topology bindings to cover the voltage
>> > domain information which is probably what is really needed to let the
>> > CPUfreq extract the information. Not sure if it was already discussed.
>>
>> Not it wasn't.
>>
>> > After all the CPU clocks, cluster, clock-gating, power domains are pretty much
>> > related. So instead of having new binding for CPUFreq, I was wondering whether
>> > we can extend the CPU topology binding information to include missing information.
>> > Scheduler work anyway needs that information.
>> >
>> > Ref: Documentation/devicetree/bindings/arm/topology.txt
>> >
>> > Does that make sense ?
>>
>> Yeah it does, but I am not sure what exactly the bindings should look then.
>> So, the most basic step could be moving the new bindings to topology.txt
>> and name clock-master to dvfs-master.
>>
>> What else?
>
> If we're going to model the hardware then the binding should not use the
> CPU phandles in "clock-master" or "dvfs-master". The correct thing to
> model for a given CPU is which clock consumes. It's not accurate to say
> that one CPU is the "master", at least not in this context.
>
> A previous approach tried to compare struct clk pointers, which is a bad
> idea since those are just cookies and should not be deref'd by drivers.
> However a similar approach would be to compare the phandle, right?

I think there needs to be a way to query whether a rate change for a
clock affects other children. As pointed out previously, the clock to
a core may not be shared, but it's parent that can change rates could
be shared. This could be done with functions
clk_get_parent_rate_change to return the clock in heirarchy which can
change rates, and clk_is_parent_clk which tells if one clock is a
child of another clock. It's been a while since I've looked at the
clock api. It could also be done by experiment. Change the rate for
core 0 and see if core 1's rate is changed and still equal. There's
probably some ordering issue with doing that though.

Rob

[RFC] cpufreq: Add bindings for CPU clock sharing topology

[Rob Herring]

On Wed, Jul 23, 2014 at 7:33 PM, Mike Turquette
<mike.turquette@linaro.org> wrote:
> Quoting Viresh Kumar (2014-07-20 05:07:32)
>> On 19 July 2014 20:54, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>> > Sorry for jumping late
>>
>> No, you aren't late. Its just 2 days old thread :)
>>
>> > but one of the point I was raising as part of your
>> > other series was to extend the CPU topology bindings to cover the voltage
>> > domain information which is probably what is really needed to let the
>> > CPUfreq extract the information. Not sure if it was already discussed.
>>
>> Not it wasn't.
>>
>> > After all the CPU clocks, cluster, clock-gating, power domains are pretty much
>> > related. So instead of having new binding for CPUFreq, I was wondering whether
>> > we can extend the CPU topology binding information to include missing information.
>> > Scheduler work anyway needs that information.
>> >
>> > Ref: Documentation/devicetree/bindings/arm/topology.txt
>> >
>> > Does that make sense ?
>>
>> Yeah it does, but I am not sure what exactly the bindings should look then.
>> So, the most basic step could be moving the new bindings to topology.txt
>> and name clock-master to dvfs-master.
>>
>> What else?
>
> If we're going to model the hardware then the binding should not use the
> CPU phandles in "clock-master" or "dvfs-master". The correct thing to
> model for a given CPU is which clock consumes. It's not accurate to say
> that one CPU is the "master", at least not in this context.
>
> A previous approach tried to compare struct clk pointers, which is a bad
> idea since those are just cookies and should not be deref'd by drivers.
> However a similar approach would be to compare the phandle, right?

I think there needs to be a way to query whether a rate change for a
clock affects other children. As pointed out previously, the clock to
a core may not be shared, but it's parent that can change rates could
be shared. This could be done with functions
clk_get_parent_rate_change to return the clock in heirarchy which can
change rates, and clk_is_parent_clk which tells if one clock is a
child of another clock. It's been a while since I've looked at the
clock api. It could also be done by experiment. Change the rate for
core 0 and see if core 1's rate is changed and still equal. There's
probably some ordering issue with doing that though.

Rob

Re: [RFC] cpufreq: Add bindings for CPU clock sharing topology

[Viresh Kumar]

On 24 July 2014 07:54, Rob Herring <rob.herring@linaro.org> wrote:

>> A previous approach tried to compare struct clk pointers, which is a bad
>> idea since those are just cookies and should not be deref'd by drivers.
>> However a similar approach would be to compare the phandle, right?

Yeah. So what's the right way then?

> I think there needs to be a way to query whether a rate change for a
> clock affects other children. As pointed out previously, the clock to
> a core may not be shared, but it's parent that can change rates could
> be shared. This could be done with functions
> clk_get_parent_rate_change to return the clock in heirarchy which can
> change rates, and clk_is_parent_clk which tells if one clock is a
> child of another clock. It's been a while since I've looked at the
> clock api. It could also be done by experiment. Change the rate for
> core 0 and see if core 1's rate is changed and still equal. There's
> probably some ordering issue with doing that though.

But Mike sort of Nak'd that as well earlier :)

[RFC] cpufreq: Add bindings for CPU clock sharing topology

[Viresh Kumar]

On 24 July 2014 07:54, Rob Herring <rob.herring@linaro.org> wrote:

>> A previous approach tried to compare struct clk pointers, which is a bad
>> idea since those are just cookies and should not be deref'd by drivers.
>> However a similar approach would be to compare the phandle, right?

Yeah. So what's the right way then?

> I think there needs to be a way to query whether a rate change for a
> clock affects other children. As pointed out previously, the clock to
> a core may not be shared, but it's parent that can change rates could
> be shared. This could be done with functions
> clk_get_parent_rate_change to return the clock in heirarchy which can
> change rates, and clk_is_parent_clk which tells if one clock is a
> child of another clock. It's been a while since I've looked at the
> clock api. It could also be done by experiment. Change the rate for
> core 0 and see if core 1's rate is changed and still equal. There's
> probably some ordering issue with doing that though.

But Mike sort of Nak'd that as well earlier :)

Re: [RFC] cpufreq: Add bindings for CPU clock sharing topology

[Mike Turquette]

Quoting Viresh Kumar (2014-07-24 03:39:31)
> On 24 July 2014 07:54, Rob Herring <rob.herring@linaro.org> wrote:
> 
> >> A previous approach tried to compare struct clk pointers, which is a bad
> >> idea since those are just cookies and should not be deref'd by drivers.
> >> However a similar approach would be to compare the phandle, right?
> 
> Yeah. So what's the right way then?
> 
> > I think there needs to be a way to query whether a rate change for a
> > clock affects other children. As pointed out previously, the clock to
> > a core may not be shared, but it's parent that can change rates could
> > be shared. This could be done with functions
> > clk_get_parent_rate_change to return the clock in heirarchy which can
> > change rates, and clk_is_parent_clk which tells if one clock is a
> > child of another clock. It's been a while since I've looked at the
> > clock api. It could also be done by experiment. Change the rate for
> > core 0 and see if core 1's rate is changed and still equal. There's
> > probably some ordering issue with doing that though.
> 
> But Mike sort of Nak'd that as well earlier :)

Sorry for being NAKy. Pushing more query functions into the clk api will
be a bad thing in the long term. First there are races: if a driver
queries the clock framework and then calls another clk.h api function
based on that result, we would need to lock the whole block of code
since the query may be invalid immediately after the result is returned.

Secondly, it is common for hardware to be designed with various use case
configurations already known. From a software point of view this is
"compile-time" info since we already know what OPPs we want to run at,
how we want to multiplex our clock outputs, etc. Often we know exactly
what we want to do from a hardware integration standpoint, and by trying
to have some algorithmic approach in software where we walk the tree and
figure stuff out dynamically is just going to make life hard for us.

I also think this whole push to consolidate every cpufreq machine driver
into cpufreq-cpu0 is complete madness. Machine drivers exist for a
reason. Now with this big push to remove cpufreq drivers we are just
pushing more and more logic into the clock framework (see some recent
patch series introducing "cpu clocks" as clock providers, which
essentially do what the old cpufreq machine drivers used to do).

So my opinion is to figure out how to specify the shared-clock versus
independent-clock parameter it DT. I think the big issue here is the
topology semantics around the cpus node, and I'll stay out of that
stuff. But let's please not introduce a random API for a single merge
window and let's also not over-consolidate machine driver code just for
the sake of having fewer C files.

Regards,
Mike

[RFC] cpufreq: Add bindings for CPU clock sharing topology

[Mike Turquette]

Quoting Viresh Kumar (2014-07-24 03:39:31)
> On 24 July 2014 07:54, Rob Herring <rob.herring@linaro.org> wrote:
> 
> >> A previous approach tried to compare struct clk pointers, which is a bad
> >> idea since those are just cookies and should not be deref'd by drivers.
> >> However a similar approach would be to compare the phandle, right?
> 
> Yeah. So what's the right way then?
> 
> > I think there needs to be a way to query whether a rate change for a
> > clock affects other children. As pointed out previously, the clock to
> > a core may not be shared, but it's parent that can change rates could
> > be shared. This could be done with functions
> > clk_get_parent_rate_change to return the clock in heirarchy which can
> > change rates, and clk_is_parent_clk which tells if one clock is a
> > child of another clock. It's been a while since I've looked at the
> > clock api. It could also be done by experiment. Change the rate for
> > core 0 and see if core 1's rate is changed and still equal. There's
> > probably some ordering issue with doing that though.
> 
> But Mike sort of Nak'd that as well earlier :)

Sorry for being NAKy. Pushing more query functions into the clk api will
be a bad thing in the long term. First there are races: if a driver
queries the clock framework and then calls another clk.h api function
based on that result, we would need to lock the whole block of code
since the query may be invalid immediately after the result is returned.

Secondly, it is common for hardware to be designed with various use case
configurations already known. From a software point of view this is
"compile-time" info since we already know what OPPs we want to run at,
how we want to multiplex our clock outputs, etc. Often we know exactly
what we want to do from a hardware integration standpoint, and by trying
to have some algorithmic approach in software where we walk the tree and
figure stuff out dynamically is just going to make life hard for us.

I also think this whole push to consolidate every cpufreq machine driver
into cpufreq-cpu0 is complete madness. Machine drivers exist for a
reason. Now with this big push to remove cpufreq drivers we are just
pushing more and more logic into the clock framework (see some recent
patch series introducing "cpu clocks" as clock providers, which
essentially do what the old cpufreq machine drivers used to do).

So my opinion is to figure out how to specify the shared-clock versus
independent-clock parameter it DT. I think the big issue here is the
topology semantics around the cpus node, and I'll stay out of that
stuff. But let's please not introduce a random API for a single merge
window and let's also not over-consolidate machine driver code just for
the sake of having fewer C files.

Regards,
Mike

Re: [RFC] cpufreq: Add bindings for CPU clock sharing topology

[Viresh Kumar]

On 26 July 2014 01:32, Mike Turquette <mike.turquette@linaro.org> wrote:
> So my opinion is to figure out how to specify the shared-clock versus
> independent-clock parameter it DT. I think the big issue here is the
> topology semantics around the cpus node, and I'll stay out of that
> stuff. But let's please not introduce a random API for a single merge
> window and let's also not over-consolidate machine driver code just for
> the sake of having fewer C files.

Returning almost after a month to this :(

- I agree that consolidation to cpufreq-cpu0 driver is good but adding a
backend clk driver for that is bad. I would look around supporting callbacks
to cpufreq-cpu0 driver. So that ->target()/->target_index() can be specified
by platform drivers and rest of the code can be used. But that's the next
problem to solve.

- Back to the first issue. How do we sharing information from DT ?

As I read them (and I may be wrong in understanding that), there were
conflicting ideas from many..

Can we please decide how we want to see these bindings? So, that I can
implement them quickly and close this thread...

Its just hanging in the middle as there wasn't a single clean solution yet.

--
viresh

[RFC] cpufreq: Add bindings for CPU clock sharing topology

[Viresh Kumar]

On 26 July 2014 01:32, Mike Turquette <mike.turquette@linaro.org> wrote:
> So my opinion is to figure out how to specify the shared-clock versus
> independent-clock parameter it DT. I think the big issue here is the
> topology semantics around the cpus node, and I'll stay out of that
> stuff. But let's please not introduce a random API for a single merge
> window and let's also not over-consolidate machine driver code just for
> the sake of having fewer C files.

Returning almost after a month to this :(

- I agree that consolidation to cpufreq-cpu0 driver is good but adding a
backend clk driver for that is bad. I would look around supporting callbacks
to cpufreq-cpu0 driver. So that ->target()/->target_index() can be specified
by platform drivers and rest of the code can be used. But that's the next
problem to solve.

- Back to the first issue. How do we sharing information from DT ?

As I read them (and I may be wrong in understanding that), there were
conflicting ideas from many..

Can we please decide how we want to see these bindings? So, that I can
implement them quickly and close this thread...

Its just hanging in the middle as there wasn't a single clean solution yet.

--
viresh