[RFC PATCH 0/5] ACPI: thermal: Migrate cpufreq cooling to generic cpu_cooling layer

[RFC PATCH 0/5] ACPI: thermal: Migrate cpufreq cooling to generic cpu_cooling layer

[Amit Daniel Kachhap]

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. CPU_COOLING_SET_STATE_PRE
2. CPU_COOLING_SET_STATE_POST
3. CPU_COOLING_GET_CUR_STATE
4. CPU_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.

Amit Daniel Kachhap (5):
  thermal: cpu_cooling: Support passing driver private data.
  thermal: cpu_cooling: Add notifications support for the clients
  thermal: cpu_cooling: Add support to find nearby frequency levels.
  thermal: cpu_cooling: Release the upper cooling limit checks
  ACPI: thermal: processor: Use the generic cpufreq infrastructure

 drivers/acpi/processor_driver.c                    |    6 +-
 drivers/acpi/processor_thermal.c                   |  210 +++++++++----------
 drivers/thermal/cpu_cooling.c                      |  220 +++++++++++++++++---
 drivers/thermal/db8500_cpufreq_cooling.c           |    2 +-
 drivers/thermal/samsung/exynos_thermal_common.c    |    5 +-
 drivers/thermal/step_wise.c                        |    2 +-
 drivers/thermal/thermal_core.c                     |    9 +-
 drivers/thermal/ti-soc-thermal/ti-thermal-common.c |    2 +-
 include/linux/cpu_cooling.h                        |   83 +++++++-
 include/linux/thermal.h                            |    1 +
 10 files changed, 378 insertions(+), 162 deletions(-)

[RFC PATCH 1/5] thermal: cpu_cooling: Support passing driver private data.

[Amit Daniel Kachhap]

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.

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/thermal/cpu_cooling.c                      |   79 +++++++++++++++----
 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 +-
 5 files changed, 68 insertions(+), 22 deletions(-)

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 4246262..21f44d4 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 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.
@@ -357,12 +359,23 @@ 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);
+	list_for_each_entry(cpufreq_device, &cpufreq_cooling_list, node)
+		if (cpufreq_device->cool_dev == cdev)
+			break;
+	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);
@@ -386,7 +399,17 @@ 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);
+	list_for_each_entry(cpufreq_device, &cpufreq_cooling_list, node)
+		if (cpufreq_device->cool_dev == cdev)
+			break;
+	mutex_unlock(&cooling_cpufreq_lock);
+	if (!cpufreq_device) {
+		/* Cooling device pointer not found */
+		return -EINVAL;
+	}
 
 	*state = cpufreq_device->cpufreq_state;
 
@@ -406,7 +429,17 @@ 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);
+	list_for_each_entry(cpufreq_device, &cpufreq_cooling_list, node)
+		if (cpufreq_device->cool_dev == cdev)
+			break;
+	mutex_unlock(&cooling_cpufreq_lock);
+	if (!cpufreq_device) {
+		/* Cooling device pointer not found */
+		return -EINVAL;
+	}
 
 	return cpufreq_apply_cooling(cpufreq_device, state);
 }
@@ -427,6 +460,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
@@ -438,7 +472,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;
@@ -477,7 +511,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);
@@ -489,10 +523,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);
 
@@ -502,6 +537,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
@@ -511,9 +547,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);
 
@@ -537,7 +573,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);
 
@@ -554,17 +590,26 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
 	if (!cdev)
 		return;
 
-	cpufreq_dev = cdev->devdata;
 	mutex_lock(&cooling_cpufreq_lock);
-	cpufreq_dev_count--;
+
+	list_for_each_entry(cpufreq_dev, &cpufreq_cooling_list, node)
+		if (cpufreq_dev->cool_dev == cdev)
+			break;
+
+	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

[RFC PATCH 1/5] thermal: cpu_cooling: Support passing driver private data.

[Eduardo Valentin]

Hello Amit,


On Thu, May 08, 2014 at 08:07:56PM +0530, Amit Daniel Kachhap wrote:
> 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.
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  drivers/thermal/cpu_cooling.c                      |   79 +++++++++++++++----
>  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 +-
>  5 files changed, 68 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index 4246262..21f44d4 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 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.
> @@ -357,12 +359,23 @@ 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);
> +	list_for_each_entry(cpufreq_device, &cpufreq_cooling_list, node)
> +		if (cpufreq_device->cool_dev == cdev)
> +			break;
> +	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);
> @@ -386,7 +399,17 @@ 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);
> +	list_for_each_entry(cpufreq_device, &cpufreq_cooling_list, node)
> +		if (cpufreq_device->cool_dev == cdev)
> +			break;
> +	mutex_unlock(&cooling_cpufreq_lock);
> +	if (!cpufreq_device) {
> +		/* Cooling device pointer not found */
> +		return -EINVAL;
> +	}
>  
>  	*state = cpufreq_device->cpufreq_state;
>  
> @@ -406,7 +429,17 @@ 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);
> +	list_for_each_entry(cpufreq_device, &cpufreq_cooling_list, node)
> +		if (cpufreq_device->cool_dev == cdev)
> +			break;
> +	mutex_unlock(&cooling_cpufreq_lock);
> +	if (!cpufreq_device) {
> +		/* Cooling device pointer not found */
> +		return -EINVAL;
> +	}
>  
>  	return cpufreq_apply_cooling(cpufreq_device, state);
>  }
> @@ -427,6 +460,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
> @@ -438,7 +472,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;
> @@ -477,7 +511,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);
> @@ -489,10 +523,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);
>  
> @@ -502,6 +537,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
> @@ -511,9 +547,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);
>  
> @@ -537,7 +573,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);
>  
> @@ -554,17 +590,26 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
>  	if (!cdev)
>  		return;
>  
> -	cpufreq_dev = cdev->devdata;
>  	mutex_lock(&cooling_cpufreq_lock);
> -	cpufreq_dev_count--;
> +
> +	list_for_each_entry(cpufreq_dev, &cpufreq_cooling_list, node)
> +		if (cpufreq_dev->cool_dev == cdev)
> +			break;
> +
> +	if (!cpufreq_dev) {
> +		/* Cooling device pointer not found */
> +		mutex_unlock(&cooling_cpufreq_lock);
> +		return;
> +	}

Because you are adding several times the same code to search through
your list of cdevs, I recommend adding at least a helper function to
perform this recurring operation.

Besides, are you sure you cannot keep both pointers, the cpufreq_dev and
the driver devdata? One way is to embedded the driver devdata inside the
cpufreq_dev and register cpufreq_dev. Then you would adapt the code to
fetch each pointer accordingly. What do you think?

> +	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
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[RFC PATCH 1/5] thermal: cpu_cooling: Support passing driver private data.

[Rafael J. Wysocki]

On Thursday, May 15, 2014 02:33:07 PM Eduardo Valentin wrote:
> Hello Amit,
> 
> 
> On Thu, May 08, 2014 at 08:07:56PM +0530, Amit Daniel Kachhap wrote:
> > 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.
> > 
> > Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> > ---
> >  drivers/thermal/cpu_cooling.c                      |   79 +++++++++++++++----
> >  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 +-
> >  5 files changed, 68 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> > index 4246262..21f44d4 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 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.
> > @@ -357,12 +359,23 @@ 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);
> > +	list_for_each_entry(cpufreq_device, &cpufreq_cooling_list, node)
> > +		if (cpufreq_device->cool_dev == cdev)
> > +			break;
> > +	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);
> > @@ -386,7 +399,17 @@ 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);
> > +	list_for_each_entry(cpufreq_device, &cpufreq_cooling_list, node)
> > +		if (cpufreq_device->cool_dev == cdev)
> > +			break;
> > +	mutex_unlock(&cooling_cpufreq_lock);
> > +	if (!cpufreq_device) {
> > +		/* Cooling device pointer not found */
> > +		return -EINVAL;
> > +	}
> >  
> >  	*state = cpufreq_device->cpufreq_state;
> >  
> > @@ -406,7 +429,17 @@ 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);
> > +	list_for_each_entry(cpufreq_device, &cpufreq_cooling_list, node)
> > +		if (cpufreq_device->cool_dev == cdev)
> > +			break;
> > +	mutex_unlock(&cooling_cpufreq_lock);
> > +	if (!cpufreq_device) {
> > +		/* Cooling device pointer not found */
> > +		return -EINVAL;
> > +	}
> >  
> >  	return cpufreq_apply_cooling(cpufreq_device, state);
> >  }
> > @@ -427,6 +460,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
> > @@ -438,7 +472,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;
> > @@ -477,7 +511,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);
> > @@ -489,10 +523,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);
> >  
> > @@ -502,6 +537,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
> > @@ -511,9 +547,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);
> >  
> > @@ -537,7 +573,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);
> >  
> > @@ -554,17 +590,26 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
> >  	if (!cdev)
> >  		return;
> >  
> > -	cpufreq_dev = cdev->devdata;
> >  	mutex_lock(&cooling_cpufreq_lock);
> > -	cpufreq_dev_count--;
> > +
> > +	list_for_each_entry(cpufreq_dev, &cpufreq_cooling_list, node)
> > +		if (cpufreq_dev->cool_dev == cdev)
> > +			break;
> > +
> > +	if (!cpufreq_dev) {
> > +		/* Cooling device pointer not found */
> > +		mutex_unlock(&cooling_cpufreq_lock);
> > +		return;
> > +	}
> 
> Because you are adding several times the same code to search through
> your list of cdevs, I recommend adding at least a helper function to
> perform this recurring operation.

Violently agreed.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[RFC PATCH 1/5] thermal: cpu_cooling: Support passing driver private data.

[amit daniel kachhap]

