* [RFC PATCH 01/26] thermal: Introduce thermal_zone_device_register() and params structure
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2024-01-15 12:39 ` Daniel Lezcano
2023-12-21 12:48 ` [RFC PATCH 02/26] thermal/of: Migrate to thermal_zone_device_register() AngeloGioacchino Del Regno
` (25 subsequent siblings)
26 siblings, 1 reply; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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 <angelogioacchino.delregno@collabora.com>
---
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;
+};
+
/* Function declarations */
#ifdef CONFIG_THERMAL_OF
struct thermal_zone_device *devm_thermal_of_zone_register(struct device *dev, int id, void *data,
@@ -310,6 +337,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);
@@ -372,6 +401,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
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [RFC PATCH 01/26] thermal: Introduce thermal_zone_device_register() and params structure
2023-12-21 12:48 ` [RFC PATCH 01/26] thermal: Introduce thermal_zone_device_register() and params structure AngeloGioacchino Del Regno
@ 2024-01-15 12:39 ` Daniel Lezcano
2024-01-16 9:58 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 42+ messages in thread
From: Daniel Lezcano @ 2024-01-15 12:39 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel, kernel
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 <angelogioacchino.delregno@collabora.com>
> ---
> 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)
> /* Function declarations */
> #ifdef CONFIG_THERMAL_OF
> struct thermal_zone_device *devm_thermal_of_zone_register(struct device *dev, int id, void *data,
> @@ -310,6 +337,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);
> @@ -372,6 +401,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)
> { }
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [RFC PATCH 01/26] thermal: Introduce thermal_zone_device_register() and params structure
2024-01-15 12:39 ` Daniel Lezcano
@ 2024-01-16 9:58 ` AngeloGioacchino Del Regno
2024-01-18 9:39 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-01-16 9:58 UTC (permalink / raw)
To: Daniel Lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel, kernel
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 <angelogioacchino.delregno@collabora.com>
>> ---
>> 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.
Cheers,
Angelo
>> /* Function declarations */
>> #ifdef CONFIG_THERMAL_OF
>> struct thermal_zone_device *devm_thermal_of_zone_register(struct device *dev,
>> int id, void *data,
>> @@ -310,6 +337,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);
>> @@ -372,6 +401,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)
>> { }
>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [RFC PATCH 01/26] thermal: Introduce thermal_zone_device_register() and params structure
2024-01-16 9:58 ` AngeloGioacchino Del Regno
@ 2024-01-18 9:39 ` AngeloGioacchino Del Regno
2024-01-18 9:46 ` AngeloGioacchino Del Regno
` (2 more replies)
0 siblings, 3 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-01-18 9:39 UTC (permalink / raw)
To: Daniel Lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel, kernel
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 <angelogioacchino.delregno@collabora.com>
>>> ---
>>> 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];
/* 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];
struct thermal_zone_params ipa_params;
};
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 {
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?
Cheers!
Angelo
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [RFC PATCH 01/26] thermal: Introduce thermal_zone_device_register() and params structure
2024-01-18 9:39 ` AngeloGioacchino Del Regno
@ 2024-01-18 9:46 ` AngeloGioacchino Del Regno
2024-01-22 19:19 ` Daniel Lezcano
2024-01-22 19:19 ` Daniel Lezcano
2 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-01-18 9:46 UTC (permalink / raw)
To: Daniel Lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel, kernel
Il 18/01/24 10:39, AngeloGioacchino Del Regno ha scritto:
> 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
>>>> <angelogioacchino.delregno@collabora.com>
>>>> ---
>>>> 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];
>
> /* 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];
> struct thermal_zone_params ipa_params;
> };
>
> 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 {
> 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" },
Whoops, sorry, this .tzp should've been removed, my bad!
> .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" }
> };
Looks like this instead:
struct thermal_zone_device_params tzdp = {
.tzp = {
.type = "acerhdf",
.ops = &acerhdf_dev_ops,
.trips = trips,
.num_trips = ARRAY_SIZE(trips),
.polling_delay = kernelmode ? interval * 1000 : 0,
},
.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?
>
> Cheers!
> Angelo
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [RFC PATCH 01/26] thermal: Introduce thermal_zone_device_register() and params structure
2024-01-18 9:39 ` AngeloGioacchino Del Regno
2024-01-18 9:46 ` AngeloGioacchino Del Regno
@ 2024-01-22 19:19 ` Daniel Lezcano
2024-01-23 10:58 ` AngeloGioacchino Del Regno
2024-01-22 19:19 ` Daniel Lezcano
2 siblings, 1 reply; 42+ messages in thread
From: Daniel Lezcano @ 2024-01-22 19:19 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel, kernel
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
>>>> <angelogioacchino.delregno@collabora.com>
>>>> ---
>>>> 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;
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 ?
> 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
> 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 :)
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [RFC PATCH 01/26] thermal: Introduce thermal_zone_device_register() and params structure
2024-01-22 19:19 ` Daniel Lezcano
@ 2024-01-23 10:58 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-01-23 10:58 UTC (permalink / raw)
To: Daniel Lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel, kernel
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
>>>>> <angelogioacchino.delregno@collabora.com>
>>>>> ---
>>>>> 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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC PATCH 01/26] thermal: Introduce thermal_zone_device_register() and params structure
2024-01-18 9:39 ` AngeloGioacchino Del Regno
2024-01-18 9:46 ` AngeloGioacchino Del Regno
2024-01-22 19:19 ` Daniel Lezcano
@ 2024-01-22 19:19 ` Daniel Lezcano
2 siblings, 0 replies; 42+ messages in thread
From: Daniel Lezcano @ 2024-01-22 19:19 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel, kernel
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
>>>> <angelogioacchino.delregno@collabora.com>
>>>> ---
>>>> 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;
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 ?
> 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
> 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 :)
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 42+ messages in thread
* [RFC PATCH 02/26] thermal/of: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 01/26] thermal: Introduce thermal_zone_device_register() and params structure AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2024-01-15 17:17 ` Daniel Lezcano
2023-12-21 12:48 ` [RFC PATCH 03/26] platform/x86: acerhdf: " AngeloGioacchino Del Regno
` (24 subsequent siblings)
26 siblings, 1 reply; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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/thermal_of.c | 37 ++++++++++++++++--------------------
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index 1e0655b63259..62a903ad649f 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -471,16 +471,12 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node *
const struct thermal_zone_device_ops *ops)
{
struct thermal_zone_device *tz;
- struct thermal_trip *trips;
- struct thermal_zone_params tzp = {};
- struct thermal_zone_device_ops *of_ops;
+ struct thermal_zone_device_params tzdp;
struct device_node *np;
- int delay, pdelay;
- int ntrips, mask;
int ret;
- of_ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
- if (!of_ops)
+ tzdp.ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
+ if (!tzdp.ops)
return ERR_PTR(-ENOMEM);
np = of_thermal_zone_find(sensor, id);
@@ -490,30 +486,29 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node *
ret = PTR_ERR(np);
goto out_kfree_of_ops;
}
+ tzdp.type = np->name;
- trips = thermal_of_trips_init(np, &ntrips);
- if (IS_ERR(trips)) {
+ tzdp.trips = thermal_of_trips_init(np, &tzdp.num_trips);
+ if (IS_ERR(tzdp.trips)) {
pr_err("Failed to find trip points for %pOFn id=%d\n", sensor, id);
- ret = PTR_ERR(trips);
+ ret = PTR_ERR(tzdp.trips);
goto out_kfree_of_ops;
}
- ret = thermal_of_monitor_init(np, &delay, &pdelay);
+ ret = thermal_of_monitor_init(np, &tzdp.polling_delay, &tzdp.passive_delay);
if (ret) {
pr_err("Failed to initialize monitoring delays from %pOFn\n", np);
goto out_kfree_trips;
}
- thermal_of_parameters_init(np, &tzp);
+ thermal_of_parameters_init(np, &tzdp.tzp);
- of_ops->bind = thermal_of_bind;
- of_ops->unbind = thermal_of_unbind;
+ tzdp.ops->bind = thermal_of_bind;
+ tzdp.ops->unbind = thermal_of_unbind;
+ tzdp.mask = GENMASK_ULL((tzdp.num_trips) - 1, 0);
+ tzdp.devdata = data;
- mask = GENMASK_ULL((ntrips) - 1, 0);
-
- tz = thermal_zone_device_register_with_trips(np->name, trips, ntrips,
- mask, data, of_ops, &tzp,
- pdelay, delay);
+ tz = thermal_zone_device_register(&tzdp);
if (IS_ERR(tz)) {
ret = PTR_ERR(tz);
pr_err("Failed to register thermal zone %pOFn: %d\n", np, ret);
@@ -531,9 +526,9 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node *
return tz;
out_kfree_trips:
- kfree(trips);
+ kfree(tzdp.trips);
out_kfree_of_ops:
- kfree(of_ops);
+ kfree(tzdp.ops);
return ERR_PTR(ret);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [RFC PATCH 02/26] thermal/of: Migrate to thermal_zone_device_register()
2023-12-21 12:48 ` [RFC PATCH 02/26] thermal/of: Migrate to thermal_zone_device_register() AngeloGioacchino Del Regno
@ 2024-01-15 17:17 ` Daniel Lezcano
2024-01-16 9:50 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 42+ messages in thread
From: Daniel Lezcano @ 2024-01-15 17:17 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel, kernel
On 21/12/2023 13:48, 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.
Sounds good to me.
May be add "No functional change intended" ?
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com > ---
> drivers/thermal/thermal_of.c | 37 ++++++++++++++++--------------------
> 1 file changed, 16 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index 1e0655b63259..62a903ad649f 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -471,16 +471,12 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node *
> const struct thermal_zone_device_ops *ops)
> {
> struct thermal_zone_device *tz;
> - struct thermal_trip *trips;
> - struct thermal_zone_params tzp = {};
> - struct thermal_zone_device_ops *of_ops;
> + struct thermal_zone_device_params tzdp;
> struct device_node *np;
> - int delay, pdelay;
> - int ntrips, mask;
> int ret;
>
> - of_ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
> - if (!of_ops)
> + tzdp.ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
> + if (!tzdp.ops)
> return ERR_PTR(-ENOMEM);
>
> np = of_thermal_zone_find(sensor, id);
> @@ -490,30 +486,29 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node *
> ret = PTR_ERR(np);
> goto out_kfree_of_ops;
> }
> + tzdp.type = np->name;
>
> - trips = thermal_of_trips_init(np, &ntrips);
> - if (IS_ERR(trips)) {
> + tzdp.trips = thermal_of_trips_init(np, &tzdp.num_trips);
> + if (IS_ERR(tzdp.trips)) {
> pr_err("Failed to find trip points for %pOFn id=%d\n", sensor, id);
> - ret = PTR_ERR(trips);
> + ret = PTR_ERR(tzdp.trips);
> goto out_kfree_of_ops;
> }
>
> - ret = thermal_of_monitor_init(np, &delay, &pdelay);
> + ret = thermal_of_monitor_init(np, &tzdp.polling_delay, &tzdp.passive_delay);
> if (ret) {
> pr_err("Failed to initialize monitoring delays from %pOFn\n", np);
> goto out_kfree_trips;
> }
>
> - thermal_of_parameters_init(np, &tzp);
> + thermal_of_parameters_init(np, &tzdp.tzp);
>
> - of_ops->bind = thermal_of_bind;
> - of_ops->unbind = thermal_of_unbind;
> + tzdp.ops->bind = thermal_of_bind;
> + tzdp.ops->unbind = thermal_of_unbind;
> + tzdp.mask = GENMASK_ULL((tzdp.num_trips) - 1, 0);
> + tzdp.devdata = data;
>
> - mask = GENMASK_ULL((ntrips) - 1, 0);
> -
> - tz = thermal_zone_device_register_with_trips(np->name, trips, ntrips,
> - mask, data, of_ops, &tzp,
> - pdelay, delay);
> + tz = thermal_zone_device_register(&tzdp);
> if (IS_ERR(tz)) {
> ret = PTR_ERR(tz);
> pr_err("Failed to register thermal zone %pOFn: %d\n", np, ret);
> @@ -531,9 +526,9 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node *
> return tz;
>
> out_kfree_trips:
> - kfree(trips);
> + kfree(tzdp.trips);
> out_kfree_of_ops:
> - kfree(of_ops);
> + kfree(tzdp.ops);
>
> return ERR_PTR(ret);
> }
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [RFC PATCH 02/26] thermal/of: Migrate to thermal_zone_device_register()
2024-01-15 17:17 ` Daniel Lezcano
@ 2024-01-16 9:50 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-01-16 9:50 UTC (permalink / raw)
To: Daniel Lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel, kernel
Il 15/01/24 18:17, Daniel Lezcano ha scritto:
> On 21/12/2023 13:48, 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.
>
> Sounds good to me.
>
> May be add "No functional change intended" ?
>
Yeah, makes sense - will add "This patch brings no functional changes".
Cheers!
>> Signed-off-by: AngeloGioacchino Del Regno
>> <angelogioacchino.delregno@collabora.com > ---
>> drivers/thermal/thermal_of.c | 37 ++++++++++++++++--------------------
>> 1 file changed, 16 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
>> index 1e0655b63259..62a903ad649f 100644
>> --- a/drivers/thermal/thermal_of.c
>> +++ b/drivers/thermal/thermal_of.c
>> @@ -471,16 +471,12 @@ static struct thermal_zone_device
>> *thermal_of_zone_register(struct device_node *
>> const struct thermal_zone_device_ops *ops)
>> {
>> struct thermal_zone_device *tz;
>> - struct thermal_trip *trips;
>> - struct thermal_zone_params tzp = {};
>> - struct thermal_zone_device_ops *of_ops;
>> + struct thermal_zone_device_params tzdp;
>> struct device_node *np;
>> - int delay, pdelay;
>> - int ntrips, mask;
>> int ret;
>> - of_ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
>> - if (!of_ops)
>> + tzdp.ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
>> + if (!tzdp.ops)
>> return ERR_PTR(-ENOMEM);
>> np = of_thermal_zone_find(sensor, id);
>> @@ -490,30 +486,29 @@ static struct thermal_zone_device
>> *thermal_of_zone_register(struct device_node *
>> ret = PTR_ERR(np);
>> goto out_kfree_of_ops;
>> }
>> + tzdp.type = np->name;
>> - trips = thermal_of_trips_init(np, &ntrips);
>> - if (IS_ERR(trips)) {
>> + tzdp.trips = thermal_of_trips_init(np, &tzdp.num_trips);
>> + if (IS_ERR(tzdp.trips)) {
>> pr_err("Failed to find trip points for %pOFn id=%d\n", sensor, id);
>> - ret = PTR_ERR(trips);
>> + ret = PTR_ERR(tzdp.trips);
>> goto out_kfree_of_ops;
>> }
>> - ret = thermal_of_monitor_init(np, &delay, &pdelay);
>> + ret = thermal_of_monitor_init(np, &tzdp.polling_delay, &tzdp.passive_delay);
>> if (ret) {
>> pr_err("Failed to initialize monitoring delays from %pOFn\n", np);
>> goto out_kfree_trips;
>> }
>> - thermal_of_parameters_init(np, &tzp);
>> + thermal_of_parameters_init(np, &tzdp.tzp);
>> - of_ops->bind = thermal_of_bind;
>> - of_ops->unbind = thermal_of_unbind;
>> + tzdp.ops->bind = thermal_of_bind;
>> + tzdp.ops->unbind = thermal_of_unbind;
>> + tzdp.mask = GENMASK_ULL((tzdp.num_trips) - 1, 0);
>> + tzdp.devdata = data;
>> - mask = GENMASK_ULL((ntrips) - 1, 0);
>> -
>> - tz = thermal_zone_device_register_with_trips(np->name, trips, ntrips,
>> - mask, data, of_ops, &tzp,
>> - pdelay, delay);
>> + tz = thermal_zone_device_register(&tzdp);
>> if (IS_ERR(tz)) {
>> ret = PTR_ERR(tz);
>> pr_err("Failed to register thermal zone %pOFn: %d\n", np, ret);
>> @@ -531,9 +526,9 @@ static struct thermal_zone_device
>> *thermal_of_zone_register(struct device_node *
>> return tz;
>> out_kfree_trips:
>> - kfree(trips);
>> + kfree(tzdp.trips);
>> out_kfree_of_ops:
>> - kfree(of_ops);
>> + kfree(tzdp.ops);
>> return ERR_PTR(ret);
>> }
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [RFC PATCH 03/26] platform/x86: acerhdf: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 01/26] thermal: Introduce thermal_zone_device_register() and params structure AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 02/26] thermal/of: Migrate to thermal_zone_device_register() AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 04/26] ACPI: thermal: " AngeloGioacchino Del Regno
` (23 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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/platform/x86/acerhdf.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index 74bcb3d13104..f7c1710f736c 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -267,14 +267,6 @@ static const struct bios_settings bios_tbl[] __initconst = {
{"", "", "", 0, 0, {0, 0}, 0}
};
-/*
- * this struct is used to instruct thermal layer to use bang_bang instead of
- * default governor for acerhdf
- */
-static struct thermal_zone_params acerhdf_zone_params = {
- .governor_name = "bang_bang",
-};
-
static int acerhdf_get_temp(int *temp)
{
u8 read_temp;
@@ -669,6 +661,18 @@ static void acerhdf_unregister_platform(void)
static int __init acerhdf_register_thermal(void)
{
+ struct thermal_zone_device_params tzdp = {
+ .type = "acerhdf",
+ /*
+ * this struct is used to instruct thermal layer to use
+ * bang_bang instead of default governor for acerhdf
+ */
+ .tzp = { .governor_name = "bang_bang" },
+ .ops = &acerhdf_dev_ops,
+ .trips = trips,
+ .num_trips = ARRAY_SIZE(trips),
+ .polling_delay = kernelmode ? interval * 1000 : 0,
+ };
int ret;
cl_dev = thermal_cooling_device_register("acerhdf-fan", NULL,
@@ -677,10 +681,7 @@ static int __init acerhdf_register_thermal(void)
if (IS_ERR(cl_dev))
return -EINVAL;
- thz_dev = thermal_zone_device_register_with_trips("acerhdf", trips, ARRAY_SIZE(trips),
- 0, NULL, &acerhdf_dev_ops,
- &acerhdf_zone_params, 0,
- (kernelmode) ? interval*1000 : 0);
+ thz_dev = thermal_zone_device_register(&tzdp);
if (IS_ERR(thz_dev))
return -EINVAL;
--
2.43.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 04/26] ACPI: thermal: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (2 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 03/26] platform/x86: acerhdf: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 05/26] thermal/drivers/da9062: " AngeloGioacchino Del Regno
` (22 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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/acpi/thermal.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index ee28ca93d983..e4f44e7bb3ec 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -664,16 +664,18 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz,
unsigned int trip_count,
int passive_delay)
{
+ struct thermal_zone_device_params tzdp = {
+ .type = "acpitz",
+ .devdata = tz,
+ .ops = &acpi_thermal_zone_ops,
+ .trips = tz->trip_table,
+ .num_trips = trip_count,
+ .passive_delay = passive_delay,
+ .polling_delay = tz->polling_frequency * 100,
+ };
int result;
- tz->thermal_zone = thermal_zone_device_register_with_trips("acpitz",
- tz->trip_table,
- trip_count,
- 0, tz,
- &acpi_thermal_zone_ops,
- NULL,
- passive_delay,
- tz->polling_frequency * 100);
+ tz->thermal_zone = thermal_zone_device_register(&tzdp);
if (IS_ERR(tz->thermal_zone))
return PTR_ERR(tz->thermal_zone);
--
2.43.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 05/26] thermal/drivers/da9062: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (3 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 04/26] ACPI: thermal: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 06/26] thermal/drivers/imx: " AngeloGioacchino Del Regno
` (21 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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 | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-thermal.c
index 160d64913057..10b1fe74c43e 100644
--- a/drivers/thermal/da9062-thermal.c
+++ b/drivers/thermal/da9062-thermal.c
@@ -158,6 +158,11 @@ MODULE_DEVICE_TABLE(of, da9062_compatible_reg_id_table);
static int da9062_thermal_probe(struct platform_device *pdev)
{
+ struct thermal_zone_device_params tzdp = {
+ .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 +201,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.type = thermal->config->name;
+ tzdp.devdata = thermal;
+ tzdp.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
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 06/26] thermal/drivers/imx: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (4 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 05/26] thermal/drivers/da9062: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 07/26] thermal/drivers/rcar: " AngeloGioacchino Del Regno
` (20 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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 | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 7019c4fdd549..61b332e1b899 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -599,6 +599,13 @@ 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 = {
+ .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 +703,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.devdata = data;
+ tzdp.trips = &trips;
+ tzdp.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
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 07/26] thermal/drivers/rcar: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (5 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 06/26] thermal/drivers/imx: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 08/26] thermal/drivers/st: " AngeloGioacchino Del Regno
` (19 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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 | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index feb848d595fa..71bc7353285d 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -488,10 +488,15 @@ 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 = {
+ .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
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 08/26] thermal/drivers/st: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (6 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 07/26] thermal/drivers/rcar: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 09/26] thermal: intel: pch_thermal: " AngeloGioacchino Del Regno
` (18 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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..ff4fe417a99b 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.type = dev_name(dev);
+ tzdp.ops = &st_tz_ops;
+ tzdp.devdata = sensor;
+ tzdp.trips = &trip;
+ tzdp.num_trips = 1;
+ tzdp.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
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 09/26] thermal: intel: pch_thermal: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (7 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 08/26] thermal/drivers/st: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 10/26] thermal: intel: quark_dts: " AngeloGioacchino Del Regno
` (17 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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 | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/thermal/intel/intel_pch_thermal.c b/drivers/thermal/intel/intel_pch_thermal.c
index b3905e34c507..dfc4b206633e 100644
--- a/drivers/thermal/intel/intel_pch_thermal.c
+++ b/drivers/thermal/intel/intel_pch_thermal.c
@@ -159,6 +159,7 @@ static const char *board_names[] = {
static int intel_pch_thermal_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
{
+ struct thermal_zone_device_params tzdp = { .ops = &tzd_ops };
enum pch_board_ids board_id = id->driver_data;
struct pch_thermal_device *ptd;
int nr_trips = 0;
@@ -233,10 +234,12 @@ 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.type = board_names[board_id];
+ tzdp.devdata = ptd;
+ tzdp.trips = ptd->trips;
+ tzdp.num_trips = nr_trips;
+
+ 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
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 10/26] thermal: intel: quark_dts: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (8 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 09/26] thermal: intel: pch_thermal: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 11/26] thermal: intel: soc_dts_iosf: " AngeloGioacchino Del Regno
` (16 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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 | 21 +++++++++++--------
1 file changed, 12 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..aba2db87c140 100644
--- a/drivers/thermal/intel/intel_quark_dts_thermal.c
+++ b/drivers/thermal/intel/intel_quark_dts_thermal.c
@@ -320,10 +320,14 @@ 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 = {
+ .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 +343,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.mask = QRK_DTS_WR_MASK_CLR;
} else {
aux_entry->locked = false;
- wr_mask = QRK_DTS_WR_MASK_SET;
+ tzdp.mask = QRK_DTS_WR_MASK_SET;
}
/* Store DTS default state if DTS registers are not locked */
@@ -368,12 +372,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.devdata = aux_entry;
+ tzdp.trips = aux_entry->trips;
+ tzdp.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
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 11/26] thermal: intel: soc_dts_iosf: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (9 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 10/26] thermal: intel: quark_dts: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 12/26] thermal: intel: int340x: " AngeloGioacchino Del Regno
` (15 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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 | 22 +++++++++++++++-------
1 file changed, 15 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..da89e4b492b1 100644
--- a/drivers/thermal/intel/intel_soc_dts_iosf.c
+++ b/drivers/thermal/intel/intel_soc_dts_iosf.c
@@ -220,8 +220,13 @@ 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 = {
+ .ops = &tzone_ops,
+ .devdata = dts,
+ .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 +258,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.type = kasprintf(GFP_KERNEL, "soc_dts%d", id);
+ if (!tzdp.type)
+ return -ENOMEM;
+
+ tzdp.mask = trip_mask;
+
+ dts->tzone = thermal_zone_device_register(&tzdp);
+ kfree(tzdp.type);
if (IS_ERR(dts->tzone)) {
ret = PTR_ERR(dts->tzone);
goto err_ret;
--
2.43.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 12/26] thermal: intel: int340x: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (10 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 11/26] thermal: intel: soc_dts_iosf: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 13/26] thermal: int340x: processor: " AngeloGioacchino Del Regno
` (14 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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 a03b67579dd9..431f09ef65e6 100644
--- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -118,19 +118,19 @@ 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_zone_device_params tzdp = {
+ .tzp = {
+ .governor_name = "user_space",
+ .no_hwmon = true,
+ }
+ };
struct int34x_thermal_zone *int34x_zone;
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 +153,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.mask = BIT(trip_cnt) - 1;
}
zone_trips = kzalloc(sizeof(*zone_trips) * (trip_cnt + INT340X_THERMAL_MAX_TRIP_COUNT),
@@ -183,13 +183,13 @@ 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.type = acpi_device_bid(adev);
+ tzdp.ops = int34x_zone->ops;
+ tzdp.devdata = int34x_zone;
+ tzdp.trips = zone_trips;
+ tzdp.num_trips = trip_cnt;
+
+ 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
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 13/26] thermal: int340x: processor: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (11 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 12/26] thermal: intel: int340x: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 14/26] thermal: intel: x86_pkg_temp: " AngeloGioacchino Del Regno
` (13 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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 | 23 +++++++++++--------
1 file changed, 14 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..1e7b9172e11d 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,17 @@ 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 = {
+ .type = "TCPU_PCI",
+ .tzp = {
+ .governor_name = "user_space",
+ .no_hwmon = true,
+ },
+ .ops = &tzone_ops,
+ .mask = 1,
+ };
struct proc_thermal_device *proc_priv;
struct proc_thermal_pci *pci_info;
int irq_flag = 0, irq, ret;
@@ -289,10 +293,11 @@ 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.devdata = pci_info;
+ tzdp.trips = &psv_trip;
+ tzdp.num_trips = 1;
+
+ 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
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 14/26] thermal: intel: x86_pkg_temp: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (12 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 13/26] thermal: int340x: processor: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 15/26] power: supply: core: " AngeloGioacchino Del Regno
` (12 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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 | 20 ++++++++++++--------
1 file changed, 12 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..8fdab44d28df 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,11 @@ 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 = {
+ .type = "x86_pkg_temp",
+ .tzp = { .no_hwmon = true },
+ .ops = &tzone_ops,
+ };
int id = topology_logical_die_id(cpu);
u32 eax, ebx, ecx, edx;
struct zone_device *zonedev;
@@ -344,10 +345,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.devdata = zonedev;
+ tzdp.trips = zonedev->trips;
+ tzdp.num_trips = thres_count;
+ tzdp.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
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 15/26] power: supply: core: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (13 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 14/26] thermal: intel: x86_pkg_temp: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 16/26] thermal/drivers/armada: " AngeloGioacchino Del Regno
` (11 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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/power/supply/power_supply_core.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 73265001dd4b..d656a2e39157 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -1297,6 +1297,12 @@ static struct thermal_zone_device_ops psy_tzd_ops = {
static int psy_register_thermal(struct power_supply *psy)
{
+ struct thermal_zone_device_params tzdp = {
+ /* Prefer our hwmon device and avoid duplicates */
+ .tzp = { .no_hwmon = IS_ENABLED(CONFIG_POWER_SUPPLY_HWMON) },
+ .ops = &psy_tzd_ops,
+ .devdata = psy
+ };
int ret;
if (psy->desc->no_thermal)
@@ -1304,12 +1310,8 @@ 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)) {
- /* Prefer our hwmon device and avoid duplicates */
- struct thermal_zone_params tzp = {
- .no_hwmon = IS_ENABLED(CONFIG_POWER_SUPPLY_HWMON)
- };
- psy->tzd = thermal_tripless_zone_device_register(psy->desc->name,
- psy, &psy_tzd_ops, &tzp);
+ tzdp.name = psy->desc->name;
+ psy->tzd = thermal_zone_device_register(&tzdp);
if (IS_ERR(psy->tzd))
return PTR_ERR(psy->tzd);
ret = thermal_zone_device_enable(psy->tzd);
--
2.43.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 16/26] thermal/drivers/armada: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (14 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 15/26] power: supply: core: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 17/26] thermal/drivers/dove: " AngeloGioacchino Del Regno
` (10 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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 | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index f783547ef964..f48a776af147 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -864,8 +864,14 @@ 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 = {
+ .ops = &legacy_ops,
+ .devdata = priv
+ };
+
/* Ensure device name is correct for the thermal core */
armada_set_sane_name(pdev, priv);
+ tzdp.type = priv->zone_name;
ret = armada_thermal_probe_legacy(pdev, priv);
if (ret)
@@ -876,9 +882,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
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 17/26] thermal/drivers/dove: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (15 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 16/26] thermal/drivers/armada: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 18/26] thermal/drivers/kirkwood: " AngeloGioacchino Del Regno
` (9 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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 | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/dove_thermal.c b/drivers/thermal/dove_thermal.c
index ac30de3c0a5f..09ed02395d12 100644
--- a/drivers/thermal/dove_thermal.c
+++ b/drivers/thermal/dove_thermal.c
@@ -117,6 +117,10 @@ 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 = {
+ .type = "dove_thermal",
+ .ops = &ops
+ };
struct thermal_zone_device *thermal = NULL;
struct dove_thermal_priv *priv;
int ret;
@@ -139,8 +143,8 @@ static int dove_thermal_probe(struct platform_device *pdev)
return ret;
}
- thermal = thermal_tripless_zone_device_register("dove_thermal", priv,
- &ops, NULL);
+ tzdp.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
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 18/26] thermal/drivers/kirkwood: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (16 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 17/26] thermal/drivers/dove: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 19/26] thermal/drivers/spear: " AngeloGioacchino Del Regno
` (8 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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 | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/kirkwood_thermal.c b/drivers/thermal/kirkwood_thermal.c
index a18158ebe65f..a9ccb27b5906 100644
--- a/drivers/thermal/kirkwood_thermal.c
+++ b/drivers/thermal/kirkwood_thermal.c
@@ -59,6 +59,10 @@ 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 = {
+ .type = "kirkwood_thermal",
+ .ops = &ops
+ };
struct thermal_zone_device *thermal = NULL;
struct kirkwood_thermal_priv *priv;
int ret;
@@ -71,8 +75,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.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
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 19/26] thermal/drivers/spear: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (17 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 18/26] thermal/drivers/kirkwood: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 20/26] thermal/drivers/int340x: " AngeloGioacchino Del Regno
` (7 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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 | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/spear_thermal.c b/drivers/thermal/spear_thermal.c
index 60a871998b07..07006957acd5 100644
--- a/drivers/thermal/spear_thermal.c
+++ b/drivers/thermal/spear_thermal.c
@@ -88,6 +88,10 @@ 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 = {
+ .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 +126,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.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
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 20/26] thermal/drivers/int340x: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (18 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 19/26] thermal/drivers/spear: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 21/26] wifi: iwlwifi: mvm: " AngeloGioacchino Del Regno
` (6 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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 | 17 +++++++++--------
1 file changed, 9 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..87bfc1f9a5a4 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 };
@@ -571,6 +566,14 @@ static void int3400_setup_gddv(struct int3400_thermal_priv *priv)
static int int3400_thermal_probe(struct platform_device *pdev)
{
+ struct thermal_zone_device_params tzdp = {
+ .type = "INT3400 Thermal",
+ .tzp = {
+ .governor_name = "user_space",
+ .no_hwmon = true
+ },
+ .ops = &int3400_thermal_ops
+ };
struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
struct int3400_thermal_priv *priv;
int result;
@@ -609,9 +612,7 @@ 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);
+ priv->thermal = thermal_zone_device_register(&tzdp);
if (IS_ERR(priv->thermal)) {
result = PTR_ERR(priv->thermal);
goto free_art_trt;
--
2.43.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 21/26] wifi: iwlwifi: mvm: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (19 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 20/26] thermal/drivers/int340x: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 22/26] cxgb4: " AngeloGioacchino Del Regno
` (5 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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/net/wireless/intel/iwlwifi/mvm/tt.c | 27 ++++++++++++---------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
index dee9c367dcd3..268ff6ca3cac 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
@@ -674,8 +674,14 @@ static struct thermal_zone_device_ops tzone_ops = {
static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
{
int i, ret;
- char name[16];
static atomic_t counter = ATOMIC_INIT(0);
+ struct thermal_zone_device_params tzdp = {
+ .ops = &tzone_ops,
+ .devdata = mvm,
+ .trips = mvm->tz_device.trips,
+ .num_trips = IWL_MAX_DTS_TRIPS,
+ .mask = IWL_WRITABLE_TRIPS_MSK,
+ };
if (!iwl_mvm_is_tt_in_fw(mvm)) {
mvm->tz_device.tzone = NULL;
@@ -683,28 +689,25 @@ static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
return;
}
- BUILD_BUG_ON(ARRAY_SIZE(name) >= THERMAL_NAME_LENGTH);
+ tzdp.type = kasprintf("iwlwifi_%u", atomic_inc_return(&counter) & 0xFF);
+ if (!tzdp.type)
+ return -ENOMEM;
- sprintf(name, "iwlwifi_%u", atomic_inc_return(&counter) & 0xFF);
- mvm->tz_device.tzone = thermal_zone_device_register_with_trips(name,
- mvm->tz_device.trips,
- IWL_MAX_DTS_TRIPS,
- IWL_WRITABLE_TRIPS_MSK,
- mvm, &tzone_ops,
- NULL, 0, 0);
+ mvm->tz_device.tzone = thermal_zone_device_register(&tzdp);
+ kfree(tzdp.type);
if (IS_ERR(mvm->tz_device.tzone)) {
IWL_DEBUG_TEMP(mvm,
"Failed to register to thermal zone (err = %ld)\n",
PTR_ERR(mvm->tz_device.tzone));
mvm->tz_device.tzone = NULL;
- return;
+ goto err_free;
}
ret = thermal_zone_device_enable(mvm->tz_device.tzone);
if (ret) {
IWL_DEBUG_TEMP(mvm, "Failed to enable thermal zone\n");
thermal_zone_device_unregister(mvm->tz_device.tzone);
- return;
+ goto err_free;
}
/* 0 is a valid temperature,
@@ -714,6 +717,8 @@ static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
mvm->tz_device.trips[i].temperature = INT_MIN;
mvm->tz_device.trips[i].type = THERMAL_TRIP_PASSIVE;
}
+
+ return;
}
static int iwl_mvm_tcool_get_max_state(struct thermal_cooling_device *cdev,
--
2.43.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 22/26] cxgb4: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (20 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 21/26] wifi: iwlwifi: mvm: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 23/26] mlxsw: core_thermal: " AngeloGioacchino Del Regno
` (4 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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>
---
.../net/ethernet/chelsio/cxgb4/cxgb4_thermal.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
index dea9d2907666..0192dba14a84 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
@@ -37,12 +37,21 @@ static struct thermal_trip trip = { .type = THERMAL_TRIP_CRITICAL } ;
int cxgb4_thermal_init(struct adapter *adap)
{
+ struct thermal_zone_device_params tzdp = {
+ .ops = &cxgb4_thermal_ops,
+ .devdata = adap,
+ .trips = &trip,
+ .num_trips = num_trip,
+ };
struct ch_thermal *ch_thermal = &adap->ch_thermal;
- char ch_tz_name[THERMAL_NAME_LENGTH];
int num_trip = CXGB4_NUM_TRIPS;
u32 param, val;
int ret;
+ tzdp.type = kasprintf("cxgb4_%s", adap->name);
+ if (!tzdp.type)
+ return -ENOMEM;
+
/* on older firmwares we may not get the trip temperature,
* set the num of trips to 0.
*/
@@ -58,11 +67,8 @@ int cxgb4_thermal_init(struct adapter *adap)
trip.temperature = val * 1000;
}
- snprintf(ch_tz_name, sizeof(ch_tz_name), "cxgb4_%s", adap->name);
- ch_thermal->tzdev = thermal_zone_device_register_with_trips(ch_tz_name, &trip, num_trip,
- 0, adap,
- &cxgb4_thermal_ops,
- NULL, 0, 0);
+ ch_thermal->tzdev = thermal_zone_device_register(&tzdp);
+ kfree(tzdp.type);
if (IS_ERR(ch_thermal->tzdev)) {
ret = PTR_ERR(ch_thermal->tzdev);
dev_err(adap->pdev_dev, "Failed to register thermal zone\n");
--
2.43.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 23/26] mlxsw: core_thermal: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (21 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 22/26] cxgb4: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 24/26] fixup! power: supply: core: " AngeloGioacchino Del Regno
` (3 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
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>
---
.../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
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 24/26] fixup! power: supply: core: Migrate to thermal_zone_device_register()
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (22 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 23/26] mlxsw: core_thermal: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 25/26] thermal: Remove tripless_zone_register and register_with_trips functions AngeloGioacchino Del Regno
` (2 subsequent siblings)
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
---
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
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 25/26] thermal: Remove tripless_zone_register and register_with_trips functions
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (23 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 24/26] fixup! power: supply: core: " AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2023-12-21 12:48 ` [RFC PATCH 26/26] thermal: Introduce thermal zones names AngeloGioacchino Del Regno
2023-12-21 13:38 ` [RFC PATCH 00/26] Add thermal zones names and new registration func Rafael J. Wysocki
26 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
There are no more users of thermal_tripless_zone_device_register() and
thermal_zone_device_register_with_trips() functions, as they were all
converted to the new thermal_zone_device_register(): remove the former
two functions.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/thermal/thermal_core.c | 111 +++++++++------------------------
include/linux/thermal.h | 15 -----
2 files changed, 29 insertions(+), 97 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 6be508eb2d72..9eb0200a85ff 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1221,21 +1221,9 @@ int thermal_zone_get_crit_temp(struct thermal_zone_device *tz, int *temp)
EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp);
/**
- * thermal_zone_device_register_with_trips() - register a new thermal zone device
- * @type: the thermal zone device type
- * @trips: a pointer to an array of thermal trips
- * @num_trips: the number of trip points the thermal zone support
- * @mask: a bit string indicating the writeablility of trip points
- * @devdata: private device data
- * @ops: standard thermal zone device callbacks
- * @tzp: thermal zone platform parameters
- * @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)
- *
- * This function is deprecated. See thermal_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
@@ -1247,25 +1235,21 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp);
* in case of error, an ERR_PTR. Caller must check return value with
* IS_ERR*() helpers.
*/
-struct thermal_zone_device *
-thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *trips, int num_trips, int mask,
- void *devdata, struct thermal_zone_device_ops *ops,
- const struct thermal_zone_params *tzp, int passive_delay,
- int polling_delay)
+struct thermal_zone_device *thermal_zone_device_register(struct thermal_zone_device_params *tzdp)
{
struct thermal_zone_device *tz;
int id;
int result;
struct thermal_governor *governor;
- if (!type || strlen(type) == 0) {
+ if (!tzdp->type || strlen(tzdp->type) == 0) {
pr_err("No thermal zone type defined\n");
return ERR_PTR(-EINVAL);
}
- if (strlen(type) >= THERMAL_NAME_LENGTH) {
+ if (strlen(tzdp->type) >= THERMAL_NAME_LENGTH) {
pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
- type, THERMAL_NAME_LENGTH);
+ tzdp->type, THERMAL_NAME_LENGTH);
return ERR_PTR(-EINVAL);
}
@@ -1282,17 +1266,18 @@ 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->num_trips > (BITS_PER_TYPE(int) - 1) || tzdp->num_trips < 0 ||
+ tzdp->mask >> tzdp->num_trips) {
pr_err("Incorrect number of thermal trips\n");
return ERR_PTR(-EINVAL);
}
- if (!ops) {
+ if (!tzdp->ops) {
pr_err("Thermal zone device ops not defined\n");
return ERR_PTR(-EINVAL);
}
- if (num_trips > 0 && !trips)
+ if (tzdp->num_trips > 0 && !tzdp->trips)
return ERR_PTR(-EINVAL);
if (!thermal_class)
@@ -1302,12 +1287,10 @@ 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) {
- result = -ENOMEM;
- goto free_tz;
- }
+ tz->tzp = kmemdup(&tzdp->tzp, sizeof(tzdp->tzp), GFP_KERNEL);
+ if (!tz->tzp) {
+ result = -ENOMEM;
+ goto free_tz;
}
INIT_LIST_HEAD(&tz->thermal_instances);
@@ -1322,23 +1305,23 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
}
tz->id = id;
- strscpy(tz->type, type, sizeof(tz->type));
+ strscpy(tz->type, tzdp->type, sizeof(tz->type));
- if (!ops->critical)
- ops->critical = thermal_zone_device_critical;
+ if (!tzdp->ops->critical)
+ tzdp->ops->critical = thermal_zone_device_critical;
- tz->ops = ops;
+ tz->ops = tzdp->ops;
tz->device.class = thermal_class;
- tz->devdata = devdata;
- tz->trips = trips;
- tz->num_trips = num_trips;
+ tz->devdata = tzdp->devdata;
+ tz->trips = tzdp->trips;
+ tz->num_trips = tzdp->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->passive_delay);
+ thermal_set_delay_jiffies(&tz->polling_delay_jiffies, tzdp->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->mask);
if (result)
goto remove_id;
@@ -1357,10 +1340,10 @@ 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);
- else
+ if (strlen(tz->tzp->governor_name) == 0)
governor = def_governor;
+ else
+ governor = __find_governor(tz->tzp->governor_name);
result = thermal_set_governor(tz, governor);
if (result) {
@@ -1370,7 +1353,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 (!tz->tzp->no_hwmon) {
result = thermal_add_hwmon_sysfs(tz);
if (result)
goto unregister;
@@ -1409,42 +1392,6 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
kfree(tz);
return ERR_PTR(result);
}
-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)
-{
- 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)
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index c6ed33a7e468..84b62575d93a 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -322,21 +322,6 @@ int thermal_zone_get_num_trips(struct thermal_zone_device *tz);
int thermal_zone_get_crit_temp(struct thermal_zone_device *tz, int *temp);
#ifdef CONFIG_THERMAL
-struct thermal_zone_device *thermal_zone_device_register_with_trips(
- const char *type,
- struct thermal_trip *trips,
- int num_trips, int mask,
- void *devdata,
- struct thermal_zone_device_ops *ops,
- const struct thermal_zone_params *tzp,
- int passive_delay, int polling_delay);
-
-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);
-
struct thermal_zone_device *thermal_zone_device_register(struct thermal_zone_device_params *tzdp);
void thermal_zone_device_unregister(struct thermal_zone_device *tz);
--
2.43.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [RFC PATCH 26/26] thermal: Introduce thermal zones names
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (24 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 25/26] thermal: Remove tripless_zone_register and register_with_trips functions AngeloGioacchino Del Regno
@ 2023-12-21 12:48 ` AngeloGioacchino Del Regno
2024-01-16 9:14 ` Daniel Lezcano
2023-12-21 13:38 ` [RFC PATCH 00/26] Add thermal zones names and new registration func Rafael J. Wysocki
26 siblings, 1 reply; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 12:48 UTC (permalink / raw)
To: daniel.lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
AngeloGioacchino Del Regno, kernel
Currently thermal zones have a "type" but this is often used, and
referenced, as a name instead in multiple kernel drivers that are
either registering a zone or grabbing a thermal zone handle and
unfortunately this is a kind of abuse/misuse of the thermal zone
concept of "type".
In order to disambiguate name<->type and to actually provide an
accepted way of giving a specific name to a thermal zone for both
platform drivers and devicetree-defined zones, add a new "name"
member in the main thermal_zone_device structure, and also to the
thermal_zone_device_params structure which is used to register a
thermal zone device.
This will enforce the following constraints:
- Multiple thermal zones may be of the same "type" (no change);
- A thermal zone may have a *unique* name: trying to register
a new zone with the same name as an already present one will
produce a failure;
---
drivers/thermal/thermal_core.c | 34 ++++++++++++++++++++++++++++++---
drivers/thermal/thermal_of.c | 1 +
drivers/thermal/thermal_sysfs.c | 9 +++++++++
drivers/thermal/thermal_trace.h | 17 +++++++++++------
include/linux/thermal.h | 4 ++++
5 files changed, 56 insertions(+), 9 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 9eb0200a85ff..adf2ac8113e1 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1238,8 +1238,8 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp);
struct thermal_zone_device *thermal_zone_device_register(struct thermal_zone_device_params *tzdp)
{
struct thermal_zone_device *tz;
- int id;
- int result;
+ int id, tz_name_len;
+ int result = 0;
struct thermal_governor *governor;
if (!tzdp->type || strlen(tzdp->type) == 0) {
@@ -1248,11 +1248,36 @@ struct thermal_zone_device *thermal_zone_device_register(struct thermal_zone_dev
}
if (strlen(tzdp->type) >= THERMAL_NAME_LENGTH) {
- pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
+ pr_err("Thermal zone type (%s) too long, should be under %d chars\n",
tzdp->type, THERMAL_NAME_LENGTH);
return ERR_PTR(-EINVAL);
}
+ tz_name_len = tzdp->name ? strlen(tzdp->name) : 0;
+ if (tz_name_len) {
+ struct thermal_zone_device *pos;
+
+ if (tz_name_len >= THERMAL_NAME_LENGTH) {
+ pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
+ tzdp->name, THERMAL_NAME_LENGTH);
+ return ERR_PTR(-EINVAL);
+ }
+
+ mutex_lock(&thermal_list_lock);
+ list_for_each_entry(pos, &thermal_tz_list, node)
+ if (!strncasecmp(tzdp->name, pos->name, THERMAL_NAME_LENGTH)) {
+ result = -EEXIST;
+ break;
+ }
+ mutex_unlock(&thermal_list_lock);
+
+ if (result) {
+ pr_err("Thermal zone name (%s) already exists and must be unique\n",
+ tzdp->name);
+ return ERR_PTR(result);
+ }
+ }
+
/*
* Max trip count can't exceed 31 as the "mask >> num_trips" condition.
* For example, shifting by 32 will result in compiler warning:
@@ -1307,6 +1332,9 @@ struct thermal_zone_device *thermal_zone_device_register(struct thermal_zone_dev
tz->id = id;
strscpy(tz->type, tzdp->type, sizeof(tz->type));
+ if (tz_name_len)
+ strscpy(tz->name, tzdp->name, sizeof(tzdp->name));
+
if (!tzdp->ops->critical)
tzdp->ops->critical = thermal_zone_device_critical;
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index 62a903ad649f..eaacc140abeb 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -486,6 +486,7 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node *
ret = PTR_ERR(np);
goto out_kfree_of_ops;
}
+ tzdp.name = np->name;
tzdp.type = np->name;
tzdp.trips = thermal_of_trips_init(np, &tzdp.num_trips);
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index f52af8a3b4b5..f4002fa6caa2 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -23,6 +23,14 @@
/* sys I/F for thermal zone */
+static ssize_t
+name_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->name);
+}
+
static ssize_t
type_show(struct device *dev, struct device_attribute *attr, char *buf)
{
@@ -341,6 +349,7 @@ create_s32_tzp_attr(offset);
* All the attributes created for tzp (create_s32_tzp_attr) also are always
* present on the sysfs interface.
*/
+static DEVICE_ATTR_RO(name);
static DEVICE_ATTR_RO(type);
static DEVICE_ATTR_RO(temp);
static DEVICE_ATTR_RW(policy);
diff --git a/drivers/thermal/thermal_trace.h b/drivers/thermal/thermal_trace.h
index 459c8ce6cf3b..c9dbae1e3b9e 100644
--- a/drivers/thermal/thermal_trace.h
+++ b/drivers/thermal/thermal_trace.h
@@ -28,6 +28,7 @@ TRACE_EVENT(thermal_temperature,
TP_ARGS(tz),
TP_STRUCT__entry(
+ __string(thermal_zone_name, tz->name)
__string(thermal_zone, tz->type)
__field(int, id)
__field(int, temp_prev)
@@ -35,15 +36,16 @@ TRACE_EVENT(thermal_temperature,
),
TP_fast_assign(
+ __assign_str(thermal_zone_name, tz->name);
__assign_str(thermal_zone, tz->type);
__entry->id = tz->id;
__entry->temp_prev = tz->last_temperature;
__entry->temp = tz->temperature;
),
- TP_printk("thermal_zone=%s id=%d temp_prev=%d temp=%d",
- __get_str(thermal_zone), __entry->id, __entry->temp_prev,
- __entry->temp)
+ TP_printk("thermal_zone=%s name=%s id=%d temp_prev=%d temp=%d",
+ __get_str(thermal_zone), __get_str(thermal_zone_name),
+ __entry->id, __entry->temp_prev, __entry->temp)
);
TRACE_EVENT(cdev_update,
@@ -73,6 +75,7 @@ TRACE_EVENT(thermal_zone_trip,
TP_ARGS(tz, trip, trip_type),
TP_STRUCT__entry(
+ __string(thermal_zone_name, tz->name)
__string(thermal_zone, tz->type)
__field(int, id)
__field(int, trip)
@@ -80,15 +83,17 @@ TRACE_EVENT(thermal_zone_trip,
),
TP_fast_assign(
+ __assign_str(thermal_zone_name, tz->name);
__assign_str(thermal_zone, tz->type);
__entry->id = tz->id;
__entry->trip = trip;
__entry->trip_type = trip_type;
),
- TP_printk("thermal_zone=%s id=%d trip=%d trip_type=%s",
- __get_str(thermal_zone), __entry->id, __entry->trip,
- show_tzt_type(__entry->trip_type))
+ TP_printk("thermal_zone=%s name=%s id=%d trip=%d trip_type=%s",
+ __get_str(thermal_zone), __get_str(thermal_zone_name),
+ __entry->id, __entry->trip,
+ show_tzt_type(__entry->trip_type))
);
#ifdef CONFIG_CPU_THERMAL
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 84b62575d93a..a06521eda162 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -115,6 +115,7 @@ struct thermal_cooling_device {
/**
* struct thermal_zone_device - structure for a thermal zone
* @id: unique id number for each thermal zone
+ * @name: name of the thermal zone device
* @type: the thermal zone device type
* @device: &struct device for this thermal zone
* @removal: removal completion
@@ -155,6 +156,7 @@ struct thermal_cooling_device {
*/
struct thermal_zone_device {
int id;
+ char name[THERMAL_NAME_LENGTH];
char type[THERMAL_NAME_LENGTH];
struct device device;
struct completion removal;
@@ -260,6 +262,7 @@ struct thermal_zone_params {
/**
* struct thermal_zone_device_params - parameters for a thermal zone device
+ * @name: name of the thermal zone device
* @type: the thermal zone device type
* @tzp: thermal zone platform parameters
* @ops: standard thermal zone device callbacks
@@ -274,6 +277,7 @@ struct thermal_zone_params {
* driven systems)
*/
struct thermal_zone_device_params {
+ const char *name;
const char *type;
struct thermal_zone_params tzp;
struct thermal_zone_device_ops *ops;
--
2.43.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [RFC PATCH 26/26] thermal: Introduce thermal zones names
2023-12-21 12:48 ` [RFC PATCH 26/26] thermal: Introduce thermal zones names AngeloGioacchino Del Regno
@ 2024-01-16 9:14 ` Daniel Lezcano
2024-01-16 9:45 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 42+ messages in thread
From: Daniel Lezcano @ 2024-01-16 9:14 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel, kernel
On 21/12/2023 13:48, AngeloGioacchino Del Regno wrote:
> Currently thermal zones have a "type" but this is often used, and
> referenced, as a name instead in multiple kernel drivers that are
> either registering a zone or grabbing a thermal zone handle and
> unfortunately this is a kind of abuse/misuse of the thermal zone
> concept of "type".
>
> In order to disambiguate name<->type and to actually provide an
> accepted way of giving a specific name to a thermal zone for both
> platform drivers and devicetree-defined zones, add a new "name"
> member in the main thermal_zone_device structure, and also to the
> thermal_zone_device_params structure which is used to register a
> thermal zone device.
>
> This will enforce the following constraints:
> - Multiple thermal zones may be of the same "type" (no change);
> - A thermal zone may have a *unique* name: trying to register
> a new zone with the same name as an already present one will
> produce a failure;
> ---
> drivers/thermal/thermal_core.c | 34 ++++++++++++++++++++++++++++++---
> drivers/thermal/thermal_of.c | 1 +
> drivers/thermal/thermal_sysfs.c | 9 +++++++++
> drivers/thermal/thermal_trace.h | 17 +++++++++++------
> include/linux/thermal.h | 4 ++++
> 5 files changed, 56 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 9eb0200a85ff..adf2ac8113e1 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1238,8 +1238,8 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp);
> struct thermal_zone_device *thermal_zone_device_register(struct thermal_zone_device_params *tzdp)
> {
> struct thermal_zone_device *tz;
> - int id;
> - int result;
> + int id, tz_name_len;
> + int result = 0;
> struct thermal_governor *governor;
>
> if (!tzdp->type || strlen(tzdp->type) == 0) {
> @@ -1248,11 +1248,36 @@ struct thermal_zone_device *thermal_zone_device_register(struct thermal_zone_dev
> }
>
> if (strlen(tzdp->type) >= THERMAL_NAME_LENGTH) {
> - pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
> + pr_err("Thermal zone type (%s) too long, should be under %d chars\n",
> tzdp->type, THERMAL_NAME_LENGTH);
> return ERR_PTR(-EINVAL);
I would keep that as is and do second round of changes to clarify the
usage of ->type
> }
>
> + tz_name_len = tzdp->name ? strlen(tzdp->name) : 0;
I suggest to change to a const char * and no longer limit to
THERMAL_NAME_LENGTH.
> + if (tz_name_len) {
> + struct thermal_zone_device *pos;
> +
> + if (tz_name_len >= THERMAL_NAME_LENGTH) {
> + pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
> + tzdp->name, THERMAL_NAME_LENGTH);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + mutex_lock(&thermal_list_lock);
> + list_for_each_entry(pos, &thermal_tz_list, node)
> + if (!strncasecmp(tzdp->name, pos->name, THERMAL_NAME_LENGTH)) {
> + result = -EEXIST;
> + break;
> + }
> + mutex_unlock(&thermal_list_lock);
> +
> + if (result) {
> + pr_err("Thermal zone name (%s) already exists and must be unique\n",
> + tzdp->name);
> + return ERR_PTR(result);
> + }
Perhaps a lookup function would be more adequate. What about reusing
thermal_zone_get_by_name() and search with tz->name if it is !NULL,
tz->type otherwise ?
> + }
> +
> /*
> * Max trip count can't exceed 31 as the "mask >> num_trips" condition.
> * For example, shifting by 32 will result in compiler warning:
> @@ -1307,6 +1332,9 @@ struct thermal_zone_device *thermal_zone_device_register(struct thermal_zone_dev
> tz->id = id;
> strscpy(tz->type, tzdp->type, sizeof(tz->type));
>
> + if (tz_name_len)
> + strscpy(tz->name, tzdp->name, sizeof(tzdp->name));
> +
> if (!tzdp->ops->critical)
> tzdp->ops->critical = thermal_zone_device_critical;
>
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index 62a903ad649f..eaacc140abeb 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -486,6 +486,7 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node *
> ret = PTR_ERR(np);
> goto out_kfree_of_ops;
> }
> + tzdp.name = np->name;
> tzdp.type = np->name;
>
> tzdp.trips = thermal_of_trips_init(np, &tzdp.num_trips);
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index f52af8a3b4b5..f4002fa6caa2 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -23,6 +23,14 @@
>
> /* sys I/F for thermal zone */
>
> +static ssize_t
> +name_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->name);
> +}
> +
> static ssize_t
> type_show(struct device *dev, struct device_attribute *attr, char *buf)
> {
> @@ -341,6 +349,7 @@ create_s32_tzp_attr(offset);
> * All the attributes created for tzp (create_s32_tzp_attr) also are always
> * present on the sysfs interface.
> */
> +static DEVICE_ATTR_RO(name);
> static DEVICE_ATTR_RO(type);
> static DEVICE_ATTR_RO(temp);
> static DEVICE_ATTR_RW(policy);
> diff --git a/drivers/thermal/thermal_trace.h b/drivers/thermal/thermal_trace.h
> index 459c8ce6cf3b..c9dbae1e3b9e 100644
> --- a/drivers/thermal/thermal_trace.h
> +++ b/drivers/thermal/thermal_trace.h
> @@ -28,6 +28,7 @@ TRACE_EVENT(thermal_temperature,
> TP_ARGS(tz),
>
> TP_STRUCT__entry(
> + __string(thermal_zone_name, tz->name)
> __string(thermal_zone, tz->type)
> __field(int, id)
> __field(int, temp_prev)
> @@ -35,15 +36,16 @@ TRACE_EVENT(thermal_temperature,
> ),
>
> TP_fast_assign(
> + __assign_str(thermal_zone_name, tz->name);
> __assign_str(thermal_zone, tz->type);
> __entry->id = tz->id;
> __entry->temp_prev = tz->last_temperature;
> __entry->temp = tz->temperature;
> ),
>
> - TP_printk("thermal_zone=%s id=%d temp_prev=%d temp=%d",
> - __get_str(thermal_zone), __entry->id, __entry->temp_prev,
> - __entry->temp)
> + TP_printk("thermal_zone=%s name=%s id=%d temp_prev=%d temp=%d",
> + __get_str(thermal_zone), __get_str(thermal_zone_name),
> + __entry->id, __entry->temp_prev, __entry->temp)
> );
>
> TRACE_EVENT(cdev_update,
> @@ -73,6 +75,7 @@ TRACE_EVENT(thermal_zone_trip,
> TP_ARGS(tz, trip, trip_type),
>
> TP_STRUCT__entry(
> + __string(thermal_zone_name, tz->name)
> __string(thermal_zone, tz->type)
> __field(int, id)
> __field(int, trip)
> @@ -80,15 +83,17 @@ TRACE_EVENT(thermal_zone_trip,
> ),
>
> TP_fast_assign(
> + __assign_str(thermal_zone_name, tz->name);
> __assign_str(thermal_zone, tz->type);
> __entry->id = tz->id;
> __entry->trip = trip;
> __entry->trip_type = trip_type;
> ),
>
> - TP_printk("thermal_zone=%s id=%d trip=%d trip_type=%s",
> - __get_str(thermal_zone), __entry->id, __entry->trip,
> - show_tzt_type(__entry->trip_type))
> + TP_printk("thermal_zone=%s name=%s id=%d trip=%d trip_type=%s",
> + __get_str(thermal_zone), __get_str(thermal_zone_name),
> + __entry->id, __entry->trip,
> + show_tzt_type(__entry->trip_type))
> );
For now, I think we can keep the traces as they are and keep passing the
tz->type. Then we can replace tz->type by tz->name without changing the
trace format.
> #ifdef CONFIG_CPU_THERMAL
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 84b62575d93a..a06521eda162 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -115,6 +115,7 @@ struct thermal_cooling_device {
> /**
> * struct thermal_zone_device - structure for a thermal zone
> * @id: unique id number for each thermal zone
> + * @name: name of the thermal zone device
> * @type: the thermal zone device type
> * @device: &struct device for this thermal zone
> * @removal: removal completion
> @@ -155,6 +156,7 @@ struct thermal_cooling_device {
> */
> struct thermal_zone_device {
> int id;
> + char name[THERMAL_NAME_LENGTH];
> char type[THERMAL_NAME_LENGTH];
> struct device device;
> struct completion removal;
> @@ -260,6 +262,7 @@ struct thermal_zone_params {
>
> /**
> * struct thermal_zone_device_params - parameters for a thermal zone device
> + * @name: name of the thermal zone device
> * @type: the thermal zone device type
> * @tzp: thermal zone platform parameters
> * @ops: standard thermal zone device callbacks
> @@ -274,6 +277,7 @@ struct thermal_zone_params {
> * driven systems)
> */
> struct thermal_zone_device_params {
> + const char *name;
> const char *type;
> struct thermal_zone_params tzp;
> struct thermal_zone_device_ops *ops;
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [RFC PATCH 26/26] thermal: Introduce thermal zones names
2024-01-16 9:14 ` Daniel Lezcano
@ 2024-01-16 9:45 ` AngeloGioacchino Del Regno
2024-01-16 11:30 ` Daniel Lezcano
0 siblings, 1 reply; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-01-16 9:45 UTC (permalink / raw)
To: Daniel Lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel, kernel
Il 16/01/24 10:14, Daniel Lezcano ha scritto:
> On 21/12/2023 13:48, AngeloGioacchino Del Regno wrote:
>> Currently thermal zones have a "type" but this is often used, and
>> referenced, as a name instead in multiple kernel drivers that are
>> either registering a zone or grabbing a thermal zone handle and
>> unfortunately this is a kind of abuse/misuse of the thermal zone
>> concept of "type".
>>
>> In order to disambiguate name<->type and to actually provide an
>> accepted way of giving a specific name to a thermal zone for both
>> platform drivers and devicetree-defined zones, add a new "name"
>> member in the main thermal_zone_device structure, and also to the
>> thermal_zone_device_params structure which is used to register a
>> thermal zone device.
>>
>> This will enforce the following constraints:
>> - Multiple thermal zones may be of the same "type" (no change);
>> - A thermal zone may have a *unique* name: trying to register
>> a new zone with the same name as an already present one will
>> produce a failure;
>> ---
>> drivers/thermal/thermal_core.c | 34 ++++++++++++++++++++++++++++++---
>> drivers/thermal/thermal_of.c | 1 +
>> drivers/thermal/thermal_sysfs.c | 9 +++++++++
>> drivers/thermal/thermal_trace.h | 17 +++++++++++------
>> include/linux/thermal.h | 4 ++++
>> 5 files changed, 56 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>> index 9eb0200a85ff..adf2ac8113e1 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -1238,8 +1238,8 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp);
>> struct thermal_zone_device *thermal_zone_device_register(struct
>> thermal_zone_device_params *tzdp)
>> {
>> struct thermal_zone_device *tz;
>> - int id;
>> - int result;
>> + int id, tz_name_len;
>> + int result = 0;
>> struct thermal_governor *governor;
>> if (!tzdp->type || strlen(tzdp->type) == 0) {
>> @@ -1248,11 +1248,36 @@ struct thermal_zone_device
>> *thermal_zone_device_register(struct thermal_zone_dev
>> }
>> if (strlen(tzdp->type) >= THERMAL_NAME_LENGTH) {
>> - pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
>> + pr_err("Thermal zone type (%s) too long, should be under %d chars\n",
>> tzdp->type, THERMAL_NAME_LENGTH);
>> return ERR_PTR(-EINVAL);
>
> I would keep that as is and do second round of changes to clarify the usage of ->type
>
Problem is, if we keep this one as-is, then we'll have ambiguous error messages in
this series... unless we stop limiting the tz type string to THERMAL_NAME_LENGTH
as well as not limit the tz name to that...
>> }
>> + tz_name_len = tzdp->name ? strlen(tzdp->name) : 0;
>
> I suggest to change to a const char * and no longer limit to THERMAL_NAME_LENGTH.
...and to be completely honest, I actually like the THERMAL_NAME_LENGTH limitation,
because this both limits the length of the related sysfs file and avoids prolonging
error messages with very-very-very long type-strings and name-strings.
What I have in my head is:
imagine having a thermal zone named "cpu-little-bottom-right-core0" and a driver
named "qualcomm-power-controller" (which doesn't exist, but there are drivers with
pretty long names in the kernel anyway).
Kernel logging *may* have very long strings, decreasing readability:
[ 0.0000100 ] qualcomm-power-controller: cpu-little-bottom-right-core0: Failed to
read thermal zone temperature -22
And sysfs would have something like
cpu-little-top-right-core0 cpu-little-top-left-core0 cpu-little-bottom-right-core0
While sysfs could be ok, are we sure that allowing such long names is a good idea?
It's a genuine question - I don't really have more than just cosmetic reasons and
those are just only personal perspectives....
>
>> + if (tz_name_len) {
>> + struct thermal_zone_device *pos;
>> +
>> + if (tz_name_len >= THERMAL_NAME_LENGTH) {
>> + pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
>> + tzdp->name, THERMAL_NAME_LENGTH);
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + mutex_lock(&thermal_list_lock);
>> + list_for_each_entry(pos, &thermal_tz_list, node)
>> + if (!strncasecmp(tzdp->name, pos->name, THERMAL_NAME_LENGTH)) {
>> + result = -EEXIST;
>> + break;
>> + }
>> + mutex_unlock(&thermal_list_lock);
>> +
>> + if (result) {
>> + pr_err("Thermal zone name (%s) already exists and must be unique\n",
>> + tzdp->name);
>> + return ERR_PTR(result);
>> + }
>
> Perhaps a lookup function would be more adequate. What about reusing
> thermal_zone_get_by_name() and search with tz->name if it is !NULL, tz->type
> otherwise ?
>
Okay yes that makes a lot of sense, and also breaks some of my brain loops around
making the migration a bit less painful.
Nice one! Will do :-D
>> + }
>> +
>> /*
>> * Max trip count can't exceed 31 as the "mask >> num_trips" condition.
>> * For example, shifting by 32 will result in compiler warning:
>> @@ -1307,6 +1332,9 @@ struct thermal_zone_device
>> *thermal_zone_device_register(struct thermal_zone_dev
>> tz->id = id;
>> strscpy(tz->type, tzdp->type, sizeof(tz->type));
>> + if (tz_name_len)
>> + strscpy(tz->name, tzdp->name, sizeof(tzdp->name));
>> +
>> if (!tzdp->ops->critical)
>> tzdp->ops->critical = thermal_zone_device_critical;
>> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
>> index 62a903ad649f..eaacc140abeb 100644
>> --- a/drivers/thermal/thermal_of.c
>> +++ b/drivers/thermal/thermal_of.c
>> @@ -486,6 +486,7 @@ static struct thermal_zone_device
>> *thermal_of_zone_register(struct device_node *
>> ret = PTR_ERR(np);
>> goto out_kfree_of_ops;
>> }
>> + tzdp.name = np->name;
>> tzdp.type = np->name;
>> tzdp.trips = thermal_of_trips_init(np, &tzdp.num_trips);
>> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
>> index f52af8a3b4b5..f4002fa6caa2 100644
>> --- a/drivers/thermal/thermal_sysfs.c
>> +++ b/drivers/thermal/thermal_sysfs.c
>> @@ -23,6 +23,14 @@
>> /* sys I/F for thermal zone */
>> +static ssize_t
>> +name_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->name);
>> +}
>> +
>> static ssize_t
>> type_show(struct device *dev, struct device_attribute *attr, char *buf)
>> {
>> @@ -341,6 +349,7 @@ create_s32_tzp_attr(offset);
>> * All the attributes created for tzp (create_s32_tzp_attr) also are always
>> * present on the sysfs interface.
>> */
>> +static DEVICE_ATTR_RO(name);
>> static DEVICE_ATTR_RO(type);
>> static DEVICE_ATTR_RO(temp);
>> static DEVICE_ATTR_RW(policy);
>> diff --git a/drivers/thermal/thermal_trace.h b/drivers/thermal/thermal_trace.h
>> index 459c8ce6cf3b..c9dbae1e3b9e 100644
>> --- a/drivers/thermal/thermal_trace.h
>> +++ b/drivers/thermal/thermal_trace.h
>> @@ -28,6 +28,7 @@ TRACE_EVENT(thermal_temperature,
>> TP_ARGS(tz),
>> TP_STRUCT__entry(
>> + __string(thermal_zone_name, tz->name)
>> __string(thermal_zone, tz->type)
>> __field(int, id)
>> __field(int, temp_prev)
>> @@ -35,15 +36,16 @@ TRACE_EVENT(thermal_temperature,
>> ),
>> TP_fast_assign(
>> + __assign_str(thermal_zone_name, tz->name);
>> __assign_str(thermal_zone, tz->type);
>> __entry->id = tz->id;
>> __entry->temp_prev = tz->last_temperature;
>> __entry->temp = tz->temperature;
>> ),
>> - TP_printk("thermal_zone=%s id=%d temp_prev=%d temp=%d",
>> - __get_str(thermal_zone), __entry->id, __entry->temp_prev,
>> - __entry->temp)
>> + TP_printk("thermal_zone=%s name=%s id=%d temp_prev=%d temp=%d",
>> + __get_str(thermal_zone), __get_str(thermal_zone_name),
>> + __entry->id, __entry->temp_prev, __entry->temp)
>> );
>> TRACE_EVENT(cdev_update,
>> @@ -73,6 +75,7 @@ TRACE_EVENT(thermal_zone_trip,
>> TP_ARGS(tz, trip, trip_type),
>> TP_STRUCT__entry(
>> + __string(thermal_zone_name, tz->name)
>> __string(thermal_zone, tz->type)
>> __field(int, id)
>> __field(int, trip)
>> @@ -80,15 +83,17 @@ TRACE_EVENT(thermal_zone_trip,
>> ),
>> TP_fast_assign(
>> + __assign_str(thermal_zone_name, tz->name);
>> __assign_str(thermal_zone, tz->type);
>> __entry->id = tz->id;
>> __entry->trip = trip;
>> __entry->trip_type = trip_type;
>> ),
>> - TP_printk("thermal_zone=%s id=%d trip=%d trip_type=%s",
>> - __get_str(thermal_zone), __entry->id, __entry->trip,
>> - show_tzt_type(__entry->trip_type))
>> + TP_printk("thermal_zone=%s name=%s id=%d trip=%d trip_type=%s",
>> + __get_str(thermal_zone), __get_str(thermal_zone_name),
>> + __entry->id, __entry->trip,
>> + show_tzt_type(__entry->trip_type))
>> );
>
> For now, I think we can keep the traces as they are and keep passing the tz->type.
> Then we can replace tz->type by tz->name without changing the trace format.
>
We can but, as a personal consideration, this looks "more complete" - as in - we
are not dropping the thermal zone *type* concept anyway (even though we're doing
"something else" with it), so including both type and name in tracing is useful
to whoever is trying to debug something.
If you have strong opinions against, though, it literally takes 30 seconds for me
to just remove that part and there's no problem in doing so!
Cheers,
Angelo
>> #ifdef CONFIG_CPU_THERMAL
>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> index 84b62575d93a..a06521eda162 100644
>> --- a/include/linux/thermal.h
>> +++ b/include/linux/thermal.h
>> @@ -115,6 +115,7 @@ struct thermal_cooling_device {
>> /**
>> * struct thermal_zone_device - structure for a thermal zone
>> * @id: unique id number for each thermal zone
>> + * @name: name of the thermal zone device
>> * @type: the thermal zone device type
>> * @device: &struct device for this thermal zone
>> * @removal: removal completion
>> @@ -155,6 +156,7 @@ struct thermal_cooling_device {
>> */
>> struct thermal_zone_device {
>> int id;
>> + char name[THERMAL_NAME_LENGTH];
>> char type[THERMAL_NAME_LENGTH];
>> struct device device;
>> struct completion removal;
>> @@ -260,6 +262,7 @@ struct thermal_zone_params {
>> /**
>> * struct thermal_zone_device_params - parameters for a thermal zone device
>> + * @name: name of the thermal zone device
>> * @type: the thermal zone device type
>> * @tzp: thermal zone platform parameters
>> * @ops: standard thermal zone device callbacks
>> @@ -274,6 +277,7 @@ struct thermal_zone_params {
>> * driven systems)
>> */
>> struct thermal_zone_device_params {
>> + const char *name;
>> const char *type;
>> struct thermal_zone_params tzp;
>> struct thermal_zone_device_ops *ops;
>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [RFC PATCH 26/26] thermal: Introduce thermal zones names
2024-01-16 9:45 ` AngeloGioacchino Del Regno
@ 2024-01-16 11:30 ` Daniel Lezcano
2024-01-16 11:37 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 42+ messages in thread
From: Daniel Lezcano @ 2024-01-16 11:30 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel, kernel
On 16/01/2024 10:45, AngeloGioacchino Del Regno wrote:
> Il 16/01/24 10:14, Daniel Lezcano ha scritto:
>> On 21/12/2023 13:48, AngeloGioacchino Del Regno wrote:
>>> Currently thermal zones have a "type" but this is often used, and
>>> referenced, as a name instead in multiple kernel drivers that are
>>> either registering a zone or grabbing a thermal zone handle and
>>> unfortunately this is a kind of abuse/misuse of the thermal zone
>>> concept of "type".
>>>
>>> In order to disambiguate name<->type and to actually provide an
>>> accepted way of giving a specific name to a thermal zone for both
>>> platform drivers and devicetree-defined zones, add a new "name"
>>> member in the main thermal_zone_device structure, and also to the
>>> thermal_zone_device_params structure which is used to register a
>>> thermal zone device.
>>>
>>> This will enforce the following constraints:
>>> - Multiple thermal zones may be of the same "type" (no change);
>>> - A thermal zone may have a *unique* name: trying to register
>>> a new zone with the same name as an already present one will
>>> produce a failure;
>>> ---
>>> drivers/thermal/thermal_core.c | 34 ++++++++++++++++++++++++++++++---
>>> drivers/thermal/thermal_of.c | 1 +
>>> drivers/thermal/thermal_sysfs.c | 9 +++++++++
>>> drivers/thermal/thermal_trace.h | 17 +++++++++++------
>>> include/linux/thermal.h | 4 ++++
>>> 5 files changed, 56 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/thermal/thermal_core.c
>>> b/drivers/thermal/thermal_core.c
>>> index 9eb0200a85ff..adf2ac8113e1 100644
>>> --- a/drivers/thermal/thermal_core.c
>>> +++ b/drivers/thermal/thermal_core.c
>>> @@ -1238,8 +1238,8 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp);
>>> struct thermal_zone_device *thermal_zone_device_register(struct
>>> thermal_zone_device_params *tzdp)
>>> {
>>> struct thermal_zone_device *tz;
>>> - int id;
>>> - int result;
>>> + int id, tz_name_len;
>>> + int result = 0;
>>> struct thermal_governor *governor;
>>> if (!tzdp->type || strlen(tzdp->type) == 0) {
>>> @@ -1248,11 +1248,36 @@ struct thermal_zone_device
>>> *thermal_zone_device_register(struct thermal_zone_dev
>>> }
>>> if (strlen(tzdp->type) >= THERMAL_NAME_LENGTH) {
>>> - pr_err("Thermal zone name (%s) too long, should be under %d
>>> chars\n",
>>> + pr_err("Thermal zone type (%s) too long, should be under %d
>>> chars\n",
>>> tzdp->type, THERMAL_NAME_LENGTH);
>>> return ERR_PTR(-EINVAL);
>>
>> I would keep that as is and do second round of changes to clarify the
>> usage of ->type
>>
>
> Problem is, if we keep this one as-is, then we'll have ambiguous error
> messages in
> this series... unless we stop limiting the tz type string to
> THERMAL_NAME_LENGTH
> as well as not limit the tz name to that...
I'm missing the point, how can it be more ambiguous if the message is
unchanged ?
>>> }
>>> + tz_name_len = tzdp->name ? strlen(tzdp->name) : 0;
>>
>> I suggest to change to a const char * and no longer limit to
>> THERMAL_NAME_LENGTH.
>
> ...and to be completely honest, I actually like the THERMAL_NAME_LENGTH
> limitation,
> because this both limits the length of the related sysfs file and avoids
> prolonging
> error messages with very-very-very long type-strings and name-strings.
>
> What I have in my head is:
>
> imagine having a thermal zone named "cpu-little-bottom-right-core0" and
> a driver
> named "qualcomm-power-controller" (which doesn't exist, but there are
> drivers with
> pretty long names in the kernel anyway).
>
> Kernel logging *may* have very long strings, decreasing readability:
> [ 0.0000100 ] qualcomm-power-controller: cpu-little-bottom-right-core0:
> Failed to read thermal zone temperature -22
>
> And sysfs would have something like
> cpu-little-top-right-core0 cpu-little-top-left-core0
> cpu-little-bottom-right-core0
>
> While sysfs could be ok, are we sure that allowing such long names is a
> good idea?
>
> It's a genuine question - I don't really have more than just cosmetic
> reasons and
> those are just only personal perspectives....
IMO, it is up to the programmer to choose a convenient name.
If the traces are giving unreadable output, then someone will send a
patch to change the name to something more readable.
In addition, having array in structure to be passed as parameter is not
good because of the limited stack size in the kernel.
>>> + if (tz_name_len) {
>>> + struct thermal_zone_device *pos;
>>> +
>>> + if (tz_name_len >= THERMAL_NAME_LENGTH) {
>>> + pr_err("Thermal zone name (%s) too long, should be under
>>> %d chars\n",
>>> + tzdp->name, THERMAL_NAME_LENGTH);
>>> + return ERR_PTR(-EINVAL);
>>> + }
>>> +
>>> + mutex_lock(&thermal_list_lock);
>>> + list_for_each_entry(pos, &thermal_tz_list, node)
>>> + if (!strncasecmp(tzdp->name, pos->name,
>>> THERMAL_NAME_LENGTH)) {
>>> + result = -EEXIST;
>>> + break;
>>> + }
>>> + mutex_unlock(&thermal_list_lock);
>>> +
>>> + if (result) {
>>> + pr_err("Thermal zone name (%s) already exists and must
>>> be unique\n",
>>> + tzdp->name);
>>> + return ERR_PTR(result);
>>> + }
>>
>> Perhaps a lookup function would be more adequate. What about reusing
>> thermal_zone_get_by_name() and search with tz->name if it is !NULL,
>> tz->type otherwise ?
>>
>
> Okay yes that makes a lot of sense, and also breaks some of my brain
> loops around
> making the migration a bit less painful.
>
> Nice one! Will do :-D
>
>>> + }
>>> +
>>> /*
>>> * Max trip count can't exceed 31 as the "mask >> num_trips"
>>> condition.
>>> * For example, shifting by 32 will result in compiler warning:
>>> @@ -1307,6 +1332,9 @@ struct thermal_zone_device
>>> *thermal_zone_device_register(struct thermal_zone_dev
>>> tz->id = id;
>>> strscpy(tz->type, tzdp->type, sizeof(tz->type));
>>> + if (tz_name_len)
>>> + strscpy(tz->name, tzdp->name, sizeof(tzdp->name));
>>> +
>>> if (!tzdp->ops->critical)
>>> tzdp->ops->critical = thermal_zone_device_critical;
>>> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
>>> index 62a903ad649f..eaacc140abeb 100644
>>> --- a/drivers/thermal/thermal_of.c
>>> +++ b/drivers/thermal/thermal_of.c
>>> @@ -486,6 +486,7 @@ static struct thermal_zone_device
>>> *thermal_of_zone_register(struct device_node *
>>> ret = PTR_ERR(np);
>>> goto out_kfree_of_ops;
>>> }
>>> + tzdp.name = np->name;
>>> tzdp.type = np->name;
>>> tzdp.trips = thermal_of_trips_init(np, &tzdp.num_trips);
>>> diff --git a/drivers/thermal/thermal_sysfs.c
>>> b/drivers/thermal/thermal_sysfs.c
>>> index f52af8a3b4b5..f4002fa6caa2 100644
>>> --- a/drivers/thermal/thermal_sysfs.c
>>> +++ b/drivers/thermal/thermal_sysfs.c
>>> @@ -23,6 +23,14 @@
>>> /* sys I/F for thermal zone */
>>> +static ssize_t
>>> +name_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->name);
>>> +}
>>> +
>>> static ssize_t
>>> type_show(struct device *dev, struct device_attribute *attr, char
>>> *buf)
>>> {
>>> @@ -341,6 +349,7 @@ create_s32_tzp_attr(offset);
>>> * All the attributes created for tzp (create_s32_tzp_attr) also
>>> are always
>>> * present on the sysfs interface.
>>> */
>>> +static DEVICE_ATTR_RO(name);
>>> static DEVICE_ATTR_RO(type);
>>> static DEVICE_ATTR_RO(temp);
>>> static DEVICE_ATTR_RW(policy);
>>> diff --git a/drivers/thermal/thermal_trace.h
>>> b/drivers/thermal/thermal_trace.h
>>> index 459c8ce6cf3b..c9dbae1e3b9e 100644
>>> --- a/drivers/thermal/thermal_trace.h
>>> +++ b/drivers/thermal/thermal_trace.h
>>> @@ -28,6 +28,7 @@ TRACE_EVENT(thermal_temperature,
>>> TP_ARGS(tz),
>>> TP_STRUCT__entry(
>>> + __string(thermal_zone_name, tz->name)
>>> __string(thermal_zone, tz->type)
>>> __field(int, id)
>>> __field(int, temp_prev)
>>> @@ -35,15 +36,16 @@ TRACE_EVENT(thermal_temperature,
>>> ),
>>> TP_fast_assign(
>>> + __assign_str(thermal_zone_name, tz->name);
>>> __assign_str(thermal_zone, tz->type);
>>> __entry->id = tz->id;
>>> __entry->temp_prev = tz->last_temperature;
>>> __entry->temp = tz->temperature;
>>> ),
>>> - TP_printk("thermal_zone=%s id=%d temp_prev=%d temp=%d",
>>> - __get_str(thermal_zone), __entry->id, __entry->temp_prev,
>>> - __entry->temp)
>>> + TP_printk("thermal_zone=%s name=%s id=%d temp_prev=%d temp=%d",
>>> + __get_str(thermal_zone), __get_str(thermal_zone_name),
>>> + __entry->id, __entry->temp_prev, __entry->temp)
>>> );
>>> TRACE_EVENT(cdev_update,
>>> @@ -73,6 +75,7 @@ TRACE_EVENT(thermal_zone_trip,
>>> TP_ARGS(tz, trip, trip_type),
>>> TP_STRUCT__entry(
>>> + __string(thermal_zone_name, tz->name)
>>> __string(thermal_zone, tz->type)
>>> __field(int, id)
>>> __field(int, trip)
>>> @@ -80,15 +83,17 @@ TRACE_EVENT(thermal_zone_trip,
>>> ),
>>> TP_fast_assign(
>>> + __assign_str(thermal_zone_name, tz->name);
>>> __assign_str(thermal_zone, tz->type);
>>> __entry->id = tz->id;
>>> __entry->trip = trip;
>>> __entry->trip_type = trip_type;
>>> ),
>>> - TP_printk("thermal_zone=%s id=%d trip=%d trip_type=%s",
>>> - __get_str(thermal_zone), __entry->id, __entry->trip,
>>> - show_tzt_type(__entry->trip_type))
>>> + TP_printk("thermal_zone=%s name=%s id=%d trip=%d trip_type=%s",
>>> + __get_str(thermal_zone), __get_str(thermal_zone_name),
>>> + __entry->id, __entry->trip,
>>> + show_tzt_type(__entry->trip_type))
>>> );
>>
>> For now, I think we can keep the traces as they are and keep passing
>> the tz->type. Then we can replace tz->type by tz->name without
>> changing the trace format.
>>
>
> We can but, as a personal consideration, this looks "more complete" - as
> in - we
> are not dropping the thermal zone *type* concept anyway (even though
> we're doing
> "something else" with it), so including both type and name in tracing is
> useful
> to whoever is trying to debug something.
>
> If you have strong opinions against, though, it literally takes 30
> seconds for me
> to just remove that part and there's no problem in doing so!
Yes, just drop it for now. We will sort it out after.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [RFC PATCH 26/26] thermal: Introduce thermal zones names
2024-01-16 11:30 ` Daniel Lezcano
@ 2024-01-16 11:37 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-01-16 11:37 UTC (permalink / raw)
To: Daniel Lezcano
Cc: rafael, rui.zhang, lukasz.luba, linux-pm, linux-kernel, kernel
Il 16/01/24 12:30, Daniel Lezcano ha scritto:
> On 16/01/2024 10:45, AngeloGioacchino Del Regno wrote:
>> Il 16/01/24 10:14, Daniel Lezcano ha scritto:
>>> On 21/12/2023 13:48, AngeloGioacchino Del Regno wrote:
>>>> Currently thermal zones have a "type" but this is often used, and
>>>> referenced, as a name instead in multiple kernel drivers that are
>>>> either registering a zone or grabbing a thermal zone handle and
>>>> unfortunately this is a kind of abuse/misuse of the thermal zone
>>>> concept of "type".
>>>>
>>>> In order to disambiguate name<->type and to actually provide an
>>>> accepted way of giving a specific name to a thermal zone for both
>>>> platform drivers and devicetree-defined zones, add a new "name"
>>>> member in the main thermal_zone_device structure, and also to the
>>>> thermal_zone_device_params structure which is used to register a
>>>> thermal zone device.
>>>>
>>>> This will enforce the following constraints:
>>>> - Multiple thermal zones may be of the same "type" (no change);
>>>> - A thermal zone may have a *unique* name: trying to register
>>>> a new zone with the same name as an already present one will
>>>> produce a failure;
>>>> ---
>>>> drivers/thermal/thermal_core.c | 34 ++++++++++++++++++++++++++++++---
>>>> drivers/thermal/thermal_of.c | 1 +
>>>> drivers/thermal/thermal_sysfs.c | 9 +++++++++
>>>> drivers/thermal/thermal_trace.h | 17 +++++++++++------
>>>> include/linux/thermal.h | 4 ++++
>>>> 5 files changed, 56 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>>>> index 9eb0200a85ff..adf2ac8113e1 100644
>>>> --- a/drivers/thermal/thermal_core.c
>>>> +++ b/drivers/thermal/thermal_core.c
>>>> @@ -1238,8 +1238,8 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp);
>>>> struct thermal_zone_device *thermal_zone_device_register(struct
>>>> thermal_zone_device_params *tzdp)
>>>> {
>>>> struct thermal_zone_device *tz;
>>>> - int id;
>>>> - int result;
>>>> + int id, tz_name_len;
>>>> + int result = 0;
>>>> struct thermal_governor *governor;
>>>> if (!tzdp->type || strlen(tzdp->type) == 0) {
>>>> @@ -1248,11 +1248,36 @@ struct thermal_zone_device
>>>> *thermal_zone_device_register(struct thermal_zone_dev
>>>> }
>>>> if (strlen(tzdp->type) >= THERMAL_NAME_LENGTH) {
>>>> - pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
>>>> + pr_err("Thermal zone type (%s) too long, should be under %d chars\n",
>>>> tzdp->type, THERMAL_NAME_LENGTH);
>>>> return ERR_PTR(-EINVAL);
>>>
>>> I would keep that as is and do second round of changes to clarify the usage of
>>> ->type
>>>
>>
>> Problem is, if we keep this one as-is, then we'll have ambiguous error messages in
>> this series... unless we stop limiting the tz type string to THERMAL_NAME_LENGTH
>> as well as not limit the tz name to that...
>
> I'm missing the point, how can it be more ambiguous if the message is unchanged ?
>
Because:
if (strlen(tzdp->type) >= THERMAL_NAME_LENGTH) {
pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
.......
+ if (tz_name_len >= THERMAL_NAME_LENGTH) {
+ pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
+ tzdp->name, THERMAL_NAME_LENGTH);
+ return ERR_PTR(-EINVAL);
+ }
...but anyway, since we're removing the THERMAL_NAME_LENGTH limit, those messages
are going away with it - this means that this is not a problem anymore.
Nevermind :-)
>>>> }
>>>> + tz_name_len = tzdp->name ? strlen(tzdp->name) : 0;
>>>
>>> I suggest to change to a const char * and no longer limit to THERMAL_NAME_LENGTH.
>>
>> ...and to be completely honest, I actually like the THERMAL_NAME_LENGTH limitation,
>> because this both limits the length of the related sysfs file and avoids prolonging
>> error messages with very-very-very long type-strings and name-strings.
>>
>> What I have in my head is:
>>
>> imagine having a thermal zone named "cpu-little-bottom-right-core0" and a driver
>> named "qualcomm-power-controller" (which doesn't exist, but there are drivers with
>> pretty long names in the kernel anyway).
>>
>> Kernel logging *may* have very long strings, decreasing readability:
>> [ 0.0000100 ] qualcomm-power-controller: cpu-little-bottom-right-core0: Failed to
>> read thermal zone temperature -22
>>
>> And sysfs would have something like
>> cpu-little-top-right-core0 cpu-little-top-left-core0 cpu-little-bottom-right-core0
>>
>> While sysfs could be ok, are we sure that allowing such long names is a good idea?
>>
>> It's a genuine question - I don't really have more than just cosmetic reasons and
>> those are just only personal perspectives....
>
> IMO, it is up to the programmer to choose a convenient name.
>
> If the traces are giving unreadable output, then someone will send a patch to
> change the name to something more readable.
>
> In addition, having array in structure to be passed as parameter is not good
> because of the limited stack size in the kernel.
>
>
Okay, will do then.
>>>> + if (tz_name_len) {
>>>> + struct thermal_zone_device *pos;
>>>> +
>>>> + if (tz_name_len >= THERMAL_NAME_LENGTH) {
>>>> + pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
>>>> + tzdp->name, THERMAL_NAME_LENGTH);
>>>> + return ERR_PTR(-EINVAL);
>>>> + }
>>>> +
>>>> + mutex_lock(&thermal_list_lock);
>>>> + list_for_each_entry(pos, &thermal_tz_list, node)
>>>> + if (!strncasecmp(tzdp->name, pos->name, THERMAL_NAME_LENGTH)) {
>>>> + result = -EEXIST;
>>>> + break;
>>>> + }
>>>> + mutex_unlock(&thermal_list_lock);
>>>> +
>>>> + if (result) {
>>>> + pr_err("Thermal zone name (%s) already exists and must be unique\n",
>>>> + tzdp->name);
>>>> + return ERR_PTR(result);
>>>> + }
>>>
>>> Perhaps a lookup function would be more adequate. What about reusing
>>> thermal_zone_get_by_name() and search with tz->name if it is !NULL, tz->type
>>> otherwise ?
>>>
>>
>> Okay yes that makes a lot of sense, and also breaks some of my brain loops around
>> making the migration a bit less painful.
>>
>> Nice one! Will do :-D
>>
>>>> + }
>>>> +
>>>> /*
>>>> * Max trip count can't exceed 31 as the "mask >> num_trips" condition.
>>>> * For example, shifting by 32 will result in compiler warning:
>>>> @@ -1307,6 +1332,9 @@ struct thermal_zone_device
>>>> *thermal_zone_device_register(struct thermal_zone_dev
>>>> tz->id = id;
>>>> strscpy(tz->type, tzdp->type, sizeof(tz->type));
>>>> + if (tz_name_len)
>>>> + strscpy(tz->name, tzdp->name, sizeof(tzdp->name));
>>>> +
>>>> if (!tzdp->ops->critical)
>>>> tzdp->ops->critical = thermal_zone_device_critical;
>>>> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
>>>> index 62a903ad649f..eaacc140abeb 100644
>>>> --- a/drivers/thermal/thermal_of.c
>>>> +++ b/drivers/thermal/thermal_of.c
>>>> @@ -486,6 +486,7 @@ static struct thermal_zone_device
>>>> *thermal_of_zone_register(struct device_node *
>>>> ret = PTR_ERR(np);
>>>> goto out_kfree_of_ops;
>>>> }
>>>> + tzdp.name = np->name;
>>>> tzdp.type = np->name;
>>>> tzdp.trips = thermal_of_trips_init(np, &tzdp.num_trips);
>>>> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
>>>> index f52af8a3b4b5..f4002fa6caa2 100644
>>>> --- a/drivers/thermal/thermal_sysfs.c
>>>> +++ b/drivers/thermal/thermal_sysfs.c
>>>> @@ -23,6 +23,14 @@
>>>> /* sys I/F for thermal zone */
>>>> +static ssize_t
>>>> +name_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->name);
>>>> +}
>>>> +
>>>> static ssize_t
>>>> type_show(struct device *dev, struct device_attribute *attr, char *buf)
>>>> {
>>>> @@ -341,6 +349,7 @@ create_s32_tzp_attr(offset);
>>>> * All the attributes created for tzp (create_s32_tzp_attr) also are always
>>>> * present on the sysfs interface.
>>>> */
>>>> +static DEVICE_ATTR_RO(name);
>>>> static DEVICE_ATTR_RO(type);
>>>> static DEVICE_ATTR_RO(temp);
>>>> static DEVICE_ATTR_RW(policy);
>>>> diff --git a/drivers/thermal/thermal_trace.h b/drivers/thermal/thermal_trace.h
>>>> index 459c8ce6cf3b..c9dbae1e3b9e 100644
>>>> --- a/drivers/thermal/thermal_trace.h
>>>> +++ b/drivers/thermal/thermal_trace.h
>>>> @@ -28,6 +28,7 @@ TRACE_EVENT(thermal_temperature,
>>>> TP_ARGS(tz),
>>>> TP_STRUCT__entry(
>>>> + __string(thermal_zone_name, tz->name)
>>>> __string(thermal_zone, tz->type)
>>>> __field(int, id)
>>>> __field(int, temp_prev)
>>>> @@ -35,15 +36,16 @@ TRACE_EVENT(thermal_temperature,
>>>> ),
>>>> TP_fast_assign(
>>>> + __assign_str(thermal_zone_name, tz->name);
>>>> __assign_str(thermal_zone, tz->type);
>>>> __entry->id = tz->id;
>>>> __entry->temp_prev = tz->last_temperature;
>>>> __entry->temp = tz->temperature;
>>>> ),
>>>> - TP_printk("thermal_zone=%s id=%d temp_prev=%d temp=%d",
>>>> - __get_str(thermal_zone), __entry->id, __entry->temp_prev,
>>>> - __entry->temp)
>>>> + TP_printk("thermal_zone=%s name=%s id=%d temp_prev=%d temp=%d",
>>>> + __get_str(thermal_zone), __get_str(thermal_zone_name),
>>>> + __entry->id, __entry->temp_prev, __entry->temp)
>>>> );
>>>> TRACE_EVENT(cdev_update,
>>>> @@ -73,6 +75,7 @@ TRACE_EVENT(thermal_zone_trip,
>>>> TP_ARGS(tz, trip, trip_type),
>>>> TP_STRUCT__entry(
>>>> + __string(thermal_zone_name, tz->name)
>>>> __string(thermal_zone, tz->type)
>>>> __field(int, id)
>>>> __field(int, trip)
>>>> @@ -80,15 +83,17 @@ TRACE_EVENT(thermal_zone_trip,
>>>> ),
>>>> TP_fast_assign(
>>>> + __assign_str(thermal_zone_name, tz->name);
>>>> __assign_str(thermal_zone, tz->type);
>>>> __entry->id = tz->id;
>>>> __entry->trip = trip;
>>>> __entry->trip_type = trip_type;
>>>> ),
>>>> - TP_printk("thermal_zone=%s id=%d trip=%d trip_type=%s",
>>>> - __get_str(thermal_zone), __entry->id, __entry->trip,
>>>> - show_tzt_type(__entry->trip_type))
>>>> + TP_printk("thermal_zone=%s name=%s id=%d trip=%d trip_type=%s",
>>>> + __get_str(thermal_zone), __get_str(thermal_zone_name),
>>>> + __entry->id, __entry->trip,
>>>> + show_tzt_type(__entry->trip_type))
>>>> );
>>>
>>> For now, I think we can keep the traces as they are and keep passing the
>>> tz->type. Then we can replace tz->type by tz->name without changing the trace
>>> format.
>>>
>>
>> We can but, as a personal consideration, this looks "more complete" - as in - we
>> are not dropping the thermal zone *type* concept anyway (even though we're doing
>> "something else" with it), so including both type and name in tracing is useful
>> to whoever is trying to debug something.
>>
>> If you have strong opinions against, though, it literally takes 30 seconds for me
>> to just remove that part and there's no problem in doing so!
>
> Yes, just drop it for now. We will sort it out after.
>
Ok I'll drop this part!
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC PATCH 00/26] Add thermal zones names and new registration func
2023-12-21 12:47 [RFC PATCH 00/26] Add thermal zones names and new registration func AngeloGioacchino Del Regno
` (25 preceding siblings ...)
2023-12-21 12:48 ` [RFC PATCH 26/26] thermal: Introduce thermal zones names AngeloGioacchino Del Regno
@ 2023-12-21 13:38 ` Rafael J. Wysocki
2023-12-21 13:54 ` AngeloGioacchino Del Regno
26 siblings, 1 reply; 42+ messages in thread
From: Rafael J. Wysocki @ 2023-12-21 13:38 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: daniel.lezcano, rafael, rui.zhang, lukasz.luba, linux-pm,
linux-kernel, kernel
On Thu, Dec 21, 2023 at 1:48 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> ** 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.
And it is very unlikely that I will be able to provide any useful
feedback on this series before 6.8-rc1 is out.
Thanks!
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [RFC PATCH 00/26] Add thermal zones names and new registration func
2023-12-21 13:38 ` [RFC PATCH 00/26] Add thermal zones names and new registration func Rafael J. Wysocki
@ 2023-12-21 13:54 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 42+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-21 13:54 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: daniel.lezcano, rui.zhang, lukasz.luba, linux-pm, linux-kernel,
kernel
Il 21/12/23 14:38, Rafael J. Wysocki ha scritto:
> On Thu, Dec 21, 2023 at 1:48 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> ** 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.
>
> And it is very unlikely that I will be able to provide any useful
> feedback on this series before 6.8-rc1 is out.
>
> Thanks!
Thanks for telling - but hey, obviously there's no rush, please take your time.
Cheers!
Angelo
^ permalink raw reply [flat|nested] 42+ messages in thread