Re: [PATCH v3 3/3] cpufreq: arm_big_little: Add support to register a cpufreq cooling device
Viresh Kumar writes:
> On 09-11-15, 17:29, Punit Agrawal wrote:
>> Register passive cooling devices when initialising cpufreq on
>> big.LITTLE systems. If the device tree provides a dynamic power
>> coefficient for the CPUs then the bound cooling device will support
>> the extensions that allow it to be used with all the existing thermal
>> governors including the power allocator governor.
>>
>> A cooling device will be created per individual frequency domain and
>> can be bound to thermal zones via the thermal DT bindings.
>>
>> Signed-off-by: Punit Agrawal
>> Acked-by: Viresh Kumar
>> Cc: Sudeep Holla
>> Cc: Eduardo Valentin
>> ---
>> drivers/cpufreq/arm_big_little.c | 52
>> ++--
>> 1 file changed, 50 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/cpufreq/arm_big_little.c
>> b/drivers/cpufreq/arm_big_little.c
>> index f1e42f8..72a2777 100644
>> --- a/drivers/cpufreq/arm_big_little.c
>> +++ b/drivers/cpufreq/arm_big_little.c
>> @@ -23,6 +23,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -55,6 +56,10 @@ static bool bL_switching_enabled;
>> #define ACTUAL_FREQ(cluster, freq) ((cluster == A7_CLUSTER) ? freq << 1 :
>> freq)
>> #define VIRT_FREQ(cluster, freq)((cluster == A7_CLUSTER) ? freq >> 1 :
>> freq)
>>
>> +struct private_data {
>> +struct thermal_cooling_device *cdev;
>> +};
>
> I think we need to be consistent within the driver, and so this must
> be stored in a similar way to what we do for other structures. We have
> static arrays for them, please do it that way only OR first change all
> of them to be part of a bigger private_data structure.
Sure, I'll switch to using static arrays. There seem to be a lot of
assumptions that might be easily broken in moving all the data into a
single structure. I'll leave that as a later change.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 3/3] cpufreq: arm_big_little: Add support to register a cpufreq cooling device
On 09-11-15, 17:29, Punit Agrawal wrote:
> Register passive cooling devices when initialising cpufreq on
> big.LITTLE systems. If the device tree provides a dynamic power
> coefficient for the CPUs then the bound cooling device will support
> the extensions that allow it to be used with all the existing thermal
> governors including the power allocator governor.
>
> A cooling device will be created per individual frequency domain and
> can be bound to thermal zones via the thermal DT bindings.
>
> Signed-off-by: Punit Agrawal
> Acked-by: Viresh Kumar
> Cc: Sudeep Holla
> Cc: Eduardo Valentin
> ---
> drivers/cpufreq/arm_big_little.c | 52
> ++--
> 1 file changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/arm_big_little.c
> b/drivers/cpufreq/arm_big_little.c
> index f1e42f8..72a2777 100644
> --- a/drivers/cpufreq/arm_big_little.c
> +++ b/drivers/cpufreq/arm_big_little.c
> @@ -23,6 +23,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -55,6 +56,10 @@ static bool bL_switching_enabled;
> #define ACTUAL_FREQ(cluster, freq) ((cluster == A7_CLUSTER) ? freq << 1 :
> freq)
> #define VIRT_FREQ(cluster, freq)((cluster == A7_CLUSTER) ? freq >> 1 :
> freq)
>
> +struct private_data {
> + struct thermal_cooling_device *cdev;
> +};
I think we need to be consistent within the driver, and so this must
be stored in a similar way to what we do for other structures. We have
static arrays for them, please do it that way only OR first change all
of them to be part of a bigger private_data structure.
--
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 3/3] cpufreq: arm_big_little: Add support to register a cpufreq cooling device
On Mon, Nov 09, 2015 at 05:29:23PM +, Punit Agrawal wrote:
> Register passive cooling devices when initialising cpufreq on
> big.LITTLE systems. If the device tree provides a dynamic power
> coefficient for the CPUs then the bound cooling device will support
> the extensions that allow it to be used with all the existing thermal
> governors including the power allocator governor.
>
> A cooling device will be created per individual frequency domain and
> can be bound to thermal zones via the thermal DT bindings.
>
> Signed-off-by: Punit Agrawal
> Acked-by: Viresh Kumar
> Cc: Sudeep Holla
> Cc: Eduardo Valentin
> ---
> drivers/cpufreq/arm_big_little.c | 52
> ++--
> 1 file changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/arm_big_little.c
> b/drivers/cpufreq/arm_big_little.c
> index f1e42f8..72a2777 100644
> --- a/drivers/cpufreq/arm_big_little.c
> +++ b/drivers/cpufreq/arm_big_little.c
> @@ -23,6 +23,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -55,6 +56,10 @@ static bool bL_switching_enabled;
> #define ACTUAL_FREQ(cluster, freq) ((cluster == A7_CLUSTER) ? freq << 1 :
> freq)
> #define VIRT_FREQ(cluster, freq)((cluster == A7_CLUSTER) ? freq >> 1 :
> freq)
>
> +struct private_data {
> + struct thermal_cooling_device *cdev;
> +};
> +
My first impression was that this is redundant and you could store the
pointer to cdev in driver_data itself. But seeing that it's mirroring
the structure in cpufreq-dt and it's simpler to do it this way I guess
it's ok to create this struct with only one pointer. You can add my
Reviewed-by: Javi Merino
> static struct cpufreq_arm_bL_ops *arm_bL_ops;
> static struct clk *clk[MAX_CLUSTERS];
> static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
> @@ -440,6 +445,7 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
> {
> u32 cur_cluster = cpu_to_cluster(policy->cpu);
> struct device *cpu_dev;
> + struct private_data *priv;
> int ret;
>
> cpu_dev = get_cpu_device(policy->cpu);
> @@ -457,9 +463,15 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
> if (ret) {
> dev_err(cpu_dev, "CPU %d, cluster: %d invalid freq table\n",
> policy->cpu, cur_cluster);
> - put_cluster_clk_and_freq_table(cpu_dev);
> - return ret;
> + goto out_free_cpufreq_table;
> + }
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> + if (!priv) {
> + ret = -ENOMEM;
> + goto out_free_cpufreq_table;
> }
> + policy->driver_data = priv;
>
> if (cur_cluster < MAX_CLUSTERS) {
> int cpu;
> @@ -484,12 +496,19 @@ static int bL_cpufreq_init(struct cpufreq_policy
> *policy)
>
> dev_info(cpu_dev, "%s: CPU %d initialized\n", __func__, policy->cpu);
> return 0;
> +
> +out_free_cpufreq_table:
> + put_cluster_clk_and_freq_table(cpu_dev);
> +
> + return ret;
> }
>
> static int bL_cpufreq_exit(struct cpufreq_policy *policy)
> {
> struct device *cpu_dev;
> + struct private_data *priv = policy->driver_data;
>
> + cpufreq_cooling_unregister(priv->cdev);
> cpu_dev = get_cpu_device(policy->cpu);
> if (!cpu_dev) {
> pr_err("%s: failed to get cpu%d device\n", __func__,
> @@ -498,11 +517,39 @@ static int bL_cpufreq_exit(struct cpufreq_policy
> *policy)
> }
>
> put_cluster_clk_and_freq_table(cpu_dev);
> + kfree(priv);
> dev_dbg(cpu_dev, "%s: Exited, cpu: %d\n", __func__, policy->cpu);
>
> return 0;
> }
>
> +static void bL_cpufreq_ready(struct cpufreq_policy *policy)
> +{
> + struct device *cpu_dev = get_cpu_device(policy->cpu);
> + struct device_node *np = of_node_get(cpu_dev->of_node);
> + struct private_data *priv = policy->driver_data;
> +
> + if (WARN_ON(!np))
> + return;
> +
> + if (of_find_property(np, "#cooling-cells", NULL)) {
> + u32 power_coefficient = 0;
> +
> + of_property_read_u32(np, "dynamic-power-coefficient",
> + &power_coefficient);
> +
> + priv->cdev = of_cpufreq_power_cooling_register(np,
> + policy->related_cpus, power_coefficient, NULL);
> + if (IS_ERR(priv->cdev)) {
> + dev_err(cpu_dev,
> + "running cpufreq without cooling device: %ld\n",
> + PTR_ERR(priv->cdev));
> + priv->cdev = NULL;
> + }
> + }
> + of_node_put(np);
> +}
> +
> static struct cpufreq_driver bL_cpufreq_driver = {
> .name = "arm-big-little",
> .flags = CPUFREQ_STICKY |
> @@ -513,6 +560,7 @@ static struct cpufreq_driver bL_cpufreq_driver = {
> .get= bL_cpufr
[PATCH v3 3/3] cpufreq: arm_big_little: Add support to register a cpufreq cooling device
Register passive cooling devices when initialising cpufreq on
big.LITTLE systems. If the device tree provides a dynamic power
coefficient for the CPUs then the bound cooling device will support
the extensions that allow it to be used with all the existing thermal
governors including the power allocator governor.
A cooling device will be created per individual frequency domain and
can be bound to thermal zones via the thermal DT bindings.
Signed-off-by: Punit Agrawal
Acked-by: Viresh Kumar
Cc: Sudeep Holla
Cc: Eduardo Valentin
---
drivers/cpufreq/arm_big_little.c | 52 ++--
1 file changed, 50 insertions(+), 2 deletions(-)
diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
index f1e42f8..72a2777 100644
--- a/drivers/cpufreq/arm_big_little.c
+++ b/drivers/cpufreq/arm_big_little.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -55,6 +56,10 @@ static bool bL_switching_enabled;
#define ACTUAL_FREQ(cluster, freq) ((cluster == A7_CLUSTER) ? freq << 1 :
freq)
#define VIRT_FREQ(cluster, freq)((cluster == A7_CLUSTER) ? freq >> 1 :
freq)
+struct private_data {
+ struct thermal_cooling_device *cdev;
+};
+
static struct cpufreq_arm_bL_ops *arm_bL_ops;
static struct clk *clk[MAX_CLUSTERS];
static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
@@ -440,6 +445,7 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
{
u32 cur_cluster = cpu_to_cluster(policy->cpu);
struct device *cpu_dev;
+ struct private_data *priv;
int ret;
cpu_dev = get_cpu_device(policy->cpu);
@@ -457,9 +463,15 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
if (ret) {
dev_err(cpu_dev, "CPU %d, cluster: %d invalid freq table\n",
policy->cpu, cur_cluster);
- put_cluster_clk_and_freq_table(cpu_dev);
- return ret;
+ goto out_free_cpufreq_table;
+ }
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ ret = -ENOMEM;
+ goto out_free_cpufreq_table;
}
+ policy->driver_data = priv;
if (cur_cluster < MAX_CLUSTERS) {
int cpu;
@@ -484,12 +496,19 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
dev_info(cpu_dev, "%s: CPU %d initialized\n", __func__, policy->cpu);
return 0;
+
+out_free_cpufreq_table:
+ put_cluster_clk_and_freq_table(cpu_dev);
+
+ return ret;
}
static int bL_cpufreq_exit(struct cpufreq_policy *policy)
{
struct device *cpu_dev;
+ struct private_data *priv = policy->driver_data;
+ cpufreq_cooling_unregister(priv->cdev);
cpu_dev = get_cpu_device(policy->cpu);
if (!cpu_dev) {
pr_err("%s: failed to get cpu%d device\n", __func__,
@@ -498,11 +517,39 @@ static int bL_cpufreq_exit(struct cpufreq_policy *policy)
}
put_cluster_clk_and_freq_table(cpu_dev);
+ kfree(priv);
dev_dbg(cpu_dev, "%s: Exited, cpu: %d\n", __func__, policy->cpu);
return 0;
}
+static void bL_cpufreq_ready(struct cpufreq_policy *policy)
+{
+ struct device *cpu_dev = get_cpu_device(policy->cpu);
+ struct device_node *np = of_node_get(cpu_dev->of_node);
+ struct private_data *priv = policy->driver_data;
+
+ if (WARN_ON(!np))
+ return;
+
+ if (of_find_property(np, "#cooling-cells", NULL)) {
+ u32 power_coefficient = 0;
+
+ of_property_read_u32(np, "dynamic-power-coefficient",
+&power_coefficient);
+
+ priv->cdev = of_cpufreq_power_cooling_register(np,
+ policy->related_cpus, power_coefficient, NULL);
+ if (IS_ERR(priv->cdev)) {
+ dev_err(cpu_dev,
+ "running cpufreq without cooling device: %ld\n",
+ PTR_ERR(priv->cdev));
+ priv->cdev = NULL;
+ }
+ }
+ of_node_put(np);
+}
+
static struct cpufreq_driver bL_cpufreq_driver = {
.name = "arm-big-little",
.flags = CPUFREQ_STICKY |
@@ -513,6 +560,7 @@ static struct cpufreq_driver bL_cpufreq_driver = {
.get= bL_cpufreq_get_rate,
.init = bL_cpufreq_init,
.exit = bL_cpufreq_exit,
+ .ready = bL_cpufreq_ready,
.attr = cpufreq_generic_attr,
};
--
2.5.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