On Sat, May 17, 2014 at 4:01 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Thursday, May 15, 2014 02:33:07 PM Eduardo Valentin wrote:
>> Hello Amit,
>>
>>
>> On Thu, May 08, 2014 at 08:07:56PM +0530, Amit Daniel Kachhap wrote:
>> > 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.
>> >
>> > Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> > ---
>> >  drivers/thermal/cpu_cooling.c                      |   79 +++++++++++++++----
>> >  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 +-
>> >  5 files changed, 68 insertions(+), 22 deletions(-)
>> >
>> > diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
>> > index 4246262..21f44d4 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 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.
>> > @@ -357,12 +359,23 @@ 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);
>> > +   list_for_each_entry(cpufreq_device, &cpufreq_cooling_list, node)
>> > +           if (cpufreq_device->cool_dev == cdev)
>> > +                   break;
>> > +   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);
>> > @@ -386,7 +399,17 @@ 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);
>> > +   list_for_each_entry(cpufreq_device, &cpufreq_cooling_list, node)
>> > +           if (cpufreq_device->cool_dev == cdev)
>> > +                   break;
>> > +   mutex_unlock(&cooling_cpufreq_lock);
>> > +   if (!cpufreq_device) {
>> > +           /* Cooling device pointer not found */
>> > +           return -EINVAL;
>> > +   }
>> >
>> >     *state = cpufreq_device->cpufreq_state;
>> >
>> > @@ -406,7 +429,17 @@ 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);
>> > +   list_for_each_entry(cpufreq_device, &cpufreq_cooling_list, node)
>> > +           if (cpufreq_device->cool_dev == cdev)
>> > +                   break;
>> > +   mutex_unlock(&cooling_cpufreq_lock);
>> > +   if (!cpufreq_device) {
>> > +           /* Cooling device pointer not found */
>> > +           return -EINVAL;
>> > +   }
>> >
>> >     return cpufreq_apply_cooling(cpufreq_device, state);
>> >  }
>> > @@ -427,6 +460,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
>> > @@ -438,7 +472,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;
>> > @@ -477,7 +511,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);
>> > @@ -489,10 +523,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);
>> >
>> > @@ -502,6 +537,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
>> > @@ -511,9 +547,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);
>> >
>> > @@ -537,7 +573,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);
>> >
>> > @@ -554,17 +590,26 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
>> >     if (!cdev)
>> >             return;
>> >
>> > -   cpufreq_dev = cdev->devdata;
>> >     mutex_lock(&cooling_cpufreq_lock);
>> > -   cpufreq_dev_count--;
>> > +
>> > +   list_for_each_entry(cpufreq_dev, &cpufreq_cooling_list, node)
>> > +           if (cpufreq_dev->cool_dev == cdev)
>> > +                   break;
>> > +
>> > +   if (!cpufreq_dev) {
>> > +           /* Cooling device pointer not found */
>> > +           mutex_unlock(&cooling_cpufreq_lock);
>> > +           return;
>> > +   }
>>
>> Because you are adding several times the same code to search through
>> your list of cdevs, I recommend adding at least a helper function to
>> perform this recurring operation.
>
> Violently agreed.
ok will use wrapper function in next iteration.
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[linux-pm] [RFC PATCH 1/5] thermal: cpu_cooling: Support passing driver private data.

[amit daniel kachhap]

Hi Eduardo,

Thanks for reviewing these patches.

On Fri, May 16, 2014 at 12:03 AM, Eduardo Valentin <edubezval@gmail.com> wrote:
> Hello Amit,
>
>
> On Thu, May 08, 2014 at 08:07:56PM +0530, Amit Daniel Kachhap wrote:
>> 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.
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  drivers/thermal/cpu_cooling.c                      |   79 +++++++++++++++----
>>  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 +-
>>  5 files changed, 68 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
>> index 4246262..21f44d4 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 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.
>> @@ -357,12 +359,23 @@ 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);
>> +     list_for_each_entry(cpufreq_device, &cpufreq_cooling_list, node)
>> +             if (cpufreq_device->cool_dev == cdev)
>> +                     break;
>> +     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);
>> @@ -386,7 +399,17 @@ 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);
>> +     list_for_each_entry(cpufreq_device, &cpufreq_cooling_list, node)
>> +             if (cpufreq_device->cool_dev == cdev)
>> +                     break;
>> +     mutex_unlock(&cooling_cpufreq_lock);
>> +     if (!cpufreq_device) {
>> +             /* Cooling device pointer not found */
>> +             return -EINVAL;
>> +     }
>>
>>       *state = cpufreq_device->cpufreq_state;
>>
>> @@ -406,7 +429,17 @@ 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);
>> +     list_for_each_entry(cpufreq_device, &cpufreq_cooling_list, node)
>> +             if (cpufreq_device->cool_dev == cdev)
>> +                     break;
>> +     mutex_unlock(&cooling_cpufreq_lock);
>> +     if (!cpufreq_device) {
>> +             /* Cooling device pointer not found */
>> +             return -EINVAL;
>> +     }
>>
>>       return cpufreq_apply_cooling(cpufreq_device, state);
>>  }
>> @@ -427,6 +460,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
>> @@ -438,7 +472,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;
>> @@ -477,7 +511,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);
>> @@ -489,10 +523,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);
>>
>> @@ -502,6 +537,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
>> @@ -511,9 +547,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);
>>
>> @@ -537,7 +573,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);
>>
>> @@ -554,17 +590,26 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
>>       if (!cdev)
>>               return;
>>
>> -     cpufreq_dev = cdev->devdata;
>>       mutex_lock(&cooling_cpufreq_lock);
>> -     cpufreq_dev_count--;
>> +
>> +     list_for_each_entry(cpufreq_dev, &cpufreq_cooling_list, node)
>> +             if (cpufreq_dev->cool_dev == cdev)
>> +                     break;
>> +
>> +     if (!cpufreq_dev) {
>> +             /* Cooling device pointer not found */
>> +             mutex_unlock(&cooling_cpufreq_lock);
>> +             return;
>> +     }
>
> Because you are adding several times the same code to search through
> your list of cdevs, I recommend adding at least a helper function to
> perform this recurring operation.
Good suggestion. will implement them in next version.

>
> Besides, are you sure you cannot keep both pointers, the cpufreq_dev and
> the driver devdata? One way is to embedded the driver devdata inside the
> cpufreq_dev and register cpufreq_dev. Then you would adapt the code to
> fetch each pointer accordingly. What do you think?
Yes I also thought of using some other ways but could not find.
Actually cpucooling layer is an intermediate
in between driver and thermal core layer. so ideally it should not
break any existing relation
between driver and thermal core layer even if cpucooling is used or not.
In one place in ACPI there is a check whether supplied driver data
matches with (struct thermal_cooling_device *)cdev->devdata.
so i had no option but to maintain cpufreq_dev in some local
structures and assign passed driver_data to cdev->devdata;
Also cpufreq_dev is a local cpucooling structures which should not be
exposed to user.
>
>> +     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
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/

[RFC PATCH 2/5] thermal: cpu_cooling: Add notifications support for the clients

[Amit Daniel Kachhap]

This patch adds notification support for those clients of cpu_cooling
APIs which may want to do something interesting after receiving these
cpu_cooling events. The notifier structure passed is of both Set/Get type.
The notfications events can be of type,
1. CPU_COOLING_SET_STATE_PRE
2. CPU_COOLING_SET_STATE_POST
3. CPU_COOLING_GET_CUR_STATE
4. CPU_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. Also some more cooling
states can be enabled when the maximum of P state is reached. Post notifications
can be used for those cases.

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/thermal/cpu_cooling.c |   99 +++++++++++++++++++++++++++++++++++++++-
 include/linux/cpu_cooling.h   |   55 +++++++++++++++++++++++
 2 files changed, 151 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 21f44d4..e2aeb36 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -50,6 +50,7 @@ struct cpufreq_cooling_device {
 	unsigned int cpufreq_state;
 	unsigned int cpufreq_val;
 	struct cpumask allowed_cpus;
+	struct cpufreq_cooling_status request_status;
 	struct list_head node;
 };
 static DEFINE_IDR(cpufreq_idr);
@@ -59,6 +60,8 @@ static DEFINE_MUTEX(cooling_cpufreq_lock);
 #define NOTIFY_INVALID NULL
 static struct cpufreq_cooling_device *notify_device;
 
+/* Notfier list to validates/updates the cpufreq cooling states */
+static BLOCKING_NOTIFIER_HEAD(cpufreq_cooling_state_notifier_list);
 /* A list to hold all the cpufreq cooling devices registered */
 static LIST_HEAD(cpufreq_cooling_list);
 
@@ -266,6 +269,21 @@ static unsigned int get_cpu_frequency(unsigned int cpu, unsigned long level)
 	return freq;
 }
 
+static int
+cpufreq_cooling_notify_states(struct cpufreq_cooling_status *request,
+			enum cpu_cooling_state_ops op)
+{
+	/* Invoke the notifiers which have registered for this state change */
+	if (op == CPU_COOLING_SET_STATE_PRE ||
+		op == CPU_COOLING_SET_STATE_POST ||
+		op == CPU_COOLING_GET_MAX_STATE ||
+		op == CPU_COOLING_GET_CUR_STATE) {
+		blocking_notifier_call_chain(
+			&cpufreq_cooling_state_notifier_list, op, request);
+	}
+	return 0;
+}
+
 /**
  * cpufreq_apply_cooling - function to apply frequency clipping.
  * @cpufreq_device: cpufreq_cooling_device pointer containing frequency
@@ -285,9 +303,18 @@ static int cpufreq_apply_cooling(struct cpufreq_cooling_device *cpufreq_device,
 	struct cpumask *mask = &cpufreq_device->allowed_cpus;
 	unsigned int cpu = cpumask_any(mask);
 
+	cpufreq_device->request_status.cur_state =
+						cpufreq_device->cpufreq_state;
+	cpufreq_device->request_status.new_state = cooling_state;
+
+	cpufreq_cooling_notify_states(&cpufreq_device->request_status,
+					CPU_COOLING_SET_STATE_PRE);
+
+	cooling_state = cpufreq_device->request_status.new_state;
 
 	/* Check if the old cooling action is same as new cooling action */
