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

From: Daniel Lezcano
Date: Mon Jan 15 2024 - 07:40:10 EST

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