[RFC 0/1] thermal: rcar-thermal: add cpu cooling

[RFC 0/1] thermal: rcar-thermal: add cpu cooling

[Patrick Titiano]

Hi,
This not-yet-ready-to-merge patch proposes to enable CPU thermal management on
Renesas platform. Currently these platforms only register a thermal zone,
but do not implement or register any cooling device.

With this patch:
 - CPU0 cpufreq driver is registered as a cooling device,
 - a new passive trip point is defined to trigger CPU scaling when temperature
   crosses it,
 - CPU0 cooling device and thermal zone 0 are being bound together,
 - set_trip_temp callback is implemented to facilitate experimentation and 
   platform tuning. 

Developed on a RCar-H2 platform, it is intended to support multi-platform.
It leverages CPUFreq support from the series Benoit Cousson recently shared
(http://www.spinics.net/lists/linux-sh/msg29236.html).
The passive trip point temperature is arbitrarily set to 70C, as no HW data was
available at the time this patch was developed.

Being multi-platform, this patch is expected to run smoothly in any situation:
 1/ Platform has no thermal sensor,
 2/ Platform has a thermal sensor and is DVFS capable,
 3/ Platform has a thermal sensor but is not DVFS capable.

This patch has no side-effect in case 1/ (thermal driver wouldn't be loaded).
Case 2/ is the regular case, no issue.
case 3/ is the problematic one.
Using cpufreq driver as a cooling device adds a dependency between those 2
drivers. This is taken care by deferring rcar-thermal probing until
cpufreq driver is up, but this becomes an issue when a platform does
not have DVFS support. cpufreq driver will never be probed, blocking
rcar-thermal probing.

We are still investigation what would be the best way to go, however we would
appreciate a lot any advice from the community.
Thanks!

Regards,
Patrick.

Patrick Titiano (1):
  thermal: rcar-thermal: add cpu cooling

 drivers/thermal/rcar_thermal.c | 103 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 96 insertions(+), 7 deletions(-)

-- 
1.8.3.2

[RFC 1/1] thermal: rcar-thermal: add cpu cooling

[Patrick Titiano]

Renesas RCar platforms only register a thermal zone, but do not implement
any thermal management.
Use CPU cooling (CPUFreq DVFS driver) to implement thermal management.
Register cpufreq cooling device, add passive trip point and bind it
to thermal zone 0.
Passive trip point temperature is set to 70C, but is adjustable
via sysfs to facilitate experimentation and platform tuning.

Signed-off-by: Patrick Titiano <ptitiano@baylibre.com>
---
 drivers/thermal/rcar_thermal.c | 103 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 96 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index 187693c..0f953cf0 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -31,6 +31,8 @@
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/thermal.h>
+#include <linux/cpufreq.h>
+#include <linux/cpu_cooling.h>
 
 #define IDLE_INTERVAL	5000
 
@@ -62,11 +64,14 @@ struct rcar_thermal_priv {
 	void __iomem *base;
 	struct rcar_thermal_common *common;
 	struct thermal_zone_device *zone;
+	struct thermal_cooling_device *cpucooldev;
 	struct delayed_work work;
 	struct mutex lock;
 	struct list_head list;
 	int id;
 	int ctemp;
+	int trip_ctemp_passive;
+	int trip_ctemp_critical;
 };
 
 #define rcar_thermal_for_each_priv(pos, common)	\
@@ -78,6 +83,9 @@ struct rcar_thermal_priv {
 #define rcar_has_irq_support(priv)	((priv)->common->base)
 #define rcar_id_to_shift(priv)		((priv)->id * 8)
 
+#define TRIP_CTEMP_PASSIVE		MCELSIUS(70)
+#define TRIP_CTEMP_CRITICAL		MCELSIUS(90)
+
 #ifdef DEBUG
 # define rcar_force_update_temp(priv)	1
 #else
@@ -224,9 +232,11 @@ static int rcar_thermal_get_trip_type(struct thermal_zone_device *zone,
 	struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
 	struct device *dev = rcar_priv_to_dev(priv);
 
-	/* see rcar_thermal_get_temp() */
 	switch (trip) {
-	case 0: /* +90 <= temp */
+	case 0:
+		*type = THERMAL_TRIP_PASSIVE;
+		break;
+	case 1:
 		*type = THERMAL_TRIP_CRITICAL;
 		break;
 	default:
@@ -243,15 +253,35 @@ static int rcar_thermal_get_trip_temp(struct thermal_zone_device *zone,
 	struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
 	struct device *dev = rcar_priv_to_dev(priv);
 
-	/* see rcar_thermal_get_temp() */
+	mutex_lock(&priv->lock);
 	switch (trip) {
-	case 0: /* +90 <= temp */
-		*temp = MCELSIUS(90);
+	case 0:
+		*temp = priv->trip_ctemp_passive;
+		break;
+	case 1:
+		*temp = priv->trip_ctemp_critical;
 		break;
 	default:
 		dev_err(dev, "rcar driver trip error\n");
 		return -EINVAL;
 	}
+	mutex_unlock(&priv->lock);
+
+	return 0;
+}
+
+static int rcar_thermal_set_trip_temp(struct thermal_zone_device *zone,
+				      int trip, unsigned long temp)
+{
+	struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
+
+	/* Do not allow PASSIVE trip temperature > CRITICAL trip temperature */
+	if (temp >= priv->trip_ctemp_critical)
+		return -EINVAL;
+
+	mutex_lock(&priv->lock);
+	priv->trip_ctemp_passive = temp;
+	mutex_unlock(&priv->lock);
 
 	return 0;
 }
@@ -274,11 +304,52 @@ static int rcar_thermal_notify(struct thermal_zone_device *zone,
 	return 0;
 }
 
+static int rcar_thermal_bind(struct thermal_zone_device *zone,
+			   struct thermal_cooling_device *cdev)
+{
+	int ret;
+
+	ret = thermal_zone_bind_cooling_device(zone, 0, cdev,
+	/* bind with min and max states defined by cpu_cooling */
+						THERMAL_NO_LIMIT,
+						THERMAL_NO_LIMIT);
+	ret += thermal_zone_bind_cooling_device(zone, 1, cdev,
+	/* bind with min and max states defined by cpu_cooling */
+						THERMAL_NO_LIMIT,
+						THERMAL_NO_LIMIT);
+
+	return ret;
+}
+
+/* Unbind callback functions for thermal zone */
+static int rcar_thermal_unbind(struct thermal_zone_device *zone,
+			     struct thermal_cooling_device *cdev)
+{
+	int ret;
+
+	ret = thermal_zone_unbind_cooling_device(zone, 0, cdev);
+	ret += thermal_zone_unbind_cooling_device(zone, 1, cdev);
+
+	return ret;
+}
+
+static int rcar_thermal_get_mode(struct thermal_zone_device *zone,
+			 enum thermal_device_mode *mode)
+{
+	*mode = THERMAL_DEVICE_ENABLED;
+
+	return 0;
+}
+
 static struct thermal_zone_device_ops rcar_thermal_zone_ops = {
+	.get_mode       = rcar_thermal_get_mode,
 	.get_temp	= rcar_thermal_get_temp,
 	.get_trip_type	= rcar_thermal_get_trip_type,
 	.get_trip_temp	= rcar_thermal_get_trip_temp,
+	.set_trip_temp	= rcar_thermal_set_trip_temp,
 	.notify		= rcar_thermal_notify,
+	.bind           = rcar_thermal_bind,
+	.unbind         = rcar_thermal_unbind,
 };
 
 /*
@@ -375,6 +446,12 @@ static int rcar_thermal_probe(struct platform_device *pdev)
 	int i;
 	int ret = -ENODEV;
 	int idle = IDLE_INTERVAL;
+	struct cpumask clip_cpus;
+
+	if (!cpufreq_get_current_driver()) {
+		dev_dbg(dev, "no cpufreq driver yet, deferring probe\n");
+		return -EPROBE_DEFER;
+	}
 
 	common = devm_kzalloc(dev, sizeof(*common), GFP_KERNEL);
 	if (!common) {
@@ -456,13 +533,24 @@ static int rcar_thermal_probe(struct platform_device *pdev)
 
 		priv->common = common;
 		priv->id = i;
+		priv->trip_ctemp_passive = TRIP_CTEMP_PASSIVE;
+		priv->trip_ctemp_critical = TRIP_CTEMP_CRITICAL;
 		mutex_init(&priv->lock);
 		INIT_LIST_HEAD(&priv->list);
 		INIT_DELAYED_WORK(&priv->work, rcar_thermal_work);
 		rcar_thermal_update_temp(priv);
 
+		cpumask_set_cpu(0, &clip_cpus);
+		priv->cpucooldev = cpufreq_cooling_register(&clip_cpus);
+		if (IS_ERR(priv->cpucooldev)) {
+			ret = PTR_ERR(priv->cpucooldev);
+			dev_err(dev, "failed to register cpufreq cooling device (%d)\n",
+				ret);
+			goto error_unpm;
+		}
+
 		priv->zone = thermal_zone_device_register("rcar_thermal",
-						1, 0, priv,
+						2, 1, priv,
 						&rcar_thermal_zone_ops, NULL, 0,
 						idle);
 		if (IS_ERR(priv->zone)) {
@@ -478,7 +566,6 @@ static int rcar_thermal_probe(struct platform_device *pdev)
 	}
 
 	platform_set_drvdata(pdev, common);
-
 	dev_info(dev, "%d sensor probed\n", i);
 
 	return 0;
@@ -513,6 +600,8 @@ static int rcar_thermal_remove(struct platform_device *pdev)
 			rcar_thermal_irq_disable(priv);
 	}
 
+	cpufreq_cooling_unregister(priv->cpucooldev);
+
 	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);
 
-- 
1.8.3.2

Re: [RFC 1/1] thermal: rcar-thermal: add cpu cooling

[Zhang Rui]

On Fri, 2014-02-28 at 16:02 +0100, Patrick Titiano wrote:
> Renesas RCar platforms only register a thermal zone, but do not implement
> any thermal management.
> Use CPU cooling (CPUFreq DVFS driver) to implement thermal management.
> Register cpufreq cooling device, add passive trip point and bind it
> to thermal zone 0.
> Passive trip point temperature is set to 70C, but is adjustable
> via sysfs to facilitate experimentation and platform tuning.
> 
> Signed-off-by: Patrick Titiano <ptitiano@baylibre.com>

Kuninori,

what do you think of this patch?

thanks,
rui
> ---
>  drivers/thermal/rcar_thermal.c | 103 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 96 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> index 187693c..0f953cf0 100644
> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
> @@ -31,6 +31,8 @@
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
>  #include <linux/thermal.h>
> +#include <linux/cpufreq.h>
> +#include <linux/cpu_cooling.h>
>  
>  #define IDLE_INTERVAL	5000
>  
> @@ -62,11 +64,14 @@ struct rcar_thermal_priv {
>  	void __iomem *base;
>  	struct rcar_thermal_common *common;
>  	struct thermal_zone_device *zone;
> +	struct thermal_cooling_device *cpucooldev;
>  	struct delayed_work work;
>  	struct mutex lock;
>  	struct list_head list;
>  	int id;
>  	int ctemp;
> +	int trip_ctemp_passive;
> +	int trip_ctemp_critical;
>  };
>  
>  #define rcar_thermal_for_each_priv(pos, common)	\
> @@ -78,6 +83,9 @@ struct rcar_thermal_priv {
>  #define rcar_has_irq_support(priv)	((priv)->common->base)
>  #define rcar_id_to_shift(priv)		((priv)->id * 8)
>  
> +#define TRIP_CTEMP_PASSIVE		MCELSIUS(70)
> +#define TRIP_CTEMP_CRITICAL		MCELSIUS(90)
> +
>  #ifdef DEBUG
>  # define rcar_force_update_temp(priv)	1
>  #else
> @@ -224,9 +232,11 @@ static int rcar_thermal_get_trip_type(struct thermal_zone_device *zone,
>  	struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
>  	struct device *dev = rcar_priv_to_dev(priv);
>  
> -	/* see rcar_thermal_get_temp() */
>  	switch (trip) {
> -	case 0: /* +90 <= temp */
> +	case 0:
> +		*type = THERMAL_TRIP_PASSIVE;
> +		break;
> +	case 1:
>  		*type = THERMAL_TRIP_CRITICAL;
>  		break;
>  	default:
> @@ -243,15 +253,35 @@ static int rcar_thermal_get_trip_temp(struct thermal_zone_device *zone,
>  	struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
>  	struct device *dev = rcar_priv_to_dev(priv);
>  
> -	/* see rcar_thermal_get_temp() */
> +	mutex_lock(&priv->lock);
>  	switch (trip) {
> -	case 0: /* +90 <= temp */
> -		*temp = MCELSIUS(90);
> +	case 0:
> +		*temp = priv->trip_ctemp_passive;
> +		break;
> +	case 1:
> +		*temp = priv->trip_ctemp_critical;
>  		break;
>  	default:
>  		dev_err(dev, "rcar driver trip error\n");
>  		return -EINVAL;
>  	}
> +	mutex_unlock(&priv->lock);
> +
> +	return 0;
> +}
> +
> +static int rcar_thermal_set_trip_temp(struct thermal_zone_device *zone,
> +				      int trip, unsigned long temp)
> +{
> +	struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
> +
> +	/* Do not allow PASSIVE trip temperature > CRITICAL trip temperature */
> +	if (temp >= priv->trip_ctemp_critical)
> +		return -EINVAL;
> +
> +	mutex_lock(&priv->lock);
> +	priv->trip_ctemp_passive = temp;
> +	mutex_unlock(&priv->lock);
>  
>  	return 0;
>  }
> @@ -274,11 +304,52 @@ static int rcar_thermal_notify(struct thermal_zone_device *zone,
>  	return 0;
>  }
>  
> +static int rcar_thermal_bind(struct thermal_zone_device *zone,
> +			   struct thermal_cooling_device *cdev)
> +{
> +	int ret;
> +
> +	ret = thermal_zone_bind_cooling_device(zone, 0, cdev,
> +	/* bind with min and max states defined by cpu_cooling */
> +						THERMAL_NO_LIMIT,
> +						THERMAL_NO_LIMIT);
> +	ret += thermal_zone_bind_cooling_device(zone, 1, cdev,
> +	/* bind with min and max states defined by cpu_cooling */
> +						THERMAL_NO_LIMIT,
> +						THERMAL_NO_LIMIT);
> +
> +	return ret;
> +}
> +
> +/* Unbind callback functions for thermal zone */
> +static int rcar_thermal_unbind(struct thermal_zone_device *zone,
> +			     struct thermal_cooling_device *cdev)
> +{
> +	int ret;
> +
> +	ret = thermal_zone_unbind_cooling_device(zone, 0, cdev);
> +	ret += thermal_zone_unbind_cooling_device(zone, 1, cdev);
> +
> +	return ret;
> +}
> +
> +static int rcar_thermal_get_mode(struct thermal_zone_device *zone,
> +			 enum thermal_device_mode *mode)
> +{
> +	*mode = THERMAL_DEVICE_ENABLED;
> +
> +	return 0;
> +}
> +
>  static struct thermal_zone_device_ops rcar_thermal_zone_ops = {
> +	.get_mode       = rcar_thermal_get_mode,
>  	.get_temp	= rcar_thermal_get_temp,
>  	.get_trip_type	= rcar_thermal_get_trip_type,
>  	.get_trip_temp	= rcar_thermal_get_trip_temp,
> +	.set_trip_temp	= rcar_thermal_set_trip_temp,
>  	.notify		= rcar_thermal_notify,
> +	.bind           = rcar_thermal_bind,
> +	.unbind         = rcar_thermal_unbind,
>  };
>  
>  /*
> @@ -375,6 +446,12 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>  	int i;
>  	int ret = -ENODEV;
>  	int idle = IDLE_INTERVAL;
> +	struct cpumask clip_cpus;
> +
> +	if (!cpufreq_get_current_driver()) {
> +		dev_dbg(dev, "no cpufreq driver yet, deferring probe\n");
> +		return -EPROBE_DEFER;
> +	}
>  
>  	common = devm_kzalloc(dev, sizeof(*common), GFP_KERNEL);
>  	if (!common) {
> @@ -456,13 +533,24 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>  
>  		priv->common = common;
>  		priv->id = i;
> +		priv->trip_ctemp_passive = TRIP_CTEMP_PASSIVE;
> +		priv->trip_ctemp_critical = TRIP_CTEMP_CRITICAL;
>  		mutex_init(&priv->lock);
>  		INIT_LIST_HEAD(&priv->list);
>  		INIT_DELAYED_WORK(&priv->work, rcar_thermal_work);
>  		rcar_thermal_update_temp(priv);
>  
> +		cpumask_set_cpu(0, &clip_cpus);
> +		priv->cpucooldev = cpufreq_cooling_register(&clip_cpus);
> +		if (IS_ERR(priv->cpucooldev)) {
> +			ret = PTR_ERR(priv->cpucooldev);
> +			dev_err(dev, "failed to register cpufreq cooling device (%d)\n",
> +				ret);
> +			goto error_unpm;
> +		}
> +
>  		priv->zone = thermal_zone_device_register("rcar_thermal",
> -						1, 0, priv,
> +						2, 1, priv,
>  						&rcar_thermal_zone_ops, NULL, 0,
>  						idle);
>  		if (IS_ERR(priv->zone)) {
> @@ -478,7 +566,6 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>  	}
>  
>  	platform_set_drvdata(pdev, common);
> -
>  	dev_info(dev, "%d sensor probed\n", i);
>  
>  	return 0;
> @@ -513,6 +600,8 @@ static int rcar_thermal_remove(struct platform_device *pdev)
>  			rcar_thermal_irq_disable(priv);
>  	}
>  
> +	cpufreq_cooling_unregister(priv->cpucooldev);
> +
>  	pm_runtime_put_sync(dev);
>  	pm_runtime_disable(dev);
>  

Re: [RFC 1/1] thermal: rcar-thermal: add cpu cooling

[Kuninori Morimoto]

Hi Zhang, Partez:

> On Fri, 2014-02-28 at 16:02 +0100, Patrick Titiano wrote:
> > Renesas RCar platforms only register a thermal zone, but do not implement
> > any thermal management.
> > Use CPU cooling (CPUFreq DVFS driver) to implement thermal management.
> > Register cpufreq cooling device, add passive trip point and bind it
> > to thermal zone 0.
> > Passive trip point temperature is set to 70C, but is adjustable
> > via sysfs to facilitate experimentation and platform tuning.
> > 
> > Signed-off-by: Patrick Titiano <ptitiano@baylibre.com>
> 
> Kuninori,
> 
> what do you think of this patch?
(snip)
> > +		priv->cpucooldev = cpufreq_cooling_register(&clip_cpus);
> > +		if (IS_ERR(priv->cpucooldev)) {
> > +			ret = PTR_ERR(priv->cpucooldev);
> > +			dev_err(dev, "failed to register cpufreq cooling device (%d)\n",
> > +				ret);
> > +			goto error_unpm;
> > +		}
(snip)
> Then why you make the rcar-thermal depends on the success of
> cpufreq_cooling_register()?
> IMO, you can just register two trip points first, and bind the cpu
> cooling device to passive trip point only if cpufreq_cooling_register()
> succeeds.

I agree to Zhang's opinion.
It is better not to break the existing environment. 

But, I guess this patch is still under "not-yet-ready-to-merge patch" ?
(this patch has "Signed-off-by" thought... :)

Re: [RFC 0/1] thermal: rcar-thermal: add cpu cooling

[Zhang Rui]

On Fri, 2014-02-28 at 16:02 +0100, Patrick Titiano wrote:
> Hi,
> This not-yet-ready-to-merge patch proposes to enable CPU thermal management on
> Renesas platform. Currently these platforms only register a thermal zone,
> but do not implement or register any cooling device.
> 
> With this patch:
>  - CPU0 cpufreq driver is registered as a cooling device,
>  - a new passive trip point is defined to trigger CPU scaling when temperature
>    crosses it,
>  - CPU0 cooling device and thermal zone 0 are being bound together,
>  - set_trip_temp callback is implemented to facilitate experimentation and 
>    platform tuning. 
> 
> Developed on a RCar-H2 platform, it is intended to support multi-platform.
> It leverages CPUFreq support from the series Benoit Cousson recently shared
> (http://www.spinics.net/lists/linux-sh/msg29236.html).
> The passive trip point temperature is arbitrarily set to 70C, as no HW data was
> available at the time this patch was developed.
> 
> Being multi-platform, this patch is expected to run smoothly in any situation:
>  1/ Platform has no thermal sensor,
>  2/ Platform has a thermal sensor and is DVFS capable,
>  3/ Platform has a thermal sensor but is not DVFS capable.
> 
> This patch has no side-effect in case 1/ (thermal driver wouldn't be loaded).
> Case 2/ is the regular case, no issue.
> case 3/ is the problematic one.
> Using cpufreq driver as a cooling device adds a dependency between those 2
> drivers. This is taken care by deferring rcar-thermal probing until
> cpufreq driver is up, but this becomes an issue when a platform does
> not have DVFS support. cpufreq driver will never be probed, blocking
> rcar-thermal probing.
> 
> We are still investigation what would be the best way to go, however we would
> appreciate a lot any advice from the community.

Then why you make the rcar-thermal depends on the success of
cpufreq_cooling_register()?
IMO, you can just register two trip points first, and bind the cpu
cooling device to passive trip point only if cpufreq_cooling_register()
succeeds.

thanks,
rui
> Thanks!
> 
> Regards,
> Patrick.
> 
> Patrick Titiano (1):
>   thermal: rcar-thermal: add cpu cooling
> 
>  drivers/thermal/rcar_thermal.c | 103 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 96 insertions(+), 7 deletions(-)
>