-	if (cpufreq_device->cpufreq_state == cooling_state)
+	if (cpufreq_device->cpufreq_state ==
+				cpufreq_device->request_status.new_state)
 		return 0;
 
 	clip_freq = get_cpu_frequency(cpu, cooling_state);
@@ -304,7 +331,8 @@ static int cpufreq_apply_cooling(struct cpufreq_cooling_device *cpufreq_device,
 	}
 
 	notify_device = NOTIFY_INVALID;
-
+	cpufreq_cooling_notify_states(&cpufreq_device->request_status,
+					CPU_COOLING_SET_STATE_POST);
 	return 0;
 }
 
@@ -383,6 +411,11 @@ static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
 	if (count > 0)
 		*state = count;
 
+	cpufreq_device->request_status.max_state = count;
+	cpufreq_cooling_notify_states(&cpufreq_device->request_status,
+					CPU_COOLING_GET_MAX_STATE);
+	*state = cpufreq_device->request_status.max_state;
+
 	return ret;
 }
 
@@ -410,8 +443,12 @@ static int cpufreq_get_cur_state(struct thermal_cooling_device *cdev,
 		/* Cooling device pointer not found */
 		return -EINVAL;
 	}
+	cpufreq_device->request_status.cur_state =
+						cpufreq_device->cpufreq_state;
+	cpufreq_cooling_notify_states(&cpufreq_device->request_status,
+					CPU_COOLING_GET_CUR_STATE);
 
-	*state = cpufreq_device->cpufreq_state;
+	*state = cpufreq_device->request_status.cur_state;
 
 	return 0;
 }
@@ -520,6 +557,7 @@ __cpufreq_cooling_register(struct device_node *np,
 	}
 	cpufreq_dev->cool_dev = cool_dev;
 	cpufreq_dev->cpufreq_state = 0;
+	cpufreq_dev->request_status.devdata = devdata;
 	mutex_lock(&cooling_cpufreq_lock);
 
 	/* Register the notifier for first cpufreq cooling device */
@@ -614,3 +652,58 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
 	kfree(cpufreq_dev);
 }
 EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister);
+
+/**
+ * cpufreq_cooling_register_notifier - register a driver with cpufreq cooling.
+ * @nb:	notifier function to register.
+ * @list: CPU_COOLING_STATE_NOTIFIER type.
+ *
+ * Add a driver to receive cpufreq cooling notifications like current state,
+ * max state and set state. The drivers after reading the events can perform
+ * some mapping between the P states and cooling states like grouping some P
+ * states into 1 cooling state.
+ *
+ * Return: 0 (success)
+ */
+int cpufreq_cooling_register_notifier(struct notifier_block *nb,
+					unsigned int list)
+{
+	int ret = 0;
+	switch (list) {
+	case CPU_COOLING_STATE_NOTIFIER:
+		ret = blocking_notifier_chain_register(
+				&cpufreq_cooling_state_notifier_list, nb);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cpufreq_cooling_register_notifier);
+
+/**
+ * cpufreq_cooling_unregister_notifier - unregisters a driver with cpufreq
+ * cooling.
+ * @nb:	notifier function to unregister.
+ * @list: CPU_COOLING_STATE_NOTIFIER type.
+ *
+ * Removes a driver to receive further cpufreq cooling notifications.
+ *
+ * Return: 0 (success)
+ */
+int cpufreq_cooling_unregister_notifier(struct notifier_block *nb,
+					unsigned int list)
+{
+	int ret = 0;
+	switch (list) {
+	case CPU_COOLING_STATE_NOTIFIER:
+		ret = blocking_notifier_chain_unregister(
+				&cpufreq_cooling_state_notifier_list, nb);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister_notifier);
diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
index aaef7d8..f786935 100644
--- a/include/linux/cpu_cooling.h
+++ b/include/linux/cpu_cooling.h
@@ -28,6 +28,22 @@
 #include <linux/thermal.h>
 #include <linux/cpumask.h>
 
+#define CPU_COOLING_STATE_NOTIFIER	(0)
+
+enum cpu_cooling_state_ops {
+	CPU_COOLING_SET_STATE_PRE,
+	CPU_COOLING_SET_STATE_POST,
+	CPU_COOLING_GET_CUR_STATE,
+	CPU_COOLING_GET_MAX_STATE,
+};
+
+struct cpufreq_cooling_status {
+	unsigned long cur_state;
+	unsigned long new_state;
+	unsigned long max_state;
+	void *devdata;
+};
+
 #ifdef CONFIG_CPU_THERMAL
 /**
  * cpufreq_cooling_register - function to create cpufreq cooling device.
@@ -62,6 +78,34 @@ 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);
+
+/**
+ * cpufreq_cooling_register_notifier - register a driver with cpufreq cooling.
+ * @nb:	notifier function to register.
+ * @list: CPU_COOLING_STATE_NOTIFIER type.
+ *
+ * Add a driver to receive cpufreq cooling notifications like current state,
+ * max state and set state. The drivers after reading the events can perform
+ * some mapping between the P states and cooling states like grouping some P
+ * states into 1 cooling state.
+ *
+ * Return: 0 (success)
+ */
+int cpufreq_cooling_register_notifier(struct notifier_block *nb,
+					unsigned int list);
+
+/**
+ * cpufreq_cooling_unregister_notifier - unregisters a driver with cpufreq
+ * cooling.
+ * @nb:	notifier function to unregister.
+ * @list: CPU_COOLING_STATE_NOTIFIER type.
+ *
+ * Removes a driver to receive further cpufreq cooling notifications.
+ *
+ * Return: 0 (success)
+ */
+int cpufreq_cooling_unregister_notifier(struct notifier_block *nb,
+					unsigned int list);
 #else /* !CONFIG_CPU_THERMAL */
 static inline struct thermal_cooling_device *
 cpufreq_cooling_register(const struct cpumask *clip_cpus, void *devdata)
@@ -84,6 +128,17 @@ unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq)
 {
 	return THERMAL_CSTATE_INVALID;
 }
+static inline int
+cpufreq_cooling_register_notifier(struct notifier_block *nb, unsigned int list)
+{
+	return 0;
+}
+static inline int
+cpufreq_cooling_unregister_notifier(struct notifier_block *nb,
+					unsigned int list)
+{
+	return 0;
+}
 #endif	/* CONFIG_CPU_THERMAL */
 
 #endif /* __CPU_COOLING_H__ */
-- 
1.7.1

[RFC PATCH 2/5] thermal: cpu_cooling: Add notifications support for the clients

[Eduardo Valentin]

Hello Amit,


On Thu, May 08, 2014 at 08:07:57PM +0530, Amit Daniel Kachhap wrote:
> This patch adds notification support for those clients of cpu_cooling
> APIs which may want to do something interesting after receiving these
> cpu_cooling events. The notifier structure passed is of both Set/Get type.
> The notfications events can be of type,
> 1. CPU_COOLING_SET_STATE_PRE
> 2. CPU_COOLING_SET_STATE_POST
> 3. CPU_COOLING_GET_CUR_STATE
> 4. CPU_COOLING_GET_MAX_STATE

What do you think about expanding this feature to all cooling devices?

I mean, why should we restrict to cpu(freq) cooling?

The clock cooling device I posted (and that I plan to merge soon) should
also benefit of this infrastructure. The clock cooling is for the
context of other processing units, that do not benefit of cpufreq.


> 
> 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. Also some more cooling
> states can be enabled when the maximum of P state is reached. Post notifications
> can be used for those cases.
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  drivers/thermal/cpu_cooling.c |   99 +++++++++++++++++++++++++++++++++++++++-
>  include/linux/cpu_cooling.h   |   55 +++++++++++++++++++++++
>  2 files changed, 151 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index 21f44d4..e2aeb36 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -50,6 +50,7 @@ struct cpufreq_cooling_device {
>  	unsigned int cpufreq_state;
>  	unsigned int cpufreq_val;
>  	struct cpumask allowed_cpus;
> +	struct cpufreq_cooling_status request_status;
>  	struct list_head node;
>  };
>  static DEFINE_IDR(cpufreq_idr);
> @@ -59,6 +60,8 @@ static DEFINE_MUTEX(cooling_cpufreq_lock);
>  #define NOTIFY_INVALID NULL
>  static struct cpufreq_cooling_device *notify_device;
>  
> +/* Notfier list to validates/updates the cpufreq cooling states */
> +static BLOCKING_NOTIFIER_HEAD(cpufreq_cooling_state_notifier_list);
>  /* A list to hold all the cpufreq cooling devices registered */
>  static LIST_HEAD(cpufreq_cooling_list);
>  
> @@ -266,6 +269,21 @@ static unsigned int get_cpu_frequency(unsigned int cpu, unsigned long level)
>  	return freq;
>  }
>  
> +static int
> +cpufreq_cooling_notify_states(struct cpufreq_cooling_status *request,
> +			enum cpu_cooling_state_ops op)
> +{
> +	/* Invoke the notifiers which have registered for this state change */
> +	if (op == CPU_COOLING_SET_STATE_PRE ||
> +		op == CPU_COOLING_SET_STATE_POST ||
> +		op == CPU_COOLING_GET_MAX_STATE ||
> +		op == CPU_COOLING_GET_CUR_STATE) {
> +		blocking_notifier_call_chain(
> +			&cpufreq_cooling_state_notifier_list, op, request);
> +	}
> +	return 0;
> +}
> +
>  /**
>   * cpufreq_apply_cooling - function to apply frequency clipping.
>   * @cpufreq_device: cpufreq_cooling_device pointer containing frequency
> @@ -285,9 +303,18 @@ static int cpufreq_apply_cooling(struct cpufreq_cooling_device *cpufreq_device,
>  	struct cpumask *mask = &cpufreq_device->allowed_cpus;
>  	unsigned int cpu = cpumask_any(mask);
>  
> +	cpufreq_device->request_status.cur_state =
> +						cpufreq_device->cpufreq_state;
> +	cpufreq_device->request_status.new_state = cooling_state;
> +
> +	cpufreq_cooling_notify_states(&cpufreq_device->request_status,
> +					CPU_COOLING_SET_STATE_PRE);
> +

Can a listener block/abort/postpone a state transition?

> +	cooling_state = cpufreq_device->request_status.new_state;
>  

So you are proposing listeners can have control on target cpufreq state,
right? Or did I get it wrong?

I think we should have this better documented. Besides, if that is the
case, what happens if we have several listeners?

>  	/* Check if the old cooling action is same as new cooling action */
> -	if (cpufreq_device->cpufreq_state == cooling_state)
> +	if (cpufreq_device->cpufreq_state ==
> +				cpufreq_device->request_status.new_state)
>  		return 0;
>  
>  	clip_freq = get_cpu_frequency(cpu, cooling_state);
> @@ -304,7 +331,8 @@ static int cpufreq_apply_cooling(struct cpufreq_cooling_device *cpufreq_device,
>  	}
>  
>  	notify_device = NOTIFY_INVALID;
> -
> +	cpufreq_cooling_notify_states(&cpufreq_device->request_status,
> +					CPU_COOLING_SET_STATE_POST);
>  	return 0;
>  }
>  
> @@ -383,6 +411,11 @@ static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
>  	if (count > 0)
>  		*state = count;
>  
> +	cpufreq_device->request_status.max_state = count;
> +	cpufreq_cooling_notify_states(&cpufreq_device->request_status,
> +					CPU_COOLING_GET_MAX_STATE);
> +	*state = cpufreq_device->request_status.max_state;
> +

same question here, who determines the priorities when several listeners
are present in the system?

>  	return ret;
>  }
>  
> @@ -410,8 +443,12 @@ static int cpufreq_get_cur_state(struct thermal_cooling_device *cdev,
>  		/* Cooling device pointer not found */
>  		return -EINVAL;
>  	}
> +	cpufreq_device->request_status.cur_state =
> +						cpufreq_device->cpufreq_state;
> +	cpufreq_cooling_notify_states(&cpufreq_device->request_status,
> +					CPU_COOLING_GET_CUR_STATE);
>  

ditto.

> -	*state = cpufreq_device->cpufreq_state;
> +	*state = cpufreq_device->request_status.cur_state;
>  
>  	return 0;
>  }
> @@ -520,6 +557,7 @@ __cpufreq_cooling_register(struct device_node *np,
>  	}
>  	cpufreq_dev->cool_dev = cool_dev;
>  	cpufreq_dev->cpufreq_state = 0;
> +	cpufreq_dev->request_status.devdata = devdata;
>  	mutex_lock(&cooling_cpufreq_lock);
>  
>  	/* Register the notifier for first cpufreq cooling device */
> @@ -614,3 +652,58 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
>  	kfree(cpufreq_dev);
>  }
>  EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister);
> +
> +/**
> + * cpufreq_cooling_register_notifier - register a driver with cpufreq cooling.
> + * @nb:	notifier function to register.
> + * @list: CPU_COOLING_STATE_NOTIFIER type.
> + *
> + * Add a driver to receive cpufreq cooling notifications like current state,
> + * max state and set state. The drivers after reading the events can perform
> + * some mapping between the P states and cooling states like grouping some P
> + * states into 1 cooling state.
> + *
> + * Return: 0 (success)
> + */
> +int cpufreq_cooling_register_notifier(struct notifier_block *nb,
> +					unsigned int list)
> +{
> +	int ret = 0;
> +	switch (list) {
> +	case CPU_COOLING_STATE_NOTIFIER:
> +		ret = blocking_notifier_chain_register(
> +				&cpufreq_cooling_state_notifier_list, nb);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(cpufreq_cooling_register_notifier);
> +
> +/**
> + * cpufreq_cooling_unregister_notifier - unregisters a driver with cpufreq
> + * cooling.
> + * @nb:	notifier function to unregister.
> + * @list: CPU_COOLING_STATE_NOTIFIER type.
> + *
> + * Removes a driver to receive further cpufreq cooling notifications.
> + *
> + * Return: 0 (success)
> + */
> +int cpufreq_cooling_unregister_notifier(struct notifier_block *nb,
> +					unsigned int list)
> +{
> +	int ret = 0;
> +	switch (list) {
> +	case CPU_COOLING_STATE_NOTIFIER:
> +		ret = blocking_notifier_chain_unregister(
> +				&cpufreq_cooling_state_notifier_list, nb);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister_notifier);

The exported symbols must be documented under Documentation/ too.

> diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
> index aaef7d8..f786935 100644
> --- a/include/linux/cpu_cooling.h
> +++ b/include/linux/cpu_cooling.h
> @@ -28,6 +28,22 @@
>  #include <linux/thermal.h>
>  #include <linux/cpumask.h>
>  
> +#define CPU_COOLING_STATE_NOTIFIER	(0)
> +
> +enum cpu_cooling_state_ops {
> +	CPU_COOLING_SET_STATE_PRE,
> +	CPU_COOLING_SET_STATE_POST,
> +	CPU_COOLING_GET_CUR_STATE,
> +	CPU_COOLING_GET_MAX_STATE,
> +};
> +
> +struct cpufreq_cooling_status {
> +	unsigned long cur_state;
> +	unsigned long new_state;
> +	unsigned long max_state;
> +	void *devdata;
> +};
> +
>  #ifdef CONFIG_CPU_THERMAL
>  /**
>   * cpufreq_cooling_register - function to create cpufreq cooling device.
> @@ -62,6 +78,34 @@ 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);
> +
> +/**
> + * cpufreq_cooling_register_notifier - register a driver with cpufreq cooling.
> + * @nb:	notifier function to register.
> + * @list: CPU_COOLING_STATE_NOTIFIER type.
> + *
> + * Add a driver to receive cpufreq cooling notifications like current state,
> + * max state and set state. The drivers after reading the events can perform
> + * some mapping between the P states and cooling states like grouping some P
> + * states into 1 cooling state.
> + *
> + * Return: 0 (success)
> + */
> +int cpufreq_cooling_register_notifier(struct notifier_block *nb,
> +					unsigned int list);
> +
> +/**
> + * cpufreq_cooling_unregister_notifier - unregisters a driver with cpufreq
> + * cooling.
> + * @nb:	notifier function to unregister.
> + * @list: CPU_COOLING_STATE_NOTIFIER type.
> + *
> + * Removes a driver to receive further cpufreq cooling notifications.
> + *
> + * Return: 0 (success)
> + */
> +int cpufreq_cooling_unregister_notifier(struct notifier_block *nb,
> +					unsigned int list);
>  #else /* !CONFIG_CPU_THERMAL */
>  static inline struct thermal_cooling_device *
>  cpufreq_cooling_register(const struct cpumask *clip_cpus, void *devdata)
> @@ -84,6 +128,17 @@ unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq)
>  {
>  	return THERMAL_CSTATE_INVALID;
>  }
> +static inline int
> +cpufreq_cooling_register_notifier(struct notifier_block *nb, unsigned int list)
> +{
> +	return 0;
> +}
> +static inline int
> +cpufreq_cooling_unregister_notifier(struct notifier_block *nb,
> +					unsigned int list)
> +{
> +	return 0;
> +}
>  #endif	/* CONFIG_CPU_THERMAL */
>  
>  #endif /* __CPU_COOLING_H__ */
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[linux-pm] [RFC PATCH 2/5] thermal: cpu_cooling: Add notifications support for the clients

[amit daniel kachhap]

On Fri, May 16, 2014 at 12:15 AM, Eduardo Valentin <edubezval@gmail.com> wrote:
> Hello Amit,
>
>
> On Thu, May 08, 2014 at 08:07:57PM +0530, Amit Daniel Kachhap wrote:
>> This patch adds notification support for those clients of cpu_cooling
>> APIs which may want to do something interesting after receiving these
>> cpu_cooling events. The notifier structure passed is of both Set/Get type.
>> The notfications events can be of type,
>> 1. CPU_COOLING_SET_STATE_PRE
>> 2. CPU_COOLING_SET_STATE_POST
>> 3. CPU_COOLING_GET_CUR_STATE
>> 4. CPU_COOLING_GET_MAX_STATE
>
> What do you think about expanding this feature to all cooling devices?
>
> I mean, why should we restrict to cpu(freq) cooling?
nice suggestion. Sure will check its feasibility.
>
> The clock cooling device I posted (and that I plan to merge soon) should
> also benefit of this infrastructure. The clock cooling is for the
> context of other processing units, that do not benefit of cpufreq.
ok let me check them.
>
>
>>
>> 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. Also some more cooling
>> states can be enabled when the maximum of P state is reached. Post notifications
>> can be used for those cases.
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  drivers/thermal/cpu_cooling.c |   99 +++++++++++++++++++++++++++++++++++++++-
>>  include/linux/cpu_cooling.h   |   55 +++++++++++++++++++++++
>>  2 files changed, 151 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
>> index 21f44d4..e2aeb36 100644
>> --- a/drivers/thermal/cpu_cooling.c
>> +++ b/drivers/thermal/cpu_cooling.c
>> @@ -50,6 +50,7 @@ struct cpufreq_cooling_device {
>>       unsigned int cpufreq_state;
>>       unsigned int cpufreq_val;
>>       struct cpumask allowed_cpus;
>> +     struct cpufreq_cooling_status request_status;
>>       struct list_head node;
>>  };
>>  static DEFINE_IDR(cpufreq_idr);
>> @@ -59,6 +60,8 @@ static DEFINE_MUTEX(cooling_cpufreq_lock);
>>  #define NOTIFY_INVALID NULL
>>  static struct cpufreq_cooling_device *notify_device;
>>
>> +/* Notfier list to validates/updates the cpufreq cooling states */
>> +static BLOCKING_NOTIFIER_HEAD(cpufreq_cooling_state_notifier_list);
>>  /* A list to hold all the cpufreq cooling devices registered */
>>  static LIST_HEAD(cpufreq_cooling_list);
>>
>> @@ -266,6 +269,21 @@ static unsigned int get_cpu_frequency(unsigned int cpu, unsigned long level)
>>       return freq;
>>  }
>>
>> +static int
>> +cpufreq_cooling_notify_states(struct cpufreq_cooling_status *request,
>> +                     enum cpu_cooling_state_ops op)
>> +{
>> +     /* Invoke the notifiers which have registered for this state change */
>> +     if (op == CPU_COOLING_SET_STATE_PRE ||
>> +             op == CPU_COOLING_SET_STATE_POST ||
>> +             op == CPU_COOLING_GET_MAX_STATE ||
>> +             op == CPU_COOLING_GET_CUR_STATE) {
>> +             blocking_notifier_call_chain(
>> +                     &cpufreq_cooling_state_notifier_list, op, request);
>> +     }
>> +     return 0;
>> +}
>> +
>>  /**
>>   * cpufreq_apply_cooling - function to apply frequency clipping.
>>   * @cpufreq_device: cpufreq_cooling_device pointer containing frequency
>> @@ -285,9 +303,18 @@ static int cpufreq_apply_cooling(struct cpufreq_cooling_device *cpufreq_device,
>>       struct cpumask *mask = &cpufreq_device->allowed_cpus;
>>       unsigned int cpu = cpumask_any(mask);
>>
>> +     cpufreq_device->request_status.cur_state =
>> +                                             cpufreq_device->cpufreq_state;
>> +     cpufreq_device->request_status.new_state = cooling_state;
>> +
>> +     cpufreq_cooling_notify_states(&cpufreq_device->request_status,
>> +                                     CPU_COOLING_SET_STATE_PRE);
>> +
>
> Can a listener block/abort/postpone a state transition?
>
>> +     cooling_state = cpufreq_device->request_status.new_state;
>>
>
> So you are proposing listeners can have control on target cpufreq state,
> right? Or did I get it wrong?
>
> I think we should have this better documented. Besides, if that is the
> case, what happens if we have several listeners?
>
>>       /* Check if the old cooling action is same as new cooling action */
>> -     if (cpufreq_device->cpufreq_state == cooling_state)
>> +     if (cpufreq_device->cpufreq_state ==
>> +                             cpufreq_device->request_status.new_state)
>>               return 0;
>>
>>       clip_freq = get_cpu_frequency(cpu, cooling_state);
>> @@ -304,7 +331,8 @@ static int cpufreq_apply_cooling(struct cpufreq_cooling_device *cpufreq_device,
>>       }
>>
>>       notify_device = NOTIFY_INVALID;
>> -
>> +     cpufreq_cooling_notify_states(&cpufreq_device->request_status,
>> +                                     CPU_COOLING_SET_STATE_POST);
>>       return 0;
>>  }
>>
>> @@ -383,6 +411,11 @@ static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
>>       if (count > 0)
>>               *state = count;
>>
>> +     cpufreq_device->request_status.max_state = count;
>> +     cpufreq_cooling_notify_states(&cpufreq_device->request_status,
>> +                                     CPU_COOLING_GET_MAX_STATE);
>> +     *state = cpufreq_device->request_status.max_state;
>> +
>
> same question here, who determines the priorities when several listeners
> are present in the system?
>
>>       return ret;
>>  }
>>
>> @@ -410,8 +443,12 @@ static int cpufreq_get_cur_state(struct thermal_cooling_device *cdev,
>>               /* Cooling device pointer not found */
>>               return -EINVAL;
>>       }
>> +     cpufreq_device->request_status.cur_state =
>> +                                             cpufreq_device->cpufreq_state;
>> +     cpufreq_cooling_notify_states(&cpufreq_device->request_status,
>> +                                     CPU_COOLING_GET_CUR_STATE);
>>
>
> ditto.
>
>> -     *state = cpufreq_device->cpufreq_state;
>> +     *state = cpufreq_device->request_status.cur_state;
>>
>>       return 0;
>>  }
>> @@ -520,6 +557,7 @@ __cpufreq_cooling_register(struct device_node *np,
>>       }
>>       cpufreq_dev->cool_dev = cool_dev;
>>       cpufreq_dev->cpufreq_state = 0;
>> +     cpufreq_dev->request_status.devdata = devdata;
>>       mutex_lock(&cooling_cpufreq_lock);
>>
>>       /* Register the notifier for first cpufreq cooling device */
>> @@ -614,3 +652,58 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
>>       kfree(cpufreq_dev);
>>  }
>>  EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister);
>> +
>> +/**
>> + * cpufreq_cooling_register_notifier - register a driver with cpufreq cooling.
>> + * @nb:      notifier function to register.
>> + * @list: CPU_COOLING_STATE_NOTIFIER type.
>> + *
>> + * Add a driver to receive cpufreq cooling notifications like current state,
>> + * max state and set state. The drivers after reading the events can perform
>> + * some mapping between the P states and cooling states like grouping some P
>> + * states into 1 cooling state.
>> + *
>> + * Return: 0 (success)
>> + */
>> +int cpufreq_cooling_register_notifier(struct notifier_block *nb,
>> +                                     unsigned int list)
>> +{
>> +     int ret = 0;
>> +     switch (list) {
>> +     case CPU_COOLING_STATE_NOTIFIER:
>> +             ret = blocking_notifier_chain_register(
>> +                             &cpufreq_cooling_state_notifier_list, nb);
>> +             break;
>> +     default:
>> +             ret = -EINVAL;
>> +     }
>> +     return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(cpufreq_cooling_register_notifier);
>> +
>> +/**
>> + * cpufreq_cooling_unregister_notifier - unregisters a driver with cpufreq
>> + * cooling.
>> + * @nb:      notifier function to unregister.
>> + * @list: CPU_COOLING_STATE_NOTIFIER type.
>> + *
>> + * Removes a driver to receive further cpufreq cooling notifications.
>> + *
>> + * Return: 0 (success)
>> + */
>> +int cpufreq_cooling_unregister_notifier(struct notifier_block *nb,
>> +                                     unsigned int list)
>> +{
>> +     int ret = 0;
>> +     switch (list) {
>> +     case CPU_COOLING_STATE_NOTIFIER:
>> +             ret = blocking_notifier_chain_unregister(
>> +                             &cpufreq_cooling_state_notifier_list, nb);
>> +             break;
>> +     default:
>> +             ret = -EINVAL;
>> +     }
>> +
>> +     return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister_notifier);
>
> The exported symbols must be documented under Documentation/ too.
>
>> diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
>> index aaef7d8..f786935 100644
>> --- a/include/linux/cpu_cooling.h
>> +++ b/include/linux/cpu_cooling.h
>> @@ -28,6 +28,22 @@
>>  #include <linux/thermal.h>
>>  #include <linux/cpumask.h>
>>
>> +#define CPU_COOLING_STATE_NOTIFIER   (0)
>> +
>> +enum cpu_cooling_state_ops {
>> +     CPU_COOLING_SET_STATE_PRE,
>> +     CPU_COOLING_SET_STATE_POST,
>> +     CPU_COOLING_GET_CUR_STATE,
>> +     CPU_COOLING_GET_MAX_STATE,
>> +};
>> +
>> +struct cpufreq_cooling_status {
>> +     unsigned long cur_state;
>> +     unsigned long new_state;
>> +     unsigned long max_state;
>> +     void *devdata;
>> +};
>> +
>>  #ifdef CONFIG_CPU_THERMAL
>>  /**
>>   * cpufreq_cooling_register - function to create cpufreq cooling device.
>> @@ -62,6 +78,34 @@ 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);
>> +
>> +/**
>> + * cpufreq_cooling_register_notifier - register a driver with cpufreq cooling.
>> + * @nb:      notifier function to register.
>> + * @list: CPU_COOLING_STATE_NOTIFIER type.
>> + *
>> + * Add a driver to receive cpufreq cooling notifications like current state,
>> + * max state and set state. The drivers after reading the events can perform
>> + * some mapping between the P states and cooling states like grouping some P
>> + * states into 1 cooling state.
>> + *
>> + * Return: 0 (success)
>> + */
>> +int cpufreq_cooling_register_notifier(struct notifier_block *nb,
>> +                                     unsigned int list);
>> +
>> +/**
>> + * cpufreq_cooling_unregister_notifier - unregisters a driver with cpufreq
>> + * cooling.
>> + * @nb:      notifier function to unregister.
>> + * @list: CPU_COOLING_STATE_NOTIFIER type.
>> + *
>> + * Removes a driver to receive further cpufreq cooling notifications.
>> + *
>> + * Return: 0 (success)
>> + */
>> +int cpufreq_cooling_unregister_notifier(struct notifier_block *nb,
>> +                                     unsigned int list);
>>  #else /* !CONFIG_CPU_THERMAL */
>>  static inline struct thermal_cooling_device *
>>  cpufreq_cooling_register(const struct cpumask *clip_cpus, void *devdata)
>> @@ -84,6 +128,17 @@ unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq)
>>  {
>>       return THERMAL_CSTATE_INVALID;
>>  }
>> +static inline int
>> +cpufreq_cooling_register_notifier(struct notifier_block *nb, unsigned int list)
>> +{
>> +     return 0;
>> +}
>> +static inline int
>> +cpufreq_cooling_unregister_notifier(struct notifier_block *nb,
>> +                                     unsigned int list)
>> +{
>> +     return 0;
>> +}
>>  #endif       /* CONFIG_CPU_THERMAL */
>>
>>  #endif /* __CPU_COOLING_H__ */
>> --
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/

