[4/5] thermal/core: Move the thermal trip code to a dedicated file

  The thermal_core.c files contains a lot of functions handling
different thermal components like the governors, the trip points, the
cooling device, the OF cooling device, etc ...

This organization does not help to migrate to a more sane code where
there is a better self-encapsulation as all the components' internals
can be directly accessed from a single file.

For the sake of clarity, let's move the thermal trip points code in a
dedicated thermal_trip.c file and add a function to browse all the
trip points like we do with the thermal zones, the govenors and the
cooling devices.

The same can be done for the cooling devices and the governor code but
that will come later as the current work in the thermal framework is
to fix the trip point handling and use a generic trip point structure.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/Makefile          |   4 +-
 drivers/thermal/thermal_core.c    |  87 --------------
 drivers/thermal/thermal_helpers.c |  62 ----------
 drivers/thermal/thermal_trip.c    | 181 ++++++++++++++++++++++++++++++
 4 files changed, 183 insertions(+), 151 deletions(-)
 create mode 100644 drivers/thermal/thermal_trip.c
  

Hi Daniel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on rafael-pm/thermal]
[also build test WARNING on next-20230118]
[cannot apply to linus/master v6.2-rc4]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Lezcano/thermal-core-Remove-unneeded-ida_destroy/20230119-051422
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git thermal
patch link:    https://lore.kernel.org/r/20230118211123.111493-4-daniel.lezcano%40linaro.org
patch subject: [PATCH 4/5] thermal/core: Move the thermal trip code to a dedicated file
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20230119/202301191017.G3eBi85j-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/039f53e8f2c0e0711f85537055e169fb216a29a9
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Daniel-Lezcano/thermal-core-Remove-unneeded-ida_destroy/20230119-051422
        git checkout 039f53e8f2c0e0711f85537055e169fb216a29a9
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/thermal/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/thermal/thermal_trip.c:12:5: warning: no previous prototype for '__for_each_thermal_trip' [-Wmissing-prototypes]
      12 | int __for_each_thermal_trip(struct thermal_zone_device *tz,
         |     ^~~~~~~~~~~~~~~~~~~~~~~


vim +/__for_each_thermal_trip +12 drivers/thermal/thermal_trip.c

    11	
  > 12	int __for_each_thermal_trip(struct thermal_zone_device *tz,
    13				    int (*cb)(struct thermal_trip *,
    14					      int trip_id, void *),
    15				    void *data)
    16	{
    17		int i, ret;
    18		struct thermal_trip trip;
    19	
    20		for (i = 0; i < tz->num_trips; i++) {
    21	
    22			ret = __thermal_zone_get_trip(tz, i, &trip);
    23			if (ret)
    24				return ret;
    25			
    26			ret = cb(&trip, i, data);
    27			if (ret)
    28				return ret;
    29		}
    30	
    31		return 0;
    32	}
    33
  

Hi, Daniel,

Just one minor comment below.

On Wed, 2023-01-18 at 22:11 +0100, Daniel Lezcano wrote:
> The thermal_core.c files contains a lot of functions handling
> different thermal components like the governors, the trip points, the
> cooling device, the OF cooling device, etc ...
> 
> This organization does not help to migrate to a more sane code where
> there is a better self-encapsulation as all the components' internals
> can be directly accessed from a single file.
> 
> For the sake of clarity, let's move the thermal trip points code in a
> dedicated thermal_trip.c file and add a function to browse all the
> trip points like we do with the thermal zones, the govenors and the
> cooling devices.
> 
> The same can be done for the cooling devices and the governor code
> but
> that will come later as the current work in the thermal framework is
> to fix the trip point handling and use a generic trip point
> structure.

"No functional changes intended."

thanks,
rui

> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/thermal/Makefile          |   4 +-
>  drivers/thermal/thermal_core.c    |  87 --------------
>  drivers/thermal/thermal_helpers.c |  62 ----------
>  drivers/thermal/thermal_trip.c    | 181
> ++++++++++++++++++++++++++++++
>  4 files changed, 183 insertions(+), 151 deletions(-)
>  create mode 100644 drivers/thermal/thermal_trip.c
> 
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 60f0dfa9aae2..2f0db88240b5 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -4,8 +4,8 @@
>  #
>  
>  obj-$(CONFIG_THERMAL)		+= thermal_sys.o
> -thermal_sys-y			+= thermal_core.o
> thermal_sysfs.o \
> -					thermal_helpers.o
> +thermal_sys-y			+= thermal_core.o
> thermal_sysfs.o	
> +thermal_sys-y			+= thermal_trip.o
> thermal_helpers.o
>  
>  # netlink interface to manage the thermal framework
>  thermal_sys-$(CONFIG_THERMAL_NETLINK)		+=
> thermal_netlink.o
> diff --git a/drivers/thermal/thermal_core.c
> b/drivers/thermal/thermal_core.c
> index ea78c93277be..d0577685085a 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1155,12 +1155,6 @@ static void thermal_set_delay_jiffies(unsigned
> long *delay_jiffies, int delay_ms
>  		*delay_jiffies = round_jiffies(*delay_jiffies);
>  }
>  
> -int thermal_zone_get_num_trips(struct thermal_zone_device *tz)
> -{
> -	return tz->num_trips;
> -}
> -EXPORT_SYMBOL_GPL(thermal_zone_get_num_trips);
> -
>  int thermal_zone_get_crit_temp(struct thermal_zone_device *tz, int
> *temp)
>  {
>  	int i, ret = -EINVAL;
> @@ -1187,87 +1181,6 @@ int thermal_zone_get_crit_temp(struct
> thermal_zone_device *tz, int *temp)
>  }
>  EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp);
>  
> -int __thermal_zone_get_trip(struct thermal_zone_device *tz, int
> trip_id,
> -			    struct thermal_trip *trip)
> -{
> -	int ret;
> -
> -	if (!tz || trip_id < 0 || trip_id >= tz->num_trips || !trip)
> -		return -EINVAL;
> -
> -	if (tz->trips) {
> -		*trip = tz->trips[trip_id];
> -		return 0;
> -	}
> -
> -	if (tz->ops->get_trip_hyst) {
> -		ret = tz->ops->get_trip_hyst(tz, trip_id, &trip-
> >hysteresis);
> -		if (ret)
> -			return ret;
> -	} else {
> -		trip->hysteresis = 0;
> -	}
> -
> -	ret = tz->ops->get_trip_temp(tz, trip_id, &trip->temperature);
> -	if (ret)
> -		return ret;
> -
> -	return tz->ops->get_trip_type(tz, trip_id, &trip->type);
> -}
> -EXPORT_SYMBOL_GPL(__thermal_zone_get_trip);
> -
> -int thermal_zone_get_trip(struct thermal_zone_device *tz, int
> trip_id,
> -			  struct thermal_trip *trip)
> -{
> -	int ret;
> -
> -	mutex_lock(&tz->lock);
> -	ret = __thermal_zone_get_trip(tz, trip_id, trip);
> -	mutex_unlock(&tz->lock);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(thermal_zone_get_trip);
> -
> -int thermal_zone_set_trip(struct thermal_zone_device *tz, int
> trip_id,
> -			  const struct thermal_trip *trip)
> -{
> -	struct thermal_trip t;
> -	int ret;
> -
> -	if (!tz->ops->set_trip_temp && !tz->ops->set_trip_hyst && !tz-
> >trips)
> -		return -EINVAL;
> -
> -	ret = __thermal_zone_get_trip(tz, trip_id, &t);
> -	if (ret)
> -		return ret;
> -
> -	if (t.type != trip->type)
> -		return -EINVAL;
> -
> -	if (t.temperature != trip->temperature && tz->ops-
> >set_trip_temp) {
> -		ret = tz->ops->set_trip_temp(tz, trip_id, trip-
> >temperature);
> -		if (ret)
> -			return ret;
> -	}
> -
> -	if (t.hysteresis != trip->hysteresis && tz->ops->set_trip_hyst) 
> {
> -		ret = tz->ops->set_trip_hyst(tz, trip_id, trip-
> >hysteresis);
> -		if (ret)
> -			return ret;
> -	}
> -
> -	if (tz->trips && (t.temperature != trip->temperature ||
> t.hysteresis != trip->hysteresis))
> -		tz->trips[trip_id] = *trip;
> -
> -	thermal_notify_tz_trip_change(tz->id, trip_id, trip->type,
> -				      trip->temperature, trip-
> >hysteresis);
> -
> -	__thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED);
> -	
> -	return 0;
> -}
> -
>  /**
>   * thermal_zone_device_register_with_trips() - register a new
> thermal zone device
>   * @type:	the thermal zone device type
> diff --git a/drivers/thermal/thermal_helpers.c
> b/drivers/thermal/thermal_helpers.c
> index 8977d5ddc23c..0f648131b0b5 100644
> --- a/drivers/thermal/thermal_helpers.c
> +++ b/drivers/thermal/thermal_helpers.c
> @@ -146,68 +146,6 @@ int thermal_zone_get_temp(struct
> thermal_zone_device *tz, int *temp)
>  }
>  EXPORT_SYMBOL_GPL(thermal_zone_get_temp);
>  
> -/**
> - * __thermal_zone_set_trips - Computes the next trip points for the
> driver
> - * @tz: a pointer to a thermal zone device structure
> - *
> - * The function computes the next temperature boundaries by browsing
> - * the trip points. The result is the closer low and high trip
> points
> - * to the current temperature. These values are passed to the
> backend
> - * driver to let it set its own notification mechanism (usually an
> - * interrupt).
> - *
> - * This function must be called with tz->lock held. Both tz and tz-
> >ops
> - * must be valid pointers.
> - *
> - * It does not return a value
> - */
> -void __thermal_zone_set_trips(struct thermal_zone_device *tz)
> -{
> -	struct thermal_trip trip;
> -	int low = -INT_MAX, high = INT_MAX;
> -	int i, ret;
> -
> -	lockdep_assert_held(&tz->lock);
> -
> -	if (!tz->ops->set_trips)
> -		return;
> -
> -	for (i = 0; i < tz->num_trips; i++) {
> -		int trip_low;
> -
> -		ret = __thermal_zone_get_trip(tz, i , &trip);
> -		if (ret)
> -			return;
> -
> -		trip_low = trip.temperature - trip.hysteresis;
> -
> -		if (trip_low < tz->temperature && trip_low > low)
> -			low = trip_low;
> -
> -		if (trip.temperature > tz->temperature &&
> -		    trip.temperature < high)
> -			high = trip.temperature;
> -	}
> -
> -	/* No need to change trip points */
> -	if (tz->prev_low_trip == low && tz->prev_high_trip == high)
> -		return;
> -
> -	tz->prev_low_trip = low;
> -	tz->prev_high_trip = high;
> -
> -	dev_dbg(&tz->device,
> -		"new temperature boundaries: %d < x < %d\n", low,
> high);
> -
> -	/*
> -	 * 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);
> -	if (ret)
> -		dev_err(&tz->device, "Failed to set trips: %d\n", ret);
> -}
> -
>  static void thermal_cdev_set_cur_state(struct thermal_cooling_device
> *cdev,
>  				       int target)
>  {
> diff --git a/drivers/thermal/thermal_trip.c
> b/drivers/thermal/thermal_trip.c
> new file mode 100644
> index 000000000000..2ef61ff7ffc3
> --- /dev/null
> +++ b/drivers/thermal/thermal_trip.c
> @@ -0,0 +1,181 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *  Copyright (C) 2008 Intel Corp
> + *  Copyright (C) 2008 Zhang Rui <rui.zhang@intel.com>
> + *  Copyright (C) 2008 Sujith Thomas <sujith.thomas@intel.com>
> + *  Copyright 2022 Linaro Limited
> + *
> + * Thermal trips handling
> + */
> +#include "thermal_core.h"
> +
> +int __for_each_thermal_trip(struct thermal_zone_device *tz,
> +			    int (*cb)(struct thermal_trip *,
> +				      int trip_id, void *),
> +			    void *data)
> +{
> +	int i, ret;
> +	struct thermal_trip trip;
> +
> +	for (i = 0; i < tz->num_trips; i++) {
> +
> +		ret = __thermal_zone_get_trip(tz, i, &trip);
> +		if (ret)
> +			return ret;
> +		
> +		ret = cb(&trip, i, data);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +int thermal_zone_get_num_trips(struct thermal_zone_device *tz)
> +{
> +	return tz->num_trips;
> +}
> +EXPORT_SYMBOL_GPL(thermal_zone_get_num_trips);
> +
> +/**
> + * __thermal_zone_set_trips - Computes the next trip points for the
> driver
> + * @tz: a pointer to a thermal zone device structure
> + *
> + * The function computes the next temperature boundaries by browsing
> + * the trip points. The result is the closer low and high trip
> points
> + * to the current temperature. These values are passed to the
> backend
> + * driver to let it set its own notification mechanism (usually an
> + * interrupt).
> + *
> + * This function must be called with tz->lock held. Both tz and tz-
> >ops
> + * must be valid pointers.
> + *
> + * It does not return a value
> + */
> +void __thermal_zone_set_trips(struct thermal_zone_device *tz)
> +{
> +	struct thermal_trip trip;
> +	int low = -INT_MAX, high = INT_MAX;
> +	int i, ret;
> +
> +	lockdep_assert_held(&tz->lock);
> +
> +	if (!tz->ops->set_trips)
> +		return;
> +
> +	for (i = 0; i < tz->num_trips; i++) {
> +		int trip_low;
> +
> +		ret = __thermal_zone_get_trip(tz, i , &trip);
> +		if (ret)
> +			return;
> +
> +		trip_low = trip.temperature - trip.hysteresis;
> +
> +		if (trip_low < tz->temperature && trip_low > low)
> +			low = trip_low;
> +
> +		if (trip.temperature > tz->temperature &&
> +		    trip.temperature < high)
> +			high = trip.temperature;
> +	}
> +
> +	/* No need to change trip points */
> +	if (tz->prev_low_trip == low && tz->prev_high_trip == high)
> +		return;
> +
> +	tz->prev_low_trip = low;
> +	tz->prev_high_trip = high;
> +
> +	dev_dbg(&tz->device,
> +		"new temperature boundaries: %d < x < %d\n", low,
> high);
> +
> +	/*
> +	 * 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);
> +	if (ret)
> +		dev_err(&tz->device, "Failed to set trips: %d\n", ret);
> +}
> +
> +int __thermal_zone_get_trip(struct thermal_zone_device *tz, int
> trip_id,
> +			    struct thermal_trip *trip)
> +{
> +	int ret;
> +
> +	if (!tz || trip_id < 0 || trip_id >= tz->num_trips || !trip)
> +		return -EINVAL;
> +
> +	if (tz->trips) {
> +		*trip = tz->trips[trip_id];
> +		return 0;
> +	}
> +
> +	if (tz->ops->get_trip_hyst) {
> +		ret = tz->ops->get_trip_hyst(tz, trip_id, &trip-
> >hysteresis);
> +		if (ret)
> +			return ret;
> +	} else {
> +		trip->hysteresis = 0;
> +	}
> +
> +	ret = tz->ops->get_trip_temp(tz, trip_id, &trip->temperature);
> +	if (ret)
> +		return ret;
> +
> +	return tz->ops->get_trip_type(tz, trip_id, &trip->type);
> +}
> +EXPORT_SYMBOL_GPL(__thermal_zone_get_trip);
> +
> +int thermal_zone_get_trip(struct thermal_zone_device *tz, int
> trip_id,
> +			  struct thermal_trip *trip)
> +{
> +	int ret;
> +
> +	mutex_lock(&tz->lock);
> +	ret = __thermal_zone_get_trip(tz, trip_id, trip);
> +	mutex_unlock(&tz->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(thermal_zone_get_trip);
> +
> +int thermal_zone_set_trip(struct thermal_zone_device *tz, int
> trip_id,
> +			  const struct thermal_trip *trip)
> +{
> +	struct thermal_trip t;
> +	int ret;
> +
> +	if (!tz->ops->set_trip_temp && !tz->ops->set_trip_hyst && !tz-
> >trips)
> +		return -EINVAL;
> +
> +	ret = __thermal_zone_get_trip(tz, trip_id, &t);
> +	if (ret)
> +		return ret;
> +
> +	if (t.type != trip->type)
> +		return -EINVAL;
> +
> +	if (t.temperature != trip->temperature && tz->ops-
> >set_trip_temp) {
> +		ret = tz->ops->set_trip_temp(tz, trip_id, trip-
> >temperature);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (t.hysteresis != trip->hysteresis && tz->ops->set_trip_hyst) 
> {
> +		ret = tz->ops->set_trip_hyst(tz, trip_id, trip-
> >hysteresis);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (tz->trips && (t.temperature != trip->temperature ||
> t.hysteresis != trip->hysteresis))
> +		tz->trips[trip_id] = *trip;
> +
> +	thermal_notify_tz_trip_change(tz->id, trip_id, trip->type,
> +				      trip->temperature, trip-
> >hysteresis);
> +
> +	__thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED);
> +	
> +	return 0;
> +}