[PATCH v1 00/18] Thermal: Part 1 - Introduce new structs and registration

[PATCH v1 00/18] Thermal: Part 1 - Introduce new structs and registration

[AngeloGioacchino Del Regno]

This series is a preparation for a bigger cleanup that will be split in
three parts in order to avoid immutable branches on multiple subsystems,
as in other parts of this series there will be changes in:
- drivers/acpi
- drivers/platform/x86
- drivers/power/supply
- drivers/net/wireless
- drivers/net/ethernet

This is the first part which adds new structures and starts reorganizing
struct members around, plus, this migrates all of the thermal drivers
found in drivers/thermal/ to the new thermal_zone_device_register()
function, and advertises that the old registration functions are
deprecated and will be removed.

The reorganization is supposed to be complete (or mostly) but...
 - struct thermal_zone_platform_params has a temporary name:
   this is done in order to avoid compile time failures for
   the drivers outside of drivers/thermal before migrating
   them to thermal_zone_device_params/thermal_zone_device_register();
 - struct thermal_zone_params temporarily has two duplicated members,
   governor_name and no_hwmon;

Part 2 of this topic will migrate all drivers that are external to
drivers/thermal to thermal_zone_device_register(); I will send that
part only after part 1 is confirmed to be acceptable, as otherwise
I'd be spamming people for no reason :-)

After all drivers will be migrated to thermal_zone_device_register(),
we won't have to care about changing anything outside of drivers/thermal
to finish this set of cleanups/changes (and no immutable branches needed)
and this means that...
Part 3 of this topic will contain the following changes:
 - thermal_zone_device_register_with_trips() will be removed
 - thermal_tripless_zone_device_register() will be removed
 - thermal_zone_params will be renamed to thermal_governor_ipa_params
   - governor_name, no_hwmon members will be removed
 - thermal_zone_platform_params will be renamed to thermal_zone_params
 - Removal of the THERMAL_NAME_LENGTH limitation for `type`

More scheduled changes, which should end up in part 3 (at least that's
my intention), or eventually entirely after this cleanup topic, include:
 - Introduction of Thermal Zone names
 - Disambiguation of TZ name and type
 - Addition of `thermal-zones`, `thermal-zone-names` parsing for
   devicetrees

... Summarizing ...

Part 1:
 - Reorganize structures (some temporary names/leftovers)
 - New registration function, deprecation of old ones
 - Migration of drivers/thermal drivers to new registration

Part 2:
 - Migration of drivers in other subsystems to new thermal registration

Part 3:
 - Remove the two leftovers in thermal.h
 - Rename structures with proper, final names
 - Everything else, anyway.


Cheers,
Angelo

AngeloGioacchino Del Regno (18):
  thermal: core: Change governor name to const char pointer
  thermal: Add new structures and thermal_zone_device_register()
  thermal: Directly use thermal_zone_platform_params for
    thermal_zone_device
  thermal/drivers/da9062: Migrate to thermal_zone_device_register()
  thermal/drivers/imx: Migrate to thermal_zone_device_register()
  thermal/drivers/rcar: Migrate to thermal_zone_device_register()
  thermal/drivers/st: Migrate to thermal_zone_device_register()
  thermal: intel: pch_thermal: Migrate to thermal_zone_device_register()
  thermal: intel: quark_dts: Migrate to thermal_zone_device_register()
  thermal: intel: soc_dts_iosf: Migrate to
    thermal_zone_device_register()
  thermal: intel: int340x: Migrate to thermal_zone_device_register()
  thermal: int340x: processor: Migrate to thermal_zone_device_register()
  thermal: intel: x86_pkg_temp: Migrate to
    thermal_zone_device_register()
  thermal/drivers/armada: Migrate to thermal_zone_device_register()
  thermal/drivers/dove: Migrate to thermal_zone_device_register()
  thermal/drivers/kirkwood: Migrate to thermal_zone_device_register()
  thermal/drivers/spear: Migrate to thermal_zone_device_register()
  thermal/drivers/int340x: Migrate to thermal_zone_device_register()

 drivers/thermal/armada_thermal.c              |  12 +-
 drivers/thermal/da9062-thermal.c              |  16 +-
 drivers/thermal/dove_thermal.c                |  10 +-
 drivers/thermal/gov_power_allocator.c         |  38 ++--
 drivers/thermal/gov_user_space.c              |   2 +-
 drivers/thermal/imx_thermal.c                 |  21 +-
 .../intel/int340x_thermal/int3400_thermal.c   |  20 +-
 .../int340x_thermal/int340x_thermal_zone.c    |  28 +--
 .../processor_thermal_device_pci.c            |  26 ++-
 drivers/thermal/intel/intel_pch_thermal.c     |  12 +-
 .../thermal/intel/intel_quark_dts_thermal.c   |  23 +-
 drivers/thermal/intel/intel_soc_dts_iosf.c    |  24 ++-
 drivers/thermal/intel/x86_pkg_temp_thermal.c  |  22 +-
 drivers/thermal/kirkwood_thermal.c            |  10 +-
 drivers/thermal/qcom/tsens.c                  |   4 +-
 drivers/thermal/rcar_thermal.c                |  15 +-
 drivers/thermal/spear_thermal.c               |  10 +-
 drivers/thermal/st/st_thermal.c               |  15 +-
 drivers/thermal/thermal_core.c                | 201 +++++++++++-------
 drivers/thermal/thermal_core.h                |   6 +-
 drivers/thermal/thermal_helpers.c             |  22 +-
 drivers/thermal/thermal_hwmon.c               |   8 +-
 drivers/thermal/thermal_of.c                  |  12 +-
 drivers/thermal/thermal_sysfs.c               |  64 +++---
 drivers/thermal/thermal_trace.h               |   8 +-
 drivers/thermal/thermal_trip.c                |  14 +-
 include/linux/thermal.h                       |  87 ++++++--
 27 files changed, 461 insertions(+), 269 deletions(-)

-- 
2.43.0

[PATCH v1 01/18] thermal: core: Change governor name to const char pointer

[AngeloGioacchino Del Regno]

All users are already assigning a const char * to the `governor_name`
member of struct thermal_zone_params and to the `name` member of
struct thermal_governor.
Even if users are technically wrong, it just makes more sense to change
this member to be a const char pointer instead of doing the other way
around.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 include/linux/thermal.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index b7a3deb372fd..65d8f92a9a0d 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -214,7 +214,7 @@ struct thermal_zone_device {
  * @governor_list:	node in thermal_governor_list (in thermal_core.c)
  */
 struct thermal_governor {
-	char name[THERMAL_NAME_LENGTH];
+	const char *name;
 	int (*bind_to_tz)(struct thermal_zone_device *tz);
 	void (*unbind_from_tz)(struct thermal_zone_device *tz);
 	int (*throttle)(struct thermal_zone_device *tz,
@@ -226,7 +226,7 @@ struct thermal_governor {
 
 /* Structure to define Thermal Zone parameters */
 struct thermal_zone_params {
-	char governor_name[THERMAL_NAME_LENGTH];
+	const char *governor_name;
 
 	/*
 	 * a boolean to indicate if the thermal to hwmon sysfs interface
-- 
2.43.0

Re: [PATCH v1 01/18] thermal: core: Change governor name to const char pointer

[Rafael J. Wysocki]

On Tue, Jan 30, 2024 at 12:13 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> All users are already assigning a const char * to the `governor_name`
> member of struct thermal_zone_params and to the `name` member of
> struct thermal_governor.
> Even if users are technically wrong, it just makes more sense to change
> this member to be a const char pointer instead of doing the other way
> around.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Acked-by: Rafael J. Wysocki <rafael@kernel.org>

or I can pick it up right away if you want me to do that.

> ---
>  include/linux/thermal.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index b7a3deb372fd..65d8f92a9a0d 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -214,7 +214,7 @@ struct thermal_zone_device {
>   * @governor_list:     node in thermal_governor_list (in thermal_core.c)
>   */
>  struct thermal_governor {
> -       char name[THERMAL_NAME_LENGTH];
> +       const char *name;
>         int (*bind_to_tz)(struct thermal_zone_device *tz);
>         void (*unbind_from_tz)(struct thermal_zone_device *tz);
>         int (*throttle)(struct thermal_zone_device *tz,
> @@ -226,7 +226,7 @@ struct thermal_governor {
>
>  /* Structure to define Thermal Zone parameters */
>  struct thermal_zone_params {
> -       char governor_name[THERMAL_NAME_LENGTH];
> +       const char *governor_name;
>
>         /*
>          * a boolean to indicate if the thermal to hwmon sysfs interface
> --

Re: [PATCH v1 01/18] thermal: core: Change governor name to const char pointer

[AngeloGioacchino Del Regno]

Il 01/02/24 19:37, Rafael J. Wysocki ha scritto:
> On Tue, Jan 30, 2024 at 12:13 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> All users are already assigning a const char * to the `governor_name`
>> member of struct thermal_zone_params and to the `name` member of
>> struct thermal_governor.
>> Even if users are technically wrong, it just makes more sense to change
>> this member to be a const char pointer instead of doing the other way
>> around.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> Acked-by: Rafael J. Wysocki <rafael@kernel.org>
> 
> or I can pick it up right away if you want me to do that.
> 

I appreciate having less patches to carry over with new series versions.

Whatever you can take, please feel free to pick directly :-)

Thanks,
Angelo

>> ---
>>   include/linux/thermal.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> index b7a3deb372fd..65d8f92a9a0d 100644
>> --- a/include/linux/thermal.h
>> +++ b/include/linux/thermal.h
>> @@ -214,7 +214,7 @@ struct thermal_zone_device {
>>    * @governor_list:     node in thermal_governor_list (in thermal_core.c)
>>    */
>>   struct thermal_governor {
>> -       char name[THERMAL_NAME_LENGTH];
>> +       const char *name;
>>          int (*bind_to_tz)(struct thermal_zone_device *tz);
>>          void (*unbind_from_tz)(struct thermal_zone_device *tz);
>>          int (*throttle)(struct thermal_zone_device *tz,
>> @@ -226,7 +226,7 @@ struct thermal_governor {
>>
>>   /* Structure to define Thermal Zone parameters */
>>   struct thermal_zone_params {
>> -       char governor_name[THERMAL_NAME_LENGTH];
>> +       const char *governor_name;
>>
>>          /*
>>           * a boolean to indicate if the thermal to hwmon sysfs interface
>> --

Re: [PATCH v1 01/18] thermal: core: Change governor name to const char pointer

[Rafael J. Wysocki]

On Fri, Feb 2, 2024 at 11:01 AM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 01/02/24 19:37, Rafael J. Wysocki ha scritto:
> > On Tue, Jan 30, 2024 at 12:13 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> >>
> >> All users are already assigning a const char * to the `governor_name`
> >> member of struct thermal_zone_params and to the `name` member of
> >> struct thermal_governor.
> >> Even if users are technically wrong, it just makes more sense to change
> >> this member to be a const char pointer instead of doing the other way
> >> around.
> >>
> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >
> > Acked-by: Rafael J. Wysocki <rafael@kernel.org>
> >
> > or I can pick it up right away if you want me to do that.
> >
>
> I appreciate having less patches to carry over with new series versions.
>
> Whatever you can take, please feel free to pick directly :-)

OK, applied (as 6.9 material).

Thanks!

[PATCH v1 02/18] thermal: Add new structures and thermal_zone_device_register()

[AngeloGioacchino Del Regno]

In preparation for extending the thermal zone devices to actually
have a name and disambiguate thermal zone types/names, and to do
a reorganization in thermal_zone_device, add some new Thermal Zone
structures:

Introduce new thermal_governor_params, thermal_zone_platform_params
and thermal_zone_device_params structures which are meant to hold
the parameters for thermal zone registration and, in the future, to
stop having a catch-all thermal_zone_device structure.

While at it, also add a new thermal_zone_device_register() function
which uses the new structure(s) for registration;
the now old functions thermal_tripless_zone_device_register() and
thermal_zone_device_register_with_trips() are now advertised as
being deprecated and changed to instead act as wrappers around the
new thermal_zone_device_register().

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/thermal/gov_power_allocator.c |  38 +++----
 drivers/thermal/qcom/tsens.c          |   4 +-
 drivers/thermal/thermal_core.c        | 146 ++++++++++++++++++--------
 drivers/thermal/thermal_helpers.c     |   8 +-
 drivers/thermal/thermal_sysfs.c       |  16 +--
 include/linux/thermal.h               |  73 +++++++++++--
 6 files changed, 202 insertions(+), 83 deletions(-)

diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index 81e061f183ad..3efbc60cd148 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -167,14 +167,14 @@ static void estimate_pid_constants(struct thermal_zone_device *tz,
 	if (!temperature_threshold)
 		return;
 
-	tz->tzp->k_po = int_to_frac(sustainable_power) /
+	tz->tgp->ipa_params.k_po = int_to_frac(sustainable_power) /
 		temperature_threshold;
 
-	tz->tzp->k_pu = int_to_frac(2 * sustainable_power) /
+	tz->tgp->ipa_params.k_pu = int_to_frac(2 * sustainable_power) /
 		temperature_threshold;
 
-	k_i = tz->tzp->k_pu / 10;
-	tz->tzp->k_i = k_i > 0 ? k_i : 1;
+	k_i = tz->tgp->ipa_params.k_pu / 10;
+	tz->tgp->ipa_params.k_i = k_i > 0 ? k_i : 1;
 
 	/*
 	 * The default for k_d and integral_cutoff is 0, so we can
@@ -199,10 +199,10 @@ static u32 get_sustainable_power(struct thermal_zone_device *tz,
 {
 	u32 sustainable_power;
 
-	if (!tz->tzp->sustainable_power)
+	if (!tz->tgp->ipa_params.sustainable_power)
 		sustainable_power = estimate_sustainable_power(tz);
 	else
-		sustainable_power = tz->tzp->sustainable_power;
+		sustainable_power = tz->tgp->ipa_params.sustainable_power;
 
 	/* Check if it's init value 0 or there was update via sysfs */
 	if (sustainable_power != params->sustainable_power) {
@@ -210,7 +210,7 @@ static u32 get_sustainable_power(struct thermal_zone_device *tz,
 				       params->trip_switch_on, control_temp);
 
 		/* Do the estimation only once and make available in sysfs */
-		tz->tzp->sustainable_power = sustainable_power;
+		tz->tgp->ipa_params.sustainable_power = sustainable_power;
 		params->sustainable_power = sustainable_power;
 	}
 
@@ -252,7 +252,7 @@ static u32 pid_controller(struct thermal_zone_device *tz,
 	err = int_to_frac(err);
 
 	/* Calculate the proportional term */
-	p = mul_frac(err < 0 ? tz->tzp->k_po : tz->tzp->k_pu, err);
+	p = mul_frac(err < 0 ? tz->tgp->ipa_params.k_po : tz->tgp->ipa_params.k_pu, err);
 
 	/*
 	 * Calculate the integral term
@@ -260,10 +260,10 @@ static u32 pid_controller(struct thermal_zone_device *tz,
 	 * if the error is less than cut off allow integration (but
 	 * the integral is limited to max power)
 	 */
-	i = mul_frac(tz->tzp->k_i, params->err_integral);
+	i = mul_frac(tz->tgp->ipa_params.k_i, params->err_integral);
 
-	if (err < int_to_frac(tz->tzp->integral_cutoff)) {
-		s64 i_next = i + mul_frac(tz->tzp->k_i, err);
+	if (err < int_to_frac(tz->tgp->ipa_params.integral_cutoff)) {
+		s64 i_next = i + mul_frac(tz->tgp->ipa_params.k_i, err);
 
 		if (abs(i_next) < max_power_frac) {
 			i = i_next;
@@ -278,7 +278,7 @@ static u32 pid_controller(struct thermal_zone_device *tz,
 	 * error (i.e. driving closer to the line) results in less
 	 * power being applied, slowing down the controller)
 	 */
-	d = mul_frac(tz->tzp->k_d, err - params->prev_err);
+	d = mul_frac(tz->tgp->ipa_params.k_d, err - params->prev_err);
 	d = div_frac(d, jiffies_to_msecs(tz->passive_delay_jiffies));
 	params->prev_err = err;
 
@@ -699,9 +699,9 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
 		return ret;
 	}
 
-	if (!tz->tzp) {
-		tz->tzp = kzalloc(sizeof(*tz->tzp), GFP_KERNEL);
-		if (!tz->tzp) {
+	if (!tz->tgp) {
+		tz->tgp = kzalloc(sizeof(*tz->tgp), GFP_KERNEL);
+		if (!tz->tgp) {
 			ret = -ENOMEM;
 			goto free_params;
 		}
@@ -709,10 +709,10 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
 		params->allocated_tzp = true;
 	}
 
-	if (!tz->tzp->sustainable_power)
+	if (!tz->tgp->ipa_params.sustainable_power)
 		dev_warn(&tz->device, "power_allocator: sustainable_power will be estimated\n");
 
-	estimate_pid_constants(tz, tz->tzp->sustainable_power,
+	estimate_pid_constants(tz, tz->tgp->ipa_params.sustainable_power,
 			       params->trip_switch_on,
 			       params->trip_max->temperature);
 
@@ -736,8 +736,8 @@ static void power_allocator_unbind(struct thermal_zone_device *tz)
 	dev_dbg(&tz->device, "Unbinding from thermal zone %d\n", tz->id);
 
 	if (params->allocated_tzp) {
-		kfree(tz->tzp);
-		tz->tzp = NULL;
+		kfree(tz->tgp);
+		tz->tgp = NULL;
 	}
 
 	kfree(params->power);
diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 6d7c16ccb44d..9eddac93d683 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -252,8 +252,8 @@ int tsens_read_calibration_legacy(struct tsens_priv *priv,
 /*
  * Use this function on devices where slope and offset calculations
  * depend on calibration data read from qfprom. On others the slope
- * and offset values are derived from tz->tzp->slope and tz->tzp->offset
- * resp.
+ * and offset values are derived from tz->tgp->ipa_params.slope and
+ * tz->tgp->ipa_params.offset resp.
  */
 void compute_intercept_slope(struct tsens_priv *priv, u32 *p1,
 			     u32 *p2, u32 mode)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index dfaa6341694a..3150475b947f 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -141,13 +141,13 @@ int thermal_register_governor(struct thermal_governor *governor)
 
 	list_for_each_entry(pos, &thermal_tz_list, node) {
 		/*
-		 * only thermal zones with specified tz->tzp->governor_name
+		 * only thermal zones with specified tz->tgp->governor_name
 		 * may run with tz->govenor unset
 		 */
 		if (pos->governor)
 			continue;
 
-		name = pos->tzp->governor_name;
+		name = pos->tgp->governor_name;
 
 		if (!strncasecmp(name, governor->name, THERMAL_NAME_LENGTH)) {
 			int ret;
@@ -1261,6 +1261,8 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp);
  *		   whether trip points have been crossed (0 for interrupt
  *		   driven systems)
  *
+ * This function is deprecated. See thermal_zone_device_register().
+ *
  * This interface function adds a new thermal zone device (sensor) to
  * /sys/class/thermal folder as thermal_zone[0-*]. It tries to bind all the
  * thermal cooling devices registered at the same time.
@@ -1277,19 +1279,80 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
 					const struct thermal_zone_params *tzp, int passive_delay,
 					int polling_delay)
 {
+	struct thermal_zone_device_params tzdp = {
+		/* Thermal Zone Platform parameters */
+		.tzp = {
+			.type = type,
+			.ops = ops,
+			.trips = trips,
+			.num_trips = num_trips,
+			.mask = mask,
+			.passive_delay = passive_delay,
+			.polling_delay = polling_delay,
+			.devdata = devdata
+		},
+	};
 	struct thermal_zone_device *tz;
+
+	/* Thermal Zone Governor parameters */
+	if (tzp) {
+		tzdp.tgp = kzalloc(sizeof(*tzdp.tgp), GFP_KERNEL);
+		if (!tzdp.tgp)
+			return ERR_PTR(-ENOMEM);
+
+		tzdp.tgp->governor_name = tzp->governor_name;
+		tzdp.tzp.no_hwmon = tzp->no_hwmon;
+		memcpy(&tzdp.tgp->ipa_params, tzp, sizeof(*tzp));
+	}
+
+	tz = thermal_zone_device_register(&tzdp);
+	kfree(tzdp.tgp);
+	return tz;
+}
+EXPORT_SYMBOL_GPL(thermal_zone_device_register_with_trips);
+
+/* This function is deprecated. See thermal_zone_device_register(). */
+struct thermal_zone_device *thermal_tripless_zone_device_register(
+					const char *type,
+					void *devdata,
+					struct thermal_zone_device_ops *ops,
+					const struct thermal_zone_params *tzp)
+{
+	return thermal_zone_device_register_with_trips(type, NULL, 0, 0, devdata,
+						       ops, tzp, 0, 0);
+}
+EXPORT_SYMBOL_GPL(thermal_tripless_zone_device_register);
+
+/**
+ * thermal_zone_device_register() - register a new thermal zone device
+ * @tzdp:	Parameters of the new thermal zone device
+ *		See struct thermal_zone_device_register.
+ *
+ * This interface function adds a new thermal zone device (sensor) to
+ * /sys/class/thermal folder as thermal_zone[0-*]. It tries to bind all the
+ * thermal cooling devices registered at the same time.
+ * thermal_zone_device_unregister() must be called when the device is no
+ * longer needed. The passive cooling depends on the .get_trend() return value.
+ *
+ * Return: a pointer to the created struct thermal_zone_device or an
+ * in case of error, an ERR_PTR. Caller must check return value with
+ * IS_ERR*() helpers.
+ */
+struct thermal_zone_device *thermal_zone_device_register(struct thermal_zone_device_params *tzdp)
+{
+	struct thermal_zone_device *tz;
+	struct thermal_governor *gov;
 	int id;
 	int result;
-	struct thermal_governor *governor;
 
-	if (!type || strlen(type) == 0) {
+	if (!tzdp->tzp.type || strlen(tzdp->tzp.type) == 0) {
 		pr_err("No thermal zone type defined\n");
 		return ERR_PTR(-EINVAL);
 	}
 
-	if (strlen(type) >= THERMAL_NAME_LENGTH) {
+	if (strlen(tzdp->tzp.type) >= THERMAL_NAME_LENGTH) {
 		pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
-		       type, THERMAL_NAME_LENGTH);
+		       tzdp->tzp.type, THERMAL_NAME_LENGTH);
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -1306,17 +1369,19 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
 	 * Check will be true when the bit 31 of the mask is set.
 	 * 32 bit shift will cause overflow of 4 byte integer.
 	 */
-	if (num_trips > (BITS_PER_TYPE(int) - 1) || num_trips < 0 || mask >> num_trips) {
+	if (tzdp->tzp.num_trips > (BITS_PER_TYPE(int) - 1) ||
+	    tzdp->tzp.num_trips < 0 ||
+	    tzdp->tzp.mask >> tzdp->tzp.num_trips) {
 		pr_err("Incorrect number of thermal trips\n");
 		return ERR_PTR(-EINVAL);
 	}
 
-	if (!ops || !ops->get_temp) {
+	if (!tzdp->tzp.ops || !tzdp->tzp.ops->get_temp) {
 		pr_err("Thermal zone device ops not defined\n");
 		return ERR_PTR(-EINVAL);
 	}
 
-	if (num_trips > 0 && !trips)
+	if (tzdp->tzp.num_trips > 0 && !tzdp->tzp.trips)
 		return ERR_PTR(-EINVAL);
 
 	if (!thermal_class)
@@ -1326,11 +1391,17 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
 	if (!tz)
 		return ERR_PTR(-ENOMEM);
 
-	if (tzp) {
-		tz->tzp = kmemdup(tzp, sizeof(*tzp), GFP_KERNEL);
-		if (!tz->tzp) {
+	tz->tzp = kmemdup(&tzdp->tzp, sizeof(tzdp->tzp), GFP_KERNEL);
+	if (!tz->tzp) {
+		result = -ENOMEM;
+		goto free_tz;
+	}
+
+	if (tzdp->tgp) {
+		tz->tgp = kmemdup(tzdp->tgp, sizeof(*tzdp->tgp), GFP_KERNEL);
+		if (!tz->tgp) {
 			result = -ENOMEM;
-			goto free_tz;
+			goto free_tzp;
 		}
 	}
 
@@ -1342,27 +1413,27 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
 	id = ida_alloc(&thermal_tz_ida, GFP_KERNEL);
 	if (id < 0) {
 		result = id;
-		goto free_tzp;
+		goto free_tgp;
 	}
 
 	tz->id = id;
-	strscpy(tz->type, type, sizeof(tz->type));
+	strscpy(tz->type, tzdp->tzp.type, sizeof(tz->type));
 
-	if (!ops->critical)
-		ops->critical = thermal_zone_device_critical;
+	if (!tzdp->tzp.ops->critical)
+		tzdp->tzp.ops->critical = thermal_zone_device_critical;
 
-	tz->ops = ops;
+	tz->ops = tzdp->tzp.ops;
 	tz->device.class = thermal_class;
-	tz->devdata = devdata;
-	tz->trips = trips;
-	tz->num_trips = num_trips;
+	tz->devdata = tzdp->tzp.devdata;
+	tz->trips = tzdp->tzp.trips;
+	tz->num_trips = tzdp->tzp.num_trips;
 
-	thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
-	thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
+	thermal_set_delay_jiffies(&tz->passive_delay_jiffies, tzdp->tzp.passive_delay);
+	thermal_set_delay_jiffies(&tz->polling_delay_jiffies, tzdp->tzp.polling_delay);
 
 	/* sys I/F */
 	/* Add nodes that are always present via .groups */
-	result = thermal_zone_create_device_groups(tz, mask);
+	result = thermal_zone_create_device_groups(tz, tzdp->tzp.mask);
 	if (result)
 		goto remove_id;
 
@@ -1381,12 +1452,12 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
 	/* Update 'this' zone's governor information */
 	mutex_lock(&thermal_governor_lock);
 
-	if (tz->tzp)
-		governor = __find_governor(tz->tzp->governor_name);
+	if (tzdp->tgp)
+		gov = __find_governor(tzdp->tgp->governor_name);
 	else
-		governor = def_governor;
+		gov = def_governor;
 
-	result = thermal_set_governor(tz, governor);
+	result = thermal_set_governor(tz, gov);
 	if (result) {
 		mutex_unlock(&thermal_governor_lock);
 		goto unregister;
@@ -1394,7 +1465,7 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
 
 	mutex_unlock(&thermal_governor_lock);
 
-	if (!tz->tzp || !tz->tzp->no_hwmon) {
+	if (!tzdp->tzp.no_hwmon) {
 		result = thermal_add_hwmon_sysfs(tz);
 		if (result)
 			goto unregister;
@@ -1426,24 +1497,15 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
 	put_device(&tz->device);
 remove_id:
 	ida_free(&thermal_tz_ida, id);
+free_tgp:
+	kfree(tz->tgp);
 free_tzp:
 	kfree(tz->tzp);
 free_tz:
 	kfree(tz);
 	return ERR_PTR(result);
 }
-EXPORT_SYMBOL_GPL(thermal_zone_device_register_with_trips);
-
-struct thermal_zone_device *thermal_tripless_zone_device_register(
-					const char *type,
-					void *devdata,
-					struct thermal_zone_device_ops *ops,
-					const struct thermal_zone_params *tzp)
-{
-	return thermal_zone_device_register_with_trips(type, NULL, 0, 0, devdata,
-						       ops, tzp, 0, 0);
-}
-EXPORT_SYMBOL_GPL(thermal_tripless_zone_device_register);
+EXPORT_SYMBOL_GPL(thermal_zone_device_register);
 
 void *thermal_zone_device_priv(struct thermal_zone_device *tzd)
 {
@@ -1514,7 +1576,7 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
 
 	device_del(&tz->device);
 
-	kfree(tz->tzp);
+	kfree(tz->tgp);
 
 	put_device(&tz->device);
 
diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
index 0329f4a71b02..a72095cfb771 100644
--- a/drivers/thermal/thermal_helpers.c
+++ b/drivers/thermal/thermal_helpers.c
@@ -211,8 +211,8 @@ void thermal_cdev_update(struct thermal_cooling_device *cdev)
  */
 int thermal_zone_get_slope(struct thermal_zone_device *tz)
 {
-	if (tz && tz->tzp)
-		return tz->tzp->slope;
+	if (tz && tz->tgp)
+		return tz->tgp->ipa_params.slope;
 	return 1;
 }
 EXPORT_SYMBOL_GPL(thermal_zone_get_slope);
@@ -226,8 +226,8 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_slope);
  */
 int thermal_zone_get_offset(struct thermal_zone_device *tz)
 {
-	if (tz && tz->tzp)
-		return tz->tzp->offset;
+	if (tz && tz->tgp)
+		return tz->tgp->ipa_params.offset;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(thermal_zone_get_offset);
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index f4033865b093..3f5f4a04a754 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -271,8 +271,8 @@ sustainable_power_show(struct device *dev, struct device_attribute *devattr,
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
 
-	if (tz->tzp)
-		return sprintf(buf, "%u\n", tz->tzp->sustainable_power);
+	if (tz->tgp)
+		return sprintf(buf, "%u\n", tz->tgp->ipa_params.sustainable_power);
 	else
 		return -EIO;
 }
@@ -284,13 +284,13 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
 	u32 sustainable_power;
 
-	if (!tz->tzp)
+	if (!tz->tgp)
 		return -EIO;
 
 	if (kstrtou32(buf, 10, &sustainable_power))
 		return -EINVAL;
 
-	tz->tzp->sustainable_power = sustainable_power;
+	tz->tgp->ipa_params.sustainable_power = sustainable_power;
 
 	return count;
 }
@@ -302,8 +302,8 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
 	{								\
 	struct thermal_zone_device *tz = to_thermal_zone(dev);		\
 									\
-	if (tz->tzp)							\
-		return sprintf(buf, "%d\n", tz->tzp->name);		\
+	if (tz->tgp)							\
+		return sprintf(buf, "%d\n", tz->tgp->ipa_params.name);	\
 	else								\
 		return -EIO;						\
 	}								\
@@ -315,13 +315,13 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
 		struct thermal_zone_device *tz = to_thermal_zone(dev);	\
 		s32 value;						\
 									\
-		if (!tz->tzp)						\
+		if (!tz->tgp)						\
 			return -EIO;					\
 									\
 		if (kstrtos32(buf, 10, &value))				\
 			return -EINVAL;					\
 									\
-		tz->tzp->name = value;					\
+		tz->tgp->ipa_params.name = value;			\
 									\
 		return count;						\
 	}								\
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 65d8f92a9a0d..7a540b746703 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -149,7 +149,8 @@ struct thermal_cooling_device {
 			passive trip point.
  * @need_update:	if equals 1, thermal_zone_device_update needs to be invoked.
  * @ops:	operations this &thermal_zone_device supports
- * @tzp:	thermal zone parameters
+ * @tzp:		Thermal zone parameters
+ * @tgp:		Thermal zone governor parameters
  * @governor:	pointer to the governor for this thermal zone
  * @governor_data:	private pointer for governor data
  * @thermal_instances:	list of &struct thermal_instance of this thermal zone
@@ -184,7 +185,8 @@ struct thermal_zone_device {
 	int prev_high_trip;
 	atomic_t need_update;
 	struct thermal_zone_device_ops *ops;
-	struct thermal_zone_params *tzp;
+	struct thermal_zone_platform_params *tzp;
+	struct thermal_governor_params *tgp;
 	struct thermal_governor *governor;
 	void *governor_data;
 	struct list_head thermal_instances;
@@ -224,15 +226,12 @@ struct thermal_governor {
 	struct list_head	governor_list;
 };
 
-/* Structure to define Thermal Zone parameters */
+/* Structure to define Thermal Zone IPA parameters */
 struct thermal_zone_params {
+	/* Scheduled for removal - see struct thermal_governor_params. */
 	const char *governor_name;
 
-	/*
-	 * a boolean to indicate if the thermal to hwmon sysfs interface
-	 * is required. when no_hwmon == false, a hwmon sysfs interface
-	 * will be created. when no_hwmon == true, nothing will be done
-	 */
+	/* Scheduled for removal - see struct thermal_zone_platform_params. */
 	bool no_hwmon;
 
 	/*
@@ -274,6 +273,58 @@ struct thermal_zone_params {
 	int offset;
 };
 
+/**
+ * struct thermal_governor_params - Thermal Zone governor parameters
+ * @governor_name:	Name of the Thermal Zone governor
+ * @ipa_params:		IPA parameters for Thermal Zone governors
+ */
+struct thermal_governor_params {
+	const char *governor_name;
+	struct thermal_zone_params ipa_params;
+};
+
+/**
+ * struct thermal_zone_platform_params - Thermal Zone parameters
+ * @type:		The thermal zone device type
+ * @ops:		Standard thermal zone device callbacks
+ * @trips:		Pointer to an array of thermal trips, if any
+ * @num_trips:		Number of trip points the thermal zone support
+ * @mask:		Bit string indicating the writeablility of trip points
+ * @passive_delay:	Number of milliseconds to wait between polls when
+ *			performing passive cooling
+ * @polling_delay:	Number of milliseconds to wait between polls when checking
+ *			whether trip points have been crossed (0 for interrupt
+ *			driven systems)
+ * @devdata:		Private device data
+ * @no_hwmon:		Indicates whether the thermal to hwmon sysfs interface is
+ *			required; this means that when no_hwmon == false, a hwmon
+ *			sysfs interface will be created and when no_hwmon == true
+ *			nothing will be done
+ */
+struct thermal_zone_platform_params {
+	const char *type;
+	struct thermal_zone_device_ops *ops;
+	struct thermal_trip *trips;
+	int num_trips;
+	int mask;
+
+	int passive_delay;
+	int polling_delay;
+
+	void *devdata;
+	bool no_hwmon;
+};
+
+/**
+ * struct thermal_zone_device_params - Thermal Zone device parameters
+ * @tzp:		Thermal zone platform parameters
+ * @tgp:		Thermal zone governor parameters
+ */
+struct thermal_zone_device_params {
+	struct thermal_zone_platform_params tzp;
+	struct thermal_governor_params *tgp;
+};
+
 /* Function declarations */
 #ifdef CONFIG_THERMAL_OF
 struct thermal_zone_device *devm_thermal_of_zone_register(struct device *dev, int id, void *data,
@@ -328,6 +379,8 @@ struct thermal_zone_device *thermal_tripless_zone_device_register(
 					struct thermal_zone_device_ops *ops,
 					const struct thermal_zone_params *tzp);
 
+struct thermal_zone_device *thermal_zone_device_register(struct thermal_zone_device_params *tzdp);
+
 void thermal_zone_device_unregister(struct thermal_zone_device *tz);
 
 void *thermal_zone_device_priv(struct thermal_zone_device *tzd);
@@ -390,6 +443,10 @@ static inline struct thermal_zone_device *thermal_tripless_zone_device_register(
 					const struct thermal_zone_params *tzp)
 { return ERR_PTR(-ENODEV); }
 
+static inline struct thermal_zone_device *thermal_zone_device_register(
+					struct thermal_zone_device_params *tzdp)
+{ return ERR_PTR(-ENODEV); }
+
 static inline void thermal_zone_device_unregister(struct thermal_zone_device *tz)
 { }
 
-- 
2.43.0

Re: [PATCH v1 02/18] thermal: Add new structures and thermal_zone_device_register()

[Geert Uytterhoeven]

Hi Angelo,

On Tue, Jan 30, 2024 at 12:13 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
> In preparation for extending the thermal zone devices to actually
> have a name and disambiguate thermal zone types/names, and to do
> a reorganization in thermal_zone_device, add some new Thermal Zone
> structures:
>
> Introduce new thermal_governor_params, thermal_zone_platform_params
> and thermal_zone_device_params structures which are meant to hold
> the parameters for thermal zone registration and, in the future, to
> stop having a catch-all thermal_zone_device structure.
>
> While at it, also add a new thermal_zone_device_register() function
> which uses the new structure(s) for registration;
> the now old functions thermal_tripless_zone_device_register() and
> thermal_zone_device_register_with_trips() are now advertised as
> being deprecated and changed to instead act as wrappers around the
> new thermal_zone_device_register().
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Thanks for your patch!

> ---
>  drivers/thermal/gov_power_allocator.c |  38 +++----
>  drivers/thermal/qcom/tsens.c          |   4 +-
>  drivers/thermal/thermal_core.c        | 146 ++++++++++++++++++--------
>  drivers/thermal/thermal_helpers.c     |   8 +-
>  drivers/thermal/thermal_sysfs.c       |  16 +--
>  include/linux/thermal.h               |  73 +++++++++++--
>  6 files changed, 202 insertions(+), 83 deletions(-)

This also needs an update to the documentation, which still refers to
the old function removed in commit edd220b33f479cf9 ("thermal: core:
Drop thermal_zone_device_register()") in v6.6.:

Documentation/driver-api/thermal/power_allocator.rst:`thermal_zone_device_register()`
Documentation/driver-api/thermal/power_allocator.rst:`thermal_zone_device_register()`
(i.e., platform code), then weights
Documentation/driver-api/thermal/sysfs-api.rst:
*thermal_zone_device_register(char *type,

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v1 02/18] thermal: Add new structures and thermal_zone_device_register()

[Rafael J. Wysocki]

On Tue, Jan 30, 2024 at 12:13 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> In preparation for extending the thermal zone devices to actually
> have a name and disambiguate thermal zone types/names, and to do
> a reorganization in thermal_zone_device, add some new Thermal Zone
> structures:
>
> Introduce new thermal_governor_params, thermal_zone_platform_params
> and thermal_zone_device_params structures which are meant to hold
> the parameters for thermal zone registration and, in the future, to
> stop having a catch-all thermal_zone_device structure.
>
> While at it, also add a new thermal_zone_device_register() function
> which uses the new structure(s) for registration;
> the now old functions thermal_tripless_zone_device_register() and
> thermal_zone_device_register_with_trips() are now advertised as
> being deprecated and changed to instead act as wrappers around the
> new thermal_zone_device_register().
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/thermal/gov_power_allocator.c |  38 +++----
>  drivers/thermal/qcom/tsens.c          |   4 +-
>  drivers/thermal/thermal_core.c        | 146 ++++++++++++++++++--------
>  drivers/thermal/thermal_helpers.c     |   8 +-
>  drivers/thermal/thermal_sysfs.c       |  16 +--
>  include/linux/thermal.h               |  73 +++++++++++--
>  6 files changed, 202 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> index 81e061f183ad..3efbc60cd148 100644
> --- a/drivers/thermal/gov_power_allocator.c
> +++ b/drivers/thermal/gov_power_allocator.c
> @@ -167,14 +167,14 @@ static void estimate_pid_constants(struct thermal_zone_device *tz,
>         if (!temperature_threshold)
>                 return;
>
> -       tz->tzp->k_po = int_to_frac(sustainable_power) /
> +       tz->tgp->ipa_params.k_po = int_to_frac(sustainable_power) /

This is getting hard to read.

I would at least introduce a local zone_ipa_params and use it to refer
to this field:

zone_ipa_params->K-no = int_to_frac(sustainable_power) / temperature_threshold;

and analogously below.

>                 temperature_threshold;
>
> -       tz->tzp->k_pu = int_to_frac(2 * sustainable_power) /
> +       tz->tgp->ipa_params.k_pu = int_to_frac(2 * sustainable_power) /
>                 temperature_threshold;
>
> -       k_i = tz->tzp->k_pu / 10;
> -       tz->tzp->k_i = k_i > 0 ? k_i : 1;
> +       k_i = tz->tgp->ipa_params.k_pu / 10;
> +       tz->tgp->ipa_params.k_i = k_i > 0 ? k_i : 1;
>
>         /*
>          * The default for k_d and integral_cutoff is 0, so we can
> @@ -199,10 +199,10 @@ static u32 get_sustainable_power(struct thermal_zone_device *tz,
>  {
>         u32 sustainable_power;
>
> -       if (!tz->tzp->sustainable_power)
> +       if (!tz->tgp->ipa_params.sustainable_power)
>                 sustainable_power = estimate_sustainable_power(tz);
>         else
> -               sustainable_power = tz->tzp->sustainable_power;
> +               sustainable_power = tz->tgp->ipa_params.sustainable_power;
>
>         /* Check if it's init value 0 or there was update via sysfs */
>         if (sustainable_power != params->sustainable_power) {
> @@ -210,7 +210,7 @@ static u32 get_sustainable_power(struct thermal_zone_device *tz,
>                                        params->trip_switch_on, control_temp);
>
>                 /* Do the estimation only once and make available in sysfs */
> -               tz->tzp->sustainable_power = sustainable_power;
> +               tz->tgp->ipa_params.sustainable_power = sustainable_power;
>                 params->sustainable_power = sustainable_power;
>         }
>
> @@ -252,7 +252,7 @@ static u32 pid_controller(struct thermal_zone_device *tz,
>         err = int_to_frac(err);
>
>         /* Calculate the proportional term */
> -       p = mul_frac(err < 0 ? tz->tzp->k_po : tz->tzp->k_pu, err);
> +       p = mul_frac(err < 0 ? tz->tgp->ipa_params.k_po : tz->tgp->ipa_params.k_pu, err);
>
>         /*
>          * Calculate the integral term
> @@ -260,10 +260,10 @@ static u32 pid_controller(struct thermal_zone_device *tz,
>          * if the error is less than cut off allow integration (but
>          * the integral is limited to max power)
>          */
> -       i = mul_frac(tz->tzp->k_i, params->err_integral);
> +       i = mul_frac(tz->tgp->ipa_params.k_i, params->err_integral);
>
> -       if (err < int_to_frac(tz->tzp->integral_cutoff)) {
> -               s64 i_next = i + mul_frac(tz->tzp->k_i, err);
> +       if (err < int_to_frac(tz->tgp->ipa_params.integral_cutoff)) {
> +               s64 i_next = i + mul_frac(tz->tgp->ipa_params.k_i, err);
>
>                 if (abs(i_next) < max_power_frac) {
>                         i = i_next;
> @@ -278,7 +278,7 @@ static u32 pid_controller(struct thermal_zone_device *tz,
>          * error (i.e. driving closer to the line) results in less
>          * power being applied, slowing down the controller)
>          */
> -       d = mul_frac(tz->tzp->k_d, err - params->prev_err);
> +       d = mul_frac(tz->tgp->ipa_params.k_d, err - params->prev_err);
>         d = div_frac(d, jiffies_to_msecs(tz->passive_delay_jiffies));
>         params->prev_err = err;
>
> @@ -699,9 +699,9 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
>                 return ret;
>         }
>
> -       if (!tz->tzp) {
> -               tz->tzp = kzalloc(sizeof(*tz->tzp), GFP_KERNEL);
> -               if (!tz->tzp) {
> +       if (!tz->tgp) {
> +               tz->tgp = kzalloc(sizeof(*tz->tgp), GFP_KERNEL);
> +               if (!tz->tgp) {
>                         ret = -ENOMEM;
>                         goto free_params;
>                 }
> @@ -709,10 +709,10 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
>                 params->allocated_tzp = true;
>         }
>
> -       if (!tz->tzp->sustainable_power)
> +       if (!tz->tgp->ipa_params.sustainable_power)
>                 dev_warn(&tz->device, "power_allocator: sustainable_power will be estimated\n");
>
> -       estimate_pid_constants(tz, tz->tzp->sustainable_power,
> +       estimate_pid_constants(tz, tz->tgp->ipa_params.sustainable_power,
>                                params->trip_switch_on,
>                                params->trip_max->temperature);
>
> @@ -736,8 +736,8 @@ static void power_allocator_unbind(struct thermal_zone_device *tz)
>         dev_dbg(&tz->device, "Unbinding from thermal zone %d\n", tz->id);
>
>         if (params->allocated_tzp) {
> -               kfree(tz->tzp);
> -               tz->tzp = NULL;
> +               kfree(tz->tgp);
> +               tz->tgp = NULL;
>         }
>
>         kfree(params->power);
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index 6d7c16ccb44d..9eddac93d683 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -252,8 +252,8 @@ int tsens_read_calibration_legacy(struct tsens_priv *priv,
>  /*
>   * Use this function on devices where slope and offset calculations
>   * depend on calibration data read from qfprom. On others the slope
> - * and offset values are derived from tz->tzp->slope and tz->tzp->offset
> - * resp.
> + * and offset values are derived from tz->tgp->ipa_params.slope and
> + * tz->tgp->ipa_params.offset resp.
>   */
>  void compute_intercept_slope(struct tsens_priv *priv, u32 *p1,
>                              u32 *p2, u32 mode)
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index dfaa6341694a..3150475b947f 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -141,13 +141,13 @@ int thermal_register_governor(struct thermal_governor *governor)
>
>         list_for_each_entry(pos, &thermal_tz_list, node) {
>                 /*
> -                * only thermal zones with specified tz->tzp->governor_name
> +                * only thermal zones with specified tz->tgp->governor_name
>                  * may run with tz->govenor unset
>                  */
>                 if (pos->governor)
>                         continue;
>
> -               name = pos->tzp->governor_name;
> +               name = pos->tgp->governor_name;
>
>                 if (!strncasecmp(name, governor->name, THERMAL_NAME_LENGTH)) {
>                         int ret;
> @@ -1261,6 +1261,8 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp);
>   *                whether trip points have been crossed (0 for interrupt
>   *                driven systems)
>   *
> + * This function is deprecated. See thermal_zone_device_register().
> + *
>   * This interface function adds a new thermal zone device (sensor) to
>   * /sys/class/thermal folder as thermal_zone[0-*]. It tries to bind all the
>   * thermal cooling devices registered at the same time.
> @@ -1277,19 +1279,80 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
>                                         const struct thermal_zone_params *tzp, int passive_delay,
>                                         int polling_delay)
>  {
> +       struct thermal_zone_device_params tzdp = {
> +               /* Thermal Zone Platform parameters */
> +               .tzp = {
> +                       .type = type,
> +                       .ops = ops,
> +                       .trips = trips,
> +                       .num_trips = num_trips,
> +                       .mask = mask,

I'm not sure about trips, num_trips and mask.

In the future, trips and mask will only be used once during zone
registration (and mask will probably go away entirely at one point)
and num_trips defines the shape of the zone, so to speak, so it makes
sense to pass it directly in the arg list.

> +                       .passive_delay = passive_delay,
> +                       .polling_delay = polling_delay,
> +                       .devdata = devdata
> +               },
> +       };
>         struct thermal_zone_device *tz;
> +
> +       /* Thermal Zone Governor parameters */
> +       if (tzp) {
> +               tzdp.tgp = kzalloc(sizeof(*tzdp.tgp), GFP_KERNEL);
> +               if (!tzdp.tgp)
> +                       return ERR_PTR(-ENOMEM);
> +
> +               tzdp.tgp->governor_name = tzp->governor_name;
> +               tzdp.tzp.no_hwmon = tzp->no_hwmon;
> +               memcpy(&tzdp.tgp->ipa_params, tzp, sizeof(*tzp));
> +       }
> +
> +       tz = thermal_zone_device_register(&tzdp);
> +       kfree(tzdp.tgp);
> +       return tz;
> +}
> +EXPORT_SYMBOL_GPL(thermal_zone_device_register_with_trips);
> +
> +/* This function is deprecated. See thermal_zone_device_register(). */

I'm not sure why you want to deprecate this function.

Tripless thermal zones are a special case and it makes sense to me to
register them in a special way.

> +struct thermal_zone_device *thermal_tripless_zone_device_register(
> +                                       const char *type,
> +                                       void *devdata,
> +                                       struct thermal_zone_device_ops *ops,
> +                                       const struct thermal_zone_params *tzp)
> +{
> +       return thermal_zone_device_register_with_trips(type, NULL, 0, 0, devdata,
> +                                                      ops, tzp, 0, 0);
> +}
> +EXPORT_SYMBOL_GPL(thermal_tripless_zone_device_register);
> +
> +/**
> + * thermal_zone_device_register() - register a new thermal zone device
> + * @tzdp:      Parameters of the new thermal zone device
> + *             See struct thermal_zone_device_register.

I don't quite agree that passing just one argument to this function is
an improvement.

First off, it is better to pass some values in the arg list just
because they will be only used once during registration and so it is
not useful to store them within the thermal zone object.  I mentioned
trips and mask above, but type is also copied into the thermal zone
object, so why to store a pointer to the original string?

> + *
> + * This interface function adds a new thermal zone device (sensor) to
> + * /sys/class/thermal folder as thermal_zone[0-*]. It tries to bind all the
> + * thermal cooling devices registered at the same time.
> + * thermal_zone_device_unregister() must be called when the device is no
> + * longer needed. The passive cooling depends on the .get_trend() return value.
> + *
> + * Return: a pointer to the created struct thermal_zone_device or an
> + * in case of error, an ERR_PTR. Caller must check return value with
> + * IS_ERR*() helpers.
> + */
> +struct thermal_zone_device *thermal_zone_device_register(struct thermal_zone_device_params *tzdp)
> +{
> +       struct thermal_zone_device *tz;
> +       struct thermal_governor *gov;
>         int id;
>         int result;
> -       struct thermal_governor *governor;
>
> -       if (!type || strlen(type) == 0) {
> +       if (!tzdp->tzp.type || strlen(tzdp->tzp.type) == 0) {
>                 pr_err("No thermal zone type defined\n");
>                 return ERR_PTR(-EINVAL);
>         }
>
> -       if (strlen(type) >= THERMAL_NAME_LENGTH) {
> +       if (strlen(tzdp->tzp.type) >= THERMAL_NAME_LENGTH) {
>                 pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
> -                      type, THERMAL_NAME_LENGTH);
> +                      tzdp->tzp.type, THERMAL_NAME_LENGTH);
>                 return ERR_PTR(-EINVAL);
>         }
>
> @@ -1306,17 +1369,19 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
>          * Check will be true when the bit 31 of the mask is set.
>          * 32 bit shift will cause overflow of 4 byte integer.
>          */
> -       if (num_trips > (BITS_PER_TYPE(int) - 1) || num_trips < 0 || mask >> num_trips) {
> +       if (tzdp->tzp.num_trips > (BITS_PER_TYPE(int) - 1) ||
> +           tzdp->tzp.num_trips < 0 ||
> +           tzdp->tzp.mask >> tzdp->tzp.num_trips) {
>                 pr_err("Incorrect number of thermal trips\n");
>                 return ERR_PTR(-EINVAL);
>         }
>
> -       if (!ops || !ops->get_temp) {
> +       if (!tzdp->tzp.ops || !tzdp->tzp.ops->get_temp) {
>                 pr_err("Thermal zone device ops not defined\n");
>                 return ERR_PTR(-EINVAL);
>         }
>
> -       if (num_trips > 0 && !trips)
> +       if (tzdp->tzp.num_trips > 0 && !tzdp->tzp.trips)
>                 return ERR_PTR(-EINVAL);
>
>         if (!thermal_class)
> @@ -1326,11 +1391,17 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
>         if (!tz)
>                 return ERR_PTR(-ENOMEM);
>
> -       if (tzp) {
> -               tz->tzp = kmemdup(tzp, sizeof(*tzp), GFP_KERNEL);
> -               if (!tz->tzp) {
> +       tz->tzp = kmemdup(&tzdp->tzp, sizeof(tzdp->tzp), GFP_KERNEL);
> +       if (!tz->tzp) {
> +               result = -ENOMEM;
> +               goto free_tz;
> +       }
> +
> +       if (tzdp->tgp) {
> +               tz->tgp = kmemdup(tzdp->tgp, sizeof(*tzdp->tgp), GFP_KERNEL);
> +               if (!tz->tgp) {
>                         result = -ENOMEM;
> -                       goto free_tz;
> +                       goto free_tzp;
>                 }
>         }
>
> @@ -1342,27 +1413,27 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
>         id = ida_alloc(&thermal_tz_ida, GFP_KERNEL);
>         if (id < 0) {
>                 result = id;
> -               goto free_tzp;
> +               goto free_tgp;
>         }
>
>         tz->id = id;
> -       strscpy(tz->type, type, sizeof(tz->type));
> +       strscpy(tz->type, tzdp->tzp.type, sizeof(tz->type));
>
> -       if (!ops->critical)
> -               ops->critical = thermal_zone_device_critical;
> +       if (!tzdp->tzp.ops->critical)
> +               tzdp->tzp.ops->critical = thermal_zone_device_critical;
>
> -       tz->ops = ops;
> +       tz->ops = tzdp->tzp.ops;
>         tz->device.class = thermal_class;
> -       tz->devdata = devdata;
> -       tz->trips = trips;
> -       tz->num_trips = num_trips;
> +       tz->devdata = tzdp->tzp.devdata;
> +       tz->trips = tzdp->tzp.trips;
> +       tz->num_trips = tzdp->tzp.num_trips;
>
> -       thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
> -       thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
> +       thermal_set_delay_jiffies(&tz->passive_delay_jiffies, tzdp->tzp.passive_delay);
> +       thermal_set_delay_jiffies(&tz->polling_delay_jiffies, tzdp->tzp.polling_delay);
>
>         /* sys I/F */
>         /* Add nodes that are always present via .groups */
> -       result = thermal_zone_create_device_groups(tz, mask);
> +       result = thermal_zone_create_device_groups(tz, tzdp->tzp.mask);
>         if (result)
>                 goto remove_id;
>
> @@ -1381,12 +1452,12 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
>         /* Update 'this' zone's governor information */
>         mutex_lock(&thermal_governor_lock);
>
> -       if (tz->tzp)
> -               governor = __find_governor(tz->tzp->governor_name);
> +       if (tzdp->tgp)
> +               gov = __find_governor(tzdp->tgp->governor_name);
>         else
> -               governor = def_governor;
> +               gov = def_governor;
>
> -       result = thermal_set_governor(tz, governor);
> +       result = thermal_set_governor(tz, gov);
>         if (result) {
>                 mutex_unlock(&thermal_governor_lock);
>                 goto unregister;
> @@ -1394,7 +1465,7 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
>
>         mutex_unlock(&thermal_governor_lock);
>
> -       if (!tz->tzp || !tz->tzp->no_hwmon) {
> +       if (!tzdp->tzp.no_hwmon) {
>                 result = thermal_add_hwmon_sysfs(tz);
>                 if (result)
>                         goto unregister;
> @@ -1426,24 +1497,15 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
>         put_device(&tz->device);
>  remove_id:
>         ida_free(&thermal_tz_ida, id);
> +free_tgp:
> +       kfree(tz->tgp);
>  free_tzp:
>         kfree(tz->tzp);
>  free_tz:
>         kfree(tz);
>         return ERR_PTR(result);
>  }
> -EXPORT_SYMBOL_GPL(thermal_zone_device_register_with_trips);
> -
> -struct thermal_zone_device *thermal_tripless_zone_device_register(
> -                                       const char *type,
> -                                       void *devdata,
> -                                       struct thermal_zone_device_ops *ops,
> -                                       const struct thermal_zone_params *tzp)
> -{
> -       return thermal_zone_device_register_with_trips(type, NULL, 0, 0, devdata,
> -                                                      ops, tzp, 0, 0);
> -}
> -EXPORT_SYMBOL_GPL(thermal_tripless_zone_device_register);
> +EXPORT_SYMBOL_GPL(thermal_zone_device_register);
>
>  void *thermal_zone_device_priv(struct thermal_zone_device *tzd)
>  {
> @@ -1514,7 +1576,7 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>
>         device_del(&tz->device);
>
> -       kfree(tz->tzp);
> +       kfree(tz->tgp);
>
>         put_device(&tz->device);
>
> diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
> index 0329f4a71b02..a72095cfb771 100644
> --- a/drivers/thermal/thermal_helpers.c
> +++ b/drivers/thermal/thermal_helpers.c
> @@ -211,8 +211,8 @@ void thermal_cdev_update(struct thermal_cooling_device *cdev)
>   */
>  int thermal_zone_get_slope(struct thermal_zone_device *tz)
>  {
> -       if (tz && tz->tzp)
> -               return tz->tzp->slope;
> +       if (tz && tz->tgp)
> +               return tz->tgp->ipa_params.slope;
>         return 1;
>  }
>  EXPORT_SYMBOL_GPL(thermal_zone_get_slope);
> @@ -226,8 +226,8 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_slope);
>   */
>  int thermal_zone_get_offset(struct thermal_zone_device *tz)
>  {
> -       if (tz && tz->tzp)
> -               return tz->tzp->offset;
> +       if (tz && tz->tgp)
> +               return tz->tgp->ipa_params.offset;
>         return 0;
>  }
>  EXPORT_SYMBOL_GPL(thermal_zone_get_offset);
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index f4033865b093..3f5f4a04a754 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -271,8 +271,8 @@ sustainable_power_show(struct device *dev, struct device_attribute *devattr,
>  {
>         struct thermal_zone_device *tz = to_thermal_zone(dev);
>
> -       if (tz->tzp)
> -               return sprintf(buf, "%u\n", tz->tzp->sustainable_power);
> +       if (tz->tgp)
> +               return sprintf(buf, "%u\n", tz->tgp->ipa_params.sustainable_power);
>         else
>                 return -EIO;
>  }
> @@ -284,13 +284,13 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
>         struct thermal_zone_device *tz = to_thermal_zone(dev);
>         u32 sustainable_power;
>
> -       if (!tz->tzp)
> +       if (!tz->tgp)
>                 return -EIO;
>
>         if (kstrtou32(buf, 10, &sustainable_power))
>                 return -EINVAL;
>
> -       tz->tzp->sustainable_power = sustainable_power;
> +       tz->tgp->ipa_params.sustainable_power = sustainable_power;
>
>         return count;
>  }
> @@ -302,8 +302,8 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
>         {                                                               \
>         struct thermal_zone_device *tz = to_thermal_zone(dev);          \
>                                                                         \
> -       if (tz->tzp)                                                    \
> -               return sprintf(buf, "%d\n", tz->tzp->name);             \
> +       if (tz->tgp)                                                    \
> +               return sprintf(buf, "%d\n", tz->tgp->ipa_params.name);  \
>         else                                                            \
>                 return -EIO;                                            \
>         }                                                               \
> @@ -315,13 +315,13 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
>                 struct thermal_zone_device *tz = to_thermal_zone(dev);  \
>                 s32 value;                                              \
>                                                                         \
> -               if (!tz->tzp)                                           \
> +               if (!tz->tgp)                                           \
>                         return -EIO;                                    \
>                                                                         \
>                 if (kstrtos32(buf, 10, &value))                         \
>                         return -EINVAL;                                 \
>                                                                         \
> -               tz->tzp->name = value;                                  \
> +               tz->tgp->ipa_params.name = value;                       \
>                                                                         \
>                 return count;                                           \
>         }                                                               \
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 65d8f92a9a0d..7a540b746703 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -149,7 +149,8 @@ struct thermal_cooling_device {
>                         passive trip point.
>   * @need_update:       if equals 1, thermal_zone_device_update needs to be invoked.
>   * @ops:       operations this &thermal_zone_device supports
> - * @tzp:       thermal zone parameters
> + * @tzp:               Thermal zone parameters
> + * @tgp:               Thermal zone governor parameters
>   * @governor:  pointer to the governor for this thermal zone
>   * @governor_data:     private pointer for governor data
>   * @thermal_instances: list of &struct thermal_instance of this thermal zone
> @@ -184,7 +185,8 @@ struct thermal_zone_device {
>         int prev_high_trip;
>         atomic_t need_update;
>         struct thermal_zone_device_ops *ops;
> -       struct thermal_zone_params *tzp;
> +       struct thermal_zone_platform_params *tzp;
> +       struct thermal_governor_params *tgp;

I agree with doing a split here, but I'm not sure about moving items
from the arg list to struct thermal_zone_platform_params (as mentioned
above).

Also the naming is quite inconsistent.  IMO it would be better to call
the first pointer "tzpp", rename struct thermal_governor_params to
struct thermal_zone_governor_params and call the second pointer
"tzgp".

>         struct thermal_governor *governor;
>         void *governor_data;
>         struct list_head thermal_instances;
> @@ -224,15 +226,12 @@ struct thermal_governor {
>         struct list_head        governor_list;
>  };
>
> -/* Structure to define Thermal Zone parameters */
> +/* Structure to define Thermal Zone IPA parameters */
>  struct thermal_zone_params {
> +       /* Scheduled for removal - see struct thermal_governor_params. */
>         const char *governor_name;
>
> -       /*
> -        * a boolean to indicate if the thermal to hwmon sysfs interface
> -        * is required. when no_hwmon == false, a hwmon sysfs interface
> -        * will be created. when no_hwmon == true, nothing will be done
> -        */
> +       /* Scheduled for removal - see struct thermal_zone_platform_params. */
>         bool no_hwmon;
>
>         /*
> @@ -274,6 +273,58 @@ struct thermal_zone_params {
>         int offset;
>  };
>
> +/**
> + * struct thermal_governor_params - Thermal Zone governor parameters
> + * @governor_name:     Name of the Thermal Zone governor
> + * @ipa_params:                IPA parameters for Thermal Zone governors
> + */
> +struct thermal_governor_params {
> +       const char *governor_name;
> +       struct thermal_zone_params ipa_params;
> +};
> +
> +/**
> + * struct thermal_zone_platform_params - Thermal Zone parameters
> + * @type:              The thermal zone device type
> + * @ops:               Standard thermal zone device callbacks
> + * @trips:             Pointer to an array of thermal trips, if any
> + * @num_trips:         Number of trip points the thermal zone support
> + * @mask:              Bit string indicating the writeablility of trip points
> + * @passive_delay:     Number of milliseconds to wait between polls when
> + *                     performing passive cooling
> + * @polling_delay:     Number of milliseconds to wait between polls when checking
> + *                     whether trip points have been crossed (0 for interrupt
> + *                     driven systems)
> + * @devdata:           Private device data
> + * @no_hwmon:          Indicates whether the thermal to hwmon sysfs interface is
> + *                     required; this means that when no_hwmon == false, a hwmon
> + *                     sysfs interface will be created and when no_hwmon == true
> + *                     nothing will be done

Could this be the other way around?  So could this field be called
hwmon and could hwmon = true cause the hwmon sysfs i/f to be created?
It would be less confusing this way IMO.

> + */
> +struct thermal_zone_platform_params {
> +       const char *type;
> +       struct thermal_zone_device_ops *ops;
> +       struct thermal_trip *trips;
> +       int num_trips;
> +       int mask;

As I said above, IMO type, trips, num_trips and mask should stay in
the registration function arg list and not be stored in
thermal_zone_platform_params.

> +
> +       int passive_delay;
> +       int polling_delay;
> +
> +       void *devdata;

Same here.  It is just copied directly into tz during registration, so
why store a pointer to it in the params structure?

And same for ops probably.

> +       bool no_hwmon;
> +};
> +
> +/**
> + * struct thermal_zone_device_params - Thermal Zone device parameters
> + * @tzp:               Thermal zone platform parameters
> + * @tgp:               Thermal zone governor parameters
> + */
> +struct thermal_zone_device_params {
> +       struct thermal_zone_platform_params tzp;
> +       struct thermal_governor_params *tgp;
> +};
> +
>  /* Function declarations */
>  #ifdef CONFIG_THERMAL_OF
>  struct thermal_zone_device *devm_thermal_of_zone_register(struct device *dev, int id, void *data,
> @@ -328,6 +379,8 @@ struct thermal_zone_device *thermal_tripless_zone_device_register(
>                                         struct thermal_zone_device_ops *ops,
>                                         const struct thermal_zone_params *tzp);
>
> +struct thermal_zone_device *thermal_zone_device_register(struct thermal_zone_device_params *tzdp);
> +
>  void thermal_zone_device_unregister(struct thermal_zone_device *tz);
>
>  void *thermal_zone_device_priv(struct thermal_zone_device *tzd);
> @@ -390,6 +443,10 @@ static inline struct thermal_zone_device *thermal_tripless_zone_device_register(
>                                         const struct thermal_zone_params *tzp)
>  { return ERR_PTR(-ENODEV); }
>
> +static inline struct thermal_zone_device *thermal_zone_device_register(
> +                                       struct thermal_zone_device_params *tzdp)
> +{ return ERR_PTR(-ENODEV); }
> +
>  static inline void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>  { }
>
> --

Re: [PATCH v1 02/18] thermal: Add new structures and thermal_zone_device_register()

[Dan Carpenter]

On Thu, Feb 01, 2024 at 08:24:15PM +0100, Rafael J. Wysocki wrote:
> On Tue, Jan 30, 2024 at 12:13 PM AngeloGioacchino Del Regno
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > index 65d8f92a9a0d..7a540b746703 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -149,7 +149,8 @@ struct thermal_cooling_device {
> >                         passive trip point.
> >   * @need_update:       if equals 1, thermal_zone_device_update needs to be invoked.
> >   * @ops:       operations this &thermal_zone_device supports
> > - * @tzp:       thermal zone parameters
> > + * @tzp:               Thermal zone parameters
> > + * @tgp:               Thermal zone governor parameters
> >   * @governor:  pointer to the governor for this thermal zone
> >   * @governor_data:     private pointer for governor data
> >   * @thermal_instances: list of &struct thermal_instance of this thermal zone
> > @@ -184,7 +185,8 @@ struct thermal_zone_device {
> >         int prev_high_trip;
> >         atomic_t need_update;
> >         struct thermal_zone_device_ops *ops;
> > -       struct thermal_zone_params *tzp;
> > +       struct thermal_zone_platform_params *tzp;
> > +       struct thermal_governor_params *tgp;
> 
> I agree with doing a split here, but I'm not sure about moving items
> from the arg list to struct thermal_zone_platform_params (as mentioned
> above).
> 
> Also the naming is quite inconsistent.  IMO it would be better to call
> the first pointer "tzpp", rename struct thermal_governor_params to
> struct thermal_zone_governor_params and call the second pointer
> "tzgp".
> 

The names "tzgp" and "tzpp" look almost identical at first glance.
Could we increase the hamming distance somehow?

regards,
dan carpenter

Re: [PATCH v1 02/18] thermal: Add new structures and thermal_zone_device_register()

[Rafael J. Wysocki]

On Fri, Feb 2, 2024 at 9:47 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> On Thu, Feb 01, 2024 at 08:24:15PM +0100, Rafael J. Wysocki wrote:
> > On Tue, Jan 30, 2024 at 12:13 PM AngeloGioacchino Del Regno
> > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > > index 65d8f92a9a0d..7a540b746703 100644
> > > --- a/include/linux/thermal.h
> > > +++ b/include/linux/thermal.h
> > > @@ -149,7 +149,8 @@ struct thermal_cooling_device {
> > >                         passive trip point.
> > >   * @need_update:       if equals 1, thermal_zone_device_update needs to be invoked.
> > >   * @ops:       operations this &thermal_zone_device supports
> > > - * @tzp:       thermal zone parameters
> > > + * @tzp:               Thermal zone parameters
> > > + * @tgp:               Thermal zone governor parameters
> > >   * @governor:  pointer to the governor for this thermal zone
> > >   * @governor_data:     private pointer for governor data
> > >   * @thermal_instances: list of &struct thermal_instance of this thermal zone
> > > @@ -184,7 +185,8 @@ struct thermal_zone_device {
> > >         int prev_high_trip;
> > >         atomic_t need_update;
> > >         struct thermal_zone_device_ops *ops;
> > > -       struct thermal_zone_params *tzp;
> > > +       struct thermal_zone_platform_params *tzp;
> > > +       struct thermal_governor_params *tgp;
> >
> > I agree with doing a split here, but I'm not sure about moving items
> > from the arg list to struct thermal_zone_platform_params (as mentioned
> > above).
> >
> > Also the naming is quite inconsistent.  IMO it would be better to call
> > the first pointer "tzpp", rename struct thermal_governor_params to
> > struct thermal_zone_governor_params and call the second pointer
> > "tzgp".
> >
>
> The names "tzgp" and "tzpp" look almost identical at first glance.
> Could we increase the hamming distance somehow?

Good point.

They may as well be gov_params and platform_params AFAIAC.

Re: [PATCH v1 02/18] thermal: Add new structures and thermal_zone_device_register()

[AngeloGioacchino Del Regno]

Il 02/02/24 18:13, Rafael J. Wysocki ha scritto:
> On Fri, Feb 2, 2024 at 9:47 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>>
>> On Thu, Feb 01, 2024 at 08:24:15PM +0100, Rafael J. Wysocki wrote:
>>> On Tue, Jan 30, 2024 at 12:13 PM AngeloGioacchino Del Regno
>>>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>>>> index 65d8f92a9a0d..7a540b746703 100644
>>>> --- a/include/linux/thermal.h
>>>> +++ b/include/linux/thermal.h
>>>> @@ -149,7 +149,8 @@ struct thermal_cooling_device {
>>>>                          passive trip point.
>>>>    * @need_update:       if equals 1, thermal_zone_device_update needs to be invoked.
>>>>    * @ops:       operations this &thermal_zone_device supports
>>>> - * @tzp:       thermal zone parameters
>>>> + * @tzp:               Thermal zone parameters
>>>> + * @tgp:               Thermal zone governor parameters
>>>>    * @governor:  pointer to the governor for this thermal zone
>>>>    * @governor_data:     private pointer for governor data
>>>>    * @thermal_instances: list of &struct thermal_instance of this thermal zone
>>>> @@ -184,7 +185,8 @@ struct thermal_zone_device {
>>>>          int prev_high_trip;
>>>>          atomic_t need_update;
>>>>          struct thermal_zone_device_ops *ops;
>>>> -       struct thermal_zone_params *tzp;
>>>> +       struct thermal_zone_platform_params *tzp;
>>>> +       struct thermal_governor_params *tgp;
>>>
>>> I agree with doing a split here, but I'm not sure about moving items
>>> from the arg list to struct thermal_zone_platform_params (as mentioned
>>> above).
>>>
>>> Also the naming is quite inconsistent.  IMO it would be better to call
>>> the first pointer "tzpp", rename struct thermal_governor_params to
>>> struct thermal_zone_governor_params and call the second pointer
>>> "tzgp".
>>>
>>
>> The names "tzgp" and "tzpp" look almost identical at first glance.
>> Could we increase the hamming distance somehow?
> 
> Good point.
> 
> They may as well be gov_params and platform_params AFAIAC.

I'm more for gov_params and zone_params, as the thermal_zone_platform_params is
supposed to get renamed to struct thermal_zone_params in part 2.

Anything against that?

Cheers,
Angelo

Re: [PATCH v1 02/18] thermal: Add new structures and thermal_zone_device_register()

[Rafael J. Wysocki]

On Mon, Feb 12, 2024 at 11:41 AM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 02/02/24 18:13, Rafael J. Wysocki ha scritto:
> > On Fri, Feb 2, 2024 at 9:47 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
> >>
> >> On Thu, Feb 01, 2024 at 08:24:15PM +0100, Rafael J. Wysocki wrote:
> >>> On Tue, Jan 30, 2024 at 12:13 PM AngeloGioacchino Del Regno
> >>>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> >>>> index 65d8f92a9a0d..7a540b746703 100644
> >>>> --- a/include/linux/thermal.h
> >>>> +++ b/include/linux/thermal.h
> >>>> @@ -149,7 +149,8 @@ struct thermal_cooling_device {
> >>>>                          passive trip point.
> >>>>    * @need_update:       if equals 1, thermal_zone_device_update needs to be invoked.
> >>>>    * @ops:       operations this &thermal_zone_device supports
> >>>> - * @tzp:       thermal zone parameters
> >>>> + * @tzp:               Thermal zone parameters
> >>>> + * @tgp:               Thermal zone governor parameters
> >>>>    * @governor:  pointer to the governor for this thermal zone
> >>>>    * @governor_data:     private pointer for governor data
> >>>>    * @thermal_instances: list of &struct thermal_instance of this thermal zone
> >>>> @@ -184,7 +185,8 @@ struct thermal_zone_device {
> >>>>          int prev_high_trip;
> >>>>          atomic_t need_update;
> >>>>          struct thermal_zone_device_ops *ops;
> >>>> -       struct thermal_zone_params *tzp;
> >>>> +       struct thermal_zone_platform_params *tzp;
> >>>> +       struct thermal_governor_params *tgp;
> >>>
> >>> I agree with doing a split here, but I'm not sure about moving items
> >>> from the arg list to struct thermal_zone_platform_params (as mentioned
> >>> above).
> >>>
> >>> Also the naming is quite inconsistent.  IMO it would be better to call
> >>> the first pointer "tzpp", rename struct thermal_governor_params to
> >>> struct thermal_zone_governor_params and call the second pointer
> >>> "tzgp".
> >>>
> >>
> >> The names "tzgp" and "tzpp" look almost identical at first glance.
> >> Could we increase the hamming distance somehow?
> >
> > Good point.
> >
> > They may as well be gov_params and platform_params AFAIAC.
>
> I'm more for gov_params and zone_params, as the thermal_zone_platform_params is
> supposed to get renamed to struct thermal_zone_params in part 2.
>
> Anything against that?

Nope, sounds good!

[PATCH v1 03/18] thermal: Directly use thermal_zone_platform_params for thermal_zone_device

[AngeloGioacchino Del Regno]

Remove all duplicate members from thermal_zone_device and directly
use the corresponding ones from struct thermal_zone_platform_params.

Please note that this also changes thermal_zone_device's type member
from char[THERMAL_NAME_LENGTH] to a const char *pointer, as the only
reason for this to not be const was that thermal_core was performing
a string copy operation, which is no more.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/thermal/gov_user_space.c  |  2 +-
 drivers/thermal/thermal_core.c    | 65 ++++++++++++++-----------------
 drivers/thermal/thermal_core.h    |  6 ++-
 drivers/thermal/thermal_helpers.c | 14 +++----
 drivers/thermal/thermal_hwmon.c   |  8 ++--
 drivers/thermal/thermal_of.c      | 12 +++---
 drivers/thermal/thermal_sysfs.c   | 48 +++++++++++------------
 drivers/thermal/thermal_trace.h   |  8 ++--
 drivers/thermal/thermal_trip.c    | 14 +++----
 include/linux/thermal.h           | 10 -----
 10 files changed, 87 insertions(+), 100 deletions(-)

diff --git a/drivers/thermal/gov_user_space.c b/drivers/thermal/gov_user_space.c
index 7a1790b7e8f5..905f89f4aa4b 100644
--- a/drivers/thermal/gov_user_space.c
+++ b/drivers/thermal/gov_user_space.c
@@ -37,7 +37,7 @@ static int notify_user_space(struct thermal_zone_device *tz,
 
 	lockdep_assert_held(&tz->lock);
 
-	thermal_prop[0] = kasprintf(GFP_KERNEL, "NAME=%s", tz->type);
+	thermal_prop[0] = kasprintf(GFP_KERNEL, "NAME=%s", tz->tzp->type);
 	thermal_prop[1] = kasprintf(GFP_KERNEL, "TEMP=%d", tz->temperature);
 	thermal_prop[2] = kasprintf(GFP_KERNEL, "TRIP=%d",
 				    thermal_zone_trip_id(tz, trip));
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 3150475b947f..5694e2df158e 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -75,7 +75,7 @@ static void bind_previous_governor(struct thermal_zone_device *tz,
 		if (tz->governor->bind_to_tz(tz)) {
 			dev_err(&tz->device,
 				"governor %s failed to bind and the previous one (%s) failed to bind again, thermal zone %s has no governor\n",
-				failed_gov_name, tz->governor->name, tz->type);
+				failed_gov_name, tz->governor->name, tz->tzp->type);
 			tz->governor = NULL;
 		}
 	}
@@ -156,7 +156,7 @@ int thermal_register_governor(struct thermal_governor *governor)
 			if (ret)
 				dev_err(&pos->device,
 					"Failed to set governor %s for thermal zone %s: %d\n",
-					governor->name, pos->type, ret);
+					governor->name, pos->tzp->type, ret);
 		}
 	}
 
@@ -327,7 +327,7 @@ static void thermal_zone_device_halt(struct thermal_zone_device *tz, bool shutdo
 	int poweroff_delay_ms = CONFIG_THERMAL_EMERGENCY_POWEROFF_DELAY_MS;
 	const char *msg = "Temperature too high";
 
-	dev_emerg(&tz->device, "%s: critical temperature reached\n", tz->type);
+	dev_emerg(&tz->device, "%s: critical temperature reached\n", tz->tzp->type);
 
 	if (shutdown)
 		hw_protection_shutdown(msg, poweroff_delay_ms);
@@ -356,9 +356,9 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
 	trace_thermal_zone_trip(tz, thermal_zone_trip_id(tz, trip), trip->type);
 
 	if (trip->type == THERMAL_TRIP_CRITICAL)
-		tz->ops->critical(tz);
-	else if (tz->ops->hot)
-		tz->ops->hot(tz);
+		tz->tzp->ops->critical(tz);
+	else if (tz->tzp->ops->hot)
+		tz->tzp->ops->hot(tz);
 }
 
 static void handle_thermal_trip(struct thermal_zone_device *tz,
@@ -493,8 +493,8 @@ static int thermal_zone_device_set_mode(struct thermal_zone_device *tz,
 		return ret;
 	}
 
-	if (tz->ops->change_mode)
-		ret = tz->ops->change_mode(tz, mode);
+	if (tz->tzp->ops->change_mode)
+		ret = tz->tzp->ops->change_mode(tz, mode);
 
 	if (!ret)
 		tz->mode = mode;
@@ -764,10 +764,10 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
 				     unsigned long upper, unsigned long lower,
 				     unsigned int weight)
 {
-	if (trip_index < 0 || trip_index >= tz->num_trips)
+	if (trip_index < 0 || trip_index >= tz->tzp->num_trips)
 		return -EINVAL;
 
-	return thermal_bind_cdev_to_trip(tz, &tz->trips[trip_index], cdev,
+	return thermal_bind_cdev_to_trip(tz, &tz->tzp->trips[trip_index], cdev,
 					 upper, lower, weight);
 }
 EXPORT_SYMBOL_GPL(thermal_zone_bind_cooling_device);
@@ -823,10 +823,10 @@ int thermal_zone_unbind_cooling_device(struct thermal_zone_device *tz,
 				       int trip_index,
 				       struct thermal_cooling_device *cdev)
 {
-	if (trip_index < 0 || trip_index >= tz->num_trips)
+	if (trip_index < 0 || trip_index >= tz->tzp->num_trips)
 		return -EINVAL;
 
-	return thermal_unbind_cdev_from_trip(tz, &tz->trips[trip_index], cdev);
+	return thermal_unbind_cdev_from_trip(tz, &tz->tzp->trips[trip_index], cdev);
 }
 EXPORT_SYMBOL_GPL(thermal_zone_unbind_cooling_device);
 
@@ -858,7 +858,7 @@ void print_bind_err_msg(struct thermal_zone_device *tz,
 			struct thermal_cooling_device *cdev, int ret)
 {
 	dev_err(&tz->device, "binding zone %s with cdev %s failed:%d\n",
-		tz->type, cdev->type, ret);
+		tz->tzp->type, cdev->type, ret);
 }
 
 static void bind_cdev(struct thermal_cooling_device *cdev)
@@ -867,8 +867,8 @@ static void bind_cdev(struct thermal_cooling_device *cdev)
 	struct thermal_zone_device *pos = NULL;
 
 	list_for_each_entry(pos, &thermal_tz_list, node) {
-		if (pos->ops->bind) {
-			ret = pos->ops->bind(pos, cdev);
+		if (pos->tzp->ops->bind) {
+			ret = pos->tzp->ops->bind(pos, cdev);
 			if (ret)
 				print_bind_err_msg(pos, cdev, ret);
 		}
@@ -1184,8 +1184,8 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
 
 	/* Unbind all thermal zones associated with 'this' cdev */
 	list_for_each_entry(tz, &thermal_tz_list, node) {
-		if (tz->ops->unbind)
-			tz->ops->unbind(tz, cdev);
+		if (tz->tzp->ops->unbind)
+			tz->tzp->ops->unbind(tz, cdev);
 	}
 
 	mutex_unlock(&thermal_list_lock);
@@ -1199,13 +1199,13 @@ static void bind_tz(struct thermal_zone_device *tz)
 	int ret;
 	struct thermal_cooling_device *pos = NULL;
 
-	if (!tz->ops->bind)
+	if (!tz->tzp->ops->bind)
 		return;
 
 	mutex_lock(&thermal_list_lock);
 
 	list_for_each_entry(pos, &thermal_cdev_list, node) {
-		ret = tz->ops->bind(tz, pos);
+		ret = tz->tzp->ops->bind(tz, pos);
 		if (ret)
 			print_bind_err_msg(tz, pos, ret);
 	}
@@ -1224,17 +1224,17 @@ int thermal_zone_get_crit_temp(struct thermal_zone_device *tz, int *temp)
 {
 	int i, ret = -EINVAL;
 
-	if (tz->ops->get_crit_temp)
-		return tz->ops->get_crit_temp(tz, temp);
+	if (tz->tzp->ops->get_crit_temp)
+		return tz->tzp->ops->get_crit_temp(tz, temp);
 
-	if (!tz->trips)
+	if (!tz->tzp->trips)
 		return -EINVAL;
 
 	mutex_lock(&tz->lock);
 
-	for (i = 0; i < tz->num_trips; i++) {
-		if (tz->trips[i].type == THERMAL_TRIP_CRITICAL) {
-			*temp = tz->trips[i].temperature;
+	for (i = 0; i < tz->tzp->num_trips; i++) {
+		if (tz->tzp->trips[i].type == THERMAL_TRIP_CRITICAL) {
+			*temp = tz->tzp->trips[i].temperature;
 			ret = 0;
 			break;
 		}
@@ -1417,16 +1417,11 @@ struct thermal_zone_device *thermal_zone_device_register(struct thermal_zone_dev
 	}
 
 	tz->id = id;
-	strscpy(tz->type, tzdp->tzp.type, sizeof(tz->type));
 
 	if (!tzdp->tzp.ops->critical)
 		tzdp->tzp.ops->critical = thermal_zone_device_critical;
 
-	tz->ops = tzdp->tzp.ops;
 	tz->device.class = thermal_class;
-	tz->devdata = tzdp->tzp.devdata;
-	tz->trips = tzdp->tzp.trips;
-	tz->num_trips = tzdp->tzp.num_trips;
 
 	thermal_set_delay_jiffies(&tz->passive_delay_jiffies, tzdp->tzp.passive_delay);
 	thermal_set_delay_jiffies(&tz->polling_delay_jiffies, tzdp->tzp.polling_delay);
@@ -1509,13 +1504,13 @@ EXPORT_SYMBOL_GPL(thermal_zone_device_register);
 
 void *thermal_zone_device_priv(struct thermal_zone_device *tzd)
 {
-	return tzd->devdata;
+	return tzd->tzp->devdata;
 }
 EXPORT_SYMBOL_GPL(thermal_zone_device_priv);
 
 const char *thermal_zone_device_type(struct thermal_zone_device *tzd)
 {
-	return tzd->type;
+	return tzd->tzp->type;
 }
 EXPORT_SYMBOL_GPL(thermal_zone_device_type);
 
@@ -1561,8 +1556,8 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
 
 	/* Unbind all cdevs associated with 'this' thermal zone */
 	list_for_each_entry(cdev, &thermal_cdev_list, node)
-		if (tz->ops->unbind)
-			tz->ops->unbind(tz, cdev);
+		if (tz->tzp->ops->unbind)
+			tz->tzp->ops->unbind(tz, cdev);
 
 	mutex_unlock(&thermal_list_lock);
 
@@ -1607,7 +1602,7 @@ struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name)
 
 	mutex_lock(&thermal_list_lock);
 	list_for_each_entry(pos, &thermal_tz_list, node)
-		if (!strncasecmp(name, pos->type, THERMAL_NAME_LENGTH)) {
+		if (!strncasecmp(name, pos->tzp->type, THERMAL_NAME_LENGTH)) {
 			found++;
 			ref = pos;
 		}
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index e9c099ecdd0f..b09c23a7a764 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -120,8 +120,10 @@ void thermal_governor_update_tz(struct thermal_zone_device *tz,
 				enum thermal_notify_event reason);
 
 /* Helpers */
-#define for_each_trip(__tz, __trip)	\
-	for (__trip = __tz->trips; __trip - __tz->trips < __tz->num_trips; __trip++)
+#define for_each_trip(__tz, __trip)				\
+	for (__trip = __tz->tzp->trips;				\
+	     __trip - __tz->tzp->trips < __tz->tzp->num_trips;	\
+	     __trip++)
 
 void __thermal_zone_set_trips(struct thermal_zone_device *tz);
 int thermal_zone_trip_id(const struct thermal_zone_device *tz,
diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
index a72095cfb771..f6d55ba853b5 100644
--- a/drivers/thermal/thermal_helpers.c
+++ b/drivers/thermal/thermal_helpers.c
@@ -26,8 +26,8 @@ int get_tz_trend(struct thermal_zone_device *tz, const struct thermal_trip *trip
 {
 	enum thermal_trend trend;
 
-	if (tz->emul_temperature || !tz->ops->get_trend ||
-	    tz->ops->get_trend(tz, trip, &trend)) {
+	if (tz->emul_temperature || !tz->tzp->ops->get_trend ||
+	    tz->tzp->ops->get_trend(tz, trip, &trend)) {
 		if (tz->temperature > tz->last_temperature)
 			trend = THERMAL_TREND_RAISING;
 		else if (tz->temperature < tz->last_temperature)
@@ -50,7 +50,7 @@ get_thermal_instance(struct thermal_zone_device *tz,
 	mutex_lock(&tz->lock);
 	mutex_lock(&cdev->lock);
 
-	trip = &tz->trips[trip_index];
+	trip = &tz->tzp->trips[trip_index];
 
 	list_for_each_entry(pos, &tz->thermal_instances, tz_node) {
 		if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev) {
@@ -74,8 +74,8 @@ EXPORT_SYMBOL(get_thermal_instance);
  * When a valid thermal zone reference is passed, it will fetch its
  * temperature and fill @temp.
  *
- * Both tz and tz->ops must be valid pointers when calling this function,
- * and the tz->ops->get_temp callback must be provided.
+ * Both tz and tz->tzp->ops must be valid pointers when calling this function,
+ * and the tz->tzp->ops->get_temp callback must be provided.
  * The function must be called under tz->lock.
  *
  * Return: On success returns 0, an error code otherwise
@@ -88,7 +88,7 @@ int __thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
 
 	lockdep_assert_held(&tz->lock);
 
-	ret = tz->ops->get_temp(tz, temp);
+	ret = tz->tzp->ops->get_temp(tz, temp);
 
 	if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz->emul_temperature) {
 		for_each_trip(tz, trip) {
@@ -132,7 +132,7 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
 
 	mutex_lock(&tz->lock);
 
-	if (!tz->ops->get_temp) {
+	if (!tz->tzp->ops->get_temp) {
 		ret = -EINVAL;
 		goto unlock;
 	}
diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
index 252116f1e535..38edad3865f8 100644
--- a/drivers/thermal/thermal_hwmon.c
+++ b/drivers/thermal/thermal_hwmon.c
@@ -80,7 +80,7 @@ temp_crit_show(struct device *dev, struct device_attribute *attr, char *buf)
 
 	mutex_lock(&tz->lock);
 
-	ret = tz->ops->get_crit_temp(tz, &temperature);
+	ret = tz->tzp->ops->get_crit_temp(tz, &temperature);
 
 	mutex_unlock(&tz->lock);
 
@@ -99,7 +99,7 @@ thermal_hwmon_lookup_by_type(const struct thermal_zone_device *tz)
 
 	mutex_lock(&thermal_hwmon_list_lock);
 	list_for_each_entry(hwmon, &thermal_hwmon_list, node) {
-		strcpy(type, tz->type);
+		strcpy(type, tz->tzp->type);
 		strreplace(type, '-', '_');
 		if (!strcmp(hwmon->type, type)) {
 			mutex_unlock(&thermal_hwmon_list_lock);
@@ -132,7 +132,7 @@ thermal_hwmon_lookup_temp(const struct thermal_hwmon_device *hwmon,
 static bool thermal_zone_crit_temp_valid(struct thermal_zone_device *tz)
 {
 	int temp;
-	return tz->ops->get_crit_temp && !tz->ops->get_crit_temp(tz, &temp);
+	return tz->tzp->ops->get_crit_temp && !tz->tzp->ops->get_crit_temp(tz, &temp);
 }
 
 int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
@@ -153,7 +153,7 @@ int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
 		return -ENOMEM;
 
 	INIT_LIST_HEAD(&hwmon->tz_list);
-	strscpy(hwmon->type, tz->type, THERMAL_NAME_LENGTH);
+	strscpy(hwmon->type, tz->tzp->type, THERMAL_NAME_LENGTH);
 	strreplace(hwmon->type, '-', '_');
 	hwmon->device = hwmon_device_register_for_thermal(&tz->device,
 							  hwmon->type, hwmon);
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index 4d6c22e0ed85..9f4c2c0cd7d8 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -274,7 +274,7 @@ static struct device_node *thermal_of_zone_get_by_name(struct thermal_zone_devic
 	if (!np)
 		return ERR_PTR(-ENODEV);
 
-	tz_np = of_get_child_by_name(np, tz->type);
+	tz_np = of_get_child_by_name(np, tz->tzp->type);
 
 	of_node_put(np);
 
@@ -310,7 +310,7 @@ static int __thermal_of_unbind(struct device_node *map_np, int index, int trip_i
 
 	ret = thermal_zone_unbind_cooling_device(tz, trip_id, cdev);
 	if (ret)
-		pr_err("Failed to unbind '%s' with '%s': %d\n", tz->type, cdev->type, ret);
+		pr_err("Failed to unbind '%s' with '%s': %d\n", tz->tzp->type, cdev->type, ret);
 
 	return ret;
 }
@@ -345,7 +345,7 @@ static int __thermal_of_bind(struct device_node *map_np, int index, int trip_id,
 					       cooling_spec.args[0],
 					       weight);
 	if (ret)
-		pr_err("Failed to bind '%s' with '%s': %d\n", tz->type, cdev->type, ret);
+		pr_err("Failed to bind '%s' with '%s': %d\n", tz->tzp->type, cdev->type, ret);
 
 	return ret;
 }
@@ -438,8 +438,8 @@ static int thermal_of_unbind(struct thermal_zone_device *tz,
  */
 static void thermal_of_zone_unregister(struct thermal_zone_device *tz)
 {
-	struct thermal_trip *trips = tz->trips;
-	struct thermal_zone_device_ops *ops = tz->ops;
+	struct thermal_trip *trips = tz->tzp->trips;
+	struct thermal_zone_device_ops *ops = tz->tzp->ops;
 
 	thermal_zone_device_disable(tz);
 	thermal_zone_device_unregister(tz);
@@ -529,7 +529,7 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node *
 	ret = thermal_zone_device_enable(tz);
 	if (ret) {
 		pr_err("Failed to enabled thermal zone '%s', id=%d: %d\n",
-		       tz->type, tz->id, ret);
+		       tz->tzp->type, tz->id, ret);
 		thermal_of_zone_unregister(tz);
 		return ERR_PTR(ret);
 	}
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index 3f5f4a04a754..87a20ced7864 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -28,7 +28,7 @@ type_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
 
-	return sprintf(buf, "%s\n", tz->type);
+	return sprintf(buf, "%s\n", tz->tzp->type);
 }
 
 static ssize_t
@@ -88,7 +88,7 @@ trip_point_type_show(struct device *dev, struct device_attribute *attr,
 	if (sscanf(attr->attr.name, "trip_point_%d_type", &trip_id) != 1)
 		return -EINVAL;
 
-	switch (tz->trips[trip_id].type) {
+	switch (tz->tzp->trips[trip_id].type) {
 	case THERMAL_TRIP_CRITICAL:
 		return sprintf(buf, "critical\n");
 	case THERMAL_TRIP_HOT:
@@ -120,11 +120,11 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr,
 
 	mutex_lock(&tz->lock);
 
-	trip = &tz->trips[trip_id];
+	trip = &tz->tzp->trips[trip_id];
 
 	if (temp != trip->temperature) {
-		if (tz->ops->set_trip_temp) {
-			ret = tz->ops->set_trip_temp(tz, trip_id, temp);
+		if (tz->tzp->ops->set_trip_temp) {
+			ret = tz->tzp->ops->set_trip_temp(tz, trip_id, temp);
 			if (ret)
 				goto unlock;
 		}
@@ -150,7 +150,7 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
 	if (sscanf(attr->attr.name, "trip_point_%d_temp", &trip_id) != 1)
 		return -EINVAL;
 
-	return sprintf(buf, "%d\n", tz->trips[trip_id].temperature);
+	return sprintf(buf, "%d\n", tz->tzp->trips[trip_id].temperature);
 }
 
 static ssize_t
@@ -171,11 +171,11 @@ trip_point_hyst_store(struct device *dev, struct device_attribute *attr,
 
 	mutex_lock(&tz->lock);
 
-	trip = &tz->trips[trip_id];
+	trip = &tz->tzp->trips[trip_id];
 
 	if (hyst != trip->hysteresis) {
-		if (tz->ops->set_trip_hyst) {
-			ret = tz->ops->set_trip_hyst(tz, trip_id, hyst);
+		if (tz->tzp->ops->set_trip_hyst) {
+			ret = tz->tzp->ops->set_trip_hyst(tz, trip_id, hyst);
 			if (ret)
 				goto unlock;
 		}
@@ -201,7 +201,7 @@ trip_point_hyst_show(struct device *dev, struct device_attribute *attr,
 	if (sscanf(attr->attr.name, "trip_point_%d_hyst", &trip_id) != 1)
 		return -EINVAL;
 
-	return sprintf(buf, "%d\n", tz->trips[trip_id].hysteresis);
+	return sprintf(buf, "%d\n", tz->tzp->trips[trip_id].hysteresis);
 }
 
 static ssize_t
@@ -250,10 +250,10 @@ emul_temp_store(struct device *dev, struct device_attribute *attr,
 
 	mutex_lock(&tz->lock);
 
-	if (!tz->ops->set_emul_temp)
+	if (!tz->tzp->ops->set_emul_temp)
 		tz->emul_temperature = temperature;
 	else
-		ret = tz->ops->set_emul_temp(tz, temperature);
+		ret = tz->tzp->ops->set_emul_temp(tz, temperature);
 
 	if (!ret)
 		__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
@@ -405,22 +405,22 @@ 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->num_trips <= 0)
+	if (tz->tzp->num_trips <= 0)
 		return -EINVAL;
 
-	tz->trip_type_attrs = kcalloc(tz->num_trips, sizeof(*tz->trip_type_attrs),
+	tz->trip_type_attrs = kcalloc(tz->tzp->num_trips, sizeof(*tz->trip_type_attrs),
 				      GFP_KERNEL);
 	if (!tz->trip_type_attrs)
 		return -ENOMEM;
 
-	tz->trip_temp_attrs = kcalloc(tz->num_trips, sizeof(*tz->trip_temp_attrs),
+	tz->trip_temp_attrs = kcalloc(tz->tzp->num_trips, sizeof(*tz->trip_temp_attrs),
 				      GFP_KERNEL);
 	if (!tz->trip_temp_attrs) {
 		kfree(tz->trip_type_attrs);
 		return -ENOMEM;
 	}
 
-	tz->trip_hyst_attrs = kcalloc(tz->num_trips,
+	tz->trip_hyst_attrs = kcalloc(tz->tzp->num_trips,
 				      sizeof(*tz->trip_hyst_attrs),
 				      GFP_KERNEL);
 	if (!tz->trip_hyst_attrs) {
@@ -429,7 +429,7 @@ static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
 		return -ENOMEM;
 	}
 
-	attrs = kcalloc(tz->num_trips * 3 + 1, sizeof(*attrs), GFP_KERNEL);
+	attrs = kcalloc(tz->tzp->num_trips * 3 + 1, sizeof(*attrs), GFP_KERNEL);
 	if (!attrs) {
 		kfree(tz->trip_type_attrs);
 		kfree(tz->trip_temp_attrs);
@@ -437,7 +437,7 @@ static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
 		return -ENOMEM;
 	}
 
-	for (indx = 0; indx < tz->num_trips; indx++) {
+	for (indx = 0; indx < tz->tzp->num_trips; indx++) {
 		/* create trip type attribute */
 		snprintf(tz->trip_type_attrs[indx].name, THERMAL_NAME_LENGTH,
 			 "trip_point_%d_type", indx);
@@ -464,7 +464,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->num_trips] = &tz->trip_temp_attrs[indx].attr.attr;
+		attrs[indx + tz->tzp->num_trips] = &tz->trip_temp_attrs[indx].attr.attr;
 
 		snprintf(tz->trip_hyst_attrs[indx].name, THERMAL_NAME_LENGTH,
 			 "trip_point_%d_hyst", indx);
@@ -474,15 +474,15 @@ static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
 					tz->trip_hyst_attrs[indx].name;
 		tz->trip_hyst_attrs[indx].attr.attr.mode = S_IRUGO;
 		tz->trip_hyst_attrs[indx].attr.show = trip_point_hyst_show;
-		if (tz->ops->set_trip_hyst) {
+		if (tz->tzp->ops->set_trip_hyst) {
 			tz->trip_hyst_attrs[indx].attr.attr.mode |= S_IWUSR;
 			tz->trip_hyst_attrs[indx].attr.store =
 					trip_point_hyst_store;
 		}
-		attrs[indx + tz->num_trips * 2] =
+		attrs[indx + tz->tzp->num_trips * 2] =
 					&tz->trip_hyst_attrs[indx].attr.attr;
 	}
-	attrs[tz->num_trips * 3] = NULL;
+	attrs[tz->tzp->num_trips * 3] = NULL;
 
 	tz->trips_attribute_group.attrs = attrs;
 
@@ -522,7 +522,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->num_trips) {
+	if (tz->tzp->num_trips) {
 		result = create_trip_attrs(tz, mask);
 		if (result) {
 			kfree(groups);
@@ -543,7 +543,7 @@ void thermal_zone_destroy_device_groups(struct thermal_zone_device *tz)
 	if (!tz)
 		return;
 
-	if (tz->num_trips)
+	if (tz->tzp->num_trips)
 		destroy_trip_attrs(tz);
 
 	kfree(tz->device.groups);
diff --git a/drivers/thermal/thermal_trace.h b/drivers/thermal/thermal_trace.h
index 459c8ce6cf3b..4bd8b0be4280 100644
--- a/drivers/thermal/thermal_trace.h
+++ b/drivers/thermal/thermal_trace.h
@@ -28,14 +28,14 @@ TRACE_EVENT(thermal_temperature,
 	TP_ARGS(tz),
 
 	TP_STRUCT__entry(
-		__string(thermal_zone, tz->type)
+		__string(thermal_zone, tz->tzp->type)
 		__field(int, id)
 		__field(int, temp_prev)
 		__field(int, temp)
 	),
 
 	TP_fast_assign(
-		__assign_str(thermal_zone, tz->type);
+		__assign_str(thermal_zone, tz->tzp->type);
 		__entry->id = tz->id;
 		__entry->temp_prev = tz->last_temperature;
 		__entry->temp = tz->temperature;
@@ -73,14 +73,14 @@ TRACE_EVENT(thermal_zone_trip,
 	TP_ARGS(tz, trip, trip_type),
 
 	TP_STRUCT__entry(
-		__string(thermal_zone, tz->type)
+		__string(thermal_zone, tz->tzp->type)
 		__field(int, id)
 		__field(int, trip)
 		__field(enum thermal_trip_type, trip_type)
 	),
 
 	TP_fast_assign(
-		__assign_str(thermal_zone, tz->type);
+		__assign_str(thermal_zone, tz->tzp->type);
 		__entry->id = tz->id;
 		__entry->trip = trip;
 		__entry->trip_type = trip_type;
diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c
index c875a26d5adf..9cccda973756 100644
--- a/drivers/thermal/thermal_trip.c
+++ b/drivers/thermal/thermal_trip.c
@@ -42,7 +42,7 @@ EXPORT_SYMBOL_GPL(thermal_zone_for_each_trip);
 
 int thermal_zone_get_num_trips(struct thermal_zone_device *tz)
 {
-	return tz->num_trips;
+	return tz->tzp->num_trips;
 }
 EXPORT_SYMBOL_GPL(thermal_zone_get_num_trips);
 
@@ -56,7 +56,7 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_num_trips);
  * driver to let it set its own notification mechanism (usually an
  * interrupt).
  *
- * This function must be called with tz->lock held. Both tz and tz->ops
+ * This function must be called with tz->lock held. Both tz and tz->tzp->ops
  * must be valid pointers.
  *
  * It does not return a value
@@ -70,7 +70,7 @@ void __thermal_zone_set_trips(struct thermal_zone_device *tz)
 
 	lockdep_assert_held(&tz->lock);
 
-	if (!tz->ops->set_trips)
+	if (!tz->tzp->ops->set_trips)
 		return;
 
 	for_each_trip(tz, trip) {
@@ -114,7 +114,7 @@ void __thermal_zone_set_trips(struct thermal_zone_device *tz)
 	 * Set a temperature window. When this window is left the driver
 	 * must inform the thermal core via thermal_zone_device_update.
 	 */
-	ret = tz->ops->set_trips(tz, low, high);
+	ret = tz->tzp->ops->set_trips(tz, low, high);
 	if (ret)
 		dev_err(&tz->device, "Failed to set trips: %d\n", ret);
 }
@@ -122,10 +122,10 @@ void __thermal_zone_set_trips(struct thermal_zone_device *tz)
 int __thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id,
 			    struct thermal_trip *trip)
 {
-	if (!tz || !tz->trips || trip_id < 0 || trip_id >= tz->num_trips || !trip)
+	if (!tz || !tz->tzp->trips || trip_id < 0 || trip_id >= tz->tzp->num_trips || !trip)
 		return -EINVAL;
 
-	*trip = tz->trips[trip_id];
+	*trip = tz->tzp->trips[trip_id];
 	return 0;
 }
 EXPORT_SYMBOL_GPL(__thermal_zone_get_trip);
@@ -150,7 +150,7 @@ int thermal_zone_trip_id(const struct thermal_zone_device *tz,
 	 * Assume the trip to be located within the bounds of the thermal
 	 * zone's trips[] table.
 	 */
-	return trip - tz->trips;
+	return trip - tz->tzp->trips;
 }
 void thermal_zone_trip_updated(struct thermal_zone_device *tz,
 			       const struct thermal_trip *trip)
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 7a540b746703..65db38acd20b 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -122,16 +122,12 @@ struct thermal_cooling_device {
 /**
  * struct thermal_zone_device - structure for a thermal zone
  * @id:		unique id number for each thermal zone
- * @type:	the thermal zone device type
  * @device:	&struct device for this thermal zone
  * @removal:	removal completion
  * @trip_temp_attrs:	attributes for trip points for sysfs: trip temperature
  * @trip_type_attrs:	attributes for trip points for sysfs: trip type
  * @trip_hyst_attrs:	attributes for trip points for sysfs: trip hysteresis
  * @mode:		current mode of this thermal zone
- * @devdata:	private pointer for device private data
- * @trips:	an array of struct thermal_trip
- * @num_trips:	number of trip points the thermal zone supports
  * @passive_delay_jiffies: number of jiffies to wait between polls when
  *			performing passive cooling.
  * @polling_delay_jiffies: number of jiffies to wait between polls when
@@ -148,7 +144,6 @@ struct thermal_cooling_device {
  * @prev_high_trip:	the above current temperature if you've crossed a
 			passive trip point.
  * @need_update:	if equals 1, thermal_zone_device_update needs to be invoked.
- * @ops:	operations this &thermal_zone_device supports
  * @tzp:		Thermal zone parameters
  * @tgp:		Thermal zone governor parameters
  * @governor:	pointer to the governor for this thermal zone
@@ -164,7 +159,6 @@ struct thermal_cooling_device {
  */
 struct thermal_zone_device {
 	int id;
-	char type[THERMAL_NAME_LENGTH];
 	struct device device;
 	struct completion removal;
 	struct attribute_group trips_attribute_group;
@@ -172,9 +166,6 @@ struct thermal_zone_device {
 	struct thermal_attr *trip_type_attrs;
 	struct thermal_attr *trip_hyst_attrs;
 	enum thermal_device_mode mode;
-	void *devdata;
-	struct thermal_trip *trips;
-	int num_trips;
 	unsigned long passive_delay_jiffies;
 	unsigned long polling_delay_jiffies;
 	int temperature;
@@ -184,7 +175,6 @@ struct thermal_zone_device {
 	int prev_low_trip;
 	int prev_high_trip;
 	atomic_t need_update;
-	struct thermal_zone_device_ops *ops;
 	struct thermal_zone_platform_params *tzp;
 	struct thermal_governor_params *tgp;
 	struct thermal_governor *governor;
-- 
2.43.0

Re: [PATCH v1 03/18] thermal: Directly use thermal_zone_platform_params for thermal_zone_device

[Rafael J. Wysocki]

On Tue, Jan 30, 2024 at 12:13 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Remove all duplicate members from thermal_zone_device and directly
> use the corresponding ones from struct thermal_zone_platform_params.

I totally disagree with this approach.

It does make sense to store these things in a tz directly.

1. devdata allows the caller of the thermal zone to get to their own
information on it conveniently.  This patch makes it harder.
2. It makes sense to copy type into tz to allow the zone creator to
free the string (should they want to do that).
3. It would make sense to copy the contents of the trips[] table to tz
to allow the zone creator to free their initial table.  This doesn't
happen today, but it is in the works and your patch goes against this.
4. It makes sense to copy num_trips directly to tz, because it is
closely related to trips[].
5. It makes sense to copy ops directly into tz, because this will
allow zone creators to use const ops for zone registration.  Again,
this doesn't happen today, but it is in the works and this patch goes
against this.

As far as I'm concerned, this one is a total no-go.

Thanks!

Re: [PATCH v1 03/18] thermal: Directly use thermal_zone_platform_params for thermal_zone_device

[AngeloGioacchino Del Regno]

Il 01/02/24 20:42, Rafael J. Wysocki ha scritto:
> On Tue, Jan 30, 2024 at 12:13 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Remove all duplicate members from thermal_zone_device and directly
>> use the corresponding ones from struct thermal_zone_platform_params.
> 
> I totally disagree with this approach.
> 
> It does make sense to store these things in a tz directly.
> 
> 1. devdata allows the caller of the thermal zone to get to their own
> information on it conveniently.  This patch makes it harder.
> 2. It makes sense to copy type into tz to allow the zone creator to
> free the string (should they want to do that).
> 3. It would make sense to copy the contents of the trips[] table to tz
> to allow the zone creator to free their initial table.  This doesn't
> happen today, but it is in the works and your patch goes against this.
> 4. It makes sense to copy num_trips directly to tz, because it is
> closely related to trips[].
> 5. It makes sense to copy ops directly into tz, because this will
> allow zone creators to use const ops for zone registration.  Again,
> this doesn't happen today, but it is in the works and this patch goes
> against this.
> 
> As far as I'm concerned, this one is a total no-go.
> 

I'm sad for all the work that went into this - but at the same time I can also
understand your reasons.

I'll drop this one then!

Cheers,
Angelo

> Thanks!

[PATCH v1 04/18] thermal/drivers/da9062: Migrate to thermal_zone_device_register()

[AngeloGioacchino Del Regno]

The thermal API has a new thermal_zone_device_register() function which
is deprecating the older thermal_zone_device_register_with_trips() and
thermal_tripless_zone_device_register().

Migrate to the new thermal zone device registration function.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/thermal/da9062-thermal.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-thermal.c
index 160d64913057..13c7a6bc36c4 100644
--- a/drivers/thermal/da9062-thermal.c
+++ b/drivers/thermal/da9062-thermal.c
@@ -158,6 +158,13 @@ MODULE_DEVICE_TABLE(of, da9062_compatible_reg_id_table);
 
 static int da9062_thermal_probe(struct platform_device *pdev)
 {
+	struct thermal_zone_device_params tzdp = {
+		.tzp = {
+			.ops = &da9062_thermal_ops,
+			.trips = trips,
+			.num_trips = ARRAY_SIZE(trips),
+		}
+	};
 	struct da9062 *chip = dev_get_drvdata(pdev->dev.parent);
 	struct da9062_thermal *thermal;
 	const struct of_device_id *match;
@@ -196,10 +203,11 @@ static int da9062_thermal_probe(struct platform_device *pdev)
 	INIT_DELAYED_WORK(&thermal->work, da9062_thermal_poll_on);
 	mutex_init(&thermal->lock);
 
-	thermal->zone = thermal_zone_device_register_with_trips(thermal->config->name,
-								trips, ARRAY_SIZE(trips), 0, thermal,
-								&da9062_thermal_ops, NULL, pp_tmp,
-								0);
+	tzdp.tzp.type = thermal->config->name;
+	tzdp.tzp.devdata = thermal;
+	tzdp.tzp.passive_delay = pp_tmp;
+
+	thermal->zone = thermal_zone_device_register(&tzdp);
 	if (IS_ERR(thermal->zone)) {
 		dev_err(&pdev->dev, "Cannot register thermal zone device\n");
 		ret = PTR_ERR(thermal->zone);
-- 
2.43.0

[PATCH v1 05/18] thermal/drivers/imx: Migrate to thermal_zone_device_register()

[AngeloGioacchino Del Regno]

The thermal API has a new thermal_zone_device_register() function which
is deprecating the older thermal_zone_device_register_with_trips() and
thermal_tripless_zone_device_register().

Migrate to the new thermal zone device registration function.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/thermal/imx_thermal.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 7019c4fdd549..16fb49194fb3 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -599,6 +599,15 @@ static inline void imx_thermal_unregister_legacy_cooling(struct imx_thermal_data
 
 static int imx_thermal_probe(struct platform_device *pdev)
 {
+	struct thermal_zone_device_params tzdp = {
+		.tzp = {
+			.type = "imx_thermal_zone",
+			.ops = &imx_tz_ops,
+			.mask = BIT(IMX_TRIP_PASSIVE),
+			.passive_delay = IMX_PASSIVE_DELAY,
+			.polling_delay = IMX_POLLING_DELAY,
+		}
+	};
 	struct imx_thermal_data *data;
 	struct regmap *map;
 	int measure_freq;
@@ -696,13 +705,11 @@ static int imx_thermal_probe(struct platform_device *pdev)
 		goto legacy_cleanup;
 	}
 
-	data->tz = thermal_zone_device_register_with_trips("imx_thermal_zone",
-							   trips,
-							   ARRAY_SIZE(trips),
-							   BIT(IMX_TRIP_PASSIVE), data,
-							   &imx_tz_ops, NULL,
-							   IMX_PASSIVE_DELAY,
-							   IMX_POLLING_DELAY);
+	tzdp.tzp.devdata = data;
+	tzdp.tzp.trips = &trips;
+	tzdp.tzp.num_trips = ARRAY_SIZE(trips);
+
+	data->tz = thermal_zone_device_register(&tzdp);
 	if (IS_ERR(data->tz)) {
 		ret = PTR_ERR(data->tz);
 		dev_err(&pdev->dev,
-- 
2.43.0

[PATCH v1 06/18] thermal/drivers/rcar: Migrate to thermal_zone_device_register()

[AngeloGioacchino Del Regno]

The thermal API has a new thermal_zone_device_register() function which
is deprecating the older thermal_zone_device_register_with_trips() and
thermal_tripless_zone_device_register().

Migrate to the new thermal zone device registration function.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/thermal/rcar_thermal.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index feb848d595fa..7bf9c1611a00 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -488,10 +488,17 @@ static int rcar_thermal_probe(struct platform_device *pdev)
 						dev, i, priv,
 						&rcar_thermal_zone_ops);
 		} else {
-			priv->zone = thermal_zone_device_register_with_trips(
-				"rcar_thermal", trips, ARRAY_SIZE(trips), 0, priv,
-						&rcar_thermal_zone_ops, NULL, 0,
-						idle);
+			struct thermal_zone_device_params tzdp = {
+				.tzp = {
+					.type = "rcar_thermal",
+					.ops = &rcar_thermal_zone_ops,
+					.devdata = priv,
+					.trips = trips,
+					.num_trips = ARRAY_SIZE(trips),
+					.polling_delay = idle,
+				}
+			};
+			priv->zone = thermal_zone_device_register(&tzdp);
 
 			ret = thermal_zone_device_enable(priv->zone);
 			if (ret) {
-- 
2.43.0

Re: [PATCH v1 06/18] thermal/drivers/rcar: Migrate to thermal_zone_device_register()

[Geert Uytterhoeven]

On Tue, Jan 30, 2024 at 12:15 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
> The thermal API has a new thermal_zone_device_register() function which
> is deprecating the older thermal_zone_device_register_with_trips() and
> thermal_tripless_zone_device_register().
>
> Migrate to the new thermal zone device registration function.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v1 06/18] thermal/drivers/rcar: Migrate to thermal_zone_device_register()

[Niklas Söderlund]

Hi AngeloGioacchino,

Thanks for your work.

On 2024-01-30 12:12:38 +0100, AngeloGioacchino Del Regno wrote:
> The thermal API has a new thermal_zone_device_register() function which
> is deprecating the older thermal_zone_device_register_with_trips() and
> thermal_tripless_zone_device_register().
> 
> Migrate to the new thermal zone device registration function.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/thermal/rcar_thermal.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> index feb848d595fa..7bf9c1611a00 100644
> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
> @@ -488,10 +488,17 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>  						dev, i, priv,
>  						&rcar_thermal_zone_ops);
>  		} else {
> -			priv->zone = thermal_zone_device_register_with_trips(
> -				"rcar_thermal", trips, ARRAY_SIZE(trips), 0, priv,
> -						&rcar_thermal_zone_ops, NULL, 0,
> -						idle);
> +			struct thermal_zone_device_params tzdp = {
> +				.tzp = {
> +					.type = "rcar_thermal",
> +					.ops = &rcar_thermal_zone_ops,
> +					.devdata = priv,
> +					.trips = trips,
> +					.num_trips = ARRAY_SIZE(trips),
> +					.polling_delay = idle,
> +				}
> +			};
> +			priv->zone = thermal_zone_device_register(&tzdp);
>  
>  			ret = thermal_zone_device_enable(priv->zone);
>  			if (ret) {
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund

[PATCH v1 07/18] thermal/drivers/st: Migrate to thermal_zone_device_register()

[AngeloGioacchino Del Regno]

The thermal API has a new thermal_zone_device_register() function which
is deprecating the older thermal_zone_device_register_with_trips() and
thermal_tripless_zone_device_register().

Migrate to the new thermal zone device registration function.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/thermal/st/st_thermal.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/st/st_thermal.c b/drivers/thermal/st/st_thermal.c
index 0d6249b36609..2f7b22764bd0 100644
--- a/drivers/thermal/st/st_thermal.c
+++ b/drivers/thermal/st/st_thermal.c
@@ -143,9 +143,9 @@ int st_thermal_register(struct platform_device *pdev,
 	struct st_thermal_sensor *sensor;
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
+	struct thermal_zone_device_params tzdp;
 	const struct of_device_id *match;
 
-	int polling_delay;
 	int ret;
 
 	if (!np) {
@@ -197,14 +197,17 @@ int st_thermal_register(struct platform_device *pdev,
 	if (ret)
 		goto sensor_off;
 
-	polling_delay = sensor->ops->register_enable_irq ? 0 : 1000;
-
 	trip.temperature = sensor->cdata->crit_temp;
 	trip.type = THERMAL_TRIP_CRITICAL;
 
-	sensor->thermal_dev =
-		thermal_zone_device_register_with_trips(dev_name(dev), &trip, 1, 0, sensor,
-							&st_tz_ops, NULL, 0, polling_delay);
+	tzdp.tzp.type = dev_name(dev);
+	tzdp.tzp.ops = &st_tz_ops;
+	tzdp.tzp.devdata = sensor;
+	tzdp.tzp.trips = &trip;
+	tzdp.tzp.num_trips = 1;
+	tzdp.tzp.polling_delay = sensor->ops->register_enable_irq ? 0 : 1000;
+
+	sensor->thermal_dev = thermal_zone_device_register(&tzdp);
 	if (IS_ERR(sensor->thermal_dev)) {
 		dev_err(dev, "failed to register thermal zone device\n");
 		ret = PTR_ERR(sensor->thermal_dev);
-- 
2.43.0

[PATCH v1 08/18] thermal: intel: pch_thermal: Migrate to thermal_zone_device_register()

[AngeloGioacchino Del Regno]

The thermal API has a new thermal_zone_device_register() function which
is deprecating the older thermal_zone_device_register_with_trips() and
thermal_tripless_zone_device_register().

Migrate to the new thermal zone device registration function.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/thermal/intel/intel_pch_thermal.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/intel/intel_pch_thermal.c b/drivers/thermal/intel/intel_pch_thermal.c
index b3905e34c507..73d7c2ac7dbc 100644
--- a/drivers/thermal/intel/intel_pch_thermal.c
+++ b/drivers/thermal/intel/intel_pch_thermal.c
@@ -160,6 +160,7 @@ static int intel_pch_thermal_probe(struct pci_dev *pdev,
 				   const struct pci_device_id *id)
 {
 	enum pch_board_ids board_id = id->driver_data;
+	struct thermal_zone_device_params tzdp;
 	struct pch_thermal_device *ptd;
 	int nr_trips = 0;
 	u16 trip_temp;
@@ -233,10 +234,13 @@ static int intel_pch_thermal_probe(struct pci_dev *pdev,
 
 	nr_trips += pch_wpt_add_acpi_psv_trip(ptd, nr_trips);
 
-	ptd->tzd = thermal_zone_device_register_with_trips(board_names[board_id],
-							   ptd->trips, nr_trips,
-							   0, ptd, &tzd_ops,
-							   NULL, 0, 0);
+	tzdp.tzp.type = board_names[board_id];
+	tzdp.tzp.devdata = ptd;
+	tzdp.tzp.trips = ptd->trips;
+	tzdp.tzp.num_trips = nr_trips;
+	tzdp.tzp.ops = &tzd_ops;
+
+	ptd->tzd = thermal_zone_device_register(&tzdp);
 	if (IS_ERR(ptd->tzd)) {
 		dev_err(&pdev->dev, "Failed to register thermal zone %s\n",
 			board_names[board_id]);
-- 
2.43.0

Re: [PATCH v1 08/18] thermal: intel: pch_thermal: Migrate to thermal_zone_device_register()

[Rafael J. Wysocki]

On Tue, Jan 30, 2024 at 12:13 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> The thermal API has a new thermal_zone_device_register() function which
> is deprecating the older thermal_zone_device_register_with_trips() and
> thermal_tripless_zone_device_register().
>
> Migrate to the new thermal zone device registration function.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/thermal/intel/intel_pch_thermal.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/thermal/intel/intel_pch_thermal.c b/drivers/thermal/intel/intel_pch_thermal.c
> index b3905e34c507..73d7c2ac7dbc 100644
> --- a/drivers/thermal/intel/intel_pch_thermal.c
> +++ b/drivers/thermal/intel/intel_pch_thermal.c
> @@ -160,6 +160,7 @@ static int intel_pch_thermal_probe(struct pci_dev *pdev,
>                                    const struct pci_device_id *id)
>  {
>         enum pch_board_ids board_id = id->driver_data;
> +       struct thermal_zone_device_params tzdp;
>         struct pch_thermal_device *ptd;
>         int nr_trips = 0;
>         u16 trip_temp;
> @@ -233,10 +234,13 @@ static int intel_pch_thermal_probe(struct pci_dev *pdev,
>
>         nr_trips += pch_wpt_add_acpi_psv_trip(ptd, nr_trips);
>
> -       ptd->tzd = thermal_zone_device_register_with_trips(board_names[board_id],
> -                                                          ptd->trips, nr_trips,
> -                                                          0, ptd, &tzd_ops,
> -                                                          NULL, 0, 0);
> +       tzdp.tzp.type = board_names[board_id];
> +       tzdp.tzp.devdata = ptd;
> +       tzdp.tzp.trips = ptd->trips;
> +       tzdp.tzp.num_trips = nr_trips;
> +       tzdp.tzp.ops = &tzd_ops;
> +
> +       ptd->tzd = thermal_zone_device_register(&tzdp);

IMV, this should be

ptd->tzd = thermal_zone_device_register(board_names[board_id],
ptd->trips, nr_trips, &tzd_ops, ptd, NULL);

and the tzdp variable is not necessary even.

Analogously in the rest of the series (ie. patches [4-18/18]).

>         if (IS_ERR(ptd->tzd)) {
>                 dev_err(&pdev->dev, "Failed to register thermal zone %s\n",
>                         board_names[board_id]);
> --

Re: [PATCH v1 08/18] thermal: intel: pch_thermal: Migrate to thermal_zone_device_register()

[AngeloGioacchino Del Regno]

Il 01/02/24 20:51, Rafael J. Wysocki ha scritto:
> On Tue, Jan 30, 2024 at 12:13 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> The thermal API has a new thermal_zone_device_register() function which
>> is deprecating the older thermal_zone_device_register_with_trips() and
>> thermal_tripless_zone_device_register().
>>
>> Migrate to the new thermal zone device registration function.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   drivers/thermal/intel/intel_pch_thermal.c | 12 ++++++++----
>>   1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/thermal/intel/intel_pch_thermal.c b/drivers/thermal/intel/intel_pch_thermal.c
>> index b3905e34c507..73d7c2ac7dbc 100644
>> --- a/drivers/thermal/intel/intel_pch_thermal.c
>> +++ b/drivers/thermal/intel/intel_pch_thermal.c
>> @@ -160,6 +160,7 @@ static int intel_pch_thermal_probe(struct pci_dev *pdev,
>>                                     const struct pci_device_id *id)
>>   {
>>          enum pch_board_ids board_id = id->driver_data;
>> +       struct thermal_zone_device_params tzdp;
>>          struct pch_thermal_device *ptd;
>>          int nr_trips = 0;
>>          u16 trip_temp;
>> @@ -233,10 +234,13 @@ static int intel_pch_thermal_probe(struct pci_dev *pdev,
>>
>>          nr_trips += pch_wpt_add_acpi_psv_trip(ptd, nr_trips);
>>
>> -       ptd->tzd = thermal_zone_device_register_with_trips(board_names[board_id],
>> -                                                          ptd->trips, nr_trips,
>> -                                                          0, ptd, &tzd_ops,
>> -                                                          NULL, 0, 0);
>> +       tzdp.tzp.type = board_names[board_id];
>> +       tzdp.tzp.devdata = ptd;
>> +       tzdp.tzp.trips = ptd->trips;
>> +       tzdp.tzp.num_trips = nr_trips;
>> +       tzdp.tzp.ops = &tzd_ops;
>> +
>> +       ptd->tzd = thermal_zone_device_register(&tzdp);
> 
> IMV, this should be
> 
> ptd->tzd = thermal_zone_device_register(board_names[board_id],
> ptd->trips, nr_trips, &tzd_ops, ptd, NULL);
> 
> and the tzdp variable is not necessary even.
> 

The whole point of thermal_zone_device_register() taking just one parameter was
that those older functions were taking a bit too many params, and with the
introduction of Thermal Zone name we'd be adding even more.

For intel_pch_thermal, things are more or less the same, assignments are done there
line by line... but for most of the others, IMO it's easier and schematized as a
single stack-initialized structure that could even be constified in the future.

> Analogously in the rest of the series (ie. patches [4-18/18]).
> 
>>          if (IS_ERR(ptd->tzd)) {
>>                  dev_err(&pdev->dev, "Failed to register thermal zone %s\n",
>>                          board_names[board_id]);
>> --

Re: [PATCH v1 08/18] thermal: intel: pch_thermal: Migrate to thermal_zone_device_register()

[Rafael J. Wysocki]

On Fri, Feb 2, 2024 at 11:10 AM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 01/02/24 20:51, Rafael J. Wysocki ha scritto:
> > On Tue, Jan 30, 2024 at 12:13 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> >>
> >> The thermal API has a new thermal_zone_device_register() function which
> >> is deprecating the older thermal_zone_device_register_with_trips() and
> >> thermal_tripless_zone_device_register().
> >>
> >> Migrate to the new thermal zone device registration function.
> >>
> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >> ---
> >>   drivers/thermal/intel/intel_pch_thermal.c | 12 ++++++++----
> >>   1 file changed, 8 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/thermal/intel/intel_pch_thermal.c b/drivers/thermal/intel/intel_pch_thermal.c
> >> index b3905e34c507..73d7c2ac7dbc 100644
> >> --- a/drivers/thermal/intel/intel_pch_thermal.c
> >> +++ b/drivers/thermal/intel/intel_pch_thermal.c
> >> @@ -160,6 +160,7 @@ static int intel_pch_thermal_probe(struct pci_dev *pdev,
> >>                                     const struct pci_device_id *id)
> >>   {
> >>          enum pch_board_ids board_id = id->driver_data;
> >> +       struct thermal_zone_device_params tzdp;
> >>          struct pch_thermal_device *ptd;
> >>          int nr_trips = 0;
> >>          u16 trip_temp;
> >> @@ -233,10 +234,13 @@ static int intel_pch_thermal_probe(struct pci_dev *pdev,
> >>
> >>          nr_trips += pch_wpt_add_acpi_psv_trip(ptd, nr_trips);
> >>
> >> -       ptd->tzd = thermal_zone_device_register_with_trips(board_names[board_id],
> >> -                                                          ptd->trips, nr_trips,
> >> -                                                          0, ptd, &tzd_ops,
> >> -                                                          NULL, 0, 0);
> >> +       tzdp.tzp.type = board_names[board_id];
> >> +       tzdp.tzp.devdata = ptd;
> >> +       tzdp.tzp.trips = ptd->trips;
> >> +       tzdp.tzp.num_trips = nr_trips;
> >> +       tzdp.tzp.ops = &tzd_ops;
> >> +
> >> +       ptd->tzd = thermal_zone_device_register(&tzdp);
> >
> > IMV, this should be
> >
> > ptd->tzd = thermal_zone_device_register(board_names[board_id],
> > ptd->trips, nr_trips, &tzd_ops, ptd, NULL);
> >
> > and the tzdp variable is not necessary even.
> >
>
> The whole point of thermal_zone_device_register() taking just one parameter was
> that those older functions were taking a bit too many params, and with the
> introduction of Thermal Zone name we'd be adding even more.

That's fair.

However, as indicated elsewhere, there are at least a few arguments of
the registration function that fit into the argument list: trips[],
num_trips, ops, devdata.

I'd add the name to that list and put the rest (including type) into
the params struct.

> For intel_pch_thermal, things are more or less the same, assignments are done there
> line by line... but for most of the others, IMO it's easier and schematized as a
> single stack-initialized structure that could even be constified in the future.

Well, it's copied into the struct thermal_zone_device, so it's better
to put it one the stack, so the memory occupied by it gets freed when
not needed any more.

[PATCH v1 09/18] thermal: intel: quark_dts: Migrate to thermal_zone_device_register()

[AngeloGioacchino Del Regno]

The thermal API has a new thermal_zone_device_register() function which
is deprecating the older thermal_zone_device_register_with_trips() and
thermal_tripless_zone_device_register().

Migrate to the new thermal zone device registration function.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../thermal/intel/intel_quark_dts_thermal.c   | 23 +++++++++++--------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/thermal/intel/intel_quark_dts_thermal.c b/drivers/thermal/intel/intel_quark_dts_thermal.c
index 646ca8bd40a9..381a806d5f62 100644
--- a/drivers/thermal/intel/intel_quark_dts_thermal.c
+++ b/drivers/thermal/intel/intel_quark_dts_thermal.c
@@ -320,10 +320,16 @@ static void free_soc_dts(struct soc_sensor_entry *aux_entry)
 
 static struct soc_sensor_entry *alloc_soc_dts(void)
 {
+	struct thermal_zone_device_params tzdp = {
+		.tzp = {
+			.type = "quark_dts",
+			.ops = &tzone_ops,
+			.polling_delay = polling_delay,
+		}
+	};
 	struct soc_sensor_entry *aux_entry;
 	int err;
 	u32 out;
-	int wr_mask;
 
 	aux_entry = kzalloc(sizeof(*aux_entry), GFP_KERNEL);
 	if (!aux_entry) {
@@ -339,10 +345,10 @@ static struct soc_sensor_entry *alloc_soc_dts(void)
 
 	if (out & QRK_DTS_LOCK_BIT) {
 		aux_entry->locked = true;
-		wr_mask = QRK_DTS_WR_MASK_CLR;
+		tzdp.tzp.mask = QRK_DTS_WR_MASK_CLR;
 	} else {
 		aux_entry->locked = false;
-		wr_mask = QRK_DTS_WR_MASK_SET;
+		tzdp.tzp.mask = QRK_DTS_WR_MASK_SET;
 	}
 
 	/* Store DTS default state if DTS registers are not locked */
@@ -368,12 +374,11 @@ static struct soc_sensor_entry *alloc_soc_dts(void)
 	aux_entry->trips[QRK_DTS_ID_TP_HOT].temperature = get_trip_temp(QRK_DTS_ID_TP_HOT);
 	aux_entry->trips[QRK_DTS_ID_TP_HOT].type = THERMAL_TRIP_HOT;
 
-	aux_entry->tzone = thermal_zone_device_register_with_trips("quark_dts",
-								   aux_entry->trips,
-								   QRK_MAX_DTS_TRIPS,
-								   wr_mask,
-								   aux_entry, &tzone_ops,
-								   NULL, 0, polling_delay);
+	tzdp.tzp.devdata = aux_entry;
+	tzdp.tzp.trips = aux_entry->trips;
+	tzdp.tzp.num_trips = QRK_MAX_DTS_TRIPS;
+
+	aux_entry->tzone = thermal_zone_device_register(&tzdp);
 	if (IS_ERR(aux_entry->tzone)) {
 		err = PTR_ERR(aux_entry->tzone);
 		goto err_ret;
-- 
2.43.0

[PATCH v1 10/18] thermal: intel: soc_dts_iosf: Migrate to thermal_zone_device_register()

[AngeloGioacchino Del Regno]

The thermal API has a new thermal_zone_device_register() function which
is deprecating the older thermal_zone_device_register_with_trips() and
thermal_tripless_zone_device_register().

Migrate to the new thermal zone device registration function.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/thermal/intel/intel_soc_dts_iosf.c | 24 +++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/intel/intel_soc_dts_iosf.c b/drivers/thermal/intel/intel_soc_dts_iosf.c
index d00def3c4703..78dceb117ed8 100644
--- a/drivers/thermal/intel/intel_soc_dts_iosf.c
+++ b/drivers/thermal/intel/intel_soc_dts_iosf.c
@@ -220,8 +220,15 @@ static void remove_dts_thermal_zone(struct intel_soc_dts_sensor_entry *dts)
 static int add_dts_thermal_zone(int id, struct intel_soc_dts_sensor_entry *dts,
 				bool critical_trip)
 {
+	struct thermal_zone_device_params tzdp = {
+		.tzp = {
+			.devdata = dts,
+			.ops = &tzone_ops,
+			.trips = dts->trips,
+			.num_trips = SOC_MAX_DTS_TRIPS,
+		}
+	};
 	int writable_trip_cnt = SOC_MAX_DTS_TRIPS;
-	char name[10];
 	unsigned long trip;
 	int trip_mask;
 	unsigned long ptps;
@@ -253,12 +260,15 @@ static int add_dts_thermal_zone(int id, struct intel_soc_dts_sensor_entry *dts,
 			trip_mask &= ~BIT(i / 8);
 	}
 	dts->trip_mask = trip_mask;
-	snprintf(name, sizeof(name), "soc_dts%d", id);
-	dts->tzone = thermal_zone_device_register_with_trips(name, dts->trips,
-							     SOC_MAX_DTS_TRIPS,
-							     trip_mask,
-							     dts, &tzone_ops,
-							     NULL, 0, 0);
+
+	tzdp.tzp.type = kasprintf(GFP_KERNEL, "soc_dts%d", id);
+	if (!tzdp.tzp.type)
+		return -ENOMEM;
+
+	tzdp.tzp.mask = trip_mask;
+
+	dts->tzone = thermal_zone_device_register(&tzdp);
+	kfree(tzdp.tzp.type);
 	if (IS_ERR(dts->tzone)) {
 		ret = PTR_ERR(dts->tzone);
 		goto err_ret;
-- 
2.43.0

[PATCH v1 11/18] thermal: intel: int340x: Migrate to thermal_zone_device_register()

[AngeloGioacchino Del Regno]

The thermal API has a new thermal_zone_device_register() function which
is deprecating the older thermal_zone_device_register_with_trips() and
thermal_tripless_zone_device_register().

Migrate to the new thermal zone device registration function.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../int340x_thermal/int340x_thermal_zone.c    | 28 +++++++++----------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
index 3e4bfe817fac..e45fdd36db55 100644
--- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -118,19 +118,17 @@ static int int340x_thermal_read_trips(struct acpi_device *zone_adev,
 	return trip_cnt;
 }
 
-static struct thermal_zone_params int340x_thermal_params = {
-	.governor_name = "user_space",
-	.no_hwmon = true,
-};
-
 struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
 						     int (*get_temp) (struct thermal_zone_device *, int *))
 {
+	struct thermal_governor_params tgp = {
+		.governor_name = "user_space"
+	};
 	struct int34x_thermal_zone *int34x_zone;
+	struct thermal_zone_device_params tzdp;
 	struct thermal_trip *zone_trips;
 	unsigned long long trip_cnt = 0;
 	unsigned long long hyst;
-	int trip_mask = 0;
 	acpi_status status;
 	int i, ret;
 
@@ -153,7 +151,7 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
 	status = acpi_evaluate_integer(adev->handle, "PATC", NULL, &trip_cnt);
 	if (ACPI_SUCCESS(status)) {
 		int34x_zone->aux_trip_nr = trip_cnt;
-		trip_mask = BIT(trip_cnt) - 1;
+		tzdp.tzp.mask = BIT(trip_cnt) - 1;
 	}
 
 	zone_trips = kzalloc(sizeof(*zone_trips) * (trip_cnt + INT340X_THERMAL_MAX_TRIP_COUNT),
@@ -183,13 +181,15 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
 
 	int34x_zone->lpat_table = acpi_lpat_get_conversion_table(adev->handle);
 
-	int34x_zone->zone = thermal_zone_device_register_with_trips(
-							acpi_device_bid(adev),
-							zone_trips, trip_cnt,
-							trip_mask, int34x_zone,
-							int34x_zone->ops,
-							&int340x_thermal_params,
-							0, 0);
+	tzdp.tzp.type = acpi_device_bid(adev);
+	tzdp.tzp.ops = int34x_zone->ops;
+	tzdp.tzp.devdata = int34x_zone;
+	tzdp.tzp.trips = zone_trips;
+	tzdp.tzp.num_trips = trip_cnt;
+	tzdp.tzp.no_hwmon = true;
+	tzdp.tgp = &tgp;
+
+	int34x_zone->zone = thermal_zone_device_register(&tzdp);
 	if (IS_ERR(int34x_zone->zone)) {
 		ret = PTR_ERR(int34x_zone->zone);
 		goto err_thermal_zone;
-- 
2.43.0

[PATCH v1 12/18] thermal: int340x: processor: Migrate to thermal_zone_device_register()

[AngeloGioacchino Del Regno]

The thermal API has a new thermal_zone_device_register() function which
is deprecating the older thermal_zone_device_register_with_trips() and
thermal_tripless_zone_device_register().

Migrate to the new thermal zone device registration function.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../processor_thermal_device_pci.c            | 26 ++++++++++++-------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
index d7495571dd5d..3170eab57e32 100644
--- a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
+++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
@@ -242,13 +242,19 @@ static struct thermal_zone_device_ops tzone_ops = {
 	.set_trip_temp	= sys_set_trip_temp,
 };
 
-static struct thermal_zone_params tzone_params = {
-	.governor_name = "user_space",
-	.no_hwmon = true,
-};
-
 static int proc_thermal_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
+	struct thermal_zone_device_params tzdp = {
+		.tzp = {
+			.type = "TCPU_PCI",
+			.mask = 1,
+			.no_hwmon = true,
+			.ops = &tzone_ops,
+		},
+	};
+	struct thermal_governor_params tgp = {
+		.governor_name = "user_space"
+	};
 	struct proc_thermal_device *proc_priv;
 	struct proc_thermal_pci *pci_info;
 	int irq_flag = 0, irq, ret;
@@ -289,10 +295,12 @@ static int proc_thermal_pci_probe(struct pci_dev *pdev, const struct pci_device_
 
 	psv_trip.temperature = get_trip_temp(pci_info);
 
-	pci_info->tzone = thermal_zone_device_register_with_trips("TCPU_PCI", &psv_trip,
-							1, 1, pci_info,
-							&tzone_ops,
-							&tzone_params, 0, 0);
+	tzdp.tzp.devdata = pci_info;
+	tzdp.tzp.trips = &psv_trip;
+	tzdp.tzp.num_trips = 1;
+	tzdp.tgp = &tgp;
+
+	pci_info->tzone = thermal_zone_device_register(&tzdp);
 	if (IS_ERR(pci_info->tzone)) {
 		ret = PTR_ERR(pci_info->tzone);
 		goto err_del_legacy;
-- 
2.43.0

[PATCH v1 13/18] thermal: intel: x86_pkg_temp: Migrate to thermal_zone_device_register()

[AngeloGioacchino Del Regno]

The thermal API has a new thermal_zone_device_register() function which
is deprecating the older thermal_zone_device_register_with_trips() and
thermal_tripless_zone_device_register().

Migrate to the new thermal zone device registration function.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/thermal/intel/x86_pkg_temp_thermal.c | 22 +++++++++++++-------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/thermal/intel/x86_pkg_temp_thermal.c b/drivers/thermal/intel/x86_pkg_temp_thermal.c
index 11a7f8108bbb..0bdc1d1855e2 100644
--- a/drivers/thermal/intel/x86_pkg_temp_thermal.c
+++ b/drivers/thermal/intel/x86_pkg_temp_thermal.c
@@ -57,10 +57,6 @@ struct zone_device {
 	struct cpumask			cpumask;
 };
 
-static struct thermal_zone_params pkg_temp_tz_params = {
-	.no_hwmon	= true,
-};
-
 /* Keep track of how many zone pointers we allocated in init() */
 static int max_id __read_mostly;
 /* Array of zone pointers */
@@ -312,6 +308,13 @@ static struct thermal_trip *pkg_temp_thermal_trips_init(int cpu, int tj_max, int
 
 static int pkg_temp_thermal_device_add(unsigned int cpu)
 {
+	struct thermal_zone_device_params tzdp = {
+		.tzp = {
+			.type = "x86_pkg_temp",
+			.ops = &tzone_ops,
+			.no_hwmon = true,
+		}
+	};
 	int id = topology_logical_die_id(cpu);
 	u32 eax, ebx, ecx, edx;
 	struct zone_device *zonedev;
@@ -344,10 +347,13 @@ static int pkg_temp_thermal_device_add(unsigned int cpu)
 
 	INIT_DELAYED_WORK(&zonedev->work, pkg_temp_thermal_threshold_work_fn);
 	zonedev->cpu = cpu;
-	zonedev->tzone = thermal_zone_device_register_with_trips("x86_pkg_temp",
-			zonedev->trips, thres_count,
-			(thres_count == MAX_NUMBER_OF_TRIPS) ? 0x03 : 0x01,
-			zonedev, &tzone_ops, &pkg_temp_tz_params, 0, 0);
+
+	tzdp.tzp.devdata = zonedev;
+	tzdp.tzp.trips = zonedev->trips;
+	tzdp.tzp.num_trips = thres_count;
+	tzdp.tzp.mask = (thres_count == MAX_NUMBER_OF_TRIPS) ? 0x03 : 0x01;
+
+	zonedev->tzone = thermal_zone_device_register(&tzdp);
 	if (IS_ERR(zonedev->tzone)) {
 		err = PTR_ERR(zonedev->tzone);
 		goto out_kfree_trips;
-- 
2.43.0

[PATCH v1 14/18] thermal/drivers/armada: Migrate to thermal_zone_device_register()

[AngeloGioacchino Del Regno]

The thermal API has a new thermal_zone_device_register() function which
is deprecating the older thermal_zone_device_register_with_trips() and
thermal_tripless_zone_device_register().

Migrate to the new thermal zone device registration function.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/thermal/armada_thermal.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index f783547ef964..e07bee0440fe 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -864,8 +864,16 @@ static int armada_thermal_probe(struct platform_device *pdev)
 	 * into it, which requires the use of regmaps across all the driver.
 	 */
 	if (IS_ERR(syscon_node_to_regmap(pdev->dev.parent->of_node))) {
+		struct thermal_zone_device_params tzdp = {
+			.tzp = {
+				.ops = &legacy_ops,
+				.devdata = priv
+			}
+		};
+
 		/* Ensure device name is correct for the thermal core */
 		armada_set_sane_name(pdev, priv);
+		tzdp.tzp.type = priv->zone_name;
 
 		ret = armada_thermal_probe_legacy(pdev, priv);
 		if (ret)
@@ -876,9 +884,7 @@ static int armada_thermal_probe(struct platform_device *pdev)
 		/* Wait the sensors to be valid */
 		armada_wait_sensor_validity(priv);
 
-		tz = thermal_tripless_zone_device_register(priv->zone_name,
-							   priv, &legacy_ops,
-							   NULL);
+		tz = thermal_zone_device_register(&tzdp);
 		if (IS_ERR(tz)) {
 			dev_err(&pdev->dev,
 				"Failed to register thermal zone device\n");
-- 
2.43.0

Re: [PATCH v1 14/18] thermal/drivers/armada: Migrate to thermal_zone_device_register()

[Miquel Raynal]

Hi,

angelogioacchino.delregno@collabora.com wrote on Tue, 30 Jan 2024
12:12:46 +0100:

> The thermal API has a new thermal_zone_device_register() function which
> is deprecating the older thermal_zone_device_register_with_trips() and
> thermal_tripless_zone_device_register().
> 
> Migrate to the new thermal zone device registration function.
> 

LGTM

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

[PATCH v1 15/18] thermal/drivers/dove: Migrate to thermal_zone_device_register()

[AngeloGioacchino Del Regno]

The thermal API has a new thermal_zone_device_register() function which
is deprecating the older thermal_zone_device_register_with_trips() and
thermal_tripless_zone_device_register().

Migrate to the new thermal zone device registration function.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/thermal/dove_thermal.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/dove_thermal.c b/drivers/thermal/dove_thermal.c
index ac30de3c0a5f..256c5dc45779 100644
--- a/drivers/thermal/dove_thermal.c
+++ b/drivers/thermal/dove_thermal.c
@@ -117,6 +117,12 @@ static const struct of_device_id dove_thermal_id_table[] = {
 
 static int dove_thermal_probe(struct platform_device *pdev)
 {
+	struct thermal_zone_device_params tzdp = {
+		.tzp = {
+			.type = "dove_thermal",
+			.ops = &ops
+		}
+	};
 	struct thermal_zone_device *thermal = NULL;
 	struct dove_thermal_priv *priv;
 	int ret;
@@ -139,8 +145,8 @@ static int dove_thermal_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	thermal = thermal_tripless_zone_device_register("dove_thermal", priv,
-							&ops, NULL);
+	tzdp.tzp.devdata = priv;
+	thermal = thermal_zone_device_register(&tzdp);
 	if (IS_ERR(thermal)) {
 		dev_err(&pdev->dev,
 			"Failed to register thermal zone device\n");
-- 
2.43.0

[PATCH v1 16/18] thermal/drivers/kirkwood: Migrate to thermal_zone_device_register()

[AngeloGioacchino Del Regno]

The thermal API has a new thermal_zone_device_register() function which
is deprecating the older thermal_zone_device_register_with_trips() and
thermal_tripless_zone_device_register().

Migrate to the new thermal zone device registration function.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/thermal/kirkwood_thermal.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/kirkwood_thermal.c b/drivers/thermal/kirkwood_thermal.c
index a18158ebe65f..0e4f24a91bb0 100644
--- a/drivers/thermal/kirkwood_thermal.c
+++ b/drivers/thermal/kirkwood_thermal.c
@@ -59,6 +59,12 @@ static const struct of_device_id kirkwood_thermal_id_table[] = {
 
 static int kirkwood_thermal_probe(struct platform_device *pdev)
 {
+	struct thermal_zone_device_params tzdp = {
+		.tzp = {
+			.type = "kirkwood_thermal",
+			.ops = &ops
+		}
+	};
 	struct thermal_zone_device *thermal = NULL;
 	struct kirkwood_thermal_priv *priv;
 	int ret;
@@ -71,8 +77,8 @@ static int kirkwood_thermal_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->sensor))
 		return PTR_ERR(priv->sensor);
 
-	thermal = thermal_tripless_zone_device_register("kirkwood_thermal",
-							priv, &ops, NULL);
+	tzdp.tzp.devdata = priv;
+	thermal = thermal_zone_device_register(&tzdp);
 	if (IS_ERR(thermal)) {
 		dev_err(&pdev->dev,
 			"Failed to register thermal zone device\n");
-- 
2.43.0

[PATCH v1 17/18] thermal/drivers/spear: Migrate to thermal_zone_device_register()

[AngeloGioacchino Del Regno]

The thermal API has a new thermal_zone_device_register() function which
is deprecating the older thermal_zone_device_register_with_trips() and
thermal_tripless_zone_device_register().

Migrate to the new thermal zone device registration function.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/thermal/spear_thermal.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/spear_thermal.c b/drivers/thermal/spear_thermal.c
index 60a871998b07..7d65f4bc95be 100644
--- a/drivers/thermal/spear_thermal.c
+++ b/drivers/thermal/spear_thermal.c
@@ -88,6 +88,12 @@ static SIMPLE_DEV_PM_OPS(spear_thermal_pm_ops, spear_thermal_suspend,
 
 static int spear_thermal_probe(struct platform_device *pdev)
 {
+	struct thermal_zone_device_params tzdp = {
+		.tzp = {
+			.type = "spear_thermal",
+			.ops = &ops
+		}
+	};
 	struct thermal_zone_device *spear_thermal = NULL;
 	struct spear_thermal_dev *stdev;
 	struct device_node *np = pdev->dev.of_node;
@@ -122,8 +128,8 @@ static int spear_thermal_probe(struct platform_device *pdev)
 	stdev->flags = val;
 	writel_relaxed(stdev->flags, stdev->thermal_base);
 
-	spear_thermal = thermal_tripless_zone_device_register("spear_thermal",
-							      stdev, &ops, NULL);
+	tzdp.tzp.devdata = stdev;
+	spear_thermal = thermal_zone_device_register(&tzdp);
 	if (IS_ERR(spear_thermal)) {
 		dev_err(&pdev->dev, "thermal zone device is NULL\n");
 		ret = PTR_ERR(spear_thermal);
-- 
2.43.0

[PATCH v1 18/18] thermal/drivers/int340x: Migrate to thermal_zone_device_register()

[AngeloGioacchino Del Regno]

The thermal API has a new thermal_zone_device_register() function which
is deprecating the older thermal_zone_device_register_with_trips() and
thermal_tripless_zone_device_register().

Migrate to the new thermal zone device registration function.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../intel/int340x_thermal/int3400_thermal.c   | 20 +++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
index 427d370648d5..d8ca7ed37950 100644
--- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
@@ -536,11 +536,6 @@ static struct thermal_zone_device_ops int3400_thermal_ops = {
 	.change_mode = int3400_thermal_change_mode,
 };
 
-static struct thermal_zone_params int3400_thermal_params = {
-	.governor_name = "user_space",
-	.no_hwmon = true,
-};
-
 static void int3400_setup_gddv(struct int3400_thermal_priv *priv)
 {
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
@@ -572,6 +567,16 @@ static void int3400_setup_gddv(struct int3400_thermal_priv *priv)
 static int int3400_thermal_probe(struct platform_device *pdev)
 {
 	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
+	struct thermal_zone_device_params tzdp = {
+		.tzp = {
+			.type = "INT3400 Thermal",
+			.no_hwmon = true,
+			.ops = &int3400_thermal_ops,
+		}
+	};
+	struct thermal_governor_params tgp = {
+		.governor_name = "user_space"
+	};
 	struct int3400_thermal_priv *priv;
 	int result;
 
@@ -609,9 +614,8 @@ static int int3400_thermal_probe(struct platform_device *pdev)
 
 	evaluate_odvp(priv);
 
-	priv->thermal = thermal_tripless_zone_device_register("INT3400 Thermal", priv,
-							      &int3400_thermal_ops,
-							      &int3400_thermal_params);
+	tzdp.tgp = &tgp;
+	priv->thermal = thermal_zone_device_register(&tzdp);
 	if (IS_ERR(priv->thermal)) {
 		result = PTR_ERR(priv->thermal);
 		goto free_art_trt;
-- 
2.43.0

Re: [PATCH v1 00/18] Thermal: Part 1 - Introduce new structs and registration

[Rafael J. Wysocki]

On Tue, Jan 30, 2024 at 12:13 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> This series is a preparation for a bigger cleanup that will be split in
> three parts in order to avoid immutable branches on multiple subsystems,
> as in other parts of this series there will be changes in:
> - drivers/acpi
> - drivers/platform/x86
> - drivers/power/supply
> - drivers/net/wireless
> - drivers/net/ethernet
>
> This is the first part which adds new structures and starts reorganizing
> struct members around, plus, this migrates all of the thermal drivers
> found in drivers/thermal/ to the new thermal_zone_device_register()
> function, and advertises that the old registration functions are
> deprecated and will be removed.
>
> The reorganization is supposed to be complete (or mostly) but...
>  - struct thermal_zone_platform_params has a temporary name:
>    this is done in order to avoid compile time failures for
>    the drivers outside of drivers/thermal before migrating
>    them to thermal_zone_device_params/thermal_zone_device_register();
>  - struct thermal_zone_params temporarily has two duplicated members,
>    governor_name and no_hwmon;
>
> Part 2 of this topic will migrate all drivers that are external to
> drivers/thermal to thermal_zone_device_register(); I will send that
> part only after part 1 is confirmed to be acceptable, as otherwise
> I'd be spamming people for no reason :-)
>
> After all drivers will be migrated to thermal_zone_device_register(),
> we won't have to care about changing anything outside of drivers/thermal
> to finish this set of cleanups/changes (and no immutable branches needed)
> and this means that...
> Part 3 of this topic will contain the following changes:
>  - thermal_zone_device_register_with_trips() will be removed
>  - thermal_tripless_zone_device_register() will be removed
>  - thermal_zone_params will be renamed to thermal_governor_ipa_params
>    - governor_name, no_hwmon members will be removed
>  - thermal_zone_platform_params will be renamed to thermal_zone_params
>  - Removal of the THERMAL_NAME_LENGTH limitation for `type`
>
> More scheduled changes, which should end up in part 3 (at least that's
> my intention), or eventually entirely after this cleanup topic, include:
>  - Introduction of Thermal Zone names
>  - Disambiguation of TZ name and type
>  - Addition of `thermal-zones`, `thermal-zone-names` parsing for
>    devicetrees

You really should start with this, because that's your goal.

It is quite arguable that it can be achieved without making all of the
changes mentioned above.

>
> ... Summarizing ...
>
> Part 1:
>  - Reorganize structures (some temporary names/leftovers)
>  - New registration function, deprecation of old ones
>  - Migration of drivers/thermal drivers to new registration

I kind of see where this is going, but I don't agree with some of the
changes made.

Let me comment on individual patches (which is not necessarily going
to happen today or even tomorrow, so let me go through the entire
series before deciding what to do next).

Thanks!

Re: [PATCH v1 00/18] Thermal: Part 1 - Introduce new structs and registration

[AngeloGioacchino Del Regno]

Il 01/02/24 19:35, Rafael J. Wysocki ha scritto:
> On Tue, Jan 30, 2024 at 12:13 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> This series is a preparation for a bigger cleanup that will be split in
>> three parts in order to avoid immutable branches on multiple subsystems,
>> as in other parts of this series there will be changes in:
>> - drivers/acpi
>> - drivers/platform/x86
>> - drivers/power/supply
>> - drivers/net/wireless
>> - drivers/net/ethernet
>>
>> This is the first part which adds new structures and starts reorganizing
>> struct members around, plus, this migrates all of the thermal drivers
>> found in drivers/thermal/ to the new thermal_zone_device_register()
>> function, and advertises that the old registration functions are
>> deprecated and will be removed.
>>
>> The reorganization is supposed to be complete (or mostly) but...
>>   - struct thermal_zone_platform_params has a temporary name:
>>     this is done in order to avoid compile time failures for
>>     the drivers outside of drivers/thermal before migrating
>>     them to thermal_zone_device_params/thermal_zone_device_register();
>>   - struct thermal_zone_params temporarily has two duplicated members,
>>     governor_name and no_hwmon;
>>
>> Part 2 of this topic will migrate all drivers that are external to
>> drivers/thermal to thermal_zone_device_register(); I will send that
>> part only after part 1 is confirmed to be acceptable, as otherwise
>> I'd be spamming people for no reason :-)
>>
>> After all drivers will be migrated to thermal_zone_device_register(),
>> we won't have to care about changing anything outside of drivers/thermal
>> to finish this set of cleanups/changes (and no immutable branches needed)
>> and this means that...
>> Part 3 of this topic will contain the following changes:
>>   - thermal_zone_device_register_with_trips() will be removed
>>   - thermal_tripless_zone_device_register() will be removed
>>   - thermal_zone_params will be renamed to thermal_governor_ipa_params
>>     - governor_name, no_hwmon members will be removed
>>   - thermal_zone_platform_params will be renamed to thermal_zone_params
>>   - Removal of the THERMAL_NAME_LENGTH limitation for `type`
>>
>> More scheduled changes, which should end up in part 3 (at least that's
>> my intention), or eventually entirely after this cleanup topic, include:
>>   - Introduction of Thermal Zone names
>>   - Disambiguation of TZ name and type
>>   - Addition of `thermal-zones`, `thermal-zone-names` parsing for
>>     devicetrees
> 
> You really should start with this, because that's your goal.
> 
> It is quite arguable that it can be achieved without making all of the
> changes mentioned above.
> 

Actually, my initial idea was exactly that... but then there were some
discussions around it, please check [1] for the thermal-zone-names, and
[2] for the rest of the discussion.

[1]: https://lore.kernel.org/all/8d42e0f5-b2d2-471b-ada9-79f76c637abe@collabora.com/

[2]: https://lore.kernel.org/all/4dd4ac79-e8bc-4d88-92d6-6061dae42092@collabora.com/

>>
>> ... Summarizing ...
>>
>> Part 1:
>>   - Reorganize structures (some temporary names/leftovers)
>>   - New registration function, deprecation of old ones
>>   - Migration of drivers/thermal drivers to new registration
> 
> I kind of see where this is going, but I don't agree with some of the
> changes made.
> 
> Let me comment on individual patches (which is not necessarily going
> to happen today or even tomorrow, so let me go through the entire
> series before deciding what to do next).
> 

Sure, thanks for your feedback!

Cheers,
Angelo