[RFC PATCH 2/5] thermal: cpu_cooling: Add notifications support for the clients

[Javi Merino]

Hi Amit,

On Thu, May 08, 2014 at 03:37:57PM +0100, Amit Daniel Kachhap wrote:
> This patch adds notification support for those clients of cpu_cooling
> APIs which may want to do something interesting after receiving these
> cpu_cooling events. The notifier structure passed is of both Set/Get type.
> The notfications events can be of type,
> 1. CPU_COOLING_SET_STATE_PRE
> 2. CPU_COOLING_SET_STATE_POST
> 3. CPU_COOLING_GET_CUR_STATE
> 4. CPU_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. Also some more cooling
> states can be enabled when the maximum of P state is reached. Post notifications
> can be used for those cases.
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  drivers/thermal/cpu_cooling.c |   99 +++++++++++++++++++++++++++++++++++++++-
>  include/linux/cpu_cooling.h   |   55 +++++++++++++++++++++++
>  2 files changed, 151 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index 21f44d4..e2aeb36 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -50,6 +50,7 @@ struct cpufreq_cooling_device {
>  	unsigned int cpufreq_state;
>  	unsigned int cpufreq_val;
>  	struct cpumask allowed_cpus;
> +	struct cpufreq_cooling_status request_status;
>  	struct list_head node;
>  };
>  static DEFINE_IDR(cpufreq_idr);
> @@ -59,6 +60,8 @@ static DEFINE_MUTEX(cooling_cpufreq_lock);
>  #define NOTIFY_INVALID NULL
>  static struct cpufreq_cooling_device *notify_device;
>  
> +/* Notfier list to validates/updates the cpufreq cooling states */
> +static BLOCKING_NOTIFIER_HEAD(cpufreq_cooling_state_notifier_list);
>  /* A list to hold all the cpufreq cooling devices registered */
>  static LIST_HEAD(cpufreq_cooling_list);
>  
> @@ -266,6 +269,21 @@ static unsigned int get_cpu_frequency(unsigned int cpu, unsigned long level)
>  	return freq;
>  }
>  
> +static int
> +cpufreq_cooling_notify_states(struct cpufreq_cooling_status *request,
> +			enum cpu_cooling_state_ops op)
> +{
> +	/* Invoke the notifiers which have registered for this state change */
> +	if (op == CPU_COOLING_SET_STATE_PRE ||
> +		op == CPU_COOLING_SET_STATE_POST ||
> +		op == CPU_COOLING_GET_MAX_STATE ||
> +		op == CPU_COOLING_GET_CUR_STATE) {
> +		blocking_notifier_call_chain(
> +			&cpufreq_cooling_state_notifier_list, op, request);
> +	}
> +	return 0;
> +}
> +
>  /**
>   * cpufreq_apply_cooling - function to apply frequency clipping.
>   * @cpufreq_device: cpufreq_cooling_device pointer containing frequency
> @@ -285,9 +303,18 @@ static int cpufreq_apply_cooling(struct cpufreq_cooling_device *cpufreq_device,
>  	struct cpumask *mask = &cpufreq_device->allowed_cpus;
>  	unsigned int cpu = cpumask_any(mask);
>  
> +	cpufreq_device->request_status.cur_state =
> +						cpufreq_device->cpufreq_state;
> +	cpufreq_device->request_status.new_state = cooling_state;
> +
> +	cpufreq_cooling_notify_states(&cpufreq_device->request_status,
> +					CPU_COOLING_SET_STATE_PRE);
> +
> +	cooling_state = cpufreq_device->request_status.new_state;
>  
>  	/* Check if the old cooling action is same as new cooling action */
> -	if (cpufreq_device->cpufreq_state == cooling_state)
> +	if (cpufreq_device->cpufreq_state ==
> +				cpufreq_device->request_status.new_state)
>  		return 0;
>  
>  	clip_freq = get_cpu_frequency(cpu, cooling_state);
> @@ -304,7 +331,8 @@ static int cpufreq_apply_cooling(struct cpufreq_cooling_device *cpufreq_device,
>  	}
>  
>  	notify_device = NOTIFY_INVALID;
> -
> +	cpufreq_cooling_notify_states(&cpufreq_device->request_status,
> +					CPU_COOLING_SET_STATE_POST);
>  	return 0;
>  }
>  
> @@ -383,6 +411,11 @@ static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
>  	if (count > 0)
>  		*state = count;
>  
> +	cpufreq_device->request_status.max_state = count;
> +	cpufreq_cooling_notify_states(&cpufreq_device->request_status,
> +					CPU_COOLING_GET_MAX_STATE);
> +	*state = cpufreq_device->request_status.max_state;
> +

