Re: [PATCH v2] thermal: consistently use int for temperatures
On Fri 2015-07-24 15:49:41, Guenter Roeck wrote: On 07/24/2015 03:11 PM, Pavel Machek wrote: On Fri 2015-07-24 06:59:26, Guenter Roeck wrote: On 07/23/2015 11:29 PM, Sascha Hauer wrote: On Thu, Jul 23, 2015 at 02:07:59PM +0200, Pavel Machek wrote: On Tue 2015-07-21 09:21:32, Sascha Hauer wrote: The thermal code uses int, long and unsigned long for temperatures in different places. Using an unsigned type limits the thermal framework to positive temperatures without need. Also several drivers currently will report temperatures near UINT_MAX for temperatures below 0°C. This will probably immediately shut the machine down due to overtemperature if started below 0°C. 'long' is 64bit on several architectures. This is not needed since INT_MAX °mC is above the melting point of all known materials. Can we do something like typedef millicelsius_t int; ...to document the units? I am not very fond of typedefs and I am not sure this adds any value. I could change it when more people ask for it, but I just sent the new version without this. I thought we are supposed to not introduce new typedefs anyway. You are not supposed to typedef struct, but typedef for millicelsius_t would be ok. And it is your only chance if you want people to pay attention. If you make it int, someone will pass it to long or something else.. Seems to me that would be just lazyness. The same person might use 'long' even if millicelsius_t is defined. A typedef doesn't preclude people from ignoring it. Well, millicelsius_t will tell people that this is temperature, and being special type, they'll try to preserve it. (And you could check with sparse if you really wanted). int and long are common enough so that people will not notice... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] thermal: consistently use int for temperatures
On Thu, Jul 23, 2015 at 02:07:59PM +0200, Pavel Machek wrote: On Tue 2015-07-21 09:21:32, Sascha Hauer wrote: The thermal code uses int, long and unsigned long for temperatures in different places. Using an unsigned type limits the thermal framework to positive temperatures without need. Also several drivers currently will report temperatures near UINT_MAX for temperatures below 0°C. This will probably immediately shut the machine down due to overtemperature if started below 0°C. 'long' is 64bit on several architectures. This is not needed since INT_MAX °mC is above the melting point of all known materials. Can we do something like typedef millicelsius_t int; ...to document the units? I am not very fond of typedefs and I am not sure this adds any value. I could change it when more people ask for it, but I just sent the new version without this. 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- | -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] thermal: consistently use int for temperatures
On 07/24/2015 03:11 PM, Pavel Machek wrote: On Fri 2015-07-24 06:59:26, Guenter Roeck wrote: On 07/23/2015 11:29 PM, Sascha Hauer wrote: On Thu, Jul 23, 2015 at 02:07:59PM +0200, Pavel Machek wrote: On Tue 2015-07-21 09:21:32, Sascha Hauer wrote: The thermal code uses int, long and unsigned long for temperatures in different places. Using an unsigned type limits the thermal framework to positive temperatures without need. Also several drivers currently will report temperatures near UINT_MAX for temperatures below 0°C. This will probably immediately shut the machine down due to overtemperature if started below 0°C. 'long' is 64bit on several architectures. This is not needed since INT_MAX °mC is above the melting point of all known materials. Can we do something like typedef millicelsius_t int; ...to document the units? I am not very fond of typedefs and I am not sure this adds any value. I could change it when more people ask for it, but I just sent the new version without this. I thought we are supposed to not introduce new typedefs anyway. You are not supposed to typedef struct, but typedef for millicelsius_t would be ok. And it is your only chance if you want people to pay attention. If you make it int, someone will pass it to long or something else.. Seems to me that would be just lazyness. The same person might use 'long' even if millicelsius_t is defined. A typedef doesn't preclude people from ignoring it. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] thermal: consistently use int for temperatures
On 07/23/2015 11:29 PM, Sascha Hauer wrote: On Thu, Jul 23, 2015 at 02:07:59PM +0200, Pavel Machek wrote: On Tue 2015-07-21 09:21:32, Sascha Hauer wrote: The thermal code uses int, long and unsigned long for temperatures in different places. Using an unsigned type limits the thermal framework to positive temperatures without need. Also several drivers currently will report temperatures near UINT_MAX for temperatures below 0°C. This will probably immediately shut the machine down due to overtemperature if started below 0°C. 'long' is 64bit on several architectures. This is not needed since INT_MAX °mC is above the melting point of all known materials. Can we do something like typedef millicelsius_t int; ...to document the units? I am not very fond of typedefs and I am not sure this adds any value. I could change it when more people ask for it, but I just sent the new version without this. I thought we are supposed to not introduce new typedefs anyway. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] thermal: consistently use int for temperatures
On Fri 2015-07-24 06:59:26, Guenter Roeck wrote: On 07/23/2015 11:29 PM, Sascha Hauer wrote: On Thu, Jul 23, 2015 at 02:07:59PM +0200, Pavel Machek wrote: On Tue 2015-07-21 09:21:32, Sascha Hauer wrote: The thermal code uses int, long and unsigned long for temperatures in different places. Using an unsigned type limits the thermal framework to positive temperatures without need. Also several drivers currently will report temperatures near UINT_MAX for temperatures below 0°C. This will probably immediately shut the machine down due to overtemperature if started below 0°C. 'long' is 64bit on several architectures. This is not needed since INT_MAX °mC is above the melting point of all known materials. Can we do something like typedef millicelsius_t int; ...to document the units? I am not very fond of typedefs and I am not sure this adds any value. I could change it when more people ask for it, but I just sent the new version without this. I thought we are supposed to not introduce new typedefs anyway. You are not supposed to typedef struct, but typedef for millicelsius_t would be ok. And it is your only chance if you want people to pay attention. If you make it int, someone will pass it to long or something else.. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] thermal: consistently use int for temperatures
As the original patch has not been in upstream, I'd prefer a refreshed patch,
rather than an incremental fix.
Thanks,
rui
-Original Message-
From: Sascha Hauer [mailto:s.ha...@pengutronix.de]
Sent: Thursday, July 23, 2015 6:38 PM
To: Zhang, Rui
Cc: Punit Agrawal; linux...@vger.kernel.org; Eduardo Valentin; linux-
ker...@vger.kernel.org; Jean Delvare; Peter Feuerer; Heiko Stuebner;
Lukasz Majewski; Stephen Warren; Thierry Reding; linux-
a...@vger.kernel.org; platform-driver-...@vger.kernel.org; linux-arm-
ker...@lists.infradead.org; linux-o...@vger.kernel.org; linux-samsung-
s...@vger.kernel.org; Guenter Roeck; Rafael J. Wysocki; Maxime Ripard;
Darren Hart; lm-sens...@lm-sensors.org
Subject: Re: [PATCH v2] thermal: consistently use int for temperatures
Importance: High
Hi Zhang,
On Tue, Jul 21, 2015 at 01:35:31PM +, Zhang, Rui wrote:
Patch applied.
Thanks for applying. I missed to convert another place, so we get a new
compiler warning. The attached patch fixes this (suitable for git rebase --
autosquash). Please let me know if you can handle this or if you prefer a new
patch instead.
Thanks
Sascha
-8-
From 4907a7c32fd16eaf9f31d9f904276c9a0176b717 Mon Sep 17 00:00:00 2001
From: Sascha Hauer s.ha...@pengutronix.de
Date: Thu, 23 Jul 2015 12:32:31 +0200
Subject: [PATCH] fixup! thermal: consistently use int for temperatures
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
fixes:
drivers/power/charger-manager.c: In function
‘cm_get_battery_temperature’:
drivers/power/charger-manager.c:622:45: warning: passing argument 2 of
‘thermal_zone_get_temp’ from incompatible pointer type
ret = thermal_zone_get_temp(cm-tzd_batt, (unsigned long *)temp);
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
---
drivers/power/charger-manager.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-
manager.c index 1c202cc..907293e 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -619,7 +619,7 @@ static int cm_get_battery_temperature(struct
charger_manager *cm,
#ifdef CONFIG_THERMAL
if (cm-tzd_batt) {
- ret = thermal_zone_get_temp(cm-tzd_batt, (unsigned long
*)temp);
+ ret = thermal_zone_get_temp(cm-tzd_batt, temp);
if (!ret)
/* Calibrate temperature unit */
*temp /= 100;
--
2.1.4
--
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- |
Re: [PATCH v2] thermal: consistently use int for temperatures
On Tue 2015-07-21 09:21:32, Sascha Hauer wrote: The thermal code uses int, long and unsigned long for temperatures in different places. Using an unsigned type limits the thermal framework to positive temperatures without need. Also several drivers currently will report temperatures near UINT_MAX for temperatures below 0°C. This will probably immediately shut the machine down due to overtemperature if started below 0°C. 'long' is 64bit on several architectures. This is not needed since INT_MAX °mC is above the melting point of all known materials. Can we do something like typedef millicelsius_t int; ...to document the units? Consistently use a plain 'int' for temperatures throughout the thermal code and the drivers. This only changes the places in the drivers where the temperature is passed around as pointer, when drivers internally use another type this is not changed. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] thermal: consistently use int for temperatures
Hi Zhang,
On Tue, Jul 21, 2015 at 01:35:31PM +, Zhang, Rui wrote:
Patch applied.
Thanks for applying. I missed to convert another place, so we get a new
compiler warning. The attached patch fixes this (suitable for git rebase
--autosquash). Please let me know if you can handle this or if you
prefer a new patch instead.
Thanks
Sascha
-8-
From 4907a7c32fd16eaf9f31d9f904276c9a0176b717 Mon Sep 17 00:00:00 2001
From: Sascha Hauer s.ha...@pengutronix.de
Date: Thu, 23 Jul 2015 12:32:31 +0200
Subject: [PATCH] fixup! thermal: consistently use int for temperatures
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
fixes:
drivers/power/charger-manager.c: In function ‘cm_get_battery_temperature’:
drivers/power/charger-manager.c:622:45: warning: passing argument 2 of
‘thermal_zone_get_temp’ from incompatible pointer type
ret = thermal_zone_get_temp(cm-tzd_batt, (unsigned long *)temp);
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
---
drivers/power/charger-manager.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index 1c202cc..907293e 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -619,7 +619,7 @@ static int cm_get_battery_temperature(struct
charger_manager *cm,
#ifdef CONFIG_THERMAL
if (cm-tzd_batt) {
- ret = thermal_zone_get_temp(cm-tzd_batt, (unsigned long
*)temp);
+ ret = thermal_zone_get_temp(cm-tzd_batt, temp);
if (!ret)
/* Calibrate temperature unit */
*temp /= 100;
--
2.1.4
--
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] thermal: consistently use int for temperatures
Hi Sascha, Sascha Hauer s.ha...@pengutronix.de writes: The thermal code uses int, long and unsigned long for temperatures in different places. Using an unsigned type limits the thermal framework to positive temperatures without need. Also several drivers currently will report temperatures near UINT_MAX for temperatures below 0°C. This will probably immediately shut the machine down due to overtemperature if started below 0°C. 'long' is 64bit on several architectures. This is not needed since INT_MAX °mC is above the melting point of all known materials. Consistently use a plain 'int' for temperatures throughout the thermal code and the drivers. This only changes the places in the drivers where the temperature is passed around as pointer, when drivers internally use another type this is not changed. Signed-off-by: Sascha Hauer s.ha...@pengutronix.de Acked-by: Geert Uytterhoeven geert+rene...@glider.be Reviewed-by: Jean Delvare jdelv...@suse.de Reviewed-by: Lukasz Majewski l.majew...@samsung.com Reviewed-by: Darren Hart dvh...@linux.intel.com Reviewed-by: Heiko Stuebner he...@sntech.de Reviewed-by: Peter Feuerer pe...@piie.net Cc: Punit Agrawal punit.agra...@arm.com The changes below look good. For the power_allocator governor and corresponding trace events, Reviewed-by: Punit Agrawal punit.agra...@arm.com Thanks Cc: Zhang Rui rui.zh...@intel.com Cc: Eduardo Valentin edubez...@gmail.com Cc: linux...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: Jean Delvare jdelv...@suse.de Cc: Peter Feuerer pe...@piie.net Cc: Heiko Stuebner he...@sntech.de Cc: Lukasz Majewski l.majew...@samsung.com Cc: Stephen Warren swar...@wwwdotorg.org Cc: Thierry Reding thierry.red...@gmail.com Cc: linux-a...@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org Cc: linux-o...@vger.kernel.org Cc: linux-samsung-soc@vger.kernel.org Cc: Guenter Roeck li...@roeck-us.net Cc: Rafael J. Wysocki r...@rjwysocki.net Cc: Maxime Ripard maxime.rip...@free-electrons.com Cc: Darren Hart dvh...@infradead.org Cc: lm-sens...@lm-sensors.org --- changes since v1: - Add missing pieces for power_allocator driver drivers/acpi/thermal.c | 12 +- drivers/hwmon/lm75.c | 2 +- drivers/hwmon/ntc_thermistor.c | 2 +- drivers/hwmon/tmp102.c | 2 +- drivers/input/touchscreen/sun4i-ts.c | 8 +++ drivers/platform/x86/acerhdf.c | 9 drivers/platform/x86/intel_mid_thermal.c | 9 drivers/power/power_supply_core.c | 2 +- drivers/thermal/armada_thermal.c | 2 +- drivers/thermal/db8500_thermal.c | 7 +++--- drivers/thermal/dove_thermal.c | 2 +- drivers/thermal/fair_share.c | 2 +- drivers/thermal/gov_bang_bang.c| 5 ++-- drivers/thermal/hisi_thermal.c | 4 ++-- drivers/thermal/imx_thermal.c | 27 +++--- drivers/thermal/int340x_thermal/int3400_thermal.c | 2 +- .../thermal/int340x_thermal/int340x_thermal_zone.c | 10 .../thermal/int340x_thermal/int340x_thermal_zone.h | 8 +++ .../int340x_thermal/processor_thermal_device.c | 4 ++-- drivers/thermal/intel_quark_dts_thermal.c | 13 +-- drivers/thermal/intel_soc_dts_iosf.c | 8 +++ drivers/thermal/kirkwood_thermal.c | 2 +- drivers/thermal/of-thermal.c | 14 +-- drivers/thermal/power_allocator.c | 16 ++--- drivers/thermal/qcom-spmi-temp-alarm.c | 2 +- drivers/thermal/rcar_thermal.c | 7 +++--- drivers/thermal/rockchip_thermal.c | 10 drivers/thermal/samsung/exynos_tmu.c | 23 +- drivers/thermal/spear_thermal.c| 2 +- drivers/thermal/st/st_thermal.c| 5 ++-- drivers/thermal/step_wise.c| 4 ++-- drivers/thermal/tegra_soctherm.c | 4 ++-- drivers/thermal/thermal_core.c | 26 ++--- drivers/thermal/thermal_hwmon.c| 10 drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 10 drivers/thermal/x86_pkg_temp_thermal.c | 10 include/linux/thermal.h| 26 + include/trace/events/thermal_power_allocator.h | 6 ++--- 38 files changed, 151 insertions(+), 166 deletions(-) [...] diff --git a/drivers/thermal/power_allocator.c b/drivers/thermal/power_allocator.c index 4672250..045aea59 100644 --- a/drivers/thermal/power_allocator.c +++ b/drivers/thermal/power_allocator.c @@ -92,8 +92,8
