linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/6] ACPI: thermal: Migrate cpufreq cooling to generic cpu_cooling layer
@ 2014-05-29  8:15 Amit Daniel Kachhap
  2014-05-29  8:15 ` [PATCH v1 1/6] thermal: cpu_cooling: Fix the notification mechanism by using per cpu structure Amit Daniel Kachhap
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Amit Daniel Kachhap @ 2014-05-29  8:15 UTC (permalink / raw)
  To: linux-pm, linux-acpi
  Cc: Zhang Rui, linux-kernel, amit.kachhap, edubezval, rjw,
	linux-arm-kernel, lenb

Changes since RFC:
* Moved the cooling state notification support to thermal core from cpu cooling
  as suggested by Eduardo.
* Used per cpu notifier structure in the cpufreq cooling implementation. This
  is a fix for race condition.
* One more comment from Eduardo was to take care when multiple listeners try to
  modify the cooling state. This is however not implemented in this patch series.
  One idea is to allow only 1 set type of clients and multiple get type of clients.
  The point to consider in these case is that cooling state management logic is now
  in control of client driver and thermal governor cooling state decisions can be
  overriden.
* Fixed other minor review comments and updated documentation section.

This patch adds notification support for those clients of cpu_cooling
APIs which may want to do something extra after receiving these
cpu_cooling events shown below. The notifier structure passed is of both Set/Get type.
The notfications events can be of the following type,

1. COOLING_SET_STATE_PRE
2. COOLING_SET_STATE_POST
3. COOLING_GET_CUR_STATE
4. COOLING_GET_MAX_STATE

The advantages of these notfications is to differentiate between different
P states in the cpufreq table and the cooling states. The clients of these
events may group few P states into 1 cooling states as done by ACPI .
Also some more cooling states can be enabled when the maximum of P state is
reached. In case of ACPI processor throttling are enabled when minimum
P-state is reached. Post notification events can be used for those cases.

All these changes are tested in samsung arndale(exynos5250) with Linaro ACPI
kernel along with few tweaks. However only cpu frequency scaling down is tested
for thermal cooling and throttling is untested as this feature is not supported
in H/W. But since the functionality is not modified so expecting the changes to
work in ACPI based platform also.  

Amit Daniel Kachhap (6):
  thermal: cpu_cooling: Fix the notification mechanism by using per cpu
    structure
  thermal: cpu_cooling: Support passing driver private data.
  thermal: thermal-core: Add notifications support for the cooling
    states
  thermal: cpu_cooling: Add support to find up/low frequency levels.
  thermal: thermal_core: Remove the max cooling limit check in
    registration
  ACPI: thermal: processor: Use the generic cpufreq infrastructure


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v1 1/6] thermal: cpu_cooling: Fix the notification mechanism by using per cpu structure
  2014-05-29  8:15 [PATCH v1 0/6] ACPI: thermal: Migrate cpufreq cooling to generic cpu_cooling layer Amit Daniel Kachhap
@ 2014-05-29  8:15 ` Amit Daniel Kachhap
  2014-05-29  8:15 ` [PATCH v1 2/6] thermal: cpu_cooling: Support passing driver private data Amit Daniel Kachhap
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Amit Daniel Kachhap @ 2014-05-29  8:15 UTC (permalink / raw)
  To: linux-pm, linux-acpi
  Cc: Zhang Rui, linux-kernel, amit.kachhap, edubezval, rjw,
	linux-arm-kernel, lenb

Currently the notification data is supplied to the thermal notifiers using
a single global pointer. This method will have race condition if cpu cooling
interfaces are used by more than 1 clients (more than 1 cdev) to cause cpufreq
clipping. Also the notifier data is presented as per cpu cpufreq policy
structure. This was needed as same cpu is used later in thermal cpufreq handler
function.

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/thermal/cpu_cooling.c |   38 +++++++++++++++-----------------------
 1 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 4246262..16388b0 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -58,7 +58,7 @@ static unsigned int cpufreq_dev_count;
 
 /* notify_table passes value to the CPUFREQ_ADJUST callback function. */
 #define NOTIFY_INVALID NULL
-static struct cpufreq_cooling_device *notify_device;
+static DEFINE_PER_CPU(struct cpufreq_cooling_device *, notify_device);
 
 /**
  * get_idr - function to get a unique id.
@@ -98,23 +98,6 @@ static void release_idr(struct idr *idr, int id)
 
 /* Below code defines functions to be used for cpufreq as cooling device */
 
-/**
- * is_cpufreq_valid - function to check frequency transitioning capability.
- * @cpu: cpu for which check is needed.
- *
- * This function will check the current state of the system if
- * it is capable of changing the frequency for a given @cpu.
- *
- * Return: 0 if the system is not currently capable of changing
- * the frequency of given cpu. !0 in case the frequency is changeable.
- */
-static int is_cpufreq_valid(int cpu)
-{
-	struct cpufreq_policy policy;
-
-	return !cpufreq_get_policy(&policy, cpu);
-}
-
 enum cpufreq_cooling_property {
 	GET_LEVEL,
 	GET_FREQ,
@@ -294,11 +277,18 @@ static int cpufreq_apply_cooling(struct cpufreq_cooling_device *cpufreq_device,
 
 	cpufreq_device->cpufreq_state = cooling_state;
 	cpufreq_device->cpufreq_val = clip_freq;
-	notify_device = cpufreq_device;
 
 	for_each_cpu(cpuid, mask) {
-		if (is_cpufreq_valid(cpuid))
+		if (cpufreq_get_policy(&policy, cpu) == 0) {
+			/*
+			 * Store the notification data in the cpu pointed by
+			 * cpufreq policy structure. This is needed as this cpu
+			 * will be used later to fetch correct data pointer.
+			 */
+			per_cpu(notify_device, policy.cpu) = cpufreq_device;
 			cpufreq_update_policy(cpuid);
+			per_cpu(notify_device, policy.cpu) = NOTIFY_INVALID;
+		}
 	}
 
 	notify_device = NOTIFY_INVALID;
@@ -323,12 +313,14 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb,
 {
 	struct cpufreq_policy *policy = data;
 	unsigned long max_freq = 0;
+	struct cpufreq_cooling_device *cpufreq_device
+				= per_cpu(notify_device, policy->cpu);
 
-	if (event != CPUFREQ_ADJUST || notify_device == NOTIFY_INVALID)
+	if (event != CPUFREQ_ADJUST || cpufreq_device == NOTIFY_INVALID)
 		return 0;
 
-	if (cpumask_test_cpu(policy->cpu, &notify_device->allowed_cpus))
-		max_freq = notify_device->cpufreq_val;
+	if (cpumask_test_cpu(policy->cpu, &cpufreq_device->allowed_cpus))
+		max_freq = cpufreq_device->cpufreq_val;
 	else
 		return 0;
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 2/6] thermal: cpu_cooling: Support passing driver private data.
  2014-05-29  8:15 [PATCH v1 0/6] ACPI: thermal: Migrate cpufreq cooling to generic cpu_cooling layer Amit Daniel Kachhap
  2014-05-29  8:15 ` [PATCH v1 1/6] thermal: cpu_cooling: Fix the notification mechanism by using per cpu structure Amit Daniel Kachhap
