Re: [PATCH v1 1/2] hwmon: Support set_trips() of thermal device ops

[Guenter Roeck <linux@roeck-us.net>]

On Sun, Jun 20, 2021 at 07:12:22PM +0300, Dmitry Osipenko wrote:
> Support set_trips() callback of thermal device ops. This allows HWMON
> device to operatively notify thermal core about temperature changes, which
> is very handy to have in a case where HWMON sensor is used by CPU thermal
> zone that performs passive cooling and emergency shutdown on overheat.
> Thermal core will be able to react faster to temperature changes.
> 

Why would this require a driver callback, and why can it not be handled
in the hwmon core alone ? The hwmon core could register a set_trip function
if the chip (driver) supports setting low and high limits, and it could
call the appropriate driver functions when hwmon_thermal_set_trips()
is called.

Guenter

> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/hwmon/hwmon.c | 12 ++++++++++++
>  include/linux/hwmon.h |  9 +++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> index fd47ab4e6892..4bd39ed86877 100644
> --- a/drivers/hwmon/hwmon.c
> +++ b/drivers/hwmon/hwmon.c
> @@ -153,8 +153,20 @@ static int hwmon_thermal_get_temp(void *data, int *temp)
>  	return 0;
>  }
>  
> +static int hwmon_thermal_set_trips(void *data, int low, int high)
> +{
> +	struct hwmon_thermal_data *tdata = data;
> +	struct hwmon_device *hwdev = to_hwmon_device(tdata->dev);
> +
> +	if (!hwdev->chip->ops->set_trips)
> +		return 0;
> +
> +	return hwdev->chip->ops->set_trips(tdata->dev, tdata->index, low, high);
> +}
> +
>  static const struct thermal_zone_of_device_ops hwmon_thermal_ops = {
>  	.get_temp = hwmon_thermal_get_temp,
> +	.set_trips = hwmon_thermal_set_trips,
>  };
>  
>  static void hwmon_thermal_remove_sensor(void *data)
> diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
> index 1e8d6ea8992e..7e5afcbf713d 100644
> --- a/include/linux/hwmon.h
> +++ b/include/linux/hwmon.h
> @@ -390,6 +390,14 @@ enum hwmon_intrusion_attributes {
>   *			Channel number
>   *		@val:	Value to write
>   *		The function returns 0 on success or a negative error number.
> + * @set_trips:	Callback to set temperature trips. Optional.
> + *		Parameters are:
> + *		@dev:	Pointer to hardware monitoring device
> + *		@channel:
> + *			Channel number
> + *		@low:	Low temperature trip
> + *		@high:	High temperature trip
> + *		The function returns 0 on success or a negative error number.
>   */
>  struct hwmon_ops {
>  	umode_t (*is_visible)(const void *drvdata, enum hwmon_sensor_types type,
> @@ -400,6 +408,7 @@ struct hwmon_ops {
>  		    u32 attr, int channel, const char **str);
>  	int (*write)(struct device *dev, enum hwmon_sensor_types type,
>  		     u32 attr, int channel, long val);
> +	int (*set_trips)(struct device *dev, int channel, int low, int high);
>  };
>  
>  /**
> -- 
> 2.30.2
>