Add ACPI trip points parser helpers

PATCH In review Daniel Lezcano 2 participants 8 messages Dec 12, 2022 – Dec 14, 2022 Thermal

Share & tools

Daniel Lezcano

author

2022-12-12 20:59 UTC #0 lore raw

cover Add ACPI trip points parser helpers

Regarding thermal, the ACPI code parsing the trip points is duplicated inside different Intel drivers. The generic ACPI thermal driver also duplicates this code.

Those ACPI parsing actions are used to define trip points and these ones are defined by, and specific to, the backend driver.

The different get_trip_* ops, also very similar between each other, are using the specific trip point structure to return the requested trip point information.

From there, there is nothing new, all this can be converted to the generic trip point.

The first patch provides the functions to parse trip point description from the ACPI table but based on the generic trip point structure. The other patches convert the different Intel driver to use the ACPI functions along with a migration to the generic trip point.

Daniel Lezcano (5):
thermal/acpi: Add ACPI trip point routines thermal/drivers/intel: Use generic trip points for intel_pch thermal/drivers/intel: Use generic trip points int340x thermal/drivers/intel: Use generic trip points for quark_dts thermal/drivers/intel: Use generic trip points for
processor_thermal_device_pci

 drivers/thermal/Kconfig                       |  13 +
 drivers/thermal/Makefile                      |   1 +
 .../int340x_thermal/int340x_thermal_zone.c    | 175 +++---------
 .../int340x_thermal/int340x_thermal_zone.h    |  10 +-
 .../processor_thermal_device_pci.c            |  53 ++--
 drivers/thermal/intel/intel_pch_thermal.c     |  88 ++-----
 .../thermal/intel/intel_quark_dts_thermal.c   |  45 ++--
 drivers/thermal/thermal_acpi.c                | 249 ++++++++++++++++++
 include/linux/thermal.h                       |  15 ++
 9 files changed, 377 insertions(+), 272 deletions(-)
 create mode 100644 drivers/thermal/thermal_acpi.c

Signature · 1 line

2.34.1

Daniel Lezcano

author

2022-12-12 20:59 UTC #1 lore raw

PATCH 1/5 thermal/acpi: Add ACPI trip point routines

From: Daniel Lezcano <[email protected]>

The ACPI specification describes the trip points, the device tree bindings as well.

The OF code uses the generic trip point structures.

The ACPI has their own trip points structure and uses the get_trip_* ops to retrieve them.

We can do the same as the OF code and create a set of ACPI functions to retrieve a trip point desciption. Having a common code for ACPI will help to cleanup the remaining Intel drivers and get rid of the get_trip_* functions.

Signed-off-by Daniel Lezcano <[email protected]>

---
 drivers/thermal/Kconfig        |  13 ++
 drivers/thermal/Makefile       |   1 +
 drivers/thermal/thermal_acpi.c | 249 +++++++++++++++++++++++++++++++++
 include/linux/thermal.h        |  15 ++
 4 files changed, 278 insertions(+)
 create mode 100644 drivers/thermal/thermal_acpi.c

drivers/thermal/Kconfig +13 -0

index e052dae614eb..e57011be7009 100644
@@ -76,6 +76,19 @@ config THERMAL_OF
 	  Say 'Y' here if you need to build thermal infrastructure
 	  based on device tree.
 
+config THERMAL_ACPI
+	bool
+	prompt "APIs to parse thermal data out of the ACPI tables"
+	depends on ACPI
+	default y
+	help
+	  This options provides helpers to add the support to
+	  read and parse thermal data definitions out of the
+	  ACPI tables blob.
+
+	  Say 'Y' here if you need to build thermal infrastructure
+	  based on ACPI.
+
 config THERMAL_WRITABLE_TRIPS
 	bool "Enable writable trip points"
 	help

drivers/thermal/Makefile +1 -0

index 2506c6c8ca83..60f0dfa9aae2 100644
@@ -13,6 +13,7 @@ thermal_sys-$(CONFIG_THERMAL_NETLINK)		+= thermal_netlink.o
 # interface to/from other layers providing sensors
 thermal_sys-$(CONFIG_THERMAL_HWMON)		+= thermal_hwmon.o
 thermal_sys-$(CONFIG_THERMAL_OF)		+= thermal_of.o
+thermal_sys-$(CONFIG_THERMAL_ACPI)		+= thermal_acpi.o
 
 # governors
 thermal_sys-$(CONFIG_THERMAL_GOV_FAIR_SHARE)	+= gov_fair_share.o

drivers/thermal/thermal_acpi.c new file +249 -0

249 lines not on this page · raw message

include/linux/thermal.h +15 -0

index e2797f314d99..b8b9d9eddb3a 100644
@@ -334,6 +334,21 @@ static inline void devm_thermal_of_zone_unregister(struct device *dev,
 }
 #endif
 
+#ifdef CONFIG_ACPI
+struct acpi_handle_list;
+int thermal_acpi_get_temp(struct acpi_device *adev, int *temperature);
+int thermal_acpi_trip_crit(struct acpi_device *adev, struct thermal_trip *trip);
+int thermal_acpi_trip_hot(struct acpi_device *adev, struct thermal_trip *trip);
+int thermal_acpi_trip_psv(struct acpi_device *adev, struct thermal_trip *trip);
+int thermal_acpi_trip_act(struct acpi_device *adev, struct thermal_trip *trip, int id);
+int thermal_acpi_trip_acl(struct acpi_device *adev, struct acpi_handle_list *devices, int id);
+int thermal_acpi_trip_psv_psl(struct acpi_device *adev, struct acpi_handle_list *devices);
+int thermal_acpi_trip_psv_tsp(struct acpi_device *adev);
+int thermal_acpi_trip_psv_tc1(struct acpi_device *adev);
+int thermal_acpi_trip_psv_tc2(struct acpi_device *adev);
+int thermal_acpi_trip_gtsh(struct acpi_device *adev);
+#endif
+
 int thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id,
 			  struct thermal_trip *trip);

Signature · 1 line

2.34.1

Daniel Lezcano

author

2022-12-12 20:59 UTC #2 lore raw

PATCH 2/5 thermal/drivers/intel: Use generic trip points for intel_pch

In reply to Daniel Lezcano

From: Daniel Lezcano <[email protected]>

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/intel/intel_pch_thermal.c | 88 +++++------------------
 1 file changed, 19 insertions(+), 69 deletions(-)

drivers/thermal/intel/intel_pch_thermal.c +19 ~~-69~~

index dabf11a687a1..530fe9b38381 100644
@@ -65,6 +65,8 @@
 #define WPT_TEMP_OFFSET	(PCH_TEMP_OFFSET * MILLIDEGREE_PER_DEGREE)
 #define GET_PCH_TEMP(x)	(((x) / 2) + PCH_TEMP_OFFSET)
 
+#define PCH_MAX_TRIPS 3 /* critical, hot, passive */
+
 /* Amount of time for each cooling delay, 100ms by default for now */
 static unsigned int delay_timeout = 100;
 module_param(delay_timeout, int, 0644);

@@ -82,12 +84,7 @@ struct pch_thermal_device {
 	const struct pch_dev_ops *ops;
 	struct pci_dev *pdev;
 	struct thermal_zone_device *tzd;
-	int crt_trip_id;
-	unsigned long crt_temp;
-	int hot_trip_id;
-	unsigned long hot_temp;
-	int psv_trip_id;
-	unsigned long psv_temp;
+	struct thermal_trip trips[PCH_MAX_TRIPS];
 	bool bios_enabled;
 };

@@ -102,33 +99,22 @@ static void pch_wpt_add_acpi_psv_trip(struct pch_thermal_device *ptd,
 				      int *nr_trips)
 {
 	struct acpi_device *adev;
-
-	ptd->psv_trip_id = -1;
+	int ret;
 
 	adev = ACPI_COMPANION(&ptd->pdev->dev);
-	if (adev) {
-		unsigned long long r;
-		acpi_status status;
-
-		status = acpi_evaluate_integer(adev->handle, "_PSV", NULL,
-					       &r);
-		if (ACPI_SUCCESS(status)) {
-			unsigned long trip_temp;
-
-			trip_temp = deci_kelvin_to_millicelsius(r);
-			if (trip_temp) {
-				ptd->psv_temp = trip_temp;
-				ptd->psv_trip_id = *nr_trips;
-				++(*nr_trips);
-			}
-		}
-	}
+	if (!adev)
+		return;
+		
+	ret = thermal_acpi_trip_psv(adev, &ptd->trips[*nr_trips]);
+	if (ret)
+		return;
+
+	++(*nr_trips);
 }
 #else
 static void pch_wpt_add_acpi_psv_trip(struct pch_thermal_device *ptd,
 				      int *nr_trips)
 {
-	ptd->psv_trip_id = -1;
 
 }
 #endif

