This work is the pre-requisite of handling correctly when the trip
point are crossed. For that we need to rework how the trip points are
declared and assigned to a thermal zone.
Even if it appears to be a common sense to have the trip points being
ordered, this no guarantee neither documentation telling that is the
case.
One solution could have been to create an ordered array of trips built
when registering the thermal zone by calling the different get_trip*
ops. However those ops receive a thermal zone pointer which is not
known as it is in the process of creating it.
This cyclic dependency shows we have to rework how we manage the trip
points.
Actually, all the trip points definition can be common to the backend
sensor drivers and we can factor out the thermal trip structure in all
of them.
Then, as we register the thermal trips array, they will be available
in the thermal zone structure and a core function can return the trip
given its id.
The get_trip_* ops won't be needed anymore and could be removed. The
resulting code will be another step forward to a self encapsulated
generic thermal framework.
Most of the drivers can be converted more or less easily. This series
does a first round with most of the drivers. Some remain and will be
converted but with a smaller set of changes as the conversion is a bit
more complex.
Changelog:
v4:
- Remove extra lines on exynos changes as reported by Krzysztof Kozlowski
- Collected tags
v3:
- Reorg the series to be git-bisect safe
- Added the set_trip generic function
- Added the get_crit_temp generic function
- Removed more dead code in the thermal-of
- Fixed the exynos changelog
- Fixed the error check for the exynos drivers
- Collected tags
v2:
- Added missing EXPORT_SYMBOL_GPL() for thermal_zone_get_trip()
- Removed tab whitespace in the acerhdf driver
- Collected tags
Cc: Raju Rangoju <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: Peter Kaestle <[email protected]>
Cc: Hans de Goede <[email protected]>
Cc: Mark Gross <[email protected]>
Cc: Miquel Raynal <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: Amit Kucheria <[email protected]>
Cc: Zhang Rui <[email protected]>
Cc: Nicolas Saenz Julienne <[email protected]>
Cc: Broadcom Kernel Team <[email protected]>
Cc: Florian Fainelli <[email protected]>
Cc: Ray Jui <[email protected]>
Cc: Scott Branden <[email protected]>
Cc: Support Opensource <[email protected]>
Cc: Lukasz Luba <[email protected]>
Cc: Shawn Guo <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: Pengutronix Kernel Team <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: NXP Linux Team <[email protected]>
Cc: Thara Gopinath <[email protected]>
Cc: Andy Gross <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: "Niklas Söderlund" <[email protected]>
Cc: Bartlomiej Zolnierkiewicz <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Alim Akhtar <[email protected]>
Cc: Thierry Reding <[email protected]>
Cc: Jonathan Hunter <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Keerthy <[email protected]>
Cc: Kunihiko Hayashi <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Antoine Tenart <[email protected]>
Cc: Srinivas Pandruvada <[email protected]>
Cc: Dmitry Osipenko <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Daniel Lezcano (30):
thermal/core: Add a generic thermal_zone_get_trip() function
thermal/sysfs: Do not make get_trip_hyst optional
thermal/core: Add a generic thermal_zone_set_trip() function
thermal/core: Add a generic thermal_zone_get_crit_temp() function
thermal/core/governors: Use thermal_zone_get_trip() instead of ops
functions
thermal/of: Use generic thermal_zone_get_trip() function
thermal/of: Remove unused functions
thermal/drivers/exynos: Use generic thermal_zone_get_trip() function
thermal/drivers/exynos: of_thermal_get_ntrips()
thermal/drivers/exynos: Replace of_thermal_is_trip_valid() by
thermal_zone_get_trip()
thermal/drivers/tegra: Use generic thermal_zone_get_trip() function
thermal/drivers/uniphier: Use generic thermal_zone_get_trip() function
thermal/drivers/hisi: Use generic thermal_zone_get_trip() function
thermal/drivers/qcom: Use generic thermal_zone_get_trip() function
thermal/drivers/armada: Use generic thermal_zone_get_trip() function
thermal/drivers/rcar_gen3: Use the generic function to get the number
of trips
thermal/of: Remove of_thermal_get_ntrips()
thermal/of: Remove of_thermal_is_trip_valid()
thermal/of: Remove of_thermal_set_trip_hyst()
thermal/of: Remove of_thermal_get_crit_temp()
thermal/drivers/st: Use generic trip points
thermal/drivers/imx: Use generic thermal_zone_get_trip() function
thermal/drivers/rcar: Use generic thermal_zone_get_trip() function
thermal/drivers/broadcom: Use generic thermal_zone_get_trip() function
thermal/drivers/da9062: Use generic thermal_zone_get_trip() function
thermal/drivers/ti: Remove unused macros ti_thermal_get_trip_value() /
ti_thermal_trip_is_valid()
thermal/drivers/acerhdf: Use generic thermal_zone_get_trip() function
thermal/drivers/cxgb4: Use generic thermal_zone_get_trip() function
thermal/intel/int340x: Replace parameter to simplify
thermal/drivers/intel: Use generic thermal_zone_get_trip() function
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 2 -
.../ethernet/chelsio/cxgb4/cxgb4_thermal.c | 41 +----
drivers/platform/x86/acerhdf.c | 73 +++-----
drivers/thermal/armada_thermal.c | 39 ++---
drivers/thermal/broadcom/bcm2835_thermal.c | 8 +-
drivers/thermal/da9062-thermal.c | 52 +-----
drivers/thermal/gov_bang_bang.c | 23 +--
drivers/thermal/gov_fair_share.c | 18 +-
drivers/thermal/gov_power_allocator.c | 51 +++---
drivers/thermal/gov_step_wise.c | 22 ++-
drivers/thermal/hisi_thermal.c | 11 +-
drivers/thermal/imx_thermal.c | 72 +++-----
.../int340x_thermal/int340x_thermal_zone.c | 31 ++--
.../int340x_thermal/int340x_thermal_zone.h | 4 +-
.../processor_thermal_device.c | 10 +-
drivers/thermal/intel/x86_pkg_temp_thermal.c | 120 +++++++------
drivers/thermal/qcom/qcom-spmi-temp-alarm.c | 39 ++---
drivers/thermal/rcar_gen3_thermal.c | 2 +-
drivers/thermal/rcar_thermal.c | 49 +-----
drivers/thermal/samsung/exynos_tmu.c | 57 +++----
drivers/thermal/st/st_thermal.c | 47 +-----
drivers/thermal/tegra/soctherm.c | 33 ++--
drivers/thermal/tegra/tegra30-tsensor.c | 17 +-
drivers/thermal/thermal_core.c | 158 +++++++++++++++---
drivers/thermal/thermal_core.h | 22 ---
drivers/thermal/thermal_helpers.c | 28 ++--
drivers/thermal/thermal_netlink.c | 21 +--
drivers/thermal/thermal_of.c | 116 -------------
drivers/thermal/thermal_sysfs.c | 127 +++++---------
drivers/thermal/ti-soc-thermal/ti-thermal.h | 15 --
drivers/thermal/uniphier_thermal.c | 27 ++-
include/linux/thermal.h | 10 ++
32 files changed, 535 insertions(+), 810 deletions(-)
--
2.34.1
The thermal framework gives the possibility to register the trip
points with the thermal zone. When that is done, no get_trip_* ops are
needed and they can be removed.
The thermal OF code uses the thermal_zone_device_register_with_trips()
function. It builds the trips array and pass it to the register
function. That means the get_trip_* ops are duplicated with what does
already the core code.
Remove them.
Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/thermal_of.c | 36 ------------------------------------
1 file changed, 36 deletions(-)
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index d4b6335ace15..5cce83639085 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -71,39 +71,6 @@ of_thermal_get_trip_points(struct thermal_zone_device *tz)
}
EXPORT_SYMBOL_GPL(of_thermal_get_trip_points);
-static int of_thermal_get_trip_type(struct thermal_zone_device *tz, int trip,
- enum thermal_trip_type *type)
-{
- if (trip >= tz->num_trips || trip < 0)
- return -EDOM;
-
- *type = tz->trips[trip].type;
-
- return 0;
-}
-
-static int of_thermal_get_trip_temp(struct thermal_zone_device *tz, int trip,
- int *temp)
-{
- if (trip >= tz->num_trips || trip < 0)
- return -EDOM;
-
- *temp = tz->trips[trip].temperature;
-
- return 0;
-}
-
-static int of_thermal_get_trip_hyst(struct thermal_zone_device *tz, int trip,
- int *hyst)
-{
- if (trip >= tz->num_trips || trip < 0)
- return -EDOM;
-
- *hyst = tz->trips[trip].hysteresis;
-
- return 0;
-}
-
static int of_thermal_set_trip_hyst(struct thermal_zone_device *tz, int trip,
int hyst)
{
@@ -626,9 +593,6 @@ struct thermal_zone_device *thermal_of_zone_register(struct device_node *sensor,
goto out_kfree_trips;
}
- of_ops->get_trip_type = of_ops->get_trip_type ? : of_thermal_get_trip_type;
- of_ops->get_trip_temp = of_ops->get_trip_temp ? : of_thermal_get_trip_temp;
- of_ops->get_trip_hyst = of_ops->get_trip_hyst ? : of_thermal_get_trip_hyst;
of_ops->set_trip_hyst = of_ops->set_trip_hyst ? : of_thermal_set_trip_hyst;
of_ops->get_crit_temp = of_ops->get_crit_temp ? : of_thermal_get_crit_temp;
of_ops->bind = thermal_of_bind;
--
2.34.1
The thermal core is providing the generic thermal_zone_set_trip()
function which does exactly what the OF ops function is doing.
It is pointless to define our own version, just remove the ops and the
thermal_zone_set_trip() will take care of it.
Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/thermal_of.c | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index 4e54d62720dc..494e9c319541 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -19,18 +19,6 @@
#include "thermal_core.h"
-static int of_thermal_set_trip_hyst(struct thermal_zone_device *tz, int trip,
- int hyst)
-{
- if (trip >= tz->num_trips || trip < 0)
- return -EDOM;
-
- /* thermal framework should take care of data->mask & (1 << trip) */
- tz->trips[trip].hysteresis = hyst;
-
- return 0;
-}
-
static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
int *temp)
{
@@ -541,7 +529,6 @@ struct thermal_zone_device *thermal_of_zone_register(struct device_node *sensor,
goto out_kfree_trips;
}
- of_ops->set_trip_hyst = of_ops->set_trip_hyst ? : of_thermal_set_trip_hyst;
of_ops->get_crit_temp = of_ops->get_crit_temp ? : of_thermal_get_crit_temp;
of_ops->bind = thermal_of_bind;
of_ops->unbind = thermal_of_unbind;
--
2.34.1
The thermal core framework allows to get the number of thermal trips,
use it instead of visiting the thermal core structure internals.
Signed-off-by: Daniel Lezcano <[email protected]>
Reviewed-by: Niklas Söderlund <[email protected]>
---
drivers/thermal/rcar_gen3_thermal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index 4c1c6f89aa2f..4ef927437842 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -529,7 +529,7 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
if (ret)
goto error_unregister;
- ret = of_thermal_get_ntrips(tsc->zone);
+ ret = thermal_zone_get_num_trips(tsc->zone);
if (ret < 0)
goto error_unregister;
--
2.34.1
Remove the dead code: of_thermal_get_trip_points()
Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/thermal_core.h | 7 -------
drivers/thermal/thermal_of.c | 17 -----------------
2 files changed, 24 deletions(-)
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 1571917bd3c8..99a5d5281037 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -139,8 +139,6 @@ thermal_cooling_device_stats_update(struct thermal_cooling_device *cdev,
#ifdef CONFIG_THERMAL_OF
int of_thermal_get_ntrips(struct thermal_zone_device *);
bool of_thermal_is_trip_valid(struct thermal_zone_device *, int);
-const struct thermal_trip *
-of_thermal_get_trip_points(struct thermal_zone_device *);
#else
static inline int of_thermal_get_ntrips(struct thermal_zone_device *tz)
{
@@ -151,11 +149,6 @@ static inline bool of_thermal_is_trip_valid(struct thermal_zone_device *tz,
{
return false;
}
-static inline const struct thermal_trip *
-of_thermal_get_trip_points(struct thermal_zone_device *tz)
-{
- return NULL;
-}
#endif
int thermal_zone_device_is_enabled(struct thermal_zone_device *tz);
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index 5cce83639085..2f533fc94917 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -54,23 +54,6 @@ bool of_thermal_is_trip_valid(struct thermal_zone_device *tz, int trip)
}
EXPORT_SYMBOL_GPL(of_thermal_is_trip_valid);
-/**
- * of_thermal_get_trip_points - function to get access to a globally exported
- * trip points
- *
- * @tz: pointer to a thermal zone
- *
- * This function provides a pointer to trip points table
- *
- * Return: pointer to trip points table, NULL otherwise
- */
-const struct thermal_trip *
-of_thermal_get_trip_points(struct thermal_zone_device *tz)
-{
- return tz->trips;
-}
-EXPORT_SYMBOL_GPL(of_thermal_get_trip_points);
-
static int of_thermal_set_trip_hyst(struct thermal_zone_device *tz, int trip,
int hyst)
{
--
2.34.1
The thermal core framework allows to get the number of thermal trips,
use it instead of visiting the thermal core structure internals.
Signed-off-by: Daniel Lezcano <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
drivers/thermal/samsung/exynos_tmu.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 0e33d32a9d2e..91e6860b5ec4 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -260,6 +260,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
{
struct exynos_tmu_data *data = platform_get_drvdata(pdev);
struct thermal_zone_device *tzd = data->tzd;
+ int num_trips = thermal_zone_get_num_trips(tzd);
unsigned int status;
int ret = 0, temp;
@@ -271,12 +272,12 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
goto out;
}
- if (of_thermal_get_ntrips(tzd) > data->ntrip) {
+ if (num_trips > data->ntrip) {
dev_info(&pdev->dev,
"More trip points than supported by this TMU.\n");
dev_info(&pdev->dev,
"%d trip points should be configured in polling mode.\n",
- (of_thermal_get_ntrips(tzd) - data->ntrip));
+ num_trips - data->ntrip);
}
mutex_lock(&data->lock);
@@ -289,7 +290,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
ret = -EBUSY;
} else {
int i, ntrips =
- min_t(int, of_thermal_get_ntrips(tzd), data->ntrip);
+ min_t(int, num_trips, data->ntrip);
data->tmu_initialize(pdev);
--
2.34.1
There is no benefit with the of_thermal_is_trip_valid() function as it
does the check the thermal_zone_get_trip() is already doing for the
sake of getting the trip point.
As all the calls have been replaced by thermal_zone_get_trip(), there
is no more users of of_thermal_is_trip_valid().
Remove the function.
Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/thermal_core.h | 10 ----------
drivers/thermal/thermal_of.c | 19 -------------------
2 files changed, 29 deletions(-)
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 0139bc1e4f87..4d1af11a6eb4 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -136,16 +136,6 @@ thermal_cooling_device_stats_update(struct thermal_cooling_device *cdev,
#endif /* CONFIG_THERMAL_STATISTICS */
/* device tree support */
-#ifdef CONFIG_THERMAL_OF
-bool of_thermal_is_trip_valid(struct thermal_zone_device *, int);
-#else
-static inline bool of_thermal_is_trip_valid(struct thermal_zone_device *tz,
- int trip)
-{
- return false;
-}
-#endif
-
int thermal_zone_device_is_enabled(struct thermal_zone_device *tz);
#endif /* __THERMAL_CORE_H__ */
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index 89afa59c4915..4e54d62720dc 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -19,25 +19,6 @@
#include "thermal_core.h"
-/**
- * of_thermal_is_trip_valid - function to check if trip point is valid
- *
- * @tz: pointer to a thermal zone
- * @trip: trip point to evaluate
- *
- * This function is responsible for checking if passed trip point is valid
- *
- * Return: true if trip point is valid, false otherwise
- */
-bool of_thermal_is_trip_valid(struct thermal_zone_device *tz, int trip)
-{
- if (trip >= tz->num_trips || trip < 0)
- return false;
-
- return true;
-}
-EXPORT_SYMBOL_GPL(of_thermal_is_trip_valid);
-
static int of_thermal_set_trip_hyst(struct thermal_zone_device *tz, int trip,
int hyst)
{
--
2.34.1
In the process of replacing the get_trip_* ops by the generic trip
points, the current code has an 'override' property to add another
indirection to a different ops.
Rework this approach to prevent this indirection and make the code
ready for the generic trip points conversion.
Actually the get_temp() is different regarding the platform, so it is
pointless to add a new set of ops but just create dynamically the ops
at init time.
Signed-off-by: Daniel Lezcano <[email protected]>
Reviewed-by: Srinivas Pandruvada <[email protected]>
---
.../int340x_thermal/int340x_thermal_zone.c | 31 ++++++++-----------
.../int340x_thermal/int340x_thermal_zone.h | 4 +--
.../processor_thermal_device.c | 10 ++----
3 files changed, 18 insertions(+), 27 deletions(-)
diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
index 62c0aa5d0783..10731b9a140a 100644
--- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -18,9 +18,6 @@ static int int340x_thermal_get_zone_temp(struct thermal_zone_device *zone,
unsigned long long tmp;
acpi_status status;
- if (d->override_ops && d->override_ops->get_temp)
- return d->override_ops->get_temp(zone, temp);
-
status = acpi_evaluate_integer(d->adev->handle, "_TMP", NULL, &tmp);
if (ACPI_FAILURE(status))
return -EIO;
@@ -46,9 +43,6 @@ static int int340x_thermal_get_trip_temp(struct thermal_zone_device *zone,
struct int34x_thermal_zone *d = zone->devdata;
int i;
- if (d->override_ops && d->override_ops->get_trip_temp)
- return d->override_ops->get_trip_temp(zone, trip, temp);
-
if (trip < d->aux_trip_nr)
*temp = d->aux_trips[trip];
else if (trip == d->crt_trip_id)
@@ -79,9 +73,6 @@ static int int340x_thermal_get_trip_type(struct thermal_zone_device *zone,
struct int34x_thermal_zone *d = zone->devdata;
int i;
- if (d->override_ops && d->override_ops->get_trip_type)
- return d->override_ops->get_trip_type(zone, trip, type);
-
if (trip < d->aux_trip_nr)
*type = THERMAL_TRIP_PASSIVE;
else if (trip == d->crt_trip_id)
@@ -112,9 +103,6 @@ static int int340x_thermal_set_trip_temp(struct thermal_zone_device *zone,
acpi_status status;
char name[10];
- if (d->override_ops && d->override_ops->set_trip_temp)
- return d->override_ops->set_trip_temp(zone, trip, temp);
-
snprintf(name, sizeof(name), "PAT%d", trip);
status = acpi_execute_simple_method(d->adev->handle, name,
millicelsius_to_deci_kelvin(temp));
@@ -134,9 +122,6 @@ static int int340x_thermal_get_trip_hyst(struct thermal_zone_device *zone,
acpi_status status;
unsigned long long hyst;
- if (d->override_ops && d->override_ops->get_trip_hyst)
- return d->override_ops->get_trip_hyst(zone, trip, temp);
-
status = acpi_evaluate_integer(d->adev->handle, "GTSH", NULL, &hyst);
if (ACPI_FAILURE(status))
*temp = 0;
@@ -217,7 +202,7 @@ static struct thermal_zone_params int340x_thermal_params = {
};
struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
- struct thermal_zone_device_ops *override_ops)
+ int (*get_temp) (struct thermal_zone_device *, int *))
{
struct int34x_thermal_zone *int34x_thermal_zone;
acpi_status status;
@@ -231,8 +216,15 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
return ERR_PTR(-ENOMEM);
int34x_thermal_zone->adev = adev;
- int34x_thermal_zone->override_ops = override_ops;
+ int34x_thermal_zone->ops = kmemdup(&int340x_thermal_zone_ops,
+ sizeof(int340x_thermal_zone_ops), GFP_KERNEL);
+ if (!int34x_thermal_zone->ops)
+ goto err_ops_alloc;
+
+ if (get_temp)
+ int34x_thermal_zone->ops->get_temp = get_temp;
+
status = acpi_evaluate_integer(adev->handle, "PATC", NULL, &trip_cnt);
if (ACPI_FAILURE(status))
trip_cnt = 0;
@@ -262,7 +254,7 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
acpi_device_bid(adev),
trip_cnt,
trip_mask, int34x_thermal_zone,
- &int340x_thermal_zone_ops,
+ int34x_thermal_zone->ops,
&int340x_thermal_params,
0, 0);
if (IS_ERR(int34x_thermal_zone->zone)) {
@@ -281,6 +273,8 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
kfree(int34x_thermal_zone->aux_trips);
err_trip_alloc:
+ kfree(int34x_thermal_zone->ops);
+err_ops_alloc:
kfree(int34x_thermal_zone);
return ERR_PTR(ret);
}
@@ -292,6 +286,7 @@ void int340x_thermal_zone_remove(struct int34x_thermal_zone
thermal_zone_device_unregister(int34x_thermal_zone->zone);
acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
kfree(int34x_thermal_zone->aux_trips);
+ kfree(int34x_thermal_zone->ops);
kfree(int34x_thermal_zone);
}
EXPORT_SYMBOL_GPL(int340x_thermal_zone_remove);
diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
index 3b4971df1b33..e28ab1ba5e06 100644
--- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
+++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
@@ -29,13 +29,13 @@ struct int34x_thermal_zone {
int hot_temp;
int hot_trip_id;
struct thermal_zone_device *zone;
- struct thermal_zone_device_ops *override_ops;
+ struct thermal_zone_device_ops *ops;
void *priv_data;
struct acpi_lpat_conversion_table *lpat_table;
};
struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *,
- struct thermal_zone_device_ops *override_ops);
+ int (*get_temp) (struct thermal_zone_device *, int *));
void int340x_thermal_zone_remove(struct int34x_thermal_zone *);
int int340x_thermal_read_trips(struct int34x_thermal_zone *int34x_zone);
diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
index a8d98f1bd6c6..317703027ce9 100644
--- a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
+++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
@@ -207,10 +207,6 @@ static int proc_thermal_get_zone_temp(struct thermal_zone_device *zone,
return ret;
}
-static struct thermal_zone_device_ops proc_thermal_local_ops = {
- .get_temp = proc_thermal_get_zone_temp,
-};
-
static int proc_thermal_read_ppcc(struct proc_thermal_device *proc_priv)
{
int i;
@@ -285,7 +281,7 @@ int proc_thermal_add(struct device *dev, struct proc_thermal_device *proc_priv)
struct acpi_device *adev;
acpi_status status;
unsigned long long tmp;
- struct thermal_zone_device_ops *ops = NULL;
+ int (*get_temp) (struct thermal_zone_device *, int *) = NULL;
int ret;
adev = ACPI_COMPANION(dev);
@@ -304,10 +300,10 @@ int proc_thermal_add(struct device *dev, struct proc_thermal_device *proc_priv)
/* there is no _TMP method, add local method */
stored_tjmax = get_tjmax();
if (stored_tjmax > 0)
- ops = &proc_thermal_local_ops;
+ get_temp = proc_thermal_get_zone_temp;
}
- proc_priv->int340x_zone = int340x_thermal_zone_add(adev, ops);
+ proc_priv->int340x_zone = int340x_thermal_zone_add(adev, get_temp);
if (IS_ERR(proc_priv->int340x_zone)) {
return PTR_ERR(proc_priv->int340x_zone);
} else
--
2.34.1
The thermal framework gives the possibility to register the trip
points with the thermal zone. When that is done, no get_trip_* ops are
needed and they can be removed.
Convert ops content logic into generic trip points and register them with the
thermal zone.
Signed-off-by: Daniel Lezcano <[email protected]>
Reviewed-by: Srinivas Pandruvada <[email protected]>
---
drivers/thermal/intel/x86_pkg_temp_thermal.c | 120 ++++++++++---------
1 file changed, 66 insertions(+), 54 deletions(-)
diff --git a/drivers/thermal/intel/x86_pkg_temp_thermal.c b/drivers/thermal/intel/x86_pkg_temp_thermal.c
index a0e234fce71a..e7c3b78d959c 100644
--- a/drivers/thermal/intel/x86_pkg_temp_thermal.c
+++ b/drivers/thermal/intel/x86_pkg_temp_thermal.c
@@ -53,6 +53,7 @@ struct zone_device {
u32 msr_pkg_therm_high;
struct delayed_work work;
struct thermal_zone_device *tzone;
+ struct thermal_trip *trips;
struct cpumask cpumask;
};
@@ -138,40 +139,6 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd, int *temp)
return -EINVAL;
}
-static int sys_get_trip_temp(struct thermal_zone_device *tzd,
- int trip, int *temp)
-{
- struct zone_device *zonedev = tzd->devdata;
- unsigned long thres_reg_value;
- u32 mask, shift, eax, edx;
- int ret;
-
- if (trip >= MAX_NUMBER_OF_TRIPS)
- return -EINVAL;
-
- if (trip) {
- mask = THERM_MASK_THRESHOLD1;
- shift = THERM_SHIFT_THRESHOLD1;
- } else {
- mask = THERM_MASK_THRESHOLD0;
- shift = THERM_SHIFT_THRESHOLD0;
- }
-
- ret = rdmsr_on_cpu(zonedev->cpu, MSR_IA32_PACKAGE_THERM_INTERRUPT,
- &eax, &edx);
- if (ret < 0)
- return ret;
-
- thres_reg_value = (eax & mask) >> shift;
- if (thres_reg_value)
- *temp = zonedev->tj_max - thres_reg_value * 1000;
- else
- *temp = THERMAL_TEMP_INVALID;
- pr_debug("sys_get_trip_temp %d\n", *temp);
-
- return 0;
-}
-
static int
sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp)
{
@@ -212,18 +179,9 @@ sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp)
l, h);
}
-static int sys_get_trip_type(struct thermal_zone_device *thermal, int trip,
- enum thermal_trip_type *type)
-{
- *type = THERMAL_TRIP_PASSIVE;
- return 0;
-}
-
/* Thermal zone callback registry */
static struct thermal_zone_device_ops tzone_ops = {
.get_temp = sys_get_curr_temp,
- .get_trip_temp = sys_get_trip_temp,
- .get_trip_type = sys_get_trip_type,
.set_trip_temp = sys_set_trip_temp,
};
@@ -328,6 +286,48 @@ static int pkg_thermal_notify(u64 msr_val)
return 0;
}
+static struct thermal_trip *pkg_temp_thermal_trips_init(int cpu, int tj_max, int num_trips)
+{
+ struct thermal_trip *trips;
+ unsigned long thres_reg_value;
+ u32 mask, shift, eax, edx;
+ int ret, i;
+
+ trips = kzalloc(sizeof(*trips) * num_trips, GFP_KERNEL);
+ if (!trips)
+ return ERR_PTR(-ENOMEM);
+
+ for (i = 0; i < num_trips; i++) {
+
+ if (i) {
+ mask = THERM_MASK_THRESHOLD1;
+ shift = THERM_SHIFT_THRESHOLD1;
+ } else {
+ mask = THERM_MASK_THRESHOLD0;
+ shift = THERM_SHIFT_THRESHOLD0;
+ }
+
+ ret = rdmsr_on_cpu(cpu, MSR_IA32_PACKAGE_THERM_INTERRUPT,
+ &eax, &edx);
+ if (ret < 0) {
+ kfree(trips);
+ return ERR_PTR(ret);
+ }
+
+ thres_reg_value = (eax & mask) >> shift;
+
+ trips[i].temperature = thres_reg_value ?
+ tj_max - thres_reg_value * 1000 : THERMAL_TEMP_INVALID;
+
+ trips[i].type = THERMAL_TRIP_PASSIVE;
+
+ pr_debug("%s: cpu=%d, trip=%d, temp=%d\n",
+ __func__, cpu, i, trips[i].temperature);
+ }
+
+ return trips;
+}
+
static int pkg_temp_thermal_device_add(unsigned int cpu)
{
int id = topology_logical_die_id(cpu);
@@ -353,24 +353,27 @@ static int pkg_temp_thermal_device_add(unsigned int cpu)
if (!zonedev)
return -ENOMEM;
+ zonedev->trips = pkg_temp_thermal_trips_init(cpu, tj_max, thres_count);
+ if (IS_ERR(zonedev->trips)) {
+ err = PTR_ERR(zonedev->trips);
+ goto out_kfree_zonedev;
+ }
+
INIT_DELAYED_WORK(&zonedev->work, pkg_temp_thermal_threshold_work_fn);
zonedev->cpu = cpu;
zonedev->tj_max = tj_max;
- zonedev->tzone = thermal_zone_device_register("x86_pkg_temp",
- thres_count,
+ zonedev->tzone = thermal_zone_device_register_with_trips("x86_pkg_temp",
+ zonedev->trips, thres_count,
(thres_count == MAX_NUMBER_OF_TRIPS) ? 0x03 : 0x01,
zonedev, &tzone_ops, &pkg_temp_tz_params, 0, 0);
if (IS_ERR(zonedev->tzone)) {
err = PTR_ERR(zonedev->tzone);
- kfree(zonedev);
- return err;
+ goto out_kfree_trips;
}
err = thermal_zone_device_enable(zonedev->tzone);
- if (err) {
- thermal_zone_device_unregister(zonedev->tzone);
- kfree(zonedev);
- return err;
- }
+ if (err)
+ goto out_unregister_tz;
+
/* Store MSR value for package thermal interrupt, to restore at exit */
rdmsr(MSR_IA32_PACKAGE_THERM_INTERRUPT, zonedev->msr_pkg_therm_low,
zonedev->msr_pkg_therm_high);
@@ -379,7 +382,14 @@ static int pkg_temp_thermal_device_add(unsigned int cpu)
raw_spin_lock_irq(&pkg_temp_lock);
zones[id] = zonedev;
raw_spin_unlock_irq(&pkg_temp_lock);
- return 0;
+
+out_unregister_tz:
+ thermal_zone_device_unregister(zonedev->tzone);
+out_kfree_trips:
+ kfree(zonedev->trips);
+out_kfree_zonedev:
+ kfree(zonedev);
+ return err;
}
static int pkg_thermal_cpu_offline(unsigned int cpu)
@@ -463,8 +473,10 @@ static int pkg_thermal_cpu_offline(unsigned int cpu)
raw_spin_unlock_irq(&pkg_temp_lock);
/* Final cleanup if this is the last cpu */
- if (lastcpu)
+ if (lastcpu) {
+ kfree(zonedev->trips);
kfree(zonedev);
+ }
return 0;
}
--
2.34.1
The thermal_zone_get_trip() does the same check as
of_thermal_is_trip_valid(). Replace the call to
of_thermal_is_trip_valid() by thermal_zone_get_trip().
Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/samsung/exynos_tmu.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 91e6860b5ec4..34b460092308 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -554,13 +554,14 @@ static void exynos4210_tmu_control(struct platform_device *pdev, bool on)
{
struct exynos_tmu_data *data = platform_get_drvdata(pdev);
struct thermal_zone_device *tz = data->tzd;
+ struct thermal_trip trip;
unsigned int con, interrupt_en = 0, i;
con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
if (on) {
for (i = 0; i < data->ntrip; i++) {
- if (!of_thermal_is_trip_valid(tz, i))
+ if (thermal_zone_get_trip(tz, i, &trip))
continue;
interrupt_en |=
@@ -584,13 +585,14 @@ static void exynos5433_tmu_control(struct platform_device *pdev, bool on)
{
struct exynos_tmu_data *data = platform_get_drvdata(pdev);
struct thermal_zone_device *tz = data->tzd;
+ struct thermal_trip trip;
unsigned int con, interrupt_en = 0, pd_det_en, i;
con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
if (on) {
for (i = 0; i < data->ntrip; i++) {
- if (!of_thermal_is_trip_valid(tz, i))
+ if (thermal_zone_get_trip(tz, i, &trip))
continue;
interrupt_en |=
@@ -615,13 +617,14 @@ static void exynos7_tmu_control(struct platform_device *pdev, bool on)
{
struct exynos_tmu_data *data = platform_get_drvdata(pdev);
struct thermal_zone_device *tz = data->tzd;
+ struct thermal_trip trip;
unsigned int con, interrupt_en = 0, i;
con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
if (on) {
for (i = 0; i < data->ntrip; i++) {
- if (!of_thermal_is_trip_valid(tz, i))
+ if (thermal_zone_get_trip(tz, i, &trip))
continue;
interrupt_en |=
--
2.34.1
The thermal framework gives the possibility to register the trip
points with the thermal zone. When that is done, no get_trip_* ops are
needed and they can be removed.
Convert ops content logic into generic trip points and register them with the
thermal zone.
Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/hisi_thermal.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
index d6974db7aaf7..45226cab466e 100644
--- a/drivers/thermal/hisi_thermal.c
+++ b/drivers/thermal/hisi_thermal.c
@@ -482,7 +482,7 @@ static int hisi_thermal_register_sensor(struct platform_device *pdev,
struct hisi_thermal_sensor *sensor)
{
int ret, i;
- const struct thermal_trip *trip;
+ struct thermal_trip trip;
sensor->tzd = devm_thermal_of_zone_register(&pdev->dev,
sensor->id, sensor,
@@ -495,11 +495,12 @@ static int hisi_thermal_register_sensor(struct platform_device *pdev,
return ret;
}
- trip = of_thermal_get_trip_points(sensor->tzd);
+ for (i = 0; i < thermal_zone_get_num_trips(sensor->tzd); i++) {
- for (i = 0; i < of_thermal_get_ntrips(sensor->tzd); i++) {
- if (trip[i].type == THERMAL_TRIP_PASSIVE) {
- sensor->thres_temp = trip[i].temperature;
+ thermal_zone_get_trip(sensor->tzd, i, &trip);
+
+ if (trip.type == THERMAL_TRIP_PASSIVE) {
+ sensor->thres_temp = trip.temperature;
break;
}
}
--
2.34.1
The thermal framework gives the possibility to register the trip
points with the thermal zone. When that is done, no get_trip_* ops are
needed and they can be removed.
Convert ops content logic into generic trip points and register them with the
thermal zone.
Signed-off-by: Daniel Lezcano <[email protected]>
Reviewed-by: Kunihiko Hayashi <[email protected]>
---
drivers/thermal/uniphier_thermal.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/drivers/thermal/uniphier_thermal.c b/drivers/thermal/uniphier_thermal.c
index 4111d99ef50e..277ae300c5b1 100644
--- a/drivers/thermal/uniphier_thermal.c
+++ b/drivers/thermal/uniphier_thermal.c
@@ -248,8 +248,7 @@ static int uniphier_tm_probe(struct platform_device *pdev)
struct regmap *regmap;
struct device_node *parent;
struct uniphier_tm_dev *tdev;
- const struct thermal_trip *trips;
- int i, ret, irq, ntrips, crit_temp = INT_MAX;
+ int i, ret, irq, crit_temp = INT_MAX;
tdev = devm_kzalloc(dev, sizeof(*tdev), GFP_KERNEL);
if (!tdev)
@@ -296,20 +295,18 @@ static int uniphier_tm_probe(struct platform_device *pdev)
return PTR_ERR(tdev->tz_dev);
}
- /* get trip points */
- trips = of_thermal_get_trip_points(tdev->tz_dev);
- ntrips = of_thermal_get_ntrips(tdev->tz_dev);
- if (ntrips > ALERT_CH_NUM) {
- dev_err(dev, "thermal zone has too many trips\n");
- return -E2BIG;
- }
-
/* set alert temperatures */
- for (i = 0; i < ntrips; i++) {
- if (trips[i].type == THERMAL_TRIP_CRITICAL &&
- trips[i].temperature < crit_temp)
- crit_temp = trips[i].temperature;
- uniphier_tm_set_alert(tdev, i, trips[i].temperature);
+ for (i = 0; i < thermal_zone_get_num_trips(tdev->tz_dev); i++) {
+ struct thermal_trip trip;
+
+ ret = thermal_zone_get_trip(tdev->tz_dev, i, &trip);
+ if (ret)
+ return ret;
+
+ if (trip.type == THERMAL_TRIP_CRITICAL &&
+ trip.temperature < crit_temp)
+ crit_temp = trip.temperature;
+ uniphier_tm_set_alert(tdev, i, trip.temperature);
tdev->alert_en[i] = true;
}
if (crit_temp > CRITICAL_TEMP_LIMIT) {
--
2.34.1
The thermal framework gives the possibility to register the trip
points with the thermal zone. When that is done, no get_trip_* ops are
needed and they can be removed.
Convert ops content logic into generic trip points and register them with the
thermal zone.
Signed-off-by: Daniel Lezcano <[email protected]>
Acked-by: Hans de Goede <[email protected]>
Acked-by: Peter Kästle <[email protected]>
---
drivers/platform/x86/acerhdf.c | 73 ++++++++++++----------------------
1 file changed, 26 insertions(+), 47 deletions(-)
diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index 3463629f8764..a7407aa032ba 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -46,6 +46,8 @@
* measured by the on-die thermal monitor are within 0 <= Tj <= 90. So,
* assume 89°C is critical temperature.
*/
+#define ACERHDF_DEFAULT_TEMP_FANON 60000
+#define ACERHDF_DEFAULT_TEMP_FANOFF 53000
#define ACERHDF_TEMP_CRIT 89000
#define ACERHDF_FAN_OFF 0
#define ACERHDF_FAN_AUTO 1
@@ -70,8 +72,8 @@ static int kernelmode;
#endif
static unsigned int interval = 10;
-static unsigned int fanon = 60000;
-static unsigned int fanoff = 53000;
+static unsigned int fanon = ACERHDF_DEFAULT_TEMP_FANON;
+static unsigned int fanoff = ACERHDF_DEFAULT_TEMP_FANOFF;
static unsigned int verbose;
static unsigned int list_supported;
static unsigned int fanstate = ACERHDF_FAN_AUTO;
@@ -137,6 +139,15 @@ struct ctrl_settings {
int mcmd_enable;
};
+static struct thermal_trip trips[] = {
+ [0] = { .temperature = ACERHDF_DEFAULT_TEMP_FANON,
+ .hysteresis = ACERHDF_DEFAULT_TEMP_FANON - ACERHDF_DEFAULT_TEMP_FANOFF,
+ .type = THERMAL_TRIP_ACTIVE },
+
+ [1] = { .temperature = ACERHDF_TEMP_CRIT,
+ .type = THERMAL_TRIP_CRITICAL }
+};
+
static struct ctrl_settings ctrl_cfg __read_mostly;
/* Register addresses and values for different BIOS versions */
@@ -326,6 +337,15 @@ static void acerhdf_check_param(struct thermal_zone_device *thermal)
fanon = ACERHDF_MAX_FANON;
}
+ if (fanon < fanoff) {
+ pr_err("fanoff temperature (%d) is above fanon temperature (%d), clamping to %d\n",
+ fanoff, fanon, fanon);
+ fanoff = fanon;
+ };
+
+ trips[0].temperature = fanon;
+ trips[0].hysteresis = fanon - fanoff;
+
if (kernelmode && prev_interval != interval) {
if (interval > ACERHDF_MAX_INTERVAL) {
pr_err("interval too high, set to %d\n",
@@ -424,43 +444,6 @@ static int acerhdf_change_mode(struct thermal_zone_device *thermal,
return 0;
}
-static int acerhdf_get_trip_type(struct thermal_zone_device *thermal, int trip,
- enum thermal_trip_type *type)
-{
- if (trip == 0)
- *type = THERMAL_TRIP_ACTIVE;
- else if (trip == 1)
- *type = THERMAL_TRIP_CRITICAL;
- else
- return -EINVAL;
-
- return 0;
-}
-
-static int acerhdf_get_trip_hyst(struct thermal_zone_device *thermal, int trip,
- int *temp)
-{
- if (trip != 0)
- return -EINVAL;
-
- *temp = fanon - fanoff;
-
- return 0;
-}
-
-static int acerhdf_get_trip_temp(struct thermal_zone_device *thermal, int trip,
- int *temp)
-{
- if (trip == 0)
- *temp = fanon;
- else if (trip == 1)
- *temp = ACERHDF_TEMP_CRIT;
- else
- return -EINVAL;
-
- return 0;
-}
-
static int acerhdf_get_crit_temp(struct thermal_zone_device *thermal,
int *temperature)
{
@@ -474,13 +457,9 @@ static struct thermal_zone_device_ops acerhdf_dev_ops = {
.unbind = acerhdf_unbind,
.get_temp = acerhdf_get_ec_temp,
.change_mode = acerhdf_change_mode,
- .get_trip_type = acerhdf_get_trip_type,
- .get_trip_hyst = acerhdf_get_trip_hyst,
- .get_trip_temp = acerhdf_get_trip_temp,
.get_crit_temp = acerhdf_get_crit_temp,
};
-
/*
* cooling device callback functions
* get maximal fan cooling state
@@ -710,10 +689,10 @@ static int __init acerhdf_register_thermal(void)
if (IS_ERR(cl_dev))
return -EINVAL;
- thz_dev = thermal_zone_device_register("acerhdf", 2, 0, NULL,
- &acerhdf_dev_ops,
- &acerhdf_zone_params, 0,
- (kernelmode) ? interval*1000 : 0);
+ thz_dev = thermal_zone_device_register_with_trips("acerhdf", trips, ARRAY_SIZE(trips),
+ 0, NULL, &acerhdf_dev_ops,
+ &acerhdf_zone_params, 0,
+ (kernelmode) ? interval*1000 : 0);
if (IS_ERR(thz_dev))
return -EINVAL;
--
2.34.1
The thermal zone ops defines a set_trip callback where we can invoke
the backend driver to set an interrupt for the next trip point
temperature being crossed the way up or down, or setting the low level
with the hysteresis.
The ops is only called from the thermal sysfs code where the userspace
has the ability to modify a trip point characteristic.
With the effort of encapsulating the thermal framework core code,
let's create a thermal_zone_set_trip() which is the writable side of
the thermal_zone_get_trip() and put there all the ops encapsulation.
Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/thermal_core.c | 44 ++++++++++++++++++++++++++++++
drivers/thermal/thermal_sysfs.c | 48 +++++++++++----------------------
include/linux/thermal.h | 3 +++
3 files changed, 63 insertions(+), 32 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 381d85ec74a0..fa0f89a24b68 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1183,6 +1183,50 @@ int thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id,
}
EXPORT_SYMBOL_GPL(thermal_zone_get_trip);
+int thermal_zone_set_trip(struct thermal_zone_device *tz, int trip_id,
+ const struct thermal_trip *trip)
+{
+ struct thermal_trip t;
+ int ret = -EINVAL;
+
+ mutex_lock(&tz->lock);
+
+ if (!tz->ops->set_trip_temp && !tz->ops->set_trip_hyst && !tz->trips)
+ goto out;
+
+ ret = __thermal_zone_get_trip(tz, trip_id, &t);
+ if (ret)
+ goto out;
+
+ if ((t.temperature != trip->temperature) && tz->ops->set_trip_temp) {
+
+ ret = tz->ops->set_trip_temp(tz, trip_id, trip->temperature);
+ if (ret)
+ goto out;
+ }
+
+ if ((t.hysteresis != trip->hysteresis) && tz->ops->set_trip_hyst) {
+
+ ret = tz->ops->set_trip_hyst(tz, trip_id, trip->hysteresis);
+ if (ret)
+ goto out;
+ }
+
+ if (((t.temperature != trip->temperature) ||
+ (t.hysteresis != trip->hysteresis)) && tz->trips)
+ tz->trips[trip_id] = *trip;
+
+out:
+ mutex_unlock(&tz->lock);
+
+ if (!ret) {
+ thermal_notify_tz_trip_change(tz->id, trip_id, trip->type,
+ trip->temperature, trip->hysteresis);
+ thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED);
+ }
+
+ return ret;
+}
/**
* thermal_zone_device_register_with_trips() - register a new thermal zone device
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index 18cdd7cd0008..8d7b25ab67c2 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -115,31 +115,20 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr,
struct thermal_trip trip;
int trip_id, ret;
- if (!tz->ops->set_trip_temp && !tz->trips)
- return -EPERM;
-
if (sscanf(attr->attr.name, "trip_point_%d_temp", &trip_id) != 1)
return -EINVAL;
- if (kstrtoint(buf, 10, &trip.temperature))
- return -EINVAL;
-
- ret = tz->ops->set_trip_temp(tz, trip_id, trip.temperature);
+ ret = thermal_zone_get_trip(tz, trip_id, &trip);
if (ret)
return ret;
- if (tz->trips)
- tz->trips[trip_id].temperature = trip.temperature;
+ if (kstrtoint(buf, 10, &trip.temperature))
+ return -EINVAL;
- ret = thermal_zone_get_trip(tz, trip_id, &trip);
+ ret = thermal_zone_set_trip(tz, trip_id, &trip);
if (ret)
return ret;
- thermal_notify_tz_trip_change(tz->id, trip_id, trip.type,
- trip.temperature, trip.hysteresis);
-
- thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
-
return count;
}
@@ -166,29 +155,24 @@ trip_point_hyst_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct thermal_zone_device *tz = to_thermal_zone(dev);
- int trip, ret;
- int temperature;
-
- if (!tz->ops->set_trip_hyst)
- return -EPERM;
+ struct thermal_trip trip;
+ int trip_id, ret;
- if (sscanf(attr->attr.name, "trip_point_%d_hyst", &trip) != 1)
+ if (sscanf(attr->attr.name, "trip_point_%d_hyst", &trip_id) != 1)
return -EINVAL;
- if (kstrtoint(buf, 10, &temperature))
- return -EINVAL;
+ ret = thermal_zone_get_trip(tz, trip_id, &trip);
+ if (ret)
+ return ret;
- /*
- * We are not doing any check on the 'temperature' value
- * here. The driver implementing 'set_trip_hyst' has to
- * take care of this.
- */
- ret = tz->ops->set_trip_hyst(tz, trip, temperature);
+ if (kstrtoint(buf, 10, &trip.hysteresis))
+ return -EINVAL;
- if (!ret)
- thermal_zone_set_trips(tz);
+ ret = thermal_zone_set_trip(tz, trip_id, &trip);
+ if (ret)
+ return ret;
- return ret ? ret : count;
+ return count;
}
static ssize_t
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 09dc09228717..5350a437f245 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -338,6 +338,9 @@ static inline void devm_thermal_of_zone_unregister(struct device *dev,
int thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id,
struct thermal_trip *trip);
+int thermal_zone_set_trip(struct thermal_zone_device *tz, int trip_id,
+ const struct thermal_trip *trip);
+
int thermal_zone_get_num_trips(struct thermal_zone_device *tz);
#ifdef CONFIG_THERMAL
--
2.34.1
On 21/09/2022 11:42, Daniel Lezcano wrote:
> The thermal_zone_get_trip() does the same check as
> of_thermal_is_trip_valid(). Replace the call to
> of_thermal_is_trip_valid() by thermal_zone_get_trip().
>
> Signed-off-by: Daniel Lezcano <[email protected]>
> ---
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
Hi Daniel,
On 21.09.2022 11:42, Daniel Lezcano wrote:
> The thermal_zone_get_trip() does the same check as
> of_thermal_is_trip_valid(). Replace the call to
> of_thermal_is_trip_valid() by thermal_zone_get_trip().
>
> Signed-off-by: Daniel Lezcano <[email protected]>
> ---
This patch landed in linux next-20220923 as commit 4a71bb8005ba
("thermal/drivers/exynos: Replace of_thermal_is_trip_valid() by
thermal_zone_get_trip()"). Unfortunately it causes deadlock on all
Exynos based board:
============================================
WARNING: possible recursive locking detected
6.0.0-rc1-00062-g4a71bb8005ba #12855 Not tainted
--------------------------------------------
swapper/0/1 is trying to acquire lock:
c263c394 (&tz->lock){+.+.}-{3:3}, at:
thermal_zone_device_update.part.0+0x114/0x538
but task is already holding lock:
c263c394 (&tz->lock){+.+.}-{3:3}, at:
thermal_zone_device_update.part.0+0x3c/0x538
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&tz->lock);
lock(&tz->lock);
*** DEADLOCK ***
May be due to missing lock nesting notation
2 locks held by swapper/0/1:
#0: c1d5248c (&dev->mutex){....}-{3:3}, at: __driver_attach+0xe4/0x1f0
#1: c263c394 (&tz->lock){+.+.}-{3:3}, at:
thermal_zone_device_update.part.0+0x3c/0x538
stack backtrace:
CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.0.0-rc1-00062-g4a71bb8005ba
#12855
Hardware name: Samsung Exynos (Flattened Device Tree)
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x58/0x70
dump_stack_lvl from __lock_acquire+0x146c/0x2a7c
__lock_acquire from lock_acquire+0x124/0x3e4
lock_acquire from __mutex_lock+0x90/0x948
__mutex_lock from mutex_lock_nested+0x1c/0x24
mutex_lock_nested from thermal_zone_device_update.part.0+0x114/0x538
thermal_zone_device_update.part.0 from
thermal_zone_device_set_mode+0x70/0x98
thermal_zone_device_set_mode from thermal_of_zone_register+0x424/0x69c
thermal_of_zone_register from devm_thermal_of_zone_register+0x58/0x94
devm_thermal_of_zone_register from exynos_tmu_probe+0x29c/0x728
exynos_tmu_probe from platform_probe+0x5c/0xb8
platform_probe from really_probe+0xe0/0x414
really_probe from __driver_probe_device+0xa0/0x208
__driver_probe_device from driver_probe_device+0x30/0xc0
driver_probe_device from __driver_attach+0xf0/0x1f0
__driver_attach from bus_for_each_dev+0x70/0xb0
bus_for_each_dev from bus_add_driver+0x174/0x218
bus_add_driver from driver_register+0x88/0x11c
driver_register from do_one_initcall+0x64/0x380
do_one_initcall from kernel_init_freeable+0x1c0/0x224
kernel_init_freeable from kernel_init+0x18/0x12c
kernel_init from ret_from_fork+0x14/0x2c
Exception stack(0xf082dfb0 to 0xf082dff8)
[deadlock]
Something is wrong in locking in the functions from the above stacktrace.
> drivers/thermal/samsung/exynos_tmu.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 91e6860b5ec4..34b460092308 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -554,13 +554,14 @@ static void exynos4210_tmu_control(struct platform_device *pdev, bool on)
> {
> struct exynos_tmu_data *data = platform_get_drvdata(pdev);
> struct thermal_zone_device *tz = data->tzd;
> + struct thermal_trip trip;
> unsigned int con, interrupt_en = 0, i;
>
> con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
>
> if (on) {
> for (i = 0; i < data->ntrip; i++) {
> - if (!of_thermal_is_trip_valid(tz, i))
> + if (thermal_zone_get_trip(tz, i, &trip))
> continue;
>
> interrupt_en |=
> @@ -584,13 +585,14 @@ static void exynos5433_tmu_control(struct platform_device *pdev, bool on)
> {
> struct exynos_tmu_data *data = platform_get_drvdata(pdev);
> struct thermal_zone_device *tz = data->tzd;
> + struct thermal_trip trip;
> unsigned int con, interrupt_en = 0, pd_det_en, i;
>
> con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
>
> if (on) {
> for (i = 0; i < data->ntrip; i++) {
> - if (!of_thermal_is_trip_valid(tz, i))
> + if (thermal_zone_get_trip(tz, i, &trip))
> continue;
>
> interrupt_en |=
> @@ -615,13 +617,14 @@ static void exynos7_tmu_control(struct platform_device *pdev, bool on)
> {
> struct exynos_tmu_data *data = platform_get_drvdata(pdev);
> struct thermal_zone_device *tz = data->tzd;
> + struct thermal_trip trip;
> unsigned int con, interrupt_en = 0, i;
>
> con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
>
> if (on) {
> for (i = 0; i < data->ntrip; i++) {
> - if (!of_thermal_is_trip_valid(tz, i))
> + if (thermal_zone_get_trip(tz, i, &trip))
> continue;
>
> interrupt_en |=
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On Wed, 2022-09-21 at 11:42 +0200, Daniel Lezcano wrote:
> The thermal zone ops defines a set_trip callback where we can invoke
> the backend driver to set an interrupt for the next trip point
> temperature being crossed the way up or down, or setting the low
> level
> with the hysteresis.
>
> The ops is only called from the thermal sysfs code where the
> userspace
> has the ability to modify a trip point characteristic.
>
> With the effort of encapsulating the thermal framework core code,
> let's create a thermal_zone_set_trip() which is the writable side of
> the thermal_zone_get_trip() and put there all the ops encapsulation.
>
> Signed-off-by: Daniel Lezcano <[email protected]>
Reviewed-by: Zhang Rui <[email protected]>
thanks,
rui
> ---
> drivers/thermal/thermal_core.c | 44 ++++++++++++++++++++++++++++++
> drivers/thermal/thermal_sysfs.c | 48 +++++++++++------------------
> ----
> include/linux/thermal.h | 3 +++
> 3 files changed, 63 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c
> b/drivers/thermal/thermal_core.c
> index 381d85ec74a0..fa0f89a24b68 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1183,6 +1183,50 @@ int thermal_zone_get_trip(struct
> thermal_zone_device *tz, int trip_id,
> }
> EXPORT_SYMBOL_GPL(thermal_zone_get_trip);
>
> +int thermal_zone_set_trip(struct thermal_zone_device *tz, int
> trip_id,
> + const struct thermal_trip *trip)
> +{
> + struct thermal_trip t;
> + int ret = -EINVAL;
> +
> + mutex_lock(&tz->lock);
> +
> + if (!tz->ops->set_trip_temp && !tz->ops->set_trip_hyst && !tz-
> >trips)
> + goto out;
> +
> + ret = __thermal_zone_get_trip(tz, trip_id, &t);
> + if (ret)
> + goto out;
> +
> + if ((t.temperature != trip->temperature) && tz->ops-
> >set_trip_temp) {
> +
> + ret = tz->ops->set_trip_temp(tz, trip_id, trip-
> >temperature);
> + if (ret)
> + goto out;
> + }
> +
> + if ((t.hysteresis != trip->hysteresis) && tz->ops-
> >set_trip_hyst) {
> +
> + ret = tz->ops->set_trip_hyst(tz, trip_id, trip-
> >hysteresis);
> + if (ret)
> + goto out;
> + }
> +
> + if (((t.temperature != trip->temperature) ||
> + (t.hysteresis != trip->hysteresis)) && tz->trips)
> + tz->trips[trip_id] = *trip;
> +
> +out:
> + mutex_unlock(&tz->lock);
> +
> + if (!ret) {
> + thermal_notify_tz_trip_change(tz->id, trip_id, trip-
> >type,
> + trip->temperature, trip-
> >hysteresis);
> + thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED);
> + }
> +
> + return ret;
> +}
>
> /**
> * thermal_zone_device_register_with_trips() - register a new
> thermal zone device
> diff --git a/drivers/thermal/thermal_sysfs.c
> b/drivers/thermal/thermal_sysfs.c
> index 18cdd7cd0008..8d7b25ab67c2 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -115,31 +115,20 @@ trip_point_temp_store(struct device *dev,
> struct device_attribute *attr,
> struct thermal_trip trip;
> int trip_id, ret;
>
> - if (!tz->ops->set_trip_temp && !tz->trips)
> - return -EPERM;
> -
> if (sscanf(attr->attr.name, "trip_point_%d_temp", &trip_id) !=
> 1)
> return -EINVAL;
>
> - if (kstrtoint(buf, 10, &trip.temperature))
> - return -EINVAL;
> -
> - ret = tz->ops->set_trip_temp(tz, trip_id, trip.temperature);
> + ret = thermal_zone_get_trip(tz, trip_id, &trip);
> if (ret)
> return ret;
>
> - if (tz->trips)
> - tz->trips[trip_id].temperature = trip.temperature;
> + if (kstrtoint(buf, 10, &trip.temperature))
> + return -EINVAL;
>
> - ret = thermal_zone_get_trip(tz, trip_id, &trip);
> + ret = thermal_zone_set_trip(tz, trip_id, &trip);
> if (ret)
> return ret;
>
> - thermal_notify_tz_trip_change(tz->id, trip_id, trip.type,
> - trip.temperature,
> trip.hysteresis);
> -
> - thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> -
> return count;
> }
>
> @@ -166,29 +155,24 @@ trip_point_hyst_store(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> struct thermal_zone_device *tz = to_thermal_zone(dev);
> - int trip, ret;
> - int temperature;
> -
> - if (!tz->ops->set_trip_hyst)
> - return -EPERM;
> + struct thermal_trip trip;
> + int trip_id, ret;
>
> - if (sscanf(attr->attr.name, "trip_point_%d_hyst", &trip) != 1)
> + if (sscanf(attr->attr.name, "trip_point_%d_hyst", &trip_id) !=
> 1)
> return -EINVAL;
>
> - if (kstrtoint(buf, 10, &temperature))
> - return -EINVAL;
> + ret = thermal_zone_get_trip(tz, trip_id, &trip);
> + if (ret)
> + return ret;
>
> - /*
> - * We are not doing any check on the 'temperature' value
> - * here. The driver implementing 'set_trip_hyst' has to
> - * take care of this.
> - */
> - ret = tz->ops->set_trip_hyst(tz, trip, temperature);
> + if (kstrtoint(buf, 10, &trip.hysteresis))
> + return -EINVAL;
>
> - if (!ret)
> - thermal_zone_set_trips(tz);
> + ret = thermal_zone_set_trip(tz, trip_id, &trip);
> + if (ret)
> + return ret;
>
> - return ret ? ret : count;
> + return count;
> }
>
> static ssize_t
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 09dc09228717..5350a437f245 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -338,6 +338,9 @@ static inline void
> devm_thermal_of_zone_unregister(struct device *dev,
> int thermal_zone_get_trip(struct thermal_zone_device *tz, int
> trip_id,
> struct thermal_trip *trip);
>
> +int thermal_zone_set_trip(struct thermal_zone_device *tz, int
> trip_id,
> + const struct thermal_trip *trip);
> +
> int thermal_zone_get_num_trips(struct thermal_zone_device *tz);
>
> #ifdef CONFIG_THERMAL
Hi Marek,
thanks for reporting
On 23/09/2022 16:09, Marek Szyprowski wrote:
[ ... ]
> Exception stack(0xf082dfb0 to 0xf082dff8)
>
> [deadlock]
>
> Something is wrong in locking in the functions from the above stacktrace.
Are you sure this deadlock is coming from this patch? Does a revert of
this patch solve the issue ?
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Hi Daniel,
On 23.09.2022 19:40, Daniel Lezcano wrote:
>
> Hi Marek,
>
> thanks for reporting
>
> On 23/09/2022 16:09, Marek Szyprowski wrote:
>
> [ ... ]
>
>> Exception stack(0xf082dfb0 to 0xf082dff8)
>>
>> [deadlock]
>>
>> Something is wrong in locking in the functions from the above
>> stacktrace.
>
> Are you sure this deadlock is coming from this patch? Does a revert of
> this patch solve the issue ?
>
Ups, my fault. It looks that I've copied SHA from the wrong window while
preparing the report. The bisection pointed to the 78ffa3e58d93
("thermal/core: Add a generic thermal_zone_get_trip() function") commit
and I've already found how to fix the deadlock. I will report it again
under the proper patch.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland