[PATCH v1 00/14] thermal: Eliminate trip IDs from thermal driver interface

[PATCH v1 00/14] thermal: Eliminate trip IDs from thermal driver interface

[Rafael J. Wysocki]

Hi Everyone,

Trip IDs, which should be internal to the thermal core, are still used in the
thermal driver interface in some places, but fortunately it does not take too
much effort to get rid of them.

First of all, the .set_trip_temp() zone callback uses a trip ID as one of its
arguments, but this isn't really necessary and there are a few weaknesses
related to doing that.  Please see the changelog of patch [04/14] for details.

Apart from that, several drivers use the thermal_zone_get_trip() helper
function for trip point lookup which can be differently.  Patches [06-12/14]
address those cases (please see the individual patch changelogs for details).

The remaining patches in the series are preliminary changes or cleanups.

The series is based on the two cleanup series posted previously:

https://lore.kernel.org/linux-pm/5794974.DvuYhMxLoT@kreacher/T/#t
https://lore.kernel.org/linux-pm/12458899.O9o76ZdvQC@kreacher/T/#t

that have been included into the linux-next thermal material.

This series is available from the thermal-core-experimental branch
in linux-pm.git.

Thanks!

[PATCH v1 01/14] thermal: imx: Drop critical trip check from imx_set_trip_temp()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Because the IMX thermal driver does not flag its critical trip as
writable, imx_set_trip_temp() will never be invoked for it and so the
critical trip check can be dropped from there.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/imx_thermal.c |    9 ---------
 1 file changed, 9 deletions(-)

