[PATCH 1/2] thermal: sysfs: Add a new sysfs node emul_temp

[PATCH 1/2] thermal: sysfs: Add a new sysfs node emul_temp

[Amit Daniel Kachhap]

This patch adds support to set the emulated temperature method in
thermal zone (sensor). After setting this feature thermal zone must
report this temperature and not the actual temperature. The actual
implementation of this emulated temperature is based on sensor
capability or platform specific. This is useful in debugging different
temperature threshold and its associated cooling action. Writing 0 on
this node should disable emulation.

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 Documentation/thermal/sysfs-api.txt |   14 ++++++++++++++
 drivers/thermal/thermal_sys.c       |   26 ++++++++++++++++++++++++++
 include/linux/thermal.h             |    1 +
 3 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 88c0233..e8f2ee4 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -55,6 +55,8 @@ temperature) and throttle appropriate devices.
 	.get_trip_type: get the type of certain trip point.
 	.get_trip_temp: get the temperature above which the certain trip point
 			will be fired.
+	.set_emul_temp: set the emulation temperature which helps in debugging
+			different threshold temperature points.
 
 1.1.2 void thermal_zone_device_unregister(struct thermal_zone_device *tz)
 
@@ -153,6 +155,7 @@ Thermal zone device sys I/F, created once it's registered:
     |---trip_point_[0-*]_temp:	Trip point temperature
     |---trip_point_[0-*]_type:	Trip point type
     |---trip_point_[0-*]_hyst:	Hysteresis value for this trip point
+    |---emul_temp:		Emulated temperature set node
 
 Thermal cooling device sys I/F, created once it's registered:
 /sys/class/thermal/cooling_device[0-*]:
@@ -252,6 +255,17 @@ passive
 	Valid values: 0 (disabled) or greater than 1000
 	RW, Optional
 
+emul_temp
+	Interface to set the emulated temperature method in thermal zone
+	(sensor). After setting this feature thermal zone must report
+	this temperature and not the actual temperature. The actual
+	implementation of this emulated	temperature is platform specific.
+	This is useful in debugging different temperature threshold and its
+	associated cooling action. Writing 0 on this node should disable
+	emulation.
+	Unit: millidegree Celsius
+	WO, Optional
+
 *****************************
 * Cooling device attributes *
 *****************************
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 8c8ce80..ecdfc7d 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -700,11 +700,31 @@ policy_show(struct device *dev, struct device_attribute *devattr, char *buf)
 	return sprintf(buf, "%s\n", tz->governor->name);
 }
 
+static ssize_t
+emul_temp_store(struct device *dev, struct device_attribute *attr,
+		     const char *buf, size_t count)
+{
+	struct thermal_zone_device *tz = to_thermal_zone(dev);
+	int ret;
+	unsigned long temperature;
+
+	if (!tz->ops->set_emul_temp)
+		return -EPERM;
+
+	if (kstrtoul(buf, 10, &temperature))
+		return -EINVAL;
+
+	ret = tz->ops->set_emul_temp(tz, temperature);
+
+	return ret ? ret : count;
+}
+
 static DEVICE_ATTR(type, 0444, type_show, NULL);
 static DEVICE_ATTR(temp, 0444, temp_show, NULL);
 static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
 static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, passive_store);
 static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);
+static DEVICE_ATTR(emul_temp, S_IWUSR, NULL, emul_temp_store);
 
 /* sys I/F for cooling device */
 #define to_cooling_device(_dev)	\
@@ -1592,6 +1612,12 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
 			goto unregister;
 	}
 
+	if (ops->set_emul_temp) {
+		result = device_create_file(&tz->device, &dev_attr_emul_temp);
+		if (result)
+			goto unregister;
+	}
+
 	/* Create policy attribute */
 	result = device_create_file(&tz->device, &dev_attr_policy);
 	if (result)
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 883bcda..fbb87d4 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -123,6 +123,7 @@ struct thermal_zone_device_ops {
 	int (*set_trip_hyst) (struct thermal_zone_device *, int,
 			      unsigned long);
 	int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
+	int (*set_emul_temp) (struct thermal_zone_device *, unsigned long);
 	int (*get_trend) (struct thermal_zone_device *, int,
 			  enum thermal_trend *);
 	int (*notify) (struct thermal_zone_device *, int,
-- 
1.7.5.4

[PATCH 2/2] thermal: exynos: Use the framework for temperature emulation support

[Amit Daniel Kachhap]

This removes the driver specific sysfs support of the temperature
emulation and uses the newly added core thermal framework for thermal
emulation. A platform specific handler is added to support this.

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 Documentation/thermal/exynos_thermal_emulation |    6 +-
 drivers/thermal/exynos_thermal.c               |  154 ++++++++++--------------
 2 files changed, 64 insertions(+), 96 deletions(-)

diff --git a/Documentation/thermal/exynos_thermal_emulation b/Documentation/thermal/exynos_thermal_emulation
index b73bbfb..570a52d 100644
--- a/Documentation/thermal/exynos_thermal_emulation
+++ b/Documentation/thermal/exynos_thermal_emulation
@@ -14,10 +14,10 @@ manually with software code and TMU will read current temperature from user valu
 sensor's value.
 
 Enabling CONFIG_EXYNOS_THERMAL_EMUL option will make this support in available.
-When it's enabled, sysfs node will be created under
-/sys/bus/platform/devices/'exynos device name'/ with name of 'emulation'.
+When it's enabled, sysfs node will be created as
+/sys/devices/virtual/thermal/thermal_zone'zone id'/emul_temp.
 
-The sysfs node, 'emulation', will contain value 0 for the initial state. When you input any
+The sysfs node, 'emul_node', will contain value 0 for the initial state. When you input any
 temperature you want to update to sysfs node, it automatically enable emulation mode and
 current temperature will be changed into it.
 (Exynos also supports user changable delay time which would be used to delay of
diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c
index 68c9280..a657903 100644
--- a/drivers/thermal/exynos_thermal.c
+++ b/drivers/thermal/exynos_thermal.c
@@ -145,6 +145,7 @@ struct	thermal_cooling_conf {
 struct thermal_sensor_conf {
 	char name[SENSOR_NAME_LEN];
 	int (*read_temperature)(void *data);
+	int (*write_emul_temp)(void *data, unsigned long temp);
 	struct thermal_trip_point_conf trip_data;
 	struct thermal_cooling_conf cooling_data;
 	void *private_data;
@@ -369,6 +370,23 @@ static int exynos_get_temp(struct thermal_zone_device *thermal,
 	return 0;
 }
 
+/* Get temperature callback functions for thermal zone */
+static int exynos_set_emul_temp(struct thermal_zone_device *thermal,
+						unsigned long temp)
+{
+	void *data;
+	int ret = -EINVAL;
+
+	if (!th_zone->sensor_conf) {
+		pr_info("Temperature sensor not initialised\n");
+		return -EINVAL;
+	}
+	data = th_zone->sensor_conf->private_data;
+	if (th_zone->sensor_conf->write_emul_temp)
+		ret = th_zone->sensor_conf->write_emul_temp(data, temp);
+	return ret;
+}
+
 /* Get the temperature trend */
 static int exynos_get_trend(struct thermal_zone_device *thermal,
 			int trip, enum thermal_trend *trend)
@@ -392,6 +410,7 @@ static struct thermal_zone_device_ops const exynos_dev_ops = {
 	.bind = exynos_bind,
 	.unbind = exynos_unbind,
 	.get_temp = exynos_get_temp,
+	.set_emul_temp = exynos_set_emul_temp,
 	.get_trend = exynos_get_trend,
 	.get_mode = exynos_get_mode,
 	.set_mode = exynos_set_mode,
@@ -714,6 +733,44 @@ static int exynos_tmu_read(struct exynos_tmu_data *data)
 	return temp;
 }
 
+#ifdef CONFIG_EXYNOS_THERMAL_EMUL
+static int exynos_tmu_set_emulation(struct exynos_tmu_data *data,
+					unsigned long temp)
+{
+	unsigned int reg;
+	int ret = -EINVAL;
+
+	if (data->soc == SOC_ARCH_EXYNOS4210)
+		goto out;
+
+	if (temp && temp < MCELSIUS)
+		goto out;
+
+	mutex_lock(&data->lock);
+	clk_enable(data->clk);
+
+	reg = readl(data->base + EXYNOS_EMUL_CON);
+
+	if (temp) {
+		temp /= MCELSIUS;
+
+		reg = (EXYNOS_EMUL_TIME << EXYNOS_EMUL_TIME_SHIFT) |
+			(temp_to_code(data, temp)
+			 << EXYNOS_EMUL_DATA_SHIFT) | EXYNOS_EMUL_ENABLE;
+	} else {
+		reg &= ~EXYNOS_EMUL_ENABLE;
+	}
+
+	writel(reg, data->base + EXYNOS_EMUL_CON);
+
+	clk_disable(data->clk);
+	mutex_unlock(&data->lock);
+	return 0;
+out:
+	return ret;
+}
+#endif/*CONFIG_EXYNOS_THERMAL_EMUL*/
+
 static void exynos_tmu_work(struct work_struct *work)
 {
 	struct exynos_tmu_data *data = container_of(work,
@@ -853,93 +910,6 @@ static inline struct  exynos_tmu_platform_data *exynos_get_driver_data(
 			platform_get_device_id(pdev)->driver_data;
 }
 
-#ifdef CONFIG_EXYNOS_THERMAL_EMUL
-static ssize_t exynos_tmu_emulation_show(struct device *dev,
-					 struct device_attribute *attr,
-					 char *buf)
-{
-	struct platform_device *pdev = container_of(dev,
-					struct platform_device, dev);
-	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
-	unsigned int reg;
-	u8 temp_code;
-	int temp = 0;
-
-	if (data->soc == SOC_ARCH_EXYNOS4210)
-		goto out;
-
-	mutex_lock(&data->lock);
-	clk_enable(data->clk);
-	reg = readl(data->base + EXYNOS_EMUL_CON);
-	clk_disable(data->clk);
-	mutex_unlock(&data->lock);
-
-	if (reg & EXYNOS_EMUL_ENABLE) {
-		reg >>= EXYNOS_EMUL_DATA_SHIFT;
-		temp_code = reg & EXYNOS_EMUL_DATA_MASK;
-		temp = code_to_temp(data, temp_code);
-	}
-out:
-	return sprintf(buf, "%d\n", temp * MCELSIUS);
-}
-
-static ssize_t exynos_tmu_emulation_store(struct device *dev,
-					struct device_attribute *attr,
-					const char *buf, size_t count)
-{
-	struct platform_device *pdev = container_of(dev,
-					struct platform_device, dev);
-	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
-	unsigned int reg;
-	int temp;
-
-	if (data->soc == SOC_ARCH_EXYNOS4210)
-		goto out;
-
-	if (!sscanf(buf, "%d\n", &temp) || temp < 0)
-		return -EINVAL;
-
-	mutex_lock(&data->lock);
-	clk_enable(data->clk);
-
-	reg = readl(data->base + EXYNOS_EMUL_CON);
-
-	if (temp) {
-		/* Both CELSIUS and MCELSIUS type are available for input */
-		if (temp > MCELSIUS)
-			temp /= MCELSIUS;
-
-		reg = (EXYNOS_EMUL_TIME << EXYNOS_EMUL_TIME_SHIFT) |
-			(temp_to_code(data, (temp / MCELSIUS))
-			 << EXYNOS_EMUL_DATA_SHIFT) | EXYNOS_EMUL_ENABLE;
-	} else {
-		reg &= ~EXYNOS_EMUL_ENABLE;
-	}
-
-	writel(reg, data->base + EXYNOS_EMUL_CON);
-
-	clk_disable(data->clk);
-	mutex_unlock(&data->lock);
-
-out:
-	return count;
-}
-
-static DEVICE_ATTR(emulation, 0644, exynos_tmu_emulation_show,
-					exynos_tmu_emulation_store);
-static int create_emulation_sysfs(struct device *dev)
-{
-	return device_create_file(dev, &dev_attr_emulation);
-}
-static void remove_emulation_sysfs(struct device *dev)
-{
-	device_remove_file(dev, &dev_attr_emulation);
-}
-#else
-static inline int create_emulation_sysfs(struct device *dev) { return 0; }
-static inline void remove_emulation_sysfs(struct device *dev) {}
-#endif
-
 static int __devinit exynos_tmu_probe(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data;
@@ -1016,6 +986,10 @@ static int __devinit exynos_tmu_probe(struct platform_device *pdev)
 
 	/* Register the sensor with thermal management interface */
 	(&exynos_sensor_conf)->private_data = data;
+#ifdef CONFIG_EXYNOS_THERMAL_EMUL
+	(&exynos_sensor_conf)->write_emul_temp =
+		(int (*)(void *, unsigned long))exynos_tmu_set_emulation;
+#endif
 	exynos_sensor_conf.trip_data.trip_count = pdata->trigger_level0_en +
 			pdata->trigger_level1_en + pdata->trigger_level2_en +
 			pdata->trigger_level3_en;
@@ -1041,10 +1015,6 @@ static int __devinit exynos_tmu_probe(struct platform_device *pdev)
 		goto err_clk;
 	}
 
-	ret = create_emulation_sysfs(&pdev->dev);
-	if (ret)
-		dev_err(&pdev->dev, "Failed to create emulation mode sysfs node\n");
-
 	return 0;
 err_clk:
 	platform_set_drvdata(pdev, NULL);
@@ -1056,8 +1026,6 @@ static int __devexit exynos_tmu_remove(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 
-	remove_emulation_sysfs(&pdev->dev);
-
 	exynos_tmu_control(pdev, false);
 
 	exynos_unregister_thermal();
-- 
1.7.5.4

Re: [PATCH 1/2] thermal: sysfs: Add a new sysfs node emul_temp

[Zhang Rui]

Hi, Amit,

On Sun, 2013-01-06 at 16:08 -0800, Amit Daniel Kachhap wrote:
> This patch adds support to set the emulated temperature method in
> thermal zone (sensor). After setting this feature thermal zone must
> report this temperature and not the actual temperature. The actual
> implementation of this emulated temperature is based on sensor
> capability or platform specific. This is useful in debugging different
> temperature threshold and its associated cooling action. Writing 0 on
> this node should disable emulation.

Question:
will this bring hardware issue? Say, critical temperature reached while
in emulation mode?

As this is for debug purpose, I'd prefer to have a seperate Kconfig
option for this feature.

> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  Documentation/thermal/sysfs-api.txt |   14 ++++++++++++++
>  drivers/thermal/thermal_sys.c       |   26 ++++++++++++++++++++++++++
>  include/linux/thermal.h             |    1 +
>  3 files changed, 41 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> index 88c0233..e8f2ee4 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -55,6 +55,8 @@ temperature) and throttle appropriate devices.
>  	.get_trip_type: get the type of certain trip point.
>  	.get_trip_temp: get the temperature above which the certain trip point
>  			will be fired.
> +	.set_emul_temp: set the emulation temperature which helps in debugging
> +			different threshold temperature points.
>  
>  1.1.2 void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>  
> @@ -153,6 +155,7 @@ Thermal zone device sys I/F, created once it's registered:
>      |---trip_point_[0-*]_temp:	Trip point temperature
>      |---trip_point_[0-*]_type:	Trip point type
>      |---trip_point_[0-*]_hyst:	Hysteresis value for this trip point
> +    |---emul_temp:		Emulated temperature set node
>  
>  Thermal cooling device sys I/F, created once it's registered:
>  /sys/class/thermal/cooling_device[0-*]:
> @@ -252,6 +255,17 @@ passive
>  	Valid values: 0 (disabled) or greater than 1000
>  	RW, Optional
>  
> +emul_temp
> +	Interface to set the emulated temperature method in thermal zone
> +	(sensor). After setting this feature thermal zone must report
> +	this temperature and not the actual temperature. The actual
> +	implementation of this emulated	temperature is platform specific.

can we have a pure software temperature emulation method?
say, the generic thermal layer caches the emulated temperature value,
and hook it in update_temperature()?
This is also useful for testing in polling mode, and it does not require
platform specific callback support. I mean thermal_ops->set_emul_temp is
optional, but thermal emulation is always available for all platforms.

thanks,
rui
> +	This is useful in debugging different temperature threshold and its
> +	associated cooling action. Writing 0 on this node should disable
> +	emulation.
> +	Unit: millidegree Celsius
> +	WO, Optional
> +
>  *****************************
>  * Cooling device attributes *
>  *****************************
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index 8c8ce80..ecdfc7d 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -700,11 +700,31 @@ policy_show(struct device *dev, struct device_attribute *devattr, char *buf)
>  	return sprintf(buf, "%s\n", tz->governor->name);
>  }
>  
> +static ssize_t
> +emul_temp_store(struct device *dev, struct device_attribute *attr,
> +		     const char *buf, size_t count)
> +{
> +	struct thermal_zone_device *tz = to_thermal_zone(dev);
> +	int ret;
> +	unsigned long temperature;
> +
> +	if (!tz->ops->set_emul_temp)
> +		return -EPERM;
> +
> +	if (kstrtoul(buf, 10, &temperature))
> +		return -EINVAL;
> +
> +	ret = tz->ops->set_emul_temp(tz, temperature);
> +
> +	return ret ? ret : count;
> +}
> +
>  static DEVICE_ATTR(type, 0444, type_show, NULL);
>  static DEVICE_ATTR(temp, 0444, temp_show, NULL);
>  static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
>  static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, passive_store);
>  static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);
> +static DEVICE_ATTR(emul_temp, S_IWUSR, NULL, emul_temp_store);
>  
>  /* sys I/F for cooling device */
>  #define to_cooling_device(_dev)	\
> @@ -1592,6 +1612,12 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
>  			goto unregister;
>  	}
>  
> +	if (ops->set_emul_temp) {
> +		result = device_create_file(&tz->device, &dev_attr_emul_temp);
> +		if (result)
> +			goto unregister;
> +	}
> +
>  	/* Create policy attribute */
>  	result = device_create_file(&tz->device, &dev_attr_policy);
>  	if (result)
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 883bcda..fbb87d4 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -123,6 +123,7 @@ struct thermal_zone_device_ops {
>  	int (*set_trip_hyst) (struct thermal_zone_device *, int,
>  			      unsigned long);
>  	int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
> +	int (*set_emul_temp) (struct thermal_zone_device *, unsigned long);
>  	int (*get_trend) (struct thermal_zone_device *, int,
>  			  enum thermal_trend *);
>  	int (*notify) (struct thermal_zone_device *, int,

Re: [PATCH 1/2] thermal: sysfs: Add a new sysfs node emul_temp

[amit kachhap]

Hi Rui,

Thanks for the review comments,
On Tue, Jan 15, 2013 at 11:33 PM, Zhang Rui <rui.zhang@intel.com> wrote:
> Hi, Amit,
>
> On Sun, 2013-01-06 at 16:08 -0800, Amit Daniel Kachhap wrote:
>> This patch adds support to set the emulated temperature method in
>> thermal zone (sensor). After setting this feature thermal zone must
>> report this temperature and not the actual temperature. The actual
>> implementation of this emulated temperature is based on sensor
>> capability or platform specific. This is useful in debugging different
>> temperature threshold and its associated cooling action. Writing 0 on
>> this node should disable emulation.
>
> Question:
> will this bring hardware issue? Say, critical temperature reached while
> in emulation mode?
No emulation does cause any h/w issue.
>
> As this is for debug purpose, I'd prefer to have a seperate Kconfig
> option for this feature.
Yes agreed. Will re-submit with kconfig option.
>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  Documentation/thermal/sysfs-api.txt |   14 ++++++++++++++
>>  drivers/thermal/thermal_sys.c       |   26 ++++++++++++++++++++++++++
>>  include/linux/thermal.h             |    1 +
>>  3 files changed, 41 insertions(+), 0 deletions(-)
>>
>> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
>> index 88c0233..e8f2ee4 100644
>> --- a/Documentation/thermal/sysfs-api.txt
>> +++ b/Documentation/thermal/sysfs-api.txt
>> @@ -55,6 +55,8 @@ temperature) and throttle appropriate devices.
>>       .get_trip_type: get the type of certain trip point.
>>       .get_trip_temp: get the temperature above which the certain trip point
>>                       will be fired.
>> +     .set_emul_temp: set the emulation temperature which helps in debugging
>> +                     different threshold temperature points.
>>
>>  1.1.2 void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>>
>> @@ -153,6 +155,7 @@ Thermal zone device sys I/F, created once it's registered:
>>      |---trip_point_[0-*]_temp:       Trip point temperature
>>      |---trip_point_[0-*]_type:       Trip point type
>>      |---trip_point_[0-*]_hyst:       Hysteresis value for this trip point
>> +    |---emul_temp:           Emulated temperature set node
>>
>>  Thermal cooling device sys I/F, created once it's registered:
>>  /sys/class/thermal/cooling_device[0-*]:
>> @@ -252,6 +255,17 @@ passive
>>       Valid values: 0 (disabled) or greater than 1000
>>       RW, Optional
>>
>> +emul_temp
>> +     Interface to set the emulated temperature method in thermal zone
>> +     (sensor). After setting this feature thermal zone must report
>> +     this temperature and not the actual temperature. The actual
>> +     implementation of this emulated temperature is platform specific.
>
> can we have a pure software temperature emulation method?
> say, the generic thermal layer caches the emulated temperature value,
> and hook it in update_temperature()?
> This is also useful for testing in polling mode, and it does not require
> platform specific callback support. I mean thermal_ops->set_emul_temp is
> optional, but thermal emulation is always available for all platforms.
Yes It makes sense and we can have pure software emulation and use the
cached temperature when no platform call is registered. In my case I
needed this in h/w so to have the same sensor trigger interrupts
behaviour.

So the code flow can be like this,

#ifdef CONFIG_THERMAL_EMULATION
if (thermal_ops->set_emul_temp)
then pass emul_temp to platform and use the normal platform
thermal_ops->get_temp
else
Store it locally and use emul_temp  instead of calling platform
thermal_ops->get_temp
#endif

I will re-submit with this change.

Thanks,
Amit
>
> thanks,
> rui
>> +     This is useful in debugging different temperature threshold and its
>> +     associated cooling action. Writing 0 on this node should disable
>> +     emulation.
>> +     Unit: millidegree Celsius
>> +     WO, Optional
>> +
>>  *****************************
>>  * Cooling device attributes *
>>  *****************************
>> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
>> index 8c8ce80..ecdfc7d 100644
>> --- a/drivers/thermal/thermal_sys.c
>> +++ b/drivers/thermal/thermal_sys.c
>> @@ -700,11 +700,31 @@ policy_show(struct device *dev, struct device_attribute *devattr, char *buf)
>>       return sprintf(buf, "%s\n", tz->governor->name);
>>  }
>>
>> +static ssize_t
>> +emul_temp_store(struct device *dev, struct device_attribute *attr,
>> +                  const char *buf, size_t count)
>> +{
>> +     struct thermal_zone_device *tz = to_thermal_zone(dev);
>> +     int ret;
>> +     unsigned long temperature;
>> +
>> +     if (!tz->ops->set_emul_temp)
>> +             return -EPERM;
>> +
>> +     if (kstrtoul(buf, 10, &temperature))
>> +             return -EINVAL;
>> +
>> +     ret = tz->ops->set_emul_temp(tz, temperature);
>> +
>> +     return ret ? ret : count;
>> +}
>> +
>>  static DEVICE_ATTR(type, 0444, type_show, NULL);
>>  static DEVICE_ATTR(temp, 0444, temp_show, NULL);
>>  static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
>>  static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, passive_store);
>>  static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);
>> +static DEVICE_ATTR(emul_temp, S_IWUSR, NULL, emul_temp_store);
>>
>>  /* sys I/F for cooling device */
>>  #define to_cooling_device(_dev)      \
>> @@ -1592,6 +1612,12 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
>>                       goto unregister;
>>       }
>>
>> +     if (ops->set_emul_temp) {
>> +             result = device_create_file(&tz->device, &dev_attr_emul_temp);
>> +             if (result)
>> +                     goto unregister;
>> +     }
>> +
>>       /* Create policy attribute */
>>       result = device_create_file(&tz->device, &dev_attr_policy);
>>       if (result)
>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> index 883bcda..fbb87d4 100644
>> --- a/include/linux/thermal.h
>> +++ b/include/linux/thermal.h
>> @@ -123,6 +123,7 @@ struct thermal_zone_device_ops {
>>       int (*set_trip_hyst) (struct thermal_zone_device *, int,
>>                             unsigned long);
>>       int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
>> +     int (*set_emul_temp) (struct thermal_zone_device *, unsigned long);
>>       int (*get_trend) (struct thermal_zone_device *, int,
>>                         enum thermal_trend *);
>>       int (*notify) (struct thermal_zone_device *, int,
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/2] thermal: sysfs: Add a new sysfs node emul_temp

[Kukjin Kim]

amit kachhap wrote:
> 
> Hi Rui,
> 
> Thanks for the review comments,
> On Tue, Jan 15, 2013 at 11:33 PM, Zhang Rui <rui.zhang@intel.com> wrote:
> > Hi, Amit,
> >
> > On Sun, 2013-01-06 at 16:08 -0800, Amit Daniel Kachhap wrote:
> >> This patch adds support to set the emulated temperature method in
> >> thermal zone (sensor). After setting this feature thermal zone must
> >> report this temperature and not the actual temperature. The actual
> >> implementation of this emulated temperature is based on sensor
> >> capability or platform specific. This is useful in debugging different
> >> temperature threshold and its associated cooling action. Writing 0 on
> >> this node should disable emulation.
> >
> > Question:
> > will this bring hardware issue? Say, critical temperature reached while
> > in emulation mode?
> No emulation does cause any h/w issue.
> >
> > As this is for debug purpose, I'd prefer to have a seperate Kconfig
> > option for this feature.
> Yes agreed. Will re-submit with kconfig option.
> >
> >> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>

Hi Zhang,

Once Amit addresses comments from you, I think, this looks good to Exynos
SoCs. And this is _really_ needed.

Feel free to add my ack on this 1st and 2nd patches:

Acked-by: Kukjin Kim <kgene.kim@samsung.com>

Thanks.

- Kukjin

Re: [PATCH 1/2] thermal: sysfs: Add a new sysfs node emul_temp

[Zhang Rui]

On Wed, 2013-01-16 at 11:30 -0800, amit kachhap wrote:
> Hi Rui,
> 
> Thanks for the review comments,
> On Tue, Jan 15, 2013 at 11:33 PM, Zhang Rui <rui.zhang@intel.com> wrote:
> > Hi, Amit,
> >
> > On Sun, 2013-01-06 at 16:08 -0800, Amit Daniel Kachhap wrote:
> >> This patch adds support to set the emulated temperature method in
> >> thermal zone (sensor). After setting this feature thermal zone must
> >> report this temperature and not the actual temperature. The actual
> >> implementation of this emulated temperature is based on sensor
> >> capability or platform specific. This is useful in debugging different
> >> temperature threshold and its associated cooling action. Writing 0 on
> >> this node should disable emulation.
> >
> > Question:
> > will this bring hardware issue? Say, critical temperature reached while
> > in emulation mode?
> No emulation does cause any h/w issue.
> >
> > As this is for debug purpose, I'd prefer to have a seperate Kconfig
> > option for this feature.
> Yes agreed. Will re-submit with kconfig option.
> >
> >> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> >> ---
> >>  Documentation/thermal/sysfs-api.txt |   14 ++++++++++++++
> >>  drivers/thermal/thermal_sys.c       |   26 ++++++++++++++++++++++++++
> >>  include/linux/thermal.h             |    1 +
> >>  3 files changed, 41 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> >> index 88c0233..e8f2ee4 100644
> >> --- a/Documentation/thermal/sysfs-api.txt
> >> +++ b/Documentation/thermal/sysfs-api.txt
> >> @@ -55,6 +55,8 @@ temperature) and throttle appropriate devices.
> >>       .get_trip_type: get the type of certain trip point.
> >>       .get_trip_temp: get the temperature above which the certain trip point
> >>                       will be fired.
> >> +     .set_emul_temp: set the emulation temperature which helps in debugging
> >> +                     different threshold temperature points.
> >>
> >>  1.1.2 void thermal_zone_device_unregister(struct thermal_zone_device *tz)
> >>
> >> @@ -153,6 +155,7 @@ Thermal zone device sys I/F, created once it's registered:
> >>      |---trip_point_[0-*]_temp:       Trip point temperature
> >>      |---trip_point_[0-*]_type:       Trip point type
> >>      |---trip_point_[0-*]_hyst:       Hysteresis value for this trip point
> >> +    |---emul_temp:           Emulated temperature set node
> >>
> >>  Thermal cooling device sys I/F, created once it's registered:
> >>  /sys/class/thermal/cooling_device[0-*]:
> >> @@ -252,6 +255,17 @@ passive
> >>       Valid values: 0 (disabled) or greater than 1000
> >>       RW, Optional
> >>
> >> +emul_temp
> >> +     Interface to set the emulated temperature method in thermal zone
> >> +     (sensor). After setting this feature thermal zone must report
> >> +     this temperature and not the actual temperature. The actual
> >> +     implementation of this emulated temperature is platform specific.
> >
> > can we have a pure software temperature emulation method?
> > say, the generic thermal layer caches the emulated temperature value,
> > and hook it in update_temperature()?
> > This is also useful for testing in polling mode, and it does not require
> > platform specific callback support. I mean thermal_ops->set_emul_temp is
> > optional, but thermal emulation is always available for all platforms.
> Yes It makes sense and we can have pure software emulation and use the
> cached temperature when no platform call is registered. In my case I
> needed this in h/w so to have the same sensor trigger interrupts
> behaviour.
> 
> So the code flow can be like this,
> 
> #ifdef CONFIG_THERMAL_EMULATION
> if (thermal_ops->set_emul_temp)
> then pass emul_temp to platform and use the normal platform
> thermal_ops->get_temp
> else
> Store it locally and use emul_temp  instead of calling platform
> thermal_ops->get_temp
> #endif
> 
No.
We should not support emulation is CONFIG_THERMAL_EMULATION is cleared.
And further more, for pure software emulation, we should check if the
real temperature reaches critical trip point.