@ 2014-05-29  8:15 ` Amit Daniel Kachhap
  2014-05-29 12:06   ` Javi Merino
  2014-05-29  8:15 ` [PATCH v1 3/6] thermal: thermal-core: Add notifications support for the cooling states Amit Daniel Kachhap
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Amit Daniel Kachhap @ 2014-05-29  8:15 UTC (permalink / raw)
  To: linux-pm, linux-acpi
  Cc: Zhang Rui, linux-kernel, amit.kachhap, edubezval, rjw,
	linux-arm-kernel, lenb

This patch allows the caller of cpufreq cooling APIs to register along
with their driver data which will be useful while receiving any cooling states
notifications.
This patch is in preparation to add notfication support for cpufrequency
cooling changes. This change also removes the unnecessary exposing of
internal housekeeping structure via thermal_cooling_device->devdata to the
users of cpufreq cooling APIs. As part of this implmentation, this private local
structure (cpufreq_dev) is now stored in a list and scanned for each call to
cpu cooling interfaces.

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 Documentation/thermal/cpu-cooling-api.txt          |    3 +-
 drivers/thermal/cpu_cooling.c                      |   89 ++++++++++++++++----
 drivers/thermal/db8500_cpufreq_cooling.c           |    2 +-
 drivers/thermal/samsung/exynos_thermal_common.c    |    2 +-
 drivers/thermal/ti-soc-thermal/ti-thermal-common.c |    2 +-
 include/linux/cpu_cooling.h                        |    5 +-
 6 files changed, 80 insertions(+), 23 deletions(-)

diff --git a/Documentation/thermal/cpu-cooling-api.txt b/Documentation/thermal/cpu-cooling-api.txt
index fca24c9..aaa07c6 100644
--- a/Documentation/thermal/cpu-cooling-api.txt
+++ b/Documentation/thermal/cpu-cooling-api.txt
@@ -17,13 +17,14 @@ the user. The registration APIs returns the cooling device pointer.
 
 1.1 cpufreq registration/unregistration APIs
 1.1.1 struct thermal_cooling_device *cpufreq_cooling_register(
-	struct cpumask *clip_cpus)
+	struct cpumask *clip_cpus, void *devdata)
 
     This interface function registers the cpufreq cooling device with the name
     "thermal-cpufreq-%x". This api can support multiple instances of cpufreq
     cooling devices.
 
    clip_cpus: cpumask of cpus where the frequency constraints will happen.
+   devdata: driver private data pointer.
 
 1.1.2 void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
 
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 16388b0..6d145d5 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -50,16 +50,18 @@ struct cpufreq_cooling_device {
 	unsigned int cpufreq_state;
 	unsigned int cpufreq_val;
 	struct cpumask allowed_cpus;
+	struct list_head node;
 };
 static DEFINE_IDR(cpufreq_idr);
 static DEFINE_MUTEX(cooling_cpufreq_lock);
 
-static unsigned int cpufreq_dev_count;
-
 /* notify_table passes value to the CPUFREQ_ADJUST callback function. */
 #define NOTIFY_INVALID NULL
 static DEFINE_PER_CPU(struct cpufreq_cooling_device *, notify_device);
 
+/* A list to hold all the cpufreq cooling devices registered */
+static LIST_HEAD(cpufreq_cooling_list);
+
 /**
  * get_idr - function to get a unique id.
  * @idr: struct idr * handle used to create a id.
@@ -98,6 +100,26 @@ static void release_idr(struct idr *idr, int id)
 
 /* Below code defines functions to be used for cpufreq as cooling device */
 
+/**
+ * cpufreq_cooling_get_info - function to cpufreq_dev for the corresponding cdev
+ * @cdev: pointer of the cooling device
+ *
+ * This function will loop through the cpufreq_cooling_device list and
+ * return the matching device
+ *
+ * Return: cpufreq_cooling_device if matched with the cdev or NULL if not
+ * matched.
+ */
+static inline struct cpufreq_cooling_device *
+cpufreq_cooling_get_info(struct thermal_cooling_device *cdev)
+{
+	struct cpufreq_cooling_device *cpufreq_dev = NULL;
+	list_for_each_entry(cpufreq_dev, &cpufreq_cooling_list, node)
+		if (cpufreq_dev->cool_dev == cdev)
+			break;
+	return cpufreq_dev;
+}
+
 enum cpufreq_cooling_property {
 	GET_LEVEL,
 	GET_FREQ,
@@ -349,12 +371,21 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb,
 static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
 				 unsigned long *state)
 {
-	struct cpufreq_cooling_device *cpufreq_device = cdev->devdata;
-	struct cpumask *mask = &cpufreq_device->allowed_cpus;
+	struct cpufreq_cooling_device *cpufreq_device = NULL;
+	struct cpumask *mask = NULL;
 	unsigned int cpu;
 	unsigned int count = 0;
 	int ret;
 
+	mutex_lock(&cooling_cpufreq_lock);
+	cpufreq_device = cpufreq_cooling_get_info(cdev);
+	mutex_unlock(&cooling_cpufreq_lock);
+	if (!cpufreq_device) {
+		/* Cooling device pointer not found */
+		return -EINVAL;
+	}
+
+	mask = &cpufreq_device->allowed_cpus;
 	cpu = cpumask_any(mask);
 
 	ret = get_property(cpu, 0, &count, GET_MAXL);
@@ -378,7 +409,15 @@ static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
 static int cpufreq_get_cur_state(struct thermal_cooling_device *cdev,
 				 unsigned long *state)
 {
-	struct cpufreq_cooling_device *cpufreq_device = cdev->devdata;
+	struct cpufreq_cooling_device *cpufreq_device = NULL;
+
+	mutex_lock(&cooling_cpufreq_lock);
+	cpufreq_device = cpufreq_cooling_get_info(cdev);
+	mutex_unlock(&cooling_cpufreq_lock);
+	if (!cpufreq_device) {
+		/* Cooling device pointer not found */
+		return -EINVAL;
+	}
 
 	*state = cpufreq_device->cpufreq_state;
 
@@ -398,7 +437,15 @@ static int cpufreq_get_cur_state(struct thermal_cooling_device *cdev,
 static int cpufreq_set_cur_state(struct thermal_cooling_device *cdev,
 				 unsigned long state)
 {
-	struct cpufreq_cooling_device *cpufreq_device = cdev->devdata;
+	struct cpufreq_cooling_device *cpufreq_device = NULL;
+
+	mutex_lock(&cooling_cpufreq_lock);
+	cpufreq_device = cpufreq_cooling_get_info(cdev);
+	mutex_unlock(&cooling_cpufreq_lock);
+	if (!cpufreq_device) {
+		/* Cooling device pointer not found */
+		return -EINVAL;
+	}
 
 	return cpufreq_apply_cooling(cpufreq_device, state);
 }
@@ -419,6 +466,7 @@ static struct notifier_block thermal_cpufreq_notifier_block = {
  * __cpufreq_cooling_register - helper function to create cpufreq cooling device
  * @np: a valid struct device_node to the cooling device device tree node
  * @clip_cpus: cpumask of cpus where the frequency constraints will happen.
+ * @devdata: driver data pointer
  *
  * This interface function registers the cpufreq cooling device with the name
  * "thermal-cpufreq-%x". This api can support multiple instances of cpufreq
@@ -430,7 +478,7 @@ static struct notifier_block thermal_cpufreq_notifier_block = {
  */
 static struct thermal_cooling_device *
 __cpufreq_cooling_register(struct device_node *np,
-			   const struct cpumask *clip_cpus)
+			   const struct cpumask *clip_cpus, void *devdata)
 {
 	struct thermal_cooling_device *cool_dev;
 	struct cpufreq_cooling_device *cpufreq_dev = NULL;
@@ -469,7 +517,7 @@ __cpufreq_cooling_register(struct device_node *np,
 	snprintf(dev_name, sizeof(dev_name), "thermal-cpufreq-%d",
 		 cpufreq_dev->id);
 
-	cool_dev = thermal_of_cooling_device_register(np, dev_name, cpufreq_dev,
+	cool_dev = thermal_of_cooling_device_register(np, dev_name, devdata,
 						      &cpufreq_cooling_ops);
 	if (IS_ERR(cool_dev)) {
 		release_idr(&cpufreq_idr, cpufreq_dev->id);
@@ -481,10 +529,11 @@ __cpufreq_cooling_register(struct device_node *np,
 	mutex_lock(&cooling_cpufreq_lock);
 
 	/* Register the notifier for first cpufreq cooling device */
-	if (cpufreq_dev_count == 0)
+	if (list_empty(&cpufreq_cooling_list))
 		cpufreq_register_notifier(&thermal_cpufreq_notifier_block,
 					  CPUFREQ_POLICY_NOTIFIER);
-	cpufreq_dev_count++;
+
+	list_add(&cpufreq_dev->node, &cpufreq_cooling_list);
 
 	mutex_unlock(&cooling_cpufreq_lock);
 
@@ -494,6 +543,7 @@ __cpufreq_cooling_register(struct device_node *np,
 /**
  * cpufreq_cooling_register - function to create cpufreq cooling device.
  * @clip_cpus: cpumask of cpus where the frequency constraints will happen.
+ * @devdata: driver data pointer
  *
  * This interface function registers the cpufreq cooling device with the name
  * "thermal-cpufreq-%x". This api can support multiple instances of cpufreq
@@ -503,9 +553,9 @@ __cpufreq_cooling_register(struct device_node *np,
  * on failure, it returns a corresponding ERR_PTR().
  */
 struct thermal_cooling_device *
-cpufreq_cooling_register(const struct cpumask *clip_cpus)
+cpufreq_cooling_register(const struct cpumask *clip_cpus, void *devdata)
 {
-	return __cpufreq_cooling_register(NULL, clip_cpus);
+	return __cpufreq_cooling_register(NULL, clip_cpus, devdata);
 }
 EXPORT_SYMBOL_GPL(cpufreq_cooling_register);
 
@@ -529,7 +579,7 @@ of_cpufreq_cooling_register(struct device_node *np,
 	if (!np)
 		return ERR_PTR(-EINVAL);
 
-	return __cpufreq_cooling_register(np, clip_cpus);
+	return __cpufreq_cooling_register(np, clip_cpus, NULL);
 }
 EXPORT_SYMBOL_GPL(of_cpufreq_cooling_register);
 
@@ -546,17 +596,22 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
 	if (!cdev)
 		return;
 
-	cpufreq_dev = cdev->devdata;
 	mutex_lock(&cooling_cpufreq_lock);
-	cpufreq_dev_count--;
+	cpufreq_dev = cpufreq_cooling_get_info(cdev);
+	if (!cpufreq_dev) {
+		/* Cooling device pointer not found */
+		mutex_unlock(&cooling_cpufreq_lock);
+		return;
+	}
+	thermal_cooling_device_unregister(cpufreq_dev->cool_dev);
+	list_del(&cpufreq_dev->node);
 
 	/* Unregister the notifier for the last cpufreq cooling device */
-	if (cpufreq_dev_count == 0)
+	if (list_empty(&cpufreq_cooling_list))
 		cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
 					    CPUFREQ_POLICY_NOTIFIER);
 	mutex_unlock(&cooling_cpufreq_lock);
 
-	thermal_cooling_device_unregister(cpufreq_dev->cool_dev);
 	release_idr(&cpufreq_idr, cpufreq_dev->id);
 	kfree(cpufreq_dev);
 }
diff --git a/drivers/thermal/db8500_cpufreq_cooling.c b/drivers/thermal/db8500_cpufreq_cooling.c
index 786d192..abb9bdd 100644
--- a/drivers/thermal/db8500_cpufreq_cooling.c
+++ b/drivers/thermal/db8500_cpufreq_cooling.c
@@ -35,7 +35,7 @@ static int db8500_cpufreq_cooling_probe(struct platform_device *pdev)
 		return -EPROBE_DEFER;
 
 	cpumask_set_cpu(0, &mask_val);
-	cdev = cpufreq_cooling_register(&mask_val);
+	cdev = cpufreq_cooling_register(&mask_val, NULL);
 
 	if (IS_ERR(cdev)) {
 		dev_err(&pdev->dev, "Failed to register cooling device\n");
diff --git a/drivers/thermal/samsung/exynos_thermal_common.c b/drivers/thermal/samsung/exynos_thermal_common.c
index 3f5ad25..a7306fa 100644
--- a/drivers/thermal/samsung/exynos_thermal_common.c
+++ b/drivers/thermal/samsung/exynos_thermal_common.c
@@ -369,7 +369,7 @@ int exynos_register_thermal(struct thermal_sensor_conf *sensor_conf)
 	if (sensor_conf->cooling_data.freq_clip_count > 0) {
 		cpumask_set_cpu(0, &mask_val);
 		th_zone->cool_dev[th_zone->cool_dev_size] =
-					cpufreq_cooling_register(&mask_val);
+				cpufreq_cooling_register(&mask_val, NULL);
 		if (IS_ERR(th_zone->cool_dev[th_zone->cool_dev_size])) {
 			dev_err(sensor_conf->dev,
 				"Failed to register cpufreq cooling device\n");
diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
index 9eec26d..7809db6 100644
--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
@@ -409,7 +409,7 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
 	}
 
 	/* Register cooling device */
-	data->cool_dev = cpufreq_cooling_register(cpu_present_mask);
+	data->cool_dev = cpufreq_cooling_register(cpu_present_mask, NULL);
 	if (IS_ERR(data->cool_dev)) {
 		dev_err(bgp->dev,
 			"Failed to register cpufreq cooling device\n");
diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
index c303d38..aaef7d8 100644
--- a/include/linux/cpu_cooling.h
+++ b/include/linux/cpu_cooling.h
@@ -32,9 +32,10 @@
 /**
  * cpufreq_cooling_register - function to create cpufreq cooling device.
  * @clip_cpus: cpumask of cpus where the frequency constraints will happen
+ * @devdata: driver data pointer
  */
 struct thermal_cooling_device *
-cpufreq_cooling_register(const struct cpumask *clip_cpus);
+cpufreq_cooling_register(const struct cpumask *clip_cpus, void *devdata);
 
 /**
  * of_cpufreq_cooling_register - create cpufreq cooling device based on DT.
@@ -63,7 +64,7 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev);
 unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq);
 #else /* !CONFIG_CPU_THERMAL */
 static inline struct thermal_cooling_device *
-cpufreq_cooling_register(const struct cpumask *clip_cpus)
+cpufreq_cooling_register(const struct cpumask *clip_cpus, void *devdata)
 {
 	return NULL;
 }
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 3/6] thermal: thermal-core: Add notifications support for the cooling states
  2014-05-29  8:15 [PATCH v1 0/6] ACPI: thermal: Migrate cpufreq cooling to generic cpu_cooling layer Amit Daniel Kachhap
  2014-05-29  8:15 ` [PATCH v1 1/6] thermal: cpu_cooling: Fix the notification mechanism by using per cpu structure Amit Daniel Kachhap
  2014-05-29  8:15 ` [PATCH v1 2/6] thermal: cpu_cooling: Support passing driver private data Amit Daniel Kachhap
@ 2014-05-29  8:15 ` Amit Daniel Kachhap
  2014-05-29 12:24   ` Javi Merino
  2014-05-29  8:15 ` [PATCH v1 4/6] thermal: cpu_cooling: Add support to find up/low frequency levels Amit Daniel Kachhap
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Amit Daniel Kachhap @ 2014-05-29  8:15 UTC (permalink / raw)
  To: linux-pm, linux-acpi
  Cc: Zhang Rui, linux-kernel, amit.kachhap, edubezval, rjw,
	linux-arm-kernel, lenb

This patch adds notification infrastructure for any requests related to cooling
states. The notifier structure passed is of both Get/Set type. So the receiver
of these can sense the new/cur/max cooling state as decided by thermal governor.
In addition to that it can also override the cooling state and may do something
interesting after receiving these CPU cooling events such as masking some
states, enabling some extra conditional states or perform any extra operation
for aggressive thermal cooling.
The notfications events can be of type,

1. COOLING_SET_STATE_PRE
2. COOLING_SET_STATE_POST
3. COOLING_GET_CUR_STATE
4. COOLING_GET_MAX_STATE

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 Documentation/thermal/sysfs-api.txt |   21 +++++++++++
 drivers/thermal/thermal_core.c      |   69 ++++++++++++++++++++++++++++++++++-
 include/linux/thermal.h             |   21 +++++++++++
 3 files changed, 109 insertions(+), 2 deletions(-)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 87519cb..5f45e03 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -92,6 +92,27 @@ temperature) and throttle appropriate devices.
     It deletes the corresponding entry form /sys/class/thermal folder and
     unbind itself from all the thermal zone devices using it.
 
+1.2.3 int thermal_cooling_register_notifier(struct notifier_block *nb)
+
+    This interface function registers the client notifier handler. The notifier
+    handler can use this to monitor or update any cooling state requests.
+    nb: notifier structure containing client notifier handler.
+
+1.2.4 int thermal_cooling_unregister_notifier(struct notifier_block *nb)
+
+    This interface function unregisters the client notifier handler.
+    nb: notifier structure containing client notifier handler.
+
+1.2.5 int thermal_cooling_notify_states(struct thermal_cooling_status *request,enum cooling_state_ops op)
+
+    This interface function invokes the earlier registered cooling states handler.
+    request: holds the relevant cooling state value.
+	.cur_state: current cooling state.
+	.new_state: new cooling state to be set.
+	.max_state: max cooling state.
+	.devdata: driver private data pointer.
+    op: describes various operation supported.
+
 1.3 interface for binding a thermal zone device with a thermal cooling device
 1.3.1 int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
 	int trip, struct thermal_cooling_device *cdev,
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 71b0ec0..1a60f83 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -52,6 +52,8 @@ static DEFINE_MUTEX(thermal_idr_lock);
 static LIST_HEAD(thermal_tz_list);
 static LIST_HEAD(thermal_cdev_list);
 static LIST_HEAD(thermal_governor_list);
+/* Notfier list to validates/updates the cpufreq cooling states */
+static BLOCKING_NOTIFIER_HEAD(cooling_state_notifier_list);
 
 static DEFINE_MUTEX(thermal_list_lock);
 static DEFINE_MUTEX(thermal_governor_lock);
