[PATCH v3 00/12] thermal OF rework

[PATCH v3 00/12] thermal OF rework

[Daniel Lezcano]

The thermal framework initialization with the device tree appears to
be complicated and hard to make it to evolve.

It contains duplication of almost the same thermal generic structures
and has an assymetric initialization making hard any kind of serious
changes for more complex features. One of them is the multiple sensors
support per thermal zone.

In order to set the scene for the aforementioned feature with generic
code, we need to cleanup and rework the device tree initialization.

However this rework is not obvious because of the multiple components
entering in the composition of a thermal zone and being initialized at
different moments. For instance, a cooling device can be initialized
before a sensor, so the thermal zones must exist before the cooling
device as well as the sensor. This asynchronous initialization forces
the thermal zone to be created with fake ops because they are
mandotory and build a list of cooling devices which is used to lookup
afterwards when the cooling device driver is registering itself.

As there could be a large number of changes, this first series provide
some steps forward for a simpler device tree initialization.

Changelog:

 - V3:
    - Removed patch 1 and 2 from the V2 which consist in renaming the
      thermal_zone_device_ops to thermal_sensor_ops and separating the
      structure. I'll do a separate proposal for that after the incoming
      cleanups

 - V2:
   - Drop patch 1/15 which contains too many changes for a simple
     structure renaming. This could be addressed in a separate series as
     it is not necessary for the OF rework
     
   - Fixed of_node_put with gchild not initialized as reported by
     kbuild and Dan Carpenter

 - V1:
   - Initial post

Daniel Lezcano (12):
  thermal/core: Remove duplicate information when an error occurs
  thermal/of: Replace device node match with device node search
  thermal/of: Remove the device node pointer for thermal_trip
  thermal/of: Move thermal_trip structure to thermal.h
  thermal/core: Remove unneeded EXPORT_SYMBOLS
  thermal/core: Move thermal_set_delay_jiffies to static
  thermal/core: Rename trips to ntrips
  thermal/core: Add thermal_trip in thermal_zone
  thermal/core: Register with the trip points
  thermal/of: Store the trips in the thermal zone
  thermal/of: Use thermal trips stored in the thermal zone
  thermal/of: Initialize trip points separately

 drivers/thermal/gov_fair_share.c        |   6 +-
 drivers/thermal/gov_power_allocator.c   |   4 +-
 drivers/thermal/tegra/tegra30-tsensor.c |   2 +-
 drivers/thermal/thermal_core.c          |  53 +++++--
 drivers/thermal/thermal_core.h          |  25 ++-
 drivers/thermal/thermal_helpers.c       |  13 +-
 drivers/thermal/thermal_netlink.c       |   2 +-
 drivers/thermal/thermal_of.c            | 201 +++++++++++++-----------
 drivers/thermal/thermal_sysfs.c         |  22 +--
 include/linux/thermal.h                 |  21 ++-
 10 files changed, 197 insertions(+), 152 deletions(-)

-- 
2.25.1

[PATCH v3 01/12] thermal/core: Remove duplicate information when an error occurs

[Daniel Lezcano]

The pr_err already tells it is an error, it is pointless to add the
'Error:' string in the messages. Remove them.

Cc: Alexandre Bailon <abailon@baylibre.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc; Eduardo Valentin <eduval@amazon.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
---
 drivers/thermal/thermal_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index cdc0552e8c42..e22e7d939c54 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1198,23 +1198,23 @@ thermal_zone_device_register(const char *type, int trips, int mask,
 	struct thermal_governor *governor;
 
 	if (!type || strlen(type) == 0) {
-		pr_err("Error: No thermal zone type defined\n");
+		pr_err("No thermal zone type defined\n");
 		return ERR_PTR(-EINVAL);
 	}
 
 	if (type && strlen(type) >= THERMAL_NAME_LENGTH) {
-		pr_err("Error: Thermal zone name (%s) too long, should be under %d chars\n",
+		pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
 		       type, THERMAL_NAME_LENGTH);
 		return ERR_PTR(-EINVAL);
 	}
 
 	if (trips > THERMAL_MAX_TRIPS || trips < 0 || mask >> trips) {
-		pr_err("Error: Incorrect number of thermal trips\n");
+		pr_err("Incorrect number of thermal trips\n");
 		return ERR_PTR(-EINVAL);
 	}
 
 	if (!ops) {
-		pr_err("Error: Thermal zone device ops not defined\n");
+		pr_err("Thermal zone device ops not defined\n");
 		return ERR_PTR(-EINVAL);
 	}
 
-- 
2.25.1

[PATCH v3 02/12] thermal/of: Replace device node match with device node search

[Daniel Lezcano]

The thermal_of code builds a trip array associated with the node
pointer in order to compare the trip point phandle with the list.

The thermal trip is a thermal zone property and should be moved
there. If some sensors have hardcoded trip points, they should use the
exported structure instead of redefining again and again their own
structure and data to describe exactly the same things.

In order to move this to the thermal.h header and allow more cleanup,
we need to remove the node pointer from the structure.

Instead of building storing the device node, we search directly in the
device tree the corresponding node. That results in a simplification
of the code and allows to move the structure to thermal.h

Cc: Alexandre Bailon <abailon@baylibre.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc; Eduardo Valentin <eduval@amazon.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
---
 drivers/thermal/thermal_of.c | 62 ++++++++++++++++++++++++------------
 1 file changed, 42 insertions(+), 20 deletions(-)

diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index b65d435cb92f..04c910ca8623 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -671,6 +671,35 @@ EXPORT_SYMBOL_GPL(devm_thermal_zone_of_sensor_unregister);
 
 /***   functions parsing device tree nodes   ***/
 
+static int of_find_trip_id(struct device_node *np, struct device_node *trip)
+{
+	struct device_node *trips;
+	struct device_node *t;
+	int i = 0;
+
+	trips = of_get_child_by_name(np, "trips");
+	if (!trips) {
+		pr_err("Failed to find 'trips' node\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Find the trip id point associated with the cooling device map
+	 */
+	for_each_child_of_node(trips, t) {
+
+		if (t == trip)
+			goto out;
+		i++;
+	}
+
+	i = -ENXIO;
+out:	
+	of_node_put(trips);
+
+	return i;
+}
+
 /**
  * thermal_of_populate_bind_params - parse and fill cooling map data
  * @np: DT node containing a cooling-map node
@@ -686,14 +715,13 @@ EXPORT_SYMBOL_GPL(devm_thermal_zone_of_sensor_unregister);
  * Return: 0 on success, proper error code otherwise
  */
 static int thermal_of_populate_bind_params(struct device_node *np,
-					   struct __thermal_bind_params *__tbp,
-					   struct thermal_trip *trips,
-					   int ntrips)
+					   struct __thermal_bind_params *__tbp)
 {
 	struct of_phandle_args cooling_spec;
 	struct __thermal_cooling_bind_param *__tcbp;
 	struct device_node *trip;
 	int ret, i, count;
+	int trip_id;
 	u32 prop;
 
 	/* Default weight. Usage is optional */
@@ -708,18 +736,14 @@ static int thermal_of_populate_bind_params(struct device_node *np,
 		return -ENODEV;
 	}
 
-	/* match using device_node */
-	for (i = 0; i < ntrips; i++)
-		if (trip == trips[i].np) {
-			__tbp->trip_id = i;
-			break;
-		}
-
-	if (i == ntrips) {
-		ret = -ENODEV;
+	trip_id = of_find_trip_id(np, trip);
+	if (trip_id < 0) {
+		ret = trip_id;
 		goto end;
 	}
 
+	__tbp->trip_id = trip_id;
+	
 	count = of_count_phandle_with_args(np, "cooling-device",
 					   "#cooling-cells");
 	if (count <= 0) {
@@ -868,6 +892,7 @@ static struct __thermal_zone
 __init *thermal_of_build_thermal_zone(struct device_node *np)
 {
 	struct device_node *child = NULL, *gchild;
+	struct device_node *trips;
 	struct __thermal_zone *tz;
 	int ret, i;
 	u32 prop, coef[2];
@@ -910,13 +935,13 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
 	}
 
 	/* trips */
-	child = of_get_child_by_name(np, "trips");
+	trips = of_get_child_by_name(np, "trips");
 
 	/* No trips provided */
-	if (!child)
+	if (!trips)
 		goto finish;
 
-	tz->ntrips = of_get_child_count(child);
+	tz->ntrips = of_get_child_count(trips);
 	if (tz->ntrips == 0) /* must have at least one child */
 		goto finish;
 
@@ -927,14 +952,12 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
 	}
 
 	i = 0;
-	for_each_child_of_node(child, gchild) {
+	for_each_child_of_node(trips, gchild) {
 		ret = thermal_of_populate_trip(gchild, &tz->trips[i++]);
 		if (ret)
 			goto free_trips;
 	}
 
-	of_node_put(child);
-
 	/* cooling-maps */
 	child = of_get_child_by_name(np, "cooling-maps");
 
@@ -954,8 +977,7 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
 
 	i = 0;
 	for_each_child_of_node(child, gchild) {
-		ret = thermal_of_populate_bind_params(gchild, &tz->tbps[i++],
-						      tz->trips, tz->ntrips);
+		ret = thermal_of_populate_bind_params(gchild, &tz->tbps[i++]);
 		if (ret)
 			goto free_tbps;
 	}
-- 
2.25.1

[PATCH v3 03/12] thermal/of: Remove the device node pointer for thermal_trip

[Daniel Lezcano]

The device node pointer is no longer needed in the thermal trip
structure, remove it.

Cc: Alexandre Bailon <abailon@baylibre.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc; Eduardo Valentin <eduval@amazon.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
---
 drivers/thermal/thermal_core.h | 2 --
 drivers/thermal/thermal_of.c   | 8 --------
 2 files changed, 10 deletions(-)

diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 726e327b4205..ff10cdda056c 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -70,13 +70,11 @@ void __thermal_cdev_update(struct thermal_cooling_device *cdev);
 
 /**
  * struct thermal_trip - representation of a point in temperature domain
- * @np: pointer to struct device_node that this trip point was created from
  * @temperature: temperature value in miliCelsius
  * @hysteresis: relative hysteresis in miliCelsius
  * @type: trip point type
  */
 struct thermal_trip {
-	struct device_node *np;
 	int temperature;
 	int hysteresis;
 	enum thermal_trip_type type;
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index 04c910ca8623..16eb18c24430 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -867,10 +867,6 @@ static int thermal_of_populate_trip(struct device_node *np,
 		return ret;
 	}
 
-	/* Required for cooling map matching */
-	trip->np = np;
-	of_node_get(np);
-
 	return 0;
 }
 
@@ -1000,8 +996,6 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
 
 	kfree(tz->tbps);
 free_trips:
-	for (i = 0; i < tz->ntrips; i++)
-		of_node_put(tz->trips[i].np);
 	kfree(tz->trips);
 	of_node_put(gchild);
 free_tz:
@@ -1026,8 +1020,6 @@ static __init void of_thermal_free_zone(struct __thermal_zone *tz)
 	}
 
 	kfree(tz->tbps);
-	for (i = 0; i < tz->ntrips; i++)
-		of_node_put(tz->trips[i].np);
 	kfree(tz->trips);
 	kfree(tz);
 }
-- 
2.25.1

[PATCH v3 04/12] thermal/of: Move thermal_trip structure to thermal.h

[Daniel Lezcano]

The structure thermal_trip is now generic and will be usable by the
different sensor drivers in place of their own structure.

Move its definition to thermal.h to make it accessible.

Cc: Alexandre Bailon <abailon@baylibre.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc; Eduardo Valentin <eduval@amazon.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
---
 drivers/thermal/thermal_core.h | 12 ------------
 include/linux/thermal.h        | 12 ++++++++++++
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index ff10cdda056c..60844e2d59bb 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -68,18 +68,6 @@ static inline bool cdev_is_power_actor(struct thermal_cooling_device *cdev)
 void thermal_cdev_update(struct thermal_cooling_device *);
 void __thermal_cdev_update(struct thermal_cooling_device *cdev);
 
-/**
- * struct thermal_trip - representation of a point in temperature domain
- * @temperature: temperature value in miliCelsius
- * @hysteresis: relative hysteresis in miliCelsius
- * @type: trip point type
- */
-struct thermal_trip {
-	int temperature;
-	int hysteresis;
-	enum thermal_trip_type type;
-};
-
 int get_tz_trend(struct thermal_zone_device *tz, int trip);
 
 struct thermal_instance *
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 365733b428d8..6289b0bb1c97 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -80,6 +80,18 @@ struct thermal_zone_device_ops {
 	void (*critical)(struct thermal_zone_device *);
 };
 
+/**
+ * struct thermal_trip - representation of a point in temperature domain
+ * @temperature: temperature value in miliCelsius
+ * @hysteresis: relative hysteresis in miliCelsius
+ * @type: trip point type
+ */
+struct thermal_trip {
+	int temperature;
+	int hysteresis;
+	enum thermal_trip_type type;
+};
+
 struct thermal_cooling_device_ops {
 	int (*get_max_state) (struct thermal_cooling_device *, unsigned long *);
 	int (*get_cur_state) (struct thermal_cooling_device *, unsigned long *);
-- 
2.25.1

[PATCH v3 05/12] thermal/core: Remove unneeded EXPORT_SYMBOLS

[Daniel Lezcano]

Different functions are exporting the symbols but are actually only
used by the thermal framework internals. Remove these EXPORT_SYMBOLS.

Cc: Alexandre Bailon <abailon@baylibre.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc; Eduardo Valentin <eduval@amazon.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
---
 drivers/thermal/thermal_helpers.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
index 3edd047e144f..f4c1e87ef040 100644
--- a/drivers/thermal/thermal_helpers.c
+++ b/drivers/thermal/thermal_helpers.c
@@ -39,7 +39,6 @@ int get_tz_trend(struct thermal_zone_device *tz, int trip)
 
 	return trend;
 }
-EXPORT_SYMBOL(get_tz_trend);
 
 struct thermal_instance *
 get_thermal_instance(struct thermal_zone_device *tz,
@@ -228,7 +227,6 @@ void thermal_cdev_update(struct thermal_cooling_device *cdev)
 	}
 	mutex_unlock(&cdev->lock);
 }
-EXPORT_SYMBOL(thermal_cdev_update);
 
 /**
  * thermal_zone_get_slope - return the slope attribute of the thermal zone
-- 
2.25.1

[PATCH v3 06/12] thermal/core: Move thermal_set_delay_jiffies to static

[Daniel Lezcano]

The function 'thermal_set_delay_jiffies' is only used in
thermal_core.c but it is defined and implemented in a separate
file. Move the function to thermal_core.c and make it static.

Cc: Alexandre Bailon <abailon@baylibre.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc; Eduardo Valentin <eduval@amazon.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
---
 drivers/thermal/thermal_core.c    | 7 +++++++
 drivers/thermal/thermal_core.h    | 1 -
 drivers/thermal/thermal_helpers.c | 7 -------
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index e22e7d939c54..a8b1628937c6 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1159,6 +1159,13 @@ static void bind_tz(struct thermal_zone_device *tz)
 	mutex_unlock(&thermal_list_lock);
 }
 
+static void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms)
+{
+	*delay_jiffies = msecs_to_jiffies(delay_ms);
+	if (delay_ms > 1000)
+		*delay_jiffies = round_jiffies(*delay_jiffies);
+}
+
 /**
  * thermal_zone_device_register() - register a new thermal zone device
  * @type:	the thermal zone device type
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 60844e2d59bb..c991bb290512 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -112,7 +112,6 @@ int thermal_build_list_of_policies(char *buf);
 
 /* Helpers */
 void thermal_zone_set_trips(struct thermal_zone_device *tz);
-void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms);
 
 /* sysfs I/F */
 int thermal_zone_create_device_groups(struct thermal_zone_device *, int);
diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
index f4c1e87ef040..60bfda1a1db7 100644
--- a/drivers/thermal/thermal_helpers.c
+++ b/drivers/thermal/thermal_helpers.c
@@ -174,13 +174,6 @@ void thermal_zone_set_trips(struct thermal_zone_device *tz)
 	mutex_unlock(&tz->lock);
 }
 