@@ -163,21 +149,19 @@ static int pch_wpt_init(struct pch_thermal_device *ptd, int *nr_trips)
 	}
 
 read_trips:
-	ptd->crt_trip_id = -1;
 	trip_temp = readw(ptd->hw_base + WPT_CTT);
 	trip_temp &= 0x1FF;
 	if (trip_temp) {
-		ptd->crt_temp = GET_WPT_TEMP(trip_temp);
-		ptd->crt_trip_id = 0;
+		ptd->trips[*nr_trips].temperature = GET_WPT_TEMP(trip_temp);
+		ptd->trips[*nr_trips].type = THERMAL_TRIP_CRITICAL;
 		++(*nr_trips);
 	}
 
-	ptd->hot_trip_id = -1;
 	trip_temp = readw(ptd->hw_base + WPT_PHL);
 	trip_temp &= 0x1FF;
 	if (trip_temp) {
-		ptd->hot_temp = GET_WPT_TEMP(trip_temp);
-		ptd->hot_trip_id = *nr_trips;
+		ptd->trips[*nr_trips].temperature = GET_WPT_TEMP(trip_temp);
+		ptd->trips[*nr_trips].type = THERMAL_TRIP_HOT;
 		++(*nr_trips);
 	}

@@ -298,39 +282,6 @@ static int pch_thermal_get_temp(struct thermal_zone_device *tzd, int *temp)
 	return	ptd->ops->get_temp(ptd, temp);
 }
 