I think this should all be inside the "if (count > 0)".  If not, then
remove it, as it is dead code now.

Cheers,
Javi

[RFC PATCH 2/5] thermal: cpu_cooling: Add notifications support for the clients

[amit daniel kachhap]

On Fri, May 16, 2014 at 11:02 PM, Javi Merino <javi.merino@arm.com> wrote:
> Hi Amit,
>
> On Thu, May 08, 2014 at 03:37:57PM +0100, Amit Daniel Kachhap wrote:
>> This patch adds notification support for those clients of cpu_cooling
>> APIs which may want to do something interesting after receiving these
>> cpu_cooling events. The notifier structure passed is of both Set/Get type.
>> The notfications events can be of type,
>> 1. CPU_COOLING_SET_STATE_PRE
>> 2. CPU_COOLING_SET_STATE_POST
>> 3. CPU_COOLING_GET_CUR_STATE
>> 4. CPU_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. Also some more cooling
>> states can be enabled when the maximum of P state is reached. Post notifications
>> can be used for those cases.
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  drivers/thermal/cpu_cooling.c |   99 +++++++++++++++++++++++++++++++++++++++-
>>  include/linux/cpu_cooling.h   |   55 +++++++++++++++++++++++
>>  2 files changed, 151 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
>> index 21f44d4..e2aeb36 100644
>> --- a/drivers/thermal/cpu_cooling.c
>> +++ b/drivers/thermal/cpu_cooling.c
>> @@ -50,6 +50,7 @@ struct cpufreq_cooling_device {
>>       unsigned int cpufreq_state;
>>       unsigned int cpufreq_val;
>>       struct cpumask allowed_cpus;
>> +     struct cpufreq_cooling_status request_status;
>>       struct list_head node;
>>  };
>>  static DEFINE_IDR(cpufreq_idr);
>> @@ -59,6 +60,8 @@ static DEFINE_MUTEX(cooling_cpufreq_lock);
>>  #define NOTIFY_INVALID NULL
>>  static struct cpufreq_cooling_device *notify_device;
>>
>> +/* Notfier list to validates/updates the cpufreq cooling states */
>> +static BLOCKING_NOTIFIER_HEAD(cpufreq_cooling_state_notifier_list);
>>  /* A list to hold all the cpufreq cooling devices registered */
>>  static LIST_HEAD(cpufreq_cooling_list);
>>
>> @@ -266,6 +269,21 @@ static unsigned int get_cpu_frequency(unsigned int cpu, unsigned long level)
>>       return freq;
>>  }
>>
>> +static int
>> +cpufreq_cooling_notify_states(struct cpufreq_cooling_status *request,
>> +                     enum cpu_cooling_state_ops op)
>> +{
>> +     /* Invoke the notifiers which have registered for this state change */
>> +     if (op == CPU_COOLING_SET_STATE_PRE ||
>> +             op == CPU_COOLING_SET_STATE_POST ||
>> +             op == CPU_COOLING_GET_MAX_STATE ||
>> +             op == CPU_COOLING_GET_CUR_STATE) {
>> +             blocking_notifier_call_chain(
>> +                     &cpufreq_cooling_state_notifier_list, op, request);
>> +     }
>> +     return 0;
>> +}
>> +
>>  /**
>>   * cpufreq_apply_cooling - function to apply frequency clipping.
>>   * @cpufreq_device: cpufreq_cooling_device pointer containing frequency
>> @@ -285,9 +303,18 @@ static int cpufreq_apply_cooling(struct cpufreq_cooling_device *cpufreq_device,
>>       struct cpumask *mask = &cpufreq_device->allowed_cpus;
>>       unsigned int cpu = cpumask_any(mask);
>>
>> +     cpufreq_device->request_status.cur_state =
>> +                                             cpufreq_device->cpufreq_state;
>> +     cpufreq_device->request_status.new_state = cooling_state;
>> +
>> +     cpufreq_cooling_notify_states(&cpufreq_device->request_status,
>> +                                     CPU_COOLING_SET_STATE_PRE);
>> +
>> +     cooling_state = cpufreq_device->request_status.new_state;
>>
>>       /* Check if the old cooling action is same as new cooling action */
>> -     if (cpufreq_device->cpufreq_state == cooling_state)
>> +     if (cpufreq_device->cpufreq_state ==
>> +                             cpufreq_device->request_status.new_state)
>>               return 0;
>>
>>       clip_freq = get_cpu_frequency(cpu, cooling_state);
>> @@ -304,7 +331,8 @@ static int cpufreq_apply_cooling(struct cpufreq_cooling_device *cpufreq_device,
>>       }
>>
>>       notify_device = NOTIFY_INVALID;
>> -
>> +     cpufreq_cooling_notify_states(&cpufreq_device->request_status,
>> +                                     CPU_COOLING_SET_STATE_POST);
>>       return 0;
>>  }
>>
>> @@ -383,6 +411,11 @@ static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
>>       if (count > 0)
>>               *state = count;
>>
>> +     cpufreq_device->request_status.max_state = count;
>> +     cpufreq_cooling_notify_states(&cpufreq_device->request_status,
>> +                                     CPU_COOLING_GET_MAX_STATE);
>> +     *state = cpufreq_device->request_status.max_state;
>> +
>
> I think this should all be inside the "if (count > 0)".  If not, then
> remove it, as it is dead code now.
yes this looks buggy. Will remove them. thanks
>
> Cheers,
> Javi
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH 3/5] thermal: cpu_cooling: Add support to find nearby frequency levels.

[Amit Daniel Kachhap]

This patch adds suuport 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>
---
 drivers/thermal/cpu_cooling.c                   |   42 +++++++++++++++++------
 drivers/thermal/samsung/exynos_thermal_common.c |    3 +-
 include/linux/cpu_cooling.h                     |   23 ++++++++++++-
 3 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index e2aeb36..6f5430e 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -120,12 +120,7 @@ static int is_cpufreq_valid(int cpu)
 	return !cpufreq_get_policy(&policy, cpu);
 }
 
-enum cpufreq_cooling_property {
-	GET_LEVEL,
-	GET_FREQ,
-	GET_MAXL,
-};
-
+#define GET_FREQ (GET_MAXL + 1)
 /**
  * get_property - fetch a property of interest for a give cpu.
  * @cpu: cpu for which the property is required
@@ -207,15 +202,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++;
 	}
@@ -227,6 +244,8 @@ static int get_property(unsigned int cpu, unsigned long input,
  * cpufreq_cooling_get_level - for a give cpu, return the cooling level.
  * @cpu: cpu for which the level is required
  * @freq: the frequency of interest
+ * @property: can be GET_LEVEL_CEIL, GET_LEVEL_FLOOR, GET_LEVEL_EXACT or
+ * GET_MAXL
  *
  * This function will match the cooling level corresponding to the
  * requested @freq and return it.
@@ -234,11 +253,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 f786935..f4d2b0e 100644
--- a/include/linux/cpu_cooling.h
+++ b/include/linux/cpu_cooling.h
@@ -44,6 +44,13 @@ struct cpufreq_cooling_status {
 	void *devdata;
 };
 
+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.
@@ -77,7 +84,21 @@ 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);
+/**
+ * cpufreq_cooling_get_level - for a give cpu, return the cooling level.
+ * @cpu: cpu for which the level is required
+ * @freq: the frequency of interest
+ * @property: can be GET_LEVEL_CEIL, GET_LEVEL_FLOOR, GET_LEVEL_EXACT or
+ * GET_MAXL
+ *
+ * This function will match the cooling level corresponding to the
+ * requested @freq and return it.
+ *
+ * Return: The matched cooling level on success or THERMAL_CSTATE_INVALID
+ * otherwise.
+ */
+unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq,
+					enum cpufreq_cooling_property);
 
 /**
  * cpufreq_cooling_register_notifier - register a driver with cpufreq cooling.
-- 
1.7.1

[RFC PATCH 3/5] thermal: cpu_cooling: Add support to find nearby frequency levels.

[Eduardo Valentin]

Hello Amit,

On Thu, May 08, 2014 at 08:07:58PM +0530, Amit Daniel Kachhap wrote:
> This patch adds suuport 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>
> ---
>  drivers/thermal/cpu_cooling.c                   |   42 +++++++++++++++++------
>  drivers/thermal/samsung/exynos_thermal_common.c |    3 +-
>  include/linux/cpu_cooling.h                     |   23 ++++++++++++-
>  3 files changed, 55 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index e2aeb36..6f5430e 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -120,12 +120,7 @@ static int is_cpufreq_valid(int cpu)
>  	return !cpufreq_get_policy(&policy, cpu);
>  }
>  
> -enum cpufreq_cooling_property {
> -	GET_LEVEL,
> -	GET_FREQ,
> -	GET_MAXL,
> -};
> -
> +#define GET_FREQ (GET_MAXL + 1)

What happens if we add another level manipulating after GET_MAXL? say,
GET_MINL?

>  /**
>   * get_property - fetch a property of interest for a give cpu.
>   * @cpu: cpu for which the property is required
> @@ -207,15 +202,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++;
>  	}
> @@ -227,6 +244,8 @@ static int get_property(unsigned int cpu, unsigned long input,
>   * cpufreq_cooling_get_level - for a give cpu, return the cooling level.
>   * @cpu: cpu for which the level is required
>   * @freq: the frequency of interest
> + * @property: can be GET_LEVEL_CEIL, GET_LEVEL_FLOOR, GET_LEVEL_EXACT or
> + * GET_MAXL
>   *
>   * This function will match the cooling level corresponding to the
>   * requested @freq and return it.
> @@ -234,11 +253,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 f786935..f4d2b0e 100644
> --- a/include/linux/cpu_cooling.h
> +++ b/include/linux/cpu_cooling.h
> @@ -44,6 +44,13 @@ struct cpufreq_cooling_status {
>  	void *devdata;
>  };
>  
> +enum cpufreq_cooling_property {
> +	GET_LEVEL_CEIL,
> +	GET_LEVEL_FLOOR,
> +	GET_LEVEL_EXACT,
> +	GET_MAXL,
> +};

Please document this enum.

> +
>  #ifdef CONFIG_CPU_THERMAL
>  /**
>   * cpufreq_cooling_register - function to create cpufreq cooling device.
> @@ -77,7 +84,21 @@ 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);
> +/**
> + * cpufreq_cooling_get_level - for a give cpu, return the cooling level.
> + * @cpu: cpu for which the level is required
> + * @freq: the frequency of interest
> + * @property: can be GET_LEVEL_CEIL, GET_LEVEL_FLOOR, GET_LEVEL_EXACT or
> + * GET_MAXL
> + *
> + * This function will match the cooling level corresponding to the
> + * requested @freq and return it.
> + *
> + * Return: The matched cooling level on success or THERMAL_CSTATE_INVALID
> + * otherwise.
> + */
> +unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq,
> +					enum cpufreq_cooling_property);
>  
>  /**
>   * cpufreq_cooling_register_notifier - register a driver with cpufreq cooling.
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[linux-pm] [RFC PATCH 3/5] thermal: cpu_cooling: Add support to find nearby frequency levels.

[amit daniel kachhap]

On Fri, May 16, 2014 at 12:22 AM, Eduardo Valentin <edubezval@gmail.com> wrote:
> Hello Amit,
>
> On Thu, May 08, 2014 at 08:07:58PM +0530, Amit Daniel Kachhap wrote:
>> This patch adds suuport 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>
>> ---
>>  drivers/thermal/cpu_cooling.c                   |   42 +++++++++++++++++------
>>  drivers/thermal/samsung/exynos_thermal_common.c |    3 +-
>>  include/linux/cpu_cooling.h                     |   23 ++++++++++++-
>>  3 files changed, 55 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
>> index e2aeb36..6f5430e 100644
>> --- a/drivers/thermal/cpu_cooling.c
>> +++ b/drivers/thermal/cpu_cooling.c
>> @@ -120,12 +120,7 @@ static int is_cpufreq_valid(int cpu)
>>       return !cpufreq_get_policy(&policy, cpu);
>>  }
>>
>> -enum cpufreq_cooling_property {
>> -     GET_LEVEL,
>> -     GET_FREQ,
>> -     GET_MAXL,
>> -};
>> -
>> +#define GET_FREQ (GET_MAXL + 1)
>
> What happens if we add another level manipulating after GET_MAXL? say,
> GET_MINL?
yes correct. Good catch.
>
>>  /**
>>   * get_property - fetch a property of interest for a give cpu.
>>   * @cpu: cpu for which the property is required
>> @@ -207,15 +202,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++;
>>       }
>> @@ -227,6 +244,8 @@ static int get_property(unsigned int cpu, unsigned long input,
>>   * cpufreq_cooling_get_level - for a give cpu, return the cooling level.
>>   * @cpu: cpu for which the level is required
>>   * @freq: the frequency of interest
>> + * @property: can be GET_LEVEL_CEIL, GET_LEVEL_FLOOR, GET_LEVEL_EXACT or
>> + * GET_MAXL
>>   *
>>   * This function will match the cooling level corresponding to the
>>   * requested @freq and return it.
>> @@ -234,11 +253,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 f786935..f4d2b0e 100644
>> --- a/include/linux/cpu_cooling.h
>> +++ b/include/linux/cpu_cooling.h
>> @@ -44,6 +44,13 @@ struct cpufreq_cooling_status {
>>       void *devdata;
>>  };
>>
>> +enum cpufreq_cooling_property {
>> +     GET_LEVEL_CEIL,
>> +     GET_LEVEL_FLOOR,
>> +     GET_LEVEL_EXACT,
>> +     GET_MAXL,
>> +};
>
> Please document this enum.
>
>> +
>>  #ifdef CONFIG_CPU_THERMAL
>>  /**
>>   * cpufreq_cooling_register - function to create cpufreq cooling device.
>> @@ -77,7 +84,21 @@ 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);
>> +/**
>> + * cpufreq_cooling_get_level - for a give cpu, return the cooling level.
>> + * @cpu: cpu for which the level is required
>> + * @freq: the frequency of interest
>> + * @property: can be GET_LEVEL_CEIL, GET_LEVEL_FLOOR, GET_LEVEL_EXACT or
>> + * GET_MAXL
>> + *
>> + * This function will match the cooling level corresponding to the
>> + * requested @freq and return it.
>> + *
>> + * Return: The matched cooling level on success or THERMAL_CSTATE_INVALID
>> + * otherwise.
>> + */
>> +unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq,
>> +                                     enum cpufreq_cooling_property);
>>
>>  /**
>>   * cpufreq_cooling_register_notifier - register a driver with cpufreq cooling.
>> --
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/

[RFC PATCH 4/5] thermal: cpu_cooling: Release the upper cooling limit checks

[Amit Daniel Kachhap]

This is required as with addition of cpufreq cooling notifiers
mechanism the client can enable some more cooling states at a later
point of time. 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 71b0ec0..36bb107 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -921,7 +921,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))
@@ -939,13 +938,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 f7e11c7..3748e6d 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

[RFC PATCH 4/5] thermal: cpu_cooling: Release the upper cooling limit checks

[Eduardo Valentin]

Hello Amit,

On Thu, May 08, 2014 at 08:07:59PM +0530, Amit Daniel Kachhap wrote:
> This is required as with addition of cpufreq cooling notifiers
> mechanism the client can enable some more cooling states at a later
> point of time. 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 71b0ec0..36bb107 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -921,7 +921,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))
> @@ -939,13 +938,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);
> -

I am not sure I follow why we need to release this check. Can't the user
use the notifier infrastructure and change the max state?

I think I need a better view of use cases where max state would change.



> -	/* 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 f7e11c7..3748e6d 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
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[linux-pm] [RFC PATCH 4/5] thermal: cpu_cooling: Release the upper cooling limit checks

[amit daniel kachhap]

On Fri, May 16, 2014 at 12:28 AM, Eduardo Valentin <edubezval@gmail.com> wrote:
> Hello Amit,
>
> On Thu, May 08, 2014 at 08:07:59PM +0530, Amit Daniel Kachhap wrote:
>> This is required as with addition of cpufreq cooling notifiers
>> mechanism the client can enable some more cooling states at a later
>> point of time. 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 71b0ec0..36bb107 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -921,7 +921,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))
>> @@ -939,13 +938,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);
>> -
>
> I am not sure I follow why we need to release this check. Can't the user
> use the notifier infrastructure and change the max state?
>
> I think I need a better view of use cases where max state would change.
I think i missed adding more description. I will add more details in
the documentation. Actually now with notifier mechanism max state may
change on some condition and at some later time so having this check
here in early stage will not allow taking cooling action for later
dynamic higher cooling states.
>
>
>
>> -     /* 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 f7e11c7..3748e6d 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
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/

[RFC PATCH 5/5] ACPI: thermal: processor: Use the generic cpufreq infrastructure

[Amit Daniel Kachhap]

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 |  210 +++++++++++++++++---------------------
 2 files changed, 99 insertions(+), 117 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..c442a58 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,28 +54,11 @@ 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;
 
 #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 int cpu_has_cpufreq(unsigned int cpu)
 {
 	struct cpufreq_policy policy;
@@ -83,30 +67,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 +83,32 @@ 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 = cpufreq_cooling_register_notifier(&cpufreq_cooling_notifier_block,
+					CPU_COOLING_STATE_NOTIFIER);
 	if (!i)
 		acpi_thermal_cpufreq_is_init = 1;
 }
@@ -158,9 +116,9 @@ 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);
+		cpufreq_cooling_unregister_notifier(
+						&cpufreq_cooling_notifier_block,
+						CPU_COOLING_STATE_NOTIFIER);
 
 	acpi_thermal_cpufreq_is_init = 0;
 }
@@ -176,13 +134,31 @@ static int cpufreq_get_cur_state(unsigned int cpu)
 	return 0;
 }
 
-static int cpufreq_set_cur_state(unsigned int cpu, int state)
+static int acpi_processor_freq_level(unsigned int cpu, int state)
 {
 	return 0;
 }
 
 #endif
 
+/*
+ * 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;
+}
+
 /* thermal cooling device callbacks */
 static int acpi_processor_max_state(struct acpi_processor *pr)
 {
@@ -198,57 +174,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 cpufreq_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 +198,61 @@ 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 == CPU_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 == CPU_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 cpufreq_cooling_status *cooling = data;
+	struct acpi_device *device = cooling->devdata;
+	struct acpi_processor *pr = acpi_driver_data(device);
+	switch (event) {
+	case CPU_COOLING_SET_STATE_PRE:
+	case CPU_COOLING_SET_STATE_POST:
+		acpi_processor_set_cur_state(pr, cooling, event);
+		break;
+	case CPU_COOLING_GET_MAX_STATE:
+		cooling->max_state = acpi_processor_max_state(pr);
+		break;
+	case CPU_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);
+	return cdev;
+}
-- 
1.7.1

[RFC PATCH 5/5] ACPI: thermal: processor: Use the generic cpufreq infrastructure

[Eduardo Valentin]

Hello Amit,

On Thu, May 08, 2014 at 08:08:00PM +0530, 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.
> 

This is a very good move as we are converging towards a more and more
common infrastructure. How did you test this code?

> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  drivers/acpi/processor_driver.c  |    6 +-
>  drivers/acpi/processor_thermal.c |  210 +++++++++++++++++---------------------
>  2 files changed, 99 insertions(+), 117 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..c442a58 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,28 +54,11 @@ 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;
>  
>  #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 int cpu_has_cpufreq(unsigned int cpu)
>  {
>  	struct cpufreq_policy policy;
> @@ -83,30 +67,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 +83,32 @@ 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 = cpufreq_cooling_register_notifier(&cpufreq_cooling_notifier_block,
> +					CPU_COOLING_STATE_NOTIFIER);
>  	if (!i)
>  		acpi_thermal_cpufreq_is_init = 1;
>  }
> @@ -158,9 +116,9 @@ 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);
> +		cpufreq_cooling_unregister_notifier(
> +						&cpufreq_cooling_notifier_block,
> +						CPU_COOLING_STATE_NOTIFIER);
>  
>  	acpi_thermal_cpufreq_is_init = 0;
>  }
> @@ -176,13 +134,31 @@ static int cpufreq_get_cur_state(unsigned int cpu)
>  	return 0;
>  }
>  
> -static int cpufreq_set_cur_state(unsigned int cpu, int state)
> +static int acpi_processor_freq_level(unsigned int cpu, int state)
>  {
>  	return 0;
>  }
>  
>  #endif
>  
> +/*
> + * 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;
> +}
> +
>  /* thermal cooling device callbacks */
>  static int acpi_processor_max_state(struct acpi_processor *pr)
>  {
> @@ -198,57 +174,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 cpufreq_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 +198,61 @@ 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 == CPU_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 == CPU_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 cpufreq_cooling_status *cooling = data;
> +	struct acpi_device *device = cooling->devdata;
> +	struct acpi_processor *pr = acpi_driver_data(device);
> +	switch (event) {
> +	case CPU_COOLING_SET_STATE_PRE:
> +	case CPU_COOLING_SET_STATE_POST:
> +		acpi_processor_set_cur_state(pr, cooling, event);
> +		break;
> +	case CPU_COOLING_GET_MAX_STATE:
> +		cooling->max_state = acpi_processor_max_state(pr);
> +		break;
> +	case CPU_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);
> +	return cdev;
> +}
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[linux-pm] [RFC PATCH 5/5] ACPI: thermal: processor: Use the generic cpufreq infrastructure

[amit daniel kachhap]

On Fri, May 16, 2014 at 12:36 AM, Eduardo Valentin <edubezval@gmail.com> wrote:
> Hello Amit,
>
> On Thu, May 08, 2014 at 08:08:00PM +0530, 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.
>>
>
> This is a very good move as we are converging towards a more and more
> common infrastructure. How did you test this code?
Thanks.
Actually with linaro acpi kernel I was able to enable acpi processor
in my arndale(exynos5250) board. After that in some hacky way I
enabled acpi __TMP method to read its tmu temperatures and able to
test thermal cpufreq scaling. Ofcourse i am not able to test
cputhrottling as it is not supported.

>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  drivers/acpi/processor_driver.c  |    6 +-
>>  drivers/acpi/processor_thermal.c |  210 +++++++++++++++++---------------------
>>  2 files changed, 99 insertions(+), 117 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..c442a58 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,28 +54,11 @@ 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;
>>
>>  #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 int cpu_has_cpufreq(unsigned int cpu)
>>  {
>>       struct cpufreq_policy policy;
>> @@ -83,30 +67,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 +83,32 @@ 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 = cpufreq_cooling_register_notifier(&cpufreq_cooling_notifier_block,
>> +                                     CPU_COOLING_STATE_NOTIFIER);
>>       if (!i)
>>               acpi_thermal_cpufreq_is_init = 1;
>>  }
>> @@ -158,9 +116,9 @@ 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);
>> +             cpufreq_cooling_unregister_notifier(
>> +                                             &cpufreq_cooling_notifier_block,
>> +                                             CPU_COOLING_STATE_NOTIFIER);
>>
>>       acpi_thermal_cpufreq_is_init = 0;
>>  }
>> @@ -176,13 +134,31 @@ static int cpufreq_get_cur_state(unsigned int cpu)
>>       return 0;
>>  }
>>
>> -static int cpufreq_set_cur_state(unsigned int cpu, int state)
>> +static int acpi_processor_freq_level(unsigned int cpu, int state)
>>  {
>>       return 0;
>>  }
>>
>>  #endif
>>
>> +/*
>> + * 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;
>> +}
>> +
>>  /* thermal cooling device callbacks */
>>  static int acpi_processor_max_state(struct acpi_processor *pr)
>>  {
>> @@ -198,57 +174,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 cpufreq_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 +198,61 @@ 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 == CPU_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 == CPU_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 cpufreq_cooling_status *cooling = data;
>> +     struct acpi_device *device = cooling->devdata;
>> +     struct acpi_processor *pr = acpi_driver_data(device);
>> +     switch (event) {
>> +     case CPU_COOLING_SET_STATE_PRE:
>> +     case CPU_COOLING_SET_STATE_POST:
>> +             acpi_processor_set_cur_state(pr, cooling, event);
>> +             break;
>> +     case CPU_COOLING_GET_MAX_STATE:
>> +             cooling->max_state = acpi_processor_max_state(pr);
>> +             break;
>> +     case CPU_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);
>> +     return cdev;
>> +}
>> --
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/