Re: [PATCH 1/8] cpufreq: arm_big_little: add cluster regulator support

2015-06-11 Thread Heiko Stübner
Hi,

Am Dienstag, 21. April 2015, 15:17:51 schrieb Bartlomiej Zolnierkiewicz:
 Add cluster regulator support as a preparation to adding
 generic arm_big_little_dt cpufreq_dt driver support for
 ODROID-XU3 board.  This allows arm_big_little[_dt] driver
 to set not only the frequency but also the voltage (which
 is obtained from operating point's voltage value) for CPU
 clusters.
 
 Cc: Kukjin Kim kgene@samsung.com
 Cc: Doug Anderson diand...@chromium.org
 Cc: Javier Martinez Canillas javier.marti...@collabora.co.uk
 Cc: Andreas Faerber afaer...@suse.de
 Cc: Sachin Kamat sachin.ka...@linaro.org
 Cc: Thomas Abraham thomas...@samsung.com
 Signed-off-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com

I gave this a spin on the rk3368 arm64 soc from Rockchip, mainly to check if
my armclk handling was correct.

Your patch here only supports individual supplies per cluster but my
current board shares the supplies over both cpu clusters, so I've cooked
up a patch to also try to support shared supplies
[0].

Nevertheless,
Tested-by: Heiko Stuebner he...@sntech.de

Do you plan to continue working on this?


Thanks
Heiko


[0]  8 -
From: Heiko Stuebner he...@sntech.de
Subject: [PATCH] cpufreq: arm_big_little: add support for shared cluster 
regulators

In some socs or board designs the supplying regulator is shared between
more than one cluster but the current regulator support for big_little
sets the target voltage without any tolerance.

So when cluster0 requests 0.9V and cluster1 1.3V no suitable frequency
span is available that fits both. To accomodate this, look for shared
regulators and calculate the maximum voltage necessary. If the regulator
of the remote cluster has a lower voltage, its maximum also gets increased.

If cluster supplies are not shared, the behaviour is the same as before
with one specific voltage being set instead of a voltage-range.

When adapting shared voltages the remote clusters need to be locked too,
because cpufreq can very well try to change more than one cluster at the
same time. While the used mutex_trylock prevents deadlocks reliably,
it might also prevent some (or a lot) frequency changes from succeeding:

lock cluster0
lock cluster1
trylock cluster1
trylock cluster0
both fail

I'm probably simply overlooking some better way currently.

Signed-off-by: Heiko Stuebner he...@sntech.de
---
 drivers/cpufreq/arm_big_little.c | 102 ++-
 1 file changed, 91 insertions(+), 11 deletions(-)

diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
index e04ca0c..c65b111 100644
--- a/drivers/cpufreq/arm_big_little.c
+++ b/drivers/cpufreq/arm_big_little.c
@@ -130,12 +130,78 @@ static unsigned int bL_cpufreq_get_rate(unsigned int cpu)
 }
 
 static int
+bL_adapt_shared_regulators(u32 cluster, unsigned long *volt_max)
+{
+   unsigned long other_volt;
+   int ret, i;
+
+   for (i = 0; i  MAX_CLUSTERS; i++) {
+   if (i == cluster || IS_ERR_OR_NULL(reg[i]))
+   continue;
+
+   if (regulator_is_match(reg[cluster], reg[i])) {
+   other_volt = regulator_get_voltage(reg[i]);
+   if (other_volt  *volt_max) {
+   *volt_max = other_volt;
+   } else {
+   pr_debug(%s: adapting shared regulator in 
cluster %d to %lu-%lu mV\n,
+__func__, i, other_volt / 1000, 
*volt_max / 1000);
+   ret = regulator_set_voltage(reg[i], other_volt, 
*volt_max);
+   if (ret) {
+   pr_err(%s: shared-supply for cluster: 
%d, failed to scale voltage up: %d\n,
+  __func__, cluster, ret);
+   return ret;
+   }
+   }
+   }
+   }
+
+   return 0;
+}
+
+static int
+bL_lock_shared_regulators(u32 cluster)
+{
+   int ret, i;
+
+   for (i = 0; i  MAX_CLUSTERS; i++) {
+   if (i == cluster || IS_ERR_OR_NULL(reg[i]))
+   continue;
+
+   if (regulator_is_match(reg[cluster], reg[i])) {
+   ret = mutex_trylock(cluster_lock[i]);
+   if (!ret) {
+   for (i--; i = 0; i--)
+   mutex_unlock(cluster_lock[i]);
+   return -EBUSY;
+   }
+   }
+   }
+
+   return 0;
+}
+
+static void
+bL_unlock_shared_regulators(u32 cluster)
+{
+   int i;
+
+   for (i = 0; i  MAX_CLUSTERS; i++) {
+   if (i == cluster || IS_ERR_OR_NULL(reg[i]))
+   continue;
+
+  

Re: [PATCH 1/8] cpufreq: arm_big_little: add cluster regulator support

2015-04-26 Thread Viresh Kumar
On 21 April 2015 at 18:47, Bartlomiej Zolnierkiewicz
b.zolnier...@samsung.com wrote:
 Add cluster regulator support as a preparation to adding
 generic arm_big_little_dt cpufreq_dt driver support for
 ODROID-XU3 board.  This allows arm_big_little[_dt] driver

This is irrelevant here, its not about XU3 but any board that
wants to use it..

 to set not only the frequency but also the voltage (which
 is obtained from operating point's voltage value) for CPU
 clusters.

 Cc: Kukjin Kim kgene@samsung.com
 Cc: Doug Anderson diand...@chromium.org
 Cc: Javier Martinez Canillas javier.marti...@collabora.co.uk
 Cc: Andreas Faerber afaer...@suse.de
 Cc: Sachin Kamat sachin.ka...@linaro.org
 Cc: Thomas Abraham thomas...@samsung.com
 Signed-off-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com
 ---
  .../bindings/cpufreq/arm_big_little_dt.txt |4 +
  drivers/cpufreq/arm_big_little.c   |  153 
 +---
  2 files changed, 139 insertions(+), 18 deletions(-)

 diff --git a/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt 
 b/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt
 index 0715695..8ca4a12 100644
 --- a/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt
 +++ b/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt
 @@ -18,6 +18,10 @@ Required properties:
  Optional properties:
  - clock-latency: Specify the possible maximum transition latency for clock,
in unit of nanoseconds.
 +- cpu-cluster.0-supply: Provides the regulator node supplying voltage to CPU
 +  cluster 0.
 +- cpu-cluster.1-supply: Provides the regulator node supplying voltage to CPU
 +  cluster 1.

I don't think you need these..

http://permalink.gmane.org/gmane.linux.power-management.general/58548

  Examples:

 diff --git a/drivers/cpufreq/arm_big_little.c 
 b/drivers/cpufreq/arm_big_little.c
 index e1a6ba6..edb461b 100644
 --- a/drivers/cpufreq/arm_big_little.c
 +++ b/drivers/cpufreq/arm_big_little.c
 @@ -31,6 +31,7 @@
  #include linux/slab.h
  #include linux/topology.h
  #include linux/types.h
 +#include linux/regulator/consumer.h
  #include asm/bL_switcher.h

  #include arm_big_little.h
 @@ -54,6 +55,9 @@ static bool bL_switching_enabled;

  static struct cpufreq_arm_bL_ops *arm_bL_ops;
  static struct clk *clk[MAX_CLUSTERS];
 +static struct regulator *reg[MAX_CLUSTERS];
 +static struct device *cpu_devs[MAX_CLUSTERS];
 +static int transition_latencies[MAX_CLUSTERS];
  static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
  static atomic_t cluster_usage[MAX_CLUSTERS + 1];

 @@ -122,7 +126,76 @@ static unsigned int bL_cpufreq_get_rate(unsigned int cpu)
 }
  }

 -static unsigned int
 +static int
 +bL_cpufreq_set_rate_cluster(u32 cpu, u32 cluster, u32 new_rate)
 +{
 +   unsigned long volt = 0, volt_old = 0;
 +   long freq_Hz;
 +   u32 old_rate;
 +   int ret;
 +
 +   freq_Hz = new_rate * 1000;
 +   old_rate = clk_get_rate(clk[cluster]) / 1000;
 +
 +   if (!IS_ERR(reg[cluster])) {
 +   struct dev_pm_opp *opp;
 +   unsigned long opp_freq;
 +
 +   rcu_read_lock();
 +   opp = dev_pm_opp_find_freq_ceil(cpu_devs[cluster], freq_Hz);
 +   if (IS_ERR(opp)) {
 +   rcu_read_unlock();
 +   pr_err(%s: cpu %d, cluster: %d, failed to find OPP 
 for %ld\n,
 +   __func__, cpu, cluster, freq_Hz);
 +   return PTR_ERR(opp);
 +   }
 +   volt = dev_pm_opp_get_voltage(opp);
 +   opp_freq = dev_pm_opp_get_freq(opp);
 +   rcu_read_unlock();
 +   volt_old = regulator_get_voltage(reg[cluster]);
 +   pr_debug(%s: cpu %d, cluster: %d, Found OPP: %ld kHz, %ld 
 uV\n,
 +   __func__, cpu, cluster, opp_freq / 1000, volt);
 +   }
 +
 +   pr_debug(%s: cpu %d, cluster: %d, %u MHz, %ld mV -- %u MHz, %ld 
 mV\n,
 +   __func__, cpu, cluster,
 +   old_rate / 1000, (volt_old  0) ? volt_old / 1000 : -1,
 +   new_rate / 1000, volt ? volt / 1000 : -1);
 +
 +   /* scaling up? scale voltage before frequency */
 +   if (!IS_ERR(reg[cluster])  new_rate  old_rate) {
 +   ret = regulator_set_voltage_tol(reg[cluster], volt, 0);
 +   if (ret) {
 +   pr_err(%s: cpu: %d, cluster: %d, failed to scale 
 voltage up: %d\n,
 +   __func__, cpu, cluster, ret);
 +   return ret;
 +   }
 +   }
 +
 +   ret = clk_set_rate(clk[cluster], new_rate * 1000);
 +   if (WARN_ON(ret)) {
 +   pr_err(%s: clk_set_rate failed: %d, cluster: %d\n,
 +   __func__, cluster, ret);
 +   if (!IS_ERR(reg[cluster])  volt_old  0)
 +   regulator_set_voltage_tol(reg[cluster], volt_old, 0);
 +   return ret;
 +  

Re: [PATCH 1/8] cpufreq: arm_big_little: add cluster regulator support

2015-04-22 Thread Lukasz Majewski
Hi Bartlomiej,

 Add cluster regulator support as a preparation to adding
 generic arm_big_little_dt cpufreq_dt driver support for
 ODROID-XU3 board.  This allows arm_big_little[_dt] driver
 to set not only the frequency but also the voltage (which
 is obtained from operating point's voltage value) for CPU
 clusters.
 
 Cc: Kukjin Kim kgene@samsung.com
 Cc: Doug Anderson diand...@chromium.org
 Cc: Javier Martinez Canillas javier.marti...@collabora.co.uk
 Cc: Andreas Faerber afaer...@suse.de
 Cc: Sachin Kamat sachin.ka...@linaro.org
 Cc: Thomas Abraham thomas...@samsung.com
 Signed-off-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com
 ---
  .../bindings/cpufreq/arm_big_little_dt.txt |4 +
  drivers/cpufreq/arm_big_little.c   |  153
 +--- 2 files changed, 139 insertions(+), 18
 deletions(-)
 
 diff --git
 a/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt
 b/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt
 index 0715695..8ca4a12 100644 ---
 a/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt +++
 b/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt @@
 -18,6 +18,10 @@ Required properties: Optional properties:
  - clock-latency: Specify the possible maximum transition latency for
 clock, in unit of nanoseconds.
 +- cpu-cluster.0-supply: Provides the regulator node supplying
 voltage to CPU
 +  cluster 0.
 +- cpu-cluster.1-supply: Provides the regulator node supplying
 voltage to CPU
 +  cluster 1.
  
  Examples:
  
 diff --git a/drivers/cpufreq/arm_big_little.c
 b/drivers/cpufreq/arm_big_little.c index e1a6ba6..edb461b 100644
 --- a/drivers/cpufreq/arm_big_little.c
 +++ b/drivers/cpufreq/arm_big_little.c
 @@ -31,6 +31,7 @@
  #include linux/slab.h
  #include linux/topology.h
  #include linux/types.h
 +#include linux/regulator/consumer.h
  #include asm/bL_switcher.h
  
  #include arm_big_little.h
 @@ -54,6 +55,9 @@ static bool bL_switching_enabled;
  
  static struct cpufreq_arm_bL_ops *arm_bL_ops;
  static struct clk *clk[MAX_CLUSTERS];
 +static struct regulator *reg[MAX_CLUSTERS];
 +static struct device *cpu_devs[MAX_CLUSTERS];
 +static int transition_latencies[MAX_CLUSTERS];
  static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
  static atomic_t cluster_usage[MAX_CLUSTERS + 1];
  
 @@ -122,7 +126,76 @@ static unsigned int bL_cpufreq_get_rate(unsigned
 int cpu) }
  }
  
 -static unsigned int
 +static int
 +bL_cpufreq_set_rate_cluster(u32 cpu, u32 cluster, u32 new_rate)
 +{
 + unsigned long volt = 0, volt_old = 0;
 + long freq_Hz;
 + u32 old_rate;
 + int ret;
 +
 + freq_Hz = new_rate * 1000;
 + old_rate = clk_get_rate(clk[cluster]) / 1000;
 +
 + if (!IS_ERR(reg[cluster])) {
 + struct dev_pm_opp *opp;
 + unsigned long opp_freq;
 +
 + rcu_read_lock();
 + opp = dev_pm_opp_find_freq_ceil(cpu_devs[cluster],
 freq_Hz);
 + if (IS_ERR(opp)) {
 + rcu_read_unlock();
 + pr_err(%s: cpu %d, cluster: %d, failed to
 find OPP for %ld\n,
 + __func__, cpu, cluster, freq_Hz);
 + return PTR_ERR(opp);
 + }
 + volt = dev_pm_opp_get_voltage(opp);
 + opp_freq = dev_pm_opp_get_freq(opp);
 + rcu_read_unlock();
 + volt_old = regulator_get_voltage(reg[cluster]);
 + pr_debug(%s: cpu %d, cluster: %d, Found OPP: %ld
 kHz, %ld uV\n,
 + __func__, cpu, cluster, opp_freq / 1000,
 volt);
 + }
 +
 + pr_debug(%s: cpu %d, cluster: %d, %u MHz, %ld mV -- %u
 MHz, %ld mV\n,
 + __func__, cpu, cluster,
 + old_rate / 1000, (volt_old  0) ? volt_old / 1000 :
 -1,
 + new_rate / 1000, volt ? volt / 1000 : -1);
 +
 + /* scaling up? scale voltage before frequency */
 + if (!IS_ERR(reg[cluster])  new_rate  old_rate) {
 + ret = regulator_set_voltage_tol(reg[cluster], volt,
 0);
 + if (ret) {
 + pr_err(%s: cpu: %d, cluster: %d, failed to
 scale voltage up: %d\n,
 + __func__, cpu, cluster, ret);
 + return ret;
 + }
 + }
 +
 + ret = clk_set_rate(clk[cluster], new_rate * 1000);
 + if (WARN_ON(ret)) {
 + pr_err(%s: clk_set_rate failed: %d, cluster: %d\n,
 + __func__, cluster, ret);
 + if (!IS_ERR(reg[cluster])  volt_old  0)
 + regulator_set_voltage_tol(reg[cluster],
 volt_old, 0);
 + return ret;
 + }
 +
 + /* scaling down? scale voltage after frequency */
 + if (!IS_ERR(reg[cluster])  new_rate  old_rate) {
 + ret = regulator_set_voltage_tol(reg[cluster], volt,
 0);
 + if (ret) {
 + pr_err(%s: cpu: %d, cluster: %d, failed to
 scale voltage down: %d\n,
 + 

[PATCH 1/8] cpufreq: arm_big_little: add cluster regulator support

2015-04-21 Thread Bartlomiej Zolnierkiewicz
Add cluster regulator support as a preparation to adding
generic arm_big_little_dt cpufreq_dt driver support for
ODROID-XU3 board.  This allows arm_big_little[_dt] driver
to set not only the frequency but also the voltage (which
is obtained from operating point's voltage value) for CPU
clusters.

Cc: Kukjin Kim kgene@samsung.com
Cc: Doug Anderson diand...@chromium.org
Cc: Javier Martinez Canillas javier.marti...@collabora.co.uk
Cc: Andreas Faerber afaer...@suse.de
Cc: Sachin Kamat sachin.ka...@linaro.org
Cc: Thomas Abraham thomas...@samsung.com
Signed-off-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com
---
 .../bindings/cpufreq/arm_big_little_dt.txt |4 +
 drivers/cpufreq/arm_big_little.c   |  153 +---
 2 files changed, 139 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt 
b/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt
index 0715695..8ca4a12 100644
--- a/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt
+++ b/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt
@@ -18,6 +18,10 @@ Required properties:
 Optional properties:
 - clock-latency: Specify the possible maximum transition latency for clock,
   in unit of nanoseconds.
+- cpu-cluster.0-supply: Provides the regulator node supplying voltage to CPU
+  cluster 0.
+- cpu-cluster.1-supply: Provides the regulator node supplying voltage to CPU
+  cluster 1.
 
 Examples:
 
diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
index e1a6ba6..edb461b 100644
--- a/drivers/cpufreq/arm_big_little.c
+++ b/drivers/cpufreq/arm_big_little.c
@@ -31,6 +31,7 @@
 #include linux/slab.h
 #include linux/topology.h
 #include linux/types.h
+#include linux/regulator/consumer.h
 #include asm/bL_switcher.h
 
 #include arm_big_little.h
@@ -54,6 +55,9 @@ static bool bL_switching_enabled;
 
 static struct cpufreq_arm_bL_ops *arm_bL_ops;
 static struct clk *clk[MAX_CLUSTERS];
+static struct regulator *reg[MAX_CLUSTERS];
+static struct device *cpu_devs[MAX_CLUSTERS];
+static int transition_latencies[MAX_CLUSTERS];
 static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
 static atomic_t cluster_usage[MAX_CLUSTERS + 1];
 
@@ -122,7 +126,76 @@ static unsigned int bL_cpufreq_get_rate(unsigned int cpu)
}
 }
 
-static unsigned int
+static int
+bL_cpufreq_set_rate_cluster(u32 cpu, u32 cluster, u32 new_rate)
+{
+   unsigned long volt = 0, volt_old = 0;
+   long freq_Hz;
+   u32 old_rate;
+   int ret;
+
+   freq_Hz = new_rate * 1000;
+   old_rate = clk_get_rate(clk[cluster]) / 1000;
+
+   if (!IS_ERR(reg[cluster])) {
+   struct dev_pm_opp *opp;
+   unsigned long opp_freq;
+
+   rcu_read_lock();
+   opp = dev_pm_opp_find_freq_ceil(cpu_devs[cluster], freq_Hz);
+   if (IS_ERR(opp)) {
+   rcu_read_unlock();
+   pr_err(%s: cpu %d, cluster: %d, failed to find OPP for 
%ld\n,
+   __func__, cpu, cluster, freq_Hz);
+   return PTR_ERR(opp);
+   }
+   volt = dev_pm_opp_get_voltage(opp);
+   opp_freq = dev_pm_opp_get_freq(opp);
+   rcu_read_unlock();
+   volt_old = regulator_get_voltage(reg[cluster]);
+   pr_debug(%s: cpu %d, cluster: %d, Found OPP: %ld kHz, %ld 
uV\n,
+   __func__, cpu, cluster, opp_freq / 1000, volt);
+   }
+
+   pr_debug(%s: cpu %d, cluster: %d, %u MHz, %ld mV -- %u MHz, %ld mV\n,
+   __func__, cpu, cluster,
+   old_rate / 1000, (volt_old  0) ? volt_old / 1000 : -1,
+   new_rate / 1000, volt ? volt / 1000 : -1);
+
+   /* scaling up? scale voltage before frequency */
+   if (!IS_ERR(reg[cluster])  new_rate  old_rate) {
+   ret = regulator_set_voltage_tol(reg[cluster], volt, 0);
+   if (ret) {
+   pr_err(%s: cpu: %d, cluster: %d, failed to scale 
voltage up: %d\n,
+   __func__, cpu, cluster, ret);
+   return ret;
+   }
+   }
+
+   ret = clk_set_rate(clk[cluster], new_rate * 1000);
+   if (WARN_ON(ret)) {
+   pr_err(%s: clk_set_rate failed: %d, cluster: %d\n,
+   __func__, cluster, ret);
+   if (!IS_ERR(reg[cluster])  volt_old  0)
+   regulator_set_voltage_tol(reg[cluster], volt_old, 0);
+   return ret;
+   }
+
+   /* scaling down? scale voltage after frequency */
+   if (!IS_ERR(reg[cluster])  new_rate  old_rate) {
+   ret = regulator_set_voltage_tol(reg[cluster], volt, 0);
+   if (ret) {
+   pr_err(%s: cpu: %d, cluster: %d, failed to scale 
voltage down: %d\n,
+   __func__, cpu, cluster, ret);
+