-void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms)
-{
-	*delay_jiffies = msecs_to_jiffies(delay_ms);
-	if (delay_ms > 1000)
-		*delay_jiffies = round_jiffies(*delay_jiffies);
-}
-
 static void thermal_cdev_set_cur_state(struct thermal_cooling_device *cdev,
 				       int target)
 {
-- 
2.25.1

[PATCH v3 07/12] thermal/core: Rename trips to ntrips

[Daniel Lezcano]

In order to use thermal trips defined in the thermal structure, rename
the 'trips' field to 'ntrips' to have the 'trips' field containing the
thermal trip points.

Cc: Alexandre Bailon <abailon@baylibre.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc; Eduardo Valentin <eduval@amazon.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
---
 drivers/thermal/gov_fair_share.c        |  6 +++---
 drivers/thermal/gov_power_allocator.c   |  4 ++--
 drivers/thermal/tegra/tegra30-tsensor.c |  2 +-
 drivers/thermal/thermal_core.c          | 20 ++++++++++----------
 drivers/thermal/thermal_helpers.c       |  4 ++--
 drivers/thermal/thermal_netlink.c       |  2 +-
 drivers/thermal/thermal_sysfs.c         | 22 +++++++++++-----------
 include/linux/thermal.h                 |  2 +-
 8 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/thermal/gov_fair_share.c b/drivers/thermal/gov_fair_share.c
index 1e5abf4822be..5d23bb5c39ea 100644
--- a/drivers/thermal/gov_fair_share.c
+++ b/drivers/thermal/gov_fair_share.c
@@ -25,10 +25,10 @@ static int get_trip_level(struct thermal_zone_device *tz)
 	int trip_temp;
 	enum thermal_trip_type trip_type;
 
-	if (tz->trips == 0 || !tz->ops->get_trip_temp)
+	if (tz->ntrips == 0 || !tz->ops->get_trip_temp)
 		return 0;
 
-	for (count = 0; count < tz->trips; count++) {
+	for (count = 0; count < tz->ntrips; count++) {
 		tz->ops->get_trip_temp(tz, count, &trip_temp);
 		if (tz->temperature < trip_temp)
 			break;
@@ -53,7 +53,7 @@ static long get_target_state(struct thermal_zone_device *tz,
 
 	cdev->ops->get_max_state(cdev, &max_state);
 
-	return (long)(percentage * level * max_state) / (100 * tz->trips);
+	return (long)(percentage * level * max_state) / (100 * tz->ntrips);
 }
 
 /**
diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index 13e375751d22..c9f4eb12b0c4 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -527,7 +527,7 @@ static void get_governor_trips(struct thermal_zone_device *tz,
 	last_active = INVALID_TRIP;
 	last_passive = INVALID_TRIP;
 
-	for (i = 0; i < tz->trips; i++) {
+	for (i = 0; i < tz->ntrips; i++) {
 		enum thermal_trip_type type;
 		int ret;
 
@@ -668,7 +668,7 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
 
 	get_governor_trips(tz, params);
 
-	if (tz->trips > 0) {
+	if (tz->ntrips > 0) {
 		ret = tz->ops->get_trip_temp(tz,
 					params->trip_max_desired_temperature,
 					&control_temp);
diff --git a/drivers/thermal/tegra/tegra30-tsensor.c b/drivers/thermal/tegra/tegra30-tsensor.c
index 9b6b693cbcf8..8ffd3dcfdb53 100644
--- a/drivers/thermal/tegra/tegra30-tsensor.c
+++ b/drivers/thermal/tegra/tegra30-tsensor.c
@@ -316,7 +316,7 @@ static void tegra_tsensor_get_hw_channel_trips(struct thermal_zone_device *tzd,
 	*hot_trip  = 85000;
 	*crit_trip = 90000;
 
-	for (i = 0; i < tzd->trips; i++) {
+	for (i = 0; i < tzd->ntrips; i++) {
 		enum thermal_trip_type type;
 		int trip_temp;
 
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index a8b1628937c6..434a675da245 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -505,7 +505,7 @@ void thermal_zone_device_update(struct thermal_zone_device *tz,
 
 	tz->notify_event = event;
 
-	for (count = 0; count < tz->trips; count++)
+	for (count = 0; count < tz->ntrips; count++)
 		handle_thermal_trip(tz, count);
 }
 EXPORT_SYMBOL_GPL(thermal_zone_device_update);
@@ -630,7 +630,7 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
 	unsigned long max_state;
 	int result, ret;
 
-	if (trip >= tz->trips || trip < 0)
+	if (trip >= tz->ntrips || trip < 0)
 		return -EINVAL;
 
 	list_for_each_entry(pos1, &thermal_tz_list, node) {
@@ -811,7 +811,7 @@ static void __bind(struct thermal_zone_device *tz, int mask,
 {
 	int i, ret;
 
-	for (i = 0; i < tz->trips; i++) {
+	for (i = 0; i < tz->ntrips; i++) {
 		if (mask & (1 << i)) {
 			unsigned long upper, lower;
 
@@ -1057,7 +1057,7 @@ static void __unbind(struct thermal_zone_device *tz, int mask,
 {
 	int i;
 
-	for (i = 0; i < tz->trips; i++)
+	for (i = 0; i < tz->ntrips; i++)
 		if (mask & (1 << i))
 			thermal_zone_unbind_cooling_device(tz, i, cdev);
 }
@@ -1169,7 +1169,7 @@ static void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms
 /**
  * thermal_zone_device_register() - register a new thermal zone device
  * @type:	the thermal zone device type
- * @trips:	the number of trip points the thermal zone support
+ * @ntrips:	the number of trip points the thermal zone support
  * @mask:	a bit string indicating the writeablility of trip points
  * @devdata:	private device data
  * @ops:	standard thermal zone device callbacks
@@ -1191,7 +1191,7 @@ static void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms
  * IS_ERR*() helpers.
  */
 struct thermal_zone_device *
-thermal_zone_device_register(const char *type, int trips, int mask,
+thermal_zone_device_register(const char *type, int ntrips, int mask,
 			     void *devdata, struct thermal_zone_device_ops *ops,
 			     struct thermal_zone_params *tzp, int passive_delay,
 			     int polling_delay)
@@ -1215,7 +1215,7 @@ thermal_zone_device_register(const char *type, int trips, int mask,
 		return ERR_PTR(-EINVAL);
 	}
 
-	if (trips > THERMAL_MAX_TRIPS || trips < 0 || mask >> trips) {
+	if (ntrips > THERMAL_MAX_TRIPS || ntrips < 0 || mask >> ntrips) {
 		pr_err("Incorrect number of thermal trips\n");
 		return ERR_PTR(-EINVAL);
 	}
@@ -1225,7 +1225,7 @@ thermal_zone_device_register(const char *type, int trips, int mask,
 		return ERR_PTR(-EINVAL);
 	}
 
-	if (trips > 0 && (!ops->get_trip_type || !ops->get_trip_temp))
+	if (ntrips > 0 && (!ops->get_trip_type || !ops->get_trip_temp))
 		return ERR_PTR(-EINVAL);
 
 	tz = kzalloc(sizeof(*tz), GFP_KERNEL);
@@ -1255,7 +1255,7 @@ thermal_zone_device_register(const char *type, int trips, int mask,
 	tz->tzp = tzp;
 	tz->device.class = &thermal_class;
 	tz->devdata = devdata;
-	tz->trips = trips;
+	tz->ntrips = ntrips;
 
 	thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
 	thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
@@ -1273,7 +1273,7 @@ thermal_zone_device_register(const char *type, int trips, int mask,
 	if (result)
 		goto release_device;
 
-	for (count = 0; count < trips; count++) {
+	for (count = 0; count < ntrips; count++) {
 		if (tz->ops->get_trip_type(tz, count, &trip_type) ||
 		    tz->ops->get_trip_temp(tz, count, &trip_temp) ||
 		    !trip_temp)
diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
index 60bfda1a1db7..c6bb8c7638da 100644
--- a/drivers/thermal/thermal_helpers.c
+++ b/drivers/thermal/thermal_helpers.c
@@ -89,7 +89,7 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
 	ret = tz->ops->get_temp(tz, temp);
 
 	if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz->emul_temperature) {
-		for (count = 0; count < tz->trips; count++) {
+		for (count = 0; count < tz->ntrips; count++) {
 			ret = tz->ops->get_trip_type(tz, count, &type);
 			if (!ret && type == THERMAL_TRIP_CRITICAL) {
 				ret = tz->ops->get_trip_temp(tz, count,
@@ -137,7 +137,7 @@ void thermal_zone_set_trips(struct thermal_zone_device *tz)
 	if (!tz->ops->set_trips || !tz->ops->get_trip_hyst)
 		goto exit;
 
-	for (i = 0; i < tz->trips; i++) {
+	for (i = 0; i < tz->ntrips; i++) {
 		int trip_low;
 
 		tz->ops->get_trip_temp(tz, i, &trip_temp);
diff --git a/drivers/thermal/thermal_netlink.c b/drivers/thermal/thermal_netlink.c
index 32fea5174cc0..122552937353 100644
--- a/drivers/thermal/thermal_netlink.c
+++ b/drivers/thermal/thermal_netlink.c
@@ -469,7 +469,7 @@ static int thermal_genl_cmd_tz_get_trip(struct param *p)
 
 	mutex_lock(&tz->lock);
 
-	for (i = 0; i < tz->trips; i++) {
+	for (i = 0; i < tz->ntrips; i++) {
 
 		enum thermal_trip_type type;
 		int temp, hyst = 0;
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index 1c4aac8464a7..e12f35eee0e8 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -416,15 +416,15 @@ static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
 	int indx;
 
 	/* This function works only for zones with at least one trip */
-	if (tz->trips <= 0)
+	if (tz->ntrips <= 0)
 		return -EINVAL;
 
-	tz->trip_type_attrs = kcalloc(tz->trips, sizeof(*tz->trip_type_attrs),
+	tz->trip_type_attrs = kcalloc(tz->ntrips, sizeof(*tz->trip_type_attrs),
 				      GFP_KERNEL);
 	if (!tz->trip_type_attrs)
 		return -ENOMEM;
 
-	tz->trip_temp_attrs = kcalloc(tz->trips, sizeof(*tz->trip_temp_attrs),
+	tz->trip_temp_attrs = kcalloc(tz->ntrips, sizeof(*tz->trip_temp_attrs),
 				      GFP_KERNEL);
 	if (!tz->trip_temp_attrs) {
 		kfree(tz->trip_type_attrs);
@@ -432,7 +432,7 @@ static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
 	}
 
 	if (tz->ops->get_trip_hyst) {
-		tz->trip_hyst_attrs = kcalloc(tz->trips,
+		tz->trip_hyst_attrs = kcalloc(tz->ntrips,
 					      sizeof(*tz->trip_hyst_attrs),
 					      GFP_KERNEL);
 		if (!tz->trip_hyst_attrs) {
@@ -442,7 +442,7 @@ static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
 		}
 	}
 
-	attrs = kcalloc(tz->trips * 3 + 1, sizeof(*attrs), GFP_KERNEL);
+	attrs = kcalloc(tz->ntrips * 3 + 1, sizeof(*attrs), GFP_KERNEL);
 	if (!attrs) {
 		kfree(tz->trip_type_attrs);
 		kfree(tz->trip_temp_attrs);
@@ -451,7 +451,7 @@ static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
 		return -ENOMEM;
 	}
 
-	for (indx = 0; indx < tz->trips; indx++) {
+	for (indx = 0; indx < tz->ntrips; indx++) {
 		/* create trip type attribute */
 		snprintf(tz->trip_type_attrs[indx].name, THERMAL_NAME_LENGTH,
 			 "trip_point_%d_type", indx);
@@ -478,7 +478,7 @@ static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
 			tz->trip_temp_attrs[indx].attr.store =
 							trip_point_temp_store;
 		}
-		attrs[indx + tz->trips] = &tz->trip_temp_attrs[indx].attr.attr;
+		attrs[indx + tz->ntrips] = &tz->trip_temp_attrs[indx].attr.attr;
 
 		/* create Optional trip hyst attribute */
 		if (!tz->ops->get_trip_hyst)
@@ -496,10 +496,10 @@ static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
 			tz->trip_hyst_attrs[indx].attr.store =
 					trip_point_hyst_store;
 		}
-		attrs[indx + tz->trips * 2] =
+		attrs[indx + tz->ntrips * 2] =
 					&tz->trip_hyst_attrs[indx].attr.attr;
 	}
-	attrs[tz->trips * 3] = NULL;
+	attrs[tz->ntrips * 3] = NULL;
 
 	tz->trips_attribute_group.attrs = attrs;
 
@@ -540,7 +540,7 @@ int thermal_zone_create_device_groups(struct thermal_zone_device *tz,
 	for (i = 0; i < size - 2; i++)
 		groups[i] = thermal_zone_attribute_groups[i];
 
-	if (tz->trips) {
+	if (tz->ntrips) {
 		result = create_trip_attrs(tz, mask);
 		if (result) {
 			kfree(groups);
@@ -561,7 +561,7 @@ void thermal_zone_destroy_device_groups(struct thermal_zone_device *tz)
 	if (!tz)
 		return;
 
-	if (tz->trips)
+	if (tz->ntrips)
 		destroy_trip_attrs(tz);
 
 	kfree(tz->device.groups);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 6289b0bb1c97..3a57878a2a6c 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -165,7 +165,7 @@ struct thermal_zone_device {
 	struct thermal_attr *trip_hyst_attrs;
 	enum thermal_device_mode mode;
 	void *devdata;
-	int trips;
+	int ntrips;
 	unsigned long trips_disabled;	/* bitmap for disabled trips */
 	unsigned long passive_delay_jiffies;
 	unsigned long polling_delay_jiffies;
-- 
2.25.1

[PATCH v3 08/12] thermal/core: Add thermal_trip in thermal_zone

[Daniel Lezcano]

The thermal trip points are properties of a thermal zone and the
different sub systems should be able to save them in the thermal zone
structure instead of having their own definition.

Give the opportunity to the drivers to create a thermal zone with
thermal trips which will be accessible directly from the thermal core
framework.

Cc: Alexandre Bailon <abailon@baylibre.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc; Eduardo Valentin <eduval@amazon.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
---
 drivers/thermal/thermal_core.h | 10 ++++++++++
 include/linux/thermal.h        |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index c991bb290512..84e341c1e0fc 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -113,6 +113,16 @@ int thermal_build_list_of_policies(char *buf);
 /* Helpers */
 void thermal_zone_set_trips(struct thermal_zone_device *tz);
 
+static inline struct thermal_trip *thermal_zone_get_trips(struct thermal_zone *tz)
+{
+	return tz->trips;
+}
+
+static inline int thermal_zone_get_ntrips(struct thermal_zone *tz)
+{
+	return tz->ntrips;
+}
+
 /* sysfs I/F */
 int thermal_zone_create_device_groups(struct thermal_zone_device *, int);
 void thermal_zone_destroy_device_groups(struct thermal_zone_device *);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 3a57878a2a6c..3733e23b6359 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -179,6 +179,7 @@ struct thermal_zone_device {
 	struct thermal_zone_device_ops *ops;
 	struct thermal_zone_params *tzp;
 	struct thermal_governor *governor;
+	struct thermal_trip *trips;
 	void *governor_data;
 	struct list_head thermal_instances;
 	struct ida ida;
-- 
2.25.1

[PATCH v3 09/12] thermal/core: Register with the trip points

[Daniel Lezcano]

As we added the thermal trip points structure in the thermal zone,
let's extend the thermal zone register function to have the thermal
trip structures as a parameter and store it in the 'trips' field of
the thermal zone structure.

Cc: Alexandre Bailon <abailon@baylibre.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc; Eduardo Valentin <eduval@amazon.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
---
 drivers/thermal/thermal_core.c | 22 +++++++++++++++++-----
 drivers/thermal/thermal_core.h |  4 ++--
 include/linux/thermal.h        |  6 ++++++
 3 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 434a675da245..e865c41d2320 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1167,8 +1167,9 @@ static void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms
 }
 
 /**
- * thermal_zone_device_register() - register a new thermal zone device
+ * thermal_zone_device_register_with_trips() - register a new thermal zone device
  * @type:	the thermal zone device type
+ * @trips:	a pointer to an array of thermal trips
  * @ntrips:	the number of trip points the thermal zone support
  * @mask:	a bit string indicating the writeablility of trip points
  * @devdata:	private device data
@@ -1191,10 +1192,10 @@ static void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms
  * IS_ERR*() helpers.
  */
 struct thermal_zone_device *
-thermal_zone_device_register(const char *type, int ntrips, int mask,
-			     void *devdata, struct thermal_zone_device_ops *ops,
-			     struct thermal_zone_params *tzp, int passive_delay,
-			     int polling_delay)
+thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *trips, int ntrips, int mask,
+					void *devdata, struct thermal_zone_device_ops *ops,
+					struct thermal_zone_params *tzp, int passive_delay,
+					int polling_delay)
 {
 	struct thermal_zone_device *tz;
 	enum thermal_trip_type trip_type;
@@ -1256,6 +1257,7 @@ thermal_zone_device_register(const char *type, int ntrips, int mask,
 	tz->device.class = &thermal_class;
 	tz->devdata = devdata;
 	tz->ntrips = ntrips;
+	tz->trips = trips;
 
 	thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
 	thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
@@ -1331,6 +1333,16 @@ thermal_zone_device_register(const char *type, int ntrips, int mask,
 	kfree(tz);
 	return ERR_PTR(result);
 }
+
+struct thermal_zone_device *thermal_zone_device_register(const char *type, int ntrips, int mask,
+							 void *devdata, struct thermal_zone_device_ops *ops,
+							 struct thermal_zone_params *tzp, int passive_delay,
+							 int polling_delay)
+{
+	return thermal_zone_device_register_with_trips(type, NULL, ntrips, mask,
+						       devdata, ops, tzp,
+						       passive_delay, polling_delay);
+}
 EXPORT_SYMBOL_GPL(thermal_zone_device_register);
 
 /**
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 84e341c1e0fc..bbe3ec26d12e 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -113,12 +113,12 @@ int thermal_build_list_of_policies(char *buf);
 /* Helpers */
 void thermal_zone_set_trips(struct thermal_zone_device *tz);
 
-static inline struct thermal_trip *thermal_zone_get_trips(struct thermal_zone *tz)
+static inline struct thermal_trip *thermal_zone_get_trips(struct thermal_zone_device *tz)
 {
 	return tz->trips;
 }
 
-static inline int thermal_zone_get_ntrips(struct thermal_zone *tz)
+static inline int thermal_zone_get_ntrips(struct thermal_zone_device *tz)
 {
 	return tz->ntrips;
 }
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 3733e23b6359..8cbe237a92d0 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -379,8 +379,14 @@ void devm_thermal_zone_of_sensor_unregister(struct device *dev,
 struct thermal_zone_device *thermal_zone_device_register(const char *, int, int,
 		void *, struct thermal_zone_device_ops *,
 		struct thermal_zone_params *, int, int);
+
 void thermal_zone_device_unregister(struct thermal_zone_device *);
 
+struct thermal_zone_device *
+thermal_zone_device_register_with_trips(const char *, struct thermal_trip *, int, int,
+					void *, struct thermal_zone_device_ops *,
+					struct thermal_zone_params *, int, int);
+
 int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
 				     struct thermal_cooling_device *,
 				     unsigned long, unsigned long,
-- 
2.25.1

[PATCH v3 10/12] thermal/of: Store the trips in the thermal zone

[Daniel Lezcano]

As the thermal zone contains the trip point, we can store them
directly in the when registering the thermal zone. That will allow
another step forward to remove the duplicate thermal zone structure we
find in the thermal_of code.

Cc: Alexandre Bailon <abailon@baylibre.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc; Eduardo Valentin <eduval@amazon.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
---
 drivers/thermal/thermal_of.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index 16eb18c24430..16b6b90a2390 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -1117,11 +1117,9 @@ int __init of_parse_thermal_zones(void)
 		tzp->slope = tz->slope;
 		tzp->offset = tz->offset;
 
-		zone = thermal_zone_device_register(child->name, tz->ntrips,
-						    mask, tz,
-						    ops, tzp,
-						    tz->passive_delay,
-						    tz->polling_delay);
+		zone = thermal_zone_device_register_with_trips(child->name, tz->trips, tz->ntrips,
+							       mask, tz, ops, tzp, tz->passive_delay,
+							       tz->polling_delay);
 		if (IS_ERR(zone)) {
 			pr_err("Failed to build %pOFn zone %ld\n", child,
 			       PTR_ERR(zone));
-- 
2.25.1

[PATCH v3 11/12] thermal/of: Use thermal trips stored in the thermal zone

[Daniel Lezcano]

Now that we have the thermal trip stored in the thermal zone in a
generic way, we can rely on them and remove one indirection we found
in the thermal_of code and do one more step forward the removal of the
duplicated structures.

Cc: Alexandre Bailon <abailon@baylibre.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc; Eduardo Valentin <eduval@amazon.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
---
 drivers/thermal/thermal_of.c | 53 +++++++++++-------------------------
 1 file changed, 16 insertions(+), 37 deletions(-)

diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index 16b6b90a2390..bc885729bf23 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -118,12 +118,7 @@ static int of_thermal_set_trips(struct thermal_zone_device *tz,
  */
 int of_thermal_get_ntrips(struct thermal_zone_device *tz)
 {
-	struct __thermal_zone *data = tz->devdata;
-
-	if (!data || IS_ERR(data))
-		return -ENODEV;
-
-	return data->ntrips;
+	return tz->ntrips;
 }
 EXPORT_SYMBOL_GPL(of_thermal_get_ntrips);
 
@@ -139,9 +134,7 @@ EXPORT_SYMBOL_GPL(of_thermal_get_ntrips);
  */
 bool of_thermal_is_trip_valid(struct thermal_zone_device *tz, int trip)
 {
-	struct __thermal_zone *data = tz->devdata;
-
-	if (!data || trip >= data->ntrips || trip < 0)
+	if (trip >= tz->ntrips || trip < 0)
 		return false;
 
 	return true;
@@ -161,12 +154,7 @@ EXPORT_SYMBOL_GPL(of_thermal_is_trip_valid);
 const struct thermal_trip *
 of_thermal_get_trip_points(struct thermal_zone_device *tz)
 {
-	struct __thermal_zone *data = tz->devdata;
-
-	if (!data)
-		return NULL;
-
-	return data->trips;
+	return tz->trips;
 }
 EXPORT_SYMBOL_GPL(of_thermal_get_trip_points);
 
@@ -281,12 +269,10 @@ static int of_thermal_unbind(struct thermal_zone_device *thermal,
 static int of_thermal_get_trip_type(struct thermal_zone_device *tz, int trip,
 				    enum thermal_trip_type *type)
 {
-	struct __thermal_zone *data = tz->devdata;
-
-	if (trip >= data->ntrips || trip < 0)
+	if (trip >= tz->ntrips || trip < 0)
 		return -EDOM;
 
-	*type = data->trips[trip].type;
+	*type = tz->trips[trip].type;
 
 	return 0;
 }
@@ -294,12 +280,10 @@ static int of_thermal_get_trip_type(struct thermal_zone_device *tz, int trip,
 static int of_thermal_get_trip_temp(struct thermal_zone_device *tz, int trip,
 				    int *temp)
 {
-	struct __thermal_zone *data = tz->devdata;
-
-	if (trip >= data->ntrips || trip < 0)
+	if (trip >= tz->ntrips || trip < 0)
 		return -EDOM;
 
-	*temp = data->trips[trip].temperature;
+	*temp = tz->trips[trip].temperature;
 
 	return 0;
 }
@@ -309,7 +293,7 @@ static int of_thermal_set_trip_temp(struct thermal_zone_device *tz, int trip,
 {
 	struct __thermal_zone *data = tz->devdata;
 
-	if (trip >= data->ntrips || trip < 0)
+	if (trip >= tz->ntrips || trip < 0)
 		return -EDOM;
 
 	if (data->ops && data->ops->set_trip_temp) {
@@ -321,7 +305,7 @@ static int of_thermal_set_trip_temp(struct thermal_zone_device *tz, int trip,
 	}
 
 	/* thermal framework should take care of data->mask & (1 << trip) */
-	data->trips[trip].temperature = temp;
+	tz->trips[trip].temperature = temp;
 
 	return 0;
 }
@@ -329,12 +313,10 @@ static int of_thermal_set_trip_temp(struct thermal_zone_device *tz, int trip,
 static int of_thermal_get_trip_hyst(struct thermal_zone_device *tz, int trip,
 				    int *hyst)
 {
-	struct __thermal_zone *data = tz->devdata;
-
-	if (trip >= data->ntrips || trip < 0)
+	if (trip >= tz->ntrips || trip < 0)
 		return -EDOM;
 
-	*hyst = data->trips[trip].hysteresis;
+	*hyst = tz->trips[trip].hysteresis;
 
 	return 0;
 }
@@ -342,13 +324,11 @@ static int of_thermal_get_trip_hyst(struct thermal_zone_device *tz, int trip,
 static int of_thermal_set_trip_hyst(struct thermal_zone_device *tz, int trip,
 				    int hyst)
 {
-	struct __thermal_zone *data = tz->devdata;
-
-	if (trip >= data->ntrips || trip < 0)
+	if (trip >= tz->ntrips || trip < 0)
 		return -EDOM;
 
 	/* thermal framework should take care of data->mask & (1 << trip) */
-	data->trips[trip].hysteresis = hyst;
+	tz->trips[trip].hysteresis = hyst;
 
 	return 0;
 }
@@ -356,12 +336,11 @@ static int of_thermal_set_trip_hyst(struct thermal_zone_device *tz, int trip,
 static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
 				    int *temp)
 {
-	struct __thermal_zone *data = tz->devdata;
 	int i;
 
-	for (i = 0; i < data->ntrips; i++)
-		if (data->trips[i].type == THERMAL_TRIP_CRITICAL) {
-			*temp = data->trips[i].temperature;
+	for (i = 0; i < tz->ntrips; i++)
+		if (tz->trips[i].type == THERMAL_TRIP_CRITICAL) {
+			*temp = tz->trips[i].temperature;
 			return 0;
 		}
 
-- 
2.25.1

[PATCH v3 12/12] thermal/of: Initialize trip points separately

[Daniel Lezcano]

Self contain the trip initialization from the device tree in a single
function for the sake of making the code flow more clear.

Cc: Alexandre Bailon <abailon@baylibre.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc; Eduardo Valentin <eduval@amazon.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
---
 drivers/thermal/thermal_of.c | 84 ++++++++++++++++++++++++------------
 1 file changed, 57 insertions(+), 27 deletions(-)

diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index bc885729bf23..1aa52df507b6 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -693,7 +693,8 @@ static int of_find_trip_id(struct device_node *np, struct device_node *trip)
  *
  * Return: 0 on success, proper error code otherwise
  */
-static int thermal_of_populate_bind_params(struct device_node *np,
+static int thermal_of_populate_bind_params(struct device_node *tz_np,
+					   struct device_node *np,
 					   struct __thermal_bind_params *__tbp)
 {
 	struct of_phandle_args cooling_spec;
@@ -715,7 +716,7 @@ static int thermal_of_populate_bind_params(struct device_node *np,
 		return -ENODEV;
 	}
 
-	trip_id = of_find_trip_id(np, trip);
+	trip_id = of_find_trip_id(tz_np, trip);
 	if (trip_id < 0) {
 		ret = trip_id;
 		goto end;
@@ -849,6 +850,53 @@ static int thermal_of_populate_trip(struct device_node *np,
 	return 0;
 }
 
+static struct thermal_trip *thermal_of_trips_init(struct device_node *np, int *ntrips)
+{
+	struct thermal_trip *tt;
+	struct device_node *trips, *trip;
+	int ret, count;
+
+	trips = of_get_child_by_name(np, "trips");
+	if (!trips) {
+		pr_err("Failed to find 'trips' node\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	count = of_get_child_count(trips);
+	if (!count) {
+		pr_err("No trip point defined\n");
+		ret = -EINVAL;
+		goto out_of_node_put;
+	}
+
+	tt = kzalloc(sizeof(*tt) * count, GFP_KERNEL);
+	if (!tt) {
+		ret = -ENOMEM;
+		goto out_of_node_put;
+	}
+
+	*ntrips = count;
+
+	count = 0;
+	for_each_child_of_node(trips, trip) {
+		ret = thermal_of_populate_trip(trip, &tt[count++]);
+		if (ret)
+			goto out_kfree;
+	}
+
+	of_node_put(trips);
+	
+	return tt;
+	
+out_kfree:
+	kfree(tt);
+	*ntrips = 0;
+out_of_node_put:
+	of_node_put(trips);
+
+	return ERR_PTR(ret);
+}
+
 /**
  * thermal_of_build_thermal_zone - parse and fill one thermal zone data
  * @np: DT node containing a thermal zone node
@@ -867,7 +915,6 @@ static struct __thermal_zone
 __init *thermal_of_build_thermal_zone(struct device_node *np)
 {
 	struct device_node *child = NULL, *gchild;
-	struct device_node *trips;
 	struct __thermal_zone *tz;
 	int ret, i;
 	u32 prop, coef[2];
@@ -909,28 +956,10 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
 		tz->offset = 0;
 	}
 
-	/* trips */
-	trips = of_get_child_by_name(np, "trips");
-
-	/* No trips provided */
-	if (!trips)
-		goto finish;
-
-	tz->ntrips = of_get_child_count(trips);
-	if (tz->ntrips == 0) /* must have at least one child */
+	tz->trips = thermal_of_trips_init(np, &tz->ntrips);
+	if (IS_ERR(tz->trips)) {
+		ret = PTR_ERR(tz->trips);
 		goto finish;
-
-	tz->trips = kcalloc(tz->ntrips, sizeof(*tz->trips), GFP_KERNEL);
-	if (!tz->trips) {
-		ret = -ENOMEM;
-		goto free_tz;
-	}
-
-	i = 0;
-	for_each_child_of_node(trips, gchild) {
-		ret = thermal_of_populate_trip(gchild, &tz->trips[i++]);
-		if (ret)
-			goto free_trips;
 	}
 
 	/* cooling-maps */
@@ -952,9 +981,11 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
 
 	i = 0;
 	for_each_child_of_node(child, gchild) {
-		ret = thermal_of_populate_bind_params(gchild, &tz->tbps[i++]);
-		if (ret)
+		ret = thermal_of_populate_bind_params(np, gchild, &tz->tbps[i++]);
+		if (ret) {
+			of_node_put(gchild);
 			goto free_tbps;
+		}
 	}
 
 finish:
@@ -976,7 +1007,6 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
 	kfree(tz->tbps);
 free_trips:
 	kfree(tz->trips);
-	of_node_put(gchild);
 free_tz:
 	kfree(tz);
 	of_node_put(child);
-- 
2.25.1

Re: [PATCH v3 05/12] thermal/core: Remove unneeded EXPORT_SYMBOLS

[Lukasz Luba]

Hi Daniel,

(+Todd and Wei on CC)


On 7/3/22 19:30, Daniel Lezcano wrote:
> Different functions are exporting the symbols but are actually only
> used by the thermal framework internals. Remove these EXPORT_SYMBOLS.
> 
> Cc: Alexandre Bailon <abailon@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc; Eduardo Valentin <eduval@amazon.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> ---
>   drivers/thermal/thermal_helpers.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
> index 3edd047e144f..f4c1e87ef040 100644
> --- a/drivers/thermal/thermal_helpers.c
> +++ b/drivers/thermal/thermal_helpers.c
> @@ -39,7 +39,6 @@ int get_tz_trend(struct thermal_zone_device *tz, int trip)
>   
>   	return trend;
>   }
> -EXPORT_SYMBOL(get_tz_trend);
>   
>   struct thermal_instance *
>   get_thermal_instance(struct thermal_zone_device *tz,
> @@ -228,7 +227,6 @@ void thermal_cdev_update(struct thermal_cooling_device *cdev)
>   	}
>   	mutex_unlock(&cdev->lock);
>   }
> -EXPORT_SYMBOL(thermal_cdev_update);

I wouldn't remove that export. I can see in my Pixel6 modules dir, that
it's called in 7 places.

I assume that in Android world this is common use.

Regards,
Lukasz

Re: [PATCH v3 01/12] thermal/core: Remove duplicate information when an error occurs

[Lukasz Luba]

On 7/3/22 19:30, Daniel Lezcano wrote:
> The pr_err already tells it is an error, it is pointless to add the
> 'Error:' string in the messages. Remove them.
> 
> Cc: Alexandre Bailon <abailon@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc; Eduardo Valentin <eduval@amazon.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> ---
>   drivers/thermal/thermal_core.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index cdc0552e8c42..e22e7d939c54 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1198,23 +1198,23 @@ thermal_zone_device_register(const char *type, int trips, int mask,
>   	struct thermal_governor *governor;
>   
>   	if (!type || strlen(type) == 0) {
> -		pr_err("Error: No thermal zone type defined\n");
> +		pr_err("No thermal zone type defined\n");
>   		return ERR_PTR(-EINVAL);
>   	}
>   
>   	if (type && strlen(type) >= THERMAL_NAME_LENGTH) {
> -		pr_err("Error: Thermal zone name (%s) too long, should be under %d chars\n",
> +		pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
>   		       type, THERMAL_NAME_LENGTH);
>   		return ERR_PTR(-EINVAL);
>   	}
>   
>   	if (trips > THERMAL_MAX_TRIPS || trips < 0 || mask >> trips) {
> -		pr_err("Error: Incorrect number of thermal trips\n");
> +		pr_err("Incorrect number of thermal trips\n");
>   		return ERR_PTR(-EINVAL);
>   	}
>   
>   	if (!ops) {
> -		pr_err("Error: Thermal zone device ops not defined\n");
> +		pr_err("Thermal zone device ops not defined\n");
>   		return ERR_PTR(-EINVAL);
>   	}
>   

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

Re: [PATCH v3 02/12] thermal/of: Replace device node match with device node search

[Lukasz Luba]

On 7/3/22 19:30, Daniel Lezcano wrote:
> The thermal_of code builds a trip array associated with the node
> pointer in order to compare the trip point phandle with the list.
> 
> The thermal trip is a thermal zone property and should be moved
> there. If some sensors have hardcoded trip points, they should use the
> exported structure instead of redefining again and again their own
> structure and data to describe exactly the same things.
> 
> In order to move this to the thermal.h header and allow more cleanup,
> we need to remove the node pointer from the structure.
> 
> Instead of building storing the device node, we search directly in the
> device tree the corresponding node. That results in a simplification
> of the code and allows to move the structure to thermal.h
> 
> Cc: Alexandre Bailon <abailon@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc; Eduardo Valentin <eduval@amazon.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> ---
>   drivers/thermal/thermal_of.c | 62 ++++++++++++++++++++++++------------
>   1 file changed, 42 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index b65d435cb92f..04c910ca8623 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -671,6 +671,35 @@ EXPORT_SYMBOL_GPL(devm_thermal_zone_of_sensor_unregister);
>   
>   /***   functions parsing device tree nodes   ***/
>   
> +static int of_find_trip_id(struct device_node *np, struct device_node *trip)
> +{
> +	struct device_node *trips;
> +	struct device_node *t;
> +	int i = 0;
> +
> +	trips = of_get_child_by_name(np, "trips");
> +	if (!trips) {
> +		pr_err("Failed to find 'trips' node\n");
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * Find the trip id point associated with the cooling device map
> +	 */
> +	for_each_child_of_node(trips, t) {
> +
> +		if (t == trip)
> +			goto out;
> +		i++;
> +	}
> +
> +	i = -ENXIO;
> +out:	
> +	of_node_put(trips);
> +
> +	return i;
> +}
> +
>   /**
>    * thermal_of_populate_bind_params - parse and fill cooling map data
>    * @np: DT node containing a cooling-map node
> @@ -686,14 +715,13 @@ EXPORT_SYMBOL_GPL(devm_thermal_zone_of_sensor_unregister);
>    * Return: 0 on success, proper error code otherwise
>    */
>   static int thermal_of_populate_bind_params(struct device_node *np,
> -					   struct __thermal_bind_params *__tbp,
> -					   struct thermal_trip *trips,
> -					   int ntrips)
> +					   struct __thermal_bind_params *__tbp)
>   {
>   	struct of_phandle_args cooling_spec;
>   	struct __thermal_cooling_bind_param *__tcbp;
>   	struct device_node *trip;
>   	int ret, i, count;
> +	int trip_id;
>   	u32 prop;
>   
>   	/* Default weight. Usage is optional */
> @@ -708,18 +736,14 @@ static int thermal_of_populate_bind_params(struct device_node *np,
>   		return -ENODEV;
>   	}
>   
> -	/* match using device_node */
> -	for (i = 0; i < ntrips; i++)
> -		if (trip == trips[i].np) {
> -			__tbp->trip_id = i;
> -			break;
> -		}
> -
> -	if (i == ntrips) {
> -		ret = -ENODEV;
> +	trip_id = of_find_trip_id(np, trip);
> +	if (trip_id < 0) {
> +		ret = trip_id;
>   		goto end;
>   	}
>   
> +	__tbp->trip_id = trip_id;
> +	
>   	count = of_count_phandle_with_args(np, "cooling-device",
>   					   "#cooling-cells");
>   	if (count <= 0) {
> @@ -868,6 +892,7 @@ static struct __thermal_zone
>   __init *thermal_of_build_thermal_zone(struct device_node *np)
>   {
>   	struct device_node *child = NULL, *gchild;
> +	struct device_node *trips;
>   	struct __thermal_zone *tz;
>   	int ret, i;
>   	u32 prop, coef[2];
> @@ -910,13 +935,13 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>   	}
>   
>   	/* trips */
> -	child = of_get_child_by_name(np, "trips");
> +	trips = of_get_child_by_name(np, "trips");
>   
>   	/* No trips provided */
> -	if (!child)
> +	if (!trips)
>   		goto finish;
>   
> -	tz->ntrips = of_get_child_count(child);
> +	tz->ntrips = of_get_child_count(trips);
>   	if (tz->ntrips == 0) /* must have at least one child */
>   		goto finish;
>   
> @@ -927,14 +952,12 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>   	}
>   
>   	i = 0;
> -	for_each_child_of_node(child, gchild) {
> +	for_each_child_of_node(trips, gchild) {
>   		ret = thermal_of_populate_trip(gchild, &tz->trips[i++]);
>   		if (ret)
>   			goto free_trips;
>   	}
>   
> -	of_node_put(child);
> -

It's probably needed to put the 'trips' here, isn't it?
Or at the end in 'free_trips'.


>   	/* cooling-maps */
>   	child = of_get_child_by_name(np, "cooling-maps");
>   
> @@ -954,8 +977,7 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>   
>   	i = 0;
>   	for_each_child_of_node(child, gchild) {
> -		ret = thermal_of_populate_bind_params(gchild, &tz->tbps[i++],
> -						      tz->trips, tz->ntrips);
> +		ret = thermal_of_populate_bind_params(gchild, &tz->tbps[i++]);
>   		if (ret)
>   			goto free_tbps;
>   	}

Re: [PATCH v3 03/12] thermal/of: Remove the device node pointer for thermal_trip

[Lukasz Luba]

On 7/3/22 19:30, Daniel Lezcano wrote:
> The device node pointer is no longer needed in the thermal trip
> structure, remove it.
> 
> Cc: Alexandre Bailon <abailon@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc; Eduardo Valentin <eduval@amazon.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> ---
>   drivers/thermal/thermal_core.h | 2 --
>   drivers/thermal/thermal_of.c   | 8 --------
>   2 files changed, 10 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
> index 726e327b4205..ff10cdda056c 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -70,13 +70,11 @@ void __thermal_cdev_update(struct thermal_cooling_device *cdev);
>   
>   /**
>    * struct thermal_trip - representation of a point in temperature domain
> - * @np: pointer to struct device_node that this trip point was created from
>    * @temperature: temperature value in miliCelsius
>    * @hysteresis: relative hysteresis in miliCelsius
>    * @type: trip point type
>    */
>   struct thermal_trip {
> -	struct device_node *np;
>   	int temperature;
>   	int hysteresis;
>   	enum thermal_trip_type type;
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index 04c910ca8623..16eb18c24430 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -867,10 +867,6 @@ static int thermal_of_populate_trip(struct device_node *np,
>   		return ret;
>   	}
>   
> -	/* Required for cooling map matching */
> -	trip->np = np;
> -	of_node_get(np);
> -
>   	return 0;
>   }
>   
> @@ -1000,8 +996,6 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>   
>   	kfree(tz->tbps);
>   free_trips:
> -	for (i = 0; i < tz->ntrips; i++)
> -		of_node_put(tz->trips[i].np);
>   	kfree(tz->trips);
>   	of_node_put(gchild);
>   free_tz:
> @@ -1026,8 +1020,6 @@ static __init void of_thermal_free_zone(struct __thermal_zone *tz)
>   	}
>   
>   	kfree(tz->tbps);
> -	for (i = 0; i < tz->ntrips; i++)
> -		of_node_put(tz->trips[i].np);
>   	kfree(tz->trips);
>   	kfree(tz);
>   }

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

Re: [PATCH v3 04/12] thermal/of: Move thermal_trip structure to thermal.h

[Lukasz Luba]

On 7/3/22 19:30, Daniel Lezcano wrote:
> The structure thermal_trip is now generic and will be usable by the
> different sensor drivers in place of their own structure.
> 
> Move its definition to thermal.h to make it accessible.
> 
> Cc: Alexandre Bailon <abailon@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc; Eduardo Valentin <eduval@amazon.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> ---
>   drivers/thermal/thermal_core.h | 12 ------------
>   include/linux/thermal.h        | 12 ++++++++++++
>   2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
> index ff10cdda056c..60844e2d59bb 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -68,18 +68,6 @@ static inline bool cdev_is_power_actor(struct thermal_cooling_device *cdev)
>   void thermal_cdev_update(struct thermal_cooling_device *);
>   void __thermal_cdev_update(struct thermal_cooling_device *cdev);
>   
> -/**
> - * struct thermal_trip - representation of a point in temperature domain
> - * @temperature: temperature value in miliCelsius
> - * @hysteresis: relative hysteresis in miliCelsius
> - * @type: trip point type
> - */
> -struct thermal_trip {
> -	int temperature;
> -	int hysteresis;
> -	enum thermal_trip_type type;
> -};
> -
>   int get_tz_trend(struct thermal_zone_device *tz, int trip);
>   
>   struct thermal_instance *
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 365733b428d8..6289b0bb1c97 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -80,6 +80,18 @@ struct thermal_zone_device_ops {
>   	void (*critical)(struct thermal_zone_device *);
>   };
>   
> +/**
> + * struct thermal_trip - representation of a point in temperature domain
> + * @temperature: temperature value in miliCelsius
> + * @hysteresis: relative hysteresis in miliCelsius
> + * @type: trip point type
> + */
> +struct thermal_trip {
> +	int temperature;
> +	int hysteresis;
> +	enum thermal_trip_type type;
> +};
> +
>   struct thermal_cooling_device_ops {
>   	int (*get_max_state) (struct thermal_cooling_device *, unsigned long *);
>   	int (*get_cur_state) (struct thermal_cooling_device *, unsigned long *);


Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

Re: [PATCH v3 06/12] thermal/core: Move thermal_set_delay_jiffies to static

[Lukasz Luba]

On 7/3/22 19:30, Daniel Lezcano wrote:
> The function 'thermal_set_delay_jiffies' is only used in
> thermal_core.c but it is defined and implemented in a separate
> file. Move the function to thermal_core.c and make it static.
> 
> Cc: Alexandre Bailon <abailon@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc; Eduardo Valentin <eduval@amazon.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> ---
>   drivers/thermal/thermal_core.c    | 7 +++++++
>   drivers/thermal/thermal_core.h    | 1 -
>   drivers/thermal/thermal_helpers.c | 7 -------
>   3 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index e22e7d939c54..a8b1628937c6 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1159,6 +1159,13 @@ static void bind_tz(struct thermal_zone_device *tz)
>   	mutex_unlock(&thermal_list_lock);
>   }
>   
> +static void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms)
> +{
> +	*delay_jiffies = msecs_to_jiffies(delay_ms);
> +	if (delay_ms > 1000)
> +		*delay_jiffies = round_jiffies(*delay_jiffies);
> +}
> +
>   /**
>    * thermal_zone_device_register() - register a new thermal zone device
>    * @type:	the thermal zone device type
> diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
> index 60844e2d59bb..c991bb290512 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -112,7 +112,6 @@ int thermal_build_list_of_policies(char *buf);
>   
>   /* Helpers */
>   void thermal_zone_set_trips(struct thermal_zone_device *tz);
> -void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms);
>   
>   /* sysfs I/F */
>   int thermal_zone_create_device_groups(struct thermal_zone_device *, int);
> diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
> index f4c1e87ef040..60bfda1a1db7 100644
> --- a/drivers/thermal/thermal_helpers.c
> +++ b/drivers/thermal/thermal_helpers.c
> @@ -174,13 +174,6 @@ void thermal_zone_set_trips(struct thermal_zone_device *tz)
>   	mutex_unlock(&tz->lock);
>   }
>   
> -void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms)
> -{
> -	*delay_jiffies = msecs_to_jiffies(delay_ms);
> -	if (delay_ms > 1000)
> -		*delay_jiffies = round_jiffies(*delay_jiffies);
> -}
> -
>   static void thermal_cdev_set_cur_state(struct thermal_cooling_device *cdev,
>   				       int target)
>   {

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

Re: [PATCH v3 07/12] thermal/core: Rename trips to ntrips

[Lukasz Luba]

On 7/3/22 19:30, Daniel Lezcano wrote:
> In order to use thermal trips defined in the thermal structure, rename
> the 'trips' field to 'ntrips' to have the 'trips' field containing the
> thermal trip points.
> 
> Cc: Alexandre Bailon <abailon@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc; Eduardo Valentin <eduval@amazon.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> ---
>   drivers/thermal/gov_fair_share.c        |  6 +++---
>   drivers/thermal/gov_power_allocator.c   |  4 ++--
>   drivers/thermal/tegra/tegra30-tsensor.c |  2 +-
>   drivers/thermal/thermal_core.c          | 20 ++++++++++----------
>   drivers/thermal/thermal_helpers.c       |  4 ++--
>   drivers/thermal/thermal_netlink.c       |  2 +-
>   drivers/thermal/thermal_sysfs.c         | 22 +++++++++++-----------
>   include/linux/thermal.h                 |  2 +-
>   8 files changed, 31 insertions(+), 31 deletions(-)


[snip]

> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 6289b0bb1c97..3a57878a2a6c 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h

Missing updated ne name in comment here:
  * @trips:      number of trip points the thermal zone supports


> @@ -165,7 +165,7 @@ struct thermal_zone_device {
>   	struct thermal_attr *trip_hyst_attrs;
>   	enum thermal_device_mode mode;
>   	void *devdata;
> -	int trips;
> +	int ntrips;
>   	unsigned long trips_disabled;	/* bitmap for disabled trips */
>   	unsigned long passive_delay_jiffies;
>   	unsigned long polling_delay_jiffies;

Maybe this is only my bias, but this new name 'ntrips' looks
like negation in electronics.

We have examples like: num_cpus, num_pins, num_leds, num_groups,
num_locks, num_buffers, num_phys, etc...

Could we have 'num_trips' and follow to this convention here as well?

Re: [PATCH v3 09/12] thermal/core: Register with the trip points

[Lukasz Luba]

On 7/3/22 19:30, Daniel Lezcano wrote:
> As we added the thermal trip points structure in the thermal zone,
> let's extend the thermal zone register function to have the thermal
> trip structures as a parameter and store it in the 'trips' field of
> the thermal zone structure.
> 
> Cc: Alexandre Bailon <abailon@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc; Eduardo Valentin <eduval@amazon.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> ---
>   drivers/thermal/thermal_core.c | 22 +++++++++++++++++-----
>   drivers/thermal/thermal_core.h |  4 ++--
>   include/linux/thermal.h        |  6 ++++++
>   3 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 434a675da245..e865c41d2320 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1167,8 +1167,9 @@ static void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms
>   }
>   
>   /**
> - * thermal_zone_device_register() - register a new thermal zone device
> + * thermal_zone_device_register_with_trips() - register a new thermal zone device
>    * @type:	the thermal zone device type
> + * @trips:	a pointer to an array of thermal trips
>    * @ntrips:	the number of trip points the thermal zone support
>    * @mask:	a bit string indicating the writeablility of trip points
>    * @devdata:	private device data
> @@ -1191,10 +1192,10 @@ static void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms
>    * IS_ERR*() helpers.
>    */
>   struct thermal_zone_device *
> -thermal_zone_device_register(const char *type, int ntrips, int mask,
> -			     void *devdata, struct thermal_zone_device_ops *ops,
> -			     struct thermal_zone_params *tzp, int passive_delay,
> -			     int polling_delay)
> +thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *trips, int ntrips, int mask,
> +					void *devdata, struct thermal_zone_device_ops *ops,
> +					struct thermal_zone_params *tzp, int passive_delay,
> +					int polling_delay)
>   {
>   	struct thermal_zone_device *tz;
>   	enum thermal_trip_type trip_type;
> @@ -1256,6 +1257,7 @@ thermal_zone_device_register(const char *type, int ntrips, int mask,
>   	tz->device.class = &thermal_class;
>   	tz->devdata = devdata;
>   	tz->ntrips = ntrips;
> +	tz->trips = trips;
>   
>   	thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
>   	thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
> @@ -1331,6 +1333,16 @@ thermal_zone_device_register(const char *type, int ntrips, int mask,
>   	kfree(tz);
>   	return ERR_PTR(result);
>   }
> +
> +struct thermal_zone_device *thermal_zone_device_register(const char *type, int ntrips, int mask,
> +							 void *devdata, struct thermal_zone_device_ops *ops,
> +							 struct thermal_zone_params *tzp, int passive_delay,
> +							 int polling_delay)
> +{
> +	return thermal_zone_device_register_with_trips(type, NULL, ntrips, mask,
> +						       devdata, ops, tzp,
> +						       passive_delay, polling_delay);
> +}
>   EXPORT_SYMBOL_GPL(thermal_zone_device_register);
>   
>   /**
> diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
> index 84e341c1e0fc..bbe3ec26d12e 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -113,12 +113,12 @@ int thermal_build_list_of_policies(char *buf);
>   /* Helpers */
>   void thermal_zone_set_trips(struct thermal_zone_device *tz);
>   
> -static inline struct thermal_trip *thermal_zone_get_trips(struct thermal_zone *tz)
> +static inline struct thermal_trip *thermal_zone_get_trips(struct thermal_zone_device *tz)
>   {
>   	return tz->trips;
>   }
>   
> -static inline int thermal_zone_get_ntrips(struct thermal_zone *tz)
> +static inline int thermal_zone_get_ntrips(struct thermal_zone_device *tz)
>   {
>   	return tz->ntrips;
>   }
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 3733e23b6359..8cbe237a92d0 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -379,8 +379,14 @@ void devm_thermal_zone_of_sensor_unregister(struct device *dev,
>   struct thermal_zone_device *thermal_zone_device_register(const char *, int, int,
>   		void *, struct thermal_zone_device_ops *,
>   		struct thermal_zone_params *, int, int);
> +
>   void thermal_zone_device_unregister(struct thermal_zone_device *);
>   
> +struct thermal_zone_device *
> +thermal_zone_device_register_with_trips(const char *, struct thermal_trip *, int, int,
> +					void *, struct thermal_zone_device_ops *,
> +					struct thermal_zone_params *, int, int);
> +
>   int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
>   				     struct thermal_cooling_device *,
>   				     unsigned long, unsigned long,

Apart from 'ntrips', which might be 'num_trips', this looks OK.

Re: [PATCH v3 08/12] thermal/core: Add thermal_trip in thermal_zone

[Lukasz Luba]

On 7/3/22 19:30, Daniel Lezcano wrote:
> The thermal trip points are properties of a thermal zone and the
> different sub systems should be able to save them in the thermal zone
> structure instead of having their own definition.
> 
> Give the opportunity to the drivers to create a thermal zone with
> thermal trips which will be accessible directly from the thermal core
> framework.
> 
> Cc: Alexandre Bailon <abailon@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc; Eduardo Valentin <eduval@amazon.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> ---
>   drivers/thermal/thermal_core.h | 10 ++++++++++
>   include/linux/thermal.h        |  1 +
>   2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
> index c991bb290512..84e341c1e0fc 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -113,6 +113,16 @@ int thermal_build_list_of_policies(char *buf);
>   /* Helpers */
>   void thermal_zone_set_trips(struct thermal_zone_device *tz);
>   
> +static inline struct thermal_trip *thermal_zone_get_trips(struct thermal_zone *tz)
> +{
> +	return tz->trips;
> +}
> +
> +static inline int thermal_zone_get_ntrips(struct thermal_zone *tz)
> +{
> +	return tz->ntrips;

'num_trips' ?

> +}
> +
>   /* sysfs I/F */
>   int thermal_zone_create_device_groups(struct thermal_zone_device *, int);
>   void thermal_zone_destroy_device_groups(struct thermal_zone_device *);
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 3a57878a2a6c..3733e23b6359 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h

Missing doc comment new line for this new field.

> @@ -179,6 +179,7 @@ struct thermal_zone_device {
>   	struct thermal_zone_device_ops *ops;
>   	struct thermal_zone_params *tzp;
>   	struct thermal_governor *governor;
> +	struct thermal_trip *trips;

I would group together two lines:
struct thermal_trip *trips;
int num_trips;

>   	void *governor_data;
>   	struct list_head thermal_instances;
>   	struct ida ida;

Re: [PATCH v3 10/12] thermal/of: Store the trips in the thermal zone

[Lukasz Luba]

On 7/3/22 19:30, Daniel Lezcano wrote:
> As the thermal zone contains the trip point, we can store them
> directly in the when registering the thermal zone. That will allow

'directly in the' is there something missing?
maybe: in the thermal zone when registering

> another step forward to remove the duplicate thermal zone structure we
> find in the thermal_of code.
> 
> Cc: Alexandre Bailon <abailon@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc; Eduardo Valentin <eduval@amazon.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> ---
>   drivers/thermal/thermal_of.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index 16eb18c24430..16b6b90a2390 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -1117,11 +1117,9 @@ int __init of_parse_thermal_zones(void)
>   		tzp->slope = tz->slope;
>   		tzp->offset = tz->offset;
>   
> -		zone = thermal_zone_device_register(child->name, tz->ntrips,
> -						    mask, tz,
> -						    ops, tzp,
> -						    tz->passive_delay,
> -						    tz->polling_delay);
> +		zone = thermal_zone_device_register_with_trips(child->name, tz->trips, tz->ntrips,
> +							       mask, tz, ops, tzp, tz->passive_delay,
> +							       tz->polling_delay);
>   		if (IS_ERR(zone)) {
>   			pr_err("Failed to build %pOFn zone %ld\n", child,
>   			       PTR_ERR(zone));

'num_trips' ?
Apart from that LGTM

Re: [PATCH v3 08/12] thermal/core: Add thermal_trip in thermal_zone

[Zhang Rui]

On Sun, 2022-07-03 at 20:30 +0200, Daniel Lezcano wrote:
> The thermal trip points are properties of a thermal zone and the
> different sub systems should be able to save them in the thermal zone
> structure instead of having their own definition.
> 
> Give the opportunity to the drivers to create a thermal zone with
> thermal trips which will be accessible directly from the thermal core
> framework.
> 
> Cc: Alexandre Bailon <abailon@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc; Eduardo Valentin <eduval@amazon.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> ---
>  drivers/thermal/thermal_core.h | 10 ++++++++++
>  include/linux/thermal.h        |  1 +
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/thermal/thermal_core.h
> b/drivers/thermal/thermal_core.h
> index c991bb290512..84e341c1e0fc 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -113,6 +113,16 @@ int thermal_build_list_of_policies(char *buf);
>  /* Helpers */
>  void thermal_zone_set_trips(struct thermal_zone_device *tz);
>  
> +static inline struct thermal_trip *thermal_zone_get_trips(struct
> thermal_zone *tz)

it should be struct thermal_zone_device?
It seems that you fixed it in patch 9/12, and leave it broke here.

> +{
> +	return tz->trips;
> +}
> +
> +static inline int thermal_zone_get_ntrips(struct thermal_zone *tz)

same problem as above.

thanks,
rui
> +{
> +	return tz->ntrips;
> +}
> +
>  /* sysfs I/F */
>  int thermal_zone_create_device_groups(struct thermal_zone_device *,
> int);
>  void thermal_zone_destroy_device_groups(struct thermal_zone_device
> *);
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 3a57878a2a6c..3733e23b6359 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -179,6 +179,7 @@ struct thermal_zone_device {
>  	struct thermal_zone_device_ops *ops;
>  	struct thermal_zone_params *tzp;
>  	struct thermal_governor *governor;
> +	struct thermal_trip *trips;
>  	void *governor_data;
>  	struct list_head thermal_instances;
>  	struct ida ida;

Re: [PATCH v3 11/12] thermal/of: Use thermal trips stored in the thermal zone

[Zhang Rui]

On Sun, 2022-07-03 at 20:30 +0200, Daniel Lezcano wrote:
> Now that we have the thermal trip stored in the thermal zone in a
> generic way, we can rely on them and remove one indirection we found
> in the thermal_of code and do one more step forward the removal of
> the
> duplicated structures.
> 
> Cc: Alexandre Bailon <abailon@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc; Eduardo Valentin <eduval@amazon.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> ---
>  drivers/thermal/thermal_of.c | 53 +++++++++++-----------------------
> --
>  1 file changed, 16 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_of.c
> b/drivers/thermal/thermal_of.c
> index 16b6b90a2390..bc885729bf23 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -118,12 +118,7 @@ static int of_thermal_set_trips(struct
> thermal_zone_device *tz,
>   */
>  int of_thermal_get_ntrips(struct thermal_zone_device *tz)
>  {
> -	struct __thermal_zone *data = tz->devdata;
> -
> -	if (!data || IS_ERR(data))
> -		return -ENODEV;
> -
> -	return data->ntrips;
> +	return tz->ntrips;
>  }
>  EXPORT_SYMBOL_GPL(of_thermal_get_ntrips);
>  
> @@ -139,9 +134,7 @@ EXPORT_SYMBOL_GPL(of_thermal_get_ntrips);
>   */
>  bool of_thermal_is_trip_valid(struct thermal_zone_device *tz, int
> trip)
>  {
> -	struct __thermal_zone *data = tz->devdata;
> -
> -	if (!data || trip >= data->ntrips || trip < 0)
> +	if (trip >= tz->ntrips || trip < 0)
>  		return false;
>  
>  	return true;
> @@ -161,12 +154,7 @@ EXPORT_SYMBOL_GPL(of_thermal_is_trip_valid);
>  const struct thermal_trip *
>  of_thermal_get_trip_points(struct thermal_zone_device *tz)
>  {
> -	struct __thermal_zone *data = tz->devdata;
> -
> -	if (!data)
> -		return NULL;
> -
> -	return data->trips;
> +	return tz->trips;
>  }
>  EXPORT_SYMBOL_GPL(of_thermal_get_trip_points);

what is the difference between
of_thermal_get_ntrips/of_thermal_get_trip_points and
thermal_zone_get_ntrips/thermal_zone_get_trips as introduced in this
patch series?

we need to remove the duplications.

thanks,
rui
>  
> @@ -281,12 +269,10 @@ static int of_thermal_unbind(struct
> thermal_zone_device *thermal,
>  static int of_thermal_get_trip_type(struct thermal_zone_device *tz,
> int trip,
>  				    enum thermal_trip_type *type)
>  {
> -	struct __thermal_zone *data = tz->devdata;
> -
> -	if (trip >= data->ntrips || trip < 0)
> +	if (trip >= tz->ntrips || trip < 0)
>  		return -EDOM;
>  
> -	*type = data->trips[trip].type;
> +	*type = tz->trips[trip].type;
>  
>  	return 0;
>  }
> @@ -294,12 +280,10 @@ static int of_thermal_get_trip_type(struct
> thermal_zone_device *tz, int trip,
>  static int of_thermal_get_trip_temp(struct thermal_zone_device *tz,
> int trip,
>  				    int *temp)
>  {
> -	struct __thermal_zone *data = tz->devdata;
> -
> -	if (trip >= data->ntrips || trip < 0)
> +	if (trip >= tz->ntrips || trip < 0)
>  		return -EDOM;
>  
> -	*temp = data->trips[trip].temperature;
> +	*temp = tz->trips[trip].temperature;
>  
>  	return 0;
>  }
> @@ -309,7 +293,7 @@ static int of_thermal_set_trip_temp(struct
> thermal_zone_device *tz, int trip,
>  {
>  	struct __thermal_zone *data = tz->devdata;
>  
> -	if (trip >= data->ntrips || trip < 0)
> +	if (trip >= tz->ntrips || trip < 0)
>  		return -EDOM;
>  
>  	if (data->ops && data->ops->set_trip_temp) {
> @@ -321,7 +305,7 @@ static int of_thermal_set_trip_temp(struct
> thermal_zone_device *tz, int trip,
>  	}
>  
>  	/* thermal framework should take care of data->mask & (1 <<
> trip) */
> -	data->trips[trip].temperature = temp;
> +	tz->trips[trip].temperature = temp;
>  
>  	return 0;
>  }
> @@ -329,12 +313,10 @@ static int of_thermal_set_trip_temp(struct
> thermal_zone_device *tz, int trip,
>  static int of_thermal_get_trip_hyst(struct thermal_zone_device *tz,
> int trip,
>  				    int *hyst)
>  {
> -	struct __thermal_zone *data = tz->devdata;
> -
> -	if (trip >= data->ntrips || trip < 0)
> +	if (trip >= tz->ntrips || trip < 0)
>  		return -EDOM;
>  
> -	*hyst = data->trips[trip].hysteresis;
> +	*hyst = tz->trips[trip].hysteresis;
>  
>  	return 0;
>  }
> @@ -342,13 +324,11 @@ static int of_thermal_get_trip_hyst(struct
> thermal_zone_device *tz, int trip,
>  static int of_thermal_set_trip_hyst(struct thermal_zone_device *tz,
> int trip,
>  				    int hyst)
>  {
> -	struct __thermal_zone *data = tz->devdata;
> -
> -	if (trip >= data->ntrips || trip < 0)
> +	if (trip >= tz->ntrips || trip < 0)
>  		return -EDOM;
>  
>  	/* thermal framework should take care of data->mask & (1 <<
> trip) */
> -	data->trips[trip].hysteresis = hyst;
> +	tz->trips[trip].hysteresis = hyst;
>  
>  	return 0;
>  }
> @@ -356,12 +336,11 @@ static int of_thermal_set_trip_hyst(struct
> thermal_zone_device *tz, int trip,
>  static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
>  				    int *temp)
>  {
> -	struct __thermal_zone *data = tz->devdata;
>  	int i;
>  
> -	for (i = 0; i < data->ntrips; i++)
> -		if (data->trips[i].type == THERMAL_TRIP_CRITICAL) {
> -			*temp = data->trips[i].temperature;
> +	for (i = 0; i < tz->ntrips; i++)
> +		if (tz->trips[i].type == THERMAL_TRIP_CRITICAL) {
> +			*temp = tz->trips[i].temperature;
>  			return 0;
>  		}
>  

Re: [PATCH v3 07/12] thermal/core: Rename trips to ntrips

[Zhang Rui]

On Mon, 2022-07-04 at 09:24 +0100, Lukasz Luba wrote:
> 
> On 7/3/22 19:30, Daniel Lezcano wrote:
> > In order to use thermal trips defined in the thermal structure,
> > rename
> > the 'trips' field to 'ntrips' to have the 'trips' field containing
> > the
> > thermal trip points.
> > 
> > Cc: Alexandre Bailon <abailon@baylibre.com>
> > Cc: Kevin Hilman <khilman@baylibre.com>
> > Cc; Eduardo Valentin <eduval@amazon.com>
> > Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> > ---
> >   drivers/thermal/gov_fair_share.c        |  6 +++---
> >   drivers/thermal/gov_power_allocator.c   |  4 ++--
> >   drivers/thermal/tegra/tegra30-tsensor.c |  2 +-
> >   drivers/thermal/thermal_core.c          | 20 ++++++++++----------
> >   drivers/thermal/thermal_helpers.c       |  4 ++--
> >   drivers/thermal/thermal_netlink.c       |  2 +-
> >   drivers/thermal/thermal_sysfs.c         | 22 +++++++++++---------
> > --
> >   include/linux/thermal.h                 |  2 +-
> >   8 files changed, 31 insertions(+), 31 deletions(-)
> 
> 
> [snip]
> 
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > index 6289b0bb1c97..3a57878a2a6c 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> 
> Missing updated ne name in comment here:
>   * @trips:      number of trip points the thermal zone supports
> 
> 
> > @@ -165,7 +165,7 @@ struct thermal_zone_device {
> >   	struct thermal_attr *trip_hyst_attrs;
> >   	enum thermal_device_mode mode;
> >   	void *devdata;
> > -	int trips;
> > +	int ntrips;
> >   	unsigned long trips_disabled;	/* bitmap for disabled
> > trips */
> >   	unsigned long passive_delay_jiffies;
> >   	unsigned long polling_delay_jiffies;
> 
> Maybe this is only my bias, but this new name 'ntrips' looks
> like negation in electronics.
> 
> We have examples like: num_cpus, num_pins, num_leds, num_groups,
> num_locks, num_buffers, num_phys, etc...
> 
> Could we have 'num_trips' and follow to this convention here as well?
> 
I'd vote for "num_trips". :)

-rui

Re: [PATCH v3 05/12] thermal/core: Remove unneeded EXPORT_SYMBOLS

[Daniel Lezcano]

On 04/07/2022 09:35, Lukasz Luba wrote:
> Hi Daniel,
> 
> (+Todd and Wei on CC)
> 
> 
> On 7/3/22 19:30, Daniel Lezcano wrote:

[ ... ]

>>   }
>> -EXPORT_SYMBOL(get_tz_trend);

[ ... ]

>>   }
>> -EXPORT_SYMBOL(thermal_cdev_update);
> 
> I wouldn't remove that export. I can see in my Pixel6 modules dir, that
> it's called in 7 places.
> 
> I assume that in Android world this is common use.

It is not possible to do changes taking into consideration out of tree 
code. Moreover there is logically no good reason to use the 
thermal_cdev_update() function from outside of the thermal core code.

Re: [PATCH v3 02/12] thermal/of: Replace device node match with device node search

[Daniel Lezcano]

On 04/07/2022 09:59, Lukasz Luba wrote:

[ ... ]

>> -    for_each_child_of_node(child, gchild) {
>> +    for_each_child_of_node(trips, gchild) {
>>           ret = thermal_of_populate_trip(gchild, &tz->trips[i++]);
>>           if (ret)
>>               goto free_trips;
>>       }
>> -    of_node_put(child);
>> -
> 
> It's probably needed to put the 'trips' here, isn't it?
> Or at the end in 'free_trips'.

Right, it is fixed later in another patch but I'll fix it here too.

Thanks for spotting this

Re: [PATCH v3 07/12] thermal/core: Rename trips to ntrips

[Daniel Lezcano]

On 04/07/2022 10:24, Lukasz Luba wrote:
> 
> 
> On 7/3/22 19:30, Daniel Lezcano wrote:
>> In order to use thermal trips defined in the thermal structure, rename
>> the 'trips' field to 'ntrips' to have the 'trips' field containing the
>> thermal trip points.
>>
>> Cc: Alexandre Bailon <abailon@baylibre.com>
>> Cc: Kevin Hilman <khilman@baylibre.com>
>> Cc; Eduardo Valentin <eduval@amazon.com>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
>> ---
>>   drivers/thermal/gov_fair_share.c        |  6 +++---
>>   drivers/thermal/gov_power_allocator.c   |  4 ++--
>>   drivers/thermal/tegra/tegra30-tsensor.c |  2 +-
>>   drivers/thermal/thermal_core.c          | 20 ++++++++++----------
>>   drivers/thermal/thermal_helpers.c       |  4 ++--
>>   drivers/thermal/thermal_netlink.c       |  2 +-
>>   drivers/thermal/thermal_sysfs.c         | 22 +++++++++++-----------
>>   include/linux/thermal.h                 |  2 +-
>>   8 files changed, 31 insertions(+), 31 deletions(-)
> 
> 
> [snip]
> 
>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> index 6289b0bb1c97..3a57878a2a6c 100644
>> --- a/include/linux/thermal.h
>> +++ b/include/linux/thermal.h
> 
> Missing updated ne name in comment here:
>   * @trips:      number of trip points the thermal zone supports
> 
> 
>> @@ -165,7 +165,7 @@ struct thermal_zone_device {
>>       struct thermal_attr *trip_hyst_attrs;
>>       enum thermal_device_mode mode;
>>       void *devdata;
>> -    int trips;
>> +    int ntrips;
>>       unsigned long trips_disabled;    /* bitmap for disabled trips */
>>       unsigned long passive_delay_jiffies;
>>       unsigned long polling_delay_jiffies;
> 
> Maybe this is only my bias, but this new name 'ntrips' looks
> like negation in electronics.
> 
> We have examples like: num_cpus, num_pins, num_leds, num_groups,
> num_locks, num_buffers, num_phys, etc...
> 
> Could we have 'num_trips' and follow to this convention here as well?

Sure, I'll do the changes accordingly

Re: [PATCH v3 08/12] thermal/core: Add thermal_trip in thermal_zone

[Daniel Lezcano]

Hi Zhang,

thanks for reviewing the series

On 04/07/2022 16:11, Zhang Rui wrote:
> On Sun, 2022-07-03 at 20:30 +0200, Daniel Lezcano wrote:
>> The thermal trip points are properties of a thermal zone and the
>> different sub systems should be able to save them in the thermal zone
>> structure instead of having their own definition.
>>
>> Give the opportunity to the drivers to create a thermal zone with
>> thermal trips which will be accessible directly from the thermal core
>> framework.
>>
>> Cc: Alexandre Bailon <abailon@baylibre.com>
>> Cc: Kevin Hilman <khilman@baylibre.com>
>> Cc; Eduardo Valentin <eduval@amazon.com>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
>> ---
>>   drivers/thermal/thermal_core.h | 10 ++++++++++
>>   include/linux/thermal.h        |  1 +
>>   2 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/thermal/thermal_core.h
>> b/drivers/thermal/thermal_core.h
>> index c991bb290512..84e341c1e0fc 100644
>> --- a/drivers/thermal/thermal_core.h
>> +++ b/drivers/thermal/thermal_core.h
>> @@ -113,6 +113,16 @@ int thermal_build_list_of_policies(char *buf);
>>   /* Helpers */
>>   void thermal_zone_set_trips(struct thermal_zone_device *tz);
>>   
>> +static inline struct thermal_trip *thermal_zone_get_trips(struct
>> thermal_zone *tz)
> 
> it should be struct thermal_zone_device?
> It seems that you fixed it in patch 9/12, and leave it broke here.

Right, I'll fix it

>> +{
>> +	return tz->trips;
>> +}
>> +
>> +static inline int thermal_zone_get_ntrips(struct thermal_zone *tz)
> 
> same problem as above.
> 
> thanks,
> rui
>> +{
>> +	return tz->ntrips;
>> +}
>> +
>>   /* sysfs I/F */
>>   int thermal_zone_create_device_groups(struct thermal_zone_device *,
>> int);
>>   void thermal_zone_destroy_device_groups(struct thermal_zone_device
>> *);
>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> index 3a57878a2a6c..3733e23b6359 100644
>> --- a/include/linux/thermal.h
>> +++ b/include/linux/thermal.h
>> @@ -179,6 +179,7 @@ struct thermal_zone_device {
>>   	struct thermal_zone_device_ops *ops;
>>   	struct thermal_zone_params *tzp;
>>   	struct thermal_governor *governor;
>> +	struct thermal_trip *trips;
>>   	void *governor_data;
>>   	struct list_head thermal_instances;
>>   	struct ida ida;
> 

Re: [PATCH v3 11/12] thermal/of: Use thermal trips stored in the thermal zone

[Daniel Lezcano]

On 04/07/2022 16:14, Zhang Rui wrote:
> On Sun, 2022-07-03 at 20:30 +0200, Daniel Lezcano wrote:
>> Now that we have the thermal trip stored in the thermal zone in a
>> generic way, we can rely on them and remove one indirection we found
>> in the thermal_of code and do one more step forward the removal of
>> the
>> duplicated structures.
>>
>> Cc: Alexandre Bailon <abailon@baylibre.com>
>> Cc: Kevin Hilman <khilman@baylibre.com>
>> Cc; Eduardo Valentin <eduval@amazon.com>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
>> ---

[ ... ]

>>   EXPORT_SYMBOL_GPL(of_thermal_get_ntrips);
>>   
>> @@ -139,9 +134,7 @@ EXPORT_SYMBOL_GPL(of_thermal_get_ntrips);
>>    */
>>   bool of_thermal_is_trip_valid(struct thermal_zone_device *tz, int
>> trip)
>>   {
>> -	struct __thermal_zone *data = tz->devdata;
>> -
>> -	if (!data || trip >= data->ntrips || trip < 0)
>> +	if (trip >= tz->ntrips || trip < 0)
>>   		return false;
>>   
>>   	return true;
>> @@ -161,12 +154,7 @@ EXPORT_SYMBOL_GPL(of_thermal_is_trip_valid);
>>   const struct thermal_trip *
>>   of_thermal_get_trip_points(struct thermal_zone_device *tz)
>>   {
>> -	struct __thermal_zone *data = tz->devdata;
>> -
>> -	if (!data)
>> -		return NULL;
>> -
>> -	return data->trips;
>> +	return tz->trips;
>>   }
>>   EXPORT_SYMBOL_GPL(of_thermal_get_trip_points);
> 
> what is the difference between
> of_thermal_get_ntrips/of_thermal_get_trip_points and
> thermal_zone_get_ntrips/thermal_zone_get_trips as introduced in this
> patch series?
> 
> we need to remove the duplications.

There is no difference between those functions. There are 34 more 
patches in the pipe to be sent after this series to do more cleanups and 
remove code duplication.

Re: [PATCH v3 11/12] thermal/of: Use thermal trips stored in the thermal zone

[Zhang Rui]

On Mon, 2022-07-04 at 23:24 +0200, Daniel Lezcano wrote:
> On 04/07/2022 16:14, Zhang Rui wrote:
> > On Sun, 2022-07-03 at 20:30 +0200, Daniel Lezcano wrote:
> > > Now that we have the thermal trip stored in the thermal zone in a
> > > generic way, we can rely on them and remove one indirection we
> > > found
> > > in the thermal_of code and do one more step forward the removal
> > > of
> > > the
> > > duplicated structures.
> > > 
> > > Cc: Alexandre Bailon <abailon@baylibre.com>
> > > Cc: Kevin Hilman <khilman@baylibre.com>
> > > Cc; Eduardo Valentin <eduval@amazon.com>
> > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> > > ---
> 
> [ ... ]
> 
> > >   EXPORT_SYMBOL_GPL(of_thermal_get_ntrips);
> > >   
> > > @@ -139,9 +134,7 @@ EXPORT_SYMBOL_GPL(of_thermal_get_ntrips);
> > >    */
> > >   bool of_thermal_is_trip_valid(struct thermal_zone_device *tz,
> > > int
> > > trip)
> > >   {
> > > -	struct __thermal_zone *data = tz->devdata;
> > > -
> > > -	if (!data || trip >= data->ntrips || trip < 0)
> > > +	if (trip >= tz->ntrips || trip < 0)
> > >   		return false;
> > >   
> > >   	return true;
> > > @@ -161,12 +154,7 @@ EXPORT_SYMBOL_GPL(of_thermal_is_trip_valid);
> > >   const struct thermal_trip *
> > >   of_thermal_get_trip_points(struct thermal_zone_device *tz)
> > >   {
> > > -	struct __thermal_zone *data = tz->devdata;
> > > -
> > > -	if (!data)
> > > -		return NULL;
> > > -
> > > -	return data->trips;
> > > +	return tz->trips;
> > >   }
> > >   EXPORT_SYMBOL_GPL(of_thermal_get_trip_points);
> > 
> > what is the difference between
> > of_thermal_get_ntrips/of_thermal_get_trip_points and
> > thermal_zone_get_ntrips/thermal_zone_get_trips as introduced in
> > this
> > patch series?
> > 
> > we need to remove the duplications.
> 
> There is no difference between those functions. There are 34 more 
> patches in the pipe to be sent after this series to do more cleanups
> and 
> remove code duplication.
> 
Good to know.

It would be nice to have a cover letter to describe the whole picture,
including this patch series and the following patches in your queue.

thanks,
rui

Re: [PATCH v3 09/12] thermal/core: Register with the trip points

[Zhang Rui]

On Sun, 2022-07-03 at 20:30 +0200, Daniel Lezcano wrote:
> As we added the thermal trip points structure in the thermal zone,
> let's extend the thermal zone register function to have the thermal
> trip structures as a parameter and store it in the 'trips' field of
> the thermal zone structure.

Just FYI.

I proposed a small topic for this year' LPC about the
thermal_zone_device_register() parameters.
We have more and more parameters introduced, IMO, it's better to use a
unified structure for registration phase configurations, or reuse
struct thermal_zone_params.
In this way, when a new parameter is needed, we only need to introduce
a new field in the structure, rather than update every caller of this
API, or introduce new wrapper functions like we did in this patch.

thanks,
rui
> 
> Cc: Alexandre Bailon <abailon@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc; Eduardo Valentin <eduval@amazon.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org>
> ---
>  drivers/thermal/thermal_core.c | 22 +++++++++++++++++-----
>  drivers/thermal/thermal_core.h |  4 ++--
>  include/linux/thermal.h        |  6 ++++++
>  3 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_core.c
> b/drivers/thermal/thermal_core.c
> index 434a675da245..e865c41d2320 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1167,8 +1167,9 @@ static void thermal_set_delay_jiffies(unsigned
> long *delay_jiffies, int delay_ms
>  }
>  
>  /**
> - * thermal_zone_device_register() - register a new thermal zone
> device
> + * thermal_zone_device_register_with_trips() - register a new
> thermal zone device
>   * @type:	the thermal zone device type
> + * @trips:	a pointer to an array of thermal trips
>   * @ntrips:	the number of trip points the thermal zone support
>   * @mask:	a bit string indicating the writeablility of trip
> points
>   * @devdata:	private device data
> @@ -1191,10 +1192,10 @@ static void
> thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms
>   * IS_ERR*() helpers.
>   */
>  struct thermal_zone_device *
> -thermal_zone_device_register(const char *type, int ntrips, int mask,
> -			     void *devdata, struct
> thermal_zone_device_ops *ops,
> -			     struct thermal_zone_params *tzp, int
> passive_delay,
> -			     int polling_delay)
> +thermal_zone_device_register_with_trips(const char *type, struct
> thermal_trip *trips, int ntrips, int mask,
> +					void *devdata, struct
> thermal_zone_device_ops *ops,
> +					struct thermal_zone_params
> *tzp, int passive_delay,
> +					int polling_delay)
>  {
>  	struct thermal_zone_device *tz;
>  	enum thermal_trip_type trip_type;
> @@ -1256,6 +1257,7 @@ thermal_zone_device_register(const char *type,
> int ntrips, int mask,
>  	tz->device.class = &thermal_class;
>  	tz->devdata = devdata;
>  	tz->ntrips = ntrips;
> +	tz->trips = trips;
>  
>  	thermal_set_delay_jiffies(&tz->passive_delay_jiffies,
> passive_delay);
>  	thermal_set_delay_jiffies(&tz->polling_delay_jiffies,
> polling_delay);
> @@ -1331,6 +1333,16 @@ thermal_zone_device_register(const char *type,
> int ntrips, int mask,
>  	kfree(tz);
>  	return ERR_PTR(result);
>  }
> +
> +struct thermal_zone_device *thermal_zone_device_register(const char
> *type, int ntrips, int mask,
> +							 void *devdata,
> struct thermal_zone_device_ops *ops,
> +							 struct
> thermal_zone_params *tzp, int passive_delay,
> +							 int
> polling_delay)
> +{
> +	return thermal_zone_device_register_with_trips(type, NULL,
> ntrips, mask,
> +						       devdata, ops,
> tzp,
> +						       passive_delay,
> polling_delay);
> +}
>  EXPORT_SYMBOL_GPL(thermal_zone_device_register);
>  
>  /**
> diff --git a/drivers/thermal/thermal_core.h
> b/drivers/thermal/thermal_core.h
> index 84e341c1e0fc..bbe3ec26d12e 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -113,12 +113,12 @@ int thermal_build_list_of_policies(char *buf);
>  /* Helpers */
>  void thermal_zone_set_trips(struct thermal_zone_device *tz);
>  
> -static inline struct thermal_trip *thermal_zone_get_trips(struct
> thermal_zone *tz)
> +static inline struct thermal_trip *thermal_zone_get_trips(struct
> thermal_zone_device *tz)
>  {
>  	return tz->trips;
>  }
>  
> -static inline int thermal_zone_get_ntrips(struct thermal_zone *tz)
> +static inline int thermal_zone_get_ntrips(struct thermal_zone_device
> *tz)
>  {
>  	return tz->ntrips;
>  }
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 3733e23b6359..8cbe237a92d0 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -379,8 +379,14 @@ void
> devm_thermal_zone_of_sensor_unregister(struct device *dev,
>  struct thermal_zone_device *thermal_zone_device_register(const char
> *, int, int,
>  		void *, struct thermal_zone_device_ops *,
>  		struct thermal_zone_params *, int, int);
> +
>  void thermal_zone_device_unregister(struct thermal_zone_device *);
>  
> +struct thermal_zone_device *
> +thermal_zone_device_register_with_trips(const char *, struct
> thermal_trip *, int, int,
> +					void *, struct
> thermal_zone_device_ops *,
> +					struct thermal_zone_params *,
> int, int);
> +
>  int thermal_zone_bind_cooling_device(struct thermal_zone_device *,
> int,
>  				     struct thermal_cooling_device *,
>  				     unsigned long, unsigned long,

Re: [PATCH v3 11/12] thermal/of: Use thermal trips stored in the thermal zone

[Daniel Lezcano]

On 05/07/2022 03:20, Zhang Rui wrote:

[ ... ]

>> There is no difference between those functions. There are 34 more
>> patches in the pipe to be sent after this series to do more cleanups
>> and
>> remove code duplication.
>>
> Good to know.
> 
> It would be nice to have a cover letter to describe the whole picture,
> including this patch series and the following patches in your queue.

https://lore.kernel.org/lkml/20220703183059.4133659-4-daniel.lezcano@linexp.org/T/

You will Cc'ed next time ;)


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

Re: [PATCH v3 05/12] thermal/core: Remove unneeded EXPORT_SYMBOLS

[Lukasz Luba]

On 7/4/22 22:14, Daniel Lezcano wrote:
> On 04/07/2022 09:35, Lukasz Luba wrote:
>> Hi Daniel,
>>
>> (+Todd and Wei on CC)
>>
>>
>> On 7/3/22 19:30, Daniel Lezcano wrote:
> 
> [ ... ]
> 
>>>   }
>>> -EXPORT_SYMBOL(get_tz_trend);
> 
> [ ... ]
> 
>>>   }
>>> -EXPORT_SYMBOL(thermal_cdev_update);
>>
>> I wouldn't remove that export. I can see in my Pixel6 modules dir, that
>> it's called in 7 places.
>>
>> I assume that in Android world this is common use.
> 
> It is not possible to do changes taking into consideration out of tree 
> code. Moreover there is logically no good reason to use the 
> thermal_cdev_update() function from outside of the thermal core code.
> 

I see your point which is 'upstream'. On the other hand the mostly
deployed kernel is in Android devices and that brings a lot to the
community.

This symbol might also be used by other distros which might have
modules for some accelerators, which also support tricky cooling.

I would keep it as is...

Re: [PATCH v3 11/12] thermal/of: Use thermal trips stored in the thermal zone

[Zhang Rui]

On Tue, 2022-07-05 at 08:44 +0200, Daniel Lezcano wrote:
> On 05/07/2022 03:20, Zhang Rui wrote:
> 
> [ ... ]
> 
> > > There is no difference between those functions. There are 34 more
> > > patches in the pipe to be sent after this series to do more
> > > cleanups
> > > and
> > > remove code duplication.
> > > 
> > 
> > Good to know.
> > 
> > It would be nice to have a cover letter to describe the whole
> > picture,
> > including this patch series and the following patches in your
> > queue.
> 
> 
https://lore.kernel.org/lkml/20220703183059.4133659-4-daniel.lezcano@linexp.org/T/
> 
> You will Cc'ed next time ;)
> 
Cool, thanks!

-rui

Re: [PATCH v3 09/12] thermal/core: Register with the trip points

[Rafael J. Wysocki]

On Tue, Jul 5, 2022 at 4:03 AM Zhang Rui <rui.zhang@intel.com> wrote:
>
> On Sun, 2022-07-03 at 20:30 +0200, Daniel Lezcano wrote:
> > As we added the thermal trip points structure in the thermal zone,
> > let's extend the thermal zone register function to have the thermal
> > trip structures as a parameter and store it in the 'trips' field of
> > the thermal zone structure.
>
> Just FYI.
>
> I proposed a small topic for this year' LPC about the
> thermal_zone_device_register() parameters.
> We have more and more parameters introduced, IMO, it's better to use a
> unified structure for registration phase configurations, or reuse
> struct thermal_zone_params.
> In this way, when a new parameter is needed, we only need to introduce
> a new field in the structure, rather than update every caller of this
> API, or introduce new wrapper functions like we did in this patch.

Sounds reasonable to me.

Re: [PATCH v3 05/12] thermal/core: Remove unneeded EXPORT_SYMBOLS

[Rafael J. Wysocki]

On Tue, Jul 5, 2022 at 9:30 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>
>
>
> On 7/4/22 22:14, Daniel Lezcano wrote:
> > On 04/07/2022 09:35, Lukasz Luba wrote:
> >> Hi Daniel,
> >>
> >> (+Todd and Wei on CC)
> >>
> >>
> >> On 7/3/22 19:30, Daniel Lezcano wrote:
> >
> > [ ... ]
> >
> >>>   }
> >>> -EXPORT_SYMBOL(get_tz_trend);
> >
> > [ ... ]
> >
> >>>   }
> >>> -EXPORT_SYMBOL(thermal_cdev_update);
> >>
> >> I wouldn't remove that export. I can see in my Pixel6 modules dir, that
> >> it's called in 7 places.
> >>
> >> I assume that in Android world this is common use.
> >
> > It is not possible to do changes taking into consideration out of tree
> > code. Moreover there is logically no good reason to use the
> > thermal_cdev_update() function from outside of the thermal core code.
> >
>
> I see your point which is 'upstream'. On the other hand the mostly
> deployed kernel is in Android devices and that brings a lot to the
> community.
>
> This symbol might also be used by other distros which might have
> modules for some accelerators, which also support tricky cooling.
>
> I would keep it as is...

I think that the long-term goal is to reduce differences between the
mainline kernel and Android.  From this angle, it would be good if
Android was aware that the mainline did stuff especially for them and
making them carry an extra patch would go a long way towards that
purpose.

Re: [PATCH v3 05/12] thermal/core: Remove unneeded EXPORT_SYMBOLS

[Lukasz Luba]

On 7/5/22 15:20, Rafael J. Wysocki wrote:
> On Tue, Jul 5, 2022 at 9:30 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>>
>>
>>
>> On 7/4/22 22:14, Daniel Lezcano wrote:
>>> On 04/07/2022 09:35, Lukasz Luba wrote:
>>>> Hi Daniel,
>>>>
>>>> (+Todd and Wei on CC)
>>>>
>>>>
>>>> On 7/3/22 19:30, Daniel Lezcano wrote:
>>>
>>> [ ... ]
>>>
>>>>>    }
>>>>> -EXPORT_SYMBOL(get_tz_trend);
>>>
>>> [ ... ]
>>>
>>>>>    }
>>>>> -EXPORT_SYMBOL(thermal_cdev_update);
>>>>
>>>> I wouldn't remove that export. I can see in my Pixel6 modules dir, that
>>>> it's called in 7 places.
>>>>
>>>> I assume that in Android world this is common use.
>>>
>>> It is not possible to do changes taking into consideration out of tree
>>> code. Moreover there is logically no good reason to use the
>>> thermal_cdev_update() function from outside of the thermal core code.
>>>
>>
>> I see your point which is 'upstream'. On the other hand the mostly
>> deployed kernel is in Android devices and that brings a lot to the
>> community.
>>
>> This symbol might also be used by other distros which might have
>> modules for some accelerators, which also support tricky cooling.
>>
>> I would keep it as is...
> 
> I think that the long-term goal is to reduce differences between the
> mainline kernel and Android.  From this angle, it would be good if
> Android was aware that the mainline did stuff especially for them and
> making them carry an extra patch would go a long way towards that
> purpose.

It's hard to judge sometimes especially on those small bits.
I've just pointed out and shared the info that this symbol is used.
What you will do with this it's up to you. You and Daniel are the
maintainers of this subsystems and have long-term plans for it.
Todd and Wei are on CC, so they will know about this change.
My job finishes here.

Re: [PATCH v3 05/12] thermal/core: Remove unneeded EXPORT_SYMBOLS

[Todd Kjos]

On Mon, Jul 4, 2022 at 2:14 PM Daniel Lezcano <daniel.lezcano@linexp.org> wrote:
>
> On 04/07/2022 09:35, Lukasz Luba wrote:
> > Hi Daniel,
> >
> > (+Todd and Wei on CC)
> >
> >
> > On 7/3/22 19:30, Daniel Lezcano wrote:
>
> [ ... ]
>
> >>   }
> >> -EXPORT_SYMBOL(get_tz_trend);
>
> [ ... ]
>
> >>   }
> >> -EXPORT_SYMBOL(thermal_cdev_update);
> >
> > I wouldn't remove that export. I can see in my Pixel6 modules dir, that
> > it's called in 7 places.
> >
> > I assume that in Android world this is common use.
>
> It is not possible to do changes taking into consideration out of tree
> code. Moreover there is logically no good reason to use the
> thermal_cdev_update() function from outside of the thermal core code.
>

I agree. It is totally appropriate for the export to be removed for
these functions if the exports are only for out of tree code. If they
are needed for Android, they can be carried in the Android kernel
trees.