This series adds support for hardware trip points. It picks up earlier work from Mikko Perttunen. Mikko implemented hardware trip points as part of the device tree support. It was suggested back then to move the functionality to the thermal core instead of putting more code into the device tree support. This series does exactly that. The majority of the patches are cleanups and fixes. Only the last two patches actually implement the hardware trip points. All comments welcome Sascha
This series adds support for hardware trip points. It picks up earlier work from Mikko Perttunen. Mikko implemented hardware trip points as part of the device tree support. It was suggested back then to move the functionality to the thermal core instead of putting more code into the device tree support. This series does exactly that. The majority of the patches are cleanups and fixes. Only the last two patches actually implement the hardware trip points. All comments welcome Sascha
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(-) 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; -- 2.1.4
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-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> --- drivers/thermal/thermal_core.c | 10 +++++----- include/linux/thermal.h | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) 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; -- 2.1.4
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(-) 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; -- 2.1.4
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/thermal_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 0e4ad7c..c735ac4c 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -402,7 +402,7 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip) } /** - * thermal_zone_get_temp() - returns its the temperature of thermal zone + * thermal_zone_get_temp() - returns the temperature of a thermal zone * @tz: a valid pointer to a struct thermal_zone_device * @temp: a valid pointer to where to store the resulting temperature. * -- 2.1.4
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/thermal_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 0e4ad7c..c735ac4c 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -402,7 +402,7 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip) } /** - * thermal_zone_get_temp() - returns its the temperature of thermal zone + * thermal_zone_get_temp() - returns the temperature of a thermal zone * @tz: a valid pointer to a struct thermal_zone_device * @temp: a valid pointer to where to store the resulting temperature. * -- 2.1.4
When the thermal zone has no get_temp callback then thermal_zone_device_register() calls thermal_zone_device_set_polling() with a polling delay of 0. This only cancels the poll_queue. Since the poll_queue hasn't been scheduled this is a no-op. Remove it. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/thermal_core.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index c735ac4c..dcea909 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -1571,9 +1571,6 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type, INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check); - if (!tz->ops->get_temp) - thermal_zone_device_set_polling(tz, 0); - thermal_zone_device_update(tz); return tz; -- 2.1.4
When the thermal zone has no get_temp callback then thermal_zone_device_register() calls thermal_zone_device_set_polling() with a polling delay of 0. This only cancels the poll_queue. Since the poll_queue hasn't been scheduled this is a no-op. Remove it. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/thermal_core.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index c735ac4c..dcea909 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -1571,9 +1571,6 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type, INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check); - if (!tz->ops->get_temp) - thermal_zone_device_set_polling(tz, 0); - thermal_zone_device_update(tz); return tz; -- 2.1.4
commit e6e238c38 (thermal: sysfs: Add a new sysfs node emul_temp for thermal emulation) promised not to emulate critical temperatures, but the check for critical temperatures is broken in multiple ways: - The code should only accept an emulated temperature when the emulated temperature is lower than the critical temperature. Instead the code accepts an emulated temperature whenever the real temperature is lower than the critical temperature. This makes no sense and trying to emulate a temperature higher than the critical temperature halts the system. - When trying to emulate a higher-than-critical temperature we should either limit the emulated temperature to the maximum non critical temperature or refuse to emulate this temperature. Instead the code just silently ignores the emulated temperature and continues with the real temperature. This patch moves the test for illegal emulated temperature to the sysfs write function so that we can properly refuse illegal temperatures here. Trying to write illegal temperatures results in an error message. While at it use IS_ENABLED() instead of #ifdefs. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/thermal_core.c | 46 ++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index dcea909..ebca854 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -414,11 +414,6 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip) int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) { int ret = -EINVAL; -#ifdef CONFIG_THERMAL_EMULATION - int count; - unsigned long crit_temp = -1UL; - enum thermal_trip_type type; -#endif if (!tz || IS_ERR(tz) || !tz->ops->get_temp) goto exit; @@ -426,25 +421,10 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) mutex_lock(&tz->lock); ret = tz->ops->get_temp(tz, temp); -#ifdef CONFIG_THERMAL_EMULATION - if (!tz->emul_temperature) - goto skip_emul; - - for (count = 0; count < tz->trips; count++) { - ret = tz->ops->get_trip_type(tz, count, &type); - if (!ret && type == THERMAL_TRIP_CRITICAL) { - ret = tz->ops->get_trip_temp(tz, count, &crit_temp); - break; - } - } - - if (ret) - goto skip_emul; - if (*temp < crit_temp) + if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz->emul_temperature) *temp = tz->emul_temperature; -skip_emul: -#endif + mutex_unlock(&tz->lock); exit: return ret; @@ -788,10 +768,32 @@ emul_temp_store(struct device *dev, struct device_attribute *attr, struct thermal_zone_device *tz = to_thermal_zone(dev); int ret = 0; unsigned long temperature; + int trip; + unsigned long crit_temp; + enum thermal_trip_type type; if (kstrtoul(buf, 10, &temperature)) return -EINVAL; + for (trip = 0; trip < tz->trips; trip++) { + ret = tz->ops->get_trip_type(tz, trip, &type); + if (ret) + return ret; + + if (type != THERMAL_TRIP_CRITICAL) + continue; + + ret = tz->ops->get_trip_temp(tz, trip, &crit_temp); + if (ret) + return ret; + + if (temperature >= crit_temp) { + dev_err(&tz->device, "Will not emulate critical temperature %luC (tcrit=%luC)\n", + temperature / 1000, crit_temp / 1000); + return -EINVAL; + } + } + if (!tz->ops->set_emul_temp) { mutex_lock(&tz->lock); tz->emul_temperature = temperature; -- 2.1.4
commit e6e238c38 (thermal: sysfs: Add a new sysfs node emul_temp for thermal emulation) promised not to emulate critical temperatures, but the check for critical temperatures is broken in multiple ways: - The code should only accept an emulated temperature when the emulated temperature is lower than the critical temperature. Instead the code accepts an emulated temperature whenever the real temperature is lower than the critical temperature. This makes no sense and trying to emulate a temperature higher than the critical temperature halts the system. - When trying to emulate a higher-than-critical temperature we should either limit the emulated temperature to the maximum non critical temperature or refuse to emulate this temperature. Instead the code just silently ignores the emulated temperature and continues with the real temperature. This patch moves the test for illegal emulated temperature to the sysfs write function so that we can properly refuse illegal temperatures here. Trying to write illegal temperatures results in an error message. While at it use IS_ENABLED() instead of #ifdefs. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/thermal_core.c | 46 ++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index dcea909..ebca854 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -414,11 +414,6 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip) int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) { int ret = -EINVAL; -#ifdef CONFIG_THERMAL_EMULATION - int count; - unsigned long crit_temp = -1UL; - enum thermal_trip_type type; -#endif if (!tz || IS_ERR(tz) || !tz->ops->get_temp) goto exit; @@ -426,25 +421,10 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) mutex_lock(&tz->lock); ret = tz->ops->get_temp(tz, temp); -#ifdef CONFIG_THERMAL_EMULATION - if (!tz->emul_temperature) - goto skip_emul; - - for (count = 0; count < tz->trips; count++) { - ret = tz->ops->get_trip_type(tz, count, &type); - if (!ret && type == THERMAL_TRIP_CRITICAL) { - ret = tz->ops->get_trip_temp(tz, count, &crit_temp); - break; - } - } - - if (ret) - goto skip_emul; - if (*temp < crit_temp) + if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz->emul_temperature) *temp = tz->emul_temperature; -skip_emul: -#endif + mutex_unlock(&tz->lock); exit: return ret; @@ -788,10 +768,32 @@ emul_temp_store(struct device *dev, struct device_attribute *attr, struct thermal_zone_device *tz = to_thermal_zone(dev); int ret = 0; unsigned long temperature; + int trip; + unsigned long crit_temp; + enum thermal_trip_type type; if (kstrtoul(buf, 10, &temperature)) return -EINVAL; + for (trip = 0; trip < tz->trips; trip++) { + ret = tz->ops->get_trip_type(tz, trip, &type); + if (ret) + return ret; + + if (type != THERMAL_TRIP_CRITICAL) + continue; + + ret = tz->ops->get_trip_temp(tz, trip, &crit_temp); + if (ret) + return ret; + + if (temperature >= crit_temp) { + dev_err(&tz->device, "Will not emulate critical temperature %luC (tcrit=%luC)\n", + temperature / 1000, crit_temp / 1000); + return -EINVAL; + } + } + if (!tz->ops->set_emul_temp) { mutex_lock(&tz->lock); tz->emul_temperature = temperature; -- 2.1.4
Inline update_temperature into its only caller to make the code more readable. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/thermal_core.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index ebca854..6d0fdad 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -431,11 +431,15 @@ exit: } EXPORT_SYMBOL_GPL(thermal_zone_get_temp); -static void update_temperature(struct thermal_zone_device *tz) +void thermal_zone_device_update(struct thermal_zone_device *tz) { + int count; unsigned long temp; int ret; + if (!tz->ops->get_temp) + return; + ret = thermal_zone_get_temp(tz, &temp); if (ret) { dev_warn(&tz->device, "failed to read out thermal zone %d\n", @@ -451,16 +455,6 @@ static void update_temperature(struct thermal_zone_device *tz) trace_thermal_temperature(tz); dev_dbg(&tz->device, "last_temperature=%lu, current_temperature=%lu\n", tz->last_temperature, tz->temperature); -} - -void thermal_zone_device_update(struct thermal_zone_device *tz) -{ - int count; - - if (!tz->ops->get_temp) - return; - - update_temperature(tz); for (count = 0; count < tz->trips; count++) handle_thermal_trip(tz, count); -- 2.1.4
Inline update_temperature into its only caller to make the code more readable. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/thermal_core.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index ebca854..6d0fdad 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -431,11 +431,15 @@ exit: } EXPORT_SYMBOL_GPL(thermal_zone_get_temp); -static void update_temperature(struct thermal_zone_device *tz) +void thermal_zone_device_update(struct thermal_zone_device *tz) { + int count; unsigned long temp; int ret; + if (!tz->ops->get_temp) + return; + ret = thermal_zone_get_temp(tz, &temp); if (ret) { dev_warn(&tz->device, "failed to read out thermal zone %d\n", @@ -451,16 +455,6 @@ static void update_temperature(struct thermal_zone_device *tz) trace_thermal_temperature(tz); dev_dbg(&tz->device, "last_temperature=%lu, current_temperature=%lu\n", tz->last_temperature, tz->temperature); -} - -void thermal_zone_device_update(struct thermal_zone_device *tz) -{ - int count; - - if (!tz->ops->get_temp) - return; - - update_temperature(tz); for (count = 0; count < tz->trips; count++) handle_thermal_trip(tz, count); -- 2.1.4
The .get_trend callback in struct thermal_zone_device_ops has the prototype: int (*get_trend) (struct thermal_zone_device *, int, enum thermal_trend *); whereas the .get_trend callback in struct thermal_zone_of_device_ops has: int (*get_trend)(void *, long *); Streamline both prototypes and add the trip argument to the OF callback aswell and use enum thermal_trend * instead of an integer pointer. While the OF prototype may be the better one, this should be decided at framework level and not on OF level. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/of-thermal.c | 11 +--------- drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 25 +++++++--------------- include/linux/thermal.h | 2 +- 3 files changed, 10 insertions(+), 28 deletions(-) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index 668fb1b..b39e22f 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -187,24 +187,15 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, enum thermal_trend *trend) { struct __thermal_zone *data = tz->devdata; - long dev_trend; int r; if (!data->ops->get_trend) return -EINVAL; - r = data->ops->get_trend(data->sensor_data, &dev_trend); + r = data->ops->get_trend(data->sensor_data, trip, trend); if (r) return r; - /* TODO: These intervals might have some thresholds, but in core code */ - if (dev_trend > 0) - *trend = THERMAL_TREND_RAISING; - else if (dev_trend < 0) - *trend = THERMAL_TREND_DROPPING; - else - *trend = THERMAL_TREND_STABLE; - return 0; } diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c index a38c175..7f8e5f3 100644 --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c @@ -238,7 +238,7 @@ static int ti_thermal_get_trip_temp(struct thermal_zone_device *thermal, return 0; } -static int __ti_thermal_get_trend(void *p, long *trend) +static int __ti_thermal_get_trend(void *p, int trip, enum thermal_trend *trend) { struct ti_thermal_data *data = p; struct ti_bandgap *bgp; @@ -251,22 +251,6 @@ static int __ti_thermal_get_trend(void *p, long *trend) if (ret) return ret; - *trend = tr; - - return 0; -} - -/* Get the temperature trend callback functions for thermal zone */ -static int ti_thermal_get_trend(struct thermal_zone_device *thermal, - int trip, enum thermal_trend *trend) -{ - int ret; - long tr; - - ret = __ti_thermal_get_trend(thermal->devdata, &tr); - if (ret) - return ret; - if (tr > 0) *trend = THERMAL_TREND_RAISING; else if (tr < 0) @@ -277,6 +261,13 @@ static int ti_thermal_get_trend(struct thermal_zone_device *thermal, return 0; } +/* Get the temperature trend callback functions for thermal zone */ +static int ti_thermal_get_trend(struct thermal_zone_device *thermal, + int trip, enum thermal_trend *trend) +{ + return __ti_thermal_get_trend(thermal->devdata, trip, trend); +} + /* Get critical temperature callback functions for thermal zone */ static int ti_thermal_get_crit_temp(struct thermal_zone_device *thermal, unsigned long *temp) diff --git a/include/linux/thermal.h b/include/linux/thermal.h index db6c12b..ba2e29a 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -273,7 +273,7 @@ struct thermal_genl_event { */ struct thermal_zone_of_device_ops { int (*get_temp)(void *, long *); - int (*get_trend)(void *, long *); + int (*get_trend)(void *, int trend, enum thermal_trend *); int (*set_emul_temp)(void *, unsigned long); }; -- 2.1.4
The .get_trend callback in struct thermal_zone_device_ops has the prototype: int (*get_trend) (struct thermal_zone_device *, int, enum thermal_trend *); whereas the .get_trend callback in struct thermal_zone_of_device_ops has: int (*get_trend)(void *, long *); Streamline both prototypes and add the trip argument to the OF callback aswell and use enum thermal_trend * instead of an integer pointer. While the OF prototype may be the better one, this should be decided at framework level and not on OF level. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/of-thermal.c | 11 +--------- drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 25 +++++++--------------- include/linux/thermal.h | 2 +- 3 files changed, 10 insertions(+), 28 deletions(-) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index 668fb1b..b39e22f 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -187,24 +187,15 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, enum thermal_trend *trend) { struct __thermal_zone *data = tz->devdata; - long dev_trend; int r; if (!data->ops->get_trend) return -EINVAL; - r = data->ops->get_trend(data->sensor_data, &dev_trend); + r = data->ops->get_trend(data->sensor_data, trip, trend); if (r) return r; - /* TODO: These intervals might have some thresholds, but in core code */ - if (dev_trend > 0) - *trend = THERMAL_TREND_RAISING; - else if (dev_trend < 0) - *trend = THERMAL_TREND_DROPPING; - else - *trend = THERMAL_TREND_STABLE; - return 0; } diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c index a38c175..7f8e5f3 100644 --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c @@ -238,7 +238,7 @@ static int ti_thermal_get_trip_temp(struct thermal_zone_device *thermal, return 0; } -static int __ti_thermal_get_trend(void *p, long *trend) +static int __ti_thermal_get_trend(void *p, int trip, enum thermal_trend *trend) { struct ti_thermal_data *data = p; struct ti_bandgap *bgp; @@ -251,22 +251,6 @@ static int __ti_thermal_get_trend(void *p, long *trend) if (ret) return ret; - *trend = tr; - - return 0; -} - -/* Get the temperature trend callback functions for thermal zone */ -static int ti_thermal_get_trend(struct thermal_zone_device *thermal, - int trip, enum thermal_trend *trend) -{ - int ret; - long tr; - - ret = __ti_thermal_get_trend(thermal->devdata, &tr); - if (ret) - return ret; - if (tr > 0) *trend = THERMAL_TREND_RAISING; else if (tr < 0) @@ -277,6 +261,13 @@ static int ti_thermal_get_trend(struct thermal_zone_device *thermal, return 0; } +/* Get the temperature trend callback functions for thermal zone */ +static int ti_thermal_get_trend(struct thermal_zone_device *thermal, + int trip, enum thermal_trend *trend) +{ + return __ti_thermal_get_trend(thermal->devdata, trip, trend); +} + /* Get critical temperature callback functions for thermal zone */ static int ti_thermal_get_crit_temp(struct thermal_zone_device *thermal, unsigned long *temp) diff --git a/include/linux/thermal.h b/include/linux/thermal.h index db6c12b..ba2e29a 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -273,7 +273,7 @@ struct thermal_genl_event { */ struct thermal_zone_of_device_ops { int (*get_temp)(void *, long *); - int (*get_trend)(void *, long *); + int (*get_trend)(void *, int trend, enum thermal_trend *); int (*set_emul_temp)(void *, unsigned long); }; -- 2.1.4
The .get_trend callback in struct thermal_zone_device_ops has the prototype: int (*get_trend) (struct thermal_zone_device *, int, enum thermal_trend *); whereas the .get_trend callback in struct thermal_zone_of_device_ops has: int (*get_trend)(void *, long *); Streamline both prototypes and add the trip argument to the OF callback aswell and use enum thermal_trend * instead of an integer pointer. While the OF prototype may be the better one, this should be decided at framework level and not on OF level. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/of-thermal.c | 11 +--------- drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 25 +++++++--------------- include/linux/thermal.h | 2 +- 3 files changed, 10 insertions(+), 28 deletions(-) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index 668fb1b..b39e22f 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -187,24 +187,15 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, enum thermal_trend *trend) { struct __thermal_zone *data = tz->devdata; - long dev_trend; int r; if (!data->ops->get_trend) return -EINVAL; - r = data->ops->get_trend(data->sensor_data, &dev_trend); + r = data->ops->get_trend(data->sensor_data, trip, trend); if (r) return r; - /* TODO: These intervals might have some thresholds, but in core code */ - if (dev_trend > 0) - *trend = THERMAL_TREND_RAISING; - else if (dev_trend < 0) - *trend = THERMAL_TREND_DROPPING; - else - *trend = THERMAL_TREND_STABLE; - return 0; } diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c index a38c175..7f8e5f3 100644 --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c @@ -238,7 +238,7 @@ static int ti_thermal_get_trip_temp(struct thermal_zone_device *thermal, return 0; } -static int __ti_thermal_get_trend(void *p, long *trend) +static int __ti_thermal_get_trend(void *p, int trip, enum thermal_trend *trend) { struct ti_thermal_data *data = p; struct ti_bandgap *bgp; @@ -251,22 +251,6 @@ static int __ti_thermal_get_trend(void *p, long *trend) if (ret) return ret; - *trend = tr; - - return 0; -} - -/* Get the temperature trend callback functions for thermal zone */ -static int ti_thermal_get_trend(struct thermal_zone_device *thermal, - int trip, enum thermal_trend *trend) -{ - int ret; - long tr; - - ret = __ti_thermal_get_trend(thermal->devdata, &tr); - if (ret) - return ret; - if (tr > 0) *trend = THERMAL_TREND_RAISING; else if (tr < 0) @@ -277,6 +261,13 @@ static int ti_thermal_get_trend(struct thermal_zone_device *thermal, return 0; } +/* Get the temperature trend callback functions for thermal zone */ +static int ti_thermal_get_trend(struct thermal_zone_device *thermal, + int trip, enum thermal_trend *trend) +{ + return __ti_thermal_get_trend(thermal->devdata, trip, trend); +} + /* Get critical temperature callback functions for thermal zone */ static int ti_thermal_get_crit_temp(struct thermal_zone_device *thermal, unsigned long *temp) diff --git a/include/linux/thermal.h b/include/linux/thermal.h index db6c12b..ba2e29a 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -273,7 +273,7 @@ struct thermal_genl_event { */ struct thermal_zone_of_device_ops { int (*get_temp)(void *, long *); - int (*get_trend)(void *, long *); + int (*get_trend)(void *, int trend, enum thermal_trend *); int (*set_emul_temp)(void *, unsigned long); }; -- 2.1.4
In the thermal framework it was decided that temperatures can't be negative, so let the .get_temp callback in struct thermal_zone_of_device_ops take an unsigned long pointer for the temperature like the .get_temp callback in struct thermal_zone_device_ops does. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/hwmon/lm75.c | 2 +- drivers/hwmon/ntc_thermistor.c | 2 +- drivers/hwmon/tmp102.c | 2 +- drivers/input/touchscreen/sun4i-ts.c | 2 +- drivers/thermal/rockchip_thermal.c | 2 +- drivers/thermal/samsung/exynos_tmu.c | 2 +- drivers/thermal/tegra_soctherm.c | 2 +- drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 2 +- include/linux/thermal.h | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c index fe41d5a..9df3ca3 100644 --- a/drivers/hwmon/lm75.c +++ b/drivers/hwmon/lm75.c @@ -104,7 +104,7 @@ static inline long lm75_reg_to_mc(s16 temp, u8 resolution) /* sysfs attributes for hwmon */ -static int lm75_read_temp(void *dev, long *temp) +static int lm75_read_temp(void *dev, unsigned long *temp) { struct lm75_data *data = lm75_update_device(dev); diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c index 112e4d4..12cb333 100644 --- a/drivers/hwmon/ntc_thermistor.c +++ b/drivers/hwmon/ntc_thermistor.c @@ -430,7 +430,7 @@ static int ntc_thermistor_get_ohm(struct ntc_data *data) return -EINVAL; } -static int ntc_read_temp(void *dev, long *temp) +static int ntc_read_temp(void *dev, unsigned long *temp) { struct ntc_data *data = dev_get_drvdata(dev); int ohm; diff --git a/drivers/hwmon/tmp102.c b/drivers/hwmon/tmp102.c index 9da2735..25bd72b 100644 --- a/drivers/hwmon/tmp102.c +++ b/drivers/hwmon/tmp102.c @@ -98,7 +98,7 @@ static struct tmp102 *tmp102_update_device(struct device *dev) return tmp102; } -static int tmp102_read_temp(void *dev, long *temp) +static int tmp102_read_temp(void *dev, unsigned long *temp) { struct tmp102 *tmp102 = tmp102_update_device(dev); diff --git a/drivers/input/touchscreen/sun4i-ts.c b/drivers/input/touchscreen/sun4i-ts.c index b93a28b..26a7cf5 100644 --- a/drivers/input/touchscreen/sun4i-ts.c +++ b/drivers/input/touchscreen/sun4i-ts.c @@ -198,7 +198,7 @@ static int sun4i_get_temp(const struct sun4i_ts_data *ts, long *temp) return 0; } -static int sun4i_get_tz_temp(void *data, long *temp) +static int sun4i_get_tz_temp(void *data, unsigned long *temp) { return sun4i_get_temp(data, temp); } diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c index 3aa46ac..67dfc67 100644 --- a/drivers/thermal/rockchip_thermal.c +++ b/drivers/thermal/rockchip_thermal.c @@ -366,7 +366,7 @@ static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev) return IRQ_HANDLED; } -static int rockchip_thermal_get_temp(void *_sensor, long *out_temp) +static int rockchip_thermal_get_temp(void *_sensor, unsigned long *out_temp) { struct rockchip_thermal_sensor *sensor = _sensor; struct rockchip_thermal_data *thermal = sensor->thermal; diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index 1d30b09..5f721f2 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -713,7 +713,7 @@ static void exynos7_tmu_control(struct platform_device *pdev, bool on) writel(con, data->base + EXYNOS_TMU_REG_CONTROL); } -static int exynos_get_temp(void *p, long *temp) +static int exynos_get_temp(void *p, unsigned long *temp) { struct exynos_tmu_data *data = p; diff --git a/drivers/thermal/tegra_soctherm.c b/drivers/thermal/tegra_soctherm.c index 9197fc0..1c4e455 100644 --- a/drivers/thermal/tegra_soctherm.c +++ b/drivers/thermal/tegra_soctherm.c @@ -306,7 +306,7 @@ static long translate_temp(u16 val) return t; } -static int tegra_thermctl_get_temp(void *data, long *out_temp) +static int tegra_thermctl_get_temp(void *data, unsigned long *out_temp) { struct tegra_thermctl_zone *zone = data; u32 val; diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c index 7f8e5f3..f480a01 100644 --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c @@ -76,7 +76,7 @@ static inline int ti_thermal_hotspot_temperature(int t, int s, int c) /* thermal zone ops */ /* Get temperature callback function for thermal zone*/ -static inline int __ti_thermal_get_temp(void *devdata, long *temp) +static inline int __ti_thermal_get_temp(void *devdata, unsigned long *temp) { struct thermal_zone_device *pcb_tz = NULL; struct ti_thermal_data *data = devdata; diff --git a/include/linux/thermal.h b/include/linux/thermal.h index ba2e29a..2f77091 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -272,7 +272,7 @@ struct thermal_genl_event { * temperature. */ struct thermal_zone_of_device_ops { - int (*get_temp)(void *, long *); + int (*get_temp)(void *, unsigned long *); int (*get_trend)(void *, int trend, enum thermal_trend *); int (*set_emul_temp)(void *, unsigned long); }; -- 2.1.4
In the thermal framework it was decided that temperatures can't be negative, so let the .get_temp callback in struct thermal_zone_of_device_ops take an unsigned long pointer for the temperature like the .get_temp callback in struct thermal_zone_device_ops does. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/hwmon/lm75.c | 2 +- drivers/hwmon/ntc_thermistor.c | 2 +- drivers/hwmon/tmp102.c | 2 +- drivers/input/touchscreen/sun4i-ts.c | 2 +- drivers/thermal/rockchip_thermal.c | 2 +- drivers/thermal/samsung/exynos_tmu.c | 2 +- drivers/thermal/tegra_soctherm.c | 2 +- drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 2 +- include/linux/thermal.h | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c index fe41d5a..9df3ca3 100644 --- a/drivers/hwmon/lm75.c +++ b/drivers/hwmon/lm75.c @@ -104,7 +104,7 @@ static inline long lm75_reg_to_mc(s16 temp, u8 resolution) /* sysfs attributes for hwmon */ -static int lm75_read_temp(void *dev, long *temp) +static int lm75_read_temp(void *dev, unsigned long *temp) { struct lm75_data *data = lm75_update_device(dev); diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c index 112e4d4..12cb333 100644 --- a/drivers/hwmon/ntc_thermistor.c +++ b/drivers/hwmon/ntc_thermistor.c @@ -430,7 +430,7 @@ static int ntc_thermistor_get_ohm(struct ntc_data *data) return -EINVAL; } -static int ntc_read_temp(void *dev, long *temp) +static int ntc_read_temp(void *dev, unsigned long *temp) { struct ntc_data *data = dev_get_drvdata(dev); int ohm; diff --git a/drivers/hwmon/tmp102.c b/drivers/hwmon/tmp102.c index 9da2735..25bd72b 100644 --- a/drivers/hwmon/tmp102.c +++ b/drivers/hwmon/tmp102.c @@ -98,7 +98,7 @@ static struct tmp102 *tmp102_update_device(struct device *dev) return tmp102; } -static int tmp102_read_temp(void *dev, long *temp) +static int tmp102_read_temp(void *dev, unsigned long *temp) { struct tmp102 *tmp102 = tmp102_update_device(dev); diff --git a/drivers/input/touchscreen/sun4i-ts.c b/drivers/input/touchscreen/sun4i-ts.c index b93a28b..26a7cf5 100644 --- a/drivers/input/touchscreen/sun4i-ts.c +++ b/drivers/input/touchscreen/sun4i-ts.c @@ -198,7 +198,7 @@ static int sun4i_get_temp(const struct sun4i_ts_data *ts, long *temp) return 0; } -static int sun4i_get_tz_temp(void *data, long *temp) +static int sun4i_get_tz_temp(void *data, unsigned long *temp) { return sun4i_get_temp(data, temp); } diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c index 3aa46ac..67dfc67 100644 --- a/drivers/thermal/rockchip_thermal.c +++ b/drivers/thermal/rockchip_thermal.c @@ -366,7 +366,7 @@ static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev) return IRQ_HANDLED; } -static int rockchip_thermal_get_temp(void *_sensor, long *out_temp) +static int rockchip_thermal_get_temp(void *_sensor, unsigned long *out_temp) { struct rockchip_thermal_sensor *sensor = _sensor; struct rockchip_thermal_data *thermal = sensor->thermal; diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index 1d30b09..5f721f2 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -713,7 +713,7 @@ static void exynos7_tmu_control(struct platform_device *pdev, bool on) writel(con, data->base + EXYNOS_TMU_REG_CONTROL); } -static int exynos_get_temp(void *p, long *temp) +static int exynos_get_temp(void *p, unsigned long *temp) { struct exynos_tmu_data *data = p; diff --git a/drivers/thermal/tegra_soctherm.c b/drivers/thermal/tegra_soctherm.c index 9197fc0..1c4e455 100644 --- a/drivers/thermal/tegra_soctherm.c +++ b/drivers/thermal/tegra_soctherm.c @@ -306,7 +306,7 @@ static long translate_temp(u16 val) return t; } -static int tegra_thermctl_get_temp(void *data, long *out_temp) +static int tegra_thermctl_get_temp(void *data, unsigned long *out_temp) { struct tegra_thermctl_zone *zone = data; u32 val; diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c index 7f8e5f3..f480a01 100644 --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c @@ -76,7 +76,7 @@ static inline int ti_thermal_hotspot_temperature(int t, int s, int c) /* thermal zone ops */ /* Get temperature callback function for thermal zone*/ -static inline int __ti_thermal_get_temp(void *devdata, long *temp) +static inline int __ti_thermal_get_temp(void *devdata, unsigned long *temp) { struct thermal_zone_device *pcb_tz = NULL; struct ti_thermal_data *data = devdata; diff --git a/include/linux/thermal.h b/include/linux/thermal.h index ba2e29a..2f77091 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -272,7 +272,7 @@ struct thermal_genl_event { * temperature. */ struct thermal_zone_of_device_ops { - int (*get_temp)(void *, long *); + int (*get_temp)(void *, unsigned long *); int (*get_trend)(void *, int trend, enum thermal_trend *); int (*set_emul_temp)(void *, unsigned long); }; -- 2.1.4
The thermal core uses the existence of the .get_temp, .get_trend and .set_emul_temp to detect whether this operation exists and should be used or whether it should be emulated in software. This makes problems for of-thermal which has to modify the struct thermal_zone_device_ops during runtime whenever a sensor is registered or unregistered. Let the core test for -ENOSYS from these callbacks and treat it like if the callbacks were not present. This allows of-thermal to always set the sensor related callbacks and to make struct thermal_zone_device_ops const again. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/thermal_core.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 6d0fdad..acf00b5 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -413,10 +413,13 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip) */ int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) { - int ret = -EINVAL; + int ret; - if (!tz || IS_ERR(tz) || !tz->ops->get_temp) - goto exit; + if (!tz || IS_ERR(tz)) + return -EINVAL; + + if (!tz->ops->get_temp) + return -ENOSYS; mutex_lock(&tz->lock); @@ -426,7 +429,7 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) *temp = tz->emul_temperature; mutex_unlock(&tz->lock); -exit: + return ret; } EXPORT_SYMBOL_GPL(thermal_zone_get_temp); @@ -437,10 +440,11 @@ void thermal_zone_device_update(struct thermal_zone_device *tz) unsigned long temp; int ret; - if (!tz->ops->get_temp) + ret = thermal_zone_get_temp(tz, &temp); + + if (ret == -ENOSYS) return; - ret = thermal_zone_get_temp(tz, &temp); if (ret) { dev_warn(&tz->device, "failed to read out thermal zone %d\n", tz->id); @@ -788,12 +792,16 @@ emul_temp_store(struct device *dev, struct device_attribute *attr, } } - if (!tz->ops->set_emul_temp) { + if (tz->ops->set_emul_temp) + ret = tz->ops->set_emul_temp(tz, temperature); + else + ret = -ENOSYS; + + if (ret == -ENOSYS) { mutex_lock(&tz->lock); tz->emul_temperature = temperature; mutex_unlock(&tz->lock); - } else { - ret = tz->ops->set_emul_temp(tz, temperature); + ret = 0; } if (!ret) -- 2.1.4
The thermal core uses the existence of the .get_temp, .get_trend and .set_emul_temp to detect whether this operation exists and should be used or whether it should be emulated in software. This makes problems for of-thermal which has to modify the struct thermal_zone_device_ops during runtime whenever a sensor is registered or unregistered. Let the core test for -ENOSYS from these callbacks and treat it like if the callbacks were not present. This allows of-thermal to always set the sensor related callbacks and to make struct thermal_zone_device_ops const again. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/thermal_core.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 6d0fdad..acf00b5 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -413,10 +413,13 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip) */ int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) { - int ret = -EINVAL; + int ret; - if (!tz || IS_ERR(tz) || !tz->ops->get_temp) - goto exit; + if (!tz || IS_ERR(tz)) + return -EINVAL; + + if (!tz->ops->get_temp) + return -ENOSYS; mutex_lock(&tz->lock); @@ -426,7 +429,7 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) *temp = tz->emul_temperature; mutex_unlock(&tz->lock); -exit: + return ret; } EXPORT_SYMBOL_GPL(thermal_zone_get_temp); @@ -437,10 +440,11 @@ void thermal_zone_device_update(struct thermal_zone_device *tz) unsigned long temp; int ret; - if (!tz->ops->get_temp) + ret = thermal_zone_get_temp(tz, &temp); + + if (ret == -ENOSYS) return; - ret = thermal_zone_get_temp(tz, &temp); if (ret) { dev_warn(&tz->device, "failed to read out thermal zone %d\n", tz->id); @@ -788,12 +792,16 @@ emul_temp_store(struct device *dev, struct device_attribute *attr, } } - if (!tz->ops->set_emul_temp) { + if (tz->ops->set_emul_temp) + ret = tz->ops->set_emul_temp(tz, temperature); + else + ret = -ENOSYS; + + if (ret == -ENOSYS) { mutex_lock(&tz->lock); tz->emul_temperature = temperature; mutex_unlock(&tz->lock); - } else { - ret = tz->ops->set_emul_temp(tz, temperature); + ret = 0; } if (!ret) -- 2.1.4
Now that the thermal core treats -ENOSYS like the callbacks were not present at all we no longer have to overwrite the ops during runtime but instead can always set them and return -ENOSYS if no sensor is registered. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/of-thermal.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index b39e22f..df14fdd 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -91,7 +91,7 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz, { struct __thermal_zone *data = tz->devdata; - if (!data->ops->get_temp) + if (!data->ops) return -EINVAL; return data->ops->get_temp(data->sensor_data, temp); @@ -178,7 +178,7 @@ static int of_thermal_set_emul_temp(struct thermal_zone_device *tz, struct __thermal_zone *data = tz->devdata; if (!data->ops || !data->ops->set_emul_temp) - return -EINVAL; + return -ENOSYS; return data->ops->set_emul_temp(data->sensor_data, temp); } @@ -189,8 +189,8 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, struct __thermal_zone *data = tz->devdata; int r; - if (!data->ops->get_trend) - return -EINVAL; + if (!data->ops || !data->ops->get_trend) + return -ENOSYS; r = data->ops->get_trend(data->sensor_data, trip, trend); if (r) @@ -366,6 +366,10 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz, } static struct thermal_zone_device_ops of_thermal_ops = { + .get_temp = of_thermal_get_temp, + .get_trend = of_thermal_get_trend, + .set_emul_temp = of_thermal_set_emul_temp, + .get_mode = of_thermal_get_mode, .set_mode = of_thermal_set_mode, @@ -399,13 +403,13 @@ thermal_zone_of_add_sensor(struct device_node *zone, if (!ops) return ERR_PTR(-EINVAL); + if (!ops->get_temp) + return ERR_PTR(-EINVAL); + mutex_lock(&tzd->lock); tz->ops = ops; tz->sensor_data = data; - tzd->ops->get_temp = of_thermal_get_temp; - tzd->ops->get_trend = of_thermal_get_trend; - tzd->ops->set_emul_temp = of_thermal_set_emul_temp; mutex_unlock(&tzd->lock); return tzd; -- 2.1.4
Now that the thermal core treats -ENOSYS like the callbacks were not present at all we no longer have to overwrite the ops during runtime but instead can always set them and return -ENOSYS if no sensor is registered. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/of-thermal.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index b39e22f..df14fdd 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -91,7 +91,7 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz, { struct __thermal_zone *data = tz->devdata; - if (!data->ops->get_temp) + if (!data->ops) return -EINVAL; return data->ops->get_temp(data->sensor_data, temp); @@ -178,7 +178,7 @@ static int of_thermal_set_emul_temp(struct thermal_zone_device *tz, struct __thermal_zone *data = tz->devdata; if (!data->ops || !data->ops->set_emul_temp) - return -EINVAL; + return -ENOSYS; return data->ops->set_emul_temp(data->sensor_data, temp); } @@ -189,8 +189,8 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, struct __thermal_zone *data = tz->devdata; int r; - if (!data->ops->get_trend) - return -EINVAL; + if (!data->ops || !data->ops->get_trend) + return -ENOSYS; r = data->ops->get_trend(data->sensor_data, trip, trend); if (r) @@ -366,6 +366,10 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz, } static struct thermal_zone_device_ops of_thermal_ops = { + .get_temp = of_thermal_get_temp, + .get_trend = of_thermal_get_trend, + .set_emul_temp = of_thermal_set_emul_temp, + .get_mode = of_thermal_get_mode, .set_mode = of_thermal_set_mode, @@ -399,13 +403,13 @@ thermal_zone_of_add_sensor(struct device_node *zone, if (!ops) return ERR_PTR(-EINVAL); + if (!ops->get_temp) + return ERR_PTR(-EINVAL); + mutex_lock(&tzd->lock); tz->ops = ops; tz->sensor_data = data; - tzd->ops->get_temp = of_thermal_get_temp; - tzd->ops->get_trend = of_thermal_get_trend; - tzd->ops->set_emul_temp = of_thermal_set_emul_temp; mutex_unlock(&tzd->lock); return tzd; -- 2.1.4
Now that the thermal core treats -ENOSYS like the callbacks were not present at all we no longer have to overwrite the ops during runtime but instead can always set them and return -ENOSYS if no sensor is registered. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/of-thermal.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index b39e22f..df14fdd 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -91,7 +91,7 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz, { struct __thermal_zone *data = tz->devdata; - if (!data->ops->get_temp) + if (!data->ops) return -EINVAL; return data->ops->get_temp(data->sensor_data, temp); @@ -178,7 +178,7 @@ static int of_thermal_set_emul_temp(struct thermal_zone_device *tz, struct __thermal_zone *data = tz->devdata; if (!data->ops || !data->ops->set_emul_temp) - return -EINVAL; + return -ENOSYS; return data->ops->set_emul_temp(data->sensor_data, temp); } @@ -189,8 +189,8 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, struct __thermal_zone *data = tz->devdata; int r; - if (!data->ops->get_trend) - return -EINVAL; + if (!data->ops || !data->ops->get_trend) + return -ENOSYS; r = data->ops->get_trend(data->sensor_data, trip, trend); if (r) @@ -366,6 +366,10 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz, } static struct thermal_zone_device_ops of_thermal_ops = { + .get_temp = of_thermal_get_temp, + .get_trend = of_thermal_get_trend, + .set_emul_temp = of_thermal_set_emul_temp, + .get_mode = of_thermal_get_mode, .set_mode = of_thermal_set_mode, @@ -399,13 +403,13 @@ thermal_zone_of_add_sensor(struct device_node *zone, if (!ops) return ERR_PTR(-EINVAL); + if (!ops->get_temp) + return ERR_PTR(-EINVAL); + mutex_lock(&tzd->lock); tz->ops = ops; tz->sensor_data = data; - tzd->ops->get_temp = of_thermal_get_temp; - tzd->ops->get_trend = of_thermal_get_trend; - tzd->ops->set_emul_temp = of_thermal_set_emul_temp; mutex_unlock(&tzd->lock); return tzd; -- 2.1.4
Now that the of thermal support no longer changes the thermal_zone_device_ops it can be const again. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/thermal_core.c | 2 +- include/linux/thermal.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index acf00b5..dcdf45e 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -1455,7 +1455,7 @@ static void remove_trip_attrs(struct thermal_zone_device *tz) */ struct thermal_zone_device *thermal_zone_device_register(const char *type, int trips, int mask, void *devdata, - struct thermal_zone_device_ops *ops, + const struct thermal_zone_device_ops *ops, const struct thermal_zone_params *tzp, int passive_delay, int polling_delay) { diff --git a/include/linux/thermal.h b/include/linux/thermal.h index 2f77091..ac2897c 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -185,7 +185,7 @@ struct thermal_zone_device { unsigned long emul_temperature; int passive; unsigned int forced_passive; - struct thermal_zone_device_ops *ops; + const struct thermal_zone_device_ops *ops; const struct thermal_zone_params *tzp; struct thermal_governor *governor; struct list_head thermal_instances; @@ -317,7 +317,7 @@ void thermal_zone_of_sensor_unregister(struct device *dev, #if IS_ENABLED(CONFIG_THERMAL) struct thermal_zone_device *thermal_zone_device_register(const char *, int, int, - void *, struct thermal_zone_device_ops *, + void *, const struct thermal_zone_device_ops *, const struct thermal_zone_params *, int, int); void thermal_zone_device_unregister(struct thermal_zone_device *); @@ -345,7 +345,7 @@ void thermal_notify_framework(struct thermal_zone_device *, int); #else static inline struct thermal_zone_device *thermal_zone_device_register( const char *type, int trips, int mask, void *devdata, - struct thermal_zone_device_ops *ops, + const struct thermal_zone_device_ops *ops, const struct thermal_zone_params *tzp, int passive_delay, int polling_delay) { return ERR_PTR(-ENODEV); } -- 2.1.4
Now that the of thermal support no longer changes the thermal_zone_device_ops it can be const again. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/thermal_core.c | 2 +- include/linux/thermal.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index acf00b5..dcdf45e 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -1455,7 +1455,7 @@ static void remove_trip_attrs(struct thermal_zone_device *tz) */ struct thermal_zone_device *thermal_zone_device_register(const char *type, int trips, int mask, void *devdata, - struct thermal_zone_device_ops *ops, + const struct thermal_zone_device_ops *ops, const struct thermal_zone_params *tzp, int passive_delay, int polling_delay) { diff --git a/include/linux/thermal.h b/include/linux/thermal.h index 2f77091..ac2897c 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -185,7 +185,7 @@ struct thermal_zone_device { unsigned long emul_temperature; int passive; unsigned int forced_passive; - struct thermal_zone_device_ops *ops; + const struct thermal_zone_device_ops *ops; const struct thermal_zone_params *tzp; struct thermal_governor *governor; struct list_head thermal_instances; @@ -317,7 +317,7 @@ void thermal_zone_of_sensor_unregister(struct device *dev, #if IS_ENABLED(CONFIG_THERMAL) struct thermal_zone_device *thermal_zone_device_register(const char *, int, int, - void *, struct thermal_zone_device_ops *, + void *, const struct thermal_zone_device_ops *, const struct thermal_zone_params *, int, int); void thermal_zone_device_unregister(struct thermal_zone_device *); @@ -345,7 +345,7 @@ void thermal_notify_framework(struct thermal_zone_device *, int); #else static inline struct thermal_zone_device *thermal_zone_device_register( const char *type, int trips, int mask, void *devdata, - struct thermal_zone_device_ops *ops, + const struct thermal_zone_device_ops *ops, const struct thermal_zone_params *tzp, int passive_delay, int polling_delay) { return ERR_PTR(-ENODEV); } -- 2.1.4
Now that we no longer modify the ops they can be const again. Also we no longer have to duplicate them. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/of-thermal.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index df14fdd..9b63193 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -365,7 +365,7 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz, return -EINVAL; } -static struct thermal_zone_device_ops of_thermal_ops = { +static const struct thermal_zone_device_ops of_thermal_ops = { .get_temp = of_thermal_get_temp, .get_trend = of_thermal_get_trend, .set_emul_temp = of_thermal_set_emul_temp, @@ -539,10 +539,6 @@ void thermal_zone_of_sensor_unregister(struct device *dev, return; mutex_lock(&tzd->lock); - tzd->ops->get_temp = NULL; - tzd->ops->get_trend = NULL; - tzd->ops->set_emul_temp = NULL; - tz->ops = NULL; tz->sensor_data = NULL; mutex_unlock(&tzd->lock); @@ -849,7 +845,6 @@ int __init of_parse_thermal_zones(void) { struct device_node *np, *child; struct __thermal_zone *tz; - struct thermal_zone_device_ops *ops; np = of_find_node_by_name(NULL, "thermal-zones"); if (!np) { @@ -873,29 +868,22 @@ int __init of_parse_thermal_zones(void) continue; } - ops = kmemdup(&of_thermal_ops, sizeof(*ops), GFP_KERNEL); - if (!ops) - goto exit_free; - tzp = kzalloc(sizeof(*tzp), GFP_KERNEL); - if (!tzp) { - kfree(ops); + if (!tzp) goto exit_free; - } /* No hwmon because there might be hwmon drivers registering */ tzp->no_hwmon = true; zone = thermal_zone_device_register(child->name, tz->ntrips, 0, tz, - ops, tzp, + &of_thermal_ops, tzp, tz->passive_delay, tz->polling_delay); if (IS_ERR(zone)) { pr_err("Failed to build %s zone %ld\n", child->name, PTR_ERR(zone)); kfree(tzp); - kfree(ops); of_thermal_free_zone(tz); /* attempting to build remaining zones still */ } -- 2.1.4
Now that we no longer modify the ops they can be const again. Also we no longer have to duplicate them. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/of-thermal.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index df14fdd..9b63193 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -365,7 +365,7 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz, return -EINVAL; } -static struct thermal_zone_device_ops of_thermal_ops = { +static const struct thermal_zone_device_ops of_thermal_ops = { .get_temp = of_thermal_get_temp, .get_trend = of_thermal_get_trend, .set_emul_temp = of_thermal_set_emul_temp, @@ -539,10 +539,6 @@ void thermal_zone_of_sensor_unregister(struct device *dev, return; mutex_lock(&tzd->lock); - tzd->ops->get_temp = NULL; - tzd->ops->get_trend = NULL; - tzd->ops->set_emul_temp = NULL; - tz->ops = NULL; tz->sensor_data = NULL; mutex_unlock(&tzd->lock); @@ -849,7 +845,6 @@ int __init of_parse_thermal_zones(void) { struct device_node *np, *child; struct __thermal_zone *tz; - struct thermal_zone_device_ops *ops; np = of_find_node_by_name(NULL, "thermal-zones"); if (!np) { @@ -873,29 +868,22 @@ int __init of_parse_thermal_zones(void) continue; } - ops = kmemdup(&of_thermal_ops, sizeof(*ops), GFP_KERNEL); - if (!ops) - goto exit_free; - tzp = kzalloc(sizeof(*tzp), GFP_KERNEL); - if (!tzp) { - kfree(ops); + if (!tzp) goto exit_free; - } /* No hwmon because there might be hwmon drivers registering */ tzp->no_hwmon = true; zone = thermal_zone_device_register(child->name, tz->ntrips, 0, tz, - ops, tzp, + &of_thermal_ops, tzp, tz->passive_delay, tz->polling_delay); if (IS_ERR(zone)) { pr_err("Failed to build %s zone %ld\n", child->name, PTR_ERR(zone)); kfree(tzp); - kfree(ops); of_thermal_free_zone(tz); /* attempting to build remaining zones still */ } -- 2.1.4
This adds support for hardware-tracked trip points to the device tree thermal sensor framework. The framework supports an arbitrary number of trip points. Whenever the current temperature is updated, the trip points immediately below and above the current temperature are found. A .set_trips callback is then called with the temperatures. If there is no trip point above or below the current temperature, the passed trip temperature will be ULONG_MAX or 0 respectively. In this callback, the driver should program the hardware such that it is notified when either of these trip points are triggered. When a trip point is triggered, the driver should call `thermal_zone_device_update' for the respective thermal zone. This will cause the trip points to be updated again. If .set_trips is not implemented, the framework behaves as before. This patch is based on an earlier version from Mikko Perttunen <mikko.perttunen@kapsi.fi> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/thermal_core.c | 41 +++++++++++++++++++++++++++++++++++++++++ include/linux/thermal.h | 3 +++ 2 files changed, 44 insertions(+) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index dcdf45e..7138f8f 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -434,6 +434,45 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) } EXPORT_SYMBOL_GPL(thermal_zone_get_temp); +static void thermal_zone_set_trips(struct thermal_zone_device *tz) +{ + unsigned long low = 0; + unsigned long high = ULONG_MAX; + unsigned long trip_temp, hysteresis; + unsigned long temp = tz->temperature; + int i; + + if (!tz->ops->set_trips) + return; + + /* No need to change trip points */ + if (temp > tz->prev_low_trip && temp < tz->prev_high_trip) + return; + + for (i = 0; i < tz->trips; i++) { + unsigned long trip_low; + + tz->ops->get_trip_temp(tz, i, &trip_temp); + tz->ops->get_trip_hyst(tz, i, &hysteresis); + + trip_low = trip_temp - hysteresis; + + if (trip_low < temp && trip_low > low) + low = trip_low; + + if (trip_temp > temp && trip_temp < high) + high = trip_temp; + } + + tz->prev_low_trip = low; + tz->prev_high_trip = high; + + dev_dbg(&tz->device, "new temperature boundaries: %lu < x < %lu\n", + low, high); + + tz->ops->set_trips(tz, low, high); +} + void thermal_zone_device_update(struct thermal_zone_device *tz) { int count; @@ -460,6 +499,8 @@ void thermal_zone_device_update(struct thermal_zone_device *tz) dev_dbg(&tz->device, "last_temperature=%lu, current_temperature=%lu\n", tz->last_temperature, tz->temperature); + thermal_zone_set_trips(tz); + for (count = 0; count < tz->trips; count++) handle_thermal_trip(tz, count); } diff --git a/include/linux/thermal.h b/include/linux/thermal.h index ac2897c..b870702 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -87,6 +87,7 @@ struct thermal_zone_device_ops { int (*unbind) (struct thermal_zone_device *, struct thermal_cooling_device *); int (*get_temp) (struct thermal_zone_device *, unsigned long *); + int (*set_trips) (struct thermal_zone_device *, unsigned long, unsigned long); int (*get_mode) (struct thermal_zone_device *, enum thermal_device_mode *); int (*set_mode) (struct thermal_zone_device *, @@ -183,6 +184,8 @@ struct thermal_zone_device { unsigned long temperature; unsigned long last_temperature; unsigned long emul_temperature; + unsigned long prev_low_trip; + unsigned long prev_high_trip; int passive; unsigned int forced_passive; const struct thermal_zone_device_ops *ops; -- 2.1.4
This adds support for hardware-tracked trip points to the device tree thermal sensor framework. The framework supports an arbitrary number of trip points. Whenever the current temperature is updated, the trip points immediately below and above the current temperature are found. A .set_trips callback is then called with the temperatures. If there is no trip point above or below the current temperature, the passed trip temperature will be ULONG_MAX or 0 respectively. In this callback, the driver should program the hardware such that it is notified when either of these trip points are triggered. When a trip point is triggered, the driver should call `thermal_zone_device_update' for the respective thermal zone. This will cause the trip points to be updated again. If .set_trips is not implemented, the framework behaves as before. This patch is based on an earlier version from Mikko Perttunen <mikko.perttunen@kapsi.fi> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/thermal_core.c | 41 +++++++++++++++++++++++++++++++++++++++++ include/linux/thermal.h | 3 +++ 2 files changed, 44 insertions(+) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index dcdf45e..7138f8f 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -434,6 +434,45 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) } EXPORT_SYMBOL_GPL(thermal_zone_get_temp); +static void thermal_zone_set_trips(struct thermal_zone_device *tz) +{ + unsigned long low = 0; + unsigned long high = ULONG_MAX; + unsigned long trip_temp, hysteresis; + unsigned long temp = tz->temperature; + int i; + + if (!tz->ops->set_trips) + return; + + /* No need to change trip points */ + if (temp > tz->prev_low_trip && temp < tz->prev_high_trip) + return; + + for (i = 0; i < tz->trips; i++) { + unsigned long trip_low; + + tz->ops->get_trip_temp(tz, i, &trip_temp); + tz->ops->get_trip_hyst(tz, i, &hysteresis); + + trip_low = trip_temp - hysteresis; + + if (trip_low < temp && trip_low > low) + low = trip_low; + + if (trip_temp > temp && trip_temp < high) + high = trip_temp; + } + + tz->prev_low_trip = low; + tz->prev_high_trip = high; + + dev_dbg(&tz->device, "new temperature boundaries: %lu < x < %lu\n", + low, high); + + tz->ops->set_trips(tz, low, high); +} + void thermal_zone_device_update(struct thermal_zone_device *tz) { int count; @@ -460,6 +499,8 @@ void thermal_zone_device_update(struct thermal_zone_device *tz) dev_dbg(&tz->device, "last_temperature=%lu, current_temperature=%lu\n", tz->last_temperature, tz->temperature); + thermal_zone_set_trips(tz); + for (count = 0; count < tz->trips; count++) handle_thermal_trip(tz, count); } diff --git a/include/linux/thermal.h b/include/linux/thermal.h index ac2897c..b870702 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -87,6 +87,7 @@ struct thermal_zone_device_ops { int (*unbind) (struct thermal_zone_device *, struct thermal_cooling_device *); int (*get_temp) (struct thermal_zone_device *, unsigned long *); + int (*set_trips) (struct thermal_zone_device *, unsigned long, unsigned long); int (*get_mode) (struct thermal_zone_device *, enum thermal_device_mode *); int (*set_mode) (struct thermal_zone_device *, @@ -183,6 +184,8 @@ struct thermal_zone_device { unsigned long temperature; unsigned long last_temperature; unsigned long emul_temperature; + unsigned long prev_low_trip; + unsigned long prev_high_trip; int passive; unsigned int forced_passive; const struct thermal_zone_device_ops *ops; -- 2.1.4
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/of-thermal.c | 12 ++++++++++++ include/linux/thermal.h | 1 + 2 files changed, 13 insertions(+) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index 9b63193..a3de5de 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -97,6 +97,17 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz, return data->ops->get_temp(data->sensor_data, temp); } +static int of_thermal_set_trips(struct thermal_zone_device *tz, + unsigned long low, unsigned long high) +{ + struct __thermal_zone *data = tz->devdata; + + if (!data->ops || !data->ops->set_trips) + return -ENOSYS; + + return data->ops->set_trips(data->sensor_data, low, high); +} + /** * of_thermal_get_ntrips - function to export number of available trip * points. @@ -367,6 +378,7 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz, static const struct thermal_zone_device_ops of_thermal_ops = { .get_temp = of_thermal_get_temp, + .set_trips = of_thermal_set_trips, .get_trend = of_thermal_get_trend, .set_emul_temp = of_thermal_set_emul_temp, diff --git a/include/linux/thermal.h b/include/linux/thermal.h index b870702..84a5b5d 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -276,6 +276,7 @@ struct thermal_genl_event { */ struct thermal_zone_of_device_ops { int (*get_temp)(void *, unsigned long *); + int (*set_trips)(void *, unsigned long, unsigned long); int (*get_trend)(void *, int trend, enum thermal_trend *); int (*set_emul_temp)(void *, unsigned long); }; -- 2.1.4
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/thermal/of-thermal.c | 12 ++++++++++++ include/linux/thermal.h | 1 + 2 files changed, 13 insertions(+) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index 9b63193..a3de5de 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -97,6 +97,17 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz, return data->ops->get_temp(data->sensor_data, temp); } +static int of_thermal_set_trips(struct thermal_zone_device *tz, + unsigned long low, unsigned long high) +{ + struct __thermal_zone *data = tz->devdata; + + if (!data->ops || !data->ops->set_trips) + return -ENOSYS; + + return data->ops->set_trips(data->sensor_data, low, high); +} + /** * of_thermal_get_ntrips - function to export number of available trip * points. @@ -367,6 +378,7 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz, static const struct thermal_zone_device_ops of_thermal_ops = { .get_temp = of_thermal_get_temp, + .set_trips = of_thermal_set_trips, .get_trend = of_thermal_get_trend, .set_emul_temp = of_thermal_set_emul_temp, diff --git a/include/linux/thermal.h b/include/linux/thermal.h index b870702..84a5b5d 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -276,6 +276,7 @@ struct thermal_genl_event { */ struct thermal_zone_of_device_ops { int (*get_temp)(void *, unsigned long *); + int (*set_trips)(void *, unsigned long, unsigned long); int (*get_trend)(void *, int trend, enum thermal_trend *); int (*set_emul_temp)(void *, unsigned long); }; -- 2.1.4
On 03/26/2015 11:53 AM, Sascha Hauer wrote:
> - The code should only accept an emulated temperature when the emulated
> temperature is lower than the critical temperature. Instead the code
Why?
Emulating temperatures higher than critical temperature is useful for
testing. For instance it allows one to validate that critical action
(i.e. shutdown) is triggered.
On 03/26/2015 11:53 AM, Sascha Hauer wrote:
> - The code should only accept an emulated temperature when the emulated
> temperature is lower than the critical temperature. Instead the code
Why?
Emulating temperatures higher than critical temperature is useful for
testing. For instance it allows one to validate that critical action
(i.e. shutdown) is triggered.
On 03/26/2015 11:53 AM, Sascha Hauer wrote:
> - The code should only accept an emulated temperature when the emulated
> temperature is lower than the critical temperature. Instead the code
Why?
Emulating temperatures higher than critical temperature is useful for
testing. For instance it allows one to validate that critical action
(i.e. shutdown) is triggered.
+Cc Amit who implemented the emulation code. On Thu, Mar 26, 2015 at 02:13:47PM -0400, Carlos Hernandez wrote: > On 03/26/2015 11:53 AM, Sascha Hauer wrote: > >- The code should only accept an emulated temperature when the emulated > > temperature is lower than the critical temperature. Instead the code > > Why? > Emulating temperatures higher than critical temperature is useful > for testing. For instance it allows one to validate that critical > action (i.e. shutdown) is triggered. Not emulating critical temperatures was the intention of the original patch adding the emulation code, but it was implemented wrongly. I just fix to the intended behaviour. However, I also find emulating critical temperatures useful. I could also remove the check instead of fixing it if we can agree on that behaviour. 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 |
+Cc Amit who implemented the emulation code. On Thu, Mar 26, 2015 at 02:13:47PM -0400, Carlos Hernandez wrote: > On 03/26/2015 11:53 AM, Sascha Hauer wrote: > >- The code should only accept an emulated temperature when the emulated > > temperature is lower than the critical temperature. Instead the code > > Why? > Emulating temperatures higher than critical temperature is useful > for testing. For instance it allows one to validate that critical > action (i.e. shutdown) is triggered. Not emulating critical temperatures was the intention of the original patch adding the emulation code, but it was implemented wrongly. I just fix to the intended behaviour. However, I also find emulating critical temperatures useful. I could also remove the check instead of fixing it if we can agree on that behaviour. 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 |
Hi Sascha, On Thu, Mar 26, 2015 at 9:23 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote: > commit e6e238c38 (thermal: sysfs: Add a new sysfs node emul_temp for > thermal emulation) promised not to emulate critical temperatures, > but the check for critical temperatures is broken in multiple ways: > > - The code should only accept an emulated temperature when the emulated > temperature is lower than the critical temperature. Instead the code > accepts an emulated temperature whenever the real temperature is lower > than the critical temperature. This makes no sense and trying to > emulate a temperature higher than the critical temperature halts the > system. Even higher than critical temperature should be accepted. see my further comments below. > - When trying to emulate a higher-than-critical temperature we should either > limit the emulated temperature to the maximum non critical temperature > or refuse to emulate this temperature. Instead the code just silently > ignores the emulated temperature and continues with the real temperature. > > This patch moves the test for illegal emulated temperature to the sysfs > write function so that we can properly refuse illegal temperatures here. > Trying to write illegal temperatures results in an error message. While > at it use IS_ENABLED() instead of #ifdefs. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/thermal_core.c | 46 ++++++++++++++++++++++-------------------- > 1 file changed, 24 insertions(+), 22 deletions(-) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index dcea909..ebca854 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -414,11 +414,6 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip) > int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) > { > int ret = -EINVAL; > -#ifdef CONFIG_THERMAL_EMULATION > - int count; > - unsigned long crit_temp = -1UL; > - enum thermal_trip_type type; > -#endif > > if (!tz || IS_ERR(tz) || !tz->ops->get_temp) > goto exit; > @@ -426,25 +421,10 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) > mutex_lock(&tz->lock); > > ret = tz->ops->get_temp(tz, temp); > -#ifdef CONFIG_THERMAL_EMULATION > - if (!tz->emul_temperature) > - goto skip_emul; > - > - for (count = 0; count < tz->trips; count++) { > - ret = tz->ops->get_trip_type(tz, count, &type); > - if (!ret && type == THERMAL_TRIP_CRITICAL) { > - ret = tz->ops->get_trip_temp(tz, count, &crit_temp); > - break; > - } > - } > - > - if (ret) > - goto skip_emul; > > - if (*temp < crit_temp) I guess this check is confusing. Actually instead of returning emulating temperature it is returning actual temperature. But the important thing to look here is that actual temperature is higher than critical temperature. So this check prevents the user from suppressing the critical temperature and hence prevents from burning up the chip. > + if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz->emul_temperature) > *temp = tz->emul_temperature; > -skip_emul: > -#endif > + > mutex_unlock(&tz->lock); > exit: > return ret; > @@ -788,10 +768,32 @@ emul_temp_store(struct device *dev, struct device_attribute *attr, > struct thermal_zone_device *tz = to_thermal_zone(dev); > int ret = 0; > unsigned long temperature; > + int trip; > + unsigned long crit_temp; > + enum thermal_trip_type type; > > if (kstrtoul(buf, 10, &temperature)) > return -EINVAL; > > + for (trip = 0; trip < tz->trips; trip++) { > + ret = tz->ops->get_trip_type(tz, trip, &type); > + if (ret) > + return ret; > + > + if (type != THERMAL_TRIP_CRITICAL) > + continue; > + > + ret = tz->ops->get_trip_temp(tz, trip, &crit_temp); > + if (ret) > + return ret; > + > + if (temperature >= crit_temp) { > + dev_err(&tz->device, "Will not emulate critical temperature %luC (tcrit=%luC)\n", > + temperature / 1000, crit_temp / 1000); > + return -EINVAL; > + } Emulating critical temperature is very much needed. > + } > + > if (!tz->ops->set_emul_temp) { > mutex_lock(&tz->lock); > tz->emul_temperature = temperature; > -- > 2.1.4 > > -- > 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
Hi Sascha, On Thu, Mar 26, 2015 at 9:23 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote: > commit e6e238c38 (thermal: sysfs: Add a new sysfs node emul_temp for > thermal emulation) promised not to emulate critical temperatures, > but the check for critical temperatures is broken in multiple ways: > > - The code should only accept an emulated temperature when the emulated > temperature is lower than the critical temperature. Instead the code > accepts an emulated temperature whenever the real temperature is lower > than the critical temperature. This makes no sense and trying to > emulate a temperature higher than the critical temperature halts the > system. Even higher than critical temperature should be accepted. see my further comments below. > - When trying to emulate a higher-than-critical temperature we should either > limit the emulated temperature to the maximum non critical temperature > or refuse to emulate this temperature. Instead the code just silently > ignores the emulated temperature and continues with the real temperature. > > This patch moves the test for illegal emulated temperature to the sysfs > write function so that we can properly refuse illegal temperatures here. > Trying to write illegal temperatures results in an error message. While > at it use IS_ENABLED() instead of #ifdefs. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/thermal_core.c | 46 ++++++++++++++++++++++-------------------- > 1 file changed, 24 insertions(+), 22 deletions(-) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index dcea909..ebca854 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -414,11 +414,6 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip) > int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) > { > int ret = -EINVAL; > -#ifdef CONFIG_THERMAL_EMULATION > - int count; > - unsigned long crit_temp = -1UL; > - enum thermal_trip_type type; > -#endif > > if (!tz || IS_ERR(tz) || !tz->ops->get_temp) > goto exit; > @@ -426,25 +421,10 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) > mutex_lock(&tz->lock); > > ret = tz->ops->get_temp(tz, temp); > -#ifdef CONFIG_THERMAL_EMULATION > - if (!tz->emul_temperature) > - goto skip_emul; > - > - for (count = 0; count < tz->trips; count++) { > - ret = tz->ops->get_trip_type(tz, count, &type); > - if (!ret && type == THERMAL_TRIP_CRITICAL) { > - ret = tz->ops->get_trip_temp(tz, count, &crit_temp); > - break; > - } > - } > - > - if (ret) > - goto skip_emul; > > - if (*temp < crit_temp) I guess this check is confusing. Actually instead of returning emulating temperature it is returning actual temperature. But the important thing to look here is that actual temperature is higher than critical temperature. So this check prevents the user from suppressing the critical temperature and hence prevents from burning up the chip. > + if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz->emul_temperature) > *temp = tz->emul_temperature; > -skip_emul: > -#endif > + > mutex_unlock(&tz->lock); > exit: > return ret; > @@ -788,10 +768,32 @@ emul_temp_store(struct device *dev, struct device_attribute *attr, > struct thermal_zone_device *tz = to_thermal_zone(dev); > int ret = 0; > unsigned long temperature; > + int trip; > + unsigned long crit_temp; > + enum thermal_trip_type type; > > if (kstrtoul(buf, 10, &temperature)) > return -EINVAL; > > + for (trip = 0; trip < tz->trips; trip++) { > + ret = tz->ops->get_trip_type(tz, trip, &type); > + if (ret) > + return ret; > + > + if (type != THERMAL_TRIP_CRITICAL) > + continue; > + > + ret = tz->ops->get_trip_temp(tz, trip, &crit_temp); > + if (ret) > + return ret; > + > + if (temperature >= crit_temp) { > + dev_err(&tz->device, "Will not emulate critical temperature %luC (tcrit=%luC)\n", > + temperature / 1000, crit_temp / 1000); > + return -EINVAL; > + } Emulating critical temperature is very much needed. > + } > + > if (!tz->ops->set_emul_temp) { > mutex_lock(&tz->lock); > tz->emul_temperature = temperature; > -- > 2.1.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Amit, On Fri, Mar 27, 2015 at 08:35:50AM +0530, amit daniel kachhap wrote: > Hi Sascha, > > > -#ifdef CONFIG_THERMAL_EMULATION > > - if (!tz->emul_temperature) > > - goto skip_emul; > > - > > - for (count = 0; count < tz->trips; count++) { > > - ret = tz->ops->get_trip_type(tz, count, &type); > > - if (!ret && type == THERMAL_TRIP_CRITICAL) { > > - ret = tz->ops->get_trip_temp(tz, count, &crit_temp); > > - break; > > - } > > - } > > - > > - if (ret) > > - goto skip_emul; > > > > - if (*temp < crit_temp) > I guess this check is confusing. Actually instead of returning > emulating temperature it is returning actual temperature. But the > important thing to look here is that actual temperature is higher than > critical temperature. So this check prevents the user from suppressing > the critical temperature and hence prevents from burning up the chip. Indeed the check is confusing, but now it makes perfectly sense. I'll look at the patch again and maybe turn into a patch just adding a comment to clarify 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-5555 |
Hi Amit, On Fri, Mar 27, 2015 at 08:35:50AM +0530, amit daniel kachhap wrote: > Hi Sascha, > > > -#ifdef CONFIG_THERMAL_EMULATION > > - if (!tz->emul_temperature) > > - goto skip_emul; > > - > > - for (count = 0; count < tz->trips; count++) { > > - ret = tz->ops->get_trip_type(tz, count, &type); > > - if (!ret && type == THERMAL_TRIP_CRITICAL) { > > - ret = tz->ops->get_trip_temp(tz, count, &crit_temp); > > - break; > > - } > > - } > > - > > - if (ret) > > - goto skip_emul; > > > > - if (*temp < crit_temp) > I guess this check is confusing. Actually instead of returning > emulating temperature it is returning actual temperature. But the > important thing to look here is that actual temperature is higher than > critical temperature. So this check prevents the user from suppressing > the critical temperature and hence prevents from burning up the chip. Indeed the check is confusing, but now it makes perfectly sense. I'll look at the patch again and maybe turn into a patch just adding a comment to clarify 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-5555 |
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(-) > [...]
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(-) > [...]
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 -- 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 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 -- 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 |
[-- Attachment #1: Type: text/plain, Size: 6648 bytes --] On Thu, Mar 26, 2015 at 04:53:48PM +0100, 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. I believe it make sense to change all to be int. int covers for the required temperature range. Rui is just introducing the concept of invalid temp, which is below 0 K. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/thermal_core.c | 10 +++++----- > include/linux/thermal.h | 6 +++--- This change is not that straight forward as it looks like. In order to standardize this, apart from the thermal core we will need to make drivers aware of the change too. That will require changing the thermal zone device ops and all its users, for the trip temperature, temperature, and hysteresis cases. > 2 files changed, 8 insertions(+), 8 deletions(-) > > 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; Something like the following would be required to do a standardization. Of course, the code below requires changing drivers too: diff --git a/include/linux/thermal.h b/include/linux/thermal.h index 6bbe11c..84c9777 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -92,7 +92,7 @@ struct thermal_zone_device_ops { struct thermal_cooling_device *); int (*unbind) (struct thermal_zone_device *, struct thermal_cooling_device *); - int (*get_temp) (struct thermal_zone_device *, unsigned long *); + int (*get_temp) (struct thermal_zone_device *, int *); int (*get_mode) (struct thermal_zone_device *, enum thermal_device_mode *); int (*set_mode) (struct thermal_zone_device *, @@ -100,15 +100,15 @@ struct thermal_zone_device_ops { int (*get_trip_type) (struct thermal_zone_device *, int, enum thermal_trip_type *); int (*get_trip_temp) (struct thermal_zone_device *, int, - unsigned long *); + int *); int (*set_trip_temp) (struct thermal_zone_device *, int, - unsigned long); + int); int (*get_trip_hyst) (struct thermal_zone_device *, int, - unsigned long *); + int *); 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); + int (*get_crit_temp) (struct thermal_zone_device *, int *); + int (*set_emul_temp) (struct thermal_zone_device *, int); int (*get_trend) (struct thermal_zone_device *, int, enum thermal_trend *); int (*notify) (struct thermal_zone_device *, int, @@ -321,9 +321,9 @@ struct thermal_genl_event { * temperature. */ struct thermal_zone_of_device_ops { - int (*get_temp)(void *, long *); - int (*get_trend)(void *, long *); - int (*set_emul_temp)(void *, unsigned long); + int (*get_temp)(void *, int *); + int (*get_trend)(void *, int *); + int (*set_emul_temp)(void *, int); }; /** @@ -336,8 +336,8 @@ struct thermal_zone_of_device_ops { struct thermal_trip { struct device_node *np; - unsigned long int temperature; - unsigned long int hysteresis; + int temperature; + int hysteresis; enum thermal_trip_type type; }; @@ -395,7 +395,7 @@ thermal_of_cooling_device_register(struct device_node *np, char *, void *, const struct thermal_cooling_device_ops *); void thermal_cooling_device_unregister(struct thermal_cooling_device *); struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name); -int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp); +int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp); int get_tz_trend(struct thermal_zone_device *, int); struct thermal_instance *get_thermal_instance(struct thermal_zone_device *, @@ -446,7 +446,7 @@ static inline struct thermal_zone_device *thermal_zone_get_zone_by_name( const char *name) { return ERR_PTR(-ENODEV); } static inline int thermal_zone_get_temp( - struct thermal_zone_device *tz, unsigned long *temp) + struct thermal_zone_device *tz, int *temp) { return -ENODEV; } static inline int get_tz_trend(struct thermal_zone_device *tz, int trip) { return -ENODEV; } > -- > 2.1.4 > [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --]
On Thu, Mar 26, 2015 at 04:53:48PM +0100, 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. I believe it make sense to change all to be int. int covers for the required temperature range. Rui is just introducing the concept of invalid temp, which is below 0 K. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/thermal_core.c | 10 +++++----- > include/linux/thermal.h | 6 +++--- This change is not that straight forward as it looks like. In order to standardize this, apart from the thermal core we will need to make drivers aware of the change too. That will require changing the thermal zone device ops and all its users, for the trip temperature, temperature, and hysteresis cases. > 2 files changed, 8 insertions(+), 8 deletions(-) > > 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; Something like the following would be required to do a standardization. Of course, the code below requires changing drivers too: diff --git a/include/linux/thermal.h b/include/linux/thermal.h index 6bbe11c..84c9777 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -92,7 +92,7 @@ struct thermal_zone_device_ops { struct thermal_cooling_device *); int (*unbind) (struct thermal_zone_device *, struct thermal_cooling_device *); - int (*get_temp) (struct thermal_zone_device *, unsigned long *); + int (*get_temp) (struct thermal_zone_device *, int *); int (*get_mode) (struct thermal_zone_device *, enum thermal_device_mode *); int (*set_mode) (struct thermal_zone_device *, @@ -100,15 +100,15 @@ struct thermal_zone_device_ops { int (*get_trip_type) (struct thermal_zone_device *, int, enum thermal_trip_type *); int (*get_trip_temp) (struct thermal_zone_device *, int, - unsigned long *); + int *); int (*set_trip_temp) (struct thermal_zone_device *, int, - unsigned long); + int); int (*get_trip_hyst) (struct thermal_zone_device *, int, - unsigned long *); + int *); 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); + int (*get_crit_temp) (struct thermal_zone_device *, int *); + int (*set_emul_temp) (struct thermal_zone_device *, int); int (*get_trend) (struct thermal_zone_device *, int, enum thermal_trend *); int (*notify) (struct thermal_zone_device *, int, @@ -321,9 +321,9 @@ struct thermal_genl_event { * temperature. */ struct thermal_zone_of_device_ops { - int (*get_temp)(void *, long *); - int (*get_trend)(void *, long *); - int (*set_emul_temp)(void *, unsigned long); + int (*get_temp)(void *, int *); + int (*get_trend)(void *, int *); + int (*set_emul_temp)(void *, int); }; /** @@ -336,8 +336,8 @@ struct thermal_zone_of_device_ops { struct thermal_trip { struct device_node *np; - unsigned long int temperature; - unsigned long int hysteresis; + int temperature; + int hysteresis; enum thermal_trip_type type; }; @@ -395,7 +395,7 @@ thermal_of_cooling_device_register(struct device_node *np, char *, void *, const struct thermal_cooling_device_ops *); void thermal_cooling_device_unregister(struct thermal_cooling_device *); struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name); -int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp); +int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp); int get_tz_trend(struct thermal_zone_device *, int); struct thermal_instance *get_thermal_instance(struct thermal_zone_device *, @@ -446,7 +446,7 @@ static inline struct thermal_zone_device *thermal_zone_get_zone_by_name( const char *name) { return ERR_PTR(-ENODEV); } static inline int thermal_zone_get_temp( - struct thermal_zone_device *tz, unsigned long *temp) + struct thermal_zone_device *tz, int *temp) { return -ENODEV; } static inline int get_tz_trend(struct thermal_zone_device *tz, int trip) { return -ENODEV; } > -- > 2.1.4 > -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 490 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150406/f80c7d4d/attachment-0001.sig>
[-- Attachment #1: Type: text/plain, Size: 1465 bytes --] 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 | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --]
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 | -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 490 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150406/5037ca27/attachment.sig>
[-- Attachment #1: Type: text/plain, Size: 900 bytes --] On Thu, Mar 26, 2015 at 04:53:49PM +0100, Sascha Hauer wrote: > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> Acked-by: Eduardo Valentin <edubezval@gmail.com> > --- > drivers/thermal/thermal_core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index 0e4ad7c..c735ac4c 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -402,7 +402,7 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip) > } > > /** > - * thermal_zone_get_temp() - returns its the temperature of thermal zone > + * thermal_zone_get_temp() - returns the temperature of a thermal zone > * @tz: a valid pointer to a struct thermal_zone_device > * @temp: a valid pointer to where to store the resulting temperature. > * > -- > 2.1.4 > [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --]
On Thu, Mar 26, 2015 at 04:53:49PM +0100, Sascha Hauer wrote: > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> Acked-by: Eduardo Valentin <edubezval@gmail.com> > --- > drivers/thermal/thermal_core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index 0e4ad7c..c735ac4c 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -402,7 +402,7 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip) > } > > /** > - * thermal_zone_get_temp() - returns its the temperature of thermal zone > + * thermal_zone_get_temp() - returns the temperature of a thermal zone > * @tz: a valid pointer to a struct thermal_zone_device > * @temp: a valid pointer to where to store the resulting temperature. > * > -- > 2.1.4 > -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 490 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150406/c7d23f2a/attachment.sig>
[-- Attachment #1: Type: text/plain, Size: 1905 bytes --] On Fri, Mar 27, 2015 at 06:23:18AM +0100, Sascha Hauer wrote: > Hi Amit, > > On Fri, Mar 27, 2015 at 08:35:50AM +0530, amit daniel kachhap wrote: > > Hi Sascha, > > > > > -#ifdef CONFIG_THERMAL_EMULATION > > > - if (!tz->emul_temperature) > > > - goto skip_emul; > > > - > > > - for (count = 0; count < tz->trips; count++) { > > > - ret = tz->ops->get_trip_type(tz, count, &type); > > > - if (!ret && type == THERMAL_TRIP_CRITICAL) { > > > - ret = tz->ops->get_trip_temp(tz, count, &crit_temp); > > > - break; > > > - } > > > - } > > > - > > > - if (ret) > > > - goto skip_emul; > > > > > > - if (*temp < crit_temp) > > I guess this check is confusing. Actually instead of returning > > emulating temperature it is returning actual temperature. But the > > important thing to look here is that actual temperature is higher than > > critical temperature. So this check prevents the user from suppressing > > the critical temperature and hence prevents from burning up the chip. > > Indeed the check is confusing, but now it makes perfectly sense. I'll > look at the patch again and maybe turn into a patch just adding a > comment to clarify this. That will be great. Thanks Sascha. > > 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 | > -- > 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 [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --]
On Fri, Mar 27, 2015 at 06:23:18AM +0100, Sascha Hauer wrote: > Hi Amit, > > On Fri, Mar 27, 2015 at 08:35:50AM +0530, amit daniel kachhap wrote: > > Hi Sascha, > > > > > -#ifdef CONFIG_THERMAL_EMULATION > > > - if (!tz->emul_temperature) > > > - goto skip_emul; > > > - > > > - for (count = 0; count < tz->trips; count++) { > > > - ret = tz->ops->get_trip_type(tz, count, &type); > > > - if (!ret && type == THERMAL_TRIP_CRITICAL) { > > > - ret = tz->ops->get_trip_temp(tz, count, &crit_temp); > > > - break; > > > - } > > > - } > > > - > > > - if (ret) > > > - goto skip_emul; > > > > > > - if (*temp < crit_temp) > > I guess this check is confusing. Actually instead of returning > > emulating temperature it is returning actual temperature. But the > > important thing to look here is that actual temperature is higher than > > critical temperature. So this check prevents the user from suppressing > > the critical temperature and hence prevents from burning up the chip. > > Indeed the check is confusing, but now it makes perfectly sense. I'll > look at the patch again and maybe turn into a patch just adding a > comment to clarify this. That will be great. Thanks Sascha. > > 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 | > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 490 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150406/2f5c1bfe/attachment.sig>
[-- Attachment #1: Type: text/plain, Size: 1675 bytes --] On Thu, Mar 26, 2015 at 04:53:52PM +0100, Sascha Hauer wrote: > Inline update_temperature into its only caller to make the code > more readable. I am not sure I understand how this is improving readability, can you please elaborate? The way it is now it is more modular at least. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/thermal_core.c | 16 +++++----------- > 1 file changed, 5 insertions(+), 11 deletions(-) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index ebca854..6d0fdad 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -431,11 +431,15 @@ exit: > } > EXPORT_SYMBOL_GPL(thermal_zone_get_temp); > > -static void update_temperature(struct thermal_zone_device *tz) > +void thermal_zone_device_update(struct thermal_zone_device *tz) > { > + int count; > unsigned long temp; > int ret; > > + if (!tz->ops->get_temp) > + return; > + > ret = thermal_zone_get_temp(tz, &temp); > if (ret) { > dev_warn(&tz->device, "failed to read out thermal zone %d\n", > @@ -451,16 +455,6 @@ static void update_temperature(struct thermal_zone_device *tz) > trace_thermal_temperature(tz); > dev_dbg(&tz->device, "last_temperature=%lu, current_temperature=%lu\n", > tz->last_temperature, tz->temperature); > -} > - > -void thermal_zone_device_update(struct thermal_zone_device *tz) > -{ > - int count; > - > - if (!tz->ops->get_temp) > - return; > - > - update_temperature(tz); > > for (count = 0; count < tz->trips; count++) > handle_thermal_trip(tz, count); > -- > 2.1.4 > [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --]
On Thu, Mar 26, 2015 at 04:53:52PM +0100, Sascha Hauer wrote: > Inline update_temperature into its only caller to make the code > more readable. I am not sure I understand how this is improving readability, can you please elaborate? The way it is now it is more modular at least. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/thermal_core.c | 16 +++++----------- > 1 file changed, 5 insertions(+), 11 deletions(-) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index ebca854..6d0fdad 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -431,11 +431,15 @@ exit: > } > EXPORT_SYMBOL_GPL(thermal_zone_get_temp); > > -static void update_temperature(struct thermal_zone_device *tz) > +void thermal_zone_device_update(struct thermal_zone_device *tz) > { > + int count; > unsigned long temp; > int ret; > > + if (!tz->ops->get_temp) > + return; > + > ret = thermal_zone_get_temp(tz, &temp); > if (ret) { > dev_warn(&tz->device, "failed to read out thermal zone %d\n", > @@ -451,16 +455,6 @@ static void update_temperature(struct thermal_zone_device *tz) > trace_thermal_temperature(tz); > dev_dbg(&tz->device, "last_temperature=%lu, current_temperature=%lu\n", > tz->last_temperature, tz->temperature); > -} > - > -void thermal_zone_device_update(struct thermal_zone_device *tz) > -{ > - int count; > - > - if (!tz->ops->get_temp) > - return; > - > - update_temperature(tz); > > for (count = 0; count < tz->trips; count++) > handle_thermal_trip(tz, count); > -- > 2.1.4 > -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 490 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150406/9e258001/attachment.sig>
[-- Attachment #1: Type: text/plain, Size: 5983 bytes --] On Thu, Mar 26, 2015 at 04:53:54PM +0100, Sascha Hauer wrote: > In the thermal framework it was decided that temperatures can't > be negative, so let the .get_temp callback in struct > thermal_zone_of_device_ops take an unsigned long pointer for > the temperature like the .get_temp callback in > struct thermal_zone_device_ops does. This change is required. However, better we move to the direction of using signed type for temperature. However, we want to have int instead of long though. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/hwmon/lm75.c | 2 +- > drivers/hwmon/ntc_thermistor.c | 2 +- > drivers/hwmon/tmp102.c | 2 +- > drivers/input/touchscreen/sun4i-ts.c | 2 +- > drivers/thermal/rockchip_thermal.c | 2 +- > drivers/thermal/samsung/exynos_tmu.c | 2 +- > drivers/thermal/tegra_soctherm.c | 2 +- > drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 2 +- > include/linux/thermal.h | 2 +- > 9 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c > index fe41d5a..9df3ca3 100644 > --- a/drivers/hwmon/lm75.c > +++ b/drivers/hwmon/lm75.c > @@ -104,7 +104,7 @@ static inline long lm75_reg_to_mc(s16 temp, u8 resolution) > > /* sysfs attributes for hwmon */ > > -static int lm75_read_temp(void *dev, long *temp) > +static int lm75_read_temp(void *dev, unsigned long *temp) > { > struct lm75_data *data = lm75_update_device(dev); > > diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c > index 112e4d4..12cb333 100644 > --- a/drivers/hwmon/ntc_thermistor.c > +++ b/drivers/hwmon/ntc_thermistor.c > @@ -430,7 +430,7 @@ static int ntc_thermistor_get_ohm(struct ntc_data *data) > return -EINVAL; > } > > -static int ntc_read_temp(void *dev, long *temp) > +static int ntc_read_temp(void *dev, unsigned long *temp) > { > struct ntc_data *data = dev_get_drvdata(dev); > int ohm; > diff --git a/drivers/hwmon/tmp102.c b/drivers/hwmon/tmp102.c > index 9da2735..25bd72b 100644 > --- a/drivers/hwmon/tmp102.c > +++ b/drivers/hwmon/tmp102.c > @@ -98,7 +98,7 @@ static struct tmp102 *tmp102_update_device(struct device *dev) > return tmp102; > } > > -static int tmp102_read_temp(void *dev, long *temp) > +static int tmp102_read_temp(void *dev, unsigned long *temp) > { > struct tmp102 *tmp102 = tmp102_update_device(dev); > > diff --git a/drivers/input/touchscreen/sun4i-ts.c b/drivers/input/touchscreen/sun4i-ts.c > index b93a28b..26a7cf5 100644 > --- a/drivers/input/touchscreen/sun4i-ts.c > +++ b/drivers/input/touchscreen/sun4i-ts.c > @@ -198,7 +198,7 @@ static int sun4i_get_temp(const struct sun4i_ts_data *ts, long *temp) > return 0; > } > > -static int sun4i_get_tz_temp(void *data, long *temp) > +static int sun4i_get_tz_temp(void *data, unsigned long *temp) > { > return sun4i_get_temp(data, temp); > } > diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c > index 3aa46ac..67dfc67 100644 > --- a/drivers/thermal/rockchip_thermal.c > +++ b/drivers/thermal/rockchip_thermal.c > @@ -366,7 +366,7 @@ static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev) > return IRQ_HANDLED; > } > > -static int rockchip_thermal_get_temp(void *_sensor, long *out_temp) > +static int rockchip_thermal_get_temp(void *_sensor, unsigned long *out_temp) > { > struct rockchip_thermal_sensor *sensor = _sensor; > struct rockchip_thermal_data *thermal = sensor->thermal; > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c > index 1d30b09..5f721f2 100644 > --- a/drivers/thermal/samsung/exynos_tmu.c > +++ b/drivers/thermal/samsung/exynos_tmu.c > @@ -713,7 +713,7 @@ static void exynos7_tmu_control(struct platform_device *pdev, bool on) > writel(con, data->base + EXYNOS_TMU_REG_CONTROL); > } > > -static int exynos_get_temp(void *p, long *temp) > +static int exynos_get_temp(void *p, unsigned long *temp) > { > struct exynos_tmu_data *data = p; > > diff --git a/drivers/thermal/tegra_soctherm.c b/drivers/thermal/tegra_soctherm.c > index 9197fc0..1c4e455 100644 > --- a/drivers/thermal/tegra_soctherm.c > +++ b/drivers/thermal/tegra_soctherm.c > @@ -306,7 +306,7 @@ static long translate_temp(u16 val) > return t; > } > > -static int tegra_thermctl_get_temp(void *data, long *out_temp) > +static int tegra_thermctl_get_temp(void *data, unsigned long *out_temp) > { > struct tegra_thermctl_zone *zone = data; > u32 val; > diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > index 7f8e5f3..f480a01 100644 > --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > @@ -76,7 +76,7 @@ static inline int ti_thermal_hotspot_temperature(int t, int s, int c) > > /* thermal zone ops */ > /* Get temperature callback function for thermal zone*/ > -static inline int __ti_thermal_get_temp(void *devdata, long *temp) > +static inline int __ti_thermal_get_temp(void *devdata, unsigned long *temp) > { > struct thermal_zone_device *pcb_tz = NULL; > struct ti_thermal_data *data = devdata; > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index ba2e29a..2f77091 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -272,7 +272,7 @@ struct thermal_genl_event { > * temperature. > */ > struct thermal_zone_of_device_ops { > - int (*get_temp)(void *, long *); > + int (*get_temp)(void *, unsigned long *); > int (*get_trend)(void *, int trend, enum thermal_trend *); > int (*set_emul_temp)(void *, unsigned long); > }; > -- > 2.1.4 > [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --]
On Thu, Mar 26, 2015 at 04:53:54PM +0100, Sascha Hauer wrote: > In the thermal framework it was decided that temperatures can't > be negative, so let the .get_temp callback in struct > thermal_zone_of_device_ops take an unsigned long pointer for > the temperature like the .get_temp callback in > struct thermal_zone_device_ops does. This change is required. However, better we move to the direction of using signed type for temperature. However, we want to have int instead of long though. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/hwmon/lm75.c | 2 +- > drivers/hwmon/ntc_thermistor.c | 2 +- > drivers/hwmon/tmp102.c | 2 +- > drivers/input/touchscreen/sun4i-ts.c | 2 +- > drivers/thermal/rockchip_thermal.c | 2 +- > drivers/thermal/samsung/exynos_tmu.c | 2 +- > drivers/thermal/tegra_soctherm.c | 2 +- > drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 2 +- > include/linux/thermal.h | 2 +- > 9 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c > index fe41d5a..9df3ca3 100644 > --- a/drivers/hwmon/lm75.c > +++ b/drivers/hwmon/lm75.c > @@ -104,7 +104,7 @@ static inline long lm75_reg_to_mc(s16 temp, u8 resolution) > > /* sysfs attributes for hwmon */ > > -static int lm75_read_temp(void *dev, long *temp) > +static int lm75_read_temp(void *dev, unsigned long *temp) > { > struct lm75_data *data = lm75_update_device(dev); > > diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c > index 112e4d4..12cb333 100644 > --- a/drivers/hwmon/ntc_thermistor.c > +++ b/drivers/hwmon/ntc_thermistor.c > @@ -430,7 +430,7 @@ static int ntc_thermistor_get_ohm(struct ntc_data *data) > return -EINVAL; > } > > -static int ntc_read_temp(void *dev, long *temp) > +static int ntc_read_temp(void *dev, unsigned long *temp) > { > struct ntc_data *data = dev_get_drvdata(dev); > int ohm; > diff --git a/drivers/hwmon/tmp102.c b/drivers/hwmon/tmp102.c > index 9da2735..25bd72b 100644 > --- a/drivers/hwmon/tmp102.c > +++ b/drivers/hwmon/tmp102.c > @@ -98,7 +98,7 @@ static struct tmp102 *tmp102_update_device(struct device *dev) > return tmp102; > } > > -static int tmp102_read_temp(void *dev, long *temp) > +static int tmp102_read_temp(void *dev, unsigned long *temp) > { > struct tmp102 *tmp102 = tmp102_update_device(dev); > > diff --git a/drivers/input/touchscreen/sun4i-ts.c b/drivers/input/touchscreen/sun4i-ts.c > index b93a28b..26a7cf5 100644 > --- a/drivers/input/touchscreen/sun4i-ts.c > +++ b/drivers/input/touchscreen/sun4i-ts.c > @@ -198,7 +198,7 @@ static int sun4i_get_temp(const struct sun4i_ts_data *ts, long *temp) > return 0; > } > > -static int sun4i_get_tz_temp(void *data, long *temp) > +static int sun4i_get_tz_temp(void *data, unsigned long *temp) > { > return sun4i_get_temp(data, temp); > } > diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c > index 3aa46ac..67dfc67 100644 > --- a/drivers/thermal/rockchip_thermal.c > +++ b/drivers/thermal/rockchip_thermal.c > @@ -366,7 +366,7 @@ static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev) > return IRQ_HANDLED; > } > > -static int rockchip_thermal_get_temp(void *_sensor, long *out_temp) > +static int rockchip_thermal_get_temp(void *_sensor, unsigned long *out_temp) > { > struct rockchip_thermal_sensor *sensor = _sensor; > struct rockchip_thermal_data *thermal = sensor->thermal; > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c > index 1d30b09..5f721f2 100644 > --- a/drivers/thermal/samsung/exynos_tmu.c > +++ b/drivers/thermal/samsung/exynos_tmu.c > @@ -713,7 +713,7 @@ static void exynos7_tmu_control(struct platform_device *pdev, bool on) > writel(con, data->base + EXYNOS_TMU_REG_CONTROL); > } > > -static int exynos_get_temp(void *p, long *temp) > +static int exynos_get_temp(void *p, unsigned long *temp) > { > struct exynos_tmu_data *data = p; > > diff --git a/drivers/thermal/tegra_soctherm.c b/drivers/thermal/tegra_soctherm.c > index 9197fc0..1c4e455 100644 > --- a/drivers/thermal/tegra_soctherm.c > +++ b/drivers/thermal/tegra_soctherm.c > @@ -306,7 +306,7 @@ static long translate_temp(u16 val) > return t; > } > > -static int tegra_thermctl_get_temp(void *data, long *out_temp) > +static int tegra_thermctl_get_temp(void *data, unsigned long *out_temp) > { > struct tegra_thermctl_zone *zone = data; > u32 val; > diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > index 7f8e5f3..f480a01 100644 > --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > @@ -76,7 +76,7 @@ static inline int ti_thermal_hotspot_temperature(int t, int s, int c) > > /* thermal zone ops */ > /* Get temperature callback function for thermal zone*/ > -static inline int __ti_thermal_get_temp(void *devdata, long *temp) > +static inline int __ti_thermal_get_temp(void *devdata, unsigned long *temp) > { > struct thermal_zone_device *pcb_tz = NULL; > struct ti_thermal_data *data = devdata; > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index ba2e29a..2f77091 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -272,7 +272,7 @@ struct thermal_genl_event { > * temperature. > */ > struct thermal_zone_of_device_ops { > - int (*get_temp)(void *, long *); > + int (*get_temp)(void *, unsigned long *); > int (*get_trend)(void *, int trend, enum thermal_trend *); > int (*set_emul_temp)(void *, unsigned long); > }; > -- > 2.1.4 > -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 490 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150406/fac001a2/attachment.sig>
[-- Attachment #1: Type: text/plain, Size: 4149 bytes --] On Thu, Mar 26, 2015 at 04:53:59PM +0100, Sascha Hauer wrote: > This adds support for hardware-tracked trip points to the device tree > thermal sensor framework. > > The framework supports an arbitrary number of trip points. Whenever > the current temperature is updated, the trip points immediately > below and above the current temperature are found. A .set_trips > callback is then called with the temperatures. If there is no trip > point above or below the current temperature, the passed trip > temperature will be ULONG_MAX or 0 respectively. In this callback, > the driver should program the hardware such that it is notified > when either of these trip points are triggered. When a trip point > is triggered, the driver should call `thermal_zone_device_update' > for the respective thermal zone. This will cause the trip points > to be updated again. > > If .set_trips is not implemented, the framework behaves as before. > > This patch is based on an earlier version from Mikko Perttunen > <mikko.perttunen@kapsi.fi> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/thermal_core.c | 41 +++++++++++++++++++++++++++++++++++++++++ > include/linux/thermal.h | 3 +++ > 2 files changed, 44 insertions(+) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index dcdf45e..7138f8f 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -434,6 +434,45 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) > } > EXPORT_SYMBOL_GPL(thermal_zone_get_temp); > > +static void thermal_zone_set_trips(struct thermal_zone_device *tz) > +{ > + unsigned long low = 0; > + unsigned long high = ULONG_MAX; > + unsigned long trip_temp, hysteresis; > + unsigned long temp = tz->temperature; > + int i; > + > + if (!tz->ops->set_trips) > + return; > + > + /* No need to change trip points */ > + if (temp > tz->prev_low_trip && temp < tz->prev_high_trip) > + return; > + > + for (i = 0; i < tz->trips; i++) { > + unsigned long trip_low; > + > + tz->ops->get_trip_temp(tz, i, &trip_temp); > + tz->ops->get_trip_hyst(tz, i, &hysteresis); > + > + trip_low = trip_temp - hysteresis; > + > + if (trip_low < temp && trip_low > low) > + low = trip_low; > + > + if (trip_temp > temp && trip_temp < high) > + high = trip_temp; > + } > + > + tz->prev_low_trip = low; > + tz->prev_high_trip = high; > + > + dev_dbg(&tz->device, "new temperature boundaries: %lu < x < %lu\n", > + low, high); > + > + tz->ops->set_trips(tz, low, high); > +} > + > void thermal_zone_device_update(struct thermal_zone_device *tz) > { > int count; > @@ -460,6 +499,8 @@ void thermal_zone_device_update(struct thermal_zone_device *tz) > dev_dbg(&tz->device, "last_temperature=%lu, current_temperature=%lu\n", > tz->last_temperature, tz->temperature); > > + thermal_zone_set_trips(tz); Do we need to lock the tz->lock to perform this operation of setting the hardware trip points? > + > for (count = 0; count < tz->trips; count++) > handle_thermal_trip(tz, count); > } > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index ac2897c..b870702 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -87,6 +87,7 @@ struct thermal_zone_device_ops { > int (*unbind) (struct thermal_zone_device *, > struct thermal_cooling_device *); > int (*get_temp) (struct thermal_zone_device *, unsigned long *); > + int (*set_trips) (struct thermal_zone_device *, unsigned long, unsigned long); > int (*get_mode) (struct thermal_zone_device *, > enum thermal_device_mode *); > int (*set_mode) (struct thermal_zone_device *, > @@ -183,6 +184,8 @@ struct thermal_zone_device { > unsigned long temperature; > unsigned long last_temperature; > unsigned long emul_temperature; > + unsigned long prev_low_trip; > + unsigned long prev_high_trip; > int passive; > unsigned int forced_passive; > const struct thermal_zone_device_ops *ops; > -- > 2.1.4 > [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --]
On Thu, Mar 26, 2015 at 04:53:59PM +0100, Sascha Hauer wrote: > This adds support for hardware-tracked trip points to the device tree > thermal sensor framework. > > The framework supports an arbitrary number of trip points. Whenever > the current temperature is updated, the trip points immediately > below and above the current temperature are found. A .set_trips > callback is then called with the temperatures. If there is no trip > point above or below the current temperature, the passed trip > temperature will be ULONG_MAX or 0 respectively. In this callback, > the driver should program the hardware such that it is notified > when either of these trip points are triggered. When a trip point > is triggered, the driver should call `thermal_zone_device_update' > for the respective thermal zone. This will cause the trip points > to be updated again. > > If .set_trips is not implemented, the framework behaves as before. > > This patch is based on an earlier version from Mikko Perttunen > <mikko.perttunen@kapsi.fi> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/thermal_core.c | 41 +++++++++++++++++++++++++++++++++++++++++ > include/linux/thermal.h | 3 +++ > 2 files changed, 44 insertions(+) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index dcdf45e..7138f8f 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -434,6 +434,45 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) > } > EXPORT_SYMBOL_GPL(thermal_zone_get_temp); > > +static void thermal_zone_set_trips(struct thermal_zone_device *tz) > +{ > + unsigned long low = 0; > + unsigned long high = ULONG_MAX; > + unsigned long trip_temp, hysteresis; > + unsigned long temp = tz->temperature; > + int i; > + > + if (!tz->ops->set_trips) > + return; > + > + /* No need to change trip points */ > + if (temp > tz->prev_low_trip && temp < tz->prev_high_trip) > + return; > + > + for (i = 0; i < tz->trips; i++) { > + unsigned long trip_low; > + > + tz->ops->get_trip_temp(tz, i, &trip_temp); > + tz->ops->get_trip_hyst(tz, i, &hysteresis); > + > + trip_low = trip_temp - hysteresis; > + > + if (trip_low < temp && trip_low > low) > + low = trip_low; > + > + if (trip_temp > temp && trip_temp < high) > + high = trip_temp; > + } > + > + tz->prev_low_trip = low; > + tz->prev_high_trip = high; > + > + dev_dbg(&tz->device, "new temperature boundaries: %lu < x < %lu\n", > + low, high); > + > + tz->ops->set_trips(tz, low, high); > +} > + > void thermal_zone_device_update(struct thermal_zone_device *tz) > { > int count; > @@ -460,6 +499,8 @@ void thermal_zone_device_update(struct thermal_zone_device *tz) > dev_dbg(&tz->device, "last_temperature=%lu, current_temperature=%lu\n", > tz->last_temperature, tz->temperature); > > + thermal_zone_set_trips(tz); Do we need to lock the tz->lock to perform this operation of setting the hardware trip points? > + > for (count = 0; count < tz->trips; count++) > handle_thermal_trip(tz, count); > } > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index ac2897c..b870702 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -87,6 +87,7 @@ struct thermal_zone_device_ops { > int (*unbind) (struct thermal_zone_device *, > struct thermal_cooling_device *); > int (*get_temp) (struct thermal_zone_device *, unsigned long *); > + int (*set_trips) (struct thermal_zone_device *, unsigned long, unsigned long); > int (*get_mode) (struct thermal_zone_device *, > enum thermal_device_mode *); > int (*set_mode) (struct thermal_zone_device *, > @@ -183,6 +184,8 @@ struct thermal_zone_device { > unsigned long temperature; > unsigned long last_temperature; > unsigned long emul_temperature; > + unsigned long prev_low_trip; > + unsigned long prev_high_trip; > int passive; > unsigned int forced_passive; > const struct thermal_zone_device_ops *ops; > -- > 2.1.4 > -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 490 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150406/d4723b2a/attachment.sig>
[-- Attachment #1: Type: text/plain, Size: 2106 bytes --] On Thu, Mar 26, 2015 at 04:54:00PM +0100, Sascha Hauer wrote: > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/of-thermal.c | 12 ++++++++++++ > include/linux/thermal.h | 1 + > 2 files changed, 13 insertions(+) Can you please include at least one user of this call back in your patch series? > > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > index 9b63193..a3de5de 100644 > --- a/drivers/thermal/of-thermal.c > +++ b/drivers/thermal/of-thermal.c > @@ -97,6 +97,17 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz, > return data->ops->get_temp(data->sensor_data, temp); > } > > +static int of_thermal_set_trips(struct thermal_zone_device *tz, > + unsigned long low, unsigned long high) > +{ > + struct __thermal_zone *data = tz->devdata; > + > + if (!data->ops || !data->ops->set_trips) > + return -ENOSYS; > + > + return data->ops->set_trips(data->sensor_data, low, high); > +} > + > /** > * of_thermal_get_ntrips - function to export number of available trip > * points. > @@ -367,6 +378,7 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz, > > static const struct thermal_zone_device_ops of_thermal_ops = { > .get_temp = of_thermal_get_temp, > + .set_trips = of_thermal_set_trips, > .get_trend = of_thermal_get_trend, > .set_emul_temp = of_thermal_set_emul_temp, > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index b870702..84a5b5d 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -276,6 +276,7 @@ struct thermal_genl_event { > */ > struct thermal_zone_of_device_ops { > int (*get_temp)(void *, unsigned long *); > + int (*set_trips)(void *, unsigned long, unsigned long); Could you please keep the kernel doc entry up to date? I know we donot have entries for all structs, but I am working in improving this. > int (*get_trend)(void *, int trend, enum thermal_trend *); > int (*set_emul_temp)(void *, unsigned long); > }; > -- > 2.1.4 > [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --]
On Thu, Mar 26, 2015 at 04:54:00PM +0100, Sascha Hauer wrote: > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/of-thermal.c | 12 ++++++++++++ > include/linux/thermal.h | 1 + > 2 files changed, 13 insertions(+) Can you please include at least one user of this call back in your patch series? > > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > index 9b63193..a3de5de 100644 > --- a/drivers/thermal/of-thermal.c > +++ b/drivers/thermal/of-thermal.c > @@ -97,6 +97,17 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz, > return data->ops->get_temp(data->sensor_data, temp); > } > > +static int of_thermal_set_trips(struct thermal_zone_device *tz, > + unsigned long low, unsigned long high) > +{ > + struct __thermal_zone *data = tz->devdata; > + > + if (!data->ops || !data->ops->set_trips) > + return -ENOSYS; > + > + return data->ops->set_trips(data->sensor_data, low, high); > +} > + > /** > * of_thermal_get_ntrips - function to export number of available trip > * points. > @@ -367,6 +378,7 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz, > > static const struct thermal_zone_device_ops of_thermal_ops = { > .get_temp = of_thermal_get_temp, > + .set_trips = of_thermal_set_trips, > .get_trend = of_thermal_get_trend, > .set_emul_temp = of_thermal_set_emul_temp, > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index b870702..84a5b5d 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -276,6 +276,7 @@ struct thermal_genl_event { > */ > struct thermal_zone_of_device_ops { > int (*get_temp)(void *, unsigned long *); > + int (*set_trips)(void *, unsigned long, unsigned long); Could you please keep the kernel doc entry up to date? I know we donot have entries for all structs, but I am working in improving this. > int (*get_trend)(void *, int trend, enum thermal_trend *); > int (*set_emul_temp)(void *, unsigned long); > }; > -- > 2.1.4 > -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 490 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150406/26e049b6/attachment.sig>
[-- Attachment #1: Type: text/plain, Size: 1097 bytes --] On Thu, Mar 26, 2015 at 04:53:50PM +0100, Sascha Hauer wrote: > When the thermal zone has no get_temp callback then thermal_zone_device_register() > calls thermal_zone_device_set_polling() with a polling delay of 0. This > only cancels the poll_queue. Since the poll_queue hasn't been scheduled this > is a no-op. Remove it. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> This seams reasonable to me: Acked-by: Eduardo Valentin <edubezval@gmail.com> > --- > drivers/thermal/thermal_core.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index c735ac4c..dcea909 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -1571,9 +1571,6 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type, > > INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check); > > - if (!tz->ops->get_temp) > - thermal_zone_device_set_polling(tz, 0); > - > thermal_zone_device_update(tz); > > return tz; > -- > 2.1.4 > [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --]
On Thu, Mar 26, 2015 at 04:53:50PM +0100, Sascha Hauer wrote: > When the thermal zone has no get_temp callback then thermal_zone_device_register() > calls thermal_zone_device_set_polling() with a polling delay of 0. This > only cancels the poll_queue. Since the poll_queue hasn't been scheduled this > is a no-op. Remove it. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> This seams reasonable to me: Acked-by: Eduardo Valentin <edubezval@gmail.com> > --- > drivers/thermal/thermal_core.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index c735ac4c..dcea909 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -1571,9 +1571,6 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type, > > INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check); > > - if (!tz->ops->get_temp) > - thermal_zone_device_set_polling(tz, 0); > - > thermal_zone_device_update(tz); > > return tz; > -- > 2.1.4 > -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 490 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150406/6f8bf09a/attachment-0001.sig>
[-- Attachment #1: Type: text/plain, Size: 4224 bytes --] On Thu, Mar 26, 2015 at 04:53:53PM +0100, Sascha Hauer wrote: > The .get_trend callback in struct thermal_zone_device_ops has the prototype: > > int (*get_trend) (struct thermal_zone_device *, int, > enum thermal_trend *); > > whereas the .get_trend callback in struct thermal_zone_of_device_ops has: > > int (*get_trend)(void *, long *); > > Streamline both prototypes and add the trip argument to the OF callback > aswell and use enum thermal_trend * instead of an integer pointer. > > While the OF prototype may be the better one, this should be decided at > framework level and not on OF level. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/of-thermal.c | 11 +--------- > drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 25 +++++++--------------- > include/linux/thermal.h | 2 +- > 3 files changed, 10 insertions(+), 28 deletions(-) > > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > index 668fb1b..b39e22f 100644 > --- a/drivers/thermal/of-thermal.c > +++ b/drivers/thermal/of-thermal.c > @@ -187,24 +187,15 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, > enum thermal_trend *trend) > { > struct __thermal_zone *data = tz->devdata; > - long dev_trend; > int r; > > if (!data->ops->get_trend) > return -EINVAL; > > - r = data->ops->get_trend(data->sensor_data, &dev_trend); > + r = data->ops->get_trend(data->sensor_data, trip, trend); > if (r) > return r; > > - /* TODO: These intervals might have some thresholds, but in core code */ > - if (dev_trend > 0) > - *trend = THERMAL_TREND_RAISING; > - else if (dev_trend < 0) > - *trend = THERMAL_TREND_DROPPING; > - else > - *trend = THERMAL_TREND_STABLE; > - > return 0; > } > > diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > index a38c175..7f8e5f3 100644 > --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > @@ -238,7 +238,7 @@ static int ti_thermal_get_trip_temp(struct thermal_zone_device *thermal, > return 0; > } > > -static int __ti_thermal_get_trend(void *p, long *trend) > +static int __ti_thermal_get_trend(void *p, int trip, enum thermal_trend *trend) > { > struct ti_thermal_data *data = p; > struct ti_bandgap *bgp; > @@ -251,22 +251,6 @@ static int __ti_thermal_get_trend(void *p, long *trend) > if (ret) > return ret; > > - *trend = tr; > - > - return 0; > -} > - > -/* Get the temperature trend callback functions for thermal zone */ > -static int ti_thermal_get_trend(struct thermal_zone_device *thermal, > - int trip, enum thermal_trend *trend) > -{ > - int ret; > - long tr; > - > - ret = __ti_thermal_get_trend(thermal->devdata, &tr); > - if (ret) > - return ret; > - > if (tr > 0) > *trend = THERMAL_TREND_RAISING; > else if (tr < 0) > @@ -277,6 +261,13 @@ static int ti_thermal_get_trend(struct thermal_zone_device *thermal, > return 0; > } > > +/* Get the temperature trend callback functions for thermal zone */ > +static int ti_thermal_get_trend(struct thermal_zone_device *thermal, > + int trip, enum thermal_trend *trend) > +{ > + return __ti_thermal_get_trend(thermal->devdata, trip, trend); > +} > + > /* Get critical temperature callback functions for thermal zone */ > static int ti_thermal_get_crit_temp(struct thermal_zone_device *thermal, > unsigned long *temp) > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index db6c12b..ba2e29a 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -273,7 +273,7 @@ struct thermal_genl_event { > */ > struct thermal_zone_of_device_ops { > int (*get_temp)(void *, long *); > - int (*get_trend)(void *, long *); > + int (*get_trend)(void *, int trend, enum thermal_trend *); Could you please keep the kernel doc entry up to date? Apart from that, looks good to me. > int (*set_emul_temp)(void *, unsigned long); > }; > > -- > 2.1.4 > [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --]
On Thu, Mar 26, 2015 at 04:53:53PM +0100, Sascha Hauer wrote: > The .get_trend callback in struct thermal_zone_device_ops has the prototype: > > int (*get_trend) (struct thermal_zone_device *, int, > enum thermal_trend *); > > whereas the .get_trend callback in struct thermal_zone_of_device_ops has: > > int (*get_trend)(void *, long *); > > Streamline both prototypes and add the trip argument to the OF callback > aswell and use enum thermal_trend * instead of an integer pointer. > > While the OF prototype may be the better one, this should be decided at > framework level and not on OF level. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/of-thermal.c | 11 +--------- > drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 25 +++++++--------------- > include/linux/thermal.h | 2 +- > 3 files changed, 10 insertions(+), 28 deletions(-) > > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > index 668fb1b..b39e22f 100644 > --- a/drivers/thermal/of-thermal.c > +++ b/drivers/thermal/of-thermal.c > @@ -187,24 +187,15 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, > enum thermal_trend *trend) > { > struct __thermal_zone *data = tz->devdata; > - long dev_trend; > int r; > > if (!data->ops->get_trend) > return -EINVAL; > > - r = data->ops->get_trend(data->sensor_data, &dev_trend); > + r = data->ops->get_trend(data->sensor_data, trip, trend); > if (r) > return r; > > - /* TODO: These intervals might have some thresholds, but in core code */ > - if (dev_trend > 0) > - *trend = THERMAL_TREND_RAISING; > - else if (dev_trend < 0) > - *trend = THERMAL_TREND_DROPPING; > - else > - *trend = THERMAL_TREND_STABLE; > - > return 0; > } > > diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > index a38c175..7f8e5f3 100644 > --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > @@ -238,7 +238,7 @@ static int ti_thermal_get_trip_temp(struct thermal_zone_device *thermal, > return 0; > } > > -static int __ti_thermal_get_trend(void *p, long *trend) > +static int __ti_thermal_get_trend(void *p, int trip, enum thermal_trend *trend) > { > struct ti_thermal_data *data = p; > struct ti_bandgap *bgp; > @@ -251,22 +251,6 @@ static int __ti_thermal_get_trend(void *p, long *trend) > if (ret) > return ret; > > - *trend = tr; > - > - return 0; > -} > - > -/* Get the temperature trend callback functions for thermal zone */ > -static int ti_thermal_get_trend(struct thermal_zone_device *thermal, > - int trip, enum thermal_trend *trend) > -{ > - int ret; > - long tr; > - > - ret = __ti_thermal_get_trend(thermal->devdata, &tr); > - if (ret) > - return ret; > - > if (tr > 0) > *trend = THERMAL_TREND_RAISING; > else if (tr < 0) > @@ -277,6 +261,13 @@ static int ti_thermal_get_trend(struct thermal_zone_device *thermal, > return 0; > } > > +/* Get the temperature trend callback functions for thermal zone */ > +static int ti_thermal_get_trend(struct thermal_zone_device *thermal, > + int trip, enum thermal_trend *trend) > +{ > + return __ti_thermal_get_trend(thermal->devdata, trip, trend); > +} > + > /* Get critical temperature callback functions for thermal zone */ > static int ti_thermal_get_crit_temp(struct thermal_zone_device *thermal, > unsigned long *temp) > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index db6c12b..ba2e29a 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -273,7 +273,7 @@ struct thermal_genl_event { > */ > struct thermal_zone_of_device_ops { > int (*get_temp)(void *, long *); > - int (*get_trend)(void *, long *); > + int (*get_trend)(void *, int trend, enum thermal_trend *); Could you please keep the kernel doc entry up to date? Apart from that, looks good to me. > int (*set_emul_temp)(void *, unsigned long); > }; > > -- > 2.1.4 > -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 490 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150406/71e18521/attachment.sig>
[-- Attachment #1: Type: text/plain, Size: 2560 bytes --] On Thu, Mar 26, 2015 at 04:53:56PM +0100, Sascha Hauer wrote: > Now that the thermal core treats -ENOSYS like the callbacks were > not present at all we no longer have to overwrite the ops during > runtime but instead can always set them and return -ENOSYS if no > sensor is registered. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/of-thermal.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > index b39e22f..df14fdd 100644 > --- a/drivers/thermal/of-thermal.c > +++ b/drivers/thermal/of-thermal.c > @@ -91,7 +91,7 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz, > { > struct __thermal_zone *data = tz->devdata; > > - if (!data->ops->get_temp) > + if (!data->ops) > return -EINVAL; > > return data->ops->get_temp(data->sensor_data, temp); > @@ -178,7 +178,7 @@ static int of_thermal_set_emul_temp(struct thermal_zone_device *tz, > struct __thermal_zone *data = tz->devdata; > > if (!data->ops || !data->ops->set_emul_temp) > - return -EINVAL; > + return -ENOSYS; > > return data->ops->set_emul_temp(data->sensor_data, temp); > } > @@ -189,8 +189,8 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, > struct __thermal_zone *data = tz->devdata; > int r; > > - if (!data->ops->get_trend) > - return -EINVAL; > + if (!data->ops || !data->ops->get_trend) > + return -ENOSYS; > > r = data->ops->get_trend(data->sensor_data, trip, trend); > if (r) > @@ -366,6 +366,10 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz, > } > > static struct thermal_zone_device_ops of_thermal_ops = { > + .get_temp = of_thermal_get_temp, > + .get_trend = of_thermal_get_trend, > + .set_emul_temp = of_thermal_set_emul_temp, > + > .get_mode = of_thermal_get_mode, > .set_mode = of_thermal_set_mode, > > @@ -399,13 +403,13 @@ thermal_zone_of_add_sensor(struct device_node *zone, > if (!ops) > return ERR_PTR(-EINVAL); > > + if (!ops->get_temp) > + return ERR_PTR(-EINVAL); > + > mutex_lock(&tzd->lock); > tz->ops = ops; > tz->sensor_data = data; > > - tzd->ops->get_temp = of_thermal_get_temp; > - tzd->ops->get_trend = of_thermal_get_trend; > - tzd->ops->set_emul_temp = of_thermal_set_emul_temp; You may want to update the thermal_zone_of_sensor_unregister too. > mutex_unlock(&tzd->lock); > > return tzd; > -- > 2.1.4 > [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --]
On Thu, Mar 26, 2015 at 04:53:56PM +0100, Sascha Hauer wrote: > Now that the thermal core treats -ENOSYS like the callbacks were > not present at all we no longer have to overwrite the ops during > runtime but instead can always set them and return -ENOSYS if no > sensor is registered. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/of-thermal.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > index b39e22f..df14fdd 100644 > --- a/drivers/thermal/of-thermal.c > +++ b/drivers/thermal/of-thermal.c > @@ -91,7 +91,7 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz, > { > struct __thermal_zone *data = tz->devdata; > > - if (!data->ops->get_temp) > + if (!data->ops) > return -EINVAL; > > return data->ops->get_temp(data->sensor_data, temp); > @@ -178,7 +178,7 @@ static int of_thermal_set_emul_temp(struct thermal_zone_device *tz, > struct __thermal_zone *data = tz->devdata; > > if (!data->ops || !data->ops->set_emul_temp) > - return -EINVAL; > + return -ENOSYS; > > return data->ops->set_emul_temp(data->sensor_data, temp); > } > @@ -189,8 +189,8 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, > struct __thermal_zone *data = tz->devdata; > int r; > > - if (!data->ops->get_trend) > - return -EINVAL; > + if (!data->ops || !data->ops->get_trend) > + return -ENOSYS; > > r = data->ops->get_trend(data->sensor_data, trip, trend); > if (r) > @@ -366,6 +366,10 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz, > } > > static struct thermal_zone_device_ops of_thermal_ops = { > + .get_temp = of_thermal_get_temp, > + .get_trend = of_thermal_get_trend, > + .set_emul_temp = of_thermal_set_emul_temp, > + > .get_mode = of_thermal_get_mode, > .set_mode = of_thermal_set_mode, > > @@ -399,13 +403,13 @@ thermal_zone_of_add_sensor(struct device_node *zone, > if (!ops) > return ERR_PTR(-EINVAL); > > + if (!ops->get_temp) > + return ERR_PTR(-EINVAL); > + > mutex_lock(&tzd->lock); > tz->ops = ops; > tz->sensor_data = data; > > - tzd->ops->get_temp = of_thermal_get_temp; > - tzd->ops->get_trend = of_thermal_get_trend; > - tzd->ops->set_emul_temp = of_thermal_set_emul_temp; You may want to update the thermal_zone_of_sensor_unregister too. > mutex_unlock(&tzd->lock); > > return tzd; > -- > 2.1.4 > -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 490 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150406/06051543/attachment.sig>
[-- Attachment #1: Type: text/plain, Size: 2465 bytes --] On Thu, Mar 26, 2015 at 04:53:58PM +0100, Sascha Hauer wrote: > Now that we no longer modify the ops they can be const again. Also > we no longer have to duplicate them. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> Should this one be merged to patch 09/13? > --- > drivers/thermal/of-thermal.c | 18 +++--------------- > 1 file changed, 3 insertions(+), 15 deletions(-) > > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > index df14fdd..9b63193 100644 > --- a/drivers/thermal/of-thermal.c > +++ b/drivers/thermal/of-thermal.c > @@ -365,7 +365,7 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz, > return -EINVAL; > } > > -static struct thermal_zone_device_ops of_thermal_ops = { > +static const struct thermal_zone_device_ops of_thermal_ops = { > .get_temp = of_thermal_get_temp, > .get_trend = of_thermal_get_trend, > .set_emul_temp = of_thermal_set_emul_temp, > @@ -539,10 +539,6 @@ void thermal_zone_of_sensor_unregister(struct device *dev, > return; > > mutex_lock(&tzd->lock); > - tzd->ops->get_temp = NULL; > - tzd->ops->get_trend = NULL; > - tzd->ops->set_emul_temp = NULL; > - > tz->ops = NULL; > tz->sensor_data = NULL; > mutex_unlock(&tzd->lock); > @@ -849,7 +845,6 @@ int __init of_parse_thermal_zones(void) > { > struct device_node *np, *child; > struct __thermal_zone *tz; > - struct thermal_zone_device_ops *ops; > > np = of_find_node_by_name(NULL, "thermal-zones"); > if (!np) { > @@ -873,29 +868,22 @@ int __init of_parse_thermal_zones(void) > continue; > } > > - ops = kmemdup(&of_thermal_ops, sizeof(*ops), GFP_KERNEL); > - if (!ops) > - goto exit_free; > - > tzp = kzalloc(sizeof(*tzp), GFP_KERNEL); > - if (!tzp) { > - kfree(ops); > + if (!tzp) > goto exit_free; > - } > > /* No hwmon because there might be hwmon drivers registering */ > tzp->no_hwmon = true; > > zone = thermal_zone_device_register(child->name, tz->ntrips, > 0, tz, > - ops, tzp, > + &of_thermal_ops, tzp, > tz->passive_delay, > tz->polling_delay); > if (IS_ERR(zone)) { > pr_err("Failed to build %s zone %ld\n", child->name, > PTR_ERR(zone)); > kfree(tzp); > - kfree(ops); > of_thermal_free_zone(tz); > /* attempting to build remaining zones still */ > } > -- > 2.1.4 > [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --]
On Thu, Mar 26, 2015 at 04:53:58PM +0100, Sascha Hauer wrote: > Now that we no longer modify the ops they can be const again. Also > we no longer have to duplicate them. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> Should this one be merged to patch 09/13? > --- > drivers/thermal/of-thermal.c | 18 +++--------------- > 1 file changed, 3 insertions(+), 15 deletions(-) > > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > index df14fdd..9b63193 100644 > --- a/drivers/thermal/of-thermal.c > +++ b/drivers/thermal/of-thermal.c > @@ -365,7 +365,7 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz, > return -EINVAL; > } > > -static struct thermal_zone_device_ops of_thermal_ops = { > +static const struct thermal_zone_device_ops of_thermal_ops = { > .get_temp = of_thermal_get_temp, > .get_trend = of_thermal_get_trend, > .set_emul_temp = of_thermal_set_emul_temp, > @@ -539,10 +539,6 @@ void thermal_zone_of_sensor_unregister(struct device *dev, > return; > > mutex_lock(&tzd->lock); > - tzd->ops->get_temp = NULL; > - tzd->ops->get_trend = NULL; > - tzd->ops->set_emul_temp = NULL; > - > tz->ops = NULL; > tz->sensor_data = NULL; > mutex_unlock(&tzd->lock); > @@ -849,7 +845,6 @@ int __init of_parse_thermal_zones(void) > { > struct device_node *np, *child; > struct __thermal_zone *tz; > - struct thermal_zone_device_ops *ops; > > np = of_find_node_by_name(NULL, "thermal-zones"); > if (!np) { > @@ -873,29 +868,22 @@ int __init of_parse_thermal_zones(void) > continue; > } > > - ops = kmemdup(&of_thermal_ops, sizeof(*ops), GFP_KERNEL); > - if (!ops) > - goto exit_free; > - > tzp = kzalloc(sizeof(*tzp), GFP_KERNEL); > - if (!tzp) { > - kfree(ops); > + if (!tzp) > goto exit_free; > - } > > /* No hwmon because there might be hwmon drivers registering */ > tzp->no_hwmon = true; > > zone = thermal_zone_device_register(child->name, tz->ntrips, > 0, tz, > - ops, tzp, > + &of_thermal_ops, tzp, > tz->passive_delay, > tz->polling_delay); > if (IS_ERR(zone)) { > pr_err("Failed to build %s zone %ld\n", child->name, > PTR_ERR(zone)); > kfree(tzp); > - kfree(ops); > of_thermal_free_zone(tz); > /* attempting to build remaining zones still */ > } > -- > 2.1.4 > -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 490 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150406/46ab7b3a/attachment-0001.sig>
[-- Attachment #1: Type: text/plain, Size: 2528 bytes --] On Thu, Mar 26, 2015 at 04:53:57PM +0100, Sascha Hauer wrote: > Now that the of thermal support no longer changes the > thermal_zone_device_ops it can be const again. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/thermal_core.c | 2 +- > include/linux/thermal.h | 6 +++--- I believe this change deserves to be done together with the required driver updates on those parts that calls this function. > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index acf00b5..dcdf45e 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -1455,7 +1455,7 @@ static void remove_trip_attrs(struct thermal_zone_device *tz) > */ > struct thermal_zone_device *thermal_zone_device_register(const char *type, > int trips, int mask, void *devdata, > - struct thermal_zone_device_ops *ops, > + const struct thermal_zone_device_ops *ops, > const struct thermal_zone_params *tzp, > int passive_delay, int polling_delay) > { > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index 2f77091..ac2897c 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -185,7 +185,7 @@ struct thermal_zone_device { > unsigned long emul_temperature; > int passive; > unsigned int forced_passive; > - struct thermal_zone_device_ops *ops; > + const struct thermal_zone_device_ops *ops; > const struct thermal_zone_params *tzp; > struct thermal_governor *governor; > struct list_head thermal_instances; > @@ -317,7 +317,7 @@ void thermal_zone_of_sensor_unregister(struct device *dev, > > #if IS_ENABLED(CONFIG_THERMAL) > struct thermal_zone_device *thermal_zone_device_register(const char *, int, int, > - void *, struct thermal_zone_device_ops *, > + void *, const struct thermal_zone_device_ops *, > const struct thermal_zone_params *, int, int); > void thermal_zone_device_unregister(struct thermal_zone_device *); > > @@ -345,7 +345,7 @@ void thermal_notify_framework(struct thermal_zone_device *, int); > #else > static inline struct thermal_zone_device *thermal_zone_device_register( > const char *type, int trips, int mask, void *devdata, > - struct thermal_zone_device_ops *ops, > + const struct thermal_zone_device_ops *ops, > const struct thermal_zone_params *tzp, > int passive_delay, int polling_delay) > { return ERR_PTR(-ENODEV); } > -- > 2.1.4 > [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --]
On Thu, Mar 26, 2015 at 04:53:57PM +0100, Sascha Hauer wrote: > Now that the of thermal support no longer changes the > thermal_zone_device_ops it can be const again. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/thermal_core.c | 2 +- > include/linux/thermal.h | 6 +++--- I believe this change deserves to be done together with the required driver updates on those parts that calls this function. > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index acf00b5..dcdf45e 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -1455,7 +1455,7 @@ static void remove_trip_attrs(struct thermal_zone_device *tz) > */ > struct thermal_zone_device *thermal_zone_device_register(const char *type, > int trips, int mask, void *devdata, > - struct thermal_zone_device_ops *ops, > + const struct thermal_zone_device_ops *ops, > const struct thermal_zone_params *tzp, > int passive_delay, int polling_delay) > { > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index 2f77091..ac2897c 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -185,7 +185,7 @@ struct thermal_zone_device { > unsigned long emul_temperature; > int passive; > unsigned int forced_passive; > - struct thermal_zone_device_ops *ops; > + const struct thermal_zone_device_ops *ops; > const struct thermal_zone_params *tzp; > struct thermal_governor *governor; > struct list_head thermal_instances; > @@ -317,7 +317,7 @@ void thermal_zone_of_sensor_unregister(struct device *dev, > > #if IS_ENABLED(CONFIG_THERMAL) > struct thermal_zone_device *thermal_zone_device_register(const char *, int, int, > - void *, struct thermal_zone_device_ops *, > + void *, const struct thermal_zone_device_ops *, > const struct thermal_zone_params *, int, int); > void thermal_zone_device_unregister(struct thermal_zone_device *); > > @@ -345,7 +345,7 @@ void thermal_notify_framework(struct thermal_zone_device *, int); > #else > static inline struct thermal_zone_device *thermal_zone_device_register( > const char *type, int trips, int mask, void *devdata, > - struct thermal_zone_device_ops *ops, > + const struct thermal_zone_device_ops *ops, > const struct thermal_zone_params *tzp, > int passive_delay, int polling_delay) > { return ERR_PTR(-ENODEV); } > -- > 2.1.4 > -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 490 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150406/ca99de30/attachment.sig>
Hi Eduardo, On Mon, Apr 06, 2015 at 07:43:08PM -0700, Eduardo Valentin wrote: > On Thu, Mar 26, 2015 at 04:54:00PM +0100, Sascha Hauer wrote: > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > drivers/thermal/of-thermal.c | 12 ++++++++++++ > > include/linux/thermal.h | 1 + > > 2 files changed, 13 insertions(+) > > Can you please include at least one user of this call back in your patch > series? Thanks for the comments on this series. I'll address them in the next round. The user for this callback is not yet ready to be posted. I'll try and move this patch to the end of the series so that the series can be applied without this one. Thanks 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 |
Hi Eduardo, On Mon, Apr 06, 2015 at 07:43:08PM -0700, Eduardo Valentin wrote: > On Thu, Mar 26, 2015 at 04:54:00PM +0100, Sascha Hauer wrote: > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > drivers/thermal/of-thermal.c | 12 ++++++++++++ > > include/linux/thermal.h | 1 + > > 2 files changed, 13 insertions(+) > > Can you please include at least one user of this call back in your patch > series? Thanks for the comments on this series. I'll address them in the next round. The user for this callback is not yet ready to be posted. I'll try and move this patch to the end of the series so that the series can be applied without this one. Thanks 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 Mon, Apr 06, 2015 at 07:23:08PM -0700, Eduardo Valentin wrote: > On Thu, Mar 26, 2015 at 04:53:52PM +0100, Sascha Hauer wrote: > > Inline update_temperature into its only caller to make the code > > more readable. > > I am not sure I understand how this is improving readability, can you > please elaborate? It improves readability because the reader does not have to lookup the definition of update_temperature but can instead just read through. The function name "update_temperature" does not give new information either, so I thought inlining the code is a good idea. Anyway, I can skip this patch if you want. 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 Mon, Apr 06, 2015 at 07:23:08PM -0700, Eduardo Valentin wrote: > On Thu, Mar 26, 2015 at 04:53:52PM +0100, Sascha Hauer wrote: > > Inline update_temperature into its only caller to make the code > > more readable. > > I am not sure I understand how this is improving readability, can you > please elaborate? It improves readability because the reader does not have to lookup the definition of update_temperature but can instead just read through. The function name "update_temperature" does not give new information either, so I thought inlining the code is a good idea. Anyway, I can skip this patch if you want. 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 Mon, Apr 06, 2015 at 07:23:08PM -0700, Eduardo Valentin wrote: > On Thu, Mar 26, 2015 at 04:53:52PM +0100, Sascha Hauer wrote: > > Inline update_temperature into its only caller to make the code > > more readable. > > I am not sure I understand how this is improving readability, can you > please elaborate? It improves readability because the reader does not have to lookup the definition of update_temperature but can instead just read through. The function name "update_temperature" does not give new information either, so I thought inlining the code is a good idea. Anyway, I can skip this patch if you want. 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 Mon, Apr 06, 2015 at 08:19:52PM -0700, Eduardo Valentin wrote: > On Thu, Mar 26, 2015 at 04:53:53PM +0100, Sascha Hauer wrote: > > The .get_trend callback in struct thermal_zone_device_ops has the prototype: > > > > int (*get_trend) (struct thermal_zone_device *, int, > > enum thermal_trend *); > > > > whereas the .get_trend callback in struct thermal_zone_of_device_ops has: > > > > int (*get_trend)(void *, long *); > > > > Streamline both prototypes and add the trip argument to the OF callback > > aswell and use enum thermal_trend * instead of an integer pointer. > > > > While the OF prototype may be the better one, this should be decided at > > framework level and not on OF level. > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > drivers/thermal/of-thermal.c | 11 +--------- > > drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 25 +++++++--------------- > > include/linux/thermal.h | 2 +- > > 3 files changed, 10 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > > index 668fb1b..b39e22f 100644 > > --- a/drivers/thermal/of-thermal.c > > +++ b/drivers/thermal/of-thermal.c > > @@ -187,24 +187,15 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, > > enum thermal_trend *trend) > > { > > struct __thermal_zone *data = tz->devdata; > > - long dev_trend; > > int r; > > > > if (!data->ops->get_trend) > > return -EINVAL; > > > > - r = data->ops->get_trend(data->sensor_data, &dev_trend); > > + r = data->ops->get_trend(data->sensor_data, trip, trend); > > if (r) > > return r; > > > > - /* TODO: These intervals might have some thresholds, but in core code */ > > - if (dev_trend > 0) > > - *trend = THERMAL_TREND_RAISING; > > - else if (dev_trend < 0) > > - *trend = THERMAL_TREND_DROPPING; > > - else > > - *trend = THERMAL_TREND_STABLE; > > - > > return 0; > > } > > > > diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > > index a38c175..7f8e5f3 100644 > > --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > > +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > > @@ -238,7 +238,7 @@ static int ti_thermal_get_trip_temp(struct thermal_zone_device *thermal, > > return 0; > > } > > > > -static int __ti_thermal_get_trend(void *p, long *trend) > > +static int __ti_thermal_get_trend(void *p, int trip, enum thermal_trend *trend) > > { > > struct ti_thermal_data *data = p; > > struct ti_bandgap *bgp; > > @@ -251,22 +251,6 @@ static int __ti_thermal_get_trend(void *p, long *trend) > > if (ret) > > return ret; > > > > - *trend = tr; > > - > > - return 0; > > -} > > - > > -/* Get the temperature trend callback functions for thermal zone */ > > -static int ti_thermal_get_trend(struct thermal_zone_device *thermal, > > - int trip, enum thermal_trend *trend) > > -{ > > - int ret; > > - long tr; > > - > > - ret = __ti_thermal_get_trend(thermal->devdata, &tr); > > - if (ret) > > - return ret; > > - > > if (tr > 0) > > *trend = THERMAL_TREND_RAISING; > > else if (tr < 0) > > @@ -277,6 +261,13 @@ static int ti_thermal_get_trend(struct thermal_zone_device *thermal, > > return 0; > > } > > > > +/* Get the temperature trend callback functions for thermal zone */ > > +static int ti_thermal_get_trend(struct thermal_zone_device *thermal, > > + int trip, enum thermal_trend *trend) > > +{ > > + return __ti_thermal_get_trend(thermal->devdata, trip, trend); > > +} > > + > > /* Get critical temperature callback functions for thermal zone */ > > static int ti_thermal_get_crit_temp(struct thermal_zone_device *thermal, > > unsigned long *temp) > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > > index db6c12b..ba2e29a 100644 > > --- a/include/linux/thermal.h > > +++ b/include/linux/thermal.h > > @@ -273,7 +273,7 @@ struct thermal_genl_event { > > */ > > struct thermal_zone_of_device_ops { > > int (*get_temp)(void *, long *); > > - int (*get_trend)(void *, long *); > > + int (*get_trend)(void *, int trend, enum thermal_trend *); > > Could you please keep the kernel doc entry up to date? What do you mean? The kernel doc entry for this function contains: * @get_trend: a pointer to a function that reads the sensor temperature trend. This is still valid with my patch, so I might not understand what I should keep up to date. That said, the argument name above should be 'int trip', not 'int trend'. I'll change that. 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 Mon, Apr 06, 2015 at 08:19:52PM -0700, Eduardo Valentin wrote: > On Thu, Mar 26, 2015 at 04:53:53PM +0100, Sascha Hauer wrote: > > The .get_trend callback in struct thermal_zone_device_ops has the prototype: > > > > int (*get_trend) (struct thermal_zone_device *, int, > > enum thermal_trend *); > > > > whereas the .get_trend callback in struct thermal_zone_of_device_ops has: > > > > int (*get_trend)(void *, long *); > > > > Streamline both prototypes and add the trip argument to the OF callback > > aswell and use enum thermal_trend * instead of an integer pointer. > > > > While the OF prototype may be the better one, this should be decided at > > framework level and not on OF level. > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > drivers/thermal/of-thermal.c | 11 +--------- > > drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 25 +++++++--------------- > > include/linux/thermal.h | 2 +- > > 3 files changed, 10 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > > index 668fb1b..b39e22f 100644 > > --- a/drivers/thermal/of-thermal.c > > +++ b/drivers/thermal/of-thermal.c > > @@ -187,24 +187,15 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, > > enum thermal_trend *trend) > > { > > struct __thermal_zone *data = tz->devdata; > > - long dev_trend; > > int r; > > > > if (!data->ops->get_trend) > > return -EINVAL; > > > > - r = data->ops->get_trend(data->sensor_data, &dev_trend); > > + r = data->ops->get_trend(data->sensor_data, trip, trend); > > if (r) > > return r; > > > > - /* TODO: These intervals might have some thresholds, but in core code */ > > - if (dev_trend > 0) > > - *trend = THERMAL_TREND_RAISING; > > - else if (dev_trend < 0) > > - *trend = THERMAL_TREND_DROPPING; > > - else > > - *trend = THERMAL_TREND_STABLE; > > - > > return 0; > > } > > > > diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > > index a38c175..7f8e5f3 100644 > > --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > > +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > > @@ -238,7 +238,7 @@ static int ti_thermal_get_trip_temp(struct thermal_zone_device *thermal, > > return 0; > > } > > > > -static int __ti_thermal_get_trend(void *p, long *trend) > > +static int __ti_thermal_get_trend(void *p, int trip, enum thermal_trend *trend) > > { > > struct ti_thermal_data *data = p; > > struct ti_bandgap *bgp; > > @@ -251,22 +251,6 @@ static int __ti_thermal_get_trend(void *p, long *trend) > > if (ret) > > return ret; > > > > - *trend = tr; > > - > > - return 0; > > -} > > - > > -/* Get the temperature trend callback functions for thermal zone */ > > -static int ti_thermal_get_trend(struct thermal_zone_device *thermal, > > - int trip, enum thermal_trend *trend) > > -{ > > - int ret; > > - long tr; > > - > > - ret = __ti_thermal_get_trend(thermal->devdata, &tr); > > - if (ret) > > - return ret; > > - > > if (tr > 0) > > *trend = THERMAL_TREND_RAISING; > > else if (tr < 0) > > @@ -277,6 +261,13 @@ static int ti_thermal_get_trend(struct thermal_zone_device *thermal, > > return 0; > > } > > > > +/* Get the temperature trend callback functions for thermal zone */ > > +static int ti_thermal_get_trend(struct thermal_zone_device *thermal, > > + int trip, enum thermal_trend *trend) > > +{ > > + return __ti_thermal_get_trend(thermal->devdata, trip, trend); > > +} > > + > > /* Get critical temperature callback functions for thermal zone */ > > static int ti_thermal_get_crit_temp(struct thermal_zone_device *thermal, > > unsigned long *temp) > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > > index db6c12b..ba2e29a 100644 > > --- a/include/linux/thermal.h > > +++ b/include/linux/thermal.h > > @@ -273,7 +273,7 @@ struct thermal_genl_event { > > */ > > struct thermal_zone_of_device_ops { > > int (*get_temp)(void *, long *); > > - int (*get_trend)(void *, long *); > > + int (*get_trend)(void *, int trend, enum thermal_trend *); > > Could you please keep the kernel doc entry up to date? What do you mean? The kernel doc entry for this function contains: * @get_trend: a pointer to a function that reads the sensor temperature trend. This is still valid with my patch, so I might not understand what I should keep up to date. That said, the argument name above should be 'int trip', not 'int trend'. I'll change that. 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 Mon, Apr 06, 2015 at 08:25:23PM -0700, Eduardo Valentin wrote: > On Thu, Mar 26, 2015 at 04:53:56PM +0100, Sascha Hauer wrote: > > Now that the thermal core treats -ENOSYS like the callbacks were > > not present at all we no longer have to overwrite the ops during > > runtime but instead can always set them and return -ENOSYS if no > > sensor is registered. > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > drivers/thermal/of-thermal.c | 18 +++++++++++------- > > 1 file changed, 11 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > > index b39e22f..df14fdd 100644 > > --- a/drivers/thermal/of-thermal.c > > +++ b/drivers/thermal/of-thermal.c > > @@ -91,7 +91,7 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz, > > { > > struct __thermal_zone *data = tz->devdata; > > > > - if (!data->ops->get_temp) > > + if (!data->ops) > > return -EINVAL; > > > > return data->ops->get_temp(data->sensor_data, temp); > > @@ -178,7 +178,7 @@ static int of_thermal_set_emul_temp(struct thermal_zone_device *tz, > > struct __thermal_zone *data = tz->devdata; > > > > if (!data->ops || !data->ops->set_emul_temp) > > - return -EINVAL; > > + return -ENOSYS; > > > > return data->ops->set_emul_temp(data->sensor_data, temp); > > } > > @@ -189,8 +189,8 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, > > struct __thermal_zone *data = tz->devdata; > > int r; > > > > - if (!data->ops->get_trend) > > - return -EINVAL; > > + if (!data->ops || !data->ops->get_trend) > > + return -ENOSYS; > > > > r = data->ops->get_trend(data->sensor_data, trip, trend); > > if (r) > > @@ -366,6 +366,10 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz, > > } > > > > static struct thermal_zone_device_ops of_thermal_ops = { > > + .get_temp = of_thermal_get_temp, > > + .get_trend = of_thermal_get_trend, > > + .set_emul_temp = of_thermal_set_emul_temp, > > + > > .get_mode = of_thermal_get_mode, > > .set_mode = of_thermal_set_mode, > > > > @@ -399,13 +403,13 @@ thermal_zone_of_add_sensor(struct device_node *zone, > > if (!ops) > > return ERR_PTR(-EINVAL); > > > > + if (!ops->get_temp) > > + return ERR_PTR(-EINVAL); > > + > > mutex_lock(&tzd->lock); > > tz->ops = ops; > > tz->sensor_data = data; > > > > - tzd->ops->get_temp = of_thermal_get_temp; > > - tzd->ops->get_trend = of_thermal_get_trend; > > - tzd->ops->set_emul_temp = of_thermal_set_emul_temp; > > You may want to update the thermal_zone_of_sensor_unregister too. Ups, indeed. That hunk accidently went into a later patch. 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 Mon, Apr 06, 2015 at 08:25:23PM -0700, Eduardo Valentin wrote: > On Thu, Mar 26, 2015 at 04:53:56PM +0100, Sascha Hauer wrote: > > Now that the thermal core treats -ENOSYS like the callbacks were > > not present at all we no longer have to overwrite the ops during > > runtime but instead can always set them and return -ENOSYS if no > > sensor is registered. > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > drivers/thermal/of-thermal.c | 18 +++++++++++------- > > 1 file changed, 11 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > > index b39e22f..df14fdd 100644 > > --- a/drivers/thermal/of-thermal.c > > +++ b/drivers/thermal/of-thermal.c > > @@ -91,7 +91,7 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz, > > { > > struct __thermal_zone *data = tz->devdata; > > > > - if (!data->ops->get_temp) > > + if (!data->ops) > > return -EINVAL; > > > > return data->ops->get_temp(data->sensor_data, temp); > > @@ -178,7 +178,7 @@ static int of_thermal_set_emul_temp(struct thermal_zone_device *tz, > > struct __thermal_zone *data = tz->devdata; > > > > if (!data->ops || !data->ops->set_emul_temp) > > - return -EINVAL; > > + return -ENOSYS; > > > > return data->ops->set_emul_temp(data->sensor_data, temp); > > } > > @@ -189,8 +189,8 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, > > struct __thermal_zone *data = tz->devdata; > > int r; > > > > - if (!data->ops->get_trend) > > - return -EINVAL; > > + if (!data->ops || !data->ops->get_trend) > > + return -ENOSYS; > > > > r = data->ops->get_trend(data->sensor_data, trip, trend); > > if (r) > > @@ -366,6 +366,10 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz, > > } > > > > static struct thermal_zone_device_ops of_thermal_ops = { > > + .get_temp = of_thermal_get_temp, > > + .get_trend = of_thermal_get_trend, > > + .set_emul_temp = of_thermal_set_emul_temp, > > + > > .get_mode = of_thermal_get_mode, > > .set_mode = of_thermal_set_mode, > > > > @@ -399,13 +403,13 @@ thermal_zone_of_add_sensor(struct device_node *zone, > > if (!ops) > > return ERR_PTR(-EINVAL); > > > > + if (!ops->get_temp) > > + return ERR_PTR(-EINVAL); > > + > > mutex_lock(&tzd->lock); > > tz->ops = ops; > > tz->sensor_data = data; > > > > - tzd->ops->get_temp = of_thermal_get_temp; > > - tzd->ops->get_trend = of_thermal_get_trend; > > - tzd->ops->set_emul_temp = of_thermal_set_emul_temp; > > You may want to update the thermal_zone_of_sensor_unregister too. Ups, indeed. That hunk accidently went into a later patch. 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 Mon, Apr 13, 2015 at 08:30:18AM +0200, Sascha Hauer wrote: > On Mon, Apr 06, 2015 at 07:43:08PM -0700, Eduardo Valentin wrote: > > On Thu, Mar 26, 2015 at 04:54:00PM +0100, Sascha Hauer wrote: > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > > --- > > > drivers/thermal/of-thermal.c | 12 ++++++++++++ > > > include/linux/thermal.h | 1 + > > > 2 files changed, 13 insertions(+) > > > > Can you please include at least one user of this call back in your patch > > series? > > Thanks for the comments on this series. I'll address them in the next > round. The user for this callback is not yet ready to be posted. I'll > try and move this patch to the end of the series so that the series can > be applied without this one. I'm actually interested in this patch, as I am developing a thermal sensor driver that has hardware-assisted temperature-trip interrupts. I'm testing out your patches now, and will probably end up using them on my local tree. With help of this (and a few other changes I need to make), my driver should be ready to post soon. So, I'll try to post my driver soon as an example, if you'd like. I'd also appreciate a CC on any future revisions, if you don't mind. I'm subscribed to the list, but it's not always easy to pick out the signal from the noise. Thanks, Brian P.S. Time for a mild complaint: I think it seems to be a bit of a plague in the thermal subsystem; there are API's, and even user-space ABI's (!!) that have no in-kernel users! e.g., generate_netlink_event() http://article.gmane.org/gmane.linux.power-management.general/58685
On Mon, Apr 13, 2015 at 08:30:18AM +0200, Sascha Hauer wrote: > On Mon, Apr 06, 2015 at 07:43:08PM -0700, Eduardo Valentin wrote: > > On Thu, Mar 26, 2015 at 04:54:00PM +0100, Sascha Hauer wrote: > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > > --- > > > drivers/thermal/of-thermal.c | 12 ++++++++++++ > > > include/linux/thermal.h | 1 + > > > 2 files changed, 13 insertions(+) > > > > Can you please include at least one user of this call back in your patch > > series? > > Thanks for the comments on this series. I'll address them in the next > round. The user for this callback is not yet ready to be posted. I'll > try and move this patch to the end of the series so that the series can > be applied without this one. I'm actually interested in this patch, as I am developing a thermal sensor driver that has hardware-assisted temperature-trip interrupts. I'm testing out your patches now, and will probably end up using them on my local tree. With help of this (and a few other changes I need to make), my driver should be ready to post soon. So, I'll try to post my driver soon as an example, if you'd like. I'd also appreciate a CC on any future revisions, if you don't mind. I'm subscribed to the list, but it's not always easy to pick out the signal from the noise. Thanks, Brian P.S. Time for a mild complaint: I think it seems to be a bit of a plague in the thermal subsystem; there are API's, and even user-space ABI's (!!) that have no in-kernel users! e.g., generate_netlink_event() http://article.gmane.org/gmane.linux.power-management.general/58685
Hi Sascha, On Thu, Mar 26, 2015 at 04:53:59PM +0100, Sascha Hauer wrote: > This adds support for hardware-tracked trip points to the device tree > thermal sensor framework. > > The framework supports an arbitrary number of trip points. Whenever > the current temperature is updated, the trip points immediately > below and above the current temperature are found. A .set_trips > callback is then called with the temperatures. If there is no trip > point above or below the current temperature, the passed trip > temperature will be ULONG_MAX or 0 respectively. In this callback, > the driver should program the hardware such that it is notified > when either of these trip points are triggered. When a trip point > is triggered, the driver should call `thermal_zone_device_update' > for the respective thermal zone. This will cause the trip points > to be updated again. > > If .set_trips is not implemented, the framework behaves as before. > > This patch is based on an earlier version from Mikko Perttunen > <mikko.perttunen@kapsi.fi> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/thermal_core.c | 41 +++++++++++++++++++++++++++++++++++++++++ > include/linux/thermal.h | 3 +++ > 2 files changed, 44 insertions(+) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index dcdf45e..7138f8f 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -434,6 +434,45 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) > } > EXPORT_SYMBOL_GPL(thermal_zone_get_temp); > > +static void thermal_zone_set_trips(struct thermal_zone_device *tz) > +{ > + unsigned long low = 0; > + unsigned long high = ULONG_MAX; > + unsigned long trip_temp, hysteresis; > + unsigned long temp = tz->temperature; > + int i; > + > + if (!tz->ops->set_trips) > + return; > + > + /* No need to change trip points */ > + if (temp > tz->prev_low_trip && temp < tz->prev_high_trip) > + return; I see that you don't actually initialize prev_{low,high}_trip during device initialization, so they both start out as zero. I suppose that works here, mostly by coincidence, because it means we'll still always recalculate trip points the first time, but might it make sense to explicitly initialize them to: low = ULONG_MAX high = 0 at init time? (Or, if these change to integer: INT_MAX and -INT_MAX?) Thanks, Brian > + > + for (i = 0; i < tz->trips; i++) { > + unsigned long trip_low; > + > + tz->ops->get_trip_temp(tz, i, &trip_temp); > + tz->ops->get_trip_hyst(tz, i, &hysteresis); > + > + trip_low = trip_temp - hysteresis; > + > + if (trip_low < temp && trip_low > low) > + low = trip_low; > + > + if (trip_temp > temp && trip_temp < high) > + high = trip_temp; > + } > + > + tz->prev_low_trip = low; > + tz->prev_high_trip = high; > + > + dev_dbg(&tz->device, "new temperature boundaries: %lu < x < %lu\n", > + low, high); > + > + tz->ops->set_trips(tz, low, high); > +} > + > void thermal_zone_device_update(struct thermal_zone_device *tz) > { > int count; > @@ -460,6 +499,8 @@ void thermal_zone_device_update(struct thermal_zone_device *tz) > dev_dbg(&tz->device, "last_temperature=%lu, current_temperature=%lu\n", > tz->last_temperature, tz->temperature); > > + thermal_zone_set_trips(tz); > + > for (count = 0; count < tz->trips; count++) > handle_thermal_trip(tz, count); > } > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index ac2897c..b870702 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -87,6 +87,7 @@ struct thermal_zone_device_ops { > int (*unbind) (struct thermal_zone_device *, > struct thermal_cooling_device *); > int (*get_temp) (struct thermal_zone_device *, unsigned long *); > + int (*set_trips) (struct thermal_zone_device *, unsigned long, unsigned long); > int (*get_mode) (struct thermal_zone_device *, > enum thermal_device_mode *); > int (*set_mode) (struct thermal_zone_device *, > @@ -183,6 +184,8 @@ struct thermal_zone_device { > unsigned long temperature; > unsigned long last_temperature; > unsigned long emul_temperature; > + unsigned long prev_low_trip; > + unsigned long prev_high_trip; > int passive; > unsigned int forced_passive; > const struct thermal_zone_device_ops *ops; > -- > 2.1.4 > > -- > 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
Hi Sascha, On Thu, Mar 26, 2015 at 04:53:59PM +0100, Sascha Hauer wrote: > This adds support for hardware-tracked trip points to the device tree > thermal sensor framework. > > The framework supports an arbitrary number of trip points. Whenever > the current temperature is updated, the trip points immediately > below and above the current temperature are found. A .set_trips > callback is then called with the temperatures. If there is no trip > point above or below the current temperature, the passed trip > temperature will be ULONG_MAX or 0 respectively. In this callback, > the driver should program the hardware such that it is notified > when either of these trip points are triggered. When a trip point > is triggered, the driver should call `thermal_zone_device_update' > for the respective thermal zone. This will cause the trip points > to be updated again. > > If .set_trips is not implemented, the framework behaves as before. > > This patch is based on an earlier version from Mikko Perttunen > <mikko.perttunen@kapsi.fi> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/thermal/thermal_core.c | 41 +++++++++++++++++++++++++++++++++++++++++ > include/linux/thermal.h | 3 +++ > 2 files changed, 44 insertions(+) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index dcdf45e..7138f8f 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -434,6 +434,45 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) > } > EXPORT_SYMBOL_GPL(thermal_zone_get_temp); > > +static void thermal_zone_set_trips(struct thermal_zone_device *tz) > +{ > + unsigned long low = 0; > + unsigned long high = ULONG_MAX; > + unsigned long trip_temp, hysteresis; > + unsigned long temp = tz->temperature; > + int i; > + > + if (!tz->ops->set_trips) > + return; > + > + /* No need to change trip points */ > + if (temp > tz->prev_low_trip && temp < tz->prev_high_trip) > + return; I see that you don't actually initialize prev_{low,high}_trip during device initialization, so they both start out as zero. I suppose that works here, mostly by coincidence, because it means we'll still always recalculate trip points the first time, but might it make sense to explicitly initialize them to: low = ULONG_MAX high = 0 at init time? (Or, if these change to integer: INT_MAX and -INT_MAX?) Thanks, Brian > + > + for (i = 0; i < tz->trips; i++) { > + unsigned long trip_low; > + > + tz->ops->get_trip_temp(tz, i, &trip_temp); > + tz->ops->get_trip_hyst(tz, i, &hysteresis); > + > + trip_low = trip_temp - hysteresis; > + > + if (trip_low < temp && trip_low > low) > + low = trip_low; > + > + if (trip_temp > temp && trip_temp < high) > + high = trip_temp; > + } > + > + tz->prev_low_trip = low; > + tz->prev_high_trip = high; > + > + dev_dbg(&tz->device, "new temperature boundaries: %lu < x < %lu\n", > + low, high); > + > + tz->ops->set_trips(tz, low, high); > +} > + > void thermal_zone_device_update(struct thermal_zone_device *tz) > { > int count; > @@ -460,6 +499,8 @@ void thermal_zone_device_update(struct thermal_zone_device *tz) > dev_dbg(&tz->device, "last_temperature=%lu, current_temperature=%lu\n", > tz->last_temperature, tz->temperature); > > + thermal_zone_set_trips(tz); > + > for (count = 0; count < tz->trips; count++) > handle_thermal_trip(tz, count); > } > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index ac2897c..b870702 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -87,6 +87,7 @@ struct thermal_zone_device_ops { > int (*unbind) (struct thermal_zone_device *, > struct thermal_cooling_device *); > int (*get_temp) (struct thermal_zone_device *, unsigned long *); > + int (*set_trips) (struct thermal_zone_device *, unsigned long, unsigned long); > int (*get_mode) (struct thermal_zone_device *, > enum thermal_device_mode *); > int (*set_mode) (struct thermal_zone_device *, > @@ -183,6 +184,8 @@ struct thermal_zone_device { > unsigned long temperature; > unsigned long last_temperature; > unsigned long emul_temperature; > + unsigned long prev_low_trip; > + unsigned long prev_high_trip; > int passive; > unsigned int forced_passive; > const struct thermal_zone_device_ops *ops; > -- > 2.1.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Brian, On Wed, Apr 15, 2015 at 10:59:36AM -0700, Brian Norris wrote: > On Mon, Apr 13, 2015 at 08:30:18AM +0200, Sascha Hauer wrote: > > On Mon, Apr 06, 2015 at 07:43:08PM -0700, Eduardo Valentin wrote: > > > On Thu, Mar 26, 2015 at 04:54:00PM +0100, Sascha Hauer wrote: > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > > > --- > > > > drivers/thermal/of-thermal.c | 12 ++++++++++++ > > > > include/linux/thermal.h | 1 + > > > > 2 files changed, 13 insertions(+) > > > > > > Can you please include at least one user of this call back in your patch > > > series? > > > > Thanks for the comments on this series. I'll address them in the next > > round. The user for this callback is not yet ready to be posted. I'll > > try and move this patch to the end of the series so that the series can > > be applied without this one. > > I'm actually interested in this patch, as I am developing a thermal > sensor driver that has hardware-assisted temperature-trip interrupts. > I'm testing out your patches now, and will probably end up using them on > my local tree. With help of this (and a few other changes I need to > make), my driver should be ready to post soon. > > So, I'll try to post my driver soon as an example, if you'd like. I'd > also appreciate a CC on any future revisions, if you don't mind. I'm > subscribed to the list, but it's not always easy to pick out the signal > from the noise. Sure, I'll add you to Cc next round. I'm halfway through updating this series according to the comments I received, but got distracted. I hope to send an update next week. 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 |
Hi Brian, On Wed, Apr 15, 2015 at 10:59:36AM -0700, Brian Norris wrote: > On Mon, Apr 13, 2015 at 08:30:18AM +0200, Sascha Hauer wrote: > > On Mon, Apr 06, 2015 at 07:43:08PM -0700, Eduardo Valentin wrote: > > > On Thu, Mar 26, 2015 at 04:54:00PM +0100, Sascha Hauer wrote: > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > > > --- > > > > drivers/thermal/of-thermal.c | 12 ++++++++++++ > > > > include/linux/thermal.h | 1 + > > > > 2 files changed, 13 insertions(+) > > > > > > Can you please include at least one user of this call back in your patch > > > series? > > > > Thanks for the comments on this series. I'll address them in the next > > round. The user for this callback is not yet ready to be posted. I'll > > try and move this patch to the end of the series so that the series can > > be applied without this one. > > I'm actually interested in this patch, as I am developing a thermal > sensor driver that has hardware-assisted temperature-trip interrupts. > I'm testing out your patches now, and will probably end up using them on > my local tree. With help of this (and a few other changes I need to > make), my driver should be ready to post soon. > > So, I'll try to post my driver soon as an example, if you'd like. I'd > also appreciate a CC on any future revisions, if you don't mind. I'm > subscribed to the list, but it's not always easy to pick out the signal > from the noise. Sure, I'll add you to Cc next round. I'm halfway through updating this series according to the comments I received, but got distracted. I hope to send an update next week. 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 , ? -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
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 , ? -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
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 -- 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 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 -- 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 |