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

Guenter Roeck
2021-06-20T17:23:41Z

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
>