Re: [PATCH v1 4/6] thermal: core: Store zone ops in struct thermal_zone_device

Stanislaw Gruszka
2024-02-08T08:48:58Z

On Mon, Feb 05, 2024 at 10:18:02PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The current code requires thermal zone creators to pass pointers to
> writable ops to thermal_zone_device_register_with_trips() which needs
> to modify the target struct thermal_zone_device_ops object if the
> "critical" operation in it is NULL.
>
> Moreover, the callers of thermal_zone_device_register_with_trips() are
> required to hold on to the struct thermal_zone_device_ops object passed
> to it until the given thermal zone is unregistered.
>
> Both of these requirements are quite inconvenient, so modify struct
> thermal_zone_device to contain struct thermal_zone_device_ops as field and
> make thermal_zone_device_register_with_trips() copy the contents of the
> struct thermal_zone_device_ops passed to it via a pointer (which can be
> const now) to that field. Also adjust the code using thermal zone ops
> accordingly.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

> ---
> drivers/thermal/thermal_core.c | 40 +++++++++++++++++++-------------------
> drivers/thermal/thermal_helpers.c | 10 ++++-----
> drivers/thermal/thermal_hwmon.c | 4 +--
> drivers/thermal/thermal_sysfs.c | 12 +++++------
> drivers/thermal/thermal_trip.c | 4 +--
> include/linux/thermal.h | 6 ++---
> 6 files changed, 38 insertions(+), 38 deletions(-)
>
> Index: linux-pm/include/linux/thermal.h
> ===================================================================
> --- linux-pm.orig/include/linux/thermal.h
> +++ linux-pm/include/linux/thermal.h
> @@ -182,7 +182,7 @@ struct thermal_zone_device {
> int prev_low_trip;
> int prev_high_trip;
> atomic_t need_update;
> - struct thermal_zone_device_ops *ops;
> + struct thermal_zone_device_ops ops;
> struct thermal_zone_params *tzp;
> struct thermal_governor *governor;
> void *governor_data;
> @@ -318,14 +318,14 @@ struct thermal_zone_device *thermal_zone
> const struct thermal_trip *trips,
> int num_trips, int mask,
> void *devdata,
> - struct thermal_zone_device_ops *ops,
> + const 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_device_ops *ops,
> const struct thermal_zone_params *tzp);
>
> void thermal_zone_device_unregister(struct thermal_zone_device *tz);
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -356,9 +356,9 @@ static void handle_critical_trips(struct
> trace_thermal_zone_trip(tz, thermal_zone_trip_id(tz, trip), trip->type);
>
> if (trip->type == THERMAL_TRIP_CRITICAL)
> - tz->ops->critical(tz);
> - else if (tz->ops->hot)
> - tz->ops->hot(tz);
> + tz->ops.critical(tz);
> + else if (tz->ops.hot)
> + tz->ops.hot(tz);
> }
>
> static void handle_thermal_trip(struct thermal_zone_device *tz,
> @@ -493,8 +493,8 @@ static int thermal_zone_device_set_mode(
> return ret;
> }
>
> - if (tz->ops->change_mode)
> - ret = tz->ops->change_mode(tz, mode);
> + if (tz->ops.change_mode)
> + ret = tz->ops.change_mode(tz, mode);
>
> if (!ret)
> tz->mode = mode;
> @@ -867,8 +867,8 @@ static void bind_cdev(struct thermal_coo
> struct thermal_zone_device *pos = NULL;
>
> list_for_each_entry(pos, &thermal_tz_list, node) {
> - if (pos->ops->bind) {
> - ret = pos->ops->bind(pos, cdev);
> + if (pos->ops.bind) {
> + ret = pos->ops.bind(pos, cdev);
> if (ret)
> print_bind_err_msg(pos, cdev, ret);
> }
> @@ -1184,8 +1184,8 @@ void thermal_cooling_device_unregister(s
>
> /* Unbind all thermal zones associated with 'this' cdev */
> list_for_each_entry(tz, &thermal_tz_list, node) {
> - if (tz->ops->unbind)
> - tz->ops->unbind(tz, cdev);
> + if (tz->ops.unbind)
> + tz->ops.unbind(tz, cdev);
> }
>
> mutex_unlock(&thermal_list_lock);
> @@ -1199,13 +1199,13 @@ static void bind_tz(struct thermal_zone_
> int ret;
> struct thermal_cooling_device *pos = NULL;
>
> - if (!tz->ops->bind)
> + if (!tz->ops.bind)
> return;
>
> mutex_lock(&thermal_list_lock);
>
> list_for_each_entry(pos, &thermal_cdev_list, node) {
> - ret = tz->ops->bind(tz, pos);
> + ret = tz->ops.bind(tz, pos);
> if (ret)
> print_bind_err_msg(tz, pos, ret);
> }
> @@ -1224,8 +1224,8 @@ int thermal_zone_get_crit_temp(struct th
> {
> int i, ret = -EINVAL;
>
> - if (tz->ops->get_crit_temp)
> - return tz->ops->get_crit_temp(tz, temp);
> + if (tz->ops.get_crit_temp)
> + return tz->ops.get_crit_temp(tz, temp);
>
> if (!tz->trips)
> return -EINVAL;
> @@ -1276,7 +1276,7 @@ thermal_zone_device_register_with_trips(
> const struct thermal_trip *trips,
> int num_trips, int mask,
> void *devdata,
> - struct thermal_zone_device_ops *ops,
> + const struct thermal_zone_device_ops *ops,
> const struct thermal_zone_params *tzp,
> int passive_delay, int polling_delay)
> {
> @@ -1350,10 +1350,10 @@ thermal_zone_device_register_with_trips(
> tz->id = id;
> strscpy(tz->type, type, sizeof(tz->type));
>
> - if (!ops->critical)
> - ops->critical = thermal_zone_device_critical;
> + tz->ops = *ops;
> + if (!tz->ops.critical)
> + tz->ops.critical = thermal_zone_device_critical;
>
> - tz->ops = ops;
> tz->device.class = thermal_class;
> tz->devdata = devdata;
> memcpy(tz->trips, trips, num_trips * sizeof(trips[0]));
> @@ -1439,7 +1439,7 @@ EXPORT_SYMBOL_GPL(thermal_zone_device_re
> struct thermal_zone_device *thermal_tripless_zone_device_register(
> const char *type,
> void *devdata,
> - struct thermal_zone_device_ops *ops,
> + const struct thermal_zone_device_ops *ops,
> const struct thermal_zone_params *tzp)
> {
> return thermal_zone_device_register_with_trips(type, NULL, 0, 0, devdata,
> @@ -1501,8 +1501,8 @@ void thermal_zone_device_unregister(stru
>
> /* Unbind all cdevs associated with 'this' thermal zone */
> list_for_each_entry(cdev, &thermal_cdev_list, node)
> - if (tz->ops->unbind)
> - tz->ops->unbind(tz, cdev);
> + if (tz->ops.unbind)
> + tz->ops.unbind(tz, cdev);
>
> mutex_unlock(&thermal_list_lock);
>
> Index: linux-pm/drivers/thermal/thermal_sysfs.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_sysfs.c
> +++ linux-pm/drivers/thermal/thermal_sysfs.c
> @@ -123,8 +123,8 @@ trip_point_temp_store(struct device *dev
> trip = &tz->trips[trip_id];
>
> if (temp != trip->temperature) {
> - if (tz->ops->set_trip_temp) {
> - ret = tz->ops->set_trip_temp(tz, trip_id, temp);
> + if (tz->ops.set_trip_temp) {
> + ret = tz->ops.set_trip_temp(tz, trip_id, temp);
> if (ret)
> goto unlock;
> }
> @@ -174,8 +174,8 @@ trip_point_hyst_store(struct device *dev
> trip = &tz->trips[trip_id];
>
> if (hyst != trip->hysteresis) {
> - if (tz->ops->set_trip_hyst) {
> - ret = tz->ops->set_trip_hyst(tz, trip_id, hyst);
> + if (tz->ops.set_trip_hyst) {
> + ret = tz->ops.set_trip_hyst(tz, trip_id, hyst);
> if (ret)
> goto unlock;
> }
> @@ -250,10 +250,10 @@ emul_temp_store(struct device *dev, stru
>
> mutex_lock(&tz->lock);
>
> - if (!tz->ops->set_emul_temp)
> + if (!tz->ops.set_emul_temp)
> tz->emul_temperature = temperature;
> else
> - ret = tz->ops->set_emul_temp(tz, temperature);
> + ret = tz->ops.set_emul_temp(tz, temperature);
>
> if (!ret)
> __thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> Index: linux-pm/drivers/thermal/thermal_helpers.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_helpers.c
> +++ linux-pm/drivers/thermal/thermal_helpers.c
> @@ -26,8 +26,8 @@ int get_tz_trend(struct thermal_zone_dev
> {
> enum thermal_trend trend;
>
> - if (tz->emul_temperature || !tz->ops->get_trend ||
> - tz->ops->get_trend(tz, trip, &trend)) {
> + if (tz->emul_temperature || !tz->ops.get_trend ||
> + tz->ops.get_trend(tz, trip, &trend)) {
> if (tz->temperature > tz->last_temperature)
> trend = THERMAL_TREND_RAISING;
> else if (tz->temperature < tz->last_temperature)
> @@ -75,7 +75,7 @@ EXPORT_SYMBOL(get_thermal_instance);
> * temperature and fill @temp.
> *
> * Both tz and tz->ops must be valid pointers when calling this function,
> - * and the tz->ops->get_temp callback must be provided.
> + * and the tz->ops.get_temp callback must be provided.
> * The function must be called under tz->lock.
> *
> * Return: On success returns 0, an error code otherwise
> @@ -88,7 +88,7 @@ int __thermal_zone_get_temp(struct therm
>
> lockdep_assert_held(&tz->lock);
>
> - ret = tz->ops->get_temp(tz, temp);
> + ret = tz->ops.get_temp(tz, temp);
>
> if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz->emul_temperature) {
> for_each_trip(tz, trip) {
> @@ -132,7 +132,7 @@ int thermal_zone_get_temp(struct thermal
>
> mutex_lock(&tz->lock);
>
> - if (!tz->ops->get_temp) {
> + if (!tz->ops.get_temp) {
> ret = -EINVAL;
> goto unlock;
> }
> Index: linux-pm/drivers/thermal/thermal_trip.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_trip.c
> +++ linux-pm/drivers/thermal/thermal_trip.c
> @@ -70,7 +70,7 @@ void __thermal_zone_set_trips(struct the
>
> lockdep_assert_held(&tz->lock);
>
> - if (!tz->ops->set_trips)
> + if (!tz->ops.set_trips)
> return;
>
> for_each_trip(tz, trip) {
> @@ -114,7 +114,7 @@ void __thermal_zone_set_trips(struct the
> * Set a temperature window. When this window is left the driver
> * must inform the thermal core via thermal_zone_device_update.
> */
> - ret = tz->ops->set_trips(tz, low, high);
> + ret = tz->ops.set_trips(tz, low, high);
> if (ret)
> dev_err(&tz->device, "Failed to set trips: %d\n", ret);
> }
> Index: linux-pm/drivers/thermal/thermal_hwmon.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_hwmon.c
> +++ linux-pm/drivers/thermal/thermal_hwmon.c
> @@ -80,7 +80,7 @@ temp_crit_show(struct device *dev, struc
>
> mutex_lock(&tz->lock);
>
> - ret = tz->ops->get_crit_temp(tz, &temperature);
> + ret = tz->ops.get_crit_temp(tz, &temperature);
>
> mutex_unlock(&tz->lock);
>
> @@ -132,7 +132,7 @@ thermal_hwmon_lookup_temp(const struct t
> static bool thermal_zone_crit_temp_valid(struct thermal_zone_device *tz)
> {
> int temp;
> - return tz->ops->get_crit_temp && !tz->ops->get_crit_temp(tz, &temp);
> + return tz->ops.get_crit_temp && !tz->ops.get_crit_temp(tz, &temp);
> }
>
> int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
>
>
>