| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 16 Jan 2024 10:58:04 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Daniel Lezcano <daniel.lezcano@...aro.org>
Cc: rafael@...nel.org, rui.zhang@...el.com, lukasz.luba@....com,
linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org, kernel@...labora.com
Subject: Re: [RFC PATCH 01/26] thermal: Introduce
thermal_zone_device_register() and params structure
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@...labora.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)
>> { }
>
Powered by blists - more mailing lists