| Message ID | 1427385240-6086-2-git-send-email-s.hauer@pengutronix.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Eduardo Valentin |
| Headers | show |
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(-) > [...] -- 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
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
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 |
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 , ?
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 --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;
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(-)