-static int pch_get_trip_type(struct thermal_zone_device *tzd, int trip,
-			     enum thermal_trip_type *type)
-{
-	struct pch_thermal_device *ptd = tzd->devdata;
-
-	if (ptd->crt_trip_id == trip)
-		*type = THERMAL_TRIP_CRITICAL;
-	else if (ptd->hot_trip_id == trip)
-		*type = THERMAL_TRIP_HOT;
-	else if (ptd->psv_trip_id == trip)
-		*type = THERMAL_TRIP_PASSIVE;
-	else
-		return -EINVAL;
-
-	return 0;
-}
-
-static int pch_get_trip_temp(struct thermal_zone_device *tzd, int trip, int *temp)
-{
-	struct pch_thermal_device *ptd = tzd->devdata;
-
-	if (ptd->crt_trip_id == trip)
-		*temp = ptd->crt_temp;
-	else if (ptd->hot_trip_id == trip)
-		*temp = ptd->hot_temp;
-	else if (ptd->psv_trip_id == trip)
-		*temp = ptd->psv_temp;
-	else
-		return -EINVAL;
-
-	return 0;
-}
-
 static void pch_critical(struct thermal_zone_device *tzd)
 {
 	dev_dbg(&tzd->device, "%s: critical temperature reached\n", tzd->type);

@@ -338,8 +289,6 @@ static void pch_critical(struct thermal_zone_device *tzd)
 
 static struct thermal_zone_device_ops tzd_ops = {
 	.get_temp = pch_thermal_get_temp,
-	.get_trip_type = pch_get_trip_type,
-	.get_trip_temp = pch_get_trip_temp,
 	.critical = pch_critical,
 };

@@ -423,8 +372,9 @@ static int intel_pch_thermal_probe(struct pci_dev *pdev,
 	if (err)
 		goto error_cleanup;
 
-	ptd->tzd = thermal_zone_device_register(bi->name, nr_trips, 0, ptd,
-						&tzd_ops, NULL, 0, 0);
+	ptd->tzd = thermal_zone_device_register_with_trips(bi->name, ptd->trips,
+							   nr_trips, 0, ptd,
+							   &tzd_ops, NULL, 0, 0);
 	if (IS_ERR(ptd->tzd)) {
 		dev_err(&pdev->dev, "Failed to register thermal zone %s\n",
 			bi->name);

Signature · 1 line

2.34.1

Daniel Lezcano

author

2022-12-12 20:59 UTC #3 lore raw

PATCH 3/5 thermal/drivers/intel: Use generic trip points int340x

In reply to Daniel Lezcano

From: Daniel Lezcano <[email protected]>

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]>

---
 .../int340x_thermal/int340x_thermal_zone.c    | 175 ++++--------------
 .../int340x_thermal/int340x_thermal_zone.h    |  10 +-
 2 files changed, 40 insertions(+), 145 deletions(-)

drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c +38 ~~-137~~

index 228f44260b27..4f2e518caf8d 100644
@@ -37,65 +37,6 @@ static int int340x_thermal_get_zone_temp(struct thermal_zone_device *zone,
 	return 0;
 }
 
-static int int340x_thermal_get_trip_temp(struct thermal_zone_device *zone,
-					 int trip, int *temp)
-{
-	struct int34x_thermal_zone *d = zone->devdata;
-	int i;
-
-	if (trip < d->aux_trip_nr)
-		*temp = d->aux_trips[trip];
-	else if (trip == d->crt_trip_id)
-		*temp = d->crt_temp;
-	else if (trip == d->psv_trip_id)
-		*temp = d->psv_temp;
-	else if (trip == d->hot_trip_id)
-		*temp = d->hot_temp;
-	else {
-		for (i = 0; i < INT340X_THERMAL_MAX_ACT_TRIP_COUNT; i++) {
-			if (d->act_trips[i].valid &&
-			    d->act_trips[i].id == trip) {
-				*temp = d->act_trips[i].temp;
-				break;
-			}
-		}
-		if (i == INT340X_THERMAL_MAX_ACT_TRIP_COUNT)
-			return -EINVAL;
-	}
-
-	return 0;
-}
-
-static int int340x_thermal_get_trip_type(struct thermal_zone_device *zone,
-					 int trip,
-					 enum thermal_trip_type *type)
-{
-	struct int34x_thermal_zone *d = zone->devdata;
-	int i;
-
-	if (trip < d->aux_trip_nr)
-		*type = THERMAL_TRIP_PASSIVE;
-	else if (trip == d->crt_trip_id)
-		*type = THERMAL_TRIP_CRITICAL;
-	else if (trip == d->hot_trip_id)
-		*type = THERMAL_TRIP_HOT;
-	else if (trip == d->psv_trip_id)
-		*type = THERMAL_TRIP_PASSIVE;
-	else {
-		for (i = 0; i < INT340X_THERMAL_MAX_ACT_TRIP_COUNT; i++) {
-			if (d->act_trips[i].valid &&
-			    d->act_trips[i].id == trip) {
-				*type = THERMAL_TRIP_ACTIVE;
-				break;
-			}
-		}
-		if (i == INT340X_THERMAL_MAX_ACT_TRIP_COUNT)
-			return -EINVAL;
-	}
-
-	return 0;
-}
-
 static int int340x_thermal_set_trip_temp(struct thermal_zone_device *zone,
 				      int trip, int temp)
 {

@@ -109,25 +50,6 @@ static int int340x_thermal_set_trip_temp(struct thermal_zone_device *zone,
 	if (ACPI_FAILURE(status))
 		return -EIO;
 
-	d->aux_trips[trip] = temp;
-
-	return 0;
-}
-
-
-static int int340x_thermal_get_trip_hyst(struct thermal_zone_device *zone,
-		int trip, int *temp)
-{
-	struct int34x_thermal_zone *d = zone->devdata;
-	acpi_status status;
-	unsigned long long hyst;
-
-	status = acpi_evaluate_integer(d->adev->handle, "GTSH", NULL, &hyst);
-	if (ACPI_FAILURE(status))
-		*temp = 0;
-	else
-		*temp = hyst * 100;
-
 	return 0;
 }

@@ -138,58 +60,36 @@ static void int340x_thermal_critical(struct thermal_zone_device *zone)
 
 static struct thermal_zone_device_ops int340x_thermal_zone_ops = {
 	.get_temp       = int340x_thermal_get_zone_temp,
-	.get_trip_temp	= int340x_thermal_get_trip_temp,
-	.get_trip_type	= int340x_thermal_get_trip_type,
 	.set_trip_temp	= int340x_thermal_set_trip_temp,
-	.get_trip_hyst =  int340x_thermal_get_trip_hyst,
 	.critical	= int340x_thermal_critical,
 };
 
-static int int340x_thermal_get_trip_config(acpi_handle handle, char *name,
-				      int *temp)
-{
-	unsigned long long r;
-	acpi_status status;
-
-	status = acpi_evaluate_integer(handle, name, NULL, &r);
-	if (ACPI_FAILURE(status))
-		return -EIO;
-
-	*temp = deci_kelvin_to_millicelsius(r);
-
-	return 0;
-}
-
 int int340x_thermal_read_trips(struct int34x_thermal_zone *int34x_zone)
 {
-	int trip_cnt = int34x_zone->aux_trip_nr;
-	int i;
+	int trip_cnt;
+	int i, ret;
 
-	int34x_zone->crt_trip_id = -1;
-	if (!int340x_thermal_get_trip_config(int34x_zone->adev->handle, "_CRT",
-					     &int34x_zone->crt_temp))
-		int34x_zone->crt_trip_id = trip_cnt++;
+	trip_cnt = int34x_zone->aux_trip_nr;
+	
+	ret = thermal_acpi_trip_crit(int34x_zone->adev, &int34x_zone->trips[trip_cnt]);
+	if (!ret)
+		trip_cnt++;
 
-	int34x_zone->hot_trip_id = -1;
-	if (!int340x_thermal_get_trip_config(int34x_zone->adev->handle, "_HOT",
-					     &int34x_zone->hot_temp))
-		int34x_zone->hot_trip_id = trip_cnt++;
+	ret = thermal_acpi_trip_hot(int34x_zone->adev, &int34x_zone->trips[trip_cnt]);
+	if (!ret)
+		trip_cnt++;
 
-	int34x_zone->psv_trip_id = -1;
-	if (!int340x_thermal_get_trip_config(int34x_zone->adev->handle, "_PSV",
-					     &int34x_zone->psv_temp))
-		int34x_zone->psv_trip_id = trip_cnt++;
+	ret = thermal_acpi_trip_psv(int34x_zone->adev, &int34x_zone->trips[trip_cnt]);
+	if (!ret)
+		trip_cnt++;
 
 	for (i = 0; i < INT340X_THERMAL_MAX_ACT_TRIP_COUNT; i++) {
-		char name[5] = { '_', 'A', 'C', '0' + i, '\0' };
 
-		if (int340x_thermal_get_trip_config(int34x_zone->adev->handle,
-					name,
-					&int34x_zone->act_trips[i].temp))
+		ret = thermal_acpi_trip_act(int34x_zone->adev, &int34x_zone->trips[trip_cnt], i);
+		if (ret)
 			break;
 
-		int34x_zone->act_trips[i].id = trip_cnt++;
-		int34x_zone->act_trips[i].valid = true;
+		trip_cnt++;
 	}
 
 	return trip_cnt;

@@ -208,7 +108,7 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
 	acpi_status status;
 	unsigned long long trip_cnt;
 	int trip_mask = 0;
-	int ret;
+	int i, ret;
 
 	int34x_thermal_zone = kzalloc(sizeof(*int34x_thermal_zone),
 				      GFP_KERNEL);

@@ -228,32 +128,33 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
 		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;
-	else {
-		int i;
-
-		int34x_thermal_zone->aux_trips =
-			kcalloc(trip_cnt,
-				sizeof(*int34x_thermal_zone->aux_trips),
-				GFP_KERNEL);
-		if (!int34x_thermal_zone->aux_trips) {
-			ret = -ENOMEM;
-			goto err_trip_alloc;
-		}
-		trip_mask = BIT(trip_cnt) - 1;
+	if (!ACPI_FAILURE(status)) {
 		int34x_thermal_zone->aux_trip_nr = trip_cnt;
-		for (i = 0; i < trip_cnt; ++i)
-			int34x_thermal_zone->aux_trips[i] = THERMAL_TEMP_INVALID;
+		trip_mask = BIT(trip_cnt) - 1;
 	}
 
+	int34x_thermal_zone->trips = kzalloc(sizeof(*int34x_thermal_zone->trips) *
+					     (INT340X_THERMAL_MAX_TRIP_COUNT + trip_cnt),
+					      GFP_KERNEL);
+	if (!int34x_thermal_zone->trips) {
+		ret = -ENOMEM;
+		goto err_trips_alloc;
+	}
+
+	for (i = 0; i < trip_cnt; ++i) {
+		int34x_thermal_zone->trips[i].hysteresis = thermal_acpi_trip_gtsh(adev);
+		int34x_thermal_zone->trips[i].type = THERMAL_TRIP_PASSIVE;
+		int34x_thermal_zone->trips[i].temperature = THERMAL_TEMP_INVALID;
+	}
+	
 	trip_cnt = int340x_thermal_read_trips(int34x_thermal_zone);
 
 	int34x_thermal_zone->lpat_table = acpi_lpat_get_conversion_table(
 								adev->handle);
 
-	int34x_thermal_zone->zone = thermal_zone_device_register(
+	int34x_thermal_zone->zone = thermal_zone_device_register_with_trips(
 						acpi_device_bid(adev),
+						int34x_thermal_zone->trips,
 						trip_cnt,
 						trip_mask, int34x_thermal_zone,
 						int34x_thermal_zone->ops,

@@ -272,9 +173,9 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
 err_enable:
 	thermal_zone_device_unregister(int34x_thermal_zone->zone);
 err_thermal_zone:
+	kfree(int34x_thermal_zone->trips);
 	acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
-	kfree(int34x_thermal_zone->aux_trips);
-err_trip_alloc:
+err_trips_alloc:
 	kfree(int34x_thermal_zone->ops);
 err_ops_alloc:
 	kfree(int34x_thermal_zone);

@@ -287,7 +188,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->trips);
 	kfree(int34x_thermal_zone->ops);
 	kfree(int34x_thermal_zone);
 }

drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h +2 -8

index e28ab1ba5e06..0c2c8de92014 100644
@@ -10,6 +10,7 @@
 #include <acpi/acpi_lpat.h>
 
 #define INT340X_THERMAL_MAX_ACT_TRIP_COUNT	10
+#define INT340X_THERMAL_MAX_TRIP_COUNT INT340X_THERMAL_MAX_ACT_TRIP_COUNT + 3
 
 struct active_trip {
 	int temp;

@@ -19,15 +20,8 @@ struct active_trip {
 
 struct int34x_thermal_zone {
 	struct acpi_device *adev;
-	struct active_trip act_trips[INT340X_THERMAL_MAX_ACT_TRIP_COUNT];
-	unsigned long *aux_trips;
+	struct thermal_trip *trips;
 	int aux_trip_nr;
-	int psv_temp;
-	int psv_trip_id;
-	int crt_temp;
-	int crt_trip_id;
-	int hot_temp;
-	int hot_trip_id;
 	struct thermal_zone_device *zone;
 	struct thermal_zone_device_ops *ops;
 	void *priv_data;

Signature · 1 line

2.34.1

Daniel Lezcano

author

2022-12-12 20:59 UTC #4 lore raw

PATCH 4/5 thermal/drivers/intel: Use generic trip points for quark_dts

In reply to Daniel Lezcano

From: Daniel Lezcano <[email protected]>

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]>

---
 .../thermal/intel/intel_quark_dts_thermal.c   | 45 ++++++++-----------
 1 file changed, 18 insertions(+), 27 deletions(-)

drivers/thermal/intel/intel_quark_dts_thermal.c +18 ~~-27~~

index 3eafc6b0e6c3..e1b84ba59a20 100644
@@ -84,6 +84,7 @@
 #define QRK_DTS_MASK_TP_THRES		0xFF
 #define QRK_DTS_SHIFT_TP		8
 #define QRK_DTS_ID_TP_CRITICAL		0
+#define QRK_DTS_ID_TP_HOT		1
 #define QRK_DTS_SAFE_TP_THRES		105
 
 /* Thermal Sensor Register Lock */

@@ -104,6 +105,7 @@ struct soc_sensor_entry {
 	u32 store_ptps;
 	u32 store_dts_enable;
 	struct thermal_zone_device *tzone;
+	struct thermal_trip trips[QRK_MAX_DTS_TRIPS];
 };
 
 static struct soc_sensor_entry *soc_dts;

@@ -172,7 +174,7 @@ static int soc_dts_disable(struct thermal_zone_device *tzd)
 	return ret;
 }
 
-static int _get_trip_temp(int trip, int *temp)
+static int get_trip_temp(int trip, int *temp)
 {
 	int status;
 	u32 out;

@@ -197,17 +199,6 @@ static int _get_trip_temp(int trip, int *temp)
 	return 0;
 }
 
-static inline int sys_get_trip_temp(struct thermal_zone_device *tzd,
-				int trip, int *temp)
-{
-	return _get_trip_temp(trip, temp);
-}
-
-static inline int sys_get_crit_temp(struct thermal_zone_device *tzd, int *temp)
-{
-	return _get_trip_temp(QRK_DTS_ID_TP_CRITICAL, temp);
-}
-
 static int update_trip_temp(struct soc_sensor_entry *aux_entry,
 				int trip, int temp)
 {

@@ -262,17 +253,6 @@ static inline int sys_set_trip_temp(struct thermal_zone_device *tzd, int trip,
 	return update_trip_temp(tzd->devdata, trip, temp);
 }
 
-static int sys_get_trip_type(struct thermal_zone_device *thermal,
-		int trip, enum thermal_trip_type *type)
-{
-	if (trip)
-		*type = THERMAL_TRIP_HOT;
-	else
-		*type = THERMAL_TRIP_CRITICAL;
-
-	return 0;
-}
-
 static int sys_get_curr_temp(struct thermal_zone_device *tzd,
 				int *temp)
 {

@@ -315,10 +295,7 @@ static int sys_change_mode(struct thermal_zone_device *tzd,
 
 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,
-	.get_crit_temp = sys_get_crit_temp,
 	.change_mode = sys_change_mode,
 };

@@ -344,7 +321,7 @@ static void free_soc_dts(struct soc_sensor_entry *aux_entry)
 static struct soc_sensor_entry *alloc_soc_dts(void)
 {
 	struct soc_sensor_entry *aux_entry;
-	int err;
+	int err, temperature;
 	u32 out;
 	int wr_mask;

@@ -385,6 +362,20 @@ static struct soc_sensor_entry *alloc_soc_dts(void)
 			goto err_ret;
 	}
 
+	err = get_trip_temp(QRK_DTS_ID_TP_CRITICAL, &temperature);
+	if (err)
+		goto err_ret;
+
+	aux_entry->trips[QRK_DTS_ID_TP_CRITICAL].temperature = temperature;
+	aux_entry->trips[QRK_DTS_ID_TP_CRITICAL].type = THERMAL_TRIP_CRITICAL;
+
+	err = get_trip_temp(QRK_DTS_ID_TP_HOT, &temperature);
+	if (err)
+		goto err_ret;
+
+	aux_entry->trips[QRK_DTS_ID_TP_HOT].temperature = temperature;
+	aux_entry->trips[QRK_DTS_ID_TP_HOT].type = THERMAL_TRIP_HOT;
+
 	aux_entry->tzone = thermal_zone_device_register("quark_dts",
 			QRK_MAX_DTS_TRIPS,
 			wr_mask,

Signature · 1 line

2.34.1

Daniel Lezcano

author

2022-12-12 20:59 UTC #5 lore raw

PATCH 5/5 thermal/drivers/intel: Use generic trip points for processor_thermal_device_pci

In reply to Daniel Lezcano

From: Daniel Lezcano <[email protected]>

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]>

---
 .../processor_thermal_device_pci.c            | 53 ++++++++-----------
 1 file changed, 22 insertions(+), 31 deletions(-)

drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c +22 ~~-31~~

index bf1b1cdfade4..c7d50862bf56 100644
@@ -144,34 +144,6 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd, int *temp)
 	return 0;
 }
 
-static int sys_get_trip_temp(struct thermal_zone_device *tzd,
-			     int trip, int *temp)
-{
-	struct proc_thermal_pci *pci_info = tzd->devdata;
-	u32 _temp;
-
-	proc_thermal_mmio_read(pci_info, PROC_THERMAL_MMIO_THRES_0, &_temp);
-	if (!_temp) {
-		*temp = THERMAL_TEMP_INVALID;
-	} else {
-		int tjmax;
-
-		proc_thermal_mmio_read(pci_info, PROC_THERMAL_MMIO_TJMAX, &tjmax);
-		_temp = tjmax - _temp;
-		*temp = (unsigned long)_temp * 1000;
-	}
-
-	return 0;
-}
-
-static int sys_get_trip_type(struct thermal_zone_device *tzd, int trip,
-			      enum thermal_trip_type *type)
-{
-	*type = THERMAL_TRIP_PASSIVE;
-
-	return 0;
-}
-
 static int sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp)
 {
 	struct proc_thermal_pci *pci_info = tzd->devdata;

@@ -200,10 +172,26 @@ static int sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp
 	return 0;
 }
 
+static int get_trip_temp(struct proc_thermal_pci *pci_info)
+{
+	int temp, tjmax;
+
+	proc_thermal_mmio_read(pci_info, PROC_THERMAL_MMIO_THRES_0, &temp);
+	if (!temp)
+		return THERMAL_TEMP_INVALID;
+
+	proc_thermal_mmio_read(pci_info, PROC_THERMAL_MMIO_TJMAX, &tjmax);
+	temp = (tjmax - temp) * 1000;
+
+	return temp;
+}
+
+static struct thermal_trip psv_trip = {
+	.type = THERMAL_TRIP_PASSIVE,
+};
+
 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,
 };

@@ -251,7 +239,10 @@ static int proc_thermal_pci_probe(struct pci_dev *pdev, const struct pci_device_
 	if (ret)
 		goto err_ret_thermal;
 
-	pci_info->tzone = thermal_zone_device_register("TCPU_PCI", 1, 1, pci_info,
+	psv_trip.temperature = get_trip_temp(pci_info);
+	
+	pci_info->tzone = thermal_zone_device_register_with_trips("TCPU_PCI", &psv_trip,
+							1, 1, pci_info,
 							&tzone_ops,
 							&tzone_params, 0, 0);
 	if (IS_ERR(pci_info->tzone)) {

Signature · 1 line

2.34.1

Rafael J. Wysocki

maintainer

2022-12-14 14:10 UTC #6 lore raw

In reply to Daniel Lezcano

Quoted 12 lines · Daniel Lezcano

Regarding thermal, the ACPI code parsing the trip points is duplicated inside different Intel drivers. The generic ACPI thermal driver also duplicates this code.
Those ACPI parsing actions are used to define trip points and these ones are defined by, and specific to, the backend driver.
The different get_trip_* ops, also very similar between each other, are using the specific trip point structure to return the requested trip point information.

Quoted 7 lines

From there, there is nothing new, all this can be converted to the generic trip point.
The first patch provides the functions to parse trip point description from the ACPI table but based on the generic trip point structure. The other patches convert the different Intel driver to use the ACPI functions along with a migration to the generic trip point.

First off, please resend this series with CCs to linux-acpi.

Second, what does it depend on? I think that it depends on the new material that hasn't been integrated yet, doesn't it?

Quoted 19 lines

Daniel Lezcano (5):
thermal/acpi: Add ACPI trip point routines thermal/drivers/intel: Use generic trip points for intel_pch thermal/drivers/intel: Use generic trip points int340x thermal/drivers/intel: Use generic trip points for quark_dts thermal/drivers/intel: Use generic trip points for
processor_thermal_device_pci
 drivers/thermal/Kconfig                       |  13 +
 drivers/thermal/Makefile                      |   1 +
 .../int340x_thermal/int340x_thermal_zone.c    | 175 +++---------
 .../int340x_thermal/int340x_thermal_zone.h    |  10 +-
 .../processor_thermal_device_pci.c            |  53 ++--
 drivers/thermal/intel/intel_pch_thermal.c     |  88 ++-----
 .../thermal/intel/intel_quark_dts_thermal.c   |  45 ++--
 drivers/thermal/thermal_acpi.c                | 249 ++++++++++++++++++
 include/linux/thermal.h                       |  15 ++
 9 files changed, 377 insertions(+), 272 deletions(-)
 create mode 100644 drivers/thermal/thermal_acpi.c

Daniel Lezcano

author

2022-12-14 14:42 UTC #7 lore raw

Quoted 22 lines · Rafael J. Wysocki

Quoted 12 lines · Daniel Lezcano
Regarding thermal, the ACPI code parsing the trip points is duplicated inside different Intel drivers. The generic ACPI thermal driver also duplicates this code.
Those ACPI parsing actions are used to define trip points and these ones are defined by, and specific to, the backend driver.
The different get_trip_* ops, also very similar between each other, are using the specific trip point structure to return the requested trip point information.
Quoted 7 lines
From there, there is nothing new, all this can be converted to the
generic trip point.
The first patch provides the functions to parse trip point description from the ACPI table but based on the generic trip point structure. The other patches convert the different Intel driver to use the ACPI functions along with a migration to the generic trip point.

Quoted 4 lines

First off, please resend this series with CCs to linux-acpi.
Second, what does it depend on? I think that it depends on the new material that hasn't been integrated yet, doesn't it?

Right, I assumed the generic trip points series was in the way to be integrated

Signature · 5 lines

<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