This series contains some cleanup patches for the thermal framework.
Some patches are already acked by Eduardo but not yet included in any
tree. Please review/apply.
Sascha
----------------------------------------------------------------
Sascha Hauer (4):
thermal: trivial: fix typo in comment
thermal: remove unnecessary call to thermal_zone_device_set_polling
thermal: Use IS_ENABLED instead of #ifdef
thermal: Add comment explaining test for critical temperature
drivers/thermal/thermal_core.c | 55 +++++++++++++++++++-----------------------
1 file changed, 25 insertions(+), 30 deletions(-)
Signed-off-by: Sascha Hauer <[email protected]>
Acked-by: Eduardo Valentin <[email protected]>
---
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 04659bf..34ce9e4 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -465,7 +465,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 <[email protected]>
Acked-by: Eduardo Valentin <[email protected]>
---
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 34ce9e4..aa0a661 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1848,9 +1848,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
Use IS_ENABLED(CONFIG_THERMAL_EMULATION) to make the code more readable
and to get rid of the addtional #ifdef around the variable definitions
in thermal_zone_get_temp().
Signed-off-by: Sascha Hauer <[email protected]>
---
drivers/thermal/thermal_core.c | 45 ++++++++++++++++++------------------------
1 file changed, 19 insertions(+), 26 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index aa0a661..67296da 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -477,11 +477,9 @@ 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;
@@ -489,25 +487,21 @@ 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 (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz->emul_temperature) {
+ 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 (*temp < crit_temp)
- *temp = tz->emul_temperature;
-skip_emul:
-#endif
+ if (!ret && *temp < crit_temp)
+ *temp = tz->emul_temperature;
+ }
+
mutex_unlock(&tz->lock);
exit:
return ret;
@@ -847,7 +841,6 @@ policy_show(struct device *dev, struct device_attribute *devattr, char *buf)
return sprintf(buf, "%s\n", tz->governor->name);
}
-#ifdef CONFIG_THERMAL_EMULATION
static ssize_t
emul_temp_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
@@ -873,7 +866,6 @@ emul_temp_store(struct device *dev, struct device_attribute *attr,
return ret ? ret : count;
}
static DEVICE_ATTR(emul_temp, S_IWUSR, NULL, emul_temp_store);
-#endif/*CONFIG_THERMAL_EMULATION*/
static ssize_t
sustainable_power_show(struct device *dev, struct device_attribute *devattr,
@@ -1802,11 +1794,12 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
goto unregister;
}
-#ifdef CONFIG_THERMAL_EMULATION
- result = device_create_file(&tz->device, &dev_attr_emul_temp);
- if (result)
- goto unregister;
-#endif
+ if (IS_ENABLED(CONFIG_THERMAL_EMULATION)) {
+ result = device_create_file(&tz->device, &dev_attr_emul_temp);
+ if (result)
+ goto unregister;
+ }
+
/* Create policy attribute */
result = device_create_file(&tz->device, &dev_attr_policy);
if (result)
--
2.1.4
The code testing if a temperature should be emulated or not is
not obvious. Add a comment explaining why this test is done.
Signed-off-by: Sascha Hauer <[email protected]>
Reviewed-by: Mikko Perttunen <[email protected]>
---
drivers/thermal/thermal_core.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 67296da..956684e 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -498,6 +498,11 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp)
}
}
+ /*
+ * Only allow emulating a temperature when the real temperature
+ * is below the critical temperature so that the emulation code
+ * cannot hide critical conditions.
+ */
if (!ret && *temp < crit_temp)
*temp = tz->emul_temperature;
}
--
2.1.4
Hi Sascha,
> Use IS_ENABLED(CONFIG_THERMAL_EMULATION) to make the code more
> readable and to get rid of the addtional #ifdef around the variable
> definitions in thermal_zone_get_temp().
Reviewed-by: Lukasz Majewski <[email protected]>
>
> Signed-off-by: Sascha Hauer <[email protected]>
> ---
> drivers/thermal/thermal_core.c | 45
> ++++++++++++++++++------------------------ 1 file changed, 19
> insertions(+), 26 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c
> b/drivers/thermal/thermal_core.c index aa0a661..67296da 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -477,11 +477,9 @@ 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;
> @@ -489,25 +487,21 @@ 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 (IS_ENABLED(CONFIG_THERMAL_EMULATION) &&
> tz->emul_temperature) {
> + 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 (*temp < crit_temp)
> - *temp = tz->emul_temperature;
> -skip_emul:
> -#endif
> + if (!ret && *temp < crit_temp)
> + *temp = tz->emul_temperature;
> + }
> +
> mutex_unlock(&tz->lock);
> exit:
> return ret;
> @@ -847,7 +841,6 @@ policy_show(struct device *dev, struct
> device_attribute *devattr, char *buf) return sprintf(buf, "%s\n",
> tz->governor->name); }
>
> -#ifdef CONFIG_THERMAL_EMULATION
> static ssize_t
> emul_temp_store(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> @@ -873,7 +866,6 @@ emul_temp_store(struct device *dev, struct
> device_attribute *attr, return ret ? ret : count;
> }
> static DEVICE_ATTR(emul_temp, S_IWUSR, NULL, emul_temp_store);
> -#endif/*CONFIG_THERMAL_EMULATION*/
>
> static ssize_t
> sustainable_power_show(struct device *dev, struct device_attribute
> *devattr, @@ -1802,11 +1794,12 @@ struct thermal_zone_device
> *thermal_zone_device_register(const char *type, goto unregister;
> }
>
> -#ifdef CONFIG_THERMAL_EMULATION
> - result = device_create_file(&tz->device,
> &dev_attr_emul_temp);
> - if (result)
> - goto unregister;
> -#endif
> + if (IS_ENABLED(CONFIG_THERMAL_EMULATION)) {
> + result = device_create_file(&tz->device,
> &dev_attr_emul_temp);
> + if (result)
> + goto unregister;
> + }
> +
> /* Create policy attribute */
> result = device_create_file(&tz->device, &dev_attr_policy);
> if (result)
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
Hi Rui,
Any input to this series? Could you apply it?
Sascha
On Mon, Jul 06, 2015 at 09:46:14AM +0200, Sascha Hauer wrote:
> Signed-off-by: Sascha Hauer <[email protected]>
> Acked-by: Eduardo Valentin <[email protected]>
> ---
> 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 04659bf..34ce9e4 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -465,7 +465,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
>
>
--
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 |