Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752338AbbKJMe6 (ORCPT ); Tue, 10 Nov 2015 07:34:58 -0500 Received: from foss.arm.com ([217.140.101.70]:55725 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751111AbbKJMe4 (ORCPT ); Tue, 10 Nov 2015 07:34:56 -0500 Date: Tue, 10 Nov 2015 12:34:52 +0000 From: Javi Merino To: Punit Agrawal Cc: devicetree@vger.kernel.org, viresh.kumar@linaro.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, edubezval@gmail.com, dawei.chien@mediatek.com, Sudeep Holla Subject: Re: [PATCH v3 3/3] cpufreq: arm_big_little: Add support to register a cpufreq cooling device Message-ID: <20151110123452.GD3551@e104805> References: <1447090163-13700-1-git-send-email-punit.agrawal@arm.com> <1447090163-13700-4-git-send-email-punit.agrawal@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1447090163-13700-4-git-send-email-punit.agrawal@arm.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5371 Lines: 157 On Mon, Nov 09, 2015 at 05:29:23PM +0000, 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_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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/