Re: [PATCH 3/3] thermal: of: notify sensor driver on trip updates

[Eduardo Valentin]

Hello Navneet,

On Wed, Nov 26, 2014 at 05:16:29PM -0800, Navneet Kumar wrote:
> From: navneet kumar <navneetk@xxxxxxxxxx>
> 
> some thermal sensor hardwares include logic which
> can raise interrupts at certain programmed temperature
> thresholds.
> 
> Drivers for such sensors should be able to learn the
> appropriate threshold temperatures for interrupts by querying
> the thermal framework.
> 
> This change provides a mechanism to allow a sensor driver to
> update it's thresholds when userspace changes a trip point
> temperature.
> 
> While this behavior may not make sense in thermal zones
> with more than one sensor, no such examples exist in
> the kernel.
> 
> Signed-off-by: navneet kumar <navneetk@xxxxxxxxxx>
> ---
>  drivers/thermal/of-thermal.c | 7 +++++++
>  include/linux/thermal.h      | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index 3d47a0cf3825..3568e4a586dc 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -258,6 +258,9 @@ static int of_thermal_set_trip_temp(struct thermal_zone_device *tz, int trip,
>  	/* thermal framework should take care of data->mask & (1 << trip) */
>  	data->trips[trip].temperature = temp;
>  
> +	if (data->sops.trip_update)
> +		data->sops.trip_update(data->sensor_data, trip);
> +
>  	return 0;
>  }
>  
> @@ -285,6 +288,9 @@ static int of_thermal_set_trip_hyst(struct thermal_zone_device *tz, int trip,
>  	/* thermal framework should take care of data->mask & (1 << trip) */
>  	data->trips[trip].hysteresis = hyst;
>  
> +	if (data->sops.trip_update)
> +		data->sops.trip_update(data->sensor_data, trip);
> +
>  	return 0;
>  }
>  
> @@ -500,6 +506,7 @@ void thermal_zone_of_sensor_unregister(struct device *dev,
>  
>  	tz->sops.get_temp = NULL;
>  	tz->sops.get_trend = NULL;
> +	tz->sops.trip_update = NULL;
>  	tz->sensor_data = NULL;
>  	mutex_unlock(&tzd->lock);
>  }
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 58341c56a01f..b93e65815175 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -292,6 +292,7 @@ struct thermal_genl_event {
>  struct thermal_of_sensor_ops {
>  	int (*get_temp)(void *, long *);
>  	int (*get_trend)(void *, long *);
> +	int (*trip_update)(void *, int);

First thing I ask you is to update your work on top of my -linus branch,
as I already mentioned. Reasoning is that part of the changes you are
sending is already there.

As for this new callback, I am fine with it as long as it is also
available for drivers that do not use of-thermal. Once again, of-thermal
is not a competitor of thermal core. It will never be. It is not a new
thermal API. 

That said, it does not make sense to have functionality in of-thermal that
do not belong to thermal core. Exceptions are, of course, for helping
doing the same operations we already have in thermal core.

All the best,

Eduardo Valentin

>  };
>  
>  /* Function declarations */
> -- 
> 1.8.1.5
>

Attachment: signature.asc
Description: Digital signature