LinuxLists.cc - [RFC PATCH 00/26] Add thermal zones names and new registration func

2023-12-21 12:48:47

Subject: [RFC PATCH 00/26] Add thermal zones names and new registration func

** This RFC was sent only to thermal API maintainers + reviewers on purpose **

As per previous discussion with Daniel [1], I've prepared this series adding
a new struct thermal_zone_device_params, used in a new registration function
for thermal zones thermal_zone_device_register(), deprecating and, finally,
replacing functions thermal_tripless_zone_device_register() and
thermal_zone_device_register_with_trips().

The new flow to register a thermal zone becomes the following:
- Declare a struct thermal_zone_device_params (`tzp` in this example)
- Fill in all the params (instead of passing all of them to a function)
- Call thermal_zone_device_register(tzp)

Moreover, I've also introduced the concept of `name` for a thermal zone,
and set, as suggested, a constraint for which:
- Multiple thermal zones can have the same `type` (so, no change), and
- A thermal zone's name must be *unique*.

This should then help (in a later series?) to disambiguate thermal zone
name vs type, as most of (if not all) the users seem to actually be
misusing the TZ type referring to / using it as a TZ name.

Please note that this series is currently a RFC because it's apparently
growing bigger than I wanted - and because I probably have to add some
more code on top. Before doing so, I'm trying to get feedback on what
I've done until now.

P.S.: I know, there's a fixup commit in the mix, will obviously fix
that for v1 :-)

[1]: https://lore.kernel.org/all/[email protected]/

AngeloGioacchino Del Regno (26):
thermal: Introduce thermal_zone_device_register() and params structure
thermal/of: Migrate to thermal_zone_device_register()
platform/x86: acerhdf: Migrate to thermal_zone_device_register()
ACPI: thermal: Migrate to thermal_zone_device_register()
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()
power: supply: core: 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()
wifi: iwlwifi: mvm: Migrate to thermal_zone_device_register()
cxgb4: Migrate to thermal_zone_device_register()
mlxsw: core_thermal: Migrate to thermal_zone_device_register()
fixup! power: supply: core: Migrate to thermal_zone_device_register()
thermal: Remove tripless_zone_register and register_with_trips
functions
thermal: Introduce thermal zones names

drivers/acpi/thermal.c | 18 +--
.../ethernet/chelsio/cxgb4/cxgb4_thermal.c | 18 ++-
.../ethernet/mellanox/mlxsw/core_thermal.c | 93 +++++++-------
drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 27 ++--
drivers/platform/x86/acerhdf.c | 25 ++--
drivers/power/supply/power_supply_core.c | 14 +-
drivers/thermal/armada_thermal.c | 10 +-
drivers/thermal/da9062-thermal.c | 14 +-
drivers/thermal/dove_thermal.c | 8 +-
drivers/thermal/imx_thermal.c | 19 ++-
.../intel/int340x_thermal/int3400_thermal.c | 17 +--
.../int340x_thermal/int340x_thermal_zone.c | 28 ++--
.../processor_thermal_device_pci.c | 23 ++--
drivers/thermal/intel/intel_pch_thermal.c | 11 +-
.../thermal/intel/intel_quark_dts_thermal.c | 21 +--
drivers/thermal/intel/intel_soc_dts_iosf.c | 22 +++-
drivers/thermal/intel/x86_pkg_temp_thermal.c | 20 +--
drivers/thermal/kirkwood_thermal.c | 8 +-
drivers/thermal/rcar_thermal.c | 13 +-
drivers/thermal/spear_thermal.c | 8 +-
drivers/thermal/st/st_thermal.c | 15 ++-
drivers/thermal/thermal_core.c | 120 +++++++++---------
drivers/thermal/thermal_of.c | 38 +++---
drivers/thermal/thermal_sysfs.c | 9 ++
drivers/thermal/thermal_trace.h | 17 ++-
include/linux/thermal.h | 50 ++++++--
26 files changed, 391 insertions(+), 275 deletions(-)

--
2.43.0

2023-12-21 12:48:51

by AngeloGioacchino Del Regno

[permalink] [raw]

Subject: [RFC PATCH 01/26] thermal: Introduce thermal_zone_device_register() and params structure

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 <[email protected]>
---
.../ethernet/mellanox/mlxsw/core_thermal.c | 93 ++++++++++---------
1 file changed, 50 insertions(+), 43 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
index f1b48d6615f6..5ad9f90d096d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
@@ -226,10 +226,6 @@ static int mlxsw_thermal_get_temp(struct thermal_zone_device *tzdev,
return 0;
}

-static struct thermal_zone_params mlxsw_thermal_params = {
- .no_hwmon = true,
-};
-
static struct thermal_zone_device_ops mlxsw_thermal_ops = {
.bind = mlxsw_thermal_bind,
.unbind = mlxsw_thermal_unbind,
@@ -408,28 +404,30 @@ static const struct thermal_cooling_device_ops mlxsw_cooling_ops = {
static int
mlxsw_thermal_module_tz_init(struct mlxsw_thermal_module *module_tz)
{
- char tz_name[THERMAL_NAME_LENGTH];
+ struct thermal_zone_device_params tzdp = {
+ .tzp = { .no_hwmon = true },
+ .ops = &mlxsw_thermal_module_ops,
+ .devdata = module_tz,
+ .trips = module_tz->trips,
+ .num_trips = MLXSW_THERMAL_NUM_TRIPS,
+ .mask = MLXSW_THERMAL_TRIP_MASK,
+ .polling_delay = module_tz->parent->polling_delay
+ };
int err;

if (module_tz->slot_index)
- snprintf(tz_name, sizeof(tz_name), "mlxsw-lc%d-module%d",
- module_tz->slot_index, module_tz->module + 1);
+ tzdp.type = kasprintf("mlxsw-lc%d-module%d",
+ module_tz->slot_index, module_tz->module + 1);
else
- snprintf(tz_name, sizeof(tz_name), "mlxsw-module%d",
- module_tz->module + 1);
- module_tz->tzdev = thermal_zone_device_register_with_trips(tz_name,
- module_tz->trips,
- MLXSW_THERMAL_NUM_TRIPS,
- MLXSW_THERMAL_TRIP_MASK,
- module_tz,
- &mlxsw_thermal_module_ops,
- &mlxsw_thermal_params,
- 0,
- module_tz->parent->polling_delay);
- if (IS_ERR(module_tz->tzdev)) {
- err = PTR_ERR(module_tz->tzdev);
- return err;
- }
+ tzdp.type = kasprintf("mlxsw-module%d", module_tz->module + 1);
+
+ if (!tzdp.type)
+ return -ENOMEM;
+
+ module_tz->tzdev = thermal_zone_device_register(&tzdp);
+ kfree(tzdp.type);
+ if (IS_ERR(module_tz->tzdev))
+ return PTR_ERR(module_tz->tzdev);

err = thermal_zone_device_enable(module_tz->tzdev);
if (err)
@@ -536,23 +534,28 @@ mlxsw_thermal_modules_fini(struct mlxsw_thermal *thermal,
static int
mlxsw_thermal_gearbox_tz_init(struct mlxsw_thermal_module *gearbox_tz)
{
- char tz_name[40];
+ struct thermal_zone_device_params tzdp = {
+ .tzp = { .no_hwmon = true },
+ .ops = &mlxsw_thermal_gearbox_ops,
+ .devdata = gearbox_tz,
+ .trips = gearbox_tz->trips,
+ .num_trips = MLXSW_THERMAL_NUM_TRIPS,
+ .mask = MLXSW_THERMAL_TRIP_MASK,
+ .polling_delay = gearbox_tz->parent->polling_delay
+ };
int ret;

if (gearbox_tz->slot_index)
- snprintf(tz_name, sizeof(tz_name), "mlxsw-lc%d-gearbox%d",
- gearbox_tz->slot_index, gearbox_tz->module + 1);
+ tzdp.type = kasprintf("mlxsw-lc%d-gearbox%d",
+ gearbox_tz->slot_index, gearbox_tz->module + 1);
else
- snprintf(tz_name, sizeof(tz_name), "mlxsw-gearbox%d",
- gearbox_tz->module + 1);
- gearbox_tz->tzdev = thermal_zone_device_register_with_trips(tz_name,
- gearbox_tz->trips,
- MLXSW_THERMAL_NUM_TRIPS,
- MLXSW_THERMAL_TRIP_MASK,
- gearbox_tz,
- &mlxsw_thermal_gearbox_ops,
- &mlxsw_thermal_params, 0,
- gearbox_tz->parent->polling_delay);
+ tzdp.type = kasprintf("mlxsw-gearbox%d", gearbox_tz->module + 1);
+
+ if (!tzdp.type)
+ return -ENOMEM;
+
+ gearbox_tz->tzdev = thermal_zone_device_register(&tzdp);
+ kfree(tzdp.type);
if (IS_ERR(gearbox_tz->tzdev))
return PTR_ERR(gearbox_tz->tzdev);

@@ -695,6 +698,12 @@ int mlxsw_thermal_init(struct mlxsw_core *core,
const struct mlxsw_bus_info *bus_info,
struct mlxsw_thermal **p_thermal)
{
+ struct thermal_zone_device_params tzdp = {
+ .type = "mlxsw",
+ .tzp = { .no_hwmon = true },
+ .ops = &mlxsw_thermal_ops,
+ .mask = MLXSW_THERMAL_TRIP_MASK
+ };
char mfcr_pl[MLXSW_REG_MFCR_LEN] = { 0 };
enum mlxsw_reg_mfcr_pwm_frequency freq;
struct device *dev = bus_info->dev;
@@ -770,14 +779,12 @@ int mlxsw_thermal_init(struct mlxsw_core *core,
MLXSW_THERMAL_SLOW_POLL_INT :
MLXSW_THERMAL_POLL_INT;

- thermal->tzdev = thermal_zone_device_register_with_trips("mlxsw",
- thermal->trips,
- MLXSW_THERMAL_NUM_TRIPS,
- MLXSW_THERMAL_TRIP_MASK,
- thermal,
- &mlxsw_thermal_ops,
- &mlxsw_thermal_params, 0,
- thermal->polling_delay);
+ tzdp.devdata = thermal;
+ tzdp.trips = thermal->trips;
+ tzdp.num_trips = MLXSW_THERMAL_NUM_TRIPS;
+ tzdp.polling_delay = thermal->polling_delay;
+
+ thermal->tzdev = thermal_zone_device_register(&tzdp);
if (IS_ERR(thermal->tzdev)) {
err = PTR_ERR(thermal->tzdev);
dev_err(dev, "Failed to register thermal zone\n");
--
2.43.0

2023-12-21 12:54:52

by AngeloGioacchino Del Regno

[permalink] [raw]

Subject: [RFC PATCH 22/26] cxgb4: Migrate to thermal_zone_device_register()

2023-12-21 12:55:07

by AngeloGioacchino Del Regno

[permalink] [raw]

Subject: [RFC PATCH 21/26] wifi: iwlwifi: mvm: Migrate to thermal_zone_device_register()

2023-12-21 12:55:10

by AngeloGioacchino Del Regno

[permalink] [raw]

Subject: [RFC PATCH 24/26] fixup! power: supply: core: Migrate to thermal_zone_device_register()

---
drivers/power/supply/power_supply_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index d656a2e39157..b0211ab97f9c 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -1310,7 +1310,7 @@ static int psy_register_thermal(struct power_supply *psy)

/* Register battery zone device psy reports temperature */
if (psy_has_property(psy->desc, POWER_SUPPLY_PROP_TEMP)) {
- tzdp.name = psy->desc->name;
+ tzdp.type = psy->desc->name;
psy->tzd = thermal_zone_device_register(&tzdp);
if (IS_ERR(psy->tzd))
return PTR_ERR(psy->tzd);
--
2.43.0

2023-12-21 12:55:19

by AngeloGioacchino Del Regno

[permalink] [raw]

2024-01-23 11:05:33

by AngeloGioacchino Del Regno

[permalink] [raw]

Subject: Re: [RFC PATCH 01/26] thermal: Introduce thermal_zone_device_register() and params structure

Il 22/01/24 20:19, Daniel Lezcano ha scritto:
> On 18/01/2024 10:39, AngeloGioacchino Del Regno wrote:
>> Il 16/01/24 10:58, AngeloGioacchino Del Regno ha scritto:
>>> Il 15/01/24 13:39, Daniel Lezcano ha scritto:
>>>> On 21/12/2023 13:48, AngeloGioacchino Del Regno wrote:
>>>>> In preparation for extending the thermal zone devices to actually have
>>>>> a name and disambiguation of thermal zone types/names, introduce a new
>>>>> thermal_zone_device_params structure which holds all of the parameters
>>>>> that are necessary to register a thermal zone device, then add a new
>>>>> function thermal_zone_device_register().
>>>>>
>>>>> The latter takes as parameter the newly introduced structure and is
>>>>> made to eventually replace all usages of the now deprecated function
>>>>> thermal_zone_device_register_with_trips() and of
>>>>> thermal_tripless_zone_device_register().
>>>>>
>>>>> Signed-off-by: AngeloGioacchino Del Regno
>>>>> <[email protected]>
>>>>> ---
>>>>> drivers/thermal/thermal_core.c | 27 +++++++++++++++++++++++++++
>>>>> include/linux/thermal.h        | 33 +++++++++++++++++++++++++++++++++
>>>>> 2 files changed, 60 insertions(+)
>>>>>
>>>>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>>>>> index e5434cdbf23b..6be508eb2d72 100644
>>>>> --- a/drivers/thermal/thermal_core.c
>>>>> +++ b/drivers/thermal/thermal_core.c
>>>>> @@ -1235,6 +1235,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.
>>>>> @@ -1409,6 +1411,7 @@ thermal_zone_device_register_with_trips(const char
>>>>> *type, struct thermal_trip *t
>>>>> }
>>>>> 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,
>>>>> @@ -1420,6 +1423,30 @@ struct thermal_zone_device
>>>>> *thermal_tripless_zone_device_register(
>>>>> }
>>>>> 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)
>>>>> +{
>>>>> +    return thermal_zone_device_register_with_trips(tzdp->type, tzdp->trips,
>>>>> tzdp->num_trips,
>>>>> +                               tzdp->mask, tzdp->devdata, tzdp->ops,
>>>>> +                               &tzdp->tzp, tzdp->passive_delay,
>>>>> +                               tzdp->polling_delay);
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(thermal_zone_device_register);
>>>>> +
>>>>> void *thermal_zone_device_priv(struct thermal_zone_device *tzd)
>>>>> {
>>>>>       return tzd->devdata;
>>>>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>>>>> index 98957bae08ff..c6ed33a7e468 100644
>>>>> --- a/include/linux/thermal.h
>>>>> +++ b/include/linux/thermal.h
>>>>> @@ -258,6 +258,33 @@ struct thermal_zone_params {
>>>>>       int offset;
>>>>> };
>>>>> +/**
>>>>> + * struct thermal_zone_device_params - parameters for a thermal zone device
>>>>> + * @type:        the thermal zone device type
>>>>> + * @tzp:        thermal zone platform parameters
>>>>> + * @ops:        standard thermal zone device callbacks
>>>>> + * @devdata:        private device data
>>>>> + * @trips:        a pointer to an array of thermal trips, if any
>>>>> + * @num_trips:        the number of trip points the thermal zone support
>>>>> + * @mask:        a 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)
>>>>> + */
>>>>> +struct thermal_zone_device_params {
>>>>> +    const char *type;
>>>>> +    struct thermal_zone_params tzp;
>>>>> +    struct thermal_zone_device_ops *ops;
>>>>> +    void *devdata;
>>>>> +    struct thermal_trip *trips;
>>>>> +    int num_trips;
>>>>> +    int mask;
>>>>> +    int passive_delay;
>>>>> +    int polling_delay;
>>>>> +};
>>>>
>>>> From my POV, this "struct thermal_zone_params" has been always a inadequate
>>>> and catch-all structure. It will confuse with thermal_zone_device_params
>>>>
>>>> I suggest to cleanup a bit that by sorting the parameters in the right
>>>> structures where the result could be something like:
>>>>
>>>> eg.
>>>>
>>>> struct thermal_zone_params {
>>>>
>>>>      const char *type;
>>>>      struct thermal_zone_device_ops *ops;
>>>>      struct thermal_trip *trips;
>>>>      int num_trips;
>>>>
>>>>      int passive_delay;
>>>>      int polling_delay;
>>>>
>>>>      void *devdata;
>>>>          bool no_hwmon;
>>>> };
>>>>
>>>> struct thermal_governor_ipa_params {
>>>>          u32 sustainable_power;
>>>>          s32 k_po;
>>>>          s32 k_pu;
>>>>          s32 k_i;
>>>>          s32 k_d;
>>>>          s32 integral_cutoff;
>>>>          int slope;
>>>>          int offset;
>>>> };
>>>>
>>>> struct thermal_governor_params {
>>>>      char governor_name[THERMAL_NAME_LENGTH];
>>>>      union {
>>>>          struct thermal_governor_ipa_params ipa_params;
>>>>      };
>>>> };
>>>>
>>>> struct thermal_zone_device_params {
>>>>      struct thermal_zone_params *tzp;
>>>>      struct thermal_governor_params *tgp;
>>>> }
>>>>
>>>> No functional changes just code reorg, being a series to be submitted before
>>>> the rest on these RFC changes (2->26)
>>>>
>>>
>>> Could work. It's true that thermal_zone_params is a catch-all structure, and it's
>>> not really the best... but I also haven't checked how complex and/or how much time
>>> would your proposed change take.
>>>
>>> Shouldn't take much as far as I can foresee, but I really have to check a bit.
>>> If I'm right as in it's not something huge, the next series will directly have
>>> this stuff sorted - if not, I'll reach to you.
>>>
>>
>> So... I've checked the situation and coded some.
>>
>> I came to the conclusion that doing it as suggested is possible but realistically
>> suboptimal, because that will need multiple immutable branches to actually end up
>> in upstream as changes would otherwise break kernel build.
>>
>> Then, here I am suggesting a different way forward, while still performing this
>> much needed cleanup and reorganization:
>>
>> First, we introduce thermal_zone_device_register() and params structure with
>> this commit, which will have
>>
>> /* Structure to define Thermal Zone parameters */
>> struct thermal_zone_params {
>>      /* Scheduled for removal - see struct thermal_zone_governor_params. */
>>      char governor_name[THERMAL_NAME_LENGTH];
>
> Take the opportunity to introduce a patch before doing a change to:
>
>     const char *governor_name;
>

Agreed!

> AFAICS, there is no place in the kernel where it is not a const char * assignation
> which is by the way wrong:
>
>     char governor_name[THERMAL_NAME_LENGTH];
>     governor_name = "step_wise";
>
>     :)
>
>>      /* Scheduled for removal - see struct thermal_zone_governor_params. */
>>      bool no_hwmon;
>>
>>      /*
>>       * Sustainable power (heat) that this thermal zone can dissipate in
>>       * mW
>>       */
>>      u32 sustainable_power;
>>
>>      /*
>>       * Proportional parameter of the PID controller when
>>       * overshooting (i.e., when temperature is below the target)
>>       */
>>      s32 k_po;
>>
>>      /*
>>       * Proportional parameter of the PID controller when
>>       * undershooting
>>       */
>>      s32 k_pu;
>>
>>      /* Integral parameter of the PID controller */
>>      s32 k_i;
>>
>>      /* Derivative parameter of the PID controller */
>>      s32 k_d;
>>
>>      /* threshold below which the error is no longer accumulated */
>>      s32 integral_cutoff;
>>
>>      /*
>>       * @slope:    slope of a linear temperature adjustment curve.
>>       *         Used by thermal zone drivers.
>>       */
>>      int slope;
>>      /*
>>       * @offset:    offset of a linear temperature adjustment curve.
>>       *         Used by thermal zone drivers (default 0).
>>       */
>>      int offset;
>> };
>>
>> struct thermal_governor_params {
>>      char governor_name[THERMAL_NAME_LENGTH];
>
>     const char *governor_name;
>
>>      struct thermal_zone_params ipa_params;
>> };
>>
>> struct thermal_zone_platform_params {
>
> The name sounds inadequate.
>
> May be just thermal_zone_params ?
>

It's not the best, but the plan is to change the struct name in the second cycle,
as keeping the thermal_zone_params struct named as it is right now is essential
to avoid immutable branches.

window 1: Reorganization with temporary struct names -> no immutable branches
window 2: Cleanup and rename -> no immutable branches

Any change from the window 2 part brought to window 1 would need immutable
branches all around... so I really can't call it "thermal_zone_params" in
window 1.

Perhaps I can change the _platform_ name to something else, but still needs
to be different from "thermal_zone_params"...

>>      const char *type;
>>      struct thermal_zone_device_ops *ops;
>
> Move the ops in the thermal_zone_device_params
>
>>      struct thermal_trip *trips;
>>      int num_trips;
>>      int mask;
>>
>>      int passive_delay;
>>      int polling_delay;
>>
>>      void *devdata;
>
> And devdata also in the thermal_zone_device_params
>

Ok! :-)

>>      bool no_hwmon;
>> };
>>
>>
>> struct thermal_zone_device_params {
>>      struct thermal_zone_platform_params tzp;
>>      struct thermal_governor_params *tgp;
>> };
>>
>> (Note that the `no_hwmon` and `governor_name` are *temporarily* duplicated, but
>> there are good reasons to do that!)
>>
>> Drivers will follow with the migration to `thermal_zone_device_register()`,
>> and that will be done *strictly* like so:
>>
>> struct thermal_zone_device_params tzdp = {
>>      .tzp = {
>>          .type = "acerhdf",
>>          .tzp = { .governor_name = "bang_bang" },
>>          .ops = &acerhdf_dev_ops,
>>          .trips = trips,
>>          .num_trips = ARRAY_SIZE(trips),
>>          .polling_delay = kernelmode ? interval * 1000 : 0,
>>          /* devdata, no_hwmon go here later in the code */
>>      },
>>      .tgp = { .governor_name = "bang_bang" }
>> };
>>
>> Notice how in this case we're never *ever* referencing to any struct name,
>> apart from struct thermal_zone_device_params (meaning, I'm never creating
>> vars/pointers to struct thermal_zone_platform_params).
>>
>> If we follow this style, at least temporarily and at least during this cleanup,
>> we will end up with a plan such that:
>>
>> 1. In the first merge window:
>>     - Drivers get migrated to thermal_zone_device_register()
>>     - Functions register_with_trips()/tripless get deprecated but not yet removed
> >
>> 2. In the next merge window (expecting all users updated from the first window):
>>     - Functions register_with_trips/tripless get removed (<- no more external refs
>>       to struct thermal_zone_params, which can be then safely renamed!)
>>     - Duplicated members governor_name and no_hwmon get removed from
>>       struct thermal_zone_params
>>     - Some structures get renamed to give them the proposed better names (which
>>       I also genuinely like)
>>     - Only Thermal API internal changes, as users (all the ones that are not in
>>       drivers/thermal/) won't need to get updated at all!
>>       ... and that's why I said "strictly like so" in the example up there.
>>
>> I think that this is the best strategy, for both ease of merging the changes and
>> internal reorganization.
>>
>> Sure in the first merge window we'll be wasting a byte or two, but I am confident
>> in that this is a very low price to pay - and even better, only temporarily - for
>> other bigger benefits.
>>
>> What do you think?
>
> Sounds like a plan :)
>
>

Cool. It's time to write a good non-RFC series then!

Cheers,
Angelo