Index: linux-pm/drivers/thermal/imx_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/imx_thermal.c
+++ linux-pm/drivers/thermal/imx_thermal.c
@@ -335,21 +335,12 @@ static int imx_set_trip_temp(struct ther
 			     int temp)
 {
 	struct imx_thermal_data *data = thermal_zone_device_priv(tz);
-	struct thermal_trip trip;
 	int ret;
 
 	ret = pm_runtime_resume_and_get(data->dev);
 	if (ret < 0)
 		return ret;
 
-	ret = __thermal_zone_get_trip(tz, trip_id, &trip);
-	if (ret)
-		return ret;
-
-	/* do not allow changing critical threshold */
-	if (trip.type == THERMAL_TRIP_CRITICAL)
-		return -EPERM;
-
 	/* do not allow passive to be set higher than critical */
 	if (temp < 0 || temp > trips[IMX_TRIP_CRITICAL].temperature)
 		return -EINVAL;

[PATCH v1 02/14] thermal: helpers: Introduce thermal_trip_is_bound_to_cdev()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Introduce a new helper function thermal_trip_is_bound_to_cdev() for
checking whether or not a given trip point has been bound to a given
cooling device.

The primary user of it will be the Tegra thermal driver.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_helpers.c |   47 ++++++++++++++++++++++++++++----------
 include/linux/thermal.h           |    3 ++
 2 files changed, 38 insertions(+), 12 deletions(-)

Index: linux-pm/drivers/thermal/thermal_helpers.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_helpers.c
+++ linux-pm/drivers/thermal/thermal_helpers.c
@@ -39,30 +39,53 @@ int get_tz_trend(struct thermal_zone_dev
 	return trend;
 }
 
+static struct thermal_instance *get_instance(struct thermal_zone_device *tz,
+					     struct thermal_cooling_device *cdev,
+					     const struct thermal_trip *trip)
+{
+	struct thermal_instance *ti;
+
+	list_for_each_entry(ti, &tz->thermal_instances, tz_node) {
+		if (ti->trip == trip && ti->cdev == cdev)
+			return ti;
+	}
+
+	return NULL;
+}
+
+bool thermal_trip_is_bound_to_cdev(struct thermal_zone_device *tz,
+				   const struct thermal_trip *trip,
+				   struct thermal_cooling_device *cdev)
+{
+	bool ret;
+
+	mutex_lock(&tz->lock);
+	mutex_lock(&cdev->lock);
+
+	ret = !!get_instance(tz, cdev, trip);
+
+	mutex_unlock(&cdev->lock);
+	mutex_unlock(&tz->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(thermal_trip_is_bound_to_cdev);
+
 struct thermal_instance *
 get_thermal_instance(struct thermal_zone_device *tz,
 		     struct thermal_cooling_device *cdev, int trip_index)
 {
-	struct thermal_instance *pos = NULL;
-	struct thermal_instance *target_instance = NULL;
-	const struct thermal_trip *trip;
+	struct thermal_instance *ti;
 
 	mutex_lock(&tz->lock);
 	mutex_lock(&cdev->lock);
 
-	trip = &tz->trips[trip_index].trip;
-
-	list_for_each_entry(pos, &tz->thermal_instances, tz_node) {
-		if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev) {
-			target_instance = pos;
-			break;
-		}
-	}
+	ti = get_instance(tz, cdev, &tz->trips[trip_index].trip);
 
 	mutex_unlock(&cdev->lock);
 	mutex_unlock(&tz->lock);
 
-	return target_instance;
+	return ti;
 }
 EXPORT_SYMBOL(get_thermal_instance);
 
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -269,6 +269,9 @@ struct thermal_zone_device *thermal_zone
 int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
 int thermal_zone_get_slope(struct thermal_zone_device *tz);
 int thermal_zone_get_offset(struct thermal_zone_device *tz);
+bool thermal_trip_is_bound_to_cdev(struct thermal_zone_device *tz,
+				   const struct thermal_trip *trip,
+				   struct thermal_cooling_device *cdev);
 
 int thermal_zone_device_enable(struct thermal_zone_device *tz);
 int thermal_zone_device_disable(struct thermal_zone_device *tz);

[PATCH v1 03/14] thermal: trip: Add conversion macros for thermal trip priv field

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Some drivers will need to store integers in the priv field of struct
thermal_trip, so add conversion macros for doing this in a consistent
way and switch over the int340x_thermal driver that already does it and
uses custom conversion functions to using the new macros.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c |   14 +----------
 include/linux/thermal.h                                      |    3 ++
 2 files changed, 5 insertions(+), 12 deletions(-)

Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -79,6 +79,9 @@ struct thermal_trip {
 #define THERMAL_TRIP_FLAG_RW	(THERMAL_TRIP_FLAG_RW_TEMP | \
 				 THERMAL_TRIP_FLAG_RW_HYST)
 
+#define THERMAL_TRIP_PRIV_TO_INT(_val_)	(uintptr_t)(_val_)
+#define THERMAL_INT_TO_TRIP_PRIV(_val_)	(void *)(uintptr_t)(_val_)
+
 struct thermal_zone_device;
 
 struct thermal_zone_device_ops {
Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -62,16 +62,6 @@ static void int340x_thermal_critical(str
 		thermal_zone_device_type(zone));
 }
 
-static inline void *int_to_trip_priv(int i)
-{
-	return (void *)(long)i;
-}
-
-static inline int trip_priv_to_int(const struct thermal_trip *trip)
-{
-	return (long)trip->priv;
-}
-
 static int int340x_thermal_read_trips(struct acpi_device *zone_adev,
 				      struct thermal_trip *zone_trips,
 				      int trip_cnt)
@@ -106,7 +96,7 @@ static int int340x_thermal_read_trips(st
 			break;
 
 		zone_trips[trip_cnt].type = THERMAL_TRIP_ACTIVE;
-		zone_trips[trip_cnt].priv = int_to_trip_priv(i);
+		zone_trips[trip_cnt].priv = THERMAL_INT_TO_TRIP_PRIV(i);
 		trip_cnt++;
 	}
 
@@ -224,7 +214,7 @@ static int int340x_update_one_trip(struc
 		break;
 	case THERMAL_TRIP_ACTIVE:
 		err = thermal_acpi_active_trip_temp(zone_adev,
-						    trip_priv_to_int(trip),
+						    THERMAL_TRIP_PRIV_TO_INT(trip->priv),
 						    &temp);
 		break;
 	default:

[PATCH v1 04/14] thermal: trip: Pass trip pointer to .set_trip_temp() thermal zone callback

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Out of several drivers implementing the .set_trip_temp() thermal zone
operation, two don't actually use the trip ID argument passed to it,
two call __thermal_zone_get_trip() to get a struct thermal_trip
corresponding to the given trip ID, and the other use the trip ID as an
index into their own data structures with the assumption that it will
always match the ordering of entries in the trips table passed to the
core during thermal zone registration, which is fragile and not really
guaranteed.

Even though the trip IDs used by core are in fact their indices in the
trips table passed to it by the thermal zone creator, that is purely a
matter of convenience and should not be relied on for correctness.

For this reason, modify trip_point_temp_store() to pass a (const) trip
pointer to .set_trip_temp() and adjust the drivers implementing it
accordingly.

This helps to simplify the drivers invoking __thermal_zone_get_trip()
from their .set_trip_temp() callback functions because they will not
need to do it now and the other drivers can store their internal
trip indices in the priv field in struct thermal_trip and their
.set_trip_temp() callback functions can get those indices from there.

The intel_quark_dts thermal driver can instead use the trip type to
determine the requisite trip index.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/tt.c                          |    2 
 drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c         |    8 +--
 drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c |    3 -
 drivers/thermal/intel/intel_quark_dts_thermal.c                      |   26 +++++++---
 drivers/thermal/intel/intel_soc_dts_iosf.c                           |   15 +++--
 drivers/thermal/intel/x86_pkg_temp_thermal.c                         |    9 ++-
 drivers/thermal/qcom/qcom-spmi-temp-alarm.c                          |   10 +--
 drivers/thermal/tegra/soctherm.c                                     |   14 +----
 drivers/thermal/thermal_sysfs.c                                      |    2 
 include/linux/thermal.h                                              |    3 -
 10 files changed, 54 insertions(+), 38 deletions(-)

Index: linux-pm/drivers/thermal/thermal_sysfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_sysfs.c
+++ linux-pm/drivers/thermal/thermal_sysfs.c
@@ -113,7 +113,7 @@ trip_point_temp_store(struct device *dev
 
 	if (temp != trip->temperature) {
 		if (tz->ops.set_trip_temp) {
-			ret = tz->ops.set_trip_temp(tz, trip_id, temp);
+			ret = tz->ops.set_trip_temp(tz, trip, temp);
 			if (ret)
 				goto unlock;
 		}
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -93,7 +93,8 @@ struct thermal_zone_device_ops {
 	int (*set_trips) (struct thermal_zone_device *, int, int);
 	int (*change_mode) (struct thermal_zone_device *,
 		enum thermal_device_mode);
-	int (*set_trip_temp) (struct thermal_zone_device *, int, int);
+	int (*set_trip_temp) (struct thermal_zone_device *,
+			      const struct thermal_trip *, int);
 	int (*get_crit_temp) (struct thermal_zone_device *, int *);
 	int (*set_emul_temp) (struct thermal_zone_device *, int);
 	int (*get_trend) (struct thermal_zone_device *,
Index: linux-pm/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
===================================================================
--- linux-pm.orig/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
+++ linux-pm/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
@@ -638,7 +638,7 @@ out:
 }
 
 static int iwl_mvm_tzone_set_trip_temp(struct thermal_zone_device *device,
-				       int trip, int temp)
+				       const struct thermal_trip *trip, int temp)
 {
 	struct iwl_mvm *mvm = thermal_zone_device_priv(device);
 	int ret;
Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -39,13 +39,14 @@ static int int340x_thermal_get_zone_temp
 }
 
 static int int340x_thermal_set_trip_temp(struct thermal_zone_device *zone,
-					 int trip, int temp)
+					 const struct thermal_trip *trip, int temp)
 {
 	struct int34x_thermal_zone *d = thermal_zone_device_priv(zone);
-	char name[] = {'P', 'A', 'T', '0' + trip, '\0'};
+	unsigned int trip_index = THERMAL_TRIP_PRIV_TO_INT(trip->priv);
+	char name[] = {'P', 'A', 'T', '0' + trip_index, '\0'};
 	acpi_status status;
 
-	if (trip > 9)
+	if (trip_index > 9)
 		return -EINVAL;
 
 	status = acpi_execute_simple_method(d->adev->handle, name,
@@ -144,6 +145,7 @@ struct int34x_thermal_zone *int340x_ther
 		zone_trips[i].type = THERMAL_TRIP_PASSIVE;
 		zone_trips[i].temperature = THERMAL_TEMP_INVALID;
 		zone_trips[i].flags |= THERMAL_TRIP_FLAG_RW_TEMP;
+		zone_trips[i].priv = THERMAL_INT_TO_TRIP_PRIV(i);
 	}
 
 	trip_cnt = int340x_thermal_read_trips(adev, zone_trips, trip_cnt);
Index: linux-pm/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
+++ linux-pm/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
@@ -193,7 +193,8 @@ static int sys_get_curr_temp(struct ther
 	return 0;
 }
 
-static int sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp)
+static int sys_set_trip_temp(struct thermal_zone_device *tzd,
+			     const struct thermal_trip *trip, int temp)
 {
 	struct proc_thermal_pci *pci_info = thermal_zone_device_priv(tzd);
 	int tjmax, _temp;
Index: linux-pm/drivers/thermal/intel/intel_quark_dts_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/intel_quark_dts_thermal.c
+++ linux-pm/drivers/thermal/intel/intel_quark_dts_thermal.c
@@ -195,7 +195,7 @@ static int get_trip_temp(int trip)
 }
 
 static int update_trip_temp(struct soc_sensor_entry *aux_entry,
-				int trip, int temp)
+				int trip_index, int temp)
 {
 	u32 out;
 	u32 temp_out;
@@ -230,9 +230,9 @@ static int update_trip_temp(struct soc_s
 	 */
 	temp_out = temp + QRK_DTS_TEMP_BASE;
 	out = (store_ptps & ~(QRK_DTS_MASK_TP_THRES <<
-		(trip * QRK_DTS_SHIFT_TP)));
+		(trip_index * QRK_DTS_SHIFT_TP)));
 	out |= (temp_out & QRK_DTS_MASK_TP_THRES) <<
-		(trip * QRK_DTS_SHIFT_TP);
+		(trip_index * QRK_DTS_SHIFT_TP);
 
 	ret = iosf_mbi_write(QRK_MBI_UNIT_RMU, MBI_REG_WRITE,
 			     QRK_DTS_REG_OFFSET_PTPS, out);
@@ -242,10 +242,24 @@ failed:
 	return ret;
 }
 
-static inline int sys_set_trip_temp(struct thermal_zone_device *tzd, int trip,
-				int temp)
+static inline int sys_set_trip_temp(struct thermal_zone_device *tzd,
+				    const struct thermal_trip *trip,
+				    int temp)
 {
-	return update_trip_temp(thermal_zone_device_priv(tzd), trip, temp);
+	unsigned int trip_index;
+
+	switch (trip->type) {
+	case THERMAL_TRIP_HOT:
+		trip_index = QRK_DTS_ID_TP_HOT;
+		break;
+	case THERMAL_TRIP_CRITICAL:
+		trip_index = QRK_DTS_ID_TP_CRITICAL;
+		break;
+	default
+		return -EINVAL;
+	}
+
+	return update_trip_temp(thermal_zone_device_priv(tzd), trip_index, temp);
 }
 
 static int sys_get_curr_temp(struct thermal_zone_device *tzd,
Index: linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/intel_soc_dts_iosf.c
+++ linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.c
@@ -129,18 +129,20 @@ err_restore_ptps:
 	return status;
 }
 
-static int sys_set_trip_temp(struct thermal_zone_device *tzd, int trip,
+static int sys_set_trip_temp(struct thermal_zone_device *tzd,
+			     const struct thermal_trip *trip,
 			     int temp)
 {
 	struct intel_soc_dts_sensor_entry *dts = thermal_zone_device_priv(tzd);
 	struct intel_soc_dts_sensors *sensors = dts->sensors;
+	unsigned int trip_index = THERMAL_TRIP_PRIV_TO_INT(trip->priv);
 	int status;
 
 	if (temp > sensors->tj_max)
 		return -EINVAL;
 
 	mutex_lock(&sensors->dts_update_lock);
-	status = update_trip_temp(sensors, trip, temp);
+	status = update_trip_temp(sensors, trip_index, temp);
 	mutex_unlock(&sensors->dts_update_lock);
 
 	return status;
@@ -293,11 +295,12 @@ static void dts_trips_reset(struct intel
 }
 
 static void set_trip(struct thermal_trip *trip, enum thermal_trip_type type,
-		     u8 flags, int temp)
+		     u8 flags, int temp, unsigned int index)
 {
 	trip->type = type;
 	trip->flags = flags;
 	trip->temperature = temp;
+	trip->priv = THERMAL_INT_TO_TRIP_PRIV(index);
 }
 
 struct intel_soc_dts_sensors *
@@ -332,7 +335,7 @@ intel_soc_dts_iosf_init(enum intel_soc_d
 		sensors->soc_dts[i].sensors = sensors;
 
 		set_trip(&trips[i][0], THERMAL_TRIP_PASSIVE,
-			 THERMAL_TRIP_FLAG_RW_TEMP, 0);
+			 THERMAL_TRIP_FLAG_RW_TEMP, 0, 0);
 
 		ret = update_trip_temp(sensors, 0, 0);
 		if (ret)
@@ -340,10 +343,10 @@ intel_soc_dts_iosf_init(enum intel_soc_d
 
 		if (critical_trip) {
 			temp = sensors->tj_max - crit_offset;
-			set_trip(&trips[i][1], THERMAL_TRIP_CRITICAL, 0, temp);
+			set_trip(&trips[i][1], THERMAL_TRIP_CRITICAL, 0, temp, 1);
 		} else {
 			set_trip(&trips[i][1], THERMAL_TRIP_PASSIVE,
-				 THERMAL_TRIP_FLAG_RW_TEMP, 0);
+				 THERMAL_TRIP_FLAG_RW_TEMP, 0, 1);
 			temp = 0;
 		}
 
Index: linux-pm/drivers/thermal/intel/x86_pkg_temp_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/x86_pkg_temp_thermal.c
+++ linux-pm/drivers/thermal/intel/x86_pkg_temp_thermal.c
@@ -119,9 +119,11 @@ static int sys_get_curr_temp(struct ther
 }
 
 static int
-sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp)
+sys_set_trip_temp(struct thermal_zone_device *tzd,
+		  const struct thermal_trip *trip, int temp)
 {
 	struct zone_device *zonedev = thermal_zone_device_priv(tzd);
+	unsigned int trip_index = THERMAL_TRIP_PRIV_TO_INT(trip->priv);
 	u32 l, h, mask, shift, intr;
 	int tj_max, val, ret;
 
@@ -132,7 +134,7 @@ sys_set_trip_temp(struct thermal_zone_de
 
 	val = (tj_max - temp)/1000;
 
-	if (trip >= MAX_NUMBER_OF_TRIPS || val < 0 || val > 0x7f)
+	if (trip_index >= MAX_NUMBER_OF_TRIPS || val < 0 || val > 0x7f)
 		return -EINVAL;
 
 	ret = rdmsr_on_cpu(zonedev->cpu, MSR_IA32_PACKAGE_THERM_INTERRUPT,
@@ -140,7 +142,7 @@ sys_set_trip_temp(struct thermal_zone_de
 	if (ret < 0)
 		return ret;
 
-	if (trip) {
+	if (trip_index) {
 		mask = THERM_MASK_THRESHOLD1;
 		shift = THERM_SHIFT_THRESHOLD1;
 		intr = THERM_INT_THRESHOLD1_ENABLE;
@@ -296,6 +298,7 @@ static int pkg_temp_thermal_trips_init(i
 
 		trips[i].type = THERMAL_TRIP_PASSIVE;
 		trips[i].flags |= THERMAL_TRIP_FLAG_RW_TEMP;
+		trips[i].priv = THERMAL_INT_TO_TRIP_PRIV(i);
 
 		pr_debug("%s: cpu=%d, trip=%d, temp=%d\n",
 			 __func__, cpu, i, trips[i].temperature);
Index: linux-pm/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
===================================================================
--- linux-pm.orig/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
+++ linux-pm/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
@@ -261,17 +261,13 @@ skip:
 	return qpnp_tm_write(chip, QPNP_TM_REG_SHUTDOWN_CTRL1, reg);
 }
 
-static int qpnp_tm_set_trip_temp(struct thermal_zone_device *tz, int trip_id, int temp)
+static int qpnp_tm_set_trip_temp(struct thermal_zone_device *tz,
+				 const struct thermal_trip *trip, int temp)
 {
 	struct qpnp_tm_chip *chip = thermal_zone_device_priv(tz);
-	struct thermal_trip trip;
 	int ret;
 
-	ret = __thermal_zone_get_trip(chip->tz_dev, trip_id, &trip);
-	if (ret)
-		return ret;
-
-	if (trip.type != THERMAL_TRIP_CRITICAL)
+	if (trip->type != THERMAL_TRIP_CRITICAL)
 		return 0;
 
 	mutex_lock(&chip->lock);
Index: linux-pm/drivers/thermal/tegra/soctherm.c
===================================================================
--- linux-pm.orig/drivers/thermal/tegra/soctherm.c
+++ linux-pm/drivers/thermal/tegra/soctherm.c
@@ -582,11 +582,11 @@ static int tsensor_group_thermtrip_get(s
 	return temp;
 }
 
-static int tegra_thermctl_set_trip_temp(struct thermal_zone_device *tz, int trip_id, int temp)
+static int tegra_thermctl_set_trip_temp(struct thermal_zone_device *tz,
+					const struct thermal_trip *trip, int temp)
 {
 	struct tegra_thermctl_zone *zone = thermal_zone_device_priv(tz);
 	struct tegra_soctherm *ts = zone->ts;
-	struct thermal_trip trip;
 	const struct tegra_tsensor_group *sg = zone->sg;
 	struct device *dev = zone->dev;
 	int ret;
@@ -594,11 +594,7 @@ static int tegra_thermctl_set_trip_temp(
 	if (!tz)
 		return -EINVAL;
 
-	ret = __thermal_zone_get_trip(tz, trip_id, &trip);
-	if (ret)
-		return ret;
-
-	if (trip.type == THERMAL_TRIP_CRITICAL) {
+	if (trip->type == THERMAL_TRIP_CRITICAL) {
 		/*
 		 * If thermtrips property is set in DT,
 		 * doesn't need to program critical type trip to HW,
@@ -609,7 +605,7 @@ static int tegra_thermctl_set_trip_temp(
 		else
 			return 0;
 
-	} else if (trip.type == THERMAL_TRIP_HOT) {
+	} else if (trip->type == THERMAL_TRIP_HOT) {
 		int i;
 
 		for (i = 0; i < THROTTLE_SIZE; i++) {
@@ -620,7 +616,7 @@ static int tegra_thermctl_set_trip_temp(
 				continue;
 
 			cdev = ts->throt_cfgs[i].cdev;
-			if (get_thermal_instance(tz, cdev, trip_id))
+			if (thermal_trip_is_bound_to_cdev(tz, trip, cdev))
 				stc = find_throttle_cfg_by_name(ts, cdev->type);
 			else
 				continue;

[PATCH v1 05/14] thermal: trip: Fold __thermal_zone_get_trip() into its caller

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Because __thermal_zone_get_trip() is only called by thermal_zone_get_trip()
now, fold the former into the latter.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_trip.c |   18 ++++--------------
 include/linux/thermal.h        |    2 --
 2 files changed, 4 insertions(+), 16 deletions(-)

Index: linux-pm/drivers/thermal/thermal_trip.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_trip.c
+++ linux-pm/drivers/thermal/thermal_trip.c
@@ -114,27 +114,17 @@ void thermal_zone_set_trips(struct therm
 		dev_err(&tz->device, "Failed to set trips: %d\n", ret);
 }
 
-int __thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id,
-			    struct thermal_trip *trip)
-{
-	if (!tz || trip_id < 0 || trip_id >= tz->num_trips || !trip)
-		return -EINVAL;
-
-	*trip = tz->trips[trip_id].trip;
-	return 0;
-}
-EXPORT_SYMBOL_GPL(__thermal_zone_get_trip);
-
 int thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id,
 			  struct thermal_trip *trip)
 {
-	int ret;
+	if (!tz || !trip || trip_id < 0 || trip_id >= tz->num_trips)
+		return -EINVAL;
 
 	mutex_lock(&tz->lock);
-	ret = __thermal_zone_get_trip(tz, trip_id, trip);
+	*trip = tz->trips[trip_id].trip;
 	mutex_unlock(&tz->lock);
 
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(thermal_zone_get_trip);
 
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -202,8 +202,6 @@ static inline void devm_thermal_of_zone_
 }
 #endif
 
-int __thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id,
-			    struct thermal_trip *trip);
 int thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id,
 			  struct thermal_trip *trip);
 int for_each_thermal_trip(struct thermal_zone_device *tz,

[PATCH v1 06/14] thermal: broadcom: Use thermal_zone_get_crit_temp() in bcm2835_thermal_probe()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Modify the bcm2835 thermal driver to use thermal_zone_get_crit_temp() in
bcm2835_thermal_probe() instead of relying on the assumption that the
critical trip index will always be 0.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/broadcom/bcm2835_thermal.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Index: linux-pm/drivers/thermal/broadcom/bcm2835_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/broadcom/bcm2835_thermal.c
+++ linux-pm/drivers/thermal/broadcom/bcm2835_thermal.c
@@ -222,8 +222,7 @@ static int bcm2835_thermal_probe(struct
 	 */
 	val = readl(data->regs + BCM2835_TS_TSENSCTL);
 	if (!(val & BCM2835_TS_TSENSCTL_RSTB)) {
-		struct thermal_trip trip;
-		int offset, slope;
+		int offset, slope, crit_temp;
 
 		slope = thermal_zone_get_slope(tz);
 		offset = thermal_zone_get_offset(tz);
@@ -231,7 +230,7 @@ static int bcm2835_thermal_probe(struct
 		 * For now we deal only with critical, otherwise
 		 * would need to iterate
 		 */
-		err = thermal_zone_get_trip(tz, 0, &trip);
+		err = thermal_zone_get_crit_temp(tz, &crit_temp);
 		if (err < 0) {
 			dev_err(&pdev->dev,
 				"Not able to read trip_temp: %d\n",
@@ -248,7 +247,7 @@ static int bcm2835_thermal_probe(struct
 		val |= (0xFE << BCM2835_TS_TSENSCTL_RSTDELAY_SHIFT);
 
 		/*  trip_adc value from info */
-		val |= bcm2835_thermal_temp2adc(trip.temperature,
+		val |= bcm2835_thermal_temp2adc(crit_temp,
 						offset,
 						slope)
 			<< BCM2835_TS_TSENSCTL_THOLD_SHIFT;

[PATCH v1 07/14] thermal: hisi: Use thermal_zone_for_each_trip() in hisi_thermal_register_sensor()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Modify hisi_thermal_register_sensor() to use thermal_zone_for_each_trip()
for walking trip points instead of iterating over trip indices and using
thermal_zone_get_trip() to get from a trip index to struct thermal_trip.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/hisi_thermal.c |   22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

Index: linux-pm/drivers/thermal/hisi_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/hisi_thermal.c
+++ linux-pm/drivers/thermal/hisi_thermal.c
@@ -470,6 +470,18 @@ static irqreturn_t hisi_thermal_alarm_ir
 	return IRQ_HANDLED;
 }
 
+static int hisi_trip_walk_cb(struct thermal_trip *trip, void *arg)
+{
+	struct hisi_thermal_sensor *sensor = arg;
+
+	if (trip->type != THERMAL_TRIP_PASSIVE)
+		return 0;
+
+	sensor->thres_temp = trip->temperature;
+	/* Return nonzero to terminate the search. */
+	return 1;
+}
+
 static int hisi_thermal_register_sensor(struct platform_device *pdev,
 					struct hisi_thermal_sensor *sensor)
 {
@@ -487,15 +499,7 @@ static int hisi_thermal_register_sensor(
 		return ret;
 	}
 
-	for (i = 0; i < thermal_zone_get_num_trips(sensor->tzd); i++) {
-
-		thermal_zone_get_trip(sensor->tzd, i, &trip);
-
-		if (trip.type == THERMAL_TRIP_PASSIVE) {
-			sensor->thres_temp = trip.temperature;
-			break;
-		}
-	}
+	thermal_zone_for_each_trip(sensor->tzd, hisi_trip_walk_cb, sensor);
 
 	return 0;
 }

[PATCH v1 08/14] thermal: qcom: Use thermal_zone_get_crit_temp() in qpnp_tm_init()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Modify qpnp_tm_init() to use thermal_zone_get_crit_temp() to get the
critical trip temperature instead of iterating over trip indices and
using thermal_zone_get_trip() to get from a trip index to struct
thermal_trip until it finds the critical one.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/qcom/qcom-spmi-temp-alarm.c |   22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

Index: linux-pm/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
===================================================================
--- linux-pm.orig/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
+++ linux-pm/drivers/thermal/qcom/qcom-spmi-temp-alarm.c
@@ -291,24 +291,6 @@ static irqreturn_t qpnp_tm_isr(int irq,
 	return IRQ_HANDLED;
 }
 
-static int qpnp_tm_get_critical_trip_temp(struct qpnp_tm_chip *chip)
-{
-	struct thermal_trip trip;
-	int i, ret;
-
-	for (i = 0; i < thermal_zone_get_num_trips(chip->tz_dev); i++) {
-
-		ret = thermal_zone_get_trip(chip->tz_dev, i, &trip);
-		if (ret)
-			continue;
-
-		if (trip.type == THERMAL_TRIP_CRITICAL)
-			return trip.temperature;
-	}
-
-	return THERMAL_TEMP_INVALID;
-}
-
 /*
  * This function initializes the internal temp value based on only the
  * current thermal stage and threshold. Setup threshold control and
@@ -343,7 +325,9 @@ static int qpnp_tm_init(struct qpnp_tm_c
 
 	mutex_unlock(&chip->lock);
 
-	crit_temp = qpnp_tm_get_critical_trip_temp(chip);
+	ret = thermal_zone_get_crit_temp(chip->tz_dev, &crit_temp);
+	if (ret)
+		crit_temp = THERMAL_TEMP_INVALID;
 
 	mutex_lock(&chip->lock);
 

[PATCH v1 09/14] thermal: tegra: Introduce struct trip_temps for critical and hot trips

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Introduce a helper structure, struct trip_temps, for storing the
temperatures of the critical and hot trip points.

This helps to make the code in tegra_tsensor_get_hw_channel_trips()
somewhat cleaner and will be useful subsequently in eliminating
iteration over trip indices from the driver.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/tegra/tegra30-tsensor.c |   34 ++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

Index: linux-pm/drivers/thermal/tegra/tegra30-tsensor.c
===================================================================
--- linux-pm.orig/drivers/thermal/tegra/tegra30-tsensor.c
+++ linux-pm/drivers/thermal/tegra/tegra30-tsensor.c
@@ -303,8 +303,13 @@ stop_channel:
 	return 0;
 }
 
+struct trip_temps {
+	int hot_trip;
+	int crit_trip;
+};
+
 static void tegra_tsensor_get_hw_channel_trips(struct thermal_zone_device *tzd,
-					       int *hot_trip, int *crit_trip)
+					       struct trip_temps *temps)
 {
 	unsigned int i;
 
@@ -312,8 +317,8 @@ static void tegra_tsensor_get_hw_channel
 	 * 90C is the maximal critical temperature of all Tegra30 SoC variants,
 	 * use it for the default trip if unspecified in a device-tree.
 	 */
-	*hot_trip  = 85000;
-	*crit_trip = 90000;
+	temps->hot_trip  = 85000;
+	temps->crit_trip = 90000;
 
 	for (i = 0; i < thermal_zone_get_num_trips(tzd); i++) {
 
@@ -322,14 +327,14 @@ static void tegra_tsensor_get_hw_channel
 		thermal_zone_get_trip(tzd, i, &trip);
 
 		if (trip.type == THERMAL_TRIP_HOT)
-			*hot_trip = trip.temperature;
+			temps->hot_trip = trip.temperature;
 
 		if (trip.type == THERMAL_TRIP_CRITICAL)
-			*crit_trip = trip.temperature;
+			temps->crit_trip = trip.temperature;
 	}
 
 	/* clamp hardware trips to the calibration limits */
-	*hot_trip = clamp(*hot_trip, 25000, 90000);
+	temps->hot_trip = clamp(temps->hot_trip, 25000, 90000);
 
 	/*
 	 * Kernel will perform a normal system shut down if it will
@@ -338,7 +343,7 @@ static void tegra_tsensor_get_hw_channel
 	 * shut down gracefully before sending signal to the Power
 	 * Management controller.
 	 */
-	*crit_trip = clamp(*crit_trip + 5000, 25000, 90000);
+	temps->crit_trip = clamp(temps->crit_trip + 5000, 25000, 90000);
 }
 
 static int tegra_tsensor_enable_hw_channel(const struct tegra_tsensor *ts,
@@ -346,7 +351,8 @@ static int tegra_tsensor_enable_hw_chann
 {
 	const struct tegra_tsensor_channel *tsc = &ts->ch[id];
 	struct thermal_zone_device *tzd = tsc->tzd;
-	int err, hot_trip = 0, crit_trip = 0;
+	struct trip_temps temps = { 0 };
+	int err;
 	u32 val;
 
 	if (!tzd) {
@@ -357,24 +363,24 @@ static int tegra_tsensor_enable_hw_chann
 		return 0;
 	}
 
-	tegra_tsensor_get_hw_channel_trips(tzd, &hot_trip, &crit_trip);
+	tegra_tsensor_get_hw_channel_trips(tzd, &temps);
 
 	dev_info_once(ts->dev, "ch%u: PMC emergency shutdown trip set to %dC\n",
-		      id, DIV_ROUND_CLOSEST(crit_trip, 1000));
+		      id, DIV_ROUND_CLOSEST(temps.crit_trip, 1000));
 
-	hot_trip  = tegra_tsensor_temp_to_counter(ts, hot_trip);
-	crit_trip = tegra_tsensor_temp_to_counter(ts, crit_trip);
+	temps.hot_trip  = tegra_tsensor_temp_to_counter(ts, temps.hot_trip);
+	temps.crit_trip = tegra_tsensor_temp_to_counter(ts, temps.crit_trip);
 
 	/* program LEVEL2 counter threshold */
 	val = readl_relaxed(tsc->regs + TSENSOR_SENSOR0_CONFIG1);
 	val &= ~TSENSOR_SENSOR0_CONFIG1_TH2;
-	val |= FIELD_PREP(TSENSOR_SENSOR0_CONFIG1_TH2, hot_trip);
+	val |= FIELD_PREP(TSENSOR_SENSOR0_CONFIG1_TH2, temps.hot_trip);
 	writel_relaxed(val, tsc->regs + TSENSOR_SENSOR0_CONFIG1);
 
 	/* program LEVEL3 counter threshold */
 	val = readl_relaxed(tsc->regs + TSENSOR_SENSOR0_CONFIG2);
 	val &= ~TSENSOR_SENSOR0_CONFIG2_TH3;
-	val |= FIELD_PREP(TSENSOR_SENSOR0_CONFIG2_TH3, crit_trip);
+	val |= FIELD_PREP(TSENSOR_SENSOR0_CONFIG2_TH3, temps.crit_trip);
 	writel_relaxed(val, tsc->regs + TSENSOR_SENSOR0_CONFIG2);
 
 	/*

[PATCH v1 10/14] thermal: tegra: Use thermal_zone_for_each_trip() for walking trip points

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

It is generally inefficient to iterate over trip indices and call
thermal_zone_get_trip() every time to get the struct thermal_trip
corresponding to the given trip index, so modify the Tegra thermal
drivers to use thermal_zone_for_each_trip() for walking trips.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/tegra/soctherm.c        |   38 ++++++++++++++++----------------
 drivers/thermal/tegra/tegra30-tsensor.c |   25 ++++++++++-----------
 2 files changed, 33 insertions(+), 30 deletions(-)

Index: linux-pm/drivers/thermal/tegra/soctherm.c
===================================================================
--- linux-pm.orig/drivers/thermal/tegra/soctherm.c
+++ linux-pm/drivers/thermal/tegra/soctherm.c
@@ -683,24 +683,25 @@ static const struct thermal_zone_device_
 	.set_trips = tegra_thermctl_set_trips,
 };
 
-static int get_hot_temp(struct thermal_zone_device *tz, int *trip_id, int *temp)
+static int get_hot_trip_cb(struct thermal_trip *trip, void *arg)
 {
-	int i, ret;
-	struct thermal_trip trip;
+	const struct thermal_trip **trip_ret = arg;
 
-	for (i = 0; i < thermal_zone_get_num_trips(tz); i++) {
+	if (trip->type != THERMAL_TRIP_HOT)
+		return 0;
 
-		ret = thermal_zone_get_trip(tz, i, &trip);
-		if (ret)
-			return -EINVAL;
-
-		if (trip.type == THERMAL_TRIP_HOT) {
-			*trip_id = i;
-			return 0;
-		}
-	}
+	*trip_ret = trip;
+	/* Return nonzero to terminate the search. */
+	return 1;
+}
 
-	return -EINVAL;
+static const struct thermal_trip *get_hot_trip(struct thermal_zone_device *tz)
+{
+	const struct thermal_trip *trip = NULL;
+
+	thermal_zone_for_each_trip(tz, get_hot_trip_cb, &trip);
+
+	return trip;
 }
 
 /**
@@ -732,8 +733,9 @@ static int tegra_soctherm_set_hwtrips(st
 				      struct thermal_zone_device *tz)
 {
 	struct tegra_soctherm *ts = dev_get_drvdata(dev);
+	const struct thermal_trip *hot_trip;
 	struct soctherm_throt_cfg *stc;
-	int i, trip, temperature, ret;
+	int i, temperature, ret;
 
 	/* Get thermtrips. If missing, try to get critical trips. */
 	temperature = tsensor_group_thermtrip_get(ts, sg->id);
@@ -750,8 +752,8 @@ static int tegra_soctherm_set_hwtrips(st
 	dev_info(dev, "thermtrip: will shut down when %s reaches %d mC\n",
 		 sg->name, temperature);
 
-	ret = get_hot_temp(tz, &trip, &temperature);
-	if (ret) {
+	hot_trip = get_hot_trip(tz);
+	if (!hot_trip) {
 		dev_info(dev, "throttrip: %s: missing hot temperature\n",
 			 sg->name);
 		return 0;
@@ -764,7 +766,7 @@ static int tegra_soctherm_set_hwtrips(st
 			continue;
 
 		cdev = ts->throt_cfgs[i].cdev;
-		if (get_thermal_instance(tz, cdev, trip))
+		if (thermal_trip_is_bound_to_cdev(tz, hot_trip, cdev))
 			stc = find_throttle_cfg_by_name(ts, cdev->type);
 		else
 			continue;
Index: linux-pm/drivers/thermal/tegra/tegra30-tsensor.c
===================================================================
--- linux-pm.orig/drivers/thermal/tegra/tegra30-tsensor.c
+++ linux-pm/drivers/thermal/tegra/tegra30-tsensor.c
@@ -308,6 +308,18 @@ struct trip_temps {
 	int crit_trip;
 };
 
+static int tegra_tsensor_get_trips_cb(struct thermal_trip *trip, void *arg)
+{
+	struct trip_temps *temps = arg;
+
+	if (trip->type == THERMAL_TRIP_HOT)
+		temps->hot_trip = trip->temperature;
+	else if (trip->type == THERMAL_TRIP_CRITICAL)
+		temps->crit_trip = trip->temperature;
+
+	return 0;
+}
+
 static void tegra_tsensor_get_hw_channel_trips(struct thermal_zone_device *tzd,
 					       struct trip_temps *temps)
 {
@@ -320,18 +332,7 @@ static void tegra_tsensor_get_hw_channel
 	temps->hot_trip  = 85000;
 	temps->crit_trip = 90000;
 
-	for (i = 0; i < thermal_zone_get_num_trips(tzd); i++) {
-
-		struct thermal_trip trip;
-
-		thermal_zone_get_trip(tzd, i, &trip);
-
-		if (trip.type == THERMAL_TRIP_HOT)
-			temps->hot_trip = trip.temperature;
-
-		if (trip.type == THERMAL_TRIP_CRITICAL)
-			temps->crit_trip = trip.temperature;
-	}
+	thermal_zone_for_each_trip(tzd, tegra_tsensor_get_trips_cb, temps);
 
 	/* clamp hardware trips to the calibration limits */
 	temps->hot_trip = clamp(temps->hot_trip, 25000, 90000);

[PATCH v1 11/14] thermal: helpers: Drop get_thermal_instance()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

There are no more users of get_thermal_instance(), so drop it.

While at it, replace get_instance() returning a pointer to struct
thermal_instance with thermal_instance_present() returning a bool
which is more straightforward.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.h    |    5 -----
 drivers/thermal/thermal_helpers.c |   30 ++++++------------------------
 2 files changed, 6 insertions(+), 29 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -191,11 +191,6 @@ void __thermal_cdev_update(struct therma
 
 int get_tz_trend(struct thermal_zone_device *tz, const struct thermal_trip *trip);
 
-struct thermal_instance *
-get_thermal_instance(struct thermal_zone_device *tz,
-		     struct thermal_cooling_device *cdev,
-		     int trip);
-
 /*
  * This structure is used to describe the behavior of
  * a certain cooling device on a certain trip point
Index: linux-pm/drivers/thermal/thermal_helpers.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_helpers.c
+++ linux-pm/drivers/thermal/thermal_helpers.c
@@ -39,18 +39,18 @@ int get_tz_trend(struct thermal_zone_dev
 	return trend;
 }
 
-static struct thermal_instance *get_instance(struct thermal_zone_device *tz,
-					     struct thermal_cooling_device *cdev,
-					     const struct thermal_trip *trip)
+static bool thermal_instance_present(struct thermal_zone_device *tz,
+				     struct thermal_cooling_device *cdev,
+				     const struct thermal_trip *trip)
 {
 	struct thermal_instance *ti;
 
 	list_for_each_entry(ti, &tz->thermal_instances, tz_node) {
 		if (ti->trip == trip && ti->cdev == cdev)
-			return ti;
+			return true;
 	}
 
-	return NULL;
+	return false;
 }
 
 bool thermal_trip_is_bound_to_cdev(struct thermal_zone_device *tz,
@@ -62,7 +62,7 @@ bool thermal_trip_is_bound_to_cdev(struc
 	mutex_lock(&tz->lock);
 	mutex_lock(&cdev->lock);
 
-	ret = !!get_instance(tz, cdev, trip);
+	ret = thermal_instance_present(tz, cdev, trip);
 
 	mutex_unlock(&cdev->lock);
 	mutex_unlock(&tz->lock);
@@ -71,24 +71,6 @@ bool thermal_trip_is_bound_to_cdev(struc
 }
 EXPORT_SYMBOL_GPL(thermal_trip_is_bound_to_cdev);
 
-struct thermal_instance *
-get_thermal_instance(struct thermal_zone_device *tz,
-		     struct thermal_cooling_device *cdev, int trip_index)
-{
-	struct thermal_instance *ti;
-
-	mutex_lock(&tz->lock);
-	mutex_lock(&cdev->lock);
-
-	ti = get_instance(tz, cdev, &tz->trips[trip_index].trip);
-
-	mutex_unlock(&cdev->lock);
-	mutex_unlock(&tz->lock);
-
-	return ti;
-}
-EXPORT_SYMBOL(get_thermal_instance);
-
 /**
  * __thermal_zone_get_temp() - returns the temperature of a thermal zone
  * @tz: a valid pointer to a struct thermal_zone_device

[PATCH v1 12/14] thermal: uniphier: Use thermal_zone_for_each_trip() for walking trip points

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

It is generally inefficient to iterate over trip indices and call
thermal_zone_get_trip() every time to get the struct thermal_trip
corresponding to the given trip index, so modify the uniphier thermal
driver to use thermal_zone_for_each_trip() for walking trips.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/uniphier_thermal.c |   37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

Index: linux-pm/drivers/thermal/uniphier_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/uniphier_thermal.c
+++ linux-pm/drivers/thermal/uniphier_thermal.c
@@ -239,13 +239,32 @@ static irqreturn_t uniphier_tm_alarm_irq
 	return IRQ_HANDLED;
 }
 
+struct trip_walk_data {
+	struct uniphier_tm_dev *tdev;
+	int crit_temp;
+	int index;
+};
+
+static int uniphier_tm_trip_walk_cb(struct thermal_trip *trip, void *arg)
+{
+	struct trip_walk_data *twd = arg;
+
+	if (trip->type == THERMAL_TRIP_CRITICAL &&
+	    trip->temperature < twd->crit_temp)
+		twd->crit_temp = trip->temperature;
+
+	uniphier_tm_set_alert(twd->tdev, twd->index, trip->temperature);
+	twd->tdev->alert_en[twd->index++] = true;
+}
+
 static int uniphier_tm_probe(struct platform_device *pdev)
 {
+	struct trip_walk_data twd = { .crit_temp = INT_MAX, .index = 0 };
 	struct device *dev = &pdev->dev;
 	struct regmap *regmap;
 	struct device_node *parent;
 	struct uniphier_tm_dev *tdev;
-	int i, ret, irq, crit_temp = INT_MAX;
+	int i, ret, irq;
 
 	tdev = devm_kzalloc(dev, sizeof(*tdev), GFP_KERNEL);
 	if (!tdev)
@@ -293,20 +312,10 @@ static int uniphier_tm_probe(struct plat
 	}
 
 	/* set alert temperatures */
-	for (i = 0; i < thermal_zone_get_num_trips(tdev->tz_dev); i++) {
-		struct thermal_trip trip;
+	twd.tdev = tdev;
+	thermal_zone_for_each_trip(tdev->tz_dev, uniphier_tm_trip_walk_cb, &twd);
 
-		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) {
+	if (twd.crit_temp > CRITICAL_TEMP_LIMIT) {
 		dev_err(dev, "critical trip is over limit(>%d), or not set\n",
 			CRITICAL_TEMP_LIMIT);
 		return -EINVAL;

Re: [PATCH v1 12/14] thermal: uniphier: Use thermal_zone_for_each_trip() for walking trip points

[Kunihiko Hayashi]

Hi Rafael,

On 2024/06/18 3:07, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> It is generally inefficient to iterate over trip indices and call
> thermal_zone_get_trip() every time to get the struct thermal_trip
> corresponding to the given trip index, so modify the uniphier thermal
> driver to use thermal_zone_for_each_trip() for walking trips.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/thermal/uniphier_thermal.c |   37
> +++++++++++++++++++++++--------------
>   1 file changed, 23 insertions(+), 14 deletions(-)
> 
> Index: linux-pm/drivers/thermal/uniphier_thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/uniphier_thermal.c
> +++ linux-pm/drivers/thermal/uniphier_thermal.c
> @@ -239,13 +239,32 @@ static irqreturn_t uniphier_tm_alarm_irq
>   	return IRQ_HANDLED;
>   }
> 
> +struct trip_walk_data {
> +	struct uniphier_tm_dev *tdev;
> +	int crit_temp;
> +	int index;
> +};
> +
> +static int uniphier_tm_trip_walk_cb(struct thermal_trip *trip, void *arg)
> +{
> +	struct trip_walk_data *twd = arg;
> +
> +	if (trip->type == THERMAL_TRIP_CRITICAL &&
> +	    trip->temperature < twd->crit_temp)
> +		twd->crit_temp = trip->temperature;
> +
> +	uniphier_tm_set_alert(twd->tdev, twd->index, trip->temperature);
> +	twd->tdev->alert_en[twd->index++] = true;
> +}
> +
>   static int uniphier_tm_probe(struct platform_device *pdev)
>   {
> +	struct trip_walk_data twd = { .crit_temp = INT_MAX, .index = 0 };
>   	struct device *dev = &pdev->dev;
>   	struct regmap *regmap;
>   	struct device_node *parent;
>   	struct uniphier_tm_dev *tdev;
> -	int i, ret, irq, crit_temp = INT_MAX;
> +	int i, ret, irq;
> 
>   	tdev = devm_kzalloc(dev, sizeof(*tdev), GFP_KERNEL);
>   	if (!tdev)
> @@ -293,20 +312,10 @@ static int uniphier_tm_probe(struct plat
>   	}
> 
>   	/* set alert temperatures */
> -	for (i = 0; i < thermal_zone_get_num_trips(tdev->tz_dev); i++) {
> -		struct thermal_trip trip;
> +	twd.tdev = tdev;
> +	thermal_zone_for_each_trip(tdev->tz_dev, uniphier_tm_trip_walk_cb, &twd);
> 
> -		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) {
> +	if (twd.crit_temp > CRITICAL_TEMP_LIMIT) {
>   		dev_err(dev, "critical trip is over limit(>%d), or not set\n",
>   			CRITICAL_TEMP_LIMIT);
>   		return -EINVAL;

I confirmed the updated code using the helper function is equivalent
to the original.

Reviewed-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

Thank you,

---
Best Regards
Kunihiko Hayashi

Re: [PATCH v1 12/14] thermal: uniphier: Use thermal_zone_for_each_trip() for walking trip points

[Rafael J. Wysocki]

Hi,

On Tue, Jun 18, 2024 at 6:05 AM Kunihiko Hayashi
<hayashi.kunihiko@socionext.com> wrote:
>
> Hi Rafael,
>
> On 2024/06/18 3:07, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > It is generally inefficient to iterate over trip indices and call
> > thermal_zone_get_trip() every time to get the struct thermal_trip
> > corresponding to the given trip index, so modify the uniphier thermal
> > driver to use thermal_zone_for_each_trip() for walking trips.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >   drivers/thermal/uniphier_thermal.c |   37
> > +++++++++++++++++++++++--------------
> >   1 file changed, 23 insertions(+), 14 deletions(-)
> >
> > Index: linux-pm/drivers/thermal/uniphier_thermal.c
> > ===================================================================
> > --- linux-pm.orig/drivers/thermal/uniphier_thermal.c
> > +++ linux-pm/drivers/thermal/uniphier_thermal.c
> > @@ -239,13 +239,32 @@ static irqreturn_t uniphier_tm_alarm_irq
> >       return IRQ_HANDLED;
> >   }
> >
> > +struct trip_walk_data {
> > +     struct uniphier_tm_dev *tdev;
> > +     int crit_temp;
> > +     int index;
> > +};
> > +
> > +static int uniphier_tm_trip_walk_cb(struct thermal_trip *trip, void *arg)
> > +{
> > +     struct trip_walk_data *twd = arg;
> > +
> > +     if (trip->type == THERMAL_TRIP_CRITICAL &&
> > +         trip->temperature < twd->crit_temp)
> > +             twd->crit_temp = trip->temperature;
> > +
> > +     uniphier_tm_set_alert(twd->tdev, twd->index, trip->temperature);
> > +     twd->tdev->alert_en[twd->index++] = true;
> > +}
> > +
> >   static int uniphier_tm_probe(struct platform_device *pdev)
> >   {
> > +     struct trip_walk_data twd = { .crit_temp = INT_MAX, .index = 0 };
> >       struct device *dev = &pdev->dev;
> >       struct regmap *regmap;
> >       struct device_node *parent;
> >       struct uniphier_tm_dev *tdev;
> > -     int i, ret, irq, crit_temp = INT_MAX;
> > +     int i, ret, irq;
> >
> >       tdev = devm_kzalloc(dev, sizeof(*tdev), GFP_KERNEL);
> >       if (!tdev)
> > @@ -293,20 +312,10 @@ static int uniphier_tm_probe(struct plat
> >       }
> >
> >       /* set alert temperatures */
> > -     for (i = 0; i < thermal_zone_get_num_trips(tdev->tz_dev); i++) {
> > -             struct thermal_trip trip;
> > +     twd.tdev = tdev;
> > +     thermal_zone_for_each_trip(tdev->tz_dev, uniphier_tm_trip_walk_cb, &twd);
> >
> > -             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) {
> > +     if (twd.crit_temp > CRITICAL_TEMP_LIMIT) {
> >               dev_err(dev, "critical trip is over limit(>%d), or not set\n",
> >                       CRITICAL_TEMP_LIMIT);
> >               return -EINVAL;
>
> I confirmed the updated code using the helper function is equivalent
> to the original.
>
> Reviewed-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

Thank you!

[PATCH v1 13/14] thermal: trip: Replace thermal_zone_get_num_trips()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The only existing caller of thermal_zone_get_num_trips(), which is
rcar_gen3_thermal_probe(), uses this function to check whether or
not the number of trips in the given thermal zone is nonzero.

Because it really only needs to know whether or not the given
thermal zone is tripless, replace thermal_zone_get_num_trips() with
thermal_zone_is_tripless() that can tell rcar_gen3_thermal_probe()
exactly what it needs to know and make it call that function.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/renesas/rcar_gen3_thermal.c |    3 +--
 drivers/thermal/thermal_trip.c              |    6 +++---
 include/linux/thermal.h                     |    2 +-
 3 files changed, 5 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/thermal/renesas/rcar_gen3_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/renesas/rcar_gen3_thermal.c
+++ linux-pm/drivers/thermal/renesas/rcar_gen3_thermal.c
@@ -563,8 +563,7 @@ static int rcar_gen3_thermal_probe(struc
 		if (ret)
 			goto error_unregister;
 
-		ret = thermal_zone_get_num_trips(tsc->zone);
-		if (ret < 0)
+		if (thermal_zone_is_tripless(tsc->zone))
 			goto error_unregister;
 
 		dev_info(dev, "Sensor %u: Loaded %d trip points\n", i, ret);
Index: linux-pm/drivers/thermal/thermal_trip.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_trip.c
+++ linux-pm/drivers/thermal/thermal_trip.c
@@ -55,11 +55,11 @@ int thermal_zone_for_each_trip(struct th
 }
 EXPORT_SYMBOL_GPL(thermal_zone_for_each_trip);
 
-int thermal_zone_get_num_trips(struct thermal_zone_device *tz)
+bool thermal_zone_is_tripless(struct thermal_zone_device *tz)
 {
-	return tz->num_trips;
+	return tz->num_trips == 0;
 }
-EXPORT_SYMBOL_GPL(thermal_zone_get_num_trips);
+EXPORT_SYMBOL_GPL(thermal_zone_is_tripless);
 
 /**
  * thermal_zone_set_trips - Computes the next trip points for the driver
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -210,7 +210,7 @@ int for_each_thermal_trip(struct thermal
 int thermal_zone_for_each_trip(struct thermal_zone_device *tz,
 			       int (*cb)(struct thermal_trip *, void *),
 			       void *data);
-int thermal_zone_get_num_trips(struct thermal_zone_device *tz);
+bool thermal_zone_is_tripless(struct thermal_zone_device *tz);
 void thermal_zone_set_trip_temp(struct thermal_zone_device *tz,
 				struct thermal_trip *trip, int temp);
 

Re: [PATCH v1 13/14] thermal: trip: Replace thermal_zone_get_num_trips()

[Niklas Söderlund]

Hi Rafael,

Thanks for your patch.

On 2024-06-17 20:11:30 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The only existing caller of thermal_zone_get_num_trips(), which is
> rcar_gen3_thermal_probe(), uses this function to check whether or
> not the number of trips in the given thermal zone is nonzero.
> 
> Because it really only needs to know whether or not the given
> thermal zone is tripless, replace thermal_zone_get_num_trips() with
> thermal_zone_is_tripless() that can tell rcar_gen3_thermal_probe()
> exactly what it needs to know and make it call that function.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/thermal/renesas/rcar_gen3_thermal.c |    3 +--
>  drivers/thermal/thermal_trip.c              |    6 +++---
>  include/linux/thermal.h                     |    2 +-
>  3 files changed, 5 insertions(+), 6 deletions(-)
> 
> Index: linux-pm/drivers/thermal/renesas/rcar_gen3_thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/renesas/rcar_gen3_thermal.c
> +++ linux-pm/drivers/thermal/renesas/rcar_gen3_thermal.c
> @@ -563,8 +563,7 @@ static int rcar_gen3_thermal_probe(struc
>  		if (ret)
>  			goto error_unregister;
>  
> -		ret = thermal_zone_get_num_trips(tsc->zone);
> -		if (ret < 0)
> +		if (thermal_zone_is_tripless(tsc->zone))

There are two issues with this change.

1. The original code only jumped to error_unregister if there where a 
   negative number of trip points, presumably to allow for an error to 
   be returned when reading the number of trip points.

   If an negative error was found it was stored in ret, and this was 
   then returned from the probe function after jumping to 
   error_unregister. This change jumps to the error out path, but do not 
   fail probe.

   However it is valid to complete probe without any trip points found.  
   So failing probe on thermal_zone_is_tripless() is not desired.

2. The value returned from thermal_zone_get_num_trips() and stored in 
   ret is used in a dev_info() below, logging how many trip points (if 
   any) where found.

   With this change that is no longer true and it will always log 0 trip 
   points found.

As there is no long (if there ever where) a reason to check for errors 
when reading the number of trip points, and no real use to log the 
number of trip points found maybe a modified patch can do what you want 
(not tested).

- ret = thermal_zone_get_num_trips(tsc->zone);
- if (ret < 0)
-    goto error_unregister;
-
- dev_info(dev, "Sensor %u: Loaded %d trip points\n", i, ret);
+ dev_info(dev, "Sensor %u: Loaded %s trip points\n", i,
+       thermal_zone_is_tripless(tsc->zone) ? "with" : "without");

What do you think?

>  			goto error_unregister;
>  
>  		dev_info(dev, "Sensor %u: Loaded %d trip points\n", i, ret);
> Index: linux-pm/drivers/thermal/thermal_trip.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_trip.c
> +++ linux-pm/drivers/thermal/thermal_trip.c
> @@ -55,11 +55,11 @@ int thermal_zone_for_each_trip(struct th
>  }
>  EXPORT_SYMBOL_GPL(thermal_zone_for_each_trip);
>  
> -int thermal_zone_get_num_trips(struct thermal_zone_device *tz)
> +bool thermal_zone_is_tripless(struct thermal_zone_device *tz)
>  {
> -	return tz->num_trips;
> +	return tz->num_trips == 0;
>  }
> -EXPORT_SYMBOL_GPL(thermal_zone_get_num_trips);
> +EXPORT_SYMBOL_GPL(thermal_zone_is_tripless);
>  
>  /**
>   * thermal_zone_set_trips - Computes the next trip points for the driver
> Index: linux-pm/include/linux/thermal.h
> ===================================================================
> --- linux-pm.orig/include/linux/thermal.h
> +++ linux-pm/include/linux/thermal.h
> @@ -210,7 +210,7 @@ int for_each_thermal_trip(struct thermal
>  int thermal_zone_for_each_trip(struct thermal_zone_device *tz,
>  			       int (*cb)(struct thermal_trip *, void *),
>  			       void *data);
> -int thermal_zone_get_num_trips(struct thermal_zone_device *tz);
> +bool thermal_zone_is_tripless(struct thermal_zone_device *tz);
>  void thermal_zone_set_trip_temp(struct thermal_zone_device *tz,
>  				struct thermal_trip *trip, int temp);
>  
> 
> 
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH v1 13/14] thermal: trip: Replace thermal_zone_get_num_trips()

[Rafael J. Wysocki]

Hi Niklas,

On Mon, Jun 17, 2024 at 8:39 PM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
>
> Hi Rafael,
>
> Thanks for your patch.
>
> On 2024-06-17 20:11:30 +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > The only existing caller of thermal_zone_get_num_trips(), which is
> > rcar_gen3_thermal_probe(), uses this function to check whether or
> > not the number of trips in the given thermal zone is nonzero.
> >
> > Because it really only needs to know whether or not the given
> > thermal zone is tripless, replace thermal_zone_get_num_trips() with
> > thermal_zone_is_tripless() that can tell rcar_gen3_thermal_probe()
> > exactly what it needs to know and make it call that function.
> >
> > No intentional functional impact.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/thermal/renesas/rcar_gen3_thermal.c |    3 +--
> >  drivers/thermal/thermal_trip.c              |    6 +++---
> >  include/linux/thermal.h                     |    2 +-
> >  3 files changed, 5 insertions(+), 6 deletions(-)
> >
> > Index: linux-pm/drivers/thermal/renesas/rcar_gen3_thermal.c
> > ===================================================================
> > --- linux-pm.orig/drivers/thermal/renesas/rcar_gen3_thermal.c
> > +++ linux-pm/drivers/thermal/renesas/rcar_gen3_thermal.c
> > @@ -563,8 +563,7 @@ static int rcar_gen3_thermal_probe(struc
> >               if (ret)
> >                       goto error_unregister;
> >
> > -             ret = thermal_zone_get_num_trips(tsc->zone);
> > -             if (ret < 0)
> > +             if (thermal_zone_is_tripless(tsc->zone))
>
> There are two issues with this change.
>
> 1. The original code only jumped to error_unregister if there where a
>    negative number of trip points, presumably to allow for an error to
>    be returned when reading the number of trip points.
>
>    If an negative error was found it was stored in ret, and this was
>    then returned from the probe function after jumping to
>    error_unregister. This change jumps to the error out path, but do not
>    fail probe.
>
>    However it is valid to complete probe without any trip points found.
>    So failing probe on thermal_zone_is_tripless() is not desired.
>
> 2. The value returned from thermal_zone_get_num_trips() and stored in
>    ret is used in a dev_info() below, logging how many trip points (if
>    any) where found.
>
>    With this change that is no longer true and it will always log 0 trip
>    points found.

You are right, I've overlooked the above.

> As there is no long (if there ever where) a reason to check for errors
> when reading the number of trip points, and no real use to log the
> number of trip points found maybe a modified patch can do what you want
> (not tested).
>
> - ret = thermal_zone_get_num_trips(tsc->zone);
> - if (ret < 0)
> -    goto error_unregister;
> -
> - dev_info(dev, "Sensor %u: Loaded %d trip points\n", i, ret);
> + dev_info(dev, "Sensor %u: Loaded %s trip points\n", i,
> +       thermal_zone_is_tripless(tsc->zone) ? "with" : "without");
>
> What do you think?

I would rather first update the driver to stop failing when the zone
is tripless, in a separate patch.

Fortunately, the $subject patch is not really needed in the series, so
please regard it as withdrawn for now and we can get back to this
later.

Thanks!

>
> >                       goto error_unregister;
> >
> >               dev_info(dev, "Sensor %u: Loaded %d trip points\n", i, ret);
> > Index: linux-pm/drivers/thermal/thermal_trip.c
> > ===================================================================
> > --- linux-pm.orig/drivers/thermal/thermal_trip.c
> > +++ linux-pm/drivers/thermal/thermal_trip.c
> > @@ -55,11 +55,11 @@ int thermal_zone_for_each_trip(struct th
> >  }
> >  EXPORT_SYMBOL_GPL(thermal_zone_for_each_trip);
> >
> > -int thermal_zone_get_num_trips(struct thermal_zone_device *tz)
> > +bool thermal_zone_is_tripless(struct thermal_zone_device *tz)
> >  {
> > -     return tz->num_trips;
> > +     return tz->num_trips == 0;
> >  }
> > -EXPORT_SYMBOL_GPL(thermal_zone_get_num_trips);
> > +EXPORT_SYMBOL_GPL(thermal_zone_is_tripless);
> >
> >  /**
> >   * thermal_zone_set_trips - Computes the next trip points for the driver
> > Index: linux-pm/include/linux/thermal.h
> > ===================================================================
> > --- linux-pm.orig/include/linux/thermal.h
> > +++ linux-pm/include/linux/thermal.h
> > @@ -210,7 +210,7 @@ int for_each_thermal_trip(struct thermal
> >  int thermal_zone_for_each_trip(struct thermal_zone_device *tz,
> >                              int (*cb)(struct thermal_trip *, void *),
> >                              void *data);
> > -int thermal_zone_get_num_trips(struct thermal_zone_device *tz);
> > +bool thermal_zone_is_tripless(struct thermal_zone_device *tz);
> >  void thermal_zone_set_trip_temp(struct thermal_zone_device *tz,
> >                               struct thermal_trip *trip, int temp);
> >
> >
> >
> >
>
> --

Re: [PATCH v1 13/14] thermal: trip: Replace thermal_zone_get_num_trips()

[Rafael J. Wysocki]

On Mon, Jun 17, 2024 at 8:55 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> Hi Niklas,
>
> On Mon, Jun 17, 2024 at 8:39 PM Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> >
> > Hi Rafael,
> >
> > Thanks for your patch.
> >
> > On 2024-06-17 20:11:30 +0200, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > >
> > > The only existing caller of thermal_zone_get_num_trips(), which is
> > > rcar_gen3_thermal_probe(), uses this function to check whether or
> > > not the number of trips in the given thermal zone is nonzero.
> > >
> > > Because it really only needs to know whether or not the given
> > > thermal zone is tripless, replace thermal_zone_get_num_trips() with
> > > thermal_zone_is_tripless() that can tell rcar_gen3_thermal_probe()
> > > exactly what it needs to know and make it call that function.
> > >
> > > No intentional functional impact.
> > >
> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > ---
> > >  drivers/thermal/renesas/rcar_gen3_thermal.c |    3 +--
> > >  drivers/thermal/thermal_trip.c              |    6 +++---
> > >  include/linux/thermal.h                     |    2 +-
> > >  3 files changed, 5 insertions(+), 6 deletions(-)
> > >
> > > Index: linux-pm/drivers/thermal/renesas/rcar_gen3_thermal.c
> > > ===================================================================
> > > --- linux-pm.orig/drivers/thermal/renesas/rcar_gen3_thermal.c
> > > +++ linux-pm/drivers/thermal/renesas/rcar_gen3_thermal.c
> > > @@ -563,8 +563,7 @@ static int rcar_gen3_thermal_probe(struc
> > >               if (ret)
> > >                       goto error_unregister;
> > >
> > > -             ret = thermal_zone_get_num_trips(tsc->zone);
> > > -             if (ret < 0)
> > > +             if (thermal_zone_is_tripless(tsc->zone))
> >
> > There are two issues with this change.
> >
> > 1. The original code only jumped to error_unregister if there where a
> >    negative number of trip points, presumably to allow for an error to
> >    be returned when reading the number of trip points.
> >
> >    If an negative error was found it was stored in ret, and this was
> >    then returned from the probe function after jumping to
> >    error_unregister. This change jumps to the error out path, but do not
> >    fail probe.
> >
> >    However it is valid to complete probe without any trip points found.
> >    So failing probe on thermal_zone_is_tripless() is not desired.
> >
> > 2. The value returned from thermal_zone_get_num_trips() and stored in
> >    ret is used in a dev_info() below, logging how many trip points (if
> >    any) where found.
> >
> >    With this change that is no longer true and it will always log 0 trip
> >    points found.
>
> You are right, I've overlooked the above.
>
> > As there is no long (if there ever where) a reason to check for errors
> > when reading the number of trip points, and no real use to log the
> > number of trip points found maybe a modified patch can do what you want
> > (not tested).
> >
> > - ret = thermal_zone_get_num_trips(tsc->zone);
> > - if (ret < 0)
> > -    goto error_unregister;
> > -
> > - dev_info(dev, "Sensor %u: Loaded %d trip points\n", i, ret);
> > + dev_info(dev, "Sensor %u: Loaded %s trip points\n", i,
> > +       thermal_zone_is_tripless(tsc->zone) ? "with" : "without");
> >
> > What do you think?
>
> I would rather first update the driver to stop failing when the zone
> is tripless, in a separate patch.

Well, it actually never fails when the zone is tripless because
thermal_zone_get_num_trips() returns a non-negative value.

So something like the above patch, but using
thermal_zone_get_num_trips() or maybe just

- ret = thermal_zone_get_num_trips(tsc->zone);
- if (ret < 0)
-    goto error_unregister;
-
- dev_info(dev, "Sensor %u: Loaded %d trip points\n", i, ret);
+ dev_info(dev, "Sensor %u: Loaded", i);

because the number of trips (and whether or not there are any) can be
checked via sysfs.

> Fortunately, the $subject patch is not really needed in the series, so
> please regard it as withdrawn for now and we can get back to this
> later.

But the above still holds.

Thanks!

[PATCH v1 14/14] thermal: trip: Drop thermal_zone_get_trip()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

There are no more callers of thermal_zone_get_trip() in the tree, so
drop it.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_trip.c |   14 --------------
 include/linux/thermal.h        |    2 --
 2 files changed, 16 deletions(-)

Index: linux-pm/drivers/thermal/thermal_trip.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_trip.c
+++ linux-pm/drivers/thermal/thermal_trip.c
@@ -114,20 +114,6 @@ void thermal_zone_set_trips(struct therm
 		dev_err(&tz->device, "Failed to set trips: %d\n", ret);
 }
 
-int thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id,
-			  struct thermal_trip *trip)
-{
-	if (!tz || !trip || trip_id < 0 || trip_id >= tz->num_trips)
-		return -EINVAL;
-
-	mutex_lock(&tz->lock);
-	*trip = tz->trips[trip_id].trip;
-	mutex_unlock(&tz->lock);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(thermal_zone_get_trip);
-
 int thermal_zone_trip_id(const struct thermal_zone_device *tz,
 			 const struct thermal_trip *trip)
 {
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -202,8 +202,6 @@ static inline void devm_thermal_of_zone_
 }
 #endif
 
-int thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id,
-			  struct thermal_trip *trip);
 int for_each_thermal_trip(struct thermal_zone_device *tz,
 			  int (*cb)(struct thermal_trip *, void *),
 			  void *data);