thanks,
rui
> I will re-submit with this change.
> 
> Thanks,
> Amit
> >
> > thanks,
> > rui
> >> +     This is useful in debugging different temperature threshold and its
> >> +     associated cooling action. Writing 0 on this node should disable
> >> +     emulation.
> >> +     Unit: millidegree Celsius
> >> +     WO, Optional
> >> +
> >>  *****************************
> >>  * Cooling device attributes *
> >>  *****************************
> >> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> >> index 8c8ce80..ecdfc7d 100644
> >> --- a/drivers/thermal/thermal_sys.c
> >> +++ b/drivers/thermal/thermal_sys.c
> >> @@ -700,11 +700,31 @@ policy_show(struct device *dev, struct device_attribute *devattr, char *buf)
> >>       return sprintf(buf, "%s\n", tz->governor->name);
> >>  }
> >>
> >> +static ssize_t
> >> +emul_temp_store(struct device *dev, struct device_attribute *attr,
> >> +                  const char *buf, size_t count)
> >> +{
> >> +     struct thermal_zone_device *tz = to_thermal_zone(dev);
> >> +     int ret;
> >> +     unsigned long temperature;
> >> +
> >> +     if (!tz->ops->set_emul_temp)
> >> +             return -EPERM;
> >> +
> >> +     if (kstrtoul(buf, 10, &temperature))
> >> +             return -EINVAL;
> >> +
> >> +     ret = tz->ops->set_emul_temp(tz, temperature);
> >> +
> >> +     return ret ? ret : count;
> >> +}
> >> +
> >>  static DEVICE_ATTR(type, 0444, type_show, NULL);
> >>  static DEVICE_ATTR(temp, 0444, temp_show, NULL);
> >>  static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
> >>  static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, passive_store);
> >>  static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);
> >> +static DEVICE_ATTR(emul_temp, S_IWUSR, NULL, emul_temp_store);
> >>
> >>  /* sys I/F for cooling device */
> >>  #define to_cooling_device(_dev)      \
> >> @@ -1592,6 +1612,12 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
> >>                       goto unregister;
> >>       }
> >>
> >> +     if (ops->set_emul_temp) {
> >> +             result = device_create_file(&tz->device, &dev_attr_emul_temp);
> >> +             if (result)
> >> +                     goto unregister;
> >> +     }
> >> +
> >>       /* Create policy attribute */
> >>       result = device_create_file(&tz->device, &dev_attr_policy);
> >>       if (result)
> >> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> >> index 883bcda..fbb87d4 100644
> >> --- a/include/linux/thermal.h
> >> +++ b/include/linux/thermal.h
> >> @@ -123,6 +123,7 @@ struct thermal_zone_device_ops {
> >>       int (*set_trip_hyst) (struct thermal_zone_device *, int,
> >>                             unsigned long);
> >>       int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
> >> +     int (*set_emul_temp) (struct thermal_zone_device *, unsigned long);
> >>       int (*get_trend) (struct thermal_zone_device *, int,
> >>                         enum thermal_trend *);
> >>       int (*notify) (struct thermal_zone_device *, int,
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] thermal: sysfs: Add a new sysfs node emul_temp

[amit kachhap]

On Mon, Jan 21, 2013 at 7:20 PM, Zhang Rui <rui.zhang@intel.com> wrote:
> On Wed, 2013-01-16 at 11:30 -0800, amit kachhap wrote:
>> Hi Rui,
>>
>> Thanks for the review comments,
>> On Tue, Jan 15, 2013 at 11:33 PM, Zhang Rui <rui.zhang@intel.com> wrote:
>> > Hi, Amit,
>> >
>> > On Sun, 2013-01-06 at 16:08 -0800, Amit Daniel Kachhap wrote:
>> >> This patch adds support to set the emulated temperature method in
>> >> thermal zone (sensor). After setting this feature thermal zone must
>> >> report this temperature and not the actual temperature. The actual
>> >> implementation of this emulated temperature is based on sensor
>> >> capability or platform specific. This is useful in debugging different
>> >> temperature threshold and its associated cooling action. Writing 0 on
>> >> this node should disable emulation.
>> >
>> > Question:
>> > will this bring hardware issue? Say, critical temperature reached while
>> > in emulation mode?
>> No emulation does cause any h/w issue.
>> >
>> > As this is for debug purpose, I'd prefer to have a seperate Kconfig
>> > option for this feature.
>> Yes agreed. Will re-submit with kconfig option.
>> >
>> >> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> >> ---
>> >>  Documentation/thermal/sysfs-api.txt |   14 ++++++++++++++
>> >>  drivers/thermal/thermal_sys.c       |   26 ++++++++++++++++++++++++++
>> >>  include/linux/thermal.h             |    1 +
>> >>  3 files changed, 41 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
>> >> index 88c0233..e8f2ee4 100644
>> >> --- a/Documentation/thermal/sysfs-api.txt
>> >> +++ b/Documentation/thermal/sysfs-api.txt
>> >> @@ -55,6 +55,8 @@ temperature) and throttle appropriate devices.
>> >>       .get_trip_type: get the type of certain trip point.
>> >>       .get_trip_temp: get the temperature above which the certain trip point
>> >>                       will be fired.
>> >> +     .set_emul_temp: set the emulation temperature which helps in debugging
>> >> +                     different threshold temperature points.
>> >>
>> >>  1.1.2 void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>> >>
>> >> @@ -153,6 +155,7 @@ Thermal zone device sys I/F, created once it's registered:
>> >>      |---trip_point_[0-*]_temp:       Trip point temperature
>> >>      |---trip_point_[0-*]_type:       Trip point type
>> >>      |---trip_point_[0-*]_hyst:       Hysteresis value for this trip point
>> >> +    |---emul_temp:           Emulated temperature set node
>> >>
>> >>  Thermal cooling device sys I/F, created once it's registered:
>> >>  /sys/class/thermal/cooling_device[0-*]:
>> >> @@ -252,6 +255,17 @@ passive
>> >>       Valid values: 0 (disabled) or greater than 1000
>> >>       RW, Optional
>> >>
>> >> +emul_temp
>> >> +     Interface to set the emulated temperature method in thermal zone
>> >> +     (sensor). After setting this feature thermal zone must report
>> >> +     this temperature and not the actual temperature. The actual
>> >> +     implementation of this emulated temperature is platform specific.
>> >
>> > can we have a pure software temperature emulation method?
>> > say, the generic thermal layer caches the emulated temperature value,
>> > and hook it in update_temperature()?
>> > This is also useful for testing in polling mode, and it does not require
>> > platform specific callback support. I mean thermal_ops->set_emul_temp is
>> > optional, but thermal emulation is always available for all platforms.
>> Yes It makes sense and we can have pure software emulation and use the
>> cached temperature when no platform call is registered. In my case I
>> needed this in h/w so to have the same sensor trigger interrupts
>> behaviour.
>>
>> So the code flow can be like this,
>>
>> #ifdef CONFIG_THERMAL_EMULATION
>> if (thermal_ops->set_emul_temp)
>> then pass emul_temp to platform and use the normal platform
>> thermal_ops->get_temp
>> else
>> Store it locally and use emul_temp  instead of calling platform
>> thermal_ops->get_temp
>> #endif
>>
> No.
> We should not support emulation is CONFIG_THERMAL_EMULATION is cleared.
> And further more, for pure software emulation, we should check if the
> real temperature reaches critical trip point.
Yes agreed. Submitted the V2 version with your suggestion.

Thanks,
Amit Daniel

>
> thanks,
> rui
>> I will re-submit with this change.
>>
>> Thanks,
>> Amit
>> >
>> > thanks,
>> > rui
>> >> +     This is useful in debugging different temperature threshold and its
>> >> +     associated cooling action. Writing 0 on this node should disable
>> >> +     emulation.
>> >> +     Unit: millidegree Celsius
>> >> +     WO, Optional
>> >> +
>> >>  *****************************
>> >>  * Cooling device attributes *
>> >>  *****************************
>> >> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
>> >> index 8c8ce80..ecdfc7d 100644
>> >> --- a/drivers/thermal/thermal_sys.c
>> >> +++ b/drivers/thermal/thermal_sys.c
>> >> @@ -700,11 +700,31 @@ policy_show(struct device *dev, struct device_attribute *devattr, char *buf)
>> >>       return sprintf(buf, "%s\n", tz->governor->name);
>> >>  }
>> >>
>> >> +static ssize_t
>> >> +emul_temp_store(struct device *dev, struct device_attribute *attr,
>> >> +                  const char *buf, size_t count)
>> >> +{
>> >> +     struct thermal_zone_device *tz = to_thermal_zone(dev);
>> >> +     int ret;
>> >> +     unsigned long temperature;
>> >> +
>> >> +     if (!tz->ops->set_emul_temp)
>> >> +             return -EPERM;
>> >> +
>> >> +     if (kstrtoul(buf, 10, &temperature))
>> >> +             return -EINVAL;
>> >> +
>> >> +     ret = tz->ops->set_emul_temp(tz, temperature);
>> >> +
>> >> +     return ret ? ret : count;
>> >> +}
>> >> +
>> >>  static DEVICE_ATTR(type, 0444, type_show, NULL);
>> >>  static DEVICE_ATTR(temp, 0444, temp_show, NULL);
>> >>  static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
>> >>  static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, passive_store);
>> >>  static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);
>> >> +static DEVICE_ATTR(emul_temp, S_IWUSR, NULL, emul_temp_store);
>> >>
>> >>  /* sys I/F for cooling device */
>> >>  #define to_cooling_device(_dev)      \
>> >> @@ -1592,6 +1612,12 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
>> >>                       goto unregister;
>> >>       }
>> >>
>> >> +     if (ops->set_emul_temp) {
>> >> +             result = device_create_file(&tz->device, &dev_attr_emul_temp);
>> >> +             if (result)
>> >> +                     goto unregister;
>> >> +     }
>> >> +
>> >>       /* Create policy attribute */
>> >>       result = device_create_file(&tz->device, &dev_attr_policy);
>> >>       if (result)
>> >> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> >> index 883bcda..fbb87d4 100644
>> >> --- a/include/linux/thermal.h
>> >> +++ b/include/linux/thermal.h
>> >> @@ -123,6 +123,7 @@ struct thermal_zone_device_ops {
>> >>       int (*set_trip_hyst) (struct thermal_zone_device *, int,
>> >>                             unsigned long);
>> >>       int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
>> >> +     int (*set_emul_temp) (struct thermal_zone_device *, unsigned long);
>> >>       int (*get_trend) (struct thermal_zone_device *, int,
>> >>                         enum thermal_trend *);
>> >>       int (*notify) (struct thermal_zone_device *, int,
>> >
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> > Please read the FAQ at  http://www.tux.org/lkml/
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>