diff mbox

[01/13] thermal: Make temperatures consistently unsigned long

Message ID 1427385240-6086-2-git-send-email-s.hauer@pengutronix.de (mailing list archive)
State New, archived
Headers show

Commit Message

Sascha Hauer March 26, 2015, 3:53 p.m. UTC
The thermal framework uses int, long and unsigned long for temperatures
in millicelsius. The majority of functions uses unsigned long, so change
the remaining functions to use this type aswell.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/thermal/thermal_core.c | 10 +++++-----
 include/linux/thermal.h        |  6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

Comments

Punit Agrawal March 27, 2015, 10:18 a.m. UTC | #1
Hi Sascha,

Sascha Hauer <s.hauer@pengutronix.de> writes:

> The thermal framework uses int, long and unsigned long for temperatures
> in millicelsius. The majority of functions uses unsigned long, so change
> the remaining functions to use this type aswell.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

I'd suggest changing to long instead. It would allow the use of the
thermal framework in environments where temperatures are below 0C -
quite easily reached in many parts of the world.

Regards,
Punit

> ---
>  drivers/thermal/thermal_core.c | 10 +++++-----
>  include/linux/thermal.h        |  6 +++---
>  2 files changed, 8 insertions(+), 8 deletions(-)
>

[...]
Sascha Hauer March 27, 2015, 7:07 p.m. UTC | #2
On Fri, Mar 27, 2015 at 10:18:14AM +0000, Punit Agrawal wrote:
> Hi Sascha,
> 
> Sascha Hauer <s.hauer@pengutronix.de> writes:
> 
> > The thermal framework uses int, long and unsigned long for temperatures
> > in millicelsius. The majority of functions uses unsigned long, so change
> > the remaining functions to use this type aswell.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> 
> I'd suggest changing to long instead. It would allow the use of the
> thermal framework in environments where temperatures are below 0C -
> quite easily reached in many parts of the world.

I agree to use a signed type. I also found it not so nice that the thermal
core does not support negative temperatures. I only chose unsigned long
because the patch got smallest that way, but I already expected this
answer ;)
We could also use int instead of long. INT_MAX °mC is still enough for using
a computer on the surface of the sun (Not for the center though)

Sascha
Eduardo Valentin April 7, 2015, 1:47 a.m. UTC | #3
On Fri, Mar 27, 2015 at 08:07:50PM +0100, Sascha Hauer wrote:
> On Fri, Mar 27, 2015 at 10:18:14AM +0000, Punit Agrawal wrote:
> > Hi Sascha,
> > 
> > Sascha Hauer <s.hauer@pengutronix.de> writes:
> > 
> > > The thermal framework uses int, long and unsigned long for temperatures
> > > in millicelsius. The majority of functions uses unsigned long, so change
> > > the remaining functions to use this type aswell.
> > >
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > 
> > I'd suggest changing to long instead. It would allow the use of the
> > thermal framework in environments where temperatures are below 0C -
> > quite easily reached in many parts of the world.
> 
> I agree to use a signed type. I also found it not so nice that the thermal
> core does not support negative temperatures. I only chose unsigned long
> because the patch got smallest that way, but I already expected this
> answer ;)
> We could also use int instead of long. INT_MAX °mC is still enough for using
> a computer on the surface of the sun (Not for the center though)

Agreed, int is the preferred type.


> 
> Sascha
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Pavel Machek April 27, 2015, 8:36 p.m. UTC | #4
On Thu 2015-03-26 16:53:48, Sascha Hauer wrote:
> The thermal framework uses int, long and unsigned long for temperatures
> in millicelsius. The majority of functions uses unsigned long, so change
> the remaining functions to use this type aswell.

Maybe millicelsius_t should be introduced, so that readers immediately know
what type it is? Some parts of kernel also use decicelsius and decikelvin, IIRC (ACPI).

									Pavel

>  	if (trip_type == THERMAL_TRIP_CRITICAL) {
>  		dev_emerg(&tz->device,
> -			  "critical temperature reached(%d C),shutting down\n",
> +			  "critical temperature reached(%lu C),shutting down\n",

space after , ?
Sascha Hauer April 28, 2015, 7:42 a.m. UTC | #5
On Mon, Apr 27, 2015 at 10:36:08PM +0200, Pavel Machek wrote:
> On Thu 2015-03-26 16:53:48, Sascha Hauer wrote:
> > The thermal framework uses int, long and unsigned long for temperatures
> > in millicelsius. The majority of functions uses unsigned long, so change
> > the remaining functions to use this type aswell.
> 
> Maybe millicelsius_t should be introduced, so that readers immediately know
> what type it is? Some parts of kernel also use decicelsius and decikelvin, IIRC (ACPI).

I have no strong opinion on this. I'll send out the next version without
typedef for now. Since in my patch I identified most places which need a
type change anyway it might be a good opportunity to change to a typedef
if we want to. Any other opinions on this?

> 
> 									Pavel
> 
> >  	if (trip_type == THERMAL_TRIP_CRITICAL) {
> >  		dev_emerg(&tz->device,
> > -			  "critical temperature reached(%d C),shutting down\n",
> > +			  "critical temperature reached(%lu C),shutting down\n",
> 
> space after , ?

Turns out that in the new version of this series I don't have to modify
this line anymore. Made this a separate patch.

Sascha
diff mbox

Patch

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 174d3bc..0e4ad7c 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -378,7 +378,7 @@  static void handle_critical_trips(struct thermal_zone_device *tz,
 
 	if (trip_type == THERMAL_TRIP_CRITICAL) {
 		dev_emerg(&tz->device,
-			  "critical temperature reached(%d C),shutting down\n",
+			  "critical temperature reached(%lu C),shutting down\n",
 			  tz->temperature / 1000);
 		orderly_poweroff(true);
 	}
@@ -453,7 +453,7 @@  EXPORT_SYMBOL_GPL(thermal_zone_get_temp);
 
 static void update_temperature(struct thermal_zone_device *tz)
 {
-	long temp;
+	unsigned long temp;
 	int ret;
 
 	ret = thermal_zone_get_temp(tz, &temp);
@@ -469,7 +469,7 @@  static void update_temperature(struct thermal_zone_device *tz)
 	mutex_unlock(&tz->lock);
 
 	trace_thermal_temperature(tz);
-	dev_dbg(&tz->device, "last_temperature=%d, current_temperature=%d\n",
+	dev_dbg(&tz->device, "last_temperature=%lu, current_temperature=%lu\n",
 				tz->last_temperature, tz->temperature);
 }
 
@@ -512,7 +512,7 @@  static ssize_t
 temp_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
-	long temperature;
+	unsigned long temperature;
 	int ret;
 
 	ret = thermal_zone_get_temp(tz, &temperature);
@@ -520,7 +520,7 @@  temp_show(struct device *dev, struct device_attribute *attr, char *buf)
 	if (ret)
 		return ret;
 
-	return sprintf(buf, "%ld\n", temperature);
+	return sprintf(buf, "%lu\n", temperature);
 }
 
 static ssize_t
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 5eac316..db6c12b 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -180,9 +180,9 @@  struct thermal_zone_device {
 	int trips;
 	int passive_delay;
 	int polling_delay;
-	int temperature;
-	int last_temperature;
-	int emul_temperature;
+	unsigned long temperature;
+	unsigned long last_temperature;
+	unsigned long emul_temperature;
 	int passive;
 	unsigned int forced_passive;
 	struct thermal_zone_device_ops *ops;