Re: [PATCH] of-thermal: Disable polling when interrupt property is found in DT

[Daniel Lezcano]

On 16/10/2019 01:13, Amit Kucheria wrote:
> Currently, in order to enable interrupt-only mode, one must set
> polling-delay-passive and polling-delay properties in the DT to 0,
> otherwise the thermal framework will continue to setup a periodic timers
> to monitor the thermal zones.
> 
> Change the behaviour, so that on DT-based systems, we no longer have to
> set the properties to zero if we find an 'interrupt' property in the
> sensor.
> 
> Following data shows the number of times
> thermal_zone_device_set_polling() is invoked with and without this
> patch. So the patch achieves the same behaviour as setting the delay
> properties to 0.
> 
> Current behaviour (without setting delay properties to 0):
>   FUNC                              COUNT
>   thermal_zone_device_update          302
>   thermal_zone_device_set_pollin     7911
> 
> Current behaviour (with delay properties set to 0):
>   FUNC                              COUNT
>   thermal_zone_device_update            3
>   thermal_zone_device_set_pollin        6
> 
> With this patch (without setting delay properties to 0):
>   FUNC                              COUNT
>   thermal_zone_device_update            3
>   thermal_zone_device_set_pollin        6
> 
> Suggested-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> Signed-off-by: Amit Kucheria <amit.kucheria@xxxxxxxxxx>
> ---
>  drivers/thermal/of-thermal.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index dc5093be553e..79ad587462b1 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -412,7 +412,8 @@ static struct thermal_zone_device_ops of_thermal_ops = {
>  static struct thermal_zone_device *
>  thermal_zone_of_add_sensor(struct device_node *zone,
>  			   struct device_node *sensor, void *data,
> -			   const struct thermal_zone_of_device_ops *ops)
> +			   const struct thermal_zone_of_device_ops *ops,
> +			   bool force_interrupts)
>  {
>  	struct thermal_zone_device *tzd;
>  	struct __thermal_zone *tz;
> @@ -433,6 +434,11 @@ thermal_zone_of_add_sensor(struct device_node *zone,
>  	tzd->ops->get_temp = of_thermal_get_temp;
>  	tzd->ops->get_trend = of_thermal_get_trend;
>  
> +	if (force_interrupts) {
> +		tz->passive_delay = 0;
> +		tz->polling_delay = 0;
> +	}
> +
>  	/*
>  	 * The thermal zone core will calculate the window if they have set the
>  	 * optional set_trips pointer.
> @@ -486,6 +492,7 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
>  {
>  	struct device_node *np, *child, *sensor_np;
>  	struct thermal_zone_device *tzd = ERR_PTR(-ENODEV);
> +	bool force_interrupts = false;
>  
>  	np = of_find_node_by_name(NULL, "thermal-zones");
>  	if (!np)
> @@ -498,6 +505,9 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
>  
>  	sensor_np = of_node_get(dev->of_node);
>  
> +	if (of_find_property(sensor_np, "interrupts", NULL))
> +		force_interrupts = true;
> +

This is hackish. It does cover only DT drivers.

It would be cleaner to add a specific flag describing the thermal sensor
driver mode and then in the core code do not start the timers if the
associated works in interrupt.

Moreover the interrupt mode can be used to activate faster the passive
delay monitoring after reaching a threshold like the hikey and hikey960.
So this patch will disable the mitigation on those boards. This is
another argument to add flags for the thermal sensor mode.

[ ... ]


-- 
 <http://www.linaro.org/> Linaro.org â Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog