Re: [PATCH v3 2/6] thermal: Add generic cpufreq cooling implementation
On 9 May 2012 01:46, Andrew Morton a...@linux-foundation.org wrote:
On Tue, 8 May 2012 21:48:14 +0530
Amit Daniel Kachhap amit.kach...@linaro.org wrote:
This patch adds support for generic cpu thermal cooling low level
implementations using frequency scaling up/down based on the registration
parameters. Different cpu related cooling devices can be registered by the
user and the binding of these cooling devices to the corresponding
trip points can be easily done as the registration APIs return the
cooling device pointer. The user of these APIs are responsible for
passing clipping frequency . The drivers can also register to recieve
notification about any cooling action called. Even the driver can effect
the cooling action by modifying the default data such as freq_clip_max if
needed.
...
+struct cpufreq_cooling_device {
+ int id;
+ struct thermal_cooling_device *cool_dev;
+ struct freq_clip_table *tab_ptr;
+ unsigned int tab_size;
+ unsigned int cpufreq_state;
+ const struct cpumask *allowed_cpus;
+ struct list_head node;
+};
It would be nice to document the fields. Especially id, tab_size,
cpufreq_state and node. For `node' we should describe the locking for
the list, and describe which list_head anchors this list.
Thanks Andrew for the detailed review. I will add more documentation
and post the next version shortly.
+static LIST_HEAD(cooling_cpufreq_list);
+static DEFINE_MUTEX(cooling_cpufreq_lock);
+static DEFINE_IDR(cpufreq_idr);
+static DEFINE_PER_CPU(unsigned int, max_policy_freq);
+static struct freq_clip_table *notify_table;
+static int notify_state;
+static BLOCKING_NOTIFIER_HEAD(cputherm_state_notifier_list);
+
+static int get_idr(struct idr *idr, struct mutex *lock, int *id)
+{
+ int err;
+again:
+ if (unlikely(idr_pre_get(idr, GFP_KERNEL) == 0))
+ return -ENOMEM;
+
+ if (lock)
+ mutex_lock(lock);
The test for NULL `lock' is unneeded. In fact the `lock' argument
could be removed altogether - just use cooling_cpufreq_lock directly.
Agreed
+ err = idr_get_new(idr, NULL, id);
+ if (lock)
+ mutex_unlock(lock);
+ if (unlikely(err == -EAGAIN))
+ goto again;
+ else if (unlikely(err))
+ return err;
+
+ *id = *id MAX_ID_MASK;
+ return 0;
+}
+
+static void release_idr(struct idr *idr, struct mutex *lock, int id)
+{
+ if (lock)
+ mutex_lock(lock);
Ditto.
+ idr_remove(idr, id);
+ if (lock)
+ mutex_unlock(lock);
+}
+
...
+
+/*Below codes defines functions to be used for cpufreq as cooling device*/
+static bool is_cpufreq_valid(int cpu)
+{
+ struct cpufreq_policy policy;
+ return !cpufreq_get_policy(policy, cpu) ? true : false;
Can use
Ok
return !cpufreq_get_policy(policy, cpu);
+}
+
+static int cpufreq_apply_cooling(struct cpufreq_cooling_device
*cpufreq_device,
+ unsigned long cooling_state)
+{
+ unsigned int event, cpuid;
+ struct freq_clip_table *th_table;
+
+ if (cooling_state cpufreq_device-tab_size)
+ return -EINVAL;
+
+ cpufreq_device-cpufreq_state = cooling_state;
+
+ /*cpufreq thermal notifier uses this cpufreq device pointer*/
This code looks like it was written by two people.
/* One who does this */
/*And one who does this*/
The first one was right. Please go through all the comments in all the
patches and get the layout consistent?
Sure will add more details.
+ notify_state = cooling_state;
+
+ if (notify_state 0) {
+ th_table = (cpufreq_device-tab_ptr[cooling_state - 1]);
+ memcpy(notify_table, th_table, sizeof(struct freq_clip_table));
+ event = CPUFREQ_COOLING_TYPE;
+ blocking_notifier_call_chain(cputherm_state_notifier_list,
+ event, notify_table);
+ }
+
+ for_each_cpu(cpuid, cpufreq_device-allowed_cpus) {
+ if (is_cpufreq_valid(cpuid))
+ cpufreq_update_policy(cpuid);
+ }
+
+ notify_state = -1;
+
+ return 0;
+}
+
+static int cpufreq_thermal_notifier(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct cpufreq_policy *policy = data;
+ unsigned long max_freq = 0;
+
+ if ((event != CPUFREQ_ADJUST) || (notify_state == -1))
Please document `notify_state', at its definition site. This reader
doesn't know what notify_state == -1 *means*.
+ return 0;
+
+ if (notify_state 0) {
+ max_freq = notify_table-freq_clip_max;
+
+ if (per_cpu(max_policy_freq, policy-cpu) == 0)
+ per_cpu(max_policy_freq, policy-cpu) = policy-max;
+ } else {
+ if (per_cpu(max_policy_freq, policy-cpu) != 0) {
+
[PATCH v3 2/6] thermal: Add generic cpufreq cooling implementation
This patch adds support for generic cpu thermal cooling low level implementations using frequency scaling up/down based on the registration parameters. Different cpu related cooling devices can be registered by the user and the binding of these cooling devices to the corresponding trip points can be easily done as the registration APIs return the cooling device pointer. The user of these APIs are responsible for passing clipping frequency . The drivers can also register to recieve notification about any cooling action called. Even the driver can effect the cooling action by modifying the default data such as freq_clip_max if needed. Signed-off-by: Amit Daniel Kachhap amit.kach...@linaro.org --- Documentation/thermal/cpu-cooling-api.txt | 60 + drivers/thermal/Kconfig | 11 + drivers/thermal/Makefile |3 +- drivers/thermal/cpu_cooling.c | 359 + include/linux/cpu_cooling.h | 62 + 5 files changed, 494 insertions(+), 1 deletions(-) create mode 100644 Documentation/thermal/cpu-cooling-api.txt create mode 100644 drivers/thermal/cpu_cooling.c create mode 100644 include/linux/cpu_cooling.h diff --git a/Documentation/thermal/cpu-cooling-api.txt b/Documentation/thermal/cpu-cooling-api.txt new file mode 100644 index 000..3720341 --- /dev/null +++ b/Documentation/thermal/cpu-cooling-api.txt @@ -0,0 +1,60 @@ +CPU cooling APIs How To +=== + +Written by Amit Daniel Kachhap amit.kach...@linaro.org + +Updated: 9 March 2012 + +Copyright (c) 2011 Samsung Electronics Co., Ltd(http://www.samsung.com) + +0. Introduction + +The generic cpu cooling(freq clipping, cpuhotplug) provides +registration/unregistration APIs to the caller. The binding of the cooling +devices to the trip point is left for the user. The registration APIs returns +the cooling device pointer. + +1. cpu cooling APIs + +1.1 cpufreq registration/unregistration APIs +1.1.1 struct thermal_cooling_device *cpufreq_cooling_register( + struct freq_clip_table *tab_ptr, unsigned int tab_size, + const struct cpumask *mask_val) + +This interface function registers the cpufreq cooling device with the name +thermal-cpufreq-%x. This api can support multiple instances of cpufreq +cooling devices. + +tab_ptr: The table containing the maximum value of frequency to be clipped +for each cooling state. + .freq_clip_max: Value of frequency to be clipped for each allowed +cpus. +tab_size: the total number of cpufreq cooling states. +mask_val: all the allowed cpu's where frequency clipping can happen. + +1.1.2 void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev) + +This interface function unregisters the thermal-cpufreq-%x cooling device. + +cdev: Cooling device pointer which has to be unregistered. + + +2. CPU cooling action notifier interface + +2.1 int cputherm_register_notifier(struct notifier_block *nb, + unsigned int list) + +This interface registers a driver with cpu cooling layer. The driver will +be notified when any cpu cooling action is called. + +nb: notifier function to register +list: CPUFREQ_COOLING_TYPE or CPUHOTPLUG_COOLING_TYPE + +2.2 int cputherm_unregister_notifier(struct notifier_block *nb, + unsigned int list) + +This interface registers a driver with cpu cooling layer. The driver will +be notified when any cpu cooling action is called. + +nb: notifier function to register +list: CPUFREQ_COOLING_TYPE or CPUHOTPLUG_COOLING_TYPE diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index 514a691..d9c529f 100644 --- a/drivers/thermal/Kconfig +++ b/drivers/thermal/Kconfig @@ -19,6 +19,17 @@ config THERMAL_HWMON depends on HWMON=y || HWMON=THERMAL default y +config CPU_THERMAL + bool generic cpu cooling support + depends on THERMAL CPU_FREQ + help + This implements the generic cpu cooling mechanism through frequency + reduction, cpu hotplug and any other ways of reducing temperature. An + ACPI version of this already exists(drivers/acpi/processor_thermal.c). + This will be useful for platforms using the generic thermal interface + and not the ACPI interface. + If you want this support, you should say Y or M here. + config SPEAR_THERMAL bool SPEAr thermal sensor driver depends on THERMAL diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile index a9fff0b..30c456c 100644 --- a/drivers/thermal/Makefile +++ b/drivers/thermal/Makefile @@ -3,4 +3,5 @@ # obj-$(CONFIG_THERMAL) += thermal_sys.o -obj-$(CONFIG_SPEAR_THERMAL)+= spear_thermal.o \ No newline at end of file +obj-$(CONFIG_CPU_THERMAL) += cpu_cooling.o +obj-$(CONFIG_SPEAR_THERMAL)+= spear_thermal.o diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c new file