@@ -1073,8 +1075,71 @@ static struct class thermal_class = {
 };
 
 /**
- * __thermal_cooling_device_register() - register a new thermal cooling device
- * @np:		a pointer to a device tree node.
+ * thermal_cooling_notify_states - Invoke the necessary cooling states handler.
+ * @request: holds the relevant cooling state value. say if the cooling state
+ * operation is of type COOLING_GET_MAX_STATE, then request holds
+ * the current max cooling state value.
+ * @op: different operations supported
+ *
+ * This API allows the registered user to recieve the different cooling
+ * notifications like current state, max state and set state.
+ *
+ * Return: 0 (success)
+ */
+int thermal_cooling_notify_states(struct thermal_cooling_status *request,
+				enum cooling_state_ops op)
+{
+	/* Invoke the notifiers which have registered for this state change */
+	if (op == COOLING_SET_STATE_PRE ||
+		op == COOLING_SET_STATE_POST ||
+		op == COOLING_GET_MAX_STATE ||
+		op == COOLING_GET_CUR_STATE) {
+		blocking_notifier_call_chain(
+			&cooling_state_notifier_list, op, request);
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(thermal_cooling_notify_states);
+
+/**
+ * thermal_cooling_register_notifier - registers a notifier with thermal cooling.
+ * @nb:	notifier function to register.
+ *
+ * Add a driver to receive all cooling notifications like current state,
+ * max state and set state. The drivers after reading the events can perform
+ * some mapping like grouping some P states into 1 cooling state.
+ *
+ * Return: 0 (success)
+ */
+int thermal_cooling_register_notifier(struct notifier_block *nb)
+{
+	int ret = 0;
+	ret = blocking_notifier_chain_register(
+				&cooling_state_notifier_list, nb);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(thermal_cooling_register_notifier);
+
+/**
+ * thermal_cooling_unregister_notifier - unregisters a notifier with thermal
+ * cooling.
+ * @nb:	notifier function to unregister.
+ *
+ * Removes a driver to receive further cooling notifications.
+ *
+ * Return: 0 (success)
+ */
+int thermal_cooling_unregister_notifier(struct notifier_block *nb)
+{
+	int ret = 0;
+	ret = blocking_notifier_chain_unregister(
+				&cooling_state_notifier_list, nb);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(thermal_cooling_unregister_notifier);
+
+/**
+ * thermal_cooling_device_register() - register a new thermal cooling device
  * @type:	the thermal cooling device type.
  * @devdata:	device private data.
  * @ops:		standard thermal cooling devices callbacks.
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index f7e11c7..3fb1b92 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -106,6 +106,21 @@ enum {
 };
 #define THERMAL_GENL_CMD_MAX (__THERMAL_GENL_CMD_MAX - 1)
 
+/* Cooling states supported operations */
+enum cooling_state_ops {
+	COOLING_SET_STATE_PRE,
+	COOLING_SET_STATE_POST,
+	COOLING_GET_CUR_STATE,
+	COOLING_GET_MAX_STATE,
+};
+
+struct thermal_cooling_status {
+	unsigned long cur_state;
+	unsigned long new_state;
+	unsigned long max_state;
+	void *devdata;
+};
+
 struct thermal_zone_device_ops {
 	int (*bind) (struct thermal_zone_device *,
 		     struct thermal_cooling_device *);
@@ -285,6 +300,12 @@ struct thermal_cooling_device *
 thermal_of_cooling_device_register(struct device_node *np, char *, void *,
 				   const struct thermal_cooling_device_ops *);
 void thermal_cooling_device_unregister(struct thermal_cooling_device *);
+
+int thermal_cooling_notify_states(struct thermal_cooling_status *request,
+				enum cooling_state_ops op);
+int thermal_cooling_register_notifier(struct notifier_block *nb);
+int thermal_cooling_unregister_notifier(struct notifier_block *nb);
+
 struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name);
 int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp);
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 4/6] thermal: cpu_cooling: Add support to find up/low frequency levels.
  2014-05-29  8:15 [PATCH v1 0/6] ACPI: thermal: Migrate cpufreq cooling to generic cpu_cooling layer Amit Daniel Kachhap
                   ` (2 preceding siblings ...)
  2014-05-29  8:15 ` [PATCH v1 3/6] thermal: thermal-core: Add notifications support for the cooling states Amit Daniel Kachhap
@ 2014-05-29  8:15 ` Amit Daniel Kachhap
  2014-05-29  8:15 ` [PATCH v1 5/6] thermal: thermal_core: Remove the max cooling limit check in registration Amit Daniel Kachhap
  2014-05-29  8:15 ` [PATCH v1 6/6] ACPI: thermal: processor: Use the generic cpufreq infrastructure Amit Daniel Kachhap
  5 siblings, 0 replies; 17+ messages in thread
From: Amit Daniel Kachhap @ 2014-05-29  8:15 UTC (permalink / raw)
  To: linux-pm, linux-acpi
  Cc: Zhang Rui, linux-kernel, amit.kachhap, edubezval, rjw,
	linux-arm-kernel, lenb

This patch adds support to get P state ceil/floor level for nearest frequency.
This will be used for consolidating ACPI cpufreq cooling via the generic cpu
cooling framework.

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 Documentation/thermal/cpu-cooling-api.txt       |   15 +++++++++
 drivers/thermal/cpu_cooling.c                   |   39 +++++++++++++++++------
 drivers/thermal/samsung/exynos_thermal_common.c |    3 +-
 include/linux/cpu_cooling.h                     |   13 ++++++-
 4 files changed, 57 insertions(+), 13 deletions(-)

diff --git a/Documentation/thermal/cpu-cooling-api.txt b/Documentation/thermal/cpu-cooling-api.txt
index aaa07c6..a1f93b8 100644
--- a/Documentation/thermal/cpu-cooling-api.txt
+++ b/Documentation/thermal/cpu-cooling-api.txt
@@ -31,3 +31,18 @@ the user. The registration APIs returns the cooling device pointer.
     This interface function unregisters the "thermal-cpufreq-%x" cooling device.
 
     cdev: Cooling device pointer which has to be unregistered.
+
+1.1.3 unsigned long cpufreq_cooling_get_level(unsigned int cpu,
+			unsigned int freq, enum cpufreq_cooling_property)
+
+    This interface gets the frequency level for the absolute frequency by
+    matching in grequency table.
+
+    cpu: cpu for which the frequency level is expected.
+    freq: absolute input frequency as found in cpu freq table.
+    cpufreq_cooling_property:
+	.GET_LEVEL_CEIL: returns the ceil of the frequency level.
+	.GET_LEVEL_FLOOR: returns the floor of the frequency level.
+	.GET_LEVEL_EXACT: returns the exact frequency level if found.
+	.GET_MAXL: returns the max frequency level.
+
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 6d145d5..4ce8803 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -120,11 +120,7 @@ cpufreq_cooling_get_info(struct thermal_cooling_device *cdev)
 	return cpufreq_dev;
 }
 
-enum cpufreq_cooling_property {
-	GET_LEVEL,
-	GET_FREQ,
-	GET_MAXL,
-};
+#define GET_FREQ (sizeof(enum cpufreq_cooling_property) + 1)
 
 /**
  * get_property - fetch a property of interest for a give cpu.
@@ -207,15 +203,37 @@ static int get_property(unsigned int cpu, unsigned long input,
 		/* now we have a valid frequency entry */
 		freq = table[i].frequency;
 
-		if (property == GET_LEVEL && (unsigned int)input == freq) {
+		if (property == GET_LEVEL_EXACT &&
+						(unsigned int)input == freq) {
 			/* get level by frequency */
 			*output = descend ? j : (max_level - j);
 			return 0;
-		}
-		if (property == GET_FREQ && level == j) {
+		} else if (property == GET_FREQ && level == j) {
 			/* get frequency by level */
 			*output = freq;
 			return 0;
+		} else if (property == GET_LEVEL_FLOOR) {
+			/* get minimum possible level by frequency */
+			if (descend && freq <= input) {
+				*output = j;
+				return 0;
+			} else if (!descend) {
+				if (freq <= input)
+					*output = (max_level - j);
+				else
+					return 0;
+			}
+		} else if (property == GET_LEVEL_CEIL) {
+			/* get maximum possible level by frequency */
+			if (!descend && freq >= input) {
+				*output = (max_level - j);
+				return 0;
+			} else if (descend) {
+				if (freq >= input)
+					*output = j;
+				else
+					return 0;
+			}
 		}
 		j++;
 	}
@@ -234,11 +252,12 @@ static int get_property(unsigned int cpu, unsigned long input,
  * Return: The matched cooling level on success or THERMAL_CSTATE_INVALID
  * otherwise.
  */
-unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq)
+unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq,
+			enum cpufreq_cooling_property property)
 {
 	unsigned int val;
 
-	if (get_property(cpu, (unsigned long)freq, &val, GET_LEVEL))
+	if (get_property(cpu, (unsigned long)freq, &val, property))
 		return THERMAL_CSTATE_INVALID;
 
 	return (unsigned long)val;
diff --git a/drivers/thermal/samsung/exynos_thermal_common.c b/drivers/thermal/samsung/exynos_thermal_common.c
index a7306fa..aa4696b 100644
--- a/drivers/thermal/samsung/exynos_thermal_common.c
+++ b/drivers/thermal/samsung/exynos_thermal_common.c
@@ -156,7 +156,8 @@ static int exynos_bind(struct thermal_zone_device *thermal,
 	/* Bind the thermal zone to the cpufreq cooling device */
 	for (i = 0; i < tab_size; i++) {
 		clip_data = (struct freq_clip_table *)&(tab_ptr[i]);
-		level = cpufreq_cooling_get_level(0, clip_data->freq_clip_max);
+		level = cpufreq_cooling_get_level(0, clip_data->freq_clip_max,
+						GET_LEVEL_EXACT);
 		if (level == THERMAL_CSTATE_INVALID)
 			return 0;
 		switch (GET_ZONE(i)) {
diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
index aaef7d8..dba52c9 100644
--- a/include/linux/cpu_cooling.h
+++ b/include/linux/cpu_cooling.h
@@ -28,6 +28,13 @@
 #include <linux/thermal.h>
 #include <linux/cpumask.h>
 
+enum cpufreq_cooling_property {
+	GET_LEVEL_CEIL,
+	GET_LEVEL_FLOOR,
+	GET_LEVEL_EXACT,
+	GET_MAXL,
+};
+
 #ifdef CONFIG_CPU_THERMAL
 /**
  * cpufreq_cooling_register - function to create cpufreq cooling device.
@@ -61,7 +68,8 @@ of_cpufreq_cooling_register(struct device_node *np,
  */
 void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev);
 
-unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq);
+unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq,
+					enum cpufreq_cooling_property);
 #else /* !CONFIG_CPU_THERMAL */
 static inline struct thermal_cooling_device *
 cpufreq_cooling_register(const struct cpumask *clip_cpus, void *devdata)
@@ -80,7 +88,8 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
 	return;
 }
 static inline
-unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq)
+unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq,
+					enum cpufreq_cooling_property property)
 {
 	return THERMAL_CSTATE_INVALID;
 }
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 5/6] thermal: thermal_core: Remove the max cooling limit check in registration
  2014-05-29  8:15 [PATCH v1 0/6] ACPI: thermal: Migrate cpufreq cooling to generic cpu_cooling layer Amit Daniel Kachhap
                   ` (3 preceding siblings ...)
  2014-05-29  8:15 ` [PATCH v1 4/6] thermal: cpu_cooling: Add support to find up/low frequency levels Amit Daniel Kachhap
@ 2014-05-29  8:15 ` Amit Daniel Kachhap
  2014-05-29  8:15 ` [PATCH v1 6/6] ACPI: thermal: processor: Use the generic cpufreq infrastructure Amit Daniel Kachhap
  5 siblings, 0 replies; 17+ messages in thread
From: Amit Daniel Kachhap @ 2014-05-29  8:15 UTC (permalink / raw)
  To: linux-pm, linux-acpi
  Cc: Zhang Rui, linux-kernel, amit.kachhap, edubezval, rjw,
	linux-arm-kernel, lenb

This is required as with the addition of the cooling notifiers mechanism the
client can enable some more cooling states at a later point of time and
hence max cooling state is dynamic entity now. Say when minimum p state
is reached then ACPI specific throttling is enabled which may add some
more cooling states.

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/thermal/step_wise.c    |    2 +-
 drivers/thermal/thermal_core.c |    9 +++------
 include/linux/thermal.h        |    1 +
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
index f251521..7d65617 100644
--- a/drivers/thermal/step_wise.c
+++ b/drivers/thermal/step_wise.c
@@ -72,7 +72,7 @@ static unsigned long get_target_state(struct thermal_instance *instance,
 		}
 		break;
 	case THERMAL_TREND_RAISE_FULL:
-		if (throttle)
+		if (instance->upper != THERMAL_CSTATE_MAX && throttle)
 			next_target = instance->upper;
 		break;
 	case THERMAL_TREND_DROPPING:
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 1a60f83..743bb83 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -923,7 +923,6 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
 	struct thermal_instance *pos;
 	struct thermal_zone_device *pos1;
 	struct thermal_cooling_device *pos2;
-	unsigned long max_state;
 	int result;
 
 	if (trip >= tz->trips || (trip < 0 && trip != THERMAL_TRIPS_NONE))
@@ -941,13 +940,11 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
 	if (tz != pos1 || cdev != pos2)
 		return -EINVAL;
 
-	cdev->ops->get_max_state(cdev, &max_state);
-
-	/* lower default 0, upper default max_state */
+	/* lower default 0, upper default THERMAL_CSTATE_MAX */
 	lower = lower == THERMAL_NO_LIMIT ? 0 : lower;
-	upper = upper == THERMAL_NO_LIMIT ? max_state : upper;
+	upper = upper == THERMAL_NO_LIMIT ? THERMAL_CSTATE_MAX : upper;
 
-	if (lower > upper || upper > max_state)
+	if (lower > upper)
 		return -EINVAL;
 
 	dev =
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 3fb1b92..6cfe8c8 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -36,6 +36,7 @@
 
 /* invalid cooling state */
 #define THERMAL_CSTATE_INVALID -1UL
+#define THERMAL_CSTATE_MAX 1UL
 
 /* No upper/lower limit requirement */
 #define THERMAL_NO_LIMIT	THERMAL_CSTATE_INVALID
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 6/6] ACPI: thermal: processor: Use the generic cpufreq infrastructure
  2014-05-29  8:15 [PATCH v1 0/6] ACPI: thermal: Migrate cpufreq cooling to generic cpu_cooling layer Amit Daniel Kachhap
                   ` (4 preceding siblings ...)
  2014-05-29  8:15 ` [PATCH v1 5/6] thermal: thermal_core: Remove the max cooling limit check in registration Amit Daniel Kachhap
@ 2014-05-29  8:15 ` Amit Daniel Kachhap
  2014-05-29 12:42   ` Javi Merino
  5 siblings, 1 reply; 17+ messages in thread
From: Amit Daniel Kachhap @ 2014-05-29  8:15 UTC (permalink / raw)
  To: linux-pm, linux-acpi
  Cc: Zhang Rui, linux-kernel, amit.kachhap, edubezval, rjw,
	linux-arm-kernel, lenb

This patch upgrades the ACPI cpufreq cooling portions to use the generic
cpufreq cooling infrastructure. There should not be any functionality
related changes as the same behaviour is provided by the generic
cpufreq APIs with the notifier mechanism.

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/acpi/processor_driver.c  |    6 +-
 drivers/acpi/processor_thermal.c |  235 ++++++++++++++++++--------------------
 include/acpi/processor.h         |    3 +-
 3 files changed, 115 insertions(+), 129 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 7f70f31..10aba4a 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -36,6 +36,7 @@
 #include <linux/cpuidle.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
+#include <linux/cpu_cooling.h>
 
 #include <acpi/processor.h>
 
@@ -178,8 +179,7 @@ static int __acpi_processor_start(struct acpi_device *device)
 	if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
 		acpi_processor_power_init(pr);
 
-	pr->cdev = thermal_cooling_device_register("Processor", device,
-						   &processor_cooling_ops);
+	pr->cdev = acpi_processor_cooling_register(device);
 	if (IS_ERR(pr->cdev)) {
 		result = PTR_ERR(pr->cdev);
 		goto err_power_exit;
@@ -250,7 +250,7 @@ static int acpi_processor_stop(struct device *dev)
 	if (pr->cdev) {
 		sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
 		sysfs_remove_link(&pr->cdev->device.kobj, "device");
-		thermal_cooling_device_unregister(pr->cdev);
+		cpufreq_cooling_unregister(pr->cdev);
 		pr->cdev = NULL;
 	}
 	return 0;
diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
index e003663..9fc4a58 100644
--- a/drivers/acpi/processor_thermal.c
+++ b/drivers/acpi/processor_thermal.c
@@ -31,6 +31,7 @@
 #include <linux/init.h>
 #include <linux/cpufreq.h>
 #include <linux/acpi.h>
+#include <linux/cpu_cooling.h>
 #include <acpi/processor.h>
 #include <asm/uaccess.h>
 
@@ -53,27 +54,13 @@ ACPI_MODULE_NAME("processor_thermal");
 
 static DEFINE_PER_CPU(unsigned int, cpufreq_thermal_reduction_pctg);
 static unsigned int acpi_thermal_cpufreq_is_init = 0;
+static struct notifier_block cpufreq_cooling_notifier_block;
+static int phys_package_first_cpu(int cpu);
 
 #define reduction_pctg(cpu) \
 	per_cpu(cpufreq_thermal_reduction_pctg, phys_package_first_cpu(cpu))
 
-/*
- * Emulate "per package data" using per cpu data (which should really be
- * provided elsewhere)
- *
- * Note we can lose a CPU on cpu hotunplug, in this case we forget the state
- * temporarily. Fortunately that's not a big issue here (I hope)
- */
-static int phys_package_first_cpu(int cpu)
-{
-	int i;
-	int id = topology_physical_package_id(cpu);
-
-	for_each_online_cpu(i)
-		if (topology_physical_package_id(i) == id)
-			return i;
-	return 0;
-}
+static DEFINE_PER_CPU(struct acpi_device *, acpi_dev);
 
 static int cpu_has_cpufreq(unsigned int cpu)
 {
@@ -83,30 +70,6 @@ static int cpu_has_cpufreq(unsigned int cpu)
 	return 1;
 }
 
-static int acpi_thermal_cpufreq_notifier(struct notifier_block *nb,
-					 unsigned long event, void *data)
-{
-	struct cpufreq_policy *policy = data;
-	unsigned long max_freq = 0;
-
-	if (event != CPUFREQ_ADJUST)
-		goto out;
-
-	max_freq = (
-	    policy->cpuinfo.max_freq *
-	    (100 - reduction_pctg(policy->cpu) * 20)
-	) / 100;
-
-	cpufreq_verify_within_limits(policy, 0, max_freq);
-
-      out:
-	return 0;
-}
-
-static struct notifier_block acpi_thermal_cpufreq_notifier_block = {
-	.notifier_call = acpi_thermal_cpufreq_notifier,
-};
-
 static int cpufreq_get_max_state(unsigned int cpu)
 {
 	if (!cpu_has_cpufreq(cpu))
@@ -123,34 +86,31 @@ static int cpufreq_get_cur_state(unsigned int cpu)
 	return reduction_pctg(cpu);
 }
 
-static int cpufreq_set_cur_state(unsigned int cpu, int state)
+static int acpi_processor_freq_level(unsigned int cpu, int state)
 {
-	int i;
+	struct cpufreq_policy policy;
+	unsigned long max_freq = 0;
+	int level = 0;
 
-	if (!cpu_has_cpufreq(cpu))
+	if (!acpi_thermal_cpufreq_is_init || cpufreq_get_policy(&policy, cpu))
 		return 0;
 
 	reduction_pctg(cpu) = state;
+	max_freq = (
+	    policy.cpuinfo.max_freq *
+	    (100 - reduction_pctg(cpu) * 20)
+	) / 100;
 
-	/*
-	 * Update all the CPUs in the same package because they all
-	 * contribute to the temperature and often share the same
-	 * frequency.
-	 */
-	for_each_online_cpu(i) {
-		if (topology_physical_package_id(i) ==
-		    topology_physical_package_id(cpu))
-			cpufreq_update_policy(i);
-	}
-	return 0;
+	level =  cpufreq_cooling_get_level(phys_package_first_cpu(cpu),
+						max_freq, GET_LEVEL_FLOOR);
+	return level;
 }
 
 void acpi_thermal_cpufreq_init(void)
 {
 	int i;
 
-	i = cpufreq_register_notifier(&acpi_thermal_cpufreq_notifier_block,
-				      CPUFREQ_POLICY_NOTIFIER);
+	i = thermal_cooling_register_notifier(&cpufreq_cooling_notifier_block);
 	if (!i)
 		acpi_thermal_cpufreq_is_init = 1;
 }
@@ -158,31 +118,30 @@ void acpi_thermal_cpufreq_init(void)
 void acpi_thermal_cpufreq_exit(void)
 {
 	if (acpi_thermal_cpufreq_is_init)
-		cpufreq_unregister_notifier
-		    (&acpi_thermal_cpufreq_notifier_block,
-		     CPUFREQ_POLICY_NOTIFIER);
+		thermal_cooling_unregister_notifier(
+					&cpufreq_cooling_notifier_block);
 
 	acpi_thermal_cpufreq_is_init = 0;
 }
 
-#else				/* ! CONFIG_CPU_FREQ */
-static int cpufreq_get_max_state(unsigned int cpu)
-{
-	return 0;
-}
-
-static int cpufreq_get_cur_state(unsigned int cpu)
+/*
+ * Emulate "per package data" using per cpu data (which should really be
+ * provided elsewhere)
+ *
+ * Note we can lose a CPU on cpu hotunplug, in this case we forget the state
+ * temporarily. Fortunately that's not a big issue here (I hope)
+ */
+static int phys_package_first_cpu(int cpu)
 {
-	return 0;
-}
+	int i;
+	int id = topology_physical_package_id(cpu);
 
-static int cpufreq_set_cur_state(unsigned int cpu, int state)
-{
+	for_each_online_cpu(i)
+		if (topology_physical_package_id(i) == id)
+			return i;
 	return 0;
 }
 
-#endif
-
 /* thermal cooling device callbacks */
 static int acpi_processor_max_state(struct acpi_processor *pr)
 {
@@ -198,57 +157,22 @@ static int acpi_processor_max_state(struct acpi_processor *pr)
 
 	return max_state;
 }
-static int
-processor_get_max_state(struct thermal_cooling_device *cdev,
-			unsigned long *state)
-{
-	struct acpi_device *device = cdev->devdata;
-	struct acpi_processor *pr;
-
-	if (!device)
-		return -EINVAL;
-
-	pr = acpi_driver_data(device);
-	if (!pr)
-		return -EINVAL;
-
-	*state = acpi_processor_max_state(pr);
-	return 0;
-}
 
-static int
-processor_get_cur_state(struct thermal_cooling_device *cdev,
-			unsigned long *cur_state)
+static int acpi_processor_cur_state(struct acpi_processor *pr)
 {
-	struct acpi_device *device = cdev->devdata;
-	struct acpi_processor *pr;
-
-	if (!device)
-		return -EINVAL;
-
-	pr = acpi_driver_data(device);
-	if (!pr)
-		return -EINVAL;
-
-	*cur_state = cpufreq_get_cur_state(pr->id);
+	int cur_state = 0;
+	cur_state = cpufreq_get_cur_state(pr->id);
 	if (pr->flags.throttling)
-		*cur_state += pr->throttling.state;
-	return 0;
+		cur_state += pr->throttling.state;
+	return cur_state;
 }
 
-static int
-processor_set_cur_state(struct thermal_cooling_device *cdev,
-			unsigned long state)
+static int acpi_processor_set_cur_state(struct acpi_processor *pr,
+		struct thermal_cooling_status *cooling,	unsigned long event)
 {
-	struct acpi_device *device = cdev->devdata;
-	struct acpi_processor *pr;
-	int result = 0;
-	int max_pstate;
-
-	if (!device)
-		return -EINVAL;
+	int result = 0, level = 0;
+	int max_pstate, state = cooling->new_state;
 
-	pr = acpi_driver_data(device);
 	if (!pr)
 		return -EINVAL;
 
@@ -257,20 +181,81 @@ processor_set_cur_state(struct thermal_cooling_device *cdev,
 	if (state > acpi_processor_max_state(pr))
 		return -EINVAL;
 
-	if (state <= max_pstate) {
+	if (state <= max_pstate && event == COOLING_SET_STATE_PRE) {
 		if (pr->flags.throttling && pr->throttling.state)
 			result = acpi_processor_set_throttling(pr, 0, false);
-		cpufreq_set_cur_state(pr->id, state);
-	} else {
-		cpufreq_set_cur_state(pr->id, max_pstate);
+	} else if (state > max_pstate && event == COOLING_SET_STATE_POST) {
 		result = acpi_processor_set_throttling(pr,
 				state - max_pstate, false);
 	}
+
+	level = acpi_processor_freq_level(pr->id, state);
+	cooling->new_state = level;
+
 	return result;
 }
 
-const struct thermal_cooling_device_ops processor_cooling_ops = {
-	.get_max_state = processor_get_max_state,
-	.get_cur_state = processor_get_cur_state,
-	.set_cur_state = processor_set_cur_state,
+static int acpi_cpufreq_cooling_notifier(struct notifier_block *nb,
+					 unsigned long event, void *data)
+{
+	struct thermal_cooling_status *cooling = data;
+	struct acpi_device *device = NULL;
+	struct acpi_processor *pr;
+	int i;
+
+	for_each_online_cpu(i)
+		if (per_cpu(acpi_dev, i) == cooling->devdata) {
+			device = cooling->devdata;
+			break;
+		}
+
+	if (device == NULL)
+		return 0; /* notfier for some other client */
+
+	pr = acpi_driver_data(device);
+	switch (event) {
+	case COOLING_SET_STATE_PRE:
+	case COOLING_SET_STATE_POST:
+		acpi_processor_set_cur_state(pr, cooling, event);
+		break;
+	case COOLING_GET_MAX_STATE:
+		cooling->max_state = acpi_processor_max_state(pr);
+		break;
+	case COOLING_GET_CUR_STATE:
+		cooling->cur_state = acpi_processor_cur_state(pr);
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static struct notifier_block cpufreq_cooling_notifier_block = {
+	.notifier_call = acpi_cpufreq_cooling_notifier,
 };
+
+struct thermal_cooling_device *
+acpi_processor_cooling_register(struct acpi_device *device)
+{
+	struct thermal_cooling_device *cdev;
+	struct acpi_processor *pr = acpi_driver_data(device);
+	int cpu = phys_package_first_cpu(pr->id);
+	int i;
+	int id = topology_physical_package_id(cpu);
+	struct cpumask cpus;
+
+	for_each_online_cpu(i)
+		if (topology_physical_package_id(i) == id)
+			cpumask_set_cpu(i, &cpus);
+
+	cdev = cpufreq_cooling_register(&cpus, (void *)device);
+	per_cpu(acpi_dev, id) = device;
+	return cdev;
+}
+#else				/* ! CONFIG_CPU_FREQ */
+struct thermal_cooling_device *
+acpi_processor_cooling_register(struct acpi_device *device)
+{
+	return NULL;
+}
+#endif
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 6eb1d3c..d6e8f67 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -348,7 +348,8 @@ static inline void acpi_processor_syscore_exit(void) {}
 
 /* in processor_thermal.c */
 int acpi_processor_get_limit_info(struct acpi_processor *pr);
-extern const struct thermal_cooling_device_ops processor_cooling_ops;
+struct thermal_cooling_device *
+acpi_processor_cooling_register(struct acpi_device *device);
 #ifdef CONFIG_CPU_FREQ
 void acpi_thermal_cpufreq_init(void);
 void acpi_thermal_cpufreq_exit(void);
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/6] thermal: cpu_cooling: Support passing driver private data.
  2014-05-29  8:15 ` [PATCH v1 2/6] thermal: cpu_cooling: Support passing driver private data Amit Daniel Kachhap
@ 2014-05-29 12:06   ` Javi Merino
  2014-06-02  9:24     ` Amit Kachhap
  0 siblings, 1 reply; 17+ messages in thread
From: Javi Merino @ 2014-05-29 12:06 UTC (permalink / raw)
  To: Amit Daniel Kachhap
  Cc: linux-pm, linux-acpi, amit.kachhap, rjw, linux-kernel, edubezval,
	Zhang Rui, linux-arm-kernel, lenb

Hi Amit,

One minor comment.

On Thu, May 29, 2014 at 09:15:30AM +0100, Amit Daniel Kachhap wrote:
> This patch allows the caller of cpufreq cooling APIs to register along
> with their driver data which will be useful while receiving any cooling states
> notifications.
> This patch is in preparation to add notfication support for cpufrequency
> cooling changes. This change also removes the unnecessary exposing of
> internal housekeeping structure via thermal_cooling_device->devdata to the
> users of cpufreq cooling APIs. As part of this implmentation, this private local
> structure (cpufreq_dev) is now stored in a list and scanned for each call to
> cpu cooling interfaces.
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  Documentation/thermal/cpu-cooling-api.txt          |    3 +-
>  drivers/thermal/cpu_cooling.c                      |   89 ++++++++++++++++----
>  drivers/thermal/db8500_cpufreq_cooling.c           |    2 +-
>  drivers/thermal/samsung/exynos_thermal_common.c    |    2 +-
>  drivers/thermal/ti-soc-thermal/ti-thermal-common.c |    2 +-
>  include/linux/cpu_cooling.h                        |    5 +-
>  6 files changed, 80 insertions(+), 23 deletions(-)
> 
> diff --git a/Documentation/thermal/cpu-cooling-api.txt b/Documentation/thermal/cpu-cooling-api.txt
> index fca24c9..aaa07c6 100644
> --- a/Documentation/thermal/cpu-cooling-api.txt
> +++ b/Documentation/thermal/cpu-cooling-api.txt
> @@ -17,13 +17,14 @@ the user. The registration APIs returns the cooling device pointer.
> 
>  1.1 cpufreq registration/unregistration APIs
>  1.1.1 struct thermal_cooling_device *cpufreq_cooling_register(
> -       struct cpumask *clip_cpus)
> +       struct cpumask *clip_cpus, void *devdata)
> 
>      This interface function registers the cpufreq cooling device with the name
>      "thermal-cpufreq-%x". This api can support multiple instances of cpufreq
>      cooling devices.
> 
>     clip_cpus: cpumask of cpus where the frequency constraints will happen.
> +   devdata: driver private data pointer.
> 
>  1.1.2 void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
> 
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index 16388b0..6d145d5 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -50,16 +50,18 @@ struct cpufreq_cooling_device {
>         unsigned int cpufreq_state;
>         unsigned int cpufreq_val;
>         struct cpumask allowed_cpus;
> +       struct list_head node;
>  };
>  static DEFINE_IDR(cpufreq_idr);
>  static DEFINE_MUTEX(cooling_cpufreq_lock);
> 
> -static unsigned int cpufreq_dev_count;
> -
>  /* notify_table passes value to the CPUFREQ_ADJUST callback function. */
>  #define NOTIFY_INVALID NULL
>  static DEFINE_PER_CPU(struct cpufreq_cooling_device *, notify_device);
> 
> +/* A list to hold all the cpufreq cooling devices registered */
> +static LIST_HEAD(cpufreq_cooling_list);
> +
>  /**
>   * get_idr - function to get a unique id.
>   * @idr: struct idr * handle used to create a id.
> @@ -98,6 +100,26 @@ static void release_idr(struct idr *idr, int id)
> 
>  /* Below code defines functions to be used for cpufreq as cooling device */
> 
> +/**
> + * cpufreq_cooling_get_info - function to cpufreq_dev for the corresponding cdev
> + * @cdev: pointer of the cooling device
> + *
> + * This function will loop through the cpufreq_cooling_device list and
> + * return the matching device
> + *

You should add a "Locking:" section here which documents that this
function must be called with cooling_cpufreq_lock held.

Cheers,
Javi

> + * Return: cpufreq_cooling_device if matched with the cdev or NULL if not
> + * matched.
> + */
> +static inline struct cpufreq_cooling_device *
> +cpufreq_cooling_get_info(struct thermal_cooling_device *cdev)
> +{
> +       struct cpufreq_cooling_device *cpufreq_dev = NULL;
> +       list_for_each_entry(cpufreq_dev, &cpufreq_cooling_list, node)
> +               if (cpufreq_dev->cool_dev == cdev)
> +                       break;
> +       return cpufreq_dev;
> +}
> +



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 3/6] thermal: thermal-core: Add notifications support for the cooling states
  2014-05-29  8:15 ` [PATCH v1 3/6] thermal: thermal-core: Add notifications support for the cooling states Amit Daniel Kachhap
@ 2014-05-29 12:24   ` Javi Merino
  2014-06-02  9:31     ` Amit Kachhap
  0 siblings, 1 reply; 17+ messages in thread
From: Javi Merino @ 2014-05-29 12:24 UTC (permalink / raw)
  To: Amit Daniel Kachhap
  Cc: linux-pm, linux-acpi, Zhang Rui, linux-kernel, amit.kachhap,
	edubezval, rjw, linux-arm-kernel, lenb

Hi Amit,

On Thu, May 29, 2014 at 09:15:31AM +0100, Amit Daniel Kachhap wrote:
> This patch adds notification infrastructure for any requests related to cooling
> states. The notifier structure passed is of both Get/Set type. So the receiver
> of these can sense the new/cur/max cooling state as decided by thermal governor.
> In addition to that it can also override the cooling state and may do something
> interesting after receiving these CPU cooling events such as masking some
> states, enabling some extra conditional states or perform any extra operation
> for aggressive thermal cooling.
> The notfications events can be of type,
> 
> 1. COOLING_SET_STATE_PRE
> 2. COOLING_SET_STATE_POST
> 3. COOLING_GET_CUR_STATE
> 4. COOLING_GET_MAX_STATE
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  Documentation/thermal/sysfs-api.txt |   21 +++++++++++
>  drivers/thermal/thermal_core.c      |   69 ++++++++++++++++++++++++++++++++++-
>  include/linux/thermal.h             |   21 +++++++++++
>  3 files changed, 109 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> index 87519cb..5f45e03 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -92,6 +92,27 @@ temperature) and throttle appropriate devices.
>      It deletes the corresponding entry form /sys/class/thermal folder and
>      unbind itself from all the thermal zone devices using it.
>  
> +1.2.3 int thermal_cooling_register_notifier(struct notifier_block *nb)
> +
> +    This interface function registers the client notifier handler. The notifier
> +    handler can use this to monitor or update any cooling state requests.
> +    nb: notifier structure containing client notifier handler.
> +
> +1.2.4 int thermal_cooling_unregister_notifier(struct notifier_block *nb)
> +
> +    This interface function unregisters the client notifier handler.
> +    nb: notifier structure containing client notifier handler.
> +
> +1.2.5 int thermal_cooling_notify_states(struct thermal_cooling_status *request,enum cooling_state_ops op)
> +
> +    This interface function invokes the earlier registered cooling states handler.
> +    request: holds the relevant cooling state value.
> +	.cur_state: current cooling state.
> +	.new_state: new cooling state to be set.
> +	.max_state: max cooling state.
> +	.devdata: driver private data pointer.
> +    op: describes various operation supported.
> +
>  1.3 interface for binding a thermal zone device with a thermal cooling device
>  1.3.1 int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
>  	int trip, struct thermal_cooling_device *cdev,
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 71b0ec0..1a60f83 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -52,6 +52,8 @@ static DEFINE_MUTEX(thermal_idr_lock);
>  static LIST_HEAD(thermal_tz_list);
>  static LIST_HEAD(thermal_cdev_list);
>  static LIST_HEAD(thermal_governor_list);
> +/* Notfier list to validates/updates the cpufreq cooling states */
> +static BLOCKING_NOTIFIER_HEAD(cooling_state_notifier_list);
>  
>  static DEFINE_MUTEX(thermal_list_lock);
>  static DEFINE_MUTEX(thermal_governor_lock);
> @@ -1073,8 +1075,71 @@ static struct class thermal_class = {
>  };
>  
>  /**
> - * __thermal_cooling_device_register() - register a new thermal cooling device
> - * @np:		a pointer to a device tree node.
> + * thermal_cooling_notify_states - Invoke the necessary cooling states handler.
> + * @request: holds the relevant cooling state value. say if the cooling state
> + * operation is of type COOLING_GET_MAX_STATE, then request holds
> + * the current max cooling state value.
> + * @op: different operations supported
> + *
> + * This API allows the registered user to recieve the different cooling
> + * notifications like current state, max state and set state.
> + *
> + * Return: 0 (success)
> + */
> +int thermal_cooling_notify_states(struct thermal_cooling_status *request,
> +				enum cooling_state_ops op)
> +{
> +	/* Invoke the notifiers which have registered for this state change */
> +	if (op == COOLING_SET_STATE_PRE ||
> +		op == COOLING_SET_STATE_POST ||
> +		op == COOLING_GET_MAX_STATE ||
> +		op == COOLING_GET_CUR_STATE) {
> +		blocking_notifier_call_chain(
> +			&cooling_state_notifier_list, op, request);
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(thermal_cooling_notify_states);
> +
> +/**
> + * thermal_cooling_register_notifier - registers a notifier with thermal cooling.
> + * @nb:	notifier function to register.
> + *
> + * Add a driver to receive all cooling notifications like current state,
> + * max state and set state. The drivers after reading the events can perform
> + * some mapping like grouping some P states into 1 cooling state.
> + *
> + * Return: 0 (success)
> + */
> +int thermal_cooling_register_notifier(struct notifier_block *nb)
> +{
> +	int ret = 0;
> +	ret = blocking_notifier_chain_register(
> +				&cooling_state_notifier_list, nb);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(thermal_cooling_register_notifier);
> +
> +/**
> + * thermal_cooling_unregister_notifier - unregisters a notifier with thermal
> + * cooling.
> + * @nb:	notifier function to unregister.
> + *
> + * Removes a driver to receive further cooling notifications.
> + *
> + * Return: 0 (success)

Or %-ENOENT if the notifier is not registered.

> + */
> +int thermal_cooling_unregister_notifier(struct notifier_block *nb)
> +{
> +	int ret = 0;
> +	ret = blocking_notifier_chain_unregister(
> +				&cooling_state_notifier_list, nb);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(thermal_cooling_unregister_notifier);
> +
> +/**
> + * thermal_cooling_device_register() - register a new thermal cooling device

This is wrong, the function below this comment is still
__thermal_cooling_device_register() whose first parameter is np, not
type.

>   * @type:	the thermal cooling device type.
>   * @devdata:	device private data.
>   * @ops:		standard thermal cooling devices callbacks.

Cheers,
Javi


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 6/6] ACPI: thermal: processor: Use the generic cpufreq infrastructure
  2014-05-29  8:15 ` [PATCH v1 6/6] ACPI: thermal: processor: Use the generic cpufreq infrastructure Amit Daniel Kachhap
@ 2014-05-29 12:42   ` Javi Merino
  2014-06-02  9:21     ` Amit Kachhap
  0 siblings, 1 reply; 17+ messages in thread
From: Javi Merino @ 2014-05-29 12:42 UTC (permalink / raw)
  To: Amit Daniel Kachhap
  Cc: linux-pm, linux-acpi, Zhang Rui, linux-kernel, amit.kachhap,
	edubezval, rjw, linux-arm-kernel, lenb

Hi Amit,

On Thu, May 29, 2014 at 09:15:34AM +0100, Amit Daniel Kachhap wrote:
> This patch upgrades the ACPI cpufreq cooling portions to use the generic
> cpufreq cooling infrastructure. There should not be any functionality
> related changes as the same behaviour is provided by the generic
> cpufreq APIs with the notifier mechanism.
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  drivers/acpi/processor_driver.c  |    6 +-
>  drivers/acpi/processor_thermal.c |  235 ++++++++++++++++++--------------------
>  include/acpi/processor.h         |    3 +-
>  3 files changed, 115 insertions(+), 129 deletions(-)
> 
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 7f70f31..10aba4a 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -36,6 +36,7 @@
>  #include <linux/cpuidle.h>
>  #include <linux/slab.h>
>  #include <linux/acpi.h>
> +#include <linux/cpu_cooling.h>
> 
>  #include <acpi/processor.h>
> 
> @@ -178,8 +179,7 @@ static int __acpi_processor_start(struct acpi_device *device)
>         if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
>                 acpi_processor_power_init(pr);
> 
> -       pr->cdev = thermal_cooling_device_register("Processor", device,
> -                                                  &processor_cooling_ops);
> +       pr->cdev = acpi_processor_cooling_register(device);

With this you have removed the only cooling device whose type was
"Processor".  There's special code for dealing with this cooling
device in drivers/thermal/thermal_core.c:passive_store():

		list_for_each_entry(cdev, &thermal_cdev_list, node) {
			if (!strncmp("Processor", cdev->type,
				     sizeof("Processor")))
				thermal_zone_bind_cooling_device(tz,
						THERMAL_TRIPS_NONE, cdev,
						THERMAL_NO_LIMIT,
						THERMAL_NO_LIMIT);
		}
		mutex_unlock(&thermal_list_lock);
		if (!tz->passive_delay)

With your change, that code is now "dead" as it can't do anything.  No
I don't know what should you do with it, either remove it or make it
match the cpufreq cooling device.  But this patch should deal with
that code as well.

Cheers,
Javi



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 6/6] ACPI: thermal: processor: Use the generic cpufreq infrastructure
  2014-05-29 12:42   ` Javi Merino
@ 2014-06-02  9:21     ` Amit Kachhap
  2014-06-02 10:20       ` Javi Merino
  0 siblings, 1 reply; 17+ messages in thread
From: Amit Kachhap @ 2014-06-02  9:21 UTC (permalink / raw)
  To: Javi Merino
  Cc: linux-pm, linux-acpi, Zhang Rui, linux-kernel, edubezval, rjw,
	linux-arm-kernel, lenb

Hi Javi,

On 5/29/14, Javi Merino <javi.merino@arm.com> wrote:
> Hi Amit,
>
> On Thu, May 29, 2014 at 09:15:34AM +0100, Amit Daniel Kachhap wrote:
>> This patch upgrades the ACPI cpufreq cooling portions to use the generic
>> cpufreq cooling infrastructure. There should not be any functionality
>> related changes as the same behaviour is provided by the generic
>> cpufreq APIs with the notifier mechanism.
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  drivers/acpi/processor_driver.c  |    6 +-
>>  drivers/acpi/processor_thermal.c |  235
>> ++++++++++++++++++--------------------
>>  include/acpi/processor.h         |    3 +-
>>  3 files changed, 115 insertions(+), 129 deletions(-)
>>
>> diff --git a/drivers/acpi/processor_driver.c
>> b/drivers/acpi/processor_driver.c
>> index 7f70f31..10aba4a 100644
>> --- a/drivers/acpi/processor_driver.c
>> +++ b/drivers/acpi/processor_driver.c
>> @@ -36,6 +36,7 @@
>>  #include <linux/cpuidle.h>
>>  #include <linux/slab.h>
>>  #include <linux/acpi.h>
>> +#include <linux/cpu_cooling.h>
>>
>>  #include <acpi/processor.h>
>>
>> @@ -178,8 +179,7 @@ static int __acpi_processor_start(struct acpi_device
>> *device)
>>         if (!cpuidle_get_driver() || cpuidle_get_driver() ==
>> &acpi_idle_driver)
>>                 acpi_processor_power_init(pr);
>>
>> -       pr->cdev = thermal_cooling_device_register("Processor", device,
>> -
>> &processor_cooling_ops);
>> +       pr->cdev = acpi_processor_cooling_register(device);
>
> With this you have removed the only cooling device whose type was
> "Processor".  There's special code for dealing with this cooling
> device in drivers/thermal/thermal_core.c:passive_store():
>
> 		list_for_each_entry(cdev, &thermal_cdev_list, node) {
> 			if (!strncmp("Processor", cdev->type,
> 				     sizeof("Processor")))
> 				thermal_zone_bind_cooling_device(tz,
> 						THERMAL_TRIPS_NONE, cdev,
> 						THERMAL_NO_LIMIT,
> 						THERMAL_NO_LIMIT);
> 		}
> 		mutex_unlock(&thermal_list_lock);
> 		if (!tz->passive_delay)
>
> With your change, that code is now "dead" as it can't do anything.  No
> I don't know what should you do with it, either remove it or make it
> match the cpufreq cooling device.  But this patch should deal with
> that code as well.
nice catch. I somehow missed modifying this section.
I think the following changes should fix this,
-                       if (!strncmp("Processor", cdev->type,
-                                    sizeof("Processor")))
+                       if (!strncmp("thermal-cpufreq", cdev->type,
+                                    sizeof("thermal-cpufreq")))
                                thermal_zone_bind_cooling_device(tz,

>
> Cheers,
> Javi
>
>
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/6] thermal: cpu_cooling: Support passing driver private data.
  2014-05-29 12:06   ` Javi Merino
@ 2014-06-02  9:24     ` Amit Kachhap
  2014-06-02 18:57       ` Eduardo Valentin
  0 siblings, 1 reply; 17+ messages in thread
From: Amit Kachhap @ 2014-06-02  9:24 UTC (permalink / raw)
  To: Javi Merino
  Cc: linux-pm, linux-acpi, rjw, linux-kernel, edubezval, Zhang Rui,
	linux-arm-kernel, lenb

On 5/29/14, Javi Merino <javi.merino@arm.com> wrote:
> Hi Amit,
>
> One minor comment.
>
> On Thu, May 29, 2014 at 09:15:30AM +0100, Amit Daniel Kachhap wrote:
>> This patch allows the caller of cpufreq cooling APIs to register along
>> with their driver data which will be useful while receiving any cooling
>> states
>> notifications.
>> This patch is in preparation to add notfication support for cpufrequency
>> cooling changes. This change also removes the unnecessary exposing of
>> internal housekeeping structure via thermal_cooling_device->devdata to
>> the
>> users of cpufreq cooling APIs. As part of this implmentation, this private
>> local
>> structure (cpufreq_dev) is now stored in a list and scanned for each call
>> to
>> cpu cooling interfaces.
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  Documentation/thermal/cpu-cooling-api.txt          |    3 +-
>>  drivers/thermal/cpu_cooling.c                      |   89
>> ++++++++++++++++----
>>  drivers/thermal/db8500_cpufreq_cooling.c           |    2 +-
>>  drivers/thermal/samsung/exynos_thermal_common.c    |    2 +-
>>  drivers/thermal/ti-soc-thermal/ti-thermal-common.c |    2 +-
>>  include/linux/cpu_cooling.h                        |    5 +-
>>  6 files changed, 80 insertions(+), 23 deletions(-)
>>
>> diff --git a/Documentation/thermal/cpu-cooling-api.txt
>> b/Documentation/thermal/cpu-cooling-api.txt
>> index fca24c9..aaa07c6 100644
>> --- a/Documentation/thermal/cpu-cooling-api.txt
>> +++ b/Documentation/thermal/cpu-cooling-api.txt
>> @@ -17,13 +17,14 @@ the user. The registration APIs returns the cooling
>> device pointer.
>>
>>  1.1 cpufreq registration/unregistration APIs
>>  1.1.1 struct thermal_cooling_device *cpufreq_cooling_register(
>> -       struct cpumask *clip_cpus)
>> +       struct cpumask *clip_cpus, void *devdata)
>>
>>      This interface function registers the cpufreq cooling device with the
>> name
>>      "thermal-cpufreq-%x". This api can support multiple instances of
>> cpufreq
>>      cooling devices.
>>
>>     clip_cpus: cpumask of cpus where the frequency constraints will
>> happen.
>> +   devdata: driver private data pointer.
>>
>>  1.1.2 void cpufreq_cooling_unregister(struct thermal_cooling_device
>> *cdev)
>>
>> diff --git a/drivers/thermal/cpu_cooling.c
>> b/drivers/thermal/cpu_cooling.c
>> index 16388b0..6d145d5 100644
>> --- a/drivers/thermal/cpu_cooling.c
>> +++ b/drivers/thermal/cpu_cooling.c
>> @@ -50,16 +50,18 @@ struct cpufreq_cooling_device {
>>         unsigned int cpufreq_state;
>>         unsigned int cpufreq_val;
>>         struct cpumask allowed_cpus;
>> +       struct list_head node;
>>  };
>>  static DEFINE_IDR(cpufreq_idr);
>>  static DEFINE_MUTEX(cooling_cpufreq_lock);
>>
>> -static unsigned int cpufreq_dev_count;
>> -
>>  /* notify_table passes value to the CPUFREQ_ADJUST callback function. */
>>  #define NOTIFY_INVALID NULL
>>  static DEFINE_PER_CPU(struct cpufreq_cooling_device *, notify_device);
>>
>> +/* A list to hold all the cpufreq cooling devices registered */
>> +static LIST_HEAD(cpufreq_cooling_list);
>> +
>>  /**
>>   * get_idr - function to get a unique id.
>>   * @idr: struct idr * handle used to create a id.
>> @@ -98,6 +100,26 @@ static void release_idr(struct idr *idr, int id)
>>
>>  /* Below code defines functions to be used for cpufreq as cooling device
>> */
>>
>> +/**
>> + * cpufreq_cooling_get_info - function to cpufreq_dev for the
>> corresponding cdev
>> + * @cdev: pointer of the cooling device
>> + *
>> + * This function will loop through the cpufreq_cooling_device list and
>> + * return the matching device
>> + *
>
> You should add a "Locking:" section here which documents that this
> function must be called with cooling_cpufreq_lock held.
Yes agreed. Will update in the next version.

Thanks,
Amit
>
> Cheers,
> Javi
>
>> + * Return: cpufreq_cooling_device if matched with the cdev or NULL if
>> not
>> + * matched.
>> + */
>> +static inline struct cpufreq_cooling_device *
>> +cpufreq_cooling_get_info(struct thermal_cooling_device *cdev)
>> +{
>> +       struct cpufreq_cooling_device *cpufreq_dev = NULL;
>> +       list_for_each_entry(cpufreq_dev, &cpufreq_cooling_list, node)
>> +               if (cpufreq_dev->cool_dev == cdev)
>> +                       break;
>> +       return cpufreq_dev;
>> +}
>> +
>
>
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 3/6] thermal: thermal-core: Add notifications support for the cooling states
  2014-05-29 12:24   ` Javi Merino
@ 2014-06-02  9:31     ` Amit Kachhap
  2014-06-02 10:14       ` Javi Merino
  0 siblings, 1 reply; 17+ messages in thread
From: Amit Kachhap @ 2014-06-02  9:31 UTC (permalink / raw)
  To: Javi Merino
  Cc: linux-pm, linux-acpi, Zhang Rui, linux-kernel, edubezval, rjw,
	linux-arm-kernel, lenb

On 5/29/14, Javi Merino <javi.merino@arm.com> wrote:
> Hi Amit,
>
> On Thu, May 29, 2014 at 09:15:31AM +0100, Amit Daniel Kachhap wrote:
>> This patch adds notification infrastructure for any requests related to
>> cooling
>> states. The notifier structure passed is of both Get/Set type. So the
>> receiver
>> of these can sense the new/cur/max cooling state as decided by thermal
>> governor.
>> In addition to that it can also override the cooling state and may do
>> something
>> interesting after receiving these CPU cooling events such as masking some
>> states, enabling some extra conditional states or perform any extra
>> operation
>> for aggressive thermal cooling.
>> The notfications events can be of type,
>>
>> 1. COOLING_SET_STATE_PRE
>> 2. COOLING_SET_STATE_POST
>> 3. COOLING_GET_CUR_STATE
>> 4. COOLING_GET_MAX_STATE
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  Documentation/thermal/sysfs-api.txt |   21 +++++++++++
>>  drivers/thermal/thermal_core.c      |   69
>> ++++++++++++++++++++++++++++++++++-
>>  include/linux/thermal.h             |   21 +++++++++++
>>  3 files changed, 109 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/thermal/sysfs-api.txt
>> b/Documentation/thermal/sysfs-api.txt
>> index 87519cb..5f45e03 100644
>> --- a/Documentation/thermal/sysfs-api.txt
>> +++ b/Documentation/thermal/sysfs-api.txt
>> @@ -92,6 +92,27 @@ temperature) and throttle appropriate devices.
>>      It deletes the corresponding entry form /sys/class/thermal folder
>> and
>>      unbind itself from all the thermal zone devices using it.
>>
>> +1.2.3 int thermal_cooling_register_notifier(struct notifier_block *nb)
>> +
>> +    This interface function registers the client notifier handler. The
>> notifier
>> +    handler can use this to monitor or update any cooling state
>> requests.
>> +    nb: notifier structure containing client notifier handler.
>> +
>> +1.2.4 int thermal_cooling_unregister_notifier(struct notifier_block *nb)
>> +
>> +    This interface function unregisters the client notifier handler.
>> +    nb: notifier structure containing client notifier handler.
>> +
>> +1.2.5 int thermal_cooling_notify_states(struct thermal_cooling_status
>> *request,enum cooling_state_ops op)
>> +
>> +    This interface function invokes the earlier registered cooling states
>> handler.
>> +    request: holds the relevant cooling state value.
>> +	.cur_state: current cooling state.
>> +	.new_state: new cooling state to be set.
>> +	.max_state: max cooling state.
>> +	.devdata: driver private data pointer.
>> +    op: describes various operation supported.
>> +
>>  1.3 interface for binding a thermal zone device with a thermal cooling
>> device
>>  1.3.1 int thermal_zone_bind_cooling_device(struct thermal_zone_device
>> *tz,
>>  	int trip, struct thermal_cooling_device *cdev,
>> diff --git a/drivers/thermal/thermal_core.c
>> b/drivers/thermal/thermal_core.c
>> index 71b0ec0..1a60f83 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -52,6 +52,8 @@ static DEFINE_MUTEX(thermal_idr_lock);
>>  static LIST_HEAD(thermal_tz_list);
>>  static LIST_HEAD(thermal_cdev_list);
>>  static LIST_HEAD(thermal_governor_list);
>> +/* Notfier list to validates/updates the cpufreq cooling states */
>> +static BLOCKING_NOTIFIER_HEAD(cooling_state_notifier_list);
>>
>>  static DEFINE_MUTEX(thermal_list_lock);
>>  static DEFINE_MUTEX(thermal_governor_lock);
>> @@ -1073,8 +1075,71 @@ static struct class thermal_class = {
>>  };
>>
>>  /**
>> - * __thermal_cooling_device_register() - register a new thermal cooling
>> device
>> - * @np:		a pointer to a device tree node.
>> + * thermal_cooling_notify_states - Invoke the necessary cooling states
>> handler.
>> + * @request: holds the relevant cooling state value. say if the cooling
>> state
>> + * operation is of type COOLING_GET_MAX_STATE, then request holds
>> + * the current max cooling state value.
>> + * @op: different operations supported
>> + *
>> + * This API allows the registered user to recieve the different cooling
>> + * notifications like current state, max state and set state.
>> + *
>> + * Return: 0 (success)
>> + */
>> +int thermal_cooling_notify_states(struct thermal_cooling_status
>> *request,
>> +				enum cooling_state_ops op)
>> +{
>> +	/* Invoke the notifiers which have registered for this state change */
>> +	if (op == COOLING_SET_STATE_PRE ||
>> +		op == COOLING_SET_STATE_POST ||
>> +		op == COOLING_GET_MAX_STATE ||
>> +		op == COOLING_GET_CUR_STATE) {
>> +		blocking_notifier_call_chain(
>> +			&cooling_state_notifier_list, op, request);
>> +	}
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(thermal_cooling_notify_states);
>> +
>> +/**
>> + * thermal_cooling_register_notifier - registers a notifier with thermal
>> cooling.
>> + * @nb:	notifier function to register.
>> + *
>> + * Add a driver to receive all cooling notifications like current state,
>> + * max state and set state. The drivers after reading the events can
>> perform
>> + * some mapping like grouping some P states into 1 cooling state.
>> + *
>> + * Return: 0 (success)
>> + */
>> +int thermal_cooling_register_notifier(struct notifier_block *nb)
>> +{
>> +	int ret = 0;
>> +	ret = blocking_notifier_chain_register(
>> +				&cooling_state_notifier_list, nb);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(thermal_cooling_register_notifier);
>> +
>> +/**
>> + * thermal_cooling_unregister_notifier - unregisters a notifier with
>> thermal
>> + * cooling.
>> + * @nb:	notifier function to unregister.
>> + *
>> + * Removes a driver to receive further cooling notifications.
>> + *
>> + * Return: 0 (success)
>
> Or %-ENOENT if the notifier is not registered.
>
>> + */
>> +int thermal_cooling_unregister_notifier(struct notifier_block *nb)
>> +{
>> +	int ret = 0;
>> +	ret = blocking_notifier_chain_unregister(
>> +				&cooling_state_notifier_list, nb);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(thermal_cooling_unregister_notifier);
>> +
>> +/**
>> + * thermal_cooling_device_register() - register a new thermal cooling
>> device
>
> This is wrong, the function below this comment is still
> __thermal_cooling_device_register() whose first parameter is np, not
> type.
I did not add this. This seems wrong. Will modify this as a fix in the version.

Thanks,
Amit
>
>>   * @type:	the thermal cooling device type.
>>   * @devdata:	device private data.
>>   * @ops:		standard thermal cooling devices callbacks.
>
> Cheers,
> Javi
>
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 3/6] thermal: thermal-core: Add notifications support for the cooling states
  2014-06-02  9:31     ` Amit Kachhap
@ 2014-06-02 10:14       ` Javi Merino
  0 siblings, 0 replies; 17+ messages in thread
From: Javi Merino @ 2014-06-02 10:14 UTC (permalink / raw)
  To: Amit Kachhap
  Cc: linux-pm, linux-acpi, Zhang Rui, linux-kernel, edubezval, rjw,
	linux-arm-kernel, lenb

On Mon, Jun 02, 2014 at 10:31:19AM +0100, Amit Kachhap wrote:
> On 5/29/14, Javi Merino <javi.merino@arm.com> wrote:
> > Hi Amit,
> >
> > On Thu, May 29, 2014 at 09:15:31AM +0100, Amit Daniel Kachhap wrote:
> >> This patch adds notification infrastructure for any requests related to
> >> cooling
> >> states. The notifier structure passed is of both Get/Set type. So the
> >> receiver
> >> of these can sense the new/cur/max cooling state as decided by thermal
> >> governor.
> >> In addition to that it can also override the cooling state and may do
> >> something
> >> interesting after receiving these CPU cooling events such as masking some
> >> states, enabling some extra conditional states or perform any extra
> >> operation
> >> for aggressive thermal cooling.
> >> The notfications events can be of type,
> >>
> >> 1. COOLING_SET_STATE_PRE
> >> 2. COOLING_SET_STATE_POST
> >> 3. COOLING_GET_CUR_STATE
> >> 4. COOLING_GET_MAX_STATE
> >>
> >> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> >> ---
> >>  Documentation/thermal/sysfs-api.txt |   21 +++++++++++
> >>  drivers/thermal/thermal_core.c      |   69
> >> ++++++++++++++++++++++++++++++++++-
> >>  include/linux/thermal.h             |   21 +++++++++++
> >>  3 files changed, 109 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/thermal/sysfs-api.txt
> >> b/Documentation/thermal/sysfs-api.txt
> >> index 87519cb..5f45e03 100644
> >> --- a/Documentation/thermal/sysfs-api.txt
> >> +++ b/Documentation/thermal/sysfs-api.txt
> >> @@ -92,6 +92,27 @@ temperature) and throttle appropriate devices.
> >>      It deletes the corresponding entry form /sys/class/thermal folder
> >> and
> >>      unbind itself from all the thermal zone devices using it.
> >>
> >> +1.2.3 int thermal_cooling_register_notifier(struct notifier_block *nb)
> >> +
> >> +    This interface function registers the client notifier handler. The
> >> notifier
> >> +    handler can use this to monitor or update any cooling state
> >> requests.
> >> +    nb: notifier structure containing client notifier handler.
> >> +
> >> +1.2.4 int thermal_cooling_unregister_notifier(struct notifier_block *nb)
> >> +
> >> +    This interface function unregisters the client notifier handler.
> >> +    nb: notifier structure containing client notifier handler.
> >> +
> >> +1.2.5 int thermal_cooling_notify_states(struct thermal_cooling_status
> >> *request,enum cooling_state_ops op)
> >> +
> >> +    This interface function invokes the earlier registered cooling states
> >> handler.
> >> +    request: holds the relevant cooling state value.
> >> +	.cur_state: current cooling state.
> >> +	.new_state: new cooling state to be set.
> >> +	.max_state: max cooling state.
> >> +	.devdata: driver private data pointer.
> >> +    op: describes various operation supported.
> >> +
> >>  1.3 interface for binding a thermal zone device with a thermal cooling
> >> device
> >>  1.3.1 int thermal_zone_bind_cooling_device(struct thermal_zone_device
> >> *tz,
> >>  	int trip, struct thermal_cooling_device *cdev,
> >> diff --git a/drivers/thermal/thermal_core.c
> >> b/drivers/thermal/thermal_core.c
> >> index 71b0ec0..1a60f83 100644
> >> --- a/drivers/thermal/thermal_core.c
> >> +++ b/drivers/thermal/thermal_core.c
> >> @@ -52,6 +52,8 @@ static DEFINE_MUTEX(thermal_idr_lock);
> >>  static LIST_HEAD(thermal_tz_list);
> >>  static LIST_HEAD(thermal_cdev_list);
> >>  static LIST_HEAD(thermal_governor_list);
> >> +/* Notfier list to validates/updates the cpufreq cooling states */
> >> +static BLOCKING_NOTIFIER_HEAD(cooling_state_notifier_list);
> >>
> >>  static DEFINE_MUTEX(thermal_list_lock);
> >>  static DEFINE_MUTEX(thermal_governor_lock);
> >> @@ -1073,8 +1075,71 @@ static struct class thermal_class = {
> >>  };
> >>
> >>  /**
> >> - * __thermal_cooling_device_register() - register a new thermal cooling
> >> device
> >> - * @np:		a pointer to a device tree node.
> >> + * thermal_cooling_notify_states - Invoke the necessary cooling states
> >> handler.
> >> + * @request: holds the relevant cooling state value. say if the cooling
> >> state
> >> + * operation is of type COOLING_GET_MAX_STATE, then request holds
> >> + * the current max cooling state value.
> >> + * @op: different operations supported
> >> + *
> >> + * This API allows the registered user to recieve the different cooling
> >> + * notifications like current state, max state and set state.
> >> + *
> >> + * Return: 0 (success)
> >> + */
> >> +int thermal_cooling_notify_states(struct thermal_cooling_status
> >> *request,
> >> +				enum cooling_state_ops op)
> >> +{
> >> +	/* Invoke the notifiers which have registered for this state change */
> >> +	if (op == COOLING_SET_STATE_PRE ||
> >> +		op == COOLING_SET_STATE_POST ||
> >> +		op == COOLING_GET_MAX_STATE ||
> >> +		op == COOLING_GET_CUR_STATE) {
> >> +		blocking_notifier_call_chain(
> >> +			&cooling_state_notifier_list, op, request);
> >> +	}
> >> +	return 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(thermal_cooling_notify_states);
> >> +
> >> +/**
> >> + * thermal_cooling_register_notifier - registers a notifier with thermal
> >> cooling.
> >> + * @nb:	notifier function to register.
> >> + *
> >> + * Add a driver to receive all cooling notifications like current state,
> >> + * max state and set state. The drivers after reading the events can
> >> perform
> >> + * some mapping like grouping some P states into 1 cooling state.
> >> + *
> >> + * Return: 0 (success)
> >> + */
> >> +int thermal_cooling_register_notifier(struct notifier_block *nb)
> >> +{
> >> +	int ret = 0;
> >> +	ret = blocking_notifier_chain_register(
> >> +				&cooling_state_notifier_list, nb);
> >> +	return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(thermal_cooling_register_notifier);
> >> +
> >> +/**
> >> + * thermal_cooling_unregister_notifier - unregisters a notifier with
> >> thermal
> >> + * cooling.
> >> + * @nb:	notifier function to unregister.
> >> + *
> >> + * Removes a driver to receive further cooling notifications.
> >> + *
> >> + * Return: 0 (success)
> >
> > Or %-ENOENT if the notifier is not registered.
> >
> >> + */
> >> +int thermal_cooling_unregister_notifier(struct notifier_block *nb)
> >> +{
> >> +	int ret = 0;
> >> +	ret = blocking_notifier_chain_unregister(
> >> +				&cooling_state_notifier_list, nb);
> >> +	return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(thermal_cooling_unregister_notifier);
> >> +
> >> +/**
> >> + * thermal_cooling_device_register() - register a new thermal cooling
> >> device
> >
> > This is wrong, the function below this comment is still
> > __thermal_cooling_device_register() whose first parameter is np, not
> > type.
> I did not add this. This seems wrong. Will modify this as a fix in the version.

You removed it, probably by mistake.  If you look at the top of this
hunk you will find:

- * __thermal_cooling_device_register() - register a new thermal cooling device
- * @np:		a pointer to a device tree node.

Cheers,
Javi


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 6/6] ACPI: thermal: processor: Use the generic cpufreq infrastructure
  2014-06-02  9:21     ` Amit Kachhap
@ 2014-06-02 10:20       ` Javi Merino
  2014-06-02 17:36         ` Eduardo Valentin
  0 siblings, 1 reply; 17+ messages in thread
From: Javi Merino @ 2014-06-02 10:20 UTC (permalink / raw)
  To: Amit Kachhap
  Cc: linux-pm, linux-acpi, Zhang Rui, linux-kernel, edubezval, rjw,
	linux-arm-kernel, lenb

On Mon, Jun 02, 2014 at 10:21:48AM +0100, Amit Kachhap wrote:
> Hi Javi,
> 
> On 5/29/14, Javi Merino <javi.merino@arm.com> wrote:
> > Hi Amit,
> >
> > On Thu, May 29, 2014 at 09:15:34AM +0100, Amit Daniel Kachhap wrote:
> >> This patch upgrades the ACPI cpufreq cooling portions to use the generic
> >> cpufreq cooling infrastructure. There should not be any functionality
> >> related changes as the same behaviour is provided by the generic
> >> cpufreq APIs with the notifier mechanism.
> >>
> >> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> >> ---
> >>  drivers/acpi/processor_driver.c  |    6 +-
> >>  drivers/acpi/processor_thermal.c |  235
> >> ++++++++++++++++++--------------------
> >>  include/acpi/processor.h         |    3 +-
> >>  3 files changed, 115 insertions(+), 129 deletions(-)
> >>
> >> diff --git a/drivers/acpi/processor_driver.c
> >> b/drivers/acpi/processor_driver.c
> >> index 7f70f31..10aba4a 100644
> >> --- a/drivers/acpi/processor_driver.c
> >> +++ b/drivers/acpi/processor_driver.c
> >> @@ -36,6 +36,7 @@
> >>  #include <linux/cpuidle.h>
> >>  #include <linux/slab.h>
> >>  #include <linux/acpi.h>
> >> +#include <linux/cpu_cooling.h>
> >>
> >>  #include <acpi/processor.h>
> >>
> >> @@ -178,8 +179,7 @@ static int __acpi_processor_start(struct acpi_device
> >> *device)
> >>         if (!cpuidle_get_driver() || cpuidle_get_driver() ==
> >> &acpi_idle_driver)
> >>                 acpi_processor_power_init(pr);
> >>
> >> -       pr->cdev = thermal_cooling_device_register("Processor", device,
> >> -
> >> &processor_cooling_ops);
> >> +       pr->cdev = acpi_processor_cooling_register(device);
> >
> > With this you have removed the only cooling device whose type was
> > "Processor".  There's special code for dealing with this cooling
> > device in drivers/thermal/thermal_core.c:passive_store():
> >
> > 		list_for_each_entry(cdev, &thermal_cdev_list, node) {
> > 			if (!strncmp("Processor", cdev->type,
> > 				     sizeof("Processor")))
> > 				thermal_zone_bind_cooling_device(tz,
> > 						THERMAL_TRIPS_NONE, cdev,
> > 						THERMAL_NO_LIMIT,
> > 						THERMAL_NO_LIMIT);
> > 		}
> > 		mutex_unlock(&thermal_list_lock);
> > 		if (!tz->passive_delay)
> >
> > With your change, that code is now "dead" as it can't do anything.  No
> > I don't know what should you do with it, either remove it or make it
> > match the cpufreq cooling device.  But this patch should deal with
> > that code as well.
> nice catch. I somehow missed modifying this section.
> I think the following changes should fix this,
> -                       if (!strncmp("Processor", cdev->type,
> -                                    sizeof("Processor")))
> +                       if (!strncmp("thermal-cpufreq", cdev->type,
> +                                    sizeof("thermal-cpufreq")))
>                                 thermal_zone_bind_cooling_device(tz,
> 

That should do it.  I don't really understand why this code is
specifically looking for ACPI processor cooling devices but I guess
that's the least disrupting change you can make.

Cheers,
Javi


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 6/6] ACPI: thermal: processor: Use the generic cpufreq infrastructure
  2014-06-02 10:20       ` Javi Merino
@ 2014-06-02 17:36         ` Eduardo Valentin
  0 siblings, 0 replies; 17+ messages in thread
From: Eduardo Valentin @ 2014-06-02 17:36 UTC (permalink / raw)
  To: Javi Merino
  Cc: Amit Kachhap, linux-pm, linux-acpi, Zhang Rui, linux-kernel, rjw,
	linux-arm-kernel, lenb

Hello Amit, Javi,

On Mon, Jun 02, 2014 at 11:20:48AM +0100, Javi Merino wrote:
> On Mon, Jun 02, 2014 at 10:21:48AM +0100, Amit Kachhap wrote:
> > Hi Javi,
> > 
> > On 5/29/14, Javi Merino <javi.merino@arm.com> wrote:
> > > Hi Amit,
> > >
> > > On Thu, May 29, 2014 at 09:15:34AM +0100, Amit Daniel Kachhap wrote:
> > >> This patch upgrades the ACPI cpufreq cooling portions to use the generic
> > >> cpufreq cooling infrastructure. There should not be any functionality
> > >> related changes as the same behaviour is provided by the generic
> > >> cpufreq APIs with the notifier mechanism.
> > >>
> > >> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> > >> ---
> > >>  drivers/acpi/processor_driver.c  |    6 +-
> > >>  drivers/acpi/processor_thermal.c |  235
> > >> ++++++++++++++++++--------------------
> > >>  include/acpi/processor.h         |    3 +-
> > >>  3 files changed, 115 insertions(+), 129 deletions(-)
> > >>
> > >> diff --git a/drivers/acpi/processor_driver.c
> > >> b/drivers/acpi/processor_driver.c
> > >> index 7f70f31..10aba4a 100644
> > >> --- a/drivers/acpi/processor_driver.c
> > >> +++ b/drivers/acpi/processor_driver.c
> > >> @@ -36,6 +36,7 @@
> > >>  #include <linux/cpuidle.h>
> > >>  #include <linux/slab.h>
> > >>  #include <linux/acpi.h>
> > >> +#include <linux/cpu_cooling.h>
> > >>
> > >>  #include <acpi/processor.h>
> > >>
> > >> @@ -178,8 +179,7 @@ static int __acpi_processor_start(struct acpi_device
> > >> *device)
> > >>         if (!cpuidle_get_driver() || cpuidle_get_driver() ==
> > >> &acpi_idle_driver)
> > >>                 acpi_processor_power_init(pr);
> > >>
> > >> -       pr->cdev = thermal_cooling_device_register("Processor", device,
> > >> -
> > >> &processor_cooling_ops);
> > >> +       pr->cdev = acpi_processor_cooling_register(device);
> > >
> > > With this you have removed the only cooling device whose type was
> > > "Processor".  There's special code for dealing with this cooling
> > > device in drivers/thermal/thermal_core.c:passive_store():
> > >
> > > 		list_for_each_entry(cdev, &thermal_cdev_list, node) {
> > > 			if (!strncmp("Processor", cdev->type,
> > > 				     sizeof("Processor")))
> > > 				thermal_zone_bind_cooling_device(tz,
> > > 						THERMAL_TRIPS_NONE, cdev,
> > > 						THERMAL_NO_LIMIT,
> > > 						THERMAL_NO_LIMIT);
> > > 		}
> > > 		mutex_unlock(&thermal_list_lock);
> > > 		if (!tz->passive_delay)
> > >
> > > With your change, that code is now "dead" as it can't do anything.  No
> > > I don't know what should you do with it, either remove it or make it
> > > match the cpufreq cooling device.  But this patch should deal with
> > > that code as well.
> > nice catch. I somehow missed modifying this section.
> > I think the following changes should fix this,
> > -                       if (!strncmp("Processor", cdev->type,
> > -                                    sizeof("Processor")))
> > +                       if (!strncmp("thermal-cpufreq", cdev->type,
> > +                                    sizeof("thermal-cpufreq")))
> >                                 thermal_zone_bind_cooling_device(tz,
> > 
> 
> That should do it.  I don't really understand why this code is
> specifically looking for ACPI processor cooling devices but I guess
> that's the least disrupting change you can make.

Well, I suggest we move slightly carefuly here. The problem is that this
change actually breaks ABI. If so, we need to follow the kernel ABI
change rules. We should never break userspace.

Rui, Do you recall what users are aware of this sysfs entry?

Cheers,

> 
> Cheers,
> Javi
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/6] thermal: cpu_cooling: Support passing driver private data.
  2014-06-02  9:24     ` Amit Kachhap
@ 2014-06-02 18:57       ` Eduardo Valentin
  0 siblings, 0 replies; 17+ messages in thread
From: Eduardo Valentin @ 2014-06-02 18:57 UTC (permalink / raw)
  To: Amit Kachhap
  Cc: Javi Merino, linux-pm, linux-acpi, rjw, linux-kernel, Zhang Rui,
	linux-arm-kernel, lenb

On Mon, Jun 02, 2014 at 02:54:04PM +0530, Amit Kachhap wrote:
> On 5/29/14, Javi Merino <javi.merino@arm.com> wrote:
> > Hi Amit,
> >
> > One minor comment.
> >
> > On Thu, May 29, 2014 at 09:15:30AM +0100, Amit Daniel Kachhap wrote:
> >> This patch allows the caller of cpufreq cooling APIs to register along
> >> with their driver data which will be useful while receiving any cooling
> >> states
> >> notifications.
> >> This patch is in preparation to add notfication support for cpufrequency
> >> cooling changes. This change also removes the unnecessary exposing of
> >> internal housekeeping structure via thermal_cooling_device->devdata to
> >> the
> >> users of cpufreq cooling APIs. As part of this implmentation, this private
> >> local
> >> structure (cpufreq_dev) is now stored in a list and scanned for each call
> >> to
> >> cpu cooling interfaces.
> >>
> >> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> >> ---
> >>  Documentation/thermal/cpu-cooling-api.txt          |    3 +-
> >>  drivers/thermal/cpu_cooling.c                      |   89
> >> ++++++++++++++++----
> >>  drivers/thermal/db8500_cpufreq_cooling.c           |    2 +-
> >>  drivers/thermal/samsung/exynos_thermal_common.c    |    2 +-
> >>  drivers/thermal/ti-soc-thermal/ti-thermal-common.c |    2 +-
> >>  include/linux/cpu_cooling.h                        |    5 +-
> >>  6 files changed, 80 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/Documentation/thermal/cpu-cooling-api.txt
> >> b/Documentation/thermal/cpu-cooling-api.txt
> >> index fca24c9..aaa07c6 100644
> >> --- a/Documentation/thermal/cpu-cooling-api.txt
> >> +++ b/Documentation/thermal/cpu-cooling-api.txt
> >> @@ -17,13 +17,14 @@ the user. The registration APIs returns the cooling
> >> device pointer.
> >>
> >>  1.1 cpufreq registration/unregistration APIs
> >>  1.1.1 struct thermal_cooling_device *cpufreq_cooling_register(
> >> -       struct cpumask *clip_cpus)
> >> +       struct cpumask *clip_cpus, void *devdata)
> >>
> >>      This interface function registers the cpufreq cooling device with the
> >> name
> >>      "thermal-cpufreq-%x". This api can support multiple instances of
> >> cpufreq
> >>      cooling devices.
> >>
> >>     clip_cpus: cpumask of cpus where the frequency constraints will
> >> happen.
> >> +   devdata: driver private data pointer.
> >>
> >>  1.1.2 void cpufreq_cooling_unregister(struct thermal_cooling_device
> >> *cdev)
> >>
> >> diff --git a/drivers/thermal/cpu_cooling.c
> >> b/drivers/thermal/cpu_cooling.c
> >> index 16388b0..6d145d5 100644
> >> --- a/drivers/thermal/cpu_cooling.c
> >> +++ b/drivers/thermal/cpu_cooling.c
> >> @@ -50,16 +50,18 @@ struct cpufreq_cooling_device {
> >>         unsigned int cpufreq_state;
> >>         unsigned int cpufreq_val;
> >>         struct cpumask allowed_cpus;
> >> +       struct list_head node;
> >>  };
> >>  static DEFINE_IDR(cpufreq_idr);
> >>  static DEFINE_MUTEX(cooling_cpufreq_lock);
> >>
> >> -static unsigned int cpufreq_dev_count;
> >> -
> >>  /* notify_table passes value to the CPUFREQ_ADJUST callback function. */
> >>  #define NOTIFY_INVALID NULL
> >>  static DEFINE_PER_CPU(struct cpufreq_cooling_device *, notify_device);
> >>
> >> +/* A list to hold all the cpufreq cooling devices registered */
> >> +static LIST_HEAD(cpufreq_cooling_list);
> >> +
> >>  /**
> >>   * get_idr - function to get a unique id.
> >>   * @idr: struct idr * handle used to create a id.
> >> @@ -98,6 +100,26 @@ static void release_idr(struct idr *idr, int id)
> >>
> >>  /* Below code defines functions to be used for cpufreq as cooling device
> >> */
> >>
> >> +/**
> >> + * cpufreq_cooling_get_info - function to cpufreq_dev for the
> >> corresponding cdev
> >> + * @cdev: pointer of the cooling device
> >> + *
> >> + * This function will loop through the cpufreq_cooling_device list and
> >> + * return the matching device
> >> + *
> >
> > You should add a "Locking:" section here which documents that this
> > function must be called with cooling_cpufreq_lock held.
> Yes agreed. Will update in the next version.

One good practice is to check the output of scripts/kerneldoc on your
changed file.

Cheers

> 
> Thanks,
> Amit
> >
> > Cheers,
> > Javi
> >
> >> + * Return: cpufreq_cooling_device if matched with the cdev or NULL if
> >> not
> >> + * matched.
> >> + */
> >> +static inline struct cpufreq_cooling_device *
> >> +cpufreq_cooling_get_info(struct thermal_cooling_device *cdev)
> >> +{
> >> +       struct cpufreq_cooling_device *cpufreq_dev = NULL;
> >> +       list_for_each_entry(cpufreq_dev, &cpufreq_cooling_list, node)
> >> +               if (cpufreq_dev->cool_dev == cdev)
> >> +                       break;
> >> +       return cpufreq_dev;
> >> +}
> >> +
> >
> >
> >

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2014-06-02 18:57 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-29  8:15 [PATCH v1 0/6] ACPI: thermal: Migrate cpufreq cooling to generic cpu_cooling layer Amit Daniel Kachhap
2014-05-29  8:15 ` [PATCH v1 1/6] thermal: cpu_cooling: Fix the notification mechanism by using per cpu structure Amit Daniel Kachhap
2014-05-29  8:15 ` [PATCH v1 2/6] thermal: cpu_cooling: Support passing driver private data Amit Daniel Kachhap
2014-05-29 12:06   ` Javi Merino
2014-06-02  9:24     ` Amit Kachhap
2014-06-02 18:57       ` Eduardo Valentin
2014-05-29  8:15 ` [PATCH v1 3/6] thermal: thermal-core: Add notifications support for the cooling states Amit Daniel Kachhap
2014-05-29 12:24   ` Javi Merino
2014-06-02  9:31     ` Amit Kachhap
2014-06-02 10:14       ` Javi Merino
2014-05-29  8:15 ` [PATCH v1 4/6] thermal: cpu_cooling: Add support to find up/low frequency levels Amit Daniel Kachhap
2014-05-29  8:15 ` [PATCH v1 5/6] thermal: thermal_core: Remove the max cooling limit check in registration Amit Daniel Kachhap
2014-05-29  8:15 ` [PATCH v1 6/6] ACPI: thermal: processor: Use the generic cpufreq infrastructure Amit Daniel Kachhap
2014-05-29 12:42   ` Javi Merino
2014-06-02  9:21     ` Amit Kachhap
2014-06-02 10:20       ` Javi Merino
2014-06-02 17:36         ` Eduardo